diff --git a/.changes/next-release/bugfix-AmazonSimpleStorageService-4b6b8dd.json b/.changes/next-release/bugfix-AmazonSimpleStorageService-4b6b8dd.json new file mode 100644 index 000000000000..0f9947d3c7b0 --- /dev/null +++ b/.changes/next-release/bugfix-AmazonSimpleStorageService-4b6b8dd.json @@ -0,0 +1,6 @@ +{ + "type": "bugfix", + "category": "Amazon Simple Storage Service", + "contributor": "", + "description": "S3 client configured with crossRegionEnabled(true) will now use us-east-1 regional endpoint instead of the global endpoint. See [#4720](https://github.com/aws/aws-sdk-java-v2/issues/4720)." +} diff --git a/services/s3/src/it/java/software/amazon/awssdk/services/s3/crossregion/S3CrossRegionAsyncIntegrationTestBase.java b/services/s3/src/it/java/software/amazon/awssdk/services/s3/crossregion/S3CrossRegionAsyncIntegrationTestBase.java index ee1b82d4da4a..59781034e827 100644 --- a/services/s3/src/it/java/software/amazon/awssdk/services/s3/crossregion/S3CrossRegionAsyncIntegrationTestBase.java +++ b/services/s3/src/it/java/software/amazon/awssdk/services/s3/crossregion/S3CrossRegionAsyncIntegrationTestBase.java @@ -30,6 +30,8 @@ import software.amazon.awssdk.services.s3.model.GetObjectResponse; import software.amazon.awssdk.services.s3.model.HeadBucketRequest; import software.amazon.awssdk.services.s3.model.HeadBucketResponse; +import software.amazon.awssdk.services.s3.model.HeadObjectRequest; +import software.amazon.awssdk.services.s3.model.HeadObjectResponse; import software.amazon.awssdk.services.s3.model.ListObjectsV2Request; import software.amazon.awssdk.services.s3.model.PutObjectRequest; import software.amazon.awssdk.services.s3.model.PutObjectResponse; @@ -74,4 +76,9 @@ protected PutObjectResponse putAPICall(PutObjectRequest putObjectRequest, String protected ResponseBytes getAPICall(GetObjectRequest getObjectRequest) { return crossRegionS3Client.getObject(getObjectRequest, AsyncResponseTransformer.toBytes()).join(); } + + @Override + protected HeadObjectResponse headObjectAPICall(HeadObjectRequest headObjectRequest) { + return crossRegionS3Client.headObject(headObjectRequest).join(); + } } diff --git a/services/s3/src/it/java/software/amazon/awssdk/services/s3/crossregion/S3CrossRegionIntegrationTestBase.java b/services/s3/src/it/java/software/amazon/awssdk/services/s3/crossregion/S3CrossRegionIntegrationTestBase.java index 006eb493d71d..d28bb071a461 100644 --- a/services/s3/src/it/java/software/amazon/awssdk/services/s3/crossregion/S3CrossRegionIntegrationTestBase.java +++ b/services/s3/src/it/java/software/amazon/awssdk/services/s3/crossregion/S3CrossRegionIntegrationTestBase.java @@ -35,6 +35,8 @@ import software.amazon.awssdk.services.s3.model.GetObjectResponse; import software.amazon.awssdk.services.s3.model.HeadBucketRequest; import software.amazon.awssdk.services.s3.model.HeadBucketResponse; +import software.amazon.awssdk.services.s3.model.HeadObjectRequest; +import software.amazon.awssdk.services.s3.model.HeadObjectResponse; import software.amazon.awssdk.services.s3.model.ListObjectsV2Request; import software.amazon.awssdk.services.s3.model.PutObjectRequest; import software.amazon.awssdk.services.s3.model.PutObjectResponse; @@ -58,6 +60,16 @@ void getApi_CrossRegionCall() { assertThat(new String(response.asByteArray())).isEqualTo("TEST_STRING"); } + @Test + void headObjectApi_CrossRegionCall() { + s3.putObject(p -> p.bucket(bucketName()).checksumAlgorithm(ChecksumAlgorithm.CRC32).key(KEY), RequestBody.fromString( + "TEST_STRING")); + HeadObjectRequest headObjectRequest = + HeadObjectRequest.builder().bucket(bucketName()).checksumMode(ChecksumMode.ENABLED).key(KEY).build(); + HeadObjectResponse response = headObjectAPICall(headObjectRequest); + assertThat(response.contentLength()).isEqualTo("TEST_STRING".length()); + } + @Test void putApi_CrossRegionCall() { s3.putObject(p -> p.bucket(bucketName()).checksumAlgorithm(ChecksumAlgorithm.CRC32).key(KEY), RequestBody.fromString( @@ -136,6 +148,7 @@ void headApi_CrossRegionCall() { protected abstract PutObjectResponse putAPICall(PutObjectRequest putObjectRequest, String testString); protected abstract ResponseBytes getAPICall(GetObjectRequest getObjectRequest); + protected abstract HeadObjectResponse headObjectAPICall(HeadObjectRequest headObjectRequest); protected abstract String bucketName(); diff --git a/services/s3/src/it/java/software/amazon/awssdk/services/s3/crossregion/S3CrossRegionSyncIntegrationTest.java b/services/s3/src/it/java/software/amazon/awssdk/services/s3/crossregion/S3CrossRegionSyncIntegrationTest.java index 20b8a0bbff1d..72e7cc217257 100644 --- a/services/s3/src/it/java/software/amazon/awssdk/services/s3/crossregion/S3CrossRegionSyncIntegrationTest.java +++ b/services/s3/src/it/java/software/amazon/awssdk/services/s3/crossregion/S3CrossRegionSyncIntegrationTest.java @@ -36,6 +36,8 @@ import software.amazon.awssdk.services.s3.model.GetObjectResponse; import software.amazon.awssdk.services.s3.model.HeadBucketRequest; import software.amazon.awssdk.services.s3.model.HeadBucketResponse; +import software.amazon.awssdk.services.s3.model.HeadObjectRequest; +import software.amazon.awssdk.services.s3.model.HeadObjectResponse; import software.amazon.awssdk.services.s3.model.ListObjectsV2Request; import software.amazon.awssdk.services.s3.model.ListObjectsV2Response; import software.amazon.awssdk.services.s3.model.PutObjectRequest; @@ -101,6 +103,11 @@ protected ResponseBytes getAPICall(GetObjectRequest getObject return crossRegionS3Client.getObject(getObjectRequest, ResponseTransformer.toBytes()); } + @Override + protected HeadObjectResponse headObjectAPICall(HeadObjectRequest headObjectRequest) { + return crossRegionS3Client.headObject(headObjectRequest); + } + @Override protected String bucketName() { return BUCKET; diff --git a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/crossregion/S3CrossRegionAsyncClient.java b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/crossregion/S3CrossRegionAsyncClient.java index f8efd780b351..0269acaee221 100644 --- a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/crossregion/S3CrossRegionAsyncClient.java +++ b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/crossregion/S3CrossRegionAsyncClient.java @@ -52,23 +52,17 @@ protected CompletableFuture invokeOperat AwsRequestOverrideConfiguration overrideConfiguration = updateUserAgentInConfig(request); T userAgentUpdatedRequest = (T) request.toBuilder().overrideConfiguration(overrideConfiguration).build(); - if (!bucket.isPresent()) { return operation.apply(userAgentUpdatedRequest); } String bucketName = bucket.get(); CompletableFuture returnFuture = new CompletableFuture<>(); - CompletableFuture apiOperationFuture = bucketToRegionCache.containsKey(bucketName) ? - operation.apply( - requestWithDecoratedEndpointProvider( - userAgentUpdatedRequest, - () -> bucketToRegionCache.get(bucketName), - serviceClientConfiguration().endpointProvider().get() - ) - ) : - operation.apply(userAgentUpdatedRequest); - + CompletableFuture apiOperationFuture = operation.apply( + requestWithDecoratedEndpointProvider(userAgentUpdatedRequest, + () -> bucketToRegionCache.get(bucketName), + serviceClientConfiguration().endpointProvider().get()) + ); apiOperationFuture.whenComplete(redirectToCrossRegionIfRedirectException(operation, userAgentUpdatedRequest, bucketName, diff --git a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/crossregion/S3CrossRegionSyncClient.java b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/crossregion/S3CrossRegionSyncClient.java index 10ac10af5816..56195bd56fa8 100644 --- a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/crossregion/S3CrossRegionSyncClient.java +++ b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/crossregion/S3CrossRegionSyncClient.java @@ -64,13 +64,10 @@ protected ReturnT invokeOperation(T request, Func } String bucketName = bucketRequest.get(); try { - if (bucketToRegionCache.containsKey(bucketName)) { - return operation.apply( - requestWithDecoratedEndpointProvider(userAgentUpdatedRequest, - () -> bucketToRegionCache.get(bucketName), - serviceClientConfiguration().endpointProvider().get())); - } - return operation.apply(userAgentUpdatedRequest); + return operation.apply( + requestWithDecoratedEndpointProvider(userAgentUpdatedRequest, + () -> bucketToRegionCache.get(bucketName), + serviceClientConfiguration().endpointProvider().get())); } catch (S3Exception exception) { if (isS3RedirectException(exception)) { updateCacheFromRedirectException(exception, bucketName); diff --git a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/crossregion/endpointprovider/BucketEndpointProvider.java b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/crossregion/endpointprovider/BucketEndpointProvider.java index ce89341753b5..84f1e69abf8a 100644 --- a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/crossregion/endpointprovider/BucketEndpointProvider.java +++ b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/crossregion/endpointprovider/BucketEndpointProvider.java @@ -44,7 +44,8 @@ public static BucketEndpointProvider create(S3EndpointProvider delegateEndPointP public CompletableFuture resolveEndpoint(S3EndpointParams endpointParams) { Region crossRegion = regionSupplier.get(); return delegateEndPointProvider.resolveEndpoint( - crossRegion != null ? endpointParams.copy(c -> c.region(crossRegion)) : endpointParams); + endpointParams.copy(c -> c.region(crossRegion == null ? endpointParams.region() : crossRegion) + .useGlobalEndpoint(false))); } } diff --git a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/crossregion/utils/CrossRegionUtils.java b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/crossregion/utils/CrossRegionUtils.java index 0a994c9743b9..450302d75968 100644 --- a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/crossregion/utils/CrossRegionUtils.java +++ b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/crossregion/utils/CrossRegionUtils.java @@ -17,7 +17,6 @@ import java.util.Arrays; -import java.util.Collections; import java.util.List; import java.util.Optional; import java.util.concurrent.CompletionException; @@ -25,7 +24,6 @@ import java.util.function.Supplier; import software.amazon.awssdk.annotations.SdkInternalApi; import software.amazon.awssdk.awscore.AwsRequestOverrideConfiguration; -import software.amazon.awssdk.awscore.exception.AwsErrorDetails; import software.amazon.awssdk.core.ApiName; import software.amazon.awssdk.endpoints.EndpointProvider; import software.amazon.awssdk.regions.Region; @@ -41,7 +39,6 @@ public final class CrossRegionUtils { public static final String AMZ_BUCKET_REGION_HEADER = "x-amz-bucket-region"; private static final List REDIRECT_STATUS_CODES = Arrays.asList(REDIRECT_STATUS_CODE, TEMPORARY_REDIRECT_STATUS_CODE); - private static final List REDIRECT_ERROR_CODES = Collections.singletonList("AuthorizationHeaderMalformed"); private static final ApiName API_NAME = ApiName.builder().version("cross-region").name("hll").build(); private static final Consumer USER_AGENT_APPLIER = b -> b.addApiName(API_NAME); @@ -65,12 +62,7 @@ private static boolean isRedirectError(S3Exception exceptionToBeChecked) { if (REDIRECT_STATUS_CODES.stream().anyMatch(status -> status.equals(exceptionToBeChecked.statusCode()))) { return true; } - if (getBucketRegionFromException(exceptionToBeChecked).isPresent()) { - return true; - } - AwsErrorDetails awsErrorDetails = exceptionToBeChecked.awsErrorDetails(); - return awsErrorDetails != null - && REDIRECT_ERROR_CODES.stream().anyMatch(code -> code.equals(awsErrorDetails.errorCode())); + return getBucketRegionFromException(exceptionToBeChecked).isPresent(); } @SuppressWarnings("unchecked") diff --git a/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/crossregion/S3CrossRegionAsyncClientRedirectTest.java b/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/crossregion/S3CrossRegionAsyncClientRedirectTest.java index c4951f3a3bb4..3af7a6651d02 100644 --- a/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/crossregion/S3CrossRegionAsyncClientRedirectTest.java +++ b/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/crossregion/S3CrossRegionAsyncClientRedirectTest.java @@ -131,14 +131,6 @@ protected void verifyHeadBucketServiceCall(int times) { verify(mockDelegateAsyncClient, times(times)).headBucket(any(Consumer.class)); } - @Override - protected void stubApiWithAuthorizationHeaderMalformedError() { - when(mockDelegateAsyncClient.listObjects(any(ListObjectsRequest.class))) - .thenReturn(CompletableFutureUtils.failedFuture( - new CompletionException(redirectException(400, null, "AuthorizationHeaderMalformed", null)))) - .thenReturn(CompletableFuture.completedFuture(ListObjectsResponse.builder().contents(S3_OBJECTS).build())); - } - @Override protected void stubApiWithAuthorizationHeaderWithInternalSoftwareError() { @@ -150,16 +142,6 @@ protected void stubApiWithAuthorizationHeaderWithInternalSoftwareError() { "InternalError", null)))); } - @Override - protected void stubHeadBucketRedirectWithAuthorizationHeaderMalformed() { - when(mockDelegateAsyncClient.headBucket(any(HeadBucketRequest.class))) - .thenReturn(CompletableFutureUtils.failedFuture( - new CompletionException(redirectException(400,CROSS_REGION.id(), "AuthorizationHeaderMalformed", null)))); - when(mockDelegateAsyncClient.headBucket(any(Consumer.class))) - .thenReturn(CompletableFutureUtils.failedFuture( - new CompletionException(redirectException(400,CROSS_REGION.id(), "AuthorizationHeaderMalformed", null)))); - } - @Override protected void verifyNoBucketCall() { assertThatExceptionOfType(CompletionException.class) diff --git a/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/crossregion/S3CrossRegionAsyncClientTest.java b/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/crossregion/S3CrossRegionAsyncClientTest.java index 061cbccffe21..a5323269f275 100644 --- a/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/crossregion/S3CrossRegionAsyncClientTest.java +++ b/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/crossregion/S3CrossRegionAsyncClientTest.java @@ -17,9 +17,10 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import static software.amazon.awssdk.services.s3.internal.crossregion.S3CrossRegionRedirectTestBase.CHANGED_CROSS_REGION; import static software.amazon.awssdk.services.s3.internal.crossregion.S3CrossRegionRedirectTestBase.CROSS_REGION; -import static software.amazon.awssdk.services.s3.internal.crossregion.S3CrossRegionRedirectTestBase.CROSS_REGION_BUCKET; import static software.amazon.awssdk.services.s3.internal.crossregion.S3CrossRegionRedirectTestBase.OVERRIDE_CONFIGURED_REGION; import static software.amazon.awssdk.services.s3.internal.crossregion.S3CrossRegionRedirectTestBase.X_AMZ_BUCKET_REGION; @@ -37,12 +38,17 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; +import org.junit.jupiter.params.provider.ValueSource; +import org.mockito.ArgumentCaptor; +import org.mockito.ArgumentMatchers; +import org.mockito.Mockito; import software.amazon.awssdk.core.ResponseBytes; import software.amazon.awssdk.core.async.AsyncResponseTransformer; 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.core.interceptor.SdkInternalExecutionAttribute; +import software.amazon.awssdk.endpoints.Endpoint; import software.amazon.awssdk.endpoints.EndpointProvider; import software.amazon.awssdk.http.AbortableInputStream; import software.amazon.awssdk.http.HttpExecuteResponse; @@ -50,8 +56,11 @@ import software.amazon.awssdk.http.SdkHttpMethod; import software.amazon.awssdk.http.SdkHttpRequest; import software.amazon.awssdk.http.SdkHttpResponse; +import software.amazon.awssdk.regions.Region; import software.amazon.awssdk.services.s3.S3AsyncClient; import software.amazon.awssdk.services.s3.S3AsyncClientBuilder; +import software.amazon.awssdk.services.s3.endpoints.S3EndpointParams; +import software.amazon.awssdk.services.s3.endpoints.S3EndpointProvider; import software.amazon.awssdk.services.s3.endpoints.internal.DefaultS3EndpointProvider; import software.amazon.awssdk.services.s3.internal.crossregion.endpointprovider.BucketEndpointProvider; import software.amazon.awssdk.services.s3.model.GetObjectRequest; @@ -87,16 +96,13 @@ private static Stream stubSuccessfulRedirectResponses() { Consumer successStubConsumer = mockAsyncHttpClient -> mockAsyncHttpClient.stubResponses(successHttpResponse(), successHttpResponse()); - Consumer malFormerAuthError = mockAsyncHttpClient -> - mockAsyncHttpClient.stubResponses( - customHttpResponse(400, "AuthorizationHeaderMalformed", null), - customHttpResponse(400, "AuthorizationHeaderMalformed", CROSS_REGION_BUCKET), - successHttpResponse()); + Consumer tempRedirectStubConsumer = mockAsyncHttpClient -> + mockAsyncHttpClient.stubResponses(customHttpResponseWithUnknownErrorCode(307, CROSS_REGION.id()), successHttpResponse()); return Stream.of( Arguments.of(redirectStubConsumer, BucketEndpointProvider.class, "Redirect Error with region in x-amz-bucket-header"), - Arguments.of(successStubConsumer, DefaultS3EndpointProvider.class, "Success response" ), - Arguments.of(malFormerAuthError, BucketEndpointProvider.class, "Authorization Malformed Error with region in x-amz-bucket-header in Head bucket response" ) + Arguments.of(successStubConsumer, BucketEndpointProvider.class, "Success response" ), + Arguments.of(tempRedirectStubConsumer, BucketEndpointProvider.class, "Permanent redirect Error with region in x-amz-bucket-header" ) ); } @@ -116,21 +122,8 @@ private static Stream stubFailureResponses() { mockAsyncHttpClient.stubResponses(customHttpResponseWithUnknownErrorCode(301, null), customHttpResponseWithUnknownErrorCode(301, null), successHttpResponse(), successHttpResponse()); - - Consumer authMalformedWithNoRegion = mockAsyncHttpClient -> - mockAsyncHttpClient.stubResponses(customHttpResponse(400, "AuthorizationHeaderMalformed", null), - customHttpResponse(400, "AuthorizationHeaderMalformed", null)); - - Consumer authMalformedAuthorizationFailureAfterRegionRetrieval = mockAsyncHttpClient -> - mockAsyncHttpClient.stubResponses(customHttpResponse(400, "AuthorizationHeaderMalformed", null), - customHttpResponse(400, "AuthorizationHeaderMalformed", CROSS_REGION.id()), - customHttpResponse(400, "AuthorizationHeaderMalformed", CROSS_REGION.id())); - return Stream.of( - Arguments.of(redirectFailedWithNoRegionFailure, 301, 2, noRegionOnHeadBucket, noregionOnHeadBucketHttpMethodListMethodList), - Arguments.of(authMalformedWithNoRegion, 400, 2, noRegionOnHeadBucket, noregionOnHeadBucketHttpMethodListMethodList), - Arguments.of(authMalformedAuthorizationFailureAfterRegionRetrieval, 400, 3, regionOnHeadBucket, - regionOnHeadBucketHttpMethodList) + Arguments.of(redirectFailedWithNoRegionFailure, 301, 2, noRegionOnHeadBucket, noregionOnHeadBucketHttpMethodListMethodList) ); } @@ -416,10 +409,49 @@ void given_SimpleClient_when_StandardOperation_then_DoesNotContainCrossRegionUse assertThat(mockAsyncHttpClient.getLastRequest().firstMatchingHeader("User-Agent").get()).doesNotContain("hll/cross"); } + @Test + void given_US_EAST_1_Client_resolvesToGlobalEndpoints_when_crossRegion_is_False(){ + mockAsyncHttpClient.stubResponses(successHttpResponse()); + S3AsyncClient s3Client = clientBuilder().region(Region.US_EAST_1).build(); + s3Client.getObject(r -> r.bucket(BUCKET).key(KEY), AsyncResponseTransformer.toBytes()).join(); + assertThat(mockAsyncHttpClient.getLastRequest().host()).isEqualTo("bucket.s3.amazonaws.com"); + assertThat(captureInterceptor.endpointProvider).isInstanceOf(DefaultS3EndpointProvider.class); + } + + @Test + void given_US_EAST_1_Client_resolveToRegionalEndpoints_when_crossRegion_is_True(){ + mockAsyncHttpClient.stubResponses(successHttpResponse()); + S3AsyncClient s3Client = clientBuilder().crossRegionAccessEnabled(true).region(Region.US_EAST_1).build(); + s3Client.getObject(r -> r.bucket(BUCKET).key(KEY), AsyncResponseTransformer.toBytes()).join(); + assertThat(mockAsyncHttpClient.getLastRequest().host()).isEqualTo("bucket.s3.us-east-1.amazonaws.com"); + assertThat(captureInterceptor.endpointProvider).isInstanceOf(BucketEndpointProvider.class); + } + + @ParameterizedTest + @ValueSource(strings = {"us-east-1", "us-east-2", "us-west-1", "aws-global"}) + void given_AnyRegion_Client_Updates_the_useGlobalEndpointFlag_asFalse(String region) { + 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 collectionCaptor = ArgumentCaptor.forClass(S3EndpointParams.class); + verify(mockEndpointProvider).resolveEndpoint(collectionCaptor.capture()); + S3EndpointParams resolvedParams = collectionCaptor.getAllValues().get(0); + assertThat(resolvedParams.region()).isEqualTo(Region.of(region)); + assertThat(resolvedParams.useGlobalEndpoint()).isFalse(); + } + + private S3AsyncClientBuilder clientBuilder() { return S3AsyncClient.builder() .httpClient(mockAsyncHttpClient) - .endpointOverride(URI.create("http://localhost")) .overrideConfiguration(c -> c.addExecutionInterceptor(captureInterceptor)); } diff --git a/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/crossregion/S3CrossRegionRedirectTestBase.java b/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/crossregion/S3CrossRegionRedirectTestBase.java index d5123149ad00..470e6f036581 100644 --- a/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/crossregion/S3CrossRegionRedirectTestBase.java +++ b/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/crossregion/S3CrossRegionRedirectTestBase.java @@ -65,7 +65,8 @@ void decoratorAttemptsToRetryWithRegionNameInErrorResponse(Integer redirect) thr ArgumentCaptor requestArgumentCaptor = ArgumentCaptor.forClass(ListObjectsRequest.class); verifyTheApiServiceCall(2, requestArgumentCaptor); - assertThat(requestArgumentCaptor.getAllValues().get(0).overrideConfiguration().get().endpointProvider()).isNotPresent(); + assertThat(requestArgumentCaptor.getAllValues().get(0).overrideConfiguration().get().endpointProvider().get()) + .isInstanceOf(BucketEndpointProvider.class); verifyTheEndPointProviderOverridden(1, requestArgumentCaptor, CROSS_REGION.id()); verifyHeadBucketServiceCall(0); @@ -86,7 +87,8 @@ void decoratorUsesCache_when_CrossRegionAlreadyPresent(Integer redirect) throws ArgumentCaptor requestArgumentCaptor = ArgumentCaptor.forClass(ListObjectsRequest.class); verifyTheApiServiceCall(3, requestArgumentCaptor); - assertThat(requestArgumentCaptor.getAllValues().get(0).overrideConfiguration().get().endpointProvider()).isNotPresent(); + assertThat(requestArgumentCaptor.getAllValues().get(0).overrideConfiguration().get().endpointProvider().get()) + .isInstanceOf(BucketEndpointProvider.class); verifyTheEndPointProviderOverridden(1, requestArgumentCaptor, CROSS_REGION.id()); verifyTheEndPointProviderOverridden(2, requestArgumentCaptor, CROSS_REGION.id()); verifyHeadBucketServiceCall(0); @@ -120,7 +122,8 @@ void headBucketCalled_when_RedirectDoesNotHasRegionName(Integer redirect) throws ArgumentCaptor requestArgumentCaptor = ArgumentCaptor.forClass(ListObjectsRequest.class); verifyTheApiServiceCall(2, requestArgumentCaptor); - assertThat(requestArgumentCaptor.getAllValues().get(0).overrideConfiguration().get().endpointProvider()).isNotPresent(); + assertThat(requestArgumentCaptor.getAllValues().get(0).overrideConfiguration().get().endpointProvider().get()) + .isInstanceOf(BucketEndpointProvider.class); verifyTheEndPointProviderOverridden(1, requestArgumentCaptor, CROSS_REGION.id()); verifyHeadBucketServiceCall(1); } @@ -144,7 +147,8 @@ void headBucketCalledAndCached__when_RedirectDoesNotHasRegionName(Integer redire // We need to captor again so that we get the args used in second API Call ArgumentCaptor overAllPostCacheCaptor = ArgumentCaptor.forClass(ListObjectsRequest.class); verifyTheApiServiceCall(3, overAllPostCacheCaptor); - assertThat(overAllPostCacheCaptor.getAllValues().get(0).overrideConfiguration().get().endpointProvider()).isNotPresent(); + assertThat(overAllPostCacheCaptor.getAllValues().get(0).overrideConfiguration().get().endpointProvider().get()) + .isInstanceOf(BucketEndpointProvider.class); verifyTheEndPointProviderOverridden(1, overAllPostCacheCaptor, CROSS_REGION.id()); verifyTheEndPointProviderOverridden(2, overAllPostCacheCaptor, CROSS_REGION.id()); verifyHeadBucketServiceCall(1); @@ -163,62 +167,8 @@ void requestsAreNotOverridden_when_NoBucketInRequest() throws Throwable { verifyHeadBucketServiceCall(0); } - @Test - void given_CrossRegionClient_when_AuthorizationFailureInMainCall_then_RegionRetrievedFromHeadBucketFailureResponse() throws Throwable { - stubServiceClientConfiguration(); - stubApiWithAuthorizationHeaderMalformedError() ; - stubHeadBucketRedirect(); - ListObjectsResponse firstListObjectCall = apiCallToService(); - assertThat(firstListObjectCall.contents()).isEqualTo(S3_OBJECTS); - - ArgumentCaptor preCacheCaptor = ArgumentCaptor.forClass(ListObjectsRequest.class); - verifyTheApiServiceCall(2, preCacheCaptor); - // We need to get the BucketEndpointProvider in order to update the cache - verifyTheEndPointProviderOverridden(1, preCacheCaptor, CROSS_REGION.id()); - - - ListObjectsResponse secondListObjectCall = apiCallToService(); - assertThat(secondListObjectCall.contents()).isEqualTo(S3_OBJECTS); - // We need to captor again so that we get the args used in second API Call - ArgumentCaptor overAllPostCacheCaptor = ArgumentCaptor.forClass(ListObjectsRequest.class); - verifyTheApiServiceCall(3, overAllPostCacheCaptor); - assertThat(overAllPostCacheCaptor.getAllValues().get(0).overrideConfiguration().get().endpointProvider()).isNotPresent(); - verifyTheEndPointProviderOverridden(1, overAllPostCacheCaptor, CROSS_REGION.id()); - verifyTheEndPointProviderOverridden(2, overAllPostCacheCaptor, CROSS_REGION.id()); - verifyHeadBucketServiceCall(1); - } - - - @Test - void given_CrossRegionClient_when_AuthorizationFailureInHeadBucketWithRegion_then_CrossRegionCallSucceeds() throws Throwable { - stubServiceClientConfiguration(); - stubApiWithAuthorizationHeaderMalformedError() ; - stubHeadBucketRedirectWithAuthorizationHeaderMalformed(); - ListObjectsResponse firstListObjectCall = apiCallToService(); - assertThat(firstListObjectCall.contents()).isEqualTo(S3_OBJECTS); - - ArgumentCaptor preCacheCaptor = ArgumentCaptor.forClass(ListObjectsRequest.class); - verifyTheApiServiceCall(2, preCacheCaptor); - // We need to get the BucketEndpointProvider in order to update the cache - verifyTheEndPointProviderOverridden(1, preCacheCaptor, CROSS_REGION.id()); - ListObjectsResponse secondListObjectCall = apiCallToService(); - assertThat(secondListObjectCall.contents()).isEqualTo(S3_OBJECTS); - // We need to captor again so that we get the args used in second API Call - ArgumentCaptor overAllPostCacheCaptor = ArgumentCaptor.forClass(ListObjectsRequest.class); - verifyTheApiServiceCall(3, overAllPostCacheCaptor); - assertThat(overAllPostCacheCaptor.getAllValues().get(0).overrideConfiguration().get().endpointProvider()).isNotPresent(); - verifyTheEndPointProviderOverridden(1, overAllPostCacheCaptor, CROSS_REGION.id()); - verifyTheEndPointProviderOverridden(2, overAllPostCacheCaptor, CROSS_REGION.id()); - verifyHeadBucketServiceCall(1); - } - protected abstract void stubApiWithAuthorizationHeaderWithInternalSoftwareError(); - - protected abstract void stubHeadBucketRedirectWithAuthorizationHeaderMalformed(); - - protected abstract void stubApiWithAuthorizationHeaderMalformedError(); - protected abstract void verifyNoBucketCall(); protected abstract void verifyNoBucketApiCall(int i, ArgumentCaptor requestArgumentCaptor); diff --git a/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/crossregion/S3CrossRegionSyncClientRedirectTest.java b/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/crossregion/S3CrossRegionSyncClientRedirectTest.java index 276cf2c43728..1b2bb7cfb0b0 100644 --- a/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/crossregion/S3CrossRegionSyncClientRedirectTest.java +++ b/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/crossregion/S3CrossRegionSyncClientRedirectTest.java @@ -49,19 +49,6 @@ protected void stubApiWithAuthorizationHeaderWithInternalSoftwareError() { when(mockDelegateClient.headBucket(any(HeadBucketRequest.class))) .thenThrow(redirectException(500, null, "InternalError", null)); } - @Override - protected void stubHeadBucketRedirectWithAuthorizationHeaderMalformed() { - when(mockDelegateClient.headBucket(any(HeadBucketRequest.class))) - .thenThrow(redirectException(400, CROSS_REGION.id(), "AuthorizationHeaderMalformed", null)) - .thenReturn(HeadBucketResponse.builder().build()); - } - - @Override - protected void stubApiWithAuthorizationHeaderMalformedError() { - when(mockDelegateClient.listObjects(any(ListObjectsRequest.class))) - .thenThrow(redirectException(400, null, "AuthorizationHeaderMalformed", null)) - .thenReturn(ListObjectsResponse.builder().contents(S3_OBJECTS).build()); - } @Override protected void verifyNoBucketCall() { diff --git a/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/crossregion/S3CrossRegionSyncClientTest.java b/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/crossregion/S3CrossRegionSyncClientTest.java index 0501768c2e73..196d92485b9c 100644 --- a/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/crossregion/S3CrossRegionSyncClientTest.java +++ b/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/crossregion/S3CrossRegionSyncClientTest.java @@ -17,6 +17,8 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import static software.amazon.awssdk.services.s3.internal.crossregion.S3CrossRegionAsyncClientTest.customHttpResponse; import static software.amazon.awssdk.services.s3.internal.crossregion.S3CrossRegionAsyncClientTest.customHttpResponseWithUnknownErrorCode; import static software.amazon.awssdk.services.s3.internal.crossregion.S3CrossRegionAsyncClientTest.successHttpResponse; @@ -25,8 +27,10 @@ import static software.amazon.awssdk.services.s3.internal.crossregion.S3CrossRegionRedirectTestBase.CROSS_REGION_BUCKET; import static software.amazon.awssdk.services.s3.internal.crossregion.S3CrossRegionRedirectTestBase.OVERRIDE_CONFIGURED_REGION; +import java.net.URI; import java.util.Arrays; import java.util.List; +import java.util.concurrent.CompletableFuture; import java.util.function.Consumer; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -35,16 +39,24 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; +import org.junit.jupiter.params.provider.ValueSource; +import org.mockito.ArgumentCaptor; +import org.mockito.ArgumentMatchers; +import org.mockito.Mock; +import org.mockito.Mockito; 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.core.interceptor.SdkInternalExecutionAttribute; +import software.amazon.awssdk.endpoints.Endpoint; import software.amazon.awssdk.endpoints.EndpointProvider; import software.amazon.awssdk.http.SdkHttpMethod; import software.amazon.awssdk.http.SdkHttpRequest; import software.amazon.awssdk.regions.Region; import software.amazon.awssdk.services.s3.S3Client; import software.amazon.awssdk.services.s3.S3ClientBuilder; +import software.amazon.awssdk.services.s3.endpoints.S3EndpointParams; +import software.amazon.awssdk.services.s3.endpoints.S3EndpointProvider; import software.amazon.awssdk.services.s3.endpoints.internal.DefaultS3EndpointProvider; import software.amazon.awssdk.services.s3.internal.crossregion.endpointprovider.BucketEndpointProvider; import software.amazon.awssdk.services.s3.model.GetObjectRequest; @@ -78,23 +90,13 @@ private static Stream stubSuccessfulRedirectResponses() { Consumer successStubConsumer = mockSyncHttpClient -> mockSyncHttpClient.stubResponses(successHttpResponse(), successHttpResponse()); - Consumer malFormerAuthError = mockSyncHttpClient -> - mockSyncHttpClient.stubResponses( - customHttpResponse(400, "AuthorizationHeaderMalformed",null ), - customHttpResponse(400, "AuthorizationHeaderMalformed",CROSS_REGION_BUCKET ), - successHttpResponse()); - return Stream.of( Arguments.of(redirectStubConsumer, BucketEndpointProvider.class, "Redirect Error with region in x-amz-bucket-header"), Arguments.of(successStubConsumer, - DefaultS3EndpointProvider.class, - "Success response" ), - Arguments.of(malFormerAuthError, BucketEndpointProvider.class, - "Authorization Malformed Error with region in x-amz-bucket-header in Head bucket response" ) - ); + "Success response" )); } public static Stream stubFailureResponses() { @@ -113,14 +115,6 @@ public static Stream stubFailureResponses() { customHttpResponseWithUnknownErrorCode(301, null), successHttpResponse(), successHttpResponse()); - Consumer authMalformedWithNoRegion = mockSyncHttpClient -> - mockSyncHttpClient.stubResponses(customHttpResponse(400, "AuthorizationHeaderMalformed", null), - customHttpResponse(400, "AuthorizationHeaderMalformed", null)); - - Consumer authMalformedAuthorizationFailureAfterRegionRetrieval = mockSyncHttpClient -> - mockSyncHttpClient.stubResponses(customHttpResponse(400, "AuthorizationHeaderMalformed", null), - customHttpResponse(400, "AuthorizationHeaderMalformed", CROSS_REGION.id()), - customHttpResponse(400, "AuthorizationHeaderMalformed", CROSS_REGION.id())); return Stream.of( @@ -129,20 +123,7 @@ public static Stream stubFailureResponses() { 2, noRegionOnHeadBucket, noregionOnHeadBucketHttpMethodListMethodList - ), - Arguments.of(authMalformedWithNoRegion, - 400, - 2, - noRegionOnHeadBucket, - noregionOnHeadBucketHttpMethodListMethodList - ), - Arguments.of(authMalformedAuthorizationFailureAfterRegionRetrieval, - 400, - 3, - regionOnHeadBucket, - regionOnHeadBucketHttpMethodList ) - ); } @@ -156,7 +137,7 @@ private static Stream stubOverriddenEndpointProviderResponses() { return Stream.of( Arguments.of(redirectStubConsumer, BucketEndpointProvider.class, CROSS_REGION, "Redirect error with Region in x-amz-bucket-region header"), - Arguments.of(successStubConsumer, TestEndpointProvider.class, OVERRIDE_CONFIGURED_REGION, + Arguments.of(successStubConsumer, BucketEndpointProvider.class, OVERRIDE_CONFIGURED_REGION, "Success response.") ); } @@ -181,10 +162,7 @@ void given_CrossRegionClientWithExistingOverrideConfig_when_StandardOperationIsP String testCaseName) { stubConsumer.accept(mockSyncHttpClient); S3Client crossRegionClient = new S3CrossRegionSyncClient(defaultS3Client); - GetObjectRequest request = GetObjectRequest.builder() - .bucket(BUCKET) - .key(KEY) - .overrideConfiguration(o -> o.putHeader("someheader", "somevalue")) + GetObjectRequest request = getObjectBuilder() .build(); crossRegionClient.getObject(request); assertThat(captureInterceptor.endpointProvider).isInstanceOf(endpointProviderType); @@ -252,10 +230,7 @@ void given_CrossRegionClientWithCustomEndpointProvider_when_StandardOperationIsP .endpointProvider(new TestEndpointProvider()) .region(OVERRIDE_CONFIGURED_REGION) .build(); - GetObjectRequest request = GetObjectRequest.builder() - .bucket(BUCKET) - .key(KEY) - .overrideConfiguration(o -> o.putHeader("someheader", "somevalue")) + GetObjectRequest request = getObjectBuilder() .build(); crossRegionClient.getObject(request); assertThat(captureInterceptor.endpointProvider).isInstanceOf(endpointProviderType); @@ -263,6 +238,51 @@ void given_CrossRegionClientWithCustomEndpointProvider_when_StandardOperationIsP assertThat(mockSyncHttpClient.getLastRequest().encodedPath()).contains("test_prefix_"); assertThat(mockSyncHttpClient.getLastRequest().host()).contains(region.id()); } + @Test + void given_US_EAST_1_Client_resolvesToGlobalEndpoints_when_crossRegion_is_False(){ + mockSyncHttpClient.stubResponses(successHttpResponse()); + S3Client s3Client = clientBuilder().region(Region.US_EAST_1).build(); + s3Client.getObject(getObjectBuilder().build()); + assertThat(mockSyncHttpClient.getLastRequest().host()).isEqualTo("bucket.s3.amazonaws.com"); + assertThat(captureInterceptor.endpointProvider).isInstanceOf(DefaultS3EndpointProvider.class); + } + + @Test + void given_US_EAST_1_Client_resolveToRegionalEndpoints_when_crossRegion_is_True(){ + mockSyncHttpClient.stubResponses(successHttpResponse()); + S3Client s3Client = clientBuilder().crossRegionAccessEnabled(true).region(Region.US_EAST_1).build(); + s3Client.getObject(getObjectBuilder().build()); + assertThat(mockSyncHttpClient.getLastRequest().host()).isEqualTo("bucket.s3.us-east-1.amazonaws.com"); + assertThat(captureInterceptor.endpointProvider).isInstanceOf(BucketEndpointProvider.class); + } + + @ParameterizedTest + @ValueSource(strings = {"us-east-1", "us-east-2", "us-west-1", "aws-global"}) + void given_AnyRegion_Client_Updates_the_useGlobalEndpointFlag_asFalse(String region) { + 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 collectionCaptor = ArgumentCaptor.forClass(S3EndpointParams.class); + verify(mockEndpointProvider).resolveEndpoint(collectionCaptor.capture()); + S3EndpointParams resolvedParams = collectionCaptor.getAllValues().get(0); + assertThat(resolvedParams.region()).isEqualTo(Region.of(region)); + assertThat(resolvedParams.useGlobalEndpoint()).isFalse(); + } + + private static GetObjectRequest.Builder getObjectBuilder() { + return GetObjectRequest.builder() + .bucket(BUCKET) + .key(KEY) + .overrideConfiguration(o -> o.putHeader("someheader", "somevalue")); + } @Test void given_CrossRegionClientWithFallbackCall_when_RegionNameNotPresent_then_CallsHeadObject() { @@ -365,7 +385,7 @@ void given_CrossRegionClient_when_noRegionInHeader_thenFallBackToRegionInHeadBuc @ParameterizedTest(name = "{index} - Status code = {1} with HeadBucket bucket regions {3}.") @MethodSource("stubFailureResponses") - void given_CrossRegionClient_when_CallsHeadObjectErrors_then_ShouldTerminateTheAPI( + void given_CrossRegionClient_when_CallsHeadBucketErrors_then_ShouldTerminateTheAPI( Consumer stubFailureResponses, Integer statusCode, Integer numberOfRequests, List redirectedBucketRegions,