From 828d8b6adcf2189ec90b1916cf1c19f50d14411d Mon Sep 17 00:00:00 2001 From: Yan Song Date: Wed, 18 Oct 2023 07:03:57 +0000 Subject: [PATCH 1/2] storage: fix compatibility on fetching token for registry backend The registry backend received an unauthorized error from Harbor registry when fetching registry token by HTTP GET method, the bug is introduced from https://github.com/dragonflyoss/image-service/pull/1425/files#diff-f7ce8f265a570c66eae48c85e0f5b6f29fdaec9cf2ee2eded95810fe320d80e1L263. We should insert the basic auth header to ensure the compatibility of fetching token by HTTP GET method. This refers to containerd implementation: https://github.com/containerd/containerd/blob/dc7dba9c20f7210c38e8255487fc0ee12692149d/remotes/docker/auth/fetch.go#L187 The change has been tested for Harbor v2.9. Signed-off-by: Yan Song --- storage/src/backend/registry.rs | 50 +++++++++++++++++++++------------ 1 file changed, 32 insertions(+), 18 deletions(-) diff --git a/storage/src/backend/registry.rs b/storage/src/backend/registry.rs index a07090c9f42..295e7ab1f95 100644 --- a/storage/src/backend/registry.rs +++ b/storage/src/backend/registry.rs @@ -144,7 +144,6 @@ struct BearerAuth { realm: String, service: String, scope: String, - header: Option, } #[derive(Debug)] @@ -257,8 +256,8 @@ impl RegistryState { } else { match self.get_token_with_post(&auth, connection) { Ok(resp) => resp, - Err(err) => { - warn!("retry http GET method to get auth token: {}", err); + Err(_) => { + warn!("retry http GET method to get auth token"); let resp = self.get_token_with_get(&auth, connection)?; // Cache http method for next use. self.cached_auth_using_http_get.set(self.host.clone(), true); @@ -303,18 +302,22 @@ impl RegistryState { form.insert("passward".to_string(), self.password.clone()); form.insert("client_id".to_string(), REGISTRY_CLIENT_ID.to_string()); - let mut headers = HeaderMap::new(); - let token_resp = connection .call::<&[u8]>( Method::POST, auth.realm.as_str(), None, Some(ReqBody::Form(form)), - &mut headers, + &mut HeaderMap::new(), true, ) - .map_err(|e| einval!(format!("registry auth server request failed {:?}", e)))?; + .map_err(|e| { + warn!( + "failed to request registry auth server by POST method: {:?}", + e + ); + einval!() + })?; Ok(token_resp) } @@ -336,6 +339,16 @@ impl RegistryState { let mut headers = HeaderMap::new(); + // Insert the basic auth header to ensure the compatibility (e.g. Harbor registry) + // of fetching token by HTTP GET method. + // This refers containerd implementation: https://github.com/containerd/containerd/blob/dc7dba9c20f7210c38e8255487fc0ee12692149d/remotes/docker/auth/fetch.go#L187 + if let Some(auth) = &self.auth { + headers.insert( + HEADER_AUTHORIZATION, + format!("Basic {}", auth).parse().unwrap(), + ); + } + let token_resp = connection .call::<&[u8]>( Method::GET, @@ -345,7 +358,13 @@ impl RegistryState { &mut headers, true, ) - .map_err(|e| einval!(format!("registry auth server request failed {:?}", e)))?; + .map_err(|e| { + warn!( + "failed to request registry auth server by GET method: {:?}", + e + ); + einval!() + })?; Ok(token_resp) } @@ -366,7 +385,7 @@ impl RegistryState { /// Parse `www-authenticate` response header respond from registry server /// The header format like: `Bearer realm="https://auth.my-registry.com/token",service="my-registry.com",scope="repository:test/repo:pull,push"` - fn parse_auth(source: &HeaderValue, auth: &Option) -> Option { + fn parse_auth(source: &HeaderValue) -> Option { let source = source.to_str().unwrap(); let source: Vec<&str> = source.splitn(2, ' ').collect(); if source.len() < 2 { @@ -403,15 +422,10 @@ impl RegistryState { return None; } - let header = auth - .as_ref() - .map(|auth| HeaderValue::from_str(&format!("Basic {}", auth)).unwrap()); - Some(Auth::Bearer(BearerAuth { realm: (*paras.get("realm").unwrap()).to_string(), service: (*paras.get("service").unwrap()).to_string(), scope: (*paras.get("scope").unwrap()).to_string(), - header, })) } _ => None, @@ -573,7 +587,7 @@ impl RegistryReader { if let Some(resp_auth_header) = resp.headers().get(HEADER_WWW_AUTHENTICATE) { // Get token from registry authorization server - if let Some(auth) = RegistryState::parse_auth(resp_auth_header, &self.state.auth) { + if let Some(auth) = RegistryState::parse_auth(resp_auth_header) { let auth_header = self .state .get_auth_header(auth, &self.connection) @@ -1068,7 +1082,7 @@ mod tests { fn test_parse_auth() { let str = "Bearer realm=\"https://auth.my-registry.com/token\",service=\"my-registry.com\",scope=\"repository:test/repo:pull,push\""; let header = HeaderValue::from_str(str).unwrap(); - let auth = RegistryState::parse_auth(&header, &None).unwrap(); + let auth = RegistryState::parse_auth(&header).unwrap(); match auth { Auth::Bearer(auth) => { assert_eq!(&auth.realm, "https://auth.my-registry.com/token"); @@ -1080,7 +1094,7 @@ mod tests { let str = "Basic realm=\"https://auth.my-registry.com/token\""; let header = HeaderValue::from_str(str).unwrap(); - let auth = RegistryState::parse_auth(&header, &None).unwrap(); + let auth = RegistryState::parse_auth(&header).unwrap(); match auth { Auth::Basic(auth) => assert_eq!(&auth.realm, "https://auth.my-registry.com/token"), _ => panic!("failed to pase `Bearer` authentication header"), @@ -1088,7 +1102,7 @@ mod tests { let str = "Base realm=\"https://auth.my-registry.com/token\""; let header = HeaderValue::from_str(str).unwrap(); - assert!(RegistryState::parse_auth(&header, &None).is_none()); + assert!(RegistryState::parse_auth(&header).is_none()); } #[test] From 2d43640d7d5e37e34f6a4c83ba1e194c677924e3 Mon Sep 17 00:00:00 2001 From: Yan Song Date: Wed, 18 Oct 2023 07:10:26 +0000 Subject: [PATCH 2/2] api: fix the log message print in macro Regardless of whether debug compilation is enabled, we should always print error messages. Otherwise, some error logs may be lost, making it difficult to debug codes. Signed-off-by: Yan Song --- api/src/error.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/api/src/error.rs b/api/src/error.rs index e160f5e34c9..ad7c391ac02 100644 --- a/api/src/error.rs +++ b/api/src/error.rs @@ -11,16 +11,16 @@ pub fn make_error( _file: &str, _line: u32, ) -> std::io::Error { - #[cfg(all(debug_assertions, feature = "error-backtrace"))] + #[cfg(all(feature = "error-backtrace"))] { if let Ok(val) = std::env::var("RUST_BACKTRACE") { if val.trim() != "0" { - log::error!("Stack:\n{:?}", backtrace::Backtrace::new()); - log::error!("Error:\n\t{:?}\n\tat {}:{}", _raw, _file, _line); + error!("Stack:\n{:?}", backtrace::Backtrace::new()); + error!("Error:\n\t{:?}\n\tat {}:{}", _raw, _file, _line); return err; } } - log::error!( + error!( "Error:\n\t{:?}\n\tat {}:{}\n\tnote: enable `RUST_BACKTRACE=1` env to display a backtrace", _raw, _file, _line );