-
Notifications
You must be signed in to change notification settings - Fork 211
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
storage: fix auth compatibility for registry backend #1425
Conversation
Hi @lihuahua123 please add the detailed commit message, and the commit title better be like |
1c02435
to
56617e9
Compare
Codecov Report
@@ Coverage Diff @@
## master #1425 +/- ##
==========================================
- Coverage 46.46% 46.40% -0.07%
==========================================
Files 123 123
Lines 38643 38745 +102
Branches 38643 38745 +102
==========================================
+ Hits 17954 17978 +24
- Misses 19716 19794 +78
Partials 973 973
|
56617e9
to
9b63c1c
Compare
storage/src/backend/registry.rs
Outdated
@@ -188,6 +189,10 @@ struct RegistryState { | |||
// Use RwLock here to avoid using mut backend trait object. | |||
// Example: RwLock<"Bearer <token>"> | |||
// RwLock<"Basic base64(<username:password>)"> | |||
|
|||
// Cache the method of getting auth, get | post | |||
cached_get_auth_method: HashCache, |
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.
How about:
// Cache for the HTTP method when getting auth, possible values: "get", "post".
// Due to the different implementations of various image registries, auth requests
// may use the GET or POST methods, we need to cache the method after the
// fallback, so it can be reused next time and reduce an unnecessary request.
cached_auth_using_http_get: HashCache,
And can we make it as HashCache<bool>
?
struct HashCache(RwLock<HashMap<String, T>>);
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.
In the context ,HashCache is implemented as
struct HashCache(RwLock<HashMap<String, String>>);
if we make it as' HashCache ', it need implement another logic
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 mean the generic type https://doc.rust-lang.org/rust-by-example/generics.html
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 have resolve it with changing the implemention of struct HashCache(RwLock<HashMap<String, String>>);
to struct HashCache<T>(RwLock<HashMap<String, T>>);
.
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.
It is reused as follows.
// Cache for the HTTP method when getting auth, possible values: "get", "post".
// Due to the different implementations of various image registries, auth requests
// may use the GET or POST methods, we need to cache the method after the
// fallback, so it can be reused next time and reduce an unnecessary request.
cached_auth_using_http_get: HashCache<bool>,
// Cache 30X redirect url
// Example: RwLock<HashMap<"<blob_id>", "<redirected_url>">>
cached_redirect: HashCache<String>,
9b63c1c
to
29a2c90
Compare
29a2c90
to
47ef123
Compare
storage/src/backend/registry.rs
Outdated
@@ -189,10 +193,14 @@ struct RegistryState { | |||
// Example: RwLock<"Bearer <token>"> | |||
// RwLock<"Basic base64(<username:password>)"> | |||
cached_auth: Cache, | |||
// Cache for the HTTP method when getting auth, possible values: "get", "post". |
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 also update the comment.
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.
Ok!
Signed-off-by: lihuahua123 <771725652@qq.com>
47ef123
to
9276471
Compare
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!
Relevant Issue (if applicable)
Details
fix auth compatibility for registry backend:
Due to the different implementations of various image registries, auth requests may use the GET or POST methods.
In the previous way, it just use 'GET' method, and in the pr, we fix it advanced, it can use and cache the GET or POST methods.
Types of changes
What types of changes does your PullRequest introduce? Put an
x
in all the boxes that apply:Checklist
Go over all the following points, and put an
x
in all the boxes that apply.