refactor(RELEASE-2471): convert check-data-keys to python by johnbieren · Pull Request #2308 · konflux-ci/release-service-catalog

johnbieren · 2026-06-17T18:29:16Z

This commit replaces the inline task script for the check-data-keys managed task with a standalone python script contained in the utils image. The tekton unit tests are updated accordingly.

Assisted-By: Cursor

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

johnbieren · 2026-06-17T18:52:52Z

/retest

johnbieren · 2026-06-17T19:43:13Z

/retest

This commit replaces the inline task script for the check-data-keys managed task with a standalone python script contained in the utils image. The tekton unit tests are updated accordingly. Assisted-By: Cursor Signed-off-by: Johnny Bieren <jbieren@redhat.com>

johnbieren · 2026-06-24T17:26:46Z

/retest

qodo-app-for-konflux-ci · 2026-06-24T18:39:32Z

PR Reviewer Guide 🔍

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 Behavior Change The task no longer accepts a `schema` param and instead relies on a schema bundled inside the `release-service-utils` image. Validate that the referenced image digest actually contains `/home/schemas/dataKeys.json` and that the new Python entrypoint reads the intended schema (and fails loudly if it’s missing) to avoid silently skipping validation or validating against an unexpected schema version. The validation schema is bundled in the release-service-utils image at `/home/schemas/dataKeys.json`. params: Compatibility The step switched from an inline bash script (which handled CA bundle, file existence checks, and mutating `.systems` into the JSON) to a Python script invoked via `command` with parameters passed as env vars. Confirm parity with the prior logic: CA trust behavior, correct resolution of `dataDir` + `dataPath`, and that `.systems` is merged exactly as before (especially preserving JSON types and not double-encoding). - name: PARAM_DATA_DIR value: $(params.dataDir) - name: PARAM_DATA_PATH value: $(params.dataPath) - name: PARAM_SYSTEMS value: $(params.systems) command: ["/home/scripts/python/tasks/managed/check_data_keys.py"] - name: create-trusted-artifact
📄 References konflux-ci/release-service-catalog/tasks/managed/check-data-keys/tests/test-check-data-keys.yaml [1-28]

qodo-app-for-konflux-ci · 2026-06-24T18:52:57Z

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (3) 📜 Skill insights (0)

Context used

✅ Compliance rules (platform): 25 rules

1. README.md edited under tasks/ 📘 Rule violation ⚙ Maintainability

Description

tasks/managed/check-data-keys/README.md was modified in this PR, which is disallowed for
auto-generated README files under tasks/ and pipelines/. Manual or direct edits can desync
generated documentation from its intended source of truth.

Code

tasks/managed/check-data-keys/README.md[R11-30]

+The validation schema is bundled in the release-service-utils image at
+`/home/schemas/dataKeys.json`.

## Parameters

-| Name                    | Description                                                                                                                | Optional | Default value                                                                                                    |
-|-------------------------|----------------------------------------------------------------------------------------------------------------------------|----------|------------------------------------------------------------------------------------------------------------------|
-| dataPath                | Path to the JSON string of the merged data to use                                                                          | No       | -                                                                                                                |
-| schema                  | URL to the JSON schema file to validate the data against                                                                   | Yes      | https://raw.githubusercontent.com/konflux-ci/release-service-catalog/refs/heads/development/schema/dataKeys.json |
-| systems                 | The systems to check that all data keys are present for                                                                    | Yes      | ""                                                                                                               |
-| 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       | -                                                                                                                |
-| 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                                                                                                    |
-| caCertPath              | Path to CA certificate bundle for TLS verification with self-signed certificates                                           | Yes      | /mnt/trusted-ca/ca-bundle.crt                                                                                    |
+| Name                    | Description                                                                                                                | Optional | Default value                 |
+|-------------------------|----------------------------------------------------------------------------------------------------------------------------|----------|-------------------------------|
+| dataPath                | Path to the JSON string of the merged data to use                                                                          | No       | -                             |
+| systems                 | The systems to check that all data keys are present for                                                                    | Yes      | ""                            |
+| 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       | -                             |
+| 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                 |
+| caCertPath              | Path to CA certificate bundle for TLS verification with self-signed certificates                                           | Yes      | /mnt/trusted-ca/ca-bundle.crt |

Relevance

⭐⭐⭐ High
Repo added README generator + check to prevent drift; manual README edits under tasks discouraged
(PR #1157).
PR-#1157

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 1260 forbids any content changes to README.md files under tasks/ and
pipelines/. The diff shows edits to tasks/managed/check-data-keys/README.md (schema location
text and parameter table).
Rule 1260: Do not manually edit auto-generated README files under tasks/ and pipelines/
tasks/managed/check-data-keys/README.md[11-30]

Agent prompt

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

## Issue description
Auto-generated README files under `tasks/` and `pipelines/` must not be edited directly, but this PR modifies `tasks/managed/check-data-keys/README.md`.

## Issue Context
Compliance requires that README.md files under these directories are not changed via PR content edits.

## Fix Focus Areas
- tasks/managed/check-data-keys/README.md[11-30]

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

2. Schema source drift 🐞 Bug ☼ Reliability

Description

check-data-keys now validates using the schema embedded in the release-service-utils image
(/home/schemas/dataKeys.json) instead of the repo’s schema/dataKeys.json. Future updates to
schema/dataKeys.json can pass CI linting but not affect runtime validation unless the utils image
digest is rebuilt/updated in lockstep.

Code

tasks/managed/check-data-keys/check-data-keys.yaml[R127-141]

+      image: quay.io/konflux-ci/release-service-utils@sha256:282c23415a9995ab3fd6eb79dd314e84aed9b51e96ac3caaa34562c08eb7cc51
      computeResources:
        limits:
          memory: 64Mi
        requests:
          memory: 64Mi  # was exiting with code 137 when set to 32Mi
          cpu: 10m
      env:
-        - name: "SCHEMA_FILE"
-          value: "$(params.schema)"
-      script: |
-        #!/usr/bin/env bash
-        set -ex
-
-        if [ -f "/mnt/trusted-ca/ca-bundle.crt" ]; then
-            export SSL_CERT_FILE="/mnt/trusted-ca/ca-bundle.crt"
-        fi
-
-        if [ ! -f "$(params.dataDir)/$(params.dataPath)" ] ; then
-            echo "No data JSON was provided."
-            exit 1
-        fi
-
-        schema="${SCHEMA_FILE/\.git\///}"
-        if ! curl -sL --fail-with-body --retry 3 --retry-delay 5 --retry-all-errors "$schema" -o /tmp/schema ; then
-            echo "Failed to download schema file: $schema"
-            exit 1
-        fi
-
-        # We want this to output the json without expansion
-        # shellcheck disable=SC2016
-        jq --argjson systems '$(params.systems)' '.systems += $systems' \
-            "$(params.dataDir)/$(params.dataPath)" > "/tmp/systems"
-        mv "/tmp/systems" "$(params.dataDir)/$(params.dataPath)"
-
-        check-jsonschema --output-format=text --schemafile "/tmp/schema"  "$(params.dataDir)/$(params.dataPath)"
+        - name: PARAM_DATA_DIR
+          value: $(params.dataDir)
+        - name: PARAM_DATA_PATH
+          value: $(params.dataPath)
+        - name: PARAM_SYSTEMS
+          value: $(params.systems)
+      command: ["/home/scripts/python/tasks/managed/check_data_keys.py"]

Relevance

⭐⭐ Medium
Past work emphasized canonical repo schema URL/param wiring (PR #1230), but no precedent on
image-bundled schema.
PR-#1230

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The task documentation and implementation explicitly switch schema consumption to an image-bundled
file, while CI still validates the repo schema file, meaning schema changes in git can diverge from
what the task enforces at runtime.
tasks/managed/check-data-keys/check-data-keys.yaml[10-21]
tasks/managed/check-data-keys/check-data-keys.yaml[126-142]
.github/workflows/lint.yaml[73-102]

Agent prompt

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

### Issue description
`check-data-keys` previously consumed the schema from the catalog repo revision via a URL param, ensuring the schema validated at runtime always matched the repo version.

After this change, the task validates against `/home/schemas/dataKeys.json` bundled in the `release-service-utils` image, while the repository still treats `schema/dataKeys.json` as the canonical schema (linted in CI). This creates a drift risk: repo schema changes won’t affect runtime validation unless the image is rebuilt and the task’s pinned digest is updated.

### Issue Context
The PR intentionally removes the `schema` param and switches execution to a python script inside the utils image. The remaining risk is keeping schema evolution synchronized.

### Fix Focus Areas
- tasks/managed/check-data-keys/check-data-keys.yaml[19-21]
- tasks/managed/check-data-keys/check-data-keys.yaml[126-142]
- tasks/managed/check-data-keys/README.md[11-12]

### Suggested implementation directions
Choose one:
1) **Reintroduce a schema input (URL/path)**: add back a `schema` param (or `schemaPath`) whose default points to the repo raw URL for the same `taskGitUrl/taskGitRevision`, and pass it to the python script via env.
2) **Fetch schema from taskGitUrl/taskGitRevision inside the task**: keep param-less interface, but have the task/script pull the schema from the repo revision at runtime (or include it as a stepaction resource).
3) **Add a guardrail**: add CI/automation to ensure the utils image’s embedded `/home/schemas/dataKeys.json` matches `schema/dataKeys.json` and bump the pinned digest automatically whenever the schema changes.

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

