diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 5950fb288d..a2f627cb62 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -33,5 +33,16 @@ jobs: with: image: ghcr.io/osbuild/osbuild-ci:latest-202308241910 run: | + # Note that only "test.run.test_stages" runs in parallel because + # the other tests are not sufficiently isolated and will cause + # random failures. But test_stages is the long running one with + # almost 2h. + if [ "${{ matrix.test }}" = "test.run.test_stages" ]; then + # Using 4 workers is a bit arbitrary, "auto" is probably too + # aggressive. + export TEST_WORKERS="-n 4" + # Share the store between the workers speeds things up further + export OSBUILD_TEST_STORE=/var/tmp/osbuild-test-store + fi TEST_CATEGORY="${{ matrix.test }}" \ tox -e "${{ matrix.environment }}" diff --git a/osbuild.spec b/osbuild.spec index a2fc4adeff..d2d48e43e1 100644 --- a/osbuild.spec +++ b/osbuild.spec @@ -1,7 +1,7 @@ %global forgeurl https://github.com/osbuild/osbuild %global selinuxtype targeted -Version: 99 +Version: 100 %forgemeta diff --git a/osbuild/__init__.py b/osbuild/__init__.py index 91a837f04f..19942f675e 100644 --- a/osbuild/__init__.py +++ b/osbuild/__init__.py @@ -10,7 +10,7 @@ from .pipeline import Manifest, Pipeline, Stage -__version__ = "99" +__version__ = "100" __all__ = [ "Manifest", diff --git a/osbuild/testutil/__init__.py b/osbuild/testutil/__init__.py index 09d6d43b24..cdaf849242 100644 --- a/osbuild/testutil/__init__.py +++ b/osbuild/testutil/__init__.py @@ -1,3 +1,8 @@ """ Test related utilities """ +import shutil + + +def has_executable(executable: str) -> bool: + return shutil.which(executable) is not None diff --git a/setup.py b/setup.py index 1a293d4d69..82be9be8ef 100644 --- a/setup.py +++ b/setup.py @@ -2,7 +2,7 @@ setuptools.setup( name="osbuild", - version="99", + version="100", description="A build system for OS images", packages=["osbuild", "osbuild.formats", "osbuild.util"], license='Apache-2.0', diff --git a/stages/org.osbuild.kickstart b/stages/org.osbuild.kickstart index 82f949c173..3994aa0429 100755 --- a/stages/org.osbuild.kickstart +++ b/stages/org.osbuild.kickstart @@ -139,6 +139,13 @@ SCHEMA = r""" "clearpart": { "description": "Removes partitions from the system, prior to creation of new partitions", "type": "object", + "anyOf": [ + {"required": ["all"]}, + {"required": ["drives"]}, + {"required": ["list"]}, + {"required": ["disklabel"]}, + {"required": ["linux"]} + ], "properties": { "all": { "description": "Erases all partitions from the system", @@ -170,6 +177,27 @@ SCHEMA = r""" "type": "boolean" } } + }, + "reboot": { + "description": "Reboot after the installation is successfully completed", + "oneOf": [ + { + "type": "boolean" + }, { + "type": "object", + "additionalProperties": false, + "anyOf": [{"required": ["eject"]}, {"required": ["kexec"]}], + "properties": { + "eject": { + "description": "Attempt to eject the installation media before rebooting", + "type": "boolean" + }, + "kexec": { + "description": "Use this option to reboot into the new system using the kexec", + "type": "boolean" + } + } + }] } } """ @@ -259,6 +287,19 @@ def make_clearpart(options: Dict) -> str: return cmd +def make_reboot(options): + reboot = options.get("reboot", None) + if not reboot: + return "" + cmd = "reboot" + if isinstance(reboot, dict): + if reboot.get("eject"): + cmd += " --eject" + if reboot.get("kexec"): + cmd += " --kexec" + return cmd + + def main(tree, options): path = options["path"].lstrip("/") ostree = options.get("ostree") @@ -302,6 +343,10 @@ def main(tree, options): if clearpart: config += [clearpart] + reboot = make_reboot(options) + if reboot: + config += [reboot] + target = os.path.join(tree, path) base = os.path.dirname(target) os.makedirs(base, exist_ok=True) diff --git a/stages/test/test_kickstart.py b/stages/test/test_kickstart.py index 22a3b78ca3..45d8956e90 100644 --- a/stages/test/test_kickstart.py +++ b/stages/test/test_kickstart.py @@ -1,23 +1,137 @@ #!/usr/bin/python3 import os.path +import subprocess import pytest +import osbuild.meta +from osbuild.testutil import has_executable from osbuild.testutil.imports import import_module_from_path - -@pytest.mark.parametrize("test_input,expected", [ +TEST_INPUT = [ ({"lang": "en_US.UTF-8"}, "lang en_US.UTF-8"), ({"keyboard": "us"}, "keyboard us"), ({"timezone": "UTC"}, "timezone UTC"), - ({"lang": "en_US.UTF-8", - "keyboard": "us", - "timezone": "UTC", - }, - "lang en_US.UTF-8\nkeyboard us\ntimezone UTC"), -]) -def test_kickstart(tmp_path, test_input, expected): + ( + { + "lang": "en_US.UTF-8", + "keyboard": "us", + "timezone": "UTC", + }, + "lang en_US.UTF-8\nkeyboard us\ntimezone UTC", + ), + ( + { + "ostree": { + "osname": "some-osname", + "url": "http://some-ostree-url.com/foo", + "ref": "some-ref", + "remote": "some-remote", + "gpg": True, + }, + "liveimg": { + "url": "some-liveimg-url", + }, + "groups": { + "somegrp": { + "gid": 2337, + }, + }, + "users": { + "someusr": { + "uid": 1337, + "gid": 1337, + "groups": [ + "grp1", + "grp2", + ], + "home": "/other/home/someusr", + "shell": "/bin/ksh", + "password": "$1$notreally", + "key": "ssh-rsa not-really-a-real-key", + }, + }, + }, + "ostreesetup --osname=some-osname --url=http://some-ostree-url.com/foo --ref=some-ref --remote=some-remote\n" + + "liveimg --url some-liveimg-url\ngroup --name somegrp --gid 2337\n" + + "user --name someusr --password $1$notreally --iscrypted --shell /bin/ksh --uid 1337 --gid 1337 --groups grp1,grp2 --homedir /other/home/someusr\n" + + 'sshkey --username someusr "ssh-rsa not-really-a-real-key"', + ), + ({"zerombr": True}, "zerombr"), + ({"clearpart": {"all": True}}, "clearpart --all"), + ( + {"clearpart": {"drives": ["sd*|hd*|vda", "/dev/vdc"]}}, + "clearpart --drives=sd*|hd*|vda,/dev/vdc", + ), + ({"clearpart": {"drives": ["hda"]}}, "clearpart --drives=hda"), + ( + {"clearpart": {"drives": ["disk/by-id/scsi-58095BEC5510947BE8C0360F604351918"]}}, + "clearpart --drives=disk/by-id/scsi-58095BEC5510947BE8C0360F604351918" + ), + ({"clearpart": {"list": ["sda2", "sda3"]}}, "clearpart --list=sda2,sda3"), + ({"clearpart": {"list": ["sda2"]}}, "clearpart --list=sda2"), + ( + {"clearpart": {"disklabel": "some-label"}}, + "clearpart --disklabel=some-label", + ), + ({"clearpart": {"linux": True}}, "clearpart --linux"), + ( + { + "clearpart": { + "all": True, + "drives": ["hda", "hdb"], + "list": ["sda2", "sda3"], + "disklabel": "some-label", + "linux": True, + }, + }, + "clearpart --all --drives=hda,hdb --list=sda2,sda3 --disklabel=some-label --linux", + ), + ( + { + "lang": "en_US.UTF-8", + "keyboard": "us", + "timezone": "UTC", + "zerombr": True, + "clearpart": {"all": True, "drives": ["sd*|hd*|vda", "/dev/vdc"]}, + }, + "lang en_US.UTF-8\nkeyboard us\ntimezone UTC\nzerombr\nclearpart --all --drives=sd*|hd*|vda,/dev/vdc", + ), + # no reboot for an empty dict + ({"reboot": True}, "reboot"), + ({"reboot": {"eject": False}}, "reboot"), + ({"reboot": {"eject": True}}, "reboot --eject"), + ({"reboot": {"kexec": False}}, "reboot"), + ({"reboot": {"kexec": True}}, "reboot --kexec"), + ({"reboot": {"eject": True, "kexec": True}}, "reboot --eject --kexec"), +] + + +def schema_validate_kickstart_stage(test_data): + name = "org.osbuild.kickstart" + root = os.path.join(os.path.dirname(__file__), "../..") + mod_info = osbuild.meta.ModuleInfo.load(root, "Stage", name) + schema = osbuild.meta.Schema(mod_info.get_schema(), name) + test_input = { + "name": "org.osbuild.kickstart", + "options": { + "path": "some-path", + } + } + test_input["options"].update(test_data) + return schema.validate(test_input) + + +@pytest.mark.parametrize("test_input,expected", TEST_INPUT) +def test_kickstart_test_cases_valid(test_input, expected): # pylint: disable=unused-argument + """ ensure all test inputs are valid """ + res = schema_validate_kickstart_stage(test_input) + assert res.valid is True, f"input: {test_input}\nerr: {[e.as_dict() for e in res.errors]}" + + +@pytest.mark.parametrize("test_input,expected", TEST_INPUT) +def test_kickstart_write(tmp_path, test_input, expected): ks_stage_path = os.path.join(os.path.dirname(__file__), "../org.osbuild.kickstart") ks_stage = import_module_from_path("ks_stage", ks_stage_path) @@ -27,6 +141,53 @@ def test_kickstart(tmp_path, test_input, expected): ks_stage.main(tmp_path, options) - with open(os.path.join(tmp_path, ks_path), encoding="utf-8") as fp: + ks_path = os.path.join(tmp_path, ks_path) + with open(ks_path, encoding="utf-8") as fp: ks_content = fp.read() assert ks_content == expected + "\n" + + +@pytest.mark.skipif(not has_executable("ksvalidator"), reason="`ksvalidator` is required") +@pytest.mark.parametrize("test_input,expected", TEST_INPUT) +def test_kickstart_valid(tmp_path, test_input, expected): # pylint: disable=unused-argument + ks_stage_path = os.path.join(os.path.dirname(__file__), "../org.osbuild.kickstart") + ks_stage = import_module_from_path("ks_stage", ks_stage_path) + + ks_path = "kickstart/kfs.cfg" + options = {"path": ks_path} + options.update(test_input) + + ks_stage.main(tmp_path, options) + + ks_path = os.path.join(tmp_path, ks_path) + + # check with pykickstart if the file looks valid + subprocess.check_call(["ksvalidator", ks_path]) + + +@pytest.mark.parametrize( + "test_data,expected_err", + [ + # BAD pattern, ensure some obvious ways to write arbitrary + # kickstart files will not work + ({"clearpart": {}}, "{} is not valid "), + ({"clearpart": {"disklabel": r"\n%pre\necho p0wnd"}}, r"p0wnd' does not match"), + ({"clearpart": {"drives": [" --spaces-dashes-not-allowed"]}}, "' --spaces-dashes-not-allowed' does not match"), + ({"clearpart": {"drives": ["\n%pre not allowed"]}}, "not allowed' does not match"), + ({"clearpart": {"drives": ["no,comma"]}}, "no,comma' does not match"), + ({"clearpart": {"list": ["\n%pre not allowed"]}}, "not allowed' does not match"), + ({"clearpart": {"list": ["no,comma"]}}, "no,comma' does not match"), + ({"clearpart": {"disklabel": "\n%pre not allowed"}}, "not allowed' does not match"), + # schema ensures reboot has at least one option set + ({"reboot": {}}, "{} is not valid under any of the given schemas"), + ({"reboot": "random-string"}, "'random-string' is not valid "), + ({"reboot": {"random": "option"}}, "{'random': 'option'} is not valid "), + ], +) +def test_schema_validation_bad_apples(test_data, expected_err): + res = schema_validate_kickstart_stage(test_data) + + assert res.valid is False + assert len(res.errors) == 1 + err_msgs = [e.as_dict()["message"] for e in res.errors] + assert expected_err in err_msgs[0] diff --git a/tox.ini b/tox.ini index 031a4d90f6..3e0bdfce5d 100644 --- a/tox.ini +++ b/tox.ini @@ -13,10 +13,14 @@ labels = description = "run osbuild unit tests" deps = pytest + pytest-xdist jsonschema mako iniparse pyyaml + pykickstart + # required by pykickstart but not pulled in automatically :/ + requests setenv = LINTABLES = osbuild/ assemblers/* devices/* inputs/* mounts/* runners/* sources/* stages/*.* stages/test/*.py @@ -26,7 +30,7 @@ passenv = TEST_CATEGORY commands = - bash -c 'python -m pytest --pyargs --rootdir=. {env:TEST_CATEGORY}' + bash -c 'python -m pytest --pyargs --rootdir=. {env:TEST_CATEGORY} {env:TEST_WORKERS}' allowlist_externals = bash