feat(config): migrate config.toml profiles to the keychain and state.toml

pkg/config/migrate.go

@@ -1,6 +1,14 @@
 package config
 
-import "os"
+import (
+	"os"
+	"sort"
+
+	log "github.com/sirupsen/logrus"
+	"github.com/spf13/viper"
+
+	"github.com/algolia/cli/pkg/keychain"
+)
 
 // ShouldMigrate reports whether the one-time config.toml → state.toml +
 // keychain migration still has to run: a legacy config.toml exists and
@@ -20,9 +28,87 @@ func (c *Config) ShouldMigrate() bool {
 // Migrate moves the legacy config.toml profiles into the new model (state.toml
 // + OS keychain). config.toml itself is never modified.
 //
-// The migration body lands in follow-up PRs; until then this is a no-op that
-// deliberately does NOT write state.toml, so ShouldMigrate keeps returning
-// true and the real migration will run once shipped.
+// Secrets go to the keychain first; state.toml is only written — atomically,
+// via State.Save's temp + rename — once every profile's keys are stored. A
+// keychain failure mid-run therefore leaves state.toml absent and the whole
+// migration retries on the next command; entries already written are simply
+// rewritten then. With nothing to migrate an empty state.toml still gets
+// written, so ShouldMigrate stops firing on every command.
 func (c *Config) Migrate() error {
-	return nil
+	state := &State{Applications: map[string]ApplicationState{}}
+
+	for _, profile := range c.migratableProfiles() {
+		secrets := keychain.AppSecrets{
+			APIKey:        profile.APIKey,
+			CrawlerAPIKey: viper.GetString(profile.GetFieldName("crawler_api_key")),
+		}
+		if err := keychain.SaveAppSecrets(profile.ApplicationID, secrets); err != nil {
+			return err
+		}
+
+		// api_key_uuid is unknown for legacy keys: left empty until an API
+		// lookup can backfill it.
+		state.UpsertApplication(profile.ApplicationID, ApplicationState{Alias: profile.Name})
+		if profile.Default {
+			state.SetCurrentApplication(profile.ApplicationID)
+		}
+	}
+
+	return state.Save(c.StateFile)
+}
+
+// migratableProfiles applies the migration skip rules to the config.toml
+// profiles before any keychain write happens:
+//
+//   - admin_api_key never moves to the new model: one log line points to its
+//     replacements, whether the profile migrates or not.
+//   - A profile without application_id or with an empty api_key has nothing
+//     usable to migrate: skipped with a log line.
+//   - Profiles sharing the same application_id would overwrite each other's
+//     keychain entry: the default = true profile wins, the others are logged
+//     as conflicts and skipped.
+//
+// Profiles are processed in name order so conflict resolution and logs stay
+// deterministic (ConfiguredProfiles iterates a map).
+func (c *Config) migratableProfiles() []*Profile {
+	profiles := c.ConfiguredProfiles()
+	sort.Slice(profiles, func(i, j int) bool { return profiles[i].Name < profiles[j].Name })
+
+	selected := make([]*Profile, 0, len(profiles))
+	owner := map[string]int{} // application ID → index in selected
+
+	for _, profile := range profiles {
+		if profile.AdminAPIKey != "" {
+			log.Warnf(
+				"config migration: profile %q: admin_api_key is not migrated, use ALGOLIA_ADMIN_API_KEY or --api-key instead",
+				profile.Name,
+			)
+		}
+		if profile.ApplicationID == "" {
+			log.Warnf("config migration: skipping profile %q: no application_id", profile.Name)
+			continue
+		}
+		if profile.APIKey == "" {
+			log.Warnf("config migration: skipping profile %q: empty api_key", profile.Name)
+			continue
+		}
+		if i, ok := owner[profile.ApplicationID]; ok {
+			kept, dropped := selected[i], profile
+			if profile.Default && !kept.Default {
+				selected[i] = profile
+				kept, dropped = profile, kept
+			}
+			log.Warnf(
+				"config migration: skipping profile %q: application %q already migrated from profile %q",
+				dropped.Name,
+				dropped.ApplicationID,
+				kept.Name,
+			)
+			continue
+		}
+		owner[profile.ApplicationID] = len(selected)
+		selected = append(selected, profile)
+	}
+
+	return selected
 }

pkg/config/migrate_test.go

@@ -5,8 +5,13 @@ import (
 	"path/filepath"
 	"testing"
 
+	logtest "github.com/sirupsen/logrus/hooks/test"
+	"github.com/spf13/viper"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"github.com/zalando/go-keyring"
+
+	"github.com/algolia/cli/pkg/keychain"
 )
 
 func TestConfig_ShouldMigrate(t *testing.T) {
@@ -66,3 +71,209 @@ func TestConfig_ShouldMigrate_unresolvedPaths(t *testing.T) {
 	cfg := &Config{}
 	assert.False(t, cfg.ShouldMigrate())
 }
+
+// migrationConfig writes a config.toml with the given content, points the
+// global viper at it (ConfiguredProfiles reads through viper) and returns a
+// Config ready to migrate.
+func migrationConfig(t *testing.T, content string) *Config {
+	t.Helper()
+
+	dir := t.TempDir()
+	configFile := filepath.Join(dir, "config.toml")
+	require.NoError(t, os.WriteFile(configFile, []byte(content), 0o600))
+
+	viper.Reset()
+	viper.SetConfigType("toml")
+	viper.SetConfigFile(configFile)
+	require.NoError(t, viper.ReadInConfig())
+	t.Cleanup(viper.Reset)
+
+	return &Config{
+		File:      configFile,
+		StateFile: filepath.Join(dir, "state.toml"),
+	}
+}
+
+func TestConfig_Migrate(t *testing.T) {
+	keyring.MockInit()
+	cfg := migrationConfig(t, `[prod]
+application_id = "APP1"
+api_key = "key-1"
+crawler_api_key = "crawler-1"
+default = true
+
+[dev]
+application_id = "APP2"
+api_key = "key-2"
+`)
+
+	require.NoError(t, cfg.Migrate())
+
+	// Secrets land in the keychain, crawler key included when set.
+	prod, err := keychain.LoadAppSecrets("APP1")
+	require.NoError(t, err)
+	require.NotNil(t, prod)
+	assert.Equal(t, "key-1", prod.APIKey)
+	assert.Equal(t, "crawler-1", prod.CrawlerAPIKey)
+
+	dev, err := keychain.LoadAppSecrets("APP2")
+	require.NoError(t, err)
+	require.NotNil(t, dev)
+	assert.Equal(t, "key-2", dev.APIKey)
+	assert.Empty(t, dev.CrawlerAPIKey)
+
+	// state.toml: one entry per application, alias = profile name, current
+	// application = the default profile's one.
+	st, err := LoadState(cfg.StateFile)
+	require.NoError(t, err)
+	assert.Equal(t, "APP1", st.CurrentApplicationID)
+	assert.Equal(t, "prod", st.Applications["APP1"].Alias)
+	assert.Equal(t, "dev", st.Applications["APP2"].Alias)
+	assert.Empty(t, st.Applications["APP1"].APIKeyUUID) // unknown for legacy keys
+
+	// state.toml now exists: the trigger turns off.
+	assert.False(t, cfg.ShouldMigrate())
+}
+
+func TestConfig_Migrate_NoDefaultProfileLeavesCurrentEmpty(t *testing.T) {
+	keyring.MockInit()
+	cfg := migrationConfig(t, `[dev]
+application_id = "APP2"
+api_key = "key-2"
+`)
+
+	require.NoError(t, cfg.Migrate())
+
+	st, err := LoadState(cfg.StateFile)
+	require.NoError(t, err)
+	assert.Empty(t, st.CurrentApplicationID)
+	assert.Equal(t, "dev", st.Applications["APP2"].Alias)
+}
+
+func TestConfig_Migrate_EmptyConfigStillWritesState(t *testing.T) {
+	keyring.MockInit()
+	cfg := migrationConfig(t, "")
+
+	require.NoError(t, cfg.Migrate())
+
+	// An empty state.toml must exist, otherwise the migration would re-run
+	// (and re-log) on every command.
+	st, err := LoadState(cfg.StateFile)
+	require.NoError(t, err)
+	assert.Empty(t, st.CurrentApplicationID)
+	assert.Empty(t, st.Applications)
+	assert.False(t, cfg.ShouldMigrate())
+}
+
+func TestConfig_Migrate_KeychainFailureLeavesStateAbsent(t *testing.T) {
+	keyring.MockInitWithError(keyring.ErrUnsupportedPlatform)
+	cfg := migrationConfig(t, `[prod]
+application_id = "APP1"
+api_key = "key-1"
+`)
+
+	require.Error(t, cfg.Migrate())
+
+	// state.toml untouched: ShouldMigrate keeps firing so the migration
+	// retries on the next run.
+	assert.NoFileExists(t, cfg.StateFile)
+	assert.True(t, cfg.ShouldMigrate())
+}
+
+func TestConfig_Migrate_SkipRules(t *testing.T) {
+	keyring.MockInit()
+	hook := logtest.NewGlobal()
+	t.Cleanup(hook.Reset)
+
+	cfg := migrationConfig(t, `[nokey]
+application_id = "APP3"
+api_key = ""
+
+[noapp]
+api_key = "key-x"
+
+[adminonly]
+application_id = "APP4"
+admin_api_key = "admin-key"
+`)
+
+	require.NoError(t, cfg.Migrate())
+
+	// Nothing migrated: no keychain entries, an empty state.toml that still
+	// turns the trigger off.
+	for _, appID := range []string{"APP3", "APP4"} {
+		secrets, err := keychain.LoadAppSecrets(appID)
+		require.NoError(t, err)
+		assert.Nil(t, secrets)
+	}
+	st, err := LoadState(cfg.StateFile)
+	require.NoError(t, err)
+	assert.Empty(t, st.Applications)
+	assert.False(t, cfg.ShouldMigrate())
+
+	// Each skip got its log line, plus the admin_api_key notice.
+	logs := make([]string, 0, len(hook.AllEntries()))
+	for _, entry := range hook.AllEntries() {
+		logs = append(logs, entry.Message)
+	}
+	assert.Contains(t, logs,
+		`config migration: skipping profile "nokey": empty api_key`)
+	assert.Contains(t, logs,
+		`config migration: skipping profile "noapp": no application_id`)
+	assert.Contains(t, logs,
+		`config migration: skipping profile "adminonly": empty api_key`)
+	assert.Contains(
+		t,
+		logs,
+		`config migration: profile "adminonly": admin_api_key is not migrated, use ALGOLIA_ADMIN_API_KEY or --api-key instead`,
+	)
+}
+
+func TestConfig_Migrate_DuplicateApplicationKeepsDefault(t *testing.T) {
+	keyring.MockInit()
+	cfg := migrationConfig(t, `[backup]
+application_id = "APP1"
+api_key = "backup-key"
+
+[prod]
+application_id = "APP1"
+api_key = "prod-key"
+default = true
+`)
+
+	require.NoError(t, cfg.Migrate())
+
+	// One single entry for APP1: the default profile's alias and key.
+	st, err := LoadState(cfg.StateFile)
+	require.NoError(t, err)
+	require.Len(t, st.Applications, 1)
+	assert.Equal(t, "prod", st.Applications["APP1"].Alias)
+	assert.Equal(t, "APP1", st.CurrentApplicationID)
+
+	secrets, err := keychain.LoadAppSecrets("APP1")
+	require.NoError(t, err)
+	require.NotNil(t, secrets)
+	assert.Equal(t, "prod-key", secrets.APIKey)
+}
+
+func TestConfig_Migrate_AdminKeyAlongsideAPIKeyStillMigrates(t *testing.T) {
+	keyring.MockInit()
+	cfg := migrationConfig(t, `[prod]
+application_id = "APP1"
+api_key = "key-1"
+admin_api_key = "admin-1"
+default = true
+`)
+
+	require.NoError(t, cfg.Migrate())
+
+	// The search key migrates; the admin key has no slot in the new model.
+	secrets, err := keychain.LoadAppSecrets("APP1")
+	require.NoError(t, err)
+	require.NotNil(t, secrets)
+	assert.Equal(t, "key-1", secrets.APIKey)
+
+	st, err := LoadState(cfg.StateFile)
+	require.NoError(t, err)
+	assert.Equal(t, "prod", st.Applications["APP1"].Alias)
+}