From af612782131fffd64d5796329bf816a7cb59c964 Mon Sep 17 00:00:00 2001 From: Madhura Bhave Date: Tue, 8 Nov 2016 15:04:35 -0800 Subject: [PATCH] Extend HealthMvcEndpoint for Cloud Foundry The CloudFoundryHealthMvcEndpoint does not perform additional security checks since security is handled by the interceptor. See gh-7108 --- .../actuate/cloudfoundry/AccessLevel.java | 2 +- .../CloudFoundryEndpointHandlerMapping.java | 13 ++++- .../CloudFoundryHealthMvcEndpoint.java | 43 ++++++++++++++++ .../endpoint/mvc/HealthMvcEndpoint.java | 2 +- ...oudFoundryEndpointHandlerMappingTests.java | 29 +++++++++++ .../CloudFoundryHealthMvcEndpointTests.java | 51 +++++++++++++++++++ 6 files changed, 137 insertions(+), 3 deletions(-) create mode 100644 spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundryHealthMvcEndpoint.java create mode 100644 spring-boot-actuator/src/test/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundryHealthMvcEndpointTests.java diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/AccessLevel.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/AccessLevel.java index fe556f2a173..3bd7e7e4a22 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/AccessLevel.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/AccessLevel.java @@ -49,7 +49,7 @@ enum AccessLevel { /** * Returns if the access level should allow access to the specified endpoint path. - * @param endpointPath the endpoitn path + * @param endpointPath the endpoint path * @return {@code true} if access is allowed */ public boolean isAccessAllowed(String endpointPath) { diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundryEndpointHandlerMapping.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundryEndpointHandlerMapping.java index 3f10756e9c0..6a9555f53da 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundryEndpointHandlerMapping.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundryEndpointHandlerMapping.java @@ -27,6 +27,7 @@ import javax.servlet.http.HttpServletRequest; import org.springframework.boot.actuate.endpoint.Endpoint; import org.springframework.boot.actuate.endpoint.mvc.AbstractEndpointHandlerMapping; import org.springframework.boot.actuate.endpoint.mvc.HalJsonMvcEndpoint; +import org.springframework.boot.actuate.endpoint.mvc.HealthMvcEndpoint; import org.springframework.boot.actuate.endpoint.mvc.MvcEndpoint; import org.springframework.boot.actuate.endpoint.mvc.NamedMvcEndpoint; import org.springframework.web.cors.CorsConfiguration; @@ -54,10 +55,20 @@ class CloudFoundryEndpointHandlerMapping protected void postProcessEndpoints(Set endpoints) { super.postProcessEndpoints(endpoints); Iterator iterator = endpoints.iterator(); + HealthMvcEndpoint healthMvcEndpoint = null; while (iterator.hasNext()) { - if (iterator.next() instanceof HalJsonMvcEndpoint) { + NamedMvcEndpoint endpoint = iterator.next(); + if (endpoint instanceof HalJsonMvcEndpoint) { iterator.remove(); } + else if (endpoint instanceof HealthMvcEndpoint) { + iterator.remove(); + healthMvcEndpoint = (HealthMvcEndpoint) endpoint; + } + } + if (healthMvcEndpoint != null) { + endpoints.add( + new CloudFoundryHealthMvcEndpoint(healthMvcEndpoint.getDelegate())); } } diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundryHealthMvcEndpoint.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundryHealthMvcEndpoint.java new file mode 100644 index 00000000000..7d25eb22342 --- /dev/null +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundryHealthMvcEndpoint.java @@ -0,0 +1,43 @@ +/* + * Copyright 2012-2016 the original author or authors. + * + * Licensed 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.springframework.boot.actuate.cloudfoundry; + +import java.security.Principal; + +import org.springframework.boot.actuate.endpoint.HealthEndpoint; +import org.springframework.boot.actuate.endpoint.mvc.HealthMvcEndpoint; + +/** + * Extension of {@link HealthMvcEndpoint} for Cloud Foundry. Since security for Cloud + * Foundry actuators is already handled by the {@link CloudFoundrySecurityInterceptor}, + * this endpoint skips the additional security checks done by the regular + * {@link HealthMvcEndpoint}. + * + * @author Madhura Bhave + */ +class CloudFoundryHealthMvcEndpoint extends HealthMvcEndpoint { + + CloudFoundryHealthMvcEndpoint(HealthEndpoint delegate) { + super(delegate); + } + + @Override + protected boolean exposeHealthDetails(Principal principal) { + return true; + } + +} diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/HealthMvcEndpoint.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/HealthMvcEndpoint.java index 02e2e510324..374ac3d577f 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/HealthMvcEndpoint.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/HealthMvcEndpoint.java @@ -178,7 +178,7 @@ public class HealthMvcEndpoint extends AbstractEndpointMvcAdapter= getDelegate().getTimeToLive(); } - private boolean exposeHealthDetails(Principal principal) { + protected boolean exposeHealthDetails(Principal principal) { return isSecure(principal) || isUnrestricted(); } diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundryEndpointHandlerMappingTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundryEndpointHandlerMappingTests.java index 613a59b60b0..a83559390ef 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundryEndpointHandlerMappingTests.java +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundryEndpointHandlerMappingTests.java @@ -22,11 +22,15 @@ import org.junit.Test; import org.mockito.Mockito; import org.springframework.boot.actuate.endpoint.AbstractEndpoint; +import org.springframework.boot.actuate.endpoint.HealthEndpoint; import org.springframework.boot.actuate.endpoint.mvc.AbstractEndpointHandlerMappingTests; import org.springframework.boot.actuate.endpoint.mvc.EndpointMvcAdapter; import org.springframework.boot.actuate.endpoint.mvc.HalJsonMvcEndpoint; +import org.springframework.boot.actuate.endpoint.mvc.HealthMvcEndpoint; import org.springframework.boot.actuate.endpoint.mvc.ManagementServletContext; import org.springframework.boot.actuate.endpoint.mvc.NamedMvcEndpoint; +import org.springframework.boot.actuate.health.HealthIndicator; +import org.springframework.boot.actuate.health.OrderedHealthAggregator; import org.springframework.context.support.StaticApplicationContext; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.web.method.HandlerMethod; @@ -88,6 +92,23 @@ public class CloudFoundryEndpointHandlerMappingTests .isInstanceOf(CloudFoundryDiscoveryMvcEndpoint.class); } + @Test + public void registersCloudFoundryHealthEndpoint() throws Exception { + StaticApplicationContext context = new StaticApplicationContext(); + HealthEndpoint delegate = new HealthEndpoint(new OrderedHealthAggregator(), + Collections.emptyMap()); + CloudFoundryEndpointHandlerMapping handlerMapping = new CloudFoundryEndpointHandlerMapping( + Collections.singleton(new TestHealthMvcEndpoint(delegate)), null, null); + handlerMapping.setPrefix("/test"); + handlerMapping.setApplicationContext(context); + handlerMapping.afterPropertiesSet(); + HandlerExecutionChain handler = handlerMapping + .getHandler(new MockHttpServletRequest("GET", "/test/health")); + HandlerMethod handlerMethod = (HandlerMethod) handler.getHandler(); + Object handlerMethodBean = handlerMethod.getBean(); + assertThat(handlerMethodBean).isInstanceOf(CloudFoundryHealthMvcEndpoint.class); + } + private static class TestEndpoint extends AbstractEndpoint { TestEndpoint(String id) { @@ -124,4 +145,12 @@ public class CloudFoundryEndpointHandlerMappingTests } + private static class TestHealthMvcEndpoint extends HealthMvcEndpoint { + + TestHealthMvcEndpoint(HealthEndpoint delegate) { + super(delegate); + } + + } + } diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundryHealthMvcEndpointTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundryHealthMvcEndpointTests.java new file mode 100644 index 00000000000..d885cd706a7 --- /dev/null +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundryHealthMvcEndpointTests.java @@ -0,0 +1,51 @@ +/* + * Copyright 2012-2016 the original author or authors. + * + * Licensed 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.springframework.boot.actuate.cloudfoundry; + +import org.junit.Test; + +import org.springframework.boot.actuate.endpoint.HealthEndpoint; +import org.springframework.boot.actuate.health.Health; +import org.springframework.boot.actuate.health.Status; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; + +/** + * Tests for {@link CloudFoundryHealthMvcEndpoint}. + * + * @author Madhura Bhave + */ +public class CloudFoundryHealthMvcEndpointTests { + + @Test + public void cloudFoundryHealthEndpointShouldAlwaysReturnAllHealthDetails() + throws Exception { + HealthEndpoint endpoint = mock(HealthEndpoint.class); + given(endpoint.isEnabled()).willReturn(true); + CloudFoundryHealthMvcEndpoint mvc = new CloudFoundryHealthMvcEndpoint(endpoint); + given(endpoint.invoke()) + .willReturn(new Health.Builder().up().withDetail("foo", "bar").build()); + given(endpoint.isSensitive()).willReturn(false); + Object result = mvc.invoke(null); + assertThat(result instanceof Health).isTrue(); + assertThat(((Health) result).getStatus() == Status.UP).isTrue(); + assertThat(((Health) result).getDetails().get("foo")).isEqualTo("bar"); + } + +}