Add purge_tags to s3_bucket to allow preservation of existing tags (#58754)

* Add purge_tags to s3_bucket to allow preservation of existing tags

Adding `purge_tags` with default `True` to maintain existing behaviour
allows users to set it to `False` to preserve existing tags

Fixes #29366

* s3_bucket: Add further tests and improve tag handling further

Additional tests for purge_tags: False suggested some incorrect
logic and thus further improvements

Increase wait timeout on bucket deletion as it wasn't always completing
in the default 100 seconds

changelogs/fragments/29366-allow-preservation-of-s3-bucket-tags.yml

@@ -0,0 +1,2 @@
+minor_changes:
+  - add purge_tags parameter to s3_bucket to allow preservation of existing tags when updating tags.

lib/ansible/modules/cloud/amazon/s3_bucket.py

@@ -68,6 +68,12 @@ options:
   tags:
     description:
       - tags dict to apply to bucket
+  purge_tags:
+    description:
+      - whether to remove tags that aren't present in the C(tags) parameter
+    type: bool
+    default: True
+    version_added: "2.9"
   versioning:
     description:
       - Whether versioning is enabled or disabled (note that once versioning is enabled, it can only be suspended)
@@ -152,6 +158,7 @@ def create_or_update_bucket(s3_client, module, location):
     name = module.params.get("name")
     requester_pays = module.params.get("requester_pays")
     tags = module.params.get("tags")
+    purge_tags = module.params.get("purge_tags")
     versioning = module.params.get("versioning")
     encryption = module.params.get("encryption")
     encryption_key_id = module.params.get("encryption_key_id")
@@ -276,6 +283,11 @@ def create_or_update_bucket(s3_client, module, location):
         if tags is not None:
             # Tags are always returned as text
             tags = dict((to_text(k), to_text(v)) for k, v in tags.items())
+            if not purge_tags:
+                # Ensure existing tags that aren't updated by desired tags remain
+                current_copy = current_tags_dict.copy()
+                current_copy.update(tags)
+                tags = current_copy
             if current_tags_dict != tags:
                 if tags:
                     try:
@@ -283,10 +295,11 @@ def create_or_update_bucket(s3_client, module, location):
                     except (BotoCoreError, ClientError) as e:
                         module.fail_json_aws(e, msg="Failed to update bucket tags")
                 else:
-                    try:
-                        delete_bucket_tagging(s3_client, name)
-                    except (BotoCoreError, ClientError) as e:
-                        module.fail_json_aws(e, msg="Failed to delete bucket tags")
+                    if purge_tags:
+                        try:
+                            delete_bucket_tagging(s3_client, name)
+                        except (BotoCoreError, ClientError) as e:
+                            module.fail_json_aws(e, msg="Failed to delete bucket tags")
                 current_tags_dict = wait_tags_are_applied(module, s3_client, name, tags)
                 changed = True
 
@@ -583,7 +596,7 @@ def destroy_bucket(s3_client, module):
 
     try:
         delete_bucket(s3_client, name)
-        s3_client.get_waiter('bucket_not_exists').wait(Bucket=name)
+        s3_client.get_waiter('bucket_not_exists').wait(Bucket=name, WaiterConfig=dict(Delay=5, MaxAttempts=60))
     except WaiterError as e:
         module.fail_json_aws(e, msg='An error occurred waiting for the bucket to be deleted.')
     except (BotoCoreError, ClientError) as e:
@@ -628,15 +641,16 @@ def main():
     argument_spec = ec2_argument_spec()
     argument_spec.update(
         dict(
-            force=dict(required=False, default='no', type='bool'),
-            policy=dict(required=False, default=None, type='json'),
-            name=dict(required=True, type='str'),
+            force=dict(default=False, type='bool'),
+            policy=dict(type='json'),
+            name=dict(required=True),
             requester_pays=dict(default=False, type='bool'),
-            s3_url=dict(aliases=['S3_URL'], type='str'),
-            state=dict(default='present', type='str', choices=['present', 'absent']),
-            tags=dict(required=False, default=None, type='dict'),
-            versioning=dict(default=None, type='bool'),
-            ceph=dict(default='no', type='bool'),
+            s3_url=dict(aliases=['S3_URL']),
+            state=dict(default='present', choices=['present', 'absent']),
+            tags=dict(type='dict'),
+            purge_tags=dict(type='bool', default=True),
+            versioning=dict(type='bool'),
+            ceph=dict(default=False, type='bool'),
             encryption=dict(choices=['none', 'AES256', 'aws:kms']),
             encryption_key_id=dict()
         )

test/integration/targets/s3_bucket/tasks/main.yml

@@ -6,10 +6,10 @@
     - name: set connection information for all tasks
       set_fact:
         aws_connection_info: &aws_connection_info
-          aws_access_key: "{{ aws_access_key }}"
-          aws_secret_key: "{{ aws_secret_key }}"
-          security_token: "{{ security_token }}"
-          region: "{{ aws_region }}"
+          aws_access_key: "{{ aws_access_key | default('') }}"
+          aws_secret_key: "{{ aws_secret_key | default('') }}"
+          security_token: "{{ security_token | default('') }}"
+          region: "{{ aws_region | default('') }}"
       no_log: true
 
     # ============================================================
@@ -191,6 +191,77 @@
       pause:
         seconds: 10
 
+    - name: Add a tag for s3_bucket with purge_tags False
+      s3_bucket:
+        name: "{{ resource_prefix }}-testbucket-ansible-complex"
+        state: present
+        policy: "{{ lookup('template','policy.json') }}"
+        requester_pays: no
+        versioning: no
+        purge_tags: no
+        tags:
+          anewtag: here
+        <<: *aws_connection_info
+      register: output
+
+    - assert:
+        that:
+          - output.changed
+          - output.tags.example == 'tag1-udpated'
+          - output.tags.anewtag == 'here'
+
+    # ============================================================
+    - name: Pause to help with s3 bucket eventual consistency
+      pause:
+        seconds: 10
+
+    - name: Update a tag for s3_bucket with purge_tags False
+      s3_bucket:
+        name: "{{ resource_prefix }}-testbucket-ansible-complex"
+        state: present
+        policy: "{{ lookup('template','policy.json') }}"
+        requester_pays: no
+        versioning: no
+        purge_tags: no
+        tags:
+          anewtag: next
+        <<: *aws_connection_info
+      register: output
+
+    - assert:
+        that:
+          - output.changed
+          - output.tags.example == 'tag1-udpated'
+          - output.tags.anewtag == 'next'
+
+    # ============================================================
+    - name: Pause to help with s3 bucket eventual consistency
+      pause:
+        seconds: 10
+
+    - name: Pass empty tags dict for s3_bucket with purge_tags False
+      s3_bucket:
+        name: "{{ resource_prefix }}-testbucket-ansible-complex"
+        state: present
+        policy: "{{ lookup('template','policy.json') }}"
+        requester_pays: no
+        versioning: no
+        purge_tags: no
+        tags: {}
+        <<: *aws_connection_info
+      register: output
+
+    - assert:
+        that:
+          - not output.changed
+          - output.tags.example == 'tag1-udpated'
+          - output.tags.anewtag == 'next'
+
+    # ============================================================
+    - name: Pause to help with s3 bucket eventual consistency
+      pause:
+        seconds: 10
+
     - name: Do not specify any tag to ensure previous tags are not removed
       s3_bucket:
         name: "{{ resource_prefix }}-testbucket-ansible-complex"