diff --git a/lib/galaxy/tools/deps/commands.py b/lib/galaxy/tools/deps/commands.py index 2adcbde83b2..0a2046c4d9e 100644 --- a/lib/galaxy/tools/deps/commands.py +++ b/lib/galaxy/tools/deps/commands.py @@ -7,6 +7,11 @@ from galaxy.util import which STDOUT_INDICATOR = "-" +try: + from shlex import quote as shell_quote +except ImportError: + from pipes import quote as shell_quote + def redirecting_io(sys=_sys): """Predicate to determine if we are redicting I/O in process.""" @@ -73,11 +78,24 @@ def execute(cmds): return _wait(cmds, shell=False, stdin=subprocess.PIPE, stdout=subprocess.PIPE) +def argv_to_str(command_argv, quote=True): + """Convert an argv command list to a string for shell subprocess. + + If None appears in the command list it is simply excluded. + + Arguments are quoted with shlex.quote. That said, this method is not meant to be + used in security critical paths of code and should not be used to sanitize + code. + """ + map_func = shell_quote if quote else lambda x: x + return " ".join([map_func(c) for c in command_argv if c is not None]) + + def _wait(cmds, **popen_kwds): p = subprocess.Popen(cmds, **popen_kwds) stdout, stderr = p.communicate() if p.returncode != 0: - raise CommandLineException(" ".join(cmds), stdout, stderr) + raise CommandLineException(argv_to_str(cmds), stdout, stderr) return stdout @@ -127,6 +145,7 @@ class CommandLineException(Exception): __all__ = [ + 'argv_to_str', 'CommandLineException', 'download_command', 'execute', @@ -134,5 +153,6 @@ __all__ = [ 'redirecting_io', 'shell', 'shell_process', + 'shell_quote', 'which', ] diff --git a/lib/galaxy/tools/deps/docker_util.py b/lib/galaxy/tools/deps/docker_util.py index 52b1ffe16a7..3c156773507 100644 --- a/lib/galaxy/tools/deps/docker_util.py +++ b/lib/galaxy/tools/deps/docker_util.py @@ -1,4 +1,9 @@ +"""Utilities for building up Docker commands... + +...using common defaults and configuration mechanisms. +""" import os +from .commands import argv_to_str, shell_quote DEFAULT_DOCKER_COMMAND = "docker" DEFAULT_SUDO = True @@ -54,6 +59,22 @@ class DockerVolume(object): return ":".join([self.from_path, self.to_path, self.how]) +def kill_command( + container, + signal=None, + **kwds +): + args = (["-s", signal] if signal else []) + [container] + return command_list("kill", args, **kwds) + + +def logs_command( + container, + **kwds +): + return command_list("logs", **kwds) + + def build_command( image, docker_build_path, @@ -61,9 +82,7 @@ def build_command( ): if os.path.isfile(docker_build_path): docker_build_path = os.path.dirname(os.path.abspath(docker_build_path)) - build_command_parts = __docker_prefix(**kwds) - build_command_parts.extend(["build", "-t", image, docker_build_path]) - return build_command_parts + return command_list("build", ["-t", image, docker_build_path], **kwds) def build_save_image_command( @@ -71,47 +90,33 @@ def build_save_image_command( destination, **kwds ): - build_command_parts = __docker_prefix(**kwds) - build_command_parts.extend(["save", "-o", destination, image]) - return build_command_parts + return command_list("save", ["-o", destination, image], **kwds) def build_pull_command( tag, **kwds ): - build_command_parts = __docker_prefix(**kwds) - build_command_parts.extend(["pull", tag]) - return build_command_parts + return command_list("pull", [tag], **kwds) def build_docker_cache_command( image, **kwds ): - inspect_command_parts = __docker_prefix(**kwds) - inspect_command_parts.extend(["inspect", image]) - inspect_image_command = " ".join(inspect_command_parts) - - pull_command_parts = __docker_prefix(**kwds) - pull_command_parts.extend(["pull", image]) - pull_image_command = " ".join(pull_command_parts) + inspect_image_command = command_shell("inspect", [image], **kwds) + pull_image_command = command_shell("pull", [image], **kwds) cache_command = "%s > /dev/null 2>&1\n[ $? -ne 0 ] && %s > /dev/null 2>&1\n" % (inspect_image_command, pull_image_command) return cache_command def build_docker_images_command(truncate=True, **kwds): - images_command_parts = __docker_prefix(**kwds) - images_command_parts.append("images") - if not truncate: - images_command_parts.append("--no-trunc") - return " ".join(images_command_parts) + args = ["--no-trunc"] if not truncate else[] + return command_shell("images", args, **kwds) def build_docker_load_command(**kwds): - load_command_parts = __docker_prefix(**kwds) - load_command_parts.append("load") - return " ".join(load_command_parts) + return command_shell("load", []) def build_docker_run_command( @@ -135,7 +140,7 @@ def build_docker_run_command( set_user=DEFAULT_SET_USER, host=DEFAULT_HOST, ): - command_parts = __docker_prefix( + command_parts = _docker_prefix( docker_cmd=docker_cmd, sudo=sudo, sudo_cmd=sudo_cmd, @@ -147,19 +152,19 @@ def build_docker_run_command( if terminal: command_parts.append("-t") for env_directive in env_directives: - command_parts.extend(["-e", env_directive]) + command_parts.extend(["-e", shell_quote(env_directive)]) for volume in volumes: - command_parts.extend(["-v", str(volume)]) + command_parts.extend(["-v", shell_quote(str(volume))]) if volumes_from: - command_parts.extend(["--volumes-from", str(volumes_from)]) + command_parts.extend(["--volumes-from", shell_quote(str(volumes_from))]) if memory: - command_parts.extend(["-m", memory]) + command_parts.extend(["-m", shell_quote(memory)]) if name: - command_parts.extend(["-name", name]) + command_parts.extend(["--name", shell_quote(name)]) if working_directory: - command_parts.extend(["-w", working_directory]) + command_parts.extend(["-w", shell_quote(working_directory)]) if net: - command_parts.extend(["--net", net]) + command_parts.extend(["--net", shell_quote(net)]) if auto_rm: command_parts.append("--rm") if run_extra_arguments: @@ -172,20 +177,31 @@ def build_docker_run_command( full_image = image if tag: full_image = "%s:%s" % (full_image, tag) - command_parts.append(full_image) + command_parts.append(shell_quote(full_image)) command_parts.append(container_command) return " ".join(command_parts) -def __docker_prefix( +def command_list(command, command_args=[], **kwds): + """Return Docker command as an argv list.""" + command_parts = _docker_prefix(**kwds) + command_parts.extend(command_parts) + return command_parts + + +def command_shell(command, command_args=[], **kwds): + """Return Docker command as a string for a shell.""" + return argv_to_str(command_list(command, command_args, **kwds)) + + +def _docker_prefix( docker_cmd=DEFAULT_DOCKER_COMMAND, sudo=DEFAULT_SUDO, sudo_cmd=DEFAULT_SUDO_COMMAND, host=DEFAULT_HOST, **kwds ): - """ Prefix to issue a docker command. - """ + """Prefix to issue a docker command.""" command_parts = [] if sudo: command_parts.append(sudo_cmd) diff --git a/lib/galaxy/tools/verify/__init__.py b/lib/galaxy/tools/verify/__init__.py index a1c675ee67c..fda23a274d5 100644 --- a/lib/galaxy/tools/verify/__init__.py +++ b/lib/galaxy/tools/verify/__init__.py @@ -45,7 +45,7 @@ def verify( md5 = attributes.get("md5") try: _verify_md5(output_content, md5) - except AssertionError, err: + except AssertionError as err: errmsg = 'History item %s different than expected\n' % (hid) errmsg += str( err ) raise AssertionError( errmsg ) @@ -62,7 +62,7 @@ def verify( log.debug('keep_outputs_dir: %s, ofn: %s', keep_outputs_dir, ofn) try: shutil.copy( temp_name, ofn ) - except Exception, exc: + except Exception as exc: error_log_msg = 'Could not save output file %s to %s: ' % (temp_name, ofn) error_log_msg += str(exc) log.error(error_log_msg, exc_info=True) @@ -95,7 +95,7 @@ def verify( extra_files = attributes.get('extra_files', None) if extra_files: verify_extra_files(extra_files) - except AssertionError, err: + except AssertionError as err: errmsg = 'History item %s different than expected, difference (using %s):\n' % ( hid, compare ) errmsg += "( %s v. %s )\n" % ( local_name, temp_name ) errmsg += str( err ) @@ -212,7 +212,7 @@ def files_diff(file1, file2, attributes=None): log.info('## files diff on %s and %s lines_diff=%d, found diff = %d, found pdf invalid diff = %d' % (file1, file2, allowed_diff_count, diff_lines, invalid_diff_lines)) if invalid_diff_lines > allowed_diff_count: # Print out diff_slice so we can see what failed - print "###### diff_slice ######" + log.info("###### diff_slice ######") raise AssertionError( "".join( diff_slice ) ) else: log.info('## files diff on %s and %s lines_diff=%d, found diff = %d' % (file1, file2, allowed_diff_count, diff_lines))