Merge pull request #167 from olehermanse/validation

Added validation for all types of cfbs.json files and all fields

JSON.md

@@ -68,16 +68,17 @@ Unless otherwise noted, all steps are run inside the module's folder (`out/steps
 #### `append <source> <destination>`
 - Append the source file to the end of destination file.
 
-#### `run <command>`
+#### `run <command ...>`
 - 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 <paths ...>`
 - 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 <source> <destination>`
 - Copy any .cf policy files recursively and add their paths to `def.json`'s `inputs`.
@@ -87,7 +88,7 @@ Unless otherwise noted, all steps are run inside the module's folder (`out/steps
 
 #### `bundles <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`.
 

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 [".", "./"]:

cfbs/cfbs_config.py

@@ -487,3 +487,30 @@ 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, search_in=("build", "provides", "index")):
+        modules = []
+
+        if "build" in search_in and "build" in self:
+            modules.extend((x["name"] for x in self["build"]))
+        if "provides" in search_in and "provides" in self:
+            modules.extend(self["provides"].keys())
+        if "index" in search_in:
+            modules.extend(self.index.keys())
+
+        return modules
+
+    def can_reach_dependency(self, name, search_in=("build", "provides", "index")):
+        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:
+            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"]:
+            return self["provides"][name]
+        if "index" in search_in and name in self.index:
+            return self.index[name]
+
+        return None

cfbs/cfbs_json.py

@@ -1,4 +1,6 @@
 from collections import OrderedDict
+from copy import deepcopy
+import logging as log
 
 from cfbs.index import Index
 from cfbs.utils import read_json, user_error
@@ -9,6 +11,7 @@
 TOP_LEVEL_KEYS = ("name", "description", "type", "index", "git", "provides", "build")
 
 MODULE_KEYS = (
+    "alias",
     "name",
     "description",
     "tags",
@@ -82,6 +85,53 @@ 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.raw_data
+        modules = []
+        if "index" in data and type(data["index"]) in (dict, OrderedDict):
+            modules += data["index"].values()
+        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"]
+        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 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(
+                    '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'
+                        % key
+                        + "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

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,
@@ -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
@@ -802,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(
@@ -887,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)
+    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)
+    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)
@@ -962,6 +978,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 +1020,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 +1056,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 +1138,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)

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()
 

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)