Merge pull request #5672 from certbot/route53_acmev2v2

Version 2 of ACMEv2 support for Route53 plugin

certbot-dns-route53/certbot_dns_route53/dns_route53.py

@@ -1,4 +1,5 @@
 """Certbot Route53 authenticator plugin."""
+import collections
 import logging
 import time
 
@@ -33,6 +34,7 @@ class Authenticator(dns_common.DNSAuthenticator):
     def __init__(self, *args, **kwargs):
         super(Authenticator, self).__init__(*args, **kwargs)
         self.r53 = boto3.client("route53")
+        self._resource_records = collections.defaultdict(list)
 
     def more_info(self):  # pylint: disable=missing-docstring,no-self-use
         return "Solve a DNS01 challenge using AWS Route53"
@@ -88,6 +90,20 @@ class Authenticator(dns_common.DNSAuthenticator):
     def _change_txt_record(self, action, validation_domain_name, validation):
         zone_id = self._find_zone_id_for_domain(validation_domain_name)
 
+        rrecords = self._resource_records[validation_domain_name]
+        challenge = {"Value": '"{0}"'.format(validation)}
+        if action == "DELETE":
+            # Remove the record being deleted from the list of tracked records
+            rrecords.remove(challenge)
+            if rrecords:
+                # Need to update instead, as we're not deleting the rrset
+                action = "UPSERT"
+            else:
+                # Create a new list containing the record to use with DELETE
+                rrecords = [challenge]
+        else:
+            rrecords.append(challenge)
+
         response = self.r53.change_resource_record_sets(
             HostedZoneId=zone_id,
             ChangeBatch={
@@ -99,11 +115,7 @@ class Authenticator(dns_common.DNSAuthenticator):
                             "Name": validation_domain_name,
                             "Type": "TXT",
                             "TTL": self.ttl,
-                            "ResourceRecords": [
+                            "ResourceRecords": rrecords,
-                                # For some reason TXT records need to be
-                                # manually quoted.
-                                {"Value": '"{0}"'.format(validation)}
-                            ],
                         }
                     }
                 ]

certbot-dns-route53/certbot_dns_route53/dns_route53_test.py

@@ -186,6 +186,48 @@ class ClientTest(unittest.TestCase):
         call_count = self.client.r53.change_resource_record_sets.call_count
         self.assertEqual(call_count, 1)
 
+    def test_change_txt_record_delete(self):
+        self.client._find_zone_id_for_domain = mock.MagicMock()
+        self.client.r53.change_resource_record_sets = mock.MagicMock(
+            return_value={"ChangeInfo": {"Id": 1}})
+
+        validation = "some-value"
+        validation_record = {"Value": '"{0}"'.format(validation)}
+        self.client._resource_records[DOMAIN] = [validation_record]
+
+        self.client._change_txt_record("DELETE", DOMAIN, validation)
+
+        call_count = self.client.r53.change_resource_record_sets.call_count
+        self.assertEqual(call_count, 1)
+        call_args = self.client.r53.change_resource_record_sets.call_args_list[0][1]
+        call_args_batch = call_args["ChangeBatch"]["Changes"][0]
+        self.assertEqual(call_args_batch["Action"], "DELETE")
+        self.assertEqual(
+            call_args_batch["ResourceRecordSet"]["ResourceRecords"],
+            [validation_record])
+
+    def test_change_txt_record_multirecord(self):
+        self.client._find_zone_id_for_domain = mock.MagicMock()
+        self.client._get_validation_rrset = mock.MagicMock()
+        self.client._resource_records[DOMAIN] = [
+            {"Value": "\"pre-existing-value\""},
+            {"Value": "\"pre-existing-value-two\""},
+        ]
+        self.client.r53.change_resource_record_sets = mock.MagicMock(
+            return_value={"ChangeInfo": {"Id": 1}})
+
+        self.client._change_txt_record("DELETE", DOMAIN, "pre-existing-value")
+
+        call_count = self.client.r53.change_resource_record_sets.call_count
+        call_args = self.client.r53.change_resource_record_sets.call_args_list[0][1]
+        call_args_batch = call_args["ChangeBatch"]["Changes"][0]
+        self.assertEqual(call_args_batch["Action"], "UPSERT")
+        self.assertEqual(
+            call_args_batch["ResourceRecordSet"]["ResourceRecords"],
+            [{"Value": "\"pre-existing-value-two\""}])
+
+        self.assertEqual(call_count, 1)
+
     def test_wait_for_change(self):
         self.client.r53.get_change = mock.MagicMock(
             side_effect=[{"ChangeInfo": {"Status": "PENDING"}},