feat(ISV-7115): add rh-direct-sign-image task

[Allda]

New task for signing images using the direct signing method, which will replace rh-sign-image that goes through Radas.

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
• If an AI agent was used, I marked that via a commit footer like Assisted-By: Cursor

[Allda]

This PR is based on another PR in utils repo: konflux-ci/release-service-utils#819

[qodo-app-for-konflux-ci]

PR Reviewer Guide 🔍

(Review updated until commit d108f22)

Warning

/review is deprecated. Use /agentic_review instead (removal date not yet scheduled).

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Test Mock The mock script is named with a .py suffix but is implemented as Bash. This can be confusing for contributors and for any tooling that assumes the file extension matches the interpreter. Consider either renaming the mock to match its shell implementation or converting it to a real Python mock. Also, since it writes to DATA_DIR, validate that the environment variable is set (or fail with a clear message) and consider using set -euo pipefail for more robust failure behavior. #!/usr/bin/env bash set -eux echo "Mock rh_direct_sign_image.py called with: $*" echo "$*" >> "${DATA_DIR}/mock_rh_direct_sign_image.txt" Fragility The yq patch targets .spec.steps[1], which is brittle if step ordering changes (e.g., adding/removing/reordering steps). Consider selecting the step by name (e.g., sign-image) rather than by index. Also, kubectl secret creation/deletion without an explicit namespace may affect the wrong namespace depending on the test harness context; consider scoping it explicitly if the test framework provides a namespace. # Inject DATA_DIR env var into the sign-image step so the mock binary can use it # without relying on Tekton parameter substitution inside the mock script itself yq -i '.spec.steps[1].env += [{"name": "DATA_DIR", "value": "$(params.dataDir)"}]' "$1" # Create a dummy pyxis secret (and delete it first if it exists) kubectl delete secret test-pyxis-image-cert --ignore-not-found kubectl create secret generic test-pyxis-image-cert --from-literal=cert=mycert --from-literal=key=mykey

📚 Focus areas based on broader codebase context Type Mismatch Several params are declared with type: string but have unquoted non-string defaults (e.g. concurrentLimit: 8, batchLimit: 15000, pipelineImage: quay.io/...:latest). This can lead to inconsistent typing/serialization across Tekton tooling and makes the task spec less consistent with other tasks’ parameter defaults. (Ref 5) - name: concurrentLimit type: string description: The maximum number of signing requests to run in parallel default: 8 - name: pipelineRunUid type: string description: >- The uid of the current pipelineRun. Used as a label value when creating internal requests - name: pyxisServer type: string description: >- The server type to use. Options are 'production','production-internal','stage-internal' and 'stage' default: production - name: pyxisSecret type: string description: | The kubernetes secret to use to authenticate to Pyxis. It needs to contain two keys: key and cert - name: batchLimit type: string description: | size of batch attributes to send to internal-request. As internal request arguments are need to be strings, size here represent maximal string length of `references` and `manifest_digests` sent to internal request default: 15000 - name: signRegistryAccessPath type: string description: | The relative path in the workspace to a text file that contains a list of repositories that needs registry.access.redhat.com image references to be signed (i.e. requires_terms=true), one repository string per line, e.g. "rhtas/cosign-rhel9" - name: ociStorage description: The OCI repository where the Trusted Artifacts are stored type: string default: "empty" - name: ociArtifactExpiresAfter description: Expiration date for the trusted artifacts created in the OCI repository. An empty string means the artifacts do not expire type: string default: "1d" - name: trustedArtifactsDebug description: >- Flag to enable debug logging in trusted artifacts. Set to a non-empty string to enable type: string default: "" - name: orasOptions description: oras options to pass to Trusted Artifacts calls type: string default: "" - name: sourceDataArtifact type: string description: Location of trusted artifacts to be used to populate data directory default: "" - name: dataDir description: The location where data will be stored type: string default: /var/workdir/release - name: taskGitUrl type: string description: >- The url to the git repo where the release-service-catalog tasks and stepactions to be used are stored - name: taskGitRevision type: string description: The revision in the taskGitUrl repo to be used - name: signingRepo type: string description: Git repository URL containing the signing tasks default: "https://gitlab.cee.redhat.com/signing/signing.git" - name: signingRevision type: string description: Git revision (branch, tag, or commit) in the signing repository default: "main" - name: signPipeline type: string description: Name of the internal pipeline to use for container signing default: "container-signing" - name: signPipelineServiceAccount type: string description: Service account to use for the signing pipeline default: "signing-pipeline-sa" - name: pipelineImage type: string description: The image to use for the signing pipeline default: quay.io/konflux-ci/signing:latest - name: caTrustConfigMapName Reference reasoning: Existing tasks define string defaults as explicit strings (e.g. empty-string defaults like default: ""), which avoids ambiguity in YAML parsing and matches the declared type. Aligning these defaults to quoted strings keeps the task consistent with established patterns and reduces risk of type coercion issues.

📄 References konflux-ci/release-service-catalog/tasks/managed/rh-sign-image/rh-sign-image.yaml [1-11] konflux-ci/release-service-catalog/tasks/managed/rh-sign-image/rh-sign-image.yaml [96-130] konflux-ci/release-service-catalog/tasks/managed/rh-sign-image/rh-sign-image.yaml [154-168] konflux-ci/release-service-catalog/tasks/managed/rh-sign-image/rh-sign-image.yaml [131-152] konflux-ci/release-service-catalog/tasks/managed/rh-sign-image-cosign/rh-sign-image-cosign.yaml [1-26] konflux-ci/release-service-catalog/tasks/managed/rh-sign-image-cosign/rh-sign-image-cosign.yaml [117-150] konflux-ci/release-service-catalog/pipelines/managed/rh-advisories/rh-advisories.yaml [597-612] konflux-ci/release-service-catalog/pipelines/managed/rh-push-to-registry-redhat-io/rh-push-to-registry-redhat-io.yaml [401-412]

[qodo-app-for-konflux-ci]

Code Review by Qodo

🐞 Bugs (5) 📘 Rule violations (6) 📜 Skill insights (0)

Context used

✅ Compliance rules (platform): 28 rules

1. Mock command-substitutes params ✓ Resolved 🐞 Bug ☼ Reliability

Description

tests/mocks/rh_direct_sign_image.py writes to "$(params.dataDir)/...", but in a bash script this
is command substitution (tries to execute params.dataDir) and will fail, preventing the test from
producing the expected mock args file. This breaks the new task’s Tekton test pipeline which asserts
on $(params.dataDir)/mock_rh_direct_sign_image.txt.

Code

tasks/managed/rh-direct-sign-image/tests/mocks/rh_direct_sign_image.py[R1-4]

+#!/usr/bin/env bash
+set -eux
+echo "Mock rh_direct_sign_image.py called with: $*"
+echo "$*" >> "$(params.dataDir)/mock_rh_direct_sign_image.txt"

Relevance

⭐⭐⭐ High

Similar issue flagged in PR#2190 mocks; team fixes breaking Tekton tests/mocks reliability patterns
frequently.

PR-#2190
PR-#2106

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

Mocks are embedded verbatim into a generated step script via a quoted heredoc (cat <<'DELIM' ...),
so Tekton placeholders inside mock files are not substituted; bash then interprets
$(params.dataDir) as command substitution. The test pipeline expects the mock to write its args
into $(params.dataDir)/mock_rh_direct_sign_image.txt, so the failure prevents the assertion from
passing.

tasks/managed/rh-direct-sign-image/tests/mocks/rh_direct_sign_image.py[1-4]
.github/scripts/render_python_task_mocks_from_yaml.py[86-99]
.github/scripts/render_python_task_mocks_from_yaml.py[192-220]
.github/scripts/test_tekton_tasks.sh[18-86]
tasks/managed/rh-direct-sign-image/tests/test-rh-direct-sign-image.yaml[124-134]

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 mock binary `rh_direct_sign_image.py` is executed by bash, not by Tekton substitution. Using `$(params.dataDir)` inside it is parsed as command substitution and fails.

### Issue Context
The Tekton test harness embeds mock binaries via a **single-quoted heredoc** (no expansion), then runs them from `$PATH`. The test later expects the mock to have written `mock_rh_direct_sign_image.txt` under `$(params.dataDir)`.

### Fix Focus Areas
- tasks/managed/rh-direct-sign-image/tests/mocks/rh_direct_sign_image.py[1-4]

### Suggested fix
Update the mock to write into the real dataDir without Tekton placeholders. Options:
1) Derive `dataDir` from the passed args (e.g., parse `--data-file <path>` or `--snapshot <path>` and strip the `/<pipelinerun_uid>/...` suffix), then write `${dataDir}/mock_rh_direct_sign_image.txt`.
2) Or write to a fixed path that matches the test’s `dataDir` (e.g., `/workspace/data/mock_rh_direct_sign_image.txt` in PVC-mode tests), and keep the test assertion aligned.

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

---

2. publish-pyxis-repository README not regenerated ✓ Resolved 📘 Rule violation ≡ Correctness

Description

The Tekton YAML description was updated to reference rh-direct-sign-image, but the generated
README still references rh-sign-image. This indicates the README was not regenerated after
modifying the task YAML, so task documentation in-repo is now inconsistent.

Code

tasks/managed/publish-pyxis-repository/publish-pyxis-repository.yaml[32]

+    for the signing tasks (and `rh-direct-sign-image` runs quite early to begin with). So this means

Relevance

⭐⭐⭐ High

Repo has repeated PRs to keep generated task READMEs in sync with YAML; drift fixes merged (e.g.,
#1161).

PR-#1161
PR-#1158
PR-#1160

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 494 requires regenerating the README whenever a Tekton YAML under tasks/ is
modified. The YAML change updates the signing task reference, but the README still contains the old
reference, showing the regeneration step is missing.

Rule 494: README must be regenerated when Tekton YAML is modified
tasks/managed/publish-pyxis-repository/publish-pyxis-repository.yaml[31-33]
tasks/managed/publish-pyxis-repository/README.md[23-25]

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 Tekton task YAML under `tasks/` was modified, but its corresponding `README.md` was not updated/regenerated, leaving documentation inconsistent.

## Issue Context
`publish-pyxis-repository.yaml` now references `rh-direct-sign-image`, while `tasks/managed/publish-pyxis-repository/README.md` still mentions `rh-sign-image`.

## Fix Focus Areas
- tasks/managed/publish-pyxis-repository/publish-pyxis-repository.yaml[31-33]
- tasks/managed/publish-pyxis-repository/README.md[23-25]

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

---

3. Missing CLI script ✓ Resolved 🐞 Bug ≡ Correctness

Description

The sign-image step runs python rh_direct_sign_image.py, but this PR does not add that file nor
does the task fetch/create it in the workspace, so the step will fail with “No such file” unless the
image happens to ship it in its working directory.

Code

tasks/managed/rh-direct-sign-image/rh-direct-sign-image.yaml[R198-214]

+        python rh_direct_sign_image.py \
+          --pyxis-server "$(params.pyxisServer)" \
+          --snapshot "$(params.dataDir)/$(params.snapshotPath)" \
+          --data-file "$(params.dataDir)/$(params.dataPath)" \
+          --sign-registry-access-file "$(params.dataDir)/$(params.signRegistryAccessPath)" \
+          --batch-max-size "$(params.batchLimit)" \
+          --requester "$(params.requester)" \
+          --pipeline "$(params.signPipeline)" \
+          --service-account "$(params.signPipelineServiceAccount)" \
+          --request-timeout "$(params.requestTimeout)" \
+          --task-id "$(context.taskRun.uid)" \
+          --pipelinerun-uid "$(params.pipelineRunUid)" \
+          --signing-repo "$(params.signingRepo)" \
+          --signing-revision "$(params.signingRevision)" \
+          --concurrent-limit "$(params.concurrentLimit)" \
+          --pipeline-image "$(params.pipelineImage)" \
+          --submit-requests

Relevance

⭐⭐⭐ High

Team fixes task runtime failures from missing prerequisites (e.g., missing creds in image caused
failures fixed in PR #2200).

PR-#2200

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The task calls a relative-path Python script, and the only other reference in this PR is a test mock
that intercepts calls to that filename; there is no step in the task that provides the script before
execution.

tasks/managed/rh-direct-sign-image/rh-direct-sign-image.yaml[176-214]
tasks/managed/rh-direct-sign-image/tests/mocks.sh[6-17]

Agent prompt

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

### Issue description
`tasks/managed/rh-direct-sign-image/rh-direct-sign-image.yaml` invokes `python rh_direct_sign_image.py`, but that script is not present in this repository and the task does not download/mount it before execution. This makes the task non-functional in a real cluster unless the container image coincidentally contains the script in the step’s working directory.

### Issue Context
The tests “pass” by prepending a mock `python()` shell function (see `tests/mocks.sh`), which bypasses the need for the real script. Production runs will not have that mock.

### Fix Focus Areas
- tasks/managed/rh-direct-sign-image/rh-direct-sign-image.yaml[176-214]

### What to change
Choose one concrete approach and implement it:
1. **Use a packaged entrypoint**: replace `python rh_direct_sign_image.py ...` with a stable, installed CLI (e.g., `rh-direct-sign-image ...` or similar) that is known to exist in the chosen image.
2. **Fetch the script at runtime**: add a step (or extend the step) to clone/download the repository that contains `rh_direct_sign_image.py` into `$(params.dataDir)` and invoke it via an **explicit path** (e.g., `python "$(params.dataDir)/.../rh_direct_sign_image.py" ...`).
3. **Pin to an image that contains the script** and invoke it via its **absolute** path in the image filesystem.

Also update/extend the tests so they validate the chosen execution path (i.e., don’t rely solely on mocking a missing script).

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

---

View more (7)

4. Missing CPU limits in steps ✓ Resolved 📘 Rule violation ☼ Reliability

Description

Tekton steps define computeResources without CPU limits, violating the requirement to specify both
CPU and memory requests/limits for every step. Missing limits can lead to noisy-neighbor issues and
unpredictable scheduling.

Code

tasks/managed/rh-direct-sign-image/rh-direct-sign-image.yaml[R149-216]

+    - name: use-trusted-artifact
+      computeResources:
+        limits:
+          memory: 64Mi
+        requests:
+          memory: 64Mi
+          cpu: 30m
+      ref:
+        resolver: "git"
+        params:
+          - name: url
+            value: $(params.taskGitUrl)
+          - name: revision
+            value: $(params.taskGitRevision)
+          - name: pathInRepo
+            value: stepactions/use-trusted-artifact/use-trusted-artifact.yaml
+      params:
+        - name: workDir
+          value: $(params.dataDir)
+        - name: sourceDataArtifact
+          value: $(params.sourceDataArtifact)
+
+    - name: sign-image
+      # TODO: Change the image once the CLI is available in quay.io/konflux-ci/release-service-utils
+      image: quay.io/konflux-ci/release-service-utils@sha256:3cb03b14ac9d90ff27070036ce2b50712e65aa285daeb28852254a745bb25dfc
+      computeResources:
+        limits:
+          memory: 4Gi
+        requests:
+          memory: 4Gi
+          cpu: "2"
+      volumeMounts:
+        - name: pyxis-secret-vol
+          mountPath: "/etc/secrets"
+      env:
+        - name: PYXIS_CERT_PATH
+          value: /etc/secrets/cert
+        - name: PYXIS_KEY_PATH
+          value: /etc/secrets/key
+      script: |
+        #!/usr/bin/env bash
+        set -eo pipefail
+
+        rh_direct_sign_image \
+          --pyxis-server "$(params.pyxisServer)" \
+          --snapshot "$(params.dataDir)/$(params.snapshotPath)" \
+          --data-file "$(params.dataDir)/$(params.dataPath)" \
+          --sign-registry-access-file "$(params.dataDir)/$(params.signRegistryAccessPath)" \
+          --batch-max-size "$(params.batchLimit)" \
+          --requester "$(params.requester)" \
+          --pipeline "$(params.signPipeline)" \
+          --service-account "$(params.signPipelineServiceAccount)" \
+          --request-timeout "$(params.requestTimeout)" \
+          --task-id "$(context.taskRun.uid)" \
+          --pipelinerun-uid "$(params.pipelineRunUid)" \
+          --signing-repo "$(params.signingRepo)" \
+          --signing-revision "$(params.signingRevision)" \
+          --concurrent-limit "$(params.concurrentLimit)" \
+          --pipeline-image "$(params.pipelineImage)" \
+          --submit-requests
+
+    - name: create-trusted-artifact
+      computeResources:
+        limits:
+          memory: 128Mi
+        requests:
+          memory: 128Mi
+          cpu: 250m

Evidence

The rule requires cpu+memory requests and limits for all Tekton steps. In the cited
computeResources blocks, limits.cpu is missing while requests.cpu may be present.

Rule 1265: Specify compute resources in all Tekton tasks
tasks/managed/rh-direct-sign-image/rh-direct-sign-image.yaml[149-156]
tasks/managed/rh-direct-sign-image/rh-direct-sign-image.yaml[174-179]
tasks/managed/rh-direct-sign-image/rh-direct-sign-image.yaml[211-216]

Agent prompt

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

## Issue description
Some Tekton steps specify memory limits/requests but omit CPU limits.

## Issue Context
Compliance requires each step to define both `cpu` and `memory` under both `requests` and `limits`.

## Fix Focus Areas
- tasks/managed/rh-direct-sign-image/rh-direct-sign-image.yaml[149-156]
- tasks/managed/rh-direct-sign-image/rh-direct-sign-image.yaml[171-179]
- tasks/managed/rh-direct-sign-image/rh-direct-sign-image.yaml[210-216]

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

---

5. pre-apply uses bare $SCRIPT_DIR ✓ Resolved 📘 Rule violation ⚙ Maintainability

Description

The script uses a bare $SCRIPT_DIR expansion instead of the required brace form ${SCRIPT_DIR}.
This violates the brace-expansion requirement and can make concatenations/templating more
error-prone.

Code

tasks/managed/rh-direct-sign-image/tests/pre-apply-task-hook.sh[R9-10]

+SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )
+yq -i '.spec.steps[1].script = load_str("'$SCRIPT_DIR'/mocks.sh") + .spec.steps[1].script' "$TASK_PATH"

Evidence

The checklist requires brace syntax for shell variable expansion. The yq command interpolates
$SCRIPT_DIR without braces.

Rule 1264: Use brace syntax for shell variable expansion
tasks/managed/rh-direct-sign-image/tests/pre-apply-task-hook.sh[9-10]

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 non-special shell variable is referenced without brace syntax.

## Issue Context
The compliance rule requires `${VAR}` form for variable expansion (except special parameters).

## Fix Focus Areas
- tasks/managed/rh-direct-sign-image/tests/pre-apply-task-hook.sh[9-10]

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

---

6. rh-direct-sign-image README.md changed 📘 Rule violation ⚙ Maintainability

Description

A README.md under tasks/ was added in this PR, which is disallowed by the compliance checklist for
auto-generated READMEs. Keeping this change risks developers manually editing generated
documentation instead of using the generator workflow.

Code

tasks/managed/rh-direct-sign-image/README.md[R1-33]

+# rh-direct-sign-image
+
+Task to create internalrequests to directly sign snapshot components
+
+## Parameters
+
+| Name                       | Description                                                                                                                                                                                                                                      | Optional | Default value                                     |
+|----------------------------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|----------|---------------------------------------------------|
+| snapshotPath               | Path to the JSON string of the mapped Snapshot spec in the data workspace                                                                                                                                                                        | No       | -                                                 |
+| dataPath                   | Path to the JSON string of the merged data to use in the data workspace                                                                                                                                                                          | No       | -                                                 |
+| requester                  | Name of the user that requested the signing, for auditing purposes                                                                                                                                                                               | No       | -                                                 |
+| requestTimeout             | InternalRequest timeout                                                                                                                                                                                                                          | Yes      | 1800                                              |
+| concurrentLimit            | The maximum number of signing requests to run in parallel                                                                                                                                                                                        | Yes      | 8                                                 |
+| pipelineRunUid             | The uid of the current pipelineRun. Used as a label value when creating internal requests                                                                                                                                                        | No       | -                                                 |
+| pyxisServer                | The server type to use. Options are 'production','production-internal,'stage-internal' and 'stage'                                                                                                                                               | Yes      | production                                        |
+| pyxisSecret                | The kubernetes secret to use to authenticate to Pyxis. It needs to contain two keys: key and cert                                                                                                                                                | No       | -                                                 |
+| batchLimit                 | size of batch attributes to send to internal-request. As internal request arguments are need to be strings, size here represent maximal string length of `references` and `manifest_digests` sent to internal request                            | Yes      | 15000                                             |
+| signRegistryAccessPath     | The relative path in the workspace to a text file that contains a list of repositories that needs registry.access.redhat.com image references to be signed (i.e. requires_terms=true), one repository string per line, e.g. "rhtas/cosign-rhel9" | No       | -                                                 |
+| ociStorage                 | The OCI repository where the Trusted Artifacts are stored                                                                                                                                                                                        | Yes      | empty                                             |
+| ociArtifactExpiresAfter    | Expiration date for the trusted artifacts created in the OCI repository. An empty string means the artifacts do not expire                                                                                                                       | Yes      | 1d                                                |
+| trustedArtifactsDebug      | Flag to enable debug logging in trusted artifacts. Set to a non-empty string to enable                                                                                                                                                           | Yes      | ""                                                |
+| orasOptions                | oras options to pass to Trusted Artifacts calls                                                                                                                                                                                                  | Yes      | ""                                                |
+| sourceDataArtifact         | Location of trusted artifacts to be used to populate data directory                                                                                                                                                                              | Yes      | ""                                                |
+| dataDir                    | The location where data will be stored                                                                                                                                                                                                           | Yes      | /var/workdir/release                              |
+| taskGitUrl                 | The url to the git repo where the release-service-catalog tasks and stepactions to be used are stored                                                                                                                                            | No       | -                                                 |
+| taskGitRevision            | The revision in the taskGitUrl repo to be used                                                                                                                                                                                                   | No       | -                                                 |
+| signingRepo                | Git repository URL containing the signing tasks                                                                                                                                                                                                  | Yes      | https://gitlab.cee.redhat.com/signing/signing.git |
+| signingRevision            | Git revision (branch, tag, or commit) in the signing repository                                                                                                                                                                                  | Yes      | main                                              |
+| signPipeline               | Name of the internal pipeline to use for container signing                                                                                                                                                                                       | Yes      | container-signing                                 |
+| signPipelineServiceAccount | Service account to use for the signing pipeline                                                                                                                                                                                                  | Yes      | signing-pipeline-sa                               |
+| pipelineImage              | The image to use for the signing pipeline                                                                                                                                                                                                        | Yes      | quay.io/konflux-ci/signing:latest                 |
+| caTrustConfigMapName       | The name of the ConfigMap to read CA bundle data from                                                                                                                                                                                            | Yes      | trusted-ca                                        |
+| caTrustConfigMapKey        | The name of the key in the ConfigMap that contains the CA bundle data                                                                                                                                                                            | Yes      | ca-bundle.crt                                     |

Evidence

The rule forbids adding/modifying README.md files under tasks/. The PR adds
tasks/managed/rh-direct-sign-image/README.md content.

Rule 1260: Do not manually edit auto-generated README files under tasks/ and pipelines/
tasks/managed/rh-direct-sign-image/README.md[1-33]

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 `README.md` under `tasks/` was added/changed, which violates the rule that forbids modifying auto-generated READMEs in `tasks/` and `pipelines/`.

## Issue Context
The compliance checklist requires that generated READMEs are not edited directly.

## Fix Focus Areas
- tasks/managed/rh-direct-sign-image/README.md[1-33]

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

---

7. sign-image missing EXIT trap 📘 Rule violation ☼ Reliability

Description

The sign-image step script does not use an EXIT trap to ensure a result status is written on
both success and failure, and instead relies on non-zero process exit codes (set -e) to signal
business-logic failure. This can cause the step to fail without producing the Tekton results
expected by downstream consumers, violating the requirement to always exit 0 and communicate status
through results.

Code

tasks/managed/rh-direct-sign-image/rh-direct-sign-image.yaml[R188-208]

+      script: |
+        #!/usr/bin/env bash
+        set -eo pipefail
+
+        rh_direct_sign_image \
+          --pyxis-server "$(params.pyxisServer)" \
+          --snapshot "$(params.dataDir)/$(params.snapshotPath)" \
+          --data-file "$(params.dataDir)/$(params.dataPath)" \
+          --sign-registry-access-file "$(params.dataDir)/$(params.signRegistryAccessPath)" \
+          --batch-max-size "$(params.batchLimit)" \
+          --requester "$(params.requester)" \
+          --pipeline "$(params.signPipeline)" \
+          --service-account "$(params.signPipelineServiceAccount)" \
+          --request-timeout "$(params.requestTimeout)" \
+          --task-id "$(context.taskRun.uid)" \
+          --pipelinerun-uid "$(params.pipelineRunUid)" \
+          --signing-repo "$(params.signingRepo)" \
+          --signing-revision "$(params.signingRevision)" \
+          --concurrent-limit "$(params.concurrentLimit)" \
+          --pipeline-image "$(params.pipelineImage)" \
+          --submit-requests

Evidence

The checklist requires Tekton step scripts to install an EXIT trap that writes a success/failure
status to Tekton results based on the script’s exit code, but the cited sign-image script contains
no trap ... EXIT and does not write a status result. Additionally, the script uses `set -eo
pipefail and directly invokes rh_direct_sign_image`, so any non-zero return will terminate the
step with a non-zero exit code, demonstrating that business-logic failures are being surfaced via
process exit status rather than via results while exiting 0 as required.

Rule 1272: Use EXIT trap in Tekton task scripts to write result status
Rule 1256: Tekton task scripts must always exit 0 and use results for status
tasks/managed/rh-direct-sign-image/rh-direct-sign-image.yaml[188-208]

Agent prompt

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

## Issue description
Update the `sign-image` Tekton step script so it always exits 0 and consistently writes a result status (success/failure) via Tekton results on both success and failure paths by using an `EXIT` trap.

## Issue Context
The current script has no `trap ... EXIT` and does not write a status result; it also uses `set -eo pipefail` with a direct `rh_direct_sign_image` invocation, so any non-zero return causes the step to fail via exit code instead of reporting logical failure through results.

## Fix Focus Areas
- tasks/managed/rh-direct-sign-image/rh-direct-sign-image.yaml[113-117]
- tasks/managed/rh-direct-sign-image/rh-direct-sign-image.yaml[188-208]

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

---

8. Test scripts missing pipefail 📘 Rule violation ☼ Reliability

Description

Several Tekton step scripts in the added pipeline tests start with set -eux and omit `-o
pipefail. This violates the requirement that Tekton task scripts must start with set -eo pipefail`
(or stronger) before any other commands.

Code

tasks/managed/rh-direct-sign-image/tests/test-rh-direct-sign-image.yaml[R126-129]

+            script: |
+              #!/usr/bin/env bash
+              set -eux
+

Evidence

The checklist requires Tekton script: blocks to begin with set -eo pipefail (or a stronger
variant including pipefail). The cited scripts use set -eux and therefore omit pipefail.

Rule 1268: Shell scripts in Tekton tasks must start with set -eo pipefail
tasks/managed/rh-direct-sign-image/tests/test-rh-direct-sign-image.yaml[126-129]
tasks/managed/rh-direct-sign-image/tests/test-rh-direct-sign-image-cli-failure.yaml[51-54]
tasks/managed/rh-direct-sign-image/tests/test-rh-direct-sign-image-cli-failure.yaml[122-125]

Agent prompt

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

## Issue description
Tekton step scripts must include `pipefail` and set it before any other executable command.

## Issue Context
The pipeline test Task steps use `set -eux` which omits `pipefail`.

## Fix Focus Areas
- tasks/managed/rh-direct-sign-image/tests/test-rh-direct-sign-image.yaml[126-129]
- tasks/managed/rh-direct-sign-image/tests/test-rh-direct-sign-image-cli-failure.yaml[51-54]
- tasks/managed/rh-direct-sign-image/tests/test-rh-direct-sign-image-cli-failure.yaml[122-125]

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

---

9. tests scripts missing strict flags 📘 Rule violation ☼ Reliability

Description

Added standalone bash scripts do not enable the required strict error handling (`set -euo
pipefail`). This can cause silent failures or unexpected behavior during test setup/mocking.

Code

tasks/managed/rh-direct-sign-image/tests/pre-apply-task-hook.sh[R1-6]

+#!/usr/bin/env bash
+#
+# Create a dummy pyxis secret (and delete it first if it exists)
+kubectl delete secret test-pyxis-image-cert --ignore-not-found
+kubectl create secret generic test-pyxis-image-cert --from-literal=cert=mycert --from-literal=key=mykey
+

Evidence

The rule requires standalone executable shell scripts to unconditionally enable set -euo pipefail.
pre-apply-task-hook.sh has no strict-mode set line, and mocks.sh uses set -eux (missing `-o
pipefail` and the required exact strict combination).

Rule 1261: Shell scripts must enable strict error handling flags
tasks/managed/rh-direct-sign-image/tests/pre-apply-task-hook.sh[1-10]
tasks/managed/rh-direct-sign-image/tests/mocks.sh[1-3]

Agent prompt

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

## Issue description
Standalone shell scripts added in this PR do not enable strict mode (`set -euo pipefail`) as required.

## Issue Context
Strict mode reduces the chance of masking errors in CI/test hooks and mock helpers.

## Fix Focus Areas
- tasks/managed/rh-direct-sign-image/tests/pre-apply-task-hook.sh[1-10]
- tasks/managed/rh-direct-sign-image/tests/mocks.sh[1-13]

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

---

10. taskGitUrl description exceeds 120 ✓ Resolved 📘 Rule violation ⚙ Maintainability

Description

Several added YAML lines exceed 120 characters, which violates the YAML formatting limit and reduces
readability/maintainability. Long strings should be wrapped using YAML folding/literal blocks or
split across lines.

Code

tasks/managed/rh-direct-sign-image/rh-direct-sign-image.yaml[81]

+      description: The url to the git repo where the release-service-catalog tasks and stepactions to be used are stored

Evidence

The checklist requires all changed YAML lines to be <= 120 characters. The cited lines contain long
unwrapped strings (descriptions and image references) that exceed the limit.

Rule 1252: Limit YAML line length to 120 characters
tasks/managed/rh-direct-sign-image/rh-direct-sign-image.yaml[81-81]
tasks/managed/rh-direct-sign-image/rh-direct-sign-image.yaml[173-173]
tasks/managed/rh-direct-sign-image/tests/test-rh-direct-sign-image.yaml[125-125]

Agent prompt

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

## Issue description
YAML files in this PR contain lines longer than 120 characters.

## Issue Context
The repository enforces a 120-character maximum visible line length for YAML; long descriptions/URLs/images should be wrapped using YAML folding (`>-`) or multi-line blocks.

## Fix Focus Areas
- tasks/managed/rh-direct-sign-image/rh-direct-sign-image.yaml[81-81]
- tasks/managed/rh-direct-sign-image/rh-direct-sign-image.yaml[173-173]
- tasks/managed/rh-direct-sign-image/tests/test-rh-direct-sign-image.yaml[125-125]

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

---

11. Test missing input files 🐞 Bug ⚙ Maintainability

Description

The new “happy path” pipeline test doesn’t create snapshot_spec.json, data.json, or
signRegistryAccess.txt even though it passes those paths into rh-direct-sign-image, so it can’t
validate that the task succeeds with real inputs. This reduces test coverage and can let real
input-handling regressions slip through.

Code

tasks/managed/rh-direct-sign-image/tests/test-rh-direct-sign-image.yaml[R29-77]

+    - name: setup
+      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)"
+        steps:
+          - name: create-trusted-artifact
+            ref:
+              name: create-trusted-artifact
+            params:
+              - name: ociStorage
+                value: $(params.ociStorage)
+              - name: workDir
+                value: $(params.dataDir)
+              - name: sourceDataArtifact
+                value: $(results.sourceDataArtifact.path)
+    - name: run-task
+      taskRef:
+        name: rh-direct-sign-image
+      params:
+        - name: requester
+          value: testuser
+        - name: pipelineRunUid
+          value: $(context.pipelineRun.uid)
+        - name: snapshotPath
+          value: $(context.pipelineRun.uid)/snapshot_spec.json
+        - name: dataPath
+          value: $(context.pipelineRun.uid)/data.json
+        - name: pyxisSecret
+          value: test-pyxis-image-cert
+        - name: pyxisServer
+          value: production
+        - name: signRegistryAccessPath
+          value: $(context.pipelineRun.uid)/signRegistryAccess.txt
+        - name: ociStorage

Relevance

⭐⭐ Medium

Team often asks for stronger Tekton task tests (setup + check-result patterns), but similar requests
show mixed/unclear outcomes.

PR-#2168
PR-#2118

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The test’s setup task only creates a trusted artifact and never writes the files that run-task
references; meanwhile, the task itself passes those params as concrete file paths to the signing
command. Existing signing task tests create these input files during setup, demonstrating the
expected pattern.

tasks/managed/rh-direct-sign-image/tests/test-rh-direct-sign-image.yaml[29-90]
tasks/managed/rh-direct-sign-image/rh-direct-sign-image.yaml[197-205]
tasks/managed/rh-sign-image/tests/test-rh-sign-image-single-component.yaml[49-126]

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 `test-rh-direct-sign-image` passes file paths for `snapshotPath`, `dataPath`, and `signRegistryAccessPath` but never writes those files into `$(params.dataDir)` before running `rh-direct-sign-image`, so the test isn’t exercising real-input behavior.

### Issue Context
`rh-direct-sign-image` passes these parameters to the signing CLI as filesystem paths (prefixed by `dataDir`). Existing signing task tests (e.g., `rh-sign-image`) create these fixtures in the `setup` task before running the task.

### Fix Focus Areas
- tasks/managed/rh-direct-sign-image/tests/test-rh-direct-sign-image.yaml[29-59]

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

---

12. Mutable signing defaults 🐞 Bug ⛨ Security

Description

The task defaults signingRevision to main and pipelineImage to a :latest tag, so executions
that rely on defaults are non-deterministic and can change behavior without updating the catalog.
This weakens reproducibility and increases supply-chain risk for the signing pipeline inputs.

Code

tasks/managed/rh-direct-sign-image/rh-direct-sign-image.yaml[R85-104]

+    - name: signingRepo
+      type: string
+      description: Git repository URL containing the signing tasks
+      default: "https://gitlab.cee.redhat.com/signing/signing.git"
+    - name: signingRevision
+      type: string
+      description: Git revision (branch, tag, or commit) in the signing repository
+      default: "main"
+    - name: signPipeline
+      type: string
+      description: Name of the internal pipeline to use for container signing
+      default: "container-signing"
+    - name: signPipelineServiceAccount
+      type: string
+      description: Service account to use for the signing pipeline
+      default: "signing-pipeline-sa"
+    - name: pipelineImage
+      type: string
+      description: The image to use for the signing pipeline
+      default: quay.io/konflux-ci/signing:latest

Evidence

The new task sets mutable defaults for the signing inputs (branch main and image tag latest).
Elsewhere, tasks require explicit git revisions (no default) and commonly pin images by digest,
indicating a stronger determinism baseline than these defaults provide.

tasks/managed/rh-direct-sign-image/rh-direct-sign-image.yaml[79-104]
tasks/managed/rh-direct-sign-image/rh-direct-sign-image.yaml[171-174]
tasks/managed/rh-sign-image/rh-sign-image.yaml[82-88]
stepactions/create-trusted-artifact/create-trusted-artifact.yaml[1-16]

Agent prompt

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

### Issue description
`rh-direct-sign-image` uses mutable defaults (`signingRevision: main`, `pipelineImage: ...:latest`). When callers omit these params, behavior can change over time without any catalog change.

### Issue Context
This repo generally pins execution inputs (e.g., task/step images via digest) and typically requires callers to pass explicit git revisions for resolver-based content.

### Fix Focus Areas
- tasks/managed/rh-direct-sign-image/rh-direct-sign-image.yaml[85-104]

### Suggested fix
- Remove these defaults so callers must explicitly provide pinned values, **or**
- Change defaults to immutable identifiers (e.g., `signingRevision` as a commit SHA/tag, `pipelineImage` as an image digest), and document the pinning requirement.

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

---

13. Path params not explicit 🐞 Bug ⚙ Maintainability

Description

rh-direct-sign-image prefixes dataDir onto snapshotPath and dataPath, but their parameter
descriptions don’t explicitly say these must be relative paths under dataDir. If a caller supplies
an absolute path, the constructed path will be invalid and the task will fail to locate inputs.

Code

tasks/managed/rh-direct-sign-image/rh-direct-sign-image.yaml[R13-18]

+    - name: snapshotPath
+      description: Path to the JSON string of the mapped Snapshot spec in the data workspace
+      type: string
+    - name: dataPath
+      description: Path to the JSON string of the merged data to use in the data workspace
+      type: string

Relevance

⭐⭐ Medium

History shows reviewers sometimes request param/README wording tweaks, but no clear precedent about
“must be relative to dataDir”.

PR-#1245
PR-#1217

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The task’s args show that it always concatenates dataDir with snapshotPath/dataPath; therefore
these params must be provided as relative paths under dataDir for the resulting paths to resolve
correctly.

tasks/managed/rh-direct-sign-image/rh-direct-sign-image.yaml[13-18]
tasks/managed/rh-direct-sign-image/rh-direct-sign-image.yaml[199-205]

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 constructs input paths as `$(params.dataDir)/$(params.snapshotPath)` and `$(params.dataDir)/$(params.dataPath)`, but the param descriptions don’t explicitly state `snapshotPath`/`dataPath` must be relative to `dataDir`.

### Issue Context
`signRegistryAccessPath` is explicitly described as a relative path, but `snapshotPath`/`dataPath` are not, despite being used the same way.

### Fix Focus Areas
- tasks/managed/rh-direct-sign-image/rh-direct-sign-image.yaml[13-18]

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

---

14. No input file precheck 🐞 Bug ☼ Reliability

Description

The sign-image step invokes rh_direct_sign_image with snapshot/data/signRegistryAccess paths
without verifying the files exist first, so misconfiguration fails with less actionable errors. This
reduces debuggability compared to existing signing tasks that validate required files before
proceeding.

Code

tasks/managed/rh-direct-sign-image/rh-direct-sign-image.yaml[R188-208]

+      script: |
+        #!/usr/bin/env bash
+        set -eo pipefail
+
+        rh_direct_sign_image \
+          --pyxis-server "$(params.pyxisServer)" \
+          --snapshot "$(params.dataDir)/$(params.snapshotPath)" \
+          --data-file "$(params.dataDir)/$(params.dataPath)" \
+          --sign-registry-access-file "$(params.dataDir)/$(params.signRegistryAccessPath)" \
+          --batch-max-size "$(params.batchLimit)" \
+          --requester "$(params.requester)" \
+          --pipeline "$(params.signPipeline)" \
+          --service-account "$(params.signPipelineServiceAccount)" \
+          --request-timeout "$(params.requestTimeout)" \
+          --task-id "$(context.taskRun.uid)" \
+          --pipelinerun-uid "$(params.pipelineRunUid)" \
+          --signing-repo "$(params.signingRepo)" \
+          --signing-revision "$(params.signingRevision)" \
+          --concurrent-limit "$(params.concurrentLimit)" \
+          --pipeline-image "$(params.pipelineImage)" \
+          --submit-requests

Evidence

The new task’s script is a direct CLI invocation with no test -f validation, while the existing
rh-sign-image task explicitly checks required input files and fails with clear messages, showing a
repo precedent for preflight validation.

tasks/managed/rh-direct-sign-image/rh-direct-sign-image.yaml[188-208]
tasks/managed/rh-sign-image/rh-sign-image.yaml[197-212]

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 passes file paths to `rh_direct_sign_image` without checking that the referenced files exist (snapshot, data, and signRegistryAccess list). When a path is wrong or missing, failures will occur inside the CLI with less context.

### Issue Context
`rh-sign-image` performs explicit `-f` checks and emits clear error messages before doing work.

### Fix Focus Areas
- tasks/managed/rh-direct-sign-image/rh-direct-sign-image.yaml[188-208]

### Suggested fix
Add bash prechecks before invoking `rh_direct_sign_image`, e.g.:
- verify `$(params.dataDir)/$(params.snapshotPath)` exists
- verify `$(params.dataDir)/$(params.dataPath)` exists
- verify `$(params.dataDir)/$(params.signRegistryAccessPath)` exists
and print explicit error messages + `exit 1` if any are missing.

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

---

15. Brittle step index patch 🐞 Bug ⚙ Maintainability

Description

tests/pre-apply-task-hook.sh injects DATA_DIR into .spec.steps[1], which assumes sign-image will
always be the second step. If steps are reordered/inserted, the env var will be injected into the
wrong step and the mock will fail due to missing DATA_DIR (breaking Tekton tests).

Code

tasks/managed/rh-direct-sign-image/tests/pre-apply-task-hook.sh[6]

+yq -i '.spec.steps[1].env += [{"name": "DATA_DIR", "value": "$(params.dataDir)"}]' "$1"

Relevance

⭐ Low

Repo frequently uses .spec.steps[<index>] in pre-apply hooks; PR #2100 kept index-based patching
despite same brittleness note.

PR-#2100
PR-#2035
PR-#1808

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The hook hard-codes .spec.steps[1] while the task identifies the target step by name
(sign-image), making the patch fragile if step order changes. The mock writes to ${DATA_DIR}, so
mis-injection results in test failure.

tasks/managed/rh-direct-sign-image/tests/pre-apply-task-hook.sh[4-7]
tasks/managed/rh-direct-sign-image/rh-direct-sign-image.yaml[152-196]
.github/scripts/test_tekton_tasks.sh[269-279]
[tasks/managed/rh-direct-sign-image/tests/mocks/rh_direct_sign_image.py[1-4]](https://github.com/konflux-ci/release-service-catalo...

[qodo-app-for-konflux-ci]

Code review by qodo was updated up to the latest commit 96cd1e5

[swickersh]

one small nit.
Lgtm, but will wait to approve until the utils dependency PR lands and the image ref is updated here.

[johnbieren]

Can you take a look at the tests for a python converted task like https://github.com/konflux-ci/release-service-catalog/tree/development/tasks/internal/check-fbc-opt-in? Basically, the mocks.sh goes away and also we only keep one tekton unit test per task (the various scenarios are covered via pytest in the utils repo). Probably the AI tool of your choice can look at the examples (the most are in tasks/internal) and update this one to follow the same style

[Allda]

Can you take a look at the tests for a python converted task like https://github.com/konflux-ci/release-service-catalog/tree/development/tasks/internal/check-fbc-opt-in? Basically, the mocks.sh goes away and also we only keep one tekton unit test per task (the various scenarios are covered via pytest in the utils repo). Probably the AI tool of your choice can look at the examples (the most are in tasks/internal) and update this one to follow the same style

@johnbieren I updated the tests based on the suggestions. Please check if it is correct now.

[qodo-app-for-konflux-ci]

Code review by qodo was updated up to the latest commit 1a2a16d

[qodo-app-for-konflux-ci]

Code review by qodo was updated up to the latest commit ae056f6

[qodo-app-for-konflux-ci]

Code review by qodo was updated up to the latest commit bbc2e7d

[qodo-app-for-konflux-ci]

Code review by qodo was updated up to the latest commit cc3edec

[qodo-app-for-konflux-ci]

Code review by qodo was updated up to the latest commit ea05ae3

[Allda]

@swickersh The reference to the utils image has been updated. Can you approve now? Thanks.

[Allda]

It turns out that there is one more issue that needs to be addressed first konflux-ci/release-service-utils#848

[swickersh]

It turns out that there is one more issue that needs to be addressed first konflux-ci/release-service-utils#848

Ok, yea just ping me again when 848 lands and the image is updated here. Thanks

[qodo-app-for-konflux-ci]

Code review by qodo was updated up to the latest commit d108f22

[Allda]

@swickersh The related PR was merged and the image reference is updated. Feel free to review/approve. Thanks

[theflockers]

file is .py and has bash inside.

[theflockers]

You need to create the files in a previous task, with your "mocking" values.