Merge pull request #1 from github/master

update 5/30/2019

CHANGELOG.md

@@ -1,7 +1,7 @@
-## v0.9.5 - 2019-??-?? - The big one, with all the dynamic stuff
+## 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
-  more flexibility and power. Also support dynamic CNAME records.
+  more flexibility and power. Also support dynamic CNAME records (alpha)
 * Route53Provider dynamic record support
 * DynProvider dynamic record support
 * SUPPORTS_DYNAMIC is an optional property, defaults to False
@@ -9,7 +9,13 @@
 * CloudflareProvider SRV record unpacking fix
 * DNSMadeEasy provider uses supports to avoid blowing up on unknown record
   types
+* Updates to AzureProvider lib versions
 * Normalize MX/CNAME/ALIAS/PTR value to lower case
+* SplitYamlProvider support added
+* DynProvider fix for Traffic Directors association to records, explicit rather
+  than "looks close enough"
+* TinyDNS support for TXT and AAAA records and fixes to ; escaping
+* pre-commit hook requires 100% code coverage
 
 ## v0.9.4 - 2019-01-28 - The one with a bunch of stuff, before the big one
 

docs/dynamic_records.md

@@ -0,0 +1,126 @@
+## Dynamic Record Support
+
+Dynamic records provide support for GeoDNS and weighting to records. `A` and `AAAA` are fully supported and reasonably well tested for both Dyn (via Traffic Directors) and Route53. There is preliminary support for `CNAME` records, but caution should be exercised as they have not been thoroughly tested.
+
+Configuring GeoDNS is complex and the details of the functionality vary widely from provider to provider. octoDNS has an opinionated view mostly to give a reasonably consistent behavior across providers which is similar to the overall philosophy and approach of octoDNS itself. It may not fit your needs or use cases, in which case please open an issue for discussion. We expect this functionality to grow and evolve over time as it's more widely used.
+
+### An Annotated Example
+
+```yaml
+
+---
+test:
+  # This is a dynamic record when used with providers that support it
+  dynamic:
+    # These are the pools of records that can be referenced and thus used by rules
+    pools:
+      apac:
+        # An optional fallback, if all of the records in this pool fail this pool should be tried
+        fallback: na
+        # One or more values for this pool
+        values:
+        - value: 1.1.1.1
+        - value: 2.2.2.2
+      eu:
+        fallback: na
+        values:
+        - value: 3.3.3.3
+          # Weight for this value, if omitted the default is 1
+          weight: 2
+        - value: 4.4.4.4
+          weight: 3
+      na:
+        # Implicit fallback to the default pool (below)
+        values:
+        - value: 5.5.5.5
+        - value: 6.6.6.6
+        - value: 7.7.7.7
+    # Rules that assign queries to pools
+    rules:
+    - geos:
+      # Geos used in matching queries
+      - AS
+      - OC
+      # The pool to service the query from
+      pool: apac
+    - geos:
+      - AF
+      - EU
+      pool: eu
+    # No geos means match all queries
+    - pool: na
+  ttl: 60
+  type: A
+  # These values become a non-healthchecked default pool
+  values:
+  - 5.5.5.5
+  - 6.6.6.6
+  - 7.7.7.7
+```
+
+#### Geo Codes
+
+Geo codes consist of one to three parts depending on the scope of the area being targeted. Examples of these look like:
+
+* 'NA-US-KY' - North America, United States, Kentucky
+* 'NA-US' - North America, United States
+* 'NA' - North America
+
+The first portion is the continent:
+
+* 'AF': 14,  # Continental Africa
+* 'AN': 17,  # Continental Antarctica
+* 'AS': 15,  # Continental Asia
+* 'EU': 13,  # Continental Europe
+* 'NA': 11,  # Continental North America
+* 'OC': 16,  # Continental Australia/Oceania
+* 'SA': 12,  # Continental South America
+
+The second is the two-letter ISO Country Code https://en.wikipedia.org/wiki/ISO_3166-2 and the third is the ISO Country Code Subdivision as per https://en.wikipedia.org/wiki/ISO_3166-2:US. Change the code at the end for the country you are subdividing. Note that these may not always be supported depending on the providers in use.
+
+### Health Checks
+
+octoDNS will automatically configure the provider to monitor each IP and check for a 200 response for **https://<ip_address>/_dns**.
+
+These checks can be customized via the `healthcheck` configuration options.
+
+```yaml
+
+---
+test:
+  ...
+  octodns:
+    healthcheck:
+      host: my-host-name
+      path: /dns-health-check
+      port: 443
+      protocol: HTTPS
+  ...
+```
+
+| Key  | Description | Default |
+|--|--|--|
+| host | FQDN for host header and SNI | - |
+| path | path to check | _dns |
+| port | port to check | 443 |
+| protocol | HTTP/HTTPS | HTTPS |
+
+#### Route53 Healtch Check Options
+
+| Key  | Description | Default |
+|--|--|--|
+| measure_latency | Show latency in AWS console | true |
+
+```yaml
+
+---
+  octodns:
+    healthcheck:
+      host: my-host-name
+      path: /dns-health-check
+      port: 443
+      protocol: HTTPS
+    route53:
+      healthcheck:
+        measure_latency: false
+```

