Skip to content

Commit

Permalink
[FIXES JENKINS-47439] Setup wizard does not resume after restart (#3166)
Browse files Browse the repository at this point in the history 
* [JENKINS-47439] Add a failing test

On first startup, the setup wizard goes into state NEW and the filter to
force display the setup wizard is installed.

On second startup, the setup wizard goes into state RESTART (which
assumes the setup wizard is done), and the setup wizard is skipped
completely.

This test expects that state NEW is retained upon restart when nothing
is done.

* [JENKINS-47439] Persist InstallState

In some cases, the heuristics to determine the current setup wizard state are
fragile. It is safer to persist the install state so that upon restart,
the setup wizard can resume where it was left off.

* Missing javadoc and since for new public methods

* s/XXX/FIXME/

* Missed that one

* Setup wizard filter should be removed when entering a state where setup is complete

* Use parameterized logging

* Improvements over previous impl

* Removed static isUsingSecurityToken. Now only determined from install
state.
* Call onInstallStateUpdate before InstallState#initializeState as the
latter can update state.

* Triggering a new build

(cherry picked from commit 30ab448)
  • Loading branch information
@Vlatombe @olivergondza
Vlatombe authored and olivergondza committed Jan 31, 2018
1 parent 7b6c86d commit c359b86
Show file tree
Hide file tree
Showing 6 changed files with 143 additions and 30 deletions.
19 changes: 19 additions & 0 deletions core/src/main/java/hudson/util/PluginServletFilter.java
View file
Expand Up @@ -113,6 +113,25 @@ public static void addFilter(Filter filter) throws ServletException {
}
}

/**
* 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) {
Expand Down
13 changes: 9 additions & 4 deletions core/src/main/java/jenkins/install/InstallState.java
View file
Expand Up @@ -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
Expand Down Expand Up @@ -94,7 +99,7 @@ public void initializeState() {
*/
@Extension
public static final InstallState INITIAL_PLUGINS_INSTALLING = new InstallState("INITIAL_PLUGINS_INSTALLING", false);

/**
* Security setup for a new Jenkins install.
*/
Expand All @@ -106,7 +111,7 @@ public void initializeState() {
} catch (Exception e) {
throw new RuntimeException(e);
}

InstallUtil.proceedToNextStateFrom(INITIAL_SECURITY_SETUP);
}
};
Expand All @@ -116,7 +121,7 @@ public void initializeState() {
*/
@Extension
public static final InstallState NEW = new InstallState("NEW", false);

/**
* Restart of an existing Jenkins install.
*/
Expand Down
1 change: 0 additions & 1 deletion core/src/main/java/jenkins/install/InstallUtil.java
View file
Expand Up @@ -93,7 +93,6 @@ public T get() {
*/
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);
}
Expand Down
81 changes: 59 additions & 22 deletions core/src/main/java/jenkins/install/SetupWizard.java
View file
Expand Up @@ -64,7 +64,6 @@
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;

Expand All @@ -77,18 +76,17 @@
@Restricted(NoExternalUse.class)
@Extension
public class SetupWizard extends PageDecorator {
public SetupWizard() {
checkFilter();
}

/**
* The security token parameter name
*/
public static String initialSetupAdminUserName = "admin";

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
*/
Expand Down Expand Up @@ -168,32 +166,43 @@ 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();
} catch (Exception e) {
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
Expand Down Expand Up @@ -487,8 +496,6 @@ public HttpResponse doCompleteInstall() throws IOException, ServletException {
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);
}

Expand All @@ -508,7 +515,28 @@ public InstallState getInstallState(String name) {
}
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
*/
Expand Down Expand Up @@ -544,4 +572,13 @@ public String getRequestURI() {
public void destroy() {
}
};

/**
* Sets up the Setup Wizard filter if the current state requires it.
*/
private void checkFilter() {
if (!Jenkins.getInstance().getInstallState().isSetupComplete()) {
setUpFilter();
}
}
}
9 changes: 6 additions & 3 deletions core/src/main/java/jenkins/model/Jenkins.java
View file
Expand Up @@ -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,
Expand Down Expand Up @@ -924,7 +924,7 @@ protected Jenkins(File root, ServletContext context, PluginManager pluginManager
System.exit(0);

setupWizard = new SetupWizard();
InstallUtil.proceedToNextStateFrom(InstallState.UNKNOWN);
getInstallState().initializeState();

launchTcpSlaveAgentListener();

Expand Down Expand Up @@ -1033,9 +1033,12 @@ public InstallState getInstallState() {
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();
}

/**
Expand Down
50 changes: 50 additions & 0 deletions test/src/test/java/jenkins/install/SetupWizardRestartTest.java
View file
@@ -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());
}
});
}

}

0 comments on commit c359b86

Please sign in to comment.