From 2cd5511dc68ccd0fad97dbc665e9cc20f20813d0 Mon Sep 17 00:00:00 2001
From: Mark Tearle <mtearle@ucc.asn.au>
Date: Sat, 30 Jan 2021 00:08:24 +0800
Subject: [PATCH] Warn that NULL SRV records are unsupported in DNSimple
 provider

DNSimple does not handle NULL SRV records correctly (either via
their web interface or API).  Flag to end user if attempted.

Issue noted with DNSimple support 2020-12-09
---
 octodns/provider/dnsimple.py            | 41 +++++++++++++++++++++++--
 tests/test_octodns_provider_dnsimple.py |  2 +-
 2 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/octodns/provider/dnsimple.py b/octodns/provider/dnsimple.py
index f83098e..647b89c 100644
--- a/octodns/provider/dnsimple.py
+++ b/octodns/provider/dnsimple.py
@@ -218,12 +218,23 @@ class DnsimpleProvider(BaseProvider):
             try:
                 weight, port, target = record['content'].split(' ', 2)
             except ValueError:
-                # see _data_for_NAPTR's continue
+                # their api/website will let you create invalid records, this
+                # 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
+                self.log.warning(
+                    '_data_for_SRV: unsupported %s record (%s)',
+                    _type,
+                    record['content']
+                )
                 continue
+
+            target = '{}.'.format(target) if target != "." else "."
+
             values.append({
                 'port': port,
                 'priority': record['priority'],
-                'target': '{}.'.format(target),
+                'target': target,
                 'weight': weight
             })
         return {
@@ -269,7 +280,12 @@ class DnsimpleProvider(BaseProvider):
         values = defaultdict(lambda: defaultdict(list))
         for record in self.zone_records(zone):
             _type = record['type']
+            data_for = getattr(self, '_data_for_{}'.format(_type), None)
             if _type not in self.SUPPORTS:
+                self.log.warning(
+                    'populate: skipping unsupported %s record',
+                    _type
+                )
                 continue
             elif _type == 'TXT' and record['content'].startswith('ALIAS for'):
                 # ALIAS has a "ride along" TXT record with 'ALIAS for XXXX',
@@ -290,6 +306,27 @@ class DnsimpleProvider(BaseProvider):
                       len(zone.records) - before, exists)
         return exists
 
+    def supports(self, record):
+        # DNSimple does not support empty/NULL SRV records
+        #
+        # Fails silently and leaves a corrupt record
+        #
+        # Skip the record and continue
+        if record._type == "SRV":
+            if 'value' in record.data:
+                targets = (record.data['value']['target'],)
+            else:
+                targets = [value['target'] for value in record.data['values']]
+
+            if "." in targets:
+                self.log.warning(
+                    'supports: unsupported %s record with target (%s)',
+                    record._type, targets
+                )
+                return False
+
+        return record._type in self.SUPPORTS
+
     def _params_for_multiple(self, record):
         for value in record.values:
             yield {
diff --git a/tests/test_octodns_provider_dnsimple.py b/tests/test_octodns_provider_dnsimple.py
index 92f32b1..9f1dab3 100644
--- a/tests/test_octodns_provider_dnsimple.py
+++ b/tests/test_octodns_provider_dnsimple.py
@@ -137,7 +137,7 @@ class TestDnsimpleProvider(TestCase):
         plan = provider.plan(self.expected)
 
         # No root NS, no ignored, no excluded
-        n = len(self.expected.records) - 4
+        n = len(self.expected.records) - 6
         self.assertEquals(n, len(plan.changes))
         self.assertEquals(n, provider.apply(plan))
         self.assertFalse(plan.exists)