From 660fc71ddec4901fb5e9905d88aa0baa1efd7b1e Mon Sep 17 00:00:00 2001 From: Lorris Saint-Genez Date: Tue, 9 Jun 2026 08:15:06 -0700 Subject: [PATCH 1/8] feat(config): add state.toml store (#232) Co-authored-by: Claude Opus 4.8 (1M context) --- pkg/config/state.go | 96 ++++++++++++++++++++++++++++++++++++++++ pkg/config/state_test.go | 83 ++++++++++++++++++++++++++++++++++ 2 files changed, 179 insertions(+) create mode 100644 pkg/config/state.go create mode 100644 pkg/config/state_test.go diff --git a/pkg/config/state.go b/pkg/config/state.go new file mode 100644 index 00000000..99d4809d --- /dev/null +++ b/pkg/config/state.go @@ -0,0 +1,96 @@ +package config + +import ( + "os" + "path/filepath" + + "github.com/BurntSushi/toml" + "github.com/algolia/cli/pkg/utils" +) + +// ApplicationState holds the non-secret, per-application data persisted in +// state.toml. Secrets (API keys) live in the OS keychain, not here. +type ApplicationState struct { + APIKeyUUID string `toml:"api_key_uuid"` + Alias string `toml:"alias"` +} + +// State is the in-memory representation of state.toml, the new source of truth +// for non-secret CLI configuration. +type State struct { + CurrentApplicationID string `toml:"current_application_id"` + Applications map[string]ApplicationState `toml:"applications"` +} + +// LoadState reads state.toml from path. A missing file is not an error: it +// returns an empty, ready-to-use State. +func LoadState(path string) (*State, error) { + state := &State{Applications: map[string]ApplicationState{}} + + if _, err := os.Stat(path); os.IsNotExist(err) { + return state, nil + } + + if _, err := toml.DecodeFile(path, state); err != nil { + return nil, err + } + + if state.Applications == nil { + state.Applications = map[string]ApplicationState{} + } + + return state, nil +} + +// Save writes the State to path atomically: it encodes to a temporary file in +// the same directory, then renames it over the target so a crash mid-write can +// never leave a half-written state.toml. +func (s *State) Save(path string) error { + if err := utils.MakePath(path); err != nil { + return err + } + + tmp, err := os.CreateTemp(filepath.Dir(path), ".state-*.toml") + if err != nil { + return err + } + tmpName := tmp.Name() + defer os.Remove(tmpName) // no-op once the rename below succeeds + + if err := toml.NewEncoder(tmp).Encode(s); err != nil { + tmp.Close() + return err + } + if err := tmp.Close(); err != nil { + return err + } + if err := os.Chmod(tmpName, 0o600); err != nil { + return err + } + + return os.Rename(tmpName, path) +} + +// SetCurrentApplication records appID as the currently selected application. +func (s *State) SetCurrentApplication(appID string) { + s.CurrentApplicationID = appID +} + +// UpsertApplication inserts or replaces the state for a given application ID. +func (s *State) UpsertApplication(appID string, app ApplicationState) { + if s.Applications == nil { + s.Applications = map[string]ApplicationState{} + } + s.Applications[appID] = app +} + +// ApplicationByAlias returns the application ID whose entry carries the given +// alias, and whether such an entry was found. +func (s *State) ApplicationByAlias(alias string) (string, bool) { + for appID, app := range s.Applications { + if app.Alias == alias { + return appID, true + } + } + return "", false +} diff --git a/pkg/config/state_test.go b/pkg/config/state_test.go new file mode 100644 index 00000000..6dbd529a --- /dev/null +++ b/pkg/config/state_test.go @@ -0,0 +1,83 @@ +package config + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestState_LoadMissingFileReturnsEmpty(t *testing.T) { + path := filepath.Join(t.TempDir(), "state.toml") + + state, err := LoadState(path) + require.NoError(t, err) + require.NotNil(t, state) + assert.Empty(t, state.CurrentApplicationID) + assert.Empty(t, state.Applications) +} + +func TestState_SaveAndLoadRoundTrip(t *testing.T) { + path := filepath.Join(t.TempDir(), "state.toml") + + state := &State{ + CurrentApplicationID: "APP1", + Applications: map[string]ApplicationState{ + "APP1": {APIKeyUUID: "uuid-1", Alias: "prod"}, + }, + } + require.NoError(t, state.Save(path)) + + loaded, err := LoadState(path) + require.NoError(t, err) + assert.Equal(t, "APP1", loaded.CurrentApplicationID) + assert.Equal(t, "uuid-1", loaded.Applications["APP1"].APIKeyUUID) + assert.Equal(t, "prod", loaded.Applications["APP1"].Alias) +} + +func TestState_MutatorsPersist(t *testing.T) { + path := filepath.Join(t.TempDir(), "state.toml") + + state := &State{} + state.UpsertApplication("APP1", ApplicationState{APIKeyUUID: "uuid-1", Alias: "prod"}) + state.SetCurrentApplication("APP1") + require.NoError(t, state.Save(path)) + + loaded, err := LoadState(path) + require.NoError(t, err) + assert.Equal(t, "APP1", loaded.CurrentApplicationID) + assert.Equal(t, "prod", loaded.Applications["APP1"].Alias) + + // Upsert replaces an existing entry rather than duplicating it. + loaded.UpsertApplication("APP1", ApplicationState{APIKeyUUID: "uuid-2", Alias: "staging"}) + assert.Len(t, loaded.Applications, 1) + assert.Equal(t, "uuid-2", loaded.Applications["APP1"].APIKeyUUID) + assert.Equal(t, "staging", loaded.Applications["APP1"].Alias) +} + +func TestState_ApplicationByAlias(t *testing.T) { + state := &State{ + Applications: map[string]ApplicationState{ + "APP1": {Alias: "prod"}, + "APP2": {Alias: "staging"}, + }, + } + + appID, found := state.ApplicationByAlias("staging") + assert.True(t, found) + assert.Equal(t, "APP2", appID) + + _, found = state.ApplicationByAlias("missing") + assert.False(t, found) +} + +func TestState_LoadCorruptFileReturnsError(t *testing.T) { + path := filepath.Join(t.TempDir(), "state.toml") + require.NoError(t, os.WriteFile(path, []byte(`key = "unterminated`), 0o600)) + + state, err := LoadState(path) + require.Error(t, err) + assert.Nil(t, state) +} From a9cfcea44c9a1b59d86ba7236587a2ff01445979 Mon Sep 17 00:00:00 2001 From: Lorris Saint-Genez Date: Tue, 9 Jun 2026 08:18:20 -0700 Subject: [PATCH 2/8] feat(keychain): add per-app secret store (#233) Co-authored-by: Claude Opus 4.8 (1M context) --- pkg/keychain/app_secrets.go | 69 +++++++++++++++++++++++++++ pkg/keychain/app_secrets_test.go | 81 ++++++++++++++++++++++++++++++++ 2 files changed, 150 insertions(+) create mode 100644 pkg/keychain/app_secrets.go create mode 100644 pkg/keychain/app_secrets_test.go diff --git a/pkg/keychain/app_secrets.go b/pkg/keychain/app_secrets.go new file mode 100644 index 00000000..18d81491 --- /dev/null +++ b/pkg/keychain/app_secrets.go @@ -0,0 +1,69 @@ +package keychain + +import ( + "encoding/json" + "errors" + "fmt" + + "github.com/zalando/go-keyring" +) + +const ( + // service is the OS keychain service the CLI stores secrets under. It is + // shared with the OAuth token entry (pkg/auth); per-application secrets are + // namespaced by their user key so they never collide. + service = "algolia-cli" + + // appSecretsUserPrefix namespaces per-application keychain entries. + appSecretsUserPrefix = "app:" +) + +// AppSecrets holds the secret credentials for a single application, persisted +// as a JSON blob in the OS keychain. +type AppSecrets struct { + APIKey string `json:"api_key"` + CrawlerAPIKey string `json:"crawler_api_key,omitempty"` +} + +// appSecretsUser returns the keychain user key for a given application ID. +func appSecretsUser(appID string) string { + return appSecretsUserPrefix + appID +} + +// SaveAppSecrets persists the secrets for an application to the OS keychain. +func SaveAppSecrets(appID string, secrets AppSecrets) error { + if appID == "" { + return fmt.Errorf("appID is required") + } + + data, err := json.Marshal(secrets) + if err != nil { + return err + } + + return keyring.Set(service, appSecretsUser(appID), string(data)) +} + +// LoadAppSecrets reads an application's secrets from the OS keychain. A missing +// entry is not an error: it returns (nil, nil). Real failures (keychain +// unavailable, malformed data) return an error. +func LoadAppSecrets(appID string) (*AppSecrets, error) { + if appID == "" { + return nil, fmt.Errorf("appID is required") + } + + secret, err := keyring.Get(service, appSecretsUser(appID)) + if errors.Is(err, keyring.ErrNotFound) { + return nil, nil + } + if err != nil { + return nil, err + } + + var secrets AppSecrets + if err := json.Unmarshal([]byte(secret), &secrets); err != nil { + return nil, err + } + + return &secrets, nil +} diff --git a/pkg/keychain/app_secrets_test.go b/pkg/keychain/app_secrets_test.go new file mode 100644 index 00000000..b938694e --- /dev/null +++ b/pkg/keychain/app_secrets_test.go @@ -0,0 +1,81 @@ +package keychain + +import ( + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/zalando/go-keyring" +) + +func TestAppSecrets_SaveAndLoadRoundTrip(t *testing.T) { + keyring.MockInit() + + require.NoError(t, SaveAppSecrets("APP1", AppSecrets{ + APIKey: "key-1", + CrawlerAPIKey: "crawler-1", + })) + + loaded, err := LoadAppSecrets("APP1") + require.NoError(t, err) + require.NotNil(t, loaded) + assert.Equal(t, "key-1", loaded.APIKey) + assert.Equal(t, "crawler-1", loaded.CrawlerAPIKey) +} + +func TestAppSecrets_LoadMissingReturnsNil(t *testing.T) { + keyring.MockInit() + + loaded, err := LoadAppSecrets("UNKNOWN") + require.NoError(t, err) + assert.Nil(t, loaded) +} + +func TestAppSecrets_PerAppIsolationAndOptionalCrawlerKey(t *testing.T) { + keyring.MockInit() + + require.NoError(t, SaveAppSecrets("APP1", AppSecrets{APIKey: "key-1"})) + require.NoError( + t, + SaveAppSecrets("APP2", AppSecrets{APIKey: "key-2", CrawlerAPIKey: "crawler-2"}), + ) + + app1, err := LoadAppSecrets("APP1") + require.NoError(t, err) + require.NotNil(t, app1) + assert.Equal(t, "key-1", app1.APIKey) + assert.Empty(t, app1.CrawlerAPIKey) // never set → stays empty + + app2, err := LoadAppSecrets("APP2") + require.NoError(t, err) + require.NotNil(t, app2) + assert.Equal(t, "key-2", app2.APIKey) + assert.Equal(t, "crawler-2", app2.CrawlerAPIKey) +} + +func TestAppSecrets_EmptyAppIDIsRejected(t *testing.T) { + keyring.MockInit() + + require.Error(t, SaveAppSecrets("", AppSecrets{APIKey: "key-1"})) + + _, err := LoadAppSecrets("") + require.Error(t, err) +} + +func TestAppSecrets_LoadKeychainErrorPropagates(t *testing.T) { + keyring.MockInitWithError(errors.New("keychain unavailable")) + + loaded, err := LoadAppSecrets("APP1") + require.Error(t, err) + assert.Nil(t, loaded) +} + +func TestAppSecrets_LoadMalformedJSONReturnsError(t *testing.T) { + keyring.MockInit() + require.NoError(t, keyring.Set(service, appSecretsUser("BAD"), "not-json")) + + loaded, err := LoadAppSecrets("BAD") + require.Error(t, err) + assert.Nil(t, loaded) +} From 153add557ba6957e96c60296ce2c11b823c71c74 Mon Sep 17 00:00:00 2001 From: Lorris Saint-Genez Date: Tue, 9 Jun 2026 09:37:03 -0700 Subject: [PATCH 3/8] feat(dashboard): return API key UUID from CreateAPIKey (#234) Co-authored-by: Claude Opus 4.8 (1M context) --- api/dashboard/client.go | 18 ++++++------- api/dashboard/client_test.go | 45 ++++++++++++++++++++++++++++++++ api/dashboard/types.go | 16 +++++++++--- pkg/cmd/shared/apputil/create.go | 5 ++-- 4 files changed, 69 insertions(+), 15 deletions(-) diff --git a/api/dashboard/client.go b/api/dashboard/client.go index 46d77a5e..98b29512 100644 --- a/api/dashboard/client.go +++ b/api/dashboard/client.go @@ -544,34 +544,34 @@ func (c *Client) CreateAPIKey( accessToken, appID string, acl []string, description string, -) (string, error) { +) (CreatedAPIKey, error) { payload := CreateAPIKeyRequest{ACL: acl, Description: description} body, err := json.Marshal(payload) if err != nil { - return "", err + return CreatedAPIKey{}, err } endpoint := fmt.Sprintf("%s/1/applications/%s/api-keys", c.APIURL, url.PathEscape(appID)) req, err := http.NewRequest(http.MethodPost, endpoint, bytes.NewReader(body)) if err != nil { - return "", err + return CreatedAPIKey{}, err } c.setAPIHeaders(req, accessToken) req.Header.Set("Content-Type", "application/json") resp, err := c.client.Do(req) if err != nil { - return "", fmt.Errorf("create API key request failed: %w", err) + return CreatedAPIKey{}, fmt.Errorf("create API key request failed: %w", err) } defer resp.Body.Close() respBody, err := io.ReadAll(resp.Body) if err != nil { - return "", fmt.Errorf("failed to read API key response: %w", err) + return CreatedAPIKey{}, fmt.Errorf("failed to read API key response: %w", err) } if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusCreated { - return "", fmt.Errorf( + return CreatedAPIKey{}, fmt.Errorf( "create API key failed with status %d: %s", resp.StatusCode, string(respBody), @@ -580,7 +580,7 @@ func (c *Client) CreateAPIKey( var keyResp CreateAPIKeyResponse if err := json.Unmarshal(respBody, &keyResp); err != nil { - return "", fmt.Errorf( + return CreatedAPIKey{}, fmt.Errorf( "failed to parse API key response: %w (body: %s)", err, string(respBody), @@ -589,13 +589,13 @@ func (c *Client) CreateAPIKey( key := keyResp.Data.Attributes.Value if key == "" { - return "", fmt.Errorf( + return CreatedAPIKey{}, fmt.Errorf( "API key creation succeeded but no key was returned in the response: %s", string(respBody), ) } - return key, nil + return CreatedAPIKey{Value: key, UUID: keyResp.Data.ID}, nil } // GetCrawlerUser gets the crawler API user data for the current authenticated user diff --git a/api/dashboard/client_test.go b/api/dashboard/client_test.go index bbd41938..3ea40d68 100644 --- a/api/dashboard/client_test.go +++ b/api/dashboard/client_test.go @@ -293,6 +293,51 @@ func TestCreateApplication_Success(t *testing.T) { assert.Equal(t, "My App", app.Name) } +func TestCreateAPIKey_ReturnsValueAndUUID(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("/1/applications/APP1/api-keys", func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, http.MethodPost, r.Method) + assert.Equal(t, "Bearer test-token", r.Header.Get("Authorization")) + + w.WriteHeader(http.StatusCreated) + require.NoError(t, json.NewEncoder(w).Encode(CreateAPIKeyResponse{ + Data: APIKeyResource{ + ID: "key-uuid-123", + Type: "api_key", + Attributes: APIKeyAttributes{Value: "secret-key"}, + }, + })) + }) + + ts, client := newTestClient(mux) + defer ts.Close() + + created, err := client.CreateAPIKey("test-token", "APP1", WriteACL, "Algolia CLI") + require.NoError(t, err) + assert.Equal(t, "secret-key", created.Value) + assert.Equal(t, "key-uuid-123", created.UUID) +} + +func TestCreateAPIKey_EmptyValueReturnsError(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("/1/applications/APP1/api-keys", func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusCreated) + require.NoError(t, json.NewEncoder(w).Encode(CreateAPIKeyResponse{ + Data: APIKeyResource{ + ID: "key-uuid-123", + Attributes: APIKeyAttributes{Value: ""}, + }, + })) + }) + + ts, client := newTestClient(mux) + defer ts.Close() + + _, err := client.CreateAPIKey("test-token", "APP1", WriteACL, "Algolia CLI") + require.Error(t, err) + assert.Contains(t, err.Error(), "no key was returned") +} + func TestUpdateApplication_Success(t *testing.T) { mux := http.NewServeMux() mux.HandleFunc("/1/applications/APP1", func(w http.ResponseWriter, r *http.Request) { diff --git a/api/dashboard/types.go b/api/dashboard/types.go index da309f4d..ecf45fdd 100644 --- a/api/dashboard/types.go +++ b/api/dashboard/types.go @@ -47,10 +47,11 @@ type ApplicationPlan struct { // Application is a flattened view of an Algolia application for CLI consumption. type Application struct { - ID string `json:"id"` - Name string `json:"name"` - APIKey string `json:"api_key,omitempty"` - PlanLabel string `json:"plan_label,omitempty"` // current plan label, e.g. "Grow Plus" + ID string `json:"id"` + Name string `json:"name"` + APIKey string `json:"api_key,omitempty"` + APIKeyUUID string `json:"api_key_uuid,omitempty"` + PlanLabel string `json:"plan_label,omitempty"` // current plan label, e.g. "Grow Plus" } // PaginationMeta contains page-based pagination metadata. @@ -145,6 +146,13 @@ type CreateAPIKeyResponse struct { Data APIKeyResource `json:"data"` } +// CreatedAPIKey is the result of creating an API key: its secret value and its +// UUID, used to reference the key when persisting or managing it later. +type CreatedAPIKey struct { + Value string + UUID string +} + // DashboardCrawlerUserData contains the user information from the crawler API type DashboardCrawlerUserData struct { ID string `json:"id"` diff --git a/pkg/cmd/shared/apputil/create.go b/pkg/cmd/shared/apputil/create.go index a5d217ee..82ce5edc 100644 --- a/pkg/cmd/shared/apputil/create.go +++ b/pkg/cmd/shared/apputil/create.go @@ -129,13 +129,14 @@ func EnsureAPIKey( ) error { cs := io.ColorScheme() io.StartProgressIndicatorWithLabel("Generating API key") - apiKey, err := client.CreateAPIKey(accessToken, app.ID, dashboard.WriteACL, "Algolia CLI") + created, err := client.CreateAPIKey(accessToken, app.ID, dashboard.WriteACL, "Algolia CLI") io.StopProgressIndicator() if err != nil { return fmt.Errorf("failed to generate API key: %w", err) } - app.APIKey = apiKey + app.APIKey = created.Value + app.APIKeyUUID = created.UUID fmt.Fprintf(io.Out, "%s API key generated for application %s\n", cs.SuccessIcon(), cs.Bold(app.ID)) return nil From 1da916fc960a9a1072e490c03bb8c123d21604d3 Mon Sep 17 00:00:00 2001 From: Lorris Saint-Genez Date: Wed, 10 Jun 2026 14:11:12 -0700 Subject: [PATCH 4/8] feat(config): resolve credentials from state.toml + keychain (#235) Co-authored-by: Claude Opus 4.8 (1M context) --- pkg/auth/auth_check.go | 2 +- pkg/cmd/root/root.go | 6 +- pkg/config/config.go | 98 +++++++++++++- pkg/config/profile.go | 41 ++++++ pkg/config/resolution_test.go | 245 ++++++++++++++++++++++++++++++++++ 5 files changed, 386 insertions(+), 6 deletions(-) create mode 100644 pkg/config/resolution_test.go diff --git a/pkg/auth/auth_check.go b/pkg/auth/auth_check.go index 5186eec9..dfe2ae4f 100644 --- a/pkg/auth/auth_check.go +++ b/pkg/auth/auth_check.go @@ -49,7 +49,7 @@ func DisableAuthCheck(cmd *cobra.Command) { cmd.Annotations["skipAuthCheck"] = "true" } -func CheckAuth(cfg config.Config) error { +func CheckAuth(cfg *config.Config) error { if cfg.Profile().Name == "" { cfg.Profile().LoadDefault() } diff --git a/pkg/cmd/root/root.go b/pkg/cmd/root/root.go index 399535e1..9fb6494f 100644 --- a/pkg/cmd/root/root.go +++ b/pkg/cmd/root/root.go @@ -134,7 +134,7 @@ func Execute() exitCode { // Set up the update notifier. updateMessageChan := make(chan *update.ReleaseInfo) go func() { - rel, err := checkForUpdate(cfg, version.Version) + rel, err := checkForUpdate(&cfg, version.Version) if err != nil && hasDebug { fmt.Fprintf(stderr, "Error checking for update: %s\n", err) } @@ -148,7 +148,7 @@ func Execute() exitCode { authError := errors.New("authError") rootCmd.PersistentPreRunE = func(cmd *cobra.Command, args []string) error { if auth.IsAuthCheckEnabled(cmd) { - if err := auth.CheckAuth(cfg); err != nil { + if err := auth.CheckAuth(&cfg); err != nil { fmt.Fprintf(stderr, "Authentication error: %s\n", err) fmt.Fprintln( stderr, @@ -305,7 +305,7 @@ func shouldCheckForUpdate() bool { return !utils.IsCI() && utils.IsTerminal(os.Stdout) && utils.IsTerminal(os.Stderr) } -func checkForUpdate(cfg config.Config, currentVersion string) (*update.ReleaseInfo, error) { +func checkForUpdate(cfg *config.Config, currentVersion string) (*update.ReleaseInfo, error) { if !shouldCheckForUpdate() { return nil, nil } diff --git a/pkg/config/config.go b/pkg/config/config.go index d25be6d5..f4e69c38 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -5,8 +5,10 @@ import ( "fmt" "os" "path/filepath" + "sync" "github.com/BurntSushi/toml" + "github.com/algolia/cli/pkg/keychain" "github.com/algolia/cli/pkg/utils" "github.com/mitchellh/go-homedir" log "github.com/sirupsen/logrus" @@ -32,13 +34,29 @@ type IConfig interface { Default() *Profile } -// Config handles all overall configuration for the CLI +// Config handles all overall configuration for the CLI. +// +// It must not be copied after InitConfig: it holds sync primitives (govet +// copylocks) and CurrentProfile holds a back-pointer to it. Pass it by pointer. type Config struct { ApplicationName string CurrentProfile Profile - File string + File string + StateFile string + + // state is the parsed state.toml, loaded once per command via loadState. + stateOnce sync.Once + state *State + + // activeApp is the resolved current application ID, computed once. + activeAppOnce sync.Once + activeApp string + + // secretsCache memoizes per-application keychain lookups (guarded by secretsMu). + secretsMu sync.Mutex + secretsCache map[string]*keychain.AppSecrets } // InitConfig reads in profiles file and ENV variables if set. @@ -49,6 +67,7 @@ func (c *Config) InitConfig() { configFolder := c.GetConfigFolder(os.Getenv("XDG_CONFIG_HOME")) configFile := filepath.Join(configFolder, "config.toml") c.File = configFile + c.StateFile = filepath.Join(configFolder, "state.toml") viper.SetConfigType("toml") viper.SetConfigFile(configFile) viper.SetConfigPermissions(os.FileMode(0o600)) @@ -61,6 +80,8 @@ func (c *Config) InitConfig() { } } + c.CurrentProfile.config = c + _ = viper.ReadInConfig() } @@ -82,6 +103,79 @@ func (c *Config) GetConfigFolder(xdgPath string) string { return filepath.Join(configPath, "algolia") } +// loadState reads state.toml once per command and caches it. A missing or +// corrupt file degrades to an empty State so resolution can fall back to +// config.toml rather than crash. +func (c *Config) loadState() *State { + c.stateOnce.Do(func() { + st, err := LoadState(c.StateFile) + if err != nil { + log.Warnf("ignoring unreadable state file %q: %s", c.StateFile, err) + st = &State{Applications: map[string]ApplicationState{}} + } + c.state = st + }) + return c.state +} + +// activeApplicationID resolves (once per command) which application the new +// model should read against. Returns "" when no new-model app applies, so the +// caller falls back to config.toml. +func (c *Config) activeApplicationID() string { + c.activeAppOnce.Do(func() { + c.activeApp = c.resolveActiveApplicationID() + }) + return c.activeApp +} + +func (c *Config) resolveActiveApplicationID() string { + if v := os.Getenv("ALGOLIA_APPLICATION_ID"); v != "" { + return v + } + + p := &c.CurrentProfile + if p.ApplicationID != "" { // --application-id flag + return p.ApplicationID + } + + st := c.loadState() + // Only a Name set explicitly (--profile flag) counts here: a name filled by + // LoadDefault must not shadow the state.toml current application. + if p.Name != "" && !p.nameFromDefault { + if appID, ok := st.ApplicationByAlias(p.Name); ok { + return appID + } + return "" // unknown alias → let the legacy config.toml profile-by-name handle it + } + + return st.CurrentApplicationID +} + +// appSecretsFor returns the cached keychain secrets for an application, loading +// them once. A missing entry or a keychain failure yields nil (also cached, so +// a single command never hits the keychain twice for the same app). The mutex +// keeps the cache safe if a getter is ever called concurrently; resolution runs +// on the main goroutine today. +func (c *Config) appSecretsFor(appID string) *keychain.AppSecrets { + c.secretsMu.Lock() + defer c.secretsMu.Unlock() + + if c.secretsCache == nil { + c.secretsCache = map[string]*keychain.AppSecrets{} + } + if cached, ok := c.secretsCache[appID]; ok { + return cached + } + + secrets, err := keychain.LoadAppSecrets(appID) + if err != nil { + log.Warnf("ignoring keychain error for application %q: %s", appID, err) + secrets = nil + } + c.secretsCache[appID] = secrets + return secrets +} + // ConfiguredProfiles return the profiles in the configuration file func (c *Config) ConfiguredProfiles() []*Profile { configs := viper.AllSettings() diff --git a/pkg/config/profile.go b/pkg/config/profile.go index ffa94df8..7cd1c3bc 100644 --- a/pkg/config/profile.go +++ b/pkg/config/profile.go @@ -22,6 +22,16 @@ type Profile struct { SearchHosts []string `mapstructure:"search_hosts"` Default bool `mapstructure:"default"` + + // config back-references the owning Config for new-model (state.toml + + // keychain) resolution. nil for standalone profiles (e.g. those returned by + // ConfiguredProfiles), which then resolve from config.toml only. + config *Config + + // nameFromDefault records that Name was filled by LoadDefault rather than + // by an explicit --profile flag, so the new-model resolver doesn't let the + // legacy default profile shadow state.toml's current application. + nameFromDefault bool } func (p *Profile) GetFieldName(field string) string { @@ -33,6 +43,7 @@ func (p *Profile) LoadDefault() { for appName := range configs { if viper.GetBool(appName + ".default") { p.Name = appName + p.nameFromDefault = true } } } @@ -46,6 +57,13 @@ func (p *Profile) GetApplicationID() (string, error) { return p.ApplicationID, nil } + // New model: state.toml current/selected application. + if p.config != nil { + if appID := p.config.activeApplicationID(); appID != "" { + return appID, nil + } + } + if p.Name == "" { p.LoadDefault() } @@ -69,6 +87,17 @@ func (p *Profile) GetAPIKey() (string, error) { return p.APIKey, nil } + // New model: once an application is resolved, its key comes only from that + // application's keychain entry — never a different profile's config.toml key. + if p.config != nil { + if appID := p.config.activeApplicationID(); appID != "" { + if secrets := p.config.appSecretsFor(appID); secrets != nil && secrets.APIKey != "" { + return secrets.APIKey, nil + } + return "", ErrAPIKeyNotConfigured + } + } + if p.Name == "" { p.LoadDefault() } @@ -161,6 +190,18 @@ func (p *Profile) GetCrawlerAPIKey() (string, error) { return os.Getenv("ALGOLIA_CRAWLER_API_KEY"), nil } + // New model: once an application is resolved, its crawler key comes only from + // that application's keychain entry — never a different profile's. + if p.config != nil { + if appID := p.config.activeApplicationID(); appID != "" { + if secrets := p.config.appSecretsFor(appID); secrets != nil && + secrets.CrawlerAPIKey != "" { + return secrets.CrawlerAPIKey, nil + } + return "", ErrCrawlerAPIKeyNotConfigured + } + } + if p.Name == "" { p.LoadDefault() } diff --git a/pkg/config/resolution_test.go b/pkg/config/resolution_test.go new file mode 100644 index 00000000..00986fd8 --- /dev/null +++ b/pkg/config/resolution_test.go @@ -0,0 +1,245 @@ +package config + +import ( + "os" + "path/filepath" + "testing" + + "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_LoadStateCachesAndToleratesMissing(t *testing.T) { + cfg := &Config{StateFile: filepath.Join(t.TempDir(), "state.toml")} + + st := cfg.loadState() + require.NotNil(t, st) + assert.Empty(t, st.CurrentApplicationID) + + // Second call returns the same cached pointer (loaded once). + assert.Same(t, st, cfg.loadState()) +} + +func TestConfig_LoadStateReadsFile(t *testing.T) { + path := filepath.Join(t.TempDir(), "state.toml") + require.NoError(t, os.WriteFile( + path, + []byte( + "current_application_id = \"APP1\"\n\n[applications.APP1]\nalias = \"prod\"\n", + ), + 0o600, + )) + + cfg := &Config{StateFile: path} + st := cfg.loadState() + assert.Equal(t, "APP1", st.CurrentApplicationID) + assert.Equal(t, "prod", st.Applications["APP1"].Alias) +} + +func TestConfig_ActiveApplicationID(t *testing.T) { + path := filepath.Join(t.TempDir(), "state.toml") + require.NoError(t, os.WriteFile( + path, + []byte( + "current_application_id = \"CURRENT\"\n\n[applications.ALIASED]\nalias = \"prod\"\n", + ), + 0o600, + )) + + t.Run("env wins", func(t *testing.T) { + t.Setenv("ALGOLIA_APPLICATION_ID", "ENV_APP") + cfg := &Config{StateFile: path} + assert.Equal(t, "ENV_APP", cfg.activeApplicationID()) + }) + + t.Run("profile alias resolves", func(t *testing.T) { + cfg := &Config{StateFile: path} + cfg.CurrentProfile.Name = "prod" + assert.Equal(t, "ALIASED", cfg.activeApplicationID()) + }) + + t.Run("falls back to current_application_id", func(t *testing.T) { + cfg := &Config{StateFile: path} + assert.Equal(t, "CURRENT", cfg.activeApplicationID()) + }) + + t.Run("unknown profile alias defers to config.toml, not current", func(t *testing.T) { + cfg := &Config{StateFile: path} + cfg.CurrentProfile.Name = "nope" + assert.Empty(t, cfg.activeApplicationID()) // "" → legacy profile-by-name, not CURRENT + }) +} + +func TestConfig_AppSecretsForCaches(t *testing.T) { + keyring.MockInit() + require.NoError(t, keychain.SaveAppSecrets("APP1", keychain.AppSecrets{APIKey: "key-1"})) + + cfg := &Config{} + got := cfg.appSecretsFor("APP1") + require.NotNil(t, got) + assert.Equal(t, "key-1", got.APIKey) + + // Missing app → nil, and a keychain error must not panic. + assert.Nil(t, cfg.appSecretsFor("MISSING")) + + // The nil result is cached: a later keychain write isn't picked up mid-command. + require.NoError(t, keychain.SaveAppSecrets("MISSING", keychain.AppSecrets{APIKey: "late"})) + assert.Nil(t, cfg.appSecretsFor("MISSING")) +} + +func TestProfile_GetApplicationID_NewModel(t *testing.T) { + path := filepath.Join(t.TempDir(), "state.toml") + require.NoError(t, os.WriteFile(path, []byte("current_application_id = \"APP1\"\n"), 0o600)) + + cfg := &Config{StateFile: path} + cfg.CurrentProfile.config = cfg + + appID, err := cfg.Profile().GetApplicationID() + require.NoError(t, err) + assert.Equal(t, "APP1", appID) +} + +func TestProfile_GetAPIKey_FromKeychain(t *testing.T) { + keyring.MockInit() + path := filepath.Join(t.TempDir(), "state.toml") + require.NoError(t, os.WriteFile(path, []byte("current_application_id = \"APP1\"\n"), 0o600)) + require.NoError(t, keychain.SaveAppSecrets("APP1", keychain.AppSecrets{APIKey: "secret-key"})) + + cfg := &Config{StateFile: path} + cfg.CurrentProfile.config = cfg + + key, err := cfg.Profile().GetAPIKey() + require.NoError(t, err) + assert.Equal(t, "secret-key", key) +} + +func TestProfile_GetCrawlerAPIKey_FromKeychain(t *testing.T) { + keyring.MockInit() + path := filepath.Join(t.TempDir(), "state.toml") + require.NoError(t, os.WriteFile(path, []byte("current_application_id = \"APP1\"\n"), 0o600)) + require.NoError(t, keychain.SaveAppSecrets("APP1", + keychain.AppSecrets{APIKey: "k", CrawlerAPIKey: "crawler-key"})) + + cfg := &Config{StateFile: path} + cfg.CurrentProfile.config = cfg + + key, err := cfg.Profile().GetCrawlerAPIKey() + require.NoError(t, err) + assert.Equal(t, "crawler-key", key) +} + +func TestProfile_EnvWinsOverKeychain(t *testing.T) { + keyring.MockInit() + path := filepath.Join(t.TempDir(), "state.toml") + require.NoError(t, os.WriteFile(path, []byte("current_application_id = \"APP1\"\n"), 0o600)) + require.NoError(t, keychain.SaveAppSecrets("APP1", keychain.AppSecrets{APIKey: "keychain-key"})) + t.Setenv("ALGOLIA_API_KEY", "env-key") + + cfg := &Config{StateFile: path} + cfg.CurrentProfile.config = cfg + + key, err := cfg.Profile().GetAPIKey() + require.NoError(t, err) + assert.Equal(t, "env-key", key) // env beats the keychain +} + +func TestProfile_FallsBackToConfigToml(t *testing.T) { + // No state.toml entry and no keychain → resolve from the legacy config.toml. + configFile := filepath.Join(t.TempDir(), "config.toml") + require.NoError(t, os.WriteFile( + configFile, + []byte( + "[legacy]\napplication_id = \"LEGACY_APP\"\napi_key = \"legacy-key\"\ndefault = true\n", + ), + 0o600, + )) + viper.Reset() + viper.SetConfigType("toml") + viper.SetConfigFile(configFile) + require.NoError(t, viper.ReadInConfig()) + t.Cleanup(viper.Reset) + + cfg := &Config{StateFile: filepath.Join(t.TempDir(), "absent.toml")} + cfg.CurrentProfile.config = cfg + + appID, err := cfg.Profile().GetApplicationID() + require.NoError(t, err) + assert.Equal(t, "LEGACY_APP", appID) + + key, err := cfg.Profile().GetAPIKey() + require.NoError(t, err) + assert.Equal(t, "legacy-key", key) +} + +func TestProfile_DefaultedNameDoesNotMaskCurrentApplication(t *testing.T) { + // CheckAuth calls LoadDefault() before any getter runs. The defaulted + // legacy profile name must NOT be mistaken for an explicit --profile flag, + // otherwise the legacy default application silently wins over state.toml's + // current_application_id. + keyring.MockInit() + statePath := filepath.Join(t.TempDir(), "state.toml") + require.NoError( + t, + os.WriteFile(statePath, []byte("current_application_id = \"APP1\"\n"), 0o600), + ) + require.NoError(t, keychain.SaveAppSecrets("APP1", keychain.AppSecrets{APIKey: "app1-key"})) + + configFile := filepath.Join(t.TempDir(), "config.toml") + require.NoError(t, os.WriteFile( + configFile, + []byte("[legacy]\napplication_id = \"LEGACY\"\napi_key = \"legacy-key\"\ndefault = true\n"), + 0o600, + )) + viper.Reset() + viper.SetConfigType("toml") + viper.SetConfigFile(configFile) + require.NoError(t, viper.ReadInConfig()) + t.Cleanup(viper.Reset) + + cfg := &Config{StateFile: statePath} + cfg.CurrentProfile.config = cfg + + cfg.CurrentProfile.LoadDefault() // what CheckAuth does when no --profile is given + require.Equal(t, "legacy", cfg.CurrentProfile.Name) + + appID, err := cfg.Profile().GetApplicationID() + require.NoError(t, err) + assert.Equal(t, "APP1", appID) + + key, err := cfg.Profile().GetAPIKey() + require.NoError(t, err) + assert.Equal(t, "app1-key", key) +} + +func TestProfile_GetAPIKey_ActiveAppWithoutKeyErrors(t *testing.T) { + keyring.MockInit() + statePath := filepath.Join(t.TempDir(), "state.toml") + require.NoError( + t, + os.WriteFile(statePath, []byte("current_application_id = \"APP1\"\n"), 0o600), + ) + + // A legacy default profile whose key must NOT leak for the resolved APP1. + configFile := filepath.Join(t.TempDir(), "config.toml") + require.NoError(t, os.WriteFile( + configFile, + []byte("[legacy]\napplication_id = \"LEGACY\"\napi_key = \"legacy-key\"\ndefault = true\n"), + 0o600, + )) + viper.Reset() + viper.SetConfigType("toml") + viper.SetConfigFile(configFile) + require.NoError(t, viper.ReadInConfig()) + t.Cleanup(viper.Reset) + + cfg := &Config{StateFile: statePath} + cfg.CurrentProfile.config = cfg + + // APP1 resolved from state but no keychain key → error, never "legacy-key". + _, err := cfg.Profile().GetAPIKey() + require.Error(t, err) +} From c8eaec7bd1e4c0934bc4bb39bdfa80337cc1a39a Mon Sep 17 00:00:00 2001 From: Lorris Saint-Genez Date: Thu, 11 Jun 2026 12:41:08 -0700 Subject: [PATCH 5/8] feat(config): write credentials to state.toml and the OS keychain (#236) --- pkg/cmd/application/selectapp/select.go | 42 +------- pkg/cmd/auth/crawler/crawler.go | 25 +++-- pkg/cmd/auth/crawler/crawler_test.go | 44 +++------ pkg/cmd/auth/login/login.go | 14 +-- pkg/cmd/factory/default.go | 31 +++++- pkg/cmd/factory/default_test.go | 70 +++++++++++++ pkg/cmd/shared/apputil/create.go | 48 +++++---- pkg/cmd/shared/apputil/create_test.go | 83 ++++++++++++++++ pkg/config/config.go | 6 ++ pkg/config/write.go | 85 ++++++++++++++++ pkg/config/write_test.go | 125 ++++++++++++++++++++++++ test/config.go | 52 ++++++++++ 12 files changed, 511 insertions(+), 114 deletions(-) create mode 100644 pkg/cmd/factory/default_test.go create mode 100644 pkg/cmd/shared/apputil/create_test.go create mode 100644 pkg/config/write.go create mode 100644 pkg/config/write_test.go diff --git a/pkg/cmd/application/selectapp/select.go b/pkg/cmd/application/selectapp/select.go index ad69d089..55fcd3d5 100644 --- a/pkg/cmd/application/selectapp/select.go +++ b/pkg/cmd/application/selectapp/select.go @@ -122,44 +122,12 @@ func runSelectCmd(opts *SelectOptions) (*dashboard.Application, error) { return nil, err } - // If a profile already exists for this app, switch the default - // and ensure it has an API key. - if exists, profileName := opts.Config.ApplicationIDExists(chosen.ID); exists { - // Read the profile BEFORE SetDefaultProfile, because viper.Set() calls - // inside SetDefaultProfile pollute the override map and cause - // UnmarshalKey to return empty fields (known viper issue). - var existingProfile *config.Profile - for _, p := range opts.Config.ConfiguredProfiles() { - if p.Name == profileName { - existingProfile = p - break - } - } - - if err := opts.Config.SetDefaultProfile(profileName); err != nil { - return nil, fmt.Errorf("failed to set default profile: %w", err) - } - fmt.Fprintf(opts.IO.Out, "%s Switched to profile %q (application %s).\n", - cs.SuccessIcon(), profileName, cs.Bold(chosen.ID)) - - if existingProfile != nil && existingProfile.APIKey == "" { - app := &dashboard.Application{ID: chosen.ID, Name: chosen.Name} - if err := apputil.EnsureAPIKey(opts.IO, client, accessToken, app); err != nil { - return nil, err - } - existingProfile.ApplicationID = chosen.ID - existingProfile.APIKey = app.APIKey - if err := existingProfile.Add(); err != nil { - return nil, err - } - fmt.Fprintf(opts.IO.Out, "%s Profile %q updated with API key.\n", - cs.SuccessIcon(), profileName) + // Reuse a key already stored for this application (keychain, then legacy + // config.toml) before creating a new one on the dashboard. + if !apputil.ReuseExistingAPIKey(opts.Config, chosen) { + if err := apputil.EnsureAPIKey(opts.IO, client, accessToken, chosen); err != nil { + return nil, err } - return chosen, nil - } - - if err := apputil.EnsureAPIKey(opts.IO, client, accessToken, chosen); err != nil { - return nil, err } if err := apputil.ConfigureProfile(opts.IO, opts.Config, chosen, "", true); err != nil { diff --git a/pkg/cmd/auth/crawler/crawler.go b/pkg/cmd/auth/crawler/crawler.go index 86292e59..06a3c6e9 100644 --- a/pkg/cmd/auth/crawler/crawler.go +++ b/pkg/cmd/auth/crawler/crawler.go @@ -45,6 +45,14 @@ func NewCrawlerCmd(f *cmdutil.Factory) *cobra.Command { func runCrawlerCmd(opts *CrawlerOptions) error { cs := opts.IO.ColorScheme() + + appID := opts.config.ActiveApplicationID() + if appID == "" { + return fmt.Errorf( + "no application configured: run `algolia auth login` or `algolia application select` first", + ) + } + dashboardClient := opts.NewDashboardClient(opts.OAuthClientID()) accessToken, err := opts.GetValidToken(dashboardClient) @@ -59,24 +67,13 @@ func runCrawlerCmd(opts *CrawlerOptions) error { return err } - currentProfileName := opts.config.Profile().Name - if currentProfileName == "" { - defaultProfile := opts.config.Default() - if defaultProfile != nil { - currentProfileName = defaultProfile.Name - opts.config.Profile().Name = currentProfileName - } - } - if currentProfileName == "" { - return fmt.Errorf("no profile selected and no default profile configured") - } - - if err = opts.config.SetCrawlerAuth(currentProfileName, crawlerUserData.ID, crawlerUserData.APIKey); err != nil { + if err := opts.config.SetCrawlerAPIKey(appID, crawlerUserData.APIKey); err != nil { return err } if opts.IO.IsStdoutTTY() { - fmt.Fprintf(opts.IO.Out, "%s Crawler API auth credentials configured for profile: %s\n", cs.SuccessIcon(), currentProfileName) + fmt.Fprintf(opts.IO.Out, "%s Crawler API key configured for application: %s\n", + cs.SuccessIcon(), appID) } return nil diff --git a/pkg/cmd/auth/crawler/crawler_test.go b/pkg/cmd/auth/crawler/crawler_test.go index 92152808..3fc54dfe 100644 --- a/pkg/cmd/auth/crawler/crawler_test.go +++ b/pkg/cmd/auth/crawler/crawler_test.go @@ -15,15 +15,12 @@ import ( "github.com/algolia/cli/test" ) -func Test_runCrawlerCmd_UsesDefaultProfile(t *testing.T) { +func Test_runCrawlerCmd_StoresCrawlerKeyForActiveApp(t *testing.T) { io, _, stdout, _ := iostreams.Test() io.SetStdoutTTY(true) - cfg := test.NewConfigStubWithProfiles([]*config.Profile{ - {Name: "default", Default: true}, - {Name: "other"}, - }) - cfg.CurrentProfile.Name = "" + cfg := test.NewDefaultConfigStub() + cfg.ActiveAppID = "APP1" server := newCrawlerTestServer(t, "token-1", "crawler-user", "crawler-key") t.Cleanup(server.Close) @@ -39,39 +36,28 @@ func Test_runCrawlerCmd_UsesDefaultProfile(t *testing.T) { }) require.NoError(t, err) - assert.Equal(t, "default", cfg.CurrentProfile.Name) - assert.Equal(t, test.CrawlerAuth{UserID: "crawler-user", APIKey: "crawler-key"}, cfg.CrawlerAuth["default"]) - assert.Contains(t, stdout.String(), "configured for profile: default") + assert.Equal(t, "crawler-key", cfg.CrawlerKeys["APP1"]) + assert.Contains(t, stdout.String(), "configured for application: APP1") } -func Test_runCrawlerCmd_UsesExplicitProfile(t *testing.T) { +func Test_runCrawlerCmd_ErrorsWithoutActiveApplication(t *testing.T) { io, _, stdout, _ := iostreams.Test() - io.SetStdoutTTY(true) - cfg := test.NewConfigStubWithProfiles([]*config.Profile{ - {Name: "target"}, - {Name: "default", Default: true}, - }) - cfg.CurrentProfile.Name = "target" - - server := newCrawlerTestServer(t, "token-2", "crawler-user-2", "crawler-key-2") - t.Cleanup(server.Close) + cfg := test.NewDefaultConfigStub() // ActiveAppID left empty err := runCrawlerCmd(&CrawlerOptions{ IO: io, config: cfg, OAuthClientID: func() string { return "test-client-id" }, - NewDashboardClient: newDashboardTestClient(server), + NewDashboardClient: func(string) *dashboard.Client { return nil }, GetValidToken: func(client *dashboard.Client) (string, error) { - return "token-2", nil + return "", nil }, }) - require.NoError(t, err) - - assert.Equal(t, test.CrawlerAuth{UserID: "crawler-user-2", APIKey: "crawler-key-2"}, cfg.CrawlerAuth["target"]) - _, hasDefault := cfg.CrawlerAuth["default"] - assert.False(t, hasDefault) - assert.Contains(t, stdout.String(), "configured for profile: target") + require.Error(t, err) + assert.Contains(t, err.Error(), "no application configured") + assert.Empty(t, cfg.CrawlerKeys) + assert.Empty(t, stdout.String()) } func Test_runCrawlerCmd_ReturnsCrawlerAPIError(t *testing.T) { @@ -81,7 +67,7 @@ func Test_runCrawlerCmd_ReturnsCrawlerAPIError(t *testing.T) { cfg := test.NewConfigStubWithProfiles([]*config.Profile{ {Name: "target"}, }) - cfg.CurrentProfile.Name = "target" + cfg.ActiveAppID = "APP1" server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { require.Equal(t, "Bearer token-3", r.Header.Get("Authorization")) @@ -104,7 +90,7 @@ func Test_runCrawlerCmd_ReturnsCrawlerAPIError(t *testing.T) { }) require.Error(t, err) assert.Contains(t, err.Error(), "failed to get crawler user data: crawler access denied") - assert.Empty(t, cfg.CrawlerAuth) + assert.Empty(t, cfg.CrawlerKeys) assert.Empty(t, stdout.String()) } diff --git a/pkg/cmd/auth/login/login.go b/pkg/cmd/auth/login/login.go index 7fdb1461..9622abbd 100644 --- a/pkg/cmd/auth/login/login.go +++ b/pkg/cmd/auth/login/login.go @@ -127,7 +127,7 @@ func RunOAuthFlow(ctx context.Context, opts *LoginOptions, signup bool) error { } appDetails = app - if !reuseExistingAPIKey(opts.Config, appDetails) { + if !apputil.ReuseExistingAPIKey(opts.Config, appDetails) { if err := apputil.EnsureAPIKey(opts.IO, client, accessToken, appDetails); err != nil { return err } @@ -168,18 +168,6 @@ func applyStoredIdentity(ctx context.Context) bool { return true } -// reuseExistingAPIKey checks if a local profile already has an API key for -// the given application. If so, it sets app.APIKey and returns true. -func reuseExistingAPIKey(cfg config.IConfig, app *dashboard.Application) bool { - for _, p := range cfg.ConfiguredProfiles() { - if p.ApplicationID == app.ID && p.APIKey != "" { - app.APIKey = p.APIKey - return true - } - } - return false -} - func selectApplication(opts *LoginOptions, apps []dashboard.Application, interactive bool) (*dashboard.Application, error) { if opts.AppName != "" { for i := range apps { diff --git a/pkg/cmd/factory/default.go b/pkg/cmd/factory/default.go index 2fd4f191..9959c214 100644 --- a/pkg/cmd/factory/default.go +++ b/pkg/cmd/factory/default.go @@ -11,6 +11,8 @@ import ( "github.com/algolia/algoliasearch-client-go/v4/algolia/transport" "github.com/algolia/cli/api/crawler" + "github.com/algolia/cli/api/dashboard" + "github.com/algolia/cli/pkg/auth" "github.com/algolia/cli/pkg/cmdutil" "github.com/algolia/cli/pkg/config" "github.com/algolia/cli/pkg/iostreams" @@ -72,15 +74,38 @@ func searchClient(f *cmdutil.Factory, appVersion string) func() (*search.APIClie } } +// fetchCrawlerUserID retrieves the crawler user ID from the dashboard API +// using the stored OAuth token. The new model never stores it locally, so it +// is fetched when needed. Overridable in tests. +var fetchCrawlerUserID = func() (string, error) { + client := dashboard.NewClient(auth.OAuthClientID()) + token, err := auth.GetValidToken(client) + if err != nil { + return "", err + } + user, err := client.GetCrawlerUser(token) + if err != nil { + return "", err + } + return user.ID, nil +} + func crawlerClient(f *cmdutil.Factory) func() (*crawler.Client, error) { return func() (*crawler.Client, error) { - userID, err := f.Config.Profile().GetCrawlerUserID() + APIKey, err := f.Config.Profile().GetCrawlerAPIKey() if err != nil { return nil, err } - APIKey, err := f.Config.Profile().GetCrawlerAPIKey() + + userID, err := f.Config.Profile().GetCrawlerUserID() if err != nil { - return nil, err + userID, err = fetchCrawlerUserID() + if err != nil { + return nil, fmt.Errorf( + "the crawler user ID is not configured and could not be fetched (run `algolia auth login`): %w", + err, + ) + } } return crawler.NewClient(userID, APIKey), nil diff --git a/pkg/cmd/factory/default_test.go b/pkg/cmd/factory/default_test.go new file mode 100644 index 00000000..bf810678 --- /dev/null +++ b/pkg/cmd/factory/default_test.go @@ -0,0 +1,70 @@ +package factory + +import ( + "errors" + "testing" + + "github.com/spf13/viper" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/algolia/cli/pkg/config" +) + +func Test_crawlerClient_UsesConfiguredUserID(t *testing.T) { + viper.Reset() + t.Cleanup(viper.Reset) + t.Setenv("ALGOLIA_CRAWLER_USER_ID", "configured-user") + t.Setenv("ALGOLIA_CRAWLER_API_KEY", "crawler-key") + + called := false + old := fetchCrawlerUserID + fetchCrawlerUserID = func() (string, error) { + called = true + return "lazy-user", nil + } + t.Cleanup(func() { fetchCrawlerUserID = old }) + + f := New("1.0.0", &config.Config{}) + _, err := f.CrawlerClient() + require.NoError(t, err) + assert.False(t, called) +} + +func Test_crawlerClient_LazyFetchesUserID(t *testing.T) { + viper.Reset() + t.Cleanup(viper.Reset) + t.Setenv("ALGOLIA_CRAWLER_USER_ID", "") + t.Setenv("ALGOLIA_CRAWLER_API_KEY", "crawler-key") + + called := false + old := fetchCrawlerUserID + fetchCrawlerUserID = func() (string, error) { + called = true + return "lazy-user", nil + } + t.Cleanup(func() { fetchCrawlerUserID = old }) + + f := New("1.0.0", &config.Config{}) + _, err := f.CrawlerClient() + require.NoError(t, err) + assert.True(t, called) +} + +func Test_crawlerClient_ErrorsWhenUserIDUnavailable(t *testing.T) { + viper.Reset() + t.Cleanup(viper.Reset) + t.Setenv("ALGOLIA_CRAWLER_USER_ID", "") + t.Setenv("ALGOLIA_CRAWLER_API_KEY", "crawler-key") + + old := fetchCrawlerUserID + fetchCrawlerUserID = func() (string, error) { + return "", errors.New("no stored token") + } + t.Cleanup(func() { fetchCrawlerUserID = old }) + + f := New("1.0.0", &config.Config{}) + _, err := f.CrawlerClient() + require.Error(t, err) + assert.Contains(t, err.Error(), "auth login") +} diff --git a/pkg/cmd/shared/apputil/create.go b/pkg/cmd/shared/apputil/create.go index 82ce5edc..7f47bfad 100644 --- a/pkg/cmd/shared/apputil/create.go +++ b/pkg/cmd/shared/apputil/create.go @@ -10,6 +10,7 @@ import ( "github.com/algolia/cli/api/dashboard" "github.com/algolia/cli/pkg/config" "github.com/algolia/cli/pkg/iostreams" + "github.com/algolia/cli/pkg/keychain" "github.com/algolia/cli/pkg/prompt" ) @@ -142,8 +143,8 @@ func EnsureAPIKey( return nil } -// ConfigureProfile creates a CLI profile from application details and -// optionally sets it as the default. +// ConfigureProfile persists the application credentials in the new model +// (state.toml + OS keychain) and optionally makes it the current application. func ConfigureProfile( io *iostreams.IOStreams, cfg config.IConfig, @@ -161,31 +162,42 @@ func ConfigureProfile( } profileName = strings.ToLower(profileName) - if exists, existingAppID := cfg.ApplicationIDForProfile(profileName); exists && existingAppID != appDetails.ID { + // Another application already carries this alias: derive a unique one. + if otherID, ok := cfg.ApplicationIDByAlias(profileName); ok && otherID != appDetails.ID { profileName = strings.ToLower(appDetails.Name + "-" + appDetails.ID) } - profile := config.Profile{ - Name: profileName, - ApplicationID: appDetails.ID, - APIKey: appDetails.APIKey, - Default: setDefault, + if err := cfg.SaveApplication( + appDetails.ID, profileName, appDetails.APIKeyUUID, appDetails.APIKey, setDefault, + ); err != nil { + return err } - if err := profile.Add(); err != nil { - return err + if io.IsStdoutTTY() { + fmt.Fprintf(io.Out, "%s Application %s configured (alias %q).\n", + cs.SuccessIcon(), cs.Bold(appDetails.ID), profileName) } - if setDefault { - if err := cfg.SetDefaultProfile(profileName); err != nil { - fmt.Fprintf(io.ErrOut, "%s Could not set default profile: %s\n", cs.WarningIcon(), err) - } + return nil +} + +// ReuseExistingAPIKey looks for an API key already stored for the application +// (keychain first, then legacy config.toml profiles). If found, it sets +// app.APIKey and returns true so callers skip creating a new key. A key reused +// from a legacy profile has no known UUID, so api_key_uuid is left as-is. +func ReuseExistingAPIKey(cfg config.IConfig, app *dashboard.Application) bool { + if secrets, err := keychain.LoadAppSecrets(app.ID); err == nil && secrets != nil && + secrets.APIKey != "" { + app.APIKey = secrets.APIKey + return true } - if io.IsStdoutTTY() { - fmt.Fprintf(io.Out, "%s Profile %q configured for application %s.\n", - cs.SuccessIcon(), profileName, cs.Bold(appDetails.ID)) + for _, p := range cfg.ConfiguredProfiles() { + if p.ApplicationID == app.ID && p.APIKey != "" { + app.APIKey = p.APIKey + return true + } } - return nil + return false } diff --git a/pkg/cmd/shared/apputil/create_test.go b/pkg/cmd/shared/apputil/create_test.go new file mode 100644 index 00000000..d2741f17 --- /dev/null +++ b/pkg/cmd/shared/apputil/create_test.go @@ -0,0 +1,83 @@ +package apputil + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/zalando/go-keyring" + + "github.com/algolia/cli/api/dashboard" + "github.com/algolia/cli/pkg/config" + "github.com/algolia/cli/pkg/iostreams" + "github.com/algolia/cli/pkg/keychain" + "github.com/algolia/cli/test" +) + +func TestConfigureProfile_SavesApplication(t *testing.T) { + io, _, _, _ := iostreams.Test() + cfg := test.NewDefaultConfigStub() + app := &dashboard.Application{ + ID: "APP1", Name: "My App", APIKey: "key-1", APIKeyUUID: "uuid-1", + } + + require.NoError(t, ConfigureProfile(io, cfg, app, "", true)) + + assert.Equal(t, + test.SavedApplication{Alias: "my app", APIKeyUUID: "uuid-1", APIKey: "key-1"}, + cfg.SavedApps["APP1"]) + assert.Equal(t, "APP1", cfg.CurrentAppID) +} + +func TestConfigureProfile_ExplicitNameAndNoDefault(t *testing.T) { + io, _, _, _ := iostreams.Test() + cfg := test.NewDefaultConfigStub() + app := &dashboard.Application{ID: "APP1", Name: "My App", APIKey: "key-1"} + + require.NoError(t, ConfigureProfile(io, cfg, app, "Prod", false)) + + assert.Equal(t, "prod", cfg.SavedApps["APP1"].Alias) + assert.Empty(t, cfg.CurrentAppID) +} + +func TestConfigureProfile_AliasCollisionDerivesUniqueAlias(t *testing.T) { + io, _, _, _ := iostreams.Test() + cfg := test.NewDefaultConfigStub() + require.NoError(t, cfg.SaveApplication("OTHER", "my app", "", "other-key", false)) + + app := &dashboard.Application{ID: "APP1", Name: "My App", APIKey: "key-1"} + require.NoError(t, ConfigureProfile(io, cfg, app, "", true)) + + assert.Equal(t, "my app-app1", cfg.SavedApps["APP1"].Alias) +} + +func TestReuseExistingAPIKey_FromKeychain(t *testing.T) { + keyring.MockInit() + require.NoError(t, keychain.SaveAppSecrets("APP1", + keychain.AppSecrets{APIKey: "stored-key"})) + cfg := test.NewDefaultConfigStub() + app := &dashboard.Application{ID: "APP1"} + + assert.True(t, ReuseExistingAPIKey(cfg, app)) + assert.Equal(t, "stored-key", app.APIKey) +} + +func TestReuseExistingAPIKey_FromLegacyProfile(t *testing.T) { + keyring.MockInit() // empty keychain → falls through to config.toml profiles + cfg := test.NewConfigStubWithProfiles([]*config.Profile{ + {Name: "legacy", ApplicationID: "APP1", APIKey: "legacy-key"}, + }) + app := &dashboard.Application{ID: "APP1"} + + assert.True(t, ReuseExistingAPIKey(cfg, app)) + assert.Equal(t, "legacy-key", app.APIKey) +} + +func TestReuseExistingAPIKey_NotFound(t *testing.T) { + keyring.MockInit() + cfg := test.NewDefaultConfigStub() + app := &dashboard.Application{ID: "APP1"} + + assert.False(t, ReuseExistingAPIKey(cfg, app)) + assert.Empty(t, app.APIKey) +} diff --git a/pkg/config/config.go b/pkg/config/config.go index f4e69c38..7822ba88 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -30,6 +30,12 @@ type IConfig interface { SetCrawlerAuth(profileName, crawlerUserID, crawlerAPIKey string) error + // New model (state.toml + OS keychain). + ActiveApplicationID() string + ApplicationIDByAlias(alias string) (string, bool) + SaveApplication(appID, alias, apiKeyUUID, apiKey string, setCurrent bool) error + SetCrawlerAPIKey(appID, crawlerAPIKey string) error + Profile() *Profile Default() *Profile } diff --git a/pkg/config/write.go b/pkg/config/write.go new file mode 100644 index 00000000..23a8c5f5 --- /dev/null +++ b/pkg/config/write.go @@ -0,0 +1,85 @@ +package config + +import ( + "github.com/algolia/cli/pkg/keychain" +) + +// ActiveApplicationID exposes the application resolved by the new model +// (env → flag → state.toml alias → current application). Empty when only the +// legacy config.toml could answer. +func (c *Config) ActiveApplicationID() string { + return c.activeApplicationID() +} + +// ApplicationIDByAlias returns the application ID carrying the given alias in +// state.toml, and whether one was found. +func (c *Config) ApplicationIDByAlias(alias string) (string, bool) { + return c.loadState().ApplicationByAlias(alias) +} + +// SaveApplication persists an application's credentials in the new model. +// The keychain is written first so a failure never leaves state.toml pointing +// at a key that was not stored. Empty alias/apiKeyUUID preserve the values +// already in state.toml, and an existing crawler key is preserved. +// +// Note: a command that already resolved its active application keeps that +// resolution for the rest of the command (per-command cache). +func (c *Config) SaveApplication(appID, alias, apiKeyUUID, apiKey string, setCurrent bool) error { + secrets, err := keychain.LoadAppSecrets(appID) + if err != nil { + return err + } + if secrets == nil { + secrets = &keychain.AppSecrets{} + } + secrets.APIKey = apiKey + if err := keychain.SaveAppSecrets(appID, *secrets); err != nil { + return err + } + c.cacheSecrets(appID, secrets) + + st := c.loadState() + app := st.Applications[appID] + if alias != "" { + app.Alias = alias + } + if apiKeyUUID != "" { + app.APIKeyUUID = apiKeyUUID + } + st.UpsertApplication(appID, app) + if setCurrent { + st.SetCurrentApplication(appID) + } + + return st.Save(c.StateFile) +} + +// SetCrawlerAPIKey stores the crawler API key in the keychain entry of the +// given application, preserving the search API key (load-modify-save). +func (c *Config) SetCrawlerAPIKey(appID, crawlerAPIKey string) error { + secrets, err := keychain.LoadAppSecrets(appID) + if err != nil { + return err + } + if secrets == nil { + secrets = &keychain.AppSecrets{} + } + secrets.CrawlerAPIKey = crawlerAPIKey + if err := keychain.SaveAppSecrets(appID, *secrets); err != nil { + return err + } + c.cacheSecrets(appID, secrets) + + return nil +} + +// cacheSecrets refreshes the per-command secrets cache after a write so reads +// in the same command observe the new values. +func (c *Config) cacheSecrets(appID string, secrets *keychain.AppSecrets) { + c.secretsMu.Lock() + defer c.secretsMu.Unlock() + if c.secretsCache == nil { + c.secretsCache = map[string]*keychain.AppSecrets{} + } + c.secretsCache[appID] = secrets +} diff --git a/pkg/config/write_test.go b/pkg/config/write_test.go new file mode 100644 index 00000000..360be9f9 --- /dev/null +++ b/pkg/config/write_test.go @@ -0,0 +1,125 @@ +package config + +import ( + "errors" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/zalando/go-keyring" + + "github.com/algolia/cli/pkg/keychain" +) + +func TestConfig_SaveApplication_WritesKeychainThenState(t *testing.T) { + keyring.MockInit() + cfg := &Config{StateFile: filepath.Join(t.TempDir(), "state.toml")} + + require.NoError(t, cfg.SaveApplication("APP1", "prod", "uuid-1", "key-1", true)) + + 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, "APP1", st.CurrentApplicationID) + assert.Equal(t, + ApplicationState{APIKeyUUID: "uuid-1", Alias: "prod"}, + st.Applications["APP1"]) +} + +func TestConfig_SaveApplication_PreservesExistingValues(t *testing.T) { + keyring.MockInit() + cfg := &Config{StateFile: filepath.Join(t.TempDir(), "state.toml")} + + require.NoError(t, keychain.SaveAppSecrets("APP1", + keychain.AppSecrets{APIKey: "old-key", CrawlerAPIKey: "crawler-key"})) + require.NoError(t, cfg.SaveApplication("APP1", "custom", "uuid-1", "old-key", false)) + + // Empty alias and UUID keep the stored values; the crawler key survives. + require.NoError(t, cfg.SaveApplication("APP1", "", "", "new-key", false)) + + secrets, err := keychain.LoadAppSecrets("APP1") + require.NoError(t, err) + assert.Equal(t, "new-key", secrets.APIKey) + assert.Equal(t, "crawler-key", secrets.CrawlerAPIKey) + + st, err := LoadState(cfg.StateFile) + require.NoError(t, err) + assert.Equal(t, + ApplicationState{APIKeyUUID: "uuid-1", Alias: "custom"}, + st.Applications["APP1"]) + assert.Empty(t, st.CurrentApplicationID) // setCurrent was always false +} + +func TestConfig_SaveApplication_KeychainErrorAborts(t *testing.T) { + keyring.MockInitWithError(errors.New("keychain locked")) + cfg := &Config{StateFile: filepath.Join(t.TempDir(), "state.toml")} + + require.Error(t, cfg.SaveApplication("APP1", "prod", "uuid-1", "key-1", true)) + + // Keychain-first: state.toml must not exist after a keychain failure. + _, err := os.Stat(cfg.StateFile) + assert.True(t, os.IsNotExist(err)) +} + +func TestConfig_SaveApplication_RefreshesSecretsCache(t *testing.T) { + keyring.MockInit() + cfg := &Config{StateFile: filepath.Join(t.TempDir(), "state.toml")} + + // Prime the negative cache: nothing stored yet. + require.Nil(t, cfg.appSecretsFor("APP1")) + + require.NoError(t, cfg.SaveApplication("APP1", "prod", "uuid-1", "key-1", true)) + + got := cfg.appSecretsFor("APP1") + require.NotNil(t, got) + assert.Equal(t, "key-1", got.APIKey) +} + +func TestConfig_SetCrawlerAPIKey_PreservesSearchKey(t *testing.T) { + keyring.MockInit() + cfg := &Config{} + + require.NoError(t, keychain.SaveAppSecrets("APP1", + keychain.AppSecrets{APIKey: "search-key"})) + require.NoError(t, cfg.SetCrawlerAPIKey("APP1", "crawler-key")) + + secrets, err := keychain.LoadAppSecrets("APP1") + require.NoError(t, err) + assert.Equal(t, "search-key", secrets.APIKey) + assert.Equal(t, "crawler-key", secrets.CrawlerAPIKey) +} + +func TestConfig_SetCrawlerAPIKey_CreatesEntryWhenMissing(t *testing.T) { + keyring.MockInit() + cfg := &Config{} + + require.NoError(t, cfg.SetCrawlerAPIKey("APP1", "crawler-key")) + + secrets, err := keychain.LoadAppSecrets("APP1") + require.NoError(t, err) + require.NotNil(t, secrets) + assert.Equal(t, "crawler-key", secrets.CrawlerAPIKey) + assert.Empty(t, secrets.APIKey) +} + +func TestConfig_ActiveApplicationIDAndAliasAccessors(t *testing.T) { + path := filepath.Join(t.TempDir(), "state.toml") + require.NoError(t, os.WriteFile( + path, + []byte("current_application_id = \"APP1\"\n\n[applications.APP1]\nalias = \"prod\"\n"), + 0o600, + )) + cfg := &Config{StateFile: path} + + assert.Equal(t, "APP1", cfg.ActiveApplicationID()) + + appID, ok := cfg.ApplicationIDByAlias("prod") + assert.True(t, ok) + assert.Equal(t, "APP1", appID) +} diff --git a/test/config.go b/test/config.go index 6ae88d3e..071ead37 100644 --- a/test/config.go +++ b/test/config.go @@ -11,10 +11,22 @@ type CrawlerAuth struct { APIKey string } +// SavedApplication records what SaveApplication stored for an application. +type SavedApplication struct { + Alias string + APIKeyUUID string + APIKey string +} + type ConfigStub struct { CurrentProfile config.Profile profiles []*config.Profile CrawlerAuth map[string]CrawlerAuth + + ActiveAppID string + CurrentAppID string + SavedApps map[string]SavedApplication + CrawlerKeys map[string]string } func (c *ConfigStub) InitConfig() {} @@ -129,3 +141,43 @@ func NewDefaultConfigStub() *ConfigStub { }, }) } + +func (c *ConfigStub) ActiveApplicationID() string { + return c.ActiveAppID +} + +func (c *ConfigStub) ApplicationIDByAlias(alias string) (string, bool) { + for appID, app := range c.SavedApps { + if app.Alias == alias { + return appID, true + } + } + return "", false +} + +func (c *ConfigStub) SaveApplication(appID, alias, apiKeyUUID, apiKey string, setCurrent bool) error { + if c.SavedApps == nil { + c.SavedApps = map[string]SavedApplication{} + } + saved := c.SavedApps[appID] + if alias != "" { + saved.Alias = alias + } + if apiKeyUUID != "" { + saved.APIKeyUUID = apiKeyUUID + } + saved.APIKey = apiKey + c.SavedApps[appID] = saved + if setCurrent { + c.CurrentAppID = appID + } + return nil +} + +func (c *ConfigStub) SetCrawlerAPIKey(appID, crawlerAPIKey string) error { + if c.CrawlerKeys == nil { + c.CrawlerKeys = map[string]string{} + } + c.CrawlerKeys[appID] = crawlerAPIKey + return nil +} From 0ce4ade9278caf4d99519ac902b25c1736c4d411 Mon Sep 17 00:00:00 2001 From: Lorris Saint-Genez Date: Thu, 11 Jun 2026 12:41:41 -0700 Subject: [PATCH 6/8] feat(profile): deprecate profile commands and --profile flag (#237) --- pkg/cmd/application/create/create.go | 8 ++++---- pkg/cmd/application/downgrade/downgrade.go | 2 +- pkg/cmd/application/list/list.go | 12 ++++++------ pkg/cmd/application/planchange/planchange.go | 2 +- pkg/cmd/application/selectapp/select.go | 10 ++++------ pkg/cmd/application/update/update.go | 4 ++-- pkg/cmd/application/upgrade/upgrade.go | 2 +- pkg/cmd/auth/crawler/crawler.go | 2 +- pkg/cmd/auth/login/login.go | 4 ++-- pkg/cmd/auth/signup/signup.go | 4 ++-- pkg/cmd/crawler/crawler.go | 2 +- pkg/cmd/profile/add/add.go | 4 +++- pkg/cmd/profile/application.go | 2 +- pkg/cmd/profile/list/list.go | 4 +++- pkg/cmd/profile/remove/remove.go | 4 +++- pkg/cmd/profile/setdefault/setdefault.go | 4 +++- pkg/cmd/root/root.go | 11 ++++++++--- 17 files changed, 46 insertions(+), 35 deletions(-) diff --git a/pkg/cmd/application/create/create.go b/pkg/cmd/application/create/create.go index ed623900..b3684534 100644 --- a/pkg/cmd/application/create/create.go +++ b/pkg/cmd/application/create/create.go @@ -54,7 +54,7 @@ func NewCreateCmd(f *cmdutil.Factory) *cobra.Command { Use: "create", Short: "Create a new Algolia application", Long: heredoc.Doc(` - Create a new Algolia application and optionally configure it as a CLI profile. + Create a new Algolia application and optionally set it as the current application. Requires an active session (run "algolia auth login" first).`), Example: heredoc.Doc(` # Create an application interactively (prompts for name, plan, and terms) @@ -66,7 +66,7 @@ func NewCreateCmd(f *cmdutil.Factory) *cobra.Command { # Create on a paid plan (requires a payment method on file) $ algolia application create --name "My App" --region CA --plan grow --accept-terms - # Create and set the new profile as the default + # Create and set the new application as the current one $ algolia application create --name "My App" --region CA --accept-terms --default # Preview what would be created without actually creating it @@ -85,10 +85,10 @@ func NewCreateCmd(f *cmdutil.Factory) *cobra.Command { cmd.Flags().StringVar(&opts.Name, "name", "My First Application", "Name for the application") cmd.Flags().StringVar(&opts.Region, "region", "", "Region code (e.g. EU, UK, USC, USE, USW)") cmd.Flags(). - StringVar(&opts.ProfileName, "profile-name", "", "Name for the CLI profile (defaults to app name)") + StringVar(&opts.ProfileName, "profile-name", "", "Alias for the application (defaults to the app name)") cmd.Flags(). StringVar(&opts.Plan, "plan", "", "Self-serve plan to create the application on (free, grow, grow-plus)") - cmd.Flags().BoolVar(&opts.Default, "default", false, "Set the new profile as the default") + cmd.Flags().BoolVar(&opts.Default, "default", false, "Set the new application as the current one") cmd.Flags(). BoolVar(&opts.DryRun, "dry-run", false, "Preview the create request without sending it") cmd.Flags(). diff --git a/pkg/cmd/application/downgrade/downgrade.go b/pkg/cmd/application/downgrade/downgrade.go index 27013e51..f9352922 100644 --- a/pkg/cmd/application/downgrade/downgrade.go +++ b/pkg/cmd/application/downgrade/downgrade.go @@ -25,7 +25,7 @@ func NewDowngradeCmd(f *cmdutil.Factory) *cobra.Command { Use: "downgrade", Short: "Downgrade the current application to a lower-tier plan", Long: heredoc.Doc(` - Change the application associated with the current CLI profile to a + Change the current application to a lower-tier self-serve plan. You must accept the target plan's terms of service before the change diff --git a/pkg/cmd/application/list/list.go b/pkg/cmd/application/list/list.go index 7dc1f3b3..3473e877 100644 --- a/pkg/cmd/application/list/list.go +++ b/pkg/cmd/application/list/list.go @@ -42,8 +42,8 @@ func NewListCmd(f *cmdutil.Factory) *cobra.Command { Long: heredoc.Doc(` List all Algolia applications associated with your account. Requires an active session (run "algolia auth login" first). - Applications that already have a local CLI profile are marked. - You can select an unconfigured application to add it as a CLI profile. + Applications already configured on this machine are marked. + You can select an unconfigured application to configure it. `), Example: heredoc.Doc(` # List applications @@ -117,7 +117,7 @@ func runListCmd(opts *ListOptions) error { profileName, configured := configuredAppIDs[app.ID] label := fmt.Sprintf(" %s %s", app.ID, app.Name) if configured { - fmt.Fprintf(opts.IO.Out, "%s %s\n", label, cs.Greenf("(profile: %s)", profileName)) + fmt.Fprintf(opts.IO.Out, "%s %s\n", label, cs.Greenf("(configured: %s)", profileName)) } else { fmt.Fprintf(opts.IO.Out, "%s %s\n", label, cs.Gray("(not configured)")) unconfigured = append(unconfigured, app) @@ -127,7 +127,7 @@ func runListCmd(opts *ListOptions) error { fmt.Fprintln(opts.IO.Out) if len(unconfigured) == 0 { - fmt.Fprintf(opts.IO.Out, "%s All applications are already configured as CLI profiles.\n", cs.SuccessIcon()) + fmt.Fprintf(opts.IO.Out, "%s All applications are already configured.\n", cs.SuccessIcon()) return nil } @@ -138,7 +138,7 @@ func runListCmd(opts *ListOptions) error { var wantConfigure bool err = prompt.SurveyAskOne( &survey.Confirm{ - Message: "Would you like to configure an unconfigured application as a CLI profile?", + Message: "Would you like to configure one of the unconfigured applications?", Default: true, }, &wantConfigure, @@ -173,7 +173,7 @@ func runListCmd(opts *ListOptions) error { var setDefault bool err = prompt.SurveyAskOne( &survey.Confirm{ - Message: "Set as the default profile?", + Message: "Set as the current application?", Default: false, }, &setDefault, diff --git a/pkg/cmd/application/planchange/planchange.go b/pkg/cmd/application/planchange/planchange.go index 9c604164..48096ac7 100644 --- a/pkg/cmd/application/planchange/planchange.go +++ b/pkg/cmd/application/planchange/planchange.go @@ -60,7 +60,7 @@ func Run(opts *Options) error { appID, err := opts.Config.Profile().GetApplicationID() if err != nil { return fmt.Errorf( - "no current application configured; configure a profile with \"algolia profile add\" or \"algolia application select\" first: %w", + "no current application configured; run \"algolia auth login\" or \"algolia application select\" first: %w", err, ) } diff --git a/pkg/cmd/application/selectapp/select.go b/pkg/cmd/application/selectapp/select.go index 55fcd3d5..fa143286 100644 --- a/pkg/cmd/application/selectapp/select.go +++ b/pkg/cmd/application/selectapp/select.go @@ -37,13 +37,11 @@ func NewSelectCmd(f *cmdutil.Factory) *cobra.Command { cmd := &cobra.Command{ Use: "select", - Short: "Select an application to use as the active profile", + Short: "Select the current application", Long: heredoc.Doc(` - Select an Algolia application to use as the default CLI profile. - Fetches your applications from the API and lets you pick one. - - If the selected application already has a local profile, it is set - as the default. Otherwise, a new profile is created and set as default. + Select an Algolia application to use as the current application for + all CLI commands. Fetches your applications from the API and lets + you pick one. `), Example: heredoc.Doc(` # Select interactively diff --git a/pkg/cmd/application/update/update.go b/pkg/cmd/application/update/update.go index 7a0cbab5..f453ffd4 100644 --- a/pkg/cmd/application/update/update.go +++ b/pkg/cmd/application/update/update.go @@ -39,7 +39,7 @@ func NewUpdateCmd(f *cmdutil.Factory) *cobra.Command { Use: "update", Short: "Rename the current Algolia application", Long: heredoc.Doc(` - Rename the application associated with the current CLI profile. + Rename the current application. Requires an active application to be selected (run "algolia application select" first). `), Example: heredoc.Doc(` @@ -69,7 +69,7 @@ func runUpdateCmd(opts *UpdateOptions) error { appID, err := opts.Config.Profile().GetApplicationID() if err != nil { return fmt.Errorf( - "no current application configured; configure a profile with \"algolia profile add\" or \"algolia application select\" first: %w", + "no current application configured; run \"algolia auth login\" or \"algolia application select\" first: %w", err, ) } diff --git a/pkg/cmd/application/upgrade/upgrade.go b/pkg/cmd/application/upgrade/upgrade.go index ed1cd632..f1d31122 100644 --- a/pkg/cmd/application/upgrade/upgrade.go +++ b/pkg/cmd/application/upgrade/upgrade.go @@ -25,7 +25,7 @@ func NewUpgradeCmd(f *cmdutil.Factory) *cobra.Command { Use: "upgrade", Short: "Upgrade the current application to a higher-tier plan", Long: heredoc.Doc(` - Change the application associated with the current CLI profile to a + Change the current application to a higher-tier self-serve plan. Paid plans require a payment method on your account; the CLI can't diff --git a/pkg/cmd/auth/crawler/crawler.go b/pkg/cmd/auth/crawler/crawler.go index 06a3c6e9..850d95f2 100644 --- a/pkg/cmd/auth/crawler/crawler.go +++ b/pkg/cmd/auth/crawler/crawler.go @@ -33,7 +33,7 @@ func NewCrawlerCmd(f *cmdutil.Factory) *cobra.Command { cmd := &cobra.Command{ Use: "crawler", - Short: "Load crawler auth details for the current profile", + Short: "Configure the crawler API key for the current application", Args: validators.NoArgs(), RunE: func(cmd *cobra.Command, args []string) error { return runCrawlerCmd(opts) diff --git a/pkg/cmd/auth/login/login.go b/pkg/cmd/auth/login/login.go index 9622abbd..8fa4b530 100644 --- a/pkg/cmd/auth/login/login.go +++ b/pkg/cmd/auth/login/login.go @@ -78,8 +78,8 @@ func NewLoginCmd(f *cmdutil.Factory) *cobra.Command { } cmd.Flags().StringVar(&opts.AppName, "app-name", "", "Auto-select application by name") - cmd.Flags().StringVar(&opts.ProfileName, "profile-name", "", "Name for the CLI profile (defaults to application name)") - cmd.Flags().BoolVar(&opts.Default, "default", true, "Set the profile as the default") + cmd.Flags().StringVar(&opts.ProfileName, "profile-name", "", "Alias for the application (defaults to the application name)") + cmd.Flags().BoolVar(&opts.Default, "default", true, "Set the application as the current one") cmd.Flags().BoolVar(&opts.NoBrowser, "no-browser", false, "Print the authorize URL instead of opening the browser") return cmd diff --git a/pkg/cmd/auth/signup/signup.go b/pkg/cmd/auth/signup/signup.go index 7b68d6da..eb3c9a8c 100644 --- a/pkg/cmd/auth/signup/signup.go +++ b/pkg/cmd/auth/signup/signup.go @@ -39,8 +39,8 @@ func NewSignupCmd(f *cmdutil.Factory) *cobra.Command { } cmd.Flags().StringVar(&opts.AppName, "app-name", "", "Name for the first application") - cmd.Flags().StringVar(&opts.ProfileName, "profile-name", "", "Name for the CLI profile (defaults to application name)") - cmd.Flags().BoolVar(&opts.Default, "default", true, "Set the profile as the default") + cmd.Flags().StringVar(&opts.ProfileName, "profile-name", "", "Alias for the application (defaults to the application name)") + cmd.Flags().BoolVar(&opts.Default, "default", true, "Set the application as the current one") cmd.Flags().BoolVar(&opts.NoBrowser, "no-browser", false, "Print the authorize URL instead of opening the browser") return cmd diff --git a/pkg/cmd/crawler/crawler.go b/pkg/cmd/crawler/crawler.go index 9e82c1b2..046df067 100644 --- a/pkg/cmd/crawler/crawler.go +++ b/pkg/cmd/crawler/crawler.go @@ -23,7 +23,7 @@ import ( const ( AuthMethodHelpMsg = `In order to use the 'crawler' commands, you will need to authenticate with the Algolia Crawler API. You can do so by either: - Export your Algolia Crawler username and API Key as ALGOLIA_CRAWLER_USER_ID and ALGOLIA_CRAWLER_API_KEY environment variables. - - Add your Algolia Crawler 'crawler_user_id' and 'crawler_api_key' credentials to your profile file (~/.config/algolia/config.tml).` + - Run 'algolia auth crawler' to store your crawler API key securely in the OS keychain.` ) // NewCrawlersCmd returns a new command to manage your Algolia Crawlers. diff --git a/pkg/cmd/profile/add/add.go b/pkg/cmd/profile/add/add.go index bab975d3..de335395 100644 --- a/pkg/cmd/profile/add/add.go +++ b/pkg/cmd/profile/add/add.go @@ -82,7 +82,7 @@ func NewAddCmd(f *cmdutil.Factory, runF func(*AddOptions) error) *cobra.Command cmd := &cobra.Command{ Use: "add", Args: validators.NoArgs(), - Short: "Add a new profile configuration to the CLI", + Short: "[Deprecated] Add a new profile configuration to the CLI", Example: heredoc.Doc(` # Add a new profile (interactive) $ algolia profile add @@ -139,6 +139,8 @@ func NewAddCmd(f *cmdutil.Factory, runF func(*AddOptions) error) *cobra.Command // runAddCmd executes the add command func runAddCmd(opts *AddOptions) error { + fmt.Fprintf(opts.IO.ErrOut, + "warning: `algolia profile add` is deprecated, use `algolia auth login` or `algolia application select` instead\n") var defaultProfile *config.Profile for _, profile := range opts.config.ConfiguredProfiles() { if profile.Default { diff --git a/pkg/cmd/profile/application.go b/pkg/cmd/profile/application.go index f3f44c85..3fb01c58 100644 --- a/pkg/cmd/profile/application.go +++ b/pkg/cmd/profile/application.go @@ -16,7 +16,7 @@ func NewProfileCmd(f *cmdutil.Factory) *cobra.Command { cmd := &cobra.Command{ Use: "profile", Aliases: []string{"profiles"}, - Short: "Manage your Algolia CLI profiles", + Short: "[Deprecated] Manage your Algolia CLI profiles", } auth.DisableAuthCheck(cmd) diff --git a/pkg/cmd/profile/list/list.go b/pkg/cmd/profile/list/list.go index c9e49c6a..115f6b34 100644 --- a/pkg/cmd/profile/list/list.go +++ b/pkg/cmd/profile/list/list.go @@ -32,7 +32,7 @@ func NewListCmd(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman Use: "list", Aliases: []string{"l"}, Args: validators.NoArgs(), - Short: "List the configured profile(s)", + Short: "[Deprecated] List the configured profile(s)", Example: heredoc.Doc(` # List the configured profiles $ algolia profile list @@ -51,6 +51,8 @@ func NewListCmd(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman // runListCmd executes the list command func runListCmd(opts *ListOptions) error { + fmt.Fprintf(opts.IO.ErrOut, + "warning: `algolia profile list` is deprecated, use `algolia application list` instead\n") profiles := opts.config.ConfiguredProfiles() if len(profiles) == 0 { fmt.Fprintln(opts.IO.ErrOut, "No configured profiles") diff --git a/pkg/cmd/profile/remove/remove.go b/pkg/cmd/profile/remove/remove.go index 094a03f0..83704396 100644 --- a/pkg/cmd/profile/remove/remove.go +++ b/pkg/cmd/profile/remove/remove.go @@ -37,7 +37,7 @@ func NewRemoveCmd(f *cmdutil.Factory, runF func(*RemoveOptions) error) *cobra.Co Use: "remove ", Args: validators.ExactArgs(1), ValidArgsFunction: cmdutil.ConfiguredProfilesCompletionFunc(f), - Short: "Remove the specified profile", + Short: "[Deprecated] Remove the specified profile", Long: `Remove the specified profile from the configuration.`, Example: heredoc.Doc(` # Remove the profile named "my-app" from the configuration @@ -81,6 +81,8 @@ func NewRemoveCmd(f *cmdutil.Factory, runF func(*RemoveOptions) error) *cobra.Co // runRemoveCmd executes the remove command func runRemoveCmd(opts *RemoveOptions) error { + fmt.Fprintf(opts.IO.ErrOut, + "warning: `algolia profile remove` is deprecated, profiles are replaced by `algolia application select` and the OS keychain\n") if opts.DoConfirm { var confirmed bool err := prompt.Confirm( diff --git a/pkg/cmd/profile/setdefault/setdefault.go b/pkg/cmd/profile/setdefault/setdefault.go index db27c597..091f6630 100644 --- a/pkg/cmd/profile/setdefault/setdefault.go +++ b/pkg/cmd/profile/setdefault/setdefault.go @@ -30,7 +30,7 @@ func NewSetDefaultCmd(f *cmdutil.Factory, runF func(*SetDefaultOptions) error) * Use: "setdefault ", Args: validators.ExactArgs(1), ValidArgsFunction: cmdutil.ConfiguredProfilesCompletionFunc(f), - Short: "Set the default profile", + Short: "[Deprecated] Set the default profile", Example: heredoc.Doc(` # Set the default profile to "my-app" $ algolia profile setdefault my-app @@ -55,6 +55,8 @@ func NewSetDefaultCmd(f *cmdutil.Factory, runF func(*SetDefaultOptions) error) * // runSetDefaultCmd executes the setdefault command func runSetDefaultCmd(opts *SetDefaultOptions) error { + fmt.Fprintf(opts.IO.ErrOut, + "warning: `algolia profile setdefault` is deprecated, use `algolia application select` instead\n") var defaultName string for _, profile := range opts.config.ConfiguredProfiles() { if profile.Default { diff --git a/pkg/cmd/root/root.go b/pkg/cmd/root/root.go index 9fb6494f..069e7f28 100644 --- a/pkg/cmd/root/root.go +++ b/pkg/cmd/root/root.go @@ -83,11 +83,16 @@ func NewRootCmd(f *cmdutil.Factory) *cobra.Command { cmd.PersistentFlags(). StringVarP(&f.Config.Profile().Name, "profile", "p", "", "The profile to use") + // Deprecated but kept visible in help (MarkDeprecated would also hide it). + cmd.PersistentFlags(). + Lookup("profile"). + Deprecated = "use --application-id or 'algolia application select' instead" _ = cmd.RegisterFlagCompletionFunc("profile", cmdutil.ConfiguredProfilesCompletionFunc(f)) cmd.PersistentFlags(). - StringVarP(&f.Config.Profile().ApplicationID, "application-id", "", "", "The application ID") - cmd.PersistentFlags().StringVarP(&f.Config.Profile().APIKey, "api-key", "", "", "The API key") + StringVarP(&f.Config.Profile().ApplicationID, "application-id", "", "", "The application ID (defaults to the current application, set with 'algolia application select')") + cmd.PersistentFlags(). + StringVarP(&f.Config.Profile().APIKey, "api-key", "", "", "The API key (defaults to the key stored for the current application)") cmd.PersistentFlags(). StringVarP(&f.Config.Profile().AdminAPIKey, "admin-api-key", "", "", "The admin API key") _ = cmd.PersistentFlags().MarkDeprecated("admin-api-key", "use --api-key instead") @@ -152,7 +157,7 @@ func Execute() exitCode { fmt.Fprintf(stderr, "Authentication error: %s\n", err) fmt.Fprintln( stderr, - "Please run `algolia profile add` to configure your first profile.", + "Please run `algolia auth login` to get started.", ) return authError } From 26ae1792b44275f7bc09144a278b9f6c0625cd1d Mon Sep 17 00:00:00 2001 From: Lorris Saint-Genez Date: Thu, 11 Jun 2026 14:21:13 -0700 Subject: [PATCH 7/8] feat(profile): warn when config.toml profile changes will be ignored MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit profile add/remove/setdefault still write config.toml; once state.toml exists (new storage model in use), changes to legacy profiles stop being honored in a future version — say so before executing. Co-Authored-By: Claude Fable 5 --- pkg/cmd/profile/add/add.go | 4 ++ pkg/cmd/profile/remove/remove.go | 4 ++ pkg/cmd/profile/setdefault/setdefault.go | 4 ++ pkg/cmd/profile/setdefault/setdefault_test.go | 38 ++++++++++++++----- pkg/config/config.go | 11 ++++++ test/config.go | 5 +++ 6 files changed, 57 insertions(+), 9 deletions(-) diff --git a/pkg/cmd/profile/add/add.go b/pkg/cmd/profile/add/add.go index de335395..98530aaa 100644 --- a/pkg/cmd/profile/add/add.go +++ b/pkg/cmd/profile/add/add.go @@ -141,6 +141,10 @@ func NewAddCmd(f *cmdutil.Factory, runF func(*AddOptions) error) *cobra.Command func runAddCmd(opts *AddOptions) error { fmt.Fprintf(opts.IO.ErrOut, "warning: `algolia profile add` is deprecated, use `algolia auth login` or `algolia application select` instead\n") + if opts.config.StateFileExists() { + fmt.Fprintf(opts.IO.ErrOut, + "warning: the CLI now stores credentials in state.toml and the OS keychain; changes to config.toml profiles will be ignored in a future version\n") + } var defaultProfile *config.Profile for _, profile := range opts.config.ConfiguredProfiles() { if profile.Default { diff --git a/pkg/cmd/profile/remove/remove.go b/pkg/cmd/profile/remove/remove.go index 83704396..61147fd6 100644 --- a/pkg/cmd/profile/remove/remove.go +++ b/pkg/cmd/profile/remove/remove.go @@ -83,6 +83,10 @@ func NewRemoveCmd(f *cmdutil.Factory, runF func(*RemoveOptions) error) *cobra.Co func runRemoveCmd(opts *RemoveOptions) error { fmt.Fprintf(opts.IO.ErrOut, "warning: `algolia profile remove` is deprecated, profiles are replaced by `algolia application select` and the OS keychain\n") + if opts.config.StateFileExists() { + fmt.Fprintf(opts.IO.ErrOut, + "warning: the CLI now stores credentials in state.toml and the OS keychain; changes to config.toml profiles will be ignored in a future version\n") + } if opts.DoConfirm { var confirmed bool err := prompt.Confirm( diff --git a/pkg/cmd/profile/setdefault/setdefault.go b/pkg/cmd/profile/setdefault/setdefault.go index 091f6630..da391616 100644 --- a/pkg/cmd/profile/setdefault/setdefault.go +++ b/pkg/cmd/profile/setdefault/setdefault.go @@ -57,6 +57,10 @@ func NewSetDefaultCmd(f *cmdutil.Factory, runF func(*SetDefaultOptions) error) * func runSetDefaultCmd(opts *SetDefaultOptions) error { fmt.Fprintf(opts.IO.ErrOut, "warning: `algolia profile setdefault` is deprecated, use `algolia application select` instead\n") + if opts.config.StateFileExists() { + fmt.Fprintf(opts.IO.ErrOut, + "warning: the CLI now stores credentials in state.toml and the OS keychain; changes to config.toml profiles will be ignored in a future version\n") + } var defaultName string for _, profile := range opts.config.ConfiguredProfiles() { if profile.Default { diff --git a/pkg/cmd/profile/setdefault/setdefault_test.go b/pkg/cmd/profile/setdefault/setdefault_test.go index 25a39c8a..ce33395e 100644 --- a/pkg/cmd/profile/setdefault/setdefault_test.go +++ b/pkg/cmd/profile/setdefault/setdefault_test.go @@ -1,6 +1,7 @@ package setdefault import ( + "strings" "testing" "github.com/algolia/cli/pkg/config" @@ -10,17 +11,21 @@ import ( func Test_runSetDefaultCmd(t *testing.T) { tests := []struct { - name string - cli string - profiles map[string]bool - wantsErr string - wantOut string + name string + cli string + profiles map[string]bool + hasStateFile bool + wantsErr string + wantOut string + wantErrOut string + notWantErrOut string }{ { - name: "existing default", - cli: "foo", - profiles: map[string]bool{"default": true, "foo": false}, - wantOut: "✓ Default profile successfuly changed from 'default' to 'foo'.\n", + name: "existing default", + cli: "foo", + profiles: map[string]bool{"default": true, "foo": false}, + wantOut: "✓ Default profile successfuly changed from 'default' to 'foo'.\n", + notWantErrOut: "state.toml", }, { name: "non-existing default", @@ -28,6 +33,14 @@ func Test_runSetDefaultCmd(t *testing.T) { profiles: map[string]bool{"foo": false}, wantOut: "✓ Default profile successfuly set to 'foo'.\n", }, + { + name: "state file exists", + cli: "foo", + profiles: map[string]bool{"default": true, "foo": false}, + hasStateFile: true, + wantOut: "✓ Default profile successfuly changed from 'default' to 'foo'.\n", + wantErrOut: "changes to config.toml profiles will be ignored in a future version", + }, } for _, tt := range tests { @@ -40,6 +53,7 @@ func Test_runSetDefaultCmd(t *testing.T) { }) } cfg := test.NewConfigStubWithProfiles(p) + cfg.HasStateFile = tt.hasStateFile f, out := test.NewFactory(true, nil, cfg, "") cmd := NewSetDefaultCmd(f, nil) out, err := test.Execute(cmd, tt.cli, out) @@ -49,6 +63,12 @@ func Test_runSetDefaultCmd(t *testing.T) { } assert.Equal(t, tt.wantOut, out.String()) + if tt.wantErrOut != "" { + assert.Equal(t, true, strings.Contains(out.Stderr(), tt.wantErrOut)) + } + if tt.notWantErrOut != "" { + assert.Equal(t, false, strings.Contains(out.Stderr(), tt.notWantErrOut)) + } }) } } diff --git a/pkg/config/config.go b/pkg/config/config.go index 7822ba88..d09d4c69 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -35,6 +35,7 @@ type IConfig interface { ApplicationIDByAlias(alias string) (string, bool) SaveApplication(appID, alias, apiKeyUUID, apiKey string, setCurrent bool) error SetCrawlerAPIKey(appID, crawlerAPIKey string) error + StateFileExists() bool Profile() *Profile Default() *Profile @@ -124,6 +125,16 @@ func (c *Config) loadState() *State { return c.state } +// StateFileExists reports whether state.toml exists on disk, i.e. the new +// storage model (state.toml + OS keychain) is already in use on this machine. +func (c *Config) StateFileExists() bool { + if c.StateFile == "" { + return false + } + _, err := os.Stat(c.StateFile) + return err == nil +} + // activeApplicationID resolves (once per command) which application the new // model should read against. Returns "" when no new-model app applies, so the // caller falls back to config.toml. diff --git a/test/config.go b/test/config.go index 071ead37..0b0828ef 100644 --- a/test/config.go +++ b/test/config.go @@ -27,6 +27,7 @@ type ConfigStub struct { CurrentAppID string SavedApps map[string]SavedApplication CrawlerKeys map[string]string + HasStateFile bool } func (c *ConfigStub) InitConfig() {} @@ -174,6 +175,10 @@ func (c *ConfigStub) SaveApplication(appID, alias, apiKeyUUID, apiKey string, se return nil } +func (c *ConfigStub) StateFileExists() bool { + return c.HasStateFile +} + func (c *ConfigStub) SetCrawlerAPIKey(appID, crawlerAPIKey string) error { if c.CrawlerKeys == nil { c.CrawlerKeys = map[string]string{} From 3261a0180f903be6601511cce8ed7fa20469d969 Mon Sep 17 00:00:00 2001 From: Lorris Saint-Genez Date: Mon, 15 Jun 2026 14:08:48 -0700 Subject: [PATCH 8/8] feat(config): one-time config.toml migration to state.toml + keychain (#248) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(config): trigger the config.toml migration at startup (#245) Co-authored-by: Claude Fable 5 * chore(config): slim down the migration comments Co-Authored-By: Claude Fable 5 * fix(config): harden the migration against bad config.toml files Two startup-path regressions found while testing edge cases: - An undecodable profile (root-level scalar, unconvertible field type) hit ConfiguredProfiles' log.Fatalf and bricked every command, --help included, forever (state.toml never written). The migration now decodes profiles itself and skips undecodable entries with a log. - An unparseable config.toml was silently read as zero profiles, so an empty state.toml marked the migration as done forever. Migrate now aborts when config.toml cannot be parsed and retries on the next run. Co-Authored-By: Claude Fable 5 * feat(config): migrate search_hosts and crawler_user_id to state.toml The remaining non-secret profile data moves with the migration so it survives the eventual config.toml removal. GetSearchHosts and GetCrawlerUserID gain a new-model branch: the resolved application's state.toml entry answers first, an empty value falls through to the legacy config.toml lookup while both models coexist. admin_api_key stays excluded as decided on the ticket. Co-Authored-By: Claude Fable 5 * refactor(config): omit empty api_key_uuid from state.toml Legacy migrated keys have no UUID, so without omitempty every migrated entry serialized a noisy api_key_uuid = "". The field is only set by new-model writes (app create/select); omitting the empty value matches search_hosts/crawler_user_id and reads back identically. Co-Authored-By: Claude Fable 5 * feat(application): mark configured apps in select from state.toml The select picker decorated choices from config.toml profiles only, so apps configured under the new model showed as unconfigured. It now marks an app "(configured)" when state.toml holds an entry for it, falling back to legacy config.toml profiles while config.toml is still supported. Adds Config.ApplicationInState for the state-only lookup. Co-Authored-By: Claude Fable 5 * feat(application): state-aware "configured" marker + describe without auth Two follow-ups from auditing the command tree against state.toml: - application list marked apps configured from config.toml profiles only, so new-model apps showed as "(not configured)". It now uses the same state-first/config-fallback check as the select picker, centralized in apputil.ApplicationConfigured. - describe walks the command tree and needs no credentials, but lacked skipAuthCheck so it failed on a machine with nothing configured. It now skips the auth check. Co-Authored-By: Claude Fable 5 * feat(config): clearer error when the current app has no keychain key When state.toml resolves a current application but its key isn't in this machine's keychain (e.g. state.toml synced across machines without it), GetAPIKey/GetCrawlerAPIKey returned the generic "not configured yet". The error now names the application and points to the fix (`application select` / `auth crawler`, or the matching env var). Re-authenticating rewrites the keychain entry, so it restores a working state. Co-Authored-By: Claude Fable 5 * perf(config): short-circuit ShouldMigrate on the state.toml check Check state.toml first: an already-migrated machine (the steady state, hit on every command) now settles in a single stat instead of also stat-ing config.toml. The boolean result is unchanged. Co-Authored-By: Claude Fable 5 * perf(application): O(1) configured-app lookup when listing/selecting Marking apps "(configured)" called ConfiguredProfiles() (a full viper re-parse) once per application — O(apps × profiles) with a heavy constant. Now the config.toml profile app IDs are collected once into a set (ProfileApplicationIDs); the per-app check is two O(1) map lookups (cached state.toml + the set). Both `application list` and `application select` build the set once before their loop. Co-Authored-By: Claude Fable 5 --------- Co-authored-by: Claude Fable 5 --- pkg/cmd/application/list/list.go | 12 +- pkg/cmd/application/selectapp/select.go | 14 +- pkg/cmd/describe/describe.go | 5 + pkg/cmd/root/root.go | 8 + pkg/cmd/shared/apputil/configured.go | 24 ++ pkg/cmd/shared/apputil/configured_test.go | 46 ++++ pkg/config/config.go | 9 + pkg/config/migrate.go | 119 ++++++++ pkg/config/migrate_test.go | 322 ++++++++++++++++++++++ pkg/config/profile.go | 35 ++- pkg/config/resolution_test.go | 70 +++++ pkg/config/state.go | 6 +- test/config.go | 10 +- 13 files changed, 656 insertions(+), 24 deletions(-) create mode 100644 pkg/cmd/shared/apputil/configured.go create mode 100644 pkg/cmd/shared/apputil/configured_test.go create mode 100644 pkg/config/migrate.go create mode 100644 pkg/config/migrate_test.go diff --git a/pkg/cmd/application/list/list.go b/pkg/cmd/application/list/list.go index 3473e877..30cd6978 100644 --- a/pkg/cmd/application/list/list.go +++ b/pkg/cmd/application/list/list.go @@ -104,20 +104,14 @@ func runListCmd(opts *ListOptions) error { return nil } - configuredProfiles := opts.Config.ConfiguredProfiles() - configuredAppIDs := make(map[string]string) - for _, p := range configuredProfiles { - configuredAppIDs[p.ApplicationID] = p.Name - } - fmt.Fprintf(opts.IO.Out, "\nYour applications:\n\n") unconfigured := make([]dashboard.Application, 0) + profileApps := apputil.ProfileApplicationIDs(opts.Config.ConfiguredProfiles()) for _, app := range apps { - profileName, configured := configuredAppIDs[app.ID] label := fmt.Sprintf(" %s %s", app.ID, app.Name) - if configured { - fmt.Fprintf(opts.IO.Out, "%s %s\n", label, cs.Greenf("(configured: %s)", profileName)) + if apputil.ApplicationConfigured(opts.Config, profileApps, app.ID) { + fmt.Fprintf(opts.IO.Out, "%s %s\n", label, cs.Green("(configured)")) } else { fmt.Fprintf(opts.IO.Out, "%s %s\n", label, cs.Gray("(not configured)")) unconfigured = append(unconfigured, app) diff --git a/pkg/cmd/application/selectapp/select.go b/pkg/cmd/application/selectapp/select.go index fa143286..eac0389c 100644 --- a/pkg/cmd/application/selectapp/select.go +++ b/pkg/cmd/application/selectapp/select.go @@ -152,21 +152,15 @@ func pickApplication( return nil, fmt.Errorf("--app-name is required in non-interactive mode") } - configuredProfiles := opts.Config.ConfiguredProfiles() - configuredAppIDs := make(map[string]string) - for _, p := range configuredProfiles { - configuredAppIDs[p.ApplicationID] = p.Name - } - cs := opts.IO.ColorScheme() + profileApps := apputil.ProfileApplicationIDs(opts.Config.ConfiguredProfiles()) appOptions := make([]string, len(apps)) for i, app := range apps { label := fmt.Sprintf("%s (%s)", app.ID, app.Name) - if profileName, ok := configuredAppIDs[app.ID]; ok { - appOptions[i] = fmt.Sprintf("%s %s", label, cs.Greenf("profile: %s", profileName)) - } else { - appOptions[i] = label + if apputil.ApplicationConfigured(opts.Config, profileApps, app.ID) { + label = fmt.Sprintf("%s %s", label, cs.Green("(configured)")) } + appOptions[i] = label } var selected int diff --git a/pkg/cmd/describe/describe.go b/pkg/cmd/describe/describe.go index 974f609d..fab8600a 100644 --- a/pkg/cmd/describe/describe.go +++ b/pkg/cmd/describe/describe.go @@ -38,6 +38,11 @@ func NewDescribeCmd(f *cmdutil.Factory) *cobra.Command { Aliases: []string{"schema"}, Args: cobra.ArbitraryArgs, Short: "Describe commands and flags as JSON.", + // Describe only walks the command tree; it needs no credentials and + // must work on a machine with nothing configured. + Annotations: map[string]string{ + "skipAuthCheck": "true", + }, Long: heredoc.Doc(` Describe the CLI's command tree in a machine-readable format. With no arguments, this command describes the root command. diff --git a/pkg/cmd/root/root.go b/pkg/cmd/root/root.go index 069e7f28..7d2626a1 100644 --- a/pkg/cmd/root/root.go +++ b/pkg/cmd/root/root.go @@ -136,6 +136,14 @@ func Execute() exitCode { cmdFactory := factory.New(version.Version, &cfg) stderr := cmdFactory.IOStreams.ErrOut + // One-time config.toml → state.toml + keychain migration (GROUT-363). Must + // run before credential resolution, which caches state.toml per command. + if cfg.ShouldMigrate() { + if err := cfg.Migrate(); err != nil && hasDebug { + fmt.Fprintf(stderr, "config migration failed (will retry on next run): %s\n", err) + } + } + // Set up the update notifier. updateMessageChan := make(chan *update.ReleaseInfo) go func() { diff --git a/pkg/cmd/shared/apputil/configured.go b/pkg/cmd/shared/apputil/configured.go new file mode 100644 index 00000000..dc6b14c1 --- /dev/null +++ b/pkg/cmd/shared/apputil/configured.go @@ -0,0 +1,24 @@ +package apputil + +import "github.com/algolia/cli/pkg/config" + +// ProfileApplicationIDs returns the set of application IDs backed by a legacy +// config.toml profile. Built once by the caller so a per-application loop tests +// membership in O(1) instead of re-parsing config.toml on every iteration. +func ProfileApplicationIDs(profiles []*config.Profile) map[string]bool { + ids := make(map[string]bool, len(profiles)) + for _, p := range profiles { + if p.ApplicationID != "" { + ids[p.ApplicationID] = true + } + } + return ids +} + +// ApplicationConfigured reports whether an application is already known to the +// CLI. state.toml is the source of truth (an O(1) cached lookup); profileApps +// is the legacy config.toml fallback while config.toml is still supported +// (remove once it's gone). +func ApplicationConfigured(cfg config.IConfig, profileApps map[string]bool, appID string) bool { + return cfg.ApplicationInState(appID) || profileApps[appID] +} diff --git a/pkg/cmd/shared/apputil/configured_test.go b/pkg/cmd/shared/apputil/configured_test.go new file mode 100644 index 00000000..e468d98e --- /dev/null +++ b/pkg/cmd/shared/apputil/configured_test.go @@ -0,0 +1,46 @@ +package apputil + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/algolia/cli/pkg/config" + "github.com/algolia/cli/test" +) + +func TestApplicationConfigured(t *testing.T) { + cfg := test.NewDefaultConfigStub() + cfg.SavedApps = map[string]test.SavedApplication{ + "STATE_APP": {Alias: "prod"}, + } + // "default" is the config.toml profile's application ID (legacy fallback). + profileApps := ProfileApplicationIDs(cfg.ConfiguredProfiles()) + + t.Run("in state.toml", func(t *testing.T) { + assert.True(t, ApplicationConfigured(cfg, profileApps, "STATE_APP")) + }) + + t.Run("only in legacy config.toml", func(t *testing.T) { + assert.True(t, ApplicationConfigured(cfg, profileApps, "default")) + }) + + t.Run("unknown application", func(t *testing.T) { + assert.False(t, ApplicationConfigured(cfg, profileApps, "UNKNOWN")) + }) +} + +func TestProfileApplicationIDs(t *testing.T) { + profiles := []*config.Profile{ + {Name: "prod", ApplicationID: "APP1"}, + {Name: "dev", ApplicationID: "APP2"}, + {Name: "broken", ApplicationID: ""}, // skipped: no app ID + } + + ids := ProfileApplicationIDs(profiles) + + assert.True(t, ids["APP1"]) + assert.True(t, ids["APP2"]) + assert.False(t, ids[""]) // empty IDs never become a member + assert.Len(t, ids, 2) +} diff --git a/pkg/config/config.go b/pkg/config/config.go index d09d4c69..097725f5 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -32,6 +32,7 @@ type IConfig interface { // New model (state.toml + OS keychain). ActiveApplicationID() string + ApplicationInState(appID string) bool ApplicationIDByAlias(alias string) (string, bool) SaveApplication(appID, alias, apiKeyUUID, apiKey string, setCurrent bool) error SetCrawlerAPIKey(appID, crawlerAPIKey string) error @@ -125,6 +126,14 @@ func (c *Config) loadState() *State { return c.state } +// ApplicationInState reports whether state.toml already holds an entry for the +// given application, i.e. the application has been configured under the new +// storage model. +func (c *Config) ApplicationInState(appID string) bool { + _, ok := c.loadState().Applications[appID] + return ok +} + // StateFileExists reports whether state.toml exists on disk, i.e. the new // storage model (state.toml + OS keychain) is already in use on this machine. func (c *Config) StateFileExists() bool { diff --git a/pkg/config/migrate.go b/pkg/config/migrate.go new file mode 100644 index 00000000..022d8c9a --- /dev/null +++ b/pkg/config/migrate.go @@ -0,0 +1,119 @@ +package config + +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: config.toml exists and state.toml +// (only written on success, so doubling as the "migrated" marker) does not. +// +// The state.toml check comes first so an already-migrated machine — the +// steady state, hit on every command — settles in a single stat instead of +// also stat-ing config.toml. +func (c *Config) ShouldMigrate() bool { + if c.File == "" || c.StateFileExists() { + return false + } + _, err := os.Stat(c.File) + return err == nil +} + +// Migrate moves the legacy config.toml profiles into the new model (state.toml +// + OS keychain); config.toml is never modified. Keychain first, state.toml +// last (atomic): a failure leaves state.toml absent, so the migration retries +// on the next run. +func (c *Config) Migrate() error { + // An unparseable config.toml must not mark the migration as done: abort + // before writing state.toml so it retries once the file is fixed. + if err := viper.ReadInConfig(); err != nil { + return err + } + + 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 + } + + state.UpsertApplication(profile.ApplicationID, ApplicationState{ + Alias: profile.Name, + SearchHosts: profile.SearchHosts, + CrawlerUserID: viper.GetString(profile.GetFieldName("crawler_user_id")), + }) + if profile.Default { + state.SetCurrentApplication(profile.ApplicationID) + } + } + + return state.Save(c.StateFile) +} + +// migratableProfiles applies the skip rules: profiles without application_id +// or api_key are dropped, admin_api_key never migrates, and the default +// profile wins when several share an application_id. Name order keeps the +// conflict resolution deterministic (ConfiguredProfiles iterates a map). +func (c *Config) migratableProfiles() []*Profile { + // Decode the profiles here rather than through ConfiguredProfiles, whose + // log.Fatalf on an undecodable entry would brick every command at startup. + configs := viper.AllSettings() + profiles := make([]*Profile, 0, len(configs)) + for name := range configs { + profile := &Profile{Name: name} + if err := viper.UnmarshalKey(name, profile); err != nil { + log.Warnf("config migration: skipping profile %q: %s", name, err) + continue + } + profiles = append(profiles, profile) + } + 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 new file mode 100644 index 00000000..841ac930 --- /dev/null +++ b/pkg/config/migrate_test.go @@ -0,0 +1,322 @@ +package config + +import ( + "os" + "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) { + tests := []struct { + name string + configFile bool + stateFile bool + want bool + }{ + { + name: "config.toml only: migration pending", + configFile: true, + stateFile: false, + want: true, + }, + { + name: "both files: already migrated (or new model in use)", + configFile: true, + stateFile: true, + want: false, + }, + { + name: "state.toml only: nothing to migrate", + configFile: false, + stateFile: true, + want: false, + }, + { + name: "no files: fresh install", + configFile: false, + stateFile: false, + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dir := t.TempDir() + cfg := &Config{ + File: filepath.Join(dir, "config.toml"), + StateFile: filepath.Join(dir, "state.toml"), + } + if tt.configFile { + require.NoError(t, os.WriteFile(cfg.File, []byte(""), 0o600)) + } + if tt.stateFile { + require.NoError(t, os.WriteFile(cfg.StateFile, []byte(""), 0o600)) + } + + assert.Equal(t, tt.want, cfg.ShouldMigrate()) + }) + } +} + +func TestConfig_ShouldMigrate_unresolvedPaths(t *testing.T) { + // InitConfig never ran: paths are empty, the trigger must stay off. + cfg := &Config{} + assert.False(t, cfg.ShouldMigrate()) +} + +// migrationConfig writes a config.toml, 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" +crawler_user_id = "crawler-user" +search_hosts = ["h1.algolia.net", "h2.algolia.net"] +default = true + +[dev] +application_id = "APP2" +api_key = "key-2" +`) + + require.NoError(t, cfg.Migrate()) + + 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) + + 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 + assert.Equal( + t, + []string{"h1.algolia.net", "h2.algolia.net"}, + st.Applications["APP1"].SearchHosts, + ) + assert.Equal(t, "crawler-user", st.Applications["APP1"].CrawlerUserID) + assert.Empty(t, st.Applications["APP2"].SearchHosts) + assert.Empty(t, st.Applications["APP2"].CrawlerUserID) + + 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 re-runs forever. + 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: 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, but the trigger still turns 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()) + + 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()) + + 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()) + + 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) +} + +func TestConfig_Migrate_UndecodableProfileSkipped(t *testing.T) { + keyring.MockInit() + cfg := migrationConfig(t, `telemetry = "off" + +[prod] +application_id = "APP1" +api_key = "key-1" +default = true + +[bad] +application_id = "APP2" +api_key = ["a", "b"] +`) + + // Undecodable entries (root scalar, wrong types) are skipped, not fatal. + require.NoError(t, cfg.Migrate()) + + st, err := LoadState(cfg.StateFile) + require.NoError(t, err) + require.Len(t, st.Applications, 1) + assert.Equal(t, "APP1", st.CurrentApplicationID) + assert.Equal(t, "prod", st.Applications["APP1"].Alias) +} + +func TestConfig_Migrate_UnreadableConfigAborts(t *testing.T) { + keyring.MockInit() + dir := t.TempDir() + configFile := filepath.Join(dir, "config.toml") + require.NoError(t, os.WriteFile(configFile, []byte("not [ valid ### toml\n"), 0o600)) + + viper.Reset() + viper.SetConfigType("toml") + viper.SetConfigFile(configFile) + _ = viper.ReadInConfig() // swallowed, like InitConfig does + t.Cleanup(viper.Reset) + + cfg := &Config{File: configFile, StateFile: filepath.Join(dir, "state.toml")} + + // No state.toml written: the migration retries once the file is fixed. + require.Error(t, cfg.Migrate()) + assert.NoFileExists(t, cfg.StateFile) + assert.True(t, cfg.ShouldMigrate()) +} diff --git a/pkg/config/profile.go b/pkg/config/profile.go index 7cd1c3bc..ba606716 100644 --- a/pkg/config/profile.go +++ b/pkg/config/profile.go @@ -1,6 +1,7 @@ package config import ( + "fmt" "os" "path/filepath" "strings" @@ -94,7 +95,12 @@ func (p *Profile) GetAPIKey() (string, error) { if secrets := p.config.appSecretsFor(appID); secrets != nil && secrets.APIKey != "" { return secrets.APIKey, nil } - return "", ErrAPIKeyNotConfigured + // The application is set but its key isn't in this machine's + // keychain (e.g. state.toml synced across machines without it). + return "", fmt.Errorf( + "no API key stored in your keychain for the current application %q; run `algolia application select` to store one, or set ALGOLIA_API_KEY", + appID, + ) } } @@ -146,6 +152,16 @@ func (p *Profile) GetSearchHosts() []string { return p.SearchHosts } + // New model: hosts recorded for the resolved application. Empty falls + // through to the legacy config.toml lookup while both models coexist. + if p.config != nil { + if appID := p.config.activeApplicationID(); appID != "" { + if hosts := p.config.loadState().Applications[appID].SearchHosts; len(hosts) > 0 { + return hosts + } + } + } + if p.Name == "" { p.LoadDefault() } @@ -170,6 +186,16 @@ func (p *Profile) GetCrawlerUserID() (string, error) { return os.Getenv("ALGOLIA_CRAWLER_USER_ID"), nil } + // New model: the user ID recorded for the resolved application. Empty + // falls through to the legacy config.toml lookup. + if p.config != nil { + if appID := p.config.activeApplicationID(); appID != "" { + if userID := p.config.loadState().Applications[appID].CrawlerUserID; userID != "" { + return userID, nil + } + } + } + if p.Name == "" { p.LoadDefault() } @@ -198,7 +224,12 @@ func (p *Profile) GetCrawlerAPIKey() (string, error) { secrets.CrawlerAPIKey != "" { return secrets.CrawlerAPIKey, nil } - return "", ErrCrawlerAPIKeyNotConfigured + // The application is set but its crawler key isn't in this + // machine's keychain. + return "", fmt.Errorf( + "no Crawler API key stored in your keychain for the current application %q; run `algolia auth crawler` to store one, or set ALGOLIA_CRAWLER_API_KEY", + appID, + ) } } diff --git a/pkg/config/resolution_test.go b/pkg/config/resolution_test.go index 00986fd8..0dd32d51 100644 --- a/pkg/config/resolution_test.go +++ b/pkg/config/resolution_test.go @@ -242,4 +242,74 @@ func TestProfile_GetAPIKey_ActiveAppWithoutKeyErrors(t *testing.T) { // APP1 resolved from state but no keychain key → error, never "legacy-key". _, err := cfg.Profile().GetAPIKey() require.Error(t, err) + // The message names the application and points to a fix, rather than the + // generic "not configured yet". + assert.Contains(t, err.Error(), "APP1") + assert.Contains(t, err.Error(), "application select") +} + +func TestProfile_GetSearchHosts_FromState(t *testing.T) { + path := filepath.Join(t.TempDir(), "state.toml") + require.NoError(t, os.WriteFile(path, []byte( + "current_application_id = \"APP1\"\n\n[applications.APP1]\nalias = \"prod\"\nsearch_hosts = [\"h1\", \"h2\"]\n", + ), 0o600)) + + cfg := &Config{StateFile: path} + cfg.CurrentProfile.config = cfg + + assert.Equal(t, []string{"h1", "h2"}, cfg.Profile().GetSearchHosts()) +} + +func TestProfile_GetSearchHosts_StateEmptyFallsBackToConfigToml(t *testing.T) { + statePath := filepath.Join(t.TempDir(), "state.toml") + require.NoError( + t, + os.WriteFile(statePath, []byte("current_application_id = \"APP1\"\n"), 0o600), + ) + + configFile := filepath.Join(t.TempDir(), "config.toml") + require.NoError(t, os.WriteFile( + configFile, + []byte( + "[legacy]\napplication_id = \"APP1\"\nsearch_hosts = [\"legacy-host\"]\ndefault = true\n", + ), + 0o600, + )) + viper.Reset() + viper.SetConfigType("toml") + viper.SetConfigFile(configFile) + require.NoError(t, viper.ReadInConfig()) + t.Cleanup(viper.Reset) + + cfg := &Config{StateFile: statePath} + cfg.CurrentProfile.config = cfg + + // No hosts in state.toml for APP1: the legacy lookup still answers while + // config.toml exists. + assert.Equal(t, []string{"legacy-host"}, cfg.Profile().GetSearchHosts()) +} + +func TestProfile_GetCrawlerUserID_FromState(t *testing.T) { + path := filepath.Join(t.TempDir(), "state.toml") + require.NoError(t, os.WriteFile(path, []byte( + "current_application_id = \"APP1\"\n\n[applications.APP1]\nalias = \"prod\"\ncrawler_user_id = \"crawler-user\"\n", + ), 0o600)) + + cfg := &Config{StateFile: path} + cfg.CurrentProfile.config = cfg + + userID, err := cfg.Profile().GetCrawlerUserID() + require.NoError(t, err) + assert.Equal(t, "crawler-user", userID) +} + +func TestConfig_ApplicationInState(t *testing.T) { + path := filepath.Join(t.TempDir(), "state.toml") + require.NoError(t, os.WriteFile(path, []byte( + "current_application_id = \"APP1\"\n\n[applications.APP1]\nalias = \"prod\"\n", + ), 0o600)) + + cfg := &Config{StateFile: path} + assert.True(t, cfg.ApplicationInState("APP1")) + assert.False(t, cfg.ApplicationInState("APP2")) } diff --git a/pkg/config/state.go b/pkg/config/state.go index 99d4809d..3b4c059f 100644 --- a/pkg/config/state.go +++ b/pkg/config/state.go @@ -11,8 +11,10 @@ import ( // ApplicationState holds the non-secret, per-application data persisted in // state.toml. Secrets (API keys) live in the OS keychain, not here. type ApplicationState struct { - APIKeyUUID string `toml:"api_key_uuid"` - Alias string `toml:"alias"` + APIKeyUUID string `toml:"api_key_uuid,omitempty"` + Alias string `toml:"alias"` + SearchHosts []string `toml:"search_hosts,omitempty"` + CrawlerUserID string `toml:"crawler_user_id,omitempty"` } // State is the in-memory representation of state.toml, the new source of truth diff --git a/test/config.go b/test/config.go index 0b0828ef..65a77ada 100644 --- a/test/config.go +++ b/test/config.go @@ -147,6 +147,11 @@ func (c *ConfigStub) ActiveApplicationID() string { return c.ActiveAppID } +func (c *ConfigStub) ApplicationInState(appID string) bool { + _, ok := c.SavedApps[appID] + return ok +} + func (c *ConfigStub) ApplicationIDByAlias(alias string) (string, bool) { for appID, app := range c.SavedApps { if app.Alias == alias { @@ -156,7 +161,10 @@ func (c *ConfigStub) ApplicationIDByAlias(alias string) (string, bool) { return "", false } -func (c *ConfigStub) SaveApplication(appID, alias, apiKeyUUID, apiKey string, setCurrent bool) error { +func (c *ConfigStub) SaveApplication( + appID, alias, apiKeyUUID, apiKey string, + setCurrent bool, +) error { if c.SavedApps == nil { c.SavedApps = map[string]SavedApplication{} }