From 3f5c0b26ceb7a3ae31f4a5de0105fdd8726f6815 Mon Sep 17 00:00:00 2001 From: Tim Ysewyn Date: Tue, 23 Jul 2019 18:00:01 +0200 Subject: [PATCH 1/5] Auto forward upstream headers in mvc and webflux proxy exchange. fixes gh-1183 fixes gh-1193 --- .../config/ProxyExchangeArgumentResolver.java | 30 ++++++++++++++++++- .../gateway/mvc/config/ProxyProperties.java | 15 ++++++++++ .../ProxyResponseAutoConfiguration.java | 2 ++ .../mvc/ProductionConfigurationTests.java | 17 +++++++---- .../config/ProxyExchangeArgumentResolver.java | 21 ++++++++++++- .../webflux/config/ProxyProperties.java | 15 ++++++++++ .../ProxyResponseAutoConfiguration.java | 2 ++ .../webflux/ProductionConfigurationTests.java | 17 +++++++---- 8 files changed, 107 insertions(+), 12 deletions(-) diff --git a/spring-cloud-gateway-mvc/src/main/java/org/springframework/cloud/gateway/mvc/config/ProxyExchangeArgumentResolver.java b/spring-cloud-gateway-mvc/src/main/java/org/springframework/cloud/gateway/mvc/config/ProxyExchangeArgumentResolver.java index 9909e3b9..87381573 100644 --- a/spring-cloud-gateway-mvc/src/main/java/org/springframework/cloud/gateway/mvc/config/ProxyExchangeArgumentResolver.java +++ b/spring-cloud-gateway-mvc/src/main/java/org/springframework/cloud/gateway/mvc/config/ProxyExchangeArgumentResolver.java @@ -18,8 +18,12 @@ package org.springframework.cloud.gateway.mvc.config; import java.lang.reflect.ParameterizedType; import java.lang.reflect.Type; +import java.util.Collections; +import java.util.Enumeration; import java.util.Set; +import javax.servlet.http.HttpServletRequest; + import org.springframework.cloud.gateway.mvc.ProxyExchange; import org.springframework.core.MethodParameter; import org.springframework.http.HttpHeaders; @@ -31,7 +35,7 @@ import org.springframework.web.method.support.ModelAndViewContainer; /** * @author Dave Syer - * + * @author Tim Ysewyn */ public class ProxyExchangeArgumentResolver implements HandlerMethodArgumentResolver { @@ -39,6 +43,8 @@ public class ProxyExchangeArgumentResolver implements HandlerMethodArgumentResol private HttpHeaders headers; + private Set autoForwardedHeaders; + private Set sensitive; public ProxyExchangeArgumentResolver(RestTemplate builder) { @@ -49,6 +55,10 @@ public class ProxyExchangeArgumentResolver implements HandlerMethodArgumentResol this.headers = headers; } + public void setAutoForwardedHeaders(Set autoForwardedHeaders) { + this.autoForwardedHeaders = autoForwardedHeaders; + } + public void setSensitive(Set sensitive) { this.sensitive = sensitive; } @@ -65,6 +75,9 @@ public class ProxyExchangeArgumentResolver implements HandlerMethodArgumentResol ProxyExchange proxy = new ProxyExchange<>(rest, webRequest, mavContainer, binderFactory, type(parameter)); proxy.headers(headers); + if (this.autoForwardedHeaders.size() > 0) { + proxy.headers(extractAutoForwardedHeaders(webRequest)); + } if (sensitive != null) { proxy.sensitive(sensitive.toArray(new String[0])); } @@ -80,4 +93,19 @@ public class ProxyExchangeArgumentResolver implements HandlerMethodArgumentResol return type; } + private HttpHeaders extractAutoForwardedHeaders(NativeWebRequest webRequest) { + HttpServletRequest nativeRequest = webRequest + .getNativeRequest(HttpServletRequest.class); + Enumeration headerNames = nativeRequest.getHeaderNames(); + HttpHeaders headers = new HttpHeaders(); + while (headerNames.hasMoreElements()) { + String header = headerNames.nextElement(); + if (this.autoForwardedHeaders.contains(header)) { + headers.addAll(header, + Collections.list(nativeRequest.getHeaders(header))); + } + } + return headers; + } + } diff --git a/spring-cloud-gateway-mvc/src/main/java/org/springframework/cloud/gateway/mvc/config/ProxyProperties.java b/spring-cloud-gateway-mvc/src/main/java/org/springframework/cloud/gateway/mvc/config/ProxyProperties.java index 6773f07f..5ee4b605 100644 --- a/spring-cloud-gateway-mvc/src/main/java/org/springframework/cloud/gateway/mvc/config/ProxyProperties.java +++ b/spring-cloud-gateway-mvc/src/main/java/org/springframework/cloud/gateway/mvc/config/ProxyProperties.java @@ -16,6 +16,7 @@ package org.springframework.cloud.gateway.mvc.config; +import java.util.HashSet; import java.util.LinkedHashMap; import java.util.Map; import java.util.Set; @@ -29,6 +30,7 @@ import org.springframework.http.HttpHeaders; * @RequestMapping methods. * * @author Dave Syer + * @author Tim Ysewyn * */ @ConfigurationProperties("spring.cloud.gateway.proxy") @@ -39,6 +41,11 @@ public class ProxyProperties { */ private Map headers = new LinkedHashMap<>(); + /** + * A set of header names that should be send downstream by default. + */ + private Set autoForward = new HashSet<>(); + /** * A set of sensitive header names that will not be sent downstream by default. */ @@ -52,6 +59,14 @@ public class ProxyProperties { this.headers = headers; } + public Set getAutoForward() { + return autoForward; + } + + public void setAutoForward(Set autoForward) { + this.autoForward = autoForward; + } + public Set getSensitive() { return sensitive; } diff --git a/spring-cloud-gateway-mvc/src/main/java/org/springframework/cloud/gateway/mvc/config/ProxyResponseAutoConfiguration.java b/spring-cloud-gateway-mvc/src/main/java/org/springframework/cloud/gateway/mvc/config/ProxyResponseAutoConfiguration.java index fc521fa2..08d48a53 100644 --- a/spring-cloud-gateway-mvc/src/main/java/org/springframework/cloud/gateway/mvc/config/ProxyResponseAutoConfiguration.java +++ b/spring-cloud-gateway-mvc/src/main/java/org/springframework/cloud/gateway/mvc/config/ProxyResponseAutoConfiguration.java @@ -43,6 +43,7 @@ import org.springframework.web.servlet.config.annotation.WebMvcConfigurer; * @RequestMapping methods. * * @author Dave Syer + * @author Tim Ysewyn */ @Configuration(proxyBeanMethods = false) @ConditionalOnWebApplication @@ -69,6 +70,7 @@ public class ProxyResponseAutoConfiguration implements WebMvcConfigurer { ProxyExchangeArgumentResolver resolver = new ProxyExchangeArgumentResolver( template); resolver.setHeaders(proxy.convertHeaders()); + resolver.setAutoForwardedHeaders(proxy.getAutoForward()); resolver.setSensitive(proxy.getSensitive()); // can be null return resolver; } diff --git a/spring-cloud-gateway-mvc/src/test/java/org/springframework/cloud/gateway/mvc/ProductionConfigurationTests.java b/spring-cloud-gateway-mvc/src/test/java/org/springframework/cloud/gateway/mvc/ProductionConfigurationTests.java index 073b4bb8..4e3f8ae7 100644 --- a/spring-cloud-gateway-mvc/src/test/java/org/springframework/cloud/gateway/mvc/ProductionConfigurationTests.java +++ b/spring-cloud-gateway-mvc/src/test/java/org/springframework/cloud/gateway/mvc/ProductionConfigurationTests.java @@ -55,7 +55,8 @@ import org.springframework.web.util.UriComponentsBuilder; import static org.assertj.core.api.Assertions.assertThat; @RunWith(SpringRunner.class) -@SpringBootTest(webEnvironment = WebEnvironment.RANDOM_PORT) +@SpringBootTest(properties = { + "spring.cloud.gateway.proxy.auto-forward=baz" }, webEnvironment = WebEnvironment.RANDOM_PORT) @ContextConfiguration(classes = TestApplication.class) public class ProductionConfigurationTests { @@ -247,15 +248,21 @@ public class ProductionConfigurationTests { @Test @SuppressWarnings({ "Duplicates", "unchecked" }) public void headers() throws Exception { - Map> headers = rest.exchange(RequestEntity - .get(rest.getRestTemplate().getUriTemplateHandler() - .expand("/proxy/headers")) - .header("foo", "bar").header("abc", "xyz").build(), Map.class).getBody(); + Map> headers = rest + .exchange( + RequestEntity + .get(rest.getRestTemplate().getUriTemplateHandler() + .expand("/proxy/headers")) + .header("foo", "bar").header("abc", "xyz") + .header("baz", "fob").build(), + Map.class) + .getBody(); assertThat(headers).doesNotContainKey("foo").doesNotContainKey("hello") .containsKeys("bar", "abc"); assertThat(headers.get("bar")).containsOnly("hello"); assertThat(headers.get("abc")).containsOnly("123"); + assertThat(headers.get("baz")).containsOnly("fob"); } @Test diff --git a/spring-cloud-gateway-webflux/src/main/java/org/springframework/cloud/gateway/webflux/config/ProxyExchangeArgumentResolver.java b/spring-cloud-gateway-webflux/src/main/java/org/springframework/cloud/gateway/webflux/config/ProxyExchangeArgumentResolver.java index fea92a1c..baecff03 100644 --- a/spring-cloud-gateway-webflux/src/main/java/org/springframework/cloud/gateway/webflux/config/ProxyExchangeArgumentResolver.java +++ b/spring-cloud-gateway-webflux/src/main/java/org/springframework/cloud/gateway/webflux/config/ProxyExchangeArgumentResolver.java @@ -34,7 +34,7 @@ import org.springframework.web.server.ServerWebExchange; /** * @author Dave Syer - * + * @author Tim Ysewyn */ public class ProxyExchangeArgumentResolver implements HandlerMethodArgumentResolver { @@ -42,6 +42,8 @@ public class ProxyExchangeArgumentResolver implements HandlerMethodArgumentResol private HttpHeaders headers; + private Set autoForwardedHeaders; + private Set sensitive; public ProxyExchangeArgumentResolver(WebClient builder) { @@ -52,6 +54,10 @@ public class ProxyExchangeArgumentResolver implements HandlerMethodArgumentResol this.headers = headers; } + public void setAutoForwardedHeaders(Set autoForwardedHeaders) { + this.autoForwardedHeaders = autoForwardedHeaders; + } + public void setSensitive(Set sensitive) { this.sensitive = sensitive; } @@ -79,10 +85,23 @@ public class ProxyExchangeArgumentResolver implements HandlerMethodArgumentResol ProxyExchange proxy = new ProxyExchange<>(rest, exchange, bindingContext, type(parameter)); proxy.headers(headers); + if (this.autoForwardedHeaders.size() > 0) { + proxy.headers(extractAutoForwardedHeaders(exchange)); + } if (sensitive != null) { proxy.sensitive(sensitive.toArray(new String[0])); } return Mono.just(proxy); } + private HttpHeaders extractAutoForwardedHeaders(ServerWebExchange exchange) { + HttpHeaders headers = new HttpHeaders(); + exchange.getRequest().getHeaders().forEach((header, values) -> { + if (this.autoForwardedHeaders.contains(header)) { + headers.addAll(header, values); + } + }); + return headers; + } + } diff --git a/spring-cloud-gateway-webflux/src/main/java/org/springframework/cloud/gateway/webflux/config/ProxyProperties.java b/spring-cloud-gateway-webflux/src/main/java/org/springframework/cloud/gateway/webflux/config/ProxyProperties.java index 100ec121..d7796a90 100644 --- a/spring-cloud-gateway-webflux/src/main/java/org/springframework/cloud/gateway/webflux/config/ProxyProperties.java +++ b/spring-cloud-gateway-webflux/src/main/java/org/springframework/cloud/gateway/webflux/config/ProxyProperties.java @@ -16,6 +16,7 @@ package org.springframework.cloud.gateway.webflux.config; +import java.util.HashSet; import java.util.LinkedHashMap; import java.util.Map; import java.util.Set; @@ -29,6 +30,7 @@ import org.springframework.http.HttpHeaders; * @RequestMapping methods. * * @author Dave Syer + * @author Tim Ysewyn * */ @ConfigurationProperties("spring.cloud.gateway.proxy") @@ -39,6 +41,11 @@ public class ProxyProperties { */ private Map headers = new LinkedHashMap<>(); + /** + * A set of header names that should be send downstream by default. + */ + private Set autoForward = new HashSet<>(); + /** * A set of sensitive header names that will not be sent downstream by default. */ @@ -52,6 +59,14 @@ public class ProxyProperties { this.headers = headers; } + public Set getAutoForward() { + return autoForward; + } + + public void setAutoForward(Set autoForward) { + this.autoForward = autoForward; + } + public Set getSensitive() { return sensitive; } diff --git a/spring-cloud-gateway-webflux/src/main/java/org/springframework/cloud/gateway/webflux/config/ProxyResponseAutoConfiguration.java b/spring-cloud-gateway-webflux/src/main/java/org/springframework/cloud/gateway/webflux/config/ProxyResponseAutoConfiguration.java index 18722916..c5fd7837 100644 --- a/spring-cloud-gateway-webflux/src/main/java/org/springframework/cloud/gateway/webflux/config/ProxyResponseAutoConfiguration.java +++ b/spring-cloud-gateway-webflux/src/main/java/org/springframework/cloud/gateway/webflux/config/ProxyResponseAutoConfiguration.java @@ -37,6 +37,7 @@ import org.springframework.web.reactive.result.method.annotation.ArgumentResolve * @RequestMapping methods. * * @author Dave Syer + * @author Tim Ysewyn */ @Configuration(proxyBeanMethods = false) @ConditionalOnWebApplication @@ -56,6 +57,7 @@ public class ProxyResponseAutoConfiguration implements WebFluxConfigurer { ProxyExchangeArgumentResolver resolver = new ProxyExchangeArgumentResolver( template); resolver.setHeaders(proxy.convertHeaders()); + resolver.setAutoForwardedHeaders(proxy.getAutoForward()); resolver.setSensitive(proxy.getSensitive()); // can be null return resolver; } diff --git a/spring-cloud-gateway-webflux/src/test/java/org/springframework/cloud/gateway/webflux/ProductionConfigurationTests.java b/spring-cloud-gateway-webflux/src/test/java/org/springframework/cloud/gateway/webflux/ProductionConfigurationTests.java index dd0e568d..6e3ac0be 100644 --- a/spring-cloud-gateway-webflux/src/test/java/org/springframework/cloud/gateway/webflux/ProductionConfigurationTests.java +++ b/spring-cloud-gateway-webflux/src/test/java/org/springframework/cloud/gateway/webflux/ProductionConfigurationTests.java @@ -58,7 +58,8 @@ import org.springframework.web.util.UriComponentsBuilder; import static org.assertj.core.api.Assertions.assertThat; @RunWith(SpringRunner.class) -@SpringBootTest(webEnvironment = WebEnvironment.RANDOM_PORT) +@SpringBootTest(properties = { + "spring.cloud.gateway.proxy.auto-forward=baz" }, webEnvironment = WebEnvironment.RANDOM_PORT) @ContextConfiguration(classes = TestApplication.class) @DirtiesContext public class ProductionConfigurationTests { @@ -193,15 +194,21 @@ public class ProductionConfigurationTests { @Test @SuppressWarnings({ "Duplicates", "unchecked" }) public void headers() throws Exception { - Map> headers = rest.exchange(RequestEntity - .get(rest.getRestTemplate().getUriTemplateHandler() - .expand("/proxy/headers")) - .header("foo", "bar").header("abc", "xyz").build(), Map.class).getBody(); + Map> headers = rest + .exchange( + RequestEntity + .get(rest.getRestTemplate().getUriTemplateHandler() + .expand("/proxy/headers")) + .header("foo", "bar").header("abc", "xyz") + .header("baz", "fob").build(), + Map.class) + .getBody(); assertThat(headers).doesNotContainKey("foo").doesNotContainKey("hello") .containsKeys("bar", "abc"); assertThat(headers.get("bar")).containsOnly("hello"); assertThat(headers.get("abc")).containsOnly("123"); + assertThat(headers.get("baz")).containsOnly("fob"); } @Test From 070c4be396489dbbc0134318e68b2fa94544895b Mon Sep 17 00:00:00 2001 From: suntiancheng Date: Tue, 4 Feb 2020 18:09:20 +0800 Subject: [PATCH 2/5] Adds max initial line length configuration. fixes gh-1554 --- .../config/GatewayAutoConfiguration.java | 5 +++ .../gateway/config/HttpClientProperties.java | 15 +++++++ .../gateway/config/MaxDataSizeValidator.java | 44 +++++++++++++++++++ .../config/GatewayAutoConfigurationTests.java | 6 +++ .../javax.validation.ConstraintValidator | 1 + 5 files changed, 71 insertions(+) create mode 100644 spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/config/MaxDataSizeValidator.java create mode 100644 spring-cloud-gateway-core/src/test/resources/META-INF/services/javax.validation.ConstraintValidator diff --git a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/config/GatewayAutoConfiguration.java b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/config/GatewayAutoConfiguration.java index ec2d2e5b..26af62ef 100644 --- a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/config/GatewayAutoConfiguration.java +++ b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/config/GatewayAutoConfiguration.java @@ -609,6 +609,11 @@ public class GatewayAutoConfiguration { spec.maxHeaderSize( (int) properties.getMaxHeaderSize().toBytes()); } + if (properties.getMaxInitialLineLength() != null) { + // cast to int is ok, since @Max is Integer.MAX_VALUE + spec.maxInitialLineLength( + (int) properties.getMaxInitialLineLength().toBytes()); + } return spec; }).tcpConfiguration(tcpClient -> { diff --git a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/config/HttpClientProperties.java b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/config/HttpClientProperties.java index 0ecfdc29..6d4f7082 100644 --- a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/config/HttpClientProperties.java +++ b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/config/HttpClientProperties.java @@ -42,11 +42,13 @@ import org.springframework.boot.web.server.WebServerException; import org.springframework.core.style.ToStringCreator; import org.springframework.util.ResourceUtils; import org.springframework.util.unit.DataSize; +import org.springframework.validation.annotation.Validated; /** * Configuration properties for the Netty {@link reactor.netty.http.client.HttpClient}. */ @ConfigurationProperties("spring.cloud.gateway.httpclient") +@Validated public class HttpClientProperties { /** The connect timeout in millis, the default is 45s. */ @@ -58,6 +60,9 @@ public class HttpClientProperties { /** The max response header size. */ private DataSize maxHeaderSize; + /** The max initial line length. */ + private DataSize maxInitialLineLength; + /** Pool configuration for Netty HttpClient. */ private Pool pool = new Pool(); @@ -98,6 +103,15 @@ public class HttpClientProperties { this.maxHeaderSize = maxHeaderSize; } + @Max(Integer.MAX_VALUE) + public DataSize getMaxInitialLineLength() { + return maxInitialLineLength; + } + + public void setMaxInitialLineLength(DataSize maxInitialLineLength) { + this.maxInitialLineLength = maxInitialLineLength; + } + public Pool getPool() { return pool; } @@ -145,6 +159,7 @@ public class HttpClientProperties { .append("connectTimeout", connectTimeout) .append("responseTimeout", responseTimeout) .append("maxHeaderSize", maxHeaderSize) + .append("maxInitialLineLength", maxInitialLineLength) .append("pool", pool) .append("proxy", proxy) .append("ssl", ssl) diff --git a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/config/MaxDataSizeValidator.java b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/config/MaxDataSizeValidator.java new file mode 100644 index 00000000..238bfe33 --- /dev/null +++ b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/config/MaxDataSizeValidator.java @@ -0,0 +1,44 @@ +/* + * Copyright 2013-2020 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 + * + * https://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.gateway.config; + +import javax.validation.ConstraintValidator; +import javax.validation.ConstraintValidatorContext; +import javax.validation.constraints.Max; + +import org.springframework.util.unit.DataSize; + +// https://in.relation.to/2017/03/02/adding-custom-constraint-definitions-via-the-java-service-loader/ +public class MaxDataSizeValidator implements ConstraintValidator { + + private long maxValue; + + @Override + public boolean isValid(DataSize value, ConstraintValidatorContext context) { + // null values are valid + if (value == null) { + return true; + } + return value.toBytes() <= maxValue; + } + + @Override + public void initialize(Max maxValue) { + this.maxValue = maxValue.value(); + } + +} diff --git a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/config/GatewayAutoConfigurationTests.java b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/config/GatewayAutoConfigurationTests.java index 7c574267..75f5b0c0 100644 --- a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/config/GatewayAutoConfigurationTests.java +++ b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/config/GatewayAutoConfigurationTests.java @@ -91,11 +91,17 @@ public class GatewayAutoConfigurationTests { "spring.cloud.gateway.httpclient.connect-timeout=10", "spring.cloud.gateway.httpclient.response-timeout=10s", "spring.cloud.gateway.httpclient.pool.type=fixed", + // greather than integer max value + "spring.cloud.gateway.httpclient.max-initial-line-length=2147483647", "spring.cloud.gateway.httpclient.proxy.host=myhost", "spring.cloud.gateway.httpclient.websocket.max-frame-payload-length=1024") .run(context -> { assertThat(context).hasSingleBean(HttpClient.class); HttpClient httpClient = context.getBean(HttpClient.class); + HttpClientProperties properties = context + .getBean(HttpClientProperties.class); + assertThat(properties.getMaxInitialLineLength().toBytes()) + .isLessThanOrEqualTo(Integer.MAX_VALUE); /* * FIXME: 2.1.0 HttpClientOptions options = httpClient.options(); * diff --git a/spring-cloud-gateway-core/src/test/resources/META-INF/services/javax.validation.ConstraintValidator b/spring-cloud-gateway-core/src/test/resources/META-INF/services/javax.validation.ConstraintValidator new file mode 100644 index 00000000..6e43c9f5 --- /dev/null +++ b/spring-cloud-gateway-core/src/test/resources/META-INF/services/javax.validation.ConstraintValidator @@ -0,0 +1 @@ +org.springframework.cloud.gateway.config.MaxDataSizeValidator \ No newline at end of file From 35eab962a168d1fe6edc341612dca5c5c96b5a3c Mon Sep 17 00:00:00 2001 From: trotman23 Date: Tue, 4 Feb 2020 17:41:10 -0600 Subject: [PATCH 3/5] Adds ServiceInstance metadata to RouteDefenition in DiscoveryClientRouteDefenitionLocator. fixes gh-1555 --- ...DiscoveryClientRouteDefinitionLocator.java | 21 +++++++++++++------ ...veryClientRouteDefinitionLocatorTests.java | 1 + 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/discovery/DiscoveryClientRouteDefinitionLocator.java b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/discovery/DiscoveryClientRouteDefinitionLocator.java index 8109fc18..0ad99d41 100644 --- a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/discovery/DiscoveryClientRouteDefinitionLocator.java +++ b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/discovery/DiscoveryClientRouteDefinitionLocator.java @@ -17,6 +17,7 @@ package org.springframework.cloud.gateway.discovery; import java.net.URI; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.function.Predicate; @@ -121,12 +122,8 @@ public class DiscoveryClientRouteDefinitionLocator implements RouteDefinitionLoc return serviceInstances.filter(instances -> !instances.isEmpty()) .map(instances -> instances.get(0)).filter(includePredicate) .map(instance -> { - String serviceId = instance.getServiceId(); - - RouteDefinition routeDefinition = new RouteDefinition(); - routeDefinition.setId(this.routeIdPrefix + serviceId); - String uri = urlExpr.getValue(evalCtxt, instance, String.class); - routeDefinition.setUri(URI.create(uri)); + RouteDefinition routeDefinition = buildRouteDefinition(urlExpr, + instance); final ServiceInstance instanceForEval = new DelegatingServiceInstance( instance, properties); @@ -159,6 +156,18 @@ public class DiscoveryClientRouteDefinitionLocator implements RouteDefinitionLoc }); } + protected RouteDefinition buildRouteDefinition(Expression urlExpr, + ServiceInstance serviceInstance) { + String serviceId = serviceInstance.getServiceId(); + RouteDefinition routeDefinition = new RouteDefinition(); + routeDefinition.setId(this.routeIdPrefix + serviceId); + String uri = urlExpr.getValue(this.evalCtxt, serviceInstance, String.class); + routeDefinition.setUri(URI.create(uri)); + // add instance metadata + routeDefinition.setMetadata(new LinkedHashMap<>(serviceInstance.getMetadata())); + return routeDefinition; + } + String getValueFromExpr(SimpleEvaluationContext evalCtxt, SpelExpressionParser parser, ServiceInstance instance, Map.Entry entry) { try { diff --git a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/discovery/DiscoveryClientRouteDefinitionLocatorTests.java b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/discovery/DiscoveryClientRouteDefinitionLocatorTests.java index 412763aa..194312ab 100644 --- a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/discovery/DiscoveryClientRouteDefinitionLocatorTests.java +++ b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/discovery/DiscoveryClientRouteDefinitionLocatorTests.java @@ -75,6 +75,7 @@ public class DiscoveryClientRouteDefinitionLocatorTests { RouteDefinition definition = definitions.get(0); assertThat(definition.getId()).isEqualTo("testedge_SERVICE1"); assertThat(definition.getUri()).hasScheme("lb").hasHost("SERVICE1"); + assertThat(definition.getMetadata()).containsEntry("edge", "true"); assertThat(definition.getPredicates()).hasSize(1); PredicateDefinition predicate = definition.getPredicates().get(0); From fc7ed5b0a6c9f78ef772b86a0df6d43c25e23e4c Mon Sep 17 00:00:00 2001 From: Nikita Konev Date: Tue, 5 Nov 2019 04:56:33 +0300 Subject: [PATCH 4/5] Fixes retry filter retries on all operations when a response timeout occurs. fixes gh-1372 fixes gh-1393 --- .../factory/RetryGatewayFilterFactory.java | 14 ++++++++-- ...yGatewayFilterFactoryIntegrationTests.java | 27 ++++++++++++++++++- 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/factory/RetryGatewayFilterFactory.java b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/factory/RetryGatewayFilterFactory.java index 7852ef5e..68161d2b 100644 --- a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/factory/RetryGatewayFilterFactory.java +++ b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/factory/RetryGatewayFilterFactory.java @@ -121,7 +121,10 @@ public class RetryGatewayFilterFactory Retry exceptionRetry = null; if (!retryConfig.getExceptions().isEmpty()) { Predicate> retryContextPredicate = context -> { - if (exceedsMaxIterations(context.applicationContext(), retryConfig)) { + + ServerWebExchange exchange = context.applicationContext(); + + if (exceedsMaxIterations(exchange, retryConfig)) { return false; } @@ -133,7 +136,14 @@ public class RetryGatewayFilterFactory trace("exception or its cause is retryable %s, configured exceptions %s", () -> getExceptionNameWithCause(exception), retryConfig::getExceptions); - return true; + + HttpMethod httpMethod = exchange.getRequest().getMethod(); + boolean retryableMethod = retryConfig.getMethods() + .contains(httpMethod); + trace("retryableMethod: %b, httpMethod %s, configured methods %s", + () -> retryableMethod, () -> httpMethod, + retryConfig::getMethods); + return retryableMethod; } } trace("exception or its cause is not retryable %s, configured exceptions %s", diff --git a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/factory/RetryGatewayFilterFactoryIntegrationTests.java b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/factory/RetryGatewayFilterFactoryIntegrationTests.java index 98c006b5..1ca17cf3 100644 --- a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/factory/RetryGatewayFilterFactoryIntegrationTests.java +++ b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/factory/RetryGatewayFilterFactoryIntegrationTests.java @@ -147,6 +147,27 @@ public class RetryGatewayFilterFactoryIntegrationTests extends BaseWebClientTest assertThat(TestConfig.map.get("sleepyRequest")).isNotNull().hasValue(3); } + @Test + public void shouldNotRetryWhenSleepyRequestPost() throws Exception { + testClient.mutate().responseTimeout(Duration.ofSeconds(10)).build().post() + .uri("/sleep?key=notRetriesSleepyRequestPost&millis=3000") + .header(HttpHeaders.HOST, "www.retry-only-get.org").exchange() + .expectStatus().isEqualTo(HttpStatus.GATEWAY_TIMEOUT); + + assertThat(TestConfig.map.get("notRetriesSleepyRequestPost")).isNotNull() + .hasValue(1); + } + + @Test + public void shouldRetryWhenSleepyRequestGet() throws Exception { + testClient.mutate().responseTimeout(Duration.ofSeconds(10)).build().get() + .uri("/sleep?key=sleepyRequestGet&millis=3000") + .header(HttpHeaders.HOST, "www.retry-only-get.org").exchange() + .expectStatus().isEqualTo(HttpStatus.GATEWAY_TIMEOUT); + + assertThat(TestConfig.map.get("sleepyRequestGet")).isNotNull().hasValue(3); + } + @Test @SuppressWarnings("unchecked") public void retryFilterLoadBalancedWithMultipleServers() { @@ -252,7 +273,11 @@ public class RetryGatewayFilterFactoryIntegrationTests extends BaseWebClientTest .retry(config -> config.setRetries(2) .setMethods(HttpMethod.POST, HttpMethod.GET))) .uri(uri)) - + .route("retry_only_get", r -> r.host("**.retry-only-get.org") + .filters(f -> f.prefixPath("/httpbin") + .retry(config -> config.setRetries(2) + .setMethods(HttpMethod.GET))) + .uri(uri)) .route("retry_with_backoff", r -> r.host("**.retrywithbackoff.org") .filters(f -> f.prefixPath("/httpbin").retry(config -> { config.setRetries(2).setBackoff(Duration.ofMillis(100), From 8ba1c06183cc4b5db2ea725551161e7932e49284 Mon Sep 17 00:00:00 2001 From: dcutic Date: Wed, 2 Oct 2019 08:47:10 +0200 Subject: [PATCH 5/5] Adds support for rate limits bellow 1 req/s Adds requestedTokens configuration option. Previously, this was hardcoded to 1. Make the 'requested tokens' redis template argument configurable in order to allow defining rate limits lower than 1 req/s, e.g. 1 req/m. This is accomplished by setting: - replenishRate = requestRate - burstRate = requestRate * timeSpanInSeconds - requestedTokens = timeSpanInSeconds For 1 req/m this would be accomplished by: - replenishRate = 1 - burstRate = 60 - requestedTokens = 60 fixes gh-1327 --- .../main/asciidoc/spring-cloud-gateway.adoc | 10 +- .../filter/ratelimit/RedisRateLimiter.java | 61 ++++++++- .../RedisRateLimiterConfigTests.java | 23 ++-- ...isRateLimiterDefaultFilterConfigTests.java | 10 +- .../ratelimit/RedisRateLimiterTests.java | 123 +++++++++++++----- .../ratelimit/RedisRateLimiterUnitTests.java | 113 ++++++++++++++++ .../application-redis-rate-limiter-config.yml | 9 ++ ...tion-redis-rate-limiter-default-config.yml | 1 + 8 files changed, 296 insertions(+), 54 deletions(-) create mode 100644 spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/ratelimit/RedisRateLimiterUnitTests.java diff --git a/docs/src/main/asciidoc/spring-cloud-gateway.adoc b/docs/src/main/asciidoc/spring-cloud-gateway.adoc index 3c5d20da..10074746 100644 --- a/docs/src/main/asciidoc/spring-cloud-gateway.adoc +++ b/docs/src/main/asciidoc/spring-cloud-gateway.adoc @@ -994,18 +994,23 @@ It requires the use of the `spring-boot-starter-data-redis-reactive` Spring Boot The algorithm used is the https://en.wikipedia.org/wiki/Token_bucket[Token Bucket Algorithm]. -The `redis-rate-limiter.replenishRate` is how many requests per second you want a user to be allowed to do, without any dropped requests. +The `redis-rate-limiter.replenishRate` property is how many requests per second you want a user to be allowed to do, without any dropped requests. This is the rate at which the token bucket is filled. -The `redis-rate-limiter.burstCapacity` is the maximum number of requests a user is allowed to do in a single second. +The `redis-rate-limiter.burstCapacity` property is the maximum number of requests a user is allowed to do in a single second. This is the number of tokens the token bucket can hold. Setting this value to zero blocks all requests. +The `redis-rate-limiter.requestedTokens` property is how many tokens a request costs. +This is the number of tokens taken from the bucket for each request and defaults to `1`. + A steady rate is accomplished by setting the same value in `replenishRate` and `burstCapacity`. Temporary bursts can be allowed by setting `burstCapacity` higher than `replenishRate`. In this case, the rate limiter needs to be allowed some time between bursts (according to `replenishRate`), as two consecutive bursts will result in dropped requests (`HTTP 429 - Too Many Requests`). The following listing configures a `redis-rate-limiter`: +Rate limits bellow `1 request/s` are accomplished by setting `replenishRate` to the wanted number of requests, `requestedTokens` to the timespan in seconds and `burstCapacity` to the product of `replenishRate` and `requestedTokens`, e.g. setting `replenishRate=1`, `requestedTokens=60` and `burstCapacity=60` will result in a limit of `1 request/min`. + .application.yml ==== [source,yaml] @@ -1021,6 +1026,7 @@ spring: args: redis-rate-limiter.replenishRate: 10 redis-rate-limiter.burstCapacity: 20 + redis-rate-limiter.requestedTokens: 1 ---- ==== diff --git a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/ratelimit/RedisRateLimiter.java b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/ratelimit/RedisRateLimiter.java index 522f70ed..85b4a623 100644 --- a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/ratelimit/RedisRateLimiter.java +++ b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/ratelimit/RedisRateLimiter.java @@ -38,6 +38,7 @@ import org.springframework.cloud.gateway.route.RouteDefinitionRouteLocator; import org.springframework.cloud.gateway.support.ConfigurationService; import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContextAware; +import org.springframework.core.style.ToStringCreator; import org.springframework.data.redis.core.ReactiveStringRedisTemplate; import org.springframework.data.redis.core.script.RedisScript; import org.springframework.validation.Validator; @@ -49,6 +50,7 @@ import org.springframework.validation.annotation.Validated; * * @author Spencer Gibb * @author Ronny Bräunlich + * @author Denis Cutic */ @ConfigurationProperties("spring.cloud.gateway.redis-rate-limiter") public class RedisRateLimiter extends AbstractRateLimiter @@ -87,10 +89,15 @@ public class RedisRateLimiter extends AbstractRateLimiter> script, ConfigurationService configurationService) { super(Config.class, CONFIGURATION_PROPERTY_NAME, configurationService); @@ -141,7 +151,7 @@ public class RedisRateLimiter extends AbstractRateLimiter getKeys(String id) { // use `{}` around keys to use Redis Key hash tags // this allows for using redis cluster @@ -194,6 +217,14 @@ public class RedisRateLimiter extends AbstractRateLimiter keys = getKeys(id); // The arguments to the LUA script. time() returns unixtime in seconds. List scriptArgs = Arrays.asList(replenishRate + "", - burstCapacity + "", Instant.now().getEpochSecond() + "", "1"); + burstCapacity + "", Instant.now().getEpochSecond() + "", + requestedTokens + ""); // allowed, tokens_left = redis.eval(SCRIPT, keys, args) Flux> flux = this.redisTemplate.execute(this.script, keys, scriptArgs); @@ -298,6 +333,8 @@ public class RedisRateLimiter extends AbstractRateLimiter r.getId().equals(key)).next() .block(); @@ -100,7 +106,8 @@ public class RedisRateLimiterConfigTests { return builder.routes().route("custom_redis_rate_limiter", r -> r.path("/custom").filters(f -> f.requestRateLimiter() .rateLimiter(RedisRateLimiter.class, - rl -> rl.setBurstCapacity(40).setReplenishRate(20)) + rl -> rl.setBurstCapacity(40).setReplenishRate(20) + .setRequestedTokens(10)) .and()).uri("http://localhost")) .route("alt_custom_redis_rate_limiter", r -> r.path("/custom") @@ -113,7 +120,7 @@ public class RedisRateLimiterConfigTests { @Bean public RedisRateLimiter myRateLimiter() { - return new RedisRateLimiter(30, 60); + return new RedisRateLimiter(30, 60, 20); } } diff --git a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/ratelimit/RedisRateLimiterDefaultFilterConfigTests.java b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/ratelimit/RedisRateLimiterDefaultFilterConfigTests.java index e1e90a7d..4c0f3506 100644 --- a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/ratelimit/RedisRateLimiterDefaultFilterConfigTests.java +++ b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/ratelimit/RedisRateLimiterDefaultFilterConfigTests.java @@ -34,6 +34,7 @@ import static org.assertj.core.api.Assertions.assertThat; /** * @author Spencer Gibb + * @author Denis Cutic */ @RunWith(SpringRunner.class) @SpringBootTest @@ -49,22 +50,23 @@ public class RedisRateLimiterDefaultFilterConfigTests { @Before public void init() { - routeLocator.getRoutes().collectList().block(); // prime routes since getRoutes() - // no longer blocks + // prime routes since getRoutes() no longer blocks + routeLocator.getRoutes().collectList().block(); } @Test public void redisRateConfiguredFromEnvironmentDefaultFilters() { String routeId = "redis_rate_limiter_config_default_test"; RedisRateLimiter.Config config = rateLimiter.loadConfiguration(routeId); - assertConfigAndRoute(routeId, 70, 80, config); + assertConfigAndRoute(routeId, 70, 80, 10, config); } private void assertConfigAndRoute(String key, int replenishRate, int burstCapacity, - RedisRateLimiter.Config config) { + int requestedTokens, RedisRateLimiter.Config config) { assertThat(config).isNotNull(); assertThat(config.getReplenishRate()).isEqualTo(replenishRate); assertThat(config.getBurstCapacity()).isEqualTo(burstCapacity); + assertThat(config.getRequestedTokens()).isEqualTo(requestedTokens); Route route = routeLocator.getRoutes().filter(r -> r.getId().equals(key)).next() .block(); diff --git a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/ratelimit/RedisRateLimiterTests.java b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/ratelimit/RedisRateLimiterTests.java index 425e9a2c..7636a858 100644 --- a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/ratelimit/RedisRateLimiterTests.java +++ b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/ratelimit/RedisRateLimiterTests.java @@ -18,6 +18,8 @@ package org.springframework.cloud.gateway.filter.ratelimit; import java.util.UUID; +import org.junit.After; +import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; @@ -45,6 +47,7 @@ import static org.springframework.boot.test.context.SpringBootTest.WebEnvironmen * * @author Spencer Gibb * @author Ronny Bräunlich + * @author Denis Cutic */ @RunWith(SpringRunner.class) @SpringBootTest(webEnvironment = RANDOM_PORT) @@ -57,52 +60,48 @@ public class RedisRateLimiterTests extends BaseWebClientTests { @Autowired private RedisRateLimiter rateLimiter; + @Before + public void setUp() throws Exception { + assumeThat("Ignore on Circle", System.getenv("CIRCLECI"), is(nullValue())); + } + + @After + public void tearDown() throws Exception { + rateLimiter.setIncludeHeaders(true); + } + @Test public void redisRateLimiterWorks() throws Exception { - assumeThat("Ignore on Circle", System.getenv("CIRCLECI"), is(nullValue())); - String id = UUID.randomUUID().toString(); int replenishRate = 10; int burstCapacity = 2 * replenishRate; + int requestedTokens = 1; String routeId = "myroute"; - rateLimiter.getConfig().put(routeId, new RedisRateLimiter.Config() - .setBurstCapacity(burstCapacity).setReplenishRate(replenishRate)); + rateLimiter.getConfig().put(routeId, + new RedisRateLimiter.Config().setBurstCapacity(burstCapacity) + .setReplenishRate(replenishRate) + .setRequestedTokens(requestedTokens)); - // Bursts work - for (int i = 0; i < burstCapacity; i++) { - Response response = rateLimiter.isAllowed(routeId, id).block(); - assertThat(response.isAllowed()).as("Burst # %s is allowed", i).isTrue(); - assertThat(response.getHeaders()) - .containsKey(RedisRateLimiter.REMAINING_HEADER); - assertThat(response.getHeaders()).containsEntry( - RedisRateLimiter.REPLENISH_RATE_HEADER, - String.valueOf(replenishRate)); - assertThat(response.getHeaders()).containsEntry( - RedisRateLimiter.BURST_CAPACITY_HEADER, - String.valueOf(burstCapacity)); - } + checkLimitEnforced(id, replenishRate, burstCapacity, requestedTokens, routeId); + } - Response response = rateLimiter.isAllowed(routeId, id).block(); - if (response.isAllowed()) { // TODO: sometimes there is an off by one error - response = rateLimiter.isAllowed(routeId, id).block(); - } - assertThat(response.isAllowed()).as("Burst # %s is not allowed", burstCapacity) - .isFalse(); + @Test + public void redisRateLimiterWorksForLowRates() throws Exception { + String id = UUID.randomUUID().toString(); - Thread.sleep(1000); + int replenishRate = 1; + int burstCapacity = 3; + int requestedTokens = 3; - // # After the burst is done, check the steady state - for (int i = 0; i < replenishRate; i++) { - response = rateLimiter.isAllowed(routeId, id).block(); - assertThat(response.isAllowed()).as("steady state # %s is allowed", i) - .isTrue(); - } + String routeId = "low_rate_route"; + rateLimiter.getConfig().put(routeId, + new RedisRateLimiter.Config().setBurstCapacity(burstCapacity) + .setReplenishRate(replenishRate) + .setRequestedTokens(requestedTokens)); - response = rateLimiter.isAllowed(routeId, id).block(); - assertThat(response.isAllowed()).as("steady state # %s is allowed", replenishRate) - .isFalse(); + checkLimitEnforced(id, replenishRate, burstCapacity, requestedTokens, routeId); } @Test @@ -113,8 +112,6 @@ public class RedisRateLimiterTests extends BaseWebClientTests { @Test public void redisRateLimiterDoesNotSendHeadersIfDeactivated() throws Exception { - assumeThat("Ignore on Circle", System.getenv("CIRCLECI"), is(nullValue())); - String id = UUID.randomUUID().toString(); String routeId = "myroute"; @@ -128,6 +125,62 @@ public class RedisRateLimiterTests extends BaseWebClientTests { .doesNotContainKey(RedisRateLimiter.REPLENISH_RATE_HEADER); assertThat(response.getHeaders()) .doesNotContainKey(RedisRateLimiter.BURST_CAPACITY_HEADER); + assertThat(response.getHeaders()) + .doesNotContainKey(RedisRateLimiter.REQUESTED_TOKENS_HEADER); + } + + private void checkLimitEnforced(String id, int replenishRate, int burstCapacity, + int requestedTokens, String routeId) throws InterruptedException { + // Bursts work + simulateBurst(id, replenishRate, burstCapacity, requestedTokens, routeId); + + checkLimitReached(id, burstCapacity, routeId); + + Thread.sleep(Math.max(1, requestedTokens / replenishRate) * 1000); + + // # After the burst is done, check the steady state + checkSteadyState(id, replenishRate, routeId); + } + + private void simulateBurst(String id, int replenishRate, int burstCapacity, + int requestedTokens, String routeId) { + for (int i = 0; i < burstCapacity / requestedTokens; i++) { + Response response = rateLimiter.isAllowed(routeId, id).block(); + assertThat(response.isAllowed()).as("Burst # %s is allowed", i).isTrue(); + assertThat(response.getHeaders()) + .containsKey(RedisRateLimiter.REMAINING_HEADER); + assertThat(response.getHeaders()).containsEntry( + RedisRateLimiter.REPLENISH_RATE_HEADER, + String.valueOf(replenishRate)); + assertThat(response.getHeaders()).containsEntry( + RedisRateLimiter.BURST_CAPACITY_HEADER, + String.valueOf(burstCapacity)); + assertThat(response.getHeaders()).containsEntry( + RedisRateLimiter.REQUESTED_TOKENS_HEADER, + String.valueOf(requestedTokens)); + } + } + + private void checkLimitReached(String id, int burstCapacity, String routeId) { + Response response = rateLimiter.isAllowed(routeId, id).block(); + if (response.isAllowed()) { // TODO: sometimes there is an off by one error + response = rateLimiter.isAllowed(routeId, id).block(); + } + assertThat(response.isAllowed()).as("Burst # %s is not allowed", burstCapacity) + .isFalse(); + } + + private void checkSteadyState(String id, int replenishRate, String routeId) { + Response response; + for (int i = 0; i < replenishRate; i++) { + response = rateLimiter.isAllowed(routeId, id).block(); + assertThat(response.isAllowed()).as("steady state # %s is allowed", i) + .isTrue(); + } + + response = rateLimiter.isAllowed(routeId, id).block(); + assertThat(response.isAllowed()).as("steady state # %s is allowed", replenishRate) + .isFalse(); } @EnableAutoConfiguration diff --git a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/ratelimit/RedisRateLimiterUnitTests.java b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/ratelimit/RedisRateLimiterUnitTests.java new file mode 100644 index 00000000..cdc0b203 --- /dev/null +++ b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/ratelimit/RedisRateLimiterUnitTests.java @@ -0,0 +1,113 @@ +/* + * Copyright 2013-2019 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 + * + * https://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.gateway.filter.ratelimit; + +import io.lettuce.core.RedisException; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.junit.MockitoJUnitRunner; +import reactor.core.publisher.Mono; + +import org.springframework.cloud.gateway.support.ConfigurationService; +import org.springframework.context.ApplicationContext; +import org.springframework.data.redis.core.ReactiveStringRedisTemplate; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.data.MapEntry.entry; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyList; +import static org.mockito.Mockito.when; + +/** + * @author Denis Cutic + */ +@RunWith(MockitoJUnitRunner.class) +public class RedisRateLimiterUnitTests { + + private static final int DEFAULT_REPLENISH_RATE = 1; + + private static final int DEFAULT_BURST_CAPACITY = 1; + + public static final String ROUTE_ID = "routeId"; + + public static final String REQUEST_ID = "id"; + + public static final String[] CONFIGURATION_SERVICE_BEANS = new String[0]; + + public static final RedisException REDIS_EXCEPTION = new RedisException( + "Mocked problem"); + + @Mock + private ApplicationContext applicationContext; + + @Mock + private ReactiveStringRedisTemplate redisTemplate; + + private RedisRateLimiter redisRateLimiter; + + @Before + public void setUp() { + when(applicationContext.getBean(ReactiveStringRedisTemplate.class)) + .thenReturn(redisTemplate); + when(applicationContext.getBeanNamesForType(ConfigurationService.class)) + .thenReturn(CONFIGURATION_SERVICE_BEANS); + redisRateLimiter = new RedisRateLimiter(DEFAULT_REPLENISH_RATE, + DEFAULT_BURST_CAPACITY); + } + + @After + public void tearDown() { + Mockito.reset(applicationContext); + } + + @Test(expected = IllegalStateException.class) + public void shouldThrowWhenNotInitialized() { + redisRateLimiter.isAllowed(ROUTE_ID, REQUEST_ID); + } + + @Test + public void shouldAllowRequestWhenRedisIssueOccurs() { + when(redisTemplate.execute(any(), anyList(), anyList())) + .thenThrow(REDIS_EXCEPTION); + redisRateLimiter.setApplicationContext(applicationContext); + Mono response = redisRateLimiter.isAllowed(ROUTE_ID, + REQUEST_ID); + assertThat(response.block()).extracting(RateLimiter.Response::isAllowed) + .isEqualTo(true); + } + + @Test + public void shouldReturnHeadersWhenRedisIssueOccurs() { + when(redisTemplate.execute(any(), anyList(), anyList())) + .thenThrow(REDIS_EXCEPTION); + redisRateLimiter.setApplicationContext(applicationContext); + Mono response = redisRateLimiter.isAllowed(ROUTE_ID, + REQUEST_ID); + assertThat(response.block().getHeaders()).containsOnly( + entry(redisRateLimiter.getRemainingHeader(), "-1"), + entry(redisRateLimiter.getBurstCapacityHeader(), + DEFAULT_BURST_CAPACITY + ""), + entry(redisRateLimiter.getReplenishRateHeader(), + DEFAULT_REPLENISH_RATE + ""), + entry(redisRateLimiter.getRequestedTokensHeader(), "1")); + } + +} diff --git a/spring-cloud-gateway-core/src/test/resources/application-redis-rate-limiter-config.yml b/spring-cloud-gateway-core/src/test/resources/application-redis-rate-limiter-config.yml index 85ac957d..a6c5726d 100644 --- a/spring-cloud-gateway-core/src/test/resources/application-redis-rate-limiter-config.yml +++ b/spring-cloud-gateway-core/src/test/resources/application-redis-rate-limiter-config.yml @@ -14,4 +14,13 @@ spring: redis-rate-limiter: replenish-rate: 10 burst-capacity: 20 + - id: redis_rate_limiter_minimal_config_test + uri: ${test.uri} + predicates: + - Path=/ + filters: + - name: RequestRateLimiter + args: + redis-rate-limiter: + replenish-rate: 2 diff --git a/spring-cloud-gateway-core/src/test/resources/application-redis-rate-limiter-default-config.yml b/spring-cloud-gateway-core/src/test/resources/application-redis-rate-limiter-default-config.yml index e1ba4079..94ae146a 100644 --- a/spring-cloud-gateway-core/src/test/resources/application-redis-rate-limiter-default-config.yml +++ b/spring-cloud-gateway-core/src/test/resources/application-redis-rate-limiter-default-config.yml @@ -7,6 +7,7 @@ spring: redis-rate-limiter: replenish-rate: 70 burst-capacity: 80 + requested-tokens: 10 routes: # ===================================== - id: redis_rate_limiter_config_default_test