DEPLOYITPB-3141 DEPLOYITPB-3142 Github #39 Github #40 Method setExecu…

…table 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".
xebialabs · Mar 20, 2012 · 440fb2e · 440fb2e
1 parent 660a54f
commit 440fb2e
Show file tree

Hide file tree

Showing 4 changed files with 117 additions and 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
@@ -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
@@ -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<OverthereFile> 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
@@ -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);
 
 }