refactor(kubernetes): A few improvements to KubernetesKind (#3834)

* test(kubernetes): Add tests to KubernetesKind

Add some tests to KubernetesKind before doing some refactoring of
that class for performance reasons.

* refactor(kubernetes): A few improvements to KubernetesKind

In preparation for some more subtle changes, make a few improvements
to the KubernetesKind class:
* Make the public static fields also final
* Make it more clear what is nullable and what isn't
* Add equals and hash code methods that allow comparing to kinds via
.equals()

* refactor(kubernetes): Change from == to equals for KubernetesKind

Most comparisons of KubernetesKind use == instead of equals(), which
works because we currently keep a static pool of all KubernetesKinds
and never create duplicates.

The cost of keeping this static pool is much higher than the benefit,
so move to using .equals() instead of == to prepare for a future where
there can be two different KubernetesKind objects that are equal.

* fix(kubernetes): Use case-insensitive comparison

Kinds should be considered equal if their name is equal ignoring
case. This matches the current logic where getOrRegisterKind will
return a kind that matches ignoring case.

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

@@ -65,7 +65,7 @@ public Collection<KubernetesCachingAgent> buildAllCachingAgents(
                 propertyRegistry.values().stream()
                     .map(KubernetesResourceProperties::getHandler)
                     .filter(Objects::nonNull)
-                    .filter(h -> v2Credentials.isValidKind(h.kind()) || h.kind() == NONE)
+                    .filter(h -> v2Credentials.isValidKind(h.kind()) || h.kind().equals(NONE))
                     .map(
                         h ->
                             h.buildCachingAgent(

clouddriver-kubernetes/src/main/groovy/com/netflix/spinnaker/clouddriver/kubernetes/v2/caching/view/model/KubernetesV2SecurityGroup.java

@@ -88,7 +88,7 @@ public static KubernetesV2SecurityGroup fromCacheData(CacheData cd) {
     Set<Rule> inboundRules = new HashSet<>();
     Set<Rule> outboundRules = new HashSet<>();
 
-    if (manifest.getKind() != KubernetesKind.NETWORK_POLICY) {
+    if (!manifest.getKind().equals(KubernetesKind.NETWORK_POLICY)) {
       log.warn("Unknown security group kind " + manifest.getKind());
     } else {
       if (manifest.getApiVersion().equals(NETWORKING_K8S_IO_V1)

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

@@ -91,7 +91,7 @@ public KubernetesV2Manifest getManifest(
             : Collections.emptyList();
 
     List<KubernetesPodMetric.ContainerMetric> metrics = Collections.emptyList();
-    if (kind == KubernetesKind.POD && credentials.isMetrics()) {
+    if (kind.equals(KubernetesKind.POD) && credentials.isMetrics()) {
       metrics =
           credentials.topPod(namespace, parsedName.getRight()).stream()
               .map(KubernetesPodMetric::getContainerMetrics)

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

@@ -86,7 +86,7 @@ public Set<KubernetesKind> allKubernetesKinds() {
 
   public Map<String, String> kubernetesToSpinnakerKindStringMap() {
     return kubernetesToSpinnaker.entrySet().stream()
-        .filter(x -> x.getKey() != KubernetesKind.NONE)
+        .filter(x -> !x.getKey().equals(KubernetesKind.NONE))
         .collect(Collectors.toMap(x -> x.getKey().toString(), x -> x.getValue().toString()));
   }
 }

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

@@ -26,88 +26,92 @@
 import java.util.Optional;
 import java.util.function.Predicate;
 import java.util.stream.Collectors;
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+import lombok.EqualsAndHashCode;
 import lombok.Getter;
 import lombok.extern.slf4j.Slf4j;
 import org.apache.commons.lang3.StringUtils;
 
+@EqualsAndHashCode(onlyExplicitlyIncluded = true)
 @Slf4j
 public final class KubernetesKind {
   @Getter
   private static final List<KubernetesKind> values =
       Collections.synchronizedList(new ArrayList<>());
 
-  public static KubernetesKind API_SERVICE =
+  public static final KubernetesKind API_SERVICE =
       new KubernetesKind(
           "apiService", KubernetesApiGroup.APIREGISTRATION_K8S_IO, null, false, false);
-  public static KubernetesKind CLUSTER_ROLE =
+  public static final KubernetesKind CLUSTER_ROLE =
       new KubernetesKind(
           "clusterRole", KubernetesApiGroup.RBAC_AUTHORIZATION_K8S_IO, null, false, false);
-  public static KubernetesKind CLUSTER_ROLE_BINDING =
+  public static final KubernetesKind CLUSTER_ROLE_BINDING =
       new KubernetesKind(
           "clusterRoleBinding", KubernetesApiGroup.RBAC_AUTHORIZATION_K8S_IO, null, false, false);
-  public static KubernetesKind CONFIG_MAP =
+  public static final KubernetesKind CONFIG_MAP =
       new KubernetesKind("configMap", KubernetesApiGroup.CORE, "cm", true, false);
-  public static KubernetesKind CONTROLLER_REVISION =
+  public static final KubernetesKind CONTROLLER_REVISION =
       new KubernetesKind("controllerRevision", KubernetesApiGroup.APPS, null, true, false);
-  public static KubernetesKind CUSTOM_RESOURCE_DEFINITION =
+  public static final KubernetesKind CUSTOM_RESOURCE_DEFINITION =
       new KubernetesKind(
           "customResourceDefinition", KubernetesApiGroup.EXTENSIONS, "crd", false, false);
-  public static KubernetesKind CRON_JOB =
+  public static final KubernetesKind CRON_JOB =
       new KubernetesKind("cronJob", KubernetesApiGroup.BATCH, null, true, false);
-  public static KubernetesKind DAEMON_SET =
+  public static final KubernetesKind DAEMON_SET =
       new KubernetesKind("daemonSet", KubernetesApiGroup.APPS, "ds", true, true);
-  public static KubernetesKind DEPLOYMENT =
+  public static final KubernetesKind DEPLOYMENT =
       new KubernetesKind("deployment", KubernetesApiGroup.APPS, "deploy", true, true);
-  public static KubernetesKind EVENT =
+  public static final KubernetesKind EVENT =
       new KubernetesKind("event", KubernetesApiGroup.CORE, null, true, false);
-  public static KubernetesKind HORIZONTAL_POD_AUTOSCALER =
+  public static final KubernetesKind HORIZONTAL_POD_AUTOSCALER =
       new KubernetesKind(
           "horizontalpodautoscaler", KubernetesApiGroup.AUTOSCALING, "hpa", true, false);
-  public static KubernetesKind INGRESS =
+  public static final KubernetesKind INGRESS =
       new KubernetesKind("ingress", KubernetesApiGroup.EXTENSIONS, "ing", true, true);
-  public static KubernetesKind JOB =
+  public static final KubernetesKind JOB =
       new KubernetesKind("job", KubernetesApiGroup.BATCH, null, true, false);
-  public static KubernetesKind MUTATING_WEBHOOK_CONFIGURATION =
+  public static final KubernetesKind MUTATING_WEBHOOK_CONFIGURATION =
       new KubernetesKind(
           "mutatingWebhookConfiguration",
           KubernetesApiGroup.ADMISSIONREGISTRATION_K8S_IO,
           null,
           false,
           false);
-  public static KubernetesKind NAMESPACE =
+  public static final KubernetesKind NAMESPACE =
       new KubernetesKind("namespace", KubernetesApiGroup.CORE, "ns", false, false);
-  public static KubernetesKind NETWORK_POLICY =
+  public static final KubernetesKind NETWORK_POLICY =
       new KubernetesKind("networkPolicy", KubernetesApiGroup.EXTENSIONS, "netpol", true, true);
-  public static KubernetesKind PERSISTENT_VOLUME =
+  public static final KubernetesKind PERSISTENT_VOLUME =
       new KubernetesKind("persistentVolume", KubernetesApiGroup.CORE, "pv", false, false);
-  public static KubernetesKind PERSISTENT_VOLUME_CLAIM =
+  public static final KubernetesKind PERSISTENT_VOLUME_CLAIM =
       new KubernetesKind("persistentVolumeClaim", KubernetesApiGroup.CORE, "pvc", true, false);
-  public static KubernetesKind POD =
+  public static final KubernetesKind POD =
       new KubernetesKind("pod", KubernetesApiGroup.CORE, "po", true, false);
-  public static KubernetesKind POD_PRESET =
+  public static final KubernetesKind POD_PRESET =
       new KubernetesKind("podPreset", KubernetesApiGroup.SETTINGS_K8S_IO, null, true, false);
-  public static KubernetesKind POD_SECURITY_POLICY =
+  public static final KubernetesKind POD_SECURITY_POLICY =
       new KubernetesKind("podSecurityPolicy", KubernetesApiGroup.EXTENSIONS, null, false, false);
-  public static KubernetesKind POD_DISRUPTION_BUDGET =
+  public static final KubernetesKind POD_DISRUPTION_BUDGET =
       new KubernetesKind("podDisruptionBudget", KubernetesApiGroup.POLICY, null, true, false);
-  public static KubernetesKind REPLICA_SET =
+  public static final KubernetesKind REPLICA_SET =
       new KubernetesKind("replicaSet", KubernetesApiGroup.APPS, "rs", true, true);
-  public static KubernetesKind ROLE =
+  public static final KubernetesKind ROLE =
       new KubernetesKind("role", KubernetesApiGroup.RBAC_AUTHORIZATION_K8S_IO, null, true, false);
-  public static KubernetesKind ROLE_BINDING =
+  public static final KubernetesKind ROLE_BINDING =
       new KubernetesKind(
           "roleBinding", KubernetesApiGroup.RBAC_AUTHORIZATION_K8S_IO, null, true, false);
-  public static KubernetesKind SECRET =
+  public static final KubernetesKind SECRET =
       new KubernetesKind("secret", KubernetesApiGroup.CORE, null, true, false);
-  public static KubernetesKind SERVICE =
+  public static final KubernetesKind SERVICE =
       new KubernetesKind("service", KubernetesApiGroup.CORE, "svc", true, true);
-  public static KubernetesKind SERVICE_ACCOUNT =
+  public static final KubernetesKind SERVICE_ACCOUNT =
       new KubernetesKind("serviceAccount", KubernetesApiGroup.CORE, "sa", true, false);
-  public static KubernetesKind STATEFUL_SET =
+  public static final KubernetesKind STATEFUL_SET =
       new KubernetesKind("statefulSet", KubernetesApiGroup.APPS, null, true, true);
-  public static KubernetesKind STORAGE_CLASS =
+  public static final KubernetesKind STORAGE_CLASS =
       new KubernetesKind("storageClass", KubernetesApiGroup.STORAGE_K8S_IO, "sc", false, false);
-  public static KubernetesKind VALIDATING_WEBHOOK_CONFIGURATION =
+  public static final KubernetesKind VALIDATING_WEBHOOK_CONFIGURATION =
       new KubernetesKind(
           "validatingWebhookConfiguration",
           KubernetesApiGroup.ADMISSIONREGISTRATION_K8S_IO,
@@ -117,11 +121,15 @@ public final class KubernetesKind {
 
   // special kind that should never be assigned to a manifest, used only to represent objects whose
   // kind is not in spinnaker's registry
-  public static KubernetesKind NONE = new KubernetesKind("none", null, null, true, false);
+  public static final KubernetesKind NONE =
+      new KubernetesKind("none", KubernetesApiGroup.NONE, null, true, false);
 
-  private final String name;
-  private final KubernetesApiGroup apiGroup;
-  private final String alias;
+  @Nonnull private final String name;
+  @EqualsAndHashCode.Include @Nonnull private final String lcName;
+  @Nonnull private final KubernetesApiGroup apiGroup;
+  @EqualsAndHashCode.Include @Nullable private final KubernetesApiGroup customApiGroup;
+
+  @Nullable private final String alias;
   @Getter private final boolean isNamespaced;
   // generally reserved for workloads, can be read as "does this belong to a spinnaker cluster?"
   private final boolean hasClusterRelationship;
@@ -131,24 +139,30 @@ public final class KubernetesKind {
   @Getter private final boolean isRegistered;
 
   private KubernetesKind(
-      String name,
-      KubernetesApiGroup apiGroup,
-      String alias,
+      @Nonnull String name,
+      @Nonnull KubernetesApiGroup apiGroup,
+      @Nullable String alias,
       boolean isNamespaced,
       boolean hasClusterRelationship) {
     this(name, apiGroup, alias, isNamespaced, hasClusterRelationship, false, true);
   }
 
   private KubernetesKind(
-      String name,
-      KubernetesApiGroup apiGroup,
-      String alias,
+      @Nonnull String name,
+      @Nonnull KubernetesApiGroup apiGroup,
+      @Nullable String alias,
       boolean isNamespaced,
       boolean hasClusterRelationship,
       boolean isDynamic,
       boolean isRegistered) {
     this.name = name;
+    this.lcName = name.toLowerCase();
     this.apiGroup = apiGroup;
+    if (apiGroup.isNativeGroup()) {
+      this.customApiGroup = null;
+    } else {
+      this.customApiGroup = apiGroup;
+    }
     this.alias = alias;
     this.isNamespaced = isNamespaced;
     this.hasClusterRelationship = hasClusterRelationship;
@@ -164,18 +178,20 @@ public boolean hasClusterRelationship() {
   @Override
   @JsonValue
   public String toString() {
-    if (apiGroup == null || apiGroup.isNativeGroup()) {
+    if (apiGroup.isNativeGroup()) {
       return name;
     }
     return name + "." + apiGroup.toString();
   }
 
   @JsonCreator
-  public static KubernetesKind fromString(String name) {
+  @Nonnull
+  public static KubernetesKind fromString(@Nonnull String name) {
     return fromString(name, true);
   }
 
-  public static KubernetesKind fromString(String name, boolean namespaced) {
+  @Nonnull
+  public static KubernetesKind fromString(@Nonnull String name, boolean namespaced) {
     KubernetesApiGroup apiGroup;
     String kindName;
     String[] parts = StringUtils.split(name, ".", 2);
@@ -189,13 +205,14 @@ public static KubernetesKind fromString(String name, boolean namespaced) {
     return KubernetesKind.getOrRegisterKind(kindName, true, namespaced, apiGroup);
   }
 
+  @Nonnull
   public static KubernetesKind getOrRegisterKind(
-      final String name,
+      @Nonnull final String name,
       final boolean registered,
       final boolean namespaced,
-      final KubernetesApiGroup apiGroup) {
+      @Nullable final KubernetesApiGroup apiGroup) {
     if (StringUtils.isEmpty(name)) {
-      return null;
+      return KubernetesKind.NONE;
     }
 
     if (name.equalsIgnoreCase(KubernetesKind.NONE.toString())) {

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

@@ -25,6 +25,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
+import javax.annotation.Nonnull;
 import lombok.Data;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.commons.lang3.tuple.ImmutablePair;
@@ -48,6 +49,7 @@ private static <T> T getRequiredField(KubernetesManifest manifest, String field)
   }
 
   @JsonIgnore
+  @Nonnull
   public KubernetesKind getKind() {
     // using ApiVersion here allows a translation from a kind of NetworkPolicy in the manifest to
     // something

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

@@ -217,7 +217,7 @@ public static Moniker getMoniker(KubernetesManifest manifest) {
                 annotations,
                 SEQUENCE,
                 new TypeReference<Integer>() {},
-                manifest.getKind() == KubernetesKind.REPLICA_SET
+                manifest.getKind().equals(KubernetesKind.REPLICA_SET)
                     ? getAnnotation(
                         annotations,
                         DEPLOYMENT_REVISION,

clouddriver-kubernetes/src/main/groovy/com/netflix/spinnaker/clouddriver/kubernetes/v2/op/job/KubernetesRunJobOperation.java

@@ -68,7 +68,7 @@ public DeploymentResult operate(List _unused) {
     getTask().updateStatus(OP_NAME, "Running Kubernetes job...");
     KubernetesManifest jobSpec = this.description.getManifest();
     KubernetesKind kind = jobSpec.getKind();
-    if (kind != KubernetesKind.JOB) {
+    if (!kind.equals(KubernetesKind.JOB)) {
       throw new IllegalArgumentException(
           "Only kind of Job is accepted for the V2 Run Job operation.");
     }

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

@@ -263,7 +263,8 @@ private void applyTraffic(KubernetesManifestTraffic traffic, KubernetesManifest
 
   private void validateManifestsForRolloutStrategies(List<KubernetesManifest> manifests) {
     if (description.getStrategy() != null
-        && (manifests.size() != 1 || manifests.get(0).getKind() != KubernetesKind.REPLICA_SET)) {
+        && (manifests.size() != 1
+            || !manifests.get(0).getKind().equals(KubernetesKind.REPLICA_SET))) {
       throw new RuntimeException(
           "Spinnaker can manage traffic for ReplicaSets only. Please deploy exactly one ReplicaSet manifest or disable rollout strategies.");
     }

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

@@ -50,6 +50,7 @@
 import java.util.concurrent.TimeUnit;
 import java.util.function.Supplier;
 import java.util.stream.Collectors;
+import javax.annotation.Nonnull;
 import lombok.Getter;
 import lombok.extern.slf4j.Slf4j;
 import org.apache.commons.lang3.StringUtils;
@@ -229,12 +230,12 @@ public String getErrorMessage(KubernetesKind kind) {
     }
   }
 
-  public boolean isValidKind(KubernetesKind kind) {
+  public boolean isValidKind(@Nonnull KubernetesKind kind) {
     return getInvalidKindReason(kind) == null;
   }
 
-  public InvalidKindReason getInvalidKindReason(KubernetesKind kind) {
-    if (kind == KubernetesKind.NONE) {
+  public InvalidKindReason getInvalidKindReason(@Nonnull KubernetesKind kind) {
+    if (kind.equals(KubernetesKind.NONE)) {
       return InvalidKindReason.KIND_NONE;
     } else if (!this.kinds.isEmpty()) {
       return !kinds.contains(kind) ? InvalidKindReason.MISSING_FROM_ALLOWED_KINDS : null;
@@ -340,7 +341,7 @@ private void determineOmitKinds() {
     Map<KubernetesKind, InvalidKindReason> unreadableKinds =
         allKinds
             .parallelStream()
-            .filter(k -> k != KubernetesKind.NONE)
+            .filter(k -> !k.equals(KubernetesKind.NONE))
             .filter(k -> !omitKinds.keySet().contains(k))
             .filter(k -> !canReadKind(k, checkNamespace))
             .collect(Collectors.toConcurrentMap(k -> k, k -> InvalidKindReason.READ_ERROR));