Copy zones early on and allow modifications after that. Doc requirements.

octodns/processor/acme.py

@@ -32,9 +32,8 @@ class AcmeMangingProcessor(BaseProcessor):
 
         self._owned = set()
 
-    def process_source_zone(self, zone, *args, **kwargs):
-        ret = zone.copy()
-        for record in zone.records:
+    def process_source_zone(self, desired, *args, **kwargs):
+        for record in desired.records:
             if record._type == 'TXT' and \
                record.name.startswith('_acme-challenge'):
                 # We have a managed acme challenge record (owned by octoDNS) so
@@ -45,12 +44,11 @@ class AcmeMangingProcessor(BaseProcessor):
                 # This assumes we'll see things as sources before targets,
                 # which is the case...
                 self._owned.add(record)
-                ret.add_record(record, replace=True)
-        return ret
+                desired.add_record(record, replace=True)
+        return desired
 
-    def process_target_zone(self, zone, *args, **kwargs):
-        ret = zone.copy()
-        for record in zone.records:
+    def process_target_zone(self, existing, *args, **kwargs):
+        for record in existing.records:
             # Uses a startswith rather than == to ignore subdomain challenges,
             # e.g. _acme-challenge.foo.domain.com when managing domain.com
             if record._type == 'TXT' and \
@@ -58,6 +56,6 @@ class AcmeMangingProcessor(BaseProcessor):
                '*octoDNS*' not in record.values and \
                record not in self._owned:
                 self.log.info('_process: ignoring %s', record.fqdn)
-                ret.remove_record(record)
+                existing.remove_record(record)
 
-        return ret
+        return existing

octodns/processor/base.py

@@ -11,14 +11,58 @@ class BaseProcessor(object):
     def __init__(self, name):
         self.name = name
 
-    def process_source_zone(self, zone, sources):
-        # sources may be empty, as will be the case for aliased zones
-        return zone
+    def process_source_zone(self, desired, sources):
+        '''
+        Called after all sources have completed populate. Provides an
+        opportunity for the processor to modify the desired `Zone` that targets
+        will recieve.
+
+        - Will see `desired` after any modifications done by
+          `Provider._process_desired_zone` and processors configured to run
+          before this one.
+        - May modify `desired` directly.
+        - Must return `desired` which will normally be the `desired` param.
+        - Must not modify records directly, `record.copy` should be called,
+          the results of which can be modified, and then `Zone.add_record` may
+          be used with `replace=True`.
+        - May call `Zone.remove_record` to remove records from `desired`.
+        - Sources may be empty, as will be the case for aliased zones.
+        '''
+        return desired
 
-    def process_target_zone(self, zone, target):
-        return zone
+    def process_target_zone(self, existing, target):
+        '''
+        Called after a target has completed `populate`, before changes are
+        computed between `existing` and `desired`. This provides an opportunity
+        to modify the `existing` `Zone`.
+
+        - Will see `existing` after any modifrications done by processors
+          configured to run before this one.
+        - May modify `existing` directly.
+        - Must return `existing` which will normally be the `existing` param.
+        - Must not modify records directly, `record.copy` should be called,
+          the results of which can be modified, and then `Zone.add_record` may
+          be used with `replace=True`.
+        - May call `Zone.remove_record` to remove records from `existing`.
+        '''
+        return existing
 
     def process_plan(self, plan, sources, target):
+        '''
+        Called after the planning phase has completed. Provides an opportunity
+        for the processors to modify the plan thus changing the actions that
+        will be displayed and potentially applied.
+
+        - `plan` may be None if no changes were detected, if so a `Plan` may
+          still be created and returned.
+        - May modify `plan.changes` directly or create a new `Plan`.
+        - Does not have to modify `plan.desired` and/or `plan.existing` to line
+          up with any modifications made to `plan.changes`.
+        - Should copy over `plan.exists`, `plan.update_pcent_threshold`, and
+          `plan.delete_pcent_threshold` when creating a new `Plan`.
+        - Must return a `Plan` which may be `plan` or can be a newly created
+          one `plan.desired` and `plan.existing` copied over as-is or modified.
+        '''
         # plan may be None if no changes were detected up until now, the
         # process may still create a plan.
         # sources may be empty, as will be the case for aliased zones

octodns/processor/filter.py

