From d80057907b5c1433a659cee9d83d9b0b449ffa9a Mon Sep 17 00:00:00 2001 From: Alina Buzachis Date: Tue, 8 Mar 2022 16:17:28 +0100 Subject: [PATCH 1/6] sns_topic - Add tags and purge_tags options Signed-off-by: Alina Buzachis --- .../fragments/972-sns_topic-add_tags.yaml | 2 + plugins/module_utils/sns.py | 37 +++++++++++++++++++ plugins/modules/sns_topic.py | 29 ++++++++++++--- 3 files changed, 62 insertions(+), 6 deletions(-) create mode 100644 changelogs/fragments/972-sns_topic-add_tags.yaml diff --git a/changelogs/fragments/972-sns_topic-add_tags.yaml b/changelogs/fragments/972-sns_topic-add_tags.yaml new file mode 100644 index 00000000000..be2690a9fca --- /dev/null +++ b/changelogs/fragments/972-sns_topic-add_tags.yaml @@ -0,0 +1,2 @@ +minor_changes: +- sns_topic - add support for ``tags`` and ``purge_tags`` (https://github.com/ansible-collections/community.aws/pull/972). diff --git a/plugins/module_utils/sns.py b/plugins/module_utils/sns.py index 27ab8773531..02c20f813fd 100644 --- a/plugins/module_utils/sns.py +++ b/plugins/module_utils/sns.py @@ -12,6 +12,10 @@ from ansible_collections.amazon.aws.plugins.module_utils.core import is_boto3_error_code from ansible_collections.amazon.aws.plugins.module_utils.ec2 import AWSRetry from ansible_collections.amazon.aws.plugins.module_utils.ec2 import camel_dict_to_snake_dict +from ansible_collections.amazon.aws.plugins.module_utils.ec2 import ansible_dict_to_boto3_tag_list +from ansible_collections.amazon.aws.plugins.module_utils.ec2 import boto3_tag_list_to_ansible_dict +from ansible.module_utils.common.dict_transformations import camel_dict_to_snake_dict +from ansible_collections.amazon.aws.plugins.module_utils.ec2 import compare_aws_tags @AWSRetry.jittered_backoff() @@ -123,3 +127,36 @@ def get_info(connection, module, topic_arn): info['subscriptions'] = [camel_dict_to_snake_dict(sub) for sub in list_topic_subscriptions(connection, module, topic_arn)] return info + + +def update_tags(client, module, topic_arn): + if module.params.get('tags') is None: + return False + + try: + existing_tags = client.list_tags_for_resource(aws_retry=True, ResourceArn=topic_arn)['Tags'] + except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: + module.fail_json_aws(e, msg="Couldn't obtain topic tags") + + to_update, to_delete = compare_aws_tags(boto3_tag_list_to_ansible_dict(existing_tags), + module.params['tags'], module.params['purge_tags']) + changed = bool(to_update or to_delete) + + if to_update: + try: + if module.check_mode: + return changed + client.tag_resource(aws_retry=True, ResourceArn=topic_arn, + Tags=ansible_dict_to_boto3_tag_list(to_update)) + except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: + module.fail_json_aws(e, msg="Couldn't add tags to topic") + if to_delete: + try: + if module.check_mode: + return changed + client.untag_resource(aws_retry=True, ResourceArn=topic_arn, + TagKeys=to_delete) + except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: + module.fail_json_aws(e, msg="Couldn't remove tags from topic") + + return changed diff --git a/plugins/modules/sns_topic.py b/plugins/modules/sns_topic.py index 166fb68a66f..58773a4bdbb 100644 --- a/plugins/modules/sns_topic.py +++ b/plugins/modules/sns_topic.py @@ -12,8 +12,7 @@ short_description: Manages AWS SNS topics and subscriptions version_added: 1.0.0 description: - - The M(community.aws.sns_topic) module allows you to create, delete, and manage subscriptions for AWS SNS topics. - - As of 2.6, this module can be use to subscribe and unsubscribe to topics outside of your AWS account. + - The M(community.aws.sns_topic) module allows you to create, delete, and manage subscriptions for AWS SNS topics. author: - "Joel Thompson (@joelthompson)" - "Fernando Jose Pando (@nand0p)" @@ -149,10 +148,13 @@ Blame Amazon." default: true type: bool +notes: + - Support for I(tags) and I(purge_tags) was added in release 5.3.0. extends_documentation_fragment: -- amazon.aws.aws -- amazon.aws.ec2 -- amazon.aws.boto3 + - amazon.aws.common.modules + - amazon.aws.region.modules + - amazon.aws.tags.modules + - amazon.aws.boto3 ''' EXAMPLES = r""" @@ -328,6 +330,7 @@ from ansible_collections.community.aws.plugins.module_utils.modules import AnsibleCommunityAWSModule as AnsibleAWSModule from ansible_collections.amazon.aws.plugins.module_utils.core import scrub_none_parameters from ansible_collections.amazon.aws.plugins.module_utils.ec2 import compare_policies +from ansible_collections.amazon.aws.plugins.module_utils.ec2 import ansible_dict_to_boto3_tag_list from ansible_collections.community.aws.plugins.module_utils.sns import list_topics from ansible_collections.community.aws.plugins.module_utils.sns import topic_arn_lookup from ansible_collections.community.aws.plugins.module_utils.sns import compare_delivery_policies @@ -349,6 +352,8 @@ def __init__(self, delivery_policy, subscriptions, purge_subscriptions, + tags, + purge_tags, check_mode): self.connection = module.client('sns') @@ -371,6 +376,8 @@ def __init__(self, self.topic_deleted = False self.topic_arn = None self.attributes_set = [] + self.tags = tags + self.purge_tags = purge_tags def _create_topic(self): attributes = {} @@ -383,6 +390,9 @@ def _create_topic(self): if not self.name.endswith('.fifo'): self.name = self.name + '.fifo' + if self.tags: + tags = ansible_dict_to_boto3_tag_list(self.tags) + if not self.check_mode: try: response = self.connection.create_topic(Name=self.name, @@ -542,12 +552,13 @@ def ensure_ok(self): elif self.display_name or self.policy or self.delivery_policy: self.module.fail_json(msg="Cannot set display name, policy or delivery policy for SNS topics not owned by this account") changed |= self._set_topic_subs() - self._init_desired_subscription_attributes() if self.topic_arn in list_topics(self.connection, self.module): changed |= self._set_topic_subs_attributes() elif any(self.desired_subscription_attributes.values()): self.module.fail_json(msg="Cannot set subscription attributes for SNS topics not owned by this account") + # Check tagging + changed |= update_tags(self.connection, self.module, self.topic_arn) return changed @@ -600,6 +611,8 @@ def main(): delivery_policy=dict(type='dict', options=delivery_args), subscriptions=dict(default=[], type='list', elements='dict'), purge_subscriptions=dict(type='bool', default=True), + tags=dict(type='dict', aliases=['resource_tags']), + purge_tags=dict(type='bool', default=True), ) module = AnsibleAWSModule(argument_spec=argument_spec, @@ -614,6 +627,8 @@ def main(): subscriptions = module.params.get('subscriptions') purge_subscriptions = module.params.get('purge_subscriptions') check_mode = module.check_mode + tags = module.params.get('tags') + purge_tags = module.params.get('purge_tags') sns_topic = SnsTopicManager(module, name, @@ -624,6 +639,8 @@ def main(): delivery_policy, subscriptions, purge_subscriptions, + tags, + purge_tags, check_mode) if state == 'present': From 792fd6e6f861cebf30ae1828fb96fab17b389b09 Mon Sep 17 00:00:00 2001 From: Alina Buzachis Date: Thu, 24 Mar 2022 13:27:23 +0100 Subject: [PATCH 2/6] Add integration tests Signed-off-by: Alina Buzachis --- plugins/module_utils/sns.py | 22 +-- plugins/modules/sns_topic.py | 1 + .../targets/sns_topic/tasks/main.yml | 164 ++++++++++++++++++ 3 files changed, 177 insertions(+), 10 deletions(-) diff --git a/plugins/module_utils/sns.py b/plugins/module_utils/sns.py index 02c20f813fd..74665e0a05b 100644 --- a/plugins/module_utils/sns.py +++ b/plugins/module_utils/sns.py @@ -91,6 +91,13 @@ def canonicalize_endpoint(protocol, endpoint): return endpoint +def get_tags(client, module, topic_arn): + try: + return boto3_tag_list_to_ansible_dict(client.list_tags_for_resource(ResourceArn=topic_arn)['Tags']) + except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: + module.fail_json_aws(e, msg="Couldn't obtain topic tags") + + def get_info(connection, module, topic_arn): name = module.params.get('name') topic_type = module.params.get('topic_type') @@ -125,7 +132,7 @@ def get_info(connection, module, topic_arn): info.update(camel_dict_to_snake_dict(connection.get_topic_attributes(TopicArn=topic_arn)['Attributes'])) info['delivery_policy'] = info.pop('effective_delivery_policy') info['subscriptions'] = [camel_dict_to_snake_dict(sub) for sub in list_topic_subscriptions(connection, module, topic_arn)] - + info["tags"] = get_tags(connection, module, topic_arn) return info @@ -133,11 +140,7 @@ def update_tags(client, module, topic_arn): if module.params.get('tags') is None: return False - try: - existing_tags = client.list_tags_for_resource(aws_retry=True, ResourceArn=topic_arn)['Tags'] - except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: - module.fail_json_aws(e, msg="Couldn't obtain topic tags") - + existing_tags = get_tags(client, module, topic_arn) to_update, to_delete = compare_aws_tags(boto3_tag_list_to_ansible_dict(existing_tags), module.params['tags'], module.params['purge_tags']) changed = bool(to_update or to_delete) @@ -146,16 +149,15 @@ def update_tags(client, module, topic_arn): try: if module.check_mode: return changed - client.tag_resource(aws_retry=True, ResourceArn=topic_arn, - Tags=ansible_dict_to_boto3_tag_list(to_update)) + client.tag_resource(ResourceArn=topic_arn, + Tags=ansible_dict_to_boto3_tag_list(to_update)) except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: module.fail_json_aws(e, msg="Couldn't add tags to topic") if to_delete: try: if module.check_mode: return changed - client.untag_resource(aws_retry=True, ResourceArn=topic_arn, - TagKeys=to_delete) + client.untag_resource(ResourceArn=topic_arn, TagKeys=to_delete) except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: module.fail_json_aws(e, msg="Couldn't remove tags from topic") diff --git a/plugins/modules/sns_topic.py b/plugins/modules/sns_topic.py index 58773a4bdbb..ac7a351f6f0 100644 --- a/plugins/modules/sns_topic.py +++ b/plugins/modules/sns_topic.py @@ -337,6 +337,7 @@ from ansible_collections.community.aws.plugins.module_utils.sns import list_topic_subscriptions from ansible_collections.community.aws.plugins.module_utils.sns import canonicalize_endpoint from ansible_collections.community.aws.plugins.module_utils.sns import get_info +from ansible_collections.community.aws.plugins.module_utils.sns import update_tags class SnsTopicManager(object): diff --git a/tests/integration/targets/sns_topic/tasks/main.yml b/tests/integration/targets/sns_topic/tasks/main.yml index b60acec62b8..3e443c02b98 100644 --- a/tests/integration/targets/sns_topic/tasks/main.yml +++ b/tests/integration/targets/sns_topic/tasks/main.yml @@ -427,6 +427,170 @@ that: - third_party_deletion is failed - third_party_topic.sns_topic.subscriptions|length == third_party_deletion_facts.sns_topic.subscriptions|length + + # Test tags + - name: create standard SNS topic + sns_topic: + name: '{{ sns_topic_topic_name }}' + display_name: My topic name + register: sns_topic_create + + - name: assert that creation worked + assert: + that: + - sns_topic_create.changed + + - name: set sns_arn fact + set_fact: + sns_arn: '{{ sns_topic_create.sns_arn }}' + + - name: Add tags to topic - CHECK_MODE + sns_topic: + name: '{{ sns_topic_topic_name }}' + tags: + tag_one: '{{ tiny_prefix }} One' + "Tag Two": 'two {{ tiny_prefix }}' + check_mode: true + register: sns_topic_tags + + - assert: + that: + - sns_topic_tags is changed + + - name: Add tags to topic + sns_topic: + name: '{{ sns_topic_topic_name }}' + tags: + tag_one: '{{ tiny_prefix }} One' + "Tag Two": 'two {{ tiny_prefix }}' + register: sns_topic_tags + + - assert: + that: + - sns_topic_tags is changed + + - name: Add tags to topic to verify idempotency - CHECK_MODE + sns_topic: + name: '{{ sns_topic_topic_name }}' + tags: + tag_one: '{{ tiny_prefix }} One' + "Tag Two": 'two {{ tiny_prefix }}' + check_mode: true + register: sns_topic_tags + + - assert: + that: + - sns_topic_tags is not changed + + - name: Add tags to topic to verify idempotency + sns_topic: + name: '{{ sns_topic_topic_name }}' + tags: + tag_one: '{{ tiny_prefix }} One' + "Tag Two": 'two {{ tiny_prefix }}' + register: sns_topic_tags + + - assert: + that: + - sns_topic_tags is not changed + + - name: Update (add/remove) tags - CHECK_MODE + sns_topic: + name: '{{ sns_topic_topic_name }}' + tags: + tag_three: '{{ tiny_prefix }} Three' + "Tag Two": 'two {{ tiny_prefix }}' + check_mode: true + register: sns_topic_tags + + - assert: + that: + - sns_topic_tags is changed + + - name: Update tags to verify idempotency + sns_topic: + name: '{{ sns_topic_topic_name }}' + tags: + tag_three: '{{ tiny_prefix }} Three' + "Tag Two": 'two {{ tiny_prefix }}' + register: sns_topic_tags + + - assert: + that: + - sns_topic_tags is changed + + - name: Update tags without purge - CHECK_MODE + sns_topic: + name: '{{ sns_topic_topic_name }}' + purge_tags: no + tags: + tag_one: '{{ tiny_prefix }} One' + check_mode: true + register: sns_topic_tags + + - assert: + that: + - sns_topic_tags is changed + + - name: Update tags without purge + sns_topic: + name: '{{ sns_topic_topic_name }}' + purge_tags: no + tags: + tag_one: '{{ tiny_prefix }} One' + register: sns_topic_tags + + - assert: + that: + - sns_topic_tags is changed + + - name: Remove all the tags - CHECK_MODE + sns_topic: + name: '{{ sns_topic_topic_name }}' + purge_tags: no + tags: {} + check_mode: true + register: sns_topic_tags + + - assert: + that: + - sns_topic_tags is changed + + - name: Remove all the tags + sns_topic: + name: '{{ sns_topic_topic_name }}' + purge_tags: no + tags: {} + register: sns_topic_tags + + - assert: + that: + - sns_topic_tags is changed + + - name: Update with CamelCase tags + sns_topic: + name: '{{ sns_topic_topic_name }}' + purge_tags: no + tags: + "lowercase spaced": 'hello cruel world' + "Title Case": 'Hello Cruel World' + CamelCase: 'SimpleCamelCase' + snake_case: 'simple_snake_case' + register: sns_topic_tags + + - assert: + that: + - sns_topic_tags is changed + + - name: Do not specify any tag to ensure previous tags are not removed + sns_topic: + name: '{{ sns_topic_topic_name }}' + purge_tags: no + register: sns_topic_tags + + - assert: + that: + - sns_topic_tags is not changed always: From 11bdaf8b0493bc7c639d6e000e83c1d3418dfc0b Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Wed, 1 Feb 2023 15:16:11 +0100 Subject: [PATCH 3/6] Reduce AuthZ issues when reading tags to a warn --- plugins/module_utils/sns.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/plugins/module_utils/sns.py b/plugins/module_utils/sns.py index 74665e0a05b..e5f928dc992 100644 --- a/plugins/module_utils/sns.py +++ b/plugins/module_utils/sns.py @@ -94,6 +94,9 @@ def canonicalize_endpoint(protocol, endpoint): def get_tags(client, module, topic_arn): try: return boto3_tag_list_to_ansible_dict(client.list_tags_for_resource(ResourceArn=topic_arn)['Tags']) + except is_boto3_error_code('AuthorizationError'): + module.warn("Permission denied accessing tags") + return {} except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: module.fail_json_aws(e, msg="Couldn't obtain topic tags") From 684d3d85d633dede4f7f9c4c80d720f7b0b6c97b Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Wed, 1 Feb 2023 15:31:55 +0100 Subject: [PATCH 4/6] Fix double-tag-list-conversion --- plugins/module_utils/sns.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/plugins/module_utils/sns.py b/plugins/module_utils/sns.py index e5f928dc992..7c5f4b0b7dc 100644 --- a/plugins/module_utils/sns.py +++ b/plugins/module_utils/sns.py @@ -144,8 +144,7 @@ def update_tags(client, module, topic_arn): return False existing_tags = get_tags(client, module, topic_arn) - to_update, to_delete = compare_aws_tags(boto3_tag_list_to_ansible_dict(existing_tags), - module.params['tags'], module.params['purge_tags']) + to_update, to_delete = compare_aws_tags(existing_tags, module.params['tags'], module.params['purge_tags']) changed = bool(to_update or to_delete) if to_update: From 68768c0064432ff89f6411a234b66949f7b4911d Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Wed, 1 Feb 2023 16:09:47 +0100 Subject: [PATCH 5/6] tagging - bail early if no changes or in check mode --- plugins/module_utils/sns.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/plugins/module_utils/sns.py b/plugins/module_utils/sns.py index 7c5f4b0b7dc..17fc136a1d6 100644 --- a/plugins/module_utils/sns.py +++ b/plugins/module_utils/sns.py @@ -140,27 +140,29 @@ def get_info(connection, module, topic_arn): def update_tags(client, module, topic_arn): + if module.params.get('tags') is None: return False existing_tags = get_tags(client, module, topic_arn) to_update, to_delete = compare_aws_tags(existing_tags, module.params['tags'], module.params['purge_tags']) - changed = bool(to_update or to_delete) + + if not bool(to_delete or to_update): + return False + + if module.check_mode: + return True if to_update: try: - if module.check_mode: - return changed client.tag_resource(ResourceArn=topic_arn, Tags=ansible_dict_to_boto3_tag_list(to_update)) except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: module.fail_json_aws(e, msg="Couldn't add tags to topic") if to_delete: try: - if module.check_mode: - return changed client.untag_resource(ResourceArn=topic_arn, TagKeys=to_delete) except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: module.fail_json_aws(e, msg="Couldn't remove tags from topic") - return changed + return True From 8166bcc0d74de8ccb2a59c95ed4298669ec2a8f0 Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Wed, 1 Feb 2023 16:10:14 +0100 Subject: [PATCH 6/6] Actually purge tags when testing purge tags --- .../targets/sns_topic/tasks/main.yml | 58 +++++++++---------- 1 file changed, 27 insertions(+), 31 deletions(-) diff --git a/tests/integration/targets/sns_topic/tasks/main.yml b/tests/integration/targets/sns_topic/tasks/main.yml index 3e443c02b98..5465aad4fdb 100644 --- a/tests/integration/targets/sns_topic/tasks/main.yml +++ b/tests/integration/targets/sns_topic/tasks/main.yml @@ -14,13 +14,9 @@ create_instance_profile: false managed_policies: - 'arn:aws:iam::aws:policy/AWSXrayWriteOnlyAccess' + wait: True register: iam_role - - name: pause if role was created - pause: - seconds: 10 - when: iam_role is changed - - name: list all the topics (check_mode) sns_topic_info: check_mode: true @@ -427,7 +423,7 @@ that: - third_party_deletion is failed - third_party_topic.sns_topic.subscriptions|length == third_party_deletion_facts.sns_topic.subscriptions|length - + # Test tags - name: create standard SNS topic sns_topic: @@ -452,11 +448,11 @@ "Tag Two": 'two {{ tiny_prefix }}' check_mode: true register: sns_topic_tags - + - assert: that: - sns_topic_tags is changed - + - name: Add tags to topic sns_topic: name: '{{ sns_topic_topic_name }}' @@ -464,11 +460,11 @@ tag_one: '{{ tiny_prefix }} One' "Tag Two": 'two {{ tiny_prefix }}' register: sns_topic_tags - + - assert: that: - sns_topic_tags is changed - + - name: Add tags to topic to verify idempotency - CHECK_MODE sns_topic: name: '{{ sns_topic_topic_name }}' @@ -477,11 +473,11 @@ "Tag Two": 'two {{ tiny_prefix }}' check_mode: true register: sns_topic_tags - + - assert: that: - sns_topic_tags is not changed - + - name: Add tags to topic to verify idempotency sns_topic: name: '{{ sns_topic_topic_name }}' @@ -489,11 +485,11 @@ tag_one: '{{ tiny_prefix }} One' "Tag Two": 'two {{ tiny_prefix }}' register: sns_topic_tags - + - assert: that: - sns_topic_tags is not changed - + - name: Update (add/remove) tags - CHECK_MODE sns_topic: name: '{{ sns_topic_topic_name }}' @@ -502,11 +498,11 @@ "Tag Two": 'two {{ tiny_prefix }}' check_mode: true register: sns_topic_tags - + - assert: that: - sns_topic_tags is changed - + - name: Update tags to verify idempotency sns_topic: name: '{{ sns_topic_topic_name }}' @@ -514,11 +510,11 @@ tag_three: '{{ tiny_prefix }} Three' "Tag Two": 'two {{ tiny_prefix }}' register: sns_topic_tags - + - assert: that: - sns_topic_tags is changed - + - name: Update tags without purge - CHECK_MODE sns_topic: name: '{{ sns_topic_topic_name }}' @@ -527,11 +523,11 @@ tag_one: '{{ tiny_prefix }} One' check_mode: true register: sns_topic_tags - + - assert: that: - sns_topic_tags is changed - + - name: Update tags without purge sns_topic: name: '{{ sns_topic_topic_name }}' @@ -539,34 +535,34 @@ tags: tag_one: '{{ tiny_prefix }} One' register: sns_topic_tags - + - assert: that: - sns_topic_tags is changed - + - name: Remove all the tags - CHECK_MODE sns_topic: name: '{{ sns_topic_topic_name }}' - purge_tags: no + purge_tags: yes tags: {} check_mode: true register: sns_topic_tags - + - assert: that: - sns_topic_tags is changed - + - name: Remove all the tags sns_topic: name: '{{ sns_topic_topic_name }}' - purge_tags: no + purge_tags: yes tags: {} register: sns_topic_tags - + - assert: that: - sns_topic_tags is changed - + - name: Update with CamelCase tags sns_topic: name: '{{ sns_topic_topic_name }}' @@ -577,17 +573,17 @@ CamelCase: 'SimpleCamelCase' snake_case: 'simple_snake_case' register: sns_topic_tags - + - assert: that: - sns_topic_tags is changed - + - name: Do not specify any tag to ensure previous tags are not removed sns_topic: name: '{{ sns_topic_topic_name }}' purge_tags: no register: sns_topic_tags - + - assert: that: - sns_topic_tags is not changed