From c11cd8986a3fae06e14bac7c9168a79d4e61a2a6 Mon Sep 17 00:00:00 2001 From: Alexei Znamensky Date: Wed, 30 Jul 2025 16:53:50 +1200 Subject: [PATCH 1/6] apk: command args as list rather than string --- plugins/modules/apk.py | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/plugins/modules/apk.py b/plugins/modules/apk.py index 7ad5d5908e..03087acf2a 100644 --- a/plugins/modules/apk.py +++ b/plugins/modules/apk.py @@ -184,7 +184,7 @@ def parse_for_packages(stdout): def update_package_db(module, exit): - cmd = "%s update" % (APK_PATH) + cmd = [APK_PATH, "update"] rc, stdout, stderr = module.run_command(cmd, check_rc=False) if rc != 0: module.fail_json(msg="could not update package db", stdout=stdout, stderr=stderr) @@ -207,7 +207,7 @@ def query_toplevel(module, name, world): def query_package(module, name): - cmd = "%s -v info --installed %s" % (APK_PATH, name) + cmd = [APK_PATH, "-v", "info", "--installed", name] rc, stdout, stderr = module.run_command(cmd, check_rc=False) if rc == 0: return True @@ -216,7 +216,7 @@ def query_package(module, name): def query_latest(module, name): - cmd = "%s version %s" % (APK_PATH, name) + cmd = [APK_PATH, "version", name] rc, stdout, stderr = module.run_command(cmd, check_rc=False) search_pattern = r"(%s)-[\d\.\w]+-[\d\w]+\s+(.)\s+[\d\.\w]+-[\d\w]+\s+" % (re.escape(name)) match = re.search(search_pattern, stdout) @@ -226,7 +226,7 @@ def query_latest(module, name): def query_virtual(module, name): - cmd = "%s -v info --description %s" % (APK_PATH, name) + cmd = [APK_PATH, "-v", "info", "--description", name] rc, stdout, stderr = module.run_command(cmd, check_rc=False) search_pattern = r"^%s: virtual meta package" % (re.escape(name)) if re.search(search_pattern, stdout): @@ -235,7 +235,7 @@ def query_virtual(module, name): def get_dependencies(module, name): - cmd = "%s -v info --depends %s" % (APK_PATH, name) + cmd = [APK_PATH, "-v", "info", "--depends", name] rc, stdout, stderr = module.run_command(cmd, check_rc=False) dependencies = stdout.split() if len(dependencies) > 1: @@ -246,11 +246,11 @@ def get_dependencies(module, name): def upgrade_packages(module, available): if module.check_mode: - cmd = "%s upgrade --simulate" % (APK_PATH) + cmd = [APK_PATH, "upgrade", "--simulate"] else: - cmd = "%s upgrade" % (APK_PATH) + cmd = [APK_PATH, "upgrade"] if available: - cmd = "%s --available" % cmd + cmd.append("--available") rc, stdout, stderr = module.run_command(cmd, check_rc=False) packagelist = parse_for_packages(stdout) if rc != 0: @@ -284,14 +284,14 @@ def install_packages(module, names, state, world): packages = " ".join(to_install + to_upgrade) if upgrade: if module.check_mode: - cmd = "%s add --upgrade --simulate %s" % (APK_PATH, packages) + cmd = [APK_PATH, "add", "--upgrade", "--simulate", packages] else: - cmd = "%s add --upgrade %s" % (APK_PATH, packages) + cmd = [APK_PATH, "add", "--upgrade", packages] else: if module.check_mode: - cmd = "%s add --simulate %s" % (APK_PATH, packages) + cmd = [APK_PATH, "add", "--simulate", packages] else: - cmd = "%s add %s" % (APK_PATH, packages) + cmd = [APK_PATH, "add", packages] rc, stdout, stderr = module.run_command(cmd, check_rc=False) packagelist = parse_for_packages(stdout) if rc != 0: @@ -308,9 +308,9 @@ def remove_packages(module, names): module.exit_json(changed=False, msg="package(s) already removed") names = " ".join(installed) if module.check_mode: - cmd = "%s del --purge --simulate %s" % (APK_PATH, names) + cmd = [APK_PATH, "del", "--purge", "--simulate", names] else: - cmd = "%s del --purge %s" % (APK_PATH, names) + cmd = [APK_PATH, "del", "--purge", names] rc, stdout, stderr = module.run_command(cmd, check_rc=False) packagelist = parse_for_packages(stdout) # Check to see if packages are still present because of dependencies From 8dfd68e2d62970913aae32f8b719447a9d1b6573 Mon Sep 17 00:00:00 2001 From: Alexei Znamensky Date: Wed, 30 Jul 2025 17:06:01 +1200 Subject: [PATCH 2/6] add changelog frag --- changelogs/fragments/10520-arg-runcommand-list.yml | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 changelogs/fragments/10520-arg-runcommand-list.yml diff --git a/changelogs/fragments/10520-arg-runcommand-list.yml b/changelogs/fragments/10520-arg-runcommand-list.yml new file mode 100644 index 0000000000..4479b3a694 --- /dev/null +++ b/changelogs/fragments/10520-arg-runcommand-list.yml @@ -0,0 +1,2 @@ +minor_changes: + - apk - using safer mechanism to run external command (https://github.com/ansible-collections/community.general/issues/10479, https://github.com/ansible-collections/community.general/pull/10520). From 535433bcdd49f16daf8cf65873d9e8ba5cc0bd38 Mon Sep 17 00:00:00 2001 From: Alexei Znamensky Date: Wed, 30 Jul 2025 17:26:21 +1200 Subject: [PATCH 3/6] APK_PATH itself should be a list not a string --- plugins/modules/apk.py | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/plugins/modules/apk.py b/plugins/modules/apk.py index 03087acf2a..f0eefdbe1b 100644 --- a/plugins/modules/apk.py +++ b/plugins/modules/apk.py @@ -184,7 +184,7 @@ def parse_for_packages(stdout): def update_package_db(module, exit): - cmd = [APK_PATH, "update"] + cmd = APK_PATH + ["update"] rc, stdout, stderr = module.run_command(cmd, check_rc=False) if rc != 0: module.fail_json(msg="could not update package db", stdout=stdout, stderr=stderr) @@ -207,7 +207,7 @@ def query_toplevel(module, name, world): def query_package(module, name): - cmd = [APK_PATH, "-v", "info", "--installed", name] + cmd = APK_PATH + ["-v", "info", "--installed", name] rc, stdout, stderr = module.run_command(cmd, check_rc=False) if rc == 0: return True @@ -216,7 +216,7 @@ def query_package(module, name): def query_latest(module, name): - cmd = [APK_PATH, "version", name] + cmd = APK_PATH + ["version", name] rc, stdout, stderr = module.run_command(cmd, check_rc=False) search_pattern = r"(%s)-[\d\.\w]+-[\d\w]+\s+(.)\s+[\d\.\w]+-[\d\w]+\s+" % (re.escape(name)) match = re.search(search_pattern, stdout) @@ -226,7 +226,7 @@ def query_latest(module, name): def query_virtual(module, name): - cmd = [APK_PATH, "-v", "info", "--description", name] + cmd = APK_PATH + ["-v", "info", "--description", name] rc, stdout, stderr = module.run_command(cmd, check_rc=False) search_pattern = r"^%s: virtual meta package" % (re.escape(name)) if re.search(search_pattern, stdout): @@ -235,7 +235,7 @@ def query_virtual(module, name): def get_dependencies(module, name): - cmd = [APK_PATH, "-v", "info", "--depends", name] + cmd = APK_PATH + ["-v", "info", "--depends", name] rc, stdout, stderr = module.run_command(cmd, check_rc=False) dependencies = stdout.split() if len(dependencies) > 1: @@ -246,9 +246,9 @@ def get_dependencies(module, name): def upgrade_packages(module, available): if module.check_mode: - cmd = [APK_PATH, "upgrade", "--simulate"] + cmd = APK_PATH + ["upgrade", "--simulate"] else: - cmd = [APK_PATH, "upgrade"] + cmd = APK_PATH + ["upgrade"] if available: cmd.append("--available") rc, stdout, stderr = module.run_command(cmd, check_rc=False) @@ -284,14 +284,14 @@ def install_packages(module, names, state, world): packages = " ".join(to_install + to_upgrade) if upgrade: if module.check_mode: - cmd = [APK_PATH, "add", "--upgrade", "--simulate", packages] + cmd = APK_PATH + ["add", "--upgrade", "--simulate", packages] else: - cmd = [APK_PATH, "add", "--upgrade", packages] + cmd = APK_PATH + ["add", "--upgrade", packages] else: if module.check_mode: - cmd = [APK_PATH, "add", "--simulate", packages] + cmd = APK_PATH + ["add", "--simulate", packages] else: - cmd = [APK_PATH, "add", packages] + cmd = APK_PATH + ["add", packages] rc, stdout, stderr = module.run_command(cmd, check_rc=False) packagelist = parse_for_packages(stdout) if rc != 0: @@ -308,9 +308,9 @@ def remove_packages(module, names): module.exit_json(changed=False, msg="package(s) already removed") names = " ".join(installed) if module.check_mode: - cmd = [APK_PATH, "del", "--purge", "--simulate", names] + cmd = APK_PATH + ["del", "--purge", "--simulate", names] else: - cmd = [APK_PATH, "del", "--purge", names] + cmd = APK_PATH + ["del", "--purge", names] rc, stdout, stderr = module.run_command(cmd, check_rc=False) packagelist = parse_for_packages(stdout) # Check to see if packages are still present because of dependencies @@ -347,7 +347,7 @@ def main(): module.run_command_environ_update = dict(LANG='C', LC_ALL='C', LC_MESSAGES='C', LC_CTYPE='C') global APK_PATH - APK_PATH = module.get_bin_path('apk', required=True) + APK_PATH = [module.get_bin_path('apk', required=True)] p = module.params @@ -355,12 +355,12 @@ def main(): module.fail_json(msg="Package name(s) cannot be empty or whitespace-only") if p['no_cache']: - APK_PATH = "%s --no-cache" % (APK_PATH, ) + APK_PATH.append("--no-cache") # add repositories to the APK_PATH if p['repository']: for r in p['repository']: - APK_PATH = "%s --repository %s --repositories-file /dev/null" % (APK_PATH, r) + APK_PATH.extend(["--repository", r, "--repositories-file", "/dev/null"]) # normalize the state parameter if p['state'] in ['present', 'installed']: From 46a6a4cc154acf2afbae56eee55c8b7daa5b73b9 Mon Sep 17 00:00:00 2001 From: Alexei Znamensky Date: Wed, 30 Jul 2025 21:17:43 +1200 Subject: [PATCH 4/6] fix mock values in unit tests --- tests/unit/plugins/modules/test_apk.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/plugins/modules/test_apk.py b/tests/unit/plugins/modules/test_apk.py index b25d10f207..2f21f9bea4 100644 --- a/tests/unit/plugins/modules/test_apk.py +++ b/tests/unit/plugins/modules/test_apk.py @@ -21,7 +21,7 @@ class TestApkQueryLatest(unittest.TestCase): @mock.patch('ansible_collections.community.general.plugins.modules.apk.AnsibleModule') def test_not_latest(self, mock_module): - apk.APK_PATH = "" + apk.APK_PATH = [""] for module_name in self.module_names: command_output = module_name + '-2.0.0-r1 < 3.0.0-r2 ' mock_module.run_command.return_value = (0, command_output, None) @@ -30,7 +30,7 @@ class TestApkQueryLatest(unittest.TestCase): @mock.patch('ansible_collections.community.general.plugins.modules.apk.AnsibleModule') def test_latest(self, mock_module): - apk.APK_PATH = "" + apk.APK_PATH = [""] for module_name in self.module_names: command_output = module_name + '-2.0.0-r1 = 2.0.0-r1 ' mock_module.run_command.return_value = (0, command_output, None) From da9bdfe8181d5a4a67d018d9005b5a6c4883cb98 Mon Sep 17 00:00:00 2001 From: Alexei Znamensky Date: Wed, 30 Jul 2025 21:51:06 +1200 Subject: [PATCH 5/6] keep package names as list --- plugins/modules/apk.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/modules/apk.py b/plugins/modules/apk.py index f0eefdbe1b..9771e827d6 100644 --- a/plugins/modules/apk.py +++ b/plugins/modules/apk.py @@ -281,7 +281,7 @@ def install_packages(module, names, state, world): upgrade = True if not to_install and not upgrade: module.exit_json(changed=False, msg="package(s) already installed") - packages = " ".join(to_install + to_upgrade) + packages = to_install + to_upgrade if upgrade: if module.check_mode: cmd = APK_PATH + ["add", "--upgrade", "--simulate", packages] @@ -306,7 +306,7 @@ def remove_packages(module, names): installed.append(name) if not installed: module.exit_json(changed=False, msg="package(s) already removed") - names = " ".join(installed) + names = installed if module.check_mode: cmd = APK_PATH + ["del", "--purge", "--simulate", names] else: From 6a4a03446c6613ca17b131c2f5ee715248f5c117 Mon Sep 17 00:00:00 2001 From: Alexei Znamensky Date: Thu, 31 Jul 2025 19:50:50 +1200 Subject: [PATCH 6/6] add package names as list to cmd line --- plugins/modules/apk.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/plugins/modules/apk.py b/plugins/modules/apk.py index 9771e827d6..272fd1be63 100644 --- a/plugins/modules/apk.py +++ b/plugins/modules/apk.py @@ -284,14 +284,14 @@ def install_packages(module, names, state, world): packages = to_install + to_upgrade if upgrade: if module.check_mode: - cmd = APK_PATH + ["add", "--upgrade", "--simulate", packages] + cmd = APK_PATH + ["add", "--upgrade", "--simulate"] + packages else: - cmd = APK_PATH + ["add", "--upgrade", packages] + cmd = APK_PATH + ["add", "--upgrade"] + packages else: if module.check_mode: - cmd = APK_PATH + ["add", "--simulate", packages] + cmd = APK_PATH + ["add", "--simulate"] + packages else: - cmd = APK_PATH + ["add", packages] + cmd = APK_PATH + ["add"] + packages rc, stdout, stderr = module.run_command(cmd, check_rc=False) packagelist = parse_for_packages(stdout) if rc != 0: @@ -308,9 +308,9 @@ def remove_packages(module, names): module.exit_json(changed=False, msg="package(s) already removed") names = installed if module.check_mode: - cmd = APK_PATH + ["del", "--purge", "--simulate", names] + cmd = APK_PATH + ["del", "--purge", "--simulate"] + names else: - cmd = APK_PATH + ["del", "--purge", names] + cmd = APK_PATH + ["del", "--purge"] + names rc, stdout, stderr = module.run_command(cmd, check_rc=False) packagelist = parse_for_packages(stdout) # Check to see if packages are still present because of dependencies