Issue-906 Adding support for query param in lb health-check
This PR is to fix issue https://github.com/spring-cloud/spring-cloud-commons/issues/906 where loadbalancer healthcheck has query param and character "?" was automatically converted to a URI encoding %3F, causing the request to fail. What has changed * Using `UriComponentsBuilder.fromUriString` this method parses the string and handles url and query param path.
This commit is contained in:
committed by
Olga MaciaszekSharma
parent
bd8d640c42
commit
de32d2b68c
@@ -27,6 +27,7 @@ import org.apache.commons.logging.LogFactory;
|
||||
import reactor.core.publisher.Mono;
|
||||
|
||||
import org.springframework.beans.factory.ObjectProvider;
|
||||
import org.springframework.cloud.client.ServiceInstance;
|
||||
import org.springframework.cloud.client.discovery.DiscoveryClient;
|
||||
import org.springframework.cloud.client.discovery.ReactiveDiscoveryClient;
|
||||
import org.springframework.cloud.client.loadbalancer.LoadBalancerProperties;
|
||||
@@ -35,6 +36,7 @@ import org.springframework.cloud.loadbalancer.config.LoadBalancerZoneConfig;
|
||||
import org.springframework.context.ConfigurableApplicationContext;
|
||||
import org.springframework.http.HttpStatus;
|
||||
import org.springframework.util.Assert;
|
||||
import org.springframework.util.StringUtils;
|
||||
import org.springframework.web.client.RestTemplate;
|
||||
import org.springframework.web.reactive.function.client.WebClient;
|
||||
import org.springframework.web.util.UriComponentsBuilder;
|
||||
@@ -287,7 +289,7 @@ public final class ServiceInstanceListSupplierBuilder {
|
||||
ServiceInstanceListSupplier delegate, LoadBalancerProperties properties) {
|
||||
return new HealthCheckServiceInstanceListSupplier(delegate, properties.getHealthCheck(),
|
||||
(serviceInstance, healthCheckPath) -> webClient.get()
|
||||
.uri(UriComponentsBuilder.fromUri(serviceInstance.getUri()).path(healthCheckPath).build()
|
||||
.uri(UriComponentsBuilder.fromUriString(getUri(serviceInstance, healthCheckPath)).build()
|
||||
.toUri())
|
||||
.exchange().flatMap(clientResponse -> clientResponse.releaseBody()
|
||||
.thenReturn(HttpStatus.OK.value() == clientResponse.rawStatusCode())));
|
||||
@@ -297,7 +299,7 @@ public final class ServiceInstanceListSupplierBuilder {
|
||||
ServiceInstanceListSupplier delegate, LoadBalancerProperties properties) {
|
||||
return new HealthCheckServiceInstanceListSupplier(delegate, properties.getHealthCheck(),
|
||||
(serviceInstance, healthCheckPath) -> Mono.defer(() -> {
|
||||
URI uri = UriComponentsBuilder.fromUri(serviceInstance.getUri()).path(healthCheckPath).build()
|
||||
URI uri = UriComponentsBuilder.fromUriString(getUri(serviceInstance, healthCheckPath)).build()
|
||||
.toUri();
|
||||
try {
|
||||
return Mono
|
||||
@@ -309,6 +311,14 @@ public final class ServiceInstanceListSupplierBuilder {
|
||||
}));
|
||||
}
|
||||
|
||||
private String getUri(ServiceInstance serviceInstance, String healthCheckPath) {
|
||||
if (StringUtils.hasText(healthCheckPath)) {
|
||||
String path = healthCheckPath.startsWith("/") ? healthCheckPath : "/" + healthCheckPath;
|
||||
return serviceInstance.getUri().toString() + path;
|
||||
}
|
||||
return serviceInstance.getUri().toString();
|
||||
}
|
||||
|
||||
/**
|
||||
* Allows creating a {@link ServiceInstanceListSupplier} instance based on provided
|
||||
* {@link ConfigurableApplicationContext}.
|
||||
|
||||
@@ -46,6 +46,7 @@ import org.springframework.cloud.loadbalancer.support.ServiceInstanceListSupplie
|
||||
import org.springframework.context.annotation.Configuration;
|
||||
import org.springframework.test.context.junit.jupiter.SpringExtension;
|
||||
import org.springframework.web.bind.annotation.GetMapping;
|
||||
import org.springframework.web.bind.annotation.RequestParam;
|
||||
import org.springframework.web.bind.annotation.RestController;
|
||||
import org.springframework.web.client.RestTemplate;
|
||||
import org.springframework.web.reactive.function.client.WebClient;
|
||||
@@ -529,6 +530,22 @@ class HealthCheckServiceInstanceListSupplierTests {
|
||||
.untilAsserted(() -> Assertions.assertThat(instancesCanceled).hasValue(1));
|
||||
}
|
||||
|
||||
@SuppressWarnings("ConstantConditions")
|
||||
@Test
|
||||
void shouldCheckInstanceWithProvidedHealthCheckPathWithQueryParams() {
|
||||
String serviceId = "ignored-service";
|
||||
healthCheck.getPath().put("ignored-service", "/health?someparam=somevalue");
|
||||
ServiceInstance serviceInstance = new DefaultServiceInstance("ignored-service-1", serviceId, "127.0.0.1", port,
|
||||
false);
|
||||
listSupplier = new HealthCheckServiceInstanceListSupplier(
|
||||
ServiceInstanceListSuppliers.from(serviceId, serviceInstance), healthCheck,
|
||||
healthCheckFunction(webClient));
|
||||
|
||||
boolean alive = listSupplier.isAlive(serviceInstance).block();
|
||||
|
||||
assertThat(alive).isTrue();
|
||||
}
|
||||
|
||||
@Configuration(proxyBeanMethods = false)
|
||||
@EnableAutoConfiguration
|
||||
@RestController
|
||||
@@ -539,7 +556,7 @@ class HealthCheckServiceInstanceListSupplierTests {
|
||||
}
|
||||
|
||||
@GetMapping("/health")
|
||||
void healthCheck() {
|
||||
void healthCheck(@RequestParam(value = "someparam", required = false) String param) {
|
||||
|
||||
}
|
||||
|
||||
|
||||
@@ -23,6 +23,7 @@ import reactor.core.publisher.Mono;
|
||||
|
||||
import org.springframework.cloud.client.ServiceInstance;
|
||||
import org.springframework.http.HttpStatus;
|
||||
import org.springframework.util.StringUtils;
|
||||
import org.springframework.web.client.RestTemplate;
|
||||
import org.springframework.web.reactive.function.client.WebClient;
|
||||
import org.springframework.web.util.UriComponentsBuilder;
|
||||
@@ -41,14 +42,14 @@ final class ServiceInstanceListSuppliersTestUtils {
|
||||
|
||||
static BiFunction<ServiceInstance, String, Mono<Boolean>> healthCheckFunction(WebClient webClient) {
|
||||
return (serviceInstance, healthCheckPath) -> webClient.get()
|
||||
.uri(UriComponentsBuilder.fromUri(serviceInstance.getUri()).path(healthCheckPath).build().toUri())
|
||||
.uri(UriComponentsBuilder.fromUriString(getUri(serviceInstance, healthCheckPath)).build().toUri())
|
||||
.exchange().flatMap(clientResponse -> clientResponse.releaseBody()
|
||||
.thenReturn(HttpStatus.OK.value() == clientResponse.rawStatusCode()));
|
||||
}
|
||||
|
||||
static BiFunction<ServiceInstance, String, Mono<Boolean>> healthCheckFunction(RestTemplate restTemplate) {
|
||||
return (serviceInstance, healthCheckPath) -> Mono.defer(() -> {
|
||||
URI uri = UriComponentsBuilder.fromUri(serviceInstance.getUri()).path(healthCheckPath).build().toUri();
|
||||
URI uri = UriComponentsBuilder.fromUriString(getUri(serviceInstance, healthCheckPath)).build().toUri();
|
||||
try {
|
||||
return Mono.just(HttpStatus.OK.equals(restTemplate.getForEntity(uri, Void.class).getStatusCode()));
|
||||
}
|
||||
@@ -58,4 +59,12 @@ final class ServiceInstanceListSuppliersTestUtils {
|
||||
});
|
||||
}
|
||||
|
||||
private static String getUri(ServiceInstance serviceInstance, String healthCheckPath) {
|
||||
if (StringUtils.hasText(healthCheckPath)) {
|
||||
String path = healthCheckPath.startsWith("/") ? healthCheckPath : "/" + healthCheckPath;
|
||||
return serviceInstance.getUri().toString() + path;
|
||||
}
|
||||
return serviceInstance.getUri().toString();
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user