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.

Sign up for GitHub

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

Remove artifact manifest #999

Merged
merged 1 commit into from
Apr 13, 2023
Merged

Remove artifact manifest #999

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ DOC_FILES := \
descriptor.md \
image-layout.md \
manifest.md \
artifact.md \
image-index.md \
layer.md \
config.md \
Expand Down
4 changes: 1 addition & 3 deletions annotations.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,8 @@ This property contains arbitrary metadata.

## Pre-Defined Annotation Keys

This specification defines the following annotation keys, intended for but not limited to [image index](image-index.md), image [manifest](manifest.md), [artifact](artifact.md), and [descriptor](descriptor.md) authors.
This specification defines the following annotation keys, intended for but not limited to [image index](image-index.md), image [manifest](manifest.md), and [descriptor](descriptor.md) authors.

* **org.opencontainers.artifact.created** date and time on which the artifact was built, conforming to [RFC 3339][rfc3339].
* **org.opencontainers.artifact.description**: human readable description for the artifact (string)
* **org.opencontainers.image.created** date and time on which the image was built, conforming to [RFC 3339][rfc3339].
* **org.opencontainers.image.authors** contact details of the people or organization responsible for the image (freeform string)
* **org.opencontainers.image.url** URL to find more information on the image (string)
Expand Down
74 changes: 0 additions & 74 deletions artifact.md
View file
Open in desktop

This file was deleted.

5 changes: 1 addition & 4 deletions descriptor.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,14 +55,11 @@ 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 `artifactType` when the descriptor references an [artifact manifest](artifact.md).
This is the value of the config descriptor `mediaType` when the descriptor references an [image manifest](manifest.md).
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 pointing to [`application/vnd.oci.artifact.manifest.v1+json`](artifact.md) SHOULD include the extended field `artifactType`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Regression versus Proposal E, should this be projected from the config mediatype, matching the referrers API behavior in distribution-spec 1.1-rc.1?

The descriptors MUST include an artifactType field that is set to the value of artifactType for an artifact manifest if present, or the configuration descriptor's mediaType for an image manifest.

Copy link
Contributor

Choose a reason for hiding this comment

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

IMO, I would love to see this language modified to MUST. That aligns with the distribution spec, descriptors are otherwise inconsistent.


### Reserved

Extended _Descriptor_ field additions proposed in other OCI specifications SHOULD first be considered for addition into this specification.
Expand Down Expand Up @@ -206,7 +203,7 @@ In the following example, the descriptor indicates the type of artifact it is re

