From 15a07387d6ce8e204761c16fb96cc7fafad12c2e Mon Sep 17 00:00:00 2001 From: Vincent Sevel Date: Wed, 22 Sep 2021 23:03:44 +0200 Subject: [PATCH] Allow retries on Vault MP Config Source initialization --- .../deployment/DevServicesVaultProcessor.java | 2 +- .../vault/runtime/VaultIOException.java | 26 +++++++++ .../runtime/client/VertxVaultClient.java | 29 ++++++++++ .../runtime/config/VaultBootstrapConfig.java | 8 ++- .../runtime/config/VaultConfigSource.java | 53 +++++++++++++++---- .../vault/VaultUnableToConnectITCase.java | 38 +++++++++++++ .../application-unable-to-connect.properties | 13 +++++ .../application-vault-approle-wrap.properties | 3 -- .../application-vault-approle.properties | 3 -- ...ication-vault-client-token-wrap.properties | 5 -- .../application-vault-enterprise.properties | 2 - .../application-vault-kubernetes.properties | 3 -- .../application-vault-multi-path.properties | 3 -- .../application-vault-pki.properties | 3 -- .../application-vault-totp.properties | 3 -- ...cation-vault-userpass-kvv1-wrap.properties | 3 -- ...cation-vault-userpass-kvv2-wrap.properties | 3 -- .../resources/application-vault.properties | 3 -- 18 files changed, 157 insertions(+), 46 deletions(-) create mode 100644 extensions/vault/runtime/src/main/java/io/quarkus/vault/runtime/VaultIOException.java create mode 100644 integration-tests/vault/src/test/java/io/quarkus/vault/VaultUnableToConnectITCase.java create mode 100644 integration-tests/vault/src/test/resources/application-unable-to-connect.properties diff --git a/extensions/vault/deployment/src/main/java/io/quarkus/vault/deployment/DevServicesVaultProcessor.java b/extensions/vault/deployment/src/main/java/io/quarkus/vault/deployment/DevServicesVaultProcessor.java index b4af708991047..080cd1e78cfda 100644 --- a/extensions/vault/deployment/src/main/java/io/quarkus/vault/deployment/DevServicesVaultProcessor.java +++ b/extensions/vault/deployment/src/main/java/io/quarkus/vault/deployment/DevServicesVaultProcessor.java @@ -132,7 +132,7 @@ private StartResult startContainer(DevServicesConfig devServicesConfig) { boolean needToStart = !ConfigUtils.isPropertyPresent(URL_CONFIG_KEY); if (!needToStart) { - log.debug("Not starting devservices for default Vault client as url have been provided"); + log.debug("Not starting devservices for default Vault client as url has been provided"); return null; } diff --git a/extensions/vault/runtime/src/main/java/io/quarkus/vault/runtime/VaultIOException.java b/extensions/vault/runtime/src/main/java/io/quarkus/vault/runtime/VaultIOException.java new file mode 100644 index 0000000000000..5058b92d13d90 --- /dev/null +++ b/extensions/vault/runtime/src/main/java/io/quarkus/vault/runtime/VaultIOException.java @@ -0,0 +1,26 @@ +package io.quarkus.vault.runtime; + +import io.quarkus.vault.VaultException; + +public class VaultIOException extends VaultException { + + public VaultIOException() { + } + + public VaultIOException(String message) { + super(message); + } + + public VaultIOException(String message, Throwable cause) { + super(message, cause); + } + + public VaultIOException(Throwable cause) { + super(cause); + } + + public VaultIOException(String message, Throwable cause, boolean enableSuppression, boolean writableStackTrace) { + super(message, cause, enableSuppression, writableStackTrace); + } + +} diff --git a/extensions/vault/runtime/src/main/java/io/quarkus/vault/runtime/client/VertxVaultClient.java b/extensions/vault/runtime/src/main/java/io/quarkus/vault/runtime/client/VertxVaultClient.java index 6c0d6ddf2e4e5..db2c5b55c87ae 100644 --- a/extensions/vault/runtime/src/main/java/io/quarkus/vault/runtime/client/VertxVaultClient.java +++ b/extensions/vault/runtime/src/main/java/io/quarkus/vault/runtime/client/VertxVaultClient.java @@ -4,6 +4,7 @@ import static io.quarkus.vault.runtime.client.MutinyVertxClientFactory.createHttpClient; import static java.util.Collections.emptyMap; +import java.net.ConnectException; import java.net.MalformedURLException; import java.net.URL; import java.time.Duration; @@ -11,6 +12,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.concurrent.CompletionException; import java.util.function.Supplier; import javax.annotation.PreDestroy; @@ -24,8 +26,10 @@ import io.quarkus.runtime.TlsConfig; import io.quarkus.vault.VaultException; import io.quarkus.vault.runtime.VaultConfigHolder; +import io.quarkus.vault.runtime.VaultIOException; import io.quarkus.vault.runtime.config.VaultBootstrapConfig; import io.smallrye.mutiny.Uni; +import io.vertx.core.VertxException; import io.vertx.core.http.HttpMethod; import io.vertx.mutiny.core.Vertx; import io.vertx.mutiny.core.buffer.Buffer; @@ -172,6 +176,7 @@ private T exec(HttpRequest request, Object body, Class resultClas try { Uni> uni = body == null ? request.send() : request.sendBuffer(Buffer.buffer(requestBody(body))); + HttpResponse response = uni.await().atMost(getRequestTimeout()); if (response.statusCode() != expectedCode) { @@ -183,8 +188,32 @@ private T exec(HttpRequest request, Object body, Class resultClas } else { return null; } + } catch (JsonProcessingException e) { throw new VaultException(e); + + } catch (io.smallrye.mutiny.TimeoutException e) { + // happens if we reach the atMost condition - see UniAwait.atMost(Duration) + throw new VaultIOException(e); + + } catch (VertxException e) { + if ("Connection was closed".equals(e.getMessage())) { + // happens if the connection gets closed (idle timeout, reset by peer, ...) + throw new VaultIOException(e); + } else { + throw e; + } + + } catch (CompletionException e) { + if (e.getCause() instanceof ConnectException) { + // unable to establish connection + throw new VaultIOException(e); + } else if (e.getCause() instanceof java.util.concurrent.TimeoutException) { + // timeout on request - see HttpRequest.timeout(long) + throw new VaultIOException(e); + } else { + throw e; + } } } diff --git a/extensions/vault/runtime/src/main/java/io/quarkus/vault/runtime/config/VaultBootstrapConfig.java b/extensions/vault/runtime/src/main/java/io/quarkus/vault/runtime/config/VaultBootstrapConfig.java index 561d38880d953..2abdc23e13c57 100644 --- a/extensions/vault/runtime/src/main/java/io/quarkus/vault/runtime/config/VaultBootstrapConfig.java +++ b/extensions/vault/runtime/src/main/java/io/quarkus/vault/runtime/config/VaultBootstrapConfig.java @@ -33,7 +33,7 @@ public class VaultBootstrapConfig { public static final String DEFAULT_SECRET_CONFIG_CACHE_PERIOD = "10M"; public static final String KUBERNETES_CACERT = "/var/run/secrets/kubernetes.io/serviceaccount/ca.crt"; public static final String DEFAULT_CONNECT_TIMEOUT = "5S"; - public static final String DEFAULT_READ_TIMEOUT = "1S"; + public static final String DEFAULT_READ_TIMEOUT = "5S"; public static final String DEFAULT_TLS_USE_KUBERNETES_CACERT = "true"; public static final String DEFAULT_KUBERNETES_AUTH_MOUNT_PATH = "auth/kubernetes"; @@ -141,6 +141,12 @@ public class VaultBootstrapConfig { @ConfigDocMapKey("prefix") public Map secretConfigKvPathPrefix; + /** + * Maximum number of attempts when fetching MP Config properties on the initial connection. + */ + @ConfigItem(defaultValue = "1") + public int mpConfigInitialAttempts; + /** * Used to hide confidential infos, for logging in particular. * Possible values are: diff --git a/extensions/vault/runtime/src/main/java/io/quarkus/vault/runtime/config/VaultConfigSource.java b/extensions/vault/runtime/src/main/java/io/quarkus/vault/runtime/config/VaultConfigSource.java index 5c9c1a50ccc3b..b77a2d3c64068 100644 --- a/extensions/vault/runtime/src/main/java/io/quarkus/vault/runtime/config/VaultConfigSource.java +++ b/extensions/vault/runtime/src/main/java/io/quarkus/vault/runtime/config/VaultConfigSource.java @@ -15,7 +15,9 @@ import org.jboss.logging.Logger; import io.quarkus.arc.Arc; +import io.quarkus.vault.VaultException; import io.quarkus.vault.VaultKVSecretEngine; +import io.quarkus.vault.runtime.VaultIOException; import io.smallrye.mutiny.infrastructure.Infrastructure; public class VaultConfigSource implements ConfigSource { @@ -24,6 +26,7 @@ public class VaultConfigSource implements ConfigSource { private AtomicReference>> cache = new AtomicReference<>(null); private VaultBootstrapConfig vaultBootstrapConfig; + private volatile boolean firstTime = true; public VaultConfigSource(VaultBootstrapConfig vaultBootstrapConfig) { this.vaultBootstrapConfig = vaultBootstrapConfig; @@ -73,21 +76,51 @@ private Map getSecretConfig() { Map properties = new HashMap<>(); - try { - // default kv paths - vaultBootstrapConfig.secretConfigKvPath.ifPresent(strings -> fetchSecrets(strings, null, properties)); - - // prefixed kv paths - vaultBootstrapConfig.secretConfigKvPathPrefix.forEach((key, value) -> fetchSecrets(value.paths, key, properties)); - - log.debug("loaded " + properties.size() + " properties from vault"); - } catch (RuntimeException e) { - return tryReturnLastKnownValue(e, cacheEntry); + if (firstTime) { + log.debug("fetch secrets first time with attempts = " + vaultBootstrapConfig.mpConfigInitialAttempts); + fetchSecretsFirstTime(properties); + firstTime = false; + } else { + try { + fetchSecrets(properties); + log.debug("refreshed " + properties.size() + " properties from vault"); + } catch (RuntimeException e) { + return tryReturnLastKnownValue(e, cacheEntry); + } } cache.set(new VaultCacheEntry(properties)); return properties; + } + + private void fetchSecretsFirstTime(Map properties) { + VaultIOException last = null; + for (int i = 0; i < vaultBootstrapConfig.mpConfigInitialAttempts; i++) { + try { + if (i > 0) { + log.debug("retrying to fetch secrets"); + } + fetchSecrets(properties); + log.debug("loaded " + properties.size() + " properties from vault"); + return; + } catch (VaultIOException e) { + log.debug("attempt " + (i + 1) + " to fetch secrets from vault failed with: " + e); + last = e; + } + } + if (last == null) { + throw new VaultException("unexpected"); + } else { + throw last; + } + } + + private void fetchSecrets(Map properties) { + // default kv paths + vaultBootstrapConfig.secretConfigKvPath.ifPresent(strings -> fetchSecrets(strings, null, properties)); + // prefixed kv paths + vaultBootstrapConfig.secretConfigKvPathPrefix.forEach((key, value) -> fetchSecrets(value.paths, key, properties)); } private void fetchSecrets(List paths, String prefix, Map properties) { diff --git a/integration-tests/vault/src/test/java/io/quarkus/vault/VaultUnableToConnectITCase.java b/integration-tests/vault/src/test/java/io/quarkus/vault/VaultUnableToConnectITCase.java new file mode 100644 index 0000000000000..8cb0f6ca6d36c --- /dev/null +++ b/integration-tests/vault/src/test/java/io/quarkus/vault/VaultUnableToConnectITCase.java @@ -0,0 +1,38 @@ +package io.quarkus.vault; + +import static io.quarkus.credentials.CredentialsProvider.PASSWORD_PROPERTY_NAME; + +import org.eclipse.microprofile.config.inject.ConfigProperty; +import org.jboss.shrinkwrap.api.ShrinkWrap; +import org.jboss.shrinkwrap.api.spec.JavaArchive; +import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.test.QuarkusUnitTest; + +@Disabled // test is expected to fail on quarkus app startup +public class VaultUnableToConnectITCase { + + // start the test with no vault listening on 4850 + // the application startup should fail with the following debug logs 3 times: + // attempt ... to fetch secrets from vault failed with: ... + // retrying to fetch secrets + // ... + // you can also validate the retry on read timeout by starting a server that answers + // POSTs on http://localhost:4850/v1/auth/userpass/login/{user} and sleeps more than 10 secs + // again you are expected to see several attempts in the logs + + @RegisterExtension + static final QuarkusUnitTest config = new QuarkusUnitTest() + .setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class) + .addAsResource("application-unable-to-connect.properties", "application.properties")); + + @ConfigProperty(name = PASSWORD_PROPERTY_NAME) + String someSecret; + + @Test + void test() { + // we will not reach that point + } +} diff --git a/integration-tests/vault/src/test/resources/application-unable-to-connect.properties b/integration-tests/vault/src/test/resources/application-unable-to-connect.properties new file mode 100644 index 0000000000000..4a285d4c72eb9 --- /dev/null +++ b/integration-tests/vault/src/test/resources/application-unable-to-connect.properties @@ -0,0 +1,13 @@ +quarkus.vault.url=http://localhost:4850 +quarkus.vault.authentication.userpass.username=bob +quarkus.vault.authentication.userpass.password=sinclair +quarkus.vault.secret-config-kv-path=config + +quarkus.vault.read-timeout=10S +quarkus.vault.mp-config-initial-attempts=3 + +quarkus.log.category."io.quarkus.vault".level=DEBUG + +#quarkus.log.level=DEBUG +quarkus.log.min-level=DEBUG +quarkus.log.console.level=DEBUG diff --git a/integration-tests/vault/src/test/resources/application-vault-approle-wrap.properties b/integration-tests/vault/src/test/resources/application-vault-approle-wrap.properties index dbd05146c2832..a5b5022242171 100644 --- a/integration-tests/vault/src/test/resources/application-vault-approle-wrap.properties +++ b/integration-tests/vault/src/test/resources/application-vault-approle-wrap.properties @@ -12,8 +12,5 @@ quarkus.vault.renew-grace-period=10 quarkus.log.category."io.quarkus.vault".level=DEBUG -# CI can sometimes be slow, there is no need to fail a test if Vault doesn't respond in 1 second which is the default -quarkus.vault.read-timeout=5S - #quarkus.log.level=DEBUG #quarkus.log.console.level=DEBUG diff --git a/integration-tests/vault/src/test/resources/application-vault-approle.properties b/integration-tests/vault/src/test/resources/application-vault-approle.properties index 9563b50fea6ae..1610ab4cc5429 100644 --- a/integration-tests/vault/src/test/resources/application-vault-approle.properties +++ b/integration-tests/vault/src/test/resources/application-vault-approle.properties @@ -12,8 +12,5 @@ quarkus.vault.renew-grace-period=10 quarkus.log.category."io.quarkus.vault".level=DEBUG -# CI can sometimes be slow, there is no need to fail a test if Vault doesn't respond in 1 second which is the default -quarkus.vault.read-timeout=5S - #quarkus.log.level=DEBUG #quarkus.log.console.level=DEBUG diff --git a/integration-tests/vault/src/test/resources/application-vault-client-token-wrap.properties b/integration-tests/vault/src/test/resources/application-vault-client-token-wrap.properties index 7eeb017496d2f..3e91e1eef4316 100644 --- a/integration-tests/vault/src/test/resources/application-vault-client-token-wrap.properties +++ b/integration-tests/vault/src/test/resources/application-vault-client-token-wrap.properties @@ -11,10 +11,5 @@ quarkus.vault.renew-grace-period=10 quarkus.log.category."io.quarkus.vault".level=DEBUG -# CI can sometimes be slow, there is no need to fail a test if Vault doesn't respond in 1 second which is the default -quarkus.vault.read-timeout=5S - #quarkus.log.level=DEBUG #quarkus.log.console.level=DEBUG - - diff --git a/integration-tests/vault/src/test/resources/application-vault-enterprise.properties b/integration-tests/vault/src/test/resources/application-vault-enterprise.properties index 92fc085d54aa8..a41a4f98db3ce 100644 --- a/integration-tests/vault/src/test/resources/application-vault-enterprise.properties +++ b/integration-tests/vault/src/test/resources/application-vault-enterprise.properties @@ -3,5 +3,3 @@ quarkus.vault.enterprise.namespace=accounting quarkus.vault.authentication.client-token=123 quarkus.vault.kv-secret-engine-version=1 quarkus.tls.trust-all=true -# CI can sometimes be slow, there is no need to fail a test if Vault doesn't respond in 1 second which is the default -quarkus.vault.read-timeout=5S diff --git a/integration-tests/vault/src/test/resources/application-vault-kubernetes.properties b/integration-tests/vault/src/test/resources/application-vault-kubernetes.properties index 271bd62d1e4a3..d1454732b01e8 100644 --- a/integration-tests/vault/src/test/resources/application-vault-kubernetes.properties +++ b/integration-tests/vault/src/test/resources/application-vault-kubernetes.properties @@ -12,8 +12,5 @@ quarkus.vault.renew-grace-period=10 quarkus.log.category."io.quarkus.vault".level=DEBUG -# CI can sometimes be slow, there is no need to fail a test if Vault doesn't respond in 1 second which is the default -quarkus.vault.read-timeout=5S - #quarkus.log.level=DEBUG #quarkus.log.console.level=DEBUG diff --git a/integration-tests/vault/src/test/resources/application-vault-multi-path.properties b/integration-tests/vault/src/test/resources/application-vault-multi-path.properties index 54fc75776fd26..49065a038b625 100644 --- a/integration-tests/vault/src/test/resources/application-vault-multi-path.properties +++ b/integration-tests/vault/src/test/resources/application-vault-multi-path.properties @@ -5,6 +5,3 @@ quarkus.vault.secret-config-kv-path=multi/default1,multi/default2 quarkus.vault.secret-config-kv-path.singer=multi/singer1,multi/singer2 quarkus.tls.trust-all=true - -# CI can sometimes be slow, there is no need to fail a test if Vault doesn't respond in 1 second which is the default -quarkus.vault.read-timeout=5S diff --git a/integration-tests/vault/src/test/resources/application-vault-pki.properties b/integration-tests/vault/src/test/resources/application-vault-pki.properties index fdcc775e0ca40..c248f90192ea4 100644 --- a/integration-tests/vault/src/test/resources/application-vault-pki.properties +++ b/integration-tests/vault/src/test/resources/application-vault-pki.properties @@ -8,6 +8,3 @@ quarkus.vault.log-confidentiality-level=low quarkus.vault.renew-grace-period=10 quarkus.log.category."io.quarkus.vault".level=DEBUG - -# CI can sometimes be slow, there is no need to fail a test if Vault doesn't respond in 1 second which is the default -quarkus.vault.read-timeout=5S diff --git a/integration-tests/vault/src/test/resources/application-vault-totp.properties b/integration-tests/vault/src/test/resources/application-vault-totp.properties index cb5c9c4037ccc..5a958a8ce761b 100644 --- a/integration-tests/vault/src/test/resources/application-vault-totp.properties +++ b/integration-tests/vault/src/test/resources/application-vault-totp.properties @@ -3,6 +3,3 @@ quarkus.vault.authentication.userpass.username=bob quarkus.vault.authentication.userpass.password=sinclair quarkus.vault.tls.skip-verify=true - -# CI can sometimes be slow, there is no need to fail a test if Vault doesn't respond in 1 second which is the default -quarkus.vault.read-timeout=5S diff --git a/integration-tests/vault/src/test/resources/application-vault-userpass-kvv1-wrap.properties b/integration-tests/vault/src/test/resources/application-vault-userpass-kvv1-wrap.properties index b22fc51e82ce7..a7582ecddf011 100644 --- a/integration-tests/vault/src/test/resources/application-vault-userpass-kvv1-wrap.properties +++ b/integration-tests/vault/src/test/resources/application-vault-userpass-kvv1-wrap.properties @@ -15,8 +15,5 @@ quarkus.vault.renew-grace-period=10 quarkus.log.category."io.quarkus.vault".level=DEBUG -# CI can sometimes be slow, there is no need to fail a test if Vault doesn't respond in 1 second which is the default -quarkus.vault.read-timeout=5S - #quarkus.log.level=DEBUG #quarkus.log.console.level=DEBUG diff --git a/integration-tests/vault/src/test/resources/application-vault-userpass-kvv2-wrap.properties b/integration-tests/vault/src/test/resources/application-vault-userpass-kvv2-wrap.properties index 8994c8fb544d2..7e75cda27f71b 100644 --- a/integration-tests/vault/src/test/resources/application-vault-userpass-kvv2-wrap.properties +++ b/integration-tests/vault/src/test/resources/application-vault-userpass-kvv2-wrap.properties @@ -12,8 +12,5 @@ quarkus.vault.renew-grace-period=10 quarkus.log.category."io.quarkus.vault".level=DEBUG -# CI can sometimes be slow, there is no need to fail a test if Vault doesn't respond in 1 second which is the default -quarkus.vault.read-timeout=5S - #quarkus.log.level=DEBUG #quarkus.log.console.level=DEBUG diff --git a/integration-tests/vault/src/test/resources/application-vault.properties b/integration-tests/vault/src/test/resources/application-vault.properties index d8a64caddb801..7ad8d1a68510b 100644 --- a/integration-tests/vault/src/test/resources/application-vault.properties +++ b/integration-tests/vault/src/test/resources/application-vault.properties @@ -20,8 +20,5 @@ quarkus.vault.renew-grace-period=10 quarkus.log.category."io.quarkus.vault".level=DEBUG -# CI can sometimes be slow, there is no need to fail a test if Vault doesn't respond in 1 second which is the default -quarkus.vault.read-timeout=5S - #quarkus.log.level=DEBUG #quarkus.log.console.level=DEBUG