Removing configurability of Jenkins.agentProtocols (#9903)

core/src/main/java/hudson/TcpSlaveAgentListener.java

@@ -271,14 +271,11 @@ public void run() {
                     String protocol = s.substring(9);
                     AgentProtocol p = AgentProtocol.of(protocol);
                     if (p != null) {
-                        if (Jenkins.get().getAgentProtocols().contains(protocol)) {
-                            LOGGER.log(p instanceof PingAgentProtocol ? Level.FINE : Level.INFO, () -> "Accepted " + protocol + " connection " + connectionInfo);
-                            p.handle(this.s);
-                        } else {
-                            error("Disabled protocol:" + s, this.s);
-                        }
-                    } else
+                        LOGGER.log(p instanceof PingAgentProtocol ? Level.FINE : Level.INFO, () -> "Accepted " + protocol + " connection " + connectionInfo);
+                        p.handle(this.s);
+                    } else {
                         error("Unknown protocol:", this.s);
+                    }
                 } else {
                     error("Unrecognized protocol: " + s, this.s);
                 }
@@ -364,21 +361,11 @@ public PingAgentProtocol() {
             ping = "Ping\n".getBytes(StandardCharsets.UTF_8);
         }
 
-        @Override
-        public boolean isRequired() {
-            return true;
-        }
-
         @Override
         public String getName() {
             return "Ping";
         }
 
-        @Override
-        public String getDisplayName() {
-            return Messages.TcpSlaveAgentListener_PingAgentProtocol_displayName();
-        }
-
         @Override
         public void handle(Socket socket) throws IOException, InterruptedException {
             try (socket) {

core/src/main/java/hudson/security/GlobalSecurityConfiguration.java

@@ -37,15 +37,12 @@
 import hudson.util.FormApply;
 import jakarta.servlet.ServletException;
 import java.io.IOException;
-import java.util.Set;
-import java.util.TreeSet;
 import java.util.function.Predicate;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import jenkins.model.GlobalConfigurationCategory;
 import jenkins.model.Jenkins;
 import jenkins.util.ServerTcpPort;
-import net.sf.json.JSONArray;
 import net.sf.json.JSONException;
 import net.sf.json.JSONObject;
 import org.jenkinsci.Symbol;
@@ -92,11 +89,6 @@ public boolean isSlaveAgentPortEnforced() {
         return Jenkins.get().isSlaveAgentPortEnforced();
     }
 
-    @NonNull
-    public Set<String> getAgentProtocols() {
-        return Jenkins.get().getAgentProtocols();
-    }
-
     public boolean isDisableRememberMe() {
         return Jenkins.get().isDisableRememberMe();
     }
@@ -149,18 +141,6 @@ public boolean configure(StaplerRequest2 req, JSONObject json) throws FormExcept
                 throw new FormException(e, "slaveAgentPortType");
             }
         }
-        Set<String> agentProtocols = new TreeSet<>();
-        if (json.has("agentProtocol")) {
-            Object protocols = json.get("agentProtocol");
-            if (protocols instanceof JSONArray) {
-                for (int i = 0; i < ((JSONArray) protocols).size(); i++) {
-                    agentProtocols.add(((JSONArray) protocols).getString(i));
-                }
-            } else {
-                agentProtocols.add(protocols.toString());
-            }
-        }
-        j.setAgentProtocols(agentProtocols);
 
         // persist all the additional security configs
         boolean result = true;

core/src/main/java/jenkins/AgentProtocol.java

@@ -7,8 +7,6 @@
 import hudson.TcpSlaveAgentListener;
 import java.io.IOException;
 import java.net.Socket;
-import java.util.Set;
-import jenkins.model.Jenkins;
 
 /**
  * Pluggable Jenkins TCP agent protocol handler called from {@link TcpSlaveAgentListener}.
@@ -18,57 +16,31 @@
  * Implementations of this extension point is singleton, and its {@link #handle(Socket)} method
  * gets invoked concurrently whenever a new connection comes in.
  *
- * <h2>Extending UI</h2>
- * <dl>
- *  <dt>description.jelly</dt>
- *  <dd>Optional protocol description</dd>
- *  <dt>deprecationCause.jelly</dt>
- *  <dd>Optional. If the protocol is marked as {@link #isDeprecated()},
- *      clarifies the deprecation reason and provides extra documentation links</dd>
- * </dl>
- *
  * @author Kohsuke Kawaguchi
  * @since 1.467
  * @see TcpSlaveAgentListener
  */
 public abstract class AgentProtocol implements ExtensionPoint {
     /**
-     * Allow experimental {@link AgentProtocol} implementations to declare being opt-in.
-     * Note that {@link Jenkins#setAgentProtocols(Set)} only records the protocols where the admin has made a
-     * conscious decision thus:
-     * <ul>
-     *     <li>if a protocol is opt-in, it records the admin enabling it</li>
-     *     <li>if a protocol is opt-out, it records the admin disabling it</li>
-     * </ul>
-     * Implementations should not transition rapidly from {@code opt-in -> opt-out -> opt-in}.
-     * Implementations should never flip-flop: {@code opt-in -> opt-out -> opt-in -> opt-out} as that will basically
-     * clear any preference that an admin has set. This latter restriction should be ok as we only ever will be
-     * adding new protocols and retiring old ones.
-     *
-     * @return {@code true} if the protocol requires explicit opt-in.
-     * @since 2.16
-     * @see Jenkins#setAgentProtocols(Set)
+     * @deprecated no longer used
      */
+    @Deprecated
     public boolean isOptIn() {
         return false;
     }
+
     /**
-     * Allow essential {@link AgentProtocol} implementations (basically {@link TcpSlaveAgentListener.PingAgentProtocol})
-     * to be always enabled.
-     *
-     * @return {@code true} if the protocol can never be disabled.
-     * @since 2.16
+     * @deprecated no longer used
      */
-
+    @Deprecated
     public boolean isRequired() {
         return false;
     }
 
     /**
-     * Checks if the protocol is deprecated.
-     *
-     * @since 2.75
+     * @deprecated no longer used
      */
+    @Deprecated
     public boolean isDeprecated() {
         return false;
     }
@@ -79,17 +51,15 @@ public boolean isDeprecated() {
      * This is a short string that consists of printable ASCII chars. Sent by the client to select the protocol.
      *
      * @return
-     *      null to be disabled. This is useful for avoiding getting used
-     *      until the protocol is properly configured.
+     *      null to be disabled
      */
+    @CheckForNull
     public abstract String getName();
 
     /**
-     * Returns the human readable protocol display name.
-     *
-     * @return the human readable protocol display name.
-     * @since 2.16
+     * @deprecated no longer used
      */
+    @Deprecated
     public String getDisplayName() {
         return getName();
     }

core/src/main/java/jenkins/model/Jenkins.java

@@ -656,47 +656,6 @@ private static int getSlaveAgentPortInitialValue(int def) {
      */
     private static final boolean SLAVE_AGENT_PORT_ENFORCE = SystemProperties.getBoolean(Jenkins.class.getName() + ".slaveAgentPortEnforce", false);
 
-    /**
-     * The TCP agent protocols that are explicitly disabled (we store the disabled ones so that newer protocols
-     * are enabled by default). Will be {@code null} instead of empty to simplify XML format.
-     *
-     * @since 2.16
-     */
-    @CheckForNull
-    @GuardedBy("this")
-    private List<String> disabledAgentProtocols;
-    /**
-     * @deprecated Just a temporary buffer for XSTream migration code from JENKINS-39465, do not use
-     */
-    @Deprecated
-    private transient String[] _disabledAgentProtocols;
-
-    /**
-     * The TCP agent protocols that are {@link AgentProtocol#isOptIn()} and explicitly enabled.
-     * Will be {@code null} instead of empty to simplify XML format.
-     *
-     * @since 2.16
-     */
-    @CheckForNull
-    @GuardedBy("this")
-    private List<String> enabledAgentProtocols;
-    /**
-     * @deprecated Just a temporary buffer for XSTream migration code from JENKINS-39465, do not use
-     */
-    @Deprecated
-    private transient String[] _enabledAgentProtocols;
-
-    /**
-     * The TCP agent protocols that are enabled. Built from {@link #disabledAgentProtocols} and
-     * {@link #enabledAgentProtocols}.
-     *
-     * @since 2.16
-     * @see #setAgentProtocols(Set)
-     * @see #getAgentProtocols()
-     */
-    @GuardedBy("this")
-    private transient Set<String> agentProtocols;
-
     /**
      * Whitespace-separated labels assigned to the built-in node as a {@link Node}.
      */
@@ -1096,18 +1055,6 @@ protected Object readResolve() {
         if (SLAVE_AGENT_PORT_ENFORCE) {
             slaveAgentPort = getSlaveAgentPortInitialValue(slaveAgentPort);
         }
-        synchronized (this) {
-            if (disabledAgentProtocols == null && _disabledAgentProtocols != null) {
-                disabledAgentProtocols = Arrays.asList(_disabledAgentProtocols);
-                _disabledAgentProtocols = null;
-            }
-            if (enabledAgentProtocols == null && _enabledAgentProtocols != null) {
-                enabledAgentProtocols = Arrays.asList(_enabledAgentProtocols);
-                _enabledAgentProtocols = null;
-            }
-            // Invalidate the protocols cache after the reload
-            agentProtocols = null;
-        }
 
         // no longer persisted
         installStateName = null;
@@ -1282,81 +1229,15 @@ private void forceSetSlaveAgentPort(int port) throws IOException {
      */
     @NonNull
     public synchronized Set<String> getAgentProtocols() {
-        if (agentProtocols == null) {
-            Set<String> result = new TreeSet<>();
-            Set<String> disabled = new TreeSet<>();
-            for (String p : Util.fixNull(disabledAgentProtocols)) {
-                disabled.add(p.trim());
-            }
-            Set<String> enabled = new TreeSet<>();
-            for (String p : Util.fixNull(enabledAgentProtocols)) {
-                enabled.add(p.trim());
-            }
-            for (AgentProtocol p : AgentProtocol.all()) {
-                String name = p.getName();
-                if (name != null && (p.isRequired()
-                        || (!disabled.contains(name) && (!p.isOptIn() || enabled.contains(name))))) {
-                    result.add(name);
-                }
-            }
-            /*
-             * An empty result is almost never valid, but it can happen due to JENKINS-70206. Since we know the result
-             * is likely incorrect, at least decline to cache it so that a correct result can be computed later on
-             * rather than continuing to deliver the incorrect result indefinitely.
-             */
-            if (!result.isEmpty()) {
-                agentProtocols = result;
-            }
-            return result;
-        }
-        return agentProtocols;
+        return AgentProtocol.all().stream().map(AgentProtocol::getName).filter(Objects::nonNull).collect(Collectors.toCollection(TreeSet::new));
     }
 
     /**
-     * Sets the enabled agent protocols.
-     *
-     * @param protocols the enabled agent protocols.
-     * @since 2.16
+     * @deprecated No longer does anything.
      */
+    @Deprecated
     public synchronized void setAgentProtocols(@NonNull Set<String> protocols) {
-        Set<String> disabled = new TreeSet<>();
-        Set<String> enabled = new TreeSet<>();
-        for (AgentProtocol p : AgentProtocol.all()) {
-            String name = p.getName();
-            if (name != null && !p.isRequired()) {
-                // we want to record the protocols where the admin has made a conscious decision
-                // thus, if a protocol is opt-in, we record the admin enabling it
-                // if a protocol is opt-out, we record the admin disabling it
-                // We should not transition rapidly from opt-in -> opt-out -> opt-in
-                // the scenario we want to have work is:
-                // 1. We introduce a new protocol, it starts off as opt-in. Some admins decide to test and opt-in
-                // 2. We decide that the protocol is ready for general use. It gets marked as opt-out. Any admins
-                //    that took part in early testing now have their config switched to not mention the new protocol
-                //    at all when they save their config as the protocol is now opt-out. Any admins that want to
-                //    disable it can do so and will have their preference recorded.
-                // 3. We decide that the protocol needs to be retired. It gets switched back to opt-in. At this point
-                //    the initial opt-in admins, assuming they visited an upgrade to a controller with step 2, will
-                //    have the protocol disabled for them. This is what we want. If they didn't upgrade to a controller
-                //    with step 2, well there is not much we can do to differentiate them from somebody who is upgrading
-                //    from a previous step 3 controller and had needed to keep the protocol turned on.
-                //
-                // What we should never do is flip-flop: opt-in -> opt-out -> opt-in -> opt-out as that will basically
-                // clear any preference that an admin has set, but this should be ok as we only ever will be
-                // adding new protocols and retiring old ones.
-                if (p.isOptIn()) {
-                    if (protocols.contains(name)) {
-                        enabled.add(name);
-                    }
-                } else {
-                    if (!protocols.contains(name)) {
-                        disabled.add(name);
-                    }
-                }
-            }
-        }
-        disabledAgentProtocols = disabled.isEmpty() ? null : new ArrayList<>(disabled);
-        enabledAgentProtocols = enabled.isEmpty() ? null : new ArrayList<>(enabled);
-        agentProtocols = null;
+        LOGGER.log(Level.WARNING, null, new IllegalStateException("Jenkins.agentProtocols no longer configurable"));
     }
 
     private void launchTcpSlaveAgentListener() throws IOException {
@@ -6000,8 +5881,6 @@ public boolean shouldShowStackTrace() {
             // for backward compatibility with <1.75, recognize the tag name "view" as well.
             XSTREAM.alias("view", ListView.class);
             XSTREAM.alias("listView", ListView.class);
-            XSTREAM.addImplicitArray(Jenkins.class, "_disabledAgentProtocols", "disabledAgentProtocol");
-            XSTREAM.addImplicitArray(Jenkins.class, "_enabledAgentProtocols", "enabledAgentProtocol");
             XSTREAM2.addCriticalField(Jenkins.class, "securityRealm");
             XSTREAM2.addCriticalField(Jenkins.class, "authorizationStrategy");
             // this seems to be necessary to force registration of converter early enough

core/src/main/java/jenkins/slaves/JnlpSlaveAgentProtocol4.java

@@ -137,16 +137,6 @@ private char[] constructPassword() {
         return "password".toCharArray();
     }
 
-    @Override
-    public boolean isOptIn() {
-        return false;
-    }
-
-    @Override
-    public String getDisplayName() {
-        return Messages.JnlpSlaveAgentProtocol4_displayName();
-    }
-
     @Override
     public String getName() {
         return "JNLP4-connect"; // matches JnlpProtocol4Handler.getName

core/src/main/resources/hudson/Messages.properties

@@ -127,7 +127,6 @@ PluginWrapper.Plugin.Has.Dependent=The plugin ''{0}'' has, at least, one depende
 PluginWrapper.Plugin.Disabled=Plugin ''{0}'' disabled
 PluginWrapper.NoSuchPlugin=No such plugin found with the name ''{0}''
 PluginWrapper.Error.Disabling=There was an error disabling the ''{0}'' plugin. Error: ''{1}''
-TcpSlaveAgentListener.PingAgentProtocol.displayName=Ping protocol
 
 ProxyConfigurationManager.DisplayName=Proxy Configuration
 ProxyConfigurationManager.Description=Configure the http proxy used by Jenkins

core/src/main/resources/hudson/Messages_bg.properties

@@ -105,9 +105,6 @@ PluginWrapper.disabledAndObsolete=\
 # {0} is disabled. To fix, enable it.
 PluginWrapper.disabled=\
  „{0}“ е изключена. Включете я.
-# Ping protocol
-TcpSlaveAgentListener.PingAgentProtocol.displayName=\
- Протокол „ping“
 # {0} v{1} failed to load. Fix this plugin first.
 PluginWrapper.failed_to_load_dependency=\
  „{0}“, версия {1} не се зареди. Оправете приставката.

core/src/main/resources/hudson/Messages_de.properties

@@ -75,5 +75,3 @@ AboutJenkins.DisplayName=Über Jenkins
 AboutJenkins.Description=Versions- und Lizenzinformationen anzeigen.
 
 Functions.NoExceptionDetails=Keine Details zum Ausnahmefehler
-
-TcpSlaveAgentListener.PingAgentProtocol.displayName=Ping-Protokoll

core/src/main/resources/hudson/Messages_es.properties

@@ -117,4 +117,3 @@ PluginWrapper.Plugin.Has.Dependant=El plugin {0} tiene, al menos, un plugin depe
 PluginWrapper.Plugin.Disabled=Plugin {0} deshabilitado
 PluginWrapper.NoSuchPlugin=No se encuentra un plugin con el nombre {0}
 PluginWrapper.Error.Disabling=Hubo un error al desactivar el plugin ''{0}''. Error: ''{1}''
-TcpSlaveAgentListener.PingAgentProtocol.displayName=Protocolo ping

core/src/main/resources/hudson/Messages_fr.properties

@@ -126,4 +126,3 @@ PluginWrapper.Plugin.Has.Dependent=Le plugin "{0}" a au moins un plugin dépenda
 PluginWrapper.Plugin.Disabled=Plugin "{0}" désactivé
 PluginWrapper.NoSuchPlugin=Aucun plugin trouvé avec le nom "{0}"
 PluginWrapper.Error.Disabling=Une erreur a été relevée lors de la désactivation du plugin "{0}". Erreur : "{1}"
-TcpSlaveAgentListener.PingAgentProtocol.displayName=Protocole de ping