-
Notifications
You must be signed in to change notification settings - Fork 662
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
Unspecified descriptor mediatypes MUST be ignored #759
Conversation
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.
LGTM
image-index.md
Outdated
@@ -40,7 +41,7 @@ For the media type(s) that this document is compatible with, see the [matrix][ma | |||
|
|||
Image indexes concerned with portability SHOULD use one of the above media types. | |||
Future versions of the spec MAY use a different mediatype (i.e. a new versioned format). | |||
An encountered `mediaType` that is unknown SHOULD be safely ignored. | |||
An encountered `mediaType` that is unspecified MUST be ignored. |
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.
It's a bit ambiguous what "unspecified" means here -- does it mean that the implementation is not aware of the media-type or it's not defined in the OCI spec? If the former, then I'd prefer to use the word "unknown" as before.
The reason for this nit is that you might have other media-types (like you are planning for the distribution-spec) that an implementation is aware of, but the lay-person reading of "an encountered mediaType that is unspecified MUST be ignored" is that you must ignore mediaTypes not defined in the OCI spec (and even then, the phrasing still sounds wrong to me).
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.
An encountered `mediaType` that is not [OCI-specified](media-types.md) or [compatible][matrix] MUST be ignored.
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.
An encountered `mediaType` that is not [OCI-specified](media-types.md) or [compatible][matrix] MUST be ignored.
Whoa -- why would you want to disallow people from having their own media-types? I think that the wording should be:
An encountered
mediaType
that is unknown MUST be ignored.
Which would result in any out-of-spec mediaTypes being usable with any implementation (to the extent that they can be used without knowing what the media-type is).
The text you are proposing would disallow implementations from defining their own out-of-spec media-types (because any such implementation that didn't ignore their own media-type would not be following the OCI spec), which kind of defeats the purpose of them. The original text of this PR was just ambiguous, but that text is far less favourable. Not to mention I know of several organisations that are using their own media-types with OCI images, so such a change would be breaking them.
My understanding of the reasons for this patch are that you actually don't want to do this, so I'm quite confused why you're proposing text that makes it harder to have arbitrary media-types in distribution-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.
The wording here is tricky.
The intention is that tools that are OCI compliant should not accidentally manipulate content that is being stored alongside it. They should all fail in the same way, which is to ignore them.
The tools that store other artifacts are not going to be OCI-compliant. To do so, they'd have to implement a bunch of container-specific details. The idea is that these tools work alongside OCI in a way that doesn't break things.
The changes we are making to the specs are to enable the flexibility to experiment with storing this content without disrupting OCI. The intention is not to make OCI support arbitrary content because it's fundamentally a container standard.
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.
I understand that, and I agree. I don't agree that the current wording you've proposed does this -- in fact in my reading it actually disallows third-party mediaType
s which is the opposite of what we both want.
I'm fine with @stevvooe's suggested wording:
An encountered
mediaType
that is unknown to the implementation but unspecified by OCI MUST be ignored.
or the simpler variant I included above:
An encountered
mediaType
that is unknown to the implementation MUST be ignored.
Which would result in mediaType
s that are unknown to an implementation (meaning that it's an "other artifact" that the implementation doesn't know how to manage) then it won't touch it.
manifest.md
Outdated
@@ -60,6 +60,7 @@ Unlike the [image index](image-index.md), which contains information about a set | |||
- [`application/vnd.oci.image.layer.nondistributable.v1.tar+gzip`](layer.md#gzip-media-types) | |||
|
|||
Manifests concerned with portability SHOULD use one of the above media types. | |||
An encountered `mediaType` that is unspecified MUST be ignored. |
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.
Ditto here.
I'm not a fan of MUST be ignored. They MUST be safe to ignore though |
@cyphar Let's see a suggested wording that helps with the intent, rather than issuing a rejection. The interpretation of "unspecified" does include OCI. That is up to the interpreter. If someone has specified it elsewhere and they don't ignore it, then they are compliant. This is to prevent implementations from rejecting mediatypes from one vendor or another. What if it was qualified like this:
This is very odd language. How can we make a strong statement about safety? We need strong language to ensure that future mediaTypes can be added without causing issues with backwards compatibility. |
That seems far more reasonable to me (and I agree with it).
I explained my concerns and provided suggested wording in this comment -- maybe you didn't see it? The main objection I have is that the currently-proposed wording (at least to me) seems to disallow I would be fine with:
or your suggested text (I'm not sure about the need to mention specified or unspecified |
Signed-off-by: Jimmy Zelinskie <[email protected]>
I moved forward with Aleksa's final wording because I'd rather have the intentions clear than be technically correct. PTAL |
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.
LGTM
LGTM, thanks @jzelinskie. I'm not sure what's up with the CI failure -- for some reason we are |
As per out discussions on the dev mailing list, this is the first step towards ensuring that storage and distribution of non-specified content can be experimented with while maintaining a standards-compliant registry.
cc @stevvooe