diff --git a/cloudapi/config.go b/cloudapi/config.go index f5ac5e2c052..b3306061019 100644 --- a/cloudapi/config.go +++ b/cloudapi/config.go @@ -1,6 +1,7 @@ package cloudapi import ( + "bytes" "encoding/json" "time" @@ -9,6 +10,9 @@ import ( "gopkg.in/guregu/null.v3" ) +// LegacyCloudConfigKey is the key used in the JSON config for the cloud output. +const LegacyCloudConfigKey = "loadimpact" + // Config holds all the necessary data and options for sending metrics to the k6 Cloud. // //nolint:lll @@ -215,7 +219,7 @@ func NewConfig() Config { } // Apply saves config non-zero config values from the passed config in the receiver. -func (c Config) Apply(cfg Config) Config { +func (c Config) Apply(cfg Config) Config { //nolint:funlen,gocognit,cyclop if cfg.Token.Valid { c.Token = cfg.Token } @@ -306,44 +310,40 @@ func (c Config) Apply(cfg Config) Config { return c } -// MergeFromExternal merges three fields from the JSON in a loadimpact key of -// the provided external map. Used for options.ext.loadimpact settings. -func MergeFromExternal(external map[string]json.RawMessage, conf *Config) error { - if val, ok := external["loadimpact"]; ok { - // TODO: Important! Separate configs and fix the whole 2 configs mess! - tmpConfig := Config{} - if err := json.Unmarshal(val, &tmpConfig); err != nil { - return err - } - // Only take out the ProjectID, Name and Token from the options.ext.loadimpact map: - if tmpConfig.ProjectID.Valid { - conf.ProjectID = tmpConfig.ProjectID - } - if tmpConfig.Name.Valid { - conf.Name = tmpConfig.Name - } - if tmpConfig.Token.Valid { - conf.Token = tmpConfig.Token - } - } - return nil -} - // GetConsolidatedConfig combines the default config values with the JSON config // values and environment variables and returns the final result. +// it also returns a warning message that could be shown to the user. +// to bring some attention to the fact that the user. func GetConsolidatedConfig( - jsonRawConf json.RawMessage, env map[string]string, configArg string, external map[string]json.RawMessage, -) (Config, error) { + jsonRawConf json.RawMessage, + env map[string]string, + configArg string, + cloudConfig json.RawMessage, + external map[string]json.RawMessage, +) (Config, string, error) { + warn := "" + result := NewConfig() if jsonRawConf != nil { jsonConf := Config{} if err := json.Unmarshal(jsonRawConf, &jsonConf); err != nil { - return result, err + return result, warn, err } result = result.Apply(jsonConf) } - if err := MergeFromExternal(external, &result); err != nil { - return result, err + + if err := mergeFromCloudOptionAndExternal(cloudConfig, external, &result); err != nil { + return result, warn, err + } + + // We want to show a warning if the user is using the only old way of defining the config. + // Note: Since the migration to the options.cloud is a long process, this warning is planned + // to be emitted for a long time (1-2 years), after some point, and depending on the state + // of migration we could re-evaluate this warning. + if cloudConfig == nil && external != nil { + if _, ok := external[LegacyCloudConfigKey]; ok { + warn = "The options.ext.loadimpact option is deprecated, please use options.cloud instead" + } } envConfig := Config{} @@ -352,7 +352,7 @@ func GetConsolidatedConfig( return v, ok }); err != nil { // TODO: get rid of envconfig and actually use the env parameter... - return result, err + return result, warn, err } result = result.Apply(envConfig) @@ -360,5 +360,85 @@ func GetConsolidatedConfig( result.Name = null.StringFrom(configArg) } - return result, nil + return result, warn, nil +} + +// mergeFromCloudOptionAndExternal merges three fields from the JSON in a cloud key of +// the provided external map. Used for options.cloud settings. +func mergeFromCloudOptionAndExternal( + cloudConfig json.RawMessage, + external map[string]json.RawMessage, + conf *Config, +) error { + source := pickSource(cloudConfig, external) + if source == nil { + return nil + } + + // Original comment + // TODO: Important! Separate configs and fix the whole 2 configs mess! + tmpConfig := Config{} + if err := json.Unmarshal(source, &tmpConfig); err != nil { + return err + } + // Only take out the ProjectID, Name and Token from the options.cloud (or legacy loadimpact struct) map: + if tmpConfig.ProjectID.Valid { + conf.ProjectID = tmpConfig.ProjectID + } + if tmpConfig.Name.Valid { + conf.Name = tmpConfig.Name + } + if tmpConfig.Token.Valid { + conf.Token = tmpConfig.Token + } + + return nil +} + +// GetTemporaryCloudConfig returns a temporary cloud config. +// Original comment +// TODO: Fix this +// We reuse cloud.Config for parsing options.cloud (or legacy loadimpact struct), but this probably shouldn't be +// done, as the idea of options.ext is that they are extensible without touching k6. But in +// order for this to happen, we shouldn't actually marshal cloud.Config on top of it, because +// it will be missing some fields that aren't actually mentioned in the struct. +// So in order for use to copy the fields that we need for k6 cloud's api we unmarshal in +// map[string]interface{} and copy what we need if it isn't set already +func GetTemporaryCloudConfig( + cloudConfig json.RawMessage, + external map[string]json.RawMessage, +) (map[string]interface{}, error) { + tmpCloudConfig := make(map[string]interface{}, 3) + + source := pickSource(cloudConfig, external) + if source == nil { + return tmpCloudConfig, nil + } + + dec := json.NewDecoder(bytes.NewReader(source)) + dec.UseNumber() // otherwise float64 are used + if err := dec.Decode(&tmpCloudConfig); err != nil { + return nil, err + } + + return tmpCloudConfig, nil +} + +// pickSource returns the config source to use. +func pickSource( + cloudConfig json.RawMessage, + external map[string]json.RawMessage, +) json.RawMessage { + // priority is the new way of defining the config + // via options.cloud + if cloudConfig != nil { + return cloudConfig + } + + // fallback to the old way of defining the config + if val, ok := external[LegacyCloudConfigKey]; ok { + return val + } + + return nil } diff --git a/cloudapi/config_test.go b/cloudapi/config_test.go index c0ae33770c2..fa81eacc8b7 100644 --- a/cloudapi/config_test.go +++ b/cloudapi/config_test.go @@ -63,17 +63,78 @@ func TestConfigApply(t *testing.T) { func TestGetConsolidatedConfig(t *testing.T) { t.Parallel() - config, err := GetConsolidatedConfig(json.RawMessage(`{"token":"jsonraw"}`), nil, "", nil) + config, warn, err := GetConsolidatedConfig(json.RawMessage(`{"token":"jsonraw"}`), nil, "", nil, nil) require.NoError(t, err) require.Equal(t, config.Token.String, "jsonraw") + require.Empty(t, warn) - config, err = GetConsolidatedConfig(json.RawMessage(`{"token":"jsonraw"}`), nil, "", + config, warn, err = GetConsolidatedConfig( + json.RawMessage(`{"token":"jsonraw"}`), + nil, + "", + json.RawMessage(`{"token":"ext"}`), + nil, + ) + require.NoError(t, err) + require.Equal(t, config.Token.String, "ext") + require.Empty(t, warn) + + config, warn, err = GetConsolidatedConfig( + json.RawMessage(`{"token":"jsonraw"}`), + map[string]string{"K6_CLOUD_TOKEN": "envvalue"}, + "", + json.RawMessage(`{"token":"ext"}`), + nil, + ) + require.NoError(t, err) + require.Equal(t, config.Token.String, "envvalue") + require.Empty(t, warn) +} + +func TestGetConsolidatedConfig_WithLegacyOnly(t *testing.T) { + t.Parallel() + + config, warn, err := GetConsolidatedConfig(json.RawMessage(`{"token":"jsonraw"}`), nil, "", nil, map[string]json.RawMessage{"loadimpact": json.RawMessage(`{"token":"ext"}`)}) require.NoError(t, err) require.Equal(t, config.Token.String, "ext") + require.NotEmpty(t, warn) - config, err = GetConsolidatedConfig(json.RawMessage(`{"token":"jsonraw"}`), map[string]string{"K6_CLOUD_TOKEN": "envvalue"}, "", + config, warn, err = GetConsolidatedConfig(json.RawMessage(`{"token":"jsonraw"}`), map[string]string{"K6_CLOUD_TOKEN": "envvalue"}, "", nil, map[string]json.RawMessage{"loadimpact": json.RawMessage(`{"token":"ext"}`)}) require.NoError(t, err) require.Equal(t, config.Token.String, "envvalue") + require.NotEmpty(t, warn) +} + +func TestGetConsolidatedConfig_LegacyHasLowerPriority(t *testing.T) { + t.Parallel() + + config, warn, err := GetConsolidatedConfig( + json.RawMessage(`{"token":"jsonraw"}`), + nil, + "", + json.RawMessage(`{"token":"cloud"}`), + map[string]json.RawMessage{"loadimpact": json.RawMessage(`{"token":"ext"}`)}, + ) + + require.NoError(t, err) + require.Equal(t, config.Token.String, "cloud") + require.Empty(t, warn) +} + +func TestGetConsolidatedConfig_EnvHasHigherPriority(t *testing.T) { + t.Parallel() + + config, warn, err := GetConsolidatedConfig( + json.RawMessage(`{"token":"jsonraw"}`), + map[string]string{"K6_CLOUD_TOKEN": "envvalue"}, + "", + json.RawMessage(`{"token":"cloud"}`), + map[string]json.RawMessage{"loadimpact": json.RawMessage(`{"token":"ext"}`)}, + ) + require.NoError(t, err) + + require.Equal(t, config.Token.String, "envvalue") + require.Empty(t, warn) } diff --git a/cmd/cloud.go b/cmd/cloud.go index edc124e69f1..6a693c12960 100644 --- a/cmd/cloud.go +++ b/cmd/cloud.go @@ -1,7 +1,6 @@ package cmd import ( - "bytes" "context" "encoding/json" "errors" @@ -106,34 +105,20 @@ func (c *cmdCloud) run(cmd *cobra.Command, args []string) error { modifyAndPrintBar(c.gs, progressBar, pb.WithConstProgress(0, "Building the archive...")) arc := testRunState.Runner.MakeArchive() - // TODO: Fix this - // We reuse cloud.Config for parsing options.ext.loadimpact, but this probably shouldn't be - // done, as the idea of options.ext is that they are extensible without touching k6. But in - // order for this to happen, we shouldn't actually marshall cloud.Config on top of it, because - // it will be missing some fields that aren't actually mentioned in the struct. - // So in order for use to copy the fields that we need for loadimpact's api we unmarshal in - // map[string]interface{} and copy what we need if it isn't set already - var tmpCloudConfig map[string]interface{} - if val, ok := arc.Options.External["loadimpact"]; ok { - dec := json.NewDecoder(bytes.NewReader(val)) - dec.UseNumber() // otherwise float64 are used - if err = dec.Decode(&tmpCloudConfig); err != nil { - return err - } + tmpCloudConfig, err := cloudapi.GetTemporaryCloudConfig(arc.Options.Cloud, arc.Options.External) + if err != nil { + return err } // Cloud config - cloudConfig, err := cloudapi.GetConsolidatedConfig( - test.derivedConfig.Collectors["cloud"], c.gs.Env, "", arc.Options.External) + cloudConfig, _, err := cloudapi.GetConsolidatedConfig( + test.derivedConfig.Collectors["cloud"], c.gs.Env, "", arc.Options.Cloud, arc.Options.External) if err != nil { return err } if !cloudConfig.Token.Valid { return errors.New("Not logged in, please use `k6 login cloud`.") //nolint:golint,revive,stylecheck } - if tmpCloudConfig == nil { - tmpCloudConfig = make(map[string]interface{}, 3) - } if cloudConfig.Token.Valid { tmpCloudConfig["token"] = cloudConfig.Token @@ -148,11 +133,15 @@ func (c *cmdCloud) run(cmd *cobra.Command, args []string) error { if arc.Options.External == nil { arc.Options.External = make(map[string]json.RawMessage) } - arc.Options.External["loadimpact"], err = json.Marshal(tmpCloudConfig) + + b, err := json.Marshal(tmpCloudConfig) if err != nil { return err } + arc.Options.Cloud = b + arc.Options.External[cloudapi.LegacyCloudConfigKey] = b + name := cloudConfig.Name.String if !cloudConfig.Name.Valid || cloudConfig.Name.String == "" { name = filepath.Base(test.sourceRootPath) diff --git a/cmd/login_cloud.go b/cmd/login_cloud.go index 3586dea8551..f39293c4643 100644 --- a/cmd/login_cloud.go +++ b/cmd/login_cloud.go @@ -23,10 +23,10 @@ func getCmdLoginCloud(gs *state.GlobalState) *cobra.Command { exampleText := getExampleText(gs, ` # Show the stored token. {{.}} login cloud -s - + # Store a token. {{.}} login cloud -t YOUR_TOKEN - + # Log in with an email/password. {{.}} login cloud`[1:]) @@ -55,11 +55,12 @@ This will set the default token used when just "k6 run -o cloud" is passed.`, // We want to use this fully consolidated config for things like // host addresses, so users can overwrite them with env vars. - consolidatedCurrentConfig, err := cloudapi.GetConsolidatedConfig( - currentJSONConfigRaw, gs.Env, "", nil) + consolidatedCurrentConfig, _, err := cloudapi.GetConsolidatedConfig( + currentJSONConfigRaw, gs.Env, "", nil, nil) if err != nil { return err } + // But we don't want to save them back to the JSON file, we only // want to save what already existed there and the login details. newCloudConf := currentJSONConfig diff --git a/cmd/tests/cmd_run_test.go b/cmd/tests/cmd_run_test.go index 618bd87453c..18470bd0729 100644 --- a/cmd/tests/cmd_run_test.go +++ b/cmd/tests/cmd_run_test.go @@ -1713,7 +1713,7 @@ func TestRunWithCloudOutputOverrides(t *testing.T) { assert.Contains(t, stdout, "iterations...........: 1") } -func TestRunWithCloudOutputCustomConfigAndOverrides(t *testing.T) { +func TestRunWithCloudOutputCustomConfigAndOverridesLegacyCloudOption(t *testing.T) { t.Parallel() script := ` @@ -1765,6 +1765,56 @@ export default function() {};` assert.Contains(t, stdout, `level=info msg="test message" output=cloud source=grafana-k6-cloud`) } +func TestRunWithCloudOutputCustomConfigAndOverrides(t *testing.T) { + t.Parallel() + + script := ` +export const options = { + cloud: { + name: 'Hello k6 Cloud!', + projectID: 123456, + }, +}; + +export default function() {};` + + ts := getSingleFileTestState(t, script, []string{"-v", "--log-output=stdout", "--out=cloud"}, 0) + + configOverride := http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) { + b, err := io.ReadAll(req.Body) + require.NoError(t, err) + + bjs := string(b) + assert.Contains(t, bjs, `"name":"Hello k6 Cloud!"`) + assert.Contains(t, bjs, `"project_id":123456`) + + resp.WriteHeader(http.StatusOK) + _, err = fmt.Fprint(resp, `{ + "reference_id": "1337", + "config": { + "webAppURL": "https://bogus.url", + "testRunDetails": "https://some.other.url/foo/tests/org/1337?bar=baz" + }, + "logs": [ + {"level": "debug", "message": "test debug message"}, + {"level": "info", "message": "test message"} + ] + }`) + assert.NoError(t, err) + }) + srv := getCloudTestEndChecker(t, 1337, configOverride, cloudapi.RunStatusFinished, cloudapi.ResultStatusPassed) + ts.Env["K6_CLOUD_HOST"] = srv.URL + + cmd.ExecuteWithGlobalState(ts.GlobalState) + + stdout := ts.Stdout.String() + t.Log(stdout) + assert.Contains(t, stdout, "execution: local") + assert.Contains(t, stdout, "output: cloud (https://some.other.url/foo/tests/org/1337?bar=baz)") + assert.Contains(t, stdout, `level=debug msg="test debug message" output=cloud source=grafana-k6-cloud`) + assert.Contains(t, stdout, `level=info msg="test message" output=cloud source=grafana-k6-cloud`) +} + func TestPrometheusRemoteWriteOutput(t *testing.T) { t.Parallel() diff --git a/lib/options.go b/lib/options.go index 5baad062b8f..06721cc001f 100644 --- a/lib/options.go +++ b/lib/options.go @@ -299,6 +299,10 @@ type Options struct { // iteration is shorter than the specified value. MinIterationDuration types.NullDuration `json:"minIterationDuration" envconfig:"K6_MIN_ITERATION_DURATION"` + // Cloud is the config for the cloud + // formally known as ext.loadimpact + Cloud json.RawMessage `json:"cloud,omitempty" ignored:"true"` + // These values are for third party collectors' benefit. // Can't be set through env vars. External map[string]json.RawMessage `json:"ext" ignored:"true"` diff --git a/output/cloud/output.go b/output/cloud/output.go index 7c5f4f0a1f7..80dc06fa017 100644 --- a/output/cloud/output.go +++ b/output/cloud/output.go @@ -78,8 +78,13 @@ func New(params output.Params) (output.Output, error) { // New creates a new cloud output. func newOutput(params output.Params) (*Output, error) { - conf, err := cloudapi.GetConsolidatedConfig( - params.JSONConfig, params.Environment, params.ConfigArgument, params.ScriptOptions.External) + conf, _, err := cloudapi.GetConsolidatedConfig( + params.JSONConfig, + params.Environment, + params.ConfigArgument, + params.ScriptOptions.Cloud, + params.ScriptOptions.External, + ) if err != nil { return nil, err } diff --git a/output/cloud/v1/output_test.go b/output/cloud/v1/output_test.go index 0d4e26a9259..770d247d39e 100644 --- a/output/cloud/v1/output_test.go +++ b/output/cloud/v1/output_test.go @@ -713,8 +713,8 @@ func TestNewOutputClientTimeout(t *testing.T) { } func newTestOutput(params output.Params) (*Output, error) { - conf, err := cloudapi.GetConsolidatedConfig( - params.JSONConfig, params.Environment, params.ConfigArgument, params.ScriptOptions.External) + conf, _, err := cloudapi.GetConsolidatedConfig( + params.JSONConfig, params.Environment, params.ConfigArgument, params.ScriptOptions.Cloud, params.ScriptOptions.External) if err != nil { return nil, err }