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

feat: implement referrers deletion #54

Merged
merged 7 commits into from
Jan 6, 2023
Merged

feat: implement referrers deletion #54

merged 7 commits into from
Jan 6, 2023

Conversation

wangxiaoxuan273
Copy link

@wangxiaoxuan273 wangxiaoxuan273 commented Jan 5, 2023

Part 2 of and Resolves #37

Basic Test result using oras:
delete a referrer of an artifact manifest
image
image

delete a referrer of an image manifest
image

Signed-off-by: wangxiaoxuan273 wangxiaoxuan119@gmail.com

@codecov-commenter
Copy link

codecov-commenter commented Jan 5, 2023

Codecov Report

Merging #54 (225e634) into main (474bc48) will decrease coverage by 0.00%.
The diff coverage is 55.55%.

@@            Coverage Diff             @@
##             main      #54      +/-   ##
==========================================
- Coverage   56.28%   56.27%   -0.01%     
==========================================
  Files         108      108              
  Lines       11213    11239      +26     
==========================================
+ Hits         6311     6325      +14     
- Misses       4194     4202       +8     
- Partials      708      712       +4     
Impacted Files Coverage Δ
registry/storage/paths.go 69.31% <ø> (ø)
registry/storage/manifeststore.go 72.64% <55.55%> (-5.38%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

}

// Ensure the blob is available for deletion
_, err := ms.blobStore.blobAccessController.Stat(ctx, dgst)

Choose a reason for hiding this comment

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

We can do ms.blobStore.Stat() to get the descriptor. If the media type is not MediaTypeArtifactManifest or MediaTypeImageManifest, we can do ms.blobStore.Delete(ctx, dgst) directly.

Copy link
Author

@wangxiaoxuan273 wangxiaoxuan273 Jan 5, 2023

Choose a reason for hiding this comment

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

The descriptor we get from ms.blobStore.Stat() has an incorrect media type.

registry/storage/manifeststore.go Outdated Show resolved Hide resolved
registry/storage/manifeststore.go Outdated Show resolved Hide resolved
registry/storage/manifeststore.go Show resolved Hide resolved
registry/storage/manifeststore.go Outdated Show resolved Hide resolved
if err != nil {
return fmt.Errorf("failed to generate referrers link path for %v", dgst)
}
if err = ms.repository.driver.Delete(ctx, referrersLinkPath); err != nil {

Choose a reason for hiding this comment

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

What if subject is specified but does not exist (as mentioned in #52)?

Copy link
Author

@wangxiaoxuan273 wangxiaoxuan273 Jan 5, 2023

Choose a reason for hiding this comment

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

If the subject does not exist in the repository, the referrers feature just works as usual, which is technically conformant to the distribution spec.

Signed-off-by: wangxiaoxuan273 <wangxiaoxuan119@gmail.com>
Signed-off-by: wangxiaoxuan273 <wangxiaoxuan119@gmail.com>
Signed-off-by: wangxiaoxuan273 <wangxiaoxuan119@gmail.com>
Signed-off-by: wangxiaoxuan273 <wangxiaoxuan119@gmail.com>
Signed-off-by: wangxiaoxuan273 <wangxiaoxuan119@gmail.com>
Signed-off-by: wangxiaoxuan273 <wangxiaoxuan119@gmail.com>
Signed-off-by: wangxiaoxuan273 <wangxiaoxuan119@gmail.com>
Copy link

@shizhMSFT shizhMSFT left a comment

Choose a reason for hiding this comment

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

LGTM

@shizhMSFT shizhMSFT merged commit f27b092 into oras-project:main Jan 6, 2023
@wangxiaoxuan273 wangxiaoxuan273 deleted the del-referrers branch January 6, 2023 09:31
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.

Enable artifact and referrers deletion.
3 participants