Skip to content

Commit

Permalink
fix(artifacts): Stop copying expectedArtifactIds to child pipelines (s…
Browse files Browse the repository at this point in the history
…pinnaker#4404)

This commit partially reverts spinnaker#4397. It broke some pipelines for a team that had a CI stage that created more artifacts with the same name as the expected artifact from the original trigger, and then had a pipeline stage. The result was that the child pipeline was triggered with multiple artifacts with the same name (but different version numbers), and then failed to start because the artifact resolver matched multiple artifacts instead of exactly one.
Turns out the changes in DependentPipelineStarter wasn't really needed to fix the issue that spinnaker#4397 tried to solve, so I'm reverting them.
  • Loading branch information
jervi authored and yugaa22 committed Jun 26, 2023
1 parent 18ff904 commit e764af1
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -84,11 +84,6 @@ 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 @@ -122,9 +117,6 @@ 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
Original file line number Diff line number Diff line change
Expand Up @@ -507,7 +507,8 @@ class DependentPipelineStarterSpec extends Specification {
triggers: [],
]
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()
Artifact testArtifact2 = Artifact.builder().type("docker/image").name("gcr.io/project/image").version("42").build()
Artifact testArtifact3 = Artifact.builder().type("docker/image").name("gcr.io/project/image").version("1337").build()
def parentPipeline = pipeline {
name = "parent"
trigger = new DefaultTrigger("webhook", null, "test", [:], [testArtifact1, testArtifact2])
Expand All @@ -523,10 +524,12 @@ class DependentPipelineStarterSpec extends Specification {
requisiteStageRefIds = ["1"]
}
}
parentPipeline.stageByRef("1").setOutputs([artifacts: [testArtifact3]])

def uuid = "8f241d2a-7fee-4a95-8d84-0a508222032c"
def expectedImage = Artifact.builder().type("docker/image").name("gcr.io/project/image").build()
ArrayList<ExpectedArtifact> expectedArtifacts = [
ExpectedArtifact.builder().id(uuid).matchArtifact(testArtifact1).build()
ExpectedArtifact.builder().id(uuid).matchArtifact(expectedImage).build()
]
parentPipeline.trigger.setOther("expectedArtifacts", expectedArtifacts)
parentPipeline.trigger.resolvedExpectedArtifacts = expectedArtifacts
Expand Down Expand Up @@ -560,17 +563,16 @@ class DependentPipelineStarterSpec extends Specification {
null,
parentPipeline,
[:],
"stage1",
"stage2",
buildAuthenticatedUser("user", [])
)

then:
result.trigger.artifacts.size() == 2
result.trigger.artifacts.size() == 3
result.trigger.artifacts*.name.contains(testArtifact1.name)
result.trigger.artifacts*.name.contains(testArtifact2.name)
result.trigger.resolvedExpectedArtifacts.size() == 1
result.trigger.resolvedExpectedArtifacts*.boundArtifact.name == [testArtifact1.name]
result.trigger.resolvedExpectedArtifacts*.id == [uuid]
result.trigger.artifacts*.name.contains(testArtifact3.name)
result.trigger.artifacts.findAll { it.name == "gcr.io/project/image" }.version.containsAll(["42", "1337"])
}

def "should resolve expressions in trigger"() {
Expand Down

0 comments on commit e764af1

Please sign in to comment.