From ce71a44ffd1965330813c15056b5e58e50217108 Mon Sep 17 00:00:00 2001 From: Matej Matuska Date: Wed, 2 Jul 2025 16:36:55 +0200 Subject: [PATCH 58/66] Fix target version format checks Currently only the format of a version specified by LEAPP_DEVEL_TARGET_RELEASE is checked, when specified via --target cmdline argument it isn't, which is a bug. This patch fixes the bug, by enabling the check for --target too. NOTE: these are only format checks, the state of support is already checked later in the upgrade process by the checktargetversion actor. Jira: RHEL-96238 --- commands/command_utils.py | 28 +++++++++------ commands/tests/test_upgrade_paths.py | 54 +++++++++++++++++++--------- commands/upgrade/util.py | 4 +-- 3 files changed, 57 insertions(+), 29 deletions(-) diff --git a/commands/command_utils.py b/commands/command_utils.py index 155bacad..e6ba6ba4 100644 --- a/commands/command_utils.py +++ b/commands/command_utils.py @@ -59,7 +59,9 @@ def assert_version_format(version_str, desired_format, version_kind): :raises: CommandError """ if not re.match(desired_format.regex, version_str): - error_str = 'Unexpected format of target version: {0}. The required format is \'{1}\'.' + error_str = ( + 'Unexpected format of target version: {0}. The required format is \'{1}\'.' + ) raise CommandError(error_str.format(version_str, desired_format.human_readable)) @@ -182,26 +184,32 @@ def get_target_version(flavour): return target_versions[-1] if target_versions else None -def vet_upgrade_path(args): +def get_target_release(args): """ - Make sure the user requested upgrade_path is a supported one. - If LEAPP_DEVEL_TARGET_RELEASE is set then it's value is not vetted against upgrade_paths_map but used as is. + Return the user selected target release or choose one from config. + + A target release can be specified, ordered by priority, by the + LEAPP_DEVEL_TARGET_RELEASE or args.target (--target cmdline arg) or in the + config file. + + NOTE: when specified via the env var or cmdline arg, the version isn't + checked against supported versions, this is done later by an actor in the + upgrade process. :return: `tuple` (target_release, flavor) """ flavor = get_upgrade_flavour() env_version_override = os.getenv('LEAPP_DEVEL_TARGET_RELEASE') - if env_version_override: + target_ver = env_version_override or args.target + if target_ver: os_release_contents = _retrieve_os_release_contents() distro_id = os_release_contents.get('ID', '') expected_version_format = _DISTRO_VERSION_FORMATS.get(distro_id, VersionFormats.MAJOR_MINOR).value - assert_version_format(env_version_override, expected_version_format, _VersionKind.TARGET) - - return (env_version_override, flavor) + assert_version_format(target_ver, expected_version_format, _VersionKind.TARGET) + return (target_ver, flavor) - target_release = args.target or get_target_version(flavor) - return (target_release, flavor) + return (get_target_version(flavor), flavor) def set_resource_limits(): diff --git a/commands/tests/test_upgrade_paths.py b/commands/tests/test_upgrade_paths.py index c2cb09aa..89b5eb71 100644 --- a/commands/tests/test_upgrade_paths.py +++ b/commands/tests/test_upgrade_paths.py @@ -1,3 +1,4 @@ +import os import resource import mock @@ -29,34 +30,53 @@ def test_get_target_version(mock_open, monkeypatch): assert command_utils.get_target_version('default') == '9.0' -@mock.patch("leapp.cli.commands.command_utils.get_upgrade_paths_config", - return_value={"default": {"7.9": ["8.4"], "8.6": ["9.0"], "7": ["8.4"], "8": ["9.0"]}}) -def test_vet_upgrade_path(mock_open, monkeypatch): +@mock.patch( + "leapp.cli.commands.command_utils.get_upgrade_paths_config", + return_value={ + "default": { + "7.9": ["8.4"], + "8.6": ["9.0", "9.2"], + "7": ["8.4"], + "8": ["9.0", "9.2"], + } + }, +) +def test_get_target_release(mock_open, monkeypatch): # do not remove mock_open monkeypatch.setattr(command_utils, 'get_os_release_version_id', lambda x: '8.6') # make sure env var LEAPP_DEVEL_TARGET_RELEASE takes precedence - # when env var set to a bad version - abort the upgrade - args = mock.Mock(target='9.0') - monkeypatch.setenv('LEAPP_DEVEL_TARGET_RELEASE', '1.2badsemver') - with pytest.raises(CommandError) as err: - command_utils.vet_upgrade_path(args) - assert 'Unexpected format of target version' in err - # MAJOR.MINOR.PATCH is considered as bad version, only MAJOR.MINOR is accepted args = mock.Mock(target='9.0') + monkeypatch.setenv('LEAPP_DEVEL_TARGET_RELEASE', '9.2') + print(os.getenv('LEAPP_DEVEL_TARGET_RELEASE')) + assert command_utils.get_target_release(args) == ('9.2', 'default') + + # when env var set to a bad version, expect an error monkeypatch.setenv('LEAPP_DEVEL_TARGET_RELEASE', '9.0.0') with pytest.raises(CommandError) as err: - command_utils.vet_upgrade_path(args) + command_utils.get_target_release(args) assert 'Unexpected format of target version' in err + # when env var set to a version not in upgrade_paths map - go on and use it + # this is checked by an actor in the IPU monkeypatch.setenv('LEAPP_DEVEL_TARGET_RELEASE', '1.2') - assert command_utils.vet_upgrade_path(args) == ('1.2', 'default') - # no env var set, --target is set to proper version + assert command_utils.get_target_release(args) == ('1.2', 'default') + + # no env var set, --target is set to proper version - use it + args = mock.Mock(target='9.0') monkeypatch.delenv('LEAPP_DEVEL_TARGET_RELEASE', raising=False) - assert command_utils.vet_upgrade_path(args) == ('9.0', 'default') - # env var is set to proper version, --target is set to a bad one - use env var and go on with the upgrade + assert command_utils.get_target_release(args) == ('9.0', 'default') + + # --target set with incorrectly formatted version, env var not set, fail + args = mock.Mock(target='9.0a') + with pytest.raises(CommandError) as err: + command_utils.get_target_release(args) + assert 'Unexpected format of target version' in err + + # env var is set to proper version, --target set to a bad one: + # env var has priority, use it and go on with the upgrade monkeypatch.setenv('LEAPP_DEVEL_TARGET_RELEASE', '9.0') - args = mock.Mock(target='1.2') - assert command_utils.vet_upgrade_path(args) == ('9.0', 'default') + args = mock.Mock(target='9.0.0') + assert command_utils.get_target_release(args) == ('9.0', 'default') def _mock_getrlimit_factory(nofile_limits=(1024, 4096), fsize_limits=(1024, 4096)): diff --git a/commands/upgrade/util.py b/commands/upgrade/util.py index b54b0b34..7d5b563e 100644 --- a/commands/upgrade/util.py +++ b/commands/upgrade/util.py @@ -253,8 +253,8 @@ def prepare_configuration(args): if args.nogpgcheck: os.environ['LEAPP_NOGPGCHECK'] = '1' - # Check upgrade path and fail early if it's unsupported - target_version, flavor = command_utils.vet_upgrade_path(args) + # Check upgrade path and fail early if it's invalid + target_version, flavor = command_utils.get_target_release(args) os.environ['LEAPP_UPGRADE_PATH_TARGET_RELEASE'] = target_version os.environ['LEAPP_UPGRADE_PATH_FLAVOUR'] = flavor -- 2.50.1