docs/geo_records.md

@@ -0,0 +1,101 @@
+## Geo Record Support
+
+Note: Geo DNS records are still supported for the time being, but it is still strongy encouraged that you look at [Dynamic Records](/docs/dynamic_records.md) instead as they are a superset of functionality.
+
+GeoDNS is currently supported for `A` and `AAAA` records on the Dyn (via Traffic Directors) and Route53 providers. Records with geo information pushed to providers without support for them will be managed as non-geo records using the base values.
+
+Configuring GeoDNS is complex and the details of the functionality vary widely from provider to provider. OctoDNS has an opinionated view of how GeoDNS should be set up and does its best to map that to each provider's offering in a way that will result in similar behavior. It may not fit your needs or use cases, in which case please open an issue for discussion. We expect this functionality to grow and evolve over time as it's more widely used.
+
+The following is an example of GeoDNS with three entries NA-US-CA, NA-US-NY, OC-AU. Octodns creates another one labeled 'default' with the details for the actual A record, This default record is the failover record if the monitoring check fails.
+
+```yaml
+---
+? ''
+: type: TXT
+  value: v=spf1 -all
+test:
+  geo:
+    NA-US-NY:
+    - 111.111.111.1
+    NA-US-CA:
+    - 111.111.111.2
+    OC-AU:
+    - 111.111.111.3
+    EU:
+    - 111.111.111.4
+  ttl: 300
+  type: A
+  value: 111.111.111.5
+```
+
+
+The geo labels breakdown based on:
+
+1.
+    - 'AF': 14,  # Continental Africa
+    - 'AN': 17,  # Continental Antarctica
+    - 'AS': 15,  # Continental Asia
+    - 'EU': 13,  # Continental Europe
+    - 'NA': 11,  # Continental North America
+    - 'OC': 16,  # Continental Australia/Oceania
+    - 'SA': 12,  # Continental South America
+
+2. ISO Country Code https://en.wikipedia.org/wiki/ISO_3166-2
+
+3. ISO Country Code Subdivision as per https://en.wikipedia.org/wiki/ISO_3166-2:US   (change the code at the end for the country you are subdividing) * these may not always be supported depending on the provider.
+
+So the example is saying:
+
+- North America - United States - New York:  gets served an "A" record of  111.111.111.1
+- North America - United States - California:   gets served an "A" record of  111.111.111.2
+- Oceania - Australia: Gets served an "A" record of 111.111.111.3
+- Europe: gets an "A" record of 111.111.111.4
+- Everyone else gets an "A" record of 111.111.111.5
+
+### Health Checks
+
+Octodns will automatically set up monitors check for a 200 response for **https://<ip_address>/_dns**.
+
+These checks can be configured by adding a `healthcheck` configuration to the record:
+
+```yaml
+---
+test:
+  geo:
+    AS:
+      - 1.2.3.4
+    EU:
+      - 2.3.4.5
+  octodns:
+    healthcheck:
+      host: my-host-name
+      path: /dns-health-check
+      port: 443
+      protocol: HTTPS
+```
+
+| Key  | Description | Default |
+|--|--|--|
+| host | FQDN for host header and SNI | - |
+| path | path to check | _dns |
+| port | port to check | 443 |
+| protocol | HTTP/HTTPS | HTTPS |
+
+#### Route53 Healtch Check Options
+
+| Key  | Description | Default |
+|--|--|--|
+| measure_latency | Show latency in AWS console | true |
+
+```yaml
+---
+  octodns:
+    healthcheck:
+      host: my-host-name
+      path: /dns-health-check
+      port: 443
+      protocol: HTTPS
+    route53:
+      healthcheck:
+        measure_latency: false
+```

docs/records.md

@@ -20,106 +20,10 @@ Underlying provider support for each of these varies and some providers have ext
 
 Adding new record types to OctoDNS is relatively straightforward, but will require careful evaluation of each provider to determine whether or not it will be supported and the addition of code in each to handle and test the new type.
 
-## GeoDNS support
-
-GeoDNS is currently supported for `A` and `AAAA` records on the Dyn (via Traffic Directors) and Route53 providers. Records with geo information pushed to providers without support for them will be managed as non-geo records using the base values.
-
-Configuring GeoDNS is complex and the details of the functionality vary widely from provider to provider. OctoDNS has an opinionated view of how GeoDNS should be set up and does its best to map that to each provider's offering in a way that will result in similar behavior. It may not fit your needs or use cases, in which case please open an issue for discussion. We expect this functionality to grow and evolve over time as it's more widely used.
-
-The following is an example of GeoDNS with three entries NA-US-CA, NA-US-NY, OC-AU. Octodns creates another one labeled 'default' with the details for the actual A record, This default record is the failover record if the monitoring check fails.
-
-```yaml
----
-? ''
-: type: TXT
-  value: v=spf1 -all
-test:
-  geo:
-    NA-US-NY:
-    - 111.111.111.1
-    NA-US-CA:
-    - 111.111.111.2
-    OC-AU:
-    - 111.111.111.3
-    EU:
-    - 111.111.111.4
-  ttl: 300
-  type: A
-  value: 111.111.111.5
-```
-
-
-The geo labels breakdown based on:
-
-1.
-    - 'AF': 14,  # Continental Africa
-    - 'AN': 17,  # Continental Antarctica
-    - 'AS': 15,  # Continental Asia
-    - 'EU': 13,  # Continental Europe
-    - 'NA': 11,  # Continental North America
-    - 'OC': 16,  # Continental Australia/Oceania
-    - 'SA': 12,  # Continental South America
-
-2. ISO Country Code https://en.wikipedia.org/wiki/ISO_3166-2
-
-3. ISO Country Code Subdevision as per https://en.wikipedia.org/wiki/ISO_3166-2:US   (change the code at the end for the country you are subdividing) * these may not always be supported depending on the provider.
-
-So the example is saying:
-
-- North America - United States - New York:  gets served an "A" record of  111.111.111.1
-- North America - United States - California:   gets served an "A" record of  111.111.111.2
-- Oceania - Australia: Gets served an "A" record of 111.111.111.3
-- Europe: gets an "A" record of 111.111.111.4
-- Everyone else gets an "A" record of 111.111.111.5
-
-### Health Checks
-
-Octodns will automatically set up monitors for each IP and check for a 200 response for **https://<ip_address>/_dns**.
-
-These checks can be configured by adding a `healthcheck` configuration to the record:
-
-```yaml
----
-test:
-  geo:
-    AS:
-      - 1.2.3.4
-    EU:
-      - 2.3.4.5
-  octodns:
-    healthcheck:
-      host: my-host-name
-      path: /dns-health-check
-      port: 443
-      protocol: HTTPS
-```
-
-| Key  | Description | Default |
-|--|--|--|
-| host | FQDN for host header and SNI | - |
-| path | path to check | _dns |
-| port | port to check | 443 |
-| protocol | HTTP/HTTPS | HTTPS |
-
-#### Route53 Healtch Check Options
-
-| Key  | Description | Default |
-|--|--|--|
-| measure_latency | Show latency in AWS console | true |
-
-```yaml
----
-  octodns:
-    healthcheck:
-      host: my-host-name
-      path: /dns-health-check
-      port: 443
-      protocol: HTTPS
-    route53:
-      healthcheck:
-        measure_latency: false
-```
+## Advanced Record Support (GeoDNS, Weighting)
 
+* [Dynamic Records](/docs/dynamic_records.md) - the preferred method for configuring geo-location, weights, and healthcheck based fallback between pools of services.
+* [Geo Records](/docs/geo_records.md) - the original implementation of geo-location based records, now superseded by Dynamic Records (above)
 
 ## Config (`YamlProvider`)
 

octodns/__init__.py

@@ -3,4 +3,4 @@
 from __future__ import absolute_import, division, print_function, \
     unicode_literals
 
-__VERSION__ = '0.9.4'
+__VERSION__ = '0.9.5'

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)

octodns/provider/route53.py

@@ -136,7 +136,7 @@ class _Route53Record(object):
         values_for = getattr(self, '_values_for_{}'.format(self._type))
         self.values = values_for(record)
 
-    def mod(self, action):
+    def mod(self, action, existing_rrsets):
         return {
             'Action': action,
             'ResourceRecordSet': {
@@ -268,7 +268,7 @@ class _Route53DynamicPool(_Route53Record):
                                      self.target_name)
         return '{}-{}'.format(self.pool_name, self.mode)
 
-    def mod(self, action):
+    def mod(self, action, existing_rrsets):
         return {
             'Action': action,
             'ResourceRecordSet': {
@@ -311,7 +311,7 @@ class _Route53DynamicRule(_Route53Record):
     def identifer(self):
         return '{}-{}-{}'.format(self.index, self.pool_name, self.geo)
 
-    def mod(self, action):
+    def mod(self, action, existing_rrsets):
         rrset = {
             'AliasTarget': {
                 'DNSName': self.target_dns_name,
@@ -379,7 +379,21 @@ class _Route53DynamicValue(_Route53Record):
     def identifer(self):
         return '{}-{:03d}'.format(self.pool_name, self.index)
 
-    def mod(self, action):
+    def mod(self, action, existing_rrsets):
+
+        if action == 'DELETE':
+            # When deleting records try and find the original rrset so that
+            # we're 100% sure to have the complete & accurate data (this mostly
+            # ensures we have the right health check id when there's multiple
+            # potential matches)
+            for existing in existing_rrsets:
+                if self.fqdn == existing.get('Name') and \
+                   self.identifer == existing.get('SetIdentifier', None):
+                    return {
+                        'Action': action,
+                        'ResourceRecordSet': existing,
+                    }
+
         return {
             'Action': action,
             'ResourceRecordSet': {
@@ -404,7 +418,7 @@ class _Route53DynamicValue(_Route53Record):
 
 class _Route53GeoDefault(_Route53Record):
 
-    def mod(self, action):
+    def mod(self, action, existing_rrsets):
         return {
             'Action': action,
             'ResourceRecordSet': {
@@ -437,15 +451,31 @@ class _Route53GeoRecord(_Route53Record):
         self.health_check_id = provider.get_health_check_id(record, value,
                                                             creating)
 
-    def mod(self, action):
+    def mod(self, action, existing_rrsets):
         geo = self.geo
+        set_identifier = geo.code
+        fqdn = self.fqdn
+
+        if action == 'DELETE':
+            # When deleting records try and find the original rrset so that
+            # we're 100% sure to have the complete & accurate data (this mostly
+            # ensures we have the right health check id when there's multiple
+            # potential matches)
+            for existing in existing_rrsets:
+                if fqdn == existing.get('Name') and \
+                   set_identifier == existing.get('SetIdentifier', None):
+                    return {
+                        'Action': action,
+                        'ResourceRecordSet': existing,
+                    }
+
         rrset = {
             'Name': self.fqdn,
             'GeoLocation': {
                 'CountryCode': '*'
             },
             'ResourceRecords': [{'Value': v} for v in geo.values],
-            'SetIdentifier': geo.code,
+            'SetIdentifier': set_identifier,
             'TTL': self.ttl,
             'Type': self._type,
         }
@@ -927,11 +957,11 @@ class Route53Provider(BaseProvider):
                       len(zone.records) - before, exists)
         return exists
 
-    def _gen_mods(self, action, records):
+    def _gen_mods(self, action, records, existing_rrsets):
         '''
         Turns `_Route53*`s in to `change_resource_record_sets` `Changes`
         '''
-        return [r.mod(action) for r in records]
+        return [r.mod(action, existing_rrsets) for r in records]
 
     @property
     def health_checks(self):
@@ -1117,15 +1147,15 @@ class Route53Provider(BaseProvider):
         '''
         return _Route53Record.new(self, record, zone_id, creating)
 
-    def _mod_Create(self, change, zone_id):
+    def _mod_Create(self, change, zone_id, existing_rrsets):
         # New is the stuff that needs to be created
         new_records = self._gen_records(change.new, zone_id, creating=True)
         # Now is a good time to clear out any unused health checks since we
         # know what we'll be using going forward
         self._gc_health_checks(change.new, new_records)
-        return self._gen_mods('CREATE', new_records)
+        return self._gen_mods('CREATE', new_records, existing_rrsets)
 
-    def _mod_Update(self, change, zone_id):
+    def _mod_Update(self, change, zone_id, existing_rrsets):
         # See comments in _Route53Record for how the set math is made to do our
         # bidding here.
         existing_records = self._gen_records(change.existing, zone_id,
@@ -1148,18 +1178,18 @@ class Route53Provider(BaseProvider):
             if new_record in existing_records:
                 upserts.add(new_record)
 
-        return self._gen_mods('DELETE', deletes) + \
-            self._gen_mods('CREATE', creates) + \
-            self._gen_mods('UPSERT', upserts)
+        return self._gen_mods('DELETE', deletes, existing_rrsets) + \
+            self._gen_mods('CREATE', creates, existing_rrsets) + \
+            self._gen_mods('UPSERT', upserts, existing_rrsets)
 
-    def _mod_Delete(self, change, zone_id):
+    def _mod_Delete(self, change, zone_id, existing_rrsets):
         # Existing is the thing that needs to be deleted
         existing_records = self._gen_records(change.existing, zone_id,
                                              creating=False)
         # Now is a good time to clear out all the health checks since we know
         # we're done with them
         self._gc_health_checks(change.existing, [])
-        return self._gen_mods('DELETE', existing_records)
+        return self._gen_mods('DELETE', existing_records, existing_rrsets)
 
     def _extra_changes_update_needed(self, record, rrset):
         healthcheck_host = record.healthcheck_host
@@ -1220,15 +1250,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 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 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 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',
@@ -1271,10 +1313,11 @@ class Route53Provider(BaseProvider):
         batch = []
         batch_rs_count = 0
         zone_id = self._get_zone_id(desired.name, True)
+        existing_rrsets = self._load_records(zone_id)
         for c in changes:
             # Generate the mods for this change
             mod_type = getattr(self, '_mod_{}'.format(c.__class__.__name__))
-            mods = mod_type(c, zone_id)
+            mods = mod_type(c, zone_id, existing_rrsets)
 
             # Order our mods to make sure targets exist before alises point to
             # them and we CRUD in the desired order

octodns/source/tinydns.py

@@ -11,6 +11,7 @@ from os import listdir
 from os.path import join
 import logging
 import re
+import textwrap
 
 from ..record import Record
 from ..zone import DuplicateRecordException, SubzoneRecordException
@@ -20,7 +21,7 @@ from .base import BaseSource
 class TinyDnsBaseSource(BaseSource):
     SUPPORTS_GEO = False
     SUPPORTS_DYNAMIC = False
-    SUPPORTS = set(('A', 'CNAME', 'MX', 'NS'))
+    SUPPORTS = set(('A', 'CNAME', 'MX', 'NS', 'TXT', 'AAAA'))
 
     split_re = re.compile(r':+')
 
@@ -45,6 +46,40 @@ class TinyDnsBaseSource(BaseSource):
             'values': values,
         }
 
+    def _data_for_AAAA(self, _type, records):
+        values = []
+        for record in records:
+            # TinyDNS files have the ipv6 address written in full, but with the
+            # colons removed. This inserts a colon every 4th character to make
+            # the address correct.
+            values.append(u":".join(textwrap.wrap(record[0], 4)))
+        try:
+            ttl = records[0][1]
+        except IndexError:
+            ttl = self.default_ttl
+        return {
+            'ttl': ttl,
+            'type': _type,
+            'values': values,
+        }
+
+    def _data_for_TXT(self, _type, records):
+        values = []
+
+        for record in records:
+            new_value = record[0].decode('unicode-escape').replace(";", "\\;")
+            values.append(new_value)
+
+        try:
+            ttl = records[0][1]
+        except IndexError:
+            ttl = self.default_ttl
+        return {
+            'ttl': ttl,
+            'type': _type,
+            'values': values,
+        }
+
     def _data_for_CNAME(self, _type, records):
         first = records[0]
         try:
@@ -104,6 +139,9 @@ class TinyDnsBaseSource(BaseSource):
             'C': 'CNAME',
             '+': 'A',
             '@': 'MX',
+            '\'': 'TXT',
+            '3': 'AAAA',
+            '6': 'AAAA',
         }
         name_re = re.compile(r'((?P<name>.+)\.)?{}$'.format(zone.name[:-1]))
 

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": "@",

tests/test_octodns_provider_route53.py

@@ -12,7 +12,8 @@ from mock import patch
 
 from octodns.record import Create, Delete, Record, Update
 from octodns.provider.route53 import Route53Provider, _Route53GeoDefault, \
-    _Route53GeoRecord, _Route53Record, _mod_keyer, _octal_replace
+    _Route53DynamicValue, _Route53GeoRecord, _Route53Record, _mod_keyer, \
+    _octal_replace
 from octodns.zone import Zone
 
 from helpers import GeoProvider
@@ -874,6 +875,25 @@ class TestRoute53Provider(TestCase):
                                  'CallerReference': ANY,
                              })
 
+        list_resource_record_sets_resp = {
+            'ResourceRecordSets': [{
+                'Name': 'a.unit.tests.',
+                'Type': 'A',
+                'GeoLocation': {
+                    'ContinentCode': 'NA',
+                },
+                'ResourceRecords': [{
+                    'Value': '2.2.3.4',
+                }],
+                'TTL': 61,
+            }],
+            'IsTruncated': False,
+            'MaxItems': '100',
+        }
+        stubber.add_response('list_resource_record_sets',
+                             list_resource_record_sets_resp,
+                             {'HostedZoneId': 'z42'})
+
         stubber.add_response('list_health_checks',
                              {
                                  'HealthChecks': self.health_checks,
@@ -1236,7 +1256,7 @@ class TestRoute53Provider(TestCase):
             'HealthCheckId': '44',
         })
         change = Create(record)
-        provider._mod_Create(change, 'z43')
+        provider._mod_Create(change, 'z43', [])
         stubber.assert_no_pending_responses()
 
         # gc through _mod_Update
@@ -1245,7 +1265,7 @@ class TestRoute53Provider(TestCase):
         })
         # first record is ignored for our purposes, we have to pass something
         change = Update(record, record)
-        provider._mod_Create(change, 'z43')
+        provider._mod_Create(change, 'z43', [])
         stubber.assert_no_pending_responses()
 
         # gc through _mod_Delete, expect 3 to go away, can't check order
@@ -1260,7 +1280,7 @@ class TestRoute53Provider(TestCase):
             'HealthCheckId': ANY,
         })
         change = Delete(record)
-        provider._mod_Delete(change, 'z43')
+        provider._mod_Delete(change, 'z43', [])
         stubber.assert_no_pending_responses()
 
         # gc only AAAA, leave the A's alone
@@ -1653,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': {
@@ -1663,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': '*',
@@ -1682,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.',
@@ -1804,40 +1859,45 @@ class TestRoute53Provider(TestCase):
 
     # _get_test_plan() returns a plan with 11 modifications, 17 RRs
 
+    @patch('octodns.provider.route53.Route53Provider._load_records')
     @patch('octodns.provider.route53.Route53Provider._really_apply')
-    def test_apply_1(self, really_apply_mock):
+    def test_apply_1(self, really_apply_mock, _):
 
         # 18 RRs with max of 19 should only get applied in one call
         provider, plan = self._get_test_plan(19)
         provider.apply(plan)
         really_apply_mock.assert_called_once()
 
+    @patch('octodns.provider.route53.Route53Provider._load_records')
     @patch('octodns.provider.route53.Route53Provider._really_apply')
-    def test_apply_2(self, really_apply_mock):
+    def test_apply_2(self, really_apply_mock, _):
 
         # 18 RRs with max of 17 should only get applied in two calls
         provider, plan = self._get_test_plan(18)
         provider.apply(plan)
         self.assertEquals(2, really_apply_mock.call_count)
 
+    @patch('octodns.provider.route53.Route53Provider._load_records')
     @patch('octodns.provider.route53.Route53Provider._really_apply')
-    def test_apply_3(self, really_apply_mock):
+    def test_apply_3(self, really_apply_mock, _):
 
         # with a max of seven modifications, four calls
         provider, plan = self._get_test_plan(7)
         provider.apply(plan)
         self.assertEquals(4, really_apply_mock.call_count)
 
+    @patch('octodns.provider.route53.Route53Provider._load_records')
     @patch('octodns.provider.route53.Route53Provider._really_apply')
-    def test_apply_4(self, really_apply_mock):
+    def test_apply_4(self, really_apply_mock, _):
 
         # with a max of 11 modifications, two calls
         provider, plan = self._get_test_plan(11)
         provider.apply(plan)
         self.assertEquals(2, really_apply_mock.call_count)
 
+    @patch('octodns.provider.route53.Route53Provider._load_records')
     @patch('octodns.provider.route53.Route53Provider._really_apply')
-    def test_apply_bad(self, really_apply_mock):
+    def test_apply_bad(self, really_apply_mock, _):
 
         # with a max of 1 modifications, fail
         provider, plan = self._get_test_plan(1)
@@ -1939,6 +1999,12 @@ class TestRoute53Provider(TestCase):
             }], [r.data for r in record.dynamic.rules])
 
 
+class DummyProvider(object):
+
+    def get_health_check_id(self, *args, **kwargs):
+        return None
+
+
 class TestRoute53Records(TestCase):
     existing = Zone('unit.tests.', [])
     record_a = Record.new(existing, '', {
@@ -2005,11 +2071,6 @@ class TestRoute53Records(TestCase):
         e = _Route53GeoDefault(None, self.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, self.record_a, 'NA-US',
                               self.record_a.geo['NA-US'], False)
@@ -2029,6 +2090,101 @@ class TestRoute53Records(TestCase):
         e.__repr__()
         f.__repr__()
 
+    def test_dynamic_value_delete(self):
+        provider = DummyProvider()
+        geo = _Route53DynamicValue(provider, self.record_a, 'iad', '2.2.2.2',
+                                   1, 0, False)
+
+        rrset = {
+            'HealthCheckId': 'x12346z',
+            'Name': '_octodns-iad-value.unit.tests.',
+            'ResourceRecords': [{
+                'Value': '2.2.2.2'
+            }],
+            'SetIdentifier': 'iad-000',
+            'TTL': 99,
+            'Type': 'A',
+            'Weight': 1,
+        }
+
+        candidates = [
+            # Empty, will test no SetIdentifier
+            {},
+            # Non-matching
+            {
+                'SetIdentifier': 'not-a-match',
+            },
+            # Same set-id, different name
+            {
+                'Name': 'not-a-match',
+                'SetIdentifier': 'x12346z',
+            },
+            rrset,
+        ]
+
+        # Provide a matching rrset so that we'll just use it for the delete
+        # rathr than building up an almost identical one, note the way we'll
+        # know that we got the one we passed in is that it'll have a
+        # HealthCheckId and one that was created wouldn't since DummyProvider
+        # stubs out the lookup for them
+        mod = geo.mod('DELETE', candidates)
+        self.assertEquals('x12346z', mod['ResourceRecordSet']['HealthCheckId'])
+
+        # If we don't provide the candidate rrsets we get back exactly what we
+        # put in minus the healthcheck
+        rrset['HealthCheckId'] = None
+        mod = geo.mod('DELETE', [])
+        self.assertEquals(rrset, mod['ResourceRecordSet'])
+
+    def test_geo_delete(self):
+        provider = DummyProvider()
+        geo = _Route53GeoRecord(provider, self.record_a, 'NA-US',
+                                self.record_a.geo['NA-US'], False)
+
+        rrset = {
+            'GeoLocation': {
+                'CountryCode': 'US'
+            },
+            'HealthCheckId': 'x12346z',
+            'Name': 'unit.tests.',
+            'ResourceRecords': [{
+                'Value': '2.2.2.2'
+            }, {
+                'Value': '3.3.3.3'
+            }],
+            'SetIdentifier': 'NA-US',
+            'TTL': 99,
+            'Type': 'A'
+        }
+
+        candidates = [
+            # Empty, will test no SetIdentifier
+            {},
+            {
+                'SetIdentifier': 'not-a-match',
+            },
+            # Same set-id, different name
+            {
+                'Name': 'not-a-match',
+                'SetIdentifier': 'x12346z',
+            },
+            rrset,
+        ]
+
+        # Provide a matching rrset so that we'll just use it for the delete
+        # rathr than building up an almost identical one, note the way we'll
+        # know that we got the one we passed in is that it'll have a
+        # HealthCheckId and one that was created wouldn't since DummyProvider
+        # stubs out the lookup for them
+        mod = geo.mod('DELETE', candidates)
+        self.assertEquals('x12346z', mod['ResourceRecordSet']['HealthCheckId'])
+
+        # If we don't provide the candidate rrsets we get back exactly what we
+        # put in minus the healthcheck
+        del rrset['HealthCheckId']
+        mod = geo.mod('DELETE', [])
+        self.assertEquals(rrset, mod['ResourceRecordSet'])
+
     def test_new_dynamic(self):
         provider = Route53Provider('test', 'abc', '123')
 
@@ -2259,7 +2415,7 @@ class TestRoute53Records(TestCase):
                 'Name': '_octodns-eu-central-1-pool.unit.tests.',
                 'SetIdentifier': 'eu-central-1-Secondary-us-east-1',
                 'Type': 'A'}
-        }], [r.mod('CREATE') for r in route53_records])
+        }], [r.mod('CREATE', []) for r in route53_records])
 
         for route53_record in route53_records:
             # Smoke test stringification

tests/test_octodns_source_tinydns.py

@@ -20,7 +20,7 @@ class TestTinyDnsFileSource(TestCase):
     def test_populate_normal(self):
         got = Zone('example.com.', [])
         self.source.populate(got)
-        self.assertEquals(11, len(got.records))
+        self.assertEquals(17, len(got.records))
 
         expected = Zone('example.com.', [])
         for name, data in (
@@ -86,6 +86,36 @@ class TestTinyDnsFileSource(TestCase):
                     'exchange': 'smtp-2-host.example.com.',
                 }]
             }),
+            ('', {
+                'type': 'TXT',
+                'ttl': 300,
+                'value': 'test TXT',
+            }),
+            ('colon', {
+                'type': 'TXT',
+                'ttl': 300,
+                'value': 'test : TXT',
+            }),
+            ('nottl', {
+                'type': 'TXT',
+                'ttl': 3600,
+                'value': 'nottl test TXT',
+            }),
+            ('ipv6-3', {
+                'type': 'AAAA',
+                'ttl': 300,
+                'value': '2a02:1348:017c:d5d0:0024:19ff:fef3:5742',
+            }),
+            ('ipv6-6', {
+                'type': 'AAAA',
+                'ttl': 3600,
+                'value': '2a02:1348:017c:d5d0:0024:19ff:fef3:5743',
+            }),
+            ('semicolon', {
+                'type': 'TXT',
+                'ttl': 300,
+                'value': 'v=DKIM1\\; k=rsa\\; p=blah',
+            }),
         ):
             record = Record.new(expected, name, data)
             expected.add_record(record)
@@ -173,4 +203,4 @@ class TestTinyDnsFileSource(TestCase):
     def test_ignores_subs(self):
         got = Zone('example.com.', ['sub'])
         self.source.populate(got)
-        self.assertEquals(10, len(got.records))
+        self.assertEquals(16, len(got.records))

tests/zones/tinydns/example.com

@@ -46,3 +46,12 @@ Ccname.other.foo:www.other.foo
 +a1.blah-asdf.subtest.com:10.2.3.5
 +a2.blah-asdf.subtest.com:10.2.3.6
 +a3.asdf.subtest.com:10.2.3.7
+
+'example.com:test TXT:300
+'colon.example.com:test \072 TXT:300
+'nottl.example.com:nottl test TXT
+
+3ipv6-3.example.com:2a021348017cd5d0002419fffef35742:300
+6ipv6-6.example.com:2a021348017cd5d0002419fffef35743
+
+'semicolon.example.com:v=DKIM1; k=rsa; p=blah:300