From 5278aaf587734f694e346455718942fda39d310f Mon Sep 17 00:00:00 2001 From: Jack Dockerty Date: Wed, 7 Aug 2024 16:50:26 +0100 Subject: [PATCH 01/12] feat: add token to gcsconfig --- core/src/services/gcs/backend.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/core/src/services/gcs/backend.rs b/core/src/services/gcs/backend.rs index 0ca6c015b0f..15c48d77fd7 100644 --- a/core/src/services/gcs/backend.rs +++ b/core/src/services/gcs/backend.rs @@ -74,6 +74,8 @@ pub struct GcsConfig { pub disable_vm_metadata: bool, /// Disable loading configuration from the environment. pub disable_config_load: bool, + /// A Google Cloud OAuth2 token. + pub token: String, } impl Debug for GcsConfig { @@ -214,6 +216,12 @@ impl GcsBuilder { self } + /// Provide the OAuth2 token to use. + pub fn token(mut self, token: String) -> Self { + self.config.token = token; + self + } + /// Disable attempting to load credentials from the GCE metadata server. pub fn disable_vm_metadata(mut self) -> Self { self.config.disable_vm_metadata = true; From 9e1badf984913c4169f4c37f996493df16028ba6 Mon Sep 17 00:00:00 2001 From: Jack Dockerty Date: Wed, 7 Aug 2024 16:50:42 +0100 Subject: [PATCH 02/12] feat: add bearer token to header requests --- core/src/services/gcs/core.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/core/src/services/gcs/core.rs b/core/src/services/gcs/core.rs index 0b9e8ae84c9..28a603a62aa 100644 --- a/core/src/services/gcs/core.rs +++ b/core/src/services/gcs/core.rs @@ -24,6 +24,7 @@ use std::time::Duration; use backon::ExponentialBuilder; use backon::Retryable; use bytes::Bytes; +use http::header; use http::header::CONTENT_LENGTH; use http::header::CONTENT_TYPE; use http::header::HOST; @@ -53,6 +54,7 @@ pub struct GcsCore { pub client: HttpClient, pub signer: GoogleSigner, pub token_loader: GoogleTokenLoader, + pub token: String, pub credential_loader: GoogleCredentialLoader, pub predefined_acl: Option, @@ -117,6 +119,12 @@ impl GcsCore { } let cred = self.load_token().await?; + if !self.token.is_empty() { + let header_value = format!("Bearer {}", self.token); + req.headers_mut() + .insert(header::AUTHORIZATION, header_value.parse().unwrap()); + } + self.signer .sign(req, &cred) .map_err(new_request_sign_error)?; @@ -141,6 +149,12 @@ impl GcsCore { return Ok(()); } + if !self.token.is_empty() { + let header_value = format!("Bearer {}", self.token); + req.headers_mut() + .insert(header::AUTHORIZATION, header_value.parse().unwrap()); + } + // Always remove host header, let users' client to set it based on HTTP // version. // From b236f5b701cb3e04b8391ebf035bcbd364392c5d Mon Sep 17 00:00:00 2001 From: Jack Dockerty Date: Wed, 7 Aug 2024 16:58:07 +0100 Subject: [PATCH 03/12] feat: add token to builder --- core/src/services/gcs/backend.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/core/src/services/gcs/backend.rs b/core/src/services/gcs/backend.rs index 15c48d77fd7..f243eac529e 100644 --- a/core/src/services/gcs/backend.rs +++ b/core/src/services/gcs/backend.rs @@ -362,6 +362,7 @@ impl Builder for GcsBuilder { client, signer, token_loader, + token: self.config.token, credential_loader: cred_loader, predefined_acl: self.config.predefined_acl.clone(), default_storage_class: self.config.default_storage_class.clone(), From 3adbe475145d1b509f6eff2e2c4f2d98a4425053 Mon Sep 17 00:00:00 2001 From: Jack Dockerty Date: Fri, 9 Aug 2024 20:57:57 +0100 Subject: [PATCH 04/12] feat: make token optional --- core/src/services/gcs/backend.rs | 6 ++++-- core/src/services/gcs/core.rs | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/core/src/services/gcs/backend.rs b/core/src/services/gcs/backend.rs index f243eac529e..a79997f24b2 100644 --- a/core/src/services/gcs/backend.rs +++ b/core/src/services/gcs/backend.rs @@ -75,7 +75,9 @@ pub struct GcsConfig { /// Disable loading configuration from the environment. pub disable_config_load: bool, /// A Google Cloud OAuth2 token. - pub token: String, + /// + /// Takes precedence over `credential` and `credential_path`. + pub token: Option, } impl Debug for GcsConfig { @@ -218,7 +220,7 @@ impl GcsBuilder { /// Provide the OAuth2 token to use. pub fn token(mut self, token: String) -> Self { - self.config.token = token; + self.config.token = Some(token); self } diff --git a/core/src/services/gcs/core.rs b/core/src/services/gcs/core.rs index 28a603a62aa..bcb3eed7d17 100644 --- a/core/src/services/gcs/core.rs +++ b/core/src/services/gcs/core.rs @@ -54,7 +54,7 @@ pub struct GcsCore { pub client: HttpClient, pub signer: GoogleSigner, pub token_loader: GoogleTokenLoader, - pub token: String, + pub token: Option, pub credential_loader: GoogleCredentialLoader, pub predefined_acl: Option, From 91887b83dec30caf0b462d6616e813fbd6434d68 Mon Sep 17 00:00:00 2001 From: Jack Dockerty Date: Fri, 9 Aug 2024 20:58:55 +0100 Subject: [PATCH 05/12] feat: place token within header --- core/src/services/gcs/core.rs | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/core/src/services/gcs/core.rs b/core/src/services/gcs/core.rs index bcb3eed7d17..ac11f8f2837 100644 --- a/core/src/services/gcs/core.rs +++ b/core/src/services/gcs/core.rs @@ -119,8 +119,8 @@ impl GcsCore { } let cred = self.load_token().await?; - if !self.token.is_empty() { - let header_value = format!("Bearer {}", self.token); + if let Some(token) = &self.token { + let header_value = format!("Bearer {}", token); req.headers_mut() .insert(header::AUTHORIZATION, header_value.parse().unwrap()); } @@ -141,6 +141,15 @@ impl GcsCore { } pub async fn sign_query(&self, req: &mut Request, duration: Duration) -> Result<()> { + if let Some(token) = &self.token { + req.headers_mut().remove(HOST); + + let header_value = format!("Bearer {}", token); + req.headers_mut() + .insert(header::AUTHORIZATION, header_value.parse().unwrap()); + return Ok(()); + } + if let Some(cred) = self.load_credential()? { self.signer .sign_query(req, duration, &cred) @@ -149,12 +158,6 @@ impl GcsCore { return Ok(()); } - if !self.token.is_empty() { - let header_value = format!("Bearer {}", self.token); - req.headers_mut() - .insert(header::AUTHORIZATION, header_value.parse().unwrap()); - } - // Always remove host header, let users' client to set it based on HTTP // version. // From afb7ef0da8e729d44d441fd3cff277ee21a7282b Mon Sep 17 00:00:00 2001 From: Jack Dockerty Date: Sun, 11 Aug 2024 14:49:49 +0100 Subject: [PATCH 06/12] feat: pass scope to gcs core --- core/src/services/gcs/backend.rs | 1 + core/src/services/gcs/core.rs | 1 + 2 files changed, 2 insertions(+) diff --git a/core/src/services/gcs/backend.rs b/core/src/services/gcs/backend.rs index a79997f24b2..0861e0bbd78 100644 --- a/core/src/services/gcs/backend.rs +++ b/core/src/services/gcs/backend.rs @@ -365,6 +365,7 @@ impl Builder for GcsBuilder { signer, token_loader, token: self.config.token, + scope: self.config.scope, credential_loader: cred_loader, predefined_acl: self.config.predefined_acl.clone(), default_storage_class: self.config.default_storage_class.clone(), diff --git a/core/src/services/gcs/core.rs b/core/src/services/gcs/core.rs index 0b232148e46..bd201423617 100644 --- a/core/src/services/gcs/core.rs +++ b/core/src/services/gcs/core.rs @@ -55,6 +55,7 @@ pub struct GcsCore { pub signer: GoogleSigner, pub token_loader: GoogleTokenLoader, pub token: Option, + pub scope: Option, pub credential_loader: GoogleCredentialLoader, pub predefined_acl: Option, From b36ca3ba5cd9a93ae965a75c7adef46eaddd082a Mon Sep 17 00:00:00 2001 From: Jack Dockerty Date: Sun, 11 Aug 2024 14:50:18 +0100 Subject: [PATCH 07/12] feat: return token directly from load_token if set --- core/src/services/gcs/core.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/core/src/services/gcs/core.rs b/core/src/services/gcs/core.rs index bd201423617..26602753c93 100644 --- a/core/src/services/gcs/core.rs +++ b/core/src/services/gcs/core.rs @@ -79,6 +79,17 @@ static BACKOFF: Lazy = impl GcsCore { async fn load_token(&self) -> Result> { + if let Some(token) = &self.token { + if let Some(scope) = &self.scope { + return Ok(Some(GoogleToken::new(token, usize::MAX, scope))); + } else { + return Err(Error::new( + ErrorKind::ConfigInvalid, + "Scope is required when token is set", + )); + } + } + let cred = { || self.token_loader.load() } .retry(&*BACKOFF) .await From 55fc7629a10fb4a17dcd7a78e2d3299efa8182b1 Mon Sep 17 00:00:00 2001 From: Jack Dockerty Date: Sun, 11 Aug 2024 14:54:01 +0100 Subject: [PATCH 08/12] refactor: use tuple match for token/scope handling --- core/src/services/gcs/core.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/core/src/services/gcs/core.rs b/core/src/services/gcs/core.rs index 26602753c93..6419fbfafac 100644 --- a/core/src/services/gcs/core.rs +++ b/core/src/services/gcs/core.rs @@ -79,17 +79,18 @@ static BACKOFF: Lazy = impl GcsCore { async fn load_token(&self) -> Result> { - if let Some(token) = &self.token { - if let Some(scope) = &self.scope { - return Ok(Some(GoogleToken::new(token, usize::MAX, scope))); - } else { + match (&self.token, &self.scope) { + (Some(token), Some(scope)) => { + return Ok(Some(GoogleToken::new(token, usize::MAX, scope))) + } + (Some(_), None) => { return Err(Error::new( ErrorKind::ConfigInvalid, "Scope is required when token is set", - )); + )) } + _ => {} } - let cred = { || self.token_loader.load() } .retry(&*BACKOFF) .await From 32d1b1d9917b6005467703d36be5b9a46d3909ab Mon Sep 17 00:00:00 2001 From: Jack Dockerty Date: Tue, 13 Aug 2024 11:12:09 +0100 Subject: [PATCH 09/12] fix: use default scope set for gcscore --- core/src/services/gcs/backend.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/services/gcs/backend.rs b/core/src/services/gcs/backend.rs index 0861e0bbd78..f9280bffdd7 100644 --- a/core/src/services/gcs/backend.rs +++ b/core/src/services/gcs/backend.rs @@ -365,7 +365,7 @@ impl Builder for GcsBuilder { signer, token_loader, token: self.config.token, - scope: self.config.scope, + scope: Some(scope.to_string()), credential_loader: cred_loader, predefined_acl: self.config.predefined_acl.clone(), default_storage_class: self.config.default_storage_class.clone(), From 3346244e274bdb09445db8a19f98a0b4bb3206c9 Mon Sep 17 00:00:00 2001 From: Jack Dockerty Date: Tue, 13 Aug 2024 11:14:41 +0100 Subject: [PATCH 10/12] fix: scope must be valid --- core/src/services/gcs/backend.rs | 2 +- core/src/services/gcs/core.rs | 16 ++++------------ 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/core/src/services/gcs/backend.rs b/core/src/services/gcs/backend.rs index f9280bffdd7..8861b22202f 100644 --- a/core/src/services/gcs/backend.rs +++ b/core/src/services/gcs/backend.rs @@ -365,7 +365,7 @@ impl Builder for GcsBuilder { signer, token_loader, token: self.config.token, - scope: Some(scope.to_string()), + scope: scope.to_string(), credential_loader: cred_loader, predefined_acl: self.config.predefined_acl.clone(), default_storage_class: self.config.default_storage_class.clone(), diff --git a/core/src/services/gcs/core.rs b/core/src/services/gcs/core.rs index 6419fbfafac..144d7520afe 100644 --- a/core/src/services/gcs/core.rs +++ b/core/src/services/gcs/core.rs @@ -55,7 +55,7 @@ pub struct GcsCore { pub signer: GoogleSigner, pub token_loader: GoogleTokenLoader, pub token: Option, - pub scope: Option, + pub scope: String, pub credential_loader: GoogleCredentialLoader, pub predefined_acl: Option, @@ -79,18 +79,10 @@ static BACKOFF: Lazy = impl GcsCore { async fn load_token(&self) -> Result> { - match (&self.token, &self.scope) { - (Some(token), Some(scope)) => { - return Ok(Some(GoogleToken::new(token, usize::MAX, scope))) - } - (Some(_), None) => { - return Err(Error::new( - ErrorKind::ConfigInvalid, - "Scope is required when token is set", - )) - } - _ => {} + if let Some(token) = &self.token { + return Ok(Some(GoogleToken::new(token, usize::MAX, &self.scope))); } + let cred = { || self.token_loader.load() } .retry(&*BACKOFF) .await From 2a98679ed87e2060c006209dddba3ef5b53c2779 Mon Sep 17 00:00:00 2001 From: Jack Dockerty Date: Tue, 13 Aug 2024 11:20:49 +0100 Subject: [PATCH 11/12] revert: do not change sign logic --- core/src/services/gcs/core.rs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/core/src/services/gcs/core.rs b/core/src/services/gcs/core.rs index 144d7520afe..6f91ce2f3c2 100644 --- a/core/src/services/gcs/core.rs +++ b/core/src/services/gcs/core.rs @@ -123,15 +123,6 @@ impl GcsCore { } pub async fn sign(&self, req: &mut Request) -> Result<()> { - if let Some(token) = &self.token { - req.headers_mut().remove(HOST); - - let header_value = format!("Bearer {}", token); - req.headers_mut() - .insert(header::AUTHORIZATION, header_value.parse().unwrap()); - return Ok(()); - } - if let Some(cred) = self.load_token().await? { self.signer .sign(req, &cred) From f1a78d7063caa7a3fcea824e604bc6d3f361b831 Mon Sep 17 00:00:00 2001 From: Jack Dockerty Date: Tue, 13 Aug 2024 11:23:11 +0100 Subject: [PATCH 12/12] revert: do not change sign_query logic --- core/src/services/gcs/core.rs | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/core/src/services/gcs/core.rs b/core/src/services/gcs/core.rs index 6f91ce2f3c2..ba11a577159 100644 --- a/core/src/services/gcs/core.rs +++ b/core/src/services/gcs/core.rs @@ -24,7 +24,6 @@ use std::time::Duration; use backon::ExponentialBuilder; use backon::Retryable; use bytes::Bytes; -use http::header; use http::header::CONTENT_LENGTH; use http::header::CONTENT_TYPE; use http::header::HOST; @@ -143,15 +142,6 @@ impl GcsCore { } pub async fn sign_query(&self, req: &mut Request, duration: Duration) -> Result<()> { - if let Some(token) = &self.token { - req.headers_mut().remove(HOST); - - let header_value = format!("Bearer {}", token); - req.headers_mut() - .insert(header::AUTHORIZATION, header_value.parse().unwrap()); - return Ok(()); - } - if let Some(cred) = self.load_credential()? { self.signer .sign_query(req, duration, &cred)