feat(config): apply skip rules during the config.toml migration

pkg/config/migrate.go

@@ -2,7 +2,9 @@ package config
 
 import (
 	"os"
+	"sort"
 
+	log "github.com/sirupsen/logrus"
 	"github.com/spf13/viper"
 
 	"github.com/algolia/cli/pkg/keychain"
@@ -35,7 +37,7 @@ func (c *Config) ShouldMigrate() bool {
 func (c *Config) Migrate() error {
 	state := &State{Applications: map[string]ApplicationState{}}
 
-	for _, profile := range c.ConfiguredProfiles() {
+	for _, profile := range c.migratableProfiles() {
 		secrets := keychain.AppSecrets{
 			APIKey:        profile.APIKey,
 			CrawlerAPIKey: viper.GetString(profile.GetFieldName("crawler_api_key")),
@@ -54,3 +56,59 @@ func (c *Config) Migrate() error {
 
 	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,6 +5,7 @@ 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"
@@ -178,3 +179,101 @@ api_key = "key-1"
 	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)
+}