diff --git a/octodns/provider/etc_hosts.py b/octodns/provider/etc_hosts.py index 92fb2be..db84ae4 100644 --- a/octodns/provider/etc_hosts.py +++ b/octodns/provider/etc_hosts.py @@ -5,8 +5,8 @@ from __future__ import absolute_import, division, print_function, \ unicode_literals -from os import makedirs -from os.path import isdir, join +from os import makedirs, path +from os.path import isdir import logging from .base import BaseProvider @@ -66,60 +66,50 @@ class EtcHostsProvider(BaseProvider): if not isdir(self.directory): makedirs(self.directory) - filename = '{}hosts'.format(join(self.directory, desired.name)) + filename = '{}hosts'.format(path.join(self.directory, desired.name)) self.log.info('_apply: filename=%s', filename) with open(filename, 'w') as fh: fh.write('##################################################\n') - fh.write('# octoDNS ') - fh.write(self.id) - fh.write(' ') - fh.write(desired.name) - fh.write('\n') + fh.write('# octoDNS {} {}\n'.format(self.id, desired.name)) fh.write('##################################################\n\n') if values: fh.write('## A & AAAA\n\n') for fqdn, value in sorted(values.items()): if fqdn[0] == '*': fh.write('# ') - fh.write(value) - fh.write('\t') - fh.write(fqdn) - fh.write('\n\n') + fh.write('{}\t{}\n\n'.format(value, fqdn)) if cnames: - fh.write('\n') - fh.write('## CNAME (mapped)\n\n') + fh.write('\n## CNAME (mapped)\n\n') for fqdn, value in sorted(cnames.items()): # Print out a comment of the first level - fh.write('# ') - fh.write(fqdn) - fh.write(' -> ') - fh.write(value) - fh.write('\n') - # No loop protection :-/ + fh.write('# {} -> {}\n'.format(fqdn, value)) + seen = set() while True: + seen.add(value) try: value = values[value] # If we're here we've found the target, print it # and break the loop - fh.write(value) - fh.write('\t') - fh.write(fqdn) - fh.write('\n') + fh.write('{}\t{}\n'.format(value, fqdn)) break except KeyError: # Try and step down one level orig = value value = cnames.get(value, None) # Print out this step - fh.write('# ') - fh.write(orig) - fh.write(' -> ') if value: - fh.write(value) + if value in seen: + # We'd loop here, break it + fh.write('# {} -> {} **loop**\n' + .format(orig, value)) + break + else: + fh.write('# {} -> {}\n' + .format(orig, value)) else: # Don't have anywhere else to go - fh.write('**unknown**') + fh.write('# {} -> **unknown**\n'.format(orig)) break - fh.write('\n') + fh.write('\n') diff --git a/tests/test_octodns_provider_etc_hosts.py b/tests/test_octodns_provider_etc_hosts.py index 9c8f06b..236164d 100644 --- a/tests/test_octodns_provider_etc_hosts.py +++ b/tests/test_octodns_provider_etc_hosts.py @@ -5,7 +5,8 @@ from __future__ import absolute_import, division, print_function, \ unicode_literals -from os.path import dirname, isfile, join +from os import path +from os.path import dirname, isfile from unittest import TestCase from octodns.provider.etc_hosts import EtcHostsProvider @@ -19,7 +20,8 @@ from helpers import TemporaryDirectory class TestEtcHostsProvider(TestCase): def test_provider(self): - source = EtcHostsProvider('test', join(dirname(__file__), 'config')) + source = EtcHostsProvider('test', path.join(dirname(__file__), + 'config')) zone = Zone('unit.tests.', []) @@ -86,8 +88,8 @@ class TestEtcHostsProvider(TestCase): with TemporaryDirectory() as td: # Add some subdirs to make sure that it can create them - directory = join(td.dirname, 'sub', 'dir') - hosts_file = join(directory, 'unit.tests.hosts') + directory = path.join(td.dirname, 'sub', 'dir') + hosts_file = path.join(directory, 'unit.tests.hosts') target = EtcHostsProvider('test', directory) # We add everything @@ -102,7 +104,7 @@ class TestEtcHostsProvider(TestCase): with open(hosts_file) as fh: data = fh.read() # v6 - self.assertTrue('2001:4860:4860::8844\tv6.unit.tests') + self.assertTrue('2001:4860:4860::8844\tv6.unit.tests' in data) # www self.assertTrue('1.1.1.1\twww.unit.tests' in data) # root ALIAS @@ -118,3 +120,60 @@ class TestEtcHostsProvider(TestCase): # second empty run that won't create dirs and overwrites file plan = Plan(zone, zone, [], True) self.assertEquals(0, target.apply(plan)) + + def test_cname_loop(self): + source = EtcHostsProvider('test', path.join(dirname(__file__), + 'config')) + + zone = Zone('unit.tests.', []) + + # We never populate anything, when acting as a source + source.populate(zone, target=source) + self.assertEquals(0, len(zone.records)) + # Same if we're acting as a target + source.populate(zone) + self.assertEquals(0, len(zone.records)) + + record = Record.new(zone, 'start', { + 'ttl': 60, + 'type': 'CNAME', + 'value': 'middle.unit.tests.', + }) + zone.add_record(record) + record = Record.new(zone, 'middle', { + 'ttl': 60, + 'type': 'CNAME', + 'value': 'loop.unit.tests.', + }) + zone.add_record(record) + record = Record.new(zone, 'loop', { + 'ttl': 60, + 'type': 'CNAME', + 'value': 'start.unit.tests.', + }) + zone.add_record(record) + + with TemporaryDirectory() as td: + # Add some subdirs to make sure that it can create them + directory = path.join(td.dirname, 'sub', 'dir') + hosts_file = path.join(directory, 'unit.tests.hosts') + target = EtcHostsProvider('test', directory) + + # We add everything + plan = target.plan(zone) + self.assertEquals(len(zone.records), len(plan.changes)) + self.assertFalse(isfile(hosts_file)) + + # Now actually do it + self.assertEquals(len(zone.records), target.apply(plan)) + self.assertTrue(isfile(hosts_file)) + + with open(hosts_file) as fh: + data = fh.read() + print(data) + self.assertTrue('# loop.unit.tests -> start.unit.tests ' + '**loop**' in data) + self.assertTrue('# middle.unit.tests -> loop.unit.tests ' + '**loop**' in data) + self.assertTrue('# start.unit.tests -> middle.unit.tests ' + '**loop**' in data)