diff --git a/AUTHORS.md b/AUTHORS.md index f76c323a5..ff5c61613 100644 --- a/AUTHORS.md +++ b/AUTHORS.md @@ -149,6 +149,7 @@ Authors * [Lior Sabag](https://github.com/liorsbg) * [Lipis](https://github.com/lipis) * [lord63](https://github.com/lord63) +* [Lorenzo Fundaró](https://github.com/lfundaro) * [Luca Beltrame](https://github.com/lbeltrame) * [Luca Ebach](https://github.com/lucebac) * [Luca Olivetti](https://github.com/olivluca) diff --git a/certbot-dns-google/certbot_dns_google/_internal/dns_google.py b/certbot-dns-google/certbot_dns_google/_internal/dns_google.py index 1bd3468da..cd4b2d2d5 100644 --- a/certbot-dns-google/certbot_dns_google/_internal/dns_google.py +++ b/certbot-dns-google/certbot_dns_google/_internal/dns_google.py @@ -240,9 +240,10 @@ class _GoogleClient(object): """ rrs_request = self.dns.resourceRecordSets() - request = rrs_request.list(managedZone=zone_id, project=self.project_id) # Add dot as the API returns absolute domains record_name += "." + request = rrs_request.list(project=self.project_id, managedZone=zone_id, name=record_name, + type="TXT") try: response = request.execute() except googleapiclient_errors.Error: @@ -250,10 +251,8 @@ class _GoogleClient(object): "requesting a wildcard certificate, this might not work.") logger.debug("Error was:", exc_info=True) else: - if response: - for rr in response["rrsets"]: - if rr["name"] == record_name and rr["type"] == "TXT": - return rr["rrdatas"] + if response and response["rrsets"]: + return response["rrsets"][0]["rrdatas"] return None def _find_managed_zone_id(self, domain): diff --git a/certbot-dns-google/tests/dns_google_test.py b/certbot-dns-google/tests/dns_google_test.py index 40002f143..bcb6bb80f 100644 --- a/certbot-dns-google/tests/dns_google_test.py +++ b/certbot-dns-google/tests/dns_google_test.py @@ -70,7 +70,7 @@ class GoogleClientTest(unittest.TestCase): zone = "ZONE_ID" change = "an-id" - def _setUp_client_with_mock(self, zone_request_side_effect): + def _setUp_client_with_mock(self, zone_request_side_effect, rrs_list_side_effect=None): from certbot_dns_google._internal.dns_google import _GoogleClient pwd = os.path.dirname(__file__) @@ -86,9 +86,16 @@ class GoogleClientTest(unittest.TestCase): 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", + def rrs_list(project=None, managedZone=None, name=None, type=None): + response = {"rrsets": []} + if name == "_acme-challenge.example.org.": + response = {"rrsets": [{"name": "_acme-challenge.example.org.", "type": "TXT", "rrdatas": ["\"example-txt-contents\""]}]} - mock_rrs.list.return_value.execute.return_value = rrsets + mock_return = mock.MagicMock() + mock_return.execute.return_value = response + mock_return.execute.side_effect = rrs_list_side_effect + return mock_return + mock_rrs.list.side_effect = rrs_list mock_changes = mock.MagicMock() client.dns.managedZones = mock.MagicMock(return_value=mock_mz) @@ -287,12 +294,19 @@ class GoogleClientTest(unittest.TestCase): @mock.patch('oauth2client.service_account.ServiceAccountCredentials.from_json_keyfile_name') @mock.patch('certbot_dns_google._internal.dns_google.open', mock.mock_open(read_data='{"project_id": "' + PROJECT_ID + '"}'), create=True) - def test_get_existing(self, unused_credential_mock): + def test_get_existing_found(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.assertEqual(found, ["\"example-txt-contents\""]) + + @mock.patch('oauth2client.service_account.ServiceAccountCredentials.from_json_keyfile_name') + @mock.patch('certbot_dns_google._internal.dns_google.open', + mock.mock_open(read_data='{"project_id": "' + PROJECT_ID + '"}'), create=True) + def test_get_existing_not_found(self, unused_credential_mock): + client, unused_changes = self._setUp_client_with_mock( + [{'managedZones': [{'id': self.zone}]}]) not_found = client.get_existing_txt_rrset(self.zone, "nonexistent.tld") self.assertEqual(not_found, None) @@ -301,10 +315,7 @@ class GoogleClientTest(unittest.TestCase): mock.mock_open(read_data='{"project_id": "' + PROJECT_ID + '"}'), create=True) def test_get_existing_fallback(self, unused_credential_mock): client, unused_changes = self._setUp_client_with_mock( - [{'managedZones': [{'id': self.zone}]}]) - mock_execute = client.dns.resourceRecordSets.return_value.list.return_value.execute - mock_execute.side_effect = API_ERROR - + [{'managedZones': [{'id': self.zone}]}], API_ERROR) rrset = client.get_existing_txt_rrset(self.zone, "_acme-challenge.example.org") self.assertFalse(rrset) diff --git a/certbot/CHANGELOG.md b/certbot/CHANGELOG.md index e4f4eda51..8bfee52be 100644 --- a/certbot/CHANGELOG.md +++ b/certbot/CHANGELOG.md @@ -22,6 +22,7 @@ Certbot adheres to [Semantic Versioning](https://semver.org/). * The Certbot snap no longer loads packages installed via `pip install --user`. This was unintended and DNS plugins should be installed via `snap` instead. +* `certbot-dns-google` would sometimes crash with HTTP 409/412 errors when used with very large zones (#6036) More details about these changes can be found on our GitHub repo.