From 371222f5fab92b2d46e49ad1b51f2e5690557918 Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Wed, 11 Aug 2021 12:25:22 +0200 Subject: [PATCH] aws_secret - fix deletion idempotency when not using instant deletion --- .../681-aws_secret-deletion-idempotency.yml | 2 + plugins/modules/aws_secret.py | 3 + tests/integration/targets/aws_secret/aliases | 5 - .../targets/aws_secret/tasks/basic.yml | 160 ++++++++++++ .../targets/aws_secret/tasks/main.yaml | 245 +----------------- .../targets/aws_secret/tasks/rotation.yml | 191 ++++++++++++++ 6 files changed, 359 insertions(+), 247 deletions(-) create mode 100644 changelogs/fragments/681-aws_secret-deletion-idempotency.yml create mode 100644 tests/integration/targets/aws_secret/tasks/basic.yml create mode 100644 tests/integration/targets/aws_secret/tasks/rotation.yml diff --git a/changelogs/fragments/681-aws_secret-deletion-idempotency.yml b/changelogs/fragments/681-aws_secret-deletion-idempotency.yml new file mode 100644 index 00000000000..ac7910ef23b --- /dev/null +++ b/changelogs/fragments/681-aws_secret-deletion-idempotency.yml @@ -0,0 +1,2 @@ +bugfixes: +- aws_secret - fix deletion idempotency when not using instant deletion (https://github.com/ansible-collections/community.aws/pull/681). diff --git a/plugins/modules/aws_secret.py b/plugins/modules/aws_secret.py index 86c6d6e3521..dfe1013194d 100644 --- a/plugins/modules/aws_secret.py +++ b/plugins/modules/aws_secret.py @@ -367,6 +367,8 @@ def main(): elif current_secret.get("DeletedDate") and recovery_window == 0: result = camel_dict_to_snake_dict(secrets_mgr.delete_secret(secret.name, recovery_window=recovery_window)) changed = True + else: + result = "secret already scheduled for deletion" else: result = "secret does not exist" if state == 'present': @@ -393,6 +395,7 @@ def main(): changed = True result = camel_dict_to_snake_dict(secrets_mgr.get_secret(secret.name)) result.pop("response_metadata") + module.exit_json(changed=changed, secret=result) diff --git a/tests/integration/targets/aws_secret/aliases b/tests/integration/targets/aws_secret/aliases index da76731e85b..4ef4b2067d0 100644 --- a/tests/integration/targets/aws_secret/aliases +++ b/tests/integration/targets/aws_secret/aliases @@ -1,6 +1 @@ -# reason: missing-policy -# reason: broken -# The tests for configuring secret rotation seem to be missing a permission -disabled - cloud/aws diff --git a/tests/integration/targets/aws_secret/tasks/basic.yml b/tests/integration/targets/aws_secret/tasks/basic.yml new file mode 100644 index 00000000000..884fdc40d36 --- /dev/null +++ b/tests/integration/targets/aws_secret/tasks/basic.yml @@ -0,0 +1,160 @@ +--- +- block: + # ============================================================ + # Module parameter testing + # ============================================================ + - name: test with no parameters + aws_secret: + register: result + ignore_errors: true + check_mode: true + + - name: assert failure when called with no parameters + assert: + that: + - result.failed + - 'result.msg.startswith("missing required arguments:")' + + # ============================================================ + # Creation/Deletion testing + # ============================================================ + - name: add secret to AWS Secrets Manager + aws_secret: + name: "{{ secret_name }}" + 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 + + - name: no changes to secret + aws_secret: + name: "{{ secret_name }}" + state: present + secret_type: 'string' + secret: "{{ super_secret_string }}" + register: result + + - name: assert correct keys are returned + assert: + that: + - not result.changed + - result.arn is not none + + - name: make change to secret + aws_secret: + name: "{{ secret_name }}" + description: 'this is a change to this secret' + state: present + secret_type: 'string' + secret: "{{ super_secret_string }}" + register: result + + - debug: + var: 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 + + - name: add tags to secret + 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' + register: result + + - name: assert correct keys are returned + assert: + that: + - result.changed + + - name: remove 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 }}" + register: result + + - name: assert correct keys are returned + assert: + that: + - result.changed + + - name: remove secret + aws_secret: + name: "{{ secret_name }}" + state: absent + recovery_window: 7 + register: result + + - name: assert key is deleted + assert: + that: + - result.changed + + - name: remove secret (idempotency) + aws_secret: + name: "{{ secret_name }}" + state: absent + recovery_window: 7 + register: result + + - name: assert no change happened + assert: + that: + - not result.changed + + - name: immediate secret removal + aws_secret: + name: "{{ secret_name }}" + state: absent + recovery_window: 0 + register: result + + - name: assert key is deleted + assert: + that: + - result.changed + + # AWS Doesn't expose when the secret will be removed, all we can do is + # check that we didn't throw an error + - name: immediate secret removal + aws_secret: + name: "{{ secret_name }}" + state: absent + recovery_window: 0 + register: result + + - name: assert no change happened + assert: + that: + - not result.failed + + always: + - name: remove secret + aws_secret: + name: "{{ secret_name }}" + state: absent + recovery_window: 0 + ignore_errors: yes diff --git a/tests/integration/targets/aws_secret/tasks/main.yaml b/tests/integration/targets/aws_secret/tasks/main.yaml index 483be47553c..00a859b5d50 100644 --- a/tests/integration/targets/aws_secret/tasks/main.yaml +++ b/tests/integration/targets/aws_secret/tasks/main.yaml @@ -9,245 +9,6 @@ - amazon.aws block: - - name: retrieve caller facts - aws_caller_info: - register: test_caller_facts - - - name: ensure IAM role exists - iam_role: - name: "{{ secret_manager_role }}" - assume_role_policy_document: "{{ lookup('file','secretsmanager-trust-policy.json') }}" - state: present - create_instance_profile: no - managed_policy: - - 'arn:aws:iam::aws:policy/SecretsManagerReadWrite' - register: iam_role - ignore_errors: yes - - - name: wait 10 seconds for role to become available - pause: - seconds: 10 - when: iam_role.changed - - # CI does not remove the role and comparing policies has a bug on Python3; fall back to use iam_role_info - - name: get IAM role - iam_role_info: - name: "{{ secret_manager_role }}" - register: iam_role_info - - - name: set iam_role_output - set_fact: - iam_role_output: "{{ iam_role_info.iam_roles[0] }}" - when: iam_role_info is defined - - - name: create a temporary directory - tempfile: - state: directory - register: tmp - - - name: move lambda into place for upload - copy: - src: "files/hello_world.zip" - dest: "{{ tmp.path }}/hello_world.zip" - - - name: dummy lambda for testing - lambda: - name: "{{ lambda_name }}" - state: present - zip_file: "{{ tmp.path }}/hello_world.zip" - runtime: 'python2.7' - role: "{{ iam_role_output.arn }}" - handler: 'hello_world.lambda_handler' - register: lambda_output - until: not lambda_output.failed - retries: 10 - delay: 5 - - - debug: - var: lambda_output - - # ============================================================ - # Module parameter testing - # ============================================================ - - name: test with no parameters - aws_secret: - register: result - ignore_errors: true - check_mode: true - - - name: assert failure when called with no parameters - assert: - that: - - result.failed - - 'result.msg.startswith("missing required arguments:")' - - # ============================================================ - # Creation/Deletion testing - # ============================================================ - - name: add secret to AWS Secrets Manager - aws_secret: - name: "{{ secret_name }}" - 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 - - - name: no changes to secret - aws_secret: - name: "{{ secret_name }}" - state: present - secret_type: 'string' - secret: "{{ super_secret_string }}" - register: result - - - name: assert correct keys are returned - assert: - that: - - not result.changed - - result.arn is not none - - - name: make change to secret - aws_secret: - name: "{{ secret_name }}" - description: 'this is a change to this secret' - state: present - secret_type: 'string' - secret: "{{ super_secret_string }}" - register: result - - - debug: - var: 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 - - - name: add tags to secret - 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' - register: result - - - name: assert correct keys are returned - assert: - that: - - result.changed - - - name: remove 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 }}" - register: result - - - name: assert correct keys are returned - assert: - that: - - result.changed - - - name: lambda policy for secrets manager - lambda_policy: - state: present - function_name: "{{ lambda_name }}" - statement_id: LambdaSecretsManagerTestPolicy - action: 'lambda:InvokeFunction' - principal: "secretsmanager.amazonaws.com" - - - name: add rotation lambda to secret - aws_secret: - name: "{{ secret_name }}" - description: 'this is a change to this secret' - state: present - secret_type: 'string' - secret: "{{ super_secret_string }}" - rotation_lambda: "arn:aws:lambda:{{ aws_region }}:{{ test_caller_facts.account }}:function:{{ lambda_name }}" - register: result - retries: 100 - delay: 5 - until: not result.failed - - - name: assert correct keys are returned - assert: - that: - - result.changed - - - name: remove rotation lambda from secret - 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 - - always: - - name: remove secret - aws_secret: - name: "{{ secret_name }}" - state: absent - secret_type: 'string' - secret: "{{ super_secret_string }}" - recovery_window: 0 - ignore_errors: yes - - - name: remove lambda policy - lambda_policy: - state: absent - function_name: "{{ lambda_name }}" - statement_id: lambda-secretsmanager-test-policy - action: lambda:InvokeFunction - principal: secretsmanager.amazonaws.com - ignore_errors: yes - - - name: remove dummy lambda - lambda: - name: "{{ lambda_name }}" - state: absent - zip_file: "{{ tmp.path }}/hello_world.zip" - runtime: 'python2.7' - role: "{{ secret_manager_role }}" - handler: 'hello_world.lambda_handler' - ignore_errors: yes - - # CI does not remove the IAM role - - name: remove IAM role - iam_role: - name: "{{ secret_manager_role }}" - assume_role_policy_document: "{{ lookup('file','secretsmanager-trust-policy.json') }}" - state: absent - create_instance_profile: no - managed_policy: - - 'arn:aws:iam::aws:policy/SecretsManagerReadWrite' - ignore_errors: yes - - - name: remove temporary dir - file: - path: "{{ tmp.path }}" - state: absent + - include_tasks: 'basic.yml' + # Permissions missing + #- include_tasks: 'rotation.yml' diff --git a/tests/integration/targets/aws_secret/tasks/rotation.yml b/tests/integration/targets/aws_secret/tasks/rotation.yml new file mode 100644 index 00000000000..823696dbcfc --- /dev/null +++ b/tests/integration/targets/aws_secret/tasks/rotation.yml @@ -0,0 +1,191 @@ +--- +- module_defaults: + group/aws: + aws_access_key: "{{ aws_access_key }}" + aws_secret_key: "{{ aws_secret_key }}" + security_token: "{{ security_token | default(omit) }}" + region: "{{ aws_region }}" + collections: + - amazon.aws + + block: + - name: retrieve caller facts + aws_caller_info: + register: test_caller_facts + + - name: ensure IAM role exists + iam_role: + name: "{{ secret_manager_role }}" + assume_role_policy_document: "{{ lookup('file','secretsmanager-trust-policy.json') }}" + state: present + create_instance_profile: no + managed_policy: + - 'arn:aws:iam::aws:policy/SecretsManagerReadWrite' + register: iam_role + ignore_errors: yes + + - name: wait 10 seconds for role to become available + pause: + seconds: 10 + when: iam_role.changed + + # CI does not remove the role and comparing policies has a bug on Python3; fall back to use iam_role_info + - name: get IAM role + iam_role_info: + name: "{{ secret_manager_role }}" + register: iam_role_info + + - name: set iam_role_output + set_fact: + iam_role_output: "{{ iam_role_info.iam_roles[0] }}" + when: iam_role_info is defined + + - name: create a temporary directory + tempfile: + state: directory + register: tmp + + - name: move lambda into place for upload + copy: + src: "files/hello_world.zip" + dest: "{{ tmp.path }}/hello_world.zip" + + - name: dummy lambda for testing + lambda: + name: "{{ lambda_name }}" + state: present + zip_file: "{{ tmp.path }}/hello_world.zip" + runtime: 'python2.7' + role: "{{ iam_role_output.arn }}" + handler: 'hello_world.lambda_handler' + register: lambda_output + until: not lambda_output.failed + retries: 10 + delay: 5 + + - debug: + var: lambda_output + + # ============================================================ + # Creation/Deletion testing + # ============================================================ + - name: add secret to AWS Secrets Manager + aws_secret: + name: "{{ secret_name }}-rotate" + 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 + + - name: lambda policy for secrets manager + lambda_policy: + state: present + function_name: "{{ lambda_name }}" + statement_id: LambdaSecretsManagerTestPolicy + action: 'lambda:InvokeFunction' + principal: "secretsmanager.amazonaws.com" + + - name: add rotation lambda to secret + aws_secret: + name: "{{ secret_name }}-rotate" + description: 'this is a change to this secret' + state: present + secret_type: 'string' + secret: "{{ super_secret_string }}" + rotation_lambda: "arn:aws:lambda:{{ aws_region }}:{{ test_caller_facts.account }}:function:{{ lambda_name }}" + register: result + retries: 100 + delay: 5 + until: not result.failed + + - name: assert correct keys are returned + assert: + that: + - result.changed + + - name: remove rotation lambda from secret + aws_secret: + name: "{{ secret_name }}-rotate" + 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 + + - name: remove rotation lambda from secret + aws_secret: + name: "{{ secret_name }}-rotate" + 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: + - not result.changed + + - name: remove secret + aws_secret: + name: "{{ secret_name }}-rotate" + state: absent + recovery_window: 0 + ignore_errors: yes + + always: + - name: remove secret + aws_secret: + name: "{{ secret_name }}-rotate" + state: absent + recovery_window: 0 + ignore_errors: yes + + - name: remove lambda policy + lambda_policy: + state: absent + function_name: "{{ lambda_name }}" + statement_id: lambda-secretsmanager-test-policy + action: lambda:InvokeFunction + principal: secretsmanager.amazonaws.com + ignore_errors: yes + + - name: remove dummy lambda + lambda: + name: "{{ lambda_name }}" + state: absent + zip_file: "{{ tmp.path }}/hello_world.zip" + runtime: 'python2.7' + role: "{{ secret_manager_role }}" + handler: 'hello_world.lambda_handler' + ignore_errors: yes + + # CI does not remove the IAM role + - name: remove IAM role + iam_role: + name: "{{ secret_manager_role }}" + assume_role_policy_document: "{{ lookup('file','secretsmanager-trust-policy.json') }}" + state: absent + create_instance_profile: no + managed_policy: + - 'arn:aws:iam::aws:policy/SecretsManagerReadWrite' + ignore_errors: yes + + - name: remove temporary dir + file: + path: "{{ tmp.path }}" + state: absent