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

Add reference field #828

Closed
wants to merge 1 commit into from
Closed

Add reference field #828

wants to merge 1 commit into from

Conversation

dlorenc
Copy link

@dlorenc dlorenc commented Mar 17, 2021

This accompanies a full proposal.

Fixes #827

Signed-off-by: Dan Lorenc dlorenc@google.com

Signed-off-by: Dan Lorenc <dlorenc@google.com>
@dlorenc dlorenc mentioned this pull request Mar 17, 2021
3 tasks
{
"mediaType": "application/vnd.example.signature+json",
"size": 3514,
"digest": "sha256:19387f68117dbe07daeef0d99e018f7bbf7a660158d24949ea47bc12a3e4ba17",
Copy link

Choose a reason for hiding this comment

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

this is the kind of place where multicodecs are useful https://github.com/multiformats/multicodec/blob/master/table.csv#L9

@SteveLasker
Copy link
Contributor

This is largely a dupe of the oci.artifact.manifest proposal. The main difference here is the references property is placed on a descriptor, as opposed to being in the manifest. Contemplating how to track garbage collection on every descriptor is something we'd have to think about,
With the overview previously defined, I've added spec details:
opencontainers/artifacts#29

@dlorenc
Copy link
Author

dlorenc commented Mar 18, 2021

This is largely a dupe of the oci.artifact.manifest proposal. The main difference here is the references property is placed on a descriptor, as opposed to being in the manifest

I wouldn't really call it a dupe. They're complementary approaches with a few subtle but important differences. I've outlined those differences in the linked issue.

The main difference here is the references property is placed on a descriptor, as opposed to being in the manifest.

In my proposal it's placed on ALL the existing types. That means the descriptor, the manifest, and the index types, not only the manifest. The artifacts proposal only adds it to the proposed artifact type, IIUC.

Copy link
Contributor

@nishakm nishakm left a comment

Choose a reason for hiding this comment

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

The reference seems to describe a 1:1 relationship with the descriptor and another mediaType. However there are situations where one needs to describe an n:1 relationship such as n SBoMs to one image or a 1:n relationship such as 1 artifact is linked to n other artifacts. Would a descriptor reference be able to handle this?

@@ -44,6 +44,11 @@ The following fields contain the primary properties that constitute a Descriptor
This OPTIONAL property contains arbitrary metadata for this descriptor.
This OPTIONAL property MUST use the [annotation rules](annotations.md#rules).

- **`reference`** *descriptor*

This OPTIONAL property contains a descriptor is the subject of the content described by this descriptor.
Copy link
Contributor

Choose a reason for hiding this comment

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

"which is the subject of"

"size": 3514,
"digest": "sha256:19387f68117dbe07daeef0d99e018f7bbf7a660158d24949ea47bc12a3e4ba17",
"reference": {
"mediaType": "application/vnd.docker.distribution.manifest.list.v2+json",
Copy link
Contributor

Choose a reason for hiding this comment

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

Is a user supposed to read this as "this is the signature of the following manifest"? How would this work with "this is a list of SBoMs for the following manifest"? or "this is a list of signatures for the following manifest"?

@dlorenc
Copy link
Author

dlorenc commented Mar 27, 2021

FYI: I updated the tracking issue here with some details on testing/validation. I've been unable to break any clients with this so far, but I appreciate any help in trying!

manifest.md Show resolved Hide resolved
Copy link
Member

@mikebrow mikebrow left a comment

Choose a reason for hiding this comment

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

It would be nice to have a single place in the spec where this feature is discussed... saying something like to add ___ capabilities.. the reference field was added to the base descriptor type. The following types inherit from descriptor.. ___ Thus the reference field is being added to these types. This allows ___.. Example:

"mediaType": "application/vnd.example.signature+json",
"size": 3514,
"digest": "sha256:19387f68117dbe07daeef0d99e018f7bbf7a660158d24949ea47bc12a3e4ba17",
"reference": {
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the use case of adding a reference to a descriptor? I understand it in the context of of a manifest or index object, but adding this field to the descriptor is creating lots of strange potential implementations where I'm not seeing the value add in present use cases. Is this being proposed in expectation of a descriptor as a manifest being approved, or are there other use cases?

@sudo-bmitch
Copy link
Contributor

Closing this one since we've merged #934.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Proposal: Add References
8 participants