Descriptor annotations #501

vbatts · 2016-12-15T20:50:59Z

this has come up in discussion and is needed for adding discoverable arbitrary data to the descriptor references. Especially in making an index for image layout.

In DRY'ing up the references to annotations. This makes the defined keys in a very logical place rather than the bottom of one of the other docs.

wking

Hooray DRY :).

wking · 2016-12-15T21:01:09Z

annotations.md

+
+## Pre-Defined Annotation Keys
+
+This specification defines the following annotation keys, which MAY be used by manifest list and image manifest authors:


Is there a reason to restrict the scope of the pre-defined keys to manifests and manifest lists? For example, org.opencontainers.source seems like it would be a good match for a descriptor referencing layer tar archive. If that's too big a shift for this PR, I'm fine spinning it off into a follow-up issue/PR.

wking · 2016-12-15T21:04:39Z

Oh, you'll also want Makefile and spec.md-ToC updates to pull in the new file. See here for a pre-spec.md example.

jonboulle · 2016-12-16T16:34:04Z

annotations.md

@@ -0,0 +1,25 @@
+# Annotations
+This OPTIONAL property MUST use the [annotation rules](annotations.md#rules).


imho something like "Several components of the specification, like Image Manifests and Descriptors, feature an optional annotations property, whose format is common and defined in this section"

jonboulle · 2016-12-16T16:34:16Z

one suggestion, looks good overall!

vbatts · 2017-01-11T14:55:02Z

updated. PTAL

wking · 2017-01-11T15:03:31Z

annotations.md

+If there are no annotations then this property MUST either be absent or be an empty map.
+Implementations that are reading/processing manifest lists MUST NOT generate an error if they encounter an unknown annotation key.
+
+See [Pre-Defined Annotation Keys](manifest.md#pre-defined-annotation-keys).


You removed this manifest.md section, and the annotations.md section if right after this line. So I think we should drop this line.

wking · 2017-01-11T15:06:54Z

annotations.md

+This specification defines the following annotation keys, intended for but not limited to manifest list and image manifest authors:
+* **org.opencontainers.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.authors** contact details of the people or organization responsible for the image (freeform string)
+* **org.opencontainers.homepage** URL to find more information on the image (string, must be a URL with scheme HTTP or HTTPS)


I think we should either drop "must be a" here or add it to org.opencontainers.created. I'm in favor of dropping it.

philips · 2017-01-18T21:37:09Z

@vbatts please fix the one nit and then we can LGTM this.

stevvooe · 2017-01-18T23:16:46Z

annotations.md

+
+This specification defines the following annotation keys, intended for but not limited to manifest list and image manifest authors:
+* **org.opencontainers.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.authors** contact details of the people or organization responsible for the image (freeform string)


How would multiple authors be specified?

stevvooe · 2017-01-18T23:44:56Z

annotations.md

+* **org.opencontainers.authors** contact details of the people or organization responsible for the image (freeform string)
+* **org.opencontainers.homepage** URL to find more information on the image (string, must be a URL with scheme HTTP or HTTPS)
+* **org.opencontainers.documentation** URL to get documentation on the image (string, must be a URL with scheme HTTP or HTTPS)
+* **org.opencontainers.source** URL to get source code for the binary files in the image (string, must be a URL with scheme HTTP or HTTPS)


Do we require a licenses field?

Yeah, as per the discussion in #506, we should add an org.opencontainers.licenses field. Because licensing is complicated, we could allow a free-form string but the string SHOULD be made up of SPDX identifiers like GPL-2.0 or MIT.

No, that's not going to work at all, you can't properly describe the license for a whole layer in any form of a string that could be standardized or parsed. SPDX is great for describing the individual licenses of things, but not for a collection of things, which almost always has an arbitrary license (example, what's the license, in a simple string, for a Ubuntu base layer?)

what's the license, in a simple string, for a Ubuntu base layer?

This is still a pain, but it's becoming almost bearable as Debian's copyright-file spec gains ground. Walking the package list (e.g. yakkety-20170104), looking up the associated copyright file (e.g. util-linux_2.28.2-1ubuntu1) and collecting the license information will result in something like this quasi-SPDX license expression:

Artistic AND BSD-2-Clause AND BSD-3-Clause AND BSD-3-Clause-with-weird-numbering AND BSD-4-Clause AND BSD-4-Clause-POWERDOG AND BZIP AND CC0 AND CC0-1.0 AND DONT-CHANGE-THE-GPL AND Expat AND ( Expat OR GPL-1+ OR Artistic ) AND GAP AND GAP~FSF AND GFDL-1.3+ AND GPL AND ( GPL-1+ OR Artistic ) AND GPL-2 AND GPL-2+ AND ( GPL-2+ OR Artistic ) AND ( GPL-2+ OR LGPL-3+ ) AND ( GPL-2+ WITH libtool exception ) AND GPL-2.0+ AND GPL-2.1+ AND GPL-3 AND GPL-3+ AND ( GPL-3+ OR BSD-3-Clause ) AND ( GPL-3+ WITH the GCC Runtime Library Exception 3.1 ) AND ( GPL-3+-WITH-BISON-EXCEPTION ) AND HPND AND ( HSIEH-DERIVATIVE OR HSIEH-BSD OR LGPL-2.1 ) AND INNER-NETv2.0 AND ISC AND ISOC-rfc WITH disclaimer AND LGPL AND LGPL-2 AND LGPL-2+ AND ( LGPL-2+ OR BSD-2-Clause OR MIT ) AND LGPL-2.0+ AND LGPL-2.1 AND LGPL-2.1+ AND LGPL-3+ AND ( LGPL-3+ OR GPL-2+ ) AND LGPL-3.1 AND MACHv3.0-modified AND MIT AND PCRE AND PD AND REGCOMP AND RFC-Reference AND S2P AND SDBM-PUBLIC-DOMAIN AND SMAIL AND Spencer-94 AND TEXT-TABS AND TinySCHEME AND Unicode AND X11 AND ZLIB AND attribution AND bzip2-1.0.6 AND permissive AND public-domain AND public-domain-md5 AND public-domain-s-s-d AND verbatim AND ( verbatim OR GFDL-1.3+ ) AND ( verbatim WITH some-TeX-stuff ) AND more because many packages don't use the copyright-file spec yet…

Besides being incomplete, the license tagging isn't particularly consistent (and SPDX 2.6 doesn't name everthing we need). But that can get fixed on a per-project basis for parse-ability and a per-license basis for registration (and/or we could follow Debian and give a standard way to inline any unregistered licenses).

You can't properly describe the license for a whole layer…

It would be a lot easier to handle this if we had per-file granularity for the license metadata. But with the current OCI image spec, layers are the closest we get to per-file. I don't think bumping the licensing information up into higher levels is going to make it any easier to set the appropriate data when you publish an image (although it may make it slightly easier to discover the data when you read the image, since you'd only have to look in one place).

It would be a lot easier to handle this if we had per-file granularity for the license metadata.

This is wildly outside of scope for the OCI image format.

What is wrong with having something where you take a layer digest and get back the licensing information about that digest? Data normalization is a good thing and applying it here is a no brainer.

Trying to make the format do everything is just going to make it bad at everything.

What is wrong with having something where you take a layer digest and get back the licensing information about that digest?

Per-descriptor metadata for licensing seems like the best way to handle licensing with the current OCI image modeling. I don't see a need to push this metadata out of band, and I don't see a benefit to documenting licensing in the config instead of in the digest linking to the licensed content.

A per-layer out of band licensing registry might be a useful way to share data among image producers (as a build-time optimization), but I think having licensing metadata in the Merkle DAG is a better experience for image consumers.

@wking

I think having licensing metadata in the Merkle DAG is a better experience for image consumers.

I don't see how this can possibly work without making a format that has an entry for every file.

Either way, let's not let this point block the PR. We can get annotations in, then have another PR to discuss standard annotation license key.

gregkh · 2017-01-20T11:34:44Z

As much as I feel "this way lies madness", I understand the need/wish to have arbitrary annotations to help describe things, so it is good to describe a way to have this.

LGTM.

caniszczyk · 2017-01-20T20:34:51Z

Can we also target this towards RC4?

jonboulle · 2017-01-23T11:00:33Z

lgtm

jonboulle · 2017-01-23T11:01:06Z

@gregkh bot didn't pick up your approval properly

gregkh · 2017-01-23T11:09:28Z

lgtm

There, that better?

gregkh · 2017-01-23T11:09:38Z

lgtm

jonboulle · 2017-01-23T11:48:10Z

Oh, oops. I forgot you aren't actually a maintainer of the spec...

gregkh · 2017-01-23T11:55:50Z

ah, that makes sense why it wasn't working for me :)

Signed-off-by: Vincent Batts <vbatts@hashbangbash.com>

vbatts · 2017-01-23T18:35:55Z

nits fixed and rebased. PTAL

stevvooe · 2017-01-23T20:37:17Z

LGTM

wking · 2017-01-23T21:57:50Z

annotations.md

+Keys SHOULD be named using a reverse domain notation - e.g. `com.example.myKey`.
+Keys using the `org.opencontainers` namespace are reserved and MUST NOT be used by other specifications and extensions.
+If there are no annotations then this property MUST either be absent or be an empty map.
+Implementations that are reading/processing manifest lists MUST NOT generate an error if they encounter an unknown annotation key.


Is this restriction specific to manifest lists? Maybe it should read:

Consumers MUST NOT generate an error if they encounter an unknown annotation key.

or something similarly generic.

In DRY'ing up the references to annotations. This makes the defined keys in a very logical place rather than the bottom of one of the other docs. Signed-off-by: Vincent Batts <vbatts@hashbangbash.com>

this has come up in discussion and is needed for adding discoverable arbitrary data to the descriptor references. Especially in making an index for image layout. Signed-off-by: Vincent Batts <vbatts@hashbangbash.com>

Signed-off-by: Vincent Batts <vbatts@hashbangbash.com>

jonboulle · 2017-01-24T15:44:04Z

lgtm

stevvooe · 2017-01-24T18:39:54Z

LGTM

As I suggested in the PR landing these blocks [1,2,3]. I've shifted the extensibility section to a separate considerations.md, since it's a generic policy that applies to both our manifest and manifest-list. The extensibility requirements might arguably apply to our other JSON types as well (like annotations, which were recently DRYed up in f15a268, annotations: make a designated doc and DRY a bit, 2016-12-15, opencontainers#501). The new extensibility section sets the stage for that, but I've left the other types off this commit to focus on making the current requirements more DRY without changing the specified behavior. My personal preference would be to have separate canonicalization.md and extensibility.md files, but Jonathan wanted the single file: On Mon, Oct 10, 2016 at 09:22:46AM -0700, Jonathan Boulle wrote [4]: > Ehh, I preferred it where it was - now worried about death by a > thousand files (extensibility, canonicalization, etc etc). Can we > combine them (into a generic "considerations" document or similar), > or just put this back? [1]: opencontainers#164 (comment) [2]: opencontainers#164 (comment) [3]: opencontainers#164 (comment) [4]: opencontainers#340 (comment) Signed-off-by: W. Trevor King <wking@tremily.us>

Instead of comma-separated short identifiers, which have unclear semantics (are the delimiters AND or OR?). I don't see any discussion of the syntax for this field in [1] (which landed it), but I'd floaded license expressions before in the sub-thread starting at [2]. Greg had pushed back against my earlier proposal (licensing information on descriptors) with [3]: > No, that's not going to work at all, you can't properly describe the > license for a whole layer in any form of a string that could be > standardized or parsed. SPDX is great for describing the individual > licenses of things, but not for a collection of things, which almost > always has an arbitrary license (example, what's the license, in a > simple string, for a Ubuntu base layer?) But SPDX License Expression are both more expressive and better defined than the current comma delimiters. Everything you could have said with the comma-delimited string you can say more clearly with a SPDX License Expression. And because the syntax is not OCI-specific, you're more likely to be able to find tooling that handles these values out of the box. [1]: opencontainers#636 [2]: opencontainers#501 (comment) [3]: opencontainers#501 (comment) Signed-off-by: W. Trevor King <wking@tremily.us>

Instead of comma-separated short identifiers, which have unclear semantics (are the delimiters AND or OR?). I don't see any discussion of the syntax for this field in [1] (which landed it), but I'd floaded license expressions before in the sub-thread starting at [2]. Greg had pushed back against my earlier proposal (licensing information on descriptors) with [3]: > No, that's not going to work at all, you can't properly describe the > license for a whole layer in any form of a string that could be > standardized or parsed. SPDX is great for describing the individual > licenses of things, but not for a collection of things, which almost > always has an arbitrary license (example, what's the license, in a > simple string, for a Ubuntu base layer?) But SPDX License Expression are both more expressive and better defined than the current comma delimiters. Everything you could have said with the comma-delimited string you can say more clearly with a SPDX License Expression. And because the syntax is not OCI-specific, you're more likely to be able to find tooling that handles these values out of the box. [1]: opencontainers/image-spec#636 [2]: opencontainers/image-spec#501 (comment) [3]: opencontainers/image-spec#501 (comment) Signed-off-by: W. Trevor King <wking@tremily.us>

wking reviewed Dec 15, 2016

View reviewed changes

jonboulle reviewed Dec 16, 2016

View reviewed changes

wking mentioned this pull request Jan 6, 2017

combinational image #508

Closed

vbatts force-pushed the descriptor_annotations branch 2 times, most recently from d1196fd to 4d581b7 Compare January 11, 2017 14:54

wking reviewed Jan 11, 2017

View reviewed changes

stevvooe reviewed Jan 18, 2017

View reviewed changes

stevvooe mentioned this pull request Jan 18, 2017

descriptor: add a license for each descriptor #506

Closed

stevvooe reviewed Jan 18, 2017

View reviewed changes

wking mentioned this pull request Jan 19, 2017

manifest: DRY annotations and extensibility #340

Closed

stevvooe added the 1.0-release-blocker label Jan 20, 2017

stevvooe added this to the v1.0.0-rc4 milestone Jan 20, 2017

descriptor: fix example formatting

4986a7e

Signed-off-by: Vincent Batts <vbatts@hashbangbash.com>

vbatts force-pushed the descriptor_annotations branch from 4d581b7 to 89c3063 Compare January 23, 2017 18:35

wking reviewed Jan 23, 2017

View reviewed changes

annotations: make a designated doc and DRY a bit

f15a268

In DRY'ing up the references to annotations. This makes the defined keys in a very logical place rather than the bottom of one of the other docs. Signed-off-by: Vincent Batts <vbatts@hashbangbash.com>

vbatts added 2 commits January 24, 2017 10:41

descriptor: add OPTIONAL annotations to this object

84b6a28

this has come up in discussion and is needed for adding discoverable arbitrary data to the descriptor references. Especially in making an index for image layout. Signed-off-by: Vincent Batts <vbatts@hashbangbash.com>

*: add new file to the TOC and docs

f185dd5

Signed-off-by: Vincent Batts <vbatts@hashbangbash.com>

vbatts force-pushed the descriptor_annotations branch from 89c3063 to f185dd5 Compare January 24, 2017 15:41

stevvooe merged commit 84a74b0 into opencontainers:master Jan 24, 2017

wking mentioned this pull request Jan 24, 2017

considerations: DRY extensibility #526

Merged

wking mentioned this pull request Jan 26, 2017

image-layout: ./refs/ -> index.json #533

Merged

vbatts mentioned this pull request Jan 27, 2017

Bump version for v1.0.0-rc4 #537

Merged

wking mentioned this pull request Feb 3, 2017

specs-go/v1: add annotations field to descriptor #551

Merged

vbatts deleted the descriptor_annotations branch February 8, 2017 14:59

wking mentioned this pull request Apr 3, 2017

config: reference common label conventions #635

Closed

caniszczyk mentioned this pull request May 18, 2017

annotation / schema items #658

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Descriptor annotations #501

Descriptor annotations #501

vbatts commented Dec 15, 2016

wking left a comment

wking Dec 15, 2016

wking commented Dec 15, 2016 •

edited

Loading

jonboulle Dec 16, 2016

jonboulle commented Dec 16, 2016

vbatts commented Jan 11, 2017

wking Jan 11, 2017

wking Jan 11, 2017

philips commented Jan 18, 2017

stevvooe Jan 18, 2017

stevvooe Jan 18, 2017

cyphar Jan 18, 2017

gregkh Jan 20, 2017

wking Jan 20, 2017

stevvooe Jan 20, 2017

wking Jan 20, 2017 •

edited

Loading

stevvooe Jan 20, 2017

gregkh commented Jan 20, 2017

caniszczyk commented Jan 20, 2017

jonboulle commented Jan 23, 2017 •

edited by caniszczyk

Loading

jonboulle commented Jan 23, 2017

gregkh commented Jan 23, 2017

gregkh commented Jan 23, 2017

jonboulle commented Jan 23, 2017

gregkh commented Jan 23, 2017

vbatts commented Jan 23, 2017

stevvooe commented Jan 23, 2017 •

edited by caniszczyk

Loading

wking Jan 23, 2017

jonboulle Jan 24, 2017

vbatts Jan 24, 2017

jonboulle commented Jan 24, 2017 •

edited by caniszczyk

Loading

stevvooe commented Jan 24, 2017 •

edited by caniszczyk

Loading


		## Pre-Defined Annotation Keys

		This specification defines the following annotation keys, which MAY be used by manifest list and image manifest authors:

		@@ -0,0 +1,25 @@
		# Annotations
		This OPTIONAL property MUST use the [annotation rules](annotations.md#rules).

Descriptor annotations #501

Descriptor annotations #501

Conversation

vbatts commented Dec 15, 2016

wking left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wking commented Dec 15, 2016 • edited Loading

Choose a reason for hiding this comment

jonboulle commented Dec 16, 2016

vbatts commented Jan 11, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

philips commented Jan 18, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wking Jan 20, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gregkh commented Jan 20, 2017

caniszczyk commented Jan 20, 2017

jonboulle commented Jan 23, 2017 • edited by caniszczyk Loading

jonboulle commented Jan 23, 2017

gregkh commented Jan 23, 2017

gregkh commented Jan 23, 2017

jonboulle commented Jan 23, 2017

gregkh commented Jan 23, 2017

vbatts commented Jan 23, 2017

stevvooe commented Jan 23, 2017 • edited by caniszczyk Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonboulle commented Jan 24, 2017 • edited by caniszczyk Loading

stevvooe commented Jan 24, 2017 • edited by caniszczyk Loading

wking commented Dec 15, 2016 •

edited

Loading

wking Jan 20, 2017 •

edited

Loading

jonboulle commented Jan 23, 2017 •

edited by caniszczyk

Loading

stevvooe commented Jan 23, 2017 •

edited by caniszczyk

Loading

jonboulle commented Jan 24, 2017 •

edited by caniszczyk

Loading

stevvooe commented Jan 24, 2017 •

edited by caniszczyk

Loading