From 886a26bc6f0ffd78bc0d41bbf88e9fc26e686b21 Mon Sep 17 00:00:00 2001
From: Ross McFarland <ross@github.com>
Date: Mon, 19 Feb 2018 11:30:35 -0800
Subject: [PATCH] Extract _record_for from populate, use round trip in
 _apply_Update

This will ensure that we have exactly the same logic/behavior across the
board when turning records into content and prevent the :-( hack that was in
here before. It was missing the max(min, ttl) bit we throw everything else
through and this makes that consistent. Most importantly it'll prevent us
from having to fix bugs or make improvements in multiple code paths in the
future.
---
 octodns/provider/cloudflare.py | 56 ++++++++++++++++------------------
 1 file changed, 27 insertions(+), 29 deletions(-)

diff --git a/octodns/provider/cloudflare.py b/octodns/provider/cloudflare.py
index 8df146d..0d058dc 100644
--- a/octodns/provider/cloudflare.py
+++ b/octodns/provider/cloudflare.py
@@ -210,6 +210,20 @@ 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)
@@ -226,21 +240,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
@@ -374,10 +375,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 +384,23 @@ 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(name)
         _type = change.new._type
         # OK, work through each record from the zone
-        for record in self.zone_records(change.new.zone):
+        for record in self.zone_records(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'] += '.'
+
+                # 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
+                r = self._record_for(zone, hostname, _type, [record], True)
+                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: