s3_bucket - Ensure public_access settings are configured before polic…

…ies (#1511)

s3_bucket - Ensure public_access settings are configured before policies

SUMMARY
At the end of April Amazon updated various S3 bucket defaults.  Buckets now have public_access blocked by default, and object_owner set to "BucketOwnerEnforced".
https://aws.amazon.com/blogs/aws/heads-up-amazon-s3-security-changes-are-coming-in-april-of-2023/
This uncovered a race condition where we set the policy before setting the public_access configs.
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
s3_bucket
ADDITIONAL INFORMATION

Reviewed-by: Alina Buzachis

changelogs/fragments/1511-s3_bucket-public_access.yml

@@ -0,0 +1,2 @@
+minor_changes:
+- s3_bucket - ensure ``public_access`` is configured before updating policies (https://github.com/ansible-collections/amazon.aws/pull/1511).

plugins/modules/s3_bucket.py

@@ -83,6 +83,10 @@
     description:
       - Configure public access block for S3 bucket.
       - This option cannot be used together with I(delete_public_access).
+      - |
+        Note: At the end of April 2023 Amazon updated the default settings to block public access by
+        default.  While the defaults for this module remain unchanged, it is necessary to explicitly
+        pass the I(public_access) parameter to enable public access ACLs.
     suboptions:
       block_public_acls:
         description: Sets BlockPublicAcls value.
@@ -122,6 +126,7 @@
         if the object is uploaded with the bucket-owner-full-control canned ACL.
       - This option cannot be used together with a I(delete_object_ownership) definition.
       - C(BucketOwnerEnforced) has been added in version 3.2.0.
+      - "Note: At the end of April 2023 Amazon updated the default setting to C(BucketOwnerEnforced)."
     choices: [ 'BucketOwnerEnforced', 'BucketOwnerPreferred', 'ObjectWriter' ]
     type: str
     version_added: 2.0.0
@@ -475,6 +480,43 @@ def create_or_update_bucket(s3_client, module):
 
         result["requester_pays"] = requester_pays
 
+    # Public access clock configuration
+    current_public_access = {}
+    try:
+        current_public_access = get_bucket_public_access(s3_client, name)
+    except is_boto3_error_code(["NotImplemented", "XNotImplemented"]) as e:
+        if public_access is not None:
+            module.fail_json_aws(e, msg="Bucket public access settings are not supported by the current S3 Endpoint")
+    except is_boto3_error_code("AccessDenied") as e:
+        if public_access is not None:
+            module.fail_json_aws(e, msg="Failed to get bucket public access configuration")
+        module.debug("AccessDenied fetching bucket public access settings")
+    except (
+        botocore.exceptions.BotoCoreError,
+        botocore.exceptions.ClientError,
+    ) as e:  # pylint: disable=duplicate-except
+        module.fail_json_aws(e, msg="Failed to get bucket public access configuration")
+    else:
+        # -- Create / Update public access block
+        if public_access is not None:
+            camel_public_block = snake_dict_to_camel_dict(public_access, capitalize_first=True)
+
+            if current_public_access == camel_public_block:
+                result["public_access_block"] = current_public_access
+            else:
+                put_bucket_public_access(s3_client, name, camel_public_block)
+                changed = True
+                result["public_access_block"] = camel_public_block
+
+        # -- Delete public access block
+        if delete_public_access:
+            if current_public_access == {}:
+                result["public_access_block"] = current_public_access
+            else:
+                delete_bucket_public_access(s3_client, name)
+                changed = True
+                result["public_access_block"] = {}
+
     # Policy
     try:
         current_policy = get_bucket_policy(s3_client, name)
@@ -606,43 +648,6 @@ def create_or_update_bucket(s3_client, module):
                     current_encryption = put_bucket_key_with_retry(module, s3_client, name, expected_encryption)
                     changed = True
         result["encryption"] = current_encryption
-    # Public access clock configuration
-    current_public_access = {}
-
-    try:
-        current_public_access = get_bucket_public_access(s3_client, name)
-    except is_boto3_error_code(["NotImplemented", "XNotImplemented"]) as e:
-        if public_access is not None:
-            module.fail_json_aws(e, msg="Bucket public access settings are not supported by the current S3 Endpoint")
-    except is_boto3_error_code("AccessDenied") as e:
-        if public_access is not None:
-            module.fail_json_aws(e, msg="Failed to get bucket public access configuration")
-        module.debug("AccessDenied fetching bucket public access settings")
-    except (
-        botocore.exceptions.BotoCoreError,
-        botocore.exceptions.ClientError,
-    ) as e:  # pylint: disable=duplicate-except
-        module.fail_json_aws(e, msg="Failed to get bucket public access configuration")
-    else:
-        # -- Create / Update public access block
-        if public_access is not None:
-            camel_public_block = snake_dict_to_camel_dict(public_access, capitalize_first=True)
-
-            if current_public_access == camel_public_block:
-                result["public_access_block"] = current_public_access
-            else:
-                put_bucket_public_access(s3_client, name, camel_public_block)
-                changed = True
-                result["public_access_block"] = camel_public_block
-
-        # -- Delete public access block
-        if delete_public_access:
-            if current_public_access == {}:
-                result["public_access_block"] = current_public_access
-            else:
-                delete_bucket_public_access(s3_client, name)
-                changed = True
-                result["public_access_block"] = {}
 
     # -- Bucket ownership
     try:

tests/integration/targets/s3_bucket/roles/s3_bucket/tasks/acl.yml

@@ -12,6 +12,7 @@
     - name: 'Create a simple bucket'
       s3_bucket:
         name: '{{ local_bucket_name }}'
+        object_ownership: BucketOwnerPreferred
         public_access:
           block_public_acls: true
           block_public_policy: true

tests/integration/targets/s3_bucket/roles/s3_bucket/tasks/complex.yml

@@ -9,6 +9,8 @@
         policy: "{{ lookup('template','policy.json') }}"
         requester_pays: yes
         versioning: yes
+        public_access:
+          block_public_acls: false
         tags:
           example: tag1
           another: tag2