fix(services/s3): remove default region us-east-1 for non-aws s3 (#…

…2812) * fix(services/s3): remove default region `us-east-1` for non-aws s3 * style: fix code style * ci(services/s3): add region env for minio * chore(services/s3): add helpful error message
apache · Aug 8, 2023 · 779088f · 779088f
1 parent 665d2c1
commit 779088f
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 14 deletions.
diff --git a/.github/workflows/fuzz_test.yml b/.github/workflows/fuzz_test.yml
@@ -108,6 +108,7 @@ jobs:
           OPENDAL_S3_ENDPOINT: "http://127.0.0.1:9000"
           OPENDAL_S3_ACCESS_KEY_ID: minioadmin
           OPENDAL_S3_SECRET_ACCESS_KEY: minioadmin
+          OPENDAL_S3_REGION: us-east-1
 
   fuzz-test-fs:
     runs-on: ubuntu-latest

diff --git a/.github/workflows/service_test_s3.yml b/.github/workflows/service_test_s3.yml
@@ -142,6 +142,7 @@ jobs:
           OPENDAL_S3_ENDPOINT: "http://127.0.0.1:9000"
           OPENDAL_S3_ACCESS_KEY_ID: minioadmin
           OPENDAL_S3_SECRET_ACCESS_KEY: minioadmin
+          OPENDAL_S3_REGION: us-east-1
 
   anonymous_minio_s3:
     runs-on: ubuntu-latest
@@ -178,6 +179,7 @@ jobs:
           OPENDAL_S3_BUCKET: test
           OPENDAL_S3_ENDPOINT: "http://127.0.0.1:9000"
           OPENDAL_S3_ALLOW_ANONYMOUS: on
+          OPENDAL_S3_REGION: us-east-1
 
   r2:
     runs-on: ubuntu-latest

diff --git a/core/src/services/s3/backend.rs b/core/src/services/s3/backend.rs
@@ -159,7 +159,7 @@ impl S3Builder {
     ///
     /// If using a custom endpoint,
     /// - If region is set, we will take user's input first.
-    /// - If not, the default `us-east-1` will be used.
+    /// - If not, we will try to load it from environment.
     pub fn region(&mut self, region: &str) -> &mut Self {
         if !region.is_empty() {
             self.region = Some(region.to_string())
@@ -772,19 +772,12 @@ impl Builder for S3Builder {
             cfg.region = Some(v);
         }
         if cfg.region.is_none() {
-            // AWS S3 requires region to be set.
-            if self.endpoint.is_none()
-                || self.endpoint.as_deref() == Some("https://s3.amazonaws.com")
-            {
-                return Err(Error::new(ErrorKind::ConfigInvalid, "region is missing")
-                    .with_operation("Builder::build")
-                    .with_context("service", Scheme::S3));
-            }
-
-            // For other compatible services, if we don't know region
-            // after loading from builder and env, we can use `us-east-1`
-            // as default.
-            cfg.region = Some("us-east-1".to_string());
+            return Err(Error::new(
+                ErrorKind::ConfigInvalid,
+                "region is missing. Please find it by S3::detect_region() or set them in env.",
+            )
+            .with_operation("Builder::build")
+            .with_context("service", Scheme::S3));
         }
 
         let region = cfg.region.to_owned().unwrap();