Support for sophisticated expressions in when conditionals (issue a…

…rgoproj#860)

CHANGELOG.md

@@ -5,6 +5,7 @@
 ### Changelog since v2.1
 + Support withItems/withParam and parameter aggregation with DAG templates (issue #801)
 + Add ability to aggregate and reference output parameters expanded by loops (issue #861)
++ Support for sophisticated expressions in `when` conditionals (issue #860)
 + Github login using go-git, with support for ssh keys (@andreimc)
 + Add `argo delete --older` flag to delete completed workflows older than a duration
 + Support submission of workflows from json files (issue #926)

Gopkg.toml

@@ -58,3 +58,7 @@ required = [
 [[constraint]]
  name = "gopkg.in/src-d/go-git.v4"
  version = "4.5.0"
+
+[[constraint]]
+ name = "github.com/Knetic/govaluate"
+ revision = "9aa49832a739dcd78a5542ff189fb82c3e423116"

workflow/controller/steps.go

@@ -3,9 +3,9 @@ package controller
 import (
  "encoding/json"
  "fmt"
- "regexp"
  "strings"
 
+ "github.com/Knetic/govaluate"
  "github.com/argoproj/argo/errors"
  wfv1 "github.com/argoproj/argo/pkg/apis/workflow/v1alpha1"
  "github.com/argoproj/argo/workflow/common"
@@ -226,28 +226,40 @@ func (woc *wfOperationCtx) executeStepGroup(stepGroup []wfv1.WorkflowStep, sgNod
  return woc.markNodePhase(node.Name, wfv1.NodeSucceeded)
 }
 
-var whenExpression = regexp.MustCompile("^(.*)(==|!=)(.*)$")
-
 // shouldExecute evaluates a already substituted when expression to decide whether or not a step should execute
 func shouldExecute(when string) (bool, error) {
  if when == "" {
  return true, nil
  }
- parts := whenExpression.FindStringSubmatch(when)
- if len(parts) == 0 {
- return false, errors.Errorf(errors.CodeBadRequest, "Invalid 'when' expression: %s", when)
+ expression, err := govaluate.NewEvaluableExpression(when)
+ if err != nil {
+ return false, errors.Errorf(errors.CodeBadRequest, "Invalid 'when' expression '%s': %v", when, err)
+ }
+ // The following loop converts govaluate variables (which we don't use), into strings. This
+ // allows us to have expressions like: "foo != bar" without requiring foo and bar to be quoted.
+ tokens := expression.Tokens()
+ for i, tok := range tokens {
+ switch tok.Kind {
+ case govaluate.VARIABLE:
+ tok.Kind = govaluate.STRING
+ default:
+ continue
+ }
+ tokens[i] = tok
+ }
+ expression, err = govaluate.NewEvaluableExpressionFromTokens(tokens)
+ if err != nil {
+ return false, errors.InternalWrapErrorf(err, "Failed to parse 'when' expression '%s': %v", when, err)
+ }
+ result, err := expression.Evaluate(nil)
+ if err != nil {
+ return false, errors.InternalWrapErrorf(err, "Failed to evaluate 'when' expresion '%s': %v", err)
  }
- var1 := strings.TrimSpace(parts[1])
- operator := parts[2]
- var2 := strings.TrimSpace(parts[3])
- switch operator {
- case "==":
- return var1 == var2, nil
- case "!=":
- return var1 != var2, nil
- default:
- return false, errors.Errorf(errors.CodeBadRequest, "Unknown operator: %s", operator)
+ boolRes, ok := result.(bool)
+ if !ok {
+ return false, errors.Errorf(errors.CodeBadRequest, "Expected boolean evaluation for '%s'. Got %v", when, result)
  }
+ return boolRes, nil
 }
 
 // resolveReferences replaces any references to outputs of previous steps, or artifacts in the inputs

workflow/controller/when_test.go

@@ -0,0 +1,49 @@
+package controller
+
+import (
+ "testing"
+
+ "github.com/stretchr/testify/assert"
+)
+
+func TestShouldExecute(t *testing.T) {
+ trueExpressions := []string{
+ "foo == foo",
+ "foo != bar",
+ "1 == 1",
+ "1 != 2",
+ "1 < 2",
+ "1 <= 1",
+ "a < b",
+ "(foo == bar) || (foo == foo)",
+ "(1 > 0) && (1 < 2)",
+ "Error in (Failed, Error)",
+ "!(Succeeded in (Failed, Error))",
+ "true == true",
+ }
+ for _, trueExp := range trueExpressions {
+ res, err := shouldExecute(trueExp)
+ assert.Nil(t, err)
+ assert.True(t, res)
+ }
+
+ falseExpressions := []string{
+ "foo != foo",
+ "foo == bar",
+ "1 != 1",
+ "1 == 2",
+ "1 > 2",
+ "1 <= 0",
+ "a > b",
+ "(foo == bar) || (bar == foo)",
+ "(1 > 0) && (11 < 2)",
+ "Succeeded in (Failed, Error)",
+ "!(Error in (Failed, Error))",
+ "false == true",
+ }
+ for _, falseExp := range falseExpressions {
+ res, err := shouldExecute(falseExp)
+ assert.Nil(t, err)
+ assert.False(t, res)
+ }
+}