fix(release): compare host-less image refs on digest alone - #3905
Merged
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>
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>
Contributor
|
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: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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.
Fixes the false
promotion changed the container repository/digest setthat blocks #3904.normalized_refs()inhack/verify-promoted-packages.shgives a host-less ref a repository equal to its own tag.hack/lib/image-refs.shemits two entries per shape-3 map, the correctly joined ref plus shape 1's recursive scrape of the baretag: <tag>@sha256:...scalar, and on that bare scalarrepo="${image%:*}"is a no-op because there is no/and no:in it. Promotion rewrites exactly those tags, sov1.6.2-rc.1tov1.6.2shows up as six changed repositories whose digests are identical on both sides.promote-retag.shnever hits this because it drops host-less refs through its ownership filter, the verifier had no equivalent.Host-less refs now compare on digest alone, which is the whole of the container identity such a ref carries. Checked against the real published artifacts: before the fix 61 entries each side differing by those six pairs, after it 60 each side with the sets equal and 48 container digests identical.
Two things this deliberately does not do, both written as comments at the fix. Not dropping host-less refs outright, because
packages/system/kuberture/values.yamlcarries animage:map with atag:and norepository:, so shape 1 is the only rule that ever sees its digest and a drop would silently stop proving it unchanged. And not reusingpromote-retag.sh's${REGISTRY}/ownership filter, because the verify job'sREGISTRYnames the private build registry while both artifacts under comparison live on the public one, so that filter drops all 48 refs, and the emptiness guard above tests the raw collection rather than the filtered set, so the check would pass by comparing two empty sets.Second commit adds the fixture that was missing. The existing suite writes a host-bearing string and exercises a passing rc to stable rewrite, so it read as coverage while never building a shape-3 split map, which the library itself calls the dominant shape. Reverting the fix reddens the new case on the digest-set assertion, and replacing the fix with an outright skip of host-less refs makes the verifier exit 0 on a genuinely changed digest.
Once this is on
release-1.6the check on #3904 re-runs against this copy, so norc.2re-cut is needed.maincarries the same script and wants the same fix.