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

Force oci-mediatypes for containerimage outputs that utilize annotations #3061

Merged
merged 3 commits into from
Aug 26, 2022

Conversation

jedevc
Copy link
Member

@jedevc jedevc commented Aug 24, 2022

Follow up to #2935, dependent on #3038.

The new annotations and attestations features both utilize annotations in the OCI image format. Many registries allow setting these annotation fields on docker formats, however, notably, GCR will reject these objects (see #1730) and only allow them for OCI media types.

To work around this, we enable oci-mediatypes when annotations that require OCI are enabled by the user, printing a warning to the user, similar to how we already do for stargz compression.

In the long-term, we should want to look at switching the default for oci-mediatypes to true, so enabling this for users who want to use the new annotations and attestations is a good way to test drive this, and try to catch any potential issues that might arise.

@jedevc jedevc requested a review from tonistiigi August 24, 2022 10:56
@jedevc jedevc force-pushed the force-ocimediatypes branch from 8b9492b to c40e12e Compare August 24, 2022 11:10
}
}

func (c *ImageCommitOpts) EnableAttestations() {
Copy link
Member

Choose a reason for hiding this comment

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

Why is this called EnableAttestations and not EnableOCITypes (if the helper is needed at all). These opts don't know about attestations afaics.

Copy link
Member Author

Choose a reason for hiding this comment

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

Have reworked the helper to EnableOCITypes that takes a reason as a log message. The helper is useful to get consistent warning messages for the different reasons we might change the oci mediatypes option, so I think it's worth keeping.

@tonistiigi
Copy link
Member

Needs rebase.

jedevc added 3 commits August 26, 2022 10:06
The new annotations and attestations features both utilize annotations
in the oci image format. Many registries allow setting these annotation
fields on docker formats, however, notably, GCR will reject these
objects and only allow them for OCI media types.

To work around this, we enable oci-mediatypes when annotations that
require OCI are enabled by the user, printing a warning to the user,
similar to how we already do for stargz compression.

Signed-off-by: Justin Chadwell <[email protected]>
@jedevc jedevc force-pushed the force-ocimediatypes branch from c40e12e to 0a5265a Compare August 26, 2022 09:25
@jedevc
Copy link
Member Author

jedevc commented Aug 26, 2022

I also noted that the previous annotations tests required containerd for part of it, so I've refactored them to push to a registry and then use the testutil read image utilities which should give us some better coverage.

@tonistiigi tonistiigi merged commit 23837e5 into moby:master Aug 26, 2022
@jedevc jedevc deleted the force-ocimediatypes branch August 30, 2022 10:53
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.

2 participants