s3_object: Add parameter acl_disabled to handle uploading files to …

…buckets with ACL disabled. (#921)

s3_object: Add parameter `acl_disabled` to handle uploading files to buckets with ACL disabled.

SUMMARY

Fixes #863
[Update: 07/07/2022]
https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/s3.html#S3.Client.get_bucket_ownership_controls can be used to determine if ACL is disabled or not.
Modified code to convert acl_disabled from a user input parameter to a variable used for testing if ACL is enabled/disabled.
Add parameter  acl_disabled to handle uploading files to buckets with ACL disabled.
If set to true, all the permission related operations are skipped in module code.
ISSUE TYPE


Bugfix Pull Request

COMPONENT NAME

s3_object
ADDITIONAL INFORMATION


AWS added the option to create S3 bucket with ACL Disabled in Nov 2021) and made it the default/suggested setting when creating S3 bucket through AWS Portal .
Currently the ACL "permission" parameter defaults to ["private"] and there is no way to tell the s3_object module to omit the ACL setting while uploading files to a bucket which has set the ACL as disabled.
Tried looking for a way to get the existing bucket info to determine if 'ACL' is enabled/disabled, but was not able to find what I was looking for in API documentation.

Reviewed-by: Alina Buzachis <None>
Reviewed-by: Jill R <None>
Reviewed-by: Mark Chappell <None>
Reviewed-by: Mandar Kulkarni <[email protected]>

changelogs/fragments/921-s3_object-handle-file-upload-to-acl-disabled-bucket.yml

