Repository navigation
Fix platform publishing by preserving child manifest digests - #412
Merged
Merged
Conversation
Signed-off-by: Ryan Cook <rcook@redhat.com>
Reviewer's GuideThe PR fixes platform publishing by pushing only the index, preserving the digests of child manifests already present in the destination repository. It adds a native local-registry regression that forces conflicting compression settings and validates successful pulls for both architectures. Sequence diagram for preserving child manifest digests during platform publishingsequenceDiagram
participant Publisher
participant Podman
participant Registry
participant Verification
Publisher->>Podman: manifest create
Publisher->>Podman: manifest add amd_digest
Publisher->>Podman: manifest add arm_digest
Publisher->>Podman: manifest push --all=false
Podman->>Registry: Publish index only
Registry-->>Podman: Preserve existing child manifests
Publisher->>Podman: manifest inspect
Podman->>Registry: Read published index
Registry-->>Verification: Return index
Verification->>Verification: Compare platform digests
Flow diagram for the platform publishing regression testflowchart LR
A[Publish architecture images with gzip setting A] --> R[Local registry]
B[Publish architecture images with gzip setting B] --> R
R --> I[Publish platform index only]
I --> V[Verify exact child manifest digests]
V --> P[Pull both platforms]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Signed-off-by: Ryan Cook <rcook@redhat.com>
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 13 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="scripts/publish-platform-index.sh" line_range="49" />
<code_context>
+# Child manifests and blobs already exist in this repository. Copying them again
+# can recompress layers and replace the exact digests pinned above. Publish only
+# the index; keep the registry verification below strict.
+podman manifest push --all=false "${options[@]}" --format=docker "$manifest" "docker://$destination"
podman manifest inspect "${options[@]}" "$destination" > "$scratch/index.json"
python3 - "$scratch/index.json" "$amd_digest" "$arm_digest" <<'PYTHON'
</code_context>
<issue_to_address>
**Platform pulls fail after publishing**
When child manifests are pushed only to the architecture-specific repositories and are absent from the destination repository, `podman manifest push --all=false` publishes only the index to the destination, while the child manifests remain in the sibling repositories. `manifest inspect` checks index descriptors, not child availability, so publishing succeeds but clients cannot fetch either platform.
Ensure both child manifests are available in the destination repository before publishing the index, and verify their availability there.
Also at `scripts/publish-platform-index.sh:50`.
</issue_to_address>
### Comment 2
<location path="scripts/publish-platform-index.sh" line_range="49" />
<code_context>
+# Child manifests and blobs already exist in this repository. Copying them again
+# can recompress layers and replace the exact digests pinned above. Publish only
+# the index; keep the registry verification below strict.
+podman manifest push --all=false "${options[@]}" --format=docker "$manifest" "docker://$destination"
podman manifest inspect "${options[@]}" "$destination" > "$scratch/index.json"
python3 - "$scratch/index.json" "$amd_digest" "$arm_digest" <<'PYTHON'
</code_context>
<issue_to_address>
**issue (review_instructions):** Index-only publishing can create a broken destination if either child manifest is missing or differs from the pinned digest.
The existing-repository assumption is not verified before publishing. Add a preflight check that fetches both child manifests from the destination repository and compares their immutable digests, failing closed if either is absent or mismatched; otherwise retain `--all` as a fallback when the children are not already present. For example: `podman manifest inspect "${options[@]}" "docker://$repository@$amd_digest" >/dev/null || { echo "missing AMD child manifest" >&2; exit 1; }` (and likewise for ARM). This prevents publishing an index that references unavailable or unexpected content.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `*`
**Instructions:**
Use comments that enhance the reliability and stability of the code by recommending best practices, and provide a safer alternative for potential security or integrity risks.
[Edit this rule](https://app.sourcery.ai/accounts/150647/repo-settings/455664976?activeTab=review-rules&rule=77cf81e0-aaed-48cb-bc7b-43cb0ecdb17b&from_surface=github_bot&intent=review_rules)
</details>
</issue_to_address>
### Comment 3
<location path="scripts/publish-platform-index.sh" line_range="49" />
<code_context>
+# Child manifests and blobs already exist in this repository. Copying them again
+# can recompress layers and replace the exact digests pinned above. Publish only
+# the index; keep the registry verification below strict.
+podman manifest push --all=false "${options[@]}" --format=docker "$manifest" "docker://$destination"
podman manifest inspect "${options[@]}" "$destination" > "$scratch/index.json"
python3 - "$scratch/index.json" "$amd_digest" "$arm_digest" <<'PYTHON'
</code_context>
<issue_to_address>
**suggestion (review_instructions):** Add an integration test covering index-only publishing and digest preservation.
Please test the edge case where the referenced child manifests already exist with pinned digests, verifying that this command does not repush or recompress them. Also cover a failed or missing child-manifest reference to ensure registry verification fails rather than publishing an incorrect index.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `*`
**Instructions:**
Include comments that suggest adding tests for edge cases or invalid inputs to improve code robustness.
[Edit this rule](https://app.sourcery.ai/accounts/150647/repo-settings/455664976?activeTab=review-rules&rule=99de523b-cf4c-499c-848e-2830fb0292eb&from_surface=github_bot&intent=review_rules)
</details>
</issue_to_address>
### Comment 4
<location path="scripts/publish-platform-index.sh" line_range="49" />
<code_context>
+# Child manifests and blobs already exist in this repository. Copying them again
+# can recompress layers and replace the exact digests pinned above. Publish only
+# the index; keep the registry verification below strict.
+podman manifest push --all=false "${options[@]}" --format=docker "$manifest" "docker://$destination"
podman manifest inspect "${options[@]}" "$destination" > "$scratch/index.json"
python3 - "$scratch/index.json" "$amd_digest" "$arm_digest" <<'PYTHON'
</code_context>
<issue_to_address>
**suggestion (review_instructions):** Omit the redundant `--all=false` flag to simplify the publish command while retaining index-only behavior.
`--all` defaults to false for `podman manifest push`, so this is equivalent and avoids an unnecessary explicit option:
```sh
podman manifest push "${options[@]}" --format=docker "$manifest" "docker://$destination"
```
<details>
<summary>Review instructions:</summary>
**Path patterns:** `*`
**Instructions:**
Verify that the suggestion simplifies the code logic or enhances performance.
[Edit this rule](https://app.sourcery.ai/accounts/150647/repo-settings/455664976?activeTab=review-rules&rule=00e6741a-525d-4304-9e65-8fbc8e563bae&from_surface=github_bot&intent=review_rules)
</details>
</issue_to_address>
### Comment 5
<location path="scripts/publish-platform-index.sh" line_range="49" />
<code_context>
+# Child manifests and blobs already exist in this repository. Copying them again
+# can recompress layers and replace the exact digests pinned above. Publish only
+# the index; keep the registry verification below strict.
+podman manifest push --all=false "${options[@]}" --format=docker "$manifest" "docker://$destination"
podman manifest inspect "${options[@]}" "$destination" > "$scratch/index.json"
python3 - "$scratch/index.json" "$amd_digest" "$arm_digest" <<'PYTHON'
</code_context>
<issue_to_address>
**issue (review_instructions):** Pushing only the index can create a dangling multi-architecture manifest when child digests are not already present at the destination.
This assumes that `$amd_digest` and `$arm_digest` exist in the repository addressed by `$destination`. If `repository` and `destination` differ, or if either child manifest has not been copied there, `--all=false` publishes an index referencing unavailable manifests; clients pulling the destination tag will then fail to resolve the AMD or ARM image. The script should verify that the child manifests exist at the destination or retain `--all` when copying across repositories.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `*`
**Instructions:**
Include comments that identify potential bugs or risks in the code and clearly explain why the code is problematic, using specific examples.
[Edit this rule](https://app.sourcery.ai/accounts/150647/repo-settings/455664976?activeTab=review-rules&rule=8a7aba75-59c7-4fb6-99cc-848bb5f45de2&from_surface=github_bot&intent=review_rules)
</details>
</issue_to_address>
### Comment 6
<location path="scripts/publish-platform-index.sh" line_range="48" />
<code_context>
-podman manifest push --all "${options[@]}" --format=docker "$manifest" "docker://$destination"
+# Child manifests and blobs already exist in this repository. Copying them again
+# can recompress layers and replace the exact digests pinned above. Publish only
+# the index; keep the registry verification below strict.
+podman manifest push --all=false "${options[@]}" --format=docker "$manifest" "docker://$destination"
podman manifest inspect "${options[@]}" "$destination" > "$scratch/index.json"
</code_context>
<issue_to_address>
**suggestion (review_instructions):** Add a concrete example showing the intended index-only push behavior.
For example, rephrase this as: “Push only the manifest index with `--all=false`; do not copy the child manifests or blobs, whose pinned digests must remain unchanged.” This makes the suggested behavior explicit and actionable.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `*`
**Instructions:**
Check that the comment includes a specific example or rephrasing to illustrate the suggestion.
[Edit this rule](https://app.sourcery.ai/accounts/150647/repo-settings/455664976?activeTab=review-rules&rule=4ea295bd-cb2d-4d56-9aa4-6a934ebb653d&from_surface=github_bot&intent=review_rules)
</details>
</issue_to_address>
### Comment 7
<location path="scripts/publish-platform-index.sh" line_range="46" />
<code_context>
podman manifest add "${options[@]}" "$manifest" "docker://$repository-amd@$amd_digest"
podman manifest add "${options[@]}" "$manifest" "docker://$repository-arm@$arm_digest"
-podman manifest push --all "${options[@]}" --format=docker "$manifest" "docker://$destination"
+# Child manifests and blobs already exist in this repository. Copying them again
+# can recompress layers and replace the exact digests pinned above. Publish only
+# the index; keep the registry verification below strict.
</code_context>
<issue_to_address>
**issue (review_instructions):** This comment explains the behavior but does not provide a concrete organizational or structural suggestion.
Please make the guidance explicit—for example, state that this command should publish only the manifest index because child manifests are managed separately, and consider documenting that contract alongside the publishing test in `tests/integration/publishing/test-platform-publish.sh` for maintainability.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `*`
**Instructions:**
Ensure the comment provides a clear suggestion for improving the organization or structure of the code or documentation.
[Edit this rule](https://app.sourcery.ai/accounts/150647/repo-settings/455664976?activeTab=review-rules&rule=5823e292-3e9c-49a0-ad36-4eb208fb9cbd&from_surface=github_bot&intent=review_rules)
</details>
</issue_to_address>
### Comment 8
<location path="scripts/publish-platform-index.sh" line_range="49" />
<code_context>
+# Child manifests and blobs already exist in this repository. Copying them again
+# can recompress layers and replace the exact digests pinned above. Publish only
+# the index; keep the registry verification below strict.
+podman manifest push --all=false "${options[@]}" --format=docker "$manifest" "docker://$destination"
podman manifest inspect "${options[@]}" "$destination" > "$scratch/index.json"
python3 - "$scratch/index.json" "$amd_digest" "$arm_digest" <<'PYTHON'
</code_context>
<issue_to_address>
**issue (review_instructions):** Preflight the referenced child manifests before publishing an index-only manifest.
This new mode assumes both child manifests and their blobs already exist at the destination; otherwise it can publish an index containing unavailable digests. Before this command, verify each child digest in the destination repository and fail with a clear error, for example: `podman manifest inspect "${options[@]}" "docker://$destination@$amd_digest" >/dev/null || { echo "missing AMD child manifest: $amd_digest" >&2; exit 1; }` (and repeat for `$arm_digest`).
<details>
<summary>Review instructions:</summary>
**Path patterns:** `*`
**Instructions:**
The comment should address potential issues such as error handling or code robustness, and include specific code examples or modifications to illustrate the suggestion.
[Edit this rule](https://app.sourcery.ai/accounts/150647/repo-settings/455664976?activeTab=review-rules&rule=5704ccf1-6084-47b5-9fbd-06d8a0441c56&from_surface=github_bot&intent=review_rules)
</details>
</issue_to_address>
### Comment 9
<location path="scripts/publish-platform-index.sh" line_range="49" />
<code_context>
+# Child manifests and blobs already exist in this repository. Copying them again
+# can recompress layers and replace the exact digests pinned above. Publish only
+# the index; keep the registry verification below strict.
+podman manifest push --all=false "${options[@]}" --format=docker "$manifest" "docker://$destination"
podman manifest inspect "${options[@]}" "$destination" > "$scratch/index.json"
python3 - "$scratch/index.json" "$amd_digest" "$arm_digest" <<'PYTHON'
</code_context>
<issue_to_address>
**issue (review_instructions):** Publishing only the index may leave its child manifests unavailable in the destination repository.
The manifest entries are sourced from `docker://$repository-amd` and `docker://$repository-arm`, while this command pushes only the index to `$destination`. Unless those child manifests have already been copied to the destination repository under the referenced digests, pulls of the published index can fail. Please verify that assumption or retain child-manifest copying where necessary.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `*`
**Instructions:**
Include comments that identify potential bugs or risks in the code, such as duplicate function definitions or mismatched interpreters.
[Edit this rule](https://app.sourcery.ai/accounts/150647/repo-settings/455664976?activeTab=review-rules&rule=2a76282a-9a4e-4383-93f6-88481d98da0c&from_surface=github_bot&intent=review_rules)
</details>
</issue_to_address>
### Comment 10
<location path="scripts/publish-platform-index.sh" line_range="49" />
<code_context>
+# Child manifests and blobs already exist in this repository. Copying them again
+# can recompress layers and replace the exact digests pinned above. Publish only
+# the index; keep the registry verification below strict.
+podman manifest push --all=false "${options[@]}" --format=docker "$manifest" "docker://$destination"
podman manifest inspect "${options[@]}" "$destination" > "$scratch/index.json"
python3 - "$scratch/index.json" "$amd_digest" "$arm_digest" <<'PYTHON'
</code_context>
<issue_to_address>
**suggestion (review_instructions):** Handle push failures explicitly so a failed index publication cannot proceed to registry verification as if it succeeded.
Because this is a security-sensitive publication step, fail closed and emit actionable diagnostics. For example: `if ! podman manifest push --all=false "${options[@]}" --format=docker "$manifest" "docker://$destination"; then echo "failed to publish platform index to $destination" >&2; exit 1; fi`. If the script already uses `set -e`, document that assumption near this command instead.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `*`
**Instructions:**
Ensure the comment suggests handling exceptions appropriately if applicable.
[Edit this rule](https://app.sourcery.ai/accounts/150647/repo-settings/455664976?activeTab=review-rules&rule=10c7919b-ce44-454b-aece-6a73c8875795&from_surface=github_bot&intent=review_rules)
</details>
</issue_to_address>
### Comment 11
<location path="scripts/publish-platform-index.sh" line_range="49" />
<code_context>
+# Child manifests and blobs already exist in this repository. Copying them again
+# can recompress layers and replace the exact digests pinned above. Publish only
+# the index; keep the registry verification below strict.
+podman manifest push --all=false "${options[@]}" --format=docker "$manifest" "docker://$destination"
podman manifest inspect "${options[@]}" "$destination" > "$scratch/index.json"
python3 - "$scratch/index.json" "$amd_digest" "$arm_digest" <<'PYTHON'
</code_context>
<issue_to_address>
**issue (review_instructions):** Disabling child-manifest push can leave the published index referencing manifests that do not exist at the destination.
This is only safe when both child manifests and their blobs are already present in the destination repository under the referenced digests. If `$repository-amd`/`$repository-arm` can differ from `$destination`, the index may be published successfully but pulls will fail with missing-manifest or missing-blob errors. Either verify destination availability before using `--all=false`, or retain `--all` when the repositories differ.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `*`
**Instructions:**
Identify and highlight potential bugs or risks in the code, such as missing syntax elements that could lead to unintended behavior.
[Edit this rule](https://app.sourcery.ai/accounts/150647/repo-settings/455664976?activeTab=review-rules&rule=5326d396-ab1b-4134-b556-367c52b2b055&from_surface=github_bot&intent=review_rules)
</details>
</issue_to_address>
### Comment 12
<location path="scripts/publish-platform-index.sh" line_range="49" />
<code_context>
+# Child manifests and blobs already exist in this repository. Copying them again
+# can recompress layers and replace the exact digests pinned above. Publish only
+# the index; keep the registry verification below strict.
+podman manifest push --all=false "${options[@]}" --format=docker "$manifest" "docker://$destination"
podman manifest inspect "${options[@]}" "$destination" > "$scratch/index.json"
python3 - "$scratch/index.json" "$amd_digest" "$arm_digest" <<'PYTHON'
</code_context>
<issue_to_address>
**issue (review_instructions):** Pushing only the index can publish references to missing child manifests or blobs.
This relies on both architecture-specific manifests and all referenced blobs already existing in the destination repository. If either source image was not previously pushed there, the registry can contain a dangling index and pulls will fail. Please either verify that prerequisite explicitly before this command (and fail with a clear error), or retain a fallback/explicit push of the child manifests when they are absent.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `*`
**Instructions:**
Identify potential bugs or risks in the code and provide a clear explanation of the issue; suggest specific improvements or revisions.
[Edit this rule](https://app.sourcery.ai/accounts/150647/repo-settings/455664976?activeTab=review-rules&rule=f09cf967-42fb-43b3-896e-6b72e9cad5be&from_surface=github_bot&intent=review_rules)
</details>
</issue_to_address>
### Comment 13
<location path="scripts/publish-platform-index.sh" line_range="46" />
<code_context>
podman manifest add "${options[@]}" "$manifest" "docker://$repository-amd@$amd_digest"
podman manifest add "${options[@]}" "$manifest" "docker://$repository-arm@$arm_digest"
-podman manifest push --all "${options[@]}" --format=docker "$manifest" "docker://$destination"
+# Child manifests and blobs already exist in this repository. Copying them again
+# can recompress layers and replace the exact digests pinned above. Publish only
+# the index; keep the registry verification below strict.
</code_context>
<issue_to_address>
**suggestion (review_instructions):** Clarify the publish behavior with a concrete command example.
For example, explain that `--all=false` publishes only the index while retaining the existing child digests (such as `$amd_digest` and `$arm_digest`). This makes the intended difference from the previous `--all` behavior explicit and helps future maintainers avoid reintroducing layer copies or digest changes.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `*`
**Instructions:**
Verify that the comment includes a specific example or code suggestion to illustrate the proposed change.
[Edit this rule](https://app.sourcery.ai/accounts/150647/repo-settings/455664976?activeTab=review-rules&rule=a97c4304-e287-4165-bcb3-931d922acf66&from_surface=github_bot&intent=review_rules)
</details>
</issue_to_address>Signed-off-by: Ryan Cook <rcook@redhat.com>
cooktheryan
enabled auto-merge (squash)
October 8, 2026 20:52
cooktheryan
disabled auto-merge
October 8, 2026 20:52
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.
The main-branch image builds and workload validation pass, but all three platform publishers fail their digest checks.
podman manifest push --allrecopies already-published child images and can recompress their layers, changing the digests pinned in the index.Publish only the index with
--all=false, referencing child manifests explicitly staged and preflighted by immutable digest in the destination repository. Keep legacy architecture repositories available as compatibility aliases. Keep exact platform/digest verification enabled. Add a native local-registry regression that forces different gzip levels for architecture publication and any attempted child recopy, then pulls both platforms. It also simulates a missing destination child and verifies no public index is created.Validation: shell syntax and diff checks pass locally. The Platform publishing regression test runs the native Podman/local-registry test in GitHub Actions; production publishing remains gated on main after merge.
Failure: https://github.com/containers/fetchit/actions/runs/37838086184/job/113524870317
Summary by Sourcery
Publish multi-platform indexes without recopied child manifests changing their pinned digests.
Bug Fixes:
Enhancements:
Tests: