Skip to content

Commit

Permalink
fix(artifacts): Parent and child pipeline artifact resolution (spinna…
Browse files Browse the repository at this point in the history
…ker#4575)

This fix allows us to revert spinnaker#4397
since that makes retrieving bound artifacts actually more strict for
most use cases. So this commit addresses the root cause that that PR was
trying to implement.

When a parent pipeline triggers a child pipeline, the child pipeline
should be able to resolve incoming artifacts properly. Right now that
does not happen because `expectedArtifactIds` is needed in the trigger
section, otherwise no resolution will occur.

This change fixes the parent payload to include `expectedArtifactIds` by
viewing the child pipeline's top level expected artifacts and finding
the intersection between expected artifacts and artifacts to be sent.
With that intersection we can grab the expected artifact ids and
properly set the the necessary field to allow for child pipelines to
ingest the artifact.

Signed-off-by: benjamin-j-powell <bjp@apple.com>
Co-authored-by: Benevolent Benjamin Powell <benjamin_j_powell@apple.com>
  • Loading branch information
2 people authored and Pranav-b-7 committed Nov 2, 2023
1 parent c3e0fdc commit 6da5ea7
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ package com.netflix.spinnaker.orca.front50
import com.fasterxml.jackson.databind.ObjectMapper
import com.netflix.spectator.api.Id
import com.netflix.spectator.api.Registry
import com.netflix.spinnaker.kork.artifacts.model.Artifact
import com.netflix.spinnaker.kork.artifacts.model.ExpectedArtifact
import com.netflix.spinnaker.kork.exceptions.ConfigurationException
import com.netflix.spinnaker.orca.api.pipeline.models.PipelineExecution
import com.netflix.spinnaker.orca.api.pipeline.models.Trigger
Expand Down Expand Up @@ -102,7 +104,6 @@ class DependentPipelineStarter implements ApplicationContextAware {
parameters : [:],
strategy : suppliedParameters.strategy == true,
correlationId : "${parentPipeline.id}_${parentPipelineStageId}_${pipelineConfig.id}_${parentPipeline.startTime}".toString(),
expectedArtifactIds : expectedArtifactIds.toSet().toList()
]
/* 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)
Expand All @@ -121,8 +122,10 @@ class DependentPipelineStarter implements ApplicationContextAware {
}
}

// keep the trigger as the preprocessor may remove it. Preprocessor will
// only remove the trigger key if the pipeline is a pipeline template
// otherwise, the trigger remains intact
def trigger = pipelineConfig.trigger
//keep the trigger as the preprocessor removes it.

if (parentPipelineStageId != null) {
pipelineConfig.receivedArtifacts = artifactUtils?.getArtifacts(parentPipeline.stageById(parentPipelineStageId))
Expand All @@ -133,6 +136,25 @@ class DependentPipelineStarter implements ApplicationContextAware {
// This is required for template source with jinja expressions
trigger.artifacts = pipelineConfig.receivedArtifacts

// expectedArtifacts for triggers are defined at the top level of the
// pipeline, so we need to extract that and match them against the incoming
// artifacts to understand which expected artifact IDs are necessary to pass
// to the artifact resolution
expectedArtifactIds.addAll(pipelineConfig.get("expectedArtifacts", []).collectMany {
def expectedArtifact = objectMapper.convertValue(it, ExpectedArtifact)
return trigger.artifacts.findAll {
// The idea here is we are only interested in matching against incoming
// artifacts. So if the expected artifact does not match on any incoming
// artifact, we will simply not add the expected artifact ID to the
// array. This is very similar to how echo works.
def artifact = objectMapper.convertValue(it, Artifact)
return expectedArtifact.matches(artifact)
}.collect {
return expectedArtifact.id
}
} as String[])
trigger.expectedArtifactIds = expectedArtifactIds

for (ExecutionPreprocessor preprocessor : executionPreprocessors.findAll {
it.supports(pipelineConfig, ExecutionPreprocessor.Type.PIPELINE)
}) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,8 @@ class DependentPipelineStarterSpec extends Specification {
result.trigger.artifacts.size() == 2
result.trigger.artifacts*.name.contains(testArtifact1.name)
result.trigger.artifacts*.name.contains(testArtifact2.name)
result.trigger.resolvedExpectedArtifacts.size() == 0
result.trigger.resolvedExpectedArtifacts.size() == 1
result.trigger.resolvedExpectedArtifacts.get(0).id.equals("id1")
}

def "should find expected artifacts from pipeline trigger"() {
Expand Down Expand Up @@ -496,7 +497,8 @@ class DependentPipelineStarterSpec extends Specification {
result.trigger.artifacts.size() == 2
result.trigger.artifacts*.name.contains(testArtifact1.name)
result.trigger.artifacts*.name.contains(testArtifact2.name)
result.trigger.resolvedExpectedArtifacts.size() == 0
result.trigger.resolvedExpectedArtifacts.size() == 1
result.trigger.resolvedExpectedArtifacts.get(0).id.equals("id1")
}

def "should find expected artifacts from parent pipeline trigger if triggered by pipeline stage"() {
Expand Down

0 comments on commit 6da5ea7

Please sign in to comment.