From da87cca7716fc39865ad452b479981ba1552f38c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adam=20Ho=C5=A1ek?= Date: Thu, 28 Nov 2024 13:10:41 +0100 Subject: [PATCH] [RHELC-1753] Fix reaching unaccessible repos from enablerepo (#1425) * [RHELC-1753] Fix reaching unaccessible repos from enablerepo Avoid reaching unaccessible repositories provided by user using --enablerepo argument. Those repos are manually disabled during analysis, but when user provides unreachable repo the analysis would fail. This change adds check if provided repos are accessible - if not, those unaccessible ones are removed from the list for manual disable during analysis. * Disable repo code cleanup --------- Co-authored-by: Michal Bocek --- .../pre_ponr_changes/handle_packages.py | 8 +- .../system_checks/is_loaded_kernel_latest.py | 3 +- convert2rhel/backup/packages.py | 2 +- convert2rhel/pkghandler.py | 6 +- convert2rhel/pkgmanager/__init__.py | 1 + convert2rhel/repo.py | 114 +++++++++++++--- convert2rhel/unit_tests/repo_test.py | 128 +++++++++++++++--- .../test_custom_repository.py | 4 +- 8 files changed, 220 insertions(+), 46 deletions(-) diff --git a/convert2rhel/actions/pre_ponr_changes/handle_packages.py b/convert2rhel/actions/pre_ponr_changes/handle_packages.py index 717d020e3a..6c246c4adf 100644 --- a/convert2rhel/actions/pre_ponr_changes/handle_packages.py +++ b/convert2rhel/actions/pre_ponr_changes/handle_packages.py @@ -43,9 +43,9 @@ def run(self): if third_party_pkgs: # RHELC-884 disable the RHEL repos to avoid reaching them when checking original system. # There is needed for avoid reaching out RHEL repositories while requesting info about pkgs. - disable_repos = repo.get_rhel_repos_to_disable() + repos_to_disable = repo.DisableReposDuringAnalysis().get_rhel_repos_to_disable() pkg_list = pkghandler.format_pkg_info( - sorted(third_party_pkgs, key=self.extract_packages), disable_repos=disable_repos + sorted(third_party_pkgs, key=self.extract_packages), disable_repos=repos_to_disable ) warning_message = ( "Only packages signed by {} are to be" @@ -130,8 +130,8 @@ def run(self): # RHELC-884 disable the RHEL repos to avoid reaching them when checking original system. # There is needed for avoid reaching out RHEL repositories while requesting info about pkgs. - disable_repos = repo.get_rhel_repos_to_disable() - pkgs_removed = _remove_packages_unless_from_redhat(pkgs_list=all_pkgs, disable_repos=disable_repos) + repos_to_disable = repo.DisableReposDuringAnalysis().get_rhel_repos_to_disable() + pkgs_removed = _remove_packages_unless_from_redhat(pkgs_list=all_pkgs, disable_repos=repos_to_disable) # https://issues.redhat.com/browse/RHELC-1677 # In some cases the {system}-release package takes ownership of the /etc/yum.repos.d/ directory, diff --git a/convert2rhel/actions/system_checks/is_loaded_kernel_latest.py b/convert2rhel/actions/system_checks/is_loaded_kernel_latest.py index 5398d8bce1..02e023b132 100644 --- a/convert2rhel/actions/system_checks/is_loaded_kernel_latest.py +++ b/convert2rhel/actions/system_checks/is_loaded_kernel_latest.py @@ -45,7 +45,8 @@ def run(self): return # RHELC-884 disable the RHEL repos to avoid reaching them when checking original system. - disable_repo_command = repo.get_rhel_disable_repos_command(repo.get_rhel_repos_to_disable()) + repos_to_disable = repo.DisableReposDuringAnalysis().get_rhel_repos_to_disable() + disable_repo_command = repo.get_rhel_disable_repos_command(repos_to_disable) cmd = [ "repoquery", diff --git a/convert2rhel/backup/packages.py b/convert2rhel/backup/packages.py index 2cbfa31b95..2ab1a44275 100644 --- a/convert2rhel/backup/packages.py +++ b/convert2rhel/backup/packages.py @@ -60,7 +60,7 @@ def __init__( self.varsdir = varsdir # RHELC-884 disable the RHEL repos to avoid downloading pkg from them. - self.disable_repos = disable_repos or repo.get_rhel_repos_to_disable() + self.disable_repos = disable_repos or repo.DisableReposDuringAnalysis().get_rhel_repos_to_disable() self._backedup_pkgs_paths = [] diff --git a/convert2rhel/pkghandler.py b/convert2rhel/pkghandler.py index 4b4cda8985..98f2a2d51c 100644 --- a/convert2rhel/pkghandler.py +++ b/convert2rhel/pkghandler.py @@ -734,12 +734,12 @@ def get_total_packages_to_update(): # RHEL repositories needs to be disabled when checking if the packages are up to date. # When user specifies rhel repos to use during conversion, disabling them is also needed. # Note: Do not depend on the --no-rhsm option, enable repos can be specified without it. - disable_repos = repo.get_rhel_repos_to_disable() + repos_to_disable = repo.DisableReposDuringAnalysis().get_rhel_repos_to_disable() if pkgmanager.TYPE == "yum": - packages = _get_packages_to_update_yum(disable_repos=disable_repos) + packages = _get_packages_to_update_yum(disable_repos=repos_to_disable) elif pkgmanager.TYPE == "dnf": - packages = _get_packages_to_update_dnf(disable_repos=disable_repos) + packages = _get_packages_to_update_dnf(disable_repos=repos_to_disable) return set(packages) diff --git a/convert2rhel/pkgmanager/__init__.py b/convert2rhel/pkgmanager/__init__.py index ace448e2a7..070fb8f306 100644 --- a/convert2rhel/pkgmanager/__init__.py +++ b/convert2rhel/pkgmanager/__init__.py @@ -243,6 +243,7 @@ def call_yum_cmd( # When using subscription-manager for the conversion, use those repos for the yum call that have been enabled # through subscription-manager repos_to_enable = system_info.get_enabled_rhel_repos() + logger.debug("Custom epos in yum cmd: {repos_to_enable}".format(repos_to_enable=repos_to_enable)) for repo in repos_to_enable: cmd.append("--enablerepo={}".format(repo)) diff --git a/convert2rhel/repo.py b/convert2rhel/repo.py index 952895151e..ddfa5690c7 100644 --- a/convert2rhel/repo.py +++ b/convert2rhel/repo.py @@ -19,6 +19,7 @@ import tempfile +import re from contextlib import closing @@ -29,6 +30,7 @@ from convert2rhel.systeminfo import system_info from convert2rhel.toolopts import tool_opts from convert2rhel.utils import TMP_DIR, store_content_to_file +from convert2rhel.pkgmanager import TYPE, call_yum_cmd DEFAULT_YUM_REPOFILE_DIR = "/etc/yum.repos.d" @@ -58,34 +60,110 @@ def get_rhel_repoids(): return repos_needed -def get_rhel_repos_to_disable(): - """Get the list of repositories which should be disabled when performing pre-conversion checks. +class DisableReposDuringAnalysis(object): + _instance = None + _repos_to_disable = None - Avoid downloading backup and up-to-date checks from them. The output list can looks like: ["rhel*"] + def __new__(cls): + """Singleton pattern""" + if cls._instance is None: + cls._instance = super(DisableReposDuringAnalysis, cls).__new__(cls) + # Cannot call the _set_rhel_repos_to_disable() directly due Python 2 support + cls._instance._initialized = False - .. note:: - If --enablerepo switch is used together with the --no-rhsm, we will return a combination of repositories to disable as following: + return cls._instance - >>> # tool_opts.enablerepo comes from the CLI option `--enablerepo`. - >>> repos_to_disable = ["rhel*"] - >>> repos_to_disable.extend(tool_opts.enablerepo) # returns: ["rhel*", "my-rhel-repo-mirror"] + def __init__(self): + if self._initialized: + return - :return: List of repositories to disable when performing checks. - :rtype: list[str] + self._set_rhel_repos_to_disable() + self._instance._initialized = True + + def _set_rhel_repos_to_disable(self): + """Set the list of repositories which should be disabled when performing pre-conversion checks. + + Avoid using RHEL repos for certain re-conversion analysis phase operations such as: + - downloading a package backup + - the package up-to-date check + - querying what repository local packages have been installed from + - the latest available kernel check + Only the original system vendor repos should be used for these pre-conversion analysis phase operations. + + .. note:: + If --enablerepo switch is used together with the --no-rhsm, we will return a combination of repositories to + disable as following: + + >>> # tool_opts.enablerepo comes from the CLI option `--enablerepo`. + >>> self.repos_to_disable = ["rhel*"] + >>> self.repos_to_disable.extend(tool_opts.enablerepo) # returns: ["rhel*", "my-rhel-repo-mirror"] + + :return: List of repoids to disable, such as ["rhel*", "my-optional-repo"] + :rtype: List + """ + # RHELC-884 disable the RHEL repos to avoid reaching them when checking original system. + self._repos_to_disable = ["rhel*"] + + # this is for the case where the user configures e.g. [my-rhel-repo-mirror] on the system and leaves it enabled + # before running convert2rhel - we want to prevent the checks from accessing it as it contains packages for RHEL + if tool_opts.no_rhsm and tool_opts.enablerepo: + logger.debug("Preparing a list of RHEL repositories to be disabled during analysis.") + self._set_custom_repos() + + return self._repos_to_disable + + def _set_custom_repos(self): + """If we are using the YUM pkg manager, we need to check if all custom repositories provided by the user through + --enablerepo are accessible. DNF package manager can handle situation of unreachable repo by skipping it.""" + if TYPE == "dnf": + self._repos_to_disable.extend(tool_opts.enablerepo) + return + + # pkg manager is yum + # copy the enablerepo list to avoid changing the original one + repos_to_check = list(tool_opts.enablerepo) + self._repos_to_disable.extend(_get_valid_custom_repos(repos_to_check)) + + def get_rhel_repos_to_disable(self): + """See the docstring of _set_rhel_repos_to_disable for details about the repos to disable.""" + return self._repos_to_disable + + +def _get_valid_custom_repos(repos_to_check): + """Check if provided repo IDs are accessible. The function is recursive. + + :arg repos_to_check: Repo IDs to be checked + :arg type: List + :return: Accessible repositories + :rtype: List """ - # RHELC-884 disable the RHEL repos to avoid reaching them when checking original system. - repos_to_disable = ["rhel*"] + if not repos_to_check: + return [] - # this is for the case where the user configures e.g. [my-rhel-repo-mirror] on the system and leaves it enabled - # before running convert2rhel - we want to prevent the checks from accessing it as it contains packages for RHEL - if tool_opts.no_rhsm: - repos_to_disable.extend(tool_opts.enablerepo) + args = ["-v", "--setopt=*.skip_if_unavailable=False"] + output, ret_code = call_yum_cmd( + command="makecache", args=args, print_output=False, disable_repos=repos_to_check, enable_repos=[] + ) + + if ret_code: + reponame_regex = r"Error getting repository data for ([^,]+)," + problematic_reponame_line = re.search(reponame_regex, output) + if problematic_reponame_line: + reponame = problematic_reponame_line.group(1) + logger.debug( + "Removed the {reponame} repository from the list of repositories to disable in certain" + " pre-conversion analysis checks as it is inaccessible at the moment and yum fails when trying to" + " disable an inaccessible repository.".format(reponame=reponame) + ) + repos_to_check.remove(reponame) + return _get_valid_custom_repos(repos_to_check) - return repos_to_disable + # The list of repositories passed to the function sans the inaccessible ones + return repos_to_check def get_rhel_disable_repos_command(disable_repos): - """Build command containing all the repos for disable. The result looks like + """Build command containing all the repos to disable. The result looks like '--disablerepo repo --disablerepo repo1 --disablerepo repo2' If provided list is empty, empty string is returned. diff --git a/convert2rhel/unit_tests/repo_test.py b/convert2rhel/unit_tests/repo_test.py index be04b0a848..192282b4e1 100644 --- a/convert2rhel/unit_tests/repo_test.py +++ b/convert2rhel/unit_tests/repo_test.py @@ -77,24 +77,6 @@ def test_get_rhel_repoids_el7(pretend_os, is_els_release, expected, monkeypatch) assert repos == expected -@pytest.mark.parametrize( - ("no_rhsm", "enablerepo", "disablerepos"), - ( - (False, [], ["rhel*"]), - (True, ["test-repo"], ["rhel*", "test-repo"]), - (True, [], ["rhel*"]), - (False, ["test-repo"], ["rhel*"]), - ), -) -def test_get_rhel_repos_to_disable(monkeypatch, global_tool_opts, no_rhsm, enablerepo, disablerepos): - monkeypatch.setattr(repo, "tool_opts", global_tool_opts) - global_tool_opts.enablerepo = enablerepo - global_tool_opts.no_rhsm = no_rhsm - - repos = repo.get_rhel_repos_to_disable() - assert repos == disablerepos - - @pytest.mark.parametrize( ("disable_repos", "command"), ( @@ -189,3 +171,113 @@ def test_write_temporary_repofile_store_failure(tmpdir, monkeypatch): assert "STORE_REPOFILE_FAILED" in execinfo._excinfo[1].id assert "Failed to store a repository file" in execinfo._excinfo[1].title assert "test" in execinfo._excinfo[1].description + + +class TestDisableReposDuringAnalysis: + def test_singleton(self, monkeypatch): + """Test if the singleton works properly and only 1 instance is created.""" + # Force remove the singleton instance + repo.DisableReposDuringAnalysis._instance = None + + set_rhel_repos_to_disable = mock.Mock() + monkeypatch.setattr(repo.DisableReposDuringAnalysis, "_set_rhel_repos_to_disable", set_rhel_repos_to_disable) + + singleton1 = repo.DisableReposDuringAnalysis() + singleton2 = repo.DisableReposDuringAnalysis() + + assert singleton1 is singleton2 + assert set_rhel_repos_to_disable.call_count == 1 + + # Force remove the singleton instance + repo.DisableReposDuringAnalysis._instance = None + + @pytest.mark.parametrize( + ("disable_repos", "yum_output", "result"), + ( + ( + ["repo1", "repo2", "repo3"], + [ + ( + """Loading "fastestmirror" plugin + Config time: 0.003 + + + Error getting repository data for repo1, repository not found""", + 1, + ), + ("", 0), + ], + ["repo2", "repo3"], + ), + ([], "Some testing output, won't be called", []), + (["repo1", "repo2"], [("Message causing exit code 1", 1)], ["repo1", "repo2"]), + ( + ["repo1"], + [("All fine", 0)], + ["repo1"], + ), + ), + ) + def test_get_valid_custom_repos(self, yum_output, disable_repos, result, monkeypatch): + """Test parsing of the yum output and getting the unavailable repositories.""" + monkeypatch.setattr(repo, "call_yum_cmd", mock.Mock(side_effect=yum_output)) + + output = repo._get_valid_custom_repos(disable_repos) + + assert output == result + + @pytest.mark.parametrize( + ("no_rhsm", "enablerepo", "disablerepos"), + ( + (False, [], ["rhel*"]), + (True, ["test-repo"], ["rhel*", "test-repo"]), + (True, [], ["rhel*"]), + (False, ["test-repo"], ["rhel*"]), + ), + ) + def test_get_rhel_repos_to_disable_dnf(self, monkeypatch, global_tool_opts, no_rhsm, enablerepo, disablerepos): + """Test getting the repositories to be disabled on systems with DNF. On DNF there is no need to check if + repositories for disabling are accessible. The package manager handles it well. + """ + # Force remove the singleton instance + repo.DisableReposDuringAnalysis._instance = None + + monkeypatch.setattr(repo, "tool_opts", global_tool_opts) + monkeypatch.setattr(repo, "TYPE", "dnf") + global_tool_opts.enablerepo = enablerepo + global_tool_opts.no_rhsm = no_rhsm + + repos = repo.DisableReposDuringAnalysis().get_rhel_repos_to_disable() + assert repos == disablerepos + + # Force remove the singleton instance + repo.DisableReposDuringAnalysis._instance = None + + @pytest.mark.parametrize( + ("no_rhsm", "enablerepo", "disablerepos"), + ( + (False, [], ["rhel*"]), + (True, ["test-repo"], ["rhel*", "test-repo"]), + (True, [], ["rhel*"]), + (False, ["test-repo"], ["rhel*"]), + ), + ) + def test_get_rhel_repos_to_disable_yum(self, monkeypatch, global_tool_opts, no_rhsm, enablerepo, disablerepos): + """Test getting the repositories to be disabled on systems with YUM. With YUM all the repositories needs + to be accessible. + """ + # Force remove the singleton instance + repo.DisableReposDuringAnalysis._instance = None + + monkeypatch.setattr(repo, "tool_opts", global_tool_opts) + monkeypatch.setattr(repo, "TYPE", "yum") + # Set the output of yum makecache empty (no problem, simulating the inaccessible repo is in different test) + monkeypatch.setattr(repo, "call_yum_cmd", mock.Mock(return_value=("", 0))) + global_tool_opts.enablerepo = enablerepo + global_tool_opts.no_rhsm = no_rhsm + + repos = repo.DisableReposDuringAnalysis().get_rhel_repos_to_disable() + assert repos == disablerepos + + # Force remove the singleton instance + repo.DisableReposDuringAnalysis._instance = None diff --git a/tests/integration/tier0/non-destructive/custom-repository/test_custom_repository.py b/tests/integration/tier0/non-destructive/custom-repository/test_custom_repository.py index 8f0db06304..518022a39d 100644 --- a/tests/integration/tier0/non-destructive/custom-repository/test_custom_repository.py +++ b/tests/integration/tier0/non-destructive/custom-repository/test_custom_repository.py @@ -44,7 +44,9 @@ def test_custom_invalid_repo_without_rhsm(shell, convert2rhel): with invalid repository passed. Make sure that after failed repo check there is a kernel installed. """ - with convert2rhel("-y --no-rhsm --enablerepo fake-rhel-8-for-x86_64-baseos-rpms --debug", unregister=True) as c2r: + with convert2rhel( + "-y --no-rhsm --enablerepo fake-rhel-repo-1 --enablerepo fake-rhel-repo-2 --debug", unregister=True + ) as c2r: c2r.expect("CUSTOM_REPOSITORIES_ARE_VALID::UNABLE_TO_ACCESS_REPOSITORIES") assert c2r.exitstatus == 2