From bbd68e26a2e8ffa8926b98fa5eced9aeb7fedfec Mon Sep 17 00:00:00 2001 From: Pino Toscano Date: Wed, 22 Mar 2023 20:19:55 +0100 Subject: [PATCH] redhat_subscription: require credentials only when needed (#5664) The module currently has a static 'required_if' statement for its parameters that forces any of 'username' or 'activationkey' or 'token' in case state=present; while this is generally a good idea, it can be an extra requirements in some cases. In particular, if the system is already registered, there is no need for credentials -- some of the operations of the module, such as manipulating pools, can be done perfectly without credentials. Hence: - change the static 'required_if' to require credentials only when forcing the registration - check for credentials manually when a registration is needed, i.e. on an unregistered system; the fail message is the same as the one shown by 'required_if' Adapt the tests to this new situation: - test_without_required_parameters now needs to mock an unregistered system - add a new version of test_without_required_parameters to test an already registered system - add a simple test case for only state=present usable on an already registered system - remove the credentials from a test case for pool attachment that mocks an already registered system --- ..._subscription-credentials-when-needed.yaml | 4 ++ plugins/modules/redhat_subscription.py | 9 +++- .../modules/test_redhat_subscription.py | 49 +++++++++++++++++-- 3 files changed, 57 insertions(+), 5 deletions(-) create mode 100644 changelogs/fragments/5664-redhat_subscription-credentials-when-needed.yaml diff --git a/changelogs/fragments/5664-redhat_subscription-credentials-when-needed.yaml b/changelogs/fragments/5664-redhat_subscription-credentials-when-needed.yaml new file mode 100644 index 0000000000..c300b957ce --- /dev/null +++ b/changelogs/fragments/5664-redhat_subscription-credentials-when-needed.yaml @@ -0,0 +1,4 @@ +minor_changes: + - redhat_subscription - credentials (``username``, ``activationkey``, and so on) are required now + only if a system needs to be registered, or ``force_register`` is specified + (https://github.com/ansible-collections/community.general/pull/5664). diff --git a/plugins/modules/redhat_subscription.py b/plugins/modules/redhat_subscription.py index d5f343f23b..45b3cafb3d 100644 --- a/plugins/modules/redhat_subscription.py +++ b/plugins/modules/redhat_subscription.py @@ -26,6 +26,11 @@ notes: config file and default to None. - It is possible to interact with C(subscription-manager) only as root, so root permissions are required to successfully run this module. + - Since community.general 6.5.0, credentials (that is, I(username) and I(password), + I(activationkey), or I(token)) are needed only in case the the system is not registered, + or I(force_register) is specified; this makes it possible to use the module to tweak an + already registered system, for example attaching pools to it (using I(pool), or I(pool_ids)), + and modifying the C(syspurpose) attributes (using I(syspurpose)). requirements: - subscription-manager - Optionally the C(dbus) Python library; this is usually included in the OS @@ -1073,7 +1078,7 @@ def main(): ['activationkey', 'environment'], ['activationkey', 'auto_attach'], ['pool', 'pool_ids']], - required_if=[['state', 'present', ['username', 'activationkey', 'token'], True]], + required_if=[['force_register', True, ['username', 'activationkey', 'token'], True]], ) if getuid() != 0: @@ -1155,6 +1160,8 @@ def main(): else: module.exit_json(changed=False, msg="System already registered.") else: + if not username and not activationkey and not token: + module.fail_json(msg="state is present but any of the following are missing: username, activationkey, token") try: rhsm.enable() rhsm.configure(**module.params) diff --git a/tests/unit/plugins/modules/test_redhat_subscription.py b/tests/unit/plugins/modules/test_redhat_subscription.py index bc1bfffe66..4bf2729163 100644 --- a/tests/unit/plugins/modules/test_redhat_subscription.py +++ b/tests/unit/plugins/modules/test_redhat_subscription.py @@ -35,10 +35,15 @@ def patch_redhat_subscription(mocker): @pytest.mark.parametrize('patch_ansible_module', [{}], indirect=['patch_ansible_module']) @pytest.mark.usefixtures('patch_ansible_module') -def test_without_required_parameters(capfd, patch_redhat_subscription): +def test_without_required_parameters_unregistered(mocker, capfd, patch_redhat_subscription): """ Failure must occurs when all parameters are missing """ + mock_run_command = mocker.patch.object( + basic.AnsibleModule, + 'run_command', + return_value=(1, 'This system is not yet registered.', '')) + with pytest.raises(SystemExit): redhat_subscription.main() out, err = capfd.readouterr() @@ -47,6 +52,27 @@ def test_without_required_parameters(capfd, patch_redhat_subscription): assert 'state is present but any of the following are missing' in results['msg'] +@pytest.mark.parametrize('patch_ansible_module', [{}], indirect=['patch_ansible_module']) +@pytest.mark.usefixtures('patch_ansible_module') +def test_without_required_parameters_registered(mocker, capfd, patch_redhat_subscription): + """ + System already registered, no parameters required (state=present is the + default) + """ + mock_run_command = mocker.patch.object( + basic.AnsibleModule, + 'run_command', + return_value=(0, 'system identity: b26df632-25ed-4452-8f89-0308bfd167cb', '')) + + with pytest.raises(SystemExit): + redhat_subscription.main() + out, err = capfd.readouterr() + results = json.loads(out) + assert 'changed' in results + if 'msg' in results: + assert results['msg'] == 'System already registered.' + + TEST_CASES = [ # Test the case, when the system is already registered [ @@ -73,6 +99,24 @@ TEST_CASES = [ 'msg': 'System already registered.' } ], + # Already registered system without credentials specified + [ + { + 'state': 'present', + }, + { + 'id': 'test_already_registered_system', + 'run_command.calls': [ + ( + ['/testbin/subscription-manager', 'identity'], + {'check_rc': False}, + (0, 'system identity: b26df632-25ed-4452-8f89-0308bfd167cb', '') + ) + ], + 'changed': False, + 'msg': 'System already registered.' + } + ], # Test simple registration using username and password [ { @@ -744,9 +788,6 @@ Entitlement Type: Physical [ { 'state': 'present', - 'username': 'admin', - 'password': 'admin', - 'org_id': 'admin', 'pool_ids': [{'ff8080816b8e967f016b8e99632804a6': 2}, {'ff8080816b8e967f016b8e99747107e9': 4}] }, {