-
Notifications
You must be signed in to change notification settings - Fork 651
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
Conversation
5ec01fd
to
dad06e2
Compare
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.
I haven't done a full review, but just noting some of my thoughts so far to hopefully spark more discussion.
artifacts-guidance.md
Outdated
Images are defined in this specification as conformant content with a [config](config.md), designed to be interpreted by a runtime that implements the [runtime-spec][]. | ||
Conversely, an Artifact is any other conformant content that **does not contain a config to be interpreted by a runtime-spec implementation.** |
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.
As a runtime-spec maintainer also, I'm very confused by this block -- the config object holds the data used to construct a runtime-spec "bundle", but it is not parsed/specified as part of the runtime-spec. The two are very loosely related. 😅
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.
Yeah, I'm not quite 100% happy with this either, but there are other parts of the spec that allude to the runtime spec, and "config that is meant to construct a runtime spec config base" still is a useful distinction, I believe.
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.
This also seems at contradiction to the statement below: It is possible that software implementing the runtime-spec may also be able to interpret Artifacts; however that is outside the scope of this spec.
Couldn't a runtime generate a runtime-spec based on something other than a classic image-config? The loose coupling seems like this would make this possible.
I am thinking eventually we will want a WASM Artifact that doesn't use the image config format but still gets converted to a runtime-spec for runtimes that support the artifact type.
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.
It definitely could; does anyone have suggestions on how to describe interpretation of the classical config.md config without muddying the waters with runtime-spec references?
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.
I think that referring to conversion.md
and the mechanism therein is the clarification I want.
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 comment
The 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 comment
The 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."
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.
I have a high level concern that we're trying to document the mixture of runnable Images and Artifacts in the same Index. Is that something we want to standardize in OCI, or would runtimes prefer that they are only asked to run an entry from an Index of only Images?
The concern I have is there's no limit to unique media types an Artifact may use, and there's likely a desire by new use cases to define new types of runnable content that may also have new media types that a current runtime would not have seen before. I'm wondering if we want to differentiate between a runtime unsure how to run the requested content vs a runtime not finding any Image content in an Index.
Feedback from @opencontainers/runtime-spec-maintainers is welcome.
Yes, this is a realistic use-case with inline metadata today, e.g. what BuildKit does.
I think this is behavior defined by the runtime; what I want to make clear is that a runtime can interpret new artifact types if it has explicit support for them. But the default should be "ignore any content I do not know how to interpret" to obviate the need for |
dad06e2
to
d5f627d
Compare
I want to be careful that we don't say "buildkit implemented it, therefore OCI needs to recommend it". It's not unlike how cosign pushed manifests that looked like an OCI image with a different layer type to get around registries that filtered unknown config mediaType values. Even though the behavior works and doesn't violate the spec, I'm not incline to recommend that as a method to push artifacts. Before I weigh in on the mixed content in an Index scenario, I'd like to hear from the impacted users that would be parsing that content (runtime maintainers). |
d5f627d
to
192c3d7
Compare
Runtimes implementing the [runtime-spec][] and following the process described in [conversion.md](conversion.md) SHOULD ignore unknown Artifacts (as determined by the presence of a descriptor `artifactType`) when selecting content from an [index](image-index.md). | ||
It is possible that software implementing the runtime-spec may also be able to interpret Artifacts; however that is outside the scope of this spec. | ||
|
||
Artifacts can be detected at runtime using by checking two keys: |
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.
Artifacts can be detected at runtime using by checking two keys: | |
Artifacts can be detected at runtime by checking two keys: |
cc @dmcgowan re: runtime behavior with mixed indexes. |
Better define images and artifacts conceptually, and add additional guidance on detecting them in code/at a data-model level. Signed-off-by: Bjorn Neergaard <bjorn.neergaard@docker.com>
Signed-off-by: Bjorn Neergaard <bjorn.neergaard@docker.com>
Signed-off-by: Bjorn Neergaard <bjorn.neergaard@docker.com>
192c3d7
to
75c75e5
Compare
This specification is primarily concerned with packaging two kinds of content: Artifacts and Images. | ||
Both are representing using a [manifest](manifest.md). | ||
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. | ||
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 comment
The reason will be displayed to describe this comment to others. Learn more.
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. | |
Conversely, an Artifact is any other conformant content that **does not** contain a [config](config.md) to be interpreted by a runtime-spec implementation using the conversion mechanism. |
1. Is an `artifactType` present in the descriptor, or in the [manifest](manifest.md)? | ||
2. Is the `config.mediaType` of the manifest something other than a [known media type](media-types.md) for [config](config.md)? | ||
|
||
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 comment
The 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 comment
The 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 comment
The 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.
if the OCI runtime spec doesn’t want the artifact type set then it can mandate that but WASM runtimes don’t need to honor that.
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.
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 platform: none/none
pattern.
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.
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 wasm
platform) needs to set artifactType
.
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 comment
The reason will be displayed to describe this comment to others. Learn more.
I see no reason that something that is otherwise a conformant image (and just uses a wasm platform) needs to set artifactType.
As we've been using it without artifactType
, we've found it is difficult to differentiate between "wasm oci image" (with custom layers) and standard images. This was one of the reasons we initially included ArtifactType
, it was a way to signify that the image has non-standard layers in it. It is additional steps to look up the config media type, open it up and look at the platform.
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 ArtifactType
and have the config.mediaType
be the standard image media type? It would still allow runtimes to reject artifact types since artifact is set?
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.
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 platform
field on the descriptor should be the same as the platform in the config
blob, so the platform of the image should be accessible via the same mechanism as the top-level artifactType
.
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?
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.
see comments...
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. |
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...
## 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 comment
The reason will be displayed to describe this comment to others. Learn more.
Both are representing using a [manifest](manifest.md). | |
Artifacts may be represented using an [image manifest](manifest.md). |
This sentence repeats the one this commit moves down... maybe just add the missing link to the prior original block?
|
||
This specification is primarily concerned with packaging two kinds of content: Artifacts and Images. | ||
Both are representing using a [manifest](manifest.md). | ||
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 comment
The 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 comment
The 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 comment
The reason will be displayed to describe this comment to others. Learn more.
fair..
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. | |
Images having conformant content with conformant [config](config.md) are processed according to [conversion.md](conversion.md) to derive a [runtime-spec][] configuration blob. |
This specification is primarily concerned with packaging two kinds of content: Artifacts and Images. | ||
Both are representing using a [manifest](manifest.md). | ||
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. | ||
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 comment
The 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
|
||
## 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 comment
The 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 comment
The 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 comment
The 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 platform: unknown/unknown
to prevent their not-images from being considered for platform selection by simply switching to artifacts instead.
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.
The issue here is not specifying a garbage platform will cause older implementations to blow up.
I'm not sure what that means for the wording here.
Putting anything in an index that is not an image manifest is not backwards compatible, I guess.
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.
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 comment
The 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?
## 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). | ||
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 comment
The reason will be displayed to describe this comment to others. Learn more.
It is possible that implementations may also be able to interpret known Artifact types; however that is outside the scope of this spec. | |
Interpretation / implementation of known Artifact types is currently outside the scope of this spec. |
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.
Can the phrase "known Artifact" be clarified here?
Does it mean artifactType field exists or not?
Does it mean artifactType field exists but uses a well-known media-type?
Does it mean well-known media-type to outside world but not to this spec?
etc ... etc ...
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). | ||
It is possible that implementations may also be able to interpret known Artifact types; however that is outside the scope of this spec. | ||
|
||
Artifacts can be detected at runtime using by checking two keys: |
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.
Artifacts can be detected at runtime using by checking two keys: | |
Artifacts can be detected at runtime by checking two key-value fields: |
It is possible that implementations may also be able to interpret known Artifact types; however that is outside the scope of this spec. | ||
|
||
Artifacts can be detected at runtime using by checking two keys: | ||
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 comment
The reason will be displayed to describe this comment to others. Learn more.
1. Is an `artifactType` present in the descriptor, or in the [manifest](manifest.md)? | |
1. Is an {`artifactType`, _string_} present, e.g. as a `subject` reference in an image/artifact manifest or as a self reference definition in the [image artifact manifest `artifactType` field](https://github.com/opencontainers/image-spec/blob/main/manifest.md#image-manifest-property-descriptions)? |
|
||
Artifacts can be detected at runtime using by checking two keys: | ||
1. Is an `artifactType` present in the descriptor, or in the [manifest](manifest.md)? | ||
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 comment
The 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 config.mediaType
is unknown it's an artifact thought..
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.
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 comment
The 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:
func checkForArtifactType(manifest) artifactType, error
{
manifestArtifactTypeMustBeSet = UNKNOWN
manifestArtifactTypeIsSet = UNKNOWN
manifestArtifactTypeMustBeValid = UNKNOWN
manifestArtifactTypeIsValid = UNKNOWN
configMediaTypeIsOCIImageType = UNKNOWN
configMediaTypeIsValidArtifact = UNKNOWN
if manifest.config.mediaType is "application/vnd.oci.empty.v1+json" {
manifestArtifactTypeMustBeSet = TRUE;
}
manifestArtifactTypeIsSet = is manifest.artifactType unset or nil?
if manifestArtifactTypeIsSet {
manifestArtifactTypeMustBeValid = TRUE;
}
if manifestArtifactTypeMustBeValid {
manifestArtifactTypeIsValid = checkRFC6838(manifest.artifactType);
if !manifestArtifactTypeIsValid {
return with error(manifest.artifactType + "is not valid")
}
return manifest.artifactType // we have the artifact type we were looking for
}
configMediaTypeIsOCIImageType = isKnownOCIImageConfigMediaType(manifest.config.mediaType)
if configMediaTypeIsOCIImageType {
return image .. no artifact found
}
// not "oci.empty" and not an oci image media type
configMediaTypeIsValidArtifact = checkRFC6838(manifest.config.mediaType);
if !configMediaTypeIsValidArtifact {
return with error(manifest.config.mediaType + "is not valid")
}
return manifest.config.mediaType // we have the artifact config media type we were looking for
} // end checkForArtifactType()
func checkRFC6838 (type string) BOOL
{
// returns true if not nil and complies with [RFC 6838](https://tools.ietf.org/html/rfc6838), including the [naming requirements in its section 4.2](https://tools.ietf.org/html/rfc6838#section-4.2)
}
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.
This would be a nice subroutine to add to this repo - likely to be heavily used going forward.
- Runnable container images
- Non-runnable not-container images (referrer or otherwise)
- Runnable non-container images
^ if I were to summarize.
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.
Just a thought ... OCI = packaging + distribution + "bring your own runtime (BYOR)"?
Given the various scenarios we're seeing, and knowing there will be more scenarios in the future, perhaps instead of being prescriptive in exactly what fields are defined and we should keep it at a higher level. Something like "container images are any content pushed to the registry designed to be executed by a container runtime, artifacts are everything else." There will be artifacts that want to have a valid platform defined, and other artifacts that want to use the image config media type. We may be able to avoid the latter with guidance in the spec, but that value isn't visible in the Index. And expecting runtimes to avoid trying to run an artifact image based on a new field in the descriptor (i.e. The two options I can think of are either to ensure artifacts are listed after any container images in an index, so that the selector for container runtimes selects the first match, or OCI recommends against mixing container images and artifacts in the same index for the best support of older runtimes. |
My phrasing is that container runtimes trying to execute/process it makes it a container image. It's not restricting what runtimes can do, it's defining whether it's a container image or artifact based on whether runtimes could execute/process it. It leaves flexibility for runtimes to extend to process more content in the future without us needing to adjust the image-spec for every runtime change. |
IMO it is not a good idea to redefine (is/are language) what a container image is inside the artifacts guidance documentation. Let's not leave out the index may or may not include platform filters, and "implementations SHOULD support" "application/vnd.oci.image.index.v1+json (nested index)" and "an encountered mediaType that is unknown to the implementation MUST NOT generate an error." |
I think it's an oci container image if it's in a format conforming to the image spec for storage or transport, and then at runtime, by conversion and augmentation via a container runtime, if it is determined by a runc compatible runtime to then adhere to the runtime spec config and rootfs requirements of a runtime bundle for a platform. |
artifacts are a grey space wrt. whether they can be considered a part of or augmentation to layers/config/index, guidance, filtering mechanisms, additional image config, even possibly a key store, alg, or content for a new digest type... runnable or not is probably up to the iana type or we possibly break with existing spec restrictions. 100% agree we need to avoid a client reading an artifact, via matching, as a default platform image if/when that is not the intended result. |
I like the high level definition here: https://github.com/opencontainers/image-spec/blob/main/spec.md#overview
|
An issue attempting to be solved in this PR is runtimes picking container images from a mixed index with artifacts included in the manifest list. The artifact entries could have matching platform values, and known media type values. My comments above are looking at what options, if any, we have to resolve that. I don't believe "MUST NOT generate an error" helps in this scenario since the values would be known. |
absolutely... 100% agree we need to avoid a client reading an artifact, via matching, as a default platform image if/when that is not the intended result. my point in including the additional cites was as a reminder:
and those current rules may be useful in finding a resolution.. For example, a) push said artifacts into an index under the platform index... (and check if clients actually recurse today for nested indexes... suggesting a solution here where platform (optional) "index" is for container image manifests and nested indexes not for artifact type manifests irregardless of the desire to filter artifacts by platform |
Dropping my block since no runtime maintainer have expressed any concerns.
|
||
## 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 comment
The reason will be displayed to describe this comment to others. Learn more.
Content other than Images MAY be packaged using the [manifest]; this is otherwise known as an Artifact. | |
Content other than Images MAY be packaged using the [manifest](manifest.md); this is otherwise known as an Artifact. |
Attempt to better clarify the conceptual differences between artifacts and images (which I tie to the config as described in this spec), and provide guidance on how to detect artifacts from code.
Also attempt to clarify how descriptors interact with artifacts in the aftermath of #999.