From acb70d42e1f7ee7a282c52c1c3714bebca8b6050 Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Mon, 8 Feb 2021 15:24:39 +0000 Subject: [PATCH] Ensure KubernetesInformerFactoryProcessor does not access any beans A `BeanDefinitionRegistryPostProcessor` should not ideally refer to any bean instances, except lazily. This change makes the bean definitions that are installed by a `KubernetesInformer` lazier by putting them behind a `Supplier` Signed-off-by: Dave Syer --- .../KubernetesInformerConfigurer.java | 44 ------ .../KubernetesInformerFactoryProcessor.java | 148 ++++++++++-------- .../KubernetesInformerAutoConfiguration.java | 7 +- .../KubernetesInformerCreatorTest.java | 4 +- .../KubernetesReconcilerCreatorTest.java | 6 +- .../KubernetesReconcilerProcessorTest.java | 5 +- 6 files changed, 91 insertions(+), 123 deletions(-) delete mode 100644 spring/src/main/java/io/kubernetes/client/spring/extended/controller/KubernetesInformerConfigurer.java diff --git a/spring/src/main/java/io/kubernetes/client/spring/extended/controller/KubernetesInformerConfigurer.java b/spring/src/main/java/io/kubernetes/client/spring/extended/controller/KubernetesInformerConfigurer.java deleted file mode 100644 index 8c02658303..0000000000 --- a/spring/src/main/java/io/kubernetes/client/spring/extended/controller/KubernetesInformerConfigurer.java +++ /dev/null @@ -1,44 +0,0 @@ -/* -Copyright 2020 The Kubernetes 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 -http://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 io.kubernetes.client.spring.extended.controller; - -import io.kubernetes.client.informer.SharedInformerFactory; -import io.kubernetes.client.openapi.ApiClient; -import org.springframework.beans.factory.FactoryBean; - -/** - * KubernetesInformerConfigurer will register a {@link KubernetesInformerFactoryProcessor} into the - * context. - */ -public class KubernetesInformerConfigurer - implements FactoryBean { - - private final ApiClient apiClient; - private final SharedInformerFactory sharedInformerFactory; - - public KubernetesInformerConfigurer( - ApiClient apiClient, SharedInformerFactory sharedInformerFactory) { - this.apiClient = apiClient; - this.sharedInformerFactory = sharedInformerFactory; - } - - @Override - public KubernetesInformerFactoryProcessor getObject() throws Exception { - return new KubernetesInformerFactoryProcessor(apiClient, sharedInformerFactory); - } - - @Override - public Class getObjectType() { - return KubernetesInformerFactoryProcessor.class; - } -} diff --git a/spring/src/main/java/io/kubernetes/client/spring/extended/controller/KubernetesInformerFactoryProcessor.java b/spring/src/main/java/io/kubernetes/client/spring/extended/controller/KubernetesInformerFactoryProcessor.java index 9ea04955c8..49f5d3a2f7 100644 --- a/spring/src/main/java/io/kubernetes/client/spring/extended/controller/KubernetesInformerFactoryProcessor.java +++ b/spring/src/main/java/io/kubernetes/client/spring/extended/controller/KubernetesInformerFactoryProcessor.java @@ -12,6 +12,7 @@ */ package io.kubernetes.client.spring.extended.controller; +import io.kubernetes.client.common.KubernetesObject; import io.kubernetes.client.informer.SharedIndexInformer; import io.kubernetes.client.informer.SharedInformer; import io.kubernetes.client.informer.SharedInformerFactory; @@ -20,18 +21,16 @@ import io.kubernetes.client.spring.extended.controller.annotation.KubernetesInformer; import io.kubernetes.client.spring.extended.controller.annotation.KubernetesInformers; import io.kubernetes.client.util.generic.GenericKubernetesApi; -import java.time.Duration; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import org.springframework.beans.BeansException; -import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; -import org.springframework.beans.factory.support.AbstractBeanDefinition; +import org.springframework.beans.factory.support.BeanDefinitionBuilder; import org.springframework.beans.factory.support.BeanDefinitionRegistry; import org.springframework.beans.factory.support.BeanDefinitionRegistryPostProcessor; import org.springframework.beans.factory.support.RootBeanDefinition; import org.springframework.core.Ordered; import org.springframework.core.ResolvableType; +import org.springframework.core.annotation.AnnotatedElementUtils; /** * The type Kubernetes informer factory processor which basically does the following things: @@ -44,73 +43,14 @@ public class KubernetesInformerFactoryProcessor implements BeanDefinitionRegistryPostProcessor, Ordered { - private static final Logger log = - LoggerFactory.getLogger(KubernetesInformerFactoryProcessor.class); - public static final int ORDER = 0; - private BeanDefinitionRegistry beanDefinitionRegistry; - - private final ApiClient apiClient; - private final SharedInformerFactory sharedInformerFactory; - - @Autowired - public KubernetesInformerFactoryProcessor( - ApiClient apiClient, SharedInformerFactory sharedInformerFactory) { - this.apiClient = apiClient; - this.sharedInformerFactory = sharedInformerFactory; - } + private ConfigurableListableBeanFactory beanFactory; @Override public void postProcessBeanFactory(ConfigurableListableBeanFactory beanFactory) throws BeansException { - - this.apiClient.setHttpClient( - this.apiClient.getHttpClient().newBuilder().readTimeout(Duration.ZERO).build()); - - KubernetesInformers kubernetesInformers = - sharedInformerFactory.getClass().getAnnotation(KubernetesInformers.class); - if (kubernetesInformers == null || kubernetesInformers.value().length == 0) { - log.info("No informers registered in the sharedInformerFactory.."); - return; - } - for (KubernetesInformer kubernetesInformer : kubernetesInformers.value()) { - final GenericKubernetesApi api = - new GenericKubernetesApi( - kubernetesInformer.apiTypeClass(), - kubernetesInformer.apiListTypeClass(), - kubernetesInformer.groupVersionResource().apiGroup(), - kubernetesInformer.groupVersionResource().apiVersion(), - kubernetesInformer.groupVersionResource().resourcePlural(), - apiClient); - SharedIndexInformer sharedIndexInformer = - sharedInformerFactory.sharedIndexInformerFor( - api, - kubernetesInformer.apiTypeClass(), - kubernetesInformer.resyncPeriodMillis(), - kubernetesInformer.namespace()); - ResolvableType informerType = - ResolvableType.forClassWithGenerics( - SharedInformer.class, kubernetesInformer.apiTypeClass()); - RootBeanDefinition informerBean = new RootBeanDefinition(); - informerBean.setTargetType(informerType); - informerBean.setAutowireMode(AbstractBeanDefinition.AUTOWIRE_BY_TYPE); - informerBean.setAutowireCandidate(true); - String informerBeanName = informerType.toString(); - this.beanDefinitionRegistry.registerBeanDefinition(informerBeanName, informerBean); - beanFactory.registerSingleton(informerBeanName, sharedIndexInformer); - - Lister lister = new Lister(sharedIndexInformer.getIndexer()); - ResolvableType listerType = - ResolvableType.forClassWithGenerics(Lister.class, kubernetesInformer.apiTypeClass()); - RootBeanDefinition listerBean = new RootBeanDefinition(); - listerBean.setTargetType(listerType); - listerBean.setAutowireMode(AbstractBeanDefinition.AUTOWIRE_BY_TYPE); - listerBean.setAutowireCandidate(true); - String listerBeanName = listerType.toString(); - this.beanDefinitionRegistry.registerBeanDefinition(listerBeanName, listerBean); - beanFactory.registerSingleton(listerBeanName, lister); - } + this.beanFactory = beanFactory; } @Override @@ -121,6 +61,80 @@ public int getOrder() { @Override public void postProcessBeanDefinitionRegistry(BeanDefinitionRegistry registry) throws BeansException { - this.beanDefinitionRegistry = registry; + if (!(registry instanceof BeanFactory)) { + return; + } + for (String name : registry.getBeanDefinitionNames()) { + Class cls = ((BeanFactory) registry).getType(name); + if (cls != null) { + KubernetesInformers kubernetesInformers = + AnnotatedElementUtils.getMergedAnnotation(cls, KubernetesInformers.class); + if (kubernetesInformers != null && kubernetesInformers.value().length > 0) { + for (KubernetesInformer kubernetesInformer : kubernetesInformers.value()) { + registerInformer(registry, kubernetesInformer); + registerLister(registry, kubernetesInformer); + } + } + } + } + } + + private void registerInformer( + BeanDefinitionRegistry registry, KubernetesInformer kubernetesInformer) { + RootBeanDefinition informerBean = + (RootBeanDefinition) + BeanDefinitionBuilder.rootBeanDefinition(SharedInformer.class).getBeanDefinition(); + informerBean.setInstanceSupplier( + () -> informer(kubernetesInformer.apiTypeClass(), kubernetesInformer)); + ResolvableType informerType = + ResolvableType.forClassWithGenerics( + SharedIndexInformer.class, kubernetesInformer.apiTypeClass()); + informerBean.setTargetType(informerType); + registry.registerBeanDefinition( + getInformerBeanName(kubernetesInformer.apiTypeClass()), informerBean); + } + + private String getInformerBeanName(Class type) { + return type.getName() + "Informer"; + } + + private void registerLister( + BeanDefinitionRegistry registry, KubernetesInformer kubernetesInformer) { + RootBeanDefinition listerBean = + (RootBeanDefinition) + BeanDefinitionBuilder.rootBeanDefinition(Lister.class).getBeanDefinition(); + listerBean.setInstanceSupplier(() -> lister(kubernetesInformer.apiTypeClass())); + ResolvableType listerType = + ResolvableType.forClassWithGenerics(Lister.class, kubernetesInformer.apiTypeClass()); + listerBean.setTargetType(listerType); + registry.registerBeanDefinition(listerType.toString(), listerBean); + } + + @SuppressWarnings({"unchecked", "rawtypes"}) + private Lister lister(Class type) { + SharedIndexInformer sharedInformer = + this.beanFactory.getBean(getInformerBeanName(type), SharedIndexInformer.class); + Lister lister = new Lister<>(sharedInformer.getIndexer()); + return lister; + } + + @SuppressWarnings({"unchecked", "rawtypes"}) + private SharedInformer informer( + Class type, KubernetesInformer kubernetesInformer) { + ApiClient apiClient = this.beanFactory.getBean(ApiClient.class); + SharedInformerFactory sharedInformerFactory = + this.beanFactory.getBean(SharedInformerFactory.class); + final GenericKubernetesApi api = + new GenericKubernetesApi( + kubernetesInformer.apiTypeClass(), + kubernetesInformer.apiListTypeClass(), + kubernetesInformer.groupVersionResource().apiGroup(), + kubernetesInformer.groupVersionResource().apiVersion(), + kubernetesInformer.groupVersionResource().resourcePlural(), + apiClient); + SharedIndexInformer sharedIndexInformer = + sharedInformerFactory.sharedIndexInformerFor( + api, type, kubernetesInformer.resyncPeriodMillis(), kubernetesInformer.namespace()); + return sharedIndexInformer; } } diff --git a/spring/src/main/java/io/kubernetes/client/spring/extended/controller/config/KubernetesInformerAutoConfiguration.java b/spring/src/main/java/io/kubernetes/client/spring/extended/controller/config/KubernetesInformerAutoConfiguration.java index a4f3a1a47e..fe1aa384e6 100644 --- a/spring/src/main/java/io/kubernetes/client/spring/extended/controller/config/KubernetesInformerAutoConfiguration.java +++ b/spring/src/main/java/io/kubernetes/client/spring/extended/controller/config/KubernetesInformerAutoConfiguration.java @@ -14,7 +14,7 @@ import io.kubernetes.client.informer.SharedInformerFactory; import io.kubernetes.client.openapi.ApiClient; -import io.kubernetes.client.spring.extended.controller.KubernetesInformerConfigurer; +import io.kubernetes.client.spring.extended.controller.KubernetesInformerFactoryProcessor; import io.kubernetes.client.util.ClientBuilder; import java.io.IOException; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; @@ -39,8 +39,7 @@ public SharedInformerFactory sharedInformerFactory() { @Bean @ConditionalOnMissingBean - public KubernetesInformerConfigurer kubernetesInformerConfigurer( - ApiClient apiClient, SharedInformerFactory sharedInformerFactory) { - return new KubernetesInformerConfigurer(apiClient, sharedInformerFactory); + public static KubernetesInformerFactoryProcessor kubernetesInformerConfigurer() { + return new KubernetesInformerFactoryProcessor(); } } diff --git a/spring/src/test/java/io/kubernetes/client/spring/extended/controller/KubernetesInformerCreatorTest.java b/spring/src/test/java/io/kubernetes/client/spring/extended/controller/KubernetesInformerCreatorTest.java index 6febe349b8..a340477e30 100644 --- a/spring/src/test/java/io/kubernetes/client/spring/extended/controller/KubernetesInformerCreatorTest.java +++ b/spring/src/test/java/io/kubernetes/client/spring/extended/controller/KubernetesInformerCreatorTest.java @@ -66,7 +66,7 @@ public ApiClient testingApiClient() { } @Bean - public SharedInformerFactory sharedInformerFactory() { + public TestSharedInformerFactory testSharedInformerFactory() { return new TestSharedInformerFactory(); } @@ -86,7 +86,7 @@ public SharedInformerFactory sharedInformerFactory() { apiVersion = "v1", resourcePlural = "configmaps")), }) - static class TestSharedInformerFactory extends SharedInformerFactory {} + static class TestSharedInformerFactory {} } @Autowired private SharedInformerFactory informerFactory; diff --git a/spring/src/test/java/io/kubernetes/client/spring/extended/controller/KubernetesReconcilerCreatorTest.java b/spring/src/test/java/io/kubernetes/client/spring/extended/controller/KubernetesReconcilerCreatorTest.java index 36370b02ad..15bb631fbb 100644 --- a/spring/src/test/java/io/kubernetes/client/spring/extended/controller/KubernetesReconcilerCreatorTest.java +++ b/spring/src/test/java/io/kubernetes/client/spring/extended/controller/KubernetesReconcilerCreatorTest.java @@ -75,8 +75,8 @@ public ApiClient testingApiClient() { } @Bean - public SharedInformerFactory sharedInformerFactory() { - return new KubernetesInformerCreatorTest.App.TestSharedInformerFactory(); + public TestSharedInformerFactory testSharedInformerFactory() { + return new TestSharedInformerFactory(); } @KubernetesInformers({ @@ -94,7 +94,7 @@ public SharedInformerFactory sharedInformerFactory() { apiVersion = "v1", resourcePlural = "configmaps")), }) - static class TestSharedInformerFactory extends SharedInformerFactory {} + static class TestSharedInformerFactory {} @Bean public TestReconciler testReconciler() { diff --git a/spring/src/test/java/io/kubernetes/client/spring/extended/controller/KubernetesReconcilerProcessorTest.java b/spring/src/test/java/io/kubernetes/client/spring/extended/controller/KubernetesReconcilerProcessorTest.java index db3e4f7df3..578ac9bd8c 100644 --- a/spring/src/test/java/io/kubernetes/client/spring/extended/controller/KubernetesReconcilerProcessorTest.java +++ b/spring/src/test/java/io/kubernetes/client/spring/extended/controller/KubernetesReconcilerProcessorTest.java @@ -19,7 +19,6 @@ import io.kubernetes.client.extended.controller.reconciler.Request; import io.kubernetes.client.extended.controller.reconciler.Result; import io.kubernetes.client.informer.SharedInformer; -import io.kubernetes.client.informer.SharedInformerFactory; import io.kubernetes.client.openapi.models.V1Pod; import io.kubernetes.client.openapi.models.V1PodList; import io.kubernetes.client.spring.extended.controller.annotation.GroupVersionResource; @@ -49,7 +48,7 @@ KubernetesReconcilerProcessor reconcilerProcessorUnderTesting() { } @Bean - SharedInformerFactory sharedInformerFactory() { + TestSharedInformerFactory testSharedInformerFactory() { return new TestSharedInformerFactory(); } @@ -59,7 +58,7 @@ SharedInformerFactory sharedInformerFactory() { apiListTypeClass = V1PodList.class, groupVersionResource = @GroupVersionResource(resourcePlural = "pods")) }) - static class TestSharedInformerFactory extends SharedInformerFactory {} + static class TestSharedInformerFactory {} @Bean("testReconciler1") TestReconciler testReconciler1ToBeInjected() {