Skip to content

Commit 297c3c2

Browse files
olivergondzaolivergondza
committed
Do not rely on Jenkins url for resource identification
This is problematic for multiple Jenkinses running on localhost(s).
1 parent 5c6e435 commit 297c3c2

File tree

7 files changed

+108
-31
lines changed

7 files changed

+108
-31
lines changed

‎plugin/src/main/java/jenkins/plugins/openstack/compute/JCloudsSlave.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,8 @@ public class JCloudsSlave extends AbstractCloudSlave implements TrackedItem {
5959

6060
/** metadata fields that aren't worth showing to the user. */
6161
private static final List<String> HIDDEN_METADATA_VALUES = Arrays.asList(
62-
Openstack.FINGERPRINT_KEY,
62+
Openstack.FINGERPRINT_KEY_FINGERPRINT,
63+
Openstack.FINGERPRINT_KEY_URL,
6364
JCloudsSlaveTemplate.OPENSTACK_CLOUD_NAME_KEY,
6465
JCloudsSlaveTemplate.OPENSTACK_TEMPLATE_NAME_KEY,
6566
ServerScope.METADATA_KEY

‎plugin/src/main/java/jenkins/plugins/openstack/compute/internal/FipScope.java‎

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,16 +29,35 @@
2929

3030
import javax.annotation.CheckForNull;
3131
import javax.annotation.Nonnull;
32+
import java.util.Objects;
3233

34+
/**
35+
* Scope of FIP for deletion.
36+
*
37+
* This uses URL+ServerID, do not need to use instance identity as the ServerID is unique.
38+
*/
3339
@Restricted(NoExternalUse.class)
3440
public class FipScope {
41+
/*package*/ static final int MAX_DESCRIPTION_LENGTH = 250;
42+
43+
public static @Nonnull String getDescription(
44+
@Nonnull String url, @Nonnull String identity, @Nonnull Server server
45+
) {
46+
String description = "{ '" + Openstack.FINGERPRINT_KEY_URL + "': '" + url + "', '"
47+
+ Openstack.FINGERPRINT_KEY_FINGERPRINT + "': '" + identity
48+
+ "', 'jenkins-scope': 'server:" + server.getId() + "' }"
49+
;
50+
51+
if (description.length() < MAX_DESCRIPTION_LENGTH) return description;
3552

36-
public static @Nonnull String getDescription(@Nonnull String instanceFingerprint, @Nonnull Server server) {
37-
return "{ '" + Openstack.FINGERPRINT_KEY + "': '" + instanceFingerprint + "', 'jenkins-scope': 'server:" + server.getId() + "' }";
53+
// Avoid URL that is used only for human consumption anyway
54+
return "{ '" + Openstack.FINGERPRINT_KEY_FINGERPRINT + "': '" + identity
55+
+ "', 'jenkins-scope': 'server:" + server.getId() + "' }"
56+
;
3857
}
3958

40-
public static @CheckForNull String getServerId(@Nonnull String instanceFingerprint, @CheckForNull String description) {
41-
String scope = getScopeString(instanceFingerprint, description);
59+
public static @CheckForNull String getServerId(@Nonnull String url, @Nonnull String identity, @CheckForNull String description) {
60+
String scope = getScopeString(url, identity, description);
4261
if (scope == null) return null;
4362

4463
if (!scope.startsWith("server:")) {
@@ -47,12 +66,16 @@ public class FipScope {
4766
return scope.substring(7);
4867
}
4968

50-
private static @CheckForNull String getScopeString(@Nonnull String instanceFingerprint, @CheckForNull String description) {
69+
private static @CheckForNull String getScopeString(@Nonnull String url, @Nonnull String identity, @CheckForNull String description) {
5170
try {
5271
JSONObject jsonObject = JSONObject.fromObject(description);
5372

54-
boolean isOurs = instanceFingerprint.equals(jsonObject.getString(Openstack.FINGERPRINT_KEY));
55-
if (!isOurs) return null;
73+
// Not ours
74+
String attachedIdentity = jsonObject.optString(Openstack.FINGERPRINT_KEY_FINGERPRINT, null);
75+
String attachedUrl = jsonObject.optString(Openstack.FINGERPRINT_KEY_URL, null);
76+
if (attachedIdentity == null && attachedUrl == null) return null;
77+
if (attachedIdentity != null && !Objects.equals(attachedIdentity, identity)) return null;
78+
if (attachedUrl != null && !Objects.equals(attachedUrl, url)) return null;
5679

5780
return jsonObject.getString("jenkins-scope");
5881
} catch (net.sf.json.JSONException ex) {

‎plugin/src/main/java/jenkins/plugins/openstack/compute/internal/Openstack.java‎

Lines changed: 44 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@
2525

2626
import java.io.File;
2727
import java.io.IOException;
28+
import java.nio.charset.Charset;
29+
import java.security.interfaces.RSAPublicKey;
2830
import java.util.ArrayList;
2931
import java.util.Collection;
3032
import java.util.Collections;
@@ -53,6 +55,7 @@
5355

5456
import com.cloudbees.plugins.credentials.common.PasswordCredentials;
5557
import com.google.common.annotations.VisibleForTesting;
58+
import com.google.common.base.Charsets;
5659
import com.google.common.base.MoreObjects;
5760
import com.google.common.cache.Cache;
5861
import com.google.common.cache.CacheBuilder;
@@ -65,6 +68,9 @@
6568
import hudson.remoting.Which;
6669
import hudson.util.FormValidation;
6770
import jenkins.plugins.openstack.compute.auth.OpenstackCredential;
71+
import org.apache.commons.codec.binary.Base64;
72+
import org.apache.commons.codec.digest.DigestUtils;
73+
import org.jenkinsci.main.modules.instance_identity.InstanceIdentity;
6874
import org.kohsuke.accmod.Restricted;
6975
import org.kohsuke.accmod.restrictions.NoExternalUse;
7076
import org.openstack4j.api.Builders;
@@ -116,7 +122,10 @@
116122
public class Openstack {
117123

118124
private static final Logger LOGGER = Logger.getLogger(Openstack.class.getName());
119-
public static final String FINGERPRINT_KEY = "jenkins-instance";
125+
public static final String FINGERPRINT_KEY_URL = "jenkins-instance";
126+
public static final String FINGERPRINT_KEY_FINGERPRINT = "jenkins-identity";
127+
128+
private static String INSTANCE_FINGERPRINT;
120129

121130
private static final Comparator<Date> ACCEPT_NULLS = Comparator.nullsLast(Comparator.naturalOrder());
122131
private static final Comparator<Flavor> FLAVOR_COMPARATOR = Comparator.nullsLast(Comparator.comparing(Flavor::getName));
@@ -367,7 +376,7 @@ public Openstack(@Nonnull final OSClient<?> client) {
367376
for (NetFloatingIP ip : clientProvider.get().networking().floatingip().list()) {
368377
if (ip.getFixedIpAddress() != null) continue; // Used
369378

370-
String serverId = FipScope.getServerId(instanceFingerprint(), ip.getDescription());
379+
String serverId = FipScope.getServerId(instanceUrl(), instanceFingerprint(), ip.getDescription());
371380
if (serverId == null) continue; // Not ours
372381

373382
freeIps.add(ip.getId());
@@ -489,20 +498,41 @@ public static boolean isOccupied(@Nonnull Server server) {
489498
}
490499

491500
private boolean isOurs(@Nonnull Server server) {
492-
return instanceFingerprint().equals(server.getMetadata().get(FINGERPRINT_KEY));
501+
Map<String, String> metadata = server.getMetadata();
502+
String serverFingerprint = metadata.get(FINGERPRINT_KEY_FINGERPRINT);
503+
return serverFingerprint == null
504+
? Objects.equals(instanceUrl(), metadata.get(FINGERPRINT_KEY_URL)) // Earlier versions ware only using URL, collect severs provisioned by those
505+
: Objects.equals(instanceUrl(), metadata.get(FINGERPRINT_KEY_URL)) && Objects.equals(instanceFingerprint(), serverFingerprint)
506+
;
493507
}
494508

495509
/**
496510
* Identification for instances launched by this instance via this plugin.
497511
*
498512
* @return Identifier to filter instances we control.
499513
*/
500-
private @Nonnull String instanceFingerprint() {
514+
@VisibleForTesting
515+
public @Nonnull String instanceUrl() {
501516
String rootUrl = Jenkins.get().getRootUrl();
502517
if (rootUrl == null) throw new IllegalStateException("Jenkins instance URL is not configured");
503518
return rootUrl;
504519
}
505520

521+
/**
522+
* Binary fingerprint to be unique worldwide.
523+
*/
524+
@VisibleForTesting
525+
public @Nonnull String instanceFingerprint() {
526+
if (INSTANCE_FINGERPRINT == null) {
527+
// Use salted hash not to disclose the public key. The key is used to authenticate agent connections.
528+
INSTANCE_FINGERPRINT = DigestUtils.sha1Hex(
529+
"openstack-cloud-plugin-identity-fingerprint:"
530+
+ new String(Base64.encodeBase64(InstanceIdentity.get().getPublic().getEncoded()), Charsets.UTF_8)
531+
);
532+
}
533+
return INSTANCE_FINGERPRINT;
534+
}
535+
506536
public @Nonnull Server getServerById(@Nonnull String id) throws NoSuchElementException {
507537
Server server = clientProvider.get().compute().servers().get(id);
508538
if (server == null) throw new NoSuchElementException("No such server running: " + id);
@@ -526,6 +556,9 @@ private boolean isOurs(@Nonnull Server server) {
526556
*/
527557
public @Nonnull Server bootAndWaitActive(@Nonnull ServerCreateBuilder request, @Nonnegative int timeout) throws ActionFailed {
528558
debug("Booting machine");
559+
560+
// Mark the server as ours
561+
attachFingerprint(request);
529562
try {
530563
Server server = _bootAndWaitActive(request, timeout);
531564
if (server == null) {
@@ -558,9 +591,14 @@ private boolean isOurs(@Nonnull Server server) {
558591
}
559592
}
560593

594+
@VisibleForTesting // mocking
595+
public void attachFingerprint(@Nonnull ServerCreateBuilder request) {
596+
request.addMetadataItem(FINGERPRINT_KEY_URL, instanceUrl());
597+
request.addMetadataItem(FINGERPRINT_KEY_FINGERPRINT, instanceFingerprint());
598+
}
599+
561600
@Restricted(NoExternalUse.class) // Test hook
562601
public Server _bootAndWaitActive(@Nonnull ServerCreateBuilder request, @Nonnegative int timeout) {
563-
request.addMetadataItem(FINGERPRINT_KEY, instanceFingerprint());
564602
return clientProvider.get().compute().servers().bootAndWaitActive(request.build(), timeout);
565603
}
566604

@@ -626,7 +664,7 @@ public void destroyServer(@Nonnull Server server) throws ActionFailed {
626664
debug("Allocating floating IP for {0} in {1}", server.getName(), server.getName());
627665
NetworkingService networking = clientProvider.get().networking();
628666

629-
String desc = FipScope.getDescription(instanceFingerprint(), server);
667+
String desc = FipScope.getDescription(instanceUrl(), instanceFingerprint(), server);
630668

631669
Port port = getServerPorts(server).get(0);
632670
Network network = networking.network().list(Collections.singletonMap("name", poolName)).get(0);

‎plugin/src/test/java/jenkins/plugins/openstack/PluginTestRule.java‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@
9191
import static org.mockito.Matchers.any;
9292
import static org.mockito.Mockito.RETURNS_SMART_NULLS;
9393
import static org.mockito.Mockito.doAnswer;
94+
import static org.mockito.Mockito.doCallRealMethod;
9495
import static org.mockito.Mockito.mock;
9596
import static org.mockito.Mockito.when;
9697
import static org.mockito.Mockito.withSettings;
@@ -374,7 +375,11 @@ public JCloudsCloud configureSlaveProvisioning(JCloudsCloud cloud, Collection<Ne
374375
Map<String, Network> nets = (Map<String, Network>) invocation.getArguments()[0];
375376
return nets.values().stream().collect(Collectors.toMap(n -> n, b -> 100));
376377
});
377-
when(os.bootAndWaitActive(any(ServerCreateBuilder.class), any(Integer.class))).thenAnswer((Answer<Server>) invocation -> {
378+
when(os.bootAndWaitActive(any(ServerCreateBuilder.class), any(Integer.class))).thenCallRealMethod();
379+
doCallRealMethod().when(os).attachFingerprint(any(ServerCreateBuilder.class));
380+
doCallRealMethod().when(os).instanceUrl();
381+
doCallRealMethod().when(os).instanceFingerprint();
382+
when(os._bootAndWaitActive(any(ServerCreateBuilder.class), any(Integer.class))).thenAnswer((Answer<Server>) invocation -> {
378383
ServerCreateBuilder builder = (ServerCreateBuilder) invocation.getArguments()[0];
379384

380385
ServerCreate create = builder.build();
@@ -508,7 +513,6 @@ public MockServerBuilder() {
508513
when(server.getStatus()).thenReturn(Server.Status.ACTIVE);
509514
when(server.getMetadata()).thenReturn(metadata);
510515
when(server.getOsExtendedVolumesAttached()).thenReturn(Collections.singletonList(UUID.randomUUID().toString()));
511-
metadata.put("jenkins-instance", jenkins.getRootUrl()); // Mark the slave as ours
512516
}
513517

514518
public MockServerBuilder name(String name) {

‎plugin/src/test/java/jenkins/plugins/openstack/compute/ProvisioningTest.java‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
import hudson.model.FreeStyleProject;
88
import hudson.model.Label;
99
import hudson.model.Node;
10-
import hudson.model.Slave;
1110
import hudson.model.TaskListener;
1211
import hudson.node_monitors.DiskSpaceMonitorDescriptor;
1312
import hudson.plugins.sshslaves.SSHLauncher;
@@ -43,13 +42,16 @@
4342
import java.util.concurrent.ExecutionException;
4443
import java.util.concurrent.Future;
4544

45+
import static jenkins.plugins.openstack.compute.internal.Openstack.FINGERPRINT_KEY_FINGERPRINT;
4646
import static org.hamcrest.MatcherAssert.assertThat;
4747
import static org.hamcrest.Matchers.containsString;
4848
import static org.hamcrest.Matchers.empty;
4949
import static org.hamcrest.Matchers.emptyIterable;
50+
import static org.hamcrest.Matchers.emptyString;
5051
import static org.hamcrest.Matchers.equalTo;
5152
import static org.hamcrest.Matchers.instanceOf;
5253
import static org.hamcrest.Matchers.iterableWithSize;
54+
import static org.hamcrest.Matchers.not;
5355
import static org.hamcrest.Matchers.startsWith;
5456
import static org.junit.Assert.assertEquals;
5557
import static org.junit.Assert.assertFalse;
@@ -299,10 +301,13 @@ public void correctMetadataSet() throws Exception {
299301
JCloudsSlaveTemplate template = j.dummySlaveTemplate("label");
300302
final JCloudsCloud cloud = j.configureSlaveProvisioningWithFloatingIP(j.dummyCloud(template));
301303

304+
assertThat(cloud.getOpenstack().instanceUrl(), not(emptyString()));
305+
assertThat(cloud.getOpenstack().instanceFingerprint(), not(emptyString()));
306+
System.out.println(cloud.getOpenstack().instanceFingerprint());
302307
Server server = template.provisionServer(null, null);
303308
Map<String, String> m = server.getMetadata();
304-
305-
assertEquals(j.getURL().toExternalForm(), m.get(Openstack.FINGERPRINT_KEY));
309+
assertEquals(cloud.getOpenstack().instanceUrl(), m.get(Openstack.FINGERPRINT_KEY_URL));
310+
assertEquals(cloud.getOpenstack().instanceFingerprint(), m.get(FINGERPRINT_KEY_FINGERPRINT));
306311
assertEquals(cloud.name, m.get(JCloudsSlaveTemplate.OPENSTACK_CLOUD_NAME_KEY));
307312
assertEquals(template.getName(), m.get(JCloudsSlaveTemplate.OPENSTACK_TEMPLATE_NAME_KEY));
308313
assertEquals(new ServerScope.Node(server.getName()).getValue(), m.get(ServerScope.METADATA_KEY));

‎plugin/src/test/java/jenkins/plugins/openstack/compute/internal/FipScopeTest.java‎

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -34,14 +34,16 @@
3434

3535
public class FipScopeTest {
3636

37-
private static final String FNGRPRNT = "https://some-quite-long-jenkins-url-to-make-sure-it-fits.acme.com:8080/jenkins";
38-
public static final String REALISTIC_EXAMPLE
39-
= "{ 'jenkins-instance': 'https://some-quite-long-jenkins-url-to-make-sure-it-fits.acme.com:8080/jenkins', 'jenkins-scope': 'server:d8eca2df-7795-4069-b2ef-1e2412491345' }";
37+
private static final String URL = "https://some-quite-long-jenkins-url-to-make-sure-it-fits.acme.com:8080/jenkins";
38+
private static final String FINGERPRINT = "3919bce9a2f5fc4f730bd6462e23454ecb1fb089";
39+
public static final String LEGACY_DESCRIPTION = "{ 'jenkins-instance': 'https://some-quite-long-jenkins-url-to-make-sure-it-fits.acme.com:8080/jenkins', 'jenkins-scope': 'server:d8eca2df-7795-4069-b2ef-1e2412491345' }";
40+
public static final String EXPECTED_DESCRIPTION = "{ 'jenkins-instance': 'https://some-quite-long-jenkins-url-to-make-sure-it-fits.acme.com:8080/jenkins', 'jenkins-identity': '3919bce9a2f5fc4f730bd6462e23454ecb1fb089', 'jenkins-scope': 'server:d8eca2df-7795-4069-b2ef-1e2412491345' }";
41+
public static final String ABBREVIATED_DESCRIPTION = "{ 'jenkins-identity': '3919bce9a2f5fc4f730bd6462e23454ecb1fb089', 'jenkins-scope': 'server:d8eca2df-7795-4069-b2ef-1e2412491345' }";
4042

4143
@Test
4244
public void getDescription() {
43-
assertEquals(REALISTIC_EXAMPLE, FipScope.getDescription(FNGRPRNT, server("d8eca2df-7795-4069-b2ef-1e2412491345")));
44-
assertThat("Possible length is larger than maximal size of FIP description", REALISTIC_EXAMPLE.length(), lessThanOrEqualTo(250));
45+
assertEquals(EXPECTED_DESCRIPTION, FipScope.getDescription(URL, FINGERPRINT, server("d8eca2df-7795-4069-b2ef-1e2412491345")));
46+
assertThat("Possible length is larger than maximal size of FIP description", EXPECTED_DESCRIPTION.length(), lessThanOrEqualTo(FipScope.MAX_DESCRIPTION_LENGTH));
4547
}
4648

4749
private Server server(String id) {
@@ -52,13 +54,16 @@ private Server server(String id) {
5254

5355
@Test
5456
public void getServerId() {
55-
assertEquals("d8eca2df-7795-4069-b2ef-1e2412491345", FipScope.getServerId(FNGRPRNT, REALISTIC_EXAMPLE));
57+
// assertEquals("d8eca2df-7795-4069-b2ef-1e2412491345", FipScope.getServerId(URL, FINGERPRINT, EXPECTED_DESCRIPTION));
58+
assertEquals("d8eca2df-7795-4069-b2ef-1e2412491345", FipScope.getServerId(URL, FINGERPRINT, LEGACY_DESCRIPTION));
59+
assertEquals("d8eca2df-7795-4069-b2ef-1e2412491345", FipScope.getServerId(URL, FINGERPRINT, ABBREVIATED_DESCRIPTION));
5660

57-
assertNull(FipScope.getServerId(FNGRPRNT, "{ 'jenkins-instance': 'https://some.other.jenkins.io', 'jenkins-scope': 'server:d8eca2df-7795-4069-b2ef-1e2412491345' }"));
58-
assertNull(FipScope.getServerId(FNGRPRNT, "{ [ 'not', 'the', 'json', 'you', 'expect' ] }"));
59-
assertNull(FipScope.getServerId(FNGRPRNT, "[ 'not', 'the', 'json', 'you', 'expect' ]"));
60-
assertNull(FipScope.getServerId(FNGRPRNT, "Human description"));
61-
assertNull(FipScope.getServerId(FNGRPRNT, ""));
62-
assertNull(FipScope.getServerId(FNGRPRNT, null));
61+
assertNull(FipScope.getServerId(URL, FINGERPRINT, "{ 'jenkins-instance': 'https://some.other.jenkins.io', 'jenkins-scope': 'server:d8eca2df-7795-4069-b2ef-1e2412491345' }"));
62+
assertNull(FipScope.getServerId(URL, FINGERPRINT, "{ 'jenkins-identity': 'different-than-expected', 'jenkins-scope': 'server:d8eca2df-7795-4069-b2ef-1e2412491345' }"));
63+
assertNull(FipScope.getServerId(URL, FINGERPRINT, "{ foo: [ 'not', 'the', 'json', 'you', 'expect' ] }"));
64+
assertNull(FipScope.getServerId(URL, FINGERPRINT, "[ 'not', 'the', 'json', 'you', 'expect' ]"));
65+
assertNull(FipScope.getServerId(URL, FINGERPRINT, "Human description"));
66+
assertNull(FipScope.getServerId(URL, FINGERPRINT, ""));
67+
assertNull(FipScope.getServerId(URL, FINGERPRINT, null));
6368
}
6469
}

‎plugin/src/test/java/jenkins/plugins/openstack/compute/internal/OpenstackTest.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -379,6 +379,7 @@ public void deleteAfterFailedBoot() {
379379

380380
doReturn(server).when(os)._bootAndWaitActive(any(ServerCreateBuilder.class), any(Integer.class));
381381
doThrow(new Openstack.ActionFailed("Fake deletion failure")).when(os).destroyServer(server);
382+
doNothing().when(os).attachFingerprint(any(ServerCreateBuilder.class));
382383

383384
try {
384385
os.bootAndWaitActive(mock(ServerCreateBuilder.class), 1);

0 commit comments

Comments
 (0)