diff --git a/convert2rhel/toolopts/__init__.py b/convert2rhel/toolopts/__init__.py index 0298754ad..deb7e8366 100644 --- a/convert2rhel/toolopts/__init__.py +++ b/convert2rhel/toolopts/__init__.py @@ -18,6 +18,8 @@ import logging +from convert2rhel.utils.subscription import setup_rhsm_parts + loggerinst = logging.getLogger(__name__) @@ -105,16 +107,22 @@ def _handle_missing_options(self): "Either the --org or the --activationkey option is missing. You can't use one without the other." ) - def _apply_cls_attributes(self, config): - for key, value in vars(config).items(): - if not hasattr(self, key): - setattr(self, key, value) - continue + def set_opts(self, key, value): + if not hasattr(self, key): + setattr(self, key, value) + return + + current_attribute_value = getattr(self, key) - current_attribute_value = getattr(self, key) + if value and not current_attribute_value: + setattr(self, key, value) - if value and not current_attribute_value: - setattr(self, key, value) + def _handle_rhsm_parts(self): + # Sending itself as the ToolOpts class contains all the attribute references. + rhsm_parts = setup_rhsm_parts(self) + + for key, value in rhsm_parts.items(): + self.set_opts(key, value) def initialize(self, config_sources): # Populate the values for each config class before applying the attribute to the class. @@ -122,7 +130,12 @@ def initialize(self, config_sources): # Apply the attributes from config classes to ToolOpts. for config in config_sources: - self._apply_cls_attributes(config) + [self.set_opts(key, value) for key, value in vars(config).items()] + + # This is being handled here because we have conditions inside the `setup_rhsm_parts` that checks for + # username/password, and since that type of information can come from CLI or Config file, we are putting it + # here. + self._handle_rhsm_parts() # Handle the conflicts between FileConfig and other Config classes self._handle_config_conflict(config_sources) diff --git a/convert2rhel/toolopts/config.py b/convert2rhel/toolopts/config.py index 2e4386956..af206fb88 100644 --- a/convert2rhel/toolopts/config.py +++ b/convert2rhel/toolopts/config.py @@ -239,9 +239,6 @@ def __init__(self, opts): self.enablerepo = [] # type: list[str] self.disablerepo = [] # type: list[str] self.pool = None # type: str - self.rhsm_hostname = None # type: str - self.rhsm_port = None # type: str - self.rhsm_prefix = None # type: str self.autoaccept = None # type: bool self.auto_attach = None # type: str self.restart = False # type: bool @@ -250,17 +247,13 @@ def __init__(self, opts): self.eus = False # type: bool self.els = False # type: bool self.activity = None # type: str + self.serverurl = None # type: str self._opts = opts # type: arpgparse.Namepsace def run(self): - # We are using the self._opts here and converting later due to the subscription.py modules expect it to be in - # the format of the `Namespace` class from argparse. - rhsm_parts = setup_rhsm_parts(self._opts) opts = vars(self._opts) - if rhsm_parts: - opts.update(rhsm_parts) opts = self._normalize_opts(opts) self._validate(opts) diff --git a/convert2rhel/unit_tests/toolopts/toolopts_test.py b/convert2rhel/unit_tests/toolopts/toolopts_test.py index efd74d3d1..be73cba02 100644 --- a/convert2rhel/unit_tests/toolopts/toolopts_test.py +++ b/convert2rhel/unit_tests/toolopts/toolopts_test.py @@ -42,13 +42,13 @@ def run(self): @pytest.mark.parametrize( ("config_sources",), ( - ([MockConfig(source="command line", org="test", activation_key=None)],), - ([MockConfig(source="command line", org=None, activation_key="test")],), - ([MockConfig(source="command line", username="test", activation_key="test", org="blabla")],), + ([MockConfig(source="command line", serverurl=None, org="test", activation_key=None)],), + ([MockConfig(source="command line", serverurl=None, org=None, activation_key="test")],), + ([MockConfig(source="command line", serverurl=None, username="test", activation_key="test", org="blabla")],), # Multiple configs ( [ - MockConfig(source="command line", username="test", activation_key="test", org="blabla"), + MockConfig(source="command line", serverurl=None, username="test", activation_key="test", org="blabla"), MockConfig( source="configuration file", username="test", @@ -88,6 +88,7 @@ def test_apply_cls_attributes(config_sources, monkeypatch): [ MockConfig( source="command line", + serverurl=None, username="test", org=None, activation_key=None, @@ -105,7 +106,13 @@ def test_apply_cls_attributes(config_sources, monkeypatch): ( [ MockConfig( - source="command line", username=None, org="test", activation_key=None, password=None, no_rpm_va=None + source="command line", + serverurl=None, + username=None, + org="test", + activation_key=None, + password=None, + no_rpm_va=None, ), MockConfig( source="configuration file", username=None, org="config test", activation_key=None, password=None @@ -118,7 +125,13 @@ def test_apply_cls_attributes(config_sources, monkeypatch): ( [ MockConfig( - source="command line", username=None, org=None, activation_key="test", password=None, no_rpm_va=None + source="command line", + serverurl=None, + username=None, + org=None, + activation_key="test", + password=None, + no_rpm_va=None, ), MockConfig( source="configuration file", username=None, org=None, activation_key="config test", password=None @@ -132,6 +145,7 @@ def test_apply_cls_attributes(config_sources, monkeypatch): [ MockConfig( source="command line", + serverurl=None, username="test", org=None, activation_key=None, @@ -150,6 +164,7 @@ def test_apply_cls_attributes(config_sources, monkeypatch): [ MockConfig( source="command line", + serverurl=None, username=None, org=None, activation_key=None, @@ -190,6 +205,7 @@ def test_handle_config_conflicts(config_sources, expected_message, expected_outp [ MockConfig( source="command line", + serverurl=None, username=None, org=None, activation_key=None, @@ -206,6 +222,7 @@ def test_handle_config_conflicts(config_sources, expected_message, expected_outp [ MockConfig( source="command line", + serverurl=None, username=None, org=None, activation_key=None, @@ -222,6 +239,7 @@ def test_handle_config_conflicts(config_sources, expected_message, expected_outp [ MockConfig( source="command line", + serverurl=None, username="test", org=None, activation_key=None, @@ -238,6 +256,7 @@ def test_handle_config_conflicts(config_sources, expected_message, expected_outp [ MockConfig( source="command line", + serverurl=None, username=None, org=None, activation_key=None, @@ -253,6 +272,7 @@ def test_handle_config_conflicts(config_sources, expected_message, expected_outp [ MockConfig( source="command line", + serverurl=None, username=None, org=None, activation_key="test", @@ -291,6 +311,7 @@ def test_handle_config_conflicts_only_warnings(config_sources, expected_message, activation_key=None, password=None, no_rpm_va=True, + serverurl=None, ), MockConfig( source="configuration file", @@ -322,8 +343,8 @@ def test_handle_config_conflicts_system_exit(config_sources): @pytest.mark.parametrize( ("config_sources",), ( - ([MockConfig(source="command line", org="test", activation_key=None)],), - ([MockConfig(source="command line", org=None, activation_key="test")],), + ([MockConfig(source="command line", serverurl=None, org="test", activation_key=None)],), + ([MockConfig(source="command line", serverurl=None, org=None, activation_key="test")],), ), ) def test_handle_missing_options(config_sources): diff --git a/convert2rhel/utils/subscription.py b/convert2rhel/utils/subscription.py index 47b8e2ae0..d23563f0f 100644 --- a/convert2rhel/utils/subscription.py +++ b/convert2rhel/utils/subscription.py @@ -27,7 +27,8 @@ def setup_rhsm_parts(opts): - rhsm_parts = {} + # We initialize the values here as a measurement to give the complete dictionary back to the caller. + rhsm_parts = {"rhsm_hostname": None, "rhsm_port": None, "rhsm_prefix": None} if opts.serverurl: if opts.no_rhsm: diff --git a/tests/integration/tier0/non-destructive/config-file/test_config_file.py b/tests/integration/tier0/non-destructive/config-file/test_config_file.py index 4406f146d..392cbf8ca 100644 --- a/tests/integration/tier0/non-destructive/config-file/test_config_file.py +++ b/tests/integration/tier0/non-destructive/config-file/test_config_file.py @@ -92,7 +92,8 @@ def test_user_path_custom_filename(convert2rhel, c2r_config_setup): filename and the path is passed to the utility command. """ with convert2rhel('--debug -c "~/.convert2rhel_custom.ini"') as c2r: - c2r.expect("DEBUG - Found activation_key in /root/.convert2rhel_custom.ini") + c2r.expect("DEBUG - Checking configuration file at /root/.convert2rhel.ini") + c2r.expect("DEBUG - Found activation_key in subscription_manager") assert c2r.exitstatus == 1 @@ -172,8 +173,10 @@ def test_config_standard_paths_priority_diff_methods(convert2rhel, c2r_config_se (password and activation key) the activation key is preferred. """ with convert2rhel(f"analyze --serverurl {TEST_VARS['RHSM_SERVER_URL']} -y --debug") as c2r: - c2r.expect("DEBUG - Found password in /root/.convert2rhel.ini") - c2r.expect("DEBUG - Found activation_key in /etc/convert2rhel.ini") + c2r.expect("DEBUG - Checking configuration file at /root/.convert2rhel.ini", timeout=120) + c2r.expect("DEBUG - Found activation_key in subscription_manager") + c2r.expect("DEBUG - Checking configuration file at /etc/convert2rhel.ini", timeout=120) + c2r.expect("DEBUG - Found password in subscription_manager") c2r.expect( "WARNING - Passing the RHSM password or activation key through the --activationkey or --password options" " is insecure as it leaks the values through the list of running processes." @@ -222,8 +225,8 @@ def test_config_standard_paths_priority(convert2rhel, c2r_config_setup): Config file located in the home folder to be preferred. """ with convert2rhel(f"analyze --serverurl {TEST_VARS['RHSM_SERVER_URL']} -y --debug") as c2r: - c2r.expect("DEBUG - Found username in /root/.convert2rhel.ini") - c2r.expect("DEBUG - Found password in /root/.convert2rhel.ini") + c2r.expect("DEBUG - Found username in subscription_manager") + c2r.expect("DEBUG - Found password in subscription_manager") c2r.expect("SUBSCRIBE_SYSTEM has succeeded") assert c2r.exitstatus == 0