diff --git a/docs/src/main/asciidoc/spring-cloud-netflix.adoc b/docs/src/main/asciidoc/spring-cloud-netflix.adoc index 91dabcb4..61adc264 100644 --- a/docs/src/main/asciidoc/spring-cloud-netflix.adoc +++ b/docs/src/main/asciidoc/spring-cloud-netflix.adoc @@ -886,6 +886,28 @@ ribbon: clients: client1, client2, client3 ---- +[[how-to-configure-hystrix-thread-pools]] +=== How to Configure Hystrix thread pools +If you change `zuul.ribbonIsolationStrategy` to THREAD, the thread isolation strategy for Hystrix will be used for all routes. In this case, the HystrixThreadPoolKey is set to "RibbonCommand" as default. It means that HystrixCommands for all routes will be executed in the same Hystrix thread pool. This behavior can be changed using the following configuration and it will result in HystrixCommands being executed in the Hystrix thread pool for each route. + +.application.yml +---- +zuul: + threadPool: + useSeparateThreadPools: true +---- + +The default HystrixThreadPoolKey in this case is same with service ID for each route. To add a prefix to HystrixThreadPoolKey, set `zuul.threadPool.threadPoolKeyPrefix` to a value that you want to add. For example: + +.application.yml +---- +zuul: + threadPool: + useSeparateThreadPools: true + threadPoolKeyPrefix: zuulgw +---- + + [[spring-cloud-feign]] == Declarative REST Client: Feign @@ -1020,8 +1042,46 @@ public class FooConfiguration { This replaces the `SpringMvcContract` with `feign.Contract.Default` and adds a `RequestInterceptor` to the collection of `RequestInterceptor`. +`@FeignClient` also can be configured using configuration properties. + +application.yml +[source,yaml] +---- +feign: + client: + config: + feignName: + connectTimeout: 5000 + readTimeout: 5000 + loggerLevel: full + errorDecoder: com.example.SimpleErrorDecoder + retryer: com.example.SimpleRetryer + requestInterceptors: + - com.example.FooRequestInterceptor + - com.example.BarRequestInterceptor + decode404: false +---- + 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. +If you prefer using configuration properties to configured all `@FeignClient`, you can create configuration properties with `default` feign name. + +application.yml +[source,yaml] +---- +feign: + client: + config: + default: + connectTimeout: 5000 + readTimeout: 5000 + loggerLevel: basic +---- + +If we create both `@Configuration` bean and configuration properties, configuration properties will win. +It will override `@Configuration` values. But if you want to change the priority to `@Configuration`, +you can change `feign.client.default-to-properties` to `false`. + NOTE: If you need to use `ThreadLocal` bound variables in your `RequestInterceptor`s you will need to either set the thread isolation strategy for Hystrix to `SEMAPHORE` or disable Hystrix in Feign. @@ -1842,6 +1902,32 @@ class MyFallbackProvider implements ZuulFallbackProvider { } ---- +[[zuul-redirect-location-rewrite]] +=== Rewriting `Location` header + +If Zuul is fronting a web application then there may be a need to re-write the `Location` header when the web application redirects through a http status code of 3XX, otherwise the browser will end up redirecting to the web application's url instead of the Zuul url. +A `LocationRewriteFilter` Zuul filter can be configured to re-write the Location header to the Zuul's url, it also adds back the stripped global and route specific prefixes. The filter can be added the following way via a Spring Configuration file: + +[source,java] +---- +import org.springframework.cloud.netflix.zuul.filters.post.LocationRewriteFilter; +... + +@Configuration +@EnableZuulProxy +public class ZuulConfig { + @Bean + public LocationRewriteFilter locationRewriteFilter() { + return new LocationRewriteFilter(); + } +} +---- + +[WARNING] +==== +Use this filter with caution though, the filter acts on the `Location` header of ALL 3XX response codes which may not be appropriate in all scenarios, say if the user is redirecting to an external URL. +==== + [[zuul-developer-guide]] === Zuul Developer Guide diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignAutoConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignAutoConfiguration.java index c643df8a..15facdff 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignAutoConfiguration.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignAutoConfiguration.java @@ -25,6 +25,7 @@ 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.boot.context.properties.EnableConfigurationProperties; import org.springframework.cloud.client.actuator.HasFeatures; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -40,6 +41,7 @@ import feign.okhttp.OkHttpClient; */ @Configuration @ConditionalOnClass(Feign.class) +@EnableConfigurationProperties({FeignClientProperties.class}) public class FeignAutoConfiguration { @Autowired(required = false) 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 48370e41..2c61e8dd 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 @@ -1,5 +1,5 @@ /* - * Copyright 2013-2016 the original author or authors. + * Copyright 2013-2017 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. @@ -18,9 +18,11 @@ package org.springframework.cloud.netflix.feign; import java.util.Map; +import org.springframework.beans.BeanUtils; import org.springframework.beans.BeansException; import org.springframework.beans.factory.FactoryBean; import org.springframework.beans.factory.InitializingBean; +import org.springframework.beans.factory.NoSuchBeanDefinitionException; import org.springframework.cloud.netflix.feign.ribbon.LoadBalancerFeignClient; import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContextAware; @@ -44,6 +46,7 @@ import lombok.EqualsAndHashCode; /** * @author Spencer Gibb * @author Venil Noronha + * @author Eko Kurniawan Khannedy */ @Data @EqualsAndHashCode(callSuper = false) @@ -74,7 +77,6 @@ class FeignClientFactoryBean implements FactoryBean, InitializingBean, Assert.hasText(this.name, "Name must be set"); } - @Override public void setApplicationContext(ApplicationContext context) throws BeansException { this.applicationContext = context; @@ -93,7 +95,29 @@ class FeignClientFactoryBean implements FactoryBean, InitializingBean, .contract(get(context, Contract.class)); // @formatter:on - // optional values + configureFeign(context, builder); + + return builder; + } + + protected void configureFeign(FeignContext context, Feign.Builder builder) { + FeignClientProperties properties = applicationContext.getBean(FeignClientProperties.class); + if (properties != null) { + if (properties.isDefaultToProperties()) { + configureUsingConfiguration(context, builder); + configureUsingProperties(properties.getConfig().get(properties.getDefaultConfig()), builder); + configureUsingProperties(properties.getConfig().get(this.name), builder); + } else { + configureUsingProperties(properties.getConfig().get(properties.getDefaultConfig()), builder); + configureUsingProperties(properties.getConfig().get(this.name), builder); + configureUsingConfiguration(context, builder); + } + } else { + configureUsingConfiguration(context, builder); + } + } + + protected void configureUsingConfiguration(FeignContext context, Feign.Builder builder) { Logger.Level level = getOptional(context, Logger.Level.class); if (level != null) { builder.logLevel(level); @@ -119,8 +143,52 @@ class FeignClientFactoryBean implements FactoryBean, InitializingBean, if (decode404) { builder.decode404(); } + } - return builder; + protected void configureUsingProperties(FeignClientProperties.FeignClientConfiguration config, Feign.Builder builder) { + if (config == null) { + return; + } + + if (config.getLoggerLevel() != null) { + builder.logLevel(config.getLoggerLevel()); + } + + if (config.getConnectTimeout() != null && config.getReadTimeout() != null) { + builder.options(new Request.Options(config.getConnectTimeout(), config.getReadTimeout())); + } + + if (config.getRetryer() != null) { + Retryer retryer = getOrInstantiate(config.getRetryer()); + builder.retryer(retryer); + } + + if (config.getErrorDecoder() != null) { + ErrorDecoder errorDecoder = getOrInstantiate(config.getErrorDecoder()); + builder.errorDecoder(errorDecoder); + } + + if (config.getRequestInterceptors() != null && !config.getRequestInterceptors().isEmpty()) { + // this will add request interceptor to builder, not replace existing + for (Class bean : config.getRequestInterceptors()) { + RequestInterceptor interceptor = getOrInstantiate(bean); + builder.requestInterceptor(interceptor); + } + } + + if (config.getDecode404() != null) { + if (config.getDecode404()) { + builder.decode404(); + } + } + } + + private T getOrInstantiate(Class tClass) { + try { + return applicationContext.getBean(tClass); + } catch (NoSuchBeanDefinitionException e) { + return BeanUtils.instantiateClass(tClass); + } } protected T get(FeignContext context, Class type) { diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientProperties.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientProperties.java new file mode 100644 index 00000000..bdb51958 --- /dev/null +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientProperties.java @@ -0,0 +1,174 @@ +/* + * Copyright 2013-2017 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.RequestInterceptor; +import feign.Retryer; +import feign.codec.ErrorDecoder; +import lombok.Data; +import org.springframework.boot.context.properties.ConfigurationProperties; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; + +/** + * @author Eko Kurniawan Khannedy + */ +@ConfigurationProperties("feign.client") +public class FeignClientProperties { + + private boolean defaultToProperties = true; + + private String defaultConfig = "default"; + + private Map config = new HashMap<>(); + + public boolean isDefaultToProperties() { + return defaultToProperties; + } + + public void setDefaultToProperties(boolean defaultToProperties) { + this.defaultToProperties = defaultToProperties; + } + + public String getDefaultConfig() { + return defaultConfig; + } + + public void setDefaultConfig(String defaultConfig) { + this.defaultConfig = defaultConfig; + } + + public Map getConfig() { + return config; + } + + public void setConfig(Map config) { + this.config = config; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + FeignClientProperties that = (FeignClientProperties) o; + return defaultToProperties == that.defaultToProperties && + Objects.equals(defaultConfig, that.defaultConfig) && + Objects.equals(config, that.config); + } + + @Override + public int hashCode() { + return Objects.hash(defaultToProperties, defaultConfig, config); + } + + public static class FeignClientConfiguration { + + private Logger.Level loggerLevel; + + private Integer connectTimeout; + + private Integer readTimeout; + + private Class retryer; + + private Class errorDecoder; + + private List> requestInterceptors; + + private Boolean decode404; + + public Logger.Level getLoggerLevel() { + return loggerLevel; + } + + public void setLoggerLevel(Logger.Level loggerLevel) { + this.loggerLevel = loggerLevel; + } + + public Integer getConnectTimeout() { + return connectTimeout; + } + + public void setConnectTimeout(Integer connectTimeout) { + this.connectTimeout = connectTimeout; + } + + public Integer getReadTimeout() { + return readTimeout; + } + + public void setReadTimeout(Integer readTimeout) { + this.readTimeout = readTimeout; + } + + public Class getRetryer() { + return retryer; + } + + public void setRetryer(Class retryer) { + this.retryer = retryer; + } + + public Class getErrorDecoder() { + return errorDecoder; + } + + public void setErrorDecoder(Class errorDecoder) { + this.errorDecoder = errorDecoder; + } + + public List> getRequestInterceptors() { + return requestInterceptors; + } + + public void setRequestInterceptors(List> requestInterceptors) { + this.requestInterceptors = requestInterceptors; + } + + public Boolean getDecode404() { + return decode404; + } + + public void setDecode404(Boolean decode404) { + this.decode404 = decode404; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + FeignClientConfiguration that = (FeignClientConfiguration) o; + return loggerLevel == that.loggerLevel && + Objects.equals(connectTimeout, that.connectTimeout) && + Objects.equals(readTimeout, that.readTimeout) && + Objects.equals(retryer, that.retryer) && + Objects.equals(errorDecoder, that.errorDecoder) && + Objects.equals(requestInterceptors, that.requestInterceptors) && + Objects.equals(decode404, that.decode404); + } + + @Override + public int hashCode() { + return Objects.hash(loggerLevel, connectTimeout, readTimeout, retryer, + errorDecoder, requestInterceptors, decode404); + } + } + +} diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/hystrix/HystrixConstants.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/hystrix/HystrixConstants.java index 559dbf00..073eec02 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/hystrix/HystrixConstants.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/hystrix/HystrixConstants.java @@ -19,8 +19,12 @@ package org.springframework.cloud.netflix.hystrix; /** * @author Spencer Gibb */ -public interface HystrixConstants { +public class HystrixConstants { - String HYSTRIX_STREAM_DESTINATION = "springCloudHystrixStream"; + public static final String HYSTRIX_STREAM_DESTINATION = "springCloudHystrixStream"; + + private HystrixConstants() { + throw new AssertionError("Must not instantiate constant utility class"); + } } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/Route.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/Route.java index 63c0d7d7..a9e95e8d 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/Route.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/Route.java @@ -42,6 +42,12 @@ public class Route { } } } + + public Route(String id, String path, String location, String prefix, + Boolean retryable, Set ignoredHeaders, boolean prefixStripped) { + this(id, path, location, prefix, retryable, ignoredHeaders); + this.prefixStripped = prefixStripped; + } private String id; @@ -58,6 +64,8 @@ public class Route { private Set sensitiveHeaders = new LinkedHashSet<>(); private boolean customSensitiveHeaders; + + private boolean prefixStripped = true; public boolean isCustomSensitiveHeaders() { return this.customSensitiveHeaders; diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/SimpleRouteLocator.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/SimpleRouteLocator.java index 28ccdf85..d5798b1e 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/SimpleRouteLocator.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/SimpleRouteLocator.java @@ -151,7 +151,8 @@ public class SimpleRouteLocator implements RouteLocator, Ordered { } return new Route(route.getId(), targetPath, route.getLocation(), prefix, retryable, - route.isCustomSensitiveHeaders() ? route.getSensitiveHeaders() : null); + route.isCustomSensitiveHeaders() ? route.getSensitiveHeaders() : null, + route.isStripPrefix()); } /** diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ZuulProperties.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ZuulProperties.java index 8c2d40ad..d25ac04a 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ZuulProperties.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ZuulProperties.java @@ -160,6 +160,8 @@ public class ZuulProperties { private ExecutionIsolationStrategy ribbonIsolationStrategy = SEMAPHORE; private HystrixSemaphore semaphore = new HystrixSemaphore(); + + private HystrixThreadPool threadPool = new HystrixThreadPool(); public Set getIgnoredHeaders() { Set ignoredHeaders = new LinkedHashSet<>(this.ignoredHeaders); @@ -302,7 +304,8 @@ public class ZuulProperties { public Route getRoute(String prefix) { return new Route(this.id, this.path, getLocation(), prefix, this.retryable, - isCustomSensitiveHeaders() ? this.sensitiveHeaders : null); + isCustomSensitiveHeaders() ? this.sensitiveHeaders : null, + this.stripPrefix); } public void setSensitiveHeaders(Set headers) { @@ -357,6 +360,39 @@ public class ZuulProperties { } + public static class HystrixThreadPool { + /** + * Flag to determine whether RibbonCommands should use separate thread pools for hystrix. + * By setting to true, RibbonCommands will be executed in a hystrix's thread pool that it is associated with. + * Each RibbonCommand will be associated with a thread pool according to its commandKey (serviceId). + * As default, all commands will be executed in a single thread pool whose threadPoolKey is "RibbonCommand". + * This property is only applicable when using THREAD as ribbonIsolationStrategy + */ + private boolean useSeparateThreadPools = false; + + /** + * A prefix for HystrixThreadPoolKey of hystrix's thread pool that is allocated to each service Id. + * This property is only applicable when using THREAD as ribbonIsolationStrategy and useSeparateThreadPools = true + */ + private String threadPoolKeyPrefix = ""; + + public boolean isUseSeparateThreadPools() { + return useSeparateThreadPools; + } + + public void setUseSeparateThreadPools(boolean useSeparateThreadPools) { + this.useSeparateThreadPools = useSeparateThreadPools; + } + + public String getThreadPoolKeyPrefix() { + return threadPoolKeyPrefix; + } + + public void setThreadPoolKeyPrefix(String threadPoolKeyPrefix) { + this.threadPoolKeyPrefix = threadPoolKeyPrefix; + } + } + public String getServletPattern() { String path = this.servletPath; if (!path.startsWith("/")) { diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/post/LocationRewriteFilter.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/post/LocationRewriteFilter.java new file mode 100644 index 00000000..1cbdf4ae --- /dev/null +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/post/LocationRewriteFilter.java @@ -0,0 +1,160 @@ +/* + * Copyright 2017 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.post; + +import com.netflix.util.Pair; +import com.netflix.zuul.ZuulFilter; +import com.netflix.zuul.context.RequestContext; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.cloud.netflix.zuul.filters.Route; +import org.springframework.cloud.netflix.zuul.filters.RouteLocator; +import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; +import org.springframework.http.HttpStatus; +import org.springframework.http.server.ServletServerHttpRequest; +import org.springframework.util.StringUtils; +import org.springframework.web.util.UriComponents; +import org.springframework.web.util.UriComponentsBuilder; +import org.springframework.web.util.UrlPathHelper; + +import java.net.URI; + +import static org.springframework.cloud.netflix.zuul.filters.support.FilterConstants.POST_TYPE; +import static org.springframework.cloud.netflix.zuul.filters.support.FilterConstants.SEND_RESPONSE_FILTER_ORDER; + +/** + * {@link ZuulFilter} Responsible for rewriting the Location header to be the Zuul URL + * + * @author Biju Kunjummen + */ +public class LocationRewriteFilter extends ZuulFilter { + + private final UrlPathHelper urlPathHelper = new UrlPathHelper(); + + @Autowired + private ZuulProperties zuulProperties; + + @Autowired + private RouteLocator routeLocator; + + private static final String LOCATION_HEADER = "Location"; + + public LocationRewriteFilter() { + } + + public LocationRewriteFilter(ZuulProperties zuulProperties, + RouteLocator routeLocator) { + this.routeLocator = routeLocator; + this.zuulProperties = zuulProperties; + } + + @Override + public String filterType() { + return POST_TYPE; + } + + @Override + public int filterOrder() { + return SEND_RESPONSE_FILTER_ORDER - 100; + } + + @Override + public boolean shouldFilter() { + RequestContext ctx = RequestContext.getCurrentContext(); + int statusCode = ctx.getResponseStatusCode(); + return HttpStatus.valueOf(statusCode).is3xxRedirection(); + } + + @Override + public Object run() { + RequestContext ctx = RequestContext.getCurrentContext(); + Route route = routeLocator.getMatchingRoute( + urlPathHelper.getPathWithinApplication(ctx.getRequest())); + + if (route != null) { + Pair lh = locationHeader(ctx); + if (lh != null) { + String location = lh.second(); + URI originalRequestUri = UriComponentsBuilder + .fromHttpRequest(new ServletServerHttpRequest(ctx.getRequest())) + .build().toUri(); + + UriComponentsBuilder redirectedUriBuilder = UriComponentsBuilder + .fromUriString(location); + + UriComponents redirectedUriComps = redirectedUriBuilder.build(); + + String newPath = getRestoredPath(this.zuulProperties, route, + redirectedUriComps); + + String modifiedLocation = redirectedUriBuilder + .scheme(originalRequestUri.getScheme()) + .host(originalRequestUri.getHost()) + .port(originalRequestUri.getPort()).replacePath(newPath).build() + .toUriString(); + + lh.setSecond(modifiedLocation); + } + } + return null; + } + + private String getRestoredPath(ZuulProperties zuulProperties, Route route, + UriComponents redirectedUriComps) { + StringBuilder path = new StringBuilder(); + String redirectedPathWithoutGlobal = downstreamHasGlobalPrefix(zuulProperties) + ? redirectedUriComps.getPath() + .substring(("/" + zuulProperties.getPrefix()).length()) + : redirectedUriComps.getPath(); + + if (downstreamHasGlobalPrefix(zuulProperties)) { + path.append("/" + zuulProperties.getPrefix()); + } + else { + path.append(zuulHasGlobalPrefix(zuulProperties) + ? "/" + zuulProperties.getPrefix() : ""); + } + + path.append(downstreamHasRoutePrefix(route) ? "" : "/" + route.getPrefix()) + .append(redirectedPathWithoutGlobal); + + return path.toString(); + } + + private boolean downstreamHasGlobalPrefix(ZuulProperties zuulProperties) { + return (!zuulProperties.isStripPrefix() + && StringUtils.hasText(zuulProperties.getPrefix())); + } + + private boolean zuulHasGlobalPrefix(ZuulProperties zuulProperties) { + return StringUtils.hasText(zuulProperties.getPrefix()); + } + + private boolean downstreamHasRoutePrefix(Route route) { + return (!route.isPrefixStripped() && StringUtils.hasText(route.getPrefix())); + } + + private Pair locationHeader(RequestContext ctx) { + if (ctx.getZuulResponseHeaders() != null) { + for (Pair pair : ctx.getZuulResponseHeaders()) { + if (pair.first().equals(LOCATION_HEADER)) { + return pair; + } + } + } + return null; + } +} 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 3c157929..5431405e 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 @@ -34,6 +34,7 @@ import com.netflix.hystrix.HystrixCommandGroupKey; import com.netflix.hystrix.HystrixCommandKey; import com.netflix.hystrix.HystrixCommandProperties; import com.netflix.hystrix.HystrixCommandProperties.ExecutionIsolationStrategy; +import com.netflix.hystrix.HystrixThreadPoolKey; import com.netflix.zuul.constants.ZuulConstants; import com.netflix.zuul.context.RequestContext; @@ -78,6 +79,9 @@ public abstract class AbstractRibbonCommand response = new TestRestTemplate().getForEntity(url, + String.class); + List locationHeaders = response.getHeaders().get("Location"); + + assertThat(locationHeaders).hasSize(1); + String locationHeader = locationHeaders.get(0); + assertThat(locationHeader).withFailMessage("Location should have prefix") + .isEqualTo( + String.format("http://localhost:%d/service/redirectedUri", port)); + + } + + @SpringBootConfiguration + @EnableAutoConfiguration + @EnableZuulProxy + @Controller + @RibbonClient(name = "aservice", configuration = RibbonConfig.class) + protected static class Config { + + @RequestMapping("/redirectingUri") + public String redirect1() { + return "redirect:/redirectedUri"; + } + + @Bean + public LocationRewriteFilter locationRewriteFilter() { + return new LocationRewriteFilter(); + } + + } + + public static class RibbonConfig { + @LocalServerPort + private int port; + + @Bean + public ServerList ribbonServerList() { + return new StaticServerList<>(new Server("localhost", this.port)); + } + + } +} diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/post/LocationRewriteFilterTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/post/LocationRewriteFilterTests.java new file mode 100644 index 00000000..c29febe7 --- /dev/null +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/post/LocationRewriteFilterTests.java @@ -0,0 +1,178 @@ +/* + * Copyright 2017 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.post; + +import com.netflix.util.Pair; +import com.netflix.zuul.context.RequestContext; +import org.junit.Before; +import org.junit.Test; +import org.springframework.cloud.netflix.zuul.filters.Route; +import org.springframework.cloud.netflix.zuul.filters.RouteLocator; +import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.mock.web.MockHttpServletResponse; + +import java.util.Collections; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +/** + * @author Biju Kunjummen + */ + +public class LocationRewriteFilterTests { + + private final String ZUUL_HOST = "myzuul.com"; + private final String ZUUL_SCHEME = "https"; + private final int ZUUL_PORT = 8443; + private final String ZUUL_BASE_URL = String.format("%s://%s:%d", ZUUL_SCHEME, + ZUUL_HOST, ZUUL_PORT); + + private final String SERVER_HOST = "someserver.com"; + private final String SERVER_SCHEME = "http"; + private final int SERVER_PORT = 8564; + private final String SERVER_BASE_URL = String.format("%s://%s:%d", SERVER_SCHEME, + SERVER_HOST, SERVER_PORT); + + @Before + public void before() { + RequestContext context = new RequestContext(); + RequestContext.testSetCurrentContext(context); + } + + @Test + public void shouldRewriteLocationHeadersWithRoutePrefix() { + RequestContext context = RequestContext.getCurrentContext(); + ZuulProperties zuulProperties = new ZuulProperties(); + LocationRewriteFilter filter = setFilterUpWith(context, zuulProperties, + new Route("service1", "/redirectingUri", "service1", "prefix", false, + Collections.EMPTY_SET, true), + "/prefix/redirectingUri", "/redirectedUri;someparam?param1=abc"); + filter.run(); + assertThat(getLocationHeader(context).second()).isEqualTo(String + .format("%s/prefix/redirectedUri;someparam?param1=abc", ZUUL_BASE_URL)); + } + + @Test + public void shouldBeUntouchedIfNoRoutesFound() { + RequestContext context = RequestContext.getCurrentContext(); + ZuulProperties zuulProperties = new ZuulProperties(); + LocationRewriteFilter filter = setFilterUpWith(context, zuulProperties, null, + "/prefix/redirectingUri", "/redirectedUri;someparam?param1=abc"); + filter.run(); + assertThat(getLocationHeader(context).second()).isEqualTo( + String.format("%s/redirectedUri;someparam?param1=abc", SERVER_BASE_URL)); + } + + @Test + public void shouldRewriteLocationHeadersIfPrefixIsNotStripped() { + RequestContext context = RequestContext.getCurrentContext(); + ZuulProperties zuulProperties = new ZuulProperties(); + LocationRewriteFilter filter = setFilterUpWith(context, zuulProperties, + new Route("service1", "/something/redirectingUri", "service1", "prefix", + false, Collections.EMPTY_SET, false), + "/prefix/redirectingUri", + "/something/redirectedUri;someparam?param1=abc"); + filter.run(); + assertThat(getLocationHeader(context).second()).isEqualTo(String.format( + "%s/something/redirectedUri;someparam?param1=abc", ZUUL_BASE_URL)); + } + + @Test + public void shouldRewriteLocationHeadersIfPrefixIsEmpty() { + RequestContext context = RequestContext.getCurrentContext(); + ZuulProperties zuulProperties = new ZuulProperties(); + LocationRewriteFilter filter = setFilterUpWith(context, zuulProperties, + new Route("service1", "/something/redirectingUri", "service1", "", false, + Collections.EMPTY_SET, true), + "/redirectingUri", "/something/redirectedUri;someparam?param1=abc"); + filter.run(); + assertThat(getLocationHeader(context).second()).isEqualTo(String.format( + "%s/something/redirectedUri;someparam?param1=abc", ZUUL_BASE_URL)); + } + + @Test + public void shouldAddBackGlobalPrefixIfPresent() { + RequestContext context = RequestContext.getCurrentContext(); + ZuulProperties zuulProperties = new ZuulProperties(); + zuulProperties.setPrefix("global"); + zuulProperties.setStripPrefix(true); + LocationRewriteFilter filter = setFilterUpWith(context, zuulProperties, + new Route("service1", "/something/redirectingUri", "service1", "prefix", + false, Collections.EMPTY_SET, true), + "/global/prefix/redirectingUri", + "/something/redirectedUri;someparam?param1=abc"); + filter.run(); + assertThat(getLocationHeader(context).second()).isEqualTo(String.format( + "%s/global/prefix/something/redirectedUri;someparam?param1=abc", + ZUUL_BASE_URL)); + } + + @Test + public void shouldNotAddBackGlobalPrefixIfNotStripped() { + RequestContext context = RequestContext.getCurrentContext(); + ZuulProperties zuulProperties = new ZuulProperties(); + zuulProperties.setPrefix("global"); + zuulProperties.setStripPrefix(false); + LocationRewriteFilter filter = setFilterUpWith(context, zuulProperties, + new Route("service1", "/something/redirectingUri", "service1", "prefix", + false, Collections.EMPTY_SET, true), + "/global/prefix/redirectingUri", + "/global/something/redirectedUri;someparam?param1=abc"); + filter.run(); + assertThat(getLocationHeader(context).second()).isEqualTo(String.format( + "%s/global/prefix/something/redirectedUri;someparam?param1=abc", + ZUUL_BASE_URL)); + } + + private LocationRewriteFilter setFilterUpWith(RequestContext context, + ZuulProperties zuulProperties, Route route, String toZuulRequestUri, + String redirectedUri) { + MockHttpServletRequest httpServletRequest = new MockHttpServletRequest(); + httpServletRequest.setRequestURI(toZuulRequestUri); + httpServletRequest.setServerName(ZUUL_HOST); + httpServletRequest.setScheme(ZUUL_SCHEME); + httpServletRequest.setServerPort(ZUUL_PORT); + context.setRequest(httpServletRequest); + + MockHttpServletResponse httpServletResponse = new MockHttpServletResponse(); + context.getZuulResponseHeaders().add(new Pair<>("Location", + String.format("%s%s", SERVER_BASE_URL, redirectedUri))); + context.setResponse(httpServletResponse); + + RouteLocator routeLocator = mock(RouteLocator.class); + when(routeLocator.getMatchingRoute(toZuulRequestUri)).thenReturn(route); + LocationRewriteFilter filter = new LocationRewriteFilter(zuulProperties, + routeLocator); + + return filter; + } + + private Pair getLocationHeader(RequestContext ctx) { + if (ctx.getZuulResponseHeaders() != null) { + for (Pair pair : ctx.getZuulResponseHeaders()) { + if (pair.first().equals("Location")) { + return pair; + } + } + } + return null; + } +} diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/support/RibbonCommandHystrixThreadPoolKeyTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/support/RibbonCommandHystrixThreadPoolKeyTests.java new file mode 100644 index 00000000..05ab8d4f --- /dev/null +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/support/RibbonCommandHystrixThreadPoolKeyTests.java @@ -0,0 +1,107 @@ +/* + * Copyright 2017 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 com.netflix.client.AbstractLoadBalancerAwareClient; +import com.netflix.client.ClientRequest; +import com.netflix.client.http.HttpResponse; +import com.netflix.hystrix.HystrixCommandProperties; +import org.junit.Before; +import org.junit.Test; +import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * @author Yongsung Yoon + */ +public class RibbonCommandHystrixThreadPoolKeyTests { + + private ZuulProperties zuulProperties; + + @Before + public void setUp() throws Exception { + zuulProperties = new ZuulProperties(); + } + + @Test + public void testDefaultHystrixThreadPoolKey() throws Exception { + zuulProperties.setRibbonIsolationStrategy(HystrixCommandProperties.ExecutionIsolationStrategy.THREAD); + + TestRibbonCommand ribbonCommand1 = new TestRibbonCommand("testCommand1", zuulProperties); + TestRibbonCommand ribbonCommand2 = new TestRibbonCommand("testCommand2", zuulProperties); + + // CommandGroupKey should be used as ThreadPoolKey as default. + assertThat(ribbonCommand1.getThreadPoolKey().name()).isEqualTo(ribbonCommand1.getCommandGroup().name()); + assertThat(ribbonCommand2.getThreadPoolKey().name()).isEqualTo(ribbonCommand2.getCommandGroup().name()); + } + + @Test + public void testUseSeparateThreadPools() throws Exception { + zuulProperties.setRibbonIsolationStrategy(HystrixCommandProperties.ExecutionIsolationStrategy.THREAD); + zuulProperties.getThreadPool().setUseSeparateThreadPools(true); + + TestRibbonCommand ribbonCommand1 = new TestRibbonCommand("testCommand1", zuulProperties); + TestRibbonCommand ribbonCommand2 = new TestRibbonCommand("testCommand2", zuulProperties); + + assertThat(ribbonCommand1.getThreadPoolKey().name()).isEqualTo("testCommand1"); + assertThat(ribbonCommand2.getThreadPoolKey().name()).isEqualTo("testCommand2"); + } + + @Test + public void testThreadPoolKeyPrefix() throws Exception { + final String prefix = "zuulgw-"; + + zuulProperties.setRibbonIsolationStrategy(HystrixCommandProperties.ExecutionIsolationStrategy.THREAD); + zuulProperties.getThreadPool().setUseSeparateThreadPools(true); + zuulProperties.getThreadPool().setThreadPoolKeyPrefix(prefix); + + TestRibbonCommand ribbonCommand1 = new TestRibbonCommand("testCommand1", zuulProperties); + TestRibbonCommand ribbonCommand2 = new TestRibbonCommand("testCommand2", zuulProperties); + + assertThat(ribbonCommand1.getThreadPoolKey().name()).isEqualTo(prefix + "testCommand1"); + assertThat(ribbonCommand2.getThreadPoolKey().name()).isEqualTo(prefix + "testCommand2"); + } + + @Test + public void testNoSideEffectOnSemaphoreIsolation() throws Exception { + final String prefix = "zuulgw-"; + + zuulProperties.setRibbonIsolationStrategy(HystrixCommandProperties.ExecutionIsolationStrategy.SEMAPHORE); + zuulProperties.getThreadPool().setUseSeparateThreadPools(true); + zuulProperties.getThreadPool().setThreadPoolKeyPrefix(prefix); + + TestRibbonCommand ribbonCommand1 = new TestRibbonCommand("testCommand1", zuulProperties); + TestRibbonCommand ribbonCommand2 = new TestRibbonCommand("testCommand2", zuulProperties); + + // There should be no side effect on semaphore isolation + assertThat(ribbonCommand1.getThreadPoolKey().name()).isEqualTo(ribbonCommand1.getCommandGroup().name()); + assertThat(ribbonCommand2.getThreadPoolKey().name()).isEqualTo(ribbonCommand2.getCommandGroup().name()); + } + + public static class TestRibbonCommand + extends AbstractRibbonCommand, ClientRequest, HttpResponse> { + public TestRibbonCommand(String commandKey, ZuulProperties zuulProperties) { + super(commandKey, null, null, zuulProperties); + } + + @Override + protected ClientRequest createRequest() throws Exception { + return null; + } + } +} \ No newline at end of file diff --git a/spring-cloud-netflix-core/src/test/resources/feign-properties.properties b/spring-cloud-netflix-core/src/test/resources/feign-properties.properties new file mode 100644 index 00000000..bc045d17 --- /dev/null +++ b/spring-cloud-netflix-core/src/test/resources/feign-properties.properties @@ -0,0 +1,19 @@ +# This configuration used by test class org.springframework.cloud.netflix.feign.FeignClientUsingPropertiesTests + +logging.level.org.springframework.cloud.netflix.feign=debug + +feign.client.default-to-properties=true +feign.client.default-config=default + +feign.client.config.default.connectTimeout=5000 +feign.client.config.default.readTimeout=5000 +feign.client.config.default.loggerLevel=full +feign.client.config.default.errorDecoder=org.springframework.cloud.netflix.feign.FeignClientUsingPropertiesTests.DefaultErrorDecoder +feign.client.config.default.retryer=org.springframework.cloud.netflix.feign.FeignClientUsingPropertiesTests.NoRetryer +feign.client.config.default.decode404=true + +feign.client.config.foo.requestInterceptors[0]=org.springframework.cloud.netflix.feign.FeignClientUsingPropertiesTests.FooRequestInterceptor +feign.client.config.foo.requestInterceptors[1]=org.springframework.cloud.netflix.feign.FeignClientUsingPropertiesTests.BarRequestInterceptor + +feign.client.config.bar.connectTimeout=1000 +feign.client.config.bar.readTimeout=1000 \ No newline at end of file diff --git a/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/EurekaClientConfigBean.java b/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/EurekaClientConfigBean.java index a7f7228c..014e39fb 100644 --- a/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/EurekaClientConfigBean.java +++ b/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/EurekaClientConfigBean.java @@ -33,12 +33,14 @@ import com.netflix.discovery.shared.transport.EurekaTransportConfig; import lombok.Data; +import static org.springframework.cloud.netflix.eureka.EurekaConstants.DEFAULT_PREFIX; + /** * @author Dave Syer */ @Data @ConfigurationProperties(EurekaClientConfigBean.PREFIX) -public class EurekaClientConfigBean implements EurekaClientConfig, EurekaConstants { +public class EurekaClientConfigBean implements EurekaClientConfig { public static final String PREFIX = "eureka.client"; diff --git a/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/EurekaConstants.java b/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/EurekaConstants.java index 75dc0493..1f24480a 100644 --- a/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/EurekaConstants.java +++ b/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/EurekaConstants.java @@ -19,8 +19,12 @@ package org.springframework.cloud.netflix.eureka; /** * @author Spencer Gibb */ -public interface EurekaConstants { +public class EurekaConstants { - String DEFAULT_PREFIX = "/eureka"; + public static final String DEFAULT_PREFIX = "/eureka"; + + private EurekaConstants() { + throw new AssertionError("Must not instantiate constant utility class"); + } } diff --git a/spring-cloud-netflix-eureka-server/src/main/java/org/springframework/cloud/netflix/eureka/server/EurekaServerConfigBean.java b/spring-cloud-netflix-eureka-server/src/main/java/org/springframework/cloud/netflix/eureka/server/EurekaServerConfigBean.java index 26ffb67d..b4532e5a 100644 --- a/spring-cloud-netflix-eureka-server/src/main/java/org/springframework/cloud/netflix/eureka/server/EurekaServerConfigBean.java +++ b/spring-cloud-netflix-eureka-server/src/main/java/org/springframework/cloud/netflix/eureka/server/EurekaServerConfigBean.java @@ -23,7 +23,6 @@ import java.util.Set; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.context.properties.ConfigurationProperties; -import org.springframework.cloud.netflix.eureka.EurekaConstants; import org.springframework.core.env.PropertyResolver; import com.netflix.eureka.EurekaServerConfig; @@ -36,7 +35,7 @@ import lombok.Data; */ @Data @ConfigurationProperties(EurekaServerConfigBean.PREFIX) -public class EurekaServerConfigBean implements EurekaServerConfig, EurekaConstants { +public class EurekaServerConfigBean implements EurekaServerConfig { public static final String PREFIX = "eureka.server";