[All] Fix tagging when stack and resource level tags have jey collisi…

…ons (aws-cloudformation#470) * [All] Make sure that stack level tags do not override resource level tags --------- Co-authored-by: Valentin Shirshov <[email protected]>
dbbh · Nov 20, 2023 · ea5e22b · ea5e22b
1 parent 6ec87f8
commit ea5e22b
Show file tree

Hide file tree

Showing 14 changed files with 224 additions and 60 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
@@ -68,13 +68,23 @@ public static TagSet exclude(final TagSet from, final TagSet what) {
                 .build();
     }
 
-    public static <K, V> Map<K, V> mergeTags(Map<K, V> tagsMap1, Map<K, V> tagsMap2) {
-        final Map<K, V> result = new LinkedHashMap<>();
-        result.putAll(Optional.ofNullable(tagsMap1).orElse(Collections.emptySortedMap()));
-        result.putAll(Optional.ofNullable(tagsMap2).orElse(Collections.emptySortedMap()));
+    public static Collection<Tag> exclude(final Collection<Tag> from, final Collection<Tag> what) {
+        final Set<Tag> result = new LinkedHashSet<>(from);
+        result.removeAll(what);
         return result;
     }
 
+
+    public static Set<Tag> translateTagsToSdk(final Collection<Tag> 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<Tag> translateTagsToSdk(final TagSet tagSet) {
         //For backward compatibility, We will resolve duplicates tags between stack level tags and resource tags.
         final Map<String, Tag> 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
@@ -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<ResourceModel, CallbackContext> 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<Tag> effectivePreviousTags = Tagging.translateTagsToSdk(previousTags);
+        final Collection<Tag> effectiveDesiredTags = Tagging.translateTagsToSdk(desiredTags);
+
+        final Collection<Tag> tagsToRemove = Tagging.exclude(effectivePreviousTags, effectiveDesiredTags);
+        final Collection<Tag> 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<ResourceModel, CallbackContext> 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
@@ -48,9 +48,9 @@ _Required_: Yes
 
 _Type_: String
 
-_Minimum_: <code>1</code>
+_Minimum Length_: <code>1</code>
 
-_Maximum_: <code>63</code>
+_Maximum Length_: <code>63</code>
 
 _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_: <code>1</code>
+_Minimum Length_: <code>1</code>
 
-_Maximum_: <code>63</code>
+_Maximum Length_: <code>63</code>
 
 _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
@@ -32,9 +32,9 @@ _Required_: Yes
 
 _Type_: String
 
-_Minimum_: <code>1</code>
+_Minimum Length_: <code>1</code>
 
-_Maximum_: <code>128</code>
+_Maximum Length_: <code>128</code>
 
 _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_: <code>256</code>
+_Maximum Length_: <code>256</code>
 
 _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/...dbclusterendpoint/src/main/java/software/amazon/rds/dbclusterendpoint/BaseHandlerStd.java b/...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,21 +99,27 @@ protected ProgressEvent<ResourceModel, CallbackContext> 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<Tag> effectivePreviousTags = Tagging.translateTagsToSdk(previousTags);
+        final Collection<Tag> effectiveDesiredTags = Tagging.translateTagsToSdk(desiredTags);
+
+        final Collection<Tag> tagsToRemove = Tagging.exclude(effectivePreviousTags, effectiveDesiredTags);
+        final Collection<Tag> 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));
         } catch (Exception exception) {
             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/...ametergroup/src/main/java/software/amazon/rds/dbclusterparametergroup/BaseHandlerStd.java b/...ametergroup/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<ResourceModel, CallbackContext> 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<Tag> effectivePreviousTags = Tagging.translateTagsToSdk(previousTags);
+        final Collection<Tag> effectiveDesiredTags = Tagging.translateTagsToSdk(desiredTags);
+
+        final Collection<Tag> tagsToRemove = Tagging.exclude(effectivePreviousTags, effectiveDesiredTags);
+        final Collection<Tag> 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<ResourceModel, CallbackContext> 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/...rgroup/src/test/java/software/amazon/rds/dbclusterparametergroup/AbstractHandlerTest.java b/...rgroup/src/test/java/software/amazon/rds/dbclusterparametergroup/AbstractHandlerTest.java
@@ -58,13 +58,17 @@ public abstract class AbstractHandlerTest extends AbstractTestBase<DBClusterPara
 
     protected static final DBClusterParameterGroup DB_CLUSTER_PARAMETER_GROUP;
     protected static final List<Tag> TAG_SET;
+    protected static final List<Tag> TAG_SET_ALTER;
+
     protected static final Parameter PARAM_1, PARAM_2;
 
     protected static final String ARN = "arn";
     protected static final String DESCRIPTION = "sample description";
     protected static final String FAMILY = "default.aurora.5";
     protected static final String TAG_KEY = "key";
     protected static final String TAG_VALUE = "value";
+    protected static final String TAG_VALUE_ALTER = "value-alter";
+
     protected static final String UPDATED_DESCRIPTION = "updated description";
 
     static {
@@ -108,6 +112,7 @@ public abstract class AbstractHandlerTest extends AbstractTestBase<DBClusterPara
                 .build();
 
         TAG_SET = Lists.newArrayList(Tag.builder().key(TAG_KEY).value(TAG_VALUE).build());
+        TAG_SET_ALTER = Lists.newArrayList(Tag.builder().key(TAG_KEY).value(TAG_VALUE_ALTER).build());
 
         DB_CLUSTER_PARAMETER_GROUP = DBClusterParameterGroup.builder()
                 .dbClusterParameterGroupArn(ARN)

diff --git a/...tergroup/src/test/java/software/amazon/rds/dbclusterparametergroup/UpdateHandlerTest.java b/...tergroup/src/test/java/software/amazon/rds/dbclusterparametergroup/UpdateHandlerTest.java
@@ -201,16 +201,16 @@ public void handleRequest_NoParametersChanged() {
 
         test_handleRequest_base(
                 callbackContext,
-                ResourceHandlerRequest.<ResourceModel>builder()
-                        .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
@@ -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<ResourceModel, CallbackContext> 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<Tag> effectivePreviousTags = Tagging.translateTagsToSdk(previousTags);
+        final Collection<Tag> effectiveDesiredTags = Tagging.translateTagsToSdk(desiredTags);
+
+        final Collection<Tag> tagsToRemove = Tagging.exclude(effectivePreviousTags, effectiveDesiredTags);
+        final Collection<Tag> 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<ResourceModel, CallbackContext> 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))
             );
         }