Removes tags as metadata.

Fixes gh-630
This commit is contained in:
Spencer Gibb
2020-04-09 16:21:50 -04:00
parent d353d0cb2f
commit c3f2367549
13 changed files with 71 additions and 284 deletions

View File

@@ -91,7 +91,7 @@ public class ConfigWatchTests {
this.configProperties.setFormat(FILES);
setupWatch(eventPublisher, new GetValue(), "/config/app.yml");
verify(eventPublisher, times(1)).publishEvent(any(RefreshEvent.class));
verify(eventPublisher, atLeastOnce()).publishEvent(any(RefreshEvent.class));
}
private void setupWatch(ApplicationEventPublisher eventPublisher, GetValue getValue,

View File

@@ -17,6 +17,7 @@
package org.springframework.cloud.consul.discovery;
import java.util.ArrayList;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
@@ -34,7 +35,6 @@ import org.springframework.cloud.client.ServiceInstance;
import org.springframework.cloud.client.discovery.DiscoveryClient;
import static org.springframework.cloud.consul.discovery.ConsulServerUtils.findHost;
import static org.springframework.cloud.consul.discovery.ConsulServerUtils.getMetadata;
/**
* @author Spencer Gibb
@@ -88,8 +88,10 @@ public class ConsulDiscoveryClient implements DiscoveryClient {
for (HealthService service : services.getValue()) {
String host = findHost(service);
Map<String, String> metadata = getMetadata(service,
this.properties.isTagsAsMetadata());
Map<String, String> metadata = service.getService().getMeta();
if (metadata == null) {
metadata = new LinkedHashMap<>();
}
boolean secure = false;
if (metadata.containsKey("secure")) {
secure = Boolean.parseBoolean(metadata.get("secure"));
@@ -113,8 +115,6 @@ public class ConsulDiscoveryClient implements DiscoveryClient {
@Override
public List<String> getServices() {
String aclToken = this.properties.getAclToken();
CatalogServicesRequest request = CatalogServicesRequest.newBuilder()
.setQueryParams(QueryParams.DEFAULT)
.setToken(this.properties.getAclToken()).build();

View File

@@ -56,10 +56,6 @@ public class ConsulDiscoveryProperties {
/** Enable tag override for the registered service. */
private Boolean enableTagOverride;
/** Use tags as metadata, defaults to true. */
@Deprecated
private boolean tagsAsMetadata = true;
/** Is service discovery enabled? */
private boolean enabled = true;
@@ -554,16 +550,6 @@ public class ConsulDiscoveryProperties {
this.order = order;
}
@Deprecated
public boolean isTagsAsMetadata() {
return this.tagsAsMetadata;
}
@Deprecated
public void setTagsAsMetadata(boolean tagsAsMetadata) {
this.tagsAsMetadata = tagsAsMetadata;
}
public Map<String, String> getManagementMetadata() {
return this.managementMetadata;
}
@@ -590,46 +576,44 @@ public class ConsulDiscoveryProperties {
@Override
public String toString() {
return new ToStringCreator(this).append("hostInfo", this.hostInfo)
.append("aclToken", this.aclToken).append("tags", this.tags)
.append("enabled", this.enabled)
.append("enableTagOverride", this.enableTagOverride)
.append("metadata", this.metadata)
.append("managementTags", this.managementTags)
.append("healthCheckPath", this.healthCheckPath)
.append("healthCheckUrl", this.healthCheckUrl)
.append("healthCheckHeaders", this.healthCheckHeaders)
.append("healthCheckInterval", this.healthCheckInterval)
.append("healthCheckTimeout", this.healthCheckTimeout)
.append("healthCheckCriticalTimeout", this.healthCheckCriticalTimeout)
.append("ipAddress", this.ipAddress).append("hostname", this.hostname)
.append("port", this.port).append("managementPort", this.managementPort)
.append("lifecycle", this.lifecycle)
.append("preferIpAddress", this.preferIpAddress)
.append("preferAgentAddress", this.preferAgentAddress)
return new ToStringCreator(this).append("aclToken", this.aclToken)
.append("catalogServicesWatchDelay", this.catalogServicesWatchDelay)
.append("catalogServicesWatchTimeout", this.catalogServicesWatchTimeout)
.append("includeHostnameInInstanceId", this.includeHostnameInInstanceId)
.append("consistencyMode", this.consistencyMode)
.append("serviceName", this.serviceName)
.append("instanceId", this.instanceId)
.append("instanceZone", this.instanceZone)
.append("instanceGroup", this.instanceGroup)
.append("defaultZoneMetadataName", this.defaultZoneMetadataName)
.append("scheme", this.scheme)
.append("managementSuffix", this.managementSuffix)
.append("serverListQueryTags", this.serverListQueryTags)
.append("datacenters", this.datacenters)
.append("defaultQueryTag", this.defaultQueryTag)
.append("queryPassing", this.queryPassing)
.append("register", this.register).append("deregister", this.deregister)
.append("registerHealthCheck", this.registerHealthCheck)
.append("failFast", this.failFast)
.append("healthCheckTlsSkipVerify", this.healthCheckTlsSkipVerify)
.append("order", this.order).append("tagsAsMetadata", this.tagsAsMetadata)
.append("defaultZoneMetadataName", this.defaultZoneMetadataName)
.append("deregister", this.deregister).append("enabled", this.enabled)
.append("enableTagOverride", this.enableTagOverride)
.append("failFast", this.failFast).append("hostInfo", this.hostInfo)
.append("healthCheckCriticalTimeout", this.healthCheckCriticalTimeout)
.append("healthCheckHeaders", this.healthCheckHeaders)
.append("healthCheckInterval", this.healthCheckInterval)
.append("healthCheckPath", this.healthCheckPath)
.append("healthCheckTimeout", this.healthCheckTimeout)
.append("healthCheckTlsSkipVerify", this.healthCheckTlsSkipVerify)
.append("healthCheckUrl", this.healthCheckUrl)
.append("hostname", this.hostname)
.append("includeHostnameInInstanceId", this.includeHostnameInInstanceId)
.append("instanceId", this.instanceId)
.append("instanceGroup", this.instanceGroup)
.append("instanceZone", this.instanceZone)
.append("ipAddress", this.ipAddress).append("lifecycle", this.lifecycle)
.append("metadata", this.metadata)
.append("managementEnableTagOverride", this.managementEnableTagOverride)
.append("managementMetadata", this.managementMetadata).toString();
.append("managementMetadata", this.managementMetadata)
.append("managementPort", this.managementPort)
.append("managementSuffix", this.managementSuffix)
.append("managementTags", this.managementTags).append("order", this.order)
.append("port", this.port)
.append("preferAgentAddress", this.preferAgentAddress)
.append("preferIpAddress", this.preferIpAddress)
.append("queryPassing", this.queryPassing)
.append("register", this.register)
.append("registerHealthCheck", this.registerHealthCheck)
.append("scheme", this.scheme).append("serviceName", this.serviceName)
.append("serverListQueryTags", this.serverListQueryTags)
.append("tags", this.tags).toString();
}
/**

View File

@@ -19,10 +19,6 @@ package org.springframework.cloud.consul.discovery;
import java.net.Inet6Address;
import java.net.InetAddress;
import java.net.UnknownHostException;
import java.util.Arrays;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import com.ecwid.consul.v1.health.model.HealthService;
import org.apache.commons.logging.Log;
@@ -69,45 +65,4 @@ public final class ConsulServerUtils {
}
}
@Deprecated
public static Map<String, String> getMetadata(HealthService healthService) {
return getMetadata(healthService, true);
}
@Deprecated
public static Map<String, String> getMetadata(HealthService healthService,
boolean tagsAsMetadata) {
if (tagsAsMetadata) {
return getMetadata(healthService.getService().getTags());
}
return healthService.getService().getMeta();
}
@Deprecated
public static Map<String, String> getMetadata(List<String> tags) {
LinkedHashMap<String, String> metadata = new LinkedHashMap<>();
if (tags != null) {
for (String tag : tags) {
String[] parts = StringUtils.delimitedListToStringArray(tag, "=");
switch (parts.length) {
case 0:
break;
case 1:
metadata.put(parts[0], parts[0]);
break;
case 2:
metadata.put(parts[0], parts[1]);
break;
default:
String[] end = Arrays.copyOfRange(parts, 1, parts.length);
metadata.put(parts[0], StringUtils.arrayToDelimitedString(end, "="));
break;
}
}
}
return metadata;
}
}

View File

@@ -18,6 +18,7 @@ package org.springframework.cloud.consul.discovery.reactive;
import java.util.ArrayList;
import java.util.Collections;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
@@ -38,7 +39,6 @@ import org.springframework.cloud.client.discovery.ReactiveDiscoveryClient;
import org.springframework.cloud.consul.discovery.ConsulDiscoveryProperties;
import static org.springframework.cloud.consul.discovery.ConsulServerUtils.findHost;
import static org.springframework.cloud.consul.discovery.ConsulServerUtils.getMetadata;
/**
* Consul version of {@link ReactiveDiscoveryClient}.
@@ -94,8 +94,10 @@ public class ConsulReactiveDiscoveryClient implements ReactiveDiscoveryClient {
private ServiceInstance mapToServiceInstance(HealthService service,
String serviceId) {
String host = findHost(service);
Map<String, String> metadata = getMetadata(service,
properties.isTagsAsMetadata());
Map<String, String> metadata = service.getService().getMeta();
if (metadata == null) {
metadata = new LinkedHashMap<>();
}
boolean secure = false;
if (metadata.containsKey("secure")) {
secure = Boolean.parseBoolean(metadata.get("secure"));

View File

@@ -16,9 +16,9 @@
package org.springframework.cloud.consul.serviceregistry;
import java.util.ArrayList;
import java.util.Collections;
import java.util.LinkedHashMap;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
@@ -89,7 +89,7 @@ public class ConsulAutoRegistration extends ConsulRegistration {
service.setAddress(properties.getHostname());
}
service.setName(normalizeForDns(appName));
service.setTags(createTags(properties));
service.setTags(new ArrayList<>(properties.getTags()));
service.setEnableTagOverride(properties.getEnableTagOverride());
service.setMeta(getMetadata(properties));
@@ -208,48 +208,25 @@ public class ConsulAutoRegistration extends ConsulRegistration {
return normalized.toString();
}
@Deprecated
public static List<String> createTags(ConsulDiscoveryProperties properties) {
List<String> tags = new LinkedList<>(properties.getTags());
if (properties.isTagsAsMetadata()) {
if (!StringUtils.isEmpty(properties.getInstanceZone())) {
tags.add(properties.getDefaultZoneMetadataName() + "="
+ properties.getInstanceZone());
}
if (!StringUtils.isEmpty(properties.getInstanceGroup())) {
tags.add("group=" + properties.getInstanceGroup());
}
// store the secure flag in the tags so that clients will be able to figure
// out whether to use http or https automatically
tags.add("secure="
+ Boolean.toString(properties.getScheme().equalsIgnoreCase("https")));
}
return tags;
}
private static Map<String, String> getMetadata(ConsulDiscoveryProperties properties) {
LinkedHashMap<String, String> metadata = new LinkedHashMap<>();
if (!CollectionUtils.isEmpty(properties.getMetadata())) {
metadata.putAll(properties.getMetadata());
}
if (!properties.isTagsAsMetadata()) {
// add metadata from other properties. See createTags above.
if (!StringUtils.isEmpty(properties.getInstanceZone())) {
metadata.put(properties.getDefaultZoneMetadataName(),
properties.getInstanceZone());
}
if (!StringUtils.isEmpty(properties.getInstanceGroup())) {
metadata.put("group", properties.getInstanceGroup());
}
// store the secure flag in the tags so that clients will be able to figure
// out whether to use http or https automatically
metadata.put("secure",
Boolean.toString(properties.getScheme().equalsIgnoreCase("https")));
// add metadata from other properties. See createTags above.
if (!StringUtils.isEmpty(properties.getInstanceZone())) {
metadata.put(properties.getDefaultZoneMetadataName(),
properties.getInstanceZone());
}
if (!StringUtils.isEmpty(properties.getInstanceGroup())) {
metadata.put("group", properties.getInstanceGroup());
}
// store the secure flag in the tags so that clients will be able to figure
// out whether to use http or https automatically
metadata.put("secure",
Boolean.toString(properties.getScheme().equalsIgnoreCase("https")));
return metadata;
}

View File

@@ -24,7 +24,6 @@ import com.ecwid.consul.v1.agent.model.NewService;
import org.springframework.cloud.client.DefaultServiceInstance;
import org.springframework.cloud.client.serviceregistry.Registration;
import org.springframework.cloud.consul.discovery.ConsulDiscoveryProperties;
import org.springframework.cloud.consul.discovery.ConsulServerUtils;
/**
* @author Spencer Gibb
@@ -78,9 +77,6 @@ public class ConsulRegistration implements Registration {
@Override
public Map<String, String> getMetadata() {
if (properties.isTagsAsMetadata()) {
return ConsulServerUtils.getMetadata(getService().getTags());
}
return getService().getMeta();
}

View File

@@ -44,7 +44,9 @@ import static org.springframework.boot.test.context.SpringBootTest.WebEnvironmen
"spring.cloud.consul.discovery.instanceId=testConsulDiscovery2Id",
"spring.cloud.consul.discovery.hostname=testConsulDiscovery2Host",
"spring.cloud.consul.discovery.registerHealthCheck=false",
"spring.cloud.consul.discovery.tags=plaintag,foo=bar,foo2=bar2=baz2" },
"spring.cloud.consul.discovery.tags=plaintag",
"spring.cloud.consul.discovery.metadata[foo]=bar",
"spring.cloud.consul.discovery.metadata[foo2]=bar2" },
webEnvironment = RANDOM_PORT)
public class ConsulDiscoveryClientCustomizedTests {
@@ -86,11 +88,8 @@ public class ConsulDiscoveryClientCustomizedTests {
String foo = metadata.get("foo");
assertThat(foo).as("metadata key foo was wrong").isEqualTo("bar");
String plaintag = metadata.get("plaintag");
assertThat(plaintag).as("metadata key plaintag was wrong").isEqualTo("plaintag");
String foo2 = metadata.get("foo2");
assertThat(foo2).as("metadata key foo2 was wrong").isEqualTo("bar2=baz2");
assertThat(foo2).as("metadata key foo2 was wrong").isEqualTo("bar2");
}
@Configuration(proxyBeanMethods = false)

View File

@@ -78,8 +78,10 @@ public class ConsulDiscoveryClientDefaultQueryTagTests {
public void shouldReturnOnlyIntgInstance() {
List<ServiceInstance> instances = this.discoveryClient.getInstances(NAME);
assertThat(instances).as("instances was wrong size").hasSize(1);
assertThat(instances.get(0).getMetadata()).as("instance is not intg")
.containsEntry("intg", "intg");
ServiceInstance serviceInstance = instances.get(0);
assertThat(serviceInstance.getPort()).isEqualTo(intgService.getPort());
assertThat(serviceInstance.getServiceId()).isEqualTo(intgService.getName());
assertThat(serviceInstance.getInstanceId()).isEqualTo(intgService.getId());
}
private NewService serviceForEnvironment(String env, int port) {

View File

@@ -43,7 +43,7 @@ import static org.springframework.boot.test.context.SpringBootTest.WebEnvironmen
@SpringBootTest(
properties = { "spring.application.name=testConsulDiscovery",
"spring.cloud.consul.discovery.prefer-ip-address=true",
"spring.cloud.consul.discovery.tags=foo=bar" },
"spring.cloud.consul.discovery.metadata[foo]=bar" },
classes = ConsulDiscoveryClientTests.MyTestConfig.class,
webEnvironment = RANDOM_PORT)
public class ConsulDiscoveryClientTests {

View File

@@ -61,8 +61,8 @@ public class ConsulAutoServiceRegistrationCustomizedInstanceZoneTests {
assertThat(service.getPort().intValue()).as("service port is 0").isNotEqualTo(0);
assertThat(service.getId()).as("service id was wrong")
.isEqualTo("myTestService1-WithZone");
assertThat(service.getTags().contains("myZone=zone1"))
.as("service zone was wrong").isTrue();
assertThat(service.getMeta()).as("service zone was wrong").containsEntry("myZone",
"zone1");
}
@Configuration(proxyBeanMethods = false)

View File

@@ -1,129 +0,0 @@
/*
* Copyright 2013-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 org.springframework.cloud.consul.serviceregistry;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import com.ecwid.consul.v1.ConsulClient;
import com.ecwid.consul.v1.QueryParams;
import com.ecwid.consul.v1.Response;
import com.ecwid.consul.v1.agent.model.Service;
import com.ecwid.consul.v1.health.HealthChecksForServiceRequest;
import com.ecwid.consul.v1.health.model.Check;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
import org.springframework.boot.autoconfigure.ImportAutoConfiguration;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.cloud.client.serviceregistry.AutoServiceRegistrationConfiguration;
import org.springframework.cloud.consul.ConsulAutoConfiguration;
import org.springframework.cloud.consul.discovery.ConsulDiscoveryProperties;
import org.springframework.context.annotation.Configuration;
import org.springframework.test.context.junit4.SpringRunner;
import static org.assertj.core.api.Assertions.assertThat;
import static org.springframework.boot.test.context.SpringBootTest.WebEnvironment.RANDOM_PORT;
/**
* @author Spencer Gibb
* @author Venil Noronha
*/
@RunWith(SpringRunner.class)
@SpringBootTest(
classes = ConsulAutoServiceRegistrationCustomizedPropsRealMetadataTests.TestPropsConfig.class,
properties = { "spring.application.name=myTestServiceRealMetadata-B",
"spring.cloud.consul.discovery.instanceId=myTestServiceRealMetadata1-B",
"spring.cloud.consul.discovery.port=4452",
"spring.cloud.consul.discovery.hostname=myhost",
"spring.cloud.consul.discovery.ipAddress=10.0.0.1",
"spring.cloud.consul.discovery.registerHealthCheck=false",
"spring.cloud.consul.discovery.failFast=false",
"spring.cloud.consul.discovery.default-zone-metadata-name=mydefaultzonemetadataname",
"spring.cloud.consul.discovery.instance-zone=myzone",
"spring.cloud.consul.discovery.instance-group=mygroup",
"spring.cloud.consul.discovery.tags[0]=mytag",
"spring.cloud.consul.discovery.enableTagOverride=true",
"spring.cloud.consul.discovery.tags-as-metadata=false",
"spring.cloud.consul.discovery.metadata.key1=value1",
"spring.cloud.consul.discovery.metadata.key2=value2" },
webEnvironment = RANDOM_PORT)
public class ConsulAutoServiceRegistrationCustomizedPropsRealMetadataTests {
@Autowired
private ConsulClient consul;
@Autowired
private ConsulDiscoveryProperties properties;
@Test
public void propertiesAreCorrect() {
Response<Map<String, Service>> response = this.consul.getAgentServices();
Map<String, Service> services = response.getValue();
Service service = services.get("myTestServiceRealMetadata1-B");
assertThat(service).as("service was null").isNotNull();
assertThat(service.getPort()).as("service port is discovery port")
.isEqualTo(4452);
assertThat("myTestServiceRealMetadata1-B").as("service id was wrong")
.isEqualTo(service.getId());
assertThat("myTestServiceRealMetadata-B").as("service name was wrong")
.isEqualTo(service.getService());
assertThat("myhost").as("property hostname was wrong")
.isEqualTo(this.properties.getHostname());
assertThat("10.0.0.1").as("property ipAddress was wrong")
.isEqualTo(this.properties.getIpAddress());
assertThat("myhost").as("service address was wrong")
.isEqualTo(service.getAddress());
assertThat(service.getEnableTagOverride())
.as("property enableTagOverride was wrong").isTrue();
assertThat(service.getTags()).as("property tags contains the wrong values")
.containsExactly("mytag");
HashMap<String, String> entries = new HashMap<>();
entries.put("key1", "value1");
entries.put("key2", "value2");
entries.put("mydefaultzonemetadataname", "myzone");
entries.put("group", "mygroup");
entries.put("secure", "false");
assertThat(service.getMeta()).as("property metadata contains the wrong entries")
.containsExactlyInAnyOrderEntriesOf(entries);
Response<List<Check>> checkResponse = this.consul.getHealthChecksForService(
"myTestServiceRealMetadata-B", HealthChecksForServiceRequest.newBuilder()
.setQueryParams(QueryParams.DEFAULT).build());
List<Check> checks = checkResponse.getValue();
assertThat(checks).as("checks was wrong size").hasSize(0);
}
@Test
public void testFailFastDisabled() {
assertThat(this.properties.isFailFast()).as("property failFast was wrong")
.isFalse();
}
@Configuration(proxyBeanMethods = false)
@EnableAutoConfiguration
@ImportAutoConfiguration({ AutoServiceRegistrationConfiguration.class,
ConsulAutoConfiguration.class,
ConsulAutoServiceRegistrationAutoConfiguration.class })
public static class TestPropsConfig {
}
}

View File

@@ -64,7 +64,6 @@ import static org.springframework.boot.test.context.SpringBootTest.WebEnvironmen
"spring.cloud.consul.discovery.metadata.key1=value1",
"spring.cloud.consul.discovery.metadata.key2=value2" },
webEnvironment = RANDOM_PORT)
@Deprecated
public class ConsulAutoServiceRegistrationCustomizedPropsTests {
@Autowired
@@ -94,11 +93,13 @@ public class ConsulAutoServiceRegistrationCustomizedPropsTests {
assertThat(service.getEnableTagOverride())
.as("property enableTagOverride was wrong").isTrue();
assertThat(service.getTags()).as("property tags contains the wrong values")
.containsExactly("mytag", "mydefaultzonemetadataname=myzone",
"group=mygroup", "secure=false");
.containsExactly("mytag");
HashMap<String, String> entries = new HashMap<>();
entries.put("key1", "value1");
entries.put("key2", "value2");
entries.put("mydefaultzonemetadataname", "myzone");
entries.put("group", "mygroup");
entries.put("secure", "false");
assertThat(service.getMeta()).as("property metadata contains the wrong entries")
.containsExactlyInAnyOrderEntriesOf(entries);