fix(kubernetes): Fix credentials endpoint with unreachable cluster (#…

…4041)

* fix(kubernetes): Fix credentials endpoint with unreachable cluster

Calling the endpoint to list credentials currently calls getNamespaces
on KubernetesNamedAccountCredentials, which in turn calls
getDeclaredNamespaces on the account.

Because of the caching logic, this sometimes results in a live call (if
the current entry is expired) which can cause the entire endpoint to
hang. We should always serve this call out of the cache (and handle any
refreshes asynchronously) so that we never hang on this call. I think
a slight increase in stale reads here is prefereable to blocking the
endpoint synchronously getting live data.

The other place this call is used is when caching agents decide which
namespaces they are responsible for caching. I don't think this change
will lead to more stale reads because the caching agent runs at regular
intervals, so we'll be regularly reading the namespaces and thus
triggering async refreshes of the cached data. (Performance-wise it also
would probably be fine to just always read live in this case, but for
simplicity I'm having this call also go through the cache.)

I think ideally we would not have the credentials endpoint get the
namespaces in every account. My understanding is that this is primarily
used by deck to populate namespace dropdowns; we could probably instead
fetch the namespaces for a given account on-demand once the account
has been selected and remove the need to return them as part of the
credentials endpoint. With that change we could potentially completely
remove the caching here, but for now this should improve the stability
of clouddriver when an account is unreachable.

* refactor(kubernetes): Make namespaces immutable

The namespaces and omitNamespaces fields on KubernetesV2Credentials
never change; let's enforce that by making them immutable.

clouddriver-kubernetes-v2/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/v2/security/KubernetesV2Credentials.java

@@ -90,9 +90,9 @@ public class KubernetesV2Credentials implements KubernetesCredentials {
 
   @Include private final String accountName;
 
-  @Include @Getter private final List<String> namespaces;
+  @Include @Getter private final ImmutableList<String> namespaces;
 
-  @Include @Getter private final List<String> omitNamespaces;
+  @Include @Getter private final ImmutableList<String> omitNamespaces;
 
   @Include private final ImmutableSet<KubernetesKind> kinds;
 
@@ -161,8 +161,8 @@ private KubernetesV2Credentials(
                 .collect(toImmutableList()));
 
     this.accountName = managedAccount.getName();
-    this.namespaces = managedAccount.getNamespaces();
-    this.omitNamespaces = managedAccount.getOmitNamespaces();
+    this.namespaces = ImmutableList.copyOf(managedAccount.getNamespaces());
+    this.omitNamespaces = ImmutableList.copyOf(managedAccount.getOmitNamespaces());
     this.kinds =
         managedAccount.getKinds().stream()
             .map(KubernetesKind::fromString)
@@ -211,7 +211,7 @@ private static class Memoizer<T> implements Supplier<T> {
     private Memoizer(Supplier<T> supplier, long expirySeconds, TimeUnit timeUnit) {
       this.cache =
           Caffeine.newBuilder()
-              .expireAfterWrite(expirySeconds, timeUnit)
+              .refreshAfterWrite(expirySeconds, timeUnit)
               .build(key -> supplier.get());
     }
 

clouddriver-kubernetes-v2/src/test/groovy/com/netflix/spinnaker/clouddriver/kubernetes/v2/validator/KubernetesValidationUtilSpec.groovy

@@ -17,6 +17,7 @@
 
 package com.netflix.spinnaker.clouddriver.kubernetes.v2.validator
 
+import com.google.common.collect.ImmutableList
 import com.netflix.spinnaker.clouddriver.kubernetes.v2.description.manifest.KubernetesKind
 import com.netflix.spinnaker.clouddriver.kubernetes.v2.description.manifest.KubernetesManifest
 import com.netflix.spinnaker.clouddriver.kubernetes.v2.security.KubernetesV2Credentials
@@ -26,14 +27,16 @@ import org.springframework.validation.Errors
 import spock.lang.Specification
 import spock.lang.Unroll
 
+import javax.annotation.Nullable
+
 class KubernetesValidationUtilSpec extends Specification {
   @Unroll
   void "wiring of kind/namespace validation"() {
     given:
     Errors errors = Mock(Errors)
     String kubernetesAccount = "testAccount"
-    def namespaces = ["test-namespace"]
-    def omitNamespaces = ["omit-namespace"]
+    def namespaces = ImmutableList.of("test-namespace")
+    def omitNamespaces = ImmutableList.of("omit-namespace")
     def kind = KubernetesKind.DEPLOYMENT
     AccountCredentials accountCredentials = Mock(AccountCredentials)
     KubernetesV2Credentials credentials = Mock(KubernetesV2Credentials)
@@ -74,8 +77,8 @@ class KubernetesValidationUtilSpec extends Specification {
     def judgement = kubernetesValidationUtil.validateNamespace(testNamespace, credentials)
 
     then:
-    credentials.getOmitNamespaces() >> omitNamespaces
-    credentials.namespaces >> namespaces
+    credentials.getOmitNamespaces() >> toImmutableList(omitNamespaces)
+    credentials.namespaces >> toImmutableList(namespaces)
     judgement == allowedNamespace
 
     where:
@@ -93,4 +96,12 @@ class KubernetesValidationUtilSpec extends Specification {
     []                 | []                 | "unknown-namespace" || true
     []                 | ["omit-namespace"] | "unknown-namespace" || true
   }
+
+  @Nullable
+  private static <T> ImmutableList<T> toImmutableList(@Nullable Iterable<T> list) {
+    if (list == null) {
+      return null;
+    }
+    return ImmutableList.copyOf(list);
+  }
 }