Merge remote-tracking branch 'Upstream/master' into simplify-retry-logic

This commit is contained in:
Ryan Baxter
2016-10-04 10:49:02 -04:00
9 changed files with 148 additions and 54 deletions

View File

@@ -1305,6 +1305,9 @@ the service-specific prefix from individual routes, e.g.
stripPrefix: false
----
NOTE: `zuul.stripPrefix` only applies to the prefix set in `zuul.prefix`. It does have any effect on prefixes
defined within a given route's `path`.
In this example, requests to "/myusers/101" will be forwarded to "/myusers/101" on the "users" service.
The `zuul.routes` entries actually bind to an object of type `ZuulProperties`. If you
@@ -1410,6 +1413,8 @@ need to set it unless you want it to be different. N.B. this is new in
Spring Cloud Netflix 1.1 (in 1.0 the user had no control over headers
and all cookies flow in both directions).
=== Ignored Headers
In addition to the per-route sensitive headers, you can set a global
value for `zuul.ignoredHeaders` for values that should be discarded
(both request and response) during interactions with downstream
@@ -1418,6 +1423,7 @@ classpath, and otherwise they are initialized to a set of well-known
"security" headers (e.g. involving caching) as specified by Spring
Security. The assumption in this case is that the downstream services
might add these headers too, and we want the values from the proxy.
To not discard these well known security headers in case Spring Security is on the classpath you can set `zuul.ignoreSecurityHeaders` to `false`. This can be useful if you disabled the HTTP Security response headers in Spring Security and want the values provided by downstream services
=== The Routes Endpoint

View File

@@ -1,18 +1,19 @@
package org.springframework.cloud.netflix.ribbon;
import java.net.URI;
import org.springframework.web.util.UriComponentsBuilder;
import com.netflix.client.config.CommonClientConfigKey;
import com.netflix.client.config.IClientConfig;
import com.netflix.client.config.IClientConfigKey;
import com.netflix.config.ConfigurationManager;
import com.netflix.config.DynamicPropertyFactory;
import com.netflix.config.DynamicStringProperty;
import com.netflix.loadbalancer.Server;
import org.springframework.web.util.UriComponentsBuilder;
import java.net.URI;
/**
* @author Spencer Gibb
* @author Jacques-Etienne Beaudet
*/
public class RibbonUtils {
@@ -67,13 +68,19 @@ public class RibbonUtils {
* @param server
* @return
*/
public static URI updateToHttpsIfNeeded(URI uri, IClientConfig config, ServerIntrospector serverIntrospector, Server server) {
public static URI updateToHttpsIfNeeded(URI uri, IClientConfig config, ServerIntrospector serverIntrospector,
Server server) {
String scheme = uri.getScheme();
if (!"https".equals(scheme) && isSecure(config, serverIntrospector, server)) {
return UriComponentsBuilder.fromUri(uri).scheme("https").build(true)
.toUri();
UriComponentsBuilder uriComponentsBuilder = UriComponentsBuilder.fromUri(uri).scheme("https");
if (uri.getRawQuery() != null) {
// When building the URI, UriComponentsBuilder verify the allowed characters and does not
// support the '+' so we replace it for its equivalent '%20'.
// See issue https://jira.spring.io/browse/SPR-10172
uriComponentsBuilder.replaceQuery(uri.getRawQuery().replace("+", "%20"));
}
return uriComponentsBuilder.build(true).toUri();
}
return uri;
}
}

View File

@@ -23,13 +23,11 @@ import org.apache.http.client.HttpClient;
import org.apache.http.client.config.RequestConfig;
import org.apache.http.client.methods.HttpUriRequest;
import org.apache.http.impl.client.HttpClientBuilder;
import org.springframework.cloud.netflix.ribbon.DefaultServerIntrospector;
import org.springframework.cloud.netflix.ribbon.ServerIntrospector;
import org.springframework.cloud.netflix.ribbon.support.AbstractLoadBalancingClient;
import org.springframework.web.util.UriComponentsBuilder;
import com.netflix.client.config.CommonClientConfigKey;
import com.netflix.client.config.DefaultClientConfigImpl;
import com.netflix.client.config.IClientConfig;
import com.netflix.loadbalancer.ILoadBalancer;
import com.netflix.loadbalancer.Server;
@@ -42,40 +40,27 @@ import static org.springframework.cloud.netflix.ribbon.RibbonUtils.updateToHttps
//TODO: rename (ie new class that extends this in Dalston) to ApacheHttpLoadBalancingClient
public class RibbonLoadBalancingHttpClient
extends
AbstractLoadBalancingClient<RibbonApacheHttpRequest, RibbonApacheHttpResponse> {
private final HttpClient delegate;
private final IClientConfig config;
private final ServerIntrospector serverIntrospector;
AbstractLoadBalancingClient<RibbonApacheHttpRequest, RibbonApacheHttpResponse, HttpClient> {
@Deprecated
public RibbonLoadBalancingHttpClient() {
this(new DefaultClientConfigImpl(), new DefaultServerIntrospector());
super();
}
@Deprecated
public RibbonLoadBalancingHttpClient(final ILoadBalancer lb) {
super(lb);
this.config = new DefaultClientConfigImpl();
this.delegate = createHttpClient(this.config);
this.serverIntrospector = new DefaultServerIntrospector();
initWithNiwsConfig(config);
}
public RibbonLoadBalancingHttpClient(IClientConfig config, ServerIntrospector serverIntrospector) {
this.delegate = createHttpClient(config);
this.config = config;
this.serverIntrospector = serverIntrospector;
initWithNiwsConfig(config);
super(config, serverIntrospector);
}
public RibbonLoadBalancingHttpClient(HttpClient delegate, IClientConfig config, ServerIntrospector serverIntrospector) {
this.delegate = delegate;
this.config = config;
this.serverIntrospector = serverIntrospector;
initWithNiwsConfig(config);
super(delegate, config, serverIntrospector);
}
protected HttpClient createHttpClient(IClientConfig config) {
protected HttpClient createDelegate(IClientConfig config) {
return HttpClientBuilder.create()
// already defaults to 0 in builder, so resetting to 0 won't hurt
.setMaxConnTotal(config.getPropertyAsInteger(CommonClientConfigKey.MaxTotalConnections, 0))
@@ -86,10 +71,6 @@ public class RibbonLoadBalancingHttpClient
.build();
}
protected HttpClient getDelegate() {
return this.delegate;
}
@Override
public RibbonApacheHttpResponse execute(RibbonApacheHttpRequest request,
final IClientConfig configOverride) throws Exception {

View File

@@ -19,13 +19,11 @@ package org.springframework.cloud.netflix.ribbon.okhttp;
import java.net.URI;
import java.util.concurrent.TimeUnit;
import org.springframework.cloud.netflix.ribbon.DefaultServerIntrospector;
import org.springframework.cloud.netflix.ribbon.ServerIntrospector;
import org.springframework.cloud.netflix.ribbon.support.AbstractLoadBalancingClient;
import org.springframework.web.util.UriComponentsBuilder;
import com.netflix.client.config.CommonClientConfigKey;
import com.netflix.client.config.DefaultClientConfigImpl;
import com.netflix.client.config.IClientConfig;
import com.netflix.loadbalancer.ILoadBalancer;
import com.netflix.loadbalancer.Server;
@@ -40,30 +38,31 @@ import static org.springframework.cloud.netflix.ribbon.RibbonUtils.updateToHttps
* @author Spencer Gibb
*/
public class OkHttpLoadBalancingClient
extends AbstractLoadBalancingClient<OkHttpRibbonRequest, OkHttpRibbonResponse> {
private final OkHttpClient delegate = new OkHttpClient();
private final IClientConfig config;
private final ServerIntrospector serverIntrospector;
extends AbstractLoadBalancingClient<OkHttpRibbonRequest, OkHttpRibbonResponse, OkHttpClient> {
@Deprecated
public OkHttpLoadBalancingClient() {
super();
config = new DefaultClientConfigImpl();
serverIntrospector = new DefaultServerIntrospector();
}
@Deprecated
public OkHttpLoadBalancingClient(final ILoadBalancer lb) {
super(lb);
config = new DefaultClientConfigImpl();
serverIntrospector = new DefaultServerIntrospector();
}
public OkHttpLoadBalancingClient(IClientConfig config,
ServerIntrospector serverIntrospector) {
this.config = config;
this.serverIntrospector = serverIntrospector;
initWithNiwsConfig(config);
super(config, serverIntrospector);
}
public OkHttpLoadBalancingClient(OkHttpClient delegate, IClientConfig config,
ServerIntrospector serverIntrospector) {
super(delegate, config, serverIntrospector);
}
@Override
protected OkHttpClient createDelegate(IClientConfig config) {
return new OkHttpClient();
}
@Override

View File

@@ -17,6 +17,9 @@
package org.springframework.cloud.netflix.ribbon.support;
import org.springframework.cloud.netflix.ribbon.DefaultServerIntrospector;
import org.springframework.cloud.netflix.ribbon.ServerIntrospector;
import com.netflix.client.AbstractLoadBalancerAwareClient;
import com.netflix.client.IResponse;
import com.netflix.client.RequestSpecificRetryHandler;
@@ -29,7 +32,7 @@ import com.netflix.loadbalancer.ILoadBalancer;
/**
* @author Spencer Gibb
*/
public abstract class AbstractLoadBalancingClient<S extends ContextAwareRequest, T extends IResponse> extends
public abstract class AbstractLoadBalancingClient<S extends ContextAwareRequest, T extends IResponse, D> extends
AbstractLoadBalancerAwareClient<S, T> {
protected int connectTimeout;
@@ -42,16 +45,47 @@ public abstract class AbstractLoadBalancingClient<S extends ContextAwareRequest,
protected boolean okToRetryOnAllOperations;
protected final D delegate;
protected final IClientConfig config;
protected final ServerIntrospector serverIntrospector;
@Deprecated
public AbstractLoadBalancingClient() {
super(null);
this.config = new DefaultClientConfigImpl();
this.delegate = createDelegate(this.config);
this.serverIntrospector = new DefaultServerIntrospector();
this.setRetryHandler(RetryHandler.DEFAULT);
initWithNiwsConfig(config);
}
@Deprecated
public AbstractLoadBalancingClient(final ILoadBalancer lb) {
super(lb);
this.config = new DefaultClientConfigImpl();
this.delegate = createDelegate(config);
this.serverIntrospector = new DefaultServerIntrospector();
this.setRetryHandler(RetryHandler.DEFAULT);
initWithNiwsConfig(config);
}
protected AbstractLoadBalancingClient(IClientConfig config, ServerIntrospector serverIntrospector) {
super(null);
this.delegate = createDelegate(config);
this.config = config;
this.serverIntrospector = serverIntrospector;
this.setRetryHandler(RetryHandler.DEFAULT);
initWithNiwsConfig(config);
}
protected AbstractLoadBalancingClient(D delegate, IClientConfig config, ServerIntrospector serverIntrospector) {
super(null);
this.delegate = delegate;
this.config = config;
this.serverIntrospector = serverIntrospector;
this.setRetryHandler(RetryHandler.DEFAULT);
initWithNiwsConfig(config);
}
@Override
public void initWithNiwsConfig(IClientConfig clientConfig) {
@@ -72,6 +106,12 @@ public abstract class AbstractLoadBalancingClient<S extends ContextAwareRequest,
DefaultClientConfigImpl.DEFAULT_OK_TO_RETRY_ON_ALL_OPERATIONS);
}
protected abstract D createDelegate(IClientConfig config);
public D getDelegate() {
return this.delegate;
}
@Override
public RequestSpecificRetryHandler getRequestSpecificRetryHandler(
final S request, final IClientConfig requestConfig) {

View File

@@ -42,6 +42,7 @@ import static com.netflix.hystrix.HystrixCommandProperties.ExecutionIsolationStr
/**
* @author Spencer Gibb
* @author Dave Syer
* @author Mathias Düsterhöft
*/
@Data
@ConfigurationProperties("zuul")
@@ -50,7 +51,7 @@ public class ZuulProperties {
/**
* Headers that are generally expected to be added by Spring Security, and hence often
* duplicated if the proxy and the backend are secured with Spring. By default they
* are added to the ignored headers if Spring Security is present.
* are added to the ignored headers if Spring Security is present and ignoreSecurityHeaders = true.
*/
public static final List<String> SECURITY_HEADERS = Arrays.asList("Pragma",
"Cache-Control", "X-Frame-Options", "X-Content-Type-Options",
@@ -101,6 +102,14 @@ public class ZuulProperties {
*/
private Set<String> ignoredHeaders = new LinkedHashSet<>();
/**
* SECURITY_HEADERS are added to ignored headers if spring security is on the classpath and ignoreSecurityHeaders = true
* By setting ignoreSecurityHeaders to false we can switch off this default behaviour. This should be used together with
* disabling the default spring security headers
* see https://docs.spring.io/spring-security/site/docs/current/reference/html/headers.html#default-security-headers
*/
private boolean ignoreSecurityHeaders = true;
/**
* Path to install Zuul as a servlet (not part of Spring MVC). The servlet is more
* memory efficient for requests with large bodies, e.g. file uploads.
@@ -148,7 +157,7 @@ public class ZuulProperties {
Set<String> ignoredHeaders = new LinkedHashSet<>(this.ignoredHeaders);
if (ClassUtils.isPresent(
"org.springframework.security.config.annotation.web.WebSecurityConfigurer",
null) && Collections.disjoint(ignoredHeaders, SECURITY_HEADERS)) {
null) && Collections.disjoint(ignoredHeaders, SECURITY_HEADERS) && ignoreSecurityHeaders) {
// Allow Spring Security in the gateway to control these headers
ignoredHeaders.addAll(SECURITY_HEADERS);
}

View File

@@ -127,6 +127,17 @@ public class RibbonClientConfigurationTests {
}
}
@Test
public void testPlusInQueryStringGetsRewrittenWhenServerIsSecure() throws Exception {
Server server = new Server("foo", 7777);
when(this.inspector.isSecure(server)).thenReturn(true);
for (AbstractLoadBalancerAwareClient client : clients()) {
URI uri = client.reconstructURIWithServer(server, new URI("http://foo/%20bar?hello=1+2"));
assertThat(uri, is(new URI("https://foo:7777/%20bar?hello=1%202")));
}
}
private List<AbstractLoadBalancerAwareClient> clients() {
ArrayList<AbstractLoadBalancerAwareClient> clients = new ArrayList<>();
clients.add(new OverrideRestClient(this.config, this.inspector));

View File

@@ -17,20 +17,23 @@
package org.springframework.cloud.netflix.ribbon;
import com.netflix.client.config.CommonClientConfigKey;
import static org.hamcrest.Matchers.is;
import static org.springframework.cloud.netflix.ribbon.RibbonUtils.*;
import java.net.URI;
import java.net.URISyntaxException;
import java.util.Map;
import org.junit.Assert;
import org.junit.Test;
import com.netflix.client.config.CommonClientConfigKey;
import com.netflix.client.config.DefaultClientConfigImpl;
import com.netflix.loadbalancer.Server;
import java.util.Map;
import static org.hamcrest.Matchers.is;
import static org.springframework.cloud.netflix.ribbon.RibbonUtils.isSecure;
/**
* @author Spencer Gibb
* @author Jacques-Etienne Beaudet
*/
public class RibbonUtilsTests {
@@ -77,6 +80,35 @@ public class RibbonUtilsTests {
Assert.assertThat("isSecure was wrong", secure, is(false));
}
@Test
public void uriIsNotChangedWhenServerIsNotSecured() throws URISyntaxException {
URI original = new URI("http://foo");
URI updated = updateToHttpsIfNeeded(original, NON_SECURE_CONFIG, NON_SECURE_INTROSPECTOR, SERVER);
Assert.assertThat("URI should not have been updated since server is not secured.", original, is(updated));
}
@Test
public void uriIsNotChangedWhenServerIsSecuredAndUriAlreadyInHttps() throws URISyntaxException {
URI original = new URI("https://foo");
URI updated = updateToHttpsIfNeeded(original, SECURE_CONFIG, SECURE_INTROSPECTOR, SERVER);
Assert.assertThat("URI should not have been updated since uri is already in https.", original, is(updated));
}
@Test
public void shouldUpgradeUriToHttpsWhenServerIsSecureAndUriNotInHttps() throws URISyntaxException {
URI original = new URI("http://foo");
URI updated = updateToHttpsIfNeeded(original, SECURE_CONFIG, SECURE_INTROSPECTOR, SERVER);
Assert.assertThat("URI should have been updated to https.", updated, is(new URI("https://foo")));
}
@Test
public void shouldSubstitutePlusInQueryParam() throws URISyntaxException {
URI original = new URI("http://foo/%20bar?hello=1+2");
URI updated = updateToHttpsIfNeeded(original, SECURE_CONFIG, SECURE_INTROSPECTOR, SERVER);
Assert.assertThat("URI should have had its plus sign replaced in query string.", updated, is(new URI(
"https://foo/%20bar?hello=1%202")));
}
static DefaultClientConfigImpl getConfig(boolean value) {
DefaultClientConfigImpl config = new DefaultClientConfigImpl();
config.setProperty(CommonClientConfigKey.IsSecure, value);

View File

@@ -29,6 +29,7 @@ import static org.junit.Assert.assertTrue;
/**
* @author Dave Syer
* @author Mathias Düsterhöft
*/
public class ZuulPropertiesTests {
@@ -46,10 +47,18 @@ public class ZuulPropertiesTests {
@Test
public void defaultIgnoredHeaders() {
assertTrue(this.zuul.isIgnoreSecurityHeaders());
assertTrue(this.zuul.getIgnoredHeaders()
.containsAll(ZuulProperties.SECURITY_HEADERS));
}
@Test
public void securityHeadersNotIgnored() {
zuul.setIgnoreSecurityHeaders(false);
assertTrue(this.zuul.getIgnoredHeaders().isEmpty());
}
@Test
public void addIgnoredHeaders() {
this.zuul.setIgnoredHeaders(Collections.singleton("x-foo"));