Simplifying JNLPLauncher (#8762)

jenkinsci · Dec 13, 2023 · 6a2d94b · 6a2d94b
1 parent 05037a0
commit 6a2d94b
Show file tree

Hide file tree

Showing 19 changed files with 102 additions and 135 deletions.
diff --git a/core/src/main/java/hudson/model/Slave.java b/core/src/main/java/hudson/model/Slave.java
@@ -245,8 +245,7 @@ public ComputerLauncher getLauncher() {
                 LOGGER.log(Level.WARNING, "could not update historical agentCommand setting to CommandLauncher", x);
             }
         }
-        // Default launcher does not use Work Directory
-        return launcher == null ? new JNLPLauncher(false) : launcher;
+        return launcher == null ? new JNLPLauncher() : launcher;
     }
 
     public void setLauncher(ComputerLauncher launcher) {

diff --git a/core/src/main/java/hudson/slaves/JNLPLauncher.java b/core/src/main/java/hudson/slaves/JNLPLauncher.java
@@ -34,7 +34,6 @@
 import hudson.model.Computer;
 import hudson.model.Descriptor;
 import hudson.model.TaskListener;
-import hudson.util.FormValidation;
 import jenkins.model.Jenkins;
 import jenkins.model.identity.InstanceIdentityProvider;
 import jenkins.slaves.RemotingWorkDirSettings;
@@ -43,10 +42,10 @@
 import org.apache.commons.lang.StringUtils;
 import org.jenkinsci.Symbol;
 import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.DoNotUse;
 import org.kohsuke.accmod.restrictions.NoExternalUse;
 import org.kohsuke.stapler.DataBoundConstructor;
 import org.kohsuke.stapler.DataBoundSetter;
-import org.kohsuke.stapler.QueryParameter;
 
 /**
  * {@link ComputerLauncher} via inbound connections.
@@ -56,28 +55,23 @@
 */
 public class JNLPLauncher extends ComputerLauncher {
     /**
-     * If the agent needs to tunnel the connection to the controller,
-     * specify the "host:port" here. This can include the special
-     * syntax "host:" and ":port" to indicate the default host/port
-     * shall be used.
-     *
-     * <p>
-     * Null if no tunneling is necessary.
-     *
-     * @since 1.250
+     * Deprecated (only used with deprecated {@code -jnlpUrl} mode), but cannot mark it as such without breaking CasC.
      */
+    @DataBoundSetter
     @CheckForNull
-    public final String tunnel;
+    public String tunnel;
 
     /**
      * @deprecated No longer used.
      */
     @Deprecated
     public final transient String vmargs = null;
 
+    @Deprecated
     @NonNull
     private RemotingWorkDirSettings workDirSettings = RemotingWorkDirSettings.getEnabledDefaults();
 
+    @Deprecated
     private boolean webSocket;
 
     /**
@@ -88,14 +82,7 @@ public class JNLPLauncher extends ComputerLauncher {
     public static final String CUSTOM_INBOUND_URL_PROPERTY = "jenkins.agent.inboundUrl";
 
     /**
-     * Constructor.
-     * @param tunnel Tunnel settings
-     * @param vmargs JVM arguments
-     * @param workDirSettings Settings for Work Directory management in Remoting.
-     *                        If {@code null}, {@link RemotingWorkDirSettings#getEnabledDefaults()}
-     *                        will be used to enable work directories by default in new agents.
-     * @since 2.68
-     * @deprecated use {@link #JNLPLauncher(String, String)} and {@link #setWorkDirSettings(RemotingWorkDirSettings)}
+     * @deprecated no useful properties, use {@link #JNLPLauncher()}
      */
     @Deprecated
     public JNLPLauncher(@CheckForNull String tunnel, @CheckForNull String vmargs, @CheckForNull RemotingWorkDirSettings workDirSettings) {
@@ -105,36 +92,30 @@ public JNLPLauncher(@CheckForNull String tunnel, @CheckForNull String vmargs, @C
         }
     }
 
-    // TODO cannot easily make tunnel into a @DataBoundSetter because then the @DataBoundConstructor would be on a no-arg constructor
-    // which is already defined and deprecated. Could retroactively let no-arg constructor use default for workDirSettings,
-    // which would be a behavioral change only for callers of the Java constructor (unlikely).
-    @DataBoundConstructor
+    /**
+     * @deprecated no useful properties, use {@link #JNLPLauncher()}
+     */
+    @Deprecated
     public JNLPLauncher(@CheckForNull String tunnel) {
         this.tunnel = Util.fixEmptyAndTrim(tunnel);
     }
 
     /**
-     * @deprecated use {@link JNLPLauncher#JNLPLauncher(String)}
+     * @deprecated no useful properties, use {@link #JNLPLauncher()}
      */
     @Deprecated
     public JNLPLauncher(@CheckForNull String tunnel, @CheckForNull String vmargs) {
         this.tunnel = Util.fixEmptyAndTrim(tunnel);
     }
 
-    /**
-     * @deprecated This Launcher does not enable the work directory.
-     *             It is recommended to use {@link #JNLPLauncher(boolean)}
-     */
-    @Deprecated
+    @DataBoundConstructor
     public JNLPLauncher() {
-        this(false);
     }
 
     /**
-     * Constructor with default options.
-     *
-     * @param enableWorkDir If {@code true}, the work directory will be enabled with default settings.
+     * @deprecated no useful properties, use {@link #JNLPLauncher()}
      */
+    @Deprecated
     public JNLPLauncher(boolean enableWorkDir) {
         this(null, null, enableWorkDir
                 ? RemotingWorkDirSettings.getEnabledDefaults()
@@ -150,16 +131,14 @@ protected Object readResolve() {
         return this;
     }
 
-    /**
-     * Returns work directory settings.
-     *
-     * @since 2.72
-     */
-    @NonNull
+    @Deprecated
     public RemotingWorkDirSettings getWorkDirSettings() {
         return workDirSettings;
     }
 
+    /**
+     * Deprecated (only used with deprecated {@code -jnlpUrl} mode), but cannot mark it as such without breaking CasC.
+     */
     @DataBoundSetter
     public final void setWorkDirSettings(@NonNull RemotingWorkDirSettings workDirSettings) {
         this.workDirSettings = workDirSettings;
@@ -170,15 +149,13 @@ public boolean isLaunchSupported() {
         return false;
     }
 
-    /**
-     * @since 2.216
-     */
+    @Deprecated
     public boolean isWebSocket() {
         return webSocket;
     }
 
     /**
-     * @since 2.216
+     * Deprecated (only used with deprecated {@code -jnlpUrl} mode), but cannot mark it as such without breaking CasC.
      */
     @DataBoundSetter
     public void setWebSocket(boolean webSocket) {
@@ -210,6 +187,11 @@ public String getRemotingOptionsWindows(@NonNull Computer computer) {
         return getRemotingOptions(escapeWindows(computer.getName()));
     }
 
+    @Restricted(DoNotUse.class)
+    public boolean isConfigured() {
+        return webSocket || tunnel != null || workDirSettings.isConfigured();
+    }
+
     private String getRemotingOptions(String computerName) {
         StringBuilder sb = new StringBuilder();
         sb.append("-name ");
@@ -296,23 +278,19 @@ public boolean isWorkDirSupported() {
             return DescriptorImpl.class.equals(getClass());
         }
 
-        public FormValidation doCheckWebSocket(@QueryParameter boolean webSocket, @QueryParameter String tunnel) {
-            if (webSocket) {
-                if (!WebSockets.isSupported()) {
-                    return FormValidation.error(Messages.JNLPLauncher_WebsocketNotEnabled());
-                }
-                if (Util.fixEmptyAndTrim(tunnel) != null) {
-                    return FormValidation.error(Messages.JNLPLauncher_TunnelingNotSupported());
-                }
-            } else {
-                if (Jenkins.get().getTcpSlaveAgentListener() == null) {
-                    return FormValidation.error(Messages.JNLPLauncher_TCPPortDisabled());
-                }
-                if (InstanceIdentityProvider.RSA.getCertificate() == null || InstanceIdentityProvider.RSA.getPrivateKey() == null) {
-                    return FormValidation.error(Messages.JNLPLauncher_InstanceIdentityRequired());
-                }
-            }
-            return FormValidation.ok();
+        @Restricted(DoNotUse.class)
+        public boolean isTcpSupported() {
+            return Jenkins.get().getTcpSlaveAgentListener() != null;
+        }
+
+        @Restricted(DoNotUse.class)
+        public boolean isInstanceIdentityInstalled() {
+            return InstanceIdentityProvider.RSA.getCertificate() != null && InstanceIdentityProvider.RSA.getPrivateKey() != null;
+        }
+
+        @Restricted(DoNotUse.class)
+        public boolean isWebSocketSupported() {
+            return WebSockets.isSupported();
         }
 
     }

diff --git a/core/src/main/java/jenkins/slaves/RemotingWorkDirSettings.java b/core/src/main/java/jenkins/slaves/RemotingWorkDirSettings.java
@@ -41,14 +41,9 @@
 import org.kohsuke.stapler.DataBoundConstructor;
 
 /**
- * Defines settings of the Remoting work directory.
- *
- * This class contains Remoting Work Directory settings, which can be used when starting Jenkins agents.
- * See <a href="https://github.com/jenkinsci/remoting/blob/master/docs/workDir.md">Remoting Work Dir Documentation</a>.
- *
- * @author Oleg Nenashev
- * @since 2.72
+ * @deprecated only used with deprecated {@code -jnlpUrl} mode
  */
+@Deprecated
 public class RemotingWorkDirSettings implements Describable<RemotingWorkDirSettings> {
 
     private static final String DEFAULT_INTERNAL_DIR = "remoting";
@@ -79,6 +74,12 @@ public RemotingWorkDirSettings() {
         this(false, null, DEFAULT_INTERNAL_DIR, false);
     }
 
+    /** if this is not {@link #ENABLED_DEFAULT} */
+    @Restricted(NoExternalUse.class)
+    public boolean isConfigured() {
+        return disabled || workDirPath != null || !DEFAULT_INTERNAL_DIR.equals(internalDir) || failIfWorkDirIsMissing;
+    }
+
     /**
      * Check if workdir is disabled.
      *

diff --git a/core/src/main/resources/hudson/slaves/JNLPLauncher/config.jelly b/core/src/main/resources/hudson/slaves/JNLPLauncher/config.jelly
@@ -24,6 +24,10 @@ THE SOFTWARE.
 
 <?jelly escape-by-default='true'?>
 <j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:d="jelly:define" xmlns:l="/lib/layout" xmlns:t="/lib/hudson" xmlns:f="/lib/form">
+ <j:if test="${it.launcher.configured}">
+  <f:block>
+    ${%configured}
+  </f:block>
   <!-- This property is included only if used directly. Causes JENKINS-45895 in the case of includes otherwise -->
   <j:if test="${descriptor.workDirSupported}"> 
     <f:property title="${%Enable work directory}" field="workDirSettings" />
@@ -36,4 +40,5 @@ THE SOFTWARE.
       <f:textbox field="tunnel"/>
     </f:entry>
   </f:advanced>
-</j:jelly>
+ </j:if>
+</j:jelly>
diff --git a/core/src/main/resources/hudson/slaves/JNLPLauncher/config.properties b/core/src/main/resources/hudson/slaves/JNLPLauncher/config.properties
@@ -0,0 +1,4 @@
+configured=\
+  Configuring any properties on the inbound agent launcher is deprecated \
+  as it would only have an effect in deprecated <code>-jnlpUrl</code> mode. \
+  Instead pass any desired options to the agent yourself.
diff --git a/core/src/main/resources/hudson/slaves/JNLPLauncher/main.jelly b/core/src/main/resources/hudson/slaves/JNLPLauncher/main.jelly
@@ -25,41 +25,13 @@ THE SOFTWARE.
 <?jelly escape-by-default='true'?>
 <j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:d="jelly:define" xmlns:l="/lib/layout" xmlns:t="/lib/hudson" xmlns:f="/lib/form" xmlns:i="jelly:fmt">
   <j:choose>
-    <j:when test="${!it.launcher.webSocket and app.slaveAgentPort == -1}">
-      <div class="error">
-        ${%slaveAgentPort.disabled}
-        <l:isAdmin><a href="${rootURL}/configureSecurity">${%configure.link.text}</a>.</l:isAdmin>
-      </div>
-    </j:when>
     <j:when test="${it.offline and !it.temporarilyOffline}">
       <j:set var="jenkinsURL" value="${h.inferHudsonURL(request)}"/>
       <j:set var="copy_agent_jar_unix" value="curl -sO ${jenkinsURL}jnlpJars/agent.jar" />
       <j:set var="copy_agent_jar_windows" value="curl.exe -sO ${jenkinsURL}jnlpJars/agent.jar" />
       <j:set var="copy_java_cmd_unix" value="java -jar agent.jar -url ${jenkinsURL} ${it.launcher.getRemotingOptionsUnix(it)}${it.launcher.getWorkDirOptions(it)}" />
       <j:set var="copy_java_cmd_windows" value="java -jar agent.jar -url ${jenkinsURL} ${it.launcher.getRemotingOptionsWindows(it)}${it.launcher.getWorkDirOptions(it)}" />
       <j:if test="${h.hasPermission(it, it.CONNECT)}">
-          <j:choose>
-            <j:when test="${it.ACL.hasPermission(app.ANONYMOUS, it.CONNECT)}">
-                <h3>
-                  ${%slaveAgent.cli.run} (Unix)
-                  <l:copyButton text="${copy_agent_jar_unix};${copy_java_cmd_unix}"/>
-                </h3>
-<pre>
-${copy_agent_jar_unix}
-${copy_java_cmd_unix}
-</pre>
-
-                <h3>
-                  ${%slaveAgent.cli.run} (Windows)
-                  <l:copyButton text="${copy_agent_jar_windows} &amp; ${copy_java_cmd_windows}"/>
-                </h3>
-<pre>
-${copy_agent_jar_windows}
-${copy_java_cmd_windows}
-</pre>
-
-            </j:when>
-            <j:otherwise>
                 <j:set var="copy_java_cmd_secret_unix" value="java -jar agent.jar -url ${jenkinsURL} -secret ${it.jnlpMac} ${it.launcher.getRemotingOptionsUnix(it)}${it.launcher.getWorkDirOptions(it)}" />
                 <j:set var="copy_java_cmd_secret_windows" value="java -jar agent.jar -url ${jenkinsURL} -secret ${it.jnlpMac} ${it.launcher.getRemotingOptionsWindows(it)}${it.launcher.getWorkDirOptions(it)}" />
                 <h3>
@@ -104,8 +76,28 @@ ${copy_secret_to_file}
 ${copy_agent_jar_windows}
 ${copy_java_cmd_secret2_windows}
 </pre>
-            </j:otherwise>
-          </j:choose>
+          <j:if test="${!it.launcher.configured}">
+              <p>
+                  ${%commonOptions}
+              </p>
+          </j:if>
+          <j:if test="${!it.launcher.descriptor.tcpSupported}">
+              <p>
+                  ${%tcp-port-disabled}
+                  <l:isAdmin><a href="${rootURL}/manage/configureSecurity/">${%configure.link.text}</a>.</l:isAdmin>
+              </p>
+          </j:if>
+          <j:if test="${!it.launcher.descriptor.instanceIdentityInstalled}">
+              <p>
+                  ${%instance-identity-missing}
+                  <l:isAdmin><a href="${rootURL}/manage/pluginManager/available">${%install-instance-identity}</a>.</l:isAdmin>
+              </p>
+          </j:if>
+          <j:if test="${!it.launcher.descriptor.webSocketSupported}">
+              <p>
+                  ${%web-socket-unsupported}
+              </p>
+          </j:if>
           <p>
             ${%powerShell.cli.curl}
           </p>

diff --git a/core/src/main/resources/hudson/slaves/JNLPLauncher/main.properties b/core/src/main/resources/hudson/slaves/JNLPLauncher/main.properties
@@ -20,9 +20,22 @@
 # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
 # THE SOFTWARE.
 
-configure.link.text=Go to security configuration screen and change it
-slaveAgentPort.disabled=JNLP agent port is disabled and agents cannot connect this way.
+configure.link.text=Go to the security configuration screen and change it
 slaveAgent.connected=Agent is connected.
 slaveAgent.cli.run=Run from agent command line:
 slaveAgent.cli.run.secret=Or run from agent command line, with the secret stored in a file:
-powerShell.cli.curl=Note: PowerShell users must use curl.exe instead of curl because curl is a default PowerShell cmdlet alias for Invoke-WebRequest.
+powerShell.cli.curl=Note: PowerShell users must use curl.exe instead of curl because curl is a default PowerShell cmdlet alias for Invoke-WebRequest.
+commonOptions=\
+  The most commonly used option is <code>-webSocket</code>. \
+  Run <code>java -jar agent.jar -help</code> for more.
+tcp-port-disabled=\
+  The TCP port is disabled so TCP agents may not be connected. \
+  You may still use WebSocket agents.
+instance-identity-missing=\
+  The <code>instance-identity</code> plugin appears to be missing. \
+  This would prevent you from connecting a TCP agent. \
+  You may still use WebSocket agents.
+install-instance-identity=Install it now
+web-socket-unsupported=\
+  WebSockets are not supported in this Jenkins installation. \
+  You may still use TCP agents.
diff --git a/core/src/main/resources/hudson/slaves/JNLPLauncher/main_bg.properties b/core/src/main/resources/hudson/slaves/JNLPLauncher/main_bg.properties
@@ -28,8 +28,6 @@ Run\ from\ agent\ command\ line\:=\
  Стартиране на агента от командния ред
 launch\ agent=\
  стартиране на агента
-slaveAgentPort.disabled=\
- Портът на агента е изключен
 Launch\ agent\ from\ browser=\
  Стартиране на агент от браузър
 Connected\ via\ JNLP\ agent.=\

diff --git a/core/src/main/resources/hudson/slaves/JNLPLauncher/main_de.properties b/core/src/main/resources/hudson/slaves/JNLPLauncher/main_de.properties
@@ -23,7 +23,6 @@
 configure.link.text=Zur globalen Sicherheitskonfiguration wechseln und ändern
 launch\ agent=Agent starten
 Connected\ via\ JNLP\ agent.=Verbunden über JNLP-Agent.
-slaveAgentPort.disabled=JNLP=Agenten können nicht verbinden, da der JNLP-Port deaktiviert ist.
 Launch\ agent\ from\ browser=JNLP-Agent via Webbrowser starten
 Connect\ agent\ to\ Jenkins\ one\ of\ these\ ways\:=Möglichkeiten, den Agent mit Jenkins zu verbinden:
 Run\ from\ agent\ command\ line\:=Auf der Befehlszeile ausführen:

diff --git a/core/src/main/resources/hudson/slaves/JNLPLauncher/main_es.properties b/core/src/main/resources/hudson/slaves/JNLPLauncher/main_es.properties
@@ -20,7 +20,6 @@
 # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
 # THE SOFTWARE.
 
-slaveAgentPort.disabled=El puerto TCP para los agentes via JNLP está deshabilitado.
 launch\ agent=Lanzar agente
 Or\ if\ the\ agent\ is\ headless\:=O si el agente no tiene pantalla
 Run\ from\ agent\ command\ line\:=Ejecutar agente desde la línea de comandos del nodo

diff --git a/core/src/main/resources/hudson/slaves/JNLPLauncher/main_it.properties b/core/src/main/resources/hudson/slaves/JNLPLauncher/main_it.properties
@@ -32,5 +32,3 @@ Or\ if\ the\ agent\ is\ headless\:=O se l''agente è senza interfaccia grafica:
 Run\ from\ agent\ command\ line\:=Esegui dalla riga di comando dell''agente
 Run\ from\ agent\ command\ line,\ with\ the\ secret\ stored\ in\ a\ file\:=\
   Esegui dalla riga di comando dell''agente con il segreto salvato in un file
-slaveAgentPort.disabled=La porta agente JNLP è disabilitata e gli agenti non \
-  possono connettersi in questa modalità.