From 33cdfd52fbb35496151a2181869d7415c75d7e1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pedro=20Gon=C3=A7alves?= Date: Thu, 7 Sep 2017 19:55:12 +0200 Subject: [PATCH] fix(pipeline_template): Build partials, execute conditionals after and trim conditions as the last step (#1602) --- .../v1schema/graph/GraphMutator.java | 4 ++- .../ConfigStageInjectionTransform.java | 19 ----------- .../transform/TrimConditionalsTransform.java | 34 +++++++++++++++++++ ...ineTemplatePipelinePreprocessorSpec.groovy | 15 ++++++++ .../templates/conditional-partials.yml | 26 ++++++++++++++ 5 files changed, 78 insertions(+), 20 deletions(-) create mode 100644 orca-pipelinetemplate/src/main/java/com/netflix/spinnaker/orca/pipelinetemplate/v1schema/graph/transform/TrimConditionalsTransform.java create mode 100644 orca-pipelinetemplate/src/test/resources/templates/conditional-partials.yml diff --git a/orca-pipelinetemplate/src/main/java/com/netflix/spinnaker/orca/pipelinetemplate/v1schema/graph/GraphMutator.java b/orca-pipelinetemplate/src/main/java/com/netflix/spinnaker/orca/pipelinetemplate/v1schema/graph/GraphMutator.java index dd1558a34c..3d1106fc1c 100644 --- a/orca-pipelinetemplate/src/main/java/com/netflix/spinnaker/orca/pipelinetemplate/v1schema/graph/GraphMutator.java +++ b/orca-pipelinetemplate/src/main/java/com/netflix/spinnaker/orca/pipelinetemplate/v1schema/graph/GraphMutator.java @@ -25,6 +25,7 @@ import com.netflix.spinnaker.orca.pipelinetemplate.v1schema.graph.transform.PipelineConfigInheritanceTransform; import com.netflix.spinnaker.orca.pipelinetemplate.v1schema.graph.transform.RenderTransform; import com.netflix.spinnaker.orca.pipelinetemplate.v1schema.graph.transform.StageInheritanceControlTransform; +import com.netflix.spinnaker.orca.pipelinetemplate.v1schema.graph.transform.TrimConditionalsTransform; import com.netflix.spinnaker.orca.pipelinetemplate.v1schema.model.PipelineTemplate; import com.netflix.spinnaker.orca.pipelinetemplate.v1schema.model.TemplateConfiguration; import com.netflix.spinnaker.orca.pipelinetemplate.v1schema.render.Renderer; @@ -39,13 +40,14 @@ public class GraphMutator { public GraphMutator(TemplateConfiguration configuration, Renderer renderer, Registry registry, Map trigger) { visitors.add(new DefaultVariableAssignmentTransform(configuration)); - visitors.add(new ConditionalStanzaTransform(configuration, renderer, trigger)); visitors.add(new ConfigModuleReplacementTransform(configuration)); visitors.add(new ConfigPartialReplacementTransform(configuration)); visitors.add(new PipelineConfigInheritanceTransform(configuration)); visitors.add(new RenderTransform(configuration, renderer, registry, trigger)); visitors.add(new ConfigStageInjectionTransform(configuration)); visitors.add(new StageInheritanceControlTransform()); + visitors.add(new ConditionalStanzaTransform(configuration, renderer, trigger)); + visitors.add(new TrimConditionalsTransform()); } public void mutate(PipelineTemplate template) { diff --git a/orca-pipelinetemplate/src/main/java/com/netflix/spinnaker/orca/pipelinetemplate/v1schema/graph/transform/ConfigStageInjectionTransform.java b/orca-pipelinetemplate/src/main/java/com/netflix/spinnaker/orca/pipelinetemplate/v1schema/graph/transform/ConfigStageInjectionTransform.java index 994ea97d28..b6bae25a72 100644 --- a/orca-pipelinetemplate/src/main/java/com/netflix/spinnaker/orca/pipelinetemplate/v1schema/graph/transform/ConfigStageInjectionTransform.java +++ b/orca-pipelinetemplate/src/main/java/com/netflix/spinnaker/orca/pipelinetemplate/v1schema/graph/transform/ConfigStageInjectionTransform.java @@ -59,7 +59,6 @@ public void visitPipelineTemplate(PipelineTemplate pipelineTemplate) { expandStagePartials(pipelineTemplate); replaceStages(pipelineTemplate); injectStages(pipelineTemplate); - trimConditionals(pipelineTemplate); } private void replaceStages(PipelineTemplate pipelineTemplate) { @@ -139,24 +138,6 @@ private void injectStages(PipelineTemplate pipelineTemplate) { injectStages(templateConfiguration.getStages(), pipelineTemplate.getStages()); } - private void trimConditionals(PipelineTemplate pipelineTemplate) { - // if stage is conditional, ensure children get linked to parents of conditional stage accordingly - pipelineTemplate.getStages() - .stream() - .filter(StageDefinition::getRemoved) - .forEach(conditionalStage -> pipelineTemplate.getStages() - .stream() - .filter(childStage -> childStage.getDependsOn().removeIf(conditionalStage.getId()::equals)) - .forEach(childStage -> childStage.getDependsOn().addAll(conditionalStage.getDependsOn()))); - - pipelineTemplate.setStages( - pipelineTemplate.getStages() - .stream() - .filter(stage -> !stage.getRemoved()) - .collect(Collectors.toList()) - ); - } - private enum Status { VISITING, VISITED diff --git a/orca-pipelinetemplate/src/main/java/com/netflix/spinnaker/orca/pipelinetemplate/v1schema/graph/transform/TrimConditionalsTransform.java b/orca-pipelinetemplate/src/main/java/com/netflix/spinnaker/orca/pipelinetemplate/v1schema/graph/transform/TrimConditionalsTransform.java new file mode 100644 index 0000000000..6e29a532e4 --- /dev/null +++ b/orca-pipelinetemplate/src/main/java/com/netflix/spinnaker/orca/pipelinetemplate/v1schema/graph/transform/TrimConditionalsTransform.java @@ -0,0 +1,34 @@ +package com.netflix.spinnaker.orca.pipelinetemplate.v1schema.graph.transform; + +import com.netflix.spinnaker.orca.pipelinetemplate.v1schema.PipelineTemplateVisitor; +import com.netflix.spinnaker.orca.pipelinetemplate.v1schema.model.PipelineTemplate; +import com.netflix.spinnaker.orca.pipelinetemplate.v1schema.model.StageDefinition; + +import java.util.stream.Collectors; + +public class TrimConditionalsTransform implements PipelineTemplateVisitor { + + @Override + public void visitPipelineTemplate(PipelineTemplate pipelineTemplate) { + trimConditionals(pipelineTemplate); + } + + private void trimConditionals(PipelineTemplate pipelineTemplate) { + // if stage is conditional, ensure children get linked to parents of conditional stage accordingly + pipelineTemplate.getStages() + .stream() + .filter(StageDefinition::getRemoved) + .forEach(conditionalStage -> pipelineTemplate.getStages() + .stream() + .filter(childStage -> childStage.getDependsOn().removeIf(conditionalStage.getId()::equals)) + .forEach(childStage -> childStage.getDependsOn().addAll(conditionalStage.getDependsOn()))); + + pipelineTemplate.setStages( + pipelineTemplate.getStages() + .stream() + .filter(stage -> !stage.getRemoved()) + .collect(Collectors.toList()) + ); + } + +} diff --git a/orca-pipelinetemplate/src/test/groovy/com/netflix/spinnaker/orca/pipelinetemplate/PipelineTemplatePipelinePreprocessorSpec.groovy b/orca-pipelinetemplate/src/test/groovy/com/netflix/spinnaker/orca/pipelinetemplate/PipelineTemplatePipelinePreprocessorSpec.groovy index 71e3989dca..88ddffc738 100644 --- a/orca-pipelinetemplate/src/test/groovy/com/netflix/spinnaker/orca/pipelinetemplate/PipelineTemplatePipelinePreprocessorSpec.groovy +++ b/orca-pipelinetemplate/src/test/groovy/com/netflix/spinnaker/orca/pipelinetemplate/PipelineTemplatePipelinePreprocessorSpec.groovy @@ -319,6 +319,21 @@ class PipelineTemplatePipelinePreprocessorSpec extends Specification { result.stages*.group == ['my group of stages: wowow waiting', 'my group of stages: wowow waiting'] } + @Unroll + def 'should not render falsy conditional stages inside partials'() { + when: + def template = createTemplateRequest('conditional-partials.yml', [includeWait: includeWait]) + def result = subject.process(template) + + then: + result.stages*.name == expectedStageNames + + where: + includeWait || expectedStageNames + true || ['stageWithPartialsAndConditional.wait', 'stageWithPartialsAndConditional.conditionalWaitOnPartial'] + false || ['stageWithPartialsAndConditional.wait'] + } + def "should respect request-defined concurrency options if configuration does not define them"() { given: def pipeline = [ diff --git a/orca-pipelinetemplate/src/test/resources/templates/conditional-partials.yml b/orca-pipelinetemplate/src/test/resources/templates/conditional-partials.yml new file mode 100644 index 0000000000..172b798b67 --- /dev/null +++ b/orca-pipelinetemplate/src/test/resources/templates/conditional-partials.yml @@ -0,0 +1,26 @@ +schema: "1" +id: simpleTemplate +variables: +- name: includeWait + type: boolean +stages: +- id: stageWithPartialsAndConditional + type: partial.partialWithConditional + dependsOn: [] + config: {} + +partials: +- id: partialWithConditional + usage: Partial that conditionally adds a step + variables: [] + stages: + - id: conditionalWaitOnPartial + type: wait + config: + waitTime: 5 + when: + - "{{ includeWait }}" + - id: wait + type: wait + config: + waitTime: 5