diff --git a/pkg/config/migrate.go b/pkg/config/migrate.go index cfee2cc3..54949f25 100644 --- a/pkg/config/migrate.go +++ b/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 +} diff --git a/pkg/config/migrate_test.go b/pkg/config/migrate_test.go index bcb08c5c..1786dc19 100644 --- a/pkg/config/migrate_test.go +++ b/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) +}