Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ModelViolationError while parsing repo files #1266

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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():
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,16 @@

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
Expand Down Expand Up @@ -116,3 +125,16 @@ def __init__(self, gr_name, gr_gid, gr_mem):
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()
17 changes: 16 additions & 1 deletion repos/system_upgrade/common/libraries/repofileutils.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,16 @@
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'
Expand All @@ -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)


Expand Down
27 changes: 27 additions & 0 deletions repos/system_upgrade/common/libraries/tests/test_repofileutils.py
Original file line number Diff line number Diff line change
@@ -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__))

Expand All @@ -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'))

Expand Down