fix(pipeline_template): Build partials, execute conditionals after an…

…d trim conditions as the last step (#1602)
spinnaker · Sep 7, 2017 · 33cdfd5 · 33cdfd5
1 parent b1a8a56
commit 33cdfd5
Show file tree

Hide file tree

Showing 5 changed files with 78 additions and 20 deletions.
diff --git a/...rc/main/java/com/netflix/spinnaker/orca/pipelinetemplate/v1schema/graph/GraphMutator.java b/...rc/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<String, Object> 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/...innaker/orca/pipelinetemplate/v1schema/graph/transform/ConfigStageInjectionTransform.java b/...innaker/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/...x/spinnaker/orca/pipelinetemplate/v1schema/graph/transform/TrimConditionalsTransform.java b/...x/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/...m/netflix/spinnaker/orca/pipelinetemplate/PipelineTemplatePipelinePreprocessorSpec.groovy b/...m/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
@@ -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