Add botocore requirements to s3_bucket ownership control management (a…

…nsible-collections#450) Add botocore requirements to s3_bucket ownership control management SUMMARY (get|set|delete)_bucket_ownership_controls requires botocore >= 1.18.11 Because we state our minimum supported version of botocore is 1.16.0 we need to explicitly call this requirement for management of bucket ownership controls. ISSUE TYPE Bugfix Pull Request COMPONENT NAME s3_bucket ADDITIONAL INFORMATION fixes: ansible-collections#449 Reviewed-by: Alina Buzachis <None> Reviewed-by: None <None>
alinabuzachis · Aug 12, 2021 · e1b2d7b · e1b2d7b
1 parent 963a829
commit e1b2d7b
Show file tree

Hide file tree

Showing 3 changed files with 125 additions and 95 deletions.
diff --git a/plugins/modules/s3_bucket.py b/plugins/modules/s3_bucket.py
@@ -129,13 +129,15 @@
       - C(ObjectWriter) - The uploading account will own the object
         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.
+      - Management of bucket ownership controls requires botocore>=1.18.11.
     choices: [ 'BucketOwnerPreferred', 'ObjectWriter' ]
     type: str
     version_added: 2.0.0
   delete_object_ownership:
     description:
       - Delete bucket's ownership controls.
       - This option cannot be used together with a I(object_ownership) definition.
+      - Management of bucket ownership controls requires botocore>=1.18.11.
     default: false
     type: bool
     version_added: 2.0.0
@@ -769,6 +771,8 @@ def get_bucket_ownership_cntrl(s3_client, module, bucket_name):
     '''
     Get current bucket public access block
     '''
+    if not module.botocore_at_least('1.18.11'):
+        return None
     try:
         bucket_ownership = s3_client.get_bucket_ownership_controls(Bucket=bucket_name)
         return bucket_ownership['OwnershipControls']['Rules'][0]['ObjectOwnership']
@@ -948,6 +952,9 @@ def main():
     delete_object_ownership = module.params.get('delete_object_ownership')
     object_ownership = module.params.get('object_ownership')
 
+    if delete_object_ownership or object_ownership:
+        module.require_botocore_at_least('1.18.11', reason='to manipulate bucket ownership controls')
+
     # Parameter validation
     if encryption_key_id is not None and encryption != 'aws:kms':
         module.fail_json(msg="Only 'aws:kms' is a valid option for encryption parameter when you specify encryption_key_id.")

diff --git a/tests/integration/targets/s3_bucket/inventory b/tests/integration/targets/s3_bucket/inventory
@@ -1,4 +1,5 @@
 [tests]
+ownership_controls
 missing
 simple
 complex
@@ -7,7 +8,6 @@ tags
 encryption_kms
 encryption_sse
 public_access
-ownership_controls
 
 [all:vars]
 ansible_connection=local

diff --git a/tests/integration/targets/s3_bucket/roles/s3_bucket/tasks/ownership_controls.yml b/tests/integration/targets/s3_bucket/roles/s3_bucket/tasks/ownership_controls.yml
@@ -6,102 +6,121 @@
       security_token: "{{ security_token | default(omit) }}"
       region: "{{ aws_region }}"
   block:
-
-    # ============================================================
+    - pip:
+        name: virtualenv
     - set_fact:
-        local_bucket_name: "{{ bucket_name | hash('md5')}}ownership"
-
-    - name: 'Create a simple bucket bad value for ownership controls'
-      s3_bucket:
-        name: '{{ local_bucket_name }}'
-        state: present
-        object_ownership: default
-      ignore_errors: true
-      register: output
-
-    - assert:
-        that:
-          - output.failed
-
-    - name: 'Create bucket with object_ownership set to object_writer'
-      s3_bucket:
-        name: '{{ local_bucket_name }}'
-        state: present
-      ignore_errors: true
-      register: output
-
-    - assert:
-        that:
-          - output.changed
-          - not output.object_ownership|bool
-
-    - name: delete s3 bucket
-      s3_bucket:
-        name: '{{ local_bucket_name }}'
-        state: absent
-
-    - name: 'create s3 bucket with object ownership controls'
-      s3_bucket:
-        name: '{{ local_bucket_name }}'
-        state: present
-        object_ownership: ObjectWriter
-      register: output
-
-    - assert:
-        that:
-          - output.changed
-          - output.object_ownership
-          - output.object_ownership == 'ObjectWriter'
-
-    - name: 'update s3 bucket ownership controls'
-      s3_bucket:
-        name: '{{ local_bucket_name }}'
-        state: present
-        object_ownership: BucketOwnerPreferred
-      register: output
-
-    - assert:
-        that:
-          - output.changed
-          - output.object_ownership
-          - output.object_ownership == 'BucketOwnerPreferred'
-
-    - name: 'test idempotency update s3 bucket ownership controls'
-      s3_bucket:
-        name: '{{ local_bucket_name }}'
-        state: present
-        object_ownership: BucketOwnerPreferred
-      register: output
-
-    - assert:
-        that:
-          - output.changed is false
-          - output.object_ownership
-          - output.object_ownership == 'BucketOwnerPreferred'
-
-    - name: 'delete s3 bucket ownership controls'
-      s3_bucket:
-        name: '{{ local_bucket_name }}'
-        state: present
-        delete_object_ownership: true
-      register: output
-
-    - assert:
-        that:
-          - output.changed
-          - not output.object_ownership|bool
-
-    - name: 'delete s3 bucket ownership controls once again (idempotency)'
-      s3_bucket:
-        name: '{{ local_bucket_name }}'
-        state: present
-        delete_object_ownership: true
-      register: idempotency
+        virtualenv: "{{ remote_tmp_dir }}/virtualenv"
+        virtualenv_command: "{{ ansible_python_interpreter }} -m virtualenv"
+    - set_fact:
+        virtualenv_interpreter: "{{ virtualenv }}/bin/python"
+    - pip:
+        name:
+          - 'boto3>=1.13.0'
+          - 'botocore==1.18.11'
+          - 'coverage<5'
+        virtualenv: '{{ virtualenv }}'
+        virtualenv_command: '{{ virtualenv_command }}'
+        virtualenv_site_packages: no
 
-    - assert:
-        that:
-          - not idempotency.changed
-          - not idempotency.object_ownership|bool
+    # ============================================================
+    - name: Wrap test in virtualenv
+      vars:
+        ansible_python_interpreter: "{{ virtualenv }}/bin/python"
+      block:
+        - set_fact:
+            local_bucket_name: "{{ bucket_name | hash('md5')}}ownership"
+
+        - name: 'Create a simple bucket bad value for ownership controls'
+          s3_bucket:
+            name: '{{ local_bucket_name }}'
+            state: present
+            object_ownership: default
+          ignore_errors: true
+          register: output
+
+        - assert:
+            that:
+              - output.failed
+
+        - name: 'Create bucket with object_ownership set to object_writer'
+          s3_bucket:
+            name: '{{ local_bucket_name }}'
+            state: present
+          ignore_errors: true
+          register: output
+
+        - assert:
+            that:
+              - output.changed
+              - not output.object_ownership|bool
+
+        - name: delete s3 bucket
+          s3_bucket:
+            name: '{{ local_bucket_name }}'
+            state: absent
+
+        - name: 'create s3 bucket with object ownership controls'
+          s3_bucket:
+            name: '{{ local_bucket_name }}'
+            state: present
+            object_ownership: ObjectWriter
+          register: output
+
+        - assert:
+            that:
+              - output.changed
+              - output.object_ownership
+              - output.object_ownership == 'ObjectWriter'
+
+        - name: 'update s3 bucket ownership controls'
+          s3_bucket:
+            name: '{{ local_bucket_name }}'
+            state: present
+            object_ownership: BucketOwnerPreferred
+          register: output
+
+        - assert:
+            that:
+              - output.changed
+              - output.object_ownership
+              - output.object_ownership == 'BucketOwnerPreferred'
+
+        - name: 'test idempotency update s3 bucket ownership controls'
+          s3_bucket:
+            name: '{{ local_bucket_name }}'
+            state: present
+            object_ownership: BucketOwnerPreferred
+          register: output
+
+        - assert:
+            that:
+              - output.changed is false
+              - output.object_ownership
+              - output.object_ownership == 'BucketOwnerPreferred'
+
+        - name: 'delete s3 bucket ownership controls'
+          s3_bucket:
+            name: '{{ local_bucket_name }}'
+            state: present
+            delete_object_ownership: true
+          register: output
+
+        - assert:
+            that:
+              - output.changed
+              - not output.object_ownership|bool
+
+        - name: 'delete s3 bucket ownership controls once again (idempotency)'
+          s3_bucket:
+            name: '{{ local_bucket_name }}'
+            state: present
+            delete_object_ownership: true
+          register: idempotency
+
+        - assert:
+            that:
+              - not idempotency.changed
+              - not idempotency.object_ownership|bool
 
   # ============================================================
   always:
@@ -117,3 +136,7 @@
         name: '{{ local_bucket_name }}'
         state: absent
       ignore_errors: yes
+
+    - file:
+        path: "{{ virtualenv }}"
+        state: absent