fix bug: media types mismatch in manifest

[xiekeyang]

It is passed validation on manifest part, even the config mediatype and layer mediatype inside is mismatch.
It had better to add media type item(config and layer) checking on manifest Validate.
bug case:

{
    "annotations": null,
    "config": {
        "digest": "sha256:2b8fd9751c4c0f5dd266fcae00707e67a2545ef34f9a29354585f93dac906749",
        "mediaType": "application/vnd.oci.image.serialization.config.v1+json",
        "size": 1459
    },  
    "layers": [
        {   
            "digest": "sha256:8ddc19f16526912237dd8af81971d5e4dd0587907234be2b83e249518d5b673f",
            "mediaType": "application/vnd.oci.image.serialization.rootfs.tar.gzip",
            "size": 667590
        }   
    ],  
    "mediaType": "application/vnd.oci.image.manifest.v1+json",
    "schemaVersion": 2
}

validate result is OK. But "mediaType": "application/vnd.oci.image.serialization.rootfs.tar.gzip" and "mediaType": "application/vnd.oci.image.serialization.config.v1+json" have been unsupported.

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

[wking]

This is not part of the JSON Schema validation. Your new logic should live in manifest.validate in image/manifest.go.

[wking]

Alternatively, you could adjust image-manifest-schema.json to replace:

"config": {
  "$ref": "content-descriptor.json"
},

with a version of descriptor where mediaType was an enum listing only application/vnd.oci.image.config.v1+json, etc.

Although see here about the spec not actually defining bounds on the referenced media types.

[xiekeyang]

@wking validate --type manifest type seems not to jump into manifest.validate, instead into Validator.Validate directly. So, manifest.validate can't be effective.

[wking]

On Wed, Sep 07, 2016 at 04:00:53AM -0700, xiekeyang wrote:

• if v == MediaTypeManifest {

@wking validate --type manifest type seems not to jump into
manifest.validate, instead into Validator.Validate directly. So,
manifest.validate can't be effective.

? ‘oci-image-tool validate …’ calls both image.Validate(Layout) 1
and schema.MediaType(Manifest|ManifestList|Config).Validate 2. I
expect we should be calling the schema validation from the image
validation functions, but either way, code in the image validation
functions is getting called by ‘oci-image-tool validate …’.

[xiekeyang]

@wking

? ‘oci-image-tool validate …’ calls both image.Validate(Layout) [1]
and schema.MediaType(Manifest|ManifestList|Config).Validate [2]. I
expect we should be calling the schema validation from the image
validation functions, but either way, code in the image validation
functions is getting called by ‘oci-image-tool validate …’.

It is some hard to do based on current code.
What you mentioned [1] is on package order main -> image -> schema;
[2] is main -> schema;
I think it had better to add code to schema package, and better to only one place to be called by up layer. For that I figure out the PR. I tried other way, but it seems to be some brittle.

[wking]

On Thu, Sep 08, 2016 at 12:29:29AM -0700, xiekeyang wrote:

• if v == MediaTypeManifest {

@wking

? ‘oci-image-tool validate …’ calls both image.Validate(Layout) [1]
and schema.MediaType(Manifest|ManifestList|Config).Validate [2]. I
expect we should be calling the schema validation from the image
validation functions, but either way, code in the image validation
functions is getting called by ‘oci-image-tool validate …’.

It is some hard to do based on current code.
What you mentioned [1] is on package order main -> image -> schema;
[2] is main -> schema;
I think it had better to add code to schema package, and better to
only one place to be called by up layer.

I'd rather not tie the JSON Schema too tightly here. Having the
schemas and schema-only tooling in their own layer makes it easier to
share them (and there is a lot of third-party JSON Schema tooling that
can consume them, opencontainers/validator.opencontainers.org#2).

On the other hand, JSON Schema cannot represent all of our
constraints. So I'd like another layer (currently in image/*.go) to
do the full validation. That full-validation layer can save itself a
lot of work by leaning on JSON Schema for part of its validation, but
whether it uses JSON Schema or not is really an internal detail.

So I think we want to use only main → image → schema, but would also
be fine with main → image/validate → schema (if you want to separate
the validation Go code from the unpacking, etc., Go code). I don't
think we want to push a lot of Go logic into the JSON Schema
directory.

[xiekeyang]

Yes. So far I find that schema/*.json should not modified because they only present the generally patten and can't cover all constraints. I'd work out on image code layer.

[xiekeyang]

WIP

[xiekeyang]

Updated， PTAL。
It is some redundant to open and read the manifest file. That is because I follow up the current code blocks, not to refactor them(may to do later).
/cc @runcom @wking @stevvooe @jonboulle

[runcom]

We may not want to error out on invalid media types based on #286 - there should just be something which says unsupported maybe but we need to ease printing to the stdout at various log level with #288.

[xiekeyang]

@runcom I think unsupported warning is inappropriate. media type is standard definition in spec.
Should we pass the validation when it not follow the spec?

[runcom]

@runcom I think unsupported warning is inappropriate. media type is standard definition in spec.
Should we pass the validation when it not follow the spec?

quoting @wking from #286 (comment)

The spec does not currently place restrictions on allowed types for
manifest-list.manifests, manifest.layers, or manifest.config

we should first assure the spec defines or not those constraints

[xiekeyang]

ping @stevvooe @wking @philips @vbatts
If the identifiable media types are constraints? I think we had better to clarify it.

[stevvooe]

MediaTypeDockerImageConfig

[xiekeyang]

Updated

[wking]

On Mon, Sep 12, 2016 at 07:08:34PM -0700, xiekeyang wrote:

If the (identifiable media
types)[https://github.com/opencontainers/image-spec/blob/master/media-types.md#media-types]
are constraints? I think we had better to clarify it.

I agree, but that's a big if. Are we sure we want to require them?
It's a trade-off 1. And we can place different constraints on
images and implementations. For example, perhaps an image MUST NOT
use a media type that's not listed in 2 and an implementation MUST
support those types.

fewer types | media-types.md | more types
--------------------------------+------------------+------------------------
← compliant images | non-compliant images →
← non-compliant implementaions | compliant images →

But I'm nervous about requiring support for non-OCI media types unless
there is a stable, testable spec we can link for the external types.

[xiekeyang]

@wking

For example, perhaps an image MUST NOT
use a media type that's not listed in [2] and an implementation MUST
support those types.

For this I think we have announced in spec Non-Distributable Layers.

As config file, it should be standard format as Container config JSON .

I'm not sure if it covers your implementation case, or can you put some use case? But now any word on mediaType field (in manifest) may be passed, that seems not to be standard.

[wking]

On Tue, Sep 13, 2016 at 12:12:02AM -0700, xiekeyang wrote:

For example, perhaps an image MUST NOT use a media type that's not
listed in [2] and an implementation MUST support those types.

For this I think we have announced in spec Non-Distributable
Layers.

(Non)distributable is about what you can push. I'm suggesting wording
around what an implementation can handle. I've submitted #304 with
explicit wording around my above suggestion (although I've softened
the instance to SHOULD). If/when something like #304 lands, testing
like you're adding here will be much easier to write (since it will be
“rewrite these requirements in Go” and not “invent requirements and
write them in Go” ;).

[xiekeyang]

This is migrated to opencontainers/image-tools/pull/10. Here should be closed.