-
Notifications
You must be signed in to change notification settings - Fork 222
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
OCI Distribution Improvements #682
Conversation
Signed-off-by: Kevin Flansburg <kevin.flansburg@gmail.com>
Signed-off-by: Kevin Flansburg <kevin.flansburg@gmail.com>
Signed-off-by: Kevin Flansburg <kevin.flansburg@gmail.com>
Signed-off-by: Kevin Flansburg <kevin.flansburg@gmail.com>
3599fea
to
9b4afee
Compare
These are all great changes, and they map back to OCI's expectations as to how these JWTs should be treated. Have you had a chance to look at the failing CI checks yet? Looks like legitimate type type mismatch and compiler errors. |
I looked at the changes too, this looks good 👏 |
Signed-off-by: Kevin Flansburg <kevin.flansburg@gmail.com>
8db3ad5
to
478c8df
Compare
Signed-off-by: Kevin Flansburg <kevin.flansburg@gmail.com>
@bacongobbler Ok, I merged main to reconcile the changes introduced from #670 . I've also addressed the type errors in the test. I dont believe the license error from cargo deny is related to this PR. |
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.
LGTM, thanks for having fixed the tests.
I agree with you about the license check, the failure doesn't seem to be related with this PR
This PR includes a number of improvements for OCI distribution:
fetch_manifest_digest
, including any authentication API calls, does not incur ratelimits.None
if a token is in the cache but is expired.Combined, these changes should make it possible to reuse the OCI client over long periods of time, with more efficient registry API usage.
Other optimizations which could be made: