diff --git a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryClient.java b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryClient.java index eaba0568..f2f8fb44 100644 --- a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryClient.java +++ b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryClient.java @@ -20,11 +20,10 @@ import java.util.ArrayList; import java.util.List; import java.util.Map; -import org.springframework.beans.BeansException; import org.springframework.cloud.client.DefaultServiceInstance; import org.springframework.cloud.client.ServiceInstance; import org.springframework.cloud.client.discovery.DiscoveryClient; -import org.springframework.context.ApplicationContext; +import org.springframework.util.StringUtils; import com.ecwid.consul.v1.ConsulClient; import com.ecwid.consul.v1.QueryParams; @@ -33,30 +32,25 @@ import com.ecwid.consul.v1.agent.model.Member; import com.ecwid.consul.v1.agent.model.Self; import com.ecwid.consul.v1.agent.model.Service; import com.ecwid.consul.v1.catalog.model.CatalogService; -import org.springframework.context.ApplicationContextAware; -import org.springframework.util.StringUtils; /** * @author Spencer Gibb */ -public class ConsulDiscoveryClient implements DiscoveryClient, ApplicationContextAware { +public class ConsulDiscoveryClient implements DiscoveryClient { - private ApplicationContext context; + private ConsulLifecycle lifecycle; private ConsulClient client; private ConsulDiscoveryProperties properties; - public ConsulDiscoveryClient(ConsulClient client, ConsulDiscoveryProperties properties) { + public ConsulDiscoveryClient(ConsulClient client, ConsulLifecycle lifecycle, + ConsulDiscoveryProperties properties) { this.client = client; + this.lifecycle = lifecycle; this.properties = properties; } - @Override - public void setApplicationContext(ApplicationContext context) throws BeansException { - this.context = context; - } - @Override public String description() { return "Spring Cloud Consul Discovery Client"; @@ -65,10 +59,10 @@ public class ConsulDiscoveryClient implements DiscoveryClient, ApplicationContex @Override public ServiceInstance getLocalServiceInstance() { Response> agentServices = client.getAgentServices(); - Service service = agentServices.getValue().get(context.getId()); + Service service = agentServices.getValue().get(lifecycle.getServiceId()); if (service == null) { throw new IllegalStateException("Unable to locate service in consul agent: " - + context.getId()); + + lifecycle.getServiceId()); } String host = "localhost"; Response agentSelf = client.getAgentSelf(); diff --git a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryClientConfiguration.java b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryClientConfiguration.java index deb61671..3350b7f7 100644 --- a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryClientConfiguration.java +++ b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryClientConfiguration.java @@ -57,7 +57,7 @@ public class ConsulDiscoveryClientConfiguration { @Bean public ConsulDiscoveryClient consulDiscoveryClient() { - return new ConsulDiscoveryClient(consulClient, consulDiscoveryProperties()); + return new ConsulDiscoveryClient(consulClient, consulLifecycle(), consulDiscoveryProperties()); } @Bean diff --git a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryProperties.java b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryProperties.java index 721040ea..e2346e15 100644 --- a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryProperties.java +++ b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryProperties.java @@ -38,6 +38,8 @@ import org.springframework.boot.context.properties.ConfigurationProperties; @CommonsLog public class ConsulDiscoveryProperties { + protected static final String MANAGEMENT = "management"; + @Getter(AccessLevel.PRIVATE) @Setter(AccessLevel.PRIVATE) private String[] hostInfo = initHostInfo(); @@ -46,7 +48,7 @@ public class ConsulDiscoveryProperties { private boolean enabled = true; - private List managementTags = Arrays.asList("management"); + private List managementTags = Arrays.asList(MANAGEMENT); private String healthCheckPath = "/health"; @@ -68,6 +70,8 @@ public class ConsulDiscoveryProperties { private String scheme = "http"; + private String managementSuffix = MANAGEMENT; + public String getHostname() { return this.preferIpAddress ? this.ipAddress : this.hostname; } diff --git a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulLifecycle.java b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulLifecycle.java index d03aa928..ee419d40 100644 --- a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulLifecycle.java +++ b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulLifecycle.java @@ -36,6 +36,8 @@ import java.util.List; @Slf4j public class ConsulLifecycle extends AbstractDiscoveryLifecycle { + public static final char SEPARATOR = '-'; + @Autowired private ConsulClient client; @@ -69,12 +71,12 @@ public class ConsulLifecycle extends AbstractDiscoveryLifecycle { String appName = getAppName(); String id; if (properties.getInstanceId() == null) { - id = getContext().getId(); + id = getServiceId(); } else { - id = properties.getInstanceId(); + id = normalizeForDns(properties.getInstanceId()); } service.setId(id); - service.setName(appName); + service.setName(normalizeForDns(appName)); service.setTags(createTags()); NewService.Check check = new NewService.Check(); @@ -95,6 +97,10 @@ public class ConsulLifecycle extends AbstractDiscoveryLifecycle { register(service); } + public String getServiceId() { + return normalizeForDns(getContext().getId()); + } + @Override protected void registerManagement() { NewService management = new NewService(); @@ -121,7 +127,7 @@ public class ConsulLifecycle extends AbstractDiscoveryLifecycle { @Override protected void deregister() { - deregister(getContext().getId()); + deregister(getServiceId()); } @Override @@ -150,4 +156,42 @@ public class ConsulLifecycle extends AbstractDiscoveryLifecycle { protected boolean isEnabled() { return properties.isEnabled(); } + + /** + * @return the serviceId of the Management Service + */ + public String getManagementServiceId() { + return normalizeForDns(getContext().getId()) + SEPARATOR + properties.getManagementSuffix(); + } + + /** + * @return the service name of the Management Service + */ + public String getManagementServiceName() { + return normalizeForDns(getAppName()) + SEPARATOR + properties.getManagementSuffix(); + } + + public static String normalizeForDns(String s) { + if (!Character.isLetter(s.charAt(0)) + || !Character.isLetterOrDigit(s.charAt(s.length()-1))) { + throw new IllegalArgumentException("Consul service ids must start with a letter, end with a letter or digit, and have as interior characters only letters, digits, and hyphen"); + } + + StringBuilder normalized = new StringBuilder(); + Character prev = null; + for (char curr : s.toCharArray()) { + Character toAppend = null; + if (Character.isLetterOrDigit(curr)) { + toAppend = curr; + } else if (prev == null || !(prev == SEPARATOR)) { + toAppend = SEPARATOR; + } + if (toAppend != null) { + normalized.append(toAppend); + prev = toAppend; + } + } + + return normalized.toString(); + } } diff --git a/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/ConsulLifecycleTests.java b/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/ConsulLifecycleTests.java index c287ba66..23ff57e2 100644 --- a/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/ConsulLifecycleTests.java +++ b/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/ConsulLifecycleTests.java @@ -17,6 +17,7 @@ package org.springframework.cloud.consul.discovery; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNotNull; @@ -46,7 +47,7 @@ import com.ecwid.consul.v1.agent.model.Service; @RunWith(SpringJUnit4ClassRunner.class) @FixMethodOrder(MethodSorters.NAME_ASCENDING) @SpringApplicationConfiguration(classes = TestConfig.class) -@WebIntegrationTest(value = "spring.application.name=myTestService", randomPort = true) +@WebIntegrationTest(value = "spring.application.name=myTestService1::something", randomPort = true) public class ConsulLifecycleTests { @Autowired @@ -62,11 +63,34 @@ public class ConsulLifecycleTests { public void contextLoads() { Response> response = consul.getAgentServices(); Map services = response.getValue(); - Service service = services.get(context.getId()); + Service service = services.get(lifecycle.getServiceId()); assertNotNull("service was null", service); assertNotEquals("service port is 0", 0, service.getPort().intValue()); - assertEquals("service id was wrong", context.getId(), service.getId()); - assertEquals("service name was wrong", "myTestService", service.getService()); + assertFalse("service id contained invalid character: " + service.getId(), service.getId().contains(":")); + assertEquals("service id was wrong", lifecycle.getServiceId(), service.getId()); + assertEquals("service name was wrong", "myTestService1-something", service.getService()); + } + + @Test + public void normalizeForDnsWorks() { + assertEquals("abc1", ConsulLifecycle.normalizeForDns("abc1")); + assertEquals("ab-c1", ConsulLifecycle.normalizeForDns("ab:c1")); + assertEquals("ab-c1", ConsulLifecycle.normalizeForDns("ab::c1")); + } + + @Test(expected = IllegalArgumentException.class) + public void normalizedFailsIfFirstCharIsNumber() { + ConsulLifecycle.normalizeForDns("9abc"); + } + + @Test(expected = IllegalArgumentException.class) + public void normalizedFailsIfFirstCharIsNotAlpha() { + ConsulLifecycle.normalizeForDns(":abc"); + } + + @Test(expected = IllegalArgumentException.class) + public void normalizedFailsIfLastCharIsNotAlphaNumeric() { + ConsulLifecycle.normalizeForDns("abc:"); } }