From dfaf809f493399e3021b227b6c190a6013c5cb4e Mon Sep 17 00:00:00 2001 From: jeyrs Date: Tue, 18 Apr 2017 13:42:53 -0700 Subject: [PATCH] fix(pipelinetemplate): Child stages of conditional stages should be preserved - Fixed an issue where children of a rendered conditional stage are dropped off the stages graph --- .../transform/ConditionalStanzaTransform.java | 32 ++++++++----------- .../ConfigStageInjectionTransform.java | 19 +++++++++++ .../v1schema/model/Conditional.java | 2 +- .../v1schema/model/StageDefinition.java | 11 +++++++ ...ineTemplatePipelinePreprocessorSpec.groovy | 18 +++++++++++ .../ConditionalStanzaTransformSpec.groovy | 5 ++- .../conditional-stages-with-children-001.yml | 25 +++++++++++++++ 7 files changed, 89 insertions(+), 23 deletions(-) create mode 100644 orca-pipelinetemplate/src/test/resources/templates/conditional-stages-with-children-001.yml diff --git a/orca-pipelinetemplate/src/main/java/com/netflix/spinnaker/orca/pipelinetemplate/v1schema/graph/transform/ConditionalStanzaTransform.java b/orca-pipelinetemplate/src/main/java/com/netflix/spinnaker/orca/pipelinetemplate/v1schema/graph/transform/ConditionalStanzaTransform.java index a31c945685..84cd1d7f34 100644 --- a/orca-pipelinetemplate/src/main/java/com/netflix/spinnaker/orca/pipelinetemplate/v1schema/graph/transform/ConditionalStanzaTransform.java +++ b/orca-pipelinetemplate/src/main/java/com/netflix/spinnaker/orca/pipelinetemplate/v1schema/graph/transform/ConditionalStanzaTransform.java @@ -25,6 +25,7 @@ import java.util.List; import java.util.Map; +import java.util.Optional; public class ConditionalStanzaTransform implements PipelineTemplateVisitor { @@ -46,25 +47,18 @@ public void visitPipelineTemplate(PipelineTemplate pipelineTemplate) { } private void trimConditionals(List stages, PipelineTemplate template) { - if (stages == null) { - return; - } - - for (T el : stages) { - if (el.getWhen() == null || el.getWhen().isEmpty()) { - continue; - } - - RenderContext context = new DefaultRenderContext(templateConfiguration.getPipeline().getApplication(), template, trigger); - context.getVariables().putAll(templateConfiguration.getPipeline().getVariables()); - - for (String conditional : el.getWhen()) { - String rendered = renderer.render(conditional, context); - if (!Boolean.parseBoolean(rendered)) { - stages.remove(el); - return; + Optional.ofNullable(stages).ifPresent( allStages -> allStages + .stream() + .filter(stage -> stage.getWhen() != null && !stage.getWhen().isEmpty()) + .forEach(stage -> { + RenderContext context = new DefaultRenderContext(templateConfiguration.getPipeline().getApplication(), template, trigger); + context.getVariables().putAll(templateConfiguration.getPipeline().getVariables()); + for (String conditional : stage.getWhen()) { + String rendered = renderer.render(conditional, context); + if (!Boolean.parseBoolean(rendered)) { + stage.setRemove(); + } } - } - } + })); } } 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 d13c3ac994..e289eaf81c 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 @@ -55,6 +55,7 @@ public ConfigStageInjectionTransform(TemplateConfiguration templateConfiguration public void visitPipelineTemplate(PipelineTemplate pipelineTemplate) { replaceStages(pipelineTemplate); injectStages(pipelineTemplate); + trimConditionals(pipelineTemplate); } private void replaceStages(PipelineTemplate pipelineTemplate) { @@ -85,6 +86,24 @@ 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/model/Conditional.java b/orca-pipelinetemplate/src/main/java/com/netflix/spinnaker/orca/pipelinetemplate/v1schema/model/Conditional.java index f586ad72dd..16f49d9d5d 100644 --- a/orca-pipelinetemplate/src/main/java/com/netflix/spinnaker/orca/pipelinetemplate/v1schema/model/Conditional.java +++ b/orca-pipelinetemplate/src/main/java/com/netflix/spinnaker/orca/pipelinetemplate/v1schema/model/Conditional.java @@ -18,6 +18,6 @@ import java.util.List; public interface Conditional { - List getWhen(); + void setRemove(); } diff --git a/orca-pipelinetemplate/src/main/java/com/netflix/spinnaker/orca/pipelinetemplate/v1schema/model/StageDefinition.java b/orca-pipelinetemplate/src/main/java/com/netflix/spinnaker/orca/pipelinetemplate/v1schema/model/StageDefinition.java index 670a2bda82..fb52d12b9c 100644 --- a/orca-pipelinetemplate/src/main/java/com/netflix/spinnaker/orca/pipelinetemplate/v1schema/model/StageDefinition.java +++ b/orca-pipelinetemplate/src/main/java/com/netflix/spinnaker/orca/pipelinetemplate/v1schema/model/StageDefinition.java @@ -33,6 +33,7 @@ public class StageDefinition implements Identifiable, Conditional { private String comments; private List when; private InheritanceControl inheritanceControl; + private Boolean removed = false; private List requisiteStageRefIds = new ArrayList<>(); @@ -218,6 +219,16 @@ public List getWhen() { return when; } + @Override + public void setRemove() { + this.removed = true; + } + + public Boolean getRemoved() { + return removed; + } + + public void setWhen(List when) { this.when = when; } 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 c6d075299c..594fa2deff 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 @@ -175,6 +175,24 @@ class PipelineTemplatePipelinePreprocessorSpec extends Specification { false || ['wait'] } + @Unroll + def 'should preserve children stages of conditional stage'() { + when: + def result = subject.process(createTemplateRequest('conditional-stages-with-children-001.yml', [includeWait: includeWait])) + + then: + result.stages*.name == expectedStageNames + + and: + result.stages.find { it.name == 'childOfConditionalStage' }.requisiteStageRefIds == childStageRequisiteRefIds + + where: + includeWait || childStageRequisiteRefIds || expectedStageNames + true || ['conditionalWait'] || ['wait', 'conditionalWait', 'childOfConditionalStage'] + false || ['wait'] || ['wait', 'childOfConditionalStage'] + } + + Map createTemplateRequest(String templatePath, Map variables = [:], List> stages = [], boolean plan = false) { return [ type: 'templatedPipeline', diff --git a/orca-pipelinetemplate/src/test/groovy/com/netflix/spinnaker/orca/pipelinetemplate/v1schema/graph/transform/ConditionalStanzaTransformSpec.groovy b/orca-pipelinetemplate/src/test/groovy/com/netflix/spinnaker/orca/pipelinetemplate/v1schema/graph/transform/ConditionalStanzaTransformSpec.groovy index f60f52b597..f9d97d5792 100644 --- a/orca-pipelinetemplate/src/test/groovy/com/netflix/spinnaker/orca/pipelinetemplate/v1schema/graph/transform/ConditionalStanzaTransformSpec.groovy +++ b/orca-pipelinetemplate/src/test/groovy/com/netflix/spinnaker/orca/pipelinetemplate/v1schema/graph/transform/ConditionalStanzaTransformSpec.groovy @@ -62,8 +62,7 @@ class ConditionalStanzaTransformSpec extends Specification { then: noExceptionThrown() - template.stages.size() == 1 - template.stages.find { it.id == 's2' } == null - + !template.stages.find { it.id == 's1' }.removed + template.stages.find { it.id == 's2' }.removed } } diff --git a/orca-pipelinetemplate/src/test/resources/templates/conditional-stages-with-children-001.yml b/orca-pipelinetemplate/src/test/resources/templates/conditional-stages-with-children-001.yml new file mode 100644 index 0000000000..7c8dff5601 --- /dev/null +++ b/orca-pipelinetemplate/src/test/resources/templates/conditional-stages-with-children-001.yml @@ -0,0 +1,25 @@ +schema: "1" +id: simpleTemplate +variables: +- name: includeWait + type: boolean +stages: +- id: wait + type: wait + config: + waitTime: 5 +- id: conditionalWait + type: wait + dependsOn: + - wait + config: + waitTime: 5 + when: + # This could also just be '{{ includeWait }}', but examples. + - '{{ includeWait == true }}' +- id: childOfConditionalStage + type: conditionalWait + dependsOn: + - conditionalWait + config: + waitTime: 5