From 74ffb29573491798b889a53d6cc57444ad37f630 Mon Sep 17 00:00:00 2001 From: Pino Toscano Date: Sun, 11 Jun 2023 10:34:25 +0200 Subject: [PATCH] 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. --- .../6669-rhsm_release-internal-sub-man-exec.yml | 5 +++++ plugins/modules/rhsm_release.py | 4 ++-- tests/unit/plugins/modules/test_rhsm_release.py | 14 ++++++++------ 3 files changed, 15 insertions(+), 8 deletions(-) create mode 100644 changelogs/fragments/6669-rhsm_release-internal-sub-man-exec.yml diff --git a/changelogs/fragments/6669-rhsm_release-internal-sub-man-exec.yml b/changelogs/fragments/6669-rhsm_release-internal-sub-man-exec.yml new file mode 100644 index 0000000000..a332a14151 --- /dev/null +++ b/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). diff --git a/plugins/modules/rhsm_release.py b/plugins/modules/rhsm_release.py index 6ac4da6e4f..4e14dff39d 100644 --- a/plugins/modules/rhsm_release.py +++ b/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): diff --git a/tests/unit/plugins/modules/test_rhsm_release.py b/tests/unit/plugins/modules/test_rhsm_release.py index c5696962b5..e8b2db6fdc 100644 --- a/tests/unit/plugins/modules/test_rhsm_release.py +++ b/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 --set 7.5', check_rc=True), + call(['/testbin/subscription-manager', 'release', '--show'], **self.SUBMAN_KWARGS), + 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 --unset', check_rc=True), + call(['/testbin/subscription-manager', 'release', '--show'], **self.SUBMAN_KWARGS), + 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):