From 56ee82d6fb5950242fb2a6ef5e2712d949b74bb8 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Tue, 23 Jan 2018 15:00:30 +0100 Subject: [PATCH] Do not register discovery client beans if Vault integration is disabled. We now no longer register discovery client beans if spring.cloud.vault.enabled is set to false. See gh-186. --- ...veryClientVaultBootstrapConfiguration.java | 70 +++++---- ...lientVaultBootstrapConfigurationTests.java | 138 ++++++++++++++++++ 2 files changed, 176 insertions(+), 32 deletions(-) create mode 100644 spring-cloud-vault-config/src/test/java/org/springframework/cloud/vault/config/DiscoveryClientVaultBootstrapConfigurationTests.java diff --git a/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/DiscoveryClientVaultBootstrapConfiguration.java b/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/DiscoveryClientVaultBootstrapConfiguration.java index b1a6f8fa..caa1b283 100644 --- a/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/DiscoveryClientVaultBootstrapConfiguration.java +++ b/spring-cloud-vault-config/src/main/java/org/springframework/cloud/vault/config/DiscoveryClientVaultBootstrapConfiguration.java @@ -24,6 +24,7 @@ import org.springframework.cloud.client.ServiceInstance; import org.springframework.cloud.client.discovery.DiscoveryClient; import org.springframework.cloud.client.discovery.EnableDiscoveryClient; import org.springframework.cloud.commons.util.UtilAutoConfiguration; +import org.springframework.cloud.vault.config.DiscoveryClientVaultBootstrapConfiguration.DiscoveryBootstrapConfiguration; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Import; @@ -45,50 +46,55 @@ import org.springframework.vault.client.VaultEndpointProvider; @EnableConfigurationProperties(VaultProperties.class) @Order(Ordered.LOWEST_PRECEDENCE - 2) @EnableDiscoveryClient -@Import(UtilAutoConfiguration.class) +@Import({ UtilAutoConfiguration.class, DiscoveryBootstrapConfiguration.class }) public class DiscoveryClientVaultBootstrapConfiguration { - private final VaultProperties vaultProperties; + @ConditionalOnProperty(name = "spring.cloud.vault.enabled", matchIfMissing = true) + @Configuration + static class DiscoveryBootstrapConfiguration { - public DiscoveryClientVaultBootstrapConfiguration(VaultProperties vaultProperties) { - this.vaultProperties = vaultProperties; - } + private final VaultProperties vaultProperties; - @Bean - @ConditionalOnMissingBean - public VaultServiceInstanceProvider vaultServerInstanceProvider( - DiscoveryClient discoveryClient) { - return new DiscoveryClientVaultServiceInstanceProvider(discoveryClient); - } + public DiscoveryBootstrapConfiguration(VaultProperties vaultProperties) { + this.vaultProperties = vaultProperties; + } + + @Bean + @ConditionalOnMissingBean + public VaultServiceInstanceProvider vaultServerInstanceProvider( + DiscoveryClient discoveryClient) { + return new DiscoveryClientVaultServiceInstanceProvider(discoveryClient); + } @Bean @ConditionalOnMissingBean public VaultEndpointProvider vaultEndpointProvider( VaultServiceInstanceProvider instanceProvider) { - final String serviceId = this.vaultProperties.getDiscovery().getServiceId(); + final String serviceId = this.vaultProperties.getDiscovery().getServiceId(); - final String fallbackScheme; + final String fallbackScheme; - if (StringUtils.hasText(this.vaultProperties.getUri())) { - fallbackScheme = URI.create(this.vaultProperties.getUri()).getScheme(); + if (StringUtils.hasText(this.vaultProperties.getUri())) { + fallbackScheme = URI.create(this.vaultProperties.getUri()).getScheme(); + } + else { + fallbackScheme = this.vaultProperties.getScheme(); + } + + ServiceInstance server = instanceProvider.getVaultServerInstance(serviceId); + + final VaultEndpoint vaultEndpoint = VaultEndpoint.create(server.getHost(), + server.getPort()); + + if (server.getMetadata().containsKey("scheme")) { + vaultEndpoint.setScheme(server.getMetadata().get("scheme")); + } + else { + vaultEndpoint.setScheme(server.isSecure() ? "https" : fallbackScheme); + } + + return () -> vaultEndpoint; } - else { - fallbackScheme = this.vaultProperties.getScheme(); - } - - ServiceInstance server = instanceProvider.getVaultServerInstance(serviceId); - - final VaultEndpoint vaultEndpoint = VaultEndpoint.create(server.getHost(), - server.getPort()); - - if (server.getMetadata().containsKey("scheme")) { - vaultEndpoint.setScheme(server.getMetadata().get("scheme")); - } - else { - vaultEndpoint.setScheme(server.isSecure() ? "https" : fallbackScheme); - } - - return () -> vaultEndpoint; } } diff --git a/spring-cloud-vault-config/src/test/java/org/springframework/cloud/vault/config/DiscoveryClientVaultBootstrapConfigurationTests.java b/spring-cloud-vault-config/src/test/java/org/springframework/cloud/vault/config/DiscoveryClientVaultBootstrapConfigurationTests.java new file mode 100644 index 00000000..22c161cc --- /dev/null +++ b/spring-cloud-vault-config/src/test/java/org/springframework/cloud/vault/config/DiscoveryClientVaultBootstrapConfigurationTests.java @@ -0,0 +1,138 @@ +/* + * Copyright 2018 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 + * + * http://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 java.net.URI; +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.Map; + +import lombok.Data; +import org.junit.Test; +import org.mockito.Mockito; + +import org.springframework.boot.test.util.TestPropertyValues; +import org.springframework.cloud.client.ServiceInstance; +import org.springframework.cloud.client.discovery.DiscoveryClient; +import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.vault.client.VaultEndpoint; +import org.springframework.vault.client.VaultEndpointProvider; + +import static org.assertj.core.api.Assertions.*; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.*; + +/** + * Tests for {@link ReactiveVaultBootstrapConfiguration}. + * + * @author Mark Paluch + */ +public class DiscoveryClientVaultBootstrapConfigurationTests { + + private AnnotationConfigApplicationContext context; + + @Test + public void shouldRegisterDefaultBeans() { + + load(DiscoveryConfiguration.class, "spring.cloud.vault.token=foo", + "spring.cloud.vault.discovery.enabled=true"); + + assertThat(context.getBean(VaultServiceInstanceProvider.class)).isInstanceOf( + DiscoveryClientVaultServiceInstanceProvider.class); + + VaultEndpointProvider endpointProvider = context + .getBean(VaultEndpointProvider.class); + VaultEndpoint vaultEndpoint = endpointProvider.getVaultEndpoint(); + assertThat(vaultEndpoint.getPort()).isEqualTo(1234); + } + + @Test + public void shouldNotRegisterBeansIfDiscoveryDisabled() { + + load(DiscoveryConfiguration.class, "spring.cloud.vault.token=foo", + "spring.cloud.vault.discovery.enabled=false"); + + assertThat(context.getBeanNamesForType(VaultServiceInstanceProvider.class)) + .isEmpty(); + } + + @Test + public void shouldNotRegisterBeansIfVaultDisabled() { + + load(DiscoveryConfiguration.class, "spring.cloud.vault.token=foo", + "spring.cloud.vault.enabled=false"); + + assertThat(context.getBeanNamesForType(VaultServiceInstanceProvider.class)) + .isEmpty(); + } + + private void load(Class config, String... environment) { + + AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(); + + TestPropertyValues.of(environment).applyTo(ctx); + + ctx.register(config); + ctx.register(DiscoveryClientVaultBootstrapConfiguration.class); + ctx.register(VaultBootstrapConfiguration.class); + ctx.refresh(); + + this.context = ctx; + } + + @Configuration + static class DiscoveryConfiguration { + + @Bean + DiscoveryClient discoveryClient() { + + DiscoveryClient mock = Mockito.mock(DiscoveryClient.class); + when(mock.getInstances(anyString())).thenReturn( + Collections.singletonList(new SimpleServiceInstance(URI + .create("https://foo:1234")))); + + return mock; + } + } + + @Data + static class SimpleServiceInstance implements ServiceInstance { + private URI uri; + private String host; + private int port; + private boolean secure; + private Map metadata = new LinkedHashMap<>(); + private String serviceId; + + public SimpleServiceInstance(URI uri) { + this.setUri(uri); + } + + public void setUri(URI uri) { + this.uri = uri; + this.host = this.uri.getHost(); + this.port = this.uri.getPort(); + String scheme = this.uri.getScheme(); + if ("https".equals(scheme)) { + this.secure = true; + } + + } + } + +}