define media type for oci-layout #610

xiekeyang · 2017-03-13T04:13:12Z

Following #579.

To add a new specific media type for oci-layout, serving over its transportation.

Signed-off-by: xiekeyang xiekeyang@huawei.com

vbatts · 2017-03-13T12:56:04Z

or should we just give it a mediaType, @stevvooe ?

jonboulle · 2017-03-13T13:38:08Z

That seems simpler than going through these contortions.

stevvooe · 2017-03-13T19:12:59Z

@vbatts @jonboulle I agree that is simpler.

We do need to be careful about contexts under which a mediatype is encountered. For example, I would never expect to encounter this in the course of arbitrary descriptors, so we may want to add some clarification that you only need to handle these when it is an oci layout file.

xiekeyang · 2017-03-14T09:56:04Z

@vbatts @jonboulle @stevvooe

I change the clarification on d678b9c

I prefer to naming the file as oci-layout, instead of providing a media type. In layout use case of a variety of different transport mechanisms, we check other JSON objects through media type because they are CAS blobs, but image-layout is always on the top of image dir, so that users can address it through name of oci-layout. And I think it might be applicable on local or repository endpoint.

If it is reasonable?

wking · 2017-03-14T10:01:41Z

image-layout.md

@@ -20,6 +22,7 @@ The image layout is as follows:
    - See [blobs](#blobs) section
 - `oci-layout` file
    - It MUST exist
+    - Its name MUST be `oci-layout`


I think this is already covered by the earlier - oci-layout file line. Note that we have no Its name MUST be blobs entry under - blobs directory above (for the same reason).

jonboulle · 2017-03-14T11:54:12Z

image-layout.md

@@ -2,6 +2,8 @@

 The OCI Image Layout is a slash separated layout of OCI content-addressable blobs and [location-addressable](https://en.wikipedia.org/wiki/Content-addressable_storage#Content-addressed_vs._location-addressed) references (refs).
 This layout MAY be used in a variety of different transport mechanisms: archive formats (e.g. tar, zip), shared filesystem environments (e.g. nfs), or networked file fetching (e.g. http, ftp, rsync).
+This section defines none of media type for image layout.


is this necessary? its inclusion just confuses me. a layout as described here couldn't really have a media type

xiekeyang · 2017-03-15T01:47:55Z

@jonboulle @wking
I had wanted to present 3 supplements according to previous confusion and discussion.

oci-layout is unchangeable. (It seems redundant to add any comment, so that I removed it)
This section defines none of media type. I think it might make sense, for this JSON object has seem to confuse developers try to find its media type. Then I want to clear it here.
How image provider or consumer to address/handle oci-layout, this seems to be acceptable.

xiekeyang · 2017-03-15T01:50:45Z

@jonboulle @wking These clarification might be not graceful, could you suggest some better words? Thanks.

wking · 2017-03-15T03:52:18Z

On Tue, Mar 14, 2017 at 06:50:47PM -0700, xiekeyang wrote: @jonboulle @wking These clarification might be not graceful, could you suggest some better words? Thanks.

I don't think it's a good idea to declare that there is *no* media type for oci-layout. If you're serving an image-layout over HTTP, you'll want to put something in the Content-Type header for that file. If we don't give it a more specific media type, then it will default to being application/json [1]. So there are two options for the type: a. Declare a specific media type for the oci-layout file (e.g. application/vnd.oci.image.layout.v1+json). b. Use application/json. With (a) we can continue to use our current media-type based spec validation [2,3] without trouble. With (b), we have two validation options: i. Use a dummy “media type” like oci-layout [4,5]. ii. Add a new field (independent of or supporting the media type) to select which schema should be used to validate the content (e.g. json,title=OCI%20Layout&mediatype=application/json&schema=oci%2Dlayout). I prefer (a), (b.ii) is ok, and (b.i) (the route taken in #519, #579, and this PR as of c0362a3) just seems confusing to me. So if you want to make it clear that we are declaring no special media type for this file (b), I'd suggest a line like: This specification does not declare a specific media type for `oci-layout`, so the media type for `oci-layout` is the generic [`application/json`][1]. But if we go that route, I'd strongly recommend we stop pretending to be using media types to select the appropriate schema for validation [7]. And that we clearly document the identifiers that we do use [3], whatever we end up calling them (“schema identifiers”?). [1]: https://tools.ietf.org/html/rfc7159#section-1.2 [2]: https://github.com/opencontainers/image-spec/blob/v1.0.0-rc5/schema/spec_test.go#L80 [3]: https://github.com/opencontainers/image-spec/blob/v1.0.0-rc5/schema/schema.go#L23-L30 [4]: https://github.com/opencontainers/image-spec/blob/v1.0.0-rc5/schema/schema.go#L29 [5]: https://github.com/opencontainers/image-spec/blob/v1.0.0-rc5/specs-go/v1/layout.go#L19 [6]: https://github.com/opencontainers/image-spec/pull/579/files#diff-99bfcb610f238b942638d49d1ff2eaf4R137 [7]: https://github.com/opencontainers/image-spec/blob/v1.0.0-rc5/schema/schema.go#L23

xiekeyang · 2017-03-15T07:22:35Z

@wking I'm not opposing specific oci-layout media type strongly. I'm just considering #610 (comment) .
Spec declares that This layout MAY be used in a variety of different transport mechanisms. In this case if it really need media type? I suspect it. For validation purpose, I have said some times that we should improve validation code, instead set up a media type. (Maybe in future we will need specific media type, I'm not sure.)

So I like the language:

This specification does not declare a specific media type for `oci-layout`, so it is the generic [`application/json`](https://tools.ietf.org/html/rfc7159#section-1.2).

This patch I would like to achieve in spec, then I will follow up patch on validation part for this.

xiekeyang · 2017-03-15T07:26:08Z

Updated in 70d7a61

jonboulle · 2017-03-15T16:52:31Z

I'm lost. I thought we established we would just add the media type.

Signed-off-by: xiekeyang <xiekeyang@huawei.com>

xiekeyang · 2017-03-16T08:18:16Z

Here are 3 commits.

According to most people's suggestion, this add specific media type for image layout in b7e8d5c

After that, image layout key word in schema should be updated accordingly, which I fix in b8e832d . Here I didn't remove ImageLayoutFile even it is not used currently. That is because the name (oci-layout) will be use by image-tools or customers when they pack the OCI image.

During making lint I encounter failure. It not caused the above commit, but failure under any commit. So I've to extent the deadline of lint fro 10s to 15s, in eb5fcf8 .

@opencontainers/image-spec-maintainers PTAL, thank so much!

jonboulle · 2017-03-16T10:49:44Z

image-layout.md

@@ -2,6 +2,7 @@

 * The OCI Image Layout is a slash separated layout of OCI content-addressable blobs and [location-addressable](https://en.wikipedia.org/wiki/Content-addressable_storage#Content-addressed_vs._location-addressed) references (refs).
 * This layout MAY be used in a variety of different transport mechanisms: archive formats (e.g. tar, zip), shared filesystem environments (e.g. nfs), or networked file fetching (e.g. http, ftp, rsync).
+* This section defines the `application/vnd.oci.image.layout.v1+json` [media type](media-types.md).


I think this line should be moved down here to the oci-layout-file section. Otherwise it seems to imply that the entire document is talking about this media type.

I think this line should be moved down here to the oci-layout-file section. Otherwise it seems to imply that the entire document is talking about this media type.

I'm a little doubting on it. It seems had better to be aligned with other spec docs (manifest, index, config..), in which define ... media type is put on the first section.

I see what you mean, but would also argue that it's more appropriate in those other documents because there the bulk of the text is describing the type, whereas in this case it's only a small part of the document. But ¯_(ツ)_/¯ , other maintainers can weigh in

Also, if we created a mediatype for the layout archive, it would probably be this, so this is very confusing.

stevvooe · 2017-03-16T20:01:21Z

Media type needs to be registered in https://github.com/opencontainers/image-spec/blob/master/media-types.md, as well.

xiekeyang · 2017-03-16T23:38:30Z

@stevvooe

Media type needs to be registered in https://github.com/opencontainers/image-spec/blob/master/media-types.md, as well.

I've registered it in b7e8d5c#diff-691102624fa3cbd2452c05ada4a3f91cR6 . Do you mean that way?

xiekeyang · 2017-03-17T08:50:21Z

@opencontainers/image-spec-maintainers I rename the oci-layout media type to application/vnd.oci.layout.version.v1+json. That express precisely the oci-layout itself, to avoid confusing customers. PTAL, thanks. Related changeset is done in media-type.md, schema and spec/v1.

stevvooe · 2017-03-17T20:38:24Z

@xiekeyang Thanks! application/vnd.oci.layout.version.v1+json is much clearer. Perhaps, pplication/vnd.oci.layout.header.v1+json might be even more accurate?

@jonboulle @vbatts Thoughts?

To add a new specific media type for oci-layout, serving over its transportation. Signed-off-by: xiekeyang <xiekeyang@huawei.com>

As we have defined media type for oci-layout, now we can use it as the key word in schema map. Signed-off-by: xiekeyang <xiekeyang@huawei.com>

xiekeyang · 2017-03-18T11:03:47Z

Perhaps, pplication/vnd.oci.layout.header.v1+json might be even more accurate?

I like it! related name is updated, PTAL.

jonboulle · 2017-03-20T13:26:51Z

sure, this is fine with me.

jonboulle · 2017-03-20T13:26:54Z

lgtm

vbatts · 2017-03-20T14:07:47Z

image-layout.md


 ### oci-layout Example

-```json,title=OCI%20Layout&mediatype=oci%2Dlayout
+```json,title=OCI%20Layout&mediatype=application/vnd.oci.layout.header.v1%2Bjson


will the TestValidateImageLayout in ./schema/spec_test.go test this now?

We have been testing this example since #579:

$ git describe v1.0.0-rc5-68-gf59a917 $ sed -i 's/imageLayoutVersion/iLV/' image-layout.md $ go test ./schema/spec_test.go { "iLV": "1.0.0" } --- --- FAIL: TestValidateImageLayout (0.00s) spec_test.go:190: invalid oci-layout OCI Layout spec_test.go:100: imageLayoutVersion: imageLayoutVersion is required …

What this PR does is give us a better identifier for this validator (application/vnd.oci.layout.header.v1+json) than the current one (oci-layout).

vbatts · 2017-03-20T18:40:37Z

LGTM

wking reviewed Mar 14, 2017

View reviewed changes

jonboulle reviewed Mar 14, 2017

View reviewed changes

xiekeyang changed the title ~~oci-layout is not media type~~ define media type for image layout Mar 16, 2017

Extension of deadline in make lint

eb5fcf8

Signed-off-by: xiekeyang <xiekeyang@huawei.com>

jonboulle reviewed Mar 16, 2017

View reviewed changes

xiekeyang changed the title ~~define media type for image layout~~ define media type for oci-layout Mar 17, 2017

xiekeyang added 2 commits March 18, 2017 18:42

define media type for oci-layout

91ab522

To add a new specific media type for oci-layout, serving over its transportation. Signed-off-by: xiekeyang <xiekeyang@huawei.com>

media type of oci-layout as key word for schema map

a7a2073

As we have defined media type for oci-layout, now we can use it as the key word in schema map. Signed-off-by: xiekeyang <xiekeyang@huawei.com>

vbatts reviewed Mar 20, 2017

View reviewed changes

vbatts merged commit d24a3c9 into opencontainers:master Mar 20, 2017

vbatts mentioned this pull request May 19, 2017

Bump version to rc6 #681

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

define media type for oci-layout #610

define media type for oci-layout #610

xiekeyang commented Mar 13, 2017 •

edited

Loading

vbatts commented Mar 13, 2017

jonboulle commented Mar 13, 2017

stevvooe commented Mar 13, 2017

xiekeyang commented Mar 14, 2017

wking Mar 14, 2017

jonboulle Mar 14, 2017

jonboulle Mar 14, 2017

xiekeyang commented Mar 15, 2017

xiekeyang commented Mar 15, 2017

wking commented Mar 15, 2017 via email •

edited

Loading

xiekeyang commented Mar 15, 2017

xiekeyang commented Mar 15, 2017

jonboulle commented Mar 15, 2017

xiekeyang commented Mar 16, 2017 •

edited

Loading

jonboulle Mar 16, 2017

xiekeyang Mar 16, 2017

jonboulle Mar 16, 2017

stevvooe Mar 16, 2017

stevvooe commented Mar 16, 2017

xiekeyang commented Mar 16, 2017

xiekeyang commented Mar 17, 2017

stevvooe commented Mar 17, 2017

xiekeyang commented Mar 18, 2017

jonboulle commented Mar 20, 2017

jonboulle commented Mar 20, 2017 •

edited by caniszczyk

Loading

vbatts Mar 20, 2017

wking Mar 20, 2017

vbatts Mar 20, 2017

vbatts commented Mar 20, 2017 •

edited by caniszczyk

Loading

define media type for oci-layout #610

define media type for oci-layout #610

Conversation

xiekeyang commented Mar 13, 2017 • edited Loading

vbatts commented Mar 13, 2017

jonboulle commented Mar 13, 2017

stevvooe commented Mar 13, 2017

xiekeyang commented Mar 14, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xiekeyang commented Mar 15, 2017

xiekeyang commented Mar 15, 2017

wking commented Mar 15, 2017 via email • edited Loading

xiekeyang commented Mar 15, 2017

xiekeyang commented Mar 15, 2017

jonboulle commented Mar 15, 2017

xiekeyang commented Mar 16, 2017 • edited Loading

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

stevvooe commented Mar 16, 2017

xiekeyang commented Mar 16, 2017

xiekeyang commented Mar 17, 2017

stevvooe commented Mar 17, 2017

xiekeyang commented Mar 18, 2017

jonboulle commented Mar 20, 2017

jonboulle commented Mar 20, 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

vbatts commented Mar 20, 2017 • edited by caniszczyk Loading

xiekeyang commented Mar 13, 2017 •

edited

Loading

wking commented Mar 15, 2017 via email •

edited

Loading

xiekeyang commented Mar 16, 2017 •

edited

Loading

jonboulle commented Mar 20, 2017 •

edited by caniszczyk

Loading

vbatts commented Mar 20, 2017 •

edited by caniszczyk

Loading