From b8b47a4280bdc3abd791149ad7e4dafc5231c24b Mon Sep 17 00:00:00 2001 From: Olga MaciaszekSharma Date: Wed, 27 Oct 2021 17:02:37 +0200 Subject: [PATCH 1/5] Add support methods for handling @ConditionalOnProperty for client-based properties. Use per-client-property for RetryAwareServiceInstanceListSupplier conditionals. --- .../LoadBalancerClientConfiguration.java | 24 ++++++---- .../support/LoadBalancerClientFactory.java | 2 +- .../LoadBalancerEnvironmentPropertyUtils.java | 46 +++++++++++++++++++ .../loadbalancer/core/LoadBalancerTests.java | 2 +- 4 files changed, 64 insertions(+), 10 deletions(-) create mode 100644 spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/support/LoadBalancerEnvironmentPropertyUtils.java diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/annotation/LoadBalancerClientConfiguration.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/annotation/LoadBalancerClientConfiguration.java index 3fc711c31..515ed6809 100644 --- a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/annotation/LoadBalancerClientConfiguration.java +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/annotation/LoadBalancerClientConfiguration.java @@ -35,13 +35,17 @@ import org.springframework.cloud.loadbalancer.core.RoundRobinLoadBalancer; import org.springframework.cloud.loadbalancer.core.ServiceInstanceListSupplier; import org.springframework.cloud.loadbalancer.support.LoadBalancerClientFactory; +import org.springframework.cloud.loadbalancer.support.LoadBalancerEnvironmentPropertyUtils; import org.springframework.context.ConfigurableApplicationContext; import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Condition; +import org.springframework.context.annotation.ConditionContext; import org.springframework.context.annotation.Conditional; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Primary; import org.springframework.core.annotation.Order; import org.springframework.core.env.Environment; +import org.springframework.core.type.AnnotatedTypeMetadata; import org.springframework.retry.support.RetryTemplate; import org.springframework.web.client.RestTemplate; import org.springframework.web.reactive.function.client.WebClient; @@ -232,11 +236,8 @@ static class LoadBalancerRetryEnabled { } - @ConditionalOnProperty(value = "spring.cloud.loadbalancer.retry.avoid-previous-instance", havingValue = "true", - matchIfMissing = true) - static class AvoidPreviousInstanceEnabled { - - } + @Conditional(AvoidPreviousInstanceEnabledCondition.class) + static class AvoidPreviousInstanceEnabled { } } @@ -251,10 +252,17 @@ static class LoadBalancerRetryEnabled { } - @ConditionalOnProperty(value = "spring.cloud.loadbalancer.retry.avoid-previous-instance", havingValue = "true", - matchIfMissing = true) - static class AvoidPreviousInstanceEnabled { + @Conditional(AvoidPreviousInstanceEnabledCondition.class) + static class AvoidPreviousInstanceEnabled { } + + } + static class AvoidPreviousInstanceEnabledCondition implements Condition { + @Override + public boolean matches(ConditionContext context, AnnotatedTypeMetadata metadata) { + return LoadBalancerEnvironmentPropertyUtils + .trueOrMissingForClientOrDefault(context + .getEnvironment(), "retry.avoid-previous-instance"); } } diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/support/LoadBalancerClientFactory.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/support/LoadBalancerClientFactory.java index d2cd44423..c2533d980 100644 --- a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/support/LoadBalancerClientFactory.java +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/support/LoadBalancerClientFactory.java @@ -65,7 +65,7 @@ public LoadBalancerClientFactory(LoadBalancerClientsProperties properties) { this.properties = properties; } - public String getName(Environment environment) { + public static String getName(Environment environment) { return environment.getProperty(PROPERTY_NAME); } diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/support/LoadBalancerEnvironmentPropertyUtils.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/support/LoadBalancerEnvironmentPropertyUtils.java new file mode 100644 index 000000000..6045d44ef --- /dev/null +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/support/LoadBalancerEnvironmentPropertyUtils.java @@ -0,0 +1,46 @@ +package org.springframework.cloud.loadbalancer.support; + +import org.springframework.core.env.Environment; + +/** + * @author Olga Maciaszek-Sharma + */ +public final class LoadBalancerEnvironmentPropertyUtils { + + private LoadBalancerEnvironmentPropertyUtils() { + throw new IllegalStateException("Should not instantiate a utility class"); + } + + public static boolean trueForClientOrDefault(Environment environment, String propertySuffix) { + String defaultValue = environment + .getProperty("spring.cloud.loadbalancer." + propertySuffix); + String clientValue = environment + .getProperty("spring.cloud.loadbalancer.clients." + + LoadBalancerClientFactory + .getName(environment) + "." + propertySuffix); + if (clientValue != null && clientValue + .equalsIgnoreCase(Boolean.TRUE.toString())) { + return true; + } + return clientValue == null && defaultValue != null && defaultValue + .equalsIgnoreCase(Boolean.TRUE.toString()); + } + + public static boolean trueOrMissingForClientOrDefault(Environment environment, String propertySuffix) { + String defaultValue = environment + .getProperty("spring.cloud.loadbalancer." + propertySuffix); + String clientValue = environment + .getProperty("spring.cloud.loadbalancer.clients." + + LoadBalancerClientFactory + .getName(environment) + "." + propertySuffix); + if (clientValue != null && clientValue + .equalsIgnoreCase(Boolean.TRUE.toString())) { + return true; + } + if (clientValue == null && defaultValue != null && defaultValue + .equalsIgnoreCase(Boolean.TRUE.toString())) { + return true; + } + return clientValue == null && defaultValue == null; + } +} diff --git a/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/core/LoadBalancerTests.java b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/core/LoadBalancerTests.java index d2081cbe2..df648106d 100644 --- a/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/core/LoadBalancerTests.java +++ b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/core/LoadBalancerTests.java @@ -188,7 +188,7 @@ protected static class MyServiceConfig { @Bean public RoundRobinLoadBalancer roundRobinContextLoadBalancer(LoadBalancerClientFactory clientFactory, Environment env) { - String serviceId = clientFactory.getName(env); + String serviceId = LoadBalancerClientFactory.getName(env); return new RoundRobinLoadBalancer( clientFactory.getLazyProvider(serviceId, ServiceInstanceListSupplier.class), serviceId, -1); } From b856e02413c0fefcdd07ebcfbba631433b71d2ee Mon Sep 17 00:00:00 2001 From: Olga MaciaszekSharma Date: Thu, 28 Oct 2021 16:08:20 +0200 Subject: [PATCH 2/5] Add more support methods for handling @ConditionalOnProperty for client-based properties. Use per-client-property for property-set configurations, avoiding same instance on retry and service instance cookie for sticky session. --- .../LoadBalancerClientConfiguration.java | 90 ++++++++++++++----- ...ngLoadBalancerClientAutoConfiguration.java | 19 +++- .../LoadBalancerEnvironmentPropertyUtils.java | 66 +++++++++----- 3 files changed, 130 insertions(+), 45 deletions(-) diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/annotation/LoadBalancerClientConfiguration.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/annotation/LoadBalancerClientConfiguration.java index 515ed6809..b8fd4fd8a 100644 --- a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/annotation/LoadBalancerClientConfiguration.java +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/annotation/LoadBalancerClientConfiguration.java @@ -79,8 +79,7 @@ public static class ReactiveSupportConfiguration { @Bean @ConditionalOnBean(ReactiveDiscoveryClient.class) @ConditionalOnMissingBean - @ConditionalOnProperty(value = "spring.cloud.loadbalancer.configurations", havingValue = "default", - matchIfMissing = true) + @Conditional(DefaultConfigurationCondition.class) public ServiceInstanceListSupplier discoveryClientServiceInstanceListSupplier( ConfigurableApplicationContext context) { return ServiceInstanceListSupplier.builder().withDiscoveryClient().withCaching().build(context); @@ -89,7 +88,7 @@ public ServiceInstanceListSupplier discoveryClientServiceInstanceListSupplier( @Bean @ConditionalOnBean(ReactiveDiscoveryClient.class) @ConditionalOnMissingBean - @ConditionalOnProperty(value = "spring.cloud.loadbalancer.configurations", havingValue = "zone-preference") + @Conditional(ZonePreferenceConfigurationCondition.class) public ServiceInstanceListSupplier zonePreferenceDiscoveryClientServiceInstanceListSupplier( ConfigurableApplicationContext context) { return ServiceInstanceListSupplier.builder().withDiscoveryClient().withZonePreference().withCaching() @@ -99,7 +98,7 @@ public ServiceInstanceListSupplier zonePreferenceDiscoveryClientServiceInstanceL @Bean @ConditionalOnBean({ ReactiveDiscoveryClient.class, WebClient.Builder.class }) @ConditionalOnMissingBean - @ConditionalOnProperty(value = "spring.cloud.loadbalancer.configurations", havingValue = "health-check") + @Conditional(HealthCheckConfigurationCondition.class) public ServiceInstanceListSupplier healthCheckDiscoveryClientServiceInstanceListSupplier( ConfigurableApplicationContext context) { return ServiceInstanceListSupplier.builder().withDiscoveryClient().withHealthChecks().build(context); @@ -108,8 +107,7 @@ public ServiceInstanceListSupplier healthCheckDiscoveryClientServiceInstanceList @Bean @ConditionalOnBean(ReactiveDiscoveryClient.class) @ConditionalOnMissingBean - @ConditionalOnProperty(value = "spring.cloud.loadbalancer.configurations", - havingValue = "request-based-sticky-session") + @Conditional(RequestBasedStickySessionConfigurationCondition.class) public ServiceInstanceListSupplier requestBasedStickySessionDiscoveryClientServiceInstanceListSupplier( ConfigurableApplicationContext context) { return ServiceInstanceListSupplier.builder().withDiscoveryClient().withRequestBasedStickySession() @@ -119,8 +117,7 @@ public ServiceInstanceListSupplier requestBasedStickySessionDiscoveryClientServi @Bean @ConditionalOnBean(ReactiveDiscoveryClient.class) @ConditionalOnMissingBean - @ConditionalOnProperty(value = "spring.cloud.loadbalancer.configurations", - havingValue = "same-instance-preference") + @Conditional(SameInstancePreferenceConfigurationCondition.class) public ServiceInstanceListSupplier sameInstancePreferenceServiceInstanceListSupplier( ConfigurableApplicationContext context) { return ServiceInstanceListSupplier.builder().withDiscoveryClient().withSameInstancePreference() @@ -137,8 +134,7 @@ public static class BlockingSupportConfiguration { @Bean @ConditionalOnBean(DiscoveryClient.class) @ConditionalOnMissingBean - @ConditionalOnProperty(value = "spring.cloud.loadbalancer.configurations", havingValue = "default", - matchIfMissing = true) + @Conditional(DefaultConfigurationCondition.class) public ServiceInstanceListSupplier discoveryClientServiceInstanceListSupplier( ConfigurableApplicationContext context) { return ServiceInstanceListSupplier.builder().withBlockingDiscoveryClient().withCaching().build(context); @@ -147,7 +143,7 @@ public ServiceInstanceListSupplier discoveryClientServiceInstanceListSupplier( @Bean @ConditionalOnBean(DiscoveryClient.class) @ConditionalOnMissingBean - @ConditionalOnProperty(value = "spring.cloud.loadbalancer.configurations", havingValue = "zone-preference") + @Conditional(ZonePreferenceConfigurationCondition.class) public ServiceInstanceListSupplier zonePreferenceDiscoveryClientServiceInstanceListSupplier( ConfigurableApplicationContext context) { return ServiceInstanceListSupplier.builder().withBlockingDiscoveryClient().withZonePreference() @@ -157,7 +153,7 @@ public ServiceInstanceListSupplier zonePreferenceDiscoveryClientServiceInstanceL @Bean @ConditionalOnBean({ DiscoveryClient.class, RestTemplate.class }) @ConditionalOnMissingBean - @ConditionalOnProperty(value = "spring.cloud.loadbalancer.configurations", havingValue = "health-check") + @Conditional(HealthCheckConfigurationCondition.class) public ServiceInstanceListSupplier healthCheckDiscoveryClientServiceInstanceListSupplier( ConfigurableApplicationContext context) { return ServiceInstanceListSupplier.builder().withBlockingDiscoveryClient().withBlockingHealthChecks() @@ -167,8 +163,7 @@ public ServiceInstanceListSupplier healthCheckDiscoveryClientServiceInstanceList @Bean @ConditionalOnBean(DiscoveryClient.class) @ConditionalOnMissingBean - @ConditionalOnProperty(value = "spring.cloud.loadbalancer.configurations", - havingValue = "request-based-sticky-session") + @Conditional(RequestBasedStickySessionConfigurationCondition.class) public ServiceInstanceListSupplier requestBasedStickySessionDiscoveryClientServiceInstanceListSupplier( ConfigurableApplicationContext context) { return ServiceInstanceListSupplier.builder().withBlockingDiscoveryClient().withRequestBasedStickySession() @@ -178,8 +173,7 @@ public ServiceInstanceListSupplier requestBasedStickySessionDiscoveryClientServi @Bean @ConditionalOnBean(DiscoveryClient.class) @ConditionalOnMissingBean - @ConditionalOnProperty(value = "spring.cloud.loadbalancer.configurations", - havingValue = "same-instance-preference") + @Conditional(SameInstancePreferenceConfigurationCondition.class) public ServiceInstanceListSupplier sameInstancePreferenceServiceInstanceListSupplier( ConfigurableApplicationContext context) { return ServiceInstanceListSupplier.builder().withBlockingDiscoveryClient().withSameInstancePreference() @@ -237,7 +231,9 @@ static class LoadBalancerRetryEnabled { } @Conditional(AvoidPreviousInstanceEnabledCondition.class) - static class AvoidPreviousInstanceEnabled { } + static class AvoidPreviousInstanceEnabled { + + } } @@ -253,16 +249,68 @@ static class LoadBalancerRetryEnabled { } @Conditional(AvoidPreviousInstanceEnabledCondition.class) - static class AvoidPreviousInstanceEnabled { } + static class AvoidPreviousInstanceEnabled { + + } } static class AvoidPreviousInstanceEnabledCondition implements Condition { + + @Override + public boolean matches(ConditionContext context, AnnotatedTypeMetadata metadata) { + return LoadBalancerEnvironmentPropertyUtils.trueOrMissingForClientOrDefault(context.getEnvironment(), + "retry.avoid-previous-instance"); + } + + } + + static class DefaultConfigurationCondition implements Condition { + + @Override + public boolean matches(ConditionContext context, AnnotatedTypeMetadata metadata) { + return LoadBalancerEnvironmentPropertyUtils.equalToOrMissingForClientOrDefault(context.getEnvironment(), + "configurations", "default"); + } + + } + + static class ZonePreferenceConfigurationCondition implements Condition { + + @Override + public boolean matches(ConditionContext context, AnnotatedTypeMetadata metadata) { + return LoadBalancerEnvironmentPropertyUtils.equalToForClientOrDefault(context.getEnvironment(), + "configurations", "zone-preference"); + } + + } + + static class HealthCheckConfigurationCondition implements Condition { + + @Override + public boolean matches(ConditionContext context, AnnotatedTypeMetadata metadata) { + return LoadBalancerEnvironmentPropertyUtils.equalToForClientOrDefault(context.getEnvironment(), + "configurations", "health-check"); + } + + } + + static class RequestBasedStickySessionConfigurationCondition implements Condition { + + @Override + public boolean matches(ConditionContext context, AnnotatedTypeMetadata metadata) { + return LoadBalancerEnvironmentPropertyUtils.equalToForClientOrDefault(context.getEnvironment(), + "configurations", "request-based-sticky-session"); + } + + } + + static class SameInstancePreferenceConfigurationCondition implements Condition { + @Override public boolean matches(ConditionContext context, AnnotatedTypeMetadata metadata) { - return LoadBalancerEnvironmentPropertyUtils - .trueOrMissingForClientOrDefault(context - .getEnvironment(), "retry.avoid-previous-instance"); + return LoadBalancerEnvironmentPropertyUtils.equalToForClientOrDefault(context.getEnvironment(), + "configurations", "same-instance-preference"); } } diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/BlockingLoadBalancerClientAutoConfiguration.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/BlockingLoadBalancerClientAutoConfiguration.java index 2da800e2f..23ae410ac 100644 --- a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/BlockingLoadBalancerClientAutoConfiguration.java +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/BlockingLoadBalancerClientAutoConfiguration.java @@ -21,7 +21,6 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; -import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.cloud.client.ServiceInstance; import org.springframework.cloud.client.loadbalancer.AsyncLoadBalancerAutoConfiguration; @@ -35,8 +34,13 @@ import org.springframework.cloud.loadbalancer.blocking.retry.BlockingLoadBalancedRetryFactory; import org.springframework.cloud.loadbalancer.core.LoadBalancerServiceInstanceCookieTransformer; import org.springframework.cloud.loadbalancer.support.LoadBalancerClientFactory; +import org.springframework.cloud.loadbalancer.support.LoadBalancerEnvironmentPropertyUtils; import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Condition; +import org.springframework.context.annotation.ConditionContext; +import org.springframework.context.annotation.Conditional; import org.springframework.context.annotation.Configuration; +import org.springframework.core.type.AnnotatedTypeMetadata; import org.springframework.retry.support.RetryTemplate; import org.springframework.web.client.RestTemplate; @@ -63,8 +67,7 @@ public LoadBalancerClient blockingLoadBalancerClient(LoadBalancerClientFactory l } @Bean - @ConditionalOnProperty(value = "spring.cloud.loadbalancer.sticky-session.add-service-instance-cookie", - havingValue = "true") + @Conditional(AddServiceInstanceCookieCondition.class) @ConditionalOnMissingBean(LoadBalancerServiceInstanceCookieTransformer.class) public LoadBalancerServiceInstanceCookieTransformer loadBalancerServiceInstanceCookieTransformer( LoadBalancerProperties properties) { @@ -85,4 +88,14 @@ LoadBalancedRetryFactory loadBalancedRetryFactory( } + static class AddServiceInstanceCookieCondition implements Condition { + + @Override + public boolean matches(ConditionContext context, AnnotatedTypeMetadata metadata) { + return LoadBalancerEnvironmentPropertyUtils.trueForClientOrDefault(context.getEnvironment(), + "sticky-session.add-service-instance-cookie"); + } + + } + } diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/support/LoadBalancerEnvironmentPropertyUtils.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/support/LoadBalancerEnvironmentPropertyUtils.java index 6045d44ef..3856dc674 100644 --- a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/support/LoadBalancerEnvironmentPropertyUtils.java +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/support/LoadBalancerEnvironmentPropertyUtils.java @@ -1,3 +1,19 @@ +/* + * Copyright 2012-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.loadbalancer.support; import org.springframework.core.env.Environment; @@ -12,35 +28,43 @@ private LoadBalancerEnvironmentPropertyUtils() { } public static boolean trueForClientOrDefault(Environment environment, String propertySuffix) { - String defaultValue = environment - .getProperty("spring.cloud.loadbalancer." + propertySuffix); - String clientValue = environment - .getProperty("spring.cloud.loadbalancer.clients." + - LoadBalancerClientFactory - .getName(environment) + "." + propertySuffix); - if (clientValue != null && clientValue - .equalsIgnoreCase(Boolean.TRUE.toString())) { + return equalToForClientOrDefault(environment, propertySuffix, Boolean.TRUE.toString()); + } + + public static boolean equalToForClientOrDefault(Environment environment, String propertySuffix, + String expectedPropertyValue) { + String defaultValue = getDefaultPropertyValue(environment, propertySuffix); + String clientValue = getClientPropertyValue(environment, propertySuffix); + if (clientValue != null && clientValue.equalsIgnoreCase(expectedPropertyValue)) { return true; } - return clientValue == null && defaultValue != null && defaultValue - .equalsIgnoreCase(Boolean.TRUE.toString()); + return clientValue == null && defaultValue != null && defaultValue.equalsIgnoreCase(expectedPropertyValue); } - public static boolean trueOrMissingForClientOrDefault(Environment environment, String propertySuffix) { - String defaultValue = environment - .getProperty("spring.cloud.loadbalancer." + propertySuffix); - String clientValue = environment - .getProperty("spring.cloud.loadbalancer.clients." + - LoadBalancerClientFactory - .getName(environment) + "." + propertySuffix); - if (clientValue != null && clientValue - .equalsIgnoreCase(Boolean.TRUE.toString())) { + public static boolean equalToOrMissingForClientOrDefault(Environment environment, String propertySuffix, + String expectedPropertyValue) { + String defaultValue = getDefaultPropertyValue(environment, propertySuffix); + String clientValue = getClientPropertyValue(environment, propertySuffix); + if (clientValue != null && clientValue.equalsIgnoreCase(expectedPropertyValue)) { return true; } - if (clientValue == null && defaultValue != null && defaultValue - .equalsIgnoreCase(Boolean.TRUE.toString())) { + if (clientValue == null && defaultValue != null && defaultValue.equalsIgnoreCase(expectedPropertyValue)) { return true; } return clientValue == null && defaultValue == null; } + + public static boolean trueOrMissingForClientOrDefault(Environment environment, String propertySuffix) { + return equalToOrMissingForClientOrDefault(environment, propertySuffix, Boolean.TRUE.toString()); + } + + private static String getClientPropertyValue(Environment environment, String propertySuffix) { + return environment.getProperty("spring.cloud.loadbalancer.clients." + + LoadBalancerClientFactory.getName(environment) + "." + propertySuffix); + } + + private static String getDefaultPropertyValue(Environment environment, String propertySuffix) { + return environment.getProperty("spring.cloud.loadbalancer." + propertySuffix); + } + } From 8c39d42932fbcfa4d89253f4c5428ba8b5b71422 Mon Sep 17 00:00:00 2001 From: Olga MaciaszekSharma Date: Thu, 28 Oct 2021 17:04:11 +0200 Subject: [PATCH 3/5] Add docs. --- docs/src/main/asciidoc/spring-cloud-commons.adoc | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/docs/src/main/asciidoc/spring-cloud-commons.adoc b/docs/src/main/asciidoc/spring-cloud-commons.adoc index cb2cbf674..10e6cd681 100644 --- a/docs/src/main/asciidoc/spring-cloud-commons.adoc +++ b/docs/src/main/asciidoc/spring-cloud-commons.adoc @@ -1269,6 +1269,15 @@ spring: The above example will result in a merged health-check `@ConfigurationProperties` object with `initial-delay=1s` and `interval=30s`. +The per-client configuration properties work for most of the properties, apart from the following global ones: + +- `spring.cloud.loadbalancer.enabled` - globally enables or disables load-balancing +- `spring.cloud.loadbalancer.retry.enabled` - globally enables or disables load-balanced retries. If you enable it globally, you can still disable retries for specific clients using the `client`-prefixed properties, but not the other way round +- `spring.cloud.loadbalancer.cache.enabled` - globally enables or disables LoadBalancer caching. If you enable it globally, you can still disable caching for specific clients by creating a <> that does not include the `CachingServiceInstanceListSupplier` in the `ServiceInstanceListSupplier` delegates hierarchy, but not the other way round. +- `spring.cloud.loadbalancer.stats.micrometer.enabled` - globally enables or disables LoadBalancer Micrometer metrics + +NOTE: For the properties where maps where already used, where you could specify a different value per-client without using the `clients` keyword (for example, `hints`, `health-check.path`), we have kept that behaviour in order to keep the library backwards compatible. It will be modified in the next major release. + == Spring Cloud Circuit Breaker include::spring-cloud-circuitbreaker.adoc[leveloffset=+1] From 3c923e8625ce6243684880f40bb718aa1f32b806 Mon Sep 17 00:00:00 2001 From: Olga MaciaszekSharma Date: Tue, 2 Nov 2021 15:26:27 +0100 Subject: [PATCH 4/5] Fix X-Forwarded headers conditions. --- .../LoadBalancerClientConfiguration.java | 3 +- ...ngLoadBalancerClientAutoConfiguration.java | 2 +- .../XForwardedConfigurationCondition.java | 38 +++++++++++++++++++ 3 files changed, 41 insertions(+), 2 deletions(-) create mode 100644 spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/XForwardedConfigurationCondition.java diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/annotation/LoadBalancerClientConfiguration.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/annotation/LoadBalancerClientConfiguration.java index af7c75e50..11e2a17fe 100644 --- a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/annotation/LoadBalancerClientConfiguration.java +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/annotation/LoadBalancerClientConfiguration.java @@ -30,6 +30,7 @@ import org.springframework.cloud.client.ServiceInstance; import org.springframework.cloud.client.discovery.DiscoveryClient; import org.springframework.cloud.client.discovery.ReactiveDiscoveryClient; +import org.springframework.cloud.loadbalancer.config.XForwardedConfigurationCondition; import org.springframework.cloud.loadbalancer.core.ReactorLoadBalancer; import org.springframework.cloud.loadbalancer.core.RetryAwareServiceInstanceListSupplier; import org.springframework.cloud.loadbalancer.core.RoundRobinLoadBalancer; @@ -99,7 +100,7 @@ public ServiceInstanceListSupplier zonePreferenceDiscoveryClientServiceInstanceL @Bean @ConditionalOnBean(XForwardedHeadersTransformer.class) @ConditionalOnMissingBean - @ConditionalOnProperty(value = "spring.cloud.loadbalancer.xForwarded.enabledXforwarded", havingValue = "true") + @Conditional(XForwardedConfigurationCondition.class) public XForwardedHeadersTransformer xForwarderHeadersTransformer(LoadBalancerClientFactory clientFactory) { return new XForwardedHeadersTransformer(clientFactory); } diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/BlockingLoadBalancerClientAutoConfiguration.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/BlockingLoadBalancerClientAutoConfiguration.java index 0457e1b1c..5b2ab0335 100644 --- a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/BlockingLoadBalancerClientAutoConfiguration.java +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/BlockingLoadBalancerClientAutoConfiguration.java @@ -77,7 +77,7 @@ public LoadBalancerServiceInstanceCookieTransformer loadBalancerServiceInstanceC } @Bean - @ConditionalOnProperty(value = "spring.cloud.loadbalancer.xforwarded.enabledXforwarded", havingValue = "true") + @Conditional(XForwardedConfigurationCondition.class) @ConditionalOnMissingBean(XForwardedHeadersTransformer.class) public XForwardedHeadersTransformer xForwarderHeadersTransformer( LoadBalancerClientFactory loadBalancerClientFactory) { diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/XForwardedConfigurationCondition.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/XForwardedConfigurationCondition.java new file mode 100644 index 000000000..f82cb0692 --- /dev/null +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/XForwardedConfigurationCondition.java @@ -0,0 +1,38 @@ +/* + * Copyright 2012-2021 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.loadbalancer.config; + +import org.springframework.cloud.loadbalancer.support.LoadBalancerEnvironmentPropertyUtils; +import org.springframework.context.annotation.Condition; +import org.springframework.context.annotation.ConditionContext; +import org.springframework.core.type.AnnotatedTypeMetadata; + +/** + * Condition matched when x-forwarded headers are enabled via properties. + * + * @author Olga Maciaszek-Sharma + * @since 3.1.0 + */ +public class XForwardedConfigurationCondition implements Condition { + + @Override + public boolean matches(ConditionContext context, AnnotatedTypeMetadata metadata) { + return LoadBalancerEnvironmentPropertyUtils.trueForClientOrDefault(context.getEnvironment(), + "spring.cloud.loadbalancer.x-forwarded.enabled"); + } + +} From 0a145294ea7a482f38af95d3f217901886a50a87 Mon Sep 17 00:00:00 2001 From: Olga MaciaszekSharma Date: Tue, 2 Nov 2021 15:45:41 +0100 Subject: [PATCH 5/5] Add tests for LoadBalancerEnvironmentPropertyUtils. --- ...BalancerEnvironmentPropertyUtilsTests.java | 85 +++++++++++++++++++ 1 file changed, 85 insertions(+) create mode 100644 spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/support/LoadBalancerEnvironmentPropertyUtilsTests.java diff --git a/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/support/LoadBalancerEnvironmentPropertyUtilsTests.java b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/support/LoadBalancerEnvironmentPropertyUtilsTests.java new file mode 100644 index 000000000..6e8105b1c --- /dev/null +++ b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/support/LoadBalancerEnvironmentPropertyUtilsTests.java @@ -0,0 +1,85 @@ +/* + * Copyright 2012-2021 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.loadbalancer.support; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import org.springframework.mock.env.MockEnvironment; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests for {@link LoadBalancerEnvironmentPropertyUtils}. + * + * @author Olga Maciaszek-Sharma + */ +class LoadBalancerEnvironmentPropertyUtilsTests { + + MockEnvironment environment = new MockEnvironment(); + + @BeforeEach + void setUp() { + environment.setProperty("loadbalancer.client.name", "x"); + } + + @Test + void shouldReturnTrueWhenTrueForClient() { + environment.setProperty("spring.cloud.loadbalancer.clients.x.test", "true"); + environment.setProperty("spring.cloud.loadbalancer.test", "false"); + + assertThat(LoadBalancerEnvironmentPropertyUtils.trueForClientOrDefault(environment, "test")).isTrue(); + assertThat(LoadBalancerEnvironmentPropertyUtils.trueOrMissingForClientOrDefault(environment, "test")).isTrue(); + } + + @Test + void shouldReturnTrueWhenTrueForDefault() { + environment.setProperty("spring.cloud.loadbalancer.test", "true"); + + assertThat(LoadBalancerEnvironmentPropertyUtils.trueForClientOrDefault(environment, "test")).isTrue(); + assertThat(LoadBalancerEnvironmentPropertyUtils.trueOrMissingForClientOrDefault(environment, "test")).isTrue(); + } + + @Test + void shouldReturnTrueWhenEqualForClient() { + environment.setProperty("spring.cloud.loadbalancer.clients.x.test", "true"); + environment.setProperty("spring.cloud.loadbalancer.test", "false"); + + assertThat(LoadBalancerEnvironmentPropertyUtils.equalToForClientOrDefault(environment, "test", "true")) + .isTrue(); + assertThat(LoadBalancerEnvironmentPropertyUtils.equalToOrMissingForClientOrDefault(environment, "test", "true")) + .isTrue(); + } + + @Test + void shouldReturnTrueWhenEqualForDefault() { + environment.setProperty("spring.cloud.loadbalancer.test", "true"); + + assertThat(LoadBalancerEnvironmentPropertyUtils.equalToForClientOrDefault(environment, "test", "true")) + .isTrue(); + assertThat(LoadBalancerEnvironmentPropertyUtils.equalToOrMissingForClientOrDefault(environment, "test", "true")) + .isTrue(); + } + + @Test + void shouldReturnTrueWhenMissingForClientAndDefault() { + assertThat(LoadBalancerEnvironmentPropertyUtils.trueOrMissingForClientOrDefault(environment, "test")).isTrue(); + assertThat(LoadBalancerEnvironmentPropertyUtils.equalToOrMissingForClientOrDefault(environment, "test", "true")) + .isTrue(); + } + +}