-
Notifications
You must be signed in to change notification settings - Fork 528
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
Support for OCI 1.1+ referrers via API #1546
Conversation
Signed-off-by: Josh Dolitsky <josh@dolit.ski>
d3bb955
to
224b9c7
Compare
Codecov Report
@@ Coverage Diff @@
## main #1546 +/- ##
==========================================
- Coverage 73.07% 72.90% -0.18%
==========================================
Files 117 118 +1
Lines 9082 9364 +282
==========================================
+ Hits 6637 6827 +190
- Misses 1780 1845 +65
- Partials 665 692 +27
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
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.
Can we add support for referrers to pkg/registry
, and use that to test this path?
Signed-off-by: Josh Dolitsky <josh@dolit.ski>
Signed-off-by: Josh Dolitsky <josh@dolit.ski>
Signed-off-by: Josh Dolitsky <josh@dolit.ski>
Signed-off-by: Josh Dolitsky <josh@dolit.ski>
Signed-off-by: Josh Dolitsky <josh@dolit.ski>
// WithReferrersSupport enables the referrers API endpoint (OCI 1.1+) | ||
func WithReferrersSupport(enabled bool) Option { | ||
return func(r *registry) { | ||
r.referrersEnabled = enabled | ||
} | ||
} |
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.
this is disabled by default, but we could go the other direction too
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.
Very nice! I think we could merge like this, just a couple questions. (edit: one question.)
// Check if the registry supports Referrers API. | ||
// TODO: This should be done once per registry, not once per subject. | ||
u := w.url(fmt.Sprintf("/v2/%s/referrers/%s", w.repo.RepositoryStr(), sub.DigestStr())) | ||
req, err := http.NewRequest(http.MethodGet, u.String(), nil) | ||
if err != nil { | ||
return err | ||
} | ||
req.Header.Set("Accept", string(types.OCIImageIndex)) | ||
resp, err := w.client.Do(req.WithContext(ctx)) | ||
if err != nil { | ||
return err | ||
} | ||
defer resp.Body.Close() | ||
|
||
if err := transport.CheckError(resp, http.StatusOK, http.StatusNotFound, http.StatusBadRequest); err != nil { | ||
return err | ||
} | ||
if resp.StatusCode == http.StatusOK { | ||
// The registry supports Referrers API. The registry is responsible for updating the referrers list. | ||
return nil | ||
} |
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.
Sounds like this will all go away once opencontainers/distribution-spec#379 is in, do we want to just start supporting that in here?
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.
i think id rather see it merged first, then come back to change here
I'll let @jonjohnsonjr clickey the mergey |
Follow-up to #1543, use the referrers API endpoint if the registry supports it, otherwise fallback to digest tag