From 8c4e2d5f52d8c0c6f3eefb36bb7288c933e0a6e3 Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Fri, 27 May 2022 10:53:40 +0200 Subject: [PATCH] Do not purge 'aws:*' tags (#825) Do not purge 'aws:*' tags SUMMARY fixes: #817 According to the AWS documentation System created tags that begin with aws: are reserved for AWS use ... You can't edit or delete a tag that begins with the aws: prefix. The most common use case here is AWS CloudFormation, which adds a "aws:cloudformation:stack-name" tag. Since we can't manage these tags at all we should ignore them for the purposes of 'purging'. If someone explicitly adds the tag, then it's reasonable to let the APIs throw an error at them, but otherwise do the best we can. ISSUE TYPE Feature Pull Request COMPONENT NAME plugins/module_utils/tagging.py ADDITIONAL INFORMATION Reviewed-by: Alina Buzachis Reviewed-by: Joseph Torcasso --- changelogs/fragments/817-skip_purge_aws.yaml | 2 ++ plugins/module_utils/tagging.py | 10 ++++-- .../integration/targets/ec2_instance/main.yml | 2 +- tests/unit/module_utils/test_tagging.py | 34 +++++++++++++++++++ 4 files changed, 45 insertions(+), 3 deletions(-) create mode 100644 changelogs/fragments/817-skip_purge_aws.yaml diff --git a/changelogs/fragments/817-skip_purge_aws.yaml b/changelogs/fragments/817-skip_purge_aws.yaml new file mode 100644 index 00000000000..f0d5b7b1446 --- /dev/null +++ b/changelogs/fragments/817-skip_purge_aws.yaml @@ -0,0 +1,2 @@ +breaking_changes: +- Tags beginning with ``aws:`` will not be removed when purging tags, these tags are reserved by Amazon and may not be updated or deleted (https://github.com/ansible-collections/amazon.aws/issues/817). diff --git a/plugins/module_utils/tagging.py b/plugins/module_utils/tagging.py index 99252bc4ee4..1568e4887f8 100644 --- a/plugins/module_utils/tagging.py +++ b/plugins/module_utils/tagging.py @@ -164,8 +164,14 @@ def compare_aws_tags(current_tags_dict, new_tags_dict, purge_tags=True): tag_key_value_pairs_to_set = {} tag_keys_to_unset = [] - for key in current_tags_dict.keys(): - if key not in new_tags_dict and purge_tags: + if purge_tags: + for key in current_tags_dict.keys(): + if key in new_tags_dict: + continue + # Amazon have reserved 'aws:*' tags, we should avoid purging them as + # this probably isn't what people want to do... + if key.startswith('aws:'): + continue tag_keys_to_unset.append(key) for key in set(new_tags_dict.keys()) - set(tag_keys_to_unset): diff --git a/tests/integration/targets/ec2_instance/main.yml b/tests/integration/targets/ec2_instance/main.yml index 34163e478e5..2261245a158 100644 --- a/tests/integration/targets/ec2_instance/main.yml +++ b/tests/integration/targets/ec2_instance/main.yml @@ -35,6 +35,6 @@ - hosts: all gather_facts: no strategy: free - serial: 6 + serial: 7 roles: - ec2_instance diff --git a/tests/unit/module_utils/test_tagging.py b/tests/unit/module_utils/test_tagging.py index 4f9b620e3fc..5f7e81236a9 100644 --- a/tests/unit/module_utils/test_tagging.py +++ b/tests/unit/module_utils/test_tagging.py @@ -41,6 +41,9 @@ def setUp(self): self.tag_minimal_dict = {'mykey': 'myvalue'} + self.tag_aws_dict = {'aws:cloudformation:stack-name': 'ExampleStack'} + self.tag_aws_changed = {'aws:cloudformation:stack-name': 'AnotherStack'} + # ======================================================== # tagging.ansible_dict_to_boto3_tag_list # ======================================================== @@ -139,6 +142,37 @@ def test_compare_aws_tags_complex_update(self): self.assertEqual(new_keys, keys_to_set) self.assertEqual(['Normal case'], keys_to_unset) + def test_compare_aws_tags_aws(self): + starting_tags = dict(self.tag_aws_dict) + desired_tags = dict(self.tag_minimal_dict) + tags_to_set, tags_to_unset = compare_aws_tags(starting_tags, desired_tags, purge_tags=True) + self.assertEqual(desired_tags, tags_to_set) + self.assertEqual([], tags_to_unset) + # If someone explicitly passes a changed 'aws:' key the APIs will probably + # throw an error, but this is their responsibility. + desired_tags.update(self.tag_aws_changed) + tags_to_set, tags_to_unset = compare_aws_tags(starting_tags, desired_tags, purge_tags=True) + self.assertEqual(desired_tags, tags_to_set) + self.assertEqual([], tags_to_unset) + + def test_compare_aws_tags_aws_complex(self): + old_dict = dict(self.tag_example_dict) + old_dict.update(self.tag_aws_dict) + # Adds 'Me too!', Changes 'UpperCamel' and removes 'Normal case' + new_dict = dict(self.tag_example_dict) + new_keys = {'UpperCamel': 'anotherCamelValue', 'Me too!': 'Contributing'} + new_dict.update(new_keys) + del new_dict['Normal case'] + keys_to_set, keys_to_unset = compare_aws_tags(old_dict, new_dict) + self.assertEqual(new_keys, keys_to_set) + self.assertEqual(['Normal case'], keys_to_unset) + keys_to_set, keys_to_unset = compare_aws_tags(old_dict, new_dict, purge_tags=False) + self.assertEqual(new_keys, keys_to_set) + self.assertEqual([], keys_to_unset) + keys_to_set, keys_to_unset = compare_aws_tags(old_dict, new_dict, purge_tags=True) + self.assertEqual(new_keys, keys_to_set) + self.assertEqual(['Normal case'], keys_to_unset) + # ======================================================== # tagging.boto3_tag_specifications # ========================================================