From 7858f9fe4ef2d302501d1f6e4d448b93672d9ac0 Mon Sep 17 00:00:00 2001 From: Chirag Choudha Date: Wed, 15 Jun 2022 15:37:04 +0530 Subject: [PATCH] adding bucket key removing default values and correcting integration tests --- plugins/modules/s3_bucket.py | 62 ++++++++---- tests/integration/targets/s3_bucket/inventory | 1 + .../s3_bucket/tasks/encryption_bucket_key.yml | 94 +++++++++++++++++++ 3 files changed, 137 insertions(+), 20 deletions(-) create mode 100644 tests/integration/targets/s3_bucket/roles/s3_bucket/tasks/encryption_bucket_key.yml diff --git a/plugins/modules/s3_bucket.py b/plugins/modules/s3_bucket.py index 0797126220b..bbf3aeb70c1 100644 --- a/plugins/modules/s3_bucket.py +++ b/plugins/modules/s3_bucket.py @@ -85,12 +85,9 @@ not specified then it will default to the AWS provided KMS key. type: str encryption_bucket_key: - description: State of Bucket Key for SSE-KMS on new objects. This parameter is allowed if I(encryption) is C(aws:kms). If - not specified then it will default to absent. + description: Bucket Key for SSE-KMS. This parameter is allowed only if encryption is aws:kms otherwise it will throw error. required: false - default: absent - choices: [ 'present', 'absent' ] - type: str + type: bool public_access: description: @@ -225,7 +222,7 @@ # Create a bucket with aws:kms encryption, Bucket key - amazon.aws.s3_bucket: name: mys3bucket - encryption_bucket_key: present + encryption_bucket_key: true # Create a bucket with aws:kms encryption, default key - amazon.aws.s3_bucket: @@ -550,16 +547,19 @@ def create_or_update_bucket(s3_client, module, location): changed = True if encryption_bucket_key is not None: + current_encryption_algorithm = current_encryption.get('SSEAlgorithm') if current_encryption else None if current_encryption_algorithm == 'aws:kms': # current_bucket_key = current_encryption['ServerSideEncryptionConfiguration']['Rules'][0]['BucketKeyEnabled'] if current_encryption else None - current_bucket_key = get_bucket_key(s3_client, name) - if encryption_bucket_key: - expected_encryption = {'BucketKeyEnabled': True} - else: - expected_encryption = {'BucketKeyEnabled': False} - current_bucket_key = put_bucket_key_with_retry(module, s3_client, name, expected_encryption) - changed = True - # if current_bucket_key supplied param from ansible + # current_encryption = get_bucket_key(s3_client, name) + if get_bucket_key(s3_client, name) != encryption_bucket_key: + if encryption_bucket_key: + expected_encryption = True + else: + expected_encryption = False + current_encryption = put_bucket_key_with_retry(module, s3_client, name, expected_encryption) + changed = True + else: + module.fail_json_aws(current_encryption_algorithm, msg="encryption algorithm is not 'aws:kms' , it is ") @@ -771,10 +771,11 @@ def put_bucket_key_with_retry(module, s3_client, name, expected_encryption): put_bucket_key(s3_client, name, expected_encryption) except (botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError) as e: # pylint: disable=duplicate-except module.fail_json_aws(e, msg="Failed to set bucket Key") - current_encryption = wait_encryption_is_applied(module, s3_client, name, expected_encryption, + result = s3_client.get_bucket_encryption(Bucket=name) + current_encryption = wait_bucket_key_is_applied(module, s3_client, name, expected_encryption, should_fail=(retries == max_retries), retries=5) if current_encryption == expected_encryption: - return current_encryption + return result # We shouldn't get here, the only time this should happen is if # current_encryption != expected_encryption and retries == max_retries @@ -784,9 +785,14 @@ def put_bucket_key_with_retry(module, s3_client, name, expected_encryption): @AWSRetry.exponential_backoff(max_delay=120, catch_extra_error_codes=['NoSuchBucket', 'OperationAborted']) def put_bucket_key(s3_client, bucket_name, encryption): - server_side_encryption_configuration = {'Rules': [{'BucketKeyEnabled': encryption}]} - s3_client.put_bucket_encryption(Bucket=bucket_name, ServerSideEncryptionConfiguration=server_side_encryption_configuration) - + # server_side_encryption_configuration ={'Rules': [{'BucketKeyEnabled': encryption}]} + encryption_status = s3_client.get_bucket_encryption(Bucket=bucket_name) + encryption_status['ServerSideEncryptionConfiguration']['Rules'][0]['BucketKeyEnabled'] = encryption + s3_client.put_bucket_encryption( + Bucket=bucket_name, + ServerSideEncryptionConfiguration=encryption_status[ + 'ServerSideEncryptionConfiguration'] + ) @AWSRetry.exponential_backoff(max_delay=120, catch_extra_error_codes=['NoSuchBucket', 'OperationAborted']) def delete_bucket_tagging(s3_client, bucket_name): @@ -896,6 +902,22 @@ def wait_encryption_is_applied(module, s3_client, bucket_name, expected_encrypti return encryption +def wait_bucket_key_is_applied(module, s3_client, bucket_name, expected_encryption, should_fail=True, retries=12): + for dummy in range(0, retries): + try: + encryption = get_bucket_key(s3_client, bucket_name) + except (botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError) as e: + module.fail_json_aws(e, msg="Failed to get updated encryption for bucket") + if encryption!= expected_encryption: + time.sleep(5) + else: + return encryption + + if should_fail: + module.fail_json(msg="Bucket Key failed to apply in the expected time", + requested_encryption=expected_encryption, live_encryption=encryption) + + return encryption def wait_versioning_is_applied(module, s3_client, bucket_name, required_versioning): for dummy in range(0, 24): @@ -1071,7 +1093,7 @@ def main(): ceph=dict(default=False, type='bool'), encryption=dict(choices=['none', 'AES256', 'aws:kms']), encryption_key_id=dict(), - encryption_bucket_key=dict(type='bool', default=False), + encryption_bucket_key=dict(type='bool'), public_access=dict(type='dict', options=dict( block_public_acls=dict(type='bool', default=False), ignore_public_acls=dict(type='bool', default=False), diff --git a/tests/integration/targets/s3_bucket/inventory b/tests/integration/targets/s3_bucket/inventory index 93963af4c94..b79b5d6cc73 100644 --- a/tests/integration/targets/s3_bucket/inventory +++ b/tests/integration/targets/s3_bucket/inventory @@ -6,6 +6,7 @@ complex dotted tags encryption_kms +encryption_bucket_key encryption_sse public_access acl diff --git a/tests/integration/targets/s3_bucket/roles/s3_bucket/tasks/encryption_bucket_key.yml b/tests/integration/targets/s3_bucket/roles/s3_bucket/tasks/encryption_bucket_key.yml new file mode 100644 index 00000000000..37c6800ce98 --- /dev/null +++ b/tests/integration/targets/s3_bucket/roles/s3_bucket/tasks/encryption_bucket_key.yml @@ -0,0 +1,94 @@ +--- +- module_defaults: + group/aws: + aws_access_key: "{{ aws_access_key }}" + aws_secret_key: "{{ aws_secret_key }}" + security_token: "{{ security_token | default(omit) }}" + region: "{{ aws_region }}" + block: + - set_fact: + local_bucket_name: "{{ bucket_name | hash('md5')}}e-kms" + # ============================================================ + + - name: 'Create a simple bucket' + s3_bucket: + name: '{{ local_bucket_name }}' + state: present + register: output + + - name: 'Enable aws:kms encryption with KMS master key' + s3_bucket: + name: '{{ local_bucket_name }}' + state: present + encryption: "aws:kms" + register: output + + - name: 'Enable bucket key for bucket with aws:kms encryption' + s3_bucket: + name: '{{ local_bucket_name }}' + state: present + encryption: "aws:kms" + encryption_bucket_key: true + register: output + + - assert: + that: + - output.changed + - output.encryption + - output.encryption.SSEAlgorithm == 'aws:kms' + + - name: 'Re-enable bucket key for bucket with aws:kms encryption (idempotent)' + s3_bucket: + name: '{{ local_bucket_name }}' + encryption_bucket_key: true + register: output + + - assert: + that: + - not output.changed + - output.encryption + - output.encryption.SSEAlgorithm == 'aws:kms' + + # ============================================================ + + - name: Disable encryption from bucket + s3_bucket: + name: '{{ local_bucket_name }}' + encryption_bucket_key: true + register: output + + - assert: + that: + - output.changed + - not output.encryption + + - name: Disable encryption from bucket (idempotent) + s3_bucket: + name: '{{ local_bucket_name }}' + encryption_bucket_key: true + register: output + + - assert: + that: + - output is not changed + - not output.encryption + + # ============================================================ + + - name: Delete encryption test s3 bucket + s3_bucket: + name: '{{ local_bucket_name }}' + state: absent + register: output + + - assert: + that: + - output.changed + + # ============================================================ + always: + - name: Ensure all buckets are deleted + s3_bucket: + name: '{{ local_bucket_name }}' + state: absent + ignore_errors: yes