dns-google: fix condition to don't use private dns zones (#9744)

* dns-google: fix condition to don't use private dns zones * update MD * Fix condition * fix condition * update testdata * fix identation * update tests * update changelog * Update dns_google.py * add test for split horizon dns google * add dnsName to managed zones
2026-01-26 07:41:33 +03:00 · 2023-08-27 00:19:38 +01:00
parent 579b39dce1
commit b1978ff188
5 changed files with 42 additions and 14 deletions
--- a/AUTHORS.md
+++ b/AUTHORS.md
@@ -209,6 +209,7 @@ Authors
 * [Patrick Heppler](https://github.com/PatrickHeppler)
 * [Paul Buonopane](https://github.com/Zenexer)
 * [Paul Feitzinger](https://github.com/pfeyz)
+* [Paulo Dias](https://github.com/paulojmdias)
 * [Pavan Gupta](https://github.com/pavgup)
 * [Pavel Pavlov](https://github.com/ghost355)
 * [Peter Conrad](https://github.com/pconrad-fb)
--- a/certbot-dns-google/certbot_dns_google/_internal/dns_google.py
+++ b/certbot-dns-google/certbot_dns_google/_internal/dns_google.py
@@ -306,7 +306,7 @@ class _GoogleClient:

            for zone in zones:
                zone_id = zone['id']
-                if 'privateVisibilityConfig' not in zone:
+                if zone['visibility'] == "public":
                    logger.debug('Found id of %s for %s using name %s', zone_id, domain, zone_name)
                    return zone_id

--- a/certbot-dns-google/certbot_dns_google/_internal/tests/dns_google_test.py
+++ b/certbot-dns-google/certbot_dns_google/_internal/tests/dns_google_test.py
@@ -88,6 +88,7 @@ class GoogleClientTest(unittest.TestCase):
    record_ttl = 42
    zone = "ZONE_ID"
    change = "an-id"
+    visibility = "public"

    def _setUp_client_with_mock(self, zone_request_side_effect, rrs_list_side_effect=None):
        from certbot_dns_google._internal.dns_google import _GoogleClient
@@ -183,7 +184,7 @@ class GoogleClientTest(unittest.TestCase):
    def test_add_txt_record(self, credential_mock):
        credential_mock.return_value = (mock.MagicMock(), PROJECT_ID)

-        client, changes = self._setUp_client_with_mock([{'managedZones': [{'id': self.zone}]}])
+        client, changes = self._setUp_client_with_mock([{'managedZones': [{'id': self.zone, 'visibility': self.visibility}]}])
        credential_mock.assert_called_once_with('/not/a/real/path.json', scopes=SCOPES)

        client.add_txt_record(DOMAIN, self.record_name, self.record_content, self.record_ttl)
@@ -211,7 +212,7 @@ class GoogleClientTest(unittest.TestCase):
    def test_add_txt_record_and_poll(self, credential_mock):
        credential_mock.return_value = (mock.MagicMock(), PROJECT_ID)

-        client, changes = self._setUp_client_with_mock([{'managedZones': [{'id': self.zone}]}])
+        client, changes = self._setUp_client_with_mock([{'managedZones': [{'id': self.zone, 'visibility': self.visibility}]}])
        changes.create.return_value.execute.return_value = {'status': 'pending', 'id': self.change}
        changes.get.return_value.execute.return_value = {'status': 'done'}

@@ -225,6 +226,26 @@ class GoogleClientTest(unittest.TestCase):
                                            managedZone=self.zone,
                                            project=PROJECT_ID)

+    @mock.patch('google.auth.load_credentials_from_file')
+    @mock.patch('certbot_dns_google._internal.dns_google.open',
+                mock.mock_open(read_data='{"project_id": "' + PROJECT_ID + '"}'), create=True)
+    def test_add_txt_record_and_poll_split_horizon(self, credential_mock):
+        credential_mock.return_value = (mock.MagicMock(), PROJECT_ID)
+
+        client, changes = self._setUp_client_with_mock([{'managedZones': [{'id': '{zone}-private'.format(zone=self.zone), 'dnsName': DOMAIN, 'visibility': 'private'},{'id': '{zone}-public'.format(zone=self.zone), 'dnsName': DOMAIN, 'visibility': self.visibility}]}])
+        changes.create.return_value.execute.return_value = {'status': 'pending', 'id': self.change}
+        changes.get.return_value.execute.return_value = {'status': 'done'}
+
+        client.add_txt_record(DOMAIN, self.record_name, self.record_content, self.record_ttl)
+
+        changes.create.assert_called_with(body=mock.ANY,
+                                               managedZone='{zone}-public'.format(zone=self.zone),
+                                               project=PROJECT_ID)
+
+        changes.get.assert_called_with(changeId=self.change,
+                                            managedZone='{zone}-public'.format(zone=self.zone),
+                                            project=PROJECT_ID)
+
    @mock.patch('google.auth.load_credentials_from_file')
    @mock.patch('certbot_dns_google._internal.dns_google.open',
                mock.mock_open(read_data='{"project_id": "' + PROJECT_ID + '"}'), create=True)
@@ -232,7 +253,7 @@ class GoogleClientTest(unittest.TestCase):
        credential_mock.return_value = (mock.MagicMock(), PROJECT_ID)

        client, changes = self._setUp_client_with_mock(
-            [{'managedZones': [{'id': self.zone}]}])
+            [{'managedZones': [{'id': self.zone, 'visibility': self.visibility}]}])
        # pylint: disable=line-too-long
        mock_get_rrs = "certbot_dns_google._internal.dns_google._GoogleClient.get_existing_txt_rrset"
        with mock.patch(mock_get_rrs) as mock_rrs:
@@ -250,7 +271,7 @@ class GoogleClientTest(unittest.TestCase):
        credential_mock.return_value = (mock.MagicMock(), PROJECT_ID)

        client, changes = self._setUp_client_with_mock(
-            [{'managedZones': [{'id': self.zone}]}])
+            [{'managedZones': [{'id': self.zone, 'visibility': self.visibility}]}])
        # pylint: disable=line-too-long
        mock_get_rrs = "certbot_dns_google._internal.dns_google._GoogleClient.get_existing_txt_rrset"
        with mock.patch(mock_get_rrs) as mock_rrs:
@@ -269,7 +290,7 @@ class GoogleClientTest(unittest.TestCase):
        credential_mock.return_value = (mock.MagicMock(), PROJECT_ID)

        client, changes = self._setUp_client_with_mock(
-            [{'managedZones': [{'id': self.zone}]}])
+            [{'managedZones': [{'id': self.zone, 'visibility': self.visibility}]}])
        client.add_txt_record(DOMAIN, "_acme-challenge.example.org",
                              "example-txt-contents", self.record_ttl)
        assert changes.create.called is False
@@ -303,7 +324,7 @@ class GoogleClientTest(unittest.TestCase):
    def test_add_txt_record_error_during_add(self, credential_mock):
        credential_mock.return_value = (mock.MagicMock(), PROJECT_ID)

-        client, changes = self._setUp_client_with_mock([{'managedZones': [{'id': self.zone}]}])
+        client, changes = self._setUp_client_with_mock([{'managedZones': [{'id': self.zone, 'visibility': self.visibility}]}])
        changes.create.side_effect = API_ERROR

        with pytest.raises(errors.PluginError):
@@ -315,7 +336,7 @@ class GoogleClientTest(unittest.TestCase):
    def test_del_txt_record_multi_rrdatas(self, credential_mock):
        credential_mock.return_value = (mock.MagicMock(), PROJECT_ID)

-        client, changes = self._setUp_client_with_mock([{'managedZones': [{'id': self.zone}]}])
+        client, changes = self._setUp_client_with_mock([{'managedZones': [{'id': self.zone, 'visibility': self.visibility}]}])
        # pylint: disable=line-too-long
        mock_get_rrs = "certbot_dns_google._internal.dns_google._GoogleClient.get_existing_txt_rrset"
        with mock.patch(mock_get_rrs) as mock_rrs:
@@ -356,7 +377,7 @@ class GoogleClientTest(unittest.TestCase):
    def test_del_txt_record_single_rrdatas(self, credential_mock):
        credential_mock.return_value = (mock.MagicMock(), PROJECT_ID)

-        client, changes = self._setUp_client_with_mock([{'managedZones': [{'id': self.zone}]}])
+        client, changes = self._setUp_client_with_mock([{'managedZones': [{'id': self.zone, 'visibility': self.visibility}]}])
        # pylint: disable=line-too-long
        mock_get_rrs = "certbot_dns_google._internal.dns_google._GoogleClient.get_existing_txt_rrset"
        with mock.patch(mock_get_rrs) as mock_rrs:
@@ -408,7 +429,7 @@ class GoogleClientTest(unittest.TestCase):
    def test_del_txt_record_error_during_delete(self, credential_mock):
        credential_mock.return_value = (mock.MagicMock(), PROJECT_ID)

-        client, changes = self._setUp_client_with_mock([{'managedZones': [{'id': self.zone}]}])
+        client, changes = self._setUp_client_with_mock([{'managedZones': [{'id': self.zone, 'visibility': self.visibility}]}])
        changes.create.side_effect = API_ERROR

        client.del_txt_record(DOMAIN, self.record_name, self.record_content, self.record_ttl)
@@ -420,7 +441,7 @@ class GoogleClientTest(unittest.TestCase):
        credential_mock.return_value = (mock.MagicMock(), PROJECT_ID)

        client, unused_changes = self._setUp_client_with_mock(
-            [{'managedZones': [{'id': self.zone}]}])
+            [{'managedZones': [{'id': self.zone, 'visibility': self.visibility}]}])
        # Record name mocked in setUp
        found = client.get_existing_txt_rrset(self.zone, "_acme-challenge.example.org")
        assert found["rrdatas"] == ["\"example-txt-contents\""]
@@ -433,7 +454,7 @@ class GoogleClientTest(unittest.TestCase):
        credential_mock.return_value = (mock.MagicMock(), PROJECT_ID)

        client, unused_changes = self._setUp_client_with_mock(
-            [{'managedZones': [{'id': self.zone}]}])
+            [{'managedZones': [{'id': self.zone, 'visibility': self.visibility}]}])
        not_found = client.get_existing_txt_rrset(self.zone, "nonexistent.tld")
        assert not_found is None

@@ -444,7 +465,7 @@ class GoogleClientTest(unittest.TestCase):
        credential_mock.return_value = (mock.MagicMock(), PROJECT_ID)

        client, unused_changes = self._setUp_client_with_mock(
-            [{'managedZones': [{'id': self.zone}]}], API_ERROR)
+            [{'managedZones': [{'id': self.zone, 'visibility': self.visibility}]}], API_ERROR)
        # Record name mocked in setUp
        found = client.get_existing_txt_rrset(self.zone, "_acme-challenge.example.org")
        assert found is None
@@ -456,7 +477,7 @@ class GoogleClientTest(unittest.TestCase):
        credential_mock.return_value = (mock.MagicMock(), PROJECT_ID)

        client, unused_changes = self._setUp_client_with_mock(
-            [{'managedZones': [{'id': self.zone}]}], API_ERROR)
+            [{'managedZones': [{'id': self.zone, 'visibility': self.visibility}]}], API_ERROR)
        rrset = client.get_existing_txt_rrset(self.zone, "_acme-challenge.example.org")
        assert not rrset

--- a/certbot-dns-google/certbot_dns_google/_internal/tests/testdata/discovery.json
+++ b/certbot-dns-google/certbot_dns_google/_internal/tests/testdata/discovery.json
@@ -389,6 +389,11 @@
     "items": {
      "type": "string"
     }
+    },
+    "visibility": {
+     "type": "string",
+     "description": "The zone's visibility: public zones are exposed to the Internet, while private zones are visible only to Virtual Private Cloud resources.",
+     "default": "public"
    }
   }
  },
--- a/certbot/CHANGELOG.md
+++ b/certbot/CHANGELOG.md
@@ -18,6 +18,7 @@ Certbot adheres to [Semantic Versioning](https://semver.org/).
 ### Fixed

 * Do not call deprecated datetime.utcnow() and datetime.utcfromtimestamp()
+* Filter zones in `certbot-dns-google` to avoid usage of private DNS zones to create records

 More details about these changes can be found on our GitHub repo.