Fix loss loss of cross region bucket read config for s3 client in the v2 migration

cloud/aws-common/pom.xml

@@ -93,5 +93,20 @@
             <artifactId>junit</artifactId>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>org.junit.vintage</groupId>
+            <artifactId>junit-vintage-engine</artifactId>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.junit.jupiter</groupId>
+            <artifactId>junit-jupiter-engine</artifactId>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.junit.jupiter</groupId>
+            <artifactId>junit-jupiter-api</artifactId>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
 </project>

cloud/aws-common/src/main/java/org/apache/druid/common/aws/AWSClientConfig.java

@@ -21,12 +21,14 @@
 
 import com.fasterxml.jackson.annotation.JsonProperty;
 
+import javax.annotation.Nullable;
+
 public class AWSClientConfig
 {
   // Default values matching AWS SDK v2 defaults
   private static final boolean DEFAULT_CHUNKED_ENCODING_DISABLED = false;
   private static final boolean DEFAULT_PATH_STYLE_ACCESS = false;
-  private static final boolean DEFAULT_FORCE_GLOBAL_BUCKET_ACCESS_ENABLED = false;
+
   private static final int DEFAULT_CONNECTION_TIMEOUT_MILLIS = 10_000;
   private static final int DEFAULT_SOCKET_TIMEOUT_MILLIS = 50_000;
   private static final int DEFAULT_MAX_CONNECTIONS = 50;
@@ -40,8 +42,17 @@ public class AWSClientConfig
   @JsonProperty
   private boolean enablePathStyleAccess = DEFAULT_PATH_STYLE_ACCESS;
 
+  /**
+   * @deprecated Use {@link #crossRegionAccessEnabled} instead.
+   */
+  @Deprecated
   @JsonProperty
-  protected boolean forceGlobalBucketAccessEnabled = DEFAULT_FORCE_GLOBAL_BUCKET_ACCESS_ENABLED;
+  @Nullable
+  protected Boolean forceGlobalBucketAccessEnabled;
+
+  @JsonProperty
+  @Nullable
+  private Boolean crossRegionAccessEnabled;
 
   @JsonProperty
   private int connectionTimeout = DEFAULT_CONNECTION_TIMEOUT_MILLIS;
@@ -67,11 +78,39 @@ public boolean isEnablePathStyleAccess()
     return enablePathStyleAccess;
   }
 
-  public boolean isForceGlobalBucketAccessEnabled()
+  /**
+   * @deprecated Use {@link #isCrossRegionAccessEnabled()} instead.
+   */
+  @Deprecated
+  @Nullable
+  public Boolean isForceGlobalBucketAccessEnabled()
   {
     return forceGlobalBucketAccessEnabled;
   }
 
