From c4a9338e13caa1c807703b411f36fc72bab67d65 Mon Sep 17 00:00:00 2001
From: Chad Wilson <chadw@thoughtworks.com>
Date: Sun, 8 Jan 2023 21:48:27 +0800
Subject: [PATCH] Avoid closing clients prematurely

Clients are cached/re-used so we should avoid closing them after each usage. Similarly, avoid
creating new clients unnecessarily where secrets sit within the same cluster.
---
 .../secrets/kubernetes/KubernetesClientFactory.java        | 5 ++---
 .../secrets/kubernetes/SecretConfigLookupExecutor.java     | 2 --
 .../go/contrib/secrets/kubernetes/models/SecretConfig.java | 7 +++++++
 .../secrets/kubernetes/validators/CredentialValidator.java | 2 --
 4 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/src/main/java/cd/go/contrib/secrets/kubernetes/KubernetesClientFactory.java b/src/main/java/cd/go/contrib/secrets/kubernetes/KubernetesClientFactory.java
index 17f9967..d5a2277 100644
--- a/src/main/java/cd/go/contrib/secrets/kubernetes/KubernetesClientFactory.java
+++ b/src/main/java/cd/go/contrib/secrets/kubernetes/KubernetesClientFactory.java
@@ -34,7 +34,7 @@ public static KubernetesClientFactory instance() {
     }
 
     public synchronized KubernetesClient client(SecretConfig secretConfig) {
-        if (secretConfig.equals(this.secretConfig) && this.client != null) {
+        if (secretConfig.hasSameTargetCluster(this.secretConfig) && this.client != null) {
             LOG.debug("Using previously created client.");
             return this.client;
         }
@@ -50,8 +50,7 @@ private KubernetesClient createClientFor(SecretConfig secretConfig) {
         final ConfigBuilder configBuilder = new ConfigBuilder()
                 .withOauthToken(secretConfig.getSecurityToken())
                 .withMasterUrl(secretConfig.getClusterUrl())
-                .withCaCertData(secretConfig.getClusterCACertData())
-                .withNamespace(secretConfig.getNamespace());
+                .withCaCertData(secretConfig.getClusterCACertData());
 
         return new KubernetesClientBuilder().withConfig(configBuilder.build()).build();
     }
diff --git a/src/main/java/cd/go/contrib/secrets/kubernetes/SecretConfigLookupExecutor.java b/src/main/java/cd/go/contrib/secrets/kubernetes/SecretConfigLookupExecutor.java
index fd8b1c7..57f400a 100644
--- a/src/main/java/cd/go/contrib/secrets/kubernetes/SecretConfigLookupExecutor.java
+++ b/src/main/java/cd/go/contrib/secrets/kubernetes/SecretConfigLookupExecutor.java
@@ -58,8 +58,6 @@ protected GoPluginApiResponse execute(SecretConfigRequest request) {
         } catch (Exception e) {
             LOG.error("Failed to lookup secret from Kubernetes Secret.", e);
             return DefaultGoPluginApiResponse.error(toJson(singletonMap("message", "Failed to lookup secrets from Kubernetes Secret. See logs for more information.")));
-        } finally {
-            client.close();
         }
     }
 
diff --git a/src/main/java/cd/go/contrib/secrets/kubernetes/models/SecretConfig.java b/src/main/java/cd/go/contrib/secrets/kubernetes/models/SecretConfig.java
index 2a24b31..8508c50 100644
--- a/src/main/java/cd/go/contrib/secrets/kubernetes/models/SecretConfig.java
+++ b/src/main/java/cd/go/contrib/secrets/kubernetes/models/SecretConfig.java
@@ -101,6 +101,13 @@ public boolean equals(Object o) {
                 Objects.equals(namespace, that.namespace);
     }
 
+    public boolean hasSameTargetCluster(SecretConfig that) {
+        if (this == that) return true;
+        return Objects.equals(clusterUrl, that.clusterUrl) &&
+                Objects.equals(securityToken, that.securityToken) &&
+                Objects.equals(clusterCACertData, that.clusterCACertData);
+    }
+
     @Override
     public int hashCode() {
         return Objects.hash(secretName, clusterUrl, securityToken, clusterCACertData, namespace);
diff --git a/src/main/java/cd/go/contrib/secrets/kubernetes/validators/CredentialValidator.java b/src/main/java/cd/go/contrib/secrets/kubernetes/validators/CredentialValidator.java
index 33b7b89..aa76931 100644
--- a/src/main/java/cd/go/contrib/secrets/kubernetes/validators/CredentialValidator.java
+++ b/src/main/java/cd/go/contrib/secrets/kubernetes/validators/CredentialValidator.java
@@ -36,8 +36,6 @@ public ValidationResult validate(Map<String, String> requestBody) {
             String errorMessage = "Could not read specified secret. Either the connection with kubernetes cluster could not be established or the kubernetes secret does not exists.";
             validationResult.add("kubernetes_secret_name", errorMessage);
             validationResult.add("kubernetes_cluster_url", errorMessage);
-        } finally {
-            client.close();
         }
 
         return validationResult;