-
Notifications
You must be signed in to change notification settings - Fork 670
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
Artifacts clarification #1141
base: main
Are you sure you want to change the base?
Artifacts clarification #1141
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
@@ -1,6 +1,30 @@ | ||||||||||
# Guidance for Artifacts Authors | ||||||||||
|
||||||||||
Content other than OCI container images MAY be packaged using the image manifest. | ||||||||||
When this is done, the `config.mediaType` value should not be a known OCI image config [media type](media-types.md). | ||||||||||
Historically, due to registry limitations, some tools have created non-OCI conformant artifacts using the `application/vnd.oci.image.config.v1+json` value for `config.mediaType` and values specific to the artifact in `layer[*].mediaType`. | ||||||||||
## Artifacts and Images | ||||||||||
|
||||||||||
This specification is primarily concerned with packaging two kinds of content: Artifacts and Images. | ||||||||||
Both are representing using a [manifest](manifest.md). | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
This sentence repeats the one this commit moves down... maybe just add the missing link to the prior original block? |
||||||||||
Images are defined in this specification as conformant content with a conformant [config](config.md), processed according to [conversion.md](conversion.md) to derive a [runtime-spec][] configuration blob. | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is probably the wrong place to define what an image is.. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's being defined in contrast to Artifacts for now... I think it's okay; this isn't the best place to have this long-term, but for now this seems like a relevant place to try and explain what the difference between an image and an artifact is. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fair..
Suggested change
|
||||||||||
Conversely, an Artifact is any other conformant content that **does not contain a config to be interpreted by a runtime-spec implementation using the conversion mechanism. | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This definition appears to be in conflict with the current guidelines https://github.com/opencontainers/image-spec/blob/main/manifest.md#guidelines-for-artifact-usage or at least is a further restriction that probably should be defined in the above link |
||||||||||
|
||||||||||
## Creating an Artifact | ||||||||||
|
||||||||||
Content other than Images MAY be packaged using the [manifest]; this is otherwise known as an Artifact. | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
When this is done, the `artifactType` should be set to a custom media type, or the `config.mediaType` should not be a known Image config [media type](media-types.md). | ||||||||||
Implementation details and examples are provided in the [image manifest specification](manifest.md#guidelines-for-artifact-usage). | ||||||||||
|
||||||||||
Note: Historically, due to registry limitations, some tools have created non-conformant Artifacts using the `application/vnd.oci.image.config.v1+json` value for `config.mediaType`. | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IMO we really can't get away from specifying that "images" have strict layer media type requirements too -- if a given runtime can't parse one of the layers, it can't reasonably assume it can (safely) do anything with the rest of that registry object (manifest, artifact, image, whatever you want to call it). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't want to address that in this PR yet. As a follow-up to this, I want us to discuss rootfs vs non-rootfs layer types and how we can consistently decide which layers must be interpreted and which are "optional." |
||||||||||
|
||||||||||
## Interacting with Artifacts | ||||||||||
|
||||||||||
Software following the process described in [conversion.md](conversion.md) to create a [runtime-spec][] configuration blob SHOULD ignore unknown Artifacts (as determined by the presence of a descriptor `artifactType`) when selecting content from an [index](image-index.md). | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What do you mean by ignore here? Probably best to direct new conversion SHOULD language to conversion.md? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This kind of makes sense, but also today no runtime ignores unknown artifact types because they aren't aware of the field since it is completely new. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, this is somewhat of a new behavior, but at the same time is just an attempt to suggest what seems the only sane default. The idea here is that tools like BuildKit could stop producing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The issue here is not specifying a garbage platform will cause older implementations to blow up. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Most tools should ignore artifacts already (but not as eagerly/not as codified as I'd like) by virtue of e.g. the config mediaType or the layer types, thankfully. See #1131 (comment) for some research into this (though I could definitely do more testing with existing implementations and mixed indexes). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess in this case the artifact type should be listed below valid runtime image manifests? |
||||||||||
It is possible that implementations may also be able to interpret known Artifact types; however that is outside the scope of this spec. | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can the phrase "known Artifact" be clarified here? |
||||||||||
|
||||||||||
Artifacts can be detected at runtime using by checking two keys: | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
1. Is an `artifactType` present in the descriptor, or in the [manifest](manifest.md)? | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
2. Is the `config.mediaType` of the manifest something other than a [known media type](media-types.md) for [config](config.md)? | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. seems like that's 3 keys.? probably a good idea to explain here, not following the if the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Now that image-spec and dist-spec officially have "artifact" support, perhaps revisit this? converging to config.mediaType=empty and basically only do 1. above? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure I see now what @neersighted meant. Proposed text: An oci image manifest can be identified as an artifact of some type other than known oci container image type by 1) first checking if the artifact type value is set and if so whether it is valid by RFC6838, 2) otherwise checking if the config mediaType value is not one of the known OCI Image Types and is valid by RFC6838. Proposed pseudo explanation:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This would be a nice subroutine to add to this repo - likely to be heavily used going forward.
^ if I were to summarize. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just a thought ... OCI = packaging + distribution + "bring your own runtime (BYOR)"? |
||||||||||
|
||||||||||
If either of these tests is true, then the content is an Artifact. | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That is covered by the case of "runtime has special knowledge of how to handle an artifact." For the WASM-with-existing-containerd case, they should not set the artifact type as they will just be a "regular" image with special layers and a WASM platform object. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I’m probably at the risk of swimming against the current here but I honestly was hoping that the artifact type could be set by anyone and not limited to. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The issue is that I need a way to tell "not runnable" from "runnable" in containerd; I'd rather not have "an image" that is also "an artifact" at the same time as we get back to the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. cc @jsturtevant; this is meant to enable (through clarity of what runtimes likely SHOULD support) efforts to make "native WASM images" work with "existing runtimes;" I see no reason that something that is otherwise a conformant image (and just uses a This is also somewhat related to #1141 (comment), which I still want to keep as a follow-up as the scope of this PR has grown quite a bit. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
As we've been using it without The Wasm OCI image agreed on is special due to the additional layers, but there isn't really a great way to determine that via the manifest (without iterating through layers or looking up the platform on the config). Is it possible to set the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's certainly possible; as discussed on the last PR having the standard config mediaType with a custom artifactType is rather outside what most would consider idiomatic, however. The Additionally, I do think that an annotation with e.g. "WASM image standard version" or similar would be perfectly idiomatic if you need some additional metadata. WDYT? |
||||||||||
|
||||||||||
[runtime-spec]: https://github.com/opencontainers/runtime-spec/blob/main/spec.md | ||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not nec. to rescope the image spec from inside this sub-doc...