From 09379686057b568fee4d430ee02d61a116ce7a76 Mon Sep 17 00:00:00 2001 From: Spencer Gibb Date: Mon, 19 Nov 2018 14:04:29 -0500 Subject: [PATCH] Propagates metadata thru load balancer client. fixes gh-424 --- .../ConsulRibbonClientConfiguration.java | 7 ++ .../discovery/ConsulServerIntrospector.java | 27 ++++++ .../HealthServiceServerListFilter.java | 2 +- .../discovery/ConsulDiscoveryClientTests.java | 6 +- .../ConsulLoadbalancerClientTests.java | 85 +++++++++++++++++++ 5 files changed, 125 insertions(+), 2 deletions(-) create mode 100644 spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulServerIntrospector.java create mode 100644 spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/ConsulLoadbalancerClientTests.java diff --git a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulRibbonClientConfiguration.java b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulRibbonClientConfiguration.java index 233daa9d..52f3ae21 100644 --- a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulRibbonClientConfiguration.java +++ b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulRibbonClientConfiguration.java @@ -73,6 +73,7 @@ public class ConsulRibbonClientConfiguration { } @Bean + @ConditionalOnMissingBean public ServerListFilter ribbonServerListFilter() { return new HealthServiceServerListFilter(); } @@ -83,6 +84,12 @@ public class ConsulRibbonClientConfiguration { return new ConsulPing(); } + @Bean + @ConditionalOnMissingBean + public ConsulServerIntrospector serverIntrospector() { + return new ConsulServerIntrospector(); + } + @PostConstruct public void preprocess() { setProp(this.serviceId, DeploymentContextBasedVipAddresses.key(), this.serviceId); diff --git a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulServerIntrospector.java b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulServerIntrospector.java new file mode 100644 index 00000000..f607ece1 --- /dev/null +++ b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/ConsulServerIntrospector.java @@ -0,0 +1,27 @@ +package org.springframework.cloud.consul.discovery; + +import com.netflix.loadbalancer.Server; +import org.springframework.cloud.netflix.ribbon.DefaultServerIntrospector; + +import java.util.Map; + +public class ConsulServerIntrospector extends DefaultServerIntrospector { + + @Override + public boolean isSecure(Server server) { + Map metadata = getMetadata(server); + if (metadata != null && metadata.containsKey("secure")) { + return metadata.getOrDefault("secure", "false").equalsIgnoreCase("true"); + } + return super.isSecure(server); + } + + @Override + public Map getMetadata(Server server) { + if (server instanceof ConsulServer) { + ConsulServer consulServer = (ConsulServer) server; + return consulServer.getMetadata(); + } + return super.getMetadata(server); + } +} diff --git a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/HealthServiceServerListFilter.java b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/HealthServiceServerListFilter.java index 25e5448b..f04a0af8 100644 --- a/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/HealthServiceServerListFilter.java +++ b/spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/HealthServiceServerListFilter.java @@ -25,7 +25,7 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; /** - * ServerList implementaion that filters ConsulServers based on if all their Health Checks are PASSING. + * ServerList implementation that filters ConsulServers based on if all their Health Checks are PASSING. * @author Spencer Gibb */ public class HealthServiceServerListFilter implements ServerListFilter { diff --git a/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryClientTests.java b/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryClientTests.java index f9eaa1f6..c27ecad2 100644 --- a/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryClientTests.java +++ b/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/ConsulDiscoveryClientTests.java @@ -16,6 +16,7 @@ package org.springframework.cloud.consul.discovery; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; @@ -43,7 +44,8 @@ import com.ecwid.consul.v1.Response; */ @RunWith(SpringRunner.class) @SpringBootTest(properties = { "spring.application.name=testConsulDiscovery", - "spring.cloud.consul.discovery.prefer-ip-address=true"}, + "spring.cloud.consul.discovery.prefer-ip-address=true", + "spring.cloud.consul.discovery.tags=foo=bar", }, classes = ConsulDiscoveryClientTests.MyTestConfig.class, webEnvironment = RANDOM_PORT) public class ConsulDiscoveryClientTests { @@ -62,6 +64,8 @@ public class ConsulDiscoveryClientTests { ServiceInstance instance = instances.get(0); assertFalse("instance was secure (https)", instance.isSecure()); assertIpAddress(instance); + assertThat(instance.getMetadata()) + .containsEntry("foo", "bar"); } @Test diff --git a/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/ConsulLoadbalancerClientTests.java b/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/ConsulLoadbalancerClientTests.java new file mode 100644 index 00000000..d6867892 --- /dev/null +++ b/spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/ConsulLoadbalancerClientTests.java @@ -0,0 +1,85 @@ +/* + * Copyright 2013-2015 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 + * + * http://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.discovery; + +import com.netflix.loadbalancer.Server; +import com.netflix.loadbalancer.ServerListFilter; +import org.junit.Test; +import org.junit.runner.RunWith; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.SpringBootConfiguration; +import org.springframework.boot.autoconfigure.EnableAutoConfiguration; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.cloud.client.ServiceInstance; +import org.springframework.cloud.client.discovery.EnableDiscoveryClient; +import org.springframework.cloud.client.loadbalancer.LoadBalancerClient; +import org.springframework.cloud.netflix.ribbon.RibbonClient; +import org.springframework.context.annotation.Bean; +import org.springframework.test.context.junit4.SpringRunner; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.Assert.assertTrue; +import static org.springframework.boot.test.context.SpringBootTest.WebEnvironment.RANDOM_PORT; + +/** + * @author Spencer Gibb + */ +@RunWith(SpringRunner.class) +@SpringBootTest(properties = { "spring.application.name=testConsulLoadBalancer", + "spring.cloud.consul.discovery.prefer-ip-address=true", + "spring.cloud.consul.discovery.tags=foo=bar", }, + webEnvironment = RANDOM_PORT) +public class ConsulLoadbalancerClientTests { + + @Autowired + private LoadBalancerClient client; + + @Test + public void chooseWorks() { + ServiceInstance instance = client.choose("testConsulLoadBalancer"); + assertThat(instance).isNotNull(); + + assertThat(instance.isSecure()).isFalse(); + assertIpAddress(instance); + assertThat(instance.getMetadata()) + .containsEntry("foo", "bar"); + } + + private void assertIpAddress(ServiceInstance instance) { + assertTrue("host isn't an ip address", + Character.isDigit(instance.getHost().charAt(0))); + } + + @SpringBootConfiguration + @EnableAutoConfiguration + @EnableDiscoveryClient + @RibbonClient(name = "testConsulLoadBalancer", configuration = MyRibbonConfig.class) + public static class MyTestConfig { } + + public static class MyRibbonConfig { + + public MyRibbonConfig() { + System.err.println("here"); + } + + @Bean + public ServerListFilter ribbonServerListFilter() { + return servers -> servers; + } + } +}