From 62a8f72b983a5dd501cbbda327291bee834544f1 Mon Sep 17 00:00:00 2001 From: Nastya Smirnova Date: Tue, 14 Nov 2017 19:54:12 +0200 Subject: [PATCH] Incorrect statusPageUrl and healthCheckUrl are reported to Eureka if management.context-path is not set (#2145) --- .../eureka/EurekaClientAutoConfiguration.java | 78 +++++----- .../DefaultManagementMetadataProvider.java | 97 ++++++++++++ .../eureka/metadata/ManagementMetadata.java | 53 +++++++ .../metadata/ManagementMetadataProvider.java | 10 ++ .../EurekaClientAutoConfigurationTests.java | 28 ++++ ...DefaultManagementMetadataProviderTest.java | 138 ++++++++++++++++++ 6 files changed, 370 insertions(+), 34 deletions(-) create mode 100644 spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/metadata/DefaultManagementMetadataProvider.java create mode 100644 spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/metadata/ManagementMetadata.java create mode 100644 spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/metadata/ManagementMetadataProvider.java create mode 100644 spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/metadata/DefaultManagementMetadataProviderTest.java diff --git a/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/EurekaClientAutoConfiguration.java b/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/EurekaClientAutoConfiguration.java index e763bbe2..d2c2a61e 100644 --- a/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/EurekaClientAutoConfiguration.java +++ b/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/EurekaClientAutoConfiguration.java @@ -24,7 +24,6 @@ import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; import java.net.MalformedURLException; -import java.net.URL; import java.util.Map; import org.springframework.beans.factory.annotation.Autowired; @@ -50,6 +49,9 @@ import org.springframework.cloud.client.serviceregistry.ServiceRegistryAutoConfi import org.springframework.cloud.commons.util.InetUtils; import org.springframework.cloud.context.scope.refresh.RefreshScope; import org.springframework.cloud.netflix.eureka.config.DiscoveryClientOptionalArgsConfiguration; +import org.springframework.cloud.netflix.eureka.metadata.DefaultManagementMetadataProvider; +import org.springframework.cloud.netflix.eureka.metadata.ManagementMetadata; +import org.springframework.cloud.netflix.eureka.metadata.ManagementMetadataProvider; import org.springframework.cloud.netflix.eureka.serviceregistry.EurekaAutoServiceRegistration; import org.springframework.cloud.netflix.eureka.serviceregistry.EurekaRegistration; import org.springframework.cloud.netflix.eureka.serviceregistry.EurekaServiceRegistry; @@ -119,60 +121,68 @@ public class EurekaClientAutoConfiguration { return client; } + @Bean + @ConditionalOnMissingBean + public ManagementMetadataProvider serviceManagementMetadataProvider() { + return new DefaultManagementMetadataProvider(); + } + @Bean @ConditionalOnMissingBean(value = EurekaInstanceConfig.class, search = SearchStrategy.CURRENT) - public EurekaInstanceConfigBean eurekaInstanceConfigBean(InetUtils inetUtils) throws MalformedURLException { + public EurekaInstanceConfigBean eurekaInstanceConfigBean(InetUtils inetUtils, + ManagementMetadataProvider managementMetadataProvider) throws MalformedURLException { PropertyResolver eurekaPropertyResolver = new RelaxedPropertyResolver(this.env, "eureka.instance."); String hostname = eurekaPropertyResolver.getProperty("hostname"); boolean preferIpAddress = Boolean.parseBoolean(eurekaPropertyResolver.getProperty("preferIpAddress")); boolean isSecurePortEnabled = Boolean.parseBoolean(eurekaPropertyResolver.getProperty("securePortEnabled")); - int nonSecurePort = Integer.valueOf(propertyResolver.getProperty("server.port", propertyResolver.getProperty("port", "8080"))); + String serverContextPath = propertyResolver.getProperty("server.contextPath", "/"); + int serverPort = Integer.valueOf(propertyResolver.getProperty("server.port", propertyResolver.getProperty("port", "8080"))); - int managementPort = Integer.valueOf(propertyResolver.getProperty("management.port", String.valueOf(nonSecurePort))); - String managementContextPath = propertyResolver.getProperty("management.contextPath", propertyResolver.getProperty("server.contextPath", "/")); + Integer managementPort = propertyResolver.getProperty("management.port", Integer.class);// nullable. should be wrapped into optional + String managementContextPath = propertyResolver.getProperty("management.contextPath");// nullable. should be wrapped into optional Integer jmxPort = propertyResolver.getProperty("com.sun.management.jmxremote.port", Integer.class);//nullable EurekaInstanceConfigBean instance = new EurekaInstanceConfigBean(inetUtils); - instance.setNonSecurePort(nonSecurePort); + + instance.setNonSecurePort(serverPort); instance.setInstanceId(getDefaultInstanceId(propertyResolver)); instance.setPreferIpAddress(preferIpAddress); if(isSecurePortEnabled) { - int securePort = Integer.valueOf(propertyResolver.getProperty("server.port", propertyResolver.getProperty("port", "8080"))); - instance.setSecurePort(securePort); + instance.setSecurePort(serverPort); } - if (managementPort != nonSecurePort && managementPort != 0) { - if (StringUtils.hasText(hostname)) { - instance.setHostname(hostname); - } - String statusPageUrlPath = eurekaPropertyResolver.getProperty("statusPageUrlPath"); - String healthCheckUrlPath = eurekaPropertyResolver.getProperty("healthCheckUrlPath"); - if (!managementContextPath.endsWith("/")) { - managementContextPath = managementContextPath + "/"; - } - if (StringUtils.hasText(statusPageUrlPath)) { - instance.setStatusPageUrlPath(statusPageUrlPath); - } - if (StringUtils.hasText(healthCheckUrlPath)) { - instance.setHealthCheckUrlPath(healthCheckUrlPath); - } - - String scheme = instance.getSecurePortEnabled() ? "https" : "http"; - URL base = new URL(scheme, instance.getHostname(), managementPort, managementContextPath); - instance.setStatusPageUrl(new URL(base, StringUtils.trimLeadingCharacter(instance.getStatusPageUrlPath(), '/')).toString()); - instance.setHealthCheckUrl(new URL(base, StringUtils.trimLeadingCharacter(instance.getHealthCheckUrlPath(), '/')).toString()); + if (StringUtils.hasText(hostname)) { + instance.setHostname(hostname); } - setupMetadataMap(instance, managementPort, jmxPort); + String statusPageUrlPath = eurekaPropertyResolver.getProperty("statusPageUrlPath"); + String healthCheckUrlPath = eurekaPropertyResolver.getProperty("healthCheckUrlPath"); + + if (StringUtils.hasText(statusPageUrlPath)) { + instance.setStatusPageUrlPath(statusPageUrlPath); + } + if (StringUtils.hasText(healthCheckUrlPath)) { + instance.setHealthCheckUrlPath(healthCheckUrlPath); + } + + ManagementMetadata metadata = managementMetadataProvider.get(instance, serverPort, + serverContextPath, managementContextPath, managementPort); + + if(metadata != null) { + instance.setStatusPageUrl(metadata.getStatusPageUrl()); + instance.setHealthCheckUrl(metadata.getHealthCheckUrl()); + Map metadataMap = instance.getMetadataMap(); + if (metadataMap.get("management.port") == null) { + metadataMap.put("management.port", String.valueOf(metadata.getManagementPort())); + } + } + + setupJmxPort(instance, jmxPort); return instance; } - private void setupMetadataMap(EurekaInstanceConfigBean instance, int managementPort, - Integer jmxPort) { + private void setupJmxPort(EurekaInstanceConfigBean instance, Integer jmxPort) { Map metadataMap = instance.getMetadataMap(); - if (metadataMap.get("management.port") == null && managementPort != 0) { - metadataMap.put("management.port", String.valueOf(managementPort)); - } if (metadataMap.get("jmx.port") == null && jmxPort != null) { metadataMap.put("jmx.port", String.valueOf(jmxPort)); } diff --git a/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/metadata/DefaultManagementMetadataProvider.java b/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/metadata/DefaultManagementMetadataProvider.java new file mode 100644 index 00000000..4f19b003 --- /dev/null +++ b/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/metadata/DefaultManagementMetadataProvider.java @@ -0,0 +1,97 @@ +package org.springframework.cloud.netflix.eureka.metadata; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.springframework.cloud.netflix.eureka.EurekaInstanceConfigBean; +import org.springframework.util.StringUtils; + +import java.net.MalformedURLException; +import java.net.URL; + +public class DefaultManagementMetadataProvider implements ManagementMetadataProvider { + + private static final int RANDOM_PORT = 0; + private static final Log log = LogFactory.getLog(DefaultManagementMetadataProvider.class); + + @Override + public ManagementMetadata get(EurekaInstanceConfigBean instance, int serverPort, + String serverContextPath, String managementContextPath, + Integer managementPort) { + if (isRandom(managementPort)) { + return null; + } + if (managementPort == null && isRandom(serverPort)) { + return null; + } + String healthCheckUrl = getHealthCheckUrl(instance, serverPort, serverContextPath, + managementContextPath, managementPort); + String statusPageUrl = getStatusPageUrl(instance, serverPort, serverContextPath, + managementContextPath, managementPort); + + return new ManagementMetadata(healthCheckUrl, statusPageUrl, managementPort == null ? serverPort : managementPort); + } + + private boolean isRandom(Integer port) { + return port != null && port == RANDOM_PORT; + } + + private String getHealthCheckUrl(EurekaInstanceConfigBean instance, int serverPort, String serverContextPath, + String managementContextPath, Integer managementPort) { + String healthCheckUrlPath = instance.getHealthCheckUrlPath(); + String healthCheckUrl = getUrl(instance, serverPort, serverContextPath, managementContextPath, + managementPort, healthCheckUrlPath); + log.debug("Constructed eureka meta-data healthcheckUrl: " + healthCheckUrl); + return healthCheckUrl; + } + + public String getStatusPageUrl(EurekaInstanceConfigBean instance, int serverPort, String serverContextPath, + String managementContextPath, Integer managementPort) { + String statusPageUrlPath = instance.getStatusPageUrlPath(); + String statusPageUrl = getUrl(instance, serverPort, serverContextPath, managementContextPath, + managementPort, statusPageUrlPath); + log.debug("Constructed eureka meta-data statusPageUrl: " + statusPageUrl); + return statusPageUrl; + } + + private String getUrl(EurekaInstanceConfigBean instance, int serverPort, + String serverContextPath, String managementContextPath, + Integer managementPort, String urlPath) { + managementContextPath = refineManagementContextPath(serverContextPath, managementContextPath, managementPort); + if (managementPort == null) { + managementPort = serverPort; + } + String scheme = instance.getSecurePortEnabled() ? "https" : "http"; + return constructValidUrl(scheme, instance.getHostname(), managementPort, managementContextPath, urlPath); + } + + private String refineManagementContextPath(String serverContextPath, String managementContextPath, + Integer managementPort) { + if(managementContextPath != null) { + return managementContextPath; + } + if(managementPort != null) { + return "/"; + } + return serverContextPath; + } + + private String constructValidUrl(String scheme, String hostname, int port, + String contextPath, String statusPath) { + try { + if (!contextPath.endsWith("/")) { + contextPath = contextPath + "/"; + } + URL base = new URL(scheme, hostname, port, contextPath); + String refinedStatusPath = StringUtils.trimLeadingCharacter(statusPath, '/'); + return new URL(base, refinedStatusPath).toString(); + } catch (MalformedURLException e) { + String message = getErrorMessage(scheme, hostname, port, contextPath, statusPath); + throw new IllegalStateException(message, e); + } + } + + private String getErrorMessage(String scheme, String hostname, int port, String contextPath, String statusPath) { + return String.format("Failed to construct url for scheme: %s, hostName: %s port: %s contextPath: %s statusPath: %s", + scheme, hostname, port, contextPath, statusPath); + } +} diff --git a/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/metadata/ManagementMetadata.java b/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/metadata/ManagementMetadata.java new file mode 100644 index 00000000..baa4a4ac --- /dev/null +++ b/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/metadata/ManagementMetadata.java @@ -0,0 +1,53 @@ +package org.springframework.cloud.netflix.eureka.metadata; + +import java.util.Objects; + +public class ManagementMetadata { + + private final String healthCheckUrl; + private final String statusPageUrl; + private final Integer managementPort; + + public ManagementMetadata(String healthCheckUrl, String statusPageUrl, Integer managementPort) { + this.healthCheckUrl = healthCheckUrl; + this.statusPageUrl = statusPageUrl; + this.managementPort = managementPort; + } + + public String getHealthCheckUrl() { + return healthCheckUrl; + } + + public String getStatusPageUrl() { + return statusPageUrl; + } + + public Integer getManagementPort() { + return managementPort; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + ManagementMetadata that = (ManagementMetadata) o; + return Objects.equals(healthCheckUrl, that.healthCheckUrl) && + Objects.equals(statusPageUrl, that.statusPageUrl) && + Objects.equals(managementPort, that.managementPort); + } + + @Override + public int hashCode() { + return Objects.hash(healthCheckUrl, statusPageUrl, managementPort); + } + + @Override + public String toString() { + final StringBuilder sb = new StringBuilder("ManagementMetadata{"); + sb.append("healthCheckUrl='").append(healthCheckUrl).append('\''); + sb.append(", statusPageUrl='").append(statusPageUrl).append('\''); + sb.append(", managementPort=").append(managementPort); + sb.append('}'); + return sb.toString(); + } +} diff --git a/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/metadata/ManagementMetadataProvider.java b/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/metadata/ManagementMetadataProvider.java new file mode 100644 index 00000000..98bc770e --- /dev/null +++ b/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/metadata/ManagementMetadataProvider.java @@ -0,0 +1,10 @@ +package org.springframework.cloud.netflix.eureka.metadata; + +import org.springframework.cloud.netflix.eureka.EurekaInstanceConfigBean; + +public interface ManagementMetadataProvider { + + ManagementMetadata get(EurekaInstanceConfigBean instance, int serverPort, + String serverContextPath, String managementContextPath, Integer managementPort); + +} diff --git a/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/EurekaClientAutoConfigurationTests.java b/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/EurekaClientAutoConfigurationTests.java index 8c7f59de..0143b7d1 100644 --- a/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/EurekaClientAutoConfigurationTests.java +++ b/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/EurekaClientAutoConfigurationTests.java @@ -189,6 +189,34 @@ public class EurekaClientAutoConfigurationTests { instance.getHealthCheckUrl().contains("/myHealthCheck")); } + @Test + public void statusPageUrl_and_healthCheckUrl_do_not_contain_server_context_path() throws Exception { + addEnvironment(this.context, "server.port=8989", + "management.port=9999", "server.contextPath=/service"); + + setupContext(RefreshAutoConfiguration.class); + EurekaInstanceConfigBean instance = this.context + .getBean(EurekaInstanceConfigBean.class); + assertTrue("Wrong status page: " + instance.getStatusPageUrl(), + instance.getStatusPageUrl().endsWith(":9999/info")); + assertTrue("Wrong health check: " + instance.getHealthCheckUrl(), + instance.getHealthCheckUrl().endsWith(":9999/health")); + } + + @Test + public void statusPageUrl_and_healthCheckUrl_contain_management_context_path() throws Exception { + addEnvironment(this.context, + "server.port=8989", "management.contextPath=/management"); + + setupContext(RefreshAutoConfiguration.class); + EurekaInstanceConfigBean instance = this.context + .getBean(EurekaInstanceConfigBean.class); + assertTrue("Wrong status page: " + instance.getStatusPageUrl(), + instance.getStatusPageUrl().endsWith(":8989/management/info")); + assertTrue("Wrong health check: " + instance.getHealthCheckUrl(), + instance.getHealthCheckUrl().endsWith(":8989/management/health")); + } + @Test public void statusPageUrlPathAndManagementPortAndContextPath() { addEnvironment(this.context, "server.port=8989", diff --git a/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/metadata/DefaultManagementMetadataProviderTest.java b/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/metadata/DefaultManagementMetadataProviderTest.java new file mode 100644 index 00000000..11b028a5 --- /dev/null +++ b/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/metadata/DefaultManagementMetadataProviderTest.java @@ -0,0 +1,138 @@ +package org.springframework.cloud.netflix.eureka.metadata; + +import org.junit.Before; +import org.junit.Test; +import org.springframework.cloud.netflix.eureka.EurekaInstanceConfigBean; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class DefaultManagementMetadataProviderTest { + + private static final EurekaInstanceConfigBean INSTANCE = mock(EurekaInstanceConfigBean.class); + private final ManagementMetadataProvider provider = new DefaultManagementMetadataProvider(); + + @Before + public void setUp() throws Exception { + when(INSTANCE.getHostname()).thenReturn("host"); + when(INSTANCE.getHealthCheckUrlPath()).thenReturn("health"); + when(INSTANCE.getStatusPageUrlPath()).thenReturn("info"); + } + + @Test + public void serverPortIsRandomAndManagementPortIsNull() throws Exception { + int serverPort = 0; + String serverContextPath = "/"; + String managementContextPath = null; + Integer managementPort = null; + ManagementMetadata actual = provider.get(INSTANCE, serverPort, serverContextPath, managementContextPath, managementPort); + + assertThat(actual).isNull(); + } + + @Test + public void managementPortIsRandom() throws Exception { + int serverPort = 0; + String serverContextPath = "/"; + String managementContextPath = null; + Integer managementPort = 0; + ManagementMetadata actual = provider.get(INSTANCE, serverPort, serverContextPath, managementContextPath, managementPort); + + assertThat(actual).isNull(); + } + + @Test + public void serverPort() throws Exception { + int serverPort = 7777; + String serverContextPath = "/"; + String managementContextPath = null; + Integer managementPort = null; + ManagementMetadata actual = provider.get(INSTANCE, serverPort, serverContextPath, managementContextPath, managementPort); + + assertThat(actual.getHealthCheckUrl()).isEqualTo("http://host:7777/health"); + assertThat(actual.getStatusPageUrl()).isEqualTo("http://host:7777/info"); + assertThat(actual.getManagementPort()).isEqualTo(7777); + } + + @Test + public void serverPortManagementPort() throws Exception { + int serverPort = 7777; + String serverContextPath = "/"; + String managementContextPath = null; + Integer managementPort = 8888; + ManagementMetadata actual = provider.get(INSTANCE, serverPort, serverContextPath, managementContextPath, managementPort); + + assertThat(actual.getHealthCheckUrl()).isEqualTo("http://host:8888/health"); + assertThat(actual.getStatusPageUrl()).isEqualTo("http://host:8888/info"); + assertThat(actual.getManagementPort()).isEqualTo(8888); + } + + @Test + public void serverPortManagementPortServerContextPath() throws Exception { + int serverPort = 7777; + String serverContextPath = "/Server"; + String managementContextPath = null; + Integer managementPort = 8888; + ManagementMetadata actual = provider.get(INSTANCE, serverPort, serverContextPath, managementContextPath, managementPort); + + assertThat(actual.getHealthCheckUrl()).isEqualTo("http://host:8888/health"); + assertThat(actual.getStatusPageUrl()).isEqualTo("http://host:8888/info"); + assertThat(actual.getManagementPort()).isEqualTo(8888); + } + + @Test + public void serverPortManagementPortServerContextPathManagementContextPath() throws Exception { + int serverPort = 7777; + String serverContextPath = "/Server"; + String managementContextPath = "/Management"; + Integer managementPort = 8888; + ManagementMetadata actual = provider.get(INSTANCE, serverPort, serverContextPath, managementContextPath, managementPort); + + assertThat(actual.getHealthCheckUrl()).isEqualTo("http://host:8888/Management/health"); + assertThat(actual.getStatusPageUrl()).isEqualTo("http://host:8888/Management/info"); + assertThat(actual.getManagementPort()).isEqualTo(8888); + } + + @Test + public void serverPortServerContextPathManagementContextPath() throws Exception { + int serverPort = 7777; + String serverContextPath = "/Server"; + String managementContextPath = "/Management"; + Integer managementPort = null; + ManagementMetadata actual = provider.get(INSTANCE, serverPort, serverContextPath, managementContextPath, managementPort); + + assertThat(actual.getHealthCheckUrl()).isEqualTo("http://host:7777/Management/health"); + assertThat(actual.getStatusPageUrl()).isEqualTo("http://host:7777/Management/info"); + assertThat(actual.getManagementPort()).isEqualTo(7777); + } + + @Test + public void serverPortServerContextPath() throws Exception { + int serverPort = 7777; + String serverContextPath = "/Server"; + String managementContextPath = null; + Integer managementPort = null; + ManagementMetadata actual = provider.get(INSTANCE, serverPort, serverContextPath, managementContextPath, managementPort); + + assertThat(actual.getHealthCheckUrl()).isEqualTo("http://host:7777/Server/health"); + assertThat(actual.getStatusPageUrl()).isEqualTo("http://host:7777/Server/info"); + assertThat(actual.getManagementPort()).isEqualTo(7777); + } + + @Test + public void serverPortManagementPortManagementContextPath() throws Exception { + int serverPort = 7777; + String serverContextPath = "/"; + String managementContextPath = "/Management"; + Integer managementPort = 8888; + ManagementMetadata actual = provider.get(INSTANCE, serverPort, serverContextPath, managementContextPath, managementPort); + + assertThat(actual.getHealthCheckUrl()).isEqualTo("http://host:8888/Management/health"); + assertThat(actual.getStatusPageUrl()).isEqualTo("http://host:8888/Management/info"); + assertThat(actual.getManagementPort()).isEqualTo(8888); + + } + + +} \ No newline at end of file