Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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."
}
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ static void clearClass() {
@BeforeEach
public void initialize() {
crossRegionS3Client = S3AsyncClient.crtBuilder()
.region(CROSS_REGION)
.region(Region.AWS_GLOBAL)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Intentional change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeap , making sure we test this in Integ too since the cross region feature depends mainly on the error headers send by S3 based on the client region configured.
We have different client configured in different regions in https://github.com/aws/aws-sdk-java-v2/tree/b415e16e755d7c95c7bfae4725de4d623fb43c5b/services/s3/src/it/java/software/amazon/awssdk/services/s3/crossregion

.crossRegionAccessEnabled(true)
.build();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -450,6 +450,28 @@ void given_AnyRegion_Client_Updates_the_useGlobalEndpointFlag_asFalse(String reg
});
}

@Test
void given_globalRegion_Client_Updates_region_to_useast1_and_useGlobalEndpointFlag_as_False() {
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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -277,6 +277,28 @@ void given_AnyRegion_Client_Updates_the_useGlobalEndpointFlag_asFalse(String reg
});
}

@Test
void given_globalRegion_Client_Updates_region_to_useast1_and_useGlobalEndpointFlag_as_False() {
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)
Expand Down