Skip to content
This repository has been archived by the owner on Sep 23, 2022. It is now read-only.

Update Proposal C with feedback and requirements #37

Merged
merged 1 commit into from
May 25, 2022

Conversation

nishakm
Copy link
Contributor

@nishakm nishakm commented Apr 1, 2022

  • Do not allow Node manifests to contain objects that link to
    other Node manifests.
  • Add how proposal solves requirements at the bottom.

Signed-off-by: nisha [email protected]

Copy link
Member

@jdolitsky jdolitsky left a comment

Choose a reason for hiding this comment

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

hold merge until after #35

docs/proposals/PROPOSAL_C.md Outdated Show resolved Hide resolved
docs/proposals/PROPOSAL_C.md Show resolved Hide resolved
- Do not allow Node manifests to contain objects that link to
  other Node manifests.
- Add how proposal solves requirements at the bottom.

Signed-off-by: nisha <[email protected]>
@nishakm nishakm force-pushed the proposal-c-updates branch from 64a0b1e to 69bece1 Compare April 7, 2022 15:37
Copy link
Contributor

@sudo-bmitch sudo-bmitch left a comment

Choose a reason for hiding this comment

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

Apologies for the delayed review. No need to block on any of my comments.

1. As a tool writer, I would like the option to perform a server side blob mount when copying a large artifact between repositories.
- Yes: If a large blob is pushed to the registry and then referenced by a Node manifest, a server side mount should be possible.
1. As a tool writer, I want to be able to include reference types within the Image Layout filesystem format.
- Yes: The existing image layout will be unaffected.
Copy link
Contributor

Choose a reason for hiding this comment

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

With client tooling that copies between registries and the layout spec, I'm not sure how I'd differentiate /v2 and /v3 in layout spec content.

1. As a user, I want to store one or more artifacts in a registry with a reference to another related artifact.
- Yes: Node manifests can group one or more artifacts with a reference to another manifest.
1. As a user, when I delete an artifact, I want the option to delete one or more artifacts referencing it.
- Partial: use `GET /v3/<name>/referrers/<reference>` to get a list of referring artifacts, use the client to delete specific ones, then push a new Node manifest containing the updated object list. To delete the whole manifest, use `DELETE /v3/<name>/manifests/<reference>`.
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 GC policies can be written that support the referrers too.

1. As a user, I want to push an artifact that references another artifact that doesn't exist on the registry yet.
- Yes: This is should be possible if registries don't perform a check that the digest exists.
1. As a user, I want to move an artifact and one or more artifacts referencing it from one registry to another.
- Partial: The client is responsible for selection of the artifacts and pushing to the target registries, including checks if the registry supports node manifests.
Copy link
Contributor

Choose a reason for hiding this comment

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

The only question mark for me is gracefully handling backwards compatibility that doesn't require pulling every reference on every sync. Otherwise I'd call this a yes, even though it requires client tooling, because there's no server side tooling in OCI for copying between registries today.

1. As an artifact producer, I want to update an existing artifact with a newer artifact.
- Partial: use `GET /v3/<name>/manifests/<reference>` to fetch a node manifest, then update the objects list with a new artifact.
1. As an artifact producer, I want to push multiple artifacts concurrently (possibly from separate systems) without encountering race conditions or other errors.
- No: If two manifests with the same digest are pushed to the same registry path, a race condition will occur. However, this problem exists in current registries.
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 one is a yes. Pushing the same digest means you're pushing the same artifact, and registries gracefully handle that today (idempotent). But, if we rely on the digest a tag points to to be unchanged between a pull and a push (to add entries into an index or node list), then we have a race.

1. As an artifact producer, I want to push multiple artifacts concurrently (possibly from separate systems) without encountering race conditions or other errors.
- No: If two manifests with the same digest are pushed to the same registry path, a race condition will occur. However, this problem exists in current registries.
1. As an artifact author, I want to document other artifacts in one or more registries that my artifact requires and/or provides.
- Partial: Use the description to indicate the reference is a "requires" or "provides" relationship.
Copy link
Contributor

Choose a reason for hiding this comment

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

This one depends on whether registries are validating that referred artifacts exist. Cross repository or even registry descriptor references won't work with most of our proposals if there's validation.

@jdolitsky jdolitsky merged commit 2cd1ed2 into opencontainers:main May 25, 2022
@nishakm nishakm deleted the proposal-c-updates branch June 30, 2022 13:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants