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..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,14 +268,72 @@ 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)
+ 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
+ # 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..f5b3d450b6 100644
--- a/libcloud/test/dns/test_route53.py
+++ b/libcloud/test/dns/test_route53.py
@@ -13,10 +13,12 @@
# See the License for the specific language governing permissions and
# limitations under the License.
+import re
import sys
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
@@ -284,6 +286,93 @@ 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_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]