diff --git a/core/src/main/java/hudson/PluginManager.java b/core/src/main/java/hudson/PluginManager.java index b35149190b..505692c81f 100644 --- a/core/src/main/java/hudson/PluginManager.java +++ b/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); } } diff --git a/core/src/main/java/hudson/util/PluginServletFilter.java b/core/src/main/java/hudson/util/PluginServletFilter.java index 9b311aa790..aa2c53973e 100644 --- a/core/src/main/java/hudson/util/PluginServletFilter.java +++ b/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) { diff --git a/core/src/main/java/jenkins/install/InstallState.java b/core/src/main/java/jenkins/install/InstallState.java index 71790d3c54..939c45f828 100644 --- a/core/src/main/java/jenkins/install/InstallState.java +++ b/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. */ diff --git a/core/src/main/java/jenkins/install/InstallUtil.java b/core/src/main/java/jenkins/install/InstallUtil.java index 5b3f95555b..48b8d33ab6 100644 --- a/core/src/main/java/jenkins/install/InstallUtil.java +++ b/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); } diff --git a/core/src/main/java/jenkins/install/SetupWizard.java b/core/src/main/java/jenkins/install/SetupWizard.java index 0f04f903db..f15b468eed 100644 --- a/core/src/main/java/jenkins/install/SetupWizard.java +++ b/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(); + } + } } diff --git a/core/src/main/java/jenkins/model/Jenkins.java b/core/src/main/java/jenkins/model/Jenkins.java index 5bcd97f138..38e6f565d5 100644 --- a/core/src/main/java/jenkins/model/Jenkins.java +++ b/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(); } /** diff --git a/test/src/test/java/jenkins/install/LoadDetachedPluginsTest.java b/test/src/test/java/jenkins/install/LoadDetachedPluginsTest.java index 856e5263ba..c8cc60bce5 100644 --- a/test/src/test/java/jenkins/install/LoadDetachedPluginsTest.java +++ b/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 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 -> { diff --git a/test/src/test/java/jenkins/install/SetupWizardRestartTest.java b/test/src/test/java/jenkins/install/SetupWizardRestartTest.java new file mode 100644 index 0000000000..474c1772f9 --- /dev/null +++ b/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()); + } + }); + } + +} diff --git a/test/src/test/resources/jenkins/install/LoadDetachedPluginsTest/upgradeFromJenkins2WithNewerPlugin.zip b/test/src/test/resources/jenkins/install/LoadDetachedPluginsTest/upgradeFromJenkins2WithNewerPlugin.zip new file mode 100644 index 0000000000..2c18d023bd Binary files /dev/null and b/test/src/test/resources/jenkins/install/LoadDetachedPluginsTest/upgradeFromJenkins2WithNewerPlugin.zip differ