Skip to content

Commit

Permalink
Exit with InvalidConfig code when running a script with invalid optio…
Browse files Browse the repository at this point in the history
…ns (#3783)
  • Loading branch information
ariasmn authored Jul 1, 2024
1 parent d9b5fd0 commit 3e70527
Show file tree
Hide file tree
Showing 7 changed files with 95 additions and 5 deletions.
6 changes: 2 additions & 4 deletions cmd/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,15 +168,13 @@ func readEnvConfig(envMap map[string]string) (Config, error) {
// TODO: add better validation, more explicit default values and improve consistency between formats
// TODO: accumulate all errors and differentiate between the layers?
func getConsolidatedConfig(gs *state.GlobalState, cliConf Config, runnerOpts lib.Options) (conf Config, err error) {
// TODO: use errext.WithExitCodeIfNone(err, exitcodes.InvalidConfig) where it makes sense?

fileConf, err := readDiskConfig(gs)
if err != nil {
return conf, err
return conf, errext.WithExitCodeIfNone(err, exitcodes.InvalidConfig)
}
envConf, err := readEnvConfig(gs.Env)
if err != nil {
return conf, err
return conf, errext.WithExitCodeIfNone(err, exitcodes.InvalidConfig)
}

conf = cliConf.Apply(fileConf)
Expand Down
47 changes: 47 additions & 0 deletions cmd/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,9 @@ func TestRunScriptErrorsAndAbort(t *testing.T) {
testFilename, name string
expErr, expLogOutput string
expExitCode exitcodes.ExitCode
configFilename string
extraArgs []string
envVars map[string]string
}{
{
testFilename: "abort.js",
Expand Down Expand Up @@ -146,6 +148,41 @@ func TestRunScriptErrorsAndAbort(t *testing.T) {
expErr: "ReferenceError: someUndefinedVar is not defined",
expExitCode: exitcodes.ScriptException,
},
{
testFilename: "invalidconfig/invalid_option.js",
name: "run should fail with exit status 104 if an invalid option value exists",
expErr: "this is an invalid type",
expExitCode: exitcodes.InvalidConfig,
},
{
testFilename: "invalidconfig/option_env.js",
name: "run should fail with exit status 104 if an invalid option is set through env variable",
expErr: "envconfig.Process",
expExitCode: exitcodes.InvalidConfig,
envVars: map[string]string{
"K6_DURATION": "fails",
},
},
{
testFilename: "invalidconfig/option_env.js",
name: "run should fail with exit status 104 if an invalid option is set through k6 variable",
expErr: "invalid duration",
expExitCode: exitcodes.InvalidConfig,
extraArgs: []string{"--env", "DURATION=fails"},
},
{
testFilename: "invalidconfig/option_env.js",
name: "run should fail with exit status 104 if an invalid option is set in a config file",
expErr: "invalid duration",
expExitCode: exitcodes.InvalidConfig,
configFilename: "invalidconfig/invalid.json",
},
{
testFilename: "invalidconfig/invalid_scenario.js",
name: "run should fail with exit status 104 if an invalid scenario exists",
expErr: "specified executor type",
expExitCode: exitcodes.InvalidConfig,
},
{
testFilename: "thresholds/non_existing_metric.js",
name: "run should fail with exit status 104 on a threshold applied to a non existing metric",
Expand Down Expand Up @@ -203,6 +240,16 @@ func TestRunScriptErrorsAndAbort(t *testing.T) {
require.NoError(t, fsext.WriteFile(ts.FS, filepath.Join(ts.Cwd, tc.testFilename), testScript, 0o644))
ts.CmdArgs = append([]string{"k6", "run", tc.testFilename}, tc.extraArgs...)

if tc.configFilename != "" {
configFile, err := os.ReadFile(path.Join("testdata", tc.configFilename)) //nolint:forbidigo
require.NoError(t, err)
require.NoError(t, fsext.WriteFile(ts.FS, filepath.Join(ts.Cwd, tc.configFilename), configFile, 0o644))
ts.Flags.ConfigFilePath = path.Join(ts.Cwd, tc.configFilename)
}
if tc.envVars != nil {
ts.Env = tc.envVars
}

ts.ExpectedExitCode = int(tc.expExitCode)
newRootCommand(ts.GlobalState).execute()

Expand Down
3 changes: 3 additions & 0 deletions cmd/testdata/invalidconfig/invalid.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"duration": "fails"
}
5 changes: 5 additions & 0 deletions cmd/testdata/invalidconfig/invalid_option.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export const options = {
vus: 'this is an invalid type',
}

export default function () {}
22 changes: 22 additions & 0 deletions cmd/testdata/invalidconfig/invalid_scenario.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
export const options = {
scenarios: {
example_scenario: {
// name of the executor to use
executor: 'shared-iterations',
// common scenario configuration
startTime: '10s',
gracefulStop: '5s',
env: { EXAMPLEVAR: 'testing' },
tags: { example_tag: 'testing' },

// executor-specific configuration
vus: 10,
iterations: 200,
maxDuration: '10s',
},
another_scenario: {
/*...*/
},
},
};

10 changes: 10 additions & 0 deletions cmd/testdata/invalidconfig/option_env.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import http from 'k6/http';

export const options = {
iterations: 1,
duration: __ENV.DURATION,
};

export default function () {
const res = http.get('https://test.k6.io');
}
7 changes: 6 additions & 1 deletion js/bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import (
"github.com/sirupsen/logrus"
"gopkg.in/guregu/null.v3"

"go.k6.io/k6/errext"
"go.k6.io/k6/errext/exitcodes"
"go.k6.io/k6/event"
"go.k6.io/k6/js/common"
"go.k6.io/k6/js/compiler"
Expand Down Expand Up @@ -191,7 +193,10 @@ func (b *Bundle) populateExports(updateOptions bool, exports *sobek.Object) erro
dec.DisallowUnknownFields()
if err := dec.Decode(&b.Options); err != nil {
if uerr := json.Unmarshal(data, &b.Options); uerr != nil {
return uerr
return errext.WithAbortReasonIfNone(
errext.WithExitCodeIfNone(uerr, exitcodes.InvalidConfig),
errext.AbortedByScriptError,
)
}
b.preInitState.Logger.WithError(err).Warn("There were unknown fields in the options exported in the script")
}
Expand Down

0 comments on commit 3e70527

Please sign in to comment.