From 064684a31b9ddf95e73c96184807036ad60fd2c0 Mon Sep 17 00:00:00 2001 From: Marta Medio Date: Wed, 26 Apr 2023 20:07:35 +0200 Subject: [PATCH 1/2] Adding grpc-status header value (adapted for 3.1.x) (#2938) --- .../gateway/tests/grpc/GRPCApplication.java | 12 +- .../tests/grpc/GRPCApplicationTests.java | 30 ++++- .../grpc/JsonToGrpcApplicationTests.java | 61 +--------- .../gateway/tests/grpc/RouteConfigurer.java | 114 ++++++++++++++++++ .../headers/GRPCResponseHeadersFilter.java | 24 +++- 5 files changed, 176 insertions(+), 65 deletions(-) create mode 100644 spring-cloud-gateway-integration-tests/grpc/src/test/java/org/springframework/cloud/gateway/tests/grpc/RouteConfigurer.java diff --git a/spring-cloud-gateway-integration-tests/grpc/src/main/java/org/springframework/cloud/gateway/tests/grpc/GRPCApplication.java b/spring-cloud-gateway-integration-tests/grpc/src/main/java/org/springframework/cloud/gateway/tests/grpc/GRPCApplication.java index a417fa3e..1f7bd2b3 100644 --- a/spring-cloud-gateway-integration-tests/grpc/src/main/java/org/springframework/cloud/gateway/tests/grpc/GRPCApplication.java +++ b/spring-cloud-gateway-integration-tests/grpc/src/main/java/org/springframework/cloud/gateway/tests/grpc/GRPCApplication.java @@ -23,6 +23,8 @@ import java.util.concurrent.TimeUnit; import io.grpc.Grpc; import io.grpc.Server; import io.grpc.ServerCredentials; +import io.grpc.Status; +import io.grpc.StatusRuntimeException; import io.grpc.TlsServerCredentials; import io.grpc.stub.StreamObserver; import org.slf4j.Logger; @@ -70,9 +72,6 @@ public class GRPCApplication { private void start() throws IOException { Integer serverPort = environment.getProperty("local.server.port", Integer.class); int grpcPort = serverPort + 1; - /* - * The port on which the server should run. We run - */ ServerCredentials creds = createServerCredentials(); server = Grpc.newServerBuilderForPort(grpcPort, creds).addService(new HelloService()).build().start(); @@ -105,6 +104,13 @@ public class GRPCApplication { @Override public void hello(HelloRequest request, StreamObserver responseObserver) { + if ("failWithRuntimeException!".equals(request.getFirstName())) { + StatusRuntimeException exception = Status.FAILED_PRECONDITION.withDescription("Invalid firstName") + .asRuntimeException(); + responseObserver.onError(exception); + responseObserver.onCompleted(); + return; + } String greeting = String.format("Hello, %s %s", request.getFirstName(), request.getLastName()); log.info("Sending response: " + greeting); diff --git a/spring-cloud-gateway-integration-tests/grpc/src/test/java/org/springframework/cloud/gateway/tests/grpc/GRPCApplicationTests.java b/spring-cloud-gateway-integration-tests/grpc/src/test/java/org/springframework/cloud/gateway/tests/grpc/GRPCApplicationTests.java index f540862a..83d74942 100644 --- a/spring-cloud-gateway-integration-tests/grpc/src/test/java/org/springframework/cloud/gateway/tests/grpc/GRPCApplicationTests.java +++ b/spring-cloud-gateway-integration-tests/grpc/src/test/java/org/springframework/cloud/gateway/tests/grpc/GRPCApplicationTests.java @@ -23,15 +23,18 @@ import javax.net.ssl.TrustManager; import javax.net.ssl.X509TrustManager; import io.grpc.ManagedChannel; +import io.grpc.StatusRuntimeException; import io.grpc.netty.GrpcSslContexts; import io.grpc.netty.NettyChannelBuilder; import org.assertj.core.api.Assertions; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.boot.web.server.LocalServerPort; import org.springframework.test.annotation.DirtiesContext; +import static io.grpc.Status.FAILED_PRECONDITION; import static io.grpc.netty.NegotiationType.TLS; import static org.springframework.boot.test.context.SpringBootTest.WebEnvironment; @@ -43,11 +46,18 @@ import static org.springframework.boot.test.context.SpringBootTest.WebEnvironmen public class GRPCApplicationTests { @LocalServerPort - private int port; + private int gatewayPort; + + @BeforeEach + void setUp() { + int grpcServerPort = gatewayPort + 1; + final RouteConfigurer configurer = new RouteConfigurer(gatewayPort); + configurer.addRoute(grpcServerPort, "/**", null); + } @Test - public void gRPCUnaryCalShouldReturnResponse() throws SSLException { - ManagedChannel channel = createSecuredChannel(port + 1); + public void gRPCUnaryCallShouldReturnResponse() throws SSLException { + ManagedChannel channel = createSecuredChannel(gatewayPort); final HelloResponse response = HelloServiceGrpc.newBlockingStub(channel) .hello(HelloRequest.newBuilder().setFirstName("Sir").setLastName("FromClient").build()); @@ -63,6 +73,20 @@ public class GRPCApplicationTests { .build(); } + @Test + public void gRPCUnaryCallShouldHandleRuntimeException() throws SSLException { + ManagedChannel channel = createSecuredChannel(gatewayPort); + + try { + HelloServiceGrpc.newBlockingStub(channel) + .hello(HelloRequest.newBuilder().setFirstName("failWithRuntimeException!").build()); + } + catch (StatusRuntimeException e) { + Assertions.assertThat(FAILED_PRECONDITION.getCode()).isEqualTo(e.getStatus().getCode()); + Assertions.assertThat("Invalid firstName").isEqualTo(e.getStatus().getDescription()); + } + } + private TrustManager[] createTrustAllTrustManager() { return new TrustManager[] { new X509TrustManager() { public X509Certificate[] getAcceptedIssuers() { diff --git a/spring-cloud-gateway-integration-tests/grpc/src/test/java/org/springframework/cloud/gateway/tests/grpc/JsonToGrpcApplicationTests.java b/spring-cloud-gateway-integration-tests/grpc/src/test/java/org/springframework/cloud/gateway/tests/grpc/JsonToGrpcApplicationTests.java index b0ec1e62..210ff8a3 100644 --- a/spring-cloud-gateway-integration-tests/grpc/src/test/java/org/springframework/cloud/gateway/tests/grpc/JsonToGrpcApplicationTests.java +++ b/spring-cloud-gateway-integration-tests/grpc/src/test/java/org/springframework/cloud/gateway/tests/grpc/JsonToGrpcApplicationTests.java @@ -19,11 +19,6 @@ package org.springframework.cloud.gateway.tests.grpc; import java.security.KeyManagementException; import java.security.KeyStoreException; import java.security.NoSuchAlgorithmException; -import java.util.Arrays; -import java.util.Collections; -import java.util.HashMap; -import java.util.Map; -import java.util.UUID; import javax.net.ssl.SSLContext; @@ -44,12 +39,7 @@ import org.junit.jupiter.api.Test; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.boot.web.server.LocalServerPort; -import org.springframework.http.HttpEntity; -import org.springframework.http.HttpMethod; -import org.springframework.http.HttpStatus; -import org.springframework.http.ResponseEntity; import org.springframework.http.client.HttpComponentsClientHttpRequestFactory; -import org.springframework.test.web.reactive.server.WebTestClient; import org.springframework.web.client.RestTemplate; import static org.springframework.boot.test.context.SpringBootTest.WebEnvironment; @@ -62,7 +52,7 @@ import static org.springframework.boot.test.context.SpringBootTest.WebEnvironmen public class JsonToGrpcApplicationTests { @LocalServerPort - private int port; + private int gatewayPort; private RestTemplate restTemplate; @@ -76,11 +66,12 @@ public class JsonToGrpcApplicationTests { // Since GRPC server and GW run in same instance and don't know server port until // test starts, // we need to configure route dynamically using the actuator endpoint. - final RouteConfigurer configurer = new RouteConfigurer(port); - configurer.addRoute(port + 1, "/json/hello", + final RouteConfigurer configurer = new RouteConfigurer(gatewayPort); + int grpcServerPort = gatewayPort + 1; + configurer.addRoute(grpcServerPort, "/json/hello", "JsonToGrpc=file:src/main/proto/hello.pb,file:src/main/proto/hello.proto,HelloService,hello"); - String response = restTemplate.postForEntity("https://localhost:" + this.port + "/json/hello", + String response = restTemplate.postForEntity("https://localhost:" + this.gatewayPort + "/json/hello", "{\"firstName\":\"Duff\", \"lastName\":\"McKagan\"}", String.class).getBody(); Assertions.assertThat(response).isNotNull(); @@ -112,46 +103,4 @@ public class JsonToGrpcApplicationTests { return new RestTemplate(requestFactory); } - class RouteConfigurer { - - private final WebTestClient actuatorWebClient; - - private final int actuatorPort; - - RouteConfigurer(int actuatorPort) { - this.actuatorPort = actuatorPort; - this.actuatorWebClient = WebTestClient.bindToServer().baseUrl("http://localhost:" + actuatorPort).build(); - - } - - public void addRoute(int uriPort, String path, String filter) { - final String routeId = "test-route-" + UUID.randomUUID(); - - Map route = new HashMap<>(); - route.put("id", routeId); - route.put("uri", "http://localhost:" + uriPort); - route.put("predicates", Collections.singletonList("Path=" + path)); - route.put("filters", Arrays.asList(filter)); - - ResponseEntity exchange = restTemplate.exchange(url("/actuator/gateway/routes/" + routeId), - HttpMethod.POST, new HttpEntity<>(route), String.class); - - assert exchange.getStatusCode() == HttpStatus.CREATED; - - refreshRoutes(); - } - - private void refreshRoutes() { - ResponseEntity exchange = restTemplate.exchange(url("/actuator/gateway/refresh"), HttpMethod.POST, - new HttpEntity<>(""), String.class); - - assert exchange.getStatusCode() == HttpStatus.OK; - } - - private String url(String context) { - return String.format("https://localhost:%s%s", this.actuatorPort, context); - } - - } - } diff --git a/spring-cloud-gateway-integration-tests/grpc/src/test/java/org/springframework/cloud/gateway/tests/grpc/RouteConfigurer.java b/spring-cloud-gateway-integration-tests/grpc/src/test/java/org/springframework/cloud/gateway/tests/grpc/RouteConfigurer.java new file mode 100644 index 00000000..cf3a494d --- /dev/null +++ b/spring-cloud-gateway-integration-tests/grpc/src/test/java/org/springframework/cloud/gateway/tests/grpc/RouteConfigurer.java @@ -0,0 +1,114 @@ +/* + * Copyright 2013-2023 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.tests.grpc; + +import java.security.KeyManagementException; +import java.security.KeyStoreException; +import java.security.NoSuchAlgorithmException; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import java.util.UUID; + +import javax.net.ssl.SSLContext; + +import org.apache.http.config.Registry; +import org.apache.http.config.RegistryBuilder; +import org.apache.http.conn.HttpClientConnectionManager; +import org.apache.http.conn.socket.ConnectionSocketFactory; +import org.apache.http.conn.socket.PlainConnectionSocketFactory; +import org.apache.http.conn.ssl.NoopHostnameVerifier; +import org.apache.http.conn.ssl.SSLConnectionSocketFactory; +import org.apache.http.conn.ssl.TrustStrategy; +import org.apache.http.impl.client.CloseableHttpClient; +import org.apache.http.impl.client.HttpClients; +import org.apache.http.impl.conn.BasicHttpClientConnectionManager; +import org.apache.http.ssl.SSLContexts; + +import org.springframework.http.HttpEntity; +import org.springframework.http.HttpMethod; +import org.springframework.http.HttpStatus; +import org.springframework.http.ResponseEntity; +import org.springframework.http.client.HttpComponentsClientHttpRequestFactory; +import org.springframework.web.client.RestTemplate; + +public class RouteConfigurer { + + private final int actuatorPort; + + private final RestTemplate restTemplate; + + RouteConfigurer(int actuatorPort) { + this.actuatorPort = actuatorPort; + this.restTemplate = createUnsecureClient(); + } + + public void addRoute(int grpcServerPort, String path, String filter) { + final String routeId = "test-route-" + UUID.randomUUID(); + + Map route = new HashMap<>(); + route.put("id", routeId); + route.put("uri", "https://localhost:" + grpcServerPort); + route.put("predicates", Collections.singletonList("Path=" + path)); + if (filter != null) { + route.put("filters", Arrays.asList(filter)); + } + + ResponseEntity exchange = restTemplate.exchange(url("/actuator/gateway/routes/" + routeId), + HttpMethod.POST, new HttpEntity<>(route), String.class); + + assert exchange.getStatusCode() == HttpStatus.CREATED; + + refreshRoutes(); + } + + private void refreshRoutes() { + ResponseEntity exchange = restTemplate.exchange(url("/actuator/gateway/refresh"), HttpMethod.POST, + new HttpEntity<>(""), String.class); + + assert exchange.getStatusCode() == HttpStatus.OK; + } + + private String url(String context) { + return String.format("https://localhost:%s%s", this.actuatorPort, context); + } + + private RestTemplate createUnsecureClient() { + TrustStrategy acceptingTrustStrategy = (cert, authType) -> true; + SSLContext sslContext; + try { + sslContext = SSLContexts.custom().loadTrustMaterial(null, acceptingTrustStrategy).build(); + } + catch (NoSuchAlgorithmException | KeyStoreException | KeyManagementException e) { + throw new RuntimeException(e); + } + SSLConnectionSocketFactory sslSocketFactory = new SSLConnectionSocketFactory(sslContext, + NoopHostnameVerifier.INSTANCE); + + Registry socketFactoryRegistry = RegistryBuilder.create() + .register("https", sslSocketFactory).register("http", new PlainConnectionSocketFactory()).build(); + + HttpClientConnectionManager connectionManager = new BasicHttpClientConnectionManager(socketFactoryRegistry); + CloseableHttpClient httpClient = HttpClients.custom().setConnectionManager(connectionManager).build(); + + HttpComponentsClientHttpRequestFactory requestFactory = new HttpComponentsClientHttpRequestFactory(httpClient); + + return new RestTemplate(requestFactory); + } + +} diff --git a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/headers/GRPCResponseHeadersFilter.java b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/headers/GRPCResponseHeadersFilter.java index 5a5a8104..25a45d29 100644 --- a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/headers/GRPCResponseHeadersFilter.java +++ b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/headers/GRPCResponseHeadersFilter.java @@ -31,12 +31,16 @@ import org.springframework.web.server.ServerWebExchange; */ public class GRPCResponseHeadersFilter implements HttpHeadersFilter, Ordered { + private static final String GRPC_STATUS_HEADER = "grpc-status"; + + private static final String GRPC_MESSAGE_HEADER = "grpc-message"; + @Override public HttpHeaders filter(HttpHeaders headers, ServerWebExchange exchange) { ServerHttpResponse response = exchange.getResponse(); HttpHeaders responseHeaders = response.getHeaders(); if (isGRPC(exchange)) { - String trailerHeaderValue = "grpc-status"; + String trailerHeaderValue = GRPC_STATUS_HEADER + "," + GRPC_MESSAGE_HEADER; String originalTrailerHeaderValue = responseHeaders.getFirst(HttpHeaders.TRAILER); if (originalTrailerHeaderValue != null) { trailerHeaderValue += "," + originalTrailerHeaderValue; @@ -47,8 +51,12 @@ public class GRPCResponseHeadersFilter implements HttpHeadersFilter, Ordered { response = ((ServerHttpResponseDecorator) response).getDelegate(); } if (response instanceof AbstractServerHttpResponse) { - ((HttpServerResponse) ((AbstractServerHttpResponse) response).getNativeResponse()) - .trailerHeaders(h -> h.set("grpc-status", "0")); + String grpcStatus = getGrpcStatus(headers); + String grpcMessage = getGrpcMessage(headers); + ((HttpServerResponse) ((AbstractServerHttpResponse) response).getNativeResponse()).trailerHeaders(h -> { + h.set(GRPC_STATUS_HEADER, grpcStatus); + h.set(GRPC_MESSAGE_HEADER, grpcMessage); + }); } } @@ -60,6 +68,16 @@ public class GRPCResponseHeadersFilter implements HttpHeadersFilter, Ordered { return StringUtils.startsWithIgnoreCase(contentTypeValue, "application/grpc"); } + private String getGrpcStatus(HttpHeaders headers) { + final String grpcStatusValue = headers.getFirst(GRPC_STATUS_HEADER); + return StringUtils.hasText(grpcStatusValue) ? grpcStatusValue : "0"; + } + + private String getGrpcMessage(HttpHeaders headers) { + final String grpcStatusValue = headers.getFirst(GRPC_MESSAGE_HEADER); + return StringUtils.hasText(grpcStatusValue) ? grpcStatusValue : ""; + } + @Override public boolean supports(Type type) { return Type.RESPONSE.equals(type); From 1c121efc6ebbbe48bd198b3607af079dee6a6924 Mon Sep 17 00:00:00 2001 From: spencergibb Date: Wed, 26 Apr 2023 19:40:12 -0400 Subject: [PATCH 2/2] Support response-timeout property as a String Fixes gh-2581 --- .../gateway/filter/NettyRoutingFilter.java | 31 ++++++++++++++----- .../NettyRoutingFilterCompatibleTests.java | 9 ++++++ .../NettyRoutingFilterIntegrationTests.java | 6 ++++ .../application-netty-routing-filter.yml | 11 +++++++ 4 files changed, 49 insertions(+), 8 deletions(-) diff --git a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/NettyRoutingFilter.java b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/NettyRoutingFilter.java index aa26b0b8..12b8f923 100644 --- a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/NettyRoutingFilter.java +++ b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/NettyRoutingFilter.java @@ -260,17 +260,32 @@ public class NettyRoutingFilter implements GlobalFilter, Ordered { } private Duration getResponseTimeout(Route route) { - Object responseTimeoutAttr = route.getMetadata().get(RESPONSE_TIMEOUT_ATTR); - if (responseTimeoutAttr != null && responseTimeoutAttr instanceof Number) { - Long routeResponseTimeout = ((Number) responseTimeoutAttr).longValue(); - if (routeResponseTimeout >= 0) { - return Duration.ofMillis(routeResponseTimeout); - } - else { - return null; + try { + if (route.getMetadata().containsKey(RESPONSE_TIMEOUT_ATTR)) { + Long routeResponseTimeout = getLong(route.getMetadata().get(RESPONSE_TIMEOUT_ATTR)); + if (routeResponseTimeout != null && routeResponseTimeout >= 0) { + return Duration.ofMillis(routeResponseTimeout); + } + else { + return null; + } } } + catch (NumberFormatException e) { + // ignore number format and use global default + } return properties.getResponseTimeout(); } + static Long getLong(Object responseTimeoutAttr) { + Long responseTimeout = null; + if (responseTimeoutAttr instanceof Number) { + responseTimeout = ((Integer) responseTimeoutAttr).longValue(); + } + else if (responseTimeoutAttr != null) { + responseTimeout = Long.parseLong(responseTimeoutAttr.toString()); + } + return responseTimeout; + } + } diff --git a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/NettyRoutingFilterCompatibleTests.java b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/NettyRoutingFilterCompatibleTests.java index 220cd869..5c5f913c 100644 --- a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/NettyRoutingFilterCompatibleTests.java +++ b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/NettyRoutingFilterCompatibleTests.java @@ -30,6 +30,7 @@ import org.springframework.test.context.ActiveProfiles; import org.springframework.web.server.ServerWebExchange; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.springframework.boot.test.context.SpringBootTest.WebEnvironment.RANDOM_PORT; /** @@ -51,6 +52,14 @@ class NettyRoutingFilterCompatibleTests extends BaseWebClientTests { assertThat(NettyRoutingFilter.getInteger(5)).isEqualTo(5); } + @Test + void getLongHandlesStringAndNumber() { + assertThat(NettyRoutingFilter.getLong("5")).isEqualTo(5); + assertThat(NettyRoutingFilter.getLong(5)).isEqualTo(5); + assertThat(NettyRoutingFilter.getLong(null)).isNull(); + assertThatThrownBy(() -> NettyRoutingFilter.getLong("notanumber")).isInstanceOf(NumberFormatException.class); + } + @Test void shouldApplyResponseTimeoutPerRoute() { testClient.get().uri("/route/delay/2").exchange().expectStatus().isEqualTo(HttpStatus.GATEWAY_TIMEOUT) diff --git a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/NettyRoutingFilterIntegrationTests.java b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/NettyRoutingFilterIntegrationTests.java index 52d4d6c5..9b0bb30e 100644 --- a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/NettyRoutingFilterIntegrationTests.java +++ b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/NettyRoutingFilterIntegrationTests.java @@ -120,6 +120,12 @@ public class NettyRoutingFilterIntegrationTests extends BaseWebClientTests { .hasMessageContaining("Timeout on blocking read for 5000000000 NANOSECONDS"); } + @Test + public void shouldApplyResponseTimeoutForPlaceholder() { + testClient.get().uri("/responseheaders/200").header("Host", "www.responsetimeoutplaceholder.org").exchange() + .expectStatus().isEqualTo(HttpStatus.OK); + } + @Test public void shouldApplyGlobalResponseTimeoutForInvalidRouteTimeoutValue() { testClient.get().uri("/invalidRoute/delay/5").exchange().expectStatus().isEqualTo(HttpStatus.GATEWAY_TIMEOUT) diff --git a/spring-cloud-gateway-server/src/test/resources/application-netty-routing-filter.yml b/spring-cloud-gateway-server/src/test/resources/application-netty-routing-filter.yml index 8c25d969..8f81445f 100644 --- a/spring-cloud-gateway-server/src/test/resources/application-netty-routing-filter.yml +++ b/spring-cloud-gateway-server/src/test/resources/application-netty-routing-filter.yml @@ -2,6 +2,8 @@ test: hostport: httpbin.org:80 uri: lb://testservice +my.timeout: 3000 + server: error: include-message: always @@ -71,6 +73,15 @@ spring: metadata: response-timeout: 1000 + # ===================================== + - id: per_route_response_timeout_placeholder + uri: ${test.uri} + predicates: + - Host=**.responsetimeoutplaceholder.org + - Path=/responseheaders/** + metadata: + response-timeout: ${my.timeout} + # ===================================== # should be last and not follow alphabetical order - id: default_path_to_httpbin