Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[JENKINS-41745] Non-Remoting-based CLI #2795

Merged
merged 64 commits into from
Apr 8, 2017
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
64 commits
Select commit Hold shift + click to select a range
492dbbe
[JENKINS-41745] Make jenkins-cli.jar connect to the SSH port by default.
jglick Mar 10, 2017
2fe2487
FindBugs, and more clearly stating which transport is in use.
jglick Mar 10, 2017
8302b85
Draft of a new CLI transport that can operate over CLIAction with the…
jglick Mar 11, 2017
7da0d39
Argument processing mistake.
jglick Mar 13, 2017
1e860c8
Apply SUREFIRE-1226 workaround across all modules, including cli.
jglick Mar 13, 2017
624ba59
Allow tests to run which use CLI.<init> in process.
jglick Mar 13, 2017
4df2bf1
Merge branch 'master' into SSH-CLI-JENKINS-41745
jglick Mar 13, 2017
c2a5d85
Establishing baseline behavior of JENKINS-12543: no workaround when u…
jglick Mar 13, 2017
8d622e0
Clarifying role of API tokens in -remoting.
jglick Mar 13, 2017
12ae48e
Deprecating --username/--password and login/logout in favor of new -a…
jglick Mar 13, 2017
b8ad360
Marking various APIs, and a couple of commands, deprecated when they …
jglick Mar 13, 2017
82cd1bd
Deleteing apparently unused class SequenceOutputStream.
jglick Mar 13, 2017
5f0bb2a
Some more work deprecating channel/checkChannel.
jglick Mar 13, 2017
9ffb5d8
Hoping to fix an unreproducible test hang.
jglick Mar 13, 2017
2cf0ac5
Pick up https://github.com/jenkinsci/sshd-module/pull/10 so we are us…
jglick Mar 15, 2017
9e427ce
Noting potential issue in ConsoleCommand.
jglick Mar 15, 2017
f437ccc
Merge branch 'master' into SSH-CLI-JENKINS-41745
jglick Mar 15, 2017
9434d0e
UI to enable or disable CLI over Remoting.
jglick Mar 16, 2017
71db95a
Allow http://repo.jenkins-ci.org/public/org/jenkins-ci/modules/sshd/1…
jglick Mar 16, 2017
996c52e
Trying to avoid a premature test timeout.
jglick Mar 17, 2017
59d5013
Merge branch 'master' into SSH-CLI-JENKINS-41745
jglick Mar 17, 2017
473744c
Merge branch 'master' into SSH-CLI-JENKINS-41745
jglick Mar 17, 2017
289db88
Using Logger consistently for all messages that are not expected to b…
jglick Mar 17, 2017
f3dae19
Fixed locale handling.
jglick Mar 17, 2017
7174e3e
Allowing install-plugin to load a file from standard input so as to w…
jglick Mar 17, 2017
7baf81d
Allowing `build -p fileParam= prj` to load a file from standard input…
jglick Mar 17, 2017
e69ba0f
Merge branch 'master' into SSH-CLI-JENKINS-41745
jglick Mar 24, 2017
60632c0
Added -strictHostKey option to CLI in -ssh mode.
jglick Mar 24, 2017
466d74d
Merge branch 'master' into SSH-CLI-JENKINS-41745
jglick Apr 3, 2017
f168936
Test failure on Windows, perhaps specific to filesystem type of Tempo…
jglick Apr 3, 2017
551808c
Using kill -QUIT to try to diagnose client hangs observed on ci.jenki…
jglick Apr 3, 2017
f6314b7
Testing interrupt behavior.
jglick Apr 3, 2017
9812db7
Improved handling of stream closure.
jglick Apr 3, 2017
71d7562
Merge branch 'master' into SSH-CLI-JENKINS-41745
jglick Apr 4, 2017
56cee95
Found another case where the Windows CI build was complaining about k…
jglick Apr 4, 2017
c74999f
Found a race condition which could explain random CI hangs of CLIActi…
jglick Apr 4, 2017
4c1910a
Silence an annoying message.
jglick Apr 4, 2017
4069e67
Merge branch 'master' into SSH-CLI-JENKINS-41745
jglick Apr 4, 2017
03aa2a1
Simpler to catch ReadPendingException in just one place.
jglick Apr 4, 2017
fb311c6
Suppress a meaningless ClosedChannelException thrown by Jetty when ki…
jglick Apr 4, 2017
caf3e8e
Removing comment corresponding to something I can no longer reproduce.
jglick Apr 4, 2017
7f633de
CLITest.interrupt could fail if you had a stale localhost entry in ~/…
jglick Apr 4, 2017
f7006b4
Jetty 9.4.3.v20170317 refuses to set an empty HTTP header, leading to…
jglick Apr 4, 2017
3b92399
Tested Jetty 9.4.3.v20170317 but ReadPendingException’s did not disap…
jglick Apr 4, 2017
94d5f96
Figured out why Security232Test.commonsCollections1 was being skipped…
jglick Apr 4, 2017
dbe24a8
Using CLICommandInvoker in cases where we did not actually need to be…
jglick Apr 4, 2017
5caee58
Noting that ssh-cli-auth is obsolete.
jglick Apr 4, 2017
9fc59bf
Finally figured out how to suppress the ReadPendingException from Jetty.
jglick Apr 4, 2017
7bca0a1
Refined implementation of -logger.
jglick Apr 5, 2017
3939820
Using logger as suggested by @rsandell.
jglick Apr 5, 2017
da58159
-http mode broke when the user omitted the mandatory final `/` in the…
jglick Apr 5, 2017
724ebbc
Improved error message displayed when using -http against a pre-JENKI…
jglick Apr 5, 2017
49f0360
Merge branch 'master' into SSH-CLI-JENKINS-41745
jglick Apr 6, 2017
c41c97b
@stephenc requested documentation of the deprecated FullDuplexHttpStr…
jglick Apr 6, 2017
6c63890
Demonstrating that interleaved stdio does work in -http mode.
jglick Apr 6, 2017
9150d5b
Suppress an idle timeout from Jetty which would otherwise interfere w…
jglick Apr 6, 2017
6ffc57a
Verifying that PlainCLIProtocol ignores unrecognized opcodes, so long…
jglick Apr 6, 2017
2e7fe04
Merge branch 'master' into SSH-CLI-JENKINS-41745
jglick Apr 7, 2017
f3da0e4
Minor review comments from @oleg-nenashev.
jglick Apr 7, 2017
0f87e10
@oleg-nenashev requested this be decoupled from https://github.com/je…
jglick Apr 7, 2017
98f227c
Showing general help message rather than suggesting obsolete authenti…
jglick Apr 7, 2017
45092f0
checkChannel() provides a better error message in case you are not us…
jglick Apr 7, 2017
27d508d
sshd 1.11
jglick Apr 7, 2017
7ae404c
Revert "Noting that ssh-cli-auth is obsolete."
jglick Apr 7, 2017
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions cli/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@
<artifactId>commons-codec</artifactId>
<version>1.4</version>
</dependency>
<dependency>
<groupId>commons-io</groupId>
<artifactId>commons-io</artifactId>
</dependency>
<dependency>
<groupId>${project.groupId}</groupId>
<artifactId>remoting</artifactId>
Expand All @@ -50,6 +54,17 @@
<version>1.24</version>
</dependency>
<dependency>
<groupId>org.apache.sshd</groupId>
<artifactId>sshd-core</artifactId>
<version>1.2.0</version> <!-- TODO 1.3.0 requires Java 8 -->
<optional>true</optional> <!-- do not expose to core -->
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's already in the core due to the sshd-module

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it is in war.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, nvm

</dependency>
<dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-nop</artifactId>
<optional>true</optional> <!-- ditto -->
</dependency>
<dependency> <!-- TODO remove and replace PrivateKeyProvider with SecurityUtils.createFileKeyPairProvider() as in SshClient -->
<groupId>org.jenkins-ci</groupId>
<artifactId>trilead-ssh2</artifactId>
<version>build214-jenkins-1</version>
Expand Down
229 changes: 217 additions & 12 deletions cli/src/main/java/hudson/cli/CLI.java
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import hudson.remoting.RemoteOutputStream;
import hudson.remoting.SocketChannelStream;
import hudson.remoting.SocketOutputStream;
import hudson.util.QuotedStringTokenizer;

import javax.crypto.SecretKey;
import javax.crypto.spec.SecretKeySpec;
Expand All @@ -57,8 +58,11 @@
import java.net.HttpURLConnection;
import java.net.InetSocketAddress;
import java.net.Socket;
import java.net.SocketAddress;
import java.net.SocketTimeoutException;
import java.net.URL;
import java.net.URLConnection;
import java.nio.charset.Charset;
import java.security.GeneralSecurityException;
import java.security.KeyPair;
import java.security.PublicKey;
Expand All @@ -70,11 +74,24 @@
import java.util.List;
import java.util.Locale;
import java.util.Properties;
import java.util.Set;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.logging.ConsoleHandler;
import java.util.logging.Level;
import java.util.logging.Logger;
import static java.util.logging.Level.*;
import org.apache.sshd.client.SshClient;
import org.apache.sshd.client.channel.ClientChannel;
import org.apache.sshd.client.channel.ClientChannelEvent;
import org.apache.sshd.client.future.ConnectFuture;
import org.apache.sshd.client.keyverifier.DefaultKnownHostsServerKeyVerifier;
import org.apache.sshd.client.keyverifier.KnownHostsServerKeyVerifier;
import org.apache.sshd.client.keyverifier.ServerKeyVerifier;
import org.apache.sshd.client.session.ClientSession;
import org.apache.sshd.common.future.WaitableFuture;
import org.apache.sshd.common.util.io.NoCloseInputStream;
import org.apache.sshd.common.util.io.NoCloseOutputStream;

/**
* CLI entry point to Jenkins.
Expand All @@ -90,6 +107,7 @@ public class CLI implements AutoCloseable {
private final String httpsProxyTunnel;
private final String authorization;

/** Connection via {@link Mode#REMOTING}, for tests only. */
public CLI(URL jenkins) throws IOException, InterruptedException {
this(jenkins,null);
}
Expand All @@ -111,7 +129,8 @@ public CLI(URL jenkins, ExecutorService exec) throws IOException, InterruptedExc
public CLI(URL jenkins, ExecutorService exec, String httpsProxyTunnel) throws IOException, InterruptedException {
this(new CLIConnectionFactory().url(jenkins).executorService(exec).httpsProxyTunnel(httpsProxyTunnel));
}


/** Connection via {@link Mode#REMOTING}. */
/*package*/ CLI(CLIConnectionFactory factory) throws IOException, InterruptedException {
URL jenkins = factory.jenkins;
this.httpsProxyTunnel = factory.httpsProxyTunnel;
Expand All @@ -128,7 +147,8 @@ public CLI(URL jenkins, ExecutorService exec, String httpsProxyTunnel) throws IO
try {
_channel = connectViaCliPort(jenkins, getCliTcpPort(url));
} catch (IOException e) {
LOGGER.log(Level.FINE,"Failed to connect via CLI port. Falling back to HTTP",e);
System.err.println("Failed to connect via CLI port. Falling back to HTTP: " + e.getMessage());
LOGGER.log(Level.FINE, null, e);
try {
_channel = connectViaHttp(url);
} catch (IOException e2) {
Expand All @@ -146,9 +166,8 @@ public CLI(URL jenkins, ExecutorService exec, String httpsProxyTunnel) throws IO
}

private Channel connectViaHttp(String url) throws IOException {
LOGGER.log(FINE, "Trying to connect to {0} via HTTP", url);
url+="cli";
URL jenkins = new URL(url);
LOGGER.log(FINE, "Trying to connect to {0} via Remoting over HTTP", url);
URL jenkins = new URL(url + "cli?remoting=true");

FullDuplexHttpStream con = new FullDuplexHttpStream(jenkins,authorization);
Channel ch = new Channel("Chunked connection to "+jenkins,
Expand All @@ -165,7 +184,7 @@ protected void onDead() {
}

private Channel connectViaCliPort(URL jenkins, CliPort clip) throws IOException {
LOGGER.log(FINE, "Trying to connect directly via TCP/IP to {0}", clip.endpoint);
LOGGER.log(FINE, "Trying to connect directly via Remoting over TCP/IP to {0}", clip.endpoint);
final Socket s = new Socket();
// this prevents a connection from silently terminated by the router in between or the other peer
// and that goes without unnoticed. However, the time out is often very long (for example 2 hours
Expand Down Expand Up @@ -375,12 +394,6 @@ public void upgrade() {
}

public static void main(final String[] _args) throws Exception {
// Logger l = Logger.getLogger(Channel.class.getName());
// l.setLevel(ALL);
// ConsoleHandler h = new ConsoleHandler();
// h.setLevel(ALL);
// l.addHandler(h);
//
try {
System.exit(_main(_args));
} catch (Throwable t) {
Expand All @@ -390,6 +403,7 @@ public static void main(final String[] _args) throws Exception {
}
}

private enum Mode {HTTP, SSH, REMOTING}
public static int _main(String[] _args) throws Exception {
List<String> args = Arrays.asList(_args);
PrivateKeyProvider provider = new PrivateKeyProvider();
Expand All @@ -403,12 +417,40 @@ public static int _main(String[] _args) throws Exception {

boolean tryLoadPKey = true;

Mode mode = null;

String user = null;

while(!args.isEmpty()) {
String head = args.get(0);
if (head.equals("-version")) {
System.out.println("Version: "+computeVersion());
return 0;
}
if (head.equals("-http")) {
if (mode != null) {
printUsage("-http clashes with previously defined mode " + mode);
return -1;
}
mode = Mode.HTTP;
args = args.subList(1, args.size());
}
if (head.equals("-ssh")) {
if (mode != null) {
printUsage("-ssh clashes with previously defined mode " + mode);
return -1;
}
mode = Mode.SSH;
args = args.subList(1, args.size());
}
if (head.equals("-remoting")) {
if (mode != null) {
printUsage("-remoting clashes with previously defined mode " + mode);
return -1;
}
mode = Mode.REMOTING;
args = args.subList(1, args.size());
}
if(head.equals("-s") && args.size()>=2) {
url = args.get(1);
args = args.subList(2,args.size());
Expand Down Expand Up @@ -446,11 +488,27 @@ public boolean verify(String s, SSLSession sslSession) {
sshAuthRequestedExplicitly = true;
continue;
}
if (head.equals("-user") && args.size() >= 2) {
user = args.get(1);
args = args.subList(2, args.size());
continue;
}
if(head.equals("-p") && args.size()>=2) {
httpProxy = args.get(1);
args = args.subList(2,args.size());
continue;
}
if (head.equals("-logger") && args.size() >= 2) {
Level level = parse(args.get(1));
ConsoleHandler h = new ConsoleHandler();
h.setLevel(level);
for (Logger logger : new Logger[] {LOGGER, PlainCLIProtocol.LOGGER}) { // perhaps also Channel
logger.setLevel(level);
logger.addHandler(h);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You seem to assume at other places that at least LOGGER already has a ConsoleHandler so wouldn't this produce double output?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually it does, though only for messages at INFO and above (if there are any at all), which matters little when you are seeing all kinds of messages at FINE or whatever you set the level to. However if you do not attach a new Handler then -logger will have no effect at all, since the default handler is set to INFO.

@ydubreuil’s trick from #2711 solves the conflict, so I will use that. As a bonus, it also makes -logger OFF and the like work.

}
args = args.subList(2, args.size());
continue;
}
break;
}

Expand All @@ -465,12 +523,35 @@ public boolean verify(String s, SSLSession sslSession) {
if (tryLoadPKey && !provider.hasKeys())
provider.readFromDefaultLocations();

if (mode == null) {
mode = Mode.HTTP;
}

LOGGER.log(FINE, "using connection mode {0}", mode);

if (mode == Mode.SSH) {
if (user == null) {
// TODO SshCliAuthenticator already autodetects the user based on public key; why cannot AsynchronousCommand.getCurrentUser do the same?
System.err.println("-user required when using -ssh");
return -1;
}
return sshConnection(url, user, args, provider);
}

if (user != null) {
System.err.println("Warning: -user ignored unless using -ssh");
}

CLIConnectionFactory factory = new CLIConnectionFactory().url(url).httpsProxyTunnel(httpProxy);
String userInfo = new URL(url).getUserInfo();
if (userInfo != null) {
factory = factory.basicAuth(userInfo);
}

if (mode == Mode.HTTP) {
return plainHttpConnection(url, args, factory);
}

CLI cli = factory.connect();
try {
if (provider.hasKeys()) {
Expand Down Expand Up @@ -507,6 +588,130 @@ public boolean verify(String s, SSLSession sslSession) {
}
}

private static int sshConnection(String jenkinsUrl, String user, List<String> args, PrivateKeyProvider provider) throws IOException {
URL url = new URL(jenkinsUrl + "/login");
URLConnection conn = url.openConnection();
String endpointDescription = conn.getHeaderField("X-SSH-Endpoint");

if (endpointDescription == null) {
System.err.println("No header 'X-SSH-Endpoint' returned by Jenkins");
return -1;
}

LOGGER.log(FINE, "Connecting via SSH to: {0}", endpointDescription);

int sshPort = Integer.parseInt(endpointDescription.split(":")[1]);
String sshHost = endpointDescription.split(":")[0];

StringBuilder command = new StringBuilder();

for (String arg : args) {
command.append(QuotedStringTokenizer.quote(arg));
command.append(' ');
}

try(SshClient client = SshClient.setUpDefaultClient()) {

KnownHostsServerKeyVerifier verifier = new DefaultKnownHostsServerKeyVerifier(new ServerKeyVerifier() {
@Override
public boolean verifyServerKey(ClientSession clientSession, SocketAddress remoteAddress, PublicKey serverKey) {
/** unknown key is okay, but log */
LOGGER.log(Level.WARNING, "Unknown host key for {0}", remoteAddress.toString());
// TODO should not trust unknown hosts by default; this should be opt-in
return true;
}
}, true);

client.setServerKeyVerifier(verifier);
client.start();

ConnectFuture cf = client.connect(user, sshHost, sshPort);
cf.await();
try (ClientSession session = cf.getSession()) {
for (KeyPair pair : provider.getKeys()) {
System.err.println("Offering " + pair.getPrivate().getAlgorithm() + " private key");
session.addPublicKeyIdentity(pair);
}
session.auth().verify(10000L);

try (ClientChannel channel = session.createExecChannel(command.toString())) {
channel.setIn(new NoCloseInputStream(System.in));
channel.setOut(new NoCloseOutputStream(System.out));
channel.setErr(new NoCloseOutputStream(System.err));
WaitableFuture wf = channel.open();
wf.await();

Set waitMask = channel.waitFor(Collections.singletonList(ClientChannelEvent.CLOSED), 0L);

if(waitMask.contains(ClientChannelEvent.TIMEOUT)) {
throw new SocketTimeoutException("Failed to retrieve command result in time: " + command);
}

Integer exitStatus = channel.getExitStatus();
return exitStatus;

}
} finally {
client.stop();
}
}
}

private static int plainHttpConnection(String url, List<String> args, CLIConnectionFactory factory) throws IOException, InterruptedException {
LOGGER.log(FINE, "Trying to connect to {0} via plain protocol over HTTP", url);
URL jenkins = new URL(url + "cli?remoting=false");
FullDuplexHttpStream streams = new FullDuplexHttpStream(jenkins, factory.authorization);
class ClientSideImpl extends PlainCLIProtocol.ClientSide {
int exit = -1;
ClientSideImpl(InputStream is, OutputStream os) throws IOException {
super(is, os);
if (is.read() != 0) { // cf. FullDuplexHttpService
throw new IOException("expected to see initial zero byte");
}
}
@Override
protected synchronized void onExit(int code) {
this.exit = code;
notify();
}
@Override
protected void onStdout(byte[] chunk) throws IOException {
System.out.write(chunk);
}
@Override
protected void onStderr(byte[] chunk) throws IOException {
System.err.write(chunk);
}
}
final ClientSideImpl connection = new ClientSideImpl(streams.getInputStream(), streams.getOutputStream());
for (String arg : args) {
connection.sendArg(arg);
}
connection.sendEncoding(Charset.defaultCharset().name());
connection.sendLocale(Locale.getDefault().toString());
connection.sendStart();
connection.begin();
final OutputStream stdin = connection.streamStdin();
new Thread("input reader") {
@Override
public void run() {
try {
int c;
while ((c = System.in.read()) != -1) {
stdin.write(c);
}
connection.sendEndStdin();
} catch (IOException x) {
x.printStackTrace();
}
}
}.start();
synchronized (connection) {
connection.wait();
}
return connection.exit;
}

private static String computeVersion() {
Properties props = new Properties();
try {
Expand Down
6 changes: 5 additions & 1 deletion cli/src/main/java/hudson/cli/CLIConnectionFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ public CLIConnectionFactory url(String jenkins) throws MalformedURLException {

/**
* This {@link ExecutorService} is used to execute closures received from the server.
* Used only in Remoting mode.
*/
public CLIConnectionFactory executorService(ExecutorService es) {
this.exec = es;
Expand Down Expand Up @@ -67,7 +68,10 @@ public CLIConnectionFactory basicAuth(String username, String password) {
public CLIConnectionFactory basicAuth(String userInfo) {
return authorization("Basic " + new String(Base64.encodeBase64((userInfo).getBytes())));
}


/**
* Used only in Remoting mode.
*/
public CLI connect() throws IOException, InterruptedException {
return new CLI(this);
}
Expand Down
Loading