[DNM] Add dummy task, pipeline, and ITS for EC-2011 POC - #3476
[DNM] Add dummy task, pipeline, and ITS for EC-2011 POC#3476simonbaird wants to merge 6 commits into
Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueNote Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a configurable scenario creation script, a snapshot-processing Tekton Pipeline, and a ChangesDummy integration test flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant create_dummy_its.sh
participant IntegrationTestScenario
participant reqd_task_poc_ec2011
participant dummy_check
participant AttestationAction
create_dummy_its.sh->>IntegrationTestScenario: Apply scenario with Git resolver parameters
IntegrationTestScenario->>reqd_task_poc_ec2011: Start with SNAPSHOT and RESULT
reqd_task_poc_ec2011->>reqd_task_poc_ec2011: Extract containerImage URL and digest
reqd_task_poc_ec2011->>dummy_check: Pass RESULT and image metadata
dummy_check->>AttestationAction: Pass image metadata and TEST_OUTPUT
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
PR Summary by QodoAdd dummy integration test pipeline/task and ITS bootstrap script (EC-2011 POC)
AI Description
Diagram
High-Level Assessment
Files changed (3)
|
|
🤖 Review · Commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@hack/create-dummy-its.sh`:
- Around line 25-26: In the reqd-task-poc-ec2011 required-task scenario, remove
the test.appstudio.openshift.io/optional label so the scenario is treated as
mandatory. Leave the remaining scenario configuration unchanged.
In `@pipelines/dummy-integration-test/0.1/dummy-integration-test.yaml`:
- Around line 51-64: Update the SNAPSHOT handling around the IMAGE extraction to
parse it as JSON, validate every component’s containerImage and required digest
separator, and reject malformed or missing values. Preserve the
image-url/image-digest contract only if SNAPSHOT is explicitly constrained to
one component; otherwise process and emit all validated components rather than
silently selecting the first one.
In `@tasks/dummy-check/0.1/dummy-check.yaml`:
- Line 66: Replace the mutable ubi-minimal:latest image reference in
tasks/dummy-check/0.1/dummy-check.yaml at lines 66-66 with an approved immutable
digest, and make the same replacement in
pipelines/dummy-integration-test/0.1/dummy-integration-test.yaml at lines 43-43.
Use the identical digest in both files.
- Around line 141-144: Pin every Git resolver revision to an approved immutable
full commit SHA: update the revision default in
tasks/dummy-check/0.1/dummy-check.yaml:141-144,
pipelines/dummy-integration-test/0.1/dummy-integration-test.yaml:81-86, and
hack/create-dummy-its.sh:8-10, replacing branch references such as main and
reqd-task-its-poc. If hack/create-dummy-its.sh enforces GIT_REVISION, validate
that the supplied value is a full commit SHA.
- Around line 122-130: Update the TEST_OUTPUT construction to use a JSON encoder
for RESULT, TIMESTAMP, NOTE, and other string fields so quotes, backslashes, and
newlines are escaped correctly. Validate SUCCESSES, FAILURES, and WARNINGS as
integers before applying %d, terminating the step for invalid values while still
accepting negative integers.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: 2155f7f0-eebb-4e9f-8e6d-45839f9106bf
📒 Files selected for processing (3)
hack/create-dummy-its.shpipelines/dummy-integration-test/0.1/dummy-integration-test.yamltasks/dummy-check/0.1/dummy-check.yaml
| labels: | ||
| test.appstudio.openshift.io/optional: "true" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- script ---'
cat -n hack/create-dummy-its.sh
printf '%s\n' '--- related references ---'
rg -n -i --glob '!vendor/**' --glob '!node_modules/**' \
'reqd-task-poc-ec2011|test\.appstudio\.openshift\.io/optional|optional.*IntegrationTestScenario|IntegrationTestScenario|required task|required-task' .Repository: conforma/cli
Length of output: 2721
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- dummy integration-test pipeline ---'
cat -n pipelines/dummy-integration-test/0.1/dummy-integration-test.yaml
printf '%s\n' '--- enterprise-contract guidance ---'
sed -n '70,110p' pipelines/enterprise-contract/0.1/enterprise-contract.yaml
printf '%s\n' '--- all optional-label and informative references ---'
rg -n -i 'test\.appstudio\.openshift\.io/optional|informative|required.*task|task.*required|IntegrationTestScenario' \
README.md docs hack pipelines internal config .github 2>/dev/null || trueRepository: conforma/cli
Length of output: 7294
🌐 Web query:
Konflux IntegrationTestScenario test.appstudio.openshift.io/optional label required task semantics
💡 Result:
The label test.appstudio.openshift.io/optional in Konflux is used to control whether an IntegrationTestScenario is mandatory for the release process [1][2]. Its semantics are as follows: - Purpose: It specifies whether the result of a PipelineRun created by the IntegrationTestScenario must be successful for an application snapshot to be considered valid [1][3]. - Values: - "true": The integration test is marked as optional. If the test fails, it does not block the release or promotion of the application snapshot [1][4][3]. - "false": The integration test is mandatory (required). If the test fails, the application snapshot is marked as failed, which prevents its release [1][4][3]. - Default Behavior: If the label is not explicitly defined in an IntegrationTestScenario custom resource, the Konflux Integration Service defaults its value to "false", meaning all integration tests are mandatory by default [1][5][3]. - Propagation: When a test runs, this label is copied from the IntegrationTestScenario resource to the resulting PipelineRun [1][2]. By marking a test as optional, users can allow specific, non-critical tests to fail without hindering the overall automated release process [4][3].
Citations:
- 1: https://konflux-ci.dev/architecture/core/integration-service/
- 2: https://konflux-ci.dev/docs/testing/integration/creating/
- 3: https://konflux-ci.dev/docs/testing/integration/adding/
- 4: https://konflux-ci.dev/docs/testing/integration/editing/
- 5: https://pkg.go.dev/github.com/konflux-ci/integration-service/e2e-tests/pkg/constants
Remove the optional label from the required-task scenario.
The reqd-task-poc-ec2011 scenario must be mandatory to enforce required-task behavior. Remove test.appstudio.openshift.io/optional: "true".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@hack/create-dummy-its.sh` around lines 25 - 26, In the reqd-task-poc-ec2011
required-task scenario, remove the test.appstudio.openshift.io/optional label so
the scenario is treated as mandatory. Leave the remaining scenario configuration
unchanged.
| IMAGE=$(echo "${SNAPSHOT}" | grep -oP '"containerImage"\s*:\s*"\K[^"]+' | head -1) | ||
| if [[ -z "${IMAGE}" ]]; then | ||
| echo "ERROR: No containerImage found in SNAPSHOT" >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| IMAGE_URL="${IMAGE%%@*}" | ||
| IMAGE_DIGEST="${IMAGE##*@}" | ||
|
|
||
| echo "Parsed image-url: ${IMAGE_URL}" | ||
| echo "Parsed image-digest: ${IMAGE_DIGEST}" | ||
|
|
||
| echo -n "${IMAGE_URL}" > "$(results.image-url.path)" | ||
| echo -n "${IMAGE_DIGEST}" > "$(results.image-digest.path)" |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- candidate files ---'
git ls-files | grep -E '(^|/)(pipelines/dummy-integration-test/0\.1/dummy-integration-test\.yaml|hack/create-dummy-its\.sh)$' || true
printf '%s\n' '--- YAML structure ---'
ast-grep outline pipelines/dummy-integration-test/0.1/dummy-integration-test.yaml --lang yaml || true
printf '%s\n' '--- YAML lines 1-90 ---'
cat -n pipelines/dummy-integration-test/0.1/dummy-integration-test.yaml | sed -n '1,90p'
printf '%s\n' '--- generator script ---'
if test -f hack/create-dummy-its.sh; then cat -n hack/create-dummy-its.sh | sed -n '1,180p'; fi
printf '%s\n' '--- related references ---'
rg -n -C 3 'containerImage|image-url|image-digest|dummy-integration-test|create-dummy-its|SNAPSHOT' pipelines hack README.md 2>/dev/null | sed -n '1,260p'Repository: conforma/cli
Length of output: 24835
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- dummy-check task ---'
fd -i 'dummy-check' . --type f | sort
for f in $(fd -i 'dummy-check' . --type f | sort); do
echo "--- $f ---"
wc -l "$f"
cat -n "$f" | sed -n '1,180p'
done
printf '%s\n' '--- pipeline and task result consumers ---'
rg -n -C 5 'image-digest|image-url|dummy-check|components.*containerImage|containerImage' pipelines tasks .github 2>/dev/null | sed -n '1,320p'
printf '%s\n' '--- deterministic parser probe ---'
python3 - <<'PY'
import json, re, subprocess
script = r'''set -euo pipefail
SNAPSHOT="$1"
IMAGE=$(echo "${SNAPSHOT}" | grep -oP '"containerImage"\s*:\s*"\K[^"]+' | head -1)
if [[ -z "${IMAGE}" ]]; then exit 1; fi
IMAGE_URL="${IMAGE%%@*}"
IMAGE_DIGEST="${IMAGE##*@}"
printf 'url=%s\ndigest=%s\n' "$IMAGE_URL" "$IMAGE_DIGEST"
'''
cases = {
"two components": {"components": [
{"containerImage": "quay.io/a@sha256:aaa"},
{"containerImage": "quay.io/b@sha256:bbb"},
]},
"tag only": {"components": [{"containerImage": "quay.io/a:latest"}]},
"invalid JSON with matching text": 'prefix "containerImage": "quay.io/a@sha256:aaa" suffix',
"escaped quote value": {"components": [{"containerImage": "quay.io/a@sha256:aa\\\"bb"}]},
"missing component image": {"components": [{"name": "a"}]},
}
for name, value in cases.items():
text = value if isinstance(value, str) else json.dumps(value)
p = subprocess.run(["bash", "-c", script, "parser", text], text=True,
capture_output=True)
print(f"[{name}] exit={p.returncode}")
print((p.stdout + p.stderr).strip() or "<no output>")
PYRepository: conforma/cli
Length of output: 24091
Parse and validate every Snapshot component.
Line 51 accepts non-JSON text and processes only the first containerImage. The task forwards one scalar image-url and image-digest pair, despite the contract requiring validation of every component.
If an image has no @, ${IMAGE##*@} writes the complete image reference as image-digest. Parse SNAPSHOT as JSON, validate every component, and either process all components or change the contract to require one component.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pipelines/dummy-integration-test/0.1/dummy-integration-test.yaml` around
lines 51 - 64, Update the SNAPSHOT handling around the IMAGE extraction to parse
it as JSON, validate every component’s containerImage and required digest
separator, and reject malformed or missing values. Preserve the
image-url/image-digest contract only if SNAPSHOT is explicitly constrained to
one component; otherwise process and emit all validated components rather than
silently selecting the first one.
| results: | ||
| - name: TEST_OUTPUT | ||
| description: JSON test results for consumption by subsequent steps. | ||
| image: registry.access.redhat.com/ubi9/ubi-minimal:latest |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Pin executable container images by digest.
Both steps use the mutable latest tag. A later image update can change task behavior without a repository change.
tasks/dummy-check/0.1/dummy-check.yaml#L66-L66: replaceubi-minimal:latestwith an approved immutable digest.pipelines/dummy-integration-test/0.1/dummy-integration-test.yaml#L43-L43: replaceubi-minimal:latestwith the same approved immutable digest.
📍 Affects 2 files
tasks/dummy-check/0.1/dummy-check.yaml#L66-L66(this comment)pipelines/dummy-integration-test/0.1/dummy-integration-test.yaml#L43-L43
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tasks/dummy-check/0.1/dummy-check.yaml` at line 66, Replace the mutable
ubi-minimal:latest image reference in tasks/dummy-check/0.1/dummy-check.yaml at
lines 66-66 with an approved immutable digest, and make the same replacement in
pipelines/dummy-integration-test/0.1/dummy-integration-test.yaml at lines 43-43.
Use the identical digest in both files.
| TEST_OUTPUT=$(printf '{ | ||
| "result": "%s", | ||
| "timestamp": "%s", | ||
| "note": "%s", | ||
| "namespace": "default", | ||
| "successes": %d, | ||
| "failures": %d, | ||
| "warnings": %d | ||
| }' "${RESULT}" "${TIMESTAMP}" "${NOTE}" "${SUCCESSES}" "${FAILURES}" "${WARNINGS}") |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
file="tasks/dummy-check/0.1/dummy-check.yaml"
printf '%s\n' '--- file outline ---'
if command -v ast-grep >/dev/null 2>&1; then
ast-grep outline "$file" || true
fi
printf '%s\n' '--- relevant task sections ---'
sed -n '1,170p' "$file"
printf '%s\n' '--- TEST_OUTPUT and attestation references ---'
rg -n -C 5 'TEST_OUTPUT|attest|NOTE|SUCCESSES|FAILURES|WARNINGS' tasks/dummy-check/0.1 "$file" tasks 2>/dev/null | head -n 300Repository: conforma/cli
Length of output: 29153
🏁 Script executed:
#!/bin/bash
set -eu
run_case() {
local label="$1"
local note="$2"
local successes="$3"
local failures="$4"
local warnings="$5"
output=$(
printf '{
"result": "%s",
"timestamp": "%s",
"note": "%s",
"namespace": "default",
"successes": %d,
"failures": %d,
"warnings": %d
}' "SUCCESS" "2026-08-01T00:00:00" "$note" "$successes" "$failures" "$warnings" 2>/tmp/printf-error
) || true
printf '%s\t%s\n' "$label" "$output"
if python3 -c 'import json, sys; json.load(sys.stdin)' <<<"$output" 2>/dev/null; then
printf '%s\n' 'JSON_PARSE=valid'
else
printf '%s\n' 'JSON_PARSE=invalid'
fi
if [ -s /tmp/printf-error ]; then
printf 'PRINTF_ERROR=%s\n' "$(cat /tmp/printf-error)"
fi
}
run_case 'plain-note' 'all good' '1' '0' '0'
run_case 'quote-note' 'contains "quote"' '1' '0' '0'
run_case 'newline-note' $'contains\nnewline' '1' '0' '0'
run_case 'backslash-note' 'contains \ backslash' '1' '0' '0'
run_case 'invalid-count' 'all good' 'not-a-number' '0' '0'
run_case 'negative-count' 'all good' '-1' '0' '0'Repository: conforma/cli
Length of output: 1415
🏁 Script executed:
#!/bin/bash
set -u
set +e
bash -c '
set -euo pipefail
TEST_OUTPUT=$(printf "{\n \"successes\": %d\n}\n" "not-a-number")
printf "WRITE_REACHED=%s\n" "$TEST_OUTPUT"
'
status=$?
printf 'COMMAND_STATUS=%d\n' "$status"
bash -c '
set -euo pipefail
TEST_OUTPUT=$(printf "{\n \"successes\": %d\n}\n" "-1")
printf "WRITE_REACHED=%s\n" "$TEST_OUTPUT"
'
status=$?
printf 'NEGATIVE_COMMAND_STATUS=%d\n' "$status"Repository: conforma/cli
Length of output: 280
Serialize TEST_OUTPUT with a JSON encoder. NOTE values containing ", \, or newlines produce invalid JSON for the attestation step. Encode all string fields before constructing the object. Validate SUCCESSES, FAILURES, and WARNINGS as non-negative integers before %d; invalid values terminate the step, while negative values are accepted.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tasks/dummy-check/0.1/dummy-check.yaml` around lines 122 - 130, Update the
TEST_OUTPUT construction to use a JSON encoder for RESULT, TIMESTAMP, NOTE, and
other string fields so quotes, backslashes, and newlines are escaped correctly.
Validate SUCCESSES, FAILURES, and WARNINGS as integers before applying %d,
terminating the step for invalid values while still accepting negative integers.
| - name: revision | ||
| value: main | ||
| - name: pathInRepo | ||
| value: stepactions/attest-test-result/0.1/attest-test-result.yaml |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
git ls-remote https://github.com/conforma/step-actions main
git ls-remote https://github.com/conforma/cli main
git ls-remote https://github.com/simonbaird/conforma-cli reqd-task-its-pocRepository: conforma/cli
Length of output: 334
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- tracked files ---'
git ls-files \
tasks/dummy-check/0.1/dummy-check.yaml \
pipelines/dummy-integration-test/0.1/dummy-integration-test.yaml \
hack/create-dummy-its.sh
printf '%s\n' '--- task definition ---'
sed -n '130,150p' tasks/dummy-check/0.1/dummy-check.yaml
printf '%s\n' '--- pipeline definition ---'
sed -n '70,95p' pipelines/dummy-integration-test/0.1/dummy-integration-test.yaml
printf '%s\n' '--- generator defaults ---'
sed -n '1,25p' hack/create-dummy-its.sh
printf '%s\n' '--- related resolver inputs ---'
rg -n -C 3 'pathInRepo|GIT_REVISION|reqd-task-its-poc|attest-test-result|dummy-integration-test' \
tasks pipelines hack .github 2>/dev/null || trueRepository: conforma/cli
Length of output: 6509
Pin every Git resolver revision to an immutable commit.
main and reqd-task-its-poc are branch references. Branch updates can change the fetched pipeline or task code without changing this scenario. Replace all three defaults with approved full commit SHAs. Validate GIT_REVISION if the script must enforce immutable revisions.
📍 Affects 3 files
tasks/dummy-check/0.1/dummy-check.yaml#L141-L144(this comment)pipelines/dummy-integration-test/0.1/dummy-integration-test.yaml#L81-L86hack/create-dummy-its.sh#L8-L10
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tasks/dummy-check/0.1/dummy-check.yaml` around lines 141 - 144, Pin every Git
resolver revision to an approved immutable full commit SHA: update the revision
default in tasks/dummy-check/0.1/dummy-check.yaml:141-144,
pipelines/dummy-integration-test/0.1/dummy-integration-test.yaml:81-86, and
hack/create-dummy-its.sh:8-10, replacing branch references such as main and
reqd-task-its-poc. If hack/create-dummy-its.sh enforces GIT_REVISION, validate
that the supplied value is a full commit SHA.
Code Review by Qodo
1.
|
![qodo-for-conforma[bot]](https://avatars.githubusercontent.com/in/3609154?s=60&v=4){kind=small}
| TEST_OUTPUT=$(printf '{ | ||
| "result": "%s", | ||
| "timestamp": "%s", | ||
| "note": "%s", |
There was a problem hiding this comment.
2. Unescaped note breaks json 🐞 Bug ≡ Correctness
dummy-check builds JSON using printf with the unescaped NOTE parameter, so a NOTE containing quotes/newlines/backslashes produces invalid JSON and can break the attest-test-result step and any downstream consumer expecting parseable JSON.
Agent Prompt
## Issue description
The task constructs JSON with `printf ... "note": "%s" ...` but does not JSON-escape `NOTE`. Any special characters in `NOTE` can invalidate the JSON output, which is then passed as `test-output` to the attestation step.
## Issue Context
The produced JSON is both:
- written to the task result `TEST_OUTPUT`
- passed into the `attest-test-result` step action
## Fix Focus Areas
- tasks/dummy-check/0.1/dummy-check.yaml[84-134]
- tasks/dummy-check/0.1/dummy-check.yaml[145-153]
## Recommended fix
Construct the JSON via a JSON-aware tool rather than `printf`, for example:
- Switch the step image to one that includes `jq` and do:
`TEST_OUTPUT=$(jq -n --arg result "$RESULT" --arg timestamp "$TIMESTAMP" --arg note "$NOTE" --arg ns "$NAMESPACE" --argjson successes "$SUCCESSES" --argjson failures "$FAILURES" --argjson warnings "$WARNINGS" '{result:$result,timestamp:$timestamp,note:$note,namespace:$ns,successes:$successes,failures:$failures,warnings:$warnings}')`
- Or, if you keep bash-only, implement proper JSON string escaping for `NOTE` before interpolation.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| IMAGE=$(echo "${SNAPSHOT}" | grep -oP '"containerImage"\s*:\s*"\K[^"]+' | head -1) | ||
| if [[ -z "${IMAGE}" ]]; then | ||
| echo "ERROR: No containerImage found in SNAPSHOT" >&2 | ||
| exit 1 |
There was a problem hiding this comment.
3. Only first component validated 🐞 Bug ≡ Correctness
The pipeline description claims each components[].containerImage is validated, but parse-snapshot selects only the first match (head -1) and also doesn’t validate that the image reference contains an @ digest, which can pass incorrect image-url/image-digest into the attestation step.
Agent Prompt
## Issue description
`parse-snapshot` extracts only the first `containerImage` occurrence and splits it with shell substring ops, but the pipeline’s own parameter description states that each image in the `components` array is validated.
## Issue Context
- Current implementation does: `... | head -1`, so additional components are ignored.
- If `containerImage` is a tag reference (no `@sha256:...`), `IMAGE_DIGEST` becomes the whole string and `image-url`/`image-digest` become inconsistent inputs to the attestation.
## Fix Focus Areas
- pipelines/dummy-integration-test/0.1/dummy-integration-test.yaml[12-23]
- pipelines/dummy-integration-test/0.1/dummy-integration-test.yaml[51-64]
## Recommended fix
- Parse the snapshot with a real JSON parser (e.g., `jq -r '.components[].containerImage'`).
- Either:
1) loop over all component images and run `dummy-check` once per image, OR
2) update the param description to explicitly say only the first component is used.
- Add an explicit check that `containerImage` contains `@` (and ideally `@sha256:`) before producing `image-url`/`image-digest`.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
12663f2 to
4830e00
Compare
|
🤖 Finished Review · ✅ Success · Started 10:50 PM UTC · Completed 11:08 PM UTC Commit: |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/checks-codecov.yaml:
- Line 37: Replace the constant if: false conditions for the Test, Acceptance,
and Upload jobs with the same non-constant repository or workflow variable gate
that evaluates false for the POC, ensuring actionlint accepts all three
conditions. Update .github/workflows/checks-codecov.yaml at lines 37-37,
100-100, and 169-169.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: 9bfdcb78-2264-4d55-bbda-e6753493c88b
📒 Files selected for processing (4)
.github/workflows/checks-codecov.yamlhack/create-dummy-its.shpipelines/dummy-integration-test/0.1/dummy-integration-test.yamltasks/dummy-check/0.1/dummy-check.yaml
🚧 Files skipped from review as they are similar to previous changes (3)
- tasks/dummy-check/0.1/dummy-check.yaml
- hack/create-dummy-its.sh
- pipelines/dummy-integration-test/0.1/dummy-integration-test.yaml
| jobs: | ||
|
|
||
| Test: | ||
| if: false # skipped for POC branch |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Replace the constant job conditions with one lint-valid POC gate.
actionlint rejects each constant if: false condition. Use the same non-constant repository or workflow variable gate for all three jobs. Keep the gate false for the POC.
.github/workflows/checks-codecov.yaml#L37-L37: update theTestjob condition..github/workflows/checks-codecov.yaml#L100-L100: update theAcceptancejob condition..github/workflows/checks-codecov.yaml#L169-L169: update theUploadjob condition.
🧰 Tools
🪛 actionlint (1.7.12)
[error] 37-37: constant expression "false" in condition. remove the if: section
(if-cond)
📍 Affects 1 file
.github/workflows/checks-codecov.yaml#L37-L37(this comment).github/workflows/checks-codecov.yaml#L100-L100.github/workflows/checks-codecov.yaml#L169-L169
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/checks-codecov.yaml at line 37, Replace the constant if:
false conditions for the Test, Acceptance, and Upload jobs with the same
non-constant repository or workflow variable gate that evaluates false for the
POC, ensuring actionlint accepts all three conditions. Update
.github/workflows/checks-codecov.yaml at lines 37-37, 100-100, and 169-169.
Source: Linters/SAST tools
ReviewFindingsHigh
Medium
Low
Next steps:
Previous runReviewFindingsCritical
High
Medium
Low
Next steps:
Previous run (2)ReviewFindingsHigh
Medium
Low
Next steps:
Previous run (3)ReviewFindingsMedium
Low
Next steps:
Previous run (4)ReviewFindingsHigh
Medium
Low
Next steps:
Previous run (5)ReviewFindingsHigh
Medium
Low
Next steps:
Previous run (6)ReviewFindingsHigh
Medium
Low
Next steps:
Previous run (7)ReviewFindingsHigh
Medium
Low
Next steps:
Previous run (8)ReviewFindingsHigh
Medium
Low
Next steps:
Previous run (9)ReviewFindingsHigh
Medium
Low
Next steps:
Previous run (10)ReviewFindingsHigh
Medium
Low
Next steps:
Previous run (11)ReviewFindingsHigh
Medium
Low
Next steps:
|
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
fullsend-ai-review
Bot
added
ci
github_actions
simonbaird
changed the title
|
🤖 Finished Review · ✅ Success · Started 2:13 AM UTC · Completed 2:31 AM UTC Commit: |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
st3penta
left a comment
st3penta
left a comment
There was a problem hiding this comment.
nice!
i won't approve since it's DNM, just to be safe, but looks good as a POC
|
🤖 Finished Review · ✅ Success · Started 3:32 PM UTC · Completed 3:51 PM UTC Commit: |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
hack/create-dummy-its.sh (1)
24-26: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftPin both Git resolver revisions to immutable commit SHAs.
GIT_REVISIONdefaults to mutablereqd-task-its-poc, and the pipeline task resolver uses the same mutable ref atpipelines/dummy-integration-test/0.1/dummy-integration-test.yaml:100. A moved ref can change the pipeline or task without changing the scenario. Use reviewed full commit SHAs for both revisions and validateGIT_REVISIONbefore applying.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hack/create-dummy-its.sh` around lines 24 - 26, Update the defaults in the dummy integration test setup to use reviewed, immutable full commit SHAs for both the top-level GIT_REVISION and the pipeline task resolver revision in dummy-integration-test.yaml. Add validation before applying GIT_REVISION so only a full commit SHA is accepted, while preserving the existing override behavior.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@hack/create-dummy-its.sh`:
- Around line 75-80: Update the ServiceAccount check and patch around
INTEGRATION_SA to query only .secrets[].name, avoiding matches from
imagePullSecrets or metadata. Make the JSON patch initialize /secrets as an
array when absent or null, and append the PUSH_SECRET only when the secrets
array already exists.
- Around line 62-71: Update the ServiceAccount setup in hack/create-dummy-its.sh
so PUSH_SECRET is not linked to the shared konflux-integration-runner used by
all IntegrationTestScenario pipelines. Run this ITS in an isolated namespace or
configure a dedicated per-ITS ServiceAccount when supported, ensuring untrusted
ITS pipelines cannot access the push credential.
---
Outside diff comments:
In `@hack/create-dummy-its.sh`:
- Around line 24-26: Update the defaults in the dummy integration test setup to
use reviewed, immutable full commit SHAs for both the top-level GIT_REVISION and
the pipeline task resolver revision in dummy-integration-test.yaml. Add
validation before applying GIT_REVISION so only a full commit SHA is accepted,
while preserving the existing override behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: 2d3208c7-bd68-442b-8547-a1be70f22d05
📒 Files selected for processing (1)
hack/create-dummy-its.sh
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
🤖 Finished Review · ✅ Success · Started 3:54 PM UTC · Completed 4:14 PM UTC Commit: |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@hack/create-dummy-its.sh`:
- Around line 88-100: Update the pull-secret presence check before the removal
block to require an exact “found” result rather than a substring match, so
“not_found” does not enter the patch path. Preserve the existing INDEX lookup
and oc patch behavior when the secret is actually present, and allow repeat runs
to complete when it is absent.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: 7f2019c3-4962-4024-9b66-5451c4125676
📒 Files selected for processing (1)
hack/create-dummy-its.sh
| print('found' if '${PULL_SECRET}' in secrets else 'not_found') | ||
| " | grep -q "found"; then | ||
| echo "Removing pull secret '${PULL_SECRET}' from SA '${INTEGRATION_SA}' to avoid credential conflict" | ||
| INDEX=$(echo "${SA_JSON}" | python3 -c " | ||
| import json, sys | ||
| sa = json.load(sys.stdin) | ||
| for i, s in enumerate(sa.get('secrets', [])): | ||
| if s['name'] == '${PULL_SECRET}': | ||
| print(i) | ||
| break | ||
| ") | ||
| oc patch sa "${INTEGRATION_SA}" -n "${NAMESPACE}" --type=json \ | ||
| -p="[{\"op\":\"remove\",\"path\":\"/secrets/${INDEX}\"}]" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Use an exact match for the pull-secret result.
Line 89 matches both found and not_found. If the pull secret is absent, INDEX is empty and the JSON Patch fails. The script cannot complete on a repeat run after the secret was removed.
Proposed fix
- " | grep -q "found"; then
+ " | grep -Fxq "found"; then📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| print('found' if '${PULL_SECRET}' in secrets else 'not_found') | |
| " | grep -q "found"; then | |
| echo "Removing pull secret '${PULL_SECRET}' from SA '${INTEGRATION_SA}' to avoid credential conflict" | |
| INDEX=$(echo "${SA_JSON}" | python3 -c " | |
| import json, sys | |
| sa = json.load(sys.stdin) | |
| for i, s in enumerate(sa.get('secrets', [])): | |
| if s['name'] == '${PULL_SECRET}': | |
| print(i) | |
| break | |
| ") | |
| oc patch sa "${INTEGRATION_SA}" -n "${NAMESPACE}" --type=json \ | |
| -p="[{\"op\":\"remove\",\"path\":\"/secrets/${INDEX}\"}]" | |
| print('found' if '${PULL_SECRET}' in secrets else 'not_found') | |
| " | grep -Fxq "found"; then | |
| echo "Removing pull secret '${PULL_SECRET}' from SA '${INTEGRATION_SA}' to avoid credential conflict" | |
| INDEX=$(echo "${SA_JSON}" | python3 -c " | |
| import json, sys | |
| sa = json.load(sys.stdin) | |
| for i, s in enumerate(sa.get('secrets', [])): | |
| if s['name'] == '${PULL_SECRET}': | |
| print(i) | |
| break | |
| ") | |
| oc patch sa "${INTEGRATION_SA}" -n "${NAMESPACE}" --type=json \ | |
| -p="[{\"op\":\"remove\",\"path\":\"/secrets/${INDEX}\"}]" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@hack/create-dummy-its.sh` around lines 88 - 100, Update the pull-secret
presence check before the removal block to require an exact “found” result
rather than a substring match, so “not_found” does not enter the patch path.
Preserve the existing INDEX lookup and oc patch behavior when the secret is
actually present, and allow repeat runs to complete when it is absent.
d1f1655 to
007c778
Compare
|
🤖 Finished Review · ✅ Success · Started 5:42 PM UTC · Completed 6:01 PM UTC Commit: |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
🤖 Finished Review · ✅ Success · Started 7:08 PM UTC · Completed 7:26 PM UTC Commit: |
229ef87 to
00d4696
Compare
|
🤖 Finished Review · ✅ Success · Started 4:49 PM UTC · Completed 5:10 PM UTC Commit: |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
00d4696 to
24ad610
Compare
|
🤖 Review · Commit: |
24ad610 to
fbf23a6
Compare
|
🤖 Finished Review · ✅ Success · Started 8:55 PM UTC · Completed 9:15 PM UTC Commit: |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
fbf23a6 to
0d01e69
Compare
|
🤖 Review · Commit: |
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
0d01e69 to
40044de
Compare
|
🤖 Finished Review · ✅ Success · Started 8:18 PM UTC · Completed 8:38 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $5.38 |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Adds a dummy-check Tekton task that produces configurable pass/fail/warn results with a test-result attestation step, a pipeline that parses a Snapshot and runs the task, and scripts to create the IntegrationTestScenario and grant push access. hack/create-dummy-its.sh manages the IntegrationTestScenario; hack/modify-sa-for-dummy-its.sh applies the shared, security-sensitive integration-runner SA push-access patch. Both support --revert. The goal is to dogfood the new method for required tasks to be run in ITS pipelines with their results visible to Conforma with a secure chain of trust. Ref: https://redhat.atlassian.net/browse/EC-2011 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
40044de to
c7707de
Compare
|
🤖 Review · Commit: |
Promote the attest-test-result StepAction's TEST_OUTPUT_ARTIFACT_OUTPUTS step result to a dummy-check Task result, and expose it as a PipelineRun result. Tekton does not auto-promote step results, and this cluster's Chains config only attests at the PipelineRun level, so the _ARTIFACT_OUTPUTS type hint must reach the PipelineRun results for Chains to create SLSA provenance for the pushed attestation. Ref: https://redhat.atlassian.net/browse/EC-2011 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
c7707de to
2371d4e
Compare
|
🤖 Finished Review · ✅ Success · Started 9:04 PM UTC · Completed 9:46 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
The previous approach wrote the StepAction's string result into an object
Task result of the same name, which failed with TaskRunValidationFailed
('expected object type but was initialized to string'). Instead, split the
StepAction's JSON into two string Task results (ARTIFACT_URI, ARTIFACT_DIGEST)
and assemble the _ARTIFACT_OUTPUTS object at the PipelineRun level where Chains
reads it. Adds a guard/debug so an unresolved StepAction result fails loudly.
Ref: https://redhat.atlassian.net/browse/EC-2011
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
🤖 Finished Review · ✅ Success · Started 4:36 PM UTC · Completed 4:56 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $7.16 |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Replace the emit-artifact-outputs step (which could not read the StepAction's result from a later step's env — it resolved to <unset>) with the stable, documented approach: declare ARTIFACT_URI/ARTIFACT_DIGEST Task results whose value is $(steps.create-test-result-attestation.results.<name>). The StepAction now emits those as plain string step results. Point the ref at the fork branch fix-chains-artifact-result until it lands on conforma main. Ref: https://redhat.atlassian.net/browse/EC-2011 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
🤖 Finished Review · ✅ Success · Started 5:51 PM UTC · Completed 6:11 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $5.87 |
| jobs: | ||
|
|
||
| Test: | ||
| if: false # skipped for POC branch |
There was a problem hiding this comment.
[high] CI-coverage-regression
Adding if: false unconditionally disables the Test, Acceptance, and Upload jobs for all PRs and pushes to main/release-*. If merged, PRs land on main without any test or coverage signal.
Suggested fix: Use a branch-name condition (e.g., if: github.head_ref != 'reqd-task-its-poc') or remove the CI-disabling changes.
| jobs: | ||
| lint: | ||
| name: Lint | ||
| if: false # skipped for POC branch |
There was a problem hiding this comment.
[high] CI-coverage-regression
Adding if: false unconditionally disables linting for all PRs and pushes. Removes the lint merge gate.
Suggested fix: Use a branch-name condition or remove this change.
|
|
||
| jobs: | ||
| analyze: | ||
| if: false # skipped for POC branch |
There was a problem hiding this comment.
[high] CI-coverage-regression
Adding if: false disables CodeQL security analysis for all PRs, pushes to main, and the scheduled weekly scan.
Suggested fix: Use a branch-name condition or remove this change.
| - name: url | ||
| value: https://github.com/simonbaird/step-actions | ||
| - name: revision | ||
| value: fix-chains-artifact-result |
There was a problem hiding this comment.
[high] Supply-chain-integrity
The create-test-result-attestation step references a StepAction from a personal fork (simonbaird/step-actions, branch fix-chains-artifact-result) with a mutable branch reference rather than a pinned digest.
Suggested fix: Pin to a specific commit SHA or use the official catalog with a pinned digest.
|
|
||
| stress: | ||
| name: Stress Benchmark | ||
| if: false # skipped for POC branch |
There was a problem hiding this comment.
[medium] scope-creep
Disabling all GitHub Actions CI jobs goes beyond the stated POC goal of dog-fooding a required task in an ITS. Blanket CI suppression is orthogonal scope.
| FAILURE) | ||
| if [[ "${FAILURES}" == "0" ]]; then FAILURES="1"; fi | ||
| if [[ "${SUCCESSES}" == "1" ]]; then SUCCESSES="0"; fi | ||
| ;; |
There was a problem hiding this comment.
[medium] malformed-output
JSON built via printf for the NOTE field without escaping JSON-special characters. Custom NOTE values containing double quotes, backslashes, or newlines produce malformed JSON output.
| # attest-test-result step also needs push access, which requires patching the | ||
| # shared konflux-integration-runner ServiceAccount — a separate, security- | ||
| # sensitive, namespace-wide change handled by hack/modify-sa-for-dummy-its.sh. | ||
| # Run that script after this one to grant push access. |
There was a problem hiding this comment.
[medium] shell-idiom
Uses set -euo pipefail instead of the long-form set -o errexit; set -o nounset; set -o pipefail convention established by other scripts in hack/.
| # in its own script (separate from create-dummy-its.sh) precisely because it is | ||
| # a shared, namespace-wide, security-sensitive change. | ||
| # | ||
| # TODO: Ideally only specific ITS pipelines that need push access should |
There was a problem hiding this comment.
[medium] shell-idiom
Uses set -euo pipefail instead of the long-form set -o errexit; set -o nounset; set -o pipefail convention established by other scripts in hack/.
|
|
||
| --- | ||
| apiVersion: tekton.dev/v1 | ||
| kind: Task |
There was a problem hiding this comment.
[low] yaml-metadata-ordering
Metadata field order (labels, annotations, name) differs from existing tasks convention (name, annotations, labels).
| name: dummy-check | ||
| spec: | ||
| description: >- | ||
| A dummy task for testing purposes. Instead of performing a real check, it |
There was a problem hiding this comment.
[low] yaml-formatting
Missing blank line between metadata and spec blocks, inconsistent with existing task YAMLs.
2 participants
For review bots: You can ignore this PR, it exists to test some pipelines and artifacts in Konflux. It won't be merged in this repo.
Summary
attest-test-resultstep action)reqd-task-poc-ec2011pipeline that parses a Snapshot and runs the dummy-check taskhack/create-dummy-its.shto create the IntegrationTestScenario in the clusterContext
POC for EC-2011 — dog-fooding a required task in an ITS instead of the build pipeline.
Test plan
rhtap-contract-tenantnamespace🤖 Generated with Claude Code