-
Notifications
You must be signed in to change notification settings - Fork 676
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
image-layout.md, annotations.md: restrict scope of ref.name #634
Conversation
👍 It doesn't make sense to interpret annotations everywhere. By making it explicit where the ref name should be interpreted, there should be no ambiguity on implementing. |
annotations.md
Outdated
@@ -22,4 +22,4 @@ This specification defines the following annotation keys, intended for but not l | |||
* **org.opencontainers.image.homepage** URL to find more information on the image (string, a URL with scheme HTTP or HTTPS) | |||
* **org.opencontainers.image.documentation** URL to get documentation on the image (string, a URL with scheme HTTP or HTTPS) | |||
* **org.opencontainers.image.source** URL to get source code for the binary files in the image (string, a URL with scheme HTTP or HTTPS) | |||
* **org.opencontainers.image.ref.name** Name of the reference (string) | |||
* **org.opencontainers.image.ref.name** Name of the reference for a target (string). Only valid when on index.json in [image layout](image-layout.md). |
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.
if we're going to narrow this, then perhaps it should specify that this is within the context of a descriptor, so that target is making more sense.
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.
@vbatts Give that wording a try.
3372192
to
db88539
Compare
annotations.md
Outdated
@@ -22,4 +22,4 @@ This specification defines the following annotation keys, intended for but not l | |||
* **org.opencontainers.image.homepage** URL to find more information on the image (string, a URL with scheme HTTP or HTTPS) | |||
* **org.opencontainers.image.documentation** URL to get documentation on the image (string, a URL with scheme HTTP or HTTPS) | |||
* **org.opencontainers.image.source** URL to get source code for the binary files in the image (string, a URL with scheme HTTP or HTTPS) | |||
* **org.opencontainers.image.ref.name** Name of the reference (string) | |||
* **org.opencontainers.image.ref.name** Name of the reference for a target (string). Only valid when on descriptor within an index.json in [image layout](image-layout.md). |
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.
this seems conflicting with L19: This specification defines the following annotation keys, intended for but not limited to [image index](image-index.md) and image [manifest](manifest.md) authors:
How about splitting "Pre-Defined Annotation Keys" section to like this:
## Common Pre-Defined Annotation Keys
This specification defines the following annotation keys, intended for but not limited to [image index](image-index.md) and image [manifest](manifest.md) authors:
* **org.opencontainers.image.created** date on which the image was built (string, date-time as defined by [RFC 3339](https://tools.ietf.org/html/rfc3339#section-5.6)).
* **org.opencontainers.image.authors** contact details of the people or organization responsible for the image (freeform string)
* **org.opencontainers.image.homepage** URL to find more information on the image (string, a URL with scheme HTTP or HTTPS)
* **org.opencontainers.image.documentation** URL to get documentation on the image (string, a URL with scheme HTTP or HTTPS)
* **org.opencontainers.image.source** URL to get source code for the binary files in the image (string, a URL with scheme HTTP or HTTPS)
## Restricted Pre-Defined Annotation Keys
This specification defines the following annotation keys, which MUST NOT be used in descriptors other than a certain descriptor.
* **org.opencontainers.image.ref.name** Name of the reference for a target (string). Only valid when on descriptor within an index.json in [image layout](image-layout.md).
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.
+1 to splitting keys which should only appear in particular contexts out into a separate section.
This specification defines the following annotation keys, which MUST NOT be used in descriptors other than a certain descriptor.
This seems too strong for annotations (which are still described as “arbitrary metadata”. Can we SHOULD this instead of MUSTing it, and say that the meaning of these keys are unspecified (or implementation-defined?) when used outside of their recommended location?
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.
Would it make sense to move this annotation definition to the layout specification to make the scope clear?
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.
If we don't see this being used elsewhere then yeah, it may make sense to move it closer to the application/vnd.oci.image.index.v1+json
media type. However in tandem we should tweak the wording of this document to be clear that these are not the only annotations.
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.
@jonboulle I think you mean application/vnd.oci.image.index.v1+json
in the context of an image layout, but I'll clear this up. This section will then link out to data type specific use of annotations.
Does that work?
When i did the OCI export for flatpak I actually added an org.opencontainers.image.ref.name annotation to the image manifest, but mostly for simplicity of generation of the index.json (it just extracts this annotation when generating the index). It never relies on or uses the annotation in the image manifest. Is there a reason to strictly forbid this (somewhat lazy) use? The main thing is that we want the enforced semantics (naming a thing) of this annotation to only be effective in the index.json context. I.e. SHOULD, rather than MUST as @wking says. |
@alexlarsson I'm of the opinion that since we reserve all rights to the |
While I agree that it's within rights to restrict the usage of keys in the |
With the stipulation that these annotations can be ignored when not on an index.json in the context of an image layout. |
@stevvooe I reckon. Because if there were an |
Exactly. This is the case that is causing confusion and I think we can avoid it by adding this restriction. @vbatts Any changes you want to see here? |
@stevvooe in sticking with the notion that annotations are arbitrary key/value stores, rather than making this "Only valid" it should be a "SHOULD" |
By restricting the scope of the annotation `org.opencontainers.image.ref.name` to only image indexes appearing in the context of an image layout, we can greatly simplify processing and selection of target content. While other properties may apply down, we ensure that naming is restricted to a single "entity", the `index.json`, ambiguity for a given reference is reduced. This does not apply to other annotations, which may "apply downward" to other resources referenced. The application of annotation downward is specific to a particular annotation. Signed-off-by: Stephen J Day <stephen.day@docker.com>
db88539
to
93a024c
Compare
@vbatts Give that a try! |
@opencontainers/image-spec-maintainers PTAL |
By restricting the scope of the annotation
org.opencontainers.image.ref.name
to only image indexes appearing inthe context of an image layout, we can greatly simplify processing and
selection of target content. While other properties may apply down, we
ensure that naming is restricted to a single "entity", the
index.json
,ambiguity for a given reference is reduced.
This does not apply to other annotations, which may "apply downward" to
other resources referenced. The application of annotation downward is
specific to a particular annotation.
Signed-off-by: Stephen J Day stephen.day@docker.com
Related to #588.