diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f4a66d3..9a5709a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -26,7 +26,7 @@ Here are a few things you can do that will increase the likelihood of your pull * Follow [pep8](https://www.python.org/dev/peps/pep-0008/) -- Write thorough tests. No PRs will be merged without :100:% code coverage. More than that tests should be very thorough and cover as many (edge) cases as possible. We're working with DNS here and bugs can have a major impact so we need to do as much as reasonably possible to ensure quality. While :100:% doesn't even begin to mean there are no bugs, getting there often requires close inspection & a relatively complete understanding of the code. More times than no the endevor will uncover at least minor problems. +- Write thorough tests. No PRs will be merged without :100:% code coverage. More than that tests should be very thorough and cover as many (edge) cases as possible. We're working with DNS here and bugs can have a major impact so we need to do as much as reasonably possible to ensure quality. While :100:% doesn't even begin to mean there are no bugs, getting there often requires close inspection & a relatively complete understanding of the code. More times than not the endeavor will uncover at least minor problems. - Bug fixes require specific tests covering the addressed behavior. diff --git a/README.md b/README.md index c10ff3c..0e63e51 100644 --- a/README.md +++ b/README.md @@ -152,18 +152,25 @@ The above command pulled the existing data out of Route53 and placed the results | [CloudflareProvider](/octodns/provider/cloudflare.py) | A, AAAA, CNAME, MX, NS, SPF, TXT | No | | | [DnsimpleProvider](/octodns/provider/dnsimple.py) | All | No | | | [DynProvider](/octodns/provider/dyn.py) | All | Yes | | +| [Ns1Provider](/octodns/provider/ns1.py) | All | No | | | [PowerDnsProvider](/octodns/provider/powerdns.py) | All | No | | | [Route53](/octodns/provider/route53.py) | A, AAAA, CNAME, MX, NAPTR, NS, PTR, SPF, SRV, TXT | Yes | | | [TinyDNSSource](/octodns/source/tinydns.py) | A, CNAME, MX, NS, PTR | No | read-only | | [YamlProvider](/octodns/provider/yaml.py) | All | Yes | config | +#### Notes + +* ALIAS support varies a lot from provider to provider care should be taken to verify that your needs are met in detail. + * Dyn's UI doesn't allow editing or view of TTL, but the API accepts and stores the value provided, this value does not appear to be used when served + * Dnsimple's uses the configured TTL when serving things through the ALIAS, there's also a secondary TXT record created alongside the ALIAS that octoDNS ignores + ## Custom Sources and Providers You can check out the [source](/octodns/source/) and [provider](/octodns/provider/) directory to see what's currently supported. Sources act as a source of record information. TinyDnsProvider is currently the only OSS source, though we have several others internally that are specific to our environment. These include something to pull host data from [gPanel](https://githubengineering.com/githubs-metal-cloud/) and a similar provider that sources information about our network gear to create both `A` & `PTR` records for their interfaces. Things that might make good OSS sources might include an `ElbSource` that pulls information about [AWS Elastic Load Balancers](https://aws.amazon.com/elasticloadbalancing/) and dynamically creates `CNAME`s for them, or `Ec2Source` that pulls instance information so that records can be created for hosts similar to how our `GPanelProvider` works. An `AxfrSource` could be really interesting as well. Another case where a source may make sense is if you'd like to export data from a legacy service that you have no plans to push changes back into. Most of the things included in OctoDNS are providers, the obvious difference being that they can serve as both sources and targets of data. We'd really like to see this list grow over time so if you use an unsupported provider then PRs are welcome. The existing providers should serve as reasonable examples. Those that have no GeoDNS support are relatively straightforward. Unfortunately most of the APIs involved to do GeoDNS style traffic management are complex and somewhat inconsistent so adding support for that function would be nice, but is optional and best done in a separate pass. -The `class` key in the providers config section can be used to point to arbitrary classes in the python path so internal or 3rd party providers can easily be included with no coordiation beyond getting them into PYTHONPATH, most likely installed into the virtualenv with OctoDNS. +The `class` key in the providers config section can be used to point to arbitrary classes in the python path so internal or 3rd party providers can easily be included with no coordination beyond getting them into PYTHONPATH, most likely installed into the virtualenv with OctoDNS. ## Other Uses diff --git a/octodns/manager.py b/octodns/manager.py index 2545e71..0366685 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -206,6 +206,13 @@ class Manager(object): if eligible_targets: targets = filter(lambda d: d in eligible_targets, targets) + if not targets: + # Don't bother planning (and more importantly populating) zones + # when we don't have any eligible targets, waste of + # time/resources + self.log.info('sync: no eligible targets, skipping') + continue + self.log.info('sync: sources=%s -> targets=%s', sources, targets) try: @@ -273,12 +280,18 @@ class Manager(object): for target, plan in plans: plan.raise_if_unsafe() - if dry_run or config.get('always-dry-run', False): + if dry_run: return 0 total_changes = 0 self.log.debug('sync: applying') + zones = self.config['zones'] for target, plan in plans: + zone_name = plan.existing.name + if zones[zone_name].get('always-dry-run', False): + self.log.info('sync: zone=%s skipping always-dry-run', + zone_name) + continue total_changes += target.apply(plan) self.log.info('sync: %d total changes', total_changes) diff --git a/octodns/provider/dnsimple.py b/octodns/provider/dnsimple.py index 7ed4fe7..763e446 100644 --- a/octodns/provider/dnsimple.py +++ b/octodns/provider/dnsimple.py @@ -120,6 +120,8 @@ class DnsimpleProvider(BaseProvider): 'value': '{}.'.format(record['content']) } + _data_for_ALIAS = _data_for_CNAME + def _data_for_MX(self, _type, records): values = [] for record in records: @@ -238,6 +240,10 @@ class DnsimpleProvider(BaseProvider): _type = record['type'] if _type == 'SOA': continue + elif _type == 'TXT' and record['content'].startswith('ALIAS for'): + # ALIAS has a "ride along" TXT record with 'ALIAS for XXXX', + # we're ignoring it + continue values[record['name']][record['type']].append(record) before = len(zone.records) @@ -273,6 +279,7 @@ class DnsimpleProvider(BaseProvider): 'type': record._type } + _params_for_ALIAS = _params_for_single _params_for_CNAME = _params_for_single _params_for_PTR = _params_for_single @@ -327,8 +334,8 @@ class DnsimpleProvider(BaseProvider): self._client.record_create(new.zone.name[:-1], params) def _apply_Update(self, change): - self._apply_Create(change) self._apply_Delete(change) + self._apply_Create(change) def _apply_Delete(self, change): existing = change.existing diff --git a/octodns/provider/dyn.py b/octodns/provider/dyn.py index 98414a4..ac0e21b 100644 --- a/octodns/provider/dyn.py +++ b/octodns/provider/dyn.py @@ -109,6 +109,7 @@ class DynProvider(BaseProvider): RECORDS_TO_TYPE = { 'a_records': 'A', 'aaaa_records': 'AAAA', + 'alias_records': 'ALIAS', 'cname_records': 'CNAME', 'mx_records': 'MX', 'naptr_records': 'NAPTR', @@ -119,19 +120,7 @@ class DynProvider(BaseProvider): 'srv_records': 'SRV', 'txt_records': 'TXT', } - TYPE_TO_RECORDS = { - 'A': 'a_records', - 'AAAA': 'aaaa_records', - 'CNAME': 'cname_records', - 'MX': 'mx_records', - 'NAPTR': 'naptr_records', - 'NS': 'ns_records', - 'PTR': 'ptr_records', - 'SSHFP': 'sshfp_records', - 'SPF': 'spf_records', - 'SRV': 'srv_records', - 'TXT': 'txt_records', - } + TYPE_TO_RECORDS = {v: k for k, v in RECORDS_TO_TYPE.items()} # https://help.dyn.com/predefined-geotm-regions-groups/ REGION_CODES = { @@ -194,6 +183,15 @@ class DynProvider(BaseProvider): _data_for_AAAA = _data_for_A + def _data_for_ALIAS(self, _type, records): + # See note on ttl in _kwargs_for_ALIAS + record = records[0] + return { + 'type': _type, + 'ttl': record.ttl, + 'value': record.alias + } + def _data_for_CNAME(self, _type, records): record = records[0] return { @@ -385,6 +383,16 @@ class DynProvider(BaseProvider): 'ttl': record.ttl, }] + def _kwargs_for_ALIAS(self, record): + # NOTE: Dyn's UI doesn't allow editing of ALIAS ttl, but the API seems + # to accept and store the values we send it just fine. No clue if they + # do anything with them. I'd assume they just obey the TTL of the + # record that we're pointed at which makes sense. + return [{ + 'alias': record.value, + 'ttl': record.ttl, + }] + def _kwargs_for_MX(self, record): return [{ 'preference': v.priority, diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py new file mode 100644 index 0000000..5a51780 --- /dev/null +++ b/octodns/provider/ns1.py @@ -0,0 +1,207 @@ +# +# +# + +from __future__ import absolute_import, division, print_function, \ + unicode_literals + +from logging import getLogger +from nsone import NSONE +from nsone.rest.errors import ResourceException + +from ..record import Record +from .base import BaseProvider + + +class Ns1Provider(BaseProvider): + ''' + Ns1 provider + + nsone: + class: octodns.provider.ns1.Ns1Provider + api_key: env/NS1_API_KEY + ''' + SUPPORTS_GEO = False + ZONE_NOT_FOUND_MESSAGE = 'server error: zone not found' + + def __init__(self, id, api_key, *args, **kwargs): + self.log = getLogger('Ns1Provider[{}]'.format(id)) + self.log.debug('__init__: id=%s, api_key=***', id) + super(Ns1Provider, self).__init__(id, *args, **kwargs) + self._client = NSONE(apiKey=api_key) + + def supports(self, record): + return record._type != 'SSHFP' + + def _data_for_A(self, _type, record): + return { + 'ttl': record['ttl'], + 'type': _type, + 'values': record['short_answers'], + } + + _data_for_AAAA = _data_for_A + _data_for_SPF = _data_for_A + _data_for_TXT = _data_for_A + + def _data_for_CNAME(self, _type, record): + return { + 'ttl': record['ttl'], + 'type': _type, + 'value': record['short_answers'][0], + } + + _data_for_ALIAS = _data_for_CNAME + _data_for_PTR = _data_for_CNAME + + def _data_for_MX(self, _type, record): + values = [] + for answer in record['short_answers']: + priority, value = answer.split(' ', 1) + values.append({ + 'priority': priority, + 'value': value, + }) + return { + 'ttl': record['ttl'], + 'type': _type, + 'values': values, + } + + def _data_for_NAPTR(self, _type, record): + values = [] + for answer in record['short_answers']: + order, preference, flags, service, regexp, replacement = \ + answer.split(' ', 5) + values.append({ + 'flags': flags, + 'order': order, + 'preference': preference, + 'regexp': regexp, + 'replacement': replacement, + 'service': service, + }) + return { + 'ttl': record['ttl'], + 'type': _type, + 'values': values, + } + + def _data_for_NS(self, _type, record): + return { + 'ttl': record['ttl'], + 'type': _type, + 'values': [a if a.endswith('.') else '{}.'.format(a) + for a in record['short_answers']], + } + + def _data_for_SRV(self, _type, record): + values = [] + for answer in record['short_answers']: + priority, weight, port, target = answer.split(' ', 3) + values.append({ + 'priority': priority, + 'weight': weight, + 'port': port, + 'target': target, + }) + return { + 'ttl': record['ttl'], + 'type': _type, + 'values': values, + } + + def populate(self, zone, target=False): + self.log.debug('populate: name=%s', zone.name) + + try: + nsone_zone = self._client.loadZone(zone.name[:-1]) + records = nsone_zone.data['records'] + except ResourceException as e: + if e.message != self.ZONE_NOT_FOUND_MESSAGE: + raise + records = [] + + before = len(zone.records) + for record in records: + _type = record['type'] + data_for = getattr(self, '_data_for_{}'.format(_type)) + name = zone.hostname_from_fqdn(record['domain']) + record = Record.new(zone, name, data_for(_type, record)) + zone.add_record(record) + + self.log.info('populate: found %s records', + len(zone.records) - before) + + def _params_for_A(self, record): + return {'answers': record.values, 'ttl': record.ttl} + + _params_for_AAAA = _params_for_A + _params_for_NS = _params_for_A + _params_for_SPF = _params_for_A + _params_for_TXT = _params_for_A + + def _params_for_CNAME(self, record): + return {'answers': [record.value], 'ttl': record.ttl} + + _params_for_ALIAS = _params_for_CNAME + _params_for_PTR = _params_for_CNAME + + def _params_for_MX(self, record): + values = [(v.priority, v.value) for v in record.values] + return {'answers': values, 'ttl': record.ttl} + + def _params_for_NAPTR(self, record): + values = [(v.order, v.preference, v.flags, v.service, v.regexp, + v.replacement) for v in record.values] + return {'answers': values, 'ttl': record.ttl} + + def _params_for_SRV(self, record): + values = [(v.priority, v.weight, v.port, v.target) + for v in record.values] + return {'answers': values, 'ttl': record.ttl} + + def _get_name(self, record): + return record.fqdn[:-1] if record.name == '' else record.name + + def _apply_Create(self, nsone_zone, change): + new = change.new + name = self._get_name(new) + _type = new._type + params = getattr(self, '_params_for_{}'.format(_type))(new) + getattr(nsone_zone, 'add_{}'.format(_type))(name, **params) + + def _apply_Update(self, nsone_zone, change): + existing = change.existing + name = self._get_name(existing) + _type = existing._type + record = nsone_zone.loadRecord(name, _type) + new = change.new + params = getattr(self, '_params_for_{}'.format(_type))(new) + record.update(**params) + + def _apply_Delete(self, nsone_zone, change): + existing = change.existing + name = self._get_name(existing) + _type = existing._type + record = nsone_zone.loadRecord(name, _type) + record.delete() + + def _apply(self, plan): + desired = plan.desired + changes = plan.changes + self.log.debug('_apply: zone=%s, len(changes)=%d', desired.name, + len(changes)) + + domain_name = desired.name[:-1] + try: + nsone_zone = self._client.loadZone(domain_name) + except ResourceException as e: + if e.message != self.ZONE_NOT_FOUND_MESSAGE: + raise + self.log.debug('_apply: no matching zone, creating') + nsone_zone = self._client.createZone(domain_name) + + for change in changes: + class_name = change.__class__.__name__ + getattr(self, '_apply_{}'.format(class_name))(nsone_zone, change) diff --git a/octodns/provider/powerdns.py b/octodns/provider/powerdns.py index 0f9190b..4ff2568 100644 --- a/octodns/provider/powerdns.py +++ b/octodns/provider/powerdns.py @@ -64,6 +64,7 @@ class PowerDnsBaseProvider(BaseProvider): 'ttl': rrset['ttl'] } + _data_for_ALIAS = _data_for_single _data_for_CNAME = _data_for_single _data_for_PTR = _data_for_single @@ -191,6 +192,7 @@ class PowerDnsBaseProvider(BaseProvider): def _records_for_single(self, record): return [{'content': record.value, 'disabled': False}] + _records_for_ALIAS = _records_for_single _records_for_CNAME = _records_for_single _records_for_PTR = _records_for_single diff --git a/octodns/provider/route53.py b/octodns/provider/route53.py index c42a394..6b8a31c 100644 --- a/octodns/provider/route53.py +++ b/octodns/provider/route53.py @@ -320,10 +320,13 @@ class Route53Provider(BaseProvider): _data_for_PTR = _data_for_single _data_for_CNAME = _data_for_single + _fix_semicolons = re.compile(r'(? creates for every record in expected @@ -103,8 +104,8 @@ class TestPowerDnsProvider(TestCase): mock.patch(ANY, status_code=201, text=assert_rrsets_callback) plan = provider.plan(expected) - self.assertEquals(len(expected.records), len(plan.changes)) - self.assertEquals(len(expected.records), provider.apply(plan)) + self.assertEquals(expected_n, len(plan.changes)) + self.assertEquals(expected_n, provider.apply(plan)) # Non-existent zone -> creates for every record in expected # OMG this is fucking ugly, probably better to ditch requests_mocks and @@ -121,8 +122,8 @@ class TestPowerDnsProvider(TestCase): mock.post(ANY, status_code=201, text=assert_rrsets_callback) plan = provider.plan(expected) - self.assertEquals(len(expected.records), len(plan.changes)) - self.assertEquals(len(expected.records), provider.apply(plan)) + self.assertEquals(expected_n, len(plan.changes)) + self.assertEquals(expected_n, provider.apply(plan)) with requests_mock() as mock: # get 422's, unknown zone @@ -166,7 +167,7 @@ class TestPowerDnsProvider(TestCase): expected = Zone('unit.tests.', []) source = YamlProvider('test', join(dirname(__file__), 'config')) source.populate(expected) - self.assertEquals(14, len(expected.records)) + self.assertEquals(15, len(expected.records)) # A small change to a single record with requests_mock() as mock: diff --git a/tests/test_octodns_provider_route53.py b/tests/test_octodns_provider_route53.py index 36b3070..5982b74 100644 --- a/tests/test_octodns_provider_route53.py +++ b/tests/test_octodns_provider_route53.py @@ -1212,6 +1212,26 @@ class TestRoute53Provider(TestCase): provider.apply(plan) self.assertTrue('modifications' in ctx.exception.message) + def test_semicolon_fixup(self): + provider = Route53Provider('test', 'abc', '123') + + self.assertEquals({ + 'type': 'TXT', + 'ttl': 30, + 'values': [ + 'abcd\\; ef\\;g', + 'hij\\; klm\\;n', + ], + }, provider._data_for_quoted({ + 'ResourceRecords': [{ + 'Value': '"abcd; ef;g"', + }, { + 'Value': '"hij\\; klm\\;n"', + }], + 'TTL': 30, + 'Type': 'TXT', + })) + class TestRoute53Records(TestCase): diff --git a/tests/test_octodns_provider_yaml.py b/tests/test_octodns_provider_yaml.py index a557bb3..05c5248 100644 --- a/tests/test_octodns_provider_yaml.py +++ b/tests/test_octodns_provider_yaml.py @@ -30,7 +30,7 @@ class TestYamlProvider(TestCase): # without it we see everything source.populate(zone) - self.assertEquals(14, len(zone.records)) + self.assertEquals(15, len(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 diff --git a/tests/test_octodns_record.py b/tests/test_octodns_record.py index 491b278..52505cb 100644 --- a/tests/test_octodns_record.py +++ b/tests/test_octodns_record.py @@ -7,9 +7,9 @@ from __future__ import absolute_import, division, print_function, \ from unittest import TestCase -from octodns.record import ARecord, AaaaRecord, CnameRecord, Create, Delete, \ - GeoValue, MxRecord, NaptrRecord, NaptrValue, NsRecord, PtrRecord, Record, \ - SshfpRecord, SpfRecord, SrvRecord, TxtRecord, Update +from octodns.record import ARecord, AaaaRecord, AliasRecord, CnameRecord, \ + Create, Delete, GeoValue, MxRecord, NaptrRecord, NaptrValue, NsRecord, \ + PtrRecord, Record, SshfpRecord, SpfRecord, SrvRecord, TxtRecord, Update from octodns.zone import Zone from helpers import GeoProvider, SimpleProvider @@ -242,6 +242,37 @@ class TestRecord(TestCase): # __repr__ doesn't blow up a.__repr__() + def test_alias(self): + a_data = {'ttl': 0, 'value': 'www.unit.tests.'} + a = AliasRecord(self.zone, '', a_data) + self.assertEquals('', a.name) + self.assertEquals('unit.tests.', a.fqdn) + self.assertEquals(0, a.ttl) + self.assertEquals(a_data['value'], a.value) + self.assertEquals(a_data, a.data) + + # missing value + with self.assertRaises(Exception) as ctx: + AliasRecord(self.zone, None, {'ttl': 0}) + self.assertTrue('missing value' in ctx.exception.message) + # bad name + with self.assertRaises(Exception) as ctx: + AliasRecord(self.zone, None, {'ttl': 0, 'value': 'www.unit.tests'}) + self.assertTrue('missing trailing .' in ctx.exception.message) + + target = SimpleProvider() + # No changes with self + self.assertFalse(a.changes(a, target)) + # Diff in value causes change + other = AliasRecord(self.zone, 'a', a_data) + other.value = 'foo.unit.tests.' + change = a.changes(other, target) + self.assertEqual(change.existing, a) + self.assertEqual(change.new, other) + + # __repr__ doesn't blow up + a.__repr__() + def test_cname(self): self.assertSingleValue(CnameRecord, 'target.foo.com.', 'other.foo.com.') diff --git a/tests/test_octodns_zone.py b/tests/test_octodns_zone.py index da83dfc..88bbb68 100644 --- a/tests/test_octodns_zone.py +++ b/tests/test_octodns_zone.py @@ -172,3 +172,36 @@ class TestZone(TestCase): with self.assertRaises(SubzoneRecordException) as ctx: zone.add_record(record) self.assertTrue('under a managed sub-zone', ctx.exception.message) + + def test_ignored_records(self): + zone_normal = Zone('unit.tests.', []) + zone_ignored = Zone('unit.tests.', []) + zone_missing = Zone('unit.tests.', []) + + normal = Record.new(zone_normal, 'www', { + 'ttl': 60, + 'type': 'A', + 'value': '9.9.9.9', + }) + zone_normal.add_record(normal) + + ignored = Record.new(zone_ignored, 'www', { + 'octodns': { + 'ignored': True + }, + 'ttl': 60, + 'type': 'A', + 'value': '9.9.9.9', + }) + zone_ignored.add_record(ignored) + + provider = SimpleProvider() + + self.assertFalse(zone_normal.changes(zone_ignored, provider)) + self.assertTrue(zone_normal.changes(zone_missing, provider)) + + self.assertFalse(zone_ignored.changes(zone_normal, provider)) + self.assertFalse(zone_ignored.changes(zone_missing, provider)) + + self.assertTrue(zone_missing.changes(zone_normal, provider)) + self.assertFalse(zone_missing.changes(zone_ignored, provider))