fix: attribute recursive copy tag failures to the destination - #2157
Merged
TerryHowe merged 1 commit intoSep 6, 2026
Merged
Conversation
When `oras cp -r` finishes copying the graph, recursiveCopy tags the root with a bare `dst.Tag` call in cmd/oras/root/cp.go. A failure there is returned unwrapped, so it is not an `oras.CopyError` and `BinaryTarget.ModifyError` cannot recover which side failed. The generic handler is used instead, and because it consults the source target first, a destination failure is printed as `Error response from registry: not found` with no indication that the local destination answered. `oras.Copy` already wraps the same root tagging in a destination `CopyError` (oras-go v2 copy.go, prepareCopy), so a plain `oras cp` gets the right attribution and only `-r` does not. Wrap the tag error the same way. The message becomes `Error from destination registry for "localhost:5000/repo:v1": not found`. Signed-off-by: Max Freedom Pollard <272618364+MaxFreedomPollard@users.noreply.github.com>
MaxFreedomPollard
requested review from
TerryHowe,
Wwwsylvia,
sabre1041,
sajayantony and
shizhMSFT
as code owners
September 6, 2026 07:05
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2157 +/- ##
=======================================
Coverage 87.62% 87.63%
=======================================
Files 139 139
Lines 5681 5685 +4
=======================================
+ Hits 4978 4982 +4
Misses 420 420
Partials 283 283 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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
2 participants
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.
What this PR does / why we need it:
oras cp -rblames the source registry when it is the destination that failed. If the final tagging of the root fails, the user seesError response from registry: not found, with no host and nothing saying which side answered, so the source registry looks broken.The cause is in
recursiveCopy(cmd/oras/root/cp.go:279), which tags the root with a baredst.Tagcall and returns the error unwrapped. That error is not anoras.CopyError, soBinaryTarget.ModifyError(cmd/oras/internal/option/binary_target.go:73) has no origin to read and falls through to the generic handler, which asks the source target first. The source then claims the error and sets the plain registry prefix.oras-go wraps exactly the same root tagging in a destination
CopyErrorinsideprepareCopy(oras-go v2.6.2, copy.go:480 and copy.go:513), which is why a plainoras cpgets the attribution right and only-rdoes not. This change wraps the tag error the same way, which is allBinaryTarget.ModifyErrorneeds.Before:
Error response from registry: not foundAfter:
Error from destination registry for "localhost:5000/gitlab-workhorse-ee:v19.3.0": not foundThis PR is limited to the copy path. The second change suggested in the issue, adding a host check to the
errdef.ErrNotFoundbranch ofTarget.ModifyError, touches every command that uses a remote target and is left out of this one.Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes #2156
Please check the following list:
Local validation (macOS arm64, Go 1.26.4):
go test -run Test_recursiveCopy_tagFailure ./cmd/oras/root/on unmodified main:FAIL cp_test.go:592: recursiveCopy() error = not found, want *oras.CopyError. With the change:ok oras.land/oras/cmd/oras/root 0.266s.go test ./cmd/oras/... ./internal/...: all packages ok.golangci-lint run ./...: 0 issues.go build ./cmd/oras: ok.E2E was not run locally since it needs a registry and Docker. No E2E case asserts the message for this path; the only
cpcase that checks a destination prefix is the not-logged-in one in test/e2e/suite/command/cp.go:130, which already expectsError from destination registry for.