From 4d063f755bcd892ee7f99a88773a805054711d19 Mon Sep 17 00:00:00 2001 From: Victor Moene Date: Fri, 6 Jun 2025 17:40:23 +0200 Subject: [PATCH 1/5] Moved update_command module update to its own function The part of the loop in update_command responsible for actually updating modules was moved to its own function. It is done by passing a module updates class that tracks the updates generally. Signed-off-by: Victor Moene --- cfbs/commands.py | 251 ++++++++++++++++++++++++++--------------------- 1 file changed, 141 insertions(+), 110 deletions(-) diff --git a/cfbs/commands.py b/cfbs/commands.py index 00b44597..eed4a457 100644 --- a/cfbs/commands.py +++ b/cfbs/commands.py @@ -665,6 +665,109 @@ def _update_variable(input_def, input_data): return changes_made +class ModuleUpdates: + + def __init__(self, config): + self.new_deps = [] + self.new_deps_added_by = dict() + self.changes_made = False + self.files = [] + self.config = config + self.msg = "" + + +def update_module(old_module, new_module, module_updates, update): + commit_differs = old_module["commit"] != new_module["commit"] + for key in old_module.keys(): + if key not in new_module or old_module[key] == new_module[key]: + continue + if key == "steps": + # same commit => user modifications, don't revert them + if commit_differs: + ans = prompt_user( + module_updates.config.non_interactive, + "Module %s has different build steps now\n" % old_module["name"] + + "old steps: %s\n" % old_module["steps"] + + "new steps: %s\n" % new_module["steps"] + + "Do you want to use the new build steps?", + choices=YES_NO_CHOICES, + default="yes", + ) + if ans.lower() in ["y", "yes"]: + old_module["steps"] = new_module["steps"] + module_updates.changes_made = True + else: + print( + "Please make sure the old build steps work" + + " with the new version of the module" + ) + elif key == "input": + if commit_differs: + old_module["input"] = new_module["input"] + + input_path = os.path.join(".", old_module["name"], "input.json") + input_data = read_json(input_path) + if input_data == None: + log.debug( + "Skipping input update for module '%s': " + + "No input found in '%s'" % (old_module["name"], input_path) + ) + else: + try: + module_updates.changes_made |= update_input_data( + old_module, input_data + ) + except InputDataUpdateFailed as e: + log.warning(e) + if prompt_user( + module_updates.config.non_interactive, + "Input for module '%s' has changed " % old_module["name"] + + "and may no longer be compatible. " + + "Do you want to re-enter input now?", + YES_NO_CHOICES, + "no", + ).lower() in ("no", "n"): + continue + input_data = copy.deepcopy(old_module["input"]) + module_updates.config.input_command( + old_module["name"], input_data + ) + module_updates.changes_made = True + + if module_updates.changes_made: + write_json(input_path, input_data) + module_updates.files.append(input_path) + else: + if key == "dependencies": + extra = set(new_module["dependencies"]) - set( + old_module["dependencies"] + ) + module_updates.new_deps.extend(extra) + module_updates.new_deps_added_by.update( + {item: old_module["name"] for item in extra} + ) + + old_module[key] = new_module[key] + module_updates.changes_made = True + + for key in set(new_module.keys()) - set(old_module.keys()): + old_module[key] = new_module[key] + if key == "dependencies": + extra = new_module["dependencies"] + module_updates.new_deps.extend(extra) + module_updates.new_deps_added_by.update( + {item: old_module["name"] for item in extra} + ) + + if not update.version: + update.version = new_module["version"] + module_updates.msg += "\n - Updated module '%s' from version %s to version %s" % ( + update.name, + old_module["version"], + update.version, + ) + + @cfbs_command("update") @commit_after_command("Updated module%s", [PLURAL_S]) def update_command(to_update) -> Result: @@ -679,173 +782,101 @@ def update_command(to_update) -> Result: else [Module(m["name"]) for m in build] ) - new_deps = [] - new_deps_added_by = dict() - changes_made = False - msg = "" - files = [] updated = [] + module_updates = ModuleUpdates(config) for update in to_update: - module = config.get_module_from_build(update.name) + old_module = config.get_module_from_build(update.name) - custom_index = module is not None and "index" in module - index = Index(module["index"]) if custom_index else config.index + custom_index = old_module is not None and "index" in old_module + index = Index(old_module["index"]) if custom_index else config.index - if not module: + if not old_module: index.translate_alias(update) - module = config.get_module_from_build(update.name) + old_module = config.get_module_from_build(update.name) - if not module: - log.warning("Module '%s' not in build. Skipping its update." % update.name) + if not old_module: + log.warning( + "old_Module '%s' not in build. Skipping its update." % update.name + ) continue - custom_index = module is not None and "index" in module - index = Index(module["index"]) if custom_index else config.index + custom_index = old_module is not None and "index" in old_module + index = Index(old_module["index"]) if custom_index else config.index - if not module: + if not old_module: index.translate_alias(update) - module = config.get_module_from_build(update.name) + old_module = config.get_module_from_build(update.name) - if not module: + if not old_module: log.warning("Module '%s' not in build. Skipping its update." % update.name) continue - if "version" not in module: + if "version" not in old_module: log.warning( - "Module '%s' not updatable. Skipping its update." % module["name"] + "Module '%s' not updatable. Skipping its update." % old_module["name"] ) - log.debug("Module '%s' has no version attribute." % module["name"]) + log.debug("Module '%s' has no version attribute." % old_module["name"]) continue - old_version = module["version"] index_info = index.get_module_object(update.name) if not index_info: log.warning( "Module '%s' not present in the index, cannot update it." - % module["name"] + % old_module["name"] ) continue local_ver = [ int(version_number) - for version_number in re.split(r"[-\.]", module["version"]) + for version_number in re.split(r"[-\.]", old_module["version"]) ] index_ver = [ int(version_number) for version_number in re.split(r"[-\.]", index_info["version"]) ] if local_ver == index_ver: - print("Module '%s' already up to date" % module["name"]) + print("Module '%s' already up to date" % old_module["name"]) continue elif local_ver > index_ver: log.warning( "The requested version of module '%s' is older than current version (%s < %s)." " Skipping its update." - % (module["name"], index_info["version"], module["version"]) + % (old_module["name"], index_info["version"], old_module["version"]) ) continue - commit_differs = module["commit"] != index_info["commit"] - for key in module.keys(): - if key not in index_info or module[key] == index_info[key]: - continue - if key == "steps": - # same commit => user modifications, don't revert them - if commit_differs: - ans = prompt_user( - config.non_interactive, - "Module %s has different build steps now\n" % module["name"] - + "old steps: %s\n" % module["steps"] - + "new steps: %s\n" % index_info["steps"] - + "Do you want to use the new build steps?", - choices=YES_NO_CHOICES, - default="yes", - ) - if ans.lower() in ["y", "yes"]: - module["steps"] = index_info["steps"] - changes_made = True - else: - print( - "Please make sure the old build steps work" - + " with the new version of the module" - ) - elif key == "input": - if commit_differs: - module["input"] = index_info["input"] - - input_path = os.path.join(".", module["name"], "input.json") - input_data = read_json(input_path) - if input_data == None: - log.debug( - "Skipping input update for module '%s': " % module["name"] - + "No input found in '%s'" % input_path - ) - else: - try: - changes_made |= update_input_data(module, input_data) - except InputDataUpdateFailed as e: - log.warning(e) - if prompt_user( - config.non_interactive, - "Input for module '%s' has changed " % module["name"] - + "and may no longer be compatible. " - + "Do you want to re-enter input now?", - YES_NO_CHOICES, - "no", - ).lower() in ("no", "n"): - continue - input_data = copy.deepcopy(module["input"]) - config.input_command(module["name"], input_data) - changes_made = True - - if changes_made: - write_json(input_path, input_data) - files.append(input_path) - else: - if key == "dependencies": - extra = set(index_info["dependencies"]) - set( - module["dependencies"] - ) - new_deps.extend(extra) - new_deps_added_by.update({item: module["name"] for item in extra}) + new_module = index_info - module[key] = index_info[key] - changes_made = True + update_module(old_module, new_module, module_updates, update) # add new items - for key in set(index_info.keys()) - set(module.keys()): - module[key] = index_info[key] - if key == "dependencies": - extra = index_info["dependencies"] - new_deps.extend(extra) - new_deps_added_by.update({item: module["name"] for item in extra}) - if not update.version: - update.version = index_info["version"] updated.append(update) - msg += "\n - Updated module '%s' from version %s to version %s" % ( - update.name, - old_version, - update.version, - ) - if new_deps: - objects = [index.get_module_object(d, new_deps_added_by[d]) for d in new_deps] + if module_updates.new_deps: + objects = [ + index.get_module_object(d, module_updates.new_deps_added_by[d]) + for d in module_updates.new_deps + ] config.add_with_dependencies(objects) config.save() - if changes_made: + if module_updates.changes_made: if len(updated) > 1: - msg = "Updated %d modules\n" % len(updated) + msg + module_updates.msg = ( + "Updated %d modules\n" % len(updated) + module_updates.msg + ) else: # Remove the '\n - ' part of the message - msg = msg[len("\n - ") :] - print("%s\n" % msg) + module_updates.msg = module_updates.msg[len("\n - ") :] + print("%s\n" % module_updates.msg) else: print("Modules are already up to date") - return Result(0, changes_made, msg, files) + return Result( + 0, module_updates.changes_made, module_updates.msg, module_updates.files + ) @cfbs_command("validate") From 858fd35773dc43a10e8abcda372edec053f4b569 Mon Sep 17 00:00:00 2001 From: Victor Moene Date: Tue, 10 Jun 2025 10:27:49 +0200 Subject: [PATCH 2/5] Made possible to update modules added by url Ticket: CFE-3847 Signed-off-by: Victor Moene --- cfbs/commands.py | 121 +++++++++++++++++++++++++++++------------------ 1 file changed, 74 insertions(+), 47 deletions(-) diff --git a/cfbs/commands.py b/cfbs/commands.py index eed4a457..d805be7d 100644 --- a/cfbs/commands.py +++ b/cfbs/commands.py @@ -13,6 +13,7 @@ from cfbs.analyze import analyze_policyset from cfbs.args import get_args +from cfbs.cfbs_json import CFBSJson from cfbs.utils import ( FetchError, cfbs_dir, @@ -44,6 +45,7 @@ from cfbs.cfbs_config import CFBSConfig, CFBSReturnWithoutCommit from cfbs.validate import validate_config from cfbs.internal_file_management import ( + clone_url_repo, SUPPORTED_URI_SCHEMES, fetch_archive, get_download_path, @@ -678,6 +680,8 @@ def __init__(self, config): def update_module(old_module, new_module, module_updates, update): commit_differs = old_module["commit"] != new_module["commit"] + old_version = old_module.get("version") + local_changes_made = False for key in old_module.keys(): if key not in new_module or old_module[key] == new_module[key]: continue @@ -695,7 +699,7 @@ def update_module(old_module, new_module, module_updates, update): ) if ans.lower() in ["y", "yes"]: old_module["steps"] = new_module["steps"] - module_updates.changes_made = True + local_changes_made = True else: print( "Please make sure the old build steps work" @@ -709,14 +713,12 @@ def update_module(old_module, new_module, module_updates, update): input_data = read_json(input_path) if input_data == None: log.debug( - "Skipping input update for module '%s': " - + "No input found in '%s'" % (old_module["name"], input_path) + "Skipping input update for module '%s': " % old_module["name"] + + "No input found in '%s'" % input_path ) else: try: - module_updates.changes_made |= update_input_data( - old_module, input_data - ) + local_changes_made |= update_input_data(old_module, input_data) except InputDataUpdateFailed as e: log.warning(e) if prompt_user( @@ -732,9 +734,9 @@ def update_module(old_module, new_module, module_updates, update): module_updates.config.input_command( old_module["name"], input_data ) - module_updates.changes_made = True + local_changes_made = True - if module_updates.changes_made: + if local_changes_made: write_json(input_path, input_data) module_updates.files.append(input_path) else: @@ -748,7 +750,7 @@ def update_module(old_module, new_module, module_updates, update): ) old_module[key] = new_module[key] - module_updates.changes_made = True + local_changes_made = True for key in set(new_module.keys()) - set(old_module.keys()): old_module[key] = new_module[key] @@ -759,13 +761,22 @@ def update_module(old_module, new_module, module_updates, update): {item: old_module["name"] for item in extra} ) - if not update.version: - update.version = new_module["version"] - module_updates.msg += "\n - Updated module '%s' from version %s to version %s" % ( - update.name, - old_module["version"], - update.version, - ) + if local_changes_made: + if new_module.get("version"): + module_updates.msg += ( + "\n - Updated module '%s' from version %s to version %s" + % ( + update.name, + old_version, + update.version if update.version else new_module["version"], + ) + ) + else: + module_updates.msg += "\n - Updated module '%s' from url" % (update.name) + else: + print("Module '%s' already up to date" % old_module["name"]) + + module_updates.changes_made |= local_changes_made @cfbs_command("update") @@ -812,41 +823,57 @@ def update_command(to_update) -> Result: log.warning("Module '%s' not in build. Skipping its update." % update.name) continue - if "version" not in old_module: - log.warning( - "Module '%s' not updatable. Skipping its update." % old_module["name"] + if "url" in old_module: + path, commit = clone_url_repo(old_module["url"]) + remote_config = CFBSJson( + path=path, url=old_module["url"], url_commit=commit ) - log.debug("Module '%s' has no version attribute." % old_module["name"]) - continue - index_info = index.get_module_object(update.name) - if not index_info: - log.warning( - "Module '%s' not present in the index, cannot update it." - % old_module["name"] - ) - continue + module_name = old_module["name"] + provides = remote_config.get_provides() - local_ver = [ - int(version_number) - for version_number in re.split(r"[-\.]", old_module["version"]) - ] - index_ver = [ - int(version_number) - for version_number in re.split(r"[-\.]", index_info["version"]) - ] - if local_ver == index_ver: - print("Module '%s' already up to date" % old_module["name"]) - continue - elif local_ver > index_ver: - log.warning( - "The requested version of module '%s' is older than current version (%s < %s)." - " Skipping its update." - % (old_module["name"], index_info["version"], old_module["version"]) - ) - continue + if not module_name or module_name not in provides: + continue + + new_module = provides[module_name] + else: + + if "version" not in old_module: + log.warning( + "Module '%s' not updatable. Skipping its update." + % old_module["name"] + ) + log.debug("Module '%s' has no version attribute." % old_module["name"]) + continue + + index_info = index.get_module_object(update.name) + if not index_info: + log.warning( + "Module '%s' not present in the index, cannot update it." + % old_module["name"] + ) + continue + + local_ver = [ + int(version_number) + for version_number in re.split(r"[-\.]", old_module["version"]) + ] + index_ver = [ + int(version_number) + for version_number in re.split(r"[-\.]", index_info["version"]) + ] + if local_ver == index_ver: + print("Module '%s' already up to date" % old_module["name"]) + continue + elif local_ver > index_ver: + log.warning( + "The requested version of module '%s' is older than current version (%s < %s)." + " Skipping its update." + % (old_module["name"], index_info["version"], old_module["version"]) + ) + continue - new_module = index_info + new_module = index_info update_module(old_module, new_module, module_updates, update) From 1e068ce665c005330485917e7069ad3befe89c40 Mon Sep 17 00:00:00 2001 From: Victor Moene Date: Tue, 10 Jun 2025 12:41:54 +0200 Subject: [PATCH 3/5] Removed url field from some cfbs build examples in tests The url field makes the update command clone the repo. However, the github profile and repo link in the build examples are placeholders, and make the test fail. Signed-off-by: Victor Moene --- tests/shell/020_update_input_list/example-cfbs.json | 1 - tests/shell/021_update_input_list_with_keys/example-cfbs.json | 1 - 2 files changed, 2 deletions(-) diff --git a/tests/shell/020_update_input_list/example-cfbs.json b/tests/shell/020_update_input_list/example-cfbs.json index 84996ddb..02b4655d 100644 --- a/tests/shell/020_update_input_list/example-cfbs.json +++ b/tests/shell/020_update_input_list/example-cfbs.json @@ -34,7 +34,6 @@ { "name": "example-module", "description": "Example module", - "url": "https://github.com/example-user/example-modules.git", "commit": "0000000000000000000000000000000000000000", "by": "https://github.com/example-user", "version": "1.0.0", diff --git a/tests/shell/021_update_input_list_with_keys/example-cfbs.json b/tests/shell/021_update_input_list_with_keys/example-cfbs.json index 9b0f7cca..b5a4d7b6 100644 --- a/tests/shell/021_update_input_list_with_keys/example-cfbs.json +++ b/tests/shell/021_update_input_list_with_keys/example-cfbs.json @@ -44,7 +44,6 @@ { "name": "example-module", "description": "Example module", - "url": "https://github.com/example-user/example-modules.git", "commit": "0000000000000000000000000000000000000000", "by": "https://github.com/example-user", "version": "1.0.0", From ff0e9df7eb832d1b81caed2149c5de33081c3847 Mon Sep 17 00:00:00 2001 From: Victor Moene Date: Mon, 23 Jun 2025 15:12:13 +0200 Subject: [PATCH 4/5] Added tests for cfbs update for modules added by url. Signed-off-by: Victor Moene --- tests/shell/042_update_from_url.sh | 15 ++++++++ .../delete-files/input.json | 19 ++++++++++ .../042_update_from_url/example-cfbs.json | 38 +++++++++++++++++++ tests/shell/all.sh | 1 + 4 files changed, 73 insertions(+) create mode 100644 tests/shell/042_update_from_url.sh create mode 100644 tests/shell/042_update_from_url/delete-files/input.json create mode 100644 tests/shell/042_update_from_url/example-cfbs.json diff --git a/tests/shell/042_update_from_url.sh b/tests/shell/042_update_from_url.sh new file mode 100644 index 00000000..e5463029 --- /dev/null +++ b/tests/shell/042_update_from_url.sh @@ -0,0 +1,15 @@ +set -e +set -x +cd tests/ +mkdir -p ./tmp/ +cd ./tmp/ +touch cfbs.json && rm cfbs.json +rm -rf .git +rm -rf delete-files + +cp ../shell/042_update_from_url/example-cfbs.json cfbs.json +cp -r ../shell/042_update_from_url/delete-files . + +cfbs --loglevel=debug --non-interactive update +grep 'Specify another file you want deleted on your hosts?' cfbs.json +grep 'Why should this file be deleted?' cfbs.json diff --git a/tests/shell/042_update_from_url/delete-files/input.json b/tests/shell/042_update_from_url/delete-files/input.json new file mode 100644 index 00000000..26a8481b --- /dev/null +++ b/tests/shell/042_update_from_url/delete-files/input.json @@ -0,0 +1,19 @@ +[ + { + "type": "list", + "variable": "files", + "namespace": "delete_files", + "bundle": "delete_files", + "label": "Files", + "subtype": [ + { + "key": "path", + "type": "string", + "label": "Path", + "question": "Path to file" + } + ], + "while": "test?", + "response": ["/tmp/file"] + } +] diff --git a/tests/shell/042_update_from_url/example-cfbs.json b/tests/shell/042_update_from_url/example-cfbs.json new file mode 100644 index 00000000..f0fb3650 --- /dev/null +++ b/tests/shell/042_update_from_url/example-cfbs.json @@ -0,0 +1,38 @@ +{ + "name": "Example project", + "description": "Example description", + "type": "policy-set", + "git": true, + "build": [ + { + "name": "delete-files", + "description": "Allows you to specify a list of files you want deleted on hosts in your infrastructure. When this module is deployed as part of your policy set, every time CFEngine runs, it will check if those files exist, and delete them if they do.", + "url": "https://github.com/nickanderson/cfengine-delete-files.git", + "commit": "0000000000000000000000000000000000000000", + "added_by": "cfbs add", + "steps": [ + "copy delete-files.cf services/cfbs/modules/delete-files/delete-files.cf", + "input delete-files/input.json def.json", + "something test" + ], + "input": [ + { + "type": "list", + "variable": "files", + "namespace": "delete_files", + "bundle": "delete_files", + "label": "Files", + "subtype": [ + { + "key": "path", + "type": "string", + "label": "Path", + "question": "Path to file" + } + ], + "while": "test?" + } + ] + } + ] +} diff --git a/tests/shell/all.sh b/tests/shell/all.sh index 71d92ee5..259f2963 100644 --- a/tests/shell/all.sh +++ b/tests/shell/all.sh @@ -45,5 +45,6 @@ bash tests/shell/038_global_dir.sh bash tests/shell/039_add_added_by_field_update_1.sh bash tests/shell/040_add_added_by_field_update_2.sh bash tests/shell/041_add_multidep.sh +bash tests/shell/042_update_from_url.sh echo "All cfbs shell tests completed successfully!" From 8698f13981849ecf5b3d483629b4763af94e553c Mon Sep 17 00:00:00 2001 From: Victor Moene Date: Mon, 30 Jun 2025 14:39:43 +0200 Subject: [PATCH 5/5] Moved update commands to updates.py Signed-off-by: Victor Moene --- cfbs/commands.py | 221 +------------------------------------------ cfbs/updates.py | 225 ++++++++++++++++++++++++++++++++++++++++++++ tests/test_input.py | 2 +- 3 files changed, 227 insertions(+), 221 deletions(-) create mode 100644 cfbs/updates.py diff --git a/cfbs/commands.py b/cfbs/commands.py index d805be7d..9b3535d6 100644 --- a/cfbs/commands.py +++ b/cfbs/commands.py @@ -14,6 +14,7 @@ from cfbs.args import get_args from cfbs.cfbs_json import CFBSJson +from cfbs.updates import ModuleUpdates, update_module from cfbs.utils import ( FetchError, cfbs_dir, @@ -67,12 +68,6 @@ from cfbs.module import Module, is_module_added_manually from cfbs.masterfiles.generate_release_information import generate_release_information - -class InputDataUpdateFailed(Exception): - def __init__(self, message): - super().__init__(message) - - _MODULES_URL = "https://archive.build.cfengine.com/modules" PLURAL_S = lambda args, _: "s" if len(args[0]) > 1 else "" @@ -565,220 +560,6 @@ def _someone_needs_me(this) -> bool: return 0 -def update_input_data(module, input_data) -> bool: - """ - Update input data from module definition - - :param module: Module with updated input definition - :param input_data: Input data to update - :return: True if changes are made - """ - module_name = module["name"] - input_def = module["input"] - - if len(input_def) != len(input_data): - raise InputDataUpdateFailed( - "Failed to update input data for module '%s': " % module_name - + "Input definition has %d variables, " % len(input_def) - + "while current input data has %d variables." % len(input_data) - ) - - def _update_keys(input_def, input_data, keys): - """ - Update keys that can be safily updated in input data. - """ - changes_made = False - for key in keys: - new = input_def.get(key) - old = input_data.get(key) - if new != old: - # Make sure that one of the keys are not 'None' - if new is None or old is None: - raise InputDataUpdateFailed( - "Failed to update input data for module '%s': " % module_name - + "Missing matching attribute '%s'." % key - ) - input_data[key] = input_def[key] - changes_made = True - log.warning( - "Updated attribute '%s' from '%s' to '%s' in module '%s'." - % (key, old, new, module_name) - ) - return changes_made - - def _check_keys(input_def, input_data, keys): - """ - Compare keys that cannot safily be updated for equality. - """ - for key in keys: - new = input_def.get(key) - old = input_data.get(key) - if new != old: - raise InputDataUpdateFailed( - "Failed to update input data for module '%s': " % module_name - + "Updating attribute '%s' from '%s' to '%s'," % (key, old, new) - + "may cause module to break." - ) - - def _update_variable(input_def, input_data): - _check_keys(input_def, input_data, ("type", "namespace", "bundle", "variable")) - changes_made = _update_keys( - input_def, input_data, ("label", "comment", "question", "while", "default") - ) - - if input_def["type"] == "list": - def_subtype = input_def["subtype"] - data_subtype = input_data["subtype"] - if type(def_subtype) != type(data_subtype): - raise InputDataUpdateFailed( - "Failed to update input data for module '%s': " % module_name - + "Different subtypes in list ('%s' != '%s')." - % (type(def_subtype).__name__, type(data_subtype).__name__) - ) - if isinstance(def_subtype, list): - if len(def_subtype) != len(data_subtype): - raise InputDataUpdateFailed( - "Failed to update input data for module '%s': " % module_name - + "Different amount of elements in list ('%s' != '%s')." - % (len(def_subtype), len(data_subtype)) - ) - for i in range(len(def_subtype)): - _check_keys(def_subtype[i], data_subtype[i], ("key", "type")) - changes_made |= _update_keys( - def_subtype[i], - data_subtype[i], - ("label", "question", "default"), - ) - elif isinstance(def_subtype, dict): - _check_keys(def_subtype, data_subtype, ("type",)) - changes_made |= _update_keys( - def_subtype, data_subtype, ("label", "question", "default") - ) - else: - user_error( - "Unsupported subtype '%s' in input definition for module '%s'." - % (type(def_subtype).__name__, module_name) - ) - return changes_made - - changes_made = False - for i in range(len(input_def)): - changes_made |= _update_variable(input_def[i], input_data[i]) - return changes_made - - -class ModuleUpdates: - - def __init__(self, config): - self.new_deps = [] - self.new_deps_added_by = dict() - self.changes_made = False - self.files = [] - self.config = config - self.msg = "" - - -def update_module(old_module, new_module, module_updates, update): - commit_differs = old_module["commit"] != new_module["commit"] - old_version = old_module.get("version") - local_changes_made = False - for key in old_module.keys(): - if key not in new_module or old_module[key] == new_module[key]: - continue - if key == "steps": - # same commit => user modifications, don't revert them - if commit_differs: - ans = prompt_user( - module_updates.config.non_interactive, - "Module %s has different build steps now\n" % old_module["name"] - + "old steps: %s\n" % old_module["steps"] - + "new steps: %s\n" % new_module["steps"] - + "Do you want to use the new build steps?", - choices=YES_NO_CHOICES, - default="yes", - ) - if ans.lower() in ["y", "yes"]: - old_module["steps"] = new_module["steps"] - local_changes_made = True - else: - print( - "Please make sure the old build steps work" - + " with the new version of the module" - ) - elif key == "input": - if commit_differs: - old_module["input"] = new_module["input"] - - input_path = os.path.join(".", old_module["name"], "input.json") - input_data = read_json(input_path) - if input_data == None: - log.debug( - "Skipping input update for module '%s': " % old_module["name"] - + "No input found in '%s'" % input_path - ) - else: - try: - local_changes_made |= update_input_data(old_module, input_data) - except InputDataUpdateFailed as e: - log.warning(e) - if prompt_user( - module_updates.config.non_interactive, - "Input for module '%s' has changed " % old_module["name"] - + "and may no longer be compatible. " - + "Do you want to re-enter input now?", - YES_NO_CHOICES, - "no", - ).lower() in ("no", "n"): - continue - input_data = copy.deepcopy(old_module["input"]) - module_updates.config.input_command( - old_module["name"], input_data - ) - local_changes_made = True - - if local_changes_made: - write_json(input_path, input_data) - module_updates.files.append(input_path) - else: - if key == "dependencies": - extra = set(new_module["dependencies"]) - set( - old_module["dependencies"] - ) - module_updates.new_deps.extend(extra) - module_updates.new_deps_added_by.update( - {item: old_module["name"] for item in extra} - ) - - old_module[key] = new_module[key] - local_changes_made = True - - for key in set(new_module.keys()) - set(old_module.keys()): - old_module[key] = new_module[key] - if key == "dependencies": - extra = new_module["dependencies"] - module_updates.new_deps.extend(extra) - module_updates.new_deps_added_by.update( - {item: old_module["name"] for item in extra} - ) - - if local_changes_made: - if new_module.get("version"): - module_updates.msg += ( - "\n - Updated module '%s' from version %s to version %s" - % ( - update.name, - old_version, - update.version if update.version else new_module["version"], - ) - ) - else: - module_updates.msg += "\n - Updated module '%s' from url" % (update.name) - else: - print("Module '%s' already up to date" % old_module["name"]) - - module_updates.changes_made |= local_changes_made - - @cfbs_command("update") @commit_after_command("Updated module%s", [PLURAL_S]) def update_command(to_update) -> Result: diff --git a/cfbs/updates.py b/cfbs/updates.py new file mode 100644 index 00000000..23904e75 --- /dev/null +++ b/cfbs/updates.py @@ -0,0 +1,225 @@ +import copy +import os +import logging as log + +from cfbs.prompts import YES_NO_CHOICES, prompt_user +from cfbs.utils import read_json, user_error, write_json + + +class ModuleUpdates: + + def __init__(self, config): + self.new_deps = [] + self.new_deps_added_by = dict() + self.changes_made = False + self.files = [] + self.config = config + self.msg = "" + + +class InputDataUpdateFailed(Exception): + def __init__(self, message): + super().__init__(message) + + +def update_input_data(module, input_data) -> bool: + """ + Update input data from module definition + + :param module: Module with updated input definition + :param input_data: Input data to update + :return: True if changes are made + """ + module_name = module["name"] + input_def = module["input"] + + if len(input_def) != len(input_data): + raise InputDataUpdateFailed( + "Failed to update input data for module '%s': " % module_name + + "Input definition has %d variables, " % len(input_def) + + "while current input data has %d variables." % len(input_data) + ) + + def _update_keys(input_def, input_data, keys): + """ + Update keys that can be safily updated in input data. + """ + changes_made = False + for key in keys: + new = input_def.get(key) + old = input_data.get(key) + if new != old: + # Make sure that one of the keys are not 'None' + if new is None or old is None: + raise InputDataUpdateFailed( + "Failed to update input data for module '%s': " % module_name + + "Missing matching attribute '%s'." % key + ) + input_data[key] = input_def[key] + changes_made = True + log.warning( + "Updated attribute '%s' from '%s' to '%s' in module '%s'." + % (key, old, new, module_name) + ) + return changes_made + + def _check_keys(input_def, input_data, keys): + """ + Compare keys that cannot safily be updated for equality. + """ + for key in keys: + new = input_def.get(key) + old = input_data.get(key) + if new != old: + raise InputDataUpdateFailed( + "Failed to update input data for module '%s': " % module_name + + "Updating attribute '%s' from '%s' to '%s'," % (key, old, new) + + "may cause module to break." + ) + + def _update_variable(input_def, input_data): + _check_keys(input_def, input_data, ("type", "namespace", "bundle", "variable")) + changes_made = _update_keys( + input_def, input_data, ("label", "comment", "question", "while", "default") + ) + + if input_def["type"] == "list": + def_subtype = input_def["subtype"] + data_subtype = input_data["subtype"] + if type(def_subtype) != type(data_subtype): + raise InputDataUpdateFailed( + "Failed to update input data for module '%s': " % module_name + + "Different subtypes in list ('%s' != '%s')." + % (type(def_subtype).__name__, type(data_subtype).__name__) + ) + if isinstance(def_subtype, list): + if len(def_subtype) != len(data_subtype): + raise InputDataUpdateFailed( + "Failed to update input data for module '%s': " % module_name + + "Different amount of elements in list ('%s' != '%s')." + % (len(def_subtype), len(data_subtype)) + ) + for i in range(len(def_subtype)): + _check_keys(def_subtype[i], data_subtype[i], ("key", "type")) + changes_made |= _update_keys( + def_subtype[i], + data_subtype[i], + ("label", "question", "default"), + ) + elif isinstance(def_subtype, dict): + _check_keys(def_subtype, data_subtype, ("type",)) + changes_made |= _update_keys( + def_subtype, data_subtype, ("label", "question", "default") + ) + else: + user_error( + "Unsupported subtype '%s' in input definition for module '%s'." + % (type(def_subtype).__name__, module_name) + ) + return changes_made + + changes_made = False + for i in range(len(input_def)): + changes_made |= _update_variable(input_def[i], input_data[i]) + return changes_made + + +def update_module(old_module, new_module, module_updates, update): + commit_differs = old_module["commit"] != new_module["commit"] + old_version = old_module.get("version") + local_changes_made = False + for key in old_module.keys(): + if key not in new_module or old_module[key] == new_module[key]: + continue + if key == "steps": + # same commit => user modifications, don't revert them + if commit_differs: + ans = prompt_user( + module_updates.config.non_interactive, + "Module %s has different build steps now\n" % old_module["name"] + + "old steps: %s\n" % old_module["steps"] + + "new steps: %s\n" % new_module["steps"] + + "Do you want to use the new build steps?", + choices=YES_NO_CHOICES, + default="yes", + ) + if ans.lower() in ["y", "yes"]: + old_module["steps"] = new_module["steps"] + local_changes_made = True + else: + print( + "Please make sure the old build steps work" + + " with the new version of the module" + ) + elif key == "input": + if commit_differs: + old_module["input"] = new_module["input"] + + input_path = os.path.join(".", old_module["name"], "input.json") + input_data = read_json(input_path) + if input_data == None: + log.debug( + "Skipping input update for module '%s': " % old_module["name"] + + "No input found in '%s'" % input_path + ) + else: + try: + local_changes_made |= update_input_data(old_module, input_data) + except InputDataUpdateFailed as e: + log.warning(e) + if prompt_user( + module_updates.config.non_interactive, + "Input for module '%s' has changed " % old_module["name"] + + "and may no longer be compatible. " + + "Do you want to re-enter input now?", + YES_NO_CHOICES, + "no", + ).lower() in ("no", "n"): + continue + input_data = copy.deepcopy(old_module["input"]) + module_updates.config.input_command( + old_module["name"], input_data + ) + local_changes_made = True + + if local_changes_made: + write_json(input_path, input_data) + module_updates.files.append(input_path) + else: + if key == "dependencies": + extra = set(new_module["dependencies"]) - set( + old_module["dependencies"] + ) + module_updates.new_deps.extend(extra) + module_updates.new_deps_added_by.update( + {item: old_module["name"] for item in extra} + ) + + old_module[key] = new_module[key] + local_changes_made = True + + for key in set(new_module.keys()) - set(old_module.keys()): + old_module[key] = new_module[key] + if key == "dependencies": + extra = new_module["dependencies"] + module_updates.new_deps.extend(extra) + module_updates.new_deps_added_by.update( + {item: old_module["name"] for item in extra} + ) + + if local_changes_made: + if new_module.get("version"): + module_updates.msg += ( + "\n - Updated module '%s' from version %s to version %s" + % ( + update.name, + old_version, + update.version if update.version else new_module["version"], + ) + ) + else: + module_updates.msg += "\n - Updated module '%s' from url" % (update.name) + else: + print("Module '%s' already up to date" % old_module["name"]) + + module_updates.changes_made |= local_changes_made diff --git a/tests/test_input.py b/tests/test_input.py index 7200e51a..94cb0723 100644 --- a/tests/test_input.py +++ b/tests/test_input.py @@ -1,5 +1,5 @@ import copy -from cfbs.commands import update_input_data, InputDataUpdateFailed +from cfbs.updates import update_input_data, InputDataUpdateFailed module = { "name": "superhero-module",