From cb96ed9775a9893e225a4809c9c15e5b9186d9d0 Mon Sep 17 00:00:00 2001 From: Valentin Shirshov Date: Mon, 9 Oct 2023 13:50:26 +0200 Subject: [PATCH 1/3] [All] Make sure that stack level tags do not override resource level tags --- .../amazon/rds/common/handler/Tagging.java | 19 +++++++++--- .../amazon/rds/dbcluster/BaseHandlerStd.java | 13 +++++++-- aws-rds-dbclusterendpoint/docs/README.md | 8 ++--- aws-rds-dbclusterendpoint/docs/tag.md | 6 ++-- .../rds/dbclusterendpoint/BaseHandlerStd.java | 14 +++++++-- .../BaseHandlerStd.java | 16 +++++++--- .../AbstractHandlerTest.java | 5 ++++ .../UpdateHandlerTest.java | 6 ++-- .../amazon/rds/dbinstance/BaseHandlerStd.java | 14 +++++++-- .../rds/dbinstance/UpdateHandlerTest.java | 16 +++++----- .../rds/dbparametergroup/BaseHandlerStd.java | 16 +++++++--- .../rds/dbsubnetgroup/BaseHandlerStd.java | 23 +++++++++++---- .../rds/eventsubscription/BaseHandlerStd.java | 16 +++++++--- .../rds/optiongroup/BaseHandlerStd.java | 29 +++++++++---------- 14 files changed, 137 insertions(+), 64 deletions(-) diff --git a/aws-rds-cfn-common/src/main/java/software/amazon/rds/common/handler/Tagging.java b/aws-rds-cfn-common/src/main/java/software/amazon/rds/common/handler/Tagging.java index ea276cc7c..451eb66ee 100644 --- a/aws-rds-cfn-common/src/main/java/software/amazon/rds/common/handler/Tagging.java +++ b/aws-rds-cfn-common/src/main/java/software/amazon/rds/common/handler/Tagging.java @@ -11,6 +11,7 @@ import com.amazonaws.util.CollectionUtils; import com.google.common.collect.Sets; +import javafx.util.Pair; import lombok.AllArgsConstructor; import lombok.Builder; import lombok.Data; @@ -68,13 +69,23 @@ public static TagSet exclude(final TagSet from, final TagSet what) { .build(); } - public static Map mergeTags(Map tagsMap1, Map tagsMap2) { - final Map result = new LinkedHashMap<>(); - result.putAll(Optional.ofNullable(tagsMap1).orElse(Collections.emptySortedMap())); - result.putAll(Optional.ofNullable(tagsMap2).orElse(Collections.emptySortedMap())); + public static Collection exclude(final Collection from, final Collection what) { + final Set result = new LinkedHashSet<>(from); + result.removeAll(what); return result; } + + public static Set translateTagsToSdk(final Collection tags) { + return Optional.ofNullable(tags).orElse(Collections.emptySet()) + .stream() + .map(tag -> Tag.builder() + .key(tag.key()) + .value(tag.value()) + .build()) + .collect(Collectors.toCollection(LinkedHashSet::new)); + } + public static Collection translateTagsToSdk(final TagSet tagSet) { //For backward compatibility, We will resolve duplicates tags between stack level tags and resource tags. final Map allTags = new LinkedHashMap<>(); diff --git a/aws-rds-dbcluster/src/main/java/software/amazon/rds/dbcluster/BaseHandlerStd.java b/aws-rds-dbcluster/src/main/java/software/amazon/rds/dbcluster/BaseHandlerStd.java index b3c55eb56..8f23fd03b 100644 --- a/aws-rds-dbcluster/src/main/java/software/amazon/rds/dbcluster/BaseHandlerStd.java +++ b/aws-rds-dbcluster/src/main/java/software/amazon/rds/dbcluster/BaseHandlerStd.java @@ -55,6 +55,7 @@ import software.amazon.awssdk.services.rds.model.StorageQuotaExceededException; import software.amazon.awssdk.services.rds.model.StorageTypeNotAvailableException; import software.amazon.awssdk.services.rds.model.StorageTypeNotSupportedException; +import software.amazon.awssdk.services.rds.model.Tag; import software.amazon.awssdk.utils.StringUtils; import software.amazon.cloudformation.exceptions.CfnNotStabilizedException; import software.amazon.cloudformation.proxy.AmazonWebServicesClientProxy; @@ -478,13 +479,19 @@ protected ProgressEvent updateTags( final Tagging.TagSet previousTags, final Tagging.TagSet desiredTags ) { - final Tagging.TagSet tagsToAdd = Tagging.exclude(desiredTags, previousTags); - final Tagging.TagSet tagsToRemove = Tagging.exclude(previousTags, desiredTags); + final Collection effectivePreviousTags = Tagging.translateTagsToSdk(previousTags); + final Collection effectiveDesiredTags = Tagging.translateTagsToSdk(desiredTags); + + final Collection tagsToRemove = Tagging.exclude(effectivePreviousTags, effectiveDesiredTags); + final Collection tagsToAdd = Tagging.exclude(effectiveDesiredTags, effectivePreviousTags); if (tagsToAdd.isEmpty() && tagsToRemove.isEmpty()) { return progress; } + final Tagging.TagSet rulesetTagsToAdd = Tagging.exclude(desiredTags, previousTags); + final Tagging.TagSet rulesetTagsToRemove = Tagging.exclude(previousTags, desiredTags); + DBCluster dbCluster; try { dbCluster = fetchDBCluster(rdsProxyClient, progress.getResourceModel()); @@ -501,7 +508,7 @@ protected ProgressEvent updateTags( return Commons.handleException( progress, exception, - DEFAULT_DB_CLUSTER_ERROR_RULE_SET.extendWith(Tagging.bestEffortErrorRuleSet(tagsToAdd, tagsToRemove)) + DEFAULT_DB_CLUSTER_ERROR_RULE_SET.extendWith(Tagging.bestEffortErrorRuleSet(rulesetTagsToAdd, rulesetTagsToRemove)) ); } diff --git a/aws-rds-dbclusterendpoint/docs/README.md b/aws-rds-dbclusterendpoint/docs/README.md index e2999be91..1eb5677ea 100644 --- a/aws-rds-dbclusterendpoint/docs/README.md +++ b/aws-rds-dbclusterendpoint/docs/README.md @@ -48,9 +48,9 @@ _Required_: Yes _Type_: String -_Minimum_: 1 +_Minimum Length_: 1 -_Maximum_: 63 +_Maximum Length_: 63 _Update requires_: [Replacement](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/using-cfn-updating-stacks-update-behaviors.html#update-replacement) @@ -62,9 +62,9 @@ _Required_: No _Type_: String -_Minimum_: 1 +_Minimum Length_: 1 -_Maximum_: 63 +_Maximum Length_: 63 _Update requires_: [Replacement](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/using-cfn-updating-stacks-update-behaviors.html#update-replacement) diff --git a/aws-rds-dbclusterendpoint/docs/tag.md b/aws-rds-dbclusterendpoint/docs/tag.md index 205b0d713..309af160c 100644 --- a/aws-rds-dbclusterendpoint/docs/tag.md +++ b/aws-rds-dbclusterendpoint/docs/tag.md @@ -32,9 +32,9 @@ _Required_: Yes _Type_: String -_Minimum_: 1 +_Minimum Length_: 1 -_Maximum_: 128 +_Maximum Length_: 128 _Update requires_: [No interruption](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/using-cfn-updating-stacks-update-behaviors.html#update-no-interrupt) @@ -46,6 +46,6 @@ _Required_: No _Type_: String -_Maximum_: 256 +_Maximum Length_: 256 _Update requires_: [No interruption](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/using-cfn-updating-stacks-update-behaviors.html#update-no-interrupt) diff --git a/aws-rds-dbclusterendpoint/src/main/java/software/amazon/rds/dbclusterendpoint/BaseHandlerStd.java b/aws-rds-dbclusterendpoint/src/main/java/software/amazon/rds/dbclusterendpoint/BaseHandlerStd.java index 26c6209d3..29562e074 100644 --- a/aws-rds-dbclusterendpoint/src/main/java/software/amazon/rds/dbclusterendpoint/BaseHandlerStd.java +++ b/aws-rds-dbclusterendpoint/src/main/java/software/amazon/rds/dbclusterendpoint/BaseHandlerStd.java @@ -1,6 +1,7 @@ package software.amazon.rds.dbclusterendpoint; import java.time.Duration; +import java.util.Collection; import java.util.Optional; import software.amazon.awssdk.services.rds.RdsClient; @@ -14,6 +15,7 @@ import software.amazon.awssdk.services.rds.model.InvalidDbClusterEndpointStateException; import software.amazon.awssdk.services.rds.model.InvalidDbClusterStateException; import software.amazon.awssdk.services.rds.model.InvalidDbInstanceStateException; +import software.amazon.awssdk.services.rds.model.Tag; import software.amazon.cloudformation.proxy.AmazonWebServicesClientProxy; import software.amazon.cloudformation.proxy.HandlerErrorCode; import software.amazon.cloudformation.proxy.Logger; @@ -97,13 +99,19 @@ protected ProgressEvent updateTags( final Tagging.TagSet previousTags, final Tagging.TagSet desiredTags ) { - final Tagging.TagSet tagsToAdd = Tagging.exclude(desiredTags, previousTags); - final Tagging.TagSet tagsToRemove = Tagging.exclude(previousTags, desiredTags); + final Collection effectivePreviousTags = Tagging.translateTagsToSdk(previousTags); + final Collection effectiveDesiredTags = Tagging.translateTagsToSdk(desiredTags); + + final Collection tagsToRemove = Tagging.exclude(effectivePreviousTags, effectiveDesiredTags); + final Collection tagsToAdd = Tagging.exclude(effectiveDesiredTags, effectivePreviousTags); if (tagsToAdd.isEmpty() && tagsToRemove.isEmpty()) { return progress; } + final Tagging.TagSet rulesetTagsToAdd = Tagging.exclude(desiredTags, previousTags); + final Tagging.TagSet rulesetTagsToRemove = Tagging.exclude(previousTags, desiredTags); + try { Tagging.removeTags(proxyClient, dbClusterEndpointArn, Tagging.translateTagsToSdk(tagsToRemove)); Tagging.addTags(proxyClient, dbClusterEndpointArn, Tagging.translateTagsToSdk(tagsToAdd)); @@ -111,7 +119,7 @@ protected ProgressEvent updateTags( return Commons.handleException( progress, exception, - DEFAULT_DB_CLUSTER_ENDPOINT_ERROR_RULE_SET.extendWith(Tagging.bestEffortErrorRuleSet(tagsToAdd, tagsToRemove)) + DEFAULT_DB_CLUSTER_ENDPOINT_ERROR_RULE_SET.extendWith(Tagging.bestEffortErrorRuleSet(rulesetTagsToAdd, rulesetTagsToRemove)) ); } return progress; diff --git a/aws-rds-dbclusterparametergroup/src/main/java/software/amazon/rds/dbclusterparametergroup/BaseHandlerStd.java b/aws-rds-dbclusterparametergroup/src/main/java/software/amazon/rds/dbclusterparametergroup/BaseHandlerStd.java index ab9f3e1d2..276d187f3 100644 --- a/aws-rds-dbclusterparametergroup/src/main/java/software/amazon/rds/dbclusterparametergroup/BaseHandlerStd.java +++ b/aws-rds-dbclusterparametergroup/src/main/java/software/amazon/rds/dbclusterparametergroup/BaseHandlerStd.java @@ -1,6 +1,7 @@ package software.amazon.rds.dbclusterparametergroup; import java.time.Duration; +import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.List; @@ -39,6 +40,7 @@ import software.amazon.awssdk.services.rds.model.Filter; import software.amazon.awssdk.services.rds.model.InvalidDbParameterGroupStateException; import software.amazon.awssdk.services.rds.model.Parameter; +import software.amazon.awssdk.services.rds.model.Tag; import software.amazon.cloudformation.exceptions.CfnInvalidRequestException; import software.amazon.cloudformation.proxy.AmazonWebServicesClientProxy; import software.amazon.cloudformation.proxy.CallChain.Completed; @@ -158,13 +160,19 @@ protected ProgressEvent updateTags( final Tagging.TagSet previousTags, final Tagging.TagSet desiredTags ) { - final Tagging.TagSet tagsToAdd = Tagging.exclude(desiredTags, previousTags); - final Tagging.TagSet tagsToRemove = Tagging.exclude(previousTags, desiredTags); + final Collection effectivePreviousTags = Tagging.translateTagsToSdk(previousTags); + final Collection effectiveDesiredTags = Tagging.translateTagsToSdk(desiredTags); + + final Collection tagsToRemove = Tagging.exclude(effectivePreviousTags, effectiveDesiredTags); + final Collection tagsToAdd = Tagging.exclude(effectiveDesiredTags, effectivePreviousTags); if (tagsToAdd.isEmpty() && tagsToRemove.isEmpty()) { return progress; } + final Tagging.TagSet rulesetTagsToAdd = Tagging.exclude(desiredTags, previousTags); + final Tagging.TagSet rulesetTagsToRemove = Tagging.exclude(previousTags, desiredTags); + try { final String arn = progress.getCallbackContext().getDbClusterParameterGroupArn(); Tagging.removeTags(rdsProxyClient, arn, Tagging.translateTagsToSdk(tagsToRemove)); @@ -175,8 +183,8 @@ protected ProgressEvent updateTags( exception, DEFAULT_DB_CLUSTER_PARAMETER_GROUP_ERROR_RULE_SET.extendWith( Tagging.bestEffortErrorRuleSet( - tagsToAdd, - tagsToRemove, + rulesetTagsToAdd, + rulesetTagsToRemove, Tagging.SOFT_FAIL_IN_PROGRESS_TAGGING_ERROR_RULE_SET, Tagging.HARD_FAIL_TAG_ERROR_RULE_SET ) diff --git a/aws-rds-dbclusterparametergroup/src/test/java/software/amazon/rds/dbclusterparametergroup/AbstractHandlerTest.java b/aws-rds-dbclusterparametergroup/src/test/java/software/amazon/rds/dbclusterparametergroup/AbstractHandlerTest.java index 324090ce5..21431f325 100644 --- a/aws-rds-dbclusterparametergroup/src/test/java/software/amazon/rds/dbclusterparametergroup/AbstractHandlerTest.java +++ b/aws-rds-dbclusterparametergroup/src/test/java/software/amazon/rds/dbclusterparametergroup/AbstractHandlerTest.java @@ -58,6 +58,8 @@ public abstract class AbstractHandlerTest extends AbstractTestBase TAG_SET; + protected static final List TAG_SET_ALTER; + protected static final Parameter PARAM_1, PARAM_2; protected static final String ARN = "arn"; @@ -65,6 +67,8 @@ public abstract class AbstractHandlerTest extends AbstractTestBasebuilder() - .desiredResourceTags(translateTagsToMap(TAG_SET)), () -> DBClusterParameterGroup.builder().dbClusterParameterGroupArn(ARN).build(), () -> RESOURCE_MODEL, - () -> RESOURCE_MODEL, + () -> RESOURCE_MODEL.toBuilder().tags(TAG_SET_ALTER).build(), expectSuccess() ); verify(rdsProxy.client(), times(1)).describeDBClusterParameterGroups(any(DescribeDbClusterParameterGroupsRequest.class)); verify(rdsProxy.client(), times(1)).addTagsToResource(any(AddTagsToResourceRequest.class)); + verify(rdsProxy.client(), times(1)).removeTagsFromResource(any(RemoveTagsFromResourceRequest.class)); + } @Test diff --git a/aws-rds-dbinstance/src/main/java/software/amazon/rds/dbinstance/BaseHandlerStd.java b/aws-rds-dbinstance/src/main/java/software/amazon/rds/dbinstance/BaseHandlerStd.java index d32131069..bc4edaa3a 100644 --- a/aws-rds-dbinstance/src/main/java/software/amazon/rds/dbinstance/BaseHandlerStd.java +++ b/aws-rds-dbinstance/src/main/java/software/amazon/rds/dbinstance/BaseHandlerStd.java @@ -73,6 +73,7 @@ import software.amazon.awssdk.services.rds.model.SnapshotQuotaExceededException; import software.amazon.awssdk.services.rds.model.StorageQuotaExceededException; import software.amazon.awssdk.services.rds.model.StorageTypeNotSupportedException; +import software.amazon.awssdk.services.rds.model.Tag; import software.amazon.awssdk.utils.StringUtils; import software.amazon.cloudformation.exceptions.CfnInvalidRequestException; import software.amazon.cloudformation.exceptions.CfnNotStabilizedException; @@ -1040,13 +1041,20 @@ protected ProgressEvent updateTags( final Tagging.TagSet previousTags, final Tagging.TagSet desiredTags ) { - final Tagging.TagSet tagsToAdd = Tagging.exclude(desiredTags, previousTags); - final Tagging.TagSet tagsToRemove = Tagging.exclude(previousTags, desiredTags); + + final Collection effectivePreviousTags = Tagging.translateTagsToSdk(previousTags); + final Collection effectiveDesiredTags = Tagging.translateTagsToSdk(desiredTags); + + final Collection tagsToRemove = Tagging.exclude(effectivePreviousTags, effectiveDesiredTags); + final Collection tagsToAdd = Tagging.exclude(effectiveDesiredTags, effectivePreviousTags); if (tagsToAdd.isEmpty() && tagsToRemove.isEmpty()) { return progress; } + final Tagging.TagSet rulesetTagsToAdd = Tagging.exclude(desiredTags, previousTags); + final Tagging.TagSet rulesetTagsToRemove = Tagging.exclude(previousTags, desiredTags); + DBInstance dbInstance; try { dbInstance = fetchDBInstance(rdsProxyClient, progress.getResourceModel()); @@ -1063,7 +1071,7 @@ protected ProgressEvent updateTags( return Commons.handleException( progress, exception, - DEFAULT_DB_INSTANCE_ERROR_RULE_SET.extendWith(Tagging.bestEffortErrorRuleSet(tagsToAdd, tagsToRemove)) + DEFAULT_DB_INSTANCE_ERROR_RULE_SET.extendWith(Tagging.bestEffortErrorRuleSet(rulesetTagsToAdd, rulesetTagsToRemove)) ); } diff --git a/aws-rds-dbinstance/src/test/java/software/amazon/rds/dbinstance/UpdateHandlerTest.java b/aws-rds-dbinstance/src/test/java/software/amazon/rds/dbinstance/UpdateHandlerTest.java index 53ed1845c..18db2b06d 100644 --- a/aws-rds-dbinstance/src/test/java/software/amazon/rds/dbinstance/UpdateHandlerTest.java +++ b/aws-rds-dbinstance/src/test/java/software/amazon/rds/dbinstance/UpdateHandlerTest.java @@ -13,7 +13,10 @@ import java.time.Duration; import java.time.Instant; +import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; +import java.util.List; import java.util.Queue; import java.util.concurrent.ConcurrentLinkedQueue; import java.util.function.Supplier; @@ -295,14 +298,14 @@ public void handleRequest_SuccessTagsAddOnly() { context.setUpdatedRoles(true); context.setStorageAllocated(true); + List updatedTags = new ArrayList<>(TAG_LIST); + updatedTags.add(Tag.builder().key("updated").value("tag").build()); + test_handleRequest_base( context, - ResourceHandlerRequest.builder() - .previousResourceTags(Collections.emptyMap()) - .desiredResourceTags(Translator.translateTagsToRequest(TAG_LIST)), () -> DB_INSTANCE_ACTIVE, () -> RESOURCE_MODEL_BLDR().build(), - () -> RESOURCE_MODEL_BLDR().build(), + () -> RESOURCE_MODEL_BLDR().tags(updatedTags).build(), expectSuccess() ); @@ -323,12 +326,9 @@ public void handleRequest_SuccessTagsRemoveOnly() { test_handleRequest_base( context, - ResourceHandlerRequest.builder() - .previousResourceTags(Translator.translateTagsToRequest(TAG_LIST)) - .desiredResourceTags(Translator.translateTagsToRequest(TAG_LIST_EMPTY)), () -> DB_INSTANCE_ACTIVE, () -> RESOURCE_MODEL_BLDR().build(), - () -> RESOURCE_MODEL_BLDR().build(), + () -> RESOURCE_MODEL_BLDR().tags(Collections.emptyList()).build(), expectSuccess() ); diff --git a/aws-rds-dbparametergroup/src/main/java/software/amazon/rds/dbparametergroup/BaseHandlerStd.java b/aws-rds-dbparametergroup/src/main/java/software/amazon/rds/dbparametergroup/BaseHandlerStd.java index ed75cf6a9..baebe5b8a 100644 --- a/aws-rds-dbparametergroup/src/main/java/software/amazon/rds/dbparametergroup/BaseHandlerStd.java +++ b/aws-rds-dbparametergroup/src/main/java/software/amazon/rds/dbparametergroup/BaseHandlerStd.java @@ -1,6 +1,7 @@ package software.amazon.rds.dbparametergroup; import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; @@ -30,6 +31,7 @@ import software.amazon.awssdk.services.rds.model.Filter; import software.amazon.awssdk.services.rds.model.InvalidDbParameterGroupStateException; import software.amazon.awssdk.services.rds.model.Parameter; +import software.amazon.awssdk.services.rds.model.Tag; import software.amazon.cloudformation.proxy.AmazonWebServicesClientProxy; import software.amazon.cloudformation.proxy.HandlerErrorCode; import software.amazon.cloudformation.proxy.Logger; @@ -133,13 +135,19 @@ protected ProgressEvent updateTags( final Tagging.TagSet desiredTags, final RequestLogger logger ) { - final Tagging.TagSet tagsToAdd = Tagging.exclude(desiredTags, previousTags); - final Tagging.TagSet tagsToRemove = Tagging.exclude(previousTags, desiredTags); + final Collection effectivePreviousTags = Tagging.translateTagsToSdk(previousTags); + final Collection effectiveDesiredTags = Tagging.translateTagsToSdk(desiredTags); + + final Collection tagsToRemove = Tagging.exclude(effectivePreviousTags, effectiveDesiredTags); + final Collection tagsToAdd = Tagging.exclude(effectiveDesiredTags, effectivePreviousTags); if (tagsToAdd.isEmpty() && tagsToRemove.isEmpty()) { return progress; } + final Tagging.TagSet rulesetTagsToAdd = Tagging.exclude(desiredTags, previousTags); + final Tagging.TagSet rulesetTagsToRemove = Tagging.exclude(previousTags, desiredTags); + try { final String arn = progress.getCallbackContext().getDbParameterGroupArn(); Tagging.removeTags(rdsProxyClient, arn, Tagging.translateTagsToSdk(tagsToRemove)); @@ -150,8 +158,8 @@ protected ProgressEvent updateTags( exception, DEFAULT_DB_PARAMETER_GROUP_ERROR_RULE_SET.extendWith( Tagging.bestEffortErrorRuleSet( - tagsToAdd, - tagsToRemove, + rulesetTagsToAdd, + rulesetTagsToRemove, Tagging.SOFT_FAIL_IN_PROGRESS_TAGGING_ERROR_RULE_SET, Tagging.HARD_FAIL_TAG_ERROR_RULE_SET ) diff --git a/aws-rds-dbsubnetgroup/src/main/java/software/amazon/rds/dbsubnetgroup/BaseHandlerStd.java b/aws-rds-dbsubnetgroup/src/main/java/software/amazon/rds/dbsubnetgroup/BaseHandlerStd.java index a3921afb7..62455c141 100644 --- a/aws-rds-dbsubnetgroup/src/main/java/software/amazon/rds/dbsubnetgroup/BaseHandlerStd.java +++ b/aws-rds-dbsubnetgroup/src/main/java/software/amazon/rds/dbsubnetgroup/BaseHandlerStd.java @@ -7,6 +7,7 @@ import software.amazon.awssdk.services.rds.model.DbSubnetGroupQuotaExceededException; import software.amazon.awssdk.services.rds.model.InvalidDbSubnetGroupStateException; import software.amazon.awssdk.services.rds.model.InvalidSubnetException; +import software.amazon.awssdk.services.rds.model.Tag; import software.amazon.cloudformation.proxy.AmazonWebServicesClientProxy; import software.amazon.cloudformation.proxy.HandlerErrorCode; import software.amazon.cloudformation.proxy.Logger; @@ -22,6 +23,8 @@ import software.amazon.rds.common.logging.RequestLogger; import software.amazon.rds.common.printer.FilteredJsonPrinter; +import java.util.Collection; + public abstract class BaseHandlerStd extends BaseHandler { protected static final int DB_SUBNET_GROUP_NAME_LENGTH = 255; protected static final String DB_SUBNET_GROUP_STATUS_COMPLETE = "Complete"; @@ -105,8 +108,18 @@ protected ProgressEvent updateTags( final Tagging.TagSet previousTags, final Tagging.TagSet desiredTags ) { - final Tagging.TagSet tagsToAdd = Tagging.exclude(desiredTags, previousTags); - final Tagging.TagSet tagsToRemove = Tagging.exclude(previousTags, desiredTags); + final Collection effectivePreviousTags = Tagging.translateTagsToSdk(previousTags); + final Collection effectiveDesiredTags = Tagging.translateTagsToSdk(desiredTags); + + final Collection tagsToRemove = Tagging.exclude(effectivePreviousTags, effectiveDesiredTags); + final Collection tagsToAdd = Tagging.exclude(effectiveDesiredTags, effectivePreviousTags); + + if (tagsToAdd.isEmpty() && tagsToRemove.isEmpty()) { + return progress; + } + + final Tagging.TagSet rulesetTagsToAdd = Tagging.exclude(desiredTags, previousTags); + final Tagging.TagSet rulesetTagsToRemove = Tagging.exclude(previousTags, desiredTags); if (tagsToAdd.isEmpty() && tagsToRemove.isEmpty()) { return progress; @@ -122,8 +135,8 @@ protected ProgressEvent updateTags( exception, DEFAULT_DB_SUBNET_GROUP_ERROR_RULE_SET.extendWith( Tagging.bestEffortErrorRuleSet( - tagsToAdd, - tagsToRemove, + rulesetTagsToAdd, + rulesetTagsToRemove, Tagging.SOFT_FAIL_IN_PROGRESS_TAGGING_ERROR_RULE_SET, Tagging.HARD_FAIL_TAG_ERROR_RULE_SET ) @@ -133,6 +146,4 @@ protected ProgressEvent updateTags( return progress; } - - } diff --git a/aws-rds-eventsubscription/src/main/java/software/amazon/rds/eventsubscription/BaseHandlerStd.java b/aws-rds-eventsubscription/src/main/java/software/amazon/rds/eventsubscription/BaseHandlerStd.java index 7ba768376..74810644a 100644 --- a/aws-rds-eventsubscription/src/main/java/software/amazon/rds/eventsubscription/BaseHandlerStd.java +++ b/aws-rds-eventsubscription/src/main/java/software/amazon/rds/eventsubscription/BaseHandlerStd.java @@ -1,5 +1,6 @@ package software.amazon.rds.eventsubscription; +import java.util.Collection; import java.util.function.BiFunction; import java.util.function.Function; @@ -10,6 +11,7 @@ import software.amazon.awssdk.services.rds.model.SourceNotFoundException; import software.amazon.awssdk.services.rds.model.SubscriptionAlreadyExistException; import software.amazon.awssdk.services.rds.model.SubscriptionNotFoundException; +import software.amazon.awssdk.services.rds.model.Tag; import software.amazon.cloudformation.proxy.AmazonWebServicesClientProxy; import software.amazon.cloudformation.proxy.HandlerErrorCode; import software.amazon.cloudformation.proxy.Logger; @@ -111,13 +113,19 @@ protected ProgressEvent updateTags( final ProgressEvent progress, final Tagging.TagSet previousTags, final Tagging.TagSet desiredTags) { - final Tagging.TagSet tagsToAdd = Tagging.exclude(desiredTags, previousTags); - final Tagging.TagSet tagsToRemove = Tagging.exclude(previousTags, desiredTags); + final Collection effectivePreviousTags = Tagging.translateTagsToSdk(previousTags); + final Collection effectiveDesiredTags = Tagging.translateTagsToSdk(desiredTags); + + final Collection tagsToRemove = Tagging.exclude(effectivePreviousTags, effectiveDesiredTags); + final Collection tagsToAdd = Tagging.exclude(effectiveDesiredTags, effectivePreviousTags); if (tagsToAdd.isEmpty() && tagsToRemove.isEmpty()) { return progress; } + final Tagging.TagSet rulesetTagsToAdd = Tagging.exclude(desiredTags, previousTags); + final Tagging.TagSet rulesetTagsToRemove = Tagging.exclude(previousTags, desiredTags); + String arn = progress.getCallbackContext().getEventSubscriptionArn(); if (arn == null) { ProgressEvent progressEvent = fetchEventSubscriptionArn(proxy, rdsProxyClient, progress); @@ -136,8 +144,8 @@ protected ProgressEvent updateTags( exception, DEFAULT_EVENT_SUBSCRIPTION_ERROR_RULE_SET.extendWith( Tagging.bestEffortErrorRuleSet( - tagsToAdd, - tagsToRemove, + rulesetTagsToAdd, + rulesetTagsToRemove, Tagging.SOFT_FAIL_IN_PROGRESS_TAGGING_ERROR_RULE_SET, Tagging.HARD_FAIL_TAG_ERROR_RULE_SET ) diff --git a/aws-rds-optiongroup/src/main/java/software/amazon/rds/optiongroup/BaseHandlerStd.java b/aws-rds-optiongroup/src/main/java/software/amazon/rds/optiongroup/BaseHandlerStd.java index d25b0179d..eb25bdfa0 100644 --- a/aws-rds-optiongroup/src/main/java/software/amazon/rds/optiongroup/BaseHandlerStd.java +++ b/aws-rds-optiongroup/src/main/java/software/amazon/rds/optiongroup/BaseHandlerStd.java @@ -1,13 +1,13 @@ package software.amazon.rds.optiongroup; import java.time.Duration; -import java.util.List; +import java.util.Collection; import software.amazon.awssdk.services.rds.RdsClient; -import software.amazon.awssdk.services.rds.model.ListTagsForResourceResponse; import software.amazon.awssdk.services.rds.model.OptionGroupAlreadyExistsException; import software.amazon.awssdk.services.rds.model.OptionGroupNotFoundException; import software.amazon.awssdk.services.rds.model.OptionGroupQuotaExceededException; +import software.amazon.awssdk.services.rds.model.Tag; import software.amazon.cloudformation.proxy.AmazonWebServicesClientProxy; import software.amazon.cloudformation.proxy.HandlerErrorCode; import software.amazon.cloudformation.proxy.Logger; @@ -119,13 +119,20 @@ protected ProgressEvent updateTags( DEFAULT_OPTION_GROUP_ERROR_RULE_SET )) .done((describeRequest, describeResponse, invocation, resourceModel, ctx) -> { - final Tagging.TagSet tagsToAdd = Tagging.exclude(desiredTags, previousTags); - final Tagging.TagSet tagsToRemove = Tagging.exclude(previousTags, desiredTags); + final Collection effectivePreviousTags = Tagging.translateTagsToSdk(previousTags); + final Collection effectiveDesiredTags = Tagging.translateTagsToSdk(desiredTags); - if (tagsToRemove.isEmpty() && tagsToAdd.isEmpty()) { + final Collection tagsToRemove = Tagging.exclude(effectivePreviousTags, effectiveDesiredTags); + final Collection tagsToAdd = Tagging.exclude(effectiveDesiredTags, effectivePreviousTags); + + if (tagsToAdd.isEmpty() && tagsToRemove.isEmpty()) { return progress; } + final Tagging.TagSet rulesetTagsToAdd = Tagging.exclude(desiredTags, previousTags); + final Tagging.TagSet rulesetTagsToRemove = Tagging.exclude(previousTags, desiredTags); + + final String arn = describeResponse.optionGroupsList().stream().findFirst().get().optionGroupArn(); try { Tagging.removeTags(proxyClient, arn, Tagging.translateTagsToSdk(tagsToRemove)); @@ -136,8 +143,8 @@ protected ProgressEvent updateTags( exception, DEFAULT_OPTION_GROUP_ERROR_RULE_SET.extendWith( Tagging.bestEffortErrorRuleSet( - tagsToAdd, - tagsToRemove, + rulesetTagsToAdd, + rulesetTagsToRemove, Tagging.SOFT_FAIL_IN_PROGRESS_TAGGING_ERROR_RULE_SET, Tagging.HARD_FAIL_TAG_ERROR_RULE_SET ) @@ -147,12 +154,4 @@ protected ProgressEvent updateTags( return ProgressEvent.progress(resourceModel, ctx); }); } - - protected List listTags(final ProxyClient proxyClient, final String arn) { - final ListTagsForResourceResponse listTagsForResourceResponse = proxyClient.injectCredentialsAndInvokeV2( - Translator.listTagsForResourceRequest(arn), - proxyClient.client()::listTagsForResource - ); - return Translator.translateTagsFromSdk(listTagsForResourceResponse.tagList()); - } } From dbe93f15c71d3aacc035ae7ff8727fb95f83d9f7 Mon Sep 17 00:00:00 2001 From: Valentin Shirshov Date: Mon, 9 Oct 2023 14:00:24 +0200 Subject: [PATCH 2/3] Remove unused import --- .../main/java/software/amazon/rds/common/handler/Tagging.java | 1 - 1 file changed, 1 deletion(-) diff --git a/aws-rds-cfn-common/src/main/java/software/amazon/rds/common/handler/Tagging.java b/aws-rds-cfn-common/src/main/java/software/amazon/rds/common/handler/Tagging.java index 451eb66ee..1aa97e351 100644 --- a/aws-rds-cfn-common/src/main/java/software/amazon/rds/common/handler/Tagging.java +++ b/aws-rds-cfn-common/src/main/java/software/amazon/rds/common/handler/Tagging.java @@ -11,7 +11,6 @@ import com.amazonaws.util.CollectionUtils; import com.google.common.collect.Sets; -import javafx.util.Pair; import lombok.AllArgsConstructor; import lombok.Builder; import lombok.Data; From cc991908168f127eb1bd9e8ff2d7db213bd2086a Mon Sep 17 00:00:00 2001 From: Valentin Shirshov Date: Mon, 9 Oct 2023 17:32:30 +0200 Subject: [PATCH 3/3] Add more tests --- .../rds/dbinstance/UpdateHandlerTest.java | 92 +++++++++++++++++++ 1 file changed, 92 insertions(+) diff --git a/aws-rds-dbinstance/src/test/java/software/amazon/rds/dbinstance/UpdateHandlerTest.java b/aws-rds-dbinstance/src/test/java/software/amazon/rds/dbinstance/UpdateHandlerTest.java index 18db2b06d..65319a8da 100644 --- a/aws-rds-dbinstance/src/test/java/software/amazon/rds/dbinstance/UpdateHandlerTest.java +++ b/aws-rds-dbinstance/src/test/java/software/amazon/rds/dbinstance/UpdateHandlerTest.java @@ -16,6 +16,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Collections; +import java.util.HashSet; import java.util.List; import java.util.Queue; import java.util.concurrent.ConcurrentLinkedQueue; @@ -313,6 +314,41 @@ public void handleRequest_SuccessTagsAddOnly() { verify(rdsProxy.client(), times(2)).describeDBInstances(any(DescribeDbInstancesRequest.class)); } + @Test + public void handleRequest_ResourceTagsPrioritizedOverStackTags() { + final AddTagsToResourceResponse addTagsToResourceResponse = AddTagsToResourceResponse.builder().build(); + when(rdsProxy.client().addTagsToResource(any(AddTagsToResourceRequest.class))).thenReturn(addTagsToResourceResponse); + + final CallbackContext context = new CallbackContext(); + context.setUpdated(true); + context.setRebooted(true); + context.setUpdatedRoles(true); + context.setStorageAllocated(true); + + List updatedTags = new ArrayList<>(TAG_LIST); + updatedTags.add(Tag.builder().key("tag-key").value("resource-level").build()); + + test_handleRequest_base( + context, + ResourceHandlerRequest.builder() + .previousResourceTags(Collections.emptyMap()) + .desiredResourceTags(Translator.translateTagsToRequest(Collections.singleton( + Tag.builder().key("tag-key").value("stack-level").build() + ))), + () -> DB_INSTANCE_ACTIVE, + () -> RESOURCE_MODEL_BLDR().build(), + () -> RESOURCE_MODEL_BLDR().tags(updatedTags).build(), + expectSuccess() + ); + + final ArgumentCaptor captor = ArgumentCaptor.forClass(AddTagsToResourceRequest.class); + verify(rdsProxy.client()).addTagsToResource(captor.capture()); + Assertions.assertThat(captor.getValue().tags()) + .containsExactlyInAnyOrder( + software.amazon.awssdk.services.rds.model.Tag .builder().key("tag-key").value("resource-level").build()); + verify(rdsProxy.client(), times(2)).describeDBInstances(any(DescribeDbInstancesRequest.class)); + } + @Test public void handleRequest_SuccessTagsRemoveOnly() { final RemoveTagsFromResourceResponse removeTagsFromResourceResponse = RemoveTagsFromResourceResponse.builder().build(); @@ -336,6 +372,62 @@ public void handleRequest_SuccessTagsRemoveOnly() { verify(rdsProxy.client(), times(2)).describeDBInstances(any(DescribeDbInstancesRequest.class)); } + @Test + public void handleRequest_StackLevelTagsRemovalDoesNotRemoveTagsIfTheyExistOnResourceLevel() { + final CallbackContext context = new CallbackContext(); + context.setUpdated(true); + context.setRebooted(true); + context.setUpdatedRoles(true); + context.setStorageAllocated(true); + + test_handleRequest_base( + context, + ResourceHandlerRequest.builder() + .previousResourceTags(Translator.translateTagsToRequest(TAG_LIST)) + .desiredResourceTags(Collections.emptyMap()), + () -> DB_INSTANCE_ACTIVE, + () -> RESOURCE_MODEL_BLDR().build(), + () -> RESOURCE_MODEL_BLDR().build(), + expectSuccess() + ); + + verify(rdsProxy.client(), times(1)).describeDBInstances(any(DescribeDbInstancesRequest.class)); + } + + @Test + public void handleRequest_FallBackToStackLevelTagsIfResourceLevelWasRemoved() { + final AddTagsToResourceResponse addTagsToResourceResponse = AddTagsToResourceResponse.builder().build(); + when(rdsProxy.client().addTagsToResource(any(AddTagsToResourceRequest.class))).thenReturn(addTagsToResourceResponse); + + final CallbackContext context = new CallbackContext(); + context.setUpdated(true); + context.setRebooted(true); + context.setUpdatedRoles(true); + context.setStorageAllocated(true); + + test_handleRequest_base( + context, + ResourceHandlerRequest.builder() + .previousResourceTags(Translator.translateTagsToRequest(Collections.singleton( + Tag.builder().key("tag-key").value("stack-level").build()))) + .desiredResourceTags(Translator.translateTagsToRequest(Collections.singleton( + Tag.builder().key("tag-key").value("stack-level").build()))), + () -> DB_INSTANCE_ACTIVE, + () -> RESOURCE_MODEL_BLDR().tags(Collections.singletonList(Tag.builder().key("tag-key").value("resource-level").build())).build(), + () -> RESOURCE_MODEL_BLDR().tags(Collections.emptyList()).build(), + expectSuccess() + ); + + final ArgumentCaptor captor = ArgumentCaptor.forClass(AddTagsToResourceRequest.class); + verify(rdsProxy.client()).addTagsToResource(captor.capture()); + Assertions.assertThat(captor.getValue().tags()) + .containsExactlyInAnyOrder( + software.amazon.awssdk.services.rds.model.Tag .builder().key("tag-key").value("stack-level").build()); + verify(rdsProxy.client()).removeTagsFromResource(any(RemoveTagsFromResourceRequest.class)); + + verify(rdsProxy.client(), times(2)).describeDBInstances(any(DescribeDbInstancesRequest.class)); + } + @Test public void handleRequest_DeleteNonExistingRole() { // compute a complete sequence of transitions from the initial set of roles to the final one.