Support for sophisticated expressions in when conditionals (issue #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)
+	}
+}