From 7d313515653a6cb5842d149bdc6b2149a18e8998 Mon Sep 17 00:00:00 2001 From: Artyom Antonov Date: Fri, 7 Jun 2024 03:20:28 +0500 Subject: [PATCH] validate consolidateBy aggregation function before passing it to graphite-clickhouse --- .../testcases/consolidateBy/carbonapi.yaml | 58 ++++++++++ .../consolidateBy/consolidateBy.yaml | 101 ++++++++++++++++++ expr/expr.go | 4 + expr/functions/consolidateBy/function.go | 11 ++ 4 files changed, 174 insertions(+) create mode 100644 cmd/mockbackend/testcases/consolidateBy/carbonapi.yaml create mode 100644 cmd/mockbackend/testcases/consolidateBy/consolidateBy.yaml diff --git a/cmd/mockbackend/testcases/consolidateBy/carbonapi.yaml b/cmd/mockbackend/testcases/consolidateBy/carbonapi.yaml new file mode 100644 index 000000000..11d7477d1 --- /dev/null +++ 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 new file mode 100644 index 000000000..997b89a61 --- /dev/null +++ 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 index 2a06650f3..1fba2faec 100644 --- 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 index 99d480da0..7ab654689 100644 --- 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 {