From d96947720c8b0a0e71b2f4018047d60c738ee790 Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Fri, 3 Jun 2022 16:10:05 -0400 Subject: [PATCH] Config server function should return empty list when discovery client disabled (#298) --- ...onditionalOnZookeeperDiscoveryEnabled.java | 7 ++- .../ZookeeperConfigServerBootstrapper.java | 9 +++- ...eeperServiceRegistryAutoConfiguration.java | 2 + ...ookeeperConfigServerBootstrapperTests.java | 50 +++++++++++++++++-- 4 files changed, 60 insertions(+), 8 deletions(-) diff --git a/spring-cloud-zookeeper-discovery/src/main/java/org/springframework/cloud/zookeeper/discovery/ConditionalOnZookeeperDiscoveryEnabled.java b/spring-cloud-zookeeper-discovery/src/main/java/org/springframework/cloud/zookeeper/discovery/ConditionalOnZookeeperDiscoveryEnabled.java index d87a51be..0466c678 100644 --- a/spring-cloud-zookeeper-discovery/src/main/java/org/springframework/cloud/zookeeper/discovery/ConditionalOnZookeeperDiscoveryEnabled.java +++ b/spring-cloud-zookeeper-discovery/src/main/java/org/springframework/cloud/zookeeper/discovery/ConditionalOnZookeeperDiscoveryEnabled.java @@ -33,7 +33,10 @@ import org.springframework.cloud.zookeeper.ConditionalOnZookeeperEnabled; @Retention(RetentionPolicy.RUNTIME) @Target({ ElementType.TYPE, ElementType.METHOD }) @ConditionalOnZookeeperEnabled -@ConditionalOnProperty(value = "spring.cloud.zookeeper.discovery.enabled", matchIfMissing = true) +@ConditionalOnProperty(value = ConditionalOnZookeeperDiscoveryEnabled.PROPERTY, matchIfMissing = true) public @interface ConditionalOnZookeeperDiscoveryEnabled { - + /** + * Property name. + */ + String PROPERTY = "spring.cloud.zookeeper.discovery.enabled"; } diff --git a/spring-cloud-zookeeper-discovery/src/main/java/org/springframework/cloud/zookeeper/discovery/configclient/ZookeeperConfigServerBootstrapper.java b/spring-cloud-zookeeper-discovery/src/main/java/org/springframework/cloud/zookeeper/discovery/configclient/ZookeeperConfigServerBootstrapper.java index 83a11868..5a67f7fc 100644 --- a/spring-cloud-zookeeper-discovery/src/main/java/org/springframework/cloud/zookeeper/discovery/configclient/ZookeeperConfigServerBootstrapper.java +++ b/spring-cloud-zookeeper-discovery/src/main/java/org/springframework/cloud/zookeeper/discovery/configclient/ZookeeperConfigServerBootstrapper.java @@ -16,6 +16,8 @@ package org.springframework.cloud.zookeeper.discovery.configclient; +import java.util.Collections; + import org.apache.curator.framework.CuratorFramework; import org.apache.curator.x.discovery.ServiceDiscovery; import org.apache.curator.x.discovery.ServiceDiscoveryBuilder; @@ -32,6 +34,7 @@ import org.springframework.cloud.commons.util.InetUtilsProperties; import org.springframework.cloud.config.client.ConfigClientProperties; import org.springframework.cloud.config.client.ConfigServerInstanceProvider; import org.springframework.cloud.zookeeper.CuratorFactory; +import org.springframework.cloud.zookeeper.discovery.ConditionalOnZookeeperDiscoveryEnabled; import org.springframework.cloud.zookeeper.discovery.ZookeeperDiscoveryClient; import org.springframework.cloud.zookeeper.discovery.ZookeeperDiscoveryProperties; import org.springframework.cloud.zookeeper.discovery.ZookeeperInstance; @@ -102,7 +105,7 @@ public class ZookeeperConfigServerBootstrapper implements BootstrapRegistryIniti // create instance provider registry.registerIfAbsent(ConfigServerInstanceProvider.Function.class, context -> { if (!isEnabled(context.get(Binder.class))) { - return null; + return (id) -> Collections.emptyList(); } return context.get(ZookeeperDiscoveryClient.class)::getInstances; }); @@ -122,7 +125,9 @@ public class ZookeeperConfigServerBootstrapper implements BootstrapRegistryIniti } private boolean isEnabled(Binder binder) { - return binder.bind(ConfigClientProperties.CONFIG_DISCOVERY_ENABLED, Boolean.class).orElse(false); + return binder.bind(ConfigClientProperties.CONFIG_DISCOVERY_ENABLED, Boolean.class).orElse(false) && + binder.bind(ConditionalOnZookeeperDiscoveryEnabled.PROPERTY, Boolean.class).orElse(true) && + binder.bind("spring.cloud.discovery.enabled", Boolean.class).orElse(true); } } diff --git a/spring-cloud-zookeeper-discovery/src/main/java/org/springframework/cloud/zookeeper/serviceregistry/ZookeeperServiceRegistryAutoConfiguration.java b/spring-cloud-zookeeper-discovery/src/main/java/org/springframework/cloud/zookeeper/serviceregistry/ZookeeperServiceRegistryAutoConfiguration.java index eeecb9e4..d07216ca 100644 --- a/spring-cloud-zookeeper-discovery/src/main/java/org/springframework/cloud/zookeeper/serviceregistry/ZookeeperServiceRegistryAutoConfiguration.java +++ b/spring-cloud-zookeeper-discovery/src/main/java/org/springframework/cloud/zookeeper/serviceregistry/ZookeeperServiceRegistryAutoConfiguration.java @@ -22,6 +22,7 @@ import org.apache.curator.x.discovery.details.JsonInstanceSerializer; import org.springframework.beans.BeansException; import org.springframework.boot.autoconfigure.AutoConfigureBefore; +import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.cloud.client.serviceregistry.ServiceRegistryAutoConfiguration; @@ -52,6 +53,7 @@ public class ZookeeperServiceRegistryAutoConfiguration } @Bean + @ConditionalOnBean(ServiceDiscovery.class) @SuppressWarnings("unchecked") public ZookeeperServiceRegistry zookeeperServiceRegistry() { return new ZookeeperServiceRegistry(this.context.getBean(ServiceDiscovery.class)); diff --git a/spring-cloud-zookeeper-discovery/src/test/java/org/springframework/cloud/zookeeper/discovery/configclient/ZookeeperConfigServerBootstrapperTests.java b/spring-cloud-zookeeper-discovery/src/test/java/org/springframework/cloud/zookeeper/discovery/configclient/ZookeeperConfigServerBootstrapperTests.java index d02cc161..a66ca956 100644 --- a/spring-cloud-zookeeper-discovery/src/test/java/org/springframework/cloud/zookeeper/discovery/configclient/ZookeeperConfigServerBootstrapperTests.java +++ b/spring-cloud-zookeeper-discovery/src/test/java/org/springframework/cloud/zookeeper/discovery/configclient/ZookeeperConfigServerBootstrapperTests.java @@ -52,15 +52,15 @@ public class ZookeeperConfigServerBootstrapperTests { } @Test - public void notEnabledDoesNotAddInstanceProviderFn() { + public void notEnabledReturnsEmptyList() { ConfigurableApplicationContext context = new SpringApplicationBuilder(TestConfig.class) .listeners(new ZookeeperTestingServer()) .properties("--server.port=0", "spring.cloud.service-registry.auto-registration.enabled=false") .addBootstrapRegistryInitializer(registry -> registry.addCloseListener(event -> { ConfigServerInstanceProvider.Function providerFn = event.getBootstrapContext() .get(ConfigServerInstanceProvider.Function.class); - assertThat(providerFn).as("ConfigServerInstanceProvider.Function was created when it shouldn't") - .isNull(); + assertThat(providerFn.apply("id")).as("ConfigServerInstanceProvider.Function should return empty list") + .isEmpty(); })).run(); CuratorFramework curatorFramework = context.getBean("curatorFramework", CuratorFramework.class); assertThat(curatorFramework).isNotNull(); @@ -69,6 +69,48 @@ public class ZookeeperConfigServerBootstrapperTests { context.close(); } + @Test + public void zookeeperDiscoveryClientDisabledReturnsEmptyList() { + ConfigurableApplicationContext context = new SpringApplicationBuilder(TestConfig.class) + .listeners(new ZookeeperTestingServer()) + .properties("--server.port=0", "spring.cloud.config.discovery.enabled=true", + "spring.cloud.zookeeper.discovery.enabled=false", + "spring.cloud.zookeeper.discovery.metadata[mymetadataprop]=mymetadataval", + "spring.cloud.service-registry.auto-registration.enabled=false") + .addBootstrapRegistryInitializer(registry -> registry.addCloseListener(event -> { + ConfigServerInstanceProvider.Function providerFn = event.getBootstrapContext() + .get(ConfigServerInstanceProvider.Function.class); + assertThat(providerFn.apply("id")).as("ConfigServerInstanceProvider.Function should return empty list") + .isEmpty(); + })).run(); + CuratorFramework curatorFramework = context.getBean("curatorFramework", CuratorFramework.class); + assertThat(curatorFramework).isNotNull(); + assertThatThrownBy(() -> + context.getBean("configDataCuratorFramework", CuratorFramework.class)).isInstanceOf(NoSuchBeanDefinitionException.class); + context.close(); + } + + @Test + public void discoveryClientDisabledReturnsEmptyList() { + ConfigurableApplicationContext context = new SpringApplicationBuilder(TestConfig.class) + .listeners(new ZookeeperTestingServer()) + .properties("--server.port=0", "spring.cloud.config.discovery.enabled=true", + "spring.cloud.discovery.enabled=false", + "spring.cloud.zookeeper.discovery.metadata[mymetadataprop]=mymetadataval", + "spring.cloud.service-registry.auto-registration.enabled=false") + .addBootstrapRegistryInitializer(registry -> registry.addCloseListener(event -> { + ConfigServerInstanceProvider.Function providerFn = event.getBootstrapContext() + .get(ConfigServerInstanceProvider.Function.class); + assertThat(providerFn.apply("id")).as("ConfigServerInstanceProvider.Function should return empty list") + .isEmpty(); + })).run(); + CuratorFramework curatorFramework = context.getBean("curatorFramework", CuratorFramework.class); + assertThat(curatorFramework).isNotNull(); + assertThatThrownBy(() -> + context.getBean("configDataCuratorFramework", CuratorFramework.class)).isInstanceOf(NoSuchBeanDefinitionException.class); + context.close(); + } + @Test public void enabledAddsInstanceProviderFn() { AtomicReference bootstrapDiscoveryClient = new AtomicReference<>(); @@ -82,7 +124,7 @@ public class ZookeeperConfigServerBootstrapperTests { .addBootstrapRegistryInitializer(registry -> registry.addCloseListener(event -> { ConfigServerInstanceProvider.Function providerFn = event.getBootstrapContext() .get(ConfigServerInstanceProvider.Function.class); - assertThat(providerFn).as("ConfigServerInstanceProvider.Function was not created when it should.") + assertThat(providerFn.apply("id")).as("Should return empty list.") .isNotNull(); bootstrapDiscoveryClient.set(event.getBootstrapContext().get(ZookeeperDiscoveryClient.class)); })).run();