refactor(kubernetes): clean up bake manifest error messaging (#3244)

* refactor(kubernetes): clean up bake manifest error messaging * refactor(kubernetes): make the java even better per ezimanyi
spinnaker · Oct 23, 2019 · c00a773 · c00a773
1 parent a4f0942
commit c00a773
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 41 deletions.
diff --git a/...rc/main/groovy/com/netflix/spinnaker/orca/bakery/tasks/manifests/BakeManifestContext.java b/...rc/main/groovy/com/netflix/spinnaker/orca/bakery/tasks/manifests/BakeManifestContext.java
@@ -18,15 +18,16 @@
 
 import com.fasterxml.jackson.annotation.JsonProperty;
 import com.netflix.spinnaker.kork.artifacts.model.ExpectedArtifact;
+import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import javax.annotation.Nullable;
 import lombok.Getter;
 
 @Getter
 public class BakeManifestContext {
-  @Nullable private final List<CreateBakeManifestTask.InputArtifactPair> inputArtifacts;
-  @Nullable private final CreateBakeManifestTask.InputArtifactPair inputArtifact;
+  private final List<CreateBakeManifestTask.InputArtifact> inputArtifacts;
   private final List<ExpectedArtifact> expectedArtifacts;
   private final Map<String, Object> overrides;
   private final Boolean evaluateOverrideExpressions;
@@ -39,24 +40,26 @@ public class BakeManifestContext {
   // Jackson can use to deserialize.
   public BakeManifestContext(
       @Nullable @JsonProperty("inputArtifacts")
-          List<CreateBakeManifestTask.InputArtifactPair> inputArtifacts,
+          List<CreateBakeManifestTask.InputArtifact> inputArtifacts,
       @JsonProperty("expectedArtifacts") List<ExpectedArtifact> expectedArtifacts,
       @JsonProperty("overrides") Map<String, Object> overrides,
       @JsonProperty("evaluateOverrideExpressions") Boolean evaluateOverrideExpressions,
       @JsonProperty("templateRenderer") String templateRenderer,
       @JsonProperty("outputName") String outputName,
       @JsonProperty("namespace") String namespace,
-      @Nullable @JsonProperty("inputArtifact")
-          CreateBakeManifestTask.InputArtifactPair inputArtifact,
+      @Nullable @JsonProperty("inputArtifact") CreateBakeManifestTask.InputArtifact inputArtifact,
       @JsonProperty("rawOverrides") Boolean rawOverrides) {
-    this.inputArtifacts = inputArtifacts;
-    this.expectedArtifacts = expectedArtifacts;
+    this.inputArtifacts = Optional.of(inputArtifacts).orElse(new ArrayList<>());
+    // Kustomize stage configs provide a single input artifact
+    if (this.inputArtifacts.isEmpty() && inputArtifact != null) {
+      this.inputArtifacts.add(inputArtifact);
+    }
+    this.expectedArtifacts = Optional.of(expectedArtifacts).orElse(new ArrayList<>());
     this.overrides = overrides;
     this.evaluateOverrideExpressions = evaluateOverrideExpressions;
     this.templateRenderer = templateRenderer;
     this.outputName = outputName;
     this.namespace = namespace;
-    this.inputArtifact = inputArtifact;
     this.rawOverrides = rawOverrides;
   }
 }
diff --git a/...main/groovy/com/netflix/spinnaker/orca/bakery/tasks/manifests/CreateBakeManifestTask.java b/...main/groovy/com/netflix/spinnaker/orca/bakery/tasks/manifests/CreateBakeManifestTask.java
@@ -17,7 +17,6 @@
 
 package com.netflix.spinnaker.orca.bakery.tasks.manifests;
 
-import com.fasterxml.jackson.databind.ObjectMapper;
 import com.netflix.spinnaker.kork.artifacts.model.Artifact;
 import com.netflix.spinnaker.kork.artifacts.model.ExpectedArtifact;
 import com.netflix.spinnaker.orca.ExecutionStatus;
@@ -34,8 +33,10 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.stream.Collectors;
 import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
 import lombok.Data;
 import lombok.extern.slf4j.Slf4j;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -54,45 +55,48 @@ public long getTimeout() {
     return 300000;
   }
 
-  @Autowired(required = false)
-  BakeryService bakery;
+  @Nullable private final BakeryService bakery;
 
-  @Autowired ArtifactResolver artifactResolver;
+  private final ArtifactResolver artifactResolver;
 
-  @Autowired ObjectMapper objectMapper;
+  private final ContextParameterProcessor contextParameterProcessor;
 
-  @Autowired ContextParameterProcessor contextParameterProcessor;
+  @Autowired
+  public CreateBakeManifestTask(
+      ArtifactResolver artifactResolver,
+      ContextParameterProcessor contextParameterProcessor,
+      Optional<BakeryService> bakery) {
+    this.artifactResolver = artifactResolver;
+    this.contextParameterProcessor = contextParameterProcessor;
+    this.bakery = bakery.orElse(null);
+  }
 
   @Nonnull
   @Override
   public TaskResult execute(@Nonnull Stage stage) {
+    if (bakery == null) {
+      throw new IllegalStateException(
+          "A BakeryService must be configured in order to run a Bake Manifest task.");
+    }
+
     BakeManifestContext context = stage.mapTo(BakeManifestContext.class);
 
-    List<InputArtifactPair> inputArtifactsObj = context.getInputArtifacts();
-    List<Artifact> inputArtifacts;
-
-    // kustomize depends on a single input artifact so we may not have a list here
-    // but we still want the resolution provided by the stream below
-    if (inputArtifactsObj == null || inputArtifactsObj.isEmpty()) {
-      if (context.getInputArtifact() != null) {
-        inputArtifactsObj.add(context.getInputArtifact());
-      } else {
-        throw new IllegalArgumentException("At least one input artifact to bake must be supplied");
-      }
+    List<InputArtifact> inputArtifacts = context.getInputArtifacts();
+    if (inputArtifacts.isEmpty()) {
+      throw new IllegalArgumentException("At least one input artifact to bake must be supplied");
     }
 
-    inputArtifacts =
-        inputArtifactsObj.stream()
+    List<Artifact> resolvedInputArtifacts =
+        inputArtifacts.stream()
             .map(
                 p -> {
                   Artifact a =
                       artifactResolver.getBoundArtifactForStage(stage, p.getId(), p.getArtifact());
                   if (a == null) {
                     throw new IllegalArgumentException(
-                        stage.getExecution().getId()
-                            + ": Input artifact "
-                            + p.getId()
-                            + " could not be found in the execution");
+                        String.format(
+                            "Input artifact (id: %s, account: %s) could not be found in execution (id: %s).",
+                            p.getId(), p.getAccount(), stage.getExecution().getId()));
                   }
                   a.setArtifactAccount(p.getAccount());
                   return a;
@@ -101,13 +105,9 @@ public TaskResult execute(@Nonnull Stage stage) {
 
     List<ExpectedArtifact> expectedArtifacts = context.getExpectedArtifacts();
 
-    if (expectedArtifacts == null || expectedArtifacts.isEmpty()) {
+    if (expectedArtifacts.size() != 1) {
       throw new IllegalArgumentException(
-          "At least one expected artifact to baked manifest must be supplied");
-    }
-
-    if (expectedArtifacts.size() > 1) {
-      throw new IllegalArgumentException("Too many artifacts provided as expected");
+          "Exactly one expected artifact must be supplied. Please ensure that your Bake stage config's `expectedArtifacts` list contains exactly one artifact.");
     }
 
     String outputArtifactName = expectedArtifacts.get(0).getMatchArtifact().getName();
@@ -126,10 +126,11 @@ public TaskResult execute(@Nonnull Stage stage) {
     switch (context.getTemplateRenderer().toUpperCase()) {
       case "HELM2":
         request =
-            new HelmBakeManifestRequest(context, inputArtifacts, outputArtifactName, overrides);
+            new HelmBakeManifestRequest(
+                context, resolvedInputArtifacts, outputArtifactName, overrides);
         break;
       case "KUSTOMIZE":
-        Artifact inputArtifact = inputArtifacts.get(0);
+        Artifact inputArtifact = resolvedInputArtifacts.get(0);
         request = new KustomizeBakeManifestRequest(context, inputArtifact, outputArtifactName);
         break;
       default:
@@ -146,7 +147,7 @@ public TaskResult execute(@Nonnull Stage stage) {
   }
 
   @Data
-  protected static class InputArtifactPair {
+  static class InputArtifact {
     String id;
     String account;
     Artifact artifact;

diff --git a/orca-bakery/src/test/groovy/com/netflix/spinnaker/orca/bakery/BakerySelectorSpec.groovy b/orca-bakery/src/test/groovy/com/netflix/spinnaker/orca/bakery/BakerySelectorSpec.groovy
@@ -23,7 +23,6 @@ import com.netflix.spinnaker.orca.bakery.api.BakeStatus
 import com.netflix.spinnaker.orca.bakery.api.BakeryService
 import com.netflix.spinnaker.orca.bakery.api.BaseImage
 import com.netflix.spinnaker.orca.bakery.api.manifests.BakeManifestRequest
-import com.netflix.spinnaker.orca.bakery.api.manifests.helm.HelmBakeManifestRequest
 import com.netflix.spinnaker.orca.bakery.config.BakeryConfigurationProperties
 import com.netflix.spinnaker.orca.pipeline.model.Execution
 import retrofit.http.Body