@@ -0,0 +1,2 @@
+minor_changes:
+- s3_object - updated module to add support for handling file upload to a bucket with ACL disabled (https://github.com/ansible-collections/amazon.aws/pull/921).

plugins/modules/s3_object.py

@@ -634,7 +634,7 @@ def option_in_extra_args(option):
         return allowed_extra_args[temp_option]
 
 
-def upload_s3file(module, s3, bucket, obj, expiry, metadata, encrypt, headers, src=None, content=None):
+def upload_s3file(module, s3, bucket, obj, expiry, metadata, encrypt, headers, src=None, content=None, acl_disabled=False):
     if module.check_mode:
         module.exit_json(msg="PUT operation skipped - running in check mode", changed=True)
     try:
@@ -677,13 +677,14 @@ def upload_s3file(module, s3, bucket, obj, expiry, metadata, encrypt, headers, s
             s3.upload_fileobj(Fileobj=f, Bucket=bucket, Key=obj, ExtraArgs=extra)
     except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:
         module.fail_json_aws(e, msg="Unable to complete PUT operation.")
-    try:
-        for acl in module.params.get('permission'):
-            s3.put_object_acl(ACL=acl, Bucket=bucket, Key=obj)
-    except is_boto3_error_code(IGNORE_S3_DROP_IN_EXCEPTIONS):
-        module.warn("PutObjectAcl is not implemented by your storage provider. Set the permission parameters to the empty list to avoid this warning")
-    except (botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError) as e:  # pylint: disable=duplicate-except
-        module.fail_json_aws(e, msg="Unable to set object ACL")
+    if not acl_disabled:
+        try:
+            for acl in module.params.get('permission'):
+                s3.put_object_acl(ACL=acl, Bucket=bucket, Key=obj)
+        except is_boto3_error_code(IGNORE_S3_DROP_IN_EXCEPTIONS):
+            module.warn("PutObjectAcl is not implemented by your storage provider. Set the permission parameters to the empty list to avoid this warning")
+        except (botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError) as e:  # pylint: disable=duplicate-except
+            module.fail_json_aws(e, msg="Unable to set object ACL")
 
     # Tags
     tags, changed = ensure_tags(s3, module, bucket, obj)
@@ -1056,12 +1057,25 @@ def main():
 
     validate = not ignore_nonexistent_bucket
 
+    # check if bucket exists, if yes, check if ACL is disabled
+    acl_disabled = False
+    exists = bucket_check(module, s3, bucket)
+    if exists:
+        try:
+            object_ownership = s3.get_bucket_ownership_controls(Bucket=bucket)['OwnershipControls']['Rules'][0]['ObjectOwnership']
+            if object_ownership == 'BucketOwnerEnforced':
+                acl_disabled = True
+        # if bucket ownership controls are not found
+        except botocore.exceptions.ClientError as e:
+            pass
+
     # separate types of ACLs
-    bucket_acl = [acl for acl in module.params.get('permission') if acl in bucket_canned_acl]
-    object_acl = [acl for acl in module.params.get('permission') if acl in object_canned_acl]
-    error_acl = [acl for acl in module.params.get('permission') if acl not in bucket_canned_acl and acl not in object_canned_acl]
-    if error_acl:
-        module.fail_json(msg='Unknown permission specified: %s' % error_acl)
+    if not acl_disabled:
+        bucket_acl = [acl for acl in module.params.get('permission') if acl in bucket_canned_acl]
+        object_acl = [acl for acl in module.params.get('permission') if acl in object_canned_acl]
+        error_acl = [acl for acl in module.params.get('permission') if acl not in bucket_canned_acl and acl not in object_canned_acl]
+        if error_acl:
+            module.fail_json(msg='Unknown permission specified: %s' % error_acl)
 
     # First, we check to see if the bucket exists, we get "bucket" returned.
     bucketrtn = bucket_check(module, s3, bucket, validate=validate)
@@ -1124,8 +1138,9 @@ def main():
                 get_download_url(module, s3, bucket, obj, expiry, tags, changed=tags_update)
 
         # only use valid object acls for the upload_s3file function
-        module.params['permission'] = object_acl
-        upload_s3file(module, s3, bucket, obj, expiry, metadata, encrypt, headers, src=src, content=bincontent)
+        if not acl_disabled:
+            module.params['permission'] = object_acl
+        upload_s3file(module, s3, bucket, obj, expiry, metadata, encrypt, headers, src=src, content=bincontent, acl_disabled=acl_disabled)
 
     # Delete an object from a bucket, not the entire bucket
     if mode == 'delobj':

tests/integration/targets/s3_object/meta/main.yml

@@ -0,0 +1,2 @@
+dependencies:
+  - setup_remote_tmp_dir

tests/integration/targets/s3_object/tasks/copy_object_acl_disabled_bucket.yml

@@ -0,0 +1,111 @@
+- name: test copying objects to bucket with ACL disabled
+  block:
+    - name: Create a bucket with ACL disabled for the test
+      s3_bucket:
+        name: "{{ bucket_name }}-acl-disabled"
+        object_ownership: BucketOwnerEnforced
+        state: present
+      register: create_result
+
+    - name: Ensure bucket creation
+      assert:
+        that:
+          - create_result is changed
+          - create_result is not failed
+          - create_result.object_ownership == "BucketOwnerEnforced"
+
+    - name: Create content
+      set_fact:
+          content: "{{ lookup('password', '/dev/null chars=ascii_letters,digits,hexdigits,punctuation') }}"
+
+    - name: Create local acl_disabled_upload_test.txt
+      copy:
+        content: "{{ content }}"
+        dest: "{{ remote_tmp_dir }}/acl_disabled_upload_test.txt"
+
+    - name: Upload a file to the bucket (check_mode)
+      amazon.aws.s3_object:
+        bucket: "{{ bucket_name }}-acl-disabled"
+        src: "{{ remote_tmp_dir }}/acl_disabled_upload_test.txt"
+        object: "acl_disabled_upload_test.txt"
+        mode: put
+      check_mode: true
+      register: upload_file_result
+
+    - assert:
+        that:
+          - upload_file_result is changed
+          - upload_file_result is not failed
+          - upload_file_result.msg == "PUT operation skipped - running in check mode"
+          - '"s3:PutObject" not in upload_file_result.resource_actions'
+
+    - name: Upload a file to the bucket
+      amazon.aws.s3_object:
+        bucket: "{{ bucket_name }}-acl-disabled"
+        src: "{{ remote_tmp_dir }}/acl_disabled_upload_test.txt"
+        object: "acl_disabled_upload_test.txt"
+        mode: put
+      register: upload_file_result
+
+    - assert:
+        that:
+          - upload_file_result is changed
+          - upload_file_result is not failed
+          - upload_file_result.msg == "PUT operation complete"
+          - '"s3:PutObject" in upload_file_result.resource_actions'
+
+    - name: Upload a file to the bucket (check_mode - idempotency)
+      amazon.aws.s3_object:
+        bucket: "{{ bucket_name }}-acl-disabled"
+        src: "{{ remote_tmp_dir }}/acl_disabled_upload_test.txt"
+        object: "acl_disabled_upload_test.txt"
+        mode: put
+      check_mode: true
+      register: upload_file_result
+
+    - assert:
+        that:
+          - upload_file_result is not changed
+          - upload_file_result is not failed
+          - upload_file_result.msg != "PUT operation complete"
+          - '"s3:PutObject" not in upload_file_result.resource_actions'
+
+    - name: Upload a file to the bucket (idempotency)
+      amazon.aws.s3_object:
+        bucket: "{{ bucket_name }}-acl-disabled"
+        src: "{{ remote_tmp_dir }}/acl_disabled_upload_test.txt"
+        object: "acl_disabled_upload_test.txt"
+        mode: put
+      register: upload_file_result
+
+    - assert:
+        that:
+          - upload_file_result is not changed
+          - upload_file_result is not failed
+          - upload_file_result.msg != "PUT operation complete"
+          - '"s3:PutObject" not in upload_file_result.resource_actions'
+
+  always:
+
+    - name: Delete the file in the bucket
+      amazon.aws.s3_object:
+        bucket: "{{ bucket_name }}-acl-disabled"
+        src: "{{ remote_tmp_dir }}/acl_disabled_upload_test.txt"
+        object: "acl_disabled_upload_test.txt"
+        mode: delobj
+      retries: 3
+      delay: 3
+      ignore_errors: true
+
+    - name: Delete bucket created in this test
+      s3_bucket:
+        name: "{{ bucket_name }}-acl-disabled"
+        object_ownership: BucketOwnerEnforced
+        state: absent
+      register: delete_result
+
+    - name: Ensure bucket deletion
+      assert:
+        that:
+          - delete_result is changed
+          - delete_result is not failed