From 9af7c9b279a098cc0fc09c20a46bf38fa5d54bf1 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Fri, 1 Dec 2023 16:44:44 +0100 Subject: [PATCH] meta: add .meta.json schema validation --- osbuild/meta.py | 55 +++++++++++++++++++++++- test/mod/test_meta.py | 99 +++++++++++++++++++++++++++++++++++-------- 2 files changed, 135 insertions(+), 19 deletions(-) diff --git a/osbuild/meta.py b/osbuild/meta.py index bef0bc65..28ce0da8 100644 --- a/osbuild/meta.py +++ b/osbuild/meta.py @@ -275,6 +275,54 @@ class Schema: return self.check().valid +META_JSON_SCHEMA = { + "type": "object", + "additionalProperties": False, + "propertyNames": { + "not": { + "const": "description", + }, + }, + "required": ["summary", "description"], + "oneOf": [ + { + "required": [ + "schema" + ] + }, + { + "required": [ + "schema_2" + ] + }, + ], + "properties": { + "summary": { + "type": "string", + }, + "description": { + "type": "array", + "minItems": 1, + "items": { + "type": "string", + }, + }, + "capabilities": { + "type": "array", + "items": { + "type": "string", + }, + }, + "schema": { + "type": "object", + }, + "schema_2": { + "type": "object", + } + } +} + + class ModuleInfo: """Meta information about a stage @@ -417,7 +465,6 @@ class ModuleInfo: try: return cls._load_from_json(path, klass, name) except FileNotFoundError: - # should we print a deprecation warning here? pass return cls._load_from_py(path, klass, name) @@ -427,6 +474,12 @@ class ModuleInfo: with open(path + meta_json_suffix, encoding="utf-8") as fp: meta = json.load(fp) + schema = Schema(META_JSON_SCHEMA, "meta.json validator") + res = schema.validate(meta) + if not res.valid: + # XXX: should we raise an exception instead? + return None + long_description = meta.get("description", "no description provided") if isinstance(long_description, list): long_description = "\n".join(long_description) diff --git a/test/mod/test_meta.py b/test/mod/test_meta.py index 1741e9d7..f1bd89f5 100644 --- a/test/mod/test_meta.py +++ b/test/mod/test_meta.py @@ -172,9 +172,29 @@ def test_schema(): assert res -def make_fake_meta_json(tmp_path, name): +def make_fake_meta_json(tmp_path, name, version): meta_json_path = pathlib.Path(f"{tmp_path}/stages/{name}.meta.json") meta_json_path.parent.mkdir(exist_ok=True) + if version == 1: + schema_part = """ + "schema": { + "properties": { + "json_filename": { + "type": "string" + } + } + } + """ + elif version == 2: + schema_part = """ + "schema_2": { + "json_devices": { + "type": "object" + } + } + """ + else: + pytest.fail(f"unexpected schema version {version}") meta_json_path.write_text(""" { "summary": "some json summary", @@ -183,20 +203,9 @@ def make_fake_meta_json(tmp_path, name): "with newlines" ], "capabilities": ["CAP_MAC_ADMIN", "CAP_BIG_MAC"], - "schema": { - "properties": { - "json_filename": { - "type": "string" - } - } - }, - "schema_2": { - "json_devices": { - "type": "object" - } - } + %s } - """.replace("\n", " "), encoding="utf-8") + """.replace("\n", " ") % schema_part, encoding="utf-8") return meta_json_path @@ -213,14 +222,14 @@ def make_fake_py_module(tmp_path, name): def test_load_from_json(tmp_path): - make_fake_meta_json(tmp_path, "org.osbuild.noop") + make_fake_meta_json(tmp_path, "org.osbuild.noop", version=1) modinfo = osbuild.meta.ModuleInfo.load(tmp_path, "Stage", "org.osbuild.noop") assert modinfo.desc == "some json summary" assert modinfo.info == "long text\nwith newlines" assert modinfo.caps == ["CAP_MAC_ADMIN", "CAP_BIG_MAC"] assert modinfo.opts == { "1": {"properties": {"json_filename": {"type": "string"}}}, - "2": {"json_devices": {"type": "object"}}, + "2": {}, } @@ -237,13 +246,67 @@ def test_load_from_py(tmp_path): def test_load_from_json_prefered(tmp_path): - make_fake_meta_json(tmp_path, "org.osbuild.noop") + make_fake_meta_json(tmp_path, "org.osbuild.noop", version=2) make_fake_py_module(tmp_path, "org.osbuild.noop") modinfo = osbuild.meta.ModuleInfo.load(tmp_path, "Stage", "org.osbuild.noop") assert modinfo.desc == "some json summary" assert modinfo.info == "long text\nwith newlines" assert modinfo.caps == ["CAP_MAC_ADMIN", "CAP_BIG_MAC"] assert modinfo.opts == { - "1": {"properties": {"json_filename": {"type": "string"}}}, + "1": {}, "2": {"json_devices": {"type": "object"}}, } + + +def test_load_from_json_validates(tmp_path): + meta_json_path = pathlib.Path(f"{tmp_path}/stages/org.osbuild.noop.meta-json") + meta_json_path.parent.mkdir(exist_ok=True) + # XXX: do a more direct test here that also validates the various + # error conditions + meta_json_path.write_text("""{"not": "following schema"}""") + modinfo = osbuild.meta.ModuleInfo.load(tmp_path, "Stage", "org.osbuild.noop") + assert modinfo is None + + +@pytest.mark.parametrize("test_input,expected_err", [ + # happy + ( + {"summary": "some", "description": ["many", "strings"], "schema": {}}, ""), ( + {"summary": "some", "description": ["many", "strings"], "schema_2": {}}, ""), ( + {"summary": "some", "description": ["many", "strings"], "schema_2": {}, "capabilities": []}, ""), ( + {"summary": "some", "description": ["many", "strings"], "schema_2": {}, "capabilities": ["cap1", "cap2"]}, ""), + # unhappy + ( + # no description + {"summary": "some", "schema": {}}, + "'description' is a required property", + ), ( + # no summary + {"description": ["yes"], "schema": {}}, + "'summary' is a required property", + ), ( + # schema{,_2} missing + {"summary": "some", "description": ["many", "strings"]}, + " is not valid", + ), + ( + # both schema{,_2} + {"summary": "some", "description": ["many", "strings"], + "schema": {}, "schema_2": {}}, + " is valid under each ", + ), ( + # capabilities of wrong type + {"summary": "some", "description": ["many", "strings"], "schema": {}, + "capabilities": [1, "cap1"]}, + " is not of type 'string'", + ), +]) +def test_meta_json_validation_schema(test_input, expected_err): + schema = osbuild.meta.Schema(osbuild.meta.META_JSON_SCHEMA, "meta.json validator") + res = schema.validate(test_input) + errs = [e.as_dict() for e in res.errors] + if expected_err: + assert len(errs) == 1, f"unexpected error {errs}" + assert expected_err in errs[0]["message"] + else: + assert not errs