diff --git a/CHANGELOG.md b/CHANGELOG.md index 1d32bd6..d353bd8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ ## v1.?.0 - 2023-??-?? - * Record.lenient property added similar to other common/standard _octodns data +* Fix bug with Record.copy when values is an empty list [] * Processor.process_source_and_target_zones added to support modifying both the desired and/or existing zones just prior to computing changes. diff --git a/README.md b/README.md index 4c5cac0..f69f17c 100644 --- a/README.md +++ b/README.md @@ -313,8 +313,8 @@ Similar to providers, but can only serve to populate records into a zone, cannot | Source | Record Support | Dynamic | Notes | |--|--|--|--| | [EnvVarSource](/octodns/source/envvar.py) | TXT | No | read-only environment variable injection | -| [AxfrSource](/octodns/source/axfr.py) | A, AAAA, CAA, CNAME, LOC, MX, NS, PTR, SPF, SRV, TXT | No | read-only | -| [ZoneFileSource](/octodns/source/axfr.py) | A, AAAA, CAA, CNAME, MX, NS, PTR, SPF, SRV, TXT | No | read-only | +| [AxfrSource](https://github.com/octodns/octodns-bind/) | A, AAAA, CAA, CNAME, LOC, MX, NS, PTR, SPF, SRV, TXT | No | read-only | +| [ZoneFileSource](https://github.com/octodns/octodns-bind/) | A, AAAA, CAA, CNAME, MX, NS, PTR, SPF, SRV, TXT | No | read-only | | [TinyDnsFileSource](/octodns/source/tinydns.py) | A, CNAME, MX, NS, PTR | No | read-only | ### Notes diff --git a/octodns/record/base.py b/octodns/record/base.py index 6f83578..443e7ba 100644 --- a/octodns/record/base.py +++ b/octodns/record/base.py @@ -296,7 +296,10 @@ class ValuesMixin(object): try: values = data['values'] except KeyError: - values = [data['value']] + try: + values = [data['value']] + except KeyError: + values = [] self.values = sorted(self._value_type.process(values)) def changes(self, other, target): @@ -306,16 +309,16 @@ class ValuesMixin(object): def _data(self): ret = super()._data() - if len(self.values) > 1: - values = [getattr(v, 'data', v) for v in self.values if v] - if len(values) > 1: - ret['values'] = values - elif len(values) == 1: - ret['value'] = values[0] - elif len(self.values) == 1: + if len(self.values) == 1: v = self.values[0] if v: ret['value'] = getattr(v, 'data', v) + else: + values = [getattr(v, 'data', v) for v in self.values if v] + if len(values) == 1: + ret['value'] = values[0] + else: + ret['values'] = values return ret @@ -371,8 +374,7 @@ class ValueMixin(object): def _data(self): ret = super()._data() - if self.value: - ret['value'] = getattr(self.value, 'data', self.value) + ret['value'] = getattr(self.value, 'data', self.value) return ret @property diff --git a/tests/test_octodns_record.py b/tests/test_octodns_record.py index 66f0b9f..c85341f 100644 --- a/tests/test_octodns_record.py +++ b/tests/test_octodns_record.py @@ -197,19 +197,19 @@ class TestRecord(TestCase): ) def test_values_mixin_data(self): - # no values, no value or values in data + # empty values -> empty values in data a = ARecord(self.zone, '', {'type': 'A', 'ttl': 600, 'values': []}) - self.assertNotIn('values', a.data) + self.assertEqual([], a.data['values']) # empty value, no value or values in data b = ARecord(self.zone, '', {'type': 'A', 'ttl': 600, 'values': ['']}) self.assertNotIn('value', b.data) - # empty/None values, no value or values in data + # empty/None values -> empty values in data c = ARecord( self.zone, '', {'type': 'A', 'ttl': 600, 'values': ['', None]} ) - self.assertNotIn('values', c.data) + self.assertEqual([], a.data['values']) # empty/None values and valid, value in data c = ARecord( @@ -225,13 +225,13 @@ class TestRecord(TestCase): a = AliasRecord( self.zone, '', {'type': 'ALIAS', 'ttl': 600, 'value': None} ) - self.assertNotIn('value', a.data) + self.assertIsNone(a.data['value']) # unspecified value, no value in data a = AliasRecord( self.zone, '', {'type': 'ALIAS', 'ttl': 600, 'value': ''} ) - self.assertNotIn('value', a.data) + self.assertIsNone(a.data['value']) def test_record_new(self): txt = Record.new( @@ -251,6 +251,20 @@ class TestRecord(TestCase): Record.new(self.zone, 'unknown', {'type': 'XXX'}) self.assertTrue('Unknown record type' in str(ctx.exception)) + def test_record_new_with_values_and_value(self): + a = Record.new( + self.zone, + 'a', + { + 'ttl': 44, + 'type': 'A', + 'value': '1.2.3.4', + 'values': ['2.3.4.5', '3.4.5.6'], + }, + ) + # values is preferred over value when both exist + self.assertEqual(['2.3.4.5', '3.4.5.6'], a.values) + def test_record_copy(self): a = Record.new( self.zone, 'a', {'ttl': 44, 'type': 'A', 'value': '1.2.3.4'} @@ -338,6 +352,27 @@ class TestRecord(TestCase): del b._octodns['key']['second'] self.assertNotEqual(a.data, b.data) + def test_record_copy_with_no_values(self): + txt = Record.new( + self.zone, + 'txt', + {'ttl': 45, 'type': 'TXT', 'values': []}, + lenient=True, + ) + + dup = txt.copy() + self.assertEqual(txt.values, dup.values) + + cname = Record.new( + self.zone, + 'cname', + {'ttl': 45, 'type': 'CNAME', 'value': ''}, + lenient=True, + ) + + dup = cname.copy() + self.assertEqual(cname.value, dup.value) + def test_change(self): existing = Record.new( self.zone, 'txt', {'ttl': 44, 'type': 'TXT', 'value': 'some text'} @@ -631,10 +666,9 @@ class TestRecordValidation(TestCase): lenient=True, ) - # __init__ may still blow up, even if validation is lenient - with self.assertRaises(KeyError) as ctx: - Record.new(self.zone, 'www', {'type': 'A', 'ttl': -1}, lenient=True) - self.assertEqual(('value',), ctx.exception.args) + # empty values is allowed with lenient + r = Record.new(self.zone, 'www', {'type': 'A', 'ttl': -1}, lenient=True) + self.assertEqual([], r.values) # no exception if we're in lenient mode from config Record.new(