From 8fd9eb72d4d14f5caea16beb02a370c8f3cab7cf Mon Sep 17 00:00:00 2001 From: Madhura Bhave Date: Tue, 17 Aug 2021 14:10:35 -0700 Subject: [PATCH] Allow part of a composite contributor in a health group Closes gh-23027 Co-authored-by: Phillip Webb --- .../IncludeExcludeGroupMemberPredicate.java | 22 ++++- ...cludeExcludeGroupMemberPredicateTests.java | 18 ++++ .../actuate/health/HealthEndpointSupport.java | 51 +++++++---- .../health/NamedContributorsMapAdapter.java | 8 +- .../health/HealthEndpointSupportTests.java | 87 +++++++++++++++++++ .../NamedContributorsMapAdapterTests.java | 8 ++ .../src/docs/asciidoc/actuator/endpoints.adoc | 14 +++ .../actuator/SampleActuatorApplication.java | 27 +++++- .../src/main/resources/application.properties | 4 + ...AndPathSampleActuatorApplicationTests.java | 11 ++- 10 files changed, 226 insertions(+), 24 deletions(-) diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/health/IncludeExcludeGroupMemberPredicate.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/health/IncludeExcludeGroupMemberPredicate.java index 463fbea66b..0f21e812f6 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/health/IncludeExcludeGroupMemberPredicate.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/health/IncludeExcludeGroupMemberPredicate.java @@ -26,6 +26,7 @@ import java.util.stream.Collectors; * Member predicate that matches based on {@code include} and {@code exclude} sets. * * @author Phillip Webb + * @author Madhura Bhave */ class IncludeExcludeGroupMemberPredicate implements Predicate { @@ -40,15 +41,30 @@ class IncludeExcludeGroupMemberPredicate implements Predicate { @Override public boolean test(String name) { + return testCleanName(clean(name)); + } + + private boolean testCleanName(String name) { return isIncluded(name) && !isExcluded(name); } private boolean isIncluded(String name) { - return this.include.isEmpty() || this.include.contains("*") || this.include.contains(clean(name)); + return this.include.isEmpty() || this.include.contains("*") || isIncludedName(name); + } + + private boolean isIncludedName(String name) { + if (this.include.contains(name)) { + return true; + } + if (name.contains("/")) { + String parent = name.substring(0, name.lastIndexOf("/")); + return isIncludedName(parent); + } + return false; } private boolean isExcluded(String name) { - return this.exclude.contains("*") || this.exclude.contains(clean(name)); + return this.exclude.contains("*") || this.exclude.contains(name); } private Set clean(Set names) { @@ -60,7 +76,7 @@ class IncludeExcludeGroupMemberPredicate implements Predicate { } private String clean(String name) { - return name.trim(); + return (name != null) ? name.trim() : null; } } diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/health/IncludeExcludeGroupMemberPredicateTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/health/IncludeExcludeGroupMemberPredicateTests.java index b61c3fa556..bdc23c0902 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/health/IncludeExcludeGroupMemberPredicateTests.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/health/IncludeExcludeGroupMemberPredicateTests.java @@ -94,6 +94,24 @@ class IncludeExcludeGroupMemberPredicateTests { assertThat(predicate).accepts("myEndpoint").rejects("d"); } + @Test + void testWhenSpecifiedIncludeWithSlash() { + Predicate predicate = include("test/a").exclude(); + assertThat(predicate).accepts("test/a").rejects("test").rejects("test/b"); + } + + @Test + void specifiedIncludeShouldIncludeNested() { + Predicate predicate = include("test").exclude(); + assertThat(predicate).accepts("test/a/d").accepts("test/b").rejects("foo"); + } + + @Test + void specifiedIncludeShouldNotIncludeExcludedNested() { + Predicate predicate = include("test").exclude("test/b"); + assertThat(predicate).accepts("test/a").rejects("test/b").rejects("foo"); + } + private Builder include(String... include) { return new Builder(include); } diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/HealthEndpointSupport.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/HealthEndpointSupport.java index 17b5197017..ee01e3fd2b 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/HealthEndpointSupport.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/HealthEndpointSupport.java @@ -25,6 +25,7 @@ import org.springframework.boot.actuate.endpoint.ApiVersion; import org.springframework.boot.actuate.endpoint.SecurityContext; import org.springframework.boot.actuate.endpoint.web.WebServerNamespace; import org.springframework.util.Assert; +import org.springframework.util.StringUtils; /** * Base class for health endpoints and health endpoint extensions. @@ -86,8 +87,12 @@ abstract class HealthEndpointSupport { return null; } Object contributor = getContributor(path, pathOffset); + if (contributor == null) { + return null; + } + String name = getName(path, pathOffset); Set groupNames = isSystemHealth ? this.groups.getNames() : null; - T health = getContribution(apiVersion, group, contributor, showComponents, showDetails, groupNames, false); + T health = getContribution(apiVersion, group, name, contributor, showComponents, showDetails, groupNames); return (health != null) ? new HealthResult<>(health, group) : null; } @@ -104,29 +109,39 @@ abstract class HealthEndpointSupport { return contributor; } + private String getName(String[] path, int pathOffset) { + StringBuilder name = new StringBuilder(); + while (pathOffset < path.length) { + name.append((name.length() != 0) ? "/" : ""); + name.append(path[pathOffset]); + pathOffset++; + } + return name.toString(); + } + @SuppressWarnings("unchecked") - private T getContribution(ApiVersion apiVersion, HealthEndpointGroup group, Object contributor, - boolean showComponents, boolean showDetails, Set groupNames, boolean isNested) { + private T getContribution(ApiVersion apiVersion, HealthEndpointGroup group, String name, Object contributor, + boolean showComponents, boolean showDetails, Set groupNames) { if (contributor instanceof NamedContributors) { - return getAggregateHealth(apiVersion, group, (NamedContributors) contributor, showComponents, - showDetails, groupNames, isNested); + return getAggregateContribution(apiVersion, group, name, (NamedContributors) contributor, showComponents, + showDetails, groupNames); } - return (contributor != null) ? getHealth((C) contributor, showDetails) : null; + if (contributor != null && (name.isEmpty() || group.isMember(name))) { + return getHealth((C) contributor, showDetails); + } + return null; } - private T getAggregateHealth(ApiVersion apiVersion, HealthEndpointGroup group, - NamedContributors namedContributors, boolean showComponents, boolean showDetails, Set groupNames, - boolean isNested) { + private T getAggregateContribution(ApiVersion apiVersion, HealthEndpointGroup group, String name, + NamedContributors namedContributors, boolean showComponents, boolean showDetails, + Set groupNames) { + String prefix = (StringUtils.hasText(name)) ? name + "/" : ""; Map contributions = new LinkedHashMap<>(); - for (NamedContributor namedContributor : namedContributors) { - String name = namedContributor.getName(); - C contributor = namedContributor.getContributor(); - if (group.isMember(name) || isNested) { - T contribution = getContribution(apiVersion, group, contributor, showComponents, showDetails, null, - true); - if (contribution != null) { - contributions.put(name, contribution); - } + for (NamedContributor child : namedContributors) { + T contribution = getContribution(apiVersion, group, prefix + child.getName(), child.getContributor(), + showComponents, showDetails, null); + if (contribution != null) { + contributions.put(child.getName(), contribution); } } if (contributions.isEmpty()) { diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/NamedContributorsMapAdapter.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/NamedContributorsMapAdapter.java index 68ceb77daa..24af8d423f 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/NamedContributorsMapAdapter.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/NamedContributorsMapAdapter.java @@ -43,13 +43,19 @@ abstract class NamedContributorsMapAdapter implements NamedContributors NamedContributorsMapAdapter(Map map, Function valueAdapter) { Assert.notNull(map, "Map must not be null"); Assert.notNull(valueAdapter, "ValueAdapter must not be null"); - map.keySet().forEach((key) -> Assert.notNull(key, "Map must not contain null keys")); + map.keySet().forEach(this::validateKey); map.values().stream().map(valueAdapter) .forEach((value) -> Assert.notNull(value, "Map must not contain null values")); this.map = Collections.unmodifiableMap(new LinkedHashMap<>(map)); this.valueAdapter = valueAdapter; } + private void validateKey(String value) { + Assert.notNull(value, "Map must not contain null keys"); + Assert.isTrue(!value.contains("/"), "Map keys must not contain a '/'"); + + } + @Override public Iterator> iterator() { Iterator> iterator = this.map.entrySet().iterator(); diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/health/HealthEndpointSupportTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/health/HealthEndpointSupportTests.java index 40a7d5dd98..70c54f6dcd 100644 --- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/health/HealthEndpointSupportTests.java +++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/health/HealthEndpointSupportTests.java @@ -19,6 +19,7 @@ package org.springframework.boot.actuate.health; import java.util.Collections; import java.util.LinkedHashMap; import java.util.Map; +import java.util.function.Predicate; import org.junit.jupiter.api.Test; @@ -224,6 +225,92 @@ abstract class HealthEndpointSupportTests, C, T assertThat(health.getComponents()).containsKey("test"); } + @Test + void getHealthWhenGroupContainsComponentOfCompositeContributorReturnsHealth() { + CompositeHealth health = getCompositeHealth((name) -> name.equals("test/spring-1")); + assertThat(health.getComponents()).containsKey("test"); + CompositeHealth test = (CompositeHealth) health.getComponents().get("test"); + assertThat(test.getComponents()).containsKey("spring-1"); + assertThat(test.getComponents()).doesNotContainKey("spring-2"); + assertThat(test.getComponents()).doesNotContainKey("test"); + } + + @Test + void getHealthWhenGroupExcludesComponentOfCompositeContributorReturnsHealth() { + CompositeHealth health = getCompositeHealth( + (name) -> name.startsWith("test/") && !name.equals("test/spring-2")); + assertThat(health.getComponents()).containsKey("test"); + CompositeHealth test = (CompositeHealth) health.getComponents().get("test"); + assertThat(test.getComponents()).containsKey("spring-1"); + assertThat(test.getComponents()).doesNotContainKey("spring-2"); + } + + @Test + void getHealthForPathWhenGroupContainsComponentOfCompositeContributorReturnsHealth() { + Map contributors = new LinkedHashMap<>(); + contributors.put("spring-1", createNestedHealthContributor("spring-1")); + contributors.put("spring-2", createNestedHealthContributor("spring-2")); + C compositeContributor = createCompositeContributor(contributors); + this.registry.registerContributor("test", compositeContributor); + TestHealthEndpointGroup testGroup = new TestHealthEndpointGroup( + (name) -> name.startsWith("test") && !name.equals("test/spring-1/b")); + HealthEndpointGroups groups = HealthEndpointGroups.of(this.primaryGroup, + Collections.singletonMap("testGroup", testGroup)); + HealthResult result = create(this.registry, groups).getHealth(ApiVersion.V3, null, SecurityContext.NONE, + false, "testGroup", "test"); + CompositeHealth health = (CompositeHealth) getHealth(result); + assertThat(health.getComponents()).containsKey("spring-1"); + assertThat(health.getComponents()).containsKey("spring-2"); + CompositeHealth spring1 = (CompositeHealth) health.getComponents().get("spring-1"); + CompositeHealth spring2 = (CompositeHealth) health.getComponents().get("spring-2"); + assertThat(spring1.getComponents()).containsKey("a"); + assertThat(spring1.getComponents()).containsKey("c"); + assertThat(spring1.getComponents()).doesNotContainKey("b"); + assertThat(spring2.getComponents()).containsKey("a"); + assertThat(spring2.getComponents()).containsKey("c"); + assertThat(spring2.getComponents()).containsKey("b"); + } + + @Test + void getHealthForComponentPathWhenNotPartOfGroup() { + Map contributors = new LinkedHashMap<>(); + contributors.put("spring-1", createNestedHealthContributor("spring-1")); + contributors.put("spring-2", createNestedHealthContributor("spring-2")); + C compositeContributor = createCompositeContributor(contributors); + this.registry.registerContributor("test", compositeContributor); + TestHealthEndpointGroup testGroup = new TestHealthEndpointGroup( + (name) -> name.startsWith("test") && !name.equals("test/spring-1/b")); + HealthEndpointGroups groups = HealthEndpointGroups.of(this.primaryGroup, + Collections.singletonMap("testGroup", testGroup)); + HealthResult result = create(this.registry, groups).getHealth(ApiVersion.V3, null, SecurityContext.NONE, + false, "testGroup", "test", "spring-1", "b"); + assertThat(result).isNull(); + } + + private CompositeHealth getCompositeHealth(Predicate memberPredicate) { + C contributor1 = createContributor(this.up); + C contributor2 = createContributor(this.down); + Map contributors = new LinkedHashMap<>(); + contributors.put("spring-1", contributor1); + contributors.put("spring-2", contributor2); + C compositeContributor = createCompositeContributor(contributors); + this.registry.registerContributor("test", compositeContributor); + TestHealthEndpointGroup testGroup = new TestHealthEndpointGroup(memberPredicate); + HealthEndpointGroups groups = HealthEndpointGroups.of(this.primaryGroup, + Collections.singletonMap("testGroup", testGroup)); + HealthResult result = create(this.registry, groups).getHealth(ApiVersion.V3, null, SecurityContext.NONE, + false, "testGroup"); + return (CompositeHealth) getHealth(result); + } + + private C createNestedHealthContributor(String name) { + Map map = new LinkedHashMap<>(); + map.put("a", createContributor(Health.up().withDetail("hello", name + "-a").build())); + map.put("b", createContributor(Health.up().withDetail("hello", name + "-b").build())); + map.put("c", createContributor(Health.up().withDetail("hello", name + "-c").build())); + return createCompositeContributor(map); + } + @Test void getHealthWhenGroupHasAdditionalPath() { this.registry.registerContributor("test", createContributor(this.up)); diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/health/NamedContributorsMapAdapterTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/health/NamedContributorsMapAdapterTests.java index 8643d3bdff..b151d45e09 100644 --- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/health/NamedContributorsMapAdapterTests.java +++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/health/NamedContributorsMapAdapterTests.java @@ -64,6 +64,14 @@ class NamedContributorsMapAdapterTests { .withMessage("Map must not contain null keys"); } + @Test + void createWhenMapContainsKeyWithSlashThrowsException() { + assertThatIllegalArgumentException() + .isThrownBy(() -> new TestNamedContributorsMapAdapter<>(Collections.singletonMap("test/key", "test"), + Function.identity())) + .withMessage("Map keys must not contain a '/'"); + } + @Test void iterateReturnsAdaptedEntries() { TestNamedContributorsMapAdapter adapter = createAdapter(); diff --git a/spring-boot-project/spring-boot-docs/src/docs/asciidoc/actuator/endpoints.adoc b/spring-boot-project/spring-boot-docs/src/docs/asciidoc/actuator/endpoints.adoc index bc589f6091..386c491b99 100644 --- a/spring-boot-project/spring-boot-docs/src/docs/asciidoc/actuator/endpoints.adoc +++ b/spring-boot-project/spring-boot-docs/src/docs/asciidoc/actuator/endpoints.adoc @@ -934,6 +934,20 @@ It's also possible to override the `show-details` and `roles` properties if requ TIP: You can use `@Qualifier("groupname")` if you need to register custom `StatusAggregator` or `HttpCodeStatusMapper` beans for use with the group. +A health group can also include/exclude a `CompositeHealthContributor`. +You can also include/exclude only a certain component of a `CompositeHealthContributor`. +This can be done using the fully qualified name of the component as follows: + +[source,properties,indent=0,subs="verbatim"] +---- + management.endpoint.health.group.custom.include="test/primary" + management.endpoint.health.group.custom.exclude="test/primary/b" +---- + +In the example above, the `custom` group will include the `HealthContributor` with the name `primary` which is a component of the composite `test`. +Here, `primary` itself is a composite and the `HealthContributor` with the name `b` will be excluded from the `custom` group. + + Health groups can be made available at an additional path on either the main or management port. This is useful in cloud environments such as Kubernetes, where it is quite common to use a separate management port for the actuator endpoints for security purposes. Having a separate port could lead to unreliable health checks because the main application might not work properly even if the health check is successful. diff --git a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-actuator/src/main/java/smoketest/actuator/SampleActuatorApplication.java b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-actuator/src/main/java/smoketest/actuator/SampleActuatorApplication.java index 18bceb8769..f659e62d3d 100644 --- a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-actuator/src/main/java/smoketest/actuator/SampleActuatorApplication.java +++ b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-actuator/src/main/java/smoketest/actuator/SampleActuatorApplication.java @@ -16,8 +16,13 @@ package smoketest.actuator; +import java.util.LinkedHashMap; +import java.util.Map; + import org.springframework.boot.SpringApplication; +import org.springframework.boot.actuate.health.CompositeHealthContributor; import org.springframework.boot.actuate.health.Health; +import org.springframework.boot.actuate.health.HealthContributor; import org.springframework.boot.actuate.health.HealthIndicator; import org.springframework.boot.autoconfigure.SpringBootApplication; import org.springframework.boot.context.properties.ConfigurationPropertiesScan; @@ -33,7 +38,27 @@ public class SampleActuatorApplication { @Bean public HealthIndicator helloHealthIndicator() { - return () -> Health.up().withDetail("hello", "world").build(); + return createHealthIndicator("world"); + } + + @Bean + public HealthContributor compositeHelloHealthContributor() { + Map map = new LinkedHashMap<>(); + map.put("spring", createNestedHealthContributor("spring")); + map.put("boot", createNestedHealthContributor("boot")); + return CompositeHealthContributor.fromMap(map); + } + + private HealthContributor createNestedHealthContributor(String name) { + Map map = new LinkedHashMap<>(); + map.put("a", createHealthIndicator(name + "-a")); + map.put("b", createHealthIndicator(name + "-b")); + map.put("c", createHealthIndicator(name + "-c")); + return CompositeHealthContributor.fromMap(map); + } + + private HealthIndicator createHealthIndicator(String value) { + return () -> Health.up().withDetail("hello", value).build(); } } diff --git a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-actuator/src/main/resources/application.properties b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-actuator/src/main/resources/application.properties index b41443e593..9b4f59ab8a 100644 --- a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-actuator/src/main/resources/application.properties +++ b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-actuator/src/main/resources/application.properties @@ -23,4 +23,8 @@ management.endpoint.health.show-details=always management.endpoint.health.group.ready.include=db,diskSpace management.endpoint.health.group.live.include=example,hello,db management.endpoint.health.group.live.show-details=never +management.endpoint.health.group.comp.include=compositeHello/spring/a,compositeHello/spring/c +management.endpoint.health.group.comp.show-details=always + management.endpoints.migrate-legacy-ids=true + diff --git a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-actuator/src/test/java/smoketest/actuator/AbstractManagementPortAndPathSampleActuatorApplicationTests.java b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-actuator/src/test/java/smoketest/actuator/AbstractManagementPortAndPathSampleActuatorApplicationTests.java index 578d6ebd8c..ac9e1b5bb0 100644 --- a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-actuator/src/test/java/smoketest/actuator/AbstractManagementPortAndPathSampleActuatorApplicationTests.java +++ b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-actuator/src/test/java/smoketest/actuator/AbstractManagementPortAndPathSampleActuatorApplicationTests.java @@ -67,7 +67,16 @@ abstract class AbstractManagementPortAndPathSampleActuatorApplicationTests { ResponseEntity entity = new TestRestTemplate().withBasicAuth("user", "password") .getForEntity("http://localhost:" + this.managementPort + "/admin/health", String.class); assertThat(entity.getStatusCode()).isEqualTo(HttpStatus.OK); - assertThat(entity.getBody()).isEqualTo("{\"status\":\"UP\",\"groups\":[\"live\",\"ready\"]}"); + assertThat(entity.getBody()).isEqualTo("{\"status\":\"UP\",\"groups\":[\"comp\",\"live\",\"ready\"]}"); + } + + @Test + void testGroupWithComposite() { + ResponseEntity entity = new TestRestTemplate().withBasicAuth("user", "password") + .getForEntity("http://localhost:" + this.managementPort + "/admin/health/comp", String.class); + assertThat(entity.getStatusCode()).isEqualTo(HttpStatus.OK); + assertThat(entity.getBody()).contains( + "components\":{\"a\":{\"status\":\"UP\",\"details\":{\"hello\":\"spring-a\"}},\"c\":{\"status\":\"UP\",\"details\":{\"hello\":\"spring-c\"}}"); } @Test