From a4ab7202e45d2425f368b0f0a18d91d2fbc99088 Mon Sep 17 00:00:00 2001 From: GomathiselviS Date: Tue, 6 Dec 2022 10:23:05 -0500 Subject: [PATCH] kms_key: Add multi region support to create_key (#1290) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit kms_key: Add multi region support to create_key Signed-off-by: GomathiselviS gomathiselvi@gmail.com SUMMARY Fixes #1281 ISSUE TYPE Feature Pull Request COMPONENT NAME ADDITIONAL INFORMATION Reviewed-by: Gonéri Le Bouder Reviewed-by: Jill R Reviewed-by: Mark Chappell Reviewed-by: Alina Buzachis Reviewed-by: GomathiselviS --- .../1290-create_multi_region_key.yml | 2 + plugins/modules/kms_key.py | 128 ++++++++++++++---- tests/integration/targets/kms_key/inventory | 2 + .../roles/aws_kms/tasks/test_multi_region.yml | 100 ++++++++++++++ .../roles/aws_kms/tasks/test_states.yml | 2 + tests/unit/plugins/modules/test_kms_key.py | 82 +++++++++++ 6 files changed, 291 insertions(+), 25 deletions(-) create mode 100644 changelogs/fragments/1290-create_multi_region_key.yml create mode 100644 tests/integration/targets/kms_key/roles/aws_kms/tasks/test_multi_region.yml create mode 100644 tests/unit/plugins/modules/test_kms_key.py diff --git a/changelogs/fragments/1290-create_multi_region_key.yml b/changelogs/fragments/1290-create_multi_region_key.yml new file mode 100644 index 00000000000..2ab78917dc9 --- /dev/null +++ b/changelogs/fragments/1290-create_multi_region_key.yml @@ -0,0 +1,2 @@ +minor_changes: +- kms_key - Add multi_region option to create_key (https://github.com/ansible-collections/amazon.aws/pull/1290). diff --git a/plugins/modules/kms_key.py b/plugins/modules/kms_key.py index 62b558d4bfc..b7ef976c8ee 100644 --- a/plugins/modules/kms_key.py +++ b/plugins/modules/kms_key.py @@ -60,6 +60,12 @@ - A description of the CMK. - Use a description that helps you decide whether the CMK is appropriate for a task. type: str + multi_region: + description: + - Whether to create a multi-Region primary key or not. + default: False + type: bool + version_added: 5.2.0 pending_window: description: - The number of days between requesting deletion of the CMK and when it will actually be deleted. @@ -158,6 +164,14 @@ Name: myKey Purpose: protect_stuff +# Create a new multi-region KMS key +- amazon.aws.kms_key: + alias: mykey + multi_region: true + tags: + Name: myKey + Purpose: protect_stuff + # Update previous key with more tags - amazon.aws.kms_key: alias: mykey @@ -409,6 +423,16 @@ description: Whether there are invalid (non-ARN) entries in the KMS entry. These don't count as a change, but will be removed if any changes are being made. type: bool returned: always +multi_region: + description: + - Indicates whether the CMK is a multi-Region C(True) or regional C(False) key. + - This value is True for multi-Region primary and replica CMKs and False for regional CMKs. + type: bool + version_added: 5.2.0 + returned: always + sample: False + + ''' # these mappings are used to go from simple labels to the actual 'Sid' values returned @@ -519,8 +543,10 @@ def get_kms_tags(connection, module, key_id): def get_kms_policies(connection, module, key_id): try: policies = list_key_policies_with_backoff(connection, key_id)['PolicyNames'] - return [get_key_policy_with_backoff(connection, key_id, policy)['Policy'] for - policy in policies] + return [ + get_key_policy_with_backoff(connection, key_id, policy)['Policy'] + for policy in policies + ] except is_boto3_error_code('AccessDeniedException'): return [] except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: # pylint: disable=duplicate-except @@ -528,7 +554,7 @@ def get_kms_policies(connection, module, key_id): def camel_to_snake_grant(grant): - ''' camel_to_snake_grant snakifies everything except the encryption context ''' + '''camel_to_snake_grant snakifies everything except the encryption context ''' constraints = grant.get('Constraints', {}) result = camel_dict_to_snake_dict(grant) if 'EncryptionContextEquals' in constraints: @@ -561,8 +587,10 @@ def get_key_details(connection, module, key_id): # grants and tags get snakified differently try: - result['grants'] = [camel_to_snake_grant(grant) for grant in - get_kms_grants_with_backoff(connection, key_id)['Grants']] + result['grants'] = [ + camel_to_snake_grant(grant) + for grant in get_kms_grants_with_backoff(connection, key_id)['Grants'] + ] except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: module.fail_json_aws(e, msg="Failed to obtain key grants") tags = get_kms_tags(connection, module, key_id) @@ -582,8 +610,9 @@ def get_kms_facts(connection, module): def convert_grant_params(grant, key): - grant_params = dict(KeyId=key['key_arn'], - GranteePrincipal=grant['grantee_principal']) + grant_params = dict( + KeyId=key['key_arn'], GranteePrincipal=grant['grantee_principal'] + ) if grant.get('operations'): grant_params['Operations'] = grant['operations'] if grant.get('retiring_principal'): @@ -751,7 +780,11 @@ def update_tags(connection, module, key, desired_tags, purge_tags): module.fail_json_aws(e, msg="Unable to remove tag") if to_add: try: - tags = ansible_dict_to_boto3_tag_list(module.params['tags'], tag_name_key_name='TagKey', tag_value_key_name='TagValue') + tags = ansible_dict_to_boto3_tag_list( + module.params['tags'], + tag_name_key_name='TagKey', + tag_value_key_name='TagValue', + ) connection.tag_resource(KeyId=key_id, Tags=tags) except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: module.fail_json_aws(e, msg="Unable to add tag to key") @@ -859,16 +892,21 @@ def update_key(connection, module, key): def create_key(connection, module): key_usage = module.params.get('key_usage') key_spec = module.params.get('key_spec') + multi_region = module.params.get('multi_region') tags_list = ansible_dict_to_boto3_tag_list( module.params['tags'] or {}, - # KMS doesn't use "Key" and "Value" as other APIs do. - tag_name_key_name='TagKey', tag_value_key_name='TagValue' + # KMS doesn't use 'Key' and 'Value' as other APIs do. + tag_name_key_name='TagKey', + tag_value_key_name='TagValue', + ) + params = dict( + BypassPolicyLockoutSafetyCheck=False, + Tags=tags_list, + KeyUsage=key_usage, + CustomerMasterKeySpec=key_spec, + Origin='AWS_KMS', + MultiRegion=multi_region, ) - params = dict(BypassPolicyLockoutSafetyCheck=False, - Tags=tags_list, - KeyUsage=key_usage, - CustomerMasterKeySpec=key_spec, - Origin='AWS_KMS') if module.check_mode: return {'changed': True} @@ -877,7 +915,6 @@ def create_key(connection, module): params['Description'] = module.params['description'] if module.params.get('policy'): params['Policy'] = module.params['policy'] - try: result = connection.create_key(**params)['KeyMetadata'] except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: @@ -940,7 +977,29 @@ def fetch_key_metadata(connection, module, key_id, alias): except connection.exceptions.NotFoundException: return None except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: - module.fail_json_aws(e, 'Failed to fetch key metadata.') + module.fail_json_aws(e, "Failed to fetch key metadata.") + + +def validate_params(module, key_metadata): + # We can't create keys with a specific ID, if we can't access the key we'll have to fail + if ( + module.params.get('state') == 'present' + and module.params.get('key_id') + and not key_metadata + ): + module.fail_json( + msg='Could not find key with id {0} to update'.format( + module.params.get('key_id') + ) + ) + if ( + module.params.get('multi_region') + and key_metadata + and module.params.get('state') == 'present' + ): + module.fail_json( + msg='You cannot change the multi-region property on an existing key.' + ) def main(): @@ -950,6 +1009,7 @@ def main(): key_id=dict(aliases=['key_arn']), description=dict(), enabled=dict(type='bool', default=True), + multi_region=dict(type='bool', default=False), tags=dict(type='dict', aliases=['resource_tags']), purge_tags=dict(type='bool', default=True), grants=dict(type='list', default=[], elements='dict'), @@ -957,9 +1017,26 @@ def main(): purge_grants=dict(type='bool', default=False), state=dict(default='present', choices=['present', 'absent']), enable_key_rotation=(dict(type='bool')), - key_spec=dict(type='str', default='SYMMETRIC_DEFAULT', aliases=['customer_master_key_spec'], - choices=['SYMMETRIC_DEFAULT', 'RSA_2048', 'RSA_3072', 'RSA_4096', 'ECC_NIST_P256', 'ECC_NIST_P384', 'ECC_NIST_P521', 'ECC_SECG_P256K1']), - key_usage=dict(type='str', default='ENCRYPT_DECRYPT', choices=['ENCRYPT_DECRYPT', 'SIGN_VERIFY']), + key_spec=dict( + type='str', + default='SYMMETRIC_DEFAULT', + aliases=['customer_master_key_spec'], + choices=[ + 'SYMMETRIC_DEFAULT', + 'RSA_2048', + 'RSA_3072', + 'RSA_4096', + 'ECC_NIST_P256', + 'ECC_NIST_P384', + 'ECC_NIST_P521', + 'ECC_SECG_P256K1', + ], + ), + key_usage=dict( + type='str', + default='ENCRYPT_DECRYPT', + choices=['ENCRYPT_DECRYPT', 'SIGN_VERIFY'], + ), ) module = AnsibleAWSModule( @@ -970,13 +1047,14 @@ def main(): kms = module.client('kms') - module.deprecate("The 'policies' return key is deprecated and will be replaced by 'key_policies'. Both values are returned for now.", - date='2024-05-01', collection_name='amazon.aws') + module.deprecate( + "The 'policies' return key is deprecated and will be replaced by 'key_policies'. Both values are returned for now.", + date='2024-05-01', + collection_name='amazon.aws', + ) key_metadata = fetch_key_metadata(kms, module, module.params.get('key_id'), module.params.get('alias')) - # We can't create keys with a specific ID, if we can't access the key we'll have to fail - if module.params.get('state') == 'present' and module.params.get('key_id') and not key_metadata: - module.fail_json(msg="Could not find key with id {0} to update".format(module.params.get('key_id'))) + validate_params(module, key_metadata) if module.params.get('state') == 'absent': if key_metadata is None: diff --git a/tests/integration/targets/kms_key/inventory b/tests/integration/targets/kms_key/inventory index 14abc6eb267..a9081eae9f1 100644 --- a/tests/integration/targets/kms_key/inventory +++ b/tests/integration/targets/kms_key/inventory @@ -4,6 +4,8 @@ states grants modify tagging +# CI's AWS account doesnot support multi region +# multi_region [all:vars] ansible_connection=local diff --git a/tests/integration/targets/kms_key/roles/aws_kms/tasks/test_multi_region.yml b/tests/integration/targets/kms_key/roles/aws_kms/tasks/test_multi_region.yml new file mode 100644 index 00000000000..c112b4571e0 --- /dev/null +++ b/tests/integration/targets/kms_key/roles/aws_kms/tasks/test_multi_region.yml @@ -0,0 +1,100 @@ +- block: + # ============================================================ + # PREPARATION + # + # Get some information about who we are before starting our tests + # we'll need this as soon as we start working on the policies + - name: get ARN of calling user + aws_caller_info: + register: aws_caller_info + - name: See whether key exists and its current state + kms_key_info: + alias: '{{ kms_key_alias }}' + - name: create a multi region key - check mode + kms_key: + alias: '{{ kms_key_alias }}-check' + tags: + Hello: World + state: present + multi_region: True + enabled: yes + register: key_check + check_mode: yes + - name: find facts about the check mode key + kms_key_info: + alias: '{{ kms_key_alias }}-check' + register: check_key + - name: ensure that check mode worked as expected + assert: + that: + - check_key.kms_keys | length == 0 + - key_check is changed + + - name: create a multi region key + kms_key: + alias: '{{ kms_key_alias }}' + tags: + Hello: World + state: present + enabled: yes + multi_region: True + enable_key_rotation: no + register: key + - name: assert that state is enabled + assert: + that: + - key is changed + - '"key_id" in key' + - key.key_id | length >= 36 + - not key.key_id.startswith("arn:aws") + - '"key_arn" in key' + - key.key_arn.endswith(key.key_id) + - key.key_arn.startswith("arn:aws") + - key.key_state == "Enabled" + - key.enabled == True + - key.tags | length == 1 + - key.tags['Hello'] == 'World' + - key.enable_key_rotation == false + - key.key_usage == 'ENCRYPT_DECRYPT' + - key.customer_master_key_spec == 'SYMMETRIC_DEFAULT' + - key.grants | length == 0 + - key.key_policies | length == 1 + - key.key_policies[0].Id == 'key-default-1' + - key.description == '' + - key.multi_region == True + + - name: Sleep to wait for updates to propagate + wait_for: + timeout: 45 + + - name: create a key (expect failure) + kms_key: + alias: '{{ kms_key_alias }}' + tags: + Hello: World + state: present + enabled: yes + multi_region: True + register: result + ignore_errors: True + + - assert: + that: + - result is failed + - result.msg != "MODULE FAILURE" + - result.changed == False + - '"You cannot change the multi-region property on an existing key." in result.msg' + + always: + # ============================================================ + # CLEAN-UP + - name: finish off by deleting keys + kms_key: + state: absent + alias: '{{ item }}' + pending_window: 7 + ignore_errors: true + loop: + - '{{ kms_key_alias }}' + - '{{ kms_key_alias }}-diff-spec-usage' + - '{{ kms_key_alias }}-check' diff --git a/tests/integration/targets/kms_key/roles/aws_kms/tasks/test_states.yml b/tests/integration/targets/kms_key/roles/aws_kms/tasks/test_states.yml index 98bf9932eeb..ae7ffffeff8 100644 --- a/tests/integration/targets/kms_key/roles/aws_kms/tasks/test_states.yml +++ b/tests/integration/targets/kms_key/roles/aws_kms/tasks/test_states.yml @@ -59,6 +59,7 @@ - key.key_policies | length == 1 - key.key_policies[0].Id == 'key-default-1' - key.description == '' + - key.multi_region == False - name: Sleep to wait for updates to propagate wait_for: @@ -105,6 +106,7 @@ - key.key_policies | length == 1 - key.key_policies[0].Id == 'key-default-1' - key.description == '' + - key.multi_region == False # ------------------------------------------------------------------------------------------ diff --git a/tests/unit/plugins/modules/test_kms_key.py b/tests/unit/plugins/modules/test_kms_key.py new file mode 100644 index 00000000000..5a53e2ddbcc --- /dev/null +++ b/tests/unit/plugins/modules/test_kms_key.py @@ -0,0 +1,82 @@ +# +# (c) 2022 Red Hat Inc. +# +# This file is part of Ansible +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +import pytest + +from unittest.mock import MagicMock, call, patch +from ansible_collections.amazon.aws.plugins.modules import kms_key + + +module_name = "ansible_collections.amazon.aws.plugins.modules.kms_key" +key_details = { + "KeyMetadata": { + "aliases": ["mykey"], + "Arn": "arn:aws:kms:us-east-1:12345678:key/mrk-12345678", + "customer_master_key_spec": "SYMMETRIC_DEFAULT", + "description": "", + "enable_key_rotation": False, + "enabled": True, + "encryption_algorithms": ["SYMMETRIC_DEFAULT"], + "grants": [], + "key_arn": "arn:aws:kms:us-east-1:12345678:key/mrk-12345678", + "key_id": "mrk-12345678", + "key_manager": "CUSTOMER", + "key_policies": [ + { + "Id": "key-default-1", + "Statement": [ + { + "Action": "kms:*", + "Effect": "Allow", + "Principal": {"AWS": "arn:aws:iam::12345678:root"}, + "Resource": "*", + "Sid": "Enable IAM User Permissions", + } + ], + "Version": "2012-10-17", + } + ], + "key_spec": "SYMMETRIC_DEFAULT", + "key_state": "Enabled", + "key_usage": "ENCRYPT_DECRYPT", + "multi_region": True, + "multi_region_configuration": { + "multi_region_key_type": "PRIM ARY", + "primary_key": { + "arn": "arn:aws:kms:us-east-1:12345678:key/mrk-12345678", + "region": "us-east-1", + }, + "replica_keys": [], + }, + "origin": "AWS_KMS", + "tags": {"Hello": "World2"}, + } +} + + +@patch(module_name + ".get_kms_metadata_with_backoff") +def test_fetch_key_metadata(m_get_kms_metadata_with_backoff): + + module = MagicMock() + kms_client = MagicMock() + + m_get_kms_metadata_with_backoff.return_value = key_details + kms_key.fetch_key_metadata(kms_client, module, "mrk-12345678", "mykey") + assert m_get_kms_metadata_with_backoff.call_count == 1 + + +def test_validate_params(): + + module = MagicMock() + module.params = { + "state": "present", + "multi_region": True + } + + result = kms_key.validate_params(module, key_details["KeyMetadata"]) + module.fail_json.assert_called_with( + msg="You cannot change the multi-region property on an existing key." + )