Add combine '--layer-method' to support 'dir.d' layers

- Add support for directory '*.d' automatic layer detection and handling within
  the ksconf combine command.  By default, ksconf attempt to guess the correct
  mode to use, but this can (and should) be set by the '-m' or '--layer-method'
  arguments.
- Added new unit test for 'dir.d' support (based on existing combine test).
- Add check/handler for files that disappear during execution (originally hit
  during test case development)
- Add helper function to determine if a given logical path exists within a
  specific layer.  This and many other minor path handling tweaks required to
  get all the unit tests working.

docs/source/dyn/cli.rst

@@ -93,8 +93,9 @@ ksconf combine
 
  .. code-block:: none
 
-    usage: ksconf combine [-h] [--target TARGET] [--dry-run] [--follow-symlink]
-                          [--banner BANNER] [--disable-marker]
+    usage: ksconf combine [-h] [--target TARGET] [-m {auto,dir.d,disable}]
+                          [--dry-run] [--follow-symlink] [--banner BANNER]
+                          [--disable-marker]
                           source [source ...]
     
     Merge .conf settings from multiple source directories into a combined target
@@ -132,6 +133,24 @@ ksconf combine
       --target TARGET, -t TARGET
                             Directory where the merged files will be stored.
                             Typically either 'default' or 'local'
+      -m {auto,dir.d,disable}, --layer-method {auto,dir.d,disable}
+                            Select the layer method in use. Currently, there are
+                            two layer management options. Most often 'dir.d' will
+                            work well when using '*.d' folders for layers. This
+                            assumes your layers are like so: 'MyApp/default.d
+                            /##-layer-name'. Using 'dir.d' mode, any layer
+                            directories that are found will be handled
+                            automatically. If you'd like to manage the layers
+                            explicitly and turn off built-in layer support, use
+                            'disable'. By default, 'auto' mode will enable
+                            transparent switching between 'dir.d' and 'disable'
+                            (legacy) behavior. In auto mode, if more than one
+                            source directory is given, then 'disable' mode is
+                            used, if only a single directory is given then 'dir.d'
+                            will be used. Version notes: dir.d was added in ksconf
+                            0.8. Starting in 1.0 the default will switch to
+                            'dir.d', so if you need the old behavior be sure to
+                            update your scripts.
       --dry-run, -D         Enable dry-run mode. Instead of writing to TARGET,
                             preview changes as a 'diff'. If TARGET doesn't exist,
                             then show the merged file.

ksconf/commands/combine.py

@@ -13,13 +13,14 @@
 import re
 from collections import defaultdict
 
-from ksconf.layer import DirectLayerRoot
+from ksconf.layer import DirectLayerRoot, DotDLayerRoot, LayerConfig
 from ksconf.commands import ConfFileProxy
 from ksconf.commands import KsconfCmd, dedent
 from ksconf.conf.delta import show_text_diff
 from ksconf.conf.merge import merge_conf_files
 from ksconf.conf.parser import PARSECONF_MID, PARSECONF_STRICT
-from ksconf.consts import EXIT_CODE_MISSING_ARG, EXIT_CODE_COMBINE_MARKER_MISSING, SMART_NOCHANGE
+from ksconf.consts import EXIT_CODE_MISSING_ARG, EXIT_CODE_COMBINE_MARKER_MISSING, SMART_NOCHANGE, \
+    EXIT_CODE_NO_SUCH_FILE, EXIT_CODE_BAD_ARGS
 from ksconf.util.compare import file_compare
 from ksconf.util.completers import DirectoriesCompleter
 from ksconf.util.file import expand_glob_list, relwalk, _is_binary_file, smart_copy
@@ -71,6 +72,25 @@ def register_args(self, parser):
             Directory where the merged files will be stored.
             Typically either 'default' or 'local'"""
                             )).completer = DirectoriesCompleter()
+        parser.add_argument("-m", "--layer-method",
+                            choices=["auto", "dir.d", "disable"],
+                            default="auto",
+                            help="""
+            Select the layer method in use.
+            Currently, there are two layer management options.
+            Most often 'dir.d' will work well when using '*.d' folders for layers.
+            This assumes your layers are like so:  'MyApp/default.d/##-layer-name'.
+            Using 'dir.d' mode, any layer directories that are found will be handled automatically.
+            If you'd like to manage the layers explicitly and turn off built-in layer support, use
+            'disable'.
+
+            By default, 'auto' mode will enable transparent switching between 'dir.d' and 'disable'
+            (legacy) behavior.
+            In auto mode, if more than one source directory is given, then 'disable' mode is used,
+            if only a single directory is given then 'dir.d' will be used.
+
+            Version notes:  dir.d was added in ksconf 0.8.  Starting in 1.0 the default will switch
+            to 'dir.d', so if you need the old behavior be sure to update your scripts.""")
         parser.add_argument("--dry-run", "-D", default=False, action="store_true", help=dedent("""
             Enable dry-run mode.
             Instead of writing to TARGET, preview changes as a 'diff'.
@@ -91,22 +111,49 @@ def register_args(self, parser):
             """.format(CONTROLLED_DIR_MARKER)))
 
     def run(self, args):
-        layer_root = DirectLayerRoot()
-        layer_root.config.follow_symlink = args.follow_symlink
-
-        # Ignores case sensitivity.  If you're on Windows, name your files right.
+        # Note this case sensitive.  Don't be lazy, name your files correctly  :-)
         conf_file_re = re.compile("([a-z]+\.conf|(default|local)\.meta)$")
+        args.source = list(expand_glob_list(args.source, do_sort=True))
+
+        config = LayerConfig()
+        config.follow_symlink = args.follow_symlink
+
+        if args.layer_method == "auto":
+            self.stderr.write(
+                "Warning:  Automatically guessing an appropriate directory layer detection.  "
+                "Consider using '--layer-method' to avoid this warning.\n")
+            if len(args.source) == 1:
+                layer_method = "dir.d"
+            else:
+                layer_method = "disable"
+        else:
+            layer_method = args.layer_method
+
+        if layer_method == "dir.d":
+            self.stderr.write("Using automatic '*.d' directory layer detection.\n")
+            if len(args.source) > 1:
+                # XXX: Lift this restriction, if possible.  Seems like this *should* be doable. idk
+                self.stderr.write("ERROR:  Only one source directory is allowed when running the "
+                                  "'dir.d' layer mode.\n")
+                return EXIT_CODE_BAD_ARGS
+
+            layer_root = DotDLayerRoot(config=config)
+            layer_root.set_root(args.source[0])
+            for (dir, layers) in layer_root._mount_points.items():
+                self.stderr.write("Found layer parent folder:  {}  with layers {}\n"
+                                  .format(dir, ", ".join(layers)))
+        else:
+            self.stderr.write("Automatic layer detection is disabled.\n")
+            layer_root = DirectLayerRoot(config=config)
+            for src in args.source:
+                self.stderr.write("Reading conf files from directory {}\n".format(src))
+                layer_root.add_layer(src)
 
         if args.target is None:
             self.stderr.write("Must provide the '--target' directory.\n")
             return EXIT_CODE_MISSING_ARG
 
         self.stderr.write("Combining conf files into directory {}\n".format(args.target))
-        args.source = list(expand_glob_list(args.source, do_sort=True))
-        for src in args.source:
-            self.stderr.write("Reading conf files from directory {}\n".format(src))
-            layer_root.add_layer(src)
-
         self.stderr.write("Layers detected:  {}\n".format(layer_root.list_layer_names()))
 
         marker_file = os.path.join(args.target, CONTROLLED_DIR_MARKER)
@@ -133,15 +180,20 @@ def run(self, args):
             for fn in files:
                 tgt_file = os.path.join(root, fn)
                 if tgt_file not in src_file_listing:
-                    if fn == CONTROLLED_DIR_MARKER or layer_root.config.blacklist_files.search(fn):
+                    if fn == CONTROLLED_DIR_MARKER or config.blacklist_files.search(fn):
                         continue  # pragma: no cover (peephole optimization)
                     target_extra_files.add(tgt_file)
 
         for src_file in sorted(src_file_listing):
             # Source file must be in sort order (10-x is lower prio and therefore replaced by 90-z)
             sources = list(layer_root.get_file(src_file))
             src_files = [src.physical_path for src in sources]
-            dest_fn = sources[0].relative_path
+            try:
+                dest_fn = sources[0].logical_path
+            except IndexError:
+                self.stderr.write("File disappeared during execution?  {}\n".format(src_file))
+                return EXIT_CODE_NO_SUCH_FILE
+
             dest_path = os.path.join(args.target, dest_fn)
 
             # Make missing destination folder, if missing
@@ -191,9 +243,10 @@ def run(self, args):
                     for src in srcs:
                         src.close()
 
-        if True and target_extra_files:  # Todo: Allow for cleanup to be disabled via CLI
+        # Todo: Allow for cleanup to be disabled via CLI
+        if True and target_extra_files:
             self.stderr.write("Cleaning up extra files not part of source tree(s):  {0} files.\n".
-                format(len(target_extra_files)))
+                              format(len(target_extra_files)))
             for dest_fn in target_extra_files:
                 self.stderr.write("Remove unwanted file {0}\n".format(dest_fn))
                 os.unlink(os.path.join(args.target, dest_fn))

ksconf/consts.py

@@ -17,6 +17,7 @@
 EXIT_CODE_USER_QUIT = 2
 EXIT_CODE_NO_SUCH_FILE = 5
 EXIT_CODE_MISSING_ARG = 6
+EXIT_CODE_BAD_ARGS = 7
 
 EXIT_CODE_DIFF_EQUAL = 0
 EXIT_CODE_DIFF_CHANGE = 3

ksconf/layer.py

@@ -41,8 +41,6 @@
 """
 
 
-
-
 def _path_join(*parts):
     """ A slightly smarter / more flexible path appender.
     Drop any None or "." elements
@@ -51,6 +49,26 @@ def _path_join(*parts):
     return os.path.join(*parts)
 
 
+def path_in_layer(layer, path, sep=os.path.sep):
+    """ Check to see if path exist within layer.
+    Returns either None, or the path without the shared prefix with layer.
+    """
+    # Using 'sep' over os.path.join / os.path.split should be okay here as we should only ever be
+    # given relative paths (no Windows UNC/drive letters)
+    if layer is None:
+        # Return as-is, since layer is root
+        return path
+    layer_parts = layer.split(sep)
+    layer_count = len(layer_parts)
+    path_parts = path.split(sep)
+    if len(path_parts) < layer_count:
+        return False
+    path_suffix = path_parts[:layer_count]
+    if layer_parts != path_suffix:
+        return False
+    return sep.join(path_parts[layer_count:])
+
+
 # Exceptions
 
 class LayerException(Exception):
@@ -115,16 +133,25 @@ def walk(self):
 
         def list_files(self):
             File = self._file_cls
-            for (root, dirs, files) in self.walk():
+            for (top, dirs, files) in self.walk():
                 for file in files:
-                    yield File(self, _path_join(root, file))
+                    yield File(self, _path_join(top, file))
 
-        def get_file(self, rel_path):
-            """ Return file object, if it exists. """
+        def get_file(self, path):
+            """ Return file object (by logical path), if it exists in this layer. """
+            # XXX: There's probably ways to optimize this.  fine for now (correctness over speed)
             File = self._file_cls
+            rel_path = path_in_layer(self.logical_path, path)
+            if not rel_path:
+                return None
+            file_ = File(self, rel_path)
+            if os.path.isfile(file_.physical_path):
+                return file_
+            '''
             path_p = _path_join(self.root, self.physical_path, rel_path)
             if os.path.isfile(path_p):
                 return File(self, rel_path)
+            '''
 
     def __init__(self, config=None):
         self._layers = []
@@ -142,7 +169,7 @@ def list_layers(self):
         return self._layers
 
     def list_layer_names(self):
-        return [l.name for l in self._layers]
+        return [l.name for l in self.list_layers()]
 
     def list_files(self):
         """ Return a list of logical paths. """
@@ -262,8 +289,8 @@ def __init__(self, path):
     mount_regex = re.compile("(?P<realname>[\w_.-]+)\.d$")
     layer_regex = re.compile("(?P<layer>\d\d-[\w_.-]+)")
 
-    def __init__(self):
-        super(DotDLayerRoot, self).__init__()
+    def __init__(self, config=None):
+        super(DotDLayerRoot, self).__init__(config)
         #self.root = None
         self._root_layer = None
         self._mount_points = defaultdict(list)
@@ -285,8 +312,8 @@ def set_root(self, root):
                         # XXX: Nested layers breakage, must substitute multiple ".d" folders in `top`
                         layer = Layer(dir_mo.group("layer"),
                                       root,
-                                      os.path.join(root, top, dir_),
-                                      os.path.join(os.path.dirname(top), mount_mo.group("realname")),
+                                      physical=os.path.join(os.path.basename(top), dir_),
+                                      logical=mount_mo.group("realname"),
                                       config=self.config,
                                       file_cls=File)
                         self.add_layer(layer)

tests/test_cli_combine.py

@@ -15,8 +15,8 @@
 
 class CliKsconfCombineTestCase(unittest.TestCase):
 
-    def test_combine_3dir(self):
-        twd = TestWorkDir()
+
+    def build_test01(self, twd):
         twd.write_file("etc/apps/Splunk_TA_aws/default.d/10-upstream/props.conf", """
         [aws:config]
         SHOULD_LINEMERGE = false
@@ -72,6 +72,10 @@ def test_combine_3dir(self):
 
         </nav>
         """)
+
+    def test_combine_3dir(self):
+        twd = TestWorkDir()
+        self.build_test01(twd)
         default = twd.get_path("etc/apps/Splunk_TA_aws/default")
         with ksconf_cli:
             ko = ksconf_cli("combine", "--dry-run", "--target", default, default + ".d/*")
@@ -112,6 +116,23 @@ def test_combine_3dir(self):
         with ksconf_cli:
             ko = ksconf_cli("combine", "--target", default, default + ".d/*")
 
+    def test_combine_dird(self):
+        twd = TestWorkDir()
+        self.build_test01(twd)
+        default = twd.get_path("etc/apps/Splunk_TA_aws")
+        target = twd.get_path("etc/apps/Splunk_TA_aws-OUTPUT")
+        with ksconf_cli:
+            ko = ksconf_cli("combine", "--layer-method", "dir.d", "--dry-run", "--target", target, default)
+            ko = ksconf_cli("combine", "--layer-method", "dir.d", "--target", target, default)
+            self.assertEqual(ko.returncode, EXIT_CODE_SUCCESS)
+            cfg = parse_conf(target + "/default/props.conf")
+            self.assertIn("aws:config", cfg)
+            self.assertEqual(cfg["aws:config"]["ANNOTATE_PUNCT"], "true")
+            self.assertEqual(cfg["aws:config"]["EVAL-change_type"], '"configuration"')
+            self.assertEqual(cfg["aws:config"]["TRUNCATE"], '9999999')
+            nav_content = twd.read_file("etc/apps/Splunk_TA_aws-OUTPUT/default/data/ui/nav/default.xml")
+            self.assertIn("My custom view", nav_content)
+
     def test_require_arg(self):
         with ksconf_cli:
             ko = ksconf_cli("combine", "source-dir")

tests/test_layer.py

@@ -27,6 +27,28 @@
 from ksconf.layer import *
 
 
+def np(p, nominal="/"):
+    """ Transform a normalize path into an OS-specific path format """
+    if os.path.sep != nominal:
+        p = p.replace(nominal, os.path.sep)
+    return p
+
+def npl(iterable, nominal="/"):
+    return [np(i, nominal) for i in iterable]
+
+
+
+class TestHelperFunctionsTestCase(unittest.TestCase):
+
+    def test_path_in_layer_01(self):
+        path = np("default/data/ui/nav/default.xml")
+        self.assertEqual(path_in_layer("default", path), np("data/ui/nav/default.xml"))
+        self.assertEqual(path_in_layer("bin", path), False)
+        self.assertEqual(path_in_layer(np("a/path/longer/than/the/given/path"), path), False)
+
+    def test_path_in_layer_nulls(self):
+        self.assertEqual(path_in_layer(None, "path"), "path")
+
 
 
 class DefaultLayerTestCase(unittest.TestCase):
@@ -100,7 +122,7 @@ def test_defaultlayer_simple01(self):
         self.assertListEqual(layers, ["10-upstream", "20-corp", "60-dept"])
         # Order doesn't matter for file names
         expect_files = [
-            "data/ui/nav/default.xml".replace("/", os.path.sep),
+            np("data/ui/nav/default.xml"),
             "props.conf",
             "transforms.conf",
         ]
@@ -119,14 +141,14 @@ def test_dotd_simple01(self):
 
         self.maxDiff = 1000
         # Order doesn't matter for file names
-        expect_files = [
+        expect_files = npl([
             "bin/hello_world.py",
             "README.md",
             "default/data/ui/nav/default.xml",
             "default/props.conf",
             "default/transforms.conf",
-        ]
-        expect_files = sorted([f.replace("/", os.path.sep) for f in expect_files])
+        ])
+        expect_files = sorted([np(f) for f in expect_files])
         self.assertListEqual(sorted(dlr.list_files()), sorted(expect_files))