From ba903ee625cfc5a27ddeaae11c46ef71a94c2e77 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Mon, 4 Dec 2023 19:11:37 +0100 Subject: [PATCH 01/64] Moved module validation function into separate function Ticket: CFE-4060 Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/validate.py | 40 ++++++++++++++++++++++------------------ 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/cfbs/validate.py b/cfbs/validate.py index 0adb4d41..6495d8e8 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -51,8 +51,7 @@ def validate_config(config, build=False): return 1 return 0 - -def _validate_index(index): +def _validate_module_object(mode, name, module, modules): def validate_alias(name, modules): if len(modules[name]) != 1: raise CFBSIndexException( @@ -161,6 +160,25 @@ def validate_url_field(name, modules, field): if url and not url.startswith("https://"): raise CFBSIndexException(name, "'%s' must be an HTTPS URL" % field) + if "alias" in modules[name]: + validate_alias(name, modules) + else: + validate_description(name, modules) + validate_tags(name, modules) + validate_repo(name, modules) + validate_by(name, modules) + if "dependencies" in modules[name]: # optional attribute + validate_dependencies(name, modules) + validate_version(name, modules) + validate_commit(name, modules) + if "subdirectory" in modules[name]: # optional attribute + validate_subdirectory(name, modules) + validate_steps(name, modules) + validate_url_field(name, modules, "website") + validate_url_field(name, modules, "documentation") + + +def _validate_index(index): # Make sure index has a collection named modules if not "index" in index: raise CFBSIndexException(None, "Missing required attribute 'index'") @@ -168,22 +186,8 @@ def validate_url_field(name, modules, field): # Validate each entry in modules for name in modules: - if "alias" in modules[name]: - validate_alias(name, modules) - else: - validate_description(name, modules) - validate_tags(name, modules) - validate_repo(name, modules) - validate_by(name, modules) - if "dependencies" in modules[name]: # optional attribute - validate_dependencies(name, modules) - validate_version(name, modules) - validate_commit(name, modules) - if "subdirectory" in modules[name]: # optional attribute - validate_subdirectory(name, modules) - validate_steps(name, modules) - validate_url_field(name, modules, "website") - validate_url_field(name, modules, "documentation") + module = modules[name] + _validate_module_object("index", name, module, modules) def _validate_config_for_build_field(config): From bed074d33904282c4324344eaea4f148c34bd32a Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Mon, 4 Dec 2023 19:14:00 +0100 Subject: [PATCH 02/64] Removed unused import Ticket: CFE-4060 Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/commands.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cfbs/commands.py b/cfbs/commands.py index eaf36f33..fd0118e5 100644 --- a/cfbs/commands.py +++ b/cfbs/commands.py @@ -36,7 +36,7 @@ ) from cfbs.cfbs_json import TOP_LEVEL_KEYS, MODULE_KEYS from cfbs.cfbs_config import CFBSConfig, CFBSReturnWithoutCommit -from cfbs.validate import CFBSIndexException, validate_config +from cfbs.validate import validate_config from cfbs.internal_file_management import ( fetch_archive, get_download_path, From 1b91d1ff0ab488fa54579937e7030a71ac483a17 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Tue, 5 Dec 2023 11:19:50 +0100 Subject: [PATCH 03/64] Renamed CFBSIndexError to CFBSValidationError Ticket: CFE-4060 Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/validate.py | 76 ++++++++++++++++++++++++------------------------ 1 file changed, 38 insertions(+), 38 deletions(-) diff --git a/cfbs/validate.py b/cfbs/validate.py index 6495d8e8..6edc7131 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -7,7 +7,7 @@ from cfbs.cfbs_config import CFBSConfig -class CFBSIndexException(Exception): +class CFBSValidationError(Exception): def __init__(self, name, message) -> None: if name is None: super().__init__("Error in index: " + message) @@ -46,7 +46,7 @@ def validate_config(config, build=False): try: _validate_index(data) - except CFBSIndexException as e: + except CFBSValidationError as e: print(e) return 1 return 0 @@ -54,111 +54,111 @@ def validate_config(config, build=False): def _validate_module_object(mode, name, module, modules): def validate_alias(name, modules): if len(modules[name]) != 1: - raise CFBSIndexException( + raise CFBSValidationError( name, "'alias' cannot be used with other attributes" ) if type(modules[name]["alias"]) != str: - raise CFBSIndexException(name, "'alias' must be of type string") + raise CFBSValidationError(name, "'alias' must be of type string") if not modules[name]["alias"]: - raise CFBSIndexException(name, "'alias' must be non-empty") + raise CFBSValidationError(name, "'alias' must be non-empty") if not modules[name]["alias"] in modules: - raise CFBSIndexException(name, "'alias' must reference another module") + raise CFBSValidationError(name, "'alias' must reference another module") if "alias" in modules[modules[name]["alias"]]: - raise CFBSIndexException(name, "'alias' cannot reference another alias") + raise CFBSValidationError(name, "'alias' cannot reference another alias") def validate_description(name, modules): if not "description" in modules[name]: - raise CFBSIndexException(name, "Missing required attribute 'description'") + raise CFBSValidationError(name, "Missing required attribute 'description'") if type(modules[name]["description"]) != str: - raise CFBSIndexException(name, "'description' must be of type string") + raise CFBSValidationError(name, "'description' must be of type string") if not modules[name]["description"]: - raise CFBSIndexException(name, "'description' must be non-empty") + raise CFBSValidationError(name, "'description' must be non-empty") def validate_tags(name, modules): if not "tags" in modules[name]: - raise CFBSIndexException(name, "Missing required attribute 'tags'") + raise CFBSValidationError(name, "Missing required attribute 'tags'") if type(modules[name]["tags"]) != list: - raise CFBSIndexException(name, "'tags' must be of type list") + raise CFBSValidationError(name, "'tags' must be of type list") for tag in modules[name]["tags"]: if type(tag) != str: - raise CFBSIndexException(name, "'tags' must be a list of strings") + raise CFBSValidationError(name, "'tags' must be a list of strings") def validate_repo(name, modules): if not "repo" in modules[name]: - raise CFBSIndexException(name, "Missing required attribute 'repo'") + raise CFBSValidationError(name, "Missing required attribute 'repo'") if type(modules[name]["repo"]) != str: - raise CFBSIndexException(name, "'repo' must be of type string") + raise CFBSValidationError(name, "'repo' must be of type string") if not modules[name]["repo"]: - raise CFBSIndexException(name, "'repo' must be non-empty") + raise CFBSValidationError(name, "'repo' must be non-empty") def validate_by(name, modules): if not "by" in modules[name]: - raise CFBSIndexException(name, "Missing reqired attribute 'by'") + raise CFBSValidationError(name, "Missing reqired attribute 'by'") if type(modules[name]["by"]) != str: - raise CFBSIndexException(name, "'by' must be of type string") + raise CFBSValidationError(name, "'by' must be of type string") if not modules[name]["by"]: - raise CFBSIndexException(name, "'by' must be non-empty") + raise CFBSValidationError(name, "'by' must be non-empty") def validate_dependencies(name, modules): if type(modules[name]["dependencies"]) != list: - raise CFBSIndexException( + raise CFBSValidationError( name, "Value of attribute 'dependencies' must be of type list" ) for dependency in modules[name]["dependencies"]: if type(dependency) != str: - raise CFBSIndexException( + raise CFBSValidationError( name, "'dependencies' must be a list of strings" ) if not dependency in modules: - raise CFBSIndexException(name, "'dependencies' reference other modules") + raise CFBSValidationError(name, "'dependencies' reference other modules") if "alias" in modules[dependency]: - raise CFBSIndexException( + raise CFBSValidationError( name, "'dependencies' cannot reference an alias" ) def validate_version(name, modules): if not "version" in modules[name]: - raise CFBSIndexException(name, "Missing required attribute 'version'") + raise CFBSValidationError(name, "Missing required attribute 'version'") if type(modules[name]["version"]) != str: - raise CFBSIndexException(name, "'version' must be of type string") + raise CFBSValidationError(name, "'version' must be of type string") regex = r"(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)(-([0-9]+))?" if re.fullmatch(regex, modules[name]["version"]) == None: - raise CFBSIndexException(name, "'version' must match regex %s" % regex) + raise CFBSValidationError(name, "'version' must match regex %s" % regex) def validate_commit(name, modules): if not "commit" in modules[name]: - raise CFBSIndexException(name, "Missing required attribute 'commit'") + raise CFBSValidationError(name, "Missing required attribute 'commit'") commit = modules[name]["commit"] if type(commit) != str: - raise CFBSIndexException(name, "'commit' must be of type string") + raise CFBSValidationError(name, "'commit' must be of type string") if not is_a_commit_hash(commit): - raise CFBSIndexException(name, "'commit' must be a commit reference") + raise CFBSValidationError(name, "'commit' must be a commit reference") def validate_subdirectory(name, modules): if type(modules[name]["subdirectory"]) != str: - raise CFBSIndexException(name, "'subdirectory' must be of type string") + raise CFBSValidationError(name, "'subdirectory' must be of type string") if not modules[name]["subdirectory"]: - raise CFBSIndexException(name, "'subdirectory' must be non-empty") + raise CFBSValidationError(name, "'subdirectory' must be non-empty") def validate_steps(name, modules): if not "steps" in modules[name]: - raise CFBSIndexException(name, "Missing required attribute 'steps'") + raise CFBSValidationError(name, "Missing required attribute 'steps'") if type(modules[name]["steps"]) != list: - raise CFBSIndexException(name, "'steps' must be of type list") + raise CFBSValidationError(name, "'steps' must be of type list") if not modules[name]["steps"]: - raise CFBSIndexException(name, "'steps' must be non-empty") + raise CFBSValidationError(name, "'steps' must be non-empty") for step in modules[name]["steps"]: if type(step) != str: - raise CFBSIndexException(name, "'steps' must be a list of strings") + raise CFBSValidationError(name, "'steps' must be a list of strings") if not step: - raise CFBSIndexException( + raise CFBSValidationError( name, "'steps' must be a list of non-empty strings" ) def validate_url_field(name, modules, field): url = modules[name].get(field) if url and not url.startswith("https://"): - raise CFBSIndexException(name, "'%s' must be an HTTPS URL" % field) + raise CFBSValidationError(name, "'%s' must be an HTTPS URL" % field) if "alias" in modules[name]: validate_alias(name, modules) @@ -181,7 +181,7 @@ def validate_url_field(name, modules, field): def _validate_index(index): # Make sure index has a collection named modules if not "index" in index: - raise CFBSIndexException(None, "Missing required attribute 'index'") + raise CFBSValidationError(None, "Missing required attribute 'index'") modules = index["index"] # Validate each entry in modules From c0e1ecae4fb16cb58d1d3637f71d2470781c9f7a Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Tue, 5 Dec 2023 11:25:45 +0100 Subject: [PATCH 04/64] Changed to double quotes in validation errors to match JSON syntax Ticket: CFE-4060 Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/validate.py | 73 +++++++++++++++++++++++++----------------------- 1 file changed, 38 insertions(+), 35 deletions(-) diff --git a/cfbs/validate.py b/cfbs/validate.py index 6edc7131..f9fbad51 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -51,114 +51,117 @@ def validate_config(config, build=False): return 1 return 0 + def _validate_module_object(mode, name, module, modules): def validate_alias(name, modules): if len(modules[name]) != 1: raise CFBSValidationError( - name, "'alias' cannot be used with other attributes" + name, '"alias" cannot be used with other attributes' ) if type(modules[name]["alias"]) != str: - raise CFBSValidationError(name, "'alias' must be of type string") + raise CFBSValidationError(name, '"alias" must be of type string') if not modules[name]["alias"]: - raise CFBSValidationError(name, "'alias' must be non-empty") + raise CFBSValidationError(name, '"alias" must be non-empty') if not modules[name]["alias"] in modules: - raise CFBSValidationError(name, "'alias' must reference another module") + raise CFBSValidationError(name, '"alias" must reference another module') if "alias" in modules[modules[name]["alias"]]: - raise CFBSValidationError(name, "'alias' cannot reference another alias") + raise CFBSValidationError(name, '"alias" cannot reference another alias') def validate_description(name, modules): if not "description" in modules[name]: - raise CFBSValidationError(name, "Missing required attribute 'description'") + raise CFBSValidationError(name, 'Missing required attribute "description"') if type(modules[name]["description"]) != str: - raise CFBSValidationError(name, "'description' must be of type string") + raise CFBSValidationError(name, '"description" must be of type string') if not modules[name]["description"]: - raise CFBSValidationError(name, "'description' must be non-empty") + raise CFBSValidationError(name, '"description" must be non-empty') def validate_tags(name, modules): if not "tags" in modules[name]: - raise CFBSValidationError(name, "Missing required attribute 'tags'") + raise CFBSValidationError(name, 'Missing required attribute "tags"') if type(modules[name]["tags"]) != list: - raise CFBSValidationError(name, "'tags' must be of type list") + raise CFBSValidationError(name, '"tags" must be of type list') for tag in modules[name]["tags"]: if type(tag) != str: - raise CFBSValidationError(name, "'tags' must be a list of strings") + raise CFBSValidationError(name, '"tags" must be a list of strings') def validate_repo(name, modules): if not "repo" in modules[name]: - raise CFBSValidationError(name, "Missing required attribute 'repo'") + raise CFBSValidationError(name, 'Missing required attribute "repo"') if type(modules[name]["repo"]) != str: - raise CFBSValidationError(name, "'repo' must be of type string") + raise CFBSValidationError(name, '"repo" must be of type string') if not modules[name]["repo"]: - raise CFBSValidationError(name, "'repo' must be non-empty") + raise CFBSValidationError(name, '"repo" must be non-empty') def validate_by(name, modules): if not "by" in modules[name]: - raise CFBSValidationError(name, "Missing reqired attribute 'by'") + raise CFBSValidationError(name, 'Missing reqired attribute "by"') if type(modules[name]["by"]) != str: - raise CFBSValidationError(name, "'by' must be of type string") + raise CFBSValidationError(name, '"by" must be of type string') if not modules[name]["by"]: - raise CFBSValidationError(name, "'by' must be non-empty") + raise CFBSValidationError(name, '"by" must be non-empty') def validate_dependencies(name, modules): if type(modules[name]["dependencies"]) != list: raise CFBSValidationError( - name, "Value of attribute 'dependencies' must be of type list" + name, 'Value of attribute "dependencies" must be of type list' ) for dependency in modules[name]["dependencies"]: if type(dependency) != str: raise CFBSValidationError( - name, "'dependencies' must be a list of strings" + name, '"dependencies" must be a list of strings' ) if not dependency in modules: - raise CFBSValidationError(name, "'dependencies' reference other modules") + raise CFBSValidationError( + name, '"dependencies" reference other modules' + ) if "alias" in modules[dependency]: raise CFBSValidationError( - name, "'dependencies' cannot reference an alias" + name, '"dependencies" cannot reference an alias' ) def validate_version(name, modules): if not "version" in modules[name]: - raise CFBSValidationError(name, "Missing required attribute 'version'") + raise CFBSValidationError(name, 'Missing required attribute "version"') if type(modules[name]["version"]) != str: - raise CFBSValidationError(name, "'version' must be of type string") + raise CFBSValidationError(name, '"version" must be of type string') regex = r"(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)(-([0-9]+))?" if re.fullmatch(regex, modules[name]["version"]) == None: - raise CFBSValidationError(name, "'version' must match regex %s" % regex) + raise CFBSValidationError(name, '"version" must match regex %s' % regex) def validate_commit(name, modules): if not "commit" in modules[name]: - raise CFBSValidationError(name, "Missing required attribute 'commit'") + raise CFBSValidationError(name, 'Missing required attribute "commit"') commit = modules[name]["commit"] if type(commit) != str: - raise CFBSValidationError(name, "'commit' must be of type string") + raise CFBSValidationError(name, '"commit" must be of type string') if not is_a_commit_hash(commit): - raise CFBSValidationError(name, "'commit' must be a commit reference") + raise CFBSValidationError(name, '"commit" must be a commit reference') def validate_subdirectory(name, modules): if type(modules[name]["subdirectory"]) != str: - raise CFBSValidationError(name, "'subdirectory' must be of type string") + raise CFBSValidationError(name, '"subdirectory" must be of type string') if not modules[name]["subdirectory"]: - raise CFBSValidationError(name, "'subdirectory' must be non-empty") + raise CFBSValidationError(name, '"subdirectory" must be non-empty') def validate_steps(name, modules): if not "steps" in modules[name]: - raise CFBSValidationError(name, "Missing required attribute 'steps'") + raise CFBSValidationError(name, 'Missing required attribute "steps"') if type(modules[name]["steps"]) != list: - raise CFBSValidationError(name, "'steps' must be of type list") + raise CFBSValidationError(name, '"steps" must be of type list') if not modules[name]["steps"]: - raise CFBSValidationError(name, "'steps' must be non-empty") + raise CFBSValidationError(name, '"steps" must be non-empty') for step in modules[name]["steps"]: if type(step) != str: - raise CFBSValidationError(name, "'steps' must be a list of strings") + raise CFBSValidationError(name, '"steps" must be a list of strings') if not step: raise CFBSValidationError( - name, "'steps' must be a list of non-empty strings" + name, '"steps" must be a list of non-empty strings' ) def validate_url_field(name, modules, field): url = modules[name].get(field) if url and not url.startswith("https://"): - raise CFBSValidationError(name, "'%s' must be an HTTPS URL" % field) + raise CFBSValidationError(name, '"%" must be an HTTPS URL' % field) if "alias" in modules[name]: validate_alias(name, modules) From 58434eb0c9d62fd74516cf62fe43ce282bd34be3 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Tue, 5 Dec 2023 11:36:53 +0100 Subject: [PATCH 05/64] Refactored module alias validation to prepare for further changes Ticket: CFE-4060 Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/validate.py | 40 +++++++++++++++++++++++++--------------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/cfbs/validate.py b/cfbs/validate.py index f9fbad51..069398d9 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -163,22 +163,32 @@ def validate_url_field(name, modules, field): if url and not url.startswith("https://"): raise CFBSValidationError(name, '"%" must be an HTTPS URL' % field) + assert module == modules[name] + assert mode in ("index", "provides", "build") + + # Step 1 - Handle special cases (alias): if "alias" in modules[name]: - validate_alias(name, modules) - else: - validate_description(name, modules) - validate_tags(name, modules) - validate_repo(name, modules) - validate_by(name, modules) - if "dependencies" in modules[name]: # optional attribute - validate_dependencies(name, modules) - validate_version(name, modules) - validate_commit(name, modules) - if "subdirectory" in modules[name]: # optional attribute - validate_subdirectory(name, modules) - validate_steps(name, modules) - validate_url_field(name, modules, "website") - validate_url_field(name, modules, "documentation") + if mode in ("index", "provides"): + validate_alias(name, modules) + return + else: + assert mode == "build" + raise ValidationError(name, '"alias" is not supported in "build"') + + # Step 2 - Validate fields: + validate_description(name, modules) + validate_tags(name, modules) + validate_repo(name, modules) + validate_by(name, modules) + if "dependencies" in modules[name]: # optional attribute + validate_dependencies(name, modules) + validate_version(name, modules) + validate_commit(name, modules) + if "subdirectory" in modules[name]: # optional attribute + validate_subdirectory(name, modules) + validate_steps(name, modules) + validate_url_field(name, modules, "website") + validate_url_field(name, modules, "documentation") def _validate_index(index): From 2ded2a57630e8c226ebd466d2b5c2d507c16e149 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Tue, 5 Dec 2023 11:45:00 +0100 Subject: [PATCH 06/64] Added separate validation for required fields Ticket: CFE-4060 Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/validate.py | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/cfbs/validate.py b/cfbs/validate.py index 069398d9..b8723338 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -175,7 +175,29 @@ def validate_url_field(name, modules, field): assert mode == "build" raise ValidationError(name, '"alias" is not supported in "build"') - # Step 2 - Validate fields: + + # Step 2 - Check for required fields: + + required_fields = ["steps"] + + if mode == "build": + required_fields.append("name") + elif mode == "provides": + required_fields.append("description") + else: + assert mode == "index" + required_fields.append("description") + required_fields.append("tags") + required_fields.append("repo") + required_fields.append("by") + required_fields.append("version") + required_fields.append("commit") + + for required_field in required_field: + if required_field not in module: + raise CFBSValidationError(name, '"%s" field is required, but missing') + + # Step 3 - Validate fields: validate_description(name, modules) validate_tags(name, modules) validate_repo(name, modules) From 0032e3231f47f9bb0e4d3d6604d10acb38cfebaf Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Tue, 5 Dec 2023 11:50:57 +0100 Subject: [PATCH 07/64] Simplified validation functions now that required fields are handled separately Ticket: CFE-4060 Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/validate.py | 56 +++++++++++++++++++++++++++--------------------- 1 file changed, 31 insertions(+), 25 deletions(-) diff --git a/cfbs/validate.py b/cfbs/validate.py index b8723338..5528e78c 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -54,6 +54,7 @@ def validate_config(config, build=False): def _validate_module_object(mode, name, module, modules): def validate_alias(name, modules): + assert "alias" in modules[name] if len(modules[name]) != 1: raise CFBSValidationError( name, '"alias" cannot be used with other attributes' @@ -68,16 +69,14 @@ def validate_alias(name, modules): raise CFBSValidationError(name, '"alias" cannot reference another alias') def validate_description(name, modules): - if not "description" in modules[name]: - raise CFBSValidationError(name, 'Missing required attribute "description"') + assert "description" in modules[name] if type(modules[name]["description"]) != str: raise CFBSValidationError(name, '"description" must be of type string') if not modules[name]["description"]: raise CFBSValidationError(name, '"description" must be non-empty') def validate_tags(name, modules): - if not "tags" in modules[name]: - raise CFBSValidationError(name, 'Missing required attribute "tags"') + assert "tags" in modules[name] if type(modules[name]["tags"]) != list: raise CFBSValidationError(name, '"tags" must be of type list') for tag in modules[name]["tags"]: @@ -85,22 +84,21 @@ def validate_tags(name, modules): raise CFBSValidationError(name, '"tags" must be a list of strings') def validate_repo(name, modules): - if not "repo" in modules[name]: - raise CFBSValidationError(name, 'Missing required attribute "repo"') + assert "repo" in modules[name] if type(modules[name]["repo"]) != str: raise CFBSValidationError(name, '"repo" must be of type string') if not modules[name]["repo"]: raise CFBSValidationError(name, '"repo" must be non-empty') def validate_by(name, modules): - if not "by" in modules[name]: - raise CFBSValidationError(name, 'Missing reqired attribute "by"') + assert "by" in modules[name] if type(modules[name]["by"]) != str: raise CFBSValidationError(name, '"by" must be of type string') if not modules[name]["by"]: raise CFBSValidationError(name, '"by" must be non-empty') def validate_dependencies(name, modules): + assert "dependencies" in modules[name] if type(modules[name]["dependencies"]) != list: raise CFBSValidationError( name, 'Value of attribute "dependencies" must be of type list' @@ -120,8 +118,7 @@ def validate_dependencies(name, modules): ) def validate_version(name, modules): - if not "version" in modules[name]: - raise CFBSValidationError(name, 'Missing required attribute "version"') + assert "version" in modules[name] if type(modules[name]["version"]) != str: raise CFBSValidationError(name, '"version" must be of type string') regex = r"(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)(-([0-9]+))?" @@ -129,8 +126,7 @@ def validate_version(name, modules): raise CFBSValidationError(name, '"version" must match regex %s' % regex) def validate_commit(name, modules): - if not "commit" in modules[name]: - raise CFBSValidationError(name, 'Missing required attribute "commit"') + assert "commit" in modules[name] commit = modules[name]["commit"] if type(commit) != str: raise CFBSValidationError(name, '"commit" must be of type string') @@ -138,14 +134,14 @@ def validate_commit(name, modules): raise CFBSValidationError(name, '"commit" must be a commit reference') def validate_subdirectory(name, modules): + assert "subdirectory" in modules[name] if type(modules[name]["subdirectory"]) != str: raise CFBSValidationError(name, '"subdirectory" must be of type string') if not modules[name]["subdirectory"]: raise CFBSValidationError(name, '"subdirectory" must be non-empty') def validate_steps(name, modules): - if not "steps" in modules[name]: - raise CFBSValidationError(name, 'Missing required attribute "steps"') + assert "steps" in modules[name] if type(modules[name]["steps"]) != list: raise CFBSValidationError(name, '"steps" must be of type list') if not modules[name]["steps"]: @@ -159,6 +155,7 @@ def validate_steps(name, modules): ) def validate_url_field(name, modules, field): + assert field in modules[name] url = modules[name].get(field) if url and not url.startswith("https://"): raise CFBSValidationError(name, '"%" must be an HTTPS URL' % field) @@ -198,19 +195,28 @@ def validate_url_field(name, modules, field): raise CFBSValidationError(name, '"%s" field is required, but missing') # Step 3 - Validate fields: - validate_description(name, modules) - validate_tags(name, modules) - validate_repo(name, modules) - validate_by(name, modules) - if "dependencies" in modules[name]: # optional attribute + if "description" in module: + validate_description(name, modules) + if "tags" in module: + validate_tags(name, modules) + if "repo" in module: + validate_repo(name, modules) + if "by" in module: + validate_by(name, modules) + if "dependencies" in module: validate_dependencies(name, modules) - validate_version(name, modules) - validate_commit(name, modules) - if "subdirectory" in modules[name]: # optional attribute + if "version" in module: + validate_version(name, modules) + if "commit" in module: + validate_commit(name, modules) + if "subdirectory" in module: validate_subdirectory(name, modules) - validate_steps(name, modules) - validate_url_field(name, modules, "website") - validate_url_field(name, modules, "documentation") + if "steps" in module: + validate_steps(name, modules) + if "website" in module: + validate_url_field(name, modules, "website") + if "documentation" in module: + validate_url_field(name, modules, "documentation") def _validate_index(index): From 3a82a9dd90d7ab42177816952f9b947b1dfd5dff Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Tue, 5 Dec 2023 11:55:19 +0100 Subject: [PATCH 08/64] Changed all validation functions which only care about a single module object to only take that module as an argument Only "dependencies" field needs to reference other modules in the dict. Ticket: CFE-4060 Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/validate.py | 115 ++++++++++++++++++++++++----------------------- 1 file changed, 58 insertions(+), 57 deletions(-) diff --git a/cfbs/validate.py b/cfbs/validate.py index 5528e78c..aa30a53d 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -53,57 +53,58 @@ def validate_config(config, build=False): def _validate_module_object(mode, name, module, modules): - def validate_alias(name, modules): - assert "alias" in modules[name] - if len(modules[name]) != 1: + def validate_alias(name, module): + assert "alias" in module + if len(module) != 1: raise CFBSValidationError( name, '"alias" cannot be used with other attributes' ) - if type(modules[name]["alias"]) != str: + if type(module["alias"]) != str: raise CFBSValidationError(name, '"alias" must be of type string') - if not modules[name]["alias"]: + if not module["alias"]: raise CFBSValidationError(name, '"alias" must be non-empty') - if not modules[name]["alias"] in modules: + if not module["alias"] in modules: raise CFBSValidationError(name, '"alias" must reference another module') - if "alias" in modules[modules[name]["alias"]]: + if "alias" in modules[module["alias"]]: raise CFBSValidationError(name, '"alias" cannot reference another alias') - def validate_description(name, modules): - assert "description" in modules[name] - if type(modules[name]["description"]) != str: + def validate_description(name, module): + assert "description" in module + if type(module["description"]) != str: raise CFBSValidationError(name, '"description" must be of type string') - if not modules[name]["description"]: + if not module["description"]: raise CFBSValidationError(name, '"description" must be non-empty') - def validate_tags(name, modules): - assert "tags" in modules[name] - if type(modules[name]["tags"]) != list: + def validate_tags(name, module): + assert "tags" in module + if type(module["tags"]) != list: raise CFBSValidationError(name, '"tags" must be of type list') - for tag in modules[name]["tags"]: + for tag in module["tags"]: if type(tag) != str: raise CFBSValidationError(name, '"tags" must be a list of strings') - def validate_repo(name, modules): - assert "repo" in modules[name] - if type(modules[name]["repo"]) != str: + def validate_repo(name, module): + assert "repo" in module + if type(module["repo"]) != str: raise CFBSValidationError(name, '"repo" must be of type string') - if not modules[name]["repo"]: + if not module["repo"]: raise CFBSValidationError(name, '"repo" must be non-empty') - def validate_by(name, modules): - assert "by" in modules[name] - if type(modules[name]["by"]) != str: + def validate_by(name, module): + assert "by" in module + if type(module["by"]) != str: raise CFBSValidationError(name, '"by" must be of type string') - if not modules[name]["by"]: + if not module["by"]: raise CFBSValidationError(name, '"by" must be non-empty') - def validate_dependencies(name, modules): - assert "dependencies" in modules[name] - if type(modules[name]["dependencies"]) != list: + def validate_dependencies(name, module, modules): + assert module == modules[name] + assert "dependencies" in module + if type(module["dependencies"]) != list: raise CFBSValidationError( name, 'Value of attribute "dependencies" must be of type list' ) - for dependency in modules[name]["dependencies"]: + for dependency in module["dependencies"]: if type(dependency) != str: raise CFBSValidationError( name, '"dependencies" must be a list of strings' @@ -117,36 +118,36 @@ def validate_dependencies(name, modules): name, '"dependencies" cannot reference an alias' ) - def validate_version(name, modules): - assert "version" in modules[name] - if type(modules[name]["version"]) != str: + def validate_version(name, module): + assert "version" in module + if type(module["version"]) != str: raise CFBSValidationError(name, '"version" must be of type string') regex = r"(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)(-([0-9]+))?" - if re.fullmatch(regex, modules[name]["version"]) == None: + if re.fullmatch(regex, module["version"]) == None: raise CFBSValidationError(name, '"version" must match regex %s' % regex) - def validate_commit(name, modules): - assert "commit" in modules[name] - commit = modules[name]["commit"] + def validate_commit(name, module): + assert "commit" in module + commit = module["commit"] if type(commit) != str: raise CFBSValidationError(name, '"commit" must be of type string') if not is_a_commit_hash(commit): raise CFBSValidationError(name, '"commit" must be a commit reference') - def validate_subdirectory(name, modules): - assert "subdirectory" in modules[name] - if type(modules[name]["subdirectory"]) != str: + def validate_subdirectory(name, module): + assert "subdirectory" in module + if type(module["subdirectory"]) != str: raise CFBSValidationError(name, '"subdirectory" must be of type string') - if not modules[name]["subdirectory"]: + if not module["subdirectory"]: raise CFBSValidationError(name, '"subdirectory" must be non-empty') - def validate_steps(name, modules): - assert "steps" in modules[name] - if type(modules[name]["steps"]) != list: + def validate_steps(name, module): + assert "steps" in module + if type(module["steps"]) != list: raise CFBSValidationError(name, '"steps" must be of type list') - if not modules[name]["steps"]: + if not module["steps"]: raise CFBSValidationError(name, '"steps" must be non-empty') - for step in modules[name]["steps"]: + for step in module["steps"]: if type(step) != str: raise CFBSValidationError(name, '"steps" must be a list of strings') if not step: @@ -154,9 +155,9 @@ def validate_steps(name, modules): name, '"steps" must be a list of non-empty strings' ) - def validate_url_field(name, modules, field): - assert field in modules[name] - url = modules[name].get(field) + def validate_url_field(name, module, field): + assert field in module + url = module.get(field) if url and not url.startswith("https://"): raise CFBSValidationError(name, '"%" must be an HTTPS URL' % field) @@ -196,27 +197,27 @@ def validate_url_field(name, modules, field): # Step 3 - Validate fields: if "description" in module: - validate_description(name, modules) + validate_description(name, module) if "tags" in module: - validate_tags(name, modules) + validate_tags(name, module) if "repo" in module: - validate_repo(name, modules) + validate_repo(name, module) if "by" in module: - validate_by(name, modules) + validate_by(name, module) if "dependencies" in module: - validate_dependencies(name, modules) + validate_dependencies(name, module, modules) if "version" in module: - validate_version(name, modules) + validate_version(name, module) if "commit" in module: - validate_commit(name, modules) + validate_commit(name, module) if "subdirectory" in module: - validate_subdirectory(name, modules) + validate_subdirectory(name, module) if "steps" in module: - validate_steps(name, modules) + validate_steps(name, module) if "website" in module: - validate_url_field(name, modules, "website") + validate_url_field(name, module, "website") if "documentation" in module: - validate_url_field(name, modules, "documentation") + validate_url_field(name, module, "documentation") def _validate_index(index): From 40d3b2f4217fa44349881702a592469d54588671 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Tue, 5 Dec 2023 12:04:01 +0100 Subject: [PATCH 09/64] Added basic validation of module names to shared validation function Ticket: CFE-4060 Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/validate.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/cfbs/validate.py b/cfbs/validate.py index aa30a53d..4031e963 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -68,6 +68,14 @@ def validate_alias(name, module): if "alias" in modules[module["alias"]]: raise CFBSValidationError(name, '"alias" cannot reference another alias') + def validate_name(name, module): + assert "name" in module + assert name == module["name"] + if type(module["name"]) != str: + raise CFBSValidationError(name, '"name" must be of type string') + if not module["name"]: + raise CFBSValidationError(name, '"name" must be non-empty') + def validate_description(name, module): assert "description" in module if type(module["description"]) != str: @@ -196,6 +204,9 @@ def validate_url_field(name, module, field): raise CFBSValidationError(name, '"%s" field is required, but missing') # Step 3 - Validate fields: + + if "name" in module: + validate_name(name, module) if "description" in module: validate_description(name, module) if "tags" in module: From ba7f69d833950d141181400011f452589a2bc6b6 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Tue, 5 Dec 2023 16:07:35 +0100 Subject: [PATCH 10/64] cfbs validate now more thoroughly validates various cfbs.json files Changed so the same module validation logic is used for the different keys ("build", "provides", "index"), and for the different types of cfbs.json files ("policy-set", "module", "index"). Ticket: CFE-4060 Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/validate.py | 61 +----------------------------------------------- 1 file changed, 1 insertion(+), 60 deletions(-) diff --git a/cfbs/validate.py b/cfbs/validate.py index 4031e963..7244de72 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -259,67 +259,8 @@ def _validate_config_for_build_field(config): "The \"build\" field in ./cfbs.json is empty - add modules with 'cfbs add'" ) for index, module in enumerate(config["build"]): - if not "name" in module: - user_error( - "The module at index " - + str(index) - + ' of "build" in ./cfbs.json is missing a "name"' - ) name = module["name"] - if type(name) is not str: - user_error( - "The module at index " - + str(index) - + ' of "build" in ./cfbs.json has a name which is not a string' - ) - if not name: - user_error( - "The module at index " - + str(index) - + ' of "build" in ./cfbs.json has an empty name' - ) - if ( - not "steps" in module - or type(module["steps"]) is not list - or module["steps"] == [] - ): - user_error( - 'Build steps are missing for the "' - + name - + '" module in ./cfbs.json - the "steps" field must have a non-empty list of steps to perform (strings)' - ) - - steps = module["steps"] - not_strings = len([step for step in steps if type(step) is not str]) - if not_strings == 1: - user_error( - "The module '" - + name - + '\' in "build" in ./cfbs.json has 1 step which is not a string' - ) - if not_strings > 1: - user_error( - "The module '" - + name - + '\' in "build" in ./cfbs.json has ' - + str(not_strings) - + " steps which are not strings" - ) - empty_strings = len([step for step in steps if step == ""]) - if empty_strings == 1: - user_error( - "The module '" - + name - + '\' in "build" in ./cfbs.json has 1 step which is empty' - ) - if empty_strings > 1: - user_error( - "The module '" - + name - + '\' in "build" in ./cfbs.json has ' - + str(empty_strings) - + " steps which are empty" - ) + _validate_module_object("build", name, module, config.index.data["index"]) def main(): From a8c1cc7123508112c0db2c95f2b3075eda105ffc Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Tue, 5 Dec 2023 16:12:46 +0100 Subject: [PATCH 11/64] validate.py: Small formatting improvements Ticket: CFE-4060 Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/validate.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cfbs/validate.py b/cfbs/validate.py index 7244de72..17273def 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -173,6 +173,7 @@ def validate_url_field(name, module, field): assert mode in ("index", "provides", "build") # Step 1 - Handle special cases (alias): + if "alias" in modules[name]: if mode in ("index", "provides"): validate_alias(name, modules) @@ -181,7 +182,6 @@ def validate_url_field(name, module, field): assert mode == "build" raise ValidationError(name, '"alias" is not supported in "build"') - # Step 2 - Check for required fields: required_fields = ["steps"] From 045c90647110a3ef50fc2607b8efaf503a49bf1c Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Tue, 5 Dec 2023 16:58:15 +0100 Subject: [PATCH 12/64] Added warnings when cfbs.json files have unknown fields Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/cfbs_json.py | 40 ++++++++++++++++++++++++++++++++++++++++ cfbs/commands.py | 9 +++++++++ cfbs/validate.py | 2 ++ 3 files changed, 51 insertions(+) diff --git a/cfbs/cfbs_json.py b/cfbs/cfbs_json.py index 92d444ac..b154270b 100644 --- a/cfbs/cfbs_json.py +++ b/cfbs/cfbs_json.py @@ -1,4 +1,5 @@ from collections import OrderedDict +import logging as log from cfbs.index import Index from cfbs.utils import read_json, user_error @@ -82,6 +83,45 @@ def __init__( else: self.index = Index() + def _find_all_module_objects(self): + data = self._data + modules = [] + if "index" in data and type(data["index"]): + modules += data["index"].values() + if "provides" in data and type(data["index"]): + modules += data["provides"].values() + if "build" in data and type(data["build"]): + modules += data["build"] + return modules + + + def warn_about_unknown_keys(self): + """Basic validation to warn the user when a cfbs.json has unknown keys. + + Unknown keys are typically due to + typos, or an outdated version of cfbs. This basic type of + validation only produces warnings (we want cfbs to still work), + and is run for all cfbs commands, not just cfbs validate. + For the more complete validation, see validate.py. + """ + + data = self._data + for key in data: + if key not in TOP_LEVEL_KEYS: + log.warning( + 'The top level key "%s" is not known to this version of cfbs.\n' + + "Is it a typo? If not, try upgrading cfbs:\n" + + "pip3 install --upgrade cfbs" + ) + for module in self._find_all_module_objects(): + for key in module: + if key not in MODULE_KEYS: + log.warning( + 'The module level key "%s" is not known to this version of cfbs.\n' + + "Is it a typo? If not, try upgrading cfbs:\n" + + "pip3 install --upgrade cfbs" + ) + def get(self, key, default=None): if not self._data: # If the specified JSON file does not exist return default diff --git a/cfbs/commands.py b/cfbs/commands.py index fd0118e5..e2f26d22 100644 --- a/cfbs/commands.py +++ b/cfbs/commands.py @@ -304,6 +304,7 @@ def init_command(index=None, masterfiles=None, non_interactive=False) -> int: @cfbs_command("status") def status_command() -> int: config = CFBSConfig.get_instance() + config.warn_about_unknown_keys() print("Name: %s" % config["name"]) print("Description: %s" % config["description"]) print("File: %s" % cfbs_filename()) @@ -380,6 +381,7 @@ def add_command( checksum=None, ) -> int: config = CFBSConfig.get_instance() + config.warn_about_unknown_keys() r = config.add_command(to_add, added_by, checksum) config.save() return r @@ -389,6 +391,7 @@ def add_command( @commit_after_command("Removed module%s %s", [PLURAL_S, FIRST_ARG_SLIST]) def remove_command(to_remove: list): config = CFBSConfig.get_instance() + config.warn_about_unknown_keys() modules = config["build"] def _get_module_by_name(name) -> dict: @@ -474,6 +477,7 @@ def clean_command(config=None): def _clean_unused_modules(config=None): if not config: config = CFBSConfig.get_instance() + config.warn_about_unknown_keys() modules = config["build"] def _someone_needs_me(this) -> bool: @@ -621,6 +625,7 @@ def _update_variable(input_def, input_data): @commit_after_command("Updated module%s", [PLURAL_S]) def update_command(to_update): config = CFBSConfig.get_instance() + config.warn_about_unknown_keys() build = config["build"] # Update all modules in build if none specified @@ -962,6 +967,7 @@ def info_command(modules): if not modules: user_error("info/show command requires one or more module names as arguments") config = CFBSConfig.get_instance() + config.warn_about_unknown_keys() index = config.index build = config.get("build", {}) @@ -1003,6 +1009,7 @@ def info_command(modules): @commit_after_command("Added input for module%s", [PLURAL_S]) def input_command(args, input_from="cfbs input"): config = CFBSConfig.get_instance() + config.warn_about_unknown_keys() do_commit = False files_to_commit = [] for module_name in args: @@ -1038,6 +1045,7 @@ def input_command(args, input_from="cfbs input"): @commit_after_command("Set input for module %s", [FIRST_ARG]) def set_input_command(name, infile): config = CFBSConfig.get_instance() + config.warn_about_unknown_keys() module = config.get_module_from_build(name) if module is None: log.error("Module '%s' not found" % name) @@ -1119,6 +1127,7 @@ def _compare_list(a, b): @cfbs_command("get-input") def get_input_command(name, outfile): config = CFBSConfig.get_instance() + config.warn_about_unknown_keys() module = config.get_module_from_build(name) if module is None: module = config.index.get_module_object(name) diff --git a/cfbs/validate.py b/cfbs/validate.py index 17273def..85612d28 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -16,6 +16,8 @@ def __init__(self, name, message) -> None: def validate_config(config, build=False): + config.warn_about_unknown_keys() + # First validate the config i.e. the user's cfbs.json # TODO: Add more validation for other things in config: From 6addb4cd281143347c764681281f3f4c6ba737cd Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Tue, 5 Dec 2023 17:19:31 +0100 Subject: [PATCH 13/64] Removed assertions which are no longer valid Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/validate.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/cfbs/validate.py b/cfbs/validate.py index 85612d28..3ba6e5d2 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -108,7 +108,6 @@ def validate_by(name, module): raise CFBSValidationError(name, '"by" must be non-empty') def validate_dependencies(name, module, modules): - assert module == modules[name] assert "dependencies" in module if type(module["dependencies"]) != list: raise CFBSValidationError( @@ -171,7 +170,6 @@ def validate_url_field(name, module, field): if url and not url.startswith("https://"): raise CFBSValidationError(name, '"%" must be an HTTPS URL' % field) - assert module == modules[name] assert mode in ("index", "provides", "build") # Step 1 - Handle special cases (alias): From f180fef71df2c5b680ef481b24615bb906a8099d Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Tue, 5 Dec 2023 17:19:49 +0100 Subject: [PATCH 14/64] Fixed typo in required fields logic Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/validate.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cfbs/validate.py b/cfbs/validate.py index 3ba6e5d2..22325b4f 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -199,7 +199,7 @@ def validate_url_field(name, module, field): required_fields.append("version") required_fields.append("commit") - for required_field in required_field: + for required_field in required_fields: if required_field not in module: raise CFBSValidationError(name, '"%s" field is required, but missing') From 9d5a224f2ee9e7e9637849828766dfd94f5e7564 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Tue, 5 Dec 2023 17:21:09 +0100 Subject: [PATCH 15/64] validate.py: Fixed typo in alias validation Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/validate.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cfbs/validate.py b/cfbs/validate.py index 22325b4f..68657070 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -176,7 +176,7 @@ def validate_url_field(name, module, field): if "alias" in modules[name]: if mode in ("index", "provides"): - validate_alias(name, modules) + validate_alias(name, module) return else: assert mode == "build" From 0f4649a60de3dfcc07aee23d44ac5db7f4fcf783 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Tue, 5 Dec 2023 18:43:40 +0100 Subject: [PATCH 16/64] Added raw_data property for working with the actual JSON data of a CFBSJson Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/cfbs_json.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/cfbs/cfbs_json.py b/cfbs/cfbs_json.py index b154270b..f453d0b8 100644 --- a/cfbs/cfbs_json.py +++ b/cfbs/cfbs_json.py @@ -1,4 +1,5 @@ from collections import OrderedDict +from copy import deepcopy import logging as log from cfbs.index import Index @@ -83,8 +84,13 @@ def __init__( else: self.index = Index() + @property + def raw_data(self): + """Read-only access to the original data, for validation purposes""" + return deepcopy(self._data) + def _find_all_module_objects(self): - data = self._data + data = self.raw_data modules = [] if "index" in data and type(data["index"]): modules += data["index"].values() @@ -105,7 +111,7 @@ def warn_about_unknown_keys(self): For the more complete validation, see validate.py. """ - data = self._data + data = self.raw_data for key in data: if key not in TOP_LEVEL_KEYS: log.warning( From fa1a66b7294c189a468b0cea481c0be708f930be Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Tue, 5 Dec 2023 18:44:23 +0100 Subject: [PATCH 17/64] Fixed issue with typechecking for index and provides fields Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/cfbs_json.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cfbs/cfbs_json.py b/cfbs/cfbs_json.py index f453d0b8..88fd7eb6 100644 --- a/cfbs/cfbs_json.py +++ b/cfbs/cfbs_json.py @@ -92,9 +92,9 @@ def raw_data(self): def _find_all_module_objects(self): data = self.raw_data modules = [] - if "index" in data and type(data["index"]): + if "index" in data and type(data["index"]) in (dict, OrderedDict): modules += data["index"].values() - if "provides" in data and type(data["index"]): + if "provides" in data and type(data["index"]) in (dict, OrderedDict): modules += data["provides"].values() if "build" in data and type(data["build"]): modules += data["build"] From ee8aac6b6da0c64d378689cdd5113a20ee1ef1ba Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Tue, 5 Dec 2023 18:44:52 +0100 Subject: [PATCH 18/64] Fixed checking of aliases Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/validate.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cfbs/validate.py b/cfbs/validate.py index 68657070..e670a8a4 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -174,7 +174,7 @@ def validate_url_field(name, module, field): # Step 1 - Handle special cases (alias): - if "alias" in modules[name]: + if "alias" in module: if mode in ("index", "provides"): validate_alias(name, module) return From 77a320a7007fb746ca40052daf400dcc6a054a72 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Tue, 5 Dec 2023 18:46:28 +0100 Subject: [PATCH 19/64] Made it possible to only pass message to CFBSValidationError Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/validate.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/cfbs/validate.py b/cfbs/validate.py index e670a8a4..6317f1ed 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -8,7 +8,13 @@ class CFBSValidationError(Exception): - def __init__(self, name, message) -> None: + def __init__(self, name_or_message, message=None) -> None: + assert name_or_message + if message: + name = name_or_message + else: + name = None + message = name_or_message if name is None: super().__init__("Error in index: " + message) else: From 9deb96073294950cb25d765ea0db9ea513105c13 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Tue, 5 Dec 2023 18:47:10 +0100 Subject: [PATCH 20/64] Updated validation log messages for being used more generally Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/validate.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cfbs/validate.py b/cfbs/validate.py index 6317f1ed..969eae57 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -16,9 +16,9 @@ def __init__(self, name_or_message, message=None) -> None: name = None message = name_or_message if name is None: - super().__init__("Error in index: " + message) + super().__init__("Error in cfbs.json: " + message) else: - super().__init__("Error in index for module '%s': " % name + message) + super().__init__("Error in cfbs.json for module '%s': " % name + message) def validate_config(config, build=False): From f656cfe1844a9162071479408c879465de01a7df Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Tue, 5 Dec 2023 18:50:06 +0100 Subject: [PATCH 21/64] Prepared CFBSValidationError to be used with an index instead of a name Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/validate.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cfbs/validate.py b/cfbs/validate.py index 969eae57..df5342a4 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -17,6 +17,8 @@ def __init__(self, name_or_message, message=None) -> None: message = name_or_message if name is None: super().__init__("Error in cfbs.json: " + message) + elif type(name) is int: + super().__init__("Error in cfbs.json for module at index %d: " % name + message) else: super().__init__("Error in cfbs.json for module '%s': " % name + message) From a36219c87225e322e543453d9a14c379a0eea77f Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Tue, 5 Dec 2023 18:53:41 +0100 Subject: [PATCH 22/64] Added validation for top level keys of cfbs.json Ticket: CFE-4060 Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/validate.py | 48 ++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 44 insertions(+), 4 deletions(-) diff --git a/cfbs/validate.py b/cfbs/validate.py index df5342a4..ba1af64b 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -2,8 +2,10 @@ import json import sys import re +from collections import OrderedDict from cfbs.utils import is_a_commit_hash, user_error +from cfbs.cfbs_json import TOP_LEVEL_KEYS from cfbs.cfbs_config import CFBSConfig @@ -22,15 +24,53 @@ def __init__(self, name_or_message, message=None) -> None: else: super().__init__("Error in cfbs.json for module '%s': " % name + message) +def _validate_top_level_keys(config): + # Convert the CFBSJson object to a simple dictionary with exactly + # what was in the file. We don't want CFBSJson / CFBSConfig to do any + # translations here: + config = config.raw_data + + # Check that required fields are there: + + required_fields = ["name", "type", "description"] + + for field in required_fields: + assert field in TOP_LEVEL_KEYS + if field not in config: + raise CFBSValidationError('The "%s" field is required in a cfbs.json file' % field) + + # Further check types / values of those required fields: + + if type(config["name"]) is not str or config["name"] == "": + raise CFBSValidationError('The "name" field must be a non-empty string') + if config["type"] not in ("policy-set", "index", "module"): + raise CFBSValidationError('The "type" field must be "policy-set", "index", or "module"') + if type(config["description"]) is not str: + raise CFBSValidationError('The "description" field must be a string') + + # Check types / values of other optional fields: + + if "git" in config and config["git"] not in (True, False): + raise CFBSValidationError('The "git" field must be true or false') + if "index" in config: + index = config["index"] + if type(index) not in (str, dict, OrderedDict): + raise CFBSValidationError('The "index" field must either be a URL / path (string) or an inline index (object / dictionary)') + if type(index) is str and index.strip() == "": + raise CFBSValidationError('The "index" string must be a URL / path (string), not "%s"' % index) + if type(index) is str and not index.endswith(".json"): + raise CFBSValidationError('The "index" string must refer to a JSON file / URL (ending in .json)') + if type(index) is str and not index.startswith(("https://", "./")): + raise CFBSValidationError('The "index" string must be a URL (starting with https://) or relative path (starting with ./)') + if type(index) is str and index.startswith("https://") and " " in index: + raise CFBSValidationError('The "index" URL must not contain spaces') def validate_config(config, build=False): - config.warn_about_unknown_keys() # First validate the config i.e. the user's cfbs.json + config.warn_about_unknown_keys() + _validate_top_level_keys(config) - # TODO: Add more validation for other things in config: - # missing keys, types of keys, accepted values, etc. - # https://northerntech.atlassian.net/browse/CFE-4060 if build: _validate_config_for_build_field(config) From fe5d7de1956c42c0f2d3a26d48c00ee7dd6835ce Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Tue, 5 Dec 2023 19:21:04 +0100 Subject: [PATCH 23/64] Removed a lot of the special handling for validation of index files We're now using the same functions for validating all cfbs.json files, including the index file. Ticket: CFE-4060 Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/validate.py | 40 ++++++++++++---------------------------- 1 file changed, 12 insertions(+), 28 deletions(-) diff --git a/cfbs/validate.py b/cfbs/validate.py index ba1af64b..69dd38df 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -39,6 +39,13 @@ def _validate_top_level_keys(config): if field not in config: raise CFBSValidationError('The "%s" field is required in a cfbs.json file' % field) + # Specific error checking for "index" type files: + + if config["type"] == "index" and "index" not in config: + raise CFBSValidationError('For a cfbs.json with "index" as type, put modules in the index by adding them to a "index" field' % field) + 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: if type(config["name"]) is not str or config["name"] == "": @@ -70,7 +77,7 @@ def validate_config(config, build=False): # First validate the config i.e. the user's cfbs.json config.warn_about_unknown_keys() _validate_top_level_keys(config) - + raw_data = config.raw_data if build: _validate_config_for_build_field(config) @@ -82,23 +89,12 @@ def validate_config(config, build=False): if "build" in config and config["build"] != []: _validate_config_for_build_field(config) - # Then resolve the index, and validate that: - index = config.index - if not index: - user_error("Index not found") - - data = index.data - if "type" not in data: - user_error("Index is missing a type field") + if "index" in raw_data and type(raw_data["index"]) in (dict, OrderedDict): + for name, module in raw_data["index"].items(): + _validate_module_object("index", name, module, raw_data["index"]) - if data["type"] != "index": - user_error("The loaded index has incorrect type: " + str(data["type"])) + # TODO: Add "provides" here - try: - _validate_index(data) - except CFBSValidationError as e: - print(e) - return 1 return 0 @@ -279,18 +275,6 @@ def validate_url_field(name, module, field): validate_url_field(name, module, "documentation") -def _validate_index(index): - # Make sure index has a collection named modules - if not "index" in index: - raise CFBSValidationError(None, "Missing required attribute 'index'") - modules = index["index"] - - # Validate each entry in modules - for name in modules: - module = modules[name] - _validate_module_object("index", name, module, modules) - - def _validate_config_for_build_field(config): """Validate that neccessary fields are in the config for the build/download commands to work""" if not "build" in config: From eb195579a2aa6f517db9cc3f21e25d04519ad23b Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Tue, 5 Dec 2023 19:22:23 +0100 Subject: [PATCH 24/64] Added "alias" as a known module key so it's recognized in validation Ticket: CFE-4060 Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/cfbs_json.py | 1 + 1 file changed, 1 insertion(+) diff --git a/cfbs/cfbs_json.py b/cfbs/cfbs_json.py index 88fd7eb6..1d6d9862 100644 --- a/cfbs/cfbs_json.py +++ b/cfbs/cfbs_json.py @@ -11,6 +11,7 @@ TOP_LEVEL_KEYS = ("name", "description", "type", "index", "git", "provides", "build") MODULE_KEYS = ( + "alias", "name", "description", "tags", From 48218aaaae894852fa9089c2f2e33be1db0aa247 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Tue, 5 Dec 2023 19:23:14 +0100 Subject: [PATCH 25/64] Fixed the warning message for unknown keys Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/cfbs_json.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cfbs/cfbs_json.py b/cfbs/cfbs_json.py index 1d6d9862..d584c7a7 100644 --- a/cfbs/cfbs_json.py +++ b/cfbs/cfbs_json.py @@ -124,7 +124,7 @@ def warn_about_unknown_keys(self): for key in module: if key not in MODULE_KEYS: log.warning( - 'The module level key "%s" is not known to this version of cfbs.\n' + 'The module level key "%s" is not known to this version of cfbs.\n' % key + "Is it a typo? If not, try upgrading cfbs:\n" + "pip3 install --upgrade cfbs" ) From 7011dc3685657a02f336203703105cd73a6b18e6 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Tue, 5 Dec 2023 19:24:06 +0100 Subject: [PATCH 26/64] Validation: Added checking to keep required_fields in sync with global lists of accepted fields Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/validate.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cfbs/validate.py b/cfbs/validate.py index 69dd38df..fa71cd65 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -5,7 +5,7 @@ from collections import OrderedDict from cfbs.utils import is_a_commit_hash, user_error -from cfbs.cfbs_json import TOP_LEVEL_KEYS +from cfbs.cfbs_json import TOP_LEVEL_KEYS, MODULE_KEYS from cfbs.cfbs_config import CFBSConfig @@ -244,6 +244,7 @@ def validate_url_field(name, module, field): required_fields.append("commit") for required_field in required_fields: + assert required_field in MODULE_KEYS if required_field not in module: raise CFBSValidationError(name, '"%s" field is required, but missing') From d5e4659080ff449cb4b46fd0ed98e05f0e5c8c6b Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Tue, 5 Dec 2023 19:25:16 +0100 Subject: [PATCH 27/64] Put the index in place of module name when module name is missing Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/validate.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cfbs/validate.py b/cfbs/validate.py index fa71cd65..fc39e358 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -292,7 +292,7 @@ def _validate_config_for_build_field(config): "The \"build\" field in ./cfbs.json is empty - add modules with 'cfbs add'" ) for index, module in enumerate(config["build"]): - name = module["name"] + name = module["name"] if "name" in module else index _validate_module_object("build", name, module, config.index.data["index"]) From d532da948cd57bf8704365061b23e735c30881ba Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Tue, 5 Dec 2023 19:54:36 +0100 Subject: [PATCH 28/64] Improved validation error messages by catching the exception Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/validate.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/cfbs/validate.py b/cfbs/validate.py index fc39e358..c392f04b 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -72,7 +72,7 @@ def _validate_top_level_keys(config): if type(index) is str and index.startswith("https://") and " " in index: raise CFBSValidationError('The "index" URL must not contain spaces') -def validate_config(config, build=False): +def _validate_config(config, build=False): # First validate the config i.e. the user's cfbs.json config.warn_about_unknown_keys() @@ -95,8 +95,13 @@ def validate_config(config, build=False): # TODO: Add "provides" here - return 0 - +def validate_config(config, build=False): + try: + _validate_config(config, build) + return 0 + except CFBSValidationError as e: + print(e) + return 1 def _validate_module_object(mode, name, module, modules): def validate_alias(name, module): @@ -302,9 +307,9 @@ def main(): args = parser.parse_args() config = CFBSConfig.get_instance(filename=args.file, non_interactive=True) - validate_config(config) + r = validate_config(config) - sys.exit(0) + sys.exit(r) if __name__ == "__main__": From 165437c3c39bfdcfc0e486f9a97736aa10b4aae1 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Thu, 7 Dec 2023 16:23:03 +0100 Subject: [PATCH 29/64] Fixed validation error message for missing index Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/validate.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cfbs/validate.py b/cfbs/validate.py index c392f04b..f0b26d9c 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -42,7 +42,7 @@ def _validate_top_level_keys(config): # Specific error checking for "index" type files: if config["type"] == "index" and "index" not in config: - raise CFBSValidationError('For a cfbs.json with "index" as type, put modules in the index by adding them to a "index" field' % field) + raise CFBSValidationError('For a cfbs.json with "index" as type, put modules in the index by adding them to a "index" field') 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) From 02c03340e0bea0b7766e53879d5586ec90667661 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Thu, 7 Dec 2023 16:23:29 +0100 Subject: [PATCH 30/64] Fixed validation error message for a required field missing inside modules Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/validate.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cfbs/validate.py b/cfbs/validate.py index f0b26d9c..ecaf8463 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -251,7 +251,7 @@ def validate_url_field(name, module, field): for required_field in required_fields: assert required_field in MODULE_KEYS if required_field not in module: - raise CFBSValidationError(name, '"%s" field is required, but missing') + raise CFBSValidationError(name, '"%s" field is required, but missing' % required_field) # Step 3 - Validate fields: From 4613db3f7973ea1ff17113b3d7dadcd240c87a31 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Thu, 7 Dec 2023 17:44:28 +0100 Subject: [PATCH 31/64] Added more thorough validation of build steps Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/build.py | 13 +++++++++++++ cfbs/validate.py | 27 +++++++++++++++++++++++++-- 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/cfbs/build.py b/cfbs/build.py index c2d9f405..e964c1b4 100644 --- a/cfbs/build.py +++ b/cfbs/build.py @@ -17,6 +17,18 @@ ) from cfbs.pretty import pretty, pretty_file +AVAILABLE_BUILD_STEPS = { + "copy": 2, + "run": 1, + "delete": 2, + "json": 2, + "append": 2, + "directory": 2, + "input": 2, + "policy_files": "1+", + "bundles": "1+", +} + def init_out_folder(): rm("out", missing_ok=True) @@ -67,6 +79,7 @@ def _perform_build_step(module, step, max_length): prefix = "%03d %s :" % (counter, pad_right(module["name"], max_length)) + assert operation in AVAILABLE_BUILD_STEPS # Should already be validated if operation == "copy": src, dst = args if dst in [".", "./"]: diff --git a/cfbs/validate.py b/cfbs/validate.py index ecaf8463..34358052 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -7,6 +7,7 @@ from cfbs.utils import is_a_commit_hash, user_error from cfbs.cfbs_json import TOP_LEVEL_KEYS, MODULE_KEYS from cfbs.cfbs_config import CFBSConfig +from cfbs.build import AVAILABLE_BUILD_STEPS class CFBSValidationError(Exception): @@ -208,10 +209,32 @@ def validate_steps(name, module): for step in module["steps"]: if type(step) != str: raise CFBSValidationError(name, '"steps" must be a list of strings') - if not step: + if not step or step.strip() == "": raise CFBSValidationError( - name, '"steps" must be a list of non-empty strings' + name, '"steps" must be a list of non-empty / non-whitespace strings' ) + step_array = step.split(" ") + operation, args = step_array[0], step_array[1:] + if not operation in AVAILABLE_BUILD_STEPS: + x = ", ".join(AVAILABLE_BUILD_STEPS) + raise CFBSValidationError( + name, 'Unknown operation in "steps", must be one of: (%s)' % x + ) + expected = AVAILABLE_BUILD_STEPS[operation] + actual = len(args) + if type(expected) is int: + if expected != actual: + raise CFBSValidationError( + name, 'The %s build step expects %d arguents, %d were given' % (operation, expected, actual) + ) + else: + # Only other option is a string of 1+, 2+ or similar: + assert type(expected) is str and expected.endswith("+") + expected = int(expected[0:-1]) + if actual < expected: + raise CFBSValidationError( + name, 'The %s build step expects %d or more arguents, %d were given' % (operation, expected, actual) + ) def validate_url_field(name, module, field): assert field in module From 471ea24c330e88f440c5af379652c0bcd55d8902 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Thu, 7 Dec 2023 17:45:48 +0100 Subject: [PATCH 32/64] Added more thorough validation of subdirectory Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/validate.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/cfbs/validate.py b/cfbs/validate.py index 34358052..1a79a853 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -199,6 +199,16 @@ def validate_subdirectory(name, module): raise CFBSValidationError(name, '"subdirectory" must be of type string') if not module["subdirectory"]: raise CFBSValidationError(name, '"subdirectory" must be non-empty') + if module["subdirectory"].startswith("./"): + raise CFBSValidationError(name, '"subdirectory" must not start with ./') + if module["subdirectory"].startswith("/"): + raise CFBSValidationError( + name, '"subdirectory" must be a relative path, not starting with /' + ) + if " " in module["subdirectory"]: + raise CFBSValidationError(name, '"subdirectory" cannot contain spaces') + if module["subdirectory"].endswith(("/", "/.")): + raise CFBSValidationError(name, '"subdirectory" must not end with / or /.') def validate_steps(name, module): assert "steps" in module From 6dd1cb305440e8ab17acd00e58c9fbc11b71580a Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Thu, 7 Dec 2023 17:46:39 +0100 Subject: [PATCH 33/64] Reformatted files with black Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/cfbs_json.py | 4 ++-- cfbs/validate.py | 54 +++++++++++++++++++++++++++++++++++------------ 2 files changed, 43 insertions(+), 15 deletions(-) diff --git a/cfbs/cfbs_json.py b/cfbs/cfbs_json.py index d584c7a7..728b59ee 100644 --- a/cfbs/cfbs_json.py +++ b/cfbs/cfbs_json.py @@ -101,7 +101,6 @@ def _find_all_module_objects(self): modules += data["build"] return modules - def warn_about_unknown_keys(self): """Basic validation to warn the user when a cfbs.json has unknown keys. @@ -124,7 +123,8 @@ def warn_about_unknown_keys(self): for key in module: if key not in MODULE_KEYS: log.warning( - 'The module level key "%s" is not known to this version of cfbs.\n' % key + 'The module level key "%s" is not known to this version of cfbs.\n' + % key + "Is it a typo? If not, try upgrading cfbs:\n" + "pip3 install --upgrade cfbs" ) diff --git a/cfbs/validate.py b/cfbs/validate.py index 1a79a853..bd29a150 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -21,10 +21,13 @@ def __init__(self, name_or_message, message=None) -> None: if name is None: super().__init__("Error in cfbs.json: " + message) elif type(name) is int: - super().__init__("Error in cfbs.json for module at index %d: " % name + message) + super().__init__( + "Error in cfbs.json for module at index %d: " % name + message + ) else: super().__init__("Error in cfbs.json for module '%s': " % name + message) + def _validate_top_level_keys(config): # Convert the CFBSJson object to a simple dictionary with exactly # what was in the file. We don't want CFBSJson / CFBSConfig to do any @@ -38,21 +41,30 @@ def _validate_top_level_keys(config): for field in required_fields: assert field in TOP_LEVEL_KEYS if field not in config: - raise CFBSValidationError('The "%s" field is required in a cfbs.json file' % field) + raise CFBSValidationError( + 'The "%s" field is required in a cfbs.json file' % field + ) # Specific error checking for "index" type files: if config["type"] == "index" and "index" not in config: - raise CFBSValidationError('For a cfbs.json with "index" as type, put modules in the index by adding them to a "index" field') + raise CFBSValidationError( + 'For a cfbs.json with "index" as type, put modules in the index by adding them to a "index" field' + ) 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) + 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: if type(config["name"]) is not str or config["name"] == "": raise CFBSValidationError('The "name" field must be a non-empty string') if config["type"] not in ("policy-set", "index", "module"): - raise CFBSValidationError('The "type" field must be "policy-set", "index", or "module"') + raise CFBSValidationError( + 'The "type" field must be "policy-set", "index", or "module"' + ) if type(config["description"]) is not str: raise CFBSValidationError('The "description" field must be a string') @@ -63,18 +75,26 @@ def _validate_top_level_keys(config): if "index" in config: index = config["index"] if type(index) not in (str, dict, OrderedDict): - raise CFBSValidationError('The "index" field must either be a URL / path (string) or an inline index (object / dictionary)') + raise CFBSValidationError( + 'The "index" field must either be a URL / path (string) or an inline index (object / dictionary)' + ) if type(index) is str and index.strip() == "": - raise CFBSValidationError('The "index" string must be a URL / path (string), not "%s"' % index) + raise CFBSValidationError( + 'The "index" string must be a URL / path (string), not "%s"' % index + ) if type(index) is str and not index.endswith(".json"): - raise CFBSValidationError('The "index" string must refer to a JSON file / URL (ending in .json)') + raise CFBSValidationError( + 'The "index" string must refer to a JSON file / URL (ending in .json)' + ) if type(index) is str and not index.startswith(("https://", "./")): - raise CFBSValidationError('The "index" string must be a URL (starting with https://) or relative path (starting with ./)') + raise CFBSValidationError( + 'The "index" string must be a URL (starting with https://) or relative path (starting with ./)' + ) if type(index) is str and index.startswith("https://") and " " in index: raise CFBSValidationError('The "index" URL must not contain spaces') -def _validate_config(config, build=False): +def _validate_config(config, build=False): # First validate the config i.e. the user's cfbs.json config.warn_about_unknown_keys() _validate_top_level_keys(config) @@ -96,6 +116,7 @@ def _validate_config(config, build=False): # TODO: Add "provides" here + def validate_config(config, build=False): try: _validate_config(config, build) @@ -104,6 +125,7 @@ def validate_config(config, build=False): print(e) return 1 + def _validate_module_object(mode, name, module, modules): def validate_alias(name, module): assert "alias" in module @@ -235,7 +257,9 @@ def validate_steps(name, module): if type(expected) is int: if expected != actual: raise CFBSValidationError( - name, 'The %s build step expects %d arguents, %d were given' % (operation, expected, actual) + name, + "The %s build step expects %d arguents, %d were given" + % (operation, expected, actual), ) else: # Only other option is a string of 1+, 2+ or similar: @@ -243,7 +267,9 @@ def validate_steps(name, module): expected = int(expected[0:-1]) if actual < expected: raise CFBSValidationError( - name, 'The %s build step expects %d or more arguents, %d were given' % (operation, expected, actual) + name, + "The %s build step expects %d or more arguents, %d were given" + % (operation, expected, actual), ) def validate_url_field(name, module, field): @@ -284,7 +310,9 @@ def validate_url_field(name, module, field): for required_field in required_fields: assert required_field in MODULE_KEYS if required_field not in module: - raise CFBSValidationError(name, '"%s" field is required, but missing' % required_field) + raise CFBSValidationError( + name, '"%s" field is required, but missing' % required_field + ) # Step 3 - Validate fields: From 447c47826dbc9d2a01c5ffbbc86cbf3f7983a014 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Thu, 7 Dec 2023 17:47:29 +0100 Subject: [PATCH 34/64] Added a unit test for validating a simple index and catching many of the errors we look for Signed-off-by: Ole Herman Schumacher Elgesem --- tests/test_validate_mock_index.py | 196 ++++++++++++++++++++++++++++++ 1 file changed, 196 insertions(+) create mode 100644 tests/test_validate_mock_index.py diff --git a/tests/test_validate_mock_index.py b/tests/test_validate_mock_index.py new file mode 100644 index 00000000..ae8aeedf --- /dev/null +++ b/tests/test_validate_mock_index.py @@ -0,0 +1,196 @@ +from collections import OrderedDict +from copy import deepcopy + +from cfbs.validate import validate_config + + +# Only 2 interactions between validate_config and the config object: +# 1. Run warn_about_unknown_keys() +# 2. Run raw_data to get just the data for validation +class MockConfig(OrderedDict): + @property + def raw_data(self): + return deepcopy(self) + + def warn_about_unknown_keys(self): + pass + + +INDEX = { + "name": "index", + "description": "An example index of 1 module", + "type": "index", + "index": { + "allow-all-hosts": { + "description": "Allows all hosts / IP addresses to connect and fetch policy.", + "tags": ["management", "experimental"], + "repo": "https://github.com/cfengine/modules", + "by": "https://github.com/olehermanse", + "version": "0.0.1", + "commit": "85f9aec38783b5a4dac4777ffa9d17fde5054d14", + "subdirectory": "management/allow-all-hosts", + "steps": ["json def.json def.json"], + } + }, +} + + +def test_validate_mock_index(): + config = MockConfig(deepcopy(INDEX)) + assert validate_config(config) == 0 + + # Deleting top level keys: + + config = MockConfig(deepcopy(INDEX)) + del config["name"] + assert validate_config(config) == 1 + + config = MockConfig(deepcopy(INDEX)) + del config["description"] + assert validate_config(config) == 1 + + config = MockConfig(deepcopy(INDEX)) + del config["type"] + assert validate_config(config) == 1 + + config = MockConfig(deepcopy(INDEX)) + del config["index"] + assert validate_config(config) == 1 + + # Deleting parts of the module: + + config = MockConfig(deepcopy(INDEX)) + del config["index"]["allow-all-hosts"]["description"] + assert validate_config(config) == 1 + + config = MockConfig(deepcopy(INDEX)) + del config["index"]["allow-all-hosts"]["tags"] + assert validate_config(config) == 1 + + config = MockConfig(deepcopy(INDEX)) + del config["index"]["allow-all-hosts"]["repo"] + assert validate_config(config) == 1 + + config = MockConfig(deepcopy(INDEX)) + del config["index"]["allow-all-hosts"]["by"] + assert validate_config(config) == 1 + + config = MockConfig(deepcopy(INDEX)) + del config["index"]["allow-all-hosts"]["version"] + assert validate_config(config) == 1 + + config = MockConfig(deepcopy(INDEX)) + del config["index"]["allow-all-hosts"]["commit"] + assert validate_config(config) == 1 + + config = MockConfig(deepcopy(INDEX)) + del config["index"]["allow-all-hosts"]["subdirectory"] + assert validate_config(config) == 0 # Optional field + + config = MockConfig(deepcopy(INDEX)) + del config["index"]["allow-all-hosts"]["steps"] + assert validate_config(config) == 1 + + # Mess with some of the values: + + config = MockConfig(deepcopy(INDEX)) + config["index"]["allow-all-hosts"]["description"] = None + assert validate_config(config) == 1 + + config = MockConfig(deepcopy(INDEX)) + config["index"]["allow-all-hosts"]["tags"] = {} + assert validate_config(config) == 1 + + config = MockConfig(deepcopy(INDEX)) + config["index"]["allow-all-hosts"]["repo"] = None + assert validate_config(config) == 1 + config["index"]["allow-all-hosts"]["repo"] = "" + assert validate_config(config) == 1 + + config = MockConfig(deepcopy(INDEX)) + config["index"]["allow-all-hosts"]["by"] = None + assert validate_config(config) == 1 + config["index"]["allow-all-hosts"]["by"] = "" + assert validate_config(config) == 1 + + config = MockConfig(deepcopy(INDEX)) + before = config["index"]["allow-all-hosts"]["version"] + config["index"]["allow-all-hosts"]["version"] = None + assert validate_config(config) == 1 + config["index"]["allow-all-hosts"]["version"] = "" + assert validate_config(config) == 1 + config["index"]["allow-all-hosts"]["version"] = "blah" + assert validate_config(config) == 1 + config["index"]["allow-all-hosts"]["version"] = "1.2" + assert validate_config(config) == 1 + config["index"]["allow-all-hosts"]["version"] = "1.2." + assert validate_config(config) == 1 + config["index"]["allow-all-hosts"]["version"] = "1.2.3.4" + assert validate_config(config) == 1 + config["index"]["allow-all-hosts"]["version"] = before + assert validate_config(config) == 0 + + config = MockConfig(deepcopy(INDEX)) + before = config["index"]["allow-all-hosts"]["commit"] + config["index"]["allow-all-hosts"]["commit"] = None + assert validate_config(config) == 1 + config["index"]["allow-all-hosts"]["commit"] = "" + assert validate_config(config) == 1 + config["index"]["allow-all-hosts"]["commit"] = "abcd" + assert validate_config(config) == 1 + config["index"]["allow-all-hosts"]["commit"] = "1234" + assert validate_config(config) == 1 + config["index"]["allow-all-hosts"]["commit"] = "1.2.3" + assert validate_config(config) == 1 + config["index"]["allow-all-hosts"]["commit"] = before + assert validate_config(config) == 0 + + config = MockConfig(deepcopy(INDEX)) + before = config["index"]["allow-all-hosts"]["subdirectory"] + config["index"]["allow-all-hosts"]["subdirectory"] = None + assert validate_config(config) == 1 + config["index"]["allow-all-hosts"]["subdirectory"] = "" + assert validate_config(config) == 1 + config["index"]["allow-all-hosts"]["subdirectory"] = " " + assert validate_config(config) == 1 + config["index"]["allow-all-hosts"]["subdirectory"] = before + "/" + assert validate_config(config) == 1 + config["index"]["allow-all-hosts"]["subdirectory"] = "./" + assert validate_config(config) == 1 + config["index"]["allow-all-hosts"]["subdirectory"] = "./" + before + assert validate_config(config) == 1 + config["index"]["allow-all-hosts"]["subdirectory"] = before + assert validate_config(config) == 0 + + config = MockConfig(deepcopy(INDEX)) + before = config["index"]["allow-all-hosts"]["steps"] + config["index"]["allow-all-hosts"]["steps"] = None + assert validate_config(config) == 1 + config["index"]["allow-all-hosts"]["steps"] = "" + assert validate_config(config) == 1 + config["index"]["allow-all-hosts"]["steps"] = [] + assert validate_config(config) == 1 + config["index"]["allow-all-hosts"]["steps"] = [""] + assert validate_config(config) == 1 + config["index"]["allow-all-hosts"]["steps"] = [" "] + assert validate_config(config) == 1 + config["index"]["allow-all-hosts"]["steps"] = ["", " ", "\n"] + assert validate_config(config) == 1 + config["index"]["allow-all-hosts"]["steps"] = ["copy"] # Too few args + assert validate_config(config) == 1 + config["index"]["allow-all-hosts"]["steps"] = ["copy a"] # Too few args + assert validate_config(config) == 1 + config["index"]["allow-all-hosts"]["steps"] = ["copy a b"] # Correct + assert validate_config(config) == 0 + config["index"]["allow-all-hosts"]["steps"] = ["copy abc def"] # Correct + assert validate_config(config) == 0 + config["index"]["allow-all-hosts"]["steps"] = ["run"] # Too few args + assert validate_config(config) == 1 + config["index"]["allow-all-hosts"]["steps"] = ["run script.sh"] # Correct + assert validate_config(config) == 0 + config["index"]["allow-all-hosts"]["steps"] = ["copy", "blah", "blah"] + assert validate_config(config) == 1 + config["index"]["allow-all-hosts"]["steps"] = ["blah blah blah"] + assert validate_config(config) == 1 + config["index"]["allow-all-hosts"]["steps"] = before + assert validate_config(config) == 0 From bfba0dc6559d8c95f1fb752a146b306c2db050bb Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Thu, 7 Dec 2023 19:29:20 +0100 Subject: [PATCH 35/64] Added validation for module input information in cfbs.json Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/validate.py | 104 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 104 insertions(+) diff --git a/cfbs/validate.py b/cfbs/validate.py index bd29a150..3d6e8ff1 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -278,6 +278,108 @@ def validate_url_field(name, module, field): if url and not url.startswith("https://"): raise CFBSValidationError(name, '"%" must be an HTTPS URL' % field) + def validate_module_input(name, module): + assert "input" in module + if type(module["input"]) is not list or not module["input"]: + raise CFBSValidationError( + name, 'The module\'s "input" must be a non-empty array' + ) + + required_string_fields = ["type", "variable", "namespace", "bundle", "label"] + + required_string_fields_subtype = ["key", "type", "label", "question"] + + for input_element in module["input"]: + if type(input_element) not in (dict, OrderedDict) or not input_element: + raise CFBSValidationError( + name, + 'The module\'s "input" array must consist of non-empty objects (dictionaries)', + ) + for field in required_string_fields: + if field not in input_element: + raise CFBSValidationError( + name, + 'The "%s" field is required in module input elements' % field, + ) + if ( + type(input_element[field]) is not str + or input_element[field].strip() == "" + ): + raise CFBSValidationError( + name, + 'The "%s" field in input elements must be a non-empty / non-whitespace string' + % field, + ) + + if input_element["type"] not in ("string", "list"): + raise CFBSValidationError( + name, + 'The input "type" must be "string" or "list", not "%s"' + % input_element["type"], + ) + if not re.fullmatch(r"[a-z_]+", input_element["variable"]): + raise CFBSValidationError( + name, + '"%s" is not an acceptable variable name' + % input_element["variable"], + ) + if not re.fullmatch(r"[a-z_]+", input_element["namespace"]): + raise CFBSValidationError( + name, + '"%s" is not an acceptable namespace' % input_element["namespace"], + ) + if not re.fullmatch(r"[a-z_]+", input_element["bundle"]): + raise CFBSValidationError( + name, + '"%s" is not an acceptable bundle name' % input_element["bundle"], + ) + + if input_element["type"] == "list": + if not "while" in input_element: + raise CFBSValidationError( + name, 'For a "list" input element, a "while" prompt is required' + ) + if ( + type(input_element["while"]) is not str + or not input_element["while"].strip() + ): + raise CFBSValidationError( + name, + 'The "while" prompt in an input "list" element must be a non-empty / non-whitespace string', + ) + if not "subtype" in input_element: + raise CFBSValidationError( + name, 'For a "list" input element, a "subtype" is required' + ) + if type(input_element["subtype"]) not in (list, dict, OrderedDict): + raise CFBSValidationError( + name, + 'The list element "subtype" must be an object or an array of objects (dictionaries)', + ) + subtype = input_element["subtype"] + if type(subtype) is not list: + subtype = [subtype] + for part in subtype: + for field in required_string_fields_subtype: + if field not in part: + raise CFBSValidationError( + name, + 'The "%s" field is required in module input "subtype" objects' + % field, + ) + if type(part[field]) is not str or part[field].strip() == "": + raise CFBSValidationError( + name, + 'The "%s" field in module input "subtype" objects must be a non-empty / non-whitespace string' + % field, + ) + if part["type"] != "string": + raise CFBSValidationError( + name, + 'Only "string" supported for the "type" of module input list elements, not "%s"' + % part["type"], + ) + assert mode in ("index", "provides", "build") # Step 1 - Handle special cases (alias): @@ -340,6 +442,8 @@ def validate_url_field(name, module, field): validate_url_field(name, module, "website") if "documentation" in module: validate_url_field(name, module, "documentation") + if "input" in module: + validate_module_input(name, module) def _validate_config_for_build_field(config): From 6906453a7f40139d0f6d461a832d67881cfc7ec9 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Thu, 7 Dec 2023 19:30:27 +0100 Subject: [PATCH 36/64] Added unit test for validation of module input inside cfbs.json Signed-off-by: Ole Herman Schumacher Elgesem --- tests/test_validate_mock_input.py | 269 ++++++++++++++++++++++++++++++ 1 file changed, 269 insertions(+) create mode 100644 tests/test_validate_mock_input.py diff --git a/tests/test_validate_mock_input.py b/tests/test_validate_mock_input.py new file mode 100644 index 00000000..80524356 --- /dev/null +++ b/tests/test_validate_mock_input.py @@ -0,0 +1,269 @@ +from collections import OrderedDict +from copy import deepcopy + +from cfbs.validate import validate_config + + +# Only 2 interactions between validate_config and the config object: +# 1. Run warn_about_unknown_keys() +# 2. Run raw_data to get just the data for validation +class MockConfig(OrderedDict): + @property + def raw_data(self): + return deepcopy(self) + + def warn_about_unknown_keys(self): + pass + + +INDEX = { + "name": "index", + "description": "An example index of 1 module", + "type": "index", + "index": { + "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.", + "tags": ["supported", "management"], + "repo": "https://github.com/nickanderson/cfengine-delete-files", + "by": "https://github.com/nickanderson", + "version": "2.0.0", + "commit": "84cce7c5653b6a5f2b5a28ebb33c697ffc676dd4", + "steps": [ + "copy delete-files.cf services/cfbs/modules/delete-files/delete-files.cf", + "input delete-files/input.json def.json", + "bundles delete_files:delete_files", + "policy_files services/cfbs/modules/delete-files/delete-files.cf", + ], + "input": [ + { + "type": "list", + "variable": "files", + "namespace": "delete_files", + "bundle": "delete_files", + "label": "Files", + "subtype": [ + { + "key": "path", + "type": "string", + "label": "Path", + "question": "Path to file", + }, + { + "key": "why", + "type": "string", + "label": "Why", + "question": "Why should this file be deleted?", + "default": "Unknown", + }, + ], + "while": "Specify another file you want deleted on your hosts?", + } + ], + } + }, +} + + +def test_validate_mock_input_delete_fields(): + config = MockConfig(deepcopy(INDEX)) + assert validate_config(config) == 0 + + config = MockConfig(deepcopy(INDEX)) + del config["index"]["delete-files"]["input"][0]["type"] + assert validate_config(config) == 1 + + config = MockConfig(deepcopy(INDEX)) + del config["index"]["delete-files"]["input"][0]["variable"] + assert validate_config(config) == 1 + + config = MockConfig(deepcopy(INDEX)) + del config["index"]["delete-files"]["input"][0]["namespace"] + assert validate_config(config) == 1 + + config = MockConfig(deepcopy(INDEX)) + del config["index"]["delete-files"]["input"][0]["bundle"] + assert validate_config(config) == 1 + + config = MockConfig(deepcopy(INDEX)) + del config["index"]["delete-files"]["input"][0]["label"] + assert validate_config(config) == 1 + + config = MockConfig(deepcopy(INDEX)) + del config["index"]["delete-files"]["input"][0]["while"] + assert validate_config(config) == 1 + + config = MockConfig(deepcopy(INDEX)) + del config["index"]["delete-files"]["input"][0]["subtype"] + assert validate_config(config) == 1 + + config = MockConfig(deepcopy(INDEX)) + del config["index"]["delete-files"]["input"][0]["subtype"][0]["key"] + assert validate_config(config) == 1 + + config = MockConfig(deepcopy(INDEX)) + del config["index"]["delete-files"]["input"][0]["subtype"][0]["type"] + assert validate_config(config) == 1 + + config = MockConfig(deepcopy(INDEX)) + del config["index"]["delete-files"]["input"][0]["subtype"][0]["label"] + assert validate_config(config) == 1 + + config = MockConfig(deepcopy(INDEX)) + del config["index"]["delete-files"]["input"][0]["subtype"][0]["question"] + assert validate_config(config) == 1 + + config = MockConfig(deepcopy(INDEX)) + del config["index"]["delete-files"]["input"][0]["subtype"][1]["key"] + assert validate_config(config) == 1 + + config = MockConfig(deepcopy(INDEX)) + del config["index"]["delete-files"]["input"][0]["subtype"][1]["type"] + assert validate_config(config) == 1 + + config = MockConfig(deepcopy(INDEX)) + del config["index"]["delete-files"]["input"][0]["subtype"][1]["label"] + assert validate_config(config) == 1 + + config = MockConfig(deepcopy(INDEX)) + del config["index"]["delete-files"]["input"][0]["subtype"][1]["question"] + assert validate_config(config) == 1 + + +def test_validate_mock_input_edit_fields(): + config = MockConfig(deepcopy(INDEX)) + assert validate_config(config) == 0 + + config = MockConfig(deepcopy(INDEX)) + before = config["index"]["delete-files"]["input"][0]["type"] + config["index"]["delete-files"]["input"][0]["type"] = None + assert validate_config(config) == 1 + config["index"]["delete-files"]["input"][0]["type"] = [] + assert validate_config(config) == 1 + config["index"]["delete-files"]["input"][0]["type"] = "" + assert validate_config(config) == 1 + config["index"]["delete-files"]["input"][0]["type"] = " " + assert validate_config(config) == 1 + config["index"]["delete-files"]["input"][0]["type"] = "blah" + assert validate_config(config) == 1 + config["index"]["delete-files"]["input"][0]["type"] = before + assert validate_config(config) == 0 + + config = MockConfig(deepcopy(INDEX)) + before = config["index"]["delete-files"]["input"][0]["variable"] + config["index"]["delete-files"]["input"][0]["variable"] = None + assert validate_config(config) == 1 + config["index"]["delete-files"]["input"][0]["variable"] = [] + assert validate_config(config) == 1 + config["index"]["delete-files"]["input"][0]["variable"] = "" + assert validate_config(config) == 1 + config["index"]["delete-files"]["input"][0]["variable"] = " " + assert validate_config(config) == 1 + config["index"]["delete-files"]["input"][0]["variable"] = "blah_variable_name" + assert validate_config(config) == 0 + config["index"]["delete-files"]["input"][0]["variable"] = "1234" + assert validate_config(config) == 1 + config["index"]["delete-files"]["input"][0]["variable"] = before + assert validate_config(config) == 0 + + config = MockConfig(deepcopy(INDEX)) + before = config["index"]["delete-files"]["input"][0]["namespace"] + config["index"]["delete-files"]["input"][0]["namespace"] = None + assert validate_config(config) == 1 + config["index"]["delete-files"]["input"][0]["namespace"] = [] + assert validate_config(config) == 1 + config["index"]["delete-files"]["input"][0]["namespace"] = "" + assert validate_config(config) == 1 + config["index"]["delete-files"]["input"][0]["namespace"] = " " + assert validate_config(config) == 1 + config["index"]["delete-files"]["input"][0]["namespace"] = "blah_namespace" + assert validate_config(config) == 0 + config["index"]["delete-files"]["input"][0]["namespace"] = "1234" + assert validate_config(config) == 1 + config["index"]["delete-files"]["input"][0]["namespace"] = before + assert validate_config(config) == 0 + + config = MockConfig(deepcopy(INDEX)) + before = config["index"]["delete-files"]["input"][0]["bundle"] + config["index"]["delete-files"]["input"][0]["bundle"] = None + assert validate_config(config) == 1 + config["index"]["delete-files"]["input"][0]["bundle"] = [] + assert validate_config(config) == 1 + config["index"]["delete-files"]["input"][0]["bundle"] = "" + assert validate_config(config) == 1 + config["index"]["delete-files"]["input"][0]["bundle"] = " " + assert validate_config(config) == 1 + config["index"]["delete-files"]["input"][0]["bundle"] = "blah_bundle" + assert validate_config(config) == 0 + config["index"]["delete-files"]["input"][0]["bundle"] = "1234" + assert validate_config(config) == 1 + config["index"]["delete-files"]["input"][0]["bundle"] = before + assert validate_config(config) == 0 + + config = MockConfig(deepcopy(INDEX)) + before = config["index"]["delete-files"]["input"][0]["label"] + config["index"]["delete-files"]["input"][0]["label"] = None + assert validate_config(config) == 1 + config["index"]["delete-files"]["input"][0]["label"] = [] + assert validate_config(config) == 1 + config["index"]["delete-files"]["input"][0]["label"] = "" + assert validate_config(config) == 1 + config["index"]["delete-files"]["input"][0]["label"] = " " + assert validate_config(config) == 1 + config["index"]["delete-files"]["input"][0]["label"] = "A label" + assert validate_config(config) == 0 + config["index"]["delete-files"]["input"][0]["label"] = before + assert validate_config(config) == 0 + + config = MockConfig(deepcopy(INDEX)) + before = config["index"]["delete-files"]["input"][0]["subtype"][0]["key"] + config["index"]["delete-files"]["input"][0]["subtype"][0]["key"] = None + assert validate_config(config) == 1 + config["index"]["delete-files"]["input"][0]["subtype"][0]["key"] = "" + assert validate_config(config) == 1 + config["index"]["delete-files"]["input"][0]["subtype"][0]["key"] = [] + assert validate_config(config) == 1 + config["index"]["delete-files"]["input"][0]["subtype"][0]["key"] = before + assert validate_config(config) == 0 + + config = MockConfig(deepcopy(INDEX)) + before = config["index"]["delete-files"]["input"][0]["subtype"][0]["type"] + config["index"]["delete-files"]["input"][0]["subtype"][0]["type"] = None + assert validate_config(config) == 1 + config["index"]["delete-files"]["input"][0]["subtype"][0]["type"] = "" + assert validate_config(config) == 1 + config["index"]["delete-files"]["input"][0]["subtype"][0]["type"] = "blah" + assert validate_config(config) == 1 + config["index"]["delete-files"]["input"][0]["subtype"][0]["type"] = [] + assert validate_config(config) == 1 + config["index"]["delete-files"]["input"][0]["subtype"][0]["type"] = before + assert validate_config(config) == 0 + + config = MockConfig(deepcopy(INDEX)) + before = config["index"]["delete-files"]["input"][0]["subtype"][0]["type"] + config["index"]["delete-files"]["input"][0]["subtype"][0]["type"] = None + assert validate_config(config) == 1 + config["index"]["delete-files"]["input"][0]["subtype"][0]["type"] = "" + assert validate_config(config) == 1 + config["index"]["delete-files"]["input"][0]["subtype"][0]["type"] = "string" + assert validate_config(config) == 0 + config["index"]["delete-files"]["input"][0]["subtype"][0]["type"] = "list" + assert validate_config(config) == 1 + config["index"]["delete-files"]["input"][0]["subtype"][0]["type"] = [] + assert validate_config(config) == 1 + config["index"]["delete-files"]["input"][0]["subtype"][0]["type"] = before + assert validate_config(config) == 0 + + config = MockConfig(deepcopy(INDEX)) + before = config["index"]["delete-files"]["input"][0]["subtype"][0]["question"] + config["index"]["delete-files"]["input"][0]["subtype"][0]["question"] = None + assert validate_config(config) == 1 + config["index"]["delete-files"]["input"][0]["subtype"][0]["question"] = "" + assert validate_config(config) == 1 + config["index"]["delete-files"]["input"][0]["subtype"][0][ + "question" + ] = "A question?" + assert validate_config(config) == 0 + config["index"]["delete-files"]["input"][0]["subtype"][0]["question"] = [] + assert validate_config(config) == 1 + config["index"]["delete-files"]["input"][0]["subtype"][0]["question"] = before + assert validate_config(config) == 0 From 0827f129aeb9e0703a21849eddd58eb6ed34bbe4 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Thu, 7 Dec 2023 19:44:35 +0100 Subject: [PATCH 37/64] Fixed exception name for alias not supported error message Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/validate.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cfbs/validate.py b/cfbs/validate.py index 3d6e8ff1..6ba42640 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -390,7 +390,7 @@ def validate_module_input(name, module): return else: assert mode == "build" - raise ValidationError(name, '"alias" is not supported in "build"') + raise CFBSValidationError(name, '"alias" is not supported in "build"') # Step 2 - Check for required fields: From 991a33e707d0f6f5f44a3c14ff4a6053c6591025 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Thu, 7 Dec 2023 19:50:20 +0100 Subject: [PATCH 38/64] Prevented an exception when trying to look for unknown keys in non-existant data Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/cfbs_json.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/cfbs/cfbs_json.py b/cfbs/cfbs_json.py index 728b59ee..be03246c 100644 --- a/cfbs/cfbs_json.py +++ b/cfbs/cfbs_json.py @@ -107,11 +107,14 @@ def warn_about_unknown_keys(self): Unknown keys are typically due to typos, or an outdated version of cfbs. This basic type of validation only produces warnings (we want cfbs to still work), - and is run for all cfbs commands, not just cfbs validate. + and is run for various cfbs commands, not just cfbs build / validate. For the more complete validation, see validate.py. """ data = self.raw_data + if not data: + return # No data, no unknown keys + for key in data: if key not in TOP_LEVEL_KEYS: log.warning( From 2cb7a86fd634a2e2edbd730c67fdb236a81a7d43 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem <4048546+olehermanse@users.noreply.github.com> Date: Fri, 8 Dec 2023 13:03:39 +0100 Subject: [PATCH 39/64] Applied suggestions from @larsewi's code review Co-authored-by: Lars Erik Wik <53906608+larsewi@users.noreply.github.com> --- cfbs/validate.py | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/cfbs/validate.py b/cfbs/validate.py index 6ba42640..e338ae74 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -258,7 +258,7 @@ def validate_steps(name, module): if expected != actual: raise CFBSValidationError( name, - "The %s build step expects %d arguents, %d were given" + "The %s build step expects %d arguments, %d were given" % (operation, expected, actual), ) else: @@ -268,7 +268,7 @@ def validate_steps(name, module): if actual < expected: raise CFBSValidationError( name, - "The %s build step expects %d or more arguents, %d were given" + "The %s build step expects %d or more arguments, %d were given" % (operation, expected, actual), ) @@ -402,12 +402,14 @@ def validate_module_input(name, module): required_fields.append("description") else: assert mode == "index" - required_fields.append("description") - required_fields.append("tags") - required_fields.append("repo") - required_fields.append("by") - required_fields.append("version") - required_fields.append("commit") + required_fields.extend([ + "description", + "tags", + "repo", + "by", + "version", + "commit", + ]) for required_field in required_fields: assert required_field in MODULE_KEYS From c9c1c5706c575a78c414a51de1f753d9d97e1c13 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Fri, 8 Dec 2023 13:14:43 +0100 Subject: [PATCH 40/64] Made cfbs validate exit with non-zero exit code in case of errors Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/commands.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cfbs/commands.py b/cfbs/commands.py index e2f26d22..4410f8b1 100644 --- a/cfbs/commands.py +++ b/cfbs/commands.py @@ -807,8 +807,7 @@ def update_command(to_update): @cfbs_command("validate") def validate_command(): config = CFBSConfig.get_instance() - validate_config(config) - return 0 + return validate_config(config) def _download_dependencies( From df2e9c07af9eb0485e7f2251e05d4ed935cab5d6 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Fri, 8 Dec 2023 13:15:10 +0100 Subject: [PATCH 41/64] Added warning message about having to fix cfbs.json for future cfbs build to work Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/commands.py | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/cfbs/commands.py b/cfbs/commands.py index 4410f8b1..7a66d5c8 100644 --- a/cfbs/commands.py +++ b/cfbs/commands.py @@ -891,14 +891,26 @@ def _download_dependencies( @cfbs_command("download") def download_command(force, ignore_versions=False): config = CFBSConfig.get_instance() - validate_config(config, build=True) + r = validate_config(config, build=True) + if r != 0: + log.warning( + "At least one error encountered while validating your cfbs.json file." + + "\nPlease see the error messages above and apply fixes accordingly." + + "\nIf not fixed, these errors will cause your project to not build in future cfbs versions." + ) _download_dependencies(config, redownload=force, ignore_versions=ignore_versions) @cfbs_command("build") def build_command(ignore_versions=False) -> int: config = CFBSConfig.get_instance() - validate_config(config, build=True) + r = validate_config(config, build=True) + if r != 0: + log.warning( + "At least one error encountered while validating your cfbs.json file." + + "\nPlease see the error messages above and apply fixes accordingly." + + "\nIf not fixed, these errors will cause your project to not build in future cfbs versions." + ) init_out_folder() _download_dependencies(config, prefer_offline=True, ignore_versions=ignore_versions) perform_build_steps(config) From 8ee820703b39ae6f2d161318fa1346eb119ca30d Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Fri, 8 Dec 2023 13:20:25 +0100 Subject: [PATCH 42/64] Removed special handling of validation in build / download commands Validation now uses the same logic whether you're running cfbs build / download / validate. How strict to be about the "build" field depends on: - The "type" ("policy-set" requires "build") - Whether the "build" field is present (then we validate it) Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/commands.py | 4 ++-- cfbs/validate.py | 15 ++++----------- 2 files changed, 6 insertions(+), 13 deletions(-) diff --git a/cfbs/commands.py b/cfbs/commands.py index 7a66d5c8..d48d0737 100644 --- a/cfbs/commands.py +++ b/cfbs/commands.py @@ -891,7 +891,7 @@ def _download_dependencies( @cfbs_command("download") def download_command(force, ignore_versions=False): config = CFBSConfig.get_instance() - r = validate_config(config, build=True) + r = validate_config(config) if r != 0: log.warning( "At least one error encountered while validating your cfbs.json file." @@ -904,7 +904,7 @@ def download_command(force, ignore_versions=False): @cfbs_command("build") def build_command(ignore_versions=False) -> int: config = CFBSConfig.get_instance() - r = validate_config(config, build=True) + r = validate_config(config) if r != 0: log.warning( "At least one error encountered while validating your cfbs.json file." diff --git a/cfbs/validate.py b/cfbs/validate.py index e338ae74..e0881b95 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -94,21 +94,14 @@ def _validate_top_level_keys(config): raise CFBSValidationError('The "index" URL must not contain spaces') -def _validate_config(config, build=False): +def _validate_config(config): # First validate the config i.e. the user's cfbs.json config.warn_about_unknown_keys() _validate_top_level_keys(config) raw_data = config.raw_data - if build: + if config["type"] == "policy-set" or "build" in config: _validate_config_for_build_field(config) - else: - # If we're not expecting to build anything yet - # (running a build or download command), - # we will accept a missing build field or empty list. - # Other bad values should still error: - if "build" in config and config["build"] != []: - _validate_config_for_build_field(config) if "index" in raw_data and type(raw_data["index"]) in (dict, OrderedDict): for name, module in raw_data["index"].items(): @@ -117,9 +110,9 @@ def _validate_config(config, build=False): # TODO: Add "provides" here -def validate_config(config, build=False): +def validate_config(config): try: - _validate_config(config, build) + _validate_config(config) return 0 except CFBSValidationError as e: print(e) From 69a6621607f33c7ea25c8101efb153558e768a4a Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Fri, 8 Dec 2023 13:22:29 +0100 Subject: [PATCH 43/64] Reformatted @larsewi's addition with black Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/validate.py | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/cfbs/validate.py b/cfbs/validate.py index e0881b95..475167ea 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -395,14 +395,16 @@ def validate_module_input(name, module): required_fields.append("description") else: assert mode == "index" - required_fields.extend([ - "description", - "tags", - "repo", - "by", - "version", - "commit", - ]) + required_fields.extend( + [ + "description", + "tags", + "repo", + "by", + "version", + "commit", + ] + ) for required_field in required_fields: assert required_field in MODULE_KEYS From 86095e58cfa9ea56002f56c417bf77e98a313a50 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Fri, 8 Dec 2023 15:20:58 +0100 Subject: [PATCH 44/64] Added can_reach_dependency() helper to make the dependency logic more simple and flexible Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/cfbs_config.py | 14 ++++++++++++++ cfbs/index.py | 3 +++ cfbs/validate.py | 14 ++++++++------ 3 files changed, 25 insertions(+), 6 deletions(-) diff --git a/cfbs/cfbs_config.py b/cfbs/cfbs_config.py index c035fa13..ac8efb64 100644 --- a/cfbs/cfbs_config.py +++ b/cfbs/cfbs_config.py @@ -485,3 +485,17 @@ def _input_list(input_data): definition["response"] = _input_list(definition) else: user_error("Unsupported input type '%s'" % definition["type"]) + + def _get_all_module_names(self): + modules = [] + + if "build" in self: + modules.extend((x["name"] for x in self["build"])) + if "provides" in self: + modules.extend(self["provides"].keys()) + modules.extend(self.index.keys()) + + return modules + + def can_reach_dependency(self, name): + return name in self._get_all_module_names() diff --git a/cfbs/index.py b/cfbs/index.py index 1706b646..e1e75000 100644 --- a/cfbs/index.py +++ b/cfbs/index.py @@ -70,6 +70,9 @@ def __contains__(self, key): def __getitem__(self, key): return self.data["index"][key] + def keys(self): + return self.data["index"].keys() + def items(self): return self.data["index"].items() diff --git a/cfbs/validate.py b/cfbs/validate.py index 475167ea..6f8f3d13 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -105,7 +105,7 @@ def _validate_config(config): if "index" in raw_data and type(raw_data["index"]) in (dict, OrderedDict): for name, module in raw_data["index"].items(): - _validate_module_object("index", name, module, raw_data["index"]) + _validate_module_object("index", name, module, config) # TODO: Add "provides" here @@ -119,7 +119,7 @@ def validate_config(config): return 1 -def _validate_module_object(mode, name, module, modules): +def _validate_module_object(mode, name, module, config): def validate_alias(name, module): assert "alias" in module if len(module) != 1: @@ -130,7 +130,7 @@ def validate_alias(name, module): raise CFBSValidationError(name, '"alias" must be of type string') if not module["alias"]: raise CFBSValidationError(name, '"alias" must be non-empty') - if not module["alias"] in modules: + if config.can_reach_dependency(module["alias"]): raise CFBSValidationError(name, '"alias" must reference another module') if "alias" in modules[module["alias"]]: raise CFBSValidationError(name, '"alias" cannot reference another alias') @@ -172,7 +172,7 @@ def validate_by(name, module): if not module["by"]: raise CFBSValidationError(name, '"by" must be non-empty') - def validate_dependencies(name, module, modules): + def validate_dependencies(name, module, config): assert "dependencies" in module if type(module["dependencies"]) != list: raise CFBSValidationError( @@ -183,9 +183,11 @@ def validate_dependencies(name, module, modules): raise CFBSValidationError( name, '"dependencies" must be a list of strings' ) - if not dependency in modules: + if not config.can_reach_dependency(dependency): raise CFBSValidationError( - name, '"dependencies" reference other modules' + name, + '"dependencies" references a module which could not be found: "%s"' + % dependency, ) if "alias" in modules[dependency]: raise CFBSValidationError( From abffd62bab47d7b53b70e21a01a05f131861e552 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Fri, 8 Dec 2023 15:57:50 +0100 Subject: [PATCH 45/64] Added validation for the "provides" field Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/validate.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/cfbs/validate.py b/cfbs/validate.py index 6f8f3d13..52e20d7c 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -72,6 +72,7 @@ def _validate_top_level_keys(config): if "git" in config and config["git"] not in (True, False): raise CFBSValidationError('The "git" field must be true or false') + if "index" in config: index = config["index"] if type(index) not in (str, dict, OrderedDict): @@ -93,6 +94,12 @@ def _validate_top_level_keys(config): if type(index) is str and index.startswith("https://") and " " in index: raise CFBSValidationError('The "index" URL must not contain spaces') + if "provides" in config: + if type(config["provides"]) not in (dict, OrderedDict): + raise CFBSValidationError( + 'The "provides" field must be an object (dictionary)' + ) + def _validate_config(config): # First validate the config i.e. the user's cfbs.json @@ -107,7 +114,9 @@ def _validate_config(config): for name, module in raw_data["index"].items(): _validate_module_object("index", name, module, config) - # TODO: Add "provides" here + if "provides" in raw_data: + for name, module in raw_data["provides"].items(): + _validate_module_object("provides", name, module, config) def validate_config(config): From 2d8fd73efbf88d4207f3b05e29570659a343334e Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Fri, 8 Dec 2023 16:30:20 +0100 Subject: [PATCH 46/64] Removed leftover print statements used while debugging Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/pretty.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/cfbs/pretty.py b/cfbs/pretty.py index 33163b94..dd0f0844 100644 --- a/cfbs/pretty.py +++ b/cfbs/pretty.py @@ -88,11 +88,7 @@ def _children_sort(child, name, sorting_rules): continue if key not in sorting_rules[name][1]: continue - print("Found list: " + key) rules = sorting_rules[name][1][key][1] - print(pretty(rules)) - if key in sorting_rules: - print("sorting_rules found for " + key) for element in child[key]: if type(element) is OrderedDict: _children_sort(element, key, rules) From 8d4fbbf728c0c77078e2cd5d0c9e05accfc4c704 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Fri, 8 Dec 2023 16:47:35 +0100 Subject: [PATCH 47/64] Added a shell test for cfbs validate Signed-off-by: Ole Herman Schumacher Elgesem --- tests/shell/037_cfbs_validate.sh | 206 +++++++++++++++++++++++++++++++ tests/shell/all.sh | 1 + 2 files changed, 207 insertions(+) create mode 100644 tests/shell/037_cfbs_validate.sh diff --git a/tests/shell/037_cfbs_validate.sh b/tests/shell/037_cfbs_validate.sh new file mode 100644 index 00000000..4671c976 --- /dev/null +++ b/tests/shell/037_cfbs_validate.sh @@ -0,0 +1,206 @@ +set -e +set -x +cd tests/ +mkdir -p ./tmp/ +cd ./tmp/ +rm -rf ./* + +# A small index: + +echo '{ + "name": "index", + "description": "The official (default) index of modules for CFEngine Build", + "type": "index", + "index": { + "autorun": { + "description": "Enables autorun functionality.", + "tags": ["supported", "management"], + "repo": "https://github.com/cfengine/modules", + "by": "https://github.com/olehermanse", + "version": "1.0.1", + "commit": "c3b7329b240cf7ad062a0a64ee8b607af2cb912a", + "subdirectory": "management/autorun", + "steps": ["json def.json def.json"] + }, + "compliance-report-imports": { + "name": "compliance-report-imports", + "description": "Used by other modules to import compliance reports to Mission Portal.", + "tags": ["experimental", "cfengine-enterprise"], + "repo": "https://github.com/nickanderson/cfengine-security-hardening", + "by": "https://github.com/nickanderson", + "version": "0.0.8", + "commit": "06f0894b662befbba4e775884f21cfe8573c32d6", + "subdirectory": "./compliance-report-imports", + "dependencies": ["autorun"], + "steps": ["copy ./compliance-report-imports.cf services/autorun/"] + }, + "compliance-report-lynis": { + "description": "Compliance report with Lynis checks.", + "tags": ["experimental", "security", "compliance"], + "repo": "https://github.com/nickanderson/cfengine-lynis", + "by": "https://github.com/nickanderson/", + "version": "3.0.9", + "commit": "9aaff7dd802cf879f40b992243e760f039e7636c", + "subdirectory": "./compliance-reports", + "dependencies": ["compliance-report-imports", "lynis"], + "steps": [ + "copy ./generated-compliance-report.json .no-distrib/compliance-report-definitions/lynis-compliance-report.json" + ] + }, + "lynis": { + "description": "Automates the installation, running, and reporting of CISOfys lynis system audits.", + "tags": ["security", "compliance"], + "repo": "https://github.com/nickanderson/cfengine-lynis", + "by": "https://github.com/nickanderson", + "version": "3.0.9", + "commit": "9aaff7dd802cf879f40b992243e760f039e7636c", + "steps": [ + "copy policy/main.cf services/lynis/main.cf", + "json cfbs/def.json def.json" + ] + } + } +} +' > cfbs.json + +cfbs validate + +# Missing dependency in index: + +echo '{ + "name": "index", + "description": "The official (default) index of modules for CFEngine Build", + "type": "index", + "index": { + "compliance-report-imports": { + "description": "Used by other modules to import compliance reports to Mission Portal.", + "tags": ["experimental", "cfengine-enterprise"], + "repo": "https://github.com/nickanderson/cfengine-security-hardening", + "by": "https://github.com/nickanderson", + "version": "0.0.8", + "commit": "06f0894b662befbba4e775884f21cfe8573c32d6", + "subdirectory": "./compliance-report-imports", + "dependencies": ["autorun"], + "steps": ["copy ./compliance-report-imports.cf services/autorun/"] + } + } +} +' > cfbs.json + +!( cfbs validate ) + +# Same, but without listing a dependency + +echo '{ + "name": "index", + "description": "The official (default) index of modules for CFEngine Build", + "type": "index", + "index": { + "compliance-report-imports": { + "description": "Used by other modules to import compliance reports to Mission Portal.", + "tags": ["experimental", "cfengine-enterprise"], + "repo": "https://github.com/nickanderson/cfengine-security-hardening", + "by": "https://github.com/nickanderson", + "version": "0.0.8", + "commit": "06f0894b662befbba4e775884f21cfe8573c32d6", + "subdirectory": "./compliance-report-imports", + "steps": ["copy ./compliance-report-imports.cf services/autorun/"] + } + } +} +' > cfbs.json + +cfbs validate + +# A typical policy set project with some local policy files + +echo '{ + "name": "Example project", + "description": "Example description", + "type": "policy-set", + "git": true, + "build": [ + { + "name": "masterfiles", + "description": "Official CFEngine Masterfiles Policy Framework (MPF).", + "tags": ["supported", "base"], + "repo": "https://github.com/cfengine/masterfiles", + "by": "https://github.com/cfengine", + "version": "3.21.3", + "commit": "ca637d4e6148432a90b7db598a4137956c0e0282", + "added_by": "cfbs add", + "steps": [ + "run EXPLICIT_VERSION=3.21.3 EXPLICIT_RELEASE=1 ./prepare.sh -y", + "copy ./ ./" + ] + }, + { + "name": "./policy.cf", + "description": "Local policy file added using cfbs command line", + "tags": ["local"], + "added_by": "cfbs add", + "dependencies": ["masterfiles"], + "steps": [ + "copy ./policy.cf services/cfbs/policy.cf", + "policy_files services/cfbs/policy.cf", + "bundles my_bundle" + ] + }, + { + "name": "./more.cf", + "description": "Local policy file added using cfbs command line", + "tags": ["local"], + "added_by": "cfbs add", + "dependencies": ["./policy.cf"], + "steps": [ + "copy ./more.cf services/cfbs/more.cf", + "policy_files services/cfbs/more.cf", + "bundles more" + ] + } + ] +}' > cfbs.json + +cfbs validate + +# Removing the second module we'll have a missing dependency: + +echo '{ + "name": "Example project", + "description": "Example description", + "type": "policy-set", + "git": true, + "build": [ + { + "name": "masterfiles", + "description": "Official CFEngine Masterfiles Policy Framework (MPF).", + "tags": ["supported", "base"], + "repo": "https://github.com/cfengine/masterfiles", + "by": "https://github.com/cfengine", + "version": "3.21.3", + "commit": "ca637d4e6148432a90b7db598a4137956c0e0282", + "added_by": "cfbs add", + "steps": [ + "run EXPLICIT_VERSION=3.21.3 EXPLICIT_RELEASE=1 ./prepare.sh -y", + "copy ./ ./" + ] + }, + { + "name": "./more.cf", + "description": "Local policy file added using cfbs command line", + "tags": ["local"], + "added_by": "cfbs add", + "dependencies": ["./policy.cf"], + "steps": [ + "copy ./more.cf services/cfbs/more.cf", + "policy_files services/cfbs/more.cf", + "bundles more" + ] + } + ] +}' > cfbs.json + +!( cfbs validate ) + +# NOTE: This shell test just covers some basic cases +# See the unit tests for more thorough testing of validation diff --git a/tests/shell/all.sh b/tests/shell/all.sh index 1992d51c..778f4075 100644 --- a/tests/shell/all.sh +++ b/tests/shell/all.sh @@ -40,5 +40,6 @@ bash tests/shell/033_add_commits_local_files.sh bash tests/shell/034_git_user_name_git_user_email.sh bash tests/shell/035_cfbs_build_compatibility_1.sh bash tests/shell/036_cfbs_build_compatibility_2.sh +bash tests/shell/037_cfbs_validate.sh echo "All cfbs shell tests completed successfully!" From 1ed9aa4739f1b55151192c35a1460c4b1439086e Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Fri, 8 Dec 2023 16:59:44 +0100 Subject: [PATCH 48/64] Changed validation to look for dependencies in the appropriate field(s) Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/cfbs_config.py | 11 ++++----- cfbs/validate.py | 38 ++++++++++++++++++++++---------- tests/shell/037_cfbs_validate.sh | 25 +++++++++++++++++++++ 3 files changed, 57 insertions(+), 17 deletions(-) diff --git a/cfbs/cfbs_config.py b/cfbs/cfbs_config.py index ac8efb64..04bf0ac0 100644 --- a/cfbs/cfbs_config.py +++ b/cfbs/cfbs_config.py @@ -486,16 +486,17 @@ def _input_list(input_data): else: user_error("Unsupported input type '%s'" % definition["type"]) - def _get_all_module_names(self): + def _get_all_module_names(self, search_in=("build", "provides", "index")): modules = [] - if "build" in self: + if "build" in search_in and "build" in self: modules.extend((x["name"] for x in self["build"])) - if "provides" in self: + if "provides" in search_in and "provides" in self: modules.extend(self["provides"].keys()) - modules.extend(self.index.keys()) + if "index" in search_in: + modules.extend(self.index.keys()) return modules - def can_reach_dependency(self, name): + def can_reach_dependency(self, name, search_in=("build", "provides", "index")): return name in self._get_all_module_names() diff --git a/cfbs/validate.py b/cfbs/validate.py index 52e20d7c..0f33d1ac 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -129,7 +129,15 @@ def validate_config(config): def _validate_module_object(mode, name, module, config): - def validate_alias(name, module): + def validate_alias(name, module, context): + if context == "index": + search_in = ("index",) + elif context == "provides": + search_in = "provides" + else: + raise CFBSValidationError( + name, '"alias" is only allowed inside "index" or "provides"' + ) assert "alias" in module if len(module) != 1: raise CFBSValidationError( @@ -139,8 +147,10 @@ def validate_alias(name, module): raise CFBSValidationError(name, '"alias" must be of type string') if not module["alias"]: raise CFBSValidationError(name, '"alias" must be non-empty') - if config.can_reach_dependency(module["alias"]): - raise CFBSValidationError(name, '"alias" must reference another module') + if config.can_reach_dependency(module["alias"], search_in): + raise CFBSValidationError( + name, '"alias" must reference another module in the index' + ) if "alias" in modules[module["alias"]]: raise CFBSValidationError(name, '"alias" cannot reference another alias') @@ -181,7 +191,14 @@ def validate_by(name, module): if not module["by"]: raise CFBSValidationError(name, '"by" must be non-empty') - def validate_dependencies(name, module, config): + def validate_dependencies(name, module, config, context): + if context == "build": + search_in = ("build",) + elif context == "provides": + search_in = ("index", "provides") + else: + assert context == "index" + search_in = ("index",) assert "dependencies" in module if type(module["dependencies"]) != list: raise CFBSValidationError( @@ -192,7 +209,7 @@ def validate_dependencies(name, module, config): raise CFBSValidationError( name, '"dependencies" must be a list of strings' ) - if not config.can_reach_dependency(dependency): + if not config.can_reach_dependency(dependency, search_in): raise CFBSValidationError( name, '"dependencies" references a module which could not be found: "%s"' @@ -389,12 +406,9 @@ def validate_module_input(name, module): # Step 1 - Handle special cases (alias): if "alias" in module: - if mode in ("index", "provides"): - validate_alias(name, module) - return - else: - assert mode == "build" - raise CFBSValidationError(name, '"alias" is not supported in "build"') + # Needs to be validated first because it's missing the other fields: + validate_alias(name, module, mode) + return # alias entries would fail the other validation below # Step 2 - Check for required fields: @@ -437,7 +451,7 @@ def validate_module_input(name, module): if "by" in module: validate_by(name, module) if "dependencies" in module: - validate_dependencies(name, module, modules) + validate_dependencies(name, module, modules, mode) if "version" in module: validate_version(name, module) if "commit" in module: diff --git a/tests/shell/037_cfbs_validate.sh b/tests/shell/037_cfbs_validate.sh index 4671c976..0af526b5 100644 --- a/tests/shell/037_cfbs_validate.sh +++ b/tests/shell/037_cfbs_validate.sh @@ -202,5 +202,30 @@ echo '{ !( cfbs validate ) +# Dependency exists in index, but not in build - should error: + +echo '{ + "name": "Example project", + "description": "Example description", + "type": "policy-set", + "git": true, + "build": [ + { + "name": "./more.cf", + "description": "Local policy file added using cfbs command line", + "tags": ["local"], + "added_by": "cfbs add", + "dependencies": ["masterfiles"], + "steps": [ + "copy ./more.cf services/cfbs/more.cf", + "policy_files services/cfbs/more.cf", + "bundles more" + ] + } + ] +}' > cfbs.json + +!( cfbs validate ) + # NOTE: This shell test just covers some basic cases # See the unit tests for more thorough testing of validation From 46690fdd0c6cd7fff0f02092029413b41ec20d2a Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Fri, 8 Dec 2023 17:15:56 +0100 Subject: [PATCH 49/64] validate.py: Improved variable name mode to context When validating a module, the context is the field you found the module inside. Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/validate.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/cfbs/validate.py b/cfbs/validate.py index 0f33d1ac..faccc4a0 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -128,7 +128,7 @@ def validate_config(config): return 1 -def _validate_module_object(mode, name, module, config): +def _validate_module_object(context, name, module, config): def validate_alias(name, module, context): if context == "index": search_in = ("index",) @@ -401,25 +401,25 @@ def validate_module_input(name, module): % part["type"], ) - assert mode in ("index", "provides", "build") + assert context in ("index", "provides", "build") # Step 1 - Handle special cases (alias): if "alias" in module: # Needs to be validated first because it's missing the other fields: - validate_alias(name, module, mode) + validate_alias(name, module, context) return # alias entries would fail the other validation below # Step 2 - Check for required fields: required_fields = ["steps"] - if mode == "build": + if context == "build": required_fields.append("name") - elif mode == "provides": + elif context == "provides": required_fields.append("description") else: - assert mode == "index" + assert context == "index" required_fields.extend( [ "description", @@ -451,7 +451,7 @@ def validate_module_input(name, module): if "by" in module: validate_by(name, module) if "dependencies" in module: - validate_dependencies(name, module, modules, mode) + validate_dependencies(name, module, modules, context) if "version" in module: validate_version(name, module) if "commit" in module: From 29b99b2646c5f5d3590f2182ec6d57c58b147dad Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Fri, 8 Dec 2023 17:38:45 +0100 Subject: [PATCH 50/64] Fixed issue with checking the target of an alias Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/cfbs_config.py | 12 ++++++++++++ cfbs/validate.py | 4 ++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/cfbs/cfbs_config.py b/cfbs/cfbs_config.py index 04bf0ac0..87a7785b 100644 --- a/cfbs/cfbs_config.py +++ b/cfbs/cfbs_config.py @@ -500,3 +500,15 @@ def _get_all_module_names(self, search_in=("build", "provides", "index")): def can_reach_dependency(self, name, search_in=("build", "provides", "index")): return name in self._get_all_module_names() + + def find_module(self, name, search_in=("build", "provides", "index")): + if "build" in search_in and "build" in self: + for module in (x for x in self["build"]): + if module["name"] == name: + return module + if "provides" in search_in and "provides" in self and name in self["provides"]: + return self["provides"][name] + if "index" in search_in and name in self.index: + return self.index[name] + + return None diff --git a/cfbs/validate.py b/cfbs/validate.py index faccc4a0..eceb6a79 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -151,7 +151,7 @@ def validate_alias(name, module, context): raise CFBSValidationError( name, '"alias" must reference another module in the index' ) - if "alias" in modules[module["alias"]]: + if "alias" in config.find_module(module["alias"], search_in): raise CFBSValidationError(name, '"alias" cannot reference another alias') def validate_name(name, module): @@ -215,7 +215,7 @@ def validate_dependencies(name, module, config, context): '"dependencies" references a module which could not be found: "%s"' % dependency, ) - if "alias" in modules[dependency]: + if "alias" in config.find_module(dependency): raise CFBSValidationError( name, '"dependencies" cannot reference an alias' ) From c2853b509316d00069b459ddfb6e93c6c6a13696 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Fri, 8 Dec 2023 17:41:53 +0100 Subject: [PATCH 51/64] Fixed argument name for validate_dependencies Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/validate.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cfbs/validate.py b/cfbs/validate.py index eceb6a79..f05c4767 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -451,7 +451,7 @@ def validate_module_input(name, module): if "by" in module: validate_by(name, module) if "dependencies" in module: - validate_dependencies(name, module, modules, context) + validate_dependencies(name, module, config, context) if "version" in module: validate_version(name, module) if "commit" in module: From 5f576e383b00ef7b459fe7f93f08f066f733e416 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Tue, 12 Dec 2023 13:00:52 +0100 Subject: [PATCH 52/64] Updated backwards compatibility tests to more strict validation Signed-off-by: Ole Herman Schumacher Elgesem --- tests/shell/035_cfbs_build_compatibility_1.sh | 31 ++++++++++++++++++- tests/shell/036_cfbs_build_compatibility_2.sh | 6 ++-- 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/tests/shell/035_cfbs_build_compatibility_1.sh b/tests/shell/035_cfbs_build_compatibility_1.sh index 7aa6d267..65f1abcb 100644 --- a/tests/shell/035_cfbs_build_compatibility_1.sh +++ b/tests/shell/035_cfbs_build_compatibility_1.sh @@ -42,7 +42,9 @@ grep 'bundle common inventory' out/masterfiles/promises.cf # These other commands should also work: cfbs status -cfbs validate +# NOTE: We expect cfbs build to work, but not cfbs validate since +# this older module entry has an empty string for "subdirectory". +!( cfbs validate ) # Once more, but let's do download and build as separate steps: rm -rf out/ @@ -54,3 +56,30 @@ cfbs build # Perform same checks again: grep 'bundle common inventory' out/masterfiles/promises.cf + +# Finally, let's see validation working if we fix the module: +rm -rf out/ +rm -rf ~/.cfengine/cfbs + +echo '{ + "name": "backwards-compatibility-test-1", + "type": "policy-set", + "description": "This project was set up to ensure projects created with CFEngine 3.21.0 / cfbs 3.2.7 still build as expected", + "build": [ + { + "name": "masterfiles", + "version": "3.21.0", + "description": "Official CFEngine Masterfiles Policy Framework (MPF).", + "tags": ["supported", "base"], + "repo": "https://github.com/cfengine/masterfiles", + "by": "https://github.com/cfengine", + "commit": "379c69aa71ab3069b2ef1c0cca526192fa77b864", + "added_by": "cfbs add", + "steps": ["run ./prepare.sh -y", "copy ./ ./"] + } + ], + "git": true +} +' > cfbs.json + +cfbs validate diff --git a/tests/shell/036_cfbs_build_compatibility_2.sh b/tests/shell/036_cfbs_build_compatibility_2.sh index a6d57b26..ea95efc7 100644 --- a/tests/shell/036_cfbs_build_compatibility_2.sh +++ b/tests/shell/036_cfbs_build_compatibility_2.sh @@ -5,7 +5,7 @@ mkdir -p ./tmp/ cd ./tmp/ rm -rf ./* -# This test is identical to the previous test, except it has more modules. +# This test is similar to the previous test, except it has more modules. echo '{ "name": "backwards-compatibility-test-2", @@ -93,7 +93,9 @@ grep '$(paths.systemctl) list-units --type=service --state=running' out/masterfi # These other commands should also work: cfbs status -cfbs validate +# NOTE: We expect cfbs build to work, but not cfbs validate since +# this older module entry has an empty string for "subdirectory". +!( cfbs validate ) # Once more, but let's do download and build as separate steps: rm -rf out/ From c25dc57813a9f3ae47ca24fcf195994eec47ff13 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem <4048546+olehermanse@users.noreply.github.com> Date: Tue, 12 Dec 2023 13:04:23 +0100 Subject: [PATCH 53/64] Applied suggestion from @larsewi's code review Co-authored-by: Lars Erik Wik <53906608+larsewi@users.noreply.github.com> --- cfbs/cfbs_config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cfbs/cfbs_config.py b/cfbs/cfbs_config.py index 87a7785b..5e4a49b0 100644 --- a/cfbs/cfbs_config.py +++ b/cfbs/cfbs_config.py @@ -503,7 +503,7 @@ def can_reach_dependency(self, name, search_in=("build", "provides", "index")): def find_module(self, name, search_in=("build", "provides", "index")): if "build" in search_in and "build" in self: - for module in (x for x in self["build"]): + for module in self["build"]: if module["name"] == name: return module if "provides" in search_in and "provides" in self and name in self["provides"]: From 83fb681b08276c3a8164a48f9eb7ae9a855a59ae Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Tue, 12 Dec 2023 13:07:08 +0100 Subject: [PATCH 54/64] Fixed 2 issues with looking for alias targets / dependencies Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/cfbs_config.py | 2 +- cfbs/validate.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cfbs/cfbs_config.py b/cfbs/cfbs_config.py index 5e4a49b0..a84d7b98 100644 --- a/cfbs/cfbs_config.py +++ b/cfbs/cfbs_config.py @@ -499,7 +499,7 @@ def _get_all_module_names(self, search_in=("build", "provides", "index")): return modules def can_reach_dependency(self, name, search_in=("build", "provides", "index")): - return name in self._get_all_module_names() + return name in self._get_all_module_names(search_in) def find_module(self, name, search_in=("build", "provides", "index")): if "build" in search_in and "build" in self: diff --git a/cfbs/validate.py b/cfbs/validate.py index f05c4767..e8c55392 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -147,7 +147,7 @@ def validate_alias(name, module, context): raise CFBSValidationError(name, '"alias" must be of type string') if not module["alias"]: raise CFBSValidationError(name, '"alias" must be non-empty') - if config.can_reach_dependency(module["alias"], search_in): + if not config.can_reach_dependency(module["alias"], search_in): raise CFBSValidationError( name, '"alias" must reference another module in the index' ) From ee2de6992b89ab191f133a5f4113d9bcd02a8357 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Tue, 12 Dec 2023 13:37:21 +0100 Subject: [PATCH 55/64] Added validation test for aliases Signed-off-by: Ole Herman Schumacher Elgesem --- tests/test_validate_index_alias.py | 61 ++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) create mode 100644 tests/test_validate_index_alias.py diff --git a/tests/test_validate_index_alias.py b/tests/test_validate_index_alias.py new file mode 100644 index 00000000..3c5a65ae --- /dev/null +++ b/tests/test_validate_index_alias.py @@ -0,0 +1,61 @@ +from collections import OrderedDict +from copy import deepcopy +from pathlib import Path + +from cfbs.pretty import pretty +from cfbs.validate import validate_config +from cfbs.cfbs_config import CFBSConfig + +INDEX_ALIAS_VALID = { + "name": "index", + "description": "An example index of 1 module", + "type": "index", + "index": { + "test-alias": { + "alias": "test-target", + }, + "test-target": { + "description": "Allows all hosts / IP addresses to connect and fetch policy.", + "tags": ["management", "experimental"], + "repo": "https://github.com/cfengine/modules", + "by": "https://github.com/olehermanse", + "version": "0.0.1", + "commit": "85f9aec38783b5a4dac4777ffa9d17fde5054d14", + "subdirectory": "management/allow-all-hosts", + "steps": ["json def.json def.json"], + }, + }, +} + +INDEX_ALIAS_INVALID = { + "name": "index", + "description": "An example index of 1 module", + "type": "index", + "index": { + "test-alias": { + "alias": "test-target", + } + }, +} + + +def test_valid_index_with_alias(): + filename = Path("tmp-cfbs.json") + with open(filename, "w") as f: + f.write(pretty(INDEX_ALIAS_VALID)) + config = CFBSConfig(filename) + r = validate_config(config) + del config + filename.unlink() + assert r == 0 + + +def test_valid_index_with_alias(): + filename = Path("tmp-cfbs.json") + with open(filename, "w") as f: + f.write(pretty(INDEX_ALIAS_INVALID)) + config = CFBSConfig(filename) + r = validate_config(config) + del config + filename.unlink() + assert r != 0 From 6973e023f52a4ca91b02fc620d684e4ab5082f5d Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Tue, 12 Dec 2023 13:44:47 +0100 Subject: [PATCH 56/64] JSON.md: Removed trailing whitespace Signed-off-by: Ole Herman Schumacher Elgesem --- JSON.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/JSON.md b/JSON.md index dafd4dc6..154a97e9 100644 --- a/JSON.md +++ b/JSON.md @@ -77,7 +77,7 @@ Unless otherwise noted, all steps are run inside the module's folder (`out/steps #### `delete ` - Delete multiple files or paths recursively. - Files are deleted from the step folder. -- Typically used before copying files to the output policy set with the `copy` step. +- Typically used before copying files to the output policy set with the `copy` step. #### `directory ` - Copy any .cf policy files recursively and add their paths to `def.json`'s `inputs`. @@ -87,7 +87,7 @@ Unless otherwise noted, all steps are run inside the module's folder (`out/steps #### `bundles ` - Ensure bundles are evaluated by adding them to the bundle sequence, using `def.json`. - - Note that this relies on using the default policy set from the CFEngine team, the Masterfiles Policy Framework, commonly added as the first module (`masterfiles`). + - Note that this relies on using the default policy set from the CFEngine team, the Masterfiles Policy Framework, commonly added as the first module (`masterfiles`). Specifically, this build step adds the bundles to the variable `default:def.control_common_bundlesequence_end`, which the MPF looks for. - Only manipulates the bundle sequence, to ensure policy files are copied and parsed, use other build steps, for example `copy` and `policy_files`. From af9210aa790e237db94555b479eedd6c7039c761 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Tue, 12 Dec 2023 13:46:55 +0100 Subject: [PATCH 57/64] Fixed validation for number of arguments for "run" build steps Signed-off-by: Ole Herman Schumacher Elgesem --- JSON.md | 3 ++- cfbs/build.py | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/JSON.md b/JSON.md index 154a97e9..a17422f4 100644 --- a/JSON.md +++ b/JSON.md @@ -68,11 +68,12 @@ Unless otherwise noted, all steps are run inside the module's folder (`out/steps #### `append ` - Append the source file to the end of destination file. -#### `run ` +#### `run ` - Run a shell command / script. - Usually used to prepare the module directory, delete files, etc. before a copy step. - Running scripts should be avoided if possible. - Script is run inside the module directory (the step folder). +- Additional space separated arguments are passed as arguments. #### `delete ` - Delete multiple files or paths recursively. diff --git a/cfbs/build.py b/cfbs/build.py index e964c1b4..14a6c620 100644 --- a/cfbs/build.py +++ b/cfbs/build.py @@ -19,7 +19,7 @@ AVAILABLE_BUILD_STEPS = { "copy": 2, - "run": 1, + "run": "1+", "delete": 2, "json": 2, "append": 2, From 81c4877fa0fccf94b604f1415a76e02c7fa70301 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Tue, 12 Dec 2023 13:51:42 +0100 Subject: [PATCH 58/64] Fixed validation for looking for dependencies of modules in "build" array Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/validate.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cfbs/validate.py b/cfbs/validate.py index e8c55392..b97a5a34 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -485,7 +485,7 @@ def _validate_config_for_build_field(config): ) for index, module in enumerate(config["build"]): name = module["name"] if "name" in module else index - _validate_module_object("build", name, module, config.index.data["index"]) + _validate_module_object("build", name, module, config) def main(): From 6d674907e1b15e54141606ddbe116b06f9944d40 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Tue, 12 Dec 2023 13:58:11 +0100 Subject: [PATCH 59/64] Updated many "subdirectory" fields to conform with the new stricter validation Signed-off-by: Ole Herman Schumacher Elgesem --- .../shell/018_update_input_one_variable/example-cfbs.json | 4 ++-- .../019_update_input_two_variables/example-cfbs.json | 4 ++-- tests/shell/020_update_input_list/example-cfbs.json | 4 ++-- .../021_update_input_list_with_keys/example-cfbs.json | 4 ++-- .../022_update_input_fail_variable/example-cfbs.json | 4 ++-- .../shell/023_update_input_fail_number/example-cfbs.json | 4 ++-- .../shell/024_update_input_fail_bundle/example-cfbs.json | 4 ++-- tests/shell/037_cfbs_validate.sh | 8 ++++---- 8 files changed, 18 insertions(+), 18 deletions(-) diff --git a/tests/shell/018_update_input_one_variable/example-cfbs.json b/tests/shell/018_update_input_one_variable/example-cfbs.json index 406b5cf9..daa3be62 100644 --- a/tests/shell/018_update_input_one_variable/example-cfbs.json +++ b/tests/shell/018_update_input_one_variable/example-cfbs.json @@ -10,7 +10,7 @@ "by": "https://github.com/example-user", "version": "1.0.1", "commit": "3827a0da69d73ec2f973660d3c640b33cad58655", - "subdirectory": "./example-module", + "subdirectory": "example-module", "input": [ { "type": "string", @@ -33,7 +33,7 @@ "commit": "0000000000000000000000000000000000000000", "by": "https://github.com/example-user", "version": "1.0.0", - "subdirectory": "./example-module", + "subdirectory": "example-module", "input": [ { "type": "string", diff --git a/tests/shell/019_update_input_two_variables/example-cfbs.json b/tests/shell/019_update_input_two_variables/example-cfbs.json index e626fc0d..0abda37f 100644 --- a/tests/shell/019_update_input_two_variables/example-cfbs.json +++ b/tests/shell/019_update_input_two_variables/example-cfbs.json @@ -10,7 +10,7 @@ "commit": "3827a0da69d73ec2f973660d3c640b33cad58655", "by": "https://github.com/larsewi", "version": "1.0.1", - "subdirectory": "./example-module", + "subdirectory": "example-module", "input": [ { "type": "string", @@ -40,7 +40,7 @@ "commit": "0000000000000000000000000000000000000000", "by": "https://github.com/example-user", "version": "1.0.0", - "subdirectory": "./example-module", + "subdirectory": "example-module", "dependencies": ["autorun"], "input": [ { diff --git a/tests/shell/020_update_input_list/example-cfbs.json b/tests/shell/020_update_input_list/example-cfbs.json index 8a78f73f..84996ddb 100644 --- a/tests/shell/020_update_input_list/example-cfbs.json +++ b/tests/shell/020_update_input_list/example-cfbs.json @@ -10,7 +10,7 @@ "commit": "3827a0da69d73ec2f973660d3c640b33cad58655", "by": "https://github.com/example-user", "version": "1.0.1", - "subdirectory": "./example-module", + "subdirectory": "example-module", "input": [ { "type": "list", @@ -38,7 +38,7 @@ "commit": "0000000000000000000000000000000000000000", "by": "https://github.com/example-user", "version": "1.0.0", - "subdirectory": "./example-module", + "subdirectory": "example-module", "input": [ { "type": "list", 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 937befc7..9b0f7cca 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 @@ -10,7 +10,7 @@ "by": "https://github.com/example-user", "version": "1.0.1", "commit": "3827a0da69d73ec2f973660d3c640b33cad58655", - "subdirectory": "./example-module", + "subdirectory": "example-module", "input": [ { "type": "list", @@ -48,7 +48,7 @@ "commit": "0000000000000000000000000000000000000000", "by": "https://github.com/example-user", "version": "1.0.0", - "subdirectory": "./example-module", + "subdirectory": "example-module", "input": [ { "type": "list", diff --git a/tests/shell/022_update_input_fail_variable/example-cfbs.json b/tests/shell/022_update_input_fail_variable/example-cfbs.json index 11def62b..46942a7d 100644 --- a/tests/shell/022_update_input_fail_variable/example-cfbs.json +++ b/tests/shell/022_update_input_fail_variable/example-cfbs.json @@ -10,7 +10,7 @@ "by": "https://github.com/example-user", "version": "1.0.1", "commit": "3827a0da69d73ec2f973660d3c640b33cad58655", - "subdirectory": "./example-module", + "subdirectory": "example-module", "input": [ { "type": "string", @@ -33,7 +33,7 @@ "commit": "0000000000000000000000000000000000000000", "by": "https://github.com/example-user", "version": "1.0.0", - "subdirectory": "./example-module", + "subdirectory": "example-module", "input": [ { "type": "string", diff --git a/tests/shell/023_update_input_fail_number/example-cfbs.json b/tests/shell/023_update_input_fail_number/example-cfbs.json index 9c9d8dca..d229f7e9 100644 --- a/tests/shell/023_update_input_fail_number/example-cfbs.json +++ b/tests/shell/023_update_input_fail_number/example-cfbs.json @@ -10,7 +10,7 @@ "by": "https://github.com/example-user", "version": "1.0.1", "commit": "3827a0da69d73ec2f973660d3c640b33cad58655", - "subdirectory": "./example-module", + "subdirectory": "example-module", "input": [ { "type": "string", @@ -40,7 +40,7 @@ "commit": "0000000000000000000000000000000000000000", "by": "https://github.com/example-user", "version": "1.0.0", - "subdirectory": "./example-module", + "subdirectory": "example-module", "input": [ { "type": "string", diff --git a/tests/shell/024_update_input_fail_bundle/example-cfbs.json b/tests/shell/024_update_input_fail_bundle/example-cfbs.json index 87b5781e..18a7e529 100644 --- a/tests/shell/024_update_input_fail_bundle/example-cfbs.json +++ b/tests/shell/024_update_input_fail_bundle/example-cfbs.json @@ -10,7 +10,7 @@ "by": "https://github.com/example-user", "version": "1.0.1", "commit": "3827a0da69d73ec2f973660d3c640b33cad58655", - "subdirectory": "./example-module", + "subdirectory": "example-module", "input": [ { "type": "string", @@ -34,7 +34,7 @@ "commit": "0000000000000000000000000000000000000000", "by": "https://github.com/example-user", "version": "1.0.0", - "subdirectory": "./example-module", + "subdirectory": "example-module", "input": [ { "type": "string", diff --git a/tests/shell/037_cfbs_validate.sh b/tests/shell/037_cfbs_validate.sh index 0af526b5..a726c2be 100644 --- a/tests/shell/037_cfbs_validate.sh +++ b/tests/shell/037_cfbs_validate.sh @@ -30,7 +30,7 @@ echo '{ "by": "https://github.com/nickanderson", "version": "0.0.8", "commit": "06f0894b662befbba4e775884f21cfe8573c32d6", - "subdirectory": "./compliance-report-imports", + "subdirectory": "compliance-report-imports", "dependencies": ["autorun"], "steps": ["copy ./compliance-report-imports.cf services/autorun/"] }, @@ -41,7 +41,7 @@ echo '{ "by": "https://github.com/nickanderson/", "version": "3.0.9", "commit": "9aaff7dd802cf879f40b992243e760f039e7636c", - "subdirectory": "./compliance-reports", + "subdirectory": "compliance-reports", "dependencies": ["compliance-report-imports", "lynis"], "steps": [ "copy ./generated-compliance-report.json .no-distrib/compliance-report-definitions/lynis-compliance-report.json" @@ -79,7 +79,7 @@ echo '{ "by": "https://github.com/nickanderson", "version": "0.0.8", "commit": "06f0894b662befbba4e775884f21cfe8573c32d6", - "subdirectory": "./compliance-report-imports", + "subdirectory": "compliance-report-imports", "dependencies": ["autorun"], "steps": ["copy ./compliance-report-imports.cf services/autorun/"] } @@ -103,7 +103,7 @@ echo '{ "by": "https://github.com/nickanderson", "version": "0.0.8", "commit": "06f0894b662befbba4e775884f21cfe8573c32d6", - "subdirectory": "./compliance-report-imports", + "subdirectory": "compliance-report-imports", "steps": ["copy ./compliance-report-imports.cf services/autorun/"] } } From d5656f79ec0b06b8bd4135572db8804182ed6ad2 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Tue, 12 Dec 2023 14:05:28 +0100 Subject: [PATCH 60/64] A tribute to Python 3.5 Signed-off-by: Ole Herman Schumacher Elgesem --- tests/test_validate_index_alias.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/test_validate_index_alias.py b/tests/test_validate_index_alias.py index 3c5a65ae..f277ea94 100644 --- a/tests/test_validate_index_alias.py +++ b/tests/test_validate_index_alias.py @@ -1,6 +1,6 @@ +import os from collections import OrderedDict from copy import deepcopy -from pathlib import Path from cfbs.pretty import pretty from cfbs.validate import validate_config @@ -40,22 +40,22 @@ def test_valid_index_with_alias(): - filename = Path("tmp-cfbs.json") + filename = "./tmp-cfbs.json" with open(filename, "w") as f: f.write(pretty(INDEX_ALIAS_VALID)) config = CFBSConfig(filename) r = validate_config(config) del config - filename.unlink() + os.remove(filename) assert r == 0 def test_valid_index_with_alias(): - filename = Path("tmp-cfbs.json") + filename = "./tmp-cfbs.json" with open(filename, "w") as f: f.write(pretty(INDEX_ALIAS_INVALID)) config = CFBSConfig(filename) r = validate_config(config) del config - filename.unlink() + os.remove(filename) assert r != 0 From 8a7ca282656d90c9f6ffb1b579c6f3c7e4bf45e9 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Tue, 12 Dec 2023 16:38:50 +0100 Subject: [PATCH 61/64] Fixed validation for when "key" is not required in module input list subtypes Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/validate.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/cfbs/validate.py b/cfbs/validate.py index b97a5a34..32b94410 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -308,7 +308,7 @@ def validate_module_input(name, module): required_string_fields = ["type", "variable", "namespace", "bundle", "label"] - required_string_fields_subtype = ["key", "type", "label", "question"] + required_string_fields_subtype = ["type", "label", "question"] for input_element in module["input"]: if type(input_element) not in (dict, OrderedDict) or not input_element: @@ -394,6 +394,19 @@ def validate_module_input(name, module): 'The "%s" field in module input "subtype" objects must be a non-empty / non-whitespace string' % field, ) + if len(subtype) > 1: + # The "key" field is used to create the JSON objects for each + # input in a list of "things" which are not just strings, + # i.e. consist of multiple values + if ( + "key" not in part + or type(part["key"]) is not str + or part["key"].strip() == "" + ): + raise CFBSValidationError( + name, + 'When using module input with type list, and subtype includes multiple values, "key" is required to distinguish them', + ) if part["type"] != "string": raise CFBSValidationError( name, From ba459ce479ef70ab3c5fa82d3ff0ab527b02334e Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Tue, 12 Dec 2023 17:13:18 +0100 Subject: [PATCH 62/64] test_validate_index_alias.py: Fixed name of one of the tests / functions Signed-off-by: Ole Herman Schumacher Elgesem --- tests/test_validate_index_alias.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_validate_index_alias.py b/tests/test_validate_index_alias.py index f277ea94..0aa5a78e 100644 --- a/tests/test_validate_index_alias.py +++ b/tests/test_validate_index_alias.py @@ -50,7 +50,7 @@ def test_valid_index_with_alias(): assert r == 0 -def test_valid_index_with_alias(): +def test_invalid_index_with_alias(): filename = "./tmp-cfbs.json" with open(filename, "w") as f: f.write(pretty(INDEX_ALIAS_INVALID)) From edd4a7b704359ddb0775156be3fffd705b77fcf5 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem <4048546+olehermanse@users.noreply.github.com> Date: Tue, 12 Dec 2023 17:16:09 +0100 Subject: [PATCH 63/64] Improved some validation warning messages Code review suggestions from @craigcomstock Co-authored-by: Craig Comstock Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/validate.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/cfbs/validate.py b/cfbs/validate.py index 32b94410..1f354831 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -271,7 +271,9 @@ def validate_steps(name, module): if not operation in AVAILABLE_BUILD_STEPS: x = ", ".join(AVAILABLE_BUILD_STEPS) raise CFBSValidationError( - name, 'Unknown operation in "steps", must be one of: (%s)' % x + name, + 'Unknown operation "%s" in "steps", must be one of: (%s)' + % (operation, x), ) expected = AVAILABLE_BUILD_STEPS[operation] actual = len(args) @@ -341,18 +343,20 @@ def validate_module_input(name, module): if not re.fullmatch(r"[a-z_]+", input_element["variable"]): raise CFBSValidationError( name, - '"%s" is not an acceptable variable name' + '"%s" is not an acceptable variable name, must match regex "[a-z_]+"' % input_element["variable"], ) if not re.fullmatch(r"[a-z_]+", input_element["namespace"]): raise CFBSValidationError( name, - '"%s" is not an acceptable namespace' % input_element["namespace"], + '"%s" is not an acceptable namespace, must match regex "[a-z_]+"' + % input_element["namespace"], ) if not re.fullmatch(r"[a-z_]+", input_element["bundle"]): raise CFBSValidationError( name, - '"%s" is not an acceptable bundle name' % input_element["bundle"], + '"%s" is not an acceptable bundle name, must match regex "[a-z_]+"' + % input_element["bundle"], ) if input_element["type"] == "list": From 0e23a7b089e074220e349c0a0bc91a8c05815061 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem <4048546+olehermanse@users.noreply.github.com> Date: Wed, 13 Dec 2023 10:02:03 +0100 Subject: [PATCH 64/64] Fixed typo / spacing by @larsewi Co-authored-by: Lars Erik Wik <53906608+larsewi@users.noreply.github.com> --- cfbs/validate.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cfbs/validate.py b/cfbs/validate.py index 1f354831..bc78c5b6 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -400,7 +400,7 @@ def validate_module_input(name, module): ) if len(subtype) > 1: # The "key" field is used to create the JSON objects for each - # input in a list of "things" which are not just strings, + # input in a list of "things" which are not just strings, # i.e. consist of multiple values if ( "key" not in part