From 68531abffec1c93f8516f6bd7ae91f249f3c3b91 Mon Sep 17 00:00:00 2001 From: Benjamin Einaudi Date: Thu, 11 Mar 2021 00:26:34 +0100 Subject: [PATCH] Fixes rewrite path example and discovery (#2144) Given that we would like to make the following rewriting (in both discovery and declarative context) - '/foo' to '/' - '/foo/' to '/' - '/foo/anything/else' to '/anything/else' We need to - force the presence of a leading '/' after rewriting as we may result in an empty uri path otherwise - exclude it from 'remaining' group See #1282 See #1359 See #1360 --- .../main/asciidoc/spring-cloud-gateway.adoc | 6 +- ...tewayDiscoveryClientAutoConfiguration.java | 2 +- ...veryClientRouteDefinitionLocatorTests.java | 2 +- ...yDiscoveryClientAutoConfigurationTest.java | 93 +++++++++++++++++++ 4 files changed, 98 insertions(+), 5 deletions(-) create mode 100644 spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/discovery/GatewayDiscoveryClientAutoConfigurationTest.java diff --git a/docs/src/main/asciidoc/spring-cloud-gateway.adoc b/docs/src/main/asciidoc/spring-cloud-gateway.adoc index 3cd5e4f6..a8a3cfe5 100644 --- a/docs/src/main/asciidoc/spring-cloud-gateway.adoc +++ b/docs/src/main/asciidoc/spring-cloud-gateway.adoc @@ -1132,7 +1132,7 @@ spring: predicates: - Path=/red/** filters: - - RewritePath=/red(?/?.*), $\{segment} + - RewritePath=/red/?(?.*), /$\{segment} ---- ==== @@ -2226,7 +2226,7 @@ By default, the gateway defines a single predicate and filter for routes created The default predicate is a path predicate defined with the pattern `/serviceId/**`, where `serviceId` is the ID of the service from the `DiscoveryClient`. -The default filter is a rewrite path filter with the regex `/serviceId/(?.*)` and the replacement `/${remaining}`. +The default filter is a rewrite path filter with the regex `/serviceId/?(?.*)` and the replacement `/${remaining}`. This strips the service ID from the path before the request is sent downstream. If you want to customize the predicates or filters used by the `DiscoveryClient` routes, set `spring.cloud.gateway.discovery.locator.predicates[x]` and `spring.cloud.gateway.discovery.locator.filters[y]`. @@ -2244,7 +2244,7 @@ spring.cloud.gateway.discovery.locator.predicates[1].args[pattern]: "'**.foo.com spring.cloud.gateway.discovery.locator.filters[0].name: CircuitBreaker spring.cloud.gateway.discovery.locator.filters[0].args[name]: serviceId spring.cloud.gateway.discovery.locator.filters[1].name: RewritePath -spring.cloud.gateway.discovery.locator.filters[1].args[regexp]: "'/' + serviceId + '/(?.*)'" +spring.cloud.gateway.discovery.locator.filters[1].args[regexp]: "'/' + serviceId + '/?(?.*)'" spring.cloud.gateway.discovery.locator.filters[1].args[replacement]: "'/${remaining}'" ---- ==== diff --git a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/discovery/GatewayDiscoveryClientAutoConfiguration.java b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/discovery/GatewayDiscoveryClientAutoConfiguration.java index a8da0ec7..94cc64e2 100644 --- a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/discovery/GatewayDiscoveryClientAutoConfiguration.java +++ b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/discovery/GatewayDiscoveryClientAutoConfiguration.java @@ -70,7 +70,7 @@ public class GatewayDiscoveryClientAutoConfiguration { // add a filter that removes /serviceId by default FilterDefinition filter = new FilterDefinition(); filter.setName(normalizeFilterFactoryName(RewritePathGatewayFilterFactory.class)); - String regex = "'/' + serviceId + '/(?.*)'"; + String regex = "'/' + serviceId + '/?(?.*)'"; String replacement = "'/${remaining}'"; filter.addArg(REGEXP_KEY, regex); filter.addArg(REPLACEMENT_KEY, replacement); diff --git a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/discovery/DiscoveryClientRouteDefinitionLocatorTests.java b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/discovery/DiscoveryClientRouteDefinitionLocatorTests.java index 7a9c9955..d46ab4bd 100644 --- a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/discovery/DiscoveryClientRouteDefinitionLocatorTests.java +++ b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/discovery/DiscoveryClientRouteDefinitionLocatorTests.java @@ -83,7 +83,7 @@ public class DiscoveryClientRouteDefinitionLocatorTests { assertThat(definition.getFilters()).hasSize(1); FilterDefinition filter = definition.getFilters().get(0); assertThat(filter.getName()).isEqualTo("RewritePath"); - assertThat(filter.getArgs()).hasSize(2).containsEntry(REGEXP_KEY, "/service1/(?.*)") + assertThat(filter.getArgs()).hasSize(2).containsEntry(REGEXP_KEY, "/service1/?(?.*)") .containsEntry(REPLACEMENT_KEY, "/${remaining}"); } diff --git a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/discovery/GatewayDiscoveryClientAutoConfigurationTest.java b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/discovery/GatewayDiscoveryClientAutoConfigurationTest.java new file mode 100644 index 00000000..474309c7 --- /dev/null +++ b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/discovery/GatewayDiscoveryClientAutoConfigurationTest.java @@ -0,0 +1,93 @@ +/* + * 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.discovery; + +import java.util.Objects; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import org.springframework.cloud.client.ServiceInstance; +import org.springframework.cloud.gateway.filter.FilterDefinition; +import org.springframework.expression.spel.standard.SpelExpressionParser; +import org.springframework.expression.spel.support.SimpleEvaluationContext; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import static org.springframework.cloud.gateway.filter.factory.RewritePathGatewayFilterFactory.REGEXP_KEY; +import static org.springframework.cloud.gateway.filter.factory.RewritePathGatewayFilterFactory.REPLACEMENT_KEY; + +public class GatewayDiscoveryClientAutoConfigurationTest { + + private static final String SERVICE_ID = "foo"; + + private static final String BASE_URI = "/" + SERVICE_ID; + + private static final SpelExpressionParser PARSER = new SpelExpressionParser(); + + private static final SimpleEvaluationContext CONTEXT = SimpleEvaluationContext.forReadOnlyDataBinding().withInstanceMethods().build(); + + private static final FilterDefinition DEFAULT_FILTER = GatewayDiscoveryClientAutoConfiguration.initFilters().get(0); + + private ServiceInstance serviceInstance; + + @BeforeEach + public void buildServiceInstance(){ + this.serviceInstance = mock(ServiceInstance.class); + when(this.serviceInstance.getServiceId()).thenReturn(SERVICE_ID); + } + + @Test + public void defaultRewritePathShouldHandleEmptyRemainingWithoutSlash(){ + String expectedRemotePath = "/"; + + final String result = replace(BASE_URI); + + assertThat(result).isEqualTo(expectedRemotePath); + } + + @Test + public void defaultRewritePathShouldHandleEmptyRemainingWithSlash(){ + String extraUri = "/"; + + final String result = replace(BASE_URI + extraUri); + + assertThat(result).isEqualTo(extraUri); + } + + + @Test + public void defaultRewritePathShouldHandleNonEmptyRemainingPath(){ + String extraUri = "/some/additional/uri"; + + final String result = replace(BASE_URI + extraUri); + + assertThat(result).isEqualTo(extraUri); + } + + private String replace(String enteringPath) { + return enteringPath.replaceAll(evaluateExpression(DEFAULT_FILTER.getArgs().get(REGEXP_KEY)), + evaluateExpression(DEFAULT_FILTER.getArgs().get(REPLACEMENT_KEY))); + } + + private String evaluateExpression(String expression) { + return Objects.requireNonNull(PARSER.parseExpression(expression) + .getValue(CONTEXT, serviceInstance, String.class)); + } + +}