From 7081598073cbf6233fbca30dd3deba7cf962e309 Mon Sep 17 00:00:00 2001 From: Fabio Erculiani Date: Sun, 4 Sep 2011 11:52:02 +0200 Subject: [PATCH] [molecule] rewrite molecule.utils.exec_cmd() to not use shell=True The following changes were required code-wide: - use shlex.split() to split commands read from spec files - use glob.glob() where required - write command splitter function and make use of it instead of simple string splitter --- molecule/specs/plugins/builtin_plugin.py | 29 +++++++++++--------- molecule/specs/plugins/image_plugin.py | 32 +++++++++++------------ molecule/specs/plugins/remaster_plugin.py | 32 +++++++++++------------ molecule/specs/plugins/tar_plugin.py | 26 +++++++++--------- 4 files changed, 61 insertions(+), 58 deletions(-) diff --git a/molecule/specs/plugins/builtin_plugin.py b/molecule/specs/plugins/builtin_plugin.py index ca6e45e..f773da4 100644 --- a/molecule/specs/plugins/builtin_plugin.py +++ b/molecule/specs/plugins/builtin_plugin.py @@ -19,6 +19,8 @@ import os import shutil import tempfile +import glob + from molecule.compat import get_stringtype from molecule.i18n import _ from molecule.output import red, brown, blue, green, purple, darkgreen, \ @@ -167,7 +169,8 @@ class MirrorHandler(GenericExecutionStep, BuiltinHandlerMixin): args = [self._config['mirror_syncer']] args.extend(self._config['mirror_syncer_builtin_args']) args.extend(self.metadata.get('extra_rsync_parameters', [])) - args.extend([self.source_dir+"/*", self.dest_dir]) + args.extend(glob.glob(self.source_dir + "/*")) + args.append(self.dest_dir) self._output.output("[%s|%s] %s: %s" % ( blue("MirrorHandler"), darkred(self.spec_name), _("spawning"), " ".join(args), @@ -608,7 +611,7 @@ class IsoHandler(GenericExecutionStep, BuiltinHandlerMixin): args.extend(self._config['iso_builder_builtin_args']) args.extend(self.metadata.get('extra_mkisofs_parameters', [])) if self.iso_title.strip(): - args.extend(["-V", '"', self.iso_title[:30], '"']) + args.extend(["-V", self.iso_title[:32]]) args.extend(['-o', self.dest_iso, self.source_path]) self._output.output("[%s|%s] %s: %s" % ( blue("IsoHandler"), darkred(self.spec_name), @@ -667,7 +670,7 @@ class LivecdSpec(GenericSpec): }, 'prechroot': { 'cb': self.valid_exec_first_list_item, - 've': self.ve_string_splitter, + 've': self.ve_command_splitter, }, 'release_string': { 'cb': self.ne_string, # validation callback @@ -695,7 +698,7 @@ class LivecdSpec(GenericSpec): }, 'extra_rsync_parameters': { 'cb': self.always_valid, - 've': self.ve_string_splitter, + 've': self.ve_command_splitter, }, 'merge_destination_chroot': { 'cb': self.valid_dir, @@ -703,23 +706,23 @@ class LivecdSpec(GenericSpec): }, 'error_script': { 'cb': self.valid_exec_first_list_item, - 've': self.ve_string_splitter, + 've': self.ve_command_splitter, }, 'outer_chroot_script': { 'cb': self.valid_exec_first_list_item, - 've': self.ve_string_splitter, + 've': self.ve_command_splitter, }, 'inner_source_chroot_script': { 'cb': self.valid_path_string_first_list_item, - 've': self.ve_string_splitter, + 've': self.ve_command_splitter, }, 'inner_chroot_script': { 'cb': self.valid_path_string_first_list_item, - 've': self.ve_string_splitter, + 've': self.ve_command_splitter, }, 'outer_chroot_script_after': { 'cb': self.valid_exec_first_list_item, - 've': self.ve_string_splitter, + 've': self.ve_command_splitter, }, 'destination_livecd_root': { 'cb': self.valid_path_string, @@ -731,19 +734,19 @@ class LivecdSpec(GenericSpec): }, 'extra_mksquashfs_parameters': { 'cb': self.always_valid, - 've': self.ve_string_splitter, + 've': self.ve_command_splitter, }, 'extra_mkisofs_parameters': { 'cb': self.always_valid, - 've': self.ve_string_splitter, + 've': self.ve_command_splitter, }, 'pre_iso_script': { 'cb': self.valid_exec_first_list_item, - 've': self.ve_string_splitter, + 've': self.ve_command_splitter, }, 'post_iso_script': { 'cb': self.valid_exec_first_list_item, - 've': self.ve_string_splitter, + 've': self.ve_command_splitter, }, 'destination_iso_directory': { 'cb': self.valid_dir, diff --git a/molecule/specs/plugins/image_plugin.py b/molecule/specs/plugins/image_plugin.py index 4c460e1..967472f 100644 --- a/molecule/specs/plugins/image_plugin.py +++ b/molecule/specs/plugins/image_plugin.py @@ -550,7 +550,7 @@ class IsoToImageSpec(GenericSpec): }, 'prechroot': { 'cb': self.valid_exec_first_list_item, - 've': self.ve_string_splitter, + 've': self.ve_command_splitter, }, 'release_string': { 'cb': self.ne_string, # validation callback @@ -578,15 +578,15 @@ class IsoToImageSpec(GenericSpec): }, 'image_formatter': { 'cb': self.ne_list, - 've': self.ve_string_splitter, + 've': self.ve_command_splitter, }, 'image_mounter': { 'cb': self.ne_list, - 've': self.ve_string_splitter, + 've': self.ve_command_splitter, }, 'image_umounter': { 'cb': self.ne_list, - 've': self.ve_string_splitter, + 've': self.ve_command_splitter, }, 'image_mb': { 'cb': self.valid_integer, @@ -598,23 +598,23 @@ class IsoToImageSpec(GenericSpec): }, 'error_script': { 'cb': self.valid_exec_first_list_item, - 've': self.ve_string_splitter, + 've': self.ve_command_splitter, }, 'outer_chroot_script': { 'cb': self.valid_exec_first_list_item, - 've': self.ve_string_splitter, + 've': self.ve_command_splitter, }, 'inner_chroot_script': { 'cb': self.valid_path_string_first_list_item, - 've': self.ve_string_splitter, + 've': self.ve_command_splitter, }, 'inner_chroot_script_after': { 'cb': self.valid_path_string_first_list_item, - 've': self.ve_string_splitter, + 've': self.ve_command_splitter, }, 'outer_chroot_script_after': { 'cb': self.valid_exec_first_list_item, - 've': self.ve_string_splitter, + 've': self.ve_command_splitter, }, 'destination_image_directory': { 'cb': self.valid_dir, @@ -622,27 +622,27 @@ class IsoToImageSpec(GenericSpec): }, 'pre_image_script': { 'cb': self.valid_exec_first_list_item, - 've': self.ve_string_splitter, + 've': self.ve_command_splitter, }, 'post_image_script': { 'cb': self.valid_exec_first_list_item, - 've': self.ve_string_splitter, + 've': self.ve_command_splitter, }, 'iso_mounter': { 'cb': self.ne_list, - 've': self.ve_string_splitter, + 've': self.ve_command_splitter, }, 'iso_umounter': { 'cb': self.ne_list, - 've': self.ve_string_splitter, + 've': self.ve_command_splitter, }, 'squash_mounter': { 'cb': self.ne_list, - 've': self.ve_string_splitter, + 've': self.ve_command_splitter, }, 'squash_umounter': { 'cb': self.ne_list, - 've': self.ve_string_splitter, + 've': self.ve_command_splitter, }, 'custom_packages_remove_cmd': { 'cb': self.valid_ascii, @@ -662,7 +662,7 @@ class IsoToImageSpec(GenericSpec): }, 'repositories_update_cmd': { 'cb': self.ne_list, - 've': self.ve_string_splitter, + 've': self.ve_command_splitter, }, 'execute_repositories_update': { 'cb': self.valid_ascii, diff --git a/molecule/specs/plugins/remaster_plugin.py b/molecule/specs/plugins/remaster_plugin.py index b8b1c7e..de46381 100644 --- a/molecule/specs/plugins/remaster_plugin.py +++ b/molecule/specs/plugins/remaster_plugin.py @@ -378,7 +378,7 @@ class RemasterSpec(GenericSpec): }, 'prechroot': { 'cb': self.valid_exec_first_list_item, - 've': self.ve_string_splitter, + 've': self.ve_command_splitter, }, 'release_string': { 'cb': self.ne_string, # validation callback @@ -406,35 +406,35 @@ class RemasterSpec(GenericSpec): }, 'error_script': { 'cb': self.valid_exec_first_list_item, - 've': self.ve_string_splitter, + 've': self.ve_command_splitter, }, 'outer_chroot_script': { 'cb': self.valid_exec_first_list_item, - 've': self.ve_string_splitter, + 've': self.ve_command_splitter, }, 'inner_chroot_script': { 'cb': self.valid_path_string_first_list_item, - 've': self.ve_string_splitter, + 've': self.ve_command_splitter, }, 'inner_chroot_script_after': { 'cb': self.valid_path_string_first_list_item, - 've': self.ve_string_splitter, + 've': self.ve_command_splitter, }, 'outer_chroot_script_after': { 'cb': self.valid_exec_first_list_item, - 've': self.ve_string_splitter, + 've': self.ve_command_splitter, }, 'extra_mkisofs_parameters': { 'cb': self.always_valid, - 've': self.ve_string_splitter, + 've': self.ve_command_splitter, }, 'pre_iso_script': { 'cb': self.valid_exec_first_list_item, - 've': self.ve_string_splitter, + 've': self.ve_command_splitter, }, 'post_iso_script': { 'cb': self.valid_exec_first_list_item, - 've': self.ve_string_splitter, + 've': self.ve_command_splitter, }, 'destination_iso_directory': { 'cb': self.valid_dir, @@ -446,19 +446,19 @@ class RemasterSpec(GenericSpec): }, 'iso_mounter': { 'cb': self.ne_list, - 've': self.ve_string_splitter, + 've': self.ve_command_splitter, }, 'iso_umounter': { 'cb': self.ne_list, - 've': self.ve_string_splitter, + 've': self.ve_command_splitter, }, 'squash_mounter': { 'cb': self.ne_list, - 've': self.ve_string_splitter, + 've': self.ve_command_splitter, }, 'squash_umounter': { 'cb': self.ne_list, - 've': self.ve_string_splitter, + 've': self.ve_command_splitter, }, 'merge_livecd_root': { 'cb': self.valid_dir, @@ -466,11 +466,11 @@ class RemasterSpec(GenericSpec): }, 'custom_packages_remove_cmd': { 'cb': self.ne_list, - 've': self.ve_string_splitter, + 've': self.ve_command_splitter, }, 'custom_packages_add_cmd': { 'cb': self.ne_list, - 've': self.ve_string_splitter, + 've': self.ve_command_splitter, }, 'packages_to_remove': { 'cb': self.ne_list, @@ -482,7 +482,7 @@ class RemasterSpec(GenericSpec): }, 'repositories_update_cmd': { 'cb': self.ne_list, - 've': self.ve_string_splitter, + 've': self.ve_command_splitter, }, 'execute_repositories_update': { 'cb': self.valid_ascii, diff --git a/molecule/specs/plugins/tar_plugin.py b/molecule/specs/plugins/tar_plugin.py index 64e97be..85bb63d 100644 --- a/molecule/specs/plugins/tar_plugin.py +++ b/molecule/specs/plugins/tar_plugin.py @@ -191,7 +191,7 @@ class IsoToTarSpec(GenericSpec): }, 'prechroot': { 'cb': self.valid_exec_first_list_item, - 've': self.ve_string_splitter, + 've': self.ve_command_splitter, }, 'release_string': { 'cb': self.ne_string, # validation callback @@ -223,23 +223,23 @@ class IsoToTarSpec(GenericSpec): }, 'error_script': { 'cb': self.valid_exec_first_list_item, - 've': self.ve_string_splitter, + 've': self.ve_command_splitter, }, 'outer_chroot_script': { 'cb': self.valid_exec_first_list_item, - 've': self.ve_string_splitter, + 've': self.ve_command_splitter, }, 'inner_chroot_script': { 'cb': self.valid_path_string_first_list_item, - 've': self.ve_string_splitter, + 've': self.ve_command_splitter, }, 'inner_chroot_script_after': { 'cb': self.valid_path_string_first_list_item, - 've': self.ve_string_splitter, + 've': self.ve_command_splitter, }, 'outer_chroot_script_after': { 'cb': self.valid_exec_first_list_item, - 've': self.ve_string_splitter, + 've': self.ve_command_splitter, }, 'destination_tar_directory': { 'cb': self.valid_dir, @@ -247,27 +247,27 @@ class IsoToTarSpec(GenericSpec): }, 'pre_tar_script': { 'cb': self.valid_exec_first_list_item, - 've': self.ve_string_splitter, + 've': self.ve_command_splitter, }, 'post_tar_script': { 'cb': self.valid_exec_first_list_item, - 've': self.ve_string_splitter, + 've': self.ve_command_splitter, }, 'iso_mounter': { 'cb': self.ne_list, - 've': self.ve_string_splitter, + 've': self.ve_command_splitter, }, 'iso_umounter': { 'cb': self.ne_list, - 've': self.ve_string_splitter, + 've': self.ve_command_splitter, }, 'squash_mounter': { 'cb': self.ne_list, - 've': self.ve_string_splitter, + 've': self.ve_command_splitter, }, 'squash_umounter': { 'cb': self.ne_list, - 've': self.ve_string_splitter, + 've': self.ve_command_splitter, }, 'custom_packages_remove_cmd': { 'cb': self.valid_ascii, @@ -287,7 +287,7 @@ class IsoToTarSpec(GenericSpec): }, 'repositories_update_cmd': { 'cb': self.ne_list, - 've': self.ve_string_splitter, + 've': self.ve_command_splitter, }, 'execute_repositories_update': { 'cb': self.valid_ascii,