diff --git a/.changes/next-release/bugfix-AmazonS3-0a45c68.json b/.changes/next-release/bugfix-AmazonS3-0a45c68.json new file mode 100644 index 000000000000..b6b877703f29 --- /dev/null +++ b/.changes/next-release/bugfix-AmazonS3-0a45c68.json @@ -0,0 +1,6 @@ +{ + "type": "bugfix", + "category": "Amazon S3", + "contributor": "", + "description": "Fix a bug where tags were being lost when creating buckets in us-east-1" +} diff --git a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/handlers/CreateBucketInterceptor.java b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/handlers/CreateBucketInterceptor.java index 027c649bcc2c..6f7ac8bfab90 100644 --- a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/handlers/CreateBucketInterceptor.java +++ b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/handlers/CreateBucketInterceptor.java @@ -23,6 +23,7 @@ import software.amazon.awssdk.core.interceptor.ExecutionInterceptor; import software.amazon.awssdk.regions.Region; import software.amazon.awssdk.services.s3.internal.BucketUtils; +import software.amazon.awssdk.services.s3.model.BucketLocationConstraint; import software.amazon.awssdk.services.s3.model.CreateBucketConfiguration; import software.amazon.awssdk.services.s3.model.CreateBucketRequest; @@ -63,7 +64,12 @@ private SdkRequest addLocationConstraintToRequest(CreateBucketRequest request, private CreateBucketConfiguration setLocationConstraint(Region region, CreateBucketConfiguration configuration) { if (region.equals(Region.US_EAST_1)) { // us-east-1 requires no location restraint. See http://docs.aws.amazon.com/AmazonS3/latest/API/RESTBucketPUT.html - return null; + // But if tags exists, must keep them + if (configuration.hasTags()) { + return configuration.copy(c -> c.locationConstraint((BucketLocationConstraint) null)); + } else { + return null; + } } return configuration.copy(c -> c.locationConstraint(region.id()).build()); } diff --git a/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/handlers/CreateBucketInterceptorTest.java b/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/handlers/CreateBucketInterceptorTest.java index 8e2e89a40dfb..d75b2a03dc62 100644 --- a/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/handlers/CreateBucketInterceptorTest.java +++ b/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/handlers/CreateBucketInterceptorTest.java @@ -16,7 +16,13 @@ import static org.assertj.core.api.Assertions.assertThat; +import java.util.Arrays; +import java.util.List; +import java.util.stream.Stream; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import software.amazon.awssdk.awscore.AwsExecutionAttribute; import software.amazon.awssdk.core.SdkRequest; import software.amazon.awssdk.core.interceptor.Context; @@ -26,11 +32,12 @@ import software.amazon.awssdk.services.s3.model.CreateBucketConfiguration; import software.amazon.awssdk.services.s3.model.CreateBucketRequest; import software.amazon.awssdk.services.s3.model.LocationInfo; +import software.amazon.awssdk.services.s3.model.Tag; public class CreateBucketInterceptorTest { @Test - public void modifyRequest_DoesNotOverrideExistingLocationConstraint() { + void modifyRequest_DoesNotOverrideExistingLocationConstraint() { CreateBucketRequest request = CreateBucketRequest.builder() .bucket("test-bucket") .createBucketConfiguration(CreateBucketConfiguration.builder() @@ -51,7 +58,7 @@ public void modifyRequest_DoesNotOverrideExistingLocationConstraint() { } @Test - public void modifyRequest_UpdatesLocationConstraint_When_NullCreateBucketConfiguration() { + void modifyRequest_UpdatesLocationConstraint_When_NullCreateBucketConfiguration() { CreateBucketRequest request = CreateBucketRequest.builder() .bucket("test-bucket") .build(); @@ -68,7 +75,7 @@ public void modifyRequest_UpdatesLocationConstraint_When_NullCreateBucketConfigu } @Test - public void modifyRequest_UpdatesLocationConstraint_When_NullLocationConstraint() { + void modifyRequest_UpdatesLocationConstraint_When_NullLocationConstraint() { CreateBucketRequest request = CreateBucketRequest.builder() .bucket("test-bucket") .createBucketConfiguration(CreateBucketConfiguration.builder() @@ -87,7 +94,7 @@ public void modifyRequest_UpdatesLocationConstraint_When_NullLocationConstraint( } @Test - public void modifyRequest_DoesNotSetLocationConstraint_When_LocationPresent() { + void modifyRequest_DoesNotSetLocationConstraint_When_LocationPresent() { LocationInfo location = LocationInfo.builder().name("name").type("type").build(); CreateBucketConfiguration configuration = CreateBucketConfiguration.builder().location(location).build(); @@ -112,7 +119,7 @@ public void modifyRequest_DoesNotSetLocationConstraint_When_LocationPresent() { * For us-east-1 there must not be a location constraint (or containing CreateBucketConfiguration) sent. */ @Test - public void modifyRequest_UsEast1_UsesNullBucketConfiguration() { + void modifyRequest_UsEast1_UsesNullBucketConfiguration() { CreateBucketRequest request = CreateBucketRequest.builder() .bucket("test-bucket") .createBucketConfiguration(CreateBucketConfiguration.builder() @@ -122,14 +129,14 @@ public void modifyRequest_UsEast1_UsesNullBucketConfiguration() { Context.ModifyRequest context = () -> request; ExecutionAttributes attributes = new ExecutionAttributes() - .putAttribute(AwsExecutionAttribute.AWS_REGION, Region.US_EAST_1); + .putAttribute(AwsExecutionAttribute.AWS_REGION, Region.US_EAST_1); SdkRequest modifiedRequest = new CreateBucketInterceptor().modifyRequest(context, attributes); assertThat(((CreateBucketRequest) modifiedRequest).createBucketConfiguration()).isNull(); } @Test - public void modifyRequest__When_NullLocationConstraint() { + void modifyRequest__When_NullLocationConstraint() { CreateBucketRequest request = CreateBucketRequest.builder() .bucket("test-bucket") .createBucketConfiguration(CreateBucketConfiguration.builder() @@ -142,8 +149,74 @@ public void modifyRequest__When_NullLocationConstraint() { .putAttribute(AwsExecutionAttribute.AWS_REGION, Region.US_WEST_2); SdkRequest modifiedRequest = new CreateBucketInterceptor().modifyRequest(context, attributes); - String locationConstraint = ((CreateBucketRequest) modifiedRequest).createBucketConfiguration().locationConstraintAsString(); + String locationConstraint = + ((CreateBucketRequest) modifiedRequest).createBucketConfiguration().locationConstraintAsString(); assertThat(locationConstraint).isEqualToIgnoringCase("us-west-2"); } + + /* + Tags on create test cases: + Region has tags locationConstraint Expected CreateBucketConfiguration body + ---------------------------------------------------------------- + us-east-1 - tags - no locationConstraint: Body with tags only + us-east-1 - tags - locationConstraint: Body with both (locationConstraint=EU) + us-east-1 - no tags - no locationConstraint: Empty body (null) + us-east-1 - no tags - locationConstraint: Body with locationConstraint=EU + + us-west-2 - tags - no locationConstraint: Body with tags and locationConstraint=us-west-2 + us-west-2 - tags - locationConstraint: Body with both (locationConstraint=EU) + us-west-2 - no tags - no locationConstraint: Body with locationConstraint=us-west-2 + us-west-2 - no tags - locationConstraint: Body with locationConstraint=EU + */ + public static Stream createWithTagsTestParam() { + List tags = Arrays.asList( + Tag.builder().key("foo-key").value("foo-value").build(), + Tag.builder().key("bar-key").value("bar-value").build() + ); + return Stream.of( + Arguments.of(Region.US_EAST_1, tags, null, + CreateBucketConfiguration.builder().tags(tags).build()), + Arguments.of(Region.US_EAST_1, tags, BucketLocationConstraint.EU, + CreateBucketConfiguration.builder().tags(tags).locationConstraint(BucketLocationConstraint.EU).build()), + Arguments.of(Region.US_EAST_1, null, null, null), + Arguments.of(Region.US_EAST_1, null, BucketLocationConstraint.EU, + CreateBucketConfiguration.builder().locationConstraint(BucketLocationConstraint.EU).build()), + + Arguments.of(Region.US_WEST_2, tags, null, + CreateBucketConfiguration.builder().tags(tags).locationConstraint(BucketLocationConstraint.US_WEST_2).build()), + Arguments.of(Region.US_WEST_2, tags, BucketLocationConstraint.EU, + CreateBucketConfiguration.builder().tags(tags).locationConstraint(BucketLocationConstraint.EU).build()), + Arguments.of(Region.US_WEST_2, null, null, + CreateBucketConfiguration.builder().locationConstraint(BucketLocationConstraint.US_WEST_2).build()), + Arguments.of(Region.US_WEST_2, null, BucketLocationConstraint.EU, + CreateBucketConfiguration.builder().locationConstraint(BucketLocationConstraint.EU).build()) + ); + } + + @ParameterizedTest + @MethodSource("createWithTagsTestParam") + void modifyRequest__CreateWithTags(Region region, + List tags, + BucketLocationConstraint locationConstraint, + CreateBucketConfiguration expected) { + CreateBucketRequest request = CreateBucketRequest.builder() + .bucket("test-bucket") + .createBucketConfiguration( + CreateBucketConfiguration.builder() + .tags(tags) + .locationConstraint(locationConstraint) + .build()) + .build(); + + Context.ModifyRequest context = () -> request; + ExecutionAttributes attributes = new ExecutionAttributes().putAttribute(AwsExecutionAttribute.AWS_REGION, region); + + CreateBucketRequest modifiedRequest = + (CreateBucketRequest) new CreateBucketInterceptor().modifyRequest(context, attributes); + + assertThat(modifiedRequest.createBucketConfiguration()).isEqualTo(expected); + + } + }