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

conformance: don't require http 202 for already-deleted manifest blob #478

Conversation

waynr
Copy link
Contributor

@waynr waynr commented Oct 8, 2023

In my registry implementation a DELETE /v2/<name>/manifests/<reference>
request deletes the manifest blob.

In the conformance tests the DELETE /v2/<name>/manifests/<reference> calls
are followed by DELETE /v2/<name>/blobs/<digest> where <digest> refers to
the digest of the manifest referred to by <reference>.

I understand why the tests are written this way - most registries seem to be
implemented in a way that it's not necessarily safe to automatically delete a
blob just because the last thing to reference it was also deleted, but my
implementation is based on storing relational metadata in an ACID-compliant DB
in such a way that bulk data is the only thing stored in the backing object
store (which for data race protection reasons isn't even keyed by content
digest but randomly-assigned UUID). Because of this design choice, it is safe
to at least attempt deleting blobs themselves when the manifest is deleted.

So because it is possible to safely delete the manifest blob during the DELETE /v2/<name>/manifests/<reference> calls, the subsequent attempt to explicitly
delete the blob in the test case should allow a 404 response.

@waynr waynr changed the title conformance: don't require http 202 for already-deleted manifest config conformance: don't require http 202 for already-deleted manifest blob Oct 8, 2023
@waynr waynr force-pushed the dont-require-http-202-for-already-deleted-manifest-config branch from 6af86bb to df1548b Compare October 8, 2023 16:47
Signed-off-by: wayne warren <wayne.warren.s@gmail.com>
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.

LGTM

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

@mikebrow mikebrow merged commit 3f68583 into opencontainers:main Oct 12, 2023
5 checks passed
@jdolitsky jdolitsky mentioned this pull request Nov 7, 2023
8 tasks
@jdolitsky jdolitsky mentioned this pull request Jan 11, 2024
8 tasks
@sudo-bmitch sudo-bmitch mentioned this pull request Feb 1, 2024
8 tasks
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.

None yet

3 participants