+  @Nullable
+  public Boolean getCrossRegionAccessEnabled()
+  {
+    return crossRegionAccessEnabled;
+  }
+
+  /**
+   * Resolves cross-region access setting. Precedence:
+   * 1. If crossRegionAccessEnabled is explicitly set, use it.
+   * 2. If forceGlobalBucketAccessEnabled (deprecated) is explicitly set, use it.
+   * 3. Otherwise, default to false.
+   */
+  public boolean isCrossRegionAccessEnabled()
+  {
+    if (crossRegionAccessEnabled != null) {
+      return crossRegionAccessEnabled;
+    }
+    if (forceGlobalBucketAccessEnabled != null) {
+      return forceGlobalBucketAccessEnabled;
+    }
+    return false;
+  }
+
   public int getConnectionTimeoutMillis()
   {
     return connectionTimeout;
@@ -94,7 +133,7 @@ public String toString()
            "protocol='" + protocol + '\'' +
            ", disableChunkedEncoding=" + disableChunkedEncoding +
            ", enablePathStyleAccess=" + enablePathStyleAccess +
-           ", forceGlobalBucketAccessEnabled=" + forceGlobalBucketAccessEnabled +
+           ", crossRegionAccessEnabled=" + isCrossRegionAccessEnabled() +
            ", connectionTimeout=" + connectionTimeout +
            ", socketTimeout=" + socketTimeout +
            ", maxConnections=" + maxConnections +

cloud/aws-common/src/test/java/org/apache/druid/common/aws/AWSClientConfigTest.java

@@ -0,0 +1,86 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.common.aws;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+public class AWSClientConfigTest
+{
+  private static final ObjectMapper MAPPER = new ObjectMapper();
+
+  @Test
+  public void testDefaultCrossRegionAccessEnabled() throws Exception
+  {
+    AWSClientConfig config = MAPPER.readValue("{}", AWSClientConfig.class);
+    Assertions.assertNull(config.isForceGlobalBucketAccessEnabled());
+    Assertions.assertFalse(config.isCrossRegionAccessEnabled());
+  }
+
+  @Test
+  public void testCrossRegionAccessEnabledExplicitlySet() throws Exception
+  {
+    AWSClientConfig config = MAPPER.readValue("{\"crossRegionAccessEnabled\": true}", AWSClientConfig.class);
+    Assertions.assertNull(config.isForceGlobalBucketAccessEnabled());
+    Assertions.assertTrue(config.isCrossRegionAccessEnabled());
+  }
+
+  @Test
+  public void testNewConfigTakesPrecedenceOverDeprecatedWhenBothSet() throws Exception
+  {
+    AWSClientConfig config = MAPPER.readValue(
+        "{\"forceGlobalBucketAccessEnabled\": true, \"crossRegionAccessEnabled\": false}",
+        AWSClientConfig.class
+    );
+    Assertions.assertFalse(config.isCrossRegionAccessEnabled());
+  }
+
+  @Test
+  public void testNewConfigTrueWinsOverDeprecatedFalse() throws Exception
+  {
+    AWSClientConfig config = MAPPER.readValue(
+        "{\"forceGlobalBucketAccessEnabled\": false, \"crossRegionAccessEnabled\": true}",
+        AWSClientConfig.class
+    );
+    Assertions.assertTrue(config.isCrossRegionAccessEnabled());
+  }
+
+  @Test
+  public void testDeprecatedForceGlobalBucketAccessAloneTrue() throws Exception
+  {
+    AWSClientConfig config = MAPPER.readValue(
+        "{\"forceGlobalBucketAccessEnabled\": true}",
+        AWSClientConfig.class
+    );
+    Assertions.assertTrue(config.isCrossRegionAccessEnabled());
+  }
+
+  @Test
+  public void testDeprecatedNotSetFallsThroughToCrossRegion() throws Exception
+  {
+    AWSClientConfig config = MAPPER.readValue(
+        "{\"crossRegionAccessEnabled\": true}",
+        AWSClientConfig.class
+    );
+    Assertions.assertNull(config.isForceGlobalBucketAccessEnabled());
+    Assertions.assertTrue(config.isCrossRegionAccessEnabled());
+  }
+}

docs/development/extensions-contrib/iceberg.md

@@ -79,7 +79,7 @@ Set the `type` property of the `warehouseSource` object to `s3` in the ingestion
     "protocol": "http",
     "disableChunkedEncoding": true,
     "enablePathStyleAccess": true,
-    "forceGlobalBucketAccessEnabled": false
+    "crossRegionAccessEnabled": false
   },
   "properties": {
     "accessKeyId": {

docs/development/extensions-core/s3.md

@@ -129,7 +129,8 @@ For example, to set the region to 'us-east-1' through system properties:
 |`druid.s3.protocol`|Communication protocol type to use when sending requests to AWS. `http` or `https` can be used. This configuration would be ignored if `druid.s3.endpoint.url` is filled with a URL with a different protocol.|`https`|
 |`druid.s3.disableChunkedEncoding`|Disables chunked encoding. See [AWS document](https://docs.aws.amazon.com/AWSJavaSDK/latest/javadoc/com/amazonaws/services/s3/AmazonS3Builder.html#disableChunkedEncoding--) for details.|false|
 |`druid.s3.enablePathStyleAccess`|Enables path style access. See [AWS document](https://docs.aws.amazon.com/AWSJavaSDK/latest/javadoc/com/amazonaws/services/s3/AmazonS3Builder.html#enablePathStyleAccess--) for details.|false|
-|`druid.s3.forceGlobalBucketAccessEnabled`|Enables global bucket access. See [AWS document](https://docs.aws.amazon.com/AWSJavaSDK/latest/javadoc/com/amazonaws/services/s3/AmazonS3Builder.html#setForceGlobalBucketAccessEnabled-java.lang.Boolean-) for details.|false|
+|`druid.s3.crossRegionAccessEnabled`|Enables cross-region access for S3 requests. When enabled, the S3 client automatically detects the correct region for a bucket on first access and caches it for subsequent requests.|false|
+|`druid.s3.forceGlobalBucketAccessEnabled`|**Deprecated.** Use `druid.s3.crossRegionAccessEnabled` instead. Only used as a fallback if `crossRegionAccessEnabled` is not explicitly set.|null|
 |`druid.s3.endpoint.url`|Service endpoint either with or without the protocol.|None|
 |`druid.s3.endpoint.signingRegion`|Region to use for SigV4 signing of requests (e.g. us-west-1).|None|
 |`druid.s3.proxy.host`|Proxy host to connect through.|None|

docs/ingestion/input-sources.md

@@ -155,7 +155,7 @@ Sample specs:
              "protocol" : "http",
              "disableChunkedEncoding" : true,
              "enablePathStyleAccess" : true,
-             "forceGlobalBucketAccessEnabled" : false
+             "crossRegionAccessEnabled" : false
          },
          "proxyConfig": {
              "host" : "proxy-s3.aws.com",

extensions-core/s3-extensions/src/main/java/org/apache/druid/data/input/s3/S3InputSource.java

@@ -160,6 +160,7 @@ public S3InputSource(
                   .pathStyleAccessEnabled(awsClientConfig.isEnablePathStyleAccess())
                   .chunkedEncodingEnabled(!awsClientConfig.isDisableChunkedEncoding());
               customBuilder.serviceConfiguration(s3ConfigBuilder.build());
+              customBuilder.crossRegionAccessEnabled(awsClientConfig.isCrossRegionAccessEnabled());
             }
 
             // Configure HTTP client with proxy if needed

extensions-core/s3-extensions/src/main/java/org/apache/druid/storage/s3/S3StorageDruidModule.java

@@ -138,6 +138,16 @@
       S3StorageConfig storageConfig
   )
   {
+    if (clientConfig.isForceGlobalBucketAccessEnabled() != null) {
+      log.warn(
+          "Configuration 'druid.s3.client.forceGlobalBucketAccessEnabled' is deprecated and will be removed in a future release. "
+          + "Please use 'druid.s3.client.crossRegionAccessEnabled' instead.%s",
+          clientConfig.getCrossRegionAccessEnabled() != null
+              ? " Note: 'crossRegionAccessEnabled' is also set and will take precedence. Removing the legacy config will stop this warning from showing up."
+              : ""
+      );
+    }
+
     final boolean useHttps = S3Utils.useHttps(clientConfig, endpointConfig);
     final URI endpointOverride = buildEndpointOverride(endpointConfig, useHttps);
     final Region region = StringUtils.isNotEmpty(endpointConfig.getSigningRegion())
@@ -166,7 +176,8 @@
           .credentialsProvider(provider)
           .httpClientBuilder(httpClientBuilder)
           .serviceConfiguration(s3Configuration)
-          .forcePathStyle(clientConfig.isEnablePathStyleAccess());
+          .forcePathStyle(clientConfig.isEnablePathStyleAccess())
+          .crossRegionAccessEnabled(clientConfig.isCrossRegionAccessEnabled());
 
       if (endpointOverride != null) {
         s3ClientBuilder.endpointOverride(endpointOverride);
@@ -190,6 +201,7 @@
           .credentialsProvider(provider)
           .httpClientBuilder(asyncHttpClientBuilder)
           .forcePathStyle(clientConfig.isEnablePathStyleAccess())
+          .crossRegionAccessEnabled(clientConfig.isCrossRegionAccessEnabled())
           .multipartEnabled(true);
 
       if (endpointOverride != null) {

extensions-core/s3-extensions/src/test/java/org/apache/druid/data/input/s3/S3InputSourceTest.java

@@ -554,7 +554,7 @@ public void testS3InputSourceUseEndPointClientProxy()
 
     EasyMock.expect(mockAwsClientConfig.isDisableChunkedEncoding()).andStubReturn(false);
     EasyMock.expect(mockAwsClientConfig.isEnablePathStyleAccess()).andStubReturn(false);
-    EasyMock.expect(mockAwsClientConfig.isForceGlobalBucketAccessEnabled()).andStubReturn(true);
+    EasyMock.expect(mockAwsClientConfig.isCrossRegionAccessEnabled()).andStubReturn(true);
     EasyMock.expect(mockAwsClientConfig.getProtocol()).andStubReturn("http");
 
     EasyMock.expect(mockAwsProxyConfig.getHost()).andStubReturn("");