Repository navigation
test: run ITS pipeline e2e checks on pull requests #3574
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| apiVersion: tekton.dev/v1 | ||
|
There was a problem hiding this comment. [low] missing-authorization The PR references EC-1943 (external Jira) and depends on conforma/e2e-tests#12 (cross-repo). No linked issue in conforma/cli exists as an authorization record in this repo's public history. Suggested fix: Open a tracking issue in conforma/cli mirroring EC-1943 and link it from the PR. There was a problem hiding this comment. [low] missing-authorization The PR references EC-1943 (external Jira) and depends on conforma/e2e-tests#12 (cross-repo). No linked issue in conforma/cli exists as an authorization record in this repo public history. Suggested fix: Open a tracking issue in conforma/cli mirroring EC-1943 and link it from the PR.
dheerajodha marked this conversation as resolved.
|
||
| kind: PipelineRun | ||
| metadata: | ||
| annotations: | ||
| build.appstudio.openshift.io/repo: https://github.com/conforma/cli?rev={{revision}} | ||
| build.appstudio.redhat.com/commit_sha: '{{revision}}' | ||
| build.appstudio.redhat.com/pull_request_number: '{{pull_request_number}}' | ||
| build.appstudio.redhat.com/target_branch: '{{target_branch}}' | ||
| pipelinesascode.tekton.dev/cancel-in-progress: "true" | ||
| pipelinesascode.tekton.dev/max-keep-runs: "3" | ||
| pipelinesascode.tekton.dev/on-cel-expression: | | ||
|
There was a problem hiding this comment. [high] fail-open-authorization The CEL at lines 13-16 filters only on event, target_branch, and pathChanged(); no author_association or label gate remains. The header comment defers authorization entirely to Pipelines-as-Code ACLs. If the deployed rhtap-contract-tenant PaC Repository/global policy is default-permissive, absent, or misconfigured (PR body states tenant configuration has not been inspected), a fork PR touching the matched paths triggers this PipelineRun under konflux-integration-runner with konflux-test-infra and mapt-kind-secret in scope. The pathChanged filter includes this file itself, so a fork PR editing this file can widen the trigger surface. The runner pipelineRef body is also fetched from the fork at PR head (its-pipeline-repo-url={{source_url}}, its-pipeline-revision={{revision}}), so fork-controlled YAML executes with production secrets attached. Suggested fix: Restore an in-file authorization gate (author_association allowlist and/or ok-to-test label check) OR block merge until the deployed PaC Repository/global policy for rhtap-contract-tenant has been inspected and documented as a hard prerequisite in the file header. Alternatively, restrict its-pipeline-repo-url to a trusted upstream URL and copy the pipeline-under-test into a workspace. There was a problem hiding this comment. [high] fail-open-authorization CEL expression (lines 13-16) filters only on event, target_branch, and pathChanged() — no author_association or ok-to-test label gate. Authorization is deferred entirely to Pipelines-as-Code ACLs/approval policy, and the PR body explicitly states tenant PaC configuration has not been inspected. If that config is default-permissive/absent/misconfigured, a fork pull_request touching matched paths triggers this PipelineRun under service account konflux-integration-runner with konflux-test-infra (line 33) and mapt-kind-secret (lines 35, 37) mounted. Amplifying: (a) ".tekton/cli-its-pull-request.yaml".pathChanged() at line 16 lets a fork PR editing this file widen its own trigger surface; (b) pipelineRef at lines 47-55 resolves the pipeline body from {{source_url}}@{{revision}} (fork at PR head), so fork-controlled pipeline YAML executes with the above secrets in scope. Suggested fix: Add an explicit CEL guard for trusted actors (author_association allowlist or ok-to-test label), OR block merge until the deployed PaC Repository CR is inspected and documented as a hard prerequisite in the file header. Additionally pin pipelineRef to a canonical conforma/* repo+SHA rather than {{source_url}}@{{revision}}.
dheerajodha marked this conversation as resolved.
|
||
| event == "pull_request" && target_branch == "main" && | ||
| ("pipelines/enterprise-contract/**".pathChanged() || | ||
|
There was a problem hiding this comment. [low] trigger-coverage-gap pathChanged() only fires on pipelines/enterprise-contract/** or this file. Changes to underlying task sources (e.g., tasks/verify-enterprise-contract/**) or Makefile/CLI code paths that ultimately shape the built bundle will not trigger this ITS run. Not a regression (this is a new job); worth confirming the scope is intentional. There was a problem hiding this comment. [low] trigger-coverage-gap The CEL only fires when pipelines/enterprise-contract/** or this file itself changes. Changes to Task source, Makefile, or CLI code paths that shape the built verify bundle will not trigger this ITS run. Informational; not a regression since this is a new job. There was a problem hiding this comment. [low] trigger-coverage-gap The CEL only fires when pipelines/enterprise-contract/** or this file itself changes. Changes to Task source, Makefile, or CLI code paths that materially shape the built verify bundle will not trigger this ITS run. Informational; not a regression since this is a new job. There was a problem hiding this comment. [high] fail-open-authorization The CEL trigger (lines 11-14) gates only on event == "pull_request", target_branch == "main", and pathChanged() over pipelines/enterprise-contract/** or .tekton/cli-its-pull-request.yaml itself. There is no author_association check and no ok-to-test label check — trust is delegated entirely to Pipelines-as-Code tenant ACLs. Amplifying facts verified against the file: (a) the CEL's pathChanged() clause at line 14 matches this file itself, so a fork PR editing it widens its own trigger surface; (b) pipelineRef (lines 43-51) resolves the pipeline body via the git resolver from a URL+revision pair currently pointing at a personal fork (line 47), meaning fork-controlled pipeline YAML would execute under serviceAccountName: konflux-integration-runner (line 53) with konflux-test-infra (line 30) and mapt-kind-secret (lines 32, 34) in scope. PaC's default fork-PR policy typically requires an owner/collaborator /ok-to-test — that mitigation is external and unverifiable from the diff. Suggested fix: Either (a) add an explicit CEL guard for trusted actors (e.g., pipelines_as_code.author_association in ["OWNER","MEMBER","COLLABORATOR"] or a hasLabel("ok-to-test") gate) so the trust posture is expressed in-tree, or (b) document the deployed tenant PaC Repository CR trust policy as a hard prerequisite. Independently, pin pipelineRef.url/revision (lines 47, 49) to a canonical conforma/* repository+SHA so fork-authored pipeline bodies cannot execute with the mounted secrets. |
||
| ".tekton/cli-its-pull-request.yaml".pathChanged()) | ||
| labels: | ||
| appstudio.openshift.io/application: ec-main | ||
| appstudio.openshift.io/component: cli-main | ||
| pipelines.appstudio.openshift.io/type: test | ||
| name: cli-its-on-pull-request | ||
| namespace: rhtap-contract-tenant | ||
| spec: | ||
| params: | ||
| - name: git-url | ||
| value: https://github.com/conforma/e2e-tests.git | ||
| - name: revision | ||
| value: eb59162d7c1d069a699f82a50e9f41db852c53d2 | ||
| - name: oci-container-repo | ||
| value: quay.io/conforma/e2e-tests | ||
| - name: oci-container-repo-credentials-secret | ||
| value: konflux-test-infra | ||
| - name: aws-credentials-secret | ||
| value: mapt-kind-secret | ||
| - name: deprovision-aws-credentials-secret | ||
| value: mapt-kind-secret | ||
| - name: its-pipeline-repo-url | ||
|
There was a problem hiding this comment. [low] param-propagation its-pipeline-repo-url uses {{source_url}} and its-pipeline-revision uses {{revision}}, so for fork PRs the ITS runner fetches the pipeline-under-test from the contributor's fork at PR head. Correct for the stated purpose, but the ITS run will fail for any PR whose fork is private or unreachable. Informational. There was a problem hiding this comment. [low] param-propagation its-pipeline-repo-url is bound to {{source_url}} and its-pipeline-revision to {{revision}}. For fork PRs the runner fetches the pipeline-under-test from the contributor's fork at PR head — correct for the stated intent, but runs will fail for any PR whose fork is private or unreachable. There was a problem hiding this comment. [low] param-propagation its-pipeline-repo-url is bound to {{source_url}} and its-pipeline-revision to {{revision}}. For fork PRs the runner fetches the pipeline-under-test from the contributor fork at PR head — correct for the stated intent, but runs will fail for any PR whose fork is private or unreachable by the Konflux runner service account.
dheerajodha marked this conversation as resolved.
|
||
| value: '{{source_url}}' | ||
|
dheerajodha marked this conversation as resolved.
There was a problem hiding this comment. [high] fail-open its-pipeline-repo-url (line 36) and its-pipeline-revision (line 38) interpolate PR-controlled {{source_url}}/{{revision}} and feed a PR-controlled Tekton PipelineRun/Task YAML to the e2e-tests runner, which executes it under serviceAccountName: konflux-integration-runner (line 53) with references to oci-container-repo-credentials-secret: konflux-test-infra (line 30), aws-credentials-secret: mapt-kind-secret (line 32), and deprovision-aws-credentials-secret: mapt-kind-secret (line 34). A fork PR could submit arbitrary Tekton task definitions to be executed in-cluster with that SA token and the mounted secrets. The only in-manifest authorization boundary is PaC default ACL (/ok-to-test); the PR body itself lists verification of the deployed PaC Repository/global policy as an unresolved checklist item. The upstream runner at conforma/e2e-tests@eb59162d could not be inspected from this review, so whether the PR-controlled ITS definition is sandboxed from the shared SA/secrets cannot be confirmed here. Suggested fix: Before merge: (1) confirm with the tenant admin that the deployed PaC Repository/global policy enforces /ok-to-test for unauthorized contributors (already an open PR checklist item); and (2) inspect .tekton/pipelines/conforma-e2e/pipeline.yaml at the pinned SHA and verify the PR-controlled ITS definition is sandboxed from the three secrets above (separate TaskRun/namespace, or not mounting konflux-test-infra/mapt-kind-secret on tasks that execute PR-controlled YAML). If either prerequisite cannot be verified, scope the SA/secrets to the minimum the outer runner needs and remove references the PR-controlled stage can reach. |
||
| - name: its-pipeline-revision | ||
| value: '{{revision}}' | ||
|
There was a problem hiding this comment. [high] fork-controlled-code-with-secrets Params its-pipeline-repo-url: {{source_url}} (line 36) and its-pipeline-revision: {{revision}} (line 38) cause the runner to fetch and execute an ITS pipeline definition drawn from the PR source repo/revision. The CEL trigger (lines 11-14) only gates on event == pull_request and target_branch == main; there is no author_association, org-membership, or /ok-to-test guard expressed in this file. A fork-authored PR that touches pipelines/enterprise-contract/** or this trigger file will therefore cause fork-controlled YAML to be interpreted by the runner in namespace rhtap-contract-tenant under service account konflux-integration-runner with konflux-test-infra (registry creds) and mapt-kind-secret (AWS creds) mounted. Protection today rests entirely on the out-of-band PaC approver gate. Suggested fix: Pick one or more of: (a) restrict its-pipeline-repo-url/its-pipeline-revision to the trusted upstream (base repo/base ref); (b) tighten the CEL to require a trusted actor, e.g. body.pull_request.author_association in [MEMBER, OWNER, COLLABORATOR]; or (c) explicitly document and enforce the PaC approver gate (/ok-to-test required for fork PRs) on this Repository CR. Also consider removing AWS/registry secrets from this PR-triggered variant if they are not needed for the ITS pipeline lint/schema checks. |
||
| - name: its-pipeline-path | ||
| value: pipelines/enterprise-contract/0.1/enterprise-contract.yaml | ||
| - name: test-label-filter | ||
|
There was a problem hiding this comment. [low] api-contract Params sent to the resolved pipeline (its-pipeline-repo-url, its-pipeline-revision, its-pipeline-path, test-label-filter) diverge from the naming used by sibling .tekton/cli-e2e-push.yaml (custom-ec-cli-url, custom-ec-cli-revision). Correctness depends on the fork pipeline yaml declaring these exact param names; the referenced pipeline file cannot be inspected from this diff. If any name/type is mismatched, the PipelineRun will fail admission or the params will be silently ignored. Suggested fix: When repointing to upstream, verify each of the four param names exists in conforma/e2e-tests .tekton/pipelines/conforma-e2e/pipeline.yaml at the pinned SHA. A one-time dry run against the upstream SHA before flipping out of draft is sufficient. |
||
| value: its-pipeline | ||
| pipelineRef: | ||
| resolver: git | ||
| params: | ||
| - name: url | ||
| value: https://github.com/conforma/e2e-tests.git | ||
| - name: revision | ||
| value: eb59162d7c1d069a699f82a50e9f41db852c53d2 | ||
| - name: pathInRepo | ||
| value: .tekton/pipelines/conforma-e2e/pipeline.yaml | ||
| taskRunTemplate: | ||
| serviceAccountName: konflux-integration-runner | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[low] missing-authorization
The PR references EC-1943 (Jira, external) and depends on conforma/e2e-tests#12 (cross-repo). No linked issue in conforma/cli exists to serve as an authorization record within this repo's public history.
Suggested fix: Open a tracking issue in conforma/cli mirroring EC-1943 and link it from the PR.