diff --git a/lib/ansible/modules/cloud/amazon/s3_bucket.py b/lib/ansible/modules/cloud/amazon/s3_bucket.py index 1e2243705e..c7c341fd86 100644 --- a/lib/ansible/modules/cloud/amazon/s3_bucket.py +++ b/lib/ansible/modules/cloud/amazon/s3_bucket.py @@ -120,6 +120,7 @@ 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.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 @@ -155,6 +156,57 @@ 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): + policy_list.sort() + return policy_list + + +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") @@ -195,9 +247,9 @@ def _create_or_update_bucket(connection, module, location): requester_pays_status = get_request_payment_status(bucket) if requester_pays_status != requester_pays: if requester_pays: - payer='Requester' + payer = 'Requester' else: - payer='BucketOwner' + payer = 'BucketOwner' bucket.set_request_payment(payer=payer) changed = True requester_pays_status = get_request_payment_status(bucket) @@ -220,9 +272,11 @@ def _create_or_update_bucket(connection, module, location): changed = bool(current_policy) elif sort_json_policy_dict(current_policy) != sort_json_policy_dict(policy): + # doesn't necessarily mean the policy has changed; syntax could differ + changed = compare_policies(sort_json_policy_dict(current_policy), sort_json_policy_dict(policy)) try: - bucket.set_policy(json.dumps(policy)) - changed = True + if changed: + bucket.set_policy(json.dumps(policy)) current_policy = json.loads(bucket.get_policy()) except S3ResponseError as e: module.fail_json(msg=e.message) @@ -291,7 +345,7 @@ def _destroy_bucket(connection, module): def _create_or_update_bucket_ceph(connection, module, location): - #TODO: add update + # TODO: add update name = module.params.get("name") @@ -349,6 +403,7 @@ def is_walrus(s3_url): else: return False + def main(): argument_spec = ec2_argument_spec() @@ -434,8 +489,8 @@ def main(): except Exception as e: module.fail_json(msg='Failed to connect to S3: %s' % str(e)) - if connection is None: # this should never happen - module.fail_json(msg ='Unknown error, failed to create s3 connection, no information from boto.') + if connection is None: # this should never happen + module.fail_json(msg='Unknown error, failed to create s3 connection, no information from boto.') state = module.params.get("state") diff --git a/test/sanity/pep8/legacy-files.txt b/test/sanity/pep8/legacy-files.txt index 4d91b444ba..3df38c839b 100644 --- a/test/sanity/pep8/legacy-files.txt +++ b/test/sanity/pep8/legacy-files.txt @@ -57,7 +57,6 @@ lib/ansible/modules/cloud/amazon/rds_subnet_group.py lib/ansible/modules/cloud/amazon/redshift.py lib/ansible/modules/cloud/amazon/route53_health_check.py lib/ansible/modules/cloud/amazon/s3.py -lib/ansible/modules/cloud/amazon/s3_bucket.py lib/ansible/modules/cloud/amazon/s3_lifecycle.py lib/ansible/modules/cloud/amazon/s3_logging.py lib/ansible/modules/cloud/amazon/s3_website.py