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

Working Group Proposal for Reference Types #335

Merged
merged 1 commit into from
Aug 25, 2022

Conversation

michaelb990
Copy link
Contributor

@michaelb990 michaelb990 commented Aug 9, 2022

This is a culmination of many people's work over the last few months to introduce reference types to OCI as part of the OCI Reference Types WG.

This PR contains the changes from our chosen proposal, Proposal E, relevant to the distribution-spec. We're looking forward to hearing feedback from distribution-spec maintainers on these changes!

We have also opened a similar PR for the image-spec: opencontainers/image-spec#934

jdolitsky
jdolitsky previously approved these changes Aug 10, 2022
@sudo-bmitch
Copy link
Contributor

sudo-bmitch commented Aug 11, 2022

@opencontainers/distribution-spec-maintainers given the size of this, we are giving this 2 weeks for any objections before merging. It was presented on the OCI call today and hopefully a recording will be posted soon. After merging it still won't be a tagged release, so if there are any issues raised, we can always open another PR.

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.

partial review pls see comments

client-implementation.md Outdated Show resolved Hide resolved
client-implementation.md Outdated Show resolved Hide resolved
client-implementation.md Outdated Show resolved Hide resolved
client-implementation.md Outdated Show resolved Hide resolved
client-implementation.md Outdated Show resolved Hide resolved
client-implementation.md Outdated Show resolved Hide resolved
client-implementation.md Outdated Show resolved Hide resolved
client-implementation.md Outdated Show resolved Hide resolved
client-implementation.md Outdated Show resolved Hide resolved
spec.md Outdated Show resolved Hide resolved
spec.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
jdolitsky
jdolitsky previously approved these changes Aug 18, 2022
spec.md Show resolved Hide resolved
spec.md Show resolved Hide resolved
*Note: this feature was added in distibution-spec 1.1.
Registries should see [Enabling the Referrers API](#enabling-the-referrers-api) before enabling this.*

To fetch the list of referrers, perform a `GET` request to a path in the following format: `/v2/<name>/referrers/<reference>` <sup>[end-12a](#endpoints)</sup>.
Copy link
Member

Choose a reason for hiding this comment

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

Reading through this and the examples below, I'm not actually clear on what string <reference> actually is. Media Type? digest? arbitrary value?

spec.md Show resolved Hide resolved
Example request with filtering:

```
GET /v2/<name>/referrers/<ref>?artifactType=application/vnd.example.sbom.v1
Copy link
Member

Choose a reason for hiding this comment

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

it's <ref> here, but <reference> everywhere else

### Backwards Compatibility

Client implementations MUST support registries that implement partial or older versions of the OCI Distribution Spec.
This section describes client fallback procedures that MUST be implemented when a new/optional API is not available from a Registry.
Copy link
Member

Choose a reason for hiding this comment

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

right, so, if there is a backward-compat condition, then is it required that this feature bumps the version to 1.1?

This tag should return an Image Index matching the expected response of the [Referrers API](#listing-referrers).
Maintaining the content of this tag is the responsibility of clients pushing and deleting Image and Artifact Manifests that contain a `refers` field.

Multiple clients could attempt to update the tag simultaneously resulting in race conditions and data loss.
Copy link
Member

Choose a reason for hiding this comment

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

**Note** or **Warning**?

@vbatts vbatts added this to the v1.1.0 milestone Aug 25, 2022
@sudo-bmitch sudo-bmitch dismissed stale reviews from jdolitsky and themself via e92b59d August 25, 2022 18:13
sudo-bmitch
sudo-bmitch previously approved these changes Aug 25, 2022
This is a culmination of many people's work over the last few months to introduce reference types to OCI as part of the [OCI Reference Types WG](oci-ref-type-wg).
This PR contains the changes from our chosen proposal, [Proposal E][proposal-e].

[oci-ref-type-wg]: https://github.com/opencontainers/tob/blob/main/proposals/wg-reference-types.md
[proposal-e]: https://github.com/opencontainers/wg-reference-types/blob/main/docs/proposals/PROPOSAL_E.md

Co-authored-by: Josh Dolitsky <josh@dolit.ski>
Co-authored-by: Josh Dolitsky <393494+jdolitsky@users.noreply.github.com>
Co-authored-by: Michael Brown <brownxmi@amazon.com>
Signed-off-by: Brandon Mitchell <git@bmitch.net>
@estesp
Copy link

estesp commented Aug 25, 2022

I said this on image-spec as well, but want to also share here: thanks so much to those who worked on this both in the WG and then getting this PR prepped (and dealing with all the feedback) and ready! IANAM but LGTM! 🎉

Copy link
Member

@sajayantony sajayantony left a comment

Choose a reason for hiding this comment

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

IANAM! But LGTM and thanks for opening the follow up issue.

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.

LGTM to move progress to subsequent PRs addressing punch list items..

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.

10 participants