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

Artifacts clarification #1141

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
30 changes: 27 additions & 3 deletions artifacts-guidance.md
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.
Copy link
Member

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...

Both are representing using a [manifest](manifest.md).
Copy link
Member

@mikebrow mikebrow Nov 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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?

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.
Copy link
Member

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..

Copy link
Author

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.

Copy link
Member

@mikebrow mikebrow Nov 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fair..

Suggested change
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.

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.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.
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.

Copy link
Member

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


## Creating an Artifact

Content other than Images MAY be packaged using the [manifest]; this is otherwise known as an Artifact.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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.

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`.
Copy link
Member

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).

Copy link
Author

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."


## 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).
Copy link
Member

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?

Copy link
Contributor

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.

Copy link
Author

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.

Copy link
Contributor

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.

Copy link
Author

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).

Copy link
Contributor

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?

It is possible that implementations may also be able to interpret known Artifact types; however that is outside the scope of this spec.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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.

Copy link

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 ...


Artifacts can be detected at runtime using by checking two keys:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Artifacts can be detected at runtime using by checking two keys:
Artifacts can be detected at runtime by checking two keys:

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Artifacts can be detected at runtime using by checking two keys:
Artifacts can be detected at runtime by checking two key-value fields:

1. Is an `artifactType` present in the descriptor, or in the [manifest](manifest.md)?
Copy link
Member

Choose a reason for hiding this comment

The 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)?
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)?

2. Is the `config.mediaType` of the manifest something other than a [known media type](media-types.md) for [config](config.md)?
Copy link
Member

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..

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?

Copy link
Member

@mikebrow mikebrow Feb 29, 2024

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)
}

Copy link

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.

  1. Runnable container images
  2. Non-runnable not-container images (referrer or otherwise)
  3. Runnable non-container images

^ if I were to summarize.

Copy link

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)"?


If either of these tests is true, then the content is an Artifact.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding is that the wasm folks want to use the artifact type. It's runnable by the wasm runtime and not something that is aligned with the runtime spec.
How can we support them by enabling the usage of artifactType ? @cpuguy83 @tianon what do you folks think?

Copy link
Author

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.

Copy link
Member

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.

Copy link
Author

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.

Copy link
Author

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.

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?

Copy link
Author

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?


[runtime-spec]: https://github.com/opencontainers/runtime-spec/blob/main/spec.md

8 changes: 5 additions & 3 deletions descriptor.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,13 @@ The following fields contain the primary properties that constitute a Descriptor

- **`artifactType`** *string*

This OPTIONAL property contains the type of an artifact when the descriptor points to an artifact.
This is the value of the config descriptor `mediaType` when the descriptor references an [image manifest](manifest.md).
This OPTIONAL property contains the type of an artifact when the descriptor points to an [Artifact](artifacts-guidance.md).
This property MUST be the same as the `artifactType` of the referenced [manifest](manifest.md), or the `mediaType` of the `config` descriptor if an `artifactType` is not set.
If defined, the value MUST comply with [RFC 6838][rfc6838], including the [naming requirements in its section 4.2][rfc6838-s4.2], and MAY be registered with [IANA][iana].

Descriptors pointing to [`application/vnd.oci.image.manifest.v1+json`](manifest.md) SHOULD include the extended field `platform`, see [Image Index Property Descriptions](image-index.md#image-index-property-descriptions) for details.
Descriptors referencing an Image (as defined in the [artifacts guidance](artifacts-guidance.md)) SHOULD include the extended field `platform`.
See [Image Index Property Descriptions](image-index.md#image-index-property-descriptions) for details.
Artifacts as defined in the artifacts guidance MAY include the extended field `platform` when it improves discoverability or interoperability of the artifact.

### Reserved

Expand Down