From a3b94cfed32d2ae02407958f02d32c809d9f87a8 Mon Sep 17 00:00:00 2001 From: Brian E Clow Date: Thu, 22 Jul 2021 14:33:06 -0700 Subject: [PATCH] Adding URLFWD type to CloudeFlare provider + testing updates --- octodns/provider/cloudflare.py | 187 +++++++++++++--- tests/test_octodns_provider_cloudflare.py | 251 ++++++++++++++++++++-- 2 files changed, 393 insertions(+), 45 deletions(-) diff --git a/octodns/provider/cloudflare.py b/octodns/provider/cloudflare.py index 9c206c0..9462704 100644 --- a/octodns/provider/cloudflare.py +++ b/octodns/provider/cloudflare.py @@ -10,6 +10,7 @@ from copy import deepcopy from logging import getLogger from requests import Session from time import sleep +from urllib.parse import urlsplit from ..record import Record, Update from .base import BaseProvider @@ -76,7 +77,7 @@ class CloudflareProvider(BaseProvider): SUPPORTS_GEO = False SUPPORTS_DYNAMIC = False SUPPORTS = set(('ALIAS', 'A', 'AAAA', 'CAA', 'CNAME', 'LOC', 'MX', 'NS', - 'PTR', 'SRV', 'SPF', 'TXT')) + 'PTR', 'SRV', 'SPF', 'TXT', 'URLFWD')) MIN_TTL = 120 TIMEOUT = 15 @@ -286,6 +287,22 @@ class CloudflareProvider(BaseProvider): 'values': values } + def _data_for_URLFWD(self, _type, records): + values = [] + for r in records: + values.append({ + 'path': r['path'], + 'target': r['url'], + 'code': r['status_code'], + 'masking': 2, + 'query': 0, + }) + return { + 'type': _type, + 'ttl': 3600, # ttl does not exist for this type, forcing a setting + 'values': values + } + def zone_records(self, zone): if zone.name not in self._zone_records: zone_id = self.zones.get(zone.name, False) @@ -305,6 +322,10 @@ class CloudflareProvider(BaseProvider): else: page = None + path = '/zones/{}/pagerules'.format(zone_id) + resp = self._try_request('GET', path, params={'status': 'active'}) + records += resp['result'] + self._zone_records[zone.name] = records return self._zone_records[zone.name] @@ -341,10 +362,30 @@ class CloudflareProvider(BaseProvider): exists = True values = defaultdict(lambda: defaultdict(list)) for record in records: - name = zone.hostname_from_fqdn(record['name']) - _type = record['type'] - if _type in self.SUPPORTS: - values[name][record['type']].append(record) + if 'targets' in record: + # assumption, targets will always contain 1 target + # API documentation only indicates 'url' as the only target + # if record['targets'][0]['target'] == 'url': + uri = record['targets'][0]['constraint']['value'] + uri = '//' + uri if not uri.startswith('http') else uri + parsed_uri = urlsplit(uri) + name = zone.hostname_from_fqdn(parsed_uri.netloc) + _path = parsed_uri.path + _type = 'URLFWD' + # assumption, actions will always contain 1 action + if record['actions'][0]['id'] == 'forwarding_url': + _values = record['actions'][0]['value'] + _values['path'] = _path + # no ttl set by pagerule, creating one + _values['ttl'] = 3600 + values[name][_type].append(_values) + # the dns_records branch + # elif 'name' in record: + else: + name = zone.hostname_from_fqdn(record['name']) + _type = record['type'] + if _type in self.SUPPORTS: + values[name][record['type']].append(record) for name, types in values.items(): for _type, records in types.items(): @@ -473,6 +514,31 @@ class CloudflareProvider(BaseProvider): } } + def _contents_for_URLFWD(self, record): + name = record.fqdn[:-1] + for value in record.values: + yield { + 'targets': [ + { + 'target': 'url', + 'constraint': { + 'operator': 'matches', + 'value': name + value.path + } + } + ], + 'actions': [ + { + 'id': 'forwarding_url', + 'value': { + 'url': value.target, + 'status_code': value.code, + } + } + ], + 'status': 'active', + } + def _record_is_proxied(self, record): return ( not self.cdn and @@ -488,20 +554,25 @@ class CloudflareProvider(BaseProvider): if _type == 'ALIAS': _type = 'CNAME' - contents_for = getattr(self, '_contents_for_{}'.format(_type)) - for content in contents_for(record): - content.update({ - 'name': name, - 'type': _type, - 'ttl': ttl, - }) - - if _type in _PROXIABLE_RECORD_TYPES: + if _type == 'URLFWD': + contents_for = getattr(self, '_contents_for_{}'.format(_type)) + for content in contents_for(record): + yield content + else: + contents_for = getattr(self, '_contents_for_{}'.format(_type)) + for content in contents_for(record): content.update({ - 'proxied': self._record_is_proxied(record) + 'name': name, + 'type': _type, + 'ttl': ttl, }) - yield content + if _type in _PROXIABLE_RECORD_TYPES: + content.update({ + 'proxied': self._record_is_proxied(record) + }) + + yield content def _gen_key(self, data): # Note that most CF record data has a `content` field the value of @@ -515,7 +586,11 @@ class CloudflareProvider(BaseProvider): # BUT... there are exceptions. MX, CAA, LOC and SRV don't have a simple # content as things are currently implemented so we need to handle # those explicitly and create unique/hashable strings for them. - _type = data['type'] + # AND... for URLFWD/Redirects additional adventures are created. + if 'targets' in data: + _type = 'URLFWD' + else: + _type = data['type'] if _type == 'MX': return '{priority} {content}'.format(**data) elif _type == 'CAA': @@ -540,12 +615,28 @@ class CloudflareProvider(BaseProvider): '{precision_horz}', '{precision_vert}') return ' '.join(loc).format(**data) + elif _type == 'URLFWD': + uri = data['targets'][0]['constraint']['value'] + uri = '//' + uri if not uri.startswith('http') else uri + parsed_uri = urlsplit(uri) + ret = {} + ret.update(data['actions'][0]['value']) + ret.update({'name': parsed_uri.netloc, 'path': parsed_uri.path}) + urlfwd = ( + '{name}', + '{path}', + '{url}', + '{status_code}') + return ' '.join(urlfwd).format(**ret) return data['content'] def _apply_Create(self, change): new = change.new zone_id = self.zones[new.zone.name] - path = '/zones/{}/dns_records'.format(zone_id) + if new._type == 'URLFWD': + path = '/zones/{}/pagerules'.format(zone_id) + else: + path = '/zones/{}/dns_records'.format(zone_id) for content in self._gen_data(new): self._try_request('POST', path, data=content) @@ -558,14 +649,28 @@ class CloudflareProvider(BaseProvider): existing = {} # Find all of the existing CF records for this name & type for record in self.zone_records(zone): - name = zone.hostname_from_fqdn(record['name']) + if 'targets' in record: + uri = record['targets'][0]['constraint']['value'] + uri = '//' + uri if not uri.startswith('http') else uri + parsed_uri = urlsplit(uri) + name = zone.hostname_from_fqdn(parsed_uri.netloc) + _path = parsed_uri.path + # assumption, populate will catch this contition + # if record['actions'][0]['id'] == 'forwarding_url': + _values = record['actions'][0]['value'] + _values['path'] = _path + _values['ttl'] = 3600 + _values['type'] = 'URLFWD' + record.update(_values) + else: + name = zone.hostname_from_fqdn(record['name']) # Use the _record_for so that we include all of standard # conversion logic r = self._record_for(zone, name, record['type'], [record], True) if hostname == r.name and _type == r._type: - # Round trip the single value through a record to contents flow - # to get a consistent _gen_data result that matches what - # went in to new_contents + # Round trip the single value through a record to contents + # flow to get a consistent _gen_data result that matches + # what went in to new_contents data = next(self._gen_data(r)) # Record the record_id and data for this existing record @@ -633,7 +738,10 @@ class CloudflareProvider(BaseProvider): # otherwise required, just makes things deterministic # Creates - path = '/zones/{}/dns_records'.format(zone_id) + if _type == 'URLFWD': + path = '/zones/{}/pagerules'.format(zone_id) + else: + path = '/zones/{}/dns_records'.format(zone_id) for _, data in sorted(creates.items()): self.log.debug('_apply_Update: creating %s', data) self._try_request('POST', path, data=data) @@ -643,7 +751,10 @@ class CloudflareProvider(BaseProvider): record_id = info['record_id'] data = info['data'] old_data = info['old_data'] - path = '/zones/{}/dns_records/{}'.format(zone_id, record_id) + if _type == 'URLFWD': + path = '/zones/{}/pagerules/{}'.format(zone_id, record_id) + else: + path = '/zones/{}/dns_records/{}'.format(zone_id, record_id) self.log.debug('_apply_Update: updating %s, %s -> %s', record_id, data, old_data) self._try_request('PUT', path, data=data) @@ -652,7 +763,10 @@ class CloudflareProvider(BaseProvider): for _, info in sorted(deletes.items()): record_id = info['record_id'] old_data = info['data'] - path = '/zones/{}/dns_records/{}'.format(zone_id, record_id) + if _type == 'URLFWD': + path = '/zones/{}/pagerules/{}'.format(zone_id, record_id) + else: + path = '/zones/{}/dns_records/{}'.format(zone_id, record_id) self.log.debug('_apply_Update: removing %s, %s', record_id, old_data) self._try_request('DELETE', path) @@ -664,11 +778,24 @@ class CloudflareProvider(BaseProvider): existing_type = 'CNAME' if existing._type == 'ALIAS' \ else existing._type for record in self.zone_records(existing.zone): - if existing_name == record['name'] and \ - existing_type == record['type']: - path = '/zones/{}/dns_records/{}'.format(record['zone_id'], - record['id']) - self._try_request('DELETE', path) + if 'targets' in record: + uri = record['targets'][0]['constraint']['value'] + uri = '//' + uri if not uri.startswith('http') else uri + parsed_uri = urlsplit(uri) + record_name = parsed_uri.netloc + record_type = 'URLFWD' + zone_id = self.zones.get(existing.zone.name, False) + if existing_name == record_name and \ + existing_type == record_type: + path = '/zones/{}/pagerules/{}' \ + .format(zone_id, record['id']) + self._try_request('DELETE', path) + else: + if existing_name == record['name'] and \ + existing_type == record['type']: + path = '/zones/{}/dns_records/{}' \ + .format(record['zone_id'], record['id']) + self._try_request('DELETE', path) def _apply(self, plan): desired = plan.desired diff --git a/tests/test_octodns_provider_cloudflare.py b/tests/test_octodns_provider_cloudflare.py index 52c261e..c2addf0 100644 --- a/tests/test_octodns_provider_cloudflare.py +++ b/tests/test_octodns_provider_cloudflare.py @@ -166,9 +166,15 @@ class TestCloudflareProvider(TestCase): json={'result': [], 'result_info': {'count': 0, 'per_page': 0}}) + base = '{}/234234243423aaabb334342aaa343435'.format(base) + + # pagerules/URLFWD + with open('tests/fixtures/cloudflare-pagerules.json') as fh: + mock.get('{}/pagerules?status=active'.format(base), + status_code=200, text=fh.read()) + # records - base = '{}/234234243423aaabb334342aaa343435/dns_records' \ - .format(base) + base = '{}/dns_records'.format(base) with open('tests/fixtures/cloudflare-dns_records-' 'page-1.json') as fh: mock.get('{}?page=1'.format(base), status_code=200, @@ -184,16 +190,16 @@ class TestCloudflareProvider(TestCase): zone = Zone('unit.tests.', []) provider.populate(zone) - self.assertEquals(16, len(zone.records)) + self.assertEquals(19, len(zone.records)) changes = self.expected.changes(zone, provider) - self.assertEquals(0, len(changes)) + self.assertEquals(4, len(changes)) # re-populating the same zone/records comes out of cache, no calls again = Zone('unit.tests.', []) provider.populate(again) - self.assertEquals(16, len(again.records)) + self.assertEquals(19, len(again.records)) def test_apply(self): provider = CloudflareProvider('test', 'email', 'token', retry_period=0) @@ -207,12 +213,12 @@ class TestCloudflareProvider(TestCase): 'id': 42, } }, # zone create - ] + [None] * 25 # individual record creates + ] + [None] * 27 # individual record creates # non-existent zone, create everything plan = provider.plan(self.expected) - self.assertEquals(16, len(plan.changes)) - self.assertEquals(16, provider.apply(plan)) + self.assertEquals(17, len(plan.changes)) + self.assertEquals(17, provider.apply(plan)) self.assertFalse(plan.exists) provider._request.assert_has_calls([ @@ -236,9 +242,31 @@ class TestCloudflareProvider(TestCase): 'name': 'txt.unit.tests', 'ttl': 600 }), + # create at least one pagerules + call('POST', '/zones/42/pagerules', data={ + 'targets': [ + { + 'target': 'url', + 'constraint': { + 'operator': 'matches', + 'value': 'urlfwd.unit.tests/' + } + } + ], + 'actions': [ + { + 'id': 'forwarding_url', + 'value': { + 'url': 'http://www.unit.tests', + 'status_code': 302 + } + } + ], + 'status': 'active' + }), ], True) # expected number of total calls - self.assertEquals(27, provider._request.call_count) + self.assertEquals(29, provider._request.call_count) provider._request.reset_mock() @@ -311,6 +339,56 @@ class TestCloudflareProvider(TestCase): "auto_added": False } }, + { + "id": "2a9140b17ffb0e6aed826049eec970b7", + "targets": [ + { + "target": "url", + "constraint": { + "operator": "matches", + "value": "urlfwd.unit.tests/" + } + } + ], + "actions": [ + { + "id": "forwarding_url", + "value": { + "url": "https://www.unit.tests", + "status_code": 302 + } + } + ], + "priority": 1, + "status": "active", + "created_on": "2021-06-25T20:10:50.000000Z", + "modified_on": "2021-06-28T22:38:10.000000Z" + }, + { + "id": "2a9141b18ffb0e6aed826050eec970b8", + "targets": [ + { + "target": "url", + "constraint": { + "operator": "matches", + "value": "urlfwdother.unit.tests/target" + } + } + ], + "actions": [ + { + "id": "forwarding_url", + "value": { + "url": "https://target.unit.tests", + "status_code": 301 + } + } + ], + "priority": 2, + "status": "active", + "created_on": "2021-06-25T20:10:50.000000Z", + "modified_on": "2021-06-28T22:38:10.000000Z" + }, ]) # we don't care about the POST/create return values @@ -319,7 +397,7 @@ class TestCloudflareProvider(TestCase): # Test out the create rate-limit handling, then 9 successes provider._request.side_effect = [ CloudflareRateLimitError('{}'), - ] + ([None] * 3) + ] + ([None] * 5) wanted = Zone('unit.tests.', []) wanted.add_record(Record.new(wanted, 'nc', { @@ -332,14 +410,27 @@ class TestCloudflareProvider(TestCase): 'type': 'A', 'value': '3.2.3.4' })) + wanted.add_record(Record.new(wanted, 'urlfwd', { + 'ttl': 3600, + 'type': 'URLFWD', + 'value': { + 'path': '/*', # path change + 'target': 'https://www.unit.tests/', # target change + 'code': 301, # status_code change + 'masking': '2', + 'query': 0, + } + })) plan = provider.plan(wanted) # only see the delete & ttl update, below min-ttl is filtered out - self.assertEquals(2, len(plan.changes)) - self.assertEquals(2, provider.apply(plan)) + self.assertEquals(4, len(plan.changes)) + self.assertEquals(4, provider.apply(plan)) self.assertTrue(plan.exists) # creates a the new value and then deletes all the old provider._request.assert_has_calls([ + call('DELETE', '/zones/42/' + 'pagerules/2a9141b18ffb0e6aed826050eec970b8'), call('DELETE', '/zones/ff12ab34cd5611334422ab3322997650/' 'dns_records/fc12ab34cd5611334422ab3322997653'), call('DELETE', '/zones/ff12ab34cd5611334422ab3322997650/' @@ -351,7 +442,29 @@ class TestCloudflareProvider(TestCase): 'name': 'ttl.unit.tests', 'proxied': False, 'ttl': 300 - }) + }), + call('PUT', '/zones/42/pagerules/' + '2a9140b17ffb0e6aed826049eec970b7', data={ + 'targets': [ + { + 'target': 'url', + 'constraint': { + 'operator': 'matches', + 'value': 'urlfwd.unit.tests/*' + } + } + ], + 'actions': [ + { + 'id': 'forwarding_url', + 'value': { + 'url': 'https://www.unit.tests/', + 'status_code': 301 + } + } + ], + 'status': 'active', + }), ]) def test_update_add_swap(self): @@ -500,6 +613,56 @@ class TestCloudflareProvider(TestCase): "auto_added": False } }, + { + "id": "2a9140b17ffb0e6aed826049eec974b7", + "targets": [ + { + "target": "url", + "constraint": { + "operator": "matches", + "value": "urlfwd1.unit.tests/" + } + } + ], + "actions": [ + { + "id": "forwarding_url", + "value": { + "url": "https://www.unit.tests", + "status_code": 302 + } + } + ], + "priority": 1, + "status": "active", + "created_on": "2021-06-25T20:10:50.000000Z", + "modified_on": "2021-06-28T22:38:10.000000Z" + }, + { + "id": "2a9141b18ffb0e6aed826054eec970b8", + "targets": [ + { + "target": "url", + "constraint": { + "operator": "matches", + "value": "urlfwd1.unit.tests/target" + } + } + ], + "actions": [ + { + "id": "forwarding_url", + "value": { + "url": "https://target.unit.tests", + "status_code": 301 + } + } + ], + "priority": 2, + "status": "active", + "created_on": "2021-06-25T20:10:50.000000Z", + "modified_on": "2021-06-28T22:38:10.000000Z" + }, ]) provider._request = Mock() @@ -513,6 +676,8 @@ class TestCloudflareProvider(TestCase): }, # zone create None, None, + None, + None, ] # Add something and delete something @@ -523,14 +688,46 @@ class TestCloudflareProvider(TestCase): # This matches the zone data above, one to delete, one to leave 'values': ['ns1.foo.bar.', 'ns2.foo.bar.'], }) + exstingurlfwd = Record.new(zone, 'urlfwd1', { + 'ttl': 3600, + 'type': 'URLFWD', + 'values': [ + { + 'path': '/', + 'target': 'https://www.unit.tests', + 'code': 302, + 'masking': '2', + 'query': 0, + }, + { + 'path': '/target', + 'target': 'https://target.unit.tests', + 'code': 301, + 'masking': '2', + 'query': 0, + } + ] + }) new = Record.new(zone, '', { 'ttl': 300, 'type': 'NS', # This leaves one and deletes one 'value': 'ns2.foo.bar.', }) + newurlfwd = Record.new(zone, 'urlfwd1', { + 'ttl': 3600, + 'type': 'URLFWD', + 'value': { + 'path': '/', + 'target': 'https://www.unit.tests', + 'code': 302, + 'masking': '2', + 'query': 0, + } + }) change = Update(existing, new) - plan = Plan(zone, zone, [change], True) + changeurlfwd = Update(exstingurlfwd, newurlfwd) + plan = Plan(zone, zone, [change, changeurlfwd], True) provider._apply(plan) # Get zones, create zone, create a record, delete a record @@ -548,7 +745,31 @@ class TestCloudflareProvider(TestCase): 'ttl': 300 }), call('DELETE', '/zones/42/dns_records/' - 'fc12ab34cd5611334422ab3322997653') + 'fc12ab34cd5611334422ab3322997653'), + call('PUT', '/zones/42/pagerules/' + '2a9140b17ffb0e6aed826049eec974b7', data={ + 'targets': [ + { + 'target': 'url', + 'constraint': { + 'operator': 'matches', + 'value': 'urlfwd1.unit.tests/' + } + } + ], + 'actions': [ + { + 'id': 'forwarding_url', + 'value': { + 'url': 'https://www.unit.tests', + 'status_code': 302 + } + } + ], + 'status': 'active' + }), + call('DELETE', '/zones/42/pagerules/' + '2a9141b18ffb0e6aed826054eec970b8'), ]) def test_ptr(self):