canonicalization: Add recommendations for canonicalization #339

wking · 2016-09-23T04:18:12Z

Spun off from #259, now that we seem to have reached a consensus for “SHOULD canonical JSON” there. I've set this up so we have space to add canonicalization recommendations for other formats, although the only other basic type discussed in this repository is a gzipped tarball and that's more than I want to bite off at the moment ;).

wking · 2016-09-24T17:19:44Z

On Sat, Sep 24, 2016 at 10:04:53AM -0700, Jonathan Boulle wrote:

@@ -0,0 +1,19 @@
+# Canonicalization

This feels a bit weird lacking context - could we not wrap it into
the document talking about the json types?

Canonicalization is a way to avoid redundant blobs for a particular
semantic payload. That idea applies to all media types, it's just
that we currently only have canonicalization advice for JSON. Once we
get canonicalization for gzipped tarballs (or any other OCI-related
types), we can add that here too. But as I said in the topic post
here, I'm not clear enough on what gzipped tarball canonicalization
would look like to add it yet ;).

jonboulle · 2016-09-26T14:10:30Z

OK, that makes sense. How about moving this to a more generic "implementers guide"-type document (as a loose example of this see appc)

wking · 2016-09-26T15:38:50Z

On Mon, Sep 26, 2016 at 07:10:31AM -0700, Jonathan Boulle wrote:

OK, that makes sense. How about moving this to a more generic
"implementers guide"-type document (as a loose example of this see
appc)

I'm ok with that, but I prefer a normative SHOULD to informative
“guidelines, tips and suggested best practices”. I expect the
validation tooling to (optionally) warn about violated SHOULDs but not
about violated informative hints. Do you see other distinctions
between SHOULD and informative hints?

jonboulle

OK, fair point, I think I'm just not quite on board with the wording yet. Added some suggestions.

jonboulle · 2016-10-04T10:46:27Z

canonicalization.md

@@ -0,0 +1,19 @@
+# Canonicalization
+
+One benefit of content-addressable storage is easy deduplication.


Slightly more intro context - something like "One of the goals of the OCI Image Specification is to leverage content-addressable storage (CAS), which provides benefits like easy deduplication"

jonboulle · 2016-10-04T13:00:53Z

canonicalization.md

+
+## JSON
+
+[JSON][] content SHOULD be serialized as [canonical JSON][canonical-json].


JSON content (for example, ...)

JSON content (for example all of the bla bla media types or bla bla manifests) SHOULD be serialized as

jonboulle · 2016-10-04T13:01:49Z

canonicalization.md

+
+One benefit of content-addressable storage is easy deduplication.
+Many images might depend on a particular [layer](layer.md), the there will only be one blob in the [store](image-layout.md).
+However, that same semantic layer serialized slightly differently would have a different hash, and if both versions of the layer are referenced there will be two blobs with the same semantic content.


that same layer, if serialized in a different way, would have an entirely different hash, and hence if both versions of the layer, there will be two underlying blobs in the store with the same semantic comment.

wking · 2016-10-04T14:54:12Z

On Tue, Oct 04, 2016 at 06:02:13AM -0700, Jonathan Boulle wrote:

+One benefit of content-addressable storage is easy deduplication.

Slightly more intro context - something like "One of the goals of
the OCI Image Specification is to leverage content-addressable
storage (CAS), which provides benefits like easy deduplication"

I don't think content-addressability is a goal in itself. It's just a
useful pattern to support for safe, efficient distribution. In
0b0c8e6 → 3eb6af7, I've rebased around #365 and added another
lead-in sentence pointing out that the current OCI spec does leverage
content-addressability. Let me know if you'd like further rewording
on this.

+## JSON
+
+[JSON][] content SHOULD be serialized as [canonical JSON][canonical-json].

JSON content (for example, ...)

This line is a normative SHOULD, and if the PR lands I will file
image-validation code that warns if a JSON blob is not canonical. I'd
rather not confuse it by adding an open-ended “for example”. Maybe
what you're looking for is a trailing line in the lead-in paragraph
like:

The following sections contain canonicalization recommendations for
OCI-related media types.

+However, that same semantic layer serialized slightly differently would have a different hash, and if both versions of the layer are referenced there will be two blobs with the same semantic content.

that same layer, if serialized in a different way, would have an
entirely different hash, and hence if both versions of the layer,
there will be two underlying blobs in the store with the same
semantic comment.

“hence if both versions of the layer” seems to be missing a word or
two. I've dropped my “However”, shifted the difference condition to
the front, and dropped “slightly” in 3eb6af7.

jonboulle · 2016-10-05T12:09:36Z

@wking if someone does an inline review, please reply inline, otherwise it's impossible to follow

jonboulle · 2016-10-05T12:10:02Z

canonicalization.md

+# Canonicalization
+
+OCI Images [are](descriptor.md) [content-addressable](image-layout.md).
+One benefit of content-addressable storage is easy deduplication.


It's already explicitly called out as a goal @ https://github.com/opencontainers/image-spec/blame/e74c6c703315a8d2baaea691b527194377c33a8d/manifest.md#L4

Ah, I stand corrected. I still think the current wording covers that though. Would you still prefer your initial suggestion?

jonboulle · 2016-10-05T12:11:37Z

canonicalization.md

+
+OCI Images [are](descriptor.md) [content-addressable](image-layout.md).
+One benefit of content-addressable storage is easy deduplication.
+Many images might depend on a particular [layer](layer.md), the there will only be one blob in the [store](image-layout.md).


"the there" needs fixing

jonboulle · 2016-10-05T12:12:14Z

media-types.md

@@ -47,3 +47,5 @@ The following figure shows how the above media types reference each other:

 [Descriptors](descriptor.md) are used for all references.
 The manifest list being a "fat manifest" references one or more image manifests per target platform. An image manifest references exactly one target configuration and possibly many layers.
+
+[JSON]: http://json.org/


What's this?

A reference-style link label defining JSON. In the in-flight opencontainers/runtime-spec#584 I change a similar runtime-spec link to point at RFC 7159, and I can do that here too if you like.

OK, fine, I asked the wrong question :/. Let me try again: why is it here? (I don't see any JSON references in this document)

First line in the JSON section ;).

That is a separate file.

Heh, so it is. Thanks for banging my head into that until I understood :p.

jonboulle · 2016-10-05T12:13:46Z

Looks okay overall after a few more fixes

wking · 2016-10-05T16:39:46Z

On Wed, Oct 05, 2016 at 05:09:38AM -0700, Jonathan Boulle wrote:

@wking if someone does an inline review, please reply inline,
otherwise it's impossible to follow

I'll try, but GitHub no longer has a way to do that via email. When I
do reply by email, I'll be sure to quote enough context so my reply
makes sense.

wking · 2016-10-05T16:54:49Z

I've pushed 3eb6af7 → 4729086 which:

Fixed “the there” → “but there” 1
Added a new sentence linking to media-types.md for our JSON
types 2. This feels pretty obvious to me, and the SHOULD
canonicalize also applies to all JSON content regardless of
whether it's an OCI type, so I'm fine dropping the new
sentence. Using a new sentence (instead of @jonboulle's
floated parenthetical) avoids complicating the normative
SHOULD.

jonboulle · 2016-10-05T16:59:45Z

canonicalization.md

+
+* [Go][]: [github.com/docker/go][], which claims to implement [canonical JSON][canonical-json] except for Unicode normalization.
+
+Of the [OCI Image Format Specification media types](media-types.md), all the types ending in `+json` contain JSON content.


Move this above the "Implementations" imo.

wking · 2016-10-05T17:07:31Z

I've pushed 4729086 → b695fb3 shifting the media-types JSON link up
before the list of implementations 1.

Spun off from [1], now that we seem to have reached a consensus for "SHOULD canonical JSON" there. I've set this up so we have space to add canonicalization recommendations for other formats, although the only other basic type discussed in this repository is a gzipped tarball and that's more than I want to bite off at the moment ;). [1]: opencontainers#259 Subject: manifest json fields order Signed-off-by: W. Trevor King <[email protected]>

wking · 2016-10-05T17:12:22Z

I've pushed b695fb3 → 1685bae dropping the JSON link label from
media-types.md 1. No idea what I was thinking with that, but thanks
to @jonboulle for pointing it out to me (repeatedly) until I caught
on.

jonboulle · 2016-10-05T17:42:06Z

LGTM for now

vbatts · 2016-10-06T16:44:02Z

while i'm not in disagreement here, i fell like this is describing something without describing anything.
Like is canonical serialization, the use of symlinks to deduplicate blobs? or something else?

wking · 2016-10-06T16:50:36Z

On Thu, Oct 06, 2016 at 09:44:02AM -0700, Vincent Batts wrote:

while i'm not in disagreement here, i fell like this is describing
something without describing anything. Like is canonical
serialization, the use of symlinks to deduplicate blobs? or
something else?

How would symlinks deduplicate blobs? Maybe if you had a cluster of
image-layouts? If so, that's a CAS-engine optimization (reducing the
disk space used to store a set of blobs by leaning on other
image-layouts). This PR is about optimizing the CAS content
(reducing the number of blobs you need to store). I expect both are
useful, but advice about CAS-engine optimization should probably be
part of a separate issue/PR.

vbatts · 2016-10-06T17:37:08Z

LGTM

stevvooe · 2016-10-12T21:21:25Z

I know we've already merged this, but how can Canonical JSON be considered a viable specification when the only reference is to a wiki page that could be changed at any time? I brought this up at the time when we started using canonical json in docker. Docker Distribution attempts to take this further, but canonical json is insufficient for this purpose.

I am really confused as to why we consider Canonical JSON a viable specification.

wking · 2016-10-12T22:12:40Z

On Wed, Oct 12, 2016 at 02:21:25PM -0700, Stephen Day wrote:

I know we've already merged this, but how can Canonical JSON be
considered a viable specification when the only reference is to a
wiki page that could be changed at any time?

I agree that a more formal spec would be nice 1. If you have one,
I'm happy to link to it instead. But this PR landed a SHOULD. In the
event that the wiki mutates under us it isn't a disaster, we just
start warning about any incompatibilities.

And the last change to the spec itself was in 2008 2, so it's fairly
stable.

I brought this up at the time when we started using canonical json
in docker. Docker
Distribution
attempts to take this further, but canonical json is insufficient
for this purpose.

The goal is to reduce the number of blobs carrying the same semantic
content. You don't have to have a complete canonicalization to get
some reduction. In practice, I think the canonical JSON spec will be
sufficient to avoid most duplication. And 3 would be fine too (is
that spec versioned independently of docker/distribution as a whole?).
But either of those are better at reducing duplication than not
providing any canonicalization advice at all.

wking mentioned this pull request Sep 23, 2016

manifest json fields order #259

Closed

jonboulle requested changes Oct 4, 2016

View reviewed changes

wking force-pushed the should-canonical-json branch from 0b0c8e6 to 3eb6af7 Compare October 4, 2016 14:52

jonboulle reviewed Oct 5, 2016

View reviewed changes

wking force-pushed the should-canonical-json branch from 3eb6af7 to 4729086 Compare October 5, 2016 16:51

jonboulle reviewed Oct 5, 2016

View reviewed changes

wking force-pushed the should-canonical-json branch from 4729086 to b695fb3 Compare October 5, 2016 17:04

wking force-pushed the should-canonical-json branch from b695fb3 to 1685bae Compare October 5, 2016 17:08

jonboulle approved these changes Oct 5, 2016

View reviewed changes

vbatts merged commit f94caad into opencontainers:master Oct 6, 2016

wking deleted the should-canonical-json branch October 18, 2016 22:27

wking mentioned this pull request Oct 25, 2016

Created should be omitted from image json #399

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

canonicalization: Add recommendations for canonicalization #339

canonicalization: Add recommendations for canonicalization #339

wking commented Sep 23, 2016

wking commented Sep 24, 2016

jonboulle commented Sep 26, 2016

wking commented Sep 26, 2016

jonboulle left a comment

jonboulle Oct 4, 2016

jonboulle Oct 4, 2016

jonboulle Oct 5, 2016

jonboulle Oct 4, 2016

wking commented Oct 4, 2016

jonboulle commented Oct 5, 2016

jonboulle Oct 5, 2016

wking Oct 5, 2016

jonboulle Oct 5, 2016

jonboulle Oct 5, 2016

wking Oct 5, 2016

jonboulle Oct 5, 2016

wking Oct 5, 2016

jonboulle Oct 5, 2016

wking Oct 5, 2016

jonboulle commented Oct 5, 2016

wking commented Oct 5, 2016

wking commented Oct 5, 2016

jonboulle Oct 5, 2016

wking commented Oct 5, 2016

wking commented Oct 5, 2016

jonboulle commented Oct 5, 2016 •

edited by caniszczyk

Loading

vbatts commented Oct 6, 2016

wking commented Oct 6, 2016

vbatts commented Oct 6, 2016 •

edited by caniszczyk

Loading

stevvooe commented Oct 12, 2016

wking commented Oct 12, 2016

		@@ -0,0 +1,19 @@
		# Canonicalization

		One benefit of content-addressable storage is easy deduplication.


		## JSON

		[JSON][] content SHOULD be serialized as [canonical JSON][canonical-json].


		* [Go][]: [github.com/docker/go][], which claims to implement [canonical JSON][canonical-json] except for Unicode normalization.

		Of the [OCI Image Format Specification media types](media-types.md), all the types ending in `+json` contain JSON content.

canonicalization: Add recommendations for canonicalization #339

canonicalization: Add recommendations for canonicalization #339

Conversation

wking commented Sep 23, 2016

wking commented Sep 24, 2016

jonboulle commented Sep 26, 2016

wking commented Sep 26, 2016

jonboulle left a 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 commented Oct 4, 2016

jonboulle commented Oct 5, 2016

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonboulle commented Oct 5, 2016

wking commented Oct 5, 2016

wking commented Oct 5, 2016

Choose a reason for hiding this comment

wking commented Oct 5, 2016

wking commented Oct 5, 2016

jonboulle commented Oct 5, 2016 • edited by caniszczyk Loading

vbatts commented Oct 6, 2016

wking commented Oct 6, 2016

vbatts commented Oct 6, 2016 • edited by caniszczyk Loading

stevvooe commented Oct 12, 2016

wking commented Oct 12, 2016

jonboulle commented Oct 5, 2016 •

edited by caniszczyk

Loading

vbatts commented Oct 6, 2016 •

edited by caniszczyk

Loading