feat(cp): add --force to handle partially-populated destinations#2061
feat(cp): add --force to handle partially-populated destinations#2061renshao wants to merge 1 commit into
Conversation
bba852e to
c17b8de
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds an opt-in --force mode to oras cp and oras push to better handle destinations (notably pull-through caches) that can report a manifest as “existing” while still missing referenced platform manifests/blobs, by forcing traversal of manifest nodes during graph copy.
Changes:
- Add
--forcetooras cpandoras push, wrapping the destination with aVerifyingTargetthat reports manifest descriptors as non-existent to defeat oras-go’s sub-DAG skip. - Introduce
internal/contentutilhelpers:IsManifestMediaType()andVerifyingTarget.Exists()override behavior, with unit tests. - Add an E2E test intended to validate copying into a partially populated destination using
--force.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
cmd/oras/root/cp.go |
Adds --force flag and wraps the destination with contentutil.VerifyingTarget when enabled. |
cmd/oras/root/push.go |
Adds --force flag and applies the contentutil.VerifyingTarget wrapper before tracking/copy. |
internal/contentutil/verify.go |
Implements IsManifestMediaType and VerifyingTarget wrapper to force manifest traversal. |
internal/contentutil/verify_test.go |
Adds unit tests for manifest media-type detection and Exists() override behavior. |
test/e2e/suite/command/cp.go |
Adds an E2E scenario for cp --force against a “partially populated” destination. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
b26caac to
206b90a
Compare
|
|
||
| // IsManifestMediaType reports whether the given media type denotes a | ||
| // manifest (i.e. a non-leaf node in a content-addressable graph). | ||
| func IsManifestMediaType(mediaType string) bool { |
There was a problem hiding this comment.
I assume oras-go has nothing like this? This seems like a nice thing for oras-go to do.
There was a problem hiding this comment.
Checked oras-go, there isn't something like this. I'm happy to create a new issue there and implement it. For this PR, let's keep it the current way. Once the oras-go has this feature we can de-dup later.
|
@TerryHowe |
TerryHowe
left a comment
There was a problem hiding this comment.
I'd like @sajayantony to weigh in on the name of this option.
Other than that, generally looks good other than the fixture changes.
@sajayantony I think we landed on |
Addresses oras-project#2072. A pull-through cache (e.g. ACR Artifact Cache, AWS ECR pull-through cache, GAR remote repositories) can report a manifest as "exists" while only holding a subset of its referenced content. oras-go's copyGraph trusts the destination's existence response and skips the sub-DAG, so the final tag PUT is rejected by the registry with 'manifest blob unknown' because referenced platform manifests or blobs are missing. Add an opt-in --force flag on `oras cp` and `oras push` that wraps the destination with a small GraphTarget reporting Exists()=false for any manifest media type. This defeats the sub-DAG skip and forces the copy to walk every referenced manifest, pushing any missing manifests or blobs before the root tag is updated. Blob existence checks still short-circuit, so already-present layers are not re-uploaded. The wrapper preserves the underlying scope hint so registry auth is unaffected. Changes: - cmd/oras/root/cp.go: register --force, document it in the examples, and wrap dst with the traversing target when the flag is set. - cmd/oras/root/push.go: register --force and apply the same wrapper before tracking starts. - internal/contentutil/traverse.go: traversing GraphTarget wrapper and manifest media-type predicate, shared by cp and push. - internal/contentutil/traverse_test.go: unit tests for the wrapper and predicate. - test/e2e/suite/command/cp.go: e2e test that uses an OCI image layout as the destination so a partial state can be staged (conformant registries reject indices whose children are missing). It fully copies a multi-arch image, deletes two platform manifest blobs on disk, then re-runs cp with --force and asserts the missing children are re-pushed before the tag is updated. Includes a negative control that runs a plain cp first and asserts the missing platform manifests remain absent, proving the bug exists and that --force fixes it. Default behavior is unchanged; --force is opt-in on both commands. Signed-off-by: Ren Shao <405516+renshao@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Ren Shao <shaoren@microsoft.com>

Addresses #2072.
A pull-through cache (e.g. ACR Artifact Cache, AWS ECR pull-through cache,
GAR remote repositories) can report a manifest as "exists" while only
holding a subset of its referenced content. oras-go's copyGraph trusts
the destination's existence response and skips the sub-DAG, so the
final tag PUT is rejected by the registry with
'manifest blob unknown' because referenced platform manifests or blobs
are missing.
Add an opt-in --force flag on
oras cpandoras pushthat wraps thedestination with a small GraphTarget reporting Exists()=false for any
manifest media type. This defeats the sub-DAG skip and forces the copy
to walk every referenced manifest, pushing any missing manifests or
blobs before the root tag is updated. Blob existence checks still
short-circuit, so already-present layers are not re-uploaded.
Changes:
and wrap dst with verifyingTarget when the flag is set.
verifyingTarget wrapper before tracking starts.
isManifestMediaType predicate, shared by cp and push.
predicate.
destination with only the linux/amd64 platform and re-runs cp with
--force to push the remaining platform manifests of a multi-arch
image before tagging.
Default behavior is unchanged; --force is opt-in on both commands.
Please check the following list:
--forcebehaviour