Makes failing on route definition errors opt-out.

Since this is a behavior change, the new behavior needs to be opt in.

See gh-1376
This commit is contained in:
Spencer Gibb
2020-02-27 23:48:08 -05:00
parent 75a34df7e2
commit 05ba82855e
5 changed files with 75 additions and 105 deletions

View File

@@ -29,6 +29,7 @@ import org.apache.commons.logging.LogFactory;
import org.springframework.boot.context.properties.ConfigurationProperties;
import org.springframework.cloud.gateway.filter.FilterDefinition;
import org.springframework.cloud.gateway.route.RouteDefinition;
import org.springframework.core.style.ToStringCreator;
import org.springframework.http.MediaType;
import org.springframework.validation.annotation.Validated;
@@ -56,6 +57,12 @@ public class GatewayProperties {
private List<MediaType> streamingMediaTypes = Arrays
.asList(MediaType.TEXT_EVENT_STREAM, MediaType.APPLICATION_STREAM_JSON);
/**
* Option to fail on route definition errors, defaults to true. Otherwise, a warning
* is logged.
*/
private boolean failOnRouteDefinitionError = true;
public List<RouteDefinition> getRoutes() {
return routes;
}
@@ -83,10 +90,22 @@ public class GatewayProperties {
this.streamingMediaTypes = streamingMediaTypes;
}
public boolean isFailOnRouteDefinitionError() {
return failOnRouteDefinitionError;
}
public void setFailOnRouteDefinitionError(boolean failOnRouteDefinitionError) {
this.failOnRouteDefinitionError = failOnRouteDefinitionError;
}
@Override
public String toString() {
return "GatewayProperties{" + "routes=" + routes + ", defaultFilters="
+ defaultFilters + ", streamingMediaTypes=" + streamingMediaTypes + '}';
return new ToStringCreator(this).append("routes", routes)
.append("defaultFilters", defaultFilters)
.append("streamingMediaTypes", streamingMediaTypes)
.append("failOnRouteDefinitionError", failOnRouteDefinitionError)
.toString();
}
}

View File

@@ -55,7 +55,8 @@ public class CompositeRouteDefinitionLocator implements RouteDefinitionLocator {
return randomId().map(id -> {
routeDefinition.setId(id);
if (log.isDebugEnabled()) {
log.debug("Id set on route definition: " + routeDefinition);
log.debug(
"Id set on route definition: " + routeDefinition);
}
return routeDefinition;
});
@@ -65,6 +66,8 @@ public class CompositeRouteDefinitionLocator implements RouteDefinitionLocator {
}
protected Mono<String> randomId() {
return Mono.fromSupplier(idGenerator::toString).publishOn(Schedulers.boundedElastic());
return Mono.fromSupplier(idGenerator::toString)
.publishOn(Schedulers.boundedElastic());
}
}

View File

@@ -143,23 +143,26 @@ public class RouteDefinitionRouteLocator
@Override
public Flux<Route> getRoutes() {
return this.routeDefinitionLocator.getRouteDefinitions().map(this::convertToRoute)
.onErrorContinue((error, obj) -> {
if (logger.isWarnEnabled()) {
logger.warn("RouteDefinition id " + ((RouteDefinition) obj).getId() + " will be ignored. Definition has invalid configs, " + error.getMessage());
}
})
.map(route -> {
if (logger.isDebugEnabled()) {
logger.debug("RouteDefinition matched: " + route.getId());
}
return route;
});
Flux<Route> routes = this.routeDefinitionLocator.getRouteDefinitions()
.map(this::convertToRoute);
/*
* TODO: trace logging if (logger.isTraceEnabled()) {
* logger.trace("RouteDefinition did not match: " + routeDefinition.getId()); }
*/
if (!gatewayProperties.isFailOnRouteDefinitionError()) {
// instead of letting error bubble up, continue
routes = routes.onErrorContinue((error, obj) -> {
if (logger.isWarnEnabled()) {
logger.warn("RouteDefinition id " + ((RouteDefinition) obj).getId()
+ " will be ignored. Definition has invalid configs, "
+ error.getMessage());
}
});
}
return routes.map(route -> {
if (logger.isDebugEnabled()) {
logger.debug("RouteDefinition matched: " + route.getId());
}
return route;
});
}
private Route convertToRoute(RouteDefinition routeDefinition) {

View File

@@ -68,24 +68,23 @@ public class RouteDefinitionRouteLocatorTests {
}
}));
PropertiesRouteDefinitionLocator routeDefinitionLocator = new PropertiesRouteDefinitionLocator(gatewayProperties);
PropertiesRouteDefinitionLocator routeDefinitionLocator = new PropertiesRouteDefinitionLocator(
gatewayProperties);
@SuppressWarnings("deprecation")
RouteDefinitionRouteLocator routeDefinitionRouteLocator = new RouteDefinitionRouteLocator(
new CompositeRouteDefinitionLocator(Flux.just(routeDefinitionLocator)),
predicates, gatewayFilterFactories, gatewayProperties,
new ConfigurationService());
StepVerifier.create(routeDefinitionRouteLocator.getRoutes())
.assertNext(route -> {
List<GatewayFilter> filters = route.getFilters();
assertThat(filters).hasSize(3);
assertThat(getFilterClassName(filters.get(0))).contains("RemoveResponseHeader");
assertThat(getFilterClassName(filters.get(1))).contains("AddResponseHeader");
assertThat(getFilterClassName(filters.get(2)))
.contains("RouteDefinitionRouteLocatorTests$TestOrderedGateway");
})
.expectComplete()
.verify();
StepVerifier.create(routeDefinitionRouteLocator.getRoutes()).assertNext(route -> {
List<GatewayFilter> filters = route.getFilters();
assertThat(filters).hasSize(3);
assertThat(getFilterClassName(filters.get(0)))
.contains("RemoveResponseHeader");
assertThat(getFilterClassName(filters.get(1))).contains("AddResponseHeader");
assertThat(getFilterClassName(filters.get(2)))
.contains("RouteDefinitionRouteLocatorTests$TestOrderedGateway");
}).expectComplete().verify();
}
@Test
@@ -98,99 +97,43 @@ public class RouteDefinitionRouteLocatorTests {
new TestOrderedGatewayFilterFactory());
GatewayProperties gatewayProperties = new GatewayProperties();
gatewayProperties.setRoutes(containsInvalidRoutes());
gatewayProperties.setFailOnRouteDefinitionError(false);
PropertiesRouteDefinitionLocator routeDefinitionLocator = new PropertiesRouteDefinitionLocator(gatewayProperties);
PropertiesRouteDefinitionLocator routeDefinitionLocator = new PropertiesRouteDefinitionLocator(
gatewayProperties);
@SuppressWarnings("deprecation")
RouteDefinitionRouteLocator routeDefinitionRouteLocator = new RouteDefinitionRouteLocator(
new CompositeRouteDefinitionLocator(Flux.just(routeDefinitionLocator)),
predicates, gatewayFilterFactories, gatewayProperties,
new ConfigurationService());
StepVerifier.create(routeDefinitionRouteLocator.getRoutes())
.assertNext(route -> {
List<GatewayFilter> filters = route.getFilters();
assertThat(filters).hasSize(3);
assertThat(getFilterClassName(filters.get(0))).contains("RemoveResponseHeader");
assertThat(getFilterClassName(filters.get(1))).contains("AddResponseHeader");
assertThat(getFilterClassName(filters.get(2)))
.contains("RouteDefinitionRouteLocatorTests$TestOrderedGateway");
})
.expectComplete()
.verify();
StepVerifier.create(routeDefinitionRouteLocator.getRoutes()).assertNext(route -> {
List<GatewayFilter> filters = route.getFilters();
assertThat(filters).hasSize(3);
assertThat(getFilterClassName(filters.get(0)))
.contains("RemoveResponseHeader");
assertThat(getFilterClassName(filters.get(1))).contains("AddResponseHeader");
assertThat(getFilterClassName(filters.get(2)))
.contains("RouteDefinitionRouteLocatorTests$TestOrderedGateway");
}).expectComplete().verify();
}
private List<RouteDefinition> containsInvalidRoutes() {
RouteDefinition foo = new RouteDefinition();
foo.setId("foo");
foo.setUri(URI.create("https://foo.example.com"));
foo.setPredicates(
Arrays.asList(new PredicateDefinition("Host=*.example.com")));
foo.setFilters(Arrays.asList(
new FilterDefinition("RemoveResponseHeader=Server"),
foo.setPredicates(Arrays.asList(new PredicateDefinition("Host=*.example.com")));
foo.setFilters(Arrays.asList(new FilterDefinition("RemoveResponseHeader=Server"),
new FilterDefinition("TestOrdered="),
new FilterDefinition("AddResponseHeader=X-Response-Foo, Bar")));
RouteDefinition bad = new RouteDefinition();
bad.setId("exceptionRaised");
bad.setUri(URI.create("https://foo.example.com"));
bad.setPredicates(
Arrays.asList(new PredicateDefinition("Host=*.example.com")));
bad.setPredicates(Arrays.asList(new PredicateDefinition("Host=*.example.com")));
bad.setFilters(Arrays.asList(new FilterDefinition("Generate exception")));
return Arrays.asList(foo, bad);
}
@Test
public void contextLoadsWithErrorRecovery() {
List<RoutePredicateFactory> predicates = Arrays
.asList(new HostRoutePredicateFactory());
List<GatewayFilterFactory> gatewayFilterFactories = Arrays.asList(
new RemoveResponseHeaderGatewayFilterFactory(),
new AddResponseHeaderGatewayFilterFactory(),
new TestOrderedGatewayFilterFactory());
GatewayProperties gatewayProperties = new GatewayProperties();
gatewayProperties.setRoutes(containsInvalidRoutes());
RouteDefinitionRouteLocator routeDefinitionRouteLocator = new RouteDefinitionRouteLocator(
new PropertiesRouteDefinitionLocator(gatewayProperties), predicates,
gatewayFilterFactories, gatewayProperties,
new DefaultConversionService());
List<Route> routes = routeDefinitionRouteLocator.getRoutes().collectList()
.block();
List<GatewayFilter> filters = routes.get(0).getFilters();
assertThat(filters).hasSize(3);
assertThat(getFilterClassName(filters.get(0))).contains("RemoveResponseHeader");
assertThat(getFilterClassName(filters.get(1))).contains("AddResponseHeader");
assertThat(getFilterClassName(filters.get(2)))
.contains("RouteDefinitionRouteLocatorTests$TestOrderedGateway");
}
private List<RouteDefinition> containsInvalidRoutes() {
return Arrays.asList(
new RouteDefinition() {
{
setId("foo");
setUri(URI.create("https://foo.example.com"));
setPredicates(
Arrays.asList(new PredicateDefinition("Host=*.example.com")));
setFilters(Arrays.asList(
new FilterDefinition("RemoveResponseHeader=Server"),
new FilterDefinition("TestOrdered="),
new FilterDefinition("AddResponseHeader=X-Response-Foo, Bar")));
}
},
new RouteDefinition() {
{
setId("exceptionRaised");
setUri(URI.create("https://foo.example.com"));
setPredicates(
Arrays.asList(new PredicateDefinition("Host=*.example.com")));
setFilters(Arrays.asList(new FilterDefinition("Generate exception")));
}
}
);
}
private String getFilterClassName(GatewayFilter target) {
if (target instanceof OrderedGatewayFilter) {
return getFilterClassName(((OrderedGatewayFilter) target).getDelegate());

View File

@@ -35,8 +35,7 @@ public class RouteConstructionIntegrationTests {
@Test
public void routesWithVerificationShouldFail() {
exception.expect(Throwable.class);
new SpringApplicationBuilder(TestConfig.class)
.profiles("verification-route")
new SpringApplicationBuilder(TestConfig.class).profiles("verification-route")
.run();
}
@@ -74,6 +73,9 @@ public class RouteConstructionIntegrationTests {
public void setArg1(String arg1) {
this.arg1 = arg1;
}
}
}
}