feat: migrate enterprise-contract e2e tests from konflux-ci/e2e-tests#1
Open
cuipinghuo wants to merge 1 commit into
Open
feat: migrate enterprise-contract e2e tests from konflux-ci/e2e-tests#1cuipinghuo wants to merge 1 commit into
cuipinghuo wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughThis PR introduces a complete end-to-end testing framework for Conforma, implementing Kubernetes/Tekton client abstractions, test utilities for pipeline generation and assertion, and a contract verification test suite deployed via Tekton pipelines. The framework validates Enterprise Contract enforcement through image build, cryptographic attestation, policy verification, and release policy checks. ChangesE2E Testing Framework & Infrastructure
🎯 4 (Complex) | ⏱️ ~75 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
cuipinghuo
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (1)
integration-tests/pipelines/konflux-e2e-tests-pipeline.yaml (1)
2-2: ⚡ Quick winUse the stable v1 API instead of v1beta1.
The Pipeline uses
apiVersion: tekton.dev/v1beta1while the Go code throughout this PR usesgithub.com/tektoncd/pipeline/pkg/apis/pipeline/v1. For consistency and to follow Tekton best practices, new resources should use the stable v1 API.♻️ Proposed fix
--- -apiVersion: tekton.dev/v1beta1 +apiVersion: tekton.dev/v1 kind: Pipeline🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@integration-tests/pipelines/konflux-e2e-tests-pipeline.yaml` at line 2, The Pipeline manifest uses the old apiVersion string "tekton.dev/v1beta1"; update that value to the stable "tekton.dev/v1" in the manifest (replace the "apiVersion: tekton.dev/v1beta1" entry), ensuring the resource matches the Go types (github.com/tektoncd/pipeline/pkg/apis/pipeline/v1) used elsewhere and revalidate the YAML after the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@e2e-tests/pkg/clients/common/controller.go`:
- Around line 241-242: The call maps.Copy(ns.Labels, requiredLabels) can panic
if ns.Labels is nil; before calling maps.Copy inside the function that updates
the namespace (the code referencing ns.Labels and requiredLabels), ensure
ns.Labels is initialized (e.g., if ns.Labels == nil { ns.Labels =
make(map[string]string) }) or replace maps.Copy with a safe loop that creates
ns.Labels if nil and then sets each key from requiredLabels into ns.Labels so
the operation cannot dereference a nil map.
- Around line 135-143: The current loop returns early when a secret is found in
sa.Secrets even if addImagePullSecrets is true and sa.ImagePullSecrets does not
contain that secret; update the logic around the loop that scans sa.Secrets so
it does not return immediately—instead check both collections: if the secret
exists in sa.Secrets and (addImagePullSecrets is false OR the secret also exists
in sa.ImagePullSecrets) then return true,nil; otherwise ensure you append the
missing entries (append to sa.Secrets using corev1.ObjectReference{Name: secret}
and, when addImagePullSecrets is true and the secret is missing, append to
sa.ImagePullSecrets using corev1.LocalObjectReference{Name: secret}) before
returning.
In `@e2e-tests/pkg/clients/tekton/chains.go`:
- Around line 20-45: The current GetTektonChainsNamespace uses sync.Once and
writes resolvedChainsErr on the first transient failure, permanently caching the
error; replace this one-shot behavior with a retryable, mutex-protected cache:
remove resolvedChainsOnce, add a sync.Mutex (e.g., resolvedChainsMu) and a
boolean or string check (resolvedChainsNs != "" or resolvedChainsResolved bool)
and in GetTektonChainsNamespace lock the mutex, if a cached namespace exists
return it, otherwise probe tektonChainsNamespaceCandidates (same loop using
t.KubeInterface().CoreV1().Pods(...)) and if you find pods set resolvedChainsNs
(and resolvedChainsResolved = true) and return nil error; if you did not find
it, return an error but do NOT persistently store it in resolvedChainsErr so
subsequent calls will retry — ensure all state writes are protected by the mutex
and only successful resolution is cached.
In `@e2e-tests/pkg/clients/tekton/containers.go`:
- Around line 13-19: The current log streaming uses
req.Stream(context.Background()) and io.ReadAll(readCloser), which can hang or
OOM; change to create a cancellable timeout context (e.g., ctx, cancel :=
context.WithTimeout(context.Background(), <reasonableDuration>) and defer
cancel()) and call req.Stream(ctx) instead of context.Background(), then wrap
readCloser with an io.LimitedReader (or io.LimitReader) to cap bytes (e.g.,
maxBytes constant) before reading to avoid unbounded memory, and add "time" to
imports; keep defer readCloser.Close() as-is and ensure the timeout/limit values
are configurable or reasonable for tests.
In `@e2e-tests/pkg/clients/tekton/signing.go`:
- Around line 15-30: The update uses a new Secret (expectedSecret) which lacks
the fetched object's resourceVersion and will fail optimistic-concurrency
checks; modify the update branch to update the retrieved secret "s" instead: set
s.Data["cosign.pub"] = publicKey (or replace s.Data with expectedSecret.Data),
then call api.Update(ctx, s, metav1.UpdateOptions{}). Keep expectedSecret for
Create, but always update the existing "s" object when updating so
resourceVersion and other metadata are preserved.
In `@e2e-tests/pkg/framework/describe.go`:
- Around line 7-9: The call to ginkgo.Describe in
EnterpriseContractSuiteDescribe incorrectly passes the variadic args slice as a
single argument; change the call to forward the variadic parameters by using
args... (i.e., call ginkgo.Describe("[enterprise-contract-suite "+text+"]",
args..., ginkgo.Ordered) or reorder so ginkgo.Ordered is included appropriately)
so the variadic argument contract of ginkgo.Describe is honored.
In `@e2e-tests/pkg/framework/framework.go`:
- Around line 99-106: The pod log stream is opened with context.Background()
which provides no timeout; change it to use a bounded context (e.g., create a
context with timeout or cancel via context.WithTimeout/WithCancel) and pass that
ctx into req.Stream so the stream cannot block indefinitely; ensure you call
cancel() in a defer after creating the context and still defer podLogs.Close()
and handle context deadline/ cancellation errors from io.Copy appropriately;
update the code around the req.Stream(...) call and podLogs handling to use the
new ctx instead of context.Background().
In `@e2e-tests/pkg/utils/tekton/cosign_results.go`:
- Around line 69-77: The Quay HTTP call in getImageInfoFromQuay uses http.Get
without a timeout and does not check HTTP status codes; replace http.Get calls
(including the similar call around lines 85-87) with an http.Client that has a
sensible Timeout, perform client.Get(fmt.Sprintf(...)) instead, check
res.StatusCode and return an error for non-2xx responses before attempting to
read the body, and keep the existing defer res.Body.Close() and error wrapping
(references: getImageInfoFromQuay and quayBaseUrl).
- Around line 28-31: The code assumes imageRef contains a digest and uses
imageInfo[1] without validation, which will panic for refs without '@'; update
the parsing in cosign_results.go to validate the result of
strings.Split(imageRef, "@") (imageInfo) before accessing imageInfo[1], and
handle the non-digest case (either return an error, fallback to parsing
tag-based refs, or treat imageTagPrefix accordingly). Specifically, check
len(imageInfo) > 1 before computing imageTagPrefix, and keep the existing
extraction of imageRegistryName and imageRepoName (derived from imageInfo[0])
but ensure any downstream logic that expects a digest handles the absence of
imageInfo[1] safely.
In `@e2e-tests/pkg/utils/tekton/matchers.go`:
- Around line 121-124: The cases in DidTaskRunSucceed dereference tr without nil
checks, which panics for typed nil pointers; update both branches handling
*tektonpipeline.PipelineRunTaskRunStatus and *tektonpipeline.TaskRunStatus to
first check if tr == nil or tr.Status == nil and return false if so, otherwise
call tr.Status.GetCondition(apis.ConditionSucceeded).IsTrue(); reference the
symbols DidTaskRunSucceed, tr, Status, GetCondition, apis.ConditionSucceeded,
tektonpipeline.PipelineRunTaskRunStatus and tektonpipeline.TaskRunStatus when
making the change.
---
Nitpick comments:
In `@integration-tests/pipelines/konflux-e2e-tests-pipeline.yaml`:
- Line 2: The Pipeline manifest uses the old apiVersion string
"tekton.dev/v1beta1"; update that value to the stable "tekton.dev/v1" in the
manifest (replace the "apiVersion: tekton.dev/v1beta1" entry), ensuring the
resource matches the Go types
(github.com/tektoncd/pipeline/pkg/apis/pipeline/v1) used elsewhere and
revalidate the YAML after the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: 927c2f34-f2d9-434a-b957-210a9f4ef6e5
⛔ Files ignored due to path filters (1)
e2e-tests/go.sumis excluded by!**/*.sum
📒 Files selected for processing (22)
Makefilee2e-tests/cmd/e2e_test.goe2e-tests/go.mode2e-tests/pkg/clients/common/controller.goe2e-tests/pkg/clients/kubernetes/client.goe2e-tests/pkg/clients/tekton/bundles.goe2e-tests/pkg/clients/tekton/chains.goe2e-tests/pkg/clients/tekton/containers.goe2e-tests/pkg/clients/tekton/controller.goe2e-tests/pkg/clients/tekton/ecp.goe2e-tests/pkg/clients/tekton/pipelineruns.goe2e-tests/pkg/clients/tekton/signing.goe2e-tests/pkg/clients/tekton/taskruns.goe2e-tests/pkg/constants/constants.goe2e-tests/pkg/framework/describe.goe2e-tests/pkg/framework/framework.goe2e-tests/pkg/utils/contract/policy.goe2e-tests/pkg/utils/tekton/cosign_results.goe2e-tests/pkg/utils/tekton/generators.goe2e-tests/pkg/utils/tekton/matchers.goe2e-tests/tests/contract/contract.gointegration-tests/pipelines/konflux-e2e-tests-pipeline.yaml
cuipinghuo
force-pushed
the
migrate-from-konflux-ci
branch
2 times, most recently
from
e230a1f to
0a29ef4
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@e2e-tests/go.mod`:
- Line 43: The go-git indirect dependency github.com/go-git/go-git/v5 is pinned
to v5.13.0 which has known vulnerabilities; update that module entry to v5.19.0
or later in go.mod (replace the version specifier for
github.com/go-git/go-git/v5) and then run your Go module tooling (e.g., go get
github.com/go-git/go-git/v5@v5.19.0 and go mod tidy) to regenerate go.sum and
ensure the updated dependency is applied across the build.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: cd95dcbe-43b4-4e54-93ac-6337a816f7fd
⛔ Files ignored due to path filters (1)
e2e-tests/go.sumis excluded by!**/*.sum
📒 Files selected for processing (22)
Makefilee2e-tests/cmd/e2e_test.goe2e-tests/go.mode2e-tests/pkg/clients/common/controller.goe2e-tests/pkg/clients/kubernetes/client.goe2e-tests/pkg/clients/tekton/bundles.goe2e-tests/pkg/clients/tekton/chains.goe2e-tests/pkg/clients/tekton/containers.goe2e-tests/pkg/clients/tekton/controller.goe2e-tests/pkg/clients/tekton/ecp.goe2e-tests/pkg/clients/tekton/pipelineruns.goe2e-tests/pkg/clients/tekton/signing.goe2e-tests/pkg/clients/tekton/taskruns.goe2e-tests/pkg/constants/constants.goe2e-tests/pkg/framework/describe.goe2e-tests/pkg/framework/framework.goe2e-tests/pkg/utils/contract/policy.goe2e-tests/pkg/utils/tekton/cosign_results.goe2e-tests/pkg/utils/tekton/generators.goe2e-tests/pkg/utils/tekton/matchers.goe2e-tests/tests/contract/contract.gointegration-tests/pipelines/konflux-e2e-tests-pipeline.yaml
✅ Files skipped from review due to trivial changes (1)
- e2e-tests/pkg/constants/constants.go
🚧 Files skipped from review as they are similar to previous changes (16)
- e2e-tests/pkg/clients/tekton/controller.go
- e2e-tests/pkg/clients/tekton/signing.go
- e2e-tests/pkg/clients/tekton/bundles.go
- e2e-tests/pkg/utils/contract/policy.go
- e2e-tests/pkg/clients/tekton/ecp.go
- e2e-tests/pkg/clients/tekton/chains.go
- e2e-tests/pkg/framework/describe.go
- e2e-tests/pkg/clients/kubernetes/client.go
- e2e-tests/pkg/clients/tekton/pipelineruns.go
- e2e-tests/pkg/utils/tekton/generators.go
- e2e-tests/cmd/e2e_test.go
- e2e-tests/pkg/utils/tekton/cosign_results.go
- integration-tests/pipelines/konflux-e2e-tests-pipeline.yaml
- e2e-tests/tests/contract/contract.go
- e2e-tests/pkg/framework/framework.go
- e2e-tests/pkg/clients/tekton/taskruns.go
cuipinghuo
force-pushed
the
migrate-from-konflux-ci
branch
2 times, most recently
from
b262eea to
b5aa837
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@integration-tests/pipelines/konflux-e2e-tests-pipeline.yaml`:
- Around line 137-138: Several steps still hardcode the secret name
"konflux-test-infra" instead of using the pipeline parameter
"konflux-test-infra-secret"; update every occurrence (deploy task, both test
task specs, teardown and other places where
oci-container-repo-credentials-secret is set) to reference the parameter rather
than the literal string. Replace the value "konflux-test-infra" with the
parameter reference (use the pipeline params syntax used elsewhere in this file,
e.g., the existing konflux-test-infra-secret param) so all occurrences of
oci-container-repo-credentials-secret consistently use the parameter; ensure you
update the spots previously noted (the deploy step, both test tasks, teardown
and the additional matches referenced).
- Around line 73-74: Several git-resolved Task/TaskRef entries currently use the
mutable branch value "revision: main"; replace each occurrence of the block that
looks like "- name: revision\n value: main" with an immutable reference (a
specific commit SHA or a release tag) for every external task ref (all other
similar "revision" entries in this pipeline). Locate the TaskRef/task
definitions that include the "revision" key (the "revision: main" pairs) and
update their "value" to a fixed commit SHA or tag for the external repository
you depend on, ensuring each git-resolved task is pinned to an immutable
revision.
In `@Makefile`:
- Around line 3-9: The Makefile currently installs ginkgo but then calls the
plain "ginkgo" binary which may not be on PATH; update both targets that run
ginkgo (the top e2e test target and test-e2e-dry-run) to invoke the installed
binary explicitly by resolving GOBIN (e.g. use $(shell go env GOBIN) or fallback
to $(shell echo $$(go env GOPATH)/bin) to construct the full path to ginkgo) so
the Makefile runs the correct $(GOBIN)/ginkgo after go install.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: 9b7668eb-1318-4b44-b0b7-0095dab4c7f3
⛔ Files ignored due to path filters (1)
e2e-tests/go.sumis excluded by!**/*.sum
📒 Files selected for processing (23)
Makefilee2e-tests/cmd/e2e_test.goe2e-tests/go.mode2e-tests/pkg/clients/common/controller.goe2e-tests/pkg/clients/kubernetes/client.goe2e-tests/pkg/clients/tekton/bundles.goe2e-tests/pkg/clients/tekton/chains.goe2e-tests/pkg/clients/tekton/containers.goe2e-tests/pkg/clients/tekton/controller.goe2e-tests/pkg/clients/tekton/ecp.goe2e-tests/pkg/clients/tekton/pipelineruns.goe2e-tests/pkg/clients/tekton/signing.goe2e-tests/pkg/clients/tekton/taskruns.goe2e-tests/pkg/constants/constants.goe2e-tests/pkg/framework/describe.goe2e-tests/pkg/framework/framework.goe2e-tests/pkg/utils/contract/policy.goe2e-tests/pkg/utils/tekton/cosign_results.goe2e-tests/pkg/utils/tekton/generators.goe2e-tests/pkg/utils/tekton/matchers.goe2e-tests/tests/contract/contract.gointegration-tests/pipelineruns/conforma-e2e-manual.yamlintegration-tests/pipelines/konflux-e2e-tests-pipeline.yaml
✅ Files skipped from review due to trivial changes (2)
- integration-tests/pipelineruns/conforma-e2e-manual.yaml
- e2e-tests/pkg/constants/constants.go
🚧 Files skipped from review as they are similar to previous changes (19)
- e2e-tests/pkg/framework/describe.go
- e2e-tests/pkg/clients/tekton/signing.go
- e2e-tests/pkg/clients/tekton/containers.go
- e2e-tests/pkg/clients/tekton/ecp.go
- e2e-tests/pkg/clients/tekton/bundles.go
- e2e-tests/pkg/clients/tekton/controller.go
- e2e-tests/pkg/utils/contract/policy.go
- e2e-tests/pkg/clients/tekton/taskruns.go
- e2e-tests/pkg/clients/tekton/chains.go
- e2e-tests/pkg/utils/tekton/matchers.go
- e2e-tests/go.mod
- e2e-tests/pkg/framework/framework.go
- e2e-tests/pkg/clients/kubernetes/client.go
- e2e-tests/cmd/e2e_test.go
- e2e-tests/pkg/clients/tekton/pipelineruns.go
- e2e-tests/pkg/utils/tekton/generators.go
- e2e-tests/pkg/utils/tekton/cosign_results.go
- e2e-tests/tests/contract/contract.go
- e2e-tests/pkg/clients/common/controller.go
Comment on lines
+73
to
+74
| - name: revision | ||
| value: main |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
wc -l integration-tests/pipelines/konflux-e2e-tests-pipeline.yamlRepository: conforma/e2e-tests
Length of output: 126
🏁 Script executed:
cat -n integration-tests/pipelines/konflux-e2e-tests-pipeline.yamlRepository: conforma/e2e-tests
Length of output: 21136
Pin external task refs to immutable revisions.
Every git-resolved task here uses revision: main, which tracks a moving branch. This makes the pipeline non-reproducible and exposes it to unrelated upstream changes that can alter or break runs without any change in this repository. Pin all these references to specific commit SHAs or release tags.
Lines affected: 73–74, 92–93, 128–129, 257–258, 274–275, 395–396, 412–413, 427–428, 453–454
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@integration-tests/pipelines/konflux-e2e-tests-pipeline.yaml` around lines 73
- 74, Several git-resolved Task/TaskRef entries currently use the mutable branch
value "revision: main"; replace each occurrence of the block that looks like "-
name: revision\n value: main" with an immutable reference (a specific commit
SHA or a release tag) for every external task ref (all other similar "revision"
entries in this pipeline). Locate the TaskRef/task definitions that include the
"revision" key (the "revision: main" pairs) and update their "value" to a fixed
commit SHA or tag for the external repository you depend on, ensuring each
git-resolved task is pinned to an immutable revision.
Comment on lines
+137
to
+138
| - name: oci-container-repo-credentials-secret | ||
| value: konflux-test-infra |
There was a problem hiding this comment.
Use konflux-test-infra-secret consistently.
The pipeline accepts konflux-test-infra-secret, but deploy, both test task specs, and teardown still hardcode konflux-test-infra. Any environment that overrides the param will provision successfully and then fail later when those steps read a different secret.
Also applies to: 176-178, 314-316, 440-441
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@integration-tests/pipelines/konflux-e2e-tests-pipeline.yaml` around lines 137
- 138, Several steps still hardcode the secret name "konflux-test-infra" instead
of using the pipeline parameter "konflux-test-infra-secret"; update every
occurrence (deploy task, both test task specs, teardown and other places where
oci-container-repo-credentials-secret is set) to reference the parameter rather
than the literal string. Replace the value "konflux-test-infra" with the
parameter reference (use the pipeline params syntax used elsewhere in this file,
e.g., the existing konflux-test-infra-secret param) so all occurrences of
oci-container-repo-credentials-secret consistently use the parameter; ensure you
update the spots previously noted (the deploy step, both test tasks, teardown
and the additional matches referenced).
A Tekton Pipeline provisions an ephemeral KinD cluster on AWS via mapt, deploys Konflux using the operator-based install from konflux-ci/konflux-ci, runs the Ginkgo test suite, and tears down. Includes: - Lean Go framework (CommonController + TektonController only, no OpenShift dependencies) - All 11 contract.go test scenarios ported with conforma module imports - Tekton Pipeline YAML using tekton-integration-catalog shared tasks for provisioning/deprovisioning, and konflux-ci/konflux-ci .tekton/tasks/deploy-konflux for operator-based Konflux deployment - Pipeline apiVersion set to tekton.dev/v1 - Makefile with test-e2e, build, and tidy targets Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
cuipinghuo
force-pushed
the
migrate-from-konflux-ci
branch
from
b5aa837 to
7932998
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
e2e-tests/pkg/clients/tekton/ecp.go (1)
26-26: ⚡ Quick winExtract hardcoded "ec-policy" string to a constant.
The string "ec-policy" is duplicated on Lines 26 and 33. Extract it to a package-level constant to improve maintainability and prevent inconsistencies.
♻️ Proposed refactor to eliminate duplication
Add this constant at the top of the file:
package tekton import ( "context" ecp "github.com/conforma/crds/api/v1alpha1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" crclient "sigs.k8s.io/controller-runtime/pkg/client" ) + +const defaultECPolicyName = "ec-policy"Then update the function:
func (t *TektonController) CreateOrUpdatePolicyConfiguration(namespace string, policy ecp.EnterpriseContractPolicySpec) error { ecPolicy := ecp.EnterpriseContractPolicy{ ObjectMeta: metav1.ObjectMeta{ - Name: "ec-policy", + Name: defaultECPolicyName, Namespace: namespace, }, } err := t.KubeRest().Get(context.Background(), crclient.ObjectKey{ Namespace: namespace, - Name: "ec-policy", + Name: defaultECPolicyName, }, &ecPolicy)Also applies to: 33-33
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e-tests/pkg/clients/tekton/ecp.go` at line 26, Extract the duplicated literal "ec-policy" into a package-level constant (e.g., const ecPolicyName = "ec-policy") declared at the top of ecp.go, then replace both occurrences where the Name field is set (the struct initializers referenced in the diff at the Name: "ec-policy" lines) with that constant (use ecPolicyName). Ensure any other occurrences in this file use the same constant to avoid future divergence.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.tekton/conforma-e2e-pull-request.yaml:
- Around line 10-18: pipelineRef currently resolves the pipeline YAML from the
PR head (params.url = "{{repo_url}}" and params.revision = "{{revision}}"),
which allows PR-authored pipeline changes to run with privileged secrets; change
pipelineRef to point to a trusted, immutable pipeline source by replacing the
params for pipelineRef (url and revision) with a fixed trusted repository and
branch/tag (e.g., the internal pipelines repo and its main or a pinned tag)
while leaving the PR's repo_url/revision only for the test checkout step; ensure
the pipeline pathInRepo remains the trusted pipeline path and update any
references that consume pipelineRef so the checkout of untrusted code is done as
a separate step using the PR metadata.
In `@e2e-tests/pkg/clients/tekton/ecp.go`:
- Around line 12-21: The CreateEnterpriseContractPolicy method uses
context.Background() which can hang; change it to use a timeout context (e.g.,
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)) and
defer cancel() and pass ctx into t.KubeRest().Create; add the time import if
missing and ensure the cancel is called to avoid leaks, keeping the rest of the
function (ec creation and return) unchanged and referencing
TektonController.CreateEnterpriseContractPolicy and t.KubeRest().Create.
- Around line 52-64: The GetEnterpriseContractPolicy method uses
context.Background() and can hang; replace it by creating a timeout context with
context.WithTimeout (e.g., 5–10s or a test-configurable duration), pass that
context into t.KubeRest().Get, and defer cancel() immediately after creating the
context; update the GetEnterpriseContractPolicy function to use the new ctx and
ensure the cancel is deferred to avoid leaks.
- Around line 23-50: In CreateOrUpdatePolicyConfiguration, replace the three
uses of context.Background() (used in t.KubeRest().Get, Create, and Update) with
context.WithTimeout() contexts to avoid hanging tests; for example, create a
ctx, cancel := context.WithTimeout(context.Background(), <reasonableDuration>)
before each API call (or share one ctx for the operation), pass ctx into
Get/Create/Update, and ensure you call defer cancel() (or call cancel after each
call) to release resources; update imports if needed and keep references to
t.KubeRest(), Get, Create, and Update unchanged.
In `@pipelines/konflux-e2e-tests-pipeline.yaml`:
- Around line 176-178: Two inline TaskSpecs hardcode the secret name as
konflux-test-infra (the konflux-infra-secrets-volume secret block) so overriding
params.konflux-test-infra-secret doesn't take effect; update both inline
TaskSpecs (PAC and ITS) to reference the parameter instead of the literal string
by replacing secretName: konflux-test-infra with secretName:
$(params.konflux-test-infra-secret) for the konflux-infra-secrets-volume entries
so the parametrized secret is consistently mounted.
In `@pipelines/pipelineruns/conforma-e2e-manual.yaml`:
- Around line 12-13: The YAML hardcodes the ephemeral branch value under the
"revision" param (value: migrate-from-konflux-ci); change this to a stable
default or a parameter reference instead (e.g., use a PipelineRun param like
revision with a sensible default such as main or reference
$(params.revision)/${revision}) so the template does not rely on a removed
feature branch, and update the other occurrence noted (lines 19-20) as well;
ensure you modify the "revision" param name/value usage consistently where
PipelineRun or pipeline resolution reads it so callers can override it when
needed.
---
Nitpick comments:
In `@e2e-tests/pkg/clients/tekton/ecp.go`:
- Line 26: Extract the duplicated literal "ec-policy" into a package-level
constant (e.g., const ecPolicyName = "ec-policy") declared at the top of ecp.go,
then replace both occurrences where the Name field is set (the struct
initializers referenced in the diff at the Name: "ec-policy" lines) with that
constant (use ecPolicyName). Ensure any other occurrences in this file use the
same constant to avoid future divergence.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: a1f75dc1-51b3-4643-b756-c6dcd650b09a
⛔ Files ignored due to path filters (1)
e2e-tests/go.sumis excluded by!**/*.sum
📒 Files selected for processing (24)
.tekton/conforma-e2e-pull-request.yamlMakefilee2e-tests/cmd/e2e_test.goe2e-tests/go.mode2e-tests/pkg/clients/common/controller.goe2e-tests/pkg/clients/kubernetes/client.goe2e-tests/pkg/clients/tekton/bundles.goe2e-tests/pkg/clients/tekton/chains.goe2e-tests/pkg/clients/tekton/containers.goe2e-tests/pkg/clients/tekton/controller.goe2e-tests/pkg/clients/tekton/ecp.goe2e-tests/pkg/clients/tekton/pipelineruns.goe2e-tests/pkg/clients/tekton/signing.goe2e-tests/pkg/clients/tekton/taskruns.goe2e-tests/pkg/constants/constants.goe2e-tests/pkg/framework/describe.goe2e-tests/pkg/framework/framework.goe2e-tests/pkg/utils/contract/policy.goe2e-tests/pkg/utils/tekton/cosign_results.goe2e-tests/pkg/utils/tekton/generators.goe2e-tests/pkg/utils/tekton/matchers.goe2e-tests/tests/contract/contract.gopipelines/konflux-e2e-tests-pipeline.yamlpipelines/pipelineruns/conforma-e2e-manual.yaml
✅ Files skipped from review due to trivial changes (2)
- e2e-tests/pkg/constants/constants.go
- e2e-tests/go.mod
🚧 Files skipped from review as they are similar to previous changes (17)
- e2e-tests/pkg/clients/tekton/containers.go
- e2e-tests/pkg/clients/tekton/controller.go
- e2e-tests/pkg/clients/kubernetes/client.go
- e2e-tests/cmd/e2e_test.go
- e2e-tests/pkg/utils/tekton/matchers.go
- e2e-tests/pkg/framework/framework.go
- e2e-tests/pkg/clients/tekton/signing.go
- e2e-tests/pkg/clients/tekton/bundles.go
- e2e-tests/pkg/framework/describe.go
- e2e-tests/pkg/utils/contract/policy.go
- e2e-tests/pkg/clients/tekton/chains.go
- e2e-tests/tests/contract/contract.go
- e2e-tests/pkg/clients/tekton/pipelineruns.go
- e2e-tests/pkg/utils/tekton/cosign_results.go
- e2e-tests/pkg/clients/tekton/taskruns.go
- e2e-tests/pkg/clients/common/controller.go
- e2e-tests/pkg/utils/tekton/generators.go
Comment on lines
+10
to
+18
| pipelineRef: | ||
| resolver: git | ||
| params: | ||
| - name: url | ||
| value: "{{repo_url}}" | ||
| - name: revision | ||
| value: "{{revision}}" | ||
| - name: pathInRepo | ||
| value: pipelines/konflux-e2e-tests-pipeline.yaml |
There was a problem hiding this comment.
Use a trusted pipeline source for privileged PR runs.
pipelineRef currently pulls the pipeline YAML from {{repo_url}}@{{revision}} (PR head). That lets PR-authored pipeline changes execute with privileged test secrets. Resolve the pipeline definition from a trusted repo/branch instead, and keep PR metadata only for the test checkout inputs.
Suggested minimal hardening
pipelineRef:
resolver: git
params:
- name: url
- value: "{{repo_url}}"
+ value: https://github.com/conforma/e2e-tests.git
- name: revision
- value: "{{revision}}"
+ value: main
- name: pathInRepo
value: pipelines/konflux-e2e-tests-pipeline.yamlAs per coding guidelines -Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| pipelineRef: | |
| resolver: git | |
| params: | |
| - name: url | |
| value: "{{repo_url}}" | |
| - name: revision | |
| value: "{{revision}}" | |
| - name: pathInRepo | |
| value: pipelines/konflux-e2e-tests-pipeline.yaml | |
| pipelineRef: | |
| resolver: git | |
| params: | |
| - name: url | |
| value: https://github.com/conforma/e2e-tests.git | |
| - name: revision | |
| value: main | |
| - name: pathInRepo | |
| value: pipelines/konflux-e2e-tests-pipeline.yaml |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.tekton/conforma-e2e-pull-request.yaml around lines 10 - 18, pipelineRef
currently resolves the pipeline YAML from the PR head (params.url =
"{{repo_url}}" and params.revision = "{{revision}}"), which allows PR-authored
pipeline changes to run with privileged secrets; change pipelineRef to point to
a trusted, immutable pipeline source by replacing the params for pipelineRef
(url and revision) with a fixed trusted repository and branch/tag (e.g., the
internal pipelines repo and its main or a pinned tag) while leaving the PR's
repo_url/revision only for the test checkout step; ensure the pipeline
pathInRepo remains the trusted pipeline path and update any references that
consume pipelineRef so the checkout of untrusted code is done as a separate step
using the PR metadata.
Comment on lines
+12
to
+21
| func (t *TektonController) CreateEnterpriseContractPolicy(name, namespace string, ecpolicy ecp.EnterpriseContractPolicySpec) (*ecp.EnterpriseContractPolicy, error) { | ||
| ec := &ecp.EnterpriseContractPolicy{ | ||
| ObjectMeta: metav1.ObjectMeta{ | ||
| Name: name, | ||
| Namespace: namespace, | ||
| }, | ||
| Spec: ecpolicy, | ||
| } | ||
| return ec, t.KubeRest().Create(context.Background(), ec) | ||
| } |
There was a problem hiding this comment.
Replace context.Background() with a timeout context to prevent test hangs.
Using context.Background() for Kubernetes API calls in e2e tests risks indefinite hangs if the API server is slow or unavailable, blocking CI pipelines. Use context.WithTimeout() to enforce a reasonable deadline.
⏱️ Proposed fix to add timeout context
func (t *TektonController) CreateEnterpriseContractPolicy(name, namespace string, ecpolicy ecp.EnterpriseContractPolicySpec) (*ecp.EnterpriseContractPolicy, error) {
ec := &ecp.EnterpriseContractPolicy{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Namespace: namespace,
},
Spec: ecpolicy,
}
- return ec, t.KubeRest().Create(context.Background(), ec)
+ ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
+ defer cancel()
+ return ec, t.KubeRest().Create(ctx, ec)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| func (t *TektonController) CreateEnterpriseContractPolicy(name, namespace string, ecpolicy ecp.EnterpriseContractPolicySpec) (*ecp.EnterpriseContractPolicy, error) { | |
| ec := &ecp.EnterpriseContractPolicy{ | |
| ObjectMeta: metav1.ObjectMeta{ | |
| Name: name, | |
| Namespace: namespace, | |
| }, | |
| Spec: ecpolicy, | |
| } | |
| return ec, t.KubeRest().Create(context.Background(), ec) | |
| } | |
| func (t *TektonController) CreateEnterpriseContractPolicy(name, namespace string, ecpolicy ecp.EnterpriseContractPolicySpec) (*ecp.EnterpriseContractPolicy, error) { | |
| ec := &ecp.EnterpriseContractPolicy{ | |
| ObjectMeta: metav1.ObjectMeta{ | |
| Name: name, | |
| Namespace: namespace, | |
| }, | |
| Spec: ecpolicy, | |
| } | |
| ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) | |
| defer cancel() | |
| return ec, t.KubeRest().Create(ctx, ec) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@e2e-tests/pkg/clients/tekton/ecp.go` around lines 12 - 21, The
CreateEnterpriseContractPolicy method uses context.Background() which can hang;
change it to use a timeout context (e.g., ctx, cancel :=
context.WithTimeout(context.Background(), 30*time.Second)) and defer cancel()
and pass ctx into t.KubeRest().Create; add the time import if missing and ensure
the cancel is called to avoid leaks, keeping the rest of the function (ec
creation and return) unchanged and referencing
TektonController.CreateEnterpriseContractPolicy and t.KubeRest().Create.
Comment on lines
+23
to
+50
| func (t *TektonController) CreateOrUpdatePolicyConfiguration(namespace string, policy ecp.EnterpriseContractPolicySpec) error { | ||
| ecPolicy := ecp.EnterpriseContractPolicy{ | ||
| ObjectMeta: metav1.ObjectMeta{ | ||
| Name: "ec-policy", | ||
| Namespace: namespace, | ||
| }, | ||
| } | ||
|
|
||
| err := t.KubeRest().Get(context.Background(), crclient.ObjectKey{ | ||
| Namespace: namespace, | ||
| Name: "ec-policy", | ||
| }, &ecPolicy) | ||
|
|
||
| exists := true | ||
| if err != nil { | ||
| if errors.IsNotFound(err) { | ||
| exists = false | ||
| } else { | ||
| return err | ||
| } | ||
| } | ||
|
|
||
| ecPolicy.Spec = policy | ||
| if !exists { | ||
| return t.KubeRest().Create(context.Background(), &ecPolicy) | ||
| } | ||
| return t.KubeRest().Update(context.Background(), &ecPolicy) | ||
| } |
There was a problem hiding this comment.
Replace context.Background() with timeout contexts to prevent test hangs.
Using context.Background() for the Get, Create, and Update operations risks indefinite hangs. Use context.WithTimeout() for all three API calls.
⏱️ Proposed fix to add timeout contexts
func (t *TektonController) CreateOrUpdatePolicyConfiguration(namespace string, policy ecp.EnterpriseContractPolicySpec) error {
ecPolicy := ecp.EnterpriseContractPolicy{
ObjectMeta: metav1.ObjectMeta{
Name: "ec-policy",
Namespace: namespace,
},
}
- err := t.KubeRest().Get(context.Background(), crclient.ObjectKey{
+ ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
+ defer cancel()
+ err := t.KubeRest().Get(ctx, crclient.ObjectKey{
Namespace: namespace,
Name: "ec-policy",
}, &ecPolicy)
exists := true
if err != nil {
if errors.IsNotFound(err) {
exists = false
} else {
return err
}
}
ecPolicy.Spec = policy
if !exists {
- return t.KubeRest().Create(context.Background(), &ecPolicy)
+ return t.KubeRest().Create(ctx, &ecPolicy)
}
- return t.KubeRest().Update(context.Background(), &ecPolicy)
+ return t.KubeRest().Update(ctx, &ecPolicy)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| func (t *TektonController) CreateOrUpdatePolicyConfiguration(namespace string, policy ecp.EnterpriseContractPolicySpec) error { | |
| ecPolicy := ecp.EnterpriseContractPolicy{ | |
| ObjectMeta: metav1.ObjectMeta{ | |
| Name: "ec-policy", | |
| Namespace: namespace, | |
| }, | |
| } | |
| err := t.KubeRest().Get(context.Background(), crclient.ObjectKey{ | |
| Namespace: namespace, | |
| Name: "ec-policy", | |
| }, &ecPolicy) | |
| exists := true | |
| if err != nil { | |
| if errors.IsNotFound(err) { | |
| exists = false | |
| } else { | |
| return err | |
| } | |
| } | |
| ecPolicy.Spec = policy | |
| if !exists { | |
| return t.KubeRest().Create(context.Background(), &ecPolicy) | |
| } | |
| return t.KubeRest().Update(context.Background(), &ecPolicy) | |
| } | |
| func (t *TektonController) CreateOrUpdatePolicyConfiguration(namespace string, policy ecp.EnterpriseContractPolicySpec) error { | |
| ecPolicy := ecp.EnterpriseContractPolicy{ | |
| ObjectMeta: metav1.ObjectMeta{ | |
| Name: "ec-policy", | |
| Namespace: namespace, | |
| }, | |
| } | |
| ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) | |
| defer cancel() | |
| err := t.KubeRest().Get(ctx, crclient.ObjectKey{ | |
| Namespace: namespace, | |
| Name: "ec-policy", | |
| }, &ecPolicy) | |
| exists := true | |
| if err != nil { | |
| if errors.IsNotFound(err) { | |
| exists = false | |
| } else { | |
| return err | |
| } | |
| } | |
| ecPolicy.Spec = policy | |
| if !exists { | |
| return t.KubeRest().Create(ctx, &ecPolicy) | |
| } | |
| return t.KubeRest().Update(ctx, &ecPolicy) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@e2e-tests/pkg/clients/tekton/ecp.go` around lines 23 - 50, In
CreateOrUpdatePolicyConfiguration, replace the three uses of
context.Background() (used in t.KubeRest().Get, Create, and Update) with
context.WithTimeout() contexts to avoid hanging tests; for example, create a
ctx, cancel := context.WithTimeout(context.Background(), <reasonableDuration>)
before each API call (or share one ctx for the operation), pass ctx into
Get/Create/Update, and ensure you call defer cancel() (or call cancel after each
call) to release resources; update imports if needed and keep references to
t.KubeRest(), Get, Create, and Update unchanged.
Comment on lines
+52
to
+64
| func (t *TektonController) GetEnterpriseContractPolicy(name, namespace string) (*ecp.EnterpriseContractPolicy, error) { | ||
| ecPolicy := ecp.EnterpriseContractPolicy{ | ||
| ObjectMeta: metav1.ObjectMeta{ | ||
| Name: name, | ||
| Namespace: namespace, | ||
| }, | ||
| } | ||
| err := t.KubeRest().Get(context.Background(), crclient.ObjectKey{ | ||
| Namespace: namespace, | ||
| Name: name, | ||
| }, &ecPolicy) | ||
| return &ecPolicy, err | ||
| } |
There was a problem hiding this comment.
Replace context.Background() with a timeout context to prevent test hangs.
Using context.Background() for the Get operation risks indefinite hangs. Use context.WithTimeout() to enforce a reasonable deadline.
⏱️ Proposed fix to add timeout context
func (t *TektonController) GetEnterpriseContractPolicy(name, namespace string) (*ecp.EnterpriseContractPolicy, error) {
ecPolicy := ecp.EnterpriseContractPolicy{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Namespace: namespace,
},
}
- err := t.KubeRest().Get(context.Background(), crclient.ObjectKey{
+ ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
+ defer cancel()
+ err := t.KubeRest().Get(ctx, crclient.ObjectKey{
Namespace: namespace,
Name: name,
}, &ecPolicy)
return &ecPolicy, err
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| func (t *TektonController) GetEnterpriseContractPolicy(name, namespace string) (*ecp.EnterpriseContractPolicy, error) { | |
| ecPolicy := ecp.EnterpriseContractPolicy{ | |
| ObjectMeta: metav1.ObjectMeta{ | |
| Name: name, | |
| Namespace: namespace, | |
| }, | |
| } | |
| err := t.KubeRest().Get(context.Background(), crclient.ObjectKey{ | |
| Namespace: namespace, | |
| Name: name, | |
| }, &ecPolicy) | |
| return &ecPolicy, err | |
| } | |
| func (t *TektonController) GetEnterpriseContractPolicy(name, namespace string) (*ecp.EnterpriseContractPolicy, error) { | |
| ecPolicy := ecp.EnterpriseContractPolicy{ | |
| ObjectMeta: metav1.ObjectMeta{ | |
| Name: name, | |
| Namespace: namespace, | |
| }, | |
| } | |
| ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) | |
| defer cancel() | |
| err := t.KubeRest().Get(ctx, crclient.ObjectKey{ | |
| Namespace: namespace, | |
| Name: name, | |
| }, &ecPolicy) | |
| return &ecPolicy, err | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@e2e-tests/pkg/clients/tekton/ecp.go` around lines 52 - 64, The
GetEnterpriseContractPolicy method uses context.Background() and can hang;
replace it by creating a timeout context with context.WithTimeout (e.g., 5–10s
or a test-configurable duration), pass that context into t.KubeRest().Get, and
defer cancel() immediately after creating the context; update the
GetEnterpriseContractPolicy function to use the new ctx and ensure the cancel is
deferred to avoid leaks.
Comment on lines
+176
to
+178
| - name: konflux-infra-secrets-volume | ||
| secret: | ||
| secretName: konflux-test-infra |
There was a problem hiding this comment.
Honor konflux-test-infra-secret consistently in both inline taskSpecs.
Both e2e taskSpecs hardcode secretName: konflux-test-infra, so overriding params.konflux-test-infra-secret won’t propagate and can break credential mounting paths.
Suggested fix pattern (apply to PAC and ITS taskSpecs)
params:
- name: git-url
value: "$(params.git-url)"
- name: git-revision
value: "$(params.revision)"
- name: oras-container
value: "$(params.oci-container-repo):$(context.pipelineRun.name)"
- name: cluster-access-secret-name
value: kfg-$(context.pipelineRun.name)
+ - name: konflux-test-infra-secret
+ value: "$(params.konflux-test-infra-secret)"
taskSpec:
params:
- name: git-url
type: string
- name: git-revision
type: string
- name: oras-container
type: string
- name: cluster-access-secret-name
type: string
+ - name: konflux-test-infra-secret
+ type: string
volumes:
- name: konflux-infra-secrets-volume
secret:
- secretName: konflux-test-infra
+ secretName: $(params.konflux-test-infra-secret)As per coding guidelines -Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Also applies to: 314-316
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pipelines/konflux-e2e-tests-pipeline.yaml` around lines 176 - 178, Two inline
TaskSpecs hardcode the secret name as konflux-test-infra (the
konflux-infra-secrets-volume secret block) so overriding
params.konflux-test-infra-secret doesn't take effect; update both inline
TaskSpecs (PAC and ITS) to reference the parameter instead of the literal string
by replacing secretName: konflux-test-infra with secretName:
$(params.konflux-test-infra-secret) for the konflux-infra-secrets-volume entries
so the parametrized secret is consistently mounted.
Comment on lines
+12
to
+13
| - name: revision | ||
| value: migrate-from-konflux-ci |
There was a problem hiding this comment.
Avoid hardcoding an ephemeral feature branch in the committed manual run template.
Using migrate-from-konflux-ci for both pipeline resolution and test revision makes this template fragile after branch cleanup and can silently drift from intended default behavior.
Suggested stable default
- name: revision
- value: migrate-from-konflux-ci
+ value: main
@@
- name: revision
- value: migrate-from-konflux-ci
+ value: mainAs per coding guidelines -Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Also applies to: 19-20
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pipelines/pipelineruns/conforma-e2e-manual.yaml` around lines 12 - 13, The
YAML hardcodes the ephemeral branch value under the "revision" param (value:
migrate-from-konflux-ci); change this to a stable default or a parameter
reference instead (e.g., use a PipelineRun param like revision with a sensible
default such as main or reference $(params.revision)/${revision}) so the
template does not rely on a removed feature branch, and update the other
occurrence noted (lines 19-20) as well; ensure you modify the "revision" param
name/value usage consistently where PipelineRun or pipeline resolution reads it
so callers can override it when needed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
konflux-ci/e2e-tests/tests/enterprise-contract/contract.gokonflux-ci/release-serviceintegration-tests/pipelines/) that provisions an ephemeral KinD cluster on AWS via mapt, deploys full Konflux with Tekton Chains, runs the Ginkgo test suite, and tears downTest plan
cd e2e-tests && go build ./...compiles cleanlymake test-e2e-dry-runlists all 11 test scenariosintegration-tests/pipelines/konflux-e2e-tests-pipeline.yaml, so that future PRs to this repo automatically trigger the full e2e cycle (provision KinD → deploy Konflux → run tests → deprovision)🤖 Generated with Claude Code