From 17f9633104171fd0794c9c6928e6480bd9e66d2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B8rgen=20Jervidalo?= Date: Mon, 7 Nov 2022 17:42:37 +0100 Subject: [PATCH] fix(artifacts): Expected Artifacts should be trigger specific (#4322) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 * Add expectedArtifactIds to pipeline triggers, add more tests and fix some tests that failed Signed-off-by: Jørgen Jervidalo * Fix typo Signed-off-by: Jørgen Jervidalo Signed-off-by: Jørgen Jervidalo --- .../orca/pipeline/util/ArtifactUtils.java | 3 + .../pipeline/util/ArtifactUtilsSpec.groovy | 32 +++- .../front50/DependentPipelineStarter.groovy | 9 +- .../DependentPipelineStarterSpec.groovy | 157 +++++++++++++++++- 4 files changed, 197 insertions(+), 4 deletions(-) diff --git a/orca-core/src/main/java/com/netflix/spinnaker/orca/pipeline/util/ArtifactUtils.java b/orca-core/src/main/java/com/netflix/spinnaker/orca/pipeline/util/ArtifactUtils.java index 8790ce2ec8..4ae052f4e5 100644 --- a/orca-core/src/main/java/com/netflix/spinnaker/orca/pipeline/util/ArtifactUtils.java +++ b/orca-core/src/main/java/com/netflix/spinnaker/orca/pipeline/util/ArtifactUtils.java @@ -197,11 +197,14 @@ public List getArtifactsForPipelineIdWithoutStageRef( public void resolveArtifacts(Map pipeline) { Map trigger = (Map) pipeline.get("trigger"); + List expectedArtifactIds = + (List) trigger.getOrDefault("expectedArtifactIds", emptyList()); ImmutableList 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 receivedArtifacts = diff --git a/orca-core/src/test/groovy/com/netflix/spinnaker/orca/pipeline/util/ArtifactUtilsSpec.groovy b/orca-core/src/test/groovy/com/netflix/spinnaker/orca/pipeline/util/ArtifactUtilsSpec.groovy index 98139bf346..b034a3202e 100644 --- a/orca-core/src/test/groovy/com/netflix/spinnaker/orca/pipeline/util/ArtifactUtilsSpec.groovy +++ b/orca-core/src/test/groovy/com/netflix/spinnaker/orca/pipeline/util/ArtifactUtilsSpec.groovy @@ -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], ] @@ -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 resolvedArtifacts = objectMapper.convertValue( + pipeline.trigger.resolvedExpectedArtifacts, + new TypeReference>() {}) + + 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() 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 954de3b1ca..91e36beb8c 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 @@ -79,6 +79,12 @@ 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]", @@ -86,7 +92,8 @@ class DependentPipelineStarter implements ApplicationContextAware { 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) 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 3e8e4e7a77..aa08e08d3e 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 @@ -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(); + } + + when: + def result = dependentPipelineStarter.trigger( + triggeredPipelineConfig, + null, + parentPipeline, + [:], + null, + buildAuthenticatedUser("user", []) + ) + then: result.trigger.artifacts.size() == 2 result.trigger.artifacts*.name.contains(testArtifact1.name) @@ -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(); + } + + 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}']]] @@ -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", @@ -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) @@ -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", @@ -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)