fix(release): compare host-less image refs on digest alone - #3905
Merged
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 <[email protected]>
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 <[email protected]>
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 |
11 tasks
myasnikovdaniil
added a commit
that referenced
this pull request
Sep 5, 2026
Ports #3905 to `main`. Same fix, same tests, two cherry-picks, no conflicts. `main`'s copy of `hack/verify-promoted-packages.sh` was 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, because `repo="${image%:*}"` is a no-op on a bare `tag@sha256:...` scalar with neither `/` nor `:` in it. `hack/lib/image-refs.sh` emits 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 as `release-1.6`: 12 pair one-to-one with a host-bearing ref at an identical digest, and the 13th is `packages/system/kuberture/values.yaml` (`tag:` only, no `repository:`, no `registry:`), 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's `global.images.kubeovn` — and `global.registry.address` gives it a shape-4 join that keeps its repository in the compared set. No map anywhere has an empty or non-string `registry:` sibling. `hack/lib/image-refs.sh` differs from `release-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 outright `continue` for host-less refs flips which case fails: the split-map one goes green and `rejects a changed digest carried only by a repository-less image map` reddens 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=sh` clean, `pre-commit` green. No Go, charts or generated files, so build, vet, test and `make generate` do not apply. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved promoted package validation for image references without explicit registries or repositories. * Promotion-only tag changes are now accepted when the image digest remains unchanged. * Digest changes continue to be detected and rejected correctly. * **Tests** * Added coverage for split image reference formats and repository-less image mappings. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
myasnikovdaniil
added a commit
that referenced
this pull request
Sep 24, 2026
…VM disks (#4431) Backport of #3455 to `release-1.6`. Management etcd on this branch stalls under the VM-heavy suites the same way main did before #3455: every operator loses its lease in the same few seconds, and install failed on it in #3550 and #3905. Second commit widens tenant node-join deadline from 12m to 18m. Most kubernetes suite failures on 1.6 are workers that get their CSR signed 6-9 minutes after VMI start and miss the 12m window, the script step already has 40m so the wider wait fits. Both commits cherry-picked clean with `-x`. ### Testing - `make unit-tests` green, POSIX sh sweep clean. - Real check is E2E on this PR. ```release-note NONE ```
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
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.