Skip to content

feat(ISV-6717): Add a new task to read keyless signing config#2042

Merged
gbenhaim merged 1 commit intokonflux-ci:developmentfrom
Allda:ISV-6717
Mar 19, 2026
Merged

feat(ISV-6717): Add a new task to read keyless signing config#2042
gbenhaim merged 1 commit intokonflux-ci:developmentfrom
Allda:ISV-6717

Conversation

@Allda
Copy link
Copy Markdown
Contributor

@Allda Allda commented Mar 4, 2026

A new task reads Konflux configuration and expose the values for keyless signing. In case the config map is not present on a cluster, the task returns empty values which indicates the keyless signing should be skipped.

Describe your changes

Relevant Jira

Checklist before requesting a review

  • I have marked as draft or added do not merge label if there's a dependency PR
    • If you want reviews on your draft PR, you can add reviewers or add the release-service-maintainers handle if you are unsure who to tag
  • My commit message includes Signed-off-by: My name <email>
  • I read CONTRIBUTING.MD and commit formatting
  • I have run the README.md generator script in .github/scripts/readme_generator.sh and verified the results using .github/scripts/check_readme.sh

@Allda Allda marked this pull request as draft March 4, 2026 14:23
@qodo-code-review
Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Add collect-signing-params task for keyless signing configuration

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Add new Tekton task to collect keyless signing configuration
• Task reads Konflux cluster-config ConfigMap from konflux-info namespace
• Extracts signing parameters (OIDC issuer, Rekor, Fulcio, TUF URLs)
• Returns empty values when ConfigMap is missing, allowing pipeline continuation
• Includes comprehensive test for missing ConfigMap scenario
Diagram
flowchart LR
  A["Tekton Task<br/>collect-signing-params"] -->|reads| B["cluster-config<br/>ConfigMap"]
  B -->|extracts| C["Signing Parameters<br/>OIDC, Rekor, Fulcio, TUF"]
  A -->|fallback| D["Empty Values<br/>if ConfigMap missing"]
  C --> E["Task Results"]
  D --> E
Loading

Grey Divider

File Changes

1. tasks/managed/collect-signing-params/collect-signing-params.yaml ✨ Enhancement +173/-0

New Tekton task for collecting signing parameters

• Defines new Tekton Task collect-signing-params for collecting keyless signing configuration
• Accepts parameters for ConfigMap location, OCI artifacts, and git repository details
• Implements two-step process: use-trusted-artifact step and collect-signing-params script step
• Script fetches cluster-config ConfigMap and extracts signing parameters (defaultOIDCIssuer,
 rekorExternalUrl, fulcioExternalUrl, tufExternalUrl, buildIdentityRegexp)
• Gracefully handles missing ConfigMap by returning empty strings for all results

tasks/managed/collect-signing-params/collect-signing-params.yaml


2. tasks/managed/collect-signing-params/README.md 📝 Documentation +24/-0

Task documentation with parameters and usage

• Documents the collect-signing-params task purpose and functionality
• Lists all task parameters with descriptions, optional flags, and default values
• Explains ConfigMap reading behavior and fallback to empty values
• Covers parameters for ConfigMap configuration, OCI artifacts, trusted artifacts, and CA bundle

tasks/managed/collect-signing-params/README.md


3. tasks/managed/collect-signing-params/tests/test-collect-signing-params-missing-cm.yaml 🧪 Tests +96/-0

Test pipeline for missing ConfigMap scenario

• Defines test pipeline for collect-signing-params task with missing ConfigMap scenario
• Includes setup task and run-task that executes collect-signing-params
• Implements check-result task that validates all signing parameters are empty strings
• Tests graceful degradation when cluster-config ConfigMap is not found

tasks/managed/collect-signing-params/tests/test-collect-signing-params-missing-cm.yaml


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review Bot commented Mar 4, 2026

Code Review by Qodo

🐞 Bugs (13) 📘 Rule violations (13) 📎 Requirement gaps (0)

Grey Divider


Action required

1. Missing EOF newline in hook 📘 Rule violation ✓ Correctness
Description
The new pre-apply-task-hook.sh file is missing a trailing newline at end-of-file, which commonly
causes pre-commit hooks (e.g., end-of-file-fixer) to fail. This can break the required `pre-commit
run --all-files` compliance gate.
Code

tasks/managed/collect-signing-params/tests/pre-apply-task-hook.sh[6]

+kubectl apply -f .github/resources/crd_rbac.yaml
Evidence
PR Compliance ID 8 requires all configured pre-commit hooks to pass. The diff explicitly indicates
\ No newline at end of file for this newly added script, a typical pre-commit failure condition.

CLAUDE.md
tasks/managed/collect-signing-params/tests/pre-apply-task-hook.sh[1-6]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new shell script is missing a newline at end-of-file, which can cause pre-commit hooks to fail.
## Issue Context
The diff indicates `No newline at end of file` for this newly added script.
## Fix Focus Areas
- tasks/managed/collect-signing-params/tests/pre-apply-task-hook.sh[1-6]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Long image line 📘 Rule violation ⛯ Reliability
Description
The added test pipeline YAML contains image: lines exceeding 120 characters, which can fail
yamllint and break CI. The release-service-utils@sha256:... reference should be shortened to
comply with repository YAML style constraints.
Code

tasks/managed/collect-signing-params/tests/test-collect-signing-params.yaml[51]

+            image: quay.io/konflux-ci/release-service-utils@sha256:5546fa78d3c88d7b6a2e8cff8902f7757f00541d0bbaf113b9f293133894afa3
Evidence
PR Compliance ID 1 requires YAML lines to be <= 120 characters. The image: lines in the new
pipeline exceed this limit.

CLAUDE.md
tasks/managed/collect-signing-params/tests/test-collect-signing-params.yaml[51-51]
tasks/managed/collect-signing-params/tests/test-collect-signing-params.yaml[119-119]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The test pipeline YAML has `image:` lines longer than 120 characters, which can fail `yamllint`.
## Issue Context
Repository YAML style constraints enforce a max 120 character line length.
## Fix Focus Areas
- tasks/managed/collect-signing-params/tests/test-collect-signing-params.yaml[51-51]
- tasks/managed/collect-signing-params/tests/test-collect-signing-params.yaml[119-119]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. taskSpec lacks runAsUser 📘 Rule violation ⛨ Security
Description
The inline taskSpec in the new test pipeline does not set securityContext.runAsUser, so steps
may run as an implicit/unspecified user. This violates the requirement that Tekton tasks explicitly
specify runAsUser.
Code

tasks/managed/collect-signing-params/tests/test-collect-signing-params.yaml[R31-48]

+      taskSpec:
+        results:
+          - name: sourceDataArtifact
+            type: string
+        volumes:
+          - name: workdir
+            emptyDir: {}
+        stepTemplate:
+          volumeMounts:
+            - mountPath: /var/workdir
+              name: workdir
+          env:
+            - name: IMAGE_EXPIRES_AFTER
+              value: $(params.ociArtifactExpiresAfter)
+            - name: "ORAS_OPTIONS"
+              value: "$(params.orasOptions)"
+            - name: "DEBUG"
+              value: "$(params.trustedArtifactsDebug)"
Evidence
PR Compliance ID 4 requires explicit runAsUser for Tekton tasks. The new pipeline defines inline
TaskSpecs (setup and check-result) without any securityContext.runAsUser.

CLAUDE.md
tasks/managed/collect-signing-params/tests/test-collect-signing-params.yaml[31-48]
tasks/managed/collect-signing-params/tests/test-collect-signing-params.yaml[105-118]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Inline `taskSpec` blocks in the test pipeline do not set `securityContext.runAsUser`, leaving the runtime user implicit.
## Issue Context
Compliance requires Tekton tasks to explicitly specify `runAsUser`.
## Fix Focus Areas
- tasks/managed/collect-signing-params/tests/test-collect-signing-params.yaml[31-49]
- tasks/managed/collect-signing-params/tests/test-collect-signing-params.yaml[105-131]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View more (22)
4. taskSpec lacks runAsUser 📘 Rule violation ⛨ Security
Description
The inline taskSpec in the missing-ConfigMap test pipeline does not set
securityContext.runAsUser, so steps may run as an implicit/unspecified user. This violates the
requirement that Tekton tasks explicitly specify runAsUser.
Code

