diff --git a/README.md b/README.md index cddd2490..31214eb8 100644 --- a/README.md +++ b/README.md @@ -385,7 +385,36 @@ claws uses your standard AWS configuration: - `~/.aws/config` - AWS configuration (region, profile) - Environment variables: `AWS_PROFILE`, `AWS_REGION`, `AWS_ACCESS_KEY_ID`, etc. -Configuration is stored in `~/.config/claws/config.yaml` for profile preferences. +### Configuration File + +Optional settings can be stored in `~/.config/claws/config.yaml`: + +```yaml +timeouts: + aws_init: 10s # AWS initialization timeout (default: 5s) + multi_region_fetch: 60s # Multi-region parallel fetch timeout (default: 30s) + tag_search: 45s # Tag search timeout (default: 30s) + metrics_load: 30s # CloudWatch metrics load timeout (default: 30s) + +concurrency: + max_fetches: 100 # Max concurrent API fetches (default: 50) + +cloudwatch: + window: 15m # Metrics data window period (default: 15m) + +persistence: + enabled: true # Save region/profile on change (default: false) + +startup: # Applied on launch if present + profile: production + regions: + - us-east-1 + - us-west-2 +``` + +The config file is **not created automatically**. Create it manually if needed. + +CLI flags (`-p`, `-r`, `--persist`, `--no-persist`) override config file settings. For required IAM permissions, see [docs/iam-permissions.md](docs/iam-permissions.md). diff --git a/cmd/claws/main.go b/cmd/claws/main.go index 4b1a468b..d9c18b49 100644 --- a/cmd/claws/main.go +++ b/cmd/claws/main.go @@ -23,9 +23,14 @@ func main() { propagateAllProxy() - // Apply CLI options to global config + fileCfg := config.File() cfg := config.Global() + // CLI persistence flags override config file + if opts.persist != nil { + fileCfg.SetPersistenceEnabled(*opts.persist) + } + // Check environment variables (CLI flags take precedence) if !opts.readOnly { if v := os.Getenv("CLAWS_READ_ONLY"); v == "1" || v == "true" { @@ -45,21 +50,7 @@ func main() { os.Exit(1) } - if opts.envCreds { - // Use environment credentials, ignore ~/.aws config - cfg.UseEnvOnly() - } else if opts.profile != "" { - cfg.UseProfile(opts.profile) - // Don't set AWS_PROFILE globally - it interferes with EnvOnly mode - // when switching profiles. SelectionLoadOptions uses WithSharedConfigProfile - // for SDK calls, and BuildSubprocessEnv handles subprocess environment. - } - // else: SDKDefault is the zero value, no action needed - if opts.region != "" { - cfg.SetRegion(opts.region) - // Don't set AWS_REGION globally - SelectionLoadOptions handles SDK calls, - // and BuildSubprocessEnv handles subprocess environment. - } + applyStartupConfig(opts, fileCfg, cfg) // Enable logging if log file specified if opts.logFile != "" { @@ -86,12 +77,12 @@ func main() { } } -// cliOptions holds command line options type cliOptions struct { profile string region string readOnly bool envCreds bool + persist *bool // nil = use config, true = enable, false = disable logFile string } @@ -118,6 +109,12 @@ func parseFlags() cliOptions { opts.readOnly = true case "-e", "--env": opts.envCreds = true + case "--persist": + t := true + opts.persist = &t + case "--no-persist": + f := false + opts.persist = &f case "-l", "--log-file": if i+1 < len(args) { i++ @@ -158,6 +155,10 @@ func printUsage() { fmt.Println(" Useful for instance profiles, ECS task roles, Lambda, etc.") fmt.Println(" -ro, --read-only") fmt.Println(" Run in read-only mode (disable dangerous actions)") + fmt.Println(" --persist") + fmt.Println(" Enable saving region/profile selection to config file") + fmt.Println(" --no-persist") + fmt.Println(" Disable saving region/profile selection to config file") fmt.Println(" -l, --log-file ") fmt.Println(" Enable debug logging to specified file") fmt.Println(" -v, --version") @@ -170,6 +171,30 @@ func printUsage() { fmt.Println(" ALL_PROXY Propagated to HTTP_PROXY/HTTPS_PROXY if not set") } +// applyStartupConfig applies profile/region config with precedence: +// 1. CLI flags (-p, -r, -e) - highest priority +// 2. Config file startup section +// 3. AWS SDK defaults +func applyStartupConfig(opts cliOptions, fileCfg *config.FileConfig, cfg *config.Config) { + startupRegions, startupProfile := fileCfg.GetStartup() + + // Apply profile: CLI > startup config + if opts.envCreds { + cfg.UseEnvOnly() + } else if opts.profile != "" { + cfg.UseProfile(opts.profile) + } else if startupProfile != "" { + cfg.UseProfile(startupProfile) + } + + // Apply region: CLI > startup config + if opts.region != "" { + cfg.SetRegion(opts.region) + } else if len(startupRegions) > 0 { + cfg.SetRegions(startupRegions) + } +} + // propagateAllProxy copies ALL_PROXY to HTTP_PROXY/HTTPS_PROXY if not set. // Go's net/http ignores ALL_PROXY, so we propagate it to the standard vars. func propagateAllProxy() { diff --git a/go.mod b/go.mod index 6be88d7e..302ee619 100644 --- a/go.mod +++ b/go.mod @@ -86,6 +86,7 @@ require ( golang.org/x/sync v0.19.0 golang.org/x/term v0.38.0 gopkg.in/ini.v1 v1.67.0 + gopkg.in/yaml.v3 v3.0.1 ) require ( diff --git a/go.sum b/go.sum index 6cd4337d..8b157de5 100644 --- a/go.sum +++ b/go.sum @@ -232,6 +232,8 @@ golang.org/x/sys v0.39.0 h1:CvCKL8MeisomCi6qNZ+wbb0DN9E5AATixKsvNtMoMFk= golang.org/x/sys v0.39.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks= golang.org/x/term v0.38.0 h1:PQ5pkm/rLO6HnxFR7N2lJHOZX6Kez5Y1gDSJla6jo7Q= golang.org/x/term v0.38.0/go.mod h1:bSEAKrOT1W+VSu9TSCMtoGEOUcKxOKgl3LE5QEF/xVg= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/ini.v1 v1.67.0 h1:Dgnx+6+nfE+IfzjUEISNeydPJh9AXNNsWbGP9KzCsOA= gopkg.in/ini.v1 v1.67.0/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= diff --git a/internal/app/app.go b/internal/app/app.go index 844e02c9..4b3a7932 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -19,9 +19,6 @@ import ( "github.com/clawscli/claws/internal/view" ) -// awsInitTimeout is the maximum time to wait for AWS context initialization -const awsInitTimeout = 5 * time.Second - // clearErrorMsg is sent to clear transient errors after a timeout type clearErrorMsg struct{} @@ -113,7 +110,7 @@ func (a *App) Init() tea.Cmd { // Initialize AWS context in background (region detection, account ID fetch) // Use timeout to avoid indefinite hang on network issues initAWSCmd := func() tea.Msg { - ctx, cancel := context.WithTimeout(a.ctx, awsInitTimeout) + ctx, cancel := context.WithTimeout(a.ctx, config.File().AWSInitTimeout()) defer cancel() err := aws.InitContext(ctx) return awsContextReadyMsg{err: err} @@ -336,6 +333,16 @@ func (a *App) Update(msg tea.Msg) (tea.Model, tea.Cmd) { case navmsg.RegionChangedMsg: log.Info("regions changed", "regions", msg.Regions) + if config.File().PersistenceEnabled() { + profile := "" + if sel := config.Global().Selection(); sel.IsNamedProfile() { + profile = sel.ProfileName + } + config.File().SetStartup(msg.Regions, profile) + if err := config.File().Save(); err != nil { + log.Warn("failed to persist config", "error", err) + } + } // Pop views until we find a refreshable one (ResourceBrowser or ServiceBrowser) for len(a.viewStack) > 0 { a.currentView = a.viewStack[len(a.viewStack)-1] @@ -356,12 +363,23 @@ func (a *App) Update(msg tea.Msg) (tea.Model, tea.Cmd) { case navmsg.ProfilesChangedMsg: log.Info("profiles changed", "count", len(msg.Selections)) + if config.File().PersistenceEnabled() { + profile := "" + if len(msg.Selections) > 0 && msg.Selections[0].IsNamedProfile() { + profile = msg.Selections[0].ProfileName + } + regions := config.Global().Regions() + config.File().SetStartup(regions, profile) + if err := config.File().Save(); err != nil { + log.Warn("failed to persist config", "error", err) + } + } a.profileRefreshID++ a.profileRefreshing = true a.profileRefreshError = nil refreshID := a.profileRefreshID refreshCmd := func() tea.Msg { - ctx, cancel := context.WithTimeout(a.ctx, awsInitTimeout) + ctx, cancel := context.WithTimeout(a.ctx, config.File().AWSInitTimeout()) defer cancel() region, accountIDs, err := aws.RefreshContextData(ctx) return profileRefreshDoneMsg{ diff --git a/internal/aws/init.go b/internal/aws/init.go index 565bf2e4..4b2259f8 100644 --- a/internal/aws/init.go +++ b/internal/aws/init.go @@ -9,10 +9,6 @@ import ( appconfig "github.com/clawscli/claws/internal/config" ) -// maxConcurrentProfileFetches limits parallel AWS config loads to prevent -// file descriptor exhaustion and excessive memory usage with many profiles. -const maxConcurrentProfileFetches = 50 - // InitContext initializes AWS context by loading config and fetching account ID. // Updates the global config with region (if not already set) and account ID. func InitContext(ctx context.Context) error { @@ -37,7 +33,7 @@ func InitContext(ctx context.Context) error { // RefreshContextData re-fetches region and account ID for the current profile selection(s). // Returns the data without modifying global state, allowing the caller to apply changes. -// Fetches up to 50 profiles concurrently. Returns partial results and first error on failure. +// Concurrency is limited by config.File().MaxConcurrentFetches(). Returns partial results and first error on failure. func RefreshContextData(ctx context.Context) (region string, accountIDs map[string]string, err error) { selections := appconfig.Global().Selections() if len(selections) == 0 { @@ -56,7 +52,7 @@ func RefreshContextData(ctx context.Context) (region string, accountIDs map[stri accountIDs = make(map[string]string) var mu sync.Mutex errChan := make(chan error, len(selections)) - sem := make(chan struct{}, maxConcurrentProfileFetches) + sem := make(chan struct{}, appconfig.File().MaxConcurrentFetches()) for _, sel := range selections { wg.Add(1) diff --git a/internal/config/config.go b/internal/config/config.go index b4c20fa1..d158e08a 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -174,7 +174,6 @@ func (s ProfileSelection) ID() string { } } -// Config holds global application configuration type Config struct { mu sync.RWMutex regions []string @@ -189,18 +188,6 @@ var ( initOnce sync.Once ) -func withRLock[T any](mu *sync.RWMutex, fn func() T) T { - mu.RLock() - defer mu.RUnlock() - return fn() -} - -func doWithLock(mu *sync.RWMutex, fn func()) { - mu.Lock() - defer mu.Unlock() - fn() -} - // Global returns the global config instance func Global() *Config { initOnce.Do(func() { diff --git a/internal/config/file.go b/internal/config/file.go new file mode 100644 index 00000000..00e84f6a --- /dev/null +++ b/internal/config/file.go @@ -0,0 +1,318 @@ +package config + +import ( + "errors" + "fmt" + "os" + "path/filepath" + "sync" + "time" + + "gopkg.in/yaml.v3" +) + +const ( + DefaultAWSInitTimeout = 5 * time.Second + DefaultMultiRegionFetchTimeout = 30 * time.Second + DefaultTagSearchTimeout = 30 * time.Second + DefaultMetricsLoadTimeout = 30 * time.Second + DefaultMetricsWindow = 15 * time.Minute + DefaultMaxConcurrentFetches = 50 +) + +func ConfigDir() (string, error) { + home, err := os.UserHomeDir() + if err != nil { + return "", fmt.Errorf("get home dir: %w", err) + } + return filepath.Join(home, ".config", "claws"), nil +} + +func ConfigPath() (string, error) { + dir, err := ConfigDir() + if err != nil { + return "", err + } + return filepath.Join(dir, "config.yaml"), nil +} + +type TimeoutConfig struct { + AWSInit Duration `yaml:"aws_init,omitempty"` + MultiRegionFetch Duration `yaml:"multi_region_fetch,omitempty"` + TagSearch Duration `yaml:"tag_search,omitempty"` + MetricsLoad Duration `yaml:"metrics_load,omitempty"` +} + +type CloudWatchConfig struct { + Window Duration `yaml:"window,omitempty"` +} + +type ConcurrencyConfig struct { + MaxFetches int `yaml:"max_fetches,omitempty"` +} + +type PersistenceConfig struct { + Enabled bool `yaml:"enabled"` +} + +type StartupConfig struct { + Regions []string `yaml:"regions,omitempty"` + Profile string `yaml:"profile,omitempty"` +} + +type FileConfig struct { + mu sync.RWMutex `yaml:"-"` + persistenceOverride *bool `yaml:"-"` // CLI flag override (not persisted) + Timeouts TimeoutConfig `yaml:"timeouts,omitempty"` + Concurrency ConcurrencyConfig `yaml:"concurrency,omitempty"` + CloudWatch CloudWatchConfig `yaml:"cloudwatch,omitempty"` + Persistence PersistenceConfig `yaml:"persistence"` + Startup StartupConfig `yaml:"startup,omitempty"` +} + +// Duration wraps time.Duration for YAML marshal/unmarshal as string (e.g., "5s", "30s") +type Duration time.Duration + +func (d Duration) Duration() time.Duration { + return time.Duration(d) +} + +func (d Duration) MarshalYAML() (interface{}, error) { + return time.Duration(d).String(), nil +} + +func (d *Duration) UnmarshalYAML(node *yaml.Node) error { + var s string + if err := node.Decode(&s); err != nil { + return err + } + if s == "" { + *d = 0 + return nil + } + dur, err := time.ParseDuration(s) + if err != nil { + return fmt.Errorf("invalid duration %q: %w", s, err) + } + *d = Duration(dur) + return nil +} + +func DefaultFileConfig() *FileConfig { + return &FileConfig{ + Timeouts: TimeoutConfig{ + AWSInit: Duration(DefaultAWSInitTimeout), + MultiRegionFetch: Duration(DefaultMultiRegionFetchTimeout), + TagSearch: Duration(DefaultTagSearchTimeout), + MetricsLoad: Duration(DefaultMetricsLoadTimeout), + }, + Concurrency: ConcurrencyConfig{ + MaxFetches: DefaultMaxConcurrentFetches, + }, + CloudWatch: CloudWatchConfig{ + Window: Duration(DefaultMetricsWindow), + }, + Persistence: PersistenceConfig{ + Enabled: false, + }, + } +} + +var ( + fileConfig *FileConfig + fileConfigOnce sync.Once +) + +func File() *FileConfig { + fileConfigOnce.Do(func() { + cfg, err := Load() + if err != nil { + cfg = DefaultFileConfig() + } + fileConfig = cfg + }) + return fileConfig +} + +func Load() (*FileConfig, error) { + path, err := ConfigPath() + if err != nil { + return DefaultFileConfig(), nil + } + + data, err := os.ReadFile(path) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + return DefaultFileConfig(), nil + } + return nil, fmt.Errorf("read config: %w", err) + } + + cfg := DefaultFileConfig() + if err := yaml.Unmarshal(data, cfg); err != nil { + return nil, fmt.Errorf("parse config: %w", err) + } + + cfg.applyDefaults() + return cfg, nil +} + +func (c *FileConfig) Save() error { + path, err := ConfigPath() + if err != nil { + return err + } + + dir := filepath.Dir(path) + if err := os.MkdirAll(dir, 0755); err != nil { + return fmt.Errorf("create config dir: %w", err) + } + + snapshot := withRLock(&c.mu, func() FileConfig { + return FileConfig{ + Timeouts: c.Timeouts, + Concurrency: c.Concurrency, + CloudWatch: c.CloudWatch, + Persistence: c.Persistence, + Startup: StartupConfig{ + Regions: append([]string(nil), c.Startup.Regions...), + Profile: c.Startup.Profile, + }, + } + }) + + data, err := yaml.Marshal(&snapshot) + if err != nil { + return fmt.Errorf("marshal config: %w", err) + } + + // Atomic write: write to temp file, then rename + tmpFile, err := os.CreateTemp(dir, ".config.yaml.tmp.*") + if err != nil { + return fmt.Errorf("create temp file: %w", err) + } + tmpPath := tmpFile.Name() + + if _, err := tmpFile.Write(data); err != nil { + _ = tmpFile.Close() + _ = os.Remove(tmpPath) + return fmt.Errorf("write temp file: %w", err) + } + if err := tmpFile.Close(); err != nil { + _ = os.Remove(tmpPath) + return fmt.Errorf("close temp file: %w", err) + } + + if err := os.Rename(tmpPath, path); err != nil { + _ = os.Remove(tmpPath) + return fmt.Errorf("rename config file: %w", err) + } + + return nil +} + +func (c *FileConfig) applyDefaults() { + if c.Timeouts.AWSInit <= 0 { + c.Timeouts.AWSInit = Duration(DefaultAWSInitTimeout) + } + if c.Timeouts.MultiRegionFetch <= 0 { + c.Timeouts.MultiRegionFetch = Duration(DefaultMultiRegionFetchTimeout) + } + if c.Timeouts.TagSearch <= 0 { + c.Timeouts.TagSearch = Duration(DefaultTagSearchTimeout) + } + if c.Timeouts.MetricsLoad <= 0 { + c.Timeouts.MetricsLoad = Duration(DefaultMetricsLoadTimeout) + } + if c.CloudWatch.Window <= 0 { + c.CloudWatch.Window = Duration(DefaultMetricsWindow) + } + if c.Concurrency.MaxFetches <= 0 { + c.Concurrency.MaxFetches = DefaultMaxConcurrentFetches + } +} + +func (c *FileConfig) AWSInitTimeout() time.Duration { + return withRLock(&c.mu, func() time.Duration { + if c.Timeouts.AWSInit == 0 { + return DefaultAWSInitTimeout + } + return c.Timeouts.AWSInit.Duration() + }) +} + +func (c *FileConfig) MultiRegionFetchTimeout() time.Duration { + return withRLock(&c.mu, func() time.Duration { + if c.Timeouts.MultiRegionFetch == 0 { + return DefaultMultiRegionFetchTimeout + } + return c.Timeouts.MultiRegionFetch.Duration() + }) +} + +func (c *FileConfig) TagSearchTimeout() time.Duration { + return withRLock(&c.mu, func() time.Duration { + if c.Timeouts.TagSearch == 0 { + return DefaultTagSearchTimeout + } + return c.Timeouts.TagSearch.Duration() + }) +} + +func (c *FileConfig) MetricsLoadTimeout() time.Duration { + return withRLock(&c.mu, func() time.Duration { + if c.Timeouts.MetricsLoad == 0 { + return DefaultMetricsLoadTimeout + } + return c.Timeouts.MetricsLoad.Duration() + }) +} + +func (c *FileConfig) MaxConcurrentFetches() int { + return withRLock(&c.mu, func() int { + if c.Concurrency.MaxFetches == 0 { + return DefaultMaxConcurrentFetches + } + return c.Concurrency.MaxFetches + }) +} + +func (c *FileConfig) MetricsWindow() time.Duration { + return withRLock(&c.mu, func() time.Duration { + if c.CloudWatch.Window == 0 { + return DefaultMetricsWindow + } + return c.CloudWatch.Window.Duration() + }) +} + +func (c *FileConfig) PersistenceEnabled() bool { + return withRLock(&c.mu, func() bool { + if c.persistenceOverride != nil { + return *c.persistenceOverride + } + return c.Persistence.Enabled + }) +} + +func (c *FileConfig) SetPersistenceEnabled(enabled bool) { + doWithLock(&c.mu, func() { c.persistenceOverride = &enabled }) +} + +func (c *FileConfig) SetStartup(regions []string, profile string) { + doWithLock(&c.mu, func() { + c.Startup.Regions = regions + c.Startup.Profile = profile + }) +} + +func (c *FileConfig) GetStartup() ([]string, string) { + type result struct { + regions []string + profile string + } + r := withRLock(&c.mu, func() result { + return result{append([]string(nil), c.Startup.Regions...), c.Startup.Profile} + }) + return r.regions, r.profile +} diff --git a/internal/config/file_test.go b/internal/config/file_test.go new file mode 100644 index 00000000..8a0043d8 --- /dev/null +++ b/internal/config/file_test.go @@ -0,0 +1,284 @@ +package config + +import ( + "os" + "path/filepath" + "testing" + "time" + + "gopkg.in/yaml.v3" +) + +func TestDuration_MarshalUnmarshal(t *testing.T) { + tests := []struct { + name string + duration Duration + want string + }{ + {"5s", Duration(5 * time.Second), "5s"}, + {"30s", Duration(30 * time.Second), "30s"}, + {"1m", Duration(1 * time.Minute), "1m0s"}, + {"zero", Duration(0), "0s"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Marshal + data, err := yaml.Marshal(tt.duration) + if err != nil { + t.Fatalf("Marshal failed: %v", err) + } + got := string(data) + // yaml.Marshal adds newline + if got != tt.want+"\n" { + t.Errorf("Marshal = %q, want %q", got, tt.want+"\n") + } + + // Unmarshal + var d Duration + if err := yaml.Unmarshal([]byte(tt.want), &d); err != nil { + t.Fatalf("Unmarshal failed: %v", err) + } + if d != tt.duration { + t.Errorf("Unmarshal = %v, want %v", d, tt.duration) + } + }) + } +} + +func TestDuration_UnmarshalEmpty(t *testing.T) { + var d Duration + if err := yaml.Unmarshal([]byte(`""`), &d); err != nil { + t.Fatalf("Unmarshal empty failed: %v", err) + } + if d != 0 { + t.Errorf("Unmarshal empty = %v, want 0", d) + } +} + +func TestDuration_UnmarshalInvalid(t *testing.T) { + var d Duration + err := yaml.Unmarshal([]byte(`"invalid"`), &d) + if err == nil { + t.Error("Unmarshal invalid should fail") + } +} + +func TestDefaultFileConfig(t *testing.T) { + cfg := DefaultFileConfig() + + if cfg.Timeouts.AWSInit.Duration() != DefaultAWSInitTimeout { + t.Errorf("AWSInit = %v, want %v", cfg.Timeouts.AWSInit.Duration(), DefaultAWSInitTimeout) + } + if cfg.Timeouts.MultiRegionFetch.Duration() != DefaultMultiRegionFetchTimeout { + t.Errorf("MultiRegionFetch = %v, want %v", cfg.Timeouts.MultiRegionFetch.Duration(), DefaultMultiRegionFetchTimeout) + } + if cfg.Timeouts.TagSearch.Duration() != DefaultTagSearchTimeout { + t.Errorf("TagSearch = %v, want %v", cfg.Timeouts.TagSearch.Duration(), DefaultTagSearchTimeout) + } + if cfg.Timeouts.MetricsLoad.Duration() != DefaultMetricsLoadTimeout { + t.Errorf("MetricsLoad = %v, want %v", cfg.Timeouts.MetricsLoad.Duration(), DefaultMetricsLoadTimeout) + } + if cfg.Concurrency.MaxFetches != DefaultMaxConcurrentFetches { + t.Errorf("MaxFetches = %d, want %d", cfg.Concurrency.MaxFetches, DefaultMaxConcurrentFetches) + } + if cfg.Persistence.Enabled { + t.Error("Persistence.Enabled should be false by default") + } +} + +func TestLoad_MissingFile(t *testing.T) { + // Use a temp dir that doesn't have config.yaml + tmpDir := t.TempDir() + origHome := os.Getenv("HOME") + defer os.Setenv("HOME", origHome) + os.Setenv("HOME", tmpDir) + + cfg, err := Load() + if err != nil { + t.Fatalf("Load failed: %v", err) + } + + // Should return defaults + if cfg.AWSInitTimeout() != DefaultAWSInitTimeout { + t.Errorf("AWSInitTimeout() = %v, want %v", cfg.AWSInitTimeout(), DefaultAWSInitTimeout) + } +} + +func TestLoad_Save_Roundtrip(t *testing.T) { + tmpDir := t.TempDir() + origHome := os.Getenv("HOME") + defer os.Setenv("HOME", origHome) + os.Setenv("HOME", tmpDir) + + // Create config with custom values + cfg := &FileConfig{ + Timeouts: TimeoutConfig{ + AWSInit: Duration(10 * time.Second), + MultiRegionFetch: Duration(60 * time.Second), + TagSearch: Duration(45 * time.Second), + MetricsLoad: Duration(20 * time.Second), + }, + Concurrency: ConcurrencyConfig{ + MaxFetches: 100, + }, + Persistence: PersistenceConfig{ + Enabled: true, + }, + Startup: StartupConfig{ + Regions: []string{"us-east-1", "us-west-2"}, + Profile: "production", + }, + } + + // Save + if err := cfg.Save(); err != nil { + t.Fatalf("Save failed: %v", err) + } + + // Verify file exists + configPath := filepath.Join(tmpDir, ".config", "claws", "config.yaml") + if _, err := os.Stat(configPath); os.IsNotExist(err) { + t.Fatal("config file was not created") + } + + // Load and verify + loaded, err := Load() + if err != nil { + t.Fatalf("Load failed: %v", err) + } + + if loaded.AWSInitTimeout() != 10*time.Second { + t.Errorf("AWSInitTimeout() = %v, want %v", loaded.AWSInitTimeout(), 10*time.Second) + } + if loaded.MultiRegionFetchTimeout() != 60*time.Second { + t.Errorf("MultiRegionFetchTimeout() = %v, want %v", loaded.MultiRegionFetchTimeout(), 60*time.Second) + } + if loaded.TagSearchTimeout() != 45*time.Second { + t.Errorf("TagSearchTimeout() = %v, want %v", loaded.TagSearchTimeout(), 45*time.Second) + } + if loaded.MetricsLoadTimeout() != 20*time.Second { + t.Errorf("MetricsLoadTimeout() = %v, want %v", loaded.MetricsLoadTimeout(), 20*time.Second) + } + if loaded.MaxConcurrentFetches() != 100 { + t.Errorf("MaxConcurrentFetches() = %d, want %d", loaded.MaxConcurrentFetches(), 100) + } + if !loaded.PersistenceEnabled() { + t.Error("PersistenceEnabled() should be true") + } + + regions, profile := loaded.GetStartup() + if len(regions) != 2 || regions[0] != "us-east-1" || regions[1] != "us-west-2" { + t.Errorf("GetStartup() regions = %v, want [us-east-1, us-west-2]", regions) + } + if profile != "production" { + t.Errorf("GetStartup() profile = %q, want %q", profile, "production") + } +} + +func TestFileConfig_ApplyDefaults(t *testing.T) { + cfg := &FileConfig{} + cfg.applyDefaults() + + if cfg.Timeouts.AWSInit.Duration() != DefaultAWSInitTimeout { + t.Errorf("AWSInit = %v, want %v", cfg.Timeouts.AWSInit.Duration(), DefaultAWSInitTimeout) + } + if cfg.Concurrency.MaxFetches != DefaultMaxConcurrentFetches { + t.Errorf("MaxFetches = %d, want %d", cfg.Concurrency.MaxFetches, DefaultMaxConcurrentFetches) + } +} + +func TestFileConfig_ApplyDefaults_NegativeValues(t *testing.T) { + cfg := &FileConfig{ + Timeouts: TimeoutConfig{ + AWSInit: Duration(-5 * time.Second), + MultiRegionFetch: Duration(-1 * time.Minute), + }, + Concurrency: ConcurrencyConfig{ + MaxFetches: -10, + }, + } + cfg.applyDefaults() + + if cfg.Timeouts.AWSInit.Duration() != DefaultAWSInitTimeout { + t.Errorf("negative AWSInit should default, got %v", cfg.Timeouts.AWSInit.Duration()) + } + if cfg.Timeouts.MultiRegionFetch.Duration() != DefaultMultiRegionFetchTimeout { + t.Errorf("negative MultiRegionFetch should default, got %v", cfg.Timeouts.MultiRegionFetch.Duration()) + } + if cfg.Concurrency.MaxFetches != DefaultMaxConcurrentFetches { + t.Errorf("negative MaxFetches should default, got %d", cfg.Concurrency.MaxFetches) + } +} + +func TestFileConfig_SetStartup(t *testing.T) { + cfg := &FileConfig{} + + cfg.SetStartup([]string{"eu-west-1"}, "dev") + + regions, profile := cfg.GetStartup() + if len(regions) != 1 || regions[0] != "eu-west-1" { + t.Errorf("GetStartup() regions = %v, want [eu-west-1]", regions) + } + if profile != "dev" { + t.Errorf("GetStartup() profile = %q, want %q", profile, "dev") + } +} + +func TestFileConfig_Getters_ZeroValues(t *testing.T) { + cfg := &FileConfig{} + + // Getters should return defaults when values are zero + if cfg.AWSInitTimeout() != DefaultAWSInitTimeout { + t.Errorf("AWSInitTimeout() = %v, want %v", cfg.AWSInitTimeout(), DefaultAWSInitTimeout) + } + if cfg.MultiRegionFetchTimeout() != DefaultMultiRegionFetchTimeout { + t.Errorf("MultiRegionFetchTimeout() = %v, want %v", cfg.MultiRegionFetchTimeout(), DefaultMultiRegionFetchTimeout) + } + if cfg.TagSearchTimeout() != DefaultTagSearchTimeout { + t.Errorf("TagSearchTimeout() = %v, want %v", cfg.TagSearchTimeout(), DefaultTagSearchTimeout) + } + if cfg.MetricsLoadTimeout() != DefaultMetricsLoadTimeout { + t.Errorf("MetricsLoadTimeout() = %v, want %v", cfg.MetricsLoadTimeout(), DefaultMetricsLoadTimeout) + } + if cfg.MaxConcurrentFetches() != DefaultMaxConcurrentFetches { + t.Errorf("MaxConcurrentFetches() = %d, want %d", cfg.MaxConcurrentFetches(), DefaultMaxConcurrentFetches) + } +} + +func TestLoad_PartialConfig(t *testing.T) { + tmpDir := t.TempDir() + origHome := os.Getenv("HOME") + defer os.Setenv("HOME", origHome) + os.Setenv("HOME", tmpDir) + + // Create config dir + configDir := filepath.Join(tmpDir, ".config", "claws") + if err := os.MkdirAll(configDir, 0755); err != nil { + t.Fatalf("MkdirAll failed: %v", err) + } + + // Write partial config (only timeouts.aws_init) + configPath := filepath.Join(configDir, "config.yaml") + data := []byte("timeouts:\n aws_init: 15s\n") + if err := os.WriteFile(configPath, data, 0644); err != nil { + t.Fatalf("WriteFile failed: %v", err) + } + + // Load should fill in defaults for missing values + cfg, err := Load() + if err != nil { + t.Fatalf("Load failed: %v", err) + } + + if cfg.AWSInitTimeout() != 15*time.Second { + t.Errorf("AWSInitTimeout() = %v, want %v", cfg.AWSInitTimeout(), 15*time.Second) + } + // Other values should be defaults + if cfg.MultiRegionFetchTimeout() != DefaultMultiRegionFetchTimeout { + t.Errorf("MultiRegionFetchTimeout() = %v, want %v", cfg.MultiRegionFetchTimeout(), DefaultMultiRegionFetchTimeout) + } + if cfg.MaxConcurrentFetches() != DefaultMaxConcurrentFetches { + t.Errorf("MaxConcurrentFetches() = %d, want %d", cfg.MaxConcurrentFetches(), DefaultMaxConcurrentFetches) + } +} diff --git a/internal/config/lock.go b/internal/config/lock.go new file mode 100644 index 00000000..182132ca --- /dev/null +++ b/internal/config/lock.go @@ -0,0 +1,15 @@ +package config + +import "sync" + +func withRLock[T any](mu *sync.RWMutex, fn func() T) T { + mu.RLock() + defer mu.RUnlock() + return fn() +} + +func doWithLock(mu *sync.RWMutex, fn func()) { + mu.Lock() + defer mu.Unlock() + fn() +} diff --git a/internal/metrics/cloudwatch.go b/internal/metrics/cloudwatch.go index f519f787..63f4ca5b 100644 --- a/internal/metrics/cloudwatch.go +++ b/internal/metrics/cloudwatch.go @@ -11,12 +11,12 @@ import ( "github.com/aws/aws-sdk-go-v2/service/cloudwatch/types" appaws "github.com/clawscli/claws/internal/aws" + "github.com/clawscli/claws/internal/config" "github.com/clawscli/claws/internal/render" ) const ( metricPeriod = 60 - metricWindow = 15 * time.Minute maxQueriesPerRequest = 500 ) @@ -39,7 +39,7 @@ func (f *Fetcher) Fetch(ctx context.Context, resourceIDs []string, spec *render. queries := f.buildQueries(resourceIDs, spec) endTime := time.Now().Truncate(time.Minute) - startTime := endTime.Add(-metricWindow) + startTime := endTime.Add(-config.File().MetricsWindow()) data := NewMetricData(spec) diff --git a/internal/view/resource_browser.go b/internal/view/resource_browser.go index 4bc16449..609fbad3 100644 --- a/internal/view/resource_browser.go +++ b/internal/view/resource_browser.go @@ -24,10 +24,7 @@ import ( // ResourceBrowser displays resources of a specific type -const ( - logTokenMaxLen = 20 - metricsLoadTimeout = 30 * time.Second -) +const logTokenMaxLen = 20 // resourceBrowserStyles holds cached lipgloss styles for performance type resourceBrowserStyles struct { diff --git a/internal/view/resource_browser_fetch.go b/internal/view/resource_browser_fetch.go index 2afe9307..f3375d11 100644 --- a/internal/view/resource_browser_fetch.go +++ b/internal/view/resource_browser_fetch.go @@ -16,11 +16,6 @@ import ( "github.com/clawscli/claws/internal/render" ) -const ( - multiRegionFetchTimeout = 30 * time.Second - maxConcurrentFetches = 50 // TODO: make configurable via config file -) - type listResourcesResult struct { resources []dao.Resource nextToken string @@ -72,11 +67,11 @@ func fetchParallel[K comparable]( fetch func(context.Context, K) ([]dao.Resource, string, error), formatError func(K, error) string, ) parallelFetchResult[K] { - ctx, cancel := context.WithTimeout(ctx, multiRegionFetchTimeout) + ctx, cancel := context.WithTimeout(ctx, config.File().MultiRegionFetchTimeout()) defer cancel() results := make(chan parallelFetchItem[K], len(keys)) - sem := make(chan struct{}, maxConcurrentFetches) + sem := make(chan struct{}, config.File().MaxConcurrentFetches()) var wg sync.WaitGroup for _, key := range keys { diff --git a/internal/view/resource_browser_metrics.go b/internal/view/resource_browser_metrics.go index 45e07391..eb797389 100644 --- a/internal/view/resource_browser_metrics.go +++ b/internal/view/resource_browser_metrics.go @@ -6,6 +6,7 @@ import ( tea "charm.land/bubbletea/v2" "github.com/clawscli/claws/internal/aws" + "github.com/clawscli/claws/internal/config" "github.com/clawscli/claws/internal/dao" "github.com/clawscli/claws/internal/metrics" "github.com/clawscli/claws/internal/render" @@ -44,7 +45,7 @@ func (r *ResourceBrowser) loadMetricsCmd() tea.Cmd { return nil } - ctx, cancel := context.WithTimeout(baseCtx, metricsLoadTimeout) + ctx, cancel := context.WithTimeout(baseCtx, config.File().MetricsLoadTimeout()) defer cancel() byRegion := make(map[string][]resourceInfo) diff --git a/internal/view/tag_search_view.go b/internal/view/tag_search_view.go index bd14a9db..fba34a59 100644 --- a/internal/view/tag_search_view.go +++ b/internal/view/tag_search_view.go @@ -6,7 +6,6 @@ import ( "sort" "strings" "sync" - "time" "charm.land/bubbles/v2/spinner" "charm.land/bubbles/v2/table" @@ -24,10 +23,7 @@ import ( "github.com/clawscli/claws/internal/ui" ) -const ( - tagSearchTimeout = 30 * time.Second - tagSearchLimit = 100 // AWS Resource Groups Tagging API max per request -) +const tagSearchLimit = 100 type taggedARN struct { ARN *aws.ARN @@ -131,7 +127,7 @@ func (v *TagSearchView) fetchTaggedResources(regions []string, existingTokens ma err error } - ctx, cancel := context.WithTimeout(v.ctx, tagSearchTimeout) + ctx, cancel := context.WithTimeout(v.ctx, config.File().TagSearchTimeout()) defer cancel() results := make(chan regionResult, len(regions))