OSAC-2188: Reconcile overlay values with chart releases - #457
minmzzhang wants to merge 3 commits into
Conversation
|
@minmzzhang: This pull request references OSAC-2188 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions 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 openshift-eng/jira-lifecycle-plugin repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: minmzzhang 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 |
|
Note 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:
WalkthroughThe release workflow pins overlay values to released versions and opens an automated pull request. A new script performs version-based rewrites, while image-tag synchronization reports SHA mismatches and skips non-SHA pins. Documentation describes the updated lifecycle. ChangesRelease overlay pinning
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant PublishWorkflow
participant PinReleaseTags
participant OverlayValues
participant GitHub
PublishWorkflow->>PinReleaseTags: pass released component versions
PinReleaseTags->>OverlayValues: rewrite image tags and projectGitBranch
PublishWorkflow->>GitHub: push versioned automation branch
GitHub->>GitHub: create or update pull request against main
🚥 Pre-merge checks | ✅ 11✅ Passed checks (11 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![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
🤖 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/publish-charts.yaml:
- Around line 181-188: Add SAST and SCA gating steps to the chart release job
before packaging or publishing, then sign the generated chart artifacts with
Sigstore/cosign before the push step. Update the workflow permissions to include
the required OIDC token permission for signing, and ensure failed security
checks or signing prevent publication.
In `@scripts/pin-release-tags.sh`:
- Line 43: Update the image-tag matching in scripts/pin-release-tags.sh lines
43-43, 47-47, and 57-57 to match complete SemVer tags, including optional
prerelease and build metadata through the terminating delimiter, for
fulfillment-service, osac-aap, and osac-ui. Update the extraction and
replacement logic in scripts/sync-image-tags.sh lines 52-57 likewise so SHA
rewrites consume the full tag and cannot leave suffixes behind.
🪄 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: osac-project/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: d1af6e03-8cac-4835-b44b-bbdc1918523b
📒 Files selected for processing (4)
.github/workflows/publish-charts.yamlAGENTS.mdscripts/pin-release-tags.shscripts/sync-image-tags.sh
|
this is a big change, and would break how people currently expect to work with osac installer. bump submodules will have no effect on the CI until the nightly job runs and publishes the next chart. why do we want to go this route? @minmzzhang @eliorerz |
The "why" can be found here https://redhat.atlassian.net/browse/OSAC-2188 |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
AGENTS.md (2)
77-88: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDocument tag-sync modes and pattern limits precisely.
This wording implies that every tag format is rewritten, but
scripts/sync-image-tags.shonly replaces non-SHA pins with--fix; its default mode skips them.scripts/pin-release-tags.shalso matches only specificsha-, numericv, andlatestpatterns. Clarify these limits so operators do not assume digest, prerelease, or arbitrary tags are reconciled.🤖 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 `@AGENTS.md` around lines 77 - 88, Update the Image Tag Lifecycle documentation to state that sync-image-tags.sh rewrites non-SHA tags only when invoked with --fix; its default mode skips them. Document that pin-release-tags.sh handles only sha-, numeric v-prefixed, and latest tags, and explicitly exclude digest, prerelease, and arbitrary tag formats from the reconciliation guarantee.
96-105: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd
pin-release-tags.shto the script inventory.The lifecycle section documents
scripts/pin-release-tags.sh, but the Scripts list omits it. Add an entry describing its release-time overlay image and AAP branch pinning behavior.🤖 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 `@AGENTS.md` around lines 96 - 105, Add pin-release-tags.sh to the Scripts inventory in AGENTS.md, describing that it pins overlay images and AAP branches for releases. Keep the entry aligned with the existing script descriptions.
🤖 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.
Outside diff comments:
In `@AGENTS.md`:
- Around line 77-88: Update the Image Tag Lifecycle documentation to state that
sync-image-tags.sh rewrites non-SHA tags only when invoked with --fix; its
default mode skips them. Document that pin-release-tags.sh handles only sha-,
numeric v-prefixed, and latest tags, and explicitly exclude digest, prerelease,
and arbitrary tag formats from the reconciliation guarantee.
- Around line 96-105: Add pin-release-tags.sh to the Scripts inventory in
AGENTS.md, describing that it pins overlay images and AAP branches for releases.
Keep the entry aligned with the existing script descriptions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: osac-project/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: e513348f-6adf-4260-aedb-58c479583887
📒 Files selected for processing (1)
AGENTS.md
3d0a752 to
653d8d1
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/publish-charts.yaml:
- Line 273: Update the publish workflow’s repository authentication around the
git remote setup so GH_TOKEN is not embedded in the remote URL or written to
.git/config; configure checkout to use the bot token and preserve its standard
persisted authentication header, then keep origin on the normal repository URL.
In `@scripts/pin-release-tags.sh`:
- Around line 16-20: Update the argument validation in
scripts/pin-release-tags.sh before any rewrite to require each of OPERATOR_VER,
SERVICE_VER, AAP_VER, BMF_VER, and UI_VER to match exactly the stable X.Y.Z
version format, rejecting prerelease or other suffixes such as -rc.1; ensure the
existing rewrite logic runs only after all five values pass validation.
🪄 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: osac-project/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: c9f86650-b6c3-47aa-8fe6-a1af22e47c67
📒 Files selected for processing (4)
.github/workflows/publish-charts.yamlAGENTS.mdscripts/pin-release-tags.shscripts/sync-image-tags.sh
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/publish-charts.yaml (1)
183-190: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse the stable component-version contract before invoking the pinning helper.
The workflow’s component validation accepts prerelease/build values such as
1.2.3-rc.1and1.2.3+build.5, butscripts/pin-release-tags.shrequires exactX.Y.Z. Such a dispatch passes resolution and then aborts at this new step. Use a separate stable regex for the component loop while retaining the broader regex for the umbrella chart version.Based on learnings, released component image tags use only plain stable
vX.Y.Z; keep both layers on that contract.Proposed fix
SEMVER_RE='^[0-9]+\.[0-9]+\.[0-9]+(-[a-zA-Z0-9.]+)?(\+[a-zA-Z0-9.]+)?$' +STABLE_VERSION_RE='^[0-9]+\.[0-9]+\.[0-9]+$' ... - if ! [[ "${ver}" =~ ${SEMVER_RE} ]]; then + if ! [[ "${ver}" =~ ${STABLE_VERSION_RE} ]]; then🤖 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/publish-charts.yaml around lines 183 - 190, Update the component-version validation used before the pin-release step and scripts/pin-release-tags.sh to accept only plain stable X.Y.Z values (with released image tags remaining vX.Y.Z), rejecting prerelease and build metadata. Keep the broader validation regex for the umbrella chart version unchanged, and use a separate stable regex for the component loop.Source: Learnings
🤖 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.
Outside diff comments:
In @.github/workflows/publish-charts.yaml:
- Around line 183-190: Update the component-version validation used before the
pin-release step and scripts/pin-release-tags.sh to accept only plain stable
X.Y.Z values (with released image tags remaining vX.Y.Z), rejecting prerelease
and build metadata. Keep the broader validation regex for the umbrella chart
version unchanged, and use a separate stable regex for the component loop.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: osac-project/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 510bc534-252d-4bbe-8760-d52485ea2e52
📒 Files selected for processing (3)
.github/workflows/publish-charts.yamlAGENTS.mdscripts/pin-release-tags.sh
Add pin-release-tags.sh called by publish-charts.yaml to pin overlays to released versions. Update sync-image-tags.sh to handle v-prefixed and latest tags for the post-release bump cycle. Assisted-by: Cursor <cursoragent@cursor.com> Signed-off-by: Min Zhang <minzhang@redhat.com>
4213065 to
35a60df
Compare
Resolve AGENTS.md conflict: keep main's local-dev submodule guidance and retain the Image Tag Lifecycle docs from OSAC-2188.
4 participants
Summary
scripts/pin-release-tags.shto pin all overlay values files to released versions at chart publish timepublish-charts.yamlto call the script and open a PR with pinned values after each releasesync-image-tags.shto handlev-prefixed andlatesttags so bump-submodules advances overlays post-releaseDepends on osac-aap#412.
Test plan
pin-release-tags.shcorrectly replacessha-,v-prefixed, andlatesttags across all overlayssync-image-tags.shverify mode skips non-sha-tags (no false CI failures)sync-image-tags.sh --fixreplaces pinnedv-tags back tosha-tagsactionlintandbash -npassFixes OSAC-2188.
Summary by CodeRabbit
latest/unversioned tags with versioned tags.