feat: add config data in image manifest #757
Conversation
|
I think the right behavior is to just drop that field as its not in the spec. EDIT: Ha, it is actually in the spec! https://github.com/opencontainers/image-spec/blob/main/descriptor.md#properties However, i am still not sure if we should keep the data field, we could just drop it and it would work fine. |
|
I originally thought it wasn't in the spec either, but had to look again as well. Turns out it's been in the spec for year! Dropping the field definitely is a solution, but considering it's in the spec, and the work is already done in the pull request, I don't see a good reason to NOT just add it? Unless the implementation is wrong or doesn't follow best practices. If so, please advise. |
|
Still, though, i think we should only inline the config if we see that the base has it too. |
The [OCI spec](https://github.com/opencontainers/image-spec/blob/main/descriptor.md#properties) provides a `data` field in order to embed a copy of an image's config directly into that image's manifest: > data string > > This OPTIONAL property contains an embedded representation of the referenced content. Values MUST conform to the Base 64 encoding, as defined in RFC 4648. The decoded data MUST be identical to the referenced content and SHOULD be verified against the digest and size fields by content consumers. See Embedded Content for when this is appropriate. This is [intended as an optimization](https://github.com/opencontainers/image-spec/blob/main/descriptor.md#embedded-content), in order to reduce the number of network round-trips incurred to pull a container from a remote repository. However, as highlighted in the release announcement for the new [`data` field](https://opencontainers.org/posts/blog/2024-03-13-image-and-distribution-1-1/#data-field), the contexts in which an embedded config is beneficial can be a subtle determination. Further, in the same announcement, the spec clarified that registry implementations usually enforce a [maximum size](https://opencontainers.org/posts/blog/2024-03-13-image-and-distribution-1-1/#manifest-maximum-size) on manifests, an obscure limit which incautious use of the `data` field can trigger. Given the trade-offs dropping any embedded data (e.g. that might have been propagated from a base image) seems the preferable option. The alternative — updating embedded data to match the config on every change, as proposed by bazel-contrib#757 — is less universally correct and only sometimes more performant. Either option would satisfy bazel-contrib#756 .
|
Just ran into this issue today. IMO the |
This PR adds the
datafield to the config of the manifest in the image ofoci_image.Fixes #756