Skip to content

S
spring-cloud-netflix
Commit f98a7d5b by Dave Syer
Options

Take into account secure port settings when looking for config server

When the config server is located via discovery (spring.cloud.config.discovery.enabled=true) the discovery process should attempt to establish whether the config server prefers to be accessed securely (e.g. it registered with Eureka as nonSecurePortEnabled=false). This change ensures that this happens. Fixes gh-450
parent 5dccd03d
Showing with 68 additions and 42 deletions
spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/config/DiscoveryClientConfigServiceBootstrapConfiguration.java
...@@ -16,9 +16,12 @@ ...@@ -16,9 +16,12 @@
package org.springframework.cloud.netflix.config; package org.springframework.cloud.netflix.config;
import java.util.List;
import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
import org.springframework.cloud.client.ServiceInstance;
import org.springframework.cloud.client.discovery.EnableDiscoveryClient; import org.springframework.cloud.client.discovery.EnableDiscoveryClient;
import org.springframework.cloud.client.discovery.event.HeartbeatEvent; import org.springframework.cloud.client.discovery.event.HeartbeatEvent;
import org.springframework.cloud.client.discovery.event.HeartbeatMonitor; import org.springframework.cloud.client.discovery.event.HeartbeatMonitor;
...@@ -39,8 +42,8 @@ import com.netflix.discovery.DiscoveryManager; ...@@ -39,8 +42,8 @@ import com.netflix.discovery.DiscoveryManager;
import lombok.extern.apachecommons.CommonsLog; import lombok.extern.apachecommons.CommonsLog;
/** /**
* Bootstrap configuration for a config client that wants to lookup the config server via * Bootstrap configuration for a config client that wants to lookup the config
* discovery. * server via discovery.
* *
* @author Dave Syer * @author Dave Syer
*/ */
...@@ -50,8 +53,7 @@ import lombok.extern.apachecommons.CommonsLog; ...@@ -50,8 +53,7 @@ import lombok.extern.apachecommons.CommonsLog;
@EnableDiscoveryClient @EnableDiscoveryClient
@Import(EurekaClientAutoConfiguration.class) @Import(EurekaClientAutoConfiguration.class)
@CommonsLog @CommonsLog
public class DiscoveryClientConfigServiceBootstrapConfiguration implements public class DiscoveryClientConfigServiceBootstrapConfiguration implements SmartApplicationListener {
SmartApplicationListener {
private HeartbeatMonitor monitor = new HeartbeatMonitor(); private HeartbeatMonitor monitor = new HeartbeatMonitor();
...@@ -59,14 +61,17 @@ public class DiscoveryClientConfigServiceBootstrapConfiguration implements ...@@ -59,14 +61,17 @@ public class DiscoveryClientConfigServiceBootstrapConfiguration implements
private ConfigClientProperties config; private ConfigClientProperties config;
@Autowired @Autowired
private org.springframework.cloud.client.discovery.DiscoveryClient client;
@Autowired
private ApplicationContext context; private ApplicationContext context;
@Override @Override
public void onApplicationEvent(ApplicationEvent event) { public void onApplicationEvent(ApplicationEvent event) {
if (event instanceof ContextRefreshedEvent && ((ContextRefreshedEvent) event).getApplicationContext()==context) { if (event instanceof ContextRefreshedEvent
&& ((ContextRefreshedEvent) event).getApplicationContext() == context) {
refresh(); refresh();
} } else if (event instanceof HeartbeatEvent) {
else if (event instanceof HeartbeatEvent) {
if (this.monitor.update(((HeartbeatEvent) event).getValue())) { if (this.monitor.update(((HeartbeatEvent) event).getValue())) {
refresh(); refresh();
} }
...@@ -92,12 +97,9 @@ public class DiscoveryClientConfigServiceBootstrapConfiguration implements ...@@ -92,12 +97,9 @@ public class DiscoveryClientConfigServiceBootstrapConfiguration implements
private void refresh() { private void refresh() {
try { try {
log.info("Locating configserver via discovery"); log.info("Locating configserver via discovery");
InstanceInfo server = DiscoveryManager InstanceInfo server = DiscoveryManager.getInstance().getDiscoveryClient()
.getInstance() .getNextServerFromEureka(this.config.getDiscovery().getServiceId(), false);
.getDiscoveryClient() String url = getHomePage(server);
.getNextServerFromEureka(this.config.getDiscovery().getServiceId(),
false);
String url = server.getHomePageUrl();
if (server.getMetadata().containsKey("password")) { if (server.getMetadata().containsKey("password")) {
String user = server.getMetadata().get("user"); String user = server.getMetadata().get("user");
user = user == null ? "user" : user; user = user == null ? "user" : user;
...@@ -113,10 +115,17 @@ public class DiscoveryClientConfigServiceBootstrapConfiguration implements ...@@ -113,10 +115,17 @@ public class DiscoveryClientConfigServiceBootstrapConfiguration implements
url = url + path; url = url + path;
} }
this.config.setUri(url); this.config.setUri(url);
} } catch (Exception ex) {
catch (Exception ex) {
log.warn("Could not locate configserver via discovery", ex); log.warn("Could not locate configserver via discovery", ex);
} }
} }
private String getHomePage(InstanceInfo server) {
List<ServiceInstance> instances = client.getInstances(this.config.getDiscovery().getServiceId());
if (instances==null || instances.isEmpty()) {
return server.getHomePageUrl();
}
return instances.get(0).getUri().toString() + "/";
}
} }
spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/eureka/EurekaDiscoveryClient.java
...@@ -110,8 +110,8 @@ public class EurekaDiscoveryClient implements DiscoveryClient { ...@@ -110,8 +110,8 @@ public class EurekaDiscoveryClient implements DiscoveryClient {
@Override @Override
public int getPort() { public int getPort() {
// assume if unsecure is enabled, that is the default // assume if secure is enabled, that is the default
if (this.instance.isPortEnabled(UNSECURE) || !this.instance.isPortEnabled(SECURE)) { if (!this.instance.isPortEnabled(SECURE)) {
return this.instance.getPort(); return this.instance.getPort();
} }
return this.instance.getSecurePort(); return this.instance.getSecurePort();
......
spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/config/DiscoveryClientConfigServiceAutoConfigurationTests.java
...@@ -25,6 +25,7 @@ import org.springframework.beans.factory.annotation.Autowired; ...@@ -25,6 +25,7 @@ import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.autoconfigure.PropertyPlaceholderAutoConfiguration; import org.springframework.boot.autoconfigure.PropertyPlaceholderAutoConfiguration;
import org.springframework.boot.test.EnvironmentTestUtils; import org.springframework.boot.test.EnvironmentTestUtils;
import org.springframework.cloud.config.client.ConfigClientProperties; import org.springframework.cloud.config.client.ConfigClientProperties;
import org.springframework.cloud.netflix.eureka.EurekaClientAutoConfiguration;
import org.springframework.context.annotation.AnnotationConfigApplicationContext; import org.springframework.context.annotation.AnnotationConfigApplicationContext;
import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Configuration;
import org.springframework.core.env.ConfigurableEnvironment; import org.springframework.core.env.ConfigurableEnvironment;
...@@ -61,20 +62,18 @@ public class DiscoveryClientConfigServiceAutoConfigurationTests { ...@@ -61,20 +62,18 @@ public class DiscoveryClientConfigServiceAutoConfigurationTests {
@Test @Test
public void onWhenRequested() throws Exception { public void onWhenRequested() throws Exception {
given(this.client.getNextServerFromEureka("CONFIGSERVER", false)).willReturn( given(this.client.getNextServerFromEureka("CONFIGSERVER", false))
this.info); .willReturn(this.info);
setup("spring.cloud.config.discovery.enabled=true"); setup("spring.cloud.config.discovery.enabled=true");
assertEquals( assertEquals(1, this.context.getBeanNamesForType(
1, DiscoveryClientConfigServiceAutoConfiguration.class).length);
this.context
.getBeanNamesForType(DiscoveryClientConfigServiceAutoConfiguration.class).length);
Mockito.verify(this.client).getNextServerFromEureka("CONFIGSERVER", false); Mockito.verify(this.client).getNextServerFromEureka("CONFIGSERVER", false);
Mockito.verify(this.client).shutdown(); Mockito.verify(this.client).shutdown();
ConfigClientProperties locator = this.context ConfigClientProperties locator = this.context
.getBean(ConfigClientProperties.class); .getBean(ConfigClientProperties.class);
assertEquals("http://foo:7001/", locator.getRawUri()); assertEquals("http://foo:7001/", locator.getRawUri());
assertEquals("bar", ApplicationInfoManager.getInstance().getInfo().getMetadata() assertEquals("bar",
.get("foo")); ApplicationInfoManager.getInstance().getInfo().getMetadata().get("foo"));
} }
private void setup(String... env) { private void setup(String... env) {
...@@ -89,7 +88,9 @@ public class DiscoveryClientConfigServiceAutoConfigurationTests { ...@@ -89,7 +88,9 @@ public class DiscoveryClientConfigServiceAutoConfigurationTests {
parent.refresh(); parent.refresh();
this.context = new AnnotationConfigApplicationContext(); this.context = new AnnotationConfigApplicationContext();
this.context.setParent(parent); this.context.setParent(parent);
this.context.register(DiscoveryClientConfigServiceAutoConfiguration.class); this.context.register(PropertyPlaceholderAutoConfiguration.class,
DiscoveryClientConfigServiceAutoConfiguration.class,
EurekaClientAutoConfiguration.class);
this.context.refresh(); this.context.refresh();
DiscoveryManager.getInstance().setDiscoveryClient(this.client); DiscoveryManager.getInstance().setDiscoveryClient(this.client);
} }
......
spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/config/DiscoveryClientConfigServiceBootstrapConfigurationTests.java
...@@ -25,12 +25,15 @@ import org.springframework.cloud.config.client.ConfigClientProperties; ...@@ -25,12 +25,15 @@ import org.springframework.cloud.config.client.ConfigClientProperties;
import org.springframework.context.annotation.AnnotationConfigApplicationContext; import org.springframework.context.annotation.AnnotationConfigApplicationContext;
import com.netflix.appinfo.InstanceInfo; import com.netflix.appinfo.InstanceInfo;
import com.netflix.appinfo.InstanceInfo.PortType;
import com.netflix.discovery.DiscoveryClient; import com.netflix.discovery.DiscoveryClient;
import com.netflix.discovery.DiscoveryManager; import com.netflix.discovery.DiscoveryManager;
import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertEquals;
import static org.mockito.BDDMockito.given; import static org.mockito.BDDMockito.given;
import java.util.Arrays;
/** /**
* @author Dave Syer * @author Dave Syer
*/ */
...@@ -55,21 +58,17 @@ public class DiscoveryClientConfigServiceBootstrapConfigurationTests { ...@@ -55,21 +58,17 @@ public class DiscoveryClientConfigServiceBootstrapConfigurationTests {
this.context = new AnnotationConfigApplicationContext( this.context = new AnnotationConfigApplicationContext(
DiscoveryClientConfigServiceBootstrapConfiguration.class); DiscoveryClientConfigServiceBootstrapConfiguration.class);
assertEquals(0, this.context.getBeanNamesForType(DiscoveryClient.class).length); assertEquals(0, this.context.getBeanNamesForType(DiscoveryClient.class).length);
assertEquals( assertEquals(0, this.context.getBeanNamesForType(
0, DiscoveryClientConfigServiceBootstrapConfiguration.class).length);
this.context
.getBeanNamesForType(DiscoveryClientConfigServiceBootstrapConfiguration.class).length);
} }
@Test @Test
public void onWhenRequested() throws Exception { public void onWhenRequested() throws Exception {
given(this.client.getNextServerFromEureka("CONFIGSERVER", false)).willReturn( given(this.client.getNextServerFromEureka("CONFIGSERVER", false))
this.info); .willReturn(this.info);
setup("spring.cloud.config.discovery.enabled=true"); setup("spring.cloud.config.discovery.enabled=true");
assertEquals( assertEquals(1, this.context.getBeanNamesForType(
1, DiscoveryClientConfigServiceBootstrapConfiguration.class).length);
this.context
.getBeanNamesForType(DiscoveryClientConfigServiceBootstrapConfiguration.class).length);
Mockito.verify(this.client).getNextServerFromEureka("CONFIGSERVER", false); Mockito.verify(this.client).getNextServerFromEureka("CONFIGSERVER", false);
ConfigClientProperties locator = this.context ConfigClientProperties locator = this.context
.getBean(ConfigClientProperties.class); .getBean(ConfigClientProperties.class);
...@@ -77,10 +76,27 @@ public class DiscoveryClientConfigServiceBootstrapConfigurationTests { ...@@ -77,10 +76,27 @@ public class DiscoveryClientConfigServiceBootstrapConfigurationTests {
} }
@Test @Test
public void secureWhenRequested() throws Exception {
info = InstanceInfo.Builder.newBuilder().setAppName("app").setHostName("foo")
.setHomePageUrl("/", null).enablePort(PortType.SECURE, true).setSecurePort(443).build();
given(this.client.getNextServerFromEureka("CONFIGSERVER", false))
.willReturn(this.info);
given(this.client.getInstancesByVipAddress("CONFIGSERVER", false))
.willReturn(Arrays.asList(info));
setup("spring.cloud.config.discovery.enabled=true");
assertEquals(1, this.context.getBeanNamesForType(
DiscoveryClientConfigServiceBootstrapConfiguration.class).length);
Mockito.verify(this.client).getNextServerFromEureka("CONFIGSERVER", false);
ConfigClientProperties locator = this.context
.getBean(ConfigClientProperties.class);
assertEquals("https://foo:443/", locator.getRawUri());
}
@Test
public void setsPasssword() throws Exception { public void setsPasssword() throws Exception {
this.info.getMetadata().put("password", "bar"); this.info.getMetadata().put("password", "bar");
given(this.client.getNextServerFromEureka("CONFIGSERVER", false)).willReturn( given(this.client.getNextServerFromEureka("CONFIGSERVER", false))
this.info); .willReturn(this.info);
setup("spring.cloud.config.discovery.enabled=true"); setup("spring.cloud.config.discovery.enabled=true");
ConfigClientProperties locator = this.context ConfigClientProperties locator = this.context
.getBean(ConfigClientProperties.class); .getBean(ConfigClientProperties.class);
...@@ -92,8 +108,8 @@ public class DiscoveryClientConfigServiceBootstrapConfigurationTests { ...@@ -92,8 +108,8 @@ public class DiscoveryClientConfigServiceBootstrapConfigurationTests {
@Test @Test
public void setsPath() throws Exception { public void setsPath() throws Exception {
this.info.getMetadata().put("configPath", "/bar"); this.info.getMetadata().put("configPath", "/bar");
given(this.client.getNextServerFromEureka("CONFIGSERVER", false)).willReturn( given(this.client.getNextServerFromEureka("CONFIGSERVER", false))
this.info); .willReturn(this.info);
setup("spring.cloud.config.discovery.enabled=true"); setup("spring.cloud.config.discovery.enabled=true");
ConfigClientProperties locator = this.context ConfigClientProperties locator = this.context
.getBean(ConfigClientProperties.class); .getBean(ConfigClientProperties.class);
...@@ -103,8 +119,8 @@ public class DiscoveryClientConfigServiceBootstrapConfigurationTests { ...@@ -103,8 +119,8 @@ public class DiscoveryClientConfigServiceBootstrapConfigurationTests {
private void setup(String... env) { private void setup(String... env) {
this.context = new AnnotationConfigApplicationContext(); this.context = new AnnotationConfigApplicationContext();
EnvironmentTestUtils.addEnvironment(this.context, env); EnvironmentTestUtils.addEnvironment(this.context, env);
this.context.getDefaultListableBeanFactory().registerSingleton( this.context.getDefaultListableBeanFactory()
"mockDiscoveryClient", this.client); .registerSingleton("mockDiscoveryClient", this.client);
DiscoveryManager.getInstance().setDiscoveryClient(this.client); DiscoveryManager.getInstance().setDiscoveryClient(this.client);
this.context.register(PropertyPlaceholderAutoConfiguration.class, this.context.register(PropertyPlaceholderAutoConfiguration.class,
DiscoveryClientConfigServiceBootstrapConfiguration.class, DiscoveryClientConfigServiceBootstrapConfiguration.class,
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment