-
Notifications
You must be signed in to change notification settings - Fork 11
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(referrers): delete manifest with subject #174
base: main
Are you sure you want to change the base?
feat(referrers): delete manifest with subject #174
Conversation
Signed-off-by: Patrick Pan <[email protected]>
Signed-off-by: Patrick Pan <[email protected]>
Signed-off-by: Patrick Pan <[email protected]>
Signed-off-by: Patrick Pan <[email protected]>
Signed-off-by: Patrick Pan <[email protected]>
Signed-off-by: Patrick Pan <[email protected]>
Signed-off-by: Patrick Pan <[email protected]>
Signed-off-by: Patrick Pan <[email protected]>
Signed-off-by: Patrick Pan <[email protected]>
Signed-off-by: Patrick Pan <[email protected]>
Signed-off-by: Patrick Pan <[email protected]>
Signed-off-by: Patrick Pan <[email protected]>
Signed-off-by: Patrick Pan <[email protected]>
Signed-off-by: Patrick Pan <[email protected]>
Signed-off-by: Patrick Pan <[email protected]>
Signed-off-by: Patrick Pan <[email protected]>
Signed-off-by: Patrick Pan <[email protected]>
Signed-off-by: Patrick Pan <[email protected]>
Signed-off-by: Patrick Pan <[email protected]>
Signed-off-by: Patrick Pan <[email protected]>
Signed-off-by: Patrick Pan <[email protected]>
Signed-off-by: Patrick Pan <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #174 +/- ##
==========================================
- Coverage 83.25% 83.24% -0.01%
==========================================
Files 37 37
Lines 1266 1355 +89
Branches 149 164 +15
==========================================
+ Hits 1054 1128 +74
- Misses 149 158 +9
- Partials 63 69 +6 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Patrick Pan <[email protected]>
Signed-off-by: Patrick Pan <[email protected]>
Signed-off-by: Patrick Pan <[email protected]>
Signed-off-by: Patrick Pan <[email protected]>
Signed-off-by: Patrick Pan <[email protected]>
Signed-off-by: Patrick Pan <[email protected]>
reference.ContentReference = Referrers.ZeroDigest; | ||
var url = new UriFactory(reference, Options.PlainHttp).BuildReferrersUrl(); | ||
var request = new HttpRequestMessage(HttpMethod.Get, url); | ||
var response = Options.HttpClient.SendAsync(request, cancellationToken).ConfigureAwait(true).GetAwaiter() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we await this function and make PingReferrers
async? But async calls are not allowed within the lock
statement. We can try SemaphoreSlim.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The SemaphoreSlim
does not seem right either. We need to consider a lock-free version.
Signed-off-by: Patrick Pan <[email protected]>
using System.Text.Json.Serialization; | ||
|
||
namespace OrasProject.Oras.Registry.Remote; | ||
|
||
public class ResponseException : HttpRequestException | ||
{ | ||
{ | ||
public static readonly string ErrorCodeNameUnknown = "NAME_UNKNOWN"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should it be grouped as an enum for ErrorCode
?
await Repository.DeleteAsync(target, true, cancellationToken).ConfigureAwait(false); | ||
return; | ||
} | ||
var manifest = await FetchAsync(target, cancellationToken).ConfigureAwait(false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The manifest is not verified against the target
descriptor. It is possible that a corrupted manifest is fetched.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to clarify that if this is to, for example, check the digests between target and manifest?
await Repository.DeleteAsync(target, true, cancellationToken).ConfigureAwait(false); | ||
return; | ||
} | ||
var manifest = await FetchAsync(target, cancellationToken).ConfigureAwait(false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The download size of the manifest is not limited / well-guarded. It means it is vulnerable to excessive resource attack.
Signed-off-by: Patrick Pan <[email protected]>
Signed-off-by: Patrick Pan <[email protected]>
Signed-off-by: Patrick Pan <[email protected]>
What this PR does / why we need it
The PR is to implement the feature to delete manifest with subject as per the OCI distribution spec v1.1.0.
Which issue(s) this PR resolves / fixes
Resolves / Fixes #160
Please check the following list