diff --git a/cli/cmd/inventory.go b/cli/cmd/inventory.go index 0b89be44..4e5b0700 100644 --- a/cli/cmd/inventory.go +++ b/cli/cmd/inventory.go @@ -84,6 +84,16 @@ func runInventorySync(cmd *cobra.Command, args []string) error { return err } + // Discovering nothing means the query looked in the wrong place — usually + // the wrong region. Falling through would write the inventory back + // unchanged and report "all values are current", a false success. + if len(instances) == 0 { + if region, rerr := cfg.ResolveRegion(); rerr == nil && cfg.IsAWS() { + return fmt.Errorf("no instances found for env=%s in %s: nothing to sync", cfg.Env, region) + } + return fmt.Errorf("no instances found for env=%s: nothing to sync", cfg.Env) + } + return applyInstanceUpdates(invPath, instances) } diff --git a/cli/cmd/root.go b/cli/cmd/root.go index 0592b805..ac8a3c05 100644 --- a/cli/cmd/root.go +++ b/cli/cmd/root.go @@ -23,6 +23,17 @@ and operational tasks like SSM session management.`, if err := config.Init(); err != nil { return err } + // A --region given on the command line outranks the region the active + // environment declares; one merely present in dreadgoad.yaml does not. + // Viper collapses both into the same key, so pass the explicit case + // through separately. + if cmd.Flags().Changed("region") { + region, err := cmd.Flags().GetString("region") + if err != nil { + return err + } + config.SetRegionOverride(region) + } cfg, err := config.Get() if err != nil { return err diff --git a/cli/internal/config/config.go b/cli/internal/config/config.go index cf7a1269..181c8dfb 100644 --- a/cli/internal/config/config.go +++ b/cli/internal/config/config.go @@ -31,6 +31,10 @@ type EnvironmentConfig struct { VariantName string `mapstructure:"variant_name"` EnabledExtensions []string `mapstructure:"enabled_extensions"` VpcCidr string `mapstructure:"vpc_cidr"` + // Region is where this environment's lab actually lives. Each environment + // gets its own because the labs are deployed to different regions, and the + // infra/ tree is laid out as {deployment}/{env}/{region}/. + Region string `mapstructure:"region"` } // InfraConfig holds infrastructure/terragrunt settings. @@ -102,14 +106,26 @@ type Config struct { Infra InfraConfig `mapstructure:"infra"` Proxmox ProxmoxConfig `mapstructure:"proxmox"` Ludus LudusConfig `mapstructure:"ludus"` + + // regionOverride is a region named explicitly on the command line or in + // the environment. It outranks the per-environment region, which viper + // cannot express on its own: a bound pflag and a config-file key both + // land in Region, but only the former should win. + regionOverride string } var ( - cfg *Config - once sync.Once - configMissing bool + cfg *Config + once sync.Once + configMissing bool + regionOverride string ) +// SetRegionOverride records a region supplied explicitly via --region, so it +// takes precedence over the active environment's configured region. Call it +// before Get(); the root command does this from PersistentPreRunE. +func SetRegionOverride(region string) { regionOverride = region } + // ConfigMissing returns true if no dreadgoad.yaml was found during Init. // Commands that depend on provider configuration should check this and warn // the user (e.g. "no config found, using defaults; run 'dreadgoad init'"). @@ -177,6 +193,18 @@ func Get() (*Config, error) { } cfg.LogDir = filepath.Join(home, ".ansible", "logs", "goad") } + + // A --region on the command line beats DREADGOAD_REGION, so only + // consult the environment when no flag was given. Both have to be + // handled here rather than left to viper's AutomaticEnv, which would + // land the variable in Region, where the per-environment region now + // outranks it. + cfg.regionOverride = regionOverride + if cfg.regionOverride == "" { + if env := os.Getenv("DREADGOAD_REGION"); env != "" { + cfg.regionOverride = env + } + } }) return cfg, initErr } @@ -185,6 +213,7 @@ func Get() (*Config, error) { func Reset() { once = sync.Once{} cfg = nil + regionOverride = "" } // InventoryPath returns the path to the inventory file for the current env. @@ -421,13 +450,25 @@ func (c *Config) IsAWS() bool { return c.ResolvedProvider() == "aws" } -// ResolveRegion returns the configured AWS region or an actionable error if -// none is set. This is the single source of truth for region resolution: every -// command that needs to talk to AWS should call it (or ResolveRegionWithInventory) -// rather than hardcoding a default. +// ResolveRegion returns the AWS region for the active environment, or an +// actionable error if none is set. This is the single source of truth for +// region resolution: every command that needs to talk to AWS should call it +// (or ResolveRegionWithInventory) rather than hardcoding a default. +// +// Region is a property of the lab, not of the CLI — staging and prod live in +// different regions — so each environment declares its own. Highest precedence +// first: an explicit --region or DREADGOAD_REGION, then the active +// environment's region, then the top-level region as a fallback for +// environments that don't declare one. func (c *Config) ResolveRegion() (string, error) { + if c.regionOverride != "" { + return c.regionOverride, nil + } + if r := c.ActiveEnvironment().Region; r != "" { + return r, nil + } if c.Region == "" { - return "", fmt.Errorf("AWS region not configured: set 'region' in dreadgoad.yaml, export DREADGOAD_REGION, or pass --region") + return "", fmt.Errorf("AWS region not configured for env %q: set 'environments.%s.region' or 'region' in dreadgoad.yaml, export DREADGOAD_REGION, or pass --region", c.Env, c.Env) } return c.Region, nil } diff --git a/cli/internal/config/config_test.go b/cli/internal/config/config_test.go index 801c86d2..c4d63cd9 100644 --- a/cli/internal/config/config_test.go +++ b/cli/internal/config/config_test.go @@ -11,25 +11,122 @@ import ( ) func TestResolveRegion(t *testing.T) { - t.Run("returns configured region", func(t *testing.T) { - c := &Config{Region: "eu-west-1"} - got, err := c.ResolveRegion() + tests := []struct { + name string + cfg *Config + want string + wantErr string // substring the error must contain; empty means success expected + }{ + { + name: "returns configured region", + cfg: &Config{Region: "eu-west-1"}, + want: "eu-west-1", + }, + { + name: "errors when region is empty", + cfg: &Config{Region: ""}, + wantErr: "region", + }, + { + name: "prefers the active environment's region over the global one", + cfg: &Config{ + Env: "staging", + Region: "us-east-1", + Environments: map[string]EnvironmentConfig{ + "staging": {Region: "us-west-1"}, + "prod": {Region: "us-east-1"}, + }, + }, + want: "us-west-1", + }, + { + name: "falls back to the global region when the environment declares none", + cfg: &Config{ + Env: "dev", + Region: "us-east-1", + Environments: map[string]EnvironmentConfig{"dev": {VpcCidr: "10.0.0.0/16"}}, + }, + want: "us-east-1", + }, + { + name: "falls back to the global region for an undefined environment", + cfg: &Config{Env: "nope", Region: "us-east-1"}, + want: "us-east-1", + }, + { + name: "an explicit override outranks the environment region", + cfg: &Config{ + Env: "staging", + Region: "us-east-1", + regionOverride: "eu-west-1", + Environments: map[string]EnvironmentConfig{"staging": {Region: "us-west-1"}}, + }, + want: "eu-west-1", + }, + { + name: "the environment region satisfies an empty global region", + cfg: &Config{ + Env: "staging", + Environments: map[string]EnvironmentConfig{"staging": {Region: "us-west-1"}}, + }, + want: "us-west-1", + }, + { + name: "error names the environment key to set", + cfg: &Config{Env: "staging"}, + wantErr: "environments.staging.region", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := tt.cfg.ResolveRegion() + if tt.wantErr != "" { + if err == nil { + t.Fatalf("expected an error containing %q, got nil", tt.wantErr) + } + if !strings.Contains(err.Error(), tt.wantErr) { + t.Errorf("error = %v, want it to contain %q", err, tt.wantErr) + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != tt.want { + t.Errorf("ResolveRegion() = %q, want %q", got, tt.want) + } + }) + } +} + +func TestGetRegionOverridePrecedence(t *testing.T) { + t.Run("an explicit --region beats DREADGOAD_REGION", func(t *testing.T) { + Reset() + defer Reset() + t.Setenv("DREADGOAD_REGION", "ap-southeast-1") + SetRegionOverride("eu-west-1") + + c, err := Get() if err != nil { t.Fatalf("unexpected error: %v", err) } - if got != "eu-west-1" { - t.Errorf("ResolveRegion() = %q, want %q", got, "eu-west-1") + if c.regionOverride != "eu-west-1" { + t.Errorf("regionOverride = %q, want the flag value %q", c.regionOverride, "eu-west-1") } }) - t.Run("errors when region is empty", func(t *testing.T) { - c := &Config{Region: ""} - _, err := c.ResolveRegion() - if err == nil { - t.Fatal("expected error for empty region, got nil") + t.Run("DREADGOAD_REGION applies when no flag is given", func(t *testing.T) { + Reset() + defer Reset() + t.Setenv("DREADGOAD_REGION", "ap-southeast-1") + + c, err := Get() + if err != nil { + t.Fatalf("unexpected error: %v", err) } - if !strings.Contains(err.Error(), "region") { - t.Errorf("error should mention region, got: %v", err) + if c.regionOverride != "ap-southeast-1" { + t.Errorf("regionOverride = %q, want the env value %q", c.regionOverride, "ap-southeast-1") } }) } diff --git a/cli/internal/config/trace.go b/cli/internal/config/trace.go index 25a3cab1..4731dff6 100644 --- a/cli/internal/config/trace.go +++ b/cli/internal/config/trace.go @@ -26,9 +26,20 @@ func TraceConfig(cfg *Config, changedFlags map[string]bool) []TraceEntry { value string } + // Report the region the CLI will actually use, not the raw top-level key, + // which the active environment's own region usually outranks. + regionValue := cfg.Region + if r, err := cfg.ResolveRegion(); err == nil { + regionValue = r + } + regionSource := "" + if cfg.regionOverride == "" && cfg.ActiveEnvironment().Region != "" { + regionSource = fmt.Sprintf("config file (environments.%s.region)", cfg.Env) + } + items := []item{ {"env", cfg.Env}, - {"region", cfg.Region}, + {"region", regionValue}, {"debug", fmt.Sprintf("%v", cfg.Debug)}, {"max_retries", fmt.Sprintf("%d", cfg.MaxRetries)}, {"retry_delay", fmt.Sprintf("%ds", cfg.RetryDelay)}, @@ -47,6 +58,9 @@ func TraceConfig(cfg *Config, changedFlags map[string]bool) []TraceEntry { value = "(unset)" } source := resolveSource(it.key, changedFlags, fileKeys, cfgFile) + if it.key == "region" && regionSource != "" && !changedFlags["region"] { + source = regionSource + } entries = append(entries, TraceEntry{Key: it.key, Value: value, Source: source}) } return entries diff --git a/docs/cli.md b/docs/cli.md index 7eac7a9f..fce71f7e 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -73,7 +73,7 @@ dreadgoad config set environments.dev.variant true # Active environment (selects into the environments map below) env: staging -# AWS region override (default: resolved from inventory) +# Fallback AWS region for environments that don't declare their own # region: us-west-2 debug: false @@ -97,10 +97,13 @@ environments: vpc_cidr: "10.0.0.0/16" # VPC CIDR block for this environment staging: variant: false + region: us-west-1 # Where this environment's lab lives vpc_cidr: "10.1.0.0/16" prod: + region: us-east-1 vpc_cidr: "10.2.0.0/16" test: + region: us-east-2 vpc_cidr: "10.8.0.0/16" ``` @@ -109,6 +112,25 @@ environments: The `environments` map lets you configure behavior per environment. The active environment is selected by the top-level `env` key. +### Region + +Each environment declares the AWS region its lab is deployed to. Region is a +property of the lab rather than of the CLI -- staging and prod live in +different regions -- and it must match the region directory in the +`infra/{deployment}/{env}/{region}/` tree. + +Resolution order, highest first: + +1. An explicit `--region` flag or `DREADGOAD_REGION` +2. `environments..region` +3. The top-level `region`, for environments that don't declare one + +Commands that talk to an already-deployed lab (`dreadgoad ssm`, `lab reset`) +prefer the inventory's own `ansible_aws_ssm_region` over all of the above. + +If an environment has no region and no fallback is set, AWS commands fail with +an error naming the key to set rather than silently querying the wrong region. + ### VPC CIDR Each environment needs a unique VPC CIDR block. Set `vpc_cidr` in the