fix: preserve sub-manifest digests during container cleanup (#254) - #257
Merged
david-waltermire merged 3 commits intoApr 23, 2026
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughBuild workflow now runs for pushed tags matching "v*". Container cleanup is parameterized (REGISTRY, OWNER, IMAGE_NAME) and computes protected sub-manifest digests from GHCR ( Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Actions as GitHub Actions
participant GHCR as GHCR API
participant Retention as Container Retention Action
Actions->>GHCR: List tags matching `latest` and `v*`
GHCR-->>Actions: Return tag list
Actions->>GHCR: Fetch each tag's manifest (Accept: application/vnd...+json)
GHCR-->>Actions: Return manifest JSONs (include .manifests[].digest)
Actions->>Actions: Extract & dedupe digests → emit `skip-shas`
Actions->>Retention: Invoke retention action with `skip-shas`
Retention-->>Actions: Perform cleanup, skipping provided sub-manifests
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/container-cleanup.yml (1)
36-62: Well-designed protection logic with good edge-case handling.The script correctly handles failure scenarios (missing tags, unavailable manifests) using
|| trueand jq null coalescing. The output clearly logs the protection count for observability.Minor nit: Line 61 has inconsistent quoting for
$tags_file(unquoted) compared to other usages (quoted). Whilemktemppaths don't contain spaces, consistent quoting is good practice.🔧 Optional: consistent variable quoting
- echo "Protecting $count sub-manifest digest(s) across $(wc -l < $tags_file | tr -d ' ') tag(s)" + echo "Protecting $count sub-manifest digest(s) across $(wc -l < "$tags_file" | tr -d ' ') tag(s)"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/container-cleanup.yml around lines 36 - 62, The unquoted file redirection using $tags_file in the echo that prints the protection count is inconsistent; update uses of the tags_file variable in command substitutions/redirections to be quoted (e.g., change occurrences like $(wc -l < $tags_file | tr -d ' ') to $(wc -l < "$tags_file" | tr -d ' ')) so "$tags_file" is quoted everywhere (notably where the "Protecting $count ..." message is constructed) to follow the quoting used for sort -u "$tags_file" -o "$tags_file".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/container-cleanup.yml:
- Around line 36-62: The unquoted file redirection using $tags_file in the echo
that prints the protection count is inconsistent; update uses of the tags_file
variable in command substitutions/redirections to be quoted (e.g., change
occurrences like $(wc -l < $tags_file | tr -d ' ') to $(wc -l < "$tags_file" |
tr -d ' ')) so "$tags_file" is quoted everywhere (notably where the "Protecting
$count ..." message is constructed) to follow the quoting used for sort -u
"$tags_file" -o "$tags_file".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d53bc429-7070-4ee1-b4cb-36fe4a909a35
📒 Files selected for processing (3)
.github/workflows/build.yml.github/workflows/container-cleanup.yml.github/workflows/container.yml
…ma-framework#254) Pulls of ghcr.io/metaschema-framework/oscal-cli:latest fail with "manifest ... not found". The daily snok/container-retention-policy run uses `image-tags: "!latest !v*"`, which keeps the tagged OCI index but does not recognise the index's untagged per-platform and attestation sub-manifests as linked. Those sub-manifests age out on the 7-day cut-off and get deleted, leaving the index pointing at nothing. Two changes: * Add a pre-step that lists the currently-tagged `latest` and `v*` images, fetches each index manifest from GHCR, and passes every referenced sub-manifest digest into `skip-shas` — the workaround documented in snok/container-retention-policy's README. * Move `latest` from "applied on every main push" to `latest=auto` and add `tags: v*` to build.yml's push triggers. `latest` now tracks the highest semver tag instead of the tip of main, which matches the pattern the retention policy protects and keeps it immutable between releases. The currently broken `:latest` in GHCR will be replaced on the next tagged release build. Closes metaschema-framework#254
david-waltermire
force-pushed
the
fix/issue-254-container-cleanup
branch
from
April 23, 2026 06:53
db7dc5c to
e792bd6
Compare
Replace type=ref,event=tag (which kept the v-prefix literally) with type=semver patterns. Pushing v3.2.0 now yields container tags 3.2.0, 3.2, and 3, alongside the existing latest (from latest=auto) and the commit-SHA tag. This matches the conventional 'majors/minors/patch' tag set consumers expect.
david-waltermire
merged commit Apr 23, 2026
fb13883
into
metaschema-framework:develop
6 checks passed
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.
Closes #254.
Summary
docker/finch pull ghcr.io/metaschema-framework/oscal-cli:latestfails withFATA[0000] failed to copy: ... content at .../manifests/sha256:... not found. Verified against the current registry state: the:latestindex (digestsha256:f304f679…) is intact, but all four of its referenced sub-manifests (amd64 image, arm64 image, and the two attestation manifests) return 404.Root cause
.github/workflows/container-cleanup.ymlruns daily withsnok/container-retention-policy, filterimage-tags: "!latest !v*". That filter keeps images taggedlatestorv*, but GHCR's packages API reports each multi-platform push as::latest), andThe action cannot infer which untagged versions are "children" of a protected index — this limitation is called out explicitly in the action's README. The untagged children match the
!latest !v*deletion filter, age past the 7-day cut-off, and are pruned, leaving the tagged index dangling.As a secondary issue,
:latestwas being set on every push tomain(flavor: latest=${{ github.ref == 'refs/heads/main' }}). That means:latestcould jump to a fresh image at any time, and the previous:latest's sub-manifests — once the tag moved off them — were immediately in the delete pool.Fix
Two changes, applied together:
container-cleanup.yml— add a pre-step that enumerates every version currently taggedlatestorv*via the GHCR packages API, fetches each index manifest from the OCI registry, extracts every referenced sub-manifest digest, and passes the deduped list throughskip-shas. This is the mitigation the action's own docs recommend.container.yml+build.yml— switch thelatesttag from "applied on every main push" toflavor: latest=auto, and addtags: v*to the build workflow's push triggers.latest=autopublishes:lateston tagged-release pushes only, so the tag tracks an immutable release instead of the tip of main. This also aligns:latestwith the existing!latest !v*protection — the tag and the version it points to are both retained.Notes
:latestin GHCR will not be repaired by this PR; the dangling sub-manifests are already gone. It will be replaced on the next tagged release build, which now publishes:latestautomatically.:latestindex references (the same four currently returning 404). Once a fresh:latestis published, those digests will be live and this skip-list will prevent the cleanup from repeating the break.Test plan
Container Image Cleanupwithdry-run: trueviaworkflow_dispatchand confirm the log line reports a non-zero digest count and that no protected digests appear in the deletion plan:latest; pullingghcr.io/metaschema-framework/oscal-cli:latestsucceedsSummary by CodeRabbit