Merge remote-tracking branch 'origin/master' into configurable-geo-healthcheck

CONTRIBUTING.md

@@ -26,7 +26,7 @@ Here are a few things you can do that will increase the likelihood of your pull
 
 * Follow [pep8](https://www.python.org/dev/peps/pep-0008/)
 
-- Write thorough tests. No PRs will be merged without :100:% code coverage. More than that tests should be very thorough and cover as many (edge) cases as possible. We're working with DNS here and bugs can have a major impact so we need to do as much as reasonably possible to ensure quality. While :100:% doesn't even begin to mean there are no bugs, getting there often requires close inspection & a relatively complete understanding of the code. More times than no the endevor will uncover at least minor problems.
+- Write thorough tests. No PRs will be merged without :100:% code coverage. More than that tests should be very thorough and cover as many (edge) cases as possible. We're working with DNS here and bugs can have a major impact so we need to do as much as reasonably possible to ensure quality. While :100:% doesn't even begin to mean there are no bugs, getting there often requires close inspection & a relatively complete understanding of the code. More times than not the endeavor will uncover at least minor problems.
 
 - Bug fixes require specific tests covering the addressed behavior.
 

README.md

@@ -160,7 +160,7 @@ The above command pulled the existing data out of Route53 and placed the results
 
 #### Notes
 
-* ALIAS support varies a lot fromm provider to provider care should be taken to verify that your needs are met in detail.
+* ALIAS support varies a lot from provider to provider care should be taken to verify that your needs are met in detail.
    * Dyn's UI doesn't allow editing or view of TTL, but the API accepts and stores the value provided, this value does not appear to be used when served
    * 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
 
@@ -170,7 +170,7 @@ You can check out the [source](/octodns/source/) and [provider](/octodns/provide
 
 Most of the things included in OctoDNS are providers, the obvious difference being that they can serve as both sources and targets of data. We'd really like to see this list grow over time so if you use an unsupported provider then PRs are welcome. The existing providers should serve as reasonable examples. Those that have no GeoDNS support are relatively straightforward. Unfortunately most of the APIs involved to do GeoDNS style traffic management are complex and somewhat inconsistent so adding support for that function would be nice, but is optional and best done in a separate pass.
 
-The `class` key in the providers config section can be used to point to arbitrary classes in the python path so internal or 3rd party providers can easily be included with no coordiation beyond getting them into PYTHONPATH, most likely installed into the virtualenv with OctoDNS.
+The `class` key in the providers config section can be used to point to arbitrary classes in the python path so internal or 3rd party providers can easily be included with no coordination beyond getting them into PYTHONPATH, most likely installed into the virtualenv with OctoDNS.
 
 ## Other Uses
 

octodns/manager.py

@@ -206,6 +206,13 @@ class Manager(object):
             if eligible_targets:
                 targets = filter(lambda d: d in eligible_targets, targets)
 
+            if not targets:
+                # Don't bother planning (and more importantly populating) zones
+                # when we don't have any eligible targets, waste of
+                # time/resources
+                self.log.info('sync:   no eligible targets, skipping')
+                continue
+
             self.log.info('sync:   sources=%s -> targets=%s', sources, targets)
 
             try:

octodns/provider/cloudflare.py

@@ -36,7 +36,7 @@ class CloudflareProvider(BaseProvider):
     '''
     SUPPORTS_GEO = False
     # TODO: support SRV
-    UNSUPPORTED_TYPES = ('NAPTR', 'PTR', 'SOA', 'SRV', 'SSHFP')
+    UNSUPPORTED_TYPES = ('ALIAS', 'NAPTR', 'PTR', 'SOA', 'SRV', 'SSHFP')
 
     MIN_TTL = 120
     TIMEOUT = 15

octodns/provider/route53.py

@@ -16,27 +16,71 @@ from ..record import Record, Update
 from .base import BaseProvider
 
 
+octal_re = re.compile(r'\\(\d\d\d)')
+
+
+def _octal_replace(s):
+    # See http://docs.aws.amazon.com/Route53/latest/DeveloperGuide/
+    #     DomainNameFormat.html
+    return octal_re.sub(lambda m: chr(int(m.group(1), 8)), s)
+
+
 class _Route53Record(object):
 
-    def __init__(self, fqdn, _type, ttl, record=None, values=None, geo=None,
-                 health_check_id=None):
-        self.fqdn = fqdn
-        self._type = _type
-        self.ttl = ttl
-        # From here on things are a little ugly, it works, but would be nice to
-        # clean up someday.
-        if record:
-            values_for = getattr(self, '_values_for_{}'.format(self._type))
-            self.values = values_for(record)
+    @classmethod
+    def new(self, provider, record, creating):
+        ret = set()
+        if getattr(record, 'geo', False):
+            ret.add(_Route53GeoDefault(provider, record, creating))
+            for ident, geo in record.geo.items():
+                ret.add(_Route53GeoRecord(provider, record, ident, geo,
+                                          creating))
         else:
-            self.values = values
-        self.geo = geo
-        self.health_check_id = health_check_id
-        self.is_geo_default = False
+            ret.add(_Route53Record(provider, record, creating))
+        return ret
 
-    @property
-    def _geo_code(self):
-        return getattr(self.geo, 'code', '')
+    def __init__(self, provider, record, creating):
+        self.fqdn = record.fqdn
+        self._type = record._type
+        self.ttl = record.ttl
+
+        values_for = getattr(self, '_values_for_{}'.format(self._type))
+        self.values = values_for(record)
+
+    def mod(self, action):
+        return {
+            'Action': action,
+            'ResourceRecordSet': {
+                'Name': self.fqdn,
+                'ResourceRecords': [{'Value': v} for v in self.values],
+                'TTL': self.ttl,
+                'Type': self._type,
+            }
+        }
+
+    # NOTE: we're using __hash__ and __cmp__ methods that consider
+    # _Route53Records equivalent if they have the same class, fqdn, and _type.
+    # Values are ignored. This is usful when computing diffs/changes.
+
+    def __hash__(self):
+        'sub-classes should never use this method'
+        return '{}:{}'.format(self.fqdn, self._type).__hash__()
+
+    def __cmp__(self, other):
+        '''sub-classes should call up to this and return its value if non-zero.
+        When it's zero they should compute their own __cmp__'''
+        if self.__class__ != other.__class__:
+            return cmp(self.__class__, other.__class__)
+        elif self.fqdn != other.fqdn:
+            return cmp(self.fqdn, other.fqdn)
+        elif self._type != other._type:
+            return cmp(self._type, other._type)
+        # We're ignoring ttl, it's not an actual differentiator
+        return 0
+
+    def __repr__(self):
+        return '_Route53Record<{} {} {} {}>'.format(self.fqdn, self._type,
+                                                    self.ttl, self.values)
 
     def _values_for_values(self, record):
         return record.values
@@ -75,68 +119,91 @@ class _Route53Record(object):
                                      v.target)
                 for v in record.values]
 
