Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 64 commits into from
Dec 13, 2023
Merged
Show file tree
Hide file tree
Changes from 61 commits
Commits
Show all changes
64 commits
Select commit Hold shift + click to select a range
ba903ee
Moved module validation function into separate function
olehermanse Dec 4, 2023
bed074d
Removed unused import
olehermanse Dec 4, 2023
1b91d1f
Renamed CFBSIndexError to CFBSValidationError
olehermanse Dec 5, 2023
c0e1eca
Changed to double quotes in validation errors to match JSON syntax
olehermanse Dec 5, 2023
58434eb
Refactored module alias validation to prepare for further changes
olehermanse Dec 5, 2023
2ded2a5
Added separate validation for required fields
olehermanse Dec 5, 2023
0032e32
Simplified validation functions now that required fields are handled …
olehermanse Dec 5, 2023
3a82a9d
Changed all validation functions which only care about a single modul…
olehermanse Dec 5, 2023
40d3b2f
Added basic validation of module names to shared validation function
olehermanse Dec 5, 2023
ba7f69d
cfbs validate now more thoroughly validates various cfbs.json files
olehermanse Dec 5, 2023
a8c1cc7
validate.py: Small formatting improvements
olehermanse Dec 5, 2023
045c906
Added warnings when cfbs.json files have unknown fields
olehermanse Dec 5, 2023
6addb4c
Removed assertions which are no longer valid
olehermanse Dec 5, 2023
f180fef
Fixed typo in required fields logic
olehermanse Dec 5, 2023
9d5a224
validate.py: Fixed typo in alias validation
olehermanse Dec 5, 2023
0f4649a
Added raw_data property for working with the actual JSON data of a CF…
olehermanse Dec 5, 2023
fa1a66b
Fixed issue with typechecking for index and provides fields
olehermanse Dec 5, 2023
ee8aac6
Fixed checking of aliases
olehermanse Dec 5, 2023
77a320a
Made it possible to only pass message to CFBSValidationError
olehermanse Dec 5, 2023
9deb960
Updated validation log messages for being used more generally
olehermanse Dec 5, 2023
f656cfe
Prepared CFBSValidationError to be used with an index instead of a name
olehermanse Dec 5, 2023
a36219c
Added validation for top level keys of cfbs.json
olehermanse Dec 5, 2023
fe5d7de
Removed a lot of the special handling for validation of index files
olehermanse Dec 5, 2023
eb19557
Added "alias" as a known module key so it's recognized in validation
olehermanse Dec 5, 2023
48218aa
Fixed the warning message for unknown keys
olehermanse Dec 5, 2023
7011dc3
Validation: Added checking to keep required_fields in sync with globa…
olehermanse Dec 5, 2023
d5e4659
Put the index in place of module name when module name is missing
olehermanse Dec 5, 2023
d532da9
Improved validation error messages by catching the exception
olehermanse Dec 5, 2023
165437c
Fixed validation error message for missing index
olehermanse Dec 7, 2023
02c0334
Fixed validation error message for a required field missing inside mo…
olehermanse Dec 7, 2023
4613db3
Added more thorough validation of build steps
olehermanse Dec 7, 2023
471ea24
Added more thorough validation of subdirectory
olehermanse Dec 7, 2023
6dd1cb3
Reformatted files with black
olehermanse Dec 7, 2023
447c478
Added a unit test for validating a simple index and catching many of …
olehermanse Dec 7, 2023
bfba0dc
Added validation for module input information in cfbs.json
olehermanse Dec 7, 2023
6906453
Added unit test for validation of module input inside cfbs.json
olehermanse Dec 7, 2023
0827f12
Fixed exception name for alias not supported error message
olehermanse Dec 7, 2023
991a33e
Prevented an exception when trying to look for unknown keys in non-ex…
olehermanse Dec 7, 2023
2cb7a86
Applied suggestions from @larsewi's code review
olehermanse Dec 8, 2023
c9c1c57
Made cfbs validate exit with non-zero exit code in case of errors
olehermanse Dec 8, 2023
df2e9c0
Added warning message about having to fix cfbs.json for future cfbs b…
olehermanse Dec 8, 2023
8ee8207
Removed special handling of validation in build / download commands
olehermanse Dec 8, 2023
69a6621
Reformatted @larsewi's addition with black
olehermanse Dec 8, 2023
86095e5
Added can_reach_dependency() helper to make the dependency logic more…
olehermanse Dec 8, 2023
abffd62
Added validation for the "provides" field
olehermanse Dec 8, 2023
2d8fd73
Removed leftover print statements used while debugging
olehermanse Dec 8, 2023
8d4fbbf
Added a shell test for cfbs validate
olehermanse Dec 8, 2023
1ed9aa4
Changed validation to look for dependencies in the appropriate field(s)
olehermanse Dec 8, 2023
46690fd
validate.py: Improved variable name mode to context
olehermanse Dec 8, 2023
29b99b2
Fixed issue with checking the target of an alias
olehermanse Dec 8, 2023
c2853b5
Fixed argument name for validate_dependencies
olehermanse Dec 8, 2023
5f576e3
Updated backwards compatibility tests to more strict validation
olehermanse Dec 12, 2023
c25dc57
Applied suggestion from @larsewi's code review
olehermanse Dec 12, 2023
83fb681
Fixed 2 issues with looking for alias targets / dependencies
olehermanse Dec 12, 2023
ee2de69
Added validation test for aliases
olehermanse Dec 12, 2023
6973e02
JSON.md: Removed trailing whitespace
olehermanse Dec 12, 2023
af9210a
Fixed validation for number of arguments for "run" build steps
olehermanse Dec 12, 2023
81c4877
Fixed validation for looking for dependencies of modules in "build" a…
olehermanse Dec 12, 2023
6d67490
Updated many "subdirectory" fields to conform with the new stricter v…
olehermanse Dec 12, 2023
d5656f7
A tribute to Python 3.5
olehermanse Dec 12, 2023
8a7ca28
Fixed validation for when "key" is not required in module input list …
olehermanse Dec 12, 2023
ba459ce
test_validate_index_alias.py: Fixed name of one of the tests / functions
olehermanse Dec 12, 2023
edd4a7b
Improved some validation warning messages
olehermanse Dec 12, 2023
0e23a7b
Fixed typo / spacing by @larsewi
olehermanse Dec 13, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions JSON.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand All @@ -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`.

Expand Down
13 changes: 13 additions & 0 deletions cfbs/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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 [".", "./"]:
Expand Down
27 changes: 27 additions & 0 deletions cfbs/cfbs_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -485,3 +485,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
50 changes: 50 additions & 0 deletions cfbs/cfbs_json.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -9,6 +11,7 @@
TOP_LEVEL_KEYS = ("name", "description", "type", "index", "git", "provides", "build")

MODULE_KEYS = (
"alias",
"name",
"description",
"tags",
Expand Down Expand Up @@ -82,6 +85,53 @@ def __init__(
else:
self.index = Index()

@property
def raw_data(self):
olehermanse marked this conversation as resolved.
Show resolved Hide resolved
"""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
Expand Down
30 changes: 25 additions & 5 deletions cfbs/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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
Expand All @@ -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:
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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", {})
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
3 changes: 3 additions & 0 deletions cfbs/index.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down
4 changes: 0 additions & 4 deletions cfbs/pretty.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading
Loading