From 1b95724e1706cf1656854edad8880da1e50dc5a5 Mon Sep 17 00:00:00 2001 From: Sham Date: Mon, 28 Jun 2021 14:18:47 -0700 Subject: [PATCH 01/21] Adding SX and UM to NA countries --- octodns/provider/ns1.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py index bf08358..9e720cd 100644 --- a/octodns/provider/ns1.py +++ b/octodns/provider/ns1.py @@ -363,7 +363,8 @@ class Ns1Provider(BaseProvider): 'NA': {'DO', 'DM', 'BB', 'BL', 'BM', 'HT', 'KN', 'JM', 'VC', 'HN', 'BS', 'BZ', 'PR', 'NI', 'LC', 'TT', 'VG', 'PA', 'TC', 'PM', 'GT', 'AG', 'GP', 'AI', 'VI', 'CA', 'GD', 'AW', 'CR', 'GL', - 'CU', 'MF', 'SV', 'US', 'MQ', 'MS', 'KY', 'MX', 'CW', 'BQ'} + 'CU', 'MF', 'SV', 'US', 'MQ', 'MS', 'KY', 'MX', 'CW', 'BQ', + 'SX', 'UM'} } def __init__(self, id, api_key, retry_count=4, monitor_regions=None, From fec33bbb10730d7efaf7f1ca2a7ef0aef5f0b0b4 Mon Sep 17 00:00:00 2001 From: jozo Date: Mon, 12 Jul 2021 11:29:59 +0200 Subject: [PATCH 02/21] Fix missing dot in example --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 733b087..999d8b4 100644 --- a/README.md +++ b/README.md @@ -89,7 +89,7 @@ zones: - dyn - route53 - example.net: + example.net.: alias: example.com. ``` From 1a1391bf67502a455f7c284c83bfc3c07f55ab3d Mon Sep 17 00:00:00 2001 From: Viranch Mehta Date: Mon, 12 Jul 2021 12:49:31 -0700 Subject: [PATCH 03/21] Fix git URL in readme --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 8486799..0056ba3 100644 --- a/README.md +++ b/README.md @@ -53,7 +53,7 @@ $ mkdir config If you'd like to install a version that has not yet been released in a repetable/safe manner you can do the following. In general octoDNS is fairly stable inbetween releases thanks to the plan and apply process, but care should be taken regardless. ```shell -$ pip install -e git+https://git@github.com/github/octodns.git@#egg=octodns +$ pip install -e git+https://git@github.com/octodns/octodns.git@#egg=octodns ``` ### Config From bb5dbd7d420a27107383d22f27cf789d344921a5 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sun, 18 Jul 2021 14:04:12 -0700 Subject: [PATCH 04/21] v0.9.13 version bump and CHANGELOG update --- CHANGELOG.md | 12 +++++++++++- octodns/__init__.py | 2 +- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e6dcb48..dca36a4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,4 @@ -## v0.9.13 - 2021-..-.. - +## v0.9.13 - 2021-07-18 - Processors Alpha #### Noteworthy changes @@ -14,6 +14,16 @@ America list for backwards compatibility reasons. They will be added in the next releaser. +#### Stuff + +* Lots of progress on the partial/beta support for dynamic records in Azure, + still not production ready. +* NS1 fix for when a pool only exists as a fallback +* Zone level lenient flag +* Validate weight makes sense for pools with a single record +* UltraDNS support for aliases and general fixes/improvements +* Misc doc fixes and improvements + ## v0.9.12 - 2021-04-30 - Enough time has passed #### Noteworthy changes diff --git a/octodns/__init__.py b/octodns/__init__.py index 1885d42..16ec066 100644 --- a/octodns/__init__.py +++ b/octodns/__init__.py @@ -3,4 +3,4 @@ from __future__ import absolute_import, division, print_function, \ unicode_literals -__VERSION__ = '0.9.12' +__VERSION__ = '0.9.13' From 1de2521c7a37923e0bc6c0482de5a8de7a49ce3a Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sun, 18 Jul 2021 14:25:44 -0700 Subject: [PATCH 05/21] Changelog entry about SX and UM inclusion in NA for NS1 --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index dca36a4..55a88c2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,10 @@ +## v0.9.14 - 2021-??-?? - ... + +#### Noteworthy changes + +* NS1 NA target now includes `SX` and `UM`. If `NA` continent is in use in + dynamic records care must be taken to upgrade/downgrade to v0.9.13. + ## v0.9.13 - 2021-07-18 - Processors Alpha #### Noteworthy changes From 5361cadd1c736e9b7dcd4f2c6287d49e824d899d Mon Sep 17 00:00:00 2001 From: Brian E Clow Date: Thu, 27 May 2021 15:36:07 -0700 Subject: [PATCH 06/21] Adding URLFWD to record framework --- octodns/record/__init__.py | 86 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/octodns/record/__init__.py b/octodns/record/__init__.py index 3ab8263..15cb143 100644 --- a/octodns/record/__init__.py +++ b/octodns/record/__init__.py @@ -106,6 +106,7 @@ class Record(EqualityTupleMixin): 'SRV': SrvRecord, 'SSHFP': SshfpRecord, 'TXT': TxtRecord, + 'URLFWD': UrlfwdRecord, }[_type] except KeyError: raise Exception('Unknown record type: "{}"'.format(_type)) @@ -1467,3 +1468,88 @@ class _TxtValue(_ChunkedValue): class TxtRecord(_ChunkedValuesMixin, Record): _type = 'TXT' _value_type = _TxtValue + + +class UrlfwdValue(EqualityTupleMixin): + # TODO: should have defaults for path, code, masking, and query + + VALID_CODES = (301, 302) + VALID_MASKS = (0, 1, 2) + VALID_QUERY = (0, 1) + + @classmethod + def validate(cls, data, _type): + if not isinstance(data, (list, tuple)): + data = (data,) + reasons = [] + for value in data: + try: + code = int(value['code']) + if code not in cls.VALID_CODES: + reasons.append('unrecognized return code "{}"' + .format(code)) + except KeyError: + reasons.append('missing code') + except ValueError: + reasons.append('invalid return code "{}"' + .format(value['code'])) + try: + masking = int(value['masking']) + if masking not in cls.VALID_MASKS: + reasons.append('unrecognized masking setting "{}"' + .format(masking)) + except KeyError: + reasons.append('missing masking') + except ValueError: + reasons.append('invalid masking setting "{}"' + .format(value['masking'])) + try: + query = int(value['query']) + if query not in cls.VALID_QUERY: + reasons.append('unrecognized query setting "{}"' + .format(query)) + except KeyError: + reasons.append('missing query') + except ValueError: + reasons.append('invalid query setting "{}"' + .format(value['query'])) + for k in ('path', 'target'): + if k not in value: + reasons.append('missing {}'.format(k)) + return reasons + + @classmethod + def process(cls, values): + return [UrlfwdValue(v) for v in values] + + def __init__(self, value): + self.path = value['path'] + self.target = value['target'] + self.code = int(value['code']) + self.masking = int(value['masking']) + self.query = int(value['query']) + + @property + def data(self): + return { + 'path': self.path, + 'target': self.target, + 'code': self.code, + 'masking': self.masking, + 'query': self.query, + } + + def __hash__(self): + return hash(self.__repr__()) + + def _equality_tuple(self): + return (self.path, self.target, self.code, self.masking, self.query) + + def __repr__(self): + return '"{}" "{}" {} {} {}'.format(self.path, self.target, self.code, + self.masking, self.query) + + +class UrlfwdRecord(_ValuesMixin, Record): + _type = 'URLFWD' + _value_type = UrlfwdValue From 2a6480bc05e15ef30a8fa578a58da5c229a840cf Mon Sep 17 00:00:00 2001 From: Brian E Clow Date: Thu, 27 May 2021 15:37:55 -0700 Subject: [PATCH 07/21] Adding URLFWD record testing --- tests/test_octodns_record.py | 307 ++++++++++++++++++++++++++++++++++- 1 file changed, 305 insertions(+), 2 deletions(-) diff --git a/tests/test_octodns_record.py b/tests/test_octodns_record.py index 315670e..3bd48e5 100644 --- a/tests/test_octodns_record.py +++ b/tests/test_octodns_record.py @@ -12,8 +12,8 @@ from octodns.record import ARecord, AaaaRecord, AliasRecord, CaaRecord, \ CaaValue, CnameRecord, DnameRecord, Create, Delete, GeoValue, LocRecord, \ LocValue, MxRecord, MxValue, NaptrRecord, NaptrValue, NsRecord, \ PtrRecord, Record, SshfpRecord, SshfpValue, SpfRecord, SrvRecord, \ - SrvValue, TxtRecord, Update, ValidationError, _Dynamic, _DynamicPool, \ - _DynamicRule + SrvValue, TxtRecord, Update, UrlfwdRecord, UrlfwdValue, ValidationError, \ + _Dynamic, _DynamicPool, _DynamicRule from octodns.zone import Zone from helpers import DynamicProvider, GeoProvider, SimpleProvider @@ -884,6 +884,112 @@ class TestRecord(TestCase): b_value = 'b other' self.assertMultipleValues(TxtRecord, a_values, b_value) + def test_urlfwd(self): + a_values = [{ + 'path': '/', + 'target': 'http://foo', + 'code': 301, + 'masking': 2, + 'query': 0, + }, { + 'path': '/target', + 'target': 'http://target', + 'code': 302, + 'masking': 2, + 'query': 0, + }] + a_data = {'ttl': 30, 'values': a_values} + a = UrlfwdRecord(self.zone, 'a', a_data) + self.assertEquals('a', a.name) + self.assertEquals('a.unit.tests.', a.fqdn) + self.assertEquals(30, a.ttl) + self.assertEquals(a_values[0]['path'], a.values[0].path) + self.assertEquals(a_values[0]['target'], a.values[0].target) + self.assertEquals(a_values[0]['code'], a.values[0].code) + self.assertEquals(a_values[0]['masking'], a.values[0].masking) + self.assertEquals(a_values[0]['query'], a.values[0].query) + self.assertEquals(a_values[1]['path'], a.values[1].path) + self.assertEquals(a_values[1]['target'], a.values[1].target) + self.assertEquals(a_values[1]['code'], a.values[1].code) + self.assertEquals(a_values[1]['masking'], a.values[1].masking) + self.assertEquals(a_values[1]['query'], a.values[1].query) + self.assertEquals(a_data, a.data) + + b_value = { + 'path': '/', + 'target': 'http://location', + 'code': 301, + 'masking': 2, + 'query': 0, + } + b_data = {'ttl': 30, 'value': b_value} + b = UrlfwdRecord(self.zone, 'b', b_data) + self.assertEquals(b_value['path'], b.values[0].path) + self.assertEquals(b_value['target'], b.values[0].target) + self.assertEquals(b_value['code'], b.values[0].code) + self.assertEquals(b_value['masking'], b.values[0].masking) + self.assertEquals(b_value['query'], b.values[0].query) + self.assertEquals(b_data, b.data) + + target = SimpleProvider() + # No changes with self + self.assertFalse(a.changes(a, target)) + # Diff in path causes change + other = UrlfwdRecord(self.zone, 'a', {'ttl': 30, 'values': a_values}) + other.values[0].path = '/change' + change = a.changes(other, target) + self.assertEqual(change.existing, a) + self.assertEqual(change.new, other) + # Diff in target causes change + other = UrlfwdRecord(self.zone, 'a', {'ttl': 30, 'values': a_values}) + other.values[0].target = 'http://target' + change = a.changes(other, target) + self.assertEqual(change.existing, a) + self.assertEqual(change.new, other) + # Diff in code causes change + other = UrlfwdRecord(self.zone, 'a', {'ttl': 30, 'values': a_values}) + other.values[0].code = 302 + change = a.changes(other, target) + self.assertEqual(change.existing, a) + self.assertEqual(change.new, other) + # Diff in masking causes change + other = UrlfwdRecord(self.zone, 'a', {'ttl': 30, 'values': a_values}) + other.values[0].masking = 0 + change = a.changes(other, target) + self.assertEqual(change.existing, a) + self.assertEqual(change.new, other) + # Diff in query causes change + other = UrlfwdRecord(self.zone, 'a', {'ttl': 30, 'values': a_values}) + other.values[0].query = 1 + change = a.changes(other, target) + self.assertEqual(change.existing, a) + self.assertEqual(change.new, other) + + # hash + v = UrlfwdValue({ + 'path': '/', + 'target': 'http://place', + 'code': 301, + 'masking': 2, + 'query': 0, + }) + o = UrlfwdValue({ + 'path': '/location', + 'target': 'http://redirect', + 'code': 302, + 'masking': 2, + 'query': 0, + }) + values = set() + values.add(v) + self.assertTrue(v in values) + self.assertFalse(o in values) + values.add(o) + self.assertTrue(o in values) + + # __repr__ doesn't blow up + a.__repr__() + def test_record_new(self): txt = Record.new(self.zone, 'txt', { 'ttl': 44, @@ -3019,6 +3125,203 @@ class TestRecordValidation(TestCase): # should be chunked values, with quoting self.assertEquals(single.chunked_values, chunked.chunked_values) + def test_URLFWD(self): + # doesn't blow up + Record.new(self.zone, '', { + 'type': 'URLFWD', + 'ttl': 600, + 'value': { + 'path': '/', + 'target': 'http://foo', + 'code': 301, + 'masking': 2, + 'query': 0, + } + }) + Record.new(self.zone, '', { + 'type': 'URLFWD', + 'ttl': 600, + 'values': [{ + 'path': '/', + 'target': 'http://foo', + 'code': 301, + 'masking': 2, + 'query': 0, + }, { + 'path': '/target', + 'target': 'http://target', + 'code': 302, + 'masking': 2, + 'query': 0, + }] + }) + + # missing path + with self.assertRaises(ValidationError) as ctx: + Record.new(self.zone, '', { + 'type': 'URLFWD', + 'ttl': 600, + 'value': { + 'target': 'http://foo', + 'code': 301, + 'masking': 2, + 'query': 0, + } + }) + self.assertEquals(['missing path'], ctx.exception.reasons) + + # missing target + with self.assertRaises(ValidationError) as ctx: + Record.new(self.zone, '', { + 'type': 'URLFWD', + 'ttl': 600, + 'value': { + 'path': '/', + 'code': 301, + 'masking': 2, + 'query': 0, + } + }) + self.assertEquals(['missing target'], ctx.exception.reasons) + + # missing code + with self.assertRaises(ValidationError) as ctx: + Record.new(self.zone, '', { + 'type': 'URLFWD', + 'ttl': 600, + 'value': { + 'path': '/', + 'target': 'http://foo', + 'masking': 2, + 'query': 0, + } + }) + self.assertEquals(['missing code'], ctx.exception.reasons) + + # invalid code + with self.assertRaises(ValidationError) as ctx: + Record.new(self.zone, '', { + 'type': 'URLFWD', + 'ttl': 600, + 'value': { + 'path': '/', + 'target': 'http://foo', + 'code': 'nope', + 'masking': 2, + 'query': 0, + } + }) + self.assertEquals(['invalid return code "nope"'], + ctx.exception.reasons) + + # unrecognized code + with self.assertRaises(ValidationError) as ctx: + Record.new(self.zone, '', { + 'type': 'URLFWD', + 'ttl': 600, + 'value': { + 'path': '/', + 'target': 'http://foo', + 'code': 3, + 'masking': 2, + 'query': 0, + } + }) + self.assertEquals(['unrecognized return code "3"'], + ctx.exception.reasons) + + # missing masking + with self.assertRaises(ValidationError) as ctx: + Record.new(self.zone, '', { + 'type': 'URLFWD', + 'ttl': 600, + 'value': { + 'path': '/', + 'target': 'http://foo', + 'code': 301, + 'query': 0, + } + }) + self.assertEquals(['missing masking'], ctx.exception.reasons) + + # invalid masking + with self.assertRaises(ValidationError) as ctx: + Record.new(self.zone, '', { + 'type': 'URLFWD', + 'ttl': 600, + 'value': { + 'path': '/', + 'target': 'http://foo', + 'code': 301, + 'masking': 'nope', + 'query': 0, + } + }) + self.assertEquals(['invalid masking setting "nope"'], + ctx.exception.reasons) + + # unrecognized masking + with self.assertRaises(ValidationError) as ctx: + Record.new(self.zone, '', { + 'type': 'URLFWD', + 'ttl': 600, + 'value': { + 'path': '/', + 'target': 'http://foo', + 'code': 301, + 'masking': 3, + 'query': 0, + } + }) + self.assertEquals(['unrecognized masking setting "3"'], + ctx.exception.reasons) + + # missing query + with self.assertRaises(ValidationError) as ctx: + Record.new(self.zone, '', { + 'type': 'URLFWD', + 'ttl': 600, + 'value': { + 'path': '/', + 'target': 'http://foo', + 'code': 301, + 'masking': 2, + } + }) + self.assertEquals(['missing query'], ctx.exception.reasons) + + # invalid query + with self.assertRaises(ValidationError) as ctx: + Record.new(self.zone, '', { + 'type': 'URLFWD', + 'ttl': 600, + 'value': { + 'path': '/', + 'target': 'http://foo', + 'code': 301, + 'masking': 2, + 'query': 'nope', + } + }) + self.assertEquals(['invalid query setting "nope"'], + ctx.exception.reasons) + + # unrecognized query + with self.assertRaises(ValidationError) as ctx: + Record.new(self.zone, '', { + 'type': 'URLFWD', + 'ttl': 600, + 'value': { + 'path': '/', + 'target': 'http://foo', + 'code': 301, + 'masking': 2, + 'query': 3, + } + }) + self.assertEquals(['unrecognized query setting "3"'], + ctx.exception.reasons) + class TestDynamicRecords(TestCase): zone = Zone('unit.tests.', []) From c5efba89fe68551b1029899b65af66416ef652ef Mon Sep 17 00:00:00 2001 From: Brian E Clow Date: Thu, 27 May 2021 15:40:44 -0700 Subject: [PATCH 08/21] Adding yaml support and testing for URLFWD --- octodns/provider/yaml.py | 3 ++- tests/config/unit.tests.yaml | 14 ++++++++++++++ tests/test_octodns_provider_yaml.py | 20 +++++++++++--------- 3 files changed, 27 insertions(+), 10 deletions(-) diff --git a/octodns/provider/yaml.py b/octodns/provider/yaml.py index 8314f38..b3dd2d9 100644 --- a/octodns/provider/yaml.py +++ b/octodns/provider/yaml.py @@ -105,7 +105,8 @@ class YamlProvider(BaseProvider): SUPPORTS_GEO = True SUPPORTS_DYNAMIC = True SUPPORTS = set(('A', 'AAAA', 'ALIAS', 'CAA', 'CNAME', 'DNAME', 'LOC', 'MX', - 'NAPTR', 'NS', 'PTR', 'SSHFP', 'SPF', 'SRV', 'TXT')) + 'NAPTR', 'NS', 'PTR', 'SSHFP', 'SPF', 'SRV', 'TXT', + 'URLFWD')) def __init__(self, id, directory, default_ttl=3600, enforce_order=True, populate_should_replace=False, *args, **kwargs): diff --git a/tests/config/unit.tests.yaml b/tests/config/unit.tests.yaml index 39e5326..c70b20c 100644 --- a/tests/config/unit.tests.yaml +++ b/tests/config/unit.tests.yaml @@ -169,6 +169,20 @@ txt: - Bah bah black sheep - have you any wool. - 'v=DKIM1\;k=rsa\;s=email\;h=sha256\;p=A/kinda+of/long/string+with+numb3rs' +urlfwd: + ttl: 300 + type: URLFWD + values: + - code: 302 + masking: 2 + path: '/' + query: 0 + target: 'http://www.unit.tests' + - code: 301 + masking: 2 + path: '/target' + query: 0 + target: 'http://target.unit.tests' www: ttl: 300 type: A diff --git a/tests/test_octodns_provider_yaml.py b/tests/test_octodns_provider_yaml.py index 872fcca..7e4f6f7 100644 --- a/tests/test_octodns_provider_yaml.py +++ b/tests/test_octodns_provider_yaml.py @@ -35,7 +35,7 @@ class TestYamlProvider(TestCase): # without it we see everything source.populate(zone) - self.assertEquals(22, len(zone.records)) + self.assertEquals(23, len(zone.records)) source.populate(dynamic_zone) self.assertEquals(6, len(dynamic_zone.records)) @@ -58,12 +58,12 @@ class TestYamlProvider(TestCase): # We add everything plan = target.plan(zone) - self.assertEquals(19, len([c for c in plan.changes + self.assertEquals(20, len([c for c in plan.changes if isinstance(c, Create)])) self.assertFalse(isfile(yaml_file)) # Now actually do it - self.assertEquals(19, target.apply(plan)) + self.assertEquals(20, target.apply(plan)) self.assertTrue(isfile(yaml_file)) # Dynamic plan @@ -87,7 +87,7 @@ class TestYamlProvider(TestCase): # A 2nd sync should still create everything plan = target.plan(zone) - self.assertEquals(19, len([c for c in plan.changes + self.assertEquals(20, len([c for c in plan.changes if isinstance(c, Create)])) with open(yaml_file) as fh: @@ -107,6 +107,7 @@ class TestYamlProvider(TestCase): self.assertTrue('values' in data.pop('sub')) self.assertTrue('values' in data.pop('txt')) self.assertTrue('values' in data.pop('loc')) + self.assertTrue('values' in data.pop('urlfwd')) # these are stored as singular 'value' self.assertTrue('value' in data.pop('_imap._tcp')) self.assertTrue('value' in data.pop('_pop3._tcp')) @@ -248,7 +249,7 @@ class TestSplitYamlProvider(TestCase): # without it we see everything source.populate(zone) - self.assertEquals(19, len(zone.records)) + self.assertEquals(20, len(zone.records)) source.populate(dynamic_zone) self.assertEquals(5, len(dynamic_zone.records)) @@ -263,12 +264,12 @@ class TestSplitYamlProvider(TestCase): # We add everything plan = target.plan(zone) - self.assertEquals(16, len([c for c in plan.changes + self.assertEquals(17, len([c for c in plan.changes if isinstance(c, Create)])) self.assertFalse(isdir(zone_dir)) # Now actually do it - self.assertEquals(16, target.apply(plan)) + self.assertEquals(17, target.apply(plan)) # Dynamic plan plan = target.plan(dynamic_zone) @@ -291,7 +292,7 @@ class TestSplitYamlProvider(TestCase): # A 2nd sync should still create everything plan = target.plan(zone) - self.assertEquals(16, len([c for c in plan.changes + self.assertEquals(17, len([c for c in plan.changes if isinstance(c, Create)])) yaml_file = join(zone_dir, '$unit.tests.yaml') @@ -306,7 +307,8 @@ class TestSplitYamlProvider(TestCase): # These records are stored as plural "values." Check each file to # ensure correctness. - for record_name in ('_srv._tcp', 'mx', 'naptr', 'sub', 'txt'): + for record_name in ('_srv._tcp', 'mx', 'naptr', 'sub', 'txt', + 'urlfwd'): yaml_file = join(zone_dir, '{}.yaml'.format(record_name)) self.assertTrue(isfile(yaml_file)) with open(yaml_file) as fh: From 9be1195d47c72a4377a9af3bc45a334e12c6ba9e Mon Sep 17 00:00:00 2001 From: Brian E Clow Date: Thu, 27 May 2021 15:42:27 -0700 Subject: [PATCH 09/21] SplitYAML testing --- tests/config/split/unit.tests.tst/urlfwd.yaml | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 tests/config/split/unit.tests.tst/urlfwd.yaml diff --git a/tests/config/split/unit.tests.tst/urlfwd.yaml b/tests/config/split/unit.tests.tst/urlfwd.yaml new file mode 100644 index 0000000..778b9b5 --- /dev/null +++ b/tests/config/split/unit.tests.tst/urlfwd.yaml @@ -0,0 +1,15 @@ +--- +urlfwd: + ttl: 300 + type: URLFWD + values: + - code: 302 + masking: 2 + path: '/' + query: 0 + target: 'http://www.unit.tests' + - code: 301 + masking: 2 + path: '/target' + query: 0 + target: 'http://target.unit.tests' From 21fcff920e7cf15ac39a4dbf0b62477d5b00faec Mon Sep 17 00:00:00 2001 From: Brian E Clow Date: Thu, 27 May 2021 15:43:36 -0700 Subject: [PATCH 10/21] Adding NS1 URLFWD support and testing --- octodns/provider/ns1.py | 24 +++++++++++++++++++++++- tests/test_octodns_provider_ns1.py | 18 +++++++++++++++++- 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py index 4886c00..6d4f84d 100644 --- a/octodns/provider/ns1.py +++ b/octodns/provider/ns1.py @@ -234,7 +234,7 @@ class Ns1Provider(BaseProvider): SUPPORTS_GEO = True SUPPORTS_DYNAMIC = True SUPPORTS = set(('A', 'AAAA', 'ALIAS', 'CAA', 'CNAME', 'MX', 'NAPTR', - 'NS', 'PTR', 'SPF', 'SRV', 'TXT')) + 'NS', 'PTR', 'SPF', 'SRV', 'TXT', 'URLFWD')) ZONE_NOT_FOUND_MESSAGE = 'server error: zone not found' @@ -749,6 +749,23 @@ class Ns1Provider(BaseProvider): 'values': values, } + def _data_for_URLFWD(self, _type, record): + values = [] + for answer in record['short_answers']: + path, target, code, masking, query = answer.split(' ', 4) + values.append({ + 'path': path, + 'target': target, + 'code': code, + 'masking': masking, + 'query': query, + }) + return { + 'ttl': record['ttl'], + 'type': _type, + 'values': values, + } + def populate(self, zone, target=False, lenient=False): self.log.debug('populate: name=%s, target=%s, lenient=%s', zone.name, @@ -1244,6 +1261,11 @@ class Ns1Provider(BaseProvider): for v in record.values] return {'answers': values, 'ttl': record.ttl}, None + def _params_for_URLFWD(self, record): + values = [(v.path, v.target, v.code, v.masking, v.query) + for v in record.values] + return {'answers': values, 'ttl': record.ttl}, None + def _get_ns1_filters(self, ns1_zone_name): ns1_filters = {} ns1_zone = {} diff --git a/tests/test_octodns_provider_ns1.py b/tests/test_octodns_provider_ns1.py index e21fe0d..875ebbf 100644 --- a/tests/test_octodns_provider_ns1.py +++ b/tests/test_octodns_provider_ns1.py @@ -109,6 +109,17 @@ class TestNs1Provider(TestCase): 'value': 'ca.unit.tests', }, })) + expected.add(Record.new(zone, 'urlfwd', { + 'ttl': 41, + 'type': 'URLFWD', + 'value': { + 'path': '/', + 'target': 'http://foo.unit.tests', + 'code': 301, + 'masking': 2, + 'query': 0, + }, + })) ns1_records = [{ 'type': 'A', @@ -164,6 +175,11 @@ class TestNs1Provider(TestCase): 'ttl': 40, 'short_answers': ['0 issue ca.unit.tests'], 'domain': 'unit.tests.', + }, { + 'type': 'URLFWD', + 'ttl': 41, + 'short_answers': ['/ http://foo.unit.tests 301 2 0'], + 'domain': 'urlfwd.unit.tests.', }] @patch('ns1.rest.records.Records.retrieve') @@ -345,7 +361,7 @@ class TestNs1Provider(TestCase): # Test out the create rate-limit handling, then 9 successes record_create_mock.side_effect = [ RateLimitException('boo', period=0), - ] + ([None] * 9) + ] + ([None] * 10) got_n = provider.apply(plan) self.assertEquals(expected_n, got_n) From f4caa35caa80dd1b8d898d8d1989130bad528f6e Mon Sep 17 00:00:00 2001 From: Brian E Clow Date: Thu, 27 May 2021 15:45:41 -0700 Subject: [PATCH 11/21] Ignoring URLFWD and adjusting test counts for other providers --- tests/test_octodns_manager.py | 14 +++++++------- tests/test_octodns_provider_constellix.py | 2 +- tests/test_octodns_provider_digitalocean.py | 2 +- tests/test_octodns_provider_dnsimple.py | 2 +- tests/test_octodns_provider_dnsmadeeasy.py | 2 +- tests/test_octodns_provider_easydns.py | 2 +- tests/test_octodns_provider_gandi.py | 2 +- tests/test_octodns_provider_hetzner.py | 2 +- tests/test_octodns_provider_powerdns.py | 4 ++-- 9 files changed, 16 insertions(+), 16 deletions(-) diff --git a/tests/test_octodns_manager.py b/tests/test_octodns_manager.py index 8bada06..96f67fd 100644 --- a/tests/test_octodns_manager.py +++ b/tests/test_octodns_manager.py @@ -121,12 +121,12 @@ class TestManager(TestCase): environ['YAML_TMP_DIR'] = tmpdir.dirname tc = Manager(get_config_filename('simple.yaml')) \ .sync(dry_run=False) - self.assertEquals(25, tc) + self.assertEquals(26, tc) # try with just one of the zones tc = Manager(get_config_filename('simple.yaml')) \ .sync(dry_run=False, eligible_zones=['unit.tests.']) - self.assertEquals(19, tc) + self.assertEquals(20, tc) # the subzone, with 2 targets tc = Manager(get_config_filename('simple.yaml')) \ @@ -141,18 +141,18 @@ class TestManager(TestCase): # Again with force tc = Manager(get_config_filename('simple.yaml')) \ .sync(dry_run=False, force=True) - self.assertEquals(25, tc) + self.assertEquals(26, tc) # Again with max_workers = 1 tc = Manager(get_config_filename('simple.yaml'), max_workers=1) \ .sync(dry_run=False, force=True) - self.assertEquals(25, tc) + self.assertEquals(26, tc) # Include meta tc = Manager(get_config_filename('simple.yaml'), max_workers=1, include_meta=True) \ .sync(dry_run=False, force=True) - self.assertEquals(29, tc) + self.assertEquals(30, tc) def test_eligible_sources(self): with TemporaryDirectory() as tmpdir: @@ -218,13 +218,13 @@ class TestManager(TestCase): fh.write('---\n{}') changes = manager.compare(['in'], ['dump'], 'unit.tests.') - self.assertEquals(19, len(changes)) + self.assertEquals(20, len(changes)) # Compound sources with varying support changes = manager.compare(['in', 'nosshfp'], ['dump'], 'unit.tests.') - self.assertEquals(18, len(changes)) + self.assertEquals(19, len(changes)) with self.assertRaises(ManagerException) as ctx: manager.compare(['nope'], ['dump'], 'unit.tests.') diff --git a/tests/test_octodns_provider_constellix.py b/tests/test_octodns_provider_constellix.py index e9ece0e..38b7ab9 100644 --- a/tests/test_octodns_provider_constellix.py +++ b/tests/test_octodns_provider_constellix.py @@ -132,7 +132,7 @@ class TestConstellixProvider(TestCase): plan = provider.plan(self.expected) # No root NS, no ignored, no excluded, no unsupported - n = len(self.expected.records) - 7 + n = len(self.expected.records) - 8 self.assertEquals(n, len(plan.changes)) self.assertEquals(n, provider.apply(plan)) diff --git a/tests/test_octodns_provider_digitalocean.py b/tests/test_octodns_provider_digitalocean.py index affd140..9ed54bf 100644 --- a/tests/test_octodns_provider_digitalocean.py +++ b/tests/test_octodns_provider_digitalocean.py @@ -163,7 +163,7 @@ class TestDigitalOceanProvider(TestCase): plan = provider.plan(self.expected) # No root NS, no ignored, no excluded, no unsupported - n = len(self.expected.records) - 9 + n = len(self.expected.records) - 10 self.assertEquals(n, len(plan.changes)) self.assertEquals(n, provider.apply(plan)) self.assertFalse(plan.exists) diff --git a/tests/test_octodns_provider_dnsimple.py b/tests/test_octodns_provider_dnsimple.py index be881e4..0b8d209 100644 --- a/tests/test_octodns_provider_dnsimple.py +++ b/tests/test_octodns_provider_dnsimple.py @@ -137,7 +137,7 @@ class TestDnsimpleProvider(TestCase): plan = provider.plan(self.expected) # No root NS, no ignored, no excluded - n = len(self.expected.records) - 7 + n = len(self.expected.records) - 8 self.assertEquals(n, len(plan.changes)) self.assertEquals(n, provider.apply(plan)) self.assertFalse(plan.exists) diff --git a/tests/test_octodns_provider_dnsmadeeasy.py b/tests/test_octodns_provider_dnsmadeeasy.py index dc104b7..9efc81d 100644 --- a/tests/test_octodns_provider_dnsmadeeasy.py +++ b/tests/test_octodns_provider_dnsmadeeasy.py @@ -134,7 +134,7 @@ class TestDnsMadeEasyProvider(TestCase): plan = provider.plan(self.expected) # No root NS, no ignored, no excluded, no unsupported - n = len(self.expected.records) - 9 + n = len(self.expected.records) - 10 self.assertEquals(n, len(plan.changes)) self.assertEquals(n, provider.apply(plan)) diff --git a/tests/test_octodns_provider_easydns.py b/tests/test_octodns_provider_easydns.py index a6de8a9..85492eb 100644 --- a/tests/test_octodns_provider_easydns.py +++ b/tests/test_octodns_provider_easydns.py @@ -374,7 +374,7 @@ class TestEasyDNSProvider(TestCase): plan = provider.plan(self.expected) # No root NS, no ignored, no excluded, no unsupported - n = len(self.expected.records) - 8 + n = len(self.expected.records) - 9 self.assertEquals(n, len(plan.changes)) self.assertEquals(n, provider.apply(plan)) self.assertFalse(plan.exists) diff --git a/tests/test_octodns_provider_gandi.py b/tests/test_octodns_provider_gandi.py index 26ffeee..f2e3028 100644 --- a/tests/test_octodns_provider_gandi.py +++ b/tests/test_octodns_provider_gandi.py @@ -193,7 +193,7 @@ class TestGandiProvider(TestCase): plan = provider.plan(self.expected) # No root NS, no ignored, no excluded, no LOC - n = len(self.expected.records) - 5 + n = len(self.expected.records) - 6 self.assertEquals(n, len(plan.changes)) self.assertEquals(n, provider.apply(plan)) self.assertFalse(plan.exists) diff --git a/tests/test_octodns_provider_hetzner.py b/tests/test_octodns_provider_hetzner.py index 4167944..218a6b2 100644 --- a/tests/test_octodns_provider_hetzner.py +++ b/tests/test_octodns_provider_hetzner.py @@ -108,7 +108,7 @@ class TestHetznerProvider(TestCase): plan = provider.plan(self.expected) # No root NS, no ignored, no excluded, no unsupported - n = len(self.expected.records) - 9 + n = len(self.expected.records) - 10 self.assertEquals(n, len(plan.changes)) self.assertEquals(n, provider.apply(plan)) self.assertFalse(plan.exists) diff --git a/tests/test_octodns_provider_powerdns.py b/tests/test_octodns_provider_powerdns.py index 5775f41..92211d1 100644 --- a/tests/test_octodns_provider_powerdns.py +++ b/tests/test_octodns_provider_powerdns.py @@ -185,7 +185,7 @@ class TestPowerDnsProvider(TestCase): expected = Zone('unit.tests.', []) source = YamlProvider('test', join(dirname(__file__), 'config')) source.populate(expected) - expected_n = len(expected.records) - 3 + expected_n = len(expected.records) - 4 self.assertEquals(19, expected_n) # No diffs == no changes @@ -291,7 +291,7 @@ class TestPowerDnsProvider(TestCase): expected = Zone('unit.tests.', []) source = YamlProvider('test', join(dirname(__file__), 'config')) source.populate(expected) - self.assertEquals(22, len(expected.records)) + self.assertEquals(23, len(expected.records)) # A small change to a single record with requests_mock() as mock: From 75423fd786f70e4fc42ae79f4c1737d2bd5d1777 Mon Sep 17 00:00:00 2001 From: Brian E Clow Date: Thu, 27 May 2021 15:46:42 -0700 Subject: [PATCH 12/21] Documentation update? --- docs/records.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/records.md b/docs/records.md index c6b2a77..1457546 100644 --- a/docs/records.md +++ b/docs/records.md @@ -19,6 +19,7 @@ OctoDNS supports the following record types: * `SRV` * `SSHFP` * `TXT` +* `URLFWD` Underlying provider support for each of these varies and some providers have extra requirements or limitations. In cases where a record type is not supported by a provider OctoDNS will ignore it there and continue to manage the record elsewhere. For example `SSHFP` is supported by Dyn, but not Route53. If your source data includes an SSHFP record OctoDNS will keep it in sync on Dyn, but not consider it when evaluating the state of Route53. The best way to find out what types are supported by a provider is to look for its `supports` method. If that method exists the logic will drive which records are supported and which are ignored. If the provider does not implement the method it will fall back to `BaseProvider.supports` which indicates full support. From 07aad177b5ddd736867b6d4ff1362f9bbcebd3c5 Mon Sep 17 00:00:00 2001 From: Brian E Clow Date: Fri, 28 May 2021 13:03:35 -0700 Subject: [PATCH 13/21] Update docs/records.md Co-authored-by: Ross McFarland --- docs/records.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/records.md b/docs/records.md index 1457546..f210846 100644 --- a/docs/records.md +++ b/docs/records.md @@ -19,7 +19,7 @@ OctoDNS supports the following record types: * `SRV` * `SSHFP` * `TXT` -* `URLFWD` +* `URLFWD` Underlying provider support for each of these varies and some providers have extra requirements or limitations. In cases where a record type is not supported by a provider OctoDNS will ignore it there and continue to manage the record elsewhere. For example `SSHFP` is supported by Dyn, but not Route53. If your source data includes an SSHFP record OctoDNS will keep it in sync on Dyn, but not consider it when evaluating the state of Route53. The best way to find out what types are supported by a provider is to look for its `supports` method. If that method exists the logic will drive which records are supported and which are ignored. If the provider does not implement the method it will fall back to `BaseProvider.supports` which indicates full support. From 096785855415636c0502a037d82c1cdf230f7048 Mon Sep 17 00:00:00 2001 From: Brian E Clow Date: Wed, 23 Jun 2021 13:38:58 -0700 Subject: [PATCH 14/21] Accounting for CloudFlare TTL alias --- octodns/provider/cloudflare.py | 21 ++++++++++++--------- tests/test_octodns_provider_cloudflare.py | 8 ++++++++ 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/octodns/provider/cloudflare.py b/octodns/provider/cloudflare.py index 4f9ba64..9c206c0 100644 --- a/octodns/provider/cloudflare.py +++ b/octodns/provider/cloudflare.py @@ -170,6 +170,9 @@ class CloudflareProvider(BaseProvider): return self._zones + def _ttl_data(self, ttl): + return 300 if ttl == 1 else ttl + def _data_for_cdn(self, name, _type, records): self.log.info('CDN rewrite for %s', records[0]['name']) _type = "CNAME" @@ -177,14 +180,14 @@ class CloudflareProvider(BaseProvider): _type = "ALIAS" return { - 'ttl': records[0]['ttl'], + 'ttl': self._ttl_data(records[0]['ttl']), 'type': _type, 'value': '{}.cdn.cloudflare.net.'.format(records[0]['name']), } def _data_for_multiple(self, _type, records): return { - 'ttl': records[0]['ttl'], + 'ttl': self._ttl_data(records[0]['ttl']), 'type': _type, 'values': [r['content'] for r in records], } @@ -195,7 +198,7 @@ class CloudflareProvider(BaseProvider): def _data_for_TXT(self, _type, records): return { - 'ttl': records[0]['ttl'], + 'ttl': self._ttl_data(records[0]['ttl']), 'type': _type, 'values': [r['content'].replace(';', '\\;') for r in records], } @@ -206,7 +209,7 @@ class CloudflareProvider(BaseProvider): data = r['data'] values.append(data) return { - 'ttl': records[0]['ttl'], + 'ttl': self._ttl_data(records[0]['ttl']), 'type': _type, 'values': values, } @@ -214,7 +217,7 @@ class CloudflareProvider(BaseProvider): def _data_for_CNAME(self, _type, records): only = records[0] return { - 'ttl': only['ttl'], + 'ttl': self._ttl_data(only['ttl']), 'type': _type, 'value': '{}.'.format(only['content']) } @@ -241,7 +244,7 @@ class CloudflareProvider(BaseProvider): 'precision_vert': float(r['precision_vert']), }) return { - 'ttl': records[0]['ttl'], + 'ttl': self._ttl_data(records[0]['ttl']), 'type': _type, 'values': values } @@ -254,14 +257,14 @@ class CloudflareProvider(BaseProvider): 'exchange': '{}.'.format(r['content']), }) return { - 'ttl': records[0]['ttl'], + 'ttl': self._ttl_data(records[0]['ttl']), 'type': _type, 'values': values, } def _data_for_NS(self, _type, records): return { - 'ttl': records[0]['ttl'], + 'ttl': self._ttl_data(records[0]['ttl']), 'type': _type, 'values': ['{}.'.format(r['content']) for r in records], } @@ -279,7 +282,7 @@ class CloudflareProvider(BaseProvider): }) return { 'type': _type, - 'ttl': records[0]['ttl'], + 'ttl': self._ttl_data(records[0]['ttl']), 'values': values } diff --git a/tests/test_octodns_provider_cloudflare.py b/tests/test_octodns_provider_cloudflare.py index 8843843..52c261e 100644 --- a/tests/test_octodns_provider_cloudflare.py +++ b/tests/test_octodns_provider_cloudflare.py @@ -1410,3 +1410,11 @@ class TestCloudflareProvider(TestCase): with self.assertRaises(CloudflareRateLimitError) as ctx: provider.zone_records(zone) self.assertEquals('last', text_type(ctx.exception)) + + def test_ttl_mapping(self): + provider = CloudflareProvider('test', 'email', 'token') + + self.assertEquals(120, provider._ttl_data(120)) + self.assertEquals(120, provider._ttl_data(120)) + self.assertEquals(3600, provider._ttl_data(3600)) + self.assertEquals(300, provider._ttl_data(1)) From a3b94cfed32d2ae02407958f02d32c809d9f87a8 Mon Sep 17 00:00:00 2001 From: Brian E Clow Date: Thu, 22 Jul 2021 14:33:06 -0700 Subject: [PATCH 15/21] 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): From 5eae6164b659e8e0cb4c3ae30b9735f3c6ed7744 Mon Sep 17 00:00:00 2001 From: Brian E Clow Date: Thu, 22 Jul 2021 14:34:10 -0700 Subject: [PATCH 16/21] Adding CloudFlare URLFWD/pagerules fixtures for testing updates --- tests/fixtures/cloudflare-pagerules.json | 103 +++++++++++++++++++++++ 1 file changed, 103 insertions(+) create mode 100644 tests/fixtures/cloudflare-pagerules.json diff --git a/tests/fixtures/cloudflare-pagerules.json b/tests/fixtures/cloudflare-pagerules.json new file mode 100644 index 0000000..7efa018 --- /dev/null +++ b/tests/fixtures/cloudflare-pagerules.json @@ -0,0 +1,103 @@ +{ + "result": [ + { + "id": "2b1ec1793185213139f22059a165376e", + "targets": [ + { + "target": "url", + "constraint": { + "operator": "matches", + "value": "urlfwd0.unit.tests/" + } + } + ], + "actions": [ + { + "id": "always_use_https" + } + ], + "priority": 4, + "status": "active", + "created_on": "2021-06-29T17:14:28.000000Z", + "modified_on": "2021-06-29T17:15:33.000000Z" + }, + { + "id": "2b1ec1793185213139f22059a165376f", + "targets": [ + { + "target": "url", + "constraint": { + "operator": "matches", + "value": "urlfwd0.unit.tests/*" + } + } + ], + "actions": [ + { + "id": "forwarding_url", + "value": { + "url": "https://www.unit.tests/", + "status_code": 301 + } + } + ], + "priority": 3, + "status": "active", + "created_on": "2021-06-29T17:07:12.000000Z", + "modified_on": "2021-06-29T17:15:12.000000Z" + }, + { + "id": "2b1ec1793185213139f22059a165377e", + "targets": [ + { + "target": "url", + "constraint": { + "operator": "matches", + "value": "urlfwd1.unit.tests/*" + } + } + ], + "actions": [ + { + "id": "forwarding_url", + "value": { + "url": "https://www.unit.tests/", + "status_code": 302 + } + } + ], + "priority": 2, + "status": "active", + "created_on": "2021-06-28T22:42:27.000000Z", + "modified_on": "2021-06-28T22:43:13.000000Z" + }, + { + "id": "2a9140b17ffb0e6aed826049eec970b8", + "targets": [ + { + "target": "url", + "constraint": { + "operator": "matches", + "value": "urlfwd2.unit.tests/*" + } + } + ], + "actions": [ + { + "id": "forwarding_url", + "value": { + "url": "https://www.unit.tests/", + "status_code": 301 + } + } + ], + "priority": 1, + "status": "active", + "created_on": "2021-06-25T20:10:50.000000Z", + "modified_on": "2021-06-28T22:38:10.000000Z" + } + ], + "success": true, + "errors": [], + "messages": [] +} From afc46f67eba9c840c2968bc95d1357dca8d06c2b Mon Sep 17 00:00:00 2001 From: Brian E Clow Date: Thu, 22 Jul 2021 15:23:09 -0700 Subject: [PATCH 17/21] When you filter is important --- octodns/provider/cloudflare.py | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/octodns/provider/cloudflare.py b/octodns/provider/cloudflare.py index 9462704..c94bffb 100644 --- a/octodns/provider/cloudflare.py +++ b/octodns/provider/cloudflare.py @@ -324,7 +324,10 @@ class CloudflareProvider(BaseProvider): path = '/zones/{}/pagerules'.format(zone_id) resp = self._try_request('GET', path, params={'status': 'active'}) - records += resp['result'] + for r in resp['result']: + # assumption, base on API guide, will only contain 1 action + if r['actions'][0]['id'] == 'forwarding_url': + records += [r] self._zone_records[zone.name] = records @@ -373,12 +376,11 @@ class CloudflareProvider(BaseProvider): _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) + _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: @@ -655,8 +657,7 @@ class CloudflareProvider(BaseProvider): 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': + # assumption, actions will always contain 1 action _values = record['actions'][0]['value'] _values['path'] = _path _values['ttl'] = 3600 From 5e2f7d43845d48fbf2e92b7bbaf2bb5b0e529b33 Mon Sep 17 00:00:00 2001 From: Brian E Clow Date: Thu, 22 Jul 2021 15:26:21 -0700 Subject: [PATCH 18/21] Removing the TODO note from record --- octodns/record/__init__.py | 1 - 1 file changed, 1 deletion(-) diff --git a/octodns/record/__init__.py b/octodns/record/__init__.py index 15cb143..5acffab 100644 --- a/octodns/record/__init__.py +++ b/octodns/record/__init__.py @@ -618,7 +618,6 @@ class _DynamicMixin(object): else: seen_default = False - # TODO: don't allow 'default' as a pool name, reserved for i, rule in enumerate(rules): rule_num = i + 1 try: From f4ccaaa7919c695aeebe2e26118fd144c6396e35 Mon Sep 17 00:00:00 2001 From: Brian E Clow Date: Fri, 23 Jul 2021 14:45:26 -0700 Subject: [PATCH 19/21] Apply suggestions from code review Thank you, Ross Co-authored-by: Ross McFarland --- octodns/provider/cloudflare.py | 37 +++++++++++++--------------------- 1 file changed, 14 insertions(+), 23 deletions(-) diff --git a/octodns/provider/cloudflare.py b/octodns/provider/cloudflare.py index c94bffb..c79b5ce 100644 --- a/octodns/provider/cloudflare.py +++ b/octodns/provider/cloudflare.py @@ -373,13 +373,13 @@ class CloudflareProvider(BaseProvider): 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 + path = parsed_uri.path _type = 'URLFWD' # assumption, actions will always contain 1 action - _values = record['actions'][0]['value'] - _values['path'] = _path + values = record['actions'][0]['value'] + values['path'] = path # no ttl set by pagerule, creating one - _values['ttl'] = 3600 + values['ttl'] = 3600 values[name][_type].append(_values) # the dns_records branch # elif 'name' in record: @@ -589,10 +589,7 @@ class CloudflareProvider(BaseProvider): # content as things are currently implemented so we need to handle # those explicitly and create unique/hashable strings for them. # AND... for URLFWD/Redirects additional adventures are created. - if 'targets' in data: - _type = 'URLFWD' - else: - _type = data['type'] + _type = data.get('type', 'URLFWD') if _type == 'MX': return '{priority} {content}'.format(**data) elif _type == 'CAA': @@ -621,15 +618,9 @@ class CloudflareProvider(BaseProvider): 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 '{name} {path} {url} {status_code}'.format(name=parsed_uri.netloc, + path=parsed_uri.path, + **data['actions'][0]['value']) return data['content'] def _apply_Create(self, change): @@ -656,13 +647,13 @@ class CloudflareProvider(BaseProvider): 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 + path = parsed_uri.path # assumption, actions will always contain 1 action - _values = record['actions'][0]['value'] - _values['path'] = _path - _values['ttl'] = 3600 - _values['type'] = 'URLFWD' - record.update(_values) + 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 From 8ca7070186c5609b4abde2c4d7ecb0fa6204c9ef Mon Sep 17 00:00:00 2001 From: Brian E Clow Date: Fri, 23 Jul 2021 15:03:51 -0700 Subject: [PATCH 20/21] Formatting, lingering pr comments, fixing resulting errors --- octodns/provider/cloudflare.py | 25 ++++++++++++----------- octodns/record/__init__.py | 2 -- tests/test_octodns_provider_cloudflare.py | 6 +++--- 3 files changed, 16 insertions(+), 17 deletions(-) diff --git a/octodns/provider/cloudflare.py b/octodns/provider/cloudflare.py index c79b5ce..01710b6 100644 --- a/octodns/provider/cloudflare.py +++ b/octodns/provider/cloudflare.py @@ -299,7 +299,7 @@ class CloudflareProvider(BaseProvider): }) return { 'type': _type, - 'ttl': 3600, # ttl does not exist for this type, forcing a setting + 'ttl': 300, # ttl does not exist for this type, forcing a setting 'values': values } @@ -376,10 +376,10 @@ class CloudflareProvider(BaseProvider): path = parsed_uri.path _type = 'URLFWD' # assumption, actions will always contain 1 action - values = record['actions'][0]['value'] - values['path'] = path + _values = record['actions'][0]['value'] + _values['path'] = path # no ttl set by pagerule, creating one - values['ttl'] = 3600 + _values['ttl'] = 300 values[name][_type].append(_values) # the dns_records branch # elif 'name' in record: @@ -618,9 +618,10 @@ class CloudflareProvider(BaseProvider): uri = data['targets'][0]['constraint']['value'] uri = '//' + uri if not uri.startswith('http') else uri parsed_uri = urlsplit(uri) - return '{name} {path} {url} {status_code}'.format(name=parsed_uri.netloc, - path=parsed_uri.path, - **data['actions'][0]['value']) + return '{name} {path} {url} {status_code}' \ + .format(name=parsed_uri.netloc, + path=parsed_uri.path, + **data['actions'][0]['value']) return data['content'] def _apply_Create(self, change): @@ -649,11 +650,11 @@ class CloudflareProvider(BaseProvider): name = zone.hostname_from_fqdn(parsed_uri.netloc) path = parsed_uri.path # assumption, actions will always contain 1 action - values = record['actions'][0]['value'] - values['path'] = path - values['ttl'] = 3600 - values['type'] = 'URLFWD' - record.update(values) + _values = record['actions'][0]['value'] + _values['path'] = path + _values['ttl'] = 300 + _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 diff --git a/octodns/record/__init__.py b/octodns/record/__init__.py index 5acffab..7714b27 100644 --- a/octodns/record/__init__.py +++ b/octodns/record/__init__.py @@ -1470,8 +1470,6 @@ class TxtRecord(_ChunkedValuesMixin, Record): class UrlfwdValue(EqualityTupleMixin): - # TODO: should have defaults for path, code, masking, and query - VALID_CODES = (301, 302) VALID_MASKS = (0, 1, 2) VALID_QUERY = (0, 1) diff --git a/tests/test_octodns_provider_cloudflare.py b/tests/test_octodns_provider_cloudflare.py index c2addf0..2cc11cb 100644 --- a/tests/test_octodns_provider_cloudflare.py +++ b/tests/test_octodns_provider_cloudflare.py @@ -411,7 +411,7 @@ class TestCloudflareProvider(TestCase): 'value': '3.2.3.4' })) wanted.add_record(Record.new(wanted, 'urlfwd', { - 'ttl': 3600, + 'ttl': 300, 'type': 'URLFWD', 'value': { 'path': '/*', # path change @@ -689,7 +689,7 @@ class TestCloudflareProvider(TestCase): 'values': ['ns1.foo.bar.', 'ns2.foo.bar.'], }) exstingurlfwd = Record.new(zone, 'urlfwd1', { - 'ttl': 3600, + 'ttl': 300, 'type': 'URLFWD', 'values': [ { @@ -715,7 +715,7 @@ class TestCloudflareProvider(TestCase): 'value': 'ns2.foo.bar.', }) newurlfwd = Record.new(zone, 'urlfwd1', { - 'ttl': 3600, + 'ttl': 300, 'type': 'URLFWD', 'value': { 'path': '/', From a9fe3b53983a99847a68f71df281085f99643e13 Mon Sep 17 00:00:00 2001 From: Brian E Clow Date: Fri, 23 Jul 2021 15:24:09 -0700 Subject: [PATCH 21/21] Adding URLFWD ttl exception handling for cloudflare --- octodns/provider/cloudflare.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/octodns/provider/cloudflare.py b/octodns/provider/cloudflare.py index 01710b6..ad057eb 100644 --- a/octodns/provider/cloudflare.py +++ b/octodns/provider/cloudflare.py @@ -419,6 +419,11 @@ class CloudflareProvider(BaseProvider): existing.update({ 'ttl': new['ttl'] }) + elif change.new._type == 'URLFWD': + existing = deepcopy(change.existing.data) + existing.update({ + 'ttl': new['ttl'] + }) else: existing = change.existing.data