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

[JEP-230] [JENKINS-55582] Convert instance-identity to a detached plugin #6585

Merged
merged 20 commits into from
Jun 22, 2022
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
d5cdafe
[JENKINS-55582] Split instance-identity to a plugin
basil May 13, 2022
cf4d77c
Merge commit 'd5cdafeb1d290e224aa7f2a8e478b08be9435f41' into instance…
jglick May 19, 2022
70f6bf2
Update to `instance-identity` 3.1
jglick May 19, 2022
1ed16c0
Add form validation for agents https://github.com/jenkinsci/jenkins/p…
jglick May 19, 2022
a3cd19a
Will also need `bouncycastle-api` 2.26 for the `instance-identity` dep
jglick May 19, 2022
ab04b07
Improved Javadoc
jglick May 19, 2022
5eb2e05
`JNLPLauncherRealTest` turned up need for https://github.com/jenkinsc…
jglick May 19, 2022
f6af760
Merge branch 'master' of https://github.com/jenkinsci/jenkins into in…
jglick May 19, 2022
d96d64b
Also need to bump `sshd` in `split-plugins.txt` or we will get an old…
jglick May 19, 2022
da0fd2d
https://github.com/jenkinsci/sshd-plugin/pull/75 released
jglick May 19, 2022
3b263bc
Discourage use of `jenkins.model.identity` from plugins.
jglick May 19, 2022
a8c46f1
Merge branch 'master' of https://github.com/jenkinsci/jenkins into in…
jglick Jun 2, 2022
fb2908c
Provisionally targeting 2.350
jglick Jun 2, 2022
83fbe36
Merge branch 'master' of https://github.com/jenkinsci/jenkins into in…
jglick Jun 8, 2022
4205138
Merge branch 'master' of https://github.com/jenkinsci/jenkins into in…
jglick Jun 8, 2022
14c075b
Merge branch 'master' of https://github.com/jenkinsci/jenkins into in…
jglick Jun 9, 2022
ecd8400
Merge branch 'master' of https://github.com/jenkinsci/jenkins into in…
jglick Jun 10, 2022
6192c7e
Merge branch 'master' of https://github.com/jenkinsci/jenkins into in…
jglick Jun 14, 2022
46d571a
Update core/src/main/resources/jenkins/split-plugins.txt
timja Jun 14, 2022
1b55251
Merge branch 'master' of https://github.com/jenkinsci/jenkins into in…
jglick Jun 22, 2022
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
5 changes: 0 additions & 5 deletions bom/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -256,11 +256,6 @@ THE SOFTWARE.
<artifactId>remoting</artifactId>
<version>${remoting.version}</version>
</dependency>
<dependency>
<groupId>org.jenkins-ci.modules</groupId>
<artifactId>instance-identity</artifactId>
Copy link
Member Author

Choose a reason for hiding this comment

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

Once this is released, bom-weekly can get it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Though if I understand correctly from jenkinsci/bom#671 jenkinsci/bom#570 (comment) jenkinsci/bom#681, it cannot be added until this PR is in the oldest supported line, so that will be a while. In the meantime the handful of plugins using this dep can specify 3.1.

<version>2.2</version>
</dependency>
<dependency>
<groupId>org.jfree</groupId>
<artifactId>jfreechart</artifactId>
Expand Down
4 changes: 4 additions & 0 deletions core/src/main/java/hudson/slaves/JNLPLauncher.java
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import hudson.model.TaskListener;
import hudson.util.FormValidation;
import jenkins.model.Jenkins;
import jenkins.model.identity.InstanceIdentityProvider;
import jenkins.slaves.RemotingWorkDirSettings;
import jenkins.util.SystemProperties;
import jenkins.websocket.WebSockets;
Expand Down Expand Up @@ -251,6 +252,9 @@ public FormValidation doCheckWebSocket(@QueryParameter boolean webSocket, @Query
if (Jenkins.get().getTcpSlaveAgentListener() == null) {
return FormValidation.error("Either WebSocket mode is selected, or the TCP port for inbound agents must be enabled");
}
if (InstanceIdentityProvider.RSA.getCertificate() == null || InstanceIdentityProvider.RSA.getPrivateKey() == null) {
return FormValidation.error("You must install the instance-identity plugin to use inbound agents in TCP mode");
}
}
return FormValidation.ok();
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package jenkins.model.identity;

import edu.umd.cs.findbugs.annotations.CheckForNull;
import edu.umd.cs.findbugs.annotations.NonNull;
import hudson.Extension;
import hudson.model.UnprotectedRootAction;
import java.nio.charset.StandardCharsets;
Expand All @@ -11,7 +13,7 @@
* A simple root action that exposes the public key to users so that they do not need to search for the
* {@code X-Instance-Identity} response header, also exposes the fingerprint of the public key so that people
* can verify a fingerprint of a master before connecting to it.
*
* <p>Do not use this class from plugins. Depend on {@code instance-identity} directly instead.
* @since 2.16
*/
@Extension
Expand All @@ -35,7 +37,9 @@ public String getUrlName() {
* Returns the PEM encoded public key.
*
* @return the PEM encoded public key.
* Null if the {@code instance-identity} plugin is not enabled.
*/
@CheckForNull
public String getPublicKey() {
RSAPublicKey key = InstanceIdentityProvider.RSA.getPublicKey();
if (key == null) {
Expand All @@ -60,6 +64,7 @@ public String getPublicKey() {
*
* @return the fingerprint of the public key.
*/
@NonNull
public String getFingerprint() {
return KeyUtils.fingerprint(InstanceIdentityProvider.RSA.getPublicKey());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,13 @@
import java.security.interfaces.RSAPublicKey;
import java.util.logging.Level;
import java.util.logging.Logger;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;

/**
* A source of instance identity.
*
* <p>Should not be used from plugins, except to be implemented by {@code instance-identity}.
* Other plugins wishing to get the RSA key may depend on {@code instance-identity} directly.
* @param <PUB> the type of public key.
* @param <PRIV> the type of private key.
* @since 2.16
Expand All @@ -58,16 +61,23 @@ public abstract class InstanceIdentityProvider<PUB extends PublicKey, PRIV exten
/**
* RSA keys.
*/
@Restricted(NoExternalUse.class)
public static final KeyTypes<RSAPublicKey, RSAPrivateKey> RSA =
new KeyTypes<>(RSAPublicKey.class, RSAPrivateKey.class);
/**
* DSA keys.
* @deprecated unused
*/
@Restricted(NoExternalUse.class)
@Deprecated
public static final KeyTypes<DSAPublicKey, DSAPrivateKey> DSA =
new KeyTypes<>(DSAPublicKey.class, DSAPrivateKey.class);
/**
* EC keys
* @deprecated unused
*/
@Restricted(NoExternalUse.class)
@Deprecated
public static final KeyTypes<ECPublicKey, ECPrivateKey> EC =
new KeyTypes<>(ECPublicKey.class, ECPrivateKey.class);

Expand All @@ -77,6 +87,7 @@ public abstract class InstanceIdentityProvider<PUB extends PublicKey, PRIV exten
* @return the {@link KeyPair} that comprises the instance identity. {@code null} could technically be returned in
* the event that a keypair could not be generated, for example if the specific key type of this provider
* is not permitted at the required length by the JCA policy.
* More commonly it just means that the {@code instance-identity} plugin needs to be installed.
*/
@CheckForNull
protected abstract KeyPair getKeyPair();
Expand Down Expand Up @@ -120,6 +131,7 @@ protected PRIV getPrivateKey() {
* @param <PUB> the type of public key.
* @param <PRIV> the type of private key.
*/
@Restricted(NoExternalUse.class)
public static final class KeyTypes<PUB extends PublicKey, PRIV extends PrivateKey> {
/**
* The interface for the public key.
Expand Down Expand Up @@ -154,6 +166,7 @@ private KeyTypes(Class<PUB> pubKeyType, Class<PRIV> privKeyType) {
private static <PUB extends PublicKey, PRIV extends PrivateKey> InstanceIdentityProvider<PUB, PRIV> get(
@NonNull KeyTypes<PUB, PRIV> type) {
for (InstanceIdentityProvider provider : ExtensionList.lookup(InstanceIdentityProvider.class)) {
LOGGER.fine(() -> "loaded " + provider + " from " + provider.getClass().getProtectionDomain().getCodeSource().getLocation());
try {
KeyPair keyPair = provider.getKeyPair();
if (keyPair != null
Expand All @@ -173,6 +186,7 @@ private static <PUB extends PublicKey, PRIV extends PrivateKey> InstanceIdentity
"Instance identity provider " + provider + " propagated an uncaught exception", e);
}
}
LOGGER.fine("no providers");
return null;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,11 +103,11 @@ public JnlpSlaveAgentProtocol4() throws KeyStoreException, KeyManagementExceptio
// prepare our local identity and certificate
X509Certificate identityCertificate = InstanceIdentityProvider.RSA.getCertificate();
if (identityCertificate == null) {
throw new KeyStoreException("JENKINS-41987: no X509Certificate found; perhaps instance-identity module is missing or too old");
throw new KeyStoreException("JENKINS-41987: no X509Certificate found; perhaps instance-identity plugin is not installed");
}
RSAPrivateKey privateKey = InstanceIdentityProvider.RSA.getPrivateKey();
if (privateKey == null) {
throw new KeyStoreException("JENKINS-41987: no RSAPrivateKey found; perhaps instance-identity module is missing or too old");
throw new KeyStoreException("JENKINS-41987: no RSAPrivateKey found; perhaps instance-identity plugin is not installed");
}

// prepare our keyStore so we can provide our authentication
Expand Down
6 changes: 6 additions & 0 deletions core/src/main/resources/jenkins/split-plugin-cycles.txt
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,9 @@ jdk-tool jaxb
javax-activation-api javax-mail-api
javax-activation-api sshd
javax-mail-api sshd

# JENKINS-55582
bouncycastle-api instance-identity
bouncycastle-api sshd
javax-activation-api instance-identity
javax-mail-api instance-identity
5 changes: 4 additions & 1 deletion core/src/main/resources/jenkins/split-plugins.txt
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,10 @@ jaxb 2.163 2.3.0
trilead-api 2.184 1.0.4

# JENKINS-64107
sshd 2.281 3.0.1
sshd 2.281 3.236.ved5e1b_cb_50b_2

javax-activation-api 2.330 1.2.0-2
javax-mail-api 2.330 1.6.2-5

# JENKINS-55582
instance-identity 2.355 3.1
jglick marked this conversation as resolved.
Show resolved Hide resolved
1 change: 0 additions & 1 deletion docs/MAINTAINERS.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ and hence the Jenkins core pull request review and merge process is more sophist
This document applies to the following components:

* Jenkins core
* Jenkins modules
* Libraries included into the Jenkins core
* Core components like Winstone, Executable WAR, etc.

Expand Down
6 changes: 6 additions & 0 deletions test/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,12 @@ THE SOFTWARE.
<artifactId>test-annotations</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.jenkins-ci.modules</groupId>
<artifactId>instance-identity</artifactId>
<version>3.1</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>antisamy-markup-formatter</artifactId>
Expand Down
78 changes: 78 additions & 0 deletions test/src/test/java/hudson/slaves/JNLPLauncherRealTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
/*
* The MIT License
*
* Copyright 2022 CloudBees, Inc.
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/

package hudson.slaves;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.is;

import hudson.ExtensionList;
import hudson.PluginWrapper;
import hudson.model.FreeStyleBuild;
import hudson.model.FreeStyleProject;
import hudson.model.Slave;
import hudson.util.FormValidation;
import jenkins.slaves.JnlpSlaveAgentProtocol4;
import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.Description;
import org.junit.runners.model.Statement;
import org.jvnet.hudson.test.For;
import org.jvnet.hudson.test.InboundAgentRule;
import org.jvnet.hudson.test.Issue;
import org.jvnet.hudson.test.JenkinsRule;
import org.jvnet.hudson.test.RealJenkinsRule;

@For({JNLPLauncher.class, JnlpSlaveAgentProtocol4.class})
public class JNLPLauncherRealTest {

@Rule public RealJenkinsRule rr = new RealJenkinsRule().includeTestClasspathPlugins(false);

@Issue("JEP-230")
@Test public void smokes() throws Throwable {
/* Since RealJenkinsRuleInit.jpi will load detached plugins, to reproduce a failure use:
FileUtils.touch(new File(rr.getHome(), "plugins/instance-identity.jpi.disabled"));
*/
rr.then(JNLPLauncherRealTest::_smokes);
}

private static void _smokes(JenkinsRule r) throws Throwable {
InboundAgentRule inboundAgents = new InboundAgentRule(); // cannot use @Rule since it would not be accessible from the controller JVM
inboundAgents.apply(new Statement() {
@Override public void evaluate() throws Throwable {
for (PluginWrapper plugin : r.jenkins.pluginManager.getPlugins()) {
System.err.println(plugin + " active=" + plugin.isActive() + " enabled=" + plugin.isEnabled());
}
assertThat(ExtensionList.lookupSingleton(JNLPLauncher.DescriptorImpl.class).doCheckWebSocket(false, null).kind, is(FormValidation.Kind.OK));
Slave agent = inboundAgents.createAgent(r, "static");
FreeStyleProject p = r.createFreeStyleProject();
p.setAssignedNode(agent);
FreeStyleBuild b = r.buildAndAssertSuccess(p);
System.err.println(JenkinsRule.getLog(b));
}
}, Description.EMPTY).evaluate();

}

}
9 changes: 0 additions & 9 deletions test/src/test/java/jenkins/security/ClassFilterImplTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@
import org.junit.Rule;
import org.junit.Test;
import org.jvnet.hudson.test.BuildWatcher;
import org.jvnet.hudson.test.Issue;
import org.jvnet.hudson.test.JenkinsRule;
import org.jvnet.hudson.test.LoggerRule;
import org.jvnet.hudson.test.TestExtension;
Expand Down Expand Up @@ -165,14 +164,6 @@ public void xstreamRequiresWhitelist() throws Exception {
assertThat(data.values().iterator().next().extra, allOf(containsString("LinkedListMultimap"), containsString("https://www.jenkins.io/redirect/class-filter/")));
}

@Test
@Issue("JENKINS-49543")
public void moduleClassesShouldBeWhitelisted() {
ClassFilterImpl filter = new ClassFilterImpl();
filter.check("org.jenkinsci.modules.windows_slave_installer.WindowsSlaveInstaller");
filter.check("org.jenkinsci.main.modules.instance_identity.PageDecoratorImpl");
}

@TestExtension("xstreamRequiresWhitelist")
public static class Config extends GlobalConfiguration {
LinkedListMultimap<?, ?> obj;
Expand Down
14 changes: 8 additions & 6 deletions war/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -96,10 +96,6 @@ THE SOFTWARE.
</exclusion>
</exclusions>
</dependency>
<dependency>
<groupId>org.jenkins-ci.modules</groupId>
<artifactId>instance-identity</artifactId>
</dependency>
<dependency>
<!--
We bundle slf4j binding since we got some components (sshd for example)
Expand Down Expand Up @@ -399,7 +395,7 @@ THE SOFTWARE.
<artifactItem>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>bouncycastle-api</artifactId>
<version>2.25</version>
<version>2.26</version>
Copy link
Member Author

Choose a reason for hiding this comment

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

dep of current instance-identity

<type>hpi</type>
</artifactItem>
<artifactItem>
Expand All @@ -423,7 +419,7 @@ THE SOFTWARE.
<artifactItem>
<groupId>org.jenkins-ci.modules</groupId>
<artifactId>sshd</artifactId>
<version>3.0.3</version>
<version>3.236.ved5e1b_cb_50b_2</version>
Copy link
Member Author

Choose a reason for hiding this comment

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

<type>hpi</type>
</artifactItem>
<artifactItem>
Expand All @@ -444,6 +440,12 @@ THE SOFTWARE.
<version>1.6.2-5</version>
<type>hpi</type>
</artifactItem>
<artifactItem>
<groupId>org.jenkins-ci.modules</groupId>
<artifactId>instance-identity</artifactId>
<version>3.1</version>
<type>hpi</type>
</artifactItem>
</artifactItems>
<outputDirectory>${project.build.directory}/${project.build.finalName}/WEB-INF/detached-plugins</outputDirectory>
<stripVersion>true</stripVersion>
Expand Down