From 9d45386f4d5fbd0041aab446fba87df777ea033b Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Tue, 27 Nov 2018 15:00:02 -0800 Subject: [PATCH 01/29] Sketch of new dynamic record config in unit.tests. --- tests/config/dynamic.tests.yaml | 66 +++++++++++++++++++++++++++++ tests/test_octodns_provider_yaml.py | 55 ++++++++++++++++++++---- 2 files changed, 112 insertions(+), 9 deletions(-) create mode 100644 tests/config/dynamic.tests.yaml diff --git a/tests/config/dynamic.tests.yaml b/tests/config/dynamic.tests.yaml new file mode 100644 index 0000000..dd844f3 --- /dev/null +++ b/tests/config/dynamic.tests.yaml @@ -0,0 +1,66 @@ +--- +a: + dynamic: + pools: + ams: + values: + - 1.1.1.1 + iad: + values: + - 2.2.2.2 + - 3.3.3.3 + lax: + value: 4.4.4.4 + sea: + value: 5.5.5.5 + rules: + - geo: EU-UK + pools: + - iad + - geo: EU + pools: + - ams + - iad + - geos: + - NA-US-CA + - NA-US-OR + - NA-US-WA + pools: + - sea + - iad + - default: + pool: iad + type: A + values: + - 2.2.2.2 + - 3.3.3.3 +cname: + dynamic: + pools: + ams: + value: target-ams.unit.tests. + iad: + value: target-iad.unit.tests. + lax: + value: target-lax.unit.tests. + sea: + value: target-sea.unit.tests. + rules: + - geo: EU-UK + pools: + - iad + - geo: EU + pools: + - ams + - iad + - geos: + - NA-US-CA + - NA-US-OR + - NA-US-WA + pools: + - sea + - iad + - default: + pool: iad + type: CNAME + value: target.unit.tests. diff --git a/tests/test_octodns_provider_yaml.py b/tests/test_octodns_provider_yaml.py index 8f1b4d3..835d53b 100644 --- a/tests/test_octodns_provider_yaml.py +++ b/tests/test_octodns_provider_yaml.py @@ -23,6 +23,7 @@ class TestYamlProvider(TestCase): source = YamlProvider('test', join(dirname(__file__), 'config')) zone = Zone('unit.tests.', []) + dynamic_zone = Zone('dynamic.tests.', []) # With target we don't add anything source.populate(zone, target=source) @@ -32,6 +33,9 @@ class TestYamlProvider(TestCase): source.populate(zone) self.assertEquals(18, len(zone.records)) + source.populate(dynamic_zone) + self.assertEquals(2, len(dynamic_zone.records)) + # Assumption here is that a clean round-trip means that everything # worked as expected, data that went in came back out and could be # pulled in yet again and still match up. That assumes that the input @@ -45,6 +49,7 @@ class TestYamlProvider(TestCase): # Add some subdirs to make sure that it can create them directory = join(td.dirname, 'sub', 'dir') yaml_file = join(directory, 'unit.tests.yaml') + dynamic_yaml_file = join(directory, 'dynamic.tests.yaml') target = YamlProvider('test', directory) # We add everything @@ -57,6 +62,15 @@ class TestYamlProvider(TestCase): self.assertEquals(15, target.apply(plan)) self.assertTrue(isfile(yaml_file)) + # Dynamic plan + plan = target.plan(dynamic_zone) + self.assertEquals(2, len(filter(lambda c: isinstance(c, Create), + plan.changes))) + self.assertFalse(isfile(dynamic_yaml_file)) + # Apply it + self.assertEquals(2, target.apply(plan)) + self.assertTrue(isfile(dynamic_yaml_file)) + # There should be no changes after the round trip reloaded = Zone('unit.tests.', []) target.populate(reloaded) @@ -77,21 +91,44 @@ class TestYamlProvider(TestCase): data = safe_load(fh.read()) # '' has some of both - roots = sorted(data[''], key=lambda r: r['type']) + roots = sorted(data.pop(''), key=lambda r: r['type']) self.assertTrue('values' in roots[0]) # A + self.assertTrue('geo' in roots[0]) # geo made the trip self.assertTrue('value' in roots[1]) # CAA self.assertTrue('values' in roots[2]) # SSHFP # these are stored as plural 'values' - self.assertTrue('values' in data['mx']) - self.assertTrue('values' in data['naptr']) - self.assertTrue('values' in data['_srv._tcp']) - self.assertTrue('values' in data['txt']) + self.assertTrue('values' in data.pop('_srv._tcp')) + self.assertTrue('values' in data.pop('mx')) + self.assertTrue('values' in data.pop('naptr')) + self.assertTrue('values' in data.pop('sub')) + self.assertTrue('values' in data.pop('txt')) # these are stored as singular 'value' - self.assertTrue('value' in data['aaaa']) - self.assertTrue('value' in data['ptr']) - self.assertTrue('value' in data['spf']) - self.assertTrue('value' in data['www']) + self.assertTrue('value' in data.pop('aaaa')) + self.assertTrue('value' in data.pop('cname')) + self.assertTrue('value' in data.pop('included')) + self.assertTrue('value' in data.pop('ptr')) + self.assertTrue('value' in data.pop('spf')) + self.assertTrue('value' in data.pop('www')) + self.assertTrue('value' in data.pop('www.sub')) + + # make sure nothing is left + self.assertEquals([], data.keys()) + + with open(dynamic_yaml_file) as fh: + data = safe_load(fh.read()) + + # make sure new dynamic records made the trip + dyna = data.pop('a') + self.assertTrue('values' in dyna) + #self.assertTrue('dynamic' in dyna) + + dyna = data.pop('cname') + self.assertTrue('value' in dyna) + #self.assertTrue('dynamic' in dyna) + + # make sure nothing is left + self.assertEquals([], data.keys()) def test_empty(self): source = YamlProvider('test', join(dirname(__file__), 'config')) From 698eb916844c5f801c72fbac4b3a2258bd2fff4a Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Wed, 28 Nov 2018 09:18:24 -0800 Subject: [PATCH 02/29] Add Dynamic Mixin class structure, all noops for now --- octodns/record.py | 76 +++++++++++++++++++++++++++-------------------- 1 file changed, 44 insertions(+), 32 deletions(-) diff --git a/octodns/record.py b/octodns/record.py index 85e6d97..be5f107 100644 --- a/octodns/record.py +++ b/octodns/record.py @@ -382,38 +382,6 @@ class _GeoMixin(_ValuesMixin): return super(_GeoMixin, self).__repr__() -class ARecord(_GeoMixin, Record): - _type = 'A' - - @classmethod - def _validate_value(self, value): - reasons = [] - try: - IPv4Address(unicode(value)) - except Exception: - reasons.append('invalid ip address "{}"'.format(value)) - return reasons - - def _process_values(self, values): - return values - - -class AaaaRecord(_GeoMixin, Record): - _type = 'AAAA' - - @classmethod - def _validate_value(self, value): - reasons = [] - try: - IPv6Address(unicode(value)) - except Exception: - reasons.append('invalid ip address "{}"'.format(value)) - return reasons - - def _process_values(self, values): - return values - - class _ValueMixin(object): @classmethod @@ -453,6 +421,50 @@ class _ValueMixin(object): self.fqdn, self.value) +class _DynamicBaseMixin(object): + pass + + +class _DynamicValuesMixin(_DynamicBaseMixin, _GeoMixin): + pass + + +class _DynamicValueMixin(_DynamicBaseMixin, _ValueMixin): + pass + + +class ARecord(_DynamicValuesMixin, Record): + _type = 'A' + + @classmethod + def _validate_value(self, value): + reasons = [] + try: + IPv4Address(unicode(value)) + except Exception: + reasons.append('invalid ip address "{}"'.format(value)) + return reasons + + def _process_values(self, values): + return values + + +class AaaaRecord(_GeoMixin, Record): + _type = 'AAAA' + + @classmethod + def _validate_value(self, value): + reasons = [] + try: + IPv6Address(unicode(value)) + except Exception: + reasons.append('invalid ip address "{}"'.format(value)) + return reasons + + def _process_values(self, values): + return values + + class AliasRecord(_ValueMixin, Record): _type = 'ALIAS' From 92b025fe1a09b3b0846e8dad205e48cd4d0496d3 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Wed, 28 Nov 2018 09:22:09 -0800 Subject: [PATCH 03/29] Support for weights of pools --- tests/config/dynamic.tests.yaml | 8 ++++---- tests/test_octodns_provider_yaml.py | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/config/dynamic.tests.yaml b/tests/config/dynamic.tests.yaml index dd844f3..d4c63e0 100644 --- a/tests/config/dynamic.tests.yaml +++ b/tests/config/dynamic.tests.yaml @@ -26,8 +26,8 @@ a: - NA-US-OR - NA-US-WA pools: - - sea - - iad + 25: iad + 75: sea - default: pool: iad type: A @@ -58,8 +58,8 @@ cname: - NA-US-OR - NA-US-WA pools: - - sea - - iad + 25: iad + 75: sea - default: pool: iad type: CNAME diff --git a/tests/test_octodns_provider_yaml.py b/tests/test_octodns_provider_yaml.py index 835d53b..02e85b2 100644 --- a/tests/test_octodns_provider_yaml.py +++ b/tests/test_octodns_provider_yaml.py @@ -121,11 +121,11 @@ class TestYamlProvider(TestCase): # make sure new dynamic records made the trip dyna = data.pop('a') self.assertTrue('values' in dyna) - #self.assertTrue('dynamic' in dyna) + # self.assertTrue('dynamic' in dyna) dyna = data.pop('cname') self.assertTrue('value' in dyna) - #self.assertTrue('dynamic' in dyna) + # self.assertTrue('dynamic' in dyna) # make sure nothing is left self.assertEquals([], data.keys()) From c41824c3e934303114678712e75a9f494bb16357 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Wed, 28 Nov 2018 09:41:25 -0800 Subject: [PATCH 04/29] Better weighting support --- tests/config/dynamic.tests.yaml | 20 +++++++++++++++++--- tests/test_octodns_provider_yaml.py | 10 +++++++--- 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/tests/config/dynamic.tests.yaml b/tests/config/dynamic.tests.yaml index d4c63e0..d4855c8 100644 --- a/tests/config/dynamic.tests.yaml +++ b/tests/config/dynamic.tests.yaml @@ -58,9 +58,23 @@ cname: - NA-US-OR - NA-US-WA pools: - 25: iad - 75: sea + 12: sea + 250: iad - default: - pool: iad + 1: sea + 4: iad type: CNAME value: target.unit.tests. +simple-weighted: + dynamic: + pools: + one: + one.unit.tests. + two: + two.unit.tests. + rules: + - default: + 100: one + 200: two + type: CNAME + value: default.unit.tests. diff --git a/tests/test_octodns_provider_yaml.py b/tests/test_octodns_provider_yaml.py index 02e85b2..a787a29 100644 --- a/tests/test_octodns_provider_yaml.py +++ b/tests/test_octodns_provider_yaml.py @@ -34,7 +34,7 @@ class TestYamlProvider(TestCase): self.assertEquals(18, len(zone.records)) source.populate(dynamic_zone) - self.assertEquals(2, len(dynamic_zone.records)) + self.assertEquals(3, len(dynamic_zone.records)) # Assumption here is that a clean round-trip means that everything # worked as expected, data that went in came back out and could be @@ -64,11 +64,11 @@ class TestYamlProvider(TestCase): # Dynamic plan plan = target.plan(dynamic_zone) - self.assertEquals(2, len(filter(lambda c: isinstance(c, Create), + self.assertEquals(3, len(filter(lambda c: isinstance(c, Create), plan.changes))) self.assertFalse(isfile(dynamic_yaml_file)) # Apply it - self.assertEquals(2, target.apply(plan)) + self.assertEquals(3, target.apply(plan)) self.assertTrue(isfile(dynamic_yaml_file)) # There should be no changes after the round trip @@ -127,6 +127,10 @@ class TestYamlProvider(TestCase): self.assertTrue('value' in dyna) # self.assertTrue('dynamic' in dyna) + dyna = data.pop('simple-weighted') + self.assertTrue('value' in dyna) + # self.assertTrue('dynamic' in dyna) + # make sure nothing is left self.assertEquals([], data.keys()) From 2829862ea5316a50afcaf40ea3b1fc55ec6853e2 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Thu, 29 Nov 2018 15:15:12 -0800 Subject: [PATCH 05/29] Major refactoring of record validation to better support (planned) complex/dynamic record types --- octodns/record.py | 420 ++++++++++++++++++-------------- tests/config/dynamic.tests.yaml | 26 +- tests/test_octodns_record.py | 117 +++++++-- 3 files changed, 344 insertions(+), 219 deletions(-) diff --git a/octodns/record.py b/octodns/record.py index be5f107..4b054db 100644 --- a/octodns/record.py +++ b/octodns/record.py @@ -253,6 +253,7 @@ class _ValuesMixin(object): @classmethod def validate(cls, name, data): reasons = super(_ValuesMixin, cls).validate(name, data) + values = [] try: values = data['values'] @@ -279,12 +280,11 @@ class _ValuesMixin(object): reasons.append('empty value') values = [] else: - values = [value] + values = value except KeyError: reasons.append('missing value(s)') - for value in values: - reasons.extend(cls._validate_value(value)) + reasons.extend(cls._value_type.validate(values)) return reasons @@ -339,8 +339,7 @@ class _GeoMixin(_ValuesMixin): # TODO: validate legal codes for code, values in geo.items(): reasons.extend(GeoValue._validate_geo(code)) - for value in values: - reasons.extend(cls._validate_value(value)) + reasons.extend(cls._value_type.validate(values)) except KeyError: pass return reasons @@ -353,6 +352,8 @@ class _GeoMixin(_ValuesMixin): super(_GeoMixin, self).__init__(zone, name, data, *args, **kwargs) try: self.geo = dict(data['geo']) + self.log.warn("'geo' support has been deprecated, " + "transition %s to use 'dynamic'", name) except KeyError: self.geo = {} for code, values in self.geo.items(): @@ -397,7 +398,7 @@ class _ValueMixin(object): except KeyError: reasons.append('missing value') if value: - reasons.extend(cls._validate_value(value)) + reasons.extend(cls._value_type.validate(value, cls)) return reasons def __init__(self, zone, name, data, source=None): @@ -421,59 +422,97 @@ class _ValueMixin(object): self.fqdn, self.value) -class _DynamicBaseMixin(object): - pass +class _DynamicMixin(object): + @classmethod + def validate(cls, name, data): + reasons = super(_DynamicMixin, cls).validate(name, data) + try: + pools = data['dynamic']['pools'] + except KeyError: + pools = {} + for pool in sorted(pools.values()): + reasons.extend(cls._value_type.validate(pool)) + return reasons -class _DynamicValuesMixin(_DynamicBaseMixin, _GeoMixin): - pass + def __init__(self, zone, name, data, *args, **kwargs): + super(_DynamicMixin, self).__init__(zone, name, data, *args, + **kwargs) + try: + self.dynamic = dict(data['dynamic']) + except: + self.dynamic = {} + # TODO: -class _DynamicValueMixin(_DynamicBaseMixin, _ValueMixin): - pass +class Ipv4List(object): + + @classmethod + def validate(cls, data): + if not isinstance(data, (list, tuple)): + data = (data,) + reasons = [] + for value in data: + try: + IPv4Address(unicode(value)) + except Exception: + reasons.append('invalid IPv4 address "{}"'.format(value)) + return reasons -class ARecord(_DynamicValuesMixin, Record): - _type = 'A' +class Ipv6List(object): @classmethod - def _validate_value(self, value): + def validate(cls, data): + if not isinstance(data, (list, tuple)): + data = (data,) reasons = [] - try: - IPv4Address(unicode(value)) - except Exception: - reasons.append('invalid ip address "{}"'.format(value)) + for value in data: + try: + IPv6Address(unicode(value)) + except Exception: + reasons.append('invalid IPv6 address "{}"'.format(value)) return reasons + +class _TargetValue(object): + + @classmethod + def validate(cls, data, record_cls): + reasons = [] + if not data.endswith('.'): + reasons.append('{} value "{}" missing trailing .' + .format(record_cls._type, data)) + return reasons + + +class CnameValue(_TargetValue): + pass + + +class ARecord(_DynamicMixin, _GeoMixin, Record): + _type = 'A' + _value_type = Ipv4List + def _process_values(self, values): return values class AaaaRecord(_GeoMixin, Record): _type = 'AAAA' - - @classmethod - def _validate_value(self, value): - reasons = [] - try: - IPv6Address(unicode(value)) - except Exception: - reasons.append('invalid ip address "{}"'.format(value)) - return reasons + _value_type = Ipv6List def _process_values(self, values): return values +class AliasValue(_TargetValue): + pass + + class AliasRecord(_ValueMixin, Record): _type = 'ALIAS' - - @classmethod - def _validate_value(self, value): - reasons = [] - if not value.endswith('.'): - reasons.append('missing trailing .') - return reasons + _value_type = AliasValue def _process_value(self, value): return value @@ -483,20 +522,22 @@ class CaaValue(object): # https://tools.ietf.org/html/rfc6844#page-5 @classmethod - def _validate_value(cls, value): + def validate(cls, data): + if not isinstance(data, (list, tuple)): + data = (data,) reasons = [] - try: - flags = int(value.get('flags', 0)) - if flags < 0 or flags > 255: - reasons.append('invalid flags "{}"'.format(flags)) - except ValueError: - reasons.append('invalid flags "{}"'.format(value['flags'])) - - if 'tag' not in value: - reasons.append('missing tag') - if 'value' not in value: - reasons.append('missing value') - + for value in data: + try: + flags = int(value.get('flags', 0)) + if flags < 0 or flags > 255: + reasons.append('invalid flags "{}"'.format(flags)) + except ValueError: + reasons.append('invalid flags "{}"'.format(value['flags'])) + + if 'tag' not in value: + reasons.append('missing tag') + if 'value' not in value: + reasons.append('missing value') return reasons def __init__(self, value): @@ -525,10 +566,7 @@ class CaaValue(object): class CaaRecord(_ValuesMixin, Record): _type = 'CAA' - - @classmethod - def _validate_value(cls, value): - return CaaValue._validate_value(value) + _value_type = CaaValue def _process_values(self, values): return [CaaValue(v) for v in values] @@ -536,6 +574,7 @@ class CaaRecord(_ValuesMixin, Record): class CnameRecord(_ValueMixin, Record): _type = 'CNAME' + _value_type = CnameValue @classmethod def validate(cls, name, data): @@ -545,13 +584,6 @@ class CnameRecord(_ValueMixin, Record): reasons.extend(super(CnameRecord, cls).validate(name, data)) return reasons - @classmethod - def _validate_value(cls, value): - reasons = [] - if not value.endswith('.'): - reasons.append('missing trailing .') - return reasons - def _process_value(self, value): return value @@ -559,25 +591,29 @@ class CnameRecord(_ValueMixin, Record): class MxValue(object): @classmethod - def _validate_value(cls, value): + def validate(cls, data): + if not isinstance(data, (list, tuple)): + data = (data,) reasons = [] - try: + for value in data: try: - int(value['preference']) + try: + int(value['preference']) + except KeyError: + int(value['priority']) except KeyError: - int(value['priority']) - except KeyError: - reasons.append('missing preference') - except ValueError: - reasons.append('invalid preference "{}"' - .format(value['preference'])) - exchange = None - try: - exchange = value.get('exchange', None) or value['value'] - if not exchange.endswith('.'): - reasons.append('missing trailing .') - except KeyError: - reasons.append('missing exchange') + reasons.append('missing preference') + except ValueError: + reasons.append('invalid preference "{}"' + .format(value['preference'])) + exchange = None + try: + exchange = value.get('exchange', None) or value['value'] + if not exchange.endswith('.'): + reasons.append('MX value "{}" missing trailing .' + .format(exchange)) + except KeyError: + reasons.append('missing exchange') return reasons def __init__(self, value): @@ -612,10 +648,7 @@ class MxValue(object): class MxRecord(_ValuesMixin, Record): _type = 'MX' - - @classmethod - def _validate_value(cls, value): - return MxValue._validate_value(value) + _value_type = MxValue def _process_values(self, values): return [MxValue(v) for v in values] @@ -625,32 +658,36 @@ class NaptrValue(object): VALID_FLAGS = ('S', 'A', 'U', 'P') @classmethod - def _validate_value(cls, data): + def validate(cls, data): + if not isinstance(data, (list, tuple)): + data = (data,) reasons = [] - try: - int(data['order']) - except KeyError: - reasons.append('missing order') - except ValueError: - reasons.append('invalid order "{}"'.format(data['order'])) - try: - int(data['preference']) - except KeyError: - reasons.append('missing preference') - except ValueError: - reasons.append('invalid preference "{}"' - .format(data['preference'])) - try: - flags = data['flags'] - if flags not in cls.VALID_FLAGS: - reasons.append('unrecognized flags "{}"'.format(flags)) - except KeyError: - reasons.append('missing flags') + for value in data: + try: + int(value['order']) + except KeyError: + reasons.append('missing order') + except ValueError: + reasons.append('invalid order "{}"'.format(value['order'])) + try: + int(value['preference']) + except KeyError: + reasons.append('missing preference') + except ValueError: + reasons.append('invalid preference "{}"' + .format(value['preference'])) + try: + flags = value['flags'] + if flags not in cls.VALID_FLAGS: + reasons.append('unrecognized flags "{}"'.format(flags)) + except KeyError: + reasons.append('missing flags') + + # TODO: validate these... they're non-trivial + for k in ('service', 'regexp', 'replacement'): + if k not in value: + reasons.append('missing {}'.format(k)) - # TODO: validate these... they're non-trivial - for k in ('service', 'regexp', 'replacement'): - if k not in data: - reasons.append('missing {}'.format(k)) return reasons def __init__(self, value): @@ -696,38 +733,41 @@ class NaptrValue(object): class NaptrRecord(_ValuesMixin, Record): _type = 'NAPTR' - - @classmethod - def _validate_value(cls, value): - return NaptrValue._validate_value(value) + _value_type = NaptrValue def _process_values(self, values): return [NaptrValue(v) for v in values] -class NsRecord(_ValuesMixin, Record): - _type = 'NS' +class _NsValue(object): @classmethod - def _validate_value(cls, value): + def validate(cls, data): + if not isinstance(data, (list, tuple)): + data = (data,) reasons = [] - if not value.endswith('.'): - reasons.append('missing trailing .') + for value in data: + if not value.endswith('.'): + reasons.append('NS value "{}" missing trailing .' + .format(value)) return reasons + +class NsRecord(_ValuesMixin, Record): + _type = 'NS' + _value_type = _NsValue + def _process_values(self, values): return values +class PtrValue(_TargetValue): + pass + + class PtrRecord(_ValueMixin, Record): _type = 'PTR' - - @classmethod - def _validate_value(cls, value): - reasons = [] - if not value.endswith('.'): - reasons.append('missing trailing .') - return reasons + _value_type = PtrValue def _process_value(self, value): return value @@ -738,28 +778,33 @@ class SshfpValue(object): VALID_FINGERPRINT_TYPES = (1, 2) @classmethod - def _validate_value(cls, value): + def validate(cls, data): + if not isinstance(data, (list, tuple)): + data = (data,) reasons = [] - try: - algorithm = int(value['algorithm']) - if algorithm not in cls.VALID_ALGORITHMS: - reasons.append('unrecognized algorithm "{}"'.format(algorithm)) - except KeyError: - reasons.append('missing algorithm') - except ValueError: - reasons.append('invalid algorithm "{}"'.format(value['algorithm'])) - try: - fingerprint_type = int(value['fingerprint_type']) - if fingerprint_type not in cls.VALID_FINGERPRINT_TYPES: - reasons.append('unrecognized fingerprint_type "{}"' - .format(fingerprint_type)) - except KeyError: - reasons.append('missing fingerprint_type') - except ValueError: - reasons.append('invalid fingerprint_type "{}"' - .format(value['fingerprint_type'])) - if 'fingerprint' not in value: - reasons.append('missing fingerprint') + for value in data: + try: + algorithm = int(value['algorithm']) + if algorithm not in cls.VALID_ALGORITHMS: + reasons.append('unrecognized algorithm "{}"' + .format(algorithm)) + except KeyError: + reasons.append('missing algorithm') + except ValueError: + reasons.append('invalid algorithm "{}"' + .format(value['algorithm'])) + try: + fingerprint_type = int(value['fingerprint_type']) + if fingerprint_type not in cls.VALID_FINGERPRINT_TYPES: + reasons.append('unrecognized fingerprint_type "{}"' + .format(fingerprint_type)) + except KeyError: + reasons.append('missing fingerprint_type') + except ValueError: + reasons.append('invalid fingerprint_type "{}"' + .format(value['fingerprint_type'])) + if 'fingerprint' not in value: + reasons.append('missing fingerprint') return reasons def __init__(self, value): @@ -789,26 +834,15 @@ class SshfpValue(object): class SshfpRecord(_ValuesMixin, Record): _type = 'SSHFP' - - @classmethod - def _validate_value(cls, value): - return SshfpValue._validate_value(value) + _value_type = SshfpValue def _process_values(self, values): return [SshfpValue(v) for v in values] -_unescaped_semicolon_re = re.compile(r'\w;') - - class _ChunkedValuesMixin(_ValuesMixin): CHUNK_SIZE = 255 - - @classmethod - def _validate_value(cls, value): - if _unescaped_semicolon_re.search(value): - return ['unescaped ;'] - return [] + _unescaped_semicolon_re = re.compile(r'\w;') def _process_values(self, values): ret = [] @@ -830,39 +864,59 @@ class _ChunkedValuesMixin(_ValuesMixin): return values +class _ChunkedValue(object): + _unescaped_semicolon_re = re.compile(r'\w;') + + @classmethod + def validate(cls, data): + if not isinstance(data, (list, tuple)): + data = (data,) + reasons = [] + for value in data: + if cls._unescaped_semicolon_re.search(value): + reasons.append('unescaped ; in "{}"'.format(value)) + return reasons + + class SpfRecord(_ChunkedValuesMixin, Record): _type = 'SPF' + _value_type = _ChunkedValue class SrvValue(object): @classmethod - def _validate_value(self, value): + def validate(cls, data): + if not isinstance(data, (list, tuple)): + data = (data,) reasons = [] - # TODO: validate algorithm and fingerprint_type values - try: - int(value['priority']) - except KeyError: - reasons.append('missing priority') - except ValueError: - reasons.append('invalid priority "{}"'.format(value['priority'])) - try: - int(value['weight']) - except KeyError: - reasons.append('missing weight') - except ValueError: - reasons.append('invalid weight "{}"'.format(value['weight'])) - try: - int(value['port']) - except KeyError: - reasons.append('missing port') - except ValueError: - reasons.append('invalid port "{}"'.format(value['port'])) - try: - if not value['target'].endswith('.'): - reasons.append('missing trailing .') - except KeyError: - reasons.append('missing target') + for value in data: + # TODO: validate algorithm and fingerprint_type values + try: + int(value['priority']) + except KeyError: + reasons.append('missing priority') + except ValueError: + reasons.append('invalid priority "{}"' + .format(value['priority'])) + try: + int(value['weight']) + except KeyError: + reasons.append('missing weight') + except ValueError: + reasons.append('invalid weight "{}"'.format(value['weight'])) + try: + int(value['port']) + except KeyError: + reasons.append('missing port') + except ValueError: + reasons.append('invalid port "{}"'.format(value['port'])) + try: + if not value['target'].endswith('.'): + reasons.append('SRV value "{}" missing trailing .' + .format(value['target'])) + except KeyError: + reasons.append('missing target') return reasons def __init__(self, value): @@ -896,6 +950,7 @@ class SrvValue(object): class SrvRecord(_ValuesMixin, Record): _type = 'SRV' + _value_type = SrvValue _name_re = re.compile(r'^_[^\.]+\.[^\.]+') @classmethod @@ -906,13 +961,14 @@ class SrvRecord(_ValuesMixin, Record): reasons.extend(super(SrvRecord, cls).validate(name, data)) return reasons - @classmethod - def _validate_value(cls, value): - return SrvValue._validate_value(value) - def _process_values(self, values): return [SrvValue(v) for v in values] +class _TxtValue(_ChunkedValue): + pass + + class TxtRecord(_ChunkedValuesMixin, Record): _type = 'TXT' + _value_type = _TxtValue diff --git a/tests/config/dynamic.tests.yaml b/tests/config/dynamic.tests.yaml index d4855c8..07d3a54 100644 --- a/tests/config/dynamic.tests.yaml +++ b/tests/config/dynamic.tests.yaml @@ -2,17 +2,12 @@ a: dynamic: pools: - ams: - values: - - 1.1.1.1 + ams: 1.1.1.1 iad: - values: - 2.2.2.2 - 3.3.3.3 - lax: - value: 4.4.4.4 - sea: - value: 5.5.5.5 + lax: 4.4.4.4 + sea: 5.5.5.5 rules: - geo: EU-UK pools: @@ -28,8 +23,7 @@ a: pools: 25: iad 75: sea - - default: - pool: iad + - pool: iad type: A values: - 2.2.2.2 @@ -60,7 +54,7 @@ cname: pools: 12: sea 250: iad - - default: + - pools: 1: sea 4: iad type: CNAME @@ -69,12 +63,12 @@ simple-weighted: dynamic: pools: one: - one.unit.tests. + value: one.unit.tests. two: - two.unit.tests. + value: two.unit.tests. rules: - - default: - 100: one - 200: two + - pools: + 100: one + 200: two type: CNAME value: default.unit.tests. diff --git a/tests/test_octodns_record.py b/tests/test_octodns_record.py index 3facd32..d761573 100644 --- a/tests/test_octodns_record.py +++ b/tests/test_octodns_record.py @@ -941,7 +941,7 @@ class TestRecordValidation(TestCase): 'ttl': 600, 'value': 'hello' }) - self.assertEquals(['invalid ip address "hello"'], + self.assertEquals(['invalid IPv4 address "hello"'], ctx.exception.reasons) # invalid ip addresses @@ -952,8 +952,8 @@ class TestRecordValidation(TestCase): 'values': ['hello', 'goodbye'] }) self.assertEquals([ - 'invalid ip address "hello"', - 'invalid ip address "goodbye"' + 'invalid IPv4 address "hello"', + 'invalid IPv4 address "goodbye"' ], ctx.exception.reasons) # invalid & valid ip addresses, no ttl @@ -964,7 +964,7 @@ class TestRecordValidation(TestCase): }) self.assertEquals([ 'missing ttl', - 'invalid ip address "hello"', + 'invalid IPv4 address "hello"', ], ctx.exception.reasons) def test_geo(self): @@ -989,7 +989,7 @@ class TestRecordValidation(TestCase): 'ttl': 600, 'value': '1.2.3.4', }) - self.assertEquals(['invalid ip address "hello"'], + self.assertEquals(['invalid IPv4 address "hello"'], ctx.exception.reasons) # invalid geo code @@ -1016,8 +1016,8 @@ class TestRecordValidation(TestCase): 'value': '1.2.3.4', }) self.assertEquals([ - 'invalid ip address "hello"', - 'invalid ip address "goodbye"' + 'invalid IPv4 address "hello"', + 'invalid IPv4 address "goodbye"' ], ctx.exception.reasons) # invalid healthcheck protocol @@ -1062,16 +1062,21 @@ class TestRecordValidation(TestCase): 'ttl': 600, 'value': 'hello' }) - self.assertEquals(['invalid ip address "hello"'], + self.assertEquals(['invalid IPv6 address "hello"'], ctx.exception.reasons) with self.assertRaises(ValidationError) as ctx: Record.new(self.zone, '', { 'type': 'AAAA', 'ttl': 600, - 'value': '1.2.3.4' + 'values': [ + '1.2.3.4', + '2.3.4.5', + ], }) - self.assertEquals(['invalid ip address "1.2.3.4"'], - ctx.exception.reasons) + self.assertEquals([ + 'invalid IPv6 address "1.2.3.4"', + 'invalid IPv6 address "2.3.4.5"', + ], ctx.exception.reasons) # invalid ip addresses with self.assertRaises(ValidationError) as ctx: @@ -1081,8 +1086,8 @@ class TestRecordValidation(TestCase): 'values': ['hello', 'goodbye'] }) self.assertEquals([ - 'invalid ip address "hello"', - 'invalid ip address "goodbye"' + 'invalid IPv6 address "hello"', + 'invalid IPv6 address "goodbye"' ], ctx.exception.reasons) def test_ALIAS_and_value_mixin(self): @@ -1126,7 +1131,8 @@ class TestRecordValidation(TestCase): 'ttl': 600, 'value': 'foo.bar.com', }) - self.assertEquals(['missing trailing .'], ctx.exception.reasons) + self.assertEquals(['ALIAS value "foo.bar.com" missing trailing .'], + ctx.exception.reasons) def test_CAA(self): # doesn't blow up @@ -1221,7 +1227,8 @@ class TestRecordValidation(TestCase): 'ttl': 600, 'value': 'foo.bar.com', }) - self.assertEquals(['missing trailing .'], ctx.exception.reasons) + self.assertEquals(['CNAME value "foo.bar.com" missing trailing .'], + ctx.exception.reasons) def test_MX(self): # doesn't blow up @@ -1278,7 +1285,8 @@ class TestRecordValidation(TestCase): 'exchange': 'foo.bar.com' } }) - self.assertEquals(['missing trailing .'], ctx.exception.reasons) + self.assertEquals(['MX value "foo.bar.com" missing trailing .'], + ctx.exception.reasons) def test_NXPTR(self): # doesn't blow up @@ -1375,7 +1383,8 @@ class TestRecordValidation(TestCase): 'ttl': 600, 'value': 'foo.bar', }) - self.assertEquals(['missing trailing .'], ctx.exception.reasons) + self.assertEquals(['NS value "foo.bar" missing trailing .'], + ctx.exception.reasons) def test_PTR(self): # doesn't blow up (name & zone here don't make any sense, but not @@ -1401,7 +1410,8 @@ class TestRecordValidation(TestCase): 'ttl': 600, 'value': 'foo.bar', }) - self.assertEquals(['missing trailing .'], ctx.exception.reasons) + self.assertEquals(['PTR value "foo.bar" missing trailing .'], + ctx.exception.reasons) def test_SSHFP(self): # doesn't blow up @@ -1534,7 +1544,8 @@ class TestRecordValidation(TestCase): 'ttl': 600, 'value': 'this has some; semi-colons\\; in it', }) - self.assertEquals(['unescaped ;'], ctx.exception.reasons) + self.assertEquals(['unescaped ; in "this has some; ' + 'semi-colons\\; in it"'], ctx.exception.reasons) def test_SRV(self): # doesn't blow up @@ -1666,7 +1677,7 @@ class TestRecordValidation(TestCase): 'target': 'foo.bar.baz' } }) - self.assertEquals(['missing trailing .'], + self.assertEquals(['SRV value "foo.bar.baz" missing trailing .'], ctx.exception.reasons) def test_TXT(self): @@ -1696,7 +1707,8 @@ class TestRecordValidation(TestCase): 'ttl': 600, 'value': 'this has some; semi-colons\\; in it', }) - self.assertEquals(['unescaped ;'], ctx.exception.reasons) + self.assertEquals(['unescaped ; in "this has some; semi-colons\\; ' + 'in it"'], ctx.exception.reasons) def test_TXT_long_value_chunking(self): expected = '"Lorem ipsum dolor sit amet, consectetur adipiscing ' \ @@ -1757,3 +1769,66 @@ class TestRecordValidation(TestCase): self.assertEquals(single.values, chunked.values) # should be chunked values, with quoting self.assertEquals(single.chunked_values, chunked.chunked_values) + + +class TestDynamicRecords(TestCase): + zone = Zone('unit.tests.', []) + + def test_simple_a_weighted(self): + a_data = { + 'dynamic': { + 'pools': { + 'one': '3.3.3.3', + 'two': [ + '4.4.4.4', + '5.5.5.5', + ], + }, + 'rules': [{ + 'pools': { + 100: 'one', + 200: 'two', + } + }], + }, + 'ttl': 60, + 'values': [ + '1.1.1.1', + '2.2.2.2', + ], + } + a = ARecord(self.zone, 'weighted', a_data) + self.assertEquals('A', a._type) + self.assertEquals(a_data['ttl'], a.ttl) + self.assertEquals(a_data['values'], a.values) + self.assertEquals(a_data['dynamic'], a.dynamic) + + def test_a_validation(self): + a_data = { + 'dynamic': { + 'pools': { + 'one': 'this-aint-right', + 'two': [ + '4.4.4.4', + 'nor-is-this', + ], + }, + 'rules': [{ + 'pools': { + 100: '5.5.5.5', + 200: '6.6.6.6', + } + }], + }, + 'ttl': 60, + 'type': 'A', + 'values': [ + '1.1.1.1', + '2.2.2.2', + ], + } + with self.assertRaises(ValidationError) as ctx: + Record.new(self.zone, 'bad', a_data) + self.assertEquals(['invalid IPv4 address "nor-is-this"', + 'invalid IPv4 address "this-aint-right"'], + ctx.exception.reasons) From c9b373f0ae7aee4867a26f196203ea70153fd2ed Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Fri, 30 Nov 2018 08:32:55 -0800 Subject: [PATCH 06/29] Further clean up/division of labor and tests --- octodns/record.py | 70 +++++++++---------- tests/test_octodns_record.py | 128 ++++++++++++++++++++++++++++++++++- 2 files changed, 158 insertions(+), 40 deletions(-) diff --git a/octodns/record.py b/octodns/record.py index 4b054db..0d6462b 100644 --- a/octodns/record.py +++ b/octodns/record.py @@ -254,35 +254,13 @@ class _ValuesMixin(object): def validate(cls, name, data): reasons = super(_ValuesMixin, cls).validate(name, data) - values = [] try: values = data['values'] - if not values: - values = [] - reasons.append('missing value(s)') - else: - # loop through copy of values - # remove invalid value from values - for value in list(values): - if value is None: - reasons.append('missing value(s)') - values.remove(value) - elif len(value) == 0: - reasons.append('empty value') - values.remove(value) except KeyError: try: - value = data['value'] - if value is None: - reasons.append('missing value(s)') - values = [] - elif len(value) == 0: - reasons.append('empty value') - values = [] - else: - values = value + values = data['value'] except KeyError: - reasons.append('missing value(s)') + values = [] reasons.extend(cls._value_type.validate(values)) @@ -391,10 +369,10 @@ class _ValueMixin(object): value = None try: value = data['value'] - if value is None: - reasons.append('missing value') - elif value == '': + if value == '': reasons.append('empty value') + elif value is None: + reasons.append('missing value') except KeyError: reasons.append('missing value') if value: @@ -451,12 +429,19 @@ class Ipv4List(object): def validate(cls, data): if not isinstance(data, (list, tuple)): data = (data,) + if len(data) == 0: + return ['missing value(s)'] reasons = [] for value in data: - try: - IPv4Address(unicode(value)) - except Exception: - reasons.append('invalid IPv4 address "{}"'.format(value)) + if value is '': + reasons.append('empty value') + elif value is None: + reasons.append('missing value(s)') + else: + try: + IPv4Address(unicode(value)) + except Exception: + reasons.append('invalid IPv4 address "{}"'.format(value)) return reasons @@ -466,12 +451,19 @@ class Ipv6List(object): def validate(cls, data): if not isinstance(data, (list, tuple)): data = (data,) + if len(data) == 0: + return ['missing value(s)'] reasons = [] for value in data: - try: - IPv6Address(unicode(value)) - except Exception: - reasons.append('invalid IPv6 address "{}"'.format(value)) + if value is '': + reasons.append('empty value') + elif value is None: + reasons.append('missing value(s)') + else: + try: + IPv6Address(unicode(value)) + except Exception: + reasons.append('invalid IPv6 address "{}"'.format(value)) return reasons @@ -743,7 +735,9 @@ class _NsValue(object): @classmethod def validate(cls, data): - if not isinstance(data, (list, tuple)): + if not data: + return ['missing value(s)'] + elif not isinstance(data, (list, tuple)): data = (data,) reasons = [] for value in data: @@ -869,7 +863,9 @@ class _ChunkedValue(object): @classmethod def validate(cls, data): - if not isinstance(data, (list, tuple)): + if not data: + return ['missing value(s)'] + elif not isinstance(data, (list, tuple)): data = (data,) reasons = [] for value in data: diff --git a/tests/test_octodns_record.py b/tests/test_octodns_record.py index d761573..c52a9aa 100644 --- a/tests/test_octodns_record.py +++ b/tests/test_octodns_record.py @@ -934,7 +934,7 @@ class TestRecordValidation(TestCase): self.assertEquals(['missing ttl', 'missing value(s)'], ctx.exception.reasons) - # invalid ip address + # invalid ipv4 address with self.assertRaises(ValidationError) as ctx: Record.new(self.zone, '', { 'type': 'A', @@ -944,7 +944,7 @@ class TestRecordValidation(TestCase): self.assertEquals(['invalid IPv4 address "hello"'], ctx.exception.reasons) - # invalid ip addresses + # invalid ipv4 addresses with self.assertRaises(ValidationError) as ctx: Record.new(self.zone, '', { 'type': 'A', @@ -956,7 +956,7 @@ class TestRecordValidation(TestCase): 'invalid IPv4 address "goodbye"' ], ctx.exception.reasons) - # invalid & valid ip addresses, no ttl + # invalid & valid ipv4 addresses, no ttl with self.assertRaises(ValidationError) as ctx: Record.new(self.zone, '', { 'type': 'A', @@ -967,6 +967,128 @@ class TestRecordValidation(TestCase): 'invalid IPv4 address "hello"', ], ctx.exception.reasons) + def test_AAAA_validation(self): + # doesn't blow up + Record.new(self.zone, '', { + 'type': 'AAAA', + 'ttl': 600, + 'value': '2601:644:500:e210:62f8:1dff:feb8:947a', + }) + Record.new(self.zone, '', { + 'type': 'AAAA', + 'ttl': 600, + 'values': [ + '2601:644:500:e210:62f8:1dff:feb8:947a', + ] + }) + Record.new(self.zone, '', { + 'type': 'AAAA', + 'ttl': 600, + 'values': [ + '2601:644:500:e210:62f8:1dff:feb8:947a', + '2601:642:500:e210:62f8:1dff:feb8:947a', + ] + }) + + # missing value(s), no value or value + with self.assertRaises(ValidationError) as ctx: + Record.new(self.zone, '', { + 'type': 'AAAA', + 'ttl': 600, + }) + self.assertEquals(['missing value(s)'], ctx.exception.reasons) + + # missing value(s), empty values + with self.assertRaises(ValidationError) as ctx: + Record.new(self.zone, 'www', { + 'type': 'AAAA', + 'ttl': 600, + 'values': [] + }) + self.assertEquals(['missing value(s)'], ctx.exception.reasons) + + # missing value(s), None values + with self.assertRaises(ValidationError) as ctx: + Record.new(self.zone, 'www', { + 'type': 'AAAA', + 'ttl': 600, + 'values': None + }) + self.assertEquals(['missing value(s)'], ctx.exception.reasons) + + # missing value(s) and empty value + with self.assertRaises(ValidationError) as ctx: + Record.new(self.zone, 'www', { + 'type': 'AAAA', + 'ttl': 600, + 'values': [None, ''] + }) + self.assertEquals(['missing value(s)', + 'empty value'], ctx.exception.reasons) + + # missing value(s), None value + with self.assertRaises(ValidationError) as ctx: + Record.new(self.zone, 'www', { + 'type': 'AAAA', + 'ttl': 600, + 'value': None + }) + self.assertEquals(['missing value(s)'], ctx.exception.reasons) + + # empty value, empty string value + with self.assertRaises(ValidationError) as ctx: + Record.new(self.zone, 'www', { + 'type': 'AAAA', + 'ttl': 600, + 'value': '' + }) + self.assertEquals(['empty value'], ctx.exception.reasons) + + # missing value(s) & ttl + with self.assertRaises(ValidationError) as ctx: + Record.new(self.zone, '', { + 'type': 'AAAA', + }) + self.assertEquals(['missing ttl', 'missing value(s)'], + ctx.exception.reasons) + + # invalid IPv6 address + with self.assertRaises(ValidationError) as ctx: + Record.new(self.zone, '', { + 'type': 'AAAA', + 'ttl': 600, + 'value': 'hello' + }) + self.assertEquals(['invalid IPv6 address "hello"'], + ctx.exception.reasons) + + # invalid IPv6 addresses + with self.assertRaises(ValidationError) as ctx: + Record.new(self.zone, '', { + 'type': 'AAAA', + 'ttl': 600, + 'values': ['hello', 'goodbye'] + }) + self.assertEquals([ + 'invalid IPv6 address "hello"', + 'invalid IPv6 address "goodbye"' + ], ctx.exception.reasons) + + # invalid & valid IPv6 addresses, no ttl + with self.assertRaises(ValidationError) as ctx: + Record.new(self.zone, '', { + 'type': 'AAAA', + 'values': [ + '2601:644:500:e210:62f8:1dff:feb8:947a', + 'hello', + '2601:642:500:e210:62f8:1dff:feb8:947a' + ] + }) + self.assertEquals([ + 'missing ttl', + 'invalid IPv6 address "hello"', + ], ctx.exception.reasons) + def test_geo(self): Record.new(self.zone, '', { 'geo': { From a29f188bc0cff0f72b39118fb5c929e5ebfd92e6 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Fri, 30 Nov 2018 08:35:56 -0800 Subject: [PATCH 07/29] Same for ValueMixin --- octodns/record.py | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/octodns/record.py b/octodns/record.py index 0d6462b..584427f 100644 --- a/octodns/record.py +++ b/octodns/record.py @@ -366,17 +366,7 @@ class _ValueMixin(object): @classmethod def validate(cls, name, data): reasons = super(_ValueMixin, cls).validate(name, data) - value = None - try: - value = data['value'] - if value == '': - reasons.append('empty value') - elif value is None: - reasons.append('missing value') - except KeyError: - reasons.append('missing value') - if value: - reasons.extend(cls._value_type.validate(value, cls)) + reasons.extend(cls._value_type.validate(data.get('value', None), cls)) return reasons def __init__(self, zone, name, data, source=None): @@ -472,7 +462,11 @@ class _TargetValue(object): @classmethod def validate(cls, data, record_cls): reasons = [] - if not data.endswith('.'): + if data == '': + reasons.append('empty value') + elif not data: + reasons.append('missing value') + elif not data.endswith('.'): reasons.append('{} value "{}" missing trailing .' .format(record_cls._type, data)) return reasons From e07165a2abe8ebc9ab15fc18fff59941594aa0f6 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Fri, 30 Nov 2018 09:14:23 -0800 Subject: [PATCH 08/29] Move _process_value to cls.process on Value types --- octodns/record.py | 90 +++++++++++++++++++++++++---------------------- 1 file changed, 47 insertions(+), 43 deletions(-) diff --git a/octodns/record.py b/octodns/record.py index 584427f..3fa4b05 100644 --- a/octodns/record.py +++ b/octodns/record.py @@ -272,7 +272,7 @@ class _ValuesMixin(object): values = data['values'] except KeyError: values = [data['value']] - self.values = sorted(self._process_values(values)) + self.values = sorted(self._value_type.process(values)) def changes(self, other, target): if self.values != other.values: @@ -371,7 +371,7 @@ class _ValueMixin(object): def __init__(self, zone, name, data, source=None): super(_ValueMixin, self).__init__(zone, name, data, source=source) - self.value = self._process_value(data['value']) + self.value = self._value_type.process(data['value']) def changes(self, other, target): if self.value != other.value: @@ -434,6 +434,10 @@ class Ipv4List(object): reasons.append('invalid IPv4 address "{}"'.format(value)) return reasons + @classmethod + def process(cls, values): + return values + class Ipv6List(object): @@ -456,6 +460,10 @@ class Ipv6List(object): reasons.append('invalid IPv6 address "{}"'.format(value)) return reasons + @classmethod + def process(cls, values): + return values + class _TargetValue(object): @@ -471,6 +479,10 @@ class _TargetValue(object): .format(record_cls._type, data)) return reasons + @classmethod + def process(self, value): + return value + class CnameValue(_TargetValue): pass @@ -480,17 +492,11 @@ class ARecord(_DynamicMixin, _GeoMixin, Record): _type = 'A' _value_type = Ipv4List - def _process_values(self, values): - return values - class AaaaRecord(_GeoMixin, Record): _type = 'AAAA' _value_type = Ipv6List - def _process_values(self, values): - return values - class AliasValue(_TargetValue): pass @@ -500,9 +506,6 @@ class AliasRecord(_ValueMixin, Record): _type = 'ALIAS' _value_type = AliasValue - def _process_value(self, value): - return value - class CaaValue(object): # https://tools.ietf.org/html/rfc6844#page-5 @@ -526,6 +529,10 @@ class CaaValue(object): reasons.append('missing value') return reasons + @classmethod + def process(cls, values): + return [CaaValue(v) for v in values] + def __init__(self, value): self.flags = int(value.get('flags', 0)) self.tag = value['tag'] @@ -554,9 +561,6 @@ class CaaRecord(_ValuesMixin, Record): _type = 'CAA' _value_type = CaaValue - def _process_values(self, values): - return [CaaValue(v) for v in values] - class CnameRecord(_ValueMixin, Record): _type = 'CNAME' @@ -570,9 +574,6 @@ class CnameRecord(_ValueMixin, Record): reasons.extend(super(CnameRecord, cls).validate(name, data)) return reasons - def _process_value(self, value): - return value - class MxValue(object): @@ -602,6 +603,10 @@ class MxValue(object): reasons.append('missing exchange') return reasons + @classmethod + def process(cls, values): + return [MxValue(v) for v in values] + def __init__(self, value): # RFC1035 says preference, half the providers use priority try: @@ -636,9 +641,6 @@ class MxRecord(_ValuesMixin, Record): _type = 'MX' _value_type = MxValue - def _process_values(self, values): - return [MxValue(v) for v in values] - class NaptrValue(object): VALID_FLAGS = ('S', 'A', 'U', 'P') @@ -676,6 +678,10 @@ class NaptrValue(object): return reasons + @classmethod + def process(cls, values): + return [NaptrValue(v) for v in values] + def __init__(self, value): self.order = int(value['order']) self.preference = int(value['preference']) @@ -721,9 +727,6 @@ class NaptrRecord(_ValuesMixin, Record): _type = 'NAPTR' _value_type = NaptrValue - def _process_values(self, values): - return [NaptrValue(v) for v in values] - class _NsValue(object): @@ -740,14 +743,15 @@ class _NsValue(object): .format(value)) return reasons + @classmethod + def process(cls, values): + return values + class NsRecord(_ValuesMixin, Record): _type = 'NS' _value_type = _NsValue - def _process_values(self, values): - return values - class PtrValue(_TargetValue): pass @@ -757,9 +761,6 @@ class PtrRecord(_ValueMixin, Record): _type = 'PTR' _value_type = PtrValue - def _process_value(self, value): - return value - class SshfpValue(object): VALID_ALGORITHMS = (1, 2, 3, 4) @@ -795,6 +796,10 @@ class SshfpValue(object): reasons.append('missing fingerprint') return reasons + @classmethod + def process(cls, values): + return [SshfpValue(v) for v in values] + def __init__(self, value): self.algorithm = int(value['algorithm']) self.fingerprint_type = int(value['fingerprint_type']) @@ -824,22 +829,11 @@ class SshfpRecord(_ValuesMixin, Record): _type = 'SSHFP' _value_type = SshfpValue - def _process_values(self, values): - return [SshfpValue(v) for v in values] - class _ChunkedValuesMixin(_ValuesMixin): CHUNK_SIZE = 255 _unescaped_semicolon_re = re.compile(r'\w;') - def _process_values(self, values): - ret = [] - for v in values: - if v and v[0] == '"': - v = v[1:-1] - ret.append(v.replace('" "', '')) - return ret - @property def chunked_values(self): values = [] @@ -867,6 +861,15 @@ class _ChunkedValue(object): reasons.append('unescaped ; in "{}"'.format(value)) return reasons + @classmethod + def process(cls, values): + ret = [] + for v in values: + if v and v[0] == '"': + v = v[1:-1] + ret.append(v.replace('" "', '')) + return ret + class SpfRecord(_ChunkedValuesMixin, Record): _type = 'SPF' @@ -909,6 +912,10 @@ class SrvValue(object): reasons.append('missing target') return reasons + @classmethod + def process(cls, values): + return [SrvValue(v) for v in values] + def __init__(self, value): self.priority = int(value['priority']) self.weight = int(value['weight']) @@ -951,9 +958,6 @@ class SrvRecord(_ValuesMixin, Record): reasons.extend(super(SrvRecord, cls).validate(name, data)) return reasons - def _process_values(self, values): - return [SrvValue(v) for v in values] - class _TxtValue(_ChunkedValue): pass From 70c35aac26606f4737df83a96b69b41cae8809e3 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 3 Dec 2018 14:24:56 -0800 Subject: [PATCH 09/29] WIP implementation of dyanmic pools & rules validation --- octodns/record.py | 47 ++++- tests/config/dynamic.tests.yaml | 15 +- tests/test_octodns_record.py | 305 +++++++++++++++++++++++++++++++- 3 files changed, 356 insertions(+), 11 deletions(-) diff --git a/octodns/record.py b/octodns/record.py index 3fa4b05..665ceba 100644 --- a/octodns/record.py +++ b/octodns/record.py @@ -395,12 +395,55 @@ class _DynamicMixin(object): @classmethod def validate(cls, name, data): reasons = super(_DynamicMixin, cls).validate(name, data) + + if 'dynamic' not in data: + return reasons + try: pools = data['dynamic']['pools'] except KeyError: pools = {} - for pool in sorted(pools.values()): - reasons.extend(cls._value_type.validate(pool)) + + if not pools: + reasons.append('missing pools') + else: + for pool in sorted(pools.values()): + reasons.extend(cls._value_type.validate(pool)) + + try: + rules = data['dynamic']['rules'] + except KeyError: + rules = [] + + if not isinstance(rules, (list, tuple)): + reasons.append('rules must be a list') + elif not rules: + reasons.append('missing rules') + else: + for rule_num, rule in enumerate(rules): + rule_num += 1 + try: + rule_pools = rule['pools'] + except KeyError: + rule_pools = {} + if not rule_pools: + reasons.append('rule {} missing pools'.format(rule_num)) + elif not isinstance(rule_pools, dict): + reasons.append('rule {} pools must be a dict' + .format(rule_num)) + else: + for weight, pool in rule_pools.items(): + try: + weight = int(weight) + if weight < 1 or weight > 255: + reasons.append('invalid pool weight "{}"' + .format(weight)) + except ValueError: + reasons.append('invalid pool weight "{}"' + .format(weight)) + if pool not in pools: + reasons.append('undefined pool "{}"'.format(pool)) + return reasons def __init__(self, zone, name, data, *args, **kwargs): diff --git a/tests/config/dynamic.tests.yaml b/tests/config/dynamic.tests.yaml index 07d3a54..18db9c9 100644 --- a/tests/config/dynamic.tests.yaml +++ b/tests/config/dynamic.tests.yaml @@ -11,11 +11,11 @@ a: rules: - geo: EU-UK pools: - - iad + 10: iad - geo: EU pools: - - ams - - iad + 10: ams + 10: iad - geos: - NA-US-CA - NA-US-OR @@ -23,7 +23,8 @@ a: pools: 25: iad 75: sea - - pool: iad + - pools: + 10: iad type: A values: - 2.2.2.2 @@ -42,11 +43,11 @@ cname: rules: - geo: EU-UK pools: - - iad + 10: iad - geo: EU pools: - - ams - - iad + 10: ams + 10: iad - geos: - NA-US-CA - NA-US-OR diff --git a/tests/test_octodns_record.py b/tests/test_octodns_record.py index c52a9aa..7a71d4c 100644 --- a/tests/test_octodns_record.py +++ b/tests/test_octodns_record.py @@ -1926,6 +1926,51 @@ class TestDynamicRecords(TestCase): self.assertEquals(a_data['dynamic'], a.dynamic) def test_a_validation(self): + # Missing pools + a_data = { + 'dynamic': { + 'rules': [{ + 'pools': { + 1: 'one', + } + }], + }, + 'ttl': 60, + 'type': 'A', + 'values': [ + '1.1.1.1', + '2.2.2.2', + ], + } + with self.assertRaises(ValidationError) as ctx: + Record.new(self.zone, 'bad', a_data) + self.assertEquals(['missing pools', 'undefined pool "one"'], + ctx.exception.reasons) + + # Empty pools + a_data = { + 'dynamic': { + 'pools': { + }, + 'rules': [{ + 'pools': { + 1: 'one', + } + }], + }, + 'ttl': 60, + 'type': 'A', + 'values': [ + '1.1.1.1', + '2.2.2.2', + ], + } + with self.assertRaises(ValidationError) as ctx: + Record.new(self.zone, 'bad', a_data) + self.assertEquals(['missing pools', 'undefined pool "one"'], + ctx.exception.reasons) + + # Invalid addresses a_data = { 'dynamic': { 'pools': { @@ -1937,8 +1982,8 @@ class TestDynamicRecords(TestCase): }, 'rules': [{ 'pools': { - 100: '5.5.5.5', - 200: '6.6.6.6', + 100: 'one', + 200: 'two', } }], }, @@ -1954,3 +1999,259 @@ class TestDynamicRecords(TestCase): self.assertEquals(['invalid IPv4 address "nor-is-this"', 'invalid IPv4 address "this-aint-right"'], ctx.exception.reasons) + + # missing value(s) + a_data = { + 'dynamic': { + 'pools': { + 'one': [], + 'two': [ + '3.3.3.3', + '4.4.4.4', + ], + }, + 'rules': [{ + 'pools': { + 100: 'one', + 200: 'two', + } + }], + }, + 'ttl': 60, + 'type': 'A', + 'values': [ + '1.1.1.1', + '2.2.2.2', + ], + } + with self.assertRaises(ValidationError) as ctx: + Record.new(self.zone, 'bad', a_data) + self.assertEquals(['missing value(s)'], ctx.exception.reasons) + + # Empty value + a_data = { + 'dynamic': { + 'pools': { + 'one': '', + 'two': [ + '3.3.3.3', + 'blip', + ], + }, + 'rules': [{ + 'pools': { + 100: 'one', + 200: 'two', + } + }], + }, + 'ttl': 60, + 'type': 'A', + 'values': [ + '1.1.1.1', + '2.2.2.2', + ], + } + with self.assertRaises(ValidationError) as ctx: + Record.new(self.zone, 'bad', a_data) + self.assertEquals(['invalid IPv4 address "blip"', 'empty value'], + ctx.exception.reasons) + + # multiple problems + a_data = { + 'dynamic': { + 'pools': { + 'one': '', + 'two': [ + '3.3.3.3', + 'blip', + ], + }, + 'rules': [{ + 'pools': { + 100: 'one', + 200: 'two', + } + }], + }, + 'ttl': 60, + 'type': 'A', + 'values': [ + '1.1.1.1', + '2.2.2.2', + ], + } + with self.assertRaises(ValidationError) as ctx: + Record.new(self.zone, 'bad', a_data) + self.assertEquals(['invalid IPv4 address "blip"', 'empty value'], + ctx.exception.reasons) + + # missing rules + a_data = { + 'dynamic': { + 'pools': { + 'one': '1.2.3.4', + }, + }, + 'ttl': 60, + 'type': 'A', + 'values': [ + '1.1.1.1', + '2.2.2.2', + ], + } + with self.assertRaises(ValidationError) as ctx: + Record.new(self.zone, 'bad', a_data) + self.assertEquals(['missing rules'], ctx.exception.reasons) + + # empty rules + a_data = { + 'dynamic': { + 'pools': { + 'one': '1.2.3.4', + }, + 'rules': [], + }, + 'ttl': 60, + 'type': 'A', + 'values': [ + '1.1.1.1', + '2.2.2.2', + ], + } + with self.assertRaises(ValidationError) as ctx: + Record.new(self.zone, 'bad', a_data) + self.assertEquals(['missing rules'], ctx.exception.reasons) + + # rules not a list/tuple + a_data = { + 'dynamic': { + 'pools': { + 'one': '1.2.3.4', + }, + 'rules': {}, + }, + 'ttl': 60, + 'type': 'A', + 'values': [ + '1.1.1.1', + '2.2.2.2', + ], + } + with self.assertRaises(ValidationError) as ctx: + Record.new(self.zone, 'bad', a_data) + self.assertEquals(['rules must be a list'], ctx.exception.reasons) + + # rule without pools + a_data = { + 'dynamic': { + 'pools': { + 'one': '1.2.3.4', + }, + 'rules': [{ + }], + }, + 'ttl': 60, + 'type': 'A', + 'values': [ + '1.1.1.1', + '2.2.2.2', + ], + } + with self.assertRaises(ValidationError) as ctx: + Record.new(self.zone, 'bad', a_data) + self.assertEquals(['rule 1 missing pools'], ctx.exception.reasons) + + # rule with non-dict pools + a_data = { + 'dynamic': { + 'pools': { + 'one': '1.2.3.4', + }, + 'rules': [{ + 'pools': ['one'], + }], + }, + 'ttl': 60, + 'type': 'A', + 'values': [ + '1.1.1.1', + '2.2.2.2', + ], + } + with self.assertRaises(ValidationError) as ctx: + Record.new(self.zone, 'bad', a_data) + self.assertEquals(["rule 1 pools must be a dict"], + ctx.exception.reasons) + + # rule references non-existant pool + a_data = { + 'dynamic': { + 'pools': { + 'one': '1.2.3.4', + }, + 'rules': [{ + 'pools': { + 10: 'non-existant' + } + }], + }, + 'ttl': 60, + 'type': 'A', + 'values': [ + '1.1.1.1', + '2.2.2.2', + ], + } + with self.assertRaises(ValidationError) as ctx: + Record.new(self.zone, 'bad', a_data) + self.assertEquals(["undefined pool \"non-existant\""], + ctx.exception.reasons) + + # invalid int weight + a_data = { + 'dynamic': { + 'pools': { + 'one': '1.2.3.4', + }, + 'rules': [{ + 'pools': { + 256: 'one' + } + }], + }, + 'ttl': 60, + 'type': 'A', + 'values': [ + '1.1.1.1', + '2.2.2.2', + ], + } + with self.assertRaises(ValidationError) as ctx: + Record.new(self.zone, 'bad', a_data) + self.assertEquals(['invalid pool weight "256"'], + ctx.exception.reasons) + + # invalid non-int weight + a_data = { + 'dynamic': { + 'pools': { + 'one': '1.2.3.4', + }, + 'rules': [{ + 'pools': { + 'foo': 'one' + } + }], + }, + 'ttl': 60, + 'type': 'A', + 'values': [ + '1.1.1.1', + '2.2.2.2', + ], + } + with self.assertRaises(ValidationError) as ctx: + Record.new(self.zone, 'bad', a_data) + self.assertEquals(['invalid pool weight "foo"'], + ctx.exception.reasons) From 303d0532c8fb7b03cff3eb668881dfa19119752b Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 3 Dec 2018 14:40:43 -0800 Subject: [PATCH 10/29] Implement SUPPORTS_DYNAMIC functionality, no providers --- octodns/manager.py | 7 +++++++ octodns/provider/azuredns.py | 1 + octodns/provider/cloudflare.py | 1 + octodns/provider/digitalocean.py | 1 + octodns/provider/dnsimple.py | 1 + octodns/provider/dnsmadeeasy.py | 1 + octodns/provider/dyn.py | 5 +++++ octodns/provider/etc_hosts.py | 1 + octodns/provider/googlecloud.py | 1 + octodns/provider/ns1.py | 1 + octodns/provider/ovh.py | 1 + octodns/provider/powerdns.py | 1 + octodns/provider/rackspace.py | 1 + octodns/provider/route53.py | 2 ++ octodns/provider/yaml.py | 1 + octodns/source/axfr.py | 1 + octodns/source/base.py | 3 +++ octodns/source/tinydns.py | 1 + tests/helpers.py | 20 ++++++++++++++++++++ tests/test_octodns_manager.py | 10 ++++++++-- tests/test_octodns_provider_base.py | 12 ++++++++++-- tests/test_octodns_zone.py | 1 + 22 files changed, 70 insertions(+), 4 deletions(-) diff --git a/octodns/manager.py b/octodns/manager.py index c3fecf4..eade87a 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -37,6 +37,13 @@ class _AggregateTarget(object): return False return True + @property + def SUPPORTS_DYNAMIC(self): + for target in self.targets: + if not target.SUPPORTS_DYNAMIC: + return False + return True + class MakeThreadFuture(object): diff --git a/octodns/provider/azuredns.py b/octodns/provider/azuredns.py index b3bff6c..fcfc591 100644 --- a/octodns/provider/azuredns.py +++ b/octodns/provider/azuredns.py @@ -239,6 +239,7 @@ class AzureProvider(BaseProvider): possible to also hard-code into the config file: eg, resource_group. ''' SUPPORTS_GEO = False + SUPPORTS_DYNAMIC = False SUPPORTS = set(('A', 'AAAA', 'CNAME', 'MX', 'NS', 'PTR', 'SRV', 'TXT')) def __init__(self, id, client_id, key, directory_id, sub_id, diff --git a/octodns/provider/cloudflare.py b/octodns/provider/cloudflare.py index e4e0a34..7418c29 100644 --- a/octodns/provider/cloudflare.py +++ b/octodns/provider/cloudflare.py @@ -59,6 +59,7 @@ class CloudflareProvider(BaseProvider): value: 1.2.3.4 ''' SUPPORTS_GEO = False + SUPPORTS_DYNAMIC = False SUPPORTS = set(('ALIAS', 'A', 'AAAA', 'CAA', 'CNAME', 'MX', 'NS', 'SRV', 'SPF', 'TXT')) diff --git a/octodns/provider/digitalocean.py b/octodns/provider/digitalocean.py index 84116a0..98a78ad 100644 --- a/octodns/provider/digitalocean.py +++ b/octodns/provider/digitalocean.py @@ -116,6 +116,7 @@ class DigitalOceanProvider(BaseProvider): token: foo ''' SUPPORTS_GEO = False + SUPPORTS_DYNAMIC = False SUPPORTS = set(('A', 'AAAA', 'CAA', 'CNAME', 'MX', 'NS', 'TXT', 'SRV')) def __init__(self, id, token, *args, **kwargs): diff --git a/octodns/provider/dnsimple.py b/octodns/provider/dnsimple.py index 18dae68..e3b0a20 100644 --- a/octodns/provider/dnsimple.py +++ b/octodns/provider/dnsimple.py @@ -91,6 +91,7 @@ class DnsimpleProvider(BaseProvider): account: 42 ''' SUPPORTS_GEO = False + SUPPORTS_DYNAMIC = False SUPPORTS = set(('A', 'AAAA', 'ALIAS', 'CAA', 'CNAME', 'MX', 'NAPTR', 'NS', 'PTR', 'SPF', 'SRV', 'SSHFP', 'TXT')) diff --git a/octodns/provider/dnsmadeeasy.py b/octodns/provider/dnsmadeeasy.py index f8cee1c..490359f 100644 --- a/octodns/provider/dnsmadeeasy.py +++ b/octodns/provider/dnsmadeeasy.py @@ -150,6 +150,7 @@ class DnsMadeEasyProvider(BaseProvider): sandbox: true ''' SUPPORTS_GEO = False + SUPPORTS_DYNAMIC = False SUPPORTS = set(('A', 'AAAA', 'CAA', 'CNAME', 'MX', 'NS', 'PTR', 'SPF', 'SRV', 'TXT')) diff --git a/octodns/provider/dyn.py b/octodns/provider/dyn.py index 0f487c1..f47f77b 100644 --- a/octodns/provider/dyn.py +++ b/octodns/provider/dyn.py @@ -259,6 +259,11 @@ class DynProvider(BaseProvider): def SUPPORTS_GEO(self): return self.traffic_directors_enabled + @property + def SUPPORTS_DYNAMIC(self): + # TODO: dynamic + return False + def _check_dyn_sess(self): # We don't have to worry about locking for the check since the # underlying pieces are pre-thread. We can check to see if this thread diff --git a/octodns/provider/etc_hosts.py b/octodns/provider/etc_hosts.py index db84ae4..b143fdf 100644 --- a/octodns/provider/etc_hosts.py +++ b/octodns/provider/etc_hosts.py @@ -25,6 +25,7 @@ class EtcHostsProvider(BaseProvider): directory: ./hosts ''' SUPPORTS_GEO = False + SUPPORTS_DYNAMIC = False SUPPORTS = set(('A', 'AAAA', 'ALIAS', 'CNAME')) def __init__(self, id, directory, *args, **kwargs): diff --git a/octodns/provider/googlecloud.py b/octodns/provider/googlecloud.py index 74c17a4..b9999d6 100644 --- a/octodns/provider/googlecloud.py +++ b/octodns/provider/googlecloud.py @@ -40,6 +40,7 @@ class GoogleCloudProvider(BaseProvider): SUPPORTS = set(('A', 'AAAA', 'CAA', 'CNAME', 'MX', 'NAPTR', 'NS', 'PTR', 'SPF', 'SRV', 'TXT')) SUPPORTS_GEO = False + SUPPORTS_DYNAMIC = False CHANGE_LOOP_WAIT = 5 diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py index 3ae6889..5fdf5b0 100644 --- a/octodns/provider/ns1.py +++ b/octodns/provider/ns1.py @@ -26,6 +26,7 @@ class Ns1Provider(BaseProvider): api_key: env/NS1_API_KEY ''' SUPPORTS_GEO = True + SUPPORTS_DYNAMIC = False SUPPORTS = set(('A', 'AAAA', 'ALIAS', 'CAA', 'CNAME', 'MX', 'NAPTR', 'NS', 'PTR', 'SPF', 'SRV', 'TXT')) diff --git a/octodns/provider/ovh.py b/octodns/provider/ovh.py index a74a3cd..d968da4 100644 --- a/octodns/provider/ovh.py +++ b/octodns/provider/ovh.py @@ -34,6 +34,7 @@ class OvhProvider(BaseProvider): """ SUPPORTS_GEO = False + SUPPORTS_DYNAMIC = False ZONE_NOT_FOUND_MESSAGE = 'This service does not exist' # This variable is also used in populate method to filter which OVH record diff --git a/octodns/provider/powerdns.py b/octodns/provider/powerdns.py index 30cd01e..8d75163 100644 --- a/octodns/provider/powerdns.py +++ b/octodns/provider/powerdns.py @@ -14,6 +14,7 @@ from .base import BaseProvider class PowerDnsBaseProvider(BaseProvider): SUPPORTS_GEO = False + SUPPORTS_DYNAMIC = False SUPPORTS = set(('A', 'AAAA', 'ALIAS', 'CAA', 'CNAME', 'MX', 'NAPTR', 'NS', 'PTR', 'SPF', 'SSHFP', 'SRV', 'TXT')) TIMEOUT = 5 diff --git a/octodns/provider/rackspace.py b/octodns/provider/rackspace.py index d2b85f3..5038929 100644 --- a/octodns/provider/rackspace.py +++ b/octodns/provider/rackspace.py @@ -38,6 +38,7 @@ def unescape_semicolon(s): class RackspaceProvider(BaseProvider): SUPPORTS_GEO = False + SUPPORTS_DYNAMIC = False SUPPORTS = set(('A', 'AAAA', 'ALIAS', 'CNAME', 'MX', 'NS', 'PTR', 'SPF', 'TXT')) TIMEOUT = 5 diff --git a/octodns/provider/route53.py b/octodns/provider/route53.py index 4b7fe66..ff83cdc 100644 --- a/octodns/provider/route53.py +++ b/octodns/provider/route53.py @@ -232,6 +232,8 @@ class Route53Provider(BaseProvider): In general the account used will need full permissions on Route53. ''' SUPPORTS_GEO = True + # TODO: dynamic + SUPPORTS_DYNAMIC = False SUPPORTS = set(('A', 'AAAA', 'CAA', 'CNAME', 'MX', 'NAPTR', 'NS', 'PTR', 'SPF', 'SRV', 'TXT')) diff --git a/octodns/provider/yaml.py b/octodns/provider/yaml.py index 7c9a915..a9631a0 100644 --- a/octodns/provider/yaml.py +++ b/octodns/provider/yaml.py @@ -31,6 +31,7 @@ class YamlProvider(BaseProvider): enforce_order: True ''' SUPPORTS_GEO = True + SUPPORTS_DYNAMIC = True SUPPORTS = set(('A', 'AAAA', 'ALIAS', 'CAA', 'CNAME', 'MX', 'NAPTR', 'NS', 'PTR', 'SSHFP', 'SPF', 'SRV', 'TXT')) diff --git a/octodns/source/axfr.py b/octodns/source/axfr.py index 715a36b..f35c4b3 100644 --- a/octodns/source/axfr.py +++ b/octodns/source/axfr.py @@ -24,6 +24,7 @@ from .base import BaseSource class AxfrBaseSource(BaseSource): SUPPORTS_GEO = False + SUPPORTS_DYNAMIC = False SUPPORTS = set(('A', 'AAAA', 'CNAME', 'MX', 'NS', 'PTR', 'SPF', 'SRV', 'TXT')) diff --git a/octodns/source/base.py b/octodns/source/base.py index ee33619..1faab7f 100644 --- a/octodns/source/base.py +++ b/octodns/source/base.py @@ -16,6 +16,9 @@ class BaseSource(object): if not hasattr(self, 'SUPPORTS_GEO'): raise NotImplementedError('Abstract base class, SUPPORTS_GEO ' 'property missing') + if not hasattr(self, 'SUPPORTS_DYNAMIC'): + raise NotImplementedError('Abstract base class, SUPPORTS_DYNAMIC ' + 'property missing') if not hasattr(self, 'SUPPORTS'): raise NotImplementedError('Abstract base class, SUPPORTS ' 'property missing') diff --git a/octodns/source/tinydns.py b/octodns/source/tinydns.py index 7b06527..679accb 100644 --- a/octodns/source/tinydns.py +++ b/octodns/source/tinydns.py @@ -19,6 +19,7 @@ from .base import BaseSource class TinyDnsBaseSource(BaseSource): SUPPORTS_GEO = False + SUPPORTS_DYNAMIC = False SUPPORTS = set(('A', 'CNAME', 'MX', 'NS')) split_re = re.compile(r':+') diff --git a/tests/helpers.py b/tests/helpers.py index 632f258..ff7f7cc 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -17,6 +17,7 @@ class SimpleSource(object): class SimpleProvider(object): SUPPORTS_GEO = False + SUPPORTS_DYNAMIC = False SUPPORTS = set(('A',)) id = 'test' @@ -35,6 +36,25 @@ class SimpleProvider(object): class GeoProvider(object): SUPPORTS_GEO = True + SUPPORTS_DYNAMIC = False + id = 'test' + + def __init__(self, id='test'): + pass + + def populate(self, zone, source=False, lenient=False): + pass + + def supports(self, record): + return True + + def __repr__(self): + return self.__class__.__name__ + + +class DynamicProvider(object): + SUPPORTS_GEO = False + SUPPORTS_DYNAMIC = True id = 'test' def __init__(self, id='test'): diff --git a/tests/test_octodns_manager.py b/tests/test_octodns_manager.py index ada54e5..0e14bab 100644 --- a/tests/test_octodns_manager.py +++ b/tests/test_octodns_manager.py @@ -14,8 +14,8 @@ from octodns.manager import _AggregateTarget, MainThreadExecutor, Manager from octodns.yaml import safe_load from octodns.zone import Zone -from helpers import GeoProvider, NoSshFpProvider, SimpleProvider, \ - TemporaryDirectory +from helpers import DynamicProvider, GeoProvider, NoSshFpProvider, \ + SimpleProvider, TemporaryDirectory config_dir = join(dirname(__file__), 'config') @@ -187,6 +187,7 @@ class TestManager(TestCase): def test_aggregate_target(self): simple = SimpleProvider() geo = GeoProvider() + dynamic = DynamicProvider() nosshfp = NoSshFpProvider() self.assertFalse(_AggregateTarget([simple, simple]).SUPPORTS_GEO) @@ -194,6 +195,11 @@ class TestManager(TestCase): self.assertFalse(_AggregateTarget([geo, simple]).SUPPORTS_GEO) self.assertTrue(_AggregateTarget([geo, geo]).SUPPORTS_GEO) + self.assertFalse(_AggregateTarget([simple, simple]).SUPPORTS_DYNAMIC) + self.assertFalse(_AggregateTarget([simple, dynamic]).SUPPORTS_DYNAMIC) + self.assertFalse(_AggregateTarget([dynamic, simple]).SUPPORTS_DYNAMIC) + self.assertTrue(_AggregateTarget([dynamic, dynamic]).SUPPORTS_DYNAMIC) + zone = Zone('unit.tests.', []) record = Record.new(zone, 'sshfp', { 'ttl': 60, diff --git a/tests/test_octodns_provider_base.py b/tests/test_octodns_provider_base.py index 22a0ee6..5a053ee 100644 --- a/tests/test_octodns_provider_base.py +++ b/tests/test_octodns_provider_base.py @@ -61,13 +61,21 @@ class TestBaseProvider(TestCase): class HasSupportsGeo(HasLog): SUPPORTS_GEO = False + with self.assertRaises(NotImplementedError) as ctx: + HasSupportsGeo('hassupportsgeo') + self.assertEquals('Abstract base class, SUPPORTS_DYNAMIC ' + 'property missing', ctx.exception.message) + + class HasSupportsDyanmic(HasSupportsGeo): + SUPPORTS_DYNAMIC = False + zone = Zone('unit.tests.', ['sub']) with self.assertRaises(NotImplementedError) as ctx: - HasSupportsGeo('hassupportsgeo').populate(zone) + HasSupportsDyanmic('hassupportsdynamic').populate(zone) self.assertEquals('Abstract base class, SUPPORTS property missing', ctx.exception.message) - class HasSupports(HasSupportsGeo): + class HasSupports(HasSupportsDyanmic): SUPPORTS = set(('A',)) with self.assertRaises(NotImplementedError) as ctx: HasSupports('hassupports').populate(zone) diff --git a/tests/test_octodns_zone.py b/tests/test_octodns_zone.py index b371590..2fff996 100644 --- a/tests/test_octodns_zone.py +++ b/tests/test_octodns_zone.py @@ -111,6 +111,7 @@ class TestZone(TestCase): class NoAaaaProvider(object): id = 'no-aaaa' SUPPORTS_GEO = False + SUPPORTS_DYNAMIC = False def supports(self, record): return record._type != 'AAAA' From 0e0e99543602f63d987f8cf692f1a7794d6fd678 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 3 Dec 2018 14:54:22 -0800 Subject: [PATCH 11/29] AaaaRecord and CnameRecord should implement _DynamicMixin, fix validate params --- octodns/record.py | 28 ++++++++++++++-------------- tests/config/dynamic.tests.yaml | 18 ++++++------------ 2 files changed, 20 insertions(+), 26 deletions(-) diff --git a/octodns/record.py b/octodns/record.py index 665ceba..2395cf0 100644 --- a/octodns/record.py +++ b/octodns/record.py @@ -262,7 +262,7 @@ class _ValuesMixin(object): except KeyError: values = [] - reasons.extend(cls._value_type.validate(values)) + reasons.extend(cls._value_type.validate(values, cls)) return reasons @@ -317,7 +317,7 @@ class _GeoMixin(_ValuesMixin): # TODO: validate legal codes for code, values in geo.items(): reasons.extend(GeoValue._validate_geo(code)) - reasons.extend(cls._value_type.validate(values)) + reasons.extend(cls._value_type.validate(values, cls)) except KeyError: pass return reasons @@ -408,7 +408,7 @@ class _DynamicMixin(object): reasons.append('missing pools') else: for pool in sorted(pools.values()): - reasons.extend(cls._value_type.validate(pool)) + reasons.extend(cls._value_type.validate(pool, cls)) try: rules = data['dynamic']['rules'] @@ -459,7 +459,7 @@ class _DynamicMixin(object): class Ipv4List(object): @classmethod - def validate(cls, data): + def validate(cls, data, record_cls): if not isinstance(data, (list, tuple)): data = (data,) if len(data) == 0: @@ -485,7 +485,7 @@ class Ipv4List(object): class Ipv6List(object): @classmethod - def validate(cls, data): + def validate(cls, data, record_cls): if not isinstance(data, (list, tuple)): data = (data,) if len(data) == 0: @@ -536,7 +536,7 @@ class ARecord(_DynamicMixin, _GeoMixin, Record): _value_type = Ipv4List -class AaaaRecord(_GeoMixin, Record): +class AaaaRecord(_DynamicMixin, _GeoMixin, Record): _type = 'AAAA' _value_type = Ipv6List @@ -554,7 +554,7 @@ class CaaValue(object): # https://tools.ietf.org/html/rfc6844#page-5 @classmethod - def validate(cls, data): + def validate(cls, data, record_cls): if not isinstance(data, (list, tuple)): data = (data,) reasons = [] @@ -605,7 +605,7 @@ class CaaRecord(_ValuesMixin, Record): _value_type = CaaValue -class CnameRecord(_ValueMixin, Record): +class CnameRecord(_DynamicMixin, _ValueMixin, Record): _type = 'CNAME' _value_type = CnameValue @@ -621,7 +621,7 @@ class CnameRecord(_ValueMixin, Record): class MxValue(object): @classmethod - def validate(cls, data): + def validate(cls, data, record_cls): if not isinstance(data, (list, tuple)): data = (data,) reasons = [] @@ -689,7 +689,7 @@ class NaptrValue(object): VALID_FLAGS = ('S', 'A', 'U', 'P') @classmethod - def validate(cls, data): + def validate(cls, data, record_cls): if not isinstance(data, (list, tuple)): data = (data,) reasons = [] @@ -774,7 +774,7 @@ class NaptrRecord(_ValuesMixin, Record): class _NsValue(object): @classmethod - def validate(cls, data): + def validate(cls, data, record_cls): if not data: return ['missing value(s)'] elif not isinstance(data, (list, tuple)): @@ -810,7 +810,7 @@ class SshfpValue(object): VALID_FINGERPRINT_TYPES = (1, 2) @classmethod - def validate(cls, data): + def validate(cls, data, record_cls): if not isinstance(data, (list, tuple)): data = (data,) reasons = [] @@ -893,7 +893,7 @@ class _ChunkedValue(object): _unescaped_semicolon_re = re.compile(r'\w;') @classmethod - def validate(cls, data): + def validate(cls, data, record_cls): if not data: return ['missing value(s)'] elif not isinstance(data, (list, tuple)): @@ -922,7 +922,7 @@ class SpfRecord(_ChunkedValuesMixin, Record): class SrvValue(object): @classmethod - def validate(cls, data): + def validate(cls, data, record_cls): if not isinstance(data, (list, tuple)): data = (data,) reasons = [] diff --git a/tests/config/dynamic.tests.yaml b/tests/config/dynamic.tests.yaml index 18db9c9..3b9632d 100644 --- a/tests/config/dynamic.tests.yaml +++ b/tests/config/dynamic.tests.yaml @@ -32,14 +32,10 @@ a: cname: dynamic: pools: - ams: - value: target-ams.unit.tests. - iad: - value: target-iad.unit.tests. - lax: - value: target-lax.unit.tests. - sea: - value: target-sea.unit.tests. + ams: target-ams.unit.tests. + iad: target-iad.unit.tests. + lax: target-lax.unit.tests. + sea: target-sea.unit.tests. rules: - geo: EU-UK pools: @@ -63,10 +59,8 @@ cname: simple-weighted: dynamic: pools: - one: - value: one.unit.tests. - two: - value: two.unit.tests. + one: one.unit.tests. + two: two.unit.tests. rules: - pools: 100: one From b650013ccbec90b81dd2de75190de2e1834b641a Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 3 Dec 2018 15:03:53 -0800 Subject: [PATCH 12/29] Add a AAAA dyanmic.tests example, comments --- octodns/record.py | 7 ++----- tests/config/dynamic.tests.yaml | 30 +++++++++++++++++++++++++++++ tests/test_octodns_provider_yaml.py | 12 +++++++++--- 3 files changed, 41 insertions(+), 8 deletions(-) diff --git a/octodns/record.py b/octodns/record.py index 2395cf0..c19f22f 100644 --- a/octodns/record.py +++ b/octodns/record.py @@ -322,10 +322,6 @@ class _GeoMixin(_ValuesMixin): pass return reasons - # TODO: support 'value' as well - # TODO: move away from "data" hash to strict params, it's kind of leaking - # the yaml implementation into here and then forcing it back out into - # non-yaml providers during input def __init__(self, zone, name, data, *args, **kwargs): super(_GeoMixin, self).__init__(zone, name, data, *args, **kwargs) try: @@ -453,7 +449,8 @@ class _DynamicMixin(object): self.dynamic = dict(data['dynamic']) except: self.dynamic = {} - # TODO: + + # TODO: implement all this class Ipv4List(object): diff --git a/tests/config/dynamic.tests.yaml b/tests/config/dynamic.tests.yaml index 3b9632d..b927300 100644 --- a/tests/config/dynamic.tests.yaml +++ b/tests/config/dynamic.tests.yaml @@ -29,6 +29,36 @@ a: values: - 2.2.2.2 - 3.3.3.3 +aaaa: + dynamic: + pools: + ams: 2601:642:500:e210:62f8:1dff:feb8:9471 + iad: + - 2601:642:500:e210:62f8:1dff:feb8:9472 + - 2601:642:500:e210:62f8:1dff:feb8:9473 + lax: 2601:642:500:e210:62f8:1dff:feb8:9474 + sea: 2601:642:500:e210:62f8:1dff:feb8:9475 + rules: + - geo: EU-UK + pools: + 10: iad + - geo: EU + pools: + 10: ams + 10: iad + - geos: + - NA-US-CA + - NA-US-OR + - NA-US-WA + pools: + 25: iad + 75: sea + - pools: + 10: iad + type: AAAA + values: + - 2601:642:500:e210:62f8:1dff:feb8:947a + - 2601:644:500:e210:62f8:1dff:feb8:947a cname: dynamic: pools: diff --git a/tests/test_octodns_provider_yaml.py b/tests/test_octodns_provider_yaml.py index a787a29..d9be9d1 100644 --- a/tests/test_octodns_provider_yaml.py +++ b/tests/test_octodns_provider_yaml.py @@ -34,7 +34,7 @@ class TestYamlProvider(TestCase): self.assertEquals(18, len(zone.records)) source.populate(dynamic_zone) - self.assertEquals(3, len(dynamic_zone.records)) + self.assertEquals(4, len(dynamic_zone.records)) # Assumption here is that a clean round-trip means that everything # worked as expected, data that went in came back out and could be @@ -64,11 +64,11 @@ class TestYamlProvider(TestCase): # Dynamic plan plan = target.plan(dynamic_zone) - self.assertEquals(3, len(filter(lambda c: isinstance(c, Create), + self.assertEquals(4, len(filter(lambda c: isinstance(c, Create), plan.changes))) self.assertFalse(isfile(dynamic_yaml_file)) # Apply it - self.assertEquals(3, target.apply(plan)) + self.assertEquals(4, target.apply(plan)) self.assertTrue(isfile(dynamic_yaml_file)) # There should be no changes after the round trip @@ -122,6 +122,12 @@ class TestYamlProvider(TestCase): dyna = data.pop('a') self.assertTrue('values' in dyna) # self.assertTrue('dynamic' in dyna) + # TODO: + + # make sure new dynamic records made the trip + dyna = data.pop('aaaa') + self.assertTrue('values' in dyna) + # self.assertTrue('dynamic' in dyna) dyna = data.pop('cname') self.assertTrue('value' in dyna) From 446f66f562b3752c9ffa807444cf3bcf48b44d99 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 3 Dec 2018 15:51:30 -0800 Subject: [PATCH 13/29] Implement _DynamicMixin's value objects, unit test them --- octodns/record.py | 97 +++++++++++++++++++++++- tests/test_octodns_record.py | 138 ++++++++++++++++++++++++++++++++++- 2 files changed, 230 insertions(+), 5 deletions(-) diff --git a/octodns/record.py b/octodns/record.py index c19f22f..caac7a0 100644 --- a/octodns/record.py +++ b/octodns/record.py @@ -386,6 +386,47 @@ class _ValueMixin(object): self.fqdn, self.value) +class _DynamicPool(object): + + def __init__(self, _id, data): + self._id = _id + # TODO: actually parse this + self.data = data + + def _data(self): + return self.data + + +class _DynamicRule(object): + + def __init__(self, i, data): + self.i = i + # TODO: actually parse this + self.data = data + + def _data(self): + return self.data + + +class _Dynamic(object): + + def __init__(self, pools, rules): + self.pools = pools + self.rules = rules + + def _data(self): + pools = {} + for _id, pool in self.pools.items(): + pools[_id] = pool._data() + rules = [] + for rule in self.rules: + rules.append(rule._data()) + return { + 'pools': pools, + 'rules': rules, + } + + class _DynamicMixin(object): @classmethod @@ -445,12 +486,62 @@ class _DynamicMixin(object): def __init__(self, zone, name, data, *args, **kwargs): super(_DynamicMixin, self).__init__(zone, name, data, *args, **kwargs) + + self.dynamic = {} + + if 'dynamic' not in data: + return + + # pools + try: + pools = dict(data['dynamic']['pools']) + except: + pools = {} + + for _id, pool in sorted(pools.items()): + pools[_id] = _DynamicPool(_id, pool) + + # rules try: - self.dynamic = dict(data['dynamic']) + rules = list(data['dynamic']['rules']) except: - self.dynamic = {} + rules = [] + + parsed = [] + for i, rule in enumerate(rules): + parsed.append(_DynamicRule(i, rule)) + + # dynamic + self.dynamic = _Dynamic(pools, parsed) + + def _data(self): + ret = super(_DynamicMixin, self)._data() + if self.dynamic: + ret['dynamic'] = self.dynamic._data() + return ret + + def changes(self, other, target): + if target.SUPPORTS_DYNAMIC: + if self.dynamic != other.dynamic: + return Update(self, other) + return super(_DynamicMixin, self).changes(other, target) - # TODO: implement all this + def __repr__(self): + # TODO: improve this whole thing, we need multi-line... + if self.dynamic: + # TODO: this hack can't going to cut it, as part of said + # improvements the value types should deal with serializing their + # value + try: + values = self.values + except AttributeError: + values = self.value + + return '<{} {} {}, {}, {}, {}>'.format(self.__class__.__name__, + self._type, self.ttl, + self.fqdn, values, + self.dynamic) + return super(_DynamicMixin, self).__repr__() class Ipv4List(object): diff --git a/tests/test_octodns_record.py b/tests/test_octodns_record.py index 7a71d4c..5c7b7f2 100644 --- a/tests/test_octodns_record.py +++ b/tests/test_octodns_record.py @@ -1923,9 +1923,98 @@ class TestDynamicRecords(TestCase): self.assertEquals('A', a._type) self.assertEquals(a_data['ttl'], a.ttl) self.assertEquals(a_data['values'], a.values) - self.assertEquals(a_data['dynamic'], a.dynamic) - def test_a_validation(self): + dynamic = a.dynamic + self.assertTrue(dynamic) + + pools = dynamic.pools + self.assertTrue(pools) + self.assertEquals(a_data['dynamic']['pools']['one'], pools['one'].data) + self.assertEquals(a_data['dynamic']['pools']['two'], pools['two'].data) + + rules = dynamic.rules + self.assertTrue(rules) + self.assertEquals(a_data['dynamic']['rules'][0], rules[0].data) + + def test_simple_aaaa_weighted(self): + aaaa_data = { + 'dynamic': { + 'pools': { + 'one': '2601:642:500:e210:62f8:1dff:feb8:9473', + 'two': [ + '2601:642:500:e210:62f8:1dff:feb8:9474', + '2601:642:500:e210:62f8:1dff:feb8:9475', + ], + }, + 'rules': [{ + 'pools': { + 100: 'one', + 200: 'two', + } + }], + }, + 'ttl': 60, + 'values': [ + '2601:642:500:e210:62f8:1dff:feb8:9471', + '2601:642:500:e210:62f8:1dff:feb8:9472', + ], + } + aaaa = AaaaRecord(self.zone, 'weighted', aaaa_data) + self.assertEquals('AAAA', aaaa._type) + self.assertEquals(aaaa_data['ttl'], aaaa.ttl) + self.assertEquals(aaaa_data['values'], aaaa.values) + + dynamic = aaaa.dynamic + self.assertTrue(dynamic) + + pools = dynamic.pools + self.assertTrue(pools) + self.assertEquals(aaaa_data['dynamic']['pools']['one'], + pools['one'].data) + self.assertEquals(aaaa_data['dynamic']['pools']['two'], + pools['two'].data) + + rules = dynamic.rules + self.assertTrue(rules) + self.assertEquals(aaaa_data['dynamic']['rules'][0], rules[0].data) + + def test_simple_cname_weighted(self): + cname_data = { + 'dynamic': { + 'pools': { + 'one': 'one.cname.target.', + 'two': 'two.cname.target.', + }, + 'rules': [{ + 'pools': { + 100: 'one', + 200: 'two', + } + }], + }, + 'ttl': 60, + 'value': 'cname.target.', + } + cname = CnameRecord(self.zone, 'weighted', cname_data) + self.assertEquals('CNAME', cname._type) + self.assertEquals(cname_data['ttl'], cname.ttl) + self.assertEquals(cname_data['value'], cname.value) + + dynamic = cname.dynamic + self.assertTrue(dynamic) + + pools = dynamic.pools + self.assertTrue(pools) + self.assertEquals(cname_data['dynamic']['pools']['one'], + pools['one'].data) + self.assertEquals(cname_data['dynamic']['pools']['two'], + pools['two'].data) + + rules = dynamic.rules + self.assertTrue(rules) + self.assertEquals(cname_data['dynamic']['rules'][0], rules[0].data) + + def test_dynamic_validation(self): # Missing pools a_data = { 'dynamic': { @@ -2255,3 +2344,48 @@ class TestDynamicRecords(TestCase): Record.new(self.zone, 'bad', a_data) self.assertEquals(['invalid pool weight "foo"'], ctx.exception.reasons) + + def test_dynamic_lenient(self): + # Missing pools + a_data = { + 'dynamic': { + 'rules': [{ + 'pools': { + 1: 'one', + } + }], + }, + 'ttl': 60, + 'type': 'A', + 'values': [ + '1.1.1.1', + '2.2.2.2', + ], + } + a = Record.new(self.zone, 'bad', a_data, lenient=True) + self.assertEquals({ + 'pools': {}, + 'rules': a_data['dynamic']['rules'], + }, a._data()['dynamic']) + + # Missing rule + a_data = { + 'dynamic': { + 'pools': { + 'one': '1.1.1.1', + }, + }, + 'ttl': 60, + 'type': 'A', + 'values': [ + '1.1.1.1', + '2.2.2.2', + ], + } + a = Record.new(self.zone, 'bad', a_data, lenient=True) + self.assertEquals({ + 'pools': { + 'one': '1.1.1.1', + }, + 'rules': [], + }, a._data()['dynamic']) From b80348c2c7c532c72fb8f132d660fa5853e08956 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 3 Dec 2018 16:46:18 -0800 Subject: [PATCH 14/29] Add __eq__, __ne__, and __repr__ to Dynamic objects and test --- octodns/record.py | 28 +++++++++ tests/test_octodns_record.py | 109 ++++++++++++++++++++++++++++++++++- 2 files changed, 136 insertions(+), 1 deletion(-) diff --git a/octodns/record.py b/octodns/record.py index caac7a0..5f81e35 100644 --- a/octodns/record.py +++ b/octodns/record.py @@ -396,6 +396,15 @@ class _DynamicPool(object): def _data(self): return self.data + def __eq__(self, other): + return self.data == other.data + + def __ne__(self, other): + return not self.__eq__(other) + + def __repr__(self): + return '{}'.format(self.data) + class _DynamicRule(object): @@ -407,6 +416,15 @@ class _DynamicRule(object): def _data(self): return self.data + def __eq__(self, other): + return self.data == other.data + + def __ne__(self, other): + return not self.__eq__(other) + + def __repr__(self): + return '{}'.format(self.data) + class _Dynamic(object): @@ -426,6 +444,16 @@ class _Dynamic(object): 'rules': rules, } + def __eq__(self, other): + ret = self.pools == other.pools and self.rules == other.rules + return ret + + def __ne__(self, other): + return not self.__eq__(other) + + def __repr__(self): + return '{}, {}'.format(self.pools, self.rules) + class _DynamicMixin(object): diff --git a/tests/test_octodns_record.py b/tests/test_octodns_record.py index 5c7b7f2..4591530 100644 --- a/tests/test_octodns_record.py +++ b/tests/test_octodns_record.py @@ -13,7 +13,7 @@ from octodns.record import ARecord, AaaaRecord, AliasRecord, CaaRecord, \ TxtRecord, Update, ValidationError from octodns.zone import Zone -from helpers import GeoProvider, SimpleProvider +from helpers import DynamicProvider, GeoProvider, SimpleProvider class TestRecord(TestCase): @@ -2389,3 +2389,110 @@ class TestDynamicRecords(TestCase): }, 'rules': [], }, a._data()['dynamic']) + + def test_dynamic_changes(self): + simple = SimpleProvider() + dynamic = DynamicProvider() + + a_data = { + 'dynamic': { + 'pools': { + 'one': '3.3.3.3', + 'two': [ + '4.4.4.4', + '5.5.5.5', + ], + }, + 'rules': [{ + 'pools': { + 100: 'one', + 200: 'two', + } + }], + }, + 'ttl': 60, + 'values': [ + '1.1.1.1', + '2.2.2.2', + ], + } + a = ARecord(self.zone, 'weighted', a_data) + dup = ARecord(self.zone, 'weighted', a_data) + + b_data = { + 'dynamic': { + 'pools': { + 'one': '3.3.3.5', + 'two': [ + '4.4.4.4', + '5.5.5.5', + ], + }, + 'rules': [{ + 'pools': { + 100: 'one', + 200: 'two', + } + }], + }, + 'ttl': 60, + 'values': [ + '1.1.1.1', + '2.2.2.2', + ], + } + b = ARecord(self.zone, 'weighted', b_data) + + c_data = { + 'dynamic': { + 'pools': { + 'one': '3.3.3.3', + 'two': [ + '4.4.4.4', + '5.5.5.5', + ], + }, + 'rules': [{ + 'pools': { + 100: 'one', + 300: 'two', + } + }], + }, + 'ttl': 60, + 'values': [ + '1.1.1.1', + '2.2.2.2', + ], + } + c = ARecord(self.zone, 'weighted', c_data) + + # a changes a (identical dup) is never true + self.assertFalse(a.changes(dup, simple)) + self.assertFalse(a.changes(dup, dynamic)) + + # a changes b is not true for simple + self.assertFalse(a.changes(b, simple)) + # but is true for dynamic + update = a.changes(b, dynamic) + self.assertEquals(a, update.existing) + self.assertEquals(b, update.new) + # transitive + self.assertFalse(b.changes(a, simple)) + update = b.changes(a, dynamic) + self.assertEquals(a, update.existing) + self.assertEquals(b, update.new) + + # same for a change c + self.assertFalse(a.changes(c, simple)) + self.assertTrue(a.changes(c, dynamic)) + self.assertFalse(c.changes(a, simple)) + self.assertTrue(c.changes(a, dynamic)) + + # smoke test some of the equiality bits + self.assertEquals(a.dynamic.pools, a.dynamic.pools) + self.assertEquals(a.dynamic.pools['one'], a.dynamic.pools['one']) + self.assertNotEquals(a.dynamic.pools['one'], a.dynamic.pools['two']) + self.assertEquals(a.dynamic.rules, a.dynamic.rules) + self.assertEquals(a.dynamic.rules[0], a.dynamic.rules[0]) + self.assertNotEquals(a.dynamic.rules[0], c.dynamic.rules[0]) From e16648ab1f036d0864f28b35bb7e022a439fa978 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Wed, 5 Dec 2018 16:28:41 -0800 Subject: [PATCH 15/29] Progress towards fully fleshed out pools & rules --- octodns/record.py | 70 ++-- tests/config/dynamic.tests.yaml | 192 +++++++--- tests/test_octodns_provider_yaml.py | 10 +- tests/test_octodns_record.py | 567 ++++++++++++++++++++++------ 4 files changed, 652 insertions(+), 187 deletions(-) diff --git a/octodns/record.py b/octodns/record.py index 5f81e35..6517d74 100644 --- a/octodns/record.py +++ b/octodns/record.py @@ -469,11 +469,43 @@ class _DynamicMixin(object): except KeyError: pools = {} - if not pools: + if not isinstance(pools, dict): + reasons.append('pools must be a dict') + elif not pools: reasons.append('missing pools') else: - for pool in sorted(pools.values()): - reasons.extend(cls._value_type.validate(pool, cls)) + for _id, pool in sorted(pools.items()): + if not isinstance(pool, dict): + reasons.append('pool "{}" must be a dict'.format(_id)) + continue + try: + values = pool['values'] + except KeyError: + reasons.append('pool "{}" is missing values'.format(_id)) + continue + + for value_num, value in enumerate(values): + value_num += 1 + try: + weight = value['weight'] + weight = int(weight) + if weight < 1 or weight > 255: + reasons.append('invalid weight "{}" in pool "{}" ' + 'value {}'.format(weight, _id, + value_num)) + except KeyError: + pass + except ValueError: + reasons.append('invalid weight "{}" in pool "{}" ' + 'value {}'.format(weight, _id, + value_num)) + + try: + value = value['value'] + reasons.extend(cls._value_type.validate(value, cls)) + except KeyError: + reasons.append('missing value in pool "{}" ' + 'value {}'.format(_id, value_num)) try: rules = data['dynamic']['rules'] @@ -488,26 +520,19 @@ class _DynamicMixin(object): for rule_num, rule in enumerate(rules): rule_num += 1 try: - rule_pools = rule['pools'] + pool = rule['pool'] except KeyError: - rule_pools = {} - if not rule_pools: - reasons.append('rule {} missing pools'.format(rule_num)) - elif not isinstance(rule_pools, dict): - reasons.append('rule {} pools must be a dict' - .format(rule_num)) - else: - for weight, pool in rule_pools.items(): - try: - weight = int(weight) - if weight < 1 or weight > 255: - reasons.append('invalid pool weight "{}"' - .format(weight)) - except ValueError: - reasons.append('invalid pool weight "{}"' - .format(weight)) - if pool not in pools: - reasons.append('undefined pool "{}"'.format(pool)) + reasons.append('rule {} missing pool'.format(rule_num)) + continue + + if not isinstance(pool, basestring): + reasons.append('rule {} invalid pool "{}"' + .format(rule_num, pool)) + elif pool not in pools: + reasons.append('rule {} undefined pool "{}"' + .format(rule_num, pool)) + + # TODO: validate GEOs if present return reasons @@ -575,6 +600,7 @@ class _DynamicMixin(object): class Ipv4List(object): @classmethod + # TODO: remove record_cls it's redudant (cls) def validate(cls, data, record_cls): if not isinstance(data, (list, tuple)): data = (data,) diff --git a/tests/config/dynamic.tests.yaml b/tests/config/dynamic.tests.yaml index b927300..ac83ad5 100644 --- a/tests/config/dynamic.tests.yaml +++ b/tests/config/dynamic.tests.yaml @@ -2,29 +2,35 @@ a: dynamic: pools: - ams: 1.1.1.1 + ams: + # TODO: make value possible + values: + - value: 1.1.1.1 iad: - - 2.2.2.2 - - 3.3.3.3 - lax: 4.4.4.4 - sea: 5.5.5.5 + values: + # TODO: make value optional + - value: 2.2.2.2 + - value: 3.3.3.3 + lax: + values: + - value: 4.4.4.4 + sea: + values: + - value: 6.6.6.6 + weight: 10 + - value: 5.5.5.5 + weight: 25 rules: - geo: EU-UK - pools: - 10: iad + pool: iad - geo: EU - pools: - 10: ams - 10: iad + pool: ams - geos: - NA-US-CA - NA-US-OR - NA-US-WA - pools: - 25: iad - 75: sea - - pools: - 10: iad + pool: sea + - pool: iad type: A values: - 2.2.2.2 @@ -32,29 +38,33 @@ a: aaaa: dynamic: pools: - ams: 2601:642:500:e210:62f8:1dff:feb8:9471 + ams: + values: + - value: 2601:642:500:e210:62f8:1dff:feb8:9471 iad: - - 2601:642:500:e210:62f8:1dff:feb8:9472 - - 2601:642:500:e210:62f8:1dff:feb8:9473 - lax: 2601:642:500:e210:62f8:1dff:feb8:9474 - sea: 2601:642:500:e210:62f8:1dff:feb8:9475 + values: + - value: 2601:642:500:e210:62f8:1dff:feb8:9472 + - value: 2601:642:500:e210:62f8:1dff:feb8:9473 + lax: + values: + - value: 2601:642:500:e210:62f8:1dff:feb8:9474 + sea: + values: + - value: 2601:642:500:e210:62f8:1dff:feb8:9475 + weight: 1 + - value: 2601:642:500:e210:62f8:1dff:feb8:9476 + weight: 2 rules: - geo: EU-UK - pools: - 10: iad + pool: iad - geo: EU - pools: - 10: ams - 10: iad + pool: ams - geos: - NA-US-CA - NA-US-OR - NA-US-WA - pools: - 25: iad - 75: sea - - pools: - 10: iad + pool: sea + - pool: iad type: AAAA values: - 2601:642:500:e210:62f8:1dff:feb8:947a @@ -62,38 +72,120 @@ aaaa: cname: dynamic: pools: - ams: target-ams.unit.tests. - iad: target-iad.unit.tests. - lax: target-lax.unit.tests. - sea: target-sea.unit.tests. + ams: + values: + - value: target-ams.unit.tests. + iad: + values: + - value: target-iad.unit.tests. + lax: + values: + - value: target-lax.unit.tests. + sea: + values: + - value: target-sea-1.unit.tests. + weight: 100 + - value: target-sea-2.unit.tests. + weight: 175 rules: - geo: EU-UK - pools: - 10: iad + pool: iad - geo: EU - pools: - 10: ams - 10: iad + pool: ams - geos: - NA-US-CA - NA-US-OR - NA-US-WA - pools: - 12: sea - 250: iad - - pools: - 1: sea - 4: iad + pool: sea + - pool: iad type: CNAME value: target.unit.tests. +real-ish-a: + dynamic: + pools: + ap-southeast-1: + values: + # ap-southeast-1a + - value: 1.4.1.1 + weight: 2 + - value: 1.4.1.2 + weight: 2 + # ap-southeast-1b + - value: 1.4.2.1 + - value: 1.4.2.2 + # ap-southeast-1c + - value: 1.4.3.1 + - value: 1.4.3.2 + eu-central-1: + values: + # eu-central-1a + - value: 1.3.1.1 + - value: 1.3.1.2 + # eu-central-1b + - value: 1.3.2.1 + - value: 1.3.2.2 + # eu-central-1c + - value: 1.3.3.1 + - value: 1.3.3.2 + us-east-1: + values: + # us-east-1a + - value: 1.1.1.1 + - value: 1.1.1.2 + # us-east-1b + - value: 1.1.2.1 + - value: 1.1.2.2 + # us-east-1c + - value: 1.1.3.1 + - value: 1.1.3.2 + us-west-2: + values: + # us-west-2a + - value: 1.2.1.1 + - value: 1.2.1.2 + # us-west-2b + - value: 1.2.2.1 + - value: 1.2.2.2 + # us-west-2c + - value: 1.2.3.1 + - value: 1.2.3.2 + rules: + - geos: + # TODO: require sorted + - NA-US-CA + - NA-US-OR + - NA-US-WA + pool: us-west-2 + - geos: + - AS-CN + pool: ap-southeast-1 + - geos: + - AF + - EU + pool: eu-central-1 + - pool: us-east-1 + type: A + values: + # Generally these should match the values of your "default" rule's pools as + # if everything fails healthchecks they'll fallback to this + - 1.1.1.1 + - 1.1.1.2 + - 1.1.2.1 + - 1.1.2.2 + - 1.1.3.1 + - 1.1.3.2 simple-weighted: dynamic: pools: - one: one.unit.tests. - two: two.unit.tests. + default: + values: + - value: one.unit.tests. + weight: 3 + - value: two.unit.tests. + weight: 2 rules: - - pools: - 100: one - 200: two + - pool: default type: CNAME + # CNAMEs don't support health checks (currently) so these will never be used + # on providers with dynamic support value: default.unit.tests. diff --git a/tests/test_octodns_provider_yaml.py b/tests/test_octodns_provider_yaml.py index d9be9d1..74261de 100644 --- a/tests/test_octodns_provider_yaml.py +++ b/tests/test_octodns_provider_yaml.py @@ -34,7 +34,7 @@ class TestYamlProvider(TestCase): self.assertEquals(18, len(zone.records)) source.populate(dynamic_zone) - self.assertEquals(4, len(dynamic_zone.records)) + self.assertEquals(5, len(dynamic_zone.records)) # Assumption here is that a clean round-trip means that everything # worked as expected, data that went in came back out and could be @@ -64,11 +64,11 @@ class TestYamlProvider(TestCase): # Dynamic plan plan = target.plan(dynamic_zone) - self.assertEquals(4, len(filter(lambda c: isinstance(c, Create), + self.assertEquals(5, len(filter(lambda c: isinstance(c, Create), plan.changes))) self.assertFalse(isfile(dynamic_yaml_file)) # Apply it - self.assertEquals(4, target.apply(plan)) + self.assertEquals(5, target.apply(plan)) self.assertTrue(isfile(dynamic_yaml_file)) # There should be no changes after the round trip @@ -133,6 +133,10 @@ class TestYamlProvider(TestCase): self.assertTrue('value' in dyna) # self.assertTrue('dynamic' in dyna) + dyna = data.pop('real-ish-a') + self.assertTrue('values' in dyna) + # self.assertTrue('dynamic' in dyna) + dyna = data.pop('simple-weighted') self.assertTrue('value' in dyna) # self.assertTrue('dynamic' in dyna) diff --git a/tests/test_octodns_record.py b/tests/test_octodns_record.py index 4591530..0a4e4df 100644 --- a/tests/test_octodns_record.py +++ b/tests/test_octodns_record.py @@ -1900,17 +1900,36 @@ class TestDynamicRecords(TestCase): a_data = { 'dynamic': { 'pools': { - 'one': '3.3.3.3', - 'two': [ - '4.4.4.4', - '5.5.5.5', - ], + 'one': { + 'values': [{ + 'value': '3.3.3.3', + }], + }, + 'two': { + 'values': [{ + 'value': '4.4.4.4', + }, { + 'value': '5.5.5.5', + }], + }, + 'three': { + 'values': [{ + 'weight': 10, + 'value': '4.4.4.4', + }, { + 'weight': 12, + 'value': '5.5.5.5', + }], + }, }, 'rules': [{ - 'pools': { - 100: 'one', - 200: 'two', - } + 'geos': ['AF', 'EU'], + 'pool': 'three', + }, { + 'geos': ['NA-US-CA'], + 'pool': 'two', + }, { + 'pool': 'one', }], }, 'ttl': 60, @@ -1931,6 +1950,8 @@ class TestDynamicRecords(TestCase): self.assertTrue(pools) self.assertEquals(a_data['dynamic']['pools']['one'], pools['one'].data) self.assertEquals(a_data['dynamic']['pools']['two'], pools['two'].data) + self.assertEquals(a_data['dynamic']['pools']['three'], + pools['three'].data) rules = dynamic.rules self.assertTrue(rules) @@ -1945,12 +1966,58 @@ class TestDynamicRecords(TestCase): '2601:642:500:e210:62f8:1dff:feb8:9474', '2601:642:500:e210:62f8:1dff:feb8:9475', ], + 'three': { + 1: '2601:642:500:e210:62f8:1dff:feb8:9476', + 2: '2601:642:500:e210:62f8:1dff:feb8:9477', + }, }, 'rules': [{ - 'pools': { - 100: 'one', - 200: 'two', - } + 'pools': [ + 'three', + 'two', + 'one', + ], + }], + }, + 'ttl': 60, + 'values': [ + '2601:642:500:e210:62f8:1dff:feb8:9471', + '2601:642:500:e210:62f8:1dff:feb8:9472', + ], + } + aaaa_data = { + 'dynamic': { + 'pools': { + 'one': { + 'values': [{ + 'value': '2601:642:500:e210:62f8:1dff:feb8:9473', + }], + }, + 'two': { + 'values': [{ + 'value': '2601:642:500:e210:62f8:1dff:feb8:9474', + }, { + 'value': '2601:642:500:e210:62f8:1dff:feb8:9475', + }], + }, + 'three': { + 'values': [{ + 'weight': 10, + 'value': '2601:642:500:e210:62f8:1dff:feb8:9476', + }, { + 'weight': 12, + 'value': '2601:642:500:e210:62f8:1dff:feb8:9477', + }], + }, + }, + 'rules': [{ + 'geos': ['AF', 'EU'], + 'pool': 'three', + }, { + 'geos': ['NA-US-CA'], + 'pool': 'two', + }, { + 'pool': 'one', }], }, 'ttl': 60, @@ -1973,6 +2040,8 @@ class TestDynamicRecords(TestCase): pools['one'].data) self.assertEquals(aaaa_data['dynamic']['pools']['two'], pools['two'].data) + self.assertEquals(aaaa_data['dynamic']['pools']['three'], + pools['three'].data) rules = dynamic.rules self.assertTrue(rules) @@ -1982,14 +2051,34 @@ class TestDynamicRecords(TestCase): cname_data = { 'dynamic': { 'pools': { - 'one': 'one.cname.target.', - 'two': 'two.cname.target.', + 'one': { + 'values': [{ + 'value': 'one.cname.target.', + }], + }, + 'two': { + 'values': [{ + 'value': 'two.cname.target.', + }], + }, + 'three': { + 'values': [{ + 'weight': 12, + 'value': 'three-1.cname.target.', + }, { + 'weight': 32, + 'value': 'three-2.cname.target.', + }] + }, }, 'rules': [{ - 'pools': { - 100: 'one', - 200: 'two', - } + 'geos': ['AF', 'EU'], + 'pool': 'three', + }, { + 'geos': ['NA-US-CA'], + 'pool': 'two', + }, { + 'pool': 'one', }], }, 'ttl': 60, @@ -2009,6 +2098,8 @@ class TestDynamicRecords(TestCase): pools['one'].data) self.assertEquals(cname_data['dynamic']['pools']['two'], pools['two'].data) + self.assertEquals(cname_data['dynamic']['pools']['three'], + pools['three'].data) rules = dynamic.rules self.assertTrue(rules) @@ -2019,9 +2110,7 @@ class TestDynamicRecords(TestCase): a_data = { 'dynamic': { 'rules': [{ - 'pools': { - 1: 'one', - } + 'pool': 'one', }], }, 'ttl': 60, @@ -2033,7 +2122,7 @@ class TestDynamicRecords(TestCase): } with self.assertRaises(ValidationError) as ctx: Record.new(self.zone, 'bad', a_data) - self.assertEquals(['missing pools', 'undefined pool "one"'], + self.assertEquals(['missing pools', 'rule 1 undefined pool "one"'], ctx.exception.reasons) # Empty pools @@ -2042,9 +2131,7 @@ class TestDynamicRecords(TestCase): 'pools': { }, 'rules': [{ - 'pools': { - 1: 'one', - } + 'pool': 'one', }], }, 'ttl': 60, @@ -2056,24 +2143,64 @@ class TestDynamicRecords(TestCase): } with self.assertRaises(ValidationError) as ctx: Record.new(self.zone, 'bad', a_data) - self.assertEquals(['missing pools', 'undefined pool "one"'], + self.assertEquals(['missing pools', 'rule 1 undefined pool "one"'], + ctx.exception.reasons) + + # pools not a dict + a_data = { + 'dynamic': { + 'pools': [], + 'rules': [{ + 'pool': 'one', + }], + }, + 'ttl': 60, + 'type': 'A', + 'values': [ + '1.1.1.1', + '2.2.2.2', + ], + } + with self.assertRaises(ValidationError) as ctx: + Record.new(self.zone, 'bad', a_data) + self.assertEquals(['pools must be a dict', + 'rule 1 undefined pool "one"'], ctx.exception.reasons) # Invalid addresses a_data = { 'dynamic': { 'pools': { - 'one': 'this-aint-right', - 'two': [ - '4.4.4.4', - 'nor-is-this', - ], + 'one': { + 'values': [{ + 'value': 'this-aint-right', + }], + }, + 'two': { + 'values': [{ + 'value': '4.4.4.4', + }, { + 'value': 'nor-is-this', + }] + }, + 'three': { + 'values': [{ + 'weight': 1, + 'value': '5.5.5.5', + }, { + 'weight': 2, + 'value': 'yet-another-bad-one', + }], + }, }, 'rules': [{ - 'pools': { - 100: 'one', - 200: 'two', - } + 'geos': ['AF', 'EU'], + 'pool': 'three', + }, { + 'geos': ['NA-US-CA'], + 'pool': 'two', + }, { + 'pool': 'one', }], }, 'ttl': 60, @@ -2085,25 +2212,42 @@ class TestDynamicRecords(TestCase): } with self.assertRaises(ValidationError) as ctx: Record.new(self.zone, 'bad', a_data) - self.assertEquals(['invalid IPv4 address "nor-is-this"', - 'invalid IPv4 address "this-aint-right"'], - ctx.exception.reasons) + self.assertEquals([ + 'invalid IPv4 address "this-aint-right"', + 'invalid IPv4 address "yet-another-bad-one"', + 'invalid IPv4 address "nor-is-this"', + ], ctx.exception.reasons) # missing value(s) a_data = { 'dynamic': { 'pools': { - 'one': [], - 'two': [ - '3.3.3.3', - '4.4.4.4', - ], + 'one': {}, + 'two': { + 'values': [{ + 'value': '4.4.4.4', + }, { + 'value': '5.5.5.5', + }] + }, + 'three': { + 'values': [{ + 'weight': 1, + 'value': '6.6.6.6', + }, { + 'weight': 2, + 'value': '7.7.7.7', + }], + }, }, 'rules': [{ - 'pools': { - 100: 'one', - 200: 'two', - } + 'geos': ['AF', 'EU'], + 'pool': 'three', + }, { + 'geos': ['NA-US-CA'], + 'pool': 'two', + }, { + 'pool': 'one', }], }, 'ttl': 60, @@ -2115,23 +2259,39 @@ class TestDynamicRecords(TestCase): } with self.assertRaises(ValidationError) as ctx: Record.new(self.zone, 'bad', a_data) - self.assertEquals(['missing value(s)'], ctx.exception.reasons) + self.assertEquals(['pool "one" is missing values'], + ctx.exception.reasons) - # Empty value + # pool valu not a dict a_data = { 'dynamic': { 'pools': { 'one': '', - 'two': [ - '3.3.3.3', - 'blip', - ], + 'two': { + 'values': [{ + 'value': '4.4.4.4', + }, { + 'value': '5.5.5.5', + }] + }, + 'three': { + 'values': [{ + 'weight': 1, + 'value': '6.6.6.6', + }, { + 'weight': 2, + 'value': '7.7.7.7', + }], + }, }, 'rules': [{ - 'pools': { - 100: 'one', - 200: 'two', - } + 'geos': ['AF', 'EU'], + 'pool': 'three', + }, { + 'geos': ['NA-US-CA'], + 'pool': 'two', + }, { + 'pool': 'one', }], }, 'ttl': 60, @@ -2143,24 +2303,39 @@ class TestDynamicRecords(TestCase): } with self.assertRaises(ValidationError) as ctx: Record.new(self.zone, 'bad', a_data) - self.assertEquals(['invalid IPv4 address "blip"', 'empty value'], + self.assertEquals(['pool "one" must be a dict'], ctx.exception.reasons) - # multiple problems + # empty pool value a_data = { 'dynamic': { 'pools': { - 'one': '', - 'two': [ - '3.3.3.3', - 'blip', - ], + 'one': {}, + 'two': { + 'values': [{ + 'value': '4.4.4.4', + }, { + 'value': '5.5.5.5', + }] + }, + 'three': { + 'values': [{ + 'weight': 1, + 'value': '6.6.6.6', + }, { + 'weight': 2, + 'value': '7.7.7.7', + }], + }, }, 'rules': [{ - 'pools': { - 100: 'one', - 200: 'two', - } + 'geos': ['AF', 'EU'], + 'pool': 'three', + }, { + 'geos': ['NA-US-CA'], + 'pool': 'two', + }, { + 'pool': 'one', }], }, 'ttl': 60, @@ -2172,15 +2347,44 @@ class TestDynamicRecords(TestCase): } with self.assertRaises(ValidationError) as ctx: Record.new(self.zone, 'bad', a_data) - self.assertEquals(['invalid IPv4 address "blip"', 'empty value'], + self.assertEquals(['pool "one" is missing values'], ctx.exception.reasons) - # missing rules + # invalid int weight a_data = { 'dynamic': { 'pools': { - 'one': '1.2.3.4', + 'one': { + 'values': [{ + 'value': '3.3.3.3', + }] + }, + 'two': { + 'values': [{ + 'value': '4.4.4.4', + }, { + 'value': '5.5.5.5', + }] + }, + 'three': { + 'values': [{ + 'weight': 1, + 'value': '6.6.6.6', + }, { + 'weight': 256, + 'value': '7.7.7.7', + }], + }, }, + 'rules': [{ + 'geos': ['AF', 'EU'], + 'pool': 'three', + }, { + 'geos': ['NA-US-CA'], + 'pool': 'two', + }, { + 'pool': 'one', + }], }, 'ttl': 60, 'type': 'A', @@ -2191,15 +2395,44 @@ class TestDynamicRecords(TestCase): } with self.assertRaises(ValidationError) as ctx: Record.new(self.zone, 'bad', a_data) - self.assertEquals(['missing rules'], ctx.exception.reasons) + self.assertEquals(['invalid weight "256" in pool "three" value 2'], + ctx.exception.reasons) - # empty rules + # invalid non-int weight a_data = { 'dynamic': { 'pools': { - 'one': '1.2.3.4', + 'one': { + 'values': [{ + 'value': '3.3.3.3', + }] + }, + 'two': { + 'values': [{ + 'value': '4.4.4.4', + }, { + 'value': '5.5.5.5', + }] + }, + 'three': { + 'values': [{ + 'weight': 1, + 'value': '6.6.6.6', + }, { + 'weight': 'foo', + 'value': '7.7.7.7', + }], + }, }, - 'rules': [], + 'rules': [{ + 'geos': ['AF', 'EU'], + 'pool': 'three', + }, { + 'geos': ['NA-US-CA'], + 'pool': 'two', + }, { + 'pool': 'one', + }], }, 'ttl': 60, 'type': 'A', @@ -2210,15 +2443,39 @@ class TestDynamicRecords(TestCase): } with self.assertRaises(ValidationError) as ctx: Record.new(self.zone, 'bad', a_data) - self.assertEquals(['missing rules'], ctx.exception.reasons) + self.assertEquals(['invalid weight "foo" in pool "three" value 2'], + ctx.exception.reasons) - # rules not a list/tuple + # multiple pool problems a_data = { 'dynamic': { 'pools': { - 'one': '1.2.3.4', + 'one': '', + 'two': { + 'values': [{ + 'value': '4.4.4.4', + }, { + 'value': 'blip', + }] + }, + 'three': { + 'values': [{ + 'weight': 1, + }, { + 'weight': 5000, + 'value': '7.7.7.7', + }], + }, }, - 'rules': {}, + 'rules': [{ + 'geos': ['AF', 'EU'], + 'pool': 'three', + }, { + 'geos': ['NA-US-CA'], + 'pool': 'two', + }, { + 'pool': 'one', + }], }, 'ttl': 60, 'type': 'A', @@ -2229,16 +2486,30 @@ class TestDynamicRecords(TestCase): } with self.assertRaises(ValidationError) as ctx: Record.new(self.zone, 'bad', a_data) - self.assertEquals(['rules must be a list'], ctx.exception.reasons) + self.assertEquals([ + 'pool "one" must be a dict', + 'missing value in pool "three" value 1', + 'invalid weight "5000" in pool "three" value 2', + 'invalid IPv4 address "blip"', + ], ctx.exception.reasons) - # rule without pools + # missing rules a_data = { 'dynamic': { 'pools': { - 'one': '1.2.3.4', + 'one': { + 'values': [{ + 'value': '3.3.3.3', + }] + }, + 'two': { + 'values': [{ + 'value': '4.4.4.4', + }, { + 'value': '5.5.5.5', + }] + }, }, - 'rules': [{ - }], }, 'ttl': 60, 'type': 'A', @@ -2249,17 +2520,26 @@ class TestDynamicRecords(TestCase): } with self.assertRaises(ValidationError) as ctx: Record.new(self.zone, 'bad', a_data) - self.assertEquals(['rule 1 missing pools'], ctx.exception.reasons) + self.assertEquals(['missing rules'], ctx.exception.reasons) - # rule with non-dict pools + # empty rules a_data = { 'dynamic': { 'pools': { - 'one': '1.2.3.4', + 'one': { + 'values': [{ + 'value': '3.3.3.3', + }] + }, + 'two': { + 'values': [{ + 'value': '4.4.4.4', + }, { + 'value': '5.5.5.5', + }] + }, }, - 'rules': [{ - 'pools': ['one'], - }], + 'rules': [], }, 'ttl': 60, 'type': 'A', @@ -2270,19 +2550,59 @@ class TestDynamicRecords(TestCase): } with self.assertRaises(ValidationError) as ctx: Record.new(self.zone, 'bad', a_data) - self.assertEquals(["rule 1 pools must be a dict"], - ctx.exception.reasons) + self.assertEquals(['missing rules'], ctx.exception.reasons) - # rule references non-existant pool + # rules not a list/tuple + a_data = { + 'dynamic': { + 'pools': { + 'one': { + 'values': [{ + 'value': '3.3.3.3', + }] + }, + 'two': { + 'values': [{ + 'value': '4.4.4.4', + }, { + 'value': '5.5.5.5', + }] + }, + }, + 'rules': {}, + }, + 'ttl': 60, + 'type': 'A', + 'values': [ + '1.1.1.1', + '2.2.2.2', + ], + } + with self.assertRaises(ValidationError) as ctx: + Record.new(self.zone, 'bad', a_data) + self.assertEquals(['rules must be a list'], ctx.exception.reasons) + + # rule without pool a_data = { 'dynamic': { 'pools': { - 'one': '1.2.3.4', + 'one': { + 'values': [{ + 'value': '3.3.3.3', + }], + }, + 'two': { + 'values': [{ + 'value': '4.4.4.4', + }, { + 'value': '5.5.5.5', + }] + }, }, 'rules': [{ - 'pools': { - 10: 'non-existant' - } + 'geos': ['NA-US-CA'], + }, { + 'pool': 'one', }], }, 'ttl': 60, @@ -2294,19 +2614,30 @@ class TestDynamicRecords(TestCase): } with self.assertRaises(ValidationError) as ctx: Record.new(self.zone, 'bad', a_data) - self.assertEquals(["undefined pool \"non-existant\""], - ctx.exception.reasons) + self.assertEquals(['rule 1 missing pool'], ctx.exception.reasons) - # invalid int weight + # rule with non-string pools a_data = { 'dynamic': { 'pools': { - 'one': '1.2.3.4', + 'one': { + 'values': [{ + 'value': '3.3.3.3', + }] + }, + 'two': { + 'values': [{ + 'value': '4.4.4.4', + }, { + 'value': '5.5.5.5', + }] + }, }, 'rules': [{ - 'pools': { - 256: 'one' - } + 'geos': ['NA-US-CA'], + 'pool': [], + }, { + 'pool': 'one', }], }, 'ttl': 60, @@ -2318,19 +2649,31 @@ class TestDynamicRecords(TestCase): } with self.assertRaises(ValidationError) as ctx: Record.new(self.zone, 'bad', a_data) - self.assertEquals(['invalid pool weight "256"'], + self.assertEquals(['rule 1 invalid pool "[]"'], ctx.exception.reasons) - # invalid non-int weight + # rule references non-existant pool a_data = { 'dynamic': { 'pools': { - 'one': '1.2.3.4', + 'one': { + 'values': [{ + 'value': '3.3.3.3', + }] + }, + 'two': { + 'values': [{ + 'value': '4.4.4.4', + }, { + 'value': '5.5.5.5', + }] + }, }, 'rules': [{ - 'pools': { - 'foo': 'one' - } + 'geos': ['NA-US-CA'], + 'pool': 'non-existant', + }, { + 'pool': 'one', }], }, 'ttl': 60, @@ -2342,7 +2685,7 @@ class TestDynamicRecords(TestCase): } with self.assertRaises(ValidationError) as ctx: Record.new(self.zone, 'bad', a_data) - self.assertEquals(['invalid pool weight "foo"'], + self.assertEquals(["rule 1 undefined pool \"non-existant\""], ctx.exception.reasons) def test_dynamic_lenient(self): From ccd9038a383c5e0a97843a705d6bd1040da0ac32 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Wed, 5 Dec 2018 16:42:38 -0800 Subject: [PATCH 16/29] Impliment Dynamic rule geo validation --- octodns/record.py | 34 +++++++++- tests/config/dynamic.tests.yaml | 18 ++++-- tests/test_octodns_record.py | 107 ++++++++++++++++++++++++++++++++ 3 files changed, 152 insertions(+), 7 deletions(-) diff --git a/octodns/record.py b/octodns/record.py index 6517d74..60a3686 100644 --- a/octodns/record.py +++ b/octodns/record.py @@ -406,6 +406,22 @@ class _DynamicPool(object): return '{}'.format(self.data) +class _DynamicRuleGeo(object): + geo_re = re.compile(r'^(?P\w\w)(-(?P\w\w)' + r'(-(?P\w\w))?)?$') + + @classmethod + def validate(cls, rule_num, code): + reasons = [] + # TODO: ideally this would validate the actual code... + match = cls.geo_re.match(code) + if not match: + reasons.append('rule {} invalid geo "{}"'.format(rule_num, code)) + return reasons + + # TODO: flesh this out + + class _DynamicRule(object): def __init__(self, i, data): @@ -517,6 +533,8 @@ class _DynamicMixin(object): elif not rules: reasons.append('missing rules') else: + seen_default = False + for rule_num, rule in enumerate(rules): rule_num += 1 try: @@ -532,7 +550,21 @@ class _DynamicMixin(object): reasons.append('rule {} undefined pool "{}"' .format(rule_num, pool)) - # TODO: validate GEOs if present + try: + geos = rule['geos'] + except KeyError: + geos = [] + if seen_default: + reasons.append('rule {} duplicate default' + .format(rule_num)) + seen_default = True + + if not isinstance(geos, (list, tuple)): + reasons.append('rule {} geos must be a list' + .format(rule_num)) + else: + for geo in geos: + reasons.extend(_DynamicRuleGeo.validate(rule_num, geo)) return reasons diff --git a/tests/config/dynamic.tests.yaml b/tests/config/dynamic.tests.yaml index ac83ad5..c48735e 100644 --- a/tests/config/dynamic.tests.yaml +++ b/tests/config/dynamic.tests.yaml @@ -21,9 +21,11 @@ a: - value: 5.5.5.5 weight: 25 rules: - - geo: EU-UK + - geos: + - EU-UK pool: iad - - geo: EU + - geos: + - EU pool: ams - geos: - NA-US-CA @@ -55,9 +57,11 @@ aaaa: - value: 2601:642:500:e210:62f8:1dff:feb8:9476 weight: 2 rules: - - geo: EU-UK + - geos: + - EU-UK pool: iad - - geo: EU + - geos: + - EU pool: ams - geos: - NA-US-CA @@ -88,9 +92,11 @@ cname: - value: target-sea-2.unit.tests. weight: 175 rules: - - geo: EU-UK + - geos: + - EU-UK pool: iad - - geo: EU + - geos: + - EU pool: ams - geos: - NA-US-CA diff --git a/tests/test_octodns_record.py b/tests/test_octodns_record.py index 0a4e4df..45853d7 100644 --- a/tests/test_octodns_record.py +++ b/tests/test_octodns_record.py @@ -2688,6 +2688,113 @@ class TestDynamicRecords(TestCase): self.assertEquals(["rule 1 undefined pool \"non-existant\""], ctx.exception.reasons) + # rule with invalid geos + a_data = { + 'dynamic': { + 'pools': { + 'one': { + 'values': [{ + 'value': '3.3.3.3', + }] + }, + 'two': { + 'values': [{ + 'value': '4.4.4.4', + }, { + 'value': '5.5.5.5', + }] + }, + }, + 'rules': [{ + 'geos': 'NA-US-CA', + 'pool': 'two', + }, { + 'pool': 'one', + }], + }, + 'ttl': 60, + 'type': 'A', + 'values': [ + '1.1.1.1', + '2.2.2.2', + ], + } + with self.assertRaises(ValidationError) as ctx: + Record.new(self.zone, 'bad', a_data) + self.assertEquals(['rule 1 geos must be a list'], + ctx.exception.reasons) + + # rule with invalid geo + a_data = { + 'dynamic': { + 'pools': { + 'one': { + 'values': [{ + 'value': '3.3.3.3', + }] + }, + 'two': { + 'values': [{ + 'value': '4.4.4.4', + }, { + 'value': '5.5.5.5', + }] + }, + }, + 'rules': [{ + 'geos': ['invalid'], + 'pool': 'two', + }, { + 'pool': 'one', + }], + }, + 'ttl': 60, + 'type': 'A', + 'values': [ + '1.1.1.1', + '2.2.2.2', + ], + } + with self.assertRaises(ValidationError) as ctx: + Record.new(self.zone, 'bad', a_data) + self.assertEquals(['rule 1 invalid geo "invalid"'], + ctx.exception.reasons) + + # multiple default rules + a_data = { + 'dynamic': { + 'pools': { + 'one': { + 'values': [{ + 'value': '3.3.3.3', + }] + }, + 'two': { + 'values': [{ + 'value': '4.4.4.4', + }, { + 'value': '5.5.5.5', + }] + }, + }, + 'rules': [{ + 'pool': 'two', + }, { + 'pool': 'one', + }], + }, + 'ttl': 60, + 'type': 'A', + 'values': [ + '1.1.1.1', + '2.2.2.2', + ], + } + with self.assertRaises(ValidationError) as ctx: + Record.new(self.zone, 'bad', a_data) + self.assertEquals(['rule 2 duplicate default'], + ctx.exception.reasons) + def test_dynamic_lenient(self): # Missing pools a_data = { From 0114e54bd3720d8600d5e4a21522f287960c3442 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Thu, 6 Dec 2018 06:14:04 -0800 Subject: [PATCH 17/29] Remove _DynamicRuleGeo, its overkill, no details parsing for now --- octodns/record.py | 26 +++++++------------------- 1 file changed, 7 insertions(+), 19 deletions(-) diff --git a/octodns/record.py b/octodns/record.py index 60a3686..61edc24 100644 --- a/octodns/record.py +++ b/octodns/record.py @@ -390,7 +390,6 @@ class _DynamicPool(object): def __init__(self, _id, data): self._id = _id - # TODO: actually parse this self.data = data def _data(self): @@ -406,27 +405,10 @@ class _DynamicPool(object): return '{}'.format(self.data) -class _DynamicRuleGeo(object): - geo_re = re.compile(r'^(?P\w\w)(-(?P\w\w)' - r'(-(?P\w\w))?)?$') - - @classmethod - def validate(cls, rule_num, code): - reasons = [] - # TODO: ideally this would validate the actual code... - match = cls.geo_re.match(code) - if not match: - reasons.append('rule {} invalid geo "{}"'.format(rule_num, code)) - return reasons - - # TODO: flesh this out - - class _DynamicRule(object): def __init__(self, i, data): self.i = i - # TODO: actually parse this self.data = data def _data(self): @@ -472,6 +454,8 @@ class _Dynamic(object): class _DynamicMixin(object): + geo_re = re.compile(r'^(?P\w\w)(-(?P\w\w)' + r'(-(?P\w\w))?)?$') @classmethod def validate(cls, name, data): @@ -564,7 +548,11 @@ class _DynamicMixin(object): .format(rule_num)) else: for geo in geos: - reasons.extend(_DynamicRuleGeo.validate(rule_num, geo)) + # TODO: ideally this would validate the actual code... + match = cls.geo_re.match(geo) + if not match: + reasons.append('rule {} invalid geo "{}"' + .format(rule_num, geo)) return reasons From e3b0ce9dcfc56f3cf409f4da557bb175fd3ef911 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Thu, 6 Dec 2018 06:17:47 -0800 Subject: [PATCH 18/29] Pass cls._type, not cls/record_cls to validate --- octodns/record.py | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/octodns/record.py b/octodns/record.py index 61edc24..dc3db15 100644 --- a/octodns/record.py +++ b/octodns/record.py @@ -262,7 +262,7 @@ class _ValuesMixin(object): except KeyError: values = [] - reasons.extend(cls._value_type.validate(values, cls)) + reasons.extend(cls._value_type.validate(values, cls._type)) return reasons @@ -317,7 +317,7 @@ class _GeoMixin(_ValuesMixin): # TODO: validate legal codes for code, values in geo.items(): reasons.extend(GeoValue._validate_geo(code)) - reasons.extend(cls._value_type.validate(values, cls)) + reasons.extend(cls._value_type.validate(values, cls._type)) except KeyError: pass return reasons @@ -362,7 +362,8 @@ class _ValueMixin(object): @classmethod def validate(cls, name, data): reasons = super(_ValueMixin, cls).validate(name, data) - reasons.extend(cls._value_type.validate(data.get('value', None), cls)) + reasons.extend(cls._value_type.validate(data.get('value', None), + cls._type)) return reasons def __init__(self, zone, name, data, source=None): @@ -502,7 +503,8 @@ class _DynamicMixin(object): try: value = value['value'] - reasons.extend(cls._value_type.validate(value, cls)) + reasons.extend(cls._value_type.validate(value, + cls._type)) except KeyError: reasons.append('missing value in pool "{}" ' 'value {}'.format(_id, value_num)) @@ -620,8 +622,7 @@ class _DynamicMixin(object): class Ipv4List(object): @classmethod - # TODO: remove record_cls it's redudant (cls) - def validate(cls, data, record_cls): + def validate(cls, data, _type): if not isinstance(data, (list, tuple)): data = (data,) if len(data) == 0: @@ -647,7 +648,7 @@ class Ipv4List(object): class Ipv6List(object): @classmethod - def validate(cls, data, record_cls): + def validate(cls, data, _type): if not isinstance(data, (list, tuple)): data = (data,) if len(data) == 0: @@ -673,7 +674,7 @@ class Ipv6List(object): class _TargetValue(object): @classmethod - def validate(cls, data, record_cls): + def validate(cls, data, _type): reasons = [] if data == '': reasons.append('empty value') @@ -681,7 +682,7 @@ class _TargetValue(object): reasons.append('missing value') elif not data.endswith('.'): reasons.append('{} value "{}" missing trailing .' - .format(record_cls._type, data)) + .format(_type, data)) return reasons @classmethod @@ -716,7 +717,7 @@ class CaaValue(object): # https://tools.ietf.org/html/rfc6844#page-5 @classmethod - def validate(cls, data, record_cls): + def validate(cls, data, _type): if not isinstance(data, (list, tuple)): data = (data,) reasons = [] @@ -783,7 +784,7 @@ class CnameRecord(_DynamicMixin, _ValueMixin, Record): class MxValue(object): @classmethod - def validate(cls, data, record_cls): + def validate(cls, data, _type): if not isinstance(data, (list, tuple)): data = (data,) reasons = [] @@ -851,7 +852,7 @@ class NaptrValue(object): VALID_FLAGS = ('S', 'A', 'U', 'P') @classmethod - def validate(cls, data, record_cls): + def validate(cls, data, _type): if not isinstance(data, (list, tuple)): data = (data,) reasons = [] @@ -936,7 +937,7 @@ class NaptrRecord(_ValuesMixin, Record): class _NsValue(object): @classmethod - def validate(cls, data, record_cls): + def validate(cls, data, _type): if not data: return ['missing value(s)'] elif not isinstance(data, (list, tuple)): @@ -972,7 +973,7 @@ class SshfpValue(object): VALID_FINGERPRINT_TYPES = (1, 2) @classmethod - def validate(cls, data, record_cls): + def validate(cls, data, _type): if not isinstance(data, (list, tuple)): data = (data,) reasons = [] @@ -1055,7 +1056,7 @@ class _ChunkedValue(object): _unescaped_semicolon_re = re.compile(r'\w;') @classmethod - def validate(cls, data, record_cls): + def validate(cls, data, _type): if not data: return ['missing value(s)'] elif not isinstance(data, (list, tuple)): @@ -1084,7 +1085,7 @@ class SpfRecord(_ChunkedValuesMixin, Record): class SrvValue(object): @classmethod - def validate(cls, data, record_cls): + def validate(cls, data, _type): if not isinstance(data, (list, tuple)): data = (data,) reasons = [] From a767a5cb252a63fa7143bcdd8a75601931e0dc2c Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Fri, 7 Dec 2018 14:29:05 -0800 Subject: [PATCH 19/29] Implement pool-level fallbacks and validations --- octodns/record.py | 19 +++++++ tests/test_octodns_record.py | 106 +++++++++++++++++++++++++++++++++++ 2 files changed, 125 insertions(+) diff --git a/octodns/record.py b/octodns/record.py index dc3db15..4e434e3 100644 --- a/octodns/record.py +++ b/octodns/record.py @@ -509,6 +509,25 @@ class _DynamicMixin(object): reasons.append('missing value in pool "{}" ' 'value {}'.format(_id, value_num)) + fallback = pool.get('fallback', None) + if fallback is not None and fallback not in pools: + reasons.append('undefined fallback "{}" for pool "{}"' + .format(fallback, _id)) + + # Check for loops + fallback = pools[_id].get('fallback', None) + seen = [_id, fallback] + while fallback is not None: + # See if there's a next fallback + fallback = pools.get(fallback, {}).get('fallback', None) + if fallback in seen: + seen = ' -> '.join(seen) + reasons.append('loop in pool fallbacks: {}' + .format(seen)) + # exit the loop + break + seen.append(fallback) + try: rules = data['dynamic']['rules'] except KeyError: diff --git a/tests/test_octodns_record.py b/tests/test_octodns_record.py index 45853d7..8f64a30 100644 --- a/tests/test_octodns_record.py +++ b/tests/test_octodns_record.py @@ -2177,6 +2177,7 @@ class TestDynamicRecords(TestCase): }], }, 'two': { + 'fallback': 'one', 'values': [{ 'value': '4.4.4.4', }, { @@ -2184,6 +2185,7 @@ class TestDynamicRecords(TestCase): }] }, 'three': { + 'fallback': 'two', 'values': [{ 'weight': 1, 'value': '5.5.5.5', @@ -2446,6 +2448,110 @@ class TestDynamicRecords(TestCase): self.assertEquals(['invalid weight "foo" in pool "three" value 2'], ctx.exception.reasons) + # invalid fallback + a_data = { + 'dynamic': { + 'pools': { + 'one': { + 'values': [{ + 'value': '3.3.3.3', + }], + }, + 'two': { + 'fallback': 'invalid', + 'values': [{ + 'value': '4.4.4.4', + }, { + 'value': '5.5.5.5', + }] + }, + 'three': { + 'fallback': 'two', + 'values': [{ + 'weight': 1, + 'value': '6.6.6.6', + }, { + 'weight': 5, + 'value': '7.7.7.7', + }], + }, + }, + 'rules': [{ + 'geos': ['AF', 'EU'], + 'pool': 'three', + }, { + 'geos': ['NA-US-CA'], + 'pool': 'two', + }, { + 'pool': 'one', + }], + }, + 'ttl': 60, + 'type': 'A', + 'values': [ + '1.1.1.1', + '2.2.2.2', + ], + } + with self.assertRaises(ValidationError) as ctx: + Record.new(self.zone, 'bad', a_data) + self.assertEquals(['undefined fallback "invalid" for pool "two"'], + ctx.exception.reasons) + + # fallback loop + a_data = { + 'dynamic': { + 'pools': { + 'one': { + 'fallback': 'three', + 'values': [{ + 'value': '3.3.3.3', + }], + }, + 'two': { + 'fallback': 'one', + 'values': [{ + 'value': '4.4.4.4', + }, { + 'value': '5.5.5.5', + }] + }, + 'three': { + 'fallback': 'two', + 'values': [{ + 'weight': 1, + 'value': '6.6.6.6', + }, { + 'weight': 5, + 'value': '7.7.7.7', + }], + }, + }, + 'rules': [{ + 'geos': ['AF', 'EU'], + 'pool': 'three', + }, { + 'geos': ['NA-US-CA'], + 'pool': 'two', + }, { + 'pool': 'one', + }], + }, + 'ttl': 60, + 'type': 'A', + 'values': [ + '1.1.1.1', + '2.2.2.2', + ], + } + with self.assertRaises(ValidationError) as ctx: + Record.new(self.zone, 'bad', a_data) + self.assertEquals([ + 'loop in pool fallbacks: one -> three -> two', + 'loop in pool fallbacks: three -> two -> one', + 'loop in pool fallbacks: two -> one -> three' + ], ctx.exception.reasons) + # multiple pool problems a_data = { 'dynamic': { From 36b4a421c9133523cb5a4d3d1183e1e46e2ce395 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 10 Dec 2018 08:24:41 -0800 Subject: [PATCH 20/29] Simplify values -> value -> [] fallback code in _ValuesMixin --- octodns/record.py | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/octodns/record.py b/octodns/record.py index 4e434e3..3a7d552 100644 --- a/octodns/record.py +++ b/octodns/record.py @@ -254,13 +254,7 @@ class _ValuesMixin(object): def validate(cls, name, data): reasons = super(_ValuesMixin, cls).validate(name, data) - try: - values = data['values'] - except KeyError: - try: - values = data['value'] - except KeyError: - values = [] + values = data.get('values', data.get('value', [])) reasons.extend(cls._value_type.validate(values, cls._type)) From 9eaac27ddf379164ad387a9bfc17aac965ae2480 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 10 Dec 2018 08:48:18 -0800 Subject: [PATCH 21/29] Avoid reusing seen, loop instead --- octodns/record.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/octodns/record.py b/octodns/record.py index 175878a..db368a7 100644 --- a/octodns/record.py +++ b/octodns/record.py @@ -515,9 +515,9 @@ class _DynamicMixin(object): # See if there's a next fallback fallback = pools.get(fallback, {}).get('fallback', None) if fallback in seen: - seen = ' -> '.join(seen) + loop = ' -> '.join(seen) reasons.append('loop in pool fallbacks: {}' - .format(seen)) + .format(loop)) # exit the loop break seen.append(fallback) From 7e1b1234b650ac23e2bcef8e14075ae7548fc12f Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 10 Dec 2018 08:54:49 -0800 Subject: [PATCH 22/29] _IpList base class extracted from v4 and v6 --- octodns/record.py | 35 ++++++++++------------------------- 1 file changed, 10 insertions(+), 25 deletions(-) diff --git a/octodns/record.py b/octodns/record.py index db368a7..21ab86f 100644 --- a/octodns/record.py +++ b/octodns/record.py @@ -632,7 +632,7 @@ class _DynamicMixin(object): return super(_DynamicMixin, self).__repr__() -class Ipv4List(object): +class _IpList(object): @classmethod def validate(cls, data, _type): @@ -648,9 +648,10 @@ class Ipv4List(object): reasons.append('missing value(s)') else: try: - IPv4Address(unicode(value)) + cls._address_type(unicode(value)) except Exception: - reasons.append('invalid IPv4 address "{}"'.format(value)) + reasons.append('invalid {} address "{}"' + .format(cls._address_name, value)) return reasons @classmethod @@ -658,30 +659,14 @@ class Ipv4List(object): return values -class Ipv6List(object): +class Ipv4List(_IpList): + _address_name = 'IPv4' + _address_type = IPv4Address - @classmethod - def validate(cls, data, _type): - if not isinstance(data, (list, tuple)): - data = (data,) - if len(data) == 0: - return ['missing value(s)'] - reasons = [] - for value in data: - if value is '': - reasons.append('empty value') - elif value is None: - reasons.append('missing value(s)') - else: - try: - IPv6Address(unicode(value)) - except Exception: - reasons.append('invalid IPv6 address "{}"'.format(value)) - return reasons - @classmethod - def process(cls, values): - return values +class Ipv6List(_IpList): + _address_name = 'IPv6' + _address_type = IPv6Address class _TargetValue(object): From 2c3b7777633780370f53fd4cffdaaa25abd443a9 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 10 Dec 2018 09:01:07 -0800 Subject: [PATCH 23/29] Move octodns/record.py to octodns/record/__init__.py --- octodns/{record.py => record/__init__.py} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename octodns/{record.py => record/__init__.py} (100%) diff --git a/octodns/record.py b/octodns/record/__init__.py similarity index 100% rename from octodns/record.py rename to octodns/record/__init__.py From 50ae8054c78d283d08555476f4a76ae508b8ec26 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 10 Dec 2018 12:55:18 -0800 Subject: [PATCH 24/29] First pass at GeoCodes and geo_data for validating/working with geo codes --- octodns/record/geo.py | 34 ++++ octodns/record/geo_data.py | 316 +++++++++++++++++++++++++++++++ requirements-dev.txt | 2 + script/generate-geo-data | 64 +++++++ tests/test_octodns_record_geo.py | 51 +++++ 5 files changed, 467 insertions(+) create mode 100644 octodns/record/geo.py create mode 100644 octodns/record/geo_data.py create mode 100755 script/generate-geo-data create mode 100644 tests/test_octodns_record_geo.py diff --git a/octodns/record/geo.py b/octodns/record/geo.py new file mode 100644 index 0000000..9a91c0e --- /dev/null +++ b/octodns/record/geo.py @@ -0,0 +1,34 @@ +# +# +# + +from .geo_data import geo_data + + +class GeoCodes(object): + __COUNTRIES = None + + @classmethod + def validate(cls, code): + ''' + Validates an octoDNS geo code making sure that it is a valid and + corresponding: + * continent + * continent & country + * continent, country, & province + ''' + reasons = [] + + pieces = code.split('-') + n = len(pieces) + if n > 3: + reasons.append('Invalid geo code "{}"'.format(code)) + elif n > 0 and pieces[0] not in geo_data: + reasons.append('Unknown continent code "{}"'.format(code)) + elif n > 1 and pieces[1] not in geo_data[pieces[0]]: + reasons.append('Unknown country code "{}"'.format(code)) + elif n > 2 and \ + pieces[2] not in geo_data[pieces[0]][pieces[1]]['provinces']: + reasons.append('Unknown province code "{}"'.format(code)) + + return reasons diff --git a/octodns/record/geo_data.py b/octodns/record/geo_data.py new file mode 100644 index 0000000..5393db0 --- /dev/null +++ b/octodns/record/geo_data.py @@ -0,0 +1,316 @@ +# +# -*- coding: utf-8 -*- +# +# This file is generated using +# ./script/generate-geo-data > octodns/record/geo_data.py +# do not modify it directly +# + +geo_data = \ + {'AF': {'AO': {'name': 'Angola'}, + 'BF': {'name': 'Burkina Faso'}, + 'BI': {'name': 'Burundi'}, + 'BJ': {'name': 'Benin'}, + 'BW': {'name': 'Botswana'}, + 'CD': {'name': 'Congo, The Democratic Republic of the'}, + 'CF': {'name': 'Central African Republic'}, + 'CG': {'name': 'Congo'}, + 'CI': {'name': "Côte d'Ivoire"}, + 'CM': {'name': 'Cameroon'}, + 'CV': {'name': 'Cabo Verde'}, + 'DJ': {'name': 'Djibouti'}, + 'DZ': {'name': 'Algeria'}, + 'EG': {'name': 'Egypt'}, + 'EH': {'name': 'Western Sahara'}, + 'ER': {'name': 'Eritrea'}, + 'ET': {'name': 'Ethiopia'}, + 'GA': {'name': 'Gabon'}, + 'GH': {'name': 'Ghana'}, + 'GM': {'name': 'Gambia'}, + 'GN': {'name': 'Guinea'}, + 'GQ': {'name': 'Equatorial Guinea'}, + 'GW': {'name': 'Guinea-Bissau'}, + 'KE': {'name': 'Kenya'}, + 'KM': {'name': 'Comoros'}, + 'LR': {'name': 'Liberia'}, + 'LS': {'name': 'Lesotho'}, + 'LY': {'name': 'Libya'}, + 'MA': {'name': 'Morocco'}, + 'MG': {'name': 'Madagascar'}, + 'ML': {'name': 'Mali'}, + 'MR': {'name': 'Mauritania'}, + 'MU': {'name': 'Mauritius'}, + 'MW': {'name': 'Malawi'}, + 'MZ': {'name': 'Mozambique'}, + 'NA': {'name': 'Namibia'}, + 'NE': {'name': 'Niger'}, + 'NG': {'name': 'Nigeria'}, + 'RE': {'name': 'Réunion'}, + 'RW': {'name': 'Rwanda'}, + 'SC': {'name': 'Seychelles'}, + 'SD': {'name': 'Sudan'}, + 'SH': {'name': 'Saint Helena, Ascension and Tristan da Cunha'}, + 'SL': {'name': 'Sierra Leone'}, + 'SN': {'name': 'Senegal'}, + 'SO': {'name': 'Somalia'}, + 'SS': {'name': 'South Sudan'}, + 'ST': {'name': 'Sao Tome and Principe'}, + 'SZ': {'name': 'Swaziland'}, + 'TD': {'name': 'Chad'}, + 'TG': {'name': 'Togo'}, + 'TN': {'name': 'Tunisia'}, + 'TZ': {'name': 'Tanzania, United Republic of'}, + 'UG': {'name': 'Uganda'}, + 'YT': {'name': 'Mayotte'}, + 'ZA': {'name': 'South Africa'}, + 'ZM': {'name': 'Zambia'}, + 'ZW': {'name': 'Zimbabwe'}}, + 'AN': {'AQ': {'name': 'Antarctica'}, + 'BV': {'name': 'Bouvet Island'}, + 'HM': {'name': 'Heard Island and McDonald Islands'}, + 'TF': {'name': 'French Southern Territories'}}, + 'AS': {'AE': {'name': 'United Arab Emirates'}, + 'AF': {'name': 'Afghanistan'}, + 'AM': {'name': 'Armenia'}, + 'AZ': {'name': 'Azerbaijan'}, + 'BD': {'name': 'Bangladesh'}, + 'BH': {'name': 'Bahrain'}, + 'BN': {'name': 'Brunei Darussalam'}, + 'BT': {'name': 'Bhutan'}, + 'CC': {'name': 'Cocos (Keeling) Islands'}, + 'CN': {'name': 'China'}, + 'CX': {'name': 'Christmas Island'}, + 'CY': {'name': 'Cyprus'}, + 'GE': {'name': 'Georgia'}, + 'HK': {'name': 'Hong Kong'}, + 'ID': {'name': 'Indonesia'}, + 'IL': {'name': 'Israel'}, + 'IN': {'name': 'India'}, + 'IO': {'name': 'British Indian Ocean Territory'}, + 'IQ': {'name': 'Iraq'}, + 'IR': {'name': 'Iran, Islamic Republic of'}, + 'JO': {'name': 'Jordan'}, + 'JP': {'name': 'Japan'}, + 'KG': {'name': 'Kyrgyzstan'}, + 'KH': {'name': 'Cambodia'}, + 'KP': {'name': "Korea, Democratic People's Republic of"}, + 'KR': {'name': 'Korea, Republic of'}, + 'KW': {'name': 'Kuwait'}, + 'KZ': {'name': 'Kazakhstan'}, + 'LA': {'name': "Lao People's Democratic Republic"}, + 'LB': {'name': 'Lebanon'}, + 'LK': {'name': 'Sri Lanka'}, + 'MM': {'name': 'Myanmar'}, + 'MN': {'name': 'Mongolia'}, + 'MO': {'name': 'Macao'}, + 'MV': {'name': 'Maldives'}, + 'MY': {'name': 'Malaysia'}, + 'NP': {'name': 'Nepal'}, + 'OM': {'name': 'Oman'}, + 'PH': {'name': 'Philippines'}, + 'PK': {'name': 'Pakistan'}, + 'PS': {'name': 'Palestine, State of'}, + 'QA': {'name': 'Qatar'}, + 'SA': {'name': 'Saudi Arabia'}, + 'SG': {'name': 'Singapore'}, + 'SY': {'name': 'Syrian Arab Republic'}, + 'TH': {'name': 'Thailand'}, + 'TJ': {'name': 'Tajikistan'}, + 'TM': {'name': 'Turkmenistan'}, + 'TR': {'name': 'Turkey'}, + 'TW': {'name': 'Taiwan, Province of China'}, + 'UZ': {'name': 'Uzbekistan'}, + 'VN': {'name': 'Viet Nam'}, + 'YE': {'name': 'Yemen'}}, + 'EU': {'AD': {'name': 'Andorra'}, + 'AL': {'name': 'Albania'}, + 'AT': {'name': 'Austria'}, + 'AX': {'name': 'Åland Islands'}, + 'BA': {'name': 'Bosnia and Herzegovina'}, + 'BE': {'name': 'Belgium'}, + 'BG': {'name': 'Bulgaria'}, + 'BY': {'name': 'Belarus'}, + 'CH': {'name': 'Switzerland'}, + 'CZ': {'name': 'Czechia'}, + 'DE': {'name': 'Germany'}, + 'DK': {'name': 'Denmark'}, + 'EE': {'name': 'Estonia'}, + 'ES': {'name': 'Spain'}, + 'FI': {'name': 'Finland'}, + 'FO': {'name': 'Faroe Islands'}, + 'FR': {'name': 'France'}, + 'GB': {'name': 'United Kingdom'}, + 'GG': {'name': 'Guernsey'}, + 'GI': {'name': 'Gibraltar'}, + 'GR': {'name': 'Greece'}, + 'HR': {'name': 'Croatia'}, + 'HU': {'name': 'Hungary'}, + 'IE': {'name': 'Ireland'}, + 'IM': {'name': 'Isle of Man'}, + 'IS': {'name': 'Iceland'}, + 'IT': {'name': 'Italy'}, + 'JE': {'name': 'Jersey'}, + 'LI': {'name': 'Liechtenstein'}, + 'LT': {'name': 'Lithuania'}, + 'LU': {'name': 'Luxembourg'}, + 'LV': {'name': 'Latvia'}, + 'MC': {'name': 'Monaco'}, + 'MD': {'name': 'Moldova, Republic of'}, + 'ME': {'name': 'Montenegro'}, + 'MK': {'name': 'Macedonia, Republic of'}, + 'MT': {'name': 'Malta'}, + 'NL': {'name': 'Netherlands'}, + 'NO': {'name': 'Norway'}, + 'PL': {'name': 'Poland'}, + 'PT': {'name': 'Portugal'}, + 'RO': {'name': 'Romania'}, + 'RS': {'name': 'Serbia'}, + 'RU': {'name': 'Russian Federation'}, + 'SE': {'name': 'Sweden'}, + 'SI': {'name': 'Slovenia'}, + 'SJ': {'name': 'Svalbard and Jan Mayen'}, + 'SK': {'name': 'Slovakia'}, + 'SM': {'name': 'San Marino'}, + 'UA': {'name': 'Ukraine'}, + 'VA': {'name': 'Holy See (Vatican City State)'}}, + 'ID': {'TL': {'name': 'Timor-Leste'}}, + 'NA': {'AG': {'name': 'Antigua and Barbuda'}, + 'AI': {'name': 'Anguilla'}, + 'AW': {'name': 'Aruba'}, + 'BB': {'name': 'Barbados'}, + 'BL': {'name': 'Saint Barthélemy'}, + 'BM': {'name': 'Bermuda'}, + 'BQ': {'name': 'Bonaire, Sint Eustatius and Saba'}, + 'BS': {'name': 'Bahamas'}, + 'BZ': {'name': 'Belize'}, + 'CA': {'name': 'Canada'}, + 'CR': {'name': 'Costa Rica'}, + 'CU': {'name': 'Cuba'}, + 'CW': {'name': 'Curaçao'}, + 'DM': {'name': 'Dominica'}, + 'DO': {'name': 'Dominican Republic'}, + 'GD': {'name': 'Grenada'}, + 'GL': {'name': 'Greenland'}, + 'GP': {'name': 'Guadeloupe'}, + 'GT': {'name': 'Guatemala'}, + 'HN': {'name': 'Honduras'}, + 'HT': {'name': 'Haiti'}, + 'JM': {'name': 'Jamaica'}, + 'KN': {'name': 'Saint Kitts and Nevis'}, + 'KY': {'name': 'Cayman Islands'}, + 'LC': {'name': 'Saint Lucia'}, + 'MF': {'name': 'Saint Martin (French part)'}, + 'MQ': {'name': 'Martinique'}, + 'MS': {'name': 'Montserrat'}, + 'MX': {'name': 'Mexico'}, + 'NI': {'name': 'Nicaragua'}, + 'PA': {'name': 'Panama'}, + 'PM': {'name': 'Saint Pierre and Miquelon'}, + 'PR': {'name': 'Puerto Rico'}, + 'SV': {'name': 'El Salvador'}, + 'SX': {'name': 'Sint Maarten (Dutch part)'}, + 'TC': {'name': 'Turks and Caicos Islands'}, + 'TT': {'name': 'Trinidad and Tobago'}, + 'US': {'name': 'United States', + 'provinces': {'AK': {'name': 'Alaska'}, + 'AL': {'name': 'Alabama'}, + 'AR': {'name': 'Arkansas'}, + 'AS': {'name': 'American Samoa'}, + 'AZ': {'name': 'Arizona'}, + 'CA': {'name': 'California'}, + 'CO': {'name': 'Colorado'}, + 'CT': {'name': 'Connecticut'}, + 'DC': {'name': 'District of Columbia'}, + 'DE': {'name': 'Delaware'}, + 'FL': {'name': 'Florida'}, + 'GA': {'name': 'Georgia'}, + 'GU': {'name': 'Guam'}, + 'HI': {'name': 'Hawaii'}, + 'IA': {'name': 'Iowa'}, + 'ID': {'name': 'Idaho'}, + 'IL': {'name': 'Illinois'}, + 'IN': {'name': 'Indiana'}, + 'KS': {'name': 'Kansas'}, + 'KY': {'name': 'Kentucky'}, + 'LA': {'name': 'Louisiana'}, + 'MA': {'name': 'Massachusetts'}, + 'MD': {'name': 'Maryland'}, + 'ME': {'name': 'Maine'}, + 'MI': {'name': 'Michigan'}, + 'MN': {'name': 'Minnesota'}, + 'MO': {'name': 'Missouri'}, + 'MP': {'name': 'Northern Mariana Islands'}, + 'MS': {'name': 'Mississippi'}, + 'MT': {'name': 'Montana'}, + 'NC': {'name': 'North Carolina'}, + 'ND': {'name': 'North Dakota'}, + 'NE': {'name': 'Nebraska'}, + 'NH': {'name': 'New Hampshire'}, + 'NJ': {'name': 'New Jersey'}, + 'NM': {'name': 'New Mexico'}, + 'NV': {'name': 'Nevada'}, + 'NY': {'name': 'New York'}, + 'OH': {'name': 'Ohio'}, + 'OK': {'name': 'Oklahoma'}, + 'OR': {'name': 'Oregon'}, + 'PA': {'name': 'Pennsylvania'}, + 'PR': {'name': 'Puerto Rico'}, + 'RI': {'name': 'Rhode Island'}, + 'SC': {'name': 'South Carolina'}, + 'SD': {'name': 'South Dakota'}, + 'TN': {'name': 'Tennessee'}, + 'TX': {'name': 'Texas'}, + 'UM': {'name': 'United States Minor Outlying ' + 'Islands'}, + 'UT': {'name': 'Utah'}, + 'VA': {'name': 'Virginia'}, + 'VI': {'name': 'Virgin Islands'}, + 'VT': {'name': 'Vermont'}, + 'WA': {'name': 'Washington'}, + 'WI': {'name': 'Wisconsin'}, + 'WV': {'name': 'West Virginia'}, + 'WY': {'name': 'Wyoming'}}}, + 'VC': {'name': 'Saint Vincent and the Grenadines'}, + 'VG': {'name': 'Virgin Islands, British'}, + 'VI': {'name': 'Virgin Islands, U.S.'}}, + 'OC': {'AS': {'name': 'American Samoa'}, + 'AU': {'name': 'Australia'}, + 'CK': {'name': 'Cook Islands'}, + 'FJ': {'name': 'Fiji'}, + 'FM': {'name': 'Micronesia, Federated States of'}, + 'GU': {'name': 'Guam'}, + 'KI': {'name': 'Kiribati'}, + 'MH': {'name': 'Marshall Islands'}, + 'MP': {'name': 'Northern Mariana Islands'}, + 'NC': {'name': 'New Caledonia'}, + 'NF': {'name': 'Norfolk Island'}, + 'NR': {'name': 'Nauru'}, + 'NU': {'name': 'Niue'}, + 'NZ': {'name': 'New Zealand'}, + 'PF': {'name': 'French Polynesia'}, + 'PG': {'name': 'Papua New Guinea'}, + 'PN': {'name': 'Pitcairn'}, + 'PW': {'name': 'Palau'}, + 'SB': {'name': 'Solomon Islands'}, + 'TK': {'name': 'Tokelau'}, + 'TO': {'name': 'Tonga'}, + 'TV': {'name': 'Tuvalu'}, + 'UM': {'name': 'United States Minor Outlying Islands'}, + 'VU': {'name': 'Vanuatu'}, + 'WF': {'name': 'Wallis and Futuna'}, + 'WS': {'name': 'Samoa'}}, + 'SA': {'AR': {'name': 'Argentina'}, + 'BO': {'name': 'Bolivia, Plurinational State of'}, + 'BR': {'name': 'Brazil'}, + 'CL': {'name': 'Chile'}, + 'CO': {'name': 'Colombia'}, + 'EC': {'name': 'Ecuador'}, + 'FK': {'name': 'Falkland Islands (Malvinas)'}, + 'GF': {'name': 'French Guiana'}, + 'GS': {'name': 'South Georgia and the South Sandwich Islands'}, + 'GY': {'name': 'Guyana'}, + 'PE': {'name': 'Peru'}, + 'PY': {'name': 'Paraguay'}, + 'SR': {'name': 'Suriname'}, + 'UY': {'name': 'Uruguay'}, + 'VE': {'name': 'Venezuela, Bolivarian Republic of'}}} diff --git a/requirements-dev.txt b/requirements-dev.txt index 5b942dd..1afee06 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -2,6 +2,8 @@ coverage mock nose pycodestyle==2.4.0 +pycountry>=18.12.8 +pycountry_convert>=0.7.2 pyflakes==1.6.0 requests_mock twine==1.11.0 diff --git a/script/generate-geo-data b/script/generate-geo-data new file mode 100755 index 0000000..87a57b1 --- /dev/null +++ b/script/generate-geo-data @@ -0,0 +1,64 @@ +#!/usr/bin/env python + +from collections import defaultdict +from pprint import pformat +from pycountry import countries, subdivisions +from pycountry_convert import country_alpha2_to_continent_code + + +subs = defaultdict(dict) +for subdivision in subdivisions: + # Route53 only supports US states, Dyn supports US states and CA provinces, but for now we'll just do US + if subdivision.country_code not in ('US'): + continue + subs[subdivision.country_code][subdivision.code[3:]] = { + 'name': subdivision.name + } +subs = dict(subs) +#pprint(subs) + +# These are best guesses at things pycountry_convert doesn't map +continent_backups = { + 'AQ': 'AN', + 'EH': 'AF', + 'PN': 'OC', + 'SX': 'NA', + 'TF': 'AN', + 'TL': 'ID', + 'UM': 'OC', + 'VA': 'EU', +} + +geos = defaultdict(dict) +for country in countries: + try: + continent_code = country_alpha2_to_continent_code(country.alpha_2) + except KeyError: + try: + continent_code = continent_backups[country.alpha_2] + except KeyError: + raise + print('{} {} {}'.format(country.alpha_2, country.name, getattr(country, 'official_name', ''))) + + geos[continent_code][country.alpha_2] = { + 'name': country.name + } + + try: + geos[continent_code][country.alpha_2]['provinces'] = subs[country.alpha_2] + except KeyError: + pass + +geos = dict(geos) +data = pformat(geos).replace('\n', '\n ') + +print('''# +# -*- coding: utf-8 -*- +# +# This file is generated using +# ./script/generate-geo-data > octodns/record/geo_data.py +# do not modify it directly +# + +geo_data = \\ + {}'''.format(data)) diff --git a/tests/test_octodns_record_geo.py b/tests/test_octodns_record_geo.py new file mode 100644 index 0000000..00e0bdc --- /dev/null +++ b/tests/test_octodns_record_geo.py @@ -0,0 +1,51 @@ +# +# +# + +from __future__ import absolute_import, division, print_function, \ + unicode_literals + +from unittest import TestCase + +from octodns.record.geo import GeoCodes + + +class TestRecordGeoCodes(TestCase): + + def test_validate(self): + # All valid + self.assertEquals([], GeoCodes.validate('NA')) + self.assertEquals([], GeoCodes.validate('NA-US')) + self.assertEquals([], GeoCodes.validate('NA-US-OR')) + + # Just plain bad + self.assertEquals(['Invalid geo code "XX-YY-ZZ-AA"'], + GeoCodes.validate('XX-YY-ZZ-AA')) + self.assertEquals(['Unknown continent code "X-Y-Z"'], + GeoCodes.validate('X-Y-Z')) + self.assertEquals(['Unknown continent code "XXX-Y-Z"'], + GeoCodes.validate('XXX-Y-Z')) + + # Bad continent + self.assertEquals(['Unknown continent code "XX"'], + GeoCodes.validate('XX')) + # Bad continent good country + self.assertEquals(['Unknown continent code "XX-US"'], + GeoCodes.validate('XX-US')) + # Bad continent good country and province + self.assertEquals(['Unknown continent code "XX-US-OR"'], + GeoCodes.validate('XX-US-OR')) + + # Bad country, good continent + self.assertEquals(['Unknown country code "NA-XX"'], + GeoCodes.validate('NA-XX')) + # Bad country, good continent and state + self.assertEquals(['Unknown country code "NA-XX-OR"'], + GeoCodes.validate('NA-XX-OR')) + # Good country, good continent, but bad match + self.assertEquals(['Unknown country code "NA-GB"'], + GeoCodes.validate('NA-GB')) + + # Bad province code, good continent and country + self.assertEquals(['Unknown province code "NA-US-XX"'], + GeoCodes.validate('NA-US-XX')) From 388e9a67e04e37ac79ae6f89c21f9a368faac634 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 10 Dec 2018 13:04:25 -0800 Subject: [PATCH 25/29] Hook up new GeoCodes validation to _DynamicMixin --- octodns/record/__init__.py | 9 +++--- octodns/record/geo.py | 11 ++++---- tests/test_octodns_record.py | 2 +- tests/test_octodns_record_geo.py | 48 +++++++++++++++++--------------- 4 files changed, 36 insertions(+), 34 deletions(-) diff --git a/octodns/record/__init__.py b/octodns/record/__init__.py index 21ab86f..ba5877f 100644 --- a/octodns/record/__init__.py +++ b/octodns/record/__init__.py @@ -9,6 +9,8 @@ from ipaddress import IPv4Address, IPv6Address from logging import getLogger import re +from .geo import GeoCodes + class Change(object): @@ -563,11 +565,8 @@ class _DynamicMixin(object): .format(rule_num)) else: for geo in geos: - # TODO: ideally this would validate the actual code... - match = cls.geo_re.match(geo) - if not match: - reasons.append('rule {} invalid geo "{}"' - .format(rule_num, geo)) + reasons.extend(GeoCodes.validate(geo, 'rule {} ' + .format(rule_num))) return reasons diff --git a/octodns/record/geo.py b/octodns/record/geo.py index 9a91c0e..c7b5468 100644 --- a/octodns/record/geo.py +++ b/octodns/record/geo.py @@ -9,7 +9,7 @@ class GeoCodes(object): __COUNTRIES = None @classmethod - def validate(cls, code): + def validate(cls, code, prefix): ''' Validates an octoDNS geo code making sure that it is a valid and corresponding: @@ -22,13 +22,14 @@ class GeoCodes(object): pieces = code.split('-') n = len(pieces) if n > 3: - reasons.append('Invalid geo code "{}"'.format(code)) + reasons.append('{}invalid geo code "{}"'.format(prefix, code)) elif n > 0 and pieces[0] not in geo_data: - reasons.append('Unknown continent code "{}"'.format(code)) + reasons.append('{}unknown continent code "{}"' + .format(prefix, code)) elif n > 1 and pieces[1] not in geo_data[pieces[0]]: - reasons.append('Unknown country code "{}"'.format(code)) + reasons.append('{}unknown country code "{}"'.format(prefix, code)) elif n > 2 and \ pieces[2] not in geo_data[pieces[0]][pieces[1]]['provinces']: - reasons.append('Unknown province code "{}"'.format(code)) + reasons.append('{}unknown province code "{}"'.format(prefix, code)) return reasons diff --git a/tests/test_octodns_record.py b/tests/test_octodns_record.py index 8f64a30..6be334d 100644 --- a/tests/test_octodns_record.py +++ b/tests/test_octodns_record.py @@ -2863,7 +2863,7 @@ class TestDynamicRecords(TestCase): } with self.assertRaises(ValidationError) as ctx: Record.new(self.zone, 'bad', a_data) - self.assertEquals(['rule 1 invalid geo "invalid"'], + self.assertEquals(['rule 1 unknown continent code "invalid"'], ctx.exception.reasons) # multiple default rules diff --git a/tests/test_octodns_record_geo.py b/tests/test_octodns_record_geo.py index 00e0bdc..6b9cd4e 100644 --- a/tests/test_octodns_record_geo.py +++ b/tests/test_octodns_record_geo.py @@ -13,39 +13,41 @@ from octodns.record.geo import GeoCodes class TestRecordGeoCodes(TestCase): def test_validate(self): + prefix = 'xyz ' + # All valid - self.assertEquals([], GeoCodes.validate('NA')) - self.assertEquals([], GeoCodes.validate('NA-US')) - self.assertEquals([], GeoCodes.validate('NA-US-OR')) + self.assertEquals([], GeoCodes.validate('NA', prefix)) + self.assertEquals([], GeoCodes.validate('NA-US', prefix)) + self.assertEquals([], GeoCodes.validate('NA-US-OR', prefix)) # Just plain bad - self.assertEquals(['Invalid geo code "XX-YY-ZZ-AA"'], - GeoCodes.validate('XX-YY-ZZ-AA')) - self.assertEquals(['Unknown continent code "X-Y-Z"'], - GeoCodes.validate('X-Y-Z')) - self.assertEquals(['Unknown continent code "XXX-Y-Z"'], - GeoCodes.validate('XXX-Y-Z')) + self.assertEquals(['xyz invalid geo code "XX-YY-ZZ-AA"'], + GeoCodes.validate('XX-YY-ZZ-AA', prefix)) + self.assertEquals(['xyz unknown continent code "X-Y-Z"'], + GeoCodes.validate('X-Y-Z', prefix)) + self.assertEquals(['xyz unknown continent code "XXX-Y-Z"'], + GeoCodes.validate('XXX-Y-Z', prefix)) # Bad continent - self.assertEquals(['Unknown continent code "XX"'], - GeoCodes.validate('XX')) + self.assertEquals(['xyz unknown continent code "XX"'], + GeoCodes.validate('XX', prefix)) # Bad continent good country - self.assertEquals(['Unknown continent code "XX-US"'], - GeoCodes.validate('XX-US')) + self.assertEquals(['xyz unknown continent code "XX-US"'], + GeoCodes.validate('XX-US', prefix)) # Bad continent good country and province - self.assertEquals(['Unknown continent code "XX-US-OR"'], - GeoCodes.validate('XX-US-OR')) + self.assertEquals(['xyz unknown continent code "XX-US-OR"'], + GeoCodes.validate('XX-US-OR', prefix)) # Bad country, good continent - self.assertEquals(['Unknown country code "NA-XX"'], - GeoCodes.validate('NA-XX')) + self.assertEquals(['xyz unknown country code "NA-XX"'], + GeoCodes.validate('NA-XX', prefix)) # Bad country, good continent and state - self.assertEquals(['Unknown country code "NA-XX-OR"'], - GeoCodes.validate('NA-XX-OR')) + self.assertEquals(['xyz unknown country code "NA-XX-OR"'], + GeoCodes.validate('NA-XX-OR', prefix)) # Good country, good continent, but bad match - self.assertEquals(['Unknown country code "NA-GB"'], - GeoCodes.validate('NA-GB')) + self.assertEquals(['xyz unknown country code "NA-GB"'], + GeoCodes.validate('NA-GB', prefix)) # Bad province code, good continent and country - self.assertEquals(['Unknown province code "NA-US-XX"'], - GeoCodes.validate('NA-US-XX')) + self.assertEquals(['xyz unknown province code "NA-US-XX"'], + GeoCodes.validate('NA-US-XX', prefix)) From f1d5808ddc358307e6e453e615378b383bce659c Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 10 Dec 2018 13:05:21 -0800 Subject: [PATCH 26/29] Heh, apparently there were some invalid Geo codes in tests --- tests/config/dynamic.tests.yaml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/config/dynamic.tests.yaml b/tests/config/dynamic.tests.yaml index c48735e..fb33aec 100644 --- a/tests/config/dynamic.tests.yaml +++ b/tests/config/dynamic.tests.yaml @@ -22,7 +22,7 @@ a: weight: 25 rules: - geos: - - EU-UK + - EU-GB pool: iad - geos: - EU @@ -58,7 +58,7 @@ aaaa: weight: 2 rules: - geos: - - EU-UK + - EU-GB pool: iad - geos: - EU @@ -93,7 +93,7 @@ cname: weight: 175 rules: - geos: - - EU-UK + - EU-GB pool: iad - geos: - EU From f95014f2f4ec8024c38215825e585add0f7e1fa8 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Wed, 12 Dec 2018 11:11:53 -0800 Subject: [PATCH 27/29] Don't deprecate geo bits yet, wait until we have full dynamic impl --- octodns/record/__init__.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/octodns/record/__init__.py b/octodns/record/__init__.py index ba5877f..b45924e 100644 --- a/octodns/record/__init__.py +++ b/octodns/record/__init__.py @@ -322,8 +322,6 @@ class _GeoMixin(_ValuesMixin): super(_GeoMixin, self).__init__(zone, name, data, *args, **kwargs) try: self.geo = dict(data['geo']) - self.log.warn("'geo' support has been deprecated, " - "transition %s to use 'dynamic'", name) except KeyError: self.geo = {} for code, values in self.geo.items(): From 0ebd5bb0d4bcf029e9198187c03cdf425de08d43 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Wed, 12 Dec 2018 11:12:31 -0800 Subject: [PATCH 28/29] Remove stale/wontfix comment --- octodns/record/__init__.py | 1 - 1 file changed, 1 deletion(-) diff --git a/octodns/record/__init__.py b/octodns/record/__init__.py index b45924e..a198cc5 100644 --- a/octodns/record/__init__.py +++ b/octodns/record/__init__.py @@ -310,7 +310,6 @@ class _GeoMixin(_ValuesMixin): reasons = super(_GeoMixin, cls).validate(name, data) try: geo = dict(data['geo']) - # TODO: validate legal codes for code, values in geo.items(): reasons.extend(GeoValue._validate_geo(code)) reasons.extend(cls._value_type.validate(values, cls._type)) From 0392864602fd36167442c25355717a3ad927b9ca Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Wed, 12 Dec 2018 11:22:02 -0800 Subject: [PATCH 29/29] Update CHANGELOG to include progress up to now --- CHANGELOG.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0a95ecb..0cdab51 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,22 @@ +## v0.9.4 - ....-..-.. - Dynamic Records Beta + +* Dynamic Records (more powerful/flexible replacement for geo) + * Will support A, AAAA, and CNAME out the gate and include the ability to + weight records. It should provide a foundation for further suppport + if/when needed. + * Major refactoring and improvements to validation of the Record hierarchy, + things are much more consisntely implemented now and error messages should + be more actionable/clear. Both the base values and dynamic values use the + same validatio logic. +* natsort version bump to address setup issues +* DNSSimple TXT record handling fixes, ; it's always ; +* Route53Provider support for sessiom tokens +* Add ALIAS to the list of Cloudflare record types that support proxying +* Fix for TTL bug in Dyn CCA records +* Records updated so that 'octodns' record metadata is persisted through + YamlProvider +* Added --version support to ArguementParser (thus all commands) + ## v0.9.3 - 2018-10-29 - Misc. stuff sort of release * ZoneFile source added