diff --git a/changelogs/fragments/8264-run_command.yml b/changelogs/fragments/8264-run_command.yml new file mode 100644 index 0000000000..dd66cd6123 --- /dev/null +++ b/changelogs/fragments/8264-run_command.yml @@ -0,0 +1,14 @@ +minor_changes: + - "aix_lvol - refactor module to pass list of arguments to ``module.run_command()`` instead of relying on interpretation by a shell (https://github.com/ansible-collections/community.general/pull/8264)." + - "apt_rpm - refactor module to pass list of arguments to ``module.run_command()`` instead of relying on interpretation by a shell (https://github.com/ansible-collections/community.general/pull/8264)." + - "btrfs_subvolume - refactor module to pass list of arguments to ``module.run_command()`` instead of relying on interpretation by a shell (https://github.com/ansible-collections/community.general/pull/8264)." + - "installp - refactor module to pass list of arguments to ``module.run_command()`` instead of relying on interpretation by a shell (https://github.com/ansible-collections/community.general/pull/8264)." + - "lvg - refactor module to pass list of arguments to ``module.run_command()`` instead of relying on interpretation by a shell (https://github.com/ansible-collections/community.general/pull/8264)." + - "lvol - refactor module to pass list of arguments to ``module.run_command()`` instead of relying on interpretation by a shell (https://github.com/ansible-collections/community.general/pull/8264)." + - "macports - refactor module to pass list of arguments to ``module.run_command()`` instead of relying on interpretation by a shell (https://github.com/ansible-collections/community.general/pull/8264)." + - "parted - refactor module to pass list of arguments to ``module.run_command()`` instead of relying on interpretation by a shell (https://github.com/ansible-collections/community.general/pull/8264)." + - "pkgin - refactor module to pass list of arguments to ``module.run_command()`` instead of relying on interpretation by a shell (https://github.com/ansible-collections/community.general/pull/8264)." + - "portinstall - refactor module to pass list of arguments to ``module.run_command()`` instead of relying on interpretation by a shell (https://github.com/ansible-collections/community.general/pull/8264)." + - "slackpkg - refactor module to pass list of arguments to ``module.run_command()`` instead of relying on interpretation by a shell (https://github.com/ansible-collections/community.general/pull/8264)." + - "svr4pkg - refactor module to pass list of arguments to ``module.run_command()`` instead of relying on interpretation by a shell (https://github.com/ansible-collections/community.general/pull/8264)." + - "swdepot - refactor module to pass list of arguments to ``module.run_command()`` instead of relying on interpretation by a shell (https://github.com/ansible-collections/community.general/pull/8264)." diff --git a/plugins/modules/aix_lvol.py b/plugins/modules/aix_lvol.py index 1e7b425687..7d0fb1ee09 100644 --- a/plugins/modules/aix_lvol.py +++ b/plugins/modules/aix_lvol.py @@ -240,8 +240,6 @@ def main(): state = module.params['state'] pvs = module.params['pvs'] - pv_list = ' '.join(pvs) - if policy == 'maximum': lv_policy = 'x' else: @@ -249,16 +247,16 @@ def main(): # Add echo command when running in check-mode if module.check_mode: - test_opt = 'echo ' + test_opt = [module.get_bin_path("echo", required=True)] else: - test_opt = '' + test_opt = [] # check if system commands are available lsvg_cmd = module.get_bin_path("lsvg", required=True) lslv_cmd = module.get_bin_path("lslv", required=True) # Get information on volume group requested - rc, vg_info, err = module.run_command("%s %s" % (lsvg_cmd, vg)) + rc, vg_info, err = module.run_command([lsvg_cmd, vg]) if rc != 0: if state == 'absent': @@ -273,8 +271,7 @@ def main(): lv_size = round_ppsize(convert_size(module, size), base=this_vg['pp_size']) # Get information on logical volume requested - rc, lv_info, err = module.run_command( - "%s %s" % (lslv_cmd, lv)) + rc, lv_info, err = module.run_command([lslv_cmd, lv]) if rc != 0: if state == 'absent': @@ -296,7 +293,7 @@ def main(): # create LV mklv_cmd = module.get_bin_path("mklv", required=True) - cmd = "%s %s -t %s -y %s -c %s -e %s %s %s %sM %s" % (test_opt, mklv_cmd, lv_type, lv, copies, lv_policy, opts, vg, lv_size, pv_list) + cmd = test_opt + [mklv_cmd, "-t", lv_type, "-y", lv, "-c", copies, "-e", lv_policy, opts, vg, "%sM" % (lv_size, )] + pvs rc, out, err = module.run_command(cmd) if rc == 0: module.exit_json(changed=True, msg="Logical volume %s created." % lv) @@ -306,7 +303,7 @@ def main(): if state == 'absent': # remove LV rmlv_cmd = module.get_bin_path("rmlv", required=True) - rc, out, err = module.run_command("%s %s -f %s" % (test_opt, rmlv_cmd, this_lv['name'])) + rc, out, err = module.run_command(test_opt + [rmlv_cmd, "-f", this_lv['name']]) if rc == 0: module.exit_json(changed=True, msg="Logical volume %s deleted." % lv) else: @@ -315,7 +312,7 @@ def main(): if this_lv['policy'] != policy: # change lv allocation policy chlv_cmd = module.get_bin_path("chlv", required=True) - rc, out, err = module.run_command("%s %s -e %s %s" % (test_opt, chlv_cmd, lv_policy, this_lv['name'])) + rc, out, err = module.run_command(test_opt + [chlv_cmd, "-e", lv_policy, this_lv['name']]) if rc == 0: module.exit_json(changed=True, msg="Logical volume %s policy changed: %s." % (lv, policy)) else: @@ -331,7 +328,7 @@ def main(): # resize LV based on absolute values if int(lv_size) > this_lv['size']: extendlv_cmd = module.get_bin_path("extendlv", required=True) - cmd = "%s %s %s %sM" % (test_opt, extendlv_cmd, lv, lv_size - this_lv['size']) + cmd = test_opt + [extendlv_cmd, lv, "%sM" % (lv_size - this_lv['size'], )] rc, out, err = module.run_command(cmd) if rc == 0: module.exit_json(changed=True, msg="Logical volume %s size extended to %sMB." % (lv, lv_size)) diff --git a/plugins/modules/apt_rpm.py b/plugins/modules/apt_rpm.py index 03b87e78f0..07da307633 100644 --- a/plugins/modules/apt_rpm.py +++ b/plugins/modules/apt_rpm.py @@ -170,7 +170,7 @@ def local_rpm_package_name(path): def query_package(module, name): # rpm -q returns 0 if the package is installed, # 1 if it is not installed - rc, out, err = module.run_command("%s -q %s" % (RPM_PATH, name)) + rc, out, err = module.run_command([RPM_PATH, "-q", name]) if rc == 0: return True else: @@ -203,7 +203,7 @@ def query_package_provides(module, name, allow_upgrade=False): name = local_rpm_package_name(name) - rc, out, err = module.run_command("%s -q --provides %s" % (RPM_PATH, name)) + rc, out, err = module.run_command([RPM_PATH, "-q", "--provides", name]) if rc == 0: if not allow_upgrade: return True @@ -253,7 +253,7 @@ def remove_packages(module, packages): if not query_package(module, package): continue - rc, out, err = module.run_command("%s -y remove %s" % (APT_PATH, package), environ_update={"LANG": "C"}) + rc, out, err = module.run_command([APT_PATH, "-y", "remove", package], environ_update={"LANG": "C"}) if rc != 0: module.fail_json(msg="failed to remove %s: %s" % (package, err)) @@ -271,14 +271,14 @@ def install_packages(module, pkgspec, allow_upgrade=False): if pkgspec is None: return (False, "Empty package list") - packages = "" + packages = [] for package in pkgspec: if not query_package_provides(module, package, allow_upgrade=allow_upgrade): - packages += "'%s' " % package + packages.append(package) - if len(packages) != 0: - - rc, out, err = module.run_command("%s -y install %s" % (APT_PATH, packages), environ_update={"LANG": "C"}) + if packages: + command = [APT_PATH, "-y", "install"] + packages + rc, out, err = module.run_command(command, environ_update={"LANG": "C"}) installed = True for package in pkgspec: @@ -287,7 +287,7 @@ def install_packages(module, pkgspec, allow_upgrade=False): # apt-rpm always have 0 for exit code if --force is used if rc or not installed: - module.fail_json(msg="'apt-get -y install %s' failed: %s" % (packages, err)) + module.fail_json(msg="'%s' failed: %s" % (" ".join(command), err)) else: return (True, "%s present(s)" % packages) else: diff --git a/plugins/modules/btrfs_subvolume.py b/plugins/modules/btrfs_subvolume.py index 864bb65a66..35327bfe02 100644 --- a/plugins/modules/btrfs_subvolume.py +++ b/plugins/modules/btrfs_subvolume.py @@ -572,10 +572,7 @@ class BtrfsSubvolumeModule(object): self.__temporary_mounts[cache_key] = mountpoint mount = self.module.get_bin_path("mount", required=True) - command = "%s -o noatime,subvolid=%d %s %s " % (mount, - subvolid, - device, - mountpoint) + command = [mount, "-o", "noatime,subvolid=%d" % subvolid, device, mountpoint] result = self.module.run_command(command, check_rc=True) return mountpoint @@ -586,10 +583,10 @@ class BtrfsSubvolumeModule(object): def __cleanup_mount(self, mountpoint): umount = self.module.get_bin_path("umount", required=True) - result = self.module.run_command("%s %s" % (umount, mountpoint)) + result = self.module.run_command([umount, mountpoint]) if result[0] == 0: rmdir = self.module.get_bin_path("rmdir", required=True) - self.module.run_command("%s %s" % (rmdir, mountpoint)) + self.module.run_command([rmdir, mountpoint]) # Format and return results def get_results(self): diff --git a/plugins/modules/installp.py b/plugins/modules/installp.py index 4b5a6949c6..1531d2cad2 100644 --- a/plugins/modules/installp.py +++ b/plugins/modules/installp.py @@ -106,7 +106,7 @@ def _check_new_pkg(module, package, repository_path): if os.path.isdir(repository_path): installp_cmd = module.get_bin_path('installp', True) - rc, package_result, err = module.run_command("%s -l -MR -d %s" % (installp_cmd, repository_path)) + rc, package_result, err = module.run_command([installp_cmd, "-l", "-MR", "-d", repository_path]) if rc != 0: module.fail_json(msg="Failed to run installp.", rc=rc, err=err) @@ -142,7 +142,7 @@ def _check_installed_pkg(module, package, repository_path): """ lslpp_cmd = module.get_bin_path('lslpp', True) - rc, lslpp_result, err = module.run_command("%s -lcq %s*" % (lslpp_cmd, package)) + rc, lslpp_result, err = module.run_command([lslpp_cmd, "-lcq", "%s*" % (package, )]) if rc == 1: package_state = ' '.join(err.split()[-2:]) @@ -173,7 +173,7 @@ def remove(module, installp_cmd, packages): if pkg_check: if not module.check_mode: - rc, remove_out, err = module.run_command("%s -u %s" % (installp_cmd, package)) + rc, remove_out, err = module.run_command([installp_cmd, "-u", package]) if rc != 0: module.fail_json(msg="Failed to run installp.", rc=rc, err=err) remove_count += 1 @@ -202,8 +202,8 @@ def install(module, installp_cmd, packages, repository_path, accept_license): already_installed_pkgs = {} accept_license_param = { - True: '-Y', - False: '', + True: ['-Y'], + False: [], } # Validate if package exists on repository path. @@ -230,7 +230,8 @@ def install(module, installp_cmd, packages, repository_path, accept_license): else: if not module.check_mode: - rc, out, err = module.run_command("%s -a %s -X -d %s %s" % (installp_cmd, accept_license_param[accept_license], repository_path, package)) + rc, out, err = module.run_command( + [installp_cmd, "-a"] + accept_license_param[accept_license] + ["-X", "-d", repository_path, package]) if rc != 0: module.fail_json(msg="Failed to run installp", rc=rc, err=err) installed_pkgs.append(package) diff --git a/plugins/modules/lvg.py b/plugins/modules/lvg.py index 8a6384369a..7ff7e3a2e7 100644 --- a/plugins/modules/lvg.py +++ b/plugins/modules/lvg.py @@ -179,7 +179,7 @@ def parse_vgs(data): def find_mapper_device_name(module, dm_device): dmsetup_cmd = module.get_bin_path('dmsetup', True) mapper_prefix = '/dev/mapper/' - rc, dm_name, err = module.run_command("%s info -C --noheadings -o name %s" % (dmsetup_cmd, dm_device)) + rc, dm_name, err = module.run_command([dmsetup_cmd, "info", "-C", "--noheadings", "-o", "name", dm_device]) if rc != 0: module.fail_json(msg="Failed executing dmsetup command.", rc=rc, err=err) mapper_device = mapper_prefix + dm_name.rstrip() @@ -204,7 +204,7 @@ def find_vg(module, vg): if not vg: return None vgs_cmd = module.get_bin_path('vgs', True) - dummy, current_vgs, dummy = module.run_command("%s --noheadings -o vg_name,pv_count,lv_count --separator ';'" % vgs_cmd, check_rc=True) + dummy, current_vgs, dummy = module.run_command([vgs_cmd, "--noheadings", "-o", "vg_name,pv_count,lv_count", "--separator", ";"], check_rc=True) vgs = parse_vgs(current_vgs) @@ -431,10 +431,10 @@ def main(): for x in itertools.chain(dev_list, module.params['pvs']) ) pvs_filter_vg_name = 'vg_name = {0}'.format(vg) - pvs_filter = "--select '{0} || {1}' ".format(pvs_filter_pv_name, pvs_filter_vg_name) + pvs_filter = ["--select", "{0} || {1}".format(pvs_filter_pv_name, pvs_filter_vg_name)] else: - pvs_filter = '' - rc, current_pvs, err = module.run_command("%s --noheadings -o pv_name,vg_name --separator ';' %s" % (pvs_cmd, pvs_filter)) + pvs_filter = [] + rc, current_pvs, err = module.run_command([pvs_cmd, "--noheadings", "-o", "pv_name,vg_name", "--separator", ";"] + pvs_filter) if rc != 0: module.fail_json(msg="Failed executing pvs command.", rc=rc, err=err) @@ -473,7 +473,7 @@ def main(): if this_vg['lv_count'] == 0 or force: # remove VG vgremove_cmd = module.get_bin_path('vgremove', True) - rc, dummy, err = module.run_command("%s --force %s" % (vgremove_cmd, vg)) + rc, dummy, err = module.run_command([vgremove_cmd, "--force", vg]) if rc == 0: module.exit_json(changed=True) else: @@ -509,7 +509,6 @@ def main(): changed = True else: if devs_to_add: - devs_to_add_string = ' '.join(devs_to_add) # create PV pvcreate_cmd = module.get_bin_path('pvcreate', True) for current_dev in devs_to_add: @@ -520,21 +519,20 @@ def main(): module.fail_json(msg="Creating physical volume '%s' failed" % current_dev, rc=rc, err=err) # add PV to our VG vgextend_cmd = module.get_bin_path('vgextend', True) - rc, dummy, err = module.run_command("%s %s %s" % (vgextend_cmd, vg, devs_to_add_string)) + rc, dummy, err = module.run_command([vgextend_cmd, vg] + devs_to_add) if rc == 0: changed = True else: - module.fail_json(msg="Unable to extend %s by %s." % (vg, devs_to_add_string), rc=rc, err=err) + module.fail_json(msg="Unable to extend %s by %s." % (vg, ' '.join(devs_to_add)), rc=rc, err=err) # remove some PV from our VG if devs_to_remove: - devs_to_remove_string = ' '.join(devs_to_remove) vgreduce_cmd = module.get_bin_path('vgreduce', True) - rc, dummy, err = module.run_command("%s --force %s %s" % (vgreduce_cmd, vg, devs_to_remove_string)) + rc, dummy, err = module.run_command([vgreduce_cmd, "--force", vg] + devs_to_remove) if rc == 0: changed = True else: - module.fail_json(msg="Unable to reduce %s by %s." % (vg, devs_to_remove_string), rc=rc, err=err) + module.fail_json(msg="Unable to reduce %s by %s." % (vg, ' '.join(devs_to_remove)), rc=rc, err=err) module.exit_json(changed=changed) diff --git a/plugins/modules/lvol.py b/plugins/modules/lvol.py index a2a870260a..3a2f5c7cdd 100644 --- a/plugins/modules/lvol.py +++ b/plugins/modules/lvol.py @@ -236,6 +236,7 @@ EXAMPLES = ''' ''' import re +import shlex from ansible.module_utils.basic import AnsibleModule @@ -281,7 +282,7 @@ def parse_vgs(data): def get_lvm_version(module): ver_cmd = module.get_bin_path("lvm", required=True) - rc, out, err = module.run_command("%s version" % (ver_cmd)) + rc, out, err = module.run_command([ver_cmd, "version"]) if rc != 0: return None m = re.search(r"LVM version:\s+(\d+)\.(\d+)\.(\d+).*(\d{4}-\d{2}-\d{2})", out) @@ -320,14 +321,14 @@ def main(): module.fail_json(msg="Failed to get LVM version number") version_yesopt = mkversion(2, 2, 99) # First LVM with the "--yes" option if version_found >= version_yesopt: - yesopt = "--yes" + yesopt = ["--yes"] else: - yesopt = "" + yesopt = [] vg = module.params['vg'] lv = module.params['lv'] size = module.params['size'] - opts = module.params['opts'] + opts = shlex.split(module.params['opts'] or '') state = module.params['state'] force = module.boolean(module.params['force']) shrink = module.boolean(module.params['shrink']) @@ -338,21 +339,13 @@ def main(): size_unit = 'm' size_operator = None snapshot = module.params['snapshot'] - pvs = module.params['pvs'] - - if pvs is None: - pvs = "" - else: - pvs = " ".join(pvs) - - if opts is None: - opts = "" + pvs = module.params['pvs'] or [] # Add --test option when running in check-mode if module.check_mode: - test_opt = ' --test' + test_opt = ['--test'] else: - test_opt = '' + test_opt = [] if size: # LVEXTEND(8)/LVREDUCE(8) -l, -L options: Check for relative value for resizing @@ -400,7 +393,7 @@ def main(): # Get information on volume group requested vgs_cmd = module.get_bin_path("vgs", required=True) rc, current_vgs, err = module.run_command( - "%s --noheadings --nosuffix -o vg_name,size,free,vg_extent_size --units %s --separator ';' %s" % (vgs_cmd, unit.lower(), vg)) + [vgs_cmd, "--noheadings", "--nosuffix", "-o", "vg_name,size,free,vg_extent_size", "--units", unit.lower(), "--separator", ";", vg]) if rc != 0: if state == 'absent': @@ -414,7 +407,7 @@ def main(): # Get information on logical volume requested lvs_cmd = module.get_bin_path("lvs", required=True) rc, current_lvs, err = module.run_command( - "%s -a --noheadings --nosuffix -o lv_name,size,lv_attr --units %s --separator ';' %s" % (lvs_cmd, unit.lower(), vg)) + [lvs_cmd, "-a", "--noheadings", "--nosuffix", "-o", "lv_name,size,lv_attr", "--units", unit.lower(), "--separator", ";", vg]) if rc != 0: if state == 'absent': @@ -474,20 +467,23 @@ def main(): # create LV lvcreate_cmd = module.get_bin_path("lvcreate", required=True) + cmd = [lvcreate_cmd] + test_opt + yesopt if snapshot is not None: if size: - cmd = "%s %s %s -%s %s%s -s -n %s %s %s/%s" % (lvcreate_cmd, test_opt, yesopt, size_opt, size, size_unit, snapshot, opts, vg, lv) - else: - cmd = "%s %s %s -s -n %s %s %s/%s" % (lvcreate_cmd, test_opt, yesopt, snapshot, opts, vg, lv) - elif thinpool and lv: - if size_opt == 'l': - module.fail_json(changed=False, msg="Thin volume sizing with percentage not supported.") - size_opt = 'V' - cmd = "%s %s %s -n %s -%s %s%s %s -T %s/%s" % (lvcreate_cmd, test_opt, yesopt, lv, size_opt, size, size_unit, opts, vg, thinpool) - elif thinpool and not lv: - cmd = "%s %s %s -%s %s%s %s -T %s/%s" % (lvcreate_cmd, test_opt, yesopt, size_opt, size, size_unit, opts, vg, thinpool) + cmd += ["-%s" % size_opt, "%s%s" % (size, size_unit)] + cmd += ["-s", "-n", snapshot] + opts + ["%s/%s" % (vg, lv)] + elif thinpool: + if lv: + if size_opt == 'l': + module.fail_json(changed=False, msg="Thin volume sizing with percentage not supported.") + size_opt = 'V' + cmd += ["-n", lv] + cmd += ["-%s" % size_opt, "%s%s" % (size, size_unit)] + cmd += opts + ["-T", "%s/%s" % (vg, thinpool)] else: - cmd = "%s %s %s -n %s -%s %s%s %s %s %s" % (lvcreate_cmd, test_opt, yesopt, lv, size_opt, size, size_unit, opts, vg, pvs) + cmd += ["-n", lv] + cmd += ["-%s" % size_opt, "%s%s" % (size, size_unit)] + cmd += opts + [vg] + pvs rc, dummy, err = module.run_command(cmd) if rc == 0: changed = True @@ -499,7 +495,7 @@ def main(): if not force: module.fail_json(msg="Sorry, no removal of logical volume %s without force=true." % (this_lv['name'])) lvremove_cmd = module.get_bin_path("lvremove", required=True) - rc, dummy, err = module.run_command("%s %s --force %s/%s" % (lvremove_cmd, test_opt, vg, this_lv['name'])) + rc, dummy, err = module.run_command([lvremove_cmd] + test_opt + ["--force", "%s/%s" % (vg, this_lv['name'])]) if rc == 0: module.exit_json(changed=True) else: @@ -527,7 +523,7 @@ def main(): if this_lv['size'] < size_requested: if (size_free > 0) and (size_free >= (size_requested - this_lv['size'])): - tool = module.get_bin_path("lvextend", required=True) + tool = [module.get_bin_path("lvextend", required=True)] else: module.fail_json( msg="Logical Volume %s could not be extended. Not enough free space left (%s%s required / %s%s available)" % @@ -539,16 +535,17 @@ def main(): elif not force: module.fail_json(msg="Sorry, no shrinking of %s without force=true" % (this_lv['name'])) else: - tool = module.get_bin_path("lvreduce", required=True) - tool = '%s %s' % (tool, '--force') + tool = [module.get_bin_path("lvreduce", required=True), '--force'] if tool: if resizefs: - tool = '%s %s' % (tool, '--resizefs') + tool += ['--resizefs'] + cmd = tool + test_opt if size_operator: - cmd = "%s %s -%s %s%s%s %s/%s %s" % (tool, test_opt, size_opt, size_operator, size, size_unit, vg, this_lv['name'], pvs) + cmd += ["-%s" % size_opt, "%s%s%s" % (size_operator, size, size_unit)] else: - cmd = "%s %s -%s %s%s %s/%s %s" % (tool, test_opt, size_opt, size, size_unit, vg, this_lv['name'], pvs) + cmd += ["-%s" % size_opt, "%s%s" % (size, size_unit)] + cmd += ["%s/%s" % (vg, this_lv['name'])] + pvs rc, out, err = module.run_command(cmd) if "Reached maximum COW size" in out: module.fail_json(msg="Unable to resize %s to %s%s" % (lv, size, size_unit), rc=rc, err=err, out=out) @@ -566,23 +563,24 @@ def main(): # resize LV based on absolute values tool = None if float(size) > this_lv['size'] or size_operator == '+': - tool = module.get_bin_path("lvextend", required=True) + tool = [module.get_bin_path("lvextend", required=True)] elif shrink and float(size) < this_lv['size'] or size_operator == '-': if float(size) == 0: module.fail_json(msg="Sorry, no shrinking of %s to 0 permitted." % (this_lv['name'])) if not force: module.fail_json(msg="Sorry, no shrinking of %s without force=true." % (this_lv['name'])) else: - tool = module.get_bin_path("lvreduce", required=True) - tool = '%s %s' % (tool, '--force') + tool = [module.get_bin_path("lvreduce", required=True), '--force'] if tool: if resizefs: - tool = '%s %s' % (tool, '--resizefs') + tool += ['--resizefs'] + cmd = tool + test_opt if size_operator: - cmd = "%s %s -%s %s%s%s %s/%s %s" % (tool, test_opt, size_opt, size_operator, size, size_unit, vg, this_lv['name'], pvs) + cmd += ["-%s" % size_opt, "%s%s%s" % (size_operator, size, size_unit)] else: - cmd = "%s %s -%s %s%s %s/%s %s" % (tool, test_opt, size_opt, size, size_unit, vg, this_lv['name'], pvs) + cmd += ["-%s" % size_opt, "%s%s" % (size, size_unit)] + cmd += ["%s/%s" % (vg, this_lv['name'])] + pvs rc, out, err = module.run_command(cmd) if "Reached maximum COW size" in out: module.fail_json(msg="Unable to resize %s to %s%s" % (lv, size, size_unit), rc=rc, err=err, out=out) @@ -598,14 +596,14 @@ def main(): if this_lv is not None: if active: lvchange_cmd = module.get_bin_path("lvchange", required=True) - rc, dummy, err = module.run_command("%s -ay %s/%s" % (lvchange_cmd, vg, this_lv['name'])) + rc, dummy, err = module.run_command([lvchange_cmd, "-ay", "%s/%s" % (vg, this_lv['name'])]) if rc == 0: module.exit_json(changed=((not this_lv['active']) or changed), vg=vg, lv=this_lv['name'], size=this_lv['size']) else: module.fail_json(msg="Failed to activate logical volume %s" % (lv), rc=rc, err=err) else: lvchange_cmd = module.get_bin_path("lvchange", required=True) - rc, dummy, err = module.run_command("%s -an %s/%s" % (lvchange_cmd, vg, this_lv['name'])) + rc, dummy, err = module.run_command([lvchange_cmd, "-an", "%s/%s" % (vg, this_lv['name'])]) if rc == 0: module.exit_json(changed=(this_lv['active'] or changed), vg=vg, lv=this_lv['name'], size=this_lv['size']) else: diff --git a/plugins/modules/macports.py b/plugins/modules/macports.py index e81fb9142c..cd620687d7 100644 --- a/plugins/modules/macports.py +++ b/plugins/modules/macports.py @@ -111,7 +111,7 @@ from ansible.module_utils.basic import AnsibleModule def selfupdate(module, port_path): """ Update Macports and the ports tree. """ - rc, out, err = module.run_command("%s -v selfupdate" % port_path) + rc, out, err = module.run_command([port_path, "-v", "selfupdate"]) if rc == 0: updated = any( @@ -135,7 +135,7 @@ def selfupdate(module, port_path): def upgrade(module, port_path): """ Upgrade outdated ports. """ - rc, out, err = module.run_command("%s upgrade outdated" % port_path) + rc, out, err = module.run_command([port_path, "upgrade", "outdated"]) # rc is 1 when nothing to upgrade so check stdout first. if out.strip() == "Nothing to upgrade.": @@ -182,7 +182,7 @@ def remove_ports(module, port_path, ports, stdout, stderr): if not query_port(module, port_path, port): continue - rc, out, err = module.run_command("%s uninstall %s" % (port_path, port)) + rc, out, err = module.run_command([port_path, "uninstall", port]) stdout += out stderr += err if query_port(module, port_path, port): @@ -206,7 +206,7 @@ def install_ports(module, port_path, ports, variant, stdout, stderr): if query_port(module, port_path, port): continue - rc, out, err = module.run_command("%s install %s %s" % (port_path, port, variant)) + rc, out, err = module.run_command([port_path, "install", port, variant]) stdout += out stderr += err if not query_port(module, port_path, port): @@ -232,7 +232,7 @@ def activate_ports(module, port_path, ports, stdout, stderr): if query_port(module, port_path, port, state="active"): continue - rc, out, err = module.run_command("%s activate %s" % (port_path, port)) + rc, out, err = module.run_command([port_path, "activate", port]) stdout += out stderr += err @@ -259,7 +259,7 @@ def deactivate_ports(module, port_path, ports, stdout, stderr): if not query_port(module, port_path, port, state="active"): continue - rc, out, err = module.run_command("%s deactivate %s" % (port_path, port)) + rc, out, err = module.run_command([port_path, "deactivate", port]) stdout += out stderr += err if query_port(module, port_path, port, state="active"): diff --git a/plugins/modules/parted.py b/plugins/modules/parted.py index 382e47a475..b3616a8ecd 100644 --- a/plugins/modules/parted.py +++ b/plugins/modules/parted.py @@ -480,12 +480,12 @@ def get_device_info(device, unit): if label_needed: return get_unlabeled_device_info(device, unit) - command = "%s -s -m %s -- unit '%s' print" % (parted_exec, device, unit) + command = [parted_exec, "-s", "-m", device, "--", "unit", unit, "print"] rc, out, err = module.run_command(command) if rc != 0 and 'unrecognised disk label' not in err: module.fail_json(msg=( "Error while getting device information with parted " - "script: '%s'" % command), + "script: '%s'" % " ".join(command)), rc=rc, out=out, err=err ) @@ -506,7 +506,7 @@ def check_parted_label(device): return False # Older parted versions return a message in the stdout and RC > 0. - rc, out, err = module.run_command("%s -s -m %s print" % (parted_exec, device)) + rc, out, err = module.run_command([parted_exec, "-s", "-m", device, "print"]) if rc != 0 and 'unrecognised disk label' in out.lower(): return True @@ -546,7 +546,7 @@ def parted_version(): """ global module, parted_exec # pylint: disable=global-variable-not-assigned - rc, out, err = module.run_command("%s --version" % parted_exec) + rc, out, err = module.run_command([parted_exec, "--version"]) if rc != 0: module.fail_json( msg="Failed to get parted version.", rc=rc, out=out, err=err @@ -580,6 +580,7 @@ def parted(script, device, align): script_option = '-s' if script and not module.check_mode: + # TODO: convert run_comand() argument to list! command = "%s %s -m %s %s -- %s" % (parted_exec, script_option, align_option, device, script) rc, out, err = module.run_command(command) diff --git a/plugins/modules/pkgin.py b/plugins/modules/pkgin.py index 5b2e478b8c..8b29655d37 100644 --- a/plugins/modules/pkgin.py +++ b/plugins/modules/pkgin.py @@ -145,18 +145,18 @@ def query_package(module, name): """ # test whether '-p' (parsable) flag is supported. - rc, out, err = module.run_command("%s -p -v" % PKGIN_PATH) + rc, out, err = module.run_command([PKGIN_PATH, "-p", "-v"]) if rc == 0: - pflag = '-p' + pflag = ['-p'] splitchar = ';' else: - pflag = '' + pflag = [] splitchar = ' ' # Use "pkgin search" to find the package. The regular expression will # only match on the complete name. - rc, out, err = module.run_command("%s %s search \"^%s$\"" % (PKGIN_PATH, pflag, name)) + rc, out, err = module.run_command([PKGIN_PATH] + pflag + ["search", "^%s$" % name]) # rc will not be 0 unless the search was a success if rc == 0: @@ -234,22 +234,19 @@ def format_pkgin_command(module, command, package=None): # an empty string. Some commands (e.g. 'update') will ignore extra # arguments, however this behaviour cannot be relied on for others. if package is None: - package = "" + packages = [] + else: + packages = [package] if module.params["force"]: - force = "-F" + force = ["-F"] else: - force = "" - - vars = {"pkgin": PKGIN_PATH, - "command": command, - "package": package, - "force": force} + force = [] if module.check_mode: - return "%(pkgin)s -n %(command)s %(package)s" % vars + return [PKGIN_PATH, "-n", command] + packages else: - return "%(pkgin)s -y %(force)s %(command)s %(package)s" % vars + return [PKGIN_PATH, "-y"] + force + [command] + packages def remove_packages(module, packages): diff --git a/plugins/modules/portinstall.py b/plugins/modules/portinstall.py index e263b71813..59dafb1eb8 100644 --- a/plugins/modules/portinstall.py +++ b/plugins/modules/portinstall.py @@ -79,12 +79,13 @@ def query_package(module, name): if pkg_info_path: pkgng = False pkg_glob_path = module.get_bin_path('pkg_glob', True) + # TODO: convert run_comand() argument to list! rc, out, err = module.run_command("%s -e `pkg_glob %s`" % (pkg_info_path, shlex_quote(name)), use_unsafe_shell=True) + pkg_info_path = [pkg_info_path] else: pkgng = True - pkg_info_path = module.get_bin_path('pkg', True) - pkg_info_path = pkg_info_path + " info" - rc, out, err = module.run_command("%s %s" % (pkg_info_path, name)) + pkg_info_path = [module.get_bin_path('pkg', True), "info"] + rc, out, err = module.run_command(pkg_info_path + [name]) found = rc == 0 @@ -94,10 +95,7 @@ def query_package(module, name): # some package is installed name_without_digits = re.sub('[0-9]', '', name) if name != name_without_digits: - if pkgng: - rc, out, err = module.run_command("%s %s" % (pkg_info_path, name_without_digits)) - else: - rc, out, err = module.run_command("%s %s" % (pkg_info_path, name_without_digits)) + rc, out, err = module.run_command(pkg_info_path + [name_without_digits]) found = rc == 0 @@ -107,13 +105,13 @@ def query_package(module, name): def matching_packages(module, name): ports_glob_path = module.get_bin_path('ports_glob', True) - rc, out, err = module.run_command("%s %s" % (ports_glob_path, name)) + rc, out, err = module.run_command([ports_glob_path, name]) # counts the number of packages found occurrences = out.count('\n') if occurrences == 0: name_without_digits = re.sub('[0-9]', '', name) if name != name_without_digits: - rc, out, err = module.run_command("%s %s" % (ports_glob_path, name_without_digits)) + rc, out, err = module.run_command([ports_glob_path, name_without_digits]) occurrences = out.count('\n') return occurrences @@ -135,10 +133,12 @@ def remove_packages(module, packages): if not query_package(module, package): continue + # TODO: convert run_comand() argument to list! rc, out, err = module.run_command("%s `%s %s`" % (pkg_delete_path, pkg_glob_path, shlex_quote(package)), use_unsafe_shell=True) if query_package(module, package): name_without_digits = re.sub('[0-9]', '', package) + # TODO: convert run_comand() argument to list! rc, out, err = module.run_command("%s `%s %s`" % (pkg_delete_path, pkg_glob_path, shlex_quote(name_without_digits)), use_unsafe_shell=True) @@ -163,13 +163,13 @@ def install_packages(module, packages, use_packages): if not portinstall_path: pkg_path = module.get_bin_path('pkg', False) if pkg_path: - module.run_command("pkg install -y portupgrade") + module.run_command([pkg_path, "install", "-y", "portupgrade"]) portinstall_path = module.get_bin_path('portinstall', True) if use_packages: - portinstall_params = "--use-packages" + portinstall_params = ["--use-packages"] else: - portinstall_params = "" + portinstall_params = [] for package in packages: if query_package(module, package): @@ -178,7 +178,7 @@ def install_packages(module, packages, use_packages): # TODO: check how many match matches = matching_packages(module, package) if matches == 1: - rc, out, err = module.run_command("%s --batch %s %s" % (portinstall_path, portinstall_params, package)) + rc, out, err = module.run_command([portinstall_path, "--batch"] + portinstall_params + [package]) if not query_package(module, package): module.fail_json(msg="failed to install %s: %s" % (package, out)) elif matches == 0: diff --git a/plugins/modules/slackpkg.py b/plugins/modules/slackpkg.py index e3d7a15429..9347db1591 100644 --- a/plugins/modules/slackpkg.py +++ b/plugins/modules/slackpkg.py @@ -106,9 +106,8 @@ def remove_packages(module, slackpkg_path, packages): continue if not module.check_mode: - rc, out, err = module.run_command("%s -default_answer=y -batch=on \ - remove %s" % (slackpkg_path, - package)) + rc, out, err = module.run_command( + [slackpkg_path, "-default_answer=y", "-batch=on", "remove", package]) if not module.check_mode and query_package(module, slackpkg_path, package): @@ -132,9 +131,8 @@ def install_packages(module, slackpkg_path, packages): continue if not module.check_mode: - rc, out, err = module.run_command("%s -default_answer=y -batch=on \ - install %s" % (slackpkg_path, - package)) + rc, out, err = module.run_command( + [slackpkg_path, "-default_answer=y", "-batch=on", "install", package]) if not module.check_mode and not query_package(module, slackpkg_path, package): @@ -155,9 +153,8 @@ def upgrade_packages(module, slackpkg_path, packages): for package in packages: if not module.check_mode: - rc, out, err = module.run_command("%s -default_answer=y -batch=on \ - upgrade %s" % (slackpkg_path, - package)) + rc, out, err = module.run_command( + [slackpkg_path, "-default_answer=y", "-batch=on", "upgrade", package]) if not module.check_mode and not query_package(module, slackpkg_path, package): @@ -174,7 +171,8 @@ def upgrade_packages(module, slackpkg_path, packages): def update_cache(module, slackpkg_path): - rc, out, err = module.run_command("%s -batch=on update" % (slackpkg_path)) + rc, out, err = module.run_command( + [slackpkg_path, "-batch=on", "update"]) if rc != 0: module.fail_json(msg="Could not update package cache") diff --git a/plugins/modules/svr4pkg.py b/plugins/modules/svr4pkg.py index db9902c770..56ded66e62 100644 --- a/plugins/modules/svr4pkg.py +++ b/plugins/modules/svr4pkg.py @@ -120,7 +120,7 @@ def package_installed(module, name, category): if category: cmd.append('-c') cmd.append(name) - rc, out, err = module.run_command(' '.join(cmd)) + rc, out, err = module.run_command(cmd) if rc == 0: return True else: diff --git a/plugins/modules/swdepot.py b/plugins/modules/swdepot.py index 28a8ce3145..9ba1b02b30 100644 --- a/plugins/modules/swdepot.py +++ b/plugins/modules/swdepot.py @@ -68,7 +68,6 @@ EXAMPLES = ''' import re from ansible.module_utils.basic import AnsibleModule -from ansible.module_utils.six.moves import shlex_quote def compare_package(version1, version2): @@ -94,13 +93,13 @@ def compare_package(version1, version2): def query_package(module, name, depot=None): """ Returns whether a package is installed or not and version. """ - cmd_list = '/usr/sbin/swlist -a revision -l product' + cmd_list = ['/usr/sbin/swlist', '-a', 'revision', '-l', 'product'] if depot: - rc, stdout, stderr = module.run_command("%s -s %s %s | grep %s" % (cmd_list, shlex_quote(depot), shlex_quote(name), shlex_quote(name)), - use_unsafe_shell=True) - else: - rc, stdout, stderr = module.run_command("%s %s | grep %s" % (cmd_list, shlex_quote(name), shlex_quote(name)), use_unsafe_shell=True) + cmd_list.extend(['-s', depot]) + cmd_list.append(name) + rc, stdout, stderr = module.run_command(cmd_list) if rc == 0: + stdout = ''.join(line for line in stdout.splitlines(True) if name in line) version = re.sub(r"\s\s+|\t", " ", stdout).strip().split()[1] else: version = None @@ -112,7 +111,7 @@ def remove_package(module, name): """ Uninstall package if installed. """ cmd_remove = '/usr/sbin/swremove' - rc, stdout, stderr = module.run_command("%s %s" % (cmd_remove, name)) + rc, stdout, stderr = module.run_command([cmd_remove, name]) if rc == 0: return rc, stdout @@ -123,8 +122,8 @@ def remove_package(module, name): def install_package(module, depot, name): """ Install package if not already installed """ - cmd_install = '/usr/sbin/swinstall -x mount_all_filesystems=false' - rc, stdout, stderr = module.run_command("%s -s %s %s" % (cmd_install, depot, name)) + cmd_install = ['/usr/sbin/swinstall', '-x', 'mount_all_filesystems=false'] + rc, stdout, stderr = module.run_command(cmd_install + ["-s", depot, name]) if rc == 0: return rc, stdout else: