From 8940b0e26f667b22147e8fa6cd3334e02c0b056d Mon Sep 17 00:00:00 2001 From: indraneelb1903 Date: Tue, 29 May 2018 11:27:28 -0400 Subject: [PATCH] Changes for High Availability for the config Server (#1017) * Changes for High Availability for the config Server --- .../main/asciidoc/spring-cloud-config.adoc | 6 + .../config/client/ConfigClientProperties.java | 65 ++++--- .../client/ConfigServerInstanceProvider.java | 9 +- .../ConfigServicePropertySourceLocator.java | 147 +++++++++------ ...ntConfigServiceBootstrapConfiguration.java | 49 +++-- ...figServiceBootstrapConfigurationTests.java | 21 ++- .../client/ConfigClientPropertiesTests.java | 130 ++++++++----- ...nfigServicePropertySourceLocatorTests.java | 172 +++++++++--------- ...figServiceBootstrapConfigurationTests.java | 30 ++- 9 files changed, 388 insertions(+), 241 deletions(-) diff --git a/docs/src/main/asciidoc/spring-cloud-config.adoc b/docs/src/main/asciidoc/spring-cloud-config.adoc index aecd11aa..17b31f90 100644 --- a/docs/src/main/asciidoc/spring-cloud-config.adoc +++ b/docs/src/main/asciidoc/spring-cloud-config.adoc @@ -1113,6 +1113,12 @@ In that case, the items in the list are tried one by one until one succeeds. This behavior can be useful when working on a feature branch. For instance, you might want to align the config label with your branch but make it optional (in that case, use `spring.cloud.config.label=myfeature,develop`). +=== Specifying Multiple Urls for the Config Server + +To ensure high availability when you have multiple instances of Config Server deployed and expect one or more instances to be unavailable from time to time, you can either specify multiple URLs (as a comma-separated list under the `spring.cloud.config.uri` property) or have all your instances register in a Service Registry like Eureka ( if using Discovery-First Bootstrap mode ). Note that doing so ensures high availability only when the Config Server is not running (that is, when the application has exited) or when a connection timeout has occurred. For example, if the Config Server returns a 500 (Internal Server Error) response or the Config Client receives a 401 from the Config Server (due to bad credentials or other causes), the Config Client does not try to fetch properties from other URLs. An error of that kind indicates a user issue rather than an availability problem. + +If you use HTTP basic security on your Config Server, it is currently possible to support per-Config Server auth credentials only if you embed the credentials in each URL you specify under the `spring.cloud.config.uri` property. If you use any other kind of security mechanism, you cannot (currently) support per-Config Server authentication and authorization. + === Security If you use HTTP Basic security on the server, clients need to know the password (and username if it is not the default). diff --git a/spring-cloud-config-client/src/main/java/org/springframework/cloud/config/client/ConfigClientProperties.java b/spring-cloud-config-client/src/main/java/org/springframework/cloud/config/client/ConfigClientProperties.java index b3c8e592..0d70fd6c 100644 --- a/spring-cloud-config-client/src/main/java/org/springframework/cloud/config/client/ConfigClientProperties.java +++ b/spring-cloud-config-client/src/main/java/org/springframework/cloud/config/client/ConfigClientProperties.java @@ -76,7 +76,7 @@ public class ConfigClientProperties { /** * The URI of the remote server (default http://localhost:8888). */ - private String uri = "http://localhost:8888"; + private String[] uri = { "http://localhost:8888" }; /** * Discovery properties. @@ -122,15 +122,11 @@ public class ConfigClientProperties { this.enabled = enabled; } - public String getRawUri() { - return extractCredentials().uri; - } - - public String getUri() { + public String[] getUri() { return this.uri; } - public void setUri(String url) { + public void setUri(String[] url) { this.uri = url; } @@ -159,7 +155,7 @@ public class ConfigClientProperties { } public String getUsername() { - return extractCredentials().username; + return username; } public void setUsername(String username) { @@ -167,13 +163,17 @@ public class ConfigClientProperties { } public String getPassword() { - return extractCredentials().password; + return password; } public void setPassword(String password) { this.password = password; } + public Credentials getCredentials(int index) { + return extractCredentials(index); + } + public Discovery getDiscovery() { return this.discovery; } @@ -216,9 +216,13 @@ public class ConfigClientProperties { this.headers = headers; } - private Credentials extractCredentials() { + private Credentials extractCredentials(int index) { Credentials result = new Credentials(); - String uri = this.uri; + int noOfUrl = this.uri.length; + if (index < 0 || index >= noOfUrl) { + throw new IllegalStateException("Trying to access an invalid array index"); + } + String uri = this.uri[index]; result.uri = uri; Credentials explicitCredentials = getUsernamePassword(); result.username = explicitCredentials.username; @@ -233,16 +237,16 @@ public class ConfigClientProperties { String bare = UriComponentsBuilder.fromHttpUrl(uri).userInfo(null).build() .toUriString(); result.uri = bare; - + // if userInfo does not contain a :, then append a : to it if (!userInfo.contains(":")) { userInfo = userInfo + ":"; } - - int sepIndex=userInfo.indexOf(":"); + + int sepIndex = userInfo.indexOf(":"); // set username and password from uri - result.username = userInfo.substring(0, sepIndex); - result.password = userInfo.substring(sepIndex +1); + result.username = userInfo.substring(0, sepIndex); + result.password = userInfo.substring(sepIndex + 1); // override password if explicitly set if (explicitCredentials.password != null) { @@ -270,24 +274,38 @@ public class ConfigClientProperties { if (StringUtils.hasText(this.username)) { credentials.username = this.username.trim(); - } else { + } + else { credentials.username = "user"; } return credentials; } - private static class Credentials { + public static class Credentials { + private String username; private String password; private String uri; + + public String getUsername() { + return username; + } + + public String getPassword() { + return password; + } + + public String getUri() { + return uri; + } } public static class Discovery { public static final String DEFAULT_CONFIG_SERVER = "configserver"; /** - * Flag to indicate that config server discovery is enabled (config server URL will be - * looked up via discovery). + * Flag to indicate that config server discovery is enabled (config server URL + * will be looked up via discovery). */ private boolean enabled; /** @@ -336,10 +354,9 @@ public class ConfigClientProperties { return "ConfigClientProperties [enabled=" + this.enabled + ", profile=" + this.profile + ", name=" + this.name + ", label=" + (this.label == null ? "" : this.label) + ", username=" + this.username - + ", password=" + this.password + ", uri=" + this.uri - + ", authorization=" + this.authorization - + ", discovery.enabled=" + this.discovery.enabled + ", failFast=" - + this.failFast + ", token=" + this.token + "]"; + + ", password=" + this.password + ", uri=" + this.uri + ", authorization=" + + this.authorization + ", discovery.enabled=" + this.discovery.enabled + + ", failFast=" + this.failFast + ", token=" + this.token + "]"; } } diff --git a/spring-cloud-config-client/src/main/java/org/springframework/cloud/config/client/ConfigServerInstanceProvider.java b/spring-cloud-config-client/src/main/java/org/springframework/cloud/config/client/ConfigServerInstanceProvider.java index 8088b077..ad3a2097 100644 --- a/spring-cloud-config-client/src/main/java/org/springframework/cloud/config/client/ConfigServerInstanceProvider.java +++ b/spring-cloud-config-client/src/main/java/org/springframework/cloud/config/client/ConfigServerInstanceProvider.java @@ -18,16 +18,15 @@ public class ConfigServerInstanceProvider { } @Retryable(interceptor = "configServerRetryInterceptor") - public ServiceInstance getConfigServerInstance(String serviceId) { + public List getConfigServerInstances(String serviceId) { logger.debug("Locating configserver (" + serviceId + ") via discovery"); List instances = this.client.getInstances(serviceId); if (instances.isEmpty()) { throw new IllegalStateException( "No instances found of configserver (" + serviceId + ")"); } - ServiceInstance instance = instances.get(0); - logger.debug( - "Located configserver (" + serviceId + ") via discovery: " + instance); - return instance; + logger.debug("Located configserver (" + serviceId + + ") via discovery. No of instances found: " + instances.size()); + return instances; } } diff --git a/spring-cloud-config-client/src/main/java/org/springframework/cloud/config/client/ConfigServicePropertySourceLocator.java b/spring-cloud-config-client/src/main/java/org/springframework/cloud/config/client/ConfigServicePropertySourceLocator.java index 5adcf222..5ef23118 100644 --- a/spring-cloud-config-client/src/main/java/org/springframework/cloud/config/client/ConfigServicePropertySourceLocator.java +++ b/spring-cloud-config-client/src/main/java/org/springframework/cloud/config/client/ConfigServicePropertySourceLocator.java @@ -26,6 +26,7 @@ import java.util.Map.Entry; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.cloud.bootstrap.config.PropertySourceLocator; +import org.springframework.cloud.config.client.ConfigClientProperties.Credentials; import org.springframework.cloud.config.environment.Environment; import org.springframework.cloud.config.environment.PropertySource; import org.springframework.core.annotation.Order; @@ -47,6 +48,7 @@ import org.springframework.util.Base64Utils; import org.springframework.util.StringUtils; import org.springframework.web.client.HttpClientErrorException; import org.springframework.web.client.HttpServerErrorException; +import org.springframework.web.client.ResourceAccessException; import org.springframework.web.client.RestTemplate; import static org.springframework.cloud.config.client.ConfigClientProperties.STATE_HEADER; @@ -76,41 +78,44 @@ public class ConfigServicePropertySourceLocator implements PropertySourceLocator org.springframework.core.env.Environment environment) { ConfigClientProperties properties = this.defaultProperties.override(environment); CompositePropertySource composite = new CompositePropertySource("configService"); - RestTemplate restTemplate = this.restTemplate == null ? getSecureRestTemplate(properties) + RestTemplate restTemplate = this.restTemplate == null + ? getSecureRestTemplate(properties) : this.restTemplate; Exception error = null; String errorBody = null; - logger.info("Fetching config from server at: " + properties.getRawUri()); try { String[] labels = new String[] { "" }; if (StringUtils.hasText(properties.getLabel())) { - labels = StringUtils.commaDelimitedListToStringArray(properties.getLabel()); + labels = StringUtils + .commaDelimitedListToStringArray(properties.getLabel()); } - String state = ConfigClientStateHolder.getState(); - // Try all the labels until one works for (String label : labels) { - Environment result = getRemoteEnvironment(restTemplate, - properties, label.trim(), state); + Environment result = getRemoteEnvironment(restTemplate, properties, + label.trim(), state); if (result != null) { log(result); - if (result.getPropertySources() != null) { // result.getPropertySources() can be null if using xml + if (result.getPropertySources() != null) { // result.getPropertySources() + // can be null if using + // xml for (PropertySource source : result.getPropertySources()) { @SuppressWarnings("unchecked") Map map = (Map) source .getSource(); - composite.addPropertySource(new MapPropertySource(source - .getName(), map)); + composite.addPropertySource( + new MapPropertySource(source.getName(), map)); } } - if (StringUtils.hasText(result.getState()) || StringUtils.hasText(result.getVersion())) { + if (StringUtils.hasText(result.getState()) + || StringUtils.hasText(result.getVersion())) { HashMap map = new HashMap<>(); putValue(map, "config.client.state", result.getState()); putValue(map, "config.client.version", result.getVersion()); - composite.addFirstPropertySource(new MapPropertySource("configClient", map)); + composite.addFirstPropertySource( + new MapPropertySource("configClient", map)); } return composite; } @@ -118,8 +123,8 @@ public class ConfigServicePropertySourceLocator implements PropertySourceLocator } catch (HttpServerErrorException e) { error = e; - if (MediaType.APPLICATION_JSON.includes(e.getResponseHeaders() - .getContentType())) { + if (MediaType.APPLICATION_JSON + .includes(e.getResponseHeaders().getContentType())) { errorBody = e.getResponseBodyAsString(); } } @@ -131,29 +136,32 @@ public class ConfigServicePropertySourceLocator implements PropertySourceLocator "Could not locate PropertySource and the fail fast property is set, failing", error); } - logger.warn("Could not locate PropertySource: " - + (errorBody == null ? error==null ? "label not found" : error.getMessage() : errorBody)); + logger.warn("Could not locate PropertySource: " + (errorBody == null + ? error == null ? "label not found" : error.getMessage() + : errorBody)); return null; } private void log(Environment result) { if (logger.isInfoEnabled()) { - logger.info(String.format("Located environment: name=%s, profiles=%s, label=%s, version=%s, state=%s", + logger.info(String.format( + "Located environment: name=%s, profiles=%s, label=%s, version=%s, state=%s", result.getName(), - result.getProfiles() == null ? "" : Arrays.asList(result.getProfiles()), + result.getProfiles() == null ? "" + : Arrays.asList(result.getProfiles()), result.getLabel(), result.getVersion(), result.getState())); } if (logger.isDebugEnabled()) { List propertySourceList = result.getPropertySources(); if (propertySourceList != null) { int propertyCount = 0; - for (PropertySource propertySource: propertySourceList) { + for (PropertySource propertySource : propertySourceList) { propertyCount += propertySource.getSource().size(); } - logger.debug(String.format("Environment %s has %d property sources with %d properties.", - result.getName(), - result.getPropertySources().size(), + logger.debug(String.format( + "Environment %s has %d property sources with %d properties.", + result.getName(), result.getPropertySources().size(), propertyCount)); } @@ -166,13 +174,16 @@ public class ConfigServicePropertySourceLocator implements PropertySourceLocator } } - private Environment getRemoteEnvironment(RestTemplate restTemplate, ConfigClientProperties properties, - String label, String state) { + private Environment getRemoteEnvironment(RestTemplate restTemplate, + ConfigClientProperties properties, String label, String state) { String path = "/{name}/{profile}"; String name = properties.getName(); String profile = properties.getProfile(); String token = properties.getToken(); - String uri = properties.getRawUri(); + int noOfUrls = properties.getUri().length; + if (noOfUrls > 1) { + logger.info("Multiple Config Server Urls found listed."); + } Object[] args = new String[] { name, profile }; if (StringUtils.hasText(label)) { @@ -184,29 +195,51 @@ public class ConfigServicePropertySourceLocator implements PropertySourceLocator } ResponseEntity response = null; - try { - HttpHeaders headers = new HttpHeaders(); - if (StringUtils.hasText(token)) { - headers.add(TOKEN_HEADER, token); + for (int i = 0; i < noOfUrls; i++) { + Credentials credentials = properties.getCredentials(i); + String uri = credentials.getUri(); + String username = credentials.getUsername(); + String password = credentials.getPassword(); + + logger.info("Fetching config from server at : " + uri); + + try { + HttpHeaders headers = new HttpHeaders(); + addAuthorizationToken(properties, headers, username, password); + if (StringUtils.hasText(token)) { + headers.add(TOKEN_HEADER, token); + } + if (StringUtils.hasText(state)) { // TODO: opt in to sending state? + headers.add(STATE_HEADER, state); + } + + final HttpEntity entity = new HttpEntity<>((Void) null, headers); + response = restTemplate.exchange(uri + path, HttpMethod.GET, entity, + Environment.class, args); } - if (StringUtils.hasText(state)) { //TODO: opt in to sending state? - headers.add(STATE_HEADER, state); + catch (HttpClientErrorException e) { + if (e.getStatusCode() != HttpStatus.NOT_FOUND) { + throw e; + } } - final HttpEntity entity = new HttpEntity<>((Void) null, headers); - response = restTemplate.exchange(uri + path, HttpMethod.GET, - entity, Environment.class, args); - } - catch (HttpClientErrorException e) { - if (e.getStatusCode() != HttpStatus.NOT_FOUND) { - throw e; + catch (ResourceAccessException e) { + logger.info("Connect Timeout Exception on Url - " + uri + + ". Will be trying the next url if available"); + if (i == noOfUrls - 1) + throw e; + else + continue; } + + if (response == null || response.getStatusCode() != HttpStatus.OK) { + return null; + } + + Environment result = response.getBody(); + return result; } - if (response == null || response.getStatusCode() != HttpStatus.OK) { - return null; - } - Environment result = response.getBody(); - return result; + return null; } public void setRestTemplate(RestTemplate restTemplate) { @@ -215,13 +248,23 @@ public class ConfigServicePropertySourceLocator implements PropertySourceLocator private RestTemplate getSecureRestTemplate(ConfigClientProperties client) { SimpleClientHttpRequestFactory requestFactory = new SimpleClientHttpRequestFactory(); - requestFactory.setReadTimeout((60 * 1000 * 3) + 5000); //TODO 3m5s, make configurable? + requestFactory.setReadTimeout((60 * 1000 * 3) + 5000); // TODO 3m5s, make + // configurable? RestTemplate template = new RestTemplate(requestFactory); - String username = client.getUsername(); - String password = client.getPassword(); - String authorization = client.getAuthorization(); Map headers = new HashMap<>(client.getHeaders()); + if (!headers.isEmpty()) { + template.setInterceptors(Arrays. asList( + new GenericRequestHeaderInterceptor(headers))); + } + + return template; + } + + private void addAuthorizationToken(ConfigClientProperties configClientProperties, + HttpHeaders httpHeaders, String username, String password) { + String authorization = configClientProperties.getAuthorization(); + if (password != null && authorization != null) { throw new IllegalStateException( "You must set either 'password' or 'authorization'"); @@ -229,18 +272,12 @@ public class ConfigServicePropertySourceLocator implements PropertySourceLocator if (password != null) { byte[] token = Base64Utils.encode((username + ":" + password).getBytes()); - headers.put("Authorization", "Basic " + new String(token)); + httpHeaders.add("Authorization", "Basic " + new String(token)); } else if (authorization != null) { - headers.put("Authorization", authorization); + httpHeaders.add("Authorization", authorization); } - if (!headers.isEmpty()) { - template.setInterceptors(Arrays. asList( - new GenericRequestHeaderInterceptor(headers))); - } - - return template; } public static class GenericRequestHeaderInterceptor diff --git a/spring-cloud-config-client/src/main/java/org/springframework/cloud/config/client/DiscoveryClientConfigServiceBootstrapConfiguration.java b/spring-cloud-config-client/src/main/java/org/springframework/cloud/config/client/DiscoveryClientConfigServiceBootstrapConfiguration.java index f7ae3f77..e3f92f3c 100644 --- a/spring-cloud-config-client/src/main/java/org/springframework/cloud/config/client/DiscoveryClientConfigServiceBootstrapConfiguration.java +++ b/spring-cloud-config-client/src/main/java/org/springframework/cloud/config/client/DiscoveryClientConfigServiceBootstrapConfiguration.java @@ -16,6 +16,9 @@ package org.springframework.cloud.config.client; +import java.util.ArrayList; +import java.util.List; + import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.beans.factory.annotation.Autowired; @@ -76,24 +79,38 @@ public class DiscoveryClientConfigServiceBootstrapConfiguration { private void refresh() { try { String serviceId = this.config.getDiscovery().getServiceId(); - ServiceInstance server = this.instanceProvider - .getConfigServerInstance(serviceId); - String url = getHomePage(server); - if (server.getMetadata().containsKey("password")) { - String user = server.getMetadata().get("user"); - user = user == null ? "user" : user; - this.config.setUsername(user); - String password = server.getMetadata().get("password"); - this.config.setPassword(password); - } - if (server.getMetadata().containsKey("configPath")) { - String path = server.getMetadata().get("configPath"); - if (url.endsWith("/") && path.startsWith("/")) { - url = url.substring(0, url.length() - 1); + List listOfUrls = new ArrayList<>(); + List serviceInstances = this.instanceProvider + .getConfigServerInstances(serviceId); + + for (int i = 0; i < serviceInstances.size(); i++) { + + ServiceInstance server = serviceInstances.get(i); + String url = getHomePage(server); + + if (server.getMetadata().containsKey("password")) { + String user = server.getMetadata().get("user"); + user = user == null ? "user" : user; + this.config.setUsername(user); + String password = server.getMetadata().get("password"); + this.config.setPassword(password); } - url = url + path; + + if (server.getMetadata().containsKey("configPath")) { + String path = server.getMetadata().get("configPath"); + if (url.endsWith("/") && path.startsWith("/")) { + url = url.substring(0, url.length() - 1); + } + url = url + path; + } + + listOfUrls.add(url); } - this.config.setUri(url); + + String[] uri = new String[listOfUrls.size()]; + uri = listOfUrls.toArray(uri); + this.config.setUri(uri); + } catch (Exception ex) { if (config.isFailFast()) { diff --git a/spring-cloud-config-client/src/test/java/org/springframework/cloud/config/client/BaseDiscoveryClientConfigServiceBootstrapConfigurationTests.java b/spring-cloud-config-client/src/test/java/org/springframework/cloud/config/client/BaseDiscoveryClientConfigServiceBootstrapConfigurationTests.java index c0d9e05c..858eaaf8 100644 --- a/spring-cloud-config-client/src/test/java/org/springframework/cloud/config/client/BaseDiscoveryClientConfigServiceBootstrapConfigurationTests.java +++ b/spring-cloud-config-client/src/test/java/org/springframework/cloud/config/client/BaseDiscoveryClientConfigServiceBootstrapConfigurationTests.java @@ -14,6 +14,7 @@ import org.springframework.cloud.client.DefaultServiceInstance; import org.springframework.cloud.client.ServiceInstance; import org.springframework.cloud.client.discovery.DiscoveryClient; import org.springframework.cloud.commons.util.UtilAutoConfiguration; +import org.springframework.cloud.config.client.ConfigClientProperties.Credentials; import org.springframework.context.annotation.AnnotationConfigApplicationContext; import static org.junit.Assert.assertEquals; @@ -51,6 +52,12 @@ public abstract class BaseDiscoveryClientConfigServiceBootstrapConfigurationTest .willReturn(Arrays.asList(this.info)); } + void givenDiscoveryClientReturnsInfoForMultipleInstances(ServiceInstance info1, + ServiceInstance info2) { + given(this.client.getInstances(DEFAULT_CONFIG_SERVER)) + .willReturn(Arrays.asList(info1, info2)); + } + void givenDiscoveryClientReturnsInfoOnThirdTry() { given(this.client.getInstances(DEFAULT_CONFIG_SERVER)) .willReturn(Collections. emptyList()) @@ -80,7 +87,19 @@ public abstract class BaseDiscoveryClientConfigServiceBootstrapConfigurationTest void expectConfigClientPropertiesHasConfiguration(final String expectedUri) { ConfigClientProperties properties = this.context .getBean(ConfigClientProperties.class); - assertEquals(expectedUri, properties.getRawUri()); + Credentials credentials = properties.getCredentials(0); + assertEquals(expectedUri, credentials.getUri()); + } + + void expectConfigClientPropertiesHasMultipleUris(final String expectedUri1, + final String expectedUri2) { + ConfigClientProperties properties = this.context + .getBean(ConfigClientProperties.class); + assertEquals(2, properties.getUri().length); + Credentials credentials1 = properties.getCredentials(0); + Credentials credentials2 = properties.getCredentials(1); + assertEquals(expectedUri1, credentials1.getUri()); + assertEquals(expectedUri2, credentials2.getUri()); } void verifyDiscoveryClientCalledThreeTimes() { diff --git a/spring-cloud-config-client/src/test/java/org/springframework/cloud/config/client/ConfigClientPropertiesTests.java b/spring-cloud-config-client/src/test/java/org/springframework/cloud/config/client/ConfigClientPropertiesTests.java index eda6afae..689e515a 100644 --- a/spring-cloud-config-client/src/test/java/org/springframework/cloud/config/client/ConfigClientPropertiesTests.java +++ b/spring-cloud-config-client/src/test/java/org/springframework/cloud/config/client/ConfigClientPropertiesTests.java @@ -16,7 +16,9 @@ package org.springframework.cloud.config.client; import org.junit.Test; +import org.junit.rules.ExpectedException; import org.springframework.boot.test.util.EnvironmentTestUtils; +import org.springframework.cloud.config.client.ConfigClientProperties.Credentials; import org.springframework.core.env.ConfigurableEnvironment; import org.springframework.core.env.StandardEnvironment; import org.springframework.mock.env.MockEnvironment; @@ -25,6 +27,8 @@ import static org.hamcrest.Matchers.equalTo; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThat; +import org.junit.Rule; + /** * @author Dave Syer * @@ -34,76 +38,86 @@ public class ConfigClientPropertiesTests { private ConfigClientProperties locator = new ConfigClientProperties( new StandardEnvironment()); + @Rule + public ExpectedException expected = ExpectedException.none(); + @Test public void vanilla() { - locator.setUri("http://localhost:9999"); + locator.setUri(new String[] { "http://localhost:9999" }); locator.setPassword("secret"); - assertEquals("http://localhost:9999", locator.getRawUri()); - assertEquals("user", locator.getUsername()); - assertEquals("secret", locator.getPassword()); + Credentials credentials = locator.getCredentials(0); + assertEquals("http://localhost:9999", credentials.getUri()); + assertEquals("user", credentials.getUsername()); + assertEquals("secret", credentials.getPassword()); } @Test public void uriCreds() { - locator.setUri("http://foo:bar@localhost:9999"); - assertEquals("http://localhost:9999", locator.getRawUri()); - assertEquals("foo", locator.getUsername()); - assertEquals("bar", locator.getPassword()); + locator.setUri(new String[] { "http://foo:bar@localhost:9999" }); + Credentials credentials = locator.getCredentials(0); + assertEquals("http://localhost:9999", credentials.getUri()); + assertEquals("foo", credentials.getUsername()); + assertEquals("bar", credentials.getPassword()); } @Test public void explicitPassword() { - locator.setUri("http://foo:bar@localhost:9999"); + locator.setUri(new String[] { "http://foo:bar@localhost:9999" }); locator.setPassword("secret"); - assertEquals("http://localhost:9999", locator.getRawUri()); - assertEquals("foo", locator.getUsername()); - assertEquals("secret", locator.getPassword()); + Credentials credentials = locator.getCredentials(0); + assertEquals("http://localhost:9999", credentials.getUri()); + assertEquals("foo", credentials.getUsername()); + assertEquals("secret", credentials.getPassword()); } - + @Test public void testIfNoColonPresentInUriCreds() { - locator.setUri("http://foobar@localhost:9999"); + locator.setUri(new String[] { "http://foobar@localhost:9999" }); locator.setPassword("secret"); - assertEquals("http://localhost:9999", locator.getRawUri()); - assertEquals("foobar", locator.getUsername()); - assertEquals("secret", locator.getPassword()); + Credentials credentials = locator.getCredentials(0); + assertEquals("http://localhost:9999", credentials.getUri()); + assertEquals("foobar", credentials.getUsername()); + assertEquals("secret", credentials.getPassword()); } @Test public void testIfColonPresentAtTheEndInUriCreds() { - locator.setUri("http://foobar:@localhost:9999"); + locator.setUri(new String[] { "http://foobar:@localhost:9999" }); locator.setPassword("secret"); - assertEquals("http://localhost:9999", locator.getRawUri()); - assertEquals("foobar", locator.getUsername()); - assertEquals("secret", locator.getPassword()); + Credentials credentials = locator.getCredentials(0); + assertEquals("http://localhost:9999", credentials.getUri()); + assertEquals("foobar", credentials.getUsername()); + assertEquals("secret", credentials.getPassword()); } - + @Test public void testIfColonPresentAtTheStartInUriCreds() { - locator.setUri("http://:foobar@localhost:9999"); - assertEquals("http://localhost:9999", locator.getRawUri()); - assertEquals("", locator.getUsername()); - assertEquals("foobar", locator.getPassword()); + locator.setUri(new String[] { "http://:foobar@localhost:9999" }); + Credentials credentials = locator.getCredentials(0); + assertEquals("http://localhost:9999", credentials.getUri()); + assertEquals("", credentials.getUsername()); + assertEquals("foobar", credentials.getPassword()); } - + @Test public void testIfColonPresentAtTheStartAndEndInUriCreds() { - locator.setUri("http://:foobar:@localhost:9999"); - assertEquals("http://localhost:9999", locator.getRawUri()); - assertEquals("", locator.getUsername()); - assertEquals("foobar:", locator.getPassword()); + locator.setUri(new String[] { "http://:foobar:@localhost:9999" }); + Credentials credentials = locator.getCredentials(0); + assertEquals("http://localhost:9999", credentials.getUri()); + assertEquals("", credentials.getUsername()); + assertEquals("foobar:", credentials.getPassword()); } - - + @Test - public void testIfsolonPresentAtTheStartAndEndInUriCreds() { - locator.setUri("http:// @localhost:9999"); + public void testIfSpacePresentAsUriCreds() { + locator.setUri(new String[] { "http:// @localhost:9999" }); locator.setPassword("secret"); - assertEquals("http://localhost:9999", locator.getRawUri()); - assertEquals(" ", locator.getUsername()); - assertEquals("secret", locator.getPassword()); + Credentials credentials = locator.getCredentials(0); + assertEquals("http://localhost:9999", credentials.getUri()); + assertEquals(" ", credentials.getUsername()); + assertEquals("secret", credentials.getPassword()); } - + @Test public void changeNameInOverride() { locator.setName("one"); @@ -115,14 +129,40 @@ public class ConfigClientPropertiesTests { @Test public void testThatExplicitUsernamePasswordTakePrecedence() { - ConfigClientProperties properties = - new ConfigClientProperties(new MockEnvironment()); + ConfigClientProperties properties = new ConfigClientProperties( + new MockEnvironment()); - properties.setUri("https://userInfoName:userInfoPW@localhost:8888/"); + properties.setUri( + new String[] { "https://userInfoName:userInfoPW@localhost:8888/" }); properties.setUsername("explicitName"); properties.setPassword("explicitPW"); - - assertThat(properties.getPassword(), equalTo("explicitPW")); - assertThat(properties.getUsername(), equalTo("explicitName")); + Credentials credentials = properties.getCredentials(0); + assertThat(credentials.getPassword(), equalTo("explicitPW")); + assertThat(credentials.getUsername(), equalTo("explicitName")); } + + @Test + public void checkIfExceptionThrownForNegativeIndex() { + locator.setUri(new String[] { "http://localhost:8888", "http://localhost:8889" }); + expected.expect(IllegalStateException.class); + expected.expectMessage("Trying to access an invalid array index"); + Credentials credentials = locator.getCredentials(-1); + } + + @Test + public void checkIfExceptionThrownForPositiveInvalidIndex() { + locator.setUri(new String[] { "http://localhost:8888", "http://localhost:8889" }); + expected.expect(IllegalStateException.class); + expected.expectMessage("Trying to access an invalid array index"); + Credentials credentials = locator.getCredentials(3); + } + + @Test + public void checkIfExceptionThrownForIndexEqualToLength() { + locator.setUri(new String[] { "http://localhost:8888", "http://localhost:8889" }); + expected.expect(IllegalStateException.class); + expected.expectMessage("Trying to access an invalid array index"); + Credentials credentials = locator.getCredentials(2); + } + } diff --git a/spring-cloud-config-client/src/test/java/org/springframework/cloud/config/client/ConfigServicePropertySourceLocatorTests.java b/spring-cloud-config-client/src/test/java/org/springframework/cloud/config/client/ConfigServicePropertySourceLocatorTests.java index 71ca4182..fd76b31b 100644 --- a/spring-cloud-config-client/src/test/java/org/springframework/cloud/config/client/ConfigServicePropertySourceLocatorTests.java +++ b/spring-cloud-config-client/src/test/java/org/springframework/cloud/config/client/ConfigServicePropertySourceLocatorTests.java @@ -50,8 +50,7 @@ public class ConfigServicePropertySourceLocatorTests { @Test public void sunnyDay() { Environment body = new Environment("app", "master"); - mockRequestResponseWithoutLabel(new ResponseEntity<>(body, - HttpStatus.OK)); + mockRequestResponseWithoutLabel(new ResponseEntity<>(body, HttpStatus.OK)); this.locator.setRestTemplate(this.restTemplate); assertNotNull(this.locator.locate(this.environment)); } @@ -59,8 +58,7 @@ public class ConfigServicePropertySourceLocatorTests { @Test public void sunnyDayWithLabel() { Environment body = new Environment("app", "master"); - mockRequestResponseWithLabel( - new ResponseEntity<>(body, HttpStatus.OK), "v1.0.0"); + mockRequestResponseWithLabel(new ResponseEntity<>(body, HttpStatus.OK), "v1.0.0"); this.locator.setRestTemplate(this.restTemplate); EnvironmentTestUtils.addEnvironment(this.environment, "spring.cloud.config.label:v1.0.0"); @@ -70,26 +68,27 @@ public class ConfigServicePropertySourceLocatorTests { @Test public void sunnyDayWithLabelThatContainsASlash() { Environment body = new Environment("app", "master"); - mockRequestResponseWithLabel( - new ResponseEntity<>(body, HttpStatus.OK), "release(_)v1.0.0"); + mockRequestResponseWithLabel(new ResponseEntity<>(body, HttpStatus.OK), + "release(_)v1.0.0"); this.locator.setRestTemplate(this.restTemplate); EnvironmentTestUtils.addEnvironment(this.environment, - "spring.cloud.config.label:release/v1.0.0"); + "spring.cloud.config.label:release/v1.0.0"); assertNotNull(this.locator.locate(this.environment)); } @Test public void sunnyDayWithNoSuchLabel() { - mockRequestResponseWithLabel(new ResponseEntity((Void) null, - HttpStatus.NOT_FOUND), "nosuchlabel"); + mockRequestResponseWithLabel( + new ResponseEntity((Void) null, HttpStatus.NOT_FOUND), + "nosuchlabel"); this.locator.setRestTemplate(this.restTemplate); assertNull(this.locator.locate(this.environment)); } @Test public void failsQuietly() { - mockRequestResponseWithoutLabel(new ResponseEntity<>("Wah!", - HttpStatus.INTERNAL_SERVER_ERROR)); + mockRequestResponseWithoutLabel( + new ResponseEntity<>("Wah!", HttpStatus.INTERNAL_SERVER_ERROR)); this.locator.setRestTemplate(this.restTemplate); assertNull(this.locator.locate(this.environment)); } @@ -100,9 +99,8 @@ public class ConfigServicePropertySourceLocatorTests { .mock(ClientHttpRequestFactory.class); ClientHttpRequest request = Mockito.mock(ClientHttpRequest.class); ClientHttpResponse response = Mockito.mock(ClientHttpResponse.class); - Mockito.when( - requestFactory.createRequest(Mockito.any(URI.class), - Mockito.any(HttpMethod.class))).thenReturn(request); + Mockito.when(requestFactory.createRequest(Mockito.any(URI.class), + Mockito.any(HttpMethod.class))).thenReturn(request); RestTemplate restTemplate = new RestTemplate(requestFactory); ConfigClientProperties defaults = new ConfigClientProperties(this.environment); defaults.setFailFast(true); @@ -112,13 +110,13 @@ public class ConfigServicePropertySourceLocatorTests { HttpHeaders headers = new HttpHeaders(); headers.setContentType(MediaType.APPLICATION_JSON); Mockito.when(response.getHeaders()).thenReturn(headers); - Mockito.when(response.getStatusCode()).thenReturn( - HttpStatus.INTERNAL_SERVER_ERROR); - Mockito.when(response.getBody()).thenReturn( - new ByteArrayInputStream("{}".getBytes())); + Mockito.when(response.getStatusCode()) + .thenReturn(HttpStatus.INTERNAL_SERVER_ERROR); + Mockito.when(response.getBody()) + .thenReturn(new ByteArrayInputStream("{}".getBytes())); this.locator.setRestTemplate(restTemplate); - this.expected.expectCause(IsInstanceOf - .instanceOf(IllegalArgumentException.class)); + this.expected + .expectCause(IsInstanceOf.instanceOf(IllegalArgumentException.class)); this.expected.expectMessage("fail fast property is set"); this.locator.locate(this.environment); } @@ -129,9 +127,8 @@ public class ConfigServicePropertySourceLocatorTests { .mock(ClientHttpRequestFactory.class); ClientHttpRequest request = Mockito.mock(ClientHttpRequest.class); ClientHttpResponse response = Mockito.mock(ClientHttpResponse.class); - Mockito.when( - requestFactory.createRequest(Mockito.any(URI.class), - Mockito.any(HttpMethod.class))).thenReturn(request); + Mockito.when(requestFactory.createRequest(Mockito.any(URI.class), + Mockito.any(HttpMethod.class))).thenReturn(request); RestTemplate restTemplate = new RestTemplate(requestFactory); ConfigClientProperties defaults = new ConfigClientProperties(this.environment); defaults.setFailFast(true); @@ -141,12 +138,12 @@ public class ConfigServicePropertySourceLocatorTests { HttpHeaders headers = new HttpHeaders(); headers.setContentType(MediaType.APPLICATION_JSON); Mockito.when(response.getHeaders()).thenReturn(headers); - Mockito.when(response.getStatusCode()).thenReturn( - HttpStatus.NOT_FOUND); - Mockito.when(response.getBody()).thenReturn( - new ByteArrayInputStream("".getBytes())); + Mockito.when(response.getStatusCode()).thenReturn(HttpStatus.NOT_FOUND); + Mockito.when(response.getBody()) + .thenReturn(new ByteArrayInputStream("".getBytes())); this.locator.setRestTemplate(restTemplate); - this.expected.expectCause(IsInstanceOf.instanceOf(IllegalArgumentException.class)); + this.expected + .expectCause(IsInstanceOf.instanceOf(IllegalArgumentException.class)); this.expected.expectMessage("fail fast property is set"); this.locator.locate(this.environment); } @@ -156,65 +153,21 @@ public class ConfigServicePropertySourceLocatorTests { ClientHttpRequestFactory requestFactory = Mockito .mock(ClientHttpRequestFactory.class); ClientHttpRequest request = Mockito.mock(ClientHttpRequest.class); - Mockito.when( - requestFactory.createRequest(Mockito.any(URI.class), - Mockito.any(HttpMethod.class))).thenReturn(request); + Mockito.when(requestFactory.createRequest(Mockito.any(URI.class), + Mockito.any(HttpMethod.class))).thenReturn(request); ConfigClientProperties defaults = new ConfigClientProperties(this.environment); + // defaults.setUri(new String[] {"http://localhost"); defaults.setFailFast(true); defaults.setUsername("username"); defaults.setPassword("password"); defaults.setAuthorization("Basic dXNlcm5hbWU6cGFzc3dvcmQNCg=="); this.locator = new ConfigServicePropertySourceLocator(defaults); - this.expected.expect(IllegalStateException.class); - this.expected.expectMessage("You must set either 'password' or 'authorization'"); + this.expected.expect(IllegalStateException.class); + this.expected.expectMessage( + "Could not locate PropertySource and the fail fast property is set, failing"); this.locator.locate(this.environment); } - @Test - public void interceptorShouldAddHeaderWhenPasswordPropertySet() throws Exception { - ClientHttpRequestFactory requestFactory = Mockito - .mock(ClientHttpRequestFactory.class); - ClientHttpRequest request = Mockito.mock(ClientHttpRequest.class); - Mockito.when(requestFactory.createRequest(Mockito.any(URI.class), - Mockito.any(HttpMethod.class))).thenReturn(request); - - ConfigClientProperties defaults = new ConfigClientProperties(this.environment); - defaults.setUsername("username"); - defaults.setPassword("password"); - this.locator = new ConfigServicePropertySourceLocator(defaults); - - RestTemplate restTemplate = ReflectionTestUtils.invokeMethod(this.locator, - "getSecureRestTemplate", defaults); - restTemplate.setRequestFactory(requestFactory); - - this.locator.setRestTemplate(restTemplate); - this.locator.locate(this.environment); - - assertThat(restTemplate.getInterceptors()).hasSize(1); - } - - @Test - public void interceptorShouldAddHeaderWhenAuthorizationPropertySet() throws Exception { - ClientHttpRequestFactory requestFactory = Mockito - .mock(ClientHttpRequestFactory.class); - ClientHttpRequest request = Mockito.mock(ClientHttpRequest.class); - Mockito.when(requestFactory.createRequest(Mockito.any(URI.class), - Mockito.any(HttpMethod.class))).thenReturn(request); - - ConfigClientProperties defaults = new ConfigClientProperties(this.environment); - defaults.setAuthorization("Basic dXNlcm5hbWU6cGFzc3dvcmQ="); - this.locator = new ConfigServicePropertySourceLocator(defaults); - - RestTemplate restTemplate = ReflectionTestUtils.invokeMethod(this.locator, - "getSecureRestTemplate", defaults); - restTemplate.setRequestFactory(requestFactory); - - this.locator.setRestTemplate(restTemplate); - this.locator.locate(this.environment); - - assertThat(restTemplate.getInterceptors()).hasSize(1); - } - @Test public void interceptorShouldAddHeadersWhenHeadersPropertySet() throws Exception { MockClientHttpRequest request = new MockClientHttpRequest(); @@ -229,28 +182,65 @@ public class ConfigServicePropertySourceLocatorTests { assertThat(request.getHeaders().getFirst("X-Example-Version")).isEqualTo("2.1"); } + @Test + public void shouldAddAuthorizationHeaderWhenPasswordSet() { + HttpHeaders headers = new HttpHeaders(); + ConfigClientProperties defaults = new ConfigClientProperties(this.environment); + this.locator = new ConfigServicePropertySourceLocator(defaults); + String username = "user"; + String password = "pass"; + ReflectionTestUtils.invokeMethod(this.locator, "addAuthorizationToken", defaults, + headers, username, password); + assertThat(headers).hasSize(1); + } + + @Test + public void shouldAddAuthorizationHeaderWhenAuthorizationSet() { + HttpHeaders headers = new HttpHeaders(); + ConfigClientProperties defaults = new ConfigClientProperties(this.environment); + defaults.setAuthorization("1234abcd"); + this.locator = new ConfigServicePropertySourceLocator(defaults); + String username = "user"; + String password = null; + ReflectionTestUtils.invokeMethod(this.locator, "addAuthorizationToken", defaults, + headers, username, password); + assertThat(headers).hasSize(1); + } + + @Test + public void shouldThrowExceptionWhenPasswordAndAuthorizationBothSet() { + HttpHeaders headers = new HttpHeaders(); + ConfigClientProperties defaults = new ConfigClientProperties(this.environment); + defaults.setAuthorization("1234abcd"); + this.locator = new ConfigServicePropertySourceLocator(defaults); + String username = "user"; + String password = "pass"; + this.expected.expect(IllegalStateException.class); + this.expected.expectMessage("You must set either 'password' or 'authorization'"); + ReflectionTestUtils.invokeMethod(this.locator, "addAuthorizationToken", defaults, + headers, username, password); + } + @SuppressWarnings("unchecked") private void mockRequestResponseWithLabel(ResponseEntity response, String label) { - Mockito.when( - this.restTemplate.exchange(Mockito.any(String.class), - Mockito.any(HttpMethod.class), Mockito.any(HttpEntity.class), - Mockito.any(Class.class), Matchers.anyString(), - Matchers.anyString(), Matchers.eq(label))).thenReturn(response); + Mockito.when(this.restTemplate.exchange(Mockito.any(String.class), + Mockito.any(HttpMethod.class), Mockito.any(HttpEntity.class), + Mockito.any(Class.class), Matchers.anyString(), Matchers.anyString(), + Matchers.eq(label))).thenReturn(response); } @SuppressWarnings("unchecked") private void mockRequestResponseWithoutLabel(ResponseEntity response) { - Mockito.when( - this.restTemplate.exchange(Mockito.any(String.class), - Mockito.any(HttpMethod.class), Mockito.any(HttpEntity.class), - Mockito.any(Class.class), Matchers.anyString(), - Matchers.anyString())).thenReturn(response); + Mockito.when(this.restTemplate.exchange(Mockito.any(String.class), + Mockito.any(HttpMethod.class), Mockito.any(HttpEntity.class), + Mockito.any(Class.class), Matchers.anyString(), Matchers.anyString())) + .thenReturn(response); } @SuppressWarnings("unchecked") - private void mockRequestResponseWithoutLabelWithExpectedName(ResponseEntity response, String expectedName) { - Mockito.when( - this.restTemplate.exchange(Mockito.any(String.class), + private void mockRequestResponseWithoutLabelWithExpectedName( + ResponseEntity response, String expectedName) { + Mockito.when(this.restTemplate.exchange(Mockito.any(String.class), Mockito.any(HttpMethod.class), Mockito.any(HttpEntity.class), Mockito.any(Class.class), Matchers.eq(expectedName), Matchers.anyString())).thenReturn(response); diff --git a/spring-cloud-config-client/src/test/java/org/springframework/cloud/config/client/DiscoveryClientConfigServiceBootstrapConfigurationTests.java b/spring-cloud-config-client/src/test/java/org/springframework/cloud/config/client/DiscoveryClientConfigServiceBootstrapConfigurationTests.java index 60ba9e34..1a9e3491 100644 --- a/spring-cloud-config-client/src/test/java/org/springframework/cloud/config/client/DiscoveryClientConfigServiceBootstrapConfigurationTests.java +++ b/spring-cloud-config-client/src/test/java/org/springframework/cloud/config/client/DiscoveryClientConfigServiceBootstrapConfigurationTests.java @@ -19,8 +19,10 @@ package org.springframework.cloud.config.client; import org.junit.Test; import org.springframework.cloud.client.DefaultServiceInstance; +import org.springframework.cloud.client.ServiceInstance; import org.springframework.cloud.client.discovery.DiscoveryClient; import org.springframework.cloud.client.discovery.event.HeartbeatEvent; +import org.springframework.cloud.config.client.ConfigClientProperties.Credentials; import org.springframework.context.annotation.AnnotationConfigApplicationContext; import static org.junit.Assert.assertEquals; @@ -28,7 +30,8 @@ import static org.junit.Assert.assertEquals; /** * @author Dave Syer */ -public class DiscoveryClientConfigServiceBootstrapConfigurationTests extends BaseDiscoveryClientConfigServiceBootstrapConfigurationTests { +public class DiscoveryClientConfigServiceBootstrapConfigurationTests + extends BaseDiscoveryClientConfigServiceBootstrapConfigurationTests { @Test public void offByDefault() throws Exception { @@ -78,6 +81,24 @@ public class DiscoveryClientConfigServiceBootstrapConfigurationTests extends Bas expectConfigClientPropertiesHasConfiguration("https://foo:443/"); } + @Test + public void multipleInstancesReturnedFromDiscovery() { + ServiceInstance info1 = new DefaultServiceInstance("app", "localhost", 8888, + true); + ServiceInstance info2 = new DefaultServiceInstance("app", "localhost1", 8888, + false); + givenDiscoveryClientReturnsInfoForMultipleInstances(info1, info2); + + setup("spring.cloud.config.discovery.enabled=true"); + + expectDiscoveryClientConfigServiceBootstrapConfigurationIsSetup(); + + verifyDiscoveryClientCalledOnce(); + expectConfigClientPropertiesHasMultipleUris("https://localhost:8888/", + "http://localhost1:8888/"); + + } + @Test public void setsPasssword() throws Exception { this.info.getMetadata().put("password", "bar"); @@ -87,9 +108,10 @@ public class DiscoveryClientConfigServiceBootstrapConfigurationTests extends Bas ConfigClientProperties locator = this.context .getBean(ConfigClientProperties.class); - assertEquals("http://foo:8877/", locator.getRawUri()); - assertEquals("bar", locator.getPassword()); - assertEquals("user", locator.getUsername()); + Credentials credentials = locator.getCredentials(0); + assertEquals("http://foo:8877/", credentials.getUri()); + assertEquals("bar", credentials.getPassword()); + assertEquals("user", credentials.getUsername()); } @Test