From 1340aee8a934c690d74001b3780880b5eee8b2fa Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Fri, 23 Jun 2017 13:04:38 -0700 Subject: [PATCH] MX RFC1035 - priority -> preference & value -> exchange --- octodns/provider/cloudflare.py | 8 ++-- octodns/provider/dnsimple.py | 8 ++-- octodns/provider/dyn.py | 6 +-- octodns/provider/ns1.py | 8 ++-- octodns/provider/powerdns.py | 8 ++-- octodns/provider/route53.py | 9 ++-- octodns/record.py | 41 +++++++++++------- octodns/source/tinydns.py | 4 +- tests/config/unit.tests.yaml | 12 +++--- tests/test_octodns_provider_dyn.py | 8 ++-- tests/test_octodns_provider_ns1.py | 8 ++-- tests/test_octodns_provider_route53.py | 12 +++--- tests/test_octodns_record.py | 58 ++++++++++++++++---------- tests/test_octodns_source_tinydns.py | 16 +++---- 14 files changed, 118 insertions(+), 88 deletions(-) diff --git a/octodns/provider/cloudflare.py b/octodns/provider/cloudflare.py index 51b2171..2ee8f8b 100644 --- a/octodns/provider/cloudflare.py +++ b/octodns/provider/cloudflare.py @@ -116,8 +116,8 @@ class CloudflareProvider(BaseProvider): values = [] for r in records: values.append({ - 'priority': r['priority'], - 'value': '{}.'.format(r['content']), + 'preference': r['priority'], + 'exchange': '{}.'.format(r['content']), }) return { 'ttl': records[0]['ttl'], @@ -207,8 +207,8 @@ class CloudflareProvider(BaseProvider): def _contents_for_MX(self, record): for value in record.values: yield { - 'priority': value.priority, - 'content': value.value + 'priority': value.preference, + 'content': value.exchange } def _apply_Create(self, change): diff --git a/octodns/provider/dnsimple.py b/octodns/provider/dnsimple.py index 91bd638..dc44d1b 100644 --- a/octodns/provider/dnsimple.py +++ b/octodns/provider/dnsimple.py @@ -128,8 +128,8 @@ class DnsimpleProvider(BaseProvider): values = [] for record in records: values.append({ - 'priority': record['priority'], - 'value': '{}.'.format(record['content']) + 'preference': record['priority'], + 'exchange': '{}.'.format(record['content']) }) return { 'ttl': records[0]['ttl'], @@ -290,9 +290,9 @@ class DnsimpleProvider(BaseProvider): def _params_for_MX(self, record): for value in record.values: yield { - 'content': value.value, + 'content': value.exchange, 'name': record.name, - 'priority': value.priority, + 'priority': value.preference, 'ttl': record.ttl, 'type': record._type } diff --git a/octodns/provider/dyn.py b/octodns/provider/dyn.py index 5e0e1f3..e21b93e 100644 --- a/octodns/provider/dyn.py +++ b/octodns/provider/dyn.py @@ -206,7 +206,7 @@ class DynProvider(BaseProvider): return { 'type': _type, 'ttl': records[0].ttl, - 'values': [{'priority': r.preference, 'value': r.exchange} + 'values': [{'preference': r.preference, 'exchange': r.exchange} for r in records], } @@ -400,8 +400,8 @@ class DynProvider(BaseProvider): def _kwargs_for_MX(self, record): return [{ - 'preference': v.priority, - 'exchange': v.value, + 'preference': v.preference, + 'exchange': v.exchange, 'ttl': record.ttl, } for v in record.values] diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py index 33fb19c..2f0a024 100644 --- a/octodns/provider/ns1.py +++ b/octodns/provider/ns1.py @@ -57,10 +57,10 @@ class Ns1Provider(BaseProvider): def _data_for_MX(self, _type, record): values = [] for answer in record['short_answers']: - priority, value = answer.split(' ', 1) + preference, exchange = answer.split(' ', 1) values.append({ - 'priority': priority, - 'value': value, + 'preference': preference, + 'exchange': exchange, }) return { 'ttl': record['ttl'], @@ -150,7 +150,7 @@ class Ns1Provider(BaseProvider): _params_for_PTR = _params_for_CNAME def _params_for_MX(self, record): - values = [(v.priority, v.value) for v in record.values] + values = [(v.preference, v.exchange) for v in record.values] return {'answers': values, 'ttl': record.ttl} def _params_for_NAPTR(self, record): diff --git a/octodns/provider/powerdns.py b/octodns/provider/powerdns.py index d8cccae..c6d11b0 100644 --- a/octodns/provider/powerdns.py +++ b/octodns/provider/powerdns.py @@ -83,10 +83,10 @@ class PowerDnsBaseProvider(BaseProvider): def _data_for_MX(self, rrset): values = [] for record in rrset['records']: - priority, value = record['content'].split(' ', 1) + preference, exchange = record['content'].split(' ', 1) values.append({ - 'priority': priority, - 'value': value, + 'preference': preference, + 'exchange': exchange, }) return { 'type': rrset['type'], @@ -208,7 +208,7 @@ class PowerDnsBaseProvider(BaseProvider): def _records_for_MX(self, record): return [{ - 'content': '{} {}'.format(v.priority, v.value), + 'content': '{} {}'.format(v.preference, v.exchange), 'disabled': False } for v in record.values] diff --git a/octodns/provider/route53.py b/octodns/provider/route53.py index 3875bd6..b4fcab8 100644 --- a/octodns/provider/route53.py +++ b/octodns/provider/route53.py @@ -96,7 +96,8 @@ class _Route53Record(object): _values_for_PTR = _values_for_value def _values_for_MX(self, record): - return ['{} {}'.format(v.priority, v.value) for v in record.values] + return ['{} {}'.format(v.preference, v.exchange) + for v in record.values] def _values_for_NAPTR(self, record): return ['{} {} "{}" "{}" "{}" {}' @@ -335,10 +336,10 @@ class Route53Provider(BaseProvider): def _data_for_MX(self, rrset): values = [] for rr in rrset['ResourceRecords']: - priority, value = rr['Value'].split(' ') + preference, exchange = rr['Value'].split(' ') values.append({ - 'priority': priority, - 'value': value, + 'preference': preference, + 'exchange': exchange, }) return { 'type': rrset['Type'], diff --git a/octodns/record.py b/octodns/record.py index 11876f5..0388911 100644 --- a/octodns/record.py +++ b/octodns/record.py @@ -424,32 +424,45 @@ class MxValue(object): @classmethod def _validate_value(cls, value): reasons = [] - if 'priority' not in value: - reasons.append('missing priority') - if 'value' not in value: - reasons.append('missing value') + if 'preference' not in value and 'priority' not in value: + reasons.append('missing 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') return reasons def __init__(self, value): - # TODO: rename preference - self.priority = int(value['priority']) - # TODO: rename to exchange? - self.value = value['value'].lower() + # RFC1035 says preference, half the providers use priority + try: + preference = value['preference'] + except KeyError: + preference = value['priority'] + self.preference = int(preference) + # UNTIL 1.0 remove value fallback + try: + exchange = value['exchange'] + except KeyError: + exchange = value['value'] + self.exchange = exchange @property def data(self): return { - 'priority': self.priority, - 'value': self.value, + 'preference': self.preference, + 'exchange': self.exchange, } def __cmp__(self, other): - if self.priority == other.priority: - return cmp(self.value, other.value) - return cmp(self.priority, other.priority) + if self.preference == other.preference: + return cmp(self.exchange, other.exchange) + return cmp(self.preference, other.preference) def __repr__(self): - return "'{} {}'".format(self.priority, self.value) + return "'{} {}'".format(self.preference, self.exchange) class MxRecord(_ValuesMixin, Record): diff --git a/octodns/source/tinydns.py b/octodns/source/tinydns.py index 1b98092..63cafa2 100644 --- a/octodns/source/tinydns.py +++ b/octodns/source/tinydns.py @@ -65,8 +65,8 @@ class TinyDnsBaseSource(BaseSource): 'ttl': ttl, 'type': _type, 'values': [{ - 'priority': r[1], - 'value': '{}.'.format(r[0]) + 'preference': r[1], + 'exchange': '{}.'.format(r[0]) } for r in records] } diff --git a/tests/config/unit.tests.yaml b/tests/config/unit.tests.yaml index d18bf59..8be1614 100644 --- a/tests/config/unit.tests.yaml +++ b/tests/config/unit.tests.yaml @@ -60,12 +60,12 @@ mx: ttl: 300 type: MX values: - - priority: 40 - value: smtp-1.unit.tests. - - priority: 20 - value: smtp-2.unit.tests. - - priority: 30 - value: smtp-3.unit.tests. + - exchange: smtp-1.unit.tests. + preference: 40 + - exchange: smtp-2.unit.tests. + preference: 20 + - exchange: smtp-3.unit.tests. + preference: 30 - priority: 10 value: smtp-4.unit.tests. naptr: diff --git a/tests/test_octodns_provider_dyn.py b/tests/test_octodns_provider_dyn.py index 307e640..bebd3e3 100644 --- a/tests/test_octodns_provider_dyn.py +++ b/tests/test_octodns_provider_dyn.py @@ -46,11 +46,11 @@ class TestDynProvider(TestCase): 'type': 'MX', 'ttl': 302, 'values': [{ - 'priority': 10, - 'value': 'smtp-1.unit.tests.' + 'preference': 10, + 'exchange': 'smtp-1.unit.tests.' }, { - 'priority': 20, - 'value': 'smtp-2.unit.tests.' + 'preference': 20, + 'exchange': 'smtp-2.unit.tests.' }] }), ('naptr', { diff --git a/tests/test_octodns_provider_ns1.py b/tests/test_octodns_provider_ns1.py index acb0125..ecc107c 100644 --- a/tests/test_octodns_provider_ns1.py +++ b/tests/test_octodns_provider_ns1.py @@ -44,11 +44,11 @@ class TestNs1Provider(TestCase): 'ttl': 35, 'type': 'MX', 'values': [{ - 'priority': 10, - 'value': 'mx1.unit.tests.', + 'preference': 10, + 'exchange': 'mx1.unit.tests.', }, { - 'priority': 20, - 'value': 'mx2.unit.tests.', + 'preference': 20, + 'exchange': 'mx2.unit.tests.', }] })) expected.add(Record.new(zone, 'naptr', { diff --git a/tests/test_octodns_provider_route53.py b/tests/test_octodns_provider_route53.py index 8960088..cad58f8 100644 --- a/tests/test_octodns_provider_route53.py +++ b/tests/test_octodns_provider_route53.py @@ -52,11 +52,11 @@ class TestRoute53Provider(TestCase): 'Goodbye World?']}), ('', {'ttl': 64, 'type': 'MX', 'values': [{ - 'priority': 10, - 'value': 'smtp-1.unit.tests.', + 'preference': 10, + 'exchange': 'smtp-1.unit.tests.', }, { - 'priority': 20, - 'value': 'smtp-2.unit.tests.', + 'preference': 20, + 'exchange': 'smtp-2.unit.tests.', }]}), ('naptr', {'ttl': 65, 'type': 'NAPTR', 'value': { @@ -1262,8 +1262,8 @@ class TestRoute53Records(TestCase): d = _Route53Record(None, Record.new(existing, '', {'ttl': 42, 'type': 'MX', 'value': { - 'priority': 10, - 'value': 'foo.bar.'}}), + 'preference': 10, + 'exchange': 'foo.bar.'}}), False) self.assertEquals(d, d) diff --git a/tests/test_octodns_record.py b/tests/test_octodns_record.py index 96a83a0..cb87c70 100644 --- a/tests/test_octodns_record.py +++ b/tests/test_octodns_record.py @@ -212,45 +212,49 @@ class TestRecord(TestCase): def test_mx(self): a_values = [{ - 'priority': 10, - 'value': 'smtp1' + 'preference': 10, + 'exchange': 'smtp1.' }, { 'priority': 20, - 'value': 'smtp2' + 'value': 'smtp2.' }] a_data = {'ttl': 30, 'values': a_values} a = MxRecord(self.zone, 'a', a_data) self.assertEquals('a', a.name) self.assertEquals('a.unit.tests.', a.fqdn) self.assertEquals(30, a.ttl) - self.assertEquals(a_values[0]['priority'], a.values[0].priority) - self.assertEquals(a_values[0]['value'], a.values[0].value) - self.assertEquals(a_values[1]['priority'], a.values[1].priority) - self.assertEquals(a_values[1]['value'], a.values[1].value) + self.assertEquals(a_values[0]['preference'], a.values[0].preference) + self.assertEquals(a_values[0]['exchange'], a.values[0].exchange) + self.assertEquals(a_values[1]['priority'], a.values[1].preference) + self.assertEquals(a_values[1]['value'], a.values[1].exchange) + a_data['values'][1] = { + 'preference': 20, + 'exchange': 'smtp2.', + } self.assertEquals(a_data, a.data) b_value = { - 'priority': 12, - 'value': 'smtp3', + 'preference': 12, + 'exchange': 'smtp3.', } b_data = {'ttl': 30, 'value': b_value} b = MxRecord(self.zone, 'b', b_data) - self.assertEquals(b_value['priority'], b.values[0].priority) - self.assertEquals(b_value['value'], b.values[0].value) + self.assertEquals(b_value['preference'], b.values[0].preference) + self.assertEquals(b_value['exchange'], b.values[0].exchange) self.assertEquals(b_data, b.data) target = SimpleProvider() # No changes with self self.assertFalse(a.changes(a, target)) - # Diff in priority causes change + # Diff in preference causes change other = MxRecord(self.zone, 'a', {'ttl': 30, 'values': a_values}) - other.values[0].priority = 22 + other.values[0].preference = 22 change = a.changes(other, target) self.assertEqual(change.existing, a) self.assertEqual(change.new, other) # Diff in value causes change - other.values[0].priority = a.values[0].priority - other.values[0].value = 'smtpX' + other.values[0].preference = a.values[0].preference + other.values[0].exchange = 'smtpX' change = a.changes(other, target) self.assertEqual(change.existing, a) self.assertEqual(change.new, other) @@ -889,8 +893,8 @@ class TestRecordValidation(TestCase): 'type': 'MX', 'ttl': 600, 'value': { - 'priority': 10, - 'value': 'foo.bar.com.' + 'preference': 10, + 'exchange': 'foo.bar.com.' } }) @@ -900,10 +904,10 @@ class TestRecordValidation(TestCase): 'type': 'MX', 'ttl': 600, 'value': { - 'value': 'foo.bar.com.' + 'exchange': 'foo.bar.com.' } }) - self.assertEquals(['missing priority'], ctx.exception.reasons) + self.assertEquals(['missing preference'], ctx.exception.reasons) # missing value with self.assertRaises(ValidationError) as ctx: @@ -911,10 +915,22 @@ class TestRecordValidation(TestCase): 'type': 'MX', 'ttl': 600, 'value': { - 'priority': 10, + 'preference': 10, } }) - self.assertEquals(['missing value'], ctx.exception.reasons) + self.assertEquals(['missing exchange'], ctx.exception.reasons) + + # missing trailing . + with self.assertRaises(ValidationError) as ctx: + Record.new(self.zone, '', { + 'type': 'MX', + 'ttl': 600, + 'value': { + 'preference': 10, + 'exchange': 'foo.bar.com' + } + }) + self.assertEquals(['missing trailing .'], ctx.exception.reasons) def test_NXPTR(self): # doesn't blow up diff --git a/tests/test_octodns_source_tinydns.py b/tests/test_octodns_source_tinydns.py index b4cea06..5792b25 100644 --- a/tests/test_octodns_source_tinydns.py +++ b/tests/test_octodns_source_tinydns.py @@ -68,22 +68,22 @@ class TestTinyDnsFileSource(TestCase): 'type': 'MX', 'ttl': 3600, 'values': [{ - 'priority': 10, - 'value': 'smtp-1-host.example.com.', + 'preference': 10, + 'exchange': 'smtp-1-host.example.com.', }, { - 'priority': 20, - 'value': 'smtp-2-host.example.com.', + 'preference': 20, + 'exchange': 'smtp-2-host.example.com.', }] }), ('smtp', { 'type': 'MX', 'ttl': 1800, 'values': [{ - 'priority': 30, - 'value': 'smtp-1-host.example.com.', + 'preference': 30, + 'exchange': 'smtp-1-host.example.com.', }, { - 'priority': 40, - 'value': 'smtp-2-host.example.com.', + 'preference': 40, + 'exchange': 'smtp-2-host.example.com.', }] }), ):