Skip to content

Commit

Permalink
[FIXED JENKINS-26558] Clients should provide a unique ID to be used f…
Browse files Browse the repository at this point in the history
…or name collision avoidance

- The current name collision avoidance uses the requests address, which could very likely be the same for all clients
  as they could be being routed through a HTTP proxy (or two) so that is not a good disambiguator
- We use a digest of the client's interfaces and MAC addresses and the remoteFSRoot to try and give a consistent ID
- We ALWAYS append the ID if we have it as otherwise during reconnect the slaves with the same name will shuffle around
  which defeats a lot of the login that Jenkins has internally based on slaves having a consistent name
- In the event of legacy clients that do not have the ID we will let them connect with their name as long as there
  is no online slave with that name. This does mean that where there are multiple legacy swarm clients with the
  same name, only one can be on-line at any moment in time, but that is an improvement on the current where
  once a shuffle starts, none can stay on-line
  • Loading branch information
stephenc committed Apr 24, 2015
1 parent 7ce651a commit ab37bc8
Show file tree
Hide file tree
Showing 3 changed files with 127 additions and 34 deletions.
2 changes: 1 addition & 1 deletion client/src/main/java/hudson/plugins/swarm/Client.java
Expand Up @@ -77,7 +77,7 @@ public void run() throws InterruptedException {
swarmClient.verifyThatUrlIsHudson(target);
}

System.out.println("Attempting to connect to " + target.url + " " + target.secret);
System.out.println("Attempting to connect to " + target.url + " " + target.secret + " with ID " + swarmClient.getHash());

// create a new swarm slave
swarmClient.createSwarmSlave(target);
Expand Down
112 changes: 91 additions & 21 deletions client/src/main/java/hudson/plugins/swarm/SwarmClient.java
Expand Up @@ -2,58 +2,70 @@

import hudson.remoting.Launcher;
import hudson.remoting.jnlp.Main;
import org.apache.commons.httpclient.HttpClient;
import org.apache.commons.httpclient.HttpStatus;
import org.apache.commons.httpclient.UsernamePasswordCredentials;
import org.apache.commons.httpclient.auth.AuthScope;
import org.apache.commons.httpclient.methods.GetMethod;
import org.apache.commons.httpclient.methods.PostMethod;
import org.apache.commons.lang.StringUtils;
import org.w3c.dom.Document;
import org.w3c.dom.Element;
import org.w3c.dom.Node;
import org.w3c.dom.Text;
import org.xml.sax.SAXException;

import javax.net.ssl.KeyManager;
import javax.net.ssl.SSLContext;
import javax.net.ssl.TrustManager;
import javax.net.ssl.X509TrustManager;
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.parsers.ParserConfigurationException;
import java.io.ByteArrayInputStream;
import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.io.UnsupportedEncodingException;
import java.math.BigInteger;
import java.net.DatagramPacket;
import java.net.DatagramSocket;
import java.net.HttpURLConnection;
import java.net.Inet4Address;
import java.net.InetAddress;
import java.net.MalformedURLException;
import java.net.NetworkInterface;
import java.net.SocketException;
import java.net.SocketTimeoutException;
import java.net.URL;
import java.net.URLEncoder;
import java.security.KeyManagementException;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
import java.security.SecureRandom;
import java.security.cert.CertificateException;
import java.security.cert.X509Certificate;
import java.text.MessageFormat;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.LinkedList;
import java.util.List;
import java.util.Random;

import javax.net.ssl.KeyManager;
import javax.net.ssl.SSLContext;
import javax.net.ssl.TrustManager;
import javax.net.ssl.X509TrustManager;
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.parsers.ParserConfigurationException;

import org.apache.commons.httpclient.HttpClient;
import org.apache.commons.httpclient.HttpStatus;
import org.apache.commons.httpclient.UsernamePasswordCredentials;
import org.apache.commons.httpclient.auth.AuthScope;
import org.apache.commons.httpclient.methods.GetMethod;
import org.apache.commons.httpclient.methods.PostMethod;
import org.apache.commons.lang.StringUtils;
import org.w3c.dom.Document;
import org.w3c.dom.Element;
import org.w3c.dom.Node;
import org.w3c.dom.Text;
import org.xml.sax.SAXException;

public class SwarmClient {

private final Options options;

private final String hash;

public SwarmClient(Options options) {
this.options = options;
this.hash = hash(options.remoteFsRoot);
}

public String getHash() {
return hash;
}

public Candidate discoverFromBroadcast() throws IOException,
Expand Down Expand Up @@ -292,7 +304,9 @@ protected void createSwarmSlave(Candidate target) throws IOException, Interrupte
+ param("labels", labelStr)
+ param("toolLocations", toolLocationsStr)
+ "&secret=" + target.secret
+ param("mode", options.mode.toUpperCase()));
+ param("mode", options.mode.toUpperCase())
+ param("hash", hash)
);

post.setDoAuthentication(true);

Expand All @@ -306,6 +320,15 @@ protected void createSwarmSlave(Candidate target) throws IOException, Interrupte
throw new RetryException(
"Failed to create a slave on Jenkins CODE: " + responseCode);
}
String name = post.getResponseBodyAsString();
if (name == null) {
return;
}
name = name.trim();
if (name.isEmpty()) {
return;
}
options.name = name;
}

private String encode(String value) throws UnsupportedEncodingException {
Expand Down Expand Up @@ -368,6 +391,53 @@ private static String getChildElementString(Element parent, String tagName) {
return null;
}

/**
* Returns a hash that should be consistent for any individual swarm client (as long as it has a persistent IP)
* and should be unique to that client.
*
* @param remoteFsRoot the file system root should be part of the hash (to support multiple swarm clients from
* the same machine)
* @return our best effort at a consistent hash
*/
public static String hash(File remoteFsRoot) {
StringBuilder buf = new StringBuilder();
try {
buf.append(remoteFsRoot.getCanonicalPath()).append('\n');
} catch (IOException e) {
buf.append(remoteFsRoot.getAbsolutePath()).append('\n');
}
try {
for (NetworkInterface ni : Collections.list(NetworkInterface.getNetworkInterfaces())) {
for (InetAddress ia : Collections.list(ni.getInetAddresses())) {
if (ia instanceof Inet4Address) {
buf.append(ia.getHostAddress()).append('\n');
}
}
byte[] hardwareAddress = ni.getHardwareAddress();
if (hardwareAddress != null) {
buf.append(Arrays.toString(hardwareAddress));
}
}
} catch (SocketException e) {
// oh well we tried
}
return getDigestOf(buf.toString()).substring(0, 8);
}

public static String getDigestOf(String text) {
try {
return toHexString(MessageDigest.getInstance("MD5").digest(text.getBytes("UTF-8")));
} catch (NoSuchAlgorithmException e) {
throw new IllegalStateException("MD5 not installed", e); // impossible according to JLS
} catch (UnsupportedEncodingException e) {
throw new IllegalStateException("UTF-8 not supported", e); // impossible according to JLS
}
}

public static String toHexString(byte[] bytes) {
return String.format("%0" + (bytes.length * 2) + "x", new BigInteger(1, bytes));
}

private static class DefaultTrustManager implements X509TrustManager {

public void checkClientTrusted(X509Certificate[] x509Certificates, String s) throws CertificateException {
Expand Down
47 changes: 35 additions & 12 deletions plugin/src/main/java/hudson/plugins/swarm/PluginImpl.java
@@ -1,6 +1,6 @@
package hudson.plugins.swarm;

import static javax.servlet.http.HttpServletResponse.*;
import com.google.common.collect.Lists;
import hudson.Plugin;
import hudson.Util;
import hudson.model.Descriptor.FormException;
Expand All @@ -10,19 +10,21 @@
import hudson.tools.ToolInstallation;
import hudson.tools.ToolLocationNodeProperty;
import hudson.tools.ToolLocationNodeProperty.ToolLocation;

import java.io.IOException;
import java.io.Writer;
import java.util.List;

import jenkins.model.Jenkins;

import org.apache.commons.lang.StringUtils;
import org.kohsuke.stapler.QueryParameter;
import org.kohsuke.stapler.StaplerRequest;
import org.kohsuke.stapler.StaplerResponse;

import com.google.common.collect.Lists;
import javax.servlet.ServletOutputStream;
import java.io.IOException;
import java.io.Writer;
import java.util.List;

import static javax.servlet.http.HttpServletResponse.SC_CONFLICT;
import static javax.servlet.http.HttpServletResponse.SC_EXPECTATION_FAILED;
import static javax.servlet.http.HttpServletResponse.SC_FORBIDDEN;
import static javax.servlet.http.HttpServletResponse.SC_SERVICE_UNAVAILABLE;

/**
* Exposes an entry point to add a new swarm slave.
Expand All @@ -36,7 +38,7 @@ public class PluginImpl extends Plugin {
*/
public void doCreateSlave(StaplerRequest req, StaplerResponse rsp, @QueryParameter String name, @QueryParameter String description, @QueryParameter int executors,
@QueryParameter String remoteFsRoot, @QueryParameter String labels, @QueryParameter String secret, @QueryParameter Node.Mode mode,
@QueryParameter String toolLocations) throws IOException {
@QueryParameter String toolLocations, @QueryParameter(fixEmpty = true) String hash) throws IOException {

if (!getSwarmSecret().equals(secret)) {
rsp.setStatus(SC_FORBIDDEN);
Expand All @@ -54,9 +56,24 @@ public void doCreateSlave(StaplerRequest req, StaplerResponse rsp, @QueryParamet
nodeProperties = Lists.newArrayList(new ToolLocationNodeProperty(parsedToolLocations));
}

// try to make the name unique. Swarm clients are often replicated VMs, and they may have the same name.
if (jenkins.getNode(name) != null) {
name = name + '-' + req.getRemoteAddr();
if (hash == null && jenkins.getNode(name) != null) {
// this is a legacy client, they won't be able to pick up the new name, so throw them away
// perhaps they can find another master to connect to
rsp.setStatus(SC_CONFLICT);
return;
}
if (hash != null) {
// try to make the name unique. Swarm clients are often replicated VMs, and they may have the same name.
name = name + '-' + hash;
}
// check for existing connections
{
Node n = jenkins.getNode(name);
if (n != null && n.toComputer().isOnline()) {
// this is an existing connection, we'll only cause issues if we trample over an online connection
rsp.setStatus(SC_CONFLICT);
return;
}
}

SwarmSlave slave = new SwarmSlave(name, "Swarm slave from " + req.getRemoteHost() + " : " + description,
Expand All @@ -70,6 +87,12 @@ public void doCreateSlave(StaplerRequest req, StaplerResponse rsp, @QueryParamet
}
jenkins.addNode(slave);
}
rsp.setContentType("text/plain; UTF-8");
byte[] response = name.getBytes("UTF-8");
rsp.setContentLength(response.length);
ServletOutputStream outputStream = rsp.getOutputStream();
outputStream.write(response);
outputStream.flush();
} catch (FormException e) {
e.printStackTrace();
}
Expand Down

0 comments on commit ab37bc8

Please sign in to comment.