Add Konflux override snapshot generation script and slash command#2879
Add Konflux override snapshot generation script and slash command#2879jeff-roche wants to merge 2 commits into
Conversation
Adds hack/generate_override_snapshot_yaml.sh to derive an override Snapshot manifest from a bundle image (pulling related image refs and git revisions), plus a /lvms:konflux:create_snapshot command that drives the generate -> save -> apply -> monitor workflow end to end.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
openshift-ci
Bot
added
the
size/L
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jeff-roche The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
|
Example Output: apiVersion: appstudio.redhat.com/v1alpha1
kind: Snapshot
metadata:
name: lvm-operator-4-21-1-override
namespace: logical-volume-manag-tenant
labels:
test.appstudio.openshift.io/type: override
spec:
application: lvm-operator-4-21
components:
- name: lvm-operator-4-21
containerImage: quay.io/redhat-user-workloads/logical-volume-manag-tenant/lvm-operator@sha256:cefc215d0e811849dcad97d6ea3f1179c9755e04a4394cdc483b1b28570d3fe8
source:
git:
url: https://github.com/openshift/lvm-operator
revision: 47c284ae7fdc66dfe65e58e18828b1a747f6db44
- name: lvms-must-gather-4-21
containerImage: quay.io/redhat-user-workloads/logical-volume-manag-tenant/lvms-must-gather@sha256:760a62a1ea3c326df44c0b96bbf4f66c88204dbec42a96dba7704504c5dca4bd
source:
git:
url: https://github.com/openshift/lvm-operator
revision: 47c284ae7fdc66dfe65e58e18828b1a747f6db44
- name: lvm-operator-bundle-4-21
containerImage: quay.io/redhat-user-workloads/logical-volume-manag-tenant/lvm-operator-bundle@sha256:7dfe7320f96cb7b2202d434524dd7c3545bb9c53ac2671e77f97f3e250bb4e35
source:
git:
url: https://github.com/openshift/lvm-operator
revision: fb6b7833a21ad2afc2def392e2b1ffbc4edb3fa4
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
hack/generate_override_snapshot_yaml.sh (1)
202-217: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSimplify YAML generation by appending text directly instead of using
yq.Using
yqto parse and append to the YAML introduces a third-party dependency that often suffers from incompatible implementations (e.g.,mikefarah/yqvskislyuk/yq). For example, running this on a system with Pythonyqinstalled will evaluate the expression but output JSON instead of YAML, breaking the manifest.Since the appended component structure is simple and static, you can safely append it as a plain string, matching the existing indentation.
Note: If you apply this change, you can also safely remove
yqfrom thecheck_dependencieslist on line 45.♻️ Proposed refactor
- topolvm_obj=$(cat <<-EOF - { - "name": "topolvm-${X}-${Y}", - "containerImage": "${TOPOLVM_IMG}", - "source": { - "git": { - "url": "https://github.com/openshift/topolvm", - "revision": "${TOPOLVM_GIT_REF}" - } - } - } - EOF - ) - - output_yaml=$(echo "${output_yaml}" | yq ".spec.components += [${topolvm_obj}]") + output_yaml="${output_yaml} + - name: topolvm-${X}-${Y} + containerImage: ${TOPOLVM_IMG} + source: + git: + url: https://github.com/openshift/topolvm + revision: ${TOPOLVM_GIT_REF}"🤖 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/generate_override_snapshot_yaml.sh` around lines 202 - 217, Replace the yq-based update in the topolvm_obj generation block with direct text appending to output_yaml using the existing YAML indentation and component structure. Preserve the generated topolvm-${X}-${Y}, TOPOLVM_IMG, and TOPOLVM_GIT_REF values, and remove yq from the check_dependencies list since it is no longer required.
🤖 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 @.claude/commands/lvms/konflux/create_snapshot.md:
- Around line 31-34: Add a final cluster-context validation immediately before
the apply and polling steps using oc whoami --show-server and oc project -q,
confirming the expected cluster and project. Ensure the background poller
inherits and uses this validated context, preventing later kubeconfig changes
from targeting or monitoring a different cluster.
- Around line 4-8: Restrict the allowed-tools entry in the create_snapshot
command so it no longer grants unrestricted Bash access. Permit only the
specific oc and snapshot-generator operations required by the workflow, or route
them through a dedicated script and allow that script instead; preserve the
existing Read and AskUserQuestion permissions.
In `@hack/generate_override_snapshot_yaml.sh`:
- Around line 57-66: Quote the image_ref expansions in get_image_sha, and
replace the echo-wrapped command substitution in get_image_git_ref with the
direct skopeo-to-jq pipeline. In hack/generate_override_snapshot_yaml.sh lines
126-132, quote the docker image argument and bundle_image passed to
get_image_git_ref; in lines 148-150, quote each command substitution passed to
get_image_sha.
- Line 16: Replace the predictable TEMP_DIR construction with a directory
created atomically via mktemp, preserving the existing temporary-directory usage
and ensuring cleanup continues to target the created path.
---
Nitpick comments:
In `@hack/generate_override_snapshot_yaml.sh`:
- Around line 202-217: Replace the yq-based update in the topolvm_obj generation
block with direct text appending to output_yaml using the existing YAML
indentation and component structure. Preserve the generated topolvm-${X}-${Y},
TOPOLVM_IMG, and TOPOLVM_GIT_REF values, and remove yq from the
check_dependencies list since it is no longer required.
🪄 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: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 9390a82b-aa9c-43bd-9d68-5606c7199762
📒 Files selected for processing (2)
.claude/commands/lvms/konflux/create_snapshot.mdhack/generate_override_snapshot_yaml.sh
- Use mktemp for the script's temp directory instead of a predictable path, and quote command substitutions/variable expansions to avoid word splitting and glob expansion. - Drop the yq dependency by appending the topolvm component as plain YAML text instead of merging JSON into the document. - Restrict the slash command's allowed-tools to the specific oc/script invocations it needs, and validate the oc cluster context right before applying and polling the snapshot. Assisted-by: Claude <noreply@anthropic.com>
|
@jeff-roche: 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
Summary
hack/generate_override_snapshot_yaml.sh, which derives an overrideSnapshotmanifest from a bundle image by pulling related image digests and git revisions./lvms:konflux:create_snapshotcommand that drives the generate → save → apply → monitor workflow for creating and applying an override snapshot in Konflux.Test plan
hack/generate_override_snapshot_yaml.shagainst a bundle image and confirmed the generated YAML matched the expectedSnapshotstructure.logical-volume-manag-tenantnamespace and confirmed it was created and began integration testing.Summary by CodeRabbit