From cbd89b4630067ccf615449a1682254feedccc674 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Riku=20L=C3=A4=C3=A4kk=C3=B6l=C3=A4?= Date: Fri, 31 Jan 2025 09:38:05 +0200 Subject: [PATCH 1/3] Add option to log name server information on apply Enable logging zone name server information on apply. This makes managing new zones easier, as otherwise one has to go look up the name server information for the zone separately in order to configure the domain registrar side records. The name server information is already present in the responses that the zone method receives, so adding it is easy enough. Downside is that a nested dict structure is required, which makes lookups a bit more complex. --- README.md | 2 ++ octodns_cloudflare/__init__.py | 33 ++++++++++++++++++----- tests/test_octodns_provider_cloudflare.py | 17 +++++++----- 3 files changed, 40 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 8820ce9..17a8ebf 100644 --- a/README.md +++ b/README.md @@ -70,6 +70,8 @@ providers: # A different limit for (non-)enterprise zone applies. # See: https://developers.cloudflare.com/dns/manage-dns-records/reference/ttl #min_ttl: 120 + # Optional. Default: False. Log zone name servers on apply. + #log_name_servers: False ``` Note: The "proxied" flag of "A", "AAAA" and "CNAME" records can be managed via the YAML provider like so: diff --git a/octodns_cloudflare/__init__.py b/octodns_cloudflare/__init__.py index 770bf9b..c6b0791 100644 --- a/octodns_cloudflare/__init__.py +++ b/octodns_cloudflare/__init__.py @@ -95,6 +95,7 @@ def __init__( zones_per_page=50, records_per_page=100, min_ttl=120, + log_name_servers=False, *args, **kwargs, ): @@ -129,6 +130,7 @@ def __init__( self.zones_per_page = zones_per_page self.records_per_page = records_per_page self.min_ttl = min_ttl + self.log_name_servers = log_name_servers self._sess = sess self._zones = None @@ -214,7 +216,15 @@ def zones(self): else: page = None - self._zones = IdnaDict({f'{z["name"]}.': z['id'] for z in zones}) + self._zones = IdnaDict( + { + f'{z["name"]}.': { + 'id': z['id'], + 'name_servers': z.get('name_servers', []), + } + for z in zones + } + ) return self._zones @@ -468,7 +478,7 @@ def _data_for_SSHFP(self, _type, records): def zone_records(self, zone): if zone.name not in self._zone_records: - zone_id = self.zones.get(zone.name, False) + zone_id = self.zones.get(zone.name, {}).get('id', False) if not zone_id: return [] @@ -1016,7 +1026,7 @@ def _gen_key(self, data): def _apply_Create(self, change): new = change.new - zone_id = self.zones[new.zone.name] + zone_id = self.zones[new.zone.name]['id'] if new._type == 'URLFWD': path = f'/zones/{zone_id}/pagerules' else: @@ -1026,7 +1036,7 @@ def _apply_Create(self, change): def _apply_Update(self, change): zone = change.new.zone - zone_id = self.zones[zone.name] + zone_id = self.zones[zone.name]['id'] hostname = zone.hostname_from_fqdn(change.new.fqdn[:-1]) _type = change.new._type @@ -1166,7 +1176,9 @@ def _apply_Delete(self, change): parsed_uri = urlsplit(uri) record_name = parsed_uri.netloc record_type = 'URLFWD' - zone_id = self.zones.get(existing.zone.name, False) + zone_id = self.zones.get(existing.zone.name, {}).get( + 'id', False + ) if ( existing_name == record_name and existing_type == record_type @@ -1199,9 +1211,18 @@ def _apply(self, plan): data['account'] = {'id': self.account_id} resp = self._try_request('POST', '/zones', data=data) zone_id = resp['result']['id'] - self.zones[name] = zone_id + name_servers = resp['result']['name_servers'] + self.zones[name] = {'id': zone_id, 'name_servers': name_servers} self._zone_records[name] = {} + if self.log_name_servers: + self.log.info( + 'zone %s (id %s) name servers: %s', + name, + self.zones[name]['id'], + self.zones[name]['name_servers'], + ) + # Force the operation order to be Delete() -> Create() -> Update() # This will help avoid problems in updating a CNAME record into an # A record and vice-versa diff --git a/tests/test_octodns_provider_cloudflare.py b/tests/test_octodns_provider_cloudflare.py index 5a90358..350f1bf 100644 --- a/tests/test_octodns_provider_cloudflare.py +++ b/tests/test_octodns_provider_cloudflare.py @@ -258,14 +258,19 @@ def test_populate(self): def test_apply(self): provider = CloudflareProvider( - 'test', 'email', 'token', retry_period=0, strict_supports=False + 'test', + 'email', + 'token', + retry_period=0, + strict_supports=False, + log_name_servers=True, ) provider._request = Mock() provider._request.side_effect = [ self.empty, # no zones - {'result': {'id': 42}}, # zone create + {'result': {'id': 42, 'name_servers': ['foo']}}, # zone create ] + [ None ] * 34 # individual record creates @@ -576,7 +581,7 @@ def test_apply(self): provider._request.side_effect = [ self.empty, # no zones - {'result': {'id': 42}}, # zone create + {'result': {'id': 42, 'name_servers': ['foo']}}, # zone create ] + [ None ] * 34 # individual record creates @@ -696,7 +701,7 @@ def test_update_add_swap(self): provider._request.side_effect = [ CloudflareRateLimitError('{}'), self.empty, # no zones - {'result': {'id': 42}}, # zone create + {'result': {'id': 42, 'name_servers': ['foo']}}, # zone create None, None, None, @@ -869,7 +874,7 @@ def test_update_delete(self): provider._request.side_effect = [ CloudflareRateLimitError('{}'), self.empty, # no zones - {'result': {'id': 42}}, # zone create + {'result': {'id': 42, 'name_servers': ['foo']}}, # zone create None, None, None, @@ -1005,7 +1010,7 @@ def test_pagerules(self): # Set things up to preexist/mock as necessary zone = Zone('unit.tests.', []) # Stuff a fake zone id in place - provider._zones = {zone.name: '42'} + provider._zones = {zone.name: {'id': '42', 'name_servers': ['foo']}} provider._request = Mock() side_effect = [ { From 0ca3b2a5f355a8271ff6e76a7ce35ff3ed8bc5b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Riku=20L=C3=A4=C3=A4kk=C3=B6l=C3=A4?= Date: Mon, 3 Feb 2025 11:25:01 +0200 Subject: [PATCH 2/3] Remove option and always log name servers --- README.md | 2 -- octodns_cloudflare/__init__.py | 15 ++++++--------- tests/test_octodns_provider_cloudflare.py | 7 +------ 3 files changed, 7 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index 17a8ebf..8820ce9 100644 --- a/README.md +++ b/README.md @@ -70,8 +70,6 @@ providers: # A different limit for (non-)enterprise zone applies. # See: https://developers.cloudflare.com/dns/manage-dns-records/reference/ttl #min_ttl: 120 - # Optional. Default: False. Log zone name servers on apply. - #log_name_servers: False ``` Note: The "proxied" flag of "A", "AAAA" and "CNAME" records can be managed via the YAML provider like so: diff --git a/octodns_cloudflare/__init__.py b/octodns_cloudflare/__init__.py index ffb33aa..c4ab981 100644 --- a/octodns_cloudflare/__init__.py +++ b/octodns_cloudflare/__init__.py @@ -100,7 +100,6 @@ def __init__( zones_per_page=50, records_per_page=100, min_ttl=120, - log_name_servers=False, *args, **kwargs, ): @@ -135,7 +134,6 @@ def __init__( self.zones_per_page = zones_per_page self.records_per_page = records_per_page self.min_ttl = min_ttl - self.log_name_servers = log_name_servers self._sess = sess self._zones = None @@ -1238,13 +1236,12 @@ def _apply(self, plan): self.zones[name] = {'id': zone_id, 'name_servers': name_servers} self._zone_records[name] = {} - if self.log_name_servers: - self.log.info( - 'zone %s (id %s) name servers: %s', - name, - self.zones[name]['id'], - self.zones[name]['name_servers'], - ) + self.log.info( + 'zone %s (id %s) name servers: %s', + name, + self.zones[name]['id'], + self.zones[name]['name_servers'], + ) # Force the operation order to be Delete() -> Create() -> Update() # This will help avoid problems in updating a CNAME record into an diff --git a/tests/test_octodns_provider_cloudflare.py b/tests/test_octodns_provider_cloudflare.py index 676b14c..83dfa97 100644 --- a/tests/test_octodns_provider_cloudflare.py +++ b/tests/test_octodns_provider_cloudflare.py @@ -278,12 +278,7 @@ def test_populate(self): def test_apply(self): provider = CloudflareProvider( - 'test', - 'email', - 'token', - retry_period=0, - strict_supports=False, - log_name_servers=True, + 'test', 'email', 'token', retry_period=0, strict_supports=False ) provider._request = Mock() From a555ed9906cd3d52ab082a5f3732538e3ea3e7a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Riku=20L=C3=A4=C3=A4kk=C3=B6l=C3=A4?= Date: Tue, 4 Feb 2025 10:21:50 +0200 Subject: [PATCH 3/3] Remove useless get --- octodns_cloudflare/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/octodns_cloudflare/__init__.py b/octodns_cloudflare/__init__.py index 088d31b..cfd69db 100644 --- a/octodns_cloudflare/__init__.py +++ b/octodns_cloudflare/__init__.py @@ -235,7 +235,7 @@ def zones(self): { f'{z["name"]}.': { 'id': z['id'], - 'name_servers': z.get('name_servers', []), + 'name_servers': z['name_servers'], } for z in zones }