fix: make tags safer, refactor legacy lambda, fix rls (#1234)

Author: iakov-gan

bump-release.py

@@ -1,14 +1,16 @@
-import os
 import sys
+import subprocess
 
-os.system('git checkout main')
-os.system('git pull')
+
+current_branch = subprocess.check_output('git branch --show-current', shell=True).decode('utf-8').strip()
+subprocess.check_output('git checkout main', shell=True)
+subprocess.check_output('git pull', shell=True)
 
 from cid._version import __version__ as old_ver
 
-bump='patch'
-if len(sys.argv)>1:
-	bump = sys.argv[1]
+bump = 'patch'
+if 'minor' in sys.argv:
+	bump = 'minor'
 
 maj, minor, patch = map(int, old_ver.split('.'))
 
@@ -19,8 +21,7 @@
 else:
 	raise NotImplementedError('only patch and minor are implemented')
 
-os.system(f"git checkout -b 'release/{new_ver}'")
-
+print(subprocess.check_output(f"git checkout -b 'release/{new_ver}'", shell=True).decode('utf-8').strip())
 
 tx = open('cid/_version.py').read()
 with open('cid/_version.py', "w") as f:
@@ -31,6 +32,14 @@
 	f.write(tx.replace(f"{old_ver}", f"{new_ver}"))
 
 
-os.system('git diff HEAD --unified=0')
-print('to undo:\n git checkout HEAD -- cfn-templates/cid-cfn.yml cid/_version.py')
-print(f"to continue:\n git commit -am 'release {new_ver}'; git push origin 'release/{new_ver}'")
+print(subprocess.check_output('git diff HEAD --unified=0 --color', shell=True).decode('utf-8'))
+
+if '--merge' in sys.argv:
+	print(subprocess.check_output(f"git commit -am 'release {new_ver}'", shell=True).decode('utf-8').strip())
+	print(subprocess.check_output(f"git checkout {current_branch}", shell=True).decode('utf-8').strip())
+	print(subprocess.check_output(f"git merge 'release/{new_ver}'", shell=True).decode('utf-8').strip())
+	print(subprocess.check_output(f"git branch -D 'release/{new_ver}'", shell=True).decode('utf-8').strip())
+	print(f'Merged to {current_branch}')
+else:
+	print('to undo:\n git checkout HEAD -- cfn-templates/cid-cfn.yml cid/_version.py')
+	print(f"to continue:\n git commit -am 'release {new_ver}'; git push origin 'release/{new_ver}'")

cfn-templates/cid-cfn.yml

@@ -1,6 +1,6 @@
 # https://github.com/aws-samples/aws-cudos-framework-deployment/blob/main/cfn-templates/cid-cfn.yml
 AWSTemplateFormatVersion: '2010-09-09'
-Description: Deployment of Cloud Intelligence Dashboards v4.2.5 - AWS Solution SO9011
+Description: Deployment of Cloud Intelligence Dashboards v4.2.6 - AWS Solution SO9011
 Metadata:
   AWS::CloudFormation::Interface:
     ParameterGroups:
@@ -605,26 +605,11 @@ Resources:
       Handler: 'index.lambda_handler'
       Code:
         ZipFile: |
-          # This is a legacy lambda. You can delete it. This was kept to disable delete workgroup functionality.
-          import json
-          import urllib3
-
+          # Legacy lambda - will be deleted in next major update
+          import cfnresponse
+          
           def lambda_handler(event, context):
-              url = event.get('ResponseURL')
-              json_body = json.dumps({
-                'Status': 'SUCCESS',
-                'Reason': 'legacy',
-                'PhysicalResourceId': 'keep_it_constant',
-                'StackId': event.get('StackId'),
-                'RequestId': event.get('RequestId'),
-                'LogicalResourceId': event.get('LogicalResourceId'),
-              })
-              try:
-                  http = urllib3.PoolManager()
-                  response = http.request('PUT', url, body=json_body, headers={'content-type' : '', 'content-length' : str(len(json_body))}, retries=False)
-                  print(f"Status code: {response}")
-              except Exception as exc:
-                  print("Failed sending PUT to CFN: " + str(exc))
+              cfnresponse.send(event, context, cfnresponse.SUCCESS, {}, 'keep_it_constant', 'legacy')
     Metadata:
       cfn_nag:
         rules_to_suppress:
@@ -2070,7 +2055,7 @@ Resources:
       SourceBucket: !Ref ReferenceAssetsBucket
       DestinationBucket: !Ref LocalAssetsBucket
       Keys:
-        - 'cid-resource-lambda-layer/cid-4.2.5.zip' #replace version here if needed
+        - 'cid-resource-lambda-layer/cid-4.2.6.zip' #replace version here if needed
 
   CidResourceLambdaLayer:
     Type: AWS::Lambda::LayerVersion
@@ -2085,7 +2070,7 @@ Resources:
             - LambdaLayerBucketPrefixIsManaged
             - !FindInMap [RegionMap, !Ref 'AWS::Region', BucketName]
             - !Sub '${LambdaLayerBucketPrefix}-${AWS::Region}' # Region added for backward compatibility
-        S3Key: 'cid-resource-lambda-layer/cid-4.2.5.zip' #replace version here if needed
+        S3Key: 'cid-resource-lambda-layer/cid-4.2.6.zip' #replace version here if needed
       CompatibleRuntimes:
         - python3.10
         - python3.11

cid/_version.py

@@ -1,2 +1,2 @@
-__version__ = '4.2.5'
+__version__ = '4.2.6'
 

cid/common.py

@@ -1668,17 +1668,11 @@ def create_or_update_dataset(self, dataset_definition: dict, dataset_id: str=Non
                         update_dataset = True
                     break
 
-            identical = False # check if dataset needs an update
-            if isinstance(found_dataset, Dataset):
-                identical = True
-                for key in 'PhysicalTableMap LogicalTableMap OutputColumns ImportMode DataSetUsageConfiguration RowLevelPermissionDataSet FieldFolders RowLevelPermissionTagConfiguration DatasetParameters'.split():
-                    if found_dataset.raw.get(key) != compiled_dataset.get(key):
-                        logger.trace(f'not identical {key} {found_dataset.raw.get(key)} != {compiled_dataset.get(key)}')
-                        identical = False
-                logger.trace(f'identical to existing = {identical}')
+            identical = Dataset.datasets_are_identical(found_dataset, compiled_dataset) # check if dataset needs an update
 
             if update_dataset and not identical:
-                self.qs.update_dataset(compiled_dataset)
+                merged_dataset = Dataset.merge_datasets(compiled_dataset, found_dataset)
+                self.qs.update_dataset(merged_dataset)
                 if compiled_dataset.get("ImportMode") == "SPICE":
                     dataset_id = compiled_dataset.get('DataSetId')
                     schedules_definitions = []

cid/helpers/quicksight/__init__.py

@@ -261,7 +261,9 @@ def set_tags(self, arn, **tags):
             return True
         except self.client.exceptions.AccessDeniedException as exc:
             logger.debug(f'Cannot tag {arn} (AccessDenied).')
-            return False
+        except self.client.exceptions.ClientError as exc:
+            logger.debug(f'Cannot tag {arn} ({exc}).')
+        return False
 
 
     def get_tags(self, arn):

cid/helpers/quicksight/dataset.py

@@ -8,6 +8,12 @@
 
 logger = logging.getLogger(__name__)
 
+DATASET_PROPERTIES = [
+    'AwsAccountId', 'DataSetId', 'Name', 'PhysicalTableMap', 'LogicalTableMap', 'ImportMode', 'ColumnGroups',
+    'RowLevelPermissionDataSet', 'RowLevelPermissionTagConfiguration', 'FieldFolders', 'ColumnLevelPermissionRules',
+    'DataSetUsageConfiguration', 'DatasetParameters', 'PerformanceConfiguration'
+]
+
 
 class Dataset(CidQsResource):
 
@@ -189,7 +195,7 @@ def _athena_to_qs_type(col, athena_type):
                 projected_cols.append(col)
 
         # filter out all columns that cannot be used for dataset creation
-        update_ = {key: value for key, value in dataset.items() if key in 'DataSetId, Name, PhysicalTableMap, LogicalTableMap, ImportMode, ColumnGroups, FieldFolders, RowLevelPermissionDataSet, RowLevelPermissionTagConfiguration, ColumnLevelPermissionRules, DataSetUsageConfiguration, DatasetParameters'.split(', ')}
+        update_ = {key: value for key, value in dataset.items() if key in DATASET_PROPERTIES}
         logger.trace(f'update_ = {update_}')
         return update_
 
@@ -230,4 +236,41 @@ def to_diffable_structure(self):
         for alias, join in join_clauses.items():
             if isinstance(data['Data'].get(alias), dict) :
                 data['Data'][alias]['clause'] = join
-        return (yaml.safe_dump(data))
+        return (yaml.safe_dump(data))
+
+    @staticmethod
+    def datasets_are_identical(dataset1, dataset2):
+        ''' Compare 2 datasets and returns True if no difference found
+        '''
+        if (not dataset1 and not dataset2):
+            return True
+        identical = False
+        if (dataset1 and not dataset2) or (not dataset1 and dataset2):
+            return identical
+        dataset1 = dataset1 if isinstance(dataset1, Dataset) else Dataset(dataset1)
+        dataset2 = dataset2 if isinstance(dataset2, Dataset) else Dataset(dataset2)
+        identical = True
+        for key in DATASET_PROPERTIES:
+            if key in ['AwsAccountId', 'DataSetId']:
+                continue
+            if dataset1.raw.get(key) != dataset2.raw.get(key):
+                logger.trace(f'not identical {key} {dataset1.raw.get(key)} != {dataset2.raw.get(key)}')
+                identical = False
+        logger.trace(f'identical to existing = {identical}')
+        return identical
+
+    @staticmethod
+    def merge_datasets(dataset1, dataset2):
+        ''' merge high level 2 datasets. Not a deep merge.
+        '''
+        if not dataset2:
+            return dataset1
+        if not dataset1:
+            return dataset2
+        dataset1 = dataset1 if isinstance(dataset1, Dataset) else Dataset(dataset1)
+        dataset2 = dataset2 if isinstance(dataset2, Dataset) else Dataset(dataset2)
+        result = {}
+        for key in DATASET_PROPERTIES:
+            if dataset1.raw.get(key) or dataset2.raw.get(key):
+                result[key] = dataset1.raw.get(key) or dataset2.raw.get(key)
+        return result