-
Notifications
You must be signed in to change notification settings - Fork 176
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
WIP: fix: add image spec option back to attach for expanded compatibility … #1227
Conversation
a18e988
to
babbf84
Compare
cmd/oras/root/attach.go
Outdated
pack := func() (ocispec.Descriptor, error) { | ||
return oras.PackManifest(ctx, store, oras.PackManifestVersion1_1_RC4, opts.artifactType, packOpts) | ||
return oras.PackManifest(ctx, store, opts.PackVersion, opts.artifactType, packOpts) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can do something like
switch opts.PackVersion {
case ImageSpecV1_0:
return ocispec.Descriptor{}, errors.New("image-spec v1.0 is not supported")
case PackManifestVersion1_1_RC2:
return oras.Pack(ctx, store, opts.artifactType, packOpts.Layers, oras.PackOptions{
Subject: packOpts.Subject,
ManifestAnnotations: packOpts.ManifestAnnotations,
PackImageManifest: true,
})
default:
return oras.PackManifest(ctx, store, opts.PackVersion, opts.artifactType, packOpts)
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We certainly can, and I'll defer to your preference. I figured we could keep the details out of the client with the initial proposal, but if we'd rather have it mostly in the client, I'll make this change.
I think that still requires a change in oras-go and a dep bump in oras (to add back the RC2 const). But let me know if I am missing something obvious!
docs/proposals/compatibility-mode.md
Outdated
- Using a flag `--distribution-spec` with `oras attach`, `oras cp`, and `oras manifest push` to configure compatibility with registry when pushing or copying an OCI image manifest. This flag is also applicable to `oras discover` for viewing and filtering the referrers. | ||
|
||
### Build and push OCI image manifest using a flag `--image-spec` | ||
|
||
Use the flag `--image-spec <spec version>` in `oras push` to specify which version of the OCI Image-spec when building and pushing an OCI image manifest. It supports specifying the option `v1.0` or `v1.1` as the spec version. The option `v1.1` is the default behavior in `oras push` since ORAS CLI v1.1.0 so users don't need to manually specify this option. | ||
Use the flag `--image-spec <spec version>` in `oras push` and `oras attach` to specify which version of the OCI Image-spec when building and pushing an OCI image manifest. It supports specifying the option `v1.0` or `v1.1` as the spec version. The option `v1.1` is the default behavior in `oras push` since ORAS CLI v1.1.0 so users don't need to manually specify this option. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oras attach
does not support --image-spec v1.0
as it does not specify the subject
field.
docs/proposals/compatibility-mode.md
Outdated
- Using a flag `--distribution-spec` with `oras attach`, `oras cp`, and `oras manifest push` to configure compatibility with registry when pushing or copying an OCI image manifest. This flag is also applicable to `oras discover` for viewing and filtering the referrers. | ||
|
||
### Build and push OCI image manifest using a flag `--image-spec` | ||
|
||
Use the flag `--image-spec <spec version>` in `oras push` to specify which version of the OCI Image-spec when building and pushing an OCI image manifest. It supports specifying the option `v1.0` or `v1.1` as the spec version. The option `v1.1` is the default behavior in `oras push` since ORAS CLI v1.1.0 so users don't need to manually specify this option. | ||
Use the flag `--image-spec <spec version>` in `oras push` and `oras attach` to specify which version of the OCI Image-spec when building and pushing an OCI image manifest. It supports specifying the option `v1.0` or `v1.1` as the spec version. The option `v1.1` is the default behavior in `oras push` since ORAS CLI v1.1.0 so users don't need to manually specify this option. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we list all available options (values) of the flag --image-spec
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, this flag should be marked as experimental since it might be deprecated in the future once all registries upgrade to the latest OCI Spec v1.1.0 (stable) someday.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might users want to use oras push
for v1.0 manifests into the future?
babbf84
to
00b47c2
Compare
…atibility (oras-project#1224) Signed-off-by: Jesse Butler <[email protected]>
00b47c2
to
b6440f1
Compare
I've modified this PR to align with feedback, but given the Gitlab registry issue seems unrelated, this looks to potentially be an ECR-only issue related to supporting requirements for client-managed referrers as specified by Image spec 1.1.0-rc2 and not as specified by rc4. Specifically, ECR allows Given fallback support as described by 1.1. release candidates should be OCI 1.0 compliant, I am going to pause this PR (keep it WIP for now) and investigate extending support in ECR. |
Still a WIP? |
I did some testing and after some internal discussions, we are going to allow Thanks so much for the support and discussion along the way. |
Fixes #1224
This is a WIP, would like to get opinion from @shizhMSFT @qweeah and others on requiring changes in oras-go or not.
The proposed fix for ECR (and hopefully Gitlab, probably others) is to reinstate the oras client 1.0 behavior which allows the user to specify
--image-spec
w/oras attach
, in order to allow users to specify usingv1.1.0-rc2
. This version image manifest was broadly supported by 1.0 registries like ECR. With the default in oras client 1.1 now usingv1.1.0-rc4
, attach is not longer working for ECR (and likely other registries).I don't think we can cleanly do this with zero changes to oras-go, because the string vals for the oras client
--image-spec
option come from oras-go (oras.PackManifestVersion
).I thought about using
oras.Pack
(as suggested by @shizhMSFT), but I at least need to addv1.1.0-rc2
as a pack manifest version.Since the RC version support is unstable and will be removed in the future, I'm leaning toward leaning into an oras-go PR (and version bump for oras client), and putting most of the changes to support deprecated image spec release candidates into oras-go, where it can be limited and eventually removed more easily.
See accompanying WIP PR in oras-go for conversation.
Please check the following list:
TODO