Skip to content

Commit

Permalink
aws_kms - stabilize and add integration tests (#1052)
Browse files Browse the repository at this point in the history
aws_kms - stabilize and add integration tests

SUMMARY

update/add integration tests for various actions
return list of policies as a list of jsons for clarity
sleep on updates (no kms waiter, attempted manual waiters but still had test failures)

ISSUE TYPE

Feature Pull Request

COMPONENT NAME
aws_kms
ADDITIONAL INFORMATION
I tried adding manual waiters for different actions like waiting for tags to be correct, policy to be updated, etc, but would still fail ~half of the time on idempotency tests. seems like after updating the key's status is a bit buggy.

Reviewed-by: Jill R <None>
Reviewed-by: Mark Chappell <None>
Reviewed-by: Joseph Torcasso <None>
Reviewed-by: Mandar Kulkarni <[email protected]>
Reviewed-by: Markus Bergholz <[email protected]>
Reviewed-by: Alina Buzachis <None>
  • Loading branch information
jatorcasso authored Apr 26, 2022
1 parent 8af7e0e commit 558a025
Show file tree
Hide file tree
Showing 7 changed files with 635 additions and 143 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
minor_changes:
- aws_kms - fix some bugs in integration tests and add check mode support for key rotation as well as document issues with time taken for requested changes to be reflected on AWS (https://github.com/ansible-collections/community.aws/pull/1052).
- aws_kms - add extra key/value pair to return data (key_policies) to return each policy as a dictionary rather than json string (https://github.com/ansible-collections/community.aws/pull/1052).
77 changes: 68 additions & 9 deletions plugins/modules/aws_kms.py
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,12 @@
- amazon.aws.aws
- amazon.aws.ec2
notes:
- There are known inconsistencies in the amount of time required for updates of KMS keys to be fully reflected on AWS.
This can cause issues when running duplicate tasks in succession or using the aws_kms_info module to fetch key metadata
shortly after modifying keys.
For this reason, it is recommended to use the return data from this module (aws_kms) to fetch a key's metadata.
'''

EXAMPLES = r'''
Expand Down Expand Up @@ -310,6 +316,11 @@
type: str
returned: always
sample: false
enable_key_rotation:
description: Whether the automatic annual key rotation is enabled. Returns None if key rotation status can't be determined.
type: bool
returned: always
sample: false
aliases:
description: list of aliases associated with the key
type: list
Expand All @@ -318,9 +329,45 @@
- aws/acm
- aws/ebs
policies:
description: list of policy documents for the keys. Empty when access is denied even if there are policies.
description: list of policy documents for the key. Empty when access is denied even if there are policies.
type: list
returned: always
elements: str
sample:
Version: "2012-10-17"
Id: "auto-ebs-2"
Statement:
- Sid: "Allow access through EBS for all principals in the account that are authorized to use EBS"
Effect: "Allow"
Principal:
AWS: "*"
Action:
- "kms:Encrypt"
- "kms:Decrypt"
- "kms:ReEncrypt*"
- "kms:GenerateDataKey*"
- "kms:CreateGrant"
- "kms:DescribeKey"
Resource: "*"
Condition:
StringEquals:
kms:CallerAccount: "111111111111"
kms:ViaService: "ec2.ap-southeast-2.amazonaws.com"
- Sid: "Allow direct access to key metadata to the account"
Effect: "Allow"
Principal:
AWS: "arn:aws:iam::111111111111:root"
Action:
- "kms:Describe*"
- "kms:Get*"
- "kms:List*"
- "kms:RevokeGrant"
Resource: "*"
key_policies:
description: list of policy documents for the key. Empty when access is denied even if there are policies.
type: list
returned: always
elements: dict
sample:
Version: "2012-10-17"
Id: "auto-ebs-2"
Expand Down Expand Up @@ -351,6 +398,7 @@
- "kms:List*"
- "kms:RevokeGrant"
Resource: "*"
version_added: 3.3.0
tags:
description: dictionary of tags applied to the key
type: dict
Expand Down Expand Up @@ -584,6 +632,7 @@ def get_key_details(connection, module, key_id):
tags = get_kms_tags(connection, module, key_id)
result['tags'] = boto3_tag_list_to_ansible_dict(tags, 'TagKey', 'TagValue')
result['policies'] = get_kms_policies(connection, module, key_id)
result['key_policies'] = [json.loads(policy) for policy in result['policies']]
return result


Expand Down Expand Up @@ -817,13 +866,15 @@ def update_key_rotation(connection, module, key, enable_key_rotation):
except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: # pylint: disable=duplicate-except
module.fail_json_aws(e, msg="Unable to get current key rotation status")

try:
if enable_key_rotation:
connection.enable_key_rotation(KeyId=key_id)
else:
connection.disable_key_rotation(KeyId=key_id)
except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:
module.fail_json_aws(e, msg="Failed to enable/disable key rotation")
if not module.check_mode:
try:
if enable_key_rotation:
connection.enable_key_rotation(KeyId=key_id)
else:
connection.disable_key_rotation(KeyId=key_id)
except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:
module.fail_json_aws(e, msg="Failed to enable/disable key rotation")

return True


Expand Down Expand Up @@ -1030,6 +1081,11 @@ def canonicalize_alias_name(alias):


def fetch_key_metadata(connection, module, key_id, alias):
# Note - fetching a key's metadata is very inconsistent shortly after any sort of update to a key has occurred.
# Combinations of manual waiters, checking expecting key values to actual key value, and static sleeps
# have all been exhausted, but none of those available options have solved the problem.
# Integration tests will wait for 10 seconds to combat this issue.
# See https://github.com/ansible-collections/community.aws/pull/1052.

alias = canonicalize_alias_name(module.params.get('alias'))

Expand Down Expand Up @@ -1104,10 +1160,13 @@ 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='community.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 %s to update")
module.fail_json(msg="Could not find key with id {0} to update".format(module.params.get('key_id')))

if module.params.get('policy_grant_types') or mode == 'deny':
module.deprecate('Managing the KMS IAM Policy via policy_mode and policy_grant_types is fragile'
Expand Down
44 changes: 43 additions & 1 deletion plugins/modules/aws_kms_info.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,9 +150,10 @@
Name: myKey
Purpose: protecting_stuff
policies:
description: list of policy documents for the keys. Empty when access is denied even if there are policies.
description: list of policy documents for the key. Empty when access is denied even if there are policies.
type: list
returned: always
elements: str
sample:
Version: "2012-10-17"
Id: "auto-ebs-2"
Expand Down Expand Up @@ -183,6 +184,42 @@
- "kms:List*"
- "kms:RevokeGrant"
Resource: "*"
key_policies:
description: list of policy documents for the key. Empty when access is denied even if there are policies.
type: list
returned: always
elements: dict
sample:
Version: "2012-10-17"
Id: "auto-ebs-2"
Statement:
- Sid: "Allow access through EBS for all principals in the account that are authorized to use EBS"
Effect: "Allow"
Principal:
AWS: "*"
Action:
- "kms:Encrypt"
- "kms:Decrypt"
- "kms:ReEncrypt*"
- "kms:GenerateDataKey*"
- "kms:CreateGrant"
- "kms:DescribeKey"
Resource: "*"
Condition:
StringEquals:
kms:CallerAccount: "111111111111"
kms:ViaService: "ec2.ap-southeast-2.amazonaws.com"
- Sid: "Allow direct access to key metadata to the account"
Effect: "Allow"
Principal:
AWS: "arn:aws:iam::111111111111:root"
Action:
- "kms:Describe*"
- "kms:Get*"
- "kms:List*"
- "kms:RevokeGrant"
Resource: "*"
version_added: 3.3.0
grants:
description: list of grants associated with a key
type: complex
Expand Down Expand Up @@ -240,6 +277,7 @@
sample: arn:aws:sts::0123456789012:assumed-role/lambda_xyz/xyz
'''

import json

try:
import botocore
Expand Down Expand Up @@ -418,6 +456,7 @@ def get_key_details(connection, module, key_id, tokens=None):
result = camel_dict_to_snake_dict(result)
result['tags'] = boto3_tag_list_to_ansible_dict(tags, 'TagKey', 'TagValue')
result['policies'] = get_kms_policies(connection, module, key_id)
result['key_policies'] = [json.loads(policy) for policy in result['policies']]
return result


Expand Down Expand Up @@ -460,6 +499,9 @@ def main():
except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:
module.fail_json_aws(e, msg='Failed to connect to 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='community.aws')

all_keys = get_kms_info(connection, module)
filtered_keys = [key for key in all_keys if key_matches_filters(key, module.params['filters'])]
ret_params = dict(kms_keys=filtered_keys)
Expand Down
3 changes: 2 additions & 1 deletion tests/integration/targets/aws_kms/aliases
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Various race conditions - likely needs waiters
# https://github.com/ansible-collections/community.aws/issues/433
unstable
# No KMS supported waiters, and manual waiting for updates didn't fix the issue either.
# Issue likely from AWS side - added waits on updates in integration tests to workaround this.

cloud/aws

Expand Down
3 changes: 1 addition & 2 deletions tests/integration/targets/aws_kms/defaults/main.yml
Original file line number Diff line number Diff line change
@@ -1,3 +1,2 @@
---
kms_role_name: 'ansible-test-{{ resource_prefix }}-kms'
kms_key_alias: '{{ resource_prefix }}-kms'
kms_key_alias: 'ansible-test-{{ resource_prefix }}-kms'
Loading

0 comments on commit 558a025

Please sign in to comment.