diff --git a/CHANGELOG.md b/CHANGELOG.md
index adb1f8c..6e2b243 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,12 @@
+## v0.9.6 - 2019-07-16 - The little one that fixes stuff from the big one
+
+* Reduced dynamic record value weight range to 0-15 so that Dyn and Route53
+ match up behaviors. Dyn is limited to 0-15 and scaling that up would lose
+ resolution that couldn't be recovered during populate.
+* Addressed issues with Route53 change set ordering for dynamic records
+* Ignore unsupported record types in DigitalOceanProvider
+* Fix bugs in Route53 extra changes handling and health check managagement
+
## v0.9.5 - 2019-05-06 - The big one, with all the dynamic stuff
* dynamic record support, essentially a v2 version of geo records with a lot
diff --git a/README.md b/README.md
index a3f3eae..163c723 100644
--- a/README.md
+++ b/README.md
@@ -1,4 +1,4 @@
-
+
## DNS as code - Tools for managing DNS across multiple providers
@@ -275,4 +275,4 @@ GitHub® and its stylized versions and the Invertocat mark are GitHub's Trademar
## Authors
-OctoDNS was designed and authored by [Ross McFarland](https://github.com/ross) and [Joe Williams](https://github.com/joewilliams). It is now maintained, reviewed, and tested by Ross, Joe, and the rest of the Site Reliability Engineering team at GitHub.
+OctoDNS was designed and authored by [Ross McFarland](https://github.com/ross) and [Joe Williams](https://github.com/joewilliams). It is now maintained, reviewed, and tested by Traffic Engineering team at GitHub.
diff --git a/octodns/__init__.py b/octodns/__init__.py
index 939c293..6422577 100644
--- a/octodns/__init__.py
+++ b/octodns/__init__.py
@@ -3,4 +3,4 @@
from __future__ import absolute_import, division, print_function, \
unicode_literals
-__VERSION__ = '0.9.5'
+__VERSION__ = '0.9.6'
diff --git a/octodns/provider/digitalocean.py b/octodns/provider/digitalocean.py
index 98a78ad..e192543 100644
--- a/octodns/provider/digitalocean.py
+++ b/octodns/provider/digitalocean.py
@@ -223,6 +223,10 @@ class DigitalOceanProvider(BaseProvider):
values = defaultdict(lambda: defaultdict(list))
for record in self.zone_records(zone):
_type = record['type']
+ if _type not in self.SUPPORTS:
+ self.log.warning('populate: skipping unsupported %s record',
+ _type)
+ continue
values[record['name']][record['type']].append(record)
before = len(zone.records)
diff --git a/octodns/provider/route53.py b/octodns/provider/route53.py
index e8b11fa..4cf7c99 100644
--- a/octodns/provider/route53.py
+++ b/octodns/provider/route53.py
@@ -519,37 +519,71 @@ class _Route53GeoRecord(_Route53Record):
self.values)
-_mod_keyer_action_order = {
- 'DELETE': 0, # Delete things first
- 'CREATE': 1, # Then Create things
- 'UPSERT': 2, # Upsert things last
-}
-
-
def _mod_keyer(mod):
rrset = mod['ResourceRecordSet']
- action_order = _mod_keyer_action_order[mod['Action']]
- # We're sorting by 3 "columns", the action, the rrset type, and finally the
- # name/id of the rrset. This ensures that Route53 won't see a RRSet that
- # targets another that hasn't been seen yet. I.e. targets must come before
- # things that target them. We sort on types of things rather than
- # explicitly looking for targeting relationships since that's sufficent and
- # easier to grok/do.
+ # Route53 requires that changes are ordered such that a target of an
+ # AliasTarget is created or upserted prior to the record that targets it.
+ # This is complicated by "UPSERT" appearing to be implemented as "DELETE"
+ # before all changes, followed by a "CREATE", internally in the AWS API.
+ # Because of this, we order changes as follows:
+ # - Delete any records that we wish to delete that are GEOS
+ # (because they are never targetted by anything)
+ # - Delete any records that we wish to delete that are SECONDARY
+ # (because they are no longer targetted by GEOS)
+ # - Delete any records that we wish to delete that are PRIMARY
+ # (because they are no longer targetted by SECONDARY)
+ # - Delete any records that we wish to delete that are VALUES
+ # (because they are no longer targetted by PRIMARY)
+ # - CREATE/UPSERT any records that are VALUES
+ # (because they don't depend on other records)
+ # - CREATE/UPSERT any records that are PRIMARY
+ # (because they always point to VALUES which now exist)
+ # - CREATE/UPSERT any records that are SECONDARY
+ # (because they now have PRIMARY records to target)
+ # - CREATE/UPSERT any records that are GEOS
+ # (because they now have all their PRIMARY pools to target)
+ # - :tada:
+ #
+ # In theory we could also do this based on actual target reference
+ # checking, but that's more complex. Since our rules have a known
+ # dependency order, we just rely on that.
+ # Get the unique ID from the name/id to get a consistent ordering.
if rrset.get('GeoLocation', False):
- return (action_order, 3, rrset['SetIdentifier'])
+ unique_id = rrset['SetIdentifier']
+ else:
+ unique_id = rrset['Name']
+
+ # Prioritise within the action_priority, ensuring targets come first.
+ if rrset.get('GeoLocation', False):
+ # Geos reference pools, so they come last.
+ record_priority = 3
elif rrset.get('AliasTarget', False):
# We use an alias
if rrset.get('Failover', False) == 'SECONDARY':
- # We're a secondary we'll ref primaries
- return (action_order, 2, rrset['Name'])
+ # We're a secondary, which reference the primary (failover, P1).
+ record_priority = 2
else:
- # We're a primary we'll ref values
- return (action_order, 1, rrset['Name'])
+ # We're a primary, we reference values (P0).
+ record_priority = 1
+ else:
+ # We're just a plain value, has no dependencies so first.
+ record_priority = 0
- # We're just a plain value, these come first
- return (action_order, 0, rrset['Name'])
+ if mod['Action'] == 'DELETE':
+ # Delete things first, so we can never trounce our own additions
+ action_priority = 0
+ # Delete in the reverse order of priority, e.g. start with the deepest
+ # reference and work back to the values, rather than starting at the
+ # values (still ref'd).
+ record_priority = -record_priority
+ else:
+ # For CREATE and UPSERT, Route53 seems to treat them the same, so
+ # interleave these, keeping the reference order described above.
+ action_priority = 1
+
+ return (action_priority, record_priority, unique_id)
def _parse_pool_name(n):
@@ -1250,15 +1284,27 @@ class Route53Provider(BaseProvider):
'%s', record.fqdn, record._type)
fqdn = record.fqdn
+ _type = record._type
# loop through all the r53 rrsets
for rrset in self._load_records(zone_id):
name = rrset['Name']
+ # Break off the first piece of the name, it'll let us figure out if
+ # this is an rrset we're interested in.
+ maybe_meta, rest = name.split('.', 1)
- if record._type == rrset['Type'] and name.endswith(fqdn) and \
- name.startswith('_octodns-') and '-value.' in name and \
- '-default-' not in name and \
- self._extra_changes_update_needed(record, rrset):
+ if not maybe_meta.startswith('_octodns-') or \
+ not maybe_meta.endswith('-value') or \
+ '-default-' in name:
+ # We're only interested in non-default dynamic value records,
+ # as that's where healthchecks live
+ continue
+
+ if rest != fqdn or _type != rrset['Type']:
+ # rrset isn't for the current record
+ continue
+
+ if self._extra_changes_update_needed(record, rrset):
# no good, doesn't have the right health check, needs an update
self.log.info('_extra_changes_dynamic_needs_update: '
'health-check caused update of %s:%s',
diff --git a/octodns/record/__init__.py b/octodns/record/__init__.py
index dca6100..83632bc 100644
--- a/octodns/record/__init__.py
+++ b/octodns/record/__init__.py
@@ -514,7 +514,7 @@ class _DynamicMixin(object):
try:
weight = value['weight']
weight = int(weight)
- if weight < 1 or weight > 255:
+ if weight < 1 or weight > 15:
reasons.append('invalid weight "{}" in pool "{}" '
'value {}'.format(weight, _id,
value_num))
diff --git a/requirements-dev.txt b/requirements-dev.txt
index 1afee06..77dd50c 100644
--- a/requirements-dev.txt
+++ b/requirements-dev.txt
@@ -6,4 +6,4 @@ pycountry>=18.12.8
pycountry_convert>=0.7.2
pyflakes==1.6.0
requests_mock
-twine==1.11.0
+twine==1.13.0
diff --git a/setup.py b/setup.py
index 7a9348e..75a39d7 100644
--- a/setup.py
+++ b/setup.py
@@ -41,6 +41,7 @@ setup(
],
license='MIT',
long_description=open('README.md').read(),
+ long_description_content_type='text/markdown',
name='octodns',
packages=find_packages(),
url='https://github.com/github/octodns',
diff --git a/tests/config/dynamic.tests.yaml b/tests/config/dynamic.tests.yaml
index 3d806f9..f826880 100644
--- a/tests/config/dynamic.tests.yaml
+++ b/tests/config/dynamic.tests.yaml
@@ -19,7 +19,7 @@ a:
- value: 6.6.6.6
weight: 10
- value: 5.5.5.5
- weight: 25
+ weight: 15
rules:
- geos:
- EU-GB
@@ -90,9 +90,9 @@ cname:
sea:
values:
- value: target-sea-1.unit.tests.
- weight: 100
+ weight: 10
- value: target-sea-2.unit.tests.
- weight: 175
+ weight: 14
rules:
- geos:
- EU-GB
diff --git a/tests/config/split/dynamic.tests./a.yaml b/tests/config/split/dynamic.tests./a.yaml
index fd748b4..f182df6 100644
--- a/tests/config/split/dynamic.tests./a.yaml
+++ b/tests/config/split/dynamic.tests./a.yaml
@@ -23,7 +23,7 @@ a:
fallback: null
values:
- value: 5.5.5.5
- weight: 25
+ weight: 15
- value: 6.6.6.6
weight: 10
rules:
diff --git a/tests/config/split/dynamic.tests./cname.yaml b/tests/config/split/dynamic.tests./cname.yaml
index a84c202..ff85955 100644
--- a/tests/config/split/dynamic.tests./cname.yaml
+++ b/tests/config/split/dynamic.tests./cname.yaml
@@ -21,9 +21,9 @@ cname:
fallback: null
values:
- value: target-sea-1.unit.tests.
- weight: 100
+ weight: 10
- value: target-sea-2.unit.tests.
- weight: 175
+ weight: 14
rules:
- geos:
- EU-GB
diff --git a/tests/fixtures/digitalocean-page-1.json b/tests/fixtures/digitalocean-page-1.json
index db231ba..c931411 100644
--- a/tests/fixtures/digitalocean-page-1.json
+++ b/tests/fixtures/digitalocean-page-1.json
@@ -1,5 +1,16 @@
{
"domain_records": [{
+ "id": null,
+ "type": "SOA",
+ "name": "@",
+ "data": null,
+ "priority": null,
+ "port": null,
+ "ttl": null,
+ "weight": null,
+ "flags": null,
+ "tag": null
+ }, {
"id": 11189874,
"type": "NS",
"name": "@",
diff --git a/tests/test_octodns_provider_route53.py b/tests/test_octodns_provider_route53.py
index added7f..265a0a7 100644
--- a/tests/test_octodns_provider_route53.py
+++ b/tests/test_octodns_provider_route53.py
@@ -700,18 +700,6 @@ class TestRoute53Provider(TestCase):
'TTL': 61,
'Type': 'A'
}
- }, {
- 'Action': 'CREATE',
- 'ResourceRecordSet': {
- 'GeoLocation': {'CountryCode': 'US',
- 'SubdivisionCode': 'CA'},
- 'HealthCheckId': u'44',
- 'Name': 'unit.tests.',
- 'ResourceRecords': [{'Value': '7.2.3.4'}],
- 'SetIdentifier': 'NA-US-CA',
- 'TTL': 61,
- 'Type': 'A'
- }
}, {
'Action': 'UPSERT',
'ResourceRecordSet': {
@@ -735,6 +723,18 @@ class TestRoute53Provider(TestCase):
'TTL': 61,
'Type': 'A'
}
+ }, {
+ 'Action': 'CREATE',
+ 'ResourceRecordSet': {
+ 'GeoLocation': {'CountryCode': 'US',
+ 'SubdivisionCode': 'CA'},
+ 'HealthCheckId': u'44',
+ 'Name': 'unit.tests.',
+ 'ResourceRecords': [{'Value': '7.2.3.4'}],
+ 'SetIdentifier': 'NA-US-CA',
+ 'TTL': 61,
+ 'Type': 'A'
+ }
}, {
'Action': 'UPSERT',
'ResourceRecordSet': {
@@ -1673,7 +1673,7 @@ class TestRoute53Provider(TestCase):
desired.add_record(record)
list_resource_record_sets_resp = {
'ResourceRecordSets': [{
- # other name
+ # Not dynamic value and other name
'Name': 'unit.tests.',
'Type': 'A',
'GeoLocation': {
@@ -1683,17 +1683,21 @@ class TestRoute53Provider(TestCase):
'Value': '1.2.3.4',
}],
'TTL': 61,
+ # All the non-matches have a different Id so we'll fail if they
+ # match
+ 'HealthCheckId': '33',
}, {
- # matching name, other type
+ # Not dynamic value, matching name, other type
'Name': 'a.unit.tests.',
'Type': 'AAAA',
'ResourceRecords': [{
'Value': '2001:0db8:3c4d:0015:0000:0000:1a2f:1a4b'
}],
'TTL': 61,
+ 'HealthCheckId': '33',
}, {
# default value pool
- 'Name': '_octodns-default-pool.a.unit.tests.',
+ 'Name': '_octodns-default-value.a.unit.tests.',
'Type': 'A',
'GeoLocation': {
'CountryCode': '*',
@@ -1702,6 +1706,37 @@ class TestRoute53Provider(TestCase):
'Value': '1.2.3.4',
}],
'TTL': 61,
+ 'HealthCheckId': '33',
+ }, {
+ # different record
+ 'Name': '_octodns-two-value.other.unit.tests.',
+ 'Type': 'A',
+ 'GeoLocation': {
+ 'CountryCode': '*',
+ },
+ 'ResourceRecords': [{
+ 'Value': '1.2.3.4',
+ }],
+ 'TTL': 61,
+ 'HealthCheckId': '33',
+ }, {
+ # same everything, but different type
+ 'Name': '_octodns-one-value.a.unit.tests.',
+ 'Type': 'AAAA',
+ 'ResourceRecords': [{
+ 'Value': '2001:0db8:3c4d:0015:0000:0000:1a2f:1a4b'
+ }],
+ 'TTL': 61,
+ 'HealthCheckId': '33',
+ }, {
+ # same everything, sub
+ 'Name': '_octodns-one-value.sub.a.unit.tests.',
+ 'Type': 'A',
+ 'ResourceRecords': [{
+ 'Value': '1.2.3.4',
+ }],
+ 'TTL': 61,
+ 'HealthCheckId': '33',
}, {
# match
'Name': '_octodns-one-value.a.unit.tests.',
@@ -2391,7 +2426,7 @@ class TestModKeyer(TestCase):
def test_mod_keyer(self):
- # First "column"
+ # First "column" is the action priority for C/R/U
# Deletes come first
self.assertEquals((0, 0, 'something'), _mod_keyer({
@@ -2409,8 +2444,8 @@ class TestModKeyer(TestCase):
}
}))
- # Then upserts
- self.assertEquals((2, 0, 'last'), _mod_keyer({
+ # Upserts are the same as creates
+ self.assertEquals((1, 0, 'last'), _mod_keyer({
'Action': 'UPSERT',
'ResourceRecordSet': {
'Name': 'last',
@@ -2420,7 +2455,7 @@ class TestModKeyer(TestCase):
# Second "column" value records tested above
# AliasTarget primary second (to value)
- self.assertEquals((0, 1, 'thing'), _mod_keyer({
+ self.assertEquals((0, -1, 'thing'), _mod_keyer({
'Action': 'DELETE',
'ResourceRecordSet': {
'AliasTarget': 'some-target',
@@ -2429,8 +2464,17 @@ class TestModKeyer(TestCase):
}
}))
+ self.assertEquals((1, 1, 'thing'), _mod_keyer({
+ 'Action': 'UPSERT',
+ 'ResourceRecordSet': {
+ 'AliasTarget': 'some-target',
+ 'Failover': 'PRIMARY',
+ 'Name': 'thing',
+ }
+ }))
+
# AliasTarget secondary third
- self.assertEquals((0, 2, 'thing'), _mod_keyer({
+ self.assertEquals((0, -2, 'thing'), _mod_keyer({
'Action': 'DELETE',
'ResourceRecordSet': {
'AliasTarget': 'some-target',
@@ -2439,8 +2483,17 @@ class TestModKeyer(TestCase):
}
}))
+ self.assertEquals((1, 2, 'thing'), _mod_keyer({
+ 'Action': 'UPSERT',
+ 'ResourceRecordSet': {
+ 'AliasTarget': 'some-target',
+ 'Failover': 'SECONDARY',
+ 'Name': 'thing',
+ }
+ }))
+
# GeoLocation fourth
- self.assertEquals((0, 3, 'some-id'), _mod_keyer({
+ self.assertEquals((0, -3, 'some-id'), _mod_keyer({
'Action': 'DELETE',
'ResourceRecordSet': {
'GeoLocation': 'some-target',
@@ -2448,4 +2501,12 @@ class TestModKeyer(TestCase):
}
}))
+ self.assertEquals((1, 3, 'some-id'), _mod_keyer({
+ 'Action': 'UPSERT',
+ 'ResourceRecordSet': {
+ 'GeoLocation': 'some-target',
+ 'SetIdentifier': 'some-id',
+ }
+ }))
+
# The third "column" has already been tested above, Name/SetIdentifier
diff --git a/tests/test_octodns_record.py b/tests/test_octodns_record.py
index 53bc5e7..2b11364 100644
--- a/tests/test_octodns_record.py
+++ b/tests/test_octodns_record.py
@@ -2460,7 +2460,7 @@ class TestDynamicRecords(TestCase):
'weight': 1,
'value': '6.6.6.6',
}, {
- 'weight': 256,
+ 'weight': 16,
'value': '7.7.7.7',
}],
},
@@ -2484,7 +2484,7 @@ class TestDynamicRecords(TestCase):
}
with self.assertRaises(ValidationError) as ctx:
Record.new(self.zone, 'bad', a_data)
- self.assertEquals(['invalid weight "256" in pool "three" value 2'],
+ self.assertEquals(['invalid weight "16" in pool "three" value 2'],
ctx.exception.reasons)
# invalid non-int weight