refactor(kubernetes): Look up CRD details on-demand (#4033)

* refactor(kubernetes): Remove useless call to register kind

On account creation, we iterate over all kinds in the account
and call registerKind. Since we only have the name of the kind
and no other information, we just assume the kind is namespaced
and pass true.

Since the kind registry will return a set of default properties
(including isNamespaced set to true) for something not in the
registry, skip the needless call to register these kinds with
default properties.

* refactor(kubernetes): Pass custom resources to registry constructor

Each account has a set of custom resources that are statically
defined in the account config. We register each of these into
the KubernetesKindRegistry, but currently do so in a somewhat
opaque way, by depending on a side-effect of the
KubernetesResourceProperties.fromCustomResource function we use
when creating a different property on the account.

Remove the side effect from the fromCustomResource function and
instead pass the list of custom resources to the constructor for
KubernetesKindRegistry, allowing it to register resources there.

* refactor(kubernetes): Use ObjectMapper to parse CRD

The current logic for reading CRD details is full of unsafe
casts and is hard to read. Define a class that has the fields
we expect on a CRD and use ObjectMapper to convert the manifest
back from the cluster into a typed object.

* refactor(kubernetes): Allow KubernetesKindRegistry to look up CRDs

To avoid needing to pre-register all CRDs into the kind registry, the
registry should be able to look up kind information in the cluster if
it comes across a kind that it does not have any information about.

When creating a KubernetesKindRegistry, the caller can provide a
Function<KubernetesKind, Optional<KubernetesKindProperties>>; when
looking up a kind that is not in the registry, the registry will
call that function to try to resolve the kind properties. If the
function returns a result, the properties are recorded in the registry
(so subsequent lookups of the same kind don't call the function again).
If the function returns an empty Optional, we return a set of default
properties as before.

This commit does not actually use the new functionality; this will
come in a later commit.

* refactor(kubernetes): Look up CRD details on-demand

Create a new function getCrdProperties that on-demand looks
up the properties of a CRD and pass this to the constructor
for the kind registry. This removes the need to pre-register
kinds that are fround when listing CRDs as the registry will
look up any kinds it needs, and also removes the last remaining
side-effect of calling liveCrdSupplier.

The only call to getCrds is from the unregistered CRD caching
agent. Given that this happens once per caching cycle (along
with a lot of other lookups) there's no real benefit to
memoizing the result with a timed expiration. Remove the
memoization and just have getCrds do a live lookup.

* refactor(kubernetes): Rename getRegisteredKind to getKindProperties

The term registered is confusing, and also somewhat redundant given
that the method is on a kind regsitry. (It's also not entirely correct
as the function can also live lookup properties and register them.)

Let's rename the function getKindProperties to be more in line with
what it does.

* fix(kubernetes): Remove extra extra the

Co-Authored-By: Maggie Neterval <mneterval@google.com>

* fix(kubernetes): Propertly handle null result from get and add log

Calling get() returns null when no resource is found rather than
throwing an exception. We should return Optional.empty() in that
case (rather than try to dereference the null in the following
logic) just as we do when we run into an exception.

Also add visibility by logging any exceptions in the exception block.

* fix(kubernetes): Fix live lookup of CRDs

The logic in an earlier commit to look up CRD properties was
incorrect as it was looking up CRDs via the command
kubectl get crd kind.apigroup; we need to use the *plural* version
of the kind in this command (as a CRD's name is based on the plural
kind).

There's unfortunately no good way to directly look up a CRD based
on the singular name. This commit works around it by just listing
all CRDs and filtering based on the name to the one we're interested
in. To avoid the performance implications of too many lookups of
the entire list of CRDs, also re-introduce memoizing.

* style(kubernetes): Collections.emptyList() -> ImmutableList.of()

clouddriver-kubernetes-v2/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/v2/caching/agent/KubernetesCacheDataConverter.java

@@ -208,7 +208,7 @@ public static void convertAsResource(
               + ":"
               + manifest.getFullResourceName());
     } else {
-      if (kindRegistry.getRegisteredKind(kind).hasClusterRelationship()) {
+      if (kindRegistry.getKindProperties(kind).hasClusterRelationship()) {
         addLogicalRelationships(kubernetesCacheData, key, account, moniker);
       }
     }
@@ -324,7 +324,7 @@ private static void logMalformedManifest(
     }
 
     if (StringUtils.isEmpty(manifest.getNamespace())
-        && kindRegistry.getRegisteredKind(manifest.getKind()).isNamespaced()) {
+        && kindRegistry.getKindProperties(manifest.getKind()).isNamespaced()) {
       log.warn("{}: manifest namespace may not be null, {}", contextMessage.get(), manifest);
     }
   }

clouddriver-kubernetes-v2/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/v2/caching/agent/KubernetesUnregisteredCustomResourceCachingAgent.java

@@ -54,7 +54,7 @@ public Collection<AgentDataType> getProvidedDataTypes() {
 
   @Override
   protected List<KubernetesKind> primaryKinds() {
-    return credentials.getCrds().stream()
+    return credentials.getCrds().keySet().stream()
         .filter(credentials::isValidKind)
         .collect(Collectors.toList());
   }

clouddriver-kubernetes-v2/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/v2/caching/agent/KubernetesV2OnDemandCachingAgent.java

@@ -302,7 +302,7 @@ public OnDemandAgent.OnDemandResult handle(ProviderCache providerCache, Map<Stri
     }
 
     if (StringUtils.isNotEmpty(namespace)
-        && !credentials.getKindRegistry().getRegisteredKind(kind).isNamespaced()) {
+        && !credentials.getKindRegistry().getKindProperties(kind).isNamespaced()) {
       log.warn(
           "{}: Kind {} is not namespace but namespace {} was provided, ignoring",
           getAgentType(),

clouddriver-kubernetes-v2/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/v2/caching/view/provider/KubernetesV2ManifestProvider.java

@@ -72,7 +72,7 @@ public KubernetesV2Manifest getManifest(
     }
 
     KubernetesKind kind = parsedName.getLeft();
-    if (!getKindRegistry(account).getRegisteredKind(kind).isNamespaced()
+    if (!getKindRegistry(account).getKindProperties(kind).isNamespaced()
         && StringUtils.isNotEmpty(location)) {
       log.warn(
           "Kind {} is not namespaced, but namespace {} was provided (ignoring)", kind, location);

clouddriver-kubernetes-v2/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/v2/description/KubernetesResourceProperties.java

@@ -24,8 +24,6 @@
 import com.netflix.spinnaker.clouddriver.kubernetes.v2.artifact.KubernetesUnversionedArtifactConverter;
 import com.netflix.spinnaker.clouddriver.kubernetes.v2.artifact.KubernetesVersionedArtifactConverter;
 import com.netflix.spinnaker.clouddriver.kubernetes.v2.description.manifest.KubernetesKind;
-import com.netflix.spinnaker.clouddriver.kubernetes.v2.description.manifest.KubernetesKindProperties;
-import com.netflix.spinnaker.clouddriver.kubernetes.v2.description.manifest.KubernetesKindRegistry;
 import com.netflix.spinnaker.clouddriver.kubernetes.v2.op.handler.CustomKubernetesHandlerFactory;
 import com.netflix.spinnaker.clouddriver.kubernetes.v2.op.handler.KubernetesHandler;
 import lombok.Getter;
@@ -48,7 +46,7 @@ public KubernetesResourceProperties(KubernetesHandler handler, boolean versioned
   }
 
   public static KubernetesResourceProperties fromCustomResource(
-      CustomKubernetesResource customResource, KubernetesKindRegistry kindRegistry) {
+      CustomKubernetesResource customResource) {
     String deployPriority = customResource.getDeployPriority();
     int deployPriorityValue;
     if (StringUtils.isEmpty(deployPriority)) {
@@ -62,20 +60,9 @@ public static KubernetesResourceProperties fromCustomResource(
       }
     }
 
-    KubernetesKind kubernetesKind = KubernetesKind.fromString(customResource.getKubernetesKind());
-    kindRegistry.getOrRegisterKind(
-        kubernetesKind,
-        () -> {
-          log.info(
-              "Dynamically registering {}, (namespaced: {})",
-              kubernetesKind.toString(),
-              customResource.isNamespaced());
-          return KubernetesKindProperties.create(kubernetesKind, customResource.isNamespaced());
-        });
-
     KubernetesHandler handler =
         CustomKubernetesHandlerFactory.create(
-            kubernetesKind,
+            KubernetesKind.fromString(customResource.getKubernetesKind()),
             SpinnakerKind.fromString(customResource.getSpinnakerKind()),
             customResource.isVersioned(),
             deployPriorityValue);

clouddriver-kubernetes-v2/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/v2/description/manifest/GlobalKubernetesKindRegistry.java

@@ -48,7 +48,7 @@ public GlobalKubernetesKindRegistry(
    * that were registered for the kind; otherwise, returns an empty {@link Optional}.
    */
   @Nonnull
-  public Optional<KubernetesKindProperties> getRegisteredKind(@Nonnull KubernetesKind kind) {
+  public Optional<KubernetesKindProperties> getKindProperties(@Nonnull KubernetesKind kind) {
     return Optional.ofNullable(nameMap.get(kind));
   }
 

clouddriver-kubernetes-v2/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/v2/description/manifest/KubernetesKind.java

@@ -19,15 +19,18 @@
 
 import com.fasterxml.jackson.annotation.JsonCreator;
 import com.fasterxml.jackson.annotation.JsonValue;
+import io.kubernetes.client.models.V1beta1CustomResourceDefinition;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
+import javax.annotation.ParametersAreNonnullByDefault;
 import lombok.EqualsAndHashCode;
 import lombok.Getter;
 import org.apache.commons.lang3.StringUtils;
 
 @EqualsAndHashCode(onlyExplicitlyIncluded = true)
+@ParametersAreNonnullByDefault
 public class KubernetesKind {
   private static final Map<KubernetesKind, KubernetesKind> aliasMap = new ConcurrentHashMap<>();
 
@@ -103,7 +106,7 @@ public class KubernetesKind {
   @Getter @Nonnull private final KubernetesApiGroup apiGroup;
   @EqualsAndHashCode.Include @Nullable private final KubernetesApiGroup customApiGroup;
 
-  private KubernetesKind(@Nonnull String name, @Nullable KubernetesApiGroup apiGroup) {
+  private KubernetesKind(String name, @Nullable KubernetesApiGroup apiGroup) {
     this.name = name;
     this.lcName = name.toLowerCase();
     this.apiGroup = apiGroup == null ? KubernetesApiGroup.NONE : apiGroup;
@@ -115,7 +118,7 @@ private KubernetesKind(@Nonnull String name, @Nullable KubernetesApiGroup apiGro
   }
 
   private static KubernetesKind createWithAlias(
-      @Nonnull String name, @Nullable String alias, @Nullable KubernetesApiGroup apiGroup) {
+      String name, @Nullable String alias, @Nullable KubernetesApiGroup apiGroup) {
     KubernetesKind kind = new KubernetesKind(name, apiGroup);
     aliasMap.put(kind, kind);
     if (alias != null) {
@@ -124,6 +127,7 @@ private static KubernetesKind createWithAlias(
     return kind;
   }
 
+  @Nonnull
   public static KubernetesKind from(@Nullable String name, @Nullable KubernetesApiGroup apiGroup) {
     if (name == null || name.isEmpty()) {
       return KubernetesKind.NONE;
@@ -132,9 +136,16 @@ public static KubernetesKind from(@Nullable String name, @Nullable KubernetesApi
     return aliasMap.getOrDefault(result, result);
   }
 
+  @Nonnull
+  public static KubernetesKind fromCustomResourceDefinition(V1beta1CustomResourceDefinition crd) {
+    return from(
+        crd.getSpec().getNames().getKind(),
+        KubernetesApiGroup.fromString(crd.getSpec().getGroup()));
+  }
+
   @Nonnull
   @JsonCreator
-  public static KubernetesKind fromString(@Nonnull String qualifiedKind) {
+  public static KubernetesKind fromString(String qualifiedKind) {
     KubernetesApiGroup apiGroup;
     String kindName;
     String[] parts = StringUtils.split(qualifiedKind, ".", 2);

clouddriver-kubernetes-v2/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/v2/description/manifest/KubernetesKindProperties.java

@@ -17,6 +17,7 @@
 package com.netflix.spinnaker.clouddriver.kubernetes.v2.description.manifest;
 
 import com.google.common.collect.ImmutableList;
+import io.kubernetes.client.models.V1beta1CustomResourceDefinition;
 import java.util.List;
 import javax.annotation.Nonnull;
 import javax.annotation.ParametersAreNonnullByDefault;
@@ -76,6 +77,7 @@ private KubernetesKindProperties(
     this.hasClusterRelationship = hasClusterRelationship;
   }
 
+  @Nonnull
   public static KubernetesKindProperties withDefaultProperties(KubernetesKind kubernetesKind) {
     return new KubernetesKindProperties(kubernetesKind, true, false);
   }
@@ -86,6 +88,14 @@ public static KubernetesKindProperties create(
     return new KubernetesKindProperties(kubernetesKind, isNamespaced, false);
   }
 
+  @Nonnull
+  public static KubernetesKindProperties fromCustomResourceDefinition(
+      V1beta1CustomResourceDefinition crd) {
+    return create(
+        KubernetesKind.fromCustomResourceDefinition(crd),
+        crd.getSpec().getScope().equalsIgnoreCase("namespaced"));
+  }
+
   public boolean hasClusterRelationship() {
     return this.hasClusterRelationship;
   }

clouddriver-kubernetes-v2/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/v2/description/manifest/KubernetesKindRegistry.java

@@ -17,55 +17,73 @@
 package com.netflix.spinnaker.clouddriver.kubernetes.v2.description.manifest;
 
 import com.google.common.collect.ImmutableCollection;
+import com.google.common.collect.ImmutableList;
+import java.util.Collection;
 import java.util.Map;
+import java.util.Optional;
 import java.util.concurrent.ConcurrentHashMap;
-import java.util.function.Supplier;
+import java.util.function.Function;
 import javax.annotation.Nonnull;
+import javax.annotation.ParametersAreNonnullByDefault;
+import lombok.AccessLevel;
+import lombok.RequiredArgsConstructor;
+import lombok.extern.slf4j.Slf4j;
 import org.springframework.stereotype.Component;
 
+@ParametersAreNonnullByDefault
+@RequiredArgsConstructor(access = AccessLevel.PRIVATE)
+@Slf4j
 public class KubernetesKindRegistry {
   private final Map<KubernetesKind, KubernetesKindProperties> kindMap = new ConcurrentHashMap<>();
   private final GlobalKubernetesKindRegistry globalKindRegistry;
+  private final Function<KubernetesKind, Optional<KubernetesKindProperties>> crdLookup;
 
-  private KubernetesKindRegistry(GlobalKubernetesKindRegistry globalKindRegistry) {
+  private KubernetesKindRegistry(
+      GlobalKubernetesKindRegistry globalKindRegistry,
+      Function<KubernetesKind, Optional<KubernetesKindProperties>> crdLookup,
+      Collection<KubernetesKindProperties> customProperties) {
     this.globalKindRegistry = globalKindRegistry;
+    this.crdLookup = crdLookup;
+    customProperties.forEach(this::registerKind);
   }
 
   /** Registers a given {@link KubernetesKindProperties} into the registry */
-  public void registerKind(@Nonnull KubernetesKindProperties kind) {
-    kindMap.put(kind.getKubernetesKind(), kind);
+  private KubernetesKindProperties registerKind(KubernetesKindProperties kindProperties) {
+    return kindMap.computeIfAbsent(
+        kindProperties.getKubernetesKind(),
+        k -> {
+          log.info(
+              "Dynamically registering {}, (namespaced: {})",
+              kindProperties.getKubernetesKind().toString(),
+              kindProperties.isNamespaced());
+          return kindProperties;
+        });
   }
 
   /**
    * Searches the registry for a {@link KubernetesKindProperties} with the supplied {@link
    * KubernetesKind}. If the kind has been registered, returns the {@link KubernetesKindProperties}
-   * that were registered for the kind; otherwise, calls the provided {@link Supplier} and registers
-   * the resulting {@link KubernetesKindProperties}.
+   * that were registered for the kind. If the kind is not registered, tries to look up the
+   * properties using the registry's CRD lookup function. If the lookup returns properties,
+   * registers them for this kind and returns them; otherwise returns a {@link
+   * KubernetesKindProperties} with default properties.
    */
   @Nonnull
-  public KubernetesKindProperties getOrRegisterKind(
-      @Nonnull KubernetesKind kind, @Nonnull Supplier<KubernetesKindProperties> supplier) {
-    return kindMap.computeIfAbsent(kind, k -> supplier.get());
-  }
+  public KubernetesKindProperties getKindProperties(KubernetesKind kind) {
+    Optional<KubernetesKindProperties> globalResult = globalKindRegistry.getKindProperties(kind);
+    if (globalResult.isPresent()) {
+      return globalResult.get();
+    }
 
-  /**
-   * Searches the registry for a {@link KubernetesKindProperties} with the supplied {@link
-   * KubernetesKind}. If the kind has been registered, returns the {@link KubernetesKindProperties}
-   * that were registered for the kind; otherwise, looks for the kind in the {@link
-   * GlobalKubernetesKindRegistry} and returns the properties found there. If the kind is not
-   * registered either globally or in the account, returns a {@link KubernetesKindProperties} with
-   * default properties.
-   */
-  @Nonnull
-  public KubernetesKindProperties getRegisteredKind(@Nonnull KubernetesKind kind) {
     KubernetesKindProperties result = kindMap.get(kind);
     if (result != null) {
       return result;
     }
 
-    return globalKindRegistry
-        .getRegisteredKind(kind)
-        .orElse(KubernetesKindProperties.withDefaultProperties(kind));
+    return crdLookup
+        .apply(kind)
+        .map(this::registerKind)
+        .orElseGet(() -> KubernetesKindProperties.withDefaultProperties(kind));
   }
 
   /** Returns a list of all global kinds */
@@ -82,9 +100,17 @@ public Factory(GlobalKubernetesKindRegistry globalKindRegistry) {
       this.globalKindRegistry = globalKindRegistry;
     }
 
+    @Nonnull
+    public KubernetesKindRegistry create(
+        Function<KubernetesKind, Optional<KubernetesKindProperties>> crdLookup,
+        Collection<KubernetesKindProperties> customProperties) {
+      return new KubernetesKindRegistry(globalKindRegistry, crdLookup, customProperties);
+    }
+
     @Nonnull
     public KubernetesKindRegistry create() {
-      return new KubernetesKindRegistry(globalKindRegistry);
+      return new KubernetesKindRegistry(
+          globalKindRegistry, k -> Optional.empty(), ImmutableList.of());
     }
   }
 }

clouddriver-kubernetes-v2/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/v2/op/manifest/KubernetesDeployManifestOperation.java

@@ -100,7 +100,7 @@ public OperationResult operate(List _unused) {
     validateManifestsForRolloutStrategies(inputManifests);
 
     for (KubernetesManifest manifest : inputManifests) {
-      if (credentials.getKindRegistry().getRegisteredKind(manifest.getKind()).isNamespaced()) {
+      if (credentials.getKindRegistry().getKindProperties(manifest.getKind()).isNamespaced()) {
         if (!StringUtils.isEmpty(description.getNamespaceOverride())) {
           manifest.setNamespace(description.getNamespaceOverride());
         } else if (StringUtils.isEmpty(manifest.getNamespace())) {