move common ha filter code to shared class and ensure classes not in scope aren't referenced
Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/3ab00406 Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/3ab00406 Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/3ab00406 Branch: refs/heads/master Commit: 3ab00406bc484ec4d8c667d1a068977a25e2aba6 Parents: 7f047ba Author: Alex Heneveld <alex.henev...@cloudsoftcorp.com> Authored: Thu Feb 18 12:15:08 2016 +0000 Committer: Alex Heneveld <alex.henev...@cloudsoftcorp.com> Committed: Thu Feb 18 13:28:54 2016 +0000 ---------------------------------------------------------------------- .../rest/filter/HaHotCheckHelperAbstract.java | 82 ++++++++++++++++++++ .../rest/filter/HaHotCheckResourceFilter.java | 72 +++++------------ .../rest/resources/CatalogResourceTest.java | 2 + .../rest/filter/HaMasterCheckFilter.java | 35 ++++----- .../brooklyn/rest/BrooklynRestApiLauncher.java | 4 - 5 files changed, 118 insertions(+), 77 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/3ab00406/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/filter/HaHotCheckHelperAbstract.java ---------------------------------------------------------------------- diff --git a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/filter/HaHotCheckHelperAbstract.java b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/filter/HaHotCheckHelperAbstract.java new file mode 100644 index 0000000..7f183a6 --- /dev/null +++ b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/filter/HaHotCheckHelperAbstract.java @@ -0,0 +1,82 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.brooklyn.rest.filter; + +import java.util.Set; + +import javax.ws.rs.core.Response; + +import org.apache.brooklyn.api.mgmt.ManagementContext; +import org.apache.brooklyn.api.mgmt.ha.ManagementNodeState; +import org.apache.brooklyn.rest.domain.ApiError; +import org.apache.brooklyn.util.guava.Maybe; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.google.common.annotations.Beta; +import com.google.common.collect.ImmutableSet; + +@Beta +public abstract class HaHotCheckHelperAbstract { + + private static final Logger log = LoggerFactory.getLogger(HaHotCheckHelperAbstract.class); + + public static final String SKIP_CHECK_HEADER = "Brooklyn-Allow-Non-Master-Access"; + + private static final Set<ManagementNodeState> HOT_STATES = ImmutableSet.of( + ManagementNodeState.MASTER, ManagementNodeState.HOT_STANDBY, ManagementNodeState.HOT_BACKUP); + + /** Returns a string describing the problem if mgmt is null or not running; returns absent if no problems */ + public static Maybe<String> getProblemMessageIfServerNotRunning(ManagementContext mgmt) { + if (mgmt==null) return Maybe.of("no management context available"); + if (!mgmt.isRunning()) return Maybe.of("server no longer running"); + if (!mgmt.isStartupComplete()) return Maybe.of("server not in required startup-completed state"); + return Maybe.absent(); + } + + public Maybe<String> getProblemMessageIfServerNotRunning() { + return getProblemMessageIfServerNotRunning(mgmt()); + } + + public Response disallowResponse(String problem, Object info) { + log.warn("Disallowing web request as "+problem+": "+info+" (caller should set '"+HaHotCheckHelperAbstract.SKIP_CHECK_HEADER+"' to force)"); + return ApiError.builder() + .message("This request is only permitted against an active master Brooklyn server") + .errorCode(Response.Status.FORBIDDEN).build().asJsonResponse(); + } + + public boolean isSkipCheckHeaderSet(String headerValueString) { + return "true".equalsIgnoreCase(headerValueString); + } + + public boolean isHaHotStatus() { + ManagementNodeState state = mgmt().getHighAvailabilityManager().getNodeState(); + return HOT_STATES.contains(state); + } + + public abstract ManagementContext mgmt(); + + // Maybe there should be a separate state to indicate that we have switched state + // but still haven't finished rebinding. (Previously there was a time delay and an + // isRebinding check, but introducing RebindManager#isAwaitingInitialRebind() seems cleaner.) + public boolean isStateNotYetValid() { + return mgmt().getRebindManager().isAwaitingInitialRebind(); + } + +} http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/3ab00406/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/filter/HaHotCheckResourceFilter.java ---------------------------------------------------------------------- diff --git a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/filter/HaHotCheckResourceFilter.java b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/filter/HaHotCheckResourceFilter.java index 13c5cbf..3c9c129 100644 --- a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/filter/HaHotCheckResourceFilter.java +++ b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/filter/HaHotCheckResourceFilter.java @@ -21,25 +21,20 @@ package org.apache.brooklyn.rest.filter; import java.io.IOException; import java.lang.annotation.Annotation; import java.lang.reflect.Method; -import java.util.Set; import javax.ws.rs.container.ContainerRequestContext; import javax.ws.rs.container.ContainerRequestFilter; import javax.ws.rs.container.ResourceInfo; import javax.ws.rs.core.Context; -import javax.ws.rs.core.Response; import javax.ws.rs.ext.ContextResolver; import javax.ws.rs.ext.Provider; import org.apache.brooklyn.api.mgmt.ManagementContext; -import org.apache.brooklyn.api.mgmt.ha.ManagementNodeState; -import org.apache.brooklyn.rest.domain.ApiError; +import org.apache.brooklyn.rest.util.BrooklynRestResourceUtils; +import org.apache.brooklyn.util.guava.Maybe; import org.apache.brooklyn.util.text.Strings; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import com.google.common.annotations.VisibleForTesting; -import com.google.common.collect.ImmutableSet; /** * Checks that if the method or resource class corresponding to a request @@ -52,13 +47,8 @@ import com.google.common.collect.ImmutableSet; */ @Provider public class HaHotCheckResourceFilter implements ContainerRequestFilter { - public static final String SKIP_CHECK_HEADER = "Brooklyn-Allow-Non-Master-Access"; + public static final String SKIP_CHECK_HEADER = HaHotCheckHelperAbstract.SKIP_CHECK_HEADER; - private static final Logger log = LoggerFactory.getLogger(HaHotCheckResourceFilter.class); - - private static final Set<ManagementNodeState> HOT_STATES = ImmutableSet.of( - ManagementNodeState.MASTER, ManagementNodeState.HOT_STANDBY, ManagementNodeState.HOT_BACKUP); - // Not quite standards compliant. Should instead be: // @Context Providers providers // .... @@ -70,6 +60,12 @@ public class HaHotCheckResourceFilter implements ContainerRequestFilter { @Context private ResourceInfo resourceInfo; + private HaHotCheckHelperAbstract helper = new HaHotCheckHelperAbstract() { + public ManagementContext mgmt() { + return mgmt.getContext(ManagementContext.class); + } + }; + public HaHotCheckResourceFilter() { } @@ -78,60 +74,44 @@ public class HaHotCheckResourceFilter implements ContainerRequestFilter { this.mgmt = mgmt; } - private ManagementContext mgmt() { - return mgmt.getContext(ManagementContext.class); - } - @Override public void filter(ContainerRequestContext requestContext) throws IOException { String problem = lookForProblem(requestContext); if (Strings.isNonBlank(problem)) { - log.warn("Disallowing web request as "+problem+": "+requestContext.getUriInfo()+"/"+resourceInfo.getResourceMethod()+" (caller should set '"+SKIP_CHECK_HEADER+"' to force)"); - requestContext.abortWith(ApiError.builder() - .message("This request is only permitted against an active hot Brooklyn server") - .errorCode(Response.Status.FORBIDDEN).build().asJsonResponse()); + requestContext.abortWith(helper.disallowResponse(problem, requestContext.getUriInfo()+"/"+resourceInfo.getResourceMethod())); } } private String lookForProblem(ContainerRequestContext requestContext) { - if (isSkipCheckHeaderSet(requestContext)) + if (helper.isSkipCheckHeaderSet(requestContext.getHeaderString(SKIP_CHECK_HEADER))) return null; if (!isHaHotStateRequired()) return null; - String problem = lookForProblemIfServerNotRunning(mgmt()); - if (Strings.isNonBlank(problem)) - return problem; + Maybe<String> problem = helper.getProblemMessageIfServerNotRunning(); + if (problem.isPresent()) + return problem.get(); - if (!isHaHotStatus()) + if (!helper.isHaHotStatus()) return "server not in required HA hot state"; - if (isStateNotYetValid()) + if (helper.isStateNotYetValid()) return "server not yet completed loading data for required HA hot state"; return null; } - + + /** @deprecated since 0.9.0 use {@link BrooklynRestResourceUtils#getProblemMessageIfServerNotRunning(ManagementContext)} */ public static String lookForProblemIfServerNotRunning(ManagementContext mgmt) { - if (mgmt==null) return "no management context available"; - if (!mgmt.isRunning()) return "server no longer running"; - if (!mgmt.isStartupComplete()) return "server not in required startup-completed state"; - return null; + return HaHotCheckHelperAbstract.getProblemMessageIfServerNotRunning(mgmt).orNull(); } - // Maybe there should be a separate state to indicate that we have switched state - // but still haven't finished rebinding. (Previously there was a time delay and an - // isRebinding check, but introducing RebindManager#isAwaitingInitialRebind() seems cleaner.) - private boolean isStateNotYetValid() { - return mgmt().getRebindManager().isAwaitingInitialRebind(); - } - - private boolean isHaHotStateRequired() { + protected boolean isHaHotStateRequired() { // TODO support super annotations Method m = resourceInfo.getResourceMethod(); return getAnnotation(m, HaHotStateRequired.class) != null; } - + private <T extends Annotation> T getAnnotation(Method m, Class<T> annotation) { T am = m.getAnnotation(annotation); if (am != null) { @@ -146,14 +126,4 @@ public class HaHotCheckResourceFilter implements ContainerRequestFilter { return null; } - private boolean isSkipCheckHeaderSet(ContainerRequestContext requestContext) { - return "true".equalsIgnoreCase(requestContext.getHeaderString(SKIP_CHECK_HEADER)); - } - - private boolean isHaHotStatus() { - ManagementNodeState state = mgmt().getHighAvailabilityManager().getNodeState(); - return HOT_STATES.contains(state); - } - - } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/3ab00406/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/resources/CatalogResourceTest.java ---------------------------------------------------------------------- diff --git a/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/resources/CatalogResourceTest.java b/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/resources/CatalogResourceTest.java index 8d5d017..f3374dc 100644 --- a/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/resources/CatalogResourceTest.java +++ b/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/resources/CatalogResourceTest.java @@ -126,6 +126,7 @@ public class CatalogResourceTest extends BrooklynRestResourceTest { // an InterfacesTag should be created for every catalog item assertEquals(entityItem.getTags().size(), 1); Object tag = entityItem.getTags().iterator().next(); + @SuppressWarnings("unchecked") List<String> actualInterfaces = ((Map<String, List<String>>) tag).get("traits"); List<Class<?>> expectedInterfaces = Reflections.getAllInterfaces(TestEntity.class); assertEquals(actualInterfaces.size(), expectedInterfaces.size()); @@ -138,6 +139,7 @@ public class CatalogResourceTest extends BrooklynRestResourceTest { } @Test + // osgi may fail in IDE, typically works on CLI though public void testRegisterOsgiPolicyTopLevelSyntax() { TestResourceUnavailableException.throwIfResourceUnavailable(getClass(), OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_PATH); http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/3ab00406/rest/rest-server/src/main/java/org/apache/brooklyn/rest/filter/HaMasterCheckFilter.java ---------------------------------------------------------------------- diff --git a/rest/rest-server/src/main/java/org/apache/brooklyn/rest/filter/HaMasterCheckFilter.java b/rest/rest-server/src/main/java/org/apache/brooklyn/rest/filter/HaMasterCheckFilter.java index 991d58c..bfb1caf 100644 --- a/rest/rest-server/src/main/java/org/apache/brooklyn/rest/filter/HaMasterCheckFilter.java +++ b/rest/rest-server/src/main/java/org/apache/brooklyn/rest/filter/HaMasterCheckFilter.java @@ -30,18 +30,14 @@ import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import javax.ws.rs.core.Response; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import org.apache.brooklyn.api.mgmt.ManagementContext; import org.apache.brooklyn.api.mgmt.ha.ManagementNodeState; -import org.apache.brooklyn.rest.domain.ApiError; +import org.apache.brooklyn.rest.util.OsgiCompat; import org.apache.brooklyn.rest.util.WebResourceUtils; -import org.apache.brooklyn.util.text.Strings; +import org.apache.brooklyn.util.guava.Maybe; import com.google.common.collect.Sets; -import org.apache.brooklyn.rest.util.OsgiCompat; /** * Checks that for requests that want HA master state, the server is up and in that state. @@ -52,13 +48,17 @@ import org.apache.brooklyn.rest.util.OsgiCompat; // TODO Merge with HaHotCheckResourceFilter so the functionality is available in Karaf public class HaMasterCheckFilter implements Filter { - private static final Logger log = LoggerFactory.getLogger(HaMasterCheckFilter.class); - private static final Set<String> SAFE_STANDBY_METHODS = Sets.newHashSet("GET", "HEAD"); protected ServletContext servletContext; protected ManagementContext mgmt; + private HaHotCheckHelperAbstract helper = new HaHotCheckHelperAbstract() { + public ManagementContext mgmt() { + return mgmt; + } + }; + @Override public void init(FilterConfig config) throws ServletException { servletContext = config.getServletContext(); @@ -66,15 +66,15 @@ public class HaMasterCheckFilter implements Filter { } private String lookForProblem(ServletRequest request) { - if (isSkipCheckHeaderSet(request)) + if (helper.isSkipCheckHeaderSet(((HttpServletRequest)request).getHeader(HaHotCheckHelperAbstract.SKIP_CHECK_HEADER))) return null; if (!isMasterRequiredForRequest(request)) return null; - String problem = HaHotCheckResourceFilter.lookForProblemIfServerNotRunning(mgmt); - if (Strings.isNonBlank(problem)) - return problem; + Maybe<String> problem = helper.getProblemMessageIfServerNotRunning(); + if (problem.isPresent()) + return problem.get(); if (!isMaster()) return "server not in required HA master state"; @@ -86,10 +86,7 @@ public class HaMasterCheckFilter implements Filter { public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException { String problem = lookForProblem(request); if (problem!=null) { - log.warn("Disallowing web request as "+problem+": "+request.getParameterMap()+" (caller should set '"+HaHotCheckResourceFilter.SKIP_CHECK_HEADER+"' to force)"); - WebResourceUtils.applyJsonResponse(servletContext, ApiError.builder() - .message("This request is only permitted against an active master Brooklyn server") - .errorCode(Response.Status.FORBIDDEN).build().asJsonResponse(), (HttpServletResponse)response); + WebResourceUtils.applyJsonResponse(servletContext, helper.disallowResponse(problem, request.getParameterMap()), (HttpServletResponse)response); } else { chain.doFilter(request, response); } @@ -124,10 +121,4 @@ public class HaMasterCheckFilter implements Filter { return true; } - private boolean isSkipCheckHeaderSet(ServletRequest httpRequest) { - if (httpRequest instanceof HttpServletRequest) - return "true".equalsIgnoreCase(((HttpServletRequest)httpRequest).getHeader(HaHotCheckResourceFilter.SKIP_CHECK_HEADER)); - return false; - } - } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/3ab00406/rest/rest-server/src/test/java/org/apache/brooklyn/rest/BrooklynRestApiLauncher.java ---------------------------------------------------------------------- diff --git a/rest/rest-server/src/test/java/org/apache/brooklyn/rest/BrooklynRestApiLauncher.java b/rest/rest-server/src/test/java/org/apache/brooklyn/rest/BrooklynRestApiLauncher.java index 8d71dcb..d9f0f1a 100644 --- a/rest/rest-server/src/test/java/org/apache/brooklyn/rest/BrooklynRestApiLauncher.java +++ b/rest/rest-server/src/test/java/org/apache/brooklyn/rest/BrooklynRestApiLauncher.java @@ -36,7 +36,6 @@ import org.apache.brooklyn.core.mgmt.internal.LocalManagementContext; import org.apache.brooklyn.core.mgmt.internal.ManagementContextInternal; import org.apache.brooklyn.core.server.BrooklynServerConfig; import org.apache.brooklyn.core.server.BrooklynServiceAttributes; -import org.apache.brooklyn.rest.apidoc.RestApiResourceScanner; import org.apache.brooklyn.rest.filter.BrooklynPropertiesSecurityFilter; import org.apache.brooklyn.rest.filter.HaMasterCheckFilter; import org.apache.brooklyn.rest.filter.LoggingFilter; @@ -44,7 +43,6 @@ import org.apache.brooklyn.rest.filter.RequestTaggingFilter; import org.apache.brooklyn.rest.security.provider.AnyoneSecurityProvider; import org.apache.brooklyn.rest.security.provider.SecurityProvider; import org.apache.brooklyn.rest.util.ManagementContextProvider; -import org.apache.brooklyn.rest.util.OsgiCompat; import org.apache.brooklyn.rest.util.ServerStoppingShutdownHandler; import org.apache.brooklyn.rest.util.ShutdownHandlerProvider; import org.apache.brooklyn.util.core.ResourceUtils; @@ -68,8 +66,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.Lists; import com.google.common.io.Files; -import io.swagger.config.ScannerFactory; - /** Convenience and demo for launching programmatically. Also used for automated tests. * <p> * BrooklynLauncher has a more full-featured CLI way to start,
