Merge pull request #1315 from octodns/yaml-include-array-support

Add `<<: !include` support to allow merging multiple files

.changelog/05f9b507087a4140932dd6a554c9d61d.md

@@ -0,0 +1,5 @@
+---
+type: minor
+---
+
+Add merge syntax support to !include tag, `<<: !include file.yaml`

docs/include_directive.rst

@@ -0,0 +1,208 @@
+YAML !include Directive
+=======================
+
+The ``!include`` directive is a powerful feature in octoDNS that allows you to
+reuse YAML content across multiple configuration files and zone files. This
+helps reduce duplication and makes your DNS configuration more maintainable.
+
+Overview
+--------
+
+The ``!include`` directive can be used anywhere in your YAML files to include
+content from other files. Files are resolved relative to the directory
+containing the file with the ``!include`` directive.
+
+Basic Usage
+-----------
+
+Single File Include
+...................
+
+The simplest form includes the entire contents of a single file::
+
+  ---
+  # main.yaml
+  common-config: !include common.yaml
+
+If ``common.yaml`` contains::
+
+  ---
+  key: value
+  setting: 42
+
+Then ``common-config`` will be set to ``{'key': 'value', 'setting': 42}``.
+
+The included file can contain any valid YAML type: dictionaries, lists, strings,
+numbers, or even ``null`` values.
+
+Merge Syntax
+............
+
+The ``!include`` directive can also be used with the merge operator, ``<<:``.
+This is useful for composing configurations from multiple sources.
+
+Merging Dictionaries
+~~~~~~~~~~~~~~~~~~~~
+
+When including multiple files that contain dictionaries, the dictionaries are
+merged together. Later files override keys from earlier files::
+
+  ---
+  # main.yaml
+  merged-config:
+    <<: !include base-config.yaml
+    <<: !include overrides.yaml
+    key: value
+
+If ``base-config.yaml`` contains::
+
+  ---
+  timeout: 30
+  retries: 3
+  debug: false
+
+And ``overrides.yaml`` contains::
+
+  ---
+  timeout: 60
+  debug: true
+  added: hi
+
+Then ``merged-config`` will be::
+
+  {
+    'timeout': 60,      # overridden
+    'retries': 3,       # from base
+    'debug': true,      # overridden
+    'added': 'hi',      # added by overrides
+    'key': 'value',     # from main
+  }
+
+Use Cases
+---------
+
+Configuration Files
+...................
+
+The ``!include`` directive is useful in octoDNS configuration files for sharing
+common provider settings, processor configurations, or zone settings.
+
+Shared Provider Configuration::
+
+  ---
+  # production.yaml
+  providers:
+    # contents will be merged with what's defined here
+    <<: !include providers/common.yaml
+
+    route53:
+      class: octodns_route53.Route53Provider
+      access_key_id: env/AWS_ACCESS_KEY_ID
+      secret_access_key: env/AWS_SECRET_ACCESS_KEY
+      # Include common retry/timeout settings
+      settings: !include providers/aws-settings.yaml
+
+  ---
+  # providers/common.yaml
+  config:
+    class: octodns.providers.yaml.YamlProvider
+    directory: ./config/
+
+  internal:
+    class: octodns_powerdns.PdnsProvider
+    ...
+
+Shared Zone Configuration::
+
+  ---
+  # config.yaml
+  zones:
+    # contents will become the value for example.com.
+    example.com.: &standard-setup !include zones/standard-setup.yaml
+    example.net.: *standard-setup
+    example.org.: *standard-setup
+
+  ---
+  # zones/standard-ssetup.yaml
+  sources:
+    - config
+  targets:
+    - internal
+    - route53
+
+Zone Files
+..........
+
+The ``!include`` directive is particularly powerful in zone files for reducing
+duplication of common record configurations.
+
+Shared APEX Records
+~~~~~~~~~~~~~~~~~~~
+
+When you have multiple zones with shared APEX records but differing records
+otherwise, you can share the APEX configuration::
+
+  ---
+  # example.com.yaml
+  '': !include common/apex.yaml
+  api:
+    type: A
+    value: 1.2.3.4
+  web:
+    type: A
+    value: 1.2.3.5
+
+
+  ---
+  # common/apex.yaml
+  - type: A
+    value: 1.2.3.4
+  - type: MX
+    values:
+      - exchange: mail1.example.com.
+        preference: 10
+      - exchange: mail2.example.com.
+        preference: 20
+  - type: NS
+    values:
+      - 6.2.3.4.
+      - 7.2.3.4.
+  - type: TXT
+    values:
+      - some-domain-claiming-value=gimme
+      - v=spf1 -all
+
+Subdirectories
+..............
+
+Files in subdirectories can be included using relative paths::
+
+  ---
+  # main.yaml
+  nested-config: !include subdir/nested.yaml
+  deeper: !include subdir/another/deep.yaml
+  parent: !include ../sibling/config.yaml
+
+Type Requirements
+-----------------
+
+Any valid YAML datatype can be used in the basic **!include** stile.
+
+When using the merge syntax all files must contain **dictionaries**.
+
+      network: !include providers/retry-settings.yaml
+
+Best Practices
+--------------
+
+1. **Organize shared files**: Create a dedicated directory structure for shared
+   configurations (e.g., ``shared/``, ``common/``)
+
+2. **Use descriptive filenames**: Name included files clearly to indicate their
+   purpose (e.g., ``spf-record.yaml``, ``geo-routing-rules.yaml``)
+
+3. **Keep includes shallow**: Avoid deeply nested includes as they can make
+   the configuration harder to understand and debug
+
+4. **Document shared files**: Add comments in shared files explaining their
+   purpose and where they're used

docs/index.rst

@@ -28,6 +28,7 @@ Documentation
    getting-started.rst
    records.md
    configuration.rst
+   include_directive.rst
    dynamic_zone_config.rst
    dynamic_records.rst
    auto_arpa.rst

octodns/yaml.py

@@ -5,7 +5,7 @@
 from os.path import dirname, join
 
 from natsort import natsort_keygen
-from yaml import SafeDumper, SafeLoader, dump, load
+from yaml import SafeDumper, SafeLoader, compose, dump, load
 from yaml.constructor import ConstructorError
 from yaml.representer import SafeRepresenter
 
@@ -17,44 +17,124 @@ _natsort_key = staticmethod(natsort_keygen())
 
 
 class ContextLoader(SafeLoader):
-    def _pairs(self, node):
-        self.flatten_mapping(node)
-        pairs = self.construct_pairs(node)
-        start_mark = node.start_mark
-        context = f'{start_mark.name}, line {start_mark.line+1}, column {start_mark.column+1}'
-        return ContextDict(pairs, context=context), pairs, context
 
-    def _construct(self, node):
-        return self._pairs(node)[0]
+    def construct_include(self, node):
+        mark = self.get_mark()
+        directory = dirname(mark.name)
 
-    def include(self, node):
+        filename = join(directory, self.construct_scalar(node))
+
+        with open(filename, 'r') as fh:
+            return load(fh, self.__class__)
+
+    def flatten_include(self, node):
         mark = self.get_mark()
         directory = dirname(mark.name)
 
         filename = join(directory, self.construct_scalar(node))
 
         with open(filename, 'r') as fh:
-            return safe_load(fh, self.__class__)
+            yield compose(fh, self.__class__).value
 
+    def construct_mapping(self, node, deep=False):
+        '''
+        Calls our parent and wraps the resulting dict with a ContextDict
+        '''
+        start_mark = node.start_mark
+        context = f'{start_mark.name}, line {start_mark.line+1}, column {start_mark.column+1}'
+        return ContextDict(
+            super().construct_mapping(node, deep), context=context
+        )
+
+    # the following 4 methods are ported out of
+    # https://github.com/yaml/pyyaml/pull/894 an intended to be used until we
+    # can (hopefully) require a version of pyyaml with that PR merged.
+
+    @classmethod
+    def add_flattener(cls, tag, flattener):
+        if not 'yaml_flatteners' in cls.__dict__:
+            cls.yaml_flatteners = {}
+        cls.yaml_flatteners[tag] = flattener
+
+    # this overwrites/ignores the built-in version of the method
+    def flatten_mapping(self, node):  # pragma: no cover
+        merge = []
+        for key_node, value_node in node.value:
+            if key_node.tag == 'tag:yaml.org,2002:merge':
+                flattener = self.yaml_flatteners.get(value_node.tag)
+                if flattener:
+                    for value in flattener(self, value_node):
+                        merge.extend(value)
+                else:
+                    raise ConstructorError(
+                        "while constructing a mapping",
+                        node.start_mark,
+                        "expected a mapping or list of mappings for merging, but found %s"
+                        % value_node.id,
+                        value_node.start_mark,
+                    )
+            elif key_node.tag == 'tag:yaml.org,2002:value':
+                key_node.tag = 'tag:yaml.org,2002:str'
+                merge.append((key_node, value_node))
+            else:
+                merge.append((key_node, value_node))
+
+        node.value = merge
+
+    def flatten_yaml_seq(self, node):  # pragma: no cover
+        submerge = []
+        for subnode in node.value:
+            # we need to flatten each item in the seq, most likely they'll be mappings,
+            # but we need to allow for custom flatteners as well.
+            flattener = self.yaml_flatteners.get(subnode.tag)
+            if flattener:
+                for value in flattener(self, subnode):
+                    submerge.append(value)
+            else:
+                raise ConstructorError(
+                    "while constructing a mapping",
+                    node.start_mark,
+                    "expected a mapping for merging, but found %s" % subnode.id,
+                    subnode.start_mark,
+                )
+        submerge.reverse()
+        for value in submerge:
+            yield value
+
+    def flatten_yaml_map(self, node):  # pragma: no cover
+        self.flatten_mapping(node)
+        yield node.value
+
+
+# These 2 add's are also ported out of the PR
+ContextLoader.add_flattener(
+    'tag:yaml.org,2002:seq', ContextLoader.flatten_yaml_seq
+)
+ContextLoader.add_flattener(
+    'tag:yaml.org,2002:map', ContextLoader.flatten_yaml_map
+)
 
-ContextLoader.add_constructor('!include', ContextLoader.include)
 ContextLoader.add_constructor(
-    ContextLoader.DEFAULT_MAPPING_TAG, ContextLoader._construct
+    ContextLoader.DEFAULT_MAPPING_TAG, ContextLoader.construct_mapping
 )
+ContextLoader.add_constructor('!include', ContextLoader.construct_include)
+ContextLoader.add_flattener('!include', ContextLoader.flatten_include)
 
 
 # Found http://stackoverflow.com/a/21912744 which guided me on how to hook in
 # here
 class SortEnforcingLoader(ContextLoader):
 
-    def _construct(self, node):
-        ret, pairs, context = self._pairs(node)
+    def construct_mapping(self, node, deep=False):
+        ret = super().construct_mapping(node, deep)
 
-        keys = [d[0] for d in pairs]
+        keys = list(ret.keys())
         keys_sorted = sorted(keys, key=self.KEYGEN)
         for key in keys:
             expected = keys_sorted.pop(0)
             if key != expected:
+                start_mark = node.start_mark
+                context = f'{start_mark.name}, line {start_mark.line+1}, column {start_mark.column+1}'
                 raise ConstructorError(
                     None,
                     None,
@@ -71,7 +151,7 @@ class NaturalSortEnforcingLoader(SortEnforcingLoader):
 
 NaturalSortEnforcingLoader.add_constructor(
     SortEnforcingLoader.DEFAULT_MAPPING_TAG,
-    NaturalSortEnforcingLoader._construct,
+    NaturalSortEnforcingLoader.construct_mapping,
 )
 
 
@@ -81,7 +161,7 @@ class SimpleSortEnforcingLoader(SortEnforcingLoader):
 
 SimpleSortEnforcingLoader.add_constructor(
     SortEnforcingLoader.DEFAULT_MAPPING_TAG,
-    SimpleSortEnforcingLoader._construct,
+    SimpleSortEnforcingLoader.construct_mapping,
 )
 
 

script/coverage

@@ -10,8 +10,8 @@ SOURCE_DIR="octodns/"
 # Don't allow disabling coverage
 PRAGMA_OUTPUT=$(grep -r -I --line-number "# pragma: \+no.*cover" "$SOURCE_DIR" || echo)
 PRAGMA_COUNT=$(echo "$PRAGMA_OUTPUT" | grep -c . || true)
-PRAGMA_ALLOWED=2
-if [ "$PRAGMA_COUNT" -gt "$PRAGMA_ALLOWED" ]; then
+PRAGMA_ALLOWED=5
+if [ "$PRAGMA_COUNT" -ne "$PRAGMA_ALLOWED" ]; then
     echo "Found $PRAGMA_COUNT instances of 'pragma: no cover' (no more than $PRAGMA_ALLOWED allowed):"
     echo "$PRAGMA_OUTPUT"
     echo "Code coverage should not be disabled, except for version handling blocks"

tests/config/include/dict.yaml

@@ -1,3 +1,4 @@
 ---
 k: v
+m: o
 z: 42

tests/config/include/dict_too.yaml

@@ -0,0 +1,5 @@
+---
+z: 43
+# keys are intentionally out of order here to ensure !include files are loaded
+# with the same laoder class and thus "settings" as their parent
+foo: bar

tests/config/include/include-array-with-dict.yaml

@@ -0,0 +1,4 @@
+---
+data: !include
+  - array.yaml
+  - dict.yaml

tests/config/include/include-array-with-non-existant.yaml

@@ -0,0 +1,4 @@
+---
+data: !include
+  - array.yaml
+  - does-not-exist.yaml

tests/config/include/include-array-with-unsupported.yaml

@@ -0,0 +1,3 @@
+---
+data: !include
+  - subdir/value.yaml

tests/config/include/include-dict-with-array.yaml

@@ -0,0 +1,4 @@
+---
+data: !include
+  - dict.yaml
+  - array.yaml

tests/config/include/merge.yaml

@@ -0,0 +1,6 @@
+---
+parent:
+  k: overwritten
+  <<: !include dict.yaml
+  child: added
+  z: overwrote

tests/test_octodns_yaml.py

@@ -69,7 +69,7 @@ class TestYaml(TestCase):
         self.assertEqual(
             {
                 'included-array': [14, 15, 16, 72],
-                'included-dict': {'k': 'v', 'z': 42},
+                'included-dict': {'k': 'v', 'm': 'o', 'z': 42},
                 'included-empty': None,
                 'included-nested': 'Hello World!',
                 'included-subdir': 'Hello World!',
@@ -87,6 +87,25 @@ class TestYaml(TestCase):
                 str(ctx.exception),
             )
 
+    def test_include_merge(self):
+        with open('tests/config/include/merge.yaml') as fh:
+            data = safe_load(fh, enforce_order=False)
+        self.assertEqual(
+            {
+                'parent': {
+                    # overwritten by include
+                    'k': 'v',
+                    # added by include
+                    'm': 'o',
+                    # explicitly in parent
+                    'child': 'added',
+                    # overrode by inlucd
+                    'z': 'overwrote',
+                }
+            },
+            data,
+        )
+
     def test_order_mode(self):
         data = {'*.1.2': 'a', '*.10.1': 'c', '*.11.2': 'd', '*.2.2': 'b'}
         natural = '''---