From 065bbefddca2d8cb4588b6fa46125db2ef85b5fa Mon Sep 17 00:00:00 2001 From: Johnny Lim Date: Thu, 23 Mar 2017 00:16:20 +0900 Subject: [PATCH 01/13] Fix typos in the reference doc (#672) --- docs/src/main/asciidoc/spring-cloud-config.adoc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/src/main/asciidoc/spring-cloud-config.adoc b/docs/src/main/asciidoc/spring-cloud-config.adoc index c6b7c938..d2bd9dd7 100644 --- a/docs/src/main/asciidoc/spring-cloud-config.adoc +++ b/docs/src/main/asciidoc/spring-cloud-config.adoc @@ -594,7 +594,7 @@ Properties written to `secret/application` are available to <<_vault_server,all applications using the Config Server>>. An application with the name `myApp` would have any properties written to `secret/myApp` and `secret/application` available to it. -When `myApp` has the `dev` profile enabled than properties written to +When `myApp` has the `dev` profile enabled then properties written to all of the above paths would be available to it, with properties in the first path in the list taking priority over the others. @@ -680,7 +680,7 @@ using one of the environment repositories from Spring Cloud. To do this your be must implement the `EnvironmentRepository` interface. If you would like to control the priority of you custom `EnvironmentRepository` within the composite environment you should also implement the `Ordered` interface and override the -`getOrdered` method. If you do not implement the `Ordered` interface than your +`getOrdered` method. If you do not implement the `Ordered` interface then your `EnvironmentRepository` will be given the lowest priority. ==== Property Overrides From 3abc1ab84c4e264a99db0d55b18abe1f873f0f7f Mon Sep 17 00:00:00 2001 From: Nastya Smirnova Date: Fri, 24 Mar 2017 22:14:51 +0200 Subject: [PATCH 02/13] Makes fetching of config server from Eureka retryable Makes fetching of config server from Eureka retryable, adds support for failfast Retry requires spring-retry to be on the classpath. --- pom.xml | 7 + spring-cloud-config-client/pom.xml | 6 + .../client/ConfigServerInstanceProvider.java | 33 ++++ ...ntConfigServiceBootstrapConfiguration.java | 27 +-- ...figServiceBootstrapConfigurationTests.java | 106 +++++++++++ ...tstrapConfigurationNoSpringRetryTests.java | 50 ++++++ ...figServiceBootstrapConfigurationTests.java | 167 +++++++++++------- 7 files changed, 317 insertions(+), 79 deletions(-) create mode 100644 spring-cloud-config-client/src/main/java/org/springframework/cloud/config/client/ConfigServerInstanceProvider.java create mode 100644 spring-cloud-config-client/src/test/java/org/springframework/cloud/config/client/BaseDiscoveryClientConfigServiceBootstrapConfigurationTests.java create mode 100644 spring-cloud-config-client/src/test/java/org/springframework/cloud/config/client/DiscoveryClientConfigServiceBootstrapConfigurationNoSpringRetryTests.java diff --git a/pom.xml b/pom.xml index 1c5460fd..a89c4d16 100644 --- a/pom.xml +++ b/pom.xml @@ -49,6 +49,13 @@ pom import + + org.springframework.cloud + spring-cloud-commons + test-jar + test + ${spring-cloud-commons.version} + diff --git a/spring-cloud-config-client/pom.xml b/spring-cloud-config-client/pom.xml index 5817b46b..fdefa944 100644 --- a/spring-cloud-config-client/pom.xml +++ b/spring-cloud-config-client/pom.xml @@ -76,6 +76,12 @@ spring-boot-starter-test test + + org.springframework.cloud + spring-cloud-commons + test-jar + test + 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 new file mode 100644 index 00000000..8088b077 --- /dev/null +++ b/spring-cloud-config-client/src/main/java/org/springframework/cloud/config/client/ConfigServerInstanceProvider.java @@ -0,0 +1,33 @@ +package org.springframework.cloud.config.client; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.springframework.cloud.client.ServiceInstance; +import org.springframework.cloud.client.discovery.DiscoveryClient; +import org.springframework.retry.annotation.Retryable; + +import java.util.List; + +public class ConfigServerInstanceProvider { + + private static Log logger = LogFactory.getLog(ConfigServerInstanceProvider.class); + private final DiscoveryClient client; + + public ConfigServerInstanceProvider(DiscoveryClient client) { + this.client = client; + } + + @Retryable(interceptor = "configServerRetryInterceptor") + public ServiceInstance getConfigServerInstance(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; + } +} 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 ca70599d..f7ae3f77 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,8 +16,6 @@ package org.springframework.cloud.config.client; -import java.util.List; - import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.beans.factory.annotation.Autowired; @@ -28,6 +26,7 @@ import org.springframework.cloud.client.discovery.EnableDiscoveryClient; import org.springframework.cloud.client.discovery.event.HeartbeatEvent; import org.springframework.cloud.client.discovery.event.HeartbeatMonitor; import org.springframework.cloud.commons.util.UtilAutoConfiguration; +import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Import; import org.springframework.context.event.ContextRefreshedEvent; @@ -52,10 +51,16 @@ public class DiscoveryClientConfigServiceBootstrapConfiguration { private ConfigClientProperties config; @Autowired - private DiscoveryClient client; + private ConfigServerInstanceProvider instanceProvider; private HeartbeatMonitor monitor = new HeartbeatMonitor(); + @Bean + public ConfigServerInstanceProvider configServerInstanceProvider( + DiscoveryClient discoveryClient) { + return new ConfigServerInstanceProvider(discoveryClient); + } + @EventListener(ContextRefreshedEvent.class) public void startup(ContextRefreshedEvent event) { refresh(); @@ -70,14 +75,9 @@ public class DiscoveryClientConfigServiceBootstrapConfiguration { private void refresh() { try { - logger.debug("Locating configserver via discovery"); String serviceId = this.config.getDiscovery().getServiceId(); - List instances = this.client.getInstances(serviceId); - if (instances.isEmpty()) { - logger.warn("No instances found of configserver (" + serviceId + ")"); - return; - } - ServiceInstance server = instances.get(0); + ServiceInstance server = this.instanceProvider + .getConfigServerInstance(serviceId); String url = getHomePage(server); if (server.getMetadata().containsKey("password")) { String user = server.getMetadata().get("user"); @@ -96,7 +96,12 @@ public class DiscoveryClientConfigServiceBootstrapConfiguration { this.config.setUri(url); } catch (Exception ex) { - logger.warn("Could not locate configserver via discovery", ex); + if (config.isFailFast()) { + throw ex; + } + else { + logger.warn("Could not locate configserver via discovery", ex); + } } } 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 new file mode 100644 index 00000000..ebc22a32 --- /dev/null +++ b/spring-cloud-config-client/src/test/java/org/springframework/cloud/config/client/BaseDiscoveryClientConfigServiceBootstrapConfigurationTests.java @@ -0,0 +1,106 @@ +package org.springframework.cloud.config.client; + +import org.junit.After; +import org.junit.Rule; +import org.junit.rules.ExpectedException; +import org.mockito.Mockito; +import org.springframework.boot.autoconfigure.PropertyPlaceholderAutoConfiguration; +import org.springframework.boot.test.util.EnvironmentTestUtils; +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.context.annotation.AnnotationConfigApplicationContext; + +import java.util.Arrays; +import java.util.Collections; + +import static org.junit.Assert.assertEquals; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.springframework.cloud.config.client.ConfigClientProperties.Discovery.DEFAULT_CONFIG_SERVER; + +public abstract class BaseDiscoveryClientConfigServiceBootstrapConfigurationTests { + + @Rule + public ExpectedException expectedException = ExpectedException.none(); + + protected AnnotationConfigApplicationContext context; + + protected DiscoveryClient client = Mockito.mock(DiscoveryClient.class); + + protected ServiceInstance info = new DefaultServiceInstance("app", "foo", 8877, + false); + + @After + public void close() { + if (this.context != null) { + this.context.close(); + } + } + + void givenDiscoveryClientReturnsNoInfo() { + given(this.client.getInstances(DEFAULT_CONFIG_SERVER)) + .willReturn(Collections. emptyList()); + } + + void givenDiscoveryClientReturnsInfo() { + given(this.client.getInstances(DEFAULT_CONFIG_SERVER)) + .willReturn(Arrays.asList(this.info)); + } + + void givenDiscoveryClientReturnsInfoOnThirdTry() { + given(this.client.getInstances(DEFAULT_CONFIG_SERVER)) + .willReturn(Collections. emptyList()) + .willReturn(Collections. emptyList()) + .willReturn(Arrays.asList(this.info)); + } + + void expectNoInstancesOfConfigServerException() { + expectedException.expect(IllegalStateException.class); + expectedException.expectMessage( + "No instances found of configserver (" + DEFAULT_CONFIG_SERVER + ")"); + } + + void expectDiscoveryClientConfigServiceBootstrapConfigurationIsSetup() { + assertEquals(1, this.context.getBeanNamesForType( + DiscoveryClientConfigServiceBootstrapConfiguration.class).length); + } + + void expectConfigClientPropertiesHasDefaultConfiguration() { + expectConfigClientPropertiesHasConfiguration("http://localhost:8888"); + } + + void expectConfigClientPropertiesHasConfigurationFromEureka() { + expectConfigClientPropertiesHasConfiguration("http://foo:8877/"); + } + + void expectConfigClientPropertiesHasConfiguration(final String expectedUri) { + ConfigClientProperties properties = this.context + .getBean(ConfigClientProperties.class); + assertEquals(expectedUri, properties.getRawUri()); + } + + void verifyDiscoveryClientCalledThreeTimes() { + verify(this.client, times(3)).getInstances(DEFAULT_CONFIG_SERVER); + } + + void verifyDiscoveryClientCalledOnce() { + verify(this.client).getInstances(DEFAULT_CONFIG_SERVER); + } + + void setup(String... env) { + this.context = new AnnotationConfigApplicationContext(); + EnvironmentTestUtils.addEnvironment(this.context, env); + EnvironmentTestUtils.addEnvironment(this.context, "eureka.client.enabled=false"); + this.context.getDefaultListableBeanFactory().registerSingleton("discoveryClient", + this.client); + this.context.register(UtilAutoConfiguration.class, + PropertyPlaceholderAutoConfiguration.class, + DiscoveryClientConfigServiceBootstrapConfiguration.class, + ConfigServiceBootstrapConfiguration.class, ConfigClientProperties.class); + this.context.refresh(); + } + +} diff --git a/spring-cloud-config-client/src/test/java/org/springframework/cloud/config/client/DiscoveryClientConfigServiceBootstrapConfigurationNoSpringRetryTests.java b/spring-cloud-config-client/src/test/java/org/springframework/cloud/config/client/DiscoveryClientConfigServiceBootstrapConfigurationNoSpringRetryTests.java new file mode 100644 index 00000000..f57831e3 --- /dev/null +++ b/spring-cloud-config-client/src/test/java/org/springframework/cloud/config/client/DiscoveryClientConfigServiceBootstrapConfigurationNoSpringRetryTests.java @@ -0,0 +1,50 @@ +package org.springframework.cloud.config.client; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.cloud.ClassPathExclusions; +import org.springframework.cloud.FilteredClassPathRunner; + +@RunWith(FilteredClassPathRunner.class) +@ClassPathExclusions({ "spring-retry-*.jar", "spring-boot-starter-aop-*.jar" }) +public class DiscoveryClientConfigServiceBootstrapConfigurationNoSpringRetryTests + extends BaseDiscoveryClientConfigServiceBootstrapConfigurationTests { + + @Test + public void shouldFailWithExceptionGetConfigServerInstanceFromDiscoveryClient() + throws Exception { + givenDiscoveryClientReturnsNoInfo(); + + expectNoInstancesOfConfigServerException(); + + setup("spring.cloud.config.discovery.enabled=true", + "spring.cloud.config.failFast=true"); + } + + @Test + public void shouldFailWithMessageGetConfigServerInstanceFromDiscoveryClient() + throws Exception { + givenDiscoveryClientReturnsNoInfo(); + + setup("spring.cloud.config.discovery.enabled=true", + "spring.cloud.config.failFast=false"); + + expectDiscoveryClientConfigServiceBootstrapConfigurationIsSetup(); + expectConfigClientPropertiesHasDefaultConfiguration(); + verifyDiscoveryClientCalledOnce(); + } + + @Test + public void shouldSucceedGetConfigServerInstanceFromDiscoveryClient() + throws Exception { + givenDiscoveryClientReturnsInfo(); + + setup("spring.cloud.config.discovery.enabled=true", + "spring.cloud.config.failFast=true"); + + expectDiscoveryClientConfigServiceBootstrapConfigurationIsSetup(); + expectConfigClientPropertiesHasConfigurationFromEureka(); + verifyDiscoveryClientCalledOnce(); + } + +} \ No newline at end of file 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 1a793e47..f1c00a61 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 @@ -16,46 +16,24 @@ package org.springframework.cloud.config.client; -import static org.junit.Assert.assertEquals; -import static org.mockito.BDDMockito.given; -import static org.springframework.cloud.config.client.ConfigClientProperties.Discovery.DEFAULT_CONFIG_SERVER; - -import java.util.Arrays; - -import org.junit.After; import org.junit.Test; -import org.mockito.Mockito; -import org.springframework.boot.autoconfigure.PropertyPlaceholderAutoConfiguration; -import org.springframework.boot.test.util.EnvironmentTestUtils; 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.commons.util.UtilAutoConfiguration; import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import static org.junit.Assert.assertEquals; + /** * @author Dave Syer */ -public class DiscoveryClientConfigServiceBootstrapConfigurationTests { - - private AnnotationConfigApplicationContext context; - - private DiscoveryClient client = Mockito.mock(DiscoveryClient.class); - - private ServiceInstance info = new DefaultServiceInstance("app", "foo", 8877, false); - - @After - public void close() { - if (this.context != null) { - this.context.close(); - } - } +public class DiscoveryClientConfigServiceBootstrapConfigurationTests extends BaseDiscoveryClientConfigServiceBootstrapConfigurationTests { @Test public void offByDefault() throws Exception { this.context = new AnnotationConfigApplicationContext( DiscoveryClientConfigServiceBootstrapConfiguration.class); + assertEquals(0, this.context.getBeanNamesForType(DiscoveryClient.class).length); assertEquals(0, this.context.getBeanNamesForType( DiscoveryClientConfigServiceBootstrapConfiguration.class).length); @@ -63,51 +41,49 @@ public class DiscoveryClientConfigServiceBootstrapConfigurationTests { @Test public void onWhenRequested() throws Exception { - given(this.client.getInstances(DEFAULT_CONFIG_SERVER)) - .willReturn(Arrays.asList(this.info)); + givenDiscoveryClientReturnsInfo(); + setup("spring.cloud.config.discovery.enabled=true"); - assertEquals(1, this.context.getBeanNamesForType( - DiscoveryClientConfigServiceBootstrapConfiguration.class).length); - Mockito.verify(this.client).getInstances(DEFAULT_CONFIG_SERVER); - ConfigClientProperties locator = this.context - .getBean(ConfigClientProperties.class); - assertEquals("http://foo:8877/", locator.getRawUri()); + + expectDiscoveryClientConfigServiceBootstrapConfigurationIsSetup(); + verifyDiscoveryClientCalledOnce(); + expectConfigClientPropertiesHasConfigurationFromEureka(); } @Test public void onWhenHeartbeat() throws Exception { setup("spring.cloud.config.discovery.enabled=true"); - assertEquals(1, this.context.getBeanNamesForType( - DiscoveryClientConfigServiceBootstrapConfiguration.class).length); - given(this.client.getInstances(DEFAULT_CONFIG_SERVER)) - .willReturn(Arrays.asList(this.info)); - Mockito.verify(this.client).getInstances(DEFAULT_CONFIG_SERVER); + + expectDiscoveryClientConfigServiceBootstrapConfigurationIsSetup(); + + givenDiscoveryClientReturnsInfo(); + verifyDiscoveryClientCalledOnce(); + context.publishEvent(new HeartbeatEvent(context, "new")); - ConfigClientProperties locator = this.context - .getBean(ConfigClientProperties.class); - assertEquals("http://foo:8877/", locator.getRawUri()); + + expectConfigClientPropertiesHasConfigurationFromEureka(); } @Test public void secureWhenRequested() throws Exception { this.info = new DefaultServiceInstance("app", "foo", 443, true); - given(this.client.getInstances(DEFAULT_CONFIG_SERVER)) - .willReturn(Arrays.asList(this.info)); + givenDiscoveryClientReturnsInfo(); + setup("spring.cloud.config.discovery.enabled=true"); - assertEquals(1, this.context.getBeanNamesForType( - DiscoveryClientConfigServiceBootstrapConfiguration.class).length); - Mockito.verify(this.client).getInstances(DEFAULT_CONFIG_SERVER); - ConfigClientProperties locator = this.context - .getBean(ConfigClientProperties.class); - assertEquals("https://foo:443/", locator.getRawUri()); + + expectDiscoveryClientConfigServiceBootstrapConfigurationIsSetup(); + + verifyDiscoveryClientCalledOnce(); + expectConfigClientPropertiesHasConfiguration("https://foo:443/"); } @Test public void setsPasssword() throws Exception { this.info.getMetadata().put("password", "bar"); - given(this.client.getInstances(DEFAULT_CONFIG_SERVER)) - .willReturn(Arrays.asList(this.info)); + givenDiscoveryClientReturnsInfo(); + setup("spring.cloud.config.discovery.enabled=true"); + ConfigClientProperties locator = this.context .getBean(ConfigClientProperties.class); assertEquals("http://foo:8877/", locator.getRawUri()); @@ -118,25 +94,80 @@ public class DiscoveryClientConfigServiceBootstrapConfigurationTests { @Test public void setsPath() throws Exception { this.info.getMetadata().put("configPath", "/bar"); - given(this.client.getInstances(DEFAULT_CONFIG_SERVER)) - .willReturn(Arrays.asList(this.info)); + givenDiscoveryClientReturnsInfo(); + setup("spring.cloud.config.discovery.enabled=true"); - ConfigClientProperties locator = this.context - .getBean(ConfigClientProperties.class); - assertEquals("http://foo:8877/bar", locator.getRawUri()); + + expectConfigClientPropertiesHasConfiguration("http://foo:8877/bar"); } - private void setup(String... env) { - this.context = new AnnotationConfigApplicationContext(); - EnvironmentTestUtils.addEnvironment(this.context, env); - EnvironmentTestUtils.addEnvironment(this.context, "eureka.client.enabled=false"); - this.context.getDefaultListableBeanFactory().registerSingleton("discoveryClient", - this.client); - this.context.register(UtilAutoConfiguration.class, - PropertyPlaceholderAutoConfiguration.class, - DiscoveryClientConfigServiceBootstrapConfiguration.class, - ConfigClientProperties.class); - this.context.refresh(); + @Test + public void shouldFailGetConfigServerInstanceFromDiscoveryClient() throws Exception { + givenDiscoveryClientReturnsNoInfo(); + + setup("spring.cloud.config.discovery.enabled=true"); + + expectDiscoveryClientConfigServiceBootstrapConfigurationIsSetup(); + verifyDiscoveryClientCalledOnce(); + expectConfigClientPropertiesHasDefaultConfiguration(); + } + + @Test + public void shouldRetryAndSucceedGetConfigServerInstanceFromDiscoveryClient() + throws Exception { + givenDiscoveryClientReturnsInfoOnThirdTry(); + + setup("spring.cloud.config.discovery.enabled=true", + "spring.cloud.config.retry.maxAttempts=3", + "spring.cloud.config.retry.initialInterval=10", + "spring.cloud.config.failFast=true"); + + expectDiscoveryClientConfigServiceBootstrapConfigurationIsSetup(); + verifyDiscoveryClientCalledThreeTimes(); + + context.publishEvent(new HeartbeatEvent(context, "new")); + + expectConfigClientPropertiesHasConfigurationFromEureka(); + } + + @Test + public void shouldNotRetryIfNotFailFastPropertySet() throws Exception { + givenDiscoveryClientReturnsInfoOnThirdTry(); + + setup("spring.cloud.config.discovery.enabled=true", + "spring.cloud.config.retry.maxAttempts=3", + "spring.cloud.config.retry.initialInterval=10"); + + expectDiscoveryClientConfigServiceBootstrapConfigurationIsSetup(); + verifyDiscoveryClientCalledOnce(); + expectConfigClientPropertiesHasDefaultConfiguration(); + } + + @Test + public void shouldRetryAndFailWithExceptionGetConfigServerInstanceFromDiscoveryClient() + throws Exception { + givenDiscoveryClientReturnsNoInfo(); + + expectNoInstancesOfConfigServerException(); + + setup("spring.cloud.config.discovery.enabled=true", + "spring.cloud.config.retry.maxAttempts=3", + "spring.cloud.config.retry.initialInterval=10", + "spring.cloud.config.failFast=true"); + } + + @Test + public void shouldRetryAndFailWithMessageGetConfigServerInstanceFromDiscoveryClient() + throws Exception { + givenDiscoveryClientReturnsNoInfo(); + + setup("spring.cloud.config.discovery.enabled=true", + "spring.cloud.config.retry.maxAttempts=3", + "spring.cloud.config.retry.initialInterval=10", + "spring.cloud.config.failFast=false"); + + expectDiscoveryClientConfigServiceBootstrapConfigurationIsSetup(); + expectConfigClientPropertiesHasDefaultConfiguration(); } } From 7d7b2c3ce014841c61cbc5eeb2ee7ba9f21b182b Mon Sep 17 00:00:00 2001 From: Sean Dukehart Date: Fri, 24 Mar 2017 17:36:17 -0400 Subject: [PATCH 03/13] Adds resolvePlaceholders to ResourceController Adds resolvePlaceholders query parameter to ResourceController to allow users the option to resolve placeholders or not. fixes gh-669 --- .../server/resource/ResourceController.java | 24 +++++++---- .../resource/ResourceControllerTests.java | 42 ++++++++++++++++--- 2 files changed, 51 insertions(+), 15 deletions(-) diff --git a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/resource/ResourceController.java b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/resource/ResourceController.java index 80f77733..feea79ae 100644 --- a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/resource/ResourceController.java +++ b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/resource/ResourceController.java @@ -25,8 +25,8 @@ import java.nio.charset.Charset; import javax.servlet.http.HttpServletRequest; +import org.springframework.cloud.config.environment.Environment; import org.springframework.cloud.config.server.environment.EnvironmentRepository; -import org.springframework.core.env.StandardEnvironment; import org.springframework.core.io.Resource; import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; @@ -35,6 +35,7 @@ import org.springframework.web.bind.annotation.ExceptionHandler; import org.springframework.web.bind.annotation.PathVariable; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestMethod; +import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.bind.annotation.ResponseStatus; import org.springframework.web.bind.annotation.RestController; import org.springframework.web.util.UrlPathHelper; @@ -68,10 +69,12 @@ public class ResourceController { } @RequestMapping("/{name}/{profile}/{label}/**") - public String resolve(@PathVariable String name, @PathVariable String profile, - @PathVariable String label, HttpServletRequest request) throws IOException { + public String retrieve(@PathVariable String name, @PathVariable String profile, + @PathVariable String label, HttpServletRequest request, + @RequestParam(defaultValue = "true") boolean resolvePlaceholders) + throws IOException { String path = getFilePath(request, name, profile, label); - return resolve(name, profile, label, path); + return retrieve(name, profile, label, path, resolvePlaceholders); } private String getFilePath(HttpServletRequest request, String name, String profile, @@ -82,21 +85,24 @@ public class ResourceController { return path; } - synchronized String resolve(String name, String profile, String label, String path) - throws IOException { + synchronized String retrieve(String name, String profile, String label, String path, + boolean resolvePlaceholders) throws IOException { if (label != null && label.contains("(_)")) { // "(_)" is uncommon in a git branch name, but "/" cannot be matched // by Spring MVC label = label.replace("(_)", "/"); } - StandardEnvironment environment = prepareEnvironment( - this.environmentRepository.findOne(name, profile, label)); // ensure InputStream will be closed to prevent file locks on Windows try (InputStream is = this.resourceRepository.findOne(name, profile, label, path) .getInputStream()) { String text = StreamUtils.copyToString(is, Charset.forName("UTF-8")); - return resolvePlaceholders(environment, text); + if (resolvePlaceholders) { + Environment environment = this.environmentRepository.findOne(name, + profile, label); + text = resolvePlaceholders(prepareEnvironment(environment), text); + } + return text; } } diff --git a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/resource/ResourceControllerTests.java b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/resource/ResourceControllerTests.java index 5d693b23..d09fa5d9 100644 --- a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/resource/ResourceControllerTests.java +++ b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/resource/ResourceControllerTests.java @@ -62,9 +62,16 @@ public class ResourceControllerTests { @Test public void templateReplacement() throws Exception { this.environmentRepository.setSearchLocations("classpath:/test"); - String resource = this.controller.resolve("foo", "bar", "dev", "template.json"); + String resource = this.controller.retrieve("foo", "bar", "dev", "template.json", true); assertTrue("Wrong content: " + resource, resource.matches("\\{\\s*\"foo\": \"dev_bar\"\\s*\\}")); } + + @Test + public void templateReplacementNotForResolvePlaceholdersFalse() throws Exception { + this.environmentRepository.setSearchLocations("classpath:/test"); + String resource = this.controller.retrieve("foo", "bar", "dev", "template.json", false); + assertTrue("Wrong content: " + resource, resource.matches("\\{\\s*\"foo\": \"\\$\\{foo\\}\"\\s*\\}")); + } @Test public void templateReplacementNotForBinary() throws Exception { @@ -76,21 +83,21 @@ public class ResourceControllerTests { @Test public void escapedPlaceholder() throws Exception { this.environmentRepository.setSearchLocations("classpath:/test"); - String resource = this.controller.resolve("foo", "bar", "dev", "placeholder.txt"); + String resource = this.controller.retrieve("foo", "bar", "dev", "placeholder.txt", true); assertEquals("foo: ${foo}", resource); } @Test public void labelWithSlash() throws Exception { this.environmentRepository.setSearchLocations("classpath:/test"); - String resource = this.controller.resolve("foo", "bar", "dev(_)spam", "foo.txt"); + String resource = this.controller.retrieve("foo", "bar", "dev(_)spam", "foo.txt", true); assertEquals("foo: dev_bar/spam", resource); } @Test public void resourceWithSlash() throws Exception { this.environmentRepository.setSearchLocations("classpath:/test"); - String resource = this.controller.resolve("foo", "bar", "dev", "spam/foo.txt"); + String resource = this.controller.retrieve("foo", "bar", "dev", "spam/foo.txt", true); assertEquals("foo: dev_bar/spam", resource); } @@ -99,7 +106,7 @@ public class ResourceControllerTests { this.environmentRepository.setSearchLocations("classpath:/test"); MockHttpServletRequest request = new MockHttpServletRequest(); request.setRequestURI("/foo/bar/dev/" + "spam/foo.txt"); - String resource = this.controller.resolve("foo", "bar", "dev", request); + String resource = this.controller.retrieve("foo", "bar", "dev", request, true); assertEquals("foo: dev_bar/spam", resource); } @@ -109,10 +116,33 @@ public class ResourceControllerTests { MockHttpServletRequest request = new MockHttpServletRequest(); request.setServletPath("/spring"); request.setRequestURI("/foo/bar/dev/" + "spam/foo.txt"); - String resource = this.controller.resolve("foo", "bar", "dev", request); + String resource = this.controller.retrieve("foo", "bar", "dev", request, true); assertEquals("foo: dev_bar/spam", resource); } + @Test + public void labelWithSlashForResolvePlaceholdersFalse() throws Exception { + this.environmentRepository.setSearchLocations("classpath:/test"); + String resource = this.controller.retrieve("foo", "bar", "dev(_)spam", "foo.txt", false); + assertEquals("foo: dev_bar/spam", resource); + } + + @Test + public void resourceWithSlashForResolvePlaceholdersFalse() throws Exception { + this.environmentRepository.setSearchLocations("classpath:/test"); + String resource = this.controller.retrieve("foo", "bar", "dev", "spam/foo.txt", false); + assertEquals("foo: dev_bar/spam", resource); + } + + @Test + public void resourceWithSlashForResolvePlaceholdersFalseRequest() throws Exception { + this.environmentRepository.setSearchLocations("classpath:/test"); + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setRequestURI("/foo/bar/dev/" + "spam/foo.txt"); + String resource = this.controller.retrieve("foo", "bar", "dev", request, false); + assertEquals("foo: dev_bar/spam", resource); + } + @Test public void labelWithSlashForBinary() throws Exception { this.environmentRepository.setSearchLocations("classpath:/test"); From 1152821221bd3eaa959981d8b750045b58466f5e Mon Sep 17 00:00:00 2001 From: ivasylyev Date: Thu, 2 Feb 2017 11:23:30 +0200 Subject: [PATCH 04/13] Add stacktrace logging --- .../CipherEnvironmentEncryptor.java | 9 +++-- .../encryption/EncryptionController.java | 26 +++++++-------- .../JGitEnvironmentRepository.java | 33 ++++++++++++------- .../MultipleJGitEnvironmentRepository.java | 4 +-- .../environment/NoSuchLabelException.java | 4 +++ .../environment/RepositoryException.java | 4 +++ .../SvnKitEnvironmentRepository.java | 10 ++++-- .../encryption/EncryptionControllerTests.java | 15 +++++++++ .../JGitEnvironmentRepositoryTests.java | 24 ++++++++++++++ 9 files changed, 95 insertions(+), 34 deletions(-) diff --git a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/encryption/CipherEnvironmentEncryptor.java b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/encryption/CipherEnvironmentEncryptor.java index a9f9a32f..04578abd 100644 --- a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/encryption/CipherEnvironmentEncryptor.java +++ b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/encryption/CipherEnvironmentEncryptor.java @@ -77,8 +77,13 @@ public class CipherEnvironmentEncryptor implements EnvironmentEncryptor { catch (Exception e) { value = ""; name = "invalid." + name; - logger.warn("Cannot decrypt key: " + key + " (" + e.getClass() - + ": " + e.getMessage() + ")"); + String message = "Cannot decrypt key: " + key + " (" + e.getClass() + + ": " + e.getMessage() + ")"; + if (logger.isDebugEnabled()) { + logger.debug(message, e); + } else if (logger.isWarnEnabled()) { + logger.warn(message); + } } map.put(name, value); } diff --git a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/encryption/EncryptionController.java b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/encryption/EncryptionController.java index 99825320..8e2162f2 100644 --- a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/encryption/EncryptionController.java +++ b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/encryption/EncryptionController.java @@ -125,19 +125,14 @@ public class EncryptionController { public String encrypt(@PathVariable String name, @PathVariable String profiles, @RequestBody String data, @RequestHeader("Content-Type") MediaType type) { checkEncryptorInstalled(name, profiles); - try { - String input = stripFormData(data, type, false); - Map keys = this.helper.getEncryptorKeys(name, profiles, - input); - String textToEncrypt = this.helper.stripPrefix(input); - String encrypted = this.helper.addPrefix(keys, - this.encryptor.locate(keys).encrypt(textToEncrypt)); - logger.info("Encrypted data"); - return encrypted; - } - catch (IllegalArgumentException e) { - throw new InvalidCipherException(); - } + String input = stripFormData(data, type, false); + Map keys = this.helper.getEncryptorKeys(name, profiles, + input); + String textToEncrypt = this.helper.stripPrefix(input); + String encrypted = this.helper.addPrefix(keys, + this.encryptor.locate(keys).encrypt(textToEncrypt)); + logger.info("Encrypted data"); + return encrypted; } @RequestMapping(value = "decrypt", method = RequestMethod.POST) @@ -161,7 +156,8 @@ public class EncryptionController { logger.info("Decrypted cipher data"); return decrypted; } - catch (IllegalArgumentException e) { + catch (IllegalArgumentException|IllegalStateException e) { + logger.error("Cannot decrypt key:" + name + ", value:" + data, e); throw new InvalidCipherException(); } } @@ -241,4 +237,4 @@ class KeyNotAvailableException extends RuntimeException { @SuppressWarnings("serial") class InvalidCipherException extends RuntimeException { -} +} \ No newline at end of file diff --git a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/JGitEnvironmentRepository.java b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/JGitEnvironmentRepository.java index cb544e7c..ae7564d3 100644 --- a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/JGitEnvironmentRepository.java +++ b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/JGitEnvironmentRepository.java @@ -197,7 +197,7 @@ public class JGitEnvironmentRepository extends AbstractScmEnvironmentRepository return git.getRepository().getRef("HEAD").getObjectId().getName(); } catch (RefNotFoundException e) { - throw new NoSuchLabelException("No such label: " + label); + throw new NoSuchLabelException("No such label: " + label, e); } catch (GitAPIException e) { throw new IllegalStateException("Cannot clone or checkout repository", e); @@ -302,14 +302,15 @@ public class JGitEnvironmentRepository extends AbstractScmEnvironmentRepository setCredentialsProvider(fetch); FetchResult result = fetch.call(); if(result.getTrackingRefUpdates() != null && result.getTrackingRefUpdates().size() > 0) { - this.logger.info("Fetched for remote " + label + " and found " + result.getTrackingRefUpdates().size() + logger.info("Fetched for remote " + label + " and found " + result.getTrackingRefUpdates().size() + " updates"); } return result; } catch (Exception ex) { - this.logger.warn("Could not fetch remote for " + label + " remote: " + git - .getRepository().getConfig().getString("remote", "origin", "url")); + String message = "Could not fetch remote for " + label + " remote: " + git + .getRepository().getConfig().getString("remote", "origin", "url"); + warn(message, ex); return null; } } @@ -325,8 +326,9 @@ public class JGitEnvironmentRepository extends AbstractScmEnvironmentRepository return result; } catch (Exception ex) { - this.logger.warn("Could not merge remote for " + label + " remote: " + git - .getRepository().getConfig().getString("remote", "origin", "url")); + String message = "Could not merge remote for " + label + " remote: " + git + .getRepository().getConfig().getString("remote", "origin", "url"); + warn(message, ex); return null; } } @@ -343,9 +345,10 @@ public class JGitEnvironmentRepository extends AbstractScmEnvironmentRepository return resetRef; } catch (Exception ex) { - this.logger.warn("Could not reset to remote for " + label + " (current ref=" + String message = "Could not reset to remote for " + label + " (current ref=" + ref + "), remote: " + git.getRepository().getConfig() - .getString("remote", "origin", "url")); + .getString("remote", "origin", "url"); + warn(message, ex); return null; } } @@ -449,10 +452,9 @@ public class JGitEnvironmentRepository extends AbstractScmEnvironmentRepository return status.call().isClean(); } catch (Exception e) { - this.logger - .warn("Could not execute status command on local repository. Cause: (" - + e.getClass().getSimpleName() + ") " + e.getMessage()); - + String message = "Could not execute status command on local repository. Cause: (" + + e.getClass().getSimpleName() + ") " + e.getMessage(); + warn(message, e); return false; } } @@ -486,6 +488,13 @@ public class JGitEnvironmentRepository extends AbstractScmEnvironmentRepository return false; } + protected void warn(String message, Exception ex) { + logger.warn(message); + if (logger.isDebugEnabled()) { + logger.debug("Stacktrace for: " + message, ex); + } + } + /** * Wraps the static method calls to {@link org.eclipse.jgit.api.Git} and * {@link org.eclipse.jgit.api.CloneCommand} allowing for easier unit testing. diff --git a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/MultipleJGitEnvironmentRepository.java b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/MultipleJGitEnvironmentRepository.java index 31a57708..83e335ae 100644 --- a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/MultipleJGitEnvironmentRepository.java +++ b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/MultipleJGitEnvironmentRepository.java @@ -119,7 +119,7 @@ public class MultipleJGitEnvironmentRepository extends JGitEnvironmentRepository if (logger.isDebugEnabled()) { this.logger.debug("Cannot retrieve resource locations from " + candidate.getUri() + ", cause: (" - + e.getClass().getSimpleName() + ") " + e.getMessage()); + + e.getClass().getSimpleName() + ") " + e.getMessage(), e); } continue; } @@ -154,7 +154,7 @@ public class MultipleJGitEnvironmentRepository extends JGitEnvironmentRepository if (logger.isDebugEnabled()) { this.logger.debug("Cannot load configuration from " + candidate.getUri() + ", cause: (" - + e.getClass().getSimpleName() + ") " + e.getMessage()); + + e.getClass().getSimpleName() + ") " + e.getMessage(), e); } continue; } diff --git a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/NoSuchLabelException.java b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/NoSuchLabelException.java index dcf3f8d2..e4939307 100644 --- a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/NoSuchLabelException.java +++ b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/NoSuchLabelException.java @@ -27,4 +27,8 @@ public class NoSuchLabelException extends RepositoryException { super(string); } + public NoSuchLabelException(String string, Exception e) { + super(string, e); + } + } diff --git a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/RepositoryException.java b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/RepositoryException.java index 71e2efa9..0803bc1e 100644 --- a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/RepositoryException.java +++ b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/RepositoryException.java @@ -27,4 +27,8 @@ public class RepositoryException extends RuntimeException { super(string); } + public RepositoryException(String message, Throwable cause) { + super(message, cause); + } + } diff --git a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/SvnKitEnvironmentRepository.java b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/SvnKitEnvironmentRepository.java index af19b126..30a7097f 100644 --- a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/SvnKitEnvironmentRepository.java +++ b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/SvnKitEnvironmentRepository.java @@ -146,9 +146,13 @@ public class SvnKitEnvironmentRepository extends AbstractScmEnvironmentRepositor return version.toString(); } catch (Exception e) { - this.logger.warn("Could not update remote for " + label + " (current local=" - + getWorkingDirectory().getPath() + "), remote: " + this.getUri() - + ")"); + String message = "Could not update remote for " + label + " (current local=" + + getWorkingDirectory().getPath() + "), remote: " + this.getUri() + ")"; + if (logger.isDebugEnabled()) { + logger.debug(message, e); + } else if (logger.isWarnEnabled()) { + logger.warn(message); + } } final SVNStatus status = SVNClientManager.newInstance().getStatusClient() diff --git a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/encryption/EncryptionControllerTests.java b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/encryption/EncryptionControllerTests.java index cc0c7797..2ff4e23c 100644 --- a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/encryption/EncryptionControllerTests.java +++ b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/encryption/EncryptionControllerTests.java @@ -55,6 +55,21 @@ public class EncryptionControllerTests { this.controller.decrypt("foo", MediaType.TEXT_PLAIN); } + @Test(expected = InvalidCipherException.class) + public void shouldThrowExceptionOnDecryptInvalidData() { + this.controller = new EncryptionController( + new SingleTextEncryptorLocator(new RsaSecretEncryptor())); + controller.decrypt("foo", MediaType.TEXT_PLAIN); + } + + @Test(expected = InvalidCipherException.class) + public void shouldThrowExceptionOnDecryptWrongKey() { + RsaSecretEncryptor encryptor = new RsaSecretEncryptor(); + this.controller = new EncryptionController( + new SingleTextEncryptorLocator(new RsaSecretEncryptor())); + controller.decrypt(encryptor.encrypt("foo"), MediaType.TEXT_PLAIN); + } + @Test public void sunnyDayRsaKey() { this.controller = new EncryptionController( diff --git a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/environment/JGitEnvironmentRepositoryTests.java b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/environment/JGitEnvironmentRepositoryTests.java index 0637eac4..6e6e6914 100644 --- a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/environment/JGitEnvironmentRepositoryTests.java +++ b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/environment/JGitEnvironmentRepositoryTests.java @@ -23,6 +23,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; +import org.apache.commons.logging.Log; import org.eclipse.jgit.api.CheckoutCommand; import org.eclipse.jgit.api.CloneCommand; import org.eclipse.jgit.api.FetchCommand; @@ -72,7 +73,9 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertThat; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyString; +import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockingDetails; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -703,6 +706,27 @@ public class JGitEnvironmentRepositoryTests { } } + @Test + public void shouldPrintStacktraceIfDebugEnabled() throws Exception { + final Log mockLogger = mock(Log.class); + JGitEnvironmentRepository envRepository = new JGitEnvironmentRepository(this.environment){ + @Override + public void afterPropertiesSet() throws Exception { + this.logger = mockLogger; + } + }; + envRepository.afterPropertiesSet(); + when(mockLogger.isDebugEnabled()).thenReturn(true); + + envRepository.warn("", new RuntimeException()); + + verify(mockLogger).warn(eq("")); + verify(mockLogger).debug(eq("Stacktrace for: "), any(RuntimeException.class)); + + int numberOfInvocations = mockingDetails(mockLogger).getInvocations().size(); + assertEquals("should call isDebugEnabled warn and debug", 3, numberOfInvocations); + } + class MockCloneCommand extends CloneCommand { private Git mockGit; From 62ef648b599f310f10240f6cdaa685e467fca4dc Mon Sep 17 00:00:00 2001 From: Spencer Gibb Date: Fri, 31 Mar 2017 14:18:30 -0600 Subject: [PATCH 05/13] polish --- .../src/test/java/sample/ApplicationFailFastTests.java | 3 ++- .../AwsCodeCommitCredentialsProviderTests.java | 9 +++++++-- .../credentials/GitCredentialsProviderFactoryTests.java | 8 ++++++-- 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/spring-cloud-config-sample/src/test/java/sample/ApplicationFailFastTests.java b/spring-cloud-config-sample/src/test/java/sample/ApplicationFailFastTests.java index ddaf5206..b2393fe9 100644 --- a/spring-cloud-config-sample/src/test/java/sample/ApplicationFailFastTests.java +++ b/spring-cloud-config-sample/src/test/java/sample/ApplicationFailFastTests.java @@ -3,7 +3,8 @@ package sample; import org.junit.Test; import org.springframework.boot.builder.SpringApplicationBuilder; -import static org.junit.Assert.*; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; public class ApplicationFailFastTests { diff --git a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/credentials/AwsCodeCommitCredentialsProviderTests.java b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/credentials/AwsCodeCommitCredentialsProviderTests.java index faf92b16..634d1bfd 100644 --- a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/credentials/AwsCodeCommitCredentialsProviderTests.java +++ b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/credentials/AwsCodeCommitCredentialsProviderTests.java @@ -16,8 +16,6 @@ package org.springframework.cloud.config.server.credentials; -import static org.junit.Assert.*; - import java.net.URISyntaxException; import org.eclipse.jgit.errors.UnsupportedCredentialItem; @@ -30,6 +28,13 @@ import org.springframework.cloud.config.server.support.GitCredentialsProviderFac import com.amazonaws.auth.AWSCredentialsProvider; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + /** * It would be nice to do an integration test, however, this would require * using real AWS credentials. How can we test the credential generation diff --git a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/credentials/GitCredentialsProviderFactoryTests.java b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/credentials/GitCredentialsProviderFactoryTests.java index 4f315ee1..7ed5107e 100644 --- a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/credentials/GitCredentialsProviderFactoryTests.java +++ b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/credentials/GitCredentialsProviderFactoryTests.java @@ -16,8 +16,6 @@ package org.springframework.cloud.config.server.credentials; -import static org.junit.Assert.*; - import org.eclipse.jgit.transport.CredentialsProvider; import org.eclipse.jgit.transport.UsernamePasswordCredentialsProvider; import org.junit.Before; @@ -26,6 +24,12 @@ import org.springframework.cloud.config.server.support.AwsCodeCommitCredentialPr import org.springframework.cloud.config.server.support.GitCredentialsProviderFactory; import org.springframework.cloud.config.server.support.PassphraseCredentialsProvider; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + /** * @author don laidlaw * From 1b701ebdb6e999d40e6e0cdf8936ab792d00a503 Mon Sep 17 00:00:00 2001 From: George Harley Date: Mon, 3 Apr 2017 18:03:00 +0100 Subject: [PATCH 06/13] Stop VaultEnvironmentRepository returning duplicate property sources (#679) When findOne() is called with an application parameter that matches the defaultKey property value then the returned Environment contains two identical PropertySource values when it should only contain one. --- .../VaultEnvironmentRepository.java | 2 +- .../VaultEnvironmentRepositoryTests.java | 100 ++++++++++++++++-- 2 files changed, 93 insertions(+), 9 deletions(-) diff --git a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/VaultEnvironmentRepository.java b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/VaultEnvironmentRepository.java index efa44779..6ac6d758 100644 --- a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/VaultEnvironmentRepository.java +++ b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/VaultEnvironmentRepository.java @@ -128,7 +128,7 @@ public class VaultEnvironmentRepository implements EnvironmentRepository, Ordere private List findKeys(String application, List profiles) { List keys = new ArrayList<>(); - if (StringUtils.hasText(this.defaultKey)) { + if (StringUtils.hasText(this.defaultKey) && !this.defaultKey.equals(application)) { keys.add(this.defaultKey); addProfiles(keys, this.defaultKey, profiles); } diff --git a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/environment/VaultEnvironmentRepositoryTests.java b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/environment/VaultEnvironmentRepositoryTests.java index 9b9ae1b5..dda69d32 100644 --- a/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/environment/VaultEnvironmentRepositoryTests.java +++ b/spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/environment/VaultEnvironmentRepositoryTests.java @@ -1,11 +1,8 @@ package org.springframework.cloud.config.server.environment; -import static org.junit.Assert.assertEquals; - import java.io.IOException; import java.util.HashMap; import java.util.Map; - import org.junit.Before; import org.junit.Test; import org.mockito.Mockito; @@ -17,6 +14,8 @@ import org.springframework.http.ResponseEntity; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.web.client.RestTemplate; +import static org.junit.Assert.assertEquals; + /** * @author Spencer Gibb * @author Ryan Baxter @@ -27,10 +26,11 @@ public class VaultEnvironmentRepositoryTests { public void init() {} @Test - public void testFindOne() throws IOException { + public void testFindOneNoDefaultKey() throws IOException { MockHttpServletRequest configRequest = new MockHttpServletRequest(); configRequest.addHeader("X-CONFIG-TOKEN", "mytoken"); RestTemplate rest = Mockito.mock(RestTemplate.class); + ResponseEntity myAppResp = Mockito.mock(ResponseEntity.class); Mockito.when(myAppResp.getStatusCode()).thenReturn(HttpStatus.OK); VaultEnvironmentRepository.VaultResponse myAppVaultResp = Mockito.mock(VaultEnvironmentRepository.VaultResponse.class); @@ -39,20 +39,104 @@ public class VaultEnvironmentRepositoryTests { Mockito.when(rest.exchange(Mockito.eq("http://127.0.0.1:8200/v1/{backend}/{key}"), Mockito.eq(HttpMethod.GET), Mockito.any(HttpEntity.class), Mockito.eq(VaultEnvironmentRepository.VaultResponse.class), Mockito.eq("secret"), Mockito.eq("myapp"))).thenReturn(myAppResp); + ResponseEntity appResp = Mockito.mock(ResponseEntity.class); Mockito.when(appResp.getStatusCode()).thenReturn(HttpStatus.OK); VaultEnvironmentRepository.VaultResponse appVaultResp = Mockito.mock(VaultEnvironmentRepository.VaultResponse.class); - Mockito.when(appVaultResp.getData()).thenReturn(null); + Mockito.when(appVaultResp.getData()).thenReturn("{\"def-foo\":\"def-bar\"}"); Mockito.when(appResp.getBody()).thenReturn(appVaultResp); Mockito.when(rest.exchange(Mockito.eq("http://127.0.0.1:8200/v1/{backend}/{key}"), Mockito.eq(HttpMethod.GET), Mockito.any(HttpEntity.class), Mockito.eq(VaultEnvironmentRepository.VaultResponse.class), Mockito.eq("secret"), Mockito.eq("application"))).thenReturn(appResp); + VaultEnvironmentRepository repo = new VaultEnvironmentRepository(configRequest, new EnvironmentWatch.Default(), rest); + Environment e = repo.findOne("myapp", null, null); - assertEquals("myapp", e.getName()); - Map result = new HashMap(); + assertEquals("Name should be the same as the application argument", "myapp", e.getName()); + assertEquals("Properties for specified application and default application with key 'application' should be returned", 2, e.getPropertySources().size()); + Map firstResult = new HashMap(); + firstResult.put("foo", "bar"); + assertEquals("Properties for specified application should be returned in priority position", firstResult, e.getPropertySources().get(0).getSource()); + + Map secondResult = new HashMap(); + secondResult.put("def-foo", "def-bar"); + assertEquals("Properties for default application with key 'application' should be returned in second position", secondResult, e.getPropertySources().get(1).getSource()); + } + + @Test + public void testFindOneDefaultKeySetAndDifferentToApplication() throws IOException { + MockHttpServletRequest configRequest = new MockHttpServletRequest(); + configRequest.addHeader("X-CONFIG-TOKEN", "mytoken"); + RestTemplate rest = Mockito.mock(RestTemplate.class); + + ResponseEntity myAppResp = Mockito.mock(ResponseEntity.class); + Mockito.when(myAppResp.getStatusCode()).thenReturn(HttpStatus.OK); + VaultEnvironmentRepository.VaultResponse myAppVaultResp = Mockito.mock(VaultEnvironmentRepository.VaultResponse.class); + Mockito.when(myAppVaultResp.getData()).thenReturn("{\"foo\":\"bar\"}"); + Mockito.when(myAppResp.getBody()).thenReturn(myAppVaultResp); + Mockito.when(rest.exchange(Mockito.eq("http://127.0.0.1:8200/v1/{backend}/{key}"), + Mockito.eq(HttpMethod.GET), Mockito.any(HttpEntity.class), Mockito.eq(VaultEnvironmentRepository.VaultResponse.class), + Mockito.eq("secret"), Mockito.eq("myapp"))).thenReturn(myAppResp); + + ResponseEntity myDefaultKeyResp = Mockito.mock(ResponseEntity.class); + Mockito.when(myDefaultKeyResp.getStatusCode()).thenReturn(HttpStatus.OK); + VaultEnvironmentRepository.VaultResponse myDefaultKeyVaultResp = Mockito.mock(VaultEnvironmentRepository.VaultResponse.class); + Mockito.when(myDefaultKeyVaultResp.getData()).thenReturn("{\"def-foo\":\"def-bar\"}"); + Mockito.when(myDefaultKeyResp.getBody()).thenReturn(myDefaultKeyVaultResp); + Mockito.when(rest.exchange(Mockito.eq("http://127.0.0.1:8200/v1/{backend}/{key}"), + Mockito.eq(HttpMethod.GET), Mockito.any(HttpEntity.class), Mockito.eq(VaultEnvironmentRepository.VaultResponse.class), + Mockito.eq("secret"), Mockito.eq("mydefaultkey"))).thenReturn(myDefaultKeyResp); + + VaultEnvironmentRepository repo = new VaultEnvironmentRepository(configRequest, new EnvironmentWatch.Default(), rest); + repo.setDefaultKey("mydefaultkey"); + + Environment e = repo.findOne("myapp", null, null); + assertEquals("Name should be the same as the application argument", "myapp", e.getName()); + assertEquals("Properties for specified application and default application with key 'mydefaultkey' should be returned", 2, e.getPropertySources().size()); + + Map firstResult = new HashMap(); + firstResult.put("foo", "bar"); + assertEquals("Properties for specified application should be returned in priority position", firstResult, e.getPropertySources().get(0).getSource()); + + Map secondResult = new HashMap(); + secondResult.put("def-foo", "def-bar"); + assertEquals("Properties for default application with key 'mydefaultkey' should be returned in second position", secondResult, e.getPropertySources().get(1).getSource()); + } + + @Test + public void testFindOneDefaultKeySetAndEqualToApplication() throws IOException { + MockHttpServletRequest configRequest = new MockHttpServletRequest(); + configRequest.addHeader("X-CONFIG-TOKEN", "mytoken"); + RestTemplate rest = Mockito.mock(RestTemplate.class); + + ResponseEntity myAppResp = Mockito.mock(ResponseEntity.class); + Mockito.when(myAppResp.getStatusCode()).thenReturn(HttpStatus.OK); + VaultEnvironmentRepository.VaultResponse myAppVaultResp = Mockito.mock(VaultEnvironmentRepository.VaultResponse.class); + Mockito.when(myAppVaultResp.getData()).thenReturn("{\"foo\":\"bar\"}"); + Mockito.when(myAppResp.getBody()).thenReturn(myAppVaultResp); + Mockito.when(rest.exchange(Mockito.eq("http://127.0.0.1:8200/v1/{backend}/{key}"), + Mockito.eq(HttpMethod.GET), Mockito.any(HttpEntity.class), Mockito.eq(VaultEnvironmentRepository.VaultResponse.class), + Mockito.eq("secret"), Mockito.eq("myapp"))).thenReturn(myAppResp); + + ResponseEntity appResp = Mockito.mock(ResponseEntity.class); + Mockito.when(appResp.getStatusCode()).thenReturn(HttpStatus.OK); + VaultEnvironmentRepository.VaultResponse appVaultResp = Mockito.mock(VaultEnvironmentRepository.VaultResponse.class); + Mockito.when(appVaultResp.getData()).thenReturn("{\"def-foo\":\"def-bar\"}"); + Mockito.when(appResp.getBody()).thenReturn(appVaultResp); + Mockito.when(rest.exchange(Mockito.eq("http://127.0.0.1:8200/v1/{backend}/{key}"), + Mockito.eq(HttpMethod.GET), Mockito.any(HttpEntity.class), Mockito.eq(VaultEnvironmentRepository.VaultResponse.class), + Mockito.eq("secret"), Mockito.eq("application"))).thenReturn(appResp); + + VaultEnvironmentRepository repo = new VaultEnvironmentRepository(configRequest, new EnvironmentWatch.Default(), rest); + repo.setDefaultKey("myapp"); + + Environment e = repo.findOne("myapp", null, null); + assertEquals("Name should be the same as the application argument", "myapp", e.getName()); + assertEquals("Only properties for specified application should be returned", 1, e.getPropertySources().size()); + + Map result = new HashMap(); result.put("foo", "bar"); - assertEquals(result, e.getPropertySources().get(0).getSource()); + assertEquals("Properties should be returned for specified application", result, e.getPropertySources().get(0).getSource()); } @Test(expected = IllegalArgumentException.class) From 45336cd11f89419b8852e1f5016528cb5293809f Mon Sep 17 00:00:00 2001 From: Mathieu Ouellet Date: Thu, 2 Mar 2017 09:14:20 -0500 Subject: [PATCH 07/13] Add headers configuration property This commit provides an alternative implementation for creating config client request using the headers config prop instead of having to create custom 'RestTemplate' when adding header to the request. See discusions at gh-650 Not covering gh-613 --- .../config/client/ConfigClientProperties.java | 18 ++++ .../ConfigServicePropertySourceLocator.java | 51 +++++------ ...nfigServicePropertySourceLocatorTests.java | 84 +++++++++++++++++++ 3 files changed, 121 insertions(+), 32 deletions(-) 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 619d924a..4bf3d3f4 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 @@ -18,10 +18,13 @@ package org.springframework.cloud.config.client; import java.net.MalformedURLException; import java.net.URL; +import java.util.HashMap; +import java.util.Map; import org.springframework.beans.BeanUtils; import org.springframework.beans.factory.annotation.Value; import org.springframework.boot.context.properties.ConfigurationProperties; +import org.springframework.boot.context.properties.DeprecatedConfigurationProperty; import org.springframework.core.env.Environment; import org.springframework.util.StringUtils; import org.springframework.web.util.UriComponentsBuilder; @@ -95,6 +98,11 @@ public class ConfigClientProperties { */ private String authorization; + /** + * Additional headers used to create the client request. + */ + private Map headers = new HashMap<>(); + private ConfigClientProperties() { } @@ -190,6 +198,8 @@ public class ConfigClientProperties { this.token = token; } + @DeprecatedConfigurationProperty(reason = "replaced by headers", replacement = "headers") + @Deprecated public String getAuthorization() { return this.authorization; } @@ -198,6 +208,14 @@ public class ConfigClientProperties { this.authorization = authorization; } + public Map getHeaders() { + return headers; + } + + public void setHeaders(Map headers) { + this.headers = headers; + } + private Credentials extractCredentials() { Credentials result = new Credentials(); String uri = this.uri; 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 2eef9879..478b4615 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 @@ -20,6 +20,7 @@ import java.io.IOException; import java.util.Arrays; import java.util.HashMap; import java.util.Map; +import java.util.Map.Entry; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -52,6 +53,7 @@ import static org.springframework.cloud.config.client.ConfigClientProperties.TOK /** * @author Dave Syer + * @author Mathieu Ouellet * */ @Order(0) @@ -191,8 +193,10 @@ public class ConfigServicePropertySourceLocator implements PropertySourceLocator SimpleClientHttpRequestFactory requestFactory = new SimpleClientHttpRequestFactory(); 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 (password != null && authorization != null) { throw new IllegalStateException( @@ -200,53 +204,36 @@ public class ConfigServicePropertySourceLocator implements PropertySourceLocator } if (password != null) { - template.setInterceptors(Arrays. asList( - new BasicAuthorizationInterceptor(client.getUsername(), password))); + byte[] token = Base64Utils.encode((username + ":" + password).getBytes()); + headers.put("Authorization", "Basic " + new String(token)); } else if (authorization != null) { + headers.put("Authorization", authorization); + } + + if (!headers.isEmpty()) { template.setInterceptors(Arrays. asList( - new GenericAuthorization(authorization))); + new GenericRequestHeaderInterceptor(headers))); } return template; } - private static class BasicAuthorizationInterceptor implements - ClientHttpRequestInterceptor { + public static class GenericRequestHeaderInterceptor + implements ClientHttpRequestInterceptor { - private final String username; + private final Map headers; - private final String password; - - public BasicAuthorizationInterceptor(String username, String password) { - this.username = username; - this.password = (password == null ? "" : password); + public GenericRequestHeaderInterceptor(Map headers) { + this.headers = headers; } @Override public ClientHttpResponse intercept(HttpRequest request, byte[] body, ClientHttpRequestExecution execution) throws IOException { - byte[] token = Base64Utils.encode((this.username + ":" + this.password).getBytes()); - request.getHeaders().add("Authorization", "Basic " + new String(token)); - return execution.execute(request, body); - } - - } - - - private static class GenericAuthorization implements - ClientHttpRequestInterceptor { - - private final String authorizationToken; - - public GenericAuthorization(String authorizationToken) { - this.authorizationToken = (authorizationToken == null ? "" : authorizationToken); - } - - @Override - public ClientHttpResponse intercept(HttpRequest request, byte[] body, ClientHttpRequestExecution execution) - throws IOException { - request.getHeaders().add("Authorization", authorizationToken); + for (Entry header : headers.entrySet()) { + request.getHeaders().add(header.getKey(), header.getValue()); + } return execution.execute(request, body); } } 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 ad727030..d9bdd904 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 @@ -1,10 +1,13 @@ package org.springframework.cloud.config.client; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import java.io.ByteArrayInputStream; import java.net.URI; +import java.util.HashMap; +import java.util.Map; import org.hamcrest.core.IsInstanceOf; import org.hamcrest.core.IsNull; @@ -24,8 +27,11 @@ import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; import org.springframework.http.ResponseEntity; import org.springframework.http.client.ClientHttpRequest; +import org.springframework.http.client.ClientHttpRequestExecution; import org.springframework.http.client.ClientHttpRequestFactory; import org.springframework.http.client.ClientHttpResponse; +import org.springframework.mock.http.client.MockClientHttpRequest; +import org.springframework.test.util.ReflectionTestUtils; import org.springframework.web.client.HttpServerErrorException; import org.springframework.web.client.RestTemplate; @@ -134,6 +140,84 @@ public class ConfigServicePropertySourceLocatorTests { assertNull(this.locator.locate(this.environment)); } + @Test + public void failFastWhenBothPasswordAndAuthorizationPropertiesSet() 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.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'"); + assertNull(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(); + ClientHttpRequestExecution execution = Mockito + .mock(ClientHttpRequestExecution.class); + byte[] body = new byte[] {}; + Map headers = new HashMap<>(); + headers.put("X-Example-Version", "2.1"); + new ConfigServicePropertySourceLocator.GenericRequestHeaderInterceptor(headers) + .intercept(request, body, execution); + Mockito.verify(execution).execute(request, body); + assertThat(request.getHeaders().getFirst("X-Example-Version")).isEqualTo("2.1"); + } + @SuppressWarnings("unchecked") private void mockRequestResponseWithLabel(ResponseEntity response, String label) { Mockito.when( From 8c36922d21926c4274e87c7fe1afc81cd9c00685 Mon Sep 17 00:00:00 2001 From: Abhijit Sarkar Date: Thu, 6 Apr 2017 06:43:59 +0530 Subject: [PATCH 08/13] Making refresh modifier public so that clients can do AOP failover (#634) --- .../config/server/environment/JGitEnvironmentRepository.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/JGitEnvironmentRepository.java b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/JGitEnvironmentRepository.java index ae7564d3..45b460b2 100644 --- a/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/JGitEnvironmentRepository.java +++ b/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/JGitEnvironmentRepository.java @@ -170,7 +170,7 @@ public class JGitEnvironmentRepository extends AbstractScmEnvironmentRepository /** * Get the working directory ready. */ - private String refresh(String label) { + public String refresh(String label) { initialize(); Git git = null; try { From e051d569651dc011ae12d58e8e22fa5c649e1c10 Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Fri, 7 Apr 2017 04:55:32 -0400 Subject: [PATCH 09/13] Update SNAPSHOT to 1.3.0.RELEASE --- docs/pom.xml | 2 +- pom.xml | 6 +++--- spring-cloud-config-client/pom.xml | 2 +- spring-cloud-config-dependencies/pom.xml | 4 ++-- spring-cloud-config-monitor/pom.xml | 4 ++-- spring-cloud-config-sample/pom.xml | 2 +- spring-cloud-config-server/pom.xml | 2 +- spring-cloud-starter-config/pom.xml | 4 ++-- 8 files changed, 13 insertions(+), 13 deletions(-) diff --git a/docs/pom.xml b/docs/pom.xml index a03d8a2b..e6f4ccad 100644 --- a/docs/pom.xml +++ b/docs/pom.xml @@ -6,7 +6,7 @@ org.springframework.cloud spring-cloud-config - 1.3.0.BUILD-SNAPSHOT + 1.3.0.RELEASE pom Spring Cloud Config Docs diff --git a/pom.xml b/pom.xml index a89c4d16..306df061 100644 --- a/pom.xml +++ b/pom.xml @@ -3,7 +3,7 @@ 4.0.0 org.springframework.cloud spring-cloud-config - 1.3.0.BUILD-SNAPSHOT + 1.3.0.RELEASE pom Spring Cloud Config Spring Cloud Config @@ -11,7 +11,7 @@ org.springframework.cloud spring-cloud-build - 1.3.1.BUILD-SNAPSHOT + 1.3.1.RELEASE @@ -22,7 +22,7 @@ config - 1.2.0.BUILD-SNAPSHOT + 1.2.0.RELEASE spring-cloud-config-dependencies diff --git a/spring-cloud-config-client/pom.xml b/spring-cloud-config-client/pom.xml index fdefa944..d2a90265 100644 --- a/spring-cloud-config-client/pom.xml +++ b/spring-cloud-config-client/pom.xml @@ -10,7 +10,7 @@ org.springframework.cloud spring-cloud-config - 1.3.0.BUILD-SNAPSHOT + 1.3.0.RELEASE .. diff --git a/spring-cloud-config-dependencies/pom.xml b/spring-cloud-config-dependencies/pom.xml index 5677b4af..fe2113d9 100644 --- a/spring-cloud-config-dependencies/pom.xml +++ b/spring-cloud-config-dependencies/pom.xml @@ -5,11 +5,11 @@ spring-cloud-dependencies-parent org.springframework.cloud - 1.3.0.BUILD-SNAPSHOT + 1.3.1.RELEASE spring-cloud-config-dependencies - 1.3.0.BUILD-SNAPSHOT + 1.3.0.RELEASE pom spring-cloud-config-dependencies Spring Cloud Config Dependencies diff --git a/spring-cloud-config-monitor/pom.xml b/spring-cloud-config-monitor/pom.xml index 8fdc64af..2b3ffe16 100644 --- a/spring-cloud-config-monitor/pom.xml +++ b/spring-cloud-config-monitor/pom.xml @@ -5,7 +5,7 @@ org.springframework.cloud spring-cloud-config - 1.3.0.BUILD-SNAPSHOT + 1.3.0.RELEASE .. spring-cloud-config-monitor @@ -13,7 +13,7 @@ Spring Cloud Config Monitor ${basedir}/../.. - 1.3.0.BUILD-SNAPSHOT + 1.3.0.RELEASE diff --git a/spring-cloud-config-sample/pom.xml b/spring-cloud-config-sample/pom.xml index 113c6933..ca9887bc 100644 --- a/spring-cloud-config-sample/pom.xml +++ b/spring-cloud-config-sample/pom.xml @@ -12,7 +12,7 @@ org.springframework.cloud spring-cloud-config - 1.3.0.BUILD-SNAPSHOT + 1.3.0.RELEASE .. diff --git a/spring-cloud-config-server/pom.xml b/spring-cloud-config-server/pom.xml index b1b5982e..ac27b474 100644 --- a/spring-cloud-config-server/pom.xml +++ b/spring-cloud-config-server/pom.xml @@ -12,7 +12,7 @@ org.springframework.cloud spring-cloud-config - 1.3.0.BUILD-SNAPSHOT + 1.3.0.RELEASE .. diff --git a/spring-cloud-starter-config/pom.xml b/spring-cloud-starter-config/pom.xml index 1a379652..c6af2404 100644 --- a/spring-cloud-starter-config/pom.xml +++ b/spring-cloud-starter-config/pom.xml @@ -5,10 +5,10 @@ org.springframework.cloud spring-cloud-config - 1.3.0.BUILD-SNAPSHOT + 1.3.0.RELEASE spring-cloud-starter-config - 1.3.0.BUILD-SNAPSHOT + 1.3.0.RELEASE spring-cloud-starter-config Spring Cloud Starter https://projects.spring.io/spring-cloud From 7bada56f306a0ea9bdb79474cb33a9b4715f9e12 Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Fri, 7 Apr 2017 05:01:59 -0400 Subject: [PATCH 10/13] Going back to snapshots --- docs/pom.xml | 2 +- pom.xml | 6 +++--- spring-cloud-config-client/pom.xml | 2 +- spring-cloud-config-dependencies/pom.xml | 4 ++-- spring-cloud-config-monitor/pom.xml | 4 ++-- spring-cloud-config-sample/pom.xml | 2 +- spring-cloud-config-server/pom.xml | 2 +- spring-cloud-starter-config/pom.xml | 4 ++-- 8 files changed, 13 insertions(+), 13 deletions(-) diff --git a/docs/pom.xml b/docs/pom.xml index e6f4ccad..a03d8a2b 100644 --- a/docs/pom.xml +++ b/docs/pom.xml @@ -6,7 +6,7 @@ org.springframework.cloud spring-cloud-config - 1.3.0.RELEASE + 1.3.0.BUILD-SNAPSHOT pom Spring Cloud Config Docs diff --git a/pom.xml b/pom.xml index 306df061..a89c4d16 100644 --- a/pom.xml +++ b/pom.xml @@ -3,7 +3,7 @@ 4.0.0 org.springframework.cloud spring-cloud-config - 1.3.0.RELEASE + 1.3.0.BUILD-SNAPSHOT pom Spring Cloud Config Spring Cloud Config @@ -11,7 +11,7 @@ org.springframework.cloud spring-cloud-build - 1.3.1.RELEASE + 1.3.1.BUILD-SNAPSHOT @@ -22,7 +22,7 @@ config - 1.2.0.RELEASE + 1.2.0.BUILD-SNAPSHOT spring-cloud-config-dependencies diff --git a/spring-cloud-config-client/pom.xml b/spring-cloud-config-client/pom.xml index d2a90265..fdefa944 100644 --- a/spring-cloud-config-client/pom.xml +++ b/spring-cloud-config-client/pom.xml @@ -10,7 +10,7 @@ org.springframework.cloud spring-cloud-config - 1.3.0.RELEASE + 1.3.0.BUILD-SNAPSHOT .. diff --git a/spring-cloud-config-dependencies/pom.xml b/spring-cloud-config-dependencies/pom.xml index fe2113d9..5677b4af 100644 --- a/spring-cloud-config-dependencies/pom.xml +++ b/spring-cloud-config-dependencies/pom.xml @@ -5,11 +5,11 @@ spring-cloud-dependencies-parent org.springframework.cloud - 1.3.1.RELEASE + 1.3.0.BUILD-SNAPSHOT spring-cloud-config-dependencies - 1.3.0.RELEASE + 1.3.0.BUILD-SNAPSHOT pom spring-cloud-config-dependencies Spring Cloud Config Dependencies diff --git a/spring-cloud-config-monitor/pom.xml b/spring-cloud-config-monitor/pom.xml index 2b3ffe16..8fdc64af 100644 --- a/spring-cloud-config-monitor/pom.xml +++ b/spring-cloud-config-monitor/pom.xml @@ -5,7 +5,7 @@ org.springframework.cloud spring-cloud-config - 1.3.0.RELEASE + 1.3.0.BUILD-SNAPSHOT .. spring-cloud-config-monitor @@ -13,7 +13,7 @@ Spring Cloud Config Monitor ${basedir}/../.. - 1.3.0.RELEASE + 1.3.0.BUILD-SNAPSHOT diff --git a/spring-cloud-config-sample/pom.xml b/spring-cloud-config-sample/pom.xml index ca9887bc..113c6933 100644 --- a/spring-cloud-config-sample/pom.xml +++ b/spring-cloud-config-sample/pom.xml @@ -12,7 +12,7 @@ org.springframework.cloud spring-cloud-config - 1.3.0.RELEASE + 1.3.0.BUILD-SNAPSHOT .. diff --git a/spring-cloud-config-server/pom.xml b/spring-cloud-config-server/pom.xml index ac27b474..b1b5982e 100644 --- a/spring-cloud-config-server/pom.xml +++ b/spring-cloud-config-server/pom.xml @@ -12,7 +12,7 @@ org.springframework.cloud spring-cloud-config - 1.3.0.RELEASE + 1.3.0.BUILD-SNAPSHOT .. diff --git a/spring-cloud-starter-config/pom.xml b/spring-cloud-starter-config/pom.xml index c6af2404..1a379652 100644 --- a/spring-cloud-starter-config/pom.xml +++ b/spring-cloud-starter-config/pom.xml @@ -5,10 +5,10 @@ org.springframework.cloud spring-cloud-config - 1.3.0.RELEASE + 1.3.0.BUILD-SNAPSHOT spring-cloud-starter-config - 1.3.0.RELEASE + 1.3.0.BUILD-SNAPSHOT spring-cloud-starter-config Spring Cloud Starter https://projects.spring.io/spring-cloud From dfa609aa0d8dcd239c418b1696f73ccbf52fe7bf Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Fri, 7 Apr 2017 05:29:57 -0400 Subject: [PATCH 11/13] Bumping version for next release --- docs/pom.xml | 2 +- pom.xml | 6 +++--- spring-cloud-config-client/pom.xml | 2 +- spring-cloud-config-dependencies/pom.xml | 4 ++-- spring-cloud-config-monitor/pom.xml | 4 ++-- spring-cloud-config-sample/pom.xml | 2 +- spring-cloud-config-server/pom.xml | 2 +- spring-cloud-starter-config/pom.xml | 4 ++-- 8 files changed, 13 insertions(+), 13 deletions(-) diff --git a/docs/pom.xml b/docs/pom.xml index a03d8a2b..bc55c423 100644 --- a/docs/pom.xml +++ b/docs/pom.xml @@ -6,7 +6,7 @@ org.springframework.cloud spring-cloud-config - 1.3.0.BUILD-SNAPSHOT + 1.3.1.BUILD-SNAPSHOT pom Spring Cloud Config Docs diff --git a/pom.xml b/pom.xml index a89c4d16..4474d2d6 100644 --- a/pom.xml +++ b/pom.xml @@ -3,7 +3,7 @@ 4.0.0 org.springframework.cloud spring-cloud-config - 1.3.0.BUILD-SNAPSHOT + 1.3.1.BUILD-SNAPSHOT pom Spring Cloud Config Spring Cloud Config @@ -11,7 +11,7 @@ org.springframework.cloud spring-cloud-build - 1.3.1.BUILD-SNAPSHOT + 1.3.2.BUILD-SNAPSHOT @@ -22,7 +22,7 @@ config - 1.2.0.BUILD-SNAPSHOT + 1.2.1.BUILD-SNAPSHOT spring-cloud-config-dependencies diff --git a/spring-cloud-config-client/pom.xml b/spring-cloud-config-client/pom.xml index fdefa944..8ce3bbe6 100644 --- a/spring-cloud-config-client/pom.xml +++ b/spring-cloud-config-client/pom.xml @@ -10,7 +10,7 @@ org.springframework.cloud spring-cloud-config - 1.3.0.BUILD-SNAPSHOT + 1.3.1.BUILD-SNAPSHOT .. diff --git a/spring-cloud-config-dependencies/pom.xml b/spring-cloud-config-dependencies/pom.xml index 5677b4af..721842e7 100644 --- a/spring-cloud-config-dependencies/pom.xml +++ b/spring-cloud-config-dependencies/pom.xml @@ -5,11 +5,11 @@ spring-cloud-dependencies-parent org.springframework.cloud - 1.3.0.BUILD-SNAPSHOT + 1.3.1.BUILD-SNAPSHOT spring-cloud-config-dependencies - 1.3.0.BUILD-SNAPSHOT + 1.3.1.BUILD-SNAPSHOT pom spring-cloud-config-dependencies Spring Cloud Config Dependencies diff --git a/spring-cloud-config-monitor/pom.xml b/spring-cloud-config-monitor/pom.xml index 8fdc64af..e1c47b6e 100644 --- a/spring-cloud-config-monitor/pom.xml +++ b/spring-cloud-config-monitor/pom.xml @@ -5,7 +5,7 @@ org.springframework.cloud spring-cloud-config - 1.3.0.BUILD-SNAPSHOT + 1.3.1.BUILD-SNAPSHOT .. spring-cloud-config-monitor @@ -13,7 +13,7 @@ Spring Cloud Config Monitor ${basedir}/../.. - 1.3.0.BUILD-SNAPSHOT + 1.3.1.BUILD-SNAPSHOT diff --git a/spring-cloud-config-sample/pom.xml b/spring-cloud-config-sample/pom.xml index 113c6933..97173b2a 100644 --- a/spring-cloud-config-sample/pom.xml +++ b/spring-cloud-config-sample/pom.xml @@ -12,7 +12,7 @@ org.springframework.cloud spring-cloud-config - 1.3.0.BUILD-SNAPSHOT + 1.3.1.BUILD-SNAPSHOT .. diff --git a/spring-cloud-config-server/pom.xml b/spring-cloud-config-server/pom.xml index b1b5982e..b9680832 100644 --- a/spring-cloud-config-server/pom.xml +++ b/spring-cloud-config-server/pom.xml @@ -12,7 +12,7 @@ org.springframework.cloud spring-cloud-config - 1.3.0.BUILD-SNAPSHOT + 1.3.1.BUILD-SNAPSHOT .. diff --git a/spring-cloud-starter-config/pom.xml b/spring-cloud-starter-config/pom.xml index 1a379652..b9aa7a50 100644 --- a/spring-cloud-starter-config/pom.xml +++ b/spring-cloud-starter-config/pom.xml @@ -5,10 +5,10 @@ org.springframework.cloud spring-cloud-config - 1.3.0.BUILD-SNAPSHOT + 1.3.1.BUILD-SNAPSHOT spring-cloud-starter-config - 1.3.0.BUILD-SNAPSHOT + 1.3.1.BUILD-SNAPSHOT spring-cloud-starter-config Spring Cloud Starter https://projects.spring.io/spring-cloud From 010b10ec19834cec8672667c92298fca29b16f26 Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Fri, 7 Apr 2017 05:55:17 -0400 Subject: [PATCH 12/13] Updating mvnw --- mvnw | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/mvnw b/mvnw index 0a7dac22..a69491ac 100755 --- a/mvnw +++ b/mvnw @@ -238,8 +238,16 @@ else echo $MAVEN_ARGS | grep -q milestone && MAVEN_ARGS=$(echo $MAVEN_ARGS | sed -e 's/-Pmilestone//') fi +if echo $VERSION | egrep -q 'RELEASE'; then + echo Activating \"central\" profile for version=\"$VERSION\" + echo $MAVEN_ARGS | grep -q milestone || MAVEN_ARGS="$MAVEN_ARGS -Pcentral" +else + echo Deactivating \"central\" profile for version=\"$VERSION\" + echo $MAVEN_ARGS | grep -q central && MAVEN_ARGS=$(echo $MAVEN_ARGS | sed -e 's/-Pcentral//') +fi + exec "$JAVACMD" \ $MAVEN_OPTS \ -classpath "$MAVEN_PROJECTBASEDIR/.mvn/wrapper/maven-wrapper.jar" \ "-Dmaven.home=${M2_HOME}" "-Dmaven.multiModuleProjectDirectory=${MAVEN_PROJECTBASEDIR}" \ - ${WRAPPER_LAUNCHER} ${MAVEN_ARGS} "$@" + ${WRAPPER_LAUNCHER} ${MAVEN_ARGS} "$@" \ No newline at end of file From 295402481de60843a0e1016c9686d90ceb832df0 Mon Sep 17 00:00:00 2001 From: Johnny Lim Date: Fri, 14 Apr 2017 13:50:57 +0900 Subject: [PATCH 13/13] Remove unreachable assertNull() --- .../client/ConfigServicePropertySourceLocatorTests.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 d9bdd904..d34b7913 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 @@ -109,7 +109,7 @@ public class ConfigServicePropertySourceLocatorTests { this.expected.expectCause(IsInstanceOf . instanceOf(HttpServerErrorException.class)); this.expected.expectMessage("fail fast property is set"); - assertNull(this.locator.locate(this.environment)); + this.locator.locate(this.environment); } @Test @@ -137,7 +137,7 @@ public class ConfigServicePropertySourceLocatorTests { this.locator.setRestTemplate(restTemplate); this.expected.expectCause(IsNull.nullValue(Throwable.class)); this.expected.expectMessage("fail fast property is set"); - assertNull(this.locator.locate(this.environment)); + this.locator.locate(this.environment); } @Test @@ -156,7 +156,7 @@ public class ConfigServicePropertySourceLocatorTests { this.locator = new ConfigServicePropertySourceLocator(defaults); this.expected.expect(IllegalStateException.class); this.expected.expectMessage("You must set either 'password' or 'authorization'"); - assertNull(this.locator.locate(this.environment)); + this.locator.locate(this.environment); } @Test