fix(artifacts): Resolving is broken in multiple places

8df68b7 broke custom triggers to Spinnaker integration. Prior to
this commit if `getBoundArtifactForStage` failed to find a bound artifact,
resolution would happen much later by utilizing the `triggers` field in
stages.  However, `getBoundArtifactForStage` now throws an exception, which
causes custom triggers to fail prematurely. Also if a custom trigger calls
orchestrate directly and any pipeline contains `getBoundArtifactForStage`,
this will throw an exception and prevent any resolution.

Signed-off-by: benjamin-j-powell <[email protected]>

orca-core/src/main/java/com/netflix/spinnaker/orca/pipeline/util/ArtifactUtils.java

@@ -36,13 +36,16 @@
 import com.netflix.spinnaker.orca.pipeline.persistence.ExecutionRepository;
 import com.netflix.spinnaker.orca.pipeline.persistence.ExecutionRepository.ExecutionCriteria;
 import java.io.IOException;
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Comparator;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
+import java.util.Set;
 import java.util.function.Predicate;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
@@ -227,10 +230,45 @@ public List<Artifact> getArtifactsForPipelineIdWithoutStageRef(
         .orElse(Collections.emptyList());
   }
 
+  private List<String> getExpectedArtifactIdsFromMap(Map<String, Object> trigger) {
+    return Optional.ofNullable((List<String>) trigger.get("expectedArtifactIds"))
+        .orElse(emptyList());
+  }
+
   public void resolveArtifacts(Map pipeline) {
     Map<String, Object> trigger = (Map<String, Object>) pipeline.get("trigger");
-    List<?> expectedArtifactIds =
-        (List<?>) trigger.getOrDefault("expectedArtifactIds", emptyList());
+    List<?> triggers = Optional.ofNullable((List<?>) pipeline.get("triggers")).orElse(emptyList());
+    Set<String> expectedArtifactIdsListConcat =
+        new HashSet<>(getExpectedArtifactIdsFromMap(trigger));
+
+    // Due to 8df68b79cf1 getBoundArtifactForStage now does resolution which can
+    // potentially return an empty list or null back, which will throw an
+    // exception. Before this commit, when getBoundArtifactForStage was called,
+    // the method would just retrieve the bound artifact from the stage context,
+    // and return it as an ExpectedArtifact type instead of a map. "Resolution"
+    // would happen later in the DependentPipelineStarter class which handles
+    // the overall pipeline triggers case which we have now added here.
+    //
+    // Another case is when any custom trigger occurs, and it runs the
+    // orchestrate endpoint and if any pipeline stage uses
+    // getBoundArtifactForStage then this will throw an exception, instead of
+    // returning the bound artifact which will be later be added as an resolved
+    // artifact in the context. A good example of this is the
+    // CreateBakeManifestTask.
+    //
+    // reference: https://github.com/spinnaker/orca/pull/4397
+    triggers.stream()
+        .map(it -> (Map<String, Object>) it)
+        // This filter prevents multiple triggers from adding its
+        // expectedArtifactIds unless it is the expected trigger type that was
+        // triggered
+        //
+        // reference: https://github.com/spinnaker/orca/pull/4322
+        .filter(it -> trigger.getOrDefault("type", "").equals(it.get("type")))
+        .map(this::getExpectedArtifactIdsFromMap)
+        .forEach(expectedArtifactIdsListConcat::addAll);
+
+    final List<String> expectedArtifactIds = new ArrayList<>(expectedArtifactIdsListConcat);
     ImmutableList<ExpectedArtifact> expectedArtifacts =
         Optional.ofNullable((List<?>) pipeline.get("expectedArtifacts"))
             .map(Collection::stream)

orca-core/src/test/groovy/com/netflix/spinnaker/orca/pipeline/util/ArtifactUtilsSpec.groovy

@@ -19,6 +19,7 @@ package com.netflix.spinnaker.orca.pipeline.util
 
 import com.fasterxml.jackson.core.type.TypeReference
 import com.fasterxml.jackson.databind.ObjectMapper
+import com.netflix.spinnaker.kork.artifacts.ArtifactTypes
 import com.netflix.spinnaker.kork.artifacts.model.Artifact
 import com.netflix.spinnaker.kork.artifacts.model.ExpectedArtifact
 import com.netflix.spinnaker.orca.api.pipeline.models.ExecutionStatus
@@ -515,6 +516,83 @@ class ArtifactUtilsSpec extends Specification {
     initialArtifacts == finalArtifacts
   }
 
+  def "should find artifact if triggers is present in pipeline"() {
+    given:
+    def defaultArtifact = Artifact.builder()
+        .customKind(true)
+        .build()
+
+    def matchArtifact = Artifact.builder()
+        .name("my-pipeline-artifact")
+        .type("embedded/base64")
+        .reference("aGVsbG8gd29ybGQK")
+        .build()
+
+    def expectedArtifact = ExpectedArtifact.builder()
+        .usePriorArtifact(false)
+        .useDefaultArtifact(false)
+        .id("my-id")
+        .defaultArtifact(defaultArtifact)
+        .matchArtifact(matchArtifact)
+        .build()
+
+    def expectedArtifact2 = ExpectedArtifact.builder()
+        .usePriorArtifact(false)
+        .useDefaultArtifact(false)
+        .id("my-id-2")
+        .defaultArtifact(defaultArtifact)
+        .matchArtifact(matchArtifact)
+        .build()
+
+    def pipeline = [
+        "id": "abc",
+        "stages": [
+            stage {
+              expectedArtifacts: [expectedArtifact]
+              inputArtifacts: [
+                  "id": "my-id"
+              ]
+            }
+        ],
+        expectedArtifacts: [
+          expectedArtifact
+        ],
+        trigger: [
+            artifacts: [
+               Artifact.builder()
+                .type(ArtifactTypes.EMBEDDED_BASE64.getMimeType())
+                .name(matchArtifact.getName())
+                .reference(matchArtifact.getReference())
+                .build()
+            ],
+            type: "some-type"
+        ],
+        triggers: [
+            [
+                enabled: true,
+                expectedArtifactIds: [
+                    expectedArtifact.getId()
+                ],
+                type: "some-type"
+            ],
+            [
+                enabled: true,
+                expectedArtifactIds: [
+                    expectedArtifact2.getId()
+                ],
+                type: "some-other-type"
+            ]
+        ]
+    ]
+
+    def pipelineMap = getObjectMapper().convertValue(pipeline, Map.class)
+    when:
+     makeArtifactUtils().resolveArtifacts(pipelineMap)
+
+    then:
+    pipelineMap.trigger.resolvedExpectedArtifacts.size() == 1
+  }
+
   private List<Artifact> extractTriggerArtifacts(Map<String, Object> trigger) {
     return objectMapper.convertValue(trigger.artifacts, new TypeReference<List<Artifact>>(){});
   }