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 index f57085a5d28..01b0bf1cbf4 100644 --- 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 @@ -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 getArtifactsForPipelineIdWithoutStageRef( .orElse(Collections.emptyList()); } + private List getExpectedArtifactIdsFromMap(Map trigger) { + return Optional.ofNullable((List) trigger.get("expectedArtifactIds")) + .orElse(emptyList()); + } + public void resolveArtifacts(Map pipeline) { Map trigger = (Map) pipeline.get("trigger"); - List expectedArtifactIds = - (List) trigger.getOrDefault("expectedArtifactIds", emptyList()); + List triggers = Optional.ofNullable((List) pipeline.get("triggers")).orElse(emptyList()); + Set 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) 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 expectedArtifactIds = new ArrayList<>(expectedArtifactIdsListConcat); ImmutableList expectedArtifacts = Optional.ofNullable((List) pipeline.get("expectedArtifacts")) .map(Collection::stream) 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 index 97ff6240d85..e8fc88b65b6 100644 --- 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 @@ -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 extractTriggerArtifacts(Map trigger) { return objectMapper.convertValue(trigger.artifacts, new TypeReference>(){}); }