Fixing the tagging logic for namespace and workgroup continued by merging all stack, resource and system tags together. (#78)

* Fixing the tagging logic for namespace and workgroup continued by merging all stack, resource and system tags together.

* Updating List to Set implementation for tags method in namespace and workgroup

* Fixing the remaining tagging test_create_with_tag_denied CV2 test for workgroup

Author: adityatomer

aws-redshiftserverless-namespace/src/main/java/software/amazon/redshiftserverless/namespace/CreateHandler.java

@@ -24,6 +24,8 @@
 
 import java.util.Collections;
 import java.util.Optional;
+import java.util.List;
+import java.util.Set;
 
 
 public class CreateHandler extends BaseHandlerStd {
@@ -40,15 +42,22 @@ protected ProgressEvent<ResourceModel, CallbackContext> handleRequest(
 
         return ProgressEvent.progress(request.getDesiredResourceState(), callbackContext)
             .then(progress -> {
+                Set<Tag>mergedTags = TagHelper.convertToTagSet(
+                        TagHelper.mergeTags(
+                                request,
+                                TagHelper.convertToMap(request.getDesiredResourceState().getTags()),
+                                request.getSystemTags(),
+                                request.getDesiredResourceTags()
+                        ));
                 return proxy.initiate("AWS-RedshiftServerless-Namespace::Create", proxyClient, progress.getResourceModel(), callbackContext)
-                    .translateToServiceRequest(Translator::translateToCreateRequest)
-                    .makeServiceCall(this::createNamespace)
-                    .stabilize(this::isNamespaceActive)
-                    .handleError(this::defaultErrorHandler)
-                    .done((_request, _response, _client, _model, _context) -> {
-                        callbackContext.setNamespaceArn(_response.namespace().namespaceArn());
-                        return ProgressEvent.progress(_model, callbackContext);
-                    });
+                        .translateToServiceRequest((model) -> Translator.translateToCreateRequest(model, mergedTags))
+                        .makeServiceCall(this::createNamespace)
+                        .stabilize(this::isNamespaceActive)
+                        .handleError(this::defaultErrorHandler)
+                        .done((_request, _response, _client, _model, _context) -> {
+                            callbackContext.setNamespaceArn(_response.namespace().namespaceArn());
+                            return ProgressEvent.progress(_model, callbackContext);
+                        });
             })
             .then(progress -> {
                 if (progress.getResourceModel().getNamespaceResourcePolicy() != null) {

aws-redshiftserverless-namespace/src/main/java/software/amazon/redshiftserverless/namespace/TagHelper.java

@@ -16,7 +16,6 @@
 import software.amazon.awssdk.awscore.AwsResponse;
 import software.amazon.awssdk.core.SdkClient;
 // TODO: Critical! Please replace the CloudFormation Tag model below with your service's own SDK Tag model
-import software.amazon.awssdk.services.cloudformation.model.Tag;
 import software.amazon.cloudformation.proxy.AmazonWebServicesClientProxy;
 import software.amazon.cloudformation.proxy.Logger;
 import software.amazon.cloudformation.proxy.ProgressEvent;
@@ -40,11 +39,11 @@ public static Map<String, String> convertToMap(final Collection<Tag> tags) {
             return Collections.emptyMap();
         }
         return tags.stream()
-            .filter(tag -> tag.value() != null)
-            .collect(Collectors.toMap(
-                Tag::key,
-                Tag::value,
-                (oldValue, newValue) -> newValue));
+                .filter(tag -> tag.getValue() != null)
+                .collect(Collectors.toMap(
+                        Tag::getKey,
+                        Tag::getValue,
+                        (oldValue, newValue) -> newValue));
     }
 
     /**
@@ -57,17 +56,17 @@ public static Map<String, String> convertToMap(final Collection<Tag> tags) {
      * @param tagMap Map of tags to convert
      * @return Set of Tag objects
      */
-    public static Set<Tag> convertToSet(final Map<String, String> tagMap) {
+    public static Set<Tag> convertToTagSet(final Map<String, String> tagMap) {
         if (MapUtils.isEmpty(tagMap)) {
             return Collections.emptySet();
         }
         return tagMap.entrySet().stream()
-            .filter(tag -> tag.getValue() != null)
-            .map(tag -> Tag.builder()
-                .key(tag.getKey())
-                .value(tag.getValue())
-                .build())
-            .collect(Collectors.toSet());
+                .filter(tag -> tag.getValue() != null)
+                .map(tag -> Tag.builder()
+                        .key(tag.getKey())
+                        .value(tag.getValue())
+                        .build())
+                .collect(Collectors.toSet());
     }
 
     /**
@@ -96,23 +95,45 @@ public final Map<String, String> generateTagsForCreate(final ResourceModel resou
      *
      * Determines whether user defined tags have been changed during update.
      */
-    public final boolean shouldUpdateTags(final ResourceModel resourceModel, final ResourceHandlerRequest<ResourceModel> handlerRequest) {
+    public static boolean shouldUpdateTags(final ResourceHandlerRequest<ResourceModel> handlerRequest) {
         final Map<String, String> previousTags = getPreviouslyAttachedTags(handlerRequest);
-        final Map<String, String> desiredTags = getNewDesiredTags(resourceModel, handlerRequest);
+        final Map<String, String> desiredTags = getNewDesiredTags(handlerRequest);
         return ObjectUtils.notEqual(previousTags, desiredTags);
     }
 
+    /**
+     * If stack tags and resource tags are not merged together in Configuration class,
+     * we will get new user defined tags from both resource model and previous stack tags.
+     */
+    public static Map<String, String> mergeTags(
+            ResourceHandlerRequest<ResourceModel> request,
+            Map<String, String> resourceTags,
+            Map<String, String> systemTags,
+            Map<String, String> stackTags) {
+        final Map<String, String> tagMap = new HashMap<>();
+        if (resourceTags != null) {
+            tagMap.putAll(resourceTags);
+        }
+        if (systemTags != null) {
+            tagMap.putAll(systemTags);
+        }
+        if (stackTags != null) {
+            tagMap.putAll(stackTags);
+        }
+        return tagMap;
+    }
+
     /**
      * getPreviouslyAttachedTags
      *
      * If stack tags and resource tags are not merged together in Configuration class,
      * we will get previous attached user defined tags from both handlerRequest.getPreviousResourceTags (stack tags)
      * and handlerRequest.getPreviousResourceState (resource tags).
      */
-    public Map<String, String> getPreviouslyAttachedTags(final ResourceHandlerRequest<ResourceModel> handlerRequest) {
+    public static Map<String, String> getPreviouslyAttachedTags(final ResourceHandlerRequest<ResourceModel> handlerRequest) {
         // get previous stack level tags from handlerRequest
         final Map<String, String> previousTags = handlerRequest.getPreviousResourceTags() != null ?
-            handlerRequest.getPreviousResourceTags() : Collections.emptyMap();
+                handlerRequest.getPreviousResourceTags() : Collections.emptyMap();
 
         // TODO: get resource level tags from previous resource state based on your tag property name
         // TODO: previousTags.putAll(handlerRequest.getPreviousResourceState().getTags());
@@ -125,10 +146,10 @@ public Map<String, String> getPreviouslyAttachedTags(final ResourceHandlerReques
      * If stack tags and resource tags are not merged together in Configuration class,
      * we will get new user defined tags from both resource model and previous stack tags.
      */
-    public Map<String, String> getNewDesiredTags(final ResourceModel resourceModel, final ResourceHandlerRequest<ResourceModel> handlerRequest) {
+    public static Map<String, String> getNewDesiredTags(final ResourceHandlerRequest<ResourceModel> handlerRequest) {
         // get new stack level tags from handlerRequest
         final Map<String, String> desiredTags = handlerRequest.getDesiredResourceTags() != null ?
-            handlerRequest.getDesiredResourceTags() : Collections.emptyMap();
+                handlerRequest.getDesiredResourceTags() : Collections.emptyMap();
 
         // TODO: get resource level tags from resource model based on your tag property name
         // TODO: desiredTags.putAll(convertToMap(resourceModel.getTags()));
@@ -140,25 +161,25 @@ public Map<String, String> getNewDesiredTags(final ResourceModel resourceModel,
      *
      * Determines the tags the customer desired to define or redefine.
      */
-    public Map<String, String> generateTagsToAdd(final Map<String, String> previousTags, final Map<String, String> desiredTags) {
+    public static Map<String, String> generateTagsToAdd(final Map<String, String> previousTags, final Map<String, String> desiredTags) {
         return desiredTags.entrySet().stream()
-            .filter(e -> !previousTags.containsKey(e.getKey()) || !Objects.equals(previousTags.get(e.getKey()), e.getValue()))
-            .collect(Collectors.toMap(
-                Map.Entry::getKey,
-                Map.Entry::getValue));
+                .filter(e -> !previousTags.containsKey(e.getKey()) || !Objects.equals(previousTags.get(e.getKey()), e.getValue()))
+                .collect(Collectors.toMap(
+                        Map.Entry::getKey,
+                        Map.Entry::getValue));
     }
 
     /**
      * getTagsToRemove
      *
      * Determines the tags the customer desired to remove from the function.
      */
-    public Set<String> generateTagsToRemove(final Map<String, String> previousTags, final Map<String, String> desiredTags) {
+    public static Set<String> generateTagsToRemove(final Map<String, String> previousTags, final Map<String, String> desiredTags) {
         final Set<String> desiredTagNames = desiredTags.keySet();
 
         return previousTags.keySet().stream()
-            .filter(tagName -> !desiredTagNames.contains(tagName))
-            .collect(Collectors.toSet());
+                .filter(tagName -> !desiredTagNames.contains(tagName))
+                .collect(Collectors.toSet());
     }
 
     /**
@@ -178,54 +199,5 @@ public Set<Tag> generateTagsToAdd(final Set<Tag> previousTags, final Set<Tag> de
     public Set<Tag> generateTagsToRemove(final Set<Tag> previousTags, final Set<Tag> desiredTags) {
         return Sets.difference(new HashSet<>(previousTags), new HashSet<>(desiredTags));
     }
-
-
-    /**
-     * tagResource during update
-     *
-     * Calls the service:TagResource API.
-     */
-    private ProgressEvent<ResourceModel, CallbackContext>
-    tagResource(final AmazonWebServicesClientProxy proxy, final ProxyClient<SdkClient> serviceClient, final ResourceModel resourceModel,
-                final ResourceHandlerRequest<ResourceModel> handlerRequest, final CallbackContext callbackContext, final Map<String, String> addedTags, final Logger logger) {
-        // TODO: add log for adding tags to resources during update
-        // e.g. logger.log(String.format("[UPDATE][IN PROGRESS] Going to add tags for ... resource: %s with AccountId: %s",
-        // resourceModel.getResourceName(), handlerRequest.getAwsAccountId()));
-
-        // TODO: change untagResource in the method to your service API according to your SDK
-        return proxy.initiate("AWS-RedshiftServerless-Namespace::TagOps", serviceClient, resourceModel, callbackContext)
-            .translateToServiceRequest(model ->
-                Translator.tagResourceRequest(model, addedTags))
-            .makeServiceCall((request, client) -> {
-                return (AwsResponse) null;
-                // TODO: replace the return null with your invoke log to call tagResource API to add tags
-                // e.g. proxy.injectCredentialsAndInvokeV2(request, client.client()::tagResource))
-            })
-            .progress();
-    }
-
-    /**
-     * untagResource during update
-     *
-     * Calls the service:UntagResource API.
-     */
-    private ProgressEvent<ResourceModel, CallbackContext>
-    untagResource(final AmazonWebServicesClientProxy proxy, final ProxyClient<SdkClient> serviceClient, final ResourceModel resourceModel,
-                  final ResourceHandlerRequest<ResourceModel> handlerRequest, final CallbackContext callbackContext, final Set<String> removedTags, final Logger logger) {
-        // TODO: add log for removing tags from resources during update
-        // e.g. logger.log(String.format("[UPDATE][IN PROGRESS] Going to remove tags for ... resource: %s with AccountId: %s",
-        // resourceModel.getResourceName(), handlerRequest.getAwsAccountId()));
-
-        // TODO: change untagResource in the method to your service API according to your SDK
-        return proxy.initiate("AWS-RedshiftServerless-Namespace::TagOps", serviceClient, resourceModel, callbackContext)
-            .translateToServiceRequest(model ->
-                Translator.untagResourceRequest(model, removedTags))
-            .makeServiceCall((request, client) -> {
-                return (AwsResponse) null;
-                // TODO: replace the return null with your invoke log to call untag API to remove tags
-                // e.g. proxy.injectCredentialsAndInvokeV2(request, client.client()::untagResource)
-            })
-            .progress();
-    }
-
 }
+

aws-redshiftserverless-namespace/src/main/java/software/amazon/redshiftserverless/namespace/Translator.java

@@ -26,6 +26,7 @@
 import software.amazon.awssdk.services.redshiftserverless.model.TagResourceRequest;
 import software.amazon.awssdk.services.redshiftserverless.model.UntagResourceRequest;
 import software.amazon.cloudformation.proxy.Logger;
+import software.amazon.cloudformation.proxy.ResourceHandlerRequest;
 
 import java.io.IOException;
 import java.net.URLDecoder;
@@ -36,6 +37,10 @@
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
+import static software.amazon.redshiftserverless.namespace.TagHelper.convertToTagSet;
+import static software.amazon.redshiftserverless.namespace.TagHelper.generateTagsToAdd;
+import static software.amazon.redshiftserverless.namespace.TagHelper.generateTagsToRemove;
+
 /**
  * This class is a centralized placeholder for
  *  - api request construction
@@ -49,9 +54,9 @@ public class Translator {
   /**
    * Request to create a resource
    * @param model resource model
-   * @return awsRequest the aws service request to create a resource
+   * @return mergedTags - tags merging resource, stack and system tags.
    */
-  static CreateNamespaceRequest translateToCreateRequest(final ResourceModel model) {
+  static CreateNamespaceRequest translateToCreateRequest(final ResourceModel model, final Set<Tag>mergedTags) {
     return CreateNamespaceRequest.builder()
             .namespaceName(model.getNamespaceName())
             .adminUsername(model.getAdminUsername())
@@ -61,7 +66,7 @@ static CreateNamespaceRequest translateToCreateRequest(final ResourceModel model
             .defaultIamRoleArn(model.getDefaultIamRoleArn())
             .iamRoles(model.getIamRoles())
             .logExportsWithStrings(model.getLogExports())
-            .tags(translateToSdkTags(model.getTags()))
+            .tags(translateToSdkTags(mergedTags))
             .manageAdminPassword(model.getManageAdminPassword())
             .adminPasswordSecretKmsKeyId(model.getAdminPasswordSecretKmsKeyId())
             .redshiftIdcApplicationArn(model.getRedshiftIdcApplicationArn())
@@ -207,6 +212,30 @@ static UpdateNamespaceRequest translateToUpdateRequest(final ResourceModel model
             .build();
   }
 
+  /**
+   * Request to update properties of a previously created resource
+   * @param previousTags Tags associated before Update operation
+   * @param desiredTags Tags to be associated after Update operation
+   * @param resourceArn ResourceArn to associate the tags
+   * @return UpdateTagsRequest
+   */
+  static UpdateTagsRequest translateToUpdateTagsRequest(Map<String, String>previousTags, Map<String, String>desiredTags, String resourceArn) {
+    Set<Tag> toBeCreatedTags = convertToTagSet(generateTagsToAdd(previousTags, desiredTags));
+
+    Set<String> tagKeysToBeDeleted = generateTagsToRemove(previousTags, desiredTags);
+
+    return UpdateTagsRequest.builder()
+            .createNewTagsRequest(TagResourceRequest.builder()
+                    .tags(translateToSdkTags(toBeCreatedTags))
+                    .resourceArn(resourceArn)
+                    .build())
+            .deleteOldTagsRequest(UntagResourceRequest.builder()
+                    .tagKeys(tagKeysToBeDeleted)
+                    .resourceArn(resourceArn)
+                    .build())
+            .build();
+  }
+
   /**
    * Request to list resources
    * @param nextToken token passed to the aws service list resources request
@@ -288,62 +317,15 @@ static ResourceModel translateFromReadTagsResponse(final ListTagsForResourceResp
             .build();
   }
 
-  /**
-   * Request to update tags for a resource
-   *
-   * @param desiredResourceState the resource model request to update tags
-   * @param currentResourceState the resource model request to delete tags
-   * @return awsRequest the aws service request to update tags of a resource
-   */
-  static UpdateTagsRequest translateToUpdateTagsRequest(final ResourceModel desiredResourceState,
-                                                        final ResourceModel currentResourceState) {
-    String resourceArn = currentResourceState.getNamespace().getNamespaceArn();
-
-    // If desiredResourceState.getTags() is null, we should preserve existing tags
-    // This ensures that when CloudFormation doesn't specify tags in the update,
-    // we don't remove existing tags
-    final List<Tag> effectiveDesiredTags;
-    if (desiredResourceState.getTags() == null && currentResourceState.getTags() != null) {
-      // Preserve existing tags by using them as the desired tags
-      effectiveDesiredTags = currentResourceState.getTags();
-    } else {
-      effectiveDesiredTags = desiredResourceState.getTags();
-    }
-
-    List<Tag> toBeCreatedTags = effectiveDesiredTags == null ? Collections.emptyList() : effectiveDesiredTags
-            .stream()
-            .filter(tag -> currentResourceState.getTags() == null || !currentResourceState.getTags().contains(tag))
-            .collect(Collectors.toList());
-
-    List<Tag> toBeDeletedTags = currentResourceState.getTags() == null ? Collections.emptyList() : currentResourceState.getTags()
-            .stream()
-            .filter(tag -> effectiveDesiredTags == null || !effectiveDesiredTags.contains(tag))
-            .collect(Collectors.toList());
-
-    return UpdateTagsRequest.builder()
-            .createNewTagsRequest(TagResourceRequest.builder()
-                    .tags(translateToSdkTags(toBeCreatedTags))
-                    .resourceArn(resourceArn)
-                    .build())
-            .deleteOldTagsRequest(UntagResourceRequest.builder()
-                    .tagKeys(toBeDeletedTags
-                            .stream()
-                            .map(Tag::getKey)
-                            .collect(Collectors.toList()))
-                    .resourceArn(resourceArn)
-                    .build())
-            .build();
-  }
-
   private static software.amazon.awssdk.services.redshiftserverless.model.Tag translateToSdkTag(Tag tag) {
     return GSON.fromJson(GSON.toJson(tag), software.amazon.awssdk.services.redshiftserverless.model.Tag.class);
   }
 
-  private static List<software.amazon.awssdk.services.redshiftserverless.model.Tag> translateToSdkTags(final List<Tag> tags) {
+  private static Set<software.amazon.awssdk.services.redshiftserverless.model.Tag> translateToSdkTags(final Set<Tag> tags) {
     return tags == null ? null : tags
             .stream()
             .map(Translator::translateToSdkTag)
-            .collect(Collectors.toList());
+            .collect(Collectors.toSet());
   }
 
   private static Tag translateToModelTag(software.amazon.awssdk.services.redshiftserverless.model.Tag tag) {