From 6676a7d7d5d8f8d03e3841cc564aa097d20c0fdd Mon Sep 17 00:00:00 2001 From: Sanjay Santhanam <51058514+Sanjays2402@users.noreply.github.com> Date: Thu, 30 Jul 2026 23:37:33 -0700 Subject: [PATCH 1/2] fix(route53): delete multi value records correctly delete_record built a DELETE changeset containing only the value of the record being deleted. Route53 requires a DELETE to list every value in the record set, so deleting any record which is part of a multi value set (for example one MX value) was rejected as an invalid change batch and surfaced as RecordDoesNotExistError. update_record already handles this via the _multi_value/_other_records metadata that _to_records attaches. delete_record now uses the same metadata and includes the values of the other records in the set. The priority prefix which _to_record strips out of MX/SRV values is restored so the emitted values match what Route53 stores. Adds a regression test asserting the full set of MX values is present in the emitted changeset. --- CHANGES.rst | 11 ++++++ libcloud/dns/drivers/route53.py | 62 ++++++++++++++++++++++++++++++- libcloud/test/dns/test_route53.py | 38 +++++++++++++++++++ 3 files changed, 109 insertions(+), 2 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 88b983e4bf..3596b58893 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -44,6 +44,17 @@ Storage (GITHUB-1698) [Sanjay Santhanam - @Sanjays2402] +DNS +~~~ + +- [Route53] Fix ``delete_record`` failing with ``RecordDoesNotExistError`` for + records which are part of a multi value record set (e.g. MX). Route53 only + accepts a DELETE changeset which lists every value in the record set, so the + values of the other records in the set are now included, mirroring how + ``update_record`` already handles multi value records. + (GITHUB-1831) + [Sanjay Santhanam - @Sanjays2402] + Changes in Apache Libcloud 3.9.1 -------------------------------- diff --git a/libcloud/dns/drivers/route53.py b/libcloud/dns/drivers/route53.py index 6140cc6cb6..585381bd60 100644 --- a/libcloud/dns/drivers/route53.py +++ b/libcloud/dns/drivers/route53.py @@ -237,13 +237,71 @@ def update_record(self, record, name=None, type=None, data=None, extra=None): def delete_record(self, record): try: r = record - batch = [("DELETE", r.name, r.type, r.data, r.extra)] - self._post_changeset(record.zone, batch) + + # Multiple value records need to be handled specially - Route53 + # only accepts a DELETE for a record set which lists every value + # in that set, so values for the other records need to be sent + # as well. + + if r.extra.get("_multi_value", False) and r.extra.get("_other_records", []): + self._delete_multi_value_record(record=r) + else: + batch = [("DELETE", r.name, r.type, r.data, r.extra)] + self._post_changeset(record.zone, batch) except InvalidChangeBatch: raise RecordDoesNotExistError(value="", driver=self, record_id=r.id) return True + def _delete_multi_value_record(self, record): + other_records = record.extra.get("_other_records", []) + + attrs = {"xmlns": NAMESPACE} + changeset = ET.Element("ChangeResourceRecordSetsRequest", attrs) + batch = ET.SubElement(changeset, "ChangeBatch") + changes = ET.SubElement(batch, "Changes") + + change = ET.SubElement(changes, "Change") + ET.SubElement(change, "Action").text = "DELETE" + + rrs = ET.SubElement(change, "ResourceRecordSet") + + if record.name: + record_name = record.name + "." + record.zone.domain + else: + record_name = record.zone.domain + + ET.SubElement(rrs, "Name").text = record_name + ET.SubElement(rrs, "Type").text = self.RECORD_TYPE_MAP[record.type] + ET.SubElement(rrs, "TTL").text = str(record.extra.get("ttl", "0")) + + rrecs = ET.SubElement(rrs, "ResourceRecords") + + rrec = ET.SubElement(rrecs, "ResourceRecord") + ET.SubElement(rrec, "Value").text = self._to_record_value(record.data, record.extra) + + for other_record in other_records: + rrec = ET.SubElement(rrecs, "ResourceRecord") + ET.SubElement(rrec, "Value").text = self._to_record_value( + other_record["data"], other_record.get("extra", {}) + ) + + uri = API_ROOT + "hostedzone/" + record.zone.id + "/rrset" + data = ET.tostring(changeset) + self.connection.set_context({"zone_id": record.zone.id}) + response = self.connection.request(uri, method="POST", data=data) + + return response.status == httplib.OK + + def _to_record_value(self, data, extra): + # "priority" is parsed out of the value by _to_record, so it needs to + # be put back to reconstruct the value Route53 stores. + + if extra and "priority" in extra: + return "{} {}".format(extra["priority"], data) + + return data + def ex_create_multi_value_record(self, name, zone, type, data, extra=None): """ Create a record with multiple values with a single call. diff --git a/libcloud/test/dns/test_route53.py b/libcloud/test/dns/test_route53.py index 2bc88d8870..1f7b56df10 100644 --- a/libcloud/test/dns/test_route53.py +++ b/libcloud/test/dns/test_route53.py @@ -13,6 +13,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import re import sys import unittest @@ -284,6 +285,43 @@ def test_delete_record(self): status = self.driver.delete_record(record=record) self.assertTrue(status) + def test_delete_multi_value_record(self): + zone = self.driver.list_zones()[0] + records = [r for r in self.driver.list_records(zone=zone) if r.type == RecordType.MX] + record = records[0] + + sent = {} + original_request = self.driver.connection.request + + def record_request(uri, *args, **kwargs): + if kwargs.get("method") == "POST": + sent["data"] = kwargs.get("data") + + return original_request(uri, *args, **kwargs) + + self.driver.connection.request = record_request + status = self.driver.delete_record(record=record) + self.assertTrue(status) + + data = sent["data"] + + if not isinstance(data, str): + data = data.decode("utf-8") + + # Route53 only accepts a DELETE which lists every value in the record + # set, so all the values need to be included in the changeset. + values = re.findall(r"(.*?)", data) + self.assertEqual( + values, + [ + "1 ASPMX.L.GOOGLE.COM.", + "5 ALT1.ASPMX.L.GOOGLE.COM.", + "5 ALT2.ASPMX.L.GOOGLE.COM.", + "10 ASPMX2.GOOGLEMAIL.COM.", + "10 ASPMX3.GOOGLEMAIL.COM.", + ], + ) + def test_delete_record_does_not_exist(self): zone = self.driver.list_zones()[0] record = self.driver.list_records(zone=zone)[0] From 6e2579e4534ec0a823dfbbaf9d32dc66f7adcab1 Mon Sep 17 00:00:00 2001 From: Sanjay Santhanam <51058514+Sanjays2402@users.noreply.github.com> Date: Fri, 31 Jul 2026 14:05:42 -0700 Subject: [PATCH 2/2] Route53: resolve record set metadata before multi value update/delete Records which did not come from list_records()/get_record() carry no _multi_value or _other_records metadata, so update_record() and delete_record() would build a changeset with a single value and Route53 would reject it. Look the record set up when the metadata is missing. --- libcloud/dns/drivers/route53.py | 34 ++++++++++++++++++++- libcloud/test/dns/test_route53.py | 51 +++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 1 deletion(-) diff --git a/libcloud/dns/drivers/route53.py b/libcloud/dns/drivers/route53.py index 585381bd60..67be669bff 100644 --- a/libcloud/dns/drivers/route53.py +++ b/libcloud/dns/drivers/route53.py @@ -199,7 +199,39 @@ def create_record(self, name, zone, type, data, extra=None): extra=extra, ) + def _with_record_set_metadata(self, record): + # ``_multi_value`` / ``_other_records`` are attached by ``_to_records``, + # so records which did not come from ``list_records`` / ``get_record`` + # (e.g. the ones returned by ``create_record`` or + # ``ex_create_multi_value_record``, or user constructed ones) carry no + # information about the rest of their record set. Re-fetch the record + # set in that case so multi value updates and deletes work regardless + # of how the record was obtained. + + if "_multi_value" in record.extra: + return record + + try: + fetched = self.list_records(zone=record.zone) + except Exception: + return record + + for candidate in fetched: + if ( + candidate.name == record.name + and candidate.type == record.type + and candidate.data == record.data + ): + extra = copy.deepcopy(candidate.extra) + extra.update({k: v for k, v in record.extra.items() if not k.startswith("_")}) + record.extra = extra + + break + + return record + def update_record(self, record, name=None, type=None, data=None, extra=None): + record = self._with_record_set_metadata(record) name = name or record.name type = type or record.type extra = extra or record.extra @@ -236,7 +268,7 @@ def update_record(self, record, name=None, type=None, data=None, extra=None): def delete_record(self, record): try: - r = record + r = self._with_record_set_metadata(record) # Multiple value records need to be handled specially - Route53 # only accepts a DELETE for a record set which lists every value diff --git a/libcloud/test/dns/test_route53.py b/libcloud/test/dns/test_route53.py index 1f7b56df10..f5b3d450b6 100644 --- a/libcloud/test/dns/test_route53.py +++ b/libcloud/test/dns/test_route53.py @@ -18,6 +18,7 @@ import unittest from libcloud.test import MockHttp +from libcloud.dns.base import Record from libcloud.dns.types import RecordType, ZoneDoesNotExistError, RecordDoesNotExistError from libcloud.utils.py3 import httplib from libcloud.test.secrets import DNS_PARAMS_ROUTE53 @@ -322,6 +323,56 @@ def record_request(uri, *args, **kwargs): ], ) + def test_delete_multi_value_record_without_record_set_metadata(self): + # Records which did not come from list_records()/get_record() (e.g. the + # ones returned by create_record()) carry no _multi_value metadata, so + # the record set has to be re-fetched for the DELETE to be valid. + zone = self.driver.list_zones()[0] + listed = [r for r in self.driver.list_records(zone=zone) if r.type == RecordType.MX][0] + + record = Record( + id=listed.id, + name=listed.name, + type=listed.type, + data=listed.data, + zone=zone, + driver=self.driver, + ttl=listed.extra.get("ttl"), + extra={"ttl": listed.extra.get("ttl"), "priority": listed.extra.get("priority")}, + ) + + sent = {} + original_request = self.driver.connection.request + + def record_request(uri, *args, **kwargs): + if kwargs.get("method") == "POST": + sent["data"] = kwargs.get("data") + + return original_request(uri, *args, **kwargs) + + self.driver.connection.request = record_request + status = self.driver.delete_record(record=record) + self.assertTrue(status) + + data = sent["data"] + + if not isinstance(data, str): + data = data.decode("utf-8") + + values = re.findall(r"(.*?)", data) + self.assertEqual( + sorted(values), + sorted( + [ + "1 ASPMX.L.GOOGLE.COM.", + "5 ALT1.ASPMX.L.GOOGLE.COM.", + "5 ALT2.ASPMX.L.GOOGLE.COM.", + "10 ASPMX2.GOOGLEMAIL.COM.", + "10 ASPMX3.GOOGLEMAIL.COM.", + ] + ), + ) + def test_delete_record_does_not_exist(self): zone = self.driver.list_zones()[0] record = self.driver.list_records(zone=zone)[0]