Making region optional (#1658)

Author: diptanu

server/src/blob_store/mod.rs

@@ -28,7 +28,7 @@ pub mod registry;
 #[derive(Debug, Clone, Serialize, Deserialize)]
 pub struct BlobStorageConfig {
     pub path: String,
-    pub region: String,
+    pub region: Option<String>,
 }
 
 impl Default for BlobStorageConfig {
@@ -43,7 +43,7 @@ impl Default for BlobStorageConfig {
         );
         BlobStorageConfig {
             path: blob_store_path,
-            region: "".to_string(),
+            region: None,
         }
     }
 }
@@ -67,7 +67,7 @@ impl BlobStorage {
     pub fn new(config: BlobStorageConfig) -> Result<Self> {
         let url = &config.path.clone();
         debug!("using blob store path: {}", url);
-        let (object_store, path) = Self::build_object_store(url, &config.region)?;
+        let (object_store, path) = Self::build_object_store(url, config.region.clone())?;
         Ok(Self {
             object_store: Arc::new(object_store),
             url_scheme: url.parse::<Url>()?.scheme().to_string(),
@@ -77,22 +77,28 @@ impl BlobStorage {
         })
     }
 
-    pub fn build_object_store(url_str: &str, region: &str) -> Result<(Box<dyn ObjectStore>, Path)> {
+    pub fn build_object_store(
+        url_str: &str,
+        region: Option<String>,
+    ) -> Result<(Box<dyn ObjectStore>, Path)> {
         let url = &url_str.parse::<Url>()?;
         let (scheme, _) = ObjectStoreScheme::parse(url)?;
         match scheme {
             ObjectStoreScheme::AmazonS3 => {
                 // inject AWS environment variables to prioritize keys over instance metadata
                 // credentials.
-                let s3_builder = AmazonS3Builder::from_env()
+                let mut s3_builder = AmazonS3Builder::from_env()
                     .with_url(url_str)
                     .with_allow_http(true)
-                    .with_conditional_put(S3ConditionalPut::ETagMatch)
-                    .with_region(region)
-                    .build()
-                    .expect("failed to create object store");
+                    .with_conditional_put(S3ConditionalPut::ETagMatch);
+
+                if let Some(region) = region {
+                    s3_builder = s3_builder.with_region(region);
+                }
+
+                let obj_store = s3_builder.build().expect("failed to create object store");
                 let (_, path) = parse_url(url)?;
-                Ok((Box::new(s3_builder), path))
+                Ok((Box::new(obj_store), path))
             }
             _ => Ok(parse_url(url)?),
         }

server/src/blob_store/registry.rs

@@ -13,21 +13,29 @@ pub struct BlobStorageRegistry {
 }
 
 impl BlobStorageRegistry {
-    pub fn new(default_blob_storage_path: &str, default_blob_storage_region: &str) -> Result<Self> {
+    pub fn new(
+        default_blob_storage_path: &str,
+        default_blob_storage_region: Option<String>,
+    ) -> Result<Self> {
         let default_blob_storage = BlobStorage::new(BlobStorageConfig {
             path: default_blob_storage_path.to_string(),
-            region: default_blob_storage_region.to_string(),
+            region: default_blob_storage_region,
         })?;
         Ok(Self {
             blob_storage_buckets: Mutex::new(HashMap::new()),
             default_blob_storage: Arc::new(default_blob_storage),
         })
     }
 
-    pub fn create_new_blob_store(&self, namespace: &str, path: &str, region: &str) -> Result<()> {
+    pub fn create_new_blob_store(
+        &self,
+        namespace: &str,
+        path: &str,
+        region: Option<String>,
+    ) -> Result<()> {
         let blob_storage = BlobStorage::new(BlobStorageConfig {
             path: path.to_string(),
-            region: region.to_string(),
+            region,
         })?;
         self.blob_storage_buckets
             .lock()

server/src/routes_internal.rs

@@ -312,15 +312,15 @@ async fn create_namespace(
     Json(namespace): Json<CreateNamespace>,
 ) -> Result<(), IndexifyAPIError> {
     if let Some(blob_storage_bucket) = &namespace.blob_storage_bucket {
-        let Some(blob_storage_region) = &namespace.blob_storage_region else {
+        let Some(_blob_storage_region) = &namespace.blob_storage_region else {
             return Err(IndexifyAPIError::bad_request(
                 "blob storage region is required",
             ));
         };
         if let Err(e) = state.blob_storage.create_new_blob_store(
             &namespace.name,
             blob_storage_bucket,
-            &blob_storage_region,
+            namespace.blob_storage_region.clone(),
         ) {
             error!("failed to create blob storage bucket: {:?}", e);
             return Err(IndexifyAPIError::internal_error(e));

server/src/service.rs

@@ -66,20 +66,16 @@ impl Service {
 
         let blob_storage_registry = Arc::new(BlobStorageRegistry::new(
             config.blob_storage.path.as_str(),
-            config.blob_storage.region.as_str(),
+            config.blob_storage.region.clone(),
         )?);
 
         let namespaces = indexify_state.reader().get_all_namespaces()?;
         for namespace in namespaces {
             if let Some(blob_storage_bucket) = namespace.blob_storage_bucket {
-                let blob_storage_region = namespace.blob_storage_region.unwrap_or_else(|| {
-                    std::env::var("AWS_REGION").unwrap_or_else(|_| "us-east-1".to_string())
-                });
-
                 blob_storage_registry.create_new_blob_store(
                     &namespace.name,
                     &blob_storage_bucket,
-                    &blob_storage_region,
+                    namespace.blob_storage_region.clone(),
                 )?;
             }
         }

server/src/state_store/kv.rs

@@ -91,7 +91,7 @@ mod tests {
                     "file://{}",
                     temp_dir.path().join("blob_store").to_str().unwrap()
                 ),
-                region: "local".to_string(),
+                region: None,
             })?),
             "test",
         )

server/src/testing.rs

@@ -66,7 +66,7 @@ impl TestService {
                     "file://{}",
                     temp_dir.path().join("blob_store").to_str().unwrap()
                 ),
-                region: "local".to_string(),
+                region: None,
             },
             ..Default::default()
         };