From 5aaeaef56fc705a1a39f9b712a27b693a941e345 Mon Sep 17 00:00:00 2001 From: erabii Date: Wed, 18 Aug 2021 07:29:46 -0400 Subject: [PATCH] fix duplicate properties coming from different config maps (#845) * dirty * some work started + tests * checkstyle fixes * add proper java docs * fabric8 changes + tests * merge 2.0.x + fix checkstyle * k8s-client changes + fabric8 trying to fix tests * minor update * added documentation * small change while writing comments to the PR * add @Deprecated * trigger build --- .../main/asciidoc/property-source-config.adoc | 129 ++++++++++ ...bernetesClientConfigMapPropertySource.java | 13 +- ...sClientConfigMapPropertySourceLocator.java | 2 +- ...tesClientConfigMapPropertySourceTests.java | 22 +- .../config/AbstractConfigProperties.java | 10 + .../config/ConfigMapConfigProperties.java | 91 +++++++- .../ConfigMapConfigPropertiesTests.java | 221 ++++++++++++++++++ .../Fabric8ConfigMapPropertySource.java | 21 +- ...Fabric8ConfigMapPropertySourceLocator.java | 3 +- .../config/ConfigMapWithPrefixTests.java | 128 ++++++++++ .../config/with_prefix/WithPrefixApp.java | 34 +++ .../with_prefix/controller/Controller.java | 55 +++++ .../config/with_prefix/properties/One.java | 34 +++ .../config/with_prefix/properties/Three.java | 34 +++ .../config/with_prefix/properties/Two.java | 34 +++ .../test/resources/same-key-with-prefix.yaml | 14 ++ 16 files changed, 830 insertions(+), 15 deletions(-) create mode 100644 spring-cloud-kubernetes-commons/src/test/java/org/springframework/cloud/kubernetes/commons/config/ConfigMapConfigPropertiesTests.java create mode 100644 spring-cloud-kubernetes-fabric8-config/src/test/java/org/springframework/cloud/kubernetes/fabric8/config/ConfigMapWithPrefixTests.java create mode 100644 spring-cloud-kubernetes-fabric8-config/src/test/java/org/springframework/cloud/kubernetes/fabric8/config/with_prefix/WithPrefixApp.java create mode 100644 spring-cloud-kubernetes-fabric8-config/src/test/java/org/springframework/cloud/kubernetes/fabric8/config/with_prefix/controller/Controller.java create mode 100644 spring-cloud-kubernetes-fabric8-config/src/test/java/org/springframework/cloud/kubernetes/fabric8/config/with_prefix/properties/One.java create mode 100644 spring-cloud-kubernetes-fabric8-config/src/test/java/org/springframework/cloud/kubernetes/fabric8/config/with_prefix/properties/Three.java create mode 100644 spring-cloud-kubernetes-fabric8-config/src/test/java/org/springframework/cloud/kubernetes/fabric8/config/with_prefix/properties/Two.java create mode 100644 spring-cloud-kubernetes-fabric8-config/src/test/resources/same-key-with-prefix.yaml diff --git a/docs/src/main/asciidoc/property-source-config.adoc b/docs/src/main/asciidoc/property-source-config.adoc index 3c1fb7f0..1ccff78c 100644 --- a/docs/src/main/asciidoc/property-source-config.adoc +++ b/docs/src/main/asciidoc/property-source-config.adoc @@ -268,6 +268,135 @@ spec: ---- ==== +You could run into a situation where there are multiple configs maps that have the same property names. For example: + +==== +[source,yaml] +---- +kind: ConfigMap +apiVersion: v1 +metadata: + name: config-map-one +data: + application.yml: |- + greeting: + message: Say Hello from one +---- +==== + +and + +==== +[source,yaml] +---- +kind: ConfigMap +apiVersion: v1 +metadata: + name: config-map-two +data: + application.yml: |- + greeting: + message: Say Hello from two +---- +==== + +Depending on the order in which you place these in `bootstrap.yaml|properties`, you might end up with an un-expected result (the last config map wins). For example: + +==== +[source,yaml] +---- +spring: + application: + name: cloud-k8s-app + cloud: + kubernetes: + config: + namespace: default-namespace + sources: + - name: config-map-two + - name: config-map-one +---- +==== + +will result in property `greetings.message` being `Say Hello from one`. + +There is a way to change this default configuration by specifying `useNameAsPrefix`. For example: + +==== +[source,yaml] +---- +spring: + application: + name: with-prefix + cloud: + kubernetes: + config: + useNameAsPrefix: true + namespace: default-namespace + sources: + - name: config-map-one + useNameAsPrefix: false + - name: config-map-two +---- +==== + +Such a configuration will result in two properties being generated: + + - `greetings.message` equal to `Say Hello from one`. + + - `config-map-two.greetings.message` equal to `Say Hello from two` + +Notice that `spring.cloud.kubernetes.config.useNameAsPrefix` has a _lower_ priority than `spring.cloud.kubernetes.config.sources.useNameAsPrefix`. +This allows you to set a "default" strategy for all sources, at the same time allowing to override only a few. + +If using the config map name is not an option, you can specify a different strategy, called : `explicitPrefix`. Since this is an _explicit_ prefix that +you select, it can only be supplied to the `sources` level. At the same time it has a higher priority than `useNameASPrefix`. Let's suppose we have a third config map with these entries: + + +==== +[source,yaml] +---- +kind: ConfigMap +apiVersion: v1 +metadata: + name: config-map-three +data: + application.yml: |- + greeting: + message: Say Hello from three +---- +==== + +A configuration like the one below: + +==== +[source,yaml] +---- +spring: + application: + name: with-prefix + cloud: + kubernetes: + config: + useNameAsPrefix: true + namespace: default-namespace + sources: + - name: config-map-one + useNameAsPrefix: false + - name: config-map-two + explicitPrefix: two + - name: config-map-three +---- +==== + +will result in three properties being generated: + + - `greetings.message` equal to `Say Hello from one`. + + - `two.greetings.message` equal to `Say Hello from two`. + + - `config-map-three.greetings.message` equal to `Say Hello from three`. + NOTE: You should check the security configuration section. To access config maps from inside a pod you need to have the correct Kubernetes service accounts, roles and role bindings. diff --git a/spring-cloud-kubernetes-client-config/src/main/java/org/springframework/cloud/kubernetes/client/config/KubernetesClientConfigMapPropertySource.java b/spring-cloud-kubernetes-client-config/src/main/java/org/springframework/cloud/kubernetes/client/config/KubernetesClientConfigMapPropertySource.java index 3a062047..0ec59fd4 100644 --- a/spring-cloud-kubernetes-client-config/src/main/java/org/springframework/cloud/kubernetes/client/config/KubernetesClientConfigMapPropertySource.java +++ b/spring-cloud-kubernetes-client-config/src/main/java/org/springframework/cloud/kubernetes/client/config/KubernetesClientConfigMapPropertySource.java @@ -30,6 +30,7 @@ import org.apache.commons.logging.LogFactory; import org.springframework.cloud.kubernetes.commons.config.ConfigMapPropertySource; import org.springframework.core.env.Environment; +import org.springframework.util.CollectionUtils; /** * @author Ryan Baxter @@ -39,12 +40,12 @@ public class KubernetesClientConfigMapPropertySource extends ConfigMapPropertySo private static final Log LOG = LogFactory.getLog(KubernetesClientConfigMapPropertySource.class); public KubernetesClientConfigMapPropertySource(CoreV1Api coreV1Api, String name, String namespace, - Environment environment) { - super(getName(name, namespace), getData(coreV1Api, name, namespace, environment)); + Environment environment, String prefix) { + super(getName(name, namespace), getData(coreV1Api, name, namespace, environment, prefix)); } private static Map getData(CoreV1Api coreV1Api, String name, String namespace, - Environment environment) { + Environment environment, String prefix) { try { Set names = new HashSet<>(); @@ -60,6 +61,12 @@ public class KubernetesClientConfigMapPropertySource extends ConfigMapPropertySo .map(map -> processAllEntries(map.getData(), environment)).collect(Collectors.toList()) .forEach(result::putAll); + if (!"".equals(prefix)) { + Map withPrefix = CollectionUtils.newHashMap(result.size()); + result.forEach((key, value) -> withPrefix.put(prefix + "." + key, value)); + return withPrefix; + } + return result; } catch (ApiException e) { diff --git a/spring-cloud-kubernetes-client-config/src/main/java/org/springframework/cloud/kubernetes/client/config/KubernetesClientConfigMapPropertySourceLocator.java b/spring-cloud-kubernetes-client-config/src/main/java/org/springframework/cloud/kubernetes/client/config/KubernetesClientConfigMapPropertySourceLocator.java index a5628d8a..1ff3d6d8 100644 --- a/spring-cloud-kubernetes-client-config/src/main/java/org/springframework/cloud/kubernetes/client/config/KubernetesClientConfigMapPropertySourceLocator.java +++ b/spring-cloud-kubernetes-client-config/src/main/java/org/springframework/cloud/kubernetes/client/config/KubernetesClientConfigMapPropertySourceLocator.java @@ -59,7 +59,7 @@ public class KubernetesClientConfigMapPropertySourceLocator extends ConfigMapPro String fallbackNamespace = kubernetesNamespaceProvider != null ? kubernetesNamespaceProvider.getNamespace() : kubernetesClientProperties.getNamespace(); return new KubernetesClientConfigMapPropertySource(coreV1Api, name, - getNamespace(normalizedSource, fallbackNamespace), environment); + getNamespace(normalizedSource, fallbackNamespace), environment, normalizedSource.getPrefix()); } } diff --git a/spring-cloud-kubernetes-client-config/src/test/java/org/springframework/cloud/kubernetes/client/config/KubernetesClientConfigMapPropertySourceTests.java b/spring-cloud-kubernetes-client-config/src/test/java/org/springframework/cloud/kubernetes/client/config/KubernetesClientConfigMapPropertySourceTests.java index f4def1fe..48e04e93 100644 --- a/spring-cloud-kubernetes-client-config/src/test/java/org/springframework/cloud/kubernetes/client/config/KubernetesClientConfigMapPropertySourceTests.java +++ b/spring-cloud-kubernetes-client-config/src/test/java/org/springframework/cloud/kubernetes/client/config/KubernetesClientConfigMapPropertySourceTests.java @@ -97,7 +97,7 @@ class KubernetesClientConfigMapPropertySourceTests { stubFor(get(API) .willReturn(aResponse().withStatus(200).withBody(new JSON().serialize(PROPERTIES_CONFIGMAP_LIST)))); KubernetesClientConfigMapPropertySource propertySource = new KubernetesClientConfigMapPropertySource(api, - "bootstrap-640", "default", new MockEnvironment()); + "bootstrap-640", "default", new MockEnvironment(), ""); verify(getRequestedFor(urlEqualTo(API))); assertThat(propertySource.containsProperty("spring.cloud.kubernetes.configuration.watcher.refreshDelay")) .isTrue(); @@ -113,7 +113,7 @@ class KubernetesClientConfigMapPropertySourceTests { CoreV1Api api = new CoreV1Api(); stubFor(get(API).willReturn(aResponse().withStatus(200).withBody(new JSON().serialize(YAML_CONFIGMAP_LIST)))); KubernetesClientConfigMapPropertySource propertySource = new KubernetesClientConfigMapPropertySource(api, - "bootstrap-641", "default", new MockEnvironment()); + "bootstrap-641", "default", new MockEnvironment(), ""); verify(getRequestedFor(urlEqualTo(API))); assertThat(propertySource.containsProperty("dummy.property.string2")).isTrue(); assertThat(propertySource.getProperty("dummy.property.string2")).isEqualTo("a"); @@ -124,4 +124,22 @@ class KubernetesClientConfigMapPropertySourceTests { } + @Test + public void propertiesFileWithPrefix() { + CoreV1Api api = new CoreV1Api(); + stubFor(get(API) + .willReturn(aResponse().withStatus(200).withBody(new JSON().serialize(PROPERTIES_CONFIGMAP_LIST)))); + KubernetesClientConfigMapPropertySource propertySource = new KubernetesClientConfigMapPropertySource(api, + "bootstrap-640", "default", new MockEnvironment(), "prefix"); + verify(getRequestedFor(urlEqualTo(API))); + assertThat(propertySource.containsProperty("prefix.spring.cloud.kubernetes.configuration.watcher.refreshDelay")) + .isTrue(); + assertThat(propertySource.getProperty("prefix.spring.cloud.kubernetes.configuration.watcher.refreshDelay")) + .isEqualTo("0"); + assertThat(propertySource.containsProperty("prefix.logging.level.org.springframework.cloud.kubernetes")) + .isTrue(); + assertThat(propertySource.getProperty("prefix.logging.level.org.springframework.cloud.kubernetes")) + .isEqualTo("TRACE"); + } + } diff --git a/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/config/AbstractConfigProperties.java b/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/config/AbstractConfigProperties.java index a3f9cbbe..4e46486d 100644 --- a/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/config/AbstractConfigProperties.java +++ b/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/config/AbstractConfigProperties.java @@ -29,6 +29,8 @@ public abstract class AbstractConfigProperties { protected String namespace; + protected boolean useNameAsPrefix; + public abstract String getConfigurationTarget(); public boolean isEnabled() { @@ -55,4 +57,12 @@ public abstract class AbstractConfigProperties { this.namespace = namespace; } + public boolean isUseNameAsPrefix() { + return useNameAsPrefix; + } + + public void setUseNameAsPrefix(boolean useNameAsPrefix) { + this.useNameAsPrefix = useNameAsPrefix; + } + } diff --git a/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/config/ConfigMapConfigProperties.java b/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/config/ConfigMapConfigProperties.java index e8755fde..11c20e06 100644 --- a/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/config/ConfigMapConfigProperties.java +++ b/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/config/ConfigMapConfigProperties.java @@ -21,6 +21,9 @@ import java.util.List; import java.util.Objects; import java.util.stream.Collectors; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; + import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.util.StringUtils; @@ -32,6 +35,8 @@ import org.springframework.util.StringUtils; @ConfigurationProperties("spring.cloud.kubernetes.config") public class ConfigMapConfigProperties extends AbstractConfigProperties { + private static final Log LOG = LogFactory.getLog(ConfigMapConfigProperties.class); + private boolean enableApi = true; private List paths = Collections.emptyList(); @@ -72,10 +77,16 @@ public class ConfigMapConfigProperties extends AbstractConfigProperties { */ public List determineSources() { if (this.sources.isEmpty()) { - return Collections.singletonList(new NormalizedSource(name, namespace)); + if (useNameAsPrefix) { + LOG.warn( + "'spring.cloud.kubernetes.config.useNameAsPrefix' is set to 'true', but 'spring.cloud.kubernetes.config.sources'" + + " is empty; as such will default 'useNameAsPrefix' to 'false'"); + } + return Collections.singletonList(new NormalizedSource(name, namespace, "")); } - return sources.stream().map(s -> s.normalize(name, namespace)).collect(Collectors.toList()); + return sources.stream().map(s -> s.normalize(name, namespace, useNameAsPrefix, s.getExplicitPrefix())) + .collect(Collectors.toList()); } @Override @@ -98,6 +109,17 @@ public class ConfigMapConfigProperties extends AbstractConfigProperties { */ private String namespace; + /** + * Use config map name as prefix for properties. Can't be a primitive, we need to + * know if it was explicitly set or not + */ + private Boolean useNameAsPrefix; + + /** + * An explicit prefix to be used for properties. + */ + private String explicitPrefix; + public Source() { } @@ -123,14 +145,60 @@ public class ConfigMapConfigProperties extends AbstractConfigProperties { this.namespace = namespace; } + public Boolean isUseNameAsPrefix() { + return useNameAsPrefix; + } + + public void setUseNameAsPrefix(Boolean useNameAsPrefix) { + this.useNameAsPrefix = useNameAsPrefix; + } + + public String getExplicitPrefix() { + return explicitPrefix; + } + + public void setExplicitPrefix(String explicitPrefix) { + this.explicitPrefix = explicitPrefix; + } + public boolean isEmpty() { return !StringUtils.hasLength(this.name) && !StringUtils.hasLength(this.namespace); } + // not used, but not removed because of potential compatibility reasons + @Deprecated public NormalizedSource normalize(String defaultName, String defaultNamespace) { String normalizedName = StringUtils.hasLength(this.name) ? this.name : defaultName; String normalizedNamespace = StringUtils.hasLength(this.namespace) ? this.namespace : defaultNamespace; - return new NormalizedSource(normalizedName, normalizedNamespace); + return new NormalizedSource(normalizedName, normalizedNamespace, ""); + } + + public NormalizedSource normalize(String defaultName, String defaultNamespace, boolean defaultUseNameAsPrefix, + String explicitPrefix) { + String normalizedName = StringUtils.hasLength(this.name) ? this.name : defaultName; + String normalizedNamespace = StringUtils.hasLength(this.namespace) ? this.namespace : defaultNamespace; + + // if explicitPrefix is set, it takes priority over useNameAsPrefix + // (either the one from 'spring.cloud.kubernetes.config' or + // 'spring.cloud.kubernetes.config.sources') + if (StringUtils.hasText(explicitPrefix)) { + return new NormalizedSource(normalizedName, normalizedNamespace, explicitPrefix); + } + + // useNameAsPrefix is a java.lang.Boolean and if it's != null, users have + // specified it explicitly + if (this.useNameAsPrefix != null) { + if (useNameAsPrefix) { + return new NormalizedSource(normalizedName, normalizedNamespace, normalizedName); + } + return new NormalizedSource(normalizedName, normalizedNamespace, ""); + } + + if (defaultUseNameAsPrefix) { + return new NormalizedSource(normalizedName, normalizedNamespace, normalizedName); + } + + return new NormalizedSource(normalizedName, normalizedNamespace, ""); } @Override @@ -158,9 +226,20 @@ public class ConfigMapConfigProperties extends AbstractConfigProperties { private final String namespace; + private final String prefix; + + // not used, but not removed because of potential compatibility reasons + @Deprecated NormalizedSource(String name, String namespace) { this.name = name; this.namespace = namespace; + this.prefix = ""; + } + + NormalizedSource(String name, String namespace, String prefix) { + this.name = name; + this.namespace = namespace; + this.prefix = Objects.requireNonNull(prefix); } public String getName() { @@ -171,9 +250,13 @@ public class ConfigMapConfigProperties extends AbstractConfigProperties { return this.namespace; } + public String getPrefix() { + return prefix; + } + @Override public String toString() { - return "{ config-map name : '" + name + "', namespace : '" + namespace + "' }"; + return "{ config-map name : '" + name + "', namespace : '" + namespace + "', prefix : '" + prefix + "' }"; } @Override diff --git a/spring-cloud-kubernetes-commons/src/test/java/org/springframework/cloud/kubernetes/commons/config/ConfigMapConfigPropertiesTests.java b/spring-cloud-kubernetes-commons/src/test/java/org/springframework/cloud/kubernetes/commons/config/ConfigMapConfigPropertiesTests.java new file mode 100644 index 00000000..05406165 --- /dev/null +++ b/spring-cloud-kubernetes-commons/src/test/java/org/springframework/cloud/kubernetes/commons/config/ConfigMapConfigPropertiesTests.java @@ -0,0 +1,221 @@ +/* + * Copyright 2013-2021 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.Arrays; +import java.util.Collections; +import java.util.List; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +/** + * @author wind57 + */ +public class ConfigMapConfigPropertiesTests { + + /** + * 
+	 * 	spring:
+	 *	  cloud:
+	 *      kubernetes:
+	 *        config:
+	 *          name: config-map-a
+	 *        	namespace: spring-k8s
+	 *
+ * + * a config as above will result in a NormalizedSource where prefix is empty + */ + @Test + public void testUseNameAsPrefixUnsetEmptySources() { + ConfigMapConfigProperties properties = new ConfigMapConfigProperties(); + properties.setSources(Collections.emptyList()); + properties.setName("config-map-a"); + properties.setNamespace("spring-k8s"); + + List sources = properties.determineSources(); + Assertions.assertEquals(sources.size(), 1, "empty sources must generate a List with a single NormalizedSource"); + + Assertions.assertEquals(sources.get(0).getPrefix(), "", + "empty sources must generate a List with a single NormalizedSource, where prefix is empty"); + } + + /** + * 
+	 * 	spring:
+	 *	  cloud:
+	 *      kubernetes:
+	 *        config:
+	 *          useNameAsPrefix: true
+	 *          name: config-map-a
+	 *        	namespace: spring-k8s
+	 *
+ * + * a config as above will result in a NormalizedSource where prefix is empty, even if + * "useNameAsPrefix: true", because sources are empty + */ + @Test + public void testUseNameAsPrefixSetEmptySources() { + ConfigMapConfigProperties properties = new ConfigMapConfigProperties(); + properties.setSources(Collections.emptyList()); + properties.setUseNameAsPrefix(true); + properties.setName("config-map-a"); + properties.setNamespace("spring-k8s"); + + List sources = properties.determineSources(); + Assertions.assertEquals(sources.size(), 1, "empty sources must generate a List with a single NormalizedSource"); + + Assertions.assertEquals(sources.get(0).getPrefix(), "", + "empty sources must generate a List with a single NormalizedSource, where prefix is empty," + + "no matter of 'spring.cloud.kubernetes.config.useNameAsPrefix' value"); + } + + /** + * 
+	 * spring:
+	 *	cloud:
+	 *    kubernetes:
+	 *      config:
+	 *        useNameAsPrefix: true
+	 *        namespace: spring-k8s
+	 *        sources:
+	 *          - name: config-map-one
+	 *
+ * + * a config as above will result in a NormalizedSource where prefix will be equal to + * the config map name + */ + @Test + public void testUseNameAsPrefixUnsetNonEmptySources() { + ConfigMapConfigProperties properties = new ConfigMapConfigProperties(); + properties.setUseNameAsPrefix(true); + properties.setNamespace("spring-k8s"); + + ConfigMapConfigProperties.Source one = new ConfigMapConfigProperties.Source(); + one.setName("config-map-one"); + properties.setSources(Collections.singletonList(one)); + + List sources = properties.determineSources(); + Assertions.assertEquals(sources.size(), 1, "a single NormalizedSource is expected"); + + Assertions.assertEquals(sources.get(0).getPrefix(), "config-map-one"); + } + + /** + * 
+	 * spring:
+	 *	cloud:
+	 *    kubernetes:
+	 *      config:
+	 *        useNameAsPrefix: true
+	 *        namespace: spring-k8s
+	 *        sources:
+	 *          - name: config-map-one
+	 *            useNameAsPrefix: false
+	 *          - name: config-map-two
+	 *            useNameAsPrefix: true
+	 *          - name: config-map-three
+	 *
+ * + * this test proves that 'spring.cloud.kubernetes.config.sources[].useNameAsPrefix' + * will override 'spring.cloud.kubernetes.config.useNameAsPrefix'. For the last entry + * in sources, since there is no explicit 'useNameAsPrefix', the one from + * 'spring.cloud.kubernetes.config.useNameAsPrefix' will be taken. + */ + @Test + public void testUseNameAsPrefixSetNonEmptySources() { + ConfigMapConfigProperties properties = new ConfigMapConfigProperties(); + properties.setUseNameAsPrefix(true); + properties.setNamespace("spring-k8s"); + + ConfigMapConfigProperties.Source one = new ConfigMapConfigProperties.Source(); + one.setName("config-map-one"); + one.setUseNameAsPrefix(false); + + ConfigMapConfigProperties.Source two = new ConfigMapConfigProperties.Source(); + two.setName("config-map-two"); + two.setUseNameAsPrefix(true); + + ConfigMapConfigProperties.Source three = new ConfigMapConfigProperties.Source(); + three.setName("config-map-three"); + + properties.setSources(Arrays.asList(one, two, three)); + + List sources = properties.determineSources(); + Assertions.assertEquals(sources.size(), 3, "3 NormalizedSources are expected"); + + Assertions.assertEquals(sources.get(0).getPrefix(), ""); + Assertions.assertEquals(sources.get(1).getPrefix(), "config-map-two"); + Assertions.assertEquals(sources.get(2).getPrefix(), "config-map-three"); + } + + /** + * 
+	 * spring:
+	 *	cloud:
+	 *    kubernetes:
+	 *      config:
+	 *        useNameAsPrefix: false
+	 *        namespace: spring-k8s
+	 *        sources:
+	 *          - name: config-map-one
+	 *            useNameAsPrefix: false
+	 *            explicitPrefix: one
+	 *          - name: config-map-two
+	 *            useNameAsPrefix: true
+	 *            explicitPrefix: two
+	 *          - name: config-map-three
+	 *            explicitPrefix: three
+	 *          - name: config-map-four
+	 *
+ * + */ + @Test + public void testMultipleCases() { + ConfigMapConfigProperties properties = new ConfigMapConfigProperties(); + properties.setUseNameAsPrefix(false); + properties.setNamespace("spring-k8s"); + + ConfigMapConfigProperties.Source one = new ConfigMapConfigProperties.Source(); + one.setNamespace("config-map-one"); + one.setUseNameAsPrefix(false); + one.setExplicitPrefix("one"); + + ConfigMapConfigProperties.Source two = new ConfigMapConfigProperties.Source(); + two.setNamespace("config-map-two"); + two.setUseNameAsPrefix(true); + two.setExplicitPrefix("two"); + + ConfigMapConfigProperties.Source three = new ConfigMapConfigProperties.Source(); + three.setNamespace("config-map-three"); + three.setExplicitPrefix("three"); + + ConfigMapConfigProperties.Source four = new ConfigMapConfigProperties.Source(); + four.setNamespace("config-map-four"); + + properties.setSources(Arrays.asList(one, two, three, four)); + + List sources = properties.determineSources(); + Assertions.assertEquals(sources.size(), 4, "4 NormalizedSources are expected"); + + Assertions.assertEquals(sources.get(0).getPrefix(), "one"); + Assertions.assertEquals(sources.get(1).getPrefix(), "two"); + Assertions.assertEquals(sources.get(2).getPrefix(), "three"); + Assertions.assertEquals(sources.get(3).getPrefix(), ""); + } + +} diff --git a/spring-cloud-kubernetes-fabric8-config/src/main/java/org/springframework/cloud/kubernetes/fabric8/config/Fabric8ConfigMapPropertySource.java b/spring-cloud-kubernetes-fabric8-config/src/main/java/org/springframework/cloud/kubernetes/fabric8/config/Fabric8ConfigMapPropertySource.java index 142009f2..6874fbce 100644 --- a/spring-cloud-kubernetes-fabric8-config/src/main/java/org/springframework/cloud/kubernetes/fabric8/config/Fabric8ConfigMapPropertySource.java +++ b/spring-cloud-kubernetes-fabric8-config/src/main/java/org/springframework/cloud/kubernetes/fabric8/config/Fabric8ConfigMapPropertySource.java @@ -27,6 +27,7 @@ import org.apache.commons.logging.LogFactory; import org.springframework.cloud.kubernetes.commons.config.ConfigMapPropertySource; import org.springframework.core.env.Environment; import org.springframework.core.env.MapPropertySource; +import org.springframework.util.CollectionUtils; import static org.springframework.cloud.kubernetes.fabric8.config.Fabric8ConfigUtils.getApplicationNamespace; import static org.springframework.cloud.kubernetes.fabric8.config.Fabric8ConfigUtils.getConfigMapData; @@ -43,17 +44,23 @@ public class Fabric8ConfigMapPropertySource extends ConfigMapPropertySource { private static final Log LOG = LogFactory.getLog(Fabric8ConfigMapPropertySource.class); public Fabric8ConfigMapPropertySource(KubernetesClient client, String name) { - this(client, name, null, null); + this(client, name, null, null, ""); } public Fabric8ConfigMapPropertySource(KubernetesClient client, String applicationName, String namespace, Environment environment) { super(getName(applicationName, getApplicationNamespace(client, namespace)), - getData(client, applicationName, getApplicationNamespace(client, namespace), environment)); + getData(client, applicationName, getApplicationNamespace(client, namespace), environment, "")); + } + + public Fabric8ConfigMapPropertySource(KubernetesClient client, String applicationName, String namespace, + Environment environment, String prefix) { + super(getName(applicationName, getApplicationNamespace(client, namespace)), + getData(client, applicationName, getApplicationNamespace(client, namespace), environment, prefix)); } private static Map getData(KubernetesClient client, String applicationName, String namespace, - Environment environment) { + Environment environment, String prefix) { try { Map data = getConfigMapData(client, namespace, applicationName); Map result = new HashMap<>(processAllEntries(data, environment)); @@ -66,11 +73,17 @@ public class Fabric8ConfigMapPropertySource extends ConfigMapPropertySource { } } + if (!"".equals(prefix)) { + Map withPrefix = CollectionUtils.newHashMap(result.size()); + result.forEach((key, value) -> withPrefix.put(prefix + "." + key, value)); + return withPrefix; + } + return result; } catch (Exception e) { - LOG.warn("Can't read configMap with name: [" + applicationName + "] in namespace:[" + namespace + LOG.warn("Can't read configMap with name: [" + applicationName + "] in namespace: [" + namespace + "]. Ignoring.", e); } diff --git a/spring-cloud-kubernetes-fabric8-config/src/main/java/org/springframework/cloud/kubernetes/fabric8/config/Fabric8ConfigMapPropertySourceLocator.java b/spring-cloud-kubernetes-fabric8-config/src/main/java/org/springframework/cloud/kubernetes/fabric8/config/Fabric8ConfigMapPropertySourceLocator.java index bde03dcc..98539c4f 100644 --- a/spring-cloud-kubernetes-fabric8-config/src/main/java/org/springframework/cloud/kubernetes/fabric8/config/Fabric8ConfigMapPropertySourceLocator.java +++ b/spring-cloud-kubernetes-fabric8-config/src/main/java/org/springframework/cloud/kubernetes/fabric8/config/Fabric8ConfigMapPropertySourceLocator.java @@ -49,7 +49,8 @@ public class Fabric8ConfigMapPropertySourceLocator extends ConfigMapPropertySour String configurationTarget, ConfigurableEnvironment environment) { String namespaceName = getApplicationNamespace(this.client, normalizedSource.getNamespace(), configurationTarget); - return new Fabric8ConfigMapPropertySource(this.client, applicationName, namespaceName, environment); + return new Fabric8ConfigMapPropertySource(this.client, applicationName, namespaceName, environment, + normalizedSource.getPrefix()); } } diff --git a/spring-cloud-kubernetes-fabric8-config/src/test/java/org/springframework/cloud/kubernetes/fabric8/config/ConfigMapWithPrefixTests.java b/spring-cloud-kubernetes-fabric8-config/src/test/java/org/springframework/cloud/kubernetes/fabric8/config/ConfigMapWithPrefixTests.java new file mode 100644 index 00000000..934034f3 --- /dev/null +++ b/spring-cloud-kubernetes-fabric8-config/src/test/java/org/springframework/cloud/kubernetes/fabric8/config/ConfigMapWithPrefixTests.java @@ -0,0 +1,128 @@ +/* + * Copyright 2013-2021 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.fabric8.config; + +import java.util.HashMap; +import java.util.Map; + +import io.fabric8.kubernetes.client.Config; +import io.fabric8.kubernetes.client.KubernetesClient; +import io.fabric8.kubernetes.client.server.mock.EnableKubernetesMockClient; +import org.hamcrest.Matchers; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.web.reactive.AutoConfigureWebTestClient; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.cloud.kubernetes.fabric8.config.with_prefix.WithPrefixApp; +import org.springframework.test.context.junit.jupiter.SpringExtension; +import org.springframework.test.web.reactive.server.WebTestClient; + +/** + * @author wind57 + */ +@ExtendWith(SpringExtension.class) +@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT, classes = WithPrefixApp.class, + properties = { "spring.cloud.bootstrap.name=same-key-with-prefix" }) +@AutoConfigureWebTestClient +@EnableKubernetesMockClient(crud = true, https = false) +public class ConfigMapWithPrefixTests { + + private static KubernetesClient mockClient; + + @Autowired + private WebTestClient webClient; + + @BeforeAll + public static void setUpBeforeClass() { + + // Configure the kubernetes master url to point to the mock server + System.setProperty(Config.KUBERNETES_MASTER_SYSTEM_PROPERTY, mockClient.getConfiguration().getMasterUrl()); + System.setProperty(Config.KUBERNETES_TRUST_CERT_SYSTEM_PROPERTY, "true"); + System.setProperty(Config.KUBERNETES_AUTH_TRYKUBECONFIG_SYSTEM_PROPERTY, "false"); + System.setProperty(Config.KUBERNETES_AUTH_TRYSERVICEACCOUNT_SYSTEM_PROPERTY, "false"); + System.setProperty(Config.KUBERNETES_NAMESPACE_SYSTEM_PROPERTY, "test"); + System.setProperty(Config.KUBERNETES_HTTP2_DISABLE, "true"); + + Map one = new HashMap<>(); + one.put("one.property", "one"); + createConfigmap(mockClient, "config-map-one", one); + + Map two = new HashMap<>(); + two.put("property", "two"); + createConfigmap(mockClient, "config-map-two", two); + + Map three = new HashMap<>(); + three.put("property", "three"); + createConfigmap(mockClient, "config-map-three", three); + + } + + private static void createConfigmap(KubernetesClient client, String name, Map data) { + + client.configMaps().inNamespace("spring-k8s").createNew().withNewMetadata().withName(name).endMetadata() + .addToData(data).done(); + } + + /** + * 
+	 *   'spring.cloud.kubernetes.config.useNameAsPrefix=true'
+	 *   'spring.cloud.kubernetes.config.sources[0].useNameAsPrefix=false'
+	 * 	 ("one.property", "one")
+	 *
+	 * 	 As such: @ConfigurationProperties("one")
+	 *
+ */ + @Test + public void testOne() { + this.webClient.get().uri("/one").exchange().expectStatus().isOk().expectBody(String.class) + .value(Matchers.equalTo("one")); + } + + /** + * 
+	 *   'spring.cloud.kubernetes.config.useNameAsPrefix=true'
+	 *   'spring.cloud.kubernetes.config.sources[1].explicitPrefix=two'
+	 * 	 ("property", "two")
+	 *
+	 * 	 As such: @ConfigurationProperties("two")
+	 *
+ */ + @Test + public void testTwo() { + this.webClient.get().uri("/two").exchange().expectStatus().isOk().expectBody(String.class) + .value(Matchers.equalTo("two")); + } + + /** + * 
+	 *   'spring.cloud.kubernetes.config.useNameAsPrefix=true'
+	 *   'spring.cloud.kubernetes.config.sources[2].name=config-map-three'
+	 * 	 ("property", "three")
+	 *
+	 * 	 As such: @ConfigurationProperties(prefix = "config-map-three")
+	 *
+ */ + @Test + public void testThree() { + this.webClient.get().uri("/three").exchange().expectStatus().isOk().expectBody(String.class) + .value(Matchers.equalTo("three")); + } + +} diff --git a/spring-cloud-kubernetes-fabric8-config/src/test/java/org/springframework/cloud/kubernetes/fabric8/config/with_prefix/WithPrefixApp.java b/spring-cloud-kubernetes-fabric8-config/src/test/java/org/springframework/cloud/kubernetes/fabric8/config/with_prefix/WithPrefixApp.java new file mode 100644 index 00000000..116ac5e5 --- /dev/null +++ b/spring-cloud-kubernetes-fabric8-config/src/test/java/org/springframework/cloud/kubernetes/fabric8/config/with_prefix/WithPrefixApp.java @@ -0,0 +1,34 @@ +/* + * Copyright 2013-2021 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.fabric8.config.with_prefix; + +import org.springframework.boot.SpringApplication; +import org.springframework.boot.autoconfigure.SpringBootApplication; +import org.springframework.boot.context.properties.EnableConfigurationProperties; +import org.springframework.cloud.kubernetes.fabric8.config.with_prefix.properties.One; +import org.springframework.cloud.kubernetes.fabric8.config.with_prefix.properties.Three; +import org.springframework.cloud.kubernetes.fabric8.config.with_prefix.properties.Two; + +@SpringBootApplication +@EnableConfigurationProperties({ One.class, Two.class, Three.class }) +public class WithPrefixApp { + + public static void main(String[] args) { + SpringApplication.run(WithPrefixApp.class, args); + } + +} diff --git a/spring-cloud-kubernetes-fabric8-config/src/test/java/org/springframework/cloud/kubernetes/fabric8/config/with_prefix/controller/Controller.java b/spring-cloud-kubernetes-fabric8-config/src/test/java/org/springframework/cloud/kubernetes/fabric8/config/with_prefix/controller/Controller.java new file mode 100644 index 00000000..86886d10 --- /dev/null +++ b/spring-cloud-kubernetes-fabric8-config/src/test/java/org/springframework/cloud/kubernetes/fabric8/config/with_prefix/controller/Controller.java @@ -0,0 +1,55 @@ +/* + * Copyright 2013-2021 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.fabric8.config.with_prefix.controller; + +import org.springframework.cloud.kubernetes.fabric8.config.with_prefix.properties.One; +import org.springframework.cloud.kubernetes.fabric8.config.with_prefix.properties.Three; +import org.springframework.cloud.kubernetes.fabric8.config.with_prefix.properties.Two; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.RestController; + +@RestController +public class Controller { + + private final One one; + + private final Two two; + + private final Three three; + + public Controller(One one, Two two, Three three) { + this.one = one; + this.two = two; + this.three = three; + } + + @GetMapping("/one") + public String one() { + return one.getProperty(); + } + + @GetMapping("/two") + public String two() { + return two.getProperty(); + } + + @GetMapping("/three") + public String three() { + return three.getProperty(); + } + +} diff --git a/spring-cloud-kubernetes-fabric8-config/src/test/java/org/springframework/cloud/kubernetes/fabric8/config/with_prefix/properties/One.java b/spring-cloud-kubernetes-fabric8-config/src/test/java/org/springframework/cloud/kubernetes/fabric8/config/with_prefix/properties/One.java new file mode 100644 index 00000000..5503f6c0 --- /dev/null +++ b/spring-cloud-kubernetes-fabric8-config/src/test/java/org/springframework/cloud/kubernetes/fabric8/config/with_prefix/properties/One.java @@ -0,0 +1,34 @@ +/* + * Copyright 2013-2021 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.fabric8.config.with_prefix.properties; + +import org.springframework.boot.context.properties.ConfigurationProperties; + +@ConfigurationProperties("one") +public class One { + + private String property; + + public String getProperty() { + return property; + } + + public void setProperty(String property) { + this.property = property; + } + +} diff --git a/spring-cloud-kubernetes-fabric8-config/src/test/java/org/springframework/cloud/kubernetes/fabric8/config/with_prefix/properties/Three.java b/spring-cloud-kubernetes-fabric8-config/src/test/java/org/springframework/cloud/kubernetes/fabric8/config/with_prefix/properties/Three.java new file mode 100644 index 00000000..93d41649 --- /dev/null +++ b/spring-cloud-kubernetes-fabric8-config/src/test/java/org/springframework/cloud/kubernetes/fabric8/config/with_prefix/properties/Three.java @@ -0,0 +1,34 @@ +/* + * Copyright 2013-2021 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.fabric8.config.with_prefix.properties; + +import org.springframework.boot.context.properties.ConfigurationProperties; + +@ConfigurationProperties(prefix = "config-map-three") +public class Three { + + private String property; + + public String getProperty() { + return property; + } + + public void setProperty(String property) { + this.property = property; + } + +} diff --git a/spring-cloud-kubernetes-fabric8-config/src/test/java/org/springframework/cloud/kubernetes/fabric8/config/with_prefix/properties/Two.java b/spring-cloud-kubernetes-fabric8-config/src/test/java/org/springframework/cloud/kubernetes/fabric8/config/with_prefix/properties/Two.java new file mode 100644 index 00000000..ee33bc75 --- /dev/null +++ b/spring-cloud-kubernetes-fabric8-config/src/test/java/org/springframework/cloud/kubernetes/fabric8/config/with_prefix/properties/Two.java @@ -0,0 +1,34 @@ +/* + * Copyright 2013-2021 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.fabric8.config.with_prefix.properties; + +import org.springframework.boot.context.properties.ConfigurationProperties; + +@ConfigurationProperties("two") +public class Two { + + private String property; + + public String getProperty() { + return property; + } + + public void setProperty(String property) { + this.property = property; + } + +} diff --git a/spring-cloud-kubernetes-fabric8-config/src/test/resources/same-key-with-prefix.yaml b/spring-cloud-kubernetes-fabric8-config/src/test/resources/same-key-with-prefix.yaml new file mode 100644 index 00000000..b52095e4 --- /dev/null +++ b/spring-cloud-kubernetes-fabric8-config/src/test/resources/same-key-with-prefix.yaml @@ -0,0 +1,14 @@ +spring: + application: + name: with-prefix + cloud: + kubernetes: + config: + useNameAsPrefix: true + namespace: spring-k8s + sources: + - name: config-map-one + useNameAsPrefix: false + - name: config-map-two + explicitPrefix: two + - name: config-map-three