adding bucket key

removing default values and correcting integration tests

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),

tests/integration/targets/s3_bucket/inventory

@@ -6,6 +6,7 @@ complex
 dotted
 tags
 encryption_kms
+encryption_bucket_key
 encryption_sse
 public_access
 acl

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