fix(expressions): various quality of life improvements for SpEL (#3272)

* fix(expressions): various quality of life improvements for SpEL

* Let `#currentStage` be used in `stageEnabled` conditions
  This requires `.withAuth` on the the entire StartStageHander::handle because the `currentStage` function relies on the execution to be present in the threadlocal (which is placed there by ``.withAuth`)

* Allow users to compare `ExecutionStatus` to string, e.g. this is now valid and works `${#stage('bake').status == 'SUCCEEDED'}`
  in the past one would have to `${#stage('bake').status.toString() == 'SUCCEEDED'}` which is annoying

* Furthermore, added `hasSucceeded` and `hasFailed` to stage
  This further simplifies above conditionals to `${#stage('bake').hasSucceeded}` & `${#stage('bake').hasFailed}`

* fixup! fix(expressions): various quality of life improvements for SpEL

orca-clouddriver/src/test/java/com/netflix/spinnaker/orca/clouddriver/tasks/providers/cf/AbstractCloudFoundryWaitForServiceOperationTaskTest.java

@@ -68,6 +68,6 @@ void testOortServiceStatus(
     TaskResult result =
         task.execute(new Stage(new Execution(PIPELINE, "orca"), operationType, context));
 
-    assertThat(result.getStatus()).isEqualTo(expectedStatus);
+    assertThat(result.getStatus().toString()).isEqualTo(expectedStatus.toString());
   }
 }

orca-core/src/main/java/com/netflix/spinnaker/orca/ExecutionStatus.java

@@ -20,7 +20,11 @@
 import java.util.Collection;
 import java.util.Collections;
 
-public enum ExecutionStatus {
+/**
+ * Execution Status enumerations used for executions and stages Note: we implement CharSequence to
+ * allow simple SpEL expressions such as: {#stage('s1').status == 'SUCCEEDED'}
+ */
+public enum ExecutionStatus implements CharSequence {
   /** The task has yet to start. */
   NOT_STARTED(false, false),
 
@@ -93,4 +97,19 @@ public boolean isSuccessful() {
   public boolean isFailure() {
     return FAILURE.contains(this);
   }
+
+  @Override
+  public int length() {
+    return toString().length();
+  }
+
+  @Override
+  public char charAt(int index) {
+    return toString().charAt(index);
+  }
+
+  @Override
+  public CharSequence subSequence(int start, int end) {
+    return toString().subSequence(start, end);
+  }
 }

orca-core/src/main/java/com/netflix/spinnaker/orca/pipeline/expressions/functions/StageExpressionFunctionProvider.java

@@ -91,7 +91,7 @@ public static Stage currentStage(Execution execution) {
    * @param refId the stage reference ID
    * @return a stage specified by refId
    */
-  static Object stageByRefId(Execution execution, String refId) {
+  public static Stage stageByRefId(Execution execution, String refId) {
     if (refId == null) {
       throw new SpelHelperFunctionException(
           format(
@@ -115,7 +115,7 @@ static Object stageByRefId(Execution execution, String refId) {
    * @param id the name or id of the stage to find
    * @return a stage specified by id
    */
-  public static Object stage(Execution execution, String id) {
+  public static Stage stage(Execution execution, String id) {
     return execution.getStages().stream()
         .filter(i -> id != null && (id.equals(i.getName()) || id.equals(i.getId())))
         .findFirst()

orca-core/src/main/java/com/netflix/spinnaker/orca/pipeline/model/Stage.java

@@ -852,5 +852,25 @@ public String toString() {
     return "Stage {id='" + id + "', executionId='" + execution.getId() + "'}";
   }
 
+  /**
+   * NOTE: this function is mostly for convenience to endusers using SpEL
+   *
+   * @return true if stage has succeeded
+   */
+  @JsonIgnore
+  public boolean getHasSucceeded() {
+    return (status == ExecutionStatus.SUCCEEDED);
+  }
+
+  /**
+   * NOTE: this function is mostly for convenience to endusers using SpEL
+   *
+   * @return true if stage has failed
+   */
+  @JsonIgnore
+  public boolean getHasFailed() {
+    return (status == ExecutionStatus.TERMINAL);
+  }
+
   public static final String STAGE_TIMEOUT_OVERRIDE_KEY = "stageTimeoutMs";
 }

orca-core/src/test/groovy/com/netflix/spinnaker/orca/pipeline/expressions/PipelineExpressionEvaluatorSpec.groovy

@@ -16,7 +16,9 @@
 
 package com.netflix.spinnaker.orca.pipeline.expressions
 
+import com.netflix.spinnaker.kork.expressions.ExpressionEvaluationSummary
 import com.netflix.spinnaker.kork.expressions.ExpressionFunctionProvider
+import com.netflix.spinnaker.orca.ExecutionStatus
 import com.netflix.spinnaker.orca.pipeline.model.Execution
 import org.pf4j.PluginManager
 import spock.lang.Specification
@@ -45,6 +47,26 @@ class PipelineExpressionEvaluatorSpec extends Specification {
     evaluator.getExecutionAwareFunctions().findAll { it.contains('functionWithExecutionContext') }.size() == 2
   }
 
+  def "should allow comparing ExecutionStatus to string"() {
+    given:
+    def source = [test: testCase]
+    PipelineExpressionEvaluator evaluator = new PipelineExpressionEvaluator([], pluginManager)
+
+    when:
+    ExpressionEvaluationSummary evaluationSummary = new ExpressionEvaluationSummary()
+    def result = evaluator.evaluate(source, [status: ExecutionStatus.TERMINAL], evaluationSummary, true)
+
+    then:
+    result.test == evalResult
+
+    where:
+    testCase                                    | evalResult
+    '${status.toString() == "TERMINAL"}'        | true
+    '${status == "TERMINAL"}'                   | true
+    '${status.toString() == "SUCCEEDED"}'       | false
+    '${status == "SUCCEEDED"}'                  | false
+  }
+
   private ExpressionFunctionProvider buildExpressionFunctionProvider(String providerName) {
     new ExpressionFunctionProvider() {
       @Override

orca-core/src/test/groovy/com/netflix/spinnaker/orca/pipeline/util/ContextParameterProcessorSpec.groovy

@@ -29,6 +29,7 @@ import spock.lang.Subject
 import spock.lang.Unroll
 
 import static com.netflix.spinnaker.orca.ExecutionStatus.SUCCEEDED
+import static com.netflix.spinnaker.orca.ExecutionStatus.TERMINAL
 import static com.netflix.spinnaker.orca.test.model.ExecutionBuilder.pipeline
 import static com.netflix.spinnaker.orca.test.model.ExecutionBuilder.stage
 
@@ -783,7 +784,7 @@ class ContextParameterProcessorSpec extends Specification {
 
   }
 
-  def "can find a stage in an execution"() {
+  def "can find a stage in an execution and get its status"() {
     given:
     def pipe = pipeline {
       stage {
@@ -798,18 +799,34 @@ class ContextParameterProcessorSpec extends Specification {
         refId = "2"
         requisiteStageRefIds = ["1"]
         context.waitTime = 1
-        context.comments = '${#stage("Wait1")["status"].toString()}'
+        context.comments = 'succeeded: ${#stage("Wait1")["status"] == "SUCCEEDED"}'
+        context.succeeded = '${#stage("Wait1").hasSucceeded}'
+        context.failed = '${#stage("Wait1").hasFailed}'
       }
     }
 
-    def stage = pipe.stageByRef("2")
-    def ctx = contextParameterProcessor.buildExecutionContext(stage)
+    def stage1 = pipe.stageByRef("1")
+    stage1.setStatus(SUCCEEDED)
+
+    def stage2 = pipe.stageByRef("2")
+    def ctx = contextParameterProcessor.buildExecutionContext(stage2)
 
     when:
-    def result = contextParameterProcessor.process(stage.context, ctx, true)
+    def result = contextParameterProcessor.process(stage2.context, ctx, true)
+
+    then:
+    result.comments == "succeeded: true"
+    result.succeeded == true
+    result.failed == false
+
+    when:
+    stage1.setStatus(TERMINAL)
+    result = contextParameterProcessor.process(stage2.context, ctx, true)
 
     then:
-    result.comments == "NOT_STARTED"
+    result.comments == "succeeded: false"
+    result.succeeded == false
+    result.failed == true
   }
 
   def "can not toJson an execution with expressions in the context"() {

orca-queue/src/main/kotlin/com/netflix/spinnaker/orca/q/handler/StartStageHandler.kt

@@ -74,56 +74,55 @@ class StartStageHandler(
 
   override fun handle(message: StartStage) {
     message.withStage { stage ->
-      if (stage.anyUpstreamStagesFailed()) {
-        // this only happens in restart scenarios
-        log.warn("Tried to start stage ${stage.id} but something upstream had failed (executionId: ${message.executionId})")
-        queue.push(CompleteExecution(message))
-      } else if (stage.allUpstreamStagesComplete()) {
-        if (stage.status != NOT_STARTED) {
-          log.warn("Ignoring $message as stage is already ${stage.status}")
-        } else if (stage.shouldSkip()) {
-          queue.push(SkipStage(message))
-        } else if (stage.isAfterStartTimeExpiry()) {
-          log.warn("Stage is being skipped because its start time is after TTL (stageId: ${stage.id}, executionId: ${message.executionId})")
-          queue.push(SkipStage(stage))
-        } else {
-          try {
-            // Set the startTime in case we throw an exception.
-            stage.startTime = clock.millis()
-            repository.storeStage(stage)
-
-            stage.withAuth {
+      stage.withAuth {
+        if (stage.anyUpstreamStagesFailed()) {
+          // this only happens in restart scenarios
+          log.warn("Tried to start stage ${stage.id} but something upstream had failed (executionId: ${message.executionId})")
+          queue.push(CompleteExecution(message))
+        } else if (stage.allUpstreamStagesComplete()) {
+          if (stage.status != NOT_STARTED) {
+            log.warn("Ignoring $message as stage is already ${stage.status}")
+          } else if (stage.shouldSkip()) {
+            queue.push(SkipStage(message))
+          } else if (stage.isAfterStartTimeExpiry()) {
+            log.warn("Stage is being skipped because its start time is after TTL (stageId: ${stage.id}, executionId: ${message.executionId})")
+            queue.push(SkipStage(stage))
+          } else {
+            try {
+              // Set the startTime in case we throw an exception.
+              stage.startTime = clock.millis()
+              repository.storeStage(stage)
               stage.plan()
-            }
-            stage.status = RUNNING
-            repository.storeStage(stage)
-
-            stage.start()
-
-            publisher.publishEvent(StageStarted(this, stage))
-            trackResult(stage)
-          } catch (e: Exception) {
-            val exceptionDetails = exceptionHandlers.shouldRetry(e, stage.name)
-            if (exceptionDetails?.shouldRetry == true) {
-              val attempts = message.getAttribute<AttemptsAttribute>()?.attempts ?: 0
-              log.warn("Error planning ${stage.type} stage for ${message.executionType}[${message.executionId}] (attempts: $attempts)")
-
-              message.setAttribute(MaxAttemptsAttribute(40))
-              queue.push(message, retryDelay)
-            } else {
-              log.error("Error running ${stage.type} stage for ${message.executionType}[${message.executionId}]", e)
-              stage.apply {
-                context["exception"] = exceptionDetails
-                context["beforeStagePlanningFailed"] = true
-              }
+              stage.status = RUNNING
               repository.storeStage(stage)
-              queue.push(CompleteStage(message))
+
+              stage.start()
+
+              publisher.publishEvent(StageStarted(this, stage))
+              trackResult(stage)
+            } catch (e: Exception) {
+              val exceptionDetails = exceptionHandlers.shouldRetry(e, stage.name)
+              if (exceptionDetails?.shouldRetry == true) {
+                val attempts = message.getAttribute<AttemptsAttribute>()?.attempts ?: 0
+                log.warn("Error planning ${stage.type} stage for ${message.executionType}[${message.executionId}] (attempts: $attempts)")
+
+                message.setAttribute(MaxAttemptsAttribute(40))
+                queue.push(message, retryDelay)
+              } else {
+                log.error("Error running ${stage.type} stage for ${message.executionType}[${message.executionId}]", e)
+                stage.apply {
+                  context["exception"] = exceptionDetails
+                  context["beforeStagePlanningFailed"] = true
+                }
+                repository.storeStage(stage)
+                queue.push(CompleteStage(message))
+              }
             }
           }
+        } else {
+          log.info("Re-queuing $message as upstream stages are not yet complete")
+          queue.push(message, retryDelay)
         }
-      } else {
-        log.info("Re-queuing $message as upstream stages are not yet complete")
-        queue.push(message, retryDelay)
       }
     }
   }