From faf2de39a6bc39232965cded2a20a334f2e1607f Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Mon, 28 Oct 2024 11:17:01 -0400 Subject: [PATCH 1/5] Introducing `ControllerToAgentCallable` --- core/src/main/java/hudson/Launcher.java | 47 +++---------------- .../security/ControllerToAgentCallable.java | 47 +++++++++++++++++++ .../security/MasterToSlaveCallable.java | 9 ++-- .../jenkins/slaves/RemotingVersionInfo.java | 3 +- 4 files changed, 58 insertions(+), 48 deletions(-) create mode 100644 core/src/main/java/jenkins/security/ControllerToAgentCallable.java diff --git a/core/src/main/java/hudson/Launcher.java b/core/src/main/java/hudson/Launcher.java index e6ba431691fb..5906617f8e40 100644 --- a/core/src/main/java/hudson/Launcher.java +++ b/core/src/main/java/hudson/Launcher.java @@ -56,6 +56,7 @@ import java.util.logging.Level; import java.util.logging.Logger; import jenkins.model.Jenkins; +import jenkins.security.ControllerToAgentCallable; import jenkins.security.MasterToSlaveCallable; import jenkins.tasks.filters.EnvVarsFilterLocalRule; import jenkins.tasks.filters.EnvVarsFilterRuleWrapper; @@ -1114,8 +1115,7 @@ public Proc launch(ProcStarter ps) throws IOException { final String workDir = psPwd == null ? null : psPwd.getRemote(); try { - RemoteLaunchCallable remote = new RemoteLaunchCallable(ps.commands, ps.masks, ps.envs, in, ps.reverseStdin, out, ps.reverseStdout, err, ps.reverseStderr, ps.quiet, workDir, listener, ps.stdoutListener); - remote.setEnvVarsFilterRuleWrapper(envVarsFilterRuleWrapper); + RemoteLaunchCallable remote = new RemoteLaunchCallable(ps.commands, ps.masks, ps.envs, in, ps.reverseStdin, out, ps.reverseStdout, err, ps.reverseStderr, ps.quiet, workDir, listener, ps.stdoutListener, envVarsFilterRuleWrapper); // reset the rules to prevent build step without rules configuration to re-use those envVarsFilterRuleWrapper = null; return new ProcImpl(getChannel().call(remote)); @@ -1334,46 +1334,13 @@ public interface RemoteProcess { IOTriplet getIOtriplet(); } - private static class RemoteLaunchCallable extends MasterToSlaveCallable { - private final @NonNull List cmd; - private final @CheckForNull boolean[] masks; - private final @CheckForNull String[] env; - private final @CheckForNull InputStream in; - private final @CheckForNull OutputStream out; - private final @CheckForNull OutputStream err; - private final @CheckForNull String workDir; - private final @NonNull TaskListener listener; - private final @CheckForNull TaskListener stdoutListener; - private final boolean reverseStdin, reverseStdout, reverseStderr; - private final boolean quiet; - - private EnvVarsFilterRuleWrapper envVarsFilterRuleWrapper; - - RemoteLaunchCallable(@NonNull List cmd, @CheckForNull boolean[] masks, @CheckForNull String[] env, + private record RemoteLaunchCallable(@NonNull List cmd, @CheckForNull boolean[] masks, @CheckForNull String[] env, @CheckForNull InputStream in, boolean reverseStdin, @CheckForNull OutputStream out, boolean reverseStdout, @CheckForNull OutputStream err, boolean reverseStderr, - boolean quiet, @CheckForNull String workDir, @NonNull TaskListener listener, @CheckForNull TaskListener stdoutListener) { - this.cmd = new ArrayList<>(cmd); - this.masks = masks; - this.env = env; - this.in = in; - this.out = out; - this.err = err; - this.workDir = workDir; - this.listener = listener; - this.stdoutListener = stdoutListener; - this.reverseStdin = reverseStdin; - this.reverseStdout = reverseStdout; - this.reverseStderr = reverseStderr; - this.quiet = quiet; - } - - @Restricted(NoExternalUse.class) - public void setEnvVarsFilterRuleWrapper(EnvVarsFilterRuleWrapper envVarsFilterRuleWrapper) { - this.envVarsFilterRuleWrapper = envVarsFilterRuleWrapper; - } - + boolean quiet, @CheckForNull String workDir, + @NonNull TaskListener listener, @CheckForNull TaskListener stdoutListener, + @CheckForNull EnvVarsFilterRuleWrapper envVarsFilterRuleWrapper) implements ControllerToAgentCallable { @Override public RemoteProcess call() throws IOException { final Channel channel = getOpenChannelOrFail(); @@ -1433,8 +1400,6 @@ public IOTriplet getIOtriplet() { } }); } - - private static final long serialVersionUID = 1L; } private static class RemoteChannelLaunchCallable extends MasterToSlaveCallable { diff --git a/core/src/main/java/jenkins/security/ControllerToAgentCallable.java b/core/src/main/java/jenkins/security/ControllerToAgentCallable.java new file mode 100644 index 000000000000..989e4392f050 --- /dev/null +++ b/core/src/main/java/jenkins/security/ControllerToAgentCallable.java @@ -0,0 +1,47 @@ +/* + * The MIT License + * + * Copyright 2024 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 jenkins.security; + +import hudson.remoting.Callable; +import jenkins.slaves.RemotingVersionInfo; +import org.jenkinsci.remoting.RoleChecker; + +/** + * {@link Callable} meant to be serialized then run on an agent. + * A typical implementation will be a {@link Record} + * since instance state merely transfers a set of parameters to an agent JVM. + *

Note that the logic within {@link #call} may not use Remoting APIs + * newer than {@link RemotingVersionInfo#getMinimumSupportedVersion}. + * (Core and plugin APIs will be identical to those run inside the controller.) + * @param the return type; note that this must either be defined in your plugin or included in the stock JEP-200 whitelist + * @since TODO + */ +public interface ControllerToAgentCallable extends Callable { + + @Override + default void checkRoles(RoleChecker checker) throws SecurityException { + checker.check(this, Roles.SLAVE); + } +} diff --git a/core/src/main/java/jenkins/security/MasterToSlaveCallable.java b/core/src/main/java/jenkins/security/MasterToSlaveCallable.java index db0406bcf548..7d0319a2e9b8 100644 --- a/core/src/main/java/jenkins/security/MasterToSlaveCallable.java +++ b/core/src/main/java/jenkins/security/MasterToSlaveCallable.java @@ -1,15 +1,12 @@ package jenkins.security; import hudson.remoting.Callable; -import jenkins.slaves.RemotingVersionInfo; import org.jenkinsci.remoting.RoleChecker; /** - * Convenient {@link Callable} meant to be run on agent. - * - * Note that the logic within {@link #call()} should use API of a minimum supported Remoting version. - * See {@link RemotingVersionInfo#getMinimumSupportedVersion()}. - * + * {@link Callable} meant to be run on agent. + * For new code, implement {@link ControllerToAgentCallable} + * which has the advantage that it can be used on {@code record}s. * @author Kohsuke Kawaguchi * @since 1.587 / 1.580.1 * @param the return type; note that this must either be defined in your plugin or included in the stock JEP-200 whitelist diff --git a/core/src/main/java/jenkins/slaves/RemotingVersionInfo.java b/core/src/main/java/jenkins/slaves/RemotingVersionInfo.java index fb17b7243c9f..dd6b3d60c0c9 100644 --- a/core/src/main/java/jenkins/slaves/RemotingVersionInfo.java +++ b/core/src/main/java/jenkins/slaves/RemotingVersionInfo.java @@ -31,6 +31,7 @@ import java.util.Properties; import java.util.logging.Level; import java.util.logging.Logger; +import jenkins.security.ControllerToAgentCallable; /** * Provides information about Remoting versions used within the core. @@ -100,7 +101,7 @@ public static VersionNumber getEmbeddedVersion() { /** * Gets Remoting version which is supported by the core. - * Jenkins core and plugins make invoke operations on agents (e.g. {@link jenkins.security.MasterToSlaveCallable}) + * Jenkins core and plugins make invoke operations on agents (e.g. {@link ControllerToAgentCallable}) * and use Remoting-internal API within them. * In such case this API should be present on the remote side. * This method defines a minimum expected version, so that all calls should use a compatible API. From 42fd2cf28c07509b2490d5176264ffdca1f136fb Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Fri, 1 Nov 2024 16:47:47 -0400 Subject: [PATCH 2/5] `MasterToSlaveCallable` can simply implement `ControllerToAgentCallable` --- .../main/java/jenkins/security/MasterToSlaveCallable.java | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/core/src/main/java/jenkins/security/MasterToSlaveCallable.java b/core/src/main/java/jenkins/security/MasterToSlaveCallable.java index 7d0319a2e9b8..b4c9701ac76b 100644 --- a/core/src/main/java/jenkins/security/MasterToSlaveCallable.java +++ b/core/src/main/java/jenkins/security/MasterToSlaveCallable.java @@ -1,7 +1,6 @@ package jenkins.security; import hudson.remoting.Callable; -import org.jenkinsci.remoting.RoleChecker; /** * {@link Callable} meant to be run on agent. @@ -11,12 +10,7 @@ * @since 1.587 / 1.580.1 * @param the return type; note that this must either be defined in your plugin or included in the stock JEP-200 whitelist */ -public abstract class MasterToSlaveCallable implements Callable { +public abstract class MasterToSlaveCallable implements ControllerToAgentCallable { private static final long serialVersionUID = 1L; - - @Override - public void checkRoles(RoleChecker checker) throws SecurityException { - checker.check(this, Roles.SLAVE); - } } From d7af17539e48552e42174b0af41a528bb5f27ed4 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Fri, 1 Nov 2024 16:49:47 -0400 Subject: [PATCH 3/5] `jenkins.agents` seems a more appropriate package; this is not really about security --- core/src/main/java/hudson/Launcher.java | 2 +- .../{security => agents}/ControllerToAgentCallable.java | 3 ++- core/src/main/java/jenkins/security/MasterToSlaveCallable.java | 1 + core/src/main/java/jenkins/slaves/RemotingVersionInfo.java | 2 +- 4 files changed, 5 insertions(+), 3 deletions(-) rename core/src/main/java/jenkins/{security => agents}/ControllerToAgentCallable.java (97%) diff --git a/core/src/main/java/hudson/Launcher.java b/core/src/main/java/hudson/Launcher.java index 5906617f8e40..195671b1b42e 100644 --- a/core/src/main/java/hudson/Launcher.java +++ b/core/src/main/java/hudson/Launcher.java @@ -55,8 +55,8 @@ import java.util.Map; import java.util.logging.Level; import java.util.logging.Logger; +import jenkins.agents.ControllerToAgentCallable; import jenkins.model.Jenkins; -import jenkins.security.ControllerToAgentCallable; import jenkins.security.MasterToSlaveCallable; import jenkins.tasks.filters.EnvVarsFilterLocalRule; import jenkins.tasks.filters.EnvVarsFilterRuleWrapper; diff --git a/core/src/main/java/jenkins/security/ControllerToAgentCallable.java b/core/src/main/java/jenkins/agents/ControllerToAgentCallable.java similarity index 97% rename from core/src/main/java/jenkins/security/ControllerToAgentCallable.java rename to core/src/main/java/jenkins/agents/ControllerToAgentCallable.java index 989e4392f050..9b0dc2576190 100644 --- a/core/src/main/java/jenkins/security/ControllerToAgentCallable.java +++ b/core/src/main/java/jenkins/agents/ControllerToAgentCallable.java @@ -22,9 +22,10 @@ * THE SOFTWARE. */ -package jenkins.security; +package jenkins.agents; import hudson.remoting.Callable; +import jenkins.security.Roles; import jenkins.slaves.RemotingVersionInfo; import org.jenkinsci.remoting.RoleChecker; diff --git a/core/src/main/java/jenkins/security/MasterToSlaveCallable.java b/core/src/main/java/jenkins/security/MasterToSlaveCallable.java index b4c9701ac76b..1f6fe3b246a4 100644 --- a/core/src/main/java/jenkins/security/MasterToSlaveCallable.java +++ b/core/src/main/java/jenkins/security/MasterToSlaveCallable.java @@ -1,6 +1,7 @@ package jenkins.security; import hudson.remoting.Callable; +import jenkins.agents.ControllerToAgentCallable; /** * {@link Callable} meant to be run on agent. diff --git a/core/src/main/java/jenkins/slaves/RemotingVersionInfo.java b/core/src/main/java/jenkins/slaves/RemotingVersionInfo.java index dd6b3d60c0c9..d595a21d22df 100644 --- a/core/src/main/java/jenkins/slaves/RemotingVersionInfo.java +++ b/core/src/main/java/jenkins/slaves/RemotingVersionInfo.java @@ -31,7 +31,7 @@ import java.util.Properties; import java.util.logging.Level; import java.util.logging.Logger; -import jenkins.security.ControllerToAgentCallable; +import jenkins.agents.ControllerToAgentCallable; /** * Provides information about Remoting versions used within the core. From 9f3cd3379a1996cd80b3853ccfe3f8bc3a827e91 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Fri, 1 Nov 2024 16:59:51 -0400 Subject: [PATCH 4/5] `ControllerToAgentFileCallable` also seems helpful --- core/src/main/java/hudson/FilePath.java | 27 ++---------- .../jenkins/MasterToSlaveFileCallable.java | 22 +++------- .../agents/ControllerToAgentFileCallable.java | 43 +++++++++++++++++++ .../security/MasterToSlaveCallable.java | 2 +- 4 files changed, 53 insertions(+), 41 deletions(-) create mode 100644 core/src/main/java/jenkins/agents/ControllerToAgentFileCallable.java diff --git a/core/src/main/java/hudson/FilePath.java b/core/src/main/java/hudson/FilePath.java index f5288b26d9d1..e48d7b45108f 100644 --- a/core/src/main/java/hudson/FilePath.java +++ b/core/src/main/java/hudson/FilePath.java @@ -124,7 +124,7 @@ import java.util.zip.GZIPInputStream; import java.util.zip.GZIPOutputStream; import jenkins.MasterToSlaveFileCallable; -import jenkins.SlaveToMasterFileCallable; +import jenkins.agents.ControllerToAgentFileCallable; import jenkins.model.Jenkins; import jenkins.security.MasterToSlaveCallable; import jenkins.util.ContextResettingExecutorService; @@ -520,21 +520,7 @@ public int archive(final ArchiverFactory factory, OutputStream os, final DirScan return act(new Archive(factory, out, scanner, verificationRoot, openOptions)); } - private static class Archive extends MasterToSlaveFileCallable { - private final ArchiverFactory factory; - private final OutputStream out; - private final DirScanner scanner; - private final String verificationRoot; - private OpenOption[] openOptions; - - Archive(ArchiverFactory factory, OutputStream out, DirScanner scanner, String verificationRoot, OpenOption... openOptions) { - this.factory = factory; - this.out = out; - this.scanner = scanner; - this.verificationRoot = verificationRoot; - this.openOptions = openOptions; - } - + private record Archive(ArchiverFactory factory, OutputStream out, DirScanner scanner, String verificationRoot, OpenOption... openOptions) implements ControllerToAgentFileCallable { @Override public Integer invoke(File f, VirtualChannel channel) throws IOException { try (Archiver a = factory.create(out)) { @@ -542,8 +528,6 @@ public Integer invoke(File f, VirtualChannel channel) throws IOException { return a.countEntries(); } } - - private static final long serialVersionUID = 1L; } public int archive(final ArchiverFactory factory, OutputStream os, final FileFilter filter) throws IOException, InterruptedException { @@ -1185,12 +1169,7 @@ public void copyFrom(org.apache.commons.fileupload.FileItem file) throws IOExcep /** * Code that gets executed on the machine where the {@link FilePath} is local. * Used to act on {@link FilePath}. - * Warning: implementations must be serializable, so prefer a static nested class to an inner class. - * - *

- * Subtypes would likely want to extend from either {@link MasterToSlaveCallable} - * or {@link SlaveToMasterFileCallable}. - * + * A typical implementation would be a {@code record} implementing {@link ControllerToAgentFileCallable}. * @see FilePath#act(FileCallable) */ public interface FileCallable extends Serializable, RoleSensitive { diff --git a/core/src/main/java/jenkins/MasterToSlaveFileCallable.java b/core/src/main/java/jenkins/MasterToSlaveFileCallable.java index 34afd3c11ae5..6296c6d65b28 100644 --- a/core/src/main/java/jenkins/MasterToSlaveFileCallable.java +++ b/core/src/main/java/jenkins/MasterToSlaveFileCallable.java @@ -1,26 +1,16 @@ package jenkins; import hudson.FilePath.FileCallable; -import hudson.remoting.VirtualChannel; -import java.io.File; -import jenkins.security.Roles; -import jenkins.slaves.RemotingVersionInfo; -import org.jenkinsci.remoting.RoleChecker; +import jenkins.agents.ControllerToAgentFileCallable; /** - * {@link FileCallable}s that are meant to be only used on the master. - * - * Note that the logic within {@link #invoke(File, VirtualChannel)} should use API of a minimum supported Remoting version. - * See {@link RemotingVersionInfo#getMinimumSupportedVersion()}. - * + * {@link FileCallable}s that could run on an agent. + * For new code, implement {@link ControllerToAgentFileCallable} + * which has the advantage that it can be used on {@code record}s. * @since 1.587 / 1.580.1 - * @param the return type; note that this must either be defined in your plugin or included in the stock JEP-200 whitelist + * @param the return type */ -public abstract class MasterToSlaveFileCallable implements FileCallable { - @Override - public void checkRoles(RoleChecker checker) throws SecurityException { - checker.check(this, Roles.SLAVE); - } +public abstract class MasterToSlaveFileCallable implements ControllerToAgentFileCallable { private static final long serialVersionUID = 1L; } diff --git a/core/src/main/java/jenkins/agents/ControllerToAgentFileCallable.java b/core/src/main/java/jenkins/agents/ControllerToAgentFileCallable.java new file mode 100644 index 000000000000..22904e0c2e43 --- /dev/null +++ b/core/src/main/java/jenkins/agents/ControllerToAgentFileCallable.java @@ -0,0 +1,43 @@ +/* + * The MIT License + * + * Copyright 2024 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 jenkins.agents; + +import hudson.FilePath; +import jenkins.security.Roles; +import org.jenkinsci.remoting.RoleChecker; + +/** + * {@link FilePath.FileCallable} meant to be serialized then run on an agent. + * Like {@link ControllerToAgentCallable} this will typically be a {@link Record}. + * @param the return type; note that this must either be defined in your plugin or included in the stock JEP-200 whitelist + * @since TODO + */ +public interface ControllerToAgentFileCallable extends FilePath.FileCallable { + + @Override + default void checkRoles(RoleChecker checker) throws SecurityException { + checker.check(this, Roles.SLAVE); + } +} diff --git a/core/src/main/java/jenkins/security/MasterToSlaveCallable.java b/core/src/main/java/jenkins/security/MasterToSlaveCallable.java index 1f6fe3b246a4..7d3830bb9562 100644 --- a/core/src/main/java/jenkins/security/MasterToSlaveCallable.java +++ b/core/src/main/java/jenkins/security/MasterToSlaveCallable.java @@ -9,7 +9,7 @@ * which has the advantage that it can be used on {@code record}s. * @author Kohsuke Kawaguchi * @since 1.587 / 1.580.1 - * @param the return type; note that this must either be defined in your plugin or included in the stock JEP-200 whitelist + * @param the return type */ public abstract class MasterToSlaveCallable implements ControllerToAgentCallable { From f073fcb2b9aea6f17e73333bf24b95e6798e0609 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Mon, 4 Nov 2024 13:18:16 -0500 Subject: [PATCH 5/5] Stray space Co-authored-by: Vincent Latombe --- .../src/main/java/jenkins/agents/ControllerToAgentCallable.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/jenkins/agents/ControllerToAgentCallable.java b/core/src/main/java/jenkins/agents/ControllerToAgentCallable.java index 9b0dc2576190..b724da77f6a2 100644 --- a/core/src/main/java/jenkins/agents/ControllerToAgentCallable.java +++ b/core/src/main/java/jenkins/agents/ControllerToAgentCallable.java @@ -39,7 +39,7 @@ * @param the return type; note that this must either be defined in your plugin or included in the stock JEP-200 whitelist * @since TODO */ -public interface ControllerToAgentCallable extends Callable { +public interface ControllerToAgentCallable extends Callable { @Override default void checkRoles(RoleChecker checker) throws SecurityException {