From ec078243771f8ef43853bd242175a612fe84f95b Mon Sep 17 00:00:00 2001 From: tomasfratrik Date: Wed, 17 Jul 2024 12:12:50 +0200 Subject: [PATCH 16/40] Raise proper error when ModelViolationError occurs This error occurs when repo file has invalid definition, specifically when the 'name' entry of the config files is invalid. Also add tests. Jira: RHEL-19249 --- .../systemfacts/libraries/systemfacts.py | 13 ++++++++- .../systemfacts/tests/test_systemfacts.py | 24 ++++++++++++++++- .../common/libraries/repofileutils.py | 17 +++++++++++- .../libraries/tests/test_repofileutils.py | 27 +++++++++++++++++++ 4 files changed, 78 insertions(+), 3 deletions(-) diff --git a/repos/system_upgrade/common/actors/systemfacts/libraries/systemfacts.py b/repos/system_upgrade/common/actors/systemfacts/libraries/systemfacts.py index d1eeb28c..f16cea1d 100644 --- a/repos/system_upgrade/common/actors/systemfacts/libraries/systemfacts.py +++ b/repos/system_upgrade/common/actors/systemfacts/libraries/systemfacts.py @@ -217,7 +217,18 @@ def get_sysctls_status(): def get_repositories_status(): """ Get a basic information about YUM repositories installed in the system """ - return RepositoriesFacts(repositories=repofileutils.get_parsed_repofiles()) + try: + return RepositoriesFacts(repositories=repofileutils.get_parsed_repofiles()) + except repofileutils.InvalidRepoDefinition as e: + raise StopActorExecutionError( + message=str(e), + details={ + 'hint': 'For more directions on how to resolve the issue, see: {url}.' + .format( + url='https://access.redhat.com/solutions/6969001' + ) + } + ) def get_selinux_status(): diff --git a/repos/system_upgrade/common/actors/systemfacts/tests/test_systemfacts.py b/repos/system_upgrade/common/actors/systemfacts/tests/test_systemfacts.py index badf174c..5831b979 100644 --- a/repos/system_upgrade/common/actors/systemfacts/tests/test_systemfacts.py +++ b/repos/system_upgrade/common/actors/systemfacts/tests/test_systemfacts.py @@ -3,7 +3,16 @@ import pwd import pytest -from leapp.libraries.actor.systemfacts import _get_system_groups, _get_system_users, anyendswith, anyhasprefix, aslist +from leapp.exceptions import StopActorExecutionError +from leapp.libraries.actor.systemfacts import ( + _get_system_groups, + _get_system_users, + anyendswith, + anyhasprefix, + aslist, + get_repositories_status +) +from leapp.libraries.common import repofileutils from leapp.libraries.common.testutils import logger_mocked from leapp.libraries.stdlib import api from leapp.snactor.fixture import current_actor_libraries @@ -116,3 +125,16 @@ def test_get_system_groups(monkeypatch, etc_group_names, skipped_group_names): assert group_name not in api.current_logger().dbgmsg[0] else: assert not api.current_logger().dbgmsg + + +def test_failed_parsed_repofiles(monkeypatch): + def _raise_invalidrepo_error(): + raise repofileutils.InvalidRepoDefinition(msg='mocked error', + repofile='/etc/yum.repos.d/mock.repo', + repoid='mocked repoid') + + monkeypatch.setattr(repofileutils, 'get_parsed_repofiles', _raise_invalidrepo_error) + monkeypatch.setattr(api, 'current_logger', logger_mocked()) + + with pytest.raises(StopActorExecutionError): + get_repositories_status() diff --git a/repos/system_upgrade/common/libraries/repofileutils.py b/repos/system_upgrade/common/libraries/repofileutils.py index a563be52..cab3c42b 100644 --- a/repos/system_upgrade/common/libraries/repofileutils.py +++ b/repos/system_upgrade/common/libraries/repofileutils.py @@ -11,6 +11,16 @@ except ImportError: api.current_logger().warning('repofileutils.py: failed to import dnf') +class InvalidRepoDefinition(Exception): + """Raised when a repository definition is invalid.""" + def __init__(self, msg, repofile, repoid): + message = 'Invalid repository definition: {repoid} in: {repofile}: {msg}'.format( + repoid=repoid, repofile=repofile, msg=msg) + super(InvalidRepoDefinition, self).__init__(message) + self.repofile = repofile + self.repoid = repoid + + def _parse_repository(repoid, repo_data): def asbool(x): return x == '1' @@ -33,12 +43,17 @@ def parse_repofile(repofile): :param repofile: Path to the repo file :type repofile: str :rtype: RepositoryFile + :raises InvalidRepoDefinition: If the repository definition is invalid, + this can for example occur if 'name' field in repository is missing or it is invalid. """ data = [] with open(repofile, mode='r') as fp: cp = utils.parse_config(fp, strict=False) for repoid in cp.sections(): - data.append(_parse_repository(repoid, dict(cp.items(repoid)))) + try: + data.append(_parse_repository(repoid, dict(cp.items(repoid)))) + except fields.ModelViolationError as e: + raise InvalidRepoDefinition(e, repofile=repofile, repoid=repoid) return RepositoryFile(file=repofile, data=data) diff --git a/repos/system_upgrade/common/libraries/tests/test_repofileutils.py b/repos/system_upgrade/common/libraries/tests/test_repofileutils.py index 51cc1c11..42c7e49e 100644 --- a/repos/system_upgrade/common/libraries/tests/test_repofileutils.py +++ b/repos/system_upgrade/common/libraries/tests/test_repofileutils.py @@ -1,7 +1,10 @@ import json import os +import pytest + from leapp.libraries.common import repofileutils +from leapp.models.fields import ModelViolationError CUR_DIR = os.path.dirname(os.path.abspath(__file__)) @@ -12,6 +15,30 @@ def test_invert_dict(): assert inv_dict == {'a': [1], 'b': [1, 2]} +@pytest.mark.parametrize( + ('repoid', 'data'), + ( + ('missing-name', {'baseurl': 'http://example.com', 'enabled': '1', 'gpgcheck': '1'}), + (None, {'name': 'name', 'baseurl': 'http://example.com', 'enabled': '1', 'gpgcheck': '1'}), + ('name-none', {'name': None, 'baseurl': 'http://example.com', 'enabled': '1', 'gpgcheck': '1'}), + ('baseurl-true', {'name': 'valid', 'baseurl': True, 'enabled': '1', 'gpgcheck': '1'}), + ) +) +def test__parse_repository_missing_name(repoid, data): + with pytest.raises(ModelViolationError): + repofileutils._parse_repository(repoid, data) + + +def test_parse_repofile_error(monkeypatch): + def _parse_repository_mocked(*args, **kwargs): + raise ModelViolationError('') + + monkeypatch.setattr(repofileutils, '_parse_repository', _parse_repository_mocked) + + with pytest.raises(repofileutils.InvalidRepoDefinition): + repofileutils.parse_repofile(os.path.join(CUR_DIR, 'sample_repos.txt')) + + def test_parse_repofile(): repofile = repofileutils.parse_repofile(os.path.join(CUR_DIR, 'sample_repos.txt')) -- 2.47.0