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

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).

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)

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}]
         },
         {