From f7d4ebc3ea0e2ac240a2ed453e24edf5005fd9a6 Mon Sep 17 00:00:00 2001 From: Vivek Dhiman Date: Tue, 9 Jun 2026 16:38:04 +0530 Subject: [PATCH 1/2] Handle MRAP URI Signed-off-by: Vivek Dhiman --- .../storage/s3/S3DataSegmentPusherConfig.java | 2 +- .../apache/druid/storage/s3/S3LoadSpec.java | 2 +- .../org/apache/druid/storage/s3/S3Utils.java | 22 ++++++++++ .../apache/druid/storage/s3/S3UtilsTest.java | 28 +++++++++++++ .../data/input/impl/CloudObjectLocation.java | 13 +++++- .../input/impl/CloudObjectLocationTest.java | 40 +++++++++++++++++++ 6 files changed, 103 insertions(+), 4 deletions(-) diff --git a/extensions-core/s3-extensions/src/main/java/org/apache/druid/storage/s3/S3DataSegmentPusherConfig.java b/extensions-core/s3-extensions/src/main/java/org/apache/druid/storage/s3/S3DataSegmentPusherConfig.java index 2a4b7babec04..dcb46c6be210 100644 --- a/extensions-core/s3-extensions/src/main/java/org/apache/druid/storage/s3/S3DataSegmentPusherConfig.java +++ b/extensions-core/s3-extensions/src/main/java/org/apache/druid/storage/s3/S3DataSegmentPusherConfig.java @@ -45,7 +45,7 @@ public class S3DataSegmentPusherConfig public void setBucket(String bucket) { - this.bucket = bucket; + this.bucket = S3Utils.normalizeBucketName(bucket); } public void setBaseKey(String baseKey) diff --git a/extensions-core/s3-extensions/src/main/java/org/apache/druid/storage/s3/S3LoadSpec.java b/extensions-core/s3-extensions/src/main/java/org/apache/druid/storage/s3/S3LoadSpec.java index ec8fca91374a..e1a6fb919019 100644 --- a/extensions-core/s3-extensions/src/main/java/org/apache/druid/storage/s3/S3LoadSpec.java +++ b/extensions-core/s3-extensions/src/main/java/org/apache/druid/storage/s3/S3LoadSpec.java @@ -64,7 +64,7 @@ public S3LoadSpec( { Preconditions.checkNotNull(bucket); Preconditions.checkNotNull(key); - this.bucket = bucket; + this.bucket = S3Utils.normalizeBucketName(bucket); this.key = key; this.rangeable = rangeable; this.puller = puller; diff --git a/extensions-core/s3-extensions/src/main/java/org/apache/druid/storage/s3/S3Utils.java b/extensions-core/s3-extensions/src/main/java/org/apache/druid/storage/s3/S3Utils.java index 82412fe412c9..506441cc323b 100644 --- a/extensions-core/s3-extensions/src/main/java/org/apache/druid/storage/s3/S3Utils.java +++ b/extensions-core/s3-extensions/src/main/java/org/apache/druid/storage/s3/S3Utils.java @@ -54,6 +54,8 @@ import java.util.ArrayList; import java.util.Iterator; import java.util.List; +import java.util.Optional; +import java.util.regex.Pattern; import java.util.stream.Collectors; /** @@ -63,6 +65,7 @@ public class S3Utils { private static final String SCHEME = S3StorageDruidModule.SCHEME; private static final Joiner JOINER = Joiner.on("/").skipNulls(); + private static final Pattern S3_ARN = Pattern.compile("^arn:(aws|aws-cn|aws-us-gov):s3:[a-z0-9-]*:\\d{12}:accesspoint[:/][A-Za-z0-9.-]+$"); private static final Logger log = new Logger(S3Utils.class); /** @@ -129,6 +132,11 @@ public static T retryS3Operation(Task f) throws Exception return RetryUtils.retry(f, S3RETRY, RetryUtils.DEFAULT_MAX_TRIES); } + public static boolean isS3Arn(String value) + { + return value != null && S3_ARN.matcher(value).matches(); + } + /** * Retries S3 operations that fail intermittently (due to io-related exceptions, during obtaining credentials, etc). * Service-level exceptions (access denied, file not found, etc) are not retried. @@ -264,6 +272,20 @@ static String grantFullControlHeaderValue(@Nullable Grant grant) return null; } + /** + * Normalizes a bucket name or ARN by replacing '/' with ':'. + * Some inputs may provide Access Point ARNs in the form + * arn:aws:s3:::accesspoint/alias.mrap, which should be normalized to + * arn:aws:s3:::accesspoint:alias.mrap. + */ + public static String normalizeBucketName(String bucket) + { + return Optional.ofNullable(bucket) + .filter(S3Utils::isS3Arn) + .map(arn -> arn.replace('/', ':')) + .orElse(bucket); + } + public static String extractS3Key(URI uri) { return StringUtils.maybeRemoveLeadingSlash(uri.getPath()); diff --git a/extensions-core/s3-extensions/src/test/java/org/apache/druid/storage/s3/S3UtilsTest.java b/extensions-core/s3-extensions/src/test/java/org/apache/druid/storage/s3/S3UtilsTest.java index dcf7b3f8d0d5..38542421ea13 100644 --- a/extensions-core/s3-extensions/src/test/java/org/apache/druid/storage/s3/S3UtilsTest.java +++ b/extensions-core/s3-extensions/src/test/java/org/apache/druid/storage/s3/S3UtilsTest.java @@ -449,4 +449,32 @@ public void testUseHttpsDefaultClientConfigSchemelessEndpointReturnsTrue() throw // Sanity check: default AWSClientConfig protocol is "https"; schemeless URL inherits "https". Assert.assertTrue(S3Utils.useHttps(new AWSClientConfig(), endpointWith("{\"url\":\"s3.example.com\"}"))); } + + @Test + public void testBucketNormalizationForMRAP() + { + String bucket = "arn:aws:s3::123456789123:accesspoint/bucket.mrap"; + Assert.assertEquals("arn:aws:s3::123456789123:accesspoint:bucket.mrap", S3Utils.normalizeBucketName(bucket)); + } + + @Test + public void testBucketNormalizationForRegional() + { + String bucket = "arn:aws:s3:us-east-1:123456789123:accesspoint/bucket"; + Assert.assertEquals("arn:aws:s3:us-east-1:123456789123:accesspoint:bucket", S3Utils.normalizeBucketName(bucket)); + } + + @Test + public void testAlreadyNormalizedBucket() + { + String bucket = "arn:aws:s3::123456789123:accesspoint:bucket.mrap"; + Assert.assertEquals(bucket, S3Utils.normalizeBucketName(bucket)); + } + + @Test + public void testNonS3Bucket() + { + String bucket = "bucket/subpath"; + Assert.assertEquals(bucket, S3Utils.normalizeBucketName(bucket)); + } } diff --git a/processing/src/main/java/org/apache/druid/data/input/impl/CloudObjectLocation.java b/processing/src/main/java/org/apache/druid/data/input/impl/CloudObjectLocation.java index c2cdd07e7766..a9a45c698aa4 100644 --- a/processing/src/main/java/org/apache/druid/data/input/impl/CloudObjectLocation.java +++ b/processing/src/main/java/org/apache/druid/data/input/impl/CloudObjectLocation.java @@ -27,6 +27,7 @@ import java.net.URI; import java.util.Objects; +import java.util.regex.Pattern; /** * Common type for 'bucket' and 'path' concept of cloud objects to allow code sharing between cloud specific @@ -46,6 +47,8 @@ */ public class CloudObjectLocation { + private static final Pattern S3_ARN = Pattern.compile("^arn:(aws|aws-cn|aws-us-gov):s3:[a-z0-9-]*:\\d{12}:accesspoint[:/][A-Za-z0-9.-]+$"); + public static URI validateUriScheme(String scheme, URI uri) { if (!scheme.equalsIgnoreCase(uri.getScheme())) { @@ -64,8 +67,9 @@ public CloudObjectLocation(@JsonProperty("bucket") String bucket, @JsonProperty( "bucket name cannot be null. Please verify if bucket name adheres to naming rules"); this.path = Preconditions.checkNotNull(StringUtils.maybeRemoveLeadingSlash(path)); Preconditions.checkArgument( - this.bucket.equals(StringUtils.urlEncode(this.bucket)), - "bucket must follow DNS-compliant naming conventions" + this.bucket.equals(StringUtils.urlEncode(this.bucket)) || isS3Arn(this.bucket), + "bucket must follow DNS-compliant naming conventions or be a valid S3 Access Point ARN" + + " or S3 Multi-Region Access Point ARN" ); } @@ -74,6 +78,11 @@ public CloudObjectLocation(URI uri) this(uri.getHost() != null ? uri.getHost() : uri.getAuthority(), uri.getPath()); } + private static boolean isS3Arn(String value) + { + return value != null && S3_ARN.matcher(value).matches(); + } + /** * Given a scheme, encode {@link #bucket} and {@link #path} into a {@link URI}. * diff --git a/processing/src/test/java/org/apache/druid/data/input/impl/CloudObjectLocationTest.java b/processing/src/test/java/org/apache/druid/data/input/impl/CloudObjectLocationTest.java index ec01274a89f7..467620a3d418 100644 --- a/processing/src/test/java/org/apache/druid/data/input/impl/CloudObjectLocationTest.java +++ b/processing/src/test/java/org/apache/druid/data/input/impl/CloudObjectLocationTest.java @@ -130,4 +130,44 @@ public void testBucketNameWithUnderscores() Assertions.assertEquals("test_bucket", s3ValidBucket.getBucket()); Assertions.assertEquals("path/to/path", s3ValidBucket.getPath()); } + + @Test + void testMRAP() + { + CloudObjectLocation location = new CloudObjectLocation("arn:aws:s3::123456789123:accesspoint:bucket.mrap", "path/to/path"); + Assertions.assertEquals("arn:aws:s3::123456789123:accesspoint:bucket.mrap", location.getBucket()); + Assertions.assertEquals("path/to/path", location.getPath()); + } + + @Test + void testMRAPWithURI() + { + CloudObjectLocation location = new CloudObjectLocation(URI.create("s3://arn:aws:s3::123456789123:accesspoint:bucket.mrap/path/to/path")); + Assertions.assertEquals("arn:aws:s3::123456789123:accesspoint:bucket.mrap", location.getBucket()); + Assertions.assertEquals("path/to/path", location.getPath()); + } + + @Test + void testRegionalARN() + { + CloudObjectLocation location = new CloudObjectLocation("arn:aws:s3:us-east-1:123456789123:accesspoint:bucket", "path/to/path"); + Assertions.assertEquals("arn:aws:s3:us-east-1:123456789123:accesspoint:bucket", location.getBucket()); + Assertions.assertEquals("path/to/path", location.getPath()); + } + + @Test + void testRegionARNWithURI() + { + CloudObjectLocation location = new CloudObjectLocation(URI.create("s3://arn:aws:s3:us-east-1:123456789123:accesspoint:bucket/path/to/path")); + Assertions.assertEquals("arn:aws:s3:us-east-1:123456789123:accesspoint:bucket", location.getBucket()); + Assertions.assertEquals("path/to/path", location.getPath()); + } + + @Test + void testMRAPWithPlusInKey() + { + CloudObjectLocation location = new CloudObjectLocation(URI.create("s3://arn:aws:s3::123456789123:accesspoint:bucket.mrap/path/with+plus")); + Assertions.assertEquals("arn:aws:s3::123456789123:accesspoint:bucket.mrap", location.getBucket()); + Assertions.assertEquals("path/with+plus", location.getPath()); + } } From 9a40fd38de39214dcd49fdd2d1871a02e3ef4591 Mon Sep 17 00:00:00 2001 From: Vivek Dhiman Date: Fri, 26 Jun 2026 15:05:35 +0530 Subject: [PATCH 2/2] Address review comment Signed-off-by: Vivek Dhiman --- .../druid/data/input/impl/CloudObjectLocation.java | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/processing/src/main/java/org/apache/druid/data/input/impl/CloudObjectLocation.java b/processing/src/main/java/org/apache/druid/data/input/impl/CloudObjectLocation.java index a9a45c698aa4..b5d8b01a7e50 100644 --- a/processing/src/main/java/org/apache/druid/data/input/impl/CloudObjectLocation.java +++ b/processing/src/main/java/org/apache/druid/data/input/impl/CloudObjectLocation.java @@ -63,7 +63,7 @@ public static URI validateUriScheme(String scheme, URI uri) @JsonCreator public CloudObjectLocation(@JsonProperty("bucket") String bucket, @JsonProperty("path") String path) { - this.bucket = Preconditions.checkNotNull(StringUtils.maybeRemoveTrailingSlash(bucket), + this.bucket = Preconditions.checkNotNull(normalizeBucket(bucket), "bucket name cannot be null. Please verify if bucket name adheres to naming rules"); this.path = Preconditions.checkNotNull(StringUtils.maybeRemoveLeadingSlash(path)); Preconditions.checkArgument( @@ -73,6 +73,17 @@ public CloudObjectLocation(@JsonProperty("bucket") String bucket, @JsonProperty( ); } + private static String normalizeBucket(String bucket) + { + String trimmed = StringUtils.maybeRemoveTrailingSlash(bucket); + + if (trimmed != null && isS3Arn(trimmed)) { + return trimmed.replace('/', ':'); + } + + return trimmed; + } + public CloudObjectLocation(URI uri) { this(uri.getHost() != null ? uri.getHost() : uri.getAuthority(), uri.getPath());