From 3d0672e159305e32ca52c787e87d30eb3b4eac8b Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Fri, 20 Mar 2020 14:11:11 +0100 Subject: [PATCH] Polishing Add this for field dereference. Tweak renewal timing for shorter test runtime. Move lifecycle configuration into SecretLeaseContainer bean method. Closes gh-393. --- .../config/consul/ConsulBackendMetadata.java | 8 ++-- .../VaultConfigConsulAutoConfiguration.java | 6 +-- .../consul/ConsulSecretIntegrationTests.java | 3 +- ...nfigConsulBootstrapConfigurationTests.java | 3 +- .../config/consul/VaultConfigConsulTests.java | 13 ++---- .../src/test/resources/bootstrap.yml | 3 ++ .../config/LeasingSecretBackendMetadata.java | 21 +++++---- .../LeasingVaultPropertySourceLocator.java | 14 +++--- ...tBootstrapPropertySourceConfiguration.java | 45 ++++++++++++------- ...strapPropertySourceConfigurationTests.java | 10 ++++- 10 files changed, 74 insertions(+), 52 deletions(-) diff --git a/spring-cloud-vault-config-consul/src/main/java/org/springframework/cloud/vault/config/consul/ConsulBackendMetadata.java b/spring-cloud-vault-config-consul/src/main/java/org/springframework/cloud/vault/config/consul/ConsulBackendMetadata.java index 476ee099..b71b0bba 100644 --- a/spring-cloud-vault-config-consul/src/main/java/org/springframework/cloud/vault/config/consul/ConsulBackendMetadata.java +++ b/spring-cloud-vault-config-consul/src/main/java/org/springframework/cloud/vault/config/consul/ConsulBackendMetadata.java @@ -90,12 +90,10 @@ class ConsulBackendMetadata implements LeasingSecretBackendMetadata { if (leaseEvent.getSource() == secret && leaseEvent instanceof SecretLeaseCreatedEvent) { - if (this.eventPublisher != null) { - if (log.isDebugEnabled()) { - log.debug("Publishing a RebindConsulEvent"); - } - this.eventPublisher.publishEvent(new RebindConsulEvent(this)); + if (this.log.isDebugEnabled()) { + this.log.debug("Publishing a RebindConsulEvent"); } + this.eventPublisher.publishEvent(new RebindConsulEvent(this)); } }); diff --git a/spring-cloud-vault-config-consul/src/main/java/org/springframework/cloud/vault/config/consul/VaultConfigConsulAutoConfiguration.java b/spring-cloud-vault-config-consul/src/main/java/org/springframework/cloud/vault/config/consul/VaultConfigConsulAutoConfiguration.java index f25877c5..85097612 100644 --- a/spring-cloud-vault-config-consul/src/main/java/org/springframework/cloud/vault/config/consul/VaultConfigConsulAutoConfiguration.java +++ b/spring-cloud-vault-config-consul/src/main/java/org/springframework/cloud/vault/config/consul/VaultConfigConsulAutoConfiguration.java @@ -31,7 +31,7 @@ import org.springframework.context.annotation.Configuration; /** * Bootstrap configuration providing support for the Consul secret backend. * - * @author Mark Paluch + * @author Spencer Gibb */ @Configuration(proxyBeanMethods = false) public class VaultConfigConsulAutoConfiguration { @@ -69,8 +69,8 @@ public class VaultConfigConsulAutoConfiguration { @Override public void onApplicationEvent(ConsulBackendMetadata.RebindConsulEvent event) { - if (log.isDebugEnabled()) { - log.debug("received RebindConsulEvent"); + if (this.log.isDebugEnabled()) { + this.log.debug("received RebindConsulEvent"); } rebind("consulDiscoveryProperties"); rebind("consulConfigProperties"); diff --git a/spring-cloud-vault-config-consul/src/test/java/org/springframework/cloud/vault/config/consul/ConsulSecretIntegrationTests.java b/spring-cloud-vault-config-consul/src/test/java/org/springframework/cloud/vault/config/consul/ConsulSecretIntegrationTests.java index 844a8f75..5b16f9da 100644 --- a/spring-cloud-vault-config-consul/src/test/java/org/springframework/cloud/vault/config/consul/ConsulSecretIntegrationTests.java +++ b/spring-cloud-vault-config-consul/src/test/java/org/springframework/cloud/vault/config/consul/ConsulSecretIntegrationTests.java @@ -136,7 +136,8 @@ public class ConsulSecretIntegrationTests extends IntegrationTestSupport { Map secretProperties = this.configOperations .read(factory.forConsul(this.consul)).getData(); - assertThat(secretProperties).containsKeys("spring.cloud.consul.token"); + assertThat(secretProperties).containsKeys("spring.cloud.consul.config.acl-token", + "spring.cloud.consul.discovery.acl-token"); } } diff --git a/spring-cloud-vault-config-consul/src/test/java/org/springframework/cloud/vault/config/consul/VaultConfigConsulBootstrapConfigurationTests.java b/spring-cloud-vault-config-consul/src/test/java/org/springframework/cloud/vault/config/consul/VaultConfigConsulBootstrapConfigurationTests.java index 895c08e3..79721c5b 100644 --- a/spring-cloud-vault-config-consul/src/test/java/org/springframework/cloud/vault/config/consul/VaultConfigConsulBootstrapConfigurationTests.java +++ b/spring-cloud-vault-config-consul/src/test/java/org/springframework/cloud/vault/config/consul/VaultConfigConsulBootstrapConfigurationTests.java @@ -67,7 +67,8 @@ public class VaultConfigConsulBootstrapConfigurationTests extends IntegrationTes @Bean @ConditionalOnProperty("VaultConfigConsulBootstrapConfigurationTests.custom.config") - ConsulSecretBackendMetadataFactory customFactory(ConfigurationPropertiesRebinder rebinder) { + ConsulSecretBackendMetadataFactory customFactory( + ConfigurationPropertiesRebinder rebinder) { return new ConsulSecretBackendMetadataFactory(null) { @Override diff --git a/spring-cloud-vault-config-consul/src/test/java/org/springframework/cloud/vault/config/consul/VaultConfigConsulTests.java b/spring-cloud-vault-config-consul/src/test/java/org/springframework/cloud/vault/config/consul/VaultConfigConsulTests.java index 23b15aa2..80991389 100644 --- a/spring-cloud-vault-config-consul/src/test/java/org/springframework/cloud/vault/config/consul/VaultConfigConsulTests.java +++ b/spring-cloud-vault-config-consul/src/test/java/org/springframework/cloud/vault/config/consul/VaultConfigConsulTests.java @@ -57,6 +57,7 @@ import static org.junit.Assume.assumeTrue; * referenced with {@code ../work/keystore.jks}. * * @author Mark Paluch + * @author Spencer Gibb */ @RunWith(SpringRunner.class) @SpringBootTest(classes = VaultConfigConsulTests.TestApplication.class, @@ -130,8 +131,8 @@ public class VaultConfigConsulTests { Map role = new LinkedHashMap<>(); role.put("policy", new String(Base64.getEncoder().encode(POLICY.getBytes()))); - role.put("ttl", "15s"); - role.put("max_ttl", "15s"); + role.put("ttl", "5s"); + role.put("max_ttl", "5s"); vaultOperations.write("consul/roles/readonly", role); } @@ -144,11 +145,6 @@ public class VaultConfigConsulTests { } } - /* - * @Test public void shouldHaveToken() { assertThat(this.token).isNotEmpty(); - * assertThat(this.discoveryProperties.getAclToken()).isEqualTo(this.token); } - */ - @Test public void shouldHaveRenewedToken() throws InterruptedException { assertThat(configToken).isNotEmpty(); @@ -156,9 +152,8 @@ public class VaultConfigConsulTests { assertThat(this.configProperties.getAclToken()).isEqualTo(configToken); assertThat(this.discoveryProperties.getAclToken()).isEqualTo(discoveryToken); - Thread.sleep(20_000L); + Thread.sleep(8_000L); - // TODO: The properties weren't rebound so this test fails. assertThat(this.configProperties.getAclToken()).isNotEmpty() .isNotEqualTo(configToken); assertThat(this.discoveryProperties.getAclToken()).isNotEmpty() diff --git a/spring-cloud-vault-config-consul/src/test/resources/bootstrap.yml b/spring-cloud-vault-config-consul/src/test/resources/bootstrap.yml index c598d962..307f7807 100644 --- a/spring-cloud-vault-config-consul/src/test/resources/bootstrap.yml +++ b/spring-cloud-vault-config-consul/src/test/resources/bootstrap.yml @@ -3,3 +3,6 @@ spring: cloud.vault.token: 00000000-0000-0000-0000-000000000000 cloud.vault.ssl.trust-store: file:../work/keystore.jks cloud.vault.ssl.trust-store-password: changeit + cloud.vault.config.lifecycle.min-renewal: 3s + cloud.vault.config.lifecycle.expiry-threshold: 3s + diff --git a/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/LeasingSecretBackendMetadata.java b/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/LeasingSecretBackendMetadata.java index 93353628..555fe47d 100644 --- a/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/LeasingSecretBackendMetadata.java +++ b/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/LeasingSecretBackendMetadata.java @@ -40,26 +40,29 @@ public interface LeasingSecretBackendMetadata extends SecretBackendMetadata { Mode getLeaseMode(); /** - * Callback method before registering a {@link RequestedSecret secret} with {@link SecretLeaseContainer}. - * Registering a {@code before} callback allows event consumption before the secrets are visible in the associated property source. - * + * Callback method before registering a {@link RequestedSecret secret} with + * {@link SecretLeaseContainer}. Registering a {@code before} callback allows event + * consumption before the secrets are visible in the associated property source. * @param secret the requested secret. * @param container the lease container that was used to request the secret. * @since 3.0 */ - default void beforeRegistration(RequestedSecret secret, SecretLeaseContainer container) { + default void beforeRegistration(RequestedSecret secret, + SecretLeaseContainer container) { } /** - * Callback method after registering a {@link RequestedSecret secret} with {@link SecretLeaseContainer}. - * Registering a {@code after} callback allows event consumption after the secrets are visible in the associated property source. - * Note that this callback does not necessarily guarantee notification of the initial secrets retrieval. - * + * Callback method after registering a {@link RequestedSecret secret} with + * {@link SecretLeaseContainer}. Registering a {@code after} callback allows event + * consumption after the secrets are visible in the associated property source. Note + * that this callback does not necessarily guarantee notification of the initial + * secrets retrieval. * @param secret the requested secret. * @param container the lease container that was used to request the secret. * @since 3.0 */ - default void afterRegistration(RequestedSecret secret, SecretLeaseContainer container) { + default void afterRegistration(RequestedSecret secret, + SecretLeaseContainer container) { } } diff --git a/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/LeasingVaultPropertySourceLocator.java b/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/LeasingVaultPropertySourceLocator.java index 57fc2113..9ed76167 100644 --- a/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/LeasingVaultPropertySourceLocator.java +++ b/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/LeasingVaultPropertySourceLocator.java @@ -148,17 +148,17 @@ class LeasingVaultPropertySourceLocator extends VaultPropertySourceLocatorSuppor SecretBackendMetadata accessor) { if (accessor instanceof LeasingSecretBackendMetadata) { - ((LeasingSecretBackendMetadata) accessor) - .beforeRegistration(secret, this.secretLeaseContainer); + ((LeasingSecretBackendMetadata) accessor).beforeRegistration(secret, + this.secretLeaseContainer); } - LeaseAwareVaultPropertySource propertySource = new LeaseAwareVaultPropertySource(accessor - .getName(), - this.secretLeaseContainer, secret, accessor.getPropertyTransformer()); + LeaseAwareVaultPropertySource propertySource = new LeaseAwareVaultPropertySource( + accessor.getName(), this.secretLeaseContainer, secret, + accessor.getPropertyTransformer()); if (accessor instanceof LeasingSecretBackendMetadata) { - ((LeasingSecretBackendMetadata) accessor) - .afterRegistration(secret, this.secretLeaseContainer); + ((LeasingSecretBackendMetadata) accessor).afterRegistration(secret, + this.secretLeaseContainer); } return propertySource; diff --git a/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultBootstrapPropertySourceConfiguration.java b/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultBootstrapPropertySourceConfiguration.java index 937b2f33..68ad8c92 100644 --- a/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultBootstrapPropertySourceConfiguration.java +++ b/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/VaultBootstrapPropertySourceConfiguration.java @@ -100,18 +100,6 @@ public class VaultBootstrapPropertySourceConfiguration implements InitializingBe SecretLeaseContainer secretLeaseContainer = secretLeaseContainerObjectFactory .getObject(); - if (lifecycle.getMinRenewal() != null) { - secretLeaseContainer.setMinRenewal(lifecycle.getMinRenewal()); - } - - if (lifecycle.getExpiryThreshold() != null) { - secretLeaseContainer.setExpiryThreshold(lifecycle.getExpiryThreshold()); - } - - if (lifecycle.getLeaseEndpoints() != null) { - secretLeaseContainer.setLeaseEndpoints(lifecycle.getLeaseEndpoints()); - } - secretLeaseContainer.start(); return new LeasingVaultPropertySourceLocator(vaultProperties, configuration, @@ -184,6 +172,7 @@ public class VaultBootstrapPropertySourceConfiguration implements InitializingBe } /** + * @param vaultProperties the {@link VaultProperties}. * @param vaultOperations the {@link VaultOperations}. * @param taskSchedulerWrapper the {@link TaskSchedulerWrapper}. * @return the {@link SessionManager} for Vault session management. @@ -193,10 +182,36 @@ public class VaultBootstrapPropertySourceConfiguration implements InitializingBe @Bean @Lazy @ConditionalOnMissingBean - public SecretLeaseContainer secretLeaseContainer(VaultOperations vaultOperations, - TaskSchedulerWrapper taskSchedulerWrapper) { - return new SecretLeaseContainer(vaultOperations, + public SecretLeaseContainer secretLeaseContainer(VaultProperties vaultProperties, + VaultOperations vaultOperations, TaskSchedulerWrapper taskSchedulerWrapper) { + + VaultProperties.Lifecycle lifecycle = vaultProperties.getConfig().getLifecycle(); + + SecretLeaseContainer container = new SecretLeaseContainer(vaultOperations, taskSchedulerWrapper.getTaskScheduler()); + + customizeContainer(lifecycle, container); + + return container; + } + + static void customizeContainer(VaultProperties.Lifecycle lifecycle, + SecretLeaseContainer container) { + + if (lifecycle.isEnabled()) { + + if (lifecycle.getMinRenewal() != null) { + container.setMinRenewal(lifecycle.getMinRenewal()); + } + + if (lifecycle.getExpiryThreshold() != null) { + container.setExpiryThreshold(lifecycle.getExpiryThreshold()); + } + + if (lifecycle.getLeaseEndpoints() != null) { + container.setLeaseEndpoints(lifecycle.getLeaseEndpoints()); + } + } } } diff --git a/spring-cloud-vault-config/src/test/java/org/springframework/cloud/vault/config/VaultBootstrapPropertySourceConfigurationTests.java b/spring-cloud-vault-config/src/test/java/org/springframework/cloud/vault/config/VaultBootstrapPropertySourceConfigurationTests.java index 73bfd83e..70569734 100644 --- a/spring-cloud-vault-config/src/test/java/org/springframework/cloud/vault/config/VaultBootstrapPropertySourceConfigurationTests.java +++ b/spring-cloud-vault-config/src/test/java/org/springframework/cloud/vault/config/VaultBootstrapPropertySourceConfigurationTests.java @@ -80,8 +80,14 @@ public class VaultBootstrapPropertySourceConfigurationTests { } @Bean - SecretLeaseContainer secretLeaseContainer() { - return mock(SecretLeaseContainer.class); + SecretLeaseContainer secretLeaseContainer(VaultProperties properties) { + + SecretLeaseContainer mock = mock(SecretLeaseContainer.class); + + VaultBootstrapPropertySourceConfiguration + .customizeContainer(properties.getConfig().getLifecycle(), mock); + + return mock; } }