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

Proposal: Resolve contradiction between image index and image layout mediaTypes #870

Open
nishakm opened this issue Oct 4, 2021 · 5 comments

Comments

@nishakm
Copy link
Contributor

nishakm commented Oct 4, 2021

Currently the image index says: Future versions of the spec MAY use a different mediatype (i.e. a new versioned format). An encountered mediaType that is unknown to the implementation MUST be ignored.

Also, the image layout includes a different mediaType.

This is a contradiction as the current version of the spec says the only mediaType allowed in an image index is application/vnd.oci.image.manifest.v1+json and application/vnd.oci.image.index.v1+json and the image layout has an errant application/xml. This is confusing to readers of the spec. Perhaps remove "Future versions" and outright say that implementers MAY use other mediaTypes.

@nishakm
Copy link
Contributor Author

nishakm commented Oct 4, 2021

@jonjohnsonjr thoughts?

@jonjohnsonjr
Copy link
Contributor

This is a contradiction as the current version of the spec says the only mediaType allowed in an image index is application/vnd.oci.image.manifest.v1+json and application/vnd.oci.image.index.v1+json

I read this differently. The spec says (emphasis mine):

Implementations MUST support at least the following media types:

  • application/vnd.oci.image.manifest.v1+json

Also, implementations SHOULD support the following media types:

  • application/vnd.oci.image.index.v1+json (nested index)

Image indexes concerned with portability SHOULD use one of the above media types.

IMO, this means that application/vnd.oci.image.manifest.v1+json is required and application/vnd.oci.image.index.v1+json is recommended. If you want your index to be broadly supported, you should use one of these two media types.

This bit explains that an index containing other media types are not an error:

Future versions of the spec MAY use a different mediatype (i.e. a new versioned format). An encountered mediaType that is unknown to the implementation MUST be ignored.

So I don't see the contradiction.

@nishakm
Copy link
Contributor Author

nishakm commented Oct 4, 2021

This bit explains that an index containing other media types are not an error:

Future versions of the spec MAY use a different mediatype (i.e. a new versioned format). An encountered mediaType that is unknown to the implementation MUST be ignored.

So I don't see the contradiction.

To me, "Future versions of the spec" means "the current version of the spec DOES NOT support mediaTypes other than manifest and index". It would be nice to either correct this phrasing, or remove the application/xml inclusion in the example.

@sudo-bmitch
Copy link
Contributor

The key phrase to me is:

An encountered mediaType that is unknown to the implementation MUST be ignored.

Which in an ideal world means the server would not throw an error, and accept it without any special validation. The manifest digest that descriptor points to should not be GC'd. And clients that don't know how to use that media type would search the index for a media type they do know how to handle.

To me this is giving implementers advance notice of where future expansion in the spec is expected, and where users may add their custom logic, so we can design tools to not fail when this occurs. This is the Index equivalent of the unknown json fields we'd see in an Image, and follows the extensibility goal.

@sajayantony
Copy link
Member

The fact that application/xml exists in the example actually shows that implementations SHOULD ignore when coming across an unknown mediaType. There are a few intereseting examples . One is buildkit cache which actually usese descriptors which are blobs and not a oci image manifest. In ACR we went ahead and supported this use case for buildkit.

moby/buildkit#2251 shows an example manifest.


{
    "schemaVersion": 2,
    "mediaType": "application/vnd.oci.image.index.v1+json",
    "manifests": [
        {
            "mediaType": "application/vnd.oci.image.layer.v1.tar+gzip",
            "digest": "sha256:136482bf81d1fa351b424ebb8c7e34d15f2c5ed3fc0b66b544b8312bda3d52d9",
            "size": 2427917,
            "annotations": {
                "buildkit/createdat": "2021-07-02T17:08:09.095229615Z",
                "containerd.io/uncompressed": "sha256:3461705ddf3646694bec0ac1cc70d5d39631ca2f4317b451c01d0f0fd0580c90"
            }
        },
        ...
        {
            "mediaType": "application/vnd.buildkit.cacheconfig.v0",
            "digest": "sha256:7aa23086ec6b7e0603a4dc72773ee13acf22cdd760613076ea53b1028d7a22d8",
            "size": 1654
        }
    ]
}

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

No branches or pull requests

4 participants