From 33864c52fd293bbb6d89f3c8aa57fe59ce9a1800 Mon Sep 17 00:00:00 2001 From: Benevolent Benjamin Powell Date: Thu, 26 Oct 2023 16:54:21 -0500 Subject: [PATCH] fix(artifacts): Parent and child pipeline artifact resolution This fix allows us to revert https://github.com/spinnaker/orca/pull/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, it 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 --- .../front50/DependentPipelineStarter.groovy | 26 +++++++++++++++++-- .../DependentPipelineStarterSpec.groovy | 6 +++-- 2 files changed, 28 insertions(+), 4 deletions(-) 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 index aa2f485b66d..d2e76f5d22c 100644 --- 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 @@ -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 @@ -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) @@ -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)) @@ -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) }) { 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 index aeae05c7bbd..89600fb9746 100644 --- 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 @@ -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"() { @@ -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"() {