Skip to content

Commit

Permalink
Fix for issue #4720 , Cross Region enabled Clients created in US-EAST…
Browse files Browse the repository at this point in the history
…-1 will by internally disable global endpoint and do a regional endpoint call. (#4849)

* Fix for issue #4720 , Cross Region enabled Clients created in US-EAST-1 will by internally disable global endpoint and do a regional endpoint call.

* Handled Zoe's comments
  • Loading branch information
joviegas committed Jan 26, 2024
1 parent 9b1f20a commit 50d97ac
Show file tree
Hide file tree
Showing 13 changed files with 170 additions and 182 deletions.
Original file line number Diff line number Diff line change
@@ -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)."
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -74,4 +76,9 @@ protected PutObjectResponse putAPICall(PutObjectRequest putObjectRequest, String
protected ResponseBytes<GetObjectResponse> getAPICall(GetObjectRequest getObjectRequest) {
return crossRegionS3Client.getObject(getObjectRequest, AsyncResponseTransformer.toBytes()).join();
}

@Override
protected HeadObjectResponse headObjectAPICall(HeadObjectRequest headObjectRequest) {
return crossRegionS3Client.headObject(headObjectRequest).join();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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(
Expand Down Expand Up @@ -136,6 +148,7 @@ void headApi_CrossRegionCall() {
protected abstract PutObjectResponse putAPICall(PutObjectRequest putObjectRequest, String testString);

protected abstract ResponseBytes<GetObjectResponse> getAPICall(GetObjectRequest getObjectRequest);
protected abstract HeadObjectResponse headObjectAPICall(HeadObjectRequest headObjectRequest);

protected abstract String bucketName();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -101,6 +103,11 @@ protected ResponseBytes<GetObjectResponse> 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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,23 +52,17 @@ protected <T extends S3Request, ReturnT> CompletableFuture<ReturnT> 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<ReturnT> returnFuture = new CompletableFuture<>();
CompletableFuture<ReturnT> apiOperationFuture = bucketToRegionCache.containsKey(bucketName) ?
operation.apply(
requestWithDecoratedEndpointProvider(
userAgentUpdatedRequest,
() -> bucketToRegionCache.get(bucketName),
serviceClientConfiguration().endpointProvider().get()
)
) :
operation.apply(userAgentUpdatedRequest);

CompletableFuture<ReturnT> apiOperationFuture = operation.apply(
requestWithDecoratedEndpointProvider(userAgentUpdatedRequest,
() -> bucketToRegionCache.get(bucketName),
serviceClientConfiguration().endpointProvider().get())
);
apiOperationFuture.whenComplete(redirectToCrossRegionIfRedirectException(operation,
userAgentUpdatedRequest,
bucketName,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,13 +64,10 @@ protected <T extends S3Request, ReturnT> 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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ public static BucketEndpointProvider create(S3EndpointProvider delegateEndPointP
public CompletableFuture<Endpoint> 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)));
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,13 @@


import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Optional;
import java.util.concurrent.CompletionException;
import java.util.function.Consumer;
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;
Expand All @@ -41,7 +39,6 @@ public final class CrossRegionUtils {
public static final String AMZ_BUCKET_REGION_HEADER = "x-amz-bucket-region";
private static final List<Integer> REDIRECT_STATUS_CODES =
Arrays.asList(REDIRECT_STATUS_CODE, TEMPORARY_REDIRECT_STATUS_CODE);
private static final List<String> REDIRECT_ERROR_CODES = Collections.singletonList("AuthorizationHeaderMalformed");
private static final ApiName API_NAME = ApiName.builder().version("cross-region").name("hll").build();
private static final Consumer<AwsRequestOverrideConfiguration.Builder> USER_AGENT_APPLIER = b -> b.addApiName(API_NAME);

Expand All @@ -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")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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() {

Expand All @@ -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)
Expand Down
Loading

0 comments on commit 50d97ac

Please sign in to comment.