perf(kubernetes): Improve performance of cache kind lookup (#3741)

* test(kubernetes): Add tests to Keys

An upcoming commit will make some performance improvements to the
Keys.Kind and Keys.LogicalKind enums, so add some tests to these.

Also add some tests to validate the parsing of cache keys containing
semicolons. This is currently broken, but add the test now validating
the incorrect behavior and update the test in the commit that fixes it.

* fix(kubernetes): Properly substitute characters in key parts

As kubernetes objects can contain the character ':' which conflicts with
our default cache key separator, the caching logic replaces ':' with ';'
before generating a cache key and then inverts this by replacing ';'
with ':' when splitting a cache key into its component parts.

The current logic that runs when reading cache keys is an expensive
no-op; it substitutes ';' with ':' in the key part but ignores the
results (as the result of replaceAll is ignored).

In addition to fixing the bug, add a performance optimization. replaceAll
always allocates a new string, regardless of whether it actually
needs to perform a substitution; this means that we're allocating
a new string for every cache key part we read from the cache (~6x the
number of keys we read) even though most keys will not have a ':'.
Perform a relatively cheap contains check before doing the replacement.

* perf(kubernetes): Improve performance of cache kind lookup

Looking up a kind and a logicalKind by name (which happens frequently
on reads from the cache) are fairly inefficient.  We use a stream
to iterate over all current enum values comparing the input string to
calling toString on the enum using a case-insensitive comparison.

toString is unnecessarily expensive as it allocates a new string on
each call via calling .toLowerCase.  Instead, store the lower-case
name as a property of the enum and return a reference to it in toString.

Improve fromString by just converting the input string to upper case
and using the enum's valueOf function, rather than looping over
the values and doing a case-insensitive search.

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

@@ -44,32 +44,44 @@ public enum Kind {
     INFRASTRUCTURE,
     KUBERNETES_METRIC;
 
+    private final String lcName;
+
+    Kind() {
+      this.lcName = name().toLowerCase();
+    }
+
     @Override
     public String toString() {
-      return name().toLowerCase();
+      return lcName;
     }
 
     @JsonCreator
     public static Kind fromString(String name) {
-      return Arrays.stream(values())
-          .filter(k -> k.toString().equalsIgnoreCase(name))
-          .findFirst()
-          .orElseThrow(
-              () -> new IllegalArgumentException("No matching kind with name " + name + " exists"));
+      try {
+        return valueOf(name.toUpperCase());
+      } catch (IllegalArgumentException e) {
+        throw new IllegalArgumentException("No matching kind with name " + name + " exists");
+      }
     }
   }
 
   public enum LogicalKind {
     APPLICATIONS,
     CLUSTERS;
 
+    private final String lcName;
+
+    LogicalKind() {
+      this.lcName = name().toLowerCase();
+    }
+
     public static boolean isLogicalGroup(String group) {
       return group.equals(APPLICATIONS.toString()) || group.equals(CLUSTERS.toString());
     }
 
     @Override
     public String toString() {
-      return name().toLowerCase();
+      return lcName;
     }
 
     public String singular() {
@@ -79,11 +91,11 @@ public String singular() {
 
     @JsonCreator
     public static LogicalKind fromString(String name) {
-      return Arrays.stream(values())
-          .filter(k -> k.toString().equalsIgnoreCase(name))
-          .findFirst()
-          .orElseThrow(
-              () -> new IllegalArgumentException("No matching kind with name " + name + " exists"));
+      try {
+        return valueOf(name.toUpperCase());
+      } catch (IllegalArgumentException e) {
+        throw new IllegalArgumentException("No matching kind with name " + name + " exists");
+      }
     }
   }
 
@@ -131,8 +143,10 @@ public static Optional<CacheKey> parseKey(String key) {
       return Optional.empty();
     }
 
-    for (String part : parts) {
-      part.replaceAll(";", ":");
+    for (int i = 0; i < parts.length; i++) {
+      if (parts[i].contains(";")) {
+        parts[i] = parts[i].replaceAll(";", ":");
+      }
     }
 
     try {

clouddriver-kubernetes/src/test/groovy/com/netflix/spinnaker/clouddriver/kubernetes/v2/caching/KeysSpec.groovy

@@ -121,4 +121,74 @@ class KeysSpec extends Specification {
     KubernetesKind.SERVICE     | KubernetesApiVersion.V1                 | "account" | "namespace" | ""
     KubernetesKind.INGRESS     | KubernetesApiVersion.EXTENSIONS_V1BETA1 | "ac"      | ""          | "nameer"
   }
+
+  def "correctly unpacks resource names containing a ';' character"() {
+    when:
+    def key = "kubernetes.v2:infrastructure:clusterRole:k8s::system;controller;resourcequota-controller"
+    def parsed = Keys.parseKey(key).get()
+
+    then:
+    parsed instanceof Keys.InfrastructureCacheKey
+    def parsedInfrastructureKey = (Keys.InfrastructureCacheKey) parsed
+    parsedInfrastructureKey.kubernetesKind == KubernetesKind.CLUSTER_ROLE
+    parsedInfrastructureKey.account == "k8s"
+    parsedInfrastructureKey.namespace == ""
+    parsedInfrastructureKey.name == "system:controller:resourcequota-controller"
+  }
+
+  @Unroll
+  def "Kind fromString returns the correct kind"() {
+    expect:
+    result == Keys.Kind.fromString(input)
+
+    where:
+    input               | result
+    "logical"           | Keys.Kind.LOGICAL
+    "LOGICAL"           | Keys.Kind.LOGICAL
+    "lOgiCAl"           | Keys.Kind.LOGICAL
+    "artifacT"          | Keys.Kind.ARTIFACT
+    "InfraStructurE"    | Keys.Kind.INFRASTRUCTURE
+    "KUBERNETES_METRIC" | Keys.Kind.KUBERNETES_METRIC
+    "kubernetes_metric" | Keys.Kind.KUBERNETES_METRIC
+    "kUbernetEs_meTriC" | Keys.Kind.KUBERNETES_METRIC
+  }
+
+  @Unroll
+  def "Kind toString correctly serializes the kind to lowercase"() {
+    expect:
+    result == input.toString()
+
+    where:
+    input                       | result
+    Keys.Kind.LOGICAL           | "logical"
+    Keys.Kind.ARTIFACT          | "artifact"
+    Keys.Kind.INFRASTRUCTURE    | "infrastructure"
+    Keys.Kind.KUBERNETES_METRIC | "kubernetes_metric"
+  }
+
+  @Unroll
+  def "LogicalKind toString returns the correct string"() {
+    expect:
+    result == Keys.LogicalKind.fromString(input)
+
+    where:
+    input          | result
+    "applications" | Keys.LogicalKind.APPLICATIONS
+    "APPLICATIONS" | Keys.LogicalKind.APPLICATIONS
+    "appliCatiOns" | Keys.LogicalKind.APPLICATIONS
+    "clusters"     | Keys.LogicalKind.CLUSTERS
+    "CLUSTERS"     | Keys.LogicalKind.CLUSTERS
+    "clUsTerS"     | Keys.LogicalKind.CLUSTERS
+  }
+
+  @Unroll
+  def "LogicalKind toString correctly serializes the logical kind to lowercase"() {
+    expect:
+    result == input.toString()
+
+    where:
+    input                         | result
+    Keys.LogicalKind.APPLICATIONS | "applications"
+    Keys.LogicalKind.CLUSTERS     | "clusters"
+  }
 }