aws_s3 - ensure overwrite: different (ansible-collections#811)

aws_s3 - ensure overwrite: different SUMMARY aws_s3 - ensure overwrite: different instead of always in order for the module to be idempotent by default. Fixes: ansible-collections#762 ISSUE TYPE Feature Pull Request COMPONENT NAME aws_s3 Reviewed-by: Jill R <None> Reviewed-by: Mark Chappell <None>
jatorcasso · Jun 24, 2022 · 512f936 · 512f936
1 parent e75fc29
commit 512f936
Show file tree

Hide file tree

Showing 4 changed files with 52 additions and 38 deletions.
diff --git a/changelogs/fragments/811-aws_s3-ovewrite-default.yml b/changelogs/fragments/811-aws_s3-ovewrite-default.yml
@@ -0,0 +1,2 @@
+breaking_changes:
+- aws_s3 - the default value for ``ensure overwrite`` has been changed to ``different`` instead of ``always`` so that the module is idempotent by default (https://github.com/ansible-collections/amazon.aws/issues/811).
diff --git a/plugins/modules/s3_object.py b/plugins/modules/s3_object.py
@@ -132,7 +132,7 @@
         U(https://docs.aws.amazon.com/AmazonS3/latest/API/RESTCommonResponseHeaders.html).
       - When I(mode=get) and I(overwrite=latest) the last modified timestamp of local file
         is compared with the 'LastModified' of the object/key in S3.
-    default: 'always'
+    default: 'different'
     aliases: ['force']
     type: str
   retries:
@@ -228,7 +228,7 @@
 author:
   - "Lester Wade (@lwade)"
   - "Sloane Hertel (@s-hertel)"
-  - "Alina Buzachis (@linabuzachis)"
+  - "Alina Buzachis (@alinabuzachis)"
 notes:
   - Support for I(tags) and I(purge_tags) was added in release 2.0.0.
 extends_documentation_fragment:
@@ -954,7 +954,7 @@ def main():
         object=dict(),
         permission=dict(type='list', elements='str', default=['private']),
         version=dict(default=None),
-        overwrite=dict(aliases=['force'], default='always'),
+        overwrite=dict(aliases=['force'], default='different'),
         prefix=dict(default=""),
         retries=dict(aliases=['retry'], type='int', default=0),
         s3_url=dict(aliases=['S3_URL'], deprecated_aliases=[dict(name='S3_URL', version='5.0.0', collection_name='amazon.aws')]),

diff --git a/tests/integration/targets/s3_object/tasks/delete_bucket.yml b/tests/integration/targets/s3_object/tasks/delete_bucket.yml
@@ -15,10 +15,11 @@
       with_items: "{{ objects.s3_keys }}"
       loop_control:
         loop_var: obj
+      when: "'s3_keys' in objects"
       ignore_errors: true
 
     - name: delete the bucket
       s3_object:
         bucket: "{{ item }}"
         mode: delete
-      ignore_errors: yes
+      ignore_errors: true
diff --git a/tests/integration/targets/s3_object/tasks/main.yml b/tests/integration/targets/s3_object/tasks/main.yml
@@ -32,7 +32,7 @@
         bucket: "{{ bucket_name }}"
         mode: create
       register: result
-      ignore_errors: yes
+      ignore_errors: true
 
     - assert:
         that:
@@ -44,7 +44,7 @@
         bucket: "{{ bucket_name }}-"
         mode: create
       register: result
-      ignore_errors: yes
+      ignore_errors: true
 
     - assert:
         that:
@@ -111,15 +111,14 @@
         jid: "{{ test_async.ansible_job_id }}"
       register: status
       until: status is finished
-      retries: 10
+      retries: 30
 
     - name: test put with overwrite=different and unmodified object
       s3_object:
         bucket: "{{ bucket_name }}"
         mode: put
         src: "{{ tmpdir.path }}/upload.txt"
         object: delete.txt
-        overwrite: different
       retries: 3
       delay: 3
       register: result
@@ -234,7 +233,6 @@
         mode: get
         dest: "{{ tmpdir.path }}/download.txt"
         object: delete.txt
-        overwrite: different
       retries: 3
       delay: 3
       register: result
@@ -269,7 +267,6 @@
         mode: get
         dest: "{{ tmpdir.path }}/download.txt"
         object: delete.txt
-        overwrite: different
       retries: 3
       delay: 3
       register: result
@@ -609,18 +606,18 @@
         that:
           - bucket_with_policy is changed
 
-    - name: fail to upload the file to the bucket with an ACL
-      s3_object:
-        bucket: "{{ bucket_name_acl }}"
-        mode: put
-        src: "{{ tmpdir.path }}/upload.txt"
-        object: file-with-permissions.txt
-        permission: private
-        ignore_nonexistent_bucket: True
-      register: upload_private
-      ignore_errors: True
-
-    # XXX Doesn't fail...
+    # # XXX Doesn't fail...  ( ? Eventual consistency ? )
+    # - name: fail to upload the file to the bucket with an ACL
+    #   s3_object:
+    #     bucket: "{{ bucket_name_acl }}"
+    #     mode: put
+    #     src: "{{ tmpdir.path }}/upload.txt"
+    #     object: file-with-permissions.txt
+    #     permission: private
+    #     ignore_nonexistent_bucket: True
+    #   register: upload_private
+    #   ignore_errors: True
+    #
     # - assert:
     #     that:
     #       - upload_private is failed
@@ -657,7 +654,6 @@
         bucket: "{{ bucket_name }}"
         object: put-content.txt
         mode: put
-        overwrite: different
         content: >-
           test content
       register: result
@@ -692,6 +688,18 @@
         that:
           - result is changed
 
+    - name: create an object from a template (idempotency)
+      aws_s3:
+        bucket: "{{ bucket_name }}"
+        object: put-template.txt
+        mode: put
+        content: "{{ lookup('template', 'templates/put-template.txt.j2')|replace('\n', '') }}"
+      register: result
+
+    - assert:
+        that:
+          - result is not changed
+
     - name: fetch template content
       s3_object:
         bucket: "{{ bucket_name }}"
@@ -720,6 +728,18 @@
         that:
           - result is changed
 
+    - name: create an object from binary data (idempotency)
+      aws_s3:
+        bucket: "{{ bucket_name }}"
+        object: put-binary.bin
+        mode: put
+        content_base64: "{{ put_binary.content }}"
+      register: result
+
+    - assert:
+        that:
+          - result is not changed
+
     - name: fetch binary content
       s3_object:
         bucket: "{{ bucket_name }}"
@@ -772,7 +792,6 @@
           bucket: "{{ bucket_name }}"
           object: put-content.txt
           mode: put
-          overwrite: different
           content: >-
             test content
           tags:
@@ -814,7 +833,6 @@
           mode: put
           content: >-
             test content
-          overwrite: different
           tags:
             tag_one: '{{ resource_prefix }} One'
         register: result
@@ -832,12 +850,11 @@
           bucket: "{{ bucket_name }}"
           object: put-content.txt
           mode: put
-          overwrite: different
           content: >-
             test content
           tags:
             tag_three: '{{ resource_prefix }} Three'
-          purge_tags: no
+          purge_tags: false
         register: result
 
       - assert:
@@ -853,12 +870,11 @@
           bucket: "{{ bucket_name }}"
           object: put-content.txt
           mode: put
-          overwrite: different
           content: >-
             test content
           tags:
             tag_three: '{{ resource_prefix }} Three'
-          purge_tags: no
+          purge_tags: false
         register: result
 
       - assert:
@@ -874,12 +890,11 @@
           bucket: "{{ bucket_name }}"
           object: put-content.txt
           mode: put
-          overwrite: different
           content: >-
             test content
           tags:
             "TagFour": '{{ resource_prefix }} tag_four'
-          purge_tags: no
+          purge_tags: false
         register: result
 
       - assert:
@@ -896,12 +911,11 @@
           bucket: "{{ bucket_name }}"
           object: put-content.txt
           mode: put
-          overwrite: different
           content: >-
             test content
           tags:
             "TagFour": '{{ resource_prefix }} tag_four'
-          purge_tags: no
+          purge_tags: false
         register: result
 
       - assert:
@@ -918,11 +932,10 @@
           bucket: "{{ bucket_name }}"
           object: put-content.txt
           mode: put
-          overwrite: different
           content: >-
             test content
           tags: {}
-          purge_tags: no
+          purge_tags: false
         register: result
 
       - assert:
@@ -939,7 +952,6 @@
           bucket: "{{ bucket_name }}"
           object: put-content.txt
           mode: put
-          overwrite: different
           content: >-
             test content
         register: result
@@ -975,7 +987,6 @@
           bucket: "{{ bucket_name }}"
           object: put-content.txt
           mode: put
-          overwrite: different
           content: >-
             test content
           tags: {}
@@ -993,7 +1004,7 @@
       file:
         state: absent
         path: "{{ tmpdir.path }}"
-      ignore_errors: yes
+      ignore_errors: true
 
     - include_tasks: delete_bucket.yml
       with_items: