OSAC-2105: pin umbrella chart image tags at publish time - #394
Conversation
The publish-charts.yaml sed only matched 'tag: latest', missing images expressed as repo:tag strings (service.images.service, aap.bootstrap.image, ui.images.ui, dbMigrate.image). Replace it with a yq expression that stamps every image using its resolved per-component version.
|
@eliorerz: This pull request references OSAC-2105 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 task 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 APPROVED This pull-request has been approved by: eliorerz 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 |
WalkthroughThe publish-charts workflow's values.yaml update step was changed from a single sed-based version substitution to targeted yq commands setting individual image/tag fields for operator, bmf, fulfillment-service, osac-aap, and osac-ui components, followed by a grep to display the updated lines. ChangesChart Values Update Mechanism
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 11✅ Passed checks (11 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/publish-charts.yaml:
- Around line 183-190: The image tag updates in the publish-charts workflow are
using version-based refs instead of the published sha- convention. Update the yq
assignments in the chart values update step to set each image tag/reference to
the corresponding sha-${SHORT_SHA}-style tag used elsewhere in the repo, and
keep this consistent for operator, bmf, service, aap, ui, and dbMigrate so the
published chart pulls valid images.
🪄 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: 8c79cf3f-36ad-4b66-8b97-92f476534ca3
📒 Files selected for processing (1)
.github/workflows/publish-charts.yaml
| yq -i " | ||
| .operator.image.tag = \"v${OPERATOR_VER}\" | | ||
| .bmf.image.tag = \"v${BMF_VER}\" | | ||
| .service.images.service = \"ghcr.io/osac-project/fulfillment-service:v${SERVICE_VER}\" | | ||
| .aap.bootstrap.image = \"ghcr.io/osac-project/osac-aap:v${AAP_VER}\" | | ||
| .ui.images.ui = \"ghcr.io/osac-project/osac-ui:v${UI_VER}\" | | ||
| .dbMigrate.image = \"ghcr.io/osac-project/fulfillment-service:v${SERVICE_VER}\" | ||
| " charts/osac/values.yaml |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find image build/push workflows and inspect their tag conventions
fd . .github/workflows -e yaml -e yml | xargs rg -n -i 'tag|IMAGE_TAG|docker push|buildah push' -A3 -B3Repository: osac-project/osac-installer
Length of output: 7430
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== scripts/sync-image-tags.sh =="
if [ -f scripts/sync-image-tags.sh ]; then
cat -n scripts/sync-image-tags.sh
else
echo "missing"
fi
echo
echo "== charts/osac/values.yaml (image refs) =="
rg -n 'ghcr\.io/osac-project|image\.tag|dbMigrate|service:|bootstrap:' charts/osac/values.yaml -A2 -B2
echo
echo "== repository search for tag conventions =="
rg -n 'sha-\$\{|v\$\{|ghcr\.io/osac-project|IMAGE_TAG|tag:' .github scripts charts -g '!**/*.png' -g '!**/*.svg' -A2 -B2Repository: osac-project/osac-installer
Length of output: 22616
Use the published sha- image tags here. In .github/workflows/publish-charts.yaml:183-190, these image refs should follow the same sha-<shortsha> convention used elsewhere in the repo; v${...} will not resolve to published images and will break pulls.
🤖 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, The image tag
updates in the publish-charts workflow are using version-based refs instead of
the published sha- convention. Update the yq assignments in the chart values
update step to set each image tag/reference to the corresponding
sha-${SHORT_SHA}-style tag used elsewhere in the repo, and keep this consistent
for operator, bmf, service, aap, ui, and dbMigrate so the published chart pulls
valid images.
2 participants
Summary
publish-charts.yamlonly rewrotetag: latestvia sed, which matchesoperator.image.tagandbmf.image.tagbut misses images expressed as combinedrepo:tagstrings (service.images.service,aap.bootstrap.image,ui.images.ui,dbMigrate.image), so those shipped:latestin every published OCI version (0.0.1-0.0.4).yqexpression that stamps each image using its already-resolved per-component version (operator_ver,service_ver,aap_ver,bmf_ver,ui_ver), matching the pattern already used for theChart.yamldependency rewrite step.charts/osac/values.yamlwith fake version env vars — all previously-:latestfields resolve correctly, nothing else changes.Fixes https://redhat.atlassian.net/browse/OSAC-2105
Test plan
workflow_dispatchonPublish Helm Charts(or dry-run the step) and confirmhelm templateoutput has no:latestimagesSummary by CodeRabbit