[PR #6669/74ffb295 backport][stable-7] rhsm_release: improve the execution of subscription-manager (#6674)

rhsm_release: improve the execution of subscription-manager (#6669)

- pass the arguments to run_command() directly as list, rather than
  joining the arguments to string, which run_command() will need to
  split again
- disable the expansions of variables, as there are none

Adapt the unit test to the different way run_command() is called,
factorizing the kwargs for run_command() so there is less repetition.

There should be no behaviour changes.

(cherry picked from commit 74ffb29573)

Co-authored-by: Pino Toscano <ptoscano@redhat.com>

changelogs/fragments/6669-rhsm_release-internal-sub-man-exec.yml

@@ -0,0 +1,5 @@
+minor_changes:
+  - |
+    rhsm_release - improve/harden the way ``subscription-manager`` is run;
+    no behaviour change is expected
+    (https://github.com/ansible-collections/community.general/pull/6669).

plugins/modules/rhsm_release.py

@@ -77,9 +77,9 @@ def _sm_release(module, *args):
     # pass args to s-m release, e.g. _sm_release(module, '--set', '0.1') becomes
     # "subscription-manager release --set 0.1"
     sm_bin = module.get_bin_path('subscription-manager', required=True)
-    cmd = '{0} release {1}'.format(sm_bin, " ".join(args))
+    cmd = [sm_bin, 'release'] + list(args)
     # delegate nonzero rc handling to run_command
-    return module.run_command(cmd, check_rc=True)
+    return module.run_command(cmd, check_rc=True, expand_user_and_vars=False)
 
 
 def get_release(module):

tests/unit/plugins/modules/test_rhsm_release.py

@@ -14,6 +14,8 @@ from ansible_collections.community.general.tests.unit.plugins.modules.utils impo
 class RhsmRepositoryReleaseModuleTestCase(ModuleTestCase):
     module = rhsm_release
 
+    SUBMAN_KWARGS = dict(check_rc=True, expand_user_and_vars=False)
+
     def setUp(self):
         super(RhsmRepositoryReleaseModuleTestCase, self).setUp()
 
@@ -63,8 +65,8 @@ class RhsmRepositoryReleaseModuleTestCase(ModuleTestCase):
         self.assertTrue(result['changed'])
         self.assertEqual('7.5', result['current_release'])
         self.module_main_command.assert_has_calls([
-            call('/testbin/subscription-manager release --show', check_rc=True),
+            call(['/testbin/subscription-manager', 'release', '--show'], **self.SUBMAN_KWARGS),
-            call('/testbin/subscription-manager release --set 7.5', check_rc=True),
+            call(['/testbin/subscription-manager', 'release', '--set', '7.5'], **self.SUBMAN_KWARGS),
         ])
 
     def test_release_set_idempotent(self):
@@ -81,7 +83,7 @@ class RhsmRepositoryReleaseModuleTestCase(ModuleTestCase):
         self.assertFalse(result['changed'])
         self.assertEqual('7.5', result['current_release'])
         self.module_main_command.assert_has_calls([
-            call('/testbin/subscription-manager release --show', check_rc=True),
+            call(['/testbin/subscription-manager', 'release', '--show'], **self.SUBMAN_KWARGS),
         ])
 
     def test_release_unset(self):
@@ -100,8 +102,8 @@ class RhsmRepositoryReleaseModuleTestCase(ModuleTestCase):
         self.assertTrue(result['changed'])
         self.assertIsNone(result['current_release'])
         self.module_main_command.assert_has_calls([
-            call('/testbin/subscription-manager release --show', check_rc=True),
+            call(['/testbin/subscription-manager', 'release', '--show'], **self.SUBMAN_KWARGS),
-            call('/testbin/subscription-manager release --unset', check_rc=True),
+            call(['/testbin/subscription-manager', 'release', '--unset'], **self.SUBMAN_KWARGS),
         ])
 
     def test_release_unset_idempotent(self):
@@ -118,7 +120,7 @@ class RhsmRepositoryReleaseModuleTestCase(ModuleTestCase):
         self.assertFalse(result['changed'])
         self.assertIsNone(result['current_release'])
         self.module_main_command.assert_has_calls([
-            call('/testbin/subscription-manager release --show', check_rc=True),
+            call(['/testbin/subscription-manager', 'release', '--show'], **self.SUBMAN_KWARGS),
         ])
 
     def test_release_insane(self):