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

[RHELC-1425] Remove deprecated CLI arguments #1058

Merged
merged 1 commit into from
Mar 12, 2024
Merged
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
46 changes: 2 additions & 44 deletions convert2rhel/toolopts.py
hosekadam marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,6 @@
"--username",
"-p",
"--password",
"-f",
"--password-from-file",
"-k",
"--activationkey",
"-o",
Expand All @@ -69,7 +67,6 @@ class ToolOpts:
def __init__(self):
self.debug = False
self.username = None
self.password_file = None
self.config_file = None
self.password = None
self.no_rhsm = False
Expand Down Expand Up @@ -274,14 +271,6 @@ def _add_subscription_manager_options(self):
" We recommend using the --config-file option instead to prevent leaking the password"
" through a list of running processes.",
)
group.add_argument(
"-f",
"--password-from-file",
help="File containing"
" password for the subscription-manager in the plain"
" text form. It's an alternative to the --password"
" option. Deprecated, use --config-file instead.",
)
group.add_argument(
"-k",
"--activationkey",
Expand Down Expand Up @@ -343,12 +332,6 @@ def _add_subscription_manager_options(self):
" (subscription.rhsm.redhat.com). It is not to be used to specify a Satellite server. For that, read"
" the product documentation at https://access.redhat.com/.",
)
group.add_argument(
"--keep-rhsm",
action="store_true",
help="Deprecated. This option has no effect. Convert2rhel will now use whatever"
" subscription-manager packages are present on the system.",
)

def _process_cli_options(self):
"""Process command line options used with the tool."""
Expand Down Expand Up @@ -379,9 +362,9 @@ def _process_cli_options(self):
tool_opts.set_opts(conf_file_opts)
config_opts = copy.copy(tool_opts)
tool_opts.config_file = parsed_opts.config_file
# corner case: password on CLI or in password file and activation-key in the config file
# corner case: password on CLI and activation-key in the config file
# password from CLI has precedence and activation-key must be deleted (unused)
if config_opts.activation_key and (parsed_opts.password or parsed_opts.password_from_file):
if config_opts.activation_key and parsed_opts.password:
tool_opts.activation_key = None

if parsed_opts.no_rpm_va:
Expand All @@ -408,11 +391,6 @@ def _process_cli_options(self):
if parsed_opts.password:
tool_opts.password = parsed_opts.password

if parsed_opts.password_from_file:
loggerinst.warning("Deprecated. Use -c | --config-file instead.")
tool_opts.password_file = parsed_opts.password_from_file
tool_opts.password = utils.get_file_content(parsed_opts.password_from_file)

if parsed_opts.enablerepo:
tool_opts.enablerepo = parsed_opts.enablerepo
if parsed_opts.disablerepo:
Expand Down Expand Up @@ -494,14 +472,6 @@ def _process_cli_options(self):
if url_parts.path:
tool_opts.rhsm_prefix = url_parts.path

if parsed_opts.keep_rhsm:
loggerinst.warning(
"The --keep-rhsm option is deprecated and will be removed in"
" the future. Convert2rhel will now always use the"
" subscription-manager packages which are already installed on"
" the system so this option has no effect."
)

tool_opts.autoaccept = parsed_opts.y
tool_opts.auto_attach = parsed_opts.auto_attach

Expand All @@ -524,12 +494,6 @@ def _process_cli_options(self):
" We're going to use the activation key."
)

if parsed_opts.password and parsed_opts.password_from_file:
loggerinst.warning(
"You have passed the RHSM password through both the --password-from-file and the --password option."
" We're going to use the password from file."
)

# Config files matches
if config_opts.username and parsed_opts.username:
loggerinst.warning(
Expand All @@ -549,12 +513,6 @@ def _process_cli_options(self):
" configuration file. We're going to use the command line values."
)

if (config_opts.activation_key or config_opts.password) and parsed_opts.password_from_file:
r0x0d marked this conversation as resolved.
Show resolved Hide resolved
loggerinst.warning(
"You have passed the RHSM credentials both through a config file and through a password file."
" We're going to use the password file."
)

if (tool_opts.org and not tool_opts.activation_key) or (not tool_opts.org and tool_opts.activation_key):
loggerinst.critical(
"Either the --organization or the --activationkey option is missing. You can't use one without the other."
Expand Down
54 changes: 0 additions & 54 deletions convert2rhel/unit_tests/toolopts_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,19 +139,6 @@ def test_serverurl_with_no_rhsm_credentials(self, caplog, monkeypatch, global_to
assert message in caplog.text


def test_keep_rhsm(monkeypatch, caplog, global_tool_opts):
monkeypatch.setattr(sys, "argv", mock_cli_arguments(["--keep-rhsm"]))

convert2rhel.toolopts.CLI()

assert (
"The --keep-rhsm option is deprecated and will be removed in"
" the future. Convert2rhel will now always use the"
" subscription-manager packages which are already installed on"
" the system so this option has no effect." in caplog.text
)


@pytest.mark.parametrize(
("argv", "warn", "ask_to_continue"),
(
Expand Down Expand Up @@ -302,13 +289,6 @@ def test_config_file(argv, content, output, message, monkeypatch, tmpdir, caplog
@pytest.mark.parametrize(
("argv", "content", "message", "output"),
(
(
mock_cli_arguments(["--password", "pass", "-f"]),
"pass_file",
"You have passed the RHSM password through both the --password-from-file and the --password option."
" We're going to use the password from file.",
{"password": "pass_file", "activation_key": None},
),
(
mock_cli_arguments(["--password", "pass", "--config-file"]),
"[subscription_manager]\nactivation_key = key_cnf_file",
Expand Down Expand Up @@ -336,40 +316,6 @@ def test_multiple_auth_src_combined(argv, content, message, output, caplog, monk
assert convert2rhel.toolopts.tool_opts.password == output["password"]


@pytest.mark.parametrize(
("argv", "content", "message", "output"),
(
(
mock_cli_arguments(["--password", "pass", "-f", "file", "--config-file", "file"]),
("pass_file", "[subscription_manager]\nactivation_key = pass_cnf_file"),
"You have passed the RHSM password through both the --password-from-file and the --password option."
" We're going to use the password from file.",
{"password": "pass_file", "activation_key": None},
),
),
)
def test_multiple_auth_src_files(argv, content, message, output, caplog, monkeypatch, tmpdir, global_tool_opts):
"""Test combination of password file, config file and CLI."""
path0 = os.path.join(str(tmpdir), "convert2rhel.password")
with open(path0, "w") as file:
file.write(content[0])
path1 = os.path.join(str(tmpdir), "convert2rhel.ini")
with open(path1, "w") as file:
file.write(content[1])
# Set the paths
argv[-3] = path0
argv[-1] = path1
os.chmod(path1, 0o600)

monkeypatch.setattr(sys, "argv", argv)
monkeypatch.setattr(convert2rhel.toolopts, "CONFIG_PATHS", value=[""])
convert2rhel.toolopts.CLI()

assert message in caplog.text
assert convert2rhel.toolopts.tool_opts.activation_key == output["activation_key"]
assert convert2rhel.toolopts.tool_opts.password == output["password"]


@pytest.mark.parametrize(
("argv", "message", "output"),
(
Expand Down
1 change: 0 additions & 1 deletion pytest.ini
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ markers =
test_config_custom_path_custom_filename
test_config_custom_path_standard_filename
test_config_cli_priority
test_config_password_file_priority
test_config_standard_paths_priority_diff_methods
test_config_standard_paths_priority
test_custom_valid_repo_provided
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ def test_offline_system_conversion(convert2rhel):
"""Test converting systems not connected to the Internet but requiring sub-mgr (e.g. managed by Satellite)."""

with convert2rhel(
"-y -k {} -o {} --keep-rhsm --debug".format(
"-y -k {} -o {} --debug".format(
env.str("SATELLITE_KEY"),
env.str("SATELLITE_ORG"),
)
Expand Down
12 changes: 0 additions & 12 deletions tests/integration/tier0/non-destructive/config-file/main.fmf
Original file line number Diff line number Diff line change
Expand Up @@ -49,18 +49,6 @@ tag+:
test: |
pytest -svv -m test_config_cli_priority

/config_password_file_priority:
summary+: |
Password file preferred
description+: |
Verify that passing the password through the password file
is preferred to the config file.
tag+:
- config-password-file-priority
- sanity
test: |
pytest -svv -m test_config_password_file_priority

/config_standard_paths_priority_diff_methods:
summary+: |
Activation key preferred to password
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,28 +76,6 @@ def test_user_path_cli_priority(convert2rhel):
remove_files(config)


@pytest.mark.test_config_password_file_priority
def test_user_path_pswd_file_priority(convert2rhel):
config = [
Config("~/.convert2rhel.ini", "[subscription_manager]\npassword = config_password"),
Config("~/password_file", "file_password"),
]
create_files(config)

with convert2rhel('-f "~/password_file" --debug') as c2r:
c2r.expect("DEBUG - Found password in /root/.convert2rhel.ini")
c2r.expect("WARNING - Deprecated. Use -c | --config-file instead.")
c2r.expect(
"WARNING - You have passed the RHSM credentials both through a config file and through a password file."
" We're going to use the password file."
)
c2r.sendcontrol("c")

assert c2r.exitstatus != 0

remove_files(config)


@pytest.mark.test_config_standard_paths_priority_diff_methods
def test_std_paths_priority_diff_methods(convert2rhel):
config = [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ def test_backup_os_release_wrong_registration(shell, convert2rhel, custom_subman
"""
assert shell("find /etc/os-release").returncode == 0

with convert2rhel("-y -k wrong_key -o rUbBiSh_pWd --debug --keep-rhsm") as c2r:
with convert2rhel("-y -k wrong_key -o rUbBiSh_pWd --debug") as c2r:
c2r.expect("Unable to register the system through subscription-manager.")
c2r.expect("Restore /etc/os-release from backup")

Expand Down Expand Up @@ -149,7 +149,7 @@ def test_backup_os_release_no_envar(

assert shell("find /etc/os-release").returncode == 0
with convert2rhel(
"-y -k {} -o {} --debug --keep-rhsm".format(
"-y -k {} -o {} --debug".format(
env.str("SATELLITE_KEY"),
env.str("SATELLITE_ORG"),
),
Expand Down Expand Up @@ -185,7 +185,7 @@ def test_backup_os_release_with_envar(
assert shell("find /etc/os-release").returncode == 0

with convert2rhel(
"-y -k {} -o {} --debug --keep-rhsm".format(
"-y -k {} -o {} --debug".format(
env.str("SATELLITE_KEY"),
env.str("SATELLITE_ORG"),
),
Expand Down