From 7d58edff4c6b947dab9495ba56ca513c5f5c0bd5 Mon Sep 17 00:00:00 2001 From: Sean Smith Date: Wed, 31 May 2023 10:58:15 -0500 Subject: [PATCH] feat: Make gcs and s3 credentials optional ``` ~/Code/github.com/glaredb/glaredb [2] % cargo run --bin glaredb -- local Finished dev [unoptimized + debuginfo] target(s) in 0.58s Running `target/debug/glaredb local` GlareDB (v0.0.11) Using in-memory catalog > create external table v from gcs options (location = 'tyrell/voight_kampff.csv', bucket='glaredb-demos'); create table > select * from v limit 10; +--------------------------------------+---------------------+--------------------+-----------------+------------------+------------+ | Test_ID | Examiner_Name | Subject_Name | Questions_Asked | Final_Assessment | Test_Date | +--------------------------------------+---------------------+--------------------+-----------------+------------------+------------+ | 273886b2-c778-4757-8779-68cf4de40154 | Angelica Wilson | Eric Jarvis | 68 | Replicant | 2020-01-01 | | b9831559-803c-40b7-bf12-b21417ed6bcd | Robert Hall | Margaret Smith | 71 | Human | 2020-01-02 | | 38de6ce6-4012-4e0f-99b1-a2f4757fe8b9 | Victoria Sims | Mrs. Sheila Hall | 83 | Human | 2020-01-03 | | 2c36ad83-7049-4cfc-ba51-af96f1c2eebe | Shannon Horton | Andrew Wiggins | 82 | Human | 2020-01-04 | | 1b8827c1-cd9b-4fa6-8fc0-e7b402acc184 | Sheila Torres | Mark Tate | 84 | Human | 2020-01-05 | | 6cd8ee64-5ff2-475b-b276-c2bda44f6330 | Nicole Taylor | Justin Fleming | 47 | Replicant | 2020-01-06 | | 5b04797c-e861-41eb-a76d-12e7c1d3e6c0 | Patrick Harrison MD | Laura Hicks | 22 | Human | 2020-01-07 | | e477d5e3-f886-4d5c-8ba4-b403513450fc | Edward Salinas | Brittany Hendricks | 101 | Replicant | 2020-01-08 | | 0be01ad5-9359-48e6-85a7-38ca0abf9276 | Michael Nelson | Betty Lawson | 40 | Replicant | 2020-01-09 | | c4c59f2d-c94a-4f3b-bc0e-9a727f889bec | Susan Chandler | Debbie Weeks | 45 | Replicant | 2020-01-10 | +--------------------------------------+---------------------+--------------------+-----------------+------------------+------------+ ``` --- crates/datasources/src/object_store/errors.rs | 3 ++ crates/datasources/src/object_store/gcs.rs | 27 ++++++------ crates/datasources/src/object_store/s3.rs | 41 ++++++++++--------- crates/metastore/proto/options.proto | 6 +-- crates/metastore/src/types/options.rs | 6 +-- crates/sqlexec/src/planner/session_planner.rs | 6 +-- 6 files changed, 47 insertions(+), 42 deletions(-) diff --git a/crates/datasources/src/object_store/errors.rs b/crates/datasources/src/object_store/errors.rs index 4e9354299..60b0b245e 100644 --- a/crates/datasources/src/object_store/errors.rs +++ b/crates/datasources/src/object_store/errors.rs @@ -22,6 +22,9 @@ pub enum ObjectStoreSourceError { #[error("This file type is not supported: {0}")] NotSupportFileType(String), + + #[error("{0}")] + Static(&'static str), } pub type Result = std::result::Result; diff --git a/crates/datasources/src/object_store/gcs.rs b/crates/datasources/src/object_store/gcs.rs index 7fd7b4a10..a1556646b 100644 --- a/crates/datasources/src/object_store/gcs.rs +++ b/crates/datasources/src/object_store/gcs.rs @@ -19,12 +19,22 @@ pub struct GcsTableAccess { /// GCS object store bucket name pub bucket_name: String, /// GCS object store service account key - pub service_acccount_key_json: String, + pub service_acccount_key_json: Option, /// GCS object store table location pub location: String, pub file_type: Option, } +impl GcsTableAccess { + fn builder(&self) -> GoogleCloudStorageBuilder { + let builder = GoogleCloudStorageBuilder::new().with_bucket_name(&self.bucket_name); + match &self.service_acccount_key_json { + Some(key) => builder.with_service_account_key(key), + None => builder, + } + } +} + #[derive(Debug)] pub struct GcsAccessor { /// GCS object store access info @@ -47,12 +57,7 @@ impl TableAccessor for GcsAccessor { impl GcsAccessor { /// Setup accessor for GCS pub async fn new(access: GcsTableAccess) -> Result { - let store = Arc::new( - GoogleCloudStorageBuilder::new() - .with_service_account_key(access.service_acccount_key_json) - .with_bucket_name(access.bucket_name) - .build()?, - ); + let store = Arc::new(access.builder().build()?); let location = ObjectStorePath::from(access.location); // Use provided file type or infer from location @@ -69,13 +74,7 @@ impl GcsAccessor { } pub async fn validate_table_access(access: GcsTableAccess) -> Result<()> { - let store = Arc::new( - GoogleCloudStorageBuilder::new() - .with_service_account_key(access.service_acccount_key_json) - .with_bucket_name(access.bucket_name) - .build()?, - ); - + let store = Arc::new(access.builder().build()?); let location = ObjectStorePath::from(access.location); store.head(&location).await?; Ok(()) diff --git a/crates/datasources/src/object_store/s3.rs b/crates/datasources/src/object_store/s3.rs index d7ac7f1e2..6760d331d 100644 --- a/crates/datasources/src/object_store/s3.rs +++ b/crates/datasources/src/object_store/s3.rs @@ -8,7 +8,7 @@ use serde::{Deserialize, Serialize}; use tracing::trace; use super::csv::CsvTableProvider; -use super::errors::Result; +use super::errors::{ObjectStoreSourceError, Result}; use super::parquet::ParquetTableProvider; use super::{file_type_from_path, FileType, TableAccessor}; @@ -20,14 +20,31 @@ pub struct S3TableAccess { /// S3 object store bucket name pub bucket_name: String, /// S3 object store access key id - pub access_key_id: String, + pub access_key_id: Option, /// S3 object store secret access key - pub secret_access_key: String, + pub secret_access_key: Option, /// S3 object store table location pub location: String, pub file_type: Option, } +impl S3TableAccess { + fn builder(&self) -> Result { + let builder = AmazonS3Builder::new() + .with_region(&self.region) + .with_bucket_name(&self.bucket_name); + match (&self.access_key_id, &self.secret_access_key) { + (Some(id), Some(secret)) => Ok(builder + .with_access_key_id(id) + .with_secret_access_key(secret)), + (None, None) => Ok(builder), + _ => Err(ObjectStoreSourceError::Static( + "Access key id and secret must both be provided", + )), + } + } +} + #[derive(Debug)] pub struct S3Accessor { /// S3 object store access info @@ -50,14 +67,7 @@ impl TableAccessor for S3Accessor { impl S3Accessor { /// Setup accessor for S3 pub async fn new(access: S3TableAccess) -> Result { - let store = Arc::new( - AmazonS3Builder::new() - .with_region(access.region) - .with_bucket_name(access.bucket_name) - .with_access_key_id(access.access_key_id) - .with_secret_access_key(access.secret_access_key) - .build()?, - ); + let store = Arc::new(access.builder()?.build()?); let location = ObjectStorePath::from(access.location); // Use provided file type or infer from location @@ -74,14 +84,7 @@ impl S3Accessor { } pub async fn validate_table_access(access: S3TableAccess) -> Result<()> { - let store = Arc::new( - AmazonS3Builder::new() - .with_region(access.region) - .with_bucket_name(access.bucket_name) - .with_access_key_id(access.access_key_id) - .with_secret_access_key(access.secret_access_key) - .build()?, - ); + let store = Arc::new(access.builder()?.build()?); let location = ObjectStorePath::from(access.location); store.head(&location).await?; diff --git a/crates/metastore/proto/options.proto b/crates/metastore/proto/options.proto index 6a2b88f31..d3cb0557d 100644 --- a/crates/metastore/proto/options.proto +++ b/crates/metastore/proto/options.proto @@ -117,14 +117,14 @@ message TableOptionsMysql { message TableOptionsLocal { string location = 1; } message TableOptionsGcs { - string service_account_key = 1; + optional string service_account_key = 1; string bucket = 2; string location = 3; } message TableOptionsS3 { - string access_key_id = 1; - string secret_access_key = 2; + optional string access_key_id = 1; + optional string secret_access_key = 2; string region = 3; string bucket = 4; string location = 5; diff --git a/crates/metastore/src/types/options.rs b/crates/metastore/src/types/options.rs index 05052548d..37ee64af2 100644 --- a/crates/metastore/src/types/options.rs +++ b/crates/metastore/src/types/options.rs @@ -585,7 +585,7 @@ impl From for options::TableOptionsLocal { #[derive(Debug, Clone, Arbitrary, PartialEq, Eq)] pub struct TableOptionsGcs { - pub service_account_key: String, + pub service_account_key: Option, pub bucket: String, pub location: String, } @@ -613,8 +613,8 @@ impl From for options::TableOptionsGcs { #[derive(Debug, Clone, Arbitrary, PartialEq, Eq)] pub struct TableOptionsS3 { - pub access_key_id: String, - pub secret_access_key: String, + pub access_key_id: Option, + pub secret_access_key: Option, pub region: String, pub bucket: String, pub location: String, diff --git a/crates/sqlexec/src/planner/session_planner.rs b/crates/sqlexec/src/planner/session_planner.rs index 3539bb2b0..3f1c4a807 100644 --- a/crates/sqlexec/src/planner/session_planner.rs +++ b/crates/sqlexec/src/planner/session_planner.rs @@ -344,7 +344,7 @@ impl<'a> SessionPlanner<'a> { TableOptions::Local(TableOptionsLocal { location }) } TableOptions::GCS => { - let service_account_key = remove_required_opt(m, "service_account_key")?; + let service_account_key = remove_optional_opt(m, "service_account_key")?; let bucket = remove_required_opt(m, "bucket")?; let location = remove_required_opt(m, "location")?; @@ -368,8 +368,8 @@ impl<'a> SessionPlanner<'a> { }) } TableOptions::S3_STORAGE => { - let access_key_id = remove_required_opt(m, "access_key_id")?; - let secret_access_key = remove_required_opt(m, "secret_access_key")?; + let access_key_id = remove_optional_opt(m, "access_key_id")?; + let secret_access_key = remove_optional_opt(m, "secret_access_key")?; let region = remove_required_opt(m, "region")?; let bucket = remove_required_opt(m, "bucket")?; let location = remove_required_opt(m, "location")?;