From 11f2e2f805db3664f91159929b049310a7fe33f4 Mon Sep 17 00:00:00 2001 From: jakub-nt <175944085+jakub-nt@users.noreply.github.com> Date: Tue, 4 Nov 2025 13:45:05 +0100 Subject: [PATCH 1/7] Fixed typo Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com> --- cfbs/commands.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cfbs/commands.py b/cfbs/commands.py index 69224a6a..ee0e57e5 100644 --- a/cfbs/commands.py +++ b/cfbs/commands.py @@ -718,7 +718,7 @@ def update_command(to_update): ) raise CFBSValidationError( "The cfbs.json was invalid before update, " - + "but updating modules did not fix it - aborting update" + + "but updating modules did not fix it - aborting update " + "(see validation error messages above)" ) config.save() From c434bd92c3a4843a085d8f335078462a42a87c0b Mon Sep 17 00:00:00 2001 From: jakub-nt <175944085+jakub-nt@users.noreply.github.com> Date: Tue, 4 Nov 2025 15:13:41 +0100 Subject: [PATCH 2/7] Added a new supported `cfbs.json` key: `"branch"` Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com> --- JSON.md | 4 ++++ cfbs/cfbs_json.py | 12 +++++++++--- cfbs/commands.py | 1 + cfbs/module.py | 1 + cfbs/pretty.py | 1 + cfbs/validate.py | 13 +++++++++++++ 6 files changed, 29 insertions(+), 3 deletions(-) diff --git a/JSON.md b/JSON.md index 73540b29..332ebe26 100644 --- a/JSON.md +++ b/JSON.md @@ -194,6 +194,10 @@ The modules inside `build`, `provides`, and `index` use these fields: - `commit` (string): Commit hash used when we download and snapshot the version of a module. Used in `index` and modules added from an index. Must be updated together with `version`. +- `branch` (string): Branch name used when updating modules added by URL. + Optional - if specified, the branch will be used during `cfbs update` instead of the default branch. + Requires the `url` field to be specified. + If the `branch` field is specified, the `commit` field must also be specified. - `subdirectory` (string): Used if the module is inside a subdirectory of a repo. See for example [the `cfbs.json` of our modules repo](https://github.com/cfengine/modules/blob/master/cfbs.json). Not used for local modules (policy files or folders) - the name is the path to the module in this case. diff --git a/cfbs/cfbs_json.py b/cfbs/cfbs_json.py index dc554366..439760e9 100644 --- a/cfbs/cfbs_json.py +++ b/cfbs/cfbs_json.py @@ -20,7 +20,7 @@ from cfbs.utils import CFBSValidationError, read_json, CFBSExitError -def _construct_provided_module(name, data, url, commit, added_by): +def _construct_provided_module(name, data, url, commit, branch, added_by): # At this point the @commit part should be removed from url so: # either url should not have an @, # or the @ should be for user@host.something @@ -35,6 +35,8 @@ def _construct_provided_module(name, data, url, commit, added_by): module["description"] = data["description"] module["url"] = url module["commit"] = commit + if branch is not None: + module["branch"] = branch subdirectory = data.get("subdirectory") if subdirectory: module["subdirectory"] = subdirectory @@ -61,11 +63,15 @@ def __init__( data=None, url=None, url_commit=None, + url_branch=None, ): assert path self.path = path + self.url = url self.url_commit = url_commit + self.url_branch = url_branch + if data: self._data = data else: @@ -184,7 +190,7 @@ def get_provides(self, added_by: Optional[str]): ) for k, v in self._data["provides"].items(): module = _construct_provided_module( - k, v, self.url, self.url_commit, added_by + k, v, self.url, self.url_commit, self.url_branch, added_by ) modules[k] = module return modules @@ -194,7 +200,7 @@ def get_module_for_build(self, name, added_by="cfbs add"): if "provides" in self._data and name in self._data["provides"]: module = self._data["provides"][name] return _construct_provided_module( - name, module, self.url, self.url_commit, added_by + name, module, self.url, self.url_commit, self.url_branch, added_by ) if name in self.index: return self.index.get_module_object(name, added_by) diff --git a/cfbs/commands.py b/cfbs/commands.py index ee0e57e5..e9a31300 100644 --- a/cfbs/commands.py +++ b/cfbs/commands.py @@ -960,6 +960,7 @@ def human_readable(key: str): "url", "repo", "version", + "branch", "commit", "by", "status", diff --git a/cfbs/module.py b/cfbs/module.py index 9106945c..a8c8029b 100644 --- a/cfbs/module.py +++ b/cfbs/module.py @@ -70,6 +70,7 @@ def attributes() -> tuple: return ( "name", "version", + "branch", "commit", "added_by", "steps", diff --git a/cfbs/pretty.py b/cfbs/pretty.py index def98f9e..9e99f1a8 100644 --- a/cfbs/pretty.py +++ b/cfbs/pretty.py @@ -21,6 +21,7 @@ "index", "version", "commit", + "branch", "subdirectory", "dependencies", "added_by", diff --git a/cfbs/validate.py b/cfbs/validate.py index 2763be8e..099d5ccd 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -502,6 +502,17 @@ def _validate_module_commit(name, module): raise CFBSValidationError(name, '"commit" must be a commit reference') +def _validate_module_branch(name, module): + assert "branch" in module + branch = module["branch"] + if type(branch) is not str: + raise CFBSValidationError(name, '"branch" must be of type string') + if not module["url"]: + raise CFBSValidationError(name, '"branch" key requires the "url" key') + if not module["commit"]: + raise CFBSValidationError(name, '"branch" key requires the "commit" key') + + def _validate_module_subdirectory(name, module): assert "subdirectory" in module if type(module["subdirectory"]) is not str: @@ -741,6 +752,8 @@ def validate_single_module(context, name, module, config, local_check=False): _validate_module_version(name, module) if "commit" in module: _validate_module_commit(name, module) + if "branch" in module: + _validate_module_branch(name, module) if "subdirectory" in module: _validate_module_subdirectory(name, module) if "steps" in module: From 85f41823486db11eb21e45fb51e49ff0fa9b4ac7 Mon Sep 17 00:00:00 2001 From: jakub-nt <175944085+jakub-nt@users.noreply.github.com> Date: Tue, 4 Nov 2025 16:20:41 +0100 Subject: [PATCH 3/7] Implemented branch support for updating modules added by URL Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com> --- cfbs/cfbs_config.py | 23 ++++++++++++++--------- cfbs/cfbs_json.py | 2 +- cfbs/commands.py | 22 +++++++++------------- cfbs/internal_file_management.py | 20 +++++++++++++++++--- 4 files changed, 41 insertions(+), 26 deletions(-) diff --git a/cfbs/cfbs_config.py b/cfbs/cfbs_config.py index 6a98c99b..7d0e5565 100644 --- a/cfbs/cfbs_config.py +++ b/cfbs/cfbs_config.py @@ -150,27 +150,32 @@ def _add_using_url( checksum=None, explicit_build_steps=None, ): - """The `url` argument can optionally also have the form `@`.""" + """The `url` argument can optionally also have the form `@`.""" url_commit = None + url_branch = None if url.endswith(SUPPORTED_ARCHIVES): config_path, url_commit = fetch_archive(url, checksum) else: assert url.startswith(SUPPORTED_URI_SCHEMES) - commit = None + reference = None if "@" in url and (url.rindex("@") > url.rindex(".")): - # commit specified in the url - url, commit = url.rsplit("@", 1) + # commit or branch specified together with the URL + url, reference = url.rsplit("@", 1) if "@" in url and (url.rindex("@") > url.rindex(".")): raise CFBSUserError( - "Cannot specify more than one commit for one add URL" + "Cannot specify more than one commit or branch for one add URL" ) - if not is_a_commit_hash(commit): - raise CFBSExitError("'%s' is not a commit reference" % commit) + config_path, url_commit = clone_url_repo(url, reference) - config_path, url_commit = clone_url_repo(url, commit) + # a branch name and a commit hash are distinguished as follows: + # if the reference matches the form of a commit hash, it is assumed to be a commit hash, otherwise it is assumed to be a branch name + if not is_a_commit_hash(reference): + url_branch = reference - remote_config = CFBSJson(path=config_path, url=url, url_commit=url_commit) + remote_config = CFBSJson( + path=config_path, url=url, url_commit=url_commit, url_branch=url_branch + ) provides = remote_config.get_provides(added_by) add_all = True diff --git a/cfbs/cfbs_json.py b/cfbs/cfbs_json.py index 439760e9..042b905c 100644 --- a/cfbs/cfbs_json.py +++ b/cfbs/cfbs_json.py @@ -62,7 +62,7 @@ def __init__( index_argument=None, data=None, url=None, - url_commit=None, + url_commit: Optional[str] = None, url_branch=None, ): assert path diff --git a/cfbs/commands.py b/cfbs/commands.py index e9a31300..6e9b1ca4 100644 --- a/cfbs/commands.py +++ b/cfbs/commands.py @@ -113,7 +113,6 @@ def search_command(terms: List[str]): git_configure_and_initialize, is_git_repo, CFBSGitError, - ls_remote, ) from cfbs.git_magic import commit_after_command, git_commit_maybe_prompt @@ -281,16 +280,12 @@ def init_command( log.debug("--masterfiles=%s appears to be a version number" % masterfiles) to_add = ["masterfiles@%s" % masterfiles] elif masterfiles != "no": - log.debug("--masterfiles=%s appears to be a branch" % masterfiles) - branch = masterfiles - remote = "https://github.com/cfengine/masterfiles" - commit = ls_remote(remote, branch) - if commit is None: - raise CFBSExitError( - "Failed to find branch or tag %s at remote %s" % (branch, remote) - ) - log.debug("Current commit for masterfiles branch %s is %s" % (branch, commit)) - to_add = ["%s@%s" % (remote, commit), "masterfiles"] + log.debug( + "--masterfiles=%s appears to be a branch or a commit hash" % masterfiles + ) + # add masterfiles from URL, instead of index by name + MPF_REPO_URL = "https://github.com/cfengine/masterfiles" + to_add = ["%s@%s" % (MPF_REPO_URL, masterfiles), "masterfiles"] if to_add: result = add_command(to_add, added_by="cfbs init") if result != 0: @@ -625,9 +620,10 @@ def update_command(to_update): log.warning("Module '%s' not in build. Skipping its update." % update.name) continue if "url" in old_module: - path, commit = clone_url_repo(old_module["url"]) + branch = old_module.get("branch") + path, commit = clone_url_repo(old_module["url"], branch) remote_config = CFBSJson( - path=path, url=old_module["url"], url_commit=commit + path=path, url=old_module["url"], url_commit=commit, url_branch=branch ) module_name = old_module["name"] diff --git a/cfbs/internal_file_management.py b/cfbs/internal_file_management.py index dcb0f4e2..842c39df 100644 --- a/cfbs/internal_file_management.py +++ b/cfbs/internal_file_management.py @@ -13,6 +13,7 @@ import shutil from typing import Optional +from cfbs.git import ls_remote from cfbs.utils import ( cfbs_dir, cp, @@ -156,8 +157,9 @@ def _clone_and_checkout(url, path, treeish): sh("git checkout " + treeish, directory=path) -def clone_url_repo(repo_url: str, commit: Optional[str] = None): - """Clones a Git repository at `repo_url` URL, optionally checking out the `commit` commit. +def clone_url_repo(repo_url: str, reference: Optional[str] = None): + """Clones a Git repository at `repo_url` URL, optionally checking out the `reference` commit or branch. + If `reference` is `None`, the repository's default branch will be used for the checkout. Returns path to the `cfbs.json` located in the cloned Git repository, and the Git commit hash. """ @@ -170,7 +172,19 @@ def clone_url_repo(repo_url: str, commit: Optional[str] = None): repo_dir = os.path.join(downloads, repo_path) os.makedirs(repo_dir, exist_ok=True) - if commit is not None: + # always store versions of the repository in cfbs/downloads by commit hash + # therefore for branches, first find the commit it points to + if reference is not None: + if is_a_commit_hash(reference): + commit = reference + else: + # `reference` is a branch + commit = ls_remote(repo_url, reference) + if commit is None: + raise CFBSExitError( + "Failed to find branch %s at %s" % (reference, repo_url) + ) + commit_path = os.path.join(repo_dir, commit) _clone_and_checkout(repo_url, commit_path, commit) else: From c3a0b95c5943f439d6b88ac811936c43b100cb74 Mon Sep 17 00:00:00 2001 From: jakub-nt <175944085+jakub-nt@users.noreply.github.com> Date: Tue, 4 Nov 2025 20:50:15 +0100 Subject: [PATCH 4/7] Fixed a throw when a module is added by URL without a specified branch Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com> --- cfbs/utils.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/cfbs/utils.py b/cfbs/utils.py index 281fbde9..abb9afc7 100644 --- a/cfbs/utils.py +++ b/cfbs/utils.py @@ -12,7 +12,7 @@ import urllib.error from collections import OrderedDict from shutil import rmtree -from typing import Iterable, List, Tuple, Union +from typing import Iterable, List, Optional, Tuple, Union from cfbs.pretty import pretty @@ -532,7 +532,9 @@ def fetch_url(url, target, checksum=None): ) from e -def is_a_commit_hash(commit): +def is_a_commit_hash(commit: Optional[str]): + if commit is None: + return False return bool(SHA1_RE.match(commit) or SHA256_RE.match(commit)) From b37ed44b707d8599c302798d31298104222ed506 Mon Sep 17 00:00:00 2001 From: jakub-nt <175944085+jakub-nt@users.noreply.github.com> Date: Tue, 4 Nov 2025 21:03:36 +0100 Subject: [PATCH 5/7] Simplified Git repository cloning logic Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com> --- cfbs/internal_file_management.py | 50 +++++++++----------------------- 1 file changed, 14 insertions(+), 36 deletions(-) diff --git a/cfbs/internal_file_management.py b/cfbs/internal_file_management.py index 842c39df..2ced1aba 100644 --- a/cfbs/internal_file_management.py +++ b/cfbs/internal_file_management.py @@ -137,19 +137,6 @@ def _get_path_from_url(url): return path -def _get_git_repo_commit_sha(repo_path): - assert os.path.isdir(os.path.join(repo_path, ".git")) - - with open(os.path.join(repo_path, ".git", "HEAD"), "r") as f: - head_ref_info = f.read() - - assert head_ref_info.startswith("ref: ") - head_ref = head_ref_info[5:].strip() - - with open(os.path.join(repo_path, ".git", head_ref)) as f: - return f.read().strip() - - def _clone_and_checkout(url, path, treeish): # NOTE: If any of these shell (git) commands fail, we will exit if not os.path.exists(os.path.join(path, ".git")): @@ -172,32 +159,23 @@ def clone_url_repo(repo_url: str, reference: Optional[str] = None): repo_dir = os.path.join(downloads, repo_path) os.makedirs(repo_dir, exist_ok=True) + if reference is None: + reference = "HEAD" + # always store versions of the repository in cfbs/downloads by commit hash # therefore for branches, first find the commit it points to - if reference is not None: - if is_a_commit_hash(reference): - commit = reference - else: - # `reference` is a branch - commit = ls_remote(repo_url, reference) - if commit is None: - raise CFBSExitError( - "Failed to find branch %s at %s" % (reference, repo_url) - ) - - commit_path = os.path.join(repo_dir, commit) - _clone_and_checkout(repo_url, commit_path, commit) + if is_a_commit_hash(reference): + commit = reference else: - master_path = os.path.join(repo_dir, "master") - sh("git clone %s %s" % (repo_url, master_path)) - commit = _get_git_repo_commit_sha(master_path) - - commit_path = os.path.join(repo_dir, commit) - if os.path.exists(commit_path): - # Already cloned in the commit dir, just remove the 'master' clone - sh("rm -rf %s" % master_path) - else: - sh("mv %s %s" % (master_path, commit_path)) + # `reference` is a branch + commit = ls_remote(repo_url, reference) + if commit is None: + raise CFBSExitError( + "Failed to find branch %s at %s" % (reference, repo_url) + ) + + commit_path = os.path.join(repo_dir, commit) + _clone_and_checkout(repo_url, commit_path, commit) json_path = os.path.join(commit_path, "cfbs.json") if os.path.exists(json_path): From ebdf0b7a4ac6f5577d0207ea779307bfdaa43e41 Mon Sep 17 00:00:00 2001 From: jakub-nt <175944085+jakub-nt@users.noreply.github.com> Date: Wed, 5 Nov 2025 12:38:32 +0100 Subject: [PATCH 6/7] Added error handling to prevent checking out non-existent references Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com> --- cfbs/git.py | 13 +++++++++++++ cfbs/internal_file_management.py | 5 ++++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/cfbs/git.py b/cfbs/git.py index c46ee679..d9ea07a3 100644 --- a/cfbs/git.py +++ b/cfbs/git.py @@ -249,3 +249,16 @@ def git_check_tracked_changes(scope=["all"]): raise CFBSGitError( "Failed to run 'git status -s -u' to check for changes." ) from cpe + + +def treeish_exists(treeish, repo_path): + command = [ + "git", + "rev-parse", + "--verify", + "--end-of-options", + treeish + r"^{object}", + ] + result = run(command, cwd=repo_path, stdout=DEVNULL, stderr=DEVNULL, check=False) + + return result.returncode == 0 diff --git a/cfbs/internal_file_management.py b/cfbs/internal_file_management.py index 2ced1aba..b4a0a636 100644 --- a/cfbs/internal_file_management.py +++ b/cfbs/internal_file_management.py @@ -13,7 +13,7 @@ import shutil from typing import Optional -from cfbs.git import ls_remote +from cfbs.git import ls_remote, treeish_exists from cfbs.utils import ( cfbs_dir, cp, @@ -141,6 +141,9 @@ def _clone_and_checkout(url, path, treeish): # NOTE: If any of these shell (git) commands fail, we will exit if not os.path.exists(os.path.join(path, ".git")): sh("git clone --no-checkout %s %s" % (url, path)) + if not treeish_exists(treeish, path): + raise CFBSExitError("%s not found in %s" % (treeish, url)) + sh("git checkout " + treeish, directory=path) From 02338a1a4ff1dbe3b6cd131cf49abcaa7d6aaed6 Mon Sep 17 00:00:00 2001 From: jakub-nt <175944085+jakub-nt@users.noreply.github.com> Date: Wed, 5 Nov 2025 13:29:53 +0100 Subject: [PATCH 7/7] Added tests for the functionality of updating by branch Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com> --- .../045_update_from_url_branch_uptodate.sh | 17 +++++++++++++++ tests/shell/046_update_from_url_branch.sh | 16 ++++++++++++++ .../046_update_from_url_branch/cfbs.json | 21 +++++++++++++++++++ tests/shell/all.sh | 2 ++ 4 files changed, 56 insertions(+) create mode 100644 tests/shell/045_update_from_url_branch_uptodate.sh create mode 100644 tests/shell/046_update_from_url_branch.sh create mode 100644 tests/shell/046_update_from_url_branch/cfbs.json diff --git a/tests/shell/045_update_from_url_branch_uptodate.sh b/tests/shell/045_update_from_url_branch_uptodate.sh new file mode 100644 index 00000000..f916f6e4 --- /dev/null +++ b/tests/shell/045_update_from_url_branch_uptodate.sh @@ -0,0 +1,17 @@ +set -e +set -x +cd tests/ +mkdir -p ./tmp/ +cd ./tmp/ +touch cfbs.json && rm cfbs.json +rm -rf .git + +cfbs --non-interactive init --masterfiles no +cfbs --non-interactive add https://github.com/cfengine/test-cfbs-static-repo/@update-test-branch test-library-parsed-local-users + +# check that cfbs.json contains the right commit hash (ideally for testing, different than the default branch's commit hash): +grep '"commit": "2152eb5a39fbf9b051105b400639b436bd53ab87"' cfbs.json +# check that branch key is correctly set: +grep '"branch": "update-test-branch"' cfbs.json + +cfbs update test-library-parsed-local-users | grep "Module 'test-library-parsed-local-users' already up to date" diff --git a/tests/shell/046_update_from_url_branch.sh b/tests/shell/046_update_from_url_branch.sh new file mode 100644 index 00000000..20f22fce --- /dev/null +++ b/tests/shell/046_update_from_url_branch.sh @@ -0,0 +1,16 @@ +set -e +set -x +cd tests/ +mkdir -p ./tmp/ +cd ./tmp/ +touch cfbs.json && rm cfbs.json +rm -rf .git + +cfbs --non-interactive init --masterfiles no +cfbs --non-interactive add https://github.com/cfengine/test-cfbs-static-repo/@update-test-branch test-library-parsed-local-users + +cp ../shell/046_update_from_url_branch/cfbs.json . + +cfbs update test-library-parsed-local-users | grep "Updated module 'test-library-parsed-local-users' from url" +# check that the commit hash changed: +grep '"commit": "2152eb5a39fbf9b051105b400639b436bd53ab87"' cfbs.json diff --git a/tests/shell/046_update_from_url_branch/cfbs.json b/tests/shell/046_update_from_url_branch/cfbs.json new file mode 100644 index 00000000..fa3b4784 --- /dev/null +++ b/tests/shell/046_update_from_url_branch/cfbs.json @@ -0,0 +1,21 @@ +{ + "name": "Example project", + "description": "Example description", + "type": "policy-set", + "git": true, + "build": [ + { + "name": "test-library-parsed-local-users", + "description": "Parse local users from /etc/passwd on the system with their attributes from /etc/shadow", + "url": "https://github.com/cfengine/test-cfbs-static-repo/", + "commit": "f702461f319e170db05bcb61f867b440b5cbd013", + "branch": "update-test-branch", + "subdirectory": "test_library_parsed_etc_passwd_shadow", + "added_by": "cfbs add", + "steps": [ + "copy ./test_library_parsed_etc_passwd_shadow.cf services/local-users/test_library_parsed_etc_passwd_shadow/", + "json cfbs/def.json def.json" + ] + } + ] +} diff --git a/tests/shell/all.sh b/tests/shell/all.sh index ab083a0e..1e3334bf 100644 --- a/tests/shell/all.sh +++ b/tests/shell/all.sh @@ -48,5 +48,7 @@ bash tests/shell/041_add_multidep.sh bash tests/shell/042_update_from_url.sh 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 echo "All cfbs shell tests completed successfully!"