From 8ab4412684c99c688bf0f8006c28c475285d74b6 Mon Sep 17 00:00:00 2001 From: Ted Timmons Date: Thu, 26 Oct 2017 06:13:29 -0700 Subject: [PATCH] aws_kms: handle updated policy format+cleanup (#30728) * aws_kms: handle updated policy format+cleanup - create slightly updated policy in that handles lists instead of a single string; the previous version's policy was being rejected if the key was new enough to have the updated base policy. - removed `dry_run` conditionals, not committing the policy anyhow. - return the policy in the return data. Leaving undocumented for now. - update exception handling: don't rethrow in `do_grant`, don't pass anything to `format_exc`. * whitespace/indent fail * fix list-plus-brackets * str and list fixes for ryansb * port changes from #31667 over, better listification --- lib/ansible/modules/cloud/amazon/aws_kms.py | 46 +++++++++++---------- 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/lib/ansible/modules/cloud/amazon/aws_kms.py b/lib/ansible/modules/cloud/amazon/aws_kms.py index 45fe303f053..385716fbab1 100644 --- a/lib/ansible/modules/cloud/amazon/aws_kms.py +++ b/lib/ansible/modules/cloud/amazon/aws_kms.py @@ -105,6 +105,7 @@ statement_label = { # import module snippets from ansible.module_utils.basic import AnsibleModule from ansible.module_utils.ec2 import boto_exception +from ansible.module_utils.six import string_types # import a class, we'll use a fully qualified path import ansible.module_utils.ec2 @@ -118,7 +119,6 @@ try: except ImportError: HAS_BOTO3 = False - def get_arn_from_kms_alias(kms, aliasname): ret = kms.list_aliases() key_id = None @@ -156,53 +156,53 @@ def do_grant(kms, keyarn, role_arn, granttypes, mode='grant', dry_run=True, clea # do we want this grant type? Are we on its statement? # and does the role have this grant type? - # Ensure statement looks as expected - if not statement.get('Principal'): - statement['Principal'] = {'AWS': []} - if not isinstance(statement['Principal']['AWS'], list): + # create Principal and 'AWS' so we can safely use them later. + if not isinstance(statement.get('Principal'), dict): + statement['Principal'] = dict() + + if 'AWS' in statement['Principal'] and isinstance(statement['Principal']['AWS'], string_types): + # convert to list statement['Principal']['AWS'] = [statement['Principal']['AWS']] + if not isinstance(statement['Principal'].get('AWS'), list): + statement['Principal']['AWS'] = list() if mode == 'grant' and statement['Sid'] == statement_label[granttype]: # we're granting and we recognize this statement ID. if granttype in granttypes: - invalid_entries = list(filter(lambda x: not x.startswith('arn:aws:iam::'), statement['Principal']['AWS'])) + invalid_entries = [item for item in statement['Principal']['AWS'] if not item.startswith('arn:aws:iam::')] if clean_invalid_entries and invalid_entries: # we have bad/invalid entries. These are roles that were deleted. # prune the list. - valid_entries = filter(lambda x: x.startswith('arn:aws:iam::'), statement['Principal']['AWS']) + valid_entries = [item for item in statement['Principal']['AWS'] if item.startswith('arn:aws:iam::')] statement['Principal']['AWS'] = valid_entries had_invalid_entries = True - if not role_arn in statement['Principal']['AWS']: # needs to be added. changes_needed[granttype] = 'add' - if not dry_run: - statement['Principal']['AWS'].append(role_arn) + statement['Principal']['AWS'].append(role_arn) elif role_arn in statement['Principal']['AWS']: # not one the places the role should be changes_needed[granttype] = 'remove' - if not dry_run: - statement['Principal']['AWS'].remove(role_arn) + statement['Principal']['AWS'].remove(role_arn) elif mode == 'deny' and statement['Sid'] == statement_label[granttype] and role_arn in statement['Principal']['AWS']: # we don't selectively deny. that's a grant with a # smaller list. so deny=remove all of this arn. changes_needed[granttype] = 'remove' - if not dry_run: - statement['Principal']['AWS'].remove(role_arn) + statement['Principal']['AWS'].remove(role_arn) try: if len(changes_needed) and not dry_run: policy_json_string = json.dumps(policy) - except Exception as e: - raise Exception("{0}: // {1}".format(e, repr(policy))) - kms.put_key_policy(KeyId=keyarn, PolicyName='default', Policy=policy_json_string) - - # returns nothing, so we have to just assume it didn't throw - ret['changed'] = changes_needed and not had_invalid_entries + kms.put_key_policy(KeyId=keyarn, PolicyName='default', Policy=policy_json_string) + # returns nothing, so we have to just assume it didn't throw + ret['changed'] = True + except: + raise ret['changes_needed'] = changes_needed ret['had_invalid_entries'] = had_invalid_entries + ret['new_policy'] = policy if dry_run: # true if changes > 0 ret['changed'] = (not len(changes_needed) == 0) @@ -280,8 +280,10 @@ def main(): if not g in statement_label: module.fail_json(msg='{} is an unknown grant type.'.format(g)) - ret = do_grant(kms, module.params['key_arn'], module.params['role_arn'], module.params['grant_types'], mode=mode, dry_run=module.check_mode, - clean_invalid_entries=module.params['clean_invalid_entries']) + ret = do_grant(kms, module.params['key_arn'], module.params['role_arn'], module.params['grant_types'], + mode=mode, + dry_run=module.check_mode, + clean_invalid_entries=module.params['clean_invalid_entries']) result.update(ret) except Exception as err: