This is an automated email from the ASF dual-hosted git repository. tustvold pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/arrow-rs.git
The following commit(s) were added to refs/heads/master by this push: new eae993fd1 feat: Allow providing a service account key directly for GCS (#3489) eae993fd1 is described below commit eae993fd196d0a8df8a90857bc4a7ae8f5a3e845 Author: Sean Smith <scsmi...@gmail.com> AuthorDate: Mon Jan 9 04:25:29 2023 -0600 feat: Allow providing a service account key directly for GCS (#3489) * feat: Allow providing a service account key directly for GCP Use case: We're storing service accounts keys external to where the object store client is being created. We do not want to have to write the key to a file before creating the object store client. This change allows for providing the key directly. * Add additional aliases for specifying service account path "google_service_account_path" and "service_account_path" can now be used. * Add test asserting aliases set appropriate config option --- object_store/src/gcp/mod.rs | 144 +++++++++++++++++++++++++++++++++++++++----- 1 file changed, 128 insertions(+), 16 deletions(-) diff --git a/object_store/src/gcp/mod.rs b/object_store/src/gcp/mod.rs index 177812fa8..28972c4a6 100644 --- a/object_store/src/gcp/mod.rs +++ b/object_store/src/gcp/mod.rs @@ -121,8 +121,13 @@ enum Error { #[snafu(display("Missing bucket name"))] MissingBucketName {}, - #[snafu(display("Missing service account path"))] - MissingServiceAccountPath, + #[snafu(display("Missing service account path or key"))] + MissingServiceAccountPathOrKey, + + #[snafu(display( + "One of service account path or service account key may be provided." + ))] + ServiceAccountPathAndKeyProvided, #[snafu(display("GCP credential error: {}", source))] Credential { source: credential::Error }, @@ -800,14 +805,15 @@ pub struct GoogleCloudStorageBuilder { bucket_name: Option<String>, url: Option<String>, service_account_path: Option<String>, + service_account_key: Option<String>, retry_config: RetryConfig, client_options: ClientOptions, } /// Configuration keys for [`GoogleCloudStorageBuilder`] /// -/// Configuration via keys can be dome via the [`try_with_option`](GoogleCloudStorageBuilder::try_with_option) -/// or [`with_options`](GoogleCloudStorageBuilder::try_with_options) methods on the builder. +/// Configuration via keys can be done via the [`try_with_option`](GoogleCloudStorageBuilder::try_with_option) +/// or [`try_with_options`](GoogleCloudStorageBuilder::try_with_options) methods on the builder. /// /// # Example /// ``` @@ -835,8 +841,17 @@ pub enum GoogleConfigKey { /// Supported keys: /// - `google_service_account` /// - `service_account` + /// - `google_service_account_path` + /// - `service_account_path` ServiceAccount, + /// The serialized service account key. + /// + /// Supported keys: + /// - `google_service_account_key` + /// - `service_account_key` + ServiceAccountKey, + /// Bucket name /// /// See [`GoogleCloudStorageBuilder::with_bucket_name`] for details. @@ -853,6 +868,7 @@ impl AsRef<str> for GoogleConfigKey { fn as_ref(&self) -> &str { match self { Self::ServiceAccount => "google_service_account", + Self::ServiceAccountKey => "google_service_account_key", Self::Bucket => "google_bucket", } } @@ -863,7 +879,13 @@ impl FromStr for GoogleConfigKey { fn from_str(s: &str) -> Result<Self, Self::Err> { match s { - "google_service_account" | "service_account" => Ok(Self::ServiceAccount), + "google_service_account" + | "service_account" + | "google_service_account_path" + | "service_account_path" => Ok(Self::ServiceAccount), + "google_service_account_key" | "service_account_key" => { + Ok(Self::ServiceAccountKey) + } "google_bucket" | "google_bucket_name" | "bucket" | "bucket_name" => { Ok(Self::Bucket) } @@ -877,6 +899,7 @@ impl Default for GoogleCloudStorageBuilder { Self { bucket_name: None, service_account_path: None, + service_account_key: None, retry_config: Default::default(), client_options: ClientOptions::new().with_allow_http(true), url: None, @@ -894,13 +917,17 @@ impl GoogleCloudStorageBuilder { /// /// Variables extracted from environment: /// * GOOGLE_SERVICE_ACCOUNT: location of service account file + /// * GOOGLE_SERVICE_ACCOUNT_PATH: (alias) location of service account file /// * SERVICE_ACCOUNT: (alias) location of service account file + /// * GOOGLE_SERVICE_ACCOUNT_KEY: JSON serialized service account key + /// * GOOGLE_BUCKET: bucket name + /// * GOOGLE_BUCKET_NAME: (alias) bucket name /// /// # Example /// ``` /// use object_store::gcp::GoogleCloudStorageBuilder; /// - /// let azure = GoogleCloudStorageBuilder::from_env() + /// let gcs = GoogleCloudStorageBuilder::from_env() /// .with_bucket_name("foo") /// .build(); /// ``` @@ -957,6 +984,9 @@ impl GoogleCloudStorageBuilder { GoogleConfigKey::ServiceAccount => { self.service_account_path = Some(value.into()) } + GoogleConfigKey::ServiceAccountKey => { + self.service_account_key = Some(value.into()) + } GoogleConfigKey::Bucket => self.bucket_name = Some(value.into()), }; Ok(self) @@ -1001,8 +1031,12 @@ impl GoogleCloudStorageBuilder { self } - /// Set the path to the service account file (required). Example - /// `"/tmp/gcs.json"` + /// Set the path to the service account file. + /// + /// This or [`GoogleCloudStorageBuilder::with_service_account_key`] must be + /// set. + /// + /// Example `"/tmp/gcs.json"`. /// /// Example contents of `gcs.json`: /// @@ -1022,6 +1056,19 @@ impl GoogleCloudStorageBuilder { self } + /// Set the service account key. The service account must be in the JSON + /// format. + /// + /// This or [`GoogleCloudStorageBuilder::with_service_account_path`] must be + /// set. + pub fn with_service_account_key( + mut self, + service_account: impl Into<String>, + ) -> Self { + self.service_account_key = Some(service_account.into()); + self + } + /// Set the retry configuration pub fn with_retry(mut self, retry_config: RetryConfig) -> Self { self.retry_config = retry_config; @@ -1048,12 +1095,19 @@ impl GoogleCloudStorageBuilder { } let bucket_name = self.bucket_name.ok_or(Error::MissingBucketName {})?; - let service_account_path = self - .service_account_path - .ok_or(Error::MissingServiceAccountPath)?; let client = self.client_options.client()?; - let credentials = reader_credentials_file(service_account_path)?; + + 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()) + } + }; // TODO: https://cloud.google.com/storage/docs/authentication#oauth-scopes let scope = "https://www.googleapis.com/auth/devstorage.full_control"; @@ -1110,6 +1164,8 @@ mod test { use bytes::Bytes; use std::collections::HashMap; use std::env; + use std::io::Write; + use tempfile::NamedTempFile; use crate::{ tests::{ @@ -1121,6 +1177,7 @@ mod test { use super::*; + const FAKE_KEY: &str = r#"{"private_key": "private_key", "client_email":"client_email", "disable_oauth":true}"#; const NON_EXISTENT_NAME: &str = "nonexistentname"; // Helper macro to skip tests if TEST_INTEGRATION and the GCP environment variables are not set. @@ -1278,11 +1335,8 @@ mod test { #[tokio::test] async fn gcs_test_proxy_url() { - use std::io::Write; - use tempfile::NamedTempFile; let mut tfile = NamedTempFile::new().unwrap(); - let creds = r#"{"private_key": "private_key", "client_email":"client_email", "disable_oauth":true}"#; - write!(tfile, "{}", creds).unwrap(); + write!(tfile, "{}", FAKE_KEY).unwrap(); let service_account_path = tfile.path(); let gcs = GoogleCloudStorageBuilder::new() .with_service_account_path(service_account_path.to_str().unwrap()) @@ -1318,6 +1372,27 @@ mod test { } } + #[test] + fn gcs_test_service_account_key_only() { + let _ = GoogleCloudStorageBuilder::new() + .with_service_account_key(FAKE_KEY) + .with_bucket_name("foo") + .build() + .unwrap(); + } + + #[test] + fn gcs_test_service_account_key_and_path() { + let mut tfile = NamedTempFile::new().unwrap(); + write!(tfile, "{}", FAKE_KEY).unwrap(); + let _ = GoogleCloudStorageBuilder::new() + .with_service_account_key(FAKE_KEY) + .with_service_account_path(tfile.path().to_str().unwrap()) + .with_bucket_name("foo") + .build() + .unwrap_err(); + } + #[test] fn gcs_test_config_from_map() { let google_service_account = "object_store:fake_service_account".to_string(); @@ -1371,4 +1446,41 @@ mod test { let builder = GoogleCloudStorageBuilder::new().try_with_options(&options); assert!(builder.is_err()); } + + #[test] + fn gcs_test_config_aliases() { + // Service account path + for alias in [ + "google_service_account", + "service_account", + "google_service_account_path", + "service_account_path", + ] { + let builder = GoogleCloudStorageBuilder::new() + .try_with_options([(alias, "/fake/path.json")]) + .unwrap(); + assert_eq!("/fake/path.json", builder.service_account_path.unwrap()); + } + + // Service account key + for alias in ["google_service_account_key", "service_account_key"] { + let builder = GoogleCloudStorageBuilder::new() + .try_with_options([(alias, FAKE_KEY)]) + .unwrap(); + assert_eq!(FAKE_KEY, builder.service_account_key.unwrap()); + } + + // Bucket name + for alias in [ + "google_bucket", + "google_bucket_name", + "bucket", + "bucket_name", + ] { + let builder = GoogleCloudStorageBuilder::new() + .try_with_options([(alias, "fake_bucket")]) + .unwrap(); + assert_eq!("fake_bucket", builder.bucket_name.unwrap()); + } + } }