From 1d9ebbfb45aaedba33d69287f94fa8bf987a4def Mon Sep 17 00:00:00 2001 From: erabii Date: Thu, 9 Sep 2021 15:55:42 -0400 Subject: [PATCH] fix proposal for 859 (#860) * trying fix * checkstyle * fix * trigger build * trigger build --- ...sClientConfigMapPropertySourceLocator.java | 7 +++-- ...tesClientSecretsPropertySourceLocator.java | 16 ++++++++-- ...tEventBasedSecretsChangeDetectorTests.java | 3 +- .../config/Fabric8BootstrapConfiguration.java | 15 ++++++--- ...Fabric8ConfigMapPropertySourceLocator.java | 25 +++++++++++++-- .../fabric8/config/Fabric8ConfigUtils.java | 22 +++++++++++++ .../config/Fabric8SecretsPropertySource.java | 6 ++-- .../Fabric8SecretsPropertySourceLocator.java | 31 ++++++++++++++++--- 8 files changed, 105 insertions(+), 20 deletions(-) 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 1ff3d6d8..043dc7c3 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 @@ -34,15 +34,16 @@ public class KubernetesClientConfigMapPropertySourceLocator extends ConfigMapPro private final CoreV1Api coreV1Api; - private KubernetesClientProperties kubernetesClientProperties; + private final KubernetesClientProperties kubernetesClientProperties; - private KubernetesNamespaceProvider kubernetesNamespaceProvider; + private final KubernetesNamespaceProvider kubernetesNamespaceProvider; public KubernetesClientConfigMapPropertySourceLocator(CoreV1Api coreV1Api, ConfigMapConfigProperties properties, KubernetesClientProperties kubernetesClientProperties) { super(properties); this.coreV1Api = coreV1Api; this.kubernetesClientProperties = kubernetesClientProperties; + this.kubernetesNamespaceProvider = null; } public KubernetesClientConfigMapPropertySourceLocator(CoreV1Api coreV1Api, ConfigMapConfigProperties properties, @@ -50,12 +51,14 @@ public class KubernetesClientConfigMapPropertySourceLocator extends ConfigMapPro super(properties); this.coreV1Api = coreV1Api; this.kubernetesNamespaceProvider = kubernetesNamespaceProvider; + this.kubernetesClientProperties = null; } @Override protected MapPropertySource getMapPropertySource(String name, ConfigMapConfigProperties.NormalizedSource normalizedSource, String configurationTarget, ConfigurableEnvironment environment) { + String fallbackNamespace = kubernetesNamespaceProvider != null ? kubernetesNamespaceProvider.getNamespace() : kubernetesClientProperties.getNamespace(); return new KubernetesClientConfigMapPropertySource(coreV1Api, name, diff --git a/spring-cloud-kubernetes-client-config/src/main/java/org/springframework/cloud/kubernetes/client/config/KubernetesClientSecretsPropertySourceLocator.java b/spring-cloud-kubernetes-client-config/src/main/java/org/springframework/cloud/kubernetes/client/config/KubernetesClientSecretsPropertySourceLocator.java index 2364acf9..c463fc00 100644 --- a/spring-cloud-kubernetes-client-config/src/main/java/org/springframework/cloud/kubernetes/client/config/KubernetesClientSecretsPropertySourceLocator.java +++ b/spring-cloud-kubernetes-client-config/src/main/java/org/springframework/cloud/kubernetes/client/config/KubernetesClientSecretsPropertySourceLocator.java @@ -33,17 +33,26 @@ import static org.springframework.cloud.kubernetes.commons.config.ConfigUtils.ge */ public class KubernetesClientSecretsPropertySourceLocator extends SecretsPropertySourceLocator { - private CoreV1Api coreV1Api; + private final CoreV1Api coreV1Api; - private KubernetesClientProperties kubernetesClientProperties; + private final KubernetesClientProperties kubernetesClientProperties; - private KubernetesNamespaceProvider kubernetesNamespaceProvider; + private final KubernetesNamespaceProvider kubernetesNamespaceProvider; + /** + * This constructor is deprecated. Its usage might cause unexpected behavior when + * looking for different properties. For example, in general, if a namespace is not + * provided, we might look it up via other means: different documented environment + * variables or from a kubernetes client itself. Using this constructor might not + * reflect that. + */ + @Deprecated public KubernetesClientSecretsPropertySourceLocator(CoreV1Api coreV1Api, KubernetesClientProperties kubernetesClientProperties, SecretsConfigProperties secretsConfigProperties) { super(secretsConfigProperties); this.coreV1Api = coreV1Api; this.kubernetesClientProperties = kubernetesClientProperties; + this.kubernetesNamespaceProvider = null; } public KubernetesClientSecretsPropertySourceLocator(CoreV1Api coreV1Api, @@ -51,6 +60,7 @@ public class KubernetesClientSecretsPropertySourceLocator extends SecretsPropert super(secretsConfigProperties); this.coreV1Api = coreV1Api; this.kubernetesNamespaceProvider = kubernetesNamespaceProvider; + this.kubernetesClientProperties = null; } @Override diff --git a/spring-cloud-kubernetes-client-config/src/test/java/org/springframework/cloud/kubernetes/client/config/reload/KubernetesClientEventBasedSecretsChangeDetectorTests.java b/spring-cloud-kubernetes-client-config/src/test/java/org/springframework/cloud/kubernetes/client/config/reload/KubernetesClientEventBasedSecretsChangeDetectorTests.java index 61d28876..4827b391 100644 --- a/spring-cloud-kubernetes-client-config/src/test/java/org/springframework/cloud/kubernetes/client/config/reload/KubernetesClientEventBasedSecretsChangeDetectorTests.java +++ b/spring-cloud-kubernetes-client-config/src/test/java/org/springframework/cloud/kubernetes/client/config/reload/KubernetesClientEventBasedSecretsChangeDetectorTests.java @@ -137,7 +137,8 @@ class KubernetesClientEventBasedSecretsChangeDetectorTests { KubernetesMockEnvironment environment = new KubernetesMockEnvironment( mock(KubernetesClientSecretsPropertySource.class)).withProperty("db-password", "p455w0rd"); KubernetesClientSecretsPropertySourceLocator locator = mock(KubernetesClientSecretsPropertySourceLocator.class); - when(locator.locate(environment)).thenAnswer(ignoreMe -> new MockPropertySource().withProperty("db-password", "p455w0rd2")); + when(locator.locate(environment)) + .thenAnswer(ignoreMe -> new MockPropertySource().withProperty("db-password", "p455w0rd2")); ConfigReloadProperties properties = new ConfigReloadProperties(); properties.setMonitoringSecrets(true); KubernetesNamespaceProvider kubernetesNamespaceProvider = mock(KubernetesNamespaceProvider.class); diff --git a/spring-cloud-kubernetes-fabric8-config/src/main/java/org/springframework/cloud/kubernetes/fabric8/config/Fabric8BootstrapConfiguration.java b/spring-cloud-kubernetes-fabric8-config/src/main/java/org/springframework/cloud/kubernetes/fabric8/config/Fabric8BootstrapConfiguration.java index 44eb0ccc..86078dd1 100644 --- a/spring-cloud-kubernetes-fabric8-config/src/main/java/org/springframework/cloud/kubernetes/fabric8/config/Fabric8BootstrapConfiguration.java +++ b/spring-cloud-kubernetes-fabric8-config/src/main/java/org/springframework/cloud/kubernetes/fabric8/config/Fabric8BootstrapConfiguration.java @@ -26,6 +26,7 @@ import org.springframework.cloud.kubernetes.commons.ConditionalOnKubernetesConfi import org.springframework.cloud.kubernetes.commons.ConditionalOnKubernetesEnabled; import org.springframework.cloud.kubernetes.commons.ConditionalOnKubernetesSecretsEnabled; import org.springframework.cloud.kubernetes.commons.KubernetesCommonsAutoConfiguration; +import org.springframework.cloud.kubernetes.commons.KubernetesNamespaceProvider; import org.springframework.cloud.kubernetes.commons.config.ConfigMapConfigProperties; import org.springframework.cloud.kubernetes.commons.config.KubernetesBootstrapConfiguration; import org.springframework.cloud.kubernetes.commons.config.SecretsConfigProperties; @@ -33,6 +34,7 @@ import org.springframework.cloud.kubernetes.fabric8.Fabric8AutoConfiguration; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Import; +import org.springframework.core.env.Environment; /** * Auto configuration that reuses Kubernetes config maps as property sources. @@ -46,18 +48,23 @@ import org.springframework.context.annotation.Import; @AutoConfigureAfter(KubernetesBootstrapConfiguration.class) public class Fabric8BootstrapConfiguration { + @Bean + public KubernetesNamespaceProvider provider(Environment env) { + return new KubernetesNamespaceProvider(env); + } + @Bean @ConditionalOnKubernetesConfigEnabled public Fabric8ConfigMapPropertySourceLocator configMapPropertySourceLocator(ConfigMapConfigProperties properties, - KubernetesClient client) { - return new Fabric8ConfigMapPropertySourceLocator(client, properties); + KubernetesClient client, KubernetesNamespaceProvider provider) { + return new Fabric8ConfigMapPropertySourceLocator(client, properties, provider); } @Bean @ConditionalOnKubernetesSecretsEnabled public Fabric8SecretsPropertySourceLocator secretsPropertySourceLocator(SecretsConfigProperties properties, - KubernetesClient client) { - return new Fabric8SecretsPropertySourceLocator(client, properties); + KubernetesClient client, KubernetesNamespaceProvider provider) { + return new Fabric8SecretsPropertySourceLocator(client, properties, provider); } } 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 98539c4f..be6b76d3 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 @@ -19,6 +19,7 @@ package org.springframework.cloud.kubernetes.fabric8.config; import io.fabric8.kubernetes.client.KubernetesClient; import org.springframework.cloud.bootstrap.config.PropertySourceLocator; +import org.springframework.cloud.kubernetes.commons.KubernetesNamespaceProvider; import org.springframework.cloud.kubernetes.commons.config.ConfigMapConfigProperties; import org.springframework.cloud.kubernetes.commons.config.ConfigMapConfigProperties.NormalizedSource; import org.springframework.cloud.kubernetes.commons.config.ConfigMapPropertySourceLocator; @@ -39,17 +40,35 @@ public class Fabric8ConfigMapPropertySourceLocator extends ConfigMapPropertySour private final KubernetesClient client; + private final KubernetesNamespaceProvider provider; + + /** + * This constructor is deprecated. Its usage might cause unexpected behavior when + * looking for different properties. For example, in general, if a namespace is not + * provided, we might look it up via other means: different documented environment + * variables or from a kubernetes client itself. Using this constructor might not + * reflect that. + */ + @Deprecated public Fabric8ConfigMapPropertySourceLocator(KubernetesClient client, ConfigMapConfigProperties properties) { super(properties); this.client = client; + this.provider = null; + } + + public Fabric8ConfigMapPropertySourceLocator(KubernetesClient client, ConfigMapConfigProperties properties, + KubernetesNamespaceProvider provider) { + super(properties); + this.client = client; + this.provider = provider; } @Override protected MapPropertySource getMapPropertySource(String applicationName, NormalizedSource normalizedSource, String configurationTarget, ConfigurableEnvironment environment) { - String namespaceName = getApplicationNamespace(this.client, normalizedSource.getNamespace(), - configurationTarget); - return new Fabric8ConfigMapPropertySource(this.client, applicationName, namespaceName, environment, + String configMapName = getApplicationNamespace(this.client, normalizedSource.getNamespace(), + configurationTarget, provider); + return new Fabric8ConfigMapPropertySource(this.client, applicationName, configMapName, environment, normalizedSource.getPrefix()); } diff --git a/spring-cloud-kubernetes-fabric8-config/src/main/java/org/springframework/cloud/kubernetes/fabric8/config/Fabric8ConfigUtils.java b/spring-cloud-kubernetes-fabric8-config/src/main/java/org/springframework/cloud/kubernetes/fabric8/config/Fabric8ConfigUtils.java index 005ba697..000fb4f5 100644 --- a/spring-cloud-kubernetes-fabric8-config/src/main/java/org/springframework/cloud/kubernetes/fabric8/config/Fabric8ConfigUtils.java +++ b/spring-cloud-kubernetes-fabric8-config/src/main/java/org/springframework/cloud/kubernetes/fabric8/config/Fabric8ConfigUtils.java @@ -24,6 +24,7 @@ import io.fabric8.kubernetes.client.KubernetesClient; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.springframework.cloud.kubernetes.commons.KubernetesNamespaceProvider; import org.springframework.util.StringUtils; /** @@ -38,6 +39,7 @@ public final class Fabric8ConfigUtils { private Fabric8ConfigUtils() { } + @Deprecated public static String getApplicationNamespace(KubernetesClient client, String namespace, String configurationTarget) { if (!StringUtils.hasLength(namespace)) { @@ -49,6 +51,26 @@ public final class Fabric8ConfigUtils { return namespace; } + static String getApplicationNamespace(KubernetesClient client, String namespace, String configurationTarget, + KubernetesNamespaceProvider provider) { + + if (StringUtils.hasText(namespace)) { + LOG.debug(configurationTarget + " namespace from normalized source : " + namespace); + return namespace; + } + + if (provider != null) { + String providerNamespace = provider.getNamespace(); + if (StringUtils.hasText(providerNamespace)) { + LOG.debug(configurationTarget + " namespace from provider : " + namespace); + } + } + + LOG.debug(configurationTarget + " namespace from client : " + client.getNamespace()); + return client.getNamespace(); + + } + public static String getApplicationNamespace(KubernetesClient client, String namespace) { return !StringUtils.hasLength(namespace) ? client.getNamespace() : namespace; } diff --git a/spring-cloud-kubernetes-fabric8-config/src/main/java/org/springframework/cloud/kubernetes/fabric8/config/Fabric8SecretsPropertySource.java b/spring-cloud-kubernetes-fabric8-config/src/main/java/org/springframework/cloud/kubernetes/fabric8/config/Fabric8SecretsPropertySource.java index 9ac051c9..32f1e570 100644 --- a/spring-cloud-kubernetes-fabric8-config/src/main/java/org/springframework/cloud/kubernetes/fabric8/config/Fabric8SecretsPropertySource.java +++ b/spring-cloud-kubernetes-fabric8-config/src/main/java/org/springframework/cloud/kubernetes/fabric8/config/Fabric8SecretsPropertySource.java @@ -60,12 +60,12 @@ public class Fabric8SecretsPropertySource extends SecretsPropertySource { } client.secrets().inNamespace(namespaceToUse).withLabels(labels).list().getItems() - .forEach(s -> putDataFromSecret(s, result, namespaceToUse)); + .forEach(s -> putDataFromSecret(s, result, namespaceToUse)); } catch (Exception e) { - LOG.warn("Can't read secret with name: [" + name + "] or labels [" + labels + "] in namespace: [" + namespaceToUse - + "] (cause: " + e.getMessage() + "). Ignoring"); + LOG.warn("Can't read secret with name: [" + name + "] or labels [" + labels + "] in namespace: [" + + namespaceToUse + "] (cause: " + e.getMessage() + "). Ignoring"); } return result; diff --git a/spring-cloud-kubernetes-fabric8-config/src/main/java/org/springframework/cloud/kubernetes/fabric8/config/Fabric8SecretsPropertySourceLocator.java b/spring-cloud-kubernetes-fabric8-config/src/main/java/org/springframework/cloud/kubernetes/fabric8/config/Fabric8SecretsPropertySourceLocator.java index d7f91984..20cab90f 100644 --- a/spring-cloud-kubernetes-fabric8-config/src/main/java/org/springframework/cloud/kubernetes/fabric8/config/Fabric8SecretsPropertySourceLocator.java +++ b/spring-cloud-kubernetes-fabric8-config/src/main/java/org/springframework/cloud/kubernetes/fabric8/config/Fabric8SecretsPropertySourceLocator.java @@ -16,9 +16,12 @@ package org.springframework.cloud.kubernetes.fabric8.config; +import java.util.Map; + import io.fabric8.kubernetes.client.KubernetesClient; import org.springframework.cloud.bootstrap.config.PropertySourceLocator; +import org.springframework.cloud.kubernetes.commons.KubernetesNamespaceProvider; import org.springframework.cloud.kubernetes.commons.config.SecretsConfigProperties; import org.springframework.cloud.kubernetes.commons.config.SecretsPropertySourceLocator; import org.springframework.core.annotation.Order; @@ -26,6 +29,7 @@ import org.springframework.core.env.ConfigurableEnvironment; import org.springframework.core.env.MapPropertySource; import static org.springframework.cloud.kubernetes.commons.config.ConfigUtils.getApplicationName; +import static org.springframework.cloud.kubernetes.fabric8.config.Fabric8ConfigUtils.getApplicationNamespace; /** * Kubernetes {@link PropertySourceLocator} for secrets. @@ -38,18 +42,37 @@ public class Fabric8SecretsPropertySourceLocator extends SecretsPropertySourceLo private final KubernetesClient client; + private final KubernetesNamespaceProvider provider; + + /** + * This constructor is deprecated. Its usage might cause unexpected behavior when + * looking for different properties. For example, in general, if a namespace is not + * provided, we might look it up via other means: different documented environment + * variables or from a kubernetes client itself. Using this constructor might not + * reflect that. + */ + @Deprecated public Fabric8SecretsPropertySourceLocator(KubernetesClient client, SecretsConfigProperties properties) { super(properties); this.client = client; + this.provider = null; + } + + public Fabric8SecretsPropertySourceLocator(KubernetesClient client, SecretsConfigProperties properties, + KubernetesNamespaceProvider provider) { + super(properties); + this.client = client; + this.provider = provider; } @Override protected MapPropertySource getPropertySource(ConfigurableEnvironment environment, SecretsConfigProperties.NormalizedSource normalizedSource, String configurationTarget) { - return new Fabric8SecretsPropertySource(this.client, - getApplicationName(environment, normalizedSource.getName(), configurationTarget), Fabric8ConfigUtils - .getApplicationNamespace(this.client, normalizedSource.getNamespace(), configurationTarget), - normalizedSource.getLabels()); + String secretName = getApplicationName(environment, normalizedSource.getName(), configurationTarget); + String secretNamespace = getApplicationNamespace(this.client, normalizedSource.getNamespace(), + configurationTarget, provider); + Map labels = normalizedSource.getLabels(); + return new Fabric8SecretsPropertySource(this.client, secretName, secretNamespace, labels); } }