From cc24d00fdeadec2bcd3348a4b42576081cbccf82 Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Wed, 19 Oct 2016 20:06:52 -0400 Subject: [PATCH 01/14] Fixes #1211 --- .../main/asciidoc/spring-cloud-netflix.adoc | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/docs/src/main/asciidoc/spring-cloud-netflix.adoc b/docs/src/main/asciidoc/spring-cloud-netflix.adoc index bd65af99..14663a1c 100644 --- a/docs/src/main/asciidoc/spring-cloud-netflix.adoc +++ b/docs/src/main/asciidoc/spring-cloud-netflix.adoc @@ -970,6 +970,46 @@ This replaces the `SpringMvcContract` with `feign.Contract.Default` and adds a ` Default configurations can be specified in the `@EnableFeignClients` attribute `defaultConfiguration` in a similar manner as described above. The difference is that this configuration will apply to _all_ feign clients. +=== Creating Feign Clients Manually + +In some cases it might be necessary to customize your Feign Clients in a way that is not +possible using the methods above. In this case you can create Clients using the +https://github.com/OpenFeign/feign/#basics[Feign Builder API]. Below is an example +which creates two Feign Clients with the same interface but configures each one with +a separate request interceptor. + +[source,java,indent=0] +---- +@Import(FeignClientsConfiguration.class) +class FooController { + + private FooClient fooClient; + + private FooClient adminClient; + + @Autowired + public FooController( + ResponseEntityDecoder decoder, SpringEncoder encoder, EurekaClient discoveryClient) { + InstanceInfo prodSvcInfo = discoveryClient.getNextServerFromEureka("PROD-SVC", false); + this.fooClient = Feign.builder() + .encoder(encoder) + .decoder(decoder) + .requestInterceptor(new BasicAuthRequestInterceptor("user", "user")) + .target(FooClient.class, prodSvcInfo.getHomePageUrl()); + this.adminClient = Feign.builder() + .encoder(encoder) + .decoder(decoder) + .requestInterceptor(new BasicAuthRequestInterceptor("admin", "admin")) + .target(FooClient.class, prodSvcInfo.getHomePageUrl()); + } +} +---- + +NOTE: In the above example `FeignClientsConfiguration.class` is the default configuration +provided by Spring Cloud Netflix. + +NOTE: `prodSvcInfo` is the service the the Clients will be making requests to. + [[spring-cloud-feign-hystrix]] === Feign Hystrix Support From 2c8bac56661f4d976519fb8f9032b4f2a5cbf5b4 Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Thu, 20 Oct 2016 13:31:41 -0400 Subject: [PATCH 02/14] Use client interface instead of Eureka directly --- .../main/asciidoc/spring-cloud-netflix.adoc | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/docs/src/main/asciidoc/spring-cloud-netflix.adoc b/docs/src/main/asciidoc/spring-cloud-netflix.adoc index 14663a1c..f99a3f93 100644 --- a/docs/src/main/asciidoc/spring-cloud-netflix.adoc +++ b/docs/src/main/asciidoc/spring-cloud-netflix.adoc @@ -989,18 +989,17 @@ class FooController { @Autowired public FooController( - ResponseEntityDecoder decoder, SpringEncoder encoder, EurekaClient discoveryClient) { - InstanceInfo prodSvcInfo = discoveryClient.getNextServerFromEureka("PROD-SVC", false); - this.fooClient = Feign.builder() + ResponseEntityDecoder decoder, SpringEncoder encoder, Client client) { + this.fooClient = Feign.builder().client(client) .encoder(encoder) - .decoder(decoder) - .requestInterceptor(new BasicAuthRequestInterceptor("user", "user")) - .target(FooClient.class, prodSvcInfo.getHomePageUrl()); - this.adminClient = Feign.builder() + .decoder(decoder) + .requestInterceptor(new BasicAuthRequestInterceptor("user", "user")) + .target(FooClient.class, "http://PROD-SVC"); + this.adminClient = Feign.builder().client(client) .encoder(encoder) - .decoder(decoder) - .requestInterceptor(new BasicAuthRequestInterceptor("admin", "admin")) - .target(FooClient.class, prodSvcInfo.getHomePageUrl()); + .decoder(decoder) + .requestInterceptor(new BasicAuthRequestInterceptor("admin", "admin")) + .target(FooClient.class, "http://PROD-SVC"); } } ---- From b16a95d0f2518bae5a608b0cbd05b56655f26aa9 Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Thu, 20 Oct 2016 13:40:59 -0400 Subject: [PATCH 03/14] Fixed typo --- docs/src/main/asciidoc/spring-cloud-netflix.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/src/main/asciidoc/spring-cloud-netflix.adoc b/docs/src/main/asciidoc/spring-cloud-netflix.adoc index f99a3f93..65052fe6 100644 --- a/docs/src/main/asciidoc/spring-cloud-netflix.adoc +++ b/docs/src/main/asciidoc/spring-cloud-netflix.adoc @@ -1007,7 +1007,7 @@ class FooController { NOTE: In the above example `FeignClientsConfiguration.class` is the default configuration provided by Spring Cloud Netflix. -NOTE: `prodSvcInfo` is the service the the Clients will be making requests to. +NOTE: `PROD-SVC` is the name of the service the Clients will be making requests to. [[spring-cloud-feign-hystrix]] === Feign Hystrix Support From 6e1ca54edcc181905e845ec79ddffc07ac692965 Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Mon, 24 Oct 2016 13:42:21 -0400 Subject: [PATCH 04/14] increased max retried to help with failures --- .../cloud/netflix/resttemplate/RestTemplateRetryTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/resttemplate/RestTemplateRetryTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/resttemplate/RestTemplateRetryTests.java index 90bf4d3a..4a885634 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/resttemplate/RestTemplateRetryTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/resttemplate/RestTemplateRetryTests.java @@ -47,7 +47,7 @@ import static org.junit.Assert.assertTrue; @SpringBootTest(classes = RestTemplateRetryTests.Application.class, webEnvironment = WebEnvironment.RANDOM_PORT, value = { "spring.application.name=resttemplatetest", "logging.level.com.netflix=DEBUG", "logging.level.org.springframework.cloud.netflix.resttemplate=DEBUG", - "logging.level.com.netflix=DEBUG", "badClients.ribbon.MaxAutoRetries=0", + "logging.level.com.netflix=DEBUG", "badClients.ribbon.MaxAutoRetries=25", "badClients.ribbon.OkToRetryOnAllOperations=true", "ribbon.http.client.enabled" }) @DirtiesContext public class RestTemplateRetryTests { From 0235d779323cbb557ee1a576006f7a62192336ed Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Mon, 24 Oct 2016 15:24:31 -0400 Subject: [PATCH 05/14] Zuul Route Hystrix Fallback Hystrix fallback support for Zuul routes. Fixes gh-250. --- .../netflix/zuul/ZuulProxyConfiguration.java | 19 ++++- .../route/RestClientRibbonCommand.java | 12 ++- .../route/RestClientRibbonCommandFactory.java | 21 +++-- .../filters/route/ZuulFallbackProvider.java | 40 +++++++++ .../route/apache/HttpClientRibbonCommand.java | 10 +++ .../HttpClientRibbonCommandFactory.java | 27 ++++-- .../route/okhttp/OkHttpRibbonCommand.java | 16 +++- .../okhttp/OkHttpRibbonCommandFactory.java | 31 +++++-- .../route/support/AbstractRibbonCommand.java | 18 +++- .../support/AbstractRibbonCommandFactory.java | 44 ++++++++++ .../HttpClientRibbonCommandFallbackTests.java | 46 ++++++++++ ...tpClientRibbonCommandIntegrationTests.java | 11 ++- .../OkHttpRibbonCommandFallbackTests.java | 44 ++++++++++ .../OkHttpRibbonCommandIntegrationTests.java | 11 ++- .../RestClientRibbonCommandFallbackTests.java | 44 ++++++++++ ...stClientRibbonCommandIntegrationTests.java | 17 ++-- .../support/RibbonCommandFallbackTests.java | 58 +++++++++++++ .../route/support/ZuulProxyTestBase.java | 84 +++++++++++++++++-- 18 files changed, 505 insertions(+), 48 deletions(-) create mode 100644 spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/ZuulFallbackProvider.java create mode 100644 spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/support/AbstractRibbonCommandFactory.java create mode 100644 spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/apache/HttpClientRibbonCommandFallbackTests.java create mode 100644 spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommandFallbackTests.java create mode 100644 spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/restclient/RestClientRibbonCommandFallbackTests.java create mode 100644 spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/support/RibbonCommandFallbackTests.java diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyConfiguration.java index 8777cd92..fa3e2838 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyConfiguration.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyConfiguration.java @@ -23,6 +23,7 @@ import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; import java.util.Collections; import java.util.List; +import java.util.Set; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.actuate.endpoint.Endpoint; @@ -52,6 +53,7 @@ import org.springframework.cloud.netflix.zuul.filters.route.RestClientRibbonComm import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandFactory; import org.springframework.cloud.netflix.zuul.filters.route.RibbonRoutingFilter; import org.springframework.cloud.netflix.zuul.filters.route.SimpleHostRoutingFilter; +import org.springframework.cloud.netflix.zuul.filters.route.ZuulFallbackProvider; import org.springframework.cloud.netflix.zuul.filters.route.apache.HttpClientRibbonCommandFactory; import org.springframework.cloud.netflix.zuul.filters.route.okhttp.OkHttpRibbonCommandFactory; import org.springframework.cloud.netflix.zuul.web.ZuulHandlerMapping; @@ -94,11 +96,14 @@ public class ZuulProxyConfiguration extends ZuulConfiguration { @ConditionalOnRibbonHttpClient protected static class HttpClientRibbonConfiguration { + @Autowired(required = false) + private Set zuulFallbackProviders = Collections.emptySet(); + @Bean @ConditionalOnMissingBean public RibbonCommandFactory ribbonCommandFactory( SpringClientFactory clientFactory, ZuulProperties zuulProperties) { - return new HttpClientRibbonCommandFactory(clientFactory, zuulProperties); + return new HttpClientRibbonCommandFactory(clientFactory, zuulProperties, zuulFallbackProviders); } } @@ -106,11 +111,15 @@ public class ZuulProxyConfiguration extends ZuulConfiguration { @ConditionalOnRibbonRestClient protected static class RestClientRibbonConfiguration { + @Autowired(required = false) + private Set zuulFallbackProviders = Collections.emptySet(); + @Bean @ConditionalOnMissingBean public RibbonCommandFactory ribbonCommandFactory( SpringClientFactory clientFactory, ZuulProperties zuulProperties) { - return new RestClientRibbonCommandFactory(clientFactory, zuulProperties); + return new RestClientRibbonCommandFactory(clientFactory, zuulProperties, + zuulFallbackProviders); } } @@ -119,11 +128,15 @@ public class ZuulProxyConfiguration extends ZuulConfiguration { @ConditionalOnClass(name = "okhttp3.OkHttpClient") protected static class OkHttpRibbonConfiguration { + @Autowired(required = false) + private Set zuulFallbackProviders = Collections.emptySet(); + @Bean @ConditionalOnMissingBean public RibbonCommandFactory ribbonCommandFactory( SpringClientFactory clientFactory, ZuulProperties zuulProperties) { - return new OkHttpRibbonCommandFactory(clientFactory, zuulProperties); + return new OkHttpRibbonCommandFactory(clientFactory, zuulProperties, + zuulFallbackProviders); } } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommand.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommand.java index d8fb01ed..bf913fe4 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommand.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommand.java @@ -17,20 +17,18 @@ package org.springframework.cloud.netflix.zuul.filters.route; -import static org.springframework.cloud.netflix.ribbon.support.RibbonRequestCustomizer.Runner.customize; - import java.io.InputStream; import java.net.URI; import java.util.List; - import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; import org.springframework.cloud.netflix.zuul.filters.route.support.AbstractRibbonCommand; import org.springframework.util.MultiValueMap; - import com.netflix.client.http.HttpRequest; import com.netflix.client.http.HttpResponse; import com.netflix.niws.client.http.RestClient; +import static org.springframework.cloud.netflix.ribbon.support.RibbonRequestCustomizer.Runner.customize; + /** * Hystrix wrapper around Eureka Ribbon command * @@ -44,6 +42,12 @@ public class RestClientRibbonCommand extends AbstractRibbonCommand { +public class RestClientRibbonCommandFactory extends AbstractRibbonCommandFactory { - private final SpringClientFactory clientFactory; + private SpringClientFactory clientFactory; private ZuulProperties zuulProperties; public RestClientRibbonCommandFactory(SpringClientFactory clientFactory) { - this(clientFactory, new ZuulProperties()); + this(clientFactory, new ZuulProperties(), Collections.emptySet()); } public RestClientRibbonCommandFactory(SpringClientFactory clientFactory, - ZuulProperties zuulProperties) { + ZuulProperties zuulProperties, + Set zuulFallbackProviders) { + super(zuulFallbackProviders); this.clientFactory = clientFactory; this.zuulProperties = zuulProperties; } @@ -45,10 +52,12 @@ public class RestClientRibbonCommandFactory implements RibbonCommandFactory { @@ -36,6 +38,14 @@ public class HttpClientRibbonCommand extends AbstractRibbonCommand { +public class HttpClientRibbonCommandFactory extends AbstractRibbonCommandFactory { private final SpringClientFactory clientFactory; private final ZuulProperties zuulProperties; + public HttpClientRibbonCommandFactory(SpringClientFactory clientFactory, ZuulProperties zuulProperties) { + this(clientFactory, zuulProperties, Collections.emptySet()); + } + + public HttpClientRibbonCommandFactory(SpringClientFactory clientFactory, ZuulProperties zuulProperties, + Set fallbackProviders) { + super(fallbackProviders); + this.clientFactory = clientFactory; + this.zuulProperties = zuulProperties; + } + @Override public HttpClientRibbonCommand create(final RibbonCommandContext context) { + ZuulFallbackProvider zuulFallbackProvider = getFallbackProvider(context.getServiceId()); final String serviceId = context.getServiceId(); final RibbonLoadBalancingHttpClient client = this.clientFactory.getClient( serviceId, RibbonLoadBalancingHttpClient.class); client.setLoadBalancer(this.clientFactory.getLoadBalancer(serviceId)); - return new HttpClientRibbonCommand(serviceId, client, context, zuulProperties); + return new HttpClientRibbonCommand(serviceId, client, context, zuulProperties, zuulFallbackProvider); } } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommand.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommand.java index f66f36b7..e4cd1972 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommand.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommand.java @@ -22,20 +22,30 @@ import org.springframework.cloud.netflix.ribbon.okhttp.OkHttpRibbonRequest; import org.springframework.cloud.netflix.ribbon.okhttp.OkHttpRibbonResponse; import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandContext; +import org.springframework.cloud.netflix.zuul.filters.route.ZuulFallbackProvider; import org.springframework.cloud.netflix.zuul.filters.route.support.AbstractRibbonCommand; /** * @author Spencer Gibb + * @author Ryan Baxter */ public class OkHttpRibbonCommand extends AbstractRibbonCommand { public OkHttpRibbonCommand(final String commandKey, - final OkHttpLoadBalancingClient client, - final RibbonCommandContext context, - final ZuulProperties zuulProperties) { + final OkHttpLoadBalancingClient client, + final RibbonCommandContext context, + final ZuulProperties zuulProperties) { super(commandKey, client, context, zuulProperties); } + public OkHttpRibbonCommand(final String commandKey, + final OkHttpLoadBalancingClient client, + final RibbonCommandContext context, + final ZuulProperties zuulProperties, + final ZuulFallbackProvider zuulFallbackProvider) { + super(commandKey, client, context, zuulProperties, zuulFallbackProvider); + } + @Override protected OkHttpRibbonRequest createRequest() throws Exception { return new OkHttpRibbonRequest(this.context); diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommandFactory.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommandFactory.java index 61001443..67ca4fcc 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommandFactory.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommandFactory.java @@ -16,33 +16,46 @@ package org.springframework.cloud.netflix.zuul.filters.route.okhttp; +import java.util.Collections; +import java.util.Set; + import org.springframework.cloud.netflix.ribbon.SpringClientFactory; import org.springframework.cloud.netflix.ribbon.okhttp.OkHttpLoadBalancingClient; import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandContext; -import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandFactory; - -import lombok.RequiredArgsConstructor; +import org.springframework.cloud.netflix.zuul.filters.route.ZuulFallbackProvider; +import org.springframework.cloud.netflix.zuul.filters.route.support.AbstractRibbonCommandFactory; /** * @author Spencer Gibb + * @author Ryan Baxter */ -@RequiredArgsConstructor -public class OkHttpRibbonCommandFactory implements - RibbonCommandFactory { +public class OkHttpRibbonCommandFactory extends AbstractRibbonCommandFactory { - private final SpringClientFactory clientFactory; + private SpringClientFactory clientFactory; - private final ZuulProperties zuulProperties; + private ZuulProperties zuulProperties; + + public OkHttpRibbonCommandFactory(SpringClientFactory clientFactory, ZuulProperties zuulProperties) { + this(clientFactory, zuulProperties, Collections.emptySet()); + } + + public OkHttpRibbonCommandFactory(SpringClientFactory clientFactory, ZuulProperties zuulProperties, + Set zuulFallbackProviders) { + super(zuulFallbackProviders); + this.clientFactory = clientFactory; + this.zuulProperties = zuulProperties; + } @Override public OkHttpRibbonCommand create(final RibbonCommandContext context) { final String serviceId = context.getServiceId(); + ZuulFallbackProvider fallbackProvider = getFallbackProvider(serviceId); final OkHttpLoadBalancingClient client = this.clientFactory.getClient( serviceId, OkHttpLoadBalancingClient.class); client.setLoadBalancer(this.clientFactory.getLoadBalancer(serviceId)); - return new OkHttpRibbonCommand(serviceId, client, context, zuulProperties); + return new OkHttpRibbonCommand(serviceId, client, context, zuulProperties, fallbackProvider); } } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/support/AbstractRibbonCommand.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/support/AbstractRibbonCommand.java index 64f74f87..f5cb685f 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/support/AbstractRibbonCommand.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/support/AbstractRibbonCommand.java @@ -21,8 +21,8 @@ import org.springframework.cloud.netflix.ribbon.RibbonHttpResponse; import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommand; import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandContext; +import org.springframework.cloud.netflix.zuul.filters.route.ZuulFallbackProvider; import org.springframework.http.client.ClientHttpResponse; - import com.netflix.client.AbstractLoadBalancerAwareClient; import com.netflix.client.ClientRequest; import com.netflix.client.http.HttpResponse; @@ -44,6 +44,7 @@ public abstract class AbstractRibbonCommand fallbackProviderCache; + + public AbstractRibbonCommandFactory(Set fallbackProviders){ + this.fallbackProviderCache = new HashMap(); + for(ZuulFallbackProvider provider : fallbackProviders) { + fallbackProviderCache.put(provider.getRoute(), provider); + } + } + + protected ZuulFallbackProvider getFallbackProvider(String route) { + return fallbackProviderCache.get(route); + } +} diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/apache/HttpClientRibbonCommandFallbackTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/apache/HttpClientRibbonCommandFallbackTests.java new file mode 100644 index 00000000..ea30e4e1 --- /dev/null +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/apache/HttpClientRibbonCommandFallbackTests.java @@ -0,0 +1,46 @@ +/* + * + * * Copyright 2013-2016 the original author or authors. + * * + * * Licensed under the Apache License, Version 2.0 (the "License"); + * * you may not use this file except in compliance with the License. + * * You may obtain a copy of the License at + * * + * * http://www.apache.org/licenses/LICENSE-2.0 + * * + * * Unless required by applicable law or agreed to in writing, software + * * distributed under the License is distributed on an "AS IS" BASIS, + * * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * * See the License for the specific language governing permissions and + * * limitations under the License. + * + */ + +package org.springframework.cloud.netflix.zuul.filters.route.apache; + +import org.junit.Before; +import org.junit.runner.RunWith; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.cloud.netflix.zuul.filters.route.support.RibbonCommandFallbackTests; +import org.springframework.test.annotation.DirtiesContext; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; + +import com.netflix.zuul.context.RequestContext; + +/** + * @author Ryan Baxter + */ +@RunWith(SpringJUnit4ClassRunner.class) +@SpringBootTest(classes = HttpClientRibbonCommandIntegrationTests.TestConfig.class, webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT, value = { + "zuul.routes.simple: /simple/**", "zuul.routes.another: /another/twolevel/**", + "ribbon.ReadTimeout: 1"}) +@DirtiesContext +public class HttpClientRibbonCommandFallbackTests extends RibbonCommandFallbackTests { + + @Before + public void init() { + RequestContext.testSetCurrentContext(null); + RequestContext.getCurrentContext().unset(); + } + +} diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/apache/HttpClientRibbonCommandIntegrationTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/apache/HttpClientRibbonCommandIntegrationTests.java index b41940e7..6f420150 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/apache/HttpClientRibbonCommandIntegrationTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/apache/HttpClientRibbonCommandIntegrationTests.java @@ -22,6 +22,9 @@ import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.springframework.http.HttpHeaders.SET_COOKIE; +import java.util.Collections; +import java.util.Set; + import javax.servlet.http.Cookie; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -29,6 +32,7 @@ import javax.servlet.http.HttpServletResponse; import org.junit.Before; 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.autoconfigure.EnableAutoConfiguration; import org.springframework.boot.autoconfigure.web.ErrorAttributes; @@ -44,6 +48,7 @@ import org.springframework.cloud.netflix.ribbon.apache.RibbonLoadBalancingHttpCl import org.springframework.cloud.netflix.zuul.EnableZuulProxy; import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandFactory; +import org.springframework.cloud.netflix.zuul.filters.route.ZuulFallbackProvider; import org.springframework.cloud.netflix.zuul.filters.route.support.ZuulProxyTestBase; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -155,6 +160,9 @@ public class HttpClientRibbonCommandIntegrationTests extends ZuulProxyTestBase { @RibbonClient(name = "singleton", configuration = SingletonRibbonClientConfiguration.class) }) static class TestConfig extends ZuulProxyTestBase.AbstractZuulProxyApplication { + @Autowired(required = false) + private Set zuulFallbackProviders = Collections.emptySet(); + @RequestMapping(value = "/local/{id}", method = RequestMethod.PATCH) public String patch(@PathVariable final String id, @RequestBody final String body) { @@ -176,7 +184,8 @@ public class HttpClientRibbonCommandIntegrationTests extends ZuulProxyTestBase { @Bean public RibbonCommandFactory ribbonCommandFactory( final SpringClientFactory clientFactory) { - return new HttpClientRibbonCommandFactory(clientFactory, new ZuulProperties()); + return new HttpClientRibbonCommandFactory(clientFactory, new ZuulProperties(), + zuulFallbackProviders); } @Bean diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommandFallbackTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommandFallbackTests.java new file mode 100644 index 00000000..864e5504 --- /dev/null +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommandFallbackTests.java @@ -0,0 +1,44 @@ +/* + * + * * Copyright 2013-2016 the original author or authors. + * * + * * Licensed under the Apache License, Version 2.0 (the "License"); + * * you may not use this file except in compliance with the License. + * * You may obtain a copy of the License at + * * + * * http://www.apache.org/licenses/LICENSE-2.0 + * * + * * Unless required by applicable law or agreed to in writing, software + * * distributed under the License is distributed on an "AS IS" BASIS, + * * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * * See the License for the specific language governing permissions and + * * limitations under the License. + * + */ + +package org.springframework.cloud.netflix.zuul.filters.route.okhttp; + +import org.junit.Before; +import org.junit.runner.RunWith; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.cloud.netflix.zuul.filters.route.support.RibbonCommandFallbackTests; +import org.springframework.test.annotation.DirtiesContext; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; + +import com.netflix.zuul.context.RequestContext; + +/** + * @author Ryan Baxter + */ +@RunWith(SpringJUnit4ClassRunner.class) +@SpringBootTest(classes = OkHttpRibbonCommandIntegrationTests.TestConfig.class, webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT, value = { + "zuul.routes.simple: /simple/**", "zuul.routes.another: /another/twolevel/**", + "ribbon.ReadTimeout: 1"}) +@DirtiesContext +public class OkHttpRibbonCommandFallbackTests extends RibbonCommandFallbackTests { + @Before + public void init() { + RequestContext.testSetCurrentContext(null); + RequestContext.getCurrentContext().unset(); + } +} diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommandIntegrationTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommandIntegrationTests.java index 80f06806..bfd5b7ea 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommandIntegrationTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommandIntegrationTests.java @@ -20,9 +20,13 @@ package org.springframework.cloud.netflix.zuul.filters.route.okhttp; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; +import java.util.Collections; +import java.util.Set; + import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.autoconfigure.EnableAutoConfiguration; import org.springframework.boot.autoconfigure.web.ErrorAttributes; import org.springframework.boot.test.context.SpringBootTest; @@ -34,6 +38,7 @@ import org.springframework.cloud.netflix.ribbon.SpringClientFactory; import org.springframework.cloud.netflix.zuul.EnableZuulProxy; import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandFactory; +import org.springframework.cloud.netflix.zuul.filters.route.ZuulFallbackProvider; import org.springframework.cloud.netflix.zuul.filters.route.support.ZuulProxyTestBase; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -100,10 +105,14 @@ public class OkHttpRibbonCommandIntegrationTests extends ZuulProxyTestBase { @RibbonClient(name = "another", configuration = AnotherRibbonClientConfiguration.class) }) static class TestConfig extends ZuulProxyTestBase.AbstractZuulProxyApplication { + @Autowired(required = false) + private Set zuulFallbackProviders = Collections.emptySet(); + @Bean public RibbonCommandFactory ribbonCommandFactory( final SpringClientFactory clientFactory) { - return new OkHttpRibbonCommandFactory(clientFactory, new ZuulProperties()); + return new OkHttpRibbonCommandFactory(clientFactory, new ZuulProperties(), + zuulFallbackProviders); } @Bean diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/restclient/RestClientRibbonCommandFallbackTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/restclient/RestClientRibbonCommandFallbackTests.java new file mode 100644 index 00000000..ca9784f3 --- /dev/null +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/restclient/RestClientRibbonCommandFallbackTests.java @@ -0,0 +1,44 @@ +/* + * + * * Copyright 2013-2016 the original author or authors. + * * + * * Licensed under the Apache License, Version 2.0 (the "License"); + * * you may not use this file except in compliance with the License. + * * You may obtain a copy of the License at + * * + * * http://www.apache.org/licenses/LICENSE-2.0 + * * + * * Unless required by applicable law or agreed to in writing, software + * * distributed under the License is distributed on an "AS IS" BASIS, + * * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * * See the License for the specific language governing permissions and + * * limitations under the License. + * + */ + +package org.springframework.cloud.netflix.zuul.filters.route.restclient; + +import org.junit.Before; +import org.junit.runner.RunWith; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.cloud.netflix.zuul.filters.route.support.RibbonCommandFallbackTests; +import org.springframework.test.annotation.DirtiesContext; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; + +import com.netflix.zuul.context.RequestContext; + +/** + * @author Ryan Baxter + */ +@RunWith(SpringJUnit4ClassRunner.class) +@SpringBootTest(classes = RestClientRibbonCommandIntegrationTests.TestConfig.class, webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT, value = { + "zuul.routes.simple: /simple/**", "zuul.routes.another: /another/twolevel/**", + "ribbon.ReadTimeout: 1"}) +@DirtiesContext +public class RestClientRibbonCommandFallbackTests extends RibbonCommandFallbackTests { + @Before + public void init() { + RequestContext.testSetCurrentContext(null); + RequestContext.getCurrentContext().unset(); + } +} diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/restclient/RestClientRibbonCommandIntegrationTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/restclient/RestClientRibbonCommandIntegrationTests.java index 7b1a4d8b..790cab60 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/restclient/RestClientRibbonCommandIntegrationTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/restclient/RestClientRibbonCommandIntegrationTests.java @@ -26,10 +26,14 @@ import static org.junit.Assert.assertTrue; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; +import java.util.Set; import java.util.UUID; import javax.servlet.http.HttpServletRequest; +import lombok.SneakyThrows; + import org.junit.Test; import org.junit.runner.RunWith; import org.springframework.beans.factory.annotation.Autowired; @@ -52,6 +56,7 @@ import org.springframework.cloud.netflix.zuul.filters.route.RestClientRibbonComm import org.springframework.cloud.netflix.zuul.filters.route.RestClientRibbonCommandFactory; import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandContext; import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandFactory; +import org.springframework.cloud.netflix.zuul.filters.route.ZuulFallbackProvider; import org.springframework.cloud.netflix.zuul.filters.route.support.NoEncodingFormHttpMessageConverter; import org.springframework.cloud.netflix.zuul.filters.route.support.ZuulProxyTestBase; import org.springframework.context.annotation.Bean; @@ -80,8 +85,6 @@ import com.netflix.loadbalancer.Server; import com.netflix.loadbalancer.ServerList; import com.netflix.niws.client.http.RestClient; -import lombok.SneakyThrows; - @RunWith(SpringJUnit4ClassRunner.class) @SpringBootTest(classes = RestClientRibbonCommandIntegrationTests.TestConfig.class, webEnvironment = WebEnvironment.RANDOM_PORT, value = { "zuul.routes.other: /test/**=http://localhost:7777/local", @@ -285,6 +288,9 @@ public class RestClientRibbonCommandIntegrationTests extends ZuulProxyTestBase { @RibbonClient(name = "another", configuration = ZuulProxyTestBase.AnotherRibbonClientConfiguration.class) }) static class TestConfig extends ZuulProxyTestBase.AbstractZuulProxyApplication { + @Autowired(required = false) + private Set fallbackProviders = Collections.emptySet(); + @RequestMapping("/trailing-slash") public String trailingSlash(HttpServletRequest request) { return request.getRequestURI(); @@ -327,7 +333,7 @@ public class RestClientRibbonCommandIntegrationTests extends ZuulProxyTestBase { @Bean public RibbonCommandFactory ribbonCommandFactory( SpringClientFactory clientFactory) { - return new MyRibbonCommandFactory(clientFactory); + return new MyRibbonCommandFactory(clientFactory, fallbackProviders); } @Bean @@ -350,8 +356,9 @@ public class RestClientRibbonCommandIntegrationTests extends ZuulProxyTestBase { private SpringClientFactory clientFactory; - public MyRibbonCommandFactory(SpringClientFactory clientFactory) { - super(clientFactory, new ZuulProperties()); + public MyRibbonCommandFactory(SpringClientFactory clientFactory, + Set fallbackProviders) { + super(clientFactory, new ZuulProperties(), fallbackProviders); this.clientFactory = clientFactory; } diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/support/RibbonCommandFallbackTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/support/RibbonCommandFallbackTests.java new file mode 100644 index 00000000..303241e1 --- /dev/null +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/support/RibbonCommandFallbackTests.java @@ -0,0 +1,58 @@ +/* + * + * * Copyright 2013-2016 the original author or authors. + * * + * * Licensed under the Apache License, Version 2.0 (the "License"); + * * you may not use this file except in compliance with the License. + * * You may obtain a copy of the License at + * * + * * http://www.apache.org/licenses/LICENSE-2.0 + * * + * * Unless required by applicable law or agreed to in writing, software + * * distributed under the License is distributed on an "AS IS" BASIS, + * * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * * See the License for the specific language governing permissions and + * * limitations under the License. + * + */ + +package org.springframework.cloud.netflix.zuul.filters.route.support; + +import org.junit.Test; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.boot.test.web.client.TestRestTemplate; +import org.springframework.http.HttpEntity; +import org.springframework.http.HttpMethod; +import org.springframework.http.HttpStatus; +import org.springframework.http.ResponseEntity; + +import static org.junit.Assert.assertEquals; + +/** + * @author Ryan Baxter + */ +public abstract class RibbonCommandFallbackTests { + + @Value("${local.server.port}") + protected int port; + + @Test + public void fallback() { + String uri = "/simple/slow"; + ResponseEntity result = new TestRestTemplate().exchange( + "http://localhost:" + this.port + uri, HttpMethod.GET, + new HttpEntity<>((Void) null), String.class); + assertEquals(HttpStatus.OK, result.getStatusCode()); + assertEquals("fallback", result.getBody()); + } + + @Test + public void noFallback() { + String uri = "/another/twolevel/slow"; + ResponseEntity result = new TestRestTemplate().exchange( + "http://localhost:" + this.port + uri, HttpMethod.GET, + new HttpEntity<>((Void) null), String.class); + System.out.println("no fallback body: " + result.getBody()); + assertEquals(HttpStatus.INTERNAL_SERVER_ERROR, result.getStatusCode()); + } +} diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/support/ZuulProxyTestBase.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/support/ZuulProxyTestBase.java index 9119c4a3..58bd1806 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/support/ZuulProxyTestBase.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/support/ZuulProxyTestBase.java @@ -17,11 +17,9 @@ package org.springframework.cloud.netflix.zuul.filters.route.support; -import static org.hamcrest.Matchers.is; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assume.assumeThat; - +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.io.InputStream; import java.nio.charset.Charset; import java.util.ArrayList; import java.util.Arrays; @@ -29,9 +27,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.concurrent.atomic.AtomicBoolean; - import javax.servlet.http.HttpServletRequest; - import org.junit.Before; import org.junit.Test; import org.springframework.beans.factory.annotation.Autowired; @@ -46,13 +42,16 @@ import org.springframework.cloud.netflix.zuul.filters.Route; import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; import org.springframework.cloud.netflix.zuul.filters.discovery.DiscoveryClientRouteLocator; import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandFactory; +import org.springframework.cloud.netflix.zuul.filters.route.ZuulFallbackProvider; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; 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.ClientHttpResponse; import org.springframework.http.converter.FormHttpMessageConverter; import org.springframework.http.converter.HttpMessageConverter; import org.springframework.http.converter.StringHttpMessageConverter; @@ -66,14 +65,19 @@ import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.servlet.config.annotation.DelegatingWebMvcConfiguration; import org.springframework.web.servlet.config.annotation.WebMvcConfigurerAdapter; import org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerMapping; - import com.netflix.loadbalancer.Server; import com.netflix.loadbalancer.ServerList; import com.netflix.zuul.ZuulFilter; import com.netflix.zuul.context.RequestContext; +import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assume.assumeThat; + /** * @author Spencer Gibb + * @author Ryan Baxter */ public abstract class ZuulProxyTestBase { @@ -363,6 +367,22 @@ public abstract class ZuulProxyTestBase { return "Hello space"; } + @RequestMapping("/slow") + public String slow() { + try { + Thread.sleep(80000); + } catch (InterruptedException e) { + e.printStackTrace(); + } + return "slow"; + + } + + @Bean + public ZuulFallbackProvider fallbackProvider() { + return new FallbackProvider(); + } + @Bean public ZuulFilter sampleFilter() { return new ZuulFilter() { @@ -393,6 +413,7 @@ public abstract class ZuulProxyTestBase { return 0; } }; + } @Override @@ -401,6 +422,53 @@ public abstract class ZuulProxyTestBase { mapping.setRemoveSemicolonContent(false); return mapping; } + + + } + + public static class FallbackProvider implements ZuulFallbackProvider { + + @Override + public String getRoute() { + return "simple"; + } + + @Override + public ClientHttpResponse fallbackResponse() { + return new ClientHttpResponse() { + @Override + public HttpStatus getStatusCode() throws IOException { + return HttpStatus.OK; + } + + @Override + public int getRawStatusCode() throws IOException { + return 200; + } + + @Override + public String getStatusText() throws IOException { + return null; + } + + @Override + public void close() { + + } + + @Override + public InputStream getBody() throws IOException { + return new ByteArrayInputStream("fallback".getBytes()); + } + + @Override + public HttpHeaders getHeaders() { + HttpHeaders headers = new HttpHeaders(); + headers.setContentType(MediaType.TEXT_HTML); + return headers; + } + }; + } } @Configuration From 0e1b856fc5e6aedd8dc6ccb9ea4dc693d89794ac Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Tue, 25 Oct 2016 07:50:32 +0100 Subject: [PATCH 06/14] Extract proxy header manipulation into method --- .../zuul/filters/pre/PreDecorationFilter.java | 93 ++++++++++--------- 1 file changed, 48 insertions(+), 45 deletions(-) diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilter.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilter.java index 6db078a9..1b5bd474 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilter.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilter.java @@ -50,12 +50,11 @@ public class PreDecorationFilter extends ZuulFilter { private ProxyRequestHelper proxyRequestHelper; - public PreDecorationFilter(RouteLocator routeLocator, String dispatcherServletPath, - ZuulProperties properties, ProxyRequestHelper proxyRequestHelper) { + public PreDecorationFilter(RouteLocator routeLocator, String dispatcherServletPath, ZuulProperties properties, + ProxyRequestHelper proxyRequestHelper) { this.routeLocator = routeLocator; this.properties = properties; - this.urlPathHelper - .setRemoveSemicolonContent(properties.isRemoveSemicolonContent()); + this.urlPathHelper.setRemoveSemicolonContent(properties.isRemoveSemicolonContent()); this.dispatcherServletPath = dispatcherServletPath; this.proxyRequestHelper = proxyRequestHelper; } @@ -81,8 +80,7 @@ public class PreDecorationFilter extends ZuulFilter { @Override public Object run() { RequestContext ctx = RequestContext.getCurrentContext(); - final String requestURI = this.urlPathHelper - .getPathWithinApplication(ctx.getRequest()); + final String requestURI = this.urlPathHelper.getPathWithinApplication(ctx.getRequest()); Route route = this.routeLocator.getMatchingRoute(requestURI); if (route != null) { String location = route.getLocation(); @@ -90,12 +88,11 @@ public class PreDecorationFilter extends ZuulFilter { ctx.put("requestURI", route.getPath()); ctx.put("proxy", route.getId()); if (!route.isCustomSensitiveHeaders()) { - this.proxyRequestHelper.addIgnoredHeaders( - this.properties.getSensitiveHeaders().toArray(new String[0])); + this.proxyRequestHelper + .addIgnoredHeaders(this.properties.getSensitiveHeaders().toArray(new String[0])); } else { - this.proxyRequestHelper.addIgnoredHeaders( - route.getSensitiveHeaders().toArray(new String[0])); + this.proxyRequestHelper.addIgnoredHeaders(route.getSensitiveHeaders().toArray(new String[0])); } if (route.getRetryable() != null) { @@ -107,8 +104,8 @@ public class PreDecorationFilter extends ZuulFilter { ctx.addOriginResponseHeader("X-Zuul-Service", location); } else if (location.startsWith("forward:")) { - ctx.set("forward.to", StringUtils.cleanPath( - location.substring("forward:".length()) + route.getPath())); + ctx.set("forward.to", + StringUtils.cleanPath(location.substring("forward:".length()) + route.getPath())); ctx.setRouteHost(null); return null; } @@ -119,35 +116,7 @@ public class PreDecorationFilter extends ZuulFilter { ctx.addOriginResponseHeader("X-Zuul-ServiceId", location); } if (this.properties.isAddProxyHeaders()) { - ctx.addZuulRequestHeader("X-Forwarded-Host", toHostHeader(ctx.getRequest())); - ctx.addZuulRequestHeader("X-Forwarded-Port", - String.valueOf(ctx.getRequest().getServerPort())); - ctx.addZuulRequestHeader(ZuulHeaders.X_FORWARDED_PROTO, - ctx.getRequest().getScheme()); - String forwardedPrefix = - ctx.getRequest().getHeader("X-Forwarded-Prefix"); - String contextPath = ctx.getRequest().getContextPath(); - String prefix = StringUtils.hasLength(forwardedPrefix) - ? forwardedPrefix - : (StringUtils.hasLength(contextPath) ? contextPath : null); - if (StringUtils.hasText(route.getPrefix())) { - StringBuilder newPrefixBuilder = new StringBuilder(); - if (prefix != null) { - if (prefix.endsWith("/") - && route.getPrefix().startsWith("/")) { - newPrefixBuilder.append(prefix, 0, - prefix.length() - 1); - } - else { - newPrefixBuilder.append(prefix); - } - } - newPrefixBuilder.append(route.getPrefix()); - prefix = newPrefixBuilder.toString(); - } - if (prefix != null) { - ctx.addZuulRequestHeader("X-Forwarded-Prefix", prefix); - } + addProxyHeaders(ctx, route); String xforwardedfor = ctx.getRequest().getHeader("X-Forwarded-For"); String remoteAddr = ctx.getRequest().getRemoteAddr(); if (xforwardedfor == null) { @@ -174,8 +143,7 @@ public class PreDecorationFilter extends ZuulFilter { if (RequestUtils.isZuulServletRequest()) { // remove the Zuul servletPath from the requestUri log.debug("zuulServletPath=" + this.properties.getServletPath()); - fallBackUri = fallBackUri.replaceFirst(this.properties.getServletPath(), - ""); + fallBackUri = fallBackUri.replaceFirst(this.properties.getServletPath(), ""); log.debug("Replaced Zuul servlet path:" + fallBackUri); } else { @@ -194,11 +162,46 @@ public class PreDecorationFilter extends ZuulFilter { return null; } + private void addProxyHeaders(RequestContext ctx, Route route) { + String host = toHostHeader(ctx.getRequest()); + String port = String.valueOf(ctx.getRequest().getServerPort()); + String proto = ctx.getRequest().getScheme(); + ctx.addZuulRequestHeader("X-Forwarded-Host", host); + ctx.addZuulRequestHeader("X-Forwarded-Port", port); + ctx.addZuulRequestHeader(ZuulHeaders.X_FORWARDED_PROTO, proto); + addProxyPrefix(ctx, route); + } + + private void addProxyPrefix(RequestContext ctx, Route route) { + String forwardedPrefix = ctx.getRequest().getHeader("X-Forwarded-Prefix"); + String contextPath = ctx.getRequest().getContextPath(); + String prefix = StringUtils.hasLength(forwardedPrefix) ? forwardedPrefix + : (StringUtils.hasLength(contextPath) ? contextPath : null); + if (StringUtils.hasText(route.getPrefix())) { + StringBuilder newPrefixBuilder = new StringBuilder(); + if (prefix != null) { + if (prefix.endsWith("/") && route.getPrefix().startsWith("/")) { + newPrefixBuilder.append(prefix, 0, prefix.length() - 1); + } + else { + newPrefixBuilder.append(prefix); + } + } + newPrefixBuilder.append(route.getPrefix()); + prefix = newPrefixBuilder.toString(); + } + if (prefix != null) { + ctx.addZuulRequestHeader("X-Forwarded-Prefix", prefix); + } + } + private String toHostHeader(HttpServletRequest request) { int port = request.getServerPort(); - if ((port == 80 && "http".equals(request.getScheme())) || (port == 443 && "https".equals(request.getScheme()))) { + if ((port == 80 && "http".equals(request.getScheme())) + || (port == 443 && "https".equals(request.getScheme()))) { return request.getServerName(); - } else { + } + else { return request.getServerName() + ":" + port; } } From 71ac67f9f17938277afe78482923695af5a60134 Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Tue, 25 Oct 2016 08:50:26 +0100 Subject: [PATCH 07/14] Suppress some warnings --- .../cloud/netflix/zuul/ZuulProxyConfiguration.java | 1 + .../cloud/netflix/AdhocTestSuite.java | 12 +++++++----- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyConfiguration.java index fa3e2838..11ee38b1 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyConfiguration.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyConfiguration.java @@ -70,6 +70,7 @@ import org.springframework.context.annotation.Configuration; @Configuration public class ZuulProxyConfiguration extends ZuulConfiguration { + @SuppressWarnings("rawtypes") @Autowired(required = false) private List requestCustomizers = Collections.emptyList(); diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/AdhocTestSuite.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/AdhocTestSuite.java index 2200743e..47a365ed 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/AdhocTestSuite.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/AdhocTestSuite.java @@ -20,7 +20,11 @@ import org.junit.Ignore; import org.junit.runner.RunWith; import org.junit.runners.Suite; import org.junit.runners.Suite.SuiteClasses; -import org.springframework.cloud.netflix.zuul.filters.route.restclient.RestClientRibbonCommandIntegrationTests; +import org.springframework.cloud.netflix.feign.encoding.FeignAcceptEncodingTests; +import org.springframework.cloud.netflix.metrics.servo.ServoMetricReaderTests; +import org.springframework.cloud.netflix.ribbon.RibbonInterceptorTests; +import org.springframework.cloud.netflix.ribbon.RibbonLoadBalancerClientTests; +import org.springframework.cloud.netflix.zuul.ZuulProxyConfigurationTests; /** * A test suite for probing weird ordering problems in the tests. @@ -28,10 +32,8 @@ import org.springframework.cloud.netflix.zuul.filters.route.restclient.RestClien * @author Dave Syer */ @RunWith(Suite.class) -@SuiteClasses({ - org.springframework.cloud.netflix.zuul.filters.ProxyRequestHelperTests.class, - RestClientRibbonCommandIntegrationTests.class, - org.springframework.cloud.netflix.zuul.FormZuulProxyApplicationTests.class }) +@SuiteClasses({ RibbonLoadBalancerClientTests.class, RibbonInterceptorTests.class, FeignAcceptEncodingTests.class, + ServoMetricReaderTests.class, ZuulProxyConfigurationTests.class }) @Ignore public class AdhocTestSuite { From 139ab95438d0eea0c90c8aea23f3098caeca2333 Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Tue, 25 Oct 2016 09:35:35 +0100 Subject: [PATCH 08/14] Make configuration imports deterministic --- .../RibbonCommandFactoryConfiguration.java | 154 ++++++++++++++++++ .../netflix/zuul/ZuulProxyConfiguration.java | 143 ++-------------- 2 files changed, 166 insertions(+), 131 deletions(-) create mode 100644 spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/RibbonCommandFactoryConfiguration.java diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/RibbonCommandFactoryConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/RibbonCommandFactoryConfiguration.java new file mode 100644 index 00000000..95741603 --- /dev/null +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/RibbonCommandFactoryConfiguration.java @@ -0,0 +1,154 @@ +/* + * Copyright 2015-2016 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.netflix.zuul; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; +import java.util.Collections; +import java.util.Set; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.autoconfigure.condition.AnyNestedCondition; +import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; +import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; +import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.cloud.netflix.ribbon.SpringClientFactory; +import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; +import org.springframework.cloud.netflix.zuul.filters.route.RestClientRibbonCommandFactory; +import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandFactory; +import org.springframework.cloud.netflix.zuul.filters.route.ZuulFallbackProvider; +import org.springframework.cloud.netflix.zuul.filters.route.apache.HttpClientRibbonCommandFactory; +import org.springframework.cloud.netflix.zuul.filters.route.okhttp.OkHttpRibbonCommandFactory; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Conditional; +import org.springframework.context.annotation.Configuration; + +/** + * @author Dave Syer + * + */ +public class RibbonCommandFactoryConfiguration { + + @Configuration + @ConditionalOnRibbonRestClient + protected static class RestClientRibbonConfiguration { + + @Autowired(required = false) + private Set zuulFallbackProviders = Collections.emptySet(); + + @Bean + @ConditionalOnMissingBean + public RibbonCommandFactory ribbonCommandFactory( + SpringClientFactory clientFactory, ZuulProperties zuulProperties) { + return new RestClientRibbonCommandFactory(clientFactory, zuulProperties, + zuulFallbackProviders); + } + } + + @Configuration + @ConditionalOnRibbonOkHttpClient + @ConditionalOnClass(name = "okhttp3.OkHttpClient") + protected static class OkHttpRibbonConfiguration { + + @Autowired(required = false) + private Set zuulFallbackProviders = Collections.emptySet(); + + @Bean + @ConditionalOnMissingBean + public RibbonCommandFactory ribbonCommandFactory( + SpringClientFactory clientFactory, ZuulProperties zuulProperties) { + return new OkHttpRibbonCommandFactory(clientFactory, zuulProperties, + zuulFallbackProviders); + } + } + + @Configuration + @ConditionalOnRibbonHttpClient + protected static class HttpClientRibbonConfiguration { + + @Autowired(required = false) + private Set zuulFallbackProviders = Collections.emptySet(); + + @Bean + @ConditionalOnMissingBean + public RibbonCommandFactory ribbonCommandFactory( + SpringClientFactory clientFactory, ZuulProperties zuulProperties) { + return new HttpClientRibbonCommandFactory(clientFactory, zuulProperties, zuulFallbackProviders); + } + } + + @Target({ ElementType.TYPE, ElementType.METHOD }) + @Retention(RetentionPolicy.RUNTIME) + @Documented + @Conditional(OnRibbonHttpClientCondition.class) + @interface ConditionalOnRibbonHttpClient { } + + private static class OnRibbonHttpClientCondition extends AnyNestedCondition { + public OnRibbonHttpClientCondition() { + super(ConfigurationPhase.PARSE_CONFIGURATION); + } + + @Deprecated //remove in Edgware" + @ConditionalOnProperty(name = "zuul.ribbon.httpclient.enabled", matchIfMissing = true) + static class ZuulProperty {} + + @ConditionalOnProperty(name = "ribbon.httpclient.enabled", matchIfMissing = true) + static class RibbonProperty {} + } + + @Target({ ElementType.TYPE, ElementType.METHOD }) + @Retention(RetentionPolicy.RUNTIME) + @Documented + @Conditional(OnRibbonOkHttpClientCondition.class) + @interface ConditionalOnRibbonOkHttpClient { } + + private static class OnRibbonOkHttpClientCondition extends AnyNestedCondition { + public OnRibbonOkHttpClientCondition() { + super(ConfigurationPhase.PARSE_CONFIGURATION); + } + + @Deprecated //remove in Edgware" + @ConditionalOnProperty("zuul.ribbon.okhttp.enabled") + static class ZuulProperty {} + + @ConditionalOnProperty("ribbon.okhttp.enabled") + static class RibbonProperty {} + } + + @Target({ ElementType.TYPE, ElementType.METHOD }) + @Retention(RetentionPolicy.RUNTIME) + @Documented + @Conditional(OnRibbonRestClientCondition.class) + @interface ConditionalOnRibbonRestClient { } + + private static class OnRibbonRestClientCondition extends AnyNestedCondition { + public OnRibbonRestClientCondition() { + super(ConfigurationPhase.PARSE_CONFIGURATION); + } + + @Deprecated //remove in Edgware" + @ConditionalOnProperty("zuul.ribbon.restclient.enabled") + static class ZuulProperty {} + + @ConditionalOnProperty("ribbon.restclient.enabled") + static class RibbonProperty {} + } + +} diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyConfiguration.java index 11ee38b1..40ea7a9c 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyConfiguration.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyConfiguration.java @@ -16,30 +16,21 @@ package org.springframework.cloud.netflix.zuul; -import java.lang.annotation.Documented; -import java.lang.annotation.ElementType; -import java.lang.annotation.Retention; -import java.lang.annotation.RetentionPolicy; -import java.lang.annotation.Target; import java.util.Collections; import java.util.List; -import java.util.Set; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.actuate.endpoint.Endpoint; import org.springframework.boot.actuate.trace.TraceRepository; -import org.springframework.boot.autoconfigure.condition.AnyNestedCondition; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingClass; -import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.cloud.client.actuator.HasFeatures; import org.springframework.cloud.client.discovery.DiscoveryClient; import org.springframework.cloud.client.discovery.event.HeartbeatEvent; import org.springframework.cloud.client.discovery.event.HeartbeatMonitor; import org.springframework.cloud.client.discovery.event.InstanceRegisteredEvent; import org.springframework.cloud.client.discovery.event.ParentHeartbeatEvent; -import org.springframework.cloud.netflix.ribbon.SpringClientFactory; import org.springframework.cloud.netflix.ribbon.support.RibbonRequestCustomizer; import org.springframework.cloud.netflix.zuul.filters.ProxyRequestHelper; import org.springframework.cloud.netflix.zuul.filters.RouteLocator; @@ -49,25 +40,24 @@ import org.springframework.cloud.netflix.zuul.filters.discovery.DiscoveryClientR import org.springframework.cloud.netflix.zuul.filters.discovery.ServiceRouteMapper; import org.springframework.cloud.netflix.zuul.filters.discovery.SimpleServiceRouteMapper; import org.springframework.cloud.netflix.zuul.filters.pre.PreDecorationFilter; -import org.springframework.cloud.netflix.zuul.filters.route.RestClientRibbonCommandFactory; import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandFactory; import org.springframework.cloud.netflix.zuul.filters.route.RibbonRoutingFilter; import org.springframework.cloud.netflix.zuul.filters.route.SimpleHostRoutingFilter; -import org.springframework.cloud.netflix.zuul.filters.route.ZuulFallbackProvider; -import org.springframework.cloud.netflix.zuul.filters.route.apache.HttpClientRibbonCommandFactory; -import org.springframework.cloud.netflix.zuul.filters.route.okhttp.OkHttpRibbonCommandFactory; import org.springframework.cloud.netflix.zuul.web.ZuulHandlerMapping; import org.springframework.context.ApplicationEvent; import org.springframework.context.ApplicationListener; import org.springframework.context.annotation.Bean; -import org.springframework.context.annotation.Conditional; import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Import; /** * @author Spencer Gibb * @author Dave Syer */ @Configuration +@Import({ RibbonCommandFactoryConfiguration.RestClientRibbonConfiguration.class, + RibbonCommandFactoryConfiguration.OkHttpRibbonConfiguration.class, + RibbonCommandFactoryConfiguration.HttpClientRibbonConfiguration.class }) public class ZuulProxyConfiguration extends ZuulConfiguration { @SuppressWarnings("rawtypes") @@ -89,135 +79,27 @@ public class ZuulProxyConfiguration extends ZuulConfiguration { @Override @ConditionalOnMissingBean(RouteLocator.class) public DiscoveryClientRouteLocator routeLocator() { - return new DiscoveryClientRouteLocator(this.server.getServletPrefix(), - this.discovery, this.zuulProperties, this.serviceRouteMapper); - } - - @Configuration - @ConditionalOnRibbonHttpClient - protected static class HttpClientRibbonConfiguration { - - @Autowired(required = false) - private Set zuulFallbackProviders = Collections.emptySet(); - - @Bean - @ConditionalOnMissingBean - public RibbonCommandFactory ribbonCommandFactory( - SpringClientFactory clientFactory, ZuulProperties zuulProperties) { - return new HttpClientRibbonCommandFactory(clientFactory, zuulProperties, zuulFallbackProviders); - } - } - - @Configuration - @ConditionalOnRibbonRestClient - protected static class RestClientRibbonConfiguration { - - @Autowired(required = false) - private Set zuulFallbackProviders = Collections.emptySet(); - - @Bean - @ConditionalOnMissingBean - public RibbonCommandFactory ribbonCommandFactory( - SpringClientFactory clientFactory, ZuulProperties zuulProperties) { - return new RestClientRibbonCommandFactory(clientFactory, zuulProperties, - zuulFallbackProviders); - } - } - - @Configuration - @ConditionalOnRibbonOkHttpClient - @ConditionalOnClass(name = "okhttp3.OkHttpClient") - protected static class OkHttpRibbonConfiguration { - - @Autowired(required = false) - private Set zuulFallbackProviders = Collections.emptySet(); - - @Bean - @ConditionalOnMissingBean - public RibbonCommandFactory ribbonCommandFactory( - SpringClientFactory clientFactory, ZuulProperties zuulProperties) { - return new OkHttpRibbonCommandFactory(clientFactory, zuulProperties, - zuulFallbackProviders); - } - } - - @Target({ ElementType.TYPE, ElementType.METHOD }) - @Retention(RetentionPolicy.RUNTIME) - @Documented - @Conditional(OnRibbonHttpClientCondition.class) - @interface ConditionalOnRibbonHttpClient { } - - private static class OnRibbonHttpClientCondition extends AnyNestedCondition { - public OnRibbonHttpClientCondition() { - super(ConfigurationPhase.REGISTER_BEAN); - } - - @Deprecated //remove in Edgware" - @ConditionalOnProperty(name = "zuul.ribbon.httpclient.enabled", matchIfMissing = true) - static class ZuulProperty {} - - @ConditionalOnProperty(name = "ribbon.httpclient.enabled", matchIfMissing = true) - static class RibbonProperty {} - } - - @Target({ ElementType.TYPE, ElementType.METHOD }) - @Retention(RetentionPolicy.RUNTIME) - @Documented - @Conditional(OnRibbonOkHttpClientCondition.class) - @interface ConditionalOnRibbonOkHttpClient { } - - private static class OnRibbonOkHttpClientCondition extends AnyNestedCondition { - public OnRibbonOkHttpClientCondition() { - super(ConfigurationPhase.REGISTER_BEAN); - } - - @Deprecated //remove in Edgware" - @ConditionalOnProperty("zuul.ribbon.okhttp.enabled") - static class ZuulProperty {} - - @ConditionalOnProperty("ribbon.okhttp.enabled") - static class RibbonProperty {} - } - - @Target({ ElementType.TYPE, ElementType.METHOD }) - @Retention(RetentionPolicy.RUNTIME) - @Documented - @Conditional(OnRibbonRestClientCondition.class) - @interface ConditionalOnRibbonRestClient { } - - private static class OnRibbonRestClientCondition extends AnyNestedCondition { - public OnRibbonRestClientCondition() { - super(ConfigurationPhase.REGISTER_BEAN); - } - - @Deprecated //remove in Edgware" - @ConditionalOnProperty("zuul.ribbon.restclient.enabled") - static class ZuulProperty {} - - @ConditionalOnProperty("ribbon.restclient.enabled") - static class RibbonProperty {} + return new DiscoveryClientRouteLocator(this.server.getServletPrefix(), this.discovery, this.zuulProperties, + this.serviceRouteMapper); } // pre filters @Bean - public PreDecorationFilter preDecorationFilter(RouteLocator routeLocator, - ProxyRequestHelper proxyRequestHelper) { - return new PreDecorationFilter(routeLocator, this.server.getServletPrefix(), - this.zuulProperties, proxyRequestHelper); + public PreDecorationFilter preDecorationFilter(RouteLocator routeLocator, ProxyRequestHelper proxyRequestHelper) { + return new PreDecorationFilter(routeLocator, this.server.getServletPrefix(), this.zuulProperties, + proxyRequestHelper); } // route filters @Bean public RibbonRoutingFilter ribbonRoutingFilter(ProxyRequestHelper helper, RibbonCommandFactory ribbonCommandFactory) { - RibbonRoutingFilter filter = new RibbonRoutingFilter(helper, ribbonCommandFactory, - this.requestCustomizers); + RibbonRoutingFilter filter = new RibbonRoutingFilter(helper, ribbonCommandFactory, this.requestCustomizers); return filter; } @Bean - public SimpleHostRoutingFilter simpleHostRoutingFilter(ProxyRequestHelper helper, - ZuulProperties zuulProperties) { + public SimpleHostRoutingFilter simpleHostRoutingFilter(ProxyRequestHelper helper, ZuulProperties zuulProperties) { return new SimpleHostRoutingFilter(helper, zuulProperties); } @@ -270,8 +152,7 @@ public class ZuulProxyConfiguration extends ZuulConfiguration { } } - private static class ZuulDiscoveryRefreshListener - implements ApplicationListener { + private static class ZuulDiscoveryRefreshListener implements ApplicationListener { private HeartbeatMonitor monitor = new HeartbeatMonitor(); From a38b7b71ac8be9608ac2530dac41cd6298d696cf Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Tue, 25 Oct 2016 12:56:49 +0100 Subject: [PATCH 09/14] Append to X-Forwarded-* headers instead of replacing them This fixes most of the issues people encounter when there are multiple proxies in the request. The tricky thing is that there is another header "Forwarded" that we don't recognize, but which backends probably do, at least some of the time (since it is from an actual RFC). The problem is that "Forwarded" does not contain the ports, so Spring UriComponentsBuilder cannot use it to rewrite links to a specific port. Since we do not support it already, this change doesn't make things any worse, but the corner case is there still. --- .../zuul/filters/pre/PreDecorationFilter.java | 30 +++++++++++++++++-- .../filters/pre/PreDecorationFilterTests.java | 17 +++++++++++ 2 files changed, 44 insertions(+), 3 deletions(-) diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilter.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilter.java index 1b5bd474..1f65218f 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilter.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilter.java @@ -163,15 +163,39 @@ public class PreDecorationFilter extends ZuulFilter { } private void addProxyHeaders(RequestContext ctx, Route route) { - String host = toHostHeader(ctx.getRequest()); - String port = String.valueOf(ctx.getRequest().getServerPort()); - String proto = ctx.getRequest().getScheme(); + HttpServletRequest request = ctx.getRequest(); + String host = toHostHeader(request); + String port = String.valueOf(request.getServerPort()); + String proto = request.getScheme(); + if (hasHeader(request, "X-Forwarded-Host")) { + host = request.getHeader("X-Forwarded-Host") + "," + host; + if (!hasHeader(request, "X-Forwarded-Port")) { + if (hasHeader(request, "X-Forwarded-Proto")) { + StringBuilder builder = new StringBuilder(); + for (String previous : StringUtils.commaDelimitedListToStringArray(request.getHeader("X-Forwarded-Proto"))) { + if (builder.length()>0) { + builder.append(","); + } + builder.append("https".equals(previous) ? "443" : "80"); + } + builder.append(",").append(port); + port = builder.toString(); + } + } else { + port = request.getHeader("X-Forwarded-Port") + "," + port; + } + proto = request.getHeader("X-Forwarded-Proto") + "," + proto; + } ctx.addZuulRequestHeader("X-Forwarded-Host", host); ctx.addZuulRequestHeader("X-Forwarded-Port", port); ctx.addZuulRequestHeader(ZuulHeaders.X_FORWARDED_PROTO, proto); addProxyPrefix(ctx, route); } + private boolean hasHeader(HttpServletRequest request, String name) { + return StringUtils.hasLength(request.getHeader(name)); + } + private void addProxyPrefix(RequestContext ctx, Route route) { String forwardedPrefix = ctx.getRequest().getHeader("X-Forwarded-Prefix"); String contextPath = ctx.getRequest().getContextPath(); diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilterTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilterTests.java index 940634ff..b07b43a2 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilterTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilterTests.java @@ -103,6 +103,23 @@ public class PreDecorationFilterTests { assertEquals("localhost:8080", ctx.getZuulRequestHeaders().get("x-forwarded-host")); } + @Test + public void xForwardedHostAppends() throws Exception { + this.properties.setPrefix("/api"); + this.request.setRequestURI("/api/foo/1"); + this.request.setRemoteAddr("5.6.7.8"); + this.request.setServerPort(8080); + this.request.addHeader("X-Forwarded-Host", "example.com"); + this.request.addHeader("X-Forwarded-Proto", "https"); + this.routeLocator.addRoute( + new ZuulRoute("foo", "/foo/**", "foo", null, false, null, null)); + this.filter.run(); + RequestContext ctx = RequestContext.getCurrentContext(); + assertEquals("example.com,localhost:8080", ctx.getZuulRequestHeaders().get("x-forwarded-host")); + assertEquals("443,8080", ctx.getZuulRequestHeaders().get("x-forwarded-port")); + assertEquals("https,http", ctx.getZuulRequestHeaders().get("x-forwarded-proto")); + } + @Test public void hostHeaderSet() throws Exception { this.properties.setPrefix("/api"); From 7f075d11ee56d57fa1a38a7d02555f9b8c3ed95e Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Tue, 25 Oct 2016 14:06:16 +0100 Subject: [PATCH 10/14] Allow Cors requests through to backend by default Instead of blocking them (which is the default behaviour of a Spring MVC controller) Cors requests will flow through the Zuul filters by default. Users can control it by grabbing the ZuulHandlerMapping in a @PostConstruct and injecting some CorsConfiguration via its setCorsConfigurations() method. --- .../netflix/zuul/web/ZuulController.java | 5 ++- .../netflix/zuul/web/ZuulHandlerMapping.java | 18 ++++++-- .../ServletPathZuulProxyApplicationTests.java | 45 ++++++++++++++++--- 3 files changed, 57 insertions(+), 11 deletions(-) diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/web/ZuulController.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/web/ZuulController.java index 8f5d4b1d..d8fe936d 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/web/ZuulController.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/web/ZuulController.java @@ -37,9 +37,10 @@ public class ZuulController extends ServletWrappingController { } @Override - protected ModelAndView handleRequestInternal(HttpServletRequest request, - HttpServletResponse response) throws Exception { + public ModelAndView handleRequest(HttpServletRequest request, HttpServletResponse response) throws Exception { try { + // We don't care about the other features of the base class, just want to + // handle the request return super.handleRequestInternal(request, response); } finally { diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/web/ZuulHandlerMapping.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/web/ZuulHandlerMapping.java index af81c9ae..c1e6dd54 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/web/ZuulHandlerMapping.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/web/ZuulHandlerMapping.java @@ -25,6 +25,8 @@ import org.springframework.cloud.netflix.zuul.filters.RefreshableRouteLocator; import org.springframework.cloud.netflix.zuul.filters.Route; import org.springframework.cloud.netflix.zuul.filters.RouteLocator; import org.springframework.util.PatternMatchUtils; +import org.springframework.web.cors.CorsConfiguration; +import org.springframework.web.servlet.HandlerExecutionChain; import org.springframework.web.servlet.handler.AbstractUrlHandlerMapping; import com.netflix.zuul.context.RequestContext; @@ -51,6 +53,16 @@ public class ZuulHandlerMapping extends AbstractUrlHandlerMapping { setOrder(-200); } + @Override + protected HandlerExecutionChain getCorsHandlerExecutionChain(HttpServletRequest request, + HandlerExecutionChain chain, CorsConfiguration config) { + if (config == null) { + // Allow CORS requests to go to the backend + return chain; + } + return super.getCorsHandlerExecutionChain(request, chain, config); + } + public void setErrorController(ErrorController errorController) { this.errorController = errorController; } @@ -63,10 +75,8 @@ public class ZuulHandlerMapping extends AbstractUrlHandlerMapping { } @Override - protected Object lookupHandler(String urlPath, HttpServletRequest request) - throws Exception { - if (this.errorController != null - && urlPath.equals(this.errorController.getErrorPath())) { + protected Object lookupHandler(String urlPath, HttpServletRequest request) throws Exception { + if (this.errorController != null && urlPath.equals(this.errorController.getErrorPath())) { return null; } String[] ignored = this.routeLocator.getIgnoredPaths().toArray(new String[0]); diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/ServletPathZuulProxyApplicationTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/ServletPathZuulProxyApplicationTests.java index 9b103cce..eb14d911 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/ServletPathZuulProxyApplicationTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/ServletPathZuulProxyApplicationTests.java @@ -18,6 +18,9 @@ package org.springframework.cloud.netflix.zuul; import static org.junit.Assert.assertEquals; +import java.net.URI; +import java.net.URISyntaxException; + import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -33,13 +36,16 @@ import org.springframework.context.annotation.Configuration; import org.springframework.http.HttpEntity; import org.springframework.http.HttpMethod; import org.springframework.http.HttpStatus; +import org.springframework.http.RequestEntity; import org.springframework.http.ResponseEntity; import org.springframework.test.annotation.DirtiesContext; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; +import org.springframework.web.bind.annotation.CrossOrigin; 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.RestController; +import org.springframework.web.client.RestClientException; import com.netflix.zuul.context.RequestContext; @@ -68,9 +74,38 @@ public class ServletPathZuulProxyApplicationTests { public void getOnSelfViaSimpleHostRoutingFilter() { this.routes.addRoute("/self/**", "http://localhost:" + this.port + "/app/local"); this.endpoint.reset(); + ResponseEntity result = new TestRestTemplate().exchange("http://localhost:" + this.port + "/app/self/1", + HttpMethod.GET, new HttpEntity<>((Void) null), String.class); + assertEquals(HttpStatus.OK, result.getStatusCode()); + assertEquals("Gotten 1!", result.getBody()); + } + + @Test + public void optionsOnRawEndpoint() throws Exception { + ResponseEntity result = new TestRestTemplate().exchange(RequestEntity + .options(new URI("http://localhost:" + this.port + "/app/local/1")) + .header("Origin", "http://localhost:9000").header("Access-Control-Request-Method", "GET").build(), + String.class); + assertEquals(HttpStatus.OK, result.getStatusCode()); + assertEquals("http://localhost:9000", result.getHeaders().getFirst("Access-Control-Allow-Origin")); + } + + @Test + public void optionsOnSelf() throws Exception { + this.routes.addRoute("/self/**", "http://localhost:" + this.port + "/app/local"); + this.endpoint.reset(); + ResponseEntity result = new TestRestTemplate().exchange(RequestEntity + .options(new URI("http://localhost:" + this.port + "/app/self/1")) + .header("Origin", "http://localhost:9000").header("Access-Control-Request-Method", "GET").build(), + String.class); + assertEquals(HttpStatus.OK, result.getStatusCode()); + assertEquals("http://localhost:9000", result.getHeaders().getFirst("Access-Control-Allow-Origin")); + } + + @Test + public void contentOnRawEndpoint() throws Exception { ResponseEntity result = new TestRestTemplate().exchange( - "http://localhost:" + this.port + "/app/self/1", HttpMethod.GET, - new HttpEntity<>((Void) null), String.class); + RequestEntity.get(new URI("http://localhost:" + this.port + "/app/local/1")).build(), String.class); assertEquals(HttpStatus.OK, result.getStatusCode()); assertEquals("Gotten 1!", result.getBody()); } @@ -80,9 +115,8 @@ public class ServletPathZuulProxyApplicationTests { this.routes.addRoute(new ZuulRoute("strip", "/strip/**", "strip", "http://localhost:" + this.port + "/app/local", false, false, null)); this.endpoint.reset(); - ResponseEntity result = new TestRestTemplate().exchange( - "http://localhost:" + this.port + "/app/strip", HttpMethod.GET, - new HttpEntity<>((Void) null), String.class); + ResponseEntity result = new TestRestTemplate().exchange("http://localhost:" + this.port + "/app/strip", + HttpMethod.GET, new HttpEntity<>((Void) null), String.class); assertEquals(HttpStatus.OK, result.getStatusCode()); // Prefix not stripped to it goes to /local/strip assertEquals("Gotten strip!", result.getBody()); @@ -96,6 +130,7 @@ public class ServletPathZuulProxyApplicationTests { static class ServletPathZuulProxyApplication { @RequestMapping(value = "/local/{id}", method = RequestMethod.GET) + @CrossOrigin(origins = "*") public String get(@PathVariable String id) { return "Gotten " + id + "!"; } From 2dfee97563b4a0658507add6984c152f5e2a7098 Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Tue, 25 Oct 2016 12:56:08 -0400 Subject: [PATCH 11/14] Resolve spring.application.name value via a property resolver instead of @Value. Fixes #1398 --- .../eureka/EurekaClientAutoConfiguration.java | 8 +++++ .../eureka/EurekaInstanceConfigBean.java | 33 +++++++++---------- .../eureka/EurekaInstanceConfigBeanTests.java | 25 ++++++++++++-- .../netflix/sidecar/SidecarConfiguration.java | 13 ++++++-- 4 files changed, 57 insertions(+), 22 deletions(-) diff --git a/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/EurekaClientAutoConfiguration.java b/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/EurekaClientAutoConfiguration.java index 9f6e5c26..0d678445 100644 --- a/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/EurekaClientAutoConfiguration.java +++ b/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/EurekaClientAutoConfiguration.java @@ -64,6 +64,7 @@ import static org.springframework.cloud.commons.util.IdUtils.getDefaultInstanceI * @author Spencer Gibb * @author Jon Schneider * @author Matt Jenkins + * @author Ryan Baxter */ @Configuration @EnableConfigurationProperties @@ -107,9 +108,16 @@ public class EurekaClientAutoConfiguration { @ConditionalOnMissingBean(value = EurekaInstanceConfig.class, search = SearchStrategy.CURRENT) public EurekaInstanceConfigBean eurekaInstanceConfigBean(InetUtils inetUtils) { RelaxedPropertyResolver relaxedPropertyResolver = new RelaxedPropertyResolver(env, "eureka.instance."); + RelaxedPropertyResolver springPropertyResolver = new RelaxedPropertyResolver(env, "spring.application."); + String springAppName = springPropertyResolver.getProperty("name"); EurekaInstanceConfigBean instance = new EurekaInstanceConfigBean(inetUtils); instance.setNonSecurePort(this.nonSecurePort); instance.setInstanceId(getDefaultInstanceId(this.env)); + if(springAppName != null) { + instance.setAppname(springAppName); + instance.setVirtualHostName(springAppName); + instance.setSecureVirtualHostName(springAppName); + } if (this.managementPort != this.nonSecurePort && this.managementPort != 0) { if (StringUtils.hasText(this.hostname)) { instance.setHostname(this.hostname); diff --git a/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/EurekaInstanceConfigBean.java b/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/EurekaInstanceConfigBean.java index cf4cc185..dbbb0726 100644 --- a/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/EurekaInstanceConfigBean.java +++ b/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/EurekaInstanceConfigBean.java @@ -16,31 +16,31 @@ package org.springframework.cloud.netflix.eureka; -import java.util.HashMap; -import java.util.Map; - -import org.springframework.beans.factory.annotation.Value; -import org.springframework.boot.context.properties.ConfigurationProperties; -import org.springframework.cloud.commons.util.InetUtils; -import org.springframework.cloud.commons.util.InetUtils.HostInfo; - -import com.netflix.appinfo.DataCenterInfo; -import com.netflix.appinfo.InstanceInfo.InstanceStatus; -import com.netflix.appinfo.MyDataCenterInfo; - import lombok.AccessLevel; import lombok.Data; import lombok.Getter; import lombok.Setter; +import java.util.HashMap; +import java.util.Map; +import org.springframework.boot.context.properties.ConfigurationProperties; +import org.springframework.cloud.commons.util.InetUtils; +import org.springframework.cloud.commons.util.InetUtils.HostInfo; +import com.netflix.appinfo.DataCenterInfo; +import com.netflix.appinfo.InstanceInfo.InstanceStatus; +import com.netflix.appinfo.MyDataCenterInfo; + /** * @author Dave Syer * @author Spencer Gibb + * @author Ryan Baxter */ @Data @ConfigurationProperties("eureka.instance") public class EurekaInstanceConfigBean implements CloudEurekaInstanceConfig { + private static final String UNKNOWN = "unknown"; + @Getter(AccessLevel.PRIVATE) @Setter(AccessLevel.PRIVATE) private HostInfo hostInfo; @@ -52,8 +52,7 @@ public class EurekaInstanceConfigBean implements CloudEurekaInstanceConfig { /** * Get the name of the application to be registered with eureka. */ - @Value("${spring.application.name:unknown}") - private String appname = "unknown"; + private String appname = UNKNOWN; /** * Get the name of the application group to be registered with eureka. @@ -119,8 +118,7 @@ public class EurekaInstanceConfigBean implements CloudEurekaInstanceConfig { * virtual host name.Think of this as similar to the fully qualified domain name, that * the users of your services will need to find this instance. */ - @Value("${spring.application.name:unknown}") - private String virtualHostName; + private String virtualHostName = UNKNOWN; /** * Get the unique Id (within the scope of the appName) of this instance to be @@ -135,8 +133,7 @@ public class EurekaInstanceConfigBean implements CloudEurekaInstanceConfig { * secure virtual host name.Think of this as similar to the fully qualified domain * name, that the users of your services will need to find this instance. */ - @Value("${spring.application.name:unknown}") - private String secureVirtualHostName; + private String secureVirtualHostName = UNKNOWN; /** * Gets the AWS autoscaling group name associated with this instance. This information diff --git a/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/EurekaInstanceConfigBeanTests.java b/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/EurekaInstanceConfigBeanTests.java index 0f3f99b6..bb745e2b 100644 --- a/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/EurekaInstanceConfigBeanTests.java +++ b/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/EurekaInstanceConfigBeanTests.java @@ -20,15 +20,17 @@ import org.junit.After; import org.junit.Before; import org.junit.Test; import org.springframework.beans.factory.BeanCreationException; +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.autoconfigure.PropertyPlaceholderAutoConfiguration; +import org.springframework.boot.bind.RelaxedPropertyResolver; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.cloud.commons.util.InetUtils; import org.springframework.cloud.commons.util.InetUtilsProperties; import org.springframework.context.annotation.AnnotationConfigApplicationContext; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.core.env.ConfigurableEnvironment; import org.springframework.test.util.ReflectionTestUtils; - import com.netflix.appinfo.InstanceInfo.InstanceStatus; import static org.junit.Assert.assertEquals; @@ -38,6 +40,7 @@ import static org.springframework.boot.test.util.EnvironmentTestUtils.addEnviron /** * @author Dave Syer * @author Spencer Gibb + * @author Ryan Baxter */ public class EurekaInstanceConfigBeanTests { @@ -185,6 +188,14 @@ public class EurekaInstanceConfigBeanTests { } + @Test + public void testDefaultAppName() throws Exception { + setupContext(); + assertEquals("default app name is wrong", "unknown", getInstanceConfig().getAppname()); + assertEquals("default virtual hostname is wrong", "unknown", getInstanceConfig().getVirtualHostName()); + assertEquals("default secure virtual hostname is wrong", "unknown", getInstanceConfig().getSecureVirtualHostName()); + } + private void setupContext() { this.context.register(PropertyPlaceholderAutoConfiguration.class, TestConfiguration.class); @@ -198,9 +209,19 @@ public class EurekaInstanceConfigBeanTests { @Configuration @EnableConfigurationProperties protected static class TestConfiguration { + @Autowired + ConfigurableEnvironment env; @Bean public EurekaInstanceConfigBean eurekaInstanceConfigBean() { - return new EurekaInstanceConfigBean(new InetUtils(new InetUtilsProperties())); + EurekaInstanceConfigBean configBean = new EurekaInstanceConfigBean(new InetUtils(new InetUtilsProperties())); + RelaxedPropertyResolver springPropertyResolver = new RelaxedPropertyResolver(env, "spring.application."); + String springAppName = springPropertyResolver.getProperty("name"); + if(springAppName != null) { + configBean.setSecureVirtualHostName(springAppName); + configBean.setVirtualHostName(springAppName); + configBean.setAppname(springAppName); + } + return configBean; } } diff --git a/spring-cloud-netflix-sidecar/src/main/java/org/springframework/cloud/netflix/sidecar/SidecarConfiguration.java b/spring-cloud-netflix-sidecar/src/main/java/org/springframework/cloud/netflix/sidecar/SidecarConfiguration.java index 52357293..17db83f5 100644 --- a/spring-cloud-netflix-sidecar/src/main/java/org/springframework/cloud/netflix/sidecar/SidecarConfiguration.java +++ b/spring-cloud-netflix-sidecar/src/main/java/org/springframework/cloud/netflix/sidecar/SidecarConfiguration.java @@ -16,10 +16,13 @@ package org.springframework.cloud.netflix.sidecar; +import static org.springframework.cloud.commons.util.IdUtils.getDefaultInstanceId; + import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.boot.bind.RelaxedPropertyResolver; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.cloud.client.actuator.HasFeatures; import org.springframework.cloud.commons.util.InetUtils; @@ -32,10 +35,9 @@ import org.springframework.util.StringUtils; import com.netflix.appinfo.HealthCheckHandler; import com.netflix.discovery.EurekaClientConfig; -import static org.springframework.cloud.commons.util.IdUtils.getDefaultInstanceId; - /** * @author Spencer Gibb + * @author Ryan Baxter */ @Configuration @EnableConfigurationProperties @@ -73,9 +75,16 @@ public class SidecarConfiguration { @Bean public EurekaInstanceConfigBean eurekaInstanceConfigBean() { EurekaInstanceConfigBean config = new EurekaInstanceConfigBean(inetUtils); + RelaxedPropertyResolver springPropertyResolver = new RelaxedPropertyResolver(env, "spring.application."); + String springAppName = springPropertyResolver.getProperty("name"); int port = this.sidecarProperties.getPort(); config.setNonSecurePort(port); config.setInstanceId(getDefaultInstanceId(this.env)); + if(StringUtils.hasText(springAppName)) { + config.setAppname(springAppName); + config.setVirtualHostName(springAppName); + config.setSecureVirtualHostName(springAppName); + } if (StringUtils.hasText(this.hostname)) { config.setHostname(this.hostname); } From 81959b5ef3e0ec7738a79fdef923caa8e31a51cc Mon Sep 17 00:00:00 2001 From: Spencer Gibb Date: Tue, 25 Oct 2016 11:24:42 -0600 Subject: [PATCH 12/14] Use new FilterRegistrationBean package. Breaks Spring Boot 1.3 compatibility, enables 1.5 --- .../cloud/netflix/eureka/server/EurekaServerConfiguration.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/spring-cloud-netflix-eureka-server/src/main/java/org/springframework/cloud/netflix/eureka/server/EurekaServerConfiguration.java b/spring-cloud-netflix-eureka-server/src/main/java/org/springframework/cloud/netflix/eureka/server/EurekaServerConfiguration.java index e5fae161..73d4571f 100644 --- a/spring-cloud-netflix-eureka-server/src/main/java/org/springframework/cloud/netflix/eureka/server/EurekaServerConfiguration.java +++ b/spring-cloud-netflix-eureka-server/src/main/java/org/springframework/cloud/netflix/eureka/server/EurekaServerConfiguration.java @@ -28,12 +28,11 @@ import javax.ws.rs.ext.Provider; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Qualifier; -import org.springframework.beans.factory.annotation.Value; import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; -import org.springframework.boot.context.embedded.FilterRegistrationBean; import org.springframework.boot.context.properties.EnableConfigurationProperties; +import org.springframework.boot.web.servlet.FilterRegistrationBean; import org.springframework.cloud.client.actuator.HasFeatures; import org.springframework.cloud.client.discovery.EnableDiscoveryClient; import org.springframework.cloud.netflix.eureka.EurekaConstants; From 85b2fe37e235013df7d5fa052aacf37f9374ced3 Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Tue, 25 Oct 2016 13:54:29 -0400 Subject: [PATCH 13/14] Added some additional tests --- .../eureka/EurekaClientAutoConfiguration.java | 2 +- .../EurekaClientAutoConfigurationTests.java | 17 +++++++++++++++++ .../eureka/EurekaInstanceConfigBeanTests.java | 3 ++- 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/EurekaClientAutoConfiguration.java b/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/EurekaClientAutoConfiguration.java index 0d678445..46b0f1fd 100644 --- a/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/EurekaClientAutoConfiguration.java +++ b/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/EurekaClientAutoConfiguration.java @@ -113,7 +113,7 @@ public class EurekaClientAutoConfiguration { EurekaInstanceConfigBean instance = new EurekaInstanceConfigBean(inetUtils); instance.setNonSecurePort(this.nonSecurePort); instance.setInstanceId(getDefaultInstanceId(this.env)); - if(springAppName != null) { + if(StringUtils.hasText(springAppName)) { instance.setAppname(springAppName); instance.setVirtualHostName(springAppName); instance.setSecureVirtualHostName(springAppName); diff --git a/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/EurekaClientAutoConfigurationTests.java b/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/EurekaClientAutoConfigurationTests.java index c1a15915..5d97b598 100644 --- a/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/EurekaClientAutoConfigurationTests.java +++ b/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/EurekaClientAutoConfigurationTests.java @@ -189,6 +189,23 @@ public class EurekaClientAutoConfigurationTests { // Mockito.verify(http).addFilter(Matchers.any(HTTPBasicAuthFilter.class)); } + @Test + public void testDefaultAppName() throws Exception { + setupContext(); + assertEquals("unknown", getInstanceConfig().getAppname()); + assertEquals("unknown", getInstanceConfig().getVirtualHostName()); + assertEquals("unknown", getInstanceConfig().getSecureVirtualHostName()); + } + + @Test + public void testAppName() throws Exception { + EnvironmentTestUtils.addEnvironment(this.context, "spring.application.name=mytest"); + setupContext(); + assertEquals("mytest", getInstanceConfig().getAppname()); + assertEquals("mytest", getInstanceConfig().getVirtualHostName()); + assertEquals("mytest", getInstanceConfig().getSecureVirtualHostName()); + } + private void testNonSecurePort(String propName) { addEnvironment(this.context, propName + ":8888"); setupContext(); diff --git a/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/EurekaInstanceConfigBeanTests.java b/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/EurekaInstanceConfigBeanTests.java index bb745e2b..284388dd 100644 --- a/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/EurekaInstanceConfigBeanTests.java +++ b/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/EurekaInstanceConfigBeanTests.java @@ -31,6 +31,7 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.core.env.ConfigurableEnvironment; import org.springframework.test.util.ReflectionTestUtils; +import org.springframework.util.StringUtils; import com.netflix.appinfo.InstanceInfo.InstanceStatus; import static org.junit.Assert.assertEquals; @@ -216,7 +217,7 @@ public class EurekaInstanceConfigBeanTests { EurekaInstanceConfigBean configBean = new EurekaInstanceConfigBean(new InetUtils(new InetUtilsProperties())); RelaxedPropertyResolver springPropertyResolver = new RelaxedPropertyResolver(env, "spring.application."); String springAppName = springPropertyResolver.getProperty("name"); - if(springAppName != null) { + if(StringUtils.hasText(springAppName)) { configBean.setSecureVirtualHostName(springAppName); configBean.setVirtualHostName(springAppName); configBean.setAppname(springAppName); From aef753a3a09d48086dc1391eb4c51a2a74b2b15a Mon Sep 17 00:00:00 2001 From: Venil Noronha Date: Tue, 25 Oct 2016 14:39:22 -0700 Subject: [PATCH 14/14] Adds Feign Logger factory interface (#1411) Fixes gh-1363 --- .../feign/DefaultFeignLoggerFactory.java | 38 ++++++ .../netflix/feign/FeignClientFactoryBean.java | 8 +- .../feign/FeignClientsConfiguration.java | 14 +- .../netflix/feign/FeignLoggerFactory.java | 36 +++++ .../feign/FeignLoggerFactoryTests.java | 126 ++++++++++++++++++ 5 files changed, 213 insertions(+), 9 deletions(-) create mode 100644 spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/DefaultFeignLoggerFactory.java create mode 100644 spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignLoggerFactory.java create mode 100644 spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/FeignLoggerFactoryTests.java diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/DefaultFeignLoggerFactory.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/DefaultFeignLoggerFactory.java new file mode 100644 index 00000000..d7ca25d1 --- /dev/null +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/DefaultFeignLoggerFactory.java @@ -0,0 +1,38 @@ +/* + * Copyright 2016 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.netflix.feign; + +import feign.Logger; +import feign.slf4j.Slf4jLogger; + +/** + * @author Venil Noronha + */ +public class DefaultFeignLoggerFactory implements FeignLoggerFactory { + + private Logger logger; + + public DefaultFeignLoggerFactory(Logger logger) { + this.logger = logger; + } + + @Override + public Logger create(Class type) { + return this.logger != null ? this.logger : new Slf4jLogger(type); + } + +} diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientFactoryBean.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientFactoryBean.java index e5fcffc7..bf21845f 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientFactoryBean.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientFactoryBean.java @@ -39,7 +39,6 @@ import feign.Target.HardCodedTarget; import feign.codec.Decoder; import feign.codec.Encoder; import feign.codec.ErrorDecoder; -import feign.slf4j.Slf4jLogger; import lombok.Data; import lombok.EqualsAndHashCode; @@ -83,11 +82,8 @@ class FeignClientFactoryBean implements FactoryBean, InitializingBean, } protected Feign.Builder feign(FeignContext context) { - Logger logger = getOptional(context, Logger.class); - - if (logger == null) { - logger = new Slf4jLogger(this.type); - } + FeignLoggerFactory loggerFactory = get(context, FeignLoggerFactory.class); + Logger logger = loggerFactory.create(this.type); // @formatter:off Feign.Builder builder = get(context, Feign.Builder.class) diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientsConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientsConfiguration.java index ee4cf568..606cbbb5 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientsConfiguration.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientsConfiguration.java @@ -19,7 +19,6 @@ package org.springframework.cloud.netflix.feign; import java.util.ArrayList; import java.util.List; -import org.apache.http.client.HttpClient; import org.springframework.beans.factory.ObjectFactory; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; @@ -39,16 +38,16 @@ import org.springframework.format.support.FormattingConversionService; import com.netflix.hystrix.HystrixCommand; -import feign.Client; import feign.Contract; import feign.Feign; +import feign.Logger; import feign.codec.Decoder; import feign.codec.Encoder; -import feign.httpclient.ApacheHttpClient; import feign.hystrix.HystrixFeign; /** * @author Dave Syer + * @author Venil Noronha */ @Configuration public class FeignClientsConfiguration { @@ -62,6 +61,9 @@ public class FeignClientsConfiguration { @Autowired(required = false) private List feignFormatterRegistrars = new ArrayList<>(); + @Autowired(required = false) + private Logger logger; + @Bean @ConditionalOnMissingBean public Decoder feignDecoder() { @@ -108,4 +110,10 @@ public class FeignClientsConfiguration { return Feign.builder(); } + @Bean + @ConditionalOnMissingBean(FeignLoggerFactory.class) + public FeignLoggerFactory feignLoggerFactory() { + return new DefaultFeignLoggerFactory(logger); + } + } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignLoggerFactory.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignLoggerFactory.java new file mode 100644 index 00000000..9440fdf0 --- /dev/null +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignLoggerFactory.java @@ -0,0 +1,36 @@ +/* + * Copyright 2016 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.netflix.feign; + +import feign.Logger; + +/** + * Allows an application to use a custom Feign {@link Logger}. + * + * @author Venil Noronha + */ +public interface FeignLoggerFactory { + + /** + * Factory method to provide a {@link Logger} for a given {@link Class}. + * + * @param type the {@link Class} for which a {@link Logger} instance is to be created + * @return a {@link Logger} instance + */ + public Logger create(Class type); + +} diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/FeignLoggerFactoryTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/FeignLoggerFactoryTests.java new file mode 100644 index 00000000..c773ab4d --- /dev/null +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/FeignLoggerFactoryTests.java @@ -0,0 +1,126 @@ +/* + * Copyright 2016 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.netflix.feign; + +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; + +import org.junit.Test; + +import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Import; + +import feign.Logger; +import feign.slf4j.Slf4jLogger; + +/** + * @author Venil Noronha + */ +public class FeignLoggerFactoryTests { + + @Test + public void testDefaultLogger() { + AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(SampleConfiguration1.class); + FeignLoggerFactory loggerFactory = context.getBean(FeignLoggerFactory.class); + assertNotNull(loggerFactory); + Logger logger = loggerFactory.create(Object.class); + assertNotNull(logger); + assertTrue(logger instanceof Slf4jLogger); + context.close(); + } + + @Configuration + @Import(FeignClientsConfiguration.class) + protected static class SampleConfiguration1 { + + } + + @Test + public void testCustomLogger() { + AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(SampleConfiguration2.class); + FeignLoggerFactory loggerFactory = context.getBean(FeignLoggerFactory.class); + assertNotNull(loggerFactory); + Logger logger = loggerFactory.create(Object.class); + assertNotNull(logger); + assertTrue(logger instanceof LoggerImpl1); + context.close(); + } + + @Configuration + @Import(FeignClientsConfiguration.class) + protected static class SampleConfiguration2 { + + @Bean + public Logger logger() { + return new LoggerImpl1(); + } + + } + + static class LoggerImpl1 extends Logger { + + @Override + protected void log(String arg0, String arg1, Object... arg2) { + // noop + } + + } + + @Test + public void testCustomLoggerFactory() { + AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(SampleConfiguration3.class); + FeignLoggerFactory loggerFactory = context.getBean(FeignLoggerFactory.class); + assertNotNull(loggerFactory); + assertTrue(loggerFactory instanceof LoggerFactoryImpl); + Logger logger = loggerFactory.create(Object.class); + assertNotNull(logger); + assertTrue(logger instanceof LoggerImpl2); + context.close(); + } + + @Configuration + @Import(FeignClientsConfiguration.class) + protected static class SampleConfiguration3 { + + @Bean + public FeignLoggerFactory feignLoggerFactory() { + return new LoggerFactoryImpl(); + } + + } + + static class LoggerFactoryImpl implements FeignLoggerFactory { + + @Override + public Logger create(Class type) { + return new LoggerImpl2(); + } + + } + + static class LoggerImpl2 extends Logger { + + @Override + protected void log(String arg0, String arg1, Object... arg2) { + // noop + } + + } + +}