diff --git a/certbot-dns-google/certbot_dns_google/dns_google.py b/certbot-dns-google/certbot_dns_google/dns_google.py index cea754c06..ab8bf20de 100644 --- a/certbot-dns-google/certbot_dns_google/dns_google.py +++ b/certbot-dns-google/certbot_dns_google/dns_google.py @@ -107,6 +107,15 @@ class _GoogleClient(object): zone_id = self._find_managed_zone_id(domain) + record_contents = self.get_existing_txt_rrset(zone_id, record_name) + add_records = record_contents[:] + + if "\""+record_content+"\"" in record_contents: + # The process was interrupted previously and validation token exists + return + + add_records.append(record_content) + data = { "kind": "dns#change", "additions": [ @@ -114,12 +123,24 @@ class _GoogleClient(object): "kind": "dns#resourceRecordSet", "type": "TXT", "name": record_name + ".", - "rrdatas": [record_content, ], + "rrdatas": add_records, "ttl": record_ttl, }, ], } + if record_contents: + # We need to remove old records in the same request + data["deletions"] = [ + { + "kind": "dns#resourceRecordSet", + "type": "TXT", + "name": record_name + ".", + "rrdatas": record_contents, + "ttl": record_ttl, + }, + ] + changes = self.dns.changes() # changes | pylint: disable=no-member try: @@ -154,6 +175,8 @@ class _GoogleClient(object): logger.warn('Error finding zone. Skipping cleanup.') return + record_contents = self.get_existing_txt_rrset(zone_id, record_name) + data = { "kind": "dns#change", "deletions": [ @@ -161,12 +184,26 @@ class _GoogleClient(object): "kind": "dns#resourceRecordSet", "type": "TXT", "name": record_name + ".", - "rrdatas": [record_content, ], + "rrdatas": record_contents, "ttl": record_ttl, }, ], } + # Remove the record being deleted from the list + readd_contents = [r for r in record_contents if r != "\"" + record_content + "\""] + if readd_contents: + # We need to remove old records in the same request + data["additions"] = [ + { + "kind": "dns#resourceRecordSet", + "type": "TXT", + "name": record_name + ".", + "rrdatas": readd_contents, + "ttl": record_ttl, + }, + ] + changes = self.dns.changes() # changes | pylint: disable=no-member try: @@ -175,6 +212,28 @@ class _GoogleClient(object): except googleapiclient_errors.Error as e: logger.warn('Encountered error deleting TXT record: %s', e) + def get_existing_txt_rrset(self, zone_id, record_name): + """ + Get existing TXT records from the RRset for the record name. + + :param str zone_id: The ID of the managed zone. + :param str record_name: The record name (typically beginning with '_acme-challenge.'). + + :returns: List of TXT record values + :rtype: `list` of `string` + + """ + rrs_request = self.dns.resourceRecordSets() # pylint: disable=no-member + request = rrs_request.list(managedZone=zone_id, project=self.project_id) + response = request.execute() + # Add dot as the API returns absolute domains + record_name += "." + if response: + for rr in response["rrsets"]: + if rr["name"] == record_name and rr["type"] == "TXT": + return rr["rrdatas"] + return [] + def _find_managed_zone_id(self, domain): """ Find the managed zone for a given domain. diff --git a/certbot-dns-google/certbot_dns_google/dns_google_test.py b/certbot-dns-google/certbot_dns_google/dns_google_test.py index 53f84dd6e..3291b2c3a 100644 --- a/certbot-dns-google/certbot_dns_google/dns_google_test.py +++ b/certbot-dns-google/certbot_dns_google/dns_google_test.py @@ -74,10 +74,15 @@ class GoogleClientTest(unittest.TestCase): mock_mz = mock.MagicMock() mock_mz.list.return_value.execute.side_effect = zone_request_side_effect + mock_rrs = mock.MagicMock() + rrsets = {"rrsets": [{"name": "_acme-challenge.example.org.", "type": "TXT", + "rrdatas": ["\"example-txt-contents\""]}]} + mock_rrs.list.return_value.execute.return_value = rrsets mock_changes = mock.MagicMock() client.dns.managedZones = mock.MagicMock(return_value=mock_mz) client.dns.changes = mock.MagicMock(return_value=mock_changes) + client.dns.resourceRecordSets = mock.MagicMock(return_value=mock_rrs) return client, mock_changes @@ -137,6 +142,30 @@ class GoogleClientTest(unittest.TestCase): managedZone=self.zone, project=PROJECT_ID) + @mock.patch('oauth2client.service_account.ServiceAccountCredentials.from_json_keyfile_name') + @mock.patch('certbot_dns_google.dns_google.open', + mock.mock_open(read_data='{"project_id": "' + PROJECT_ID + '"}'), create=True) + def test_add_txt_record_delete_old(self, unused_credential_mock): + client, changes = self._setUp_client_with_mock( + [{'managedZones': [{'id': self.zone}]}]) + mock_get_rrs = "certbot_dns_google.dns_google._GoogleClient.get_existing_txt_rrset" + with mock.patch(mock_get_rrs) as mock_rrs: + mock_rrs.return_value = ["sample-txt-contents"] + client.add_txt_record(DOMAIN, self.record_name, self.record_content, self.record_ttl) + self.assertTrue(changes.create.called) + self.assertTrue("sample-txt-contents" in + changes.create.call_args_list[0][1]["body"]["deletions"][0]["rrdatas"]) + + @mock.patch('oauth2client.service_account.ServiceAccountCredentials.from_json_keyfile_name') + @mock.patch('certbot_dns_google.dns_google.open', + mock.mock_open(read_data='{"project_id": "' + PROJECT_ID + '"}'), create=True) + def test_add_txt_record_noop(self, unused_credential_mock): + client, changes = self._setUp_client_with_mock( + [{'managedZones': [{'id': self.zone}]}]) + client.add_txt_record(DOMAIN, "_acme-challenge.example.org", + "example-txt-contents", self.record_ttl) + self.assertFalse(changes.create.called) + @mock.patch('oauth2client.service_account.ServiceAccountCredentials.from_json_keyfile_name') @mock.patch('certbot_dns_google.dns_google.open', mock.mock_open(read_data='{"project_id": "' + PROJECT_ID + '"}'), create=True) @@ -172,7 +201,12 @@ class GoogleClientTest(unittest.TestCase): def test_del_txt_record(self, unused_credential_mock): client, changes = self._setUp_client_with_mock([{'managedZones': [{'id': self.zone}]}]) - client.del_txt_record(DOMAIN, self.record_name, self.record_content, self.record_ttl) + mock_get_rrs = "certbot_dns_google.dns_google._GoogleClient.get_existing_txt_rrset" + with mock.patch(mock_get_rrs) as mock_rrs: + mock_rrs.return_value = ["\"sample-txt-contents\"", + "\"example-txt-contents\""] + client.del_txt_record(DOMAIN, "_acme-challenge.example.org", + "example-txt-contents", self.record_ttl) expected_body = { "kind": "dns#change", @@ -180,8 +214,17 @@ class GoogleClientTest(unittest.TestCase): { "kind": "dns#resourceRecordSet", "type": "TXT", - "name": self.record_name + ".", - "rrdatas": [self.record_content, ], + "name": "_acme-challenge.example.org.", + "rrdatas": ["\"sample-txt-contents\"", "\"example-txt-contents\""], + "ttl": self.record_ttl, + }, + ], + "additions": [ + { + "kind": "dns#resourceRecordSet", + "type": "TXT", + "name": "_acme-challenge.example.org.", + "rrdatas": ["\"sample-txt-contents\"", ], "ttl": self.record_ttl, }, ], @@ -217,6 +260,18 @@ class GoogleClientTest(unittest.TestCase): client.del_txt_record(DOMAIN, self.record_name, self.record_content, self.record_ttl) + @mock.patch('oauth2client.service_account.ServiceAccountCredentials.from_json_keyfile_name') + @mock.patch('certbot_dns_google.dns_google.open', + mock.mock_open(read_data='{"project_id": "' + PROJECT_ID + '"}'), create=True) + def test_get_existing(self, unused_credential_mock): + client, unused_changes = self._setUp_client_with_mock( + [{'managedZones': [{'id': self.zone}]}]) + # Record name mocked in setUp + found = client.get_existing_txt_rrset(self.zone, "_acme-challenge.example.org") + self.assertEquals(found, ["\"example-txt-contents\""]) + not_found = client.get_existing_txt_rrset(self.zone, "nonexistent.tld") + self.assertEquals(not_found, []) + def test_get_project_id(self): from certbot_dns_google.dns_google import _GoogleClient