From 323a78c4b93da696e80008a215697d3edd06df5a Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Tue, 24 Sep 2019 17:56:36 -0700 Subject: [PATCH] Add property to migrate deprecated endoint IDs Allow legacy actuator endpoint IDs that contain dots to be transparently migrated to the new format. This update will allow Spring Cloud users to proactively migrate from endpoints such as `hystrix.stream` to `hystrixstream`. Closes gh-18148 --- .../condition/AbstractEndpointCondition.java | 2 +- .../OnAvailableEndpointCondition.java | 2 +- .../boot/actuate/endpoint/EndpointId.java | 24 +++++++++++++ .../annotation/EndpointDiscoverer.java | 13 +++---- ...itional-spring-configuration-metadata.json | 10 ++++++ .../actuate/endpoint/EndpointIdTests.java | 11 ++++++ .../actuator/SampleLegacyEndpoint.java | 35 +++++++++++++++++++ .../src/main/resources/application.properties | 1 + .../SampleActuatorApplicationTests.java | 12 +++++++ 9 files changed, 102 insertions(+), 8 deletions(-) create mode 100644 spring-boot-project/spring-boot-actuator/src/main/resources/META-INF/additional-spring-configuration-metadata.json create mode 100644 spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-actuator/src/main/java/smoketest/actuator/SampleLegacyEndpoint.java diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/endpoint/condition/AbstractEndpointCondition.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/endpoint/condition/AbstractEndpointCondition.java index 1747da82b64..b8e9a76ecdf 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/endpoint/condition/AbstractEndpointCondition.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/endpoint/condition/AbstractEndpointCondition.java @@ -62,7 +62,7 @@ abstract class AbstractEndpointCondition extends SpringBootCondition { Class annotationClass) { Environment environment = context.getEnvironment(); AnnotationAttributes attributes = getEndpointAttributes(annotationClass, context, metadata); - EndpointId id = EndpointId.of(attributes.getString("id")); + EndpointId id = EndpointId.of(environment, attributes.getString("id")); String key = "management.endpoint." + id.toLowerCaseString() + ".enabled"; Boolean userDefinedEnabled = environment.getProperty(key, Boolean.class); if (userDefinedEnabled != null) { diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/endpoint/condition/OnAvailableEndpointCondition.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/endpoint/condition/OnAvailableEndpointCondition.java index 36809efa2dc..b13faff3d78 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/endpoint/condition/OnAvailableEndpointCondition.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/endpoint/condition/OnAvailableEndpointCondition.java @@ -62,7 +62,7 @@ class OnAvailableEndpointCondition extends AbstractEndpointCondition { } AnnotationAttributes attributes = getEndpointAttributes(ConditionalOnAvailableEndpoint.class, context, metadata); - EndpointId id = EndpointId.of(attributes.getString("id")); + EndpointId id = EndpointId.of(environment, attributes.getString("id")); Set exposureInformations = getExposureInformation(environment); for (ExposureInformation exposureInformation : exposureInformations) { if (exposureInformation.isExposed(id)) { diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/EndpointId.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/EndpointId.java index b39d33e5420..4df8a27f059 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/EndpointId.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/EndpointId.java @@ -24,6 +24,7 @@ import java.util.regex.Pattern; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.springframework.core.env.Environment; import org.springframework.util.Assert; /** @@ -44,6 +45,8 @@ public final class EndpointId { private static final Pattern WARNING_PATTERN = Pattern.compile("[\\.\\-]+"); + private static final String MIGRATE_LEGACY_NAMES_PROPRTY = "management.endpoints.migrate-legacy-ids"; + private final String value; private final String lowerCaseValue; @@ -112,6 +115,27 @@ public final class EndpointId { return new EndpointId(value); } + /** + * Factory method to create a new {@link EndpointId} of the specified value. This + * variant will respect the {@code management.endpoints.migrate-legacy-names} property + * if it has been set in the {@link Environment}. + * @param environment the Spring environment + * @param value the endpoint ID value + * @return an {@link EndpointId} instance + * @since 2.2.0 + */ + public static EndpointId of(Environment environment, String value) { + Assert.notNull(environment, "Environment must not be null"); + return new EndpointId(migrateLegacyId(environment, value)); + } + + private static String migrateLegacyId(Environment environment, String value) { + if (environment.getProperty(MIGRATE_LEGACY_NAMES_PROPRTY, Boolean.class, false)) { + return value.replace(".", ""); + } + return value; + } + /** * Factory method to create a new {@link EndpointId} from a property value. More * lenient than {@link #of(String)} to allow for common "relaxed" property variants. diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/annotation/EndpointDiscoverer.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/annotation/EndpointDiscoverer.java index bc966fc5bc5..01259e98f58 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/annotation/EndpointDiscoverer.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/annotation/EndpointDiscoverer.java @@ -47,6 +47,7 @@ import org.springframework.core.ResolvableType; import org.springframework.core.annotation.MergedAnnotation; import org.springframework.core.annotation.MergedAnnotations; import org.springframework.core.annotation.MergedAnnotations.SearchStrategy; +import org.springframework.core.env.Environment; import org.springframework.util.Assert; import org.springframework.util.CollectionUtils; import org.springframework.util.LinkedMultiValueMap; @@ -140,7 +141,7 @@ public abstract class EndpointDiscoverer, O exten private EndpointBean createEndpointBean(String beanName) { Object bean = this.applicationContext.getBean(beanName); - return new EndpointBean(beanName, bean); + return new EndpointBean(this.applicationContext.getEnvironment(), beanName, bean); } private void addExtensionBeans(Collection endpointBeans) { @@ -159,7 +160,7 @@ public abstract class EndpointDiscoverer, O exten private ExtensionBean createExtensionBean(String beanName) { Object bean = this.applicationContext.getBean(beanName); - return new ExtensionBean(beanName, bean); + return new ExtensionBean(this.applicationContext.getEnvironment(), beanName, bean); } private void addExtensionBean(EndpointBean endpointBean, ExtensionBean extensionBean) { @@ -401,7 +402,7 @@ public abstract class EndpointDiscoverer, O exten private Set extensions = new LinkedHashSet<>(); - EndpointBean(String beanName, Object bean) { + EndpointBean(Environment environment, String beanName, Object bean) { MergedAnnotation annotation = MergedAnnotations .from(bean.getClass(), SearchStrategy.TYPE_HIERARCHY).get(Endpoint.class); String id = annotation.getString("id"); @@ -409,7 +410,7 @@ public abstract class EndpointDiscoverer, O exten () -> "No @Endpoint id attribute specified for " + bean.getClass().getName()); this.beanName = beanName; this.bean = bean; - this.id = EndpointId.of(id); + this.id = EndpointId.of(environment, id); this.enabledByDefault = annotation.getBoolean("enableByDefault"); this.filter = getFilter(this.bean.getClass()); } @@ -462,7 +463,7 @@ public abstract class EndpointDiscoverer, O exten private final Class filter; - ExtensionBean(String beanName, Object bean) { + ExtensionBean(Environment environment, String beanName, Object bean) { this.bean = bean; this.beanName = beanName; MergedAnnotation extensionAnnotation = MergedAnnotations @@ -472,7 +473,7 @@ public abstract class EndpointDiscoverer, O exten .from(endpointType, SearchStrategy.TYPE_HIERARCHY).get(Endpoint.class); Assert.state(endpointAnnotation.isPresent(), () -> "Extension " + endpointType.getName() + " does not specify an endpoint"); - this.endpointId = EndpointId.of(endpointAnnotation.getString("id")); + this.endpointId = EndpointId.of(environment, endpointAnnotation.getString("id")); this.filter = extensionAnnotation.getClass("filter"); } diff --git a/spring-boot-project/spring-boot-actuator/src/main/resources/META-INF/additional-spring-configuration-metadata.json b/spring-boot-project/spring-boot-actuator/src/main/resources/META-INF/additional-spring-configuration-metadata.json new file mode 100644 index 00000000000..c50a6d86e8d --- /dev/null +++ b/spring-boot-project/spring-boot-actuator/src/main/resources/META-INF/additional-spring-configuration-metadata.json @@ -0,0 +1,10 @@ +{ + "properties": [ + { + "name": "management.endpoints.migrate-legacy-ids", + "type": "java.lang.Boolean", + "description": "Whether to transparently migrate legacy endpoint IDs.", + "defaultValue": false + } + ] +} diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/EndpointIdTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/EndpointIdTests.java index 393ede404de..a25ee122a1c 100644 --- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/EndpointIdTests.java +++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/EndpointIdTests.java @@ -21,6 +21,7 @@ import org.junit.jupiter.api.extension.ExtendWith; import org.springframework.boot.test.system.CapturedOutput; import org.springframework.boot.test.system.OutputCaptureExtension; +import org.springframework.mock.env.MockEnvironment; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; @@ -92,6 +93,16 @@ class EndpointIdTests { .contains("Endpoint ID 'foo-bar' contains invalid characters, please migrate to a valid format"); } + @Test + void ofWhenMigratingLegacyNameRemovesDots(CapturedOutput output) { + EndpointId.resetLoggedWarnings(); + MockEnvironment environment = new MockEnvironment(); + environment.setProperty("management.endpoints.migrate-legacy-ids", "true"); + EndpointId endpointId = EndpointId.of(environment, "foo.bar"); + assertThat(endpointId.toString()).isEqualTo("foobar"); + assertThat(output).doesNotContain("contains invalid characters"); + } + @Test void equalsAndHashCode() { EndpointId one = EndpointId.of("foobar1"); diff --git a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-actuator/src/main/java/smoketest/actuator/SampleLegacyEndpoint.java b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-actuator/src/main/java/smoketest/actuator/SampleLegacyEndpoint.java new file mode 100644 index 00000000000..913bb6e5bc6 --- /dev/null +++ b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-actuator/src/main/java/smoketest/actuator/SampleLegacyEndpoint.java @@ -0,0 +1,35 @@ +/* + * Copyright 2012-2019 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 + * + * https://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 smoketest.actuator; + +import java.util.Collections; +import java.util.Map; + +import org.springframework.boot.actuate.endpoint.annotation.Endpoint; +import org.springframework.boot.actuate.endpoint.annotation.ReadOperation; +import org.springframework.stereotype.Component; + +@Component +@Endpoint(id = "lega.cy") +public class SampleLegacyEndpoint { + + @ReadOperation + public Map example() { + return Collections.singletonMap("legacy", "legacy"); + } + +} 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 0abee45e6f1..77ecca44312 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 @@ -24,3 +24,4 @@ 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.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/SampleActuatorApplicationTests.java b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-actuator/src/test/java/smoketest/actuator/SampleActuatorApplicationTests.java index 8058b40199c..b2e8c0c09e5 100644 --- a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-actuator/src/test/java/smoketest/actuator/SampleActuatorApplicationTests.java +++ b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-actuator/src/test/java/smoketest/actuator/SampleActuatorApplicationTests.java @@ -36,6 +36,7 @@ import org.springframework.http.MediaType; import org.springframework.http.ResponseEntity; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.entry; /** * Basic integration tests for service demo application. @@ -188,6 +189,17 @@ class SampleActuatorApplicationTests { assertThat(beans).containsKey("spring.datasource-" + DataSourceProperties.class.getName()); } + @Test + void testLegacy() { + @SuppressWarnings("rawtypes") + ResponseEntity entity = this.restTemplate.withBasicAuth("user", getPassword()) + .getForEntity("/actuator/legacy", Map.class); + assertThat(entity.getStatusCode()).isEqualTo(HttpStatus.OK); + @SuppressWarnings("unchecked") + Map body = entity.getBody(); + assertThat(body).contains(entry("legacy", "legacy")); + } + private String getPassword() { return "password"; }