From 5477b529915dd739e7c3496bf25cb023435fb7c6 Mon Sep 17 00:00:00 2001
From: Dmytro Kozlov <kozlovdmitriyy@gmail.com>
Date: Fri, 15 Sep 2023 13:15:23 +0200
Subject: [PATCH] vmagent: add validation of MetricsQL functions (#4991)

Co-authored-by: Aliaksandr Valialkin <valyala@victoriametrics.com>
---
 app/vmselect/promql/exec_test.go              |  2 +-
 docs/CHANGELOG.md                             |  1 +
 go.mod                                        |  2 +-
 go.sum                                        |  4 +-
 .../VictoriaMetrics/metricsql/aggr.go         |  3 +-
 .../VictoriaMetrics/metricsql/optimizer.go    |  2 +-
 .../VictoriaMetrics/metricsql/parser.go       |  9 ++--
 .../VictoriaMetrics/metricsql/utils.go        | 42 ++++++++++++++++++-
 vendor/modules.txt                            |  2 +-
 9 files changed, 56 insertions(+), 11 deletions(-)

diff --git a/app/vmselect/promql/exec_test.go b/app/vmselect/promql/exec_test.go
index f1562864ae..700b8e8164 100644
--- a/app/vmselect/promql/exec_test.go
+++ b/app/vmselect/promql/exec_test.go
@@ -46,7 +46,7 @@ func TestEscapeDotsInRegexpLabelFilters(t *testing.T) {
 	f("2", "2")
 	f(`foo.bar + 123`, `foo.bar + 123`)
 	f(`foo{bar=~"baz.xx.yyy"}`, `foo{bar=~"baz\\.xx\\.yyy"}`)
-	f(`foo(a.b{c="d.e",x=~"a.b.+[.a]",y!~"aaa.bb|cc.dd"}) + x.y(1,sum({x=~"aa.bb"}))`, `foo(a.b{c="d.e",x=~"a\\.b.+[\\.a]",y!~"aaa\\.bb|cc\\.dd"}) + x.y(1, sum({x=~"aa\\.bb"}))`)
+	f(`sum(a.b{c="d.e",x=~"a.b.+[.a]",y!~"aaa.bb|cc.dd"}) + avg_over_time(1,sum({x=~"aa.bb"}))`, `sum(a.b{c="d.e",x=~"a\\.b.+[\\.a]",y!~"aaa\\.bb|cc\\.dd"}) + avg_over_time(1, sum({x=~"aa\\.bb"}))`)
 }
 
 func TestExecSuccess(t *testing.T) {
diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md
index 115b61ac21..30c8178794 100644
--- a/docs/CHANGELOG.md
+++ b/docs/CHANGELOG.md
@@ -42,6 +42,7 @@ The following `tip` changes can be tested by building VictoriaMetrics components
 ssue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4825) and [these docs](https://docs.victoriametrics.com/vmauth.html#auth-config) for details.
 * FEATURE: [vmauth](https://docs.victoriametrics.com/vmauth.html): add ability to retry requests to the [remaining backends](https://docs.victoriametrics.com/vmauth.html#load-balancing) if they return response status codes specified in the `retry_status_codes` list. See [this feature request](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4893).
 * FEATURE: [vmalert](https://docs.victoriametrics.com/vmalert.html): add `eval_offset` attribute for [Groups](https://docs.victoriametrics.com/vmalert.html#groups). If specified, Group will be evaluated at the exact time offset on the range of [0...evaluationInterval]. The setting might be useful for cron-like rules which must be evaluated at specific moments of time. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3409) for details.
+* FEATURE: [vmalert](https://docs.victoriametrics.com/vmalert.html): validate [MetricsQL](https://docs.victoriametrics.com/MetricsQL.html) function names in alerting and recording rules when `vmalert` runs with `-dryRun` command-line flag. Previously it was allowed to use unknown (aka invalid) MetricsQL function names there. For example, `foo()` was counted as a valid query. See [this feature request](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4933).
 * FEATURE: limit the length of string params in log messages to 500 chars. Longer string params are replaced with the `first_250_chars..last_250_chars`. This prevents from too long log lines, which can be emitted by VictoriaMetrics components.
 
 * BUGFIX: [Official Grafana dashboards for VictoriaMetrics](https://grafana.com/orgs/victoriametrics): fix display of ingested rows rate for `Samples ingested/s` and `Samples rate` panels for vmagent's dasbhoard. Previously, not all ingested protocols were accounted in these panels. An extra panel `Rows rate` was added to `Ingestion` section to display the split for rows ingested rate by protocol.
diff --git a/go.mod b/go.mod
index 603c4a5678..a6ee13e10e 100644
--- a/go.mod
+++ b/go.mod
@@ -12,7 +12,7 @@ require (
 	// like https://github.com/valyala/fasthttp/commit/996610f021ff45fdc98c2ce7884d5fa4e7f9199b
 	github.com/VictoriaMetrics/fasthttp v1.2.0
 	github.com/VictoriaMetrics/metrics v1.24.0
-	github.com/VictoriaMetrics/metricsql v0.64.0
+	github.com/VictoriaMetrics/metricsql v0.65.0
 	github.com/aws/aws-sdk-go-v2 v1.21.0
 	github.com/aws/aws-sdk-go-v2/config v1.18.39
 	github.com/aws/aws-sdk-go-v2/feature/s3/manager v1.11.83
diff --git a/go.sum b/go.sum
index 0b5db09a06..9bfe01c62c 100644
--- a/go.sum
+++ b/go.sum
@@ -70,8 +70,8 @@ github.com/VictoriaMetrics/fasthttp v1.2.0 h1:nd9Wng4DlNtaI27WlYh5mGXCJOmee/2c2b
 github.com/VictoriaMetrics/fasthttp v1.2.0/go.mod h1:zv5YSmasAoSyv8sBVexfArzFDIGGTN4TfCKAtAw7IfE=
 github.com/VictoriaMetrics/metrics v1.24.0 h1:ILavebReOjYctAGY5QU2F9X0MYvkcrG3aEn2RKa1Zkw=
 github.com/VictoriaMetrics/metrics v1.24.0/go.mod h1:eFT25kvsTidQFHb6U0oa0rTrDRdz4xTYjpL8+UPohys=
-github.com/VictoriaMetrics/metricsql v0.64.0 h1:uty6AXQFY3OpQ+eopo1jDjCcTctuqkqYLnRbQVhukW8=
-github.com/VictoriaMetrics/metricsql v0.64.0/go.mod h1:k4UaP/+CjuZslIjd+kCigNG9TQmUqh5v0TP/nMEy90I=
+github.com/VictoriaMetrics/metricsql v0.65.0 h1:+/Oit3QycM8z/NbMHy4KENSUDS5q9QRx8h2x6cvoQOk=
+github.com/VictoriaMetrics/metricsql v0.65.0/go.mod h1:k4UaP/+CjuZslIjd+kCigNG9TQmUqh5v0TP/nMEy90I=
 github.com/VividCortex/ewma v1.2.0 h1:f58SaIzcDXrSy3kWaHNvuJgJ3Nmz59Zji6XoJR/q1ow=
 github.com/VividCortex/ewma v1.2.0/go.mod h1:nz4BbCtbLyFDeC9SUHbtcT5644juEuWfUAUnGx7j5l4=
 github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc=
diff --git a/vendor/github.com/VictoriaMetrics/metricsql/aggr.go b/vendor/github.com/VictoriaMetrics/metricsql/aggr.go
index b8c77c3493..51bc36a0f9 100644
--- a/vendor/github.com/VictoriaMetrics/metricsql/aggr.go
+++ b/vendor/github.com/VictoriaMetrics/metricsql/aggr.go
@@ -43,7 +43,8 @@ var aggrFuncs = map[string]bool{
 	"zscore":         true,
 }
 
-func isAggrFunc(s string) bool {
+// IsAggrFunc returns whether funcName is a known aggregate function.
+func IsAggrFunc(s string) bool {
 	s = strings.ToLower(s)
 	return aggrFuncs[s]
 }
diff --git a/vendor/github.com/VictoriaMetrics/metricsql/optimizer.go b/vendor/github.com/VictoriaMetrics/metricsql/optimizer.go
index 8072667cd6..046a8f1b89 100644
--- a/vendor/github.com/VictoriaMetrics/metricsql/optimizer.go
+++ b/vendor/github.com/VictoriaMetrics/metricsql/optimizer.go
@@ -362,7 +362,7 @@ func getFuncArgIdxForOptimization(funcName string, args []Expr) int {
 	if IsTransformFunc(funcName) {
 		return getTransformArgIdxForOptimization(funcName, args)
 	}
-	if isAggrFunc(funcName) {
+	if IsAggrFunc(funcName) {
 		return getAggrArgIdxForOptimization(funcName, args)
 	}
 	return -1
diff --git a/vendor/github.com/VictoriaMetrics/metricsql/parser.go b/vendor/github.com/VictoriaMetrics/metricsql/parser.go
index bc3f83e7ff..c408768577 100644
--- a/vendor/github.com/VictoriaMetrics/metricsql/parser.go
+++ b/vendor/github.com/VictoriaMetrics/metricsql/parser.go
@@ -31,6 +31,9 @@ func Parse(s string) (Expr, error) {
 	}
 	e = removeParensExpr(e)
 	e = simplifyConstants(e)
+	if err := checkSupportedFunctions(e); err != nil {
+		return nil, err
+	}
 	return e, nil
 }
 
@@ -596,7 +599,7 @@ func (p *parser) parseParensExpr() (*parensExpr, error) {
 }
 
 func (p *parser) parseAggrFuncExpr() (*AggrFuncExpr, error) {
-	if !isAggrFunc(p.lex.Token) {
+	if !IsAggrFunc(p.lex.Token) {
 		return nil, fmt.Errorf(`AggrFuncExpr: unexpected token %q; want aggregate func`, p.lex.Token)
 	}
 
@@ -1608,7 +1611,7 @@ func (p *parser) parseIdentExpr() (Expr, error) {
 	}
 	if isIdentPrefix(p.lex.Token) {
 		p.lex.Prev()
-		if isAggrFunc(p.lex.Token) {
+		if IsAggrFunc(p.lex.Token) {
 			return p.parseAggrFuncExpr()
 		}
 		return p.parseMetricExpr()
@@ -1620,7 +1623,7 @@ func (p *parser) parseIdentExpr() (Expr, error) {
 	switch p.lex.Token {
 	case "(":
 		p.lex.Prev()
-		if isAggrFunc(p.lex.Token) {
+		if IsAggrFunc(p.lex.Token) {
 			return p.parseAggrFuncExpr()
 		}
 		return p.parseFuncExpr()
diff --git a/vendor/github.com/VictoriaMetrics/metricsql/utils.go b/vendor/github.com/VictoriaMetrics/metricsql/utils.go
index 11828f39fc..2405b1fef4 100644
--- a/vendor/github.com/VictoriaMetrics/metricsql/utils.go
+++ b/vendor/github.com/VictoriaMetrics/metricsql/utils.go
@@ -1,5 +1,10 @@
 package metricsql
 
+import (
+	"fmt"
+	"strings"
+)
+
 // ExpandWithExprs expands WITH expressions inside q and returns the resulting
 // PromQL without WITH expressions.
 func ExpandWithExprs(q string) (string, error) {
@@ -14,7 +19,7 @@ func ExpandWithExprs(q string) (string, error) {
 // VisitAll recursively calls f for all the Expr children in e.
 //
 // It visits leaf children at first and then visits parent nodes.
-// It is safe modifying expr in f.
+// It is safe modifying e in f.
 func VisitAll(e Expr, f func(expr Expr)) {
 	switch expr := e.(type) {
 	case *BinaryOpExpr:
@@ -36,3 +41,38 @@ func VisitAll(e Expr, f func(expr Expr)) {
 	}
 	f(e)
 }
+
+// IsSupportedFunction returns true if funcName contains supported MetricsQL function
+func IsSupportedFunction(funcName string) bool {
+	funcName = strings.ToLower(funcName)
+	if IsRollupFunc(funcName) {
+		return true
+	}
+	if IsTransformFunc(funcName) {
+		return true
+	}
+	if IsAggrFunc(funcName) {
+		return true
+	}
+	return false
+}
+
+func checkSupportedFunctions(e Expr) error {
+	var err error
+	VisitAll(e, func(expr Expr) {
+		if err != nil {
+			return
+		}
+		switch t := expr.(type) {
+		case *FuncExpr:
+			if !IsRollupFunc(t.Name) && !IsTransformFunc(t.Name) {
+				err = fmt.Errorf("unsupported function %q", t.Name)
+			}
+		case *AggrFuncExpr:
+			if !IsAggrFunc(t.Name) {
+				err = fmt.Errorf("unsupported aggregate function %q", t.Name)
+			}
+		}
+	})
+	return err
+}
diff --git a/vendor/modules.txt b/vendor/modules.txt
index 1dbceba9ae..7c406cb47a 100644
--- a/vendor/modules.txt
+++ b/vendor/modules.txt
@@ -99,7 +99,7 @@ github.com/VictoriaMetrics/fasthttp/stackless
 # github.com/VictoriaMetrics/metrics v1.24.0
 ## explicit; go 1.20
 github.com/VictoriaMetrics/metrics
-# github.com/VictoriaMetrics/metricsql v0.64.0
+# github.com/VictoriaMetrics/metricsql v0.65.0
 ## explicit; go 1.13
 github.com/VictoriaMetrics/metricsql
 github.com/VictoriaMetrics/metricsql/binaryop