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 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 diff --git a/pom.xml b/pom.xml index b2ab4e47..a46b87c1 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 7df33ff2..114a7423 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/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/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/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/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/ConfigServicePropertySourceLocatorTests.java b/spring-cloud-config-client/src/test/java/org/springframework/cloud/config/client/ConfigServicePropertySourceLocatorTests.java index ad727030..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 @@ -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; @@ -103,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 @@ -131,7 +137,85 @@ 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 + 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'"); + 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") 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(); } } diff --git a/spring-cloud-config-monitor/pom.xml b/spring-cloud-config-monitor/pom.xml index 6d5e88b8..35ac016b 100644 --- a/spring-cloud-config-monitor/pom.xml +++ b/spring-cloud-config-monitor/pom.xml @@ -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/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/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..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 { @@ -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/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/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/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 * 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; 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) 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");