We now have at least two situations we need to modify the spec. We made the tradeoff to have Spec as immutable. The missing piece is actually transfer Spec into SpecBuilder and consume the original Spec. Unfortunately, the derive-builder lib we use don't generate the From methods automatically, which I think they should. Otherwise, immutable structure becomes really hard to use. Most of the functional programming languages implement this transparently in the runtime with copy on write, but here we have to do this ourselves. There is an PR (https://github.com/colin-kiegel/rust-derive-builder/pull/215) in the derive-builder lib, but looks like it's not merging anytime soon based on the conversation. So for the moment, let's implement this and any other struct in the spec that we need to modify.

Fix https://github.com/containers/youki/issues/351.

I think we should set up a basic CI pipeline for the crate, which does:

• [x] build
• [x] unit test including code coverage
• [x] clippy lint
• [x] rustfmt
• [x] build docs and push to gh-pages branch
• [x] crates.io publishing

Changes:

• https://github.com/containers/oci-spec-rs/pull/87
• https://github.com/containers/oci-spec-rs/pull/89
• https://github.com/containers/oci-spec-rs/pull/90
• https://github.com/containers/oci-spec-rs/pull/91

Hi, @rhatdan, @saschagrunert

I'd like to invite @Furisto to the containers member. WDYT? He is a active contributor of containers/oci-spec-rs, containers/youki and `containers/containrs . He is youki's number one contributor. Of course, he has interested in containers member. There is no particular change, but I do not think there is any room for denial in my opinion for him to become a member of this group.

Closes https://github.com/containers/oci-spec-rs/issues/105.

I chose to_v2s2 over the shorter to_docker_v2s2 after I noted that v2s2 is the shorthand used by podman. But happy to change to the more explicit to_docker_v2s2 is you feel that is appropriate.

https://github.com/containers/oci-spec-rs/blob/2440343d3433762c359c7a096648ca685f8ddd97/src/image/config.rs#L189

We can't just assume the world is Linux/x86_64 by default. (And definitely not x86_64 :wink: )

I have this code in ostree-rs-ext: https://github.com/ostreedev/ostree-rs-ext/blob/bb4423d5af109cf6268e91c77cc4c9ca850e579a/lib/src/container/ocidir.rs#L26

WDYT about having this code in this crate? Alternatively, I think we should drop the default() method.

Or in theory, these helper methods could go in some other containers/ related crate? I hesitate at the overhead for that. Maybe we introduce a oci_spec::util module?

Part of https://github.com/containers/oci-spec-rs/issues/86

An obvious operation to support is generating a derived container image. Here we want to take some existing data such as the image manifest and configuration, and append to key fields such as the image layers, the config diffids and the history.

Another common operation is adding/changing labels.

Having to create a deep-copy of all this data is unfortunate. I am not very concerned about performance here, but it's also an ergonomic hit.

Obviously giving raw &mut access to fields means that it goes "behind the back" of the derive builder, and if we ever started trying to use more advanced things like validation, that wouldn't work.

Honestly, I am increasingly convinced the builder thing is just not worth it and it'd be better to have plain structs with pub fields. But for now, let's allow direct mutation.

I know the builder pattern has been debated many times here. This is quite similar to https://github.com/containers/oci-spec-rs/pull/74

It was concluded to add setters. But, I still see two (related) issues:

Fallible construction and required fields

Using .build() is a fallible operation because some fields are required. This forces use of unwrap(). I think instead we should add methods that take all the required fields to start. This is much like how std::process::Command::new() requires a single string - the binary to run. Then converting the builder into a final struct can be an infallible operation.

What tripped me up twice is that it's needed to manually set .schema_version() e.g. on ImageManifest. I caught this in unit tests (so not the end of the world or anything) but in Rust we can do this stuff at compile time with the type system.

Mutation requires deep-copying

An operation like appending a layer requires deep-copying 3 Vec instances - the layers Vec and the rootfs and history in the config struct.

This isn't terrible, but WDYT about adding a method like this?

