From 155cade0795c280fa553758b6287794b6e7150d1 Mon Sep 17 00:00:00 2001 From: pmv <pmv@users.noreply.github.com> Date: Wed, 3 Mar 2021 01:05:08 -0600 Subject: [PATCH 1/4] Retry mechanism --- spring-cloud-vault-config/pom.xml | 12 ++++ .../vault/config/RetryableVaultTemplate.java | 53 ++++++++++++++ .../vault/config/VaultAutoConfiguration.java | 53 +++++++++++++- .../cloud/vault/config/VaultProperties.java | 71 +++++++++++++++++++ 4 files changed, 187 insertions(+), 2 deletions(-) create mode 100644 spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/RetryableVaultTemplate.java diff --git a/spring-cloud-vault-config/pom.xml b/spring-cloud-vault-config/pom.xml index 06e9709e4..152cecfd4 100644 --- a/spring-cloud-vault-config/pom.xml +++ b/spring-cloud-vault-config/pom.xml @@ -59,6 +59,18 @@ <optional>true</optional> </dependency> + <dependency> + <groupId>org.springframework.retry</groupId> + <artifactId>spring-retry</artifactId> + <optional>true</optional> + </dependency> + + <dependency> + <groupId>org.aspectj</groupId> + <artifactId>aspectjweaver</artifactId> + <optional>true</optional> + </dependency> + <!-- HTTP Client Libraries --> <dependency> <groupId>org.apache.httpcomponents</groupId> diff --git a/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/RetryableVaultTemplate.java b/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/RetryableVaultTemplate.java new file mode 100644 index 000000000..592b39a2a --- /dev/null +++ b/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/RetryableVaultTemplate.java @@ -0,0 +1,53 @@ +/* + * Copyright 2017-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.vault.config; + +import org.springframework.retry.annotation.Retryable; +import org.springframework.vault.authentication.SessionManager; +import org.springframework.vault.client.RestTemplateBuilder; +import org.springframework.vault.core.VaultTemplate; +import org.springframework.vault.support.VaultResponse; +import org.springframework.vault.support.VaultResponseSupport; + +/** + * + * VaultTemplate extended with retry configuration + * + */ +class RetryableVaultTemplate extends VaultTemplate { + + RetryableVaultTemplate(RestTemplateBuilder restTemplateBuilder, SessionManager sessionManager) { + super(restTemplateBuilder, sessionManager); + } + + RetryableVaultTemplate(RestTemplateBuilder restTemplateBuilder) { + super(restTemplateBuilder); + } + + @Retryable(interceptor = "vaultRetryInterceptor") + @Override + public VaultResponse read(final String path) { + return super.read(path); + } + + @Retryable(interceptor = "vaultRetryInterceptor") + @Override + public <T> VaultResponseSupport<T> read(final String path, final Class<T> responseType) { + return super.read(path, responseType); + } + +} diff --git a/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultAutoConfiguration.java b/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultAutoConfiguration.java index a53e28275..3586e58d6 100644 --- a/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultAutoConfiguration.java +++ b/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultAutoConfiguration.java @@ -20,25 +20,34 @@ import java.util.Collections; import java.util.List; +import org.aspectj.lang.annotation.Aspect; import org.springframework.beans.factory.DisposableBean; import org.springframework.beans.factory.InitializingBean; import org.springframework.beans.factory.ObjectFactory; import org.springframework.beans.factory.ObjectProvider; import org.springframework.boot.autoconfigure.EnableAutoConfiguration; +import org.springframework.boot.autoconfigure.aop.AopAutoConfiguration; +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.context.ConfigurableApplicationContext; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Import; import org.springframework.context.annotation.Lazy; import org.springframework.core.Ordered; import org.springframework.core.annotation.AnnotationAwareOrderComparator; import org.springframework.core.annotation.Order; import org.springframework.core.task.AsyncTaskExecutor; import org.springframework.http.client.ClientHttpRequestFactory; +import org.springframework.retry.annotation.EnableRetry; +import org.springframework.retry.annotation.Retryable; +import org.springframework.retry.interceptor.RetryInterceptorBuilder; +import org.springframework.retry.interceptor.RetryOperationsInterceptor; import org.springframework.scheduling.TaskScheduler; import org.springframework.scheduling.concurrent.ThreadPoolTaskScheduler; +import org.springframework.util.ClassUtils; import org.springframework.vault.authentication.ClientAuthentication; import org.springframework.vault.authentication.LifecycleAwareSessionManager; import org.springframework.vault.authentication.SessionManager; @@ -157,15 +166,37 @@ public RestTemplateFactory vaultRestTemplateFactory(ClientFactoryWrapper clientF @ConditionalOnMissingBean(VaultOperations.class) public VaultTemplate vaultTemplate(ClientFactoryWrapper clientFactoryWrapper) { - VaultProperties.AuthenticationMethod authentication = this.vaultProperties.getAuthentication(); RestTemplateBuilder restTemplateBuilder = restTemplateBuilder( clientFactoryWrapper.getClientHttpRequestFactory()); + if (ClassUtils.isPresent("org.springframework.retry.support.RetryTemplate", getClass().getClassLoader()) + && this.vaultProperties.isFailFast() && this.applicationContext.containsBean("vaultRetryInterceptor")) { + return buildRetryableVaultTemplate(restTemplateBuilder); + } + else { + return buildVaultTemplate(restTemplateBuilder); + } + } + + private VaultTemplate buildVaultTemplate(RestTemplateBuilder restTemplateBuilder) { + VaultProperties.AuthenticationMethod authentication = this.vaultProperties.getAuthentication(); if (authentication == VaultProperties.AuthenticationMethod.NONE) { return new VaultTemplate(restTemplateBuilder); } + else { + return new VaultTemplate(restTemplateBuilder, this.applicationContext.getBean(SessionManager.class)); + } + } - return new VaultTemplate(restTemplateBuilder, this.applicationContext.getBean(SessionManager.class)); + private VaultTemplate buildRetryableVaultTemplate(RestTemplateBuilder restTemplateBuilder) { + VaultProperties.AuthenticationMethod authentication = this.vaultProperties.getAuthentication(); + if (authentication == VaultProperties.AuthenticationMethod.NONE) { + return new RetryableVaultTemplate(restTemplateBuilder); + } + else { + return new RetryableVaultTemplate(restTemplateBuilder, + this.applicationContext.getBean(SessionManager.class)); + } } /** @@ -232,6 +263,24 @@ public ClientAuthentication clientAuthentication(ClientFactoryWrapper clientFact return factory.createClientAuthentication(); } + @ConditionalOnProperty("spring.cloud.vault.fail-fast") + @ConditionalOnClass({ Retryable.class, Aspect.class, AopAutoConfiguration.class }) + @Configuration(proxyBeanMethods = false) + @EnableRetry(proxyTargetClass = true) + @Import(AopAutoConfiguration.class) + protected static class RetryConfiguration { + + @Bean + @ConditionalOnMissingBean(name = "vaultRetryInterceptor") + public RetryOperationsInterceptor vaultRetryInterceptor(VaultProperties properties) { + VaultProperties.Retry retryProperties = properties.getRetry(); + return RetryInterceptorBuilder.stateless().backOffOptions(retryProperties.getInitialInterval(), + retryProperties.getMultiplier(), retryProperties.getMaxInterval()) + .maxAttempts(retryProperties.getMaxAttempts()).build(); + } + + } + /** * Wrapper to keep {@link TaskScheduler} local to Spring Cloud Vault. */ diff --git a/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultProperties.java b/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultProperties.java index f3a38f50b..ecfd395c8 100644 --- a/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultProperties.java +++ b/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultProperties.java @@ -130,6 +130,8 @@ public class VaultProperties implements EnvironmentAware { private Session session = new Session(); + private Retry retry = new Retry(); + /** * Application name for AppId authentication. */ @@ -350,6 +352,14 @@ public void setAuthentication(AuthenticationMethod authentication) { this.authentication = authentication; } + public Retry getRetry() { + return this.retry; + } + + public void setRetry(Retry retry) { + this.retry = retry; + } + /** * Enumeration of authentication methods. */ @@ -1287,4 +1297,65 @@ public void setExpiryThreshold(Duration expiryThreshold) { } + /** + * Vault retry configuration + * + * @since 3.0.2 + */ + public static class Retry { + + /** + * Initial retry interval in milliseconds. + */ + long initialInterval = 1000; + + /** + * Multiplier for next interval. + */ + double multiplier = 1.1; + + /** + * Maximum interval for backoff. + */ + long maxInterval = 2000; + + /** + * Maximum number of attempts. + */ + int maxAttempts = 6; + + public long getInitialInterval() { + return this.initialInterval; + } + + public void setInitialInterval(long initialInterval) { + this.initialInterval = initialInterval; + } + + public double getMultiplier() { + return this.multiplier; + } + + public void setMultiplier(double multiplier) { + this.multiplier = multiplier; + } + + public long getMaxInterval() { + return this.maxInterval; + } + + public void setMaxInterval(long maxInterval) { + this.maxInterval = maxInterval; + } + + public int getMaxAttempts() { + return this.maxAttempts; + } + + public void setMaxAttempts(int maxAttempts) { + this.maxAttempts = maxAttempts; + } + + } + } From d6e53e5bb1a8cc0fbd6b763ab29d8ad39825f978 Mon Sep 17 00:00:00 2001 From: pmv <pmv@users.noreply.github.com> Date: Sun, 7 Mar 2021 23:25:16 -0600 Subject: [PATCH 2/4] https://github.com/spring-cloud/spring-cloud-vault/issues/236 PR feedback - remove AOP and retry-enable ClientHttpRequestFactory --- pom.xml | 1 + spring-cloud-vault-config-tests/pom.xml | 71 +++++++++++++++++++ ...utoConfigurationRetryUnavailableTests.java | 54 ++++++++++++++ spring-cloud-vault-config/pom.xml | 6 -- .../config/RetryableClientHttpRequest.java | 67 +++++++++++++++++ .../vault/config/RetryableVaultTemplate.java | 53 -------------- .../vault/config/VaultAutoConfiguration.java | 62 +++------------- .../vault/config/VaultConfiguration.java | 5 ++ .../cloud/vault/config/VaultRetryUtil.java | 53 ++++++++++++++ .../RetryableClientHttpRequestTests.java | 56 +++++++++++++++ .../VaultAutoConfigurationRetryTests.java | 67 +++++++++++++++++ 11 files changed, 385 insertions(+), 110 deletions(-) create mode 100644 spring-cloud-vault-config-tests/pom.xml create mode 100644 spring-cloud-vault-config-tests/src/test/java/org/springframework/cloud/vault/config/VaultAutoConfigurationRetryUnavailableTests.java create mode 100644 spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/RetryableClientHttpRequest.java delete mode 100644 spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/RetryableVaultTemplate.java create mode 100644 spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultRetryUtil.java create mode 100644 spring-cloud-vault-config/src/test/java/org/springframework/cloud/vault/config/RetryableClientHttpRequestTests.java create mode 100644 spring-cloud-vault-config/src/test/java/org/springframework/cloud/vault/config/VaultAutoConfigurationRetryTests.java diff --git a/pom.xml b/pom.xml index 9b774067f..814180bed 100644 --- a/pom.xml +++ b/pom.xml @@ -22,6 +22,7 @@ <modules> <module>spring-cloud-vault-dependencies</module> <module>spring-cloud-vault-config</module> + <module>spring-cloud-vault-config-tests</module> <module>spring-cloud-vault-config-databases</module> <module>spring-cloud-vault-config-consul</module> <module>spring-cloud-vault-config-rabbitmq</module> diff --git a/spring-cloud-vault-config-tests/pom.xml b/spring-cloud-vault-config-tests/pom.xml new file mode 100644 index 000000000..9614dc9de --- /dev/null +++ b/spring-cloud-vault-config-tests/pom.xml @@ -0,0 +1,71 @@ +<?xml version="1.0" encoding="UTF-8"?> +<project xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" + xmlns="http://maven.apache.org/POM/4.0.0" + xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd"> + <modelVersion>4.0.0</modelVersion> + + <parent> + <groupId>org.springframework.cloud</groupId> + <artifactId>spring-cloud-vault-parent</artifactId> + <version>3.0.2-SNAPSHOT</version> + <relativePath>..</relativePath> + </parent> + + <artifactId>spring-cloud-vault-config-tests</artifactId> + <name>Spring Cloud Vault Config Tests</name> + <description>Module for testing spring-cloud-vault-config with a limited number of dependencies</description> + + <dependencies> + <dependency> + <groupId>org.springframework.cloud</groupId> + <artifactId>spring-cloud-starter-vault-config</artifactId> + <version>${project.version}</version> + </dependency> + + <dependency> + <groupId>org.springframework.boot</groupId> + <artifactId>spring-boot-test</artifactId> + <scope>test</scope> + </dependency> + <dependency> + <groupId>org.junit.vintage</groupId> + <artifactId>junit-vintage-engine</artifactId> + <scope>test</scope> + </dependency> + </dependencies> + + <build> + <plugins> + <plugin> + <groupId>org.apache.maven.plugins</groupId> + <artifactId>maven-deploy-plugin</artifactId> + <configuration> + <!--test module only - should not be deployed --> + <skip>true</skip> + </configuration> + </plugin> + <plugin> + <groupId>org.apache.maven.plugins</groupId> + <artifactId>maven-enforcer-plugin</artifactId> + <executions> + <execution> + <id>enforce-banned-dependencies</id> + <goals> + <goal>enforce</goal> + </goals> + <configuration> + <rules> + <bannedDependencies> + <excludes> + <exclude>org.springframework.retry:spring-retry</exclude> + </excludes> + </bannedDependencies> + </rules> + <fail>true</fail> + </configuration> + </execution> + </executions> + </plugin> + </plugins> + </build> +</project> \ No newline at end of file diff --git a/spring-cloud-vault-config-tests/src/test/java/org/springframework/cloud/vault/config/VaultAutoConfigurationRetryUnavailableTests.java b/spring-cloud-vault-config-tests/src/test/java/org/springframework/cloud/vault/config/VaultAutoConfigurationRetryUnavailableTests.java new file mode 100644 index 000000000..7183917df --- /dev/null +++ b/spring-cloud-vault-config-tests/src/test/java/org/springframework/cloud/vault/config/VaultAutoConfigurationRetryUnavailableTests.java @@ -0,0 +1,54 @@ +/* + * Copyright 2016-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.vault.config; + +import org.junit.Test; +import org.springframework.boot.autoconfigure.AutoConfigurations; +import org.springframework.boot.test.context.runner.ApplicationContextRunner; +import org.springframework.http.HttpMethod; +import org.springframework.http.client.ClientHttpRequest; +import org.springframework.http.client.ClientHttpRequestFactory; +import org.springframework.vault.config.AbstractVaultConfiguration; + +import java.net.URI; + +import static org.assertj.core.api.AssertionsForClassTypes.assertThat; + +/** + * Tests without spring-retry on the classpath to be sure there is no hard dependency + */ +public class VaultAutoConfigurationRetryUnavailableTests { + + private ApplicationContextRunner contextRunner = new ApplicationContextRunner() + .withConfiguration(AutoConfigurations.of(VaultAutoConfiguration.class)); + + @Test + public void shouldNotBeConfigured() { + + this.contextRunner.withPropertyValues("spring.cloud.vault.kv.enabled=false", + "spring.cloud.vault.fail-fast=true", "spring.cloud.vault.token=foo").run(context -> { + assertThat(context.containsBean("vaultRetryOperations")).isFalse(); + AbstractVaultConfiguration.ClientFactoryWrapper clientFactoryWrapper = context + .getBean(AbstractVaultConfiguration.ClientFactoryWrapper.class); + ClientHttpRequestFactory requestFactory = clientFactoryWrapper.getClientHttpRequestFactory(); + ClientHttpRequest request = requestFactory.createRequest(new URI("https://spring.io/"), + HttpMethod.GET); + assertThat(request instanceof RetryableClientHttpRequest).isFalse(); + }); + } + +} diff --git a/spring-cloud-vault-config/pom.xml b/spring-cloud-vault-config/pom.xml index 152cecfd4..551ca0d92 100644 --- a/spring-cloud-vault-config/pom.xml +++ b/spring-cloud-vault-config/pom.xml @@ -65,12 +65,6 @@ <optional>true</optional> </dependency> - <dependency> - <groupId>org.aspectj</groupId> - <artifactId>aspectjweaver</artifactId> - <optional>true</optional> - </dependency> - <!-- HTTP Client Libraries --> <dependency> <groupId>org.apache.httpcomponents</groupId> diff --git a/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/RetryableClientHttpRequest.java b/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/RetryableClientHttpRequest.java new file mode 100644 index 000000000..45a8a6a01 --- /dev/null +++ b/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/RetryableClientHttpRequest.java @@ -0,0 +1,67 @@ +/* + * Copyright 2016-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.vault.config; + +import org.springframework.http.HttpHeaders; +import org.springframework.http.client.ClientHttpRequest; +import org.springframework.http.client.ClientHttpResponse; +import org.springframework.retry.RetryOperations; + +import java.io.IOException; +import java.io.OutputStream; +import java.net.URI; + +/** + * {@link ClientHttpRequest} configured with retry support + */ +class RetryableClientHttpRequest implements ClientHttpRequest { + + private final ClientHttpRequest delegateRequest; + + private final RetryOperations retryOperations; + + RetryableClientHttpRequest(ClientHttpRequest request, RetryOperations retryOperations) { + this.delegateRequest = request; + this.retryOperations = retryOperations; + } + + @Override + public ClientHttpResponse execute() throws IOException { + return retryOperations.execute(retryContext -> delegateRequest.execute()); + } + + @Override + public OutputStream getBody() throws IOException { + return delegateRequest.getBody(); + } + + @Override + public String getMethodValue() { + return delegateRequest.getMethodValue(); + } + + @Override + public URI getURI() { + return delegateRequest.getURI(); + } + + @Override + public HttpHeaders getHeaders() { + return delegateRequest.getHeaders(); + } + +} diff --git a/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/RetryableVaultTemplate.java b/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/RetryableVaultTemplate.java deleted file mode 100644 index 592b39a2a..000000000 --- a/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/RetryableVaultTemplate.java +++ /dev/null @@ -1,53 +0,0 @@ -/* - * Copyright 2017-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.vault.config; - -import org.springframework.retry.annotation.Retryable; -import org.springframework.vault.authentication.SessionManager; -import org.springframework.vault.client.RestTemplateBuilder; -import org.springframework.vault.core.VaultTemplate; -import org.springframework.vault.support.VaultResponse; -import org.springframework.vault.support.VaultResponseSupport; - -/** - * - * VaultTemplate extended with retry configuration - * - */ -class RetryableVaultTemplate extends VaultTemplate { - - RetryableVaultTemplate(RestTemplateBuilder restTemplateBuilder, SessionManager sessionManager) { - super(restTemplateBuilder, sessionManager); - } - - RetryableVaultTemplate(RestTemplateBuilder restTemplateBuilder) { - super(restTemplateBuilder); - } - - @Retryable(interceptor = "vaultRetryInterceptor") - @Override - public VaultResponse read(final String path) { - return super.read(path); - } - - @Retryable(interceptor = "vaultRetryInterceptor") - @Override - public <T> VaultResponseSupport<T> read(final String path, final Class<T> responseType) { - return super.read(path, responseType); - } - -} diff --git a/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultAutoConfiguration.java b/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultAutoConfiguration.java index 3586e58d6..31a9d22f0 100644 --- a/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultAutoConfiguration.java +++ b/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultAutoConfiguration.java @@ -20,31 +20,23 @@ import java.util.Collections; import java.util.List; -import org.aspectj.lang.annotation.Aspect; import org.springframework.beans.factory.DisposableBean; import org.springframework.beans.factory.InitializingBean; import org.springframework.beans.factory.ObjectFactory; import org.springframework.beans.factory.ObjectProvider; import org.springframework.boot.autoconfigure.EnableAutoConfiguration; -import org.springframework.boot.autoconfigure.aop.AopAutoConfiguration; -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.context.ConfigurableApplicationContext; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; -import org.springframework.context.annotation.Import; import org.springframework.context.annotation.Lazy; import org.springframework.core.Ordered; import org.springframework.core.annotation.AnnotationAwareOrderComparator; import org.springframework.core.annotation.Order; import org.springframework.core.task.AsyncTaskExecutor; import org.springframework.http.client.ClientHttpRequestFactory; -import org.springframework.retry.annotation.EnableRetry; -import org.springframework.retry.annotation.Retryable; -import org.springframework.retry.interceptor.RetryInterceptorBuilder; -import org.springframework.retry.interceptor.RetryOperationsInterceptor; import org.springframework.scheduling.TaskScheduler; import org.springframework.scheduling.concurrent.ThreadPoolTaskScheduler; import org.springframework.util.ClassUtils; @@ -138,7 +130,15 @@ protected RestTemplateBuilder restTemplateBuilder(ClientHttpRequestFactory reque @Bean @ConditionalOnMissingBean public ClientFactoryWrapper clientHttpRequestFactoryWrapper() { - return new ClientFactoryWrapper(this.configuration.createClientHttpRequestFactory()); + ClientHttpRequestFactory clientHttpRequestFactory; + if (ClassUtils.isPresent("org.springframework.retry.support.RetryTemplate", getClass().getClassLoader()) + && this.vaultProperties.isFailFast()) { + clientHttpRequestFactory = this.configuration.createRetryableClientHttpRequestFactory(); + } + else { + clientHttpRequestFactory = this.configuration.createClientHttpRequestFactory(); + } + return new ClientFactoryWrapper(clientHttpRequestFactory); } /** @@ -166,37 +166,15 @@ public RestTemplateFactory vaultRestTemplateFactory(ClientFactoryWrapper clientF @ConditionalOnMissingBean(VaultOperations.class) public VaultTemplate vaultTemplate(ClientFactoryWrapper clientFactoryWrapper) { + VaultProperties.AuthenticationMethod authentication = this.vaultProperties.getAuthentication(); RestTemplateBuilder restTemplateBuilder = restTemplateBuilder( clientFactoryWrapper.getClientHttpRequestFactory()); - if (ClassUtils.isPresent("org.springframework.retry.support.RetryTemplate", getClass().getClassLoader()) - && this.vaultProperties.isFailFast() && this.applicationContext.containsBean("vaultRetryInterceptor")) { - return buildRetryableVaultTemplate(restTemplateBuilder); - } - else { - return buildVaultTemplate(restTemplateBuilder); - } - } - - private VaultTemplate buildVaultTemplate(RestTemplateBuilder restTemplateBuilder) { - VaultProperties.AuthenticationMethod authentication = this.vaultProperties.getAuthentication(); if (authentication == VaultProperties.AuthenticationMethod.NONE) { return new VaultTemplate(restTemplateBuilder); } - else { - return new VaultTemplate(restTemplateBuilder, this.applicationContext.getBean(SessionManager.class)); - } - } - private VaultTemplate buildRetryableVaultTemplate(RestTemplateBuilder restTemplateBuilder) { - VaultProperties.AuthenticationMethod authentication = this.vaultProperties.getAuthentication(); - if (authentication == VaultProperties.AuthenticationMethod.NONE) { - return new RetryableVaultTemplate(restTemplateBuilder); - } - else { - return new RetryableVaultTemplate(restTemplateBuilder, - this.applicationContext.getBean(SessionManager.class)); - } + return new VaultTemplate(restTemplateBuilder, this.applicationContext.getBean(SessionManager.class)); } /** @@ -263,24 +241,6 @@ public ClientAuthentication clientAuthentication(ClientFactoryWrapper clientFact return factory.createClientAuthentication(); } - @ConditionalOnProperty("spring.cloud.vault.fail-fast") - @ConditionalOnClass({ Retryable.class, Aspect.class, AopAutoConfiguration.class }) - @Configuration(proxyBeanMethods = false) - @EnableRetry(proxyTargetClass = true) - @Import(AopAutoConfiguration.class) - protected static class RetryConfiguration { - - @Bean - @ConditionalOnMissingBean(name = "vaultRetryInterceptor") - public RetryOperationsInterceptor vaultRetryInterceptor(VaultProperties properties) { - VaultProperties.Retry retryProperties = properties.getRetry(); - return RetryInterceptorBuilder.stateless().backOffOptions(retryProperties.getInitialInterval(), - retryProperties.getMultiplier(), retryProperties.getMaxInterval()) - .maxAttempts(retryProperties.getMaxAttempts()).build(); - } - - } - /** * Wrapper to keep {@link TaskScheduler} local to Spring Cloud Vault. */ diff --git a/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultConfiguration.java b/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultConfiguration.java index 1df81b921..32151dde6 100644 --- a/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultConfiguration.java +++ b/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultConfiguration.java @@ -111,6 +111,11 @@ ClientHttpRequestFactory createClientHttpRequestFactory() { return ClientHttpRequestFactoryFactory.create(clientOptions, sslConfiguration); } + public ClientHttpRequestFactory createRetryableClientHttpRequestFactory() { + ClientHttpRequestFactory delegate = createClientHttpRequestFactory(); + return VaultRetryUtil.createRetryableClientHttpRequestFactory(this.vaultProperties, delegate); + } + /** * Create a {@link VaultEndpoint} from {@link VaultProperties}. * @return the endpoint. diff --git a/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultRetryUtil.java b/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultRetryUtil.java new file mode 100644 index 000000000..e50c6f05b --- /dev/null +++ b/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultRetryUtil.java @@ -0,0 +1,53 @@ +/* + * Copyright 2018-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.vault.config; + +import org.springframework.http.client.ClientHttpRequest; +import org.springframework.http.client.ClientHttpRequestFactory; +import org.springframework.retry.backoff.ExponentialBackOffPolicy; +import org.springframework.retry.policy.SimpleRetryPolicy; +import org.springframework.retry.support.RetryTemplate; + +/** + * Util class for building objects that rely on spring-retry + */ +final class VaultRetryUtil { + + private VaultRetryUtil() { + + } + + static ClientHttpRequestFactory createRetryableClientHttpRequestFactory(VaultProperties vaultProperties, + ClientHttpRequestFactory delegate) { + VaultProperties.Retry retryProperties = vaultProperties.getRetry(); + RetryTemplate retryTemplate = new RetryTemplate(); + + ExponentialBackOffPolicy policy = new ExponentialBackOffPolicy(); + policy.setInitialInterval(retryProperties.getInitialInterval()); + policy.setMultiplier(retryProperties.getMultiplier()); + policy.setMaxInterval(retryProperties.getMaxInterval()); + + retryTemplate.setBackOffPolicy(policy); + retryTemplate.setRetryPolicy(new SimpleRetryPolicy(retryProperties.getMaxAttempts())); + + return (uri, httpMethod) -> retryTemplate.execute(retryContext -> { + ClientHttpRequest request = delegate.createRequest(uri, httpMethod); + return new RetryableClientHttpRequest(request, retryTemplate); + }); + } + +} diff --git a/spring-cloud-vault-config/src/test/java/org/springframework/cloud/vault/config/RetryableClientHttpRequestTests.java b/spring-cloud-vault-config/src/test/java/org/springframework/cloud/vault/config/RetryableClientHttpRequestTests.java new file mode 100644 index 000000000..07113a16c --- /dev/null +++ b/spring-cloud-vault-config/src/test/java/org/springframework/cloud/vault/config/RetryableClientHttpRequestTests.java @@ -0,0 +1,56 @@ +/* + * Copyright 2016-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.vault.config; + +import org.junit.jupiter.api.Test; +import org.springframework.http.HttpMethod; +import org.springframework.http.client.ClientHttpRequest; +import org.springframework.http.client.ClientHttpRequestFactory; + +import java.io.IOException; +import java.net.SocketTimeoutException; +import java.net.URI; +import java.net.URISyntaxException; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +public class RetryableClientHttpRequestTests { + + @Test + public void shouldRetry() throws URISyntaxException, IOException { + ClientHttpRequestFactory delegate = mock(ClientHttpRequestFactory.class); + ClientHttpRequest delegateRequest = mock(ClientHttpRequest.class); + when(delegateRequest.execute()).thenThrow(new SocketTimeoutException()); + when(delegate.createRequest(any(), any())).thenReturn(delegateRequest); + ClientHttpRequestFactory retryableFactory = VaultRetryUtil + .createRetryableClientHttpRequestFactory(new VaultProperties(), delegate); + ClientHttpRequest request = retryableFactory.createRequest(new URI("https://spring.io/"), HttpMethod.GET); + try { + request.execute(); + } + catch (SocketTimeoutException e) { + // expected + } + verify(delegateRequest, times(6)).execute(); + + } + +} diff --git a/spring-cloud-vault-config/src/test/java/org/springframework/cloud/vault/config/VaultAutoConfigurationRetryTests.java b/spring-cloud-vault-config/src/test/java/org/springframework/cloud/vault/config/VaultAutoConfigurationRetryTests.java new file mode 100644 index 000000000..b1d66e42f --- /dev/null +++ b/spring-cloud-vault-config/src/test/java/org/springframework/cloud/vault/config/VaultAutoConfigurationRetryTests.java @@ -0,0 +1,67 @@ +/* + * Copyright 2016-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.vault.config; + +import org.junit.Test; +import org.springframework.boot.autoconfigure.AutoConfigurations; +import org.springframework.boot.test.context.runner.ApplicationContextRunner; +import org.springframework.http.HttpMethod; +import org.springframework.http.client.ClientHttpRequest; +import org.springframework.http.client.ClientHttpRequestFactory; +import org.springframework.vault.config.AbstractVaultConfiguration; + +import java.net.URI; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests around retry functionality + */ +public class VaultAutoConfigurationRetryTests { + + private ApplicationContextRunner contextRunner = new ApplicationContextRunner() + .withConfiguration(AutoConfigurations.of(VaultAutoConfiguration.class)); + + @Test + public void shouldNotRetryFailFastMissing() { + + this.contextRunner.withPropertyValues("spring.cloud.vault.kv.enabled=false", "spring.cloud.vault.token=foo") + .run(context -> { + AbstractVaultConfiguration.ClientFactoryWrapper clientFactoryWrapper = context + .getBean(AbstractVaultConfiguration.ClientFactoryWrapper.class); + ClientHttpRequestFactory requestFactory = clientFactoryWrapper.getClientHttpRequestFactory(); + ClientHttpRequest request = requestFactory.createRequest(new URI("https://spring.io/"), + HttpMethod.GET); + assertThat(request instanceof RetryableClientHttpRequest).isFalse(); + }); + } + + @Test + public void shouldBeConfiguredToRetry() { + + this.contextRunner.withPropertyValues("spring.cloud.vault.kv.enabled=false", + "spring.cloud.vault.fail-fast=true", "spring.cloud.vault.token=foo").run(context -> { + AbstractVaultConfiguration.ClientFactoryWrapper clientFactoryWrapper = context + .getBean(AbstractVaultConfiguration.ClientFactoryWrapper.class); + ClientHttpRequestFactory requestFactory = clientFactoryWrapper.getClientHttpRequestFactory(); + ClientHttpRequest request = requestFactory.createRequest(new URI("https://spring.io/"), + HttpMethod.GET); + assertThat(request instanceof RetryableClientHttpRequest).isTrue(); + }); + } + +} From 2fa16a63f5f33cad976f590333e0ee965107a62e Mon Sep 17 00:00:00 2001 From: pmv <pmv@users.noreply.github.com> Date: Wed, 12 May 2021 14:03:31 -0500 Subject: [PATCH 3/4] PR feedback; changes to make ConfigData flow also retry --- docs/src/main/asciidoc/_configprops.adoc | 6 +- docs/src/main/asciidoc/other-topics.adoc | 9 ++ pom.xml | 1 - spring-cloud-vault-config-tests/pom.xml | 71 ----------- spring-cloud-vault-config/pom.xml | 7 ++ .../cloud/vault/config/RetryProperties.java | 81 +++++++++++++ .../vault/config/VaultAutoConfiguration.java | 35 ++++-- .../config/VaultBootstrapConfiguration.java | 7 +- .../vault/config/VaultConfigDataLoader.java | 26 +++- .../VaultConfigDataLocationResolver.java | 3 + .../vault/config/VaultConfiguration.java | 5 - .../cloud/vault/config/VaultProperties.java | 68 ++--------- .../cloud/vault/config/VaultRetryUtil.java | 22 +++- .../RetryableClientHttpRequestTests.java | 2 +- ...utoConfigurationRetryUnavailableTests.java | 5 + .../config/VaultConfigLoaderRetryTests.java | 111 ++++++++++++++++++ ...aultConfigLoaderRetryUnavailableTests.java | 97 +++++++++++++++ .../VaultReactiveConfigLoaderTests.java | 67 +++++++++++ 18 files changed, 466 insertions(+), 157 deletions(-) delete mode 100644 spring-cloud-vault-config-tests/pom.xml create mode 100644 spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/RetryProperties.java rename {spring-cloud-vault-config-tests => spring-cloud-vault-config}/src/test/java/org/springframework/cloud/vault/config/VaultAutoConfigurationRetryUnavailableTests.java (90%) create mode 100644 spring-cloud-vault-config/src/test/java/org/springframework/cloud/vault/config/VaultConfigLoaderRetryTests.java create mode 100644 spring-cloud-vault-config/src/test/java/org/springframework/cloud/vault/config/VaultConfigLoaderRetryUnavailableTests.java create mode 100644 spring-cloud-vault-config/src/test/java/org/springframework/cloud/vault/config/VaultReactiveConfigLoaderTests.java diff --git a/docs/src/main/asciidoc/_configprops.adoc b/docs/src/main/asciidoc/_configprops.adoc index 3067273e2..6a9322513 100644 --- a/docs/src/main/asciidoc/_configprops.adoc +++ b/docs/src/main/asciidoc/_configprops.adoc @@ -118,6 +118,10 @@ |spring.cloud.vault.rabbitmq.role | | Role name for credentials. |spring.cloud.vault.rabbitmq.username-property | `spring.rabbitmq.username` | Target property for the obtained username. |spring.cloud.vault.read-timeout | `15000` | Read timeout. +|spring.cloud.vault.retry.initial-interval | `1000` | Initial retry interval in milliseconds. +|spring.cloud.vault.retry.multiplier | `1.1` | Multiplier for next interval. +|spring.cloud.vault.retry.max-interval | `2000` | Maximum interval for backoff. +|spring.cloud.vault.retry.max-attempts | `6` | Maximum number of attempts. |spring.cloud.vault.scheme | `https` | Protocol scheme. Can be either "http" or "https". |spring.cloud.vault.session.lifecycle.enabled | `true` | Enable session lifecycle management. |spring.cloud.vault.session.lifecycle.expiry-threshold | `7s` | The expiry threshold for a {@link LoginToken}. The threshold represents a minimum TTL duration to consider a login token as valid. Tokens with a shorter TTL are considered expired and are not used anymore. Should be greater than {@code refreshBeforeExpiry} to prevent token expiry. @@ -134,4 +138,4 @@ |spring.cloud.vault.token | | Static vault token. Required if {@link #authentication} is {@code TOKEN}. |spring.cloud.vault.uri | | Vault URI. Can be set with scheme, host and port. -|=== \ No newline at end of file +|=== diff --git a/docs/src/main/asciidoc/other-topics.adoc b/docs/src/main/asciidoc/other-topics.adoc index 67ef84e69..d709cb895 100644 --- a/docs/src/main/asciidoc/other-topics.adoc +++ b/docs/src/main/asciidoc/other-topics.adoc @@ -35,6 +35,15 @@ spring.cloud.vault: ---- ==== +[[vault.config.retry]] +== Vault Client Retry + +If you expect that the config server may occasionally be unavailable when your application starts, you can make it keep trying after a failure. +First, you need to set `spring.cloud.vault.fail-fast=true`. +Then you need to add `spring-retry` to your classpath. +The default behavior is to retry six times with an initial backoff interval of 1000ms and an exponential multiplier of 1.1 for subsequent backoffs. +You can configure these properties by setting the `spring.cloud.config.retry.*` configuration properties. + [[vault.config.namespaces]] == Vault Enterprise Namespace Support diff --git a/pom.xml b/pom.xml index ea228128c..211386ebd 100644 --- a/pom.xml +++ b/pom.xml @@ -22,7 +22,6 @@ <modules> <module>spring-cloud-vault-dependencies</module> <module>spring-cloud-vault-config</module> - <module>spring-cloud-vault-config-tests</module> <module>spring-cloud-vault-config-databases</module> <module>spring-cloud-vault-config-consul</module> <module>spring-cloud-vault-config-rabbitmq</module> diff --git a/spring-cloud-vault-config-tests/pom.xml b/spring-cloud-vault-config-tests/pom.xml deleted file mode 100644 index 9614dc9de..000000000 --- a/spring-cloud-vault-config-tests/pom.xml +++ /dev/null @@ -1,71 +0,0 @@ -<?xml version="1.0" encoding="UTF-8"?> -<project xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" - xmlns="http://maven.apache.org/POM/4.0.0" - xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd"> - <modelVersion>4.0.0</modelVersion> - - <parent> - <groupId>org.springframework.cloud</groupId> - <artifactId>spring-cloud-vault-parent</artifactId> - <version>3.0.2-SNAPSHOT</version> - <relativePath>..</relativePath> - </parent> - - <artifactId>spring-cloud-vault-config-tests</artifactId> - <name>Spring Cloud Vault Config Tests</name> - <description>Module for testing spring-cloud-vault-config with a limited number of dependencies</description> - - <dependencies> - <dependency> - <groupId>org.springframework.cloud</groupId> - <artifactId>spring-cloud-starter-vault-config</artifactId> - <version>${project.version}</version> - </dependency> - - <dependency> - <groupId>org.springframework.boot</groupId> - <artifactId>spring-boot-test</artifactId> - <scope>test</scope> - </dependency> - <dependency> - <groupId>org.junit.vintage</groupId> - <artifactId>junit-vintage-engine</artifactId> - <scope>test</scope> - </dependency> - </dependencies> - - <build> - <plugins> - <plugin> - <groupId>org.apache.maven.plugins</groupId> - <artifactId>maven-deploy-plugin</artifactId> - <configuration> - <!--test module only - should not be deployed --> - <skip>true</skip> - </configuration> - </plugin> - <plugin> - <groupId>org.apache.maven.plugins</groupId> - <artifactId>maven-enforcer-plugin</artifactId> - <executions> - <execution> - <id>enforce-banned-dependencies</id> - <goals> - <goal>enforce</goal> - </goals> - <configuration> - <rules> - <bannedDependencies> - <excludes> - <exclude>org.springframework.retry:spring-retry</exclude> - </excludes> - </bannedDependencies> - </rules> - <fail>true</fail> - </configuration> - </execution> - </executions> - </plugin> - </plugins> - </build> -</project> \ No newline at end of file diff --git a/spring-cloud-vault-config/pom.xml b/spring-cloud-vault-config/pom.xml index 686e6bbea..75db8eee2 100644 --- a/spring-cloud-vault-config/pom.xml +++ b/spring-cloud-vault-config/pom.xml @@ -193,6 +193,13 @@ <artifactId>junit-vintage-engine</artifactId> <scope>test</scope> </dependency> + + <dependency> + <groupId>org.springframework.cloud</groupId> + <artifactId>spring-cloud-test-support</artifactId> + <version>${spring-cloud-commons.version}</version> + <scope>test</scope> + </dependency> </dependencies> <build> diff --git a/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/RetryProperties.java b/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/RetryProperties.java new file mode 100644 index 000000000..6d09b9fdd --- /dev/null +++ b/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/RetryProperties.java @@ -0,0 +1,81 @@ +/* + * Copyright 2014-2019 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.vault.config; + +import org.springframework.boot.context.properties.ConfigurationProperties; + +@ConfigurationProperties(RetryProperties.PREFIX) +public class RetryProperties { + + /** + * ConfigurationProperties prefix. + */ + public static final String PREFIX = "spring.cloud.vault.retry"; + + /** + * Initial retry interval in milliseconds. + */ + long initialInterval = 1000; + + /** + * Multiplier for next interval. + */ + double multiplier = 1.1; + + /** + * Maximum interval for backoff. + */ + long maxInterval = 2000; + + /** + * Maximum number of attempts. + */ + int maxAttempts = 6; + + public long getInitialInterval() { + return this.initialInterval; + } + + public void setInitialInterval(long initialInterval) { + this.initialInterval = initialInterval; + } + + public double getMultiplier() { + return this.multiplier; + } + + public void setMultiplier(double multiplier) { + this.multiplier = multiplier; + } + + public long getMaxInterval() { + return this.maxInterval; + } + + public void setMaxInterval(long maxInterval) { + this.maxInterval = maxInterval; + } + + public int getMaxAttempts() { + return this.maxAttempts; + } + + public void setMaxAttempts(int maxAttempts) { + this.maxAttempts = maxAttempts; + } + +} diff --git a/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultAutoConfiguration.java b/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultAutoConfiguration.java index b2cc7f3d2..d8f1bdc17 100644 --- a/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultAutoConfiguration.java +++ b/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultAutoConfiguration.java @@ -19,7 +19,10 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.Map; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.springframework.beans.factory.DisposableBean; import org.springframework.beans.factory.InitializingBean; import org.springframework.beans.factory.ObjectFactory; @@ -37,6 +40,7 @@ import org.springframework.core.annotation.Order; import org.springframework.core.task.AsyncTaskExecutor; import org.springframework.http.client.ClientHttpRequestFactory; +import org.springframework.retry.support.RetryTemplate; import org.springframework.scheduling.TaskScheduler; import org.springframework.scheduling.concurrent.ThreadPoolTaskScheduler; import org.springframework.util.ClassUtils; @@ -65,14 +69,20 @@ */ @Configuration(proxyBeanMethods = false) @ConditionalOnProperty(name = "spring.cloud.vault.enabled", matchIfMissing = true) -@EnableConfigurationProperties(VaultProperties.class) +@EnableConfigurationProperties({ VaultProperties.class, RetryProperties.class }) @Order(Ordered.LOWEST_PRECEDENCE - 5) public class VaultAutoConfiguration { + private final Log log = LogFactory.getLog(getClass()); + + private static final String RETRY_TEMPLATE = "org.springframework.retry.support.RetryTemplate"; + private final ConfigurableApplicationContext applicationContext; private final VaultProperties vaultProperties; + private final RetryProperties retryProperties; + private final VaultConfiguration configuration; private final VaultEndpointProvider endpointProvider; @@ -82,12 +92,13 @@ public class VaultAutoConfiguration { private final List<RestTemplateRequestCustomizer<?>> requestCustomizers; public VaultAutoConfiguration(ConfigurableApplicationContext applicationContext, VaultProperties vaultProperties, - ObjectProvider<VaultEndpointProvider> endpointProvider, + RetryProperties retryProperties, ObjectProvider<VaultEndpointProvider> endpointProvider, ObjectProvider<List<RestTemplateCustomizer>> customizers, ObjectProvider<List<RestTemplateRequestCustomizer<?>>> requestCustomizers) { this.applicationContext = applicationContext; this.vaultProperties = vaultProperties; + this.retryProperties = retryProperties; this.configuration = new VaultConfiguration(vaultProperties); VaultEndpointProvider provider = endpointProvider.getIfAvailable(); @@ -130,13 +141,19 @@ protected RestTemplateBuilder restTemplateBuilder(ClientHttpRequestFactory reque @Bean @ConditionalOnMissingBean public ClientFactoryWrapper clientHttpRequestFactoryWrapper() { - ClientHttpRequestFactory clientHttpRequestFactory; - if (ClassUtils.isPresent("org.springframework.retry.support.RetryTemplate", getClass().getClassLoader()) - && this.vaultProperties.isFailFast()) { - clientHttpRequestFactory = this.configuration.createRetryableClientHttpRequestFactory(); - } - else { - clientHttpRequestFactory = this.configuration.createClientHttpRequestFactory(); + ClientHttpRequestFactory clientHttpRequestFactory = this.configuration.createClientHttpRequestFactory(); + if (ClassUtils.isPresent(RETRY_TEMPLATE, getClass().getClassLoader()) && this.vaultProperties.isFailFast()) { + Map<String, RetryTemplate> beans = applicationContext.getBeansOfType(RetryTemplate.class); + if (!beans.isEmpty()) { + Map.Entry<String, RetryTemplate> existingBean = beans.entrySet().stream().findFirst().get(); + log.info("Using existing RestTemplate '" + existingBean.getKey() + "' for vault retries"); + clientHttpRequestFactory = VaultRetryUtil + .createRetryableClientHttpRequestFactory(existingBean.getValue(), clientHttpRequestFactory); + } + else { + clientHttpRequestFactory = VaultRetryUtil.createRetryableClientHttpRequestFactory(retryProperties, + clientHttpRequestFactory); + } } return new ClientFactoryWrapper(clientHttpRequestFactory); } diff --git a/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultBootstrapConfiguration.java b/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultBootstrapConfiguration.java index de44953b0..dafa4b11b 100644 --- a/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultBootstrapConfiguration.java +++ b/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultBootstrapConfiguration.java @@ -38,16 +38,17 @@ * {@code @EnableAutoConfiguration}. */ @ConditionalOnProperty(name = "spring.cloud.vault.enabled", matchIfMissing = true) -@EnableConfigurationProperties(VaultProperties.class) +@EnableConfigurationProperties({ VaultProperties.class, RetryProperties.class }) @Order(Ordered.LOWEST_PRECEDENCE - 5) @Deprecated public class VaultBootstrapConfiguration extends VaultAutoConfiguration { public VaultBootstrapConfiguration(ConfigurableApplicationContext applicationContext, - VaultProperties vaultProperties, ObjectProvider<VaultEndpointProvider> endpointProvider, + VaultProperties vaultProperties, RetryProperties retryProperties, + ObjectProvider<VaultEndpointProvider> endpointProvider, ObjectProvider<List<RestTemplateCustomizer>> customizers, ObjectProvider<List<RestTemplateRequestCustomizer<?>>> requestCustomizers) { - super(applicationContext, vaultProperties, endpointProvider, customizers, requestCustomizers); + super(applicationContext, vaultProperties, retryProperties, endpointProvider, customizers, requestCustomizers); } } diff --git a/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultConfigDataLoader.java b/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultConfigDataLoader.java index 31b4ee6e4..d2b58ae22 100644 --- a/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultConfigDataLoader.java +++ b/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultConfigDataLoader.java @@ -47,6 +47,7 @@ import org.springframework.core.env.PropertySource; import org.springframework.http.client.ClientHttpRequestFactory; import org.springframework.lang.Nullable; +import org.springframework.retry.support.RetryTemplate; import org.springframework.scheduling.concurrent.ThreadPoolTaskScheduler; import org.springframework.util.ClassUtils; import org.springframework.util.ReflectionUtils; @@ -101,6 +102,9 @@ public class VaultConfigDataLoader implements ConfigDataLoader<VaultConfigLocati private final static boolean REGISTER_REACTIVE_INFRASTRUCTURE = FLUX_AVAILABLE && WEBCLIENT_AVAILABLE; + private final static boolean RETRY_AVAILABLE = ClassUtils + .isPresent("org.springframework.retry.support.RetryTemplate", VaultConfigDataLoader.class.getClassLoader()); + private final DeferredLogFactory logFactory; public VaultConfigDataLoader(DeferredLogFactory logFactory) { @@ -114,15 +118,16 @@ public ConfigData load(ConfigDataLoaderContext context, VaultConfigLocation loca ConfigurableBootstrapContext bootstrap = context.getBootstrapContext(); VaultProperties vaultProperties = bootstrap.get(VaultProperties.class); + RetryProperties retryProperties = bootstrap.get(RetryProperties.class); if (vaultProperties.getSession().getLifecycle().isEnabled() || vaultProperties.getConfig().getLifecycle().isEnabled()) { registerVaultTaskScheduler(bootstrap); } - registerImperativeInfrastructure(bootstrap, vaultProperties); + registerImperativeInfrastructure(bootstrap, vaultProperties, retryProperties); - if (REGISTER_REACTIVE_INFRASTRUCTURE) { + if (REGISTER_REACTIVE_INFRASTRUCTURE && vaultProperties.getReactive().isEnabled()) { registerReactiveInfrastructure(bootstrap, vaultProperties); } @@ -168,9 +173,10 @@ private ConfigData loadConfigData(VaultConfigLocation location, ConfigurableBoot } private void registerImperativeInfrastructure(ConfigurableBootstrapContext bootstrap, - VaultProperties vaultProperties) { + VaultProperties vaultProperties, RetryProperties retryProperties) { - ImperativeInfrastructure infra = new ImperativeInfrastructure(bootstrap, vaultProperties, this.logFactory); + ImperativeInfrastructure infra = new ImperativeInfrastructure(bootstrap, vaultProperties, retryProperties, + this.logFactory); infra.registerClientHttpRequestFactoryWrapper(); infra.registerRestTemplateBuilder(); @@ -186,7 +192,7 @@ private void registerImperativeInfrastructure(ConfigurableBootstrapContext boots infra.registerClientAuthentication(); - if (!REGISTER_REACTIVE_INFRASTRUCTURE) { + if (!(REGISTER_REACTIVE_INFRASTRUCTURE && vaultProperties.getReactive().isEnabled())) { infra.registerVaultSessionManager(); } @@ -423,6 +429,8 @@ static class ImperativeInfrastructure { private final VaultProperties vaultProperties; + private final RetryProperties retryProperties; + private final VaultConfiguration configuration; private final VaultEndpointProvider endpointProvider; @@ -430,9 +438,10 @@ static class ImperativeInfrastructure { private final DeferredLogFactory logFactory; ImperativeInfrastructure(ConfigurableBootstrapContext bootstrap, VaultProperties vaultProperties, - DeferredLogFactory logFactory) { + RetryProperties retryProperties, DeferredLogFactory logFactory) { this.bootstrap = bootstrap; this.vaultProperties = vaultProperties; + this.retryProperties = retryProperties; this.configuration = new VaultConfiguration(vaultProperties); this.endpointProvider = SimpleVaultEndpointProvider.of(this.configuration.createVaultEndpoint()); this.logFactory = logFactory; @@ -442,6 +451,11 @@ void registerClientHttpRequestFactoryWrapper() { registerIfAbsent(this.bootstrap, "clientHttpRequestFactoryWrapper", ClientFactoryWrapper.class, () -> { ClientHttpRequestFactory factory = this.configuration.createClientHttpRequestFactory(); + if (RETRY_AVAILABLE && this.vaultProperties.isFailFast()) { + RetryTemplate retryTemplate = bootstrap.getOrElse(RetryTemplate.class, + VaultRetryUtil.createRetryTemplate(retryProperties)); + factory = VaultRetryUtil.createRetryableClientHttpRequestFactory(retryTemplate, factory); + } // early initialization try { diff --git a/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultConfigDataLocationResolver.java b/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultConfigDataLocationResolver.java index 436e0689d..3ac73ba6f 100644 --- a/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultConfigDataLocationResolver.java +++ b/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultConfigDataLocationResolver.java @@ -131,6 +131,9 @@ private static void registerVaultProperties(ConfigDataLocationResolverContext co return vaultProperties; }); + + context.getBootstrapContext().registerIfAbsent(RetryProperties.class, + ignore -> context.getBinder().bindOrCreate(RetryProperties.PREFIX, RetryProperties.class)); } private List<SecretBackendMetadata> getSecretBackends(ConfigDataLocationResolverContext context, diff --git a/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultConfiguration.java b/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultConfiguration.java index 8dc506e23..2330067b9 100644 --- a/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultConfiguration.java +++ b/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultConfiguration.java @@ -111,11 +111,6 @@ ClientHttpRequestFactory createClientHttpRequestFactory() { return ClientHttpRequestFactoryFactory.create(clientOptions, sslConfiguration); } - public ClientHttpRequestFactory createRetryableClientHttpRequestFactory() { - ClientHttpRequestFactory delegate = createClientHttpRequestFactory(); - return VaultRetryUtil.createRetryableClientHttpRequestFactory(this.vaultProperties, delegate); - } - /** * Create a {@link VaultEndpoint} from {@link VaultProperties}. * @return the endpoint. diff --git a/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultProperties.java b/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultProperties.java index c64ec7d10..be419b899 100644 --- a/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultProperties.java +++ b/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultProperties.java @@ -132,7 +132,7 @@ public class VaultProperties implements EnvironmentAware { private Session session = new Session(); - private Retry retry = new Retry(); + private Reactive reactive = new Reactive(); /** * Application name for AppId authentication. @@ -354,12 +354,12 @@ public void setAuthentication(AuthenticationMethod authentication) { this.authentication = authentication; } - public Retry getRetry() { - return this.retry; + public Reactive getReactive() { + return reactive; } - public void setRetry(Retry retry) { - this.retry = retry; + public void setReactive(Reactive reactive) { + this.reactive = reactive; } /** @@ -1327,63 +1327,19 @@ public void setExpiryThreshold(Duration expiryThreshold) { } - /** - * Vault retry configuration - * - * @since 3.0.2 - */ - public static class Retry { - - /** - * Initial retry interval in milliseconds. - */ - long initialInterval = 1000; + public static class Reactive { /** - * Multiplier for next interval. + * Enable reactive beans if dependencies are available */ - double multiplier = 1.1; - - /** - * Maximum interval for backoff. - */ - long maxInterval = 2000; - - /** - * Maximum number of attempts. - */ - int maxAttempts = 6; - - public long getInitialInterval() { - return this.initialInterval; - } - - public void setInitialInterval(long initialInterval) { - this.initialInterval = initialInterval; - } - - public double getMultiplier() { - return this.multiplier; - } - - public void setMultiplier(double multiplier) { - this.multiplier = multiplier; - } - - public long getMaxInterval() { - return this.maxInterval; - } - - public void setMaxInterval(long maxInterval) { - this.maxInterval = maxInterval; - } + private boolean enabled = true; - public int getMaxAttempts() { - return this.maxAttempts; + public boolean isEnabled() { + return this.enabled; } - public void setMaxAttempts(int maxAttempts) { - this.maxAttempts = maxAttempts; + public void setEnabled(boolean enabled) { + this.enabled = enabled; } } diff --git a/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultRetryUtil.java b/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultRetryUtil.java index e50c6f05b..81f6ac67a 100644 --- a/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultRetryUtil.java +++ b/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultRetryUtil.java @@ -1,5 +1,5 @@ /* - * Copyright 2018-2020 the original author or authors. + * Copyright 2018-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. @@ -16,6 +16,8 @@ package org.springframework.cloud.vault.config; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.springframework.http.client.ClientHttpRequest; import org.springframework.http.client.ClientHttpRequestFactory; import org.springframework.retry.backoff.ExponentialBackOffPolicy; @@ -27,13 +29,13 @@ */ final class VaultRetryUtil { + private static final Log log = LogFactory.getLog(VaultRetryUtil.class); + private VaultRetryUtil() { } - static ClientHttpRequestFactory createRetryableClientHttpRequestFactory(VaultProperties vaultProperties, - ClientHttpRequestFactory delegate) { - VaultProperties.Retry retryProperties = vaultProperties.getRetry(); + static RetryTemplate createRetryTemplate(RetryProperties retryProperties) { RetryTemplate retryTemplate = new RetryTemplate(); ExponentialBackOffPolicy policy = new ExponentialBackOffPolicy(); @@ -44,10 +46,22 @@ static ClientHttpRequestFactory createRetryableClientHttpRequestFactory(VaultPro retryTemplate.setBackOffPolicy(policy); retryTemplate.setRetryPolicy(new SimpleRetryPolicy(retryProperties.getMaxAttempts())); + return retryTemplate; + } + + static ClientHttpRequestFactory createRetryableClientHttpRequestFactory(RetryTemplate retryTemplate, + ClientHttpRequestFactory delegate) { return (uri, httpMethod) -> retryTemplate.execute(retryContext -> { ClientHttpRequest request = delegate.createRequest(uri, httpMethod); return new RetryableClientHttpRequest(request, retryTemplate); }); } + static ClientHttpRequestFactory createRetryableClientHttpRequestFactory(RetryProperties retryProperties, + ClientHttpRequestFactory delegate) { + RetryTemplate retryTemplate = createRetryTemplate(retryProperties); + + return createRetryableClientHttpRequestFactory(retryTemplate, delegate); + } + } diff --git a/spring-cloud-vault-config/src/test/java/org/springframework/cloud/vault/config/RetryableClientHttpRequestTests.java b/spring-cloud-vault-config/src/test/java/org/springframework/cloud/vault/config/RetryableClientHttpRequestTests.java index 07113a16c..fb6156c6a 100644 --- a/spring-cloud-vault-config/src/test/java/org/springframework/cloud/vault/config/RetryableClientHttpRequestTests.java +++ b/spring-cloud-vault-config/src/test/java/org/springframework/cloud/vault/config/RetryableClientHttpRequestTests.java @@ -41,7 +41,7 @@ public void shouldRetry() throws URISyntaxException, IOException { when(delegateRequest.execute()).thenThrow(new SocketTimeoutException()); when(delegate.createRequest(any(), any())).thenReturn(delegateRequest); ClientHttpRequestFactory retryableFactory = VaultRetryUtil - .createRetryableClientHttpRequestFactory(new VaultProperties(), delegate); + .createRetryableClientHttpRequestFactory(new RetryProperties(), delegate); ClientHttpRequest request = retryableFactory.createRequest(new URI("https://spring.io/"), HttpMethod.GET); try { request.execute(); diff --git a/spring-cloud-vault-config-tests/src/test/java/org/springframework/cloud/vault/config/VaultAutoConfigurationRetryUnavailableTests.java b/spring-cloud-vault-config/src/test/java/org/springframework/cloud/vault/config/VaultAutoConfigurationRetryUnavailableTests.java similarity index 90% rename from spring-cloud-vault-config-tests/src/test/java/org/springframework/cloud/vault/config/VaultAutoConfigurationRetryUnavailableTests.java rename to spring-cloud-vault-config/src/test/java/org/springframework/cloud/vault/config/VaultAutoConfigurationRetryUnavailableTests.java index 7183917df..53906105c 100644 --- a/spring-cloud-vault-config-tests/src/test/java/org/springframework/cloud/vault/config/VaultAutoConfigurationRetryUnavailableTests.java +++ b/spring-cloud-vault-config/src/test/java/org/springframework/cloud/vault/config/VaultAutoConfigurationRetryUnavailableTests.java @@ -17,8 +17,11 @@ package org.springframework.cloud.vault.config; import org.junit.Test; +import org.junit.runner.RunWith; import org.springframework.boot.autoconfigure.AutoConfigurations; import org.springframework.boot.test.context.runner.ApplicationContextRunner; +import org.springframework.cloud.test.ClassPathExclusions; +import org.springframework.cloud.test.ModifiedClassPathRunner; import org.springframework.http.HttpMethod; import org.springframework.http.client.ClientHttpRequest; import org.springframework.http.client.ClientHttpRequestFactory; @@ -31,6 +34,8 @@ /** * Tests without spring-retry on the classpath to be sure there is no hard dependency */ +@RunWith(ModifiedClassPathRunner.class) +@ClassPathExclusions({ "spring-retry-*.jar" }) public class VaultAutoConfigurationRetryUnavailableTests { private ApplicationContextRunner contextRunner = new ApplicationContextRunner() diff --git a/spring-cloud-vault-config/src/test/java/org/springframework/cloud/vault/config/VaultConfigLoaderRetryTests.java b/spring-cloud-vault-config/src/test/java/org/springframework/cloud/vault/config/VaultConfigLoaderRetryTests.java new file mode 100644 index 000000000..f4fa6fbf3 --- /dev/null +++ b/spring-cloud-vault-config/src/test/java/org/springframework/cloud/vault/config/VaultConfigLoaderRetryTests.java @@ -0,0 +1,111 @@ +/* + * Copyright 2016-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.vault.config; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.junit.jupiter.api.Test; +import org.springframework.boot.DefaultBootstrapContext; +import org.springframework.boot.SpringApplication; +import org.springframework.boot.WebApplicationType; +import org.springframework.boot.autoconfigure.EnableAutoConfiguration; +import org.springframework.boot.context.config.ConfigDataEnvironmentPostProcessor; +import org.springframework.boot.logging.DeferredLog; +import org.springframework.boot.logging.DeferredLogs; +import org.springframework.boot.test.context.runner.ApplicationContextRunner; +import org.springframework.cloud.autoconfigure.RefreshAutoConfiguration; +import org.springframework.context.ConfigurableApplicationContext; +import org.springframework.core.env.ConfigurableEnvironment; +import org.springframework.core.env.MapPropertySource; +import org.springframework.core.env.StandardEnvironment; +import org.springframework.http.HttpMethod; +import org.springframework.http.client.ClientHttpRequest; +import org.springframework.http.client.ClientHttpRequestFactory; +import org.springframework.test.context.support.TestPropertySourceUtils; +import org.springframework.vault.VaultException; +import org.springframework.vault.config.AbstractVaultConfiguration; + +import java.io.IOException; +import java.net.URI; +import java.net.URISyntaxException; +import java.util.Map; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests around retry functionality + */ +public class VaultConfigLoaderRetryTests { + + private final Log log = LogFactory.getLog(VaultConfigLoaderRetryTests.class); + + private ApplicationContextRunner contextRunner = new ApplicationContextRunner(); + + @Test + public void shouldNotRetryFailFastMissing() throws URISyntaxException, IOException { + SpringApplication app = new SpringApplication(TestApplication.class); + Map<String, Object> properties = TestPropertySourceUtils.convertInlinedPropertiesToMap( + "spring.cloud.vault.lifecycle.enabled=false", + "spring.cloud.vault.uri=http://serverhostdoesnotexist:1234", "spring.config.import=vault://"); + app.setDefaultProperties(properties); + app.setWebApplicationType(WebApplicationType.NONE); + + ConfigurableApplicationContext context = app.run(); + AbstractVaultConfiguration.ClientFactoryWrapper clientFactoryWrapper = context + .getBean(AbstractVaultConfiguration.ClientFactoryWrapper.class); + ClientHttpRequestFactory requestFactory = clientFactoryWrapper.getClientHttpRequestFactory(); + ClientHttpRequest request = requestFactory.createRequest(new URI("https://spring.io/"), HttpMethod.GET); + assertThat(request instanceof RetryableClientHttpRequest).isFalse(); + } + + @Test + public void shouldBeConfiguredToRetry() throws URISyntaxException, IOException { + SpringApplication app = new SpringApplication(TestApplication.class); + app.setWebApplicationType(WebApplicationType.NONE); + DefaultBootstrapContext bootstrapContext = new DefaultBootstrapContext(); + + Map<String, Object> properties = TestPropertySourceUtils.convertInlinedPropertiesToMap( + "spring.profiles.active=test", "spring.cloud.vault.fail-fast=true", + "spring.cloud.vault.reactive.enabled=false", "spring.cloud.vault.retry.max-attempts=2", + "spring.cloud.vault.lifecycle.enabled=false", + "spring.cloud.vault.uri=http://serverhostdoesnotexist:1234", "spring.config.import=vault://"); + ConfigurableEnvironment environment = new StandardEnvironment(); + environment.getPropertySources().addFirst(new MapPropertySource("testPropertiesSource", properties)); + DeferredLogs logs = new DeferredLogs(); + ConfigDataEnvironmentPostProcessor configDataEnvironmentPostProcessor = new ConfigDataEnvironmentPostProcessor( + new DeferredLogs(), bootstrapContext); + try { + configDataEnvironmentPostProcessor.postProcessEnvironment(environment, app); + } + catch (VaultException ve) { + // expected since fail-fast is true + } + ((DeferredLog) logs.getLog(ConfigDataEnvironmentPostProcessor.class)).replayTo(log); + + AbstractVaultConfiguration.ClientFactoryWrapper clientFactoryWrapper = bootstrapContext + .get(AbstractVaultConfiguration.ClientFactoryWrapper.class); + ClientHttpRequestFactory requestFactory = clientFactoryWrapper.getClientHttpRequestFactory(); + ClientHttpRequest request = requestFactory.createRequest(new URI("https://spring.io/"), HttpMethod.GET); + assertThat(request instanceof RetryableClientHttpRequest).isTrue(); + } + + @EnableAutoConfiguration(exclude = RefreshAutoConfiguration.class) + public static class TestApplication { + + } + +} diff --git a/spring-cloud-vault-config/src/test/java/org/springframework/cloud/vault/config/VaultConfigLoaderRetryUnavailableTests.java b/spring-cloud-vault-config/src/test/java/org/springframework/cloud/vault/config/VaultConfigLoaderRetryUnavailableTests.java new file mode 100644 index 000000000..474849361 --- /dev/null +++ b/spring-cloud-vault-config/src/test/java/org/springframework/cloud/vault/config/VaultConfigLoaderRetryUnavailableTests.java @@ -0,0 +1,97 @@ +/* + * Copyright 2016-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.vault.config; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.boot.DefaultBootstrapContext; +import org.springframework.boot.SpringApplication; +import org.springframework.boot.WebApplicationType; +import org.springframework.boot.autoconfigure.EnableAutoConfiguration; +import org.springframework.boot.context.config.ConfigDataEnvironmentPostProcessor; +import org.springframework.boot.logging.DeferredLog; +import org.springframework.boot.logging.DeferredLogs; +import org.springframework.boot.test.context.runner.ApplicationContextRunner; +import org.springframework.cloud.autoconfigure.RefreshAutoConfiguration; +import org.springframework.cloud.test.ClassPathExclusions; +import org.springframework.cloud.test.ModifiedClassPathRunner; +import org.springframework.core.env.ConfigurableEnvironment; +import org.springframework.core.env.MapPropertySource; +import org.springframework.core.env.StandardEnvironment; +import org.springframework.http.HttpMethod; +import org.springframework.http.client.ClientHttpRequest; +import org.springframework.http.client.ClientHttpRequestFactory; +import org.springframework.test.context.support.TestPropertySourceUtils; +import org.springframework.vault.VaultException; +import org.springframework.vault.config.AbstractVaultConfiguration; + +import java.io.IOException; +import java.net.URI; +import java.net.URISyntaxException; +import java.util.Map; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests without spring-retry on the classpath to be sure there is no hard dependency + */ +@RunWith(ModifiedClassPathRunner.class) +@ClassPathExclusions({ "spring-retry-*.jar" }) +public class VaultConfigLoaderRetryUnavailableTests { + + private final Log log = LogFactory.getLog(getClass()); + + private ApplicationContextRunner contextRunner = new ApplicationContextRunner(); + + @Test + public void shouldNotBeConfiguredToRetry() throws URISyntaxException, IOException { + SpringApplication app = new SpringApplication(TestApplication.class); + app.setWebApplicationType(WebApplicationType.NONE); + DefaultBootstrapContext bootstrapContext = new DefaultBootstrapContext(); + + Map<String, Object> properties = TestPropertySourceUtils.convertInlinedPropertiesToMap( + "spring.profiles.active=test", "spring.cloud.vault.fail-fast=true", + "spring.cloud.vault.retry.max-attempts=2", "spring.cloud.vault.lifecycle.enabled=false", + "spring.cloud.vault.uri=http://serverhostdoesnotexist:1234", "spring.config.import=vault://"); + ConfigurableEnvironment environment = new StandardEnvironment(); + environment.getPropertySources().addFirst(new MapPropertySource("testPropertiesSource", properties)); + DeferredLogs logs = new DeferredLogs(); + ConfigDataEnvironmentPostProcessor configDataEnvironmentPostProcessor = new ConfigDataEnvironmentPostProcessor( + new DeferredLogs(), bootstrapContext); + try { + configDataEnvironmentPostProcessor.postProcessEnvironment(environment, app); + } + catch (VaultException ve) { + // expected since fail-fast is true + } + ((DeferredLog) logs.getLog(ConfigDataEnvironmentPostProcessor.class)).replayTo(log); + + AbstractVaultConfiguration.ClientFactoryWrapper clientFactoryWrapper = bootstrapContext + .get(AbstractVaultConfiguration.ClientFactoryWrapper.class); + ClientHttpRequestFactory requestFactory = clientFactoryWrapper.getClientHttpRequestFactory(); + ClientHttpRequest request = requestFactory.createRequest(new URI("https://spring.io/"), HttpMethod.GET); + assertThat(request instanceof RetryableClientHttpRequest).isFalse(); + } + + @EnableAutoConfiguration(exclude = RefreshAutoConfiguration.class) + public static class TestApplication { + + } + +} diff --git a/spring-cloud-vault-config/src/test/java/org/springframework/cloud/vault/config/VaultReactiveConfigLoaderTests.java b/spring-cloud-vault-config/src/test/java/org/springframework/cloud/vault/config/VaultReactiveConfigLoaderTests.java new file mode 100644 index 000000000..a7fa95b85 --- /dev/null +++ b/spring-cloud-vault-config/src/test/java/org/springframework/cloud/vault/config/VaultReactiveConfigLoaderTests.java @@ -0,0 +1,67 @@ +/* + * Copyright 2016-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.vault.config; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.junit.jupiter.api.Test; +import org.springframework.boot.DefaultBootstrapContext; +import org.springframework.boot.SpringApplication; +import org.springframework.boot.WebApplicationType; +import org.springframework.boot.context.config.ConfigDataEnvironmentPostProcessor; +import org.springframework.boot.logging.DeferredLog; +import org.springframework.boot.logging.DeferredLogs; +import org.springframework.core.env.ConfigurableEnvironment; +import org.springframework.core.env.MapPropertySource; +import org.springframework.core.env.StandardEnvironment; +import org.springframework.test.context.support.TestPropertySourceUtils; +import org.springframework.vault.core.ReactiveVaultTemplate; + +import java.util.Map; + +import static org.assertj.core.api.Assertions.assertThatIllegalStateException; + +public class VaultReactiveConfigLoaderTests { + + private final Log log = LogFactory.getLog(VaultReactiveConfigLoaderTests.class); + + /** + * Config loader equivalent of + * {@link VaultReactiveBootstrapConfigurationTests#shouldNotConfigureReactiveSupport()} + */ + @Test + public void shouldNotConfigureReactiveSupport() { + SpringApplication app = new SpringApplication(VaultConfigLoaderRetryTests.TestApplication.class); + app.setWebApplicationType(WebApplicationType.NONE); + DefaultBootstrapContext bootstrapContext = new DefaultBootstrapContext(); + + Map<String, Object> properties = TestPropertySourceUtils.convertInlinedPropertiesToMap( + "spring.profiles.active=test", "spring.cloud.vault.reactive.enabled=false", + "spring.cloud.vault.uri=http://serverhostdoesnotexist:1234", "spring.config.import=vault://"); + ConfigurableEnvironment environment = new StandardEnvironment(); + environment.getPropertySources().addFirst(new MapPropertySource("testPropertiesSource", properties)); + DeferredLogs logs = new DeferredLogs(); + ConfigDataEnvironmentPostProcessor configDataEnvironmentPostProcessor = new ConfigDataEnvironmentPostProcessor( + new DeferredLogs(), bootstrapContext); + configDataEnvironmentPostProcessor.postProcessEnvironment(environment, app); + ((DeferredLog) logs.getLog(ConfigDataEnvironmentPostProcessor.class)).replayTo(log); + + assertThatIllegalStateException().isThrownBy(() -> bootstrapContext.get(ReactiveVaultTemplate.class)) + .withMessageContaining("has not been registered"); + } + +} From b3edca9e52e338ac005ff92211508941c09c991a Mon Sep 17 00:00:00 2001 From: pmv <pmv@users.noreply.github.com> Date: Thu, 8 Sep 2022 12:20:34 -0500 Subject: [PATCH 4/4] Fix doc Detect and throw exception if retry properties are set but spring-retry is not on the classpath --- docs/src/main/asciidoc/other-topics.adoc | 4 +-- .../cloud/vault/config/RetryProperties.java | 8 +++++ .../vault/config/VaultAutoConfiguration.java | 30 +++++++++++++------ .../vault/config/VaultConfigDataLoader.java | 20 ++++++++++--- .../cloud/vault/config/VaultProperties.java | 27 ----------------- ...utoConfigurationRetryUnavailableTests.java | 18 +++++++++++ 6 files changed, 65 insertions(+), 42 deletions(-) diff --git a/docs/src/main/asciidoc/other-topics.adoc b/docs/src/main/asciidoc/other-topics.adoc index d709cb895..5b6de0657 100644 --- a/docs/src/main/asciidoc/other-topics.adoc +++ b/docs/src/main/asciidoc/other-topics.adoc @@ -38,11 +38,11 @@ spring.cloud.vault: [[vault.config.retry]] == Vault Client Retry -If you expect that the config server may occasionally be unavailable when your application starts, you can make it keep trying after a failure. +If you expect that the vault server may occasionally be unavailable when your application starts, you can make it keep trying after a failure. First, you need to set `spring.cloud.vault.fail-fast=true`. Then you need to add `spring-retry` to your classpath. The default behavior is to retry six times with an initial backoff interval of 1000ms and an exponential multiplier of 1.1 for subsequent backoffs. -You can configure these properties by setting the `spring.cloud.config.retry.*` configuration properties. +You can configure these properties by setting the `spring.cloud.vault.retry.*` configuration properties. [[vault.config.namespaces]] == Vault Enterprise Namespace Support diff --git a/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/RetryProperties.java b/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/RetryProperties.java index 6d09b9fdd..240441128 100644 --- a/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/RetryProperties.java +++ b/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/RetryProperties.java @@ -18,6 +18,10 @@ import org.springframework.boot.context.properties.ConfigurationProperties; +import java.util.Arrays; +import java.util.HashSet; +import java.util.Set; + @ConfigurationProperties(RetryProperties.PREFIX) public class RetryProperties { @@ -26,6 +30,10 @@ public class RetryProperties { */ public static final String PREFIX = "spring.cloud.vault.retry"; + public static final Set<String> PROPERTY_SET = new HashSet<>( + Arrays.asList("spring.cloud.vault.retry.max-attempts", "spring.cloud.vault.retry.multiplier", + "spring.cloud.vault.retry.initial-interval", "spring.cloud.vault.retry.max-interval")); + /** * Initial retry interval in milliseconds. */ diff --git a/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultAutoConfiguration.java b/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultAutoConfiguration.java index d8f1bdc17..8b3fa3cc9 100644 --- a/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultAutoConfiguration.java +++ b/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultAutoConfiguration.java @@ -38,6 +38,7 @@ import org.springframework.core.Ordered; import org.springframework.core.annotation.AnnotationAwareOrderComparator; import org.springframework.core.annotation.Order; +import org.springframework.core.env.ConfigurableEnvironment; import org.springframework.core.task.AsyncTaskExecutor; import org.springframework.http.client.ClientHttpRequestFactory; import org.springframework.retry.support.RetryTemplate; @@ -142,17 +143,28 @@ protected RestTemplateBuilder restTemplateBuilder(ClientHttpRequestFactory reque @ConditionalOnMissingBean public ClientFactoryWrapper clientHttpRequestFactoryWrapper() { ClientHttpRequestFactory clientHttpRequestFactory = this.configuration.createClientHttpRequestFactory(); - if (ClassUtils.isPresent(RETRY_TEMPLATE, getClass().getClassLoader()) && this.vaultProperties.isFailFast()) { - Map<String, RetryTemplate> beans = applicationContext.getBeansOfType(RetryTemplate.class); - if (!beans.isEmpty()) { - Map.Entry<String, RetryTemplate> existingBean = beans.entrySet().stream().findFirst().get(); - log.info("Using existing RestTemplate '" + existingBean.getKey() + "' for vault retries"); - clientHttpRequestFactory = VaultRetryUtil - .createRetryableClientHttpRequestFactory(existingBean.getValue(), clientHttpRequestFactory); + if (this.vaultProperties.isFailFast()) { + if (ClassUtils.isPresent(RETRY_TEMPLATE, getClass().getClassLoader())) { + Map<String, RetryTemplate> beans = applicationContext.getBeansOfType(RetryTemplate.class); + if (!beans.isEmpty()) { + Map.Entry<String, RetryTemplate> existingBean = beans.entrySet().stream().findFirst().get(); + log.info("Using existing RestTemplate '" + existingBean.getKey() + "' for vault retries"); + clientHttpRequestFactory = VaultRetryUtil + .createRetryableClientHttpRequestFactory(existingBean.getValue(), clientHttpRequestFactory); + } + else { + clientHttpRequestFactory = VaultRetryUtil.createRetryableClientHttpRequestFactory(retryProperties, + clientHttpRequestFactory); + } } else { - clientHttpRequestFactory = VaultRetryUtil.createRetryableClientHttpRequestFactory(retryProperties, - clientHttpRequestFactory); + ConfigurableEnvironment env = applicationContext.getEnvironment(); + boolean retryPropertySet = RetryProperties.PROPERTY_SET.stream() + .anyMatch(s -> env.getProperty(s) != null); + if (retryPropertySet) { + throw new IllegalStateException( + "One or more spring.cloud.vault.retry.* properties are set, but spring-retry is not on the classpath"); + } } } return new ClientFactoryWrapper(clientHttpRequestFactory); diff --git a/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultConfigDataLoader.java b/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultConfigDataLoader.java index 8a162e587..80e473624 100644 --- a/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultConfigDataLoader.java +++ b/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultConfigDataLoader.java @@ -43,6 +43,7 @@ import org.springframework.context.ApplicationEventPublisherAware; import org.springframework.context.ConfigurableApplicationContext; import org.springframework.context.support.GenericApplicationContext; +import org.springframework.core.env.Environment; import org.springframework.core.env.PropertySource; import org.springframework.http.client.ClientHttpRequestFactory; import org.springframework.lang.Nullable; @@ -456,10 +457,21 @@ void registerClientHttpRequestFactoryWrapper() { registerIfAbsent(this.bootstrap, "clientHttpRequestFactoryWrapper", ClientFactoryWrapper.class, () -> { ClientHttpRequestFactory factory = this.configuration.createClientHttpRequestFactory(); - if (RETRY_AVAILABLE && this.vaultProperties.isFailFast()) { - RetryTemplate retryTemplate = bootstrap.getOrElse(RetryTemplate.class, - VaultRetryUtil.createRetryTemplate(retryProperties)); - factory = VaultRetryUtil.createRetryableClientHttpRequestFactory(retryTemplate, factory); + if (this.vaultProperties.isFailFast()) { + if (RETRY_AVAILABLE) { + RetryTemplate retryTemplate = bootstrap.getOrElse(RetryTemplate.class, + VaultRetryUtil.createRetryTemplate(retryProperties)); + factory = VaultRetryUtil.createRetryableClientHttpRequestFactory(retryTemplate, factory); + } + else { + Environment env = bootstrap.get(Environment.class); + boolean retryPropertySet = RetryProperties.PROPERTY_SET.stream() + .anyMatch(s -> env.getProperty(s) != null); + if (retryPropertySet) { + throw new IllegalStateException( + "One or more spring.cloud.vault.retry.* properties are set, but spring-retry is not on the classpath"); + } + } } // early initialization diff --git a/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultProperties.java b/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultProperties.java index 19a3e451d..4b1834080 100644 --- a/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultProperties.java +++ b/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultProperties.java @@ -137,8 +137,6 @@ public class VaultProperties implements EnvironmentAware { private Session session = new Session(); - private Reactive reactive = new Reactive(); - /** * Application name for AppId authentication. */ @@ -367,14 +365,6 @@ public void setAuthentication(AuthenticationMethod authentication) { this.authentication = authentication; } - public Reactive getReactive() { - return reactive; - } - - public void setReactive(Reactive reactive) { - this.reactive = reactive; - } - /** * Enumeration of authentication methods. */ @@ -1362,21 +1352,4 @@ public void setExpiryThreshold(Duration expiryThreshold) { } - public static class Reactive { - - /** - * Enable reactive beans if dependencies are available - */ - private boolean enabled = true; - - public boolean isEnabled() { - return this.enabled; - } - - public void setEnabled(boolean enabled) { - this.enabled = enabled; - } - - } - } diff --git a/spring-cloud-vault-config/src/test/java/org/springframework/cloud/vault/config/VaultAutoConfigurationRetryUnavailableTests.java b/spring-cloud-vault-config/src/test/java/org/springframework/cloud/vault/config/VaultAutoConfigurationRetryUnavailableTests.java index 53906105c..d2e36d08a 100644 --- a/spring-cloud-vault-config/src/test/java/org/springframework/cloud/vault/config/VaultAutoConfigurationRetryUnavailableTests.java +++ b/spring-cloud-vault-config/src/test/java/org/springframework/cloud/vault/config/VaultAutoConfigurationRetryUnavailableTests.java @@ -18,10 +18,12 @@ import org.junit.Test; import org.junit.runner.RunWith; +import org.springframework.beans.factory.BeanCreationException; import org.springframework.boot.autoconfigure.AutoConfigurations; import org.springframework.boot.test.context.runner.ApplicationContextRunner; import org.springframework.cloud.test.ClassPathExclusions; import org.springframework.cloud.test.ModifiedClassPathRunner; +import org.springframework.core.NestedExceptionUtils; import org.springframework.http.HttpMethod; import org.springframework.http.client.ClientHttpRequest; import org.springframework.http.client.ClientHttpRequestFactory; @@ -56,4 +58,20 @@ public void shouldNotBeConfigured() { }); } + @Test + public void shouldThrowErrorIfRetryPropertiesConfigured() { + + try { + this.contextRunner + .withPropertyValues("spring.cloud.vault.kv.enabled=false", "spring.cloud.vault.fail-fast=true", + "spring.cloud.vault.token=foo", "spring.cloud.vault.retry.max-attempts=5") + .run(context -> { + }); + } + catch (BeanCreationException e) { + Throwable cause = NestedExceptionUtils.getRootCause(e); + assertThat(cause.getMessage()).contains("spring-retry is not on the classpath"); + } + } + }