diff --git a/JSON.md b/JSON.md index 4333293e..c9c9840d 100644 --- a/JSON.md +++ b/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`. diff --git a/cfbs/cfbs_config.py b/cfbs/cfbs_config.py index bec6b3e1..70f0f219 100644 --- a/cfbs/cfbs_config.py +++ b/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"] diff --git a/cfbs/commands.py b/cfbs/commands.py index 5ed4ed09..af4d5b4e 100644 --- a/cfbs/commands.py +++ b/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 diff --git a/cfbs/git.py b/cfbs/git.py index d79b1205..5cb033ec 100644 --- a/cfbs/git.py +++ b/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() diff --git a/cfbs/index.py b/cfbs/index.py index 4edda454..176d86bf 100644 --- a/cfbs/index.py +++ b/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: diff --git a/cfbs/internal_file_management.py b/cfbs/internal_file_management.py index b4a0a636..1c9e4627 100644 --- a/cfbs/internal_file_management.py +++ b/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: diff --git a/cfbs/module.py b/cfbs/module.py index a8c8029b..8ea9e901 100644 --- a/cfbs/module.py +++ b/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""" diff --git a/cfbs/validate.py b/cfbs/validate.py index 15ff5755..b8168ad3 100644 --- a/cfbs/validate.py +++ b/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""" diff --git a/tests/shell/all.sh b/tests/shell/all.sh index 1e3334bf..2357ebe3 100644 --- a/tests/shell/all.sh +++ b/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!"