diff --git a/CHANGELOG.md b/CHANGELOG.md index 6777ea9..3d8b441 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -60,7 +60,7 @@ better in the future :fingers_crossed: #### Miscellaneous -* Use a 3rd party lib for nautrual sorting of keys, rather than my old +* Use a 3rd party lib for natural sorting of keys, rather than my old implementation. Sorting can be disabled in the YamlProvider with `enforce_order: False`. * Semi-colon/escaping fixes and improvements. diff --git a/README.md b/README.md index b99e836..0abf9c6 100644 --- a/README.md +++ b/README.md @@ -169,6 +169,7 @@ The above command pulled the existing data out of Route53 and placed the results * 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 +* octoDNS itself supports non-ASCII character sets, but in testing Cloudflare is the only provider where that is currently functional end-to-end. Others have failures either in the client libraries or API calls ## Custom Sources and Providers diff --git a/docs/records.md b/docs/records.md index 82d9a37..75f4d0b 100644 --- a/docs/records.md +++ b/docs/records.md @@ -53,11 +53,11 @@ The geo labels breakdown based on: 1. - 'AF': 14, # Continental Africa - - 'AN': 17, # Continental Antartica - - 'AS': 15, # Contentinal Asia - - 'EU': 13, # Contentinal Europe + - 'AN': 17, # Continental Antarctica + - 'AS': 15, # Continental Asia + - 'EU': 13, # Continental Europe - 'NA': 11, # Continental North America - - 'OC': 16, # Contentinal Austrailia/Oceania + - 'OC': 16, # Continental Australia/Oceania - 'SA': 12, # Continental South America 2. ISO Country Code https://en.wikipedia.org/wiki/ISO_3166-2 diff --git a/octodns/cmds/report.py b/octodns/cmds/report.py index 06a4484..2b32e77 100755 --- a/octodns/cmds/report.py +++ b/octodns/cmds/report.py @@ -65,7 +65,7 @@ def main(): resolver = AsyncResolver(configure=False, num_workers=int(args.num_workers)) if not ip_addr_re.match(server): - server = str(query(server, 'A')[0]) + server = unicode(query(server, 'A')[0]) log.info('server=%s', server) resolver.nameservers = [server] resolver.lifetime = int(args.timeout) @@ -81,12 +81,12 @@ def main(): stdout.write(',') stdout.write(record._type) stdout.write(',') - stdout.write(str(record.ttl)) + stdout.write(unicode(record.ttl)) compare = {} for future in futures: stdout.write(',') try: - answers = [str(r) for r in future.result()] + answers = [unicode(r) for r in future.result()] except (NoAnswer, NoNameservers): answers = ['*no answer*'] except NXDOMAIN: diff --git a/octodns/cmds/sync.py b/octodns/cmds/sync.py index 4dd3e87..60793e7 100755 --- a/octodns/cmds/sync.py +++ b/octodns/cmds/sync.py @@ -26,7 +26,7 @@ def main(): help='Limit sync to the specified zone(s)') # --sources isn't an option here b/c filtering sources out would be super - # dangerous since you could eaily end up with an empty zone and delete + # dangerous since you could easily end up with an empty zone and delete # everything, or even just part of things when there are multiple sources parser.add_argument('--target', default=[], action='append', diff --git a/octodns/manager.py b/octodns/manager.py index d4debf6..027df54 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -51,7 +51,7 @@ class MakeThreadFuture(object): class MainThreadExecutor(object): ''' - Dummy executor that runs things on the main thread during the involcation + Dummy executor that runs things on the main thread during the invocation of submit, but still returns a future object with the result. This allows code to be written to handle async, even in the case where we don't want to use multiple threads/workers and would prefer that things flow as if @@ -361,7 +361,7 @@ class Manager(object): plan = target.plan(zone) if plan is None: - plan = Plan(zone, zone, []) + plan = Plan(zone, zone, [], False) target.apply(plan) def validate_configs(self): diff --git a/octodns/provider/azuredns.py b/octodns/provider/azuredns.py index 1757274..1f14c4b 100644 --- a/octodns/provider/azuredns.py +++ b/octodns/provider/azuredns.py @@ -39,7 +39,7 @@ class _AzureRecord(object): } def __init__(self, resource_group, record, delete=False): - '''Contructor for _AzureRecord. + '''Constructor for _AzureRecord. Notes on Azure records: An Azure record set has the form RecordSet(name=<...>, type=<...>, arecords=[...], aaaa_records, ..) @@ -222,7 +222,7 @@ class AzureProvider(BaseProvider): azuredns: class: octodns.provider.azuredns.AzureProvider client_id: env/AZURE_APPLICATION_ID - key: env/AZURE_AUTHENICATION_KEY + key: env/AZURE_AUTHENTICATION_KEY directory_id: env/AZURE_DIRECTORY_ID sub_id: env/AZURE_SUBSCRIPTION_ID resource_group: 'TestResource1' @@ -322,6 +322,8 @@ class AzureProvider(BaseProvider): :type return: void ''' self.log.debug('populate: name=%s', zone.name) + + exists = False before = len(zone.records) zone_name = zone.name[:len(zone.name) - 1] @@ -331,6 +333,7 @@ class AzureProvider(BaseProvider): _records = set() records = self._dns_client.record_sets.list_by_dns_zone if self._check_zone(zone_name): + exists = True for azrecord in records(self._resource_group, zone_name): if _parse_azure_type(azrecord.type) in self.SUPPORTS: _records.add(azrecord) @@ -344,7 +347,9 @@ class AzureProvider(BaseProvider): record = Record.new(zone, record_name, data, source=self) zone.add_record(record) - self.log.info('populate: found %s records', len(zone.records) - before) + self.log.info('populate: found %s records, exists=%s', + len(zone.records) - before, exists) + return exists def _data_for_A(self, azrecord): return {'values': [ar.ipv4_address for ar in azrecord.arecords]} diff --git a/octodns/provider/base.py b/octodns/provider/base.py index 2d4680f..b0d6ea8 100644 --- a/octodns/provider/base.py +++ b/octodns/provider/base.py @@ -17,7 +17,8 @@ class BaseProvider(BaseSource): delete_pcent_threshold=Plan.MAX_SAFE_DELETE_PCENT): super(BaseProvider, self).__init__(id) self.log.debug('__init__: id=%s, apply_disabled=%s, ' - 'update_pcent_threshold=%d, delete_pcent_threshold=%d', + 'update_pcent_threshold=%.2f' + 'delete_pcent_threshold=%.2f', id, apply_disabled, update_pcent_threshold, @@ -29,14 +30,14 @@ class BaseProvider(BaseSource): def _include_change(self, change): ''' An opportunity for providers to filter out false positives due to - pecularities in their implementation. E.g. minimum TTLs. + peculiarities in their implementation. E.g. minimum TTLs. ''' return True def _extra_changes(self, existing, changes): ''' An opportunity for providers to add extra changes to the plan that are - necessary to update ancilary record data or configure the zone. E.g. + necessary to update ancillary record data or configure the zone. E.g. base NS records. ''' return [] @@ -45,7 +46,12 @@ class BaseProvider(BaseSource): self.log.info('plan: desired=%s', desired.name) existing = Zone(desired.name, desired.sub_zones) - self.populate(existing, target=True, lenient=True) + exists = self.populate(existing, target=True, lenient=True) + if exists is None: + # If your code gets this warning see Source.populate for more + # information + self.log.warn('Provider %s used in target mode did not return ' + 'exists', self.id) # compute the changes at the zone/record level changes = existing.changes(desired, self) @@ -61,11 +67,11 @@ class BaseProvider(BaseSource): extra = self._extra_changes(existing, changes) if extra: self.log.info('plan: extra changes\n %s', '\n ' - .join([str(c) for c in extra])) + .join([unicode(c) for c in extra])) changes += extra if changes: - plan = Plan(existing, desired, changes, + plan = Plan(existing, desired, changes, exists, self.update_pcent_threshold, self.delete_pcent_threshold) self.log.info('plan: %s', plan) diff --git a/octodns/provider/cloudflare.py b/octodns/provider/cloudflare.py index 8df146d..a8dda5d 100644 --- a/octodns/provider/cloudflare.py +++ b/octodns/provider/cloudflare.py @@ -210,13 +210,29 @@ class CloudflareProvider(BaseProvider): return self._zone_records[zone.name] + def _record_for(self, zone, name, _type, records, lenient): + # rewrite Cloudflare proxied records + if self.cdn and records[0]['proxied']: + data = self._data_for_cdn(name, _type, records) + else: + # Cloudflare supports ALIAS semantics with root CNAMEs + if _type == 'CNAME' and name == '': + _type = 'ALIAS' + + data_for = getattr(self, '_data_for_{}'.format(_type)) + data = data_for(_type, records) + + return Record.new(zone, name, data, source=self, lenient=lenient) + def populate(self, zone, target=False, lenient=False): self.log.debug('populate: name=%s, target=%s, lenient=%s', zone.name, target, lenient) + exists = False before = len(zone.records) records = self.zone_records(zone) if records: + exists = True values = defaultdict(lambda: defaultdict(list)) for record in records: name = zone.hostname_from_fqdn(record['name']) @@ -226,21 +242,8 @@ class CloudflareProvider(BaseProvider): for name, types in values.items(): for _type, records in types.items(): - - # rewrite Cloudflare proxied records - if self.cdn and records[0]['proxied']: - data = self._data_for_cdn(name, _type, records) - - else: - # Cloudflare supports ALIAS semantics with root CNAMEs - if _type == 'CNAME' and name == '': - _type = 'ALIAS' - - data_for = getattr(self, '_data_for_{}'.format(_type)) - data = data_for(_type, records) - - record = Record.new(zone, name, data, source=self, - lenient=lenient) + record = self._record_for(zone, name, _type, records, + lenient) # only one rewrite is needed for names where the proxy is # enabled at multiple records with a different type but @@ -252,14 +255,15 @@ class CloudflareProvider(BaseProvider): zone.add_record(record) - self.log.info('populate: found %s records', - len(zone.records) - before) + self.log.info('populate: found %s records, exists=%s', + len(zone.records) - before, exists) + return exists def _include_change(self, change): if isinstance(change, Update): existing = change.existing.data new = change.new.data - new['ttl'] = max(120, new['ttl']) + new['ttl'] = max(self.MIN_TTL, new['ttl']) if new == existing: return False @@ -374,10 +378,6 @@ class CloudflareProvider(BaseProvider): for c in self._gen_contents(change.new) } - # We need a list of keys to consider for diffs, use the first content - # before we muck with anything - keys = existing_contents.values()[0].keys() - # Find the things we need to add adds = [] for k, content in new_contents.items(): @@ -387,22 +387,25 @@ class CloudflareProvider(BaseProvider): except KeyError: adds.append(content) - zone_id = self.zones[change.new.zone.name] + zone = change.new.zone + zone_id = self.zones[zone.name] # Find things we need to remove - name = change.new.fqdn[:-1] + hostname = zone.hostname_from_fqdn(change.new.fqdn[:-1]) _type = change.new._type # OK, work through each record from the zone - for record in self.zone_records(change.new.zone): - if name == record['name'] and _type == record['type']: - # This is match for our name and type, we need to look at - # contents now, build a dict of the relevant keys and vals - content = {} - for k in keys: - content[k] = record[k] - # :-( - if _type in ('CNAME', 'MX', 'NS'): - content['content'] += '.' + for record in self.zone_records(zone): + name = zone.hostname_from_fqdn(record['name']) + # Use the _record_for so that we include all of standard + # conversion logic + r = self._record_for(zone, name, record['type'], [record], True) + if hostname == r.name and _type == r._type: + + # Round trip the single value through a record to contents flow + # to get a consistent _gen_contents result that matches what + # went in to new_contents + content = self._gen_contents(r).next() + # If the hash of that dict isn't in new this record isn't # needed if self._hash_content(content) not in new_contents: diff --git a/octodns/provider/digitalocean.py b/octodns/provider/digitalocean.py index c68e7d6..052a6b9 100644 --- a/octodns/provider/digitalocean.py +++ b/octodns/provider/digitalocean.py @@ -56,7 +56,7 @@ class DigitalOceanClient(object): self._request('POST', '/domains', data={'name': name, 'ip_address': '192.0.2.1'}) - # After the zone is created, immeadiately delete the record + # After the zone is created, immediately delete the record records = self.records(name) for record in records: if record['name'] == '' and record['type'] == 'A': @@ -232,8 +232,10 @@ class DigitalOceanProvider(BaseProvider): source=self, lenient=lenient) zone.add_record(record) - self.log.info('populate: found %s records', - len(zone.records) - before) + exists = zone.name in self._zone_records + self.log.info('populate: found %s records, exists=%s', + len(zone.records) - before, exists) + return exists def _params_for_multiple(self, record): for value in record.values: diff --git a/octodns/provider/dnsimple.py b/octodns/provider/dnsimple.py index 43b5b9b..a5b78a8 100644 --- a/octodns/provider/dnsimple.py +++ b/octodns/provider/dnsimple.py @@ -160,7 +160,7 @@ class DnsimpleProvider(BaseProvider): record['content'].split(' ', 5) except ValueError: # their api will let you create invalid records, this - # essnetially handles that by ignoring them for values + # essentially handles that by ignoring them for values # purposes. That will cause updates to happen to delete them if # they shouldn't exist or update them if they're wrong continue @@ -272,8 +272,10 @@ class DnsimpleProvider(BaseProvider): source=self, lenient=lenient) zone.add_record(record) - self.log.info('populate: found %s records', - len(zone.records) - before) + exists = zone.name in self._zone_records + self.log.info('populate: found %s records, exists=%s', + len(zone.records) - before, exists) + return exists def _params_for_multiple(self, record): for value in record.values: diff --git a/octodns/provider/dyn.py b/octodns/provider/dyn.py index 010fd31..2799368 100644 --- a/octodns/provider/dyn.py +++ b/octodns/provider/dyn.py @@ -40,7 +40,7 @@ class _CachingDynZone(DynZone): cls.log.debug('get: fetched') except DynectGetError: if not create: - cls.log.debug("get: does't exist") + cls.log.debug("get: doesn't exist") return None # this value shouldn't really matter, it's not tied to # whois or anything @@ -129,11 +129,11 @@ class DynProvider(BaseProvider): REGION_CODES = { 'NA': 11, # Continental North America 'SA': 12, # Continental South America - 'EU': 13, # Contentinal Europe + 'EU': 13, # Continental Europe 'AF': 14, # Continental Africa - 'AS': 15, # Contentinal Asia - 'OC': 16, # Contentinal Austrailia/Oceania - 'AN': 17, # Continental Antartica + 'AS': 15, # Continental Asia + 'OC': 16, # Continental Australia/Oceania + 'AN': 17, # Continental Antarctica } _sess_create_lock = Lock() @@ -166,7 +166,7 @@ class DynProvider(BaseProvider): if DynectSession.get_session() is None: # We need to create a new session for this thread and DynectSession # creation is not thread-safe so we have to do the locking. If we - # don't and multiple sessions start creattion before the the first + # don't and multiple sessions start creation before the the first # has finished (long time b/c it makes http calls) the subsequent # creates will blow away DynectSession._instances, potentially # multiple times if there are multiple creates in flight. Only the @@ -291,7 +291,7 @@ class DynProvider(BaseProvider): try: fqdn, _type = td.label.split(':', 1) except ValueError as e: - self.log.warn("Failed to load TraficDirector '%s': %s", + self.log.warn("Failed to load TrafficDirector '%s': %s", td.label, e.message) continue tds[fqdn][_type] = td @@ -353,6 +353,7 @@ class DynProvider(BaseProvider): self.log.debug('populate: name=%s, target=%s, lenient=%s', zone.name, target, lenient) + exists = False before = len(zone.records) self._check_dyn_sess() @@ -360,10 +361,12 @@ class DynProvider(BaseProvider): td_records = set() if self.traffic_directors_enabled: td_records = self._populate_traffic_directors(zone) + exists = True dyn_zone = _CachingDynZone.get(zone.name[:-1]) if dyn_zone: + exists = True values = defaultdict(lambda: defaultdict(list)) for _type, records in dyn_zone.get_all_records().items(): if _type == 'soa_records': @@ -382,8 +385,9 @@ class DynProvider(BaseProvider): if record not in td_records: zone.add_record(record) - self.log.info('populate: found %s records', - len(zone.records) - before) + self.log.info('populate: found %s records, exists=%s', + len(zone.records) - before, exists) + return exists def _kwargs_for_A(self, record): return [{ diff --git a/octodns/provider/googlecloud.py b/octodns/provider/googlecloud.py index c2cb30f..b762879 100644 --- a/octodns/provider/googlecloud.py +++ b/octodns/provider/googlecloud.py @@ -204,11 +204,14 @@ class GoogleCloudProvider(BaseProvider): self.log.debug('populate: name=%s, target=%s, lenient=%s', zone.name, target, lenient) + + exists = False before = len(zone.records) gcloud_zone = self.gcloud_zones.get(zone.name) if gcloud_zone: + exists = True for gcloud_record in self._get_gcloud_records(gcloud_zone): if gcloud_record.record_type.upper() not in self.SUPPORTS: continue @@ -229,7 +232,9 @@ class GoogleCloudProvider(BaseProvider): record = Record.new(zone, record_name, data, source=self) zone.add_record(record) - self.log.info('populate: found %s records', len(zone.records) - before) + self.log.info('populate: found %s records, exists=%s', + len(zone.records) - before, exists) + return exists def _data_for_A(self, gcloud_record): return { diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py index 80797d8..2e0ade7 100644 --- a/octodns/provider/ns1.py +++ b/octodns/provider/ns1.py @@ -75,9 +75,9 @@ class Ns1Provider(BaseProvider): else: values.extend(answer['answer']) codes.append([]) - values = [str(x) for x in values] + values = [unicode(x) for x in values] geo = OrderedDict( - {str(k): [str(x) for x in v] for k, v in geo.items()} + {unicode(k): [unicode(x) for x in v] for k, v in geo.items()} ) data['values'] = values data['geo'] = geo @@ -190,11 +190,13 @@ class Ns1Provider(BaseProvider): nsone_zone = self._client.loadZone(zone.name[:-1]) records = nsone_zone.data['records'] geo_records = nsone_zone.search(has_geo=True) + exists = True except ResourceException as e: if e.message != self.ZONE_NOT_FOUND_MESSAGE: raise records = [] geo_records = [] + exists = False before = len(zone.records) # geo information isn't returned from the main endpoint, so we need @@ -208,8 +210,9 @@ class Ns1Provider(BaseProvider): source=self, lenient=lenient) zone_hash[(_type, name)] = record [zone.add_record(r) for r in zone_hash.values()] - self.log.info('populate: found %s records', - len(zone.records) - before) + self.log.info('populate: found %s records, exists=%s', + len(zone.records) - before, exists) + return exists def _params_for_A(self, record): params = {'answers': record.values, 'ttl': record.ttl} diff --git a/octodns/provider/ovh.py b/octodns/provider/ovh.py index 4c10646..7bb3feb 100644 --- a/octodns/provider/ovh.py +++ b/octodns/provider/ovh.py @@ -11,6 +11,7 @@ import logging from collections import defaultdict import ovh +from ovh import ResourceNotFoundError from octodns.record import Record from .base import BaseProvider @@ -33,6 +34,7 @@ class OvhProvider(BaseProvider): """ SUPPORTS_GEO = False + ZONE_NOT_FOUND_MESSAGE = 'This service does not exist' # This variable is also used in populate method to filter which OVH record # types are supported by octodns @@ -57,7 +59,14 @@ class OvhProvider(BaseProvider): self.log.debug('populate: name=%s, target=%s, lenient=%s', zone.name, target, lenient) zone_name = zone.name[:-1] - records = self.get_records(zone_name=zone_name) + try: + records = self.get_records(zone_name=zone_name) + exists = True + except ResourceNotFoundError as e: + if e.message != self.ZONE_NOT_FOUND_MESSAGE: + raise + exists = False + records = [] values = defaultdict(lambda: defaultdict(list)) for record in records: @@ -75,8 +84,9 @@ class OvhProvider(BaseProvider): source=self, lenient=lenient) zone.add_record(record) - self.log.info('populate: found %s records', - len(zone.records) - before) + self.log.info('populate: found %s records, exists=%s', + len(zone.records) - before, exists) + return exists def _apply(self, plan): desired = plan.desired diff --git a/octodns/provider/plan.py b/octodns/provider/plan.py index 3e86826..bae244f 100644 --- a/octodns/provider/plan.py +++ b/octodns/provider/plan.py @@ -21,12 +21,13 @@ class Plan(object): MAX_SAFE_DELETE_PCENT = .3 MIN_EXISTING_RECORDS = 10 - def __init__(self, existing, desired, changes, + def __init__(self, existing, desired, changes, exists, update_pcent_threshold=MAX_SAFE_UPDATE_PCENT, delete_pcent_threshold=MAX_SAFE_DELETE_PCENT): self.existing = existing self.desired = desired self.changes = changes + self.exists = exists self.update_pcent_threshold = update_pcent_threshold self.delete_pcent_threshold = delete_pcent_threshold @@ -60,17 +61,17 @@ class Plan(object): delete_pcent = self.change_counts['Delete'] / existing_record_count if update_pcent > self.update_pcent_threshold: - raise UnsafePlan('Too many updates, {} is over {} percent' + raise UnsafePlan('Too many updates, {:.2f} is over {:.2f} %' '({}/{})'.format( - update_pcent, - self.MAX_SAFE_UPDATE_PCENT * 100, + update_pcent * 100, + self.update_pcent_threshold * 100, self.change_counts['Update'], existing_record_count)) if delete_pcent > self.delete_pcent_threshold: - raise UnsafePlan('Too many deletes, {} is over {} percent' + raise UnsafePlan('Too many deletes, {:.2f} is over {:.2f} %' '({}/{})'.format( - delete_pcent, - self.MAX_SAFE_DELETE_PCENT * 100, + delete_pcent * 100, + self.delete_pcent_threshold * 100, self.change_counts['Delete'], existing_record_count)) @@ -123,6 +124,12 @@ class PlanLogger(_PlanOutput): buf.write(' (') buf.write(target) buf.write(')\n* ') + + if plan.exists is False: + buf.write('Create ') + buf.write(str(plan.desired)) + buf.write('\n* ') + for change in plan.changes: buf.write(change.__repr__(leader='* ')) buf.write('\n* ') @@ -140,11 +147,11 @@ class PlanLogger(_PlanOutput): def _value_stringifier(record, sep): try: - values = [str(v) for v in record.values] + values = [unicode(v) for v in record.values] except AttributeError: values = [record.value] for code, gv in sorted(getattr(record, 'geo', {}).items()): - vs = ', '.join([str(v) for v in gv.values]) + vs = ', '.join([unicode(v) for v in gv.values]) values.append('{}: {}'.format(code, vs)) return sep.join(values) @@ -168,6 +175,11 @@ class PlanMarkdown(_PlanOutput): fh.write('| Operation | Name | Type | TTL | Value | Source |\n' '|--|--|--|--|--|--|\n') + if plan.exists is False: + fh.write('| Create | ') + fh.write(str(plan.desired)) + fh.write(' | | | | |\n') + for change in plan.changes: existing = change.existing new = change.new @@ -181,7 +193,7 @@ class PlanMarkdown(_PlanOutput): fh.write(' | ') # TTL if existing: - fh.write(str(existing.ttl)) + fh.write(unicode(existing.ttl)) fh.write(' | ') fh.write(_value_stringifier(existing, '; ')) fh.write(' | |\n') @@ -189,15 +201,16 @@ class PlanMarkdown(_PlanOutput): fh.write('| | | | ') if new: - fh.write(str(new.ttl)) + fh.write(unicode(new.ttl)) fh.write(' | ') fh.write(_value_stringifier(new, '; ')) fh.write(' | ') - fh.write(new.source.id) + if new.source: + fh.write(new.source.id) fh.write(' |\n') fh.write('\nSummary: ') - fh.write(str(plan)) + fh.write(unicode(plan)) fh.write('\n\n') else: fh.write('## No changes were planned\n') @@ -229,6 +242,11 @@ class PlanHtml(_PlanOutput): ''') + if plan.exists is False: + fh.write(' \n Create\n ') + fh.write(str(plan.desired)) + fh.write('\n \n') + for change in plan.changes: existing = change.existing new = change.new @@ -243,7 +261,7 @@ class PlanHtml(_PlanOutput): # TTL if existing: fh.write(' ') - fh.write(str(existing.ttl)) + fh.write(unicode(existing.ttl)) fh.write('\n ') fh.write(_value_stringifier(existing, '
')) fh.write('\n \n \n') @@ -252,15 +270,16 @@ class PlanHtml(_PlanOutput): if new: fh.write(' ') - fh.write(str(new.ttl)) + fh.write(unicode(new.ttl)) fh.write('\n ') fh.write(_value_stringifier(new, '
')) fh.write('\n ') - fh.write(new.source.id) + if new.source: + fh.write(new.source.id) fh.write('\n \n') fh.write(' \n Summary: ') - fh.write(str(plan)) + fh.write(unicode(plan)) fh.write('\n \n\n') else: fh.write('No changes were planned') diff --git a/octodns/provider/powerdns.py b/octodns/provider/powerdns.py index 4527f8e..f6f218e 100644 --- a/octodns/provider/powerdns.py +++ b/octodns/provider/powerdns.py @@ -178,7 +178,7 @@ class PowerDnsBaseProvider(BaseProvider): raise Exception('PowerDNS unauthorized host={}' .format(self.host)) elif e.response.status_code == 422: - # 422 means powerdns doesn't know anything about the requsted + # 422 means powerdns doesn't know anything about the requested # domain. We'll just ignore it here and leave the zone # untouched. pass @@ -187,8 +187,10 @@ class PowerDnsBaseProvider(BaseProvider): raise before = len(zone.records) + exists = False if resp: + exists = True for rrset in resp.json()['rrsets']: _type = rrset['type'] if _type == 'SOA': @@ -199,8 +201,9 @@ class PowerDnsBaseProvider(BaseProvider): source=self, lenient=lenient) zone.add_record(record) - self.log.info('populate: found %s records', - len(zone.records) - before) + self.log.info('populate: found %s records, exists=%s', + len(zone.records) - before, exists) + return exists def _records_for_multiple(self, record): return [{'content': v, 'disabled': False} @@ -294,8 +297,8 @@ class PowerDnsBaseProvider(BaseProvider): return [] # sorting mostly to make things deterministic for testing, but in - # theory it let us find what we're after quickier (though sorting would - # ve more exepensive.) + # theory it let us find what we're after quicker (though sorting would + # be more expensive.) for record in sorted(existing.records): if record == ns: # We've found the top-level NS record, return any changes @@ -341,7 +344,7 @@ class PowerDnsBaseProvider(BaseProvider): e.response.text) raise self.log.info('_apply: creating zone=%s', desired.name) - # 422 means powerdns doesn't know anything about the requsted + # 422 means powerdns doesn't know anything about the requested # domain. We'll try to create it with the correct records instead # of update. Hopefully all the mods are creates :-) data = { diff --git a/octodns/provider/rackspace.py b/octodns/provider/rackspace.py index 12b2c54..35e27dc 100644 --- a/octodns/provider/rackspace.py +++ b/octodns/provider/rackspace.py @@ -130,17 +130,9 @@ class RackspaceProvider(BaseProvider): def _delete(self, path, data=None): return self._request('DELETE', path, data=data) - @staticmethod - def _as_unicode(s, codec): - if not isinstance(s, unicode): - return unicode(s, codec) - return s - @classmethod def _key_for_record(cls, rs_record): - return cls._as_unicode(rs_record['type'], 'ascii'), \ - cls._as_unicode(rs_record['name'], 'utf-8'), \ - cls._as_unicode(rs_record['data'], 'utf-8') + return rs_record['type'], rs_record['name'], rs_record['data'] def _data_for_multiple(self, rrset): return { @@ -208,7 +200,7 @@ class RackspaceProvider(BaseProvider): raise Exception('Rackspace request unauthorized') elif e.response.status_code == 404: # Zone not found leaves the zone empty instead of failing. - return + return False raise before = len(zone.records) @@ -225,8 +217,9 @@ class RackspaceProvider(BaseProvider): source=self) zone.add_record(record) - self.log.info('populate: found %s records', + self.log.info('populate: found %s records, exists=True', len(zone.records) - before) + return True def _group_records(self, all_records): records = defaultdict(lambda: defaultdict(list)) diff --git a/octodns/provider/route53.py b/octodns/provider/route53.py index 5bda074..3333f4e 100644 --- a/octodns/provider/route53.py +++ b/octodns/provider/route53.py @@ -61,7 +61,7 @@ class _Route53Record(object): # 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. + # Values are ignored. This is useful when computing diffs/changes. def __hash__(self): 'sub-classes should never use this method' @@ -451,9 +451,11 @@ class Route53Provider(BaseProvider): target, lenient) before = len(zone.records) + exists = False zone_id = self._get_zone_id(zone.name) if zone_id: + exists = True records = defaultdict(lambda: defaultdict(list)) for rrset in self._load_records(zone_id): record_name = zone.hostname_from_fqdn(rrset['Name']) @@ -483,8 +485,9 @@ class Route53Provider(BaseProvider): lenient=lenient) zone.add_record(record) - self.log.info('populate: found %s records', - len(zone.records) - before) + self.log.info('populate: found %s records, exists=%s', + len(zone.records) - before, exists) + return exists def _gen_mods(self, action, records): ''' @@ -679,7 +682,7 @@ class Route53Provider(BaseProvider): .get('CountryCode', False) == '*': # it's a default record continue - # we expect a healtcheck now + # we expect a healthcheck now try: health_check_id = rrset['HealthCheckId'] caller_ref = \ @@ -730,7 +733,7 @@ class Route53Provider(BaseProvider): batch_rs_count) # send the batch self._really_apply(batch, zone_id) - # start a new batch with the lefovers + # start a new batch with the leftovers batch = mods batch_rs_count = mods_rs_count diff --git a/octodns/provider/yaml.py b/octodns/provider/yaml.py index 752e793..0241d50 100644 --- a/octodns/provider/yaml.py +++ b/octodns/provider/yaml.py @@ -52,7 +52,7 @@ class YamlProvider(BaseProvider): if target: # When acting as a target we ignore any existing records so that we # create a completely new copy - return + return False before = len(zone.records) filename = join(self.directory, '{}yaml'.format(zone.name)) @@ -69,8 +69,9 @@ class YamlProvider(BaseProvider): lenient=lenient) zone.add_record(record) - self.log.info('populate: found %s records', + self.log.info('populate: found %s records, exists=False', len(zone.records) - before) + return False def _apply(self, plan): desired = plan.desired diff --git a/octodns/record.py b/octodns/record.py index b608ef6..6d1e25b 100644 --- a/octodns/record.py +++ b/octodns/record.py @@ -122,7 +122,7 @@ class Record(object): self.__class__.__name__, name) self.zone = zone # force everything lower-case just to be safe - self.name = str(name).lower() if name else name + self.name = unicode(name).lower() if name else name self.source = source self.ttl = int(data['ttl']) @@ -151,7 +151,7 @@ class Record(object): # NOTE: we're using __hash__ and __cmp__ methods that consider Records # equivalent if they have the same name & _type. Values are ignored. This - # is usful when computing diffs/changes. + # is useful when computing diffs/changes. def __hash__(self): return '{}:{}'.format(self.name, self._type).__hash__() @@ -274,7 +274,8 @@ class _ValuesMixin(object): return ret def __repr__(self): - values = "['{}']".format("', '".join([str(v) for v in self.values])) + values = "['{}']".format("', '".join([unicode(v) + for v in self.values])) return '<{} {} {}, {}, {}>'.format(self.__class__.__name__, self._type, self.ttl, self.fqdn, values) diff --git a/octodns/source/base.py b/octodns/source/base.py index 4ace09f..ee33619 100644 --- a/octodns/source/base.py +++ b/octodns/source/base.py @@ -22,7 +22,7 @@ class BaseSource(object): def populate(self, zone, target=False, lenient=False): ''' - Loads all zones the provider knows about + Loads all records the provider knows about for the provided zone When `target` is True the populate call is being made to load the current state of the provider. @@ -31,6 +31,9 @@ class BaseSource(object): 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 ;. + + When target is True (loading current state) this method should return + True if the zone exists or False if it does not. ''' raise NotImplementedError('Abstract base class, populate method ' 'missing') diff --git a/octodns/zone.py b/octodns/zone.py index bbc38d0..bed3a59 100644 --- a/octodns/zone.py +++ b/octodns/zone.py @@ -37,10 +37,10 @@ class Zone(object): if not name[-1] == '.': raise Exception('Invalid zone name {}, missing ending dot' .format(name)) - # Force everyting to lowercase just to be safe - self.name = str(name).lower() if name else name + # Force everything to lowercase just to be safe + self.name = unicode(name).lower() if name else name self.sub_zones = sub_zones - # We're grouping by node, it allows us to efficently search for + # We're grouping by node, it allows us to efficiently search for # duplicates and detect when CNAMEs co-exist with other records self._records = defaultdict(set) # optional leading . to match empty hostname diff --git a/script/release b/script/release index d8fabf2..bcc0ba3 100755 --- a/script/release +++ b/script/release @@ -11,4 +11,4 @@ git tag -s v$VERSION -m "Release $VERSION" git push origin v$VERSION echo "Tagged and pushed v$VERSION" python setup.py sdist upload -echo "Updloaded $VERSION" +echo "Uploaded $VERSION" diff --git a/tests/test_octodns_plan.py b/tests/test_octodns_plan.py index ea35243..7d849be 100644 --- a/tests/test_octodns_plan.py +++ b/tests/test_octodns_plan.py @@ -16,15 +16,6 @@ from octodns.zone import Zone from helpers import SimpleProvider -class TestPlanLogger(TestCase): - - def test_invalid_level(self): - with self.assertRaises(Exception) as ctx: - PlanLogger('invalid', 'not-a-level') - self.assertEquals('Unsupported level: not-a-level', - ctx.exception.message) - - simple = SimpleProvider() zone = Zone('unit.tests.', []) existing = Record.new(zone, 'a', { @@ -48,15 +39,45 @@ create = Create(Record.new(zone, 'b', { 'type': 'CNAME', 'value': 'foo.unit.tests.' }, simple)) +create2 = Create(Record.new(zone, 'c', { + 'ttl': 60, + 'type': 'CNAME', + 'value': 'foo.unit.tests.' +})) update = Update(existing, new) delete = Delete(new) -changes = [create, delete, update] +changes = [create, create2, delete, update] plans = [ - (simple, Plan(zone, zone, changes)), - (simple, Plan(zone, zone, changes)), + (simple, Plan(zone, zone, changes, True)), + (simple, Plan(zone, zone, changes, False)), ] +class TestPlanLogger(TestCase): + + def test_invalid_level(self): + with self.assertRaises(Exception) as ctx: + PlanLogger('invalid', 'not-a-level') + self.assertEquals('Unsupported level: not-a-level', + ctx.exception.message) + + def test_create(self): + + class MockLogger(object): + + def __init__(self): + self.out = StringIO() + + def log(self, level, msg): + self.out.write(msg) + + log = MockLogger() + PlanLogger('logger').run(log, plans) + out = log.out.getvalue() + self.assertTrue('Summary: Creates=2, Updates=1, ' + 'Deletes=1, Existing Records=0' in out) + + class TestPlanHtml(TestCase): log = getLogger('TestPlanHtml') @@ -69,7 +90,7 @@ class TestPlanHtml(TestCase): out = StringIO() PlanHtml('html').run(plans, fh=out) out = out.getvalue() - self.assertTrue(' Summary: Creates=1, Updates=1, ' + self.assertTrue(' Summary: Creates=2, Updates=1, ' 'Deletes=1, Existing Records=0' in out) diff --git a/tests/test_octodns_provider_azuredns.py b/tests/test_octodns_provider_azuredns.py index 598fe48..9784945 100644 --- a/tests/test_octodns_provider_azuredns.py +++ b/tests/test_octodns_provider_azuredns.py @@ -302,7 +302,8 @@ class TestAzureDnsProvider(TestCase): record_list = provider._dns_client.record_sets.list_by_dns_zone record_list.return_value = rs - provider.populate(zone) + exists = provider.populate(zone) + self.assertTrue(exists) self.assertEquals(len(zone.records), 16) @@ -338,8 +339,10 @@ class TestAzureDnsProvider(TestCase): changes.append(Create(i)) deletes.append(Delete(i)) - self.assertEquals(13, provider.apply(Plan(None, zone, changes))) - self.assertEquals(13, provider.apply(Plan(zone, zone, deletes))) + self.assertEquals(13, provider.apply(Plan(None, zone, + changes, True))) + self.assertEquals(13, provider.apply(Plan(zone, zone, + deletes, True))) def test_create_zone(self): provider = self._get_provider() @@ -354,7 +357,8 @@ class TestAzureDnsProvider(TestCase): _get = provider._dns_client.zones.get _get.side_effect = CloudError(Mock(status=404), err_msg) - self.assertEquals(13, provider.apply(Plan(None, desired, changes))) + self.assertEquals(13, provider.apply(Plan(None, desired, changes, + True))) def test_check_zone_no_create(self): provider = self._get_provider() @@ -374,6 +378,7 @@ class TestAzureDnsProvider(TestCase): _get = provider._dns_client.zones.get _get.side_effect = CloudError(Mock(status=404), err_msg) - provider.populate(Zone('unit3.test.', [])) + exists = provider.populate(Zone('unit3.test.', [])) + self.assertFalse(exists) self.assertEquals(len(zone.records), 0) diff --git a/tests/test_octodns_provider_base.py b/tests/test_octodns_provider_base.py index 472b008..64c0377 100644 --- a/tests/test_octodns_provider_base.py +++ b/tests/test_octodns_provider_base.py @@ -63,14 +63,14 @@ class TestBaseProvider(TestCase): zone = Zone('unit.tests.', []) with self.assertRaises(NotImplementedError) as ctx: - HasSupportsGeo('hassupportesgeo').populate(zone) + HasSupportsGeo('hassupportsgeo').populate(zone) self.assertEquals('Abstract base class, SUPPORTS property missing', ctx.exception.message) class HasSupports(HasSupportsGeo): SUPPORTS = set(('A',)) with self.assertRaises(NotImplementedError) as ctx: - HasSupports('hassupportes').populate(zone) + HasSupports('hassupports').populate(zone) self.assertEquals('Abstract base class, populate method missing', ctx.exception.message) @@ -94,7 +94,7 @@ class TestBaseProvider(TestCase): 'value': '1.2.3.4' })) - self.assertTrue(HasSupports('hassupportesgeo') + self.assertTrue(HasSupports('hassupportsgeo') .supports(list(zone.records)[0])) plan = HasPopulate('haspopulate').plan(zone) @@ -153,7 +153,7 @@ class TestBaseProvider(TestCase): def test_safe_none(self): # No changes is safe - Plan(None, None, []).raise_if_unsafe() + Plan(None, None, [], True).raise_if_unsafe() def test_safe_creates(self): # Creates are safe when existing records is under MIN_EXISTING_RECORDS @@ -164,7 +164,8 @@ class TestBaseProvider(TestCase): 'type': 'A', 'value': '1.2.3.4', }) - Plan(zone, zone, [Create(record) for i in range(10)]).raise_if_unsafe() + Plan(zone, zone, [Create(record) for i in range(10)], True) \ + .raise_if_unsafe() def test_safe_min_existing_creates(self): # Creates are safe when existing records is over MIN_EXISTING_RECORDS @@ -177,13 +178,14 @@ class TestBaseProvider(TestCase): }) for i in range(int(Plan.MIN_EXISTING_RECORDS)): - zone.add_record(Record.new(zone, str(i), { + zone.add_record(Record.new(zone, unicode(i), { 'ttl': 60, 'type': 'A', 'value': '2.3.4.5' })) - Plan(zone, zone, [Create(record) for i in range(10)]).raise_if_unsafe() + Plan(zone, zone, [Create(record) for i in range(10)], True) \ + .raise_if_unsafe() def test_safe_no_existing(self): # existing records fewer than MIN_EXISTING_RECORDS is safe @@ -195,7 +197,7 @@ class TestBaseProvider(TestCase): }) updates = [Update(record, record), Update(record, record)] - Plan(zone, zone, updates).raise_if_unsafe() + Plan(zone, zone, updates, True).raise_if_unsafe() def test_safe_updates_min_existing(self): # MAX_SAFE_UPDATE_PCENT+1 fails when more @@ -208,7 +210,7 @@ class TestBaseProvider(TestCase): }) for i in range(int(Plan.MIN_EXISTING_RECORDS)): - zone.add_record(Record.new(zone, str(i), { + zone.add_record(Record.new(zone, unicode(i), { 'ttl': 60, 'type': 'A', 'value': '2.3.4.5' @@ -219,7 +221,7 @@ class TestBaseProvider(TestCase): Plan.MAX_SAFE_UPDATE_PCENT) + 1)] with self.assertRaises(UnsafePlan) as ctx: - Plan(zone, zone, changes).raise_if_unsafe() + Plan(zone, zone, changes, True).raise_if_unsafe() self.assertTrue('Too many updates' in ctx.exception.message) @@ -234,7 +236,7 @@ class TestBaseProvider(TestCase): }) for i in range(int(Plan.MIN_EXISTING_RECORDS)): - zone.add_record(Record.new(zone, str(i), { + zone.add_record(Record.new(zone, unicode(i), { 'ttl': 60, 'type': 'A', 'value': '2.3.4.5' @@ -243,7 +245,7 @@ class TestBaseProvider(TestCase): for i in range(int(Plan.MIN_EXISTING_RECORDS * Plan.MAX_SAFE_UPDATE_PCENT))] - Plan(zone, zone, changes).raise_if_unsafe() + Plan(zone, zone, changes, True).raise_if_unsafe() def test_safe_deletes_min_existing(self): # MAX_SAFE_DELETE_PCENT+1 fails when more @@ -256,7 +258,7 @@ class TestBaseProvider(TestCase): }) for i in range(int(Plan.MIN_EXISTING_RECORDS)): - zone.add_record(Record.new(zone, str(i), { + zone.add_record(Record.new(zone, unicode(i), { 'ttl': 60, 'type': 'A', 'value': '2.3.4.5' @@ -267,7 +269,7 @@ class TestBaseProvider(TestCase): Plan.MAX_SAFE_DELETE_PCENT) + 1)] with self.assertRaises(UnsafePlan) as ctx: - Plan(zone, zone, changes).raise_if_unsafe() + Plan(zone, zone, changes, True).raise_if_unsafe() self.assertTrue('Too many deletes' in ctx.exception.message) @@ -282,7 +284,7 @@ class TestBaseProvider(TestCase): }) for i in range(int(Plan.MIN_EXISTING_RECORDS)): - zone.add_record(Record.new(zone, str(i), { + zone.add_record(Record.new(zone, unicode(i), { 'ttl': 60, 'type': 'A', 'value': '2.3.4.5' @@ -291,7 +293,7 @@ class TestBaseProvider(TestCase): for i in range(int(Plan.MIN_EXISTING_RECORDS * Plan.MAX_SAFE_DELETE_PCENT))] - Plan(zone, zone, changes).raise_if_unsafe() + Plan(zone, zone, changes, True).raise_if_unsafe() def test_safe_updates_min_existing_override(self): safe_pcent = .4 @@ -305,7 +307,7 @@ class TestBaseProvider(TestCase): }) for i in range(int(Plan.MIN_EXISTING_RECORDS)): - zone.add_record(Record.new(zone, str(i), { + zone.add_record(Record.new(zone, unicode(i), { 'ttl': 60, 'type': 'A', 'value': '2.3.4.5' @@ -316,7 +318,7 @@ class TestBaseProvider(TestCase): safe_pcent) + 1)] with self.assertRaises(UnsafePlan) as ctx: - Plan(zone, zone, changes, + Plan(zone, zone, changes, True, update_pcent_threshold=safe_pcent).raise_if_unsafe() self.assertTrue('Too many updates' in ctx.exception.message) @@ -333,7 +335,7 @@ class TestBaseProvider(TestCase): }) for i in range(int(Plan.MIN_EXISTING_RECORDS)): - zone.add_record(Record.new(zone, str(i), { + zone.add_record(Record.new(zone, unicode(i), { 'ttl': 60, 'type': 'A', 'value': '2.3.4.5' @@ -344,7 +346,7 @@ class TestBaseProvider(TestCase): safe_pcent) + 1)] with self.assertRaises(UnsafePlan) as ctx: - Plan(zone, zone, changes, + Plan(zone, zone, changes, True, delete_pcent_threshold=safe_pcent).raise_if_unsafe() self.assertTrue('Too many deletes' in ctx.exception.message) diff --git a/tests/test_octodns_provider_cloudflare.py b/tests/test_octodns_provider_cloudflare.py index 7bb1b6a..7462b9f 100644 --- a/tests/test_octodns_provider_cloudflare.py +++ b/tests/test_octodns_provider_cloudflare.py @@ -166,6 +166,7 @@ class TestCloudflareProvider(TestCase): plan = provider.plan(self.expected) self.assertEquals(12, len(plan.changes)) self.assertEquals(12, provider.apply(plan)) + self.assertFalse(plan.exists) provider._request.assert_has_calls([ # created the domain @@ -285,6 +286,7 @@ class TestCloudflareProvider(TestCase): # only see the delete & ttl update, below min-ttl is filtered out self.assertEquals(2, len(plan.changes)) self.assertEquals(2, provider.apply(plan)) + self.assertTrue(plan.exists) # recreate for update, and deletes for the 2 parts of the other provider._request.assert_has_calls([ call('PUT', '/zones/ff12ab34cd5611334422ab3322997650/dns_records/' @@ -366,7 +368,7 @@ class TestCloudflareProvider(TestCase): 'values': ['2.2.2.2', '3.3.3.3', '4.4.4.4'], }) change = Update(existing, new) - plan = Plan(zone, zone, [change]) + plan = Plan(zone, zone, [change], True) provider._apply(plan) provider._request.assert_has_calls([ @@ -451,7 +453,7 @@ class TestCloudflareProvider(TestCase): 'value': 'ns2.foo.bar.', }) change = Update(existing, new) - plan = Plan(zone, zone, [change]) + plan = Plan(zone, zone, [change], True) provider._apply(plan) provider._request.assert_has_calls([ @@ -599,7 +601,8 @@ class TestCloudflareProvider(TestCase): zone = Zone('unit.tests.', []) provider.populate(zone) - # the two A records get merged into one CNAME record poining to the CDN + # the two A records get merged into one CNAME record pointing to + # the CDN. self.assertEquals(3, len(zone.records)) record = list(zone.records)[0] @@ -621,7 +624,7 @@ class TestCloudflareProvider(TestCase): self.assertEquals('a.unit.tests.cdn.cloudflare.net.', record.value) # CDN enabled records can't be updated, we don't know the real values - # never point a Cloudflare record to itsself. + # never point a Cloudflare record to itself. wanted = Zone('unit.tests.', []) wanted.add_record(Record.new(wanted, 'cname', { 'ttl': 300, @@ -676,7 +679,7 @@ class TestCloudflareProvider(TestCase): self.assertEquals('unit.tests.cdn.cloudflare.net.', record.value) # CDN enabled records can't be updated, we don't know the real values - # never point a Cloudflare record to itsself. + # never point a Cloudflare record to itself. wanted = Zone('unit.tests.', []) wanted.add_record(Record.new(wanted, '', { 'ttl': 300, diff --git a/tests/test_octodns_provider_digitalocean.py b/tests/test_octodns_provider_digitalocean.py index 5fb47c5..ddc6bc2 100644 --- a/tests/test_octodns_provider_digitalocean.py +++ b/tests/test_octodns_provider_digitalocean.py @@ -165,6 +165,7 @@ class TestDigitalOceanProvider(TestCase): n = len(self.expected.records) - 7 self.assertEquals(n, len(plan.changes)) self.assertEquals(n, provider.apply(plan)) + self.assertFalse(plan.exists) provider._client._request.assert_has_calls([ # created the domain @@ -225,6 +226,7 @@ class TestDigitalOceanProvider(TestCase): })) plan = provider.plan(wanted) + self.assertTrue(plan.exists) self.assertEquals(2, len(plan.changes)) self.assertEquals(2, provider.apply(plan)) # recreate for update, and delete for the 2 parts of the other diff --git a/tests/test_octodns_provider_dnsimple.py b/tests/test_octodns_provider_dnsimple.py index 60dacf1..896425e 100644 --- a/tests/test_octodns_provider_dnsimple.py +++ b/tests/test_octodns_provider_dnsimple.py @@ -133,6 +133,7 @@ class TestDnsimpleProvider(TestCase): n = len(self.expected.records) - 3 self.assertEquals(n, len(plan.changes)) self.assertEquals(n, provider.apply(plan)) + self.assertFalse(plan.exists) provider._client._request.assert_has_calls([ # created the domain @@ -186,6 +187,7 @@ class TestDnsimpleProvider(TestCase): })) plan = provider.plan(wanted) + self.assertTrue(plan.exists) self.assertEquals(2, len(plan.changes)) self.assertEquals(2, provider.apply(plan)) # recreate for update, and deletes for the 2 parts of the other diff --git a/tests/test_octodns_provider_dyn.py b/tests/test_octodns_provider_dyn.py index 4415347..85de59c 100644 --- a/tests/test_octodns_provider_dyn.py +++ b/tests/test_octodns_provider_dyn.py @@ -430,6 +430,7 @@ class TestDynProvider(TestCase): update_mock.assert_not_called() provider.apply(plan) update_mock.assert_called() + self.assertFalse(plan.exists) add_mock.assert_called() # Once for each dyn record (8 Records, 2 of which have dual values) self.assertEquals(15, len(add_mock.call_args_list)) @@ -474,6 +475,7 @@ class TestDynProvider(TestCase): plan = provider.plan(new) provider.apply(plan) update_mock.assert_called() + self.assertTrue(plan.exists) # we expect 4 deletes, 2 from actual deletes and 2 from # updates which delete and recreate self.assertEquals(4, len(delete_mock.call_args_list)) @@ -491,7 +493,7 @@ class TestDynProviderGeo(TestCase): traffic_director_response = loads(fh.read()) @property - def traffic_directors_reponse(self): + def traffic_directors_response(self): return { 'data': [{ 'active': 'Y', @@ -607,7 +609,7 @@ class TestDynProviderGeo(TestCase): mock.side_effect = [{'data': []}] self.assertEquals({}, provider.traffic_directors) - # a supported td and an ingored one + # a supported td and an ignored one response = { 'data': [{ 'active': 'Y', @@ -650,7 +652,7 @@ class TestDynProviderGeo(TestCase): set(tds.keys())) self.assertEquals(['A'], tds['unit.tests.'].keys()) self.assertEquals(['A'], tds['geo.unit.tests.'].keys()) - provider.log.warn.assert_called_with("Failed to load TraficDirector " + provider.log.warn.assert_called_with("Failed to load TrafficDirector " "'%s': %s", 'something else', 'need more than 1 value to ' 'unpack') @@ -758,7 +760,7 @@ class TestDynProviderGeo(TestCase): # only traffic director mock.side_effect = [ # get traffic directors - self.traffic_directors_reponse, + self.traffic_directors_response, # get traffic director self.traffic_director_response, # get zone @@ -809,7 +811,7 @@ class TestDynProviderGeo(TestCase): # both traffic director and regular, regular is ignored mock.side_effect = [ # get traffic directors - self.traffic_directors_reponse, + self.traffic_directors_response, # get traffic director self.traffic_director_response, # get zone @@ -859,7 +861,7 @@ class TestDynProviderGeo(TestCase): # busted traffic director mock.side_effect = [ # get traffic directors - self.traffic_directors_reponse, + self.traffic_directors_response, # get traffic director busted_traffic_director_response, # get zone @@ -913,7 +915,7 @@ class TestDynProviderGeo(TestCase): Delete(geo), Delete(regular), ] - plan = Plan(None, desired, changes) + plan = Plan(None, desired, changes, True) provider._apply(plan) mock.assert_has_calls([ call('/Zone/unit.tests/', 'GET', {}), @@ -932,14 +934,14 @@ class TestDynProviderGeo(TestCase): provider = DynProvider('test', 'cust', 'user', 'pass', traffic_directors_enabled=True) - # will be tested seperately + # will be tested separately provider._mod_rulesets = MagicMock() mock.side_effect = [ # create traffic director self.traffic_director_response, # get traffic directors - self.traffic_directors_reponse + self.traffic_directors_response ] provider._mod_geo_Create(None, Create(self.geo_record)) # td now lives in cache diff --git a/tests/test_octodns_provider_googlecloud.py b/tests/test_octodns_provider_googlecloud.py index c2677af..3a3e600 100644 --- a/tests/test_octodns_provider_googlecloud.py +++ b/tests/test_octodns_provider_googlecloud.py @@ -263,7 +263,8 @@ class TestGoogleCloudProvider(TestCase): provider.apply(Plan( existing=[update_existing_r, delete_r], desired=desired, - changes=changes + changes=changes, + exists=True )) calls_mock = gcloud_zone_mock.changes.return_value @@ -295,7 +296,8 @@ class TestGoogleCloudProvider(TestCase): provider.apply(Plan( existing=[update_existing_r, delete_r], desired=desired, - changes=changes + changes=changes, + exists=True )) unsupported_change = Mock() @@ -357,15 +359,17 @@ class TestGoogleCloudProvider(TestCase): "unit.tests.") test_zone = Zone('unit.tests.', []) - provider.populate(test_zone) + exists = provider.populate(test_zone) + self.assertTrue(exists) # test_zone gets fed the same records as zone does, except it's in # the format returned by google API, so after populate they should look - # excactly the same. + # exactly the same. self.assertEqual(test_zone.records, zone.records) - test_zone2 = Zone('nonexistant.zone.', []) - provider.populate(test_zone2, False, False) + test_zone2 = Zone('nonexistent.zone.', []) + exists = provider.populate(test_zone2, False, False) + self.assertFalse(exists) self.assertEqual(len(test_zone2.records), 0, msg="Zone should not get records from wrong domain") @@ -401,8 +405,8 @@ class TestGoogleCloudProvider(TestCase): provider.gcloud_client.list_zones = Mock( return_value=DummyIterator([])) - self.assertIsNone(provider.gcloud_zones.get("nonexistant.xone"), - msg="Check that nonexistant zones return None when" + self.assertIsNone(provider.gcloud_zones.get("nonexistent.zone"), + msg="Check that nonexistent zones return None when" "there's no create=True flag") def test__get_rrsets(self): @@ -423,7 +427,7 @@ class TestGoogleCloudProvider(TestCase): provider.gcloud_client.list_zones = Mock( return_value=DummyIterator([])) - mock_zone = provider._create_gcloud_zone("nonexistant.zone.mock") + mock_zone = provider._create_gcloud_zone("nonexistent.zone.mock") mock_zone.create.assert_called() provider.gcloud_client.zone.assert_called() diff --git a/tests/test_octodns_provider_ns1.py b/tests/test_octodns_provider_ns1.py index 0e3a286..fa6cf2d 100644 --- a/tests/test_octodns_provider_ns1.py +++ b/tests/test_octodns_provider_ns1.py @@ -196,9 +196,10 @@ class TestNs1Provider(TestCase): load_mock.side_effect = \ ResourceException('server error: zone not found') zone = Zone('unit.tests.', []) - provider.populate(zone) + exists = provider.populate(zone) self.assertEquals(set(), zone.records) self.assertEquals(('unit.tests',), load_mock.call_args[0]) + self.assertFalse(exists) # Existing zone w/o records load_mock.reset_mock() @@ -269,6 +270,7 @@ class TestNs1Provider(TestCase): # everything except the root NS expected_n = len(self.expected) - 1 self.assertEquals(expected_n, len(plan.changes)) + self.assertTrue(plan.exists) # Fails, general error load_mock.reset_mock() diff --git a/tests/test_octodns_provider_ovh.py b/tests/test_octodns_provider_ovh.py index 9404f9e..5a62094 100644 --- a/tests/test_octodns_provider_ovh.py +++ b/tests/test_octodns_provider_ovh.py @@ -8,7 +8,7 @@ from __future__ import absolute_import, division, print_function, \ from unittest import TestCase from mock import patch, call -from ovh import APIError +from ovh import APIError, ResourceNotFoundError, InvalidCredential from octodns.provider.ovh import OvhProvider from octodns.record import Record @@ -199,14 +199,14 @@ class TestOvhProvider(TestCase): api_record.append({ 'fieldType': 'SPF', 'ttl': 1000, - 'target': 'v=spf1 include:unit.texts.rerirect ~all', + 'target': 'v=spf1 include:unit.texts.redirect ~all', 'subDomain': '', 'id': 13 }) expected.add(Record.new(zone, '', { 'ttl': 1000, 'type': 'SPF', - 'value': 'v=spf1 include:unit.texts.rerirect ~all' + 'value': 'v=spf1 include:unit.texts.redirect ~all' })) # SSHFP @@ -307,18 +307,30 @@ class TestOvhProvider(TestCase): with patch.object(provider._client, 'get') as get_mock: zone = Zone('unit.tests.', []) - get_mock.side_effect = APIError('boom') + get_mock.side_effect = ResourceNotFoundError('boom') with self.assertRaises(APIError) as ctx: provider.populate(zone) self.assertEquals(get_mock.side_effect, ctx.exception) - with patch.object(provider._client, 'get') as get_mock: + get_mock.side_effect = InvalidCredential('boom') + with self.assertRaises(APIError) as ctx: + provider.populate(zone) + self.assertEquals(get_mock.side_effect, ctx.exception) + + zone = Zone('unit.tests.', []) + get_mock.side_effect = ResourceNotFoundError('This service does ' + 'not exist') + exists = provider.populate(zone) + self.assertEquals(set(), zone.records) + self.assertFalse(exists) + zone = Zone('unit.tests.', []) get_returns = [[record['id'] for record in self.api_record]] get_returns += self.api_record get_mock.side_effect = get_returns - provider.populate(zone) + exists = provider.populate(zone) self.assertEquals(self.expected, zone.records) + self.assertTrue(exists) @patch('ovh.Client') def test_is_valid_dkim(self, client_mock): @@ -404,7 +416,7 @@ class TestOvhProvider(TestCase): call(u'/domain/zone/unit.tests/record', fieldType=u'SPF', subDomain=u'', ttl=1000, target=u'v=spf1 include:unit.texts.' - u'rerirect ~all', + u'redirect ~all', ), call(u'/domain/zone/unit.tests/record', fieldType=u'A', subDomain='sub', target=u'1.2.3.4', ttl=200), diff --git a/tests/test_octodns_provider_powerdns.py b/tests/test_octodns_provider_powerdns.py index 22ccdd6..2722991 100644 --- a/tests/test_octodns_provider_powerdns.py +++ b/tests/test_octodns_provider_powerdns.py @@ -100,12 +100,13 @@ class TestPowerDnsProvider(TestCase): # No existing records -> creates for every record in expected with requests_mock() as mock: mock.get(ANY, status_code=200, text=EMPTY_TEXT) - # post 201, is reponse to the create with data + # post 201, is response to the create with data mock.patch(ANY, status_code=201, text=assert_rrsets_callback) plan = provider.plan(expected) self.assertEquals(expected_n, len(plan.changes)) self.assertEquals(expected_n, provider.apply(plan)) + self.assertTrue(plan.exists) # Non-existent zone -> creates for every record in expected # OMG this is fucking ugly, probably better to ditch requests_mocks and @@ -118,12 +119,13 @@ class TestPowerDnsProvider(TestCase): mock.get(ANY, status_code=422, text='') # patch 422's, unknown zone mock.patch(ANY, status_code=422, text=dumps(not_found)) - # post 201, is reponse to the create with data + # post 201, is response to the create with data mock.post(ANY, status_code=201, text=assert_rrsets_callback) plan = provider.plan(expected) self.assertEquals(expected_n, len(plan.changes)) self.assertEquals(expected_n, provider.apply(plan)) + self.assertFalse(plan.exists) with requests_mock() as mock: # get 422's, unknown zone diff --git a/tests/test_octodns_provider_rackspace.py b/tests/test_octodns_provider_rackspace.py index 274d63e..c467dec 100644 --- a/tests/test_octodns_provider_rackspace.py +++ b/tests/test_octodns_provider_rackspace.py @@ -73,9 +73,10 @@ class TestRackspaceProvider(TestCase): json={'error': "Could not find domain 'unit.tests.'"}) zone = Zone('unit.tests.', []) - self.provider.populate(zone) + exists = self.provider.populate(zone) self.assertEquals(set(), zone.records) self.assertTrue(mock.called_once) + self.assertFalse(exists) def test_multipage_populate(self): with requests_mock() as mock: @@ -109,6 +110,7 @@ class TestRackspaceProvider(TestCase): plan = self.provider.plan(expected) self.assertTrue(mock.called) + self.assertTrue(plan.exists) # OctoDNS does not propagate top-level NS records. self.assertEquals(1, len(plan.changes)) diff --git a/tests/test_octodns_provider_route53.py b/tests/test_octodns_provider_route53.py index 1cd4548..3839a16 100644 --- a/tests/test_octodns_provider_route53.py +++ b/tests/test_octodns_provider_route53.py @@ -331,9 +331,9 @@ class TestRoute53Provider(TestCase): stubber.assert_no_pending_responses() # Populate a zone that doesn't exist - noexist = Zone('does.not.exist.', []) - provider.populate(noexist) - self.assertEquals(set(), noexist.records) + nonexistent = Zone('does.not.exist.', []) + provider.populate(nonexistent) + self.assertEquals(set(), nonexistent.records) def test_sync(self): provider, stubber = self._get_stubbed_provider() @@ -361,6 +361,7 @@ class TestRoute53Provider(TestCase): plan = provider.plan(self.expected) self.assertEquals(9, len(plan.changes)) + self.assertTrue(plan.exists) for change in plan.changes: self.assertIsInstance(change, Create) stubber.assert_no_pending_responses() @@ -593,6 +594,7 @@ class TestRoute53Provider(TestCase): plan = provider.plan(self.expected) self.assertEquals(9, len(plan.changes)) + self.assertFalse(plan.exists) for change in plan.changes: self.assertIsInstance(change, Create) stubber.assert_no_pending_responses() diff --git a/tests/test_octodns_record.py b/tests/test_octodns_record.py index 9f3dc33..56502a0 100644 --- a/tests/test_octodns_record.py +++ b/tests/test_octodns_record.py @@ -430,7 +430,7 @@ class TestRecord(TestCase): self.assertEqual(change.new, other) # full sorting - # equivilent + # equivalent b_naptr_value = b.values[0] self.assertEquals(0, b_naptr_value.__cmp__(b_naptr_value)) # by order @@ -710,7 +710,7 @@ class TestRecord(TestCase): Record.new(self.zone, 'unknown', {}) self.assertTrue('missing type' in ctx.exception.message) - # Unkown type + # Unknown type with self.assertRaises(Exception) as ctx: Record.new(self.zone, 'unknown', { 'type': 'XXX',