fix(pipelinetemplate): Child stages of conditional stages should be p…

…reserved

- Fixed an issue where children of a rendered conditional stage are dropped off the stages graph

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 <T extends Conditional> void trimConditionals(List<T> 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();
+          }
         }
-      }
-    }
+      }));
   }
 }

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

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<String> getWhen();
+  void setRemove();
 }

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<String> when;
   private InheritanceControl inheritanceControl;
+  private Boolean removed = false;
 
   private List<String> requisiteStageRefIds = new ArrayList<>();
 
@@ -218,6 +219,16 @@ public List<String> getWhen() {
     return when;
   }
 
+  @Override
+  public void setRemove() {
+    this.removed = true;
+  }
+
+  public Boolean getRemoved() {
+    return removed;
+  }
+
+
   public void setWhen(List<String> when) {
     this.when = when;
   }

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<String, Object> createTemplateRequest(String templatePath, Map<String, Object> variables = [:], List<Map<String, Object>> stages = [], boolean plan = false) {
     return [
       type: 'templatedPipeline',

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
   }
 }

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