fix: Kubernetes extension doesn't leak KubernetesClient resources

Signed-off-by: Marc Nuri <[email protected]>
quarkusio · Feb 1, 2023 · 9d5d197 · 9d5d197
1 parent 86a6040
commit 9d5d197
Show file tree

Hide file tree

Showing 8 changed files with 105 additions and 89 deletions.
diff --git a/...ent/src/main/java/io/quarkus/container/image/openshift/deployment/OpenshiftProcessor.java b/...ent/src/main/java/io/quarkus/container/image/openshift/deployment/OpenshiftProcessor.java
@@ -231,7 +231,7 @@ public void openshiftRequirementsNative(OpenshiftConfig openshiftConfig,
     public void openshiftBuildFromJar(OpenshiftConfig openshiftConfig,
             S2iConfig s2iConfig,
             ContainerImageConfig containerImageConfig,
-            KubernetesClientBuildItem kubernetesClient,
+            KubernetesClientBuildItem kubernetesClientSupplier,
             ContainerImageInfoBuildItem containerImage,
             ArchiveRootBuildItem archiveRoot, OutputTargetBuildItem out, PackageConfig packageConfig,
             List<GeneratedFileSystemResourceBuildItem> generatedResources,
@@ -263,26 +263,28 @@ public void openshiftBuildFromJar(OpenshiftConfig openshiftConfig,
             return;
         }
 
-        String namespace = Optional.ofNullable(kubernetesClient.getClient().getNamespace()).orElse("default");
-        LOG.info("Starting (in-cluster) container image build for jar using: " + config.buildStrategy + " on server: "
-                + kubernetesClient.getClient().getMasterUrl() + " in namespace:" + namespace + ".");
-        //The contextRoot is where inside the tarball we will add the jars. A null value means everything will be added under '/' while "target" means everything will be added under '/target'.
-        //For docker kind of builds where we use instructions like: `COPY target/*.jar /deployments` it using '/target' is a requirement.
-        //For s2i kind of builds where jars are expected directly in the '/' we have to use null.
-        String outputDirName = out.getOutputDirectory().getFileName().toString();
-        String contextRoot = getContextRoot(outputDirName, packageConfig.isFastJar(), config.buildStrategy);
-        if (packageConfig.isFastJar()) {
-            createContainerImage(kubernetesClient, openshiftYml.get(), config, contextRoot, jar.getPath().getParent(),
-                    jar.getPath().getParent());
-        } else if (jar.getLibraryDir() != null) { //When using uber-jar the libraryDir is going to be null, potentially causing NPE.
-            createContainerImage(kubernetesClient, openshiftYml.get(), config, contextRoot, jar.getPath().getParent(),
-                    jar.getPath(), jar.getLibraryDir());
-        } else {
-            createContainerImage(kubernetesClient, openshiftYml.get(), config, contextRoot, jar.getPath().getParent(),
-                    jar.getPath());
+        try (KubernetesClient kubernetesClient = kubernetesClientSupplier.getClient().get()) {
+            String namespace = Optional.ofNullable(kubernetesClient.getNamespace()).orElse("default");
+            LOG.info("Starting (in-cluster) container image build for jar using: " + config.buildStrategy + " on server: "
+                    + kubernetesClient.getMasterUrl() + " in namespace:" + namespace + ".");
+            //The contextRoot is where inside the tarball we will add the jars. A null value means everything will be added under '/' while "target" means everything will be added under '/target'.
+            //For docker kind of builds where we use instructions like: `COPY target/*.jar /deployments` it using '/target' is a requirement.
+            //For s2i kind of builds where jars are expected directly in the '/' we have to use null.
+            String outputDirName = out.getOutputDirectory().getFileName().toString();
+            String contextRoot = getContextRoot(outputDirName, packageConfig.isFastJar(), config.buildStrategy);
+            if (packageConfig.isFastJar()) {
+                createContainerImage(kubernetesClient, openshiftYml.get(), config, contextRoot, jar.getPath().getParent(),
+                        jar.getPath().getParent());
+            } else if (jar.getLibraryDir() != null) { //When using uber-jar the libraryDir is going to be null, potentially causing NPE.
+                createContainerImage(kubernetesClient, openshiftYml.get(), config, contextRoot, jar.getPath().getParent(),
+                        jar.getPath(), jar.getLibraryDir());
+            } else {
+                createContainerImage(kubernetesClient, openshiftYml.get(), config, contextRoot, jar.getPath().getParent(),
+                        jar.getPath());
+            }
+            artifactResultProducer.produce(new ArtifactResultBuildItem(null, "jar-container", Collections.emptyMap()));
+            containerImageBuilder.produce(new ContainerImageBuilderBuildItem(OPENSHIFT));
         }
-        artifactResultProducer.produce(new ArtifactResultBuildItem(null, "jar-container", Collections.emptyMap()));
-        containerImageBuilder.produce(new ContainerImageBuilderBuildItem(OPENSHIFT));
     }
 
     private String getContextRoot(String outputDirName, boolean isFastJar, BuildStrategy buildStrategy) {
@@ -298,7 +300,7 @@ private String getContextRoot(String outputDirName, boolean isFastJar, BuildStra
     @BuildStep(onlyIf = { IsNormalNotRemoteDev.class, OpenshiftBuild.class, NativeBuild.class })
     public void openshiftBuildFromNative(OpenshiftConfig openshiftConfig, S2iConfig s2iConfig,
             ContainerImageConfig containerImageConfig,
-            KubernetesClientBuildItem kubernetesClient,
+            KubernetesClientBuildItem kubernetesClientSupplier,
             ContainerImageInfoBuildItem containerImage,
             ArchiveRootBuildItem archiveRoot, OutputTargetBuildItem out, PackageConfig packageConfig,
             List<GeneratedFileSystemResourceBuildItem> generatedResources,
@@ -319,31 +321,33 @@ public void openshiftBuildFromNative(OpenshiftConfig openshiftConfig, S2iConfig
             return;
         }
 
-        String namespace = Optional.ofNullable(kubernetesClient.getClient().getNamespace()).orElse("default");
+        try (KubernetesClient kubernetesClient = kubernetesClientSupplier.getClient().get()) {
+            String namespace = Optional.ofNullable(kubernetesClient.getNamespace()).orElse("default");
 
-        LOG.info("Starting (in-cluster) container image build for jar using: " + config.buildStrategy + " on server: "
-                + kubernetesClient.getClient().getMasterUrl() + " in namespace:" + namespace + ".");
-        Optional<GeneratedFileSystemResourceBuildItem> openshiftYml = generatedResources
-                .stream()
-                .filter(r -> r.getName().endsWith("kubernetes" + File.separator + "openshift.yml"))
-                .findFirst();
+            LOG.info("Starting (in-cluster) container image build for jar using: " + config.buildStrategy + " on server: "
+                    + kubernetesClient.getMasterUrl() + " in namespace:" + namespace + ".");
+            Optional<GeneratedFileSystemResourceBuildItem> openshiftYml = generatedResources
+                    .stream()
+                    .filter(r -> r.getName().endsWith("kubernetes" + File.separator + "openshift.yml"))
+                    .findFirst();
 
-        if (openshiftYml.isEmpty()) {
-            LOG.warn(
-                    "No Openshift manifests were generated so no openshift build process will be taking place");
-            return;
+            if (openshiftYml.isEmpty()) {
+                LOG.warn(
+                        "No Openshift manifests were generated so no openshift build process will be taking place");
+                return;
+            }
+            //The contextRoot is where inside the tarball we will add the jars. A null value means everything will be added under '/' while "target" means everything will be added under '/target'.
+            //For docker kind of builds where we use instructions like: `COPY target/*.jar /deployments` it using '/target' is a requirement.
+            //For s2i kind of builds where jars are expected directly in the '/' we have to use null.
+            String contextRoot = config.buildStrategy == BuildStrategy.DOCKER ? "target" : null;
+            createContainerImage(kubernetesClient, openshiftYml.get(), config, contextRoot, out.getOutputDirectory(),
+                    nativeImage.getPath());
+            artifactResultProducer.produce(new ArtifactResultBuildItem(null, "native-container", Collections.emptyMap()));
+            containerImageBuilder.produce(new ContainerImageBuilderBuildItem(OPENSHIFT));
         }
-        //The contextRoot is where inside the tarball we will add the jars. A null value means everything will be added under '/' while "target" means everything will be added under '/target'.
-        //For docker kind of builds where we use instructions like: `COPY target/*.jar /deployments` it using '/target' is a requirement.
-        //For s2i kind of builds where jars are expected directly in the '/' we have to use null.
-        String contextRoot = config.buildStrategy == BuildStrategy.DOCKER ? "target" : null;
-        createContainerImage(kubernetesClient, openshiftYml.get(), config, contextRoot, out.getOutputDirectory(),
-                nativeImage.getPath());
-        artifactResultProducer.produce(new ArtifactResultBuildItem(null, "native-container", Collections.emptyMap()));
-        containerImageBuilder.produce(new ContainerImageBuilderBuildItem(OPENSHIFT));
     }
 
-    public static void createContainerImage(KubernetesClientBuildItem kubernetesClient,
+    public static void createContainerImage(KubernetesClient kubernetesClient,
             GeneratedFileSystemResourceBuildItem openshiftManifests,
             OpenshiftConfig openshiftConfig,
             String base,
@@ -360,7 +364,7 @@ public static void createContainerImage(KubernetesClientBuildItem kubernetesClie
             throw new RuntimeException("Error creating the openshift binary build archive.", e);
         }
 
-        Config config = kubernetesClient.getClient().getConfiguration();
+        Config config = kubernetesClient.getConfiguration();
         //Let's disable http2 as it causes issues with duplicate build triggers.
         config.setHttp2Disable(true);
         try (KubernetesClient client = Clients.fromConfig(config)) {

diff --git a/...-s2i/deployment/src/main/java/io/quarkus/container/image/s2i/deployment/S2iProcessor.java b/...-s2i/deployment/src/main/java/io/quarkus/container/image/s2i/deployment/S2iProcessor.java
@@ -171,7 +171,7 @@ public void s2iRequirementsNative(S2iConfig s2iConfig,
 
     @BuildStep(onlyIf = { IsNormalNotRemoteDev.class, S2iBuild.class }, onlyIfNot = NativeBuild.class)
     public void s2iBuildFromJar(S2iConfig s2iConfig, ContainerImageConfig containerImageConfig,
-            KubernetesClientBuildItem kubernetesClient,
+            KubernetesClientBuildItem kubernetesClientSupplier,
             ContainerImageInfoBuildItem containerImage,
             ArchiveRootBuildItem archiveRoot, OutputTargetBuildItem out, PackageConfig packageConfig,
             List<GeneratedFileSystemResourceBuildItem> generatedResources,
@@ -201,20 +201,21 @@ public void s2iBuildFromJar(S2iConfig s2iConfig, ContainerImageConfig containerI
                     "No Openshift manifests were generated so no s2i process will be taking place");
             return;
         }
-
-        String namespace = Optional.ofNullable(kubernetesClient.getClient().getNamespace()).orElse("default");
-        LOG.info("Performing s2i binary build with jar on server: " + kubernetesClient.getClient().getMasterUrl()
-                + " in namespace:" + namespace + ".");
-
-        createContainerImage(kubernetesClient, openshiftYml.get(), s2iConfig, out.getOutputDirectory(), jar.getPath(),
-                out.getOutputDirectory().resolve("lib"));
-        artifactResultProducer.produce(new ArtifactResultBuildItem(null, "jar-container", Collections.emptyMap()));
-        containerImageBuilder.produce(new ContainerImageBuilderBuildItem(S2I));
+        try (KubernetesClient kubernetesClient = kubernetesClientSupplier.getClient().get()) {
+            String namespace = Optional.ofNullable(kubernetesClient.getNamespace()).orElse("default");
+            LOG.info("Performing s2i binary build with jar on server: " + kubernetesClient.getMasterUrl()
+                    + " in namespace:" + namespace + ".");
+
+            createContainerImage(kubernetesClient, openshiftYml.get(), s2iConfig, out.getOutputDirectory(), jar.getPath(),
+                    out.getOutputDirectory().resolve("lib"));
+            artifactResultProducer.produce(new ArtifactResultBuildItem(null, "jar-container", Collections.emptyMap()));
+            containerImageBuilder.produce(new ContainerImageBuilderBuildItem(S2I));
+        }
     }
 
     @BuildStep(onlyIf = { IsNormalNotRemoteDev.class, S2iBuild.class, NativeBuild.class })
     public void s2iBuildFromNative(S2iConfig s2iConfig, ContainerImageConfig containerImageConfig,
-            KubernetesClientBuildItem kubernetesClient,
+            KubernetesClientBuildItem kubernetesClientSupplier,
             ContainerImageInfoBuildItem containerImage,
             ArchiveRootBuildItem archiveRoot, OutputTargetBuildItem out, PackageConfig packageConfig,
             List<GeneratedFileSystemResourceBuildItem> generatedResources,
@@ -233,28 +234,30 @@ public void s2iBuildFromNative(S2iConfig s2iConfig, ContainerImageConfig contain
             return;
         }
 
-        String namespace = Optional.ofNullable(kubernetesClient.getClient().getNamespace()).orElse("default");
-        LOG.info("Performing s2i binary build with native image on server: " + kubernetesClient.getClient().getMasterUrl()
-                + " in namespace:" + namespace + ".");
+        try (KubernetesClient kubernetesClient = kubernetesClientSupplier.getClient().get()) {
+            String namespace = Optional.ofNullable(kubernetesClient.getNamespace()).orElse("default");
+            LOG.info("Performing s2i binary build with native image on server: " + kubernetesClient.getMasterUrl()
+                    + " in namespace:" + namespace + ".");
 
-        Optional<GeneratedFileSystemResourceBuildItem> openshiftYml = generatedResources
-                .stream()
-                .filter(r -> r.getName().endsWith("kubernetes" + File.separator + "openshift.yml"))
-                .findFirst();
+            Optional<GeneratedFileSystemResourceBuildItem> openshiftYml = generatedResources
+                    .stream()
+                    .filter(r -> r.getName().endsWith("kubernetes" + File.separator + "openshift.yml"))
+                    .findFirst();
 
-        if (openshiftYml.isEmpty()) {
-            LOG.warn(
-                    "No Openshift manifests were generated so no s2i process will be taking place");
-            return;
-        }
+            if (openshiftYml.isEmpty()) {
+                LOG.warn(
+                        "No Openshift manifests were generated so no s2i process will be taking place");
+                return;
+            }
 
-        createContainerImage(kubernetesClient, openshiftYml.get(), s2iConfig, out.getOutputDirectory(),
-                nativeImage.getPath());
-        artifactResultProducer.produce(new ArtifactResultBuildItem(null, "native-container", Collections.emptyMap()));
-        containerImageBuilder.produce(new ContainerImageBuilderBuildItem(S2I));
+            createContainerImage(kubernetesClient, openshiftYml.get(), s2iConfig, out.getOutputDirectory(),
+                    nativeImage.getPath());
+            artifactResultProducer.produce(new ArtifactResultBuildItem(null, "native-container", Collections.emptyMap()));
+            containerImageBuilder.produce(new ContainerImageBuilderBuildItem(S2I));
+        }
     }
 
-    public static void createContainerImage(KubernetesClientBuildItem kubernetesClient,
+    public static void createContainerImage(KubernetesClient kubernetesClient,
             GeneratedFileSystemResourceBuildItem openshiftManifests,
             S2iConfig s2iConfig,
             Path output,
@@ -270,7 +273,7 @@ public static void createContainerImage(KubernetesClientBuildItem kubernetesClie
             throw new RuntimeException("Error creating the s2i binary build archive.", e);
         }
 
-        Config config = kubernetesClient.getClient().getConfiguration();
+        Config config = kubernetesClient.getConfiguration();
         //Let's disable http2 as it causes issues with duplicate build triggers.
         config.setHttp2Disable(true);
         try (KubernetesClient client = Clients.fromConfig(config)) {

diff --git a/...rnal/src/main/java/io/quarkus/kubernetes/client/deployment/KubernetesClientBuildStep.java b/...rnal/src/main/java/io/quarkus/kubernetes/client/deployment/KubernetesClientBuildStep.java
@@ -13,6 +13,6 @@ public class KubernetesClientBuildStep {
 
     @BuildStep
     public KubernetesClientBuildItem process(TlsConfig tlsConfig) {
-        return new KubernetesClientBuildItem(createClient(buildConfig, tlsConfig));
+        return new KubernetesClientBuildItem(createConfig(buildConfig, tlsConfig));
     }
 }
diff --git a/.../runtime/src/main/java/io/quarkus/kubernetes/client/runtime/QuarkusHttpClientFactory.java b/.../runtime/src/main/java/io/quarkus/kubernetes/client/runtime/QuarkusHttpClientFactory.java
@@ -1,12 +1,12 @@
 package io.quarkus.kubernetes.client.runtime;
 
+import javax.enterprise.inject.spi.CDI;
+
 import io.fabric8.kubernetes.client.Config;
 import io.fabric8.kubernetes.client.http.HttpClient;
 import io.fabric8.kubernetes.client.vertx.VertxHttpClientBuilder;
 import io.vertx.core.Vertx;
 
-import javax.enterprise.inject.spi.CDI;
-
 public class QuarkusHttpClientFactory implements io.fabric8.kubernetes.client.http.HttpClient.Factory {
 
     private Vertx vertx;

diff --git a/...-client/spi/src/main/java/io/quarkus/kubernetes/client/spi/KubernetesClientBuildItem.java b/...-client/spi/src/main/java/io/quarkus/kubernetes/client/spi/KubernetesClientBuildItem.java
@@ -1,18 +1,26 @@
 package io.quarkus.kubernetes.client.spi;
 
+import java.util.function.Supplier;
+
+import io.fabric8.kubernetes.client.Config;
 import io.fabric8.kubernetes.client.KubernetesClient;
+import io.fabric8.kubernetes.client.KubernetesClientBuilder;
 import io.quarkus.builder.item.SimpleBuildItem;
 
 public final class KubernetesClientBuildItem extends SimpleBuildItem {
 
-    private final KubernetesClient client;
+    private final Config config;
+
+    public KubernetesClientBuildItem(Config config) {
+        this.config = config;
+    }
 
-    public KubernetesClientBuildItem(KubernetesClient client) {
-        this.client = client;
+    public Supplier<KubernetesClient> getClient() {
+        return () -> new KubernetesClientBuilder().withConfig(config).build();
     }
 
-    public KubernetesClient getClient() {
-        return this.client;
+    public Config getConfig() {
+        return config;
     }
 
 }
diff --git a/...es/vanilla/deployment/src/main/java/io/quarkus/kubernetes/deployment/KnativeDeployer.java b/...es/vanilla/deployment/src/main/java/io/quarkus/kubernetes/deployment/KnativeDeployer.java
@@ -17,14 +17,14 @@ public class KnativeDeployer {
     @BuildStep
     public void checkEnvironment(Optional<SelectedKubernetesDeploymentTargetBuildItem> selectedDeploymentTarget,
             List<GeneratedKubernetesResourceBuildItem> resources,
-            KubernetesClientBuildItem client, BuildProducer<KubernetesDeploymentClusterBuildItem> deploymentCluster) {
+            KubernetesClientBuildItem clientSupplier, BuildProducer<KubernetesDeploymentClusterBuildItem> deploymentCluster) {
         selectedDeploymentTarget.ifPresent(target -> {
             if (!KubernetesDeploy.INSTANCE.checkSilently()) {
                 return;
             }
             if (target.getEntry().getName().equals(KNATIVE)) {
-                try (DefaultKnativeClient knativeClient = client.getClient().adapt(DefaultKnativeClient.class)) {
-                    if (knativeClient.isSupported()) {
+                try (DefaultKnativeClient client = clientSupplier.getClient().get().adapt(DefaultKnativeClient.class)) {
+                    if (client.isSupported()) {
                         deploymentCluster.produce(new KubernetesDeploymentClusterBuildItem(KNATIVE));
                     } else {
                         throw new IllegalStateException(