manifest: add annotations

[vbatts]

This introduces OPTIONAL annotations, which are consistent with the
annotations in the runtime-spec.

While the spec does not exclude use of arbitrary fields, this OPTIONAL
property gives a guided place for manifest authors to isolate their
arbitrary metadata.

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

[philips]

@vbatts Can you provide a few examples of what annotations are for? We have a list in appc you can steal: https://github.com/appc/spec/blob/master/spec/aci.md#image-manifest-schema

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)

[vbatts]

@philips good point.

Also, while I like the ACI array of {"name":"","value":""} objects, it feels like a hashmap is simpler and cleaner. Thoughts?

[philips]

@vbatts the reason we did that is because JSON hash maps have no ordering or rules for key collision. In fact most JSON libraries have no way of telling you you have duplicate keys. The goal was to avoid "mysterious" bugs.

But, this spec already does this for things like volumes so I am OK with making it a hashmap.

cc @jonboulle

[vbatts]

Yeah, I don't expect order in this list, but I see where that is valuable. The key-collision is undefined behaviour IMO. Though the ACI approach would leave that up to the implementation to handle it gracefully.

[philips]

@vbatts right, it is a workaround to a library and JSON spec problem. But, I am OK with the spec saying "don't do that".

[wking]

re: hashmaps, I still see no need to require a particular format for
opaque fields 1.

[vbatts]

@wking because specifying a golang interface{} is just one library's approach to parsing. That would end up with more undefined behaviour.

[wking]

On Wed, Apr 27, 2016 at 12:21:59PM -0700, Vincent Batts wrote:

@wking because specifying a golang interface{} is just one
library's approach to parsing.

So the Markdown should say ‘JSON’ or ‘object’ 1. The whole field is
already semantically opaque. What undefined behavior are you
concerned about getting after relaxing the structure restriction?

[philips]

@wking we need to dictate some sort of schema for tooling and display. I disagree with your assessment.

[wking]

On Wed, Apr 27, 2016 at 01:49:47PM -0700, Brandon Philips wrote: “@wking we need to dictate some sort of schema for tooling and display.” I think any tooling / display that intends to consume this information should be the one specifying the structure.

[philips]

The tooling needs some expectation that there will be key/value strings so
that they can be displayed in terminals, searched in web front ends, etc. I
guess we will have to agree to disagree.

On Wed, Apr 27, 2016 at 2:46 PM W. Trevor King notifications@github.com
wrote:

On Wed, Apr 27, 2016 at 01:49:47PM -0700, Brandon Philips wrote:
“@wking we need to dictate some sort of schema for tooling and
display.”

I think any tooling / display that intends to consume this information
should be the one specifying the structure.

—
You are receiving this because you were mentioned.

Reply to this email directly or view it on GitHub
#44 (comment)

[wking]

On Wed, Apr 27, 2016 at 03:14:18PM -0700, Brandon Philips wrote:

The tooling needs some expectation that there will be key/value
strings so that they can be displayed in terminals, searched in web
front ends, etc. I guess we will have to agree to disagree.

You can do all of those things with opaque JSON. In fact, there are a
number of search indexes that focus on indexing only opaque JSON ;).
Eventually, someone is going to assign semantic meaning to the data
that gets put in here, and I don't see a benefit to tying their hands
on the structure. And if your particular infrastructure does require
a flat string→string object, you can certainly impose that restriction
at a higher level. If you bake it into the spec at this level, nobody
can experiment with other structures (even people who aren't using
your more-restrictive tooling).

[philips]

Again as someone who has been involved in building tools (actool, quay,
kubernetes) it is far easier to deal with a flat schema instead of a deeply
nested one for this sort of metadata. I don't think we are making forward
progress in our discussion or adding new information.

As a general rule I would rather we lean towards restrictive schema's
rather than support a blob of whatever JSON objects someone would like to
use. If we make it too restrictive we can have annotationObjects or
something in the future when someone comes up with a concrete use case.

On Wed, Apr 27, 2016 at 3:23 PM W. Trevor King notifications@github.com
wrote:

On Wed, Apr 27, 2016 at 03:14:18PM -0700, Brandon Philips wrote:

The tooling needs some expectation that there will be key/value
strings so that they can be displayed in terminals, searched in web
front ends, etc. I guess we will have to agree to disagree.

