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

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

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

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

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