```json,title=Content%20Descriptor&mediatype=application/vnd.oci.descriptor.v1%2Bjson
{
"mediaType": "application/vnd.oci.artifact.manifest.v1+json",
"mediaType": "application/vnd.oci.image.manifest.v1+json",
"size": 123,
"digest": "sha256:87923725d74f4bfb94c9e86d64170f7521aad8221a5de834851470ca142da630",
"artifactType": "application/vnd.example.sbom.v1"
Expand Down
10 changes: 2 additions & 8 deletions image-index.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ For the media type(s) that this document is compatible with, see the [matrix][ma
Implementations MUST support at least the following media types:

- [`application/vnd.oci.image.manifest.v1+json`](manifest.md)
- [`application/vnd.oci.artifact.manifest.v1+json`](artifact.md)
Copy link
Member

Choose a reason for hiding this comment

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

Maybe just move this line to SHOULD support, then no need to remove the entire artifact spec?

Copy link
Member

Choose a reason for hiding this comment

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

I think this would be a good change regardless. Basically, all registries MUST support image manifests, and SHOULD (or even MAY) support the artifact manifest.

Without this change, a registry could choose to only support the artifact manifest, which would probably only cause a deeper fracture in the ecosystem.

Copy link
Contributor

Choose a reason for hiding this comment

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

Without any change, registries MUST support both the artifact and image manifests.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this would lead to a great deal of confusion for OCI 1.1 compliance and support for artifacts. With so many registries already adopting Artifact in it's RC form, what is gained by weakening this MUST to SHOULD?

Copy link
Member

Choose a reason for hiding this comment

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

@AaronFriel it lets registries adopt the references API (much more valuable imo) and claim 1.1 conformance. If they have to adopt both to be 1.1, they might prefer not to invest in either.

Copy link
Member

@imjasonh imjasonh Jan 28, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Without any change, registries MUST support both the artifact and image manifests.

Oh you're correct, I misread.

I'd still like to advocate for "MUST support at least one of these media types"

Copy link
Contributor

Choose a reason for hiding this comment

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

@AaronFriel it lets registries adopt the references API (much more valuable imo) and claim 1.1 conformance. If they have to adopt both to be 1.1, they might prefer not to invest in either.

What utility do references have absent artifacts? If registries can claim conformance with OCI 1.1 but no one can upload an sbom, NPM package, cat gif, etc., I don't see the point of OCI 1.1.

Copy link
Member

Choose a reason for hiding this comment

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

Clients have been successfully uploading SBOMs, NPM packages, cat pictures, etc., for a couple years now at least under 1.0.

1.1 will codify some of those patterns and augment them with a first-class supported API that doesn't require polluting tag hacks.


Also, implementations SHOULD support the following media types:

Expand Down Expand Up @@ -157,14 +156,9 @@ When the variant of the CPU is not listed in the table, values are implementatio
}
},
{
"mediaType": "application/vnd.oci.artifact.manifest.v1+json",
"mediaType": "application/vnd.oci.image.index.v1+json",
jonjohnsonjr marked this conversation as resolved.
Show resolved Hide resolved
"size": 7682,
"digest": "sha256:601570aaff1b68a61eb9c85b8beca1644e698003e0cdb5bce960f193d265a8b7",
"artifactType": "application/example",
"annotations": {
"com.example.artifactKey1": "value1",
"com.example.artifactKey2": "value2"
}
Comment on lines -163 to -167
Copy link
Contributor

Choose a reason for hiding this comment

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

Will artifactType and annotations be dropped from projection into the descriptor if the items are artifacts? That's a regression vs OCI 1.1-rc.2 and Proposal E.

Implementations such as Homebrew currently rely on (unsafely!) using the platform field; annotations are more extensible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The artifactType and annotations projections are more distribution-spec-isms than image-spec. I have not dropped the fields from descriptor.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see, this would be up to the user pushing the index to opt in to including in the descriptor.

It would be nice to see one of the examples in the page show that behavior with artifactType. It's not clear here or in distribution spec that if specified in the descriptor, artifactType should equal the target manifest's config mediatype.

"digest": "sha256:601570aaff1b68a61eb9c85b8beca1644e698003e0cdb5bce960f193d265a8b7"
}
],
"annotations": {
Expand Down
5 changes: 0 additions & 5 deletions img/media-types.dot
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ digraph G {
{
rank=same
manifest [shape=note, label="Image manifest\napplication/vnd.oci.image.manifest.v1+json"]
artifact [shape=note, label="Artifact Manifest\napplication/vnd.oci.artifact.manifest.v1+json"]
}
config [shape=note, label="Image config JSON\napplication/vnd.oci.image.config.v1+json"]
layer [shape=note, label="Layer tar archive\napplication/vnd.oci.image.layer.v1.tar\napplication/vnd.oci.image.layer.v1.tar+gzip\napplication/vnd.oci.image.layer.nondistributable.v1.tar\napplication/vnd.oci.image.layer.nondistributable.v1.tar+gzip"]
Expand All @@ -14,9 +13,5 @@ digraph G {
imageIndex -> manifest [label="1..*"]
manifest -> config [label="1..1"]
manifest -> layer [label="1..*"]
artifact -> manifest [label="0..1"] [constraint = false];
artifact -> artifact [label="0..1"];
manifest -> manifest [label="0..1"];
artifact -> imageIndex [label="0..1"]
artifact -> layer[label="0..*"]
Comment on lines 15 to -21
Copy link
Contributor

Choose a reason for hiding this comment

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

Will 0-layer images be supported?

Copy link
Member

Choose a reason for hiding this comment

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

Aren't 0-layer images already supported today?

$ crane manifest tianon/scratch:oci
{
  "schemaVersion": 2,
  "mediaType": "application/vnd.oci.image.manifest.v1+json",
  "config": {
    "mediaType": "application/vnd.oci.image.config.v1+json",
    "size": 290,
    "digest": "sha256:132144ab4e026a9b1ca95b5dfeb9892d12f0c2c1368ae0b4bdca408e62768396"
  },
  "layers": []
}

(Maybe the spec disagrees, but as an admittedly non-image-spec maintainer I don't see a good reason for it to do so 🙈)

Copy link
Member

Choose a reason for hiding this comment

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

Currently I feel the spec can be interpreted as Layer[0] is required. (MUST/IS) we should update this to call out that Layers are option and if used to define an image ... and so forth.

Copy link
Member

Choose a reason for hiding this comment

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

I feel like this has been unclear and therefore exists in both ways. If we firm up the language to explicitly allow zero-layer images, then it should not be considered a breaking change.

}
Binary file modified img/media-types.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 0 additions & 2 deletions spec.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ The goal of this specification is to enable the creation of interoperable tools
- [Image Index](image-index.md)
- [Filesystem Layers](layer.md)
- [Image Configuration](config.md)
- [Artifact Manifest](artifact.md)
- [Annotations](annotations.md)
- [Conversion](conversion.md)
- [Considerations](considerations.md)
Expand Down Expand Up @@ -54,7 +53,6 @@ The high-level components of the spec include:

* [Image Manifest](manifest.md) - a document describing the components that make up a container image
* [Image Index](image-index.md) - an annotated list of manifests
* [Artifact Manifest](artifact.md) - a document describing the components that make up an artifact
* [Image Layout](image-layout.md) - a filesystem layout representing the contents of an image
* [Filesystem Layer](layer.md) - a changeset that describes a container's filesystem
* [Image Configuration](config.md) - a document determining layer ordering and configuration of the image suitable for translation into a [runtime bundle][runtime-spec]
Expand Down
34 changes: 0 additions & 34 deletions specs-go/v1/artifact.go
View file
Open in desktop

This file was deleted.

3 changes: 0 additions & 3 deletions specs-go/v1/mediatype.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,4 @@ const (

// MediaTypeScratch specifies the media type for an unused blob containing the value `{}`
MediaTypeScratch = "application/vnd.oci.scratch.v1+json"

// MediaTypeArtifactManifest specifies the media type for a content descriptor.
MediaTypeArtifactManifest = "application/vnd.oci.artifact.manifest.v1+json"
)