diff --git a/cloudapi/config.go b/cloudapi/config.go index 603d97f66091..8e7402ba76d5 100644 --- a/cloudapi/config.go +++ b/cloudapi/config.go @@ -1,6 +1,7 @@ package cloudapi import ( + "bytes" "encoding/json" "time" @@ -9,6 +10,11 @@ import ( "gopkg.in/guregu/null.v3" ) +// LegacyCloudConfigKey is the key used in the JSON config for the cloud output. +// +// Deprecated: Remove this when migration from ext.loadimpact will be completed +const LegacyCloudConfigKey = "loadimpact" + // Config holds all the necessary data and options for sending metrics to the k6 Cloud. // //nolint:lll @@ -158,6 +164,9 @@ type Config struct { // Deprecated: Remove this when migration from the cloud output v1 will be completed MaxMetricSamplesPerPackage null.Int `json:"maxMetricSamplesPerPackage" envconfig:"K6_CLOUD_MAX_METRIC_SAMPLES_PER_PACKAGE"` + + // WarningMessage is a string that will be shown to the user if the config has some invalid values. + WarningMessage string `json:"-"` } // NewConfig creates a new Config instance with default values for some fields. @@ -199,7 +208,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 } @@ -290,33 +299,14 @@ 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. func GetConsolidatedConfig( - jsonRawConf json.RawMessage, env map[string]string, configArg string, external map[string]json.RawMessage, + jsonRawConf json.RawMessage, + env map[string]string, + configArg string, + cloudConfig json.RawMessage, + external map[string]json.RawMessage, ) (Config, error) { result := NewConfig() if jsonRawConf != nil { @@ -326,9 +316,16 @@ func GetConsolidatedConfig( } result = result.Apply(jsonConf) } - if err := MergeFromExternal(external, &result); err != nil { + + if err := mergeFromCloudOptionAndExternal(cloudConfig, external, &result); err != nil { return result, err } + // we want to show a warning if the user is using the only old way of defining the config + if cloudConfig == nil && external != nil { + if _, ok := external[LegacyCloudConfigKey]; ok { + result.WarningMessage = "The options.ext.loadimpact is deprecated, please use the new options.cloud config instead" + } + } envConfig := Config{} if err := envconfig.Process("", &envConfig, func(key string) (string, bool) { @@ -346,3 +343,83 @@ func GetConsolidatedConfig( return result, nil } + +// mergeFromCloudOptionAndExternal merges three fields from the JSON in a loadimpact key of +// the provided external map. Used for options.ext.loadimpact 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.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 +} + +// GetTemporaryCloudConfig returns a temporary cloud config. +// Original comment +// 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 +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 opt.config + 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 c0ae33770c24..82d198571ae5 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, err := GetConsolidatedConfig(json.RawMessage(`{"token":"jsonraw"}`), nil, "", nil, nil) require.NoError(t, err) require.Equal(t, config.Token.String, "jsonraw") + require.Empty(t, config.WarningMessage) - config, err = GetConsolidatedConfig(json.RawMessage(`{"token":"jsonraw"}`), nil, "", + config, 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, config.WarningMessage) + + config, 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, config.WarningMessage) +} + +func TestGetConsolidatedConfig_WithLegacyOnly(t *testing.T) { + t.Parallel() + + config, 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, config.WarningMessage) - config, err = GetConsolidatedConfig(json.RawMessage(`{"token":"jsonraw"}`), map[string]string{"K6_CLOUD_TOKEN": "envvalue"}, "", + config, 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, config.WarningMessage) +} + +func TestGetConsolidatedConfig_LegacyHasLowerPriority(t *testing.T) { + t.Parallel() + + config, 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, config.WarningMessage) +} + +func TestGetConsolidatedConfig_EnvHasHigherPriority(t *testing.T) { + t.Parallel() + + config, 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, config.WarningMessage) } diff --git a/cmd/cloud.go b/cmd/cloud.go index edc124e69f1e..8ac615961b8b 100644 --- a/cmd/cloud.go +++ b/cmd/cloud.go @@ -1,7 +1,6 @@ package cmd import ( - "bytes" "context" "encoding/json" "errors" @@ -106,33 +105,24 @@ 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) + 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) + + // Display config warning if needed + if cloudConfig.WarningMessage != "" { + modifyAndPrintBar(c.gs, progressBar, pb.WithConstProgress(0, "Warning: "+cloudConfig.WarningMessage)) } if cloudConfig.Token.Valid { @@ -148,11 +138,16 @@ 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 + // we explicitly want to use the legacy key for backwards compatibility + arc.Options.External[cloudapi.LegacyCloudConfigKey] = b //nolint:staticcheck + 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 3586dea85516..836e37a69a2f 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:]) @@ -56,10 +56,15 @@ 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) + currentJSONConfigRaw, gs.Env, "", nil, nil) if err != nil { return err } + + if consolidatedCurrentConfig.WarningMessage != "" { + gs.Logger.Warn(consolidatedCurrentConfig.WarningMessage) + } + // 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/lib/options.go b/lib/options.go index 5baad062b8fb..06721cc001fa 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 7c5f4f0a1f73..a3e4dab5789d 100644 --- a/output/cloud/output.go +++ b/output/cloud/output.go @@ -79,11 +79,20 @@ 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) + params.JSONConfig, + params.Environment, + params.ConfigArgument, + params.ScriptOptions.Cloud, + params.ScriptOptions.External, + ) if err != nil { return nil, err } + if conf.WarningMessage != "" { + params.Logger.Warn(conf.WarningMessage) + } + if err := validateRequiredSystemTags(params.ScriptOptions.SystemTags); err != nil { return nil, err } diff --git a/output/cloud/v1/output_test.go b/output/cloud/v1/output_test.go index 0d4e26a9259e..cdfeca9cba18 100644 --- a/output/cloud/v1/output_test.go +++ b/output/cloud/v1/output_test.go @@ -714,7 +714,7 @@ 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) + params.JSONConfig, params.Environment, params.ConfigArgument, params.ScriptOptions.Cloud, params.ScriptOptions.External) if err != nil { return nil, err }