From 0406d2bd4e31329bfb2b8a10b97a21cebeaf4d79 Mon Sep 17 00:00:00 2001 From: gruebel Date: Tue, 27 Oct 2020 17:58:28 +0100 Subject: [PATCH 1/7] Add tag support to LogGroup resource --- aws-logs-loggroup/aws-logs-loggroup.json | 48 ++- aws-logs-loggroup/resource-role.yaml | 3 + .../amazon/logs/loggroup/ListHandler.java | 15 +- .../amazon/logs/loggroup/ReadHandler.java | 8 +- .../amazon/logs/loggroup/Translator.java | 52 +++- .../amazon/logs/loggroup/UpdateHandler.java | 53 +++- .../logs/loggroup/CreateHandlerTest.java | 11 +- .../amazon/logs/loggroup/ListHandlerTest.java | 31 ++ .../amazon/logs/loggroup/ReadHandlerTest.java | 22 +- .../amazon/logs/loggroup/TranslatorTest.java | 115 ++++++- .../logs/loggroup/UpdateHandlerTest.java | 282 ++++++++++++++++-- aws-logs-loggroup/template.yml | 1 + 12 files changed, 592 insertions(+), 49 deletions(-) diff --git a/aws-logs-loggroup/aws-logs-loggroup.json b/aws-logs-loggroup/aws-logs-loggroup.json index 646543b..5364a9c 100644 --- a/aws-logs-loggroup/aws-logs-loggroup.json +++ b/aws-logs-loggroup/aws-logs-loggroup.json @@ -2,6 +2,32 @@ "typeName": "AWS::Logs::LogGroup", "description": "Resource schema for AWS::Logs::LogGroup", "sourceUrl": "https://github.com/aws-cloudformation/aws-cloudformation-resource-providers-logs.git", + "definitions": { + "Tag": { + "description": "A key-value pair to associate with a resource.", + "type": "object", + "properties": { + "Key": { + "type": "string", + "description": "The key name of the tag. You can specify a value that is 1 to 128 Unicode characters in length and cannot be prefixed with aws:. You can use any of the following characters: the set of Unicode letters, digits, whitespace, _, ., :, /, =, +, - and @.", + "minLength": 1, + "maxLength": 128, + "pattern": "^([\\w\\s\\d_.:/=+\\-@]+)$" + }, + "Value": { + "type": "string", + "description": "The value for the tag. You can specify a value that is 0 to 256 Unicode characters in length. You can use any of the following characters: the set of Unicode letters, digits, whitespace, _, ., :, /, =, +, - and @.", + "minLength": 0, + "maxLength": 256, + "pattern": "^([\\w\\s\\d_.:/=+\\-@]*)$" + } + }, + "required": [ + "Key", + "Value" + ] + } + }, "properties": { "LogGroupName": { "description": "The name of the log group. If you don't specify a name, AWS CloudFormation generates a unique ID for the log group.", @@ -39,6 +65,15 @@ 3653 ] }, + "Tags": { + "description": "An array of key-value pairs to apply to this resource.", + "type": "array", + "uniqueItems": true, + "insertionOrder": false, + "items": { + "$ref": "#/definitions/Tag" + } + }, "Arn": { "description": "The CloudWatch log group ARN.", "type": "string" @@ -49,12 +84,14 @@ "permissions": [ "logs:DescribeLogGroups", "logs:CreateLogGroup", - "logs:PutRetentionPolicy" + "logs:PutRetentionPolicy", + "logs:TagLogGroup" ] }, "read": { "permissions": [ - "logs:DescribeLogGroups" + "logs:DescribeLogGroups", + "logs:ListTagsLogGroup" ] }, "update": { @@ -63,7 +100,9 @@ "logs:AssociateKmsKey", "logs:DisassociateKmsKey", "logs:PutRetentionPolicy", - "logs:DeleteRetentionPolicy" + "logs:DeleteRetentionPolicy", + "logs:TagLogGroup", + "logs:UntagLogGroup" ] }, "delete": { @@ -74,7 +113,8 @@ }, "list": { "permissions": [ - "logs:DescribeLogGroups" + "logs:DescribeLogGroups", + "logs:ListTagsLogGroup" ] } }, diff --git a/aws-logs-loggroup/resource-role.yaml b/aws-logs-loggroup/resource-role.yaml index 57a3f2d..4e97078 100644 --- a/aws-logs-loggroup/resource-role.yaml +++ b/aws-logs-loggroup/resource-role.yaml @@ -29,7 +29,10 @@ Resources: - "logs:DeleteRetentionPolicy" - "logs:DescribeLogGroups" - "logs:DisassociateKmsKey" + - "logs:ListTagsLogGroup" - "logs:PutRetentionPolicy" + - "logs:TagLogGroup" + - "logs:UntagLogGroup" Resource: "*" Outputs: ExecutionRoleArn: diff --git a/aws-logs-loggroup/src/main/java/software/amazon/logs/loggroup/ListHandler.java b/aws-logs-loggroup/src/main/java/software/amazon/logs/loggroup/ListHandler.java index 24033b6..442b965 100644 --- a/aws-logs-loggroup/src/main/java/software/amazon/logs/loggroup/ListHandler.java +++ b/aws-logs-loggroup/src/main/java/software/amazon/logs/loggroup/ListHandler.java @@ -1,5 +1,7 @@ package software.amazon.logs.loggroup; +import software.amazon.awssdk.services.cloudwatchlogs.model.ListTagsLogGroupResponse; +import software.amazon.awssdk.services.cloudwatchlogs.model.LogGroup; import software.amazon.cloudformation.proxy.AmazonWebServicesClientProxy; import software.amazon.cloudformation.proxy.Logger; import software.amazon.cloudformation.proxy.OperationStatus; @@ -7,6 +9,9 @@ import software.amazon.cloudformation.proxy.ResourceHandlerRequest; import software.amazon.awssdk.services.cloudwatchlogs.model.DescribeLogGroupsResponse; +import java.util.Map; +import java.util.stream.Collectors; + public class ListHandler extends BaseHandler { @Override @@ -19,9 +24,17 @@ public ProgressEvent handleRequest( final DescribeLogGroupsResponse response = proxy.injectCredentialsAndInvokeV2(Translator.translateToListRequest(request.getNextToken()), ClientBuilder.getClient()::describeLogGroups); + + final Map tagResponses = Translator.streamOfOrEmpty(response.logGroups()) + .collect(Collectors.toMap( + LogGroup::logGroupName, + logGroup -> proxy.injectCredentialsAndInvokeV2(Translator.translateToListTagsLogGroupRequest(logGroup.logGroupName()), + ClientBuilder.getClient()::listTagsLogGroup)) + ); + return ProgressEvent.builder() .status(OperationStatus.SUCCESS) - .resourceModels(Translator.translateForList(response)) + .resourceModels(Translator.translateForList(response, tagResponses)) .nextToken(response.nextToken()) .build(); } diff --git a/aws-logs-loggroup/src/main/java/software/amazon/logs/loggroup/ReadHandler.java b/aws-logs-loggroup/src/main/java/software/amazon/logs/loggroup/ReadHandler.java index 5a52b11..52d357f 100644 --- a/aws-logs-loggroup/src/main/java/software/amazon/logs/loggroup/ReadHandler.java +++ b/aws-logs-loggroup/src/main/java/software/amazon/logs/loggroup/ReadHandler.java @@ -1,5 +1,6 @@ package software.amazon.logs.loggroup; +import software.amazon.awssdk.services.cloudwatchlogs.model.ListTagsLogGroupResponse; import software.amazon.cloudformation.proxy.AmazonWebServicesClientProxy; import software.amazon.cloudformation.proxy.Logger; import software.amazon.cloudformation.proxy.ProgressEvent; @@ -25,14 +26,17 @@ public ProgressEvent handleRequest( } DescribeLogGroupsResponse response = null; + ListTagsLogGroupResponse tagsResponse = null; try { response = proxy.injectCredentialsAndInvokeV2(Translator.translateToReadRequest(model), - ClientBuilder.getClient()::describeLogGroups); + ClientBuilder.getClient()::describeLogGroups); + tagsResponse = proxy.injectCredentialsAndInvokeV2(Translator.translateToListTagsLogGroupRequest(model.getLogGroupName()), + ClientBuilder.getClient()::listTagsLogGroup); } catch (final ResourceNotFoundException e) { throwNotFoundException(model); } - final ResourceModel modelFromReadResult = Translator.translateForRead(response); + final ResourceModel modelFromReadResult = Translator.translateForRead(response, tagsResponse); if (modelFromReadResult.getLogGroupName() == null) { throwNotFoundException(model); } diff --git a/aws-logs-loggroup/src/main/java/software/amazon/logs/loggroup/Translator.java b/aws-logs-loggroup/src/main/java/software/amazon/logs/loggroup/Translator.java index 66d333d..472b747 100644 --- a/aws-logs-loggroup/src/main/java/software/amazon/logs/loggroup/Translator.java +++ b/aws-logs-loggroup/src/main/java/software/amazon/logs/loggroup/Translator.java @@ -5,14 +5,21 @@ import software.amazon.awssdk.services.cloudwatchlogs.model.DeleteRetentionPolicyRequest; import software.amazon.awssdk.services.cloudwatchlogs.model.DescribeLogGroupsRequest; import software.amazon.awssdk.services.cloudwatchlogs.model.DescribeLogGroupsResponse; +import software.amazon.awssdk.services.cloudwatchlogs.model.ListTagsLogGroupRequest; +import software.amazon.awssdk.services.cloudwatchlogs.model.ListTagsLogGroupResponse; import software.amazon.awssdk.services.cloudwatchlogs.model.PutRetentionPolicyRequest; import software.amazon.awssdk.services.cloudwatchlogs.model.DisassociateKmsKeyRequest; import software.amazon.awssdk.services.cloudwatchlogs.model.AssociateKmsKeyRequest; +import software.amazon.awssdk.services.cloudwatchlogs.model.TagLogGroupRequest; +import software.amazon.awssdk.services.cloudwatchlogs.model.UntagLogGroupRequest; import java.util.Collection; +import java.util.Collections; import java.util.List; +import java.util.Map; import java.util.Objects; import java.util.Optional; +import java.util.Set; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -42,6 +49,7 @@ static CreateLogGroupRequest translateToCreateRequest(final ResourceModel model) return CreateLogGroupRequest.builder() .logGroupName(model.getLogGroupName()) .kmsKeyId(model.getKmsKeyId()) + .tags(translateTagsToSdk(model.getTags())) .build(); } @@ -71,7 +79,27 @@ static AssociateKmsKeyRequest translateToAssociateKmsKeyRequest(final ResourceMo .build(); } - static ResourceModel translateForRead(final DescribeLogGroupsResponse response) { + static ListTagsLogGroupRequest translateToListTagsLogGroupRequest(final String logGroupName) { + return ListTagsLogGroupRequest.builder() + .logGroupName(logGroupName) + .build(); + } + + static TagLogGroupRequest translateToTagLogGroupRequest(final String logGroupName, final Set tags) { + return TagLogGroupRequest.builder() + .logGroupName(logGroupName) + .tags(translateTagsToSdk(tags)) + .build(); + } + + static UntagLogGroupRequest translateToUntagLogGroupRequest(final String logGroupName, final List tagKeys) { + return UntagLogGroupRequest.builder() + .logGroupName(logGroupName) + .tags(tagKeys) + .build(); + } + + static ResourceModel translateForRead(final DescribeLogGroupsResponse response, final ListTagsLogGroupResponse tagsResponse) { final String logGroupName = streamOfOrEmpty(response.logGroups()) .map(software.amazon.awssdk.services.cloudwatchlogs.model.LogGroup::logGroupName) .filter(Objects::nonNull) @@ -92,26 +120,29 @@ static ResourceModel translateForRead(final DescribeLogGroupsResponse response) .filter(Objects::nonNull) .findAny() .orElse(null); + final Set tags = translateSdkToTags(tagsResponse.tags()); return ResourceModel.builder() .arn(logGroupArn) .logGroupName(logGroupName) .retentionInDays(retentionInDays) .kmsKeyId(kmsKeyId) + .tags(tags) .build(); } - static List translateForList(final DescribeLogGroupsResponse response) { + static List translateForList(final DescribeLogGroupsResponse response, final Map tagResponses) { return streamOfOrEmpty(response.logGroups()) .map(logGroup -> ResourceModel.builder() .arn(logGroup.arn()) .logGroupName(logGroup.logGroupName()) .retentionInDays(logGroup.retentionInDays()) .kmsKeyId(logGroup.kmsKeyId()) + .tags(translateSdkToTags(tagResponses.get(logGroup.logGroupName()).tags())) .build()) .collect(Collectors.toList()); } - private static Stream streamOfOrEmpty(final Collection collection) { + static Stream streamOfOrEmpty(final Collection collection) { return Optional.ofNullable(collection) .map(Collection::stream) .orElseGet(Stream::empty); @@ -128,4 +159,19 @@ static String buildResourceDoesNotExistErrorMessage(final String resourceIdentif ResourceModel.TYPE_NAME, resourceIdentifier); } + + static Map translateTagsToSdk(final Set tags) { + if (tags == null || tags.isEmpty()) { + return null; + } + return tags.stream().collect(Collectors.toMap(Tag::getKey, Tag::getValue)); + } + + static Set translateSdkToTags(final Map tags) { + if (tags == null || tags.isEmpty()) { + return null; + } + return tags.entrySet().stream().map(tag -> new Tag(tag.getKey(), tag.getValue())) + .collect(Collectors.toSet()); + } } diff --git a/aws-logs-loggroup/src/main/java/software/amazon/logs/loggroup/UpdateHandler.java b/aws-logs-loggroup/src/main/java/software/amazon/logs/loggroup/UpdateHandler.java index d95e015..d262b2b 100644 --- a/aws-logs-loggroup/src/main/java/software/amazon/logs/loggroup/UpdateHandler.java +++ b/aws-logs-loggroup/src/main/java/software/amazon/logs/loggroup/UpdateHandler.java @@ -1,5 +1,6 @@ package software.amazon.logs.loggroup; +import com.google.common.collect.Sets; import software.amazon.cloudformation.exceptions.CfnInternalFailureException; import software.amazon.cloudformation.exceptions.CfnResourceConflictException; import software.amazon.cloudformation.exceptions.CfnServiceInternalErrorException; @@ -16,7 +17,12 @@ import software.amazon.awssdk.services.cloudwatchlogs.model.ResourceNotFoundException; import software.amazon.awssdk.services.cloudwatchlogs.model.ServiceUnavailableException; +import java.util.HashSet; +import java.util.List; import java.util.Objects; +import java.util.Optional; +import java.util.Set; +import java.util.stream.Collectors; public class UpdateHandler extends BaseHandler { @@ -27,11 +33,12 @@ public ProgressEvent handleRequest( final CallbackContext callbackContext, final Logger logger) { - // Everything except RetentionPolicyInDays and KmsKeyId is createOnly + // Everything except RetentionPolicyInDays, KmsKeyId and Tags is createOnly final ResourceModel model = request.getDesiredResourceState(); final ResourceModel previousModel = request.getPreviousResourceState(); final boolean retentionChanged = ! retentionUnchanged(previousModel, model); final boolean kmsKeyChanged = ! kmsKeyUnchanged(previousModel, model); + final boolean tagsChanged = ! tagsUnchanged(previousModel, model); if (retentionChanged && model.getRetentionInDays() == null) { deleteRetentionPolicy(proxy, request, logger); } else if (retentionChanged){ @@ -47,6 +54,10 @@ public ProgressEvent handleRequest( associateKmsKey(proxy, request, logger); } + if (tagsChanged) { + updateTags(proxy, previousModel, model, logger); + } + return ProgressEvent.defaultSuccessHandler(model); } @@ -148,6 +159,39 @@ private void associateKmsKey(final AmazonWebServicesClientProxy proxy, logger.log(kmsKeyMessage); } + private void updateTags(final AmazonWebServicesClientProxy proxy, + final ResourceModel previousModel, + final ResourceModel model, + final Logger logger) { + final Set previousTags = Optional.ofNullable(previousModel).map(ResourceModel::getTags).orElse(new HashSet<>()); + final Set newTags = Optional.ofNullable(model.getTags()).orElse(new HashSet<>()); + final Set tagsToRemove = Sets.difference(previousTags, newTags); + final Set tagsToAdd = Sets.difference(newTags, previousTags); + try { + if (!tagsToRemove.isEmpty()) { + final List tagKeys = tagsToRemove.stream().map(Tag::getKey).collect(Collectors.toList()); + proxy.injectCredentialsAndInvokeV2(Translator.translateToUntagLogGroupRequest(model.getLogGroupName(), tagKeys), + ClientBuilder.getClient()::untagLogGroup); + + final String message = + String.format("%s [%s] successfully removed tags: [%s]", + ResourceModel.TYPE_NAME, model.getLogGroupName(), tagKeys); + logger.log(message); + } + if(!tagsToAdd.isEmpty()) { + proxy.injectCredentialsAndInvokeV2(Translator.translateToTagLogGroupRequest(model.getLogGroupName(), tagsToAdd), + ClientBuilder.getClient()::tagLogGroup); + + final String message = + String.format("%s [%s] successfully added tags: [%s]", + ResourceModel.TYPE_NAME, model.getLogGroupName(), tagsToAdd); + logger.log(message); + } + } catch (final ResourceNotFoundException e) { + throwNotFoundException(model); + } + } + private void throwNotFoundException(final ResourceModel model) { throw new software.amazon.cloudformation.exceptions.ResourceNotFoundException(ResourceModel.TYPE_NAME, Objects.toString(model.getPrimaryIdentifier())); @@ -161,4 +205,11 @@ private static boolean retentionUnchanged(final ResourceModel previousModel, fin private static boolean kmsKeyUnchanged(final ResourceModel previousModel, final ResourceModel model) { return (previousModel != null && Objects.equals(model.getKmsKeyId(), previousModel.getKmsKeyId())); } + + private static boolean tagsUnchanged(final ResourceModel previousModel, final ResourceModel model) { + if (previousModel == null && model.getTags() == null) { + return true; + } + return (previousModel != null && Objects.equals(model.getTags(), previousModel.getTags())); + } } diff --git a/aws-logs-loggroup/src/test/java/software/amazon/logs/loggroup/CreateHandlerTest.java b/aws-logs-loggroup/src/test/java/software/amazon/logs/loggroup/CreateHandlerTest.java index b334e70..db1c92a 100644 --- a/aws-logs-loggroup/src/test/java/software/amazon/logs/loggroup/CreateHandlerTest.java +++ b/aws-logs-loggroup/src/test/java/software/amazon/logs/loggroup/CreateHandlerTest.java @@ -18,9 +18,12 @@ import software.amazon.awssdk.services.cloudwatchlogs.model.PutRetentionPolicyResponse; import software.amazon.awssdk.services.cloudwatchlogs.model.ResourceAlreadyExistsException; +import java.util.Arrays; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.Map; +import java.util.Set; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -55,6 +58,10 @@ public void handleRequest_Success() { .retentionInDays(1) .kmsKeyId("arn:aws:kms:us-east-1:$123456789012:key/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa") .build(); + final Set tags = new HashSet<>(Arrays.asList( + Tag.builder().key("key-1").value("value-1").build(), + Tag.builder().key("key-2").value("value-2").build() + )); doReturn(describeResponseInitial, createLogGroupResponse, putRetentionPolicyResponse) .when(proxy) @@ -67,6 +74,7 @@ public void handleRequest_Success() { .logGroupName("LogGroup") .retentionInDays(1) .kmsKeyId("arn:aws:kms:us-east-1:$123456789012:key/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa") + .tags(tags) .build(); final ResourceHandlerRequest request = ResourceHandlerRequest.builder() @@ -80,7 +88,8 @@ public void handleRequest_Success() { assertThat(response.getCallbackContext()).isNull(); assertThat(response.getCallbackDelaySeconds()).isEqualTo(0); assertThat(response.getResourceModels()).isNull(); - assertThat(response.getResourceModel()).isEqualToComparingFieldByField(logGroup); + assertThat(response.getResourceModel()).isEqualToComparingOnlyGivenFields(logGroup); + assertThat(response.getResourceModel().getTags()).isEqualTo(tags); assertThat(response.getMessage()).isNull(); assertThat(response.getErrorCode()).isNull(); } diff --git a/aws-logs-loggroup/src/test/java/software/amazon/logs/loggroup/ListHandlerTest.java b/aws-logs-loggroup/src/test/java/software/amazon/logs/loggroup/ListHandlerTest.java index 22256ee..b06311e 100644 --- a/aws-logs-loggroup/src/test/java/software/amazon/logs/loggroup/ListHandlerTest.java +++ b/aws-logs-loggroup/src/test/java/software/amazon/logs/loggroup/ListHandlerTest.java @@ -1,5 +1,6 @@ package software.amazon.logs.loggroup; +import software.amazon.awssdk.services.cloudwatchlogs.model.ListTagsLogGroupResponse; import software.amazon.cloudformation.proxy.AmazonWebServicesClientProxy; import software.amazon.cloudformation.proxy.Logger; import software.amazon.cloudformation.proxy.OperationStatus; @@ -15,6 +16,8 @@ import software.amazon.awssdk.services.cloudwatchlogs.model.LogGroup; import java.util.Arrays; +import java.util.HashSet; +import java.util.Set; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.doReturn; @@ -44,15 +47,29 @@ public void handleRequest_Success() { .retentionInDays(1) .kmsKeyId("arn:aws:kms:us-east-1:$123456789012:key/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa") .build(); + final Set tags = new HashSet<>(Arrays.asList( + Tag.builder().key("key-1").value("value-1").build(), + Tag.builder().key("key-2").value("value-2").build() + )); final LogGroup logGroup2 = LogGroup.builder() .logGroupName("LogGroup2") .retentionInDays(2) .kmsKeyId("arn:aws:kms:us-east-1:$123456789012:key/bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb") .build(); + final Set tags2 = new HashSet<>(Arrays.asList( + Tag.builder().key("key-3").value("value-3").build(), + Tag.builder().key("key-4").value("value-4").build() + )); final DescribeLogGroupsResponse describeResponse = DescribeLogGroupsResponse.builder() .logGroups(Arrays.asList(logGroup, logGroup2)) .nextToken("token2") .build(); + final ListTagsLogGroupResponse tagsResponse = ListTagsLogGroupResponse.builder() + .tags(Translator.translateTagsToSdk(tags)) + .build(); + final ListTagsLogGroupResponse tagsResponse2 = ListTagsLogGroupResponse.builder() + .tags(Translator.translateTagsToSdk(tags2)) + .build(); doReturn(describeResponse) .when(proxy) @@ -60,17 +77,31 @@ public void handleRequest_Success() { ArgumentMatchers.any(), ArgumentMatchers.any() ); + doReturn(tagsResponse) + .when(proxy) + .injectCredentialsAndInvokeV2( + ArgumentMatchers.eq(Translator.translateToListTagsLogGroupRequest(logGroup.logGroupName())), + ArgumentMatchers.any() + ); + doReturn(tagsResponse2) + .when(proxy) + .injectCredentialsAndInvokeV2( + ArgumentMatchers.eq(Translator.translateToListTagsLogGroupRequest(logGroup2.logGroupName())), + ArgumentMatchers.any() + ); final ResourceModel model1 = ResourceModel.builder() .logGroupName("LogGroup") .retentionInDays(1) .kmsKeyId("arn:aws:kms:us-east-1:$123456789012:key/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa") + .tags(tags) .build(); final ResourceModel model2 = ResourceModel.builder() .logGroupName("LogGroup2") .retentionInDays(2) .kmsKeyId("arn:aws:kms:us-east-1:$123456789012:key/bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb") + .tags(tags2) .build(); final ResourceHandlerRequest request = ResourceHandlerRequest.builder() diff --git a/aws-logs-loggroup/src/test/java/software/amazon/logs/loggroup/ReadHandlerTest.java b/aws-logs-loggroup/src/test/java/software/amazon/logs/loggroup/ReadHandlerTest.java index 93453e6..7cfb5de 100644 --- a/aws-logs-loggroup/src/test/java/software/amazon/logs/loggroup/ReadHandlerTest.java +++ b/aws-logs-loggroup/src/test/java/software/amazon/logs/loggroup/ReadHandlerTest.java @@ -1,5 +1,6 @@ package software.amazon.logs.loggroup; +import software.amazon.awssdk.services.cloudwatchlogs.model.ListTagsLogGroupResponse; import software.amazon.cloudformation.exceptions.ResourceNotFoundException; import software.amazon.cloudformation.proxy.AmazonWebServicesClientProxy; import software.amazon.cloudformation.proxy.Logger; @@ -15,7 +16,10 @@ import software.amazon.awssdk.services.cloudwatchlogs.model.DescribeLogGroupsResponse; import software.amazon.awssdk.services.cloudwatchlogs.model.LogGroup; +import java.util.Arrays; import java.util.Collections; +import java.util.HashSet; +import java.util.Set; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -47,11 +51,18 @@ public void handleRequest_Success() { .retentionInDays(1) .kmsKeyId("arn:aws:kms:us-east-1:$123456789012:key/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa") .build(); + final Set tags = new HashSet<>(Arrays.asList( + Tag.builder().key("key-1").value("value-1").build(), + Tag.builder().key("key-2").value("value-2").build() + )); final DescribeLogGroupsResponse describeResponse = DescribeLogGroupsResponse.builder() .logGroups(Collections.singletonList(logGroup)) .build(); + final ListTagsLogGroupResponse tagsResponse = ListTagsLogGroupResponse.builder() + .tags(Translator.translateTagsToSdk(tags)) + .build(); - doReturn(describeResponse) + doReturn(describeResponse, tagsResponse) .when(proxy) .injectCredentialsAndInvokeV2( ArgumentMatchers.any(), @@ -62,6 +73,7 @@ public void handleRequest_Success() { .logGroupName("LogGroup") .retentionInDays(1) .kmsKeyId("arn:aws:kms:us-east-1:$123456789012:key/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa") + .tags(tags) .build(); final ResourceHandlerRequest request = ResourceHandlerRequest.builder() @@ -75,7 +87,8 @@ public void handleRequest_Success() { assertThat(response.getCallbackContext()).isNull(); assertThat(response.getCallbackDelaySeconds()).isEqualTo(0); assertThat(response.getResourceModels()).isNull(); - assertThat(response.getResourceModel()).isEqualToComparingFieldByField(logGroup); + assertThat(response.getResourceModel()).isEqualToComparingOnlyGivenFields(logGroup); + assertThat(response.getResourceModel().getTags()).isEqualTo(tags); assertThat(response.getMessage()).isNull(); assertThat(response.getErrorCode()).isNull(); } @@ -85,8 +98,11 @@ public void handleRequest_FailureNotFound_EmptyLogGroupResponse() { final DescribeLogGroupsResponse describeResponse = DescribeLogGroupsResponse.builder() .logGroups(Collections.emptyList()) .build(); + final ListTagsLogGroupResponse tagsResponse = ListTagsLogGroupResponse.builder() + .tags(Collections.emptyMap()) + .build(); - doReturn(describeResponse) + doReturn(describeResponse, tagsResponse) .when(proxy) .injectCredentialsAndInvokeV2( ArgumentMatchers.any(), diff --git a/aws-logs-loggroup/src/test/java/software/amazon/logs/loggroup/TranslatorTest.java b/aws-logs-loggroup/src/test/java/software/amazon/logs/loggroup/TranslatorTest.java index 63f4083..cc815ac 100644 --- a/aws-logs-loggroup/src/test/java/software/amazon/logs/loggroup/TranslatorTest.java +++ b/aws-logs-loggroup/src/test/java/software/amazon/logs/loggroup/TranslatorTest.java @@ -8,22 +8,44 @@ import software.amazon.awssdk.services.cloudwatchlogs.model.DeleteRetentionPolicyRequest; import software.amazon.awssdk.services.cloudwatchlogs.model.DescribeLogGroupsRequest; import software.amazon.awssdk.services.cloudwatchlogs.model.DescribeLogGroupsResponse; +import software.amazon.awssdk.services.cloudwatchlogs.model.ListTagsLogGroupRequest; +import software.amazon.awssdk.services.cloudwatchlogs.model.ListTagsLogGroupResponse; import software.amazon.awssdk.services.cloudwatchlogs.model.LogGroup; import software.amazon.awssdk.services.cloudwatchlogs.model.PutRetentionPolicyRequest; import software.amazon.awssdk.services.cloudwatchlogs.model.DisassociateKmsKeyRequest; import software.amazon.awssdk.services.cloudwatchlogs.model.AssociateKmsKeyRequest; +import software.amazon.awssdk.services.cloudwatchlogs.model.TagLogGroupRequest; +import software.amazon.awssdk.services.cloudwatchlogs.model.UntagLogGroupRequest; +import java.util.Arrays; import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; import static org.assertj.core.api.Assertions.assertThat; @ExtendWith(MockitoExtension.class) public class TranslatorTest { + private static final Set SET_TAGS = new HashSet<>(Arrays.asList( + Tag.builder().key("key-1").value("value-1").build(), + Tag.builder().key("key-2").value("value-2").build() + )); + + private static final Map MAP_TAGS = new HashMap() {{ + put("key-1", "value-1"); + put("key-2", "value-2"); + }}; + private static final ResourceModel RESOURCE_MODEL = ResourceModel.builder() - .retentionInDays(1) - .kmsKeyId("arn:aws:kms:us-east-1:$123456789012:key/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa") - .logGroupName("LogGroup") - .build(); + .retentionInDays(1) + .kmsKeyId("arn:aws:kms:us-east-1:$123456789012:key/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa") + .logGroupName("LogGroup") + .tags(SET_TAGS) + .build(); @Test public void testTranslateToRead() { @@ -53,9 +75,10 @@ public void testTranslateToDelete() { @Test public void testTranslateToCreate() { final CreateLogGroupRequest request = CreateLogGroupRequest.builder() - .logGroupName(RESOURCE_MODEL.getLogGroupName()) - .kmsKeyId(RESOURCE_MODEL.getKmsKeyId()) - .build(); + .logGroupName(RESOURCE_MODEL.getLogGroupName()) + .kmsKeyId(RESOURCE_MODEL.getKmsKeyId()) + .tags(MAP_TAGS) + .build(); assertThat(Translator.translateToCreateRequest(RESOURCE_MODEL)).isEqualToComparingFieldByField(request); } @@ -101,6 +124,39 @@ public void testTranslateToAssociateKmsKeyRequest() { .isEqualToComparingFieldByField(request); } + @Test + public void testTranslateToListTagsLogGroupRequest() { + final ListTagsLogGroupRequest request = ListTagsLogGroupRequest.builder() + .logGroupName(RESOURCE_MODEL.getLogGroupName()) + .build(); + + assertThat(Translator.translateToListTagsLogGroupRequest(RESOURCE_MODEL.getLogGroupName())) + .isEqualToComparingFieldByField(request); + } + + @Test + public void testTranslateToTagLogGroupRequest() { + final TagLogGroupRequest request = TagLogGroupRequest.builder() + .logGroupName(RESOURCE_MODEL.getLogGroupName()) + .tags(MAP_TAGS) + .build(); + + assertThat(Translator.translateToTagLogGroupRequest(RESOURCE_MODEL.getLogGroupName(), SET_TAGS)) + .isEqualToComparingFieldByField(request); + } + + @Test + public void testTranslateToUntagLogGroupRequest() { + final List tagKeys = SET_TAGS.stream().map(Tag::getKey).collect(Collectors.toList()); + final UntagLogGroupRequest request = UntagLogGroupRequest.builder() + .logGroupName(RESOURCE_MODEL.getLogGroupName()) + .tags(tagKeys) + .build(); + + assertThat(Translator.translateToUntagLogGroupRequest(RESOURCE_MODEL.getLogGroupName(), tagKeys)) + .isEqualToComparingFieldByField(request); + } + @Test public void testTranslateForRead() { final LogGroup logGroup = LogGroup.builder() @@ -112,7 +168,10 @@ public void testTranslateForRead() { final DescribeLogGroupsResponse response = DescribeLogGroupsResponse.builder() .logGroups(Collections.singletonList(logGroup)) .build(); - assertThat(Translator.translateForRead(response)).isEqualToComparingFieldByField(RESOURCE_MODEL); + final ListTagsLogGroupResponse tagsResponse = ListTagsLogGroupResponse.builder() + .tags(MAP_TAGS) + .build(); + assertThat(Translator.translateForRead(response, tagsResponse)).isEqualToComparingFieldByField(RESOURCE_MODEL); } @Test @@ -120,11 +179,15 @@ public void testTranslateForRead_logGroupEmpty() { final DescribeLogGroupsResponse response = DescribeLogGroupsResponse.builder() .logGroups(Collections.emptyList()) .build(); + final ListTagsLogGroupResponse tagsResponse = ListTagsLogGroupResponse.builder() + .tags(Collections.emptyMap()) + .build(); final ResourceModel emptyModel = ResourceModel.builder() .retentionInDays(null) .logGroupName(null) + .tags(null) .build(); - assertThat(Translator.translateForRead(response)).isEqualToComparingFieldByField(emptyModel); + assertThat(Translator.translateForRead(response, tagsResponse)).isEqualToComparingFieldByField(emptyModel); } @Test @@ -132,11 +195,15 @@ public void testTranslateForRead_LogGroupHasNullMembers() { final DescribeLogGroupsResponse response = DescribeLogGroupsResponse.builder() .logGroups(Collections.singletonList(LogGroup.builder().build())) .build(); + final ListTagsLogGroupResponse tagsResponse = ListTagsLogGroupResponse.builder() + .tags(Collections.emptyMap()) + .build(); final ResourceModel emptyModel = ResourceModel.builder() - .retentionInDays(null) - .logGroupName(null) - .build(); - assertThat(Translator.translateForRead(response)).isEqualToComparingFieldByField(emptyModel); + .retentionInDays(null) + .logGroupName(null) + .tags(null) + .build(); + assertThat(Translator.translateForRead(response, tagsResponse)).isEqualToComparingFieldByField(emptyModel); } @Test @@ -150,4 +217,26 @@ public void buildResourceDoesNotExistErrorMessage() { final String expected = "Resource of type 'AWS::Logs::LogGroup' with identifier 'ID' was not found."; assertThat(Translator.buildResourceDoesNotExistErrorMessage("ID")).isEqualTo(expected); } + + @Test + public void translateTagsToSdk() { + assertThat(Translator.translateTagsToSdk(SET_TAGS)).isEqualTo(MAP_TAGS); + } + + @Test + public void translateTagsToSdk_TagsEmpty() { + assertThat(Translator.translateTagsToSdk(null)).isNull(); + assertThat(Translator.translateTagsToSdk(Collections.emptySet())).isNull(); + } + + @Test + public void translateSdkToTags() { + assertThat(Translator.translateSdkToTags(MAP_TAGS)).isEqualTo(SET_TAGS); + } + + @Test + public void translateSdkToTags_TagsEmpty() { + assertThat(Translator.translateSdkToTags(null)).isNull(); + assertThat(Translator.translateSdkToTags(Collections.emptyMap())).isNull(); + } } diff --git a/aws-logs-loggroup/src/test/java/software/amazon/logs/loggroup/UpdateHandlerTest.java b/aws-logs-loggroup/src/test/java/software/amazon/logs/loggroup/UpdateHandlerTest.java index a63a3cf..1075e26 100644 --- a/aws-logs-loggroup/src/test/java/software/amazon/logs/loggroup/UpdateHandlerTest.java +++ b/aws-logs-loggroup/src/test/java/software/amazon/logs/loggroup/UpdateHandlerTest.java @@ -1,6 +1,18 @@ package software.amazon.logs.loggroup; +import com.google.common.collect.Sets; import software.amazon.awssdk.services.cloudwatchlogs.model.CloudWatchLogsRequest; +import software.amazon.awssdk.services.cloudwatchlogs.model.DeleteRetentionPolicyResponse; +import software.amazon.awssdk.services.cloudwatchlogs.model.DescribeLogGroupsResponse; +import software.amazon.awssdk.services.cloudwatchlogs.model.LogGroup; +import software.amazon.awssdk.services.cloudwatchlogs.model.PutRetentionPolicyResponse; +import software.amazon.awssdk.services.cloudwatchlogs.model.DisassociateKmsKeyResponse; +import software.amazon.awssdk.services.cloudwatchlogs.model.PutRetentionPolicyRequest; +import software.amazon.awssdk.services.cloudwatchlogs.model.DeleteRetentionPolicyRequest; +import software.amazon.awssdk.services.cloudwatchlogs.model.AssociateKmsKeyRequest; +import software.amazon.awssdk.services.cloudwatchlogs.model.DisassociateKmsKeyRequest; +import software.amazon.awssdk.services.cloudwatchlogs.model.TagLogGroupRequest; +import software.amazon.awssdk.services.cloudwatchlogs.model.UntagLogGroupRequest; import software.amazon.cloudformation.proxy.AmazonWebServicesClientProxy; import software.amazon.cloudformation.proxy.Logger; import software.amazon.cloudformation.proxy.OperationStatus; @@ -13,17 +25,13 @@ import org.mockito.ArgumentMatchers; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; -import software.amazon.awssdk.services.cloudwatchlogs.model.DeleteRetentionPolicyResponse; -import software.amazon.awssdk.services.cloudwatchlogs.model.DescribeLogGroupsResponse; -import software.amazon.awssdk.services.cloudwatchlogs.model.LogGroup; -import software.amazon.awssdk.services.cloudwatchlogs.model.PutRetentionPolicyResponse; -import software.amazon.awssdk.services.cloudwatchlogs.model.DisassociateKmsKeyResponse; -import software.amazon.awssdk.services.cloudwatchlogs.model.PutRetentionPolicyRequest; -import software.amazon.awssdk.services.cloudwatchlogs.model.DeleteRetentionPolicyRequest; -import software.amazon.awssdk.services.cloudwatchlogs.model.AssociateKmsKeyRequest; -import software.amazon.awssdk.services.cloudwatchlogs.model.DisassociateKmsKeyRequest; +import java.util.Arrays; import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import java.util.stream.Collectors; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -59,6 +67,10 @@ public void handleRequest_Success() { .retentionInDays(1) .kmsKeyId("arn:aws:kms:us-east-1:$123456789012:key/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa") .build(); + final Set tags = new HashSet<>(Arrays.asList( + Tag.builder().key("key-1").value("value-1").build(), + Tag.builder().key("key-2").value("value-2").build() + )); final DescribeLogGroupsResponse describeResponse = DescribeLogGroupsResponse.builder() .logGroups(Collections.singletonList(logGroup)) .build(); @@ -74,6 +86,7 @@ public void handleRequest_Success() { .logGroupName("LogGroup") .retentionInDays(1) .kmsKeyId("arn:aws:kms:us-east-1:$123456789012:key/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa") + .tags(tags) .build(); final ResourceHandlerRequest request = ResourceHandlerRequest.builder() @@ -87,7 +100,8 @@ public void handleRequest_Success() { assertThat(response.getCallbackContext()).isNull(); assertThat(response.getCallbackDelaySeconds()).isEqualTo(0); assertThat(response.getResourceModels()).isNull(); - assertThat(response.getResourceModel()).isEqualToComparingFieldByField(logGroup); + assertThat(response.getResourceModel()).isEqualToComparingOnlyGivenFields(logGroup); + assertThat(response.getResourceModel().getTags()).isEqualTo(tags); assertThat(response.getMessage()).isNull(); assertThat(response.getErrorCode()).isNull(); } @@ -126,7 +140,7 @@ public void handleRequest_Success_RetentionPolicyDeleted() { assertThat(response.getCallbackContext()).isNull(); assertThat(response.getCallbackDelaySeconds()).isEqualTo(0); assertThat(response.getResourceModels()).isNull(); - assertThat(response.getResourceModel()).isEqualToComparingFieldByField(logGroup); + assertThat(response.getResourceModel()).isEqualToComparingOnlyGivenFields(logGroup); assertThat(response.getMessage()).isNull(); assertThat(response.getErrorCode()).isNull(); } @@ -162,7 +176,7 @@ public void handleRequest_Success_KmsKeyIdDeleted() { assertThat(response.getCallbackContext()).isNull(); assertThat(response.getCallbackDelaySeconds()).isEqualTo(0); assertThat(response.getResourceModels()).isNull(); - assertThat(response.getResourceModel()).isEqualToComparingFieldByField(logGroup); + assertThat(response.getResourceModel()).isEqualToComparingOnlyGivenFields(logGroup); assertThat(response.getMessage()).isNull(); assertThat(response.getErrorCode()).isNull(); } @@ -210,7 +224,7 @@ public void handleRequest_SuccessNoChange() { assertThat(response.getCallbackContext()).isNull(); assertThat(response.getCallbackDelaySeconds()).isEqualTo(0); assertThat(response.getResourceModels()).isNull(); - assertThat(response.getResourceModel()).isEqualToComparingFieldByField(logGroup); + assertThat(response.getResourceModel()).isEqualToComparingOnlyGivenFields(logGroup); assertThat(response.getMessage()).isNull(); assertThat(response.getErrorCode()).isNull(); } @@ -222,16 +236,22 @@ public void handleRequest_SuccessNoChange_NoAction_WithPreviousModel() { .retentionInDays(1) .kmsKeyId("arn:aws:kms:us-east-1:$123456789012:key/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa") .build(); + final Set tags = new HashSet<>(Arrays.asList( + Tag.builder().key("key-1").value("value-1").build(), + Tag.builder().key("key-2").value("value-2").build() + )); final ResourceModel previousModel = ResourceModel.builder() .logGroupName("LogGroup") .retentionInDays(1) .kmsKeyId("arn:aws:kms:us-east-1:$123456789012:key/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa") + .tags(tags) .build(); final ResourceModel model = ResourceModel.builder() .logGroupName("LogGroup") .retentionInDays(1) .kmsKeyId("arn:aws:kms:us-east-1:$123456789012:key/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa") + .tags(tags) .build(); final ResourceHandlerRequest request = ResourceHandlerRequest.builder() @@ -246,27 +266,33 @@ public void handleRequest_SuccessNoChange_NoAction_WithPreviousModel() { assertThat(response.getCallbackContext()).isNull(); assertThat(response.getCallbackDelaySeconds()).isEqualTo(0); assertThat(response.getResourceModels()).isNull(); - assertThat(response.getResourceModel()).isEqualToComparingFieldByField(logGroup); + assertThat(response.getResourceModel()).isEqualToComparingOnlyGivenFields(logGroup); + assertThat(response.getResourceModel().getTags()).isEqualTo(tags); assertThat(response.getMessage()).isNull(); assertThat(response.getErrorCode()).isNull(); } @Test - public void handleRequest_Success_UpdateWith_RetentionAndKms() { + public void handleRequest_Success_UpdateWith_RetentionAndKmsAndTags() { final LogGroup logGroup = LogGroup.builder() .logGroupName("LogGroup") .retentionInDays(2) .kmsKeyId("arn:aws:kms:us-east-1:$123456789012:key/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa") .build(); + final Set tags = new HashSet<>(Arrays.asList( + Tag.builder().key("key-1").value("value-1").build(), + Tag.builder().key("key-2").value("value-2").build() + )); final ResourceModel previousModel = ResourceModel.builder() .logGroupName("LogGroup") .build(); final ResourceModel model = ResourceModel.builder() - .logGroupName("LogGroup") - .retentionInDays(2) - .kmsKeyId("arn:aws:kms:us-east-1:$123456789012:key/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa") - .build(); + .logGroupName("LogGroup") + .retentionInDays(2) + .kmsKeyId("arn:aws:kms:us-east-1:$123456789012:key/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa") + .tags(tags) + .build(); final ResourceHandlerRequest request = ResourceHandlerRequest.builder() .previousResourceState(previousModel) @@ -276,7 +302,7 @@ public void handleRequest_Success_UpdateWith_RetentionAndKms() { final ProgressEvent response = handler.handleRequest(proxy, request, null, logger); ArgumentCaptor requests = ArgumentCaptor.forClass(CloudWatchLogsRequest.class); - verify(proxy, times(2)).injectCredentialsAndInvokeV2(requests.capture(), any()); + verify(proxy, times(3)).injectCredentialsAndInvokeV2(requests.capture(), any()); assertThat(requests.getAllValues().get(0)).isEqualTo(PutRetentionPolicyRequest.builder() .logGroupName("LogGroup") .retentionInDays(2) @@ -287,12 +313,166 @@ public void handleRequest_Success_UpdateWith_RetentionAndKms() { .kmsKeyId("arn:aws:kms:us-east-1:$123456789012:key/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa") .build()); + assertThat(requests.getAllValues().get(2)).isEqualTo(TagLogGroupRequest.builder() + .logGroupName("LogGroup") + .tags(Translator.translateTagsToSdk(tags)) + .build()); + assertThat(response).isNotNull(); assertThat(response.getStatus()).isEqualTo(OperationStatus.SUCCESS); assertThat(response.getCallbackContext()).isNull(); assertThat(response.getCallbackDelaySeconds()).isEqualTo(0); assertThat(response.getResourceModels()).isNull(); - assertThat(response.getResourceModel()).isEqualToComparingFieldByField(logGroup); + assertThat(response.getResourceModel()).isEqualToComparingOnlyGivenFields(logGroup); + assertThat(response.getResourceModel().getTags()).isEqualTo(tags); + assertThat(response.getMessage()).isNull(); + assertThat(response.getErrorCode()).isNull(); + } + + @Test + public void handleRequest_Success_AddTags() { + final LogGroup logGroup = LogGroup.builder() + .logGroupName("LogGroup") + .build(); + final Set previousTags = new HashSet<>(Arrays.asList( + Tag.builder().key("key-1").value("value-1").build(), + Tag.builder().key("key-2").value("value-2").build() + )); + final ResourceModel previousModel = ResourceModel.builder() + .logGroupName("LogGroup") + .tags(previousTags) + .build(); + + final Set newTags = new HashSet<>(Arrays.asList( + Tag.builder().key("key-3").value("value-3").build(), + Tag.builder().key("key-4").value("value-4").build() + )); + final Set tags = new HashSet<>(); + tags.addAll(previousTags); + tags.addAll(newTags); + final ResourceModel model = ResourceModel.builder() + .logGroupName("LogGroup") + .tags(tags) + .build(); + + final ResourceHandlerRequest request = ResourceHandlerRequest.builder() + .previousResourceState(previousModel) + .desiredResourceState(model) + .build(); + + final ProgressEvent response = handler.handleRequest(proxy, request, null, logger); + + ArgumentCaptor requests = ArgumentCaptor.forClass(CloudWatchLogsRequest.class); + verify(proxy, times(1)).injectCredentialsAndInvokeV2(requests.capture(), any()); + assertThat(requests.getAllValues().get(0)).isEqualTo(TagLogGroupRequest.builder() + .logGroupName("LogGroup") + .tags(Translator.translateTagsToSdk(newTags)) + .build()); + + assertThat(response).isNotNull(); + assertThat(response.getStatus()).isEqualTo(OperationStatus.SUCCESS); + assertThat(response.getCallbackContext()).isNull(); + assertThat(response.getCallbackDelaySeconds()).isEqualTo(0); + assertThat(response.getResourceModels()).isNull(); + assertThat(response.getResourceModel()).isEqualToComparingOnlyGivenFields(logGroup); + assertThat(response.getResourceModel().getTags()).isEqualTo(tags); + assertThat(response.getMessage()).isNull(); + assertThat(response.getErrorCode()).isNull(); + } + + @Test + public void handleRequest_Success_UpdateTags() { + final LogGroup logGroup = LogGroup.builder() + .logGroupName("LogGroup") + .build(); + final Set previousTags = new HashSet<>(Arrays.asList( + Tag.builder().key("key-1").value("value-1").build(), + Tag.builder().key("key-2").value("value-2").build() + )); + final ResourceModel previousModel = ResourceModel.builder() + .logGroupName("LogGroup") + .tags(previousTags) + .build(); + + final Set tags = new HashSet<>(Arrays.asList( + Tag.builder().key("key-2").value("value-2-new").build(), + Tag.builder().key("key-3").value("value-3").build() + )); + final ResourceModel model = ResourceModel.builder() + .logGroupName("LogGroup") + .tags(tags) + .build(); + + final ResourceHandlerRequest request = ResourceHandlerRequest.builder() + .previousResourceState(previousModel) + .desiredResourceState(model) + .build(); + + final ProgressEvent response = handler.handleRequest(proxy, request, null, logger); + + ArgumentCaptor requests = ArgumentCaptor.forClass(CloudWatchLogsRequest.class); + verify(proxy, times(2)).injectCredentialsAndInvokeV2(requests.capture(), any()); + final List removedTagKeys = Sets.difference(previousTags, tags).stream().map(Tag::getKey).collect(Collectors.toList()); + assertThat(requests.getAllValues().get(0)).isEqualTo(UntagLogGroupRequest.builder() + .logGroupName("LogGroup") + .tags(removedTagKeys) + .build()); + assertThat(requests.getAllValues().get(1)).isEqualTo(TagLogGroupRequest.builder() + .logGroupName("LogGroup") + .tags(Translator.translateTagsToSdk(tags)) + .build()); + + assertThat(response).isNotNull(); + assertThat(response.getStatus()).isEqualTo(OperationStatus.SUCCESS); + assertThat(response.getCallbackContext()).isNull(); + assertThat(response.getCallbackDelaySeconds()).isEqualTo(0); + assertThat(response.getResourceModels()).isNull(); + assertThat(response.getResourceModel()).isEqualToComparingOnlyGivenFields(logGroup); + assertThat(response.getResourceModel().getTags()).isEqualTo(tags); + assertThat(response.getMessage()).isNull(); + assertThat(response.getErrorCode()).isNull(); + } + + @Test + public void handleRequest_Success_RemoveTags() { + final LogGroup logGroup = LogGroup.builder() + .logGroupName("LogGroup") + .build(); + final Set tags = new HashSet<>(Arrays.asList( + Tag.builder().key("key-1").value("value-1").build(), + Tag.builder().key("key-2").value("value-2").build() + )); + final ResourceModel previousModel = ResourceModel.builder() + .logGroupName("LogGroup") + .tags(tags) + .build(); + + final ResourceModel model = ResourceModel.builder() + .logGroupName("LogGroup") + .build(); + + final ResourceHandlerRequest request = ResourceHandlerRequest.builder() + .previousResourceState(previousModel) + .desiredResourceState(model) + .build(); + + final ProgressEvent response = handler.handleRequest(proxy, request, null, logger); + + ArgumentCaptor requests = ArgumentCaptor.forClass(CloudWatchLogsRequest.class); + verify(proxy, times(1)).injectCredentialsAndInvokeV2(requests.capture(), any()); + final List removedTagKeys = tags.stream().map(Tag::getKey).collect(Collectors.toList()); + assertThat(requests.getAllValues().get(0)).isEqualTo(UntagLogGroupRequest.builder() + .logGroupName("LogGroup") + .tags(removedTagKeys) + .build()); + + assertThat(response).isNotNull(); + assertThat(response.getStatus()).isEqualTo(OperationStatus.SUCCESS); + assertThat(response.getCallbackContext()).isNull(); + assertThat(response.getCallbackDelaySeconds()).isEqualTo(0); + assertThat(response.getResourceModels()).isNull(); + assertThat(response.getResourceModel()).isEqualToComparingOnlyGivenFields(logGroup); + assertThat(response.getResourceModel().getTags()).isNull(); assertThat(response.getMessage()).isNull(); assertThat(response.getErrorCode()).isNull(); } @@ -599,4 +779,64 @@ public void handleRequest_DisassociateKms_ServiceUnavailable_ServiceException() assertThrows(software.amazon.cloudformation.exceptions.CfnServiceInternalErrorException.class, () -> handler.handleRequest(proxy, request, null, logger)); } + + @Test + public void handleRequest_AddTags_FailureNotFound_ServiceException() { + final ResourceModel previousModel = ResourceModel.builder() + .logGroupName("LogGroup") + .build(); + doThrow(software.amazon.awssdk.services.cloudwatchlogs.model.ResourceNotFoundException.class) + .when(proxy) + .injectCredentialsAndInvokeV2( + ArgumentMatchers.isA(TagLogGroupRequest.class), + any() + ); + + final Set tags = new HashSet<>(Arrays.asList( + Tag.builder().key("key-1").value("value-1").build(), + Tag.builder().key("key-2").value("value-2").build() + )); + final ResourceModel model = ResourceModel.builder() + .logGroupName("LogGroup") + .tags(tags) + .build(); + + final ResourceHandlerRequest request = ResourceHandlerRequest.builder() + .previousResourceState(previousModel) + .desiredResourceState(model) + .build(); + + assertThrows(software.amazon.cloudformation.exceptions.ResourceNotFoundException.class, + () -> handler.handleRequest(proxy, request, null, logger)); + } + + @Test + public void handleRequest_RemoveTags_FailureNotFound_ServiceException() { + final Set previousTags = new HashSet<>(Arrays.asList( + Tag.builder().key("key-1").value("value-1").build(), + Tag.builder().key("key-2").value("value-2").build() + )); + final ResourceModel previousModel = ResourceModel.builder() + .logGroupName("LogGroup") + .tags(previousTags) + .build(); + doThrow(software.amazon.awssdk.services.cloudwatchlogs.model.ResourceNotFoundException.class) + .when(proxy) + .injectCredentialsAndInvokeV2( + ArgumentMatchers.isA(UntagLogGroupRequest.class), + any() + ); + + final ResourceModel model = ResourceModel.builder() + .logGroupName("LogGroup") + .build(); + + final ResourceHandlerRequest request = ResourceHandlerRequest.builder() + .previousResourceState(previousModel) + .desiredResourceState(model) + .build(); + + assertThrows(software.amazon.cloudformation.exceptions.ResourceNotFoundException.class, + () -> handler.handleRequest(proxy, request, null, logger)); + } } diff --git a/aws-logs-loggroup/template.yml b/aws-logs-loggroup/template.yml index a439697..438be3d 100644 --- a/aws-logs-loggroup/template.yml +++ b/aws-logs-loggroup/template.yml @@ -5,6 +5,7 @@ Description: AWS SAM template for the AWS::Logs::LogGroup resource type Globals: Function: Timeout: 60 # docker start-up times can be long for SAM CLI + MemorySize: 256 Resources: TypeFunction: From d35f9a49ea1e719a34a355cc964ecc967087f004 Mon Sep 17 00:00:00 2001 From: gruebel Date: Thu, 12 Nov 2020 22:27:55 +0100 Subject: [PATCH 2/7] Add missing exception handling --- .../amazon/logs/loggroup/UpdateHandler.java | 3 +- .../logs/loggroup/UpdateHandlerTest.java | 55 +++++++++++++++++++ 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/aws-logs-loggroup/src/main/java/software/amazon/logs/loggroup/UpdateHandler.java b/aws-logs-loggroup/src/main/java/software/amazon/logs/loggroup/UpdateHandler.java index d262b2b..14f33d7 100644 --- a/aws-logs-loggroup/src/main/java/software/amazon/logs/loggroup/UpdateHandler.java +++ b/aws-logs-loggroup/src/main/java/software/amazon/logs/loggroup/UpdateHandler.java @@ -33,7 +33,6 @@ public ProgressEvent handleRequest( final CallbackContext callbackContext, final Logger logger) { - // Everything except RetentionPolicyInDays, KmsKeyId and Tags is createOnly final ResourceModel model = request.getDesiredResourceState(); final ResourceModel previousModel = request.getPreviousResourceState(); final boolean retentionChanged = ! retentionUnchanged(previousModel, model); @@ -189,6 +188,8 @@ private void updateTags(final AmazonWebServicesClientProxy proxy, } } catch (final ResourceNotFoundException e) { throwNotFoundException(model); + } catch (final InvalidParameterException e) { + throw new CfnInternalFailureException(e); } } diff --git a/aws-logs-loggroup/src/test/java/software/amazon/logs/loggroup/UpdateHandlerTest.java b/aws-logs-loggroup/src/test/java/software/amazon/logs/loggroup/UpdateHandlerTest.java index 1075e26..708b0bb 100644 --- a/aws-logs-loggroup/src/test/java/software/amazon/logs/loggroup/UpdateHandlerTest.java +++ b/aws-logs-loggroup/src/test/java/software/amazon/logs/loggroup/UpdateHandlerTest.java @@ -810,6 +810,33 @@ public void handleRequest_AddTags_FailureNotFound_ServiceException() { () -> handler.handleRequest(proxy, request, null, logger)); } + @Test + public void handleRequest_AddTags_InvalidParameter_ServiceException() { + final ResourceModel previousModel = ResourceModel.builder() + .logGroupName("LogGroup") + .build(); + doThrow(software.amazon.awssdk.services.cloudwatchlogs.model.InvalidParameterException.class) + .when(proxy) + .injectCredentialsAndInvokeV2( + ArgumentMatchers.isA(TagLogGroupRequest.class), + any() + ); + + final Set tags = Collections.singleton(Tag.builder().key("key-1").value("value-1").build()); + final ResourceModel model = ResourceModel.builder() + .logGroupName("LogGroup") + .tags(tags) + .build(); + + final ResourceHandlerRequest request = ResourceHandlerRequest.builder() + .previousResourceState(previousModel) + .desiredResourceState(model) + .build(); + + assertThrows(software.amazon.cloudformation.exceptions.CfnInternalFailureException.class, + () -> handler.handleRequest(proxy, request, null, logger)); + } + @Test public void handleRequest_RemoveTags_FailureNotFound_ServiceException() { final Set previousTags = new HashSet<>(Arrays.asList( @@ -839,4 +866,32 @@ public void handleRequest_RemoveTags_FailureNotFound_ServiceException() { assertThrows(software.amazon.cloudformation.exceptions.ResourceNotFoundException.class, () -> handler.handleRequest(proxy, request, null, logger)); } + + + @Test + public void handleRequest_RemoveTags_InvalidParameter_ServiceException() { + final Set tags = Collections.singleton(Tag.builder().key("key-1").value("value-1").build()); + final ResourceModel previousModel = ResourceModel.builder() + .logGroupName("LogGroup") + .tags(tags) + .build(); + doThrow(software.amazon.awssdk.services.cloudwatchlogs.model.InvalidParameterException.class) + .when(proxy) + .injectCredentialsAndInvokeV2( + ArgumentMatchers.isA(UntagLogGroupRequest.class), + any() + ); + + final ResourceModel model = ResourceModel.builder() + .logGroupName("LogGroup") + .build(); + + final ResourceHandlerRequest request = ResourceHandlerRequest.builder() + .previousResourceState(previousModel) + .desiredResourceState(model) + .build(); + + assertThrows(software.amazon.cloudformation.exceptions.CfnInternalFailureException.class, + () -> handler.handleRequest(proxy, request, null, logger)); + } } From a1ab81568478d097a6327080423b66a1f8f8f309 Mon Sep 17 00:00:00 2001 From: gruebel Date: Fri, 13 Nov 2020 00:16:31 +0100 Subject: [PATCH 3/7] removed Tag patterns and upgraded dependency, due to failing cotract test --- aws-logs-loggroup/aws-logs-loggroup.json | 6 ++---- aws-logs-loggroup/pom.xml | 2 +- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/aws-logs-loggroup/aws-logs-loggroup.json b/aws-logs-loggroup/aws-logs-loggroup.json index 5364a9c..36f4da0 100644 --- a/aws-logs-loggroup/aws-logs-loggroup.json +++ b/aws-logs-loggroup/aws-logs-loggroup.json @@ -11,15 +11,13 @@ "type": "string", "description": "The key name of the tag. You can specify a value that is 1 to 128 Unicode characters in length and cannot be prefixed with aws:. You can use any of the following characters: the set of Unicode letters, digits, whitespace, _, ., :, /, =, +, - and @.", "minLength": 1, - "maxLength": 128, - "pattern": "^([\\w\\s\\d_.:/=+\\-@]+)$" + "maxLength": 128 }, "Value": { "type": "string", "description": "The value for the tag. You can specify a value that is 0 to 256 Unicode characters in length. You can use any of the following characters: the set of Unicode letters, digits, whitespace, _, ., :, /, =, +, - and @.", "minLength": 0, - "maxLength": 256, - "pattern": "^([\\w\\s\\d_.:/=+\\-@]*)$" + "maxLength": 256 } }, "required": [ diff --git a/aws-logs-loggroup/pom.xml b/aws-logs-loggroup/pom.xml index 5a75c6f..b9f2360 100644 --- a/aws-logs-loggroup/pom.xml +++ b/aws-logs-loggroup/pom.xml @@ -43,7 +43,7 @@ software.amazon.awssdk cloudwatchlogs - 2.13.18 + 2.15.26 From b79f1876bebf7157e45ed184979954675fc13dc5 Mon Sep 17 00:00:00 2001 From: gruebel Date: Mon, 16 Nov 2020 10:51:15 +0100 Subject: [PATCH 4/7] pin awssdk version to same as Java plugin --- aws-logs-loggroup/pom.xml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/aws-logs-loggroup/pom.xml b/aws-logs-loggroup/pom.xml index b9f2360..c1705b4 100644 --- a/aws-logs-loggroup/pom.xml +++ b/aws-logs-loggroup/pom.xml @@ -16,6 +16,7 @@ 1.8 UTF-8 UTF-8 + 2.15.19 @@ -43,7 +44,7 @@ software.amazon.awssdk cloudwatchlogs - 2.15.26 + ${awssdk.version} From e8b0238ab049827898d58c1707252adaac3f97db Mon Sep 17 00:00:00 2001 From: gruebel Date: Wed, 9 Dec 2020 17:41:55 +0100 Subject: [PATCH 5/7] Silent fail when list tags permission is missing --- .../amazon/logs/loggroup/ReadHandler.java | 16 ++- .../amazon/logs/loggroup/Translator.java | 4 +- .../amazon/logs/loggroup/ReadHandlerTest.java | 101 ++++++++++++++++++ 3 files changed, 118 insertions(+), 3 deletions(-) diff --git a/aws-logs-loggroup/src/main/java/software/amazon/logs/loggroup/ReadHandler.java b/aws-logs-loggroup/src/main/java/software/amazon/logs/loggroup/ReadHandler.java index 52d357f..caeecd7 100644 --- a/aws-logs-loggroup/src/main/java/software/amazon/logs/loggroup/ReadHandler.java +++ b/aws-logs-loggroup/src/main/java/software/amazon/logs/loggroup/ReadHandler.java @@ -1,5 +1,6 @@ package software.amazon.logs.loggroup; +import software.amazon.awssdk.services.cloudwatchlogs.model.CloudWatchLogsException; import software.amazon.awssdk.services.cloudwatchlogs.model.ListTagsLogGroupResponse; import software.amazon.cloudformation.proxy.AmazonWebServicesClientProxy; import software.amazon.cloudformation.proxy.Logger; @@ -12,6 +13,8 @@ public class ReadHandler extends BaseHandler { + private static final String ACCESS_DENIED_ERROR_CODE = "AccessDeniedException"; + @Override public ProgressEvent handleRequest( final AmazonWebServicesClientProxy proxy, @@ -30,8 +33,17 @@ public ProgressEvent handleRequest( try { response = proxy.injectCredentialsAndInvokeV2(Translator.translateToReadRequest(model), ClientBuilder.getClient()::describeLogGroups); - tagsResponse = proxy.injectCredentialsAndInvokeV2(Translator.translateToListTagsLogGroupRequest(model.getLogGroupName()), - ClientBuilder.getClient()::listTagsLogGroup); + try { + tagsResponse = proxy.injectCredentialsAndInvokeV2(Translator.translateToListTagsLogGroupRequest(model.getLogGroupName()), + ClientBuilder.getClient()::listTagsLogGroup); + } catch (final CloudWatchLogsException e) { + if (ACCESS_DENIED_ERROR_CODE.equals(e.awsErrorDetails().errorCode())) { + // fail silently, if there is no permission to list tags + logger.log(e.getMessage()); + } else { + throw e; + } + } } catch (final ResourceNotFoundException e) { throwNotFoundException(model); } diff --git a/aws-logs-loggroup/src/main/java/software/amazon/logs/loggroup/Translator.java b/aws-logs-loggroup/src/main/java/software/amazon/logs/loggroup/Translator.java index 472b747..1064b3c 100644 --- a/aws-logs-loggroup/src/main/java/software/amazon/logs/loggroup/Translator.java +++ b/aws-logs-loggroup/src/main/java/software/amazon/logs/loggroup/Translator.java @@ -120,7 +120,9 @@ static ResourceModel translateForRead(final DescribeLogGroupsResponse response, .filter(Objects::nonNull) .findAny() .orElse(null); - final Set tags = translateSdkToTags(tagsResponse.tags()); + final Set tags = translateSdkToTags(Optional.ofNullable(tagsResponse) + .map(ListTagsLogGroupResponse::tags) + .orElse(null)); return ResourceModel.builder() .arn(logGroupArn) .logGroupName(logGroupName) diff --git a/aws-logs-loggroup/src/test/java/software/amazon/logs/loggroup/ReadHandlerTest.java b/aws-logs-loggroup/src/test/java/software/amazon/logs/loggroup/ReadHandlerTest.java index 7cfb5de..d18aee0 100644 --- a/aws-logs-loggroup/src/test/java/software/amazon/logs/loggroup/ReadHandlerTest.java +++ b/aws-logs-loggroup/src/test/java/software/amazon/logs/loggroup/ReadHandlerTest.java @@ -1,5 +1,10 @@ package software.amazon.logs.loggroup; +import software.amazon.awssdk.awscore.exception.AwsErrorDetails; +import software.amazon.awssdk.awscore.exception.AwsServiceException; +import software.amazon.awssdk.services.cloudwatchlogs.model.CloudWatchLogsException; +import software.amazon.awssdk.services.cloudwatchlogs.model.DescribeLogGroupsRequest; +import software.amazon.awssdk.services.cloudwatchlogs.model.ListTagsLogGroupRequest; import software.amazon.awssdk.services.cloudwatchlogs.model.ListTagsLogGroupResponse; import software.amazon.cloudformation.exceptions.ResourceNotFoundException; import software.amazon.cloudformation.proxy.AmazonWebServicesClientProxy; @@ -22,6 +27,7 @@ import java.util.Set; import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.doThrow; @@ -167,4 +173,99 @@ public void handleRequest_FailureNotFound_NullModel() { assertThrows(ResourceNotFoundException.class, () -> handler.handleRequest(proxy, request, null, logger)); } + + @Test + public void handleRequest_FailureListTagsAccessDenied_NoException() { + final LogGroup logGroup = LogGroup.builder() + .logGroupName("LogGroup") + .retentionInDays(1) + .kmsKeyId("arn:aws:kms:us-east-1:$123456789012:key/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa") + .build(); + final DescribeLogGroupsResponse describeResponse = DescribeLogGroupsResponse.builder() + .logGroups(Collections.singletonList(logGroup)) + .build(); + + doReturn(describeResponse) + .when(proxy) + .injectCredentialsAndInvokeV2( + ArgumentMatchers.isA(DescribeLogGroupsRequest.class), + ArgumentMatchers.any() + ); + final AwsServiceException exception = CloudWatchLogsException.builder() + .awsErrorDetails(AwsErrorDetails.builder() + .errorCode("AccessDeniedException") + .build()) + .build(); + doThrow(exception) + .when(proxy) + .injectCredentialsAndInvokeV2( + ArgumentMatchers.isA(ListTagsLogGroupRequest.class), + ArgumentMatchers.any() + ); + + final ResourceModel model = ResourceModel.builder() + .logGroupName("LogGroup") + .retentionInDays(1) + .kmsKeyId("arn:aws:kms:us-east-1:$123456789012:key/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa") + .build(); + + final ResourceHandlerRequest request = ResourceHandlerRequest.builder() + .desiredResourceState(model) + .build(); + + final ProgressEvent response = handler.handleRequest(proxy, request, null, logger); + + assertThat(response).isNotNull(); + assertThat(response.getStatus()).isEqualTo(OperationStatus.SUCCESS); + assertThat(response.getCallbackContext()).isNull(); + assertThat(response.getCallbackDelaySeconds()).isEqualTo(0); + assertThat(response.getResourceModels()).isNull(); + assertThat(response.getResourceModel()).isEqualToComparingOnlyGivenFields(logGroup); + assertThat(response.getResourceModel().getTags()).isNull(); + assertThat(response.getMessage()).isNull(); + assertThat(response.getErrorCode()).isNull(); + } + + @Test + public void handleRequest_FailureListTags_WithException() { + final LogGroup logGroup = LogGroup.builder() + .logGroupName("LogGroup") + .retentionInDays(1) + .kmsKeyId("arn:aws:kms:us-east-1:$123456789012:key/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa") + .build(); + final DescribeLogGroupsResponse describeResponse = DescribeLogGroupsResponse.builder() + .logGroups(Collections.singletonList(logGroup)) + .build(); + + doReturn(describeResponse) + .when(proxy) + .injectCredentialsAndInvokeV2( + ArgumentMatchers.isA(DescribeLogGroupsRequest.class), + ArgumentMatchers.any() + ); + final AwsServiceException exception = CloudWatchLogsException.builder() + .awsErrorDetails(AwsErrorDetails.builder() + .errorCode("InternalFailure") + .build()) + .build(); + doThrow(exception) + .when(proxy) + .injectCredentialsAndInvokeV2( + ArgumentMatchers.isA(ListTagsLogGroupRequest.class), + ArgumentMatchers.any() + ); + + final ResourceModel model = ResourceModel.builder() + .logGroupName("LogGroup") + .retentionInDays(1) + .kmsKeyId("arn:aws:kms:us-east-1:$123456789012:key/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa") + .build(); + + final ResourceHandlerRequest request = ResourceHandlerRequest.builder() + .desiredResourceState(model) + .build(); + + assertThrows(AwsServiceException.class, + () -> handler.handleRequest(proxy, request, null, logger)); + } } From f5fd0dc7e9d18d85a4c5ba270dc583405af2c6c6 Mon Sep 17 00:00:00 2001 From: gruebel Date: Sun, 13 Dec 2020 21:41:46 +0100 Subject: [PATCH 6/7] Add missing stack tags --- .../amazon/logs/loggroup/Configuration.java | 10 +- .../amazon/logs/loggroup/CreateHandler.java | 2 +- .../amazon/logs/loggroup/Translator.java | 13 +- .../amazon/logs/loggroup/UpdateHandler.java | 45 ++++-- .../logs/loggroup/ConfigurationTest.java | 28 ++++ .../logs/loggroup/CreateHandlerTest.java | 34 ++-- .../amazon/logs/loggroup/TranslatorTest.java | 4 +- .../logs/loggroup/UpdateHandlerTest.java | 149 +++++++++--------- 8 files changed, 171 insertions(+), 114 deletions(-) create mode 100644 aws-logs-loggroup/src/test/java/software/amazon/logs/loggroup/ConfigurationTest.java diff --git a/aws-logs-loggroup/src/main/java/software/amazon/logs/loggroup/Configuration.java b/aws-logs-loggroup/src/main/java/software/amazon/logs/loggroup/Configuration.java index 2c95be4..6f9900c 100644 --- a/aws-logs-loggroup/src/main/java/software/amazon/logs/loggroup/Configuration.java +++ b/aws-logs-loggroup/src/main/java/software/amazon/logs/loggroup/Configuration.java @@ -2,8 +2,10 @@ import org.json.JSONObject; import org.json.JSONTokener; +import software.amazon.awssdk.utils.CollectionUtils; import java.util.Map; +import java.util.stream.Collectors; class Configuration extends BaseConfiguration { @@ -16,6 +18,12 @@ public JSONObject resourceSchemaJSONObject() { } public Map resourceDefinedTags(final ResourceModel resourceModel) { - return null; + if (CollectionUtils.isNullOrEmpty(resourceModel.getTags())) { + return null; + } + + return resourceModel.getTags() + .stream() + .collect(Collectors.toMap(Tag::getKey, Tag::getValue, (value1, value2) -> value2)); } } diff --git a/aws-logs-loggroup/src/main/java/software/amazon/logs/loggroup/CreateHandler.java b/aws-logs-loggroup/src/main/java/software/amazon/logs/loggroup/CreateHandler.java index 5fcbc94..79206ab 100644 --- a/aws-logs-loggroup/src/main/java/software/amazon/logs/loggroup/CreateHandler.java +++ b/aws-logs-loggroup/src/main/java/software/amazon/logs/loggroup/CreateHandler.java @@ -26,7 +26,7 @@ public ProgressEvent handleRequest( final ResourceModel model = request.getDesiredResourceState(); try { - proxy.injectCredentialsAndInvokeV2(Translator.translateToCreateRequest(model), + proxy.injectCredentialsAndInvokeV2(Translator.translateToCreateRequest(model, request.getDesiredResourceTags()), ClientBuilder.getClient()::createLogGroup); } catch (final ResourceAlreadyExistsException e) { throw new CfnAlreadyExistsException(ResourceModel.TYPE_NAME, diff --git a/aws-logs-loggroup/src/main/java/software/amazon/logs/loggroup/Translator.java b/aws-logs-loggroup/src/main/java/software/amazon/logs/loggroup/Translator.java index 1064b3c..9dff99d 100644 --- a/aws-logs-loggroup/src/main/java/software/amazon/logs/loggroup/Translator.java +++ b/aws-logs-loggroup/src/main/java/software/amazon/logs/loggroup/Translator.java @@ -12,6 +12,7 @@ import software.amazon.awssdk.services.cloudwatchlogs.model.AssociateKmsKeyRequest; import software.amazon.awssdk.services.cloudwatchlogs.model.TagLogGroupRequest; import software.amazon.awssdk.services.cloudwatchlogs.model.UntagLogGroupRequest; +import software.amazon.awssdk.utils.CollectionUtils; import java.util.Collection; import java.util.Collections; @@ -45,11 +46,11 @@ static DeleteLogGroupRequest translateToDeleteRequest(final ResourceModel model) .build(); } - static CreateLogGroupRequest translateToCreateRequest(final ResourceModel model) { + static CreateLogGroupRequest translateToCreateRequest(final ResourceModel model, final Map tags) { return CreateLogGroupRequest.builder() .logGroupName(model.getLogGroupName()) .kmsKeyId(model.getKmsKeyId()) - .tags(translateTagsToSdk(model.getTags())) + .tags(tags) .build(); } @@ -85,10 +86,10 @@ static ListTagsLogGroupRequest translateToListTagsLogGroupRequest(final String l .build(); } - static TagLogGroupRequest translateToTagLogGroupRequest(final String logGroupName, final Set tags) { + static TagLogGroupRequest translateToTagLogGroupRequest(final String logGroupName, final Map tags) { return TagLogGroupRequest.builder() .logGroupName(logGroupName) - .tags(translateTagsToSdk(tags)) + .tags(tags) .build(); } @@ -163,14 +164,14 @@ static String buildResourceDoesNotExistErrorMessage(final String resourceIdentif } static Map translateTagsToSdk(final Set tags) { - if (tags == null || tags.isEmpty()) { + if (CollectionUtils.isNullOrEmpty(tags)) { return null; } return tags.stream().collect(Collectors.toMap(Tag::getKey, Tag::getValue)); } static Set translateSdkToTags(final Map tags) { - if (tags == null || tags.isEmpty()) { + if (CollectionUtils.isNullOrEmpty(tags)) { return null; } return tags.entrySet().stream().map(tag -> new Tag(tag.getKey(), tag.getValue())) diff --git a/aws-logs-loggroup/src/main/java/software/amazon/logs/loggroup/UpdateHandler.java b/aws-logs-loggroup/src/main/java/software/amazon/logs/loggroup/UpdateHandler.java index 14f33d7..0919385 100644 --- a/aws-logs-loggroup/src/main/java/software/amazon/logs/loggroup/UpdateHandler.java +++ b/aws-logs-loggroup/src/main/java/software/amazon/logs/loggroup/UpdateHandler.java @@ -1,6 +1,7 @@ package software.amazon.logs.loggroup; -import com.google.common.collect.Sets; +import com.google.common.collect.MapDifference; +import com.google.common.collect.Maps; import software.amazon.cloudformation.exceptions.CfnInternalFailureException; import software.amazon.cloudformation.exceptions.CfnResourceConflictException; import software.amazon.cloudformation.exceptions.CfnServiceInternalErrorException; @@ -17,12 +18,14 @@ import software.amazon.awssdk.services.cloudwatchlogs.model.ResourceNotFoundException; import software.amazon.awssdk.services.cloudwatchlogs.model.ServiceUnavailableException; -import java.util.HashSet; +import java.util.ArrayList; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Objects; import java.util.Optional; -import java.util.Set; import java.util.stream.Collectors; +import java.util.stream.Stream; public class UpdateHandler extends BaseHandler { @@ -35,9 +38,12 @@ public ProgressEvent handleRequest( final ResourceModel model = request.getDesiredResourceState(); final ResourceModel previousModel = request.getPreviousResourceState(); + final Map tags = request.getDesiredResourceTags(); + final Map previousTags = request.getPreviousResourceTags(); + final boolean retentionChanged = ! retentionUnchanged(previousModel, model); final boolean kmsKeyChanged = ! kmsKeyUnchanged(previousModel, model); - final boolean tagsChanged = ! tagsUnchanged(previousModel, model); + final boolean tagsChanged = ! tagsUnchanged(previousTags, tags); if (retentionChanged && model.getRetentionInDays() == null) { deleteRetentionPolicy(proxy, request, logger); } else if (retentionChanged){ @@ -54,7 +60,7 @@ public ProgressEvent handleRequest( } if (tagsChanged) { - updateTags(proxy, previousModel, model, logger); + updateTags(proxy, model, previousTags, tags, logger); } return ProgressEvent.defaultSuccessHandler(model); @@ -159,16 +165,21 @@ private void associateKmsKey(final AmazonWebServicesClientProxy proxy, } private void updateTags(final AmazonWebServicesClientProxy proxy, - final ResourceModel previousModel, final ResourceModel model, + final Map previousTags, + final Map tags, final Logger logger) { - final Set previousTags = Optional.ofNullable(previousModel).map(ResourceModel::getTags).orElse(new HashSet<>()); - final Set newTags = Optional.ofNullable(model.getTags()).orElse(new HashSet<>()); - final Set tagsToRemove = Sets.difference(previousTags, newTags); - final Set tagsToAdd = Sets.difference(newTags, previousTags); + MapDifference tagsDifference = Maps.difference(Optional.ofNullable(previousTags).orElse(new HashMap<>()), + Optional.ofNullable(tags).orElse(new HashMap<>())); + final Map tagsToRemove = tagsDifference.entriesOnlyOnLeft(); + final Map tagsToAdd = tagsDifference.entriesOnlyOnRight(); + final Map tagsToDiffer = tagsDifference.entriesDiffering().entrySet().stream() + .collect(Collectors.toMap(Map.Entry::getKey, tag -> tag.getValue().rightValue())); + final Map tagsToUpdate = Stream.concat(tagsToAdd.entrySet().stream(), tagsToDiffer.entrySet().stream()) + .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); try { if (!tagsToRemove.isEmpty()) { - final List tagKeys = tagsToRemove.stream().map(Tag::getKey).collect(Collectors.toList()); + final List tagKeys = new ArrayList<>(tagsToRemove.keySet()); proxy.injectCredentialsAndInvokeV2(Translator.translateToUntagLogGroupRequest(model.getLogGroupName(), tagKeys), ClientBuilder.getClient()::untagLogGroup); @@ -177,13 +188,13 @@ private void updateTags(final AmazonWebServicesClientProxy proxy, ResourceModel.TYPE_NAME, model.getLogGroupName(), tagKeys); logger.log(message); } - if(!tagsToAdd.isEmpty()) { - proxy.injectCredentialsAndInvokeV2(Translator.translateToTagLogGroupRequest(model.getLogGroupName(), tagsToAdd), + if(!tagsToUpdate.isEmpty()) { + proxy.injectCredentialsAndInvokeV2(Translator.translateToTagLogGroupRequest(model.getLogGroupName(), tagsToUpdate), ClientBuilder.getClient()::tagLogGroup); final String message = String.format("%s [%s] successfully added tags: [%s]", - ResourceModel.TYPE_NAME, model.getLogGroupName(), tagsToAdd); + ResourceModel.TYPE_NAME, model.getLogGroupName(), tagsToUpdate); logger.log(message); } } catch (final ResourceNotFoundException e) { @@ -207,10 +218,10 @@ private static boolean kmsKeyUnchanged(final ResourceModel previousModel, final return (previousModel != null && Objects.equals(model.getKmsKeyId(), previousModel.getKmsKeyId())); } - private static boolean tagsUnchanged(final ResourceModel previousModel, final ResourceModel model) { - if (previousModel == null && model.getTags() == null) { + private static boolean tagsUnchanged(final Map previousTags, final Map tags) { + if (previousTags == null && tags == null) { return true; } - return (previousModel != null && Objects.equals(model.getTags(), previousModel.getTags())); + return (previousTags != null && Objects.equals(previousTags, tags)); } } diff --git a/aws-logs-loggroup/src/test/java/software/amazon/logs/loggroup/ConfigurationTest.java b/aws-logs-loggroup/src/test/java/software/amazon/logs/loggroup/ConfigurationTest.java new file mode 100644 index 0000000..c4fe0ed --- /dev/null +++ b/aws-logs-loggroup/src/test/java/software/amazon/logs/loggroup/ConfigurationTest.java @@ -0,0 +1,28 @@ +package software.amazon.logs.loggroup; + +import org.junit.jupiter.api.Test; + +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.Set; + +import static org.assertj.core.api.Assertions.assertThat; + +class ConfigurationTest { + + @Test + public void testResourceDefinedTags_MergeDuplicateKeys() { + final Set tags = new HashSet<>(Arrays.asList( + Tag.builder().key("key-1").value("value-1").build(), + Tag.builder().key("key-1").value("value-2").build() + )); + final ResourceModel model = ResourceModel.builder() + .tags(tags) + .build(); + + final Configuration configuration = new Configuration(); + + assertThat(configuration.resourceDefinedTags(model)).isEqualTo(Collections.singletonMap("key-1", "value-2")); + } +} diff --git a/aws-logs-loggroup/src/test/java/software/amazon/logs/loggroup/CreateHandlerTest.java b/aws-logs-loggroup/src/test/java/software/amazon/logs/loggroup/CreateHandlerTest.java index db1c92a..eb4c942 100644 --- a/aws-logs-loggroup/src/test/java/software/amazon/logs/loggroup/CreateHandlerTest.java +++ b/aws-logs-loggroup/src/test/java/software/amazon/logs/loggroup/CreateHandlerTest.java @@ -1,5 +1,9 @@ package software.amazon.logs.loggroup; +import org.mockito.ArgumentCaptor; +import software.amazon.awssdk.services.cloudwatchlogs.model.CloudWatchLogsRequest; +import software.amazon.awssdk.services.cloudwatchlogs.model.CreateLogGroupRequest; +import software.amazon.awssdk.services.cloudwatchlogs.model.PutRetentionPolicyRequest; import software.amazon.cloudformation.exceptions.CfnAlreadyExistsException; import software.amazon.cloudformation.proxy.AmazonWebServicesClientProxy; import software.amazon.cloudformation.proxy.Logger; @@ -18,12 +22,9 @@ import software.amazon.awssdk.services.cloudwatchlogs.model.PutRetentionPolicyResponse; import software.amazon.awssdk.services.cloudwatchlogs.model.ResourceAlreadyExistsException; -import java.util.Arrays; import java.util.Collections; import java.util.HashMap; -import java.util.HashSet; import java.util.Map; -import java.util.Set; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -58,10 +59,10 @@ public void handleRequest_Success() { .retentionInDays(1) .kmsKeyId("arn:aws:kms:us-east-1:$123456789012:key/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa") .build(); - final Set tags = new HashSet<>(Arrays.asList( - Tag.builder().key("key-1").value("value-1").build(), - Tag.builder().key("key-2").value("value-2").build() - )); + final Map tags = new HashMap() {{ + put("key-1", "value-1"); + put("key-2", "value-2"); + }}; doReturn(describeResponseInitial, createLogGroupResponse, putRetentionPolicyResponse) .when(proxy) @@ -74,22 +75,33 @@ public void handleRequest_Success() { .logGroupName("LogGroup") .retentionInDays(1) .kmsKeyId("arn:aws:kms:us-east-1:$123456789012:key/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa") - .tags(tags) .build(); final ResourceHandlerRequest request = ResourceHandlerRequest.builder() - .desiredResourceState(model) - .build(); + .desiredResourceState(model) + .desiredResourceTags(tags) + .build(); final ProgressEvent response = handler.handleRequest(proxy, request, null, logger); + ArgumentCaptor requests = ArgumentCaptor.forClass(CloudWatchLogsRequest.class); + verify(proxy, times(2)).injectCredentialsAndInvokeV2(requests.capture(), any()); + assertThat(requests.getAllValues().get(0)).isEqualTo(CreateLogGroupRequest.builder() + .logGroupName("LogGroup") + .kmsKeyId("arn:aws:kms:us-east-1:$123456789012:key/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa") + .tags(tags) + .build()); + assertThat(requests.getAllValues().get(1)).isEqualTo(PutRetentionPolicyRequest.builder() + .logGroupName("LogGroup") + .retentionInDays(1) + .build()); + assertThat(response).isNotNull(); assertThat(response.getStatus()).isEqualTo(OperationStatus.SUCCESS); assertThat(response.getCallbackContext()).isNull(); assertThat(response.getCallbackDelaySeconds()).isEqualTo(0); assertThat(response.getResourceModels()).isNull(); assertThat(response.getResourceModel()).isEqualToComparingOnlyGivenFields(logGroup); - assertThat(response.getResourceModel().getTags()).isEqualTo(tags); assertThat(response.getMessage()).isNull(); assertThat(response.getErrorCode()).isNull(); } diff --git a/aws-logs-loggroup/src/test/java/software/amazon/logs/loggroup/TranslatorTest.java b/aws-logs-loggroup/src/test/java/software/amazon/logs/loggroup/TranslatorTest.java index cc815ac..c76184c 100644 --- a/aws-logs-loggroup/src/test/java/software/amazon/logs/loggroup/TranslatorTest.java +++ b/aws-logs-loggroup/src/test/java/software/amazon/logs/loggroup/TranslatorTest.java @@ -79,7 +79,7 @@ public void testTranslateToCreate() { .kmsKeyId(RESOURCE_MODEL.getKmsKeyId()) .tags(MAP_TAGS) .build(); - assertThat(Translator.translateToCreateRequest(RESOURCE_MODEL)).isEqualToComparingFieldByField(request); + assertThat(Translator.translateToCreateRequest(RESOURCE_MODEL, MAP_TAGS)).isEqualToComparingFieldByField(request); } @Test @@ -141,7 +141,7 @@ public void testTranslateToTagLogGroupRequest() { .tags(MAP_TAGS) .build(); - assertThat(Translator.translateToTagLogGroupRequest(RESOURCE_MODEL.getLogGroupName(), SET_TAGS)) + assertThat(Translator.translateToTagLogGroupRequest(RESOURCE_MODEL.getLogGroupName(), MAP_TAGS)) .isEqualToComparingFieldByField(request); } diff --git a/aws-logs-loggroup/src/test/java/software/amazon/logs/loggroup/UpdateHandlerTest.java b/aws-logs-loggroup/src/test/java/software/amazon/logs/loggroup/UpdateHandlerTest.java index 708b0bb..018f6ad 100644 --- a/aws-logs-loggroup/src/test/java/software/amazon/logs/loggroup/UpdateHandlerTest.java +++ b/aws-logs-loggroup/src/test/java/software/amazon/logs/loggroup/UpdateHandlerTest.java @@ -1,6 +1,6 @@ package software.amazon.logs.loggroup; -import com.google.common.collect.Sets; +import com.google.common.collect.Maps; import software.amazon.awssdk.services.cloudwatchlogs.model.CloudWatchLogsRequest; import software.amazon.awssdk.services.cloudwatchlogs.model.DeleteRetentionPolicyResponse; import software.amazon.awssdk.services.cloudwatchlogs.model.DescribeLogGroupsResponse; @@ -26,12 +26,13 @@ import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; -import java.util.Arrays; +import java.util.ArrayList; import java.util.Collections; -import java.util.HashSet; +import java.util.HashMap; import java.util.List; -import java.util.Set; +import java.util.Map; import java.util.stream.Collectors; +import java.util.stream.Stream; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -67,10 +68,10 @@ public void handleRequest_Success() { .retentionInDays(1) .kmsKeyId("arn:aws:kms:us-east-1:$123456789012:key/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa") .build(); - final Set tags = new HashSet<>(Arrays.asList( - Tag.builder().key("key-1").value("value-1").build(), - Tag.builder().key("key-2").value("value-2").build() - )); + final Map tags = new HashMap() {{ + put("key-1", "value-1"); + put("key-2", "value-2"); + }}; final DescribeLogGroupsResponse describeResponse = DescribeLogGroupsResponse.builder() .logGroups(Collections.singletonList(logGroup)) .build(); @@ -86,11 +87,11 @@ public void handleRequest_Success() { .logGroupName("LogGroup") .retentionInDays(1) .kmsKeyId("arn:aws:kms:us-east-1:$123456789012:key/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa") - .tags(tags) .build(); final ResourceHandlerRequest request = ResourceHandlerRequest.builder() .desiredResourceState(model) + .desiredResourceTags(tags) .build(); final ProgressEvent response = handler.handleRequest(proxy, request, null, logger); @@ -101,7 +102,6 @@ public void handleRequest_Success() { assertThat(response.getCallbackDelaySeconds()).isEqualTo(0); assertThat(response.getResourceModels()).isNull(); assertThat(response.getResourceModel()).isEqualToComparingOnlyGivenFields(logGroup); - assertThat(response.getResourceModel().getTags()).isEqualTo(tags); assertThat(response.getMessage()).isNull(); assertThat(response.getErrorCode()).isNull(); } @@ -236,38 +236,40 @@ public void handleRequest_SuccessNoChange_NoAction_WithPreviousModel() { .retentionInDays(1) .kmsKeyId("arn:aws:kms:us-east-1:$123456789012:key/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa") .build(); - final Set tags = new HashSet<>(Arrays.asList( - Tag.builder().key("key-1").value("value-1").build(), - Tag.builder().key("key-2").value("value-2").build() - )); + final Map tags = new HashMap() {{ + put("key-1", "value-1"); + put("key-2", "value-2"); + }}; final ResourceModel previousModel = ResourceModel.builder() .logGroupName("LogGroup") .retentionInDays(1) .kmsKeyId("arn:aws:kms:us-east-1:$123456789012:key/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa") - .tags(tags) .build(); final ResourceModel model = ResourceModel.builder() .logGroupName("LogGroup") .retentionInDays(1) .kmsKeyId("arn:aws:kms:us-east-1:$123456789012:key/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa") - .tags(tags) .build(); final ResourceHandlerRequest request = ResourceHandlerRequest.builder() .previousResourceState(previousModel) .desiredResourceState(model) + .previousResourceTags(tags) + .desiredResourceTags(tags) .build(); final ProgressEvent response = handler.handleRequest(proxy, request, null, logger); + ArgumentCaptor requests = ArgumentCaptor.forClass(CloudWatchLogsRequest.class); + verify(proxy, times(0)).injectCredentialsAndInvokeV2(requests.capture(), any()); + assertThat(response).isNotNull(); assertThat(response.getStatus()).isEqualTo(OperationStatus.SUCCESS); assertThat(response.getCallbackContext()).isNull(); assertThat(response.getCallbackDelaySeconds()).isEqualTo(0); assertThat(response.getResourceModels()).isNull(); assertThat(response.getResourceModel()).isEqualToComparingOnlyGivenFields(logGroup); - assertThat(response.getResourceModel().getTags()).isEqualTo(tags); assertThat(response.getMessage()).isNull(); assertThat(response.getErrorCode()).isNull(); } @@ -279,25 +281,25 @@ public void handleRequest_Success_UpdateWith_RetentionAndKmsAndTags() { .retentionInDays(2) .kmsKeyId("arn:aws:kms:us-east-1:$123456789012:key/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa") .build(); - final Set tags = new HashSet<>(Arrays.asList( - Tag.builder().key("key-1").value("value-1").build(), - Tag.builder().key("key-2").value("value-2").build() - )); final ResourceModel previousModel = ResourceModel.builder() .logGroupName("LogGroup") .build(); + final Map tags = new HashMap() {{ + put("key-1", "value-1"); + put("key-2", "value-2"); + }}; final ResourceModel model = ResourceModel.builder() .logGroupName("LogGroup") .retentionInDays(2) .kmsKeyId("arn:aws:kms:us-east-1:$123456789012:key/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa") - .tags(tags) .build(); final ResourceHandlerRequest request = ResourceHandlerRequest.builder() - .previousResourceState(previousModel) - .desiredResourceState(model) - .build(); + .previousResourceState(previousModel) + .desiredResourceState(model) + .desiredResourceTags(tags) + .build(); final ProgressEvent response = handler.handleRequest(proxy, request, null, logger); @@ -315,7 +317,7 @@ public void handleRequest_Success_UpdateWith_RetentionAndKmsAndTags() { assertThat(requests.getAllValues().get(2)).isEqualTo(TagLogGroupRequest.builder() .logGroupName("LogGroup") - .tags(Translator.translateTagsToSdk(tags)) + .tags(tags) .build()); assertThat(response).isNotNull(); @@ -324,7 +326,6 @@ public void handleRequest_Success_UpdateWith_RetentionAndKmsAndTags() { assertThat(response.getCallbackDelaySeconds()).isEqualTo(0); assertThat(response.getResourceModels()).isNull(); assertThat(response.getResourceModel()).isEqualToComparingOnlyGivenFields(logGroup); - assertThat(response.getResourceModel().getTags()).isEqualTo(tags); assertThat(response.getMessage()).isNull(); assertThat(response.getErrorCode()).isNull(); } @@ -334,30 +335,29 @@ public void handleRequest_Success_AddTags() { final LogGroup logGroup = LogGroup.builder() .logGroupName("LogGroup") .build(); - final Set previousTags = new HashSet<>(Arrays.asList( - Tag.builder().key("key-1").value("value-1").build(), - Tag.builder().key("key-2").value("value-2").build() - )); + final Map previousTags = new HashMap() {{ + put("key-1", "value-1"); + put("key-2", "value-2"); + }}; final ResourceModel previousModel = ResourceModel.builder() .logGroupName("LogGroup") - .tags(previousTags) .build(); - final Set newTags = new HashSet<>(Arrays.asList( - Tag.builder().key("key-3").value("value-3").build(), - Tag.builder().key("key-4").value("value-4").build() - )); - final Set tags = new HashSet<>(); - tags.addAll(previousTags); - tags.addAll(newTags); + final Map newTags = new HashMap() {{ + put("key-3", "value-3"); + put("key-4", "value-4"); + }}; + final Map tags = Stream.concat(previousTags.entrySet().stream(), newTags.entrySet().stream()) + .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); final ResourceModel model = ResourceModel.builder() .logGroupName("LogGroup") - .tags(tags) .build(); final ResourceHandlerRequest request = ResourceHandlerRequest.builder() .previousResourceState(previousModel) .desiredResourceState(model) + .previousResourceTags(previousTags) + .desiredResourceTags(tags) .build(); final ProgressEvent response = handler.handleRequest(proxy, request, null, logger); @@ -366,7 +366,7 @@ public void handleRequest_Success_AddTags() { verify(proxy, times(1)).injectCredentialsAndInvokeV2(requests.capture(), any()); assertThat(requests.getAllValues().get(0)).isEqualTo(TagLogGroupRequest.builder() .logGroupName("LogGroup") - .tags(Translator.translateTagsToSdk(newTags)) + .tags(newTags) .build()); assertThat(response).isNotNull(); @@ -375,7 +375,6 @@ public void handleRequest_Success_AddTags() { assertThat(response.getCallbackDelaySeconds()).isEqualTo(0); assertThat(response.getResourceModels()).isNull(); assertThat(response.getResourceModel()).isEqualToComparingOnlyGivenFields(logGroup); - assertThat(response.getResourceModel().getTags()).isEqualTo(tags); assertThat(response.getMessage()).isNull(); assertThat(response.getErrorCode()).isNull(); } @@ -385,41 +384,41 @@ public void handleRequest_Success_UpdateTags() { final LogGroup logGroup = LogGroup.builder() .logGroupName("LogGroup") .build(); - final Set previousTags = new HashSet<>(Arrays.asList( - Tag.builder().key("key-1").value("value-1").build(), - Tag.builder().key("key-2").value("value-2").build() - )); + final Map previousTags = new HashMap() {{ + put("key-1", "value-1"); + put("key-2", "value-2"); + }}; final ResourceModel previousModel = ResourceModel.builder() .logGroupName("LogGroup") - .tags(previousTags) .build(); - final Set tags = new HashSet<>(Arrays.asList( - Tag.builder().key("key-2").value("value-2-new").build(), - Tag.builder().key("key-3").value("value-3").build() - )); + final Map tags = new HashMap() {{ + put("key-2", "value-2-new"); + put("key-3", "value-3"); + }}; final ResourceModel model = ResourceModel.builder() .logGroupName("LogGroup") - .tags(tags) .build(); final ResourceHandlerRequest request = ResourceHandlerRequest.builder() .previousResourceState(previousModel) .desiredResourceState(model) + .previousResourceTags(previousTags) + .desiredResourceTags(tags) .build(); final ProgressEvent response = handler.handleRequest(proxy, request, null, logger); ArgumentCaptor requests = ArgumentCaptor.forClass(CloudWatchLogsRequest.class); verify(proxy, times(2)).injectCredentialsAndInvokeV2(requests.capture(), any()); - final List removedTagKeys = Sets.difference(previousTags, tags).stream().map(Tag::getKey).collect(Collectors.toList()); + final List removedTagKeys = new ArrayList<>(Maps.difference(previousTags, tags).entriesOnlyOnLeft().keySet()); assertThat(requests.getAllValues().get(0)).isEqualTo(UntagLogGroupRequest.builder() .logGroupName("LogGroup") .tags(removedTagKeys) .build()); assertThat(requests.getAllValues().get(1)).isEqualTo(TagLogGroupRequest.builder() .logGroupName("LogGroup") - .tags(Translator.translateTagsToSdk(tags)) + .tags(tags) .build()); assertThat(response).isNotNull(); @@ -428,7 +427,6 @@ public void handleRequest_Success_UpdateTags() { assertThat(response.getCallbackDelaySeconds()).isEqualTo(0); assertThat(response.getResourceModels()).isNull(); assertThat(response.getResourceModel()).isEqualToComparingOnlyGivenFields(logGroup); - assertThat(response.getResourceModel().getTags()).isEqualTo(tags); assertThat(response.getMessage()).isNull(); assertThat(response.getErrorCode()).isNull(); } @@ -438,13 +436,12 @@ public void handleRequest_Success_RemoveTags() { final LogGroup logGroup = LogGroup.builder() .logGroupName("LogGroup") .build(); - final Set tags = new HashSet<>(Arrays.asList( - Tag.builder().key("key-1").value("value-1").build(), - Tag.builder().key("key-2").value("value-2").build() - )); + final Map previousTags = new HashMap() {{ + put("key-1", "value-1"); + put("key-2", "value-2"); + }}; final ResourceModel previousModel = ResourceModel.builder() .logGroupName("LogGroup") - .tags(tags) .build(); final ResourceModel model = ResourceModel.builder() @@ -454,13 +451,14 @@ public void handleRequest_Success_RemoveTags() { final ResourceHandlerRequest request = ResourceHandlerRequest.builder() .previousResourceState(previousModel) .desiredResourceState(model) + .previousResourceTags(previousTags) .build(); final ProgressEvent response = handler.handleRequest(proxy, request, null, logger); ArgumentCaptor requests = ArgumentCaptor.forClass(CloudWatchLogsRequest.class); verify(proxy, times(1)).injectCredentialsAndInvokeV2(requests.capture(), any()); - final List removedTagKeys = tags.stream().map(Tag::getKey).collect(Collectors.toList()); + final List removedTagKeys = new ArrayList<>(previousTags.keySet()); assertThat(requests.getAllValues().get(0)).isEqualTo(UntagLogGroupRequest.builder() .logGroupName("LogGroup") .tags(removedTagKeys) @@ -472,7 +470,6 @@ public void handleRequest_Success_RemoveTags() { assertThat(response.getCallbackDelaySeconds()).isEqualTo(0); assertThat(response.getResourceModels()).isNull(); assertThat(response.getResourceModel()).isEqualToComparingOnlyGivenFields(logGroup); - assertThat(response.getResourceModel().getTags()).isNull(); assertThat(response.getMessage()).isNull(); assertThat(response.getErrorCode()).isNull(); } @@ -792,18 +789,18 @@ public void handleRequest_AddTags_FailureNotFound_ServiceException() { any() ); - final Set tags = new HashSet<>(Arrays.asList( - Tag.builder().key("key-1").value("value-1").build(), - Tag.builder().key("key-2").value("value-2").build() - )); + final Map tags = new HashMap() {{ + put("key-1", "value-1"); + put("key-2", "value-2"); + }}; final ResourceModel model = ResourceModel.builder() .logGroupName("LogGroup") - .tags(tags) .build(); final ResourceHandlerRequest request = ResourceHandlerRequest.builder() .previousResourceState(previousModel) .desiredResourceState(model) + .desiredResourceTags(tags) .build(); assertThrows(software.amazon.cloudformation.exceptions.ResourceNotFoundException.class, @@ -822,15 +819,15 @@ public void handleRequest_AddTags_InvalidParameter_ServiceException() { any() ); - final Set tags = Collections.singleton(Tag.builder().key("key-1").value("value-1").build()); + final Map tags = Collections.singletonMap("key-1", "value-1"); final ResourceModel model = ResourceModel.builder() .logGroupName("LogGroup") - .tags(tags) .build(); final ResourceHandlerRequest request = ResourceHandlerRequest.builder() .previousResourceState(previousModel) .desiredResourceState(model) + .desiredResourceTags(tags) .build(); assertThrows(software.amazon.cloudformation.exceptions.CfnInternalFailureException.class, @@ -839,13 +836,12 @@ public void handleRequest_AddTags_InvalidParameter_ServiceException() { @Test public void handleRequest_RemoveTags_FailureNotFound_ServiceException() { - final Set previousTags = new HashSet<>(Arrays.asList( - Tag.builder().key("key-1").value("value-1").build(), - Tag.builder().key("key-2").value("value-2").build() - )); + final Map previousTags = new HashMap() {{ + put("key-1", "value-1"); + put("key-2", "value-2"); + }}; final ResourceModel previousModel = ResourceModel.builder() .logGroupName("LogGroup") - .tags(previousTags) .build(); doThrow(software.amazon.awssdk.services.cloudwatchlogs.model.ResourceNotFoundException.class) .when(proxy) @@ -861,6 +857,7 @@ public void handleRequest_RemoveTags_FailureNotFound_ServiceException() { final ResourceHandlerRequest request = ResourceHandlerRequest.builder() .previousResourceState(previousModel) .desiredResourceState(model) + .previousResourceTags(previousTags) .build(); assertThrows(software.amazon.cloudformation.exceptions.ResourceNotFoundException.class, @@ -870,10 +867,9 @@ public void handleRequest_RemoveTags_FailureNotFound_ServiceException() { @Test public void handleRequest_RemoveTags_InvalidParameter_ServiceException() { - final Set tags = Collections.singleton(Tag.builder().key("key-1").value("value-1").build()); + final Map previousTags = Collections.singletonMap("key-1", "value-1"); final ResourceModel previousModel = ResourceModel.builder() .logGroupName("LogGroup") - .tags(tags) .build(); doThrow(software.amazon.awssdk.services.cloudwatchlogs.model.InvalidParameterException.class) .when(proxy) @@ -889,6 +885,7 @@ public void handleRequest_RemoveTags_InvalidParameter_ServiceException() { final ResourceHandlerRequest request = ResourceHandlerRequest.builder() .previousResourceState(previousModel) .desiredResourceState(model) + .previousResourceTags(previousTags) .build(); assertThrows(software.amazon.cloudformation.exceptions.CfnInternalFailureException.class, From e427257b8ead59b35d81b5a5c621a4debd34590b Mon Sep 17 00:00:00 2001 From: gruebel Date: Mon, 10 May 2021 23:22:23 +0200 Subject: [PATCH 7/7] Silent fail when add or remove tags permission is missing --- .../amazon/logs/loggroup/ReadHandler.java | 4 +- .../amazon/logs/loggroup/Translator.java | 3 + .../amazon/logs/loggroup/UpdateHandler.java | 8 + .../amazon/logs/loggroup/ReadHandlerTest.java | 1 - .../logs/loggroup/UpdateHandlerTest.java | 152 ++++++++++++++++++ 5 files changed, 164 insertions(+), 4 deletions(-) diff --git a/aws-logs-loggroup/src/main/java/software/amazon/logs/loggroup/ReadHandler.java b/aws-logs-loggroup/src/main/java/software/amazon/logs/loggroup/ReadHandler.java index caeecd7..0ff0ff6 100644 --- a/aws-logs-loggroup/src/main/java/software/amazon/logs/loggroup/ReadHandler.java +++ b/aws-logs-loggroup/src/main/java/software/amazon/logs/loggroup/ReadHandler.java @@ -13,8 +13,6 @@ public class ReadHandler extends BaseHandler { - private static final String ACCESS_DENIED_ERROR_CODE = "AccessDeniedException"; - @Override public ProgressEvent handleRequest( final AmazonWebServicesClientProxy proxy, @@ -37,7 +35,7 @@ public ProgressEvent handleRequest( tagsResponse = proxy.injectCredentialsAndInvokeV2(Translator.translateToListTagsLogGroupRequest(model.getLogGroupName()), ClientBuilder.getClient()::listTagsLogGroup); } catch (final CloudWatchLogsException e) { - if (ACCESS_DENIED_ERROR_CODE.equals(e.awsErrorDetails().errorCode())) { + if (Translator.ACCESS_DENIED_ERROR_CODE.equals(e.awsErrorDetails().errorCode())) { // fail silently, if there is no permission to list tags logger.log(e.getMessage()); } else { diff --git a/aws-logs-loggroup/src/main/java/software/amazon/logs/loggroup/Translator.java b/aws-logs-loggroup/src/main/java/software/amazon/logs/loggroup/Translator.java index 9dff99d..9eb9e33 100644 --- a/aws-logs-loggroup/src/main/java/software/amazon/logs/loggroup/Translator.java +++ b/aws-logs-loggroup/src/main/java/software/amazon/logs/loggroup/Translator.java @@ -25,6 +25,9 @@ import java.util.stream.Stream; final class Translator { + + static final String ACCESS_DENIED_ERROR_CODE = "AccessDeniedException"; + private Translator() {} static DescribeLogGroupsRequest translateToReadRequest(final ResourceModel model) { diff --git a/aws-logs-loggroup/src/main/java/software/amazon/logs/loggroup/UpdateHandler.java b/aws-logs-loggroup/src/main/java/software/amazon/logs/loggroup/UpdateHandler.java index 0919385..3feccd6 100644 --- a/aws-logs-loggroup/src/main/java/software/amazon/logs/loggroup/UpdateHandler.java +++ b/aws-logs-loggroup/src/main/java/software/amazon/logs/loggroup/UpdateHandler.java @@ -9,6 +9,7 @@ import software.amazon.cloudformation.proxy.Logger; import software.amazon.cloudformation.proxy.ProgressEvent; import software.amazon.cloudformation.proxy.ResourceHandlerRequest; +import software.amazon.awssdk.services.cloudwatchlogs.model.CloudWatchLogsException; import software.amazon.awssdk.services.cloudwatchlogs.model.DeleteRetentionPolicyRequest; import software.amazon.awssdk.services.cloudwatchlogs.model.PutRetentionPolicyRequest; import software.amazon.awssdk.services.cloudwatchlogs.model.DisassociateKmsKeyRequest; @@ -201,6 +202,13 @@ private void updateTags(final AmazonWebServicesClientProxy proxy, throwNotFoundException(model); } catch (final InvalidParameterException e) { throw new CfnInternalFailureException(e); + } catch (final CloudWatchLogsException e) { + if (Translator.ACCESS_DENIED_ERROR_CODE.equals(e.awsErrorDetails().errorCode())) { + // fail silently, if there is no permission to list tags + logger.log(e.getMessage()); + } else { + throw e; + } } } diff --git a/aws-logs-loggroup/src/test/java/software/amazon/logs/loggroup/ReadHandlerTest.java b/aws-logs-loggroup/src/test/java/software/amazon/logs/loggroup/ReadHandlerTest.java index d18aee0..99e4b0e 100644 --- a/aws-logs-loggroup/src/test/java/software/amazon/logs/loggroup/ReadHandlerTest.java +++ b/aws-logs-loggroup/src/test/java/software/amazon/logs/loggroup/ReadHandlerTest.java @@ -27,7 +27,6 @@ import java.util.Set; import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.doThrow; diff --git a/aws-logs-loggroup/src/test/java/software/amazon/logs/loggroup/UpdateHandlerTest.java b/aws-logs-loggroup/src/test/java/software/amazon/logs/loggroup/UpdateHandlerTest.java index 018f6ad..28d8c48 100644 --- a/aws-logs-loggroup/src/test/java/software/amazon/logs/loggroup/UpdateHandlerTest.java +++ b/aws-logs-loggroup/src/test/java/software/amazon/logs/loggroup/UpdateHandlerTest.java @@ -1,6 +1,9 @@ package software.amazon.logs.loggroup; import com.google.common.collect.Maps; +import software.amazon.awssdk.awscore.exception.AwsErrorDetails; +import software.amazon.awssdk.awscore.exception.AwsServiceException; +import software.amazon.awssdk.services.cloudwatchlogs.model.CloudWatchLogsException; import software.amazon.awssdk.services.cloudwatchlogs.model.CloudWatchLogsRequest; import software.amazon.awssdk.services.cloudwatchlogs.model.DeleteRetentionPolicyResponse; import software.amazon.awssdk.services.cloudwatchlogs.model.DescribeLogGroupsResponse; @@ -834,6 +837,80 @@ public void handleRequest_AddTags_InvalidParameter_ServiceException() { () -> handler.handleRequest(proxy, request, null, logger)); } + @Test + public void handleRequest_AddTags_AccessDenied_NoException() { + final ResourceModel previousModel = ResourceModel.builder() + .logGroupName("LogGroup") + .build(); + + final AwsServiceException exception = CloudWatchLogsException.builder() + .awsErrorDetails(AwsErrorDetails.builder() + .errorCode("AccessDeniedException") + .build()) + .build(); + doThrow(exception) + .when(proxy) + .injectCredentialsAndInvokeV2( + ArgumentMatchers.isA(TagLogGroupRequest.class), + any() + ); + + final Map tags = Collections.singletonMap("key-1", "value-1"); + final ResourceModel model = ResourceModel.builder() + .logGroupName("LogGroup") + .build(); + + final ResourceHandlerRequest request = ResourceHandlerRequest.builder() + .previousResourceState(previousModel) + .desiredResourceState(model) + .desiredResourceTags(tags) + .build(); + + final ProgressEvent response = handler.handleRequest(proxy, request, null, logger); + + assertThat(response).isNotNull(); + assertThat(response.getStatus()).isEqualTo(OperationStatus.SUCCESS); + assertThat(response.getCallbackContext()).isNull(); + assertThat(response.getCallbackDelaySeconds()).isEqualTo(0); + assertThat(response.getResourceModels()).isNull(); + assertThat(response.getResourceModel()).isEqualToComparingOnlyGivenFields(model); + assertThat(response.getMessage()).isNull(); + assertThat(response.getErrorCode()).isNull(); + } + + @Test + public void handleRequest_AddTags_InternalFailure_WithException() { + final ResourceModel previousModel = ResourceModel.builder() + .logGroupName("LogGroup") + .build(); + + final AwsServiceException exception = CloudWatchLogsException.builder() + .awsErrorDetails(AwsErrorDetails.builder() + .errorCode("InternalFailure") + .build()) + .build(); + doThrow(exception) + .when(proxy) + .injectCredentialsAndInvokeV2( + ArgumentMatchers.isA(TagLogGroupRequest.class), + any() + ); + + final Map tags = Collections.singletonMap("key-1", "value-1"); + final ResourceModel model = ResourceModel.builder() + .logGroupName("LogGroup") + .build(); + + final ResourceHandlerRequest request = ResourceHandlerRequest.builder() + .previousResourceState(previousModel) + .desiredResourceState(model) + .desiredResourceTags(tags) + .build(); + + assertThrows(AwsServiceException.class, + () -> handler.handleRequest(proxy, request, null, logger)); + } + @Test public void handleRequest_RemoveTags_FailureNotFound_ServiceException() { final Map previousTags = new HashMap() {{ @@ -891,4 +968,79 @@ public void handleRequest_RemoveTags_InvalidParameter_ServiceException() { assertThrows(software.amazon.cloudformation.exceptions.CfnInternalFailureException.class, () -> handler.handleRequest(proxy, request, null, logger)); } + + @Test + public void handleRequest_RemoveTags_AccessDenied_NoException() { + final Map previousTags = Collections.singletonMap("key-1", "value-1"); + final ResourceModel previousModel = ResourceModel.builder() + .logGroupName("LogGroup") + .build(); + + final AwsServiceException exception = CloudWatchLogsException.builder() + .awsErrorDetails(AwsErrorDetails.builder() + .errorCode("AccessDeniedException") + .build()) + .build(); + doThrow(exception) + .when(proxy) + .injectCredentialsAndInvokeV2( + ArgumentMatchers.isA(UntagLogGroupRequest.class), + any() + ); + + final ResourceModel model = ResourceModel.builder() + .logGroupName("LogGroup") + .build(); + + final ResourceHandlerRequest request = ResourceHandlerRequest.builder() + .previousResourceState(previousModel) + .desiredResourceState(model) + .previousResourceTags(previousTags) + .build(); + + final ProgressEvent response = handler.handleRequest(proxy, request, null, logger); + + assertThat(response).isNotNull(); + assertThat(response.getStatus()).isEqualTo(OperationStatus.SUCCESS); + assertThat(response.getCallbackContext()).isNull(); + assertThat(response.getCallbackDelaySeconds()).isEqualTo(0); + assertThat(response.getResourceModels()).isNull(); + assertThat(response.getResourceModel()).isEqualToComparingOnlyGivenFields(model); + assertThat(response.getMessage()).isNull(); + assertThat(response.getErrorCode()).isNull(); + } + + @Test + public void handleRequest_RemoveTags_InternalFailure_WithException() { + final Map previousTags = Collections.singletonMap("key-1", "value-1"); + final ResourceModel previousModel = ResourceModel.builder() + .logGroupName("LogGroup") + .build(); + + final AwsServiceException exception = CloudWatchLogsException.builder() + .awsErrorDetails(AwsErrorDetails.builder() + .errorCode("InternalFailure") + .build()) + .build(); + doThrow(exception) + .when(proxy) + .injectCredentialsAndInvokeV2( + ArgumentMatchers.isA(UntagLogGroupRequest.class), + any() + ); + + final Map tags = Collections.singletonMap("key-1", "value-1"); + final ResourceModel model = ResourceModel.builder() + .logGroupName("LogGroup") + .build(); + + final ResourceHandlerRequest request = ResourceHandlerRequest.builder() + .previousResourceState(previousModel) + .desiredResourceState(model) + .previousResourceTags(previousTags) + .build(); + + assertThrows(AwsServiceException.class, + () -> handler.handleRequest(proxy, request, null, logger)); + } }