populating existing provider state is lenient

- adds lenient flag to Record.new, problems during validation are just
  warnings if it's true
- target populate calls during the plan phase pass lenient=True
- make all of the provider.populate call logging consistent including both
  target and lenient
- add source=self to Record.new in a few places that were missing it

octodns/provider/base.py

@@ -104,7 +104,7 @@ class BaseProvider(BaseSource):
         self.log.info('plan: desired=%s', desired.name)
 
         existing = Zone(desired.name, desired.sub_zones)
-        self.populate(existing, target=True)
+        self.populate(existing, target=True, lenient=True)
 
         # compute the changes at the zone/record level
         changes = existing.changes(desired, self)

octodns/provider/cloudflare.py

@@ -154,8 +154,9 @@ class CloudflareProvider(BaseProvider):
 
         return self._zone_records[zone.name]
 
-    def populate(self, zone, target=False):
-        self.log.debug('populate: name=%s', zone.name)
+    def populate(self, zone, target=False, lenient=False):
+        self.log.debug('populate: name=%s, target=%s, lenient=%s', zone.name,
+                       target, lenient)
 
         before = len(zone.records)
         records = self.zone_records(zone)
@@ -171,7 +172,8 @@ class CloudflareProvider(BaseProvider):
                 for _type, records in types.items():
                     data_for = getattr(self, '_data_for_{}'.format(_type))
                     data = data_for(_type, records)
-                    record = Record.new(zone, name, data, source=self)
+                    record = Record.new(zone, name, data, source=self,
+                                        lenient=lenient)
                     zone.add_record(record)
 
         self.log.info('populate:   found %s records',

octodns/provider/dnsimple.py

@@ -234,8 +234,9 @@ class DnsimpleProvider(BaseProvider):
 
         return self._zone_records[zone.name]
 
-    def populate(self, zone, target=False):
-        self.log.debug('populate: name=%s', zone.name)
+    def populate(self, zone, target=False, lenient=False):
+        self.log.debug('populate: name=%s, target=%s, lenient=%s', zone.name,
+                       target, lenient)
 
         values = defaultdict(lambda: defaultdict(list))
         for record in self.zone_records(zone):
@@ -252,7 +253,8 @@ class DnsimpleProvider(BaseProvider):
         for name, types in values.items():
             for _type, records in types.items():
                 data_for = getattr(self, '_data_for_{}'.format(_type))
-                record = Record.new(zone, name, data_for(_type, records))
+                record = Record.new(zone, name, data_for(_type, records),
+                                    source=self, lenient=lenient)
                 zone.add_record(record)
 
         self.log.info('populate:   found %s records',

octodns/provider/dyn.py

@@ -338,8 +338,10 @@ class DynProvider(BaseProvider):
 
         return td_records
 
-    def populate(self, zone, target=False):
-        self.log.info('populate: zone=%s', zone.name)
+    def populate(self, zone, target=False, lenient=False):
+        self.log.debug('populate: name=%s, target=%s, lenient=%s', zone.name,
+                       target, lenient)
+
         before = len(zone.records)
 
         self._check_dyn_sess()
@@ -364,7 +366,8 @@ class DynProvider(BaseProvider):
                 for _type, records in types.items():
                     data_for = getattr(self, '_data_for_{}'.format(_type))
                     data = data_for(_type, records)
-                    record = Record.new(zone, name, data, source=self)
+                    record = Record.new(zone, name, data, source=self,
+                                        lenient=lenient)
                     if record not in td_records:
                         zone.add_record(record)
 

octodns/provider/ns1.py

@@ -111,8 +111,9 @@ class Ns1Provider(BaseProvider):
             'values': values,
         }
 
-    def populate(self, zone, target=False):
-        self.log.debug('populate: name=%s', zone.name)
+    def populate(self, zone, target=False, lenient=False):
+        self.log.debug('populate: name=%s, target=%s, lenient=%s', zone.name,
+                       target, lenient)
 
         try:
             nsone_zone = self._client.loadZone(zone.name[:-1])
@@ -127,7 +128,8 @@ class Ns1Provider(BaseProvider):
             _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))
+            record = Record.new(zone, name, data_for(_type, record),
+                                source=self, lenient=lenient)
             zone.add_record(record)
 
         self.log.info('populate:   found %s records',

octodns/provider/powerdns.py

@@ -146,8 +146,9 @@ class PowerDnsBaseProvider(BaseProvider):
             'ttl': rrset['ttl']
         }
 
-    def populate(self, zone, target=False):
-        self.log.debug('populate: name=%s', zone.name)
+    def populate(self, zone, target=False, lenient=False):
+        self.log.debug('populate: name=%s, target=%s, lenient=%s', zone.name,
+                       target, lenient)
 
         resp = None
         try:
@@ -177,7 +178,7 @@ class PowerDnsBaseProvider(BaseProvider):
                 data_for = getattr(self, '_data_for_{}'.format(_type))
                 record_name = zone.hostname_from_fqdn(rrset['name'])
                 record = Record.new(zone, record_name, data_for(rrset),
-                                    source=self)
+                                    source=self, lenient=lenient)
                 zone.add_record(record)
 
         self.log.info('populate:   found %s records',

octodns/provider/route53.py

@@ -418,8 +418,10 @@ class Route53Provider(BaseProvider):
 
         return self._r53_rrsets[zone_id]
 
-    def populate(self, zone, target=False):
-        self.log.debug('populate: name=%s', zone.name)
+    def populate(self, zone, target=False, lenient=False):
+        self.log.debug('populate: name=%s, target=%s, lenient=%s', zone.name,
+                       target, lenient)
+
         before = len(zone.records)
 
         zone_id = self._get_zone_id(zone.name)
@@ -449,7 +451,8 @@ class Route53Provider(BaseProvider):
                         data['geo'] = geo
                     else:
                         data = data[0]
-                    record = Record.new(zone, name, data, source=self)
+                    record = Record.new(zone, name, data, source=self,
+                                        lenient=lenient)
                     zone.add_record(record)
 
         self.log.info('populate:   found %s records',

octodns/provider/yaml.py

@@ -45,8 +45,10 @@ class YamlProvider(BaseProvider):
         self.default_ttl = default_ttl
         self.enforce_order = enforce_order
 
-    def populate(self, zone, target=False):
-        self.log.debug('populate: zone=%s, target=%s', zone.name, target)
+    def populate(self, zone, target=False, lenient=False):
+        self.log.debug('populate: name=%s, target=%s, lenient=%s', zone.name,
+                       target, lenient)
+
         if target:
             # When acting as a target we ignore any existing records so that we
             # create a completely new copy
@@ -63,7 +65,8 @@ class YamlProvider(BaseProvider):
                     for d in data:
                         if 'ttl' not in d:
                             d['ttl'] = self.default_ttl
-                        record = Record.new(zone, name, d, source=self)
+                        record = Record.new(zone, name, d, source=self,
+                                            lenient=lenient)
                         zone.add_record(record)
 
         self.log.info('populate:   found %s records',

octodns/record.py

@@ -56,10 +56,12 @@ class Delete(Change):
 
 class ValidationError(Exception):
 
+    @classmethod
+    def build_message(cls, fqdn, reasons):
+        return 'Invalid record {}\n  - {}'.format(fqdn, '\n  - '.join(reasons))
+
     def __init__(self, fqdn, reasons):
-        message = 'Invalid record {}\n  - {}' \
-            .format(fqdn, '\n  - '.join(reasons))
-        super(Exception, self).__init__(message)
+        super(Exception, self).__init__(self.build_message(fqdn, reasons))
         self.fqdn = fqdn
         self.reasons = reasons
 
@@ -68,7 +70,7 @@ class Record(object):
     log = getLogger('Record')
 
     @classmethod
-    def new(cls, zone, name, data, source=None):
+    def new(cls, zone, name, data, source=None, lenient=False):
         fqdn = '{}.{}'.format(name, zone.name) if name else zone.name
         try:
             _type = data['type']
@@ -107,7 +109,10 @@ class Record(object):
             raise Exception('Unknown record type: "{}"'.format(_type))
         reasons = _class.validate(name, data)
         if reasons:
-            raise ValidationError(fqdn, reasons)
+            if lenient:
+                cls.log.warn(ValidationError.build_message(fqdn, reasons))
+            else:
+                raise ValidationError(fqdn, reasons)
         return _class(zone, name, data, source=source)
 
     @classmethod

octodns/source/base.py

@@ -23,6 +23,14 @@ class BaseSource(object):
     def populate(self, zone, target=False):
         '''
         Loads all zones the provider knows about
+
+        When `target` is True the populate call is being made to load the
+        current state of the provider.
+
+        When `lenient` is True the populate call may skip record validation and
+        do a "best effort" load of data. That will allow through some common,
+        but not best practices stuff that we otherwise would reject. E.g. no
+        trailing . or mising escapes for ;.
         '''
         raise NotImplementedError('Abstract base class, populate method '
                                   'missing')

octodns/source/tinydns.py

@@ -81,19 +81,21 @@ class TinyDnsBaseSource(BaseSource):
             'values': ['{}.'.format(r[0]) for r in records]
         }
 
-    def populate(self, zone, target=False):
-        self.log.debug('populate: zone=%s', zone.name)
+    def populate(self, zone, target=False, lenient=False):
+        self.log.debug('populate: name=%s, target=%s, lenient=%s', zone.name,
+                       target, lenient)
+
         before = len(zone.records)
 
         if zone.name.endswith('in-addr.arpa.'):
-            self._populate_in_addr_arpa(zone)
+            self._populate_in_addr_arpa(zone, lenient)
         else:
-            self._populate_normal(zone)
+            self._populate_normal(zone, lenient)
 
         self.log.info('populate:   found %s records',
                       len(zone.records) - before)
 
-    def _populate_normal(self, zone):
+    def _populate_normal(self, zone, lenient):
         type_map = {
             '=': 'A',
             '^': None,
@@ -129,14 +131,15 @@ class TinyDnsBaseSource(BaseSource):
                 data_for = getattr(self, '_data_for_{}'.format(_type))
                 data = data_for(_type, d)
                 if data:
-                    record = Record.new(zone, name, data, source=self)
+                    record = Record.new(zone, name, data, source=self,
+                                        lenient=lenient)
                     try:
                         zone.add_record(record)
                     except SubzoneRecordException:
                         self.log.debug('_populate_normal: skipping subzone '
                                        'record=%s', record)
 
-    def _populate_in_addr_arpa(self, zone):
+    def _populate_in_addr_arpa(self, zone, lenient):
         name_re = re.compile('(?P<name>.+)\.{}$'.format(zone.name[:-1]))
 
         for line in self._lines():
@@ -170,7 +173,7 @@ class TinyDnsBaseSource(BaseSource):
                     'ttl': ttl,
                     'type': 'PTR',
                     'value': value
-                }, source=self)
+                }, source=self, lenient=lenient)
                 try:
                     zone.add_record(record)
                 except DuplicateRecordException:

tests/helpers.py

@@ -22,7 +22,7 @@ class SimpleProvider(object):
     def __init__(self, id='test'):
         pass
 
-    def populate(self, zone, source=True):
+    def populate(self, zone, source=False, lenient=False):
         pass
 
     def supports(self, record):
@@ -38,7 +38,7 @@ class GeoProvider(object):
     def __init__(self, id='test'):
         pass
 
-    def populate(self, zone, source=True):
+    def populate(self, zone, source=False, lenient=False):
         pass
 
     def supports(self, record):

tests/test_octodns_provider_base.py

@@ -24,7 +24,7 @@ class HelperProvider(BaseProvider):
         self.apply_disabled = apply_disabled
         self.include_change_callback = include_change_callback
 
-    def populate(self, zone, target=False):
+    def populate(self, zone, target=False, lenient=False):
         pass
 
     def _include_change(self, change):
@@ -72,7 +72,7 @@ class TestBaseProvider(TestCase):
 
         class HasPopulate(HasSupports):
 
-            def populate(self, zone, target=False):
+            def populate(self, zone, target=False, lenient=False):
                 zone.add_record(Record.new(zone, '', {
                     'ttl': 60,
                     'type': 'A',

tests/test_octodns_provider_route53.py

@@ -370,7 +370,7 @@ class TestRoute53Provider(TestCase):
         stubber.assert_no_pending_responses()
 
         # Delete by monkey patching in a populate that includes an extra record
-        def add_extra_populate(existing, target):
+        def add_extra_populate(existing, target, lenient):
             for record in self.expected.records:
                 existing.records.add(record)
             record = Record.new(existing, 'extra',
@@ -406,7 +406,7 @@ class TestRoute53Provider(TestCase):
 
         # Update by monkey patching in a populate that modifies the A record
         # with geos
-        def mod_geo_populate(existing, target):
+        def mod_geo_populate(existing, target, lenient):
             for record in self.expected.records:
                 if record._type != 'A' or not record.geo:
                     existing.records.add(record)
@@ -502,7 +502,7 @@ class TestRoute53Provider(TestCase):
 
         # Update converting to non-geo by monkey patching in a populate that
         # modifies the A record with geos
-        def mod_add_geo_populate(existing, target):
+        def mod_add_geo_populate(existing, target, lenient):
             for record in self.expected.records:
                 if record._type != 'A' or record.geo:
                     existing.records.add(record)

tests/test_octodns_record.py

@@ -643,6 +643,7 @@ class TestRecordValidation(TestCase):
                 'value': '1.2.3.4',
             })
         self.assertEquals(['missing ttl'], ctx.exception.reasons)
+
         # invalid ttl
         with self.assertRaises(ValidationError) as ctx:
             Record.new(self.zone, 'www', {
@@ -653,6 +654,21 @@ class TestRecordValidation(TestCase):
         self.assertEquals('www.unit.tests.', ctx.exception.fqdn)
         self.assertEquals(['invalid ttl'], ctx.exception.reasons)
 
+        # no exception if we're in lenient mode
+        Record.new(self.zone, 'www', {
+            'type': 'A',
+            'ttl': -1,
+            'value': '1.2.3.4',
+        }, lenient=True)
+
+        # __init__ may still blow up, even if validation is lenient
+        with self.assertRaises(KeyError) as ctx:
+            Record.new(self.zone, 'www', {
+                'type': 'A',
+                'ttl': -1,
+            }, lenient=True)
+        self.assertEquals(('value',), ctx.exception.args)
+
     def test_A_and_values_mixin(self):
         # doesn't blow up
         Record.new(self.zone, '', {