feat(api): added full trigger validation in api

api/dto/triggers.go

@@ -178,7 +178,7 @@ func (trigger *Trigger) Bind(request *http.Request) error {
 		return err
 	}
 
-	if err := checkTTLSanity(trigger, metricsSource); err != nil {
+	if err = checkTTLSanity(trigger, metricsSource); err != nil {
 		return api.ErrInvalidRequestContent{ValidationError: err}
 	}
 
@@ -195,7 +195,7 @@ func (trigger *Trigger) Bind(request *http.Request) error {
 
 	middleware.SetTimeSeriesNames(request, metricsDataNames)
 
-	if _, err := triggerExpression.Evaluate(); err != nil {
+	if _, err = triggerExpression.Evaluate(); err != nil {
 		return err
 	}
 

expression/expression.go

@@ -4,9 +4,12 @@ import (
 	"fmt"
 	"strings"
 
+	"github.com/antonmedv/expr"
+	"github.com/antonmedv/expr/vm"
 	"github.com/patrickmn/go-cache"
 
 	"github.com/Knetic/govaluate"
+
 	"github.com/moira-alert/moira"
 )
 
@@ -95,13 +98,62 @@ func (triggerExpression *TriggerExpression) Evaluate() (moira.State, error) {
 	}
 }
 
-func validateUserExpression(triggerExpression *TriggerExpression, userExpression *govaluate.EvaluableExpression) (*govaluate.EvaluableExpression, error) {
-	for _, v := range userExpression.Vars() {
-		if _, err := triggerExpression.Get(v); err != nil {
-			return nil, fmt.Errorf("invalid variable value: %w", err)
+// validateUserExpression uses expr package to validate <user defined> trigger expressions
+func validateUserExpression(
+	triggerExpression *TriggerExpression,
+	userExpression *govaluate.EvaluableExpression,
+) (*govaluate.EvaluableExpression, error) {
+	expression := *triggerExpression.Expression
+
+	cacheKey := fmt.Sprintf("[VALIDATED]%s", expression)
+	pr, validated := exprCache.Get(cacheKey)
+	if validated {
+		_, ok := pr.(*vm.Program)
+		if ok {
+			return userExpression, nil
+		} else {
+			validated = false
+			exprCache.Delete(cacheKey)
 		}
 	}
-	return userExpression, nil
+
+	env := map[string]interface{}{
+		"ok":         moira.StateOK,
+		"error":      moira.StateERROR,
+		"warn":       moira.StateWARN,
+		"warning":    moira.StateWARN,
+		"nodata":     moira.StateNODATA,
+		"t1":         triggerExpression.MainTargetValue,
+		"prev_state": triggerExpression.PreviousState,
+	}
+	if triggerExpression.WarnValue != nil {
+		env["warn_value"] = *triggerExpression.WarnValue
+	}
+	if triggerExpression.ErrorValue != nil {
+		env["error_value"] = *triggerExpression.ErrorValue
+	}
+	for k, v := range triggerExpression.AdditionalTargetsValues {
+		env[k] = v
+	}
+	program, err := expr.Compile(
+		strings.ToLower(expression),
+		expr.Optimize(true),
+		expr.Env(env),
+	)
+	if err != nil {
+		return nil, err
+	}
+	result, err := expr.Run(program, env)
+	if err != nil {
+		return nil, err
+	}
+	switch result.(type) {
+	case moira.State:
+		exprCache.Set(cacheKey, program, cache.NoExpiration)
+		return userExpression, nil
+	default:
+		return nil, ErrInvalidExpression{internalError: fmt.Errorf("expression result must be state value")}
+	}
 }
 
 func getExpression(triggerExpression *TriggerExpression) (*govaluate.EvaluableExpression, error) {
@@ -114,7 +166,6 @@ func getExpression(triggerExpression *TriggerExpression) (*govaluate.EvaluableEx
 		if err != nil {
 			return nil, err
 		}
-
 		return validateUserExpression(triggerExpression, userExpression)
 	}
 	return getSimpleExpression(triggerExpression)

expression/expression_test.go

@@ -4,8 +4,9 @@ import (
 	"fmt"
 	"testing"
 
-	"github.com/moira-alert/moira"
 	. "github.com/smartystreets/goconvey/convey"
+
+	"github.com/moira-alert/moira"
 )
 
 type getExpressionValuesTest struct {
@@ -89,12 +90,16 @@ func TestExpression(t *testing.T) {
 
 		expression = "t1 > 10 && t2 > 3 ? OK : ddd"
 		result, err = (&TriggerExpression{Expression: &expression, MainTargetValue: 11.0, AdditionalTargetsValues: map[string]float64{"t2": 4.0}, TriggerType: moira.ExpressionTrigger}).Evaluate()
-		So(err, ShouldResemble, ErrInvalidExpression{fmt.Errorf("invalid variable value: %w", fmt.Errorf("no value with name ddd"))})
+		So(err.Error(), ShouldResemble, `unknown name ddd (1:26)
+ | t1 > 10 && t2 > 3 ? ok : ddd
+ | .........................^`)
 		So(result, ShouldBeEmpty)
 
 		expression = "t1 > 10 ? OK : (t2 < 5 ? WARN : ERROR)"
 		result, err = (&TriggerExpression{Expression: &expression, MainTargetValue: 11.0, AdditionalTargetsValues: map[string]float64{}, TriggerType: moira.ExpressionTrigger}).Evaluate()
-		So(err, ShouldResemble, ErrInvalidExpression{fmt.Errorf("invalid variable value: %w", fmt.Errorf("no value with name t2"))})
+		So(err.Error(), ShouldResemble, `unknown name t2 (1:17)
+ | t1 > 10 ? ok : (t2 < 5 ? warn : error)
+ | ................^`)
 		So(result, ShouldBeEmpty)
 	})
 }

go.mod

@@ -6,6 +6,7 @@ require (
 	github.com/Knetic/govaluate v3.0.1-0.20171022003610-9aa49832a739+incompatible
 	github.com/PagerDuty/go-pagerduty v1.5.1
 	github.com/ansel1/merry v1.6.2
+	github.com/antonmedv/expr v1.12.7
 	github.com/aws/aws-sdk-go v1.44.219
 	github.com/blevesearch/bleve/v2 v2.3.8
 	github.com/bwmarrin/discordgo v0.25.0
@@ -27,6 +28,7 @@ require (
 	github.com/gregdel/pushover v1.1.0
 	github.com/karriereat/blackfriday-slack v0.1.0
 	github.com/mattermost/mattermost-server/v6 v6.0.0-20230405170428-2a75f997ee6c // it is last commit of 7.9.2 (https://github.com/mattermost/mattermost-server/commits/v7.9.2). Can't use v7, issue https://github.com/mattermost/mattermost-server/issues/20817
+	github.com/mitchellh/mapstructure v1.5.0
 	github.com/moira-alert/go-chart v0.0.0-20230220064910-812fb2829b9b
 	github.com/opsgenie/opsgenie-go-sdk-v2 v1.2.13
 	github.com/patrickmn/go-cache v2.1.0+incompatible
@@ -47,8 +49,6 @@ require (
 	gopkg.in/yaml.v2 v2.4.0
 )
 
-require github.com/mitchellh/mapstructure v1.5.0
-
 require (
 	bitbucket.org/tebeka/strftime v0.0.0-20140926081919-2194253a23c0 // indirect
 	github.com/JaderDias/movingmedian v0.0.0-20220813210630-d8c6b6de8835 // indirect

go.sum

@@ -438,13 +438,14 @@ github.com/ansel1/merry/v2 v2.1.1 h1:Ax0gQh7Z/GfimoVg2EDBAU6CJIieWwVvhtBKJdkCE1M
 github.com/ansel1/merry/v2 v2.1.1/go.mod h1:4p/FFyQbCgqlDbseWOVQaL5USpgkE9sr5xh4V6Ry0JU=
 github.com/ansel1/vespucci/v4 v4.1.1/go.mod h1:zzdrO4IgBfgcGMbGTk/qNGL8JPslmW3nPpcBHKReFYY=
 github.com/antihax/optional v1.0.0/go.mod h1:uupD/76wgC+ih3iEmQUL+0Ugr19nfwCT1kdvxnR2qWY=
+github.com/antonmedv/expr v1.12.7 h1:jfV/l/+dHWAadLwAtESXNxXdfbK9bE4+FNMHYCMntwk=
+github.com/antonmedv/expr v1.12.7/go.mod h1:FPC8iWArxls7axbVLsW+kpg1mz29A1b2M6jt+hZfDkU=
 github.com/armon/go-radix v0.0.0-20180808171621-7fddfc383310/go.mod h1:ufUuZ+zHj4x4TnLV4JWEpy2hxWSpsRywHrMgIH9cCH8=
 github.com/aws/aws-sdk-go v1.44.219 h1:YOFxTUQZvdRzgwb6XqLFRwNHxoUdKBuunITC7IFhvbc=
 github.com/aws/aws-sdk-go v1.44.219/go.mod h1:aVsgQcEevwlmQ7qHE9I3h+dtQgpqhFB+i8Phjh7fkwI=
 github.com/barkimedes/go-deepcopy v0.0.0-20220514131651-17c30cfc62df h1:GSoSVRLoBaFpOOds6QyY1L8AX7uoY+Ln3BHc22W40X0=
 github.com/barkimedes/go-deepcopy v0.0.0-20220514131651-17c30cfc62df/go.mod h1:hiVxq5OP2bUGBRNS3Z/bt/reCLFNbdcST6gISi1fiOM=
 github.com/benbjohnson/clock v1.1.0 h1:Q92kusRqC1XV2MjkWETPvjJVqKetz1OzxZB7mHJLju8=
-github.com/benbjohnson/clock v1.1.0/go.mod h1:J11/hYXuz8f4ySSvYwY0FKfm+ezbsZBKZxNJlLklBHA=
 github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973/go.mod h1:Dwedo/Wpr24TaqPxmxbtue+5NUziq4I4S80YR8gNf3Q=
 github.com/beorn7/perks v1.0.0/go.mod h1:KWe93zE9D1o94FZ5RNwFwVgaQK1VOXiVxmqh+CedLV8=
 github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM=
@@ -689,7 +690,6 @@ github.com/google/go-querystring v1.0.0/go.mod h1:odCYkC5MyYFN7vkCjXpyrEuKhc/BUO
 github.com/google/go-querystring v1.1.0 h1:AnCroh3fv4ZBgVIf1Iwtovgjaw/GiKJo8M8yD/fhyJ8=
 github.com/google/go-querystring v1.1.0/go.mod h1:Kcdr2DB4koayq7X8pmAG4sNG59So17icRSOU623lUBU=
 github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg=
-github.com/google/gofuzz v1.2.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg=
 github.com/google/martian v2.1.0+incompatible/go.mod h1:9I4somxYTbIHy5NJKHRl3wXiIaQGbYVAs8BPL6v8lEs=
 github.com/google/martian/v3 v3.0.0/go.mod h1:y5Zk1BBys9G+gd6Jrk0W3cC1+ELVxBWuIGO+w/tUAp0=
 github.com/google/martian/v3 v3.1.0/go.mod h1:y5Zk1BBys9G+gd6Jrk0W3cC1+ELVxBWuIGO+w/tUAp0=
@@ -1111,7 +1111,6 @@ go.uber.org/atomic v1.10.0/go.mod h1:LUxbIzbOniOlMKjJjyPfpl4v+PKK2cNJn91OQbhoJI0
 go.uber.org/automaxprocs v1.5.1 h1:e1YG66Lrk73dn4qhg8WFSvhF0JuFQF0ERIp4rpuV8Qk=
 go.uber.org/automaxprocs v1.5.1/go.mod h1:BF4eumQw0P9GtnuxxovUd06vwm1o18oMzFtK66vU6XU=
 go.uber.org/goleak v1.1.11 h1:wy28qYRKZgnJTxGxvye5/wgWr1EKjmUDGYox5mGlRlI=
-go.uber.org/goleak v1.1.11/go.mod h1:cwTWslyiVhfpKIDGSZEM2HlOvcqm+tG4zioyIeLoqMQ=
 go.uber.org/multierr v1.6.0/go.mod h1:cdWPpRnG4AhwMwsgIHip0KRBQjJy5kYEpYjJxpXp9iU=
 go.uber.org/multierr v1.8.0 h1:dg6GjLku4EH+249NNmoIciG9N/jURbDG+pFlTkhzIC8=
 go.uber.org/multierr v1.8.0/go.mod h1:7EAYxJLBy9rStEaz58O2t4Uvip6FSURkq8/ppBp95ak=

perfomance_tests/expression/expression_test.go

@@ -3,6 +3,7 @@ package expression
 import (
 	"testing"
 
+	"github.com/moira-alert/moira"
 	"github.com/moira-alert/moira/expression"
 )
 
@@ -13,9 +14,13 @@ func BenchmarkDefault1Expr(b *testing.B) {
 		MainTargetValue: 10.0,
 		WarnValue:       &warnValue,
 		ErrorValue:      &errorValue,
+		TriggerType:     moira.RisingTrigger,
 	}
 	for i := 0; i < b.N; i++ {
-		(expr).Evaluate() //nolint
+		_, err := expr.Evaluate()
+		if err != nil {
+			b.Log(err)
+		}
 	}
 }
 
@@ -26,19 +31,52 @@ func BenchmarkDefault2Expr(b *testing.B) {
 		MainTargetValue: 10.0,
 		WarnValue:       &warnValue,
 		ErrorValue:      &errorValue,
+		TriggerType:     moira.RisingTrigger,
 	}
 	for i := 0; i < b.N; i++ {
-		(expr).Evaluate() //nolint
+		_, err := expr.Evaluate()
+		if err != nil {
+			b.Log(err)
+		}
 	}
 }
 
 func BenchmarkCustomExpr(b *testing.B) {
 	expressionStr := "t1 > 10 && t2 > 3 ? ERROR : OK"
 	expr := &expression.TriggerExpression{
 		Expression:              &expressionStr,
+		TriggerType:             moira.ExpressionTrigger,
 		MainTargetValue:         11.0,
 		AdditionalTargetsValues: map[string]float64{"t2": 4.0}}
 	for i := 0; i < b.N; i++ {
-		(expr).Evaluate() //nolint
+		_, err := expr.Evaluate()
+		if err != nil {
+			b.Log(err)
+		}
+	}
+}
+
+func BenchmarkEvaluateComplex(b *testing.B) {
+	expressionStr := "(t1 * 2 > t2 && t2 / 2 != 0) || (t3 * t4 == t5 && t6 < t7) ? (t8 > t9 ? OK : WARN) : ERROR"
+	expr := &expression.TriggerExpression{
+		Expression:      &expressionStr,
+		TriggerType:     moira.ExpressionTrigger,
+		MainTargetValue: 4,
+		AdditionalTargetsValues: map[string]float64{
+			"t2": 5,
+			"t3": 3,
+			"t4": 6,
+			"t5": 18,
+			"t6": 10,
+			"t7": 15,
+			"t8": 20,
+			"t9": 10,
+		},
+	}
+	for i := 0; i < b.N; i++ {
+		_, err := expr.Evaluate()
+		if err != nil {
+			b.Log(err)
+		}
 	}
 }