feat!: support OCI image fallback for oras push and oras attach
#665
Conversation
Codecov Report
@@ Coverage Diff @@
## main #665 +/- ##
=======================================
Coverage 72.12% 72.12%
=======================================
Files 14 14
Lines 513 513
=======================================
Hits 370 370
Misses 114 114
Partials 29 29 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
qweeah
force-pushed
the
oci-fallback
branch
2 times, most recently
from
9f673f7 to
83b48fd
Compare
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
cmd/oras/attach.go
Outdated
| o.FindSuccessors = func(ctx context.Context, fetcher content.Fetcher, desc ocispec.Descriptor) ([]ocispec.Descriptor, error) { | ||
| successors, err := findSuccessors(ctx, fetcher, desc) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| if !content.Equal(desc, root) { | ||
| return successors, nil | ||
| } | ||
|
|
||
| // skip subject to save one HEAD towards dst | ||
| j := len(successors) - 1 | ||
| for i, s := range successors { | ||
| if content.Equal(s, subject) { | ||
| // swap subject to end and slice it off | ||
| successors[i] = successors[j] | ||
| return successors[:j], nil | ||
| } | ||
| } | ||
| return nil, fmt.Errorf("failed to find subject %v in the packed root %v", subject, root) |
There was a problem hiding this comment.
Can you help me to understand this?
There was a problem hiding this comment.
This is to skip all successor but leaving packed config, refactored in the new code.
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
| // As of November 2022, ECR is known to return UNSUPPORTED error when | ||
| // putting an OCI artifact manifest. | ||
| switch errCode.Code { | ||
| case errcode.ErrorCodeManifestInvalid, errcode.ErrorCodeUnsupported: |
There was a problem hiding this comment.
Probably a non-issue, just curious how do we deal with the casing (upper-case/lower-case) here? (e.g., UNSUPPORTED vs Unsupported)
Error: PUT "https://080565210187.dkr.ecr.us-east-1.amazonaws.com/v2/utnubu/manifests/oras.unknown":
unexpected status code 400: unsupported:
Unsupported manifest type: "application/vnd.oci.artifact.manifest.v1+json"
vs
% git grep ErrorCodeUnsupported
registry/remote/errcode/errors.go:42: ErrorCodeUnsupported = "UNSUPPORTED"
There was a problem hiding this comment.
I think it aligns with the casing here - https://github.com/opencontainers/distribution-spec/blob/main/spec.md#error-codes
There was a problem hiding this comment.
@nima Can you help confirm if the fallback works for ECR? I notice that the in-body error message was not capitalized in ECR, shown as below:
unexpected status code 400: unsupported: Unsupported manifest type: "application/vnd.oci.artifact.manifest.v1+json"
You can make from source and run a oras attach based on ECR
| return root, nil | ||
| } | ||
|
|
||
| func isArtifactUnsupported(err error) bool { |
There was a problem hiding this comment.
Curious why we chose names that result in double-negatives? For example "is it not not-supported" vs "is it supported".
There was a problem hiding this comment.
+1 on IsArtifactSupported
There was a problem hiding this comment.
@nima Nice catch! @shizhMSFT How about renaming it to NoFallbackToOciImage or something indicates that we should not retry with OCI Image?
The reason is both isArtifactUnsupported and isArtifactSupported is not accurate, we don't know if artifact is supported if the other HTTP error happens.
There was a problem hiding this comment.
Moving the disscusion to #672
…ras-project#665) Since oras-go will take care of fallback with tag schema if referrer API not supported, this PR provides OCI Image fallback when-and-only-when OCI artifact is not supported. Resolves: oras-project#654 Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Since oras-go will take care of fallback with tag schema if referrer API not supported, this PR provides OCI Image fallback when-and-only-when OCI artifact is not supported.
Resolves: #654