From 045632f8b1187be817a67bd475cf6f417caca5df Mon Sep 17 00:00:00 2001 From: Helen Bailey Date: Mon, 12 Jun 2023 13:17:05 -0400 Subject: [PATCH 1/4] Use correct boto3 client call when updating vault tags --- plugins/modules/backup_vault.py | 8 ++----- .../targets/backup_vault/tasks/main.yml | 23 +++++++++++++++++-- 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/plugins/modules/backup_vault.py b/plugins/modules/backup_vault.py index 8d7452431e..32e91b1b2c 100644 --- a/plugins/modules/backup_vault.py +++ b/plugins/modules/backup_vault.py @@ -93,7 +93,6 @@ from ansible_collections.amazon.aws.plugins.module_utils.tagging import compare_aws_tags -from ansible_collections.amazon.aws.plugins.module_utils.tagging import ansible_dict_to_boto3_tag_list from ansible_collections.amazon.aws.plugins.module_utils.modules import AnsibleAWSModule from ansible_collections.amazon.aws.plugins.module_utils.botocore import is_boto3_error_code from ansible.module_utils.common.dict_transformations import camel_dict_to_snake_dict @@ -150,17 +149,14 @@ def tag_vault(module, client, tags, vault_arn, curr_tags=None, purge_tags=True): return True if tags_to_remove: - remove = {k: curr_tags[k] for k in tags_to_remove} - tags_to_remove = ansible_dict_to_boto3_tag_list(remove) try: - client.remove_tags(ResourceId=vault_arn, TagsList=tags_to_remove) + client.untag_resource(ResourceArn=vault_arn, Tags=tags_to_remove) except (BotoCoreError, ClientError) as err: module.fail_json_aws(err, msg="Failed to remove tags from the vault") if tags_to_add: - tags_to_add = ansible_dict_to_boto3_tag_list(tags_to_add) try: - client.add_tags(ResourceId=vault_arn, TagsList=tags_to_add) + client.tag_resource(ResourceArn=vault_arn, Tags=tags_to_add) except (BotoCoreError, ClientError) as err: module.fail_json_aws(err, msg="Failed to add tags to Vault") diff --git a/tests/integration/targets/backup_vault/tasks/main.yml b/tests/integration/targets/backup_vault/tasks/main.yml index 9397c60afc..78b0b85cb2 100644 --- a/tests/integration/targets/backup_vault/tasks/main.yml +++ b/tests/integration/targets/backup_vault/tasks/main.yml @@ -28,6 +28,7 @@ - backup_vault_result_check is changed - backup_vault_result_check.vault.backup_vault_name == backup_vault_name - backup_vault_result_check.vault.encryption_key_arn == "" + - backup_vault_result_check.vault.tags.environment == "dev" - name: Create an AWS Backup Vault amazon.aws.backup_vault: @@ -42,7 +43,8 @@ - backup_vault_result is changed - backup_vault_result.vault.backup_vault_name == backup_vault_name - backup_vault_result.vault.encryption_key_arn == key.key_arn - + - backup_vault_result.vault.tags.environment == "dev" + - name: Get backup vault info by passing the vault name amazon.aws.backup_vault_info: backup_vault_names: @@ -53,6 +55,7 @@ that: - vault_info.backup_vaults[0].backup_vault_name == backup_vault_result.vault.backup_vault_name - vault_info.backup_vaults[0].backup_vault_arn == backup_vault_result.vault.backup_vault_arn + - vault_info.backup_vaults[0].tags.environment == "dev" - name: Create an AWS Backup Vault - idempotency check amazon.aws.backup_vault: @@ -67,7 +70,23 @@ - backup_vault_result_idem is not changed - backup_vault_result_idem.vault.backup_vault_name == backup_vault_name - backup_vault_result_idem.vault.encryption_key_arn == key.key_arn - + - backup_vault_result_idem.vault.tags.environment == "dev" + + - name: Update AWS Backup Vault + amazon.aws.backup_vault: + backup_vault_name: "{{ backup_vault_name }}" + encryption_key_arn: "{{ key.key_arn }}" + tags: + environment: test + register: backup_vault_update_result + + - assert: + that: + - backup_vault_update_result is changed + - backup_vault_update_result.vault.backup_vault_name == backup_vault_name + - backup_vault_update_result.vault.encryption_key_arn == key.key_arn + - backup_vault_update_result.vault.tags.environment == "test" + always: - name: Delete AWS Backup Vault created during this test amazon.aws.backup_vault: From d5c9ef04234c5e0acbc454f332a18ed378e6fd74 Mon Sep 17 00:00:00 2001 From: Helen Bailey Date: Mon, 12 Jun 2023 14:00:51 -0400 Subject: [PATCH 2/4] Add changelog fragment --- changelogs/fragments/20230612-backup_vault-fix-tag-update.yml | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelogs/fragments/20230612-backup_vault-fix-tag-update.yml diff --git a/changelogs/fragments/20230612-backup_vault-fix-tag-update.yml b/changelogs/fragments/20230612-backup_vault-fix-tag-update.yml new file mode 100644 index 0000000000..f17bdd3b29 --- /dev/null +++ b/changelogs/fragments/20230612-backup_vault-fix-tag-update.yml @@ -0,0 +1,3 @@ +--- +bugfixes: + - backup_vault - fix error when updating tags on a backup vault by using the correct boto3 client methods for tagging and untagging backup resources. From 5f6d529f9cb3399b83f5970f0b63ffd093b52b4c Mon Sep 17 00:00:00 2001 From: Helen Bailey Date: Tue, 13 Jun 2023 10:19:28 -0400 Subject: [PATCH 3/4] Update changelogs/fragments/20230612-backup_vault-fix-tag-update.yml Co-authored-by: Alina Buzachis --- changelogs/fragments/20230612-backup_vault-fix-tag-update.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelogs/fragments/20230612-backup_vault-fix-tag-update.yml b/changelogs/fragments/20230612-backup_vault-fix-tag-update.yml index f17bdd3b29..7ed9c45ca0 100644 --- a/changelogs/fragments/20230612-backup_vault-fix-tag-update.yml +++ b/changelogs/fragments/20230612-backup_vault-fix-tag-update.yml @@ -1,3 +1,3 @@ --- bugfixes: - - backup_vault - fix error when updating tags on a backup vault by using the correct boto3 client methods for tagging and untagging backup resources. + - backup_vault - fix error when updating tags on a backup vault by using the correct boto3 client methods for tagging and untagging backup resources (https://github.com/ansible-collections/amazon.aws/pull/1610). From 512ee9c39ecda2ef099e9def5ff0529f4709b6fa Mon Sep 17 00:00:00 2001 From: Helen Bailey Date: Mon, 26 Jun 2023 14:15:06 -0400 Subject: [PATCH 4/4] Add tests and fix remaining bug --- plugins/modules/backup_vault.py | 2 +- .../targets/backup_vault/tasks/main.yml | 165 +++++++++++++++++- 2 files changed, 162 insertions(+), 5 deletions(-) diff --git a/plugins/modules/backup_vault.py b/plugins/modules/backup_vault.py index 32e91b1b2c..5ae309d105 100644 --- a/plugins/modules/backup_vault.py +++ b/plugins/modules/backup_vault.py @@ -150,7 +150,7 @@ def tag_vault(module, client, tags, vault_arn, curr_tags=None, purge_tags=True): if tags_to_remove: try: - client.untag_resource(ResourceArn=vault_arn, Tags=tags_to_remove) + client.untag_resource(ResourceArn=vault_arn, TagKeyList=tags_to_remove) except (BotoCoreError, ClientError) as err: module.fail_json_aws(err, msg="Failed to remove tags from the vault") diff --git a/tests/integration/targets/backup_vault/tasks/main.yml b/tests/integration/targets/backup_vault/tasks/main.yml index 78b0b85cb2..24f27b7d59 100644 --- a/tests/integration/targets/backup_vault/tasks/main.yml +++ b/tests/integration/targets/backup_vault/tasks/main.yml @@ -72,20 +72,177 @@ - backup_vault_result_idem.vault.encryption_key_arn == key.key_arn - backup_vault_result_idem.vault.tags.environment == "dev" + - name: Update AWS Backup Vault - check mode + amazon.aws.backup_vault: + backup_vault_name: "{{ backup_vault_name }}" + tags: + owner: ansible + purge_tags: false + check_mode: true + register: backup_vault_update_check_mode_result + + - name: Verify check mode update result + assert: + that: + - backup_vault_update_check_mode_result is changed + - backup_vault_update_check_mode_result.vault.backup_vault_name == backup_vault_name + - backup_vault_update_check_mode_result.vault.encryption_key_arn == key.key_arn + - backup_vault_update_check_mode_result.vault.tags.environment == "dev" + - backup_vault_update_check_mode_result.vault.tags.owner == "ansible" + + - name: Get backup vault info + amazon.aws.backup_vault_info: + backup_vault_names: + - "{{ backup_vault_name }}" + register: update_check_mode_vault_info + + - name: Verify backup vault was not updated in check mode + ansible.builtin.assert: + that: + - update_check_mode_vault_info.backup_vaults[0].backup_vault_name == vault_info.backup_vaults[0].backup_vault_name + - update_check_mode_vault_info.backup_vaults[0].encryption_key_arn == vault_info.backup_vaults[0].encryption_key_arn + - update_check_mode_vault_info.backup_vaults[0].backup_vault_arn == vault_info.backup_vaults[0].backup_vault_arn + - update_check_mode_vault_info.backup_vaults[0].tags == vault_info.backup_vaults[0].tags + - name: Update AWS Backup Vault amazon.aws.backup_vault: backup_vault_name: "{{ backup_vault_name }}" - encryption_key_arn: "{{ key.key_arn }}" tags: - environment: test + owner: ansible + purge_tags: false register: backup_vault_update_result - - assert: + - name: Verify update result + ansible.builtin.assert: that: - backup_vault_update_result is changed - backup_vault_update_result.vault.backup_vault_name == backup_vault_name - backup_vault_update_result.vault.encryption_key_arn == key.key_arn - - backup_vault_update_result.vault.tags.environment == "test" + - backup_vault_update_result.vault.tags.environment == "dev" + - backup_vault_update_check_mode_result.vault.tags.owner == "ansible" + + - name: Get updated backup vault info + amazon.aws.backup_vault_info: + backup_vault_names: + - "{{ backup_vault_name }}" + register: updated_vault_info + + - name: Verify backup vault was updated + ansible.builtin.assert: + that: + - updated_vault_info.backup_vaults[0].backup_vault_name == vault_info.backup_vaults[0].backup_vault_name + - updated_vault_info.backup_vaults[0].backup_vault_arn == vault_info.backup_vaults[0].backup_vault_arn + - updated_vault_info.backup_vaults[0].encryption_key_arn == vault_info.backup_vaults[0].encryption_key_arn + - updated_vault_info.backup_vaults[0].tags != vault_info.backup_vaults[0].tags + + - name: Update AWS Backup Vault - idempotency + amazon.aws.backup_vault: + backup_vault_name: "{{ backup_vault_name }}" + tags: + owner: ansible + purge_tags: false + register: backup_vault_update_idempotency_result + + - name: Verify idempotency update result + ansible.builtin.assert: + that: + - backup_vault_update_idempotency_result is not changed + + - name: Get backup vault info + amazon.aws.backup_vault_info: + backup_vault_names: + - "{{ backup_vault_name }}" + register: updated_vault_info_idempotency + + - name: Verify backup vault was not updated + ansible.builtin.assert: + that: + - updated_vault_info_idempotency.backup_vaults[0].backup_vault_name == updated_vault_info.backup_vaults[0].backup_vault_name + - updated_vault_info_idempotency.backup_vaults[0].backup_vault_arn == updated_vault_info.backup_vaults[0].backup_vault_arn + - updated_vault_info_idempotency.backup_vaults[0].encryption_key_arn == updated_vault_info.backup_vaults[0].encryption_key_arn + - updated_vault_info_idempotency.backup_vaults[0].tags == updated_vault_info.backup_vaults[0].tags + + - name: Update tags with purge - check mode + amazon.aws.backup_vault: + backup_vault_name: "{{ backup_vault_name }}" + tags: + environment: test + purge_tags: true + check_mode: true + register: backup_vault_update_tags_check_mode_result + + - name: Verify check mode tag update result + ansible.builtin.assert: + that: + - backup_vault_update_tags_check_mode_result is changed + - backup_vault_update_tags_check_mode_result.vault.backup_vault_name == backup_vault_name + - backup_vault_update_tags_check_mode_result.vault.tags | length == 1 + - backup_vault_update_tags_check_mode_result.vault.tags.environment == "test" + + - name: Get backup vault info + amazon.aws.backup_vault_info: + backup_vault_names: + - "{{ backup_vault_name }}" + register: update_tags_check_mode_info + + - name: Verify backup vault tags were not updated in check mode + ansible.builtin.assert: + that: + - update_tags_check_mode_info.backup_vaults[0].backup_vault_name == updated_vault_info.backup_vaults[0].backup_vault_name + - update_tags_check_mode_info.backup_vaults[0].tags == updated_vault_info.backup_vaults[0].tags + + - name: Update tags with purge + amazon.aws.backup_vault: + backup_vault_name: "{{ backup_vault_name }}" + tags: + environment: test + purge_tags: true + register: backup_vault_update_tags_result + + - name: Verify update tags with purge result + ansible.builtin.assert: + that: + - backup_vault_update_tags_result is changed + - backup_vault_update_tags_result.vault.backup_vault_name == backup_vault_name + - backup_vault_update_tags_result.vault.tags | length == 1 + - backup_vault_update_tags_result.vault.tags.environment == "test" + + - name: Get backup vault info + amazon.aws.backup_vault_info: + backup_vault_names: + - "{{ backup_vault_name }}" + register: updated_tags_info + + - name: Verify backup vault tags were updated + ansible.builtin.assert: + that: + - updated_tags_info.backup_vaults[0].backup_vault_name == updated_vault_info.backup_vaults[0].backup_vault_name + - updated_tags_info.backup_vaults[0].tags != updated_vault_info.backup_vaults[0].tags + + - name: Update tags with purge - idempotency + amazon.aws.backup_vault: + backup_vault_name: "{{ backup_vault_name }}" + tags: + environment: test + purge_tags: true + register: backup_vault_update_tags_idempotency_result + + - name: Verify update tags with purge idempotency result + ansible.builtin.assert: + that: + - backup_vault_update_tags_idempotency_result is not changed + + - name: Get backup vault info + amazon.aws.backup_vault_info: + backup_vault_names: + - "{{ backup_vault_name }}" + register: updated_tags_idempotency_info + + - name: Verify no changes were made + ansible.builtin.assert: + that: + - updated_tags_idempotency_info.backup_vaults[0].backup_vault_name == updated_tags_info.backup_vaults[0].backup_vault_name + - updated_tags_idempotency_info.backup_vaults[0].tags == updated_tags_info.backup_vaults[0].tags always: - name: Delete AWS Backup Vault created during this test