From a0497becbb270cc3641cb75c4f3ace6bc69136bf Mon Sep 17 00:00:00 2001 From: Erik Skultety Date: Thu, 11 Apr 2024 15:43:08 +0200 Subject: [PATCH] package_managers: rpm: Allow specifying DNF options via CLI input JSON Introduce a new input JSON attribute 'options' for the PoC rpm package manager allowing consumers to pass arbitrary DNF repo configuration options that would be applied later through the inject-files command. The general schema for the input JSON is as follows: { type: path: options: { : { ..: Union[str, Any] } } } which translated to DNF repo options for RPM might then look like: { type: rpm path: . options: { dnf: { repoid1: {gpgcheck: 0, deltarpm: 1}, repoid2: {fastestmirro: 1, sslverify: 0} } } } This implementation is trying to be generic enough to be potentially later extended to other package managers as well which also means that due to the fairly complex (multi-level nested dictionary) input a new intermediary model _DNFOptions was needed to be introduced in this patch, but most importantly a custom 'before' model validator for this model had to be implemented to generate stable and deterministic errors which could be reliably unit-tested (default pydantic error messages for such complex structures are madness). Worth noting would be that unless options were specified, they would not appear in the build-config.json --> exclude_none=True Signed-off-by: Erik Skultety --- cachi2/core/models/input.py | 69 ++++++++++++++- cachi2/core/models/output.py | 5 +- cachi2/core/package_managers/rpm/main.py | 46 +++++++++- cachi2/core/resolver.py | 1 + cachi2/interface/cli.py | 8 +- tests/unit/models/test_input.py | 53 +++++++++++- tests/unit/package_managers/test_rpm.py | 102 ++++++++++++++++++++--- 7 files changed, 264 insertions(+), 20 deletions(-) diff --git a/cachi2/core/models/input.py b/cachi2/core/models/input.py index f278a33b0..709671c83 100644 --- a/cachi2/core/models/input.py +++ b/cachi2/core/models/input.py @@ -1,5 +1,17 @@ from pathlib import Path -from typing import TYPE_CHECKING, Annotated, Callable, ClassVar, Literal, Optional, TypeVar, Union +from typing import ( + TYPE_CHECKING, + Annotated, + Any, + Callable, + ClassVar, + Dict, + List, + Literal, + Optional, + TypeVar, + Union, +) import pydantic @@ -65,6 +77,60 @@ def _path_is_relative(cls, path: Path) -> Path: return check_sane_relpath(path) +class _DNFOptions(pydantic.BaseModel, extra="forbid"): + """DNF options model. + + DNF options can be provided via 2 'streams': + 1) global /etc/dnf/dnf.conf OR + 2) /etc/yum.repos.d/.repo files + + Config options are specified via INI format based on sections. There are 2 types of sections: + 1) global 'main' - either global repo options or DNF control-only options + - NOTE: there must always ever be a single "main" section + + 2) sections - options tied specifically to a given defined repo + + [1] https://man7.org/linux/man-pages/man5/dnf.conf.5.html + """ + + # Don't model all known DNF options for validation purposes - it's user's responsibility! + dnf: Dict[Union[Literal["main"], str], Dict[str, Any]] + + @pydantic.model_validator(mode="before") + def _validate_dnf_options(cls, data: Any, info: pydantic.ValidationInfo) -> Optional[Dict]: + """Fail if the user passes unexpected configuration options namespace.""" + + def _raise_unexpected_type(repr_: str, *prefixes: str) -> None: + loc = ".".join(prefixes + (repr_,)) + raise ValueError(f"Unexpected data type for '{loc}' in input JSON: expected 'dict'") + + prefixes: List[str] = ["options"] + + if not data: + return None + + if not isinstance(data, dict): + _raise_unexpected_type(data, *prefixes) + + if "dnf" not in data: + raise ValueError(f"Missing required namespace attribute in '{data}': 'dnf'") + + if diff := set(cls.model_fields) - set(data.keys()): + raise ValueError(f"Extra attributes passed in '{data}': {diff}") + + prefixes.append("dnf") + options_scope = data["dnf"] + if not isinstance(options_scope, dict): + _raise_unexpected_type(options_scope, *prefixes) + + for repo, repo_options in options_scope.items(): + prefixes.append(repo) + if not isinstance(repo_options, dict): + _raise_unexpected_type(repo_options, *prefixes) + + return data + + class GomodPackageInput(_PackageInputBase): """Accepted input for a gomod package.""" @@ -104,6 +170,7 @@ class RpmPackageInput(_PackageInputBase): """Accepted input for a rpm package.""" type: Literal["rpm"] + options: Optional[Union[_DNFOptions]] = None class YarnPackageInput(_PackageInputBase): diff --git a/cachi2/core/models/output.py b/cachi2/core/models/output.py index 60e1069bc..a555a243d 100644 --- a/cachi2/core/models/output.py +++ b/cachi2/core/models/output.py @@ -1,7 +1,7 @@ import logging import string from pathlib import Path -from typing import Dict, Literal, Optional, Set +from typing import Any, Dict, Literal, Optional, Set import pydantic @@ -133,6 +133,7 @@ class BuildConfig(pydantic.BaseModel): environment_variables: list[EnvironmentVariable] = [] project_files: list[ProjectFile] = [] + options: Optional[Dict[str, Any]] = None @pydantic.field_validator("environment_variables") def _unique_env_vars(cls, env_vars: list[EnvironmentVariable]) -> list[EnvironmentVariable]: @@ -170,6 +171,7 @@ def from_obj_list( components: list[Component], environment_variables: Optional[list[EnvironmentVariable]] = None, project_files: Optional[list[ProjectFile]] = None, + options: Optional[Dict[str, Any]] = None, ) -> "RequestOutput": """Create a RequestOutput from components, environment variables and project files.""" if environment_variables is None: @@ -183,5 +185,6 @@ def from_obj_list( build_config=BuildConfig( environment_variables=environment_variables, project_files=project_files, + options=options, ), ) diff --git a/cachi2/core/package_managers/rpm/main.py b/cachi2/core/package_managers/rpm/main.py index 9511a3321..99748730d 100644 --- a/cachi2/core/package_managers/rpm/main.py +++ b/cachi2/core/package_managers/rpm/main.py @@ -5,7 +5,7 @@ from configparser import ConfigParser from os import PathLike from pathlib import Path -from typing import Any, Union, no_type_check +from typing import Any, Dict, Optional, Union, no_type_check from urllib.parse import quote import yaml @@ -74,14 +74,39 @@ def write(self, fileobject, space_around_delimiters=True) -> None: def fetch_rpm_source(request: Request) -> RequestOutput: """Process all the rpm source directories in a request.""" components: list[Component] = [] + options: Dict[str, Any] = {} + noptions = 0 + for package in request.rpm_packages: path = request.source_dir.join_within_root(package.path) components.extend(_resolve_rpm_project(path, request.output_dir)) + # FIXME: this is only ever good enough for a PoC, but needs to be handled properly in the + # future. + # It's unlikely that a project would be split into multiple packages, i.e. supplying + # multiple rpms.lock.yaml files. We'd end up generating a single .repo file anyway, + # however, although trying to pass conflicting options to DNF for identical repoids via the + # input JSON doesn't make much sense from practical perspective (i.e. there's going to be a + # single .repo file) the CLI technically allows it in the input JSON. + # We're deliberately taking the easy route here by only assuming the "last" set of options + # we found in the input JSON instead of doing a deep merge of all the nested dicts. + # Nevertheless, we'll at least emit a warning at the end so that the user is informed + if package.options and package.options.dnf: + options = package.options.model_dump() + noptions += 1 + + if noptions > 1: + log.warning( + "Multiple sets of DNF options detected on the input: " + "Only one input RPM project package can specify extra DNF options, " + "the last one seen will take effect" + ) + return RequestOutput.from_obj_list( components=components, environment_variables=[], project_files=[], + options={"rpm": options} if options else None, ) @@ -272,7 +297,7 @@ def inject_files_post(from_output_dir: Path, for_output_dir: Path, **kwargs: Any """Run extra tasks for the RPM package manager (callback method) within `inject-files` cmd.""" if Path.exists(from_output_dir.joinpath(DEFAULT_PACKAGE_DIR)): _generate_repos(from_output_dir) - _generate_repofiles(from_output_dir, for_output_dir) + _generate_repofiles(from_output_dir, for_output_dir, kwargs.get("options")) def _generate_repos(from_output_dir: Path) -> None: @@ -298,7 +323,9 @@ def _createrepo(reponame: str, repodir: Path) -> None: log.debug(stdout) -def _generate_repofiles(from_output_dir: Path, for_output_dir: Path) -> None: +def _generate_repofiles( + from_output_dir: Path, for_output_dir: Path, options: Optional[Dict] = None +) -> None: """ Generate templates of repofiles for all arches. @@ -308,6 +335,13 @@ def _generate_repofiles(from_output_dir: Path, for_output_dir: Path) -> None: Repofiles are not directly created from the templates in this method - templates are stored in the project file. """ + dnf_options = None + dnf_options_repos = None + + if options: + dnf_options = options.get("rpm", {}).get("dnf", {}) + dnf_options_repos = dnf_options.keys() + package_dir = from_output_dir.joinpath(DEFAULT_PACKAGE_DIR) for arch in package_dir.iterdir(): if not arch.is_dir(): @@ -321,6 +355,12 @@ def _generate_repofiles(from_output_dir: Path, for_output_dir: Path) -> None: repoid = entry.name repofile[repoid] = {} + # TODO: purposefully ignoring the fact that options might be passed within the "main" + # context of DNF options which would mean we'd have to generate a dnf.conf since such + # options are global, skipping that for now + if dnf_options and dnf_options_repos and repoid in dnf_options_repos: + repofile[repoid] = dnf_options[repoid] + localpath = for_output_dir.joinpath(DEFAULT_PACKAGE_DIR, arch.name, repoid) repofile[repoid]["baseurl"] = f"file://{localpath}" diff --git a/cachi2/core/resolver.py b/cachi2/core/resolver.py index 27e27f325..2f18d8147 100644 --- a/cachi2/core/resolver.py +++ b/cachi2/core/resolver.py @@ -83,6 +83,7 @@ def _merge_outputs(outputs: Iterable[RequestOutput]) -> RequestOutput: components=components, environment_variables=env_vars, project_files=project_files, + options=output.build_config.options if output.build_config.options else None, ) diff --git a/cachi2/interface/cli.py b/cachi2/interface/cli.py index 5f4eb0954..b53c93d9c 100644 --- a/cachi2/interface/cli.py +++ b/cachi2/interface/cli.py @@ -260,7 +260,7 @@ def combine_option_and_json_flags(json_flags: list[Flag]) -> list[str]: request.output_dir.path.mkdir(parents=True, exist_ok=True) request.output_dir.join_within_root(".build-config.json").path.write_text( - request_output.build_config.model_dump_json(indent=2) + request_output.build_config.model_dump_json(indent=2, exclude_none=True) ) sbom = request_output.generate_sbom() @@ -339,7 +339,11 @@ def inject_files( content = project_file.resolve_content(output_dir=for_output_dir) project_file.abspath.write_text(content) - inject_files_post(from_output_dir=from_output_dir, for_output_dir=for_output_dir) + inject_files_post( + from_output_dir=from_output_dir, + for_output_dir=for_output_dir, + options=fetch_deps_output.options, + ) def _get_build_config(output_dir: Path) -> BuildConfig: diff --git a/tests/unit/models/test_input.py b/tests/unit/models/test_input.py index 5f45c2344..2a6cb0f8e 100644 --- a/tests/unit/models/test_input.py +++ b/tests/unit/models/test_input.py @@ -12,6 +12,7 @@ PackageInput, PipPackageInput, Request, + RpmPackageInput, parse_user_input, ) from cachi2.core.rooted_path import RootedPath @@ -60,6 +61,35 @@ class TestPackageInput: "allow_binary": True, }, ), + ( + {"type": "rpm"}, + { + "type": "rpm", + "path": Path("."), + "options": None, + }, + ), + ( + { + "type": "rpm", + "options": { + "dnf": { + "main": {"best": True, "debuglevel": 2}, + "foorepo": {"arch": "x86_64", "enabled": True}, + } + }, + }, + { + "type": "rpm", + "path": Path("."), + "options": { + "dnf": { + "main": {"best": True, "debuglevel": 2}, + "foorepo": {"arch": "x86_64", "enabled": True}, + } + }, + }, + ), ], ) def test_valid_packages(self, input_data: dict[str, Any], expect_data: dict[str, Any]) -> None: @@ -113,6 +143,26 @@ def test_valid_packages(self, input_data: dict[str, Any], expect_data: dict[str, r"none is not an allowed value", id="pip_no_requirements_build_files", ), + pytest.param( + {"type": "rpm", "options": "bad_type"}, + r"Unexpected data type for 'options.bad_type' in input JSON", + id="rpm_bad_options_type", + ), + pytest.param( + {"type": "rpm", "options": {"unknown": "foo"}}, + r"Missing required namespace attribute in '{\'unknown\': \'foo\'}': 'dnf'", + id="rpm_missing_required_namespace_dnf", + ), + pytest.param( + {"type": "rpm", "options": {"dnf": "bad_type"}}, + r"Unexpected data type for 'options.dnf.bad_type' in input JSON", + id="rpm_bad_type_for_dnf_namespace", + ), + pytest.param( + {"type": "rpm", "options": {"dnf": {"repo": "bad_type"}}}, + r"Unexpected data type for 'options.dnf.repo.bad_type' in input JSON", + id="rpm_bad_type_for_dnf_options", + ), ], ) def test_invalid_packages(self, input_data: dict[str, Any], expect_error: str) -> None: @@ -165,11 +215,12 @@ def test_valid_request(self, tmp_path: Path) -> None: assert isinstance(request.output_dir, RootedPath) def test_packages_properties(self, tmp_path: Path) -> None: - packages = [{"type": "gomod"}, {"type": "npm"}, {"type": "pip"}] + packages = [{"type": "gomod"}, {"type": "npm"}, {"type": "pip"}, {"type": "rpm"}] request = Request(source_dir=tmp_path, output_dir=tmp_path, packages=packages) assert request.gomod_packages == [GomodPackageInput(type="gomod")] assert request.npm_packages == [NpmPackageInput(type="npm")] assert request.pip_packages == [PipPackageInput(type="pip")] + assert request.rpm_packages == [RpmPackageInput(type="rpm")] @pytest.mark.parametrize("which_path", ["source_dir", "output_dir"]) def test_path_not_absolute(self, which_path: str) -> None: diff --git a/tests/unit/package_managers/test_rpm.py b/tests/unit/package_managers/test_rpm.py index ff38d45ee..56d20f225 100644 --- a/tests/unit/package_managers/test_rpm.py +++ b/tests/unit/package_managers/test_rpm.py @@ -1,13 +1,15 @@ from configparser import ConfigParser from pathlib import Path -from typing import Any, Dict, Optional +from typing import Any, Dict, List, Optional, Union from unittest import mock from urllib.parse import quote import pytest import yaml +from _pytest.logging import LogCaptureFixture from cachi2.core.errors import PackageManagerError, PackageRejected +from cachi2.core.models.input import RpmPackageInput, _DNFOptions from cachi2.core.models.sbom import Component, Property from cachi2.core.package_managers.rpm import fetch_rpm_source, inject_files_post from cachi2.core.package_managers.rpm.main import ( @@ -43,20 +45,68 @@ """ +@pytest.mark.parametrize( + "model_input,result_options", + [ + pytest.param(mock.Mock(options=None), None, id="fetch_with_no_options"), + pytest.param( + RpmPackageInput.model_construct( + type="rpm", + options=_DNFOptions.model_construct(dnf={"foorepo": {"foo": 1, "bar": False}}), + ), + {"rpm": {"dnf": {"foorepo": {"foo": 1, "bar": False}}}}, + id="fetch_with_dnf_options", + ), + pytest.param( + [ + RpmPackageInput.model_construct( + type="rpm", + options=_DNFOptions.model_construct(dnf={"foorepo": {"foo": 1, "bar": False}}), + ), + RpmPackageInput.model_construct( + type="rpm", + options=_DNFOptions.model_construct(dnf={"bazrepo": {"baz": 0}}), + ), + ], + {"rpm": {"dnf": {"bazrepo": {"baz": 0}}}}, + id="fetch_multiple_dnf_option_sets", + ), + ], +) @mock.patch("cachi2.core.package_managers.rpm.main.RequestOutput.from_obj_list") @mock.patch("cachi2.core.package_managers.rpm.main._resolve_rpm_project") def test_fetch_rpm_source( mock_resolve_rpm_project: mock.Mock, mock_from_obj_list: mock.Mock, + model_input: Union[mock.Mock, RpmPackageInput, List[RpmPackageInput]], + result_options: Optional[Dict[str, Dict[str, Any]]], + caplog: LogCaptureFixture, ) -> None: - mock_component = mock.Mock() - mock_resolve_rpm_project.return_value = [mock_component] + def _has_multiple_options(rpm_models: List[RpmPackageInput]) -> bool: + options = 0 + for model in rpm_models: + if model.options: + options += 1 + return options > 1 + + mock_components = [mock.Mock()] + mock_resolve_rpm_project.return_value = mock_components mock_request = mock.Mock() - mock_request.rpm_packages = [mock.Mock()] + mock_request.rpm_packages = model_input if isinstance(model_input, list) else [model_input] fetch_rpm_source(mock_request) - mock_resolve_rpm_project.assert_called_once() - mock_from_obj_list.assert_called_once_with( - components=[mock_component], environment_variables=[], project_files=[] + + if isinstance(model_input, list): + mock_components *= len(model_input) + + if _has_multiple_options(model_input): + assert "Multiple sets of DNF options detected on the input:" in caplog.text + + mock_resolve_rpm_project.assert_called() + mock_from_obj_list.assert_called_with( + components=mock_components, + environment_variables=[], + project_files=[], + options=result_options, ) @@ -314,9 +364,10 @@ def test_generate_repos(mock_createrepo: mock.Mock, rooted_tmp_path: RootedPath) @pytest.mark.parametrize( - "expected_repofile", + "options, expected_repofile", [ pytest.param( + None, """ [repo1] baseurl=file://{output_dir}/repo1 @@ -329,15 +380,40 @@ def test_generate_repos(mock_createrepo: mock.Mock, rooted_tmp_path: RootedPath) """, id="no_repo_options", ), + pytest.param( + { + "rpm": { + "dnf": { + "repo1": {"gpgcheck": 0}, + "cachi2-repo": {"sslverify": False, "timeout": 4}, + } + } + }, + """ + [repo1] + baseurl=file://{output_dir}/repo1 + gpgcheck=0 + + [cachi2-repo] + name=Packages unaffiliated with an official repository + baseurl=file://{output_dir}/cachi2-repo + gpgcheck=1 + sslverify=False + timeout=4 + """, + id="dnf_repo_options", + ), ], ) -def test_generate_repofiles(rooted_tmp_path: RootedPath, expected_repofile: str) -> None: +def test_generate_repofiles( + rooted_tmp_path: RootedPath, expected_repofile: str, options: Optional[Dict[str, Any]] +) -> None: package_dir = rooted_tmp_path.join_within_root(DEFAULT_PACKAGE_DIR) arch_dir = Path(package_dir.path, "x86_64") for dir_ in ["repo1", "cachi2-repo", "repos.d"]: Path(arch_dir, dir_).mkdir(parents=True) - _generate_repofiles(rooted_tmp_path.path, rooted_tmp_path.path) + _generate_repofiles(rooted_tmp_path.path, rooted_tmp_path.path, options) repopath = arch_dir.joinpath("repos.d", "cachi2.repo") with open(repopath) as f: actual = ConfigParser() @@ -417,9 +493,11 @@ def test_inject_files_post( mock_path: mock.Mock, rooted_tmp_path: RootedPath, ) -> None: - inject_files_post(from_output_dir=rooted_tmp_path.path, for_output_dir=rooted_tmp_path.path) + inject_files_post( + from_output_dir=rooted_tmp_path.path, for_output_dir=rooted_tmp_path.path, options={} + ) mock_generate_repos.assert_called_once_with(rooted_tmp_path.path) - mock_generate_repofiles.assert_called_with(rooted_tmp_path.path, rooted_tmp_path.path) + mock_generate_repofiles.assert_called_with(rooted_tmp_path.path, rooted_tmp_path.path, {}) @mock.patch("cachi2.core.package_managers.rpm.main.asyncio.run")