From bf4b79fccaebf95ebb4aad7d6e140665d1b0703e Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Fri, 20 May 2022 13:45:40 +0200 Subject: [PATCH] aws_secret - Support purge_tags (#1150) aws_secret - Support purge_tags SUMMARY aws_secret currently defaults to purging all tags (even if tags isn't specified), this is a little aggressive. Add purge_tags parameter Only purge tags if tags: {} is set (rather than when tags is None ISSUE TYPE Feature Pull Request COMPONENT NAME aws_secret ADDITIONAL INFORMATION Related to #1146 Reviewed-by: Markus Bergholz Reviewed-by: Mark Chappell Reviewed-by: Alina Buzachis --- .../fragments/1146-aws_secret-purge_tags.yml | 4 + plugins/modules/aws_secret.py | 70 ++- .../targets/aws_secret/defaults/main.yaml | 4 +- .../targets/aws_secret/tasks/basic.yml | 535 ++++++++++++++++-- .../aws_secret/templates/secret-policy.j2 | 4 +- 5 files changed, 566 insertions(+), 51 deletions(-) create mode 100644 changelogs/fragments/1146-aws_secret-purge_tags.yml diff --git a/changelogs/fragments/1146-aws_secret-purge_tags.yml b/changelogs/fragments/1146-aws_secret-purge_tags.yml new file mode 100644 index 00000000000..c6242f33218 --- /dev/null +++ b/changelogs/fragments/1146-aws_secret-purge_tags.yml @@ -0,0 +1,4 @@ +breaking_changes: +- aws_secret - tags are no longer removed when the ``tags`` parameter is not set. To remove all tags set ``tags={}`` (https://github.com/ansible-collections/community.aws/issues/1146). +minor_changes: +- aws_secret - addition of the ``purge_tags`` parameter (https://github.com/ansible-collections/community.aws/issues/1146). diff --git a/plugins/modules/aws_secret.py b/plugins/modules/aws_secret.py index 03f4a8d3592..7ebce8da603 100644 --- a/plugins/modules/aws_secret.py +++ b/plugins/modules/aws_secret.py @@ -62,8 +62,17 @@ version_added: 3.1.0 tags: description: - - Specifies a list of user-defined tags that are attached to the secret. + - Specifies a dictionary of user-defined tags that are attached to the secret. + - To remove all tags set I(tags={}) and I(purge_tags=true). type: dict + purge_tags: + description: + - If I(purge_tags=true) and I(tags) is set, existing tags will be purged from the resource + to match exactly what is defined by I(tags) parameter. + type: bool + required: false + default: true + version_added: 4.0.0 rotation_lambda: description: - Specifies the ARN of the Lambda function that can rotate the secret. @@ -110,12 +119,17 @@ type: complex contains: arn: - description: The ARN of the secret + description: The ARN of the secret. returned: always type: str sample: arn:aws:secretsmanager:eu-west-1:xxxxxxxxxx:secret:xxxxxxxxxxx + description: + description: A description of the secret. + returned: when the secret has a description + type: str + sample: An example description last_accessed_date: - description: The date the secret was last accessed + description: The date the secret was last accessed. returned: always type: str sample: '2018-11-20T01:00:00+01:00' @@ -139,6 +153,29 @@ returned: always type: dict sample: { "dc1ed59b-6d8e-4450-8b41-536dfe4600a9": [ "AWSCURRENT" ] } + tags: + description: + - A list of dictionaries representing the tags associated with the secret in the standard boto3 format. + returned: when the secret has tags + type: list + elements: dict + contains: + key: + description: The name or key of the tag. + type: str + example: MyTag + returned: success + value: + description: The value of the tag. + type: str + example: Some value. + returned: success + tags_dict: + description: A dictionary representing the tags associated with the secret. + type: dict + returned: when the secret has tags + example: {'MyTagName': 'Some Value'} + version_added: 4.0.0 ''' from ansible.module_utils._text import to_bytes @@ -328,12 +365,16 @@ def update_rotation(self, secret): return response def tag_secret(self, secret_name, tags): + if self.module.check_mode: + self.module.exit_json(changed=True) try: self.client.tag_resource(SecretId=secret_name, Tags=tags) except (BotoCoreError, ClientError) as e: self.module.fail_json_aws(e, msg="Failed to add tag(s) to secret") def untag_secret(self, secret_name, tag_keys): + if self.module.check_mode: + self.module.exit_json(changed=True) try: self.client.untag_resource(SecretId=secret_name, TagKeys=tag_keys) except (BotoCoreError, ClientError) as e: @@ -391,7 +432,8 @@ def main(): 'secret_type': dict(choices=['binary', 'string'], default="string"), 'secret': dict(default="", no_log=True), 'resource_policy': dict(type='json', default=None), - 'tags': dict(type='dict', default={}), + 'tags': dict(type='dict', default=None), + 'purge_tags': dict(type='bool', default=True), 'rotation_lambda': dict(), 'rotation_interval': dict(type='int', default=30), 'recovery_window': dict(type='int', default=30), @@ -414,6 +456,7 @@ def main(): lambda_arn=module.params.get('rotation_lambda'), rotation_interval=module.params.get('rotation_interval') ) + purge_tags = module.params.get('purge_tags') current_secret = secrets_mgr.get_secret(secret.name) @@ -453,15 +496,18 @@ def main(): else: result = secrets_mgr.put_resource_policy(secret) changed = True - current_tags = boto3_tag_list_to_ansible_dict(current_secret.get('Tags', [])) - tags_to_add, tags_to_remove = compare_aws_tags(current_tags, secret.tags) - if tags_to_add: - secrets_mgr.tag_secret(secret.name, ansible_dict_to_boto3_tag_list(tags_to_add)) - changed = True - if tags_to_remove: - secrets_mgr.untag_secret(secret.name, tags_to_remove) - changed = True + if module.params.get('tags') is not None: + current_tags = boto3_tag_list_to_ansible_dict(current_secret.get('Tags', [])) + tags_to_add, tags_to_remove = compare_aws_tags(current_tags, secret.tags, purge_tags) + if tags_to_add: + secrets_mgr.tag_secret(secret.name, ansible_dict_to_boto3_tag_list(tags_to_add)) + changed = True + if tags_to_remove: + secrets_mgr.untag_secret(secret.name, tags_to_remove) + changed = True result = camel_dict_to_snake_dict(secrets_mgr.get_secret(secret.name)) + if result.get('tags', None) is not None: + result['tags_dict'] = boto3_tag_list_to_ansible_dict(result.get('tags', [])) result.pop("response_metadata") module.exit_json(changed=changed, secret=result) diff --git a/tests/integration/targets/aws_secret/defaults/main.yaml b/tests/integration/targets/aws_secret/defaults/main.yaml index cfdab552493..5cd06e010e4 100644 --- a/tests/integration/targets/aws_secret/defaults/main.yaml +++ b/tests/integration/targets/aws_secret/defaults/main.yaml @@ -1,5 +1,5 @@ --- super_secret_string: 'Test12345' secret_manager_role: "{{ resource_prefix }}-secrets-manager" -secret_name: "{{ resource_prefix }}-test-secret-string" -lambda_name: "{{ resource_prefix }}-hello-world" +secret_name: "{{ resource_prefix }}-secrets-manager-secret" +lambda_name: "{{ resource_prefix }}-secrets-manager-secret" diff --git a/tests/integration/targets/aws_secret/tasks/basic.yml b/tests/integration/targets/aws_secret/tasks/basic.yml index ffe5314c148..2f504c2fb06 100644 --- a/tests/integration/targets/aws_secret/tasks/basic.yml +++ b/tests/integration/targets/aws_secret/tasks/basic.yml @@ -1,31 +1,96 @@ --- -- block: +############################################################ +# This Integration test is currently very minimal +# It would be good if someone would update the tests to run through the standard 4-step checks for +# all of the module parameters +# +# 1) Set Value (Check mode) +# - changed is True +# - object *NOT* updated +# 2) Set Value +# - changed is True +# - object updated +# 3) Set same Value (Check mode) +# - changed is False +# - object *NOT* updated +# - Tests idemoptency +# 4) Set same Value +# - changed is False +# - object *NOT* updated +# - Tests idemoptency + +- vars: + first_tags: + 'Key with Spaces': Value with spaces + CamelCaseKey: CamelCaseValue + pascalCaseKey: pascalCaseValue + snake_case_key: snake_case_value + second_tags: + 'New Key with Spaces': Value with spaces + NewCamelCaseKey: CamelCaseValue + newPascalCaseKey: pascalCaseValue + new_snake_case_key: snake_case_value + third_tags: + 'Key with Spaces': Value with spaces + CamelCaseKey: CamelCaseValue + pascalCaseKey: pascalCaseValue + snake_case_key: snake_case_value + 'New Key with Spaces': Updated Value with spaces + final_tags: + 'Key with Spaces': Value with spaces + CamelCaseKey: CamelCaseValue + pascalCaseKey: pascalCaseValue + snake_case_key: snake_case_value + 'New Key with Spaces': Updated Value with spaces + NewCamelCaseKey: CamelCaseValue + newPascalCaseKey: pascalCaseValue + new_snake_case_key: snake_case_value + block: # ============================================================ # Preparation # ============================================================ - name: 'Retrieve caller facts' aws_caller_info: - register: aws_caller_info + register: caller_info + + - name: 'Generate the ARN pattern to search for' + vars: + _caller_info: '{{ caller_info.arn.split(":") }}' + _base_arn: 'arn:{{_caller_info[1]}}:secretsmanager:{{ aws_region }}' + set_fact: + account_arn: '{{_base_arn}}:{{_caller_info[4]}}:secret:' # ============================================================ - # Module parameter testing + # Creation testing # ============================================================ - - name: test with no parameters + - name: add secret to AWS Secrets Manager aws_secret: + name: "{{ secret_name }}" + state: present + secret_type: 'string' + secret: "{{ super_secret_string }}" register: result - ignore_errors: true - check_mode: true - - name: assert failure when called with no parameters + - name: assert correct keys are returned assert: that: - - result.failed - - 'result.msg.startswith("missing required arguments:")' + - result.changed + - '"arn" in result.secret' + # - '"last_accessed_date" in result.secret' + - '"last_changed_date" in result.secret' + - '"name" in result.secret' + - '"version_ids_to_stages" in result.secret' + - result.secret.arn.startswith(account_arn) + # ARN includes a random string after the name + - secret_name in result.secret.arn + - result.secret.name == secret_name + - result.secret.version_ids_to_stages | length == 1 - # ============================================================ - # Creation/Deletion testing - # ============================================================ - - name: add secret to AWS Secrets Manager + - name: save the ARN for later comparison + set_fact: + secret_arn: '{{ result.secret.arn }}' + + - name: no changes to secret aws_secret: name: "{{ secret_name }}" state: present @@ -33,80 +98,476 @@ secret: "{{ super_secret_string }}" register: result + - name: assert correct keys are returned + assert: + that: + - not result.changed + - '"arn" in result.secret' + - '"created_date" in result.secret' + - '"last_accessed_date" in result.secret' + - '"last_changed_date" in result.secret' + - '"name" in result.secret' + - '"version_ids_to_stages" in result.secret' + - result.secret.arn == secret_arn + - result.secret.name == secret_name + - result.secret.version_ids_to_stages | length == 1 + + - name: Set secret description + aws_secret: + name: "{{ secret_name }}" + description: 'this is a change to this secret' + state: present + secret_type: 'string' + secret: "{{ super_secret_string }}" + register: result + - name: assert correct keys are returned assert: that: - result.changed - - result.arn is not none - - result.name is not none - - result.tags is not none - - result.version_ids_to_stages is not none + - '"arn" in result.secret' + - '"created_date" in result.secret' + - '"description" in result.secret' + - '"last_accessed_date" in result.secret' + - '"last_changed_date" in result.secret' + - '"name" in result.secret' + - '"version_ids_to_stages" in result.secret' + - result.secret.description == "this is a change to this secret" + - result.secret.arn == secret_arn + - result.secret.name == secret_name + - result.secret.version_ids_to_stages | length == 2 - - name: no changes to secret + ############################################################### + # Tagging + ############################################################### + + - name: Set tags (CHECK_MODE) aws_secret: name: "{{ secret_name }}" + description: 'this is a change to this secret' state: present secret_type: 'string' secret: "{{ super_secret_string }}" + tags: "{{ first_tags }}" + register: result + check_mode: True + + - name: assert changed + assert: + that: + - result is changed + + - name: Set tags + aws_secret: + name: "{{ secret_name }}" + description: 'this is a change to this secret' + state: present + secret_type: 'string' + secret: "{{ super_secret_string }}" + tags: "{{ first_tags }}" register: result - name: assert correct keys are returned assert: that: - - not result.changed - - result.arn is not none + - result is changed + - '"arn" in result.secret' + - '"created_date" in result.secret' + - '"description" in result.secret' + - '"last_accessed_date" in result.secret' + - '"last_changed_date" in result.secret' + - '"name" in result.secret' + - '"tags" in result.secret' + - '"tags_dict" in result.secret' + - '"version_ids_to_stages" in result.secret' + - result.secret.description == "this is a change to this secret" + - result.secret.arn == secret_arn + - result.secret.name == secret_name + - result.secret.tags | length == 4 + - result.secret.tags_dict == first_tags + - result.secret.version_ids_to_stages | length == 2 - - name: make change to secret + - name: Set tags - idempotency (CHECK_MODE) aws_secret: name: "{{ secret_name }}" description: 'this is a change to this secret' state: present secret_type: 'string' secret: "{{ super_secret_string }}" + tags: "{{ first_tags }}" register: result + check_mode: True + + - name: assert changed + assert: + that: + - result is not changed - - debug: - var: result + - name: Set tags - idempotency + aws_secret: + name: "{{ secret_name }}" + description: 'this is a change to this secret' + state: present + secret_type: 'string' + secret: "{{ super_secret_string }}" + tags: "{{ first_tags }}" + register: result - name: assert correct keys are returned assert: that: - - result.changed - - result.arn is not none - - result.name is not none - - result.tags is not none - - result.version_ids_to_stages is not none + - result is not changed + - '"arn" in result.secret' + - '"created_date" in result.secret' + - '"description" in result.secret' + - '"last_accessed_date" in result.secret' + - '"last_changed_date" in result.secret' + - '"name" in result.secret' + - '"tags" in result.secret' + - '"tags_dict" in result.secret' + - '"version_ids_to_stages" in result.secret' + - result.secret.description == "this is a change to this secret" + - result.secret.arn == secret_arn + - result.secret.name == secret_name + - result.secret.tags | length == 4 + - result.secret.tags_dict == first_tags + - result.secret.version_ids_to_stages | length == 2 + + ### + + - name: Update tags with purge (CHECK_MODE) + aws_secret: + name: "{{ secret_name }}" + description: 'this is a change to this secret' + state: present + secret_type: 'string' + secret: "{{ super_secret_string }}" + tags: "{{ second_tags }}" + register: result + check_mode: True + + - name: assert changed + assert: + that: + - result is changed - - name: add tags to secret + - name: Update tags with purge aws_secret: name: "{{ secret_name }}" description: 'this is a change to this secret' state: present secret_type: 'string' secret: "{{ super_secret_string }}" - tags: - Foo: 'Bar' - Test: 'Tag' + tags: "{{ second_tags }}" register: result - name: assert correct keys are returned assert: that: - - result.changed + - result is changed + - '"arn" in result.secret' + - '"created_date" in result.secret' + - '"description" in result.secret' + - '"last_accessed_date" in result.secret' + - '"last_changed_date" in result.secret' + - '"name" in result.secret' + - '"tags" in result.secret' + - '"tags_dict" in result.secret' + - '"version_ids_to_stages" in result.secret' + - result.secret.description == "this is a change to this secret" + - result.secret.arn == secret_arn + - result.secret.name == secret_name + - result.secret.tags | length == 4 + - result.secret.tags_dict == second_tags + - result.secret.version_ids_to_stages | length == 2 - - name: remove tags from secret + - name: Update tags with purge - idempotency (CHECK_MODE) aws_secret: name: "{{ secret_name }}" description: 'this is a change to this secret' state: present secret_type: 'string' secret: "{{ super_secret_string }}" + tags: "{{ second_tags }}" + register: result + check_mode: True + + - name: assert changed + assert: + that: + - result is not changed + + - name: Update tags with purge - idempotency + aws_secret: + name: "{{ secret_name }}" + description: 'this is a change to this secret' + state: present + secret_type: 'string' + secret: "{{ super_secret_string }}" + tags: "{{ second_tags }}" register: result - name: assert correct keys are returned assert: that: - - result.changed + - result is not changed + - '"arn" in result.secret' + - '"created_date" in result.secret' + - '"description" in result.secret' + - '"last_accessed_date" in result.secret' + - '"last_changed_date" in result.secret' + - '"name" in result.secret' + - '"tags" in result.secret' + - '"tags_dict" in result.secret' + - '"version_ids_to_stages" in result.secret' + - result.secret.description == "this is a change to this secret" + - result.secret.arn == secret_arn + - result.secret.name == secret_name + - result.secret.tags | length == 4 + - result.secret.tags_dict == second_tags + - result.secret.version_ids_to_stages | length == 2 + + ### + + - name: Update tags without purge (CHECK_MODE) + aws_secret: + name: "{{ secret_name }}" + description: 'this is a change to this secret' + state: present + secret_type: 'string' + secret: "{{ super_secret_string }}" + tags: "{{ third_tags }}" + purge_tags: false + register: result + check_mode: True + + - name: assert changed + assert: + that: + - result is changed + + - name: Update tags without purge + aws_secret: + name: "{{ secret_name }}" + description: 'this is a change to this secret' + state: present + secret_type: 'string' + secret: "{{ super_secret_string }}" + tags: "{{ third_tags }}" + purge_tags: false + register: result + + - name: assert correct keys are returned + assert: + that: + - result is changed + - '"arn" in result.secret' + - '"created_date" in result.secret' + - '"description" in result.secret' + - '"last_accessed_date" in result.secret' + - '"last_changed_date" in result.secret' + - '"name" in result.secret' + - '"tags" in result.secret' + - '"tags_dict" in result.secret' + - '"version_ids_to_stages" in result.secret' + - result.secret.description == "this is a change to this secret" + - result.secret.arn == secret_arn + - result.secret.name == secret_name + - result.secret.tags | length == 8 + - result.secret.tags_dict == final_tags + - result.secret.version_ids_to_stages | length == 2 + + - name: Update tags without purge - idempotency (CHECK_MODE) + aws_secret: + name: "{{ secret_name }}" + description: 'this is a change to this secret' + state: present + secret_type: 'string' + secret: "{{ super_secret_string }}" + tags: "{{ third_tags }}" + purge_tags: false + register: result + check_mode: True + + - name: assert changed + assert: + that: + - result is not changed + + - name: Update tags without purge - idempotency + aws_secret: + name: "{{ secret_name }}" + description: 'this is a change to this secret' + state: present + secret_type: 'string' + secret: "{{ super_secret_string }}" + tags: "{{ third_tags }}" + purge_tags: false + register: result + + - name: assert correct keys are returned + assert: + that: + - result is not changed + - '"arn" in result.secret' + - '"created_date" in result.secret' + - '"description" in result.secret' + - '"last_accessed_date" in result.secret' + - '"last_changed_date" in result.secret' + - '"name" in result.secret' + - '"tags" in result.secret' + - '"tags_dict" in result.secret' + - '"version_ids_to_stages" in result.secret' + - result.secret.description == "this is a change to this secret" + - result.secret.arn == secret_arn + - result.secret.name == secret_name + - result.secret.tags | length == 8 + - result.secret.tags_dict == final_tags + - result.secret.version_ids_to_stages | length == 2 + + ### + + - name: Tags not set - idempotency (CHECK_MODE) + aws_secret: + name: "{{ secret_name }}" + description: 'this is a change to this secret' + state: present + secret_type: 'string' + secret: "{{ super_secret_string }}" + register: result + check_mode: True + + - name: assert changed + assert: + that: + - result is not changed + + - name: Tags not set - idempotency + aws_secret: + name: "{{ secret_name }}" + description: 'this is a change to this secret' + state: present + secret_type: 'string' + secret: "{{ super_secret_string }}" + register: result + + - name: assert correct keys are returned + assert: + that: + - result is not changed + - '"arn" in result.secret' + - '"created_date" in result.secret' + - '"description" in result.secret' + - '"last_accessed_date" in result.secret' + - '"last_changed_date" in result.secret' + - '"name" in result.secret' + - '"tags" in result.secret' + - '"tags_dict" in result.secret' + - '"version_ids_to_stages" in result.secret' + - result.secret.description == "this is a change to this secret" + - result.secret.arn == secret_arn + - result.secret.name == secret_name + - result.secret.tags | length == 8 + - result.secret.tags_dict == final_tags + - result.secret.version_ids_to_stages | length == 2 + + ### + + - name: remove all tags from secret (CHECK_MODE) + aws_secret: + name: "{{ secret_name }}" + description: 'this is a change to this secret' + state: present + secret_type: 'string' + secret: "{{ super_secret_string }}" + tags: {} + register: result + check_mode: true + + - name: assert correct keys are returned + assert: + that: + - result is changed + + - name: remove all tags from secret + aws_secret: + name: "{{ secret_name }}" + description: 'this is a change to this secret' + state: present + secret_type: 'string' + secret: "{{ super_secret_string }}" + tags: {} + register: result + + - name: assert correct keys are returned + assert: + that: + - result is changed + - '"arn" in result.secret' + - '"created_date" in result.secret' + - '"description" in result.secret' + - '"last_accessed_date" in result.secret' + - '"last_changed_date" in result.secret' + - '"name" in result.secret' + - '"tags" in result.secret' + - '"tags_dict" in result.secret' + - '"version_ids_to_stages" in result.secret' + - result.secret.description == "this is a change to this secret" + - result.secret.arn == secret_arn + - result.secret.name == secret_name + - result.secret.tags | length == 0 + - result.secret.tags_dict | length == 0 + - result.secret.version_ids_to_stages | length == 2 + + - name: remove all tags from secret - idempotency (CHECK_MODE) + aws_secret: + name: "{{ secret_name }}" + description: 'this is a change to this secret' + state: present + secret_type: 'string' + secret: "{{ super_secret_string }}" + tags: {} + register: result + check_mode: true + + - name: assert correct keys are returned + assert: + that: + - result is not changed + + - name: remove all tags from secret + aws_secret: + name: "{{ secret_name }}" + description: 'this is a change to this secret' + state: present + secret_type: 'string' + secret: "{{ super_secret_string }}" + tags: {} + register: result + + - name: assert correct keys are returned - idempotency + assert: + that: + - result is not changed + - '"arn" in result.secret' + - '"created_date" in result.secret' + - '"description" in result.secret' + - '"last_accessed_date" in result.secret' + - '"last_changed_date" in result.secret' + - '"name" in result.secret' + - '"tags" in result.secret' + - '"tags_dict" in result.secret' + - '"version_ids_to_stages" in result.secret' + - result.secret.description == "this is a change to this secret" + - result.secret.arn == secret_arn + - result.secret.name == secret_name + - result.secret.tags | length == 0 + - result.secret.tags_dict | length == 0 + - result.secret.version_ids_to_stages | length == 2 + + ############################################################### + # Resource policy + ############################################################### - name: add resource policy to secret aws_secret: @@ -151,6 +612,10 @@ that: - not result.changed + # ============================================================ + # Removal testing + # ============================================================ + - name: remove secret aws_secret: name: "{{ secret_name }}" diff --git a/tests/integration/targets/aws_secret/templates/secret-policy.j2 b/tests/integration/targets/aws_secret/templates/secret-policy.j2 index 77438091b25..8b084e16676 100644 --- a/tests/integration/targets/aws_secret/templates/secret-policy.j2 +++ b/tests/integration/targets/aws_secret/templates/secret-policy.j2 @@ -3,9 +3,9 @@ "Statement" : [ { "Effect" : "Allow", "Principal" : { - "AWS" : "arn:aws:iam::{{ aws_caller_info.account }}:root" + "AWS" : "arn:aws:iam::{{ caller_info.account }}:root" }, "Action" : "secretsmanager:*", "Resource" : "*" } ] -} \ No newline at end of file +}