test(kubernetes): Add some tests on replaceAll (#4799)

* test(kubernetes): Convert kubernetes artifact tests to Java

There aren't a lot of these, and it will be easier to add new ones
(and easier to refactor and see at compile-time what tests use
functions) if we convert them to Java.

* test(kubernetes): Add some tests on replaceAll

The functionality to replace artifacts in a manifest before deploying
is fairly important but currently completely untested. Give it some
good test coverage before making changes.

* refactor(kubernetes): Minor simplifications to artifact replacer

(1) There are a lot of small functions with unclear names that
make the logic hard to follow; inline these where it makes the
logic clearer.
(2) The result of a document.read lets you iterate over the resulting
nodes, and convert them to text. This is much clearer (and will perform
better) than using an ObjectMapper for this.
(3) The Artifact passed in to replaceIfPossible is always non-null,
because it ultimately comes from the ImmutableCollection created
by filterKubernetesArtifactsByNamespaceAndAccount. That function
also filters out artifacts without a type set so we don't need to
explicitly check again (though we'll still be safe and invert the
.equals below because we can't really guarantee with an annotation
that this particular artifact has a non-null type even though it
effectively always does).

* refactor(kubernetes): Move ArtifactProvider to clouddriver-kubernetes

This is only implmented/used in the kubernetes provider, and it's not
clear whether this is an abstraction useful to other providers. Let's
move it to clouddriver-kubernetes. If this becomes something useful
for other providers we can abstract common functionality to an interface
in clouddriver-core at that point.

We can probably just delete the interface as it has only one implementation,
but keeping it for now.

* style(tests): Add comment about manifest mutation in test

clouddriver-artifacts/src/main/java/com/netflix/spinnaker/clouddriver/artifacts/kubernetes/KubernetesArtifactType.java

@@ -17,6 +17,9 @@
 
 package com.netflix.spinnaker.clouddriver.artifacts.kubernetes;
 
+import com.netflix.spinnaker.kork.annotations.NonnullByDefault;
+
+@NonnullByDefault
 public enum KubernetesArtifactType {
   DockerImage("docker/image"),
   ConfigMap("kubernetes/configMap"),

clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/artifact/KubernetesArtifactConverter.java

@@ -18,8 +18,8 @@
 package com.netflix.spinnaker.clouddriver.kubernetes.artifact;
 
 import com.netflix.spinnaker.clouddriver.kubernetes.KubernetesCloudProvider;
+import com.netflix.spinnaker.clouddriver.kubernetes.caching.view.provider.ArtifactProvider;
 import com.netflix.spinnaker.clouddriver.kubernetes.description.manifest.KubernetesManifest;
-import com.netflix.spinnaker.clouddriver.model.ArtifactProvider;
 import com.netflix.spinnaker.kork.artifacts.model.Artifact;
 
 public abstract class KubernetesArtifactConverter {

clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/artifact/KubernetesUnversionedArtifactConverter.java

@@ -17,8 +17,8 @@
 
 package com.netflix.spinnaker.clouddriver.kubernetes.artifact;
 
+import com.netflix.spinnaker.clouddriver.kubernetes.caching.view.provider.ArtifactProvider;
 import com.netflix.spinnaker.clouddriver.kubernetes.description.manifest.KubernetesManifest;
-import com.netflix.spinnaker.clouddriver.model.ArtifactProvider;
 import com.netflix.spinnaker.kork.artifacts.model.Artifact;
 import java.util.HashMap;
 import java.util.Map;

clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/artifact/KubernetesVersionedArtifactConverter.java

@@ -18,8 +18,8 @@
 package com.netflix.spinnaker.clouddriver.kubernetes.artifact;
 
 import com.fasterxml.jackson.databind.ObjectMapper;
+import com.netflix.spinnaker.clouddriver.kubernetes.caching.view.provider.ArtifactProvider;
 import com.netflix.spinnaker.clouddriver.kubernetes.description.manifest.KubernetesManifest;
-import com.netflix.spinnaker.clouddriver.model.ArtifactProvider;
 import com.netflix.spinnaker.kork.artifacts.model.Artifact;
 import java.util.HashMap;
 import java.util.List;

clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/artifact/Replacer.java

@@ -19,19 +19,17 @@
 
 import static com.google.common.collect.ImmutableList.toImmutableList;
 
-import com.fasterxml.jackson.core.type.TypeReference;
-import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.node.ArrayNode;
 import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableCollection;
 import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Streams;
 import com.jayway.jsonpath.DocumentContext;
 import com.jayway.jsonpath.PathNotFoundException;
 import com.netflix.spinnaker.clouddriver.artifacts.kubernetes.KubernetesArtifactType;
 import com.netflix.spinnaker.kork.artifacts.model.Artifact;
 import java.util.Collection;
-import java.util.List;
-import java.util.Optional;
 import java.util.function.Function;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
@@ -44,29 +42,11 @@
 @ParametersAreNonnullByDefault
 @Slf4j
 public class Replacer {
-  private static final ObjectMapper mapper = new ObjectMapper();
-
   @Nonnull private final String replacePath;
   @Nonnull private final String findPath;
   @Nullable private final Function<String, String> nameFromReference;
   @Nonnull private final KubernetesArtifactType type;
 
-  private static String substituteField(String result, String fieldName, @Nullable String field) {
-    return result.replace("{%" + fieldName + "%}", Optional.ofNullable(field).orElse(""));
-  }
-
-  private static String processPath(String path, Artifact artifact) {
-    String result = substituteField(path, "name", artifact.getName());
-    result = substituteField(result, "type", artifact.getType());
-    result = substituteField(result, "version", artifact.getVersion());
-    result = substituteField(result, "reference", artifact.getReference());
-    return result;
-  }
-
-  private ArrayNode findAll(DocumentContext obj) {
-    return obj.read(findPath);
-  }
-
   @Nonnull
   private Artifact artifactFromReference(String s) {
     return Artifact.builder().type(type.getType()).reference(s).name(nameFromReference(s)).build();
@@ -83,7 +63,8 @@ private String nameFromReference(String s) {
 
   @Nonnull
   ImmutableCollection<Artifact> getArtifacts(DocumentContext document) {
-    return mapper.convertValue(findAll(document), new TypeReference<List<String>>() {}).stream()
+    return Streams.stream(document.<ArrayNode>read(findPath).elements())
+        .map(JsonNode::asText)
         .map(this::artifactFromReference)
         .collect(toImmutableList());
   }
@@ -102,16 +83,19 @@ ImmutableCollection<Artifact> replaceArtifacts(
     return replacedArtifacts.build();
   }
 
-  private boolean replaceIfPossible(DocumentContext obj, @Nullable Artifact artifact) {
-    if (artifact == null || Strings.isNullOrEmpty(artifact.getType())) {
-      throw new IllegalArgumentException("Artifact and artifact type must be set.");
-    }
-
-    if (!artifact.getType().equals(type.getType())) {
+  private boolean replaceIfPossible(DocumentContext obj, @Nonnull Artifact artifact) {
+    if (!type.getType().equals(artifact.getType())) {
       return false;
     }
 
-    String jsonPath = processPath(replacePath, artifact);
+    // Here, given the current artifact, we are constructing a JsonPath query to find nodes in
+    // the document where we should replace the contents with information from the artifact.
+    String jsonPath =
+        replacePath
+            .replace("{%name%}", Strings.nullToEmpty(artifact.getName()))
+            .replace("{%type%}", Strings.nullToEmpty(artifact.getType()))
+            .replace("{%version%}", Strings.nullToEmpty(artifact.getVersion()))
+            .replace("{%reference%}", Strings.nullToEmpty(artifact.getReference()));
 
     log.debug("Processed jsonPath == {}", jsonPath);
 

clouddriver-core/src/main/groovy/com/netflix/spinnaker/clouddriver/model/ArtifactProvider.java → clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/caching/view/provider/ArtifactProvider.java

@@ -15,7 +15,7 @@
  *
  */
 
-package com.netflix.spinnaker.clouddriver.model;
+package com.netflix.spinnaker.clouddriver.kubernetes.caching.view.provider;
 
 import com.netflix.spinnaker.kork.artifacts.model.Artifact;
 import java.util.List;

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

@@ -20,7 +20,6 @@
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.netflix.spinnaker.cats.cache.CacheData;
 import com.netflix.spinnaker.clouddriver.kubernetes.caching.Keys;
-import com.netflix.spinnaker.clouddriver.model.ArtifactProvider;
 import com.netflix.spinnaker.kork.artifacts.model.Artifact;
 import java.util.Comparator;
 import java.util.List;

clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/converter/artifact/KubernetesCleanupArtifactsConverter.java

@@ -20,10 +20,10 @@
 import static com.netflix.spinnaker.clouddriver.orchestration.AtomicOperations.CLEANUP_ARTIFACTS;
 
 import com.netflix.spinnaker.clouddriver.kubernetes.KubernetesOperation;
+import com.netflix.spinnaker.clouddriver.kubernetes.caching.view.provider.ArtifactProvider;
 import com.netflix.spinnaker.clouddriver.kubernetes.deploy.converters.KubernetesAtomicOperationConverterHelper;
 import com.netflix.spinnaker.clouddriver.kubernetes.description.artifact.KubernetesCleanupArtifactsDescription;
 import com.netflix.spinnaker.clouddriver.kubernetes.op.artifact.KubernetesCleanupArtifactsOperation;
-import com.netflix.spinnaker.clouddriver.model.ArtifactProvider;
 import com.netflix.spinnaker.clouddriver.orchestration.AtomicOperation;
 import com.netflix.spinnaker.clouddriver.security.AbstractAtomicOperationsCredentialsSupport;
 import java.util.Map;

clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/op/artifact/KubernetesCleanupArtifactsOperation.java

@@ -20,6 +20,7 @@
 import com.google.common.base.Strings;
 import com.netflix.spinnaker.clouddriver.data.task.Task;
 import com.netflix.spinnaker.clouddriver.data.task.TaskRepository;
+import com.netflix.spinnaker.clouddriver.kubernetes.caching.view.provider.ArtifactProvider;
 import com.netflix.spinnaker.clouddriver.kubernetes.description.KubernetesResourceProperties;
 import com.netflix.spinnaker.clouddriver.kubernetes.description.artifact.KubernetesCleanupArtifactsDescription;
 import com.netflix.spinnaker.clouddriver.kubernetes.description.manifest.KubernetesKind;
@@ -29,7 +30,6 @@
 import com.netflix.spinnaker.clouddriver.kubernetes.op.OperationResult;
 import com.netflix.spinnaker.clouddriver.kubernetes.op.handler.KubernetesHandler;
 import com.netflix.spinnaker.clouddriver.kubernetes.security.KubernetesCredentials;
-import com.netflix.spinnaker.clouddriver.model.ArtifactProvider;
 import com.netflix.spinnaker.clouddriver.orchestration.AtomicOperation;
 import com.netflix.spinnaker.kork.artifacts.model.Artifact;
 import io.kubernetes.client.openapi.models.V1DeleteOptions;

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

@@ -19,6 +19,7 @@
 
 import com.netflix.spinnaker.clouddriver.data.task.Task;
 import com.netflix.spinnaker.clouddriver.data.task.TaskRepository;
+import com.netflix.spinnaker.clouddriver.kubernetes.caching.view.provider.ArtifactProvider;
 import com.netflix.spinnaker.clouddriver.kubernetes.description.job.KubernetesRunJobOperationDescription;
 import com.netflix.spinnaker.clouddriver.kubernetes.description.manifest.KubernetesDeployManifestDescription;
 import com.netflix.spinnaker.clouddriver.kubernetes.description.manifest.KubernetesKind;
@@ -27,7 +28,6 @@
 import com.netflix.spinnaker.clouddriver.kubernetes.description.manifest.KubernetesManifestStrategy.DeployStrategy;
 import com.netflix.spinnaker.clouddriver.kubernetes.op.OperationResult;
 import com.netflix.spinnaker.clouddriver.kubernetes.op.manifest.KubernetesDeployManifestOperation;
-import com.netflix.spinnaker.clouddriver.model.ArtifactProvider;
 import com.netflix.spinnaker.clouddriver.orchestration.AtomicOperation;
 import com.netflix.spinnaker.moniker.Moniker;
 import java.util.*;

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

@@ -24,6 +24,7 @@
 import com.netflix.spinnaker.clouddriver.data.task.TaskRepository;
 import com.netflix.spinnaker.clouddriver.kubernetes.artifact.ArtifactReplacer.ReplaceResult;
 import com.netflix.spinnaker.clouddriver.kubernetes.artifact.KubernetesArtifactConverter;
+import com.netflix.spinnaker.clouddriver.kubernetes.caching.view.provider.ArtifactProvider;
 import com.netflix.spinnaker.clouddriver.kubernetes.description.KubernetesResourceProperties;
 import com.netflix.spinnaker.clouddriver.kubernetes.description.manifest.*;
 import com.netflix.spinnaker.clouddriver.kubernetes.description.manifest.KubernetesManifestStrategy.Versioned;
@@ -32,7 +33,6 @@
 import com.netflix.spinnaker.clouddriver.kubernetes.op.handler.CanScale;
 import com.netflix.spinnaker.clouddriver.kubernetes.op.handler.KubernetesHandler;
 import com.netflix.spinnaker.clouddriver.kubernetes.security.KubernetesCredentials;
-import com.netflix.spinnaker.clouddriver.model.ArtifactProvider;
 import com.netflix.spinnaker.clouddriver.orchestration.AtomicOperation;
 import com.netflix.spinnaker.kork.artifacts.model.Artifact;
 import com.netflix.spinnaker.moniker.Moniker;