Skip to content

Commit

Permalink
fix(artifacts): Expected Artifacts should be trigger specific (#4322)
Browse files Browse the repository at this point in the history 
* fix(artifacts): Expected Artifacts should be trigger specific

When setting up multiple triggers with different expected artifacts, triggering the pipeline fails if _any_ defined expected artifact is missing, even if it is defined on another trigger. This fix filters the list of expected artifacts to make sure only the artifacts that are relevant for the current trigger is evaluated.

* Fix tests in ArtifactUtilsSpec

Signed-off-by: Jørgen Jervidalo <jorgen.jervidalo@schibsted.com>

* Add expectedArtifactIds to pipeline triggers, add more tests and fix some tests that failed

Signed-off-by: Jørgen Jervidalo <jorgen.jervidalo@schibsted.com>

* Fix typo

Signed-off-by: Jørgen Jervidalo <jorgen.jervidalo@schibsted.com>

Signed-off-by: Jørgen Jervidalo <jorgen.jervidalo@schibsted.com>
  • Loading branch information
@jervi
jervi authored Nov 7, 2022
1 parent 0e74af2 commit 17f9633
Show file tree
Hide file tree
Showing 4 changed files with 197 additions and 4 deletions.
3 changes: 3 additions & 0 deletions orca-core/src/main/java/com/netflix/spinnaker/orca/pipeline/util/ArtifactUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -197,11 +197,14 @@ public List<Artifact> getArtifactsForPipelineIdWithoutStageRef(

public void resolveArtifacts(Map 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()))
.collect(toImmutableList());

ImmutableSet<Artifact> receivedArtifacts =
Expand Down
32 changes: 30 additions & 2 deletions orca-core/src/test/groovy/com/netflix/spinnaker/orca/pipeline/util/ArtifactUtilsSpec.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -369,11 +369,11 @@ class ArtifactUtilsSpec extends Specification {
def "resolveArtifacts sets the bound artifact on an expected artifact"() {
given:
def matchArtifact = Artifact.builder().type("docker/.*").build()
def expectedArtifact = ExpectedArtifact.builder().matchArtifact(matchArtifact).build()
def expectedArtifact = ExpectedArtifact.builder().id("543ef192-82a2-4805-8d0c-827f2f976a1c").matchArtifact(matchArtifact).build()
def receivedArtifact = Artifact.builder().name("my-artifact").type("docker/image").build()
def pipeline = [
id: "abc",
trigger: [:],
trigger: ["expectedArtifactIds": ["543ef192-82a2-4805-8d0c-827f2f976a1c"]],
expectedArtifacts: [expectedArtifact],
receivedArtifacts: [receivedArtifact],
]
Expand All @@ -390,6 +390,34 @@ class ArtifactUtilsSpec extends Specification {
resolvedArtifacts.get(0).getBoundArtifact() == receivedArtifact
}

def "resolveArtifacts ignores expected artifacts from unrelated triggers"() {
given:
def matchArtifact = Artifact.builder().type("docker/.*").build()
def expectedArtifact1 = ExpectedArtifact.builder().id("expected-artifact-id").matchArtifact(matchArtifact).build()
def expectedArtifact2 = ExpectedArtifact.builder().id("irrelevant-artifact-id").matchArtifact(matchArtifact).build()
def receivedArtifact = Artifact.builder().name("my-artifact").type("docker/image").build()
def pipeline = [
id: "abc",
trigger: [
type: "jenkins",
expectedArtifactIds: ["expected-artifact-id"]
],
expectedArtifacts: [expectedArtifact1, expectedArtifact2],
receivedArtifacts: [receivedArtifact],
]
def artifactUtils = makeArtifactUtils()

when:
artifactUtils.resolveArtifacts(pipeline)
List<ExpectedArtifact> resolvedArtifacts = objectMapper.convertValue(
pipeline.trigger.resolvedExpectedArtifacts,
new TypeReference<List<ExpectedArtifact>>() {})

then:
resolvedArtifacts.size() == 1
resolvedArtifacts.get(0).getBoundArtifact() == receivedArtifact
}

def "resolveArtifacts adds received artifacts to the trigger, skipping duplicates"() {
given:
def matchArtifact = Artifact.builder().name("my-pipeline-artifact").type("docker/.*").build()
Expand Down
9 changes: 8 additions & 1 deletion ...ront50/src/main/groovy/com/netflix/spinnaker/orca/front50/DependentPipelineStarter.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,14 +79,21 @@ class DependentPipelineStarter implements ApplicationContextAware {
objectMapper.writeValueAsString(pipelineConfig))
}

def expectedArtifactIds = pipelineConfig.get("triggers", []).findAll {
it.type == "pipeline" && it.pipeline == parentPipeline.pipelineConfigId
} collectMany {
it.expectedArtifactIds ?: []
}

pipelineConfig.trigger = [
type : "pipeline",
user : authenticationDetails?.user ?: user ?: "[anonymous]",
parentExecution : parentPipeline,
parentPipelineStageId: parentPipelineStageId,
parameters : [:],
strategy : suppliedParameters.strategy == true,
correlationId : "${parentPipeline.id}_${parentPipelineStageId}_${pipelineConfig.id}_${parentPipeline.startTime}".toString()
correlationId : "${parentPipeline.id}_${parentPipelineStageId}_${pipelineConfig.id}_${parentPipeline.startTime}".toString(),
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)
Expand Down
157 changes: 156 additions & 1 deletion ...50/src/test/groovy/com/netflix/spinnaker/orca/front50/DependentPipelineStarterSpec.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,76 @@ class DependentPipelineStarterSpec extends Specification {
buildAuthenticatedUser("user", [])
)

then:
result.trigger.artifacts.size() == 2
result.trigger.artifacts*.name.contains(testArtifact1.name)
result.trigger.artifacts*.name.contains(testArtifact2.name)
result.trigger.resolvedExpectedArtifacts.size() == 0
}

def "should find expected artifacts from pipeline trigger"() {
given:
def triggeredPipelineConfig = [
name: "triggered",
id: "triggered",
expectedArtifacts: [[
id: "id1",
matchArtifact: [
kind: "gcs",
name: "gs://test/file.yaml",
type: "gcs/object"
]
]],
triggers: [[
type: "pipeline",
pipeline: "5e96d1e8-a3c0-4458-b3a4-fda17e0d5ab5",
expectedArtifactIds: ["id1"]
], [
type: "jenkins"
]]
]
Artifact testArtifact1 = Artifact.builder().type("gcs/object").name("gs://test/file.yaml").build()
Artifact testArtifact2 = Artifact.builder().type("docker/image").name("gcr.io/project/image").build()
def parentPipeline = pipeline {
name = "parent"
trigger = new DefaultTrigger("webhook", null, "test", [:], [testArtifact1, testArtifact2])
authentication = new PipelineExecution.AuthenticationDetails("parentUser", "acct1", "acct2")
pipelineConfigId = "5e96d1e8-a3c0-4458-b3a4-fda17e0d5ab5"
}
def executionLauncher = Mock(ExecutionLauncher)
def applicationContext = new StaticApplicationContext()
applicationContext.beanFactory.registerSingleton("pipelineLauncher", executionLauncher)
dependentPipelineStarter = new DependentPipelineStarter(
applicationContext,
mapper,
new ContextParameterProcessor(),
Optional.empty(),
Optional.of(artifactUtils),
new NoopRegistry()
)

and:
executionLauncher.start(*_) >> { _, p ->
return pipeline {
name = p.name
id = p.name
trigger = mapper.convertValue(p.trigger, Trigger)
}
}
artifactUtils.getArtifactsForPipelineId(*_) >> {
return new ArrayList<Artifact>();
}

when:
def result = dependentPipelineStarter.trigger(
triggeredPipelineConfig,
null,
parentPipeline,
[:],
null,
buildAuthenticatedUser("user", [])
)

then:
result.trigger.artifacts.size() == 2
result.trigger.artifacts*.name.contains(testArtifact1.name)
Expand All @@ -357,6 +427,76 @@ class DependentPipelineStarterSpec extends Specification {
result.trigger.resolvedExpectedArtifacts*.boundArtifact.name == [testArtifact1.name]
}

def "should ignore expected artifacts from unrelated trigger"() {
given:
def triggeredPipelineConfig = [
name: "triggered",
id: "triggered",
expectedArtifacts: [[
id: "id1",
matchArtifact: [
kind: "gcs",
name: "gs://test/file.yaml",
type: "gcs/object"
]
]],
triggers: [[
type: "pipeline",
pipeline: "5e96d1e8-a3c0-4458-b3a4-fda17e0d5ab5"
], [
type: "jenkins",
expectedArtifactIds: ["id1"]
]]
]
Artifact testArtifact1 = Artifact.builder().type("gcs/object").name("gs://test/file.yaml").build()
Artifact testArtifact2 = Artifact.builder().type("docker/image").name("gcr.io/project/image").build()
def parentPipeline = pipeline {
name = "parent"
trigger = new DefaultTrigger("webhook", null, "test", [:], [testArtifact1, testArtifact2])
authentication = new PipelineExecution.AuthenticationDetails("parentUser", "acct1", "acct2")
pipelineConfigId = "5e96d1e8-a3c0-4458-b3a4-fda17e0d5ab5"
}
def executionLauncher = Mock(ExecutionLauncher)
def applicationContext = new StaticApplicationContext()
applicationContext.beanFactory.registerSingleton("pipelineLauncher", executionLauncher)
dependentPipelineStarter = new DependentPipelineStarter(
applicationContext,
mapper,
new ContextParameterProcessor(),
Optional.empty(),
Optional.of(artifactUtils),
new NoopRegistry()
)

and:
executionLauncher.start(*_) >> { _, p ->
return pipeline {
name = p.name
id = p.name
trigger = mapper.convertValue(p.trigger, Trigger)
}
}
artifactUtils.getArtifactsForPipelineId(*_) >> {
return new ArrayList<Artifact>();
}

when:
def result = dependentPipelineStarter.trigger(
triggeredPipelineConfig,
null,
parentPipeline,
[:],
null,
buildAuthenticatedUser("user", [])
)

then:
result.trigger.artifacts.size() == 2
result.trigger.artifacts*.name.contains(testArtifact1.name)
result.trigger.artifacts*.name.contains(testArtifact2.name)
result.trigger.resolvedExpectedArtifacts.size() == 0
}

def "should resolve expressions in trigger"() {
given:
def triggeredPipelineConfig = [name: "triggered", id: "triggered", parameterConfig: [[name: 'a', default: '${2 == 2}']]]
Expand Down Expand Up @@ -476,6 +616,14 @@ class DependentPipelineStarterSpec extends Specification {
useDefaultArtifact: false,
usePriorArtifact: true
]],
triggers: [[
type : "pipeline",
pipeline : "5e96d1e8-a3c0-4458-b3a4-fda17e0d5ab5",
expectedArtifactIds : ["28907e3a-e529-473d-bf2d-b3737c9d6dc6"]
], [
type : "jenkins",
expectedArtifactIds : ["unrelated_id"]
]]
]
def triggeredPipelineTemplate = mapper.convertValue([
schema: "1",
Expand Down Expand Up @@ -506,6 +654,7 @@ class DependentPipelineStarterSpec extends Specification {
name = "parent"
trigger = new DefaultTrigger("manual", null, "user@schibsted.com", [:], [], [], false, true)
authentication = new PipelineExecution.AuthenticationDetails("parentUser", "acct1", "acct2")
pipelineConfigId = "5e96d1e8-a3c0-4458-b3a4-fda17e0d5ab5"
}
def executionLauncher = Mock(ExecutionLauncher)
def templateLoader = Mock(TemplateLoader)
Expand Down Expand Up @@ -580,7 +729,12 @@ class DependentPipelineStarterSpec extends Specification {
]
],
schema: "1"
]
],
triggers: [[
type : "pipeline",
pipeline : "5e96d1e8-a3c0-4458-b3a4-fda17e0d5ab5",
expectedArtifactIds : ["helm-chart"]
]]
]
def triggeredPipelineTemplate = mapper.convertValue([
schema: "1",
Expand Down Expand Up @@ -637,6 +791,7 @@ class DependentPipelineStarterSpec extends Specification {
name = "parent"
trigger = new DefaultTrigger("webhook", null, "test", [:], [testArtifact])
authentication = new PipelineExecution.AuthenticationDetails("parentUser", "acct1", "acct2")
pipelineConfigId = "5e96d1e8-a3c0-4458-b3a4-fda17e0d5ab5"
}
def executionLauncher = Mock(ExecutionLauncher)
def templateLoader = Mock(TemplateLoader)
Expand Down

0 comments on commit 17f9633

Please sign in to comment.