From ec9ecb28185d56ed9a4b41e89fd574fa2e978739 Mon Sep 17 00:00:00 2001 From: erabii Date: Thu, 10 Nov 2022 15:42:53 +0200 Subject: [PATCH] Simplify Kubernetes native config data location resolver (#1129) --- .../config/KubernetesClientConfigContext.java | 2 +- ...netesClientConfigDataLocationResolver.java | 76 ++---- ...ClientConfigDataLocationResolverTests.java | 227 ++++++++++++++++++ .../KubernetesConfigDataLocationResolver.java | 5 - 4 files changed, 243 insertions(+), 67 deletions(-) create mode 100644 spring-cloud-kubernetes-client-config/src/test/java/org/springframework/cloud/kubernetes/client/config/KubernetesClientConfigDataLocationResolverTests.java diff --git a/spring-cloud-kubernetes-client-config/src/main/java/org/springframework/cloud/kubernetes/client/config/KubernetesClientConfigContext.java b/spring-cloud-kubernetes-client-config/src/main/java/org/springframework/cloud/kubernetes/client/config/KubernetesClientConfigContext.java index 6a7fa922..91a5556a 100644 --- a/spring-cloud-kubernetes-client-config/src/main/java/org/springframework/cloud/kubernetes/client/config/KubernetesClientConfigContext.java +++ b/spring-cloud-kubernetes-client-config/src/main/java/org/springframework/cloud/kubernetes/client/config/KubernetesClientConfigContext.java @@ -26,6 +26,6 @@ import org.springframework.core.env.Environment; * * @author wind57 */ -public final record KubernetesClientConfigContext(CoreV1Api client, NormalizedSource normalizedSource, String namespace, +public record KubernetesClientConfigContext(CoreV1Api client, NormalizedSource normalizedSource, String namespace, Environment environment) { } diff --git a/spring-cloud-kubernetes-client-config/src/main/java/org/springframework/cloud/kubernetes/client/config/KubernetesClientConfigDataLocationResolver.java b/spring-cloud-kubernetes-client-config/src/main/java/org/springframework/cloud/kubernetes/client/config/KubernetesClientConfigDataLocationResolver.java index cd22c50a..02a9ac4e 100644 --- a/spring-cloud-kubernetes-client-config/src/main/java/org/springframework/cloud/kubernetes/client/config/KubernetesClientConfigDataLocationResolver.java +++ b/spring-cloud-kubernetes-client-config/src/main/java/org/springframework/cloud/kubernetes/client/config/KubernetesClientConfigDataLocationResolver.java @@ -19,13 +19,11 @@ package org.springframework.cloud.kubernetes.client.config; import io.kubernetes.client.openapi.ApiClient; import io.kubernetes.client.openapi.apis.CoreV1Api; -import org.springframework.boot.BootstrapRegistry.InstanceSupplier; import org.springframework.boot.ConfigurableBootstrapContext; import org.springframework.boot.context.config.ConfigDataLocation; import org.springframework.boot.context.config.ConfigDataLocationResolverContext; import org.springframework.boot.context.config.Profiles; import org.springframework.boot.logging.DeferredLogFactory; -import org.springframework.cloud.kubernetes.client.KubernetesClientPodUtils; import org.springframework.cloud.kubernetes.commons.KubernetesClientProperties; import org.springframework.cloud.kubernetes.commons.KubernetesNamespaceProvider; import org.springframework.cloud.kubernetes.commons.config.ConfigDataRetryableConfigMapPropertySourceLocator; @@ -38,6 +36,7 @@ import org.springframework.cloud.kubernetes.commons.config.SecretsPropertySource import org.springframework.core.env.Environment; import static org.springframework.cloud.kubernetes.client.KubernetesClientUtils.kubernetesApiClient; +import static org.springframework.cloud.kubernetes.commons.config.ConfigUtils.registerSingle; /** * @author Ryan Baxter @@ -52,49 +51,13 @@ public class KubernetesClientConfigDataLocationResolver extends KubernetesConfig protected void registerBeans(ConfigDataLocationResolverContext resolverContext, ConfigDataLocation location, Profiles profiles, KubernetesConfigDataLocationResolver.PropertyHolder propertyHolder, KubernetesNamespaceProvider namespaceProvider) { + KubernetesClientProperties kubernetesClientProperties = propertyHolder.kubernetesClientProperties(); ConfigMapConfigProperties configMapProperties = propertyHolder.configMapConfigProperties(); SecretsConfigProperties secretsProperties = propertyHolder.secretsProperties(); ConfigurableBootstrapContext bootstrapContext = resolverContext.getBootstrapContext(); - ApiClient apiClient = kubernetesApiClient(); - bootstrapContext.registerIfAbsent(ApiClient.class, InstanceSupplier.of(apiClient)); - bootstrapContext.addCloseListener(event -> event.getApplicationContext().getBeanFactory() - .registerSingleton("configDataApiClient", event.getBootstrapContext().get(ApiClient.class))); + CoreV1Api coreV1Api = registerClientAndCoreV1Api(bootstrapContext, kubernetesClientProperties); - CoreV1Api coreV1Api = coreApi(apiClient); - bootstrapContext.registerIfAbsent(CoreV1Api.class, InstanceSupplier.of(coreV1Api)); - bootstrapContext.addCloseListener(event -> event.getApplicationContext().getBeanFactory() - .registerSingleton("configCoreV1Api", event.getBootstrapContext().get(CoreV1Api.class))); - - if (isRetryEnabled(configMapProperties, secretsProperties)) { - registerRetryBeans(configMapProperties, secretsProperties, bootstrapContext, coreV1Api, namespaceProvider); - } - else { - if (configMapProperties != null && configMapProperties.enabled()) { - KubernetesClientConfigMapPropertySourceLocator configMapPropertySourceLocator = new KubernetesClientConfigMapPropertySourceLocator( - coreV1Api, configMapProperties, namespaceProvider); - bootstrapContext.registerIfAbsent(ConfigMapPropertySourceLocator.class, - InstanceSupplier.of(configMapPropertySourceLocator)); - bootstrapContext.addCloseListener(event -> event.getApplicationContext().getBeanFactory() - .registerSingleton("configDataConfigMapPropertySourceLocator", - event.getBootstrapContext().get(ConfigMapPropertySourceLocator.class))); - } - - if (secretsProperties != null && secretsProperties.enabled()) { - KubernetesClientSecretsPropertySourceLocator secretsPropertySourceLocator = new KubernetesClientSecretsPropertySourceLocator( - coreV1Api, namespaceProvider, secretsProperties); - bootstrapContext.registerIfAbsent(SecretsPropertySourceLocator.class, - InstanceSupplier.of(secretsPropertySourceLocator)); - bootstrapContext.addCloseListener(event -> event.getApplicationContext().getBeanFactory() - .registerSingleton("configDataSecretsPropertySourceLocator", - event.getBootstrapContext().get(SecretsPropertySourceLocator.class))); - } - } - } - - private void registerRetryBeans(ConfigMapConfigProperties configMapProperties, - SecretsConfigProperties secretsProperties, ConfigurableBootstrapContext bootstrapContext, - CoreV1Api coreV1Api, KubernetesNamespaceProvider namespaceProvider) { if (configMapProperties != null && configMapProperties.enabled()) { ConfigMapPropertySourceLocator configMapPropertySourceLocator = new KubernetesClientConfigMapPropertySourceLocator( coreV1Api, configMapProperties, namespaceProvider); @@ -103,11 +66,8 @@ public class KubernetesClientConfigDataLocationResolver extends KubernetesConfig configMapPropertySourceLocator, configMapProperties); } - bootstrapContext.registerIfAbsent(ConfigMapPropertySourceLocator.class, - InstanceSupplier.of(configMapPropertySourceLocator)); - bootstrapContext.addCloseListener(event -> event.getApplicationContext().getBeanFactory().registerSingleton( - "configDataConfigMapPropertySourceLocator", - event.getBootstrapContext().get(ConfigMapPropertySourceLocator.class))); + registerSingle(bootstrapContext, ConfigMapPropertySourceLocator.class, configMapPropertySourceLocator, + "configDataConfigMapPropertySourceLocator"); } if (secretsProperties != null && secretsProperties.enabled()) { @@ -118,31 +78,25 @@ public class KubernetesClientConfigDataLocationResolver extends KubernetesConfig secretsPropertySourceLocator, secretsProperties); } - bootstrapContext.registerIfAbsent(SecretsPropertySourceLocator.class, - InstanceSupplier.of(secretsPropertySourceLocator)); - bootstrapContext.addCloseListener(event -> event.getApplicationContext().getBeanFactory().registerSingleton( - "configDataSecretsPropertySourceLocator", - event.getBootstrapContext().get(SecretsPropertySourceLocator.class))); + registerSingle(bootstrapContext, SecretsPropertySourceLocator.class, secretsPropertySourceLocator, + "configDataSecretsPropertySourceLocator"); } } - protected ApiClient apiClient(KubernetesClientProperties properties) { + private CoreV1Api registerClientAndCoreV1Api(ConfigurableBootstrapContext bootstrapContext, + KubernetesClientProperties kubernetesClientProperties) { ApiClient apiClient = kubernetesApiClient(); - apiClient.setUserAgent(properties.userAgent()); - return apiClient; - } + apiClient.setUserAgent(kubernetesClientProperties.userAgent()); + registerSingle(bootstrapContext, ApiClient.class, apiClient, "configDataApiClient"); - protected CoreV1Api coreApi(ApiClient apiClient) { - return new CoreV1Api(apiClient); + CoreV1Api coreV1Api = new CoreV1Api(apiClient); + registerSingle(bootstrapContext, CoreV1Api.class, coreV1Api, "configCoreV1Api"); + + return coreV1Api; } protected KubernetesNamespaceProvider kubernetesNamespaceProvider(Environment environment) { return new KubernetesNamespaceProvider(environment); } - protected KubernetesClientPodUtils kubernetesPodUtils(CoreV1Api client, - KubernetesNamespaceProvider kubernetesNamespaceProvider) { - return new KubernetesClientPodUtils(client, kubernetesNamespaceProvider.getNamespace()); - } - } diff --git a/spring-cloud-kubernetes-client-config/src/test/java/org/springframework/cloud/kubernetes/client/config/KubernetesClientConfigDataLocationResolverTests.java b/spring-cloud-kubernetes-client-config/src/test/java/org/springframework/cloud/kubernetes/client/config/KubernetesClientConfigDataLocationResolverTests.java new file mode 100644 index 00000000..3e234007 --- /dev/null +++ b/spring-cloud-kubernetes-client-config/src/test/java/org/springframework/cloud/kubernetes/client/config/KubernetesClientConfigDataLocationResolverTests.java @@ -0,0 +1,227 @@ +/* + * Copyright 2013-2022 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.client.config; + +import java.util.function.Supplier; + +import io.kubernetes.client.openapi.ApiClient; +import io.kubernetes.client.openapi.apis.CoreV1Api; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; + +import org.springframework.boot.DefaultBootstrapContext; +import org.springframework.boot.context.config.ConfigDataLocation; +import org.springframework.boot.context.config.ConfigDataLocationResolverContext; +import org.springframework.boot.context.config.Profiles; +import org.springframework.boot.context.properties.bind.Binder; +import org.springframework.boot.context.properties.source.ConfigurationPropertySources; +import org.springframework.boot.logging.DeferredLogFactory; +import org.springframework.cloud.kubernetes.commons.KubernetesClientProperties; +import org.springframework.cloud.kubernetes.commons.config.ConfigDataRetryableConfigMapPropertySourceLocator; +import org.springframework.cloud.kubernetes.commons.config.ConfigDataRetryableSecretsPropertySourceLocator; +import org.springframework.cloud.kubernetes.commons.config.ConfigMapConfigProperties; +import org.springframework.cloud.kubernetes.commons.config.ConfigMapPropertySourceLocator; +import org.springframework.cloud.kubernetes.commons.config.SecretsConfigProperties; +import org.springframework.cloud.kubernetes.commons.config.SecretsPropertySourceLocator; +import org.springframework.mock.env.MockEnvironment; + +/** + * @author wind57 + */ +class KubernetesClientConfigDataLocationResolverTests { + + private static final DeferredLogFactory FACTORY = Supplier::get; + + private static final ConfigDataLocationResolverContext RESOLVER_CONTEXT = Mockito + .mock(ConfigDataLocationResolverContext.class); + + private static final KubernetesClientConfigDataLocationResolver RESOLVER = new KubernetesClientConfigDataLocationResolver( + FACTORY); + + /* + * both ConfigMapConfigProperties and SecretsConfigProperties are null, thus they are + * not registered. It also means that ConfigMapPropertySourceLocator and + * SecretsPropertySourceLocator are not registered either. + */ + @Test + void testBothMissing() { + MockEnvironment environment = new MockEnvironment(); + environment.setProperty("spring.cloud.kubernetes.config.enabled", "false"); + environment.setProperty("spring.cloud.kubernetes.secrets.enabled", "false"); + ConfigurationPropertySources.attach(environment); + Binder binder = new Binder(ConfigurationPropertySources.get(environment)); + + DefaultBootstrapContext context = new DefaultBootstrapContext(); + Mockito.when(RESOLVER_CONTEXT.getBinder()).thenReturn(binder); + Mockito.when(RESOLVER_CONTEXT.getBootstrapContext()).thenReturn(context); + + Profiles profiles = Mockito.mock(Profiles.class); + ConfigDataLocation configDataLocation = ConfigDataLocation.of("kubernetes:abc"); + RESOLVER.resolveProfileSpecific(RESOLVER_CONTEXT, configDataLocation, profiles); + + Assertions.assertTrue(context.isRegistered(KubernetesClientProperties.class)); + Assertions.assertTrue(context.isRegistered(CoreV1Api.class)); + Assertions.assertTrue(context.isRegistered(ApiClient.class)); + + Assertions.assertFalse(context.isRegistered(ConfigMapConfigProperties.class)); + Assertions.assertFalse(context.isRegistered(SecretsConfigProperties.class)); + + Assertions.assertFalse(context.isRegistered(ConfigMapPropertySourceLocator.class)); + Assertions.assertFalse(context.isRegistered(SecretsPropertySourceLocator.class)); + } + + /* + * both ConfigMapConfigProperties and SecretsConfigProperties are enabled + * (via @Default on 'spring.cloud.kubernetes.config.enabled' and + * 'spring.cloud.kubernetes.secrets.enabled'); as such they are both registered. + * + * It also means that ConfigMapPropertySourceLocator and SecretsPropertySourceLocator + * are registered too. + * + * Since retry is not enabled explicitly, we also assert the types to ensure that + * these are not retryable beans. + */ + @Test + void testBothPresent() { + MockEnvironment environment = new MockEnvironment(); + ConfigurationPropertySources.attach(environment); + Binder binder = new Binder(ConfigurationPropertySources.get(environment)); + + DefaultBootstrapContext context = new DefaultBootstrapContext(); + Mockito.when(RESOLVER_CONTEXT.getBinder()).thenReturn(binder); + Mockito.when(RESOLVER_CONTEXT.getBootstrapContext()).thenReturn(context); + + Profiles profiles = Mockito.mock(Profiles.class); + ConfigDataLocation configDataLocation = ConfigDataLocation.of("kubernetes:abc"); + RESOLVER.resolveProfileSpecific(RESOLVER_CONTEXT, configDataLocation, profiles); + + Assertions.assertTrue(context.isRegistered(KubernetesClientProperties.class)); + Assertions.assertTrue(context.isRegistered(CoreV1Api.class)); + Assertions.assertTrue(context.isRegistered(ApiClient.class)); + + Assertions.assertTrue(context.isRegistered(ConfigMapConfigProperties.class)); + Assertions.assertTrue(context.isRegistered(SecretsConfigProperties.class)); + + Assertions.assertTrue(context.isRegistered(ConfigMapPropertySourceLocator.class)); + Assertions.assertTrue(context.isRegistered(SecretsPropertySourceLocator.class)); + + ConfigMapPropertySourceLocator configMapPropertySourceLocator = context + .get(ConfigMapPropertySourceLocator.class); + Assertions.assertSame(KubernetesClientConfigMapPropertySourceLocator.class, + configMapPropertySourceLocator.getClass()); + + SecretsPropertySourceLocator secretsPropertySourceLocator = context.get(SecretsPropertySourceLocator.class); + Assertions.assertSame(KubernetesClientSecretsPropertySourceLocator.class, + secretsPropertySourceLocator.getClass()); + + } + + /** + * both ConfigMapConfigProperties and SecretsConfigProperties are enabled explicitly, + * as such they are both registered. + * + * It also means that ConfigMapPropertySourceLocator and SecretsPropertySourceLocator + * are registered too. + * + * Since retry is not enabled explicitly, we also assert the types to ensure that + * these are not retryable beans. + */ + @Test + void testBothPresentExplicitly() { + MockEnvironment environment = new MockEnvironment(); + environment.setProperty("spring.cloud.kubernetes.config.enabled", "true"); + environment.setProperty("spring.cloud.kubernetes.secrets.enabled", "true"); + ConfigurationPropertySources.attach(environment); + Binder binder = new Binder(ConfigurationPropertySources.get(environment)); + + DefaultBootstrapContext context = new DefaultBootstrapContext(); + Mockito.when(RESOLVER_CONTEXT.getBinder()).thenReturn(binder); + Mockito.when(RESOLVER_CONTEXT.getBootstrapContext()).thenReturn(context); + + Profiles profiles = Mockito.mock(Profiles.class); + ConfigDataLocation configDataLocation = ConfigDataLocation.of("kubernetes:abc"); + RESOLVER.resolveProfileSpecific(RESOLVER_CONTEXT, configDataLocation, profiles); + + Assertions.assertTrue(context.isRegistered(KubernetesClientProperties.class)); + Assertions.assertTrue(context.isRegistered(CoreV1Api.class)); + Assertions.assertTrue(context.isRegistered(ApiClient.class)); + + Assertions.assertTrue(context.isRegistered(ConfigMapConfigProperties.class)); + Assertions.assertTrue(context.isRegistered(SecretsConfigProperties.class)); + + ConfigMapPropertySourceLocator configMapPropertySourceLocator = context + .get(ConfigMapPropertySourceLocator.class); + Assertions.assertSame(KubernetesClientConfigMapPropertySourceLocator.class, + configMapPropertySourceLocator.getClass()); + + SecretsPropertySourceLocator secretsPropertySourceLocator = context.get(SecretsPropertySourceLocator.class); + Assertions.assertSame(KubernetesClientSecretsPropertySourceLocator.class, + secretsPropertySourceLocator.getClass()); + } + + /* + * both ConfigMapConfigProperties and SecretsConfigProperties are enabled + * (via @Default on 'spring.cloud.kubernetes.config.enabled' and + * 'spring.cloud.kubernetes.secrets.enabled'); as such they are both registered. + * + * It also means that ConfigMapPropertySourceLocator and SecretsPropertySourceLocator + * are registered too. + * + * Since retry is enabled explicitly, we also assert the types to ensure that these + * are retryable beans. + */ + @Test + void testBothPresentAndRetryEnabled() { + MockEnvironment environment = new MockEnvironment(); + environment.setProperty("spring.cloud.kubernetes.config.retry.enabled", "true"); + environment.setProperty("spring.cloud.kubernetes.config.fail-fast", "true"); + environment.setProperty("spring.cloud.kubernetes.secrets.retry.enabled", "true"); + environment.setProperty("spring.cloud.kubernetes.secrets.fail-fast", "true"); + ConfigurationPropertySources.attach(environment); + Binder binder = new Binder(ConfigurationPropertySources.get(environment)); + + DefaultBootstrapContext context = new DefaultBootstrapContext(); + Mockito.when(RESOLVER_CONTEXT.getBinder()).thenReturn(binder); + Mockito.when(RESOLVER_CONTEXT.getBootstrapContext()).thenReturn(context); + + Profiles profiles = Mockito.mock(Profiles.class); + ConfigDataLocation configDataLocation = ConfigDataLocation.of("kubernetes:abc"); + RESOLVER.resolveProfileSpecific(RESOLVER_CONTEXT, configDataLocation, profiles); + + Assertions.assertTrue(context.isRegistered(KubernetesClientProperties.class)); + Assertions.assertTrue(context.isRegistered(CoreV1Api.class)); + Assertions.assertTrue(context.isRegistered(ApiClient.class)); + + Assertions.assertTrue(context.isRegistered(ConfigMapConfigProperties.class)); + Assertions.assertTrue(context.isRegistered(SecretsConfigProperties.class)); + + Assertions.assertTrue(context.isRegistered(ConfigMapPropertySourceLocator.class)); + Assertions.assertTrue(context.isRegistered(SecretsPropertySourceLocator.class)); + + ConfigMapPropertySourceLocator configMapPropertySourceLocator = context + .get(ConfigMapPropertySourceLocator.class); + Assertions.assertSame(ConfigDataRetryableConfigMapPropertySourceLocator.class, + configMapPropertySourceLocator.getClass()); + + SecretsPropertySourceLocator secretsPropertySourceLocator = context.get(SecretsPropertySourceLocator.class); + Assertions.assertSame(ConfigDataRetryableSecretsPropertySourceLocator.class, + secretsPropertySourceLocator.getClass()); + + } + +} diff --git a/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/config/KubernetesConfigDataLocationResolver.java b/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/config/KubernetesConfigDataLocationResolver.java index ce1f552f..9463b86b 100644 --- a/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/config/KubernetesConfigDataLocationResolver.java +++ b/spring-cloud-kubernetes-commons/src/main/java/org/springframework/cloud/kubernetes/commons/config/KubernetesConfigDataLocationResolver.java @@ -120,11 +120,6 @@ public abstract class KubernetesConfigDataLocationResolver ConfigDataLocation location, Profiles profiles, PropertyHolder propertyHolder, KubernetesNamespaceProvider namespaceProvider); - protected final boolean isRetryEnabled(ConfigMapConfigProperties configMapProperties, - SecretsConfigProperties secretsProperties) { - return isRetryEnabledForConfigMap(configMapProperties) || isRetryEnabledForSecrets(secretsProperties); - } - protected final boolean isRetryEnabledForConfigMap(ConfigMapConfigProperties configMapProperties) { return RETRY_IS_PRESENT && configMapProperties != null && configMapProperties.retry().enabled() && configMapProperties.failFast();