Fix for Issue #4912 where Cross Region S3 Access using AWS_GLOBAL End…

…point failed.
aws · Feb 13, 2024 · 2385f9b · 2385f9b
1 parent 6671c79
commit 2385f9b
Show file tree

Hide file tree

Showing 5 changed files with 70 additions and 6 deletions.
diff --git a/.changes/next-release/bugfix-AmazonSimpleStorageService-7648336.json b/.changes/next-release/bugfix-AmazonSimpleStorageService-7648336.json
@@ -0,0 +1,6 @@
+{
+    "type": "bugfix",
+    "category": "Amazon Simple Storage Service",
+    "contributor": "",
+    "description": "Fix for Issue [#4912](https://github.com/aws/aws-sdk-java-v2/issues/4912) where client region with AWS_GLOBAL calls failed for cross region access."
+}
diff --git a/.../java/software/amazon/awssdk/services/s3/crossregion/S3CrossRegionCrtIntegrationTest.java b/.../java/software/amazon/awssdk/services/s3/crossregion/S3CrossRegionCrtIntegrationTest.java
@@ -46,7 +46,7 @@ static void clearClass() {
     @BeforeEach
     public void initialize() {
         crossRegionS3Client = S3AsyncClient.crtBuilder()
-                                           .region(CROSS_REGION)
+                                           .region(Region.AWS_GLOBAL)
                                            .crossRegionAccessEnabled(true)
                                              .build();
     }

diff --git a/...azon/awssdk/services/s3/internal/crossregion/endpointprovider/BucketEndpointProvider.java b/...azon/awssdk/services/s3/internal/crossregion/endpointprovider/BucketEndpointProvider.java
@@ -43,9 +43,20 @@ public static BucketEndpointProvider create(S3EndpointProvider delegateEndPointP
     @Override
     public CompletableFuture<Endpoint> resolveEndpoint(S3EndpointParams endpointParams) {
         Region crossRegion = regionSupplier.get();
-        return delegateEndPointProvider.resolveEndpoint(
-            endpointParams.copy(c -> c.region(crossRegion == null ? endpointParams.region() : crossRegion)
-                                      .useGlobalEndpoint(false)));
+        S3EndpointParams.Builder endpointParamsBuilder = endpointParams.toBuilder();
+        // Check if cross-region resolution has already occurred.
+        if (crossRegion != null) {
+            endpointParamsBuilder.region(crossRegion);
+        } else {
+            // For global regions, set the region to "us-east-1" to use regional endpoints.
+            if (Region.AWS_GLOBAL.equals(endpointParams.region())) {
+                endpointParamsBuilder.region(Region.US_EAST_1);
+            }
+            // Disable the global endpoint as S3 can properly redirect regions in the 'x-amz-bucket-region' header
+            // only for regional endpoints.
+            endpointParamsBuilder.useGlobalEndpoint(false);
+        }
+        return delegateEndPointProvider.resolveEndpoint(endpointParamsBuilder.build());
     }
 }
 
diff --git a/...software/amazon/awssdk/services/s3/internal/crossregion/S3CrossRegionAsyncClientTest.java b/...software/amazon/awssdk/services/s3/internal/crossregion/S3CrossRegionAsyncClientTest.java
@@ -429,7 +429,7 @@ void given_US_EAST_1_Client_resolveToRegionalEndpoints_when_crossRegion_is_True(
     }
 
     @ParameterizedTest
-    @ValueSource(strings = {"us-east-1", "us-east-2", "us-west-1", "aws-global"})
+    @ValueSource(strings = {"us-east-1", "us-east-2", "us-west-1"})
     void given_AnyRegion_Client_Updates_the_useGlobalEndpointFlag_asFalse(String region) {
         mockAsyncHttpClient.stubResponses(successHttpResponse());
         S3EndpointProvider mockEndpointProvider = Mockito.mock(S3EndpointProvider.class);
@@ -450,6 +450,31 @@ void given_AnyRegion_Client_Updates_the_useGlobalEndpointFlag_asFalse(String reg
         });
     }
 