diff --git a/src/image/manifest.rs b/src/image/manifest.rs
index 7fbcef49..d4a09510 100644
--- a/src/image/manifest.rs
+++ b/src/image/manifest.rs
@@ -164,6 +164,10 @@ impl ImageManifest {
     pub fn to_writer_pretty<W: Write>(&self, writer: &mut W) -> Result<()> {
         to_writer(&self, writer, true)
     }
+
+    pub fn push_layer(&mut self, blob: Descriptor) {
+        self.layers.push(blob)
+    }
 }
 
 #[cfg(test)]

Or to summarize, short term I think I'm arguing to keep the builder pattern, but add helper methods.

Longer term, it's not really clear to me that all this builder stuff is adding significant value over just having a crate with struct types where all the fields are pub, and helper methods on top of that.

Make the setter method available as a library. Some of them already provide setter methods. We will use this PR as a model to provide setter methods for other structures as well. https://github.com/containers/oci-spec-rs/pull/76

As discussed in #74, this will greatly reduce the need to copy spec/mount when modifying these structures. Leaving the rest as is at the moment. Hopefully, this is enough.

Although spec says that detail field in ErrorInfo is string with unstructured data it seems that all registries make detail field an object.

For example, Dockerhub returns this for unknown manifest:

{
    "errors": [
        {
            "code": "MANIFEST_UNKNOWN",
            "message": "manifest unknown",
            "detail": {
                "Tag": "lates"
            }
        }
    ]
}

Quay:

{
    "errors": [
        {
            "code": "MANIFEST_UNKNOWN",
            "detail": {},
            "message": "manifest unknown"
        }
    ]
}

Microsoft's MCR :

{
    "errors": [
        {
            "code": "MANIFEST_UNKNOWN",
            "message": "manifest tagged by \"lates\" is not found",
            "detail": {
                "Tag": "lates"
            }
        }
    ]
}

This results in failure to deserialize any of the above ErrorResponse structs.

Is this the problem of registry implementers, this library or the spec itself?

Motivation

OCI Artifact manifest specifiaction will be introduced in OCI image spec v1.1.0 (currently rc2) with referrer API in OCI distribution spec in order to store non-image artifacts, e.g. helm chart or SBOM (Software Bill Of Materials) by sigstore/cosign into OCI registry.

Rationale and alternatives

There are two artifact manifest specifications:

• OCI Artifact manifest application/vnd.oci.artifact.manifest.v1+json definition by OCI image spec
  • This is still in draft, introduced in OCI image spec 1.1.0-rc1
  • Announcing Docker Hub OCI Artifacts Support
• ORAS Artifact manifest application/vnd.cncf.oras.artifact.manifest.v1+json by oras-project/artifact-spec
  • This is experimentally supported by Azure container registry

As far as I studied (https://github.com/termoshtt/ocipkg/issues/78), ORAS project has started specifying the artifact manifest, and created own manifest "ORAS Artifact manifest" as pre-draft for submitting to OCI. It has been submitted to OCI as "OCI Artifact manifest" with some fixes, and merged to OCI image spec at 1.1.0-rc1.

Unresolved questions

IANA media type

Because OCI Artifact stores several types of artifact, it stores the media type of its artifact, e.g. application/gzip when storing *.gz file. About this media type slot, OCI Artifact manifest specification says:

https://github.com/opencontainers/image-spec/blob/main/artifact.md

This property SHOULD be used and contain the mediaType of the referenced artifact. If defined, the value MUST comply with RFC 6838, including the naming requirements in its section 4.2, and MAY be registered with IANA.

The media types registered by IANA (Internet Assigned Numbers Authority) is listed in https://www.iana.org/assignments/media-types/media-types.xhtml . I guess it will be updated frequently (last update is 2022-11-10). Since the media type always can be represented by a String and be managed by oci_spec::MediaType::Other, however, I hope this also be statically typed. I experimentally created iana-media-types crate which translate the media type list in IANA into Rust code automatically. I am planning to re-generate this crate using GitHub Actions as upstream IANA definition upgrades.

So my question is

• Using iana-media-types crate is acceptable in terms of managing oci_spec crate?

I willing to integrate iana-media-types crate into this repository, or transfer current repository.