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

purge: do not delete contents of oci indexes #130

Merged
merged 1 commit into from
Feb 1, 2023

Conversation

jedevc
Copy link
Contributor

@jedevc jedevc commented Jan 27, 2023

The purge command attempts to detect if manifest lists are being used, and will avoid deleting manifests that are part of them.

The OCI specification defines an index media type, that can contain manifests as well, as an alternative to manifest lists - functionally, these are the same as manifest lists, and ACR appears to support them.

The purge command should therefore avoid removing manifests that are part of indexes as well, which, if the user is using OCI images, will cause their images to be unintentionally deleted.

See docker/build-push-action#778 (comment) for more information, where this affected docker buildx users using ACR.

@northtyphoon
Copy link
Member

northtyphoon commented Feb 1, 2023

@jedevc thanks for the contribution. Can you please add a unit test for the new oci index multi-arch scenario? You can take a look an existing one under https://github.com/Azure/acr-cli/blob/main/cmd/acr/purge_test.go#L380

Copy link
Member

@northtyphoon northtyphoon left a comment

Choose a reason for hiding this comment

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

Can you update the unit test to test the oci index media type?

The purge command attempts to detect if manifest lists are being used,
and will avoid deleting manifests that are part of them.

The OCI specification defines an index media type, that can contain
manifests as well, as an alternative to manifest lists - functionally,
these are the same as manifest lists, and ACR appears to support them.

The purge command should therefore avoid removing manifests that are
part of indexes as well, which, if the user is using OCI images, will
cause their images to be unintentionally deleted.

Signed-off-by: Justin Chadwell <me@jedevc.com>
@jedevc
Copy link
Contributor Author

jedevc commented Feb 1, 2023

@microsoft-github-policy-service agree [company="Docker Inc"]

@jedevc
Copy link
Contributor Author

jedevc commented Feb 1, 2023

@microsoft-github-policy-service agree company="Docker Inc"

Copy link
Member

@northtyphoon northtyphoon left a comment

Choose a reason for hiding this comment

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

LGTM

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