Plugin likely leaks client connections when using multiple secrets across clusters

[chadlwilson]

The current code caches a kubernetes client. Since each secret configured has to configure the cluster details individually it is clearly intended that you have a single Secret and lots of entries within it.

In any case, when the code detects a new client is needed it does not close the old client gracefully.

gocd-kubernetes-based-secrets-plugin/src/main/java/cd/go/contrib/secrets/kubernetes/KubernetesClientFactory.java

Lines 36 to 47 in c4a9338

	public synchronized KubernetesClient client(SecretConfig secretConfig) {
	if (secretConfig.hasSameTargetCluster(this.secretConfig) && this.client != null) {
	LOG.debug("Using previously created client.");
	return this.client;
	}
	LOG.debug(format("Creating a new client because {0}.", (client == null) ? "client is null" : "secret configuration has changed"));
	this.secretConfig = secretConfig;
	this.client = createClientFor(secretConfig);
	LOG.debug("New client is created.");
	return this.client;
	}

Doing so in the client() method before recreating (which sounds easy) would possibly lead to race conditions since other threads may have just been given the previously cached client and trying to use it.

The change in c4a9338 avoids creating new clients when unnecessary, so this should only happen when pointing it to other clusters or in more obscure situations. Additionally, perhaps okhttp does something to clean up and close idle connections anyway. Not sure - raising it here in case it is confirmed as an issue.