From 031952541dee7e2f4a33565db75cca570daba5fe Mon Sep 17 00:00:00 2001 From: oleiade Date: Wed, 26 Jan 2022 16:33:47 +0100 Subject: [PATCH] Validate threshold configuration before starting the execution This commit makes sure that the threshold configuration (as passed in the script exported options for instance) is valid, before starting the execution. A valid threshold must pass the following assertion: - Its expression is syntaxically correct and is parsable - It applies to a metrics that's known to k6, either builtin or custom - Its expression's aggregation method is valid for the metric it applies to Threshold validation will be made in the context of the `run`, `cloud`, `archive`, `inspect`, and `archive` commands. If a threshold definition is invalid, the k6 program will exit with a status code of 104. ref #2330 --- cmd/archive.go | 2 +- cmd/cloud.go | 2 +- cmd/config.go | 8 +++- cmd/config_test.go | 5 ++- cmd/inspect.go | 2 +- cmd/run.go | 7 ++- cmd/run_test.go | 45 +++++++++++++++++++ .../thresholds/malformed_expression.js | 11 +++++ .../thresholds/non_existing_metric.js | 13 ++++++ .../unsupported_aggregation_method.js | 15 +++++++ 10 files changed, 103 insertions(+), 7 deletions(-) create mode 100644 cmd/testdata/thresholds/malformed_expression.js create mode 100644 cmd/testdata/thresholds/non_existing_metric.js create mode 100644 cmd/testdata/thresholds/unsupported_aggregation_method.js diff --git a/cmd/archive.go b/cmd/archive.go index a83af6fcde90..b5a706cabf8d 100644 --- a/cmd/archive.go +++ b/cmd/archive.go @@ -78,7 +78,7 @@ An archive is a fully self-contained test run, and can be executed identically e return err } - _, err = deriveAndValidateConfig(conf, r.IsExecutable) + _, err = deriveAndValidateConfig(conf, registry, r.IsExecutable) if err != nil { return err } diff --git a/cmd/cloud.go b/cmd/cloud.go index 0cf2a0b9f980..85f4875c4cc6 100644 --- a/cmd/cloud.go +++ b/cmd/cloud.go @@ -120,7 +120,7 @@ This will execute the test on the k6 cloud service. Use "k6 login cloud" to auth return err } - derivedConf, err := deriveAndValidateConfig(conf, r.IsExecutable) + derivedConf, err := deriveAndValidateConfig(conf, registry, r.IsExecutable) if err != nil { return err } diff --git a/cmd/config.go b/cmd/config.go index c6f76c3b9157..823c2f4cba0e 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -234,12 +234,18 @@ func applyDefault(conf Config) Config { return conf } -func deriveAndValidateConfig(conf Config, isExecutable func(string) bool) (result Config, err error) { +func deriveAndValidateConfig(conf Config, registry *metrics.Registry, isExecutable func(string) bool) (result Config, err error) { result = conf result.Options, err = executor.DeriveScenariosFromShortcuts(conf.Options) if err == nil { err = validateConfig(result, isExecutable) } + + err = validateThresholdsConfig(conf, registry) + if err != nil { + return result, err + } + return result, errext.WithExitCodeIfNone(err, exitcodes.InvalidConfig) } diff --git a/cmd/config_test.go b/cmd/config_test.go index 893ac3fe7a1e..29f86d15f39d 100644 --- a/cmd/config_test.go +++ b/cmd/config_test.go @@ -32,7 +32,6 @@ import ( "go.k6.io/k6/lib" "go.k6.io/k6/lib/executor" "go.k6.io/k6/lib/metrics" - "go.k6.io/k6/lib/testutils" "go.k6.io/k6/lib/types" "go.k6.io/k6/stats" "gopkg.in/guregu/null.v3" @@ -205,7 +204,7 @@ func TestDeriveAndValidateConfig(t *testing.T) { tc := tc t.Run(tc.name, func(t *testing.T) { t.Parallel() - _, err := deriveAndValidateConfig(tc.conf, + _, err := deriveAndValidateConfig(tc.conf, nil, func(_ string) bool { return tc.isExec }) if tc.err != "" { var ecerr errext.HasExitCode @@ -220,6 +219,7 @@ func TestDeriveAndValidateConfig(t *testing.T) { } func TestValidateThresholdsConfigWithNilRegistry(t *testing.T) { + t.Parallel() var registry *metrics.Registry config := Config{} var wantErrType errext.HasExitCode @@ -231,6 +231,7 @@ func TestValidateThresholdsConfigWithNilRegistry(t *testing.T) { } func TestValidateThresholdsConfigAppliesToBuiltinMetrics(t *testing.T) { + t.Parallel() // Prepare a registry loaded with builtin metrics registry := metrics.NewRegistry() metrics.RegisterBuiltinMetrics(registry) diff --git a/cmd/inspect.go b/cmd/inspect.go index 431d0f6bfe20..9f83a37f5f5b 100644 --- a/cmd/inspect.go +++ b/cmd/inspect.go @@ -124,7 +124,7 @@ func addExecRequirements(b *js.Bundle, return nil, err } - conf, err = deriveAndValidateConfig(conf, runner.IsExecutable) + conf, err = deriveAndValidateConfig(conf, registry, runner.IsExecutable) if err != nil { return nil, err } diff --git a/cmd/run.go b/cmd/run.go index 7045d4b50169..7727c942e131 100644 --- a/cmd/run.go +++ b/cmd/run.go @@ -51,6 +51,7 @@ import ( "go.k6.io/k6/lib/consts" "go.k6.io/k6/lib/metrics" "go.k6.io/k6/loader" + "go.k6.io/k6/stats" "go.k6.io/k6/ui/pb" ) @@ -114,6 +115,10 @@ a commandline interface for interacting with it.`, builtinMetrics := metrics.RegisterBuiltinMetrics(registry) initRunner, err := newRunner(logger, src, runType, filesystems, runtimeOptions, builtinMetrics, registry) if err != nil { + if errors.Is(err, stats.ErrThresholdParsing) { + return errext.WithExitCodeIfNone(err, exitcodes.InvalidConfig) + } + return common.UnwrapGojaInterruptedError(err) } @@ -128,7 +133,7 @@ a commandline interface for interacting with it.`, return err } - conf, err = deriveAndValidateConfig(conf, initRunner.IsExecutable) + conf, err = deriveAndValidateConfig(conf, registry, initRunner.IsExecutable) if err != nil { return err } diff --git a/cmd/run_test.go b/cmd/run_test.go index 873edcd846dc..17e6dc678340 100644 --- a/cmd/run_test.go +++ b/cmd/run_test.go @@ -216,3 +216,48 @@ func TestInitErrExitCode(t *testing.T) { "Status code must be %d", exitcodes.ScriptException) assert.Contains(t, err.Error(), "ReferenceError: someUndefinedVar is not defined") } + +func TestInvalidOptionsThresholdErrExitCode(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + testFilename string + }{ + { + "run should fail with exit status 104 on a malformed threshold expression", + "testdata/thresholds/malformed_expression.js", + }, + { + "run should fail with exit status 104 on a threshold applied to a non existing metric", + "testdata/thresholds/non_existing_metric.js", + }, + { + "run should fail with exit status 104 on a threshold method being unsupported by the metric", + "testdata/thresholds/unsupported_aggregation_method.js", + }, + } + + for _, testCase := range testCases { + testCase := testCase + t.Run(testCase.name, func(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + cmd := getRunCmd(ctx, testutils.NewLogger(t)) + a, err := filepath.Abs(testCase.testFilename) + require.NoError(t, err) + cmd.SetArgs([]string{a}) + wantExitCode := exitcodes.InvalidConfig + + var gotErrExt errext.HasExitCode + gotErr := cmd.Execute() + + require.ErrorAs(t, gotErr, &gotErrExt) + assert.Equalf(t, wantExitCode, gotErrExt.ExitCode(), + "status code must be %d", wantExitCode, + ) + }) + } +} diff --git a/cmd/testdata/thresholds/malformed_expression.js b/cmd/testdata/thresholds/malformed_expression.js new file mode 100644 index 000000000000..59a3a452f5f2 --- /dev/null +++ b/cmd/testdata/thresholds/malformed_expression.js @@ -0,0 +1,11 @@ +export const options = { + thresholds: { + http_reqs: ["foo>0"], // Counter + }, +}; + +export default function () { + console.log( + "asserting that a malformed threshold fails with exit code 104 (Invalid config)" + ); +} diff --git a/cmd/testdata/thresholds/non_existing_metric.js b/cmd/testdata/thresholds/non_existing_metric.js new file mode 100644 index 000000000000..e4cdc4cfd7b1 --- /dev/null +++ b/cmd/testdata/thresholds/non_existing_metric.js @@ -0,0 +1,13 @@ +export const options = { + thresholds: { + // non existing is neither registered, nor a builtin metric. + // k6 should catch that. + "non existing": ["rate>0"], + }, +}; + +export default function () { + console.log( + "asserting that a threshold over a non-existing metric fails with exit code 104 (Invalid config)" + ); +} diff --git a/cmd/testdata/thresholds/unsupported_aggregation_method.js b/cmd/testdata/thresholds/unsupported_aggregation_method.js new file mode 100644 index 000000000000..4c2038edfd54 --- /dev/null +++ b/cmd/testdata/thresholds/unsupported_aggregation_method.js @@ -0,0 +1,15 @@ +export const options = { + thresholds: { + // http_reqs is a Counter metric. As such, it supports + // only the 'count' and 'rate' operations. Thus, 'value' + // being a Gauge's metric aggregation method, the threshold + // configuration evaluation should fail. + http_reqs: ["value>0"], + }, +}; + +export default function () { + console.log( + "asserting that a threshold applying a method over a metric not supporting it fails with exit code 104 (Invalid config)" + ); +}