From b2300f1eae1323e3e8bc45f97e530ce129dff12e Mon Sep 17 00:00:00 2001 From: Bert Blommers Date: Sat, 10 Dec 2022 10:07:30 -0100 Subject: [PATCH] Route53: Validate deletion of RRsets (#5743) --- moto/route53/models.py | 31 +++++++- tests/test_route53/test_change_set_model.py | 58 ++++++++++++++ tests/test_route53/test_route53.py | 84 ++++++++++++++++----- 3 files changed, 154 insertions(+), 19 deletions(-) create mode 100644 tests/test_route53/test_change_set_model.py diff --git a/moto/route53/models.py b/moto/route53/models.py index a1e0fb5a7..f593373bf 100644 --- a/moto/route53/models.py +++ b/moto/route53/models.py @@ -20,6 +20,7 @@ from moto.route53.exceptions import ( QueryLoggingConfigAlreadyExists, DnsNameInvalidForZone, ChangeSetAlreadyExists, + InvalidInput, ) from moto.core import BaseBackend, BackendDict, BaseModel, CloudFormationModel from moto.moto_api._internal import mock_random as random @@ -262,6 +263,20 @@ def reverse_domain_name(domain_name): return ".".join(reversed(domain_name.split("."))) +class ChangeList(list): + """ + Contains a 'clean' list of ResourceRecordChangeSets + """ + + def append(self, item) -> None: + item["ResourceRecordSet"]["Name"] = item["ResourceRecordSet"]["Name"].strip(".") + super().append(item) + + def __contains__(self, item): + item["ResourceRecordSet"]["Name"] = item["ResourceRecordSet"]["Name"].strip(".") + return super().__contains__(item) + + class FakeZone(CloudFormationModel): def __init__( self, @@ -279,7 +294,7 @@ class FakeZone(CloudFormationModel): self.private_zone = private_zone self.rrsets = [] self.delegation_set = delegation_set - self.rr_changes = [] + self.rr_changes = ChangeList() def add_rrset(self, record_set): record_set = RecordSet(record_set) @@ -540,6 +555,20 @@ class Route53Backend(BaseBackend): action=rr["Action"], name=name, _type=_type ) + for value in change_list: + if value["Action"] == "DELETE": + # To delete a resource record set, you must specify all the same values that you specified when you created it. + corresponding_create = copy.deepcopy(value) + corresponding_create["Action"] = "CREATE" + corresponding_upsert = copy.deepcopy(value) + corresponding_upsert["Action"] = "UPSERT" + if ( + corresponding_create not in the_zone.rr_changes + and corresponding_upsert not in the_zone.rr_changes + ): + msg = f"Invalid request: Expected exactly one of [AliasTarget, all of [TTL, and ResourceRecords], or TrafficPolicyInstanceId], but found none in Change with [Action=DELETE, Name={value['ResourceRecordSet']['Name']}, Type={value['ResourceRecordSet']['Type']}, SetIdentifier={value['ResourceRecordSet'].get('SetIdentifier', 'null')}]" + raise InvalidInput(msg) + for value in change_list: original_change = copy.deepcopy(value) action = value["Action"] diff --git a/tests/test_route53/test_change_set_model.py b/tests/test_route53/test_change_set_model.py new file mode 100644 index 000000000..dcefef1b0 --- /dev/null +++ b/tests/test_route53/test_change_set_model.py @@ -0,0 +1,58 @@ +import copy +import sure # noqa # pylint: disable=unused-import +from moto.route53.models import ChangeList + + +def test_last_dot_in_name_is_ignored(): + change1 = { + "Action": "CREATE", + "ResourceRecordSet": { + "Name": "test.without.dot", + "Type": "CNAME", + "TTL": 600, + "ResourceRecords": [{"Value": "0.0.0.0"}], + }, + } + change1_alt = copy.deepcopy(change1) + change1_alt["ResourceRecordSet"]["Name"] = "test.without.dot." + + change2 = copy.deepcopy(change1) + change2["ResourceRecordSet"]["Name"] = "test.with.dot." + change2_alt = copy.deepcopy(change1) + change2_alt["ResourceRecordSet"]["Name"] = "test.with.dot" + + change_list = ChangeList() + change_list.append(change1) + change_list.append(change2) + + change_list.should.contain(change1) + change_list.should.contain(change1_alt) + change_list.should.contain(change2) + change_list.should.contain(change2_alt) + + +def test_last_dot_is_not_stored(): + change1 = { + "Action": "CREATE", + "ResourceRecordSet": { + "Name": "test.google.com.", + "Type": "CNAME", + "TTL": 600, + "ResourceRecords": [{"Value": "0.0.0.0"}], + }, + } + change_list = ChangeList() + change_list.append(change1) + + change_list[0]["ResourceRecordSet"]["Name"].should.equal("test.google.com") + + +def test_optional_fields(): + change = { + "Action": "UPSERT", + "ResourceRecordSet": {"Name": "test.google.com.", "Type": "CNAME"}, + } + change_list = ChangeList() + change_list.append(change) + + change_list.should.equal([change]) diff --git a/tests/test_route53/test_route53.py b/tests/test_route53/test_route53.py index 509b94b0d..6e4beb92c 100644 --- a/tests/test_route53/test_route53.py +++ b/tests/test_route53/test_route53.py @@ -308,6 +308,7 @@ def test_deleting_weighted_route(): "Name": "cname.testdns.aws.com", "Type": "CNAME", "SetIdentifier": "success-test-foo", + "Weight": 50, }, } ] @@ -374,6 +375,8 @@ def test_deleting_latency_route(): "Name": "cname.testdns.aws.com", "Type": "CNAME", "SetIdentifier": "success-test-foo", + "Region": "us-west-2", + "ResourceRecords": [{"Value": "example.com"}], }, } ] @@ -708,29 +711,18 @@ def test_change_resource_record_sets_crud_valid(): ) cname_alias_record_detail.should_not.contain("ResourceRecords") - # Delete record with wrong type. - delete_payload = { - "Comment": "delete prod.redis.db", - "Changes": [ - { - "Action": "DELETE", - "ResourceRecordSet": {"Name": "prod.redis.db", "Type": "CNAME"}, - } - ], - } - conn.change_resource_record_sets( - HostedZoneId=hosted_zone_id, ChangeBatch=delete_payload - ) - response = conn.list_resource_record_sets(HostedZoneId=hosted_zone_id) - len(response["ResourceRecordSets"]).should.equal(2) - # Delete record. delete_payload = { "Comment": "delete prod.redis.db", "Changes": [ { "Action": "DELETE", - "ResourceRecordSet": {"Name": "prod.redis.db", "Type": "A"}, + "ResourceRecordSet": { + "Name": "prod.redis.db.", + "Type": "A", + "TTL": 60, + "ResourceRecords": [{"Value": "192.168.1.1"}], + }, } ], } @@ -818,7 +810,12 @@ def test_change_resource_record_sets_crud_valid_with_special_xml_chars(): "Changes": [ { "Action": "DELETE", - "ResourceRecordSet": {"Name": "prod.redis.db", "Type": "TXT"}, + "ResourceRecordSet": { + "Name": "prod.redis.db.", + "Type": "TXT", + "TTL": 10, + "ResourceRecords": [{"Value": "SomeInitialValue"}], + }, } ], } @@ -829,6 +826,57 @@ def test_change_resource_record_sets_crud_valid_with_special_xml_chars(): len(response["ResourceRecordSets"]).should.equal(1) +@mock_route53 +def test_change_resource_record_set__delete_should_match_create(): + # To delete a resource record set, you must specify all the same values that you specified when you created it. + client = boto3.client("route53", region_name="us-east-1") + name = "example.com" + hosted_zone_id = client.create_hosted_zone(Name=name, CallerReference=name)[ + "HostedZone" + ]["Id"] + + create_call = client.change_resource_record_sets( + HostedZoneId=hosted_zone_id, + ChangeBatch={ + "Changes": [ + { + "Action": "CREATE", + "ResourceRecordSet": { + "Name": name, + "Type": "A", + "TTL": 300, + "ResourceRecords": [{"Value": "192.168.0.1"}], + }, + } + ] + }, + ) + waiter = client.get_waiter("resource_record_sets_changed") + waiter.wait(Id=create_call["ChangeInfo"]["Id"]) + + with pytest.raises(ClientError) as exc: + client.change_resource_record_sets( + HostedZoneId=hosted_zone_id, + ChangeBatch={ + "Changes": [ + { + "Action": "DELETE", + "ResourceRecordSet": { + "Name": name, + "Type": "A" + # Missing TTL and ResourceRecords + }, + } + ] + }, + ) + err = exc.value.response["Error"] + err["Code"].should.equal("InvalidInput") + err["Message"].should.equal( + "Invalid request: Expected exactly one of [AliasTarget, all of [TTL, and ResourceRecords], or TrafficPolicyInstanceId], but found none in Change with [Action=DELETE, Name=example.com, Type=A, SetIdentifier=null]" + ) + + @mock_route53 def test_change_weighted_resource_record_sets(): conn = boto3.client("route53", region_name="us-east-2")