From d3f20694a90209d1dd39f27e528edfa7d170d429 Mon Sep 17 00:00:00 2001 From: Ivan Zubenko Date: Tue, 20 Apr 2021 14:50:05 +0300 Subject: [PATCH] sort Route53 resource record sets (#3864) * sort Route53 resource record sets * address comments --- moto/route53/models.py | 28 +++++++++------- moto/route53/responses.py | 4 +++ tests/test_route53/test_route53.py | 54 +++++++++++++++++++----------- 3 files changed, 54 insertions(+), 32 deletions(-) diff --git a/moto/route53/models.py b/moto/route53/models.py index f4303c2ae..8cb4520c1 100644 --- a/moto/route53/models.py +++ b/moto/route53/models.py @@ -1,5 +1,6 @@ from __future__ import unicode_literals +import itertools from collections import defaultdict import string @@ -264,20 +265,21 @@ class FakeZone(CloudFormationModel): ] def get_record_sets(self, start_type, start_name): - record_sets = list(self.rrsets) # Copy the list + def predicate(rrset): + rrset_name_reversed = reverse_domain_name(rrset.name) + start_name_reversed = reverse_domain_name(start_name) + return rrset_name_reversed < start_name_reversed or ( + rrset_name_reversed == start_name_reversed and rrset.type_ < start_type + ) + + record_sets = sorted( + self.rrsets, + key=lambda rrset: (reverse_domain_name(rrset.name), rrset.type_), + ) + if start_name: - record_sets = [ - record_set - for record_set in record_sets - if reverse_domain_name(record_set.name) - >= reverse_domain_name(start_name) - ] - if start_type: - record_sets = [ - record_set - for record_set in record_sets - if record_set.type_ >= start_type - ] + start_type = start_type or "" + record_sets = itertools.dropwhile(predicate, record_sets) return record_sets diff --git a/moto/route53/responses.py b/moto/route53/responses.py index f804fa302..7c3410791 100644 --- a/moto/route53/responses.py +++ b/moto/route53/responses.py @@ -166,6 +166,10 @@ class Route53(BaseResponse): template = Template(LIST_RRSET_RESPONSE) start_type = querystring.get("type", [None])[0] start_name = querystring.get("name", [None])[0] + + if start_type and not start_name: + return 400, headers, "The input is not valid" + record_sets = the_zone.get_record_sets(start_type, start_name) return 200, headers, template.render(record_sets=record_sets) diff --git a/tests/test_route53/test_route53.py b/tests/test_route53/test_route53.py index 135f411e5..f00af594e 100644 --- a/tests/test_route53/test_route53.py +++ b/tests/test_route53/test_route53.py @@ -43,23 +43,27 @@ def test_hosted_zone(): def test_rrset(): conn = boto.connect_route53("the_key", "the_secret") - conn.get_all_rrsets.when.called_with("abcd", type="A").should.throw( + conn.get_all_rrsets.when.called_with("abcd").should.throw( boto.route53.exception.DNSServerError, "404 Not Found" ) zone = conn.create_hosted_zone("testdns.aws.com") zoneid = zone["CreateHostedZoneResponse"]["HostedZone"]["Id"].split("/")[-1] + conn.get_all_rrsets.when.called_with(zoneid, type="A").should.throw( + boto.route53.exception.DNSServerError, "400 Bad Request" + ) + changes = ResourceRecordSets(conn, zoneid) change = changes.add_change("CREATE", "foo.bar.testdns.aws.com", "A") change.add_value("1.2.3.4") changes.commit() - rrsets = conn.get_all_rrsets(zoneid, type="A") + rrsets = conn.get_all_rrsets(zoneid) rrsets.should.have.length_of(1) rrsets[0].resource_records[0].should.equal("1.2.3.4") - rrsets = conn.get_all_rrsets(zoneid, type="CNAME") + rrsets = conn.get_all_rrsets(zoneid, name="foo.bar.testdns.aws.com.", type="CNAME") rrsets.should.have.length_of(0) changes = ResourceRecordSets(conn, zoneid) @@ -68,7 +72,7 @@ def test_rrset(): change.add_value("5.6.7.8") changes.commit() - rrsets = conn.get_all_rrsets(zoneid, type="A") + rrsets = conn.get_all_rrsets(zoneid) rrsets.should.have.length_of(1) rrsets[0].resource_records[0].should.equal("5.6.7.8") @@ -84,7 +88,7 @@ def test_rrset(): change.add_value("1.2.3.4") changes.commit() - rrsets = conn.get_all_rrsets(zoneid, type="A") + rrsets = conn.get_all_rrsets(zoneid) rrsets.should.have.length_of(1) rrsets[0].resource_records[0].should.equal("1.2.3.4") @@ -93,7 +97,7 @@ def test_rrset(): change.add_value("5.6.7.8") changes.commit() - rrsets = conn.get_all_rrsets(zoneid, type="A") + rrsets = conn.get_all_rrsets(zoneid) rrsets.should.have.length_of(1) rrsets[0].resource_records[0].should.equal("5.6.7.8") @@ -113,24 +117,36 @@ def test_rrset(): changes.commit() changes = ResourceRecordSets(conn, zoneid) + change = changes.add_change("CREATE", "bar.foo.testdns.aws.com", "TXT") + change.add_value("bar") change = changes.add_change("CREATE", "foo.bar.testdns.aws.com", "A") change.add_value("1.2.3.4") change = changes.add_change("CREATE", "bar.foo.testdns.aws.com", "A") change.add_value("5.6.7.8") changes.commit() - rrsets = conn.get_all_rrsets(zoneid, type="A") - rrsets.should.have.length_of(2) - - rrsets = conn.get_all_rrsets(zoneid, name="bar.foo.testdns.aws.com", type="A") - rrsets.should.have.length_of(1) - rrsets[0].resource_records[0].should.equal("5.6.7.8") + rrsets = conn.get_all_rrsets(zoneid) + rrsets.should.have.length_of(3) + rrsets[0].resource_records[0].should.equal("1.2.3.4") + rrsets[1].resource_records[0].should.equal("5.6.7.8") + rrsets[2].resource_records[0].should.equal("bar") rrsets = conn.get_all_rrsets(zoneid, name="foo.bar.testdns.aws.com", type="A") - rrsets.should.have.length_of(2) + rrsets.should.have.length_of(3) resource_records = [rr for rr_set in rrsets for rr in rr_set.resource_records] resource_records.should.contain("1.2.3.4") resource_records.should.contain("5.6.7.8") + resource_records.should.contain("bar") + + rrsets = conn.get_all_rrsets(zoneid, name="foo.testdns.aws.com", type="A") + rrsets.should.have.length_of(2) + rrsets[0].resource_records[0].should.equal("5.6.7.8") + rrsets[1].resource_records[0].should.equal("bar") + + rrsets = conn.get_all_rrsets(zoneid, name="foo.bar.testdns.aws.com", type="TXT") + rrsets.should.have.length_of(2) + rrsets[0].resource_records[0].should.equal("5.6.7.8") + rrsets[1].resource_records[0].should.equal("bar") rrsets = conn.get_all_rrsets(zoneid, name="foo.foo.testdns.aws.com", type="A") rrsets.should.have.length_of(0) @@ -148,7 +164,7 @@ def test_rrset_with_multiple_values(): change.add_value("5.6.7.8") changes.commit() - rrsets = conn.get_all_rrsets(zoneid, type="A") + rrsets = conn.get_all_rrsets(zoneid) rrsets.should.have.length_of(1) set(rrsets[0].resource_records).should.equal(set(["1.2.3.4", "5.6.7.8"])) @@ -176,17 +192,17 @@ def test_alias_rrset(): ) changes.commit() - rrsets = conn.get_all_rrsets(zoneid, type="A") + rrsets = conn.get_all_rrsets(zoneid, name="alias.testdns.aws.com") alias_targets = [rr_set.alias_dns_name for rr_set in rrsets] alias_targets.should.have.length_of(2) alias_targets.should.contain("foo.testdns.aws.com") alias_targets.should.contain("bar.testdns.aws.com") - rrsets[0].alias_dns_name.should.equal("foo.testdns.aws.com") - rrsets[0].resource_records.should.have.length_of(0) - rrsets = conn.get_all_rrsets(zoneid, type="CNAME") - rrsets.should.have.length_of(1) rrsets[0].alias_dns_name.should.equal("bar.testdns.aws.com") rrsets[0].resource_records.should.have.length_of(0) + rrsets = conn.get_all_rrsets(zoneid, name="foo.alias.testdns.aws.com", type="A") + rrsets.should.have.length_of(1) + rrsets[0].alias_dns_name.should.equal("foo.testdns.aws.com") + rrsets[0].resource_records.should.have.length_of(0) @mock_route53_deprecated