diff --git a/JSON.md b/JSON.md index 112c5bc0..4ad4a639 100644 --- a/JSON.md +++ b/JSON.md @@ -222,7 +222,7 @@ These are copies of the module directories, where it's more "safe" to do things ## All available build steps The build steps below manipulate the temporary files in the steps directories and write results to the output policy set, in `out/masterfiles`. -Unless otherwise noted, all steps are run inside the module's folder (`out/steps/...`) with sources / file paths relative to that folder, and targets / destinations mentioned below are relative to the output policy set (`out/masterfiles`, which in the end will be deployed as `/var/cfengine/masterfiles`). +Unless otherwise noted, all steps are run inside the module's folder (`out/steps/...`) with sources / file paths relative to that folder, and targets / destinations mentioned below are relative to the output policy set (`out/masterfiles`, which in the end will be deployed as `/var/cfengine/masterfiles`). In `cfbs.json`'s `"steps"`, the build step name must be separated from the rest of the build step by a regular space. * `copy ` * Copy a single file or a directory recursively. diff --git a/cfbs/build.py b/cfbs/build.py index ebfc1121..9b8ac196 100644 --- a/cfbs/build.py +++ b/cfbs/build.py @@ -1,18 +1,17 @@ import os import logging as log +from typing import List, Tuple from cfbs.utils import ( canonify, cp, deduplicate_def_json, find, - is_valid_arg_count, merge_json, mkdir, pad_right, read_json, rm, sh, - split_command, strip_left, touch, user_error, @@ -73,8 +72,31 @@ def _generate_augment(module_name, input_data): return augment +def split_build_step(command) -> Tuple[str, List[str]]: + terms = command.split(" ") + operation, args = terms[0], terms[1:] + return operation, args + + +def step_has_valid_arg_count(args, expected): + actual = len(args) + + if type(expected) is int: + if actual != expected: + return False + + 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: + return False + + return True + + def _perform_build_step(module, step, max_length): - operation, args = split_command(step) + operation, args = split_build_step(step) source = module["_directory"] counter = module["_counter"] destination = "out/masterfiles" @@ -245,7 +267,7 @@ def perform_build_steps(config) -> int: # mini-validation for module in config.get("build", []): for step in module["steps"]: - operation, args = split_command(step) + operation, args = split_build_step(step) if step.split() != [operation] + args: user_error( @@ -258,7 +280,7 @@ def perform_build_steps(config) -> int: expected = AVAILABLE_BUILD_STEPS[operation] actual = len(args) - if not is_valid_arg_count(args, expected): + if not step_has_valid_arg_count(args, expected): if type(expected) is int: user_error( "The `%s` build step expects %d arguments, %d were given" diff --git a/cfbs/utils.py b/cfbs/utils.py index ca042e00..4729557e 100644 --- a/cfbs/utils.py +++ b/cfbs/utils.py @@ -5,8 +5,6 @@ import copy import subprocess import hashlib -import logging as log -from typing import List, Tuple import urllib import urllib.request # needed on some platforms from collections import OrderedDict @@ -79,35 +77,12 @@ def cp(src, dst): sh("rsync -r %s/ %s" % (src, dst)) -def pad_left(s, n) -> int: - return s if len(s) >= n else " " * (n - len(s)) + s +def pad_left(s, n): + return s.rjust(n) -def pad_right(s, n) -> int: - return s if len(s) >= n else s + " " * (n - len(s)) - - -def split_command(command) -> Tuple[str, List[str]]: - terms = command.split(" ") - operation, args = terms[0], terms[1:] - return operation, args - - -def is_valid_arg_count(args, expected): - actual = len(args) - - if type(expected) is int: - if actual != expected: - return False - - 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: - return False - - return True +def pad_right(s, n): + return s.ljust(n) def user_error(msg: str): @@ -137,12 +112,14 @@ def item_index(iterable, item, extra_at_end=True): def strip_right(string, ending): + # can be replaced with str.removesuffix from Python 3.9 onwards if not string.endswith(ending): return string return string[0 : -len(ending)] def strip_left(string, beginning): + # can be replaced with str.removeprefix from Python 3.9 onwards if not string.startswith(beginning): return string return string[len(beginning) :] @@ -163,7 +140,7 @@ def save_file(path, data): f.write(data) -def read_json(path): +def read_json(path) -> OrderedDict: try: with open(path, "r") as f: return json.loads(f.read(), object_pairs_hook=OrderedDict) @@ -172,7 +149,7 @@ def read_json(path): except NotADirectoryError: return None except json.decoder.JSONDecodeError as ex: - print("Error reading json file {} : {}".format(path, ex)) + print("Error reading json file '{}': {}".format(path, ex)) sys.exit(1) @@ -272,15 +249,24 @@ def is_cfbs_repo() -> bool: def immediate_subdirectories(path): - return [f.name for f in os.scandir(path) if f.is_dir()] + l = [f.name for f in os.scandir(path) if f.is_dir()] + + # `os.scandir` returns the entries in arbitrary order, so sort for determinism + l = sorted(l) + + return l def immediate_files(path): - return [f.name for f in os.scandir(path) if not f.is_dir()] + l = [f.name for f in os.scandir(path) if not f.is_dir()] + + # `os.scandir` returns the entries in arbitrary order, so sort for determinism + l = sorted(l) + + return l def path_append(dir, subdir): - dir = os.path.abspath(os.path.expanduser(dir)) return dir if not subdir else os.path.join(dir, subdir) @@ -296,7 +282,10 @@ def are_paths_equal(path_a, path_b) -> bool: def cfengine_dir(subdir=None): - return path_append("~/.cfengine/", subdir) + CFENGINE_DIR = "~/.cfengine/" + cfengine_dir_abspath = os.path.abspath(os.path.expanduser(CFENGINE_DIR)) + + return path_append(cfengine_dir_abspath, subdir) def cfbs_dir(append=None) -> str: diff --git a/cfbs/validate.py b/cfbs/validate.py index 5bc7751e..3f9fa553 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -1,13 +1,15 @@ import argparse -import json import sys import re from collections import OrderedDict -from cfbs.utils import is_valid_arg_count, is_a_commit_hash, split_command, user_error +from cfbs.utils import ( + is_a_commit_hash, + user_error, +) from cfbs.pretty import TOP_LEVEL_KEYS, MODULE_KEYS from cfbs.cfbs_config import CFBSConfig -from cfbs.build import AVAILABLE_BUILD_STEPS +from cfbs.build import AVAILABLE_BUILD_STEPS, step_has_valid_arg_count, split_build_step class CFBSValidationError(Exception): @@ -266,7 +268,7 @@ def validate_steps(name, module): raise CFBSValidationError( name, '"steps" must be a list of non-empty / non-whitespace strings' ) - operation, args = split_command(step) + operation, args = split_build_step(step) if not operation in AVAILABLE_BUILD_STEPS: x = ", ".join(AVAILABLE_BUILD_STEPS) raise CFBSValidationError( @@ -276,7 +278,7 @@ def validate_steps(name, module): ) expected = AVAILABLE_BUILD_STEPS[operation] actual = len(args) - if not is_valid_arg_count(args, expected): + if not step_has_valid_arg_count(args, expected): if type(expected) is int: raise CFBSValidationError( name, diff --git a/tests/sample/sample_dir/sample_file_1.txt b/tests/sample/sample_dir/sample_file_1.txt new file mode 100644 index 00000000..4c22e6e0 --- /dev/null +++ b/tests/sample/sample_dir/sample_file_1.txt @@ -0,0 +1,2 @@ +sample_string +123 \ No newline at end of file diff --git a/tests/sample/sample_dir/sample_file_2.txt b/tests/sample/sample_dir/sample_file_2.txt new file mode 100644 index 00000000..e69de29b diff --git a/tests/sample/sample_dir/sample_subdir_A/sample_file_3.txt b/tests/sample/sample_dir/sample_subdir_A/sample_file_3.txt new file mode 100644 index 00000000..e69de29b diff --git a/tests/sample/sample_dir/sample_subdir_B/sample_subdir_C/sample_file_4.txt b/tests/sample/sample_dir/sample_subdir_B/sample_subdir_C/sample_file_4.txt new file mode 100644 index 00000000..e69de29b diff --git a/tests/sample/sample_json.json b/tests/sample/sample_json.json new file mode 100644 index 00000000..8d4419b3 --- /dev/null +++ b/tests/sample/sample_json.json @@ -0,0 +1,7 @@ +{ + "a": 1, + "b": { + "c": "value", + "d": [2, "string"] + } +} \ No newline at end of file diff --git a/tests/shell/035_cfbs_build_compatibility_1.sh b/tests/shell/035_cfbs_build_compatibility_1.sh index 0d7b5b93..8a3fc142 100644 --- a/tests/shell/035_cfbs_build_compatibility_1.sh +++ b/tests/shell/035_cfbs_build_compatibility_1.sh @@ -6,7 +6,7 @@ cd ./tmp/ rm -rf ./* # The purpose of this test is to ensure that older CFEngine Build projects -# still buid in newer versions of cfbs +# still build in newer versions of cfbs # The below cfbs.json file was generated using cfbs 3.2.7 # which is the cfbs version shipped with CFEngine Enterprise 3.21.0 diff --git a/tests/shell/039_add_added_by_field_update_1.sh b/tests/shell/039_add_added_by_field_update_1.sh new file mode 100644 index 00000000..4e89f82a --- /dev/null +++ b/tests/shell/039_add_added_by_field_update_1.sh @@ -0,0 +1,23 @@ +set -e +set -x +cd tests/ +mkdir -p ./tmp/ +cd ./tmp/ +touch cfbs.json && rm cfbs.json +rm -rf .git + +cfbs --non-interactive init + +# Ensure adding a module during initialization is treated as adding manually +cat cfbs.json | grep -F "added_by" | grep -F "cfbs init" +# TODO: the case of custom non-masterfiles module(s) should also be tested + +# Manually adding a module then manually adding its dependency should update the latter's `"added_by"` field in `cfbs.json` + +cfbs --non-interactive add package-method-winget +cat cfbs.json | grep -F "added_by" | grep -F "package-method-winget" +[ "$(cat cfbs.json | grep -F "added_by" | grep -F "cfbs add" -c)" -eq 1 ] + +cfbs --non-interactive add powershell-execution-policy +! ( cat cfbs.json | grep -F "added_by" | grep -F "package-method-winget" ) +[ "$(cat cfbs.json | grep -F "added_by" | grep -F "cfbs add" -c)" -eq 2 ] diff --git a/tests/shell/040_add_added_by_field_update_2.sh b/tests/shell/040_add_added_by_field_update_2.sh new file mode 100644 index 00000000..1ede9509 --- /dev/null +++ b/tests/shell/040_add_added_by_field_update_2.sh @@ -0,0 +1,17 @@ +set -e +set -x +cd tests/ +mkdir -p ./tmp/ +cd ./tmp/ +touch cfbs.json && rm cfbs.json +rm -rf .git + +cfbs --non-interactive init + +# Manually adding a dependency of a module then manually adding that module should not update the latter's `"added_by"` field in `cfbs.json` + +cfbs --non-interactive add powershell-execution-policy +cat cfbs.json | grep -F "added_by" | grep -F "cfbs add" + +cfbs --non-interactive add package-method-winget +! ( cat cfbs.json | grep -F "added_by" | grep -F "package-method-winget" ) diff --git a/tests/shell/all.sh b/tests/shell/all.sh index c6b55ec2..58a2fe48 100644 --- a/tests/shell/all.sh +++ b/tests/shell/all.sh @@ -42,5 +42,7 @@ 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 bash tests/shell/038_global_dir.sh +bash tests/shell/039_add_added_by_field_update_1.sh +bash tests/shell/040_add_added_by_field_update_2.sh echo "All cfbs shell tests completed successfully!" diff --git a/tests/test_utils.py b/tests/test_utils.py index edba41a6..cc5bb6bd 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -1,14 +1,77 @@ +from collections import OrderedDict from cfbs.utils import ( + are_paths_equal, canonify, deduplicate_def_json, + deduplicate_list, dict_diff, + dict_sorted_by_key, file_sha256, + immediate_files, + immediate_subdirectories, + is_a_commit_hash, merge_json, loads_bundlenames, + pad_left, + pad_right, + path_append, + read_file, + read_json, string_sha256, + strip_left, + strip_right, ) +def test_pad_left(): + s = "module_name" + n = 20 + + assert pad_left(s, n) == " module_name" + + +def test_pad_right(): + s = "module_name" + n = 20 + + assert pad_right(s, n) == "module_name " + + +def test_strip_right(): + s = "abab" + + assert strip_right(s, "ab") == "ab" + assert strip_right(s, "a") == "abab" + + +def test_strip_left(): + s = "abab" + + assert strip_left(s, "ab") == "ab" + assert strip_left(s, "b") == "abab" + + +def test_read_file(): + file_path = "tests/sample/sample_dir/sample_file_1.txt" + expected_str = "sample_string\n123" + nonpath = "tests/sample/sample_dir/sample_file_doesnt_exist.txt" + + assert read_file(file_path) == expected_str + assert read_file(nonpath) is None + + +def test_read_json(): + json_path = "tests/sample/sample_json.json" + expected_dict = OrderedDict( + [("a", 1), ("b", OrderedDict([("c", "value"), ("d", [2, "string"])]))] + ) + + assert read_json(json_path) == expected_dict + + assert read_json("tests/thisfiledoesntexist.json") is None + assert read_json("tests/thisdirdoesntexist/file.json") is None + + def test_merge_json(): original = {"classes": {"services_autorun": ["any"]}} extras = { @@ -142,6 +205,20 @@ def test_deduplicate_def_json(): assert deduplicated == expected +def test_deduplicate_list(): + l = [1, 2, 3, 3, 1, 4] + + assert deduplicate_list(l) == [1, 2, 3, 4] + + +def test_dict_sorted_by_key(): + d = {"b": 1, "c": 3, "a": 2} + + expected_dict = OrderedDict([("a", 2), ("b", 1), ("c", 3)]) + + assert dict_sorted_by_key(d) == expected_dict + + def test_dict_diff(): A = {"A": "a", "B": "b", "C": "c"} B = {"A": "a", "B": "c", "D": "d"} @@ -149,6 +226,34 @@ def test_dict_diff(): assert dict_diff(A, B) == (["C"], ["D"], [("B", "b", "c")]) +def test_immediate_subdirectories(): + path = "tests/sample/sample_dir" + expected = ["sample_subdir_A", "sample_subdir_B"] + + assert immediate_subdirectories(path) == expected + + +def test_immediate_files(): + path = "tests/sample/sample_dir" + expected = ["sample_file_1.txt", "sample_file_2.txt"] + + assert immediate_files(path) == expected + + +def test_path_append(): + path = "tests/sample/sample_dir" + + assert path_append(path, "abc") == path + "/abc" + assert path_append(path, None) == path + + +def test_are_paths_equal(): + path_a = "abc" + path_b = "abc/..//abc/" + + assert are_paths_equal(path_a, path_b) + + def test_string_sha256(): s = "cfbs/masterfiles/" checksum = "9e63d3266f80328fb6547b3462e81ab55b13f689d6b0944e242e2b3a0f3a32a3" @@ -163,6 +268,15 @@ def test_file_sha256(): assert file_sha256(file_path) == checksum +def test_is_a_commit_hash(): + assert is_a_commit_hash("304d123ac7ff50714a1eb57077acf159f923c941") == True + sha256_hash = "98142d6fa7e2e5f0942b0a215c1c4b976e7ae2ee5edb61cef974f1ba6756cbbc" + assert is_a_commit_hash(sha256_hash) == True + # at least currently, commit cannot be a shortened hash + assert is_a_commit_hash("4738c43") == False + assert is_a_commit_hash("") == False + + def test_canonify(): assert canonify("Hello CFEngine!") == "Hello_CFEngine_" assert canonify("/etc/os-release") == "_etc_os_release"