From f30a733a474d94a30af542148ad38e1743d879e0 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Thu, 3 Jul 2025 16:05:42 +0200 Subject: [PATCH 01/12] Moved replace_version to separate function Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/build.py | 64 +++++++++++++++++++++++++++------------------------ 1 file changed, 34 insertions(+), 30 deletions(-) diff --git a/cfbs/build.py b/cfbs/build.py index 3e6994a0..095bba49 100644 --- a/cfbs/build.py +++ b/cfbs/build.py @@ -127,6 +127,38 @@ def _perform_replace_step(n, a, b, filename): user_error("Failed to write to '%s'" % (filename,)) +def _perform_replace_version(to_replace, filename, version): + if not os.path.isfile(filename): + user_error( + "No such file '%s' in replace_version for module '%s" + % (file, module["name"]) + ) + try: + with open(filename, "r") as f: + content = f.read() + except: + user_error( + "Could not open/read '%s' in replace_version for module '%s" + % (filename, module["name"]) + ) + new_content = content.replace(to_replace, version, 1) + if new_content == content: + user_error( + "replace_version requires that '%s' has exactly 1 occurence of '%s' - 0 found" + % (filename, to_replace) + ) + if to_replace in new_content: + user_error( + "replace_version requires that '%s' has exactly 1 occurence of '%s' - more than 1 found" + % (filename, to_replace) + ) + try: + with open(filename, "w") as f: + f.write(new_content) + except: + user_error("Failed to write to '%s'" % (filename,)) + + def _perform_build_step(module, step, max_length): operation, args = split_build_step(step) source = module["_directory"] @@ -299,38 +331,10 @@ def _perform_build_step(module, step, max_length): elif operation == "replace_version": assert len(args) == 2 print("%s replace_version '%s'" % (prefix, "' '".join(args))) - file = os.path.join(destination, args[1]) - if not os.path.isfile(file): - user_error( - "No such file '%s' in replace_version for module '%s" - % (file, module["name"]) - ) - try: - with open(file, "r") as f: - content = f.read() - except: - user_error( - "Could not open/read '%s' in replace_version for module '%s" - % (file, module["name"]) - ) to_replace = args[0] + filename = os.path.join(destination, args[1]) version = module["version"] - new_content = content.replace(to_replace, version, 1) - if new_content == content: - user_error( - "replace_version requires that '%s' has exactly 1 occurence of '%s' - 0 found" - % (file, to_replace) - ) - if to_replace in new_content: - user_error( - "replace_version requires that '%s' has exactly 1 occurence of '%s' - more than 1 found" - % (file, to_replace) - ) - try: - with open(file, "w") as f: - f.write(new_content) - except: - user_error("Failed to write to '%s'" % (file,)) + _perform_replace_version(to_replace, filename, version) def perform_build(config) -> int: From 3f7671e2e8d41b0e764c7e3cc713e95fb34bbe06 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Thu, 3 Jul 2025 16:09:09 +0200 Subject: [PATCH 02/12] JSON.md: Fixed formatting mistake Signed-off-by: Ole Herman Schumacher Elgesem --- JSON.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/JSON.md b/JSON.md index 0c5ac4ca..c15e1a0a 100644 --- a/JSON.md +++ b/JSON.md @@ -265,14 +265,12 @@ In `cfbs.json`'s `"steps"`, the build step name must be separated from the rest - Source is relative to module directory and target is relative to `out/masterfiles`. - In most cases, the build step should be: `input ./input.json def.json` - `replace ` - - Replace string `` with string ``, exactly `` times, in file `filename`. - string `` must not contain string ``, as that could lead to confusing / recursive replacement situations. - The number of occurences is strict: It will error if the string cannot be found, cannot be replaced exactly `` times, or can still be found after replacements are done. (This is to try to catch mistakes). - `n` must be an integer, from 1 to 1000, and may optionally have a trailing `+` to signify "or more". At most 1000 replacements will be performed, regardless of whether you specify `+` or not. - - `replace_version ` - Replace the string inside the file with the version number of that module. - The module must have a version and the string must occur exactly once in the file. From c75b875155feaad7272d511a24c351903c9e7b4a Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Thu, 3 Jul 2025 16:15:21 +0200 Subject: [PATCH 03/12] Fixed file not found error handling in replace and replace_version Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/build.py | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/cfbs/build.py b/cfbs/build.py index 095bba49..b985bb12 100644 --- a/cfbs/build.py +++ b/cfbs/build.py @@ -95,11 +95,11 @@ def _perform_replace_step(n, a, b, filename): user_error( "'%s' must not contain '%s' (could lead to recursive replacing)" % (a, b) ) - if not os.path.isfile(filename): - user_error("No such file '%s' in replace build step" % (filename,)) try: with open(filename, "r") as f: content = f.read() + except FileNotFoundError: + user_error("No such file '%s' in replace build step" % (filename,)) except: user_error("Could not open/read '%s' in replace build step" % (filename,)) new_content = previous_content = content @@ -128,19 +128,13 @@ def _perform_replace_step(n, a, b, filename): def _perform_replace_version(to_replace, filename, version): - if not os.path.isfile(filename): - user_error( - "No such file '%s' in replace_version for module '%s" - % (file, module["name"]) - ) try: with open(filename, "r") as f: content = f.read() + except FileNotFoundError: + user_error("No such file '%s' in replace build step" % (filename,)) except: - user_error( - "Could not open/read '%s' in replace_version for module '%s" - % (filename, module["name"]) - ) + user_error("Could not open/read '%s' in replace_version" % (filename,)) new_content = content.replace(to_replace, version, 1) if new_content == content: user_error( From 23f8f00eb64208d6e7e56a92ea2f9ccfe82dddfa Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Thu, 3 Jul 2025 16:59:33 +0200 Subject: [PATCH 04/12] Moved validation for replace and replace_version steps into validate.py Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/build.py | 36 ++++++++++++++++----------------- cfbs/validate.py | 52 ++++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 67 insertions(+), 21 deletions(-) diff --git a/cfbs/build.py b/cfbs/build.py index b985bb12..7c2a58b6 100644 --- a/cfbs/build.py +++ b/cfbs/build.py @@ -35,6 +35,7 @@ MAX_REPLACEMENTS, step_has_valid_arg_count, split_build_step, + validate_build_step, ) @@ -79,22 +80,14 @@ def _generate_augment(module_name, input_data): def _perform_replace_step(n, a, b, filename): + assert n and a and b and filename + assert a not in b + or_more = False if n.endswith("+"): n = n[0:-1] or_more = True n = int(n) - if n <= 0: - user_error("replace build step cannot replace something %s times" % (n)) - if n > MAX_REPLACEMENTS or n == MAX_REPLACEMENTS and or_more: - user_error( - "replace build step cannot replace something more than %s times" - % (MAX_REPLACEMENTS) - ) - if a in b and (n >= 2 or or_more): - user_error( - "'%s' must not contain '%s' (could lead to recursive replacing)" % (a, b) - ) try: with open(filename, "r") as f: content = f.read() @@ -153,13 +146,14 @@ def _perform_replace_version(to_replace, filename, version): user_error("Failed to write to '%s'" % (filename,)) -def _perform_build_step(module, step, max_length): +def _perform_build_step(module, i, step, max_length): operation, args = split_build_step(step) + name = module["name"] source = module["_directory"] counter = module["_counter"] destination = "out/masterfiles" - prefix = "%03d %s :" % (counter, pad_right(module["name"], max_length)) + prefix = "%03d %s :" % (counter, pad_right(name, max_length)) assert operation in AVAILABLE_BUILD_STEPS # Should already be validated if operation == "copy": @@ -241,14 +235,14 @@ def _perform_build_step(module, step, max_length): if dst in [".", "./"]: dst = "" print("%s input '%s' 'masterfiles/%s'" % (prefix, src, dst)) - if src.startswith(module["name"] + "/"): + if src.startswith(name + "/"): log.warning( "Deprecated 'input' build step behavior - it should be: 'input ./input.json def.json'" ) # We'll translate it to what it should be # TODO: Consider removing this behavior for cfbs 4? - src = "." + src[len(module["name"]) :] - src = os.path.join(module["name"], src) + src = "." + src[len(name) :] + src = os.path.join(name, src) dst = os.path.join(destination, dst) if not os.path.isfile(os.path.join(src)): log.warning( @@ -257,7 +251,7 @@ def _perform_build_step(module, step, max_length): ) return extras, original = read_json(src), read_json(dst) - extras = _generate_augment(module["name"], extras) + extras = _generate_augment(name, extras) log.debug("Generated augment: %s", pretty(extras)) if not extras: user_error( @@ -319,11 +313,15 @@ def _perform_build_step(module, step, max_length): elif operation == "replace": assert len(args) == 4 print("%s replace '%s'" % (prefix, "' '".join(args))) + # New build step so let's be a bit strict about validating it: + validate_build_step(module, i, operation, args, strict=True) n, a, b, file = args file = os.path.join(destination, file) _perform_replace_step(n, a, b, file) elif operation == "replace_version": assert len(args) == 2 + # New build step so let's be a bit strict about validating it: + validate_build_step(module, i, operation, args, strict=True) print("%s replace_version '%s'" % (prefix, "' '".join(args))) to_replace = args[0] filename = os.path.join(destination, args[1]) @@ -367,8 +365,8 @@ def perform_build(config) -> int: print("\nSteps:") module_name_length = config.longest_module_key_length("name") for module in config.get("build", []): - for step in module["steps"]: - _perform_build_step(module, step, module_name_length) + for i, step in enumerate(module["steps"]): + _perform_build_step(module, i, step, module_name_length) assert os.path.isdir("./out/masterfiles/") shutil.copyfile("./cfbs.json", "./out/masterfiles/cfbs.json") if os.path.isfile("out/masterfiles/def.json"): diff --git a/cfbs/validate.py b/cfbs/validate.py index d6bf1864..9a23e774 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -190,7 +190,8 @@ def validate_config(config, empty_build_list_ok=False): return 1 -def validate_build_step(name, i, operation, args): +def validate_build_step(module, i, operation, args, strict=False): + name = module["name"] if not operation in AVAILABLE_BUILD_STEPS: raise CFBSValidationError( name, @@ -213,6 +214,53 @@ def validate_build_step(name, i, operation, args): "The %s build step expects %d or more arguments, %d were given" % (operation, expected, actual), ) + if not strict: + return + if operation == "replace": + assert len(args) == 4 + n, a, b, filename = args + assert type(a) is str and a != "" + assert type(b) is str and b != "" + assert type(filename) is str and filename != "" + or_more = False + if n.endswith("+"): + n = n[0:-1] + or_more = True + try: + n = int(n) + assert n >= 1 + except: + raise CFBSValidationError( + "replace build step cannot replace something '%s' times" % (args[0],) + ) + if n > MAX_REPLACEMENTS or n == MAX_REPLACEMENTS and or_more: + raise CFBSValidationError( + "replace build step cannot replace something more than %s times" + % (MAX_REPLACEMENTS) + ) + if a in b: + raise CFBSValidationError( + "'%s' must not contain '%s' in replace build step (could lead to recursive replacing)" + % (a, b) + ) + elif operation == "replace_version": + assert len(args) == 2 + to_replace, filename = args + + # These should be guaranteed by the build step splitting logic: + assert type(to_replace) is str and to_replace != "" + assert type(filename) is str and filename != "" + + # replace_version requires the module to have a version field: + if not "version" in module: + raise CFBSValidationError( + name, + "Module '%s' missing \"version\" field for replace_version build step" + % (name,), + ) + else: + # TODO: Add more validation of other build steps. + pass def _validate_module_object(context, name, module, config): @@ -354,7 +402,7 @@ def validate_steps(name, module): name, '"steps" must be a list of non-empty / non-whitespace strings' ) operation, args = split_build_step(step) - validate_build_step(name, i, operation, args) + validate_build_step(module, i, operation, args) def validate_url_field(name, module, field): assert field in module From 17adbe5efac4e1191e6a93238cd5eb22285ad624 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Thu, 3 Jul 2025 17:53:34 +0200 Subject: [PATCH 05/12] Unified replace_version and replace to use shared code Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/build.py | 35 +++---------------- cfbs/validate.py | 12 +++++-- .../043_replace_version/example-cfbs.json | 2 +- 3 files changed, 15 insertions(+), 34 deletions(-) diff --git a/cfbs/build.py b/cfbs/build.py index 7c2a58b6..12b7136a 100644 --- a/cfbs/build.py +++ b/cfbs/build.py @@ -120,32 +120,6 @@ def _perform_replace_step(n, a, b, filename): user_error("Failed to write to '%s'" % (filename,)) -def _perform_replace_version(to_replace, filename, version): - try: - with open(filename, "r") as f: - content = f.read() - except FileNotFoundError: - user_error("No such file '%s' in replace build step" % (filename,)) - except: - user_error("Could not open/read '%s' in replace_version" % (filename,)) - new_content = content.replace(to_replace, version, 1) - if new_content == content: - user_error( - "replace_version requires that '%s' has exactly 1 occurence of '%s' - 0 found" - % (filename, to_replace) - ) - if to_replace in new_content: - user_error( - "replace_version requires that '%s' has exactly 1 occurence of '%s' - more than 1 found" - % (filename, to_replace) - ) - try: - with open(filename, "w") as f: - f.write(new_content) - except: - user_error("Failed to write to '%s'" % (filename,)) - - def _perform_build_step(module, i, step, max_length): operation, args = split_build_step(step) name = module["name"] @@ -319,14 +293,15 @@ def _perform_build_step(module, i, step, max_length): file = os.path.join(destination, file) _perform_replace_step(n, a, b, file) elif operation == "replace_version": - assert len(args) == 2 + assert len(args) == 3 # New build step so let's be a bit strict about validating it: validate_build_step(module, i, operation, args, strict=True) print("%s replace_version '%s'" % (prefix, "' '".join(args))) - to_replace = args[0] - filename = os.path.join(destination, args[1]) + n = args[0] + to_replace = args[1] + filename = os.path.join(destination, args[2]) version = module["version"] - _perform_replace_version(to_replace, filename, version) + _perform_replace_step(n, to_replace, version, filename) def perform_build(config) -> int: diff --git a/cfbs/validate.py b/cfbs/validate.py index 9a23e774..98c10df2 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -43,7 +43,7 @@ "policy_files": "1+", "bundles": "1+", "replace": 4, # n, a, b, filename - "replace_version": 2, # string to replace and filename + "replace_version": 3, # n, string to replace, filename } MAX_REPLACEMENTS = 1000 @@ -244,10 +244,11 @@ def validate_build_step(module, i, operation, args, strict=False): % (a, b) ) elif operation == "replace_version": - assert len(args) == 2 - to_replace, filename = args + assert len(args) == 3 + n, to_replace, filename = args # These should be guaranteed by the build step splitting logic: + assert type(n) is str and n != "" assert type(to_replace) is str and to_replace != "" assert type(filename) is str and filename != "" @@ -258,6 +259,11 @@ def validate_build_step(module, i, operation, args, strict=False): "Module '%s' missing \"version\" field for replace_version build step" % (name,), ) + version = module["version"] + # Reuse validation logic for replace: + validate_build_step( + name, module, i, "replace", [n, to_replace, version, filename], strict + ) else: # TODO: Add more validation of other build steps. pass diff --git a/tests/shell/043_replace_version/example-cfbs.json b/tests/shell/043_replace_version/example-cfbs.json index 54f1e458..f01db962 100644 --- a/tests/shell/043_replace_version/example-cfbs.json +++ b/tests/shell/043_replace_version/example-cfbs.json @@ -11,7 +11,7 @@ "version": "1.2.3", "steps": [ "copy example.py services/cfbs/subdir/example.py", - "replace_version 0.0.0 services/cfbs/subdir/example.py" + "replace_version 1 0.0.0 services/cfbs/subdir/example.py" ] } ] From 69a30996fa17ed8a304a70621e6326f17961cf47 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Thu, 3 Jul 2025 17:23:08 +0200 Subject: [PATCH 06/12] Started skeleton of better handling exceptions in main() Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/main.py | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/cfbs/main.py b/cfbs/main.py index fcd99c06..c82b9760 100644 --- a/cfbs/main.py +++ b/cfbs/main.py @@ -6,6 +6,7 @@ import logging as log import sys +from cfbs.validate import CFBSValidationError from cfbs.version import string as version from cfbs.utils import user_error, is_cfbs_repo, ProgrammerError from cfbs.cfbs_config import CFBSConfig @@ -37,7 +38,16 @@ def does_log_info(level): return level == "info" or level == "debug" -def main() -> int: +def _main() -> int: + """Actual body of main function. + + Mainly for getting command line arguments and calling the appropriate + functions based on command line arguments. + + Actual logic should be implemented elsewhere (primarily in commands.py). + + This function is wrapped by main() which catches exceptions. + """ args = get_args() init_logging(args.loglevel) if args.manual: @@ -223,3 +233,16 @@ def main() -> int: raise ProgrammerError( "Command '%s' not handled appropriately by the code above" % args.command ) + + +def main() -> int: + """Entry point + + The only thing we want to do here is call _main() and handle exceptions (errors). + """ + try: + return _main() + except CFBSValidationError as e: + print("Error: " + str(e)) + # TODO: Handle other exceptions + return 1 From 8cec9758d21a8b3a6ce3352983dbddc1768588e9 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Thu, 3 Jul 2025 17:29:13 +0200 Subject: [PATCH 07/12] Converted user_error to an exception - UserError There are a few reasons for doing this: - Makes it possible to catch this exception. - Makes it possible for tools to know what return types a function return. Before this, things like PyRight aren't able to deduce that the function "stops" at user_error(). - Same argument could be made for making it easier for humans to read. - Make it possible to handle this error in one centralized place along with the other error types, in main(). Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/analyze.py | 12 +++--- cfbs/args.py | 8 ++-- cfbs/build.py | 28 +++++++------- cfbs/cfbs_config.py | 22 +++++------ cfbs/cfbs_json.py | 8 ++-- cfbs/commands.py | 38 ++++++++++--------- cfbs/index.py | 10 ++--- cfbs/internal_file_management.py | 32 ++++++++-------- cfbs/main.py | 32 +++++++++------- cfbs/man_generator.py | 4 +- .../masterfiles/check_download_matches_git.py | 4 +- cfbs/masterfiles/download_all_versions.py | 8 ++-- cfbs/updates.py | 4 +- cfbs/utils.py | 10 ++--- cfbs/validate.py | 8 ++-- 15 files changed, 117 insertions(+), 111 deletions(-) diff --git a/cfbs/analyze.py b/cfbs/analyze.py index 7c6a1769..488eba6e 100644 --- a/cfbs/analyze.py +++ b/cfbs/analyze.py @@ -17,7 +17,7 @@ immediate_subdirectories, mkdir, read_json, - user_error, + UserError, ) @@ -112,11 +112,11 @@ def mpf_vcf_dicts(offline=False): cfbs_ri_dir = os.path.join(cfbs_dir(), RI_SUBDIRS) if not os.path.exists(cfbs_ri_dir): - user_error(ERROR_MESSAGE) + raise UserError(ERROR_MESSAGE) ri_versions = immediate_subdirectories(cfbs_ri_dir) if len(ri_versions) == 0: - user_error(ERROR_MESSAGE) + raise UserError(ERROR_MESSAGE) ri_latest_version = max(ri_versions) mpf_vcf_path = os.path.join( @@ -134,7 +134,7 @@ def mpf_vcf_dicts(offline=False): try: latest_release_data = get_json(LATEST_RELEASE_API_URL) except FetchError as e: - user_error( + raise UserError( "Downloading CFEngine release information failed - check your Wi-Fi / network settings." ) @@ -155,7 +155,7 @@ def mpf_vcf_dicts(offline=False): try: fetch_url(ri_checksums_url, archive_checksums_path) except FetchError as e: - user_error(str(e)) + raise UserError(str(e)) with open(archive_checksums_path) as file: lines = [line.rstrip() for line in file] @@ -614,7 +614,7 @@ def analyze_policyset( + "\n ".join(possible_policyset_relpaths) + "\n" ) - user_error( + raise UserError( "There doesn't seem to be a valid policy set in the supplied path.\n Usage: cfbs analyze path/to/policy-set\n" + extra_error_text ) diff --git a/cfbs/args.py b/cfbs/args.py index 76d0f742..0e9f6796 100644 --- a/cfbs/args.py +++ b/cfbs/args.py @@ -2,7 +2,7 @@ import os from cfbs import commands -from cfbs.utils import cache, user_error +from cfbs.utils import cache, UserError def get_args(): @@ -23,13 +23,13 @@ def get_manual(): with open(file_path, "r", encoding="utf-8") as man_file: man = man_file.read() if not man: - user_error("Manual file is empty") + raise UserError("Manual file is empty") else: return man except OSError: - user_error("Error reading manual file " + file_path) + raise UserError("Error reading manual file " + file_path) else: - user_error("Manual file does not exist") + raise UserError("Manual file does not exist") @cache diff --git a/cfbs/build.py b/cfbs/build.py index 12b7136a..4aea4c0f 100644 --- a/cfbs/build.py +++ b/cfbs/build.py @@ -26,7 +26,7 @@ sh, strip_left, touch, - user_error, + UserError, write_json, ) from cfbs.pretty import pretty, pretty_file @@ -92,15 +92,15 @@ def _perform_replace_step(n, a, b, filename): with open(filename, "r") as f: content = f.read() except FileNotFoundError: - user_error("No such file '%s' in replace build step" % (filename,)) + raise UserError("No such file '%s' in replace build step" % (filename,)) except: - user_error("Could not open/read '%s' in replace build step" % (filename,)) + raise UserError("Could not open/read '%s' in replace build step" % (filename,)) new_content = previous_content = content for i in range(0, n): previous_content = new_content new_content = previous_content.replace(a, b, 1) if new_content == previous_content: - user_error( + raise UserError( "replace build step could only replace '%s' in '%s' %s times, not %s times (required)" % (a, filename, i, n) ) @@ -112,12 +112,12 @@ def _perform_replace_step(n, a, b, filename): if new_content == previous_content: break if a in new_content: - user_error("too many occurences of '%s' in '%s'" % (a, filename)) + raise UserError("too many occurences of '%s' in '%s'" % (a, filename)) try: with open(filename, "w") as f: f.write(new_content) except: - user_error("Failed to write to '%s'" % (filename,)) + raise UserError("Failed to write to '%s'" % (filename,)) def _perform_build_step(module, i, step, max_length): @@ -158,7 +158,7 @@ def _perform_build_step(module, i, step, max_length): dst = "" print("%s json '%s' 'masterfiles/%s'" % (prefix, src, dst)) if not os.path.isfile(os.path.join(source, src)): - user_error("'%s' is not a file" % src) + raise UserError("'%s' is not a file" % src) src, dst = os.path.join(source, src), os.path.join(destination, dst) extras, original = read_json(src), read_json(dst) if not extras: @@ -228,7 +228,7 @@ def _perform_build_step(module, i, step, max_length): extras = _generate_augment(name, extras) log.debug("Generated augment: %s", pretty(extras)) if not extras: - user_error( + raise UserError( "Input data '%s' is incomplete: Skipping build step." % os.path.basename(src) ) @@ -251,7 +251,7 @@ def _perform_build_step(module, i, step, max_length): cf_files = find("out/masterfiles/" + file, extension=".cf") files += (strip_left(f, "out/masterfiles/") for f in cf_files) else: - user_error( + raise UserError( "Unsupported filetype '%s' for build step '%s': " % (file, operation) + "Expected directory (*/) of policy file (*.cf)" @@ -306,7 +306,7 @@ def _perform_build_step(module, i, step, max_length): def perform_build(config) -> int: if not config.get("build"): - user_error("No 'build' key found in the configuration") + raise UserError("No 'build' key found in the configuration") # mini-validation for module in config.get("build", []): @@ -314,25 +314,25 @@ def perform_build(config) -> int: operation, args = split_build_step(step) if step.split() != [operation] + args: - user_error( + raise UserError( "Incorrect whitespace in the `%s` build step - singular spaces are required" % step ) if operation not in AVAILABLE_BUILD_STEPS: - user_error("Unknown build step operation: %s" % operation) + raise UserError("Unknown build step operation: %s" % operation) expected = AVAILABLE_BUILD_STEPS[operation] actual = len(args) if not step_has_valid_arg_count(args, expected): if type(expected) is int: - user_error( + raise UserError( "The `%s` build step expects %d arguments, %d were given" % (step, expected, actual) ) else: expected = int(expected[0:-1]) - user_error( + raise UserError( "The `%s` build step expects %d or more arguments, %d were given" % (step, expected, actual) ) diff --git a/cfbs/cfbs_config.py b/cfbs/cfbs_config.py index 941a2240..c00694ba 100644 --- a/cfbs/cfbs_config.py +++ b/cfbs/cfbs_config.py @@ -7,7 +7,7 @@ from cfbs.result import Result from cfbs.utils import ( - user_error, + UserError, read_file, write_json, load_bundlenames, @@ -100,7 +100,7 @@ def add_with_dependencies( module_str = module module = (remote_config or self).get_module_for_build(module, str_added_by) if not module: - user_error("Module '%s' not found" % module_str) + raise UserError("Module '%s' not found" % module_str) assert "name" in module name = module["name"] assert "steps" in module @@ -148,7 +148,7 @@ def _add_using_url( if len(to_add) == 0: modules = list(provides.values()) if not any(modules): - user_error("no modules available, nothing to do") + raise UserError("no modules available, nothing to do") print("Found %d modules in '%s':" % (len(modules), url)) for m in modules: deps = m.get("dependencies", []) @@ -166,7 +166,7 @@ def _add_using_url( else: missing = [k for k in to_add if k not in provides] if missing: - user_error("Missing modules: " + ", ".join(missing)) + raise UserError("Missing modules: " + ", ".join(missing)) modules = [provides[k] for k in to_add] for i, module in enumerate(modules, start=1): @@ -382,7 +382,7 @@ def add_command( checksum=None, ) -> int: if not to_add: - user_error("Must specify at least one module to add") + raise UserError("Must specify at least one module to add") before = {m["name"] for m in self.get("build", [])} @@ -396,7 +396,7 @@ def add_command( else: # for this `if` to be valid, module names containing `://` should be illegal if "://" in to_add[0]: - user_error( + raise UserError( "URI scheme not supported. The supported URI schemes are: " + ", ".join(SUPPORTED_URI_SCHEMES) ) @@ -450,7 +450,7 @@ def input_command(self, module_name, input_data): def _check_keys(keys, input_data): for key in keys: if key not in input_data: - user_error( + raise UserError( "Expected attribute '%s' in input definition: %s" % (key, pretty(input_data)) ) @@ -469,7 +469,7 @@ def _input_elements(subtype): for element in subtype: _check_keys(["type", "label", "question", "key"], element) if element["type"] != "string": - user_error( + raise UserError( "Subtype of type '%s' not supported for type list" % element["type"] ) @@ -496,7 +496,7 @@ def _input_list(input_data): elif isinstance(subtype, dict): _check_keys(["type", "label", "question"], subtype) if subtype["type"] != "string": - user_error( + raise UserError( "Subtype of type '%s' not supported for type list" % subtype["type"] ) @@ -509,7 +509,7 @@ def _input_list(input_data): ).lower() in ("yes", "y"): result.append(_input_string(subtype)) return result - user_error( + raise UserError( "Expected the value of attribute 'subtype' to be a JSON list or object, not: %s" % pretty(input_data["subtype"]) ) @@ -523,7 +523,7 @@ def _input_list(input_data): elif definition["type"] == "list": definition["response"] = _input_list(definition) else: - user_error("Unsupported input type '%s'" % definition["type"]) + raise UserError("Unsupported input type '%s'" % definition["type"]) def _get_all_module_names(self, search_in=("build", "provides", "index")): modules = [] diff --git a/cfbs/cfbs_json.py b/cfbs/cfbs_json.py index 53cff6fd..84983f68 100644 --- a/cfbs/cfbs_json.py +++ b/cfbs/cfbs_json.py @@ -4,7 +4,7 @@ from cfbs.index import Index from cfbs.pretty import pretty, TOP_LEVEL_KEYS, MODULE_KEYS -from cfbs.utils import read_json, user_error +from cfbs.utils import read_json, UserError def _construct_provided_module(name, data, url, commit, added_by="cfbs add"): @@ -16,7 +16,7 @@ def _construct_provided_module(name, data, url, commit, added_by="cfbs add"): module = OrderedDict() module["name"] = name if "description" not in data: - user_error( + raise UserError( "missing required key 'description' in module definition: %s" % pretty(data) ) module["description"] = data["description"] @@ -31,7 +31,7 @@ def _construct_provided_module(name, data, url, commit, added_by="cfbs add"): if "input" in data: module["input"] = data["input"] if "steps" not in data: - user_error( + raise UserError( "missing required key 'steps' in module definition: %s" % pretty(data) ) module["steps"] = data["steps"] @@ -126,7 +126,7 @@ def __contains__(self, key): def get_provides(self, added_by="cfbs add"): modules = OrderedDict() if "provides" not in self._data: - user_error( + raise UserError( "missing required key 'provides' in module definition: %s" % pretty(self._data) ) diff --git a/cfbs/commands.py b/cfbs/commands.py index f405934b..95a82cc9 100644 --- a/cfbs/commands.py +++ b/cfbs/commands.py @@ -21,7 +21,7 @@ cfbs_filename, is_cfbs_repo, read_json, - user_error, + UserError, strip_right, pad_right, ProgrammerError, @@ -96,13 +96,13 @@ def get_command_names(): @cfbs_command("pretty") def pretty_command(filenames: list, check: bool, keep_order: bool) -> int: if not filenames: - user_error("Filenames missing for cfbs pretty command") + raise UserError("Filenames missing for cfbs pretty command") sorting_rules = CFBS_DEFAULT_SORTING_RULES if keep_order else None num_files = 0 for f in filenames: if not f or not f.endswith(".json"): - user_error( + raise UserError( "cfbs pretty command can only be used with .json files, not '%s'" % os.path.basename(f) ) @@ -114,9 +114,9 @@ def pretty_command(filenames: list, check: bool, keep_order: bool) -> int: else: pretty_file(f, sorting_rules) except FileNotFoundError: - user_error("File '%s' not found" % f) + raise UserError("File '%s' not found" % f) except json.decoder.JSONDecodeError as ex: - user_error("Error reading json file '{}': {}".format(f, ex)) + raise UserError("Error reading json file '{}': {}".format(f, ex)) if check: print("Would reformat %d file(s)" % num_files) return 1 if num_files > 0 else 0 @@ -126,7 +126,7 @@ def pretty_command(filenames: list, check: bool, keep_order: bool) -> int: @cfbs_command("init") def init_command(index=None, masterfiles=None, non_interactive=False) -> int: if is_cfbs_repo(): - user_error("Already initialized - look at %s" % cfbs_filename()) + raise UserError("Already initialized - look at %s" % cfbs_filename()) name = prompt_user( non_interactive, @@ -274,7 +274,7 @@ def init_command(index=None, masterfiles=None, non_interactive=False) -> int: remote = "https://github.com/cfengine/masterfiles" commit = ls_remote(remote, branch) if commit is None: - user_error( + raise UserError( "Failed to find branch or tag %s at remote %s" % (branch, remote) ) log.debug("Current commit for masterfiles branch %s is %s" % (branch, commit)) @@ -398,7 +398,7 @@ def remove_command(to_remove: list): config = CFBSConfig.get_instance() config.warn_about_unknown_keys() if not "build" in config: - user_error( + raise UserError( 'Cannot remove any modules because the "build" key is missing from cfbs.json' ) modules = config["build"] @@ -456,7 +456,7 @@ def _get_modules_by_url(name) -> list: if name.startswith(SUPPORTED_URI_SCHEMES): matches = _get_modules_by_url(name) if not matches: - user_error("Could not find module with URL '%s'" % name) + raise UserError("Could not find module with URL '%s'" % name) for module in matches: answer = _remove_module_user_prompt(module) if answer.lower() in ("yes", "y"): @@ -710,10 +710,10 @@ def _download_dependencies( counter += 1 continue if "commit" not in module: - user_error("module %s must have a commit property" % name) + raise UserError("module %s must have a commit property" % name) commit = module["commit"] if not is_a_commit_hash(commit): - user_error("'%s' is not a commit reference" % commit) + raise UserError("'%s' is not a commit reference" % commit) url = module.get("url") or module["repo"] url = strip_right(url, ".git") @@ -727,7 +727,7 @@ def _download_dependencies( if not os.path.exists(module_dir): if url.endswith(SUPPORTED_ARCHIVES): if os.path.exists(commit_dir) and "subdirectory" in module: - user_error( + raise UserError( "Subdirectory '%s' for module '%s' was not found in fetched archive '%s': " % (module["subdirectory"], name, url) + "Please check cfbs.json for possible typos." @@ -738,7 +738,7 @@ def _download_dependencies( # - added by URL instead of name (no version property in module data) elif "index" in module or "url" in module or ignore_versions: if os.path.exists(commit_dir) and "subdirectory" in module: - user_error( + raise UserError( "Subdirectory '%s' for module '%s' was not found in cloned repository '%s': " % (module["subdirectory"], name, url) + "Please check cfbs.json for possible typos." @@ -749,13 +749,13 @@ def _download_dependencies( try: versions = get_json(_VERSION_INDEX) except FetchError as e: - user_error( + raise UserError( "Downloading CFEngine Build Module Index failed - check your Wi-Fi / network settings." ) try: checksum = versions[name][module["version"]]["archive_sha256"] except KeyError: - user_error("Cannot verify checksum of the '%s' module" % name) + raise UserError("Cannot verify checksum of the '%s' module" % name) module_archive_url = os.path.join( _MODULES_URL, name, commit + ".tar.gz" ) @@ -810,7 +810,7 @@ def build_command(ignore_versions=False) -> int: @cfbs_command("install") def install_command(args) -> int: if len(args) > 1: - user_error( + raise UserError( "Only one destination is allowed for command: cfbs install [destination]" ) if not os.path.exists("out/masterfiles"): @@ -867,7 +867,9 @@ def _print_module_info(data): @cfbs_command("info") def info_command(modules): if not modules: - user_error("info/show command requires one or more module names as arguments") + raise UserError( + "info/show command requires one or more module names as arguments" + ) config = CFBSConfig.get_instance() config.warn_about_unknown_keys() index = config.index @@ -934,7 +936,7 @@ def analyze_command( ) if not os.path.isdir(path): - user_error("the provided policy set path is not a directory") + raise UserError("the provided policy set path is not a directory") if masterfiles_dir is None: masterfiles_dir = "masterfiles" diff --git a/cfbs/index.py b/cfbs/index.py index 80c87d81..0cd4bc4b 100644 --- a/cfbs/index.py +++ b/cfbs/index.py @@ -2,7 +2,7 @@ from collections import OrderedDict from cfbs.module import Module -from cfbs.utils import FetchError, get_or_read_json, user_error, get_json +from cfbs.utils import FetchError, get_or_read_json, UserError, get_json from cfbs.internal_file_management import local_module_name _DEFAULT_INDEX = ( @@ -90,7 +90,7 @@ def _expand_index(self): try: self._data = get_or_read_json(index) except FetchError as e: - user_error( + raise UserError( "Downloading index '%s' failed - check your Wi-Fi / network settings." % index ) @@ -130,7 +130,7 @@ def exists(self, module): try: versions = get_json(_VERSION_INDEX) except FetchError as e: - user_error( + raise UserError( "Downloading CFEngine Build Module Index failed - check your Wi-Fi / network settings." ) @@ -140,7 +140,7 @@ def check_existence(self, modules: list): for module in modules: assert isinstance(module, Module) if not self.exists(module): - user_error( + raise UserError( "Module '%s'%s does not exist" % ( module.name, @@ -177,7 +177,7 @@ def get_module_object(self, module, added_by=None): try: versions = get_json(_VERSION_INDEX) except FetchError as e: - user_error( + raise UserError( "Downloading CFEngine Build Module Index failed - check your Wi-Fi / network settings." ) new_values = versions[name][version] diff --git a/cfbs/internal_file_management.py b/cfbs/internal_file_management.py index 4cbf2ef4..657e7ea1 100644 --- a/cfbs/internal_file_management.py +++ b/cfbs/internal_file_management.py @@ -23,7 +23,7 @@ rm, sh, strip_right, - user_error, + UserError, ) _SUPPORTED_TAR_TYPES = (".tar.gz", ".tgz") @@ -38,11 +38,11 @@ def local_module_name(module_path): if module.endswith((".cf", ".json", "/")) and not module.startswith("./"): module = "./" + module if not module.startswith("./"): - user_error("Please prepend local files or folders with './' to avoid ambiguity") + raise UserError("Please prepend local files or folders with './' to avoid ambiguity") for illegal in ["//", "..", " ", "\n", "\t", " "]: if illegal in module: - user_error("Module path cannot contain %s" % repr(illegal)) + raise UserError("Module path cannot contain %s" % repr(illegal)) if os.path.isdir(module) and not module.endswith("/"): module = module + "/" @@ -52,10 +52,10 @@ def local_module_name(module_path): assert os.path.exists(module) if os.path.isfile(module): if not module.endswith((".cf", ".json")): - user_error("Only .cf and .json files supported currently") + raise UserError("Only .cf and .json files supported currently") else: if not os.path.isdir(module): - user_error("'%s' must be either a directory or a file" % module) + raise UserError("'%s' must be either a directory or a file" % module) return module @@ -65,7 +65,7 @@ def get_download_path(module) -> str: commit = module["commit"] if not is_a_commit_hash(commit): - user_error("'%s' is not a commit reference" % commit) + raise UserError("'%s' is not a commit reference" % commit) url = module.get("url") or module["repo"] if url.endswith(SUPPORTED_ARCHIVES): @@ -92,9 +92,9 @@ def _prettify_name(name): def local_module_copy(module, counter, max_length): name = module["name"] if not name.startswith("./"): - user_error("module %s must start with ./" % name) + raise UserError("module %s must start with ./" % name) if not os.path.isfile(name) and not os.path.isdir(name): - user_error("module %s does not exist" % name) + raise UserError("module %s does not exist" % name) pretty_name = _prettify_name(name) target = "out/steps/%03d_%s_local/" % (counter, pretty_name) module["_directory"] = target @@ -116,7 +116,7 @@ def local_module_copy(module, counter, max_length): def _get_path_from_url(url): if not url.startswith(SUPPORTED_URI_SCHEMES): if "://" in url: - return user_error("Unsupported URL protocol in '%s'" % url) + return raise UserError("Unsupported URL protocol in '%s'" % url) else: # It's a path already, just remove trailing slashes (if any). return url.rstrip("/") @@ -161,7 +161,7 @@ def clone_url_repo(repo_url): # commit specified in the url repo_url, commit = repo_url.rsplit("@", 1) if not is_a_commit_hash(commit): - user_error("'%s' is not a commit reference" % commit) + raise UserError("'%s' is not a commit reference" % commit) downloads = os.path.join(cfbs_dir(), "downloads") @@ -188,7 +188,7 @@ def clone_url_repo(repo_url): if os.path.exists(json_path): return (json_path, commit) else: - user_error( + raise UserError( "Repository '%s' doesn't contain a valid cfbs.json index file" % repo_url ) @@ -207,7 +207,7 @@ def fetch_archive( archive_type = ext break else: - user_error("Unsupported archive type: '%s'" % url) + raise UserError("Unsupported archive type: '%s'" % url) archive_name = strip_right(archive_filename, archive_type) downloads = os.path.join(cfbs_dir(), "downloads") @@ -220,7 +220,7 @@ def fetch_archive( try: archive_checksum = fetch_url(url, archive_path, checksum) except FetchError as e: - user_error(str(e)) + raise UserError(str(e)) content_dir = os.path.join(downloads, archive_dir, archive_checksum) if extract_to_directory: @@ -238,12 +238,12 @@ def fetch_archive( if shutil.which("tar"): sh("cd %s; tar -xf %s" % (content_dir, archive_path)) else: - user_error("Working with .tar archives requires the 'tar' utility") + raise UserError("Working with .tar archives requires the 'tar' utility") elif archive_type == (".zip"): if shutil.which("unzip"): sh("cd %s; unzip %s" % (content_dir, archive_path)) else: - user_error("Working with .zip archives requires the 'unzip' utility") + raise UserError("Working with .zip archives requires the 'unzip' utility") else: raise RuntimeError( "Unhandled archive type: '%s'. Please report this at %s." @@ -270,7 +270,7 @@ def fetch_archive( if os.path.exists(index_path): return (index_path, archive_checksum) else: - user_error( + raise UserError( "Archive '%s' doesn't contain a valid cfbs.json index file" % url ) else: diff --git a/cfbs/main.py b/cfbs/main.py index c82b9760..3d96e884 100644 --- a/cfbs/main.py +++ b/cfbs/main.py @@ -8,7 +8,7 @@ from cfbs.validate import CFBSValidationError from cfbs.version import string as version -from cfbs.utils import user_error, is_cfbs_repo, ProgrammerError +from cfbs.utils import UserError, is_cfbs_repo, ProgrammerError from cfbs.cfbs_config import CFBSConfig from cfbs import commands from cfbs.args import get_args, print_help, get_manual @@ -60,62 +60,62 @@ def _main() -> int: if not args.command: print_help() print("") - user_error("No command given") + raise UserError("No command given") if args.command not in commands.get_command_names(): print_help() - user_error("Command '%s' not found" % args.command) + raise UserError("Command '%s' not found" % args.command) if args.masterfiles and args.command != "init": - user_error( + raise UserError( "The option --masterfiles is only for 'cfbs init', not 'cfbs %s'" % args.command ) if args.omit_download and args.command != "generate-release-information": - user_error( + raise UserError( "The option --omit-download is only for 'cfbs generate-release-information', not 'cfbs %s'" % args.command ) if args.check_against_git and args.command != "generate-release-information": - user_error( + raise UserError( "The option --check-against-git is only for 'cfbs generate-release-information', not 'cfbs %s'" % args.command ) if args.minimum_version and args.command != "generate-release-information": - user_error( + raise UserError( "The option --from is only for 'cfbs generate-release-information', not 'cfbs %s'" % args.command ) if args.masterfiles_dir and args.command not in ("analyze", "analyse"): - user_error( + raise UserError( "The option --masterfiles-dir is only for 'cfbs analyze', not 'cfbs %s'" % args.command ) if args.reference_version and args.command not in ("analyze", "analyse"): - user_error( + raise UserError( "The option --reference-version is only for 'cfbs analyze', not 'cfbs %s'" % args.command ) if args.to_json and args.command not in ("analyze", "analyse"): - user_error( + raise UserError( "The option --to-json is only for 'cfbs analyze', not 'cfbs %s'" % args.command ) if args.ignored_path_components and args.command not in ("analyze", "analyse"): - user_error( + raise UserError( "The option --ignored-path-components is only for 'cfbs analyze', not 'cfbs %s'" % args.command ) if args.offline and args.command not in ("analyze", "analyse"): - user_error( + raise UserError( "The option --offline is only for 'cfbs analyze', not 'cfbs %s'" % args.command ) @@ -128,7 +128,9 @@ def _main() -> int: "update", "input", ): - user_error("The option --non-interactive is not for cfbs %s" % (args.command)) + raise UserError( + "The option --non-interactive is not for cfbs %s" % (args.command) + ) # Commands you can run outside a cfbs repo: if args.command == "help": @@ -172,7 +174,7 @@ def _main() -> int: ) if not is_cfbs_repo(): - user_error("This is not a cfbs repo, to get started, type: cfbs init") + raise UserError("This is not a cfbs repo, to get started, type: cfbs init") if args.command == "status": return commands.status_command() @@ -244,5 +246,7 @@ def main() -> int: return _main() except CFBSValidationError as e: print("Error: " + str(e)) + except UserError as e: + print("Error: " + str(e)) # TODO: Handle other exceptions return 1 diff --git a/cfbs/man_generator.py b/cfbs/man_generator.py index 8ef39ad7..84aeef10 100644 --- a/cfbs/man_generator.py +++ b/cfbs/man_generator.py @@ -1,11 +1,11 @@ import os -from utils import user_error +from utils import UserError from args import get_arg_parser try: from build_manpages.manpage import Manpage except ImportError: - user_error( + raise UserError( "Missing dependency, install from PyPI: 'pip install argparse-manpage setuptools'" ) diff --git a/cfbs/masterfiles/check_download_matches_git.py b/cfbs/masterfiles/check_download_matches_git.py index af103709..f91989b4 100644 --- a/cfbs/masterfiles/check_download_matches_git.py +++ b/cfbs/masterfiles/check_download_matches_git.py @@ -1,7 +1,7 @@ from collections import OrderedDict from cfbs.masterfiles.analyze import version_as_comparable_list -from cfbs.utils import dict_diff, read_json, user_error, write_json +from cfbs.utils import dict_diff, read_json, UserError, write_json def check_download_matches_git(versions): @@ -78,7 +78,7 @@ def check_download_matches_git(versions): write_json("differences.json", diffs_dict) if len(nonmatching_versions) > 0: - user_error( + raise UserError( "The masterfiles downloaded from github.com and cfengine.com do not match - found " + str(extraneous_count) + " extraneous file" diff --git a/cfbs/masterfiles/download_all_versions.py b/cfbs/masterfiles/download_all_versions.py index 49c1c124..812bb6c9 100644 --- a/cfbs/masterfiles/download_all_versions.py +++ b/cfbs/masterfiles/download_all_versions.py @@ -2,7 +2,7 @@ import shutil from cfbs.masterfiles.analyze import version_is_at_least -from cfbs.utils import FetchError, fetch_url, get_json, mkdir, user_error +from cfbs.utils import FetchError, fetch_url, get_json, mkdir, UserError ENTERPRISE_RELEASES_URL = "https://cfengine.com/release-data/enterprise/releases.json" @@ -16,7 +16,7 @@ def get_download_urls_enterprise(min_version=None): try: data = get_json(ENTERPRISE_RELEASES_URL) except FetchError as e: - user_error( + raise UserError( "Downloading CFEngine release data failed - check your Wi-Fi / network settings." ) @@ -46,7 +46,7 @@ def get_download_urls_enterprise(min_version=None): try: subdata = get_json(release_url) except FetchError as e: - user_error( + raise UserError( "Downloading CFEngine release data for version %s failed - check your Wi-Fi / network settings." % version ) @@ -98,7 +98,7 @@ def download_versions_from_urls(download_path, download_urls, reported_checksums try: fetch_url(url, tarball_path, checksum) except FetchError as e: - user_error("For version " + version + ": " + str(e)) + raise UserError("For version " + version + ": " + str(e)) tarball_dir_path = os.path.join(version_path, "tarball") shutil.unpack_archive(tarball_path, tarball_dir_path) diff --git a/cfbs/updates.py b/cfbs/updates.py index 23904e75..717d38a0 100644 --- a/cfbs/updates.py +++ b/cfbs/updates.py @@ -3,7 +3,7 @@ import logging as log from cfbs.prompts import YES_NO_CHOICES, prompt_user -from cfbs.utils import read_json, user_error, write_json +from cfbs.utils import read_json, UserError, write_json class ModuleUpdates: @@ -112,7 +112,7 @@ def _update_variable(input_def, input_data): def_subtype, data_subtype, ("label", "question", "default") ) else: - user_error( + raise UserError( "Unsupported subtype '%s' in input definition for module '%s'." % (type(def_subtype).__name__, module_name) ) diff --git a/cfbs/utils.py b/cfbs/utils.py index 17d70656..a2087256 100644 --- a/cfbs/utils.py +++ b/cfbs/utils.py @@ -20,6 +20,10 @@ class ProgrammerError(RuntimeError): pass +class UserError(Exception): + pass + + def _sh(cmd: str): # print(cmd) try: @@ -31,7 +35,7 @@ def _sh(cmd: str): stderr=subprocess.STDOUT, ) except subprocess.CalledProcessError as e: - user_error("Command failed - %s\n%s" % (cmd, e.stdout.decode("utf-8"))) + raise UserError("Command failed - %s\n%s" % (cmd, e.stdout.decode("utf-8"))) def sh(cmd: str, directory=None): @@ -85,10 +89,6 @@ def pad_right(s, n): return s.ljust(n) -def user_error(msg: str): - sys.exit("Error: " + msg) - - def get_json(url: str) -> OrderedDict: try: with urllib.request.urlopen(url) as r: diff --git a/cfbs/validate.py b/cfbs/validate.py index 98c10df2..dd08baca 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -27,7 +27,7 @@ from cfbs.utils import ( is_a_commit_hash, - user_error, + UserError, ) from cfbs.pretty import TOP_LEVEL_KEYS, MODULE_KEYS from cfbs.cfbs_config import CFBSConfig @@ -603,12 +603,12 @@ def validate_module_input(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""" if not "build" in config: - user_error( + raise UserError( 'A "build" field is missing in ./cfbs.json' + " - The 'cfbs build' command loops through all modules in this list to find build steps to perform" ) if type(config["build"]) is not list: - user_error( + raise UserError( 'The "build" field in ./cfbs.json must be a list (of modules involved in the build)' ) if len(config["build"]) > 0: @@ -617,7 +617,7 @@ def _validate_config_for_build_field(config, empty_build_list_ok=False): name = module["name"] if "name" in module else index _validate_module_object("build", name, module, config) elif not empty_build_list_ok: - user_error( + raise UserError( "The \"build\" field in ./cfbs.json is empty - add modules with 'cfbs add'" ) From 00bff676892d250947aaf051bd86f4a3fa5d8e35 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Thu, 3 Jul 2025 17:34:42 +0200 Subject: [PATCH 08/12] Renamed UserError to GenericExitError This is a more honest name, given that we've been using this everywhere, even for things which are not the user's fault. Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/analyze.py | 12 +++--- cfbs/args.py | 8 ++-- cfbs/build.py | 30 ++++++++------- cfbs/cfbs_config.py | 24 ++++++------ cfbs/cfbs_json.py | 8 ++-- cfbs/commands.py | 38 ++++++++++--------- cfbs/index.py | 10 ++--- cfbs/internal_file_management.py | 38 +++++++++++-------- cfbs/main.py | 32 ++++++++-------- cfbs/man_generator.py | 4 +- .../masterfiles/check_download_matches_git.py | 4 +- cfbs/masterfiles/download_all_versions.py | 8 ++-- cfbs/updates.py | 4 +- cfbs/utils.py | 6 ++- cfbs/validate.py | 8 ++-- 15 files changed, 125 insertions(+), 109 deletions(-) diff --git a/cfbs/analyze.py b/cfbs/analyze.py index 488eba6e..b6f63a45 100644 --- a/cfbs/analyze.py +++ b/cfbs/analyze.py @@ -17,7 +17,7 @@ immediate_subdirectories, mkdir, read_json, - UserError, + GenericExitError, ) @@ -112,11 +112,11 @@ def mpf_vcf_dicts(offline=False): cfbs_ri_dir = os.path.join(cfbs_dir(), RI_SUBDIRS) if not os.path.exists(cfbs_ri_dir): - raise UserError(ERROR_MESSAGE) + raise GenericExitError(ERROR_MESSAGE) ri_versions = immediate_subdirectories(cfbs_ri_dir) if len(ri_versions) == 0: - raise UserError(ERROR_MESSAGE) + raise GenericExitError(ERROR_MESSAGE) ri_latest_version = max(ri_versions) mpf_vcf_path = os.path.join( @@ -134,7 +134,7 @@ def mpf_vcf_dicts(offline=False): try: latest_release_data = get_json(LATEST_RELEASE_API_URL) except FetchError as e: - raise UserError( + raise GenericExitError( "Downloading CFEngine release information failed - check your Wi-Fi / network settings." ) @@ -155,7 +155,7 @@ def mpf_vcf_dicts(offline=False): try: fetch_url(ri_checksums_url, archive_checksums_path) except FetchError as e: - raise UserError(str(e)) + raise GenericExitError(str(e)) with open(archive_checksums_path) as file: lines = [line.rstrip() for line in file] @@ -614,7 +614,7 @@ def analyze_policyset( + "\n ".join(possible_policyset_relpaths) + "\n" ) - raise UserError( + raise GenericExitError( "There doesn't seem to be a valid policy set in the supplied path.\n Usage: cfbs analyze path/to/policy-set\n" + extra_error_text ) diff --git a/cfbs/args.py b/cfbs/args.py index 0e9f6796..7e3b015e 100644 --- a/cfbs/args.py +++ b/cfbs/args.py @@ -2,7 +2,7 @@ import os from cfbs import commands -from cfbs.utils import cache, UserError +from cfbs.utils import cache, GenericExitError def get_args(): @@ -23,13 +23,13 @@ def get_manual(): with open(file_path, "r", encoding="utf-8") as man_file: man = man_file.read() if not man: - raise UserError("Manual file is empty") + raise GenericExitError("Manual file is empty") else: return man except OSError: - raise UserError("Error reading manual file " + file_path) + raise GenericExitError("Error reading manual file " + file_path) else: - raise UserError("Manual file does not exist") + raise GenericExitError("Manual file does not exist") @cache diff --git a/cfbs/build.py b/cfbs/build.py index 4aea4c0f..6496620f 100644 --- a/cfbs/build.py +++ b/cfbs/build.py @@ -26,7 +26,7 @@ sh, strip_left, touch, - UserError, + GenericExitError, write_json, ) from cfbs.pretty import pretty, pretty_file @@ -92,15 +92,17 @@ def _perform_replace_step(n, a, b, filename): with open(filename, "r") as f: content = f.read() except FileNotFoundError: - raise UserError("No such file '%s' in replace build step" % (filename,)) + raise GenericExitError("No such file '%s' in replace build step" % (filename,)) except: - raise UserError("Could not open/read '%s' in replace build step" % (filename,)) + raise GenericExitError( + "Could not open/read '%s' in replace build step" % (filename,) + ) new_content = previous_content = content for i in range(0, n): previous_content = new_content new_content = previous_content.replace(a, b, 1) if new_content == previous_content: - raise UserError( + raise GenericExitError( "replace build step could only replace '%s' in '%s' %s times, not %s times (required)" % (a, filename, i, n) ) @@ -112,12 +114,12 @@ def _perform_replace_step(n, a, b, filename): if new_content == previous_content: break if a in new_content: - raise UserError("too many occurences of '%s' in '%s'" % (a, filename)) + raise GenericExitError("too many occurences of '%s' in '%s'" % (a, filename)) try: with open(filename, "w") as f: f.write(new_content) except: - raise UserError("Failed to write to '%s'" % (filename,)) + raise GenericExitError("Failed to write to '%s'" % (filename,)) def _perform_build_step(module, i, step, max_length): @@ -158,7 +160,7 @@ def _perform_build_step(module, i, step, max_length): dst = "" print("%s json '%s' 'masterfiles/%s'" % (prefix, src, dst)) if not os.path.isfile(os.path.join(source, src)): - raise UserError("'%s' is not a file" % src) + raise GenericExitError("'%s' is not a file" % src) src, dst = os.path.join(source, src), os.path.join(destination, dst) extras, original = read_json(src), read_json(dst) if not extras: @@ -228,7 +230,7 @@ def _perform_build_step(module, i, step, max_length): extras = _generate_augment(name, extras) log.debug("Generated augment: %s", pretty(extras)) if not extras: - raise UserError( + raise GenericExitError( "Input data '%s' is incomplete: Skipping build step." % os.path.basename(src) ) @@ -251,7 +253,7 @@ def _perform_build_step(module, i, step, max_length): cf_files = find("out/masterfiles/" + file, extension=".cf") files += (strip_left(f, "out/masterfiles/") for f in cf_files) else: - raise UserError( + raise GenericExitError( "Unsupported filetype '%s' for build step '%s': " % (file, operation) + "Expected directory (*/) of policy file (*.cf)" @@ -306,7 +308,7 @@ def _perform_build_step(module, i, step, max_length): def perform_build(config) -> int: if not config.get("build"): - raise UserError("No 'build' key found in the configuration") + raise GenericExitError("No 'build' key found in the configuration") # mini-validation for module in config.get("build", []): @@ -314,25 +316,25 @@ def perform_build(config) -> int: operation, args = split_build_step(step) if step.split() != [operation] + args: - raise UserError( + raise GenericExitError( "Incorrect whitespace in the `%s` build step - singular spaces are required" % step ) if operation not in AVAILABLE_BUILD_STEPS: - raise UserError("Unknown build step operation: %s" % operation) + raise GenericExitError("Unknown build step operation: %s" % operation) expected = AVAILABLE_BUILD_STEPS[operation] actual = len(args) if not step_has_valid_arg_count(args, expected): if type(expected) is int: - raise UserError( + raise GenericExitError( "The `%s` build step expects %d arguments, %d were given" % (step, expected, actual) ) else: expected = int(expected[0:-1]) - raise UserError( + raise GenericExitError( "The `%s` build step expects %d or more arguments, %d were given" % (step, expected, actual) ) diff --git a/cfbs/cfbs_config.py b/cfbs/cfbs_config.py index c00694ba..f9750891 100644 --- a/cfbs/cfbs_config.py +++ b/cfbs/cfbs_config.py @@ -7,7 +7,7 @@ from cfbs.result import Result from cfbs.utils import ( - UserError, + GenericExitError, read_file, write_json, load_bundlenames, @@ -100,7 +100,7 @@ def add_with_dependencies( module_str = module module = (remote_config or self).get_module_for_build(module, str_added_by) if not module: - raise UserError("Module '%s' not found" % module_str) + raise GenericExitError("Module '%s' not found" % module_str) assert "name" in module name = module["name"] assert "steps" in module @@ -148,7 +148,7 @@ def _add_using_url( if len(to_add) == 0: modules = list(provides.values()) if not any(modules): - raise UserError("no modules available, nothing to do") + raise GenericExitError("no modules available, nothing to do") print("Found %d modules in '%s':" % (len(modules), url)) for m in modules: deps = m.get("dependencies", []) @@ -166,7 +166,7 @@ def _add_using_url( else: missing = [k for k in to_add if k not in provides] if missing: - raise UserError("Missing modules: " + ", ".join(missing)) + raise GenericExitError("Missing modules: " + ", ".join(missing)) modules = [provides[k] for k in to_add] for i, module in enumerate(modules, start=1): @@ -382,7 +382,7 @@ def add_command( checksum=None, ) -> int: if not to_add: - raise UserError("Must specify at least one module to add") + raise GenericExitError("Must specify at least one module to add") before = {m["name"] for m in self.get("build", [])} @@ -396,7 +396,7 @@ def add_command( else: # for this `if` to be valid, module names containing `://` should be illegal if "://" in to_add[0]: - raise UserError( + raise GenericExitError( "URI scheme not supported. The supported URI schemes are: " + ", ".join(SUPPORTED_URI_SCHEMES) ) @@ -450,7 +450,7 @@ def input_command(self, module_name, input_data): def _check_keys(keys, input_data): for key in keys: if key not in input_data: - raise UserError( + raise GenericExitError( "Expected attribute '%s' in input definition: %s" % (key, pretty(input_data)) ) @@ -469,7 +469,7 @@ def _input_elements(subtype): for element in subtype: _check_keys(["type", "label", "question", "key"], element) if element["type"] != "string": - raise UserError( + raise GenericExitError( "Subtype of type '%s' not supported for type list" % element["type"] ) @@ -496,7 +496,7 @@ def _input_list(input_data): elif isinstance(subtype, dict): _check_keys(["type", "label", "question"], subtype) if subtype["type"] != "string": - raise UserError( + raise GenericExitError( "Subtype of type '%s' not supported for type list" % subtype["type"] ) @@ -509,7 +509,7 @@ def _input_list(input_data): ).lower() in ("yes", "y"): result.append(_input_string(subtype)) return result - raise UserError( + raise GenericExitError( "Expected the value of attribute 'subtype' to be a JSON list or object, not: %s" % pretty(input_data["subtype"]) ) @@ -523,7 +523,9 @@ def _input_list(input_data): elif definition["type"] == "list": definition["response"] = _input_list(definition) else: - raise UserError("Unsupported input type '%s'" % definition["type"]) + raise GenericExitError( + "Unsupported input type '%s'" % definition["type"] + ) def _get_all_module_names(self, search_in=("build", "provides", "index")): modules = [] diff --git a/cfbs/cfbs_json.py b/cfbs/cfbs_json.py index 84983f68..c345ebfd 100644 --- a/cfbs/cfbs_json.py +++ b/cfbs/cfbs_json.py @@ -4,7 +4,7 @@ from cfbs.index import Index from cfbs.pretty import pretty, TOP_LEVEL_KEYS, MODULE_KEYS -from cfbs.utils import read_json, UserError +from cfbs.utils import read_json, GenericExitError def _construct_provided_module(name, data, url, commit, added_by="cfbs add"): @@ -16,7 +16,7 @@ def _construct_provided_module(name, data, url, commit, added_by="cfbs add"): module = OrderedDict() module["name"] = name if "description" not in data: - raise UserError( + raise GenericExitError( "missing required key 'description' in module definition: %s" % pretty(data) ) module["description"] = data["description"] @@ -31,7 +31,7 @@ def _construct_provided_module(name, data, url, commit, added_by="cfbs add"): if "input" in data: module["input"] = data["input"] if "steps" not in data: - raise UserError( + raise GenericExitError( "missing required key 'steps' in module definition: %s" % pretty(data) ) module["steps"] = data["steps"] @@ -126,7 +126,7 @@ def __contains__(self, key): def get_provides(self, added_by="cfbs add"): modules = OrderedDict() if "provides" not in self._data: - raise UserError( + raise GenericExitError( "missing required key 'provides' in module definition: %s" % pretty(self._data) ) diff --git a/cfbs/commands.py b/cfbs/commands.py index 95a82cc9..f32b9baa 100644 --- a/cfbs/commands.py +++ b/cfbs/commands.py @@ -21,7 +21,7 @@ cfbs_filename, is_cfbs_repo, read_json, - UserError, + GenericExitError, strip_right, pad_right, ProgrammerError, @@ -96,13 +96,13 @@ def get_command_names(): @cfbs_command("pretty") def pretty_command(filenames: list, check: bool, keep_order: bool) -> int: if not filenames: - raise UserError("Filenames missing for cfbs pretty command") + raise GenericExitError("Filenames missing for cfbs pretty command") sorting_rules = CFBS_DEFAULT_SORTING_RULES if keep_order else None num_files = 0 for f in filenames: if not f or not f.endswith(".json"): - raise UserError( + raise GenericExitError( "cfbs pretty command can only be used with .json files, not '%s'" % os.path.basename(f) ) @@ -114,9 +114,9 @@ def pretty_command(filenames: list, check: bool, keep_order: bool) -> int: else: pretty_file(f, sorting_rules) except FileNotFoundError: - raise UserError("File '%s' not found" % f) + raise GenericExitError("File '%s' not found" % f) except json.decoder.JSONDecodeError as ex: - raise UserError("Error reading json file '{}': {}".format(f, ex)) + raise GenericExitError("Error reading json file '{}': {}".format(f, ex)) if check: print("Would reformat %d file(s)" % num_files) return 1 if num_files > 0 else 0 @@ -126,7 +126,7 @@ def pretty_command(filenames: list, check: bool, keep_order: bool) -> int: @cfbs_command("init") def init_command(index=None, masterfiles=None, non_interactive=False) -> int: if is_cfbs_repo(): - raise UserError("Already initialized - look at %s" % cfbs_filename()) + raise GenericExitError("Already initialized - look at %s" % cfbs_filename()) name = prompt_user( non_interactive, @@ -274,7 +274,7 @@ def init_command(index=None, masterfiles=None, non_interactive=False) -> int: remote = "https://github.com/cfengine/masterfiles" commit = ls_remote(remote, branch) if commit is None: - raise UserError( + raise GenericExitError( "Failed to find branch or tag %s at remote %s" % (branch, remote) ) log.debug("Current commit for masterfiles branch %s is %s" % (branch, commit)) @@ -398,7 +398,7 @@ def remove_command(to_remove: list): config = CFBSConfig.get_instance() config.warn_about_unknown_keys() if not "build" in config: - raise UserError( + raise GenericExitError( 'Cannot remove any modules because the "build" key is missing from cfbs.json' ) modules = config["build"] @@ -456,7 +456,7 @@ def _get_modules_by_url(name) -> list: if name.startswith(SUPPORTED_URI_SCHEMES): matches = _get_modules_by_url(name) if not matches: - raise UserError("Could not find module with URL '%s'" % name) + raise GenericExitError("Could not find module with URL '%s'" % name) for module in matches: answer = _remove_module_user_prompt(module) if answer.lower() in ("yes", "y"): @@ -710,10 +710,10 @@ def _download_dependencies( counter += 1 continue if "commit" not in module: - raise UserError("module %s must have a commit property" % name) + raise GenericExitError("module %s must have a commit property" % name) commit = module["commit"] if not is_a_commit_hash(commit): - raise UserError("'%s' is not a commit reference" % commit) + raise GenericExitError("'%s' is not a commit reference" % commit) url = module.get("url") or module["repo"] url = strip_right(url, ".git") @@ -727,7 +727,7 @@ def _download_dependencies( if not os.path.exists(module_dir): if url.endswith(SUPPORTED_ARCHIVES): if os.path.exists(commit_dir) and "subdirectory" in module: - raise UserError( + raise GenericExitError( "Subdirectory '%s' for module '%s' was not found in fetched archive '%s': " % (module["subdirectory"], name, url) + "Please check cfbs.json for possible typos." @@ -738,7 +738,7 @@ def _download_dependencies( # - added by URL instead of name (no version property in module data) elif "index" in module or "url" in module or ignore_versions: if os.path.exists(commit_dir) and "subdirectory" in module: - raise UserError( + raise GenericExitError( "Subdirectory '%s' for module '%s' was not found in cloned repository '%s': " % (module["subdirectory"], name, url) + "Please check cfbs.json for possible typos." @@ -749,13 +749,15 @@ def _download_dependencies( try: versions = get_json(_VERSION_INDEX) except FetchError as e: - raise UserError( + raise GenericExitError( "Downloading CFEngine Build Module Index failed - check your Wi-Fi / network settings." ) try: checksum = versions[name][module["version"]]["archive_sha256"] except KeyError: - raise UserError("Cannot verify checksum of the '%s' module" % name) + raise GenericExitError( + "Cannot verify checksum of the '%s' module" % name + ) module_archive_url = os.path.join( _MODULES_URL, name, commit + ".tar.gz" ) @@ -810,7 +812,7 @@ def build_command(ignore_versions=False) -> int: @cfbs_command("install") def install_command(args) -> int: if len(args) > 1: - raise UserError( + raise GenericExitError( "Only one destination is allowed for command: cfbs install [destination]" ) if not os.path.exists("out/masterfiles"): @@ -867,7 +869,7 @@ def _print_module_info(data): @cfbs_command("info") def info_command(modules): if not modules: - raise UserError( + raise GenericExitError( "info/show command requires one or more module names as arguments" ) config = CFBSConfig.get_instance() @@ -936,7 +938,7 @@ def analyze_command( ) if not os.path.isdir(path): - raise UserError("the provided policy set path is not a directory") + raise GenericExitError("the provided policy set path is not a directory") if masterfiles_dir is None: masterfiles_dir = "masterfiles" diff --git a/cfbs/index.py b/cfbs/index.py index 0cd4bc4b..3724f573 100644 --- a/cfbs/index.py +++ b/cfbs/index.py @@ -2,7 +2,7 @@ from collections import OrderedDict from cfbs.module import Module -from cfbs.utils import FetchError, get_or_read_json, UserError, get_json +from cfbs.utils import FetchError, get_or_read_json, GenericExitError, get_json from cfbs.internal_file_management import local_module_name _DEFAULT_INDEX = ( @@ -90,7 +90,7 @@ def _expand_index(self): try: self._data = get_or_read_json(index) except FetchError as e: - raise UserError( + raise GenericExitError( "Downloading index '%s' failed - check your Wi-Fi / network settings." % index ) @@ -130,7 +130,7 @@ def exists(self, module): try: versions = get_json(_VERSION_INDEX) except FetchError as e: - raise UserError( + raise GenericExitError( "Downloading CFEngine Build Module Index failed - check your Wi-Fi / network settings." ) @@ -140,7 +140,7 @@ def check_existence(self, modules: list): for module in modules: assert isinstance(module, Module) if not self.exists(module): - raise UserError( + raise GenericExitError( "Module '%s'%s does not exist" % ( module.name, @@ -177,7 +177,7 @@ def get_module_object(self, module, added_by=None): try: versions = get_json(_VERSION_INDEX) except FetchError as e: - raise UserError( + raise GenericExitError( "Downloading CFEngine Build Module Index failed - check your Wi-Fi / network settings." ) new_values = versions[name][version] diff --git a/cfbs/internal_file_management.py b/cfbs/internal_file_management.py index 657e7ea1..8abd3366 100644 --- a/cfbs/internal_file_management.py +++ b/cfbs/internal_file_management.py @@ -23,7 +23,7 @@ rm, sh, strip_right, - UserError, + GenericExitError, ) _SUPPORTED_TAR_TYPES = (".tar.gz", ".tgz") @@ -38,11 +38,13 @@ def local_module_name(module_path): if module.endswith((".cf", ".json", "/")) and not module.startswith("./"): module = "./" + module if not module.startswith("./"): - raise UserError("Please prepend local files or folders with './' to avoid ambiguity") + raise GenericExitError( + "Please prepend local files or folders with './' to avoid ambiguity" + ) for illegal in ["//", "..", " ", "\n", "\t", " "]: if illegal in module: - raise UserError("Module path cannot contain %s" % repr(illegal)) + raise GenericExitError("Module path cannot contain %s" % repr(illegal)) if os.path.isdir(module) and not module.endswith("/"): module = module + "/" @@ -52,10 +54,10 @@ def local_module_name(module_path): assert os.path.exists(module) if os.path.isfile(module): if not module.endswith((".cf", ".json")): - raise UserError("Only .cf and .json files supported currently") + raise GenericExitError("Only .cf and .json files supported currently") else: if not os.path.isdir(module): - raise UserError("'%s' must be either a directory or a file" % module) + raise GenericExitError("'%s' must be either a directory or a file" % module) return module @@ -65,7 +67,7 @@ def get_download_path(module) -> str: commit = module["commit"] if not is_a_commit_hash(commit): - raise UserError("'%s' is not a commit reference" % commit) + raise GenericExitError("'%s' is not a commit reference" % commit) url = module.get("url") or module["repo"] if url.endswith(SUPPORTED_ARCHIVES): @@ -92,9 +94,9 @@ def _prettify_name(name): def local_module_copy(module, counter, max_length): name = module["name"] if not name.startswith("./"): - raise UserError("module %s must start with ./" % name) + raise GenericExitError("module %s must start with ./" % name) if not os.path.isfile(name) and not os.path.isdir(name): - raise UserError("module %s does not exist" % name) + raise GenericExitError("module %s does not exist" % name) pretty_name = _prettify_name(name) target = "out/steps/%03d_%s_local/" % (counter, pretty_name) module["_directory"] = target @@ -116,7 +118,7 @@ def local_module_copy(module, counter, max_length): def _get_path_from_url(url): if not url.startswith(SUPPORTED_URI_SCHEMES): if "://" in url: - return raise UserError("Unsupported URL protocol in '%s'" % url) + return raise GenericExitError("Unsupported URL protocol in '%s'" % url) else: # It's a path already, just remove trailing slashes (if any). return url.rstrip("/") @@ -161,7 +163,7 @@ def clone_url_repo(repo_url): # commit specified in the url repo_url, commit = repo_url.rsplit("@", 1) if not is_a_commit_hash(commit): - raise UserError("'%s' is not a commit reference" % commit) + raise GenericExitError("'%s' is not a commit reference" % commit) downloads = os.path.join(cfbs_dir(), "downloads") @@ -188,7 +190,7 @@ def clone_url_repo(repo_url): if os.path.exists(json_path): return (json_path, commit) else: - raise UserError( + raise GenericExitError( "Repository '%s' doesn't contain a valid cfbs.json index file" % repo_url ) @@ -207,7 +209,7 @@ def fetch_archive( archive_type = ext break else: - raise UserError("Unsupported archive type: '%s'" % url) + raise GenericExitError("Unsupported archive type: '%s'" % url) archive_name = strip_right(archive_filename, archive_type) downloads = os.path.join(cfbs_dir(), "downloads") @@ -220,7 +222,7 @@ def fetch_archive( try: archive_checksum = fetch_url(url, archive_path, checksum) except FetchError as e: - raise UserError(str(e)) + raise GenericExitError(str(e)) content_dir = os.path.join(downloads, archive_dir, archive_checksum) if extract_to_directory: @@ -238,12 +240,16 @@ def fetch_archive( if shutil.which("tar"): sh("cd %s; tar -xf %s" % (content_dir, archive_path)) else: - raise UserError("Working with .tar archives requires the 'tar' utility") + raise GenericExitError( + "Working with .tar archives requires the 'tar' utility" + ) elif archive_type == (".zip"): if shutil.which("unzip"): sh("cd %s; unzip %s" % (content_dir, archive_path)) else: - raise UserError("Working with .zip archives requires the 'unzip' utility") + raise GenericExitError( + "Working with .zip archives requires the 'unzip' utility" + ) else: raise RuntimeError( "Unhandled archive type: '%s'. Please report this at %s." @@ -270,7 +276,7 @@ def fetch_archive( if os.path.exists(index_path): return (index_path, archive_checksum) else: - raise UserError( + raise GenericExitError( "Archive '%s' doesn't contain a valid cfbs.json index file" % url ) else: diff --git a/cfbs/main.py b/cfbs/main.py index 3d96e884..bb7026bf 100644 --- a/cfbs/main.py +++ b/cfbs/main.py @@ -8,7 +8,7 @@ from cfbs.validate import CFBSValidationError from cfbs.version import string as version -from cfbs.utils import UserError, is_cfbs_repo, ProgrammerError +from cfbs.utils import GenericExitError, is_cfbs_repo, ProgrammerError from cfbs.cfbs_config import CFBSConfig from cfbs import commands from cfbs.args import get_args, print_help, get_manual @@ -60,62 +60,62 @@ def _main() -> int: if not args.command: print_help() print("") - raise UserError("No command given") + raise GenericExitError("No command given") if args.command not in commands.get_command_names(): print_help() - raise UserError("Command '%s' not found" % args.command) + raise GenericExitError("Command '%s' not found" % args.command) if args.masterfiles and args.command != "init": - raise UserError( + raise GenericExitError( "The option --masterfiles is only for 'cfbs init', not 'cfbs %s'" % args.command ) if args.omit_download and args.command != "generate-release-information": - raise UserError( + raise GenericExitError( "The option --omit-download is only for 'cfbs generate-release-information', not 'cfbs %s'" % args.command ) if args.check_against_git and args.command != "generate-release-information": - raise UserError( + raise GenericExitError( "The option --check-against-git is only for 'cfbs generate-release-information', not 'cfbs %s'" % args.command ) if args.minimum_version and args.command != "generate-release-information": - raise UserError( + raise GenericExitError( "The option --from is only for 'cfbs generate-release-information', not 'cfbs %s'" % args.command ) if args.masterfiles_dir and args.command not in ("analyze", "analyse"): - raise UserError( + raise GenericExitError( "The option --masterfiles-dir is only for 'cfbs analyze', not 'cfbs %s'" % args.command ) if args.reference_version and args.command not in ("analyze", "analyse"): - raise UserError( + raise GenericExitError( "The option --reference-version is only for 'cfbs analyze', not 'cfbs %s'" % args.command ) if args.to_json and args.command not in ("analyze", "analyse"): - raise UserError( + raise GenericExitError( "The option --to-json is only for 'cfbs analyze', not 'cfbs %s'" % args.command ) if args.ignored_path_components and args.command not in ("analyze", "analyse"): - raise UserError( + raise GenericExitError( "The option --ignored-path-components is only for 'cfbs analyze', not 'cfbs %s'" % args.command ) if args.offline and args.command not in ("analyze", "analyse"): - raise UserError( + raise GenericExitError( "The option --offline is only for 'cfbs analyze', not 'cfbs %s'" % args.command ) @@ -128,7 +128,7 @@ def _main() -> int: "update", "input", ): - raise UserError( + raise GenericExitError( "The option --non-interactive is not for cfbs %s" % (args.command) ) @@ -174,7 +174,9 @@ def _main() -> int: ) if not is_cfbs_repo(): - raise UserError("This is not a cfbs repo, to get started, type: cfbs init") + raise GenericExitError( + "This is not a cfbs repo, to get started, type: cfbs init" + ) if args.command == "status": return commands.status_command() @@ -246,7 +248,7 @@ def main() -> int: return _main() except CFBSValidationError as e: print("Error: " + str(e)) - except UserError as e: + except GenericExitError as e: print("Error: " + str(e)) # TODO: Handle other exceptions return 1 diff --git a/cfbs/man_generator.py b/cfbs/man_generator.py index 84aeef10..97318ebe 100644 --- a/cfbs/man_generator.py +++ b/cfbs/man_generator.py @@ -1,11 +1,11 @@ import os -from utils import UserError +from utils import GenericExitError from args import get_arg_parser try: from build_manpages.manpage import Manpage except ImportError: - raise UserError( + raise GenericExitError( "Missing dependency, install from PyPI: 'pip install argparse-manpage setuptools'" ) diff --git a/cfbs/masterfiles/check_download_matches_git.py b/cfbs/masterfiles/check_download_matches_git.py index f91989b4..029458df 100644 --- a/cfbs/masterfiles/check_download_matches_git.py +++ b/cfbs/masterfiles/check_download_matches_git.py @@ -1,7 +1,7 @@ from collections import OrderedDict from cfbs.masterfiles.analyze import version_as_comparable_list -from cfbs.utils import dict_diff, read_json, UserError, write_json +from cfbs.utils import dict_diff, read_json, GenericExitError, write_json def check_download_matches_git(versions): @@ -78,7 +78,7 @@ def check_download_matches_git(versions): write_json("differences.json", diffs_dict) if len(nonmatching_versions) > 0: - raise UserError( + raise GenericExitError( "The masterfiles downloaded from github.com and cfengine.com do not match - found " + str(extraneous_count) + " extraneous file" diff --git a/cfbs/masterfiles/download_all_versions.py b/cfbs/masterfiles/download_all_versions.py index 812bb6c9..ac9e6a6b 100644 --- a/cfbs/masterfiles/download_all_versions.py +++ b/cfbs/masterfiles/download_all_versions.py @@ -2,7 +2,7 @@ import shutil from cfbs.masterfiles.analyze import version_is_at_least -from cfbs.utils import FetchError, fetch_url, get_json, mkdir, UserError +from cfbs.utils import FetchError, fetch_url, get_json, mkdir, GenericExitError ENTERPRISE_RELEASES_URL = "https://cfengine.com/release-data/enterprise/releases.json" @@ -16,7 +16,7 @@ def get_download_urls_enterprise(min_version=None): try: data = get_json(ENTERPRISE_RELEASES_URL) except FetchError as e: - raise UserError( + raise GenericExitError( "Downloading CFEngine release data failed - check your Wi-Fi / network settings." ) @@ -46,7 +46,7 @@ def get_download_urls_enterprise(min_version=None): try: subdata = get_json(release_url) except FetchError as e: - raise UserError( + raise GenericExitError( "Downloading CFEngine release data for version %s failed - check your Wi-Fi / network settings." % version ) @@ -98,7 +98,7 @@ def download_versions_from_urls(download_path, download_urls, reported_checksums try: fetch_url(url, tarball_path, checksum) except FetchError as e: - raise UserError("For version " + version + ": " + str(e)) + raise GenericExitError("For version " + version + ": " + str(e)) tarball_dir_path = os.path.join(version_path, "tarball") shutil.unpack_archive(tarball_path, tarball_dir_path) diff --git a/cfbs/updates.py b/cfbs/updates.py index 717d38a0..10f7d1fa 100644 --- a/cfbs/updates.py +++ b/cfbs/updates.py @@ -3,7 +3,7 @@ import logging as log from cfbs.prompts import YES_NO_CHOICES, prompt_user -from cfbs.utils import read_json, UserError, write_json +from cfbs.utils import read_json, GenericExitError, write_json class ModuleUpdates: @@ -112,7 +112,7 @@ def _update_variable(input_def, input_data): def_subtype, data_subtype, ("label", "question", "default") ) else: - raise UserError( + raise GenericExitError( "Unsupported subtype '%s' in input definition for module '%s'." % (type(def_subtype).__name__, module_name) ) diff --git a/cfbs/utils.py b/cfbs/utils.py index a2087256..045eee2c 100644 --- a/cfbs/utils.py +++ b/cfbs/utils.py @@ -20,7 +20,7 @@ class ProgrammerError(RuntimeError): pass -class UserError(Exception): +class GenericExitError(Exception): pass @@ -35,7 +35,9 @@ def _sh(cmd: str): stderr=subprocess.STDOUT, ) except subprocess.CalledProcessError as e: - raise UserError("Command failed - %s\n%s" % (cmd, e.stdout.decode("utf-8"))) + raise GenericExitError( + "Command failed - %s\n%s" % (cmd, e.stdout.decode("utf-8")) + ) def sh(cmd: str, directory=None): diff --git a/cfbs/validate.py b/cfbs/validate.py index dd08baca..226a6227 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -27,7 +27,7 @@ from cfbs.utils import ( is_a_commit_hash, - UserError, + GenericExitError, ) from cfbs.pretty import TOP_LEVEL_KEYS, MODULE_KEYS from cfbs.cfbs_config import CFBSConfig @@ -603,12 +603,12 @@ def validate_module_input(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""" if not "build" in config: - raise UserError( + raise GenericExitError( 'A "build" field is missing in ./cfbs.json' + " - The 'cfbs build' command loops through all modules in this list to find build steps to perform" ) if type(config["build"]) is not list: - raise UserError( + raise GenericExitError( 'The "build" field in ./cfbs.json must be a list (of modules involved in the build)' ) if len(config["build"]) > 0: @@ -617,7 +617,7 @@ def _validate_config_for_build_field(config, empty_build_list_ok=False): name = module["name"] if "name" in module else index _validate_module_object("build", name, module, config) elif not empty_build_list_ok: - raise UserError( + raise GenericExitError( "The \"build\" field in ./cfbs.json is empty - add modules with 'cfbs add'" ) From 22170c0aebc294af4e7ce89a2231b20c9b2ce292 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Thu, 3 Jul 2025 17:43:55 +0200 Subject: [PATCH 09/12] Fixed find and replace mistake with return vs raise Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/internal_file_management.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cfbs/internal_file_management.py b/cfbs/internal_file_management.py index 8abd3366..05a64ece 100644 --- a/cfbs/internal_file_management.py +++ b/cfbs/internal_file_management.py @@ -118,7 +118,7 @@ def local_module_copy(module, counter, max_length): def _get_path_from_url(url): if not url.startswith(SUPPORTED_URI_SCHEMES): if "://" in url: - return raise GenericExitError("Unsupported URL protocol in '%s'" % url) + raise GenericExitError("Unsupported URL protocol in '%s'" % url) else: # It's a path already, just remove trailing slashes (if any). return url.rstrip("/") From 1cb22430a703c67d71e63bec6758c71d2a40cafd Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Thu, 3 Jul 2025 17:44:50 +0200 Subject: [PATCH 10/12] Fixed mistake with modules lacking name inside dict Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/build.py | 4 ++-- cfbs/validate.py | 8 +++++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/cfbs/build.py b/cfbs/build.py index 6496620f..f587b5cb 100644 --- a/cfbs/build.py +++ b/cfbs/build.py @@ -290,14 +290,14 @@ def _perform_build_step(module, i, step, max_length): assert len(args) == 4 print("%s replace '%s'" % (prefix, "' '".join(args))) # New build step so let's be a bit strict about validating it: - validate_build_step(module, i, operation, args, strict=True) + validate_build_step(name, module, i, operation, args, strict=True) n, a, b, file = args file = os.path.join(destination, file) _perform_replace_step(n, a, b, file) elif operation == "replace_version": assert len(args) == 3 # New build step so let's be a bit strict about validating it: - validate_build_step(module, i, operation, args, strict=True) + validate_build_step(name, module, i, operation, args, strict=True) print("%s replace_version '%s'" % (prefix, "' '".join(args))) n = args[0] to_replace = args[1] diff --git a/cfbs/validate.py b/cfbs/validate.py index 226a6227..4fe9c516 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -190,8 +190,10 @@ def validate_config(config, empty_build_list_ok=False): return 1 -def validate_build_step(module, i, operation, args, strict=False): - name = module["name"] +def validate_build_step(name, module, i, operation, args, strict=False): + assert type(name) is str + assert type(module) is not str + assert type(i) is int if not operation in AVAILABLE_BUILD_STEPS: raise CFBSValidationError( name, @@ -408,7 +410,7 @@ def validate_steps(name, module): name, '"steps" must be a list of non-empty / non-whitespace strings' ) operation, args = split_build_step(step) - validate_build_step(module, i, operation, args) + validate_build_step(name, module, i, operation, args) def validate_url_field(name, module, field): assert field in module From 5da814748e0c63dde35a0a8a9ebd2472f5249c4c Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Thu, 3 Jul 2025 18:42:07 +0200 Subject: [PATCH 11/12] Added more strict validation rules for replace filenames Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/validate.py | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/cfbs/validate.py b/cfbs/validate.py index 4fe9c516..ad5ef3e5 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -46,7 +46,11 @@ "replace_version": 3, # n, string to replace, filename } +# Constants / regexes / limits for validating build steps: MAX_REPLACEMENTS = 1000 +FILENAME_RE = r"[-_/a-zA-Z0-9\.]+" +MAX_FILENAME_LENGTH = 128 +MAX_BUILD_STEP_LENGTH = 256 def split_build_step(command) -> Tuple[str, List[str]]: @@ -194,6 +198,14 @@ def validate_build_step(name, module, i, operation, args, strict=False): assert type(name) is str assert type(module) is not str assert type(i) is int + + if strict: + step = operation + " " + " ".join(args) + if len(step) > MAX_FILENAME_LENGTH: + raise CFBSValidationError( + "%s build step in '%s' is too long" % (operation, name) + ) + if not operation in AVAILABLE_BUILD_STEPS: raise CFBSValidationError( name, @@ -245,6 +257,34 @@ def validate_build_step(name, module, i, operation, args, strict=False): "'%s' must not contain '%s' in replace build step (could lead to recursive replacing)" % (a, b) ) + if filename == "." or filename.endswith(("/", "/.")): + raise CFBSValidationError( + "replace build step works on files, not '%s'" % (filename,) + ) + if filename.startswith("/"): + raise CFBSValidationError( + "replace build step works on relative file paths, not '%s'" + % (filename,) + ) + if filename.startswith("./"): + raise CFBSValidationError( + "replace file paths are always relative, drop the ./ in '%s'" + % (filename,) + ) + if ".." in filename: + raise CFBSValidationError( + ".. not allowed in replace file path ('%s')" % (filename,) + ) + if "/./" in filename: + raise CFBSValidationError( + "/./ not allowed in replace file path ('%s')" % (filename,) + ) + if not re.fullmatch(FILENAME_RE, filename): + raise CFBSValidationError( + "filename in replace build step contains illegal characters ('%s')" + % (filename,) + ) + elif operation == "replace_version": assert len(args) == 3 n, to_replace, filename = args From 1f3612b154d3378ae42a7c810fa821f7af46d2ed Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Thu, 3 Jul 2025 18:42:28 +0200 Subject: [PATCH 12/12] Fixed mistake in string formatting Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/validate.py | 1 - 1 file changed, 1 deletion(-) diff --git a/cfbs/validate.py b/cfbs/validate.py index ad5ef3e5..2f1855bc 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -120,7 +120,6 @@ def _validate_top_level_keys(config): if config["type"] == "index" and type(config["index"]) not in (dict, OrderedDict): raise CFBSValidationError( 'For a cfbs.json with "index" as type, the "index" field must be an object / dictionary' - % field ) # Further check types / values of those required fields: