From 8a02f894a6c3a3b1fb5c85125062957dcdbb66f5 Mon Sep 17 00:00:00 2001 From: Sloane Hertel Date: Thu, 28 Feb 2019 16:09:00 -0600 Subject: [PATCH] replace uses of sort_json_policy_dict with compare_policies (#52943) * replace uses of sort_json_policy_dict with compare_policies which is compatible with Python 3 and remove sort_json_policy_dict from the AWS guidelines since it only works on Python 2 * Sort any lists containing only strings * Sort the original policy too --- ...thon3-compatibility-with-AWS-policies.yaml | 3 +++ .../modules/cloud/amazon/GUIDELINES.md | 9 --------- lib/ansible/modules/cloud/amazon/ecs_ecr.py | 20 +++++++++++++++---- lib/ansible/modules/cloud/amazon/iam_role.py | 5 ++--- 4 files changed, 21 insertions(+), 16 deletions(-) create mode 100644 changelogs/fragments/fix-python3-compatibility-with-AWS-policies.yaml diff --git a/changelogs/fragments/fix-python3-compatibility-with-AWS-policies.yaml b/changelogs/fragments/fix-python3-compatibility-with-AWS-policies.yaml new file mode 100644 index 0000000000..e43fccbbf9 --- /dev/null +++ b/changelogs/fragments/fix-python3-compatibility-with-AWS-policies.yaml @@ -0,0 +1,3 @@ +bugfixes: + - ecs_ecr and iam_role - replace uses of sort_json_policy_dict with compare_policies which is compatible with Python 3 + - Remove recommendation to use sort_json_policy_dict in the AWS guidelines diff --git a/lib/ansible/modules/cloud/amazon/GUIDELINES.md b/lib/ansible/modules/cloud/amazon/GUIDELINES.md index b8becb0819..4132bedb8e 100644 --- a/lib/ansible/modules/cloud/amazon/GUIDELINES.md +++ b/lib/ansible/modules/cloud/amazon/GUIDELINES.md @@ -563,15 +563,6 @@ if there are. This recursively sorts the dicts and makes them hashable before co This method should be used any time policies are being compared so that a change in order doesn't result in unnecessary changes. -#### sort_json_policy_dict - -Pass any JSON policy dict to this function in order to sort any list contained therein. This is -useful because AWS rarely return lists in the same order that they were submitted so without this -function, comparison of identical policies returns false. - -Note if your goal is to check if two policies are the same you're better to use the `compare_policies` -helper which sorts recursively. - #### compare_aws_tags Pass two dicts of tags and an optional purge parameter and this function will return a dict diff --git a/lib/ansible/modules/cloud/amazon/ecs_ecr.py b/lib/ansible/modules/cloud/amazon/ecs_ecr.py index f11a978806..b16d6b5124 100644 --- a/lib/ansible/modules/cloud/amazon/ecs_ecr.py +++ b/lib/ansible/modules/cloud/amazon/ecs_ecr.py @@ -134,7 +134,8 @@ except ImportError: from ansible.module_utils.basic import AnsibleModule from ansible.module_utils.ec2 import (HAS_BOTO3, boto3_conn, boto_exception, ec2_argument_spec, - get_aws_connection_info, sort_json_policy_dict) + get_aws_connection_info, compare_policies) +from ansible.module_utils.six import string_types def build_kwargs(registry_id): @@ -250,6 +251,15 @@ class EcsEcr: return None +def sort_lists_of_strings(policy): + for statement_index in range(0, len(policy.get('Statement', []))): + for key in policy['Statement'][statement_index]: + value = policy['Statement'][statement_index][key] + if isinstance(value, list) and all(isinstance(item, string_types) for item in value): + policy['Statement'][statement_index][key] = sorted(value) + return policy + + def run(ecr, params, verbosity): # type: (EcsEcr, dict, int) -> Tuple[bool, dict] result = {} @@ -292,19 +302,21 @@ def run(ecr, params, verbosity): elif policy_text is not None: try: - policy = sort_json_policy_dict(policy) + # Sort any lists containing only string types + policy = sort_lists_of_strings(policy) + if verbosity >= 2: result['policy'] = policy original_policy = ecr.get_repository_policy( registry_id, name) if original_policy: - original_policy = sort_json_policy_dict(original_policy) + original_policy = sort_lists_of_strings(original_policy) if verbosity >= 3: result['original_policy'] = original_policy - if original_policy != policy: + if compare_policies(original_policy, policy): ecr.set_repository_policy( registry_id, name, policy_text, force_set_policy) result['changed'] = True diff --git a/lib/ansible/modules/cloud/amazon/iam_role.py b/lib/ansible/modules/cloud/amazon/iam_role.py index b98400da73..9f549e1db0 100644 --- a/lib/ansible/modules/cloud/amazon/iam_role.py +++ b/lib/ansible/modules/cloud/amazon/iam_role.py @@ -158,7 +158,7 @@ iam_role: from ansible.module_utils._text import to_native from ansible.module_utils.aws.core import AnsibleAWSModule -from ansible.module_utils.ec2 import camel_dict_to_snake_dict, ec2_argument_spec, get_aws_connection_info, boto3_conn, sort_json_policy_dict +from ansible.module_utils.ec2 import camel_dict_to_snake_dict, ec2_argument_spec, get_aws_connection_info, boto3_conn, compare_policies from ansible.module_utils.ec2 import AWSRetry import json @@ -171,8 +171,7 @@ except ImportError: def compare_assume_role_policy_doc(current_policy_doc, new_policy_doc): - - if sort_json_policy_dict(current_policy_doc) == sort_json_policy_dict(json.loads(new_policy_doc)): + if not compare_policies(current_policy_doc, json.loads(new_policy_doc)): return True else: return False