Introducing options.cloud

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
+}

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)
 }

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)

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

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"`

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
 	}