[repository-s3] remove endpointOverride and let AWS SDK V2 S3 determine the s3 url based on bucket name or arn provided

CHANGELOG.md

@@ -25,6 +25,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Fix Netty deprecation warnings in transport-netty4 module ([#20233](https://github.com/opensearch-project/OpenSearch/pull/20233))
 - Fix snapshot restore when an index sort is present ([#20284](https://github.com/opensearch-project/OpenSearch/pull/20284))
 - Fix SearchPhaseExecutionException to properly initCause ([#20320](https://github.com/opensearch-project/OpenSearch/pull/20320))
+- [repository-s3] remove endpointOverride and let AWS SDK V2 S3 determine the s3 url based on bucket name or arn provided ([#20345](https://github.com/opensearch-project/OpenSearch/pull/20345))
 
 ### Dependencies
 - Bump `com.google.auth:google-auth-library-oauth2-http` from 1.38.0 to 1.41.0 ([#20183](https://github.com/opensearch-project/OpenSearch/pull/20183))

plugins/repository-s3/src/main/java/org/opensearch/repositories/s3/S3Service.java

@@ -95,6 +95,7 @@
 import java.security.SecureRandom;
 import java.time.Duration;
 import java.util.Map;
+import java.util.Optional;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ScheduledExecutorService;
 
@@ -221,23 +222,10 @@ AmazonS3WithCredentials buildClient(final S3ClientSettings clientSettings) {
         builder.httpClientBuilder(buildHttpClient(clientSettings));
         builder.overrideConfiguration(buildOverrideConfiguration(clientSettings, clientExecutorService));
 
-        String endpoint = Strings.hasLength(clientSettings.endpoint) ? clientSettings.endpoint : DEFAULT_S3_ENDPOINT;
-        if ((endpoint.startsWith("http://") || endpoint.startsWith("https://")) == false) {
-            // Manually add the schema to the endpoint to work around https://github.com/aws/aws-sdk-java/issues/2274
-            // TODO: Remove this once fixed in the AWS SDK
-            endpoint = clientSettings.protocol.toString() + "://" + endpoint;
-        }
-        logger.debug("using endpoint [{}] and region [{}]", endpoint, clientSettings.region);
-
-        // If the endpoint configuration isn't set on the builder then the default behaviour is to try
-        // and work out what region we are in and use an appropriate endpoint - see AwsClientBuilder#setRegion.
-        // In contrast, directly-constructed clients use s3.amazonaws.com unless otherwise instructed. We currently
-        // use a directly-constructed client, and need to keep the existing behaviour to avoid a breaking change,
-        // so to move to using the builder we must set it explicitly to keep the existing behaviour.
-        //
-        // We do this because directly constructing the client is deprecated (was already deprecated in 1.1.223 too)
-        // so this change removes that usage of a deprecated API.
-        builder.endpointOverride(URI.create(endpoint));
+        // Only apply endpointOverride when the user explicitly configures "endpoint".
+        // If "endpoint" is absent, DO NOT override; allow the AWS SDK to resolve endpoints dynamically.
+        // This is required for ARN buckets (access points / outposts / MRAP), and is also correct for normal buckets.
+        resolveEndpointOverride(clientSettings).ifPresent(builder::endpointOverride);
         if (Strings.hasText(clientSettings.region)) {
             builder.region(Region.of(clientSettings.region));
         }
@@ -256,6 +244,28 @@ AmazonS3WithCredentials buildClient(final S3ClientSettings clientSettings) {
         return AmazonS3WithCredentials.create(client, credentials);
     }
 
+    /**
+      * Returns an endpoint override ONLY when the user explicitly provided one.
+      * Otherwise returns Optional.empty().
+      *
+      * Package-private to allow unit testing.
+      */
+    Optional<URI> resolveEndpointOverride(final S3ClientSettings clientSettings) {
+        if (Strings.hasLength(clientSettings.endpoint) == false) {
+            return Optional.empty();
+        }
+
+        String endpoint = clientSettings.endpoint;
+        if ((endpoint.startsWith("http://") || endpoint.startsWith("https://")) == false) {
+            // Manually add the schema to the endpoint to work around https://github.com/aws/aws-sdk-java/issues/2274
+            // TODO: Remove this once fixed in the AWS SDK
+            endpoint = clientSettings.protocol.toString() + "://" + endpoint;
+        }
+
+        // Use URI.create for simplicity; if your codebase prefers checked handling, swap to new URI(endpoint).
+        return Optional.of(URI.create(endpoint));
+    }
+
     // Aws v2 sdk tries to load a default profile from home path which is restricted. Hence, setting these to random
     // valid paths.
     @SuppressForbidden(reason = "Need to provide this override to v2 SDK so that path does not default to home path")

plugins/repository-s3/src/test/java/org/opensearch/repositories/s3/S3ArnEndpointResolutionTests.java

@@ -0,0 +1,196 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ */
+
+package org.opensearch.repositories.s3;
+
+import software.amazon.awssdk.auth.credentials.AwsBasicCredentials;
+import software.amazon.awssdk.auth.credentials.StaticCredentialsProvider;
+import software.amazon.awssdk.core.client.config.ClientOverrideConfiguration;
+import software.amazon.awssdk.core.interceptor.Context;
+import software.amazon.awssdk.core.interceptor.ExecutionAttributes;
+import software.amazon.awssdk.core.interceptor.ExecutionInterceptor;
+import software.amazon.awssdk.http.ExecutableHttpRequest;
+import software.amazon.awssdk.http.HttpExecuteRequest;
+import software.amazon.awssdk.http.HttpExecuteResponse;
+import software.amazon.awssdk.http.SdkHttpClient;
+import software.amazon.awssdk.regions.Region;
+import software.amazon.awssdk.services.s3.S3Client;
+import software.amazon.awssdk.services.s3.S3Configuration;
+import software.amazon.awssdk.services.s3.model.HeadBucketRequest;
+
+import java.net.URI;
+import java.util.concurrent.atomic.AtomicReference;
+
+/**
+ * Tests for AWS SDK endpoint resolution behavior for S3 bucket ARNs (Outposts / Access Points),
+ * and OpenSearch repository-s3 behavior around endpoint overrides.
+ *
+ * These tests are intentionally "no-network": they capture the resolved request URI using an
+ * ExecutionInterceptor and abort the request via a failing HTTP client.
+ */
+public class S3ArnEndpointResolutionTests extends AbstractS3RepositoryTestCase {
+
+    private static final StaticCredentialsProvider DUMMY_CREDENTIALS = StaticCredentialsProvider.create(
+        AwsBasicCredentials.create("dummy-access-key", "dummy-secret-key")
+    );
+
+    // ---- Helpers ----
+
+    private static final class CapturingInterceptor implements ExecutionInterceptor {
+        private final AtomicReference<URI> captured = new AtomicReference<>();
+        private final AtomicReference<String> capturedSigningName = new AtomicReference<>();
+
+        @Override
+        public void beforeTransmission(Context.BeforeTransmission context, ExecutionAttributes executionAttributes) {
+            captured.set(context.httpRequest().getUri());
+            // Capture the signing name from the Authorization header
+            // Format: AWS4-HMAC-SHA256 Credential=AKID/20250101/us-east-1/s3-outposts/aws4_request, ...
+            context.httpRequest().firstMatchingHeader("Authorization").ifPresent(auth -> {
+                int credIdx = auth.indexOf("Credential=");
+                if (credIdx >= 0) {
+                    String[] parts = auth.substring(credIdx).split("/");
+                    if (parts.length >= 5) {
+                        capturedSigningName.set(parts[3]); // service name is 4th part (index 3)
+                    }
+                }
+            });
+        }
+
+        URI capturedUri() {
+            return captured.get();
+        }
+
+        String capturedSigningName() {
+            return capturedSigningName.get();
+        }
+    }
+
+    private static SdkHttpClient failingHttpClient() {
+        return new SdkHttpClient() {
+            @Override
+            public ExecutableHttpRequest prepareRequest(HttpExecuteRequest request) {
+                return new ExecutableHttpRequest() {
+                    @Override
+                    public HttpExecuteResponse call() {
+                        throw new RuntimeException("stop after endpoint resolution");
+                    }
+
+                    @Override
+                    public void abort() {
+                        // no-op
+                    }
+                };
+            }
+
+            @Override
+            public void close() {
+                // no-op
+            }
+        };
+    }
+
+    private static void issueNoNetworkRequest(S3Client client, String bucket) {
+        try {
+            client.headBucket(HeadBucketRequest.builder().bucket(bucket).build());
+            fail("Expected to fail due to no-network http client");
+        } catch (RuntimeException e) {
+            // expected
+        }
+    }
+
+    // ---- SDK contract tests ----
+
+    public void testSdkResolvesOutpostsEndpointFromBucketArn() {
+        // Shape matters more than reality for endpoint logic; account/outpost IDs can be dummy.
+        final String bucketArn = "arn:aws:s3-outposts:us-east-1:111122223333:outpost/op-0123456789abcdef/accesspoint/my-ap";
+
+        final CapturingInterceptor interceptor = new CapturingInterceptor();
+
+        try (
+            S3Client client = S3Client.builder()
+                .region(Region.US_EAST_1)
+                .credentialsProvider(DUMMY_CREDENTIALS)
+                .serviceConfiguration(
+                    S3Configuration.builder()
+                        // Ensure ARN region behavior is enabled; harmless for this test if already defaulted.
+                        .useArnRegionEnabled(true)
+                        .build()
+                )
+                .overrideConfiguration(ClientOverrideConfiguration.builder().addExecutionInterceptor(interceptor).build())
+                .httpClient(failingHttpClient())
+                // IMPORTANT: do NOT set endpointOverride(...) in these SDK tests.
+                .build()
+        ) {
+
+            issueNoNetworkRequest(client, bucketArn);
+        }
+
+        final URI uri = interceptor.capturedUri();
+        assertNotNull("SDK did not reach beforeTransmission; endpoint not captured", uri);
+        assertNotNull(uri.getHost());
+        assertTrue("Expected outposts endpoint but was: " + uri, uri.getHost().contains("s3-outposts"));
+
+        // Verify SDK uses correct signing service for Outposts
+        assertEquals("s3-outposts", interceptor.capturedSigningName());
+    }
+
+    public void testSdkResolvesAccessPointEndpointFromBucketArn() {
+        final String bucketArn = "arn:aws:s3:us-west-2:111122223333:accesspoint/my-ap";
+
+        final CapturingInterceptor interceptor = new CapturingInterceptor();
+
+        try (
+            S3Client client = S3Client.builder()
+                .region(Region.US_WEST_2)
+                .credentialsProvider(DUMMY_CREDENTIALS)
+                .serviceConfiguration(S3Configuration.builder().useArnRegionEnabled(true).build())
+                .overrideConfiguration(ClientOverrideConfiguration.builder().addExecutionInterceptor(interceptor).build())
+                .httpClient(failingHttpClient())
+                .build()
+        ) {
+
+            issueNoNetworkRequest(client, bucketArn);
+        }
+
+        final URI uri = interceptor.capturedUri();
+        assertNotNull("SDK did not reach beforeTransmission; endpoint not captured", uri);
+        assertNotNull(uri.getHost());
+        assertTrue("Expected accesspoint endpoint but was: " + uri, uri.getHost().contains("s3-accesspoint"));
+
+        // Verify SDK uses correct signing service for access points
+        assertEquals("s3", interceptor.capturedSigningName());
+    }
+
+    public void testSdkResolvesRegularBucketToRegularS3Endpoint() {
+        final String bucket = "my-standard-bucket";
+
+        final CapturingInterceptor interceptor = new CapturingInterceptor();
+
+        try (
+            S3Client client = S3Client.builder()
+                .region(Region.US_EAST_1)
+                .credentialsProvider(DUMMY_CREDENTIALS)
+                .overrideConfiguration(ClientOverrideConfiguration.builder().addExecutionInterceptor(interceptor).build())
+                .httpClient(failingHttpClient())
+                .build()
+        ) {
+
+            issueNoNetworkRequest(client, bucket);
+        }
+
+        final URI uri = interceptor.capturedUri();
+        assertNotNull("SDK did not reach beforeTransmission; endpoint not captured", uri);
+        assertNotNull(uri.getHost());
+
+        final String host = uri.getHost();
+        // Be tolerant across SDK versions/partitions: just assert it isn't ARN-specific routing.
+        assertFalse("Unexpected accesspoint host for non-ARN bucket: " + uri, host.contains("s3-accesspoint"));
+        assertFalse("Unexpected outposts host for non-ARN bucket: " + uri, host.contains("s3-outposts"));
+        assertTrue("Expected some form of S3 host but was: " + uri, host.contains("s3"));
+    }
+}

plugins/repository-s3/src/test/java/org/opensearch/repositories/s3/S3ServiceTests.java

@@ -36,7 +36,9 @@
 import org.opensearch.common.settings.Settings;
 import org.opensearch.secure_sm.AccessController;
 
+import java.net.URI;
 import java.util.Map;
+import java.util.Optional;
 
 public class S3ServiceTests extends AbstractS3RepositoryTestCase {
     public void testCachedClientsAreReleased() {
@@ -83,4 +85,47 @@ public void testCachedClientsWithCredentialsAreReleased() {
         final S3ClientSettings clientSettingsReloaded = s3Service.settings(metadata1);
         assertNotSame(clientSettings, clientSettingsReloaded);
     }
+
+    public void testResolveEndpointOverrideAbsentWhenEndpointNotProvided() {
+        final S3Service s3Service = new S3Service(configPath());
+        final Settings repoSettings = Settings.builder()
+            // region is required by S3Service.settings(...) in this test suite
+            .put("region", "us-east-1")
+            // intentionally omit "endpoint"
+            .build();
+        final RepositoryMetadata metadata = new RepositoryMetadata("no-endpoint", "s3", repoSettings);
+
+        final S3ClientSettings clientSettings = s3Service.settings(metadata);
+        final Optional<URI> override = s3Service.resolveEndpointOverride(clientSettings);
+        assertTrue("Expected no endpoint override when endpoint setting is absent", override.isEmpty());
+    }
+
+    public void testResolveEndpointOverrideAddsSchemeWhenMissing() {
+        final S3Service s3Service = new S3Service(configPath());
+        final Settings repoSettings = Settings.builder()
+            .put("region", "us-east-1")
+            // no scheme on purpose
+            .put("endpoint", "s3.us-east-1.amazonaws.com")
+            .build();
+        final RepositoryMetadata metadata = new RepositoryMetadata("endpoint-no-scheme", "s3", repoSettings);
+
+        final S3ClientSettings clientSettings = s3Service.settings(metadata);
+        final Optional<URI> override = s3Service.resolveEndpointOverride(clientSettings);
+        assertTrue("Expected endpoint override to be present when endpoint setting is provided", override.isPresent());
+
+        // S3 client settings default protocol in tests should be https unless explicitly configured otherwise.
+        // If this ever changes in the future, update this assertion accordingly.
+        assertEquals("https://s3.us-east-1.amazonaws.com", override.get().toString());
+    }
+
+    public void testResolveEndpointOverridePreservesExplicitScheme() {
+        final S3Service s3Service = new S3Service(configPath());
+        final Settings repoSettings = Settings.builder().put("region", "us-east-1").put("endpoint", "http://localhost:9000").build();
+        final RepositoryMetadata metadata = new RepositoryMetadata("endpoint-with-scheme", "s3", repoSettings);
+
+        final S3ClientSettings clientSettings = s3Service.settings(metadata);
+        final Optional<URI> override = s3Service.resolveEndpointOverride(clientSettings);
+        assertTrue("Expected endpoint override to be present when endpoint has explicit scheme", override.isPresent());
+        assertEquals("http://localhost:9000", override.get().toString());
+    }
 }