Merge branch 'stable-2.89' of git://github.com/jenkinsci/jenkins into stable-2.89

core/src/main/java/hudson/PluginManager.java

@@ -705,13 +705,14 @@ public abstract class PluginManager extends AbstractModelObject implements OnMas
                     // If this was a plugin that was detached some time in the past i.e. not just one of the
                     // plugins that was bundled "for fun".
                     if (ClassicPluginStrategy.isDetachedPlugin(name)) {
-                        // If it's already installed and the installed version is older
-                        // than the bundled version, then we upgrade. The bundled version is the min required version
-                        // for "this" version of Jenkins, so we must upgrade.
                         VersionNumber installedVersion = getPluginVersion(rootDir, name);
                         VersionNumber bundledVersion = getPluginVersion(dir, name);
-                        if (installedVersion != null && bundledVersion != null && installedVersion.isOlderThan(bundledVersion)) {
-                            return true;
+                        // If the plugin is already installed, we need to decide whether to replace it with the bundled version.
+                        if (installedVersion != null && bundledVersion != null) {
+                            // If the installed version is older than the bundled version, then it MUST be upgraded.
+                            // If the installed version is newer than the bundled version, then it MUST NOT be upgraded.
+                            // If the versions are equal we just keep the installed version.
+                            return installedVersion.isOlderThan(bundledVersion);
                         }
                     }
 

core/src/main/java/hudson/util/PluginServletFilter.java

@@ -113,6 +113,25 @@ public class PluginServletFilter implements Filter, ExtensionPoint {
         }
     }
 
+    /**
+     * Checks whether the given filter is already registered in the chain.
+     * @param filter the filter to check.
+     * @return true if the filter is already registered in the chain.
+     * @since FIXME
+     */
+    public static boolean hasFilter(Filter filter) {
+        Jenkins j = Jenkins.getInstanceOrNull();
+        PluginServletFilter container = null;
+        if(j != null) {
+            container = getInstance(j.servletContext);
+        }
+        if (j == null || container == null) {
+            return LEGACY.contains(filter);
+        } else {
+            return container.list.contains(filter);
+        }
+    }
+
     public static void removeFilter(Filter filter) throws ServletException {
         Jenkins j = Jenkins.getInstanceOrNull();
         if (j==null || getInstance(j.servletContext) == null) {

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

@@ -49,7 +49,12 @@ public class InstallState implements ExtensionPoint {
      * Need InstallState != NEW for tests by default
      */
     @Extension
-    public static final InstallState UNKNOWN = new InstallState("UNKNOWN", true);
+    public static final InstallState UNKNOWN = new InstallState("UNKNOWN", true) {
+        @Override
+        public void initializeState() {
+            InstallUtil.proceedToNextStateFrom(this);
+        }
+    };
     
     /**
      * After any setup / restart / etc. hooks are done, states should be running
@@ -94,7 +99,7 @@ public class InstallState implements ExtensionPoint {
      */
     @Extension
     public static final InstallState INITIAL_PLUGINS_INSTALLING = new InstallState("INITIAL_PLUGINS_INSTALLING", false);
-    
+
     /**
      * Security setup for a new Jenkins install.
      */
@@ -106,7 +111,7 @@ public class InstallState implements ExtensionPoint {
             } catch (Exception e) {
                 throw new RuntimeException(e);
             }
-            
+
             InstallUtil.proceedToNextStateFrom(INITIAL_SECURITY_SETUP);
         }
     };
@@ -116,7 +121,7 @@ public class InstallState implements ExtensionPoint {
      */
     @Extension
     public static final InstallState NEW = new InstallState("NEW", false);
-    
+
     /**
      * Restart of an existing Jenkins install.
      */

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

@@ -93,7 +93,6 @@ public class InstallUtil {
      */
     public static void proceedToNextStateFrom(InstallState prior) {
         InstallState next = getNextInstallState(prior);
-        if (Main.isDevelopmentMode) LOGGER.info("Install state transitioning from: " + prior + " to: " + next);
         if (next != null) {
             Jenkins.getInstance().setInstallState(next);
         }

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

@@ -64,7 +64,6 @@ import net.sf.json.JSONArray;
 import net.sf.json.JSONObject;
 import org.apache.commons.io.FileUtils;
 import org.apache.commons.io.IOUtils;
-import org.jenkinsci.remoting.engine.JnlpProtocol4Handler;
 import org.kohsuke.accmod.restrictions.DoNotUse;
 import org.kohsuke.stapler.interceptor.RequirePOST;
 
@@ -77,6 +76,10 @@ import org.kohsuke.stapler.interceptor.RequirePOST;
 @Restricted(NoExternalUse.class)
 @Extension
 public class SetupWizard extends PageDecorator {
+    public SetupWizard() {
+        checkFilter();
+    }
+
     /**
      * The security token parameter name
      */
@@ -84,11 +87,6 @@ public class SetupWizard extends PageDecorator {
 
     private static final Logger LOGGER = Logger.getLogger(SetupWizard.class.getName());
     
-    /**
-     * Used to determine if this was a new install (vs. an upgrade, restart, or otherwise)
-     */
-    private static boolean isUsingSecurityToken = false;
-
     /**
      * Initialize the setup wizard, this will process any current state initializations
      */
@@ -168,17 +166,8 @@ public class SetupWizard extends PageDecorator {
                         + "*************************************************************" + ls
                         + "*************************************************************" + ls);
             }
-            
-            try {
-                PluginServletFilter.addFilter(FORCE_SETUP_WIZARD_FILTER);
-                // if we're not using security defaults, we should not show the security token screen
-                // users will likely be sent to a login screen instead
-                isUsingSecurityToken = isUsingSecurityDefaults();
-            } catch (ServletException e) {
-                throw new RuntimeException("Unable to add PluginServletFilter for the SetupWizard", e);
-            }
         }
-        
+
         try {
             // Make sure plugin metadata is up to date
             UpdateCenter.updateDefaultSite();
@@ -186,14 +175,34 @@ public class SetupWizard extends PageDecorator {
             LOGGER.log(Level.WARNING, e.getMessage(), e);
         }
     }
-    
+
+    private void setUpFilter() {
+        try {
+            if (!PluginServletFilter.hasFilter(FORCE_SETUP_WIZARD_FILTER)) {
+                PluginServletFilter.addFilter(FORCE_SETUP_WIZARD_FILTER);
+            }
+        } catch (ServletException e) {
+            throw new RuntimeException("Unable to add PluginServletFilter for the SetupWizard", e);
+        }
+    }
+
+    private void tearDownFilter() {
+        try {
+            if (PluginServletFilter.hasFilter(FORCE_SETUP_WIZARD_FILTER)) {
+                PluginServletFilter.removeFilter(FORCE_SETUP_WIZARD_FILTER);
+            }
+        } catch (ServletException e) {
+            throw new RuntimeException("Unable to remove PluginServletFilter for the SetupWizard", e);
+        }
+    }
+
     /**
      * Indicates a generated password should be used - e.g. this is a new install, no security realm set up
      */
+    @SuppressWarnings("unused") // used by jelly
     public boolean isUsingSecurityToken() {
         try {
-            return isUsingSecurityToken // only ever show the unlock page if using the security token
-                    && !Jenkins.getInstance().getInstallState().isSetupComplete()
+            return !Jenkins.getInstance().getInstallState().isSetupComplete()
                     && isUsingSecurityDefaults();
         } catch (Exception e) {
             // ignore
@@ -487,8 +496,6 @@ public class SetupWizard extends PageDecorator {
         Jenkins.getInstance().checkPermission(Jenkins.ADMINISTER);
         InstallUtil.saveLastExecVersion();
         setCurrentLevel(Jenkins.getVersion());
-        PluginServletFilter.removeFilter(FORCE_SETUP_WIZARD_FILTER);
-        isUsingSecurityToken = false; // this should not be considered new anymore
         InstallUtil.proceedToNextStateFrom(InstallState.INITIAL_SETUP_COMPLETED);
     }
     
@@ -508,7 +515,28 @@ public class SetupWizard extends PageDecorator {
         }
         return InstallState.valueOf(name);
     }
-    
+
+    /**
+     * Called upon install state update.
+     * @param state the new install state.
+     * @since FIXME
+     */
+    public void onInstallStateUpdate(InstallState state) {
+        if (state.isSetupComplete()) {
+            tearDownFilter();
+        } else {
+            setUpFilter();
+        }
+    }
+
+    /**
+     * Returns whether the setup wizard filter is currently registered.
+     * @since FIXME
+     */
+    public boolean hasSetupWizardFilter() {
+        return PluginServletFilter.hasFilter(FORCE_SETUP_WIZARD_FILTER);
+    }
+
     /**
      * This filter will validate that the security token is provided
      */
@@ -544,4 +572,13 @@ public class SetupWizard extends PageDecorator {
         public void destroy() {
         }
     };
+
+    /**
+     * Sets up the Setup Wizard filter if the current state requires it.
+     */
+    private void checkFilter() {
+        if (!Jenkins.getInstance().getInstallState().isSetupComplete()) {
+            setUpFilter();
+        }
+    }
 }

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

@@ -328,7 +328,7 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve
     /**
      * The Jenkins instance startup type i.e. NEW, UPGRADE etc
      */
-    private transient InstallState installState = InstallState.UNKNOWN;
+    private InstallState installState;
     
     /**
      * If we're in the process of an initial setup, 
@@ -924,7 +924,7 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve
                 System.exit(0);
 
             setupWizard = new SetupWizard();
-            InstallUtil.proceedToNextStateFrom(InstallState.UNKNOWN);
+            getInstallState().initializeState();
 
             launchTcpSlaveAgentListener();
 
@@ -1033,9 +1033,12 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve
     public void setInstallState(@Nonnull InstallState newState) {
         InstallState prior = installState;
         installState = newState;
-        if (!prior.equals(newState)) {
+        LOGGER.log(Main.isDevelopmentMode ? Level.INFO : Level.FINE, "Install state transitioning from: {0} to : {1}", new Object[] { prior, installState });
+        if (!newState.equals(prior)) {
+            getSetupWizard().onInstallStateUpdate(newState);
             newState.initializeState();
         }
+        saveQuietly();
     }
 
     /**

test/src/test/java/jenkins/install/LoadDetachedPluginsTest.java

@@ -120,6 +120,25 @@ public class LoadDetachedPluginsTest {
         });
     }
 
+    @Issue("JENKINS-48899")
+    @Test
+    @LocalData
+    public void upgradeFromJenkins2WithNewerPlugin() {
+        // @LocalData has command-launcher 1.2 installed, which should not be downgraded to the detached version: 1.0.
+        VersionNumber since = new VersionNumber("2.0");
+        rr.then(r -> {
+            List<DetachedPlugin> detachedPlugins = ClassicPluginStrategy.getDetachedPlugins(since);
+            assertThat("Plugins have been detached since the pre-upgrade version",
+                    detachedPlugins.size(), greaterThan(1));
+            assertThat("Plugins detached between the pre-upgrade version and the current version should be installed",
+                    getInstalledDetachedPlugins(r, detachedPlugins).size(), equalTo(detachedPlugins.size()));
+            Plugin commandLauncher = r.jenkins.getPlugin("command-launcher");
+            assertThat("Installed detached plugins should not be overwritten by older versions",
+                    commandLauncher.getWrapper().getVersionNumber(), equalTo(new VersionNumber("1.2")));
+            assertNoFailedPlugins(r);
+        });
+    }
+
     @Test
     public void newInstallation() {
         rr.then(r -> {

test/src/test/java/jenkins/install/SetupWizardRestartTest.java

@@ -0,0 +1,50 @@
+package jenkins.install;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import java.io.IOException;
+
+import org.apache.commons.io.FileUtils;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.runners.model.Statement;
+import org.jvnet.hudson.test.Issue;
+import org.jvnet.hudson.test.RestartableJenkinsRule;
+
+import hudson.Main;
+import jenkins.model.Jenkins;
+
+public class SetupWizardRestartTest {
+    @Rule
+    public RestartableJenkinsRule rr = new RestartableJenkinsRule();
+
+    @Issue("JENKINS-47439")
+    @Test
+    public void restartKeepsSetupWizardState() {
+        rr.addStep(new Statement() {
+            @Override
+            public void evaluate() throws IOException {
+                // Modify state so that we get into the same conditions as a real start
+                Main.isUnitTest = false;
+                FileUtils.write(InstallUtil.getLastExecVersionFile(), "");
+                Jenkins j = rr.j.getInstance();
+                // Re-evaluate current state based on the new context
+                InstallUtil.proceedToNextStateFrom(InstallState.UNKNOWN);
+                assertEquals("Unexpected install state", InstallState.NEW, j.getInstallState());
+                assertTrue("Expecting setup wizard filter to be up", j.getSetupWizard().hasSetupWizardFilter());
+                InstallUtil.saveLastExecVersion();
+            }
+        });
+        // Check that the state is retained after a restart
+        rr.addStep(new Statement() {
+            @Override
+            public void evaluate() {
+                Jenkins j = rr.j.getInstance();
+                assertEquals("Unexpected install state", InstallState.NEW, j.getInstallState());
+                assertTrue("Expecting setup wizard filter to be up after restart",  j.getSetupWizard().hasSetupWizardFilter());
+            }
+        });
+    }
+
+}