You can do all of those things with opaque JSON. In fact, there are a
number of search indexes that focus on indexing only opaque JSON ;).
Eventually, someone is going to assign semantic meaning to the data
that gets put in here, and I don't see a benefit to tying their hands
on the structure. And if your particular infrastructure does require
a flat string→string object, you can certainly impose that restriction
at a higher level. If you bake it into the spec at this level, nobody
can experiment with other structures (even people who aren't using
your more-restrictive tooling).

—
You are receiving this because you were mentioned.

Reply to this email directly or view it on GitHub
#44 (comment)

[wking]

On Wed, Apr 27, 2016 at 03:58:44PM -0700, Brandon Philips wrote:

Again as someone who has been involved in building tools (actool,
quay, kubernetes) it is far easier to deal with a flat schema
instead of a deeply nested one for this sort of metadata. I don't
think we are making forward progress in our discussion or adding new
information.

You're suggesting a semi-opaque field and I'm trying to understand why
;). I agree that if you're actually using the contained data for some
purpose, a string→string map may be appropriate. Are actool, quay, or
kubernetes actually using the annotation fields listed in 1? How
would those use-cases be negatively impacted if a bundle author stored
additional JSON (e.g. nested objects) as a separate field inside
‘annotations’?

I think the point of having an opaque field is that we can stop
arguing about motivational use cases in the spec. Folks can just put
in whatever they like, and higher-level tooling for which this
information is not opaque can specify what they'd like to see, all
without involving this level of the stack.

[mikedanese]

I don't think the string to string object precludes storage of structured types. A user can very easily serialize a structured type to bytes and store it in a string field. What it does allow is flexibility in the serialization format a user chooses. For example a user may want to store a plaintext paragraph under a "description" annotation, or a build system may want to store a proto with build metadata. However string->string is less flexible in other ways, e.g. it would prevent generic field based ACL'ing to apply inside an annotation value.

A similar discussion happened regarding kubernetes objects:

kubernetes/kubernetes#1201
kubernetes/kubernetes#12226

I'd like to highlight the conclusion here:

We discussed this today some and while there are some short term advantages, the addition of additional unstructured structure does not dramatically improve the ability of the annotation to fulfill its core purpose - allowing third parties to carry opaque data with arbitrary objects. The json structure of the child object is only truly beneficial in a json serialization - and we already anticipate allowing a formal protobuf representation of all objects. The protobuf representation of these annotations would not be any easier to work with than a map, and would require more complex client logic for handling edge cases (whether numbers are allowed, is their format the same as json or different, etc).

We feel that map[string]string is the simplest construct that accomplishes the goal, and additional structure is only situationally relevant. We should however better document best practices and patterns for working with annotations, planning for versioning, and storing complex data.

We originally went with string->string annotations and found it suitable in use cases, and preferable in most so we decided to stick with it. It's possible that same arguments don't apply here but I think there was a consensus here that a decision one way or the other was relatively inconsequential.

[mikedanese]

FWIW I'm in favor of map of string to string.

[wking]

On Wed, Apr 27, 2016 at 04:18:19PM -0700, Mike Danese wrote:
“I'd like to highlight the conclusion here”

Ahh, thanks for giving this some technical grounding :). It sounds
like the pushback was “what's the point if we pass this information
around in many non-JSON forms (YAML, protobuf, …)?” and “can we do
this without breaking backwards compatibility?”. Constructs supported
by JSON are also supported by YAML and protobuf, so I don't see a
problem with JSON ↔ protobuf (or whatever) also translating the opaque
‘annotations’ value. And we're pre-1.0 here, so I don't see concerns
with backwards compatibility.

And it sounds like JSON-parsing library support wasn't an issue
[1,2,3].

[vbatts]

@wking Sorry, i don't get that impression from you links. But rather that it would cause unwanted complexity.

[vbatts]

@mikedanese Thanks for your input. I'm also in favor of a string-string hashmap, but was willing to be convinced of an ordered array of {"name":"","value":""} objects.

[wking]

On Wed, Apr 27, 2016 at 07:13:04PM -0700, Vincent Batts wrote:

@wking Sorry, i don't get that impression from you links.

Links were for JSON-library support, and relevant snippets from each
are:

• “I know we can trick Go into doing things” 1, presumably with
  interface{}.
• “No big deal for the Java client” 2.
• “It shouldn't be a problem for the Ruby kubeclient client library
  itself, it's pretty flexible” 3.

There are some concerns around backwards compatibility in 3, but if
you start with the understanding that the annotations are completely
opaque, you don't have kubernetes problem with existing tooling that
assumes a string → string map.

[mikedanese]

an ordered array of {"name":"","value":""} objects.

What does the order imply?

Arrays are not friendly to multiple owners. JSON merge patch replaces lists e.g.:

merge_patch(
  {"annotations":{"a":"foo"}}
  {"annotations":{"b":"bar"}}
) returns
  {"annotations":{"a":"foo","b":"bar"}}

but

merge_patch(
  {"annotations":[{"a":"foo"}]}
  {"annotations":[{"b":"bar"}]}
) returns
  {"annotations":[{"b":"bar"}]}

https://tools.ietf.org/html/rfc7396

So the api either need to define a nonstandard merge routine or the user always read before write to ensure that other annotations aren't mucked with (which also requires transaction support).

[vbatts]

@mikedanese good point, which was not where my thought was (regarding outcome of merging). I don't care to solve such problems in this spec.

I was referring to two parts. 1) perhaps users of the annotations field may value having the list be ordered. 2) if there were multiple keys (for whatever awful reason), then it could be more allowable by the data structure.

As it is, I'm more of the mind to have it be a string-string hashmap. I can add wording around lack of order and all keys are unique.

[vbatts]

Updated. PTAL.

[mikedanese]

Are annotations intended to be set by tooling? We can recommend aggressive namespacing and versioning to avoid conflicts. e.g.:

{
  "annotations": {
    "ociplugin.v1alpha1.jenkins.io/started": "1461862737",
    "ociplugin.v1alpha1.jenkins.io/finished": "1461862772",
    "ociplugin.v1alpha1.jenkins.io/started-by": "mikedanese",
    "ociplugin.v1alpha1.jenkins.io/user-agent": "workstation-anacron",
    "clair.beta.coreos.com/started": "1461862940",
    "clair.beta.coreos.com/vulnerability-report": "{\"passed\":true,\"description\":\"I'd say this image is good to go\"}",
    "clair.beta.coreos.com/vulnerability-report-gpg": "owEBRgG5/pANAwACAX9bb5A9WD91AawWYgh0ZXN0LnR4dFciRQFmb28gYmFyCokBHAQAAQIABgUCVyJFAQAKCRB/W2+QPVg/dcreB/9k2E4Ygj7aTkdEWr3geIqG6O8nKe+4ni4BSbK/8aRrFkghccNcw4JbZq/kHBLFQnTccwMr0uSx6zVV7M5sF/7r78rThBB5gDY3y/hPhILztOXCoilb9nAZDsxyz6zK00bMEGCakYLVE8tafZQVYeqkP50J7W3kJfn1maCE+cSmd+nP7UaWWIcSSAiHSvgxqGVMEceahuAMf+iCI5mDtImDFLzUTwz4eNlgOBrq6FIlrbK5cPXLi1ZfZaoUnlRI2E2C26crXFgf61z40vqVoSlu59i0voRkqcB1n3leSGvG88UoOQyv9r7643ZSdWq0H1u1I2Ipoh2pdrexE1HO+vSI",
  }
}

[mikedanese]

I don't care to solve such problems in this spec.

Metapoint: I think it's useful to consider the application of a spec when designing a spec to observe and potentially avoid problems that exist in usage. I wouldn't drive the design with usability issues but perhaps they can serve as tie breakers.

[philips]

@mikedanese +1 on aggressive namespacing.

Wording from appc that can be a source of inspiration:

annotations (list of objects, optional) any extra metadata you wish to add to the image. Each object has two key-value pairs: the name is restricted to the AC Identifier formatting and value is an arbitrary string. Annotation names must be unique within the list. Annotations can be used by systems outside of the ACE (ACE can override). If you are defining new annotations, please consider submitting them to the specification. If you intend for your field to remain special to your application please be a good citizen and prefix an appropriate namespace to your key names.

[vbatts]

Updated. PTAL.

[philips]

LGTM