Make service id's names dns compatible

fixes gh-68

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<Map<String, Service>> 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<Self> agentSelf = client.getAgentSelf();

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

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<String> managementTags = Arrays.asList("management");
+	private List<String> 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;
 	}

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();
+	}
 }

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<Map<String, Service>> response = consul.getAgentServices();
 		Map<String, Service> 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:");
 	}
 }