diff --git a/cloudapi/config.go b/cloudapi/config.go index c244130290a..402b6130b2e 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 @@ -165,44 +169,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{} @@ -211,7 +211,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) @@ -219,5 +219,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 6e84a147052..e67a2576e14 100644 --- a/cloudapi/config_test.go +++ b/cloudapi/config_test.go @@ -55,17 +55,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/archive_test.go b/cmd/archive_test.go index 7f5f6bfa4f4..acbe6944bdb 100644 --- a/cmd/archive_test.go +++ b/cmd/archive_test.go @@ -123,6 +123,110 @@ func TestArchiveContainsEnv(t *testing.T) { require.Equal(t, "ipsum", metadata.Env["ENV2"]) } +func TestArchiveContainsLegacyCloudSettings(t *testing.T) { + t.Parallel() + + // given a script with the cloud options + fileName := "script.js" + testScript := []byte(` + export let options = { + ext: { + loadimpact: { + distribution: { + one: { loadZone: 'amazon:us:ashburn', percent: 30 }, + two: { loadZone: 'amazon:ie:dublin', percent: 70 }, + }, + }, + }, + }; + export default function () {} + `) + ts := tests.NewGlobalTestState(t) + require.NoError(t, fsext.WriteFile(ts.FS, filepath.Join(ts.Cwd, fileName), testScript, 0o644)) + + // when we do archiving + ts.CmdArgs = []string{"k6", "archive", fileName} + + newRootCommand(ts.GlobalState).execute() + require.NoError(t, testutils.Untar(t, ts.FS, "archive.tar", "tmp/")) + + data, err := fsext.ReadFile(ts.FS, "tmp/metadata.json") + require.NoError(t, err) + + // we just need some basic struct + metadata := struct { + Options struct { + Ext struct { + LoadImpact struct { + Distribution map[string]struct { + LoadZone string `json:"loadZone"` + Percent float64 `json:"percent"` + } `json:"distribution"` + } `json:"loadimpact"` + } `json:"ext"` + } `json:"options"` + }{} + + // then unpacked metadata should contain a ext.loadimpact struct the proper values + require.NoError(t, json.Unmarshal(data, &metadata)) + require.Len(t, metadata.Options.Ext.LoadImpact.Distribution, 2) + + require.Equal(t, metadata.Options.Ext.LoadImpact.Distribution["one"].LoadZone, "amazon:us:ashburn") + require.Equal(t, metadata.Options.Ext.LoadImpact.Distribution["one"].Percent, 30.) + require.Equal(t, metadata.Options.Ext.LoadImpact.Distribution["two"].LoadZone, "amazon:ie:dublin") + require.Equal(t, metadata.Options.Ext.LoadImpact.Distribution["two"].Percent, 70.) +} + +func TestArchiveContainsCloudSettings(t *testing.T) { + t.Parallel() + + // given a script with the cloud options + fileName := "script.js" + testScript := []byte(` + export let options = { + cloud: { + distribution: { + one: { loadZone: 'amazon:us:ashburn', percent: 30 }, + two: { loadZone: 'amazon:ie:dublin', percent: 70 }, + }, + }, + }; + export default function () {} + `) + ts := tests.NewGlobalTestState(t) + require.NoError(t, fsext.WriteFile(ts.FS, filepath.Join(ts.Cwd, fileName), testScript, 0o644)) + + // when we do archiving + ts.CmdArgs = []string{"k6", "archive", fileName} + + newRootCommand(ts.GlobalState).execute() + require.NoError(t, testutils.Untar(t, ts.FS, "archive.tar", "tmp/")) + + data, err := fsext.ReadFile(ts.FS, "tmp/metadata.json") + require.NoError(t, err) + + // we just need some basic struct + metadata := struct { + Options struct { + Cloud struct { + Distribution map[string]struct { + LoadZone string `json:"loadZone"` + Percent float64 `json:"percent"` + } `json:"distribution"` + } `json:"cloud"` + } `json:"options"` + }{} + + // then unpacked metadata should contain options.cloud + require.NoError(t, json.Unmarshal(data, &metadata)) + require.Len(t, metadata.Options.Cloud.Distribution, 2) + + require.Equal(t, metadata.Options.Cloud.Distribution["one"].LoadZone, "amazon:us:ashburn") + require.Equal(t, metadata.Options.Cloud.Distribution["one"].Percent, 30.) + require.Equal(t, metadata.Options.Cloud.Distribution["two"].LoadZone, "amazon:ie:dublin") + require.Equal(t, metadata.Options.Cloud.Distribution["two"].Percent, 70.) +} + func TestArchiveNotContainsEnv(t *testing.T) { t.Parallel() diff --git a/cmd/cloud.go b/cmd/cloud.go index 9712d335b6e..eef83c5e19b 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 6740d50feed..fadd6cdb0db 100644 --- a/cmd/tests/cmd_run_test.go +++ b/cmd/tests/cmd_run_test.go @@ -1720,7 +1720,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 := ` @@ -1772,6 +1772,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 fafdcc4fee5..9331028fec4 100644 --- a/lib/options.go +++ b/lib/options.go @@ -307,6 +307,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"` + // These values are for third party collectors' benefit. // Can't be set through env vars. External map[string]json.RawMessage `json:"ext" ignored:"true"` @@ -468,6 +472,9 @@ func (o Options) Apply(opts Options) Options { if opts.NoCookiesReset.Valid { o.NoCookiesReset = opts.NoCookiesReset } + if opts.Cloud != nil { + o.Cloud = opts.Cloud + } if opts.External != nil { o.External = opts.External } diff --git a/output/cloud/output.go b/output/cloud/output.go index 08eb52017ea..bc670bf38dc 100644 --- a/output/cloud/output.go +++ b/output/cloud/output.go @@ -77,8 +77,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 }