Skip to content
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.

Sign up for GitHub

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

Add some text about extensions #164

Merged
merged 1 commit into from
Sep 13, 2016
Merged

Add some text about extensions #164

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 38 additions & 22 deletions manifest.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,17 +61,22 @@ A client will distinguish a manifest list from an image manifest based on the Co

This OPTIONAL property specifies an array of strings, each specifying a mandatory CPU feature (for example `sse4` or `aes`).

- **`annotations`** *string-string hashmap*
- **`annotations`** *string-string map*

This OPTIONAL property contains arbitrary metadata for the manifest list.
Annotations is a key-value, unordered hashmap.
Keys are unique, and best practice is to namespace the keys.
Common annotation keys include:
* **created** date on which the image was built (string, timestamps type)
* **authors** contact details of the people or organization responsible for the image (freeform string)
* **homepage** URL to find more information on the image (string, must be a URL with scheme HTTP or HTTPS)
* **documentation** URL to get documentation on the image (string, must be a URL with scheme HTTP or HTTPS)
Annotations MUST be a key-value map where both the key and value MUST be strings.
While the value MUST be present, it MAY be an empty string.
Keys MUST be unique within this map, and best practice is to namespace the keys.
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.
If there are no annotations then this property MUST either be absent or be an empty map.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an odd requirement. I think “absent” is covered by the earlier “This OPTIONAL property…”. I think “empty map” is covered by there not being any required keys.

And I can't think of a way to test this. You'd need to do something like “make a manifest without annotations” and check for an unset or empty-map annotations. But the only way I can think of saying “make a make a manifest without annotations” is to say “annotations should be unset (or an empty map)”, so my test would be tautological.

If we don't have a clear idea of a better test for this line, can we drop it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the point of this was to make it clear that if you don't have annotation you can still have the "annotations" property but it needs to be empty. What I was trying to avoid was someone claiming an impl is not compliant because they put an empty map instead of dropping the property all together. OPTIONAL only covers the ability to leave it off entirely, not for it to be empty.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On Wed, Sep 07, 2016 at 11:10:55AM -0700, Doug Davis wrote:

  • If there are no annotations then this property MUST either be absent or be an empty map.

… What I was trying to avoid was someone claiming an impl is not
compliant because they put an empty map instead of dropping the
property all together.

Under what grounds would they make that claim? There is no “MUST
contain at least one key” requirement.

OPTIONAL only covers the ability to leave it off entirely, not for
it to be empty.

Agreed. The empty map case is covered by there not being any required
keys 1, because the spec'ed type is a JSON object, and an empty
object is still an object. Array/object/string properties allow the
empty value by default unless the spec explicitly forbids it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The points isn't that from a pure geeky perspective whether or not an empty set is allowed, it is. The problem is when humans get involved and they don't fully understand all of the specs when they come together. I've seen this type of interop problem before and I'm trying to avoid it. As an example, I've seen it in Docker where two different people were treating foo:null and the absence of "foo" differently - one thought they were the semantic equivalent and the other did not. Let's just be explicit and avoid it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On Wed, Sep 07, 2016 at 11:47:09AM -0700, Doug Davis wrote:

As an example, I've seen it in Docker where two different people
were treating foo:null and the absence of "foo" differently - one
thought they were the semantic equivalent and the other did not.
Let's just be explicit and avoid it.

I'm ok with explicitly saying that unset and null have the same
semantics as an empty array/map/string. But I think we should be
making that point at a spec-wide level, and not just for the
annotations property. Array/map/string properties for which that
general rule does not hold can override it explicitly on a
per-property level (to pick an example from runtime-spec, an unset or
null process.capabilities presumably means “use the platform default
capabilities” while an empty array means “drop all capabilities” 1).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we save that for another PR so we can get this behind us?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On Wed, Sep 07, 2016 at 02:44:03PM -0700, Doug Davis wrote:

  • If there are no annotations then this property MUST either be absent or be an empty map.

Can we save that for another PR so we can get this behind us?

If you prefer. In that case, I'd rather remove this line with the
understanding that we'll be adding more broadly-scoped language soon.
But I'm ok with this landing and then getting pulled back out when the
more broadly-scoped language lands.

Implementations that are reading/processing manifest lists MUST NOT generate an error if they encounter an unknown annotation key.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you want to see this same sentence on the other section too?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

darn- I think I moved instead of copied. fixed -thanks!


See [Pre-Defined Annotation Keys](#pre-defined-annotation-keys).

### Extensibility
Implementations that are reading/processing manifest lists MUST NOT generate an error if they encounter an unknown property.
Instead they MUST ignore unknown properties.

## Example Manifest List

Expand Down Expand Up @@ -104,8 +109,8 @@ A client will distinguish a manifest list from an image manifest based on the Co
}
],
"annotations": {
"key1": "value1",
"key2": "value2"
"com.example.key1": "value1",
"com.example.key2": "value2"
}
}
```
Expand Down Expand Up @@ -139,18 +144,22 @@ Unlike the [Manifest List](#manifest-list), which contains information about a s
Subsequent layers MUST then follow in the order in which they are to be layered on top of each other.
The algorithm to create the final unpacked filesystem layout MUST be to first unpack the layer at index 0, then index 1, and so on.

- **`annotations`** *hashmap*
- **`annotations`** *string-string map*

This OPTIONAL property contains arbitrary metadata for the manifest list.
Annotations is a key-value, unordered hashmap.
Keys MUST be unique within the hashmap, and best practice is to namespace the keys.
Common annotation keys include:
* **created** date on which the image was built (string, timestamps type)
* **authors** contact details of the people or organization responsible for the image (freeform string)
* **homepage** URL to find more information on the image (string, must be a URL with scheme HTTP or HTTPS)
* **documentation** URL to get documentation on the image (string, must be a URL with scheme HTTP or HTTPS)
* **source** URL to get the source code for the binary files in the image (string, must be a URL with scheme HTTP or HTTPS)
This OPTIONAL property contains arbitrary metadata for the image manifest.
Annotations MUST be a key-value map where both the key and value MUST be strings.
While the value MUST be present, it MAY be an empty string.
Keys MUST be unique within this map, and best practice is to namespace the keys.
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.
If there are no annotations then this property MUST either be absent or be an empty map.
Implementations that are reading/processing the image manifest MUST NOT generate an error if they encounter an unknown annotation key.

See [Pre-Defined Annotation Keys](#pre-defined-annotation-keys).

### Extensibility
Implementations that are reading/processing image manifests MUST NOT generate an error if they encounter an unknown property.
Instead they MUST ignore unknown properties.

## Example Image Manifest

Expand Down Expand Up @@ -182,8 +191,15 @@ Unlike the [Manifest List](#manifest-list), which contains information about a s
}
],
"annotations": {
"key1": "value1",
"key2": "value2"
"com.example.key1": "value1",
"com.example.key2": "value2"
}
}
```

# Pre-Defined Annotation Keys
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This DRYs up the pre-defined annotation keys, which is good. But there's still a lot of duplicate language about extensibility and the annotations field itself. Can we collect all of that into one location and link to it from other consumers?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bump, I'd still prefer this so there's less chance of getting out of sync later

This specification defines the following annotation keys, which MAY be used by 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)
* **org.opencontainers.documentation** URL to get documentation on the image (string, must be a URL with scheme HTTP or HTTPS)