Implement new API in ConfigServerInstanceProvider.Function

pom.xml

@@ -27,6 +27,7 @@
 		<spring-cloud-openfeign.version>3.1.8-SNAPSHOT</spring-cloud-openfeign.version>
 		<spring-cloud-stream.version>3.2.9-SNAPSHOT</spring-cloud-stream.version>
 		<testcontainers.version>1.17.6</testcontainers.version>
+		<mockserverclient.version>5.15.0</mockserverclient.version>
 	</properties>
 
 	<scm>
@@ -156,11 +157,21 @@
 				<artifactId>consul</artifactId>
 				<version>${testcontainers.version}</version>
 			</dependency>
+			<dependency>
+				<groupId>org.testcontainers</groupId>
+				<artifactId>mockserver</artifactId>
+				<version>${testcontainers.version}</version>
+			</dependency>
 			<dependency>
 				<groupId>org.testcontainers</groupId>
 				<artifactId>junit-jupiter</artifactId>
 				<version>${testcontainers.version}</version>
 			</dependency>
+			<dependency>
+				<groupId>org.mock-server</groupId>
+				<artifactId>mockserver-client-java</artifactId>
+				<version>${mockserverclient.version}</version>
+			</dependency>
 		</dependencies>
 	</dependencyManagement>
 

spring-cloud-consul-discovery/pom.xml

@@ -134,6 +134,16 @@
 			<artifactId>consul</artifactId>
 			<scope>test</scope>
 		</dependency>
+		<dependency>
+			<groupId>org.testcontainers</groupId>
+			<artifactId>mockserver</artifactId>
+			<scope>test</scope>
+		</dependency>
+		<dependency>
+			<groupId>org.mock-server</groupId>
+			<artifactId>mockserver-client-java</artifactId>
+			<scope>test</scope>
+		</dependency>
 		<dependency>
 			<groupId>org.testcontainers</groupId>
 			<artifactId>junit-jupiter</artifactId>

spring-cloud-consul-discovery/src/main/java/org/springframework/cloud/consul/discovery/configclient/ConsulConfigServerBootstrapper.java

@@ -17,14 +17,18 @@
 package org.springframework.cloud.consul.discovery.configclient;
 
 import java.util.Collections;
+import java.util.List;
 
 import com.ecwid.consul.v1.ConsulClient;
+import org.apache.commons.logging.Log;
 
+import org.springframework.boot.BootstrapContext;
 import org.springframework.boot.BootstrapRegistry;
 import org.springframework.boot.BootstrapRegistryInitializer;
 import org.springframework.boot.context.properties.bind.BindHandler;
 import org.springframework.boot.context.properties.bind.Bindable;
 import org.springframework.boot.context.properties.bind.Binder;
+import org.springframework.cloud.client.ServiceInstance;
 import org.springframework.cloud.commons.util.InetUtils;
 import org.springframework.cloud.commons.util.InetUtilsProperties;
 import org.springframework.cloud.config.client.ConfigClientProperties;
@@ -84,24 +88,67 @@ public void initialize(BootstrapRegistry registry) {
 						discoveryClient);
 			}
 		});
-		registry.registerIfAbsent(ConfigServerInstanceProvider.Function.class, context -> {
-			if (!isDiscoveryEnabled(context.get(Binder.class))) {
-				return (id) -> Collections.emptyList();
-			}
-			ConsulDiscoveryClient discoveryClient = context.get(ConsulDiscoveryClient.class);
-			return discoveryClient::getInstances;
-		});
 
+		// We need to pass the lambda here so we do not create a new instance of
+		// ConfigServerInstanceProvider.Function
+		// which would result in a ClassNotFoundException when Spring Cloud Config is not
+		// on the classpath
+		registry.registerIfAbsent(ConfigServerInstanceProvider.Function.class, ConsulFunction::new);
 	}
 
 	private BindHandler getBindHandler(org.springframework.boot.BootstrapContext context) {
 		return context.getOrElse(BindHandler.class, null);
 	}
 
