diff --git a/cloud/aws-common/pom.xml b/cloud/aws-common/pom.xml index 4532eca39e8c..64a37d4a5272 100644 --- a/cloud/aws-common/pom.xml +++ b/cloud/aws-common/pom.xml @@ -93,5 +93,20 @@ junit test + + org.junit.vintage + junit-vintage-engine + test + + + org.junit.jupiter + junit-jupiter-engine + test + + + org.junit.jupiter + junit-jupiter-api + test + diff --git a/cloud/aws-common/src/main/java/org/apache/druid/common/aws/AWSClientConfig.java b/cloud/aws-common/src/main/java/org/apache/druid/common/aws/AWSClientConfig.java index 9f5e2189f199..e8d299cbd851 100644 --- a/cloud/aws-common/src/main/java/org/apache/druid/common/aws/AWSClientConfig.java +++ b/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 + diff --git a/cloud/aws-common/src/test/java/org/apache/druid/common/aws/AWSClientConfigTest.java b/cloud/aws-common/src/test/java/org/apache/druid/common/aws/AWSClientConfigTest.java new file mode 100644 index 000000000000..4e8837566ca7 --- /dev/null +++ b/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()); + } +} diff --git a/docs/development/extensions-contrib/iceberg.md b/docs/development/extensions-contrib/iceberg.md index 6bee8b2e6f31..f3eed056422b 100644 --- a/docs/development/extensions-contrib/iceberg.md +++ b/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": { diff --git a/docs/development/extensions-core/s3.md b/docs/development/extensions-core/s3.md index ae8f0a9f5644..6ad71adc74d7 100644 --- a/docs/development/extensions-core/s3.md +++ b/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| diff --git a/docs/ingestion/input-sources.md b/docs/ingestion/input-sources.md index cf6aecfe8c57..a74e61db5682 100644 --- a/docs/ingestion/input-sources.md +++ b/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", diff --git a/extensions-core/s3-extensions/src/main/java/org/apache/druid/data/input/s3/S3InputSource.java b/extensions-core/s3-extensions/src/main/java/org/apache/druid/data/input/s3/S3InputSource.java index 7660cadcf1a9..9b85c5872972 100644 --- a/extensions-core/s3-extensions/src/main/java/org/apache/druid/data/input/s3/S3InputSource.java +++ b/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 diff --git a/extensions-core/s3-extensions/src/main/java/org/apache/druid/storage/s3/S3StorageDruidModule.java b/extensions-core/s3-extensions/src/main/java/org/apache/druid/storage/s3/S3StorageDruidModule.java index 986731c532a8..54d275f7818c 100644 --- a/extensions-core/s3-extensions/src/main/java/org/apache/druid/storage/s3/S3StorageDruidModule.java +++ b/extensions-core/s3-extensions/src/main/java/org/apache/druid/storage/s3/S3StorageDruidModule.java @@ -138,6 +138,16 @@ public ServerSideEncryptingAmazonS3.Builder getServerSideEncryptingAmazonS3Build 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 @@ public ServerSideEncryptingAmazonS3.Builder getServerSideEncryptingAmazonS3Build .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 @@ public ServerSideEncryptingAmazonS3.Builder getServerSideEncryptingAmazonS3Build .credentialsProvider(provider) .httpClientBuilder(asyncHttpClientBuilder) .forcePathStyle(clientConfig.isEnablePathStyleAccess()) + .crossRegionAccessEnabled(clientConfig.isCrossRegionAccessEnabled()) .multipartEnabled(true); if (endpointOverride != null) { diff --git a/extensions-core/s3-extensions/src/test/java/org/apache/druid/data/input/s3/S3InputSourceTest.java b/extensions-core/s3-extensions/src/test/java/org/apache/druid/data/input/s3/S3InputSourceTest.java index 16cbd1e7bc1a..22c2c767355a 100644 --- a/extensions-core/s3-extensions/src/test/java/org/apache/druid/data/input/s3/S3InputSourceTest.java +++ b/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("");