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

Adding proposal D, "no changes" #13

Merged
merged 2 commits into from
Mar 8, 2022

Conversation

sudo-bmitch
Copy link
Contributor

@sudo-bmitch sudo-bmitch commented Mar 2, 2022

Signed-off-by: Brandon Mitchell [email protected]

This is proposal D from our discussions. It looks at how reference types could be implemented without changes to the current OCI specs, similar to the current OCI Artifact definition. I'm assuming this won't satisfy everyone's needs, but it gives us a baseline to compare other proposals, and also provides a fallback option for how to work with registries that don't support any APIs we add.

Fixes #18

Copy link
Member

@imjasonh imjasonh left a comment

Choose a reason for hiding this comment

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

Thanks for this proposal! An option that relies on conventions within the existing specs is worth considering in my opinion. 👍

"variant": "v7"
},
"annotations": {
"vnd.oci.artifact.sig.data": "<base64-encoded-artifact>" // embedding small artifacts
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup. Though in this case the artifact is different from the content the descriptor points to. I.e. if all the signatures are small enough, a multi-platform index could be updated with only annotations, reducing the risk of runtime issues picking the wrong descriptor.

Comment on lines 74 to 77
"platform": {
"architecture": "arm64",
"os": "linux"
},
Copy link
Member

Choose a reason for hiding this comment

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

Is there a concern with this that a runtime might incorrectly pull this SBOM and interpret it as an image? It'd be looking for mediaType == "image" && platform == $myplatform, which this satisfies.

I guess that's what language introduced in opencontainers/image-spec#880 is meant to guard against.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's certainly a concern of that. Also if we have a reference to an index, we could send up with an index pointing to an index, which may not be traversed by runtimes. Potentially we could use annotations rather than the platform section to associate one descriptor to another.

"os": "linux"
},
"annotations": {
"vnd.oci.artifact": "true", // this descriptor points to an artifact
Copy link
Member

Choose a reason for hiding this comment

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

nit: Would it be enough to say that if a artifact.type is set, that it's an artifact? That would let us remove one more moving part in this proposal.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My thought on this one is we only depend on the top level artifact = true field, and everything else is an optional field that can be used by clients to filter their future requests.

@jdolitsky
Copy link
Member

I happy to see this roll in. Would you be willing to rework this into a new format once we come up a template like we discussed on the call yesterday? @sudo-bmitch

@sudo-bmitch
Copy link
Contributor Author

@jdolitsky Absolutely. Just wanted to get this started while it was fresh in my head.

@jdolitsky
Copy link
Member

See #14 for proposal template PR

@nishakm nishakm mentioned this pull request Mar 4, 2022
@jdolitsky
Copy link
Member

@sudo-bmitch please see #18 - can you re-format this, or open another PR?

Signed-off-by: Brandon Mitchell <[email protected]>
README.md Outdated Show resolved Hide resolved
Signed-off-by: Brandon Mitchell <[email protected]>
@jdolitsky jdolitsky merged commit 2065623 into opencontainers:main Mar 8, 2022
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.

Submission for proposal D
3 participants