From dcbef3afbafd978868b2652284b2b8961a35eadd Mon Sep 17 00:00:00 2001 From: mark-woolley Date: Fri, 12 Nov 2021 15:43:23 +0000 Subject: [PATCH 1/8] Apply max_items fix to route53_info --- plugins/modules/route53_info.py | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/plugins/modules/route53_info.py b/plugins/modules/route53_info.py index abdf7e44709..9176d339efd 100644 --- a/plugins/modules/route53_info.py +++ b/plugins/modules/route53_info.py @@ -72,7 +72,7 @@ description: - The type of DNS record. required: false - choices: [ 'A', 'CNAME', 'MX', 'AAAA', 'TXT', 'PTR', 'SRV', 'SPF', 'CAA', 'NS' ] + choices: [ 'SOA', 'A', 'TXT', 'NS', 'CNAME', 'MX', 'NAPTR', 'PTR', 'SRV', 'SPF', 'AAAA', 'CAA', 'DS' ] type: str dns_name: description: @@ -362,19 +362,23 @@ def record_sets_details(client, module): else: module.fail_json(msg="Hosted Zone Id is required") - if module.params.get('max_items'): - params['MaxItems'] = module.params.get('max_items') - if module.params.get('start_record_name'): params['StartRecordName'] = module.params.get('start_record_name') + # Check that both params are set if type is applied if module.params.get('type') and not module.params.get('start_record_name'): module.fail_json(msg="start_record_name must be specified if type is set") - elif module.params.get('type'): + + if module.params.get('type'): params['StartRecordType'] = module.params.get('type') + # Set PaginationConfig with max_items + if module.params.get('max_items'): + params['PaginationConfig']['MaxItems'] = module.params.get('max_items') + paginator = client.get_paginator('list_resource_record_sets') record_sets = paginator.paginate(**params).build_full_result()['ResourceRecordSets'] + return { "ResourceRecordSets": record_sets, "list": record_sets, @@ -424,8 +428,8 @@ def main(): next_marker=dict(), delegation_set_id=dict(), start_record_name=dict(), - type=dict(choices=[ - 'A', 'CNAME', 'MX', 'AAAA', 'TXT', 'PTR', 'SRV', 'SPF', 'CAA', 'NS' + type=dict(type='str', choices=[ + 'SOA', 'A', 'TXT', 'NS', 'CNAME', 'MX', 'NAPTR', 'PTR', 'SRV', 'SPF', 'AAAA', 'CAA', 'DS' ]), dns_name=dict(), resource_id=dict(type='list', aliases=['resource_ids'], elements='str'), From a4e4086d2544610c1268f29bd28b621ad9a44f90 Mon Sep 17 00:00:00 2001 From: mark-woolley Date: Fri, 12 Nov 2021 19:05:28 +0000 Subject: [PATCH 2/8] Add pagination config --- plugins/modules/route53_info.py | 9 ++++- .../targets/route53/tasks/main.yml | 39 ++++++++++++++++++- 2 files changed, 44 insertions(+), 4 deletions(-) diff --git a/plugins/modules/route53_info.py b/plugins/modules/route53_info.py index 9176d339efd..f2de32b583a 100644 --- a/plugins/modules/route53_info.py +++ b/plugins/modules/route53_info.py @@ -374,7 +374,12 @@ def record_sets_details(client, module): # Set PaginationConfig with max_items if module.params.get('max_items'): - params['PaginationConfig']['MaxItems'] = module.params.get('max_items') + pagination_config = dict( + PaginationConfig = dict( + MaxItems = int(module.params.get('max_items')) + ) + ) + params['PaginationConfig'] = pagination_config paginator = client.get_paginator('list_resource_record_sets') record_sets = paginator.paginate(**params).build_full_result()['ResourceRecordSets'] @@ -424,7 +429,7 @@ def main(): ], required=True), change_id=dict(), hosted_zone_id=dict(), - max_items=dict(), + max_items=dict(type='str'), next_marker=dict(), delegation_set_id=dict(), start_record_name=dict(), diff --git a/tests/integration/targets/route53/tasks/main.yml b/tests/integration/targets/route53/tasks/main.yml index f827387d83d..161ae43e6e3 100644 --- a/tests/integration/targets/route53/tasks/main.yml +++ b/tests/integration/targets/route53/tasks/main.yml @@ -164,6 +164,7 @@ record_set: '{{ get_result.set }}' qdn_record: 'qdn_test.{{ zone_one }}' + ## test A recordset creation and order adjustments - name: 'Create same A record using zone non-qualified domain' route53: state: present @@ -220,17 +221,18 @@ - mv_a_record is not failed - mv_a_record is not changed + # Get resulting A record and ensure max_items is applied - name: 'get Route53 A record information' route53_info: type: A query: record_sets hosted_zone_id: '{{ z1.zone_id }}' start_record_name: 'order_test.{{ zone_one }}' - max_items: 50 + max_items: 1 register: records - assert: that: - - records.ResourceRecordSets|length == 3 + - records.ResourceRecordSets|length == 1 - records.ResourceRecordSets[0].ResourceRecords|length == 2 - records.ResourceRecordSets[0].ResourceRecords[0].Value == '192.0.2.2' - records.ResourceRecordSets[0].ResourceRecords[1].Value == '192.0.2.1' @@ -261,6 +263,7 @@ - '192.0.2.2' register: del_a_record ignore_errors: true + - name: 'This should not fail, because `overwrite` is true' assert: that: @@ -281,6 +284,37 @@ - records.ResourceRecordSets[0].ResourceRecords|length == 1 - records.ResourceRecordSets[0].ResourceRecords[0].Value == '192.0.2.2' + ## Test CNAME record creation and retrive info + - name: "Create CNAME record" + route53: + state: present + zone: "{{ zone_one }}" + type: CNAME + record: "cname_test.{{ zone_one }}" + value: "order_test.{{ zone_one }}" + register: cname_record + + - assert: + that: + - cname_record is not failed + - cname_record is not changed + + - name: "Get Route53 CNAME record information" + route53_info: + type: CNAME + query: record_sets + hosted_zone_id: "{{ z1.zone_id }}" + start_record_name: "cname_test.{{ zone_one }}" + max_items: 1 + register: cname_records + + - assert: + that: + - cname_records.ResourceRecordSets|length == 3 + - cname_records.ResourceRecordSets[0].ResourceRecords|length == 1 + - cname_records.ResourceRecordSets[0].ResourceRecords[0].Value == "order_test.{{ zone_one }}" + + ## Test CAA record creation - name: 'Create a LetsEncrypt CAA record' route53: state: present @@ -604,6 +638,7 @@ query: record_sets hosted_zone_id: '{{ z1.zone_id }}' register: z1_records + - name: 'Loop over A/AAAA/CNAME Alias records and delete them' route53: state: absent From ce47d903b4155d4c6f98122e6a7bb89f6a6d381f Mon Sep 17 00:00:00 2001 From: mark-woolley Date: Mon, 29 Nov 2021 10:08:47 +0000 Subject: [PATCH 3/8] Add pagination config --- plugins/modules/route53_info.py | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/plugins/modules/route53_info.py b/plugins/modules/route53_info.py index f2de32b583a..20cd26a1c09 100644 --- a/plugins/modules/route53_info.py +++ b/plugins/modules/route53_info.py @@ -272,8 +272,14 @@ def list_hosted_zones_by_name(client, module): if module.params.get('dns_name'): params['DNSName'] = module.params.get('dns_name') + # Set PaginationConfig with max_items if module.params.get('max_items'): - params['MaxItems'] = module.params.get('max_items') + pagination_config = dict( + PaginationConfig = dict( + MaxItems = module.params.get('max_items') + ) + ) + params['PaginationConfig'] = pagination_config return client.list_hosted_zones_by_name(**params) @@ -340,12 +346,18 @@ def get_resource_tags(client, module): def list_health_checks(client, module): params = dict() - if module.params.get('max_items'): - params['MaxItems'] = module.params.get('max_items') - if module.params.get('next_marker'): params['Marker'] = module.params.get('next_marker') + # Set PaginationConfig with max_items + if module.params.get('max_items'): + pagination_config = dict( + PaginationConfig = dict( + MaxItems = module.params.get('max_items') + ) + ) + params['PaginationConfig'] = pagination_config + paginator = client.get_paginator('list_health_checks') health_checks = paginator.paginate(**params).build_full_result()['HealthChecks'] return { @@ -376,7 +388,7 @@ def record_sets_details(client, module): if module.params.get('max_items'): pagination_config = dict( PaginationConfig = dict( - MaxItems = int(module.params.get('max_items')) + MaxItems = module.params.get('max_items') ) ) params['PaginationConfig'] = pagination_config From f72e8bbe74a8b4912aba2e81f01a64023f67d834 Mon Sep 17 00:00:00 2001 From: mark-woolley Date: Mon, 29 Nov 2021 15:51:58 +0000 Subject: [PATCH 4/8] Fix tests for route53_info testing Update tests and add soft warning for boto3 version requirement remove boto requirement change Fix change made to delegation_set details --- plugins/modules/route53_info.py | 32 +++++++++---------- .../targets/route53/tasks/main.yml | 21 ++++++++---- 2 files changed, 30 insertions(+), 23 deletions(-) diff --git a/plugins/modules/route53_info.py b/plugins/modules/route53_info.py index 20cd26a1c09..87ed43af29d 100644 --- a/plugins/modules/route53_info.py +++ b/plugins/modules/route53_info.py @@ -228,9 +228,13 @@ def get_hosted_zone(client, module): def reusable_delegation_set_details(client, module): params = dict() + if not module.params.get('delegation_set_id'): + # Set PaginationConfig with max_items if module.params.get('max_items'): - params['MaxItems'] = module.params.get('max_items') + params['PaginationConfig'] = dict( + MaxItems = module.params.get('max_items') + ) if module.params.get('next_marker'): params['Marker'] = module.params.get('next_marker') @@ -246,8 +250,11 @@ def reusable_delegation_set_details(client, module): def list_hosted_zones(client, module): params = dict() + # Set PaginationConfig with max_items if module.params.get('max_items'): - params['MaxItems'] = module.params.get('max_items') + params['PaginationConfig'] = dict( + MaxItems = module.params.get('max_items') + ) if module.params.get('next_marker'): params['Marker'] = module.params.get('next_marker') @@ -274,12 +281,9 @@ def list_hosted_zones_by_name(client, module): # Set PaginationConfig with max_items if module.params.get('max_items'): - pagination_config = dict( - PaginationConfig = dict( - MaxItems = module.params.get('max_items') - ) + params['PaginationConfig'] = dict( + MaxItems = module.params.get('max_items') ) - params['PaginationConfig'] = pagination_config return client.list_hosted_zones_by_name(**params) @@ -351,12 +355,9 @@ def list_health_checks(client, module): # Set PaginationConfig with max_items if module.params.get('max_items'): - pagination_config = dict( - PaginationConfig = dict( - MaxItems = module.params.get('max_items') - ) + params['PaginationConfig'] = dict( + MaxItems = module.params.get('max_items') ) - params['PaginationConfig'] = pagination_config paginator = client.get_paginator('list_health_checks') health_checks = paginator.paginate(**params).build_full_result()['HealthChecks'] @@ -386,12 +387,9 @@ def record_sets_details(client, module): # Set PaginationConfig with max_items if module.params.get('max_items'): - pagination_config = dict( - PaginationConfig = dict( - MaxItems = module.params.get('max_items') - ) + params['PaginationConfig'] = dict( + MaxItems = module.params.get('max_items') ) - params['PaginationConfig'] = pagination_config paginator = client.get_paginator('list_resource_record_sets') record_sets = paginator.paginate(**params).build_full_result()['ResourceRecordSets'] diff --git a/tests/integration/targets/route53/tasks/main.yml b/tests/integration/targets/route53/tasks/main.yml index 161ae43e6e3..92f8601e64a 100644 --- a/tests/integration/targets/route53/tasks/main.yml +++ b/tests/integration/targets/route53/tasks/main.yml @@ -230,6 +230,7 @@ start_record_name: 'order_test.{{ zone_one }}' max_items: 1 register: records + - assert: that: - records.ResourceRecordSets|length == 1 @@ -278,6 +279,7 @@ start_record_name: 'order_test.{{ zone_one }}' max_items: 50 register: records + - assert: that: - records.ResourceRecordSets|length == 3 @@ -297,7 +299,7 @@ - assert: that: - cname_record is not failed - - cname_record is not changed + - cname_record is changed - name: "Get Route53 CNAME record information" route53_info: @@ -310,7 +312,7 @@ - assert: that: - - cname_records.ResourceRecordSets|length == 3 + - cname_records.ResourceRecordSets|length == 1 - cname_records.ResourceRecordSets[0].ResourceRecords|length == 1 - cname_records.ResourceRecordSets[0].ResourceRecords[0].Value == "order_test.{{ zone_one }}" @@ -423,7 +425,7 @@ - wc_a_record.diff.after == {} - name: create a record with different TTL - community.aws.route53: + route53: state: present zone: '{{ zone_one }}' record: 'localhost.{{ zone_one }}' @@ -438,7 +440,7 @@ - ttl30 is changed - name: delete previous record without mention ttl and value - community.aws.route53: + route53: state: absent zone: '{{ zone_one }}' record: 'localhost.{{ zone_one }}' @@ -450,7 +452,7 @@ - ttl30 is changed - name: immutable delete previous record without mention ttl and value - community.aws.route53: + route53: state: absent zone: '{{ zone_one }}' record: 'localhost.{{ zone_one }}' @@ -652,6 +654,7 @@ loop: '{{ z1_records.ResourceRecordSets | selectattr("Type", "in", ["A", "AAAA", "CNAME", "CAA"]) | list }}' when: - '"AliasTarget" in item' + - name: 'Loop over A/AAAA/CNAME records and delete them' route53: state: absent @@ -659,6 +662,7 @@ record: '{{ item.Name }}' type: '{{ item.Type }}' value: '{{ item.ResourceRecords | map(attribute="Value") | join(",") }}' + weight: '{{ item.Weight | default(omit) }}' identifier: '{{ item.SetIdentifier }}' region: '{{ omit }}' ignore_errors: True @@ -666,6 +670,7 @@ when: - '"ResourceRecords" in item' - '"SetIdentifier" in item' + - name: 'Loop over A/AAAA/CNAME records and delete them' route53: state: absent @@ -682,6 +687,7 @@ query: record_sets hosted_zone_id: '{{ z2.zone_id }}' register: z2_records + - name: 'Loop over A/AAAA/CNAME Alias records and delete them' route53: state: absent @@ -696,6 +702,7 @@ loop: '{{ z2_records.ResourceRecordSets | selectattr("Type", "in", ["A", "AAAA", "CNAME", "CAA"]) | list }}' when: - '"AliasTarget" in item' + - name: 'Loop over A/AAAA/CNAME records and delete them' route53: state: absent @@ -711,6 +718,7 @@ when: - '"ResourceRecords" in item' - '"SetIdentifier" in item' + - name: 'Loop over A/AAAA/CNAME records and delete them' route53: state: absent @@ -732,6 +740,7 @@ ignore_errors: yes retries: 10 until: delete_one is not failed + - name: 'Delete test zone two {{ zone_two }}' route53_zone: state: absent @@ -740,7 +749,7 @@ ignore_errors: yes retries: 10 until: delete_two is not failed - when: false + - name: destroy VPC ec2_vpc_net: cidr_block: 192.0.2.0/24 From c9a5f84234ad045ddc4359915bce58d5a0632b1e Mon Sep 17 00:00:00 2001 From: mark-woolley Date: Mon, 29 Nov 2021 18:12:08 +0000 Subject: [PATCH 5/8] add changelog --- changelogs/fragments/813-route53_info-fix-max_items.yml | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 changelogs/fragments/813-route53_info-fix-max_items.yml diff --git a/changelogs/fragments/813-route53_info-fix-max_items.yml b/changelogs/fragments/813-route53_info-fix-max_items.yml new file mode 100644 index 00000000000..8280f9a81d1 --- /dev/null +++ b/changelogs/fragments/813-route53_info-fix-max_items.yml @@ -0,0 +1,2 @@ +minor_changes: + - route53_info - `max_items` and `type` are no longer ignored fixing a regression (https://github.com/ansible-collections/community.aws/pull/813). From 8e2fb9edc0ab31fa3737a7c0e16ba35a1cd8325f Mon Sep 17 00:00:00 2001 From: mark-woolley Date: Mon, 29 Nov 2021 18:13:33 +0000 Subject: [PATCH 6/8] Fix indentation --- plugins/modules/route53_info.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/plugins/modules/route53_info.py b/plugins/modules/route53_info.py index 87ed43af29d..044f20d478f 100644 --- a/plugins/modules/route53_info.py +++ b/plugins/modules/route53_info.py @@ -233,7 +233,7 @@ def reusable_delegation_set_details(client, module): # Set PaginationConfig with max_items if module.params.get('max_items'): params['PaginationConfig'] = dict( - MaxItems = module.params.get('max_items') + MaxItems = module.params.get('max_items') ) if module.params.get('next_marker'): @@ -253,7 +253,7 @@ def list_hosted_zones(client, module): # Set PaginationConfig with max_items if module.params.get('max_items'): params['PaginationConfig'] = dict( - MaxItems = module.params.get('max_items') + MaxItems = module.params.get('max_items') ) if module.params.get('next_marker'): @@ -282,7 +282,7 @@ def list_hosted_zones_by_name(client, module): # Set PaginationConfig with max_items if module.params.get('max_items'): params['PaginationConfig'] = dict( - MaxItems = module.params.get('max_items') + MaxItems = module.params.get('max_items') ) return client.list_hosted_zones_by_name(**params) @@ -356,7 +356,7 @@ def list_health_checks(client, module): # Set PaginationConfig with max_items if module.params.get('max_items'): params['PaginationConfig'] = dict( - MaxItems = module.params.get('max_items') + MaxItems = module.params.get('max_items') ) paginator = client.get_paginator('list_health_checks') @@ -388,7 +388,7 @@ def record_sets_details(client, module): # Set PaginationConfig with max_items if module.params.get('max_items'): params['PaginationConfig'] = dict( - MaxItems = module.params.get('max_items') + MaxItems = module.params.get('max_items') ) paginator = client.get_paginator('list_resource_record_sets') From d280145f915f2c45b8701a7e36785c86ff726664 Mon Sep 17 00:00:00 2001 From: mark-woolley Date: Tue, 30 Nov 2021 09:15:16 +0000 Subject: [PATCH 7/8] lint fix and PR feedback --- plugins/modules/route53_info.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/plugins/modules/route53_info.py b/plugins/modules/route53_info.py index 044f20d478f..94d110a0444 100644 --- a/plugins/modules/route53_info.py +++ b/plugins/modules/route53_info.py @@ -72,7 +72,7 @@ description: - The type of DNS record. required: false - choices: [ 'SOA', 'A', 'TXT', 'NS', 'CNAME', 'MX', 'NAPTR', 'PTR', 'SRV', 'SPF', 'AAAA', 'CAA', 'DS' ] + choices: [ 'A', 'CNAME', 'MX', 'AAAA', 'TXT', 'PTR', 'SRV', 'SPF', 'CAA', 'NS', 'NAPTR', 'SOA', 'DS' ] type: str dns_name: description: @@ -233,7 +233,7 @@ def reusable_delegation_set_details(client, module): # Set PaginationConfig with max_items if module.params.get('max_items'): params['PaginationConfig'] = dict( - MaxItems = module.params.get('max_items') + MaxItems=module.params.get('max_items') ) if module.params.get('next_marker'): @@ -253,7 +253,7 @@ def list_hosted_zones(client, module): # Set PaginationConfig with max_items if module.params.get('max_items'): params['PaginationConfig'] = dict( - MaxItems = module.params.get('max_items') + MaxItems=module.params.get('max_items') ) if module.params.get('next_marker'): @@ -282,7 +282,7 @@ def list_hosted_zones_by_name(client, module): # Set PaginationConfig with max_items if module.params.get('max_items'): params['PaginationConfig'] = dict( - MaxItems = module.params.get('max_items') + MaxItems=module.params.get('max_items') ) return client.list_hosted_zones_by_name(**params) @@ -356,7 +356,7 @@ def list_health_checks(client, module): # Set PaginationConfig with max_items if module.params.get('max_items'): params['PaginationConfig'] = dict( - MaxItems = module.params.get('max_items') + MaxItems=module.params.get('max_items') ) paginator = client.get_paginator('list_health_checks') @@ -388,7 +388,7 @@ def record_sets_details(client, module): # Set PaginationConfig with max_items if module.params.get('max_items'): params['PaginationConfig'] = dict( - MaxItems = module.params.get('max_items') + MaxItems=module.params.get('max_items') ) paginator = client.get_paginator('list_resource_record_sets') @@ -444,7 +444,7 @@ def main(): delegation_set_id=dict(), start_record_name=dict(), type=dict(type='str', choices=[ - 'SOA', 'A', 'TXT', 'NS', 'CNAME', 'MX', 'NAPTR', 'PTR', 'SRV', 'SPF', 'AAAA', 'CAA', 'DS' + 'A', 'CNAME', 'MX', 'AAAA', 'TXT', 'PTR', 'SRV', 'SPF', 'CAA', 'NS', 'NAPTR', 'SOA', 'DS' ]), dns_name=dict(), resource_id=dict(type='list', aliases=['resource_ids'], elements='str'), From 55dd13b77e02ab277582ddc441deb3d47592c8fb Mon Sep 17 00:00:00 2001 From: mark-woolley Date: Tue, 30 Nov 2021 13:54:30 +0000 Subject: [PATCH 8/8] PR feedback --- plugins/modules/route53_info.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/modules/route53_info.py b/plugins/modules/route53_info.py index 94d110a0444..322ce7b0523 100644 --- a/plugins/modules/route53_info.py +++ b/plugins/modules/route53_info.py @@ -47,7 +47,7 @@ description: - Maximum number of items to return for various get/list requests. required: false - type: str + type: int next_marker: description: - "Some requests such as list_command: hosted_zones will return a maximum @@ -439,7 +439,7 @@ def main(): ], required=True), change_id=dict(), hosted_zone_id=dict(), - max_items=dict(type='str'), + max_items=dict(type='int'), next_marker=dict(), delegation_set_id=dict(), start_record_name=dict(),