Merge branch 'main' of https://github.com/spring-cloud/spring-cloud-k…

…ubernetes
wind57 · Feb 2, 2024 · 4e292ec · 4e292ec
2 parents 0043006 + 3720fb0
commit 4e292ec
Show file tree

Hide file tree

Showing 15 changed files with 677 additions and 189 deletions.
diff --git a/docs/modules/ROOT/pages/getting-started.adoc b/docs/modules/ROOT/pages/getting-started.adoc
@@ -8,6 +8,9 @@ provide implementations using the https://github.com/fabric8io/kubernetes-client
 Starters that begin with
 `spring-cloud-starter-kubernetes-client` provide implementations using the https://github.com/kubernetes-client/java[Kubernetes Java Client].
 
+NOTE:  You CANNOT combine starters from Fabric8 and Kubernetes Java Clients.  You must pick one library to
+use and use the starters for that library only.
+
 [cols="a,d"]
 |===
 | Starter | Features

diff --git a/docs/modules/ROOT/pages/property-source-config/configmap-propertysource.adoc b/docs/modules/ROOT/pages/property-source-config/configmap-propertysource.adoc
@@ -83,6 +83,34 @@ This is what we will end-up loading:
  - `not-my-app.yaml` _ignored_, since it does not match `spring.application.name`
  - `someProp: someValue` plain property
 
+The order of loading properties is a as follows:
+
+- first load all properties from `my-app.yaml`
+- then all from profile-based sources: `my-app-k8s.yaml`
+- then all plain properties `someProp: someValue`
+
+This means that profile based sources take precedence over non-profile based sources (just like in a vanilla Spring app); and plain properties take precedence over both profile and non-profile based sources. Here is an example:
+
+====
+[source]
+----
+kind: ConfigMap
+apiVersion: v1
+metadata:
+  name: my-app
+data:
+  my-app-k8s.yaml: |-
+    key1=valueA
+	key2=valueB
+  my-app.yaml: |-
+    key1=valueC
+    key2=valueA
+  key1: valueD
+----
+====
+
+After processing such a ConfigMap, this is what you will get in the properties: `key1=valueD`, `key2=valueB`.
+
 The single exception to the aforementioned flow is when the `ConfigMap` contains a *single* key that indicates
 the file is a YAML or properties file. In that case, the name of the key does NOT have to be `application.yaml` or
 `application.properties` (it can be anything) and the value of the property is treated correctly.

diff --git a/pom.xml b/pom.xml
@@ -70,7 +70,7 @@
 		<failsafe-reports-directory>failsafe-reports</failsafe-reports-directory>
 		<!-- Dependency Versions -->
 		<mockito-inline.version>4.8.1</mockito-inline.version>
-		<spring-cloud-commons.version>4.1.1-SNAPSHOT</spring-cloud-commons.version>
+		<spring-cloud-commons.version>4.1.2-SNAPSHOT</spring-cloud-commons.version>
 		<spring-cloud-config.version>4.1.1-SNAPSHOT</spring-cloud-config.version>
 		<spring-cloud-bus.version>4.1.1-SNAPSHOT</spring-cloud-bus.version>
 		<spring-cloud-contract.version>4.1.1-SNAPSHOT</spring-cloud-contract.version>

diff --git a/scripts/integration-tests.sh b/scripts/integration-tests.sh
@@ -1,5 +1,10 @@
 #!/usr/bin/env bash
 set -e
 
+rm ~/.testcontainers.properties
+echo 'testcontainers.reuse.enable=true' > ~/.testcontainers.properties
+
 ./mvnw clean install -B -Pdocs ${@}
 
+rm ~/.testcontainers.properties
+docker kill $(docker ps -q)
diff --git a/...framework/cloud/kubernetes/client/discovery/KubernetesClientConfigServerBootstrapper.java b/...framework/cloud/kubernetes/client/discovery/KubernetesClientConfigServerBootstrapper.java
@@ -17,7 +17,6 @@
 package org.springframework.cloud.kubernetes.client.discovery;
 
 import java.util.Collections;
-import java.util.List;
 
 import io.kubernetes.client.informer.SharedIndexInformer;
 import io.kubernetes.client.informer.SharedInformerFactory;
@@ -30,19 +29,16 @@
 import io.kubernetes.client.util.Namespaces;
 import io.kubernetes.client.util.generic.GenericKubernetesApi;
 import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
 
-import org.springframework.boot.BootstrapContext;
 import org.springframework.boot.BootstrapRegistry;
-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.config.client.ConfigServerConfigDataLocationResolver;
+import org.springframework.cloud.config.client.ConfigServerConfigDataLocationResolver.PropertyResolver;
 import org.springframework.cloud.config.client.ConfigServerInstanceProvider;
 import org.springframework.cloud.kubernetes.client.KubernetesClientAutoConfiguration;
 import org.springframework.cloud.kubernetes.commons.KubernetesClientProperties;
 import org.springframework.cloud.kubernetes.commons.KubernetesNamespaceProvider;
 import org.springframework.cloud.kubernetes.commons.config.KubernetesConfigServerBootstrapper;
-import org.springframework.cloud.kubernetes.commons.config.KubernetesConfigServerInstanceProvider;
 import org.springframework.cloud.kubernetes.commons.discovery.KubernetesDiscoveryProperties;
 import org.springframework.core.env.AbstractEnvironment;
 import org.springframework.core.env.Environment;
@@ -54,65 +50,45 @@
  */
 class KubernetesClientConfigServerBootstrapper extends KubernetesConfigServerBootstrapper {
 
+	private static final Log LOG = LogFactory.getLog(KubernetesClientConfigServerBootstrapper.class);
+
 	@Override
 	public void initialize(BootstrapRegistry registry) {
 		if (hasConfigServerInstanceProvider()) {
 			return;
 		}
-		// We need to pass a lambda here rather than create a new instance of
-		// ConfigServerInstanceProvider.Function
-		// or else we will get ClassNotFoundExceptions if Spring Cloud Config is not on
-		// the classpath
-		registry.registerIfAbsent(ConfigServerInstanceProvider.Function.class, KubernetesFunction::create);
-	}
-
-	final static class KubernetesFunction implements ConfigServerInstanceProvider.Function {
-
-		private final BootstrapContext context;
-
-		private KubernetesFunction(BootstrapContext context) {
-			this.context = context;
-		}
-
-		static KubernetesFunction create(BootstrapContext context) {
-			return new KubernetesFunction(context);
-		}
-
-		@Override
-		public List<ServiceInstance> apply(String serviceId, Binder binder, BindHandler bindHandler, Log log) {
-			if (binder == null || bindHandler == null || !getDiscoveryEnabled(binder, bindHandler)) {
-				// If we don't have the Binder or BinderHandler from the
-				// ConfigDataLocationResolverContext
-				// we won't be able to create the necessary configuration
-				// properties to configure the
-				// Kubernetes DiscoveryClient
-				return Collections.emptyList();
+		registry.registerIfAbsent(KubernetesDiscoveryProperties.class, context -> {
+			if (!getDiscoveryEnabled(context)) {
+				return null;
 			}
-			KubernetesDiscoveryProperties discoveryProperties = createKubernetesDiscoveryProperties(binder,
-					bindHandler);
-			KubernetesClientProperties clientProperties = createKubernetesClientProperties(binder, bindHandler);
-			return getInstanceProvider(discoveryProperties, clientProperties, context, binder, bindHandler, log)
-					.getInstances(serviceId);
-		}
+			return createKubernetesDiscoveryProperties(context);
+		});
 
-		private KubernetesConfigServerInstanceProvider getInstanceProvider(
-				KubernetesDiscoveryProperties discoveryProperties, KubernetesClientProperties clientProperties,
-				BootstrapContext context, Binder binder, BindHandler bindHandler, Log log) {
+		registry.registerIfAbsent(KubernetesClientProperties.class, context -> {
+			if (!getDiscoveryEnabled(context)) {
+				return null;
+			}
+			return createKubernetesClientProperties(context);
+		});
+		registry.registerIfAbsent(ConfigServerInstanceProvider.Function.class, context -> {
+			if (!getDiscoveryEnabled(context)) {
+				return (id) -> Collections.emptyList();
+			}
 			if (context.isRegistered(KubernetesInformerDiscoveryClient.class)) {
 				KubernetesInformerDiscoveryClient client = context.get(KubernetesInformerDiscoveryClient.class);
 				return client::getInstances;
 			}
 			else {
-
+				PropertyResolver propertyResolver = getPropertyResolver(context);
 				ApiClient defaultApiClient = kubernetesApiClient();
-				defaultApiClient.setUserAgent(binder.bind("spring.cloud.kubernetes.client.user-agent", String.class)
-						.orElse(KubernetesClientProperties.DEFAULT_USER_AGENT));
+				defaultApiClient.setUserAgent(propertyResolver.get("spring.cloud.kubernetes.client.user-agent",
+						String.class, KubernetesClientProperties.DEFAULT_USER_AGENT));
 				KubernetesClientAutoConfiguration clientAutoConfiguration = new KubernetesClientAutoConfiguration();
 				ApiClient apiClient = context.getOrElseSupply(ApiClient.class, () -> defaultApiClient);
 
 				KubernetesNamespaceProvider kubernetesNamespaceProvider = clientAutoConfiguration
-						.kubernetesNamespaceProvider(getNamespaceEnvironment(binder, bindHandler));
-
+						.kubernetesNamespaceProvider(getNamespaceEnvironment(propertyResolver));
+				KubernetesDiscoveryProperties discoveryProperties = context.get(KubernetesDiscoveryProperties.class);
 				String namespace = getInformerNamespace(kubernetesNamespaceProvider, discoveryProperties);
 				SharedInformerFactory sharedInformerFactory = new SharedInformerFactory(apiClient);
 				GenericKubernetesApi<V1Service, V1ServiceList> servicesApi = new GenericKubernetesApi<>(V1Service.class,
@@ -133,40 +109,32 @@ private KubernetesConfigServerInstanceProvider getInstanceProvider(
 					return discoveryClient::getInstances;
 				}
 				catch (Exception e) {
-					if (log != null) {
-						log.warn("Error initiating informer discovery client", e);
-					}
+					LOG.warn("Error initiating informer discovery client", e);
 					return (serviceId) -> Collections.emptyList();
 				}
 				finally {
 					sharedInformerFactory.stopAllRegisteredInformers();
 				}
 			}
-		}
+		});
 
-		private String getInformerNamespace(KubernetesNamespaceProvider kubernetesNamespaceProvider,
-				KubernetesDiscoveryProperties discoveryProperties) {
-			return discoveryProperties.allNamespaces() ? Namespaces.NAMESPACE_ALL
-					: kubernetesNamespaceProvider.getNamespace() == null ? Namespaces.NAMESPACE_DEFAULT
-							: kubernetesNamespaceProvider.getNamespace();
-		}
-
-		private Environment getNamespaceEnvironment(Binder binder, BindHandler bindHandler) {
-			return new AbstractEnvironment() {
-				@Override
-				public String getProperty(String key) {
-					return binder.bind(key, Bindable.of(String.class), bindHandler).orElse(super.getProperty(key));
-				}
-			};
-		}
+	}
 
-		// This method should never be called, but is there for backward
-		// compatibility purposes
-		@Override
-		public List<ServiceInstance> apply(String serviceId) {
-			return apply(serviceId, null, null, null);
-		}
+	private String getInformerNamespace(KubernetesNamespaceProvider kubernetesNamespaceProvider,
+			KubernetesDiscoveryProperties discoveryProperties) {
+		return discoveryProperties.allNamespaces() ? Namespaces.NAMESPACE_ALL
+				: kubernetesNamespaceProvider.getNamespace() == null ? Namespaces.NAMESPACE_DEFAULT
+						: kubernetesNamespaceProvider.getNamespace();
+	}
 
+	private Environment getNamespaceEnvironment(
+			ConfigServerConfigDataLocationResolver.PropertyResolver propertyResolver) {
+		return new AbstractEnvironment() {
+			@Override
+			public String getProperty(String key) {
+				return propertyResolver.get(key, String.class, super.getProperty(key));
+			}
+		};
 	}
 
 }
diff --git a/...ent/discovery/catalog/KubernetesCatalogWatchAutoConfigurationApplicationContextTests.java b/...ent/discovery/catalog/KubernetesCatalogWatchAutoConfigurationApplicationContextTests.java
@@ -98,7 +98,7 @@ void disableBlockingAndReactive() {
 	@Test
 	void disableBlockingEnableReactive() {
 		setup("spring.main.cloud-platform=KUBERNETES", "spring.cloud.config.enabled=false",
-			"spring.cloud.discovery.blocking.enabled=false", "spring.cloud.discovery.reactive.enabled=true");
+				"spring.cloud.discovery.blocking.enabled=false", "spring.cloud.discovery.reactive.enabled=true");
 		applicationContextRunner.run(context -> {
 			assertThat(context).hasSingleBean(KubernetesCatalogWatch.class);
 		});
@@ -110,14 +110,15 @@ void disableBlockingEnableReactive() {
 	@Test
 	void enableBlockingDisableReactive() {
 		setup("spring.main.cloud-platform=KUBERNETES", "spring.cloud.config.enabled=false",
-			"spring.cloud.discovery.blocking.enabled=true", "spring.cloud.discovery.reactive.enabled=false");
+				"spring.cloud.discovery.blocking.enabled=true", "spring.cloud.discovery.reactive.enabled=false");
 		applicationContextRunner.run(context -> {
 			assertThat(context).hasSingleBean(KubernetesCatalogWatch.class);
 		});
 	}
 
 	/**
-	 * spring.cloud.kubernetes.discovery.enabled is false, catalog watcher is disabled also.
+	 * spring.cloud.kubernetes.discovery.enabled is false, catalog watcher is disabled
+	 * also.
 	 */
 	@Test
 	void disableKubernetesDiscovery() {

diff --git a/...c/main/java/org/springframework/cloud/kubernetes/commons/KubernetesNamespaceProvider.java b/...c/main/java/org/springframework/cloud/kubernetes/commons/KubernetesNamespaceProvider.java
@@ -35,8 +35,6 @@
  */
 public class KubernetesNamespaceProvider {
 
-	private static final DeferredLog LOG = new DeferredLog();
-
 	/**
 	 * Property name for namespace.
 	 */
@@ -47,6 +45,10 @@ public class KubernetesNamespaceProvider {
 	 */
 	public static final String NAMESPACE_PATH_PROPERTY = "spring.cloud.kubernetes.client.serviceAccountNamespacePath";
 
+	private static final DeferredLog LOG = new DeferredLog();
+
+	private String namespacePropertyValue;
+
 	private BindHandler bindHandler;
 
 	private String serviceAccountNamespace;
@@ -65,7 +67,36 @@ public KubernetesNamespaceProvider(Binder binder, BindHandler bindHandler) {
 		this.bindHandler = bindHandler;
 	}
 
+	public KubernetesNamespaceProvider(String namespacePropertyValue) {
+		this.namespacePropertyValue = namespacePropertyValue;
+	}
+
+	public static String getNamespaceFromServiceAccountFile(String path) {
+		String namespace = null;
+		LOG.debug("Looking for service account namespace at: [" + path + "].");
+		Path serviceAccountNamespacePath = Paths.get(path);
+		boolean serviceAccountNamespaceExists = Files.isRegularFile(serviceAccountNamespacePath);
+		if (serviceAccountNamespaceExists) {
+			LOG.debug("Found service account namespace at: [" + serviceAccountNamespacePath + "].");
+
+			try {
+				namespace = new String(Files.readAllBytes((serviceAccountNamespacePath)));
+				LOG.debug("Service account namespace value: " + serviceAccountNamespacePath);
+			}
+			catch (IOException ioe) {
+				LOG.error("Error reading service account namespace from: [" + serviceAccountNamespacePath + "].", ioe);
+			}
+
+		}
+		return namespace;
+	}
+
 	public String getNamespace() {
+		// If they provided the namespace in the constructor just return that
+		if (!ObjectUtils.isEmpty(namespacePropertyValue)) {
+			return namespacePropertyValue;
+		}
+		// No namespace provided so try to get it from another source
 		String namespace = null;
 		if (environment != null) {
 			namespace = environment.getProperty(NAMESPACE_PROPERTY);
@@ -96,24 +127,4 @@ private String getServiceAccountNamespace() {
 		return serviceAccountNamespace;
 	}
 
-	public static String getNamespaceFromServiceAccountFile(String path) {
-		String namespace = null;
-		LOG.debug("Looking for service account namespace at: [" + path + "].");
-		Path serviceAccountNamespacePath = Paths.get(path);
-		boolean serviceAccountNamespaceExists = Files.isRegularFile(serviceAccountNamespacePath);
-		if (serviceAccountNamespaceExists) {
-			LOG.debug("Found service account namespace at: [" + serviceAccountNamespacePath + "].");
-
-			try {
-				namespace = new String(Files.readAllBytes((serviceAccountNamespacePath)));
-				LOG.debug("Service account namespace value: " + serviceAccountNamespacePath);
-			}
-			catch (IOException ioe) {
-				LOG.error("Error reading service account namespace from: [" + serviceAccountNamespacePath + "].", ioe);
-			}
-
-		}
-		return namespace;
-	}
-
 }