Use only ConfigFileApplicationListener in server mini-application

In the server we use a SpringApplicationEnvironmentRepository to load
the YAML and properties files from git (or locally). It creates a mini
SpringApplication so as to faithfully replicate the way the Environment
is created. Unfortunately that can have side effects on the server
application itself (e.g. setting log levels). In particular if the
mini SpringApplication fails to start then the log levels could be
left in a "preInitialized" state with all log levels OFF by default.

This change ensures that the server logs all errors when loading YAML
and properties files, and also that the client logs the error response
if it is JSON (as it should be).

Fixes gh-66, fixes gh-67

spring-cloud-config-client/src/main/java/org/springframework/cloud/config/client/ConfigServicePropertySourceLocator.java

@@ -30,10 +30,12 @@ import org.springframework.core.env.MapPropertySource;
 import org.springframework.http.HttpEntity;
 import org.springframework.http.HttpMethod;
 import org.springframework.http.HttpRequest;
+import org.springframework.http.MediaType;
 import org.springframework.http.client.ClientHttpRequestExecution;
 import org.springframework.http.client.ClientHttpRequestInterceptor;
 import org.springframework.http.client.ClientHttpResponse;
 import org.springframework.security.crypto.codec.Base64;
+import org.springframework.web.client.HttpServerErrorException;
 import org.springframework.web.client.RestTemplate;
 
 /**
@@ -60,6 +62,8 @@ public class ConfigServicePropertySourceLocator implements PropertySourceLocator
 		CompositePropertySource composite = new CompositePropertySource("configService");
 		RestTemplate restTemplate = this.restTemplate == null ? getSecureRestTemplate(client)
 				: this.restTemplate;
+		RuntimeException error = null;
+		String errorBody = null;
 		try {
 			Environment result = restTemplate.exchange(
 					client.getUri() + "/{name}/{profile}/{label}", HttpMethod.GET,
@@ -72,15 +76,23 @@ public class ConfigServicePropertySourceLocator implements PropertySourceLocator
 			}
 			return composite;
 		}
-		catch (Exception e) {
-			if (client != null && client.isFailFast()) {
-				throw new IllegalStateException(
-						"Could not locate PropertySource. The fail fast property is set, failing",
-						e);
+		catch (HttpServerErrorException e) {
+			error = e;
+			if (MediaType.APPLICATION_JSON.includes(e.getResponseHeaders().getContentType())) {
+				errorBody = e.getResponseBodyAsString();
 			}
-			logger.error("Could not locate PropertySource: " + e.getMessage());
-			return null;
 		}
+		catch (Exception e) {
+			error = new IllegalStateException(
+					"Could not locate PropertySource. The fail fast property is set, failing",
+					e);
+		}
+		if (client != null && client.isFailFast()) {
+			throw error;
+		}
+		logger.error("Could not locate PropertySource: "
+				+ (errorBody == null ? error.getMessage() : errorBody));
+		return null;
 
 	}
 

spring-cloud-config-client/src/test/java/org/springframework/cloud/bootstrap/config/BootstrapConfigurationTests.java

@@ -28,7 +28,9 @@ import java.util.HashMap;
 import java.util.Map;
 
 import org.junit.After;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 import org.springframework.boot.builder.SpringApplicationBuilder;
 import org.springframework.boot.context.properties.ConfigurationProperties;
 import org.springframework.boot.context.properties.EnableConfigurationProperties;
@@ -50,11 +52,15 @@ import org.springframework.core.env.StandardEnvironment;
 public class BootstrapConfigurationTests {
 
 	private ConfigurableApplicationContext context;
+	
+	@Rule
+	public ExpectedException expected = ExpectedException.none();
 
 	@After
 	public void close() {
 		// Expected.* is bound to the PropertySourceConfiguration below
 		System.clearProperty("expected.name");
+		System.clearProperty("expected.fail");
 		// Used to test system properties override
 		System.clearProperty("bootstrap.foo");
 		PropertySourceConfiguration.MAP.clear();
@@ -109,6 +115,14 @@ public class BootstrapConfigurationTests {
 		assertNotNull(context.getBean(ConfigClientProperties.class));
 	}
 
+	@Test
+	public void failsOnPropertySource() {
+		System.setProperty("expected.fail", "true");
+		expected.expectMessage("Planned");
+		context = new SpringApplicationBuilder().web(false)
+				.sources(BareConfiguration.class).run();
+	}
+
 	@Test
 	public void overrideSystemPropertySourceByDefault() {
 		PropertySourceConfiguration.MAP.put("bootstrap.foo", "bar");
@@ -272,12 +286,17 @@ public class BootstrapConfigurationTests {
 				Collections.<String, Object> singletonMap("bootstrap.foo", "bar"));
 
 		private String name;
+		
+		private boolean fail = false;
 
 		@Override
 		public PropertySource<?> locate(Environment environment) {
 			if (name != null) {
 				assertEquals(name, environment.getProperty("spring.application.name"));
 			}
+			if (fail) {
+				throw new RuntimeException("Planned");
+			}
 			return new MapPropertySource("testBootstrap", MAP);
 		}
 
@@ -288,6 +307,14 @@ public class BootstrapConfigurationTests {
 		public void setName(String name) {
 			this.name = name;
 		}
+
+		public boolean isFail() {
+			return fail;
+		}
+
+		public void setFail(boolean fail) {
+			this.fail = fail;
+		}
 	}
 
 }

spring-cloud-config-client/src/test/java/org/springframework/cloud/config/client/ConfigServicePropertySourceLocatorTests.java

@@ -0,0 +1,92 @@
+package org.springframework.cloud.config.client;
+
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+
+import java.io.ByteArrayInputStream;
+import java.net.URI;
+
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+import org.mockito.Matchers;
+import org.mockito.Mockito;
+import org.springframework.cloud.config.Environment;
+import org.springframework.core.env.ConfigurableEnvironment;
+import org.springframework.core.env.StandardEnvironment;
+import org.springframework.http.HttpEntity;
+import org.springframework.http.HttpHeaders;
+import org.springframework.http.HttpMethod;
+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.ClientHttpRequestFactory;
+import org.springframework.http.client.ClientHttpResponse;
+import org.springframework.web.client.HttpServerErrorException;
+import org.springframework.web.client.RestTemplate;
+
+public class ConfigServicePropertySourceLocatorTests {
+
+	@Rule
+	public ExpectedException expected = ExpectedException.none();
+
+	private ConfigurableEnvironment environment = new StandardEnvironment();
+
+	private ConfigServicePropertySourceLocator locator = new ConfigServicePropertySourceLocator(
+			new ConfigClientProperties(environment));
+
+	private RestTemplate restTemplate = Mockito.mock(RestTemplate.class);
+
+	@Test
+	public void sunnyDay() {
+		Environment body = new Environment("app", "master");
+		mockRequestResponse(new ResponseEntity<Environment>(body, HttpStatus.OK));
+		locator.setRestTemplate(restTemplate);
+		assertNotNull(locator.locate(environment));
+	}
+
+	@Test
+	public void failsQuietly() {
+		mockRequestResponse(new ResponseEntity<String>("Wah!",
+				HttpStatus.INTERNAL_SERVER_ERROR));
+		locator.setRestTemplate(restTemplate);
+		assertNull(locator.locate(environment));
+	}
+
+	@Test
+	public void failFast() throws Exception {
+		ClientHttpRequestFactory requestFactory = Mockito
+				.mock(ClientHttpRequestFactory.class);
+		ClientHttpRequest request = Mockito.mock(ClientHttpRequest.class);
+		ClientHttpResponse response = Mockito.mock(ClientHttpResponse.class);
+		Mockito.when(
+				requestFactory.createRequest(Mockito.any(URI.class),
+						Mockito.any(HttpMethod.class))).thenReturn(request);
+		RestTemplate restTemplate = new RestTemplate(requestFactory);
+		ConfigClientProperties defaults = new ConfigClientProperties(environment);
+		defaults.setFailFast(true);
+		locator = new ConfigServicePropertySourceLocator(defaults);
+		Mockito.when(request.getHeaders()).thenReturn(new HttpHeaders());
+		Mockito.when(request.execute()).thenReturn(response);
+		HttpHeaders headers = new HttpHeaders();
+		headers.setContentType(MediaType.APPLICATION_JSON);
+		Mockito.when(response.getHeaders()).thenReturn(headers);
+		Mockito.when(response.getStatusCode()).thenReturn(HttpStatus.INTERNAL_SERVER_ERROR);
+		Mockito.when(response.getBody()).thenReturn(new ByteArrayInputStream("{}".getBytes()));
+		locator.setRestTemplate(restTemplate);
+		expected.expect(HttpServerErrorException.class);
+		expected.expectMessage("500");
+		assertNull(locator.locate(environment));
+	}
+
+	@SuppressWarnings("unchecked")
+	private void mockRequestResponse(ResponseEntity<?> response) {
+		Mockito.when(
+				restTemplate.exchange(Mockito.any(String.class),
+						Mockito.any(HttpMethod.class), Mockito.any(HttpEntity.class),
+						Mockito.any(Class.class), Matchers.anyString(),
+						Matchers.anyString(), Matchers.anyString())).thenReturn(response);
+	}
+
+}

spring-cloud-config-sample/src/test/java/sample/ApplicationTests.java

@@ -30,21 +30,26 @@ public class ApplicationTests {
 	@Value("${local.server.port}")
 	private int port;
 
+	private static ConfigurableApplicationContext server;
+
 	@BeforeClass
 	public static void startConfigServer() throws IOException {
 		String repo = ConfigServerTestUtils.prepareLocalRepo();
-		ConfigurableApplicationContext context = SpringApplication.run(
+		server = SpringApplication.run(
 				org.springframework.cloud.config.server.ConfigServerApplication.class,
 				"--server.port=" + configPort, "--spring.config.name=server",
 				"--spring.cloud.config.server.git.uri=" + repo);
-		configPort = ((EmbeddedWebApplicationContext) context)
+		configPort = ((EmbeddedWebApplicationContext) server)
 				.getEmbeddedServletContainer().getPort();
 		System.setProperty("config.port", "" + configPort);
 	}
 	
 	@AfterClass
 	public static void close() {
-		System.clearProperty("config.port");		
+		System.clearProperty("config.port");
+		if (server!=null) {
+			server.close();
+		}
 	}
 
 	@Test

spring-cloud-config-sample/src/test/java/sample/ServerNativeApplicationTests.java

@@ -0,0 +1,73 @@
+package sample;
+
+import static org.junit.Assert.assertTrue;
+
+import java.io.IOException;
+import java.util.Map;
+
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.beans.factory.annotation.Value;
+import org.springframework.boot.SpringApplication;
+import org.springframework.boot.context.embedded.EmbeddedWebApplicationContext;
+import org.springframework.boot.test.IntegrationTest;
+import org.springframework.boot.test.SpringApplicationConfiguration;
+import org.springframework.cloud.config.server.ConfigServerTestUtils;
+import org.springframework.context.ConfigurableApplicationContext;
+import org.springframework.core.env.ConfigurableEnvironment;
+import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
+import org.springframework.test.context.web.WebAppConfiguration;
+
+@RunWith(SpringJUnit4ClassRunner.class)
+@SpringApplicationConfiguration(classes = Application.class)
+@IntegrationTest({"server.port:0", "spring.application.name:bad"})
+@WebAppConfiguration
+public class ServerNativeApplicationTests {
+
+	private static int configPort = 0;
+	
+	@Autowired
+	private ConfigurableEnvironment environment;
+
+	@Value("${local.server.port}")
+	private int port;
+
+	private static ConfigurableApplicationContext server;
+
+	@BeforeClass
+	public static void startConfigServer() throws IOException {
+		String repo = ConfigServerTestUtils.prepareLocalRepo();
+		server = SpringApplication.run(
+				org.springframework.cloud.config.server.ConfigServerApplication.class,
+				"--server.port=" + configPort, "--spring.config.name=server",
+				"--spring.cloud.config.server.git.uri=" + repo, "--spring.profiles.active=native");
+		configPort = ((EmbeddedWebApplicationContext) server)
+				.getEmbeddedServletContainer().getPort();
+		System.setProperty("config.port", "" + configPort);
+	}
+	
+	@AfterClass
+	public static void close() {
+		System.clearProperty("config.port");		
+		if (server!=null) {
+			server.close();
+		}
+	}
+
+	@SuppressWarnings("rawtypes")
+	@Test
+	public void contextLoads() {
+		// The remote config was bad so there is no bootstrap
+		assertTrue(((Map)environment.getPropertySources().get("bootstrap").getSource()).isEmpty());
+	}
+
+	public static void main(String[] args) throws IOException {
+		configPort = 8888;
+		startConfigServer();
+		SpringApplication.run(Application.class, args);
+	}
+
+}

spring-cloud-config-sample/src/test/resources/bad.yml

@@ -0,0 +1,4 @@
+foo:
+# whitespace error!
+	bar: spam
+    bucket: wham

spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/EnvironmentController.java

@@ -29,7 +29,7 @@ import org.yaml.snakeyaml.Yaml;
 @RestController
 @RequestMapping("${spring.cloud.config.server.prefix:}")
 public class EnvironmentController {
-
+	
 	private EnvironmentRepository repository;
 
 	private EncryptionController encryption;
@@ -61,7 +61,7 @@ public class EnvironmentController {
 		}
 		return environment;
 	}
-
+	
 	@RequestMapping("/{name}-{profiles}.properties")
 	public ResponseEntity<String> properties(@PathVariable String name,
 			@PathVariable String profiles) throws IOException {

spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/SpringApplicationEnvironmentRepository.java

@@ -22,6 +22,7 @@ import java.util.List;
 
 import org.springframework.boot.autoconfigure.PropertyPlaceholderAutoConfiguration;
 import org.springframework.boot.builder.SpringApplicationBuilder;
+import org.springframework.boot.context.config.ConfigFileApplicationListener;
 import org.springframework.boot.context.properties.ConfigurationProperties;
 import org.springframework.cloud.config.Environment;
 import org.springframework.context.ConfigurableApplicationContext;
@@ -66,6 +67,10 @@ public class SpringApplicationEnvironmentRepository implements EnvironmentReposi
 		builder.environment(environment);
 		builder.web(false).showBanner(false);
 		String[] args = getArgs(config, label);
+		// Explicitly set the listeners (to exclude logging listener which would change log
+		// levels in the caller)
+		builder.application().setListeners(
+				Collections.singletonList(new ConfigFileApplicationListener()));
 		ConfigurableApplicationContext context = builder.run(args);
 		environment.getPropertySources().remove("profiles");
 		try {
@@ -121,7 +126,7 @@ public class SpringApplicationEnvironmentRepository implements EnvironmentReposi
 		this.locations = locations;
 		for (int i = 0; i < locations.length; i++) {
 			String location = locations[i];
-			if (isDirectory(location)&& !location.endsWith("/")) {
+			if (isDirectory(location) && !location.endsWith("/")) {
 				location = location + "/";
 			}
 			locations[i] = location;

spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/NativeConfigServerIntegrationTests.java

@@ -0,0 +1,58 @@
+package org.springframework.cloud.config.server;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+
+import java.io.IOException;
+
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.springframework.beans.factory.annotation.Value;
+import org.springframework.boot.builder.SpringApplicationBuilder;
+import org.springframework.boot.test.IntegrationTest;
+import org.springframework.boot.test.SpringApplicationConfiguration;
+import org.springframework.boot.test.TestRestTemplate;
+import org.springframework.cloud.config.Environment;
+import org.springframework.http.HttpStatus;
+import org.springframework.http.ResponseEntity;
+import org.springframework.test.context.ActiveProfiles;
+import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
+import org.springframework.test.context.web.WebAppConfiguration;
+
+@RunWith(SpringJUnit4ClassRunner.class)
+@SpringApplicationConfiguration(classes = ConfigServerApplication.class)
+@IntegrationTest({ "server.port:0", "spring.config.name:configserver" })
+@WebAppConfiguration
+@ActiveProfiles({ "test", "native" })
+public class NativeConfigServerIntegrationTests {
+	
+	@Value("${local.server.port}")
+	private int port;
+	
+	@BeforeClass
+	public static void init() throws IOException{
+		ConfigServerTestUtils.prepareLocalRepo();
+	}
+
+	@Test
+	public void contextLoads() {
+		Environment environment = new TestRestTemplate().getForObject("http://localhost:" + port + "/foo/development/", Environment.class);
+		assertFalse(environment.getPropertySources().isEmpty());
+		assertEquals("overrides", environment.getPropertySources().get(0).getName());
+		assertEquals("{spring.cloud.config.enabled=true}", environment.getPropertySources().get(0).getSource().toString());
+	}
+
+	@Test
+	public void badYaml() {
+		ResponseEntity<String> response = new TestRestTemplate().getForEntity("http://localhost:"
+				+ port + "/bad/default/", String.class);
+		assertEquals(HttpStatus.INTERNAL_SERVER_ERROR, response.getStatusCode());
+	}
+
+	public static void main(String[] args) {
+		new SpringApplicationBuilder(ConfigServerApplication.class).profiles("native").properties(
+				"spring.config.name=configserver").run(args);
+	}
+
+}

spring-cloud-config-server/src/test/java/org/springframework/cloud/config/server/VanillaConfigServerIntegrationTests.java

@@ -23,21 +23,23 @@ import org.springframework.test.context.web.WebAppConfiguration;
 @WebAppConfiguration
 @ActiveProfiles("test")
 public class VanillaConfigServerIntegrationTests {
-	
+
 	@Value("${local.server.port}")
 	private int port;
-	
+
 	@BeforeClass
-	public static void init() throws IOException{
+	public static void init() throws IOException {
 		ConfigServerTestUtils.prepareLocalRepo();
 	}
 
 	@Test
 	public void contextLoads() {
-		Environment environment = new TestRestTemplate().getForObject("http://localhost:" + port + "/foo/development/", Environment.class);
+		Environment environment = new TestRestTemplate().getForObject("http://localhost:"
+				+ port + "/foo/development/", Environment.class);
 		assertFalse(environment.getPropertySources().isEmpty());
 		assertEquals("overrides", environment.getPropertySources().get(0).getName());
-		assertEquals("{spring.cloud.config.enabled=true}", environment.getPropertySources().get(0).getSource().toString());
+		assertEquals("{spring.cloud.config.enabled=true}", environment
+				.getPropertySources().get(0).getSource().toString());
 	}
 
 }

spring-cloud-config-server/src/test/resources/bad.yml

@@ -0,0 +1,4 @@
+foo:
+# whitespace error!
+	bar: spam
+    bucket: wham