From 440fb2e6f86d037b64786540f7855afced995c29 Mon Sep 17 00:00:00 2001 From: Vincent Partington Date: Tue, 20 Mar 2012 12:12:20 +0100 Subject: [PATCH] DEPLOYITPB-3141 DEPLOYITPB-3142 Github #39 Github #40 Method setExecutable invokes chmod a+x instead of chmod +x, current working directory not set using "cd" when invoking commands for SshScpFile. - Improved logging of SshScpFile.getInputStream() and SshScpFile.getOutputStream(). - Improved logging of handling of "sudo" and "cd" and the corresponding pseudocommands "nosudo" and "nocd". --- .../overthere/ssh/SshConnection.java | 42 ++++++++++-- .../xebialabs/overthere/ssh/SshScpFile.java | 64 +++++++++++-------- .../overthere/ssh/SshSudoConnection.java | 59 +++++++++-------- .../xebialabs/overthere/ssh/SshSudoFile.java | 17 ++--- 4 files changed, 117 insertions(+), 65 deletions(-) diff --git a/overthere/src/main/java/com/xebialabs/overthere/ssh/SshConnection.java b/overthere/src/main/java/com/xebialabs/overthere/ssh/SshConnection.java index c24826ba..a22005a9 100644 --- a/overthere/src/main/java/com/xebialabs/overthere/ssh/SshConnection.java +++ b/overthere/src/main/java/com/xebialabs/overthere/ssh/SshConnection.java @@ -73,6 +73,8 @@ abstract class SshConnection extends OverthereConnection { public static final String PTY_PATTERN = "(\\w+):(\\d+):(\\d+):(\\d+):(\\d+)"; + public static final String NOCD_PSEUDO_COMMAND = "nocd"; + protected final SshConnectionType sshConnectionType; protected final String host; @@ -248,20 +250,35 @@ public OverthereProcess startProcess(final CmdLine commandLine) { } protected CmdLine processCommandLine(final CmdLine commandLine) { - if(getWorkingDirectory() != null) { - CmdLine commandLineWithCd = new CmdLine(); - commandLineWithCd.addArgument("cd"); - commandLineWithCd.addArgument(workingDirectory.getPath()); - commandLineWithCd.addRaw(os.getCommandSeparator()); + if (startsWithPseudoCommand(commandLine, NOCD_PSEUDO_COMMAND)) { + logger.trace("Not prefixing command line with cd statement because the " + NOCD_PSEUDO_COMMAND + " pseudo command was present, but the pseudo command will be stripped"); + logger.trace("Replacing: {}", commandLine); + CmdLine cmd = stripPrefixedPseudoCommand(commandLine); + logger.trace("With : {}", cmd); + return cmd; + } else if(getWorkingDirectory() != null) { + logger.trace("Prefixing command line with cd statement because the current working directory was set"); + logger.trace("Replacing: {}", commandLine); + CmdLine cmd = new CmdLine(); + cmd.addArgument("cd"); + cmd.addArgument(workingDirectory.getPath()); + cmd.addRaw(os.getCommandSeparator()); for (CmdLineArgument a : commandLine.getArguments()) { - commandLineWithCd.add(a); + cmd.add(a); } - return commandLineWithCd; + logger.trace("With : {}", cmd); + return cmd; } else { + logger.trace("Not prefixing command line with cd statement because the current working directory was not set"); + logger.trace("Keeping : {}", commandLine); return commandLine; } } + protected boolean startsWithPseudoCommand(final CmdLine commandLine, final String pseudoCommand) { + return commandLine.getArguments().size() >= 2 && commandLine.getArguments().get(0).toString(os, false).equals(pseudoCommand); + } + protected SshProcess createProcess(Session session, CmdLine commandLine) throws TransportException, ConnectionException { return new SshProcess(this, os, session, commandLine); } @@ -271,6 +288,17 @@ public String toString() { return "ssh:" + sshConnectionType.toString().toLowerCase() + "://" + username + "@" + host + ":" + port; } + protected static CmdLine stripPrefixedPseudoCommand(final CmdLine commandLine) { + return new CmdLine().add(commandLine.getArguments().subList(1, commandLine.getArguments().size())); + } + + protected static CmdLine prefixWithPseudoCommand(final CmdLine commandLine, final String pseudoCommand) { + CmdLine nosudoCommandLine = new CmdLine(); + nosudoCommandLine.addArgument(pseudoCommand); + nosudoCommandLine.add(commandLine.getArguments()); + return nosudoCommandLine; + } + private static Logger logger = LoggerFactory.getLogger(SshConnection.class); } diff --git a/overthere/src/main/java/com/xebialabs/overthere/ssh/SshScpFile.java b/overthere/src/main/java/com/xebialabs/overthere/ssh/SshScpFile.java index 22a705b6..aff52f74 100644 --- a/overthere/src/main/java/com/xebialabs/overthere/ssh/SshScpFile.java +++ b/overthere/src/main/java/com/xebialabs/overthere/ssh/SshScpFile.java @@ -17,26 +17,34 @@ package com.xebialabs.overthere.ssh; -import com.google.common.base.Joiner; -import com.xebialabs.overthere.CmdLine; -import com.xebialabs.overthere.OverthereFile; -import com.xebialabs.overthere.RuntimeIOException; -import com.xebialabs.overthere.util.CapturingOverthereProcessOutputHandler; +import static com.google.common.collect.Lists.newArrayList; +import static com.xebialabs.overthere.CmdLine.build; +import static com.xebialabs.overthere.ssh.SshConnection.NOCD_PSEUDO_COMMAND; +import static com.xebialabs.overthere.util.CapturingOverthereProcessOutputHandler.capturingHandler; +import static com.xebialabs.overthere.util.LoggingOverthereProcessOutputHandler.loggingHandler; +import static com.xebialabs.overthere.util.MultipleOverthereProcessOutputHandler.multiHandler; + +import java.io.File; +import java.io.FileInputStream; +import java.io.FileOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.util.List; +import java.util.StringTokenizer; + import net.schmizz.sshj.xfer.LocalFileFilter; import net.schmizz.sshj.xfer.LocalSourceFile; import net.schmizz.sshj.xfer.scp.SCPUploadClient; + import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.io.*; -import java.util.List; -import java.util.StringTokenizer; - -import static com.google.common.collect.Lists.newArrayList; -import static com.xebialabs.overthere.CmdLine.build; -import static com.xebialabs.overthere.util.CapturingOverthereProcessOutputHandler.capturingHandler; -import static com.xebialabs.overthere.util.LoggingOverthereProcessOutputHandler.loggingHandler; -import static com.xebialabs.overthere.util.MultipleOverthereProcessOutputHandler.multiHandler; +import com.google.common.base.Joiner; +import com.xebialabs.overthere.CmdLine; +import com.xebialabs.overthere.OverthereFile; +import com.xebialabs.overthere.RuntimeIOException; +import com.xebialabs.overthere.util.CapturingOverthereProcessOutputHandler; /** * A file on a host connected through SSH w/ SCP. @@ -106,7 +114,7 @@ public long length() { public LsResults getFileInfo() throws RuntimeIOException { LsResults results = new LsResults(); CapturingOverthereProcessOutputHandler capturedOutput = capturingHandler(); - int errno = executeCommand(capturedOutput, CmdLine.build("ls", "-ld", getPath())); + int errno = executeCommand(capturedOutput, CmdLine.build(NOCD_PSEUDO_COMMAND, "ls", "-ld", getPath())); if (errno == 0) { results.exists = true; if (capturedOutput.getOutputLines().size() > 0) { @@ -161,18 +169,22 @@ public static class LsResults { @Override public InputStream getInputStream() throws RuntimeIOException { - logger.debug("Opening ssh:scp: input stream to read from file {}", this); try { final File tempFile = File.createTempFile("scp_download", ".tmp"); tempFile.deleteOnExit(); + + logger.debug("Downloading contents of {} to temporary file {}", this, tempFile); connection.getSshClient().newSCPFileTransfer().download(getPath(), tempFile.getPath()); + + logger.debug("Opening input stream to temporary file {} to retrieve contents download from {}. Temporary file will be deleted when the stream is closed", tempFile, this); return new FileInputStream(tempFile) { @Override public void close() throws IOException { try { super.close(); } finally { + logger.debug("Removing temporary file {}", tempFile); tempFile.delete(); } @@ -185,11 +197,11 @@ public void close() throws IOException { @Override public OutputStream getOutputStream() throws RuntimeIOException { - logger.debug("Opening ssh:scp: output stream to write to file {}", this); - try { final File tempFile = File.createTempFile("scp_upload", ".tmp"); tempFile.deleteOnExit(); + + logger.debug("Opening output stream to temporary file {} to store contents to be uploaded to {} when the stream is closed", tempFile, this); return new FileOutputStream(tempFile) { @Override public void close() throws IOException { @@ -201,9 +213,11 @@ public void close() throws IOException { } private void uploadAndDelete(File tempFile) throws IOException { + logger.debug("Uploading contents of temporary file {} to to {}", tempFile, this); try { connection.getSshClient().newSCPFileTransfer().upload(tempFile.getPath(), getPath()); } finally { + logger.debug("Removing temporary file {}", tempFile); tempFile.delete(); } } @@ -220,7 +234,7 @@ public List listFiles() { CapturingOverthereProcessOutputHandler capturedOutput = capturingHandler(); // Yes, this *is* meant to be 'el es minus one'! Each file should go one a separate line, even if we create a pseudo-tty. Long format is NOT what we // want here. - int errno = executeCommand(multiHandler(loggingHandler(logger), capturedOutput), build("ls", "-1", getPath())); + int errno = executeCommand(multiHandler(loggingHandler(logger), capturedOutput), build(NOCD_PSEUDO_COMMAND, "ls", "-1", getPath())); if (errno != 0) { throw new RuntimeIOException("Cannot list directory " + this + ": " + capturedOutput.getError() + " (errno=" + errno + ")"); } @@ -246,7 +260,7 @@ public void mkdirs() { } protected void mkdir(String... mkdirOptions) throws RuntimeIOException { - CmdLine commandLine = CmdLine.build("mkdir"); + CmdLine commandLine = CmdLine.build(NOCD_PSEUDO_COMMAND, "mkdir"); for (String opt : mkdirOptions) { commandLine.addArgument(opt); } @@ -271,7 +285,7 @@ public void renameTo(OverthereFile dest) { SshScpFile sshScpDestFile = (SshScpFile) dest; if (sshScpDestFile.getConnection() == getConnection()) { CapturingOverthereProcessOutputHandler capturedOutput = capturingHandler(); - int errno = executeCommand(multiHandler(loggingHandler(logger), capturedOutput), CmdLine.build("mv", getPath(), sshScpDestFile.getPath())); + int errno = executeCommand(multiHandler(loggingHandler(logger), capturedOutput), CmdLine.build(NOCD_PSEUDO_COMMAND, "mv", getPath(), sshScpDestFile.getPath())); if (errno != 0) { throw new RuntimeIOException("Cannot rename file/directory " + this + ": " + capturedOutput.getError() + " (errno=" + errno + ")"); } @@ -290,7 +304,7 @@ public void setExecutable(boolean executable) { logger.debug("Setting execute permission on {} to {}", this, executable); CapturingOverthereProcessOutputHandler capturedOutput = capturingHandler(); - int errno = executeCommand(multiHandler(loggingHandler(logger), capturedOutput), CmdLine.build("chmod", executable ? "+x" : "-x", getPath())); + int errno = executeCommand(multiHandler(loggingHandler(logger), capturedOutput), CmdLine.build(NOCD_PSEUDO_COMMAND, "chmod", executable ? "a+x" : "a-x", getPath())); if (errno != 0) { throw new RuntimeIOException("Cannot set execute permission on file " + this + " to " + executable + ": " + capturedOutput.getError() + " (errno=" + errno + ")"); } @@ -301,7 +315,7 @@ protected void deleteDirectory() { logger.debug("Deleting directory {}", this); CapturingOverthereProcessOutputHandler capturedOutput = capturingHandler(); - int errno = executeCommand(multiHandler(loggingHandler(logger), capturedOutput), CmdLine.build("rmdir", getPath())); + int errno = executeCommand(multiHandler(loggingHandler(logger), capturedOutput), CmdLine.build(NOCD_PSEUDO_COMMAND, "rmdir", getPath())); if (errno != 0) { throw new RuntimeIOException("Cannot delete directory " + this + ": " + capturedOutput.getError() + " (errno=" + errno + ")"); } @@ -312,7 +326,7 @@ protected void deleteFile() { logger.debug("Deleting file {}", this); CapturingOverthereProcessOutputHandler capturedOutput = capturingHandler(); - int errno = executeCommand(multiHandler(loggingHandler(logger), capturedOutput), CmdLine.build("rm", "-f", getPath())); + int errno = executeCommand(multiHandler(loggingHandler(logger), capturedOutput), CmdLine.build(NOCD_PSEUDO_COMMAND, "rm", "-f", getPath())); if (errno != 0) { throw new RuntimeIOException("Cannot delete file " + this + ": " + capturedOutput.getError() + " (errno=" + errno + ")"); } @@ -323,7 +337,7 @@ public void deleteRecursively() throws RuntimeIOException { logger.debug("Recursively deleting file or directory {}", this); CapturingOverthereProcessOutputHandler capturedOutput = capturingHandler(); - int errno = executeCommand(multiHandler(loggingHandler(logger), capturedOutput), CmdLine.build("rm", "-rf", getPath())); + int errno = executeCommand(multiHandler(loggingHandler(logger), capturedOutput), CmdLine.build(NOCD_PSEUDO_COMMAND, "rm", "-rf", getPath())); if (errno != 0) { throw new RuntimeIOException("Cannot recursively delete file or directory " + this + ": " + capturedOutput.getError() + " (errno=" + errno + ")"); } diff --git a/overthere/src/main/java/com/xebialabs/overthere/ssh/SshSudoConnection.java b/overthere/src/main/java/com/xebialabs/overthere/ssh/SshSudoConnection.java index 6cced245..9e09cd8b 100644 --- a/overthere/src/main/java/com/xebialabs/overthere/ssh/SshSudoConnection.java +++ b/overthere/src/main/java/com/xebialabs/overthere/ssh/SshSudoConnection.java @@ -17,21 +17,30 @@ package com.xebialabs.overthere.ssh; -import com.xebialabs.overthere.*; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; +import static com.xebialabs.overthere.ssh.SshConnectionBuilder.SUDO_COMMAND_PREFIX; +import static com.xebialabs.overthere.ssh.SshConnectionBuilder.SUDO_COMMAND_PREFIX_DEFAULT; +import static com.xebialabs.overthere.ssh.SshConnectionBuilder.SUDO_OVERRIDE_UMASK; +import static com.xebialabs.overthere.ssh.SshConnectionBuilder.SUDO_OVERRIDE_UMASK_DEFAULT; +import static com.xebialabs.overthere.ssh.SshConnectionBuilder.SUDO_QUOTE_COMMAND; +import static com.xebialabs.overthere.ssh.SshConnectionBuilder.SUDO_QUOTE_COMMAND_DEFAULT; +import static com.xebialabs.overthere.ssh.SshConnectionBuilder.SUDO_USERNAME; import java.text.MessageFormat; -import static com.xebialabs.overthere.ssh.SshConnectionBuilder.*; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.xebialabs.overthere.CmdLine; +import com.xebialabs.overthere.CmdLineArgument; +import com.xebialabs.overthere.ConnectionOptions; +import com.xebialabs.overthere.OverthereFile; +import com.xebialabs.overthere.RuntimeIOException; /** * A connection to a Unix host using SSH w/ SUDO. */ class SshSudoConnection extends SshScpConnection { - public static final String SUDO_COMMAND = "sudo"; - public static final String NOSUDO_PSEUDO_COMMAND = "nosudo"; protected String sudoUsername; @@ -53,18 +62,29 @@ public SshSudoConnection(String type, ConnectionOptions options) { @Override protected CmdLine processCommandLine(final CmdLine commandLine) { CmdLine cmd; - if (commandLine.getArguments().size() >= 2 && commandLine.getArguments().get(0).toString(os, false).equals(NOSUDO_PSEUDO_COMMAND)) { - cmd = stripNosudoCommand(commandLine); + if (startsWithPseudoCommand(commandLine, NOSUDO_PSEUDO_COMMAND)) { + logger.trace("Not prefixing command line with sudo statement because the " + NOSUDO_PSEUDO_COMMAND + " pseudo command was present, but the pseudo command will be stripped"); + logger.trace("Replacing: {}", commandLine); + cmd = stripPrefixedPseudoCommand(commandLine); + logger.trace("With : {}", cmd); } else { - cmd = prefixWithSudoCommand(commandLine); + logger.trace("Prefixing command line with sudo statement"); + logger.trace("Replacing: {}", commandLine); + boolean nocd = startsWithPseudoCommand(commandLine, NOCD_PSEUDO_COMMAND); + if(nocd) { + cmd = stripPrefixedPseudoCommand(commandLine); + } else { + cmd = commandLine; + } + cmd = prefixWithSudoCommand(cmd); + if(nocd) { + cmd = prefixWithPseudoCommand(cmd, NOCD_PSEUDO_COMMAND); + } + logger.trace("With : {}", cmd); } return super.processCommandLine(cmd); } - protected CmdLine stripNosudoCommand(final CmdLine commandLine) { - return new CmdLine().add(commandLine.getArguments().subList(1, commandLine.getArguments().size())); - } - protected CmdLine prefixWithSudoCommand(final CmdLine commandLine) { CmdLine commandLineWithSudo = new CmdLine(); addSudoStatement(commandLineWithSudo); @@ -88,17 +108,6 @@ protected void addSudoStatement(CmdLine sudoCommandLine) { } } - protected int noSudoExecute(OverthereProcessOutputHandler handler, CmdLine commandLine) { - if (logger.isDebugEnabled()) { - logger.debug("NOT adding sudo statement"); - } - - CmdLine nosudoCommandLine = new CmdLine(); - nosudoCommandLine.addArgument(NOSUDO_PSEUDO_COMMAND); - nosudoCommandLine.add(commandLine.getArguments()); - return execute(handler, nosudoCommandLine); - } - @Override protected OverthereFile getFile(String hostPath, boolean isTempFile) throws RuntimeIOException { return new SshSudoFile(this, hostPath, isTempFile); @@ -109,7 +118,7 @@ public String toString() { return "ssh:" + sshConnectionType.toString().toLowerCase() + "://" + username + ":" + sudoUsername + "@" + host + ":" + port; } - private Logger logger = LoggerFactory.getLogger(SshSudoConnection.class); + private Logger logger = LoggerFactory.getLogger(SshSudoFile.class); } diff --git a/overthere/src/main/java/com/xebialabs/overthere/ssh/SshSudoFile.java b/overthere/src/main/java/com/xebialabs/overthere/ssh/SshSudoFile.java index d9a433c3..609637f4 100644 --- a/overthere/src/main/java/com/xebialabs/overthere/ssh/SshSudoFile.java +++ b/overthere/src/main/java/com/xebialabs/overthere/ssh/SshSudoFile.java @@ -30,6 +30,8 @@ import java.io.OutputStream; import static com.xebialabs.overthere.CmdLine.build; +import static com.xebialabs.overthere.ssh.SshConnection.NOCD_PSEUDO_COMMAND; +import static com.xebialabs.overthere.ssh.SshSudoConnection.NOSUDO_PSEUDO_COMMAND; import static com.xebialabs.overthere.util.CapturingOverthereProcessOutputHandler.capturingHandler; import static com.xebialabs.overthere.util.LoggingOverthereProcessOutputHandler.loggingHandler; import static com.xebialabs.overthere.util.MultipleOverthereProcessOutputHandler.multiHandler; @@ -59,10 +61,9 @@ public SshSudoFile(SshSudoConnection connection, String remotePath, boolean isTe @Override protected int executeCommand(OverthereProcessOutputHandler handler, CmdLine commandLine) { if (isTempFile) { - return ((SshSudoConnection) connection).noSudoExecute(handler, commandLine); - } else { - return super.executeCommand(handler, commandLine); + commandLine = SshConnection.prefixWithPseudoCommand(commandLine, NOSUDO_PSEUDO_COMMAND); } + return super.executeCommand(handler, commandLine); } @Override @@ -150,8 +151,8 @@ private void overrideUmask(OverthereFile remoteFile) { if (((SshSudoConnection) connection).sudoOverrideUmask) { logger.debug("Overriding umask by recursively setting permissions on files and/or directories copied with scp to be readable and executable (if needed) by group and other"); CapturingOverthereProcessOutputHandler capturedOutput = capturingHandler(); - int errno = ((SshSudoConnection) connection).noSudoExecute(multiHandler(loggingHandler(logger), capturedOutput), - CmdLine.build("chmod", "-R", "go+rX", remoteFile.getPath())); + int errno = connection.execute(multiHandler(loggingHandler(logger), capturedOutput), + CmdLine.build(NOSUDO_PSEUDO_COMMAND, NOCD_PSEUDO_COMMAND, "chmod", "-R", "go+rX", remoteFile.getPath())); if (errno != 0) { throw new RuntimeIOException("Cannot set permissions on file " + this + " to go+rX: " + capturedOutput.getError() + " (errno=" + errno + ")"); } @@ -162,14 +163,14 @@ void copyToTempFile(OverthereFile tempFile) { logger.debug("Copying actual file {} to temporary file {} before download", this, tempFile); CapturingOverthereProcessOutputHandler cpCapturedOutput = CapturingOverthereProcessOutputHandler.capturingHandler(); - int cpResult = getConnection().execute(multiHandler(loggingHandler(logger), cpCapturedOutput), build("cp", "-pr", this.getPath(), tempFile.getPath())); + int cpResult = getConnection().execute(multiHandler(loggingHandler(logger), cpCapturedOutput), build(NOCD_PSEUDO_COMMAND, "cp", "-pr", this.getPath(), tempFile.getPath())); if (cpResult != 0) { String errorMessage = cpCapturedOutput.getAll(); throw new RuntimeIOException("Cannot copy actual file " + this + " to temporary file " + tempFile + " before download: " + errorMessage); } CapturingOverthereProcessOutputHandler chmodCapturedOutput = capturingHandler(); - int chmodResult = getConnection().execute(multiHandler(loggingHandler(logger), chmodCapturedOutput), CmdLine.build("chmod", "-R", "go+rX", tempFile.getPath())); + int chmodResult = getConnection().execute(multiHandler(loggingHandler(logger), chmodCapturedOutput), CmdLine.build(NOCD_PSEUDO_COMMAND, "chmod", "-R", "go+rX", tempFile.getPath())); if (chmodResult != 0) { String errorMessage = chmodCapturedOutput.getAll(); throw new RuntimeIOException("Cannot grant group and other read and execute permissions (chmod -R go+rX) to file " + tempFile + " before download: " + errorMessage); @@ -180,7 +181,7 @@ void copyfromTempFile(OverthereFile tempFile) { logger.debug("Copying temporary file {} to actual file {} after upload", tempFile, this); CapturingOverthereProcessOutputHandler cpCapturedOutput = capturingHandler(); - CmdLine cmdLine = CmdLine.build("cp", "-pr"); + CmdLine cmdLine = CmdLine.build(NOCD_PSEUDO_COMMAND, "cp", "-pr"); String sourcePath = tempFile.getPath(); if (this.exists() && tempFile.isDirectory()) { sourcePath += "/*"; // FIXME: This will not copy hidden files!