diff --git a/octodns/provider/yaml.py b/octodns/provider/yaml.py index b5fd3d7..1a2a512 100644 --- a/octodns/provider/yaml.py +++ b/octodns/provider/yaml.py @@ -73,6 +73,12 @@ class YamlProvider(BaseProvider): # (optional, default False) disable_zonefile: false + Note + ---- + + When using this provider as a target any existing comments or formatting + in the zone files will be lost when changes are applyed. + Split Details ------------- @@ -342,11 +348,6 @@ class YamlProvider(BaseProvider): lenient, ) - if target: - # When acting as a target we ignore any existing records so that we - # create a completely new copy - return False - before = len(zone.records) sources = [] @@ -363,33 +364,37 @@ class YamlProvider(BaseProvider): if self.shared_filename: sources.append(join(self.directory, self.shared_filename)) - if not sources: + if not sources and not target: raise ProviderException(f'no YAMLs found for {zone.decoded_name}') - # determinstically order our sources + # deterministically order our sources sources.sort() for source in sources: self._populate_from_file(source, zone, lenient) + exists = len(sources) > 0 self.log.info( - 'populate: found %s records, exists=False', + 'populate: found %s records, exists=%s', len(zone.records) - before, + exists, ) - return False + return exists def _apply(self, plan): - desired = plan.desired + # make a copy of existing we can muck with + copy = plan.existing.copy() changes = plan.changes self.log.debug( - '_apply: zone=%s, len(changes)=%d', - desired.decoded_name, - len(changes), + '_apply: zone=%s, len(changes)=%d', copy.decoded_name, len(changes) ) - # Since we don't have existing we'll only see creates - records = [c.new for c in changes] - # Order things alphabetically (records sort that way - records.sort() + + # apply our pending changes to that copy + copy.apply(changes) + + # we now have the records we need to write out, order things + # alphabetically (records sort that way + records = sorted(copy.records) data = defaultdict(list) for record in records: d = record.data @@ -411,7 +416,7 @@ class YamlProvider(BaseProvider): if self.split_extension: # we're going to do split files - decoded_name = desired.decoded_name[:-1] + decoded_name = copy.decoded_name[:-1] directory = join( self.directory, f'{decoded_name}{self.split_extension}' ) @@ -443,7 +448,7 @@ class YamlProvider(BaseProvider): else: # single large file - filename = join(self.directory, f'{desired.decoded_name}yaml') + filename = join(self.directory, f'{copy.decoded_name}yaml') self.log.debug('_apply: writing filename=%s', filename) with open(filename, 'w') as fh: safe_dump( diff --git a/octodns/zone.py b/octodns/zone.py index 5f23267..a94cda9 100644 --- a/octodns/zone.py +++ b/octodns/zone.py @@ -340,6 +340,16 @@ class Zone(object): return changes + def apply(self, changes): + ''' + Apply the provided changes to the zone. + ''' + for change in changes: + if isinstance(change, Delete): + self.remove_record(change.existing) + else: + self.add_record(change.new, replace=True, lenient=True) + def hydrate(self): ''' Take a shallow copy Zone and make it a deeper copy holding its own diff --git a/tests/test_octodns_manager.py b/tests/test_octodns_manager.py index 271268d..76e5027 100644 --- a/tests/test_octodns_manager.py +++ b/tests/test_octodns_manager.py @@ -2,7 +2,7 @@ # # -from os import environ, listdir +from os import environ, listdir, remove from os.path import dirname, isfile, join from unittest import TestCase from unittest.mock import MagicMock, patch @@ -39,6 +39,13 @@ def get_config_filename(which): return join(config_dir, which) +def reset(directory): + for filename in listdir(directory): + if filename.endswith('.yaml'): + filename = join(directory, filename) + remove(filename) + + class TestManager(TestCase): def test_missing_provider_class(self): with self.assertRaises(ManagerException) as ctx: @@ -147,40 +154,47 @@ class TestManager(TestCase): with TemporaryDirectory() as tmpdir: environ['YAML_TMP_DIR'] = tmpdir.dirname environ['YAML_TMP_DIR2'] = tmpdir.dirname + tc = Manager(get_config_filename('simple.yaml')).sync(dry_run=False) self.assertEqual(28, tc) # try with just one of the zones + reset(tmpdir.dirname) tc = Manager(get_config_filename('simple.yaml')).sync( dry_run=False, eligible_zones=['unit.tests.'] ) self.assertEqual(22, tc) # the subzone, with 2 targets + reset(tmpdir.dirname) tc = Manager(get_config_filename('simple.yaml')).sync( dry_run=False, eligible_zones=['subzone.unit.tests.'] ) self.assertEqual(6, tc) # and finally the empty zone + reset(tmpdir.dirname) tc = Manager(get_config_filename('simple.yaml')).sync( dry_run=False, eligible_zones=['empty.'] ) self.assertEqual(0, tc) # Again with force + reset(tmpdir.dirname) tc = Manager(get_config_filename('simple.yaml')).sync( dry_run=False, force=True ) self.assertEqual(28, tc) # Again with max_workers = 1 + reset(tmpdir.dirname) tc = Manager( get_config_filename('simple.yaml'), max_workers=1 ).sync(dry_run=False, force=True) self.assertEqual(28, tc) # Include meta + reset(tmpdir.dirname) tc = Manager( get_config_filename('simple.yaml'), max_workers=1, @@ -1011,22 +1025,23 @@ class TestManager(TestCase): ) def test_auto_arpa(self): - manager = Manager(get_config_filename('simple-arpa.yaml')) + with TemporaryDirectory() as tmpdir: + environ['YAML_TMP_DIR'] = tmpdir.dirname - # provider config - self.assertEqual( - True, manager.providers.get("auto-arpa").populate_should_replace - ) - self.assertEqual(1800, manager.providers.get("auto-arpa").ttl) + manager = Manager(get_config_filename('simple-arpa.yaml')) - # processor config - self.assertEqual( - True, manager.processors.get("auto-arpa").populate_should_replace - ) - self.assertEqual(1800, manager.processors.get("auto-arpa").ttl) + # provider config + self.assertEqual( + True, manager.providers.get("auto-arpa").populate_should_replace + ) + self.assertEqual(1800, manager.providers.get("auto-arpa").ttl) - with TemporaryDirectory() as tmpdir: - environ['YAML_TMP_DIR'] = tmpdir.dirname + # processor config + self.assertEqual( + True, + manager.processors.get("auto-arpa").populate_should_replace, + ) + self.assertEqual(1800, manager.processors.get("auto-arpa").ttl) # we can sync eligible_zones so long as they're not arpa tc = manager.sync(dry_run=False, eligible_zones=['unit.tests.']) @@ -1043,6 +1058,7 @@ class TestManager(TestCase): ) # same for eligible_sources + reset(tmpdir.dirname) tc = manager.sync( dry_run=False, eligible_zones=['unit.tests.'], @@ -1058,6 +1074,7 @@ class TestManager(TestCase): ) # same for eligible_targets + reset(tmpdir.dirname) tc = manager.sync( dry_run=False, eligible_zones=['unit.tests.'], @@ -1073,10 +1090,11 @@ class TestManager(TestCase): ) # full sync with arpa is fine, 2 extra records from it + reset(tmpdir.dirname) tc = manager.sync(dry_run=False) self.assertEqual(26, tc) - def test_dynamic_config(self): + def test_dynamic_config_targeted(self): with TemporaryDirectory() as tmpdir: environ['YAML_TMP_DIR'] = tmpdir.dirname @@ -1100,6 +1118,12 @@ class TestManager(TestCase): ), ) + def test_dynamic_config_all(self): + with TemporaryDirectory() as tmpdir: + environ['YAML_TMP_DIR'] = tmpdir.dirname + + manager = Manager(get_config_filename('dynamic-config.yaml')) + # should sync everything across all zones, total of 32 records self.assertEqual(32, manager.sync(dry_run=False)) diff --git a/tests/test_octodns_provider_yaml.py b/tests/test_octodns_provider_yaml.py index 06a6466..77a4f1e 100644 --- a/tests/test_octodns_provider_yaml.py +++ b/tests/test_octodns_provider_yaml.py @@ -29,11 +29,12 @@ class TestYamlProvider(TestCase): zone = Zone('unit.tests.', []) dynamic_zone = Zone('dynamic.tests.', []) - # With target we don't add anything + # With target we see everything source.populate(zone, target=source) - self.assertEqual(0, len(zone.records)) + self.assertEqual(25, len(zone.records)) # without it we see everything + zone = Zone('unit.tests.', []) source.populate(zone) self.assertEqual(25, len(zone.records)) @@ -95,11 +96,9 @@ class TestYamlProvider(TestCase): self.assertFalse(zone.changes(reloaded, target=source)) - # A 2nd sync should still create everything + # A 2nd sync should result in no changes, thus no plan plan = target.plan(zone) - self.assertEqual( - 22, len([c for c in plan.changes if isinstance(c, Create)]) - ) + self.assertFalse(plan) with open(yaml_file) as fh: data = safe_load(fh.read()) @@ -177,7 +176,8 @@ class TestYamlProvider(TestCase): zone = Zone(idna_encode(name), []) # create a idna named file - with open(join(td.dirname, idna_encode(filename)), 'w') as fh: + idna_filename = join(td.dirname, idna_encode(filename)) + with open(idna_filename, 'w') as fh: fh.write( '''--- '': @@ -204,7 +204,14 @@ xn--dj-kia8a: self.assertEqual(['2.3.4.5'], d['xn--dj-kia8a'].values) self.assertEqual(['3.4.5.6'], d['xn--28jm5b5a8k5k8cra'].values) - # create a utf8 named file (provider always writes utf-8 filenames + # if we plan there'll be nothing to do + plan = provider.plan(zone) + self.assertFalse(plan) + + # get rid of the idna file + remove(idna_filename) + # create a utf8 named file (provider always writes utf-8 filenames, + # no file should have a plan now plan = provider.plan(zone) provider.apply(plan) @@ -214,6 +221,9 @@ xn--dj-kia8a: self.assertTrue('déjà:' in content) self.assertTrue('これはテストです:' in content) + # recreate the idna version of the file + with open(idna_filename, 'w') as fh: + fh.write('') # does not allow both idna and utf8 named files with self.assertRaises(ProviderException) as ctx: provider.populate(zone) @@ -495,11 +505,12 @@ class TestSplitYamlProvider(TestCase): zone = Zone('unit.tests.', []) dynamic_zone = Zone('dynamic.tests.', []) - # With target we don't add anything + # With target we still see whatever is in the file source.populate(zone, target=source) - self.assertEqual(0, len(zone.records)) + self.assertEqual(20, len(zone.records)) - # without it we see everything + # without it we see everything too, doesn't make a difference + zone = Zone('unit.tests.', []) source.populate(zone) self.assertEqual(20, len(zone.records)) self.assertFalse([r for r in zone.records if r.name.startswith('only')]) @@ -586,11 +597,9 @@ class TestSplitYamlProvider(TestCase): self.assertFalse(zone.changes(reloaded, target=source)) - # A 2nd sync should still create everything + # A 2nd sync should have nothing to do plan = target.plan(zone) - self.assertEqual( - 17, len([c for c in plan.changes if isinstance(c, Create)]) - ) + self.assertFalse(plan) yaml_file = join(zone_dir, '$unit.tests.yaml') self.assertTrue(isfile(yaml_file)) diff --git a/tests/test_octodns_zone.py b/tests/test_octodns_zone.py index 4008210..16789d2 100644 --- a/tests/test_octodns_zone.py +++ b/tests/test_octodns_zone.py @@ -179,6 +179,9 @@ class TestZone(TestCase): # before == after -> no changes self.assertFalse(before.changes(after, target)) + copy = before.copy() + copy.apply([]) + self.assertEqual(copy.records, before.records) # add a record, delete a record -> [Delete, Create] c = ARecord(before, 'c', {'ttl': 42, 'value': '1.1.1.1'}) @@ -199,6 +202,15 @@ class TestZone(TestCase): delete.__repr__() create.__repr__() + # make a copy of before + copy = before.copy() + # apply the changes to it + copy.apply(changes) + # copy should not match it's origin any longer + self.assertNotEqual(copy.records, before.records) + # and it should now match the target + self.assertEqual(copy.records, after.records) + after = Zone('unit.tests.', []) changed = ARecord(before, 'a', {'ttl': 42, 'value': '2.2.2.2'}) after.add_record(changed)