Moving artifact type out of annotations #59

sudo-bmitch · 2022-07-05T23:46:46Z

This looks at what happens if we move artifact type out of annotations and into a dedicated field in the manifest.

Pros:

Registries are more likely to parse and provide filtering on the field
Client tooling may find it easier to avoid parsing annotations for this field

Cons:

Extends the descriptor struct to support pulling up the type field
We lose support for registries that do not allow custom media types on the config descriptor (notably Docker Hub)
The mediaType will not fit in the tag extension, so a mapping is needed for the tag digest scheme

Voting for this option is happening in #64.

Signed-off-by: Brandon Mitchell git@bmitch.net

Signed-off-by: Brandon Mitchell <git@bmitch.net>

sudo-bmitch · 2022-07-05T23:49:34Z

@michaelb990 I think I captured the request here. Comments/reviews welcome.

jdolitsky · 2022-07-06T00:14:16Z

Is this meant to be a valid IANA-style mediatype?

sudo-bmitch · 2022-07-06T01:19:34Z

Is this meant to be a valid IANA-style mediatype?

We should probably follow the requirements for the config mediaType in artifacts today which defers to IANA in some parts but I don't think it's a strict requirement:

https://github.com/opencontainers/artifacts/blob/main/artifact-authors.md#defining-a-unique-artifact-type

SteveLasker · 2022-07-06T01:26:54Z

IANA was used as the clearing house. If two artifacts want to claim a mediaType, the one that registers with IANA would be "the official" one. This took OCI out of arbitrating a decision.

rchincha · 2022-07-11T22:10:24Z

docs/proposals/PROPOSAL_E.md

@@ -59,6 +59,7 @@ Create a new artifact media type to support future use cases where a separate co
 {
  "schemaVersion": 2,


https://github.com/opencontainers/image-spec/blob/main/schema/image-manifest-schema.json#L8
^ isn't this supposed to indicate schema version? which means we are allowed to make schema changes and move forward? Is this being considered at all? Hope I am making sense here.

In theory I think you are correct, but I imagine a lot of implementations are keying off of mediatype vs. this field in determining the schema to use... probably worth a discussion

I guess the choices are:

(1) do we announce and ask implementations to fix said bug (ignoring schemaVersion field when there is explicitly one sounds like a unmarshalling-101 bug to me)

OR

(2) do we guarantee that our changes must forever be forward-compatible with older code/tools, which means schemaVersion can be deprecated?

PS: Just stating choices, not making one.

We've been keeping it for historical reasons. https://github.com/opencontainers/image-spec/blob/main/manifest.md#image-manifest-property-descriptions

I suppose it could be removed, or made optional, since no older runtimes should be parsing a newer artifact-spec manifest (the manifest isn't for packaging images that would be used by those runtimes).

For moving forward, I believe @jdolitsky is right, we would be creating a new media type (there's a version in that field). However, those fields are parsed by registries, so a new media type, or new version number in an otherwise known media type, will be rejected by almost every existing registry out there (they parse the manifest to know the associated blobs, perform validation, and often handle garbage collection) until those registries are upgraded. There's no automatic forward compatibility.

"For this version of the specification, this MUST be 2 to ensure backward compatibility with older versions of Docker."

The above statement doesn't hold then for another version (> v2s2) of the specification? If so, this field does offer an out for a schema selector instead of deprecation, either to support or not support schema versions, with minimal code changes. Thoughts? Furthermore, in the long-term, "backward compatibility with Docker" seems an artificial constraint for an OCI spec.

@rchincha are you suggesting using "schemaVersion": 3 instead of adding application/vnd.oci.artifact.manifest.v1+json altogether?

"schemaVersion": 3 for sure and subsequent schema changes per community consensus.

For old tools,
(in pseudocode) ...

# this is the skip-unsupported change to be added! read schema as JSON key-val pairs schemaVersion = json.keys["schemaVersion"] if schemaVersion not in [v1, v2s1, v2s2]: fail with unsupported-error process further because supported

PS: Wouldn't this be a more acceptable path in reality for folks who are not interested in supporting schema changes but want to fail gracefully instead.

sounds reasonable to me, others may have more background on this field

From today's call given that the schemaVersion is for historic reasons, carrying this over to the new manifest might not be needed.
#73

michaelb990

This is exactly what I was imagining for the manifest. Thanks for putting it together!

I was also hoping to add filtering on the referrers API for the artifactType field. Are you thinking we should do that in a separate PR? I'm happy to work on that one if it helps.

jdolitsky · 2022-07-12T17:25:02Z

will this resolve #46 then?

docs/proposals/PROPOSAL_E.md

SteveLasker · 2022-07-13T15:08:45Z

Moving questions and comments from #64 to here:

artifactType as defined in Proposal A and the ORAS Artifact manifest is intended to represent an extensibility point.

artifactType string

The REQUIRED artifactType is a unique value, as registered with iana.org. The artifactType values are equivalent to the values used in the manifest.config.mediaType in OCI Artifacts. Examples include sbom/example, application/vnd.cncf.notary.v2. For details on creating a unique artifactType, see OCI Artifact Authors Guidance

It's the same behavior as defined in OCI Artifacts, where the Helm team can define their type, the SPDX SBOM team can define theirs, Joe Curly can define their type. OCI Artifacts used the config.mediaType. The new manifest raises config.mediaType to an artifactType property.

When we debated this in OCI Artifacts, rather than make OCI the clearing house for who owns the mediaType, we deferred to iana.org.

Similar to operating system extensibility points, where someone can say the .txt format is associated with a "Text Document", there's an issue that captures localizable strings and icons: opencontainers/artifacts#45.
This way, OCI isn't the clearing house/moderator. And registries can choose to support different artifactTypes with localized strings/icons

michaelb990

L39: - org.opencontainers.artifact.type: type of artifact (sig, sbom, etc)

I think this can be removed if we pull the artifactType into a top-level field.

michaelb990 · 2022-07-13T17:39:25Z

docs/proposals/PROPOSAL_E.md

@@ -59,6 +59,7 @@ Create a new artifact media type to support future use cases where a separate co
 {
  "schemaVersion": 2,
  "mediaType": "application/vnd.oci.artifact.manifest.v1+json",
+  "artifactType": "example/icecream", // used in place of config mediaType


I'm expecting this to be another media type, similar to the mediaType field.

Suggested change

"artifactType": "example/icecream", // used in place of config mediaType

"artifactType": "application/vnd.example.icecream.v1", // used in place of config mediaType

michaelb990 · 2022-07-13T17:39:50Z

docs/proposals/PROPOSAL_E.md

@@ -80,7 +81,11 @@ Extend the Image Manifest with a refers field (existing registries should ignore
 {
  "schemaVersion": 2,
  "mediaType": "application/vnd.oci.image.manifest.v1+json",
-  "config": { ... },
+  "config": { 
+    "mediaType": "example/icecream",


Same here:

Suggested change

"mediaType": "example/icecream",

"mediaType": "application/vnd.example.icecream.v1",

jdolitsky · 2022-07-13T19:37:54Z

@SteveLasker - could you help outline the shape of new artifact manifest for a standalone Helm artifact?

Docs: https://helm.sh/docs/topics/registries/#helm-chart-manifest

Would it be something like

Old application/vnd.oci.image.manifest.v1+json (OCI artifact)

{
  "schemaVersion": 2,
  "config": {
    "mediaType": "application/vnd.cncf.helm.config.v1+json",
    "digest": "sha256:8ec7c0f2f6860037c19b54c3cfbab48d9b4b21b485a93d87b64690fdb68c2111",
    "size": 117
  },
  "layers": [
    {
      "mediaType": "application/vnd.cncf.helm.chart.content.v1.tar+gzip",
      "digest": "sha256:1b251d38cfe948dfc0a5745b7af5ca574ecb61e52aed10b19039db39af6e1617",
      "size": 2487
    }
  ]
}

New application/vnd.oci.artifact.manifest.v1+json

{
  "mediaType": "application/vnd.oci.artifact.manifest.v1+json",
  "artifactType": "application/vnd.cncf.helm.chart.envelope.v1+json",
  "blobs": [
     {
       "mediaType": "application/vnd.cncf.helm.chart.content.v1.tar+gzip",
       "digest": "sha256:1b251d38cfe948dfc0a5745b7af5ca574ecb61e52aed10b19039db39af6e1617",
       "size": 2487
     }
  ]
}

SteveLasker · 2022-07-13T21:34:35Z

could you help outline the shape of new artifact manifest for a standalone Helm artifact?

Your example above would be the functional equivalent, which also shows how the OCI Image manifest.config.mediaType = Artifact manifest.artifactType

There's two additional points:

We initially focused on putting config in the mediaType, and including a +json on the extension, simply as momentum for how they were represented at the time.
We later clarified config should be reserved when there's actually a config blob, and the +json|+yaml should be reserved to represent how the optional config blob is formatted. See Defining a Unique Artifact Type
A better example would be: "artifactType" : "application/vnd.cncf.helm.chart.v1"
_Note: I'd also suggest avoiding envelope, as it's an overloaded term for signatures. It's really a Helm Artifact Type. envelope feels redundant as the manifest is an "envelope" for all types that have some content.
See OCI Artifacts issue: Add clarity when config should be used in the mediaType #60
To @sudo-bmitch's point about broad adoption, I wouldn't suggest top level artifacts adopt the new manifest yet. They can functionally get the same benefit using the OCI Image manifest, following the OCI Artifact guidance. I would reserve using the new manifest for reference types, like signatures, sboms, scan results, claims, and endorsements as these have unique functionality.

I'd also suggest the helm team shouldn't change, as the config.mediaType|.artifactType is a detail for the plumbers. We're really discussing what new artifacts would do.

If you really wanted to get interesting, consider this example. If we were to create a new runtime container image format, with the artifact manifest, we could put the config as just another one of the blobs. If you split the collection of blobs on mediaType, you could choose to sort the layers, while pulling out config separately. Why was "config" so special to make it's single property? Maybe new types have various types of blobs "config", "binaries", "___?"

OCI Image as an Artifact

{
  "mediaType": "application/vnd.cncf.oras.artifact.manifest.v1+json",
  "artifactType": "application/vnd.oci.image.manifest.v1+json",
  "blobs": [
    {
      "mediaType": "application/vnd.oci.image.config.v1+json",
      "digest": "sha256:e752324f6804d5d0b2c098f84507d095a8fd0031cf06cdb3c7ad1625dcd1b399",
      "size": 7097
    },
    {
      "mediaType": "application/vnd.oci.image.layer.v1.tar+gzip",
      "digest": "sha256:83c5cfdaa5385ea6fc4d31e724fd4dc5d74de847a7bdd968555b8f2c558dac0e",
      "size": 25851449
    },
    {
      "mediaType": "application/vnd.oci.image.layer.v1.tar+gzip",
      "digest": "sha256:7445693bd43e8246a8c166233392b33143f7f5e396c480f74538e5738fb6bd6e",
      "size": 226
    }
  ],
  "annotations": {
    "io.cncf.oras.artifact.created": "2022-06-13T22:11:10.08Z"
  }
}

Notice the config object is in the blobs collection, and we know it's a config, because: "mediaType": "application/vnd.oci.image.config.v1+json"
Again, not suggesting we change, just showing the flexibility of minor tweaks of evolution to the awesome designs we're starting with.

jdolitsky · 2022-07-13T23:25:33Z

The image example is interesting, thanks. All SGTM.

I just opened #66 the track the other ongoing convo about schemaVersion field in a thread above.

SteveLasker

Some nits on the consistency of examples, but otherwise LGTM for the scope of the PR.

docs/proposals/PROPOSAL_E.md

SteveLasker · 2022-07-14T18:45:56Z

docs/proposals/PROPOSAL_E.md

@@ -121,21 +126,21 @@ The response is an Index of descriptors:
  "manifests": [
    {
      "mediaType": "application/vnd.oci.image.manifest.v1+json",
+      "artifactType": "example/icecream", // pulled up from manifest


Would this match the examples above in line 85?
FWIW, I beleive iana recommends thing/example, but that gets overly simplified, so I'm also ok with `"application/vnd.example.icecream.v1" example @michaelb990 provided above.

docs/proposals/PROPOSAL_E.md

SteveLasker · 2022-07-15T02:13:26Z

@sudo-bmitch, are you ready to take this PR out of draft status?

sudo-bmitch · 2022-07-15T03:56:20Z

@sudo-bmitch, are you ready to take this PR out of draft status?

@SteveLasker no, we are still voting on this option in #64.

- Removes the unused annotion - Removes the schemaVersion - Uses IANA syntax for mediaType Signed-off-by: Brandon Mitchell <git@bmitch.net>

Moving artifact type out of annotations

95ce02a

Signed-off-by: Brandon Mitchell <git@bmitch.net>

sudo-bmitch requested review from dmcgowan, imjasonh, jdolitsky, jonjohnsonjr, justincormack, lachie83, michaelb990, nishakm and SteveLasker as code owners July 5, 2022 23:46

sudo-bmitch marked this pull request as draft July 5, 2022 23:47

sudo-bmitch mentioned this pull request Jul 6, 2022

Updates to Proposal E #42

Closed

5 tasks

rchincha reviewed Jul 11, 2022

View reviewed changes

michaelb990 approved these changes Jul 12, 2022

View reviewed changes

sajayantony approved these changes Jul 12, 2022

View reviewed changes

jdolitsky reviewed Jul 12, 2022

View reviewed changes

docs/proposals/PROPOSAL_E.md Show resolved Hide resolved

sudo-bmitch mentioned this pull request Jul 12, 2022

[Vote] Should artifact type move out of annotations #64

Closed

michaelb990 reviewed Jul 13, 2022

View reviewed changes

jdolitsky mentioned this pull request Jul 13, 2022

Why not bump schemaVersion on existing manifest? #66

Closed

jdolitsky approved these changes Jul 13, 2022

View reviewed changes

SteveLasker approved these changes Jul 14, 2022

View reviewed changes

Updates to make a dedicated artifactType

d5b4815

- Removes the unused annotion - Removes the schemaVersion - Uses IANA syntax for mediaType Signed-off-by: Brandon Mitchell <git@bmitch.net>

sudo-bmitch marked this pull request as ready for review July 23, 2022 19:10

sudo-bmitch mentioned this pull request Jul 23, 2022

Remove schemaVersion from new artifact manifest #73

Closed

sajayantony approved these changes Jul 25, 2022

View reviewed changes

michaelb990 mentioned this pull request Jul 26, 2022

Adding artifactType filtering to the Referrers API #74

Merged

sudo-bmitch merged commit 40298b8 into opencontainers:main Jul 26, 2022

sudo-bmitch deleted the pr-artifact-manifest-type branch July 26, 2022 18:06

michaelb990 mentioned this pull request Aug 4, 2022

[Vote] Allow for server-side filtering using artifactType #78

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Moving artifact type out of annotations #59

Moving artifact type out of annotations #59

sudo-bmitch commented Jul 5, 2022 •

edited

sudo-bmitch commented Jul 5, 2022

jdolitsky commented Jul 6, 2022

sudo-bmitch commented Jul 6, 2022

SteveLasker commented Jul 6, 2022

rchincha Jul 11, 2022

jdolitsky Jul 11, 2022

rchincha Jul 11, 2022

sudo-bmitch Jul 12, 2022

sudo-bmitch Jul 12, 2022

rchincha Jul 12, 2022

jdolitsky Jul 13, 2022

rchincha Jul 13, 2022

jdolitsky Jul 13, 2022

sajayantony Jul 19, 2022 •

edited

michaelb990 left a comment

jdolitsky commented Jul 12, 2022

SteveLasker commented Jul 13, 2022 •

edited

michaelb990 left a comment

michaelb990 Jul 13, 2022

michaelb990 Jul 13, 2022

jdolitsky commented Jul 13, 2022

SteveLasker commented Jul 13, 2022

jdolitsky commented Jul 13, 2022

SteveLasker left a comment

SteveLasker Jul 14, 2022

SteveLasker commented Jul 15, 2022

sudo-bmitch commented Jul 15, 2022

		@@ -59,6 +59,7 @@ Create a new artifact media type to support future use cases where a separate co
		{
		"schemaVersion": 2,

	"artifactType": "example/icecream", // used in place of config mediaType
	"artifactType": "application/vnd.example.icecream.v1", // used in place of config mediaType

	"mediaType": "example/icecream",
	"mediaType": "application/vnd.example.icecream.v1",

Moving artifact type out of annotations #59

Moving artifact type out of annotations #59

Conversation

sudo-bmitch commented Jul 5, 2022 • edited

sudo-bmitch commented Jul 5, 2022

jdolitsky commented Jul 6, 2022

sudo-bmitch commented Jul 6, 2022

SteveLasker commented Jul 6, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sajayantony Jul 19, 2022 • edited

Choose a reason for hiding this comment

michaelb990 left a comment

Choose a reason for hiding this comment

jdolitsky commented Jul 12, 2022

SteveLasker commented Jul 13, 2022 • edited

michaelb990 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jdolitsky commented Jul 13, 2022

SteveLasker commented Jul 13, 2022

jdolitsky commented Jul 13, 2022

SteveLasker left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SteveLasker commented Jul 15, 2022

sudo-bmitch commented Jul 15, 2022

sudo-bmitch commented Jul 5, 2022 •

edited

sajayantony Jul 19, 2022 •

edited

SteveLasker commented Jul 13, 2022 •

edited