feat: add container image version bumper (make container-bump-image-versions)#5811
feat: add container image version bumper (make container-bump-image-versions)#5811jonpspri wants to merge 4 commits into
Conversation
6e419fc to
043e8eb
Compare
|
@jonpspri with respect to this PR there are Two Blocking Issues Issue 1 : repo_for_arg maintains a parallel hard-coded map that can drift from the actual image path already available in the Containerfile. The Pyxis repo path should be derived from ${value%:*} (which the script already extracts as image_of[$arg]) rather than a separate static case statement. If a deliberate minor-bump changes the path, the map would silently query the wrong endpoint. Issue 2: sed -i '' in the bats test is macOS-only. GNU sed (Ubuntu — the likely CI runner) has no empty-suffix form; it will fail with "No such file or directory". The main script uses the portable sed -i.bak pattern already; the test should match it. |
| local f | ||
| for f in "${files[@]}"; do | ||
| if sed -i.bak "s|^ARG ${arg}=.*|ARG ${arg}=${image_of[$arg]}:${new_tag[$arg]}|" "$f"; then | ||
| rm -f "${f}.bak" |
There was a problem hiding this comment.
Potential .bak leak: sed -i.bak renames the original to ${f}.bak before writing. For UBI_MINIMAL, files has two entries. If sed succeeds on the first file (its .bak is deleted by line 128), then sed fails on the second file (disk full, permissions changed mid-run, etc.), the second .bak is left on disk. die calls exit 1 immediately with no cleanup.
Low probability, but easy to close with a trap:
declare -a _baks=()
trap 'rm -f "${_baks[@]}"' EXIT
# then in the loop:
sed -i.bak "..." "$f" && _baks+=("${f}.bak") && rm -f "${f}.bak"| local files=("$CONTAINERFILE_PATH") | ||
| # UBI_MINIMAL is pinned in both Containerfiles; keep them identical. | ||
| if [ "$arg" = "UBI_MINIMAL" ]; then | ||
| files+=("$WHEELS_CONTAINERFILE_PATH") |
There was a problem hiding this comment.
Optional Style/Consistency note: ${!new_tag[@]} iterates associative array keys in undefined order in bash. The sed calls for each ARG are independent today so the result is correct regardless of order, but this diverges from the plan and report loops which both iterate "${MANAGED_ARGS[@]}" in declaration order. If the apply logic ever gains an ordering dependency (e.g. a second ARG's sed pattern could match the first ARG's line), this becomes a latent bug.
Consistent, order-safe alternative:
for arg in "${MANAGED_ARGS[@]}"; do
[[ -v new_tag[$arg] ]] || continue
...
Signed-off-by: Jonathan Springer <jps@s390x.com>
Signed-off-by: Jonathan Springer <jps@s390x.com>
Remove the hard-coded ARG-to-repository map (repo_for_arg) that could drift from the actual image path in the Containerfile; the Pyxis repository is now derived from each pin's image reference, with a fail-fast guard for pins outside registry.access.redhat.com. Replace the macOS-only 'sed -i ""' in the bats test with the portable sed -i.bak pattern so the suite runs on GNU sed (Ubuntu CI). Add regression tests for the derived-repository lookup and the foreign-registry refusal. Signed-off-by: Jonathan Springer <jps@s390x.com>
3d44aa2 to
07983eb
Compare
Install an EXIT trap removing Containerfile .bak files so a sed failure mid-apply (die path) cannot strand backup files on disk; the redundant per-file rm in the loop is folded into the trap. Iterate the apply phase over MANAGED_ARGS in declaration order instead of undefined associative-array key order, matching the plan and report loops and removing a latent ordering hazard. Add a regression test asserting no .bak files remain after an update. Signed-off-by: Jonathan Springer <jps@s390x.com>
07983eb to
15eb823
Compare
2 participants
What
Adds
scripts/container-bump-image-versions.sh, exposed asmake container-bump-image-versions, which bumps the pinned Red Hat UBI image tags in the repo's Containerfiles to the latest build tag within their current minor line.Why
The base-image pins (
UBI_BASE,NODEJS_IMAGE,UBI_MINIMAL) are full build tags (<minor>-<epoch-buildid>) chosen for reproducibility, but nothing keeps them current — they drift until someone checks by hand. No Dependabot/Renovate config exists in the repo today, and Renovate would treat the twoUBI_MINIMALpin sites as independent updates, whereas they must change atomically.How it works
catalog.redhat.com/api/containers/v1/...);registry.access.redhat.comallows anonymous access, unlikeregistry.redhat.io.<minor>-<epoch-buildid>, so "latest" is the numeric max of the epoch component, restricted to each pin's current minor line. Bumping minors (e.g. 10.2 → 10.3) stays a deliberate, reviewed change.UBI_MINIMALis pinned in bothContainerfileandinfra/wheels/Containerfile; both are always updated together.ENABLE_FIPS=truebuild path overrides these ARGs with UBI 9 images at build time and is intentionally not managed.Testing
tests/scripts/container-bump-image-versions.bats(11 tests: tag filtering, atomic two-file update, no-op idempotence, minor-line policy, zero-write failure paths, non-pinned-tag refusal), run via the newmake batstarget, which discovers alltests/**/*.bats.sort_by=last_update_date[desc]query parameter.Notes for reviewers
brew install bats-core);make batsfails gracefully with an install hint when absent.make testor CI — worth deciding in review whether it should be..secrets.baselineline-number drift in the Makefile commit is the detect-secrets hook's own regeneration.