Merge pull request #834 from mchrome/fix/validate-consolidateBy-argum…

…ents fix: validate consolidateBy arguments
go-graphite · Jul 5, 2024 · d9996b2 · d9996b2
2 parents c2229ea + 7d31351
commit d9996b2
Show file tree

Hide file tree

Showing 4 changed files with 174 additions and 0 deletions.
diff --git a/cmd/mockbackend/testcases/consolidateBy/carbonapi.yaml b/cmd/mockbackend/testcases/consolidateBy/carbonapi.yaml
@@ -0,0 +1,58 @@
+listen: "localhost:8081"
+expvar:
+  enabled: true
+  pprofEnabled: false
+  listen: ""
+concurency: 1000
+notFoundStatusCode: 200
+passFunctionsToBackend: true
+cache:
+   type: "mem"
+   size_mb: 0
+   defaultTimeoutSec: 60
+cpus: 0
+tz: ""
+maxBatchSize: 0
+graphite:
+    host: ""
+    interval: "60s"
+    prefix: "carbon.api"
+    pattern: "{prefix}.{fqdn}"
+idleConnections: 10
+pidFile: ""
+upstreams:
+    buckets: 10
+    timeouts:
+        find: "2s"
+        render: "10s"
+        connect: "200ms"
+    concurrencyLimitPerServer: 0
+    keepAliveInterval: "30s"
+    maxIdleConnsPerHost: 100
+    backendsv2:
+        backends:
+          -
+            groupName: "mock-001"
+            protocol: "auto"
+            lbMethod: "all"
+            maxTries: 3
+            maxBatchSize: 0
+            keepAliveInterval: "10s"
+            concurrencyLimit: 0
+            forceAttemptHTTP2: true
+            maxIdleConnsPerHost: 1000
+            timeouts:
+                find: "15000s"
+                render: "5000s"
+                connect: "200ms"
+            servers:
+                - "http://127.0.0.1:9070"
+graphite09compat: false
+expireDelaySec: 10
+logger:
+    - logger: ""
+      file: "stderr"
+      level: "debug"
+      encoding: "console"
+      encodingTime: "iso8601"
+      encodingDuration: "seconds"
diff --git a/cmd/mockbackend/testcases/consolidateBy/consolidateBy.yaml b/cmd/mockbackend/testcases/consolidateBy/consolidateBy.yaml
@@ -0,0 +1,101 @@
+version: "v1"
+test:
+    apps:
+        - name: "carbonapi"
+          binary: "./carbonapi"
+          args:
+              - "-config"
+              - "./cmd/mockbackend/testcases/consolidateBy/carbonapi.yaml"
+    queries:
+            - endpoint: "http://127.0.0.1:8081"
+              type: "GET"
+              URL: "/render?format=json&target=consolidateBy(metric*, 'max')&maxDataPoints=2"
+              expectedResponse:
+                  httpCode: 200
+                  contentType: "application/json"
+            - endpoint: "http://127.0.0.1:8081"
+              type: "GET"
+              URL: "/render?format=json&target=consolidateBy(metric*, 'min')&maxDataPoints=2"
+              expectedResponse:
+                  httpCode: 200
+                  contentType: "application/json"
+            - endpoint: "http://127.0.0.1:8081"
+              type: "GET"
+              URL: "/render?format=json&target=consolidateBy(metric*, 'sum')&maxDataPoints=2"
+              expectedResponse:
+                  httpCode: 200
+                  contentType: "application/json"
+            - endpoint: "http://127.0.0.1:8081"
+              type: "GET"
+              URL: "/render?format=json&target=consolidateBy(metric*, 'avg')&maxDataPoints=2"
+              expectedResponse:
+                  httpCode: 200
+                  contentType: "application/json"
+            - endpoint: "http://127.0.0.1:8081"
+              type: "GET"
+              URL: "/render?format=json&target=consolidateBy(metric*, 'average')&maxDataPoints=2"
+              expectedResponse:
+                  httpCode: 200
+                  contentType: "application/json"
+            - endpoint: "http://127.0.0.1:8081"
+              type: "GET"
+              URL: "/render?format=json&target=consolidateBy(metric*, 'last')&maxDataPoints=2"
+              expectedResponse:
+                  httpCode: 200
+                  contentType: "application/json"
+            - endpoint: "http://127.0.0.1:8081"
+              type: "GET"
+              URL: "/render?format=json&target=consolidateBy(metric*, 'first')&maxDataPoints=2"
+              expectedResponse:
+                  httpCode: 200
+                  contentType: "application/json"
+            - endpoint: "http://127.0.0.1:8081"
+              type: "GET"
+              URL: "/render?format=json&target=consolidateBy(metric*, 'maximum')&maxDataPoints=2"
+              expectedResponse:
+                  httpCode: 400
+                  contentType: "text/plain; charset=utf-8"
+            - endpoint: "http://127.0.0.1:8081"
+              type: "GET"
+              URL: "/render?format=json&target=consolidateBy(metric*, 'minimum')&maxDataPoints=2"
+              expectedResponse:
+                  httpCode: 400
+                  contentType: "text/plain; charset=utf-8"   
+            - endpoint: "http://127.0.0.1:8081"
+              type: "GET"
+              URL: "/render?format=json&target=consolidateBy(metric*, 'somefunc')&maxDataPoints=2"
+              expectedResponse:
+                  httpCode: 400
+                  contentType: "text/plain; charset=utf-8"
+            - endpoint: "http://127.0.0.1:8081"
+              type: "GET"
+              URL: "/render?format=json&target=limit(metric*, 2)&maxDataPoints=2"
+              expectedResponse:
+                  httpCode: 200
+                  contentType: "application/json"
+            - endpoint: "http://127.0.0.1:8081"
+              type: "GET"
+              URL: "/render?format=json&target=consolidateBy(seriesByTag('name=rps', 'env=prod'), 'sum')&maxDataPoints=2"
+              expectedResponse:
+                  httpCode: 200
+                  contentType: "application/json"    
+
+
+listeners:
+        - address: ":9070"
+          expressions:
+            "metric*":
+                pathExpression: "metric*"
+                data:
+                    - metricName: "metricNaN"
+                      values: [.NaN, .NaN, .NaN, .NaN, .NaN]
+                      step: 1
+                      startTime: 1
+                    - metricName: "metricZ1"
+                      values: [4.0, 6.0, 2.0, 2.0, 3.0]
+                      step: 1
+                      startTime: 1
+                    - metricName: "metricZ2"
+                      values: [8.0, 1.0, 1.0, 7.0, 4.0]
+                      step: 1
+                      startTime: 1
diff --git a/expr/expr.go b/expr/expr.go
@@ -8,6 +8,7 @@ import (
 	pb "github.com/go-graphite/protocol/carbonapi_v3_pb"
 
 	_ "github.com/go-graphite/carbonapi/expr/functions"
+	"github.com/go-graphite/carbonapi/expr/functions/consolidateBy"
 	"github.com/go-graphite/carbonapi/expr/helper"
 	"github.com/go-graphite/carbonapi/expr/interfaces"
 	"github.com/go-graphite/carbonapi/expr/metadata"
@@ -55,6 +56,9 @@ func (eval Evaluator) Fetch(ctx context.Context, exprs []parser.Expr, from, unti
 			}
 
 			if eval.passFunctionsToBackend && m.ConsolidationFunc != "" {
+				if _, ok := consolidateBy.ValidAggregateFunctions[m.ConsolidationFunc]; !ok {
+					return nil, merry.WithMessagef(parser.ErrInvalidArg, "invalid consolidateBy argument: '%s'", m.ConsolidationFunc)
+				}
 				fetchRequest.FilterFunctions = append(fetchRequest.FilterFunctions, &pb.FilteringFunction{
 					Name:      "consolidateBy",
 					Arguments: []string{m.ConsolidationFunc},

diff --git a/expr/functions/consolidateBy/function.go b/expr/functions/consolidateBy/function.go
@@ -26,6 +26,17 @@ func New(configFile string) []interfaces.FunctionMetadata {
 	return res
 }
 
+// The set of valid aggregation methods for consolidateBy function.
+var ValidAggregateFunctions = map[string]struct{}{
+	"average": {},
+	"avg":     {},
+	"max":     {},
+	"min":     {},
+	"sum":     {},
+	"first":   {},
+	"last":    {},
+}
+
 // consolidateBy(seriesList, aggregationMethod)
 func (f *consolidateBy) Do(ctx context.Context, eval interfaces.Evaluator, e parser.Expr, from, until int64, values map[parser.MetricRequest][]*types.MetricData) ([]*types.MetricData, error) {
 	if e.ArgsLen() < 2 {