Skip to content

Commit

Permalink
fix(artifacts): Be more lenient when filtering expected artifacts
Browse files Browse the repository at this point in the history
It is currently hard/impossible to resolve artifacts in pipelines that doesn't have any triggers defined by themselves, and are triggered by a pipeline stage in another pipeline. Previously it was possible to get it to work by editing the json of the pipeline or in the UI by adding artifacts in a temp trigger and then removing it (this would keep the expected artifacts around).
When I introduced trigger specific artifact constraints in spinnaker#4322, I made it a lot harder (if not impossible) to do this because no triggers are used and thus all expected artifacts are filtered out.

This commit contains implementation code to fix the tests added in the previous commit. It will fix the issue by copying expected artifacts from the parent execution (only if triggered by a pipeline stage), thus avoiding the whole situation altogether. It will also not filter away any expected artifacts where `useDefaultArtifact` or `usePriorArtifact` is set to `true`, and it will bind artifacts defined inline in stages (so that you can match artifacts using regex and not only SpEL).
  • Loading branch information
jervi committed Feb 21, 2023
1 parent 0b14c2d commit 4816f8c
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,20 +28,28 @@
import com.netflix.spinnaker.kork.annotations.NonnullByDefault;
import com.netflix.spinnaker.kork.artifacts.model.Artifact;
import com.netflix.spinnaker.kork.artifacts.model.ExpectedArtifact;
import com.netflix.spinnaker.kork.web.exceptions.InvalidRequestException;
import com.netflix.spinnaker.orca.api.pipeline.models.PipelineExecution;
import com.netflix.spinnaker.orca.api.pipeline.models.StageExecution;
import com.netflix.spinnaker.orca.pipeline.model.StageContext;
import com.netflix.spinnaker.orca.pipeline.model.StageExecutionImpl;
import com.netflix.spinnaker.orca.pipeline.persistence.ExecutionRepository;
import com.netflix.spinnaker.orca.pipeline.persistence.ExecutionRepository.ExecutionCriteria;
import java.io.IOException;
import java.util.*;
import java.util.Collection;
import java.util.Collections;
import java.util.Comparator;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.function.Predicate;
import java.util.stream.Collectors;
import java.util.stream.Stream;
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;
Expand Down Expand Up @@ -133,7 +141,31 @@ private List<Artifact> getAllArtifacts(
contextParameterProcessor.process(
boundArtifactMap, contextParameterProcessor.buildExecutionContext(stage), true);

return objectMapper.convertValue(evaluatedBoundArtifactMap, Artifact.class);
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();
}
}

public @Nullable Artifact getBoundArtifactForId(StageExecution stage, @Nullable String id) {
Expand Down Expand Up @@ -204,7 +236,11 @@ public void resolveArtifacts(Map pipeline) {
.map(Collection::stream)
.orElse(Stream.empty())
.map(it -> objectMapper.convertValue(it, ExpectedArtifact.class))
.filter(artifact -> expectedArtifactIds.contains(artifact.getId()))
.filter(
artifact ->
expectedArtifactIds.contains(artifact.getId())
|| artifact.isUseDefaultArtifact()
|| artifact.isUsePriorArtifact())
.collect(toImmutableList());

ImmutableSet<Artifact> receivedArtifacts =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,11 @@ class DependentPipelineStarter implements ApplicationContextAware {
} collectMany {
it.expectedArtifactIds ?: []
}
if (!expectedArtifactIds && parentPipelineStageId) {
expectedArtifactIds = parentPipeline.trigger.resolvedExpectedArtifacts.collect {
it.id
}
}

pipelineConfig.trigger = [
type : "pipeline",
Expand Down Expand Up @@ -117,6 +122,9 @@ class DependentPipelineStarter implements ApplicationContextAware {

if (parentPipelineStageId != null) {
pipelineConfig.receivedArtifacts = artifactUtils?.getArtifacts(parentPipeline.stageById(parentPipelineStageId))
if (!pipelineConfig.expectedArtifacts) {
pipelineConfig.expectedArtifacts = parentPipeline.trigger.getOther().getOrDefault("expectedArtifacts", [])
}
} else {
pipelineConfig.receivedArtifacts = artifactUtils?.getAllArtifacts(parentPipeline)
}
Expand Down

0 comments on commit 4816f8c

Please sign in to comment.