Skip to content

Commit

Permalink
breaking change: Remove deprecated CLI arguments
Browse files Browse the repository at this point in the history
This removes a few deprecated CLI arguments, they will no longer work
or give a warning about its deprecation.

This includes
- Removal of `convert2rhel -f|--password-from-file`
- Removal of unused `convert2rhel --keep-rhsm`
  • Loading branch information
Venefilyn committed Mar 5, 2024
1 parent 5959d52 commit c922249
Show file tree
Hide file tree
Showing 8 changed files with 9 additions and 139 deletions.
45 changes: 2 additions & 43 deletions convert2rhel/toolopts.py
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 Down Expand Up @@ -274,14 +272,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 +333,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 +363,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 +392,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 +473,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 +495,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 +514,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:
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 @@ -141,19 +141,6 @@ def test_serverurl_with_no_rhsm_credentials(self, caplog, monkeypatch, global_to
assert message in caplog.text


def test_keep_rhsm(monkeypatch, caplog):
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):
"""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
6 changes: 3 additions & 3 deletions scripts/manpage_generation.sh
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@ VER=$(grep -oP '^Version:\s+\K\S+' packaging/convert2rhel.spec)

echo Generating for version $VER
# Generate a file with convert2rhel synopsis for argparse-manpage
/usr/bin/python -c 'from convert2rhel import toolopts; print("[synopsis]\n."+toolopts.CLI.usage())' > man/synopsis
python3 -c 'from convert2rhel import toolopts; print("[synopsis]\n."+toolopts.CLI.usage())' > man/synopsis

/usr/bin/python -m pip install argparse-manpage six pexpect
python3 -m pip install argparse-manpage six pexpect

# Generate the manpage using argparse-manpage
PYTHONPATH=. /usr/bin/python /home/runner/.local/bin/argparse-manpage --pyfile man/__init__.py --function get_parser --manual-title="General Commands Manual" --description="Automates the conversion of Red Hat Enterprise Linux derivative distributions to Red Hat Enterprise Linux." --project-name "convert2rhel $VER" --prog="convert2rhel" --include man/distribution --include man/synopsis > "$MANPAGE_DIR/convert2rhel.8"
PYTHONPATH=. python3 /home/vscode/.local/bin/argparse-manpage --pyfile man/__init__.py --function get_parser --manual-title="General Commands Manual" --description="Automates the conversion of Red Hat Enterprise Linux derivative distributions to Red Hat Enterprise Linux." --project-name "convert2rhel $VER" --prog="convert2rhel" --include man/distribution --include man/synopsis > "$MANPAGE_DIR/convert2rhel.8"

git status
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

0 comments on commit c922249

Please sign in to comment.