3. Long YAML image line 📘 Rule violation ⚙ Maintainability

Description

A modified YAML line exceeds the 120 character limit due to an unwrapped image: reference with a
full SHA digest. Overlong YAML lines reduce readability and violate the repository's formatting
requirement.

Code

tasks/managed/check-data-keys/check-data-keys.yaml[127]

+      image: quay.io/konflux-ci/release-service-utils@sha256:282c23415a9995ab3fd6eb79dd314e84aed9b51e96ac3caaa34562c08eb7cc51

Relevance

⭐ Low
Similar “wrap long image line” suggestion was rejected by reviewers in PR #2197.
PR-#2197

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 1252 requires all changed YAML lines to be <= 120 characters. The image: lines in
the task and its test pipeline include a full SHA digest on a single line, making them exceed the
limit.
Rule 1252: Limit YAML line length to 120 characters
tasks/managed/check-data-keys/check-data-keys.yaml[127-127]
tasks/managed/check-data-keys/tests/test-check-data-keys.yaml[51-51]

Agent prompt

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

## Issue description
Changed YAML files must not contain any line longer than 120 characters, but the updated `image:` line exceeds this limit.

## Issue Context
The `image:` value includes a full `@sha256:` digest which pushes the line length beyond 120 characters.

## Fix Focus Areas
- tasks/managed/check-data-keys/check-data-keys.yaml[127-127]
- tasks/managed/check-data-keys/tests/test-check-data-keys.yaml[51-51]

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

4. Missing CPU limit in step 📘 Rule violation ☼ Reliability

Description

The modified Tekton task step check-data-keys specifies memory limits/requests and a CPU request
but does not set a CPU limit. This violates the requirement that each Tekton step define both CPU
and memory for requests and limits.

Code

tasks/managed/check-data-keys/check-data-keys.yaml[R126-133]

    - name: check-data-keys
-      image: quay.io/konflux-ci/release-service-utils@sha256:5546fa78d3c88d7b6a2e8cff8902f7757f00541d0bbaf113b9f293133894afa3
+      image: quay.io/konflux-ci/release-service-utils@sha256:282c23415a9995ab3fd6eb79dd314e84aed9b51e96ac3caaa34562c08eb7cc51
      computeResources:
        limits:
          memory: 64Mi
        requests:
          memory: 64Mi  # was exiting with code 137 when set to 32Mi
          cpu: 10m

Relevance

⭐ Low
CPU limits requirement was reverted; computeResources check no longer enforces limits.cpu (PR
#2088).
PR-#2088

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 1265 requires every Tekton task step to include both CPU and memory in requests
and limits. In the check-data-keys step, limits includes only memory while cpu is present
only under requests.
Rule 1265: Specify compute resources in all Tekton tasks
tasks/managed/check-data-keys/check-data-keys.yaml[126-133]

Agent prompt

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

## Issue description
Each Tekton task step must define `requests` and `limits` for both `cpu` and `memory`. The `check-data-keys` step lacks a CPU limit.

## Issue Context
The `check-data-keys` step was modified in this PR (image/env/command), so its resource specification must meet the compliance requirement.

## Fix Focus Areas
- tasks/managed/check-data-keys/check-data-keys.yaml[126-133]

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

5. Misleading test hook comment 🐞 Bug ⚙ Maintainability

Description

pre-apply-task-hook.sh still claims RBAC is needed to retrieve a ConfigMap, but the hook no longer
creates/uses the schema ConfigMap after the refactor. This stale comment obscures why RBAC is
applied and can lead to incorrect future edits.

Code

tasks/managed/check-data-keys/tests/pre-apply-task-hook.sh[R5-6]

# Add RBAC so that the SA executing the tests can retrieve configMap
kubectl apply -f .github/resources/crd_rbac.yaml
-
-# Create a configMap with the schema to be used by the task
-kubectl delete configmap check-data-keys-schema --ignore-not-found
-kubectl create configmap check-data-keys-schema --from-file=dataKeys="$SCRIPT_DIR/../../../../schema/dataKeys.json"

Relevance

⭐ Low
Team previously rejected comment rewording/staleness cleanup in tasks (PR #1463).
PR-#1463

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The only remaining logic is applying RBAC, but the comment still references ConfigMap retrieval
despite the ConfigMap creation logic being removed.
tasks/managed/check-data-keys/tests/pre-apply-task-hook.sh[1-6]

Agent prompt

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

### Issue description
The test pre-apply hook comment mentions ConfigMap retrieval, but the schema ConfigMap setup was removed. This comment is now misleading.

### Issue Context
The schema is now bundled in the utils image, so the tests no longer create/read a schema ConfigMap.

### Fix Focus Areas
- tasks/managed/check-data-keys/tests/pre-apply-task-hook.sh[1-6]

### Suggested fix
Update the comment to reflect the actual reason RBAC is applied (or remove the comment / RBAC apply if truly no longer needed for these tests).

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

johnbieren · 2026-06-29T12:19:32Z

/retest

johnbieren · 2026-06-30T12:11:51Z

/retest

github-actions Bot added the ai-assisted label Jun 17, 2026

johnbieren force-pushed the release2471 branch 2 times, most recently from ee80cc7 to 45c9d33 Compare June 17, 2026 18:45

johnbieren force-pushed the release2471 branch from 45c9d33 to 0b3c112 Compare June 17, 2026 19:43

johnbieren force-pushed the release2471 branch from 0b3c112 to 59f14aa Compare June 24, 2026 17:26

johnbieren marked this pull request as ready for review June 24, 2026 18:38

johnbieren requested review from arewm, gbenhaim, ralphbean and twaugh as code owners June 24, 2026 18:38

release-service-bot assigned happybhati and midnightercz Jun 24, 2026

qodo-app-for-konflux-ci Bot added the Review effort 3/5 label Jun 24, 2026

qodo-app-for-konflux-ci Bot reviewed Jun 24, 2026

View reviewed changes

Comment thread tasks/managed/check-data-keys/README.md

midnightercz approved these changes Jun 29, 2026

View reviewed changes

johnbieren added the prod/needs-approval label Jun 29, 2026

happybhati approved these changes Jun 29, 2026

View reviewed changes

davidmogar approved these changes Jun 30, 2026

View reviewed changes

ach912 approved these changes Jul 1, 2026

View reviewed changes

twaugh approved these changes Jul 1, 2026

View reviewed changes

ach912 added prod/approved and removed prod/needs-approval labels Jul 1, 2026