From 3d1a447aaf40be462f9368fec533796daa6d09aa Mon Sep 17 00:00:00 2001 From: codebien <2103732+codebien@users.noreply.github.com> Date: Thu, 30 Jan 2025 12:39:21 +0100 Subject: [PATCH 01/13] Set a warning if the legacy config path is found --- internal/cmd/config.go | 34 ++++++++++++++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) diff --git a/internal/cmd/config.go b/internal/cmd/config.go index bdb5e3c9a88..d6be0baa381 100644 --- a/internal/cmd/config.go +++ b/internal/cmd/config.go @@ -124,6 +124,10 @@ func getConfig(flags *pflag.FlagSet) (Config, error) { }, nil } +func legacyConfigFilePath(gs *state.GlobalState) string { + return filepath.Join(gs.UserOSConfigDir, "loadimpact", "k6", "config.json") +} + // Reads the configuration file from the supplied filesystem and returns it or // an error. The only situation in which an error won't be returned is if the // user didn't explicitly specify a config file path and the default config file @@ -151,6 +155,24 @@ func readDiskConfig(gs *state.GlobalState) (Config, error) { return conf, nil } +func readLegacyDiskConfig(gs *state.GlobalState) (Config, error) { + // Try to see if the legacy config exists in the supplied filesystem + legacyPath := filepath.Join(gs.UserOSConfigDir, "loadimpact", "k6", "config.json") + if _, err := gs.FS.Stat(legacyPath); err != nil { + return Config{}, err + } + data, err := fsext.ReadFile(gs.FS, legacyPath) + if err != nil { + return Config{}, fmt.Errorf("couldn't load the configuration from %q: %w", legacyPath, err) + } + var conf Config + err = json.Unmarshal(data, &conf) + if err != nil { + return Config{}, fmt.Errorf("couldn't parse the configuration from %q: %w", legacyPath, err) + } + return conf, nil +} + // Serializes the configuration to a JSON file and writes it in the supplied // location on the supplied filesystem func writeDiskConfig(gs *state.GlobalState, conf Config) error { @@ -187,10 +209,18 @@ 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) { - fileConf, err := readDiskConfig(gs) - if err != nil { + fileConf, err := readLegacyDiskConfig(gs) + if errors.Is(err, fs.ErrNotExist) { + fileConf, err = readDiskConfig(gs) + if err != nil { + return conf, errext.WithExitCodeIfNone(err, exitcodes.InvalidConfig) + } + } else if err != nil { return conf, errext.WithExitCodeIfNone(err, exitcodes.InvalidConfig) + } else { + gs.Logger.Warn("The configuration file has been found on the old path. Please, run again `k6 cloud login` or `k6 login` commands to migrate it to the new path. If you migrated it manually, then remove the old config file.") } + envConf, err := readEnvConfig(gs.Env) if err != nil { return conf, errext.WithExitCodeIfNone(err, exitcodes.InvalidConfig) From 0f91d527c22f16a71c29a676d4878e38969ca852 Mon Sep 17 00:00:00 2001 From: codebien <2103732+codebien@users.noreply.github.com> Date: Thu, 30 Jan 2025 12:42:37 +0100 Subject: [PATCH 02/13] Changed the default config file path --- cmd/state/state.go | 50 ++++++++++++++++++++++++---------------------- 1 file changed, 26 insertions(+), 24 deletions(-) diff --git a/cmd/state/state.go b/cmd/state/state.go index 4d375ff50f4..ae2fabc88ac 100644 --- a/cmd/state/state.go +++ b/cmd/state/state.go @@ -35,12 +35,13 @@ const defaultConfigFileName = "config.json" type GlobalState struct { Ctx context.Context - FS fsext.Fs - Getwd func() (string, error) - BinaryName string - CmdArgs []string - Env map[string]string - Events *event.System + FS fsext.Fs + Getwd func() (string, error) + UserOSConfigDir string + BinaryName string + CmdArgs []string + Env map[string]string + Events *event.System DefaultFlags, Flags GlobalFlags @@ -106,23 +107,24 @@ func NewGlobalState(ctx context.Context) *GlobalState { defaultFlags := GetDefaultFlags(confDir) return &GlobalState{ - Ctx: ctx, - FS: fsext.NewOsFs(), - Getwd: os.Getwd, - BinaryName: filepath.Base(binary), - CmdArgs: os.Args, - Env: env, - Events: event.NewEventSystem(100, logger), - DefaultFlags: defaultFlags, - Flags: getFlags(defaultFlags, env), - OutMutex: outMutex, - Stdout: stdout, - Stderr: stderr, - Stdin: os.Stdin, - OSExit: os.Exit, - SignalNotify: signal.Notify, - SignalStop: signal.Stop, - Logger: logger, + Ctx: ctx, + FS: fsext.NewOsFs(), + Getwd: os.Getwd, + UserOSConfigDir: confDir, + BinaryName: filepath.Base(binary), + CmdArgs: os.Args, + Env: env, + Events: event.NewEventSystem(100, logger), + DefaultFlags: defaultFlags, + Flags: getFlags(defaultFlags, env), + OutMutex: outMutex, + Stdout: stdout, + Stderr: stderr, + Stdin: os.Stdin, + OSExit: os.Exit, + SignalNotify: signal.Notify, + SignalStop: signal.Stop, + Logger: logger, FallbackLogger: &logrus.Logger{ // we may modify the other one Out: stderr, Formatter: new(logrus.TextFormatter), // no fancy formatting here @@ -149,7 +151,7 @@ func GetDefaultFlags(homeDir string) GlobalFlags { return GlobalFlags{ Address: "localhost:6565", ProfilingEnabled: false, - ConfigFilePath: filepath.Join(homeDir, "loadimpact", "k6", defaultConfigFileName), + ConfigFilePath: filepath.Join(homeDir, "k6", defaultConfigFileName), LogOutput: "stderr", } } From 5d21cdbf63b51e45d1e12c931aee97051bd51efa Mon Sep 17 00:00:00 2001 From: codebien <2103732+codebien@users.noreply.github.com> Date: Thu, 30 Jan 2025 16:14:25 +0100 Subject: [PATCH 03/13] Call the migration procedure from login commands --- internal/cmd/cloud_login.go | 7 ++++++- internal/cmd/login_cloud.go | 4 ++++ internal/cmd/login_influxdb.go | 4 ++++ 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/internal/cmd/cloud_login.go b/internal/cmd/cloud_login.go index e220315a285..83f95ccdba7 100644 --- a/internal/cmd/cloud_login.go +++ b/internal/cmd/cloud_login.go @@ -38,7 +38,7 @@ func getCmdCloudLogin(gs *state.GlobalState) *cobra.Command { # Display the stored token $ {{.}} cloud login -s - + # Reset the stored token $ {{.}} cloud login -r`[1:]) @@ -66,6 +66,11 @@ the "k6 run -o cloud" command. // run is the code that runs when the user executes `k6 cloud login` func (c *cmdCloudLogin) run(cmd *cobra.Command, _ []string) error { + err := migrateLegacyConfigFileIfAny(c.globalState) + if err != nil { + return err + } + currentDiskConf, err := readDiskConfig(c.globalState) if err != nil { return err diff --git a/internal/cmd/login_cloud.go b/internal/cmd/login_cloud.go index 95d1376f98c..6fb03a1d868 100644 --- a/internal/cmd/login_cloud.go +++ b/internal/cmd/login_cloud.go @@ -42,6 +42,10 @@ This will set the default token used when just "k6 run -o cloud" is passed.`, Please use the "k6 cloud login" command instead. `, RunE: func(cmd *cobra.Command, _ []string) error { + if err := migrateLegacyConfigFileIfAny(gs); err != nil { + return err + } + currentDiskConf, err := readDiskConfig(gs) if err != nil { return err diff --git a/internal/cmd/login_influxdb.go b/internal/cmd/login_influxdb.go index ca5e3bfb00a..96e56f83c8b 100644 --- a/internal/cmd/login_influxdb.go +++ b/internal/cmd/login_influxdb.go @@ -25,6 +25,10 @@ func getCmdLoginInfluxDB(gs *state.GlobalState) *cobra.Command { This will set the default server used when just "-o influxdb" is passed.`, Args: cobra.MaximumNArgs(1), RunE: func(_ *cobra.Command, args []string) error { + if err := migrateLegacyConfigFileIfAny(gs); err != nil { + return err + } + config, err := readDiskConfig(gs) if err != nil { return err From 2837203c326937427b3f72bc7947d0032c485f8b Mon Sep 17 00:00:00 2001 From: codebien <2103732+codebien@users.noreply.github.com> Date: Thu, 30 Jan 2025 16:15:06 +0100 Subject: [PATCH 04/13] Migration to the new default config path --- internal/cmd/config.go | 41 +++++++++++++++++++++++---- internal/cmd/config_test.go | 55 ++++++++++++++++++++++++++++++++++--- 2 files changed, 87 insertions(+), 9 deletions(-) diff --git a/internal/cmd/config.go b/internal/cmd/config.go index d6be0baa381..64a6c18795d 100644 --- a/internal/cmd/config.go +++ b/internal/cmd/config.go @@ -124,10 +124,6 @@ func getConfig(flags *pflag.FlagSet) (Config, error) { }, nil } -func legacyConfigFilePath(gs *state.GlobalState) string { - return filepath.Join(gs.UserOSConfigDir, "loadimpact", "k6", "config.json") -} - // Reads the configuration file from the supplied filesystem and returns it or // an error. The only situation in which an error won't be returned is if the // user didn't explicitly specify a config file path and the default config file @@ -155,6 +151,10 @@ func readDiskConfig(gs *state.GlobalState) (Config, error) { return conf, nil } +func legacyConfigFilePath(gs *state.GlobalState) string { + return filepath.Join(gs.UserOSConfigDir, "loadimpact", "k6", "config.json") +} + func readLegacyDiskConfig(gs *state.GlobalState) (Config, error) { // Try to see if the legacy config exists in the supplied filesystem legacyPath := filepath.Join(gs.UserOSConfigDir, "loadimpact", "k6", "config.json") @@ -218,7 +218,7 @@ func getConsolidatedConfig(gs *state.GlobalState, cliConf Config, runnerOpts lib } else if err != nil { return conf, errext.WithExitCodeIfNone(err, exitcodes.InvalidConfig) } else { - gs.Logger.Warn("The configuration file has been found on the old path. Please, run again `k6 cloud login` or `k6 login` commands to migrate it to the new path. If you migrated it manually, then remove the old config file.") + gs.Logger.Warn("The configuration file has been found on the old path. Please, run again `k6 cloud login` or `k6 login` commands to migrate to the new path. If you already migrated it manually, then remove the file from the old path.\n\n") } envConf, err := readEnvConfig(gs.Env) @@ -333,3 +333,34 @@ func validateScenarioConfig(conf lib.ExecutorConfig, isExecutable func(string) b } return nil } + +// migrateLegacyConfigFileIfAny moves the configuration file from +// the old default `~/.config/loadimpact/...` folder +// to the new `~/.config/k6/...` default folder. +func migrateLegacyConfigFileIfAny(gs *state.GlobalState) error { + fn := func() error { + legacyFpath := legacyConfigFilePath(gs) + if _, err := gs.FS.Stat(legacyFpath); errors.Is(err, fs.ErrNotExist) { + return nil + } else if err != nil { + return err + } + + if err := gs.FS.MkdirAll(filepath.Dir(gs.Flags.ConfigFilePath), 0o755); err != nil { + return err + } + + err := gs.FS.Rename(legacyFpath, gs.Flags.ConfigFilePath) + if err != nil { + return err + } + + gs.Logger.Infof("Note, the configuration file has been migrated "+ + "from old default path (%q) to the new version (%q).\n\n", legacyFpath, gs.Flags.ConfigFilePath) + return nil + } + if err := fn(); err != nil { + return fmt.Errorf("move from the old to the new configuration's filepath failed: %w", err) + } + return nil +} diff --git a/internal/cmd/config_test.go b/internal/cmd/config_test.go index 9c03d320598..52953ea2507 100644 --- a/internal/cmd/config_test.go +++ b/internal/cmd/config_test.go @@ -2,11 +2,13 @@ package cmd import ( "encoding/json" + "io" "io/fs" "testing" "time" "github.com/mstoykov/envconfig" + "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "gopkg.in/guregu/null.v3" @@ -213,7 +215,7 @@ func TestReadDiskConfigWithDefaultFlags(t *testing.T) { memfs := fsext.NewMemMapFs() conf := []byte(`{"iterations":1028,"cloud":{"field1":"testvalue"}}`) - defaultConfigPath := ".config/loadimpact/k6/config.json" + defaultConfigPath := ".config/k6/config.json" require.NoError(t, fsext.WriteFile(memfs, defaultConfigPath, conf, 0o644)) defaultFlags := state.GetDefaultFlags(".config") @@ -298,7 +300,7 @@ func TestReadDiskConfigNotJSONContentError(t *testing.T) { memfs := fsext.NewMemMapFs() conf := []byte(`bad json format`) - defaultConfigPath := ".config/loadimpact/k6/config.json" + defaultConfigPath := ".config/k6/config.json" require.NoError(t, fsext.WriteFile(memfs, defaultConfigPath, conf, 0o644)) gs := &state.GlobalState{ @@ -342,7 +344,7 @@ func TestWriteDiskConfigWithDefaultFlags(t *testing.T) { err := writeDiskConfig(gs, c) require.NoError(t, err) - finfo, err := memfs.Stat(".config/loadimpact/k6/config.json") + finfo, err := memfs.Stat(".config/k6/config.json") require.NoError(t, err) assert.NotEmpty(t, finfo.Size()) } @@ -352,7 +354,7 @@ func TestWriteDiskConfigOverwrite(t *testing.T) { memfs := fsext.NewMemMapFs() conf := []byte(`{"iterations":1028,"cloud":{"field1":"testvalue"}}`) - defaultConfigPath := ".config/loadimpact/k6/config.json" + defaultConfigPath := ".config/k6/config.json" require.NoError(t, fsext.WriteFile(memfs, defaultConfigPath, conf, 0o644)) defaultFlags := state.GetDefaultFlags(".config") @@ -405,3 +407,48 @@ func TestWriteDiskConfigNoJSONContentError(t *testing.T) { var serr *json.SyntaxError assert.ErrorAs(t, err, &serr) } + +func TestMigrateLegacyConfigFileIfAny(t *testing.T) { + memfs := fsext.NewMemMapFs() + + conf := []byte(`{"iterations":1028,"cloud":{"field1":"testvalue"}}`) + legacyConfigPath := ".config/loadimpact/k6/config.json" + fsext.WriteFile(memfs, legacyConfigPath, conf, 0o644) + + logger := logrus.New() + logger.SetOutput(io.Discard) + + defaultFlags := state.GetDefaultFlags(".config") + gs := &state.GlobalState{ + FS: memfs, + Flags: defaultFlags, + DefaultFlags: defaultFlags, + UserOSConfigDir: ".config", + Logger: logger, + } + + err := migrateLegacyConfigFileIfAny(gs) + require.NoError(t, err) + + f, err := fsext.ReadFile(memfs, ".config/k6/config.json") + require.NoError(t, err) + assert.Equal(t, f, conf) + + _, err = memfs.Stat(legacyConfigPath) + assert.ErrorIs(t, err, fs.ErrNotExist) +} + +func TestMigrateLegacyConfigFileIfAnyWhenFileDoesNotExist(t *testing.T) { + memfs := fsext.NewMemMapFs() + + defaultFlags := state.GetDefaultFlags(".config") + gs := &state.GlobalState{ + FS: memfs, + Flags: defaultFlags, + DefaultFlags: defaultFlags, + UserOSConfigDir: ".config", + } + + err := migrateLegacyConfigFileIfAny(gs) + require.NoError(t, err) +} From 43a3cd99aa8fba484ca7c45803aa34b0123a162a Mon Sep 17 00:00:00 2001 From: codebien <2103732+codebien@users.noreply.github.com> Date: Thu, 30 Jan 2025 17:12:43 +0100 Subject: [PATCH 05/13] Fix lint errors --- internal/cmd/config.go | 6 ++++-- internal/cmd/config_test.go | 4 +++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/internal/cmd/config.go b/internal/cmd/config.go index 64a6c18795d..d0442262faa 100644 --- a/internal/cmd/config.go +++ b/internal/cmd/config.go @@ -210,7 +210,7 @@ func readEnvConfig(envMap map[string]string) (Config, error) { // TODO: accumulate all errors and differentiate between the layers? func getConsolidatedConfig(gs *state.GlobalState, cliConf Config, runnerOpts lib.Options) (conf Config, err error) { fileConf, err := readLegacyDiskConfig(gs) - if errors.Is(err, fs.ErrNotExist) { + if errors.Is(err, fs.ErrNotExist) { //nolint:gocritic fileConf, err = readDiskConfig(gs) if err != nil { return conf, errext.WithExitCodeIfNone(err, exitcodes.InvalidConfig) @@ -218,7 +218,9 @@ func getConsolidatedConfig(gs *state.GlobalState, cliConf Config, runnerOpts lib } else if err != nil { return conf, errext.WithExitCodeIfNone(err, exitcodes.InvalidConfig) } else { - gs.Logger.Warn("The configuration file has been found on the old path. Please, run again `k6 cloud login` or `k6 login` commands to migrate to the new path. If you already migrated it manually, then remove the file from the old path.\n\n") + gs.Logger.Warn("The configuration file has been found on the old path. " + + "Please, run again `k6 cloud login` or `k6 login` commands to migrate to the new path. " + + "If you already migrated it manually, then remove the file from the old path.\n\n") } envConf, err := readEnvConfig(gs.Env) diff --git a/internal/cmd/config_test.go b/internal/cmd/config_test.go index 52953ea2507..6f8f62ede91 100644 --- a/internal/cmd/config_test.go +++ b/internal/cmd/config_test.go @@ -409,11 +409,12 @@ func TestWriteDiskConfigNoJSONContentError(t *testing.T) { } func TestMigrateLegacyConfigFileIfAny(t *testing.T) { + t.Parallel() memfs := fsext.NewMemMapFs() conf := []byte(`{"iterations":1028,"cloud":{"field1":"testvalue"}}`) legacyConfigPath := ".config/loadimpact/k6/config.json" - fsext.WriteFile(memfs, legacyConfigPath, conf, 0o644) + require.NoError(t, fsext.WriteFile(memfs, legacyConfigPath, conf, 0o644)) logger := logrus.New() logger.SetOutput(io.Discard) @@ -439,6 +440,7 @@ func TestMigrateLegacyConfigFileIfAny(t *testing.T) { } func TestMigrateLegacyConfigFileIfAnyWhenFileDoesNotExist(t *testing.T) { + t.Parallel() memfs := fsext.NewMemMapFs() defaultFlags := state.GetDefaultFlags(".config") From 364b3695d55a1a042ee24b57723bbed37ba34267 Mon Sep 17 00:00:00 2001 From: codebien <2103732+codebien@users.noreply.github.com> Date: Tue, 4 Feb 2025 11:04:31 +0100 Subject: [PATCH 06/13] Do not use named return params --- internal/cmd/config.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/internal/cmd/config.go b/internal/cmd/config.go index d0442262faa..214affd5743 100644 --- a/internal/cmd/config.go +++ b/internal/cmd/config.go @@ -208,15 +208,15 @@ func readEnvConfig(envMap map[string]string) (Config, error) { // - set some defaults if they weren't previously specified // 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) { +func getConsolidatedConfig(gs *state.GlobalState, cliConf Config, runnerOpts lib.Options) (Config, error) { fileConf, err := readLegacyDiskConfig(gs) if errors.Is(err, fs.ErrNotExist) { //nolint:gocritic fileConf, err = readDiskConfig(gs) if err != nil { - return conf, errext.WithExitCodeIfNone(err, exitcodes.InvalidConfig) + return Config{}, errext.WithExitCodeIfNone(err, exitcodes.InvalidConfig) } } else if err != nil { - return conf, errext.WithExitCodeIfNone(err, exitcodes.InvalidConfig) + return Config{}, errext.WithExitCodeIfNone(err, exitcodes.InvalidConfig) } else { gs.Logger.Warn("The configuration file has been found on the old path. " + "Please, run again `k6 cloud login` or `k6 login` commands to migrate to the new path. " + @@ -225,10 +225,10 @@ func getConsolidatedConfig(gs *state.GlobalState, cliConf Config, runnerOpts lib envConf, err := readEnvConfig(gs.Env) if err != nil { - return conf, errext.WithExitCodeIfNone(err, exitcodes.InvalidConfig) + return Config{}, errext.WithExitCodeIfNone(err, exitcodes.InvalidConfig) } - conf = cliConf.Apply(fileConf) + conf := cliConf.Apply(fileConf) warnOnShortHandOverride(conf.Options, runnerOpts, "script", gs.Logger) conf = conf.Apply(Config{Options: runnerOpts}) @@ -247,7 +247,7 @@ func getConsolidatedConfig(gs *state.GlobalState, cliConf Config, runnerOpts lib // (e.g. env vars) overrode our default value. This is not done in // lib.Options.Validate to avoid circular imports. if _, err = metrics.GetResolversForTrendColumns(conf.SummaryTrendStats); err != nil { - return conf, err + return Config{}, err } return conf, nil From 70a236843fb1e6146fc08f28e85842e9a440bc92 Mon Sep 17 00:00:00 2001 From: codebien <2103732+codebien@users.noreply.github.com> Date: Tue, 4 Feb 2025 11:11:26 +0100 Subject: [PATCH 07/13] Reuse the utility function for the legacy path generation --- internal/cmd/config.go | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/internal/cmd/config.go b/internal/cmd/config.go index 214affd5743..e0d87b329dc 100644 --- a/internal/cmd/config.go +++ b/internal/cmd/config.go @@ -151,13 +151,15 @@ func readDiskConfig(gs *state.GlobalState) (Config, error) { return conf, nil } +// legacyConfigFilePath returns the path of the old location, +// which is now deprecated and superseded by a new default. func legacyConfigFilePath(gs *state.GlobalState) string { return filepath.Join(gs.UserOSConfigDir, "loadimpact", "k6", "config.json") } func readLegacyDiskConfig(gs *state.GlobalState) (Config, error) { - // Try to see if the legacy config exists in the supplied filesystem - legacyPath := filepath.Join(gs.UserOSConfigDir, "loadimpact", "k6", "config.json") + // CHeck if the legacy config exists in the supplied filesystem + legacyPath := legacyConfigFilePath(gs) if _, err := gs.FS.Stat(legacyPath); err != nil { return Config{}, err } @@ -342,9 +344,11 @@ func validateScenarioConfig(conf lib.ExecutorConfig, isExecutable func(string) b func migrateLegacyConfigFileIfAny(gs *state.GlobalState) error { fn := func() error { legacyFpath := legacyConfigFilePath(gs) - if _, err := gs.FS.Stat(legacyFpath); errors.Is(err, fs.ErrNotExist) { + _, err := gs.FS.Stat(legacyFpath) + if errors.Is(err, fs.ErrNotExist) { return nil - } else if err != nil { + } + if err != nil { return err } @@ -352,7 +356,7 @@ func migrateLegacyConfigFileIfAny(gs *state.GlobalState) error { return err } - err := gs.FS.Rename(legacyFpath, gs.Flags.ConfigFilePath) + err = gs.FS.Rename(legacyFpath, gs.Flags.ConfigFilePath) if err != nil { return err } From 89f454f82ed6bd2c4ec533b531d6625b00e501f2 Mon Sep 17 00:00:00 2001 From: codebien <2103732+codebien@users.noreply.github.com> Date: Tue, 4 Feb 2025 19:05:43 +0100 Subject: [PATCH 08/13] Load the legacy only after attempted the default --- internal/cmd/config.go | 48 ++++++++++++++++++++++++++++++++---------- 1 file changed, 37 insertions(+), 11 deletions(-) diff --git a/internal/cmd/config.go b/internal/cmd/config.go index e0d87b329dc..ebf32157d6b 100644 --- a/internal/cmd/config.go +++ b/internal/cmd/config.go @@ -201,6 +201,40 @@ func readEnvConfig(envMap map[string]string) (Config, error) { return conf, err } +// loadConfigFile wraps the ordinary readDiskConfig operation. +// It adds the capability to fallbacks on the legacy default path if required. +// +// Unfortunately, readDiskConfig() silences the NotFound error. +// We don't want to change it as it is used across several places; +// and, hopefully, this code will be available only for a single major version. +// After we should restore to lookup only in a single location for config file (the default). +func loadConfigFile(gs *state.GlobalState) (Config, error) { + // use directly the main flow if the user passed a custom path + if gs.Flags.ConfigFilePath != gs.DefaultFlags.ConfigFilePath { + return readDiskConfig(gs) + } + + _, err := gs.FS.Stat(gs.Flags.ConfigFilePath) + if err != nil && errors.Is(err, fs.ErrNotExist) { + // if the passed path does not exist (custom one or the default) + // then we attempt to load the legacy path + legacyConf, legacyErr := readLegacyDiskConfig(gs) + if legacyErr != nil && !errors.Is(legacyErr, fs.ErrNotExist) { + return Config{}, legacyErr + } + // a legacy file has been found + if legacyErr == nil { + gs.Logger.Warn("The configuration file has been found on the old path. " + + "Please, run again `k6 cloud login` or `k6 login` commands to migrate to the new path. " + + "If you already migrated it manually, then remove the file from the old path.\n\n") + return legacyConf, nil + } + // the legacy file doesn't exist, then we fallback on the main flow + // to return the silenced error for not existing config file + } + return readDiskConfig(gs) +} + // Assemble the final consolidated configuration from all of the different sources: // - start with the CLI-provided options to get shadowed (non-Valid) defaults in there // - add the global file config options @@ -211,18 +245,10 @@ 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) (Config, error) { - fileConf, err := readLegacyDiskConfig(gs) - if errors.Is(err, fs.ErrNotExist) { //nolint:gocritic - fileConf, err = readDiskConfig(gs) - if err != nil { - return Config{}, errext.WithExitCodeIfNone(err, exitcodes.InvalidConfig) - } - } else if err != nil { + fileConf, err := loadConfigFile(gs) + if err != nil { + err = fmt.Errorf("failed to load the configuration file from the local file system: %w", err) return Config{}, errext.WithExitCodeIfNone(err, exitcodes.InvalidConfig) - } else { - gs.Logger.Warn("The configuration file has been found on the old path. " + - "Please, run again `k6 cloud login` or `k6 login` commands to migrate to the new path. " + - "If you already migrated it manually, then remove the file from the old path.\n\n") } envConf, err := readEnvConfig(gs.Env) From e07da87694849ad1c942f2700ad33f205944b339 Mon Sep 17 00:00:00 2001 From: codebien <2103732+codebien@users.noreply.github.com> Date: Wed, 5 Feb 2025 10:30:10 +0100 Subject: [PATCH 09/13] Adjust logs text --- internal/cmd/config.go | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/internal/cmd/config.go b/internal/cmd/config.go index ebf32157d6b..947ccec4640 100644 --- a/internal/cmd/config.go +++ b/internal/cmd/config.go @@ -158,7 +158,7 @@ func legacyConfigFilePath(gs *state.GlobalState) string { } func readLegacyDiskConfig(gs *state.GlobalState) (Config, error) { - // CHeck if the legacy config exists in the supplied filesystem + // Check if the legacy config exists in the supplied filesystem legacyPath := legacyConfigFilePath(gs) if _, err := gs.FS.Stat(legacyPath); err != nil { return Config{}, err @@ -224,9 +224,8 @@ func loadConfigFile(gs *state.GlobalState) (Config, error) { } // a legacy file has been found if legacyErr == nil { - gs.Logger.Warn("The configuration file has been found on the old path. " + - "Please, run again `k6 cloud login` or `k6 login` commands to migrate to the new path. " + - "If you already migrated it manually, then remove the file from the old path.\n\n") + gs.Logger.Warnf("The configuration file has been found on the old path (%q). "+ + "Please, run again `k6 cloud login` or `k6 login` commands to migrate to the new path.", legacyConfigFilePath(gs)) return legacyConf, nil } // the legacy file doesn't exist, then we fallback on the main flow @@ -377,18 +376,15 @@ func migrateLegacyConfigFileIfAny(gs *state.GlobalState) error { if err != nil { return err } - if err := gs.FS.MkdirAll(filepath.Dir(gs.Flags.ConfigFilePath), 0o755); err != nil { return err } - err = gs.FS.Rename(legacyFpath, gs.Flags.ConfigFilePath) if err != nil { return err } - gs.Logger.Infof("Note, the configuration file has been migrated "+ - "from old default path (%q) to the new version (%q).\n\n", legacyFpath, gs.Flags.ConfigFilePath) + "from the old default path (%q) to the new one (%q).\n\n", legacyFpath, gs.Flags.ConfigFilePath) return nil } if err := fn(); err != nil { From 3cb578d94f28a17148a0ab5acf8636b79150516a Mon Sep 17 00:00:00 2001 From: codebien <2103732+codebien@users.noreply.github.com> Date: Thu, 6 Feb 2025 12:01:31 +0100 Subject: [PATCH 10/13] Use always the default path for the migration --- internal/cmd/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/cmd/config.go b/internal/cmd/config.go index 947ccec4640..dfe05e7cd4f 100644 --- a/internal/cmd/config.go +++ b/internal/cmd/config.go @@ -376,7 +376,7 @@ func migrateLegacyConfigFileIfAny(gs *state.GlobalState) error { if err != nil { return err } - if err := gs.FS.MkdirAll(filepath.Dir(gs.Flags.ConfigFilePath), 0o755); err != nil { + if err := gs.FS.MkdirAll(filepath.Dir(gs.DefaultFlags.ConfigFilePath), 0o755); err != nil { return err } err = gs.FS.Rename(legacyFpath, gs.Flags.ConfigFilePath) From 7096b8c76954b092b5385112674dbe32daa3e29a Mon Sep 17 00:00:00 2001 From: codebien <2103732+codebien@users.noreply.github.com> Date: Thu, 6 Feb 2025 12:44:01 +0100 Subject: [PATCH 11/13] Do not delete the old path --- internal/cmd/config.go | 18 ++++++++++++++---- internal/cmd/config_test.go | 12 +++++++----- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/internal/cmd/config.go b/internal/cmd/config.go index dfe05e7cd4f..b2ff5b262a7 100644 --- a/internal/cmd/config.go +++ b/internal/cmd/config.go @@ -363,9 +363,11 @@ func validateScenarioConfig(conf lib.ExecutorConfig, isExecutable func(string) b return nil } -// migrateLegacyConfigFileIfAny moves the configuration file from +// migrateLegacyConfigFileIfAny copies the configuration file from // the old default `~/.config/loadimpact/...` folder // to the new `~/.config/k6/...` default folder. +// If the old file is not found no error is returned. +// It keeps the old file as a backup. func migrateLegacyConfigFileIfAny(gs *state.GlobalState) error { fn := func() error { legacyFpath := legacyConfigFilePath(gs) @@ -376,15 +378,23 @@ func migrateLegacyConfigFileIfAny(gs *state.GlobalState) error { if err != nil { return err } - if err := gs.FS.MkdirAll(filepath.Dir(gs.DefaultFlags.ConfigFilePath), 0o755); err != nil { + newPath := gs.DefaultFlags.ConfigFilePath + if err := gs.FS.MkdirAll(filepath.Dir(newPath), 0o755); err != nil { return err } - err = gs.FS.Rename(legacyFpath, gs.Flags.ConfigFilePath) + // copy the config file leaving the old available as a backup + f, err := fsext.ReadFile(gs.FS, legacyFpath) + if err != nil { + return err + } + err = fsext.WriteFile(gs.FS, newPath, f, 0o644) if err != nil { return err } gs.Logger.Infof("Note, the configuration file has been migrated "+ - "from the old default path (%q) to the new one (%q).\n\n", legacyFpath, gs.Flags.ConfigFilePath) + "from the old default path (%q) to the new one (%q). "+ + "Clean up the old path after you verified that you can run tests by using the new configuration.\n\n", + legacyFpath, newPath) return nil } if err := fn(); err != nil { diff --git a/internal/cmd/config_test.go b/internal/cmd/config_test.go index 6f8f62ede91..cbf82c741b2 100644 --- a/internal/cmd/config_test.go +++ b/internal/cmd/config_test.go @@ -2,7 +2,6 @@ package cmd import ( "encoding/json" - "io" "io/fs" "testing" "time" @@ -16,6 +15,7 @@ import ( "go.k6.io/k6/cmd/state" "go.k6.io/k6/errext" "go.k6.io/k6/errext/exitcodes" + "go.k6.io/k6/internal/lib/testutils" "go.k6.io/k6/lib" "go.k6.io/k6/lib/executor" "go.k6.io/k6/lib/fsext" @@ -416,8 +416,8 @@ func TestMigrateLegacyConfigFileIfAny(t *testing.T) { legacyConfigPath := ".config/loadimpact/k6/config.json" require.NoError(t, fsext.WriteFile(memfs, legacyConfigPath, conf, 0o644)) - logger := logrus.New() - logger.SetOutput(io.Discard) + l, hook := testutils.NewLoggerWithHook(t) + logger := l.(*logrus.Logger) defaultFlags := state.GetDefaultFlags(".config") gs := &state.GlobalState{ @@ -435,8 +435,7 @@ func TestMigrateLegacyConfigFileIfAny(t *testing.T) { require.NoError(t, err) assert.Equal(t, f, conf) - _, err = memfs.Stat(legacyConfigPath) - assert.ErrorIs(t, err, fs.ErrNotExist) + testutils.LogContains(hook.Drain(), logrus.InfoLevel, "migrated") } func TestMigrateLegacyConfigFileIfAnyWhenFileDoesNotExist(t *testing.T) { @@ -453,4 +452,7 @@ func TestMigrateLegacyConfigFileIfAnyWhenFileDoesNotExist(t *testing.T) { err := migrateLegacyConfigFileIfAny(gs) require.NoError(t, err) + + _, err = fsext.ReadFile(memfs, ".config/k6/config.json") + assert.ErrorIs(t, err, fs.ErrNotExist) } From fb14f73b835e4d1c0f29069e9ea106ea0e386b8c Mon Sep 17 00:00:00 2001 From: codebien <2103732+codebien@users.noreply.github.com> Date: Thu, 6 Feb 2025 14:42:00 +0100 Subject: [PATCH 12/13] Added tests for load config operation --- internal/cmd/config.go | 7 ++-- internal/cmd/config_test.go | 70 ++++++++++++++++++++++++++++++++++++- 2 files changed, 73 insertions(+), 4 deletions(-) diff --git a/internal/cmd/config.go b/internal/cmd/config.go index b2ff5b262a7..8981d7d1ebe 100644 --- a/internal/cmd/config.go +++ b/internal/cmd/config.go @@ -216,7 +216,7 @@ func loadConfigFile(gs *state.GlobalState) (Config, error) { _, err := gs.FS.Stat(gs.Flags.ConfigFilePath) if err != nil && errors.Is(err, fs.ErrNotExist) { - // if the passed path does not exist (custom one or the default) + // if the passed path (the default) does not exist // then we attempt to load the legacy path legacyConf, legacyErr := readLegacyDiskConfig(gs) if legacyErr != nil && !errors.Is(legacyErr, fs.ErrNotExist) { @@ -224,8 +224,9 @@ func loadConfigFile(gs *state.GlobalState) (Config, error) { } // a legacy file has been found if legacyErr == nil { - gs.Logger.Warnf("The configuration file has been found on the old path (%q). "+ - "Please, run again `k6 cloud login` or `k6 login` commands to migrate to the new path.", legacyConfigFilePath(gs)) + gs.Logger.Warnf("The configuration file has been found on the old default path (%q). "+ + "Please, run again `k6 cloud login` or `k6 login` commands to migrate to the new default path.\n\n", + legacyConfigFilePath(gs)) return legacyConf, nil } // the legacy file doesn't exist, then we fallback on the main flow diff --git a/internal/cmd/config_test.go b/internal/cmd/config_test.go index cbf82c741b2..81e31fbbe37 100644 --- a/internal/cmd/config_test.go +++ b/internal/cmd/config_test.go @@ -417,7 +417,7 @@ func TestMigrateLegacyConfigFileIfAny(t *testing.T) { require.NoError(t, fsext.WriteFile(memfs, legacyConfigPath, conf, 0o644)) l, hook := testutils.NewLoggerWithHook(t) - logger := l.(*logrus.Logger) + logger := l.(*logrus.Logger) //nolint:forbidigo // no alternative, required defaultFlags := state.GetDefaultFlags(".config") gs := &state.GlobalState{ @@ -456,3 +456,71 @@ func TestMigrateLegacyConfigFileIfAnyWhenFileDoesNotExist(t *testing.T) { _, err = fsext.ReadFile(memfs, ".config/k6/config.json") assert.ErrorIs(t, err, fs.ErrNotExist) } + +func TestLoadConfig(t *testing.T) { + t.Parallel() + + testcases := []struct { + name string + memfs fsext.Fs + expConf Config + expLegacyWarn bool + }{ + { + name: "old and new default paths both populated", + memfs: testutils.MakeMemMapFs(t, map[string][]byte{ + ".config/loadimpact/k6/config.json": []byte(`{"iterations":1028}`), + ".config/k6/config.json": []byte(`{"iterations":1027}`), // use different conf to be sure on assertions + }), + expConf: Config{Options: lib.Options{Iterations: null.IntFrom(1027)}}, + expLegacyWarn: false, + }, + { + name: "only old path", + memfs: testutils.MakeMemMapFs(t, map[string][]byte{ + ".config/loadimpact/k6/config.json": []byte(`{"iterations":1028}`), + }), + expConf: Config{Options: lib.Options{Iterations: null.IntFrom(1028)}}, + expLegacyWarn: true, + }, + { + name: "only new path", + memfs: testutils.MakeMemMapFs(t, map[string][]byte{ + ".config/k6/config.json": []byte(`{"iterations":1028}`), + }), + expConf: Config{Options: lib.Options{Iterations: null.IntFrom(1028)}}, + expLegacyWarn: false, + }, + { + name: "no config files", // the startup condition + memfs: fsext.NewMemMapFs(), + expConf: Config{}, + expLegacyWarn: false, + }, + } + for _, tc := range testcases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + l, hook := testutils.NewLoggerWithHook(t) + logger := l.(*logrus.Logger) //nolint:forbidigo // no alternative, required + + defaultFlags := state.GetDefaultFlags(".config") + gs := &state.GlobalState{ + FS: tc.memfs, + Flags: defaultFlags, + DefaultFlags: defaultFlags, + UserOSConfigDir: ".config", + Logger: logger, + } + + c, err := loadConfigFile(gs) + require.NoError(t, err) + + assert.Equal(t, tc.expConf, c) + + // it reads only the new one and it doesn't care about the old path + assert.Equal(t, tc.expLegacyWarn, testutils.LogContains(hook.Drain(), logrus.WarnLevel, "old default path")) + }) + } +} From 9b2125c2a3ca464ed0f4d36250c82027815e86a7 Mon Sep 17 00:00:00 2001 From: codebien <2103732+codebien@users.noreply.github.com> Date: Thu, 6 Feb 2025 17:32:17 +0100 Subject: [PATCH 13/13] Docs nitpicks --- internal/cmd/config.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/internal/cmd/config.go b/internal/cmd/config.go index 8981d7d1ebe..1d1126af54e 100644 --- a/internal/cmd/config.go +++ b/internal/cmd/config.go @@ -91,7 +91,7 @@ func (c Config) Apply(cfg Config) Config { return c } -// Returns a Config but only parses the Options inside. +// getPartialConfig returns a Config but only parses the Options inside. func getPartialConfig(flags *pflag.FlagSet) (Config, error) { opts, err := getOptions(flags) if err != nil { @@ -124,7 +124,7 @@ func getConfig(flags *pflag.FlagSet) (Config, error) { }, nil } -// Reads the configuration file from the supplied filesystem and returns it or +// readDiskConfig reads the configuration file from the supplied filesystem and returns it or // an error. The only situation in which an error won't be returned is if the // user didn't explicitly specify a config file path and the default config file // doesn't exist. @@ -157,6 +157,7 @@ func legacyConfigFilePath(gs *state.GlobalState) string { return filepath.Join(gs.UserOSConfigDir, "loadimpact", "k6", "config.json") } +// readLegacyDiskConfig reads the configuration file stored on the old default path. func readLegacyDiskConfig(gs *state.GlobalState) (Config, error) { // Check if the legacy config exists in the supplied filesystem legacyPath := legacyConfigFilePath(gs) @@ -175,8 +176,8 @@ func readLegacyDiskConfig(gs *state.GlobalState) (Config, error) { return conf, nil } -// Serializes the configuration to a JSON file and writes it in the supplied -// location on the supplied filesystem +// writeDiskConfig serializes the configuration to a JSON file and writes it in the supplied +// location on the supplied filesystem. func writeDiskConfig(gs *state.GlobalState, conf Config) error { data, err := json.MarshalIndent(conf, "", " ") if err != nil { @@ -190,7 +191,7 @@ func writeDiskConfig(gs *state.GlobalState, conf Config) error { return fsext.WriteFile(gs.FS, gs.Flags.ConfigFilePath, data, 0o644) } -// Reads configuration variables from the environment. +// readEnvConfig reads configuration variables from the environment. func readEnvConfig(envMap map[string]string) (Config, error) { // TODO: replace envconfig and refactor the whole configuration from the ground up :/ conf := Config{} @@ -235,7 +236,7 @@ func loadConfigFile(gs *state.GlobalState) (Config, error) { return readDiskConfig(gs) } -// Assemble the final consolidated configuration from all of the different sources: +// getConsolidatedConfig assemble the final consolidated configuration from all of the different sources: // - start with the CLI-provided options to get shadowed (non-Valid) defaults in there // - add the global file config options // - add the Runner-provided options (they may come from Bundle too if applicable)