fix(artifacts): separate resolve artifacts for trigger of type pipeline

sergio-quintero · nemesisOsorio · nemesisOsorio · commit f1d0a7774071 · 2023-10-24T13:12:05.000-06:00
Co-authored-by: Nemesis Osorio &lt;nemesis211_6@hotmail.com&gt;
diff --git a/orca-core/src/main/java/com/netflix/spinnaker/orca/pipeline/util/ArtifactResolver.java b/orca-core/src/main/java/com/netflix/spinnaker/orca/pipeline/util/ArtifactResolver.java
@@ -17,7 +17,6 @@
 package com.netflix.spinnaker.orca.pipeline.util;
 
 import static com.google.common.collect.ImmutableList.toImmutableList;
-import static java.lang.String.format;
 
 import com.google.common.base.Suppliers;
 import com.google.common.collect.ImmutableList;
@@ -84,6 +83,10 @@ public static ArtifactResolver getInstance(
     return new ArtifactResolver(currentArtifacts, ImmutableList::of, requireUniqueMatches);
   }
 
+  public ResolveResult resolveExpectedArtifacts(Iterable<ExpectedArtifact> expectedArtifacts) {
+    return resolveExpectedArtifacts(expectedArtifacts, true);
+  }
+
   /**
    * Resolves the input expected artifacts, returning the result of the resolution as a {@link
    * ResolveResult}.
@@ -111,7 +114,8 @@ public static ArtifactResolver getInstance(
    * @param expectedArtifacts The expected artifacts to resolve
    * @return The result of the artifact resolution
    */
-  public ResolveResult resolveExpectedArtifacts(Iterable<ExpectedArtifact> expectedArtifacts) {
+  public ResolveResult resolveExpectedArtifacts(
+      Iterable<ExpectedArtifact> expectedArtifacts, boolean throwsExceptionIfNotFound) {
     // We keep track of resolved artifacts in an ImmutableSet.Builder so that duplicates are not
     // added (in the case that an artifact matches more than one expected artifact). An ImmutableSet
     // iterates in the order elements were added (including via the builder), so calling asList()
@@ -120,17 +124,22 @@ public ResolveResult resolveExpectedArtifacts(Iterable<ExpectedArtifact> expecte
     ImmutableList.Builder<ExpectedArtifact> boundExpectedArtifacts = ImmutableList.builder();
 
     for (ExpectedArtifact expectedArtifact : expectedArtifacts) {
-      Artifact resolved =
-          resolveSingleArtifact(expectedArtifact)
-              .orElseThrow(
-                  () ->
-                      new InvalidRequestException(
-                          format(
-                              "Unmatched expected artifact %s could not be resolved.",
-                              expectedArtifact)));
-      resolvedArtifacts.add(resolved);
-      boundExpectedArtifacts.add(expectedArtifact.toBuilder().boundArtifact(resolved).build());
+      Optional<Artifact> resolvedOptional = resolveSingleArtifact(expectedArtifact);
+
+      if (resolvedOptional.isEmpty() && throwsExceptionIfNotFound) {
+        throw new InvalidRequestException(
+            String.format(
+                "Unmatched expected artifact %s could not be resolved.", expectedArtifact));
+      }
+
+      if (resolvedOptional.isPresent()) {
+        Artifact resolvedArtifact = resolvedOptional.get();
+        resolvedArtifacts.add(resolvedArtifact);
+        boundExpectedArtifacts.add(
+            expectedArtifact.toBuilder().boundArtifact(resolvedArtifact).build());
+      }
     }
+
     return new ResolveResult(resolvedArtifacts.build().asList(), boundExpectedArtifacts.build());
   }
 
diff --git a/orca-core/src/main/java/com/netflix/spinnaker/orca/pipeline/util/ArtifactUtils.java b/orca-core/src/main/java/com/netflix/spinnaker/orca/pipeline/util/ArtifactUtils.java
@@ -52,7 +52,6 @@
 import javax.annotation.CheckReturnValue;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
-import org.apache.commons.lang3.ObjectUtils;
 import org.apache.commons.lang3.StringUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -144,31 +143,7 @@ private List<Artifact> getAllArtifacts(
         contextParameterProcessor.process(
             boundArtifactMap, contextParameterProcessor.buildExecutionContext(stage), true);
 
-    Artifact evaluatedArtifact =
-        objectMapper.convertValue(evaluatedBoundArtifactMap, Artifact.class);
-    return getBoundInlineArtifact(evaluatedArtifact, stage.getExecution())
-        .orElse(evaluatedArtifact);
-  }
-
-  private Optional<Artifact> getBoundInlineArtifact(
-      @Nullable Artifact artifact, PipelineExecution execution) {
-    if (ObjectUtils.anyNull(
-        artifact, execution.getTrigger(), execution.getTrigger().getArtifacts())) {
-      return Optional.empty();
-    }
-    try {
-      ExpectedArtifact expectedArtifact =
-          ExpectedArtifact.builder().matchArtifact(artifact).build();
-      return ArtifactResolver.getInstance(execution.getTrigger().getArtifacts(), true)
-          .resolveExpectedArtifacts(List.of(expectedArtifact))
-          .getResolvedExpectedArtifacts()
-          .stream()
-          .findFirst()
-          .flatMap(this::getBoundArtifact);
-    } catch (InvalidRequestException e) {
-      log.debug("Could not match inline artifact with trigger bound artifacts", e);
-      return Optional.empty();
-    }
+    return objectMapper.convertValue(evaluatedBoundArtifactMap, Artifact.class);
   }
 
   public @Nullable Artifact getBoundArtifactForId(StageExecution stage, @Nullable String id) {
@@ -230,6 +205,107 @@ public List<Artifact> getArtifactsForPipelineIdWithoutStageRef(
         .orElse(Collections.emptyList());
   }
 
+  /**
+   * The intention is to throw an exception if all the expected artifacts are not provided in the
+   * trigger <br>
+   * For now this is only for triggers of type pipeline
+   *
+   * @param pipeline
+   * @throws InvalidRequestException if the trigger does not provide all the expected artifacts
+   */
+  public void validateArtifactConstraintsFromTrigger(Map<String, Object> pipeline) {
+    Map<String, Object> trigger = (Map<String, Object>) pipeline.get("trigger");
+    List<?> expectedArtifactIds =
+        (List<?>) trigger.getOrDefault("expectedArtifactIds", emptyList());
+
+    ImmutableList<ExpectedArtifact> expectedArtifacts =
+        Optional.ofNullable((List<?>) pipeline.get("expectedArtifacts"))
+            .map(Collection::stream)
+            .orElse(Stream.empty())
+            .map(it -> objectMapper.convertValue(it, ExpectedArtifact.class))
+            .filter(
+                artifact ->
+                    expectedArtifactIds.contains(artifact.getId())
+                        || artifact.isUseDefaultArtifact()
+                        || artifact.isUsePriorArtifact())
+            .collect(toImmutableList());
+
+    ImmutableSet<Artifact> receivedArtifacts = concatReceivedArtifacts(pipeline, trigger);
+
+    ArtifactResolver.getInstance(
+            receivedArtifacts, () -> getPriorArtifacts(pipeline), /* requireUniqueMatches= */ true)
+        .resolveExpectedArtifacts(expectedArtifacts);
+  }
+
+  private ImmutableSet<Artifact> concatReceivedArtifacts(
+      Map<String, Object> pipeline, Map<String, Object> trigger) {
+    return Stream.concat(
+            Optional.ofNullable((List<?>) pipeline.get("receivedArtifacts"))
+                .map(Collection::stream)
+                .orElse(Stream.empty()),
+            Optional.ofNullable((List<?>) trigger.get("artifacts"))
+                .map(Collection::stream)
+                .orElse(Stream.empty()))
+        .map(it -> objectMapper.convertValue(it, Artifact.class))
+        .collect(toImmutableSet());
+  }
+
+  /**
+   * This will only resolve the expected artifacts that match received artifacts. <br>
+   * If the expected artifact is not present in received artifacts, it is not resolved. <br>
+   * This resolves as many artifacts as possible, because there is no standard way to include all
+   * the stages using expected artifacts. <br>
+   * The best scenario is: the trigger pipeline provides all the expected artifacts. <br>
+   * The worst scenario is: the pipeline will fail until the stage using the expected artifact is
+   * executed. <br>
+   * For now this is only for triggers of type pipeline
+   *
+   * @param pipeline
+   */
+  public void resolveArtifactsFromTrigger(Map pipeline) {
+    Map<String, Object> trigger = (Map<String, Object>) pipeline.get("trigger");
+    ImmutableList<ExpectedArtifact> expectedArtifacts =
+        Optional.ofNullable((List<?>) pipeline.get("expectedArtifacts"))
+            .map(Collection::stream)
+            .orElse(Stream.empty())
+            .map(it -> objectMapper.convertValue(it, ExpectedArtifact.class))
+            .collect(toImmutableList());
+
+    ImmutableSet<Artifact> receivedArtifacts = concatReceivedArtifacts(pipeline, trigger);
+
+    ArtifactResolver.ResolveResult resolveResult =
+        ArtifactResolver.getInstance(
+                receivedArtifacts,
+                () -> getPriorArtifacts(pipeline),
+                /* requireUniqueMatches= */ true)
+            .resolveExpectedArtifacts(expectedArtifacts, false);
+
+    ImmutableSet<Artifact> allArtifacts =
+        ImmutableSet.<Artifact>builder()
+            .addAll(receivedArtifacts)
+            .addAll(resolveResult.getResolvedArtifacts())
+            .build();
+
+    try {
+      trigger.put(
+          "artifacts",
+          objectMapper.readValue(objectMapper.writeValueAsString(allArtifacts), List.class));
+      trigger.put(
+          "expectedArtifacts",
+          objectMapper.readValue(
+              objectMapper.writeValueAsString(resolveResult.getResolvedExpectedArtifacts()),
+              List.class));
+      trigger.put(
+          "resolvedExpectedArtifacts",
+          objectMapper.readValue(
+              objectMapper.writeValueAsString(resolveResult.getResolvedExpectedArtifacts()),
+              List.class)); // Add the actual expectedArtifacts we included in the ids.
+    } catch (IOException e) {
+      throw new ArtifactResolutionException(
+          "Failed to store artifacts in trigger: " + e.getMessage(), e);
+    }
+  }
+
   private List<String> getExpectedArtifactIdsFromMap(Map<String, Object> trigger) {
     List<String> expectedArtifactIds = (List<String>) trigger.get("expectedArtifactIds");
     return (expectedArtifactIds != null) ? expectedArtifactIds : emptyList();
diff --git a/orca-core/src/test/groovy/com/netflix/spinnaker/orca/pipeline/util/ArtifactUtilsSpec.groovy b/orca-core/src/test/groovy/com/netflix/spinnaker/orca/pipeline/util/ArtifactUtilsSpec.groovy
@@ -82,29 +82,6 @@ class ArtifactUtilsSpec extends Specification {
     artifact.name == 'build/libs/my-jar-100.jar'
   }
 
-  def "should bind stage-inlined artifacts to trigger artifacts"() {
-    setup:
-    def execution = pipeline {
-      stage {
-        name = "upstream stage"
-        type = "stage1"
-        refId = "1"
-      }
-    }
-
-    execution.trigger = new DefaultTrigger('manual')
-    execution.trigger.artifacts.add(Artifact.builder().type('http/file').name('build/libs/my-jar-100.jar').build())
-
-    when:
-    def artifact = makeArtifactUtils().getBoundArtifactForStage(execution.stages[0], null, Artifact.builder()
-        .type('http/file')
-        .name('build/libs/my-jar-\\d+.jar')
-        .build())
-
-    then:
-    artifact.name == 'build/libs/my-jar-100.jar'
-  }
-
   def "should find upstream artifacts in small pipeline"() {
     when:
     def desired = execution.getStages().find { it.name == "desired" }
diff --git a/orca-front50/src/main/groovy/com/netflix/spinnaker/orca/front50/DependentPipelineStarter.groovy b/orca-front50/src/main/groovy/com/netflix/spinnaker/orca/front50/DependentPipelineStarter.groovy
@@ -85,15 +85,6 @@ class DependentPipelineStarter implements ApplicationContextAware {
       it.expectedArtifactIds ?: []
     }
 
-    // we are following a similar approach as triggers above
-    // expectedArtifacts can be used in triggers and stages
-    // for now we identified DeployManifestStage
-    // in ResolveDeploySourceManifestTask using ManifestEvaluator.getRequiredArtifacts
-    def requiredArtifactIds = pipelineConfig.get("stages", []).collectMany {
-      it.requiredArtifactIds ?: []
-    }
-    expectedArtifactIds.addAll(requiredArtifactIds)
-
     pipelineConfig.trigger = [
       type                 : "pipeline",
       user                 : authenticationDetails?.user ?: user ?: "[anonymous]",
@@ -102,7 +93,7 @@ class DependentPipelineStarter implements ApplicationContextAware {
       parameters           : [:],
       strategy             : suppliedParameters.strategy == true,
       correlationId        : "${parentPipeline.id}_${parentPipelineStageId}_${pipelineConfig.id}_${parentPipeline.startTime}".toString(),
-      expectedArtifactIds  : expectedArtifactIds.toSet().toList()
+      expectedArtifactIds  : expectedArtifactIds
     ]
     /* correlationId is added so that two pipelines aren't triggered when a pipeline is canceled.
      * parentPipelineStageId is added so that a child pipeline (via pipeline stage)
@@ -144,7 +135,8 @@ class DependentPipelineStarter implements ApplicationContextAware {
     def artifactError = null
 
     try {
-      artifactUtils?.resolveArtifacts(pipelineConfig)
+      artifactUtils?.validateArtifactConstraintsFromTrigger(pipelineConfig)
+      artifactUtils?.resolveArtifactsFromTrigger(pipelineConfig)
     } catch (Exception e) {
       artifactError = e
     }
diff --git a/orca-front50/src/test/groovy/com/netflix/spinnaker/orca/front50/DependentPipelineStarterSpec.groovy b/orca-front50/src/test/groovy/com/netflix/spinnaker/orca/front50/DependentPipelineStarterSpec.groovy