+
+
+    @Test
+    void given_global_Client_Updates_the_regionAnduseGlobalEndpointFlag_asFalse() {
+        String region = Region.AWS_GLOBAL.id();
+        mockAsyncHttpClient.stubResponses(successHttpResponse());
+        S3EndpointProvider mockEndpointProvider = Mockito.mock(S3EndpointProvider.class);
+
+        when(mockEndpointProvider.resolveEndpoint(ArgumentMatchers.any(S3EndpointParams.class)))
+            .thenReturn(CompletableFuture.completedFuture(Endpoint.builder().url(URI.create("https://bucket.s3.amazonaws.com")).build()));
+
+        S3AsyncClient s3Client = clientBuilder().crossRegionAccessEnabled(true)
+                                                .region(Region.of(region))
+                                                .endpointProvider(mockEndpointProvider).build();
+        s3Client.getObject(r -> r.bucket(BUCKET).key(KEY), AsyncResponseTransformer.toBytes()).join();
+        assertThat(captureInterceptor.endpointProvider).isInstanceOf(BucketEndpointProvider.class);
+        ArgumentCaptor<S3EndpointParams> collectionCaptor = ArgumentCaptor.forClass(S3EndpointParams.class);
+        verify(mockEndpointProvider, atLeastOnce()).resolveEndpoint(collectionCaptor.capture());
+        collectionCaptor.getAllValues().forEach(resolvedParams -> {
+            assertThat(resolvedParams.region()).isEqualTo(Region.US_EAST_1);
+            assertThat(resolvedParams.useGlobalEndpoint()).isFalse();
+        });
+    }
+
+
     private S3AsyncClientBuilder clientBuilder() {
         return S3AsyncClient.builder()
                             .httpClient(mockAsyncHttpClient)

diff --git a/.../software/amazon/awssdk/services/s3/internal/crossregion/S3CrossRegionSyncClientTest.java b/.../software/amazon/awssdk/services/s3/internal/crossregion/S3CrossRegionSyncClientTest.java
@@ -256,7 +256,7 @@ void given_US_EAST_1_Client_resolveToRegionalEndpoints_when_crossRegion_is_True(
     }
 
     @ParameterizedTest
-    @ValueSource(strings = {"us-east-1", "us-east-2", "us-west-1", "aws-global"})
+    @ValueSource(strings = {"us-east-1", "us-east-2", "us-west-1"})
     void given_AnyRegion_Client_Updates_the_useGlobalEndpointFlag_asFalse(String region) {
         mockSyncHttpClient.stubResponses(successHttpResponse());
         S3EndpointProvider mockEndpointProvider = Mockito.mock(S3EndpointProvider.class);
@@ -277,6 +277,28 @@ void given_AnyRegion_Client_Updates_the_useGlobalEndpointFlag_asFalse(String reg
         });
     }
 
+    @Test
+    void globalRegion() {
+        String region = Region.AWS_GLOBAL.id();
+        mockSyncHttpClient.stubResponses(successHttpResponse());
+        S3EndpointProvider mockEndpointProvider = Mockito.mock(S3EndpointProvider.class);
+
+        when(mockEndpointProvider.resolveEndpoint(ArgumentMatchers.any(S3EndpointParams.class)))
+            .thenReturn(CompletableFuture.completedFuture(Endpoint.builder().url(URI.create("https://bucket.s3.amazonaws.com")).build()));
+
+        S3Client s3Client = clientBuilder().crossRegionAccessEnabled(true)
+                                           .region(Region.of(region))
+                                           .endpointProvider(mockEndpointProvider).build();
+        s3Client.getObject(getObjectBuilder().build());
+        assertThat(captureInterceptor.endpointProvider).isInstanceOf(BucketEndpointProvider.class);
+        ArgumentCaptor<S3EndpointParams> collectionCaptor = ArgumentCaptor.forClass(S3EndpointParams.class);
+        verify(mockEndpointProvider,  atLeastOnce()).resolveEndpoint(collectionCaptor.capture());
+        collectionCaptor.getAllValues().forEach(resolvedParams ->{
+            assertThat(resolvedParams.region()).isEqualTo(Region.US_EAST_1);
+            assertThat(resolvedParams.useGlobalEndpoint()).isFalse();
+        });
+    }
+
     private static GetObjectRequest.Builder getObjectBuilder() {
         return GetObjectRequest.builder()
                                .bucket(BUCKET)