Made RequestReadTimeout and whether to sendState configurable (#1034)

Made RequestReadTimeout and whether to send State configurable.
This commit is contained in:
Indraneel Bende
2018-06-13 16:55:39 -04:00
committed by Spencer Gibb
parent d136aea540
commit 9ab7acaf6f
4 changed files with 87 additions and 23 deletions

View File

@@ -1203,6 +1203,10 @@ To ensure high availability when you have multiple instances of Config Server de
If you use HTTP basic security on your Config Server, it is currently possible to support per-Config Server auth credentials only if you embed the credentials in each URL you specify under the `spring.cloud.config.uri` property. If you use any other kind of security mechanism, you cannot (currently) support per-Config Server authentication and authorization.
=== Configuring Read Timeouts
If you want to configure read timeout, this can be done by using the property `spring.cloud.config.request-read-timeout`.
=== Security
If you use HTTP Basic security on the server, clients need to know the password (and username if it is not the default).

View File

@@ -18,6 +18,7 @@ package org.springframework.cloud.config.client;
import java.net.MalformedURLException;
import java.net.URL;
import java.util.Arrays;
import java.util.HashMap;
import java.util.Map;
@@ -39,6 +40,7 @@ public class ConfigClientProperties {
public static final String PREFIX = "spring.cloud.config";
public static final String TOKEN_HEADER = "X-Config-Token";
public static final String STATE_HEADER = "X-Config-State";
public static final String AUTHORIZATION = "authorization";
/**
* Flag to say that remote configuration is enabled. Default true;
@@ -94,9 +96,14 @@ public class ConfigClientProperties {
private String token;
/**
* Authorization token used by the client to connect to the server.
* timeout on waiting to read data from the Config Server.
*/
private String authorization;
private int requestReadTimeout = (60 * 1000 * 3) + 5000;
/**
* Flag to indicate whether to send state. Default true.
*/
private boolean sendState = true;
/**
* Additional headers used to create the client request.
@@ -198,14 +205,20 @@ public class ConfigClientProperties {
this.token = token;
}
@DeprecatedConfigurationProperty(reason = "replaced by headers", replacement = "headers")
@Deprecated
public String getAuthorization() {
return this.authorization;
public int getRequestReadTimeout() {
return requestReadTimeout;
}
public void setAuthorization(String authorization) {
this.authorization = authorization;
public void setRequestReadTimeout(int requestReadTimeout) {
this.requestReadTimeout = requestReadTimeout;
}
public boolean isSendState() {
return sendState;
}
public void setSendState(boolean sendState) {
this.sendState = sendState;
}
public Map<String, String> getHeaders() {
@@ -351,12 +364,14 @@ public class ConfigClientProperties {
@Override
public String toString() {
return "ConfigClientProperties [enabled=" + this.enabled + ", profile="
+ this.profile + ", name=" + this.name + ", label="
+ (this.label == null ? "" : this.label) + ", username=" + this.username
+ ", password=" + this.password + ", uri=" + this.uri + ", authorization="
+ this.authorization + ", discovery.enabled=" + this.discovery.enabled
+ ", failFast=" + this.failFast + ", token=" + this.token + "]";
return "ConfigClientProperties [enabled=" + enabled + ", profile=" + profile
+ ", name=" + name + ", label=" + label + ", username=" + username
+ ", password=" + password + ", uri=" + Arrays.toString(uri)
+ ", discovery=" + discovery + ", failFast=" + failFast + ", token="
+ token + ", requestReadTimeout=" + requestReadTimeout + ", sendState="
+ sendState + ", headers=" + headers + "]";
}
}

View File

@@ -53,6 +53,7 @@ import org.springframework.web.client.RestTemplate;
import static org.springframework.cloud.config.client.ConfigClientProperties.STATE_HEADER;
import static org.springframework.cloud.config.client.ConfigClientProperties.TOKEN_HEADER;
import static org.springframework.cloud.config.client.ConfigClientProperties.AUTHORIZATION;
/**
* @author Dave Syer
@@ -209,7 +210,7 @@ public class ConfigServicePropertySourceLocator implements PropertySourceLocator
if (StringUtils.hasText(token)) {
headers.add(TOKEN_HEADER, token);
}
if (StringUtils.hasText(state)) { // TODO: opt in to sending state?
if (StringUtils.hasText(state) && properties.isSendState()) {
headers.add(STATE_HEADER, state);
}
@@ -248,11 +249,15 @@ public class ConfigServicePropertySourceLocator implements PropertySourceLocator
private RestTemplate getSecureRestTemplate(ConfigClientProperties client) {
SimpleClientHttpRequestFactory requestFactory = new SimpleClientHttpRequestFactory();
requestFactory.setReadTimeout((60 * 1000 * 3) + 5000); // TODO 3m5s, make
// configurable?
if (client.getRequestReadTimeout() < 0) {
throw new IllegalStateException("Invalid Value for Read Timeout set.");
}
requestFactory.setReadTimeout(client.getRequestReadTimeout());
RestTemplate template = new RestTemplate(requestFactory);
Map<String, String> headers = new HashMap<>(client.getHeaders());
if (headers.containsKey(AUTHORIZATION)) {
headers.remove(AUTHORIZATION); // To avoid redundant addition of header
}
if (!headers.isEmpty()) {
template.setInterceptors(Arrays.<ClientHttpRequestInterceptor> asList(
new GenericRequestHeaderInterceptor(headers)));
@@ -263,7 +268,7 @@ public class ConfigServicePropertySourceLocator implements PropertySourceLocator
private void addAuthorizationToken(ConfigClientProperties configClientProperties,
HttpHeaders httpHeaders, String username, String password) {
String authorization = configClientProperties.getAuthorization();
String authorization = configClientProperties.getHeaders().get(AUTHORIZATION);
if (password != null && authorization != null) {
throw new IllegalStateException(
@@ -297,5 +302,10 @@ public class ConfigServicePropertySourceLocator implements PropertySourceLocator
}
return execution.execute(request, body);
}
protected Map<String, String> getHeaders() {
return headers;
}
}
}

View File

@@ -1,12 +1,14 @@
package org.springframework.cloud.config.client;
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.Assert.assertEquals;
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.Iterator;
import java.util.Map;
import org.hamcrest.core.IsInstanceOf;
@@ -17,6 +19,7 @@ import org.junit.rules.ExpectedException;
import org.mockito.Matchers;
import org.mockito.Mockito;
import org.springframework.boot.test.util.EnvironmentTestUtils;
import org.springframework.cloud.config.client.ConfigServicePropertySourceLocator.GenericRequestHeaderInterceptor;
import org.springframework.cloud.config.environment.Environment;
import org.springframework.core.env.ConfigurableEnvironment;
import org.springframework.core.env.StandardEnvironment;
@@ -29,12 +32,16 @@ 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.ClientHttpRequestInterceptor;
import org.springframework.http.client.ClientHttpResponse;
import org.springframework.http.client.SimpleClientHttpRequestFactory;
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;
import static org.springframework.cloud.config.client.ConfigClientProperties.AUTHORIZATION;
public class ConfigServicePropertySourceLocatorTests {
@Rule
@@ -156,11 +163,10 @@ public class ConfigServicePropertySourceLocatorTests {
Mockito.when(requestFactory.createRequest(Mockito.any(URI.class),
Mockito.any(HttpMethod.class))).thenReturn(request);
ConfigClientProperties defaults = new ConfigClientProperties(this.environment);
// defaults.setUri(new String[] {"http://localhost");
defaults.setFailFast(true);
defaults.setUsername("username");
defaults.setPassword("password");
defaults.setAuthorization("Basic dXNlcm5hbWU6cGFzc3dvcmQNCg==");
defaults.getHeaders().put(AUTHORIZATION, "Basic dXNlcm5hbWU6cGFzc3dvcmQNCg==");
this.locator = new ConfigServicePropertySourceLocator(defaults);
this.expected.expect(IllegalStateException.class);
this.expected.expectMessage(
@@ -198,7 +204,7 @@ public class ConfigServicePropertySourceLocatorTests {
public void shouldAddAuthorizationHeaderWhenAuthorizationSet() {
HttpHeaders headers = new HttpHeaders();
ConfigClientProperties defaults = new ConfigClientProperties(this.environment);
defaults.setAuthorization("1234abcd");
defaults.getHeaders().put(AUTHORIZATION, "Basic dXNlcm5hbWU6cGFzc3dvcmQNCg==");
this.locator = new ConfigServicePropertySourceLocator(defaults);
String username = "user";
String password = null;
@@ -211,7 +217,7 @@ public class ConfigServicePropertySourceLocatorTests {
public void shouldThrowExceptionWhenPasswordAndAuthorizationBothSet() {
HttpHeaders headers = new HttpHeaders();
ConfigClientProperties defaults = new ConfigClientProperties(this.environment);
defaults.setAuthorization("1234abcd");
defaults.getHeaders().put(AUTHORIZATION, "Basic dXNlcm5hbWU6cGFzc3dvcmQNCg==");
this.locator = new ConfigServicePropertySourceLocator(defaults);
String username = "user";
String password = "pass";
@@ -221,6 +227,34 @@ public class ConfigServicePropertySourceLocatorTests {
headers, username, password);
}
@Test
public void shouldThrowExceptionWhenNegativeReadTimeoutSet() {
ConfigClientProperties defaults = new ConfigClientProperties(this.environment);
defaults.setRequestReadTimeout(-1);
this.locator = new ConfigServicePropertySourceLocator(defaults);
this.expected.expect(IllegalStateException.class);
this.expected.expectMessage("Invalid Value for Read Timeout set.");
ReflectionTestUtils.invokeMethod(this.locator, "getSecureRestTemplate", defaults);
}
@Test
public void checkInterceptorHasNoAuthorizationHeaderPresent() {
ConfigClientProperties defaults = new ConfigClientProperties(this.environment);
defaults.getHeaders().put(AUTHORIZATION, "Basic dXNlcm5hbWU6cGFzc3dvcmQNCg==");
defaults.getHeaders().put("key", "value");
this.locator = new ConfigServicePropertySourceLocator(defaults);
RestTemplate restTemplate = ReflectionTestUtils.invokeMethod(this.locator,
"getSecureRestTemplate", defaults);
Iterator<ClientHttpRequestInterceptor> iterator = restTemplate.getInterceptors()
.iterator();
while (iterator.hasNext()) {
GenericRequestHeaderInterceptor genericRequestHeaderInterceptor = (GenericRequestHeaderInterceptor) iterator
.next();
assertEquals(null,
genericRequestHeaderInterceptor.getHeaders().get(AUTHORIZATION));
}
}
@SuppressWarnings("unchecked")
private void mockRequestResponseWithLabel(ResponseEntity<?> response, String label) {
Mockito.when(this.restTemplate.exchange(Mockito.any(String.class),
@@ -245,4 +279,5 @@ public class ConfigServicePropertySourceLocatorTests {
Mockito.any(Class.class), Matchers.eq(expectedName),
Matchers.anyString())).thenReturn(response);
}
}