From 13867a04405c74e278c8cca553886baa87130aee Mon Sep 17 00:00:00 2001 From: msailes Date: Thu, 10 Sep 2020 13:35:08 +0100 Subject: [PATCH 1/3] feat: Added support for object canned access control lists. --- README.md | 2 +- pom.xml | 2 +- .../PayloadStorageConfiguration.java | 41 +++++++++++++++++++ .../S3BackedPayloadStore.java | 9 +++- .../amazon/payloadoffloading/S3Dao.java | 8 +++- .../PayloadStorageConfigurationTest.java | 18 +++++++- .../S3BackedPayloadStoreTest.java | 8 +++- 7 files changed, 81 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 84175ca..3307474 100644 --- a/README.md +++ b/README.md @@ -24,7 +24,7 @@ You can download release builds through the [releases section of this](https://g software.amazon.payloadoffloading payloadoffloading-common - 2.0.0 + 2.1.0 ``` diff --git a/pom.xml b/pom.xml index 74976a3..8b32280 100644 --- a/pom.xml +++ b/pom.xml @@ -6,7 +6,7 @@ software.amazon.payloadoffloading payloadoffloading-common - 2.0.0 + 2.1.0 jar Payload offloading common library for AWS Common library between extended Amazon AWS clients to save payloads up to 2GB on Amazon S3. diff --git a/src/main/java/software/amazon/payloadoffloading/PayloadStorageConfiguration.java b/src/main/java/software/amazon/payloadoffloading/PayloadStorageConfiguration.java index 7ec1449..d57bcc2 100644 --- a/src/main/java/software/amazon/payloadoffloading/PayloadStorageConfiguration.java +++ b/src/main/java/software/amazon/payloadoffloading/PayloadStorageConfiguration.java @@ -5,6 +5,7 @@ import software.amazon.awssdk.annotations.NotThreadSafe; import software.amazon.awssdk.core.exception.SdkClientException; import software.amazon.awssdk.services.s3.S3Client; +import software.amazon.awssdk.services.s3.model.ObjectCannedACL; /** *

Amazon payload storage configuration options such as Amazon S3 client, @@ -42,11 +43,16 @@ public class PayloadStorageConfiguration { * This field is optional, it is set only when we want to configure S3 Server Side Encryption with KMS. */ private ServerSideEncryptionStrategy serverSideEncryptionStrategy; + /** + * This field is optional, it is set only when we want to add access control list to Amazon S3 buckets and objects + */ + private ObjectCannedACL objectCannedACL; public PayloadStorageConfiguration() { s3 = null; s3BucketName = null; serverSideEncryptionStrategy = null; + objectCannedACL = null; } public PayloadStorageConfiguration(PayloadStorageConfiguration other) { @@ -56,6 +62,7 @@ public PayloadStorageConfiguration(PayloadStorageConfiguration other) { this.alwaysThroughS3 = other.isAlwaysThroughS3(); this.payloadSizeThreshold = other.getPayloadSizeThreshold(); this.serverSideEncryptionStrategy = other.getServerSideEncryptionStrategy(); + this.objectCannedACL = other.getObjectCannedACL(); } /** @@ -235,4 +242,38 @@ public ServerSideEncryptionStrategy getServerSideEncryptionStrategy() { return this.serverSideEncryptionStrategy; } + /** + * Configures the ACL to apply to the Amazon S3 putObject request. + * @param objectCannedACL + * The ACL to be used when storing objects in Amazon S3 + */ + public void setObjectCannedACL(ObjectCannedACL objectCannedACL) { + this.objectCannedACL = objectCannedACL; + } + + /** + * Configures the ACL to apply to the Amazon S3 putObject request. + * @param objectCannedACL + * The ACL to be used when storing objects in Amazon S3 + */ + public PayloadStorageConfiguration withObjectCannedACL(ObjectCannedACL objectCannedACL) { + setObjectCannedACL(objectCannedACL); + return this; + } + + /** + * Checks whether an ACL have been configured for storing objects in Amazon S3. + * @return True if ACL is defined + */ + public boolean isObjectCannedACLDefined() { + return null != objectCannedACL; + } + + /** + * Gets the AWS ACL to apply to the Amazon S3 putObject request. + * @return Amazon S3 object ACL + */ + public ObjectCannedACL getObjectCannedACL() { + return objectCannedACL; + } } diff --git a/src/main/java/software/amazon/payloadoffloading/S3BackedPayloadStore.java b/src/main/java/software/amazon/payloadoffloading/S3BackedPayloadStore.java index b8eb6c5..99dfaf8 100644 --- a/src/main/java/software/amazon/payloadoffloading/S3BackedPayloadStore.java +++ b/src/main/java/software/amazon/payloadoffloading/S3BackedPayloadStore.java @@ -2,6 +2,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import software.amazon.awssdk.services.s3.model.ObjectCannedACL; import java.util.UUID; @@ -14,15 +15,21 @@ public class S3BackedPayloadStore implements PayloadStore { private final String s3BucketName; private final S3Dao s3Dao; private final ServerSideEncryptionStrategy serverSideEncryptionStrategy; + private final ObjectCannedACL objectCannedACL; public S3BackedPayloadStore(S3Dao s3Dao, String s3BucketName) { this(s3Dao, s3BucketName, null); } public S3BackedPayloadStore(S3Dao s3Dao, String s3BucketName, ServerSideEncryptionStrategy serverSideEncryptionStrategy) { + this(s3Dao, s3BucketName, serverSideEncryptionStrategy, null); + } + + public S3BackedPayloadStore(S3Dao s3Dao, String s3BucketName, ServerSideEncryptionStrategy serverSideEncryptionStrategy, ObjectCannedACL objectCannedACL) { this.s3BucketName = s3BucketName; this.s3Dao = s3Dao; this.serverSideEncryptionStrategy = serverSideEncryptionStrategy; + this.objectCannedACL = objectCannedACL; } @Override @@ -30,7 +37,7 @@ public String storeOriginalPayload(String payload) { String s3Key = UUID.randomUUID().toString(); // Store the payload content in S3. - s3Dao.storeTextInS3(s3BucketName, s3Key, serverSideEncryptionStrategy, payload); + s3Dao.storeTextInS3(s3BucketName, s3Key, serverSideEncryptionStrategy, objectCannedACL, payload); LOG.info("S3 object created, Bucket name: " + s3BucketName + ", Object key: " + s3Key + "."); // Convert S3 pointer (bucket name, key, etc) to JSON string diff --git a/src/main/java/software/amazon/payloadoffloading/S3Dao.java b/src/main/java/software/amazon/payloadoffloading/S3Dao.java index 14d7d75..398a01f 100644 --- a/src/main/java/software/amazon/payloadoffloading/S3Dao.java +++ b/src/main/java/software/amazon/payloadoffloading/S3Dao.java @@ -10,6 +10,7 @@ import software.amazon.awssdk.services.s3.model.DeleteObjectRequest; import software.amazon.awssdk.services.s3.model.GetObjectRequest; import software.amazon.awssdk.services.s3.model.GetObjectResponse; +import software.amazon.awssdk.services.s3.model.ObjectCannedACL; import software.amazon.awssdk.services.s3.model.PutObjectRequest; import software.amazon.awssdk.utils.IoUtils; @@ -56,11 +57,16 @@ public String getTextFromS3(String s3BucketName, String s3Key) { return embeddedText; } - public void storeTextInS3(String s3BucketName, String s3Key, ServerSideEncryptionStrategy serverSideEncryptionStrategy, String payloadContentStr) { + public void storeTextInS3(String s3BucketName, String s3Key, ServerSideEncryptionStrategy serverSideEncryptionStrategy, + ObjectCannedACL objectCannedACL, String payloadContentStr) { PutObjectRequest.Builder putObjectRequestBuilder = PutObjectRequest.builder() .bucket(s3BucketName) .key(s3Key); + if (objectCannedACL != null) { + putObjectRequestBuilder.acl(objectCannedACL); + } + // https://docs.aws.amazon.com/AmazonS3/latest/dev/kms-using-sdks.html if (serverSideEncryptionStrategy != null) { serverSideEncryptionStrategy.decorate(putObjectRequestBuilder); diff --git a/src/test/java/software/amazon/payloadoffloading/PayloadStorageConfigurationTest.java b/src/test/java/software/amazon/payloadoffloading/PayloadStorageConfigurationTest.java index b1dad99..fe5735d 100644 --- a/src/test/java/software/amazon/payloadoffloading/PayloadStorageConfigurationTest.java +++ b/src/test/java/software/amazon/payloadoffloading/PayloadStorageConfigurationTest.java @@ -1,7 +1,9 @@ package software.amazon.payloadoffloading; +import org.junit.Before; import org.junit.Test; import software.amazon.awssdk.services.s3.S3Client; +import software.amazon.awssdk.services.s3.model.ObjectCannedACL; import static org.mockito.Mockito.mock; import static org.junit.Assert.*; @@ -14,6 +16,7 @@ public class PayloadStorageConfigurationTest { private static final String s3BucketName = "test-bucket-name"; private static final String s3ServerSideEncryptionKMSKeyId = "test-customer-managed-kms-key-id"; private static final ServerSideEncryptionStrategy SERVER_SIDE_ENCRYPTION_STRATEGY = ServerSideEncryptionFactory.awsManagedCmk(); + private final ObjectCannedACL objectCannelACL = ObjectCannedACL.BUCKET_OWNER_FULL_CONTROL; @Test public void testCopyConstructor() { @@ -27,7 +30,8 @@ public void testCopyConstructor() { payloadStorageConfiguration.withPayloadSupportEnabled(s3, s3BucketName) .withAlwaysThroughS3(alwaysThroughS3) .withPayloadSizeThreshold(payloadSizeThreshold) - .withServerSideEncryption(SERVER_SIDE_ENCRYPTION_STRATEGY); + .withServerSideEncryption(SERVER_SIDE_ENCRYPTION_STRATEGY) + .withObjectCannedACL(objectCannelACL); PayloadStorageConfiguration newPayloadStorageConfiguration = new PayloadStorageConfiguration(payloadStorageConfiguration); @@ -35,6 +39,7 @@ public void testCopyConstructor() { assertEquals(s3BucketName, newPayloadStorageConfiguration.getS3BucketName()); assertEquals(SERVER_SIDE_ENCRYPTION_STRATEGY, newPayloadStorageConfiguration.getServerSideEncryptionStrategy()); assertTrue(newPayloadStorageConfiguration.isPayloadSupportEnabled()); + assertEquals(objectCannelACL, newPayloadStorageConfiguration.getObjectCannedACL()); assertEquals(alwaysThroughS3, newPayloadStorageConfiguration.isAlwaysThroughS3()); assertEquals(payloadSizeThreshold, newPayloadStorageConfiguration.getPayloadSizeThreshold()); assertNotSame(newPayloadStorageConfiguration, payloadStorageConfiguration); @@ -80,4 +85,15 @@ public void testSseAwsKeyManagementParams() { payloadStorageConfiguration.setServerSideEncryptionStrategy(SERVER_SIDE_ENCRYPTION_STRATEGY); assertEquals(SERVER_SIDE_ENCRYPTION_STRATEGY, payloadStorageConfiguration.getServerSideEncryptionStrategy()); } + + @Test + public void testCannedAccessControlList() { + PayloadStorageConfiguration payloadStorageConfiguration = new PayloadStorageConfiguration(); + + assertFalse(payloadStorageConfiguration.isObjectCannedACLDefined()); + + payloadStorageConfiguration.withObjectCannedACL(objectCannelACL); + assertTrue(payloadStorageConfiguration.isObjectCannedACLDefined()); + assertEquals(objectCannelACL, payloadStorageConfiguration.getObjectCannedACL()); + } } diff --git a/src/test/java/software/amazon/payloadoffloading/S3BackedPayloadStoreTest.java b/src/test/java/software/amazon/payloadoffloading/S3BackedPayloadStoreTest.java index e9e12c1..fb99d62 100644 --- a/src/test/java/software/amazon/payloadoffloading/S3BackedPayloadStoreTest.java +++ b/src/test/java/software/amazon/payloadoffloading/S3BackedPayloadStoreTest.java @@ -11,6 +11,7 @@ import org.mockito.ArgumentCaptor; import software.amazon.awssdk.core.exception.SdkClientException; import software.amazon.awssdk.core.exception.SdkException; +import software.amazon.awssdk.services.s3.model.ObjectCannedACL; import java.util.Objects; @@ -78,9 +79,10 @@ public void testStoreOriginalPayloadOnSuccess(PayloadStore payloadStore, ServerS ArgumentCaptor keyCaptor = ArgumentCaptor.forClass(String.class); ArgumentCaptor sseArgsCaptor = ArgumentCaptor.forClass(ServerSideEncryptionStrategy.class); + ArgumentCaptor cannedArgsCaptor = ArgumentCaptor.forClass(ObjectCannedACL.class); verify(mockS3Dao, times(1)).storeTextInS3(eq(S3_BUCKET_NAME), keyCaptor.capture(), - sseArgsCaptor.capture(), eq(ANY_PAYLOAD)); + sseArgsCaptor.capture(), cannedArgsCaptor.capture(), eq(ANY_PAYLOAD)); PayloadS3Pointer expectedPayloadPointer = new PayloadS3Pointer(S3_BUCKET_NAME, keyCaptor.getValue()); assertEquals(expectedPayloadPointer.toJson(), actualPayloadPointer); @@ -105,9 +107,10 @@ public void testStoreOriginalPayloadDoesAlwaysCreateNewObjects(PayloadStore payl ArgumentCaptor anyOtherKeyCaptor = ArgumentCaptor.forClass(String.class); ArgumentCaptor sseArgsCaptor = ArgumentCaptor.forClass(ServerSideEncryptionStrategy.class); + ArgumentCaptor cannedArgsCaptor = ArgumentCaptor.forClass(ObjectCannedACL.class); verify(mockS3Dao, times(2)).storeTextInS3(eq(S3_BUCKET_NAME), anyOtherKeyCaptor.capture(), - sseArgsCaptor.capture(), eq(ANY_PAYLOAD)); + sseArgsCaptor.capture(), cannedArgsCaptor.capture(), eq(ANY_PAYLOAD)); String anyS3Key = anyOtherKeyCaptor.getAllValues().get(0); String anyOtherS3Key = anyOtherKeyCaptor.getAllValues().get(1); @@ -139,6 +142,7 @@ public void testStoreOriginalPayloadOnS3Failure(PayloadStore payloadStore, Serve any(String.class), // Can be String or null any(), + any(), any(String.class)); exception.expect(SdkException.class); From 9da00fdd4d1d6ad9bc3a2628f81460e21c452e9f Mon Sep 17 00:00:00 2001 From: msailes Date: Fri, 9 Oct 2020 22:22:16 +0100 Subject: [PATCH 2/3] feat: Added support for object canned access control lists. --- README.md | 2 +- .../S3BackedPayloadStore.java | 16 +--- .../amazon/payloadoffloading/S3Dao.java | 11 ++- .../S3BackedPayloadStoreTest.java | 75 ++----------------- .../amazon/payloadoffloading/S3DaoTest.java | 75 +++++++++++++++++++ 5 files changed, 94 insertions(+), 85 deletions(-) create mode 100644 src/test/java/software/amazon/payloadoffloading/S3DaoTest.java diff --git a/README.md b/README.md index 3307474..6eb8ba2 100644 --- a/README.md +++ b/README.md @@ -33,7 +33,7 @@ You can download release builds through the [releases section of this](https://g software.amazon.payloadoffloading payloadoffloading-common - 1.0.0 + 1.1.0 ``` diff --git a/src/main/java/software/amazon/payloadoffloading/S3BackedPayloadStore.java b/src/main/java/software/amazon/payloadoffloading/S3BackedPayloadStore.java index 99dfaf8..0cb13cd 100644 --- a/src/main/java/software/amazon/payloadoffloading/S3BackedPayloadStore.java +++ b/src/main/java/software/amazon/payloadoffloading/S3BackedPayloadStore.java @@ -2,7 +2,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import software.amazon.awssdk.services.s3.model.ObjectCannedACL; import java.util.UUID; @@ -14,30 +13,17 @@ public class S3BackedPayloadStore implements PayloadStore { private final String s3BucketName; private final S3Dao s3Dao; - private final ServerSideEncryptionStrategy serverSideEncryptionStrategy; - private final ObjectCannedACL objectCannedACL; public S3BackedPayloadStore(S3Dao s3Dao, String s3BucketName) { - this(s3Dao, s3BucketName, null); - } - - public S3BackedPayloadStore(S3Dao s3Dao, String s3BucketName, ServerSideEncryptionStrategy serverSideEncryptionStrategy) { - this(s3Dao, s3BucketName, serverSideEncryptionStrategy, null); - } - - public S3BackedPayloadStore(S3Dao s3Dao, String s3BucketName, ServerSideEncryptionStrategy serverSideEncryptionStrategy, ObjectCannedACL objectCannedACL) { this.s3BucketName = s3BucketName; this.s3Dao = s3Dao; - this.serverSideEncryptionStrategy = serverSideEncryptionStrategy; - this.objectCannedACL = objectCannedACL; } @Override public String storeOriginalPayload(String payload) { String s3Key = UUID.randomUUID().toString(); - // Store the payload content in S3. - s3Dao.storeTextInS3(s3BucketName, s3Key, serverSideEncryptionStrategy, objectCannedACL, payload); + s3Dao.storeTextInS3(s3BucketName, s3Key, payload); LOG.info("S3 object created, Bucket name: " + s3BucketName + ", Object key: " + s3Key + "."); // Convert S3 pointer (bucket name, key, etc) to JSON string diff --git a/src/main/java/software/amazon/payloadoffloading/S3Dao.java b/src/main/java/software/amazon/payloadoffloading/S3Dao.java index 398a01f..2b03dd5 100644 --- a/src/main/java/software/amazon/payloadoffloading/S3Dao.java +++ b/src/main/java/software/amazon/payloadoffloading/S3Dao.java @@ -22,9 +22,17 @@ public class S3Dao { private static final Logger LOG = LoggerFactory.getLogger(S3Dao.class); private final S3Client s3Client; + private final ServerSideEncryptionStrategy serverSideEncryptionStrategy; + private final ObjectCannedACL objectCannedACL; public S3Dao(S3Client s3Client) { + this(s3Client, null, null); + } + + public S3Dao(S3Client s3Client, ServerSideEncryptionStrategy serverSideEncryptionStrategy, ObjectCannedACL objectCannedACL) { this.s3Client = s3Client; + this.serverSideEncryptionStrategy = serverSideEncryptionStrategy; + this.objectCannedACL = objectCannedACL; } public String getTextFromS3(String s3BucketName, String s3Key) { @@ -57,8 +65,7 @@ public String getTextFromS3(String s3BucketName, String s3Key) { return embeddedText; } - public void storeTextInS3(String s3BucketName, String s3Key, ServerSideEncryptionStrategy serverSideEncryptionStrategy, - ObjectCannedACL objectCannedACL, String payloadContentStr) { + public void storeTextInS3(String s3BucketName, String s3Key, String payloadContentStr) { PutObjectRequest.Builder putObjectRequestBuilder = PutObjectRequest.builder() .bucket(s3BucketName) .key(s3Key); diff --git a/src/test/java/software/amazon/payloadoffloading/S3BackedPayloadStoreTest.java b/src/test/java/software/amazon/payloadoffloading/S3BackedPayloadStoreTest.java index fb99d62..c3209a2 100644 --- a/src/test/java/software/amazon/payloadoffloading/S3BackedPayloadStoreTest.java +++ b/src/test/java/software/amazon/payloadoffloading/S3BackedPayloadStoreTest.java @@ -1,7 +1,6 @@ package software.amazon.payloadoffloading; import junitparams.JUnitParamsRunner; -import junitparams.Parameters; import org.hamcrest.Matchers; import org.junit.Before; import org.junit.Rule; @@ -13,8 +12,6 @@ import software.amazon.awssdk.core.exception.SdkException; import software.amazon.awssdk.services.s3.model.ObjectCannedACL; -import java.util.Objects; - import static org.junit.Assert.*; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.*; @@ -22,13 +19,9 @@ @RunWith(JUnitParamsRunner.class) public class S3BackedPayloadStoreTest { private static final String S3_BUCKET_NAME = "test-bucket-name"; - private static final String S3_SERVER_SIDE_ENCRYPTION_KMS_KEY_ID = "test-customer-managed-kms-key-id"; - private static final ServerSideEncryptionStrategy KMS_WITH_CUSTOMER_KEY = ServerSideEncryptionFactory.customerKey(S3_SERVER_SIDE_ENCRYPTION_KMS_KEY_ID); - private static final ServerSideEncryptionStrategy KMS_WITH_AWS_MANAGED_CMK = ServerSideEncryptionFactory.awsManagedCmk(); private static final String ANY_PAYLOAD = "AnyPayload"; private static final String ANY_S3_KEY = "AnyS3key"; private static final String INCORRECT_POINTER_EXCEPTION_MSG = "Failed to read the S3 object pointer from given string"; - private static final Long ANY_PAYLOAD_LENGTH = 300000L; private PayloadStore payloadStore; private S3Dao s3Dao; @@ -41,64 +34,23 @@ public void setup() { payloadStore = new S3BackedPayloadStore(s3Dao, S3_BUCKET_NAME); } - private Object[] testData() { - // Here, we create separate mock of S3Dao because JUnitParamsRunner collects parameters - // for tests well before invocation of @Before or @BeforeClass methods. - // That means our default s3Dao mock isn't instantiated until then. For parameterized tests, - // we instantiate our local S3Dao mock per combination, pass it to S3BackedPayloadStore and also pass it - // as test parameter to allow verifying calls to the mockS3Dao. - S3Dao noEncryptionS3Dao = mock(S3Dao.class); - S3Dao defaultEncryptionS3Dao = mock(S3Dao.class); - S3Dao customerKMSKeyEncryptionS3Dao = mock(S3Dao.class); - return new Object[][]{ - // No S3 SSE-KMS encryption - { - new S3BackedPayloadStore(noEncryptionS3Dao, S3_BUCKET_NAME), - null, - noEncryptionS3Dao - }, - // S3 SSE-KMS encryption with AWS managed KMS keys - { - new S3BackedPayloadStore(defaultEncryptionS3Dao, S3_BUCKET_NAME, KMS_WITH_AWS_MANAGED_CMK), - KMS_WITH_AWS_MANAGED_CMK, - defaultEncryptionS3Dao - }, - // S3 SSE-KMS encryption with customer managed KMS key - { - new S3BackedPayloadStore(customerKMSKeyEncryptionS3Dao, S3_BUCKET_NAME, KMS_WITH_CUSTOMER_KEY), - KMS_WITH_CUSTOMER_KEY, - customerKMSKeyEncryptionS3Dao - } - }; - } - @Test - @Parameters(method = "testData") - public void testStoreOriginalPayloadOnSuccess(PayloadStore payloadStore, ServerSideEncryptionStrategy expectedParams, S3Dao mockS3Dao) { + public void testStoreOriginalPayloadOnSuccess() { String actualPayloadPointer = payloadStore.storeOriginalPayload(ANY_PAYLOAD); ArgumentCaptor keyCaptor = ArgumentCaptor.forClass(String.class); ArgumentCaptor sseArgsCaptor = ArgumentCaptor.forClass(ServerSideEncryptionStrategy.class); ArgumentCaptor cannedArgsCaptor = ArgumentCaptor.forClass(ObjectCannedACL.class); - verify(mockS3Dao, times(1)).storeTextInS3(eq(S3_BUCKET_NAME), keyCaptor.capture(), - sseArgsCaptor.capture(), cannedArgsCaptor.capture(), eq(ANY_PAYLOAD)); + verify(s3Dao, times(1)).storeTextInS3(eq(S3_BUCKET_NAME), keyCaptor.capture(), + eq(ANY_PAYLOAD)); PayloadS3Pointer expectedPayloadPointer = new PayloadS3Pointer(S3_BUCKET_NAME, keyCaptor.getValue()); assertEquals(expectedPayloadPointer.toJson(), actualPayloadPointer); - - if (expectedParams == null) { - assertTrue(sseArgsCaptor.getValue() == null); - } else { - assertEquals(expectedParams, sseArgsCaptor.getValue()); - } } @Test - @Parameters(method = "testData") - public void testStoreOriginalPayloadDoesAlwaysCreateNewObjects(PayloadStore payloadStore, - ServerSideEncryptionStrategy expectedParams, - S3Dao mockS3Dao) { + public void testStoreOriginalPayloadDoesAlwaysCreateNewObjects() { //Store any payload String anyActualPayloadPointer = payloadStore.storeOriginalPayload(ANY_PAYLOAD); @@ -109,8 +61,8 @@ public void testStoreOriginalPayloadDoesAlwaysCreateNewObjects(PayloadStore payl ArgumentCaptor sseArgsCaptor = ArgumentCaptor.forClass(ServerSideEncryptionStrategy.class); ArgumentCaptor cannedArgsCaptor = ArgumentCaptor.forClass(ObjectCannedACL.class); - verify(mockS3Dao, times(2)).storeTextInS3(eq(S3_BUCKET_NAME), anyOtherKeyCaptor.capture(), - sseArgsCaptor.capture(), cannedArgsCaptor.capture(), eq(ANY_PAYLOAD)); + verify(s3Dao, times(2)).storeTextInS3(eq(S3_BUCKET_NAME), anyOtherKeyCaptor.capture(), + eq(ANY_PAYLOAD)); String anyS3Key = anyOtherKeyCaptor.getAllValues().get(0); String anyOtherS3Key = anyOtherKeyCaptor.getAllValues().get(1); @@ -123,26 +75,15 @@ public void testStoreOriginalPayloadDoesAlwaysCreateNewObjects(PayloadStore payl assertThat(anyS3Key, Matchers.not(anyOtherS3Key)); assertThat(anyActualPayloadPointer, Matchers.not(anyOtherActualPayloadPointer)); - - if (expectedParams == null) { - assertTrue(sseArgsCaptor.getAllValues().stream().allMatch(Objects::isNull)); - } else { - assertTrue(sseArgsCaptor.getAllValues().stream().allMatch(actualParams -> - actualParams.equals(expectedParams))); - } } @Test - @Parameters(method = "testData") - public void testStoreOriginalPayloadOnS3Failure(PayloadStore payloadStore, ServerSideEncryptionStrategy awsKmsKeyId, S3Dao mockS3Dao) { + public void testStoreOriginalPayloadOnS3Failure() { doThrow(SdkException.create("S3 Exception", new Throwable())) - .when(mockS3Dao) + .when(s3Dao) .storeTextInS3( any(String.class), any(String.class), - // Can be String or null - any(), - any(), any(String.class)); exception.expect(SdkException.class); diff --git a/src/test/java/software/amazon/payloadoffloading/S3DaoTest.java b/src/test/java/software/amazon/payloadoffloading/S3DaoTest.java new file mode 100644 index 0000000..2feeda5 --- /dev/null +++ b/src/test/java/software/amazon/payloadoffloading/S3DaoTest.java @@ -0,0 +1,75 @@ +package software.amazon.payloadoffloading; + +import software.amazon.awssdk.core.sync.RequestBody; +import software.amazon.awssdk.services.s3.S3Client; +import software.amazon.awssdk.services.s3.model.ObjectCannedACL; +import junitparams.JUnitParamsRunner; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import software.amazon.awssdk.services.s3.model.PutObjectRequest; +import software.amazon.awssdk.services.s3.model.ServerSideEncryption; + +import static org.junit.Assert.*; +import static org.mockito.Mockito.*; + +@RunWith(JUnitParamsRunner.class) +public class S3DaoTest { + + private static String s3ServerSideEncryptionKMSKeyId = "test-customer-managed-kms-key-id"; + private static final String S3_BUCKET_NAME = "test-bucket-name"; + private static final String ANY_PAYLOAD = "AnyPayload"; + private static final String ANY_S3_KEY = "AnyS3key"; + private ServerSideEncryptionStrategy serverSideEncryptionStrategy = ServerSideEncryptionFactory.awsManagedCmk(); + private ObjectCannedACL objectCannedACL = ObjectCannedACL.PUBLIC_READ; + private S3Client s3Client; + private S3Dao dao; + + @Before + public void setup() { + s3Client = mock(S3Client.class); + } + + @Test + public void storeTextInS3WithoutSSEOrCannedTest() { + dao = new S3Dao(s3Client); + ArgumentCaptor argument = ArgumentCaptor.forClass(PutObjectRequest.class); + + dao.storeTextInS3(S3_BUCKET_NAME, ANY_S3_KEY, ANY_PAYLOAD); + + verify(s3Client, times(1)).putObject(argument.capture(), any(RequestBody.class)); + + assertNull(argument.getValue().serverSideEncryption()); + assertNull(argument.getValue().acl()); + assertEquals(S3_BUCKET_NAME, argument.getValue().bucket()); + } + + @Test + public void storeTextInS3WithSSETest() { + dao = new S3Dao(s3Client, serverSideEncryptionStrategy, null); + ArgumentCaptor argument = ArgumentCaptor.forClass(PutObjectRequest.class); + + dao.storeTextInS3(S3_BUCKET_NAME, ANY_S3_KEY, ANY_PAYLOAD); + + verify(s3Client, times(1)).putObject(argument.capture(), any(RequestBody.class)); + + assertEquals(ServerSideEncryption.AWS_KMS, argument.getValue().serverSideEncryption()); + assertNull(argument.getValue().acl()); + assertEquals(S3_BUCKET_NAME, argument.getValue().bucket()); + } + + @Test + public void storeTextInS3WithBothTest() { + dao = new S3Dao(s3Client, serverSideEncryptionStrategy, objectCannedACL); + ArgumentCaptor argument = ArgumentCaptor.forClass(PutObjectRequest.class); + + dao.storeTextInS3(S3_BUCKET_NAME, ANY_S3_KEY, ANY_PAYLOAD); + + verify(s3Client, times(1)).putObject(argument.capture(), any(RequestBody.class)); + + assertEquals(ServerSideEncryption.AWS_KMS, argument.getValue().serverSideEncryption()); + assertEquals(objectCannedACL, argument.getValue().acl()); + assertEquals(S3_BUCKET_NAME, argument.getValue().bucket()); + } +} \ No newline at end of file From 98f12a6e52fea52d7cc1c31fa16a8cba7d81461f Mon Sep 17 00:00:00 2001 From: msailes Date: Thu, 15 Oct 2020 09:53:53 +0100 Subject: [PATCH 3/3] code review feedback --- .../PayloadStorageConfigurationTest.java | 2 -- .../java/software/amazon/payloadoffloading/S3DaoTest.java | 8 ++++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/test/java/software/amazon/payloadoffloading/PayloadStorageConfigurationTest.java b/src/test/java/software/amazon/payloadoffloading/PayloadStorageConfigurationTest.java index fe5735d..e9b473a 100644 --- a/src/test/java/software/amazon/payloadoffloading/PayloadStorageConfigurationTest.java +++ b/src/test/java/software/amazon/payloadoffloading/PayloadStorageConfigurationTest.java @@ -1,6 +1,5 @@ package software.amazon.payloadoffloading; -import org.junit.Before; import org.junit.Test; import software.amazon.awssdk.services.s3.S3Client; import software.amazon.awssdk.services.s3.model.ObjectCannedACL; @@ -14,7 +13,6 @@ public class PayloadStorageConfigurationTest { private static final String s3BucketName = "test-bucket-name"; - private static final String s3ServerSideEncryptionKMSKeyId = "test-customer-managed-kms-key-id"; private static final ServerSideEncryptionStrategy SERVER_SIDE_ENCRYPTION_STRATEGY = ServerSideEncryptionFactory.awsManagedCmk(); private final ObjectCannedACL objectCannelACL = ObjectCannedACL.BUCKET_OWNER_FULL_CONTROL; diff --git a/src/test/java/software/amazon/payloadoffloading/S3DaoTest.java b/src/test/java/software/amazon/payloadoffloading/S3DaoTest.java index 2feeda5..22274d5 100644 --- a/src/test/java/software/amazon/payloadoffloading/S3DaoTest.java +++ b/src/test/java/software/amazon/payloadoffloading/S3DaoTest.java @@ -11,8 +11,12 @@ import software.amazon.awssdk.services.s3.model.PutObjectRequest; import software.amazon.awssdk.services.s3.model.ServerSideEncryption; -import static org.junit.Assert.*; -import static org.mockito.Mockito.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; @RunWith(JUnitParamsRunner.class) public class S3DaoTest {