@@ -15,12 +15,11 @@ class TypeAllowlistFilter(BaseProcessor):
         self.allowlist = set(allowlist)
 
     def _process(self, zone, *args, **kwargs):
-        ret = zone.copy()
         for record in zone.records:
             if record._type not in self.allowlist:
-                ret.remove_record(record)
+                zone.remove_record(record)
 
-        return ret
+        return zone
 
     process_source_zone = _process
     process_target_zone = _process
@@ -33,12 +32,11 @@ class TypeRejectlistFilter(BaseProcessor):
         self.rejectlist = set(rejectlist)
 
     def _process(self, zone, *args, **kwargs):
-        ret = zone.copy()
         for record in zone.records:
             if record._type in self.rejectlist:
-                ret.remove_record(record)
+                zone.remove_record(record)
 
-        return ret
+        return zone
 
     process_source_zone = _process
     process_target_zone = _process

octodns/processor/ownership.py

@@ -24,9 +24,8 @@ class OwnershipProcessor(BaseProcessor):
         self.txt_value = txt_value
         self._txt_values = [txt_value]
 
-    def process_source_zone(self, zone, *args, **kwargs):
-        ret = zone.copy()
-        for record in zone.records:
+    def process_source_zone(self, desired, *args, **kwargs):
+        for record in desired.records:
             # Then create and add an ownership TXT for each of them
             record_name = record.name.replace('*', '_wildcard')
             if record.name:
@@ -34,14 +33,14 @@ class OwnershipProcessor(BaseProcessor):
                                          record_name)
             else:
                 name = '{}.{}'.format(self.txt_name, record._type)
-            txt = Record.new(zone, name, {
+            txt = Record.new(desired, name, {
                 'type': 'TXT',
                 'ttl': 60,
                 'value': self.txt_value,
             })
-            ret.add_record(txt)
+            desired.add_record(txt)
 
-        return ret
+        return desired
 
     def _is_ownership(self, record):
         return record._type == 'TXT' and \

octodns/provider/base.py

@@ -38,25 +38,22 @@ class BaseProvider(BaseSource):
         planning. `desired` is a "shallow" copy, see `Zone.copy` for more
         information
 
-        - Must do their work and then call `super` with the results of that
-          work, returning the result of the `super` call.
-        - Must not modify `desired` directly, should call `desired.copy` and
-          modify the shallow copy returned from that.
+        - Must call `super` at an appropriate point for their work, generally
+          that means as the final step of the method, returning the result of
+          the `super` call.
+        - May modify `desired` directly.
         - Must not modify records directly, `record.copy` should be called,
           the results of which can be modified, and then `Zone.add_record` may
-          be used with `replace=True`
-        - Must call `Zone.remove_record` to remove records from the copy of
-          `desired`
+          be used with `replace=True`.
+        - May call `Zone.remove_record` to remove records from `desired`.
         - Must call supports_warn_or_except with information about any changes
           that are made to have them logged or throw errors depending on the
-          provider configuration
+          provider configuration.
         '''
         if self.SUPPORTS_MUTLIVALUE_PTR:
             # nothing do here
             return desired
 
-        # Shallow copy
-        new_desired = desired.copy()
         for record in desired.records:
             if record._type == 'PTR' and len(record.values) > 1:
                 # replace with a single-value copy
@@ -67,9 +64,9 @@ class BaseProvider(BaseSource):
                 self.supports_warn_or_except(msg, fallback)
                 record = record.copy()
                 record.values = [record.value]
-                new_desired.add_record(record, replace=True)
+                desired.add_record(record, replace=True)
 
-        return new_desired
+        return desired
 
     def _include_change(self, change):
         '''
@@ -94,7 +91,11 @@ class BaseProvider(BaseSource):
     def plan(self, desired, processors=[]):
         self.log.info('plan: desired=%s', desired.name)
 
-        # process desired zone for any custom zone/record modification
+        # Make a (shallow) copy of the desired state so that everything from
+        # now on (in this target) can modify it as they see fit without
+        # worrying about impacting other targets.
+        desired = desired.copy()
+
         desired = self._process_desired_zone(desired)
 
         existing = Zone(desired.name, desired.sub_zones)

octodns/provider/route53.py

@@ -925,7 +925,6 @@ class Route53Provider(BaseProvider):
         return data
 
     def _process_desired_zone(self, desired):
-        ret = desired.copy()
         for record in desired.records:
             if getattr(record, 'dynamic', False):
                 # Make a copy of the record in case we have to muck with it
@@ -958,9 +957,9 @@ class Route53Provider(BaseProvider):
                 if rules != dynamic.rules:
                     record = record.copy()
                     record.dynamic.rules = rules
-                    ret.add_record(record, replace=True)
+                    desired.add_record(record, replace=True)
 
-        return super(Route53Provider, self)._process_desired_zone(ret)
+        return super(Route53Provider, self)._process_desired_zone(desired)
 
     def populate(self, zone, target=False, lenient=False):
         self.log.debug('populate: name=%s, target=%s, lenient=%s', zone.name,

tests/test_octodns_processor_filter.py

@@ -47,20 +47,20 @@ class TestTypeAllowListFilter(TestCase):
     def test_basics(self):
         filter_a = TypeAllowlistFilter('only-a', set(('A')))
 
-        got = filter_a.process_source_zone(zone)
+        got = filter_a.process_source_zone(zone.copy())
         self.assertEquals(['a', 'a2'], sorted([r.name for r in got.records]))
 
         filter_aaaa = TypeAllowlistFilter('only-aaaa', ('AAAA',))
-        got = filter_aaaa.process_source_zone(zone)
+        got = filter_aaaa.process_source_zone(zone.copy())
         self.assertEquals(['aaaa'], sorted([r.name for r in got.records]))
 
         filter_txt = TypeAllowlistFilter('only-txt', ['TXT'])
-        got = filter_txt.process_target_zone(zone)
+        got = filter_txt.process_target_zone(zone.copy())
         self.assertEquals(['txt', 'txt2'],
                           sorted([r.name for r in got.records]))
 
         filter_a_aaaa = TypeAllowlistFilter('only-aaaa', set(('A', 'AAAA')))
-        got = filter_a_aaaa.process_target_zone(zone)
+        got = filter_a_aaaa.process_target_zone(zone.copy())
         self.assertEquals(['a', 'a2', 'aaaa'],
                           sorted([r.name for r in got.records]))
 
@@ -70,21 +70,21 @@ class TestTypeRejectListFilter(TestCase):
     def test_basics(self):
         filter_a = TypeRejectlistFilter('not-a', set(('A')))
 
-        got = filter_a.process_source_zone(zone)
+        got = filter_a.process_source_zone(zone.copy())
         self.assertEquals(['aaaa', 'txt', 'txt2'],
                           sorted([r.name for r in got.records]))
 
         filter_aaaa = TypeRejectlistFilter('not-aaaa', ('AAAA',))
-        got = filter_aaaa.process_source_zone(zone)
+        got = filter_aaaa.process_source_zone(zone.copy())
         self.assertEquals(['a', 'a2', 'txt', 'txt2'],
                           sorted([r.name for r in got.records]))
 
         filter_txt = TypeRejectlistFilter('not-txt', ['TXT'])
-        got = filter_txt.process_target_zone(zone)
+        got = filter_txt.process_target_zone(zone.copy())
         self.assertEquals(['a', 'a2', 'aaaa'],
                           sorted([r.name for r in got.records]))
 
         filter_a_aaaa = TypeRejectlistFilter('not-a-aaaa', set(('A', 'AAAA')))
-        got = filter_a_aaaa.process_target_zone(zone)
+        got = filter_a_aaaa.process_target_zone(zone.copy())
         self.assertEquals(['txt', 'txt2'],
                           sorted([r.name for r in got.records]))

tests/test_octodns_processor_ownership.py

@@ -55,7 +55,7 @@ class TestOwnershipProcessor(TestCase):
     def test_process_source_zone(self):
         ownership = OwnershipProcessor('ownership')
 
-        got = ownership.process_source_zone(zone)
+        got = ownership.process_source_zone(zone.copy())
         self.assertEquals([
             '',
             '*',
@@ -88,7 +88,7 @@ class TestOwnershipProcessor(TestCase):
         self.assertFalse(ownership.process_plan(None))
 
         # Nothing exists create both records and ownership
-        ownership_added = ownership.process_source_zone(zone)
+        ownership_added = ownership.process_source_zone(zone.copy())
         plan = provider.plan(ownership_added)
         self.assertTrue(plan)
         # Double the number of records