From 5d47c2c2255c2ac6713162bf713c094b75f05cb7 Mon Sep 17 00:00:00 2001 From: Alina Buzachis Date: Wed, 5 May 2021 17:04:08 +0200 Subject: [PATCH] * Tags fix and add missming integration tests Signed-off-by: Alina Buzachis --- plugins/modules/aws_s3.py | 97 +++---- .../integration/targets/aws_s3/tasks/main.yml | 238 +++++++++++++++++- 2 files changed, 286 insertions(+), 49 deletions(-) diff --git a/plugins/modules/aws_s3.py b/plugins/modules/aws_s3.py index 815291d0073..4768bfd9641 100644 --- a/plugins/modules/aws_s3.py +++ b/plugins/modules/aws_s3.py @@ -527,8 +527,7 @@ def option_in_extra_args(option): def upload_s3file(module, s3, bucket, obj, expiry, metadata, encrypt, headers, src=None, content=None): - tags = module.params.get("tags") - purge_tags = module.params.get("purge_tags") + result = {} if module.check_mode: module.exit_json(msg="PUT operation skipped - running in check mode", changed=True) @@ -574,35 +573,7 @@ def upload_s3file(module, s3, bucket, obj, expiry, metadata, encrypt, headers, s module.fail_json_aws(e, msg="Unable to set object ACL") # Tags - try: - current_tags_dict = get_current_object_tags_dict(s3, bucket, obj) - except is_boto3_error_code(IGNORE_S3_DROP_IN_EXCEPTIONS): - module.warn("GetObjectTagging is not implemented by your storage provider. Set the permission parameters to the empty list to avoid this warning.") - except (botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError) as e: # pylint: disable=duplicate-except - module.fail_json_aws(e, msg="Failed to get object tags.") - else: - 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: - put_object_tagging(s3, bucket, obj, tags) - except (botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError) as e: - module.fail_json_aws(e, msg="Failed to update object tags.") - else: - if purge_tags: - try: - delete_object_tagging(s3, bucket, obj) - except (botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError) as e: - module.fail_json_aws(e, msg="Failed to delete object tags.") - current_tags_dict = wait_tags_are_applied(module, s3, bucket, obj, tags) - changed = True + tags, changed = ensure_tags(s3, module, bucket, obj) try: url = s3.generate_presigned_url(ClientMethod='put_object', @@ -611,7 +582,7 @@ def upload_s3file(module, s3, bucket, obj, expiry, metadata, encrypt, headers, s except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: module.fail_json_aws(e, msg="Unable to generate presigned URL") - module.exit_json(msg="PUT operation complete", url=url, changed=True) + module.exit_json(msg="PUT operation complete", url=url, tags=tags, changed=True) def download_s3file(module, s3, bucket, obj, dest, retries, version=None): @@ -669,12 +640,12 @@ def download_s3str(module, s3, bucket, obj, version=None, validate=True): module.fail_json_aws(e, msg="Failed while getting contents of object %s as a string." % obj) -def get_download_url(module, s3, bucket, obj, expiry, changed=True): +def get_download_url(module, s3, bucket, obj, expiry, tags=None, changed=True): try: url = s3.generate_presigned_url(ClientMethod='get_object', Params={'Bucket': bucket, 'Key': obj}, ExpiresIn=expiry) - module.exit_json(msg="Download url:", url=url, expiry=expiry, changed=changed) + module.exit_json(msg="Download url:", url=url, tags=tags, expiry=expiry, changed=changed) except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: module.fail_json_aws(e, msg="Failed while getting download url.") @@ -720,9 +691,12 @@ def get_s3_connection(module, aws_connect_kwargs, location, rgw, s3_url, sig_4=F return boto3_conn(**params) -def get_current_object_tags_dict(s3, bucket, obj): +def get_current_object_tags_dict(s3, bucket, obj, version=None): try: - current_tags = s3.get_object_tagging(Bucket=bucket, Key=obj).get('TagSet') + if version: + current_tags = s3.get_object_tagging(Bucket=bucket, Key=obj, VersionId=version).get('TagSet') + else: + current_tags = s3.get_object_tagging(Bucket=bucket, Key=obj).get('TagSet') except is_boto3_error_code('NoSuchTagSet'): return {} except is_boto3_error_code('NoSuchTagSetError'): # pylint: disable=duplicate-except @@ -741,10 +715,10 @@ def delete_object_tagging(s3, bucket, obj): s3.delete_object_tagging(Bucket=bucket, Key=obj) -def wait_tags_are_applied(module, s3, bucket, obj, expected_tags_dict): +def wait_tags_are_applied(module, s3, bucket, obj, expected_tags_dict, version=None): for dummy in range(0, 12): try: - current_tags_dict = get_current_object_tags_dict(s3, bucket, obj) + current_tags_dict = get_current_object_tags_dict(s3, bucket, obj, version) except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: module.fail_json_aws(e, msg="Failed to get object tags.") if current_tags_dict != expected_tags_dict: @@ -756,6 +730,41 @@ def wait_tags_are_applied(module, s3, bucket, obj, expected_tags_dict): requested_tags=expected_tags_dict, live_tags=current_tags_dict) +def ensure_tags(client, module, bucket, obj): + tags = module.params.get("tags") + purge_tags = module.params.get("purge_tags") + changed = False + + try: + current_tags_dict = get_current_object_tags_dict(client, bucket, obj) + except is_boto3_error_code(IGNORE_S3_DROP_IN_EXCEPTIONS): + module.warn("GetObjectTagging is not implemented by your storage provider. Set the permission parameters to the empty list to avoid this warning.") + except (botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError) as e: # pylint: disable=duplicate-except + module.fail_json_aws(e, msg="Failed to get object tags.") + else: + if tags is not None: + 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: + put_object_tagging(client, bucket, obj, tags) + except (botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError) as e: + module.fail_json_aws(e, msg="Failed to update object tags.") + else: + if purge_tags: + try: + delete_object_tagging(client, bucket, obj) + except (botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError) as e: + module.fail_json_aws(e, msg="Failed to delete object tags.") + current_tags_dict = wait_tags_are_applied(module, client, bucket, obj, tags) + changed = True + return current_tags_dict, changed + + def main(): argument_spec = dict( bucket=dict(required=True), @@ -912,9 +921,9 @@ def main(): # these were separated into the variables bucket_acl and object_acl above if content is None and content_base64 is None and src is None: - module.fail_json('Either content, content_base64 or src must be specified for PUT operations') + module.fail_json(msg='Either content, content_base64 or src must be specified for PUT operations') if src is not None and not path_check(src): - module.fail_json('Local object "%s" does not exist for PUT operation' % (src)) + module.fail_json(msg='Local object "%s" does not exist for PUT operation' % (src)) keyrtn = None if bucketrtn: @@ -934,8 +943,9 @@ def main(): if keyrtn and overwrite != 'always': if overwrite == 'never' or etag_compare(module, s3, bucket, obj, version=version, local_file=src, content=bincontent): - # Return the download URL for the existing object - get_download_url(module, s3, bucket, obj, expiry, changed=False) + # Return the download URL for the existing object and ensure tags are updated + tags, tags_update = ensure_tags(s3, module, bucket, obj) + get_download_url(module, s3, bucket, obj, expiry, tags, changed=tags_update) # only use valid object acls for the upload_s3file function module.params['permission'] = object_acl @@ -1012,7 +1022,8 @@ def main(): keyrtn = key_check(module, s3, bucket, obj, version=version, validate=validate) if keyrtn: - get_download_url(module, s3, bucket, obj, expiry) + tags = get_current_object_tags_dict(s3, bucket, obj, version=version) + get_download_url(module, s3, bucket, obj, expiry, tags) else: module.fail_json(msg="Key %s does not exist." % obj) diff --git a/tests/integration/targets/aws_s3/tasks/main.yml b/tests/integration/targets/aws_s3/tasks/main.yml index aa6166a4e3e..54af80d1d50 100644 --- a/tests/integration/targets/aws_s3/tasks/main.yml +++ b/tests/integration/targets/aws_s3/tasks/main.yml @@ -633,21 +633,247 @@ block: # ============================================================ - - name: Test putting an object in the bucket and assign tags + + - name: create an object from static content aws_s3: bucket: "{{ bucket_name }}" + object: put-content.txt mode: put - src: "{{ tmpdir.path }}/upload.txt" - object: delete.txt + content: >- + test content tags: tag_one: '{{ resource_prefix }} One' "Tag Two": 'two {{ resource_prefix }}' - register: output - + register: result + + - assert: + that: + - result is changed + - "'tags' in result" + - (result.tags | length) == 2 + - result.tags["tag_one"] == '{{ resource_prefix }} One' + - result.tags["Tag Two"] == 'two {{ resource_prefix }}' + + - name: ensure idempotency on static content + aws_s3: + bucket: "{{ bucket_name }}" + object: put-content.txt + mode: put + overwrite: different + content: >- + test content + tags: + tag_one: '{{ resource_prefix }} One' + "Tag Two": 'two {{ resource_prefix }}' + register: result + + - assert: + that: + - result is not changed + - "'tags' in result" + - (result.tags | length) == 2 + - result.tags["tag_one"] == '{{ resource_prefix }} One' + - result.tags["Tag Two"] == 'two {{ resource_prefix }}' + + - name: Remove a tag from an S3 object + aws_s3: + bucket: "{{ bucket_name }}" + object: put-content.txt + mode: put + content: >- + test content + tags: + tag_one: '{{ resource_prefix }} One' + register: result + + - assert: + that: + - result is changed + - "'tags' in result" + - (result.tags | length) == 1 + - result.tags["tag_one"] == "{{ resource_prefix }} One" + - "'Tag Two' not in result.tags" + + - name: Remove the tag from an S3 object (idempotency) + aws_s3: + bucket: "{{ bucket_name }}" + object: put-content.txt + mode: put + content: >- + test content + overwrite: different + tags: + tag_one: '{{ resource_prefix }} One' + register: result + + - assert: + that: + - result is not changed + - "'tags' in result" + - (result.tags | length) == 1 + - result.tags["tag_one"] == "{{ resource_prefix }} One" + - "'Tag Two' not in result.tags" + + - name: Add a tag for an S3 object with purge_tags False + aws_s3: + bucket: "{{ bucket_name }}" + object: put-content.txt + mode: put + overwrite: different + content: >- + test content + tags: + tag_three: '{{ resource_prefix }} Three' + purge_tags: no + register: result + + - assert: + that: + - result is changed + - "'tags' in result" + - (result.tags | length) == 2 + - result.tags["tag_three"] == '{{ resource_prefix }} Three' + - result.tags["tag_one"] == '{{ resource_prefix }} One' + + - name: Add a tag for an S3 object with purge_tags False (idempotency) + aws_s3: + bucket: "{{ bucket_name }}" + object: put-content.txt + mode: put + overwrite: different + content: >- + test content + tags: + tag_three: '{{ resource_prefix }} Three' + purge_tags: no + register: result + + - assert: + that: + - result is not changed + - "'tags' in result" + - (result.tags | length) == 2 + - result.tags["tag_three"] == '{{ resource_prefix }} Three' + - result.tags["tag_one"] == '{{ resource_prefix }} One' + + - name: Update tags for an S3 object with purge_tags False + aws_s3: + bucket: "{{ bucket_name }}" + object: put-content.txt + mode: put + overwrite: different + content: >- + test content + tags: + "TagFour": '{{ resource_prefix }} tag_four' + purge_tags: no + register: result + - assert: that: - - output.changed + - result is changed + - "'tags' in result" + - (result.tags | length) == 3 + - result.tags["tag_one"] == '{{ resource_prefix }} One' + - result.tags["tag_three"] == '{{ resource_prefix }} Three' + - result.tags["TagFour"] == '{{ resource_prefix }} tag_four' + + - name: Update tags for an S3 object with purge_tags False (idempotency) + aws_s3: + bucket: "{{ bucket_name }}" + object: put-content.txt + mode: put + overwrite: different + content: >- + test content + tags: + "TagFour": '{{ resource_prefix }} tag_four' + purge_tags: no + register: result + + - assert: + that: + - result is not changed + - "'tags' in result" + - (result.tags | length) == 3 + - result.tags["tag_one"] == '{{ resource_prefix }} One' + - result.tags["tag_three"] == '{{ resource_prefix }} Three' + - result.tags["TagFour"] == '{{ resource_prefix }} tag_four' + + - name: Specify empty tags for an S3 object with purge_tags False + aws_s3: + bucket: "{{ bucket_name }}" + object: put-content.txt + mode: put + overwrite: different + content: >- + test content + tags: {} + purge_tags: no + register: result + - assert: + that: + - result is not changed + - "'tags' in result" + - (result.tags | length) == 3 + - result.tags["tag_one"] == '{{ resource_prefix }} One' + - result.tags["tag_three"] == '{{ resource_prefix }} Three' + - result.tags["TagFour"] == '{{ resource_prefix }} tag_four' + + - name: Do not specify any tag to ensure previous tags are not removed + aws_s3: + bucket: "{{ bucket_name }}" + object: put-content.txt + mode: put + overwrite: different + content: >- + test content + register: result + + - assert: + that: + - result is not changed + - "'tags' in result" + - (result.tags | length) == 3 + - result.tags["tag_one"] == '{{ resource_prefix }} One' + - result.tags["tag_three"] == '{{ resource_prefix }} Three' + - result.tags["TagFour"] == '{{ resource_prefix }} tag_four' + + - name: Remove all tags + aws_s3: + bucket: "{{ bucket_name }}" + object: put-content.txt + mode: put + overwrite: different + content: >- + test content + tags: {} + register: result + + - assert: + that: + - result is changed + - "'tags' in result" + - (result.tags | length) == 0 + + - name: Remove all tags (idempotency) + aws_s3: + bucket: "{{ bucket_name }}" + object: put-content.txt + mode: put + overwrite: different + content: >- + test content + tags: {} + register: result + + - assert: + that: + - result is not changed + - "'tags' in result" + - (result.tags | length) == 0 + always: - name: remove uploaded files aws_s3: