Browse Source

Merge pull request #761 from octodns/zone-shallow-copy

Zone shallow copy support and usage
pull/767/head
Ross McFarland 4 years ago
committed by GitHub
parent
commit
4c9f0137a1
No known key found for this signature in database GPG Key ID: 4AEE18F83AFDEB23
15 changed files with 262 additions and 78 deletions
  1. +10
    -1
      CHANGELOG.md
  2. +26
    -0
      README.md
  3. +8
    -11
      octodns/processor/acme.py
  4. +48
    -9
      octodns/processor/base.py
  5. +6
    -8
      octodns/processor/filter.py
  6. +5
    -8
      octodns/processor/ownership.py
  7. +4
    -0
      octodns/provider/__init__.py
  8. +22
    -14
      octodns/provider/base.py
  9. +5
    -7
      octodns/provider/route53.py
  10. +53
    -2
      octodns/zone.py
  11. +3
    -3
      tests/test_octodns_manager.py
  12. +8
    -8
      tests/test_octodns_processor_filter.py
  13. +2
    -2
      tests/test_octodns_processor_ownership.py
  14. +6
    -5
      tests/test_octodns_provider_base.py
  15. +56
    -0
      tests/test_octodns_zone.py

+ 10
- 1
CHANGELOG.md View File

@ -1,7 +1,16 @@
## v0.9.14 - 2021-??-?? - ...
## v0.9.14 - 2021-??-?? - A new supports system
#### Noteworthy changes
* Provider `strict_supports` param added, currently defaults to `false`, along
with Provider._process_desired_zone this forms the foundations of a new
"supports" system where providers will warn or error (depending on the value
of `strict_supports`) during planning about their inability to do what
they're being asked. When `false` they will warn and "adjust" the desired
records. When true they will abort with an error indicating the problem. Over
time it is expected that all "supports" checking/handling will move into this
paradigm and `strict_supports` will likely be changed to default to `true`.
* Zone shallow copy support, reworking of Processors (alpha) semantics
* NS1 NA target now includes `SX` and `UM`. If `NA` continent is in use in
dynamic records care must be taken to upgrade/downgrade to v0.9.13.
* Ns1Provider now supports a new parameter, shared_notifylist, which results in


+ 26
- 0
README.md View File

@ -228,6 +228,32 @@ The above command pulled the existing data out of Route53 and placed the results
* 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
* octoDNS itself supports non-ASCII character sets, but in testing Cloudflare is the only provider where that is currently functional end-to-end. Others have failures either in the client libraries or API calls
## Compatibilty & Compliance
### `lenient`
`lenient` mostly focuses on the details of `Record`s and standards compliance. When set to `true` octoDNS will allow allow non-compliant configurations & values where possible. For example CNAME values that don't end with a `.`, label length restrictions, and invalid geo codes on `dynamic` records. When in lenient mode octoDNS will log validation problems at `WARNING` and try and continue with the configuration or source data as it exists. See [Lenience](/docs/records.md#lenience) for more information on the concept and how it can be configured.
### `strict_supports` (Work In Progress)
`strict_supports` is a `Provider` level parameter that comes into play when a provider has been asked to create a record that it is unable to support. The simplest case of this would be record type, e.g. `SSHFP` not being supported by `AzureProvider`. If such a record is passed to an `AzureProvider` as a target the provider will take action based on the `strict_supports`. When `true` it will throw an exception saying that it's unable to create the record, when set to `false` it will log at `WARNING` with information about what it's unable to do and how it is attempting to working around it. Other examples of things that cannot be supported would be `dynamic` records on a provider that only supports simple or the lack of support for specific geos in a provider, e.g. Route53Provider does not support `NA-CA-*`.
It is worth noting that these errors will happen during the plan phase of things so that problems will be visible without having to make changes.
This concept is currently a work in progress and only partially implemented. While work is on-going `strict_supports` will default to `false`. Once the work is considered complete & ready the default will change to `true` as it's a much safer and less surprising default as what you configure is what you'll get unless an error is throw telling you why it cannot be done. You will then have the choice to explicitly request that things continue with work-arounds with `strict_supports` set to false`. In the meantime it is encouraged that you manually configure the parameter to `true` in your provider configs.
### Configuring `strict_supports`
The `strict_supports` parameter is available on all providers and can be configured in YAML as follows:
```yaml
providers:
someprovider:
class: whatever.TheProvider
...
strict_supports: true
```
## 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. AxfrSource and TinyDnsFileSource are currently the only OSS sources, 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.


+ 8
- 11
octodns/processor/acme.py View File

@ -32,9 +32,8 @@ class AcmeMangingProcessor(BaseProcessor):
self._owned = set()
def process_source_zone(self, zone, *args, **kwargs):
ret = self._clone_zone(zone)
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)
return ret
desired.add_record(record, replace=True)
return desired
def process_target_zone(self, zone, *args, **kwargs):
ret = self._clone_zone(zone)
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,7 +56,6 @@ class AcmeMangingProcessor(BaseProcessor):
'*octoDNS*' not in record.values and \
record not in self._owned:
self.log.info('_process: ignoring %s', record.fqdn)
continue
ret.add_record(record)
existing.remove_record(record)
return ret
return existing

+ 48
- 9
octodns/processor/base.py View File

@ -5,25 +5,64 @@
from __future__ import absolute_import, division, print_function, \
unicode_literals
from ..zone import Zone
class BaseProcessor(object):
def __init__(self, name):
self.name = name
def _clone_zone(self, zone):
return Zone(zone.name, sub_zones=zone.sub_zones)
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.
def process_source_zone(self, zone, sources):
# sources may be empty, as will be the case for aliased zones
return zone
- 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


+ 6
- 8
octodns/processor/filter.py View File

@ -15,12 +15,11 @@ class TypeAllowlistFilter(BaseProcessor):
self.allowlist = set(allowlist)
def _process(self, zone, *args, **kwargs):
ret = self._clone_zone(zone)
for record in zone.records:
if record._type in self.allowlist:
ret.add_record(record)
if record._type not in self.allowlist:
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 = self._clone_zone(zone)
for record in zone.records:
if record._type not in self.rejectlist:
ret.add_record(record)
if record._type in self.rejectlist:
zone.remove_record(record)
return ret
return zone
process_source_zone = _process
process_target_zone = _process

+ 5
- 8
octodns/processor/ownership.py View File

@ -24,11 +24,8 @@ class OwnershipProcessor(BaseProcessor):
self.txt_value = txt_value
self._txt_values = [txt_value]
def process_source_zone(self, zone, *args, **kwargs):
ret = self._clone_zone(zone)
for record in zone.records:
# Always copy over the source records
ret.add_record(record)
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:
@ -36,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 \


+ 4
- 0
octodns/provider/__init__.py View File

@ -8,3 +8,7 @@ from __future__ import absolute_import, division, print_function, \
class ProviderException(Exception):
pass
class SupportsException(ProviderException):
pass

+ 22
- 14
octodns/provider/base.py View File

@ -10,7 +10,7 @@ from six import text_type
from ..source.base import BaseSource
from ..zone import Zone
from .plan import Plan
from . import ProviderException
from . import SupportsException
class BaseProvider(BaseSource):
@ -34,21 +34,26 @@ class BaseProvider(BaseSource):
def _process_desired_zone(self, desired):
'''
An opportunity for providers to modify that desired zone records before
planning.
- Must do their work and then call super with the results of that work
- Must not modify the `desired` parameter or its records and should
make a copy of anything it's modifying
An opportunity for providers to modify the desired zone records before
planning. `desired` is a "shallow" copy, see `Zone.copy` for more
information
- 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`.
- 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
configuration
provider configuration.
'''
if self.SUPPORTS_MUTLIVALUE_PTR:
# nothing do here
return desired
new_desired = Zone(desired.name, desired.sub_zones)
for record in desired.records:
if record._type == 'PTR' and len(record.values) > 1:
# replace with a single-value copy
@ -59,10 +64,9 @@ class BaseProvider(BaseSource):
self.supports_warn_or_except(msg, fallback)
record = record.copy()
record.values = [record.value]
desired.add_record(record, replace=True)
new_desired.add_record(record)
return new_desired
return desired
def _include_change(self, change):
'''
@ -81,13 +85,17 @@ class BaseProvider(BaseSource):
def supports_warn_or_except(self, msg, fallback):
if self.strict_supports:
raise ProviderException('{}: {}'.format(self.id, msg))
raise SupportsException('{}: {}'.format(self.id, msg))
self.log.warning('{}; {}'.format(msg, fallback))
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)


+ 5
- 7
octodns/provider/route53.py View File

@ -19,7 +19,6 @@ from six import text_type
from ..equality import EqualityTupleMixin
from ..record import Record, Update
from ..record.geo import GeoCodes
from ..zone import Zone
from . import ProviderException
from .base import BaseProvider
@ -927,11 +926,9 @@ class Route53Provider(BaseProvider):
return data
def _process_desired_zone(self, desired):
ret = Zone(desired.name, desired.sub_zones)
for record in desired.records:
if getattr(record, 'dynamic', False):
# Make a copy of the record in case we have to muck with it
record = record.copy()
dynamic = record.dynamic
rules = []
for i, rule in enumerate(dynamic.rules):
@ -958,11 +955,12 @@ class Route53Provider(BaseProvider):
rule.data['geos'] = filtered_geos
rules.append(rule)
dynamic.rules = rules
if rules != dynamic.rules:
record = record.copy()
record.dynamic.rules = rules
desired.add_record(record, replace=True)
ret.add_record(record)
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,


+ 53
- 2
octodns/zone.py View File

@ -49,16 +49,26 @@ class Zone(object):
# optional trailing . b/c some sources don't have it on their fqdn
self._name_re = re.compile(r'\.?{}?$'.format(name))
# Copy-on-write semantics support, when `not None` this property will
# point to a location with records for this `Zone`. Once `hydrated`
# this property will be set to None
self._origin = None
self.log.debug('__init__: zone=%s, sub_zones=%s', self, sub_zones)
@property
def records(self):
if self._origin:
return self._origin.records
return set([r for _, node in self._records.items() for r in node])
def hostname_from_fqdn(self, fqdn):
return self._name_re.sub('', fqdn)
def add_record(self, record, replace=False, lenient=False):
if self._origin:
self.hydrate()
name = record.name
last = name.split('.')[-1]
@ -94,10 +104,14 @@ class Zone(object):
node.add(record)
def _remove_record(self, record):
'Only for use in tests'
def remove_record(self, record):
if self._origin:
self.hydrate()
self._records[record.name].discard(record)
# TODO: delete this
_remove_record = remove_record
def changes(self, desired, target):
self.log.debug('changes: zone=%s, target=%s', self, target)
@ -184,5 +198,42 @@ class Zone(object):
return changes
def hydrate(self):
'''
Take a shallow copy Zone and make it a deeper copy holding its own
reference to records. These records will still be the originals and
they should not be modified. Changes should be made by calling
`add_record`, often with `replace=True`, and/or `remove_record`.
Note: This method does not need to be called under normal circumstances
as `add_record` and `remove_record` will automatically call it when
appropriate.
'''
origin = self._origin
if origin is None:
return False
# Need to clear this before the copy to prevent recursion
self._origin = None
for record in origin.records:
# Use lenient as we're copying origin and should take its records
# regardless
self.add_record(record, lenient=True)
return True
def copy(self):
'''
Copy-on-write semantics support. This method will create a shallow
clone of the zone which will be hydrated the first time `add_record` or
`remove_record` is called.
This allows low-cost copies of things to be made in situations where
changes are unlikely and only incurs the "expense" of actually
copying the records when required. The actual record copy will not be
"deep" meaning that records should not be modified directly.
'''
copy = Zone(self.name, self.sub_zones)
copy._origin = self
return copy
def __repr__(self):
return 'Zone<{}>'.format(self.name)

+ 3
- 3
tests/test_octodns_manager.py View File

@ -464,7 +464,7 @@ class TestManager(TestCase):
class MockProcessor(BaseProcessor):
def process_source_zone(self, zone, sources):
zone = self._clone_zone(zone)
zone = zone.copy()
zone.add_record(record)
return zone
@ -480,7 +480,7 @@ class TestManager(TestCase):
class MockProcessor(BaseProcessor):
def process_target_zone(self, zone, target):
zone = self._clone_zone(zone)
zone = zone.copy()
zone.add_record(record)
return zone
@ -496,7 +496,7 @@ class TestManager(TestCase):
class MockProcessor(BaseProcessor):
def process_target_zone(self, zone, target):
zone = self._clone_zone(zone)
zone = zone.copy()
zone.add_record(record)
return zone


+ 8
- 8
tests/test_octodns_processor_filter.py View File

@ -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]))

+ 2
- 2
tests/test_octodns_processor_ownership.py View File

@ -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


+ 6
- 5
tests/test_octodns_provider_base.py View File

@ -11,7 +11,8 @@ from six import text_type
from unittest import TestCase
from octodns.processor.base import BaseProcessor
from octodns.provider.base import BaseProvider, ProviderException
from octodns.provider import SupportsException
from octodns.provider.base import BaseProvider
from octodns.provider.plan import Plan, UnsafePlan
from octodns.record import Create, Delete, Record, Update
from octodns.zone import Zone
@ -61,11 +62,11 @@ class TrickyProcessor(BaseProcessor):
self.existing = existing
self.target = target
new = self._clone_zone(existing)
new = existing.copy()
for record in existing.records:
new.add_record(record)
new.add_record(record, replace=True)
for record in self.add_during_process_target_zone:
new.add_record(record)
new.add_record(record, replace=True)
return new
@ -465,7 +466,7 @@ class TestBaseProvider(TestCase):
strict = MinimalProvider(strict_supports=True)
# Should log and not expect
with self.assertRaises(ProviderException) as ctx:
with self.assertRaises(SupportsException) as ctx:
strict.supports_warn_or_except('Hello World!', 'Will not see')
self.assertEquals('minimal: Hello World!', text_type(ctx.exception))
strict.log.warning.assert_not_called()

+ 56
- 0
tests/test_octodns_zone.py View File

@ -355,3 +355,59 @@ class TestZone(TestCase):
self.assertTrue(zone_missing.changes(zone_normal, provider))
self.assertFalse(zone_missing.changes(zone_included, provider))
def assertEqualsNameAndValues(self, a, b):
a = dict([(r.name, r.values[0]) for r in a])
b = dict([(r.name, r.values[0]) for r in b])
self.assertEquals(a, b)
def test_copy(self):
zone = Zone('unit.tests.', [])
a = ARecord(zone, 'a', {'ttl': 42, 'value': '1.1.1.1'})
zone.add_record(a)
b = ARecord(zone, 'b', {'ttl': 42, 'value': '1.1.1.2'})
zone.add_record(b)
# Sanity check
self.assertEqualsNameAndValues(set((a, b)), zone.records)
copy = zone.copy()
# We have an origin set and it is the source/original zone
self.assertEquals(zone, copy._origin)
# Our records are zone's records to start (references)
self.assertEqualsNameAndValues(zone.records, copy.records)
# If we try and change something that's already there we realize and
# then get an error about a duplicate
b_prime = ARecord(zone, 'b', {'ttl': 42, 'value': '1.1.1.3'})
with self.assertRaises(DuplicateRecordException):
copy.add_record(b_prime)
self.assertIsNone(copy._origin)
# Unchanged, straight copies
self.assertEqualsNameAndValues(zone.records, copy.records)
# If we add with replace things will be realized and the record will
# have changed
copy = zone.copy()
copy.add_record(b_prime, replace=True)
self.assertIsNone(copy._origin)
self.assertEqualsNameAndValues(set((a, b_prime)), copy.records)
# If we add another record, things are reliazed and it has been added
copy = zone.copy()
c = ARecord(zone, 'c', {'ttl': 42, 'value': '1.1.1.3'})
copy.add_record(c)
self.assertEqualsNameAndValues(set((a, b, c)), copy.records)
# If we remove a record, things are reliazed and it has been removed
copy = zone.copy()
copy.remove_record(a)
self.assertEqualsNameAndValues(set((b,)), copy.records)
# Re-realizing is a noop
copy = zone.copy()
# Happens the first time
self.assertTrue(copy.hydrate())
# Doesn't the second
self.assertFalse(copy.hydrate())

Loading…
Cancel
Save