From c04a320cfd6a1f9225261f2e8bde2cd2d44e8ea1 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 15 Jan 2024 16:02:35 -0800 Subject: [PATCH] Treat value/values interchangably when configuring records - All of the `if data isn't a list turn it into one in the value type validates are no longer needed, they'll always be passed a list now` - Special case to handle PTR/target values since it previously was single value and is now values See README for more information --- CHANGELOG.md | 9 +++-- octodns/record/base.py | 20 +++-------- octodns/record/caa.py | 2 -- octodns/record/loc.py | 2 -- octodns/record/mx.py | 2 -- octodns/record/naptr.py | 2 -- octodns/record/srv.py | 2 -- octodns/record/sshfp.py | 2 -- octodns/record/target.py | 4 +-- octodns/record/tlsa.py | 2 -- octodns/record/urlfwd.py | 2 -- tests/test_octodns_record.py | 64 ++++++++++++++++++++++-------------- 12 files changed, 51 insertions(+), 62 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1822f88..08e6ab0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,9 +8,12 @@ * Support added for config env variable expansion on nested levels, not just top-level provider/processor keys * _ChunkedValue ASCII validation added, SPF & TXT -* Minor validation improvement to catch cases where Record values is provided a - single a-single-value by accident instead of a list, a common type-o that - can results in each character being a value in the resulting Record data +* Re-work value/values handling to always try and do the "right" thing based on + the content, so both singular values and lists will be handled identically + regardless of whether the key is value or values. This may result in + changes/fixes on the first sync after updating IFF you currently have + `values: a-single-thing`, which would have previously been pushed up as bunch + of single character values. ## v1.4.0 - 2023-12-04 - Minor Meta diff --git a/octodns/record/base.py b/octodns/record/base.py index 44c1941..058b12d 100644 --- a/octodns/record/base.py +++ b/octodns/record/base.py @@ -277,14 +277,8 @@ class ValuesMixin(object): def validate(cls, name, fqdn, data): reasons = super().validate(name, fqdn, data) - try: - values = data['values'] - if isinstance(values, str): - reasons.append( - f'single value provided under values key, "{values}"' - ) - except KeyError: - values = data.get('value', []) + values = data.get('values', data.get('value', [])) + values = values if isinstance(values, (list, tuple)) else [values] reasons.extend(cls._value_type.validate(values, cls._type)) @@ -300,13 +294,9 @@ class ValuesMixin(object): def __init__(self, zone, name, data, source=None, context=None): super().__init__(zone, name, data, source=source, context=context) - try: - values = data['values'] - except KeyError: - try: - values = [data['value']] - except KeyError: - values = [] + + values = data.get('values', data.get('value', [])) + values = values if isinstance(values, (list, tuple)) else [values] self.values = sorted(self._value_type.process(values)) def changes(self, other, target): diff --git a/octodns/record/caa.py b/octodns/record/caa.py index e95bb6b..f512e9a 100644 --- a/octodns/record/caa.py +++ b/octodns/record/caa.py @@ -26,8 +26,6 @@ class CaaValue(EqualityTupleMixin, dict): @classmethod def validate(cls, data, _type): - if not isinstance(data, (list, tuple)): - data = (data,) reasons = [] for value in data: try: diff --git a/octodns/record/loc.py b/octodns/record/loc.py index 7c55ecb..babbb93 100644 --- a/octodns/record/loc.py +++ b/octodns/record/loc.py @@ -110,8 +110,6 @@ class LocValue(EqualityTupleMixin, dict): direction_keys = ['lat_direction', 'long_direction'] - if not isinstance(data, (list, tuple)): - data = (data,) reasons = [] for value in data: for key in int_keys: diff --git a/octodns/record/mx.py b/octodns/record/mx.py index d24aa97..36b48b5 100644 --- a/octodns/record/mx.py +++ b/octodns/record/mx.py @@ -26,8 +26,6 @@ class MxValue(EqualityTupleMixin, dict): @classmethod def validate(cls, data, _type): - if not isinstance(data, (list, tuple)): - data = (data,) reasons = [] for value in data: try: diff --git a/octodns/record/naptr.py b/octodns/record/naptr.py index 14d541d..8c5ac0c 100644 --- a/octodns/record/naptr.py +++ b/octodns/record/naptr.py @@ -43,8 +43,6 @@ class NaptrValue(EqualityTupleMixin, dict): @classmethod def validate(cls, data, _type): - if not isinstance(data, (list, tuple)): - data = (data,) reasons = [] for value in data: try: diff --git a/octodns/record/srv.py b/octodns/record/srv.py index 058db79..e885735 100644 --- a/octodns/record/srv.py +++ b/octodns/record/srv.py @@ -41,8 +41,6 @@ class SrvValue(EqualityTupleMixin, dict): @classmethod def validate(cls, data, _type): - if not isinstance(data, (list, tuple)): - data = (data,) reasons = [] for value in data: # TODO: validate algorithm and fingerprint_type values diff --git a/octodns/record/sshfp.py b/octodns/record/sshfp.py index d92cbd2..e1c9de0 100644 --- a/octodns/record/sshfp.py +++ b/octodns/record/sshfp.py @@ -34,8 +34,6 @@ class SshfpValue(EqualityTupleMixin, dict): @classmethod def validate(cls, data, _type): - if not isinstance(data, (list, tuple)): - data = (data,) reasons = [] for value in data: try: diff --git a/octodns/record/target.py b/octodns/record/target.py index f9dbc18..3d6cea7 100644 --- a/octodns/record/target.py +++ b/octodns/record/target.py @@ -51,10 +51,8 @@ class _TargetsValue(str): @classmethod def validate(cls, data, _type): - if not data: + if not data or all(not d for d in data): return ['missing value(s)'] - elif not isinstance(data, (list, tuple)): - data = (data,) reasons = [] for value in data: value = idna_encode(value) diff --git a/octodns/record/tlsa.py b/octodns/record/tlsa.py index 77a0ec5..ed7b267 100644 --- a/octodns/record/tlsa.py +++ b/octodns/record/tlsa.py @@ -41,8 +41,6 @@ class TlsaValue(EqualityTupleMixin, dict): @classmethod def validate(cls, data, _type): - if not isinstance(data, (list, tuple)): - data = (data,) reasons = [] for value in data: try: diff --git a/octodns/record/urlfwd.py b/octodns/record/urlfwd.py index 04f9f6a..d2fde09 100644 --- a/octodns/record/urlfwd.py +++ b/octodns/record/urlfwd.py @@ -13,8 +13,6 @@ class UrlfwdValue(EqualityTupleMixin, dict): @classmethod def validate(cls, data, _type): - if not isinstance(data, (list, tuple)): - data = (data,) reasons = [] for value in data: try: diff --git a/tests/test_octodns_record.py b/tests/test_octodns_record.py index 8ae5238..c80058f 100644 --- a/tests/test_octodns_record.py +++ b/tests/test_octodns_record.py @@ -683,42 +683,56 @@ class TestRecordValidation(TestCase): lenient=True, ) - def test_values_is_single_value(self): - with self.assertRaises(ValidationError) as ctx: - Record.new( - self.zone, - 'thing', - {'type': 'TXT', 'ttl': 42, 'values': 'just one'}, - ) - self.assertEqual( - ['single value provided under values key, "just one"'], - ctx.exception.reasons, + def test_values_and_value(self): + # value w/one + r = Record.new( + self.zone, 'thing', {'type': 'TXT', 'ttl': 42, 'value': 'just one'} ) + self.assertEqual(['just one'], r.values) - # same thing is fine as `value` - txt = Record.new( - self.zone, 'thing', {'type': 'TXT', 'ttl': 42, 'value': 'just one'} + # value w/multiple + r = Record.new( + self.zone, + 'thing', + {'type': 'TXT', 'ttl': 42, 'value': ['the first', 'the second']}, ) - self.assertEqual(1, len(txt.values)) - self.assertEqual('just one', txt.values[0]) + self.assertEqual(['the first', 'the second'], r.values) - # same thing is fine when a list - txt = Record.new( + # values w/one + r = Record.new( + self.zone, 'thing', {'type': 'TXT', 'ttl': 42, 'values': 'just one'} + ) + self.assertEqual(['just one'], r.values) + + # values w/multiple + r = Record.new( self.zone, 'thing', - {'type': 'TXT', 'ttl': 42, 'values': ['just one']}, + {'type': 'TXT', 'ttl': 42, 'values': ['the first', 'the second']}, ) - self.assertEqual(1, len(txt.values)) - self.assertEqual('just one', txt.values[0]) + self.assertEqual(['the first', 'the second'], r.values) - # or tuple - txt = Record.new( + # tuples work too + r = Record.new( self.zone, 'thing', - {'type': 'TXT', 'ttl': 42, 'values': ('just one',)}, + {'type': 'TXT', 'ttl': 42, 'values': ('the first', 'the second')}, + ) + self.assertEqual(['the first', 'the second'], r.values) + + # values is preferred over value + # values w/multiple + r = Record.new( + self.zone, + 'thing', + { + 'type': 'TXT', + 'ttl': 42, + 'values': ['the first', 'the second'], + 'value': ['not used', 'not used'], + }, ) - self.assertEqual(1, len(txt.values)) - self.assertEqual('just one', txt.values[0]) + self.assertEqual(['the first', 'the second'], r.values) def test_validation_context(self): # fails validation, no context