Add a more generic tree->graph parser

This moves to a more generic config parser that doesn't have plugins
parsing part of the tree.

I understand why it ended up that way; we have "partitions" key which
has special semantics compared to others keys and there was a desire
to keep it isolated from core tree->graph code.  But this isn't really
isolated; you have to reverse-engineer several module-crossing
boundaries, extras classes and repetitive recursive functions.

Ultimately, plugins should have access to the node graph, but not
participate in configuration parsing.  This way we ensure that plugins
can't invent new methods of configuration parsing.

Note: unit tests produce the same tree -> graph conversion as the old
method.  i.e. this is not intended to have a functional change.

Change-Id: I8a5d62a076a5a50597f2f1df3a8615afba6dadb2

diskimage_builder/block_device/blockdevice.py

@@ -22,6 +22,8 @@ import yaml
 
 from stevedore import extension
 
+from diskimage_builder.block_device.config import \
+    config_tree_to_graph
 from diskimage_builder.block_device.exception import \
     BlockDeviceSetupException
 from diskimage_builder.block_device.utils import exec_sudo
@@ -119,30 +121,6 @@ class BlockDevice(object):
                         else:
                             v['label'] = "cloudimg-rootfs"
 
-    @staticmethod
-    def _config_tree_to_digraph(tconfig, plugin_manager):
-        """Converts a possible tree-like config into a complete digraph"""
-        dconfig = []
-        for config_entry in tconfig:
-            if len(config_entry) != 1:
-                logger.error("Invalid config entry: more than one key "
-                             "on top level [%s]" % config_entry)
-                raise BlockDeviceSetupException(
-                    "Top level config must contain exactly one key per entry")
-            logger.debug("Config entry [%s]" % config_entry)
-            config_key = list(config_entry)[0]
-            config_value = config_entry[config_key]
-            name = config_value['name'] \
-                   if 'name' in config_value else None
-            if config_key not in plugin_manager:
-                dconfig.append(config_entry)
-            else:
-                plugin_manager[config_key].plugin \
-                    .tree_config.config_tree_to_digraph(
-                        config_key, config_value, dconfig, name,
-                        plugin_manager)
-        return dconfig
-
     @staticmethod
     def _load_json(file_name):
         if os.path.exists(file_name):
@@ -211,6 +189,7 @@ class BlockDevice(object):
                 return 1
             cfg_obj = self.plugin_manager[cfg_obj_name].plugin(
                 cfg_obj_val, default_config)
+
             # At this point it is only possible to add the nodes:
             # adding the edges needs all nodes first.
             cfg_obj.insert_nodes(dg)
@@ -238,8 +217,7 @@ class BlockDevice(object):
         with open(self.params['config'], "rt") as config_fd:
             self.config = yaml.safe_load(config_fd)
         logger.debug("Config before merge [%s]" % self.config)
-        self.config = self._config_tree_to_digraph(self.config,
+        self.config = config_tree_to_graph(self.config)
-                                                   self.plugin_manager)
         logger.debug("Config before merge [%s]" % self.config)
         self._merge_into_config()
         logger.debug("Final config [%s]" % self.config)

diskimage_builder/block_device/config.py

@@ -0,0 +1,173 @@
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+import logging
+
+from stevedore import extension
+
+from diskimage_builder.block_device.exception import \
+    BlockDeviceSetupException
+
+
+logger = logging.getLogger(__name__)
+
+_extensions = extension.ExtensionManager(
+    namespace='diskimage_builder.block_device.plugin',
+    invoke_on_load=False)
+
+
+# check if a given name is registered as a plugin
+def is_a_plugin(name):
+    return any(
+        _extensions.map(lambda x: x.name == name))
+
+
+def recurse_config(config, parent_base=None):
+    """Convert a config "tree" to it's canonical name/base graph version
+
+    This is a recursive function to convert a YAML layout "tree"
+    config into a "flat" graph-based config.
+
+    Arguments:
+    :param config: the incoming config dictionary
+    :param parent_base: the name of the parent node, if any
+    :return: a list of expanded, graph-based config items
+
+    """
+    output = []
+    this = {}
+
+    # We should only have one key, with multiple values, being the
+    # config entries.  e.g. (this was checked by config_tree_to_graph)
+    #  mkfs:
+    #    type: ext4
+    #    label: 1234
+    assert len(config.items()) == 1
+    for k, v in config.items():
+        key = k
+        values = v
+
+    # If we don't have a base, we take the parent base; first element
+    # can have no base, however.
+    if 'base' not in values:
+        if parent_base is not None:
+            this['base'] = parent_base
+    else:
+        this['base'] = values['base']
+
+    # If we don't have a name, it is made up as "key_base"
+    if 'name' not in values:
+        this['name'] = "%s_%s" % (key, this['base'])
+    else:
+        this['name'] = values['name']
+
+    # Go through the the values dictionary.  Either this is a "plugin"
+    # key that needs to be recursed, or it is a value that is part of
+    # this config entry.
+    for nk, nv in values.items():
+        if nk == "partitions":
+            # "partitions" is a special key of the "partitioning"
+            # object.  It is a list.  Each list-entry gets treated
+            # as a top-level entry, so we need to recurse it's
+            # keys.  But instead of becoming its own entry in the
+            # graph, it gets attached to the .partitions attribute
+            # of the parent. (see end for example)
+            this['partitions'] = []
+            for partition in nv:
+                new_part = {}
+                for pk, pv in partition.items():
+                    if is_a_plugin(pk):
+                        output.extend(
+                            recurse_config({pk: pv}, partition['name']))
+                    else:
+                        new_part[pk] = pv
+                new_part['base'] = this['base']
+                this['partitions'].append(new_part)
+        elif is_a_plugin(nk):
+            # is this key a plugin directive?  If so, we recurse
+            # into it.
+            output.extend(recurse_config({nk: nv}, this['name']))
+        else:
+            # A value entry; just save as part of this entry
+            this[nk] = nv
+
+    output.append({k: this})
+    return output
+
+
+def config_tree_to_graph(config):
+    """Turn a YAML config into a graph config
+
+    Our YAML config is a list of entries.  Each
+
+    Arguments:
+    :parm config: YAML config; either graph or tree
+    :return: graph-based result
+
+    """
+    output = []
+
+    for entry in config:
+        # Top-level entries should be a dictionary and have a plugin
+        # registered for it
+        if not isinstance(entry, dict):
+            raise BlockDeviceSetupException(
+                "Config entry not a dict: %s" % entry)
+
+        keys = list(entry.keys())
+
+        if len(keys) != 1:
+            raise BlockDeviceSetupException(
+                "Config entry top-level should be a single dict: %s" % entry)
+
+        if not is_a_plugin(keys[0]):
+            raise BlockDeviceSetupException(
+                "Config entry is not a plugin value: %s" % entry)
+
+        output.extend(recurse_config(entry))
+
+    return output
+
+
+#
+# On partitioning: objects
+#
+# To be concrete --
+#
+# partitioning:
+#   base: loop0
+#   name: mbr
+#   partitions:
+#     - name: partition1
+#       foo: bar
+#       mkfs:
+#         type: xfs
+#         mount:
+#           mount_point: /
+#
+# gets turned into the following graph:
+#
+# partitioning:
+#   partitions:
+#     - name: partition1
+#       base: image0
+#       foo: bar
+#
+# mkfs:
+#   base: partition1
+#   name: mkfs_partition1
+#   type: xfs
+#
+# mount:
+#   base: mkfs_partition1
+#   name: mount_mkfs_partition1
+#   mount_point: /

diskimage_builder/block_device/level0/localloop.py

@@ -18,7 +18,6 @@ import subprocess
 
 from diskimage_builder.block_device.exception import \
     BlockDeviceSetupException
-from diskimage_builder.block_device.tree_config import TreeConfig
 from diskimage_builder.block_device.utils import parse_abs_size_spec
 from diskimage_builder.graph.digraph import Digraph
 
@@ -32,8 +31,6 @@ class LocalLoop(Digraph.Node):
     This class handles local loop devices that can be used
     for VM image installation.
     """
-    tree_config = TreeConfig("local_loop")
-
     def __init__(self, config, default_config):
         logger.debug("Creating LocalLoop object; config [%s] "
                      "default_config [%s]" % (config, default_config))

diskimage_builder/block_device/level1/__init__.py

@@ -14,7 +14,5 @@
 # under the License.
 
 from diskimage_builder.block_device.level1.partitioning import Partitioning
-from diskimage_builder.block_device.level1.partitioning \
-    import PartitioningTreeConfig
 
-__all__ = [Partitioning, PartitioningTreeConfig]
+__all__ = [Partitioning]

diskimage_builder/block_device/level1/partition.py

@@ -14,44 +14,15 @@ import logging
 
 from diskimage_builder.block_device.exception import \
     BlockDeviceSetupException
-from diskimage_builder.block_device.tree_config import TreeConfig
 from diskimage_builder.graph.digraph import Digraph
 
 
 logger = logging.getLogger(__name__)
 
 
-class PartitionTreeConfig(object):
-
-    @staticmethod
-    def config_tree_to_digraph(config_key, config_value, pconfig, dconfig,
-                               base_name, plugin_manager):
-        logger.debug("called [%s] [%s] [%s]"
-                     % (config_key, config_value, base_name))
-        assert config_key == Partition.type_string
-
-        for partition in config_value:
-            name = partition['name']
-            nconfig = {
-                'name': name,
-                'base': base_name
-            }
-            for k, v in partition.items():
-                if k not in plugin_manager:
-                    nconfig[k] = v
-                else:
-                    plugin_manager[k].plugin \
-                                     .tree_config.config_tree_to_digraph(
-                                         k, v, dconfig, name, plugin_manager)
-            pconfig.append(nconfig)
-
-        logger.debug("finished [%s] [%s]" % (nconfig, dconfig))
-
-
 class Partition(Digraph.Node):
 
     type_string = "partitions"
-    tree_config = TreeConfig("partitions")
 
     flag_boot = 1
     flag_primary = 2

diskimage_builder/block_device/level1/partitioning.py

@@ -22,8 +22,6 @@ from diskimage_builder.block_device.exception import \
 from diskimage_builder.block_device.level1.mbr import MBR
 from diskimage_builder.block_device.level1.partition import \
     Partition
-from diskimage_builder.block_device.level1.partition import \
-    PartitionTreeConfig
 from diskimage_builder.block_device.utils import exec_sudo
 from diskimage_builder.block_device.utils import parse_abs_size_spec
 from diskimage_builder.block_device.utils import parse_rel_size_spec
@@ -33,34 +31,8 @@ from diskimage_builder.graph.digraph import Digraph
 logger = logging.getLogger(__name__)
 
 
-class PartitioningTreeConfig(object):
-
-    @staticmethod
-    def config_tree_to_digraph(config_key, config_value, dconfig,
-                               default_base_name, plugin_manager):
-        logger.debug("called [%s] [%s] [%s]"
-                     % (config_key, config_value, default_base_name))
-        assert config_key == "partitioning"
-        base_name = config_value['base'] if 'base' in config_value \
-                    else default_base_name
-        nconfig = {'base': base_name}
-        for k, v in config_value.items():
-            if k != 'partitions':
-                nconfig[k] = v
-            else:
-                pconfig = []
-                PartitionTreeConfig.config_tree_to_digraph(
-                    k, v, pconfig, dconfig, base_name, plugin_manager)
-                nconfig['partitions'] = pconfig
-
-        dconfig.append({config_key: nconfig})
-        logger.debug("finished new [%s] complete [%s]" % (nconfig, dconfig))
-
-
 class Partitioning(Digraph.Node):
 
-    tree_config = PartitioningTreeConfig()
-
     def __init__(self, config, default_config):
         logger.debug("Creating Partitioning object; config [%s]" % config)
         # Because using multiple partitions of one base is done

diskimage_builder/block_device/level2/mkfs.py

@@ -17,7 +17,6 @@ import uuid
 
 from diskimage_builder.block_device.exception \
     import BlockDeviceSetupException
-from diskimage_builder.block_device.tree_config import TreeConfig
 from diskimage_builder.block_device.utils import exec_sudo
 from diskimage_builder.graph.digraph import Digraph
 
@@ -164,7 +163,6 @@ class Mkfs(object):
     """
 
     type_string = "mkfs"
-    tree_config = TreeConfig("mkfs")
 
     def __init__(self, config, default_config):
         logger.debug("Create Mkfs object; config [%s]" % config)

diskimage_builder/block_device/level3/mount.py

@@ -17,7 +17,6 @@ import os
 
 from diskimage_builder.block_device.exception \
     import BlockDeviceSetupException
-from diskimage_builder.block_device.tree_config import TreeConfig
 from diskimage_builder.block_device.utils import exec_sudo
 from diskimage_builder.block_device.utils import sort_mount_points
 from diskimage_builder.graph.digraph import Digraph
@@ -131,7 +130,6 @@ class MountPoint(Digraph.Node):
 class Mount(object):
 
     type_string = "mount"
-    tree_config = TreeConfig("mount")
 
     def __init__(self, config, params):
         logger.debug("Mounting object; config [%s]" % config)

diskimage_builder/block_device/level4/fstab.py

@@ -14,7 +14,6 @@
 
 import logging
 
-from diskimage_builder.block_device.tree_config import TreeConfig
 from diskimage_builder.graph.digraph import Digraph
 
 
@@ -24,7 +23,6 @@ logger = logging.getLogger(__name__)
 class Fstab(Digraph.Node):
 
     type_string = "fstab"
-    tree_config = TreeConfig("fstab")
 
     def __init__(self, config, params):
         logger.debug("Fstab object; config [%s]" % config)

diskimage_builder/block_device/tests/config/deep_graph.yaml

@@ -1,5 +1,4 @@
 - local_loop:
-    base: image0
     name: image0
 
 - partitioning:

diskimage_builder/block_device/tests/config/multiple_partitions_graph.yaml

@@ -1,5 +1,4 @@
 - local_loop:
-    base: image0
     name: image0
 
 - partitioning:

diskimage_builder/block_device/tests/test_config.py

@@ -18,6 +18,10 @@ import yaml
 
 from diskimage_builder.block_device.blockdevice \
     import BlockDevice
+from diskimage_builder.block_device.config \
+    import config_tree_to_graph
+from diskimage_builder.block_device.exception import \
+    BlockDeviceSetupException
 
 
 logger = logging.getLogger(__name__)
@@ -62,52 +66,40 @@ class TestGraphGeneration(TestConfig):
         self.bd = BlockDevice(self.fake_default_config)
 
 
-# NOTE: inherits from TestGraphGeneration for simplicity to get
+class TestConfigParsing(TestConfig):
-# BlockDevice.plugin_manager object for _config_tree_to_diagraph.
-# Config parsing can be moved separately (and not require a
-# BlockDevice object) in a later change.
-class TestConfigParsing(TestGraphGeneration):
     """Test parsing config file into a graph"""
 
     def test_config_bad_plugin(self):
-        # Currently, configuration parsing does not notice a missing
+        config = self.load_config_file('bad_plugin.yaml')
-        # plugin.  This is left as a stub
+        self.assertRaises(BlockDeviceSetupException,
-        return
+                          config_tree_to_graph,
-        # config = self.load_config_file('bad_plugin.yaml')
+                          config)
-        # self.assertRaises(BlockDeviceSetupException,
-        #                  self.bd._config_tree_to_digraph,
-        #                  config, self.bd.plugin_manager)
 
     # a graph should remain the same
     def test_graph(self):
         graph = self.load_config_file('simple_graph.yaml')
-        parsed_graph = self.bd._config_tree_to_digraph(graph,
+        parsed_graph = config_tree_to_graph(graph)
-                                                       self.bd.plugin_manager)
+        self.assertItemsEqual(parsed_graph, graph)
-        self.assertEqual(parsed_graph, graph)
 
     # equivalence of simple tree to graph
     def test_simple_tree(self):
         tree = self.load_config_file('simple_tree.yaml')
         graph = self.load_config_file('simple_graph.yaml')
-        parsed_graph = self.bd.\
+        parsed_graph = config_tree_to_graph(tree)
-                       _config_tree_to_digraph(tree,
-                                               self.bd.plugin_manager)
         self.assertItemsEqual(parsed_graph, graph)
 
     # equivalence of a deeper tree to graph
     def test_deep_tree(self):
         tree = self.load_config_file('deep_tree.yaml')
         graph = self.load_config_file('deep_graph.yaml')
-        parsed_graph = self.bd.\
+        parsed_graph = config_tree_to_graph(tree)
-                       _config_tree_to_digraph(tree, self.bd.plugin_manager)
         self.assertItemsEqual(parsed_graph, graph)
 
     # equivalence of a complicated multi-partition tree to graph
     def test_multipart_tree(self):
         tree = self.load_config_file('multiple_partitions_tree.yaml')
         graph = self.load_config_file('multiple_partitions_graph.yaml')
-        parsed_graph = self.bd._config_tree_to_digraph(tree,
+        parsed_graph = config_tree_to_graph(tree)
-                                                       self.bd.plugin_manager)
         logger.debug(parsed_graph)
         self.assertItemsEqual(parsed_graph, graph)
 

diskimage_builder/block_device/tree_config.py

@@ -1,60 +0,0 @@
-# Copyright 2016-2017 Andreas Florath (andreas@florath.net)
-#
-# Licensed under the Apache License, Version 2.0 (the "License"); you may
-# not use this file except in compliance with the License. You may obtain
-# a copy of the License at
-#
-#     http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
-# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
-# License for the specific language governing permissions and limitations
-# under the License.
-
-import logging
-
-logger = logging.getLogger(__name__)
-
-
-class TreeConfig(object):
-    """Supports simple tree-like configuration
-
-    When using the new (complete) configuration there is a need to
-    specify the complete digraph of block level configurations.  This
-    provides great flexibility for the cost of complex configurations:
-    each and every single element must be completely specified.  In
-    many simple use cases the configuration flexibility is not
-    needed.
-
-    With the help of this object the simple to use and short tree-like
-    configuration is converted automatically into the complete digraph
-    configuration which can be used to create the block device
-    elements.
-    """
-
-    def __init__(self, type_string):
-        self.type_string = type_string
-
-    def config_tree_to_digraph(self, config_key, config_value, dconfig,
-                               default_base_name, plugin_manager):
-        logger.debug("called [%s] [%s] [%s]"
-                     % (config_key, config_value, default_base_name))
-        base_name = config_value['base'] if 'base' in config_value \
-                    else default_base_name
-        name = config_value['name'] \
-               if 'name' in config_value \
-                  else "%s_%s" % (config_key, base_name)
-        assert config_key == self.type_string
-
-        nconfig = {'base': base_name, 'name': name}
-        for k, v in config_value.items():
-            if k not in plugin_manager:
-                nconfig[k] = v
-            else:
-                plugin_manager[k].plugin \
-                    .tree_config.config_tree_to_digraph(
-                        k, v, dconfig, name, plugin_manager)
-
-        dconfig.append({self.type_string: nconfig})
-        logger.debug("finished new [%s] complete [%s]" % (nconfig, dconfig))

doc/source/user_guide/building_an_image.rst

@@ -157,7 +157,7 @@ is exactly the same as writing
    mount:
      name: mount_root_fs
      base: root_fs
-       mount_point: /
+     mount_point: /
 
 Non existing `name` and `base` entries in the tree notation are
 automatically generated: the `name` is the name of the base module