fix (#1334)

spring-cloud · May 1, 2023 · 002c3fd · 002c3fd
1 parent 88ebe79
commit 002c3fd
Show file tree

Hide file tree

Showing 4 changed files with 144 additions and 8 deletions.
diff --git a/...a/org/springframework/cloud/kubernetes/commons/config/ConfigMapPropertySourceLocator.java b/...a/org/springframework/cloud/kubernetes/commons/config/ConfigMapPropertySourceLocator.java
@@ -97,6 +97,7 @@ private MapPropertySource getMapPropertySourceForSingleConfigMap(ConfigurableEnv
 
 	private void addPropertySourcesFromPaths(Environment environment, CompositePropertySource composite) {
 		Set<String> uniquePaths = new LinkedHashSet<>(properties.getPaths());
+		LOG.debug("paths property sources : " + uniquePaths);
 		uniquePaths.stream().map(Paths::get).filter(p -> {
 			boolean exists = Files.exists(p);
 			if (!exists) {
@@ -137,7 +138,8 @@ private void addPropertySourceIfNeeded(Function<String, Map<String, Object>> con
 			LOG.warn("Property source: " + name + "will be ignored because no properties could be found");
 		}
 		else {
-			composite.addFirstPropertySource(new MapPropertySource(name, map));
+			LOG.debug("will add file-based property source : " + name);
+			composite.addFirstPropertySource(new MountConfigMapPropertySource(name, map));
 		}
 	}
 

diff --git a/...ava/org/springframework/cloud/kubernetes/commons/config/MountConfigMapPropertySource.java b/...ava/org/springframework/cloud/kubernetes/commons/config/MountConfigMapPropertySource.java
@@ -0,0 +1,29 @@
+/*
+ * Copyright 2013-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.kubernetes.commons.config;
+
+import java.util.Map;
+
+import org.springframework.core.env.MapPropertySource;
+
+public final class MountConfigMapPropertySource extends MapPropertySource {
+
+	public MountConfigMapPropertySource(String name, Map<String, Object> source) {
+		super(name, source);
+	}
+
+}
diff --git a/...g/springframework/cloud/kubernetes/commons/config/reload/ConfigurationChangeDetector.java b/...g/springframework/cloud/kubernetes/commons/config/reload/ConfigurationChangeDetector.java
@@ -28,6 +28,7 @@
 
 import org.springframework.cloud.bootstrap.config.BootstrapPropertySource;
 import org.springframework.cloud.bootstrap.config.PropertySourceLocator;
+import org.springframework.cloud.kubernetes.commons.config.MountConfigMapPropertySource;
 import org.springframework.core.env.CompositePropertySource;
 import org.springframework.core.env.ConfigurableEnvironment;
 import org.springframework.core.env.Environment;
@@ -82,8 +83,6 @@ public boolean changed(MapPropertySource left, MapPropertySource right) {
 
 	public boolean changed(List<? extends MapPropertySource> left, List<? extends MapPropertySource> right) {
 		if (left.size() != right.size()) {
-			this.log.warn("The current number of ConfigMap PropertySources does not match "
-					+ "the ones loaded from the Kubernetes - No reload will take place");
 
 			if (log.isDebugEnabled()) {
 				this.log.debug(String.format("source 1: %d", left.size()));
@@ -92,12 +91,19 @@ public boolean changed(List<? extends MapPropertySource> left, List<? extends Ma
 				this.log.debug(String.format("source 2: %d", right.size()));
 				right.forEach(item -> log.debug(item));
 			}
+			this.log.warn("The current number of ConfigMap PropertySources does not match "
+				+ "the ones loaded from the Kubernetes - No reload will take place");
 			return false;
 		}
 
 		for (int i = 0; i < left.size(); i++) {
 			if (changed(left.get(i), right.get(i))) {
-				return true;
+				MapPropertySource leftPropertySource = left.get(i);
+				MapPropertySource rightPropertySource = right.get(i);
+				if (changed(leftPropertySource, rightPropertySource)) {
+					this.log.debug("found change in : " + leftPropertySource);
+					return true;
+				}
 			}
 		}
 		return false;
@@ -131,8 +137,8 @@ public <S extends PropertySource<?>> List<S> findPropertySources(Class<S> source
 
 		LinkedList<PropertySource<?>> sources = toLinkedList(this.environment.getPropertySources());
 		this.log.debug("findPropertySources");
-		this.log.debug(String.format("environment: %s", this.environment));
-		this.log.debug(String.format("environment sources: %s", sources));
+		this.log.debug(String.format("environment from findPropertySources : %s", this.environment));
+		this.log.debug(String.format("environment sources from findPropertySources : %s", sources));
 
 		while (!sources.isEmpty()) {
 			PropertySource<?> source = sources.pop();
@@ -143,14 +149,24 @@ public <S extends PropertySource<?>> List<S> findPropertySources(Class<S> source
 			else if (sourceClass.isInstance(source)) {
 				managedSources.add(sourceClass.cast(source));
 			}
+			else if (source instanceof MountConfigMapPropertySource) {
+				// we know that the type is correct here
+				managedSources.add((S) source);
+			}
 			else if (source instanceof BootstrapPropertySource) {
 				PropertySource<?> propertySource = ((BootstrapPropertySource<?>) source).getDelegate();
 				if (sourceClass.isInstance(propertySource)) {
 					sources.add(propertySource);
 				}
+				else if (propertySource instanceof MountConfigMapPropertySource) {
+					// we know that the type is correct here
+					managedSources.add((S) propertySource);
+				}
 			}
 		}
 
+		this.log.debug("findPropertySources : " + managedSources.stream().map(PropertySource::getName)
+			.collect(Collectors.toList()));
 		return managedSources;
 	}
 
@@ -188,8 +204,8 @@ else if (propertySource instanceof CompositePropertySource) {
 		}
 
 		this.log.debug("locateMapPropertySources");
-		this.log.debug(String.format("environment: %s", environment));
-		this.log.debug(String.format("sources: %s", result));
+		this.log.debug(String.format("environment from locateMapPropertySources : %s", environment));
+		this.log.debug(String.format("sources from locateMapPropertySources : %s", result));
 
 		return result;
 	}

diff --git a/...ringframework/cloud/kubernetes/fabric8/config/reload/ConfigurationChangeDetectorTest.java b/...ringframework/cloud/kubernetes/fabric8/config/reload/ConfigurationChangeDetectorTest.java
@@ -16,18 +16,27 @@
 
 package org.springframework.cloud.kubernetes.fabric8.config.reload;
 
+import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
+import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
 
+import org.springframework.cloud.bootstrap.config.BootstrapPropertySource;
+import org.springframework.cloud.kubernetes.commons.config.MountConfigMapPropertySource;
 import org.springframework.cloud.kubernetes.commons.config.reload.ConfigReloadProperties;
 import org.springframework.cloud.kubernetes.commons.config.reload.ConfigurationChangeDetector;
 import org.springframework.cloud.kubernetes.commons.config.reload.ConfigurationUpdateStrategy;
+import org.springframework.core.env.CompositePropertySource;
 import org.springframework.core.env.ConfigurableEnvironment;
+import org.springframework.core.env.EnumerablePropertySource;
 import org.springframework.core.env.MapPropertySource;
+import org.springframework.core.env.MutablePropertySources;
+import org.springframework.core.env.PropertySource;
+import org.springframework.mock.env.MockEnvironment;
 
 import static org.assertj.core.api.Assertions.assertThat;
 
@@ -119,6 +128,34 @@ public void testChangedListSameSizesEqual() {
 		assertThat(changed).isTrue();
 	}
 
+	@Test
+	void testFindPropertySources() {
+		MockEnvironment environment = new MockEnvironment();
+		MutablePropertySources propertySources = environment.getPropertySources();
+		propertySources.addFirst(new OneComposite());
+		propertySources.addFirst(new PlainPropertySource("plain"));
+		propertySources.addFirst(new OneBootstrap(new EnumerablePropertySource<String>("enumerable") {
+			@Override
+			public String[] getPropertyNames() {
+				return new String[0];
+			}
+
+			@Override
+			public Object getProperty(String name) {
+				return null;
+			}
+		}));
+		propertySources.addFirst(new MountConfigMapPropertySource("mounted", Collections.singletonMap("a", "b")));
+
+		ConfigurationChangeDetectorStub local = new ConfigurationChangeDetectorStub(environment, null, null);
+		List<? extends PropertySource> result = local.findPropertySources(PlainPropertySource.class);
+		Assertions.assertEquals(4, result.size());
+		Assertions.assertEquals("b", result.get(0).getProperty("a"));
+		Assertions.assertEquals("plain", result.get(1).getProperty(""));
+		Assertions.assertEquals("from-bootstrap", result.get(2).getProperty(""));
+		Assertions.assertEquals("from-inner-two-composite", result.get(3).getProperty(""));
+	}
+
 	/**
 	 * only needed to test some protected methods it defines
 	 */
@@ -131,4 +168,56 @@ private ConfigurationChangeDetectorStub(ConfigurableEnvironment environment, Con
 
 	}
 
+	private static final class OneComposite extends CompositePropertySource {
+
+		private OneComposite() {
+			super("one");
+		}
+
+		@Override
+		public Collection<PropertySource<?>> getPropertySources() {
+			return Collections.singletonList(new TwoComposite());
+		}
+
+	}
+
+	private static final class TwoComposite extends CompositePropertySource {
+
+		private TwoComposite() {
+			super("two");
+		}
+
+		@Override
+		public Collection<PropertySource<?>> getPropertySources() {
+			return Collections.singletonList(new PlainPropertySource("from-inner-two-composite"));
+		}
+
+	}
+
+	private static final class PlainPropertySource extends PropertySource<String> {
+
+		private PlainPropertySource(String name) {
+			super(name);
+		}
+
+		@Override
+		public Object getProperty(String name) {
+			return this.name;
+		}
+
+	}
+
+	private static final class OneBootstrap extends BootstrapPropertySource<String> {
+
+		private OneBootstrap(EnumerablePropertySource<String> delegate) {
+			super(delegate);
+		}
+
+		@Override
+		public PropertySource<String> getDelegate() {
+			return new PlainPropertySource("from-bootstrap");
+		}
+
+	}
+
 }