-	private boolean isDiscoveryEnabled(Binder binder) {
+	private static boolean isDiscoveryEnabled(Binder binder) {
 		return binder.bind(ConfigClientProperties.CONFIG_DISCOVERY_ENABLED, Boolean.class).orElse(false)
 				&& binder.bind(ConditionalOnConsulDiscoveryEnabled.PROPERTY, Boolean.class).orElse(true)
 				&& binder.bind("spring.cloud.discovery.enabled", Boolean.class).orElse(true);
 	}
 
+	/*
+	 * This Function is executed when loading config data. Because of this we cannot rely
+	 * on the BootstrapContext because Boot has not finished loading all the configuration
+	 * data so if we ask the BootstrapContext for configuration data it will not have it.
+	 * The apply method in this function is passed the Binder and BindHandler from the
+	 * config data context which has the configuration properties that have been loaded so
+	 * far in the config data process.
+	 *
+	 * We will create many of the same beans in this function as we do above in the
+	 * initializer above. We do both to maintain compatibility since we are promoting
+	 * those beans to the main application context.
+	 */
+	static final class ConsulFunction implements ConfigServerInstanceProvider.Function {
+
+		private final BootstrapContext context;
+
+		private ConsulFunction(BootstrapContext context) {
+			this.context = context;
+		}
+
+		@Override
+		public List<ServiceInstance> apply(String serviceId) {
+			return apply(serviceId, null, null, null);
+		}
+
+		@Override
+		public List<ServiceInstance> apply(String serviceId, Binder binder, BindHandler bindHandler, Log log) {
+			if (binder == null || !isDiscoveryEnabled(binder)) {
+				return Collections.emptyList();
+			}
+
+			ConsulProperties consulProperties = binder
+					.bind(ConsulProperties.PREFIX, Bindable.of(ConsulProperties.class), bindHandler)
+					.orElseGet(ConsulProperties::new);
+			ConsulClient consulClient = ConsulAutoConfiguration.createConsulClient(consulProperties);
+			ConsulDiscoveryProperties properties = binder
+					.bind(ConsulDiscoveryProperties.PREFIX, Bindable.of(ConsulDiscoveryProperties.class), bindHandler)
+					.orElseGet(() -> new ConsulDiscoveryProperties(new InetUtils(new InetUtilsProperties())));
+			ConsulDiscoveryClient discoveryClient = new ConsulDiscoveryClient(consulClient, properties);
+
+			return discoveryClient.getInstances(serviceId);
+		}
+
+	}
+
 }

spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/configclient/ConsulConfigServerBootstrapperIT.java

@@ -0,0 +1,135 @@
+/*
+ * Copyright 2012-2023 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.consul.discovery.configclient;
+
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.LinkedHashSet;
+import java.util.Map;
+import java.util.Set;
+
+import com.ecwid.consul.v1.ConsulClient;
+import com.ecwid.consul.v1.agent.model.NewService;
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.mockserver.client.MockServerClient;
+import org.testcontainers.consul.ConsulContainer;
+import org.testcontainers.containers.MockServerContainer;
+import org.testcontainers.junit.jupiter.Container;
+import org.testcontainers.junit.jupiter.Testcontainers;
+import org.testcontainers.utility.DockerImageName;
+
+import org.springframework.boot.SpringBootConfiguration;
+import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
+import org.springframework.boot.builder.SpringApplicationBuilder;
+import org.springframework.cloud.config.environment.Environment;
+import org.springframework.cloud.config.environment.PropertySource;
+import org.springframework.cloud.consul.ConsulAutoConfiguration;
+import org.springframework.cloud.consul.ConsulProperties;
+import org.springframework.cloud.consul.test.ConsulTestcontainers;
+import org.springframework.context.ConfigurableApplicationContext;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockserver.model.HttpRequest.request;
+import static org.mockserver.model.HttpResponse.response;
+
+/**
+ * @author Ryan Baxter
+ */
+
+@Testcontainers
+public class ConsulConfigServerBootstrapperIT {
+
+	public static final DockerImageName MOCKSERVER_IMAGE = DockerImageName.parse("mockserver/mockserver")
+			.withTag("mockserver-" + MockServerClient.class.getPackage().getImplementationVersion());
+
+	@Container
+	static ConsulContainer consul = ConsulTestcontainers.createConsulContainer("1.10");
+
+	@Container
+	static MockServerContainer mockServer = new MockServerContainer(MOCKSERVER_IMAGE);
+
+	private ConfigurableApplicationContext context;
+
+	@BeforeEach
+	void before() {
+		ConsulProperties consulProperties = new ConsulProperties();
+		consulProperties.setHost(consul.getHost());
+		consulProperties.setPort(consul.getMappedPort(ConsulTestcontainers.DEFAULT_PORT));
+		ConsulClient client = ConsulAutoConfiguration.createConsulClient(consulProperties);
+		NewService newService = new NewService();
+		newService.setId("consul-configserver");
+		newService.setName("consul-configserver");
+		newService.setAddress(mockServer.getHost());
+		newService.setPort(mockServer.getServerPort());
+		client.agentServiceRegister(newService);
+
+	}
+
+	@AfterEach
+	void after() {
+		this.context.close();
+	}
+
+	@Test
+	public void contextLoads() throws JsonProcessingException {
+		Environment environment = new Environment("test", "default");
+		Map<String, Object> properties = new HashMap<>();
+		properties.put("hello", "world");
+		PropertySource p = new PropertySource("p1", properties);
+		environment.add(p);
+		ObjectMapper objectMapper = new ObjectMapper();
+		try (MockServerClient mockServerClient = new MockServerClient(mockServer.getHost(),
+				mockServer.getMappedPort(MockServerContainer.PORT))) {
+			mockServerClient.when(request().withPath("/application/default"))
+					.respond(response().withBody(objectMapper.writeValueAsString(environment))
+							.withHeader("content-type", "application/json"));
+			this.context = setup().run();
+			assertThat(this.context.getEnvironment().getProperty("hello")).isEqualTo("world");
+		}
+
+	}
+
+	SpringApplicationBuilder setup(String... env) {
+		SpringApplicationBuilder builder = new SpringApplicationBuilder(TestConfig.class)
+				.properties(addDefaultEnv(env));
+		return builder;
+	}
+
+	private String[] addDefaultEnv(String[] env) {
+		Set<String> set = new LinkedHashSet<>();
+		if (env != null && env.length > 0) {
+			set.addAll(Arrays.asList(env));
+		}
+		set.add("spring.config.import=classpath:bootstrapper.yaml");
+		set.add("spring.cloud.config.enabled=true");
+		set.add("spring.cloud.service-registry.auto-registration.enabled=false");
+		set.add(ConsulProperties.PREFIX + ".host=" + consul.getHost());
+		set.add(ConsulProperties.PREFIX + ".port=" + consul.getMappedPort(ConsulTestcontainers.DEFAULT_PORT));
+		return set.toArray(new String[0]);
+	}
+
+	@SpringBootConfiguration
+	@EnableAutoConfiguration
+	static class TestConfig {
+
+	}
+
+}

spring-cloud-consul-discovery/src/test/java/org/springframework/cloud/consul/discovery/configclient/ConsulConfigServerBootstrapperTests.java

@@ -20,6 +20,7 @@
 import java.util.concurrent.atomic.AtomicReference;
 
 import com.ecwid.consul.transport.TransportException;
+import org.apache.commons.logging.Log;
 import org.junit.jupiter.api.Test;
 
 import org.springframework.boot.BootstrapRegistry;
@@ -30,13 +31,15 @@
 import org.springframework.boot.context.properties.bind.BindContext;
 import org.springframework.boot.context.properties.bind.BindHandler;
 import org.springframework.boot.context.properties.bind.Bindable;