tasks/managed/collect-signing-params/tests/test-collect-signing-params-missing-cm.yaml[R31-48]

+      taskSpec:
+        results:
+          - name: sourceDataArtifact
+            type: string
+        volumes:
+          - name: workdir
+            emptyDir: {}
+        stepTemplate:
+          volumeMounts:
+            - mountPath: /var/workdir
+              name: workdir
+          env:
+            - name: IMAGE_EXPIRES_AFTER
+              value: $(params.ociArtifactExpiresAfter)
+            - name: "ORAS_OPTIONS"
+              value: "$(params.orasOptions)"
+            - name: "DEBUG"
+              value: "$(params.trustedArtifactsDebug)"
Evidence
PR Compliance ID 4 requires explicit runAsUser for Tekton tasks. The new pipeline defines inline
TaskSpecs (setup and check-result) without any securityContext.runAsUser.

CLAUDE.md
tasks/managed/collect-signing-params/tests/test-collect-signing-params-missing-cm.yaml[31-48]
tasks/managed/collect-signing-params/tests/test-collect-signing-params-missing-cm.yaml[90-103]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Inline `taskSpec` blocks in the missing-ConfigMap test pipeline do not set `securityContext.runAsUser`, leaving the runtime user implicit.
## Issue Context
Compliance requires Tekton tasks to explicitly specify `runAsUser`.
## Fix Focus Areas
- tasks/managed/collect-signing-params/tests/test-collect-signing-params-missing-cm.yaml[31-49]
- tasks/managed/collect-signing-params/tests/test-collect-signing-params-missing-cm.yaml[90-116]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


5. collect-signing-params missing integration tests 📘 Rule violation ⛯ Reliability
Description
This PR adds a new task but only introduces tests under tasks/managed/.../tests rather than
adding/updating integration tests under integration-tests/. This violates the requirement that
task changes be covered by integration tests in the integration-tests harness.
Code

tasks/managed/collect-signing-params/tests/test-collect-signing-params.yaml[R1-6]

+---
+apiVersion: tekton.dev/v1
+kind: Pipeline
+metadata:
+  name: test-collect-signing-params
+spec:
Evidence
PR Compliance ID 7 requires integration tests to be added/updated under integration-tests/ when
tasks change. The PR adds test pipelines under the task directory instead.

CLAUDE.md
tasks/managed/collect-signing-params/tests/test-collect-signing-params.yaml[1-6]
tasks/managed/collect-signing-params/tests/test-collect-signing-params-missing-cm.yaml[1-6]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A new task was added, but no corresponding integration tests were added/updated under `integration-tests/`, as required.
## Issue Context
The PR adds test pipelines under `tasks/managed/collect-signing-params/tests/`, but the compliance requirement is specifically to cover task changes in `integration-tests/`.
## Fix Focus Areas
- tasks/managed/collect-signing-params/tests/test-collect-signing-params.yaml[1-139]
- tasks/managed/collect-signing-params/tests/test-collect-signing-params-missing-cm.yaml[1-124]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


6. taskGitUrl description exceeds limit 📘 Rule violation ⛯ Reliability
Description
The new Tekton Task YAML contains lines longer than 120 characters, which violates the repository
yamllint style rules and can fail CI linting. Long lines appear in the param description, the pinned
image reference, and a long inline bash command.
Code

tasks/managed/collect-signing-params/collect-signing-params.yaml[49]

+      description: The url to the git repo where the release-service-catalog tasks and stepactions to be used are stored
Evidence
PR Compliance ID 1 requires YAML changes to keep line length at or below 120 characters. The added
Task YAML includes multiple single lines that clearly exceed that limit (e.g., a very long
description: value, a long image digest line, and a long kubectl command line within the YAML
script: block).

CLAUDE.md
tasks/managed/collect-signing-params/collect-signing-params.yaml[49-49]
tasks/managed/collect-signing-params/collect-signing-params.yaml[134-134]
tasks/managed/collect-signing-params/collect-signing-params.yaml[148-148]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new task YAML has lines exceeding 120 characters (yamllint max-line-length), which can break CI lint checks.
## Issue Context
This repository enforces YAML formatting constraints including max 120-character line length.
## Fix Focus Areas
- tasks/managed/collect-signing-params/collect-signing-params.yaml[49-49]
- tasks/managed/collect-signing-params/collect-signing-params.yaml[134-134]
- tasks/managed/collect-signing-params/collect-signing-params.yaml[148-148]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


7. taskGitUrl description exceeds limit 📘 Rule violation ⛯ Reliability
Description
The new Tekton Task YAML contains lines longer than 120 characters, which violates the repository
yamllint style rules and can fail CI linting. Long lines appear in the param description, the pinned
image reference, and a long inline bash command.
Code

tasks/managed/collect-signing-params/collect-signing-params.yaml[49]

+      description: The url to the git repo where the release-service-catalog tasks and stepactions to be used are stored
Evidence
PR Compliance ID 1 requires YAML changes to keep line length at or below 120 characters. The added
Task YAML includes multiple single lines that clearly exceed that limit (e.g., a very long
description: value, a long image digest line, and a long kubectl command line within the YAML
script: block).

CLAUDE.md
tasks/managed/collect-signing-params/collect-signing-params.yaml[49-49]
tasks/managed/collect-signing-params/collect-signing-params.yaml[134-134]
tasks/managed/collect-signing-params/collect-signing-params.yaml[148-148]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new task YAML has lines exceeding 120 characters (yamllint max-line-length), which can break CI lint checks.
## Issue Context
This repository enforces YAML formatting constraints including max 120-character line length.
## Fix Focus Areas
- tasks/managed/collect-signing-params/collect-signing-params.yaml[49-49]
- tasks/managed/collect-signing-params/collect-signing-params.yaml[134-134]
- tasks/managed/collect-signing-params/collect-signing-params.yaml[148-148]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


8. Test pipeline image too-long 📘 Rule violation ⛯ Reliability
Description
The new test Pipeline YAML includes a single image: line exceeding 120 characters, violating the
repository yamllint style rules. This can cause lint/CI failures for YAML formatting checks.
Code

tasks/managed/collect-signing-params/tests/test-collect-signing-params-missing-cm.yaml[76]

+            image: quay.io/konflux-ci/release-service-utils@sha256:5546fa78d3c88d7b6a2e8cff8902f7757f00541d0bbaf113b9f293133894afa3
Evidence
PR Compliance ID 1 enforces a maximum 120-character line length for YAML files. The added test
pipeline contains a long, pinned image digest on a single line that exceeds this limit.

CLAUDE.md
tasks/managed/collect-signing-params/tests/test-collect-signing-params-missing-cm.yaml[76-76]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The test pipeline YAML uses a pinned `image:` value on a single line that exceeds the 120-character yamllint limit.
## Issue Context
YAML changes must conform to repository yamllint rules, including max line length.
## Fix Focus Areas
- tasks/managed/collect-signing-params/tests/test-collect-signing-params-missing-cm.yaml[76-76]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


9. Test pipeline image too-long 📘 Rule violation ⛯ Reliability
Description
The new test Pipeline YAML includes a single image: line exceeding 120 characters, violating the
repository yamllint style rules. This can cause lint/CI failures for YAML formatting checks.
Code

tasks/managed/collect-signing-params/tests/test-collect-signing-params-missing-cm.yaml[76]

+            image: quay.io/konflux-ci/release-service-utils@sha256:5546fa78d3c88d7b6a2e8cff8902f7757f00541d0bbaf113b9f293133894afa3
Evidence
PR Compliance ID 1 enforces a maximum 120-character line length for YAML files. The added test
pipeline contains a long, pinned image digest on a single line that exceeds this limit.

CLAUDE.md
tasks/managed/collect-signing-params/tests/test-collect-signing-params-missing-cm.yaml[76-76]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The test pipeline YAML uses a pinned `image:` value on a single line that exceeds the 120-character yamllint limit.
## Issue Context
YAML changes must conform to repository yamllint rules, including max line length.
## Fix Focus Areas
- tasks/managed/collect-signing-params/tests/test-collect-signing-params-missing-cm.yaml[76-76]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


10. Test pipeline invalid 🐞 Bug ✓ Correctness
Description
The added test Pipeline references a missing setup task and uses its results, and the inline
check-result taskSpec has duplicate/missing param declarations. As written, this Pipeline will
fail Tekton validation and never execute the task behavior it intends to test.
Code

