[JENKINS-48480] Switch deprecated protocols to opt-in. (#3188)

* [JENKINS-48480] Switch deprecated protocols to opt-in.

They no longer need to be removed by setup wizard since they won't be
enabled in the first place.

* [JENKINS-48480] Fix tests

* [JENKINS-48480] Remove useless calls to CLI.get().setEnabled(true)

It is already enabled.

* [JENKINS-48480] Bump remoting.minimum.supported.version to 3.4

core/src/main/java/hudson/cli/CliProtocol.java

@@ -39,7 +39,7 @@ public class CliProtocol extends AgentProtocol {
      */
     @Override
     public boolean isOptIn() {
-        return OPT_IN;
+        return true;
     }
 
     @Override
@@ -109,14 +109,4 @@ public class CliProtocol extends AgentProtocol {
             channel.join();
         }
     }
-
-    /**
-     * A/B test turning off this protocol by default.
-     */
-    private static final boolean OPT_IN;
-
-    static {
-        byte hash = Util.fromHexString(Jenkins.getInstance().getLegacyInstanceId())[0];
-        OPT_IN = (hash % 10) == 0;
-    }
 }

core/src/main/java/hudson/cli/CliProtocol2.java

@@ -35,7 +35,7 @@ public class CliProtocol2 extends CliProtocol {
      */
     @Override
     public boolean isOptIn() {
-        return false;
+        return true;
     }
 
     @Override

core/src/main/java/jenkins/install/SetupWizard.java

@@ -133,13 +133,6 @@ public class SetupWizard extends PageDecorator {
 
                     // Disable CLI over Remoting
                     CLI.get().setEnabled(false);
-
-                    // Disable old Non-Encrypted protocols ()
-                    HashSet<String> newProtocols = new HashSet<>(jenkins.getAgentProtocols());
-                    newProtocols.removeAll(Arrays.asList(
-                            "JNLP2-connect", "JNLP-connect", "CLI-connect"   
-                    ));
-                    jenkins.setAgentProtocols(newProtocols);
                     
                     // require a crumb issuer
                     jenkins.setCrumbIssuer(new DefaultCrumbIssuer(SystemProperties.getBoolean(Jenkins.class.getName() + ".crumbIssuerProxyCompatibility",false)));

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

@@ -76,7 +76,7 @@ public class JnlpSlaveAgentProtocol extends AgentProtocol {
      */
     @Override
     public boolean isOptIn() {
-        return OPT_IN;
+        return true;
     }
 
     @Override
@@ -104,14 +104,4 @@ public class JnlpSlaveAgentProtocol extends AgentProtocol {
                 ExtensionList.lookup(JnlpAgentReceiver.class));
     }
 
-
-    /**
-     * A/B test turning off this protocol by default.
-     */
-    private static final boolean OPT_IN;
-
-    static {
-        byte hash = Util.fromHexString(Jenkins.getInstance().getLegacyInstanceId())[0];
-        OPT_IN = (hash % 10) == 0;
-    }
 }

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

@@ -50,7 +50,7 @@ public class JnlpSlaveAgentProtocol2 extends AgentProtocol {
      */
     @Override
     public boolean isOptIn() {
-        return false;
+        return true;
     }
 
     @Override

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

@@ -48,15 +48,12 @@ public class JnlpSlaveAgentProtocol3 extends AgentProtocol {
      */
     @Override
     public boolean isOptIn() {
-        return !ENABLED;
+        return true ;
     }
 
     @Override
     public String getName() {
-        // we only want to force the protocol off for users that have explicitly banned it via system property
-        // everyone on the A/B test will just have the opt-in flag toggled
-        // TODO strip all this out and hardcode OptIn==TRUE once JENKINS-36871 is merged
-        return forceEnabled != Boolean.FALSE ? handler.getName() : null;
+        return handler.isEnabled() ? handler.getName() : null;
     }
 
     /**
@@ -79,26 +76,4 @@ public class JnlpSlaveAgentProtocol3 extends AgentProtocol {
                 ExtensionList.lookup(JnlpAgentReceiver.class));
     }
 
-    /**
-     * Flag to control the activation of JNLP3 protocol.
-     *
-     * <p>
-     * Once this will be on by default, the flag and this field will disappear. The system property is
-     * an escape hatch for those who hit any issues and those who are trying this out.
-     */
-    @Restricted(NoExternalUse.class)
-    @SuppressFBWarnings(value = "MS_SHOULD_BE_REFACTORED_TO_BE_FINAL",
-            justification = "Part of the administrative API for System Groovy scripts.")
-    public static boolean ENABLED;
-    private static final Boolean forceEnabled;
-
-    static {
-        forceEnabled = SystemProperties.optBoolean(JnlpSlaveAgentProtocol3.class.getName() + ".enabled");
-        if (forceEnabled != null) {
-            ENABLED = forceEnabled;
-        } else {
-            byte hash = Util.fromHexString(Jenkins.getActiveInstance().getLegacyInstanceId())[0];
-            ENABLED = (hash % 10) == 0;
-        }
-    }
 }

pom.xml

@@ -108,7 +108,7 @@ THE SOFTWARE.
     <!-- Bundled Remoting version -->
     <remoting.version>3.21</remoting.version>
     <!-- Minimum Remoting version, which is tested for API compatibility -->
-    <remoting.minimum.supported.version>2.60</remoting.minimum.supported.version>
+    <remoting.minimum.supported.version>3.4</remoting.minimum.supported.version>
 
     <!-- TODO: JENKINS-36716 - Switch to Medium once FindBugs is cleaned up, 430 issues on Mar 10, 2018 -->
     <findbugs.effort>Max</findbugs.effort>

test/src/test/java/hudson/cli/CLIActionTest.java

 package hudson.cli;
 
 import com.google.common.collect.Lists;
+
+import hudson.ExtensionList;
 import hudson.Functions;
 import hudson.Launcher;
 import hudson.Proc;
@@ -22,7 +24,9 @@ import java.net.HttpURLConnection;
 import java.net.URL;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Set;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.TimeUnit;
@@ -36,6 +40,8 @@ import org.apache.commons.io.output.TeeOutputStream;
 import org.codehaus.groovy.runtime.Security218;
 import static org.hamcrest.Matchers.containsString;
 import static org.junit.Assert.*;
+
+import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
@@ -62,6 +68,13 @@ public class CLIActionTest {
 
     private ExecutorService pool;
 
+    @Before
+    public void setUp() {
+        Set<String> agentProtocols = new HashSet<>(j.jenkins.getAgentProtocols());
+        agentProtocols.add(ExtensionList.lookupSingleton(CliProtocol2.class).getName());
+        j.jenkins.setAgentProtocols(agentProtocols);
+    }
+
     /**
      * Makes sure that the /cli endpoint is functioning.
      */

test/src/test/java/hudson/cli/ClientAuthenticationCacheTest.java

@@ -25,6 +25,8 @@
 package hudson.cli;
 
 import com.google.common.collect.Lists;
+
+import hudson.ExtensionList;
 import hudson.Launcher;
 import hudson.security.FullControlOnceLoggedInAuthorizationStrategy;
 import hudson.util.Secret;
@@ -32,13 +34,17 @@ import hudson.util.StreamTaskListener;
 import java.io.ByteArrayOutputStream;
 import java.io.File;
 import java.util.Arrays;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Set;
 import java.util.logging.Level;
 import javax.annotation.CheckForNull;
 import jenkins.model.JenkinsLocationConfiguration;
 import org.apache.commons.io.FileUtils;
 import org.apache.commons.io.output.TeeOutputStream;
 import static org.hamcrest.Matchers.containsString;
+
+import org.junit.Before;
 import org.junit.Test;
 import static org.junit.Assert.*;
 import org.junit.Ignore;
@@ -59,6 +65,13 @@ public class ClientAuthenticationCacheTest {
     @Rule
     public LoggerRule logging = new LoggerRule().record(ClientAuthenticationCache.class, Level.FINER);
 
+    @Before
+    public void setUp() {
+        Set<String> agentProtocols = new HashSet<>(r.jenkins.getAgentProtocols());
+        agentProtocols.add(ExtensionList.lookupSingleton(CliProtocol2.class).getName());
+        r.jenkins.setAgentProtocols(agentProtocols);
+    }
+
     @Issue("SECURITY-466")
     @Test
     public void login() throws Exception {

test/src/test/java/hudson/security/CliAuthenticationTest.java

 package hudson.security;
 
+import hudson.ExtensionList;
 import hudson.ExtensionList;
 import hudson.cli.CLI;
 import hudson.cli.CLICommand;
+import hudson.cli.CliProtocol2;
 import jenkins.model.Jenkins;
 import jenkins.security.SecurityListener;
 import jenkins.security.SpySecurityListener;
@@ -26,6 +28,8 @@ import org.springframework.dao.DataAccessException;
 
 import java.io.ByteArrayOutputStream;
 import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Set;
 
 import static org.junit.Assert.*;
 
@@ -46,6 +50,13 @@ public class CliAuthenticationTest {
         this.spySecurityListener = ExtensionList.lookup(SecurityListener.class).get(SpySecurityListenerImpl.class);
     }
 
+    @Before
+    public void setUp() {
+        Set<String> agentProtocols = new HashSet<>(j.jenkins.getAgentProtocols());
+        agentProtocols.add(ExtensionList.lookupSingleton(CliProtocol2.class).getName());
+       j.jenkins.setAgentProtocols(agentProtocols);
+    }
+
     @Test
     public void test() throws Exception {
         // dummy security realm that authenticates when username==password