+import org.springframework.boot.context.properties.bind.Binder;
 import org.springframework.boot.context.properties.source.ConfigurationPropertyName;
 import org.springframework.cloud.config.client.ConfigServerInstanceProvider;
 import org.springframework.cloud.consul.discovery.ConsulDiscoveryClient;
 import org.springframework.context.ConfigurableApplicationContext;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.mockito.Mockito.mock;
 
 public class ConsulConfigServerBootstrapperTests {
 
@@ -47,37 +50,41 @@ public void notEnabledDoesNotAddInstanceProviderFn() {
 				.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")
-							.isEqualTo(Collections.EMPTY_LIST);
+					Log log = mock(Log.class);
+					assertThat(providerFn.apply("id", event.getBootstrapContext().get(Binder.class),
+							event.getBootstrapContext().get(BindHandler.class), log))
+									.as("ConfigServerInstanceProvider.Function should return empty list")
+									.isEqualTo(Collections.EMPTY_LIST);
 				})).run().close();
 	}
 
 	@Test
 	public void consulDiscoveryClientNotEnabledProvidesEmptyList() {
 		new SpringApplicationBuilder(TestConfig.class)
-				.properties("--server.port=0", "spring.cloud.service-registry.auto-registration.enabled=false",
-						"spring.cloud.config.discovery.enabled=true", "spring.cloud.consul.discovery.enabled=false")
+				.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.apply("id"))
-							.as("ConfigServerInstanceProvider.Function should return empty list")
-							.isEqualTo(Collections.EMPTY_LIST);
+					Log log = mock(Log.class);
+					assertThat(providerFn.apply("id", event.getBootstrapContext().get(Binder.class),
+							event.getBootstrapContext().get(BindHandler.class), log))
+									.as("ConfigServerInstanceProvider.Function should return empty list")
+									.isEqualTo(Collections.EMPTY_LIST);
 				})).run().close();
 	}
 
 	@Test
 	public void springCloudDiscoveryClientNotEnabledProvidesEmptyList() {
 		new SpringApplicationBuilder(TestConfig.class)
-				.properties("--server.port=0", "spring.cloud.service-registry.auto-registration.enabled=false",
-						"spring.cloud.config.discovery.enabled=true", "spring.cloud.discovery.enabled=false")
+				.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.apply("id"))
-							.as("ConfigServerInstanceProvider.Function should return empty list")
-							.isEqualTo(Collections.EMPTY_LIST);
+					Log log = mock(Log.class);
+					assertThat(providerFn.apply("id", event.getBootstrapContext().get(Binder.class),
+							event.getBootstrapContext().get(BindHandler.class), log))
+									.as("ConfigServerInstanceProvider.Function should return empty list")
+									.isEqualTo(Collections.EMPTY_LIST);
 				})).run().close();
 	}
 
@@ -88,17 +95,19 @@ public void enabledAddsInstanceProviderFn() {
 		ConfigurableApplicationContext context = new SpringApplicationBuilder(TestConfig.class)
 				.properties("--server.port=0", "spring.cloud.config.discovery.enabled=true",
 						"spring.cloud.consul.discovery.hostname=myhost",
-						"spring.cloud.service-registry.auto-registration.enabled=false",
-						"spring.cloud.consul.host=localhost")
+						"spring.cloud.service-registry.auto-registration.enabled=false")
 				.addBootstrapRegistryInitializer(bindHandlerBootstrapper)
 				.addBootstrapRegistryInitializer(registry -> registry.addCloseListener(event -> {
 					bootstrapDiscoveryClient.set(event.getBootstrapContext().get(ConsulDiscoveryClient.class));
 					ConfigServerInstanceProvider.Function providerFn = event.getBootstrapContext()
 							.get(ConfigServerInstanceProvider.Function.class);
-					assertThatThrownBy(() -> providerFn.apply("id")).isInstanceOf(TransportException.class)
-							.hasMessageContaining(
-									"org.apache.http.conn.HttpHostConnectException: Connect to localhost:8500")
-							.as("Should have tried to reach out to Consul to get config server instance").isNotNull();
+					assertThatThrownBy(() -> providerFn.apply("id", event.getBootstrapContext().get(Binder.class),
+							event.getBootstrapContext().get(BindHandler.class), mock(Log.class)))
+									.isInstanceOf(TransportException.class)
+									.hasMessageContaining(
+											"org.apache.http.conn.HttpHostConnectException: Connect to localhost:8500")
+									.as("Should have tried to reach out to Consul to get config server instance")
+									.isNotNull();
 				})).run();
 		ConsulDiscoveryClient discoveryClient = context.getBean(ConsulDiscoveryClient.class);
 		assertThat(discoveryClient == bootstrapDiscoveryClient.get()).isTrue();

spring-cloud-consul-discovery/src/test/resources/bootstrapper.yaml

@@ -0,0 +1,8 @@
+spring:
+  config:
+    import: "optional:configserver:"
+  cloud:
+    config:
+      discovery:
+        service-id: consul-configserver
+        enabled: true