From 20f2afd75e506d90a72fb27e4cba3bb09f707494 Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Tue, 7 Jun 2022 09:38:08 +0200 Subject: [PATCH] aws_codebuild - Add resource_tags parameter and deprecate tags (#1221) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit aws_codebuild - Add resource_tags parameter and deprecate tags SUMMARY aws_codebuild currently uses the boto3 style 'list of dictionaries' format rather than the usual dictionary format. Add a resource_tags parameter that accepts the usual dictionary format Add the purge_tags parameter deprecate the tags parameter in preparation for switching it to the usual dict format expand integration tests for tags and description make source and artifacts optional unless creating a new project fix bug with inconsistent "changed" state due to tag order not being guaranteed ISSUE TYPE Bugfix Pull Request Feature Pull Request COMPONENT NAME aws_codebuild ADDITIONAL INFORMATION The (boto3) tags format in the return value when describing a project makes no guarantees about the order it'll return the key/value pairs. As such, when multiple tags were set the naïve original == new comparison would sporadically return "changed" when no change had occurred. Reviewed-by: Joseph Torcasso Reviewed-by: Mark Chappell --- .../fragments/1221-aws_codebuild-tagging.yml | 20 ++ plugins/modules/aws_codebuild.py | 104 +++++++- .../targets/aws_codebuild/defaults/main.yml | 1 + .../aws_codebuild/tasks/description.yml | 125 +++++++++ .../targets/aws_codebuild/tasks/main.yml | 9 +- .../targets/aws_codebuild/tasks/tagging.yml | 250 ++++++++++++++++++ 6 files changed, 495 insertions(+), 14 deletions(-) create mode 100644 changelogs/fragments/1221-aws_codebuild-tagging.yml create mode 100644 tests/integration/targets/aws_codebuild/tasks/description.yml create mode 100644 tests/integration/targets/aws_codebuild/tasks/tagging.yml diff --git a/changelogs/fragments/1221-aws_codebuild-tagging.yml b/changelogs/fragments/1221-aws_codebuild-tagging.yml new file mode 100644 index 00000000000..810bb3eb5ef --- /dev/null +++ b/changelogs/fragments/1221-aws_codebuild-tagging.yml @@ -0,0 +1,20 @@ +minor_changes: +- aws_codebuild - add support for ``purge_tags`` parameter + (https://github.com/ansible-collections/community.aws/pull/1221). +- aws_codebuild - the ``source`` and ``artifacts`` parameters are now optional unless creating a new project + (https://github.com/ansible-collections/community.aws/pull/1221). +- aws_codebuild - add the ``resource_tags`` parameter which takes the dictionary format for tags instead of the list of dictionaries format + (https://github.com/ansible-collections/community.aws/pull/1221). +- aws_codebuild - add the ``resource_tags`` return value which returns the standard dictionary format for tags instead of the list of + dictionaries format + (https://github.com/ansible-collections/community.aws/pull/1221). + +bugfixes: +- aws_codebuild - fix bug where the result may be spuriously flagged as ``changed`` when multiple tags were set on the project + (https://github.com/ansible-collections/community.aws/pull/1221). + +deprecated_features: +- aws_codebuild - The ``tags`` parameter currently uses a non-standard format and has been deprecated. In release 6.0.0 this parameter will + accept a simple key/value pair dictionary instead of the current list of dictionaries. It is recommended to migrate to using the + resource_tags parameter which already accepts the simple dictionary format + (https://github.com/ansible-collections/community.aws/pull/1221). diff --git a/plugins/modules/aws_codebuild.py b/plugins/modules/aws_codebuild.py index 9462d180e78..92e65ec1fe0 100644 --- a/plugins/modules/aws_codebuild.py +++ b/plugins/modules/aws_codebuild.py @@ -31,7 +31,7 @@ source: description: - Configure service and location for the build input source. - required: true + - I(source) is required when creating a new project. suboptions: type: description: @@ -58,7 +58,7 @@ artifacts: description: - Information about the build output artifacts for the build project. - required: true + - I(artifacts) is required when creating a new project. suboptions: type: description: @@ -137,6 +137,11 @@ tags: description: - A set of tags for the build project. + - Mutually exclusive with the I(resource_tags) parameter. + - In release 6.0.0 this parameter will accept a simple dictionary + instead of the list of dictionaries format. To use the simple + dictionary format prior to release 6.0.0 the I(resource_tags) can + be used instead of I(tags). type: list elements: dict suboptions: @@ -156,9 +161,30 @@ default: 'present' choices: ['present', 'absent'] type: str + resource_tags: + description: + - A dictionary representing the tags to be applied to the build project. + - If the I(resource_tags) parameter is not set then tags will not be modified. + - Mutually exclusive with the I(tags) parameter. + type: dict + required: false + purge_tags: + description: + - If I(purge_tags=true) and I(tags) is set, existing tags will be purged + from the resource to match exactly what is defined by I(tags) parameter. + - If the I(resource_tags) parameter is not set then tags will not be modified, even + if I(purge_tags=True). + - Tag keys beginning with C(aws:) are reserved by Amazon and can not be + modified. As such they will be ignored for the purposes of the + I(purge_tags) parameter. See the Amazon documentation for more information + U(https://docs.aws.amazon.com/general/latest/gr/aws_tagging.html#tag-conventions). + type: bool + default: true + required: false + extends_documentation_fragment: -- amazon.aws.aws -- amazon.aws.ec2 + - amazon.aws.aws + - amazon.aws.ec2 ''' @@ -275,9 +301,20 @@ type: int sample: 60 tags: - description: Tags added to the project + description: + - Tags added to the project in the boto3 list of dictionaries format. + - I(tags) and I(reource_tags) represent the same information in + different formats. returned: when configured type: list + reource_tags: + description: + - A simple dictionary representing the tags added to the project. + - I(tags) and I(reource_tags) represent the same information in + different formats. + returned: when configured + type: dict + version_added: 4.0.0 created: description: Timestamp of the create time of the project returned: always @@ -285,8 +322,13 @@ sample: "2018-04-17T16:56:03.245000+02:00" ''' -from ansible_collections.amazon.aws.plugins.module_utils.core import AnsibleAWSModule, get_boto3_client_method_parameters -from ansible_collections.amazon.aws.plugins.module_utils.ec2 import camel_dict_to_snake_dict, snake_dict_to_camel_dict +from ansible.module_utils.common.dict_transformations import camel_dict_to_snake_dict +from ansible.module_utils.common.dict_transformations import snake_dict_to_camel_dict + +from ansible_collections.amazon.aws.plugins.module_utils.core import AnsibleAWSModule +from ansible_collections.amazon.aws.plugins.module_utils.core import get_boto3_client_method_parameters +from ansible_collections.amazon.aws.plugins.module_utils.tagging import ansible_dict_to_boto3_tag_list +from ansible_collections.amazon.aws.plugins.module_utils.tagging import boto3_tag_list_to_ansible_dict try: @@ -312,20 +354,36 @@ def create_or_update_project(client, params, module): if 'name' in found: found_project = found + found_tags = found_project.pop('tags', []) + # Support tagging using a dict instead of the list of dicts + if params['resource_tags'] is not None: + if params['purge_tags']: + tags = dict() + else: + tags = boto3_tag_list_to_ansible_dict(found_tags) + tags.update(params['resource_tags']) + formatted_update_params['tags'] = ansible_dict_to_boto3_tag_list(tags, tag_name_key_name='key', tag_value_key_name='value') + resp = update_project(client=client, params=formatted_update_params, module=module) updated_project = resp['project'] # Prep both dicts for sensible change comparison: found_project.pop('lastModified') updated_project.pop('lastModified') - if 'tags' not in updated_project: - updated_project['tags'] = [] + updated_tags = updated_project.pop('tags', []) + found_project['ResourceTags'] = boto3_tag_list_to_ansible_dict(found_tags) + updated_project['ResourceTags'] = boto3_tag_list_to_ansible_dict(updated_tags) if updated_project != found_project: changed = True + updated_project['tags'] = updated_tags return resp, changed # Or create new project: try: + if params['source'] is None or params['artifacts'] is None: + module.fail_json( + "The source and artifacts parameters must be provided when " + "creating a new project. No existing project was found.") resp = client.create_project(**formatted_create_params) changed = True return resp, changed @@ -367,18 +425,30 @@ def describe_project(client, name, module): module.fail_json_aws(e, msg="Unable to describe CodeBuild projects") +def format_project_result(project_result): + formated_result = camel_dict_to_snake_dict(project_result) + project = project_result.get('project', {}) + if project: + tags = project.get('tags', []) + formated_result['project']['resource_tags'] = boto3_tag_list_to_ansible_dict(tags) + formated_result['ORIGINAL'] = project_result + return formated_result + + def main(): argument_spec = dict( name=dict(required=True), description=dict(), - source=dict(required=True, type='dict'), - artifacts=dict(required=True, type='dict'), + source=dict(type='dict'), + artifacts=dict(type='dict'), cache=dict(type='dict'), environment=dict(type='dict'), service_role=dict(), timeout_in_minutes=dict(type='int', default=60), encryption_key=dict(no_log=False), tags=dict(type='list', elements='dict'), + resource_tags=dict(type='dict'), + purge_tags=dict(type='bool', default=True), vpc_config=dict(type='dict'), state=dict(choices=['present', 'absent'], default='present') ) @@ -389,6 +459,15 @@ def main(): state = module.params.get('state') changed = False + if module.params['tags']: + module.deprecate( + 'The tags parameter currently uses a non-standard format and has ' + 'been deprecated. In release 6.0.0 this paramater will accept ' + 'a simple key/value pair dictionary instead of the current list ' + 'of dictionaries. It is recommended to migrate to using the ' + 'resource_tags parameter which already accepts the simple dictionary ' + 'format.', version='6.0.0', collection_name='community.aws') + if state == 'present': project_result, changed = create_or_update_project( client=client_conn, @@ -397,7 +476,8 @@ def main(): elif state == 'absent': project_result, changed = delete_project(client=client_conn, name=module.params['name'], module=module) - module.exit_json(changed=changed, **camel_dict_to_snake_dict(project_result)) + formatted_result = format_project_result(project_result) + module.exit_json(changed=changed, **formatted_result) if __name__ == '__main__': diff --git a/tests/integration/targets/aws_codebuild/defaults/main.yml b/tests/integration/targets/aws_codebuild/defaults/main.yml index da382aedf39..663264d5560 100644 --- a/tests/integration/targets/aws_codebuild/defaults/main.yml +++ b/tests/integration/targets/aws_codebuild/defaults/main.yml @@ -4,3 +4,4 @@ # IAM role names have to be less than 64 characters # we hash the resource_prefix to get a shorter, unique string iam_role_name: "ansible-test-{{ tiny_prefix }}-codebuild-service-role" +project_name: "{{ resource_prefix }}-codebuild" diff --git a/tests/integration/targets/aws_codebuild/tasks/description.yml b/tests/integration/targets/aws_codebuild/tasks/description.yml new file mode 100644 index 00000000000..13c12b5b639 --- /dev/null +++ b/tests/integration/targets/aws_codebuild/tasks/description.yml @@ -0,0 +1,125 @@ +- name: Tests relating to setting description on aws_codebuild + vars: + description_one: 'a Description - {{ resource_prefix }}' + description_two: 'Another_Description - {{ resource_prefix }}' + # Mandatory settings + module_defaults: + community.aws.aws_codebuild: + name: '{{ project_name }}' +# community.aws.aws_codebuild_info: +# name: '{{ project_name }}' + block: + +# - name: test setting description aws_codebuild (check mode) +# aws_codebuild: +# description: '{{ description_one }}' +# register: update_result +# check_mode: yes +# - name: assert that update succeeded +# assert: +# that: +# - update_result is changed + + - name: test setting description aws_codebuild + aws_codebuild: + description: '{{ description_one }}' + register: update_result + - name: assert that update succeeded + assert: + that: + - update_result is changed + - update_result.project.description == description_one + +# - name: test setting description aws_codebuild - idempotency (check mode) +# aws_codebuild: +# description: '{{ description_one }}' +# register: update_result +# check_mode: yes +# - name: assert that update succeeded +# assert: +# that: +# - update_result is not changed + + - name: test setting description aws_codebuild - idempotency + aws_codebuild: + description: '{{ description_one }}' + register: update_result + - name: assert that update succeeded + assert: + that: + - update_result is not changed + - update_result.project.description == description_one + + ### + +# - name: test updating description on aws_codebuild (check mode) +# aws_codebuild: +# description: '{{ description_two }}' +# register: update_result +# check_mode: yes +# - name: assert that update succeeded +# assert: +# that: +# - update_result is changed + + - name: test updating description on aws_codebuild + aws_codebuild: + description: '{{ description_two }}' + register: update_result + - name: assert that update succeeded + assert: + that: + - update_result is changed + - update_result.project.description == description_two + +# - name: test updating description on aws_codebuild - idempotency (check mode) +# aws_codebuild: +# description: '{{ description_two }}' +# register: update_result +# check_mode: yes +# - name: assert that update succeeded +# assert: +# that: +# - update_result is not changed + + - name: test updating description on aws_codebuild - idempotency + aws_codebuild: + description: '{{ description_two }}' + register: update_result + - name: assert that update succeeded + assert: + that: + - update_result is not changed + - update_result.project.description == description_two + +# ### +# +# - name: test that aws_codebuild_info returns the description +# aws_codebuild_info: +# register: tag_info +# - name: assert description present +# assert: +# that: +# - tag_info.project.description == description_two +# +# ### + +# - name: test no description param aws_codebuild (check mode) +# aws_codebuild: {} +# register: update_result +# check_mode: yes +# - name: assert no change +# assert: +# that: +# - update_result is not changed +# - update_result.project.description == description_two + + + - name: test no description param aws_codebuild + aws_codebuild: {} + register: update_result + - name: assert no change + assert: + that: + - update_result is not changed + - update_result.project.description == description_two diff --git a/tests/integration/targets/aws_codebuild/tasks/main.yml b/tests/integration/targets/aws_codebuild/tasks/main.yml index 43cb3490394..f674aba24eb 100644 --- a/tests/integration/targets/aws_codebuild/tasks/main.yml +++ b/tests/integration/targets/aws_codebuild/tasks/main.yml @@ -28,7 +28,7 @@ - name: create CodeBuild project aws_codebuild: - name: "{{ resource_prefix }}-test-ansible-codebuild" + name: "{{ project_name }}" description: Build project for testing the Ansible aws_codebuild module service_role: "{{ codebuild_iam_role.iam_role.arn }}" timeout_in_minutes: 30 @@ -58,10 +58,11 @@ - assert: that: - "output.project.description == 'Build project for testing the Ansible aws_codebuild module'" + - output.project.resource_tags.purpose == "ansible-test" - name: idempotence check rerunning same Codebuild task aws_codebuild: - name: "{{ resource_prefix }}-test-ansible-codebuild" + name: "{{ project_name }}" description: Build project for testing the Ansible aws_codebuild module service_role: "{{ codebuild_iam_role.iam_role.arn }}" timeout_in_minutes: 30 @@ -89,6 +90,10 @@ - assert: that: - "rerun_test_output.project.created == output.project.created" + - rerun_test_output.project.resource_tags.purpose == "ansible-test" + + - include_tasks: 'tagging.yml' + - include_tasks: 'description.yml' - name: delete CodeBuild project aws_codebuild: diff --git a/tests/integration/targets/aws_codebuild/tasks/tagging.yml b/tests/integration/targets/aws_codebuild/tasks/tagging.yml new file mode 100644 index 00000000000..a26f2a33708 --- /dev/null +++ b/tests/integration/targets/aws_codebuild/tasks/tagging.yml @@ -0,0 +1,250 @@ +- name: Tests relating to tagging aws_codebuild + vars: + first_tags: + 'Key with Spaces': Value with spaces + CamelCaseKey: CamelCaseValue + pascalCaseKey: pascalCaseValue + snake_case_key: snake_case_value + second_tags: + 'New Key with Spaces': Value with spaces + NewCamelCaseKey: CamelCaseValue + newPascalCaseKey: pascalCaseValue + new_snake_case_key: snake_case_value + third_tags: + 'Key with Spaces': Value with spaces + CamelCaseKey: CamelCaseValue + pascalCaseKey: pascalCaseValue + snake_case_key: snake_case_value + 'New Key with Spaces': Updated Value with spaces + final_tags: + 'Key with Spaces': Value with spaces + CamelCaseKey: CamelCaseValue + pascalCaseKey: pascalCaseValue + snake_case_key: snake_case_value + 'New Key with Spaces': Updated Value with spaces + NewCamelCaseKey: CamelCaseValue + newPascalCaseKey: pascalCaseValue + new_snake_case_key: snake_case_value + # Mandatory settings + module_defaults: + community.aws.aws_codebuild: + name: '{{ project_name }}' +# community.aws.aws_codebuild_info: +# name: '{{ project_name }}' + block: + + ### + +# - name: test adding tags to aws_codebuild (check mode) +# aws_codebuild: +# resource_tags: '{{ first_tags }}' +# purge_tags: True +# register: update_result +# check_mode: yes +# - name: assert that update succeeded +# assert: +# that: +# - update_result is changed + + - name: test adding tags to aws_codebuild + aws_codebuild: + resource_tags: '{{ first_tags }}' + purge_tags: True + register: update_result + - name: assert that update succeeded + assert: + that: + - update_result is changed + - update_result.project.resource_tags == first_tags + +# - name: test adding tags to aws_codebuild - idempotency (check mode) +# aws_codebuild: +# resource_tags: '{{ first_tags }}' +# purge_tags: True +# register: update_result +# check_mode: yes +# - name: assert that update succeeded +# assert: +# that: +# - update_result is not changed + + - name: test adding tags to aws_codebuild - idempotency + aws_codebuild: + resource_tags: '{{ first_tags }}' + purge_tags: True + register: update_result + - name: assert that update succeeded + assert: + that: + - update_result is not changed + - update_result.project.resource_tags == first_tags + + ### + +# - name: test updating tags with purge on aws_codebuild (check mode) +# aws_codebuild: +# resource_tags: '{{ second_tags }}' +# purge_tags: True +# register: update_result +# check_mode: yes +# - name: assert that update succeeded +# assert: +# that: +# - update_result is changed + + - name: test updating tags with purge on aws_codebuild + aws_codebuild: + resource_tags: '{{ second_tags }}' + purge_tags: True + register: update_result + - name: assert that update succeeded + assert: + that: + - update_result is changed + - update_result.project.resource_tags == second_tags + +# - name: test updating tags with purge on aws_codebuild - idempotency (check mode) +# aws_codebuild: +# resource_tags: '{{ second_tags }}' +# purge_tags: True +# register: update_result +# check_mode: yes +# - name: assert that update succeeded +# assert: +# that: +# - update_result is not changed + + - name: test updating tags with purge on aws_codebuild - idempotency + aws_codebuild: + resource_tags: '{{ second_tags }}' + purge_tags: True + register: update_result + - name: assert that update succeeded + assert: + that: + - update_result is not changed + - update_result.project.resource_tags == second_tags + + ### + +# - name: test updating tags without purge on aws_codebuild (check mode) +# aws_codebuild: +# resource_tags: '{{ third_tags }}' +# purge_tags: False +# register: update_result +# check_mode: yes +# - name: assert that update succeeded +# assert: +# that: +# - update_result is changed + + - name: test updating tags without purge on aws_codebuild + aws_codebuild: + resource_tags: '{{ third_tags }}' + purge_tags: False + register: update_result + - name: assert that update succeeded + assert: + that: + - update_result is changed + - update_result.project.resource_tags == final_tags + +# - name: test updating tags without purge on aws_codebuild - idempotency (check mode) +# aws_codebuild: +# resource_tags: '{{ third_tags }}' +# purge_tags: False +# register: update_result +# check_mode: yes +# - name: assert that update succeeded +# assert: +# that: +# - update_result is not changed + + - name: test updating tags without purge on aws_codebuild - idempotency + aws_codebuild: + resource_tags: '{{ third_tags }}' + purge_tags: False + register: update_result + - name: assert that update succeeded + assert: + that: + - update_result is not changed + - update_result.project.resource_tags == final_tags + +# ### +# +# - name: test that aws_codebuild_info returns the tags +# aws_codebuild_info: +# register: tag_info +# - name: assert tags present +# assert: +# that: +# - tag_info.project.tags == final_tags +# +# ### + +# - name: test no tags param aws_codebuild (check mode) +# aws_codebuild: {} +# register: update_result +# check_mode: yes +# - name: assert no change +# assert: +# that: +# - update_result is not changed +# - update_result.project.resource_tags == final_tags +# + + - name: test no tags param aws_codebuild + aws_codebuild: {} + register: update_result + - name: assert no change + assert: + that: + - update_result is not changed + - update_result.project.resource_tags == final_tags + + ### + +# - name: test removing tags from aws_codebuild (check mode) +# aws_codebuild: +# resource_tags: {} +# purge_tags: True +# register: update_result +# check_mode: yes +# - name: assert that update succeeded +# assert: +# that: +# - update_result is changed + + - name: test removing tags from aws_codebuild + aws_codebuild: + resource_tags: {} + purge_tags: True + register: update_result + - name: assert that update succeeded + assert: + that: + - update_result is changed + - update_result.project.resource_tags == {} + +# - name: test removing tags from aws_codebuild - idempotency (check mode) +# aws_codebuild: +# resource_tags: {} +# purge_tags: True +# register: update_result +# check_mode: yes +# - name: assert that update succeeded +# assert: +# that: +# - update_result is not changed + + - name: test removing tags from aws_codebuild - idempotency + aws_codebuild: + resource_tags: {} + purge_tags: True + register: update_result + - name: assert that update succeeded + assert: + that: + - update_result is not changed + - update_result.project.resource_tags == {}