feat(RELEASE-2214): add multi-version opt-in scenario

[theflockers]

Describe your changes

Relevant Jira

RELEASE-2214

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

[qodo-app-for-konflux-ci]

Code Review by Qodo

Sorry, something went wrong

We weren't able to complete the code review on our side. Please try again

[qodo-app-for-konflux-ci]

Code Review by Qodo

🐞 Bugs (6) 📘 Rule violations (6) 📎 Requirement gaps (0) 📜 Skill insights (0)

Context used

✅ Tickets: RELEASE-2214

✅ Compliance rules (platform): 25 rules

1. stage-internal Pyxis URL mismatch 🐞 Bug ≡ Correctness ⭐ New

Description

In prepare-fbc-parameters, the new pyxisUrl override only triggers when pyxisServer is exactly
"stage", so "stage-internal" will still use the default production Pyxis URL when invoking
check-fbc-opt-in. This can route opt-in checks to the wrong Pyxis environment for stage-internal
executions.

Code

tasks/managed/prepare-fbc-parameters/prepare-fbc-parameters.yaml[R396-400]

+        # PYXIS_URL should be changed to stage when params.pyxisServer is set to "stage"
+        PYXIS_URL_PARAMS=()
+        if [ "$(params.pyxisServer)" = "stage" ]; then
+            PYXIS_URL_PARAMS=(-p pyxisUrl=https://pyxis.stage.engineering.redhat.com/v1)
+        fi

Relevance

⭐⭐⭐ High

Repo explicitly treats stage-internal like stage for Pyxis endpoints (PR1727); likely to align
override accordingly.

PR-#1727
PR-#1638

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The new logic only switches pyxisUrl for stage, but other tasks in this repo treat
stage-internal as a supported pyxisServer value, so stage-internal runs will not get the stage
Pyxis URL override.

tasks/managed/prepare-fbc-parameters/prepare-fbc-parameters.yaml[396-417]
tasks/managed/sign-index-image/sign-index-image.yaml[234-245]

Agent prompt

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

## Issue description
`prepare-fbc-parameters` only appends `-p pyxisUrl=...stage...` when `params.pyxisServer` equals `stage`. This leaves `stage-internal` (which is treated as a valid pyxisServer elsewhere in the repo) using the default production Pyxis URL when calling the `check-fbc-opt-in` internal pipeline.

## Issue Context
The `check-fbc-opt-in` pipeline/task now accepts `pyxisUrl`, so `prepare-fbc-parameters` is responsible for ensuring the URL matches the chosen environment. Other managed tasks explicitly support `stage-internal`.

## Fix Focus Areas
- tasks/managed/prepare-fbc-parameters/prepare-fbc-parameters.yaml[396-400]

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

---

2. setup-values script lacks exit 0 📘 Rule violation ≡ Correctness ⭐ New

Description

The new Tekton script: block in the stage-pyxis test pipeline has no explicit exit 0, so a
command failure can still terminate the step with a non-zero exit code. This violates the
requirement that Tekton task scripts must always exit 0 and use results (not process exit status) to
signal outcome.

Code

tasks/managed/prepare-fbc-parameters/tests/test-prepare-fbc-parameters-stage-pyxis.yaml[R58-101]

+            script: |
+              #!/usr/bin/env bash
+
+              # Ensure data directory exists
+              mkdir -p "$(params.dataDir)/$(context.pipelineRun.uid)"
+
+              # Create test data for successful validation (using actual FBC test images)
+              # Note: ocpVersion is now attached by filter-published-fbc-images task
+              cat > "$(params.dataDir)/$(params.snapshotPath)" << 'EOF'
+              {
+                "components": [
+                  {
+                    "name": "test-fbc-component-1",
+                    "containerImage": "quay.io/hacbs-release-tests/test-ocp-version/test-fbc-component@sha256:f6e744662e342c1321deddb92469b55197002717a15f8c0b1bb2d9440aac2297",
+                    "ocpVersion": "v4.14"
+                  },
+                  {
+                    "name": "test-fbc-component-2",
+                    "containerImage": "quay.io/hacbs-release-tests/test-ocp-version/test-fbc-component@sha256:42183bc9716448d0c25dbe35bb1c61661449270c89576bed8b9d46ec8df08a4a",
+                    "ocpVersion": "v4.14"
+                  }
+                ]
+              }
+              EOF
+
+              cat > "$(params.dataDir)/data.json" << 'EOF'
+              {
+                "fbc": {
+                  "allowedPackages": [
+                    "test-operator",
+                    "example-operator",
+                    "test-package",
+                    "test-package-1",
+                    "test-package-2",
+                    "operator-foundry-e2e-example-operator"
+                  ],
+                  "hotfix": false,
+                  "preGA": false,
+                  "stagedIndex": false
+                }
+              }
+              EOF
+
+              echo "Test data created for stage pyxis integration test"

Relevance

⭐⭐ Medium

“Scripts must exit 0” rule lacks clear repo acceptance; similar results-based-status suggestion not
clearly accepted (PR2191).

PR-#2191

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 1256 requires Tekton task scripts to always end in exit 0 and not rely on
non-zero exits; the newly added setup-values script: block contains multiple commands but no
explicit exit 0 at the end.

Rule 1256: Tekton task scripts must always exit 0 and use results for status
tasks/managed/prepare-fbc-parameters/tests/test-prepare-fbc-parameters-stage-pyxis.yaml[58-101]

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 newly added Tekton `script:` block can exit non-zero because it does not end with an explicit `exit 0`.

## Issue Context
Compliance requires Tekton task scripts to always exit 0 and use results for signaling status instead of the process exit code.

## Fix Focus Areas
- tasks/managed/prepare-fbc-parameters/tests/test-prepare-fbc-parameters-stage-pyxis.yaml[58-101]

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

---

3. Single snapshot selection ambiguous 🐞 Bug ☼ Reliability ⭐ New

Description

The tenant kustomization now always deploys component2.yaml into the main application, but the
“single” release path still selects the newest snapshot with exactly one component for the
application label only. With multiple components in the same application, this can select a snapshot
for the wrong component (whichever built last) and make single-* scenarios nondeterministic/flaky.

Code

integration-tests/fbc-release/resources/tenant/kustomization.yaml[R9-12]

+  - component2.yaml
+  - optin-component.yaml
+  - multi-optin-component-1.yaml
+  - multi-optin-component-2.yaml

Relevance

⭐⭐ Medium

No direct precedent on single-snapshot picker ambiguity; integration-test robustness suggestions
often rejected (PR2071).

PR-#2071
PR-#2156

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The PR makes component2.yaml always present in the tenant, while the single-snapshot picker is
only application-scoped and chooses the most recent 1-component snapshot—this becomes ambiguous when
multiple components in the same application produce 1-component snapshots.

integration-tests/fbc-release/resources/tenant/kustomization.yaml[6-12]
integration-tests/fbc-release/test.sh[260-277]
integration-tests/lib/test-functions.sh[1027-1039]

Agent prompt

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

## Issue description
`component2.yaml` is now always part of the tenant resources, so the main application has multiple components. The shared helper `wait_for_single_component_snapshot()` selects the latest snapshot where `.spec.components | length == 1` scoped only by application label, which can correspond to *either* component and break the intent of `single-*` scenarios.

## Issue Context
`trigger_configured_releases()` calls `wait_for_single_component_snapshot` for non-optin single scenarios without any component constraint. The selector in `wait_for_single_component_snapshot()` does not filter by component name.

## Fix Focus Areas
- integration-tests/fbc-release/resources/tenant/kustomization.yaml[6-12]
- integration-tests/fbc-release/test.sh[260-277]
- integration-tests/lib/test-functions.sh[1027-1039]

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

---

View more (4)

4. Early return skips components 🐞 Bug ≡ Correctness

Description

verify_multi_component_release() returns from inside the per-component loop for non-optin
scenarios, so only the first component is validated when component_count>1. This can hide
failures/regressions in later components for multi-component releases.

Code

integration-tests/fbc-release/test.sh[R534-538]

+        # the upcoming checks are only required for multi optin components
+        # otherwise return earlier
+        if [  "$scenario" != "optin" ]; then
+            return $failures
+        fi

Relevance

⭐⭐⭐ High

Team has recently strengthened multi-component verification logic; skipping components undermines
intent (PR 2144).

PR-#2144

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The non-optin guard uses return $failures inside the per-component loop, which exits the entire
function during the first iteration and skips validation of any remaining components.

integration-tests/fbc-release/test.sh[455-538]

Agent prompt

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

## Issue description
`verify_multi_component_release()` returns from inside the `for((i=0; i<component_count; i++))` loop when the scenario is not `optin`, which prevents validating components at indexes > 0.

## Issue Context
The function is intended to validate every release artifact component; only the *extra* checks (skopeo/BUILD_VERSION validation) should be opt-in-only.

## Fix Focus Areas
- integration-tests/fbc-release/test.sh[455-569]

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

---

5. Tekton script missing set -eo pipefail 📘 Rule violation ☼ Reliability

Description

The new Tekton script: block in the test pipeline does not enable strict error handling, which can
allow failures to be silently ignored. This violates the repository shell strictness standard for
Tekton task scripts.

Code

tasks/managed/prepare-fbc-parameters/tests/test-prepare-fbc-parameters-stage-pyxis.yaml[R58-66]

+            script: |
+              #!/usr/bin/env bash
+
+              # Ensure data directory exists
+              mkdir -p "$(params.dataDir)/$(context.pipelineRun.uid)"
+
+              # Create test data for successful validation (using actual FBC test images)
+              # Note: ocpVersion is now attached by filter-published-fbc-images task
+              cat > "$(params.dataDir)/$(params.snapshotPath)" << 'EOF'

Relevance

⭐⭐ Medium

Strict shell-mode requirements are inconsistent; adding strict mode was rejected in one e2e script
(PR 2136).

PR-#2136

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 6 requires Tekton task scripts to include at least set -eo pipefail. The new
script begins with a bash shebang but does not set strict options before executing commands.

AGENTS.md: Shell scripts must set strict error-handling options appropriate to context
tasks/managed/prepare-fbc-parameters/tests/test-prepare-fbc-parameters-stage-pyxis.yaml[58-66]

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 `script:` block was added without strict bash error handling options.

## Issue Context
Per repo standards, Tekton task scripts must include at least `set -eo pipefail` to avoid silent failures.

## Fix Focus Areas
- tasks/managed/prepare-fbc-parameters/tests/test-prepare-fbc-parameters-stage-pyxis.yaml[58-66]

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

---

6. Unquoted $1 assignment 📘 Rule violation ≡ Correctness

Description

The new logic assigns effective_application_name=$1 without quotes or brace form, risking
word-splitting/globbing and violating the shell expansion standard. Similar unquoted expansions
appear in new calls using variables.

Code

integration-tests/fbc-release/test.sh[R159-163]

wait_for_multi_component_snapshot() {
+    # replace global with local values
+    local effective_application_name=${application_name}
+    [ -n "$1" ] && effective_application_name=$1
+

Relevance

⭐⭐ Medium

No clear history enforcing shell quoting; similar strict-mode/robustness nits often rejected (PR
2136).

PR-#2136

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 7 requires quoting and brace-form expansions. The updated function assigns from
$1 without quotes/braces, and later invokes registry_authenticate with an unquoted variable
argument.

AGENTS.md: Shell variable and command expansions must be quoted and use brace form
integration-tests/fbc-release/test.sh[159-163]
integration-tests/fbc-release/test.sh[546-549]

Agent prompt

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

## Issue description
New shell code uses unquoted/unbraced variable and positional-parameter expansions (e.g., `effective_application_name=$1`). This can break when values contain spaces or glob characters.

## Issue Context
Repo shell standards require brace form and quoting: `"${VAR}"` and `"${1}"`.

## Fix Focus Areas
- integration-tests/fbc-release/test.sh[159-163]
- integration-tests/fbc-release/test.sh[546-549]

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

---

7. jq index treated as string 🐞 Bug ≡ Correctness

Description

verify_multi_component_release() uses jq --arg i "$i" while indexing an array with
components[$i], passing the index as a string rather than a number. This is inconsistent with
other array indexing in the same script (--argjson i) and can cause null/empty reads from the
release JSON and incorrect verification results.

Code

integration-tests/fbc-release/test.sh[R461-466]

+        fbc_fragment=$(jq -r --arg i "$i" ".status.artifacts.components[$i].fbc_fragment // \"\"" <<< "${release_json}")
+        ocp_version=$(jq -r --arg i "$i" ".status.artifacts.components[$i].ocp_version // \"\"" <<< "${release_json}")
+        iib_log=$(jq -r --arg i "$i" ".status.artifacts.components[$i].iibLog // \"\"" <<< "${release_json}")
+        index_image=$(jq -r --arg i "$i" ".status.artifacts.components[$i].index_image // \"\"" <<< "${release_json}")
+        index_image_resolved=$(jq -r --arg i "$i" ".status.artifacts.components[$i].index_image_resolved // \"\"" <<< "${release_json}")
+        target_index=$(jq -r --arg i "$i" ".status.artifacts.components[$i].target_index // \"\"" <<< "${release_json}")

Relevance

⭐⭐ Medium

No direct historical decisions on --arg vs --argjson indexing; could be treated as correctness but
evidence lacking.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

verify_multi_component_release() indexes .status.artifacts.components[$i] using --arg i
(string). Elsewhere in the same file, array indexing is done with --argjson i for numeric indices,
showing the expected safe pattern.

integration-tests/fbc-release/test.sh[461-466]
integration-tests/fbc-release/test.sh[610-613]
integration-tests/fbc-release/test.sh[682-685]

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 loop index is passed to jq via `--arg i` (string), but used as an array index (`components[$i]`). This can yield incorrect/null lookups.

## Issue Context
The same script already uses `--argjson i` for array indexing in other functions, which indicates the intended pattern.

## Fix Focus Areas
- integration-tests/fbc-release/test.sh[461-466]

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

---

8. Unsafe index digest rewrite 🐞 Bug ≡ Correctness

Description

verify_multi_component_release() rewrites index_image to a quay.io digest using
${index_image_resolved#*@} without validating that index_image_resolved is non-empty and actually
contains an '@' digest separator, which can construct an invalid image reference and make skopeo
inspect fail.

Code

integration-tests/fbc-release/test.sh[R540-545]

+        # we can't reach the image in registry-proxy, so we need to resolve it to quay.io
+        # a conditional is not necessary because the test optin index image is only
+        # available in quay.io/redhat/redhat----preview-operator-index.
+        image_sha="${index_image_resolved#*@}"
+        index_image="quay.io/redhat/redhat----preview-operator-index@${image_sha}"
+

Relevance

⭐⭐ Medium

No clear prior acceptance/rejection for validating '@' digest before ${var#*@}; robustness changes
in tests often mixed.

PR-#2144
PR-#2183

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The code reads index_image_resolved from JSON as a free-form string and then blindly strips
everything up to '@' to form a digest; there is no guard ensuring the value contains '@' or is
non-empty.

integration-tests/fbc-release/test.sh[461-466]
integration-tests/fbc-release/test.sh[540-545]

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 multi opt-in verification path assumes `index_image_resolved` is a digest reference and rewrites it into a hard-coded quay.io digest form via shell parameter expansion. If `index_image_resolved` is empty or tag-based (no `@`), the resulting `index_image` becomes invalid and `skopeo inspect` will fail.

### Issue Context
`index_image_resolved` is pulled from release status JSON and is not guaranteed by this script to be digest-shaped.

### Fix Focus Areas
- integration-tests/fbc-release/test.sh[461-466]
- integration-tests/fbc-release/test.sh[540-553]

### Suggested fix approach
- Validate before rewriting:
 - `index_image_resolved` must be non-empty
 - must match `*@sha256:*` (or at least contain `@sha256:`)
- If validation fails, increment `failures` with a clear error message and `continue` (or `return`), instead of constructing a broken reference.
- Consider using the resolved reference directly if it’s already pullable/authenticated, rather than rewriting to a hard-coded repo.

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

---

9. Auth config not validated 🐞 Bug ☼ Reliability

Description

registry_authenticate() blindly decodes the first Secret whose name contains a substring and writes
DOCKER_CONFIG/config.json without verifying a match was found (or that only one matched), so later
skopeo inspect can fail with confusing auth errors.

Code

integration-tests/fbc-release/test.sh[R409-425]

+registry_authenticate() {
+    # k8s secret containing the dockerconfigjson
+    SECRET_YAML_PATH="$1"
+    SECRET_NAME="$2"
+
+    # skopeo, podman and docker auth
+    DOCKER_CONFIG="$(mktemp -d)"
+    REGISTRY_AUTH_FILE="${DOCKER_CONFIG}/auth.json"
+
+    export DOCKER_CONFIG SECRET_NAME REGISTRY_AUTH_FILE
+
+    yq '. | select(.metadata.name | contains(env(SECRET_NAME))) | .data.".dockerconfigjson"' \
+    "${SECRET_YAML_PATH}" | base64 -d > "${DOCKER_CONFIG}/config.json"
+
+    # creates auth.json for podman
+    ln -s ${DOCKER_CONFIG}/config.json ${DOCKER_CONFIG}/auth.json
+}

Relevance

⭐ Low

Similar hardening of dockerconfig secret selection was explicitly rejected in PR #2136 review
history.

PR-#2136

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The function uses a substring selector and pipes its output straight into base64 decoding, with no
checks for null/multiple matches or decode failure, so it can generate an invalid docker config that
breaks skopeo auth.

integration-tests/fbc-release/test.sh[409-425]

Agent prompt

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

### Issue description
`registry_authenticate()` builds a docker auth config by selecting a Secret via a substring match and piping the result directly into `base64 -d`. If the selector matches **zero** secrets (null/empty) or **multiple** secrets (multiple base64 blobs), the generated config can be invalid and downstream `skopeo inspect` failures become non-obvious.

### Issue Context
This function is used in the new multi opt-in verification path to authenticate `skopeo` against quay.

### Fix Focus Areas
- integration-tests/fbc-release/test.sh[409-425]

### Suggested fix approach
- Make `SECRET_YAML_PATH` / `SECRET_NAME` local.
- Extract matches into a variable and validate:
 - non-empty
 - exactly one match (e.g., count matches; fail with a clear message otherwise)
- Check `base64 -d` exit status and abort with an explicit error if decoding fails.
- (Optional) Prefer exact match on `.metadata.name` if you know the expected secret name, instead of `contains()`.

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

---

10. Edited pipelines/.../README.md 📘 Rule violation ⚙ Maintainability

Description

pipelines/internal/check-fbc-opt-in/README.md and tasks/internal/check-fbc-opt-in/README.md were
modified in this PR, which is not allowed for auto-generated README files under pipelines/ and
tasks/. Manual edits can drift from the generator output and cause inconsistent documentation.

Code

pipelines/internal/check-fbc-opt-in/README.md[R8-15]

+| Name                    | Description                                                                                           | Optional | Default value                           |
+|-------------------------|-------------------------------------------------------------------------------------------------------|----------|-----------------------------------------|
+| containerImages         | JSON array of container images to check for FBC opt-in status                                         | No       | -                                       |
+| iibServiceAccountSecret | Secret with IIB service account credentials to be used for Pyxis authentication                       | No       | -                                       |
+| pyxisUrl                | Pyxis Url to use                                                                                      | Yes      | https://pyxis.engineering.redhat.com/v1 |
+| pyxisServer             | Pyxis server to use                                                                                   | Yes      | production                              |
+| 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       | -                                       |

Relevance

⭐ Low

Repo expects README updates via generator; large README.md updates have been merged (generator PR
#1157), so edits aren’t forbidden.

PR-#1157
PR-#1159

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 1260 forbids modifying auto-generated README.md files under tasks/ and
pipelines/, and the cited line ranges show direct content changes to the parameter documentation:
updated parameter table content in pipelines/internal/check-fbc-opt-in/README.md and updated
parameter rows in tasks/internal/check-fbc-opt-in/README.md, demonstrating that these generated
files were edited manually in the PR.

Rule 1260: Do not manually edit auto-generated README files under tasks/ and pipelines/
pipelines/internal/check-fbc-opt-in/README.md[8-15]
tasks/internal/check-fbc-opt-in/README.md[12-13]

Agent prompt

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

## Issue description
READMEs under `pipelines/` and `tasks/` were modified directly, which violates the rule forbidding manual edits to auto-generated READMEs in `tasks/` and `pipelines/`.

## Issue Context
These READMEs are expected to be maintained via the repo's README generator, not by hand edits.

## Fix Focus Areas
- pipelines/internal/check-fbc-opt-in/README.md[8-15]
- tasks/internal/check-fbc-opt-in/README.md[12-13]

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

---

11. DOCKER_CONFIG temp dir uncleaned 📘 Rule violation ☼ Reliability

Description

registry_authenticate() creates a temporary directory with mktemp -d but never cleans it up,
which can leak temporary artifacts across runs. This violates the temp-file lifecycle management
requirement.

Code

integration-tests/fbc-release/test.sh[R409-425]

+registry_authenticate() {
+    # k8s secret containing the dockerconfigjson
+    SECRET_YAML_PATH="$1"
+    SECRET_NAME="$2"
+
+    # skopeo, podman and docker auth
+    DOCKER_CONFIG="$(mktemp -d)"
+    REGISTRY_AUTH_FILE="${DOCKER_CONFIG}/auth.json"
+
+    export DOCKER_CONFIG SECRET_NAME REGISTRY_AUTH_FILE
+
+    yq '. | select(.metadata.name | contains(env(SECRET_NAME))) | .data.".dockerconfigjson"' \
+    "${SECRET_YAML_PATH}" | base64 -d > "${DOCKER_CONFIG}/config.json"
+
+    # creates auth.json for podman
+    ln -s ${DOCKER_CONFIG}/config.json ${DOCKER_CONFIG}/auth.json
+}

Relevance

⭐ Low

Team has rejected cleanup/trap hardening for mktemp/temp artifacts in past reviews (PR 2236).

PR-#2236

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 14 requires temporary directories/files to be created with mktemp and cleaned up
via an EXIT trap. The function creates DOCKER_CONFIG with mktemp -d and does not remove it or
trap cleanup.

AGENTS.md: Temporary files and directory changes must be safely managed (cleanup traps and pushd/popd)
integration-tests/fbc-release/test.sh[409-425]

Agent prompt

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

## Issue description
`registry_authenticate()` creates a temp directory via `mktemp -d` but does not register an EXIT trap to remove it.

## Issue Context
Repo scripting standards require temporary files/directories created with `mktemp` to be cleaned up via an EXIT trap to prevent leaks.

## Fix Focus Areas
- integration-tests/fbc-release/test.sh[409-425]

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

---

View more (1)

12. YAML line exceeds 120 chars 📘 Rule violation ⚙ Maintainability

Description

The new Tekton test pipeline YAML includes very long single-line scalar values (e.g., image: and
containerImage: digests) that exceed the repository’s 120-character limit. This violates the YAML
style conventions required by the repo.

Code

tasks/managed/prepare-fbc-parameters/tests/test-prepare-fbc-parameters-stage-pyxis.yaml[R56-77]

+          - name: setup-values
+            image: quay.io/konflux-ci/release-service-utils@sha256:5546fa78d3c88d7b6a2e8cff8902f7757f00541d0bbaf113b9f293133894afa3
+            script: |
+              #!/usr/bin/env bash
+
+              # Ensure data directory exists
+              mkdir -p "$(params.dataDir)/$(context.pipelineRun.uid)"
+
+              # Create test data for successful validation (using actual FBC test images)
+              # Note: ocpVersion is now attached by filter-published-fbc-images task
+              cat > "$(params.dataDir)/$(params.snapshotPath)" << 'EOF'
+              {
+                "components": [
+                  {
+                    "name": "test-fbc-component-1",
+                    "containerImage": "quay.io/hacbs-release-tests/test-ocp-version/test-fbc-component@sha256:f6e744662e342c1321deddb92469b55197002717a15f8c0b1bb2d9440aac2297",
+                    "ocpVersion": "v4.14"
+                  },
+                  {
+                    "name": "test-fbc-component-2",
+                    "containerImage": "quay.io/hacbs-release-tests/test-ocp-version/test-fbc-component@sha256:42183bc9716448d0c25dbe35bb1c61661449270c89576bed8b9d46ec8df08a4a",
+                    "ocpVersion": "v4.14"

Relevance

⭐ Low

Line-length wrapping for long image: digests has been explicitly rejected before (PR 2197).

PR-#2197

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 5 requires YAML to stay within 120 characters per line. The new image: line and
containerImage: digest lines are long single-line values that exceed this limit.

AGENTS.md: YAML files must follow repository YAML style conventions
tasks/managed/prepare-fbc-parameters/tests/test-prepare-fbc-parameters-stage-pyxis.yaml[56-58]
tasks/managed/prepare-fbc-parameters/tests/test-prepare-fbc-parameters-stage-pyxis.yaml[70-77]

Agent prompt

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

## Issue description
New YAML lines exceed the 120 character limit (notably long image references and digests).

## Issue Context
Repo YAML style conventions require a maximum of 120 characters per line.

## Fix Focus Areas
- tasks/managed/prepare-fbc-parameters/tests/test-prepare-fbc-parameters-stage-pyxis.yaml[56-77]

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

---

Previous review results

Review updated until commit f15a166

Results up to commit 0efda72

🐞 Bugs (2) 📘 Rule violations (4) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Early return skips components 🐞 Bug ≡ Correctness

Description

verify_multi_component_release() returns from inside the per-component loop for non-optin
scenarios, so only the first component is validated when component_count>1. This can hide
failures/regressions in later components for multi-component releases.

Code

integration-tests/fbc-release/test.sh[R534-538]

+        # the upcoming checks are only required for multi optin components
+        # otherwise return earlier
+        if [  "$scenario" != "optin" ]; then
+            return $failures
+        fi

Relevance

⭐⭐⭐ High

Team has recently strengthened multi-component verification logic; skipping components undermines
intent (PR 2144).

PR-#2144

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The non-optin guard uses return $failures inside the per-component loop, which exits the entire
function during the first iteration and skips validation of any remaining components.

integration-tests/fbc-release/test.sh[455-538]

Agent prompt

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

## Issue description
`verify_multi_component_release()` returns from inside the `for((i=0; i<component_count; i++))` loop when the scenario is not `optin`, which prevents validating components at indexes > 0.

## Issue Context
The function is intended to validate every release artifact component; only the *extra* checks (skopeo/BUILD_VERSION validation) should be opt-in-only.

## Fix Focus Areas
- integration-tests/fbc-release/test.sh[455-569]

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

---

2. Tekton script missing set -eo pipefail 📘 Rule violation ☼ Reliability

Description

The new Tekton script: block in the test pipeline does not enable strict error handling, which can
allow failures to be silently ignored. This violates the repository shell strictness standard for
Tekton task scripts.

Code

tasks/managed/prepare-fbc-parameters/tests/test-prepare-fbc-parameters-stage-pyxis.yaml[R58-66]

+            script: |
+              #!/usr/bin/env bash
+
+              # Ensure data directory exists
+              mkdir -p "$(params.dataDir)/$(context.pipelineRun.uid)"
+
+              # Create test data for successful validation (using actual FBC test images)
+              # Note: ocpVersion is now attached by filter-published-fbc-images task
+              cat > "$(params.dataDir)/$(params.snapshotPath)" << 'EOF'

Relevance

⭐⭐ Medium

Strict shell-mode requirements are inconsistent; adding strict mode was rejected in one e2e script
(PR 2136).

PR-#2136

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 6 requires Tekton task scripts to include at least set -eo pipefail. The new
script begins with a bash shebang but does not set strict options before executing commands.

AGENTS.md: Shell scripts must set strict error-handling options appropriate to context
tasks/managed/prepare-fbc-parameters/tests/test-prepare-fbc-parameters-stage-pyxis.yaml[58-66]

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 `script:` block was added without strict bash error handling options.

## Issue Context
Per repo standards, Tekton task scripts must include at least `set -eo pipefail` to avoid silent failures.

## Fix Focus Areas
- tasks/managed/prepare-fbc-parameters/tests/test-prepare-fbc-parameters-stage-pyxis.yaml[58-66]

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

---

3. Unquoted $1 assignment 📘 Rule violation ≡ Correctness

Description

The new logic assigns effective_application_name=$1 without quotes or brace form, risking
word-splitting/globbing and violating the shell expansion standard. Similar unquoted expansions
appear in new calls using variables.

Code

integration-tests/fbc-release/test.sh[R159-163]

wait_for_multi_component_snapshot() {
+    # replace global with local values
+    local effective_application_name=${application_name}
+    [ -n "$1" ] && effective_application_name=$1
+

Relevance

⭐⭐ Medium

No clear history enforcing shell quoting; similar strict-mode/robustness nits often rejected (PR
2136).

PR-#2136

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 7 requires quoting and brace-form expansions. The updated function assigns from
$1 without quotes/braces, and later invokes registry_authenticate with an unquoted variable
argument.

AGENTS.md: Shell variable and command expansions must be quoted and use brace form
integration-tests/fbc-release/test.sh[159-163]
integration-tests/fbc-release/test.sh[546-549]

Agent prompt

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

## Issue description
New shell code uses unquoted/unbraced variable and positional-parameter expansions (e.g., `effective_application_name=$1`). This can break when values contain spaces or glob characters.

## Issue Context
Repo shell standards require brace form and quoting: `"${VAR}"` and `"${1}"`.

## Fix Focus Areas
- integration-tests/fbc-release/test.sh[159-163]
- integration-tests/fbc-release/test.sh[546-549]

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

---

View more (1)

4. jq index treated as string 🐞 Bug ≡ Correctness

Description

verify_multi_component_release() uses jq --arg i "$i" while indexing an array with
components[$i], passing the index as a string rather than a number. This is inconsistent with
other array indexing in the same script (--argjson i) and can cause null/empty reads from the
release JSON and incorrect verification results.

Code

integration-tests/fbc-release/test.sh[R461-466]

+        fbc_fragment=$(jq -r --arg i "$i" ".status.artifacts.components[$i].fbc_fragment // \"\"" <<< "${release_json}")
+        ocp_version=$(jq -r --arg i "$i" ".status.artifacts.components[$i].ocp_version // \"\"" <<< "${release_json}")
+        iib_log=$(jq -r --arg i "$i" ".status.artifacts.components[$i].iibLog // \"\"" <<< "${release_json}")
+        index_image=$(jq -r --arg i "$i" ".status.artifacts.components[$i].index_image // \"\"" <<< "${release_json}")
+        index_image_resolved=$(jq -r --arg i "$i" ".status.artifacts.components[$i].index_image_resolved // \"\"" <<< "${release_json}")
+        target_index=$(jq -r --arg i "$i" ".status.artifacts.components[$i].target_index // \"\"" <<< "${release_json}")

Relevance

⭐⭐ Medium

No direct historical decisions on --arg vs --argjson indexing; could be treated as correctness but
evidence lacking.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

verify_multi_component_release() indexes .status.artifacts.components[$i] using --arg i
(string). Elsewhere in the same file, array indexing is done with --argjson i for numeric indices,
showing the expected safe pattern.

integration-tests/fbc-release/test.sh[461-466]
integration-tests/fbc-release/test.sh[610-613]
integration-tests/fbc-release/test.sh[682-685]

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 loop index is passed to jq via `--arg i` (string), but used as an array index (`components[$i]`). This can yield incorrect/null lookups.

## Issue Context
The same script already uses `--argjson i` for array indexing in other functions, which indicates the intended pattern.

## Fix Focus Areas
- integration-tests/fbc-release/test.sh[461-466]

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

---

5. DOCKER_CONFIG temp dir uncleaned 📘 Rule violation ☼ Reliability

Description

registry_authenticate() creates a temporary directory with mktemp -d but never cleans it up,
which can leak temporary artifacts across runs. This violates the temp-file lifecycle management
requirement.

Code

integration-tests/fbc-release/test.sh[R409-425]

+registry_authenticate() {
+    # k8s secret containing the dockerconfigjson
+    SECRET_YAML_PATH="$1"
+    SECRET_NAME="$2"
+
+    # skopeo, podman and docker auth
+    DOCKER_CONFIG="$(mktemp -d)"
+    REGISTRY_AUTH_FILE="${DOCKER_CONFIG}/auth.json"
+
+    export DOCKER_CONFIG SECRET_NAME REGISTRY_AUTH_FILE
+
+    yq '. | select(.metadata.name | contains(env(SECRET_NAME))) | .data.".dockerconfigjson"' \
+    "${SECRET_YAML_PATH}" | base64 -d > "${DOCKER_CONFIG}/config.json"
+
+    # creates auth.json for podman
+    ln -s ${DOCKER_CONFIG}/config.json ${DOCKER_CONFIG}/auth.json
+}

Relevance

⭐ Low

Team has rejected cleanup/trap hardening for mktemp/temp artifacts in past reviews (PR 2236).

PR-#2236

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 14 requires temporary directories/files to be created with mktemp and cleaned up
via an EXIT trap. The function creates DOCKER_CONFIG with mktemp -d and does not remove it or
trap cleanup.

AGENTS.md: Temporary files and directory changes must be safely managed (cleanup traps and pushd/popd)
integration-tests/fbc-release/test.sh[409-425]

Agent prompt

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

## Issue description
`registry_authenticate()` creates a temp directory via `mktemp -d` but does not register an EXIT trap to remove it.

## Issue Context
Repo scripting standards require temporary files/directories created with `mktemp` to be cleaned up via an EXIT trap to prevent leaks.

## Fix Focus Areas
- integration-tests/fbc-release/test.sh[409-425]

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

---

6. YAML line exceeds 120 chars 📘 Rule violation ⚙ Maintainability

Description

The new Tekton test pipeline YAML includes very long single-line scalar values (e.g., image: and
containerImage: digests) that exceed the repository’s 120-character limit. This violates the YAML
style conventions required by the repo.

Code

tasks/managed/prepare-fbc-parameters/tests/test-prepare-fbc-parameters-stage-pyxis.yaml[R56-77]

+          - name: setup-values
+            image: quay.io/konflux-ci/release-service-utils@sha256:5546fa78d3c88d7b6a2e8cff8902f7757f00541d0bbaf113b9f293133894afa3
+            script: |
+              #!/usr/bin/env bash
+
+              # Ensure data directory exists
+              mkdir -p "$(params.dataDir)/$(context.pipelineRun.uid)"
+
+              # Create test data for successful validation (using actual FBC test images)
+              # Note: ocpVersion is now attached by filter-published-fbc-images task
+              cat > "$(params.dataDir)/$(params.snapshotPath)" << 'EOF'
+              {
+                "components": [
+                  {
+                    "name": "test-fbc-component-1",
+                    "containerImage": "quay.io/hacbs-release-tests/test-ocp-version/test-fbc-component@sha256:f6e744662e342c1321deddb92469b55197002717a15f8c0b1bb2d9440aac2297",
+                    "ocpVersion": "v4.14"
+                  },
+                  {
+                    "name": "test-fbc-component-2",
+                    "containerImage": "quay.io/hacbs-release-tests/test-ocp-version/test-fbc-component@sha256:42183bc9716448d0c25dbe35bb1c61661449270c89576bed8b9d46ec8df08a4a",
+                    "ocpVersion": "v4.14"

Relevance

⭐ Low

Line-length wrapping for long image: digests has been explicitly rejected before (PR 2197).

PR-#2197

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 5 requires YAML to stay within 120 characters per line. The new image: line and
containerImage: digest lines are long single-line values that exceed this limit.

AGENTS.md: YAML files must follow repository YAML style conventions
tasks/managed/prepare-fbc-parameters/tests/test-prepare-fbc-parameters-stage-pyxis.yaml[56-58]
tasks/managed/prepare-fbc-parameters/tests/test-prepare-fbc-parameters-stage-pyxis.yaml[70-77]

Agent prompt

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

## Issue description
New YAML lines exceed the 120 character limit (notably long image references and digests).

## Issue Context
Repo YAML style conventions require a maximum of 120 characters per line.

## Fix Focus Areas
- tasks/managed/prepare-fbc-parameters/tests/test-prepare-fbc-parameters-stage-pyxis.yaml[56-77]

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

[theflockers]

/retest

[qodo-app-for-konflux-ci]

Code review by qodo was updated up to the latest commit 43e0fcf

[qodo-app-for-konflux-ci]

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

[davidmogar]

Production Approval Record

Field	Value
Action	APPROVED
Reviewer	@davidmogar
Timestamp	2026-06-23T13:16:11.913Z
Criteria Override	AI review findings not addressed