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.

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

serialization: add test, make volumes null #151

Closed

Conversation

philips
Copy link
Contributor

@philips philips commented Jun 16, 2016

This is a spec change based on testing live Docker Hub configs. Volume may
be null as found in the hub.docker.com/library/docker image.

This is a continuation of #145

Signed-off-by: Brandon Philips [email protected]

Brandon Philips added 2 commits June 15, 2016 22:48
This adds a test of the schema compatibility by taking the manifest of
library/docker from the docker hub, with sha256 digest verification, and
simply finding/replacing the four constant strings documented in
media-types.md and ensuring that it passes OCI validation.

https://github.com/opencontainers/image-spec/blob/master/media-types.md

This test is functionaly equivalent to downloading a Docker v2.2
manifest into the file `docker-manifest:

```
$ curl -L -H "Authorization: Bearer ..." \
	-H "Accept: application/vnd.docker.distribution.manifest.v2+json" \
	https://registry-1.docker.io/v2/library/docker/manifests/sha256fg:888206c77cd2811ec47e752ba291e5b7734e3ef137dfd222daadaca39a9f17bc > docker-manifest
```

And then running a regex to change `docker.distribution` to `oci.image`
and changing `v2` to `v1`

```
$ cat docker-manifest | sed -e "s%docker\.distribution%oci\.image%g" -e "s%v2%v1%g" > oci-manifest
```

And finally testing the manifest with oci-image-tool:

```
$ oci-image-tool validate oci-manifest
oci-manifest: OK
```

Signed-off-by: Brandon Philips <[email protected]>
This is a spec change based on testing live Docker Hub configs. Volume
may be null as found in the hub.docker.com/library/docker image.

Signed-off-by: Brandon Philips <[email protected]>
@philips
Copy link
Contributor Author

philips commented Jun 16, 2016

Please merge #145 first. I wanted to separate these out because this is a spec change to make the Volumes field OPTIONAL

"$ref": "defs.json#/definitions/mapStringObject"
"oneOf": [
{"$ref": "defs.json#/definitions/mapStringObject"},
{"type": "null"}
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't look like jq ;). Did you run make fmt (with the fix from #148)?

@runcom
Copy link
Member

runcom commented Jun 16, 2016

LGTM (beside @wking nit) (IANAM)

@runcom
Copy link
Member

runcom commented Jun 21, 2016

This can be closed now - #152 has been merged with the commits from here

@vbatts vbatts closed this Jun 21, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants