From e1b2d7b4aaab5bd02c79119861082887d25df0c6 Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Fri, 13 Aug 2021 00:58:28 +0200 Subject: [PATCH] Add botocore requirements to s3_bucket ownership control management (#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: #449 Reviewed-by: Alina Buzachis Reviewed-by: None --- plugins/modules/s3_bucket.py | 7 + tests/integration/targets/s3_bucket/inventory | 2 +- .../s3_bucket/tasks/ownership_controls.yml | 211 ++++++++++-------- 3 files changed, 125 insertions(+), 95 deletions(-) diff --git a/plugins/modules/s3_bucket.py b/plugins/modules/s3_bucket.py index ab33dace01b..2dd5dc934a5 100644 --- a/plugins/modules/s3_bucket.py +++ b/plugins/modules/s3_bucket.py @@ -129,6 +129,7 @@ - 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 @@ -136,6 +137,7 @@ 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 index 62dcff0fbe7..08de6917968 100644 --- 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 index c651aac861e..4d68138804c 100644 --- 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