fix(release): make promote-retag digest verification media-type-agnostic - #3435
Conversation
skopeo inspect --format '{{.Digest}}' fails on OCI artifacts: for
cozystack-packages (config.mediaType
application/vnd.cncf.flux.config.v1+json) it prints nothing and exits
nonzero, so the v1.6.0 finalize aborted its post-copy verification right
after both copies had already succeeded, leaving 32 of 43 repos without
stable tags and skipping the cozy-installer publish. --dry-run never
executes this branch, so rehearsals could not catch it.
Compute the digest as sha256 of the raw manifest instead, which is how
registries define it and works for any media type. The helper gates the
hash on skopeo's exit status via a temp file rather than a pipeline:
a missing tag still yields an empty string (pre-check reads that as
"tag absent, proceed to copy"), never the empty-input hash
sha256:e3b0c442..., and never an abort under set -eu.
Verified against live GHCR: the raw-manifest hash of
cozystack-packages:v1.6.0 matches the digest finalize expected
(bf68208730860fa8...), and agrees byte-for-byte with the old method on
regular multi-arch images. New cozytest cases cover the OCI-artifact
digest, a post-copy mismatch, and the missing-tag path.
Assisted-By: GPT-5 <noreply@openai.com>
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
📝 WalkthroughWalkthroughThe retagging script now hashes raw OCI manifests with ChangesRaw manifest digest retagging
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant promote-retag.sh
participant skopeo
participant sha256sum
promote-retag.sh->>skopeo: Inspect stable tag raw manifest
skopeo-->>promote-retag.sh: Return raw bytes or registry error
promote-retag.sh->>sha256sum: Hash non-empty manifest bytes
sha256sum-->>promote-retag.sh: Return manifest digest
promote-retag.sh->>skopeo: Copy RC reference to stable tag
promote-retag.sh->>skopeo: Inspect stable tag raw manifest again
skopeo-->>promote-retag.sh: Return copied manifest bytes
promote-retag.sh->>sha256sum: Hash copied manifest bytes
sha256sum-->>promote-retag.sh: Return resolved digest
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 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 |
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
IvanHunters
left a comment
There was a problem hiding this comment.
LGTM — no blockers.
Empirically verified the core digest claim: sha256 of skopeo inspect --raw bytes matches the registry's Docker-Content-Digest byte-for-byte (checked on alpine:3.19 and on the actual cozystack-packages:v1.6.0 artifact that broke finalize). No re-serialization, multi-arch index handled correctly, genuine mismatch still exits loudly. All 3 new bats tests pass in a hermetic run.
Non-blocking notes:
- hack/promote-retag.sh:86-87 —
_ref/_manifestare not declaredlocal(POSIX sh) and leak to global scope; no collision today, cosmetic. - Test gap: the write-once "refuses to move" branch (:151-154) has no dedicated test — only the idempotent-skip (equal) and missing-tag (empty) cases.
- Defense-in-depth: if skopeo ever exited 0 with empty stdout, the function would return the hash of empty input; a
[ -s "$_manifest" ]guard would close that theoretical gap.
Review notes from #3435, and a correction to my first attempt at them. manifest_digest gated on skopeo`s exit status alone, so a zero exit with no bytes hashed the empty string to sha256:e3b0c442... — a digest that looks real and belongs to no manifest. The first fix simply required non-empty bytes, which was worse than it looked: the caller turns an empty result into "not published, safe to copy", so a registry answering 200 with an empty body went from an accidental refusal (the bogus digest mismatched, so the write-once check refused) to an actual copy over whatever the tag really held. Verified against real skopeo 1.23: a 200 with Content-Length 0 does exit 0 with no stdout. The helper is now explicitly three-state. A digest, or empty output for a tag the registry PROVED absent, or non-zero for anything indeterminate — which under set -e aborts the promotion before it writes. Absence has to be reported as "manifest unknown"; 429, 5xx, auth and network failures all exit non-zero with no bytes too, and reading those as "unpublished" is the same fail-open one level down. finalize retags ~42 refs with no retry wrapper, so a single rate-limit was enough to reach that path. skopeo`s stderr is captured instead of discarded, so the diagnostics now name the real failure. Tests: the write-once refusal branch the review found untested (an existing stable tag at a different digest must be refused before any write), the empty- body case (refuses, writes nothing, and the empty-input hash never appears in the diagnostics), and a 429 that must not read as an absent tag. The mock now emits realistic registry errors, since a bare non-zero exit is no longer proof of absence. Both new guards are mutation-checked: accepting any non-zero exit as absence fails the 429 test, dropping the empty-bytes check fails the empty-body test. 11/11 tests green, hack/image-pin-consistency.bats still 2/2, sh -n clean. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
hack/promote-retag_test.bats (1)
49-54: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
MOCK_MISSING_ONCElacks the${VAR:-0}default used by its sibling flags, risking an "unbound variable" abort.
MOCK_TRANSIENT_ERROR(L45) andMOCK_EMPTY_MANIFEST(L56) both default via${VAR:-0}, butMOCK_MISSING_ONCE(L49) is referenced bare. Since the shim runs underset -eu(L38), any test invocation that forgets to exportMOCK_MISSING_ONCEwill crash the mock with an unbound-variable error instead of a clear test failure. All tests shown here (L300, L322, L354) happen to set it, but this is a footgun for future additions and worth aligning with the other two flags for consistency.🧪 Proposed fix
- if [ "$MOCK_MISSING_ONCE" = "1" ] && [ ! -f "$MOCK_STATE" ]; then + if [ "${MOCK_MISSING_ONCE:-0}" = "1" ] && [ ! -f "$MOCK_STATE" ]; then🤖 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 `@hack/promote-retag_test.bats` around lines 49 - 54, Update the MOCK_MISSING_ONCE check in the mock script to use the same default-value expansion as MOCK_TRANSIENT_ERROR and MOCK_EMPTY_MANIFEST, treating an unset variable as 0 so set -u cannot abort the shim.
🤖 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 `@hack/promote-retag.sh`:
- Around line 114-123: Update the absence check in the manifest inspection flow
to remove the broad bare “not found” and “404” alternatives from the grep
pattern. Retain only the existing manifest-specific messages such as “manifest
unknown,” “manifest not known,” and “manifest for … not found,” so unrelated or
indeterminate errors continue to return failure.
---
Nitpick comments:
In `@hack/promote-retag_test.bats`:
- Around line 49-54: Update the MOCK_MISSING_ONCE check in the mock script to
use the same default-value expansion as MOCK_TRANSIENT_ERROR and
MOCK_EMPTY_MANIFEST, treating an unset variable as 0 so set -u cannot abort the
shim.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 6142263f-0ee3-49e9-a179-9ffe272a75da
📒 Files selected for processing (2)
hack/promote-retag.shhack/promote-retag_test.bats
| # Non-zero. Only a registry that says the manifest is unknown proves absence; | ||
| # 429/5xx/auth/network failures all also exit non-zero with no bytes, and | ||
| # reading those as "not published" is exactly the fail-open this guards. | ||
| if grep -qiE 'manifest unknown|manifest not known|manifest for [^ ]* not found|not found|404' "$_mderr"; then | ||
| rm -f "$_manifest" "$_mderr" | ||
| return 0 | ||
| fi | ||
| echo "::error::skopeo inspect of ${_ref} failed and did not report the manifest as unknown, so it cannot be treated as unpublished: $(tr '\n' ' ' <"$_mderr")" >&2 | ||
| rm -f "$_manifest" "$_mderr" | ||
| return 1 |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
git ls-files | rg '(^|/)hack/promote-retag\.sh$|promote-retag' || true
echo "== relevant function/context =="
if [ -f hack/promote-retag.sh ]; then
wc -l hack/promote-retag.sh
ast-grep outline hack/promote-retag.sh --view expanded || true
sed -n '1,180p' hack/promote-retag.sh
fi
echo "== skopeo inspect missing-manifest docs/samples from repo/test files =="
rg -n "skopeo inspect|manifest unknown|manifest not known|not found|404|_mderr|_manifest" hack scripts . --glob '!vendor/**' --glob '!node_modules/**' | sed -n '1,220p' || true
echo "== behavioral regex probe =="
python3 - <<'PY'
import re
patterns = {
'current': re.compile(r'manifest unknown|manifest not known|manifest for [^ ]* not found|not found|404', re.I),
'proposed': re.compile(r'manifest unknown|manifest not known|manifest for [^ ]* not found|manifest.*(not found|404)|(not found|404).*manifest', re.I),
}
samples = [
"manifest unknown: manifest unknown",
"manifest not known",
"manifest for example/foo/bar:v1 not found",
"skopeo inspect failed: 5xx transient failure",
"429 too many requests",
"http client error: 404 not found",
"manifest not found: generic failure",
"DNS failed: could not resolve host; not found",
"network fail: 404",
]
for s in samples:
print(f"{s!r}: current={bool(patterns['current'].search(s))} proposed={bool(patterns['proposed'].search(s))}")
PYRepository: cozystack/cozystack
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
git ls-files | rg '(^|/)hack/promote-retag\.sh$|promote-retag' || true
echo "== relevant function/context =="
if [ -f hack/promote-retag.sh ]; then
wc -l hack/promote-retag.sh
ast-grep outline hack/promote-retag.sh --view expanded || true
sed -n '1,180p' hack/promote-retag.sh
fi
echo "== skopeo inspect missing-manifest docs/samples from repo/test files =="
rg -n "skopeo inspect|manifest unknown|manifest not known|not found|404|_mderr|_manifest" hack scripts . --glob '!vendor/**' -- glob '!node_modules/**' | sed -n '1,220p' || true
echo "== behavioral regex probe =="
python3 - <<'PY'
import re
patterns = {
'current': re.compile(r'manifest unknown|manifest not known|manifest for [^ ]* not found|not found|404', re.I),
'proposed': re.compile(r'manifest unknown|manifest not known|manifest for [^ ]* not found|manifest.*(not found|404)|(not found|404).*manifest', re.I),
}
samples = [
"manifest unknown: manifest unknown",
"manifest not known",
"manifest for example/foo/bar:v1 not found",
"skopeo inspect failed: 5xx transient failure",
"429 too many requests",
"http client error: 404 not found",
"manifest not found: generic failure",
"DNS failed: could not resolve host; not found",
"network fail: 404",
]
for s in samples:
print(f"{s!r}: current={bool(patterns['current'].search(s))} proposed={bool(patterns['proposed'].search(s))}")
PYRepository: cozystack/cozystack
Length of output: 50375
🌐 Web query:
skopeo inspect --raw manifest unknown missing manifest 404 manifest not found
💡 Result:
When running skopeo inspect --raw, encountering a "manifest unknown" or "404 manifest not found" error typically indicates that the registry cannot locate the specified image, tag, or digest [1][2]. Common causes and troubleshooting steps include: 1. Verification of Image Name and Tag: The most frequent cause is a typo in the image name, repository path, or tag [1]. Verify that the image exists in the target registry and that the tag is correct. If you are using a custom or private registry, ensure your authentication credentials (e.g., --creds or registry login) are valid [3]. 2. Handling Manifest Lists vs. Single Manifests: If the image is a multi-architecture manifest list, running a standard skopeo inspect command (without --raw) may attempt to resolve the image for your host's local architecture/OS [4][5]. If that specific platform combination is missing from the list, it will fail [6][7][4]. Using --raw bypasses this lookup, allowing you to see the manifest list index itself [8][4]. 3. Registry API Compatibility: Some registries may return 404 errors if they do not support standard Docker Registry API calls or if they are configured in a way that differs from standard expectations (e.g., repository path structures or proxy issues) [9][2]. 4. Authentication: Ensure you have sufficient permissions. In some cases, a 404 "Not Found" may actually be a masked 403 "Forbidden" error, where the registry returns 404 to avoid leaking information about the existence of private repositories to unauthorized users [1]. To debug further, you can use the --debug global flag to see the underlying HTTP requests being made, which will confirm whether the 404 is coming from the registry's root path or a specific repository endpoint [9].
Citations:
- 1: feature: Add proper error reporting for
inspectpodman-container-tools/skopeo#2589 - 2: skopeo inspect: a way how to avoid fetching all tags from repository podman-container-tools/skopeo#785
- 3: Skopeo errors pulling images from Azure Container Registry anchore/anchore-engine#18
- 4: no image found in manifest list for architecture amd64, variant "", OS darwin FAIL podman-container-tools/skopeo#1586
- 5: Skopeo inspect: provide --top-level about included architectures podman-container-tools/skopeo#1283
- 6:
inspect --rawfails for manifest list with no amd64 platform podman-container-tools/skopeo#729 - 7: inspect --raw: skip the NewImage() step podman-container-tools/skopeo#708
- 8: Allow reading an architecture-specific manifest from a list with skopeo inspect --raw podman-container-tools/skopeo#740
- 9: skopeo 404 against nexus oss docker registry podman-container-tools/skopeo#1000
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== promote-retag tests around absent/error behavior =="
sed -n '40,70p' hack/promote-retag_test.bats
sed -n '330,370p' hack/promote-retag_test.bats
echo "== regex match behavior for skopeo-like and unrelated stderr samples =="
python3 - <<'PY'
import re
patterns = {
'current': re.compile(r'manifest unknown|manifest not known|manifest for [^ ]* not found|not found|404', re.I),
'proposed': re.compile(r'manifest unknown|manifest not known|manifest for [^ ]* not found|manifest.*(not found|404)|(not found|404).*manifest', re.I),
}
samples = [
"manifest unknown: manifest unknown",
"manifest not known",
"manifest for example/foo/bar:v1 not found",
"manifest not found: generic failure",
"error: manifest 404",
"skopeo inspect failed: 5xx transient failure",
"429 too many requests",
"http client error: 404 not found during skopeo inspect",
"DNS failed: could not resolve host; not found",
"network fail: 404",
"unrelated text not found",
"unrelated text 404",
]
for s in samples:
print(f"{s!r}: current={bool(patterns['current'].search(s))} proposed={bool(patterns['proposed'].search(s))}")
PYRepository: cozystack/cozystack
Length of output: 3600
🌐 Web query:
containers skopeo inspect missing image "manifest unknown" "404" "manifest not found"
💡 Result:
When skopeo inspect returns a "manifest unknown" error or a 404 (Not Found) status, it indicates that the registry could not provide the requested manifest [1][2][3]. While this is often straightforward, the error is sometimes ambiguous due to how registries handle permissions and missing resources [4][3]. Common Causes: 1. Missing Image or Tag: The most frequent cause is that the specified tag does not exist, has been deleted, or the repository name/namespace is incorrect [2][3]. 2. Authentication/Authorization: Some registries return 404 (Not Found) or "manifest unknown" instead of 401 (Unauthorized) or 403 (Forbidden) to avoid leaking information about private repositories [4][2][3]. If you are not properly authenticated, the registry may hide the image's existence [3]. 3. Registry Implementation/API Issues: Some registries (e.g., older versions of Harbor or specific cloud-hosted registries) may return non-standard error codes or fail when queried for tag lists [5][6]. 4. Incorrect Endpoint/Mirror: You may be querying the wrong registry host, or a local mirror/proxy is not synchronized with the upstream registry [2]. 5. Platform Mismatch: If an image is not available for the architecture you are running on, some registries or configurations might fail to return the expected manifest [3]. Troubleshooting Steps: - Verify the Image Reference: Double-check the registry host, namespace, repository name, and tag [2][3]. - Check Authentication: Ensure you are logged in to the registry with appropriate credentials [3]. - Use skopeo list-tags: Run skopeo list-tags docker://<image-path> to confirm if the tag exists and if your credentials allow you to access the repository [3]. - Try --no-tags: If the error is occurring during a tag listing operation (e.g., on some cloud ECR setups), using skopeo inspect --no-tags may work around the issue [6]. - Debug with Verbose Output: Run the command with the --debug flag (e.g., skopeo --debug inspect ...) to see the exact HTTP requests and responses [1][5][7]. This helps identify if the 404 is coming from the registry root or a specific path [7]. Note that there is no universal way to distinguish between a missing image and a permission error, as registries have different behaviors regarding what they reveal when access is denied [4][8].
Citations:
- 1: Skopeo copy does not allow to pull the destination images afterwards podman-container-tools/skopeo#384
- 2: https://cr0x.net/en/docker-manifest-unknown-tags-digests/
- 3: https://thelinuxcode.com/how-to-fix-the-docker-manifest-unknown-manifest-unknown-error-a-practical-2026-playbook/
- 4: main: return different exit code when an input is not found podman-container-tools/skopeo#2236
- 5: isManifestUnknownError fails against Harbor registries, breaking sigstore signature upload containers/image#2203
- 6: skopeo inspect does not work with public.ecr.aws: listing tags fails with 404 (Not Found) podman-container-tools/skopeo#1230
- 7: skopeo 404 against nexus oss docker registry podman-container-tools/skopeo#1000
- 8: feature: Add proper error reporting for
inspectpodman-container-tools/skopeo#2589
Scope the manifest-absence fallback to manifest-specific messages.
The current regex lets stderr containing bare not found or 404 pass as proof that the tag is absent, while empty/erroneous errors are documented as indeterminate. Require manifest context unless this script intentionally should accept generic 404s; otherwise use the existing manifest-aware alternatives.
🤖 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 `@hack/promote-retag.sh` around lines 114 - 123, Update the absence check in
the manifest inspection flow to remove the broad bare “not found” and “404”
alternatives from the grep pattern. Retain only the existing manifest-specific
messages such as “manifest unknown,” “manifest not known,” and “manifest for …
not found,” so unrelated or indeterminate errors continue to return failure.
|
Successfully created backport PR for |
What this PR does
The finalize post-copy verification runs
skopeo inspect --format '{{.Digest}}'on every freshly retagged stable ref. For OCI artifacts that command prints nothing and exits non-zero —cozystack-packagesis a flux artifact withconfig.mediaType: application/vnd.cncf.flux.config.v1+json— so the v1.6.0 finalize (run 29932878820) aborted right after both copies of that artifact had already succeeded, leaving 32 of 43 repos without stable tags and skipping the cozy-installer publish.--dry-runnever reaches this branch, which is why rehearsals did not catch it.This PR computes the digest as sha256 over the raw manifest instead (
skopeo inspect --raw), which is how registries define the manifest digest and works identically for container images and OCI artifacts. The helper gates the hash on skopeo's exit status via a temp file rather than a pipeline, so a missing tag still resolves to an empty string (the pre-check reads that as "tag absent, proceed to copy") and never to the empty-input hashsha256:e3b0c442…, and never aborts underset -eu.Verified against live GHCR: the raw-manifest hash of
cozystack-packages:v1.6.0matches the digest finalize expected (bf68208730860fa8…), and the old and new methods agree byte-for-byte on regular multi-arch images (cozystack-operator,kubeovn,cozystack-controller,kubevirt-cloud-provider). New cozytest cases cover the OCI-artifact digest resolution, a post-copy mismatch, and the missing-tag path; the mock skopeo asserts--raw, so the suite fails against the old code.Release note
Summary by CodeRabbit