s3_sync - Improve error handling when testing for existing files (#1330…

…) (#1335)

[PR #1330/40984a40 backport][stable-4] s3_sync - Improve error handling when testing for existing files

This is a backport of PR #1330 as merged into main (40984a4).
SUMMARY
fixes: #58

Simplifies handling of '404' codes (use is_boto3_error_code)
Assume 403 files need updating (it's the best we can do, and mimics aws cli)
Allows Boto3 exceptions to fall through to the outer try/except clause and cleanly fail rather than rethrowing it as an Exception()

ISSUE TYPE

Feature Pull Request

COMPONENT NAME
s3_sync
ADDITIONAL INFORMATION

Reviewed-by: Mark Chappell <None>

changelogs/fragments/58-s3_sync.yml

@@ -0,0 +1,2 @@
+minor_changes:
+- s3_sync - improves error handling during ``HEAD`` operation to compare existing files (https://github.com/ansible-collections/community.aws/issues/58).

plugins/modules/s3_sync.py

@@ -265,6 +265,7 @@
 
 # import module snippets
 from ansible_collections.amazon.aws.plugins.module_utils.core import AnsibleAWSModule
+from ansible_collections.amazon.aws.plugins.module_utils.core import is_boto3_error_code
 
 
 # the following function, calculate_multipart_etag, is from tlastowka
@@ -427,17 +428,12 @@ def head_s3(s3, bucket, s3keys):
     retkeys = []
     for entry in s3keys:
         retentry = entry.copy()
-        # don't modify the input dict
         try:
             retentry['s3_head'] = s3.head_object(Bucket=bucket, Key=entry['s3_path'])
-        except botocore.exceptions.ClientError as err:
-            if (hasattr(err, 'response') and
-                    'ResponseMetadata' in err.response and
-                    'HTTPStatusCode' in err.response['ResponseMetadata'] and
-                    str(err.response['ResponseMetadata']['HTTPStatusCode']) == '404'):
-                pass
-            else:
-                raise Exception(err)
+        # 404 (Missing) - File doesn't exist, we'll need to upload
+        # 403 (Denied) - Sometimes we can write but not read, assume we'll need to upload
+        except is_boto3_error_code(['404', '403']):
+            pass
         retkeys.append(retentry)
     return retkeys
 

tests/integration/targets/s3_sync/tasks/main.yml

@@ -44,7 +44,7 @@
         dd if=/dev/zero of=test4.txt bs=1M count=10
       args:
         chdir: "{{ output_dir }}/s3_sync"
-   
+
     - name: Sync files with remote bucket
       s3_sync:
         bucket: "{{ test_bucket }}"
@@ -66,7 +66,7 @@
           - output.changed
           - output.name == "{{ test_bucket_2 }}"
           - not output.requester_pays
-    
+
     - name: Sync files with remote bucket using glacier storage class
       s3_sync:
         bucket: "{{ test_bucket_2 }}"
@@ -77,7 +77,7 @@
     - assert:
         that:
           - output is changed
-    
+
     # ============================================================
 
     - name: Sync files already present
@@ -99,7 +99,7 @@
     - assert:
         that:
           - output is not changed
- 
+
     # ============================================================
     - name: Create a third s3_bucket
       s3_bucket:
@@ -112,7 +112,7 @@
           - output.changed
           - output.name == "{{ test_bucket_3 }}"
           - not output.requester_pays
-   
+
     - name: Sync individual file with remote bucket
       s3_sync:
         bucket: "{{ test_bucket_3 }}"
@@ -168,9 +168,9 @@
             object: "{{ obj }}"
           with_items: "{{ objects.s3_keys }}"
           loop_control:
-            loop_var: obj      
+            loop_var: obj
           ignore_errors: true
-        
+
         - name: list test_bucket_2 objects
           aws_s3:
             bucket: "{{ test_bucket_2 }}"
@@ -185,9 +185,9 @@
             object: "{{ obj }}"
           with_items: "{{ objects.s3_keys }}"
           loop_control:
-            loop_var: obj      
+            loop_var: obj
           ignore_errors: true
-        
+
         - name: list test_bucket_3 objects
           aws_s3:
             bucket: "{{ test_bucket_3 }}"
@@ -202,7 +202,7 @@
             object: "{{ obj }}"
           with_items: "{{ objects.s3_keys }}"
           loop_control:
-            loop_var: obj      
+            loop_var: obj
           ignore_errors: true
 
     - name: Ensure all buckets are deleted