manifest: provide guidance on SCRATCH descriptor (config and layer) #1023

vbatts · 2023-02-12T17:47:11Z

Through the conversations between artifact manifest and standardizing the misuse of the image-manifest one of the topics was around when there is not a config needed for the layers/blobs, and if there was not a layer needed. How best to be portable.

Since the config is a REQUIRED field, it meant setting this to some valid value.

This guidance intends to set a norm for a blob that need only be pushed to a registry a single time, and then save on round trips for all future SCRATCH configs, while also being most widely portability.

Signed-off-by: Vincent Batts vbatts@hashbangbash.com

vbatts · 2023-02-12T17:49:10Z

I would like feedback on whether to add sha256:44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a as a const in ./specs-go/v1/manifest.go

manifest.md

sajayantony · 2023-02-13T09:42:01Z

+1 on adding the digest. I think it solidifies what an empty config is and clients/registries can optimize this if they haven’t already.

vbatts · 2023-02-13T12:16:17Z

updated PTAL

specs-go/v1/manifest.go

vbatts · 2023-02-13T17:51:20Z

I thought about that, but then the marshalling or string handling... I figured just to provide the sum that can be compared. And include in variable name that it's sha256 for _whenever_ we move on to something else. Instead I'll improve upon the comment to show that it's `{}`

tianon · 2023-02-13T18:32:34Z

Is there much difference between v1.ScratchConfigDigestSHA256 and v1.ScratchConfig.Digest? (which is then also a string, but a typed string with more helpful methods?)

var ScratchConfig = imagespec.Descriptor{
	Digest: `sha256:44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a`,
	Size:   2,
	Data:   []byte(`{}`),
}

This then allows comparison directly against .Digest (as a string, which it is), but you can then push it more trivially if you want/need to (and there's a descriptor that's basically ready-to-use, although since it's a struct it can't be const because Go 🙈).

sajayantony · 2023-02-13T20:05:47Z

@vbatts When we are adding the digest as a const, shouldn't the actual data value also be present. Some feel @tianon's proposal helps others consume this. Basically there is a scratchDescriptor :)

vbatts · 2023-02-13T21:03:57Z

right on!

vbatts · 2023-02-13T21:32:21Z

updated PTAL

specs-go/v1/manifest.go

vbatts · 2023-02-16T10:31:31Z

updated PTAL

brackendawson · 2023-02-16T13:32:36Z

I think #1025 supports this guidance. 👍

manifest.md

sajayantony · 2023-02-24T19:16:00Z

Adding this to 1.1 milestone as #999 would remove the artifact manifest and this would define how artifact authors can use the config without content. If there is any problem add this to 1.1 release please respond or we can bring this up in the call next week.

vbatts · 2023-02-24T22:25:47Z

updated, and including verbiage for layers. PTAL

sudo-bmitch

I'm leaning towards moving most of the text into a "guidance" section, to make a clear separation between the spec and recommended usage of the spec. In the guidance I'd focus on an example with a config and artifact layer first, with the scratch example as a recommended solution only for extreme scenarios. We also need to clarify the deviation of the media type from the blob contents in some scenarios.

sudo-bmitch · 2023-02-24T22:58:51Z

specs-go/v1/mediatype.go

@@ -70,6 +70,9 @@ const (
 	// MediaTypeImageConfig specifies the media type for the image configuration.
 	MediaTypeImageConfig = "application/vnd.oci.image.config.v1+json"

+	// MediaTypeImageScratch specifies the media type for a SCRATCH blob.
+	MediaTypeImageScratch = "application/vnd.oci.example+json"


This should be a custom value for each implementation, so I don't think there's a way to include it in the spec.

Agree, there are 0 use cases for this.

sudo-bmitch · 2023-02-24T23:00:30Z

specs-go/v1/manifest.go

+const ScratchDigestSHA256 = `sha256:44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a`
+
+// ScratchDescriptor is a SCRATCH descriptor for Manifest's `config` when that manifest does not have a config to provide.
+func ScratchDescriptor() Descriptor {


I'd be interested in hearing the use cases for the full descriptor to be sure we aren't adding complexity that isn't needed.

I think it makes it easier for clients to create a base descriptor object. Its a good helper method.
Is there really a concern having this method? Hoping to get this or the @sudo-bmitch's PR through or we combine the 2 to and merge 999.

specs-go/v1/manifest.go

sajayantony · 2023-02-25T02:05:27Z

I'm leaning towards moving most of the text into a "guidance" section

@sudo-bmitch I think the data of {} and hash being in the go files are helpful and will avoid a lot of inconsistencies when clients create this and coming back to spec for the hash/data can be avoided. Does that sound reasonable?

sudo-bmitch · 2023-02-25T02:31:53Z

I'm leaning towards moving most of the text into a "guidance" section

@sudo-bmitch I think the data of {} and hash being in the go files are helpful and will avoid a lot of inconsistencies when clients create this and coming back to spec for the hash/data can be avoided. Does that sound reasonable?

I like having the digest and the data, but the rest of the descriptor bothers me. It can't be compared since we don't know the media type, and other fields could be defined in the descriptor being compared. And it's a mix of an input (the digest) and an output (the returned blob content) in the same struct, so any usage of that feels awkward to me. See #1029 where I just set them as two const values instead.

AkihiroSuda · 2023-02-27T02:09:33Z

manifest.md

@@ -42,6 +42,12 @@ Unlike the [image index](image-index.md), which contains information about a set

        If the manifest uses a different media type than the above, it 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].

+    To set an effectively NULL or SCRATCH config and maintain portability the following is considered GUIDANCE.
+    While an empty blob (`size` of 0) may be preferable, practice has shown that not to be ubiquitiously supported.


practice has shown that not to be ubiquitiously supported.

This might not be a problem, because implementations do not need to unmarshal this "JSON" object (and the unmarshallized object is simply useless)

Practice has shown zero size blobs to fail.

Implementations lack any information to determine whether the `config` property of an Image Manifest describes the `artifactType` of the manifest, or the media type of the referenced content. This is necessitated by the removal of artifact manifest (opencontainers#999) and addition of scratch descriptors (opencontainers#1023) to support artifacts. If opencontainers#999 is withdrawn, this pull request should be withdrawn as well and opencontainers#1028 considered instead.

Implementations lack any information to determine whether the `config` property of an Image Manifest describes the `artifactType` of the manifest, or the media type of the referenced content. This is necessitated by the removal of artifact manifest (opencontainers#999) and addition of scratch descriptors (opencontainers#1023) to support artifacts. If opencontainers#999 is withdrawn, this pull request should be withdrawn as well and opencontainers#1028 considered instead. Signed-off-by: Aaron Friel <mayreply@aaronfriel.com>

Ref: opencontainers#1025 Through the conversations between artifact manifest and standardizing the misuse of the image-manifest one of the topics was around when there is _not_ a `config` needed for the `layers`/blobs. Since the `config` is a REQUIRED field, it meant setting this to some valid value. This guidance intends to set a norm for a blob that need only be pushed to a registry a single time, and then save on round trips for all future SCRATCH configs, while also being most widely portability. to facilitate use by tools that import this reference code to identify this specific blob when working with an empty/scratch config blob and layer blob. This will utilize the same blob for both Signed-off-by: Vincent Batts <vbatts@hashbangbash.com>

sudo-bmitch

I've got a few nits that I'll put in #1029 later. This looks good to move forwards. Thanks!

vbatts requested review from jonjohnsonjr, jonboulle, stevvooe, sudo-bmitch, sajayantony and cyphar as code owners February 12, 2023 17:47

brackendawson reviewed Feb 12, 2023

View reviewed changes

manifest.md Outdated Show resolved Hide resolved

vbatts force-pushed the scratch_config branch from 4bcc39f to 95b5dc7 Compare February 13, 2023 12:16

tianon reviewed Feb 13, 2023

View reviewed changes

specs-go/v1/manifest.go Outdated Show resolved Hide resolved

vbatts force-pushed the scratch_config branch from 95b5dc7 to e978aea Compare February 13, 2023 18:27

vbatts force-pushed the scratch_config branch from e978aea to 68a742c Compare February 13, 2023 19:27

vbatts force-pushed the scratch_config branch from 68a742c to 9642ef4 Compare February 13, 2023 21:32

tianon reviewed Feb 13, 2023

View reviewed changes

specs-go/v1/manifest.go Outdated Show resolved Hide resolved

vbatts force-pushed the scratch_config branch 2 times, most recently from 2174a9b to 86570c6 Compare February 16, 2023 10:31

sajayantony previously approved these changes Feb 16, 2023

View reviewed changes

sudo-bmitch reviewed Feb 16, 2023

View reviewed changes

manifest.md Outdated Show resolved Hide resolved

AaronFriel mentioned this pull request Feb 17, 2023

Remove artifact manifest #999

Merged

sajayantony added this to the v1.1 milestone Feb 24, 2023

vbatts dismissed sajayantony’s stale review via 242f551 February 24, 2023 19:59

vbatts force-pushed the scratch_config branch 3 times, most recently from 7a8bdc6 to 1454d6b Compare February 24, 2023 22:24

vbatts changed the title ~~manifest: provide guidance on SCRATCH config descriptor~~ manifest: provide guidance on SCRATCH descriptor (config and layer) Feb 24, 2023

sudo-bmitch requested changes Feb 24, 2023

View reviewed changes

sudo-bmitch reviewed Feb 25, 2023

View reviewed changes

specs-go/v1/manifest.go Outdated Show resolved Hide resolved

sudo-bmitch mentioned this pull request Feb 25, 2023

Add guidelines for artifacts #1029

Closed

AkihiroSuda reviewed Feb 27, 2023

View reviewed changes

qweeah mentioned this pull request Mar 2, 2023

feat!: disable auto type conversion for manifests oras-project/oras#797

Merged

AaronFriel mentioned this pull request Mar 2, 2023

image manifest: support use as artifact #1030

Merged

vbatts mentioned this pull request Mar 2, 2023

manifest: clarify the zero length layers array #1013

Closed

vbatts force-pushed the scratch_config branch 2 times, most recently from fae4eb6 to 87f98aa Compare March 10, 2023 16:27

vbatts force-pushed the scratch_config branch from 87f98aa to 729a03e Compare March 10, 2023 16:31

sajayantony approved these changes Mar 10, 2023

View reviewed changes

sajayantony requested a review from sudo-bmitch March 10, 2023 16:56

sudo-bmitch approved these changes Mar 10, 2023

View reviewed changes

sudo-bmitch merged commit 2a860ae into opencontainers:main Mar 10, 2023

vbatts deleted the scratch_config branch April 6, 2023 18:00

sajayantony mentioned this pull request Apr 13, 2023

Rename the scratch variable and layer requirements #1042

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

manifest: provide guidance on SCRATCH descriptor (config and layer) #1023

manifest: provide guidance on SCRATCH descriptor (config and layer) #1023

vbatts commented Feb 12, 2023 •

edited

Loading

vbatts commented Feb 12, 2023

sajayantony commented Feb 13, 2023

vbatts commented Feb 13, 2023

vbatts commented Feb 13, 2023 via email

tianon commented Feb 13, 2023

sajayantony commented Feb 13, 2023 •

edited

Loading

vbatts commented Feb 13, 2023

vbatts commented Feb 13, 2023

vbatts commented Feb 16, 2023

brackendawson commented Feb 16, 2023

sajayantony commented Feb 24, 2023

vbatts commented Feb 24, 2023

sudo-bmitch left a comment

sudo-bmitch Feb 24, 2023

AaronFriel Mar 2, 2023

sudo-bmitch Feb 24, 2023

sajayantony Mar 9, 2023

sajayantony commented Feb 25, 2023 •

edited

Loading

sudo-bmitch commented Feb 25, 2023 •

edited

Loading

AkihiroSuda Feb 27, 2023

brackendawson Feb 27, 2023

vbatts Mar 2, 2023

sudo-bmitch left a comment

manifest: provide guidance on SCRATCH descriptor (config and layer) #1023

manifest: provide guidance on SCRATCH descriptor (config and layer) #1023

Conversation

vbatts commented Feb 12, 2023 • edited Loading

vbatts commented Feb 12, 2023

sajayantony commented Feb 13, 2023

vbatts commented Feb 13, 2023

vbatts commented Feb 13, 2023 via email

tianon commented Feb 13, 2023

sajayantony commented Feb 13, 2023 • edited Loading

vbatts commented Feb 13, 2023

vbatts commented Feb 13, 2023

vbatts commented Feb 16, 2023

brackendawson commented Feb 16, 2023

sajayantony commented Feb 24, 2023

vbatts commented Feb 24, 2023

sudo-bmitch left a comment

Choose a reason for hiding this comment

sudo-bmitch Feb 24, 2023

Choose a reason for hiding this comment

AaronFriel Mar 2, 2023

Choose a reason for hiding this comment

sudo-bmitch Feb 24, 2023

Choose a reason for hiding this comment

sajayantony Mar 9, 2023

Choose a reason for hiding this comment

sajayantony commented Feb 25, 2023 • edited Loading

sudo-bmitch commented Feb 25, 2023 • edited Loading

AkihiroSuda Feb 27, 2023

Choose a reason for hiding this comment

brackendawson Feb 27, 2023

Choose a reason for hiding this comment

vbatts Mar 2, 2023

Choose a reason for hiding this comment

sudo-bmitch left a comment

Choose a reason for hiding this comment

vbatts commented Feb 12, 2023 •

edited

Loading

sajayantony commented Feb 13, 2023 •

edited

Loading

sajayantony commented Feb 25, 2023 •

edited

Loading

sudo-bmitch commented Feb 25, 2023 •

edited

Loading