From bcae284dd9343be846d7e13ef4792afb0aedbf54 Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Thu, 5 Dec 2013 08:46:20 +0000 Subject: [PATCH] Add a shim Endpoint if management context is child When management endpoints are on a different port the HandlerMappings are restricted to a single EndpointHandlerMapping, so the error controller (which is a normal @Controller with @RequestMappings) does not get mapped. Fixed by addinga shim Endpoint on "/error" that delegates to the ErrorController (which interface picks up an extra method). --- ...dpointWebMvcChildContextConfiguration.java | 31 +++++++++++++++++++ .../ErrorMvcAutoConfiguration.java | 3 +- .../actuate/web/BasicErrorController.java | 24 +++++++++----- .../boot/actuate/web/ErrorController.java | 12 +++++++ .../SampleActuatorUiApplicationPortTests.java | 7 ++--- ...AddressSampleActuatorApplicationTests.java | 11 ++----- 6 files changed, 66 insertions(+), 22 deletions(-) diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/EndpointWebMvcChildContextConfiguration.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/EndpointWebMvcChildContextConfiguration.java index 44071b5ef77..cb37692695e 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/EndpointWebMvcChildContextConfiguration.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/EndpointWebMvcChildContextConfiguration.java @@ -16,6 +16,8 @@ package org.springframework.boot.actuate.autoconfigure; +import java.util.Map; + import javax.servlet.Filter; import org.springframework.beans.factory.BeanFactory; @@ -23,18 +25,25 @@ import org.springframework.beans.factory.BeanFactoryUtils; import org.springframework.beans.factory.HierarchicalBeanFactory; import org.springframework.beans.factory.ListableBeanFactory; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.boot.actuate.endpoint.AbstractEndpoint; +import org.springframework.boot.actuate.endpoint.Endpoint; import org.springframework.boot.actuate.endpoint.mvc.EndpointHandlerAdapter; import org.springframework.boot.actuate.endpoint.mvc.EndpointHandlerMapping; import org.springframework.boot.actuate.properties.ManagementServerProperties; +import org.springframework.boot.actuate.web.ErrorController; import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.SearchStrategy; import org.springframework.boot.context.embedded.ConfigurableEmbeddedServletContainerFactory; import org.springframework.boot.context.embedded.EmbeddedServletContainer; import org.springframework.boot.context.embedded.EmbeddedServletContainerCustomizer; +import org.springframework.boot.context.embedded.ErrorPage; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity; +import org.springframework.web.context.request.RequestAttributes; +import org.springframework.web.context.request.RequestContextHolder; import org.springframework.web.servlet.DispatcherServlet; import org.springframework.web.servlet.HandlerAdapter; import org.springframework.web.servlet.HandlerMapping; @@ -52,6 +61,9 @@ public class EndpointWebMvcChildContextConfiguration { protected static class ServerCustomization implements EmbeddedServletContainerCustomizer { + @Value("${error.path:/error}") + private String errorPath = "/error"; + @Autowired private ListableBeanFactory beanFactory; @@ -69,6 +81,7 @@ public class EndpointWebMvcChildContextConfiguration { factory.setPort(this.managementServerProperties.getPort()); factory.setAddress(this.managementServerProperties.getAddress()); factory.setContextPath(this.managementServerProperties.getContextPath()); + factory.addErrorPages(new ErrorPage(this.errorPath)); } } @@ -96,6 +109,24 @@ public class EndpointWebMvcChildContextConfiguration { return new EndpointHandlerAdapter(); } + /* + * The error controller is present but not mapped as an endpoint in this context + * because of the DispatcherServlet having had it's HandlerMapping explicitly + * disabled. So this tiny shim exposes the same feature but only for machine + * endpoints. + */ + @Bean + public Endpoint> errorEndpoint(final ErrorController controller) { + return new AbstractEndpoint>("/error", false, true) { + @Override + protected Map doInvoke() { + RequestAttributes attributes = RequestContextHolder + .currentRequestAttributes(); + return controller.extract(attributes, false); + } + }; + } + @Configuration @ConditionalOnClass({ EnableWebSecurity.class, Filter.class }) @ConditionalOnBean(name = "springSecurityFilterChain", search = SearchStrategy.PARENTS) diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/ErrorMvcAutoConfiguration.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/ErrorMvcAutoConfiguration.java index cef6bb7bbef..e13612e15b3 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/ErrorMvcAutoConfiguration.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/ErrorMvcAutoConfiguration.java @@ -33,6 +33,7 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnExpression; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnWebApplication; +import org.springframework.boot.autoconfigure.condition.SearchStrategy; import org.springframework.boot.autoconfigure.condition.SpringBootCondition; import org.springframework.boot.autoconfigure.thymeleaf.ThymeleafAutoConfiguration.DefaultTemplateResolverConfiguration; import org.springframework.boot.autoconfigure.web.WebMvcAutoConfiguration; @@ -72,7 +73,7 @@ public class ErrorMvcAutoConfiguration implements EmbeddedServletContainerCustom private String errorPath = "/error"; @Bean - @ConditionalOnMissingBean(ErrorController.class) + @ConditionalOnMissingBean(value = ErrorController.class, search = SearchStrategy.CURRENT) public BasicErrorController basicErrorController() { return new BasicErrorController(); } diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/web/BasicErrorController.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/web/BasicErrorController.java index 4cda5f4d77d..151a20ce2d9 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/web/BasicErrorController.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/web/BasicErrorController.java @@ -33,6 +33,8 @@ import org.springframework.http.HttpStatus; import org.springframework.stereotype.Controller; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.ResponseBody; +import org.springframework.web.context.request.RequestAttributes; +import org.springframework.web.context.request.ServletRequestAttributes; import org.springframework.web.servlet.ModelAndView; /** @@ -61,19 +63,27 @@ public class BasicErrorController implements ErrorController { @RequestMapping(value = "${error.path:/error}", produces = "text/html") public ModelAndView errorHtml(HttpServletRequest request) { - Map map = error(request); + Map map = extract(new ServletRequestAttributes(request), false); return new ModelAndView(ERROR_KEY, map); } @RequestMapping(value = "${error.path:/error}") @ResponseBody public Map error(HttpServletRequest request) { + ServletRequestAttributes attributes = new ServletRequestAttributes(request); + String trace = request.getParameter("trace"); + return extract(attributes, trace != null && !"false".equals(trace.toLowerCase())); + } + + @Override + public Map extract(RequestAttributes attributes, boolean trace) { Map map = new LinkedHashMap(); map.put("timestamp", new Date()); try { - Throwable error = (Throwable) request - .getAttribute("javax.servlet.error.exception"); - Object obj = request.getAttribute("javax.servlet.error.status_code"); + Throwable error = (Throwable) attributes.getAttribute( + "javax.servlet.error.exception", RequestAttributes.SCOPE_REQUEST); + Object obj = attributes.getAttribute("javax.servlet.error.status_code", + RequestAttributes.SCOPE_REQUEST); int status = 999; if (obj != null) { status = (Integer) obj; @@ -89,8 +99,7 @@ public class BasicErrorController implements ErrorController { } map.put("exception", error.getClass().getName()); map.put("message", error.getMessage()); - String trace = request.getParameter("trace"); - if (trace != null && !"false".equals(trace.toLowerCase())) { + if (trace) { StringWriter stackTrace = new StringWriter(); error.printStackTrace(new PrintWriter(stackTrace)); stackTrace.flush(); @@ -99,7 +108,8 @@ public class BasicErrorController implements ErrorController { this.logger.error(error); } else { - Object message = request.getAttribute("javax.servlet.error.message"); + Object message = attributes.getAttribute("javax.servlet.error.message", + RequestAttributes.SCOPE_REQUEST); map.put("message", message == null ? "No message available" : message); } return map; diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/web/ErrorController.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/web/ErrorController.java index 8e638bbcbf4..7de4555567b 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/web/ErrorController.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/web/ErrorController.java @@ -16,7 +16,10 @@ package org.springframework.boot.actuate.web; +import java.util.Map; + import org.springframework.stereotype.Controller; +import org.springframework.web.context.request.RequestAttributes; /** * Marker interface used to indicate that a {@link Controller @Controller} is used to @@ -31,4 +34,13 @@ public interface ErrorController { */ public String getErrorPath(); + /** + * Extract a useful model of the error from the request attributes. + * + * @param attributes the request attributes + * @param trace flag to indicate that stack trace information should be included + * @return a model containing error messages and codes etc. + */ + public Map extract(RequestAttributes attributes, boolean trace); + } diff --git a/spring-boot-samples/spring-boot-sample-actuator-ui/src/test/java/org/springframework/boot/sample/ops/ui/SampleActuatorUiApplicationPortTests.java b/spring-boot-samples/spring-boot-sample-actuator-ui/src/test/java/org/springframework/boot/sample/ops/ui/SampleActuatorUiApplicationPortTests.java index c05db04bf0c..95487b418ae 100644 --- a/spring-boot-samples/spring-boot-sample-actuator-ui/src/test/java/org/springframework/boot/sample/ops/ui/SampleActuatorUiApplicationPortTests.java +++ b/spring-boot-samples/spring-boot-sample-actuator-ui/src/test/java/org/springframework/boot/sample/ops/ui/SampleActuatorUiApplicationPortTests.java @@ -16,6 +16,8 @@ package org.springframework.boot.sample.ops.ui; +import static org.junit.Assert.assertEquals; + import java.io.IOException; import java.util.Map; import java.util.concurrent.Callable; @@ -25,7 +27,6 @@ import java.util.concurrent.TimeUnit; import org.junit.AfterClass; import org.junit.BeforeClass; -import org.junit.Ignore; import org.junit.Test; import org.springframework.boot.SpringApplication; import org.springframework.context.ConfigurableApplicationContext; @@ -35,8 +36,6 @@ import org.springframework.http.client.ClientHttpResponse; import org.springframework.web.client.DefaultResponseErrorHandler; import org.springframework.web.client.RestTemplate; -import static org.junit.Assert.assertEquals; - /** * Integration tests for separate management and main service ports. * @@ -80,9 +79,7 @@ public class SampleActuatorUiApplicationPortTests { } @Test - @Ignore public void testMetrics() throws Exception { - // FIXME broken since error page is not rendered @SuppressWarnings("rawtypes") ResponseEntity entity = getRestTemplate().getForEntity( "http://localhost:" + managementPort + "/metrics", Map.class); diff --git a/spring-boot-samples/spring-boot-sample-actuator/src/test/java/org/springframework/boot/sample/ops/ManagementAddressSampleActuatorApplicationTests.java b/spring-boot-samples/spring-boot-sample-actuator/src/test/java/org/springframework/boot/sample/ops/ManagementAddressSampleActuatorApplicationTests.java index 3e75fc36934..7456e49fc9f 100644 --- a/spring-boot-samples/spring-boot-sample-actuator/src/test/java/org/springframework/boot/sample/ops/ManagementAddressSampleActuatorApplicationTests.java +++ b/spring-boot-samples/spring-boot-sample-actuator/src/test/java/org/springframework/boot/sample/ops/ManagementAddressSampleActuatorApplicationTests.java @@ -16,6 +16,8 @@ package org.springframework.boot.sample.ops; +import static org.junit.Assert.assertEquals; + import java.io.IOException; import java.util.ArrayList; import java.util.List; @@ -27,7 +29,6 @@ import java.util.concurrent.TimeUnit; import org.junit.AfterClass; import org.junit.BeforeClass; -import org.junit.Ignore; import org.junit.Test; import org.springframework.boot.SpringApplication; import org.springframework.boot.autoconfigure.security.SecurityProperties; @@ -44,8 +45,6 @@ import org.springframework.security.crypto.codec.Base64; import org.springframework.web.client.DefaultResponseErrorHandler; import org.springframework.web.client.RestTemplate; -import static org.junit.Assert.assertEquals; - /** * Integration tests for separate management and main service ports. * @@ -93,9 +92,7 @@ public class ManagementAddressSampleActuatorApplicationTests { } @Test - @Ignore public void testMetrics() throws Exception { - // FIXME broken because error page is no longer exposed on management port testHome(); // makes sure some requests have been made @SuppressWarnings("rawtypes") ResponseEntity entity = getRestTemplate().getForEntity( @@ -104,9 +101,7 @@ public class ManagementAddressSampleActuatorApplicationTests { } @Test - @Ignore public void testHealth() throws Exception { - // FIXME broken because error page is no longer exposed on management port ResponseEntity entity = getRestTemplate().getForEntity( "http://localhost:" + managementPort + "/health", String.class); assertEquals(HttpStatus.OK, entity.getStatusCode()); @@ -114,9 +109,7 @@ public class ManagementAddressSampleActuatorApplicationTests { } @Test - @Ignore public void testErrorPage() throws Exception { - // FIXME broken because error page is no longer exposed on management port @SuppressWarnings("rawtypes") ResponseEntity entity = getRestTemplate().getForEntity( "http://localhost:" + managementPort + "/error", Map.class);