Merge pull request #924 from octodns/yaml-supports-fixes

Yaml supports fixes

CHANGELOG.md

@@ -1,3 +1,17 @@
+## v0.9.19 - 2022-??-?? - ???
+
+* Addressed shortcomings with YamlProvider.SUPPORTS in that it didn't include
+  dynamically registered types, was a static list that could have drifted over
+  time even ignoring 3rd party types.
+* Provider._process_desired_zone needed to call Provider.supports rather than
+  doing it's own `_type in provider.SUPPORTS`. The default behavior in
+  Source.supports is ^, but it's possible for providers to override that
+  behavior and do special checking and `_process_desired_zone` wasn't taking
+  that into account.
+* Now that it's used as it needed to be YamlProvider overrides
+  Provider.supports and just always says Yes so that any dynamically registered
+  types will be supported.
+
 ## v0.9.18 - 2022-08-14 - Subzone handling
 
 * Fixed issue with sub-zone handling introduced in 0.9.18

octodns/provider/base.py

@@ -59,7 +59,7 @@ class BaseProvider(BaseSource):
         '''
 
         for record in desired.records:
-            if record._type not in self.SUPPORTS:
+            if not self.supports(record):
                 msg = f'{record._type} records not supported for {record.fqdn}'
                 fallback = 'omitting record'
                 self.supports_warn_or_except(msg, fallback)

octodns/provider/yaml.py

@@ -111,26 +111,6 @@ class YamlProvider(BaseProvider):
     SUPPORTS_DYNAMIC = True
     SUPPORTS_POOL_VALUE_STATUS = True
     SUPPORTS_MULTIVALUE_PTR = True
-    SUPPORTS = set(
-        (
-            'A',
-            'AAAA',
-            'ALIAS',
-            'CAA',
-            'CNAME',
-            'DNAME',
-            'LOC',
-            'MX',
-            'NAPTR',
-            'NS',
-            'PTR',
-            'SSHFP',
-            'SPF',
-            'SRV',
-            'TXT',
-            'URLFWD',
-        )
-    )
 
     def __init__(
         self,
@@ -167,6 +147,22 @@ class YamlProvider(BaseProvider):
         del args['log']
         return self.__class__(**args)
 
+    @property
+    def SUPPORTS(self):
+        # The yaml provider supports all record types even those defined by 3rd
+        # party modules that we know nothing about, thus we dynamically return
+        # the types list that is registered in Record, everything that's know as
+        # of the point in time we're asked
+        return set(Record.registered_types().keys())
+
+    def supports(self, record):
+        # We're overriding this as a performance tweak, namely to avoid calling
+        # the implementation of the SUPPORTS property to create a set from a
+        # dict_keys every single time something checked whether we support a
+        # record, the answer is always yes so that's overkill and we can just
+        # return True here and be done with it
+        return True
+
     @property
     def SUPPORTS_ROOT_NS(self):
         return self.supports_root_ns

octodns/record/__init__.py

@@ -102,6 +102,10 @@ class Record(EqualityTupleMixin):
             raise RecordException(msg)
         cls._CLASSES[_type] = _class
 
+    @classmethod
+    def registered_types(cls):
+        return cls._CLASSES
+
     @classmethod
     def new(cls, zone, name, data, source=None, lenient=False):
         name = str(name).lower()

tests/test_octodns_provider_yaml.py

@@ -15,7 +15,7 @@ from unittest import TestCase
 from yaml import safe_load
 from yaml.constructor import ConstructorError
 
-from octodns.record import Create
+from octodns.record import _NsValue, Create, Record, ValuesMixin
 from octodns.provider.base import Plan
 from octodns.provider.yaml import (
     _list_all_yaml_files,
@@ -217,6 +217,36 @@ class TestYamlProvider(TestCase):
             str(ctx.exception),
         )
 
+    def test_SUPPORTS(self):
+        source = YamlProvider('test', join(dirname(__file__), 'config'))
+        # make sure the provider supports all the registered types
+        self.assertEqual(Record.registered_types().keys(), source.SUPPORTS)
+
+        class YamlRecord(ValuesMixin, Record):
+            _type = 'YAML'
+            _value_type = _NsValue
+
+        # don't know anything about a yaml type
+        self.assertTrue('YAML' not in source.SUPPORTS)
+        # register it
+        Record.register_type(YamlRecord)
+        # when asked again we'll now include it in our list of supports
+        self.assertTrue('YAML' in source.SUPPORTS)
+
+    def test_supports(self):
+        source = YamlProvider('test', join(dirname(__file__), 'config'))
+
+        class DummyType(object):
+            def __init__(self, _type):
+                self._type = _type
+
+        # No matter what we check it's always supported
+        self.assertTrue(source.supports(DummyType(None)))
+        self.assertTrue(source.supports(DummyType(42)))
+        self.assertTrue(source.supports(DummyType('A')))
+        self.assertTrue(source.supports(DummyType(source)))
+        self.assertTrue(source.supports(DummyType(self)))
+
 
 class TestSplitYamlProvider(TestCase):
     def test_list_all_yaml_files(self):

tests/test_octodns_record.py

@@ -69,6 +69,8 @@ class TestRecord(TestCase):
             _type = 'AA'
             _value_type = _NsValue
 
+        self.assertTrue('AA' not in Record.registered_types())
+
         Record.register_type(AaRecord)
         aa = Record.new(
             self.zone,
@@ -77,6 +79,8 @@ class TestRecord(TestCase):
         )
         self.assertEqual(AaRecord, aa.__class__)
 
+        self.assertTrue('AA' in Record.registered_types())
+
     def test_lowering(self):
         record = ARecord(
             self.zone, 'MiXeDcAsE', {'ttl': 30, 'type': 'A', 'value': '1.2.3.4'}