ENT-12974: Implemented absolute path Git repository modules

JSON.md

@@ -174,9 +174,10 @@ The modules inside `build`, `provides`, and `index` use these fields:
   For `provides` and `index` dictionaries, this name must be the key of each entry (not a field inside).
   For the `build` array, it must be inside each module object (with `name` as the key).
   Local modules (files and folders in same directory as `cfbs.json`), must start with `./`, and end with `/` if it's a directory.
+  Absolute modules (a directory given by absolute path containing a Git repository) must start with `/` and end with `/`.
   Module names should not be longer than 64 characters.
-  Module names (not including adfixes `./`, `/`, `.cf`, `.json` for local modules) should only contain lowercase ASCII alphanumeric characters possibly separated by dashes, and should start with a letter.
-  Local module names can contain underscores instead of dashes.
+  Module names (not including adfixes `./`, `/`, `.cf`, `.json` for local and absolute modules) should only contain lowercase ASCII alphanumeric characters possibly separated by dashes, and should start with a letter.
+  Local and absolute module names can contain underscores instead of dashes.
 - `description` (string): Human readable description of what this module does.
 - `tags` (array of strings): Mostly used for information / finding modules on [build.cfengine.com](https://build.cfengine.com).
   Some common examples include `supported`, `experimental`, `security`, `library`, `promise-type`.

cfbs/cfbs_config.py

@@ -367,6 +367,23 @@ def _handle_local_module(self, module, use_default_build_steps=True):
             self._add_policy_files_build_step(module)
             self._add_bundles_build_step(module, policy_files)
 
+    def _handle_absolute_module(self, module):
+        name = module["name"]
+        if not (
+            name.startswith("/") and name.endswith("/") and "absolute" in module["tags"]
+        ):
+            log.debug("Module '%s' does not appear to be an absolute module" % name)
+            return
+
+        pattern = "%s/**/*.cf" % name
+        policy_files = glob.glob(pattern, recursive=True)
+
+        # TODO: handle absolute modules with autorun tag
+
+        # TODO: check that these build steps are written correctly
+        self._add_policy_files_build_step(module)
+        self._add_bundles_build_step(module, policy_files)
+
     def _add_without_dependencies(self, modules, use_default_build_steps=True):
         """Note: `use_default_build_steps` is only relevant for local modules."""
         if not use_default_build_steps:
@@ -392,6 +409,7 @@ def _add_without_dependencies(self, modules, use_default_build_steps=True):
             )
             self["build"].append(module)
             self._handle_local_module(module, use_default_build_steps)
+            self._handle_absolute_module(module)
 
             assert "added_by" in module
             added_by = module["added_by"]

cfbs/commands.py

@@ -104,6 +104,7 @@ def search_command(terms: List[str]):
     validate_single_module,
 )
 from cfbs.internal_file_management import (
+    absolute_module_copy,
     clone_url_repo,
     SUPPORTED_URI_SCHEMES,
     fetch_archive,
@@ -120,7 +121,7 @@ def search_command(terms: List[str]):
 
 from cfbs.git_magic import commit_after_command, git_commit_maybe_prompt
 from cfbs.prompts import prompt_user, prompt_user_yesno
-from cfbs.module import Module, is_module_added_manually
+from cfbs.module import Module, is_module_absolute, is_module_added_manually
 from cfbs.masterfiles.generate_release_information import generate_release_information
 
 _MODULES_URL = "https://archive.build.cfengine.com/modules"
@@ -634,8 +635,11 @@ def update_command(to_update):
                 continue
 
             new_module = provides[module_name]
+        elif is_module_absolute(old_module["name"]):
+            # TODO: update an absolute module
+            # check the HEAD at path, update commit
+            new_module = old_module
         else:
-
             if "version" not in old_module:
                 log.warning(
                     "Module '%s' not updatable. Skipping its update."
@@ -806,6 +810,10 @@ def _download_dependencies(config: CFBSConfig, redownload=False, ignore_versions
             local_module_copy(module, counter, max_length)
             counter += 1
             continue
+        if name.startswith("/"):
+            absolute_module_copy(module, counter, max_length)
+            counter += 1
+            continue
         if "commit" not in module:
             raise CFBSExitError("module %s must have a commit property" % name)
         commit = module["commit"]
@@ -902,6 +910,7 @@ def build_command(ignore_versions=False, diffs_filename=None):
         # We want the cfbs build command to be as backwards compatible as possible,
         # so we try building anyway and don't return error(s)
     init_out_folder()
+    # TODO: handle build of absolute module
     _download_dependencies(config, ignore_versions=ignore_versions)
     r = perform_build(config, diffs_filename)
     return r

cfbs/git.py

@@ -258,3 +258,15 @@ def treeish_exists(treeish, repo_path):
     result = run(command, cwd=repo_path, stdout=DEVNULL, stderr=DEVNULL, check=False)
 
     return result.returncode == 0
+
+
+def head_commit_hash(repo_path):
+    result = run(
+        ["git", "rev-parse", "HEAD"],
+        cwd=repo_path,
+        stdout=PIPE,
+        stderr=DEVNULL,
+        check=True,
+    )
+
+    return result.stdout.decode("utf-8").strip()

cfbs/index.py

@@ -3,9 +3,10 @@
 from collections import OrderedDict
 from typing import List, Optional, Union
 
-from cfbs.module import Module
+from cfbs.git import head_commit_hash
+from cfbs.module import Module, is_module_absolute
 from cfbs.utils import CFBSNetworkError, get_or_read_json, CFBSExitError, get_json
-from cfbs.internal_file_management import local_module_name
+from cfbs.internal_file_management import absolute_module_name, local_module_name
 
 _DEFAULT_INDEX = (
     "https://raw.githubusercontent.com/cfengine/build-index/master/cfbs.json"
@@ -48,6 +49,30 @@ def _local_module_data_subdir(
         "description": "Local subdirectory added using cfbs command line",
         "tags": ["local"],
         "steps": build_steps,
+        # TODO: turn this into an argument, for when it's not "cfbs add" adding the module
+        "added_by": "cfbs add",
+    }
+
+
+def _absolute_module_data(module_name: str, version: Optional[str]):
+    assert module_name.startswith("/")
+    assert module_name.endswith("/")
+
+    if version is not None:
+        commit_hash = version
+    else:
+        # TODO: validate Git repository earlier?
+        commit_hash = head_commit_hash(module_name)
+        print("|%s|" % commit_hash)
+
+    dst = os.path.join("services", "cfbs", module_name[1:])
+    build_steps = ["directory ./ {}".format(dst)]
+    return {
+        "description": "Module added via absolute path to a Git repository directory",
+        "tags": ["absolute"],
+        "steps": build_steps,
+        "commit": commit_hash,
+        # TODO: turn this into an argument, for when it's not "cfbs add" adding the module
         "added_by": "cfbs add",
     }
 
@@ -67,6 +92,14 @@ def _generate_local_module_object(
         return _local_module_data_json_file(module_name)
 
 
+def _generate_absolute_module_object(module_name: str, version: Optional[str]):
+    assert module_name.startswith("/")
+    assert module_name.endswith("/")
+    assert os.path.isdir(module_name)
+
+    return _absolute_module_data(module_name, version)
+
+
 class Index:
     """Class representing the cfbs.json containing the index of available modules"""
 
@@ -171,7 +204,10 @@ def translate_alias(self, module: Module):
                 module.name = data["alias"]
         else:
             if os.path.exists(module.name):
-                module.name = local_module_name(module.name)
+                if module.name.startswith("/"):
+                    module.name = absolute_module_name(module.name)
+                else:
+                    module.name = local_module_name(module.name)
 
     def get_module_object(
         self,
@@ -187,6 +223,12 @@ def get_module_object(
 
         if name.startswith("./"):
             object = _generate_local_module_object(name, explicit_build_steps)
+        elif is_module_absolute(name):
+            object = _generate_absolute_module_object(name, version)
+            # currently, the argument of cfbs-add is split by `@` in the `Module` constructor
+            # due to that, this hack is used to prevent creating the "version" field
+            module = Module(name).to_dict()
+            # TODO: a refactor to better handle arguments to cfbs-add that aren't just a name
         else:
             object = self[name]
             if version:

cfbs/internal_file_management.py

@@ -33,7 +33,7 @@
 SUPPORTED_URI_SCHEMES = ("https://", "ssh://", "git://")
 
 
-def local_module_name(module_path):
+def local_module_name(module_path: str):
     assert os.path.exists(module_path)
     module = module_path
 
@@ -64,6 +64,27 @@ def local_module_name(module_path):
     return module
 
 
+def absolute_module_name(module_path: str):
+    assert os.path.exists(module_path)
+    module = module_path
+    assert module.startswith("/")
+
+    for illegal in ["//", "..", " ", "\n", "\t", " "]:
+        if illegal in module:
+            raise CFBSExitError("Module path cannot contain %s" % repr(illegal))
+
+    if not module.endswith("/"):
+        module = module + "/"
+    while "/./" in module:
+        module = module.replace("/./", "/")
+
+    assert os.path.exists(module)
+    if not os.path.isdir(module):
+        raise CFBSExitError("'%s' must be a directory" % module)
+
+    return module
+
+
 def get_download_path(module) -> str:
     downloads = os.path.join(cfbs_dir(), "downloads")
 
@@ -117,6 +138,12 @@ def local_module_copy(module, counter, max_length):
     )
 
 
+def absolute_module_copy(module, counter, max_length):
+    # TODO: handle building an absolute module
+    # directory copy, Git checkout, etc.
+    return
+
+
 def _get_path_from_url(url):
     if not url.startswith(SUPPORTED_URI_SCHEMES):
         if "://" in url:

cfbs/module.py

@@ -15,6 +15,13 @@ def is_module_local(name: str):
     return name.startswith("./")
 
 
+def is_module_absolute(name: str):
+    """A module might contain `"absolute"` in its `"tags"` but this is not required.
+    The source of truth for whether the module is absolute is whether it starts with `/`.
+    """
+    return name.startswith("/")
+
+
 class Module:
     """Class representing a module in cfbs.json"""
 

cfbs/validate.py

@@ -31,14 +31,17 @@
 """
 
 import logging as log
+import os
 import re
 from collections import OrderedDict
 from typing import List, Tuple
 
-from cfbs.module import is_module_local
+from cfbs.git import is_git_repo, treeish_exists
+from cfbs.module import is_module_absolute, is_module_local
 from cfbs.utils import (
     is_a_commit_hash,
     strip_left,
+    strip_right,
     strip_right_any,
     CFBSExitError,
     CFBSValidationError,
@@ -169,16 +172,50 @@ def _validate_top_level_keys(config):
             )
 
 
-def validate_module_name_content(name):
-    MAX_MODULE_NAME_LENGTH = 64
+def validate_absolute_module(name_path, module):
+    if not (os.path.exists(name_path) and os.path.isdir(name_path)):
+        raise CFBSValidationError(
+            name_path,
+            "Absolute module's directory does not exist",
+        )
+
+    if not is_git_repo(name_path):
+        raise CFBSValidationError(name_path, "Absolute module is not a Git repository")
 
-    if len(name) > MAX_MODULE_NAME_LENGTH:
+    if not module["commit"]:
         raise CFBSValidationError(
-            name,
-            "Module name is too long (over "
-            + str(MAX_MODULE_NAME_LENGTH)
-            + " characters)",
+            name_path, 'Absolute modules require the "commit" key'
         )
+    commit = module["commit"]
+
+    # TODO: unless a branch / tag name is a valid "commit" field value, this needs to be checked differently
+    if not treeish_exists(commit, name_path):
+        raise CFBSValidationError(
+            name_path,
+            "Git repository of the absolute module does not contain the specified commit",
+        )
+
+
+def validate_module_name_content(name):
+    MAX_MODULE_NAME_LENGTH = 64
+
+    if is_module_absolute(name):
+        for component in name.split("/"):
+            if len(component) > MAX_MODULE_NAME_LENGTH:
+                raise CFBSValidationError(
+                    name,
+                    "Path component '%s' in module name is too long (over " % component
+                    + str(MAX_MODULE_NAME_LENGTH)
+                    + " characters)",
+                )
+    else:
+        if len(name) > MAX_MODULE_NAME_LENGTH:
+            raise CFBSValidationError(
+                name,
+                "Module name is too long (over "
+                + str(MAX_MODULE_NAME_LENGTH)
+                + " characters)",
+            )
 
     # lowercase ASCII alphanumericals, starting with a letter, and possible singular dashes in the middle
     r = "[a-z][a-z0-9]*(-[a-z0-9]+)*"
@@ -200,9 +237,33 @@ def validate_module_name_content(name):
         # only validate the local module's name, not the entire path to it
         proper_name = proper_name.split("/")[-1]
 
-        # allow underscores, only for local modules
+        # allow underscores, only for local and absolute modules
         proper_name = proper_name.replace("_", "-")
 
+    if is_module_absolute(name):
+        if not name.startswith("/"):
+            raise CFBSValidationError(
+                name, "Absolute module names should begin with `/`"
+            )
+
+        if not name.endswith("/"):
+            raise CFBSValidationError(
+                name,
+                "Absolute module names should end with `/`",
+            )
+        proper_name = strip_right(proper_name, "/")
+        # TODO: more validation of the entire path instead of just the name (final component), since the module "name" is actually a path
+        proper_name = proper_name.split("/")[-1]
+
+        # allow underscores, only for local and absolute modules
+        proper_name = proper_name.replace("_", "-")
+
+    if len(proper_name) == 0:
+        raise CFBSValidationError(
+            name,
+            "Module name proper is empty",
+        )
+
     if not re.fullmatch(r, proper_name):
         raise CFBSValidationError(
             name,
@@ -395,6 +456,7 @@ def _validate_module_alias(name, module, context, config):
 
 def _validate_module_name(name: str, module):
     assert "name" in module
+    # NOTE: this assert fails for an empty module name, preventing the latter `if not module["name"]` check, TODO re-write
     assert name
     assert type(name) is str
     assert module["name"]
@@ -778,6 +840,9 @@ def validate_single_module(context, name, module, config, local_check=False):
     if "name" not in module:
         validate_module_name_content(name)
 
+    if is_module_absolute(name):
+        validate_absolute_module(name, module)
+
 
 def _validate_config_for_build_field(config, empty_build_list_ok=False):
     """Validate that neccessary fields are in the config for the build/download commands to work"""

tests/shell/all.sh

@@ -50,5 +50,6 @@ bash tests/shell/043_replace_version.sh
 bash tests/shell/044_replace.sh
 bash tests/shell/045_update_from_url_branch_uptodate.sh
 bash tests/shell/046_update_from_url_branch.sh
+# TODO: add 047 test for a usual absolute module flow: init, add, build, update, build - including a sample directory
 
 echo "All cfbs shell tests completed successfully!"