You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2023/01/17 02:25:23 UTC
[GitHub] [arrow-rs] winding-lines opened a new pull request, #3541: Additional GCP authentication
winding-lines opened a new pull request, #3541:
URL: https://github.com/apache/arrow-rs/pull/3541
# Which issue does this PR close?
<!--
We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes #123` indicates that this PR will close issue #123.
-->
Closes #3533.
# Rationale for this change
<!--
Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed.
Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes.
-->
# What changes are included in this PR?
<!--
There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR.
-->
This is WIP. I may be missing something, however workload identity federation is big. https://google.aip.dev/auth/4117. Definitely more than one of my weekends :) Is this what you had in mind @tustvold ?
I am happy to keep on pushing on this but I am wondering if we could maybe land the Application Default Credentials in the mean time because that would unblock some end-user scenarios.
# Are there any user-facing changes?
<!--
If there are user-facing changes then we may require documentation to be updated before approving the PR.
-->
<!---
If there are any breaking changes to public APIs, please add the `breaking change` label.
-->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] winding-lines commented on a diff in pull request #3541: Additional GCP authentication
Posted by "winding-lines (via GitHub)" <gi...@apache.org>.
winding-lines commented on code in PR #3541:
URL: https://github.com/apache/arrow-rs/pull/3541#discussion_r1086761410
##########
object_store/src/gcp/credential.rs:
##########
@@ -329,39 +319,69 @@ fn b64_encode_obj<T: serde::Serialize>(obj: &T) -> Result<String> {
}
/// A provider that uses the Google Cloud Platform metadata server to fetch a token.
+///
+/// <https://cloud.google.com/docs/authentication/get-id-token#metadata-server>
#[derive(Debug, Default)]
pub struct InstanceCredentialProvider {
audience: String,
+ client_options: ClientOptions,
Review Comment:
Done :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] winding-lines commented on pull request #3541: Additional GCP authentication
Posted by "winding-lines (via GitHub)" <gi...@apache.org>.
winding-lines commented on PR #3541:
URL: https://github.com/apache/arrow-rs/pull/3541#issuecomment-1403063365
I think I addressed the current round of feedback, let me know what you think :)
Also tested with Application Default Credentials and Google cloud.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] tustvold commented on a diff in pull request #3541: Additional GCP authentication
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on code in PR #3541:
URL: https://github.com/apache/arrow-rs/pull/3541#discussion_r1083955697
##########
object_store/src/gcp/mod.rs:
##########
@@ -185,32 +181,6 @@ impl From<Error> for super::Error {
}
}
-/// A deserialized `service-account-********.json`-file.
Review Comment:
This is moved into credentials
##########
object_store/src/gcp/credential.rs:
##########
@@ -104,6 +115,15 @@ struct TokenResponse {
expires_in: u64,
}
+#[async_trait]
+pub(crate) trait TokenProvider: std::fmt::Debug + Send + Sync {
Review Comment:
```suggestion
pub trait TokenProvider: std::fmt::Debug + Send + Sync {
```
FWIW this module as a whole is crate private so this could just be pub
##########
object_store/src/gcp/credential.rs:
##########
@@ -222,3 +327,118 @@ fn b64_encode_obj<T: serde::Serialize>(obj: &T) -> Result<String> {
let string = serde_json::to_string(obj).context(EncodeSnafu)?;
Ok(BASE64_URL_SAFE_NO_PAD.encode(string))
}
+
+/// A provider that uses the Google Cloud Platform metadata server to fetch a token.
+#[derive(Debug, Default)]
Review Comment:
```suggestion
/// <https://cloud.google.com/docs/authentication/get-id-token#metadata-server>
#[derive(Debug, Default)]
```
##########
object_store/src/gcp/credential.rs:
##########
@@ -138,9 +158,12 @@ impl OAuthProvider {
random: ring::rand::SystemRandom::new(),
})
}
+}
+#[async_trait]
+impl TokenProvider for OAuthProvider {
Review Comment:
:+1: to using a trait here, whilst it does mean the future is boxed, we're doing network IO here so it doesn't matter :smile:
##########
object_store/src/gcp/credential.rs:
##########
@@ -195,6 +218,88 @@ impl OAuthProvider {
}
}
+fn reader_credentials_file<T>(
Review Comment:
```suggestion
fn read_credentials_file<T>(
```
Appreciate this was copied, but could take the opportunity to fix the name
##########
object_store/src/gcp/credential.rs:
##########
@@ -195,6 +218,88 @@ impl OAuthProvider {
}
}
+fn reader_credentials_file<T>(
+ service_account_path: impl AsRef<std::path::Path>,
+) -> Result<T>
+where
+ T: serde::de::DeserializeOwned,
+{
+ let file = File::open(service_account_path).context(OpenCredentialsSnafu)?;
+ let reader = BufReader::new(file);
+ serde_json::from_reader(reader).context(DecodeCredentialsSnafu)
+}
+
+/// A deserialized `service-account-********.json`-file.
+#[derive(serde::Deserialize, Debug)]
+pub(crate) struct ServiceAccountCredentials {
+ /// The private key in RSA format.
+ pub private_key: String,
+
+ /// The email address associated with the service account.
+ pub client_email: String,
+
+ /// Base URL for GCS
+ #[serde(default = "default_gcs_base_url")]
+ pub gcs_base_url: String,
+
+ /// Disable oauth and use empty tokens.
+ #[serde(default = "default_disable_oauth")]
+ pub disable_oauth: bool,
+}
+
+pub(crate) fn default_gcs_base_url() -> String {
Review Comment:
I appreciate these were just copied but they could be constants instead of functions FWIW
##########
object_store/src/gcp/credential.rs:
##########
@@ -222,3 +327,118 @@ fn b64_encode_obj<T: serde::Serialize>(obj: &T) -> Result<String> {
let string = serde_json::to_string(obj).context(EncodeSnafu)?;
Ok(BASE64_URL_SAFE_NO_PAD.encode(string))
}
+
+/// A provider that uses the Google Cloud Platform metadata server to fetch a token.
+#[derive(Debug, Default)]
+pub struct InstanceCredentialProvider {
+ audience: String,
+}
+
+impl InstanceCredentialProvider {
+ pub fn new<T: Into<String>>(audience: T) -> Self {
+ Self {
+ audience: audience.into(),
+ }
+ }
+}
+
+#[async_trait]
+impl TokenProvider for InstanceCredentialProvider {
+ async fn fetch_token(
+ &self,
+ client: &Client,
+ retry: &RetryConfig,
+ ) -> Result<TemporaryToken<String>> {
+ println!("fetching token from metadata server");
+ const TOKEN_URL: &str =
+ "http://metadata/computeMetadata/v1/instance/service-accounts/default/token";
+ let response: TokenResponse = client
+ .request(Method::GET, TOKEN_URL)
+ .header("Metadata-Flavor", "Google")
+ .query(&[("audience", &self.audience)])
+ .send_retry(retry)
+ .await
+ .context(TokenRequestSnafu)?
+ .json()
+ .await
+ .context(TokenResponseBodySnafu)?;
+ let token = TemporaryToken {
+ token: response.access_token,
+ expiry: Instant::now() + Duration::from_secs(response.expires_in),
+ };
+ Ok(token)
+ }
+}
+
+/// A deserialized `application_default_credentials.json`-file.
+#[derive(serde::Deserialize, Debug)]
+pub struct ApplicationDefaultCredentials {
+ client_id: String,
+ client_secret: String,
+ refresh_token: String,
+ #[serde(rename = "type")]
+ type_: String,
+}
+
+impl ApplicationDefaultCredentials {
+ const DEFAULT_TOKEN_GCP_URI: &'static str =
+ "https://accounts.google.com/o/oauth2/token";
+ const CREDENTIALS_PATH: &'static str =
+ ".config/gcloud/application_default_credentials.json";
+ const EXPECTED_TYPE: &str = "authorized_user";
+
+ // Create a new application default credential in the following situations:
+ // 1. a file is passed in and the type matches.
+ // 2. without argument if the well-known configuration file is present.
+ pub fn new(path: Option<&str>) -> Result<Option<Self>, Error> {
+ if let Some(path) = path {
+ if let Ok(credentials) = reader_credentials_file::<Self>(path) {
+ if credentials.type_ == Self::EXPECTED_TYPE {
+ return Ok(Some(credentials));
+ }
+ }
+ // Other credential mechanisms may be able to use this path.
+ return Ok(None);
Review Comment:
I think if a path has been specified and it doesn't exist or is not the expected type we should return an error
##########
object_store/src/gcp/mod.rs:
##########
@@ -1098,44 +1083,71 @@ impl GoogleCloudStorageBuilder {
let client = self.client_options.client()?;
- let credentials = match (self.service_account_path, self.service_account_key) {
- (Some(path), None) => reader_credentials_file(path)?,
- (None, Some(key)) => {
- serde_json::from_str(&key).context(DecodeCredentialsSnafu)?
- }
- (None, None) => return Err(Error::MissingServiceAccountPathOrKey.into()),
- (Some(_), Some(_)) => {
- return Err(Error::ServiceAccountPathAndKeyProvided.into())
- }
- };
+ // First try to initialize from the service account information.
+ let service_account_credentials =
+ match (self.service_account_path, self.service_account_key) {
+ (Some(path), None) => Some(
+ ServiceAccountCredentials::from_file(path)
+ .context(CredentialSnafu)?,
+ ),
+ (None, Some(key)) => Some(
+ ServiceAccountCredentials::from_key(&key).context(CredentialSnafu)?,
+ ),
+ (None, None) => None,
+ (Some(_), Some(_)) => {
+ return Err(Error::ServiceAccountPathAndKeyProvided.into())
+ }
+ };
+
+ // Then try to initialize from the application credentials file, or the environment.
+ let application_default_credentials = ApplicationDefaultCredentials::new(
+ self.application_credentials_path.as_deref(),
+ )
+ .context(CredentialSnafu)?;
+
+ let disable_oauth = service_account_credentials
+ .as_ref()
+ .map(|c| c.disable_oauth)
+ .unwrap_or(false);
+
+ let gcs_base_url = service_account_credentials
+ .as_ref()
+ .map(|c| c.gcs_base_url.clone())
+ .unwrap_or_else(default_gcs_base_url);
// TODO: https://cloud.google.com/storage/docs/authentication#oauth-scopes
let scope = "https://www.googleapis.com/auth/devstorage.full_control";
- let audience = "https://www.googleapis.com/oauth2/v4/token".to_string();
-
- let oauth_provider = (!credentials.disable_oauth)
- .then(|| {
- OAuthProvider::new(
- credentials.client_email,
- credentials.private_key,
- scope.to_string(),
- audience,
- )
- })
- .transpose()
- .context(CredentialSnafu)?;
+ let audience = "https://www.googleapis.com/oauth2/v4/token";
+
+ let token_provider = if disable_oauth {
+ None
+ } else {
+ let best_provider = service_account_credentials
+ .map(|credentials| credentials.token_provider(scope, audience))
+ .transpose()
+ .context(CredentialSnafu)?
+ .or_else(|| {
+ application_default_credentials
+ .map(|a| Box::new(a) as Box<dyn TokenProvider>)
+ })
+ .or_else(|| Some(Box::new(InstanceCredentialProvider::new(audience))));
+
+ // A provider is required at this point, bail out if we don't have one.
+ if best_provider.is_some() {
+ best_provider
+ } else {
+ return Err(Error::MissingCredentials.into());
+ }
Review Comment:
```suggestion
best_provider.ok_or(Error::MissingCredentials)?
```
##########
object_store/src/gcp/credential.rs:
##########
@@ -222,3 +327,118 @@ fn b64_encode_obj<T: serde::Serialize>(obj: &T) -> Result<String> {
let string = serde_json::to_string(obj).context(EncodeSnafu)?;
Ok(BASE64_URL_SAFE_NO_PAD.encode(string))
}
+
+/// A provider that uses the Google Cloud Platform metadata server to fetch a token.
+#[derive(Debug, Default)]
+pub struct InstanceCredentialProvider {
+ audience: String,
+}
+
+impl InstanceCredentialProvider {
+ pub fn new<T: Into<String>>(audience: T) -> Self {
+ Self {
+ audience: audience.into(),
+ }
+ }
+}
+
+#[async_trait]
+impl TokenProvider for InstanceCredentialProvider {
+ async fn fetch_token(
+ &self,
+ client: &Client,
+ retry: &RetryConfig,
+ ) -> Result<TemporaryToken<String>> {
+ println!("fetching token from metadata server");
+ const TOKEN_URL: &str =
+ "http://metadata/computeMetadata/v1/instance/service-accounts/default/token";
+ let response: TokenResponse = client
+ .request(Method::GET, TOKEN_URL)
+ .header("Metadata-Flavor", "Google")
+ .query(&[("audience", &self.audience)])
+ .send_retry(retry)
+ .await
+ .context(TokenRequestSnafu)?
+ .json()
+ .await
+ .context(TokenResponseBodySnafu)?;
+ let token = TemporaryToken {
+ token: response.access_token,
+ expiry: Instant::now() + Duration::from_secs(response.expires_in),
+ };
+ Ok(token)
+ }
+}
+
+/// A deserialized `application_default_credentials.json`-file.
+#[derive(serde::Deserialize, Debug)]
+pub struct ApplicationDefaultCredentials {
+ client_id: String,
+ client_secret: String,
+ refresh_token: String,
+ #[serde(rename = "type")]
+ type_: String,
+}
+
+impl ApplicationDefaultCredentials {
+ const DEFAULT_TOKEN_GCP_URI: &'static str =
+ "https://accounts.google.com/o/oauth2/token";
+ const CREDENTIALS_PATH: &'static str =
+ ".config/gcloud/application_default_credentials.json";
+ const EXPECTED_TYPE: &str = "authorized_user";
+
+ // Create a new application default credential in the following situations:
+ // 1. a file is passed in and the type matches.
+ // 2. without argument if the well-known configuration file is present.
+ pub fn new(path: Option<&str>) -> Result<Option<Self>, Error> {
+ if let Some(path) = path {
+ if let Ok(credentials) = reader_credentials_file::<Self>(path) {
+ if credentials.type_ == Self::EXPECTED_TYPE {
+ return Ok(Some(credentials));
+ }
+ }
+ // Other credential mechanisms may be able to use this path.
+ return Ok(None);
+ }
+ if let Some(home) = env::var_os("HOME") {
Review Comment:
This is potentially fragile, but adding an additional dependency is probably not worth it so LGTM
##########
object_store/src/gcp/credential.rs:
##########
@@ -222,3 +327,118 @@ fn b64_encode_obj<T: serde::Serialize>(obj: &T) -> Result<String> {
let string = serde_json::to_string(obj).context(EncodeSnafu)?;
Ok(BASE64_URL_SAFE_NO_PAD.encode(string))
}
+
+/// A provider that uses the Google Cloud Platform metadata server to fetch a token.
+#[derive(Debug, Default)]
+pub struct InstanceCredentialProvider {
+ audience: String,
+}
+
+impl InstanceCredentialProvider {
+ pub fn new<T: Into<String>>(audience: T) -> Self {
+ Self {
+ audience: audience.into(),
+ }
+ }
+}
+
+#[async_trait]
+impl TokenProvider for InstanceCredentialProvider {
+ async fn fetch_token(
+ &self,
+ client: &Client,
+ retry: &RetryConfig,
+ ) -> Result<TemporaryToken<String>> {
+ println!("fetching token from metadata server");
+ const TOKEN_URL: &str =
+ "http://metadata/computeMetadata/v1/instance/service-accounts/default/token";
+ let response: TokenResponse = client
+ .request(Method::GET, TOKEN_URL)
+ .header("Metadata-Flavor", "Google")
+ .query(&[("audience", &self.audience)])
+ .send_retry(retry)
+ .await
+ .context(TokenRequestSnafu)?
+ .json()
+ .await
+ .context(TokenResponseBodySnafu)?;
+ let token = TemporaryToken {
+ token: response.access_token,
+ expiry: Instant::now() + Duration::from_secs(response.expires_in),
+ };
+ Ok(token)
+ }
+}
+
+/// A deserialized `application_default_credentials.json`-file.
+#[derive(serde::Deserialize, Debug)]
Review Comment:
```suggestion
/// <https://cloud.google.com/docs/authentication/application-default-credentials#personal>
#[derive(serde::Deserialize, Debug)]
```
##########
object_store/src/gcp/credential.rs:
##########
@@ -222,3 +327,118 @@ fn b64_encode_obj<T: serde::Serialize>(obj: &T) -> Result<String> {
let string = serde_json::to_string(obj).context(EncodeSnafu)?;
Ok(BASE64_URL_SAFE_NO_PAD.encode(string))
}
+
+/// A provider that uses the Google Cloud Platform metadata server to fetch a token.
+#[derive(Debug, Default)]
+pub struct InstanceCredentialProvider {
+ audience: String,
+}
+
+impl InstanceCredentialProvider {
+ pub fn new<T: Into<String>>(audience: T) -> Self {
+ Self {
+ audience: audience.into(),
+ }
+ }
+}
+
+#[async_trait]
+impl TokenProvider for InstanceCredentialProvider {
+ async fn fetch_token(
+ &self,
+ client: &Client,
+ retry: &RetryConfig,
+ ) -> Result<TemporaryToken<String>> {
+ println!("fetching token from metadata server");
Review Comment:
```suggestion
info!("fetching token from metadata server");
```
##########
object_store/src/gcp/credential.rs:
##########
@@ -222,3 +327,118 @@ fn b64_encode_obj<T: serde::Serialize>(obj: &T) -> Result<String> {
let string = serde_json::to_string(obj).context(EncodeSnafu)?;
Ok(BASE64_URL_SAFE_NO_PAD.encode(string))
}
+
+/// A provider that uses the Google Cloud Platform metadata server to fetch a token.
+#[derive(Debug, Default)]
+pub struct InstanceCredentialProvider {
+ audience: String,
+}
+
+impl InstanceCredentialProvider {
+ pub fn new<T: Into<String>>(audience: T) -> Self {
+ Self {
+ audience: audience.into(),
+ }
+ }
+}
+
+#[async_trait]
+impl TokenProvider for InstanceCredentialProvider {
+ async fn fetch_token(
+ &self,
+ client: &Client,
+ retry: &RetryConfig,
+ ) -> Result<TemporaryToken<String>> {
+ println!("fetching token from metadata server");
+ const TOKEN_URL: &str =
+ "http://metadata/computeMetadata/v1/instance/service-accounts/default/token";
+ let response: TokenResponse = client
Review Comment:
```suggestion
// TODO: Add IP fallback
// See https://github.com/googleapis/google-cloud-go/blob/main/compute/metadata/metadata.go#L123
let response: TokenResponse = client
```
Not something we necessarily need in this PR
##########
object_store/src/gcp/credential.rs:
##########
@@ -195,6 +218,88 @@ impl OAuthProvider {
}
}
+fn reader_credentials_file<T>(
+ service_account_path: impl AsRef<std::path::Path>,
+) -> Result<T>
+where
+ T: serde::de::DeserializeOwned,
+{
+ let file = File::open(service_account_path).context(OpenCredentialsSnafu)?;
+ let reader = BufReader::new(file);
+ serde_json::from_reader(reader).context(DecodeCredentialsSnafu)
+}
+
+/// A deserialized `service-account-********.json`-file.
+#[derive(serde::Deserialize, Debug)]
+pub(crate) struct ServiceAccountCredentials {
+ /// The private key in RSA format.
+ pub private_key: String,
+
+ /// The email address associated with the service account.
+ pub client_email: String,
+
+ /// Base URL for GCS
+ #[serde(default = "default_gcs_base_url")]
+ pub gcs_base_url: String,
+
+ /// Disable oauth and use empty tokens.
+ #[serde(default = "default_disable_oauth")]
+ pub disable_oauth: bool,
+}
+
+pub(crate) fn default_gcs_base_url() -> String {
+ "https://storage.googleapis.com".to_owned()
+}
+
+pub(crate) fn default_disable_oauth() -> bool {
+ false
+}
+
+impl ServiceAccountCredentials {
+ /// Create a new [`ServiceAccountCredentials`] from a file.
+ pub fn from_file<P: AsRef<std::path::Path>>(path: P) -> Result<Self> {
+ reader_credentials_file(path)
+ }
+
+ /// Create a new [`ServiceAccountCredentials`] from a string.
+ pub fn from_key(key: &str) -> Result<Self> {
+ serde_json::from_str(key).context(DecodeCredentialsSnafu)
+ }
+
+ /// Create an [`OAuthProvider`] from this credentials struct.
+ pub fn token_provider(
+ self,
+ scope: &str,
+ audience: &str,
+ ) -> Result<Box<dyn TokenProvider>> {
+ Ok(Box::new(OAuthProvider::new(
+ self.client_email,
+ self.private_key,
+ scope.to_string(),
+ audience.to_string(),
+ )?) as Box<dyn TokenProvider>)
+ }
+}
+
+/// A no-op provider that returns empty tokens
+#[derive(Debug)]
+pub struct NoOpProvider;
+
+#[async_trait]
+impl TokenProvider for NoOpProvider {
+ /// Fetch a fresh token
+ async fn fetch_token(
+ &self,
+ _client: &Client,
+ _retry: &RetryConfig,
+ ) -> Result<TemporaryToken<String>> {
+ Ok(TemporaryToken {
+ token: "".to_string(),
+ expiry: Instant::now(),
+ })
+ }
+}
+
Review Comment:
```suggestion
```
This doesn't appear to be used
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] winding-lines commented on a diff in pull request #3541: Additional GCP authentication
Posted by "winding-lines (via GitHub)" <gi...@apache.org>.
winding-lines commented on code in PR #3541:
URL: https://github.com/apache/arrow-rs/pull/3541#discussion_r1085292445
##########
object_store/src/gcp/credential.rs:
##########
@@ -138,9 +158,12 @@ impl OAuthProvider {
random: ring::rand::SystemRandom::new(),
})
}
+}
+#[async_trait]
+impl TokenProvider for OAuthProvider {
Review Comment:
Yeap, I copied the pattern from the `aws` implementation but used a simpler type.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] winding-lines commented on a diff in pull request #3541: Additional GCP authentication
Posted by "winding-lines (via GitHub)" <gi...@apache.org>.
winding-lines commented on code in PR #3541:
URL: https://github.com/apache/arrow-rs/pull/3541#discussion_r1085355009
##########
object_store/src/gcp/credential.rs:
##########
@@ -222,3 +327,118 @@ fn b64_encode_obj<T: serde::Serialize>(obj: &T) -> Result<String> {
let string = serde_json::to_string(obj).context(EncodeSnafu)?;
Ok(BASE64_URL_SAFE_NO_PAD.encode(string))
}
+
+/// A provider that uses the Google Cloud Platform metadata server to fetch a token.
+#[derive(Debug, Default)]
+pub struct InstanceCredentialProvider {
+ audience: String,
+}
+
+impl InstanceCredentialProvider {
+ pub fn new<T: Into<String>>(audience: T) -> Self {
+ Self {
+ audience: audience.into(),
+ }
+ }
+}
+
+#[async_trait]
+impl TokenProvider for InstanceCredentialProvider {
+ async fn fetch_token(
+ &self,
+ client: &Client,
+ retry: &RetryConfig,
+ ) -> Result<TemporaryToken<String>> {
+ println!("fetching token from metadata server");
+ const TOKEN_URL: &str =
+ "http://metadata/computeMetadata/v1/instance/service-accounts/default/token";
+ let response: TokenResponse = client
+ .request(Method::GET, TOKEN_URL)
+ .header("Metadata-Flavor", "Google")
+ .query(&[("audience", &self.audience)])
+ .send_retry(retry)
+ .await
+ .context(TokenRequestSnafu)?
+ .json()
+ .await
+ .context(TokenResponseBodySnafu)?;
+ let token = TemporaryToken {
+ token: response.access_token,
+ expiry: Instant::now() + Duration::from_secs(response.expires_in),
+ };
+ Ok(token)
+ }
+}
+
+/// A deserialized `application_default_credentials.json`-file.
+#[derive(serde::Deserialize, Debug)]
Review Comment:
Nice!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] tustvold commented on a diff in pull request #3541: Additional GCP authentication
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on code in PR #3541:
URL: https://github.com/apache/arrow-rs/pull/3541#discussion_r1086676477
##########
object_store/src/gcp/credential.rs:
##########
@@ -329,39 +319,69 @@ fn b64_encode_obj<T: serde::Serialize>(obj: &T) -> Result<String> {
}
/// A provider that uses the Google Cloud Platform metadata server to fetch a token.
+///
+/// <https://cloud.google.com/docs/authentication/get-id-token#metadata-server>
#[derive(Debug, Default)]
pub struct InstanceCredentialProvider {
audience: String,
+ client_options: ClientOptions,
Review Comment:
Perhaps we could store the `Client` on here instead of creating it every time?
##########
object_store/src/gcp/credential.rs:
##########
@@ -329,39 +319,69 @@ fn b64_encode_obj<T: serde::Serialize>(obj: &T) -> Result<String> {
}
/// A provider that uses the Google Cloud Platform metadata server to fetch a token.
+///
+/// <https://cloud.google.com/docs/authentication/get-id-token#metadata-server>
#[derive(Debug, Default)]
pub struct InstanceCredentialProvider {
audience: String,
+ client_options: ClientOptions,
}
impl InstanceCredentialProvider {
- pub fn new<T: Into<String>>(audience: T) -> Self {
+ /// Create a new [`InstanceCredentialProvider`], we need to control the client in order to enable http access so save the options.
+ pub fn new<T: Into<String>>(audience: T, client_options: ClientOptions) -> Self {
Self {
audience: audience.into(),
+ client_options: client_options.with_allow_http(true),
}
}
}
+/// Make a request to the metadata server to fetch a token, using a a given hostname.
+async fn make_metadata_request(
+ client: &Client,
+ hostname: &str,
+ retry: &RetryConfig,
+ audience: &str,
+) -> Result<TokenResponse> {
+ let url = format!(
+ "http://{}/computeMetadata/v1/instance/service-accounts/default/token",
+ hostname
+ );
+ println!("gcp/credentials getting token from url: {}", url);
Review Comment:
```suggestion
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] winding-lines commented on a diff in pull request #3541: Additional GCP authentication
Posted by "winding-lines (via GitHub)" <gi...@apache.org>.
winding-lines commented on code in PR #3541:
URL: https://github.com/apache/arrow-rs/pull/3541#discussion_r1085313973
##########
object_store/src/gcp/credential.rs:
##########
@@ -222,3 +327,118 @@ fn b64_encode_obj<T: serde::Serialize>(obj: &T) -> Result<String> {
let string = serde_json::to_string(obj).context(EncodeSnafu)?;
Ok(BASE64_URL_SAFE_NO_PAD.encode(string))
}
+
+/// A provider that uses the Google Cloud Platform metadata server to fetch a token.
+#[derive(Debug, Default)]
+pub struct InstanceCredentialProvider {
+ audience: String,
+}
+
+impl InstanceCredentialProvider {
+ pub fn new<T: Into<String>>(audience: T) -> Self {
+ Self {
+ audience: audience.into(),
+ }
+ }
+}
+
+#[async_trait]
+impl TokenProvider for InstanceCredentialProvider {
+ async fn fetch_token(
+ &self,
+ client: &Client,
Review Comment:
Interesting, I had not even noticed that we do this over `http` (as documented in the link above). Makes sense since it's on the same server and there is no authentication but I did not notice that.
There doesn't seem to be an easy to use signal to detect when running on Google Cloud: https://cloud.google.com/docs/authentication/get-id-token#metadata-server
It's either making the actual request or running an external command.
This seems to work for me and I don't see the usefulness of the object_store crate without https.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] yonch commented on pull request #3541: Additional GCP authentication
Posted by "yonch (via GitHub)" <gi...@apache.org>.
yonch commented on PR #3541:
URL: https://github.com/apache/arrow-rs/pull/3541#issuecomment-1405086255
I was looking for this functionality just yesterday and saw this commit. Thank you @winding-lines and @tustvold for getting this out!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] tustvold commented on pull request #3541: Additional GCP authentication
Posted by GitBox <gi...@apache.org>.
tustvold commented on PR #3541:
URL: https://github.com/apache/arrow-rs/pull/3541#issuecomment-1385465419
I think a good place to start would be just supporting workload identity (#3490) as documented [here](https://cloud.google.com/docs/authentication/get-id-token#metadata-server). This is very similar to the AWS [InstanceCredentialProvider](https://github.com/apache/arrow-rs/blob/master/object_store/src/aws/credential.rs#L312).
We can then look to extend this to support application default credentials as a follow up
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] winding-lines commented on a diff in pull request #3541: Additional GCP authentication
Posted by "winding-lines (via GitHub)" <gi...@apache.org>.
winding-lines commented on code in PR #3541:
URL: https://github.com/apache/arrow-rs/pull/3541#discussion_r1085297606
##########
object_store/src/gcp/credential.rs:
##########
@@ -195,6 +218,88 @@ impl OAuthProvider {
}
}
+fn reader_credentials_file<T>(
+ service_account_path: impl AsRef<std::path::Path>,
+) -> Result<T>
+where
+ T: serde::de::DeserializeOwned,
+{
+ let file = File::open(service_account_path).context(OpenCredentialsSnafu)?;
+ let reader = BufReader::new(file);
+ serde_json::from_reader(reader).context(DecodeCredentialsSnafu)
+}
+
+/// A deserialized `service-account-********.json`-file.
+#[derive(serde::Deserialize, Debug)]
+pub(crate) struct ServiceAccountCredentials {
+ /// The private key in RSA format.
+ pub private_key: String,
+
+ /// The email address associated with the service account.
+ pub client_email: String,
+
+ /// Base URL for GCS
+ #[serde(default = "default_gcs_base_url")]
+ pub gcs_base_url: String,
+
+ /// Disable oauth and use empty tokens.
+ #[serde(default = "default_disable_oauth")]
+ pub disable_oauth: bool,
+}
+
+pub(crate) fn default_gcs_base_url() -> String {
+ "https://storage.googleapis.com".to_owned()
+}
+
+pub(crate) fn default_disable_oauth() -> bool {
+ false
+}
+
+impl ServiceAccountCredentials {
+ /// Create a new [`ServiceAccountCredentials`] from a file.
+ pub fn from_file<P: AsRef<std::path::Path>>(path: P) -> Result<Self> {
+ reader_credentials_file(path)
+ }
+
+ /// Create a new [`ServiceAccountCredentials`] from a string.
+ pub fn from_key(key: &str) -> Result<Self> {
+ serde_json::from_str(key).context(DecodeCredentialsSnafu)
+ }
+
+ /// Create an [`OAuthProvider`] from this credentials struct.
+ pub fn token_provider(
+ self,
+ scope: &str,
+ audience: &str,
+ ) -> Result<Box<dyn TokenProvider>> {
+ Ok(Box::new(OAuthProvider::new(
+ self.client_email,
+ self.private_key,
+ scope.to_string(),
+ audience.to_string(),
+ )?) as Box<dyn TokenProvider>)
+ }
+}
+
+/// A no-op provider that returns empty tokens
+#[derive(Debug)]
+pub struct NoOpProvider;
+
+#[async_trait]
+impl TokenProvider for NoOpProvider {
+ /// Fetch a fresh token
+ async fn fetch_token(
+ &self,
+ _client: &Client,
+ _retry: &RetryConfig,
+ ) -> Result<TemporaryToken<String>> {
+ Ok(TemporaryToken {
+ token: "".to_string(),
+ expiry: Instant::now(),
+ })
+ }
+}
+
Review Comment:
Good catch, deleted.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] winding-lines commented on a diff in pull request #3541: Additional GCP authentication
Posted by "winding-lines (via GitHub)" <gi...@apache.org>.
winding-lines commented on code in PR #3541:
URL: https://github.com/apache/arrow-rs/pull/3541#discussion_r1085314973
##########
object_store/src/gcp/credential.rs:
##########
@@ -222,3 +327,118 @@ fn b64_encode_obj<T: serde::Serialize>(obj: &T) -> Result<String> {
let string = serde_json::to_string(obj).context(EncodeSnafu)?;
Ok(BASE64_URL_SAFE_NO_PAD.encode(string))
}
+
+/// A provider that uses the Google Cloud Platform metadata server to fetch a token.
+#[derive(Debug, Default)]
+pub struct InstanceCredentialProvider {
+ audience: String,
+}
+
+impl InstanceCredentialProvider {
+ pub fn new<T: Into<String>>(audience: T) -> Self {
+ Self {
+ audience: audience.into(),
+ }
+ }
+}
+
+#[async_trait]
+impl TokenProvider for InstanceCredentialProvider {
+ async fn fetch_token(
+ &self,
+ client: &Client,
+ retry: &RetryConfig,
+ ) -> Result<TemporaryToken<String>> {
+ println!("fetching token from metadata server");
Review Comment:
Added, good idea.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] winding-lines commented on a diff in pull request #3541: Additional GCP authentication
Posted by "winding-lines (via GitHub)" <gi...@apache.org>.
winding-lines commented on code in PR #3541:
URL: https://github.com/apache/arrow-rs/pull/3541#discussion_r1086177439
##########
object_store/src/gcp/credential.rs:
##########
@@ -222,3 +327,118 @@ fn b64_encode_obj<T: serde::Serialize>(obj: &T) -> Result<String> {
let string = serde_json::to_string(obj).context(EncodeSnafu)?;
Ok(BASE64_URL_SAFE_NO_PAD.encode(string))
}
+
+/// A provider that uses the Google Cloud Platform metadata server to fetch a token.
+#[derive(Debug, Default)]
+pub struct InstanceCredentialProvider {
+ audience: String,
+}
+
+impl InstanceCredentialProvider {
+ pub fn new<T: Into<String>>(audience: T) -> Self {
+ Self {
+ audience: audience.into(),
+ }
+ }
+}
+
+#[async_trait]
+impl TokenProvider for InstanceCredentialProvider {
+ async fn fetch_token(
+ &self,
+ client: &Client,
+ retry: &RetryConfig,
+ ) -> Result<TemporaryToken<String>> {
+ println!("fetching token from metadata server");
+ const TOKEN_URL: &str =
+ "http://metadata/computeMetadata/v1/instance/service-accounts/default/token";
+ let response: TokenResponse = client
+ .request(Method::GET, TOKEN_URL)
+ .header("Metadata-Flavor", "Google")
+ .query(&[("audience", &self.audience)])
+ .send_retry(retry)
+ .await
+ .context(TokenRequestSnafu)?
+ .json()
+ .await
+ .context(TokenResponseBodySnafu)?;
+ let token = TemporaryToken {
+ token: response.access_token,
+ expiry: Instant::now() + Duration::from_secs(response.expires_in),
+ };
+ Ok(token)
+ }
+}
+
+/// A deserialized `application_default_credentials.json`-file.
+#[derive(serde::Deserialize, Debug)]
+pub struct ApplicationDefaultCredentials {
+ client_id: String,
+ client_secret: String,
+ refresh_token: String,
+ #[serde(rename = "type")]
+ type_: String,
+}
+
+impl ApplicationDefaultCredentials {
+ const DEFAULT_TOKEN_GCP_URI: &'static str =
+ "https://accounts.google.com/o/oauth2/token";
+ const CREDENTIALS_PATH: &'static str =
+ ".config/gcloud/application_default_credentials.json";
+ const EXPECTED_TYPE: &str = "authorized_user";
+
+ // Create a new application default credential in the following situations:
+ // 1. a file is passed in and the type matches.
+ // 2. without argument if the well-known configuration file is present.
+ pub fn new(path: Option<&str>) -> Result<Option<Self>, Error> {
+ if let Some(path) = path {
+ if let Ok(credentials) = reader_credentials_file::<Self>(path) {
+ if credentials.type_ == Self::EXPECTED_TYPE {
+ return Ok(Some(credentials));
+ }
+ }
+ // Other credential mechanisms may be able to use this path.
+ return Ok(None);
+ }
+ if let Some(home) = env::var_os("HOME") {
Review Comment:
`dirs` look like a good option but you mentioned wanting to keep the dependencies lean :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] winding-lines commented on a diff in pull request #3541: Additional GCP authentication
Posted by "winding-lines (via GitHub)" <gi...@apache.org>.
winding-lines commented on code in PR #3541:
URL: https://github.com/apache/arrow-rs/pull/3541#discussion_r1086722962
##########
object_store/src/gcp/credential.rs:
##########
@@ -329,39 +319,69 @@ fn b64_encode_obj<T: serde::Serialize>(obj: &T) -> Result<String> {
}
/// A provider that uses the Google Cloud Platform metadata server to fetch a token.
+///
+/// <https://cloud.google.com/docs/authentication/get-id-token#metadata-server>
#[derive(Debug, Default)]
pub struct InstanceCredentialProvider {
audience: String,
+ client_options: ClientOptions,
}
impl InstanceCredentialProvider {
- pub fn new<T: Into<String>>(audience: T) -> Self {
+ /// Create a new [`InstanceCredentialProvider`], we need to control the client in order to enable http access so save the options.
+ pub fn new<T: Into<String>>(audience: T, client_options: ClientOptions) -> Self {
Self {
audience: audience.into(),
+ client_options: client_options.with_allow_http(true),
}
}
}
+/// Make a request to the metadata server to fetch a token, using a a given hostname.
+async fn make_metadata_request(
+ client: &Client,
+ hostname: &str,
+ retry: &RetryConfig,
+ audience: &str,
+) -> Result<TokenResponse> {
+ let url = format!(
+ "http://{}/computeMetadata/v1/instance/service-accounts/default/token",
+ hostname
+ );
+ println!("gcp/credentials getting token from url: {}", url);
Review Comment:
oops, deleted
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] tustvold commented on a diff in pull request #3541: Additional GCP authentication
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on code in PR #3541:
URL: https://github.com/apache/arrow-rs/pull/3541#discussion_r1085321384
##########
object_store/src/gcp/credential.rs:
##########
@@ -222,3 +327,118 @@ fn b64_encode_obj<T: serde::Serialize>(obj: &T) -> Result<String> {
let string = serde_json::to_string(obj).context(EncodeSnafu)?;
Ok(BASE64_URL_SAFE_NO_PAD.encode(string))
}
+
+/// A provider that uses the Google Cloud Platform metadata server to fetch a token.
+#[derive(Debug, Default)]
+pub struct InstanceCredentialProvider {
+ audience: String,
+}
+
+impl InstanceCredentialProvider {
+ pub fn new<T: Into<String>>(audience: T) -> Self {
+ Self {
+ audience: audience.into(),
+ }
+ }
+}
+
+#[async_trait]
+impl TokenProvider for InstanceCredentialProvider {
+ async fn fetch_token(
+ &self,
+ client: &Client,
Review Comment:
The problem is if you enable only https on ClientOptions, this credential provider will now fail. This needs to use its own client, same as the AWS one
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] winding-lines commented on pull request #3541: Additional GCP authentication
Posted by GitBox <gi...@apache.org>.
winding-lines commented on PR #3541:
URL: https://github.com/apache/arrow-rs/pull/3541#issuecomment-1385498280
Ok, so the intent is just to fetch the token from the local metadata server when running on GKE and similar. Thanks for the clarification 🙏🏻
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] winding-lines commented on a diff in pull request #3541: Additional GCP authentication
Posted by "winding-lines (via GitHub)" <gi...@apache.org>.
winding-lines commented on code in PR #3541:
URL: https://github.com/apache/arrow-rs/pull/3541#discussion_r1085299109
##########
object_store/src/gcp/credential.rs:
##########
@@ -222,3 +327,118 @@ fn b64_encode_obj<T: serde::Serialize>(obj: &T) -> Result<String> {
let string = serde_json::to_string(obj).context(EncodeSnafu)?;
Ok(BASE64_URL_SAFE_NO_PAD.encode(string))
}
+
+/// A provider that uses the Google Cloud Platform metadata server to fetch a token.
+#[derive(Debug, Default)]
Review Comment:
Nice, added :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] winding-lines commented on a diff in pull request #3541: Additional GCP authentication
Posted by "winding-lines (via GitHub)" <gi...@apache.org>.
winding-lines commented on code in PR #3541:
URL: https://github.com/apache/arrow-rs/pull/3541#discussion_r1085290566
##########
object_store/src/gcp/credential.rs:
##########
@@ -104,6 +115,15 @@ struct TokenResponse {
expires_in: u64,
}
+#[async_trait]
+pub(crate) trait TokenProvider: std::fmt::Debug + Send + Sync {
Review Comment:
Changed, I was overly cautious here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] winding-lines commented on a diff in pull request #3541: Additional GCP authentication
Posted by "winding-lines (via GitHub)" <gi...@apache.org>.
winding-lines commented on code in PR #3541:
URL: https://github.com/apache/arrow-rs/pull/3541#discussion_r1085351047
##########
object_store/src/gcp/credential.rs:
##########
@@ -222,3 +327,118 @@ fn b64_encode_obj<T: serde::Serialize>(obj: &T) -> Result<String> {
let string = serde_json::to_string(obj).context(EncodeSnafu)?;
Ok(BASE64_URL_SAFE_NO_PAD.encode(string))
}
+
+/// A provider that uses the Google Cloud Platform metadata server to fetch a token.
+#[derive(Debug, Default)]
+pub struct InstanceCredentialProvider {
+ audience: String,
+}
+
+impl InstanceCredentialProvider {
+ pub fn new<T: Into<String>>(audience: T) -> Self {
+ Self {
+ audience: audience.into(),
+ }
+ }
+}
+
+#[async_trait]
+impl TokenProvider for InstanceCredentialProvider {
+ async fn fetch_token(
+ &self,
+ client: &Client,
+ retry: &RetryConfig,
+ ) -> Result<TemporaryToken<String>> {
+ println!("fetching token from metadata server");
+ const TOKEN_URL: &str =
+ "http://metadata/computeMetadata/v1/instance/service-accounts/default/token";
+ let response: TokenResponse = client
Review Comment:
Added, need to test now :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] winding-lines commented on a diff in pull request #3541: Additional GCP authentication
Posted by "winding-lines (via GitHub)" <gi...@apache.org>.
winding-lines commented on code in PR #3541:
URL: https://github.com/apache/arrow-rs/pull/3541#discussion_r1085331866
##########
object_store/src/gcp/credential.rs:
##########
@@ -222,3 +327,118 @@ fn b64_encode_obj<T: serde::Serialize>(obj: &T) -> Result<String> {
let string = serde_json::to_string(obj).context(EncodeSnafu)?;
Ok(BASE64_URL_SAFE_NO_PAD.encode(string))
}
+
+/// A provider that uses the Google Cloud Platform metadata server to fetch a token.
+#[derive(Debug, Default)]
+pub struct InstanceCredentialProvider {
+ audience: String,
+}
+
+impl InstanceCredentialProvider {
+ pub fn new<T: Into<String>>(audience: T) -> Self {
+ Self {
+ audience: audience.into(),
+ }
+ }
+}
+
+#[async_trait]
+impl TokenProvider for InstanceCredentialProvider {
+ async fn fetch_token(
+ &self,
+ client: &Client,
Review Comment:
Ok, having a different client reduces the scope of the change, thanks for explaining.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] tustvold commented on pull request #3541: Additional GCP authentication
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on PR #3541:
URL: https://github.com/apache/arrow-rs/pull/3541#issuecomment-1403791838
Thanks again
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] winding-lines commented on a diff in pull request #3541: Additional GCP authentication
Posted by "winding-lines (via GitHub)" <gi...@apache.org>.
winding-lines commented on code in PR #3541:
URL: https://github.com/apache/arrow-rs/pull/3541#discussion_r1085358261
##########
object_store/src/gcp/credential.rs:
##########
@@ -222,3 +327,118 @@ fn b64_encode_obj<T: serde::Serialize>(obj: &T) -> Result<String> {
let string = serde_json::to_string(obj).context(EncodeSnafu)?;
Ok(BASE64_URL_SAFE_NO_PAD.encode(string))
}
+
+/// A provider that uses the Google Cloud Platform metadata server to fetch a token.
+#[derive(Debug, Default)]
+pub struct InstanceCredentialProvider {
+ audience: String,
+}
+
+impl InstanceCredentialProvider {
+ pub fn new<T: Into<String>>(audience: T) -> Self {
+ Self {
+ audience: audience.into(),
+ }
+ }
+}
+
+#[async_trait]
+impl TokenProvider for InstanceCredentialProvider {
+ async fn fetch_token(
+ &self,
+ client: &Client,
+ retry: &RetryConfig,
+ ) -> Result<TemporaryToken<String>> {
+ println!("fetching token from metadata server");
+ const TOKEN_URL: &str =
+ "http://metadata/computeMetadata/v1/instance/service-accounts/default/token";
+ let response: TokenResponse = client
+ .request(Method::GET, TOKEN_URL)
+ .header("Metadata-Flavor", "Google")
+ .query(&[("audience", &self.audience)])
+ .send_retry(retry)
+ .await
+ .context(TokenRequestSnafu)?
+ .json()
+ .await
+ .context(TokenResponseBodySnafu)?;
+ let token = TemporaryToken {
+ token: response.access_token,
+ expiry: Instant::now() + Duration::from_secs(response.expires_in),
+ };
+ Ok(token)
+ }
+}
+
+/// A deserialized `application_default_credentials.json`-file.
+#[derive(serde::Deserialize, Debug)]
+pub struct ApplicationDefaultCredentials {
+ client_id: String,
+ client_secret: String,
+ refresh_token: String,
+ #[serde(rename = "type")]
+ type_: String,
+}
+
+impl ApplicationDefaultCredentials {
+ const DEFAULT_TOKEN_GCP_URI: &'static str =
+ "https://accounts.google.com/o/oauth2/token";
+ const CREDENTIALS_PATH: &'static str =
+ ".config/gcloud/application_default_credentials.json";
+ const EXPECTED_TYPE: &str = "authorized_user";
+
+ // Create a new application default credential in the following situations:
+ // 1. a file is passed in and the type matches.
+ // 2. without argument if the well-known configuration file is present.
+ pub fn new(path: Option<&str>) -> Result<Option<Self>, Error> {
+ if let Some(path) = path {
+ if let Ok(credentials) = reader_credentials_file::<Self>(path) {
+ if credentials.type_ == Self::EXPECTED_TYPE {
+ return Ok(Some(credentials));
+ }
+ }
+ // Other credential mechanisms may be able to use this path.
+ return Ok(None);
Review Comment:
My thinking here is that in addition to the Application Default Credentials a configuration file is also used by Workload Identity Federation. So I wanted to leave this open for additional processing. I see your point that the additional implementation is not part of this PR so we should have the error checking to be consistent.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] jychen7 commented on a diff in pull request #3541: Additional GCP authentication
Posted by "jychen7 (via GitHub)" <gi...@apache.org>.
jychen7 commented on code in PR #3541:
URL: https://github.com/apache/arrow-rs/pull/3541#discussion_r1085691517
##########
object_store/src/gcp/credential.rs:
##########
@@ -222,3 +327,118 @@ fn b64_encode_obj<T: serde::Serialize>(obj: &T) -> Result<String> {
let string = serde_json::to_string(obj).context(EncodeSnafu)?;
Ok(BASE64_URL_SAFE_NO_PAD.encode(string))
}
+
+/// A provider that uses the Google Cloud Platform metadata server to fetch a token.
+#[derive(Debug, Default)]
+pub struct InstanceCredentialProvider {
+ audience: String,
+}
+
+impl InstanceCredentialProvider {
+ pub fn new<T: Into<String>>(audience: T) -> Self {
+ Self {
+ audience: audience.into(),
+ }
+ }
+}
+
+#[async_trait]
+impl TokenProvider for InstanceCredentialProvider {
+ async fn fetch_token(
+ &self,
+ client: &Client,
+ retry: &RetryConfig,
+ ) -> Result<TemporaryToken<String>> {
+ println!("fetching token from metadata server");
+ const TOKEN_URL: &str =
+ "http://metadata/computeMetadata/v1/instance/service-accounts/default/token";
Review Comment:
understood, I think Google just have inconsistent documents. Both work for me.
ps: I was looking at https://cloud.google.com/kubernetes-engine/docs/concepts/workload-identity#metadata_server and it mentions the full hostname `metadata.google.internal` and IP `169.254.169.254`. It also reference https://cloud.google.com/compute/docs/metadata/overview
I agree that https://cloud.google.com/docs/authentication/get-id-token#metadata-server only show `metadata`
(sorry, I can not close this comment, maybe no permission. Feel free to ignore)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] winding-lines commented on pull request #3541: Additional GCP authentication
Posted by "winding-lines (via GitHub)" <gi...@apache.org>.
winding-lines commented on PR #3541:
URL: https://github.com/apache/arrow-rs/pull/3541#issuecomment-1403801829
Thanks a lot @tustvold, looking forward to contributing more in the near future.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] winding-lines commented on a diff in pull request #3541: Additional GCP authentication
Posted by "winding-lines (via GitHub)" <gi...@apache.org>.
winding-lines commented on code in PR #3541:
URL: https://github.com/apache/arrow-rs/pull/3541#discussion_r1086723328
##########
object_store/src/gcp/credential.rs:
##########
@@ -329,39 +319,69 @@ fn b64_encode_obj<T: serde::Serialize>(obj: &T) -> Result<String> {
}
/// A provider that uses the Google Cloud Platform metadata server to fetch a token.
+///
+/// <https://cloud.google.com/docs/authentication/get-id-token#metadata-server>
#[derive(Debug, Default)]
pub struct InstanceCredentialProvider {
audience: String,
+ client_options: ClientOptions,
Review Comment:
Yeah, I was thinking about that. Let me try it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] winding-lines commented on a diff in pull request #3541: Additional GCP authentication
Posted by "winding-lines (via GitHub)" <gi...@apache.org>.
winding-lines commented on code in PR #3541:
URL: https://github.com/apache/arrow-rs/pull/3541#discussion_r1085314973
##########
object_store/src/gcp/credential.rs:
##########
@@ -222,3 +327,118 @@ fn b64_encode_obj<T: serde::Serialize>(obj: &T) -> Result<String> {
let string = serde_json::to_string(obj).context(EncodeSnafu)?;
Ok(BASE64_URL_SAFE_NO_PAD.encode(string))
}
+
+/// A provider that uses the Google Cloud Platform metadata server to fetch a token.
+#[derive(Debug, Default)]
+pub struct InstanceCredentialProvider {
+ audience: String,
+}
+
+impl InstanceCredentialProvider {
+ pub fn new<T: Into<String>>(audience: T) -> Self {
+ Self {
+ audience: audience.into(),
+ }
+ }
+}
+
+#[async_trait]
+impl TokenProvider for InstanceCredentialProvider {
+ async fn fetch_token(
+ &self,
+ client: &Client,
+ retry: &RetryConfig,
+ ) -> Result<TemporaryToken<String>> {
+ println!("fetching token from metadata server");
Review Comment:
Changed, good idea. I did not actually mean to leave that in there so I am ok to also delete it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] tustvold merged pull request #3541: Additional GCP authentication
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold merged PR #3541:
URL: https://github.com/apache/arrow-rs/pull/3541
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] jychen7 commented on a diff in pull request #3541: Additional GCP authentication
Posted by "jychen7 (via GitHub)" <gi...@apache.org>.
jychen7 commented on code in PR #3541:
URL: https://github.com/apache/arrow-rs/pull/3541#discussion_r1085691517
##########
object_store/src/gcp/credential.rs:
##########
@@ -222,3 +327,118 @@ fn b64_encode_obj<T: serde::Serialize>(obj: &T) -> Result<String> {
let string = serde_json::to_string(obj).context(EncodeSnafu)?;
Ok(BASE64_URL_SAFE_NO_PAD.encode(string))
}
+
+/// A provider that uses the Google Cloud Platform metadata server to fetch a token.
+#[derive(Debug, Default)]
+pub struct InstanceCredentialProvider {
+ audience: String,
+}
+
+impl InstanceCredentialProvider {
+ pub fn new<T: Into<String>>(audience: T) -> Self {
+ Self {
+ audience: audience.into(),
+ }
+ }
+}
+
+#[async_trait]
+impl TokenProvider for InstanceCredentialProvider {
+ async fn fetch_token(
+ &self,
+ client: &Client,
+ retry: &RetryConfig,
+ ) -> Result<TemporaryToken<String>> {
+ println!("fetching token from metadata server");
+ const TOKEN_URL: &str =
+ "http://metadata/computeMetadata/v1/instance/service-accounts/default/token";
Review Comment:
understood, I think Google just have inconsistent documents. Both work for me.
ps: I was looking at https://cloud.google.com/kubernetes-engine/docs/concepts/workload-identity#metadata_server and it mentions the full hostname `metadata.google.internal` and IP `169.254.169.254`. It also reference https://cloud.google.com/compute/docs/metadata/overview
I agree that https://cloud.google.com/docs/authentication/get-id-token#metadata-server only show `metadata`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] jychen7 commented on a diff in pull request #3541: Additional GCP authentication
Posted by "jychen7 (via GitHub)" <gi...@apache.org>.
jychen7 commented on code in PR #3541:
URL: https://github.com/apache/arrow-rs/pull/3541#discussion_r1085691517
##########
object_store/src/gcp/credential.rs:
##########
@@ -222,3 +327,118 @@ fn b64_encode_obj<T: serde::Serialize>(obj: &T) -> Result<String> {
let string = serde_json::to_string(obj).context(EncodeSnafu)?;
Ok(BASE64_URL_SAFE_NO_PAD.encode(string))
}
+
+/// A provider that uses the Google Cloud Platform metadata server to fetch a token.
+#[derive(Debug, Default)]
+pub struct InstanceCredentialProvider {
+ audience: String,
+}
+
+impl InstanceCredentialProvider {
+ pub fn new<T: Into<String>>(audience: T) -> Self {
+ Self {
+ audience: audience.into(),
+ }
+ }
+}
+
+#[async_trait]
+impl TokenProvider for InstanceCredentialProvider {
+ async fn fetch_token(
+ &self,
+ client: &Client,
+ retry: &RetryConfig,
+ ) -> Result<TemporaryToken<String>> {
+ println!("fetching token from metadata server");
+ const TOKEN_URL: &str =
+ "http://metadata/computeMetadata/v1/instance/service-accounts/default/token";
Review Comment:
understood, I think Google just have inconsistent documents. Both work for me.
ps: I was looking at https://cloud.google.com/kubernetes-engine/docs/concepts/workload-identity#metadata_server and it mentions the full hostname `metadata.google.internal` and IP `169.254.169.254`. It also reference https://cloud.google.com/compute/docs/metadata/overview
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] winding-lines commented on pull request #3541: Additional GCP authentication
Posted by "winding-lines (via GitHub)" <gi...@apache.org>.
winding-lines commented on PR #3541:
URL: https://github.com/apache/arrow-rs/pull/3541#issuecomment-1399722806
Great! I work on this mostly in the weekends, but of course I don't expect weekend reviewers.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] tustvold commented on a diff in pull request #3541: Additional GCP authentication
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on code in PR #3541:
URL: https://github.com/apache/arrow-rs/pull/3541#discussion_r1084117015
##########
object_store/src/gcp/credential.rs:
##########
@@ -222,3 +327,118 @@ fn b64_encode_obj<T: serde::Serialize>(obj: &T) -> Result<String> {
let string = serde_json::to_string(obj).context(EncodeSnafu)?;
Ok(BASE64_URL_SAFE_NO_PAD.encode(string))
}
+
+/// A provider that uses the Google Cloud Platform metadata server to fetch a token.
+#[derive(Debug, Default)]
+pub struct InstanceCredentialProvider {
+ audience: String,
+}
+
+impl InstanceCredentialProvider {
+ pub fn new<T: Into<String>>(audience: T) -> Self {
+ Self {
+ audience: audience.into(),
+ }
+ }
+}
+
+#[async_trait]
+impl TokenProvider for InstanceCredentialProvider {
+ async fn fetch_token(
+ &self,
+ client: &Client,
Review Comment:
One thing to double-check is what happens if https is disabled for the client. At least for AWS the client is created as part of the credential, and explicitly permits http for the `InstanceCredentialProvider`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] jychen7 commented on a diff in pull request #3541: Additional GCP authentication
Posted by "jychen7 (via GitHub)" <gi...@apache.org>.
jychen7 commented on code in PR #3541:
URL: https://github.com/apache/arrow-rs/pull/3541#discussion_r1084807893
##########
object_store/src/gcp/credential.rs:
##########
@@ -222,3 +327,118 @@ fn b64_encode_obj<T: serde::Serialize>(obj: &T) -> Result<String> {
let string = serde_json::to_string(obj).context(EncodeSnafu)?;
Ok(BASE64_URL_SAFE_NO_PAD.encode(string))
}
+
+/// A provider that uses the Google Cloud Platform metadata server to fetch a token.
+#[derive(Debug, Default)]
+pub struct InstanceCredentialProvider {
+ audience: String,
+}
+
+impl InstanceCredentialProvider {
+ pub fn new<T: Into<String>>(audience: T) -> Self {
+ Self {
+ audience: audience.into(),
+ }
+ }
+}
+
+#[async_trait]
+impl TokenProvider for InstanceCredentialProvider {
+ async fn fetch_token(
+ &self,
+ client: &Client,
+ retry: &RetryConfig,
+ ) -> Result<TemporaryToken<String>> {
+ println!("fetching token from metadata server");
+ const TOKEN_URL: &str =
+ "http://metadata/computeMetadata/v1/instance/service-accounts/default/token";
Review Comment:
```suggestion
"http://metadata.google.internal/computeMetadata/v1/instance/service-accounts/default/token";
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] winding-lines commented on pull request #3541: Additional GCP authentication
Posted by "winding-lines (via GitHub)" <gi...@apache.org>.
winding-lines commented on PR #3541:
URL: https://github.com/apache/arrow-rs/pull/3541#issuecomment-1399403751
@tustvold here is the code to implement
1. fetching the token from the metadata server on Google Cloud
2. fetching the token from the Application Default Credentials
I got an instance on Google Cloud and tested this code.
Looking forward to your feedback :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] winding-lines commented on pull request #3541: Additional GCP authentication
Posted by GitBox <gi...@apache.org>.
winding-lines commented on PR #3541:
URL: https://github.com/apache/arrow-rs/pull/3541#issuecomment-1385441224
Thanks for your reply. I am not using workload identity in my daily work, I am just trying to help move forward a great project. Could you be more specific about what the library should support?
I also looked in the azure and aws implementations but jt is still not clear to me.
Many thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] tustvold commented on pull request #3541: Additional GCP authentication
Posted by GitBox <gi...@apache.org>.
tustvold commented on PR #3541:
URL: https://github.com/apache/arrow-rs/pull/3541#issuecomment-1385136760
I'm confused workload identity **federation** is different from workload identity? The ticket is for the latter but this PR references the former? I agree workload identity federation is highly complex, and may warrant a different approach, but I'm also not sure if this is something we need to support?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] ursabot commented on pull request #3541: Additional GCP authentication
Posted by "ursabot (via GitHub)" <gi...@apache.org>.
ursabot commented on PR #3541:
URL: https://github.com/apache/arrow-rs/pull/3541#issuecomment-1403803161
Benchmark runs are scheduled for baseline = 73ce0760d849722e71a8a1a44b6b3230ae68f8b3 and contender = 42b2d55e407217a95fab70e8b6c9834f2a39fa1e. 42b2d55e407217a95fab70e8b6c9834f2a39fa1e is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Skipped :warning: Benchmarking of arrow-rs-commits is not supported on ec2-t3-xlarge-us-east-2] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/dc3e3e7600ee42cd86932d24b1063758...9f05aaabead34f54bb08a3025f05a61d/)
[Skipped :warning: Benchmarking of arrow-rs-commits is not supported on test-mac-arm] [test-mac-arm](https://conbench.ursa.dev/compare/runs/c2865d9bf8cf491caf46d761426da7e9...8d821c675e484cbf967cad7664c546eb/)
[Skipped :warning: Benchmarking of arrow-rs-commits is not supported on ursa-i9-9960x] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/a2fafbbbe7b248b6a907018e412bf41f...2e582a5ae741412f9936b43eee469228/)
[Skipped :warning: Benchmarking of arrow-rs-commits is not supported on ursa-thinkcentre-m75q] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/e6aebb39697d42d0b2998669eb591705...7ab75a58436849b2b9f5d2ed5c956236/)
Buildkite builds:
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] winding-lines commented on a diff in pull request #3541: Additional GCP authentication
Posted by "winding-lines (via GitHub)" <gi...@apache.org>.
winding-lines commented on code in PR #3541:
URL: https://github.com/apache/arrow-rs/pull/3541#discussion_r1085338917
##########
object_store/src/gcp/mod.rs:
##########
@@ -1098,44 +1083,71 @@ impl GoogleCloudStorageBuilder {
let client = self.client_options.client()?;
- let credentials = match (self.service_account_path, self.service_account_key) {
- (Some(path), None) => reader_credentials_file(path)?,
- (None, Some(key)) => {
- serde_json::from_str(&key).context(DecodeCredentialsSnafu)?
- }
- (None, None) => return Err(Error::MissingServiceAccountPathOrKey.into()),
- (Some(_), Some(_)) => {
- return Err(Error::ServiceAccountPathAndKeyProvided.into())
- }
- };
+ // First try to initialize from the service account information.
+ let service_account_credentials =
+ match (self.service_account_path, self.service_account_key) {
+ (Some(path), None) => Some(
+ ServiceAccountCredentials::from_file(path)
+ .context(CredentialSnafu)?,
+ ),
+ (None, Some(key)) => Some(
+ ServiceAccountCredentials::from_key(&key).context(CredentialSnafu)?,
+ ),
+ (None, None) => None,
+ (Some(_), Some(_)) => {
+ return Err(Error::ServiceAccountPathAndKeyProvided.into())
+ }
+ };
+
+ // Then try to initialize from the application credentials file, or the environment.
+ let application_default_credentials = ApplicationDefaultCredentials::new(
+ self.application_credentials_path.as_deref(),
+ )
+ .context(CredentialSnafu)?;
+
+ let disable_oauth = service_account_credentials
+ .as_ref()
+ .map(|c| c.disable_oauth)
+ .unwrap_or(false);
+
+ let gcs_base_url = service_account_credentials
+ .as_ref()
+ .map(|c| c.gcs_base_url.clone())
+ .unwrap_or_else(default_gcs_base_url);
// TODO: https://cloud.google.com/storage/docs/authentication#oauth-scopes
let scope = "https://www.googleapis.com/auth/devstorage.full_control";
- let audience = "https://www.googleapis.com/oauth2/v4/token".to_string();
-
- let oauth_provider = (!credentials.disable_oauth)
- .then(|| {
- OAuthProvider::new(
- credentials.client_email,
- credentials.private_key,
- scope.to_string(),
- audience,
- )
- })
- .transpose()
- .context(CredentialSnafu)?;
+ let audience = "https://www.googleapis.com/oauth2/v4/token";
+
+ let token_provider = if disable_oauth {
+ None
+ } else {
+ let best_provider = service_account_credentials
+ .map(|credentials| credentials.token_provider(scope, audience))
+ .transpose()
+ .context(CredentialSnafu)?
+ .or_else(|| {
+ application_default_credentials
+ .map(|a| Box::new(a) as Box<dyn TokenProvider>)
+ })
+ .or_else(|| Some(Box::new(InstanceCredentialProvider::new(audience))));
+
+ // A provider is required at this point, bail out if we don't have one.
+ if best_provider.is_some() {
+ best_provider
+ } else {
+ return Err(Error::MissingCredentials.into());
+ }
Review Comment:
Duh :) Thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] winding-lines commented on a diff in pull request #3541: Additional GCP authentication
Posted by "winding-lines (via GitHub)" <gi...@apache.org>.
winding-lines commented on code in PR #3541:
URL: https://github.com/apache/arrow-rs/pull/3541#discussion_r1085318073
##########
object_store/src/gcp/credential.rs:
##########
@@ -222,3 +327,118 @@ fn b64_encode_obj<T: serde::Serialize>(obj: &T) -> Result<String> {
let string = serde_json::to_string(obj).context(EncodeSnafu)?;
Ok(BASE64_URL_SAFE_NO_PAD.encode(string))
}
+
+/// A provider that uses the Google Cloud Platform metadata server to fetch a token.
+#[derive(Debug, Default)]
+pub struct InstanceCredentialProvider {
+ audience: String,
+}
+
+impl InstanceCredentialProvider {
+ pub fn new<T: Into<String>>(audience: T) -> Self {
+ Self {
+ audience: audience.into(),
+ }
+ }
+}
+
+#[async_trait]
+impl TokenProvider for InstanceCredentialProvider {
+ async fn fetch_token(
+ &self,
+ client: &Client,
+ retry: &RetryConfig,
+ ) -> Result<TemporaryToken<String>> {
+ println!("fetching token from metadata server");
+ const TOKEN_URL: &str =
+ "http://metadata/computeMetadata/v1/instance/service-accounts/default/token";
Review Comment:
Google's documentation specifically has only the hostname without the `google.internal` domain. I was able to test with the hostname only. I see the value of using the full domain here but I prefer to stick with the documenation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] tustvold commented on pull request #3541: Additional GCP authentication
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on PR #3541:
URL: https://github.com/apache/arrow-rs/pull/3541#issuecomment-1399598871
I intend to review this first thing tomorrow
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] winding-lines commented on a diff in pull request #3541: Additional GCP authentication
Posted by "winding-lines (via GitHub)" <gi...@apache.org>.
winding-lines commented on code in PR #3541:
URL: https://github.com/apache/arrow-rs/pull/3541#discussion_r1085296501
##########
object_store/src/gcp/credential.rs:
##########
@@ -195,6 +218,88 @@ impl OAuthProvider {
}
}
+fn reader_credentials_file<T>(
+ service_account_path: impl AsRef<std::path::Path>,
+) -> Result<T>
+where
+ T: serde::de::DeserializeOwned,
+{
+ let file = File::open(service_account_path).context(OpenCredentialsSnafu)?;
+ let reader = BufReader::new(file);
+ serde_json::from_reader(reader).context(DecodeCredentialsSnafu)
+}
+
+/// A deserialized `service-account-********.json`-file.
+#[derive(serde::Deserialize, Debug)]
+pub(crate) struct ServiceAccountCredentials {
+ /// The private key in RSA format.
+ pub private_key: String,
+
+ /// The email address associated with the service account.
+ pub client_email: String,
+
+ /// Base URL for GCS
+ #[serde(default = "default_gcs_base_url")]
+ pub gcs_base_url: String,
+
+ /// Disable oauth and use empty tokens.
+ #[serde(default = "default_disable_oauth")]
+ pub disable_oauth: bool,
+}
+
+pub(crate) fn default_gcs_base_url() -> String {
Review Comment:
I think these are used to enable some integration testing and as such will never be in the json file in normal operation. According to the `serde` documentation they need to be functions... https://serde.rs/field-attrs.html#default--path
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] winding-lines commented on a diff in pull request #3541: Additional GCP authentication
Posted by "winding-lines (via GitHub)" <gi...@apache.org>.
winding-lines commented on code in PR #3541:
URL: https://github.com/apache/arrow-rs/pull/3541#discussion_r1085286727
##########
object_store/src/gcp/credential.rs:
##########
@@ -195,6 +218,88 @@ impl OAuthProvider {
}
}
+fn reader_credentials_file<T>(
Review Comment:
Agree, changed to `read_credentials_file`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] jychen7 commented on a diff in pull request #3541: Additional GCP authentication
Posted by "jychen7 (via GitHub)" <gi...@apache.org>.
jychen7 commented on code in PR #3541:
URL: https://github.com/apache/arrow-rs/pull/3541#discussion_r1085691517
##########
object_store/src/gcp/credential.rs:
##########
@@ -222,3 +327,118 @@ fn b64_encode_obj<T: serde::Serialize>(obj: &T) -> Result<String> {
let string = serde_json::to_string(obj).context(EncodeSnafu)?;
Ok(BASE64_URL_SAFE_NO_PAD.encode(string))
}
+
+/// A provider that uses the Google Cloud Platform metadata server to fetch a token.
+#[derive(Debug, Default)]
+pub struct InstanceCredentialProvider {
+ audience: String,
+}
+
+impl InstanceCredentialProvider {
+ pub fn new<T: Into<String>>(audience: T) -> Self {
+ Self {
+ audience: audience.into(),
+ }
+ }
+}
+
+#[async_trait]
+impl TokenProvider for InstanceCredentialProvider {
+ async fn fetch_token(
+ &self,
+ client: &Client,
+ retry: &RetryConfig,
+ ) -> Result<TemporaryToken<String>> {
+ println!("fetching token from metadata server");
+ const TOKEN_URL: &str =
+ "http://metadata/computeMetadata/v1/instance/service-accounts/default/token";
Review Comment:
understood, I think Google just have inconsistent documents. Both work for me.
ps: I was looking at https://cloud.google.com/kubernetes-engine/docs/concepts/workload-identity#metadata_server and it mentions the full hostname `metadata.google.internal` and IP `169.254.169.254`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org