tasks/managed/collect-signing-params/tests/test-collect-signing-params-missing-cm.yaml[R30-87]

+    - name: run-task
+      taskRef:
+        name: collect-signing-params
+      params:
+        - name: ociStorage
+          value: $(params.ociStorage)
+        - name: orasOptions
+          value: $(params.orasOptions)
+        - name: sourceDataArtifact
+          value: "$(tasks.setup.results.sourceDataArtifact)=$(params.dataDir)"
+        - name: dataDir
+          value: $(params.dataDir)
+        - name: trustedArtifactsDebug
+          value: $(params.trustedArtifactsDebug)
+        - name: taskGitUrl
+          value: "http://localhost"
+        - name: taskGitRevision
+          value: "main"
+      runAfter:
+        - setup
+    - name: check-result
+      params:
+        - name: defaultOIDCIssuer
+          value: $(tasks.run-task.results.defaultOIDCIssuer)
+        - name: rekorExternalUrl
+          value: $(tasks.run-task.results.rekorExternalUrl)
+        - name: fulcioExternalUrl
+          value: $(tasks.run-task.results.fulcioExternalUrl)
+        - name: tufExternalUrl
+          value: $(tasks.run-task.results.tufExternalUrl)
+        - name: buildIdentityRegexp
+          value: $(tasks.run-task.results.buildIdentityRegexp)
+      taskSpec:
+        params:
+          - name: buildIdentityRegexp
+            type: string
+          - name: rekorExternalUrl
+            type: string
+          - name: fulcioExternalUrl
+            type: string
+          - name: tufExternalUrl
+            type: string
+          - name: buildIdentityRegexp
+            type: string
+        steps:
+          - name: check-result
+            image: quay.io/konflux-ci/release-service-utils@sha256:5546fa78d3c88d7b6a2e8cff8902f7757f00541d0bbaf113b9f293133894afa3
+            env:
+              - name: "DEFAULT_OIDC_ISSUER"
+                value: '$(params.defaultOIDCIssuer)'
+              - name: "REKOR_EXTERNAL_URL"
+                value: '$(params.rekorExternalUrl)'
+              - name: "FULCIO_EXTERNAL_URL"
+                value: '$(params.fulcioExternalUrl)'
+              - name: "TUF_EXTERNAL_URL"
+                value: '$(params.tufExternalUrl)'
+              - name: "BUILD_IDENTITY_REGEXP"
+                value: '$(params.buildIdentityRegexp)'
Evidence
The new test Pipeline uses runAfter: setup and $(tasks.setup.results.sourceDataArtifact) but
does not define a setup task, and its check-result taskSpec declares buildIdentityRegexp twice
while not declaring defaultOIDCIssuer even though it is referenced. Existing tests in this repo
consistently define a setup task that produces sourceDataArtifact and define taskSpec params
that match the params they pass/use.

tasks/managed/collect-signing-params/tests/test-collect-signing-params-missing-cm.yaml[30-49]
tasks/managed/collect-signing-params/tests/test-collect-signing-params-missing-cm.yaml[62-87]
tasks/managed/collect-atlas-params/tests/test-collect-atlas-params-nonexistent.yaml[30-37]
tasks/managed/collect-atlas-params/tests/test-collect-atlas-params-nonexistent.yaml[68-90]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new test Pipeline `test-collect-signing-params-missing-cm` cannot validate/run because it references a missing `setup` task and defines an invalid `check-result` taskSpec parameter list.
### Issue Context
Other managed-task tests in this repo use a `setup` task to produce `sourceDataArtifact`, then run the task under test, then a `check-result` task with a taskSpec that declares exactly the params it uses.
### Fix Focus Areas
- tasks/managed/collect-signing-params/tests/test-collect-signing-params-missing-cm.yaml[30-49]
- tasks/managed/collect-signing-params/tests/test-collect-signing-params-missing-cm.yaml[62-87]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


11. Test pipeline invalid 🐞 Bug ✓ Correctness
Description
The added test Pipeline references a missing setup task and uses its results, and the inline
check-result taskSpec has duplicate/missing param declarations. As written, this Pipeline will
fail Tekton validation and never execute the task behavior it intends to test.
Code

tasks/managed/collect-signing-params/tests/test-collect-signing-params-missing-cm.yaml[R30-87]

+    - name: run-task
+      taskRef:
+        name: collect-signing-params
+      params:
+        - name: ociStorage
+          value: $(params.ociStorage)
+        - name: orasOptions
+          value: $(params.orasOptions)
+        - name: sourceDataArtifact
+          value: "$(tasks.setup.results.sourceDataArtifact)=$(params.dataDir)"
+        - name: dataDir
+          value: $(params.dataDir)
+        - name: trustedArtifactsDebug
+          value: $(params.trustedArtifactsDebug)
+        - name: taskGitUrl
+          value: "http://localhost"
+        - name: taskGitRevision
+          value: "main"
+      runAfter:
+        - setup
+    - name: check-result
+      params:
+        - name: defaultOIDCIssuer
+          value: $(tasks.run-task.results.defaultOIDCIssuer)
+        - name: rekorExternalUrl
+          value: $(tasks.run-task.results.rekorExternalUrl)
+        - name: fulcioExternalUrl
+          value: $(tasks.run-task.results.fulcioExternalUrl)
+        - name: tufExternalUrl
+          value: $(tasks.run-task.results.tufExternalUrl)
+        - name: buildIdentityRegexp
+          value: $(tasks.run-task.results.buildIdentityRegexp)
+      taskSpec:
+        params:
+          - name: buildIdentityRegexp
+            type: string
+          - name: rekorExternalUrl
+            type: string
+          - name: fulcioExternalUrl
+            type: string
+          - name: tufExternalUrl
+            type: string
+          - name: buildIdentityRegexp
+            type: string
+        steps:
+          - name: check-result
+            image: quay.io/konflux-ci/release-service-utils@sha256:5546fa78d3c88d7b6a2e8cff8902f7757f00541d0bbaf113b9f293133894afa3
+            env:
+              - name: "DEFAULT_OIDC_ISSUER"
+                value: '$(params.defaultOIDCIssuer)'
+              - name: "REKOR_EXTERNAL_URL"
+                value: '$(params.rekorExternalUrl)'
+              - name: "FULCIO_EXTERNAL_URL"
+                value: '$(params.fulcioExternalUrl)'
+              - name: "TUF_EXTERNAL_URL"
+                value: '$(params.tufExternalUrl)'
+              - name: "BUILD_IDENTITY_REGEXP"
+                value: '$(params.buildIdentityRegexp)'
Evidence
The new test Pipeline uses runAfter: setup and $(tasks.setup.results.sourceDataArtifact) but
does not define a setup task, and its check-result taskSpec declares buildIdentityRegexp twice
while not declaring defaultOIDCIssuer even though it is referenced. Existing tests in this repo
consistently define a setup task that produces sourceDataArtifact and define taskSpec params
that match the params they pass/use.

tasks/managed/collect-signing-params/tests/test-collect-signing-params-missing-cm.yaml[30-49]
tasks/managed/collect-signing-params/tests/test-collect-signing-params-missing-cm.yaml[62-87]
tasks/managed/collect-atlas-params/tests/test-collect-atlas-params-nonexistent.yaml[30-37]
tasks/managed/collect-atlas-params/tests/test-collect-atlas-params-nonexistent.yaml[68-90]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new test Pipeline `test-collect-signing-params-missing-cm` cannot validate/run because it references a missing `setup` task and defines an invalid `check-result` taskSpec parameter list.
### Issue Context
Other managed-task tests in this repo use a `setup` task to produce `sourceDataArtifact`, then run the task under test, then a `check-result` task with a taskSpec that declares exactly the params it uses.
### Fix Focus Areas
- tasks/managed/collect-signing-params/tests/test-collect-signing-params-missing-cm.yaml[30-49]
- tasks/managed/collect-signing-params/tests/test-collect-signing-params-missing-cm.yaml[62-87]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


12. Unquoted kubectl params 🐞 Bug ⛨ Security
Description
The task builds a kubectl command with unquoted Tekton params, which allows word-splitting and
potential shell metacharacter injection if params are attacker-controlled. This is inconsistent with
other tasks in the repo that quote variables passed to kubectl.
Code

tasks/managed/collect-signing-params/collect-signing-params.yaml[148]

+        if kubectl get configmap $(params.configMapName) -n $(params.configMapNamespace) -o json > $KFLX_CONFIG_PATH 2>/dev/null; then
Evidence
The kubectl invocation interpolates $(params.configMapName) and $(params.configMapNamespace)
without quotes. In bash, that can lead to word splitting and command injection. A comparable managed
task in the repo demonstrates the expected quoting pattern when passing dynamic values to kubectl.

tasks/managed/collect-signing-params/collect-signing-params.yaml[147-149]
tasks/managed/push-artifacts-to-cdn/push-artifacts-to-cdn.yaml[183-185]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The task runs `kubectl get configmap` using unquoted Tekton parameters, which enables word-splitting and potential command injection.
### Issue Context
Other tasks in this repo quote dynamic values when calling `kubectl`.
### Fix Focus Areas
- tasks/managed/collect-signing-params/collect-signing-params.yaml[148-148]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


13. Unquoted kubectl params 🐞 Bug ⛨ Security
Description
The task builds a kubectl command with unquoted Tekton params, which allows word-splitting and
potential shell metacharacter injection if params are attacker-controlled. This is inconsistent with
other tasks in the repo that quote variables passed to kubectl.
Code

tasks/managed/collect-signing-params/collect-signing-params.yaml[148]

+        if kubectl get configmap $(params.configMapName) -n $(params.configMapNamespace) -o json > $KFLX_CONFIG_PATH 2>/dev/null; then
Evidence
The kubectl invocation interpolates $(params.configMapName) and $(params.configMapNamespace)
without quotes. In bash, that can lead to word splitting and command injection. A comparable managed
task in the repo demonstrates the expected quoting pattern when passing dynamic values to kubectl.

tasks/managed/collect-signing-params/collect-signing-params.yaml[147-149]
tasks/managed/push-artifacts-to-cdn/push-artifacts-to-cdn.yaml[183-185]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The task runs `kubectl get configmap` using unquoted Tekton parameters, which enables word-splitting and potential command injection.
### Issue Context
Other tasks in this repo quote dynamic values when calling `kubectl`.
### Fix Focus Areas
- tasks/managed/collect-signing-params/collect-signing-params.yaml[148-148]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


14. Kubectl errors hidden 🐞 Bug ⛯ Reliability
Description
All kubectl failures (including RBAC denial or cluster issues) are treated as “ConfigMap not found”
and converted into empty outputs, which can silently disable signing and makes debugging difficult.
The README states empty outputs should indicate the ConfigMap is missing, but stderr is suppressed
and the error type is not checked.
Code

tasks/managed/collect-signing-params/collect-signing-params.yaml[R147-166]

+        # Attempt to fetch the ConfigMap, capture exit code
+        if kubectl get configmap $(params.configMapName) -n $(params.configMapNamespace) -o json > $KFLX_CONFIG_PATH 2>/dev/null; then
+            echo "ConfigMap found, extracting signing parameters"
+
+            # Extract signing parameters from ConfigMap data, defaulting to empty string if not found
+            defaultOIDCIssuer=$(jq -r '.data.defaultOIDCIssuer // ""' "$KFLX_CONFIG_PATH")
+            rekorExternalUrl=$(jq -r '.data.rekorExternalUrl // ""' "$KFLX_CONFIG_PATH")
+            fulcioExternalUrl=$(jq -r '.data.fulcioExternalUrl // ""' "$KFLX_CONFIG_PATH")
+            tufExternalUrl=$(jq -r '.data.tufExternalUrl // ""' "$KFLX_CONFIG_PATH")
+            buildIdentityRegexp=$(jq -r '.data.buildIdentityRegexp // ""' "$KFLX_CONFIG_PATH")
+        else
+            echo "ConfigMap not found, using default empty values"
+
+            # Set all parameters to empty strings when ConfigMap doesn't exist
+            defaultOIDCIssuer=""
+            rekorExternalUrl=""
+            fulcioExternalUrl=""
+            tufExternalUrl=""
+            buildIdentityRegexp=""
+        fi
Evidence
The task redirects kubectl stderr to /dev/null and branches to the “not found” behavior for any
non-zero exit. That means permission errors and transient failures look identical to missing
ConfigMap. The README explicitly frames empty outputs as the behavior when the ConfigMap is not
found.

tasks/managed/collect-signing-params/collect-signing-params.yaml[147-166]
tasks/managed/collect-signing-params/README.md[7-8]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The task suppresses kubectl stderr and treats any kubectl failure as a missing ConfigMap, which can silently skip signing on RBAC/outage errors.
### Issue Context
The README describes empty outputs as the behavior when the ConfigMap is not found. The implementation should match that intent by distinguishing NotFound from other errors.
### Fix Focus Areas
- tasks/managed/collect-signing-params/collect-signing-params.yaml[147-166]
- tasks/managed/collect-signing-params/README.md[7-8]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


15. Kubectl errors hidden 🐞 Bug ⛯ Reliability
Description
All kubectl failures (including RBAC denial or cluster issues) are treated as “ConfigMap not found”
and converted into empty outputs, which can silently disable signing and makes debugging difficult.
The README states empty outputs should indicate the ConfigMap is missing, but stderr is suppressed
and the error type is not checked.
Code

tasks/managed/collect-signing-params/collect-signing-params.yaml[R147-166]

+        # Attempt to fetch the ConfigMap, capture exit code
+        if kubectl get configmap $(params.configMapName) -n $(params.configMapNamespace) -o json > $KFLX_CONFIG_PATH 2>/dev/null; then
+            echo "ConfigMap found, extracting signing parameters"
+
+            # Extract signing parameters from ConfigMap data, defaulting to empty string if not found
+            defaultOIDCIssuer=$(jq -r '.data.defaultOIDCIssuer // ""' "$KFLX_CONFIG_PATH")
+            rekorExternalUrl=$(jq -r '.data.rekorExternalUrl // ""' "$KFLX_CONFIG_PATH")
+            fulcioExternalUrl=$(jq -r '.data.fulcioExternalUrl // ""' "$KFLX_CONFIG_PATH")
+            tufExternalUrl=$(jq -r '.data.tufExternalUrl // ""' "$KFLX_CONFIG_PATH")
+            buildIdentityRegexp=$(jq -r '.data.buildIdentityRegexp // ""' "$KFLX_CONFIG_PATH")
+        else
+            echo "ConfigMap not found, using default empty values"
+
+            # Set all parameters to empty strings when ConfigMap doesn't exist
+            defaultOIDCIssuer=""
+            rekorExternalUrl=""
+            fulcioExternalUrl=""
+            tufExternalUrl=""
+            buildIdentityRegexp=""
+        fi
Evidence
The task redirects kubectl stderr to /dev/null and branches to the “not found” behavior for any
non-zero exit. That means permission errors and transient failures look identical to missing
ConfigMap. The README explicitly frames empty outputs as the behavior when the ConfigMap is not
found.

tasks/managed/collect-signing-params/collect-signing-params.yaml[147-166]
tasks/managed/collect-signing-params/README.md[7-8]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The task suppresses kubectl stderr and treats any kubectl failure as a missing ConfigMap, which can silently skip signing on RBAC/outage errors.
### Issue Context
The README describes empty outputs as the behavior when the ConfigMap is not found. The implementation should match that intent by distinguishing NotFound from other errors.
### Fix Focus Areas
- tasks/managed/collect-signing-params/collect-signing-params.yaml[147-166]
- tasks/managed/collect-signing-params/README.md[7-8]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


16. Test creates namespace 🐞 Bug ✓ Correctness
Description
The success-path test creates $(context.pipelineRun.namespace) under set -eux; the CI test
harness runs pipelines in the current (already-existing) namespace, so this can fail with
AlreadyExists and break CI.
Code

tasks/managed/collect-signing-params/tests/test-collect-signing-params.yaml[R53-57]

+              #!/usr/bin/env bash
+              set -eux
+
+              kubectl create namespace "$(context.pipelineRun.namespace)"
+              kubectl create configmap cluster-config --namespace "$(context.pipelineRun.namespace)" \
Evidence
The test step runs kubectl create namespace ... with set -eux so any AlreadyExists is fatal. The
shared CI harness applies the Pipeline and starts it without specifying a namespace, meaning it runs
in the current namespace (which must already exist for kubectl apply / tkn to work), making
namespace creation redundant and likely failing.

tasks/managed/collect-signing-params/tests/test-collect-signing-params.yaml[53-62]
.github/scripts/test_tekton_tasks.sh[232-265]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`test-collect-signing-params.yaml` runs `kubectl create namespace $(context.pipelineRun.namespace)` under `set -eux`, which will fail if the namespace already exists (common in CI harnesses that run all tests in a fixed namespace).
## Issue Context
The repo CI harness applies the Pipeline and starts it without setting a namespace explicitly, so tests run in the current namespace (already exists).
## Fix Focus Areas
- tasks/managed/collect-signing-params/tests/test-collect-signing-params.yaml[53-62]
- .github/scripts/test_tekton_tasks.sh[232-265]
### Suggested approach
- Remove the `kubectl create namespace ...` line entirely, or
- Make it idempotent, e.g. `kubectl get namespace &amp;amp;amp;amp;amp;amp;amp;amp;amp;quot;$ns&amp;amp;amp;amp;amp;amp;amp;amp;amp;quot; &amp;amp;amp;amp;amp;amp;amp;amp;amp;gt;/dev/null 2&amp;amp;amp;amp;amp;amp;amp;amp;amp;gt;&amp;amp;amp;amp;amp;amp;amp;amp;amp;amp;1 || kubectl create namespace &amp;amp;amp;amp;amp;amp;amp;amp;amp;quot;$ns&amp;amp;amp;amp;amp;amp;amp;amp;amp;quot;`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


17. taskGitUrl description exceeds limit 📘 Rule violation ⛯ Reliability
Description
The new Tekton Task YAML contains lines longer than 120 characters, which violates the repository
yamllint style rules and can fail CI linting. Long lines appear in the param description, the pinned
image reference, and a long inline bash command.
Code

tasks/managed/collect-signing-params/collect-signing-params.yaml[49]

+      description: The url to the git repo where the release-service-catalog tasks and stepactions to be used are stored
Evidence
PR Compliance ID 1 requires YAML changes to keep line length at or below 120 characters. The added
Task YAML includes multiple single lines that clearly exceed that limit (e.g., a very long
description: value, a long image digest line, and a long kubectl command line within the YAML
script: block).

CLAUDE.md
tasks/managed/collect-signing-params/collect-signing-params.yaml[49-49]
tasks/managed/collect-signing-params/collect-signing-params.yaml[134-134]
tasks/managed/collect-signing-params/collect-signing-params.yaml[148-148]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new task YAML has lines exceeding 120 characters (yamllint max-line-length), which can break CI lint checks.
## Issue Context
This repository enforces YAML formatting constraints including max 120-character line length.
## Fix Focus Areas
- tasks/managed/collect-signing-params/collect-signing-params.yaml[49-49]
- tasks/managed/collect-signing-params/collect-signing-params.yaml[134-134]
- tasks/managed/collect-signing-params/collect-signing-params.yaml[148-148]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


18. Test pipeline image too-long 📘 Rule violation ⛯ Reliability
Description
The new test Pipeline YAML includes a single image: line exceeding 120 characters, violating the
repository yamllint style rules. This can cause lint/CI failures for YAML formatting checks.
Code

tasks/managed/collect-signing-params/tests/test-collect-signing-params-missing-cm.yaml[76]

+            image: quay.io/konflux-ci/release-service-utils@sha256:5546fa78d3c88d7b6a2e8cff8902f7757f00541d0bbaf113b9f293133894afa3
Evidence
PR Compliance ID 1 enforces a maximum 120-character line length for YAML files. The added test
pipeline contains a long, pinned image digest on a single line that exceeds this limit.

CLAUDE.md
tasks/managed/collect-signing-params/tests/test-collect-signing-params-missing-cm.yaml[76-76]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The test pipeline YAML uses a pinned `image:` value on a single line that exceeds the 120-character yamllint limit.
## Issue Context
YAML changes must conform to repository yamllint rules, including max line length.
## Fix Focus Areas
- tasks/managed/collect-signing-params/tests/test-collect-signing-params-missing-cm.yaml[76-76]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


19. Test pipeline invalid 🐞 Bug ✓ Correctness
Description
The added test Pipeline references a missing setup task and uses its results, and the inline
check-result taskSpec has duplicate/missing param declarations. As written, this Pipeline will
fail Tekton validation and never execute the task behavior it intends to test.
Code

tasks/managed/collect-signing-params/tests/test-collect-signing-params-missing-cm.yaml[R30-87]

+    - name: run-task
+      taskRef:
+        name: collect-signing-params
+      params:
+        - name: ociStorage
+          value: $(params.ociStorage)
+        - name: orasOptions
+          value: $(params.orasOptions)
+        - name: sourceDataArtifact
+          value: "$(tasks.setup.results.sourceDataArtifact)=$(params.dataDir)"
+        - name: dataDir
+          value: $(params.dataDir)
+        - name: trustedArtifactsDebug
+          value: $(params.trustedArtifactsDebug)
+        - name: taskGitUrl
+          value: "http://localhost"
+        - name: taskGitRevision
+          value: "main"
+      runAfter:
+        - setup
+    - name: check-result
+      params:
+        - name: defaultOIDCIssuer
+          value: $(tasks.run-task.results.defaultOIDCIssuer)
+        - name: rekorExternalUrl
+          value: $(tasks.run-task.results.rekorExternalUrl)
+        - name: fulcioExternalUrl
+          value: $(tasks.run-task.results.fulcioExternalUrl)
+        - name: tufExternalUrl
+          value: $(tasks.run-task.results.tufExternalUrl)
+        - name: buildIdentityRegexp
+          value: $(tasks.run-task.results.buildIdentityRegexp)
+      taskSpec:
+        params:
+          - name: buildIdentityRegexp
+            type: string
+          - name: rekorExternalUrl
+            type: string
+          - name: fulcioExternalUrl
+            type: string
+          - name: tufExternalUrl
+            type: string
+          - name: buildIdentityRegexp
+            type: string
+        steps:
+          - name: check-result
+            image: quay.io/konflux-ci/release-service-utils@sha256:5546fa78d3c88d7b6a2e8cff8902f7757f00541d0bbaf113b9f293133894afa3
+            env:
+              - name: "DEFAULT_OIDC_ISSUER"
+                value: '$(params.defaultOIDCIssuer)'
+              - name: "REKOR_EXTERNAL_URL"
+                value: '$(params.rekorExternalUrl)'
+              - name: "FULCIO_EXTERNAL_URL"
+                value: '$(params.fulcioExternalUrl)'
+              - name: "TUF_EXTERNAL_URL"
+                value: '$(params.tufExternalUrl)'
+              - name: "BUILD_IDENTITY_REGEXP"
+                value: '$(params.buildIdentityRegexp)'
Evidence
The new test Pipeline uses runAfter: setup and $(tasks.setup.results.sourceDataArtifact) but
does not define a setup task, and its check-result taskSpec declares buildIdentityRegexp twice
while not declaring defaultOIDCIssuer even though it is referenced. Existing tests in this repo
consistently define a setup task that produces sourceDataArtifact and define taskSpec params
that match the params they pass/use.

tasks/managed/collect-signing-params/tests/test-collect-signing-params-missing-cm.yaml[30-49]
tasks/managed/collect-signing-params/tests/test-collect-signing-params-missing-cm.yaml[62-87]
tasks/managed/collect-atlas-params/tests/test-collect-atlas-params-nonexistent.yaml[30-37]
tasks/managed/collect-atlas-params/tests/test-collect-atlas-params-nonexistent.yaml[68-90]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new test Pipeline `test-collect-signing-params-missing-cm` cannot validate/run because it references a missing `setup` task and defines an invalid `check-result` taskSpec parameter list.
### Issue Context
Other managed-task tests in this repo use a `setup` task to produce `sourceDataArtifact`, then run the task under test, then a `check-result` task with a taskSpec that declares exactly the params it uses.
### Fix Focus Areas
- tasks/managed/collect-signing-params/tests/test-collect-signing-params-missing-cm.yaml[30-49]
- tasks/managed/collect-signing-params/tests/test-collect-signing-params-missing-cm.yaml[62-87]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


20. Test creates namespace 🐞 Bug ✓ Correctness
Description
The success-path test creates $(context.pipelineRun.namespace) under set -eux; the CI test
harness runs pipelines in the current (already-existing) namespace, so this can fail with
AlreadyExists and break CI.
Code

tasks/managed/collect-signing-params/tests/test-collect-signing-params.yaml[R53-57]

+              #!/usr/bin/env bash
+              set -eux
+
+              kubectl create namespace "$(context.pipelineRun.namespace)"
+              kubectl create configmap cluster-config --namespace "$(context.pipelineRun.namespace)" \
Evidence
The test step runs kubectl create namespace ... with set -eux so any AlreadyExists is fatal. The
shared CI harness applies the Pipeline and starts it without specifying a namespace, meaning it runs
in the current namespace (which must already exist for kubectl apply / tkn to work), making
namespace creation redundant and likely failing.

tasks/managed/collect-signing-params/tests/test-collect-signing-params.yaml[53-62]
[.github/scripts/test_tekton_tasks.sh...

@snyk-io
Copy link
Copy Markdown

snyk-io Bot commented Mar 4, 2026

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Code Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

Comment thread tasks/managed/collect-signing-params/collect-signing-params.yaml Outdated
Comment thread tasks/managed/collect-signing-params/collect-signing-params.yaml Outdated
Comment thread tasks/managed/collect-signing-params/collect-signing-params.yaml Outdated
@Allda Allda force-pushed the ISV-6717 branch 6 times, most recently from 0588e49 to ae98b1d Compare March 4, 2026 16:14
@Allda Allda marked this pull request as ready for review March 4, 2026 16:14
@qodo-code-review
Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Add collect-signing-params task for keyless signing configuration

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Add new Tekton task to collect keyless signing configuration
• Task reads Konflux cluster-config ConfigMap from konflux-info namespace
• Returns empty values when ConfigMap is missing, allowing pipeline continuation
• Includes comprehensive test coverage for both success and missing ConfigMap scenarios
Diagram
flowchart LR
  A["Tekton Pipeline"] -->|"reads cluster-config ConfigMap"| B["collect-signing-params Task"]
  B -->|"ConfigMap found"| C["Extract signing parameters"]
  B -->|"ConfigMap missing"| D["Return empty values"]
  C -->|"outputs"| E["OIDC Issuer, Rekor, Fulcio, TUF URLs"]
  D -->|"outputs"| E
Loading

Grey Divider

File Changes

1. tasks/managed/collect-signing-params/README.md 📝 Documentation +24/-0

Task documentation with parameters and usage

• Documents the new collect-signing-params task purpose and functionality
• Lists all task parameters with descriptions, optional flags, and default values
• Explains behavior when ConfigMap is not found on cluster

tasks/managed/collect-signing-params/README.md


2. tasks/managed/collect-signing-params/collect-signing-params.yaml ✨ Enhancement +174/-0

Main task implementation for signing config collection

• Defines Tekton Task to collect keyless signing configuration from cluster-config ConfigMap
• Implements graceful fallback to empty values when ConfigMap is missing
• Extracts five signing parameters: defaultOIDCIssuer, rekorExternalUrl, fulcioExternalUrl,
 tufExternalUrl, buildIdentityRegexp
• Uses trusted-artifact step and kubectl/jq for ConfigMap retrieval and parsing

tasks/managed/collect-signing-params/collect-signing-params.yaml


3. tasks/managed/collect-signing-params/tests/test-collect-signing-params.yaml 🧪 Tests +140/-0

Test pipeline for successful ConfigMap retrieval

• Test pipeline that verifies task behavior when ConfigMap exists
• Creates cluster-config ConfigMap with test values in setup step
• Validates that task correctly extracts and returns all signing parameters
• Includes result verification step to assert expected values

tasks/managed/collect-signing-params/tests/test-collect-signing-params.yaml


View more (1)
4. tasks/managed/collect-signing-params/tests/test-collect-signing-params-missing-cm.yaml 🧪 Tests +124/-0

Test pipeline for missing ConfigMap scenario

• Test pipeline that verifies task behavior when ConfigMap is missing
• Runs task without creating the cluster-config ConfigMap
• Validates that task returns empty strings for all signing parameters
• Ensures pipeline continues gracefully without signing configuration

tasks/managed/collect-signing-params/tests/test-collect-signing-params-missing-cm.yaml


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review Bot commented Mar 4, 2026

Code Review by Qodo

🐞 Bugs (5) 📘 Rule violations (2) 📎 Requirement gaps (0)

Grey Divider


Action required

1. Test creates namespace 🐞 Bug ✓ Correctness ⭐ New
Description
The success-path test creates $(context.pipelineRun.namespace) under set -eux; the CI test
harness runs pipelines in the current (already-existing) namespace, so this can fail with
AlreadyExists and break CI.
Code

tasks/managed/collect-signing-params/tests/test-collect-signing-params.yaml[R53-57]

+              #!/usr/bin/env bash
+              set -eux
+
+              kubectl create namespace "$(context.pipelineRun.namespace)"
+              kubectl create configmap cluster-config --namespace "$(context.pipelineRun.namespace)" \
Evidence
The test step runs kubectl create namespace ... with set -eux so any AlreadyExists is fatal. The
shared CI harness applies the Pipeline and starts it without specifying a namespace, meaning it runs
in the current namespace (which must already exist for kubectl apply / tkn to work), making
namespace creation redundant and likely failing.

tasks/managed/collect-signing-params/tests/test-collect-signing-params.yaml[53-62]
.github/scripts/test_tekton_tasks.sh[232-265]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`test-collect-signing-params.yaml` runs `kubectl create namespace $(context.pipelineRun.namespace)` under `set -eux`, which will fail if the namespace already exists (common in CI harnesses that run all tests in a fixed namespace).

## Issue Context
The repo CI harness applies the Pipeline and starts it without setting a namespace explicitly, so tests run in the current namespace (already exists).

## Fix Focus Areas
- tasks/managed/collect-signing-params/tests/test-collect-signing-params.yaml[53-62]
- .github/scripts/test_tekton_tasks.sh[232-265]

### Suggested approach
- Remove the `kubectl create namespace ...` line entirely, or
- Make it idempotent, e.g. `kubectl get namespace &quot;$ns&quot; &gt;/dev/null 2&gt;&amp;1 || kubectl create namespace &quot;$ns&quot;`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. taskGitUrl description exceeds limit 📘 Rule violation ⛯ Reliability
Description
The new Tekton Task YAML contains lines longer than 120 characters, which violates the repository
yamllint style rules and can fail CI linting. Long lines appear in the param description, the pinned
image reference, and a long inline bash command.
Code

tasks/managed/collect-signing-params/collect-signing-params.yaml[49]

+      description: The url to the git repo where the release-service-catalog tasks and stepactions to be used are stored
Evidence
PR Compliance ID 1 requires YAML changes to keep line length at or below 120 characters. The added
Task YAML includes multiple single lines that clearly exceed that limit (e.g., a very long
description: value, a long image digest line, and a long kubectl command line within the YAML
script: block).

CLAUDE.md
tasks/managed/collect-signing-params/collect-signing-params.yaml[49-49]
tasks/managed/collect-signing-params/collect-signing-params.yaml[134-134]
tasks/managed/collect-signing-params/collect-signing-params.yaml[148-148]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new task YAML has lines exceeding 120 characters (yamllint max-line-length), which can break CI lint checks.
## Issue Context
This repository enforces YAML formatting constraints including max 120-character line length.
## Fix Focus Areas
- tasks/managed/collect-signing-params/collect-signing-params.yaml[49-49]
- tasks/managed/collect-signing-params/collect-signing-params.yaml[134-134]
- tasks/managed/collect-signing-params/collect-signing-params.yaml[148-148]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Test pipeline image too-long 📘 Rule violation ⛯ Reliability
Description
The new test Pipeline YAML includes a single image: line exceeding 120 characters, violating the
repository yamllint style rules. This can cause lint/CI failures for YAML formatting checks.
Code

tasks/managed/collect-signing-params/tests/test-collect-signing-params-missing-cm.yaml[76]

+            image: quay.io/konflux-ci/release-service-utils@sha256:5546fa78d3c88d7b6a2e8cff8902f7757f00541d0bbaf113b9f293133894afa3
Evidence
PR Compliance ID 1 enforces a maximum 120-character line length for YAML files. The added test
pipeline contains a long, pinned image digest on a single line that exceeds this limit.

CLAUDE.md
tasks/managed/collect-signing-params/tests/test-collect-signing-params-missing-cm.yaml[76-76]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The test pipeline YAML uses a pinned `image:` value on a single line that exceeds the 120-character yamllint limit.
## Issue Context
YAML changes must conform to repository yamllint rules, including max line length.
## Fix Focus Areas
- tasks/managed/collect-signing-params/tests/test-collect-signing-params-missing-cm.yaml[76-76]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View more (3)
4. Test pipeline invalid 🐞 Bug ✓ Correctness
Description
The added test Pipeline references a missing setup task and uses its results, and the inline
check-result taskSpec has duplicate/missing param declarations. As written, this Pipeline will
fail Tekton validation and never execute the task behavior it intends to test.
Code

tasks/managed/collect-signing-params/tests/test-collect-signing-params-missing-cm.yaml[R30-87]

+    - name: run-task
+      taskRef:
+        name: collect-signing-params
+      params:
+        - name: ociStorage
+          value: $(params.ociStorage)
+        - name: orasOptions
+          value: $(params.orasOptions)
+        - name: sourceDataArtifact
+          value: "$(tasks.setup.results.sourceDataArtifact)=$(params.dataDir)"
+        - name: dataDir
+          value: $(params.dataDir)
+        - name: trustedArtifactsDebug
+          value: $(params.trustedArtifactsDebug)
+        - name: taskGitUrl
+          value: "http://localhost"
+        - name: taskGitRevision
+          value: "main"
+      runAfter:
+        - setup
+    - name: check-result
+      params:
+        - name: defaultOIDCIssuer
+          value: $(tasks.run-task.results.defaultOIDCIssuer)
+        - name: rekorExternalUrl
+          value: $(tasks.run-task.results.rekorExternalUrl)
+        - name: fulcioExternalUrl
+          value: $(tasks.run-task.results.fulcioExternalUrl)
+        - name: tufExternalUrl
+          value: $(tasks.run-task.results.tufExternalUrl)
+        - name: buildIdentityRegexp
+          value: $(tasks.run-task.results.buildIdentityRegexp)
+      taskSpec:
+        params:
+          - name: buildIdentityRegexp
+            type: string
+          - name: rekorExternalUrl
+            type: string
+          - name: fulcioExternalUrl
+            type: string
+          - name: tufExternalUrl
+            type: string
+          - name: buildIdentityRegexp
+            type: string
+        steps:
+          - name: check-result
+            image: quay.io/konflux-ci/release-service-utils@sha256:5546fa78d3c88d7b6a2e8cff8902f7757f00541d0bbaf113b9f293133894afa3
+            env:
+              - name: "DEFAULT_OIDC_ISSUER"
+                value: '$(params.defaultOIDCIssuer)'
+              - name: "REKOR_EXTERNAL_URL"
+                value: '$(params.rekorExternalUrl)'
+              - name: "FULCIO_EXTERNAL_URL"
+                value: '$(params.fulcioExternalUrl)'
+              - name: "TUF_EXTERNAL_URL"
+                value: '$(params.tufExternalUrl)'
+              - name: "BUILD_IDENTITY_REGEXP"
+                value: '$(params.buildIdentityRegexp)'
Evidence
The new test Pipeline uses runAfter: setup and $(tasks.setup.results.sourceDataArtifact) but
does not define a setup task, and its check-result taskSpec declares buildIdentityRegexp twice
while not declaring defaultOIDCIssuer even though it is referenced. Existing tests in this repo
consistently define a setup task that produces sourceDataArtifact and define taskSpec params
that match the params they pass/use.

tasks/managed/collect-signing-params/tests/test-collect-signing-params-missing-cm.yaml[30-49]
tasks/managed/collect-signing-params/tests/test-collect-signing-params-missing-cm.yaml[62-87]
tasks/managed/collect-atlas-params/tests/test-collect-atlas-params-nonexistent.yaml[30-37]
tasks/managed/collect-atlas-params/tests/test-collect-atlas-params-nonexistent.yaml[68-90]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new test Pipeline `test-collect-signing-params-missing-cm` cannot validate/run because it references a missing `setup` task and defines an invalid `check-result` taskSpec parameter list.
### Issue Context
Other managed-task tests in this repo use a `setup` task to produce `sourceDataArtifact`, then run the task under test, then a `check-result` task with a taskSpec that declares exactly the params it uses.
### Fix Focus Areas
- tasks/managed/collect-signing-params/tests/test-collect-signing-params-missing-cm.yaml[30-49]
- tasks/managed/collect-signing-params/tests/test-collect-signing-params-missing-cm.yaml[62-87]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


5. Unquoted kubectl params 🐞 Bug ⛨ Security
Description
The task builds a kubectl command with unquoted Tekton params, which allows word-splitting and
potential shell metacharacter injection if params are attacker-controlled. This is inconsistent with
other tasks in the repo that quote variables passed to kubectl.
Code

tasks/managed/collect-signing-params/collect-signing-params.yaml[148]

+        if kubectl get configmap $(params.configMapName) -n $(params.configMapNamespace) -o json > $KFLX_CONFIG_PATH 2>/dev/null; then
Evidence
The kubectl invocation interpolates $(params.configMapName) and $(params.configMapNamespace)
without quotes. In bash, that can lead to word splitting and command injection. A comparable managed
task in the repo demonstrates the expected quoting pattern when passing dynamic values to kubectl.

tasks/managed/collect-signing-params/collect-signing-params.yaml[147-149]
tasks/managed/push-artifacts-to-cdn/push-artifacts-to-cdn.yaml[183-185]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The task runs `kubectl get configmap` using unquoted Tekton parameters, which enables word-splitting and potential command injection.
### Issue Context
Other tasks in this repo quote dynamic values when calling `kubectl`.
### Fix Focus Areas
- tasks/managed/collect-signing-params/collect-signing-params.yaml[148-148]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


6. Kubectl errors hidden 🐞 Bug ⛯ Reliability
Description
All kubectl failures (including RBAC denial or cluster issues) are treated as “ConfigMap not found”
and converted into empty outputs, which can silently disable signing and makes debugging difficult.
The README states empty outputs should indicate the ConfigMap is missing, but stderr is suppressed
and the error type is not checked.
Code

tasks/managed/collect-signing-params/collect-signing-params.yaml[R147-166]

+        # Attempt to fetch the ConfigMap, capture exit code
+        if kubectl get configmap $(params.configMapName) -n $(params.configMapNamespace) -o json > $KFLX_CONFIG_PATH 2>/dev/null; then
+            echo "ConfigMap found, extracting signing parameters"
+
+            # Extract signing parameters from ConfigMap data, defaulting to empty string if not found
+            defaultOIDCIssuer=$(jq -r '.data.defaultOIDCIssuer // ""' "$KFLX_CONFIG_PATH")
+            rekorExternalUrl=$(jq -r '.data.rekorExternalUrl // ""' "$KFLX_CONFIG_PATH")
+            fulcioExternalUrl=$(jq -r '.data.fulcioExternalUrl // ""' "$KFLX_CONFIG_PATH")
+            tufExternalUrl=$(jq -r '.data.tufExternalUrl // ""' "$KFLX_CONFIG_PATH")
+            buildIdentityRegexp=$(jq -r '.data.buildIdentityRegexp // ""' "$KFLX_CONFIG_PATH")
+        else
+            echo "ConfigMap not found, using default empty values"
+
+            # Set all parameters to empty strings when ConfigMap doesn't exist
+            defaultOIDCIssuer=""
+            rekorExternalUrl=""
+            fulcioExternalUrl=""
+            tufExternalUrl=""
+            buildIdentityRegexp=""
+        fi
Evidence
The task redirects kubectl stderr to /dev/null and branches to the “not found” behavior for any
non-zero exit. That means permission errors and transient failures look identical to missing
ConfigMap. The README explicitly frames empty outputs as the behavior when the ConfigMap is not
found.

tasks/managed/collect-signing-params/collect-signing-params.yaml[147-166]
tasks/managed/collect-signing-params/README.md[7-8]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The task suppresses kubectl stderr and treats any kubectl failure as a missing ConfigMap, which can silently skip signing on RBAC/outage errors.
### Issue Context
The README describes empty outputs as the behavior when the ConfigMap is not found. The implementation should match that intent by distinguishing NotFound from other errors.
### Fix Focus Areas
- tasks/managed/collect-signing-params/collect-signing-params.yaml[147-166]
- tasks/managed/collect-signing-params/README.md[7-8]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

7. jq parse not checked 🐞 Bug ⛯ Reliability ⭐ New
Description
If jq fails to parse the ConfigMap JSON, the script continues and emits empty results, making a
parsing/format error indistinguishable from a deliberate “missing ConfigMap” skip.
Code

tasks/managed/collect-signing-params/collect-signing-params.yaml[R152-157]

+            # Extract signing parameters from ConfigMap data, defaulting to empty string if not found
+            defaultOIDCIssuer=$(jq -r '.data.defaultOIDCIssuer // ""' "$KFLX_CONFIG_PATH")
+            rekorExternalUrl=$(jq -r '.data.rekorExternalUrl // ""' "$KFLX_CONFIG_PATH")
+            fulcioExternalUrl=$(jq -r '.data.fulcioExternalUrl // ""' "$KFLX_CONFIG_PATH")
+            tufExternalUrl=$(jq -r '.data.tufExternalUrl // ""' "$KFLX_CONFIG_PATH")
+            buildIdentityRegexp=$(jq -r '.data.buildIdentityRegexp // ""' "$KFLX_CONFIG_PATH")
Evidence
The script assigns from jq without strict mode or explicit error checking. If jq returns non-zero,
the task can still proceed and later write empty/unset values as results. In contrast, other tasks
in this repo frequently use set -ex to fail fast on errors.

tasks/managed/collect-signing-params/collect-signing-params.yaml[142-157]
tasks/managed/update-cr-status/update-cr-status.yaml[124-132]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`jq` extraction failures are not checked, so a malformed/empty JSON file can lead to empty results and a successful task run.

## Issue Context
This is distinct from the intentional “ConfigMap missing =&gt; empty values” behavior: if the ConfigMap fetch succeeded, extraction/parsing problems should be surfaced.

## Fix Focus Areas
- tasks/managed/collect-signing-params/collect-signing-params.yaml[142-157]

### Suggested approach
- After a successful `kubectl get`, validate the JSON before extracting (e.g., `jq -e . &quot;$KFLX_CONFIG_PATH&quot; &gt;/dev/null`).
- Consider enabling `set -euo pipefail` *after* you’ve handled the “NotFound is acceptable” branch, or use explicit `|| { ...; exit 1; }` guards for the jq invocations.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review Bot commented Mar 4, 2026

Persistent review updated to latest commit b595bd5

1 similar comment
@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review Bot commented Mar 4, 2026

Persistent review updated to latest commit b595bd5

Comment thread tasks/managed/collect-signing-params/tests/test-collect-signing-params.yaml Outdated
@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review Bot commented Mar 4, 2026

Persistent review updated to latest commit c806bca

Comment thread tasks/managed/collect-signing-params/tests/pre-apply-task-hook.sh Outdated
@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review Bot commented Mar 4, 2026

Persistent review updated to latest commit c806bca

@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review Bot commented Mar 4, 2026

Persistent review updated to latest commit da81013

Comment thread tasks/managed/collect-signing-params/collect-signing-params.yaml
@gbenhaim
Copy link
Copy Markdown
Member

/test

@qodo-code-review
Copy link
Copy Markdown
Contributor

PR-Agent: Missing component name in test command: /test <component_name>.
Running automatically on the largest changed component.


✨ Test tool usage guide:

The test tool generate tests for a selected component, based on the PR code changes.
It can be invoked manually by commenting on any PR:

/test component_name

where 'component_name' is the name of a specific component in the PR. To get a list of the components that changed in the PR, use the analyze tool.
Language that are currently supported: Python, Java, C++, JavaScript, TypeScript.

Configuration options:

  • num_tests: number of tests to generate. Default is 3.
  • testing_framework: the testing framework to use. If not set, for Python it will use pytest, for Java it will use JUnit, for C++ it will use Catch2, and for JavaScript and TypeScript it will use jest.
  • avoid_mocks: if set to true, the tool will try to avoid using mocks in the generated tests. Note that even if this option is set to true, the tool might still use mocks if it cannot generate a test without them. Default is true.
  • extra_instructions: Optional extra instructions to the tool. For example: "use the following mock injection scheme: ...".
  • file: in case there are several components with the same name, you can specify the relevant file.
  • class_name: in case there are several components with the same name in the same file, you can specify the relevant class name.

See more information about the test tool in the docs.

@davidmogar
Copy link
Copy Markdown
Collaborator

/retest

2 similar comments
@Allda
Copy link
Copy Markdown
Contributor Author

Allda commented Mar 17, 2026

/retest

@midnightercz
Copy link
Copy Markdown
Contributor

/retest

@gbenhaim
Copy link
Copy Markdown
Member

Is there an issue with the e2e tests?

@Allda
Copy link
Copy Markdown
Contributor Author

Allda commented Mar 17, 2026

Is there an issue with the e2e tests?

Apparently yes, the attempt to fix it is here #2063

Comment thread tasks/managed/collect-signing-params/collect-signing-params.yaml
Comment thread tasks/managed/collect-signing-params/collect-signing-params.yaml
@Allda
Copy link
Copy Markdown
Contributor Author

Allda commented Mar 17, 2026

/retest

@gbenhaim
Copy link
Copy Markdown
Member

@johnbieren was the collector fixed? If so, how to update tests to use the fix?

@johnbieren
Copy link
Copy Markdown
Collaborator

@johnbieren was the collector fixed? If so, how to update tests to use the fix?

The collectors are fixed, but the collector test won't work until
#2060
#2059
#2061
are merged, since the collector test runs the rh-advisories pipeline

Copy link
Copy Markdown
Collaborator

@johnbieren johnbieren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll respond here when a rebase should result in passing e2e. Hopefully early tomorrow

@johnbieren
Copy link
Copy Markdown
Collaborator

I'll respond here when a rebase should result in passing e2e. Hopefully early tomorrow

@Allda tests should pass with a rebase now

A new task reads Konflux configuration and expose the
values for keyless signing. In case the config map is not present on a
cluster, the task returns empty values which indicates the keyless
signing should be skipped.

Signed-off-by: Ales Raszka <araszka@redhat.com>

feat(ISV-6717): add collect-signing-params to rh-advisory pipeline

A rh-advisory pipeline will sign sboms using keyless method and this
commit prepares the configuration.

Signed-off-by: Ales Raszka <araszka@redhat.com>

feat(ISV-6717): update mobster tasks in push-rpms-to-pulp

A latest version of Mobster git revision. It doesn't have any affect on
the pipeline since there are no changes. It just allows deprecate older
version.

Signed-off-by: Ales Raszka <araszka@redhat.com>

feat(ISV-6717): add collect-signing-params to push-to-external-registry

A task reads a configuration for keyless signing. The output of the task
will be used in other PR implementing a SBOM singature.

Signed-off-by: Ales Raszka <araszka@redhat.com>

fix: readme format

Signed-off-by: Ales Raszka <araszka@redhat.com>

fix: apply Qodo code review suggestions

Signed-off-by: Ales Raszka <araszka@redhat.com>

fix: address code review comments

Signed-off-by: Ales Raszka <araszka@redhat.com>

fix: remove trusted artifact from task

Signed-off-by: Ales Raszka <araszka@redhat.com>
Comment thread tasks/managed/collect-signing-params/collect-signing-params.yaml
Comment thread pipelines/managed/rh-advisories/README.md
@Allda
Copy link
Copy Markdown
Contributor Author

Allda commented Mar 18, 2026

@johnbieren test passed. Ready to merge?

@johnbieren
Copy link
Copy Markdown
Collaborator

@johnbieren test passed. Ready to merge?

I marked my approval, but you need @davidmogar @ach912 and a codeowner like @gbenhaim to approve before we can merge

@Allda
Copy link
Copy Markdown
Contributor Author

Allda commented Mar 19, 2026

@davidmogar @ach912 Could you please approve this PR? It is blocking several other PRs and it was just wating till the CI is fixed.

@BorekZnovustvoritel
Copy link
Copy Markdown
Contributor

For context: this change blocks work that is necessary for completing TSF work, which has a deadline by the end of March. 2-3 other PRs need to be based on this.

@gbenhaim gbenhaim enabled auto-merge March 19, 2026 11:03
@gbenhaim gbenhaim added this pull request to the merge queue Mar 19, 2026
Merged via the queue into konflux-ci:development with commit b7a76b1 Mar 19, 2026
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants