allow REST server shutdown POST when running as standby or backup modifies HaMasterCheckFilter, simplifying the code, and adding an extra check to allow a "shutdown (leave apps running)" POST (otherwise all other POSTs and PATCHes are disallowed, unless a special header is added, but GET and HEAD *are* allowed)
Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/e400d777 Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/e400d777 Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/e400d777 Branch: refs/heads/master Commit: e400d777b0b3dea91c285ff4680fc757b1e9d49a Parents: cda8069 Author: Alex Heneveld <[email protected]> Authored: Thu Mar 19 16:23:00 2015 +0000 Committer: Alex Heneveld <[email protected]> Committed: Fri Mar 20 14:06:19 2015 +0000 ---------------------------------------------------------------------- .../rebind/InitialFullRebindIteration.java | 11 +++++---- .../rest/filter/HaMasterCheckFilter.java | 25 ++++++++++++++++---- .../brooklyn/rest/resources/ServerResource.java | 7 ++++++ 3 files changed, 35 insertions(+), 8 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/e400d777/core/src/main/java/brooklyn/entity/rebind/InitialFullRebindIteration.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/brooklyn/entity/rebind/InitialFullRebindIteration.java b/core/src/main/java/brooklyn/entity/rebind/InitialFullRebindIteration.java index a84d34b..02cc9e8 100644 --- a/core/src/main/java/brooklyn/entity/rebind/InitialFullRebindIteration.java +++ b/core/src/main/java/brooklyn/entity/rebind/InitialFullRebindIteration.java @@ -77,13 +77,16 @@ public class InitialFullRebindIteration extends RebindIteration { preprocessManifestFiles(); - if (mode!=ManagementNodeState.HOT_STANDBY && mode!=ManagementNodeState.HOT_BACKUP) { - if (!isEmpty) { + if (!isEmpty) { + if (!ManagementNodeState.isHotProxy(mode) || readOnlyRebindCount.get()==1) { LOG.info("Rebinding from "+getPersister().getBackingStoreDescription()+" for "+Strings.toLowerCase(Strings.toString(mode))+" "+managementContext.getManagementNodeId()+"..."); - } else { + } + } else { + if (!ManagementNodeState.isHotProxy(mode)) { LOG.info("Rebind check: no existing state; will persist new items to "+getPersister().getBackingStoreDescription()); } - + } + if (!ManagementNodeState.isHotProxy(mode)) { if (!managementContext.getEntityManager().getEntities().isEmpty() || !managementContext.getLocationManager().getLocations().isEmpty()) { // this is discouraged if we were already master Entity anEntity = Iterables.getFirst(managementContext.getEntityManager().getEntities(), null); http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/e400d777/usage/rest-server/src/main/java/brooklyn/rest/filter/HaMasterCheckFilter.java ---------------------------------------------------------------------- diff --git a/usage/rest-server/src/main/java/brooklyn/rest/filter/HaMasterCheckFilter.java b/usage/rest-server/src/main/java/brooklyn/rest/filter/HaMasterCheckFilter.java index bb0663f..03d75e0 100644 --- a/usage/rest-server/src/main/java/brooklyn/rest/filter/HaMasterCheckFilter.java +++ b/usage/rest-server/src/main/java/brooklyn/rest/filter/HaMasterCheckFilter.java @@ -20,6 +20,7 @@ package brooklyn.rest.filter; import java.io.IOException; import java.util.Set; + import javax.servlet.Filter; import javax.servlet.FilterChain; import javax.servlet.FilterConfig; @@ -29,12 +30,15 @@ import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import com.google.common.collect.Sets; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import brooklyn.config.BrooklynServiceAttributes; import brooklyn.management.ManagementContext; import brooklyn.management.ha.ManagementNodeState; +import com.google.common.collect.Sets; + /** * Checks that the request is appropriate given the high availability status of the server. * @@ -42,6 +46,8 @@ import brooklyn.management.ha.ManagementNodeState; */ public class HaMasterCheckFilter implements Filter { + private static final Logger log = LoggerFactory.getLogger(HaMasterCheckFilter.class); + public static final String SKIP_CHECK_HEADER = "Brooklyn-Allow-Non-Master-Access"; private static final Set<String> SAFE_STANDBY_METHODS = Sets.newHashSet("GET", "HEAD"); @@ -54,7 +60,8 @@ public class HaMasterCheckFilter implements Filter { @Override public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException { - if (!isMaster() && isUnsafeRequest(request)) { + if (!isMaster() && !isRequestAllowedForNonMaster(request)) { + log.warn("Disallowed request to non-HA master: "+request+"/"+request.getParameterMap()+" (caller should set '"+SKIP_CHECK_HEADER+"' to force)"); HttpServletResponse httpResponse = (HttpServletResponse) response; httpResponse.setStatus(HttpServletResponse.SC_FORBIDDEN); httpResponse.setContentType("application/json"); @@ -73,13 +80,23 @@ public class HaMasterCheckFilter implements Filter { return ManagementNodeState.MASTER.equals(mgmt.getHighAvailabilityManager().getNodeState()); } - private boolean isUnsafeRequest(ServletRequest request) { + private boolean isRequestAllowedForNonMaster(ServletRequest request) { if (request instanceof HttpServletRequest) { HttpServletRequest httpRequest = (HttpServletRequest) request; String checkOverridden = httpRequest.getHeader(SKIP_CHECK_HEADER); + if ("true".equalsIgnoreCase(checkOverridden)) return true; + String method = httpRequest.getMethod().toUpperCase(); - return !"true".equalsIgnoreCase(checkOverridden) && !SAFE_STANDBY_METHODS.contains(method); + if (SAFE_STANDBY_METHODS.contains(method)) return true; + + // explicitly allow calls to shutdown + // (if stopAllApps is specified, the method itself will fail; but we do not want to consume parameters here, that breaks things!) + // TODO combine with HaHotCheckResourceFilter and use an annotation HaAnyStateAllowed or similar + if ("/v1/server/shutdown".equals(httpRequest.getRequestURI())) return true; + + return false; } + // previously non-HttpServletRequests were allowed but I don't think they should be return false; } } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/e400d777/usage/rest-server/src/main/java/brooklyn/rest/resources/ServerResource.java ---------------------------------------------------------------------- diff --git a/usage/rest-server/src/main/java/brooklyn/rest/resources/ServerResource.java b/usage/rest-server/src/main/java/brooklyn/rest/resources/ServerResource.java index 6f8465e..0714c4b 100644 --- a/usage/rest-server/src/main/java/brooklyn/rest/resources/ServerResource.java +++ b/usage/rest-server/src/main/java/brooklyn/rest/resources/ServerResource.java @@ -87,6 +87,10 @@ public class ServerResource extends AbstractBrooklynRestResource implements Serv brooklyn().reloadBrooklynProperties(); } + private boolean isMaster() { + return ManagementNodeState.MASTER.equals(mgmt().getHighAvailabilityManager().getNodeState()); + } + @Override public void shutdown(final boolean stopAppsFirst, final boolean forceShutdownOnError, String shutdownTimeoutRaw, String requestTimeoutRaw, String delayForHttpReturnRaw, @@ -97,6 +101,9 @@ public class ServerResource extends AbstractBrooklynRestResource implements Serv log.info("REST call to shutdown server, stopAppsFirst="+stopAppsFirst+", delayForHttpReturn="+shutdownTimeoutRaw); + if (stopAppsFirst && !isMaster()) { + throw WebResourceUtils.serverError("Not allowed to stop all apps when server is not master"); + } final Duration shutdownTimeout = parseDuration(shutdownTimeoutRaw, Duration.of(20, TimeUnit.SECONDS)); Duration requestTimeout = parseDuration(requestTimeoutRaw, Duration.of(20, TimeUnit.SECONDS)); final Duration delayForHttpReturn;
