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.
gocd · Jan 8, 2023 · c4a9338 · c4a9338
1 parent ab9acee
commit c4a9338
Show file tree

Hide file tree

Showing 4 changed files with 9 additions and 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
@@ -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
@@ -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
@@ -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
@@ -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;