Skip to content

schema: Limit schemaVersion to 2 and enumerate mediaType#404

Merged
stevvooe merged 3 commits intoopencontainers:masterfrom
wking:schema-version-json-schema
Nov 17, 2016
Merged

schema: Limit schemaVersion to 2 and enumerate mediaType#404
stevvooe merged 3 commits intoopencontainers:masterfrom
wking:schema-version-json-schema

Conversation

@wking
Copy link
Contributor

@wking wking commented Oct 20, 2016

For manifests and manifest lists, to match the requirements from their Markdown definitions.

This was referenced Oct 20, 2016
Closed
Closed
@xiekeyang
Copy link
Contributor

xiekeyang commented Oct 21, 2016

@wking

  1. Do you forget to refresh the generated virtual file system, to see https://github.com/opencontainers/image-spec/blob/master/HACKING.md#virtual-schema-httpfilesystem?
  2. It seems that you want to reject all media types in compatibility-matrix, that may be conflict to our previous conclusion. I worry that we fall into the confusion again that media type should be loose or strict. After read Intra-blob JSON validation correctness and image-spec release policy #406 , I still think this PR is too strict.

@wking
Copy link
Contributor Author

wking commented Oct 21, 2016

On Thu, Oct 20, 2016 at 09:48:41PM -0700, xiekeyang wrote:

  1. Do you forget to refresh the generated virtual file system…

Oops, yeah. Fixed with the just-pushed 00f177b.

  1. It seems that you want to reject all media types in compatibility-matrix…

I think there are two things going on here: what you are and what you reference. #304 is about who you reference, and the pattern is that we require implementations to support appropriate OCI types but allow images and implementations to use other types as well. This PR, on the other hand, is about declaring what you are, and there's only one good answer for that.

An application/vnd.oci.image.manifest.v1+json with application/vnd.docker.container.image.v1+json in config.mediaType is legal (although there may be compliant runtimes that don't support it). An application/vnd.oci.image.manifest.v1+json with application/vnd.docker.distribution.manifest.v2+json in mediaType is not legal.

@wking
Copy link
Contributor Author

wking commented Oct 21, 2016

I've just pushed an additional commit (801699e) fixing the convertFormats direction to make Travis happy, so this stricter checking has already been useful :).

@vbatts
Copy link
Member

vbatts commented Nov 1, 2016

needs a rebase

@wking wking force-pushed the schema-version-json-schema branch from 801699e to c23165c Compare November 1, 2016 17:07
@wking
Copy link
Contributor Author

wking commented Nov 1, 2016

On Tue, Nov 01, 2016 at 07:51:41AM -0700, Vincent Batts wrote:

needs a rebase

Rebased around #413's schema/fs.go bump with 801699e → c23165c.

This was referenced Nov 4, 2016
Merged
Merged
@philips
Copy link
Contributor

philips commented Nov 16, 2016 •
edited by caniszczyk
Loading

LGTM

Conflict though.

Approved with PullApprove

wking added 3 commits November 16, 2016 15:47
@wking
schema: Limit schemaVersion to 2 and enumerate mediaType
5969565 
For manifests and manifest lists, to match the requirements from their
Markdown definitions.

Signed-off-by: W. Trevor King <wking@tremily.us>
@wking
schema/fs.go: Rebuild the schema
4ba6b9d 
Generated with:

  $ make schema-fs

Signed-off-by: W. Trevor King <wking@tremily.us>
@wking
schema/manifest_backwards_compatibility_test: Fix conversion direction
dad01a1 
We want to go from the Docker type (the key) to the OCI type (the
value), not the other way around.

Signed-off-by: W. Trevor King <wking@tremily.us>
@wking wking force-pushed the schema-version-json-schema branch from c23165c to dad01a1 Compare November 16, 2016 23:48
@wking
Copy link
Contributor Author

wking commented Nov 16, 2016

On Wed, Nov 16, 2016 at 02:38:58PM -0800, Brandon Philips wrote:

Conflict though.

Rebased onto master and rebuilt schema/fs.go with c23165c → dad01a1.

It's probably best if you order up all the JSON-Schema-touching PRs
(maybe tag them all?), because all of them are going to conflict on
fs.go.

An alternative would be to ask contributors to not update fs.go and
have the merging maintainer rebuild fs.go after merging the change.
In this case you'd also want to add a Travis line rebuilding fs.go
before running the tests.

@philips
Copy link
Contributor

philips commented Nov 17, 2016 •
edited by caniszczyk
Loading

LGTM

ptal @opencontainers/image-spec-maintainers

Approved with PullApprove

@stevvooe
Copy link
Contributor

stevvooe commented Nov 17, 2016 •
edited by caniszczyk
Loading

LGTM

Approved with PullApprove

@stevvooe stevvooe merged commit ebee8d2 into opencontainers:master Nov 17, 2016
@wking wking mentioned this pull request Nov 17, 2016
Closed
@wking wking deleted the schema-version-json-schema branch January 19, 2017 23:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@wking @xiekeyang @vbatts @philips @stevvooe