+
+class _Route53GeoDefault(_Route53Record):
+
     def mod(self, action):
+        return {
+            'Action': action,
+            'ResourceRecordSet': {
+                'Name': self.fqdn,
+                'GeoLocation': {
+                    'CountryCode': '*'
+                },
+                'ResourceRecords': [{'Value': v} for v in self.values],
+                'SetIdentifier': 'default',
+                'TTL': self.ttl,
+                'Type': self._type,
+            }
+        }
+
+    def __hash__(self):
+        return '{}:{}:default'.format(self.fqdn, self._type).__hash__()
+
+    def __repr__(self):
+        return '_Route53GeoDefault<{} {} {} {}>'.format(self.fqdn, self._type,
+                                                        self.ttl, self.values)
+
+
+class _Route53GeoRecord(_Route53Record):
+
+    def __init__(self, provider, record, ident, geo, creating):
+        super(_Route53GeoRecord, self).__init__(provider, record, creating)
+        self.geo = geo
+
+        self.health_check_id = provider.get_health_check_id(record, ident,
+                                                            geo, creating)
+
+    def mod(self, action):
+        geo = self.geo
         rrset = {
             'Name': self.fqdn,
-            'Type': self._type,
-            'TTL': self.ttl,
-            'ResourceRecords': [{'Value': v} for v in self.values],
-        }
-        if self.is_geo_default:
-            rrset['GeoLocation'] = {
+            'GeoLocation': {
                 'CountryCode': '*'
+            },
+            'ResourceRecords': [{'Value': v} for v in geo.values],
+            'SetIdentifier': geo.code,
+            'TTL': self.ttl,
+            'Type': self._type,
+        }
+
+        if self.health_check_id:
+            rrset['HealthCheckId'] = self.health_check_id
+
+        if geo.subdivision_code:
+            rrset['GeoLocation'] = {
+                'CountryCode': geo.country_code,
+                'SubdivisionCode': geo.subdivision_code
+            }
+        elif geo.country_code:
+            rrset['GeoLocation'] = {
+                'CountryCode': geo.country_code
+            }
+        else:
+            rrset['GeoLocation'] = {
+                'ContinentCode': geo.continent_code
             }
-            rrset['SetIdentifier'] = 'default'
-        elif self.geo:
-            geo = self.geo
-            rrset['SetIdentifier'] = geo.code
-            if self.health_check_id:
-                rrset['HealthCheckId'] = self.health_check_id
-            if geo.subdivision_code:
-                rrset['GeoLocation'] = {
-                    'CountryCode': geo.country_code,
-                    'SubdivisionCode': geo.subdivision_code
-                }
-            elif geo.country_code:
-                rrset['GeoLocation'] = {
-                    'CountryCode': geo.country_code
-                }
-            else:
-                rrset['GeoLocation'] = {
-                    'ContinentCode': geo.continent_code
-                }
 
         return {
             'Action': action,
             'ResourceRecordSet': rrset,
         }
 
-    # NOTE: we're using __hash__ and __cmp__ methods that consider
-    # _Route53Records equivalent if they have the same fqdn, _type, and
-    # geo.ident. Values are ignored. This is usful when computing
-    # diffs/changes.
-
     def __hash__(self):
         return '{}:{}:{}'.format(self.fqdn, self._type,
-                                 self._geo_code).__hash__()
+                                 self.geo.code).__hash__()
 
     def __cmp__(self, other):
-        return 0 if (self.fqdn == other.fqdn and
-                     self._type == other._type and
-                     self._geo_code == other._geo_code) else 1
+        ret = super(_Route53GeoRecord, self).__cmp__(other)
+        if ret != 0:
+            return ret
+        return cmp(self.geo.code, other.geo.code)
 
     def __repr__(self):
-        return '_Route53Record<{} {:>5} {:8} {}>' \
-            .format(self.fqdn, self._type, self._geo_code, self.values)
-
-
-octal_re = re.compile(r'\\(\d\d\d)')
-
-
-def _octal_replace(s):
-    # See http://docs.aws.amazon.com/Route53/latest/DeveloperGuide/
-    #     DomainNameFormat.html
-    return octal_re.sub(lambda m: chr(int(m.group(1), 8)), s)
+        return '_Route53GeoRecord<{} {} {} {} {}>'.format(self.fqdn,
+                                                          self._type, self.ttl,
+                                                          self.geo.code,
+                                                          self.values)
 
 
 class Route53Provider(BaseProvider):
@@ -173,7 +240,7 @@ class Route53Provider(BaseProvider):
         self._health_checks = None
 
     def supports(self, record):
-        return record._type != 'SSHFP'
+        return record._type not in ('ALIAS', 'SSHFP')
 
     @property
     def r53_zones(self):
@@ -391,7 +458,7 @@ class Route53Provider(BaseProvider):
 
     def _gen_mods(self, action, records):
         '''
-        Turns `_Route53Record`s in to `change_resource_record_sets` `Changes`
+        Turns `_Route53*`s in to `change_resource_record_sets` `Changes`
         '''
         return [r.mod(action) for r in records]
 
@@ -427,14 +494,14 @@ class Route53Provider(BaseProvider):
             path == config['ResourcePath'] and \
             (first_value is None or first_value == config['IPAddress'])
 
-    def _get_health_check_id(self, record, ident, geo, create):
+    def get_health_check_id(self, record, ident, geo, create):
         # fqdn & the first value are special, we use them to match up health
         # checks to their records. Route53 health checks check a single ip and
         # we're going to assume that ips are interchangeable to avoid
         # health-checking each one independently
         fqdn = record.fqdn
         first_value = geo.values[0]
-        self.log.debug('_get_health_check_id: fqdn=%s, type=%s, geo=%s, '
+        self.log.debug('get_health_check_id: fqdn=%s, type=%s, geo=%s, '
                        'first_value=%s', fqdn, record._type, ident,
                        first_value)
 
@@ -480,7 +547,7 @@ class Route53Provider(BaseProvider):
         # store the new health check so that we'll be able to find it in the
         # future
         self._health_checks[id] = health_check
-        self.log.info('_get_health_check_id: created id=%s, host=%s, path=%s'
+        self.log.info('get_health_check_id: created id=%s, host=%s, path=%s'
                       'first_value=%s', id, healthcheck_host, healthcheck_path,
                       first_value)
         return id
@@ -492,8 +559,9 @@ class Route53Provider(BaseProvider):
         # Find the health checks we're using for the new route53 records
         in_use = set()
         for r in new:
-            if r.health_check_id:
-                in_use.add(r.health_check_id)
+            hc_id = getattr(r, 'health_check_id', False)
+            if hc_id:
+                in_use.add(hc_id)
         self.log.debug('_gc_health_checks:   in_use=%s', in_use)
         # Now we need to run through ALL the health checks looking for those
         # that apply to this record, deleting any that do and are no longer in
@@ -520,23 +588,9 @@ class Route53Provider(BaseProvider):
 
     def _gen_records(self, record, creating=False):
         '''
-        Turns an octodns.Record into one or more `_Route53Record`s
+        Turns an octodns.Record into one or more `_Route53*`s
         '''
-        records = set()
-        base = _Route53Record(record.fqdn, record._type, record.ttl,
-                              record=record)
-        records.add(base)
-        if getattr(record, 'geo', False):
-            base.is_geo_default = True
-            for ident, geo in record.geo.items():
-                health_check_id = self._get_health_check_id(record, ident, geo,
-                                                            creating)
-                records.add(_Route53Record(record.fqdn, record._type,
-                                           record.ttl, values=geo.values,
-                                           geo=geo,
-                                           health_check_id=health_check_id))
-
-        return records
+        return _Route53Record.new(self, record, creating)
 
     def _mod_Create(self, change):
         # New is the stuff that needs to be created
@@ -562,24 +616,11 @@ class Route53Provider(BaseProvider):
         # things that haven't actually changed, but that's for another day.
         # We can't use set math here b/c we won't be able to control which of
         # the two objects will be in the result and we need to ensure it's the
-        # new one and we have to include some special handling when converting
-        # to/from a GEO enabled record
+        # new one.
         upserts = set()
-        existing_records = {r: r for r in existing_records}
         for new_record in new_records:
-            try:
-                existing_record = existing_records[new_record]
-                if new_record.is_geo_default != existing_record.is_geo_default:
-                    # going from normal to geo or geo to normal, need a delete
-                    # and create
-                    deletes.add(existing_record)
-                    creates.add(new_record)
-                else:
-                    # just an update
-                    upserts.add(new_record)
-            except KeyError:
-                # Completely new record, ignore
-                pass
+            if new_record in existing_records:
+                upserts.add(new_record)
 
         return self._gen_mods('DELETE', deletes) + \
             self._gen_mods('CREATE', creates) + \

octodns/provider/yaml.py

@@ -26,16 +26,22 @@ class YamlProvider(BaseProvider):
         # The ttl to use for records when not specified in the data
         # (optional, default 3600)
         default_ttl: 3600
+        # Whether or not to enforce sorting order on the yaml config
+        # (optional, default True)
+        enforce_order: True
     '''
     SUPPORTS_GEO = True
 
-    def __init__(self, id, directory, default_ttl=3600, *args, **kwargs):
+    def __init__(self, id, directory, default_ttl=3600, enforce_order=True,
+                 *args, **kwargs):
         self.log = logging.getLogger('YamlProvider[{}]'.format(id))
-        self.log.debug('__init__: id=%s, directory=%s, default_ttl=%d', id,
-                       directory, default_ttl)
+        self.log.debug('__init__: id=%s, directory=%s, default_ttl=%d, '
+                       'enforce_order=%d', id, directory, default_ttl,
+                       enforce_order)
         super(YamlProvider, self).__init__(id, *args, **kwargs)
         self.directory = directory
         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)
@@ -47,7 +53,7 @@ class YamlProvider(BaseProvider):
         before = len(zone.records)
         filename = join(self.directory, '{}yaml'.format(zone.name))
         with open(filename, 'r') as fh:
-            yaml_data = safe_load(fh)
+            yaml_data = safe_load(fh, enforce_order=self.enforce_order)
             if yaml_data:
                 for name, data in yaml_data.items():
                     if not isinstance(data, list):

octodns/yaml.py

@@ -5,25 +5,12 @@
 from __future__ import absolute_import, division, print_function, \
     unicode_literals
 
+from natsort import natsort_keygen
 from yaml import SafeDumper, SafeLoader, load, dump
 from yaml.constructor import ConstructorError
-import re
 
 
-# zero-padded sort, simplified version of
-# https://www.xormedia.com/natural-sort-order-with-zero-padding/
-_pad_re = re.compile('\d+')
-
-
-def _zero_pad(match):
-    return '{:04d}'.format(int(match.group(0)))
-
-
-def _zero_padded_numbers(s):
-    try:
-        int(s)
-    except ValueError:
-        return _pad_re.sub(lambda d: _zero_pad(d), s)
+_natsort_key = natsort_keygen()
 
 
 # Found http://stackoverflow.com/a/21912744 which guided me on how to hook in
@@ -34,7 +21,7 @@ class SortEnforcingLoader(SafeLoader):
         self.flatten_mapping(node)
         ret = self.construct_pairs(node)
         keys = [d[0] for d in ret]
-        if keys != sorted(keys, key=_zero_padded_numbers):
+        if keys != sorted(keys, key=_natsort_key):
             raise ConstructorError(None, None, "keys out of order: {}"
                                    .format(', '.join(keys)), node.start_mark)
         return dict(ret)
@@ -59,7 +46,7 @@ class SortingDumper(SafeDumper):
 
     def _representer(self, data):
         data = data.items()
-        data.sort(key=lambda d: _zero_padded_numbers(d[0]))
+        data.sort(key=lambda d: _natsort_key(d[0]))
         return self.represent_mapping(self.DEFAULT_MAPPING_TAG, data)
 
 

requirements.txt

@@ -10,6 +10,7 @@ futures==3.0.5
 incf.countryutils==1.0
 ipaddress==1.0.18
 jmespath==0.9.0
+natsort==5.0.3
 nsone==0.9.10
 python-dateutil==2.6.0
 requests==2.13.0

setup.py

@@ -34,6 +34,7 @@ setup(
         'futures>=3.0.5',
         'incf.countryutils>=1.0',
         'ipaddress>=1.0.18',
+        'natsort>=5.0.3',
         'python-dateutil>=2.6.0',
         'requests>=2.13.0'
     ],

tests/test_octodns_provider_route53.py

@@ -11,8 +11,8 @@ from unittest import TestCase
 from mock import patch
 
 from octodns.record import Create, Delete, Record, Update
-from octodns.provider.route53 import _Route53Record, Route53Provider, \
-    _octal_replace
+from octodns.provider.route53 import Route53Provider, _Route53GeoDefault, \
+    _Route53GeoRecord, _Route53Record, _octal_replace
 from octodns.zone import Zone
 
 from helpers import GeoProvider
@@ -531,16 +531,6 @@ class TestRoute53Provider(TestCase):
         change_resource_record_sets_params = {
             'ChangeBatch': {
                 'Changes': [{
-                    'Action': 'DELETE',
-                    'ResourceRecordSet': {
-                        'GeoLocation': {'ContinentCode': 'OC'},
-                        'Name': 'simple.unit.tests.',
-                        'ResourceRecords': [{'Value': '3.2.3.4'},
-                                            {'Value': '4.2.3.4'}],
-                        'SetIdentifier': 'OC',
-                        'TTL': 61,
-                        'Type': 'A'}
-                }, {
                     'Action': 'DELETE',
                     'ResourceRecordSet': {
                         'GeoLocation': {'CountryCode': '*'},
@@ -550,6 +540,16 @@ class TestRoute53Provider(TestCase):
                         'SetIdentifier': 'default',
                         'TTL': 61,
                         'Type': 'A'}
+                }, {
+                    'Action': 'DELETE',
+                    'ResourceRecordSet': {
+                        'GeoLocation': {'ContinentCode': 'OC'},
+                        'Name': 'simple.unit.tests.',
+                        'ResourceRecords': [{'Value': '3.2.3.4'},
+                                            {'Value': '4.2.3.4'}],
+                        'SetIdentifier': 'OC',
+                        'TTL': 61,
+                        'Type': 'A'}
                 }, {
                     'Action': 'CREATE',
                     'ResourceRecordSet': {
@@ -708,8 +708,7 @@ class TestRoute53Provider(TestCase):
                 'AF': ['4.2.3.4'],
             }
         })
-        id = provider._get_health_check_id(record, 'AF', record.geo['AF'],
-                                           True)
+        id = provider.get_health_check_id(record, 'AF', record.geo['AF'], True)
         self.assertEquals('42', id)
 
     def test_health_check_create(self):
@@ -782,13 +781,12 @@ class TestRoute53Provider(TestCase):
         })
 
         # if not allowed to create returns none
-        id = provider._get_health_check_id(record, 'AF', record.geo['AF'],
-                                           False)
+        id = provider.get_health_check_id(record, 'AF', record.geo['AF'],
+                                          False)
         self.assertFalse(id)
 
         # when allowed to create we do
-        id = provider._get_health_check_id(record, 'AF', record.geo['AF'],
-                                           True)
+        id = provider.get_health_check_id(record, 'AF', record.geo['AF'], True)
         self.assertEquals('42', id)
         stubber.assert_no_pending_responses()
 
@@ -1201,10 +1199,6 @@ class TestRoute53Provider(TestCase):
         self.assertEquals(1, len(extra))
         stubber.assert_no_pending_responses()
 
-    def test_route_53_record(self):
-        # Just make sure it doesn't blow up
-        _Route53Record('foo.unit.tests.', 'A', 30).__repr__()
-
     def _get_test_plan(self, max_changes):
 
         provider = Route53Provider('test', 'abc', '123', max_changes)
@@ -1332,3 +1326,71 @@ class TestRoute53Provider(TestCase):
             'TTL': 30,
             'Type': 'TXT',
         }))
+
+
+class TestRoute53Records(TestCase):
+
+    def test_route53_record(self):
+        existing = Zone('unit.tests.', [])
+        record_a = Record.new(existing, '', {
+            'geo': {
+                'NA-US': ['2.2.2.2', '3.3.3.3'],
+                'OC': ['4.4.4.4', '5.5.5.5']
+            },
+            'ttl': 99,
+            'type': 'A',
+            'values': ['9.9.9.9']
+        })
+        a = _Route53Record(None, record_a, False)
+        self.assertEquals(a, a)
+        b = _Route53Record(None, Record.new(existing, '',
+                                            {'ttl': 32, 'type': 'A',
+                                             'values': ['8.8.8.8',
+                                                        '1.1.1.1']}),
+                           False)
+        self.assertEquals(b, b)
+        c = _Route53Record(None, Record.new(existing, 'other',
+                                            {'ttl': 99, 'type': 'A',
+                                             'values': ['9.9.9.9']}),
+                           False)
+        self.assertEquals(c, c)
+        d = _Route53Record(None, Record.new(existing, '',
+                                            {'ttl': 42, 'type': 'CNAME',
+                                             'value': 'foo.bar.'}),
+                           False)
+        self.assertEquals(d, d)
+
+        # Same fqdn & type is same record
+        self.assertEquals(a, b)
+        # Same name & different type is not the same
+        self.assertNotEquals(a, d)
+        # Different name & same type is not the same
+        self.assertNotEquals(a, c)
+
+        # Same everything, different class is not the same
+        e = _Route53GeoDefault(None, record_a, False)
+        self.assertNotEquals(a, e)
+
+        class DummyProvider(object):
+
+            def get_health_check_id(self, *args, **kwargs):
+                return None
+
+        provider = DummyProvider()
+        f = _Route53GeoRecord(provider, record_a, 'NA-US',
+                              record_a.geo['NA-US'], False)
+        self.assertEquals(f, f)
+        g = _Route53GeoRecord(provider, record_a, 'OC',
+                              record_a.geo['OC'], False)
+        self.assertEquals(g, g)
+
+        # Geo and non-geo are not the same, using Geo as primary to get it's
+        # __cmp__
+        self.assertNotEquals(f, a)
+        # Same everything, different geo's is not the same
+        self.assertNotEquals(f, g)
+
+        # Make sure it doesn't blow up
+        a.__repr__()
+        e.__repr__()
+        f.__repr__()

tests/test_octodns_provider_yaml.py

@@ -100,6 +100,12 @@ class TestYamlProvider(TestCase):
         with self.assertRaises(ConstructorError):
             source.populate(zone)
 
+        source = YamlProvider('test', join(dirname(__file__), 'config'),
+                              enforce_order=False)
+        # no exception
+        source.populate(zone)
+        self.assertEqual(2, len(zone.records))
+
     def test_subzone_handling(self):
         source = YamlProvider('test', join(dirname(__file__), 'config'))
 

tests/test_octodns_yaml.py

@@ -59,3 +59,12 @@ class TestYaml(TestCase):
         }, buf)
         self.assertEquals("---\n'*.1.1': 42\n'*.2.1': 44\n'*.11.1': 43\n",
                           buf.getvalue())
+
+        # hex sorting isn't ideal, not treated as hex, this make sure we don't
+        # change the behavior
+        buf = StringIO()
+        safe_dump({
+            '45a03129': 42,
+            '45a0392a': 43,
+        }, buf)
+        self.assertEquals("---\n45a0392a: 43\n45a03129: 42\n", buf.getvalue())