fix(release): compare host-less image refs on digest alone - #3908
Open
myasnikovdaniil wants to merge 2 commits into
Open
fix(release): compare host-less image refs on digest alone#3908myasnikovdaniil wants to merge 2 commits into
myasnikovdaniil wants to merge 2 commits into
Conversation
hack/verify-promoted-packages.sh reported "promotion changed the
container repository/digest set" for a v1.6.2 promotion that moved no
container bytes at all: all 48 real repo@digest refs and all 48 digests
were byte-identical between the rc artifact and the promotion candidate.
The six entries it flagged as changed repositories were not
repositories. They were bare tag strings -- v1.6.2-rc.1 on the rc side,
v1.6.2 on the candidate side -- pairing one-to-one by identical digest.
hack/lib/image-refs.sh emits two entries per shape-3 split map: the
correctly joined repository@digest, plus shape 1's recursive scrape of
the raw .tag scalar, which carries no repository. The library documents
that degeneracy and leaves host-less refs to its callers;
promote-retag.sh drops them, the verifier had only a
*/cozystack-packages exclusion. So `${image%:*}` -- a no-op on a string
holding no colon -- handed the tag over as the repository, and promotion
rewrites exactly those tags.
Set an empty repository for a ref with no `/` ahead of its digest, so it
compares on the digest, which is the whole container identity such a ref
carries and the only part promotion must not move. Dropping these refs
instead would be wrong rather than blunter: packages/system/kuberture
carries an `image:` map with a `tag:` and no `repository:`, so shape 1
is the only rule that sees its digest, and skipping it would silently
stop proving that digest unchanged.
Reproduced over the release-1.6 packages tree with the shape-3 tags
rewritten rc-to-stable: before, the six pairs above diverge; after, both
sides normalize to 60 identical entries, kuberture's digest included and
kube-ovn's real repository still visible through its shape-4 join.
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
(cherry picked from commit 4fcc111)
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
No fixture in verify-promoted-packages_test.bats built a shape-3 split map -- `repository:` on one key, `tag: <tag>@sha256:<digest>` on another -- which hack/lib/image-refs.sh itself calls the dominant shape in the tree. The existing fixture's single-string ref carries a registry host, so its rc-to-stable rewrite normalizes through the other branch and passes whether or not host-less refs are handled, which is why the suite looked covered while a clean promotion could still be rejected. Add two cases. The first promotes a split map whose tag moves from rc to stable with the repository and digest held constant; reverting the normalization fix reddens it on "promotion changed the container repository/digest set", diffing v9.9.9-rc.3@sha256:eee... against v9.9.9@sha256:eee... -- the same tag-as-repository pair, with identical digests, that CI reported. The second pins the property a blunter fix would break. A digest-only `image:` map with no `repository:` (the kuberture shape) is seen by no rule but shape 1, so it must still contribute its digest: the case moves that digest and nothing else, and asserts the rejection comes from the digest set rather than a neighbouring leg. Replacing the fix with an outright `continue` over host-less refs makes the verifier exit 0 on that changed digest and reddens the case at its exit-status assertion. Negative assertions are counted rather than `!`-negated: under hack/cozytest.sh a `! grep -q` cannot fail, so it would be a comment shaped like an assertion. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> (cherry picked from commit 2cb8c74) Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
myasnikovdaniil
requested review from
Andrei Kvapil (kvaps),
Aleksei Sviridkin (lexfrei) and
Timofei Larkin (lllamnyp)
as code owners
August 19, 2026 08:29
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe verification script now compares host-less image references by digest without deriving repositories from tags. Tests cover accepted RC-to-stable tag changes and rejected digest changes in repository-less image maps. ChangesPromoted package verification
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This PR makes a localized correction to image-reference verification and adds coverage for the affected cases; no actionable merge-blocking risk remains after normal checks and review. Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ports #3905 to
main. Same fix, same tests, two cherry-picks, no conflicts.main's copy ofhack/verify-promoted-packages.shwas byte-identical to the buggy one, so the next promotion cut from here fails the way v1.6.2 just did:normalized_refs()gives a host-less ref a repository equal to its own tag, becauserepo="${image%:*}"is a no-op on a baretag@sha256:...scalar with neither/nor:in it.hack/lib/image-refs.shemits one of those per shape-3 split map, promotion rewrites exactly those tags, and the check then reports changed repositories at identical digests. It cost several rounds on v1.6.2 before the cause was clear, so it is worth not paying twice.Checked against
main's own tree rather than assumed from the 1.6 port. 13 host-less refs, same count asrelease-1.6: 12 pair one-to-one with a host-bearing ref at an identical digest, and the 13th ispackages/system/kuberture/values.yaml(tag:only, norepository:, noregistry:), which is genuinely digest-only and is the case the comment says must not be dropped.The narrowing the 1.6 review left open still does not exist here. Of 14 repository-bearing digest maps, exactly one has a slash-free
repository:— kube-ovn'sglobal.images.kubeovn— andglobal.registry.addressgives it a shape-4 join that keeps its repository in the compared set. No map anywhere has an empty or non-stringregistry:sibling.hack/lib/image-refs.shdiffers fromrelease-1.6's in comments only: 47 code lines either side and an empty diff once comments are stripped, same globs and same selectors, so the refs reaching the verifier are unchanged. Not modified here.Red phase checked both directions rather than inherited. Reverting the one line reddens the new split-map case on the digest-set assertion. Replacing
repo=""with an outrightcontinuefor host-less refs flips which case fails: the split-map one goes green andrejects a changed digest carried only by a repository-less image mapreddens at its exit-status assertion, because the verifier then exits 0 on a genuinely changed digest. That second direction is the one worth having a test for, since dropping host-less refs is the obvious-looking fix.Gate: the four promote bats suites individually green,
shellcheck -x --shell=shclean,pre-commitgreen. No Go, charts or generated files, so build, vet, test andmake generatedo not apply.Summary by CodeRabbit
Bug Fixes
Tests