iam_managed_policy: use python 3 compatible policy comparison - fixes #31474 (#31535)

* Move compare_policies and hashable_policy functions into module_utils/ec2

* Use compare_policies which is compatible with python 2 and 3.

* rename function to indicate internal use

* s3_bucket: don't set changed to false if it has had the chance to be changed to true already.

lib/ansible/module_utils/ec2.py

@@ -29,8 +29,9 @@
 import os
 import re
 
-from ansible.module_utils._text import to_native
+from ansible.module_utils._text import to_native, to_text
 from ansible.module_utils.cloud import CloudRetry
+from ansible.module_utils.six import string_types, binary_type, text_type
 
 try:
     import boto
@@ -46,7 +47,13 @@ try:
 except:
     HAS_BOTO3 = False
 
-from ansible.module_utils.six import string_types, binary_type, text_type
+try:
+    # Although this is to allow Python 3 the ability to use the custom comparison as a key, Python 2.7 also
+    # uses this (and it works as expected). Python 2.6 will trigger the ImportError.
+    from functools import cmp_to_key
+    PY3_COMPARISON = True
+except ImportError:
+    PY3_COMPARISON = False
 
 
 class AnsibleAWSError(Exception):
@@ -566,6 +573,82 @@ def get_ec2_security_group_ids_from_names(sec_group_list, ec2_connection, vpc_id
     return sec_group_id_list
 
 
+def _hashable_policy(policy, policy_list):
+    """
+        Takes a policy and returns a list, the contents of which are all hashable and sorted.
+        Example input policy:
+        {'Version': '2012-10-17',
+         'Statement': [{'Action': 's3:PutObjectAcl',
+                        'Sid': 'AddCannedAcl2',
+                        'Resource': 'arn:aws:s3:::test_policy/*',
+                        'Effect': 'Allow',
+                        'Principal': {'AWS': ['arn:aws:iam::XXXXXXXXXXXX:user/username1', 'arn:aws:iam::XXXXXXXXXXXX:user/username2']}
+                       }]}
+        Returned value:
+        [('Statement',  ((('Action', (u's3:PutObjectAcl',)),
+                          ('Effect', (u'Allow',)),
+                          ('Principal', ('AWS', ((u'arn:aws:iam::XXXXXXXXXXXX:user/username1',), (u'arn:aws:iam::XXXXXXXXXXXX:user/username2',)))),
+                          ('Resource', (u'arn:aws:s3:::test_policy/*',)), ('Sid', (u'AddCannedAcl2',)))),
+         ('Version', (u'2012-10-17',)))]
+
+    """
+    if isinstance(policy, list):
+        for each in policy:
+            tupleified = _hashable_policy(each, [])
+            if isinstance(tupleified, list):
+                tupleified = tuple(tupleified)
+            policy_list.append(tupleified)
+    elif isinstance(policy, string_types):
+        return [(to_text(policy))]
+    elif isinstance(policy, dict):
+        sorted_keys = list(policy.keys())
+        sorted_keys.sort()
+        for key in sorted_keys:
+            tupleified = _hashable_policy(policy[key], [])
+            if isinstance(tupleified, list):
+                tupleified = tuple(tupleified)
+            policy_list.append((key, tupleified))
+
+    # ensure we aren't returning deeply nested structures of length 1
+    if len(policy_list) == 1 and isinstance(policy_list[0], tuple):
+        policy_list = policy_list[0]
+    if isinstance(policy_list, list):
+        if PY3_COMPARISON:
+            policy_list.sort(key=cmp_to_key(py3cmp))
+        else:
+            policy_list.sort()
+    return policy_list
+
+
+def py3cmp(a, b):
+    """ Python 2 can sort lists of mixed types. Strings < tuples. Without this function this fails on Python 3."""
+    try:
+        if a > b:
+            return 1
+        elif a < b:
+            return -1
+        else:
+            return 0
+    except TypeError as e:
+        # check to see if they're tuple-string
+        # always say strings are less than tuples (to maintain compatibility with python2)
+        str_ind = to_text(e).find('str')
+        tup_ind = to_text(e).find('tuple')
+        if -1 not in (str_ind, tup_ind):
+            if str_ind < tup_ind:
+                return -1
+            elif tup_ind < str_ind:
+                return 1
+        raise
+
+
+def compare_policies(current_policy, new_policy):
+    """ Compares the existing policy and the updated policy
+        Returns True if there is a difference between policies.
+    """
+    return set(_hashable_policy(new_policy, [])) != set(_hashable_policy(current_policy, []))
+
+
 def sort_json_policy_dict(policy_dict):
 
     """ Sort any lists in an IAM JSON policy so that comparison of two policies with identical values but

lib/ansible/modules/cloud/amazon/iam_managed_policy.py

@@ -117,7 +117,7 @@ except ImportError:
 
 from ansible.module_utils.basic import AnsibleModule
 from ansible.module_utils.ec2 import (boto3_conn, get_aws_connection_info, ec2_argument_spec, AWSRetry,
-                                      sort_json_policy_dict, camel_dict_to_snake_dict, HAS_BOTO3)
+                                      camel_dict_to_snake_dict, HAS_BOTO3, compare_policies)
 from ansible.module_utils._text import to_native
 
 
@@ -174,8 +174,8 @@ def get_or_create_policy_version(module, iam, policy, policy_document):
             module.fail_json(msg="Couldn't get policy version %s: %s" % (v['VersionId'], str(e)),
                              exception=traceback.format_exc(),
                              **camel_dict_to_snake_dict(e.response))
-        if sort_json_policy_dict(document) == sort_json_policy_dict(
+        # If the current policy matches the existing one
-                json.loads(policy_document)):
+        if not compare_policies(document, json.loads(to_native(policy_document))):
             return v, False
 
     # No existing version so create one

lib/ansible/modules/cloud/amazon/s3_bucket.py

@@ -120,10 +120,10 @@ import xml.etree.ElementTree as ET
 
 import ansible.module_utils.six.moves.urllib.parse as urlparse
 from ansible.module_utils.six import string_types
-from ansible.module_utils._text import to_text
+from ansible.module_utils._text import to_native
 from ansible.module_utils.basic import AnsibleModule
 from ansible.module_utils.ec2 import get_aws_connection_info, ec2_argument_spec
-from ansible.module_utils.ec2 import sort_json_policy_dict
+from ansible.module_utils.ec2 import sort_json_policy_dict, compare_policies
 
 try:
     import boto.ec2
@@ -134,14 +134,6 @@ try:
 except ImportError:
     HAS_BOTO = False
 
-try:
-    # Although this is to allow Python 3 the ability to use the custom comparison as a key, Python 2.7 also
-    # uses this (and it works as expected). Python 2.6 will trigger the ImportError.
-    from functools import cmp_to_key
-    PY3_COMPARISON = True
-except ImportError:
-    PY3_COMPARISON = False
-
 
 def get_request_payment_status(bucket):
 
@@ -164,82 +156,6 @@ def create_tags_container(tags):
     return tags_obj
 
 
-def hashable_policy(policy, policy_list):
-    """
-        Takes a policy and returns a list, the contents of which are all hashable and sorted.
-        Example input policy:
-        {'Version': '2012-10-17',
-         'Statement': [{'Action': 's3:PutObjectAcl',
-                        'Sid': 'AddCannedAcl2',
-                        'Resource': 'arn:aws:s3:::test_policy/*',
-                        'Effect': 'Allow',
-                        'Principal': {'AWS': ['arn:aws:iam::XXXXXXXXXXXX:user/username1', 'arn:aws:iam::XXXXXXXXXXXX:user/username2']}
-                       }]}
-        Returned value:
-        [('Statement',  ((('Action', (u's3:PutObjectAcl',)),
-                          ('Effect', (u'Allow',)),
-                          ('Principal', ('AWS', ((u'arn:aws:iam::XXXXXXXXXXXX:user/username1',), (u'arn:aws:iam::XXXXXXXXXXXX:user/username2',)))),
-                          ('Resource', (u'arn:aws:s3:::test_policy/*',)), ('Sid', (u'AddCannedAcl2',)))),
-         ('Version', (u'2012-10-17',)))]
-
-    """
-    if isinstance(policy, list):
-        for each in policy:
-            tupleified = hashable_policy(each, [])
-            if isinstance(tupleified, list):
-                tupleified = tuple(tupleified)
-            policy_list.append(tupleified)
-    elif isinstance(policy, string_types):
-        return [(to_text(policy))]
-    elif isinstance(policy, dict):
-        sorted_keys = list(policy.keys())
-        sorted_keys.sort()
-        for key in sorted_keys:
-            tupleified = hashable_policy(policy[key], [])
-            if isinstance(tupleified, list):
-                tupleified = tuple(tupleified)
-            policy_list.append((key, tupleified))
-
-    # ensure we aren't returning deeply nested structures of length 1
-    if len(policy_list) == 1 and isinstance(policy_list[0], tuple):
-        policy_list = policy_list[0]
-    if isinstance(policy_list, list):
-        if PY3_COMPARISON:
-            policy_list.sort(key=cmp_to_key(py3cmp))
-        else:
-            policy_list.sort()
-    return policy_list
-
-
-def py3cmp(a, b):
-    """ Python 2 can sort lists of mixed types. Strings < tuples. Without this function this fails on Python 3."""
-    try:
-        if a > b:
-            return 1
-        elif a < b:
-            return -1
-        else:
-            return 0
-    except TypeError as e:
-        # check to see if they're tuple-string
-        # always say strings are less than tuples (to maintain compatibility with python2)
-        str_ind = to_text(e).find('str')
-        tup_ind = to_text(e).find('tuple')
-        if -1 not in (str_ind, tup_ind):
-            if str_ind < tup_ind:
-                return -1
-            elif tup_ind < str_ind:
-                return 1
-        raise
-
-
-def compare_policies(current_policy, new_policy):
-    """ Compares the existing policy and the updated policy
-        Returns True if there is a difference between policies.
-    """
-    return set(hashable_policy(new_policy, [])) != set(hashable_policy(current_policy, []))
-
-
 def _create_or_update_bucket(connection, module, location):
 
     policy = module.params.get("policy")
@@ -289,7 +205,7 @@ def _create_or_update_bucket(connection, module, location):
 
     # Policy
     try:
-        current_policy = json.loads(bucket.get_policy())
+        current_policy = json.loads(to_native(bucket.get_policy()))
     except S3ResponseError as e:
         if e.error_code == "NoSuchBucketPolicy":
             current_policy = {}
@@ -304,13 +220,11 @@ def _create_or_update_bucket(connection, module, location):
             # only show changed if there was already a policy
             changed = bool(current_policy)
 
-        elif sort_json_policy_dict(current_policy) != sort_json_policy_dict(policy):
+        elif compare_policies(current_policy, policy):
-            # doesn't necessarily mean the policy has changed; syntax could differ
+            changed = True
-            changed = compare_policies(sort_json_policy_dict(current_policy), sort_json_policy_dict(policy))
             try:
-                if changed:
                 bucket.set_policy(json.dumps(policy))
-                current_policy = json.loads(bucket.get_policy())
+                current_policy = json.loads(to_native(bucket.get_policy()))
             except S3ResponseError as e:
                 module.fail_json(msg=e.message)