diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index c1fbb10..08928d2 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -29,6 +29,8 @@ jobs: - run: pipenv run coverage run -m pytest tests/ -svv - name: upload coverage to coveralls uses: coverallsapp/github-action@v2 + with: + fail-on-error: false sam-build-and-lint: runs-on: ubuntu-latest diff --git a/set_tags/set_bucket_tags.py b/set_tags/set_bucket_tags.py index 020925a..6dae4f6 100644 --- a/set_tags/set_bucket_tags.py +++ b/set_tags/set_bucket_tags.py @@ -49,7 +49,7 @@ def apply_tags(name, tags): @helper.create @helper.update def create_or_update(event, context): - '''Handles customm resource create and update events''' + '''Handles custom resource create and update events''' log.debug('Received event: ' + json.dumps(event, sort_keys=False)) log.info('Start Lambda processing') bucket_name = get_bucket_name(event) @@ -58,7 +58,11 @@ def create_or_update(event, context): synapse_tags = utils.get_synapse_tags(synapse_owner_id) # put_bucket_tagging is a replace operation. need to give it all # tags otherwise it will remove existing tags not in the list - all_tags = list(bucket_tags + synapse_tags) + # + # In case of duplication, Synapse tags should replace existing + # bucket tags. + all_tags = utils.merge_tags(bucket_tags, synapse_tags) + log.debug(f'Apply tags: {all_tags} to bucket {bucket_name}') apply_tags(bucket_name, all_tags) diff --git a/set_tags/set_instance_tags.py b/set_tags/set_instance_tags.py index 24efef3..5b36987 100644 --- a/set_tags/set_instance_tags.py +++ b/set_tags/set_instance_tags.py @@ -83,7 +83,12 @@ def create_or_update(event, context): extra_tags.append(provisioned_product_name_tag) access_approved_role_tag = utils.get_access_approved_role_tag(instance_tags) extra_tags.append(access_approved_role_tag) - all_tags = list(synapse_tags + extra_tags) + # + # In case of duplication, Synapse tags should replace existing + # bucket tags. + # + all_tags = utils.merge_tags(extra_tags, synapse_tags) + volume_ids = get_volume_ids(instance_id) log.debug(f'Apply tags: {all_tags} to volume {volume_ids}') diff --git a/set_tags/utils.py b/set_tags/utils.py index 9fb94be..3275c19 100644 --- a/set_tags/utils.py +++ b/set_tags/utils.py @@ -311,3 +311,13 @@ def get_access_approved_role_tag(tags): return access_approved_role_tag else: raise ValueError(f'Expected to find {PRINCIPAL_ARN_TAG_KEY} in {tags}') + +def merge_tags(list1, list2): + '''Merge two lists of tags, the second overriding the first on key collisions + ''' + unique_tags = format_tags_kv_kp(list1) + unique_tags.update(format_tags_kv_kp(list2)) + result=[] + for entry in unique_tags: + result.append({'Key':entry,'Value':unique_tags[entry]}) + return result diff --git a/tests/unit/utils/test_merge_tags.py b/tests/unit/utils/test_merge_tags.py new file mode 100644 index 0000000..c1715a0 --- /dev/null +++ b/tests/unit/utils/test_merge_tags.py @@ -0,0 +1,15 @@ +import unittest + +from set_tags import utils + +class TestMergeTags(unittest.TestCase): + + def test_happy_case(self): + tags = [] + result = utils.merge_tags([{'Key':'foo','Value':'bar'}],[{'Key':'foo2','Value':'bar2'}]) + self.assertEqual(result, [{'Key':'foo','Value':'bar'},{'Key':'foo2','Value':'bar2'}]) + + def test_collision(self): + tags = [] + result = utils.merge_tags([{'Key':'foo','Value':'bar'}],[{'Key':'foo','Value':'bar2'}]) + self.assertEqual(result, [{'Key':'foo','Value':'bar2'}])