konflux: add integration test pipeline of updating bundle from latest snapshot from main branch.#1792
konflux: add integration test pipeline of updating bundle from latest snapshot from main branch.#1792raptorsun wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a new Tekton Pipeline, ChangesBundle Update Pipeline
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Pipeline as bundle-update-integration-test
participant SourceRepo as Source Git Repo
participant ForkRepo as Fork Git Repo
participant GitHubAPI as GitHub API
participant Quay as Quay Log Export
Pipeline->>SourceRepo: Clone sourceBranch
Pipeline->>Pipeline: Create forkBranch locally
Pipeline->>Pipeline: Update related_images.json from SNAPSHOT
Pipeline->>Pipeline: Run make bundle
alt changes detected
Pipeline->>Pipeline: git commit
Pipeline->>ForkRepo: git push forkBranch
Pipeline->>GitHubAPI: Create pull request
GitHubAPI-->>Pipeline: pull-request-url
else no changes
Pipeline->>Pipeline: Write no-change and no-pr-needed
end
Pipeline->>Quay: Export logs in finally block
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
/hold |
openshift-ci
Bot
added
the
do-not-merge/hold
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
.tekton/integration-tests/pipelines/bundle-update-pipeline.yaml (2)
19-19: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePersonal fork/user/email hardcoded as defaults.
forkGitUrl/forkUserdefault to an individual's personal fork (raptorsun), and the git author is fixed tohasun@redhat.com/Haoyu Sun's Bot. For a shared integration-test pipeline this is fragile and ties the flow to one contributor. Consider a team/bot-owned fork and a generic bot identity.Also applies to: 27-27, 137-138
🤖 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 @.tekton/integration-tests/pipelines/bundle-update-pipeline.yaml at line 19, The default values in the bundle update pipeline are tied to a personal fork and individual author identity, so update the configuration in the pipeline definition to use team-owned/bot-owned defaults instead of the current contributor-specific Git URL, forkUser, git author email, and display name. Review the settings for forkGitUrl, forkUser, and the git author fields in the bundle update pipeline and replace them with shared integration-test values that are stable for all contributors.
170-171: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePR target repo is hardcoded and ignores
sourceGitUrl.The pull request is always opened against
openshift/lightspeed-operator, while the clone source is driven by$(params.sourceGitUrl). OverridingsourceGitUrlwill clone one repo but attempt to open the PR against another. Derive theowner/repofromsourceGitUrl(or document that this param is fixed) to keep the two consistent.🤖 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 @.tekton/integration-tests/pipelines/bundle-update-pipeline.yaml around lines 170 - 171, The PR creation step is hardcoded to openshift/lightspeed-operator, which can diverge from the repository selected by sourceGitUrl. Update the PR target in the bundle update pipeline to derive the owner/repo from sourceGitUrl, or explicitly document that sourceGitUrl is fixed and must match the target repo. Use the PR creation command in the bundle update pipeline and the params.sourceGitUrl / params.sourceBranch / params.forkUser / params.forkBranch values to keep cloning and PR destination consistent.
🤖 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 @.tekton/integration-tests/pipelines/bundle-update-pipeline.yaml:
- Around line 165-174: The GitHub PR creation step in the bundle update pipeline
is swallowing HTTP failures and still writing a bogus result. Update the
curl-based request that assigns RESPONSE to fail on non-2xx responses and only
emit pull-request-url after verifying the response contains a valid html_url; if
the API call fails or returns an error payload, stop the step instead of piping
null into the result. Use the RESPONSE handling and jq extraction in this step
as the main places to harden.
- Around line 106-122: The jq update in the related_images.json generation step
is dropping existing entries whenever a snapshot component is missing. Update
the transformation around the lightspeed-service, lightspeed-console,
lightspeed-operator, openshift-mcp-server, and lightspeed-ocp-rag selects so
each target entry is only replaced when a matching snapshot component exists,
otherwise the prior related_images.json value is preserved. Keep the existing
lightspeed-operator-bundle, lightspeed-to-dataverse-exporter, and
lightspeed-postgresql entries intact while merging in any available
snapshot-derived images.
---
Nitpick comments:
In @.tekton/integration-tests/pipelines/bundle-update-pipeline.yaml:
- Line 19: The default values in the bundle update pipeline are tied to a
personal fork and individual author identity, so update the configuration in the
pipeline definition to use team-owned/bot-owned defaults instead of the current
contributor-specific Git URL, forkUser, git author email, and display name.
Review the settings for forkGitUrl, forkUser, and the git author fields in the
bundle update pipeline and replace them with shared integration-test values that
are stable for all contributors.
- Around line 170-171: The PR creation step is hardcoded to
openshift/lightspeed-operator, which can diverge from the repository selected by
sourceGitUrl. Update the PR target in the bundle update pipeline to derive the
owner/repo from sourceGitUrl, or explicitly document that sourceGitUrl is fixed
and must match the target repo. Use the PR creation command in the bundle update
pipeline and the params.sourceGitUrl / params.sourceBranch / params.forkUser /
params.forkBranch values to keep cloning and PR destination consistent.
🪄 Autofix (Beta)
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: a42eca87-1453-47f6-abc0-c5bf7a7d66db
📒 Files selected for processing (1)
.tekton/integration-tests/pipelines/bundle-update-pipeline.yaml
| jq --argjson snapshot "${SNAPSHOT}" ' | ||
| [.[] | select( | ||
| .name == "lightspeed-operator-bundle" or | ||
| .name == "lightspeed-to-dataverse-exporter" or | ||
| .name == "lightspeed-postgresql" | ||
| )] + | ||
| [$snapshot.components[] | select(.name == "lightspeed-service") | | ||
| {name: "lightspeed-service-api", image: .containerImage, revision: .source.git.revision}] + | ||
| [$snapshot.components[] | select(.name == "lightspeed-console") | | ||
| {name: "lightspeed-console-plugin", image: .containerImage, revision: .source.git.revision}] + | ||
| [$snapshot.components[] | select(.name == "lightspeed-operator") | | ||
| {name: "lightspeed-operator", image: .containerImage, revision: .source.git.revision}] + | ||
| [$snapshot.components[] | select(.name == "openshift-mcp-server") | | ||
| {name: "openshift-mcp-server", image: .containerImage, revision: .source.git.revision}] + | ||
| [$snapshot.components[] | select(.name == "lightspeed-ocp-rag") | | ||
| {name: "lightspeed-ocp-rag", image: .containerImage, revision: .source.git.revision}] | ||
| ' related_images.json > tmp.json && mv tmp.json related_images.json |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect current related_images.json to enumerate expected entries and cross-check names used by the jq mapping.
fd -t f 'related_images.json' --exec cat {}Repository: openshift/lightspeed-operator
Length of output: 2134
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the pipeline context around the jq transformation.
file=".tekton/integration-tests/pipelines/bundle-update-pipeline.yaml"
wc -l "$file"
sed -n '1,220p' "$file"
# Find where SNAPSHOT comes from and where related_images.json is used.
rg -n --no-heading 'SNAPSHOT|related_images\.json|lightspeed-service-api|lightspeed-console-plugin|lightspeed-operator|openshift-mcp-server|lightspeed-ocp-rag' .tekton . -g '!**/.git/**'Repository: openshift/lightspeed-operator
Length of output: 50386
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file=".tekton/integration-tests/pipelines/bundle-update-pipeline.yaml"
echo "== file length =="
wc -l "$file"
echo "== relevant lines =="
sed -n '1,220p' "$file"
echo "== where this pipeline is referenced =="
rg -n --no-heading 'bundle-update-pipeline\.yaml|bundle-update' .tekton -g '!**/.git/**'
echo "== snapshot/component usage in this pipeline =="
rg -n --no-heading 'SNAPSHOT|related_images\.json|lightspeed-service-api|lightspeed-console-plugin|lightspeed-operator|openshift-mcp-server|lightspeed-ocp-rag' "$file"Repository: openshift/lightspeed-operator
Length of output: 13976
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== bundle-update scenario references =="
rg -n --no-heading 'bundle-update-integration-test|bundle-update-pipeline|update-related-images|related_images\.json|SNAPSHOT' .tekton -g '!**/.git/**'
echo "== bundle-update related scenario file if any =="
fd -a 'bundle-update' .tekton
echo "== integration-test scenario excerpts =="
sed -n '1,220p' .tekton/integration-tests/integration-test-scenarios/konflux-integration-test-scenarios.yaml
echo "== release bundle-update pipeline excerpt =="
sed -n '1,220p' .tekton/release/bundle-update/pipeline.yamlRepository: openshift/lightspeed-operator
Length of output: 50385
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== snapshot_to_image_list.sh =="
fd -a 'snapshot_to_image_list.sh' .
script="$(fd -a 'snapshot_to_image_list.sh' . | head -n 1)"
wc -l "$script"
cat -n "$script"
echo "== bundle-update release pipeline jq logic around related images =="
sed -n '96,116p' .tekton/release/bundle-update/pipeline.yamlRepository: openshift/lightspeed-operator
Length of output: 14848
🏁 Script executed:
#!/bin/bash
set -euo pipefail
script="$(fd -a 'snapshot_to_image_list.sh' . | head -n 1)"
echo "== $script =="
wc -l "$script"
cat -n "$script"
echo "== related image handling in .tekton/release/bundle-update/pipeline.yaml =="
sed -n '96,116p' .tekton/release/bundle-update/pipeline.yamlRepository: openshift/lightspeed-operator
Length of output: 14830
🌐 Web query:
Konflux AppStudio snapshot spec components always include all components application under test documentation
💡 Result:
In Konflux AppStudio, a Snapshot does not necessarily include all components of an application under test in every instance. A Snapshot represents an immutable collection of container images for the components of an application at a specific point in time [1]. When the Integration Service creates a snapshot following a successful component build, it includes the newly built component and the component references from the application's Global Candidate List (GCL) [2][3]. The GCL tracks the most recently promoted image and commit for each component [3]. While the snapshot is intended to represent the state of the application components to be tested together [1][4], users can also create manual snapshots or override snapshots [5][6]. An override snapshot allows users to explicitly define the state of specific components, effectively resetting the GCL for those components [5][3]. Therefore, the components listed in the snapshot spec are the specific collection that the Integration Service will test as a whole [2][4]. Top Results: [1][2][3]
Citations:
- 1: https://konflux-ci.dev/docs/testing/integration/snapshots/
- 2: https://konflux-ci.dev/architecture/core/integration-service/
- 3: https://konflux-ci.dev/docs/testing/integration/
- 4: https://konflux-ci.dev/docs/getting-started/
- 5: https://konflux-ci.dev/docs/testing/integration/snapshots/override-snapshots/
- 6: https://konflux-ci.dev/docs/testing/integration/snapshots/working-with-snapshots/
Preserve existing entries when a snapshot component is absent Konflux snapshots can be partial, so each select(.name == …) here can drop lightspeed-service-api, lightspeed-console-plugin, lightspeed-operator, openshift-mcp-server, or lightspeed-ocp-rag from related_images.json instead of keeping the previous value. That leaves the generated bundle incomplete.
🤖 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 @.tekton/integration-tests/pipelines/bundle-update-pipeline.yaml around lines
106 - 122, The jq update in the related_images.json generation step is dropping
existing entries whenever a snapshot component is missing. Update the
transformation around the lightspeed-service, lightspeed-console,
lightspeed-operator, openshift-mcp-server, and lightspeed-ocp-rag selects so
each target entry is only replaced when a matching snapshot component exists,
otherwise the prior related_images.json value is preserved. Keep the existing
lightspeed-operator-bundle, lightspeed-to-dataverse-exporter, and
lightspeed-postgresql entries intact while merging in any available
snapshot-derived images.
| RESPONSE=$(curl -L \ | ||
| -X POST \ | ||
| -H "Accept: application/vnd.github+json" \ | ||
| -H "Authorization: Bearer ${GITHUB_TOKEN}" \ | ||
| -H "X-GitHub-Api-Version: 2022-11-28" \ | ||
| https://api.github.com/repos/openshift/lightspeed-operator/pulls \ | ||
| -d '{"title":"Bundle Update from Integration Test Snapshot","body":"This PR updates the related_images and bundle based on the latest snapshot from Konflux integration test.","head":"$(params.forkUser):$(params.forkBranch)","base":"$(params.sourceBranch)"}') | ||
| echo "Github response:" | ||
| echo "$RESPONSE" | ||
| jq -r '.html_url' <<< "$RESPONSE" | tee $(results.pull-request-url.path) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
PR creation failures are silently swallowed; the pipeline still reports success.
curl -L without -f/--fail-with-body exits 0 on HTTP 4xx/5xx (e.g., a 422 when the PR already exists, or auth failure). With set -euo pipefail the step still succeeds, RESPONSE holds an error body, and jq -r '.html_url' writes the literal null into the pull-request-url result. The task then reports success with no usable PR URL.
Fail fast and validate the response before publishing the result:
🛡️ Proposed hardening for the GitHub call
- RESPONSE=$(curl -L \
- -X POST \
+ HTTP_CODE=$(curl -sS -L \
+ -o response.json -w '%{http_code}' \
+ -X POST \
-H "Accept: application/vnd.github+json" \
-H "Authorization: Bearer ${GITHUB_TOKEN}" \
-H "X-GitHub-Api-Version: 2022-11-28" \
https://api.github.com/repos/openshift/lightspeed-operator/pulls \
-d '{"title":"Bundle Update from Integration Test Snapshot","body":"This PR updates the related_images and bundle based on the latest snapshot from Konflux integration test.","head":"$(params.forkUser):$(params.forkBranch)","base":"$(params.sourceBranch)"}')
echo "Github response:"
- echo "$RESPONSE"
- jq -r '.html_url' <<< "$RESPONSE" | tee $(results.pull-request-url.path)
+ cat response.json
+ if [ "$HTTP_CODE" -lt 200 ] || [ "$HTTP_CODE" -ge 300 ]; then
+ echo "GitHub PR creation failed with HTTP $HTTP_CODE" >&2
+ exit 1
+ fi
+ jq -r '.html_url' response.json | tee $(results.pull-request-url.path)📝 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.
| RESPONSE=$(curl -L \ | |
| -X POST \ | |
| -H "Accept: application/vnd.github+json" \ | |
| -H "Authorization: Bearer ${GITHUB_TOKEN}" \ | |
| -H "X-GitHub-Api-Version: 2022-11-28" \ | |
| https://api.github.com/repos/openshift/lightspeed-operator/pulls \ | |
| -d '{"title":"Bundle Update from Integration Test Snapshot","body":"This PR updates the related_images and bundle based on the latest snapshot from Konflux integration test.","head":"$(params.forkUser):$(params.forkBranch)","base":"$(params.sourceBranch)"}') | |
| echo "Github response:" | |
| echo "$RESPONSE" | |
| jq -r '.html_url' <<< "$RESPONSE" | tee $(results.pull-request-url.path) | |
| HTTP_CODE=$(curl -sS -L \ | |
| -o response.json -w '%{http_code}' \ | |
| -X POST \ | |
| -H "Accept: application/vnd.github+json" \ | |
| -H "Authorization: Bearer ${GITHUB_TOKEN}" \ | |
| -H "X-GitHub-Api-Version: 2022-11-28" \ | |
| https://api.github.com/repos/openshift/lightspeed-operator/pulls \ | |
| -d '{"title":"Bundle Update from Integration Test Snapshot","body":"This PR updates the related_images and bundle based on the latest snapshot from Konflux integration test.","head":"$(params.forkUser):$(params.forkBranch)","base":"$(params.sourceBranch)"}') | |
| echo "Github response:" | |
| cat response.json | |
| if [ "$HTTP_CODE" -lt 200 ] || [ "$HTTP_CODE" -ge 300 ]; then | |
| echo "GitHub PR creation failed with HTTP $HTTP_CODE" >&2 | |
| exit 1 | |
| fi | |
| jq -r '.html_url' response.json | tee $(results.pull-request-url.path) |
🤖 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 @.tekton/integration-tests/pipelines/bundle-update-pipeline.yaml around lines
165 - 174, The GitHub PR creation step in the bundle update pipeline is
swallowing HTTP failures and still writing a bogus result. Update the curl-based
request that assigns RESPONSE to fail on non-2xx responses and only emit
pull-request-url after verifying the response contains a valid html_url; if the
API call fails or returns an error payload, stop the step instead of piping null
into the result. Use the RESPONSE handling and jq extraction in this step as the
main places to harden.
raptorsun
force-pushed
the
bundle-update-pipeline
branch
from
2d90f1d to
f05cb66
Compare
… snapshot from main branch. Signed-off-by: Haoyu Sun <hasun@redhat.com>
raptorsun
force-pushed
the
bundle-update-pipeline
branch
from
f05cb66 to
b0fa331
Compare
|
@raptorsun: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
1 participant
Description
this replace the final pipeline of bundle update, while waiting for the EC exception approval that deblocks our release.
Type of change
Related Tickets & Documents
Checklist before requesting a review
Testing
Summary by CodeRabbit