diff --git a/.github/ISSUE_TEMPLATE/bug_report.yml b/.github/ISSUE_TEMPLATE/bug_report.yml new file mode 100644 index 00000000..eb82d347 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/bug_report.yml @@ -0,0 +1,123 @@ +name: Bug report +description: Report a problem with the DreadGOAD CLI, Ansible roles, Terraform modules, or another part of the tooling. +title: "[bug]: " +labels: ["bug", "triage"] +body: + - type: markdown + attributes: + value: | + Thanks for taking the time to file a bug report. + + Before you continue, please confirm: + + - The issue is in the **DreadGOAD tooling** (CLI, Ansible, Terraform/Terragrunt, Packer, Warpgate, variant generator). Weak passwords and other intentional vulnerabilities inside the deployed labs are by design — please do not file those as bugs. + - If you believe you have found a **security vulnerability in the tooling itself**, do not open a public issue. Use [private vulnerability reporting](https://github.com/dreadnode/DreadGOAD/security/advisories/new) instead. See [SECURITY.md](https://github.com/dreadnode/DreadGOAD/blob/main/SECURITY.md). + + - type: checkboxes + id: preflight + attributes: + label: Pre-flight checks + options: + - label: I have searched existing issues and this is not a duplicate. + required: true + - label: I have run `dreadgoad doctor` (or the equivalent) and read its output. + required: false + - label: This is a bug in the DreadGOAD tooling, not an intentional lab vulnerability. + required: true + + - type: input + id: version + attributes: + label: DreadGOAD version + description: "Output of `dreadgoad --version`, or the commit SHA of `main` you are on." + placeholder: "e.g. 1.2.3 (commit: abc1234, built: 2026-04-07) -or- main @ abc1234" + validations: + required: true + + - type: dropdown + id: provider + attributes: + label: Provider + description: Which infrastructure provider were you using? + options: + - VirtualBox + - VMware + - Proxmox + - AWS + - Azure + - Ludus + - Not provider-specific + - Other (please describe in the bug report) + validations: + required: true + + - type: dropdown + id: lab + attributes: + label: Lab + description: Which lab were you deploying when you hit the issue? + options: + - GOAD + - GOAD-Light + - GOAD-Mini + - MINILAB + - SCCM + - NHA + - DRACARYS + - A generated variant + - Not lab-specific + - Other (please describe in the bug report) + validations: + required: true + + - type: input + id: os + attributes: + label: Operator OS + description: The OS where you are running the DreadGOAD CLI / Ansible (not the lab VMs). + placeholder: "e.g. macOS 14.5 (arm64), Ubuntu 22.04, Windows 11 + WSL2" + validations: + required: true + + - type: textarea + id: description + attributes: + label: What happened? + description: A clear description of the bug, including what you expected to happen and what actually happened. + validations: + required: true + + - type: textarea + id: repro + attributes: + label: Steps to reproduce + description: | + The exact commands you ran. Please include the full command line, any relevant config snippets (`dreadgoad.yaml`, `globalsettings.ini`), and the working directory. + placeholder: | + 1. `git clone https://github.com/dreadnode/DreadGOAD.git` + 2. `cd cli && go build -o dreadgoad .` + 3. `./dreadgoad provision --lab GOAD-Light --provider virtualbox` + 4. ... + render: shell + validations: + required: true + + - type: textarea + id: logs + attributes: + label: Relevant logs + description: | + Paste the relevant CLI output, Ansible log lines, or Terraform error. + + Tip: rerun with `--debug` for more detail. Logs are also written to `~/.ansible/logs/goad/`. + render: shell + validations: + required: false + + - type: textarea + id: extra + attributes: + label: Anything else? + description: Screenshots, related issues, recent changes you made, ideas about the cause, etc. + validations: + required: false diff --git a/.github/ISSUE_TEMPLATE/config.yml b/.github/ISSUE_TEMPLATE/config.yml new file mode 100644 index 00000000..f08a1ea7 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/config.yml @@ -0,0 +1,11 @@ +blank_issues_enabled: false +contact_links: + - name: Security vulnerability in the DreadGOAD tooling + url: https://github.com/dreadnode/DreadGOAD/security/advisories/new + about: "Report security issues in the CLI, Ansible collection, Terraform / Terragrunt modules, or other tooling privately. Do NOT use a public issue. Note: weak passwords and other intentional vulnerabilities inside the deployed labs are by design and should not be reported." + - name: Upstream GOAD documentation + url: https://orange-cyberdefense.github.io/GOAD/ + about: "Background on the original GOAD project that DreadGOAD is forked from." + - name: DreadGOAD documentation + url: https://github.com/dreadnode/DreadGOAD/tree/main/docs + about: "Provider guides, lab descriptions, validation guide, and the full vulnerability catalog." diff --git a/.github/ISSUE_TEMPLATE/feature_request.yml b/.github/ISSUE_TEMPLATE/feature_request.yml new file mode 100644 index 00000000..97be7ca2 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/feature_request.yml @@ -0,0 +1,70 @@ +name: Feature request +description: Suggest a new feature, improvement, or change to DreadGOAD. +title: "[feature]: " +labels: ["enhancement", "triage"] +body: + - type: markdown + attributes: + value: | + Thanks for suggesting an improvement to DreadGOAD. + + Before you continue, please check the [existing issues](https://github.com/dreadnode/DreadGOAD/issues?q=is%3Aissue) to see whether something similar has already been proposed. + + - type: checkboxes + id: preflight + attributes: + label: Pre-flight checks + options: + - label: I have searched existing issues and discussions for a similar request. + required: true + + - type: dropdown + id: area + attributes: + label: Area + description: Which part of the project does this affect? + options: + - CLI (`dreadgoad`) + - Ansible collection / roles + - Terraform / Terragrunt modules + - Packer / Warpgate templates + - A specific lab (GOAD, GOAD-Light, MINILAB, SCCM, NHA, DRACARYS, ...) + - A specific extension (ELK, Exchange, Wazuh, Guacamole, ...) + - Variant generator + - Documentation + - CI / release tooling + - Other + validations: + required: true + + - type: textarea + id: problem + attributes: + label: What problem does this solve? + description: Describe the use case and the pain point you are running into. "I'm always frustrated when..." + validations: + required: true + + - type: textarea + id: proposal + attributes: + label: Proposed solution + description: How would you like this to work? Be as concrete as you can — example commands, config snippets, or UI sketches all help. + validations: + required: true + + - type: textarea + id: alternatives + attributes: + label: Alternatives considered + description: Other approaches you considered and why you rejected them. + validations: + required: false + + - type: textarea + id: extra + attributes: + label: Additional context + description: Links, references, related issues, or anything else useful. + validations: + required: false diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md new file mode 100644 index 00000000..384bddbf --- /dev/null +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -0,0 +1,77 @@ + + +## Summary + + + +## Type of change + + + +- [ ] Bug fix (non-breaking change that fixes an issue) +- [ ] New feature (non-breaking change that adds functionality) +- [ ] Breaking change (fix or feature that would change existing behavior) +- [ ] New lab, lab variant, or extension +- [ ] New / updated provider support +- [ ] Refactor / internal cleanup (no functional change) +- [ ] Documentation +- [ ] CI / build / release tooling +- [ ] Dependency update + +## Area + + + +- [ ] CLI (`cli/`) +- [ ] Ansible collection (`ansible/`) +- [ ] Terraform / Terragrunt (`infra/`, `modules/`) +- [ ] Packer / Warpgate (`packer/`, `warpgate-templates/`) +- [ ] Lab definitions (`ad/`) +- [ ] Extensions (`extensions/`) +- [ ] Variant generator / tools (`tools/`) +- [ ] Documentation (`docs/`, `README.md`, etc.) +- [ ] CI workflows (`.github/`) + +## Related issues + + + +## How was this tested? + + + +- Provider(s) tested: +- Lab(s) tested: +- Operator OS: + +## Screenshots / logs (optional) + + + +## Checklist + +- [ ] I have read [CONTRIBUTING.md](../CONTRIBUTING.md). +- [ ] My changes follow the existing code style of the area I touched. +- [ ] I have added or updated tests where it makes sense (Go tests under `cli/`, Ansible syntax checks, etc.). +- [ ] I have updated documentation (README, `docs/`, role README, command help text) where relevant. +- [ ] I have checked that I am not committing real secrets, personal credentials, or internal hostnames. (Intentional lab credentials inside `ad/`, `ansible/`, and `extensions/` are expected and fine.) +- [ ] If this PR changes user-facing CLI behavior, I have updated the relevant `--help` text and any docs that reference it. +- [ ] If this PR introduces a breaking change, I have called it out in the **Summary** above. diff --git a/ansible/galaxy.yml b/ansible/galaxy.yml index 984a8d1a..f7a42650 100644 --- a/ansible/galaxy.yml +++ b/ansible/galaxy.yml @@ -19,7 +19,7 @@ tags: dependencies: amazon.aws: ">=9.0.0" ansible.windows: ">=2.5.0" - community.general: "*" + community.general: ">=9.0.0,<11.0.0" community.windows: ">=2.3.0" chocolatey.chocolatey: ">=1.5.3" repository: https://github.com/dreadnode/ansible-collection-goad diff --git a/ansible/requirements.yml b/ansible/requirements.yml index fecf0f6f..5da3d607 100644 --- a/ansible/requirements.yml +++ b/ansible/requirements.yml @@ -3,6 +3,7 @@ collections: - name: ansible.windows version: ">=2.5.0" - name: community.general + version: ">=9.0.0,<11.0.0" - name: community.windows version: ">=2.3.0" - name: chocolatey.chocolatey @@ -14,3 +15,4 @@ collections: roles: - name: geerlingguy.mysql + version: ">=4.0.0,<5.0.0" diff --git a/cli/cmd/ami.go b/cli/cmd/ami.go index 40bb2656..a55868cd 100644 --- a/cli/cmd/ami.go +++ b/cli/cmd/ami.go @@ -127,8 +127,14 @@ func runAMIBuild(cmd *cobra.Command, args []string) error { verbose := viper.GetBool("debug") + // Region precedence for `ami build`: --region flag > cfg.Region (from + // dreadgoad.yaml or DREADGOAD_REGION) > the template's own embedded region. + // The empty fallback (instead of a hardcoded default) is what allows the + // template's embedded region to win when neither --region nor cfg.Region + // is set; if either is set, it overrides the template even though the + // user didn't ask for this specific build. bf := buildFlags{ - region: getFlagString(cmd, "region", cfg.Region, "us-west-1"), + region: getFlagString(cmd, "region", cfg.Region, ""), instanceType: getFlagStringOpt(cmd, "instance-type"), profile: getFlagStringOpt(cmd, "profile"), instanceProfile: getFlagStringOpt(cmd, "instance-profile"), @@ -297,7 +303,17 @@ func getFlagBool(cmd *cobra.Command, name string) bool { } func newAWSClients(cmd *cobra.Command, cfg *config.Config) (*ami.AWSClients, error) { - region := getFlagString(cmd, "region", cfg.Region, "us-west-1") + // For ami list-resources / purge there is no warpgate template fallback, + // so a region is required. Local --region flag wins over cfg.Region; + // otherwise we error out via ResolveRegion rather than silently picking one. + region := getFlagStringOpt(cmd, "region") + if region == "" { + var err error + region, err = cfg.ResolveRegion() + if err != nil { + return nil, err + } + } profile := getFlagStringOpt(cmd, "profile") return ami.NewAWSClients(context.Background(), ami.ClientConfig{ Region: region, diff --git a/cli/cmd/config_cmd.go b/cli/cmd/config_cmd.go index 45c419eb..5dc98aef 100644 --- a/cli/cmd/config_cmd.go +++ b/cli/cmd/config_cmd.go @@ -25,7 +25,7 @@ var configShowCmd = &cobra.Command{ return err } fmt.Printf("Environment: %s\n", cfg.Env) - fmt.Printf("Region: %s\n", valueOrDefault(cfg.Region, "(from inventory)")) + fmt.Printf("Region: %s\n", valueOrDefault(cfg.Region, "(unset — required for AWS commands)")) fmt.Printf("Debug: %v\n", cfg.Debug) fmt.Printf("Max Retries: %d\n", cfg.MaxRetries) fmt.Printf("Retry Delay: %ds\n", cfg.RetryDelay) @@ -84,7 +84,7 @@ var configInitCmd = &cobra.Command{ content := `# DreadGOAD CLI Configuration env: staging -# region: us-west-2 # Override AWS region (default: from inventory) +# region: us-east-1 # AWS region (required for AWS commands; can also be set via DREADGOAD_REGION or --region) debug: false max_retries: 3 retry_delay: 30 diff --git a/cli/cmd/env_cmd.go b/cli/cmd/env_cmd.go index 728ae1f7..fb6c36cf 100644 --- a/cli/cmd/env_cmd.go +++ b/cli/cmd/env_cmd.go @@ -51,7 +51,7 @@ func init() { envCmd.AddCommand(envCreateCmd) envCmd.AddCommand(envListCmd) - envCreateCmd.Flags().String("region", "us-east-1", "AWS region for the environment") + envCreateCmd.Flags().String("region", "", "AWS region for the environment (required; or set via dreadgoad.yaml / DREADGOAD_REGION / --region)") envCreateCmd.Flags().String("vpc-cidr", "", "VPC CIDR block (default: auto-assigned)") envCreateCmd.Flags().String("reference", "staging", "Reference environment to copy infrastructure from") envCreateCmd.Flags().Bool("variant", false, "Generate randomized variant config") @@ -70,17 +70,26 @@ func runEnvCreate(cmd *cobra.Command, args []string) error { } region, _ := cmd.Flags().GetString("region") + if region == "" { + region, err = cfg.ResolveRegion() + if err != nil { + return fmt.Errorf("env create requires a region: pass --region, set 'region' in dreadgoad.yaml, or export DREADGOAD_REGION") + } + } vpcCIDR, _ := cmd.Flags().GetString("vpc-cidr") reference, _ := cmd.Flags().GetString("reference") useVariant, _ := cmd.Flags().GetBool("variant") force, _ := cmd.Flags().GetBool("force") - deployment := cfg.Infra.Deployment - infraBase := filepath.Join(cfg.ProjectRoot, "infra", deployment) - if vpcCIDR == "" { vpcCIDR = cfg.VpcCIDR(envName) } + + return scaffoldEnv(cfg, envName, region, vpcCIDR, reference, useVariant, force) +} + +func scaffoldEnv(cfg *config.Config, envName, region, vpcCIDR, reference string, useVariant, force bool) error { + infraBase := filepath.Join(cfg.ProjectRoot, "infra", cfg.Infra.Deployment) envDir := filepath.Join(infraBase, envName) regionDir := filepath.Join(envDir, region) diff --git a/cli/cmd/infra.go b/cli/cmd/infra.go index f9c67f13..6d07e1ab 100644 --- a/cli/cmd/infra.go +++ b/cli/cmd/infra.go @@ -29,9 +29,9 @@ func requireInfra(ctx context.Context) (*infraContext, error) { return nil, err } - region := cfg.Region - if region == "" { - region = "us-west-1" + region, err := cfg.ResolveRegion() + if err != nil { + return nil, err } client, err := daws.NewClient(ctx, region) diff --git a/cli/cmd/infra_cmd.go b/cli/cmd/infra_cmd.go index 0d73225b..07b6c70f 100644 --- a/cli/cmd/infra_cmd.go +++ b/cli/cmd/infra_cmd.go @@ -95,9 +95,9 @@ func runInfraAction(action string) func(*cobra.Command, []string) error { exclude, _ := cmd.Flags().GetString("exclude") deployment := resolveDeployment(cmd, cfg) - region := cfg.Region - if region == "" { - region = "us-west-1" + region, err := cfg.ResolveRegion() + if err != nil { + return err } opts := terragrunt.Options{ @@ -185,9 +185,9 @@ func runInfraOutput(cmd *cobra.Command, args []string) error { module, _ := cmd.Flags().GetString("module") deployment := resolveDeployment(cmd, cfg) - region := cfg.Region - if region == "" { - region = "us-west-1" + region, err := cfg.ResolveRegion() + if err != nil { + return err } workDir := filepath.Join(cfg.ProjectRoot, "infra", deployment, cfg.Env, region) @@ -224,9 +224,9 @@ func runInfraValidate(cmd *cobra.Command, args []string) error { deployment := resolveDeployment(cmd, cfg) - region := cfg.Region - if region == "" { - region = "us-west-1" + region, err := cfg.ResolveRegion() + if err != nil { + return err } basePath := filepath.Join(cfg.ProjectRoot, "infra", deployment) diff --git a/cli/cmd/inventory.go b/cli/cmd/inventory.go index ebf2a9f6..a48e9e56 100644 --- a/cli/cmd/inventory.go +++ b/cli/cmd/inventory.go @@ -78,7 +78,7 @@ func runInventorySync(cmd *cobra.Command, args []string) error { } jsonFile, _ := cmd.Flags().GetString("json") - instances, err := loadInstances(context.Background(), jsonFile, invPath, cfg.Env) + instances, err := loadInstances(context.Background(), jsonFile, invPath, cfg) if err != nil { return err } @@ -112,7 +112,7 @@ func updateEnvField(invPath, env string) error { return nil } -func loadInstances(ctx context.Context, jsonFile, invPath, env string) ([]instanceInfo, error) { +func loadInstances(ctx context.Context, jsonFile, invPath string, cfg *config.Config) ([]instanceInfo, error) { if jsonFile != "" { raw, err := os.ReadFile(jsonFile) if err != nil { @@ -129,11 +129,15 @@ func loadInstances(ctx context.Context, jsonFile, invPath, env string) ([]instan if err != nil { return nil, err } - client, err := daws.NewClient(ctx, parsed.Region()) + region, err := cfg.ResolveRegionWithInventory(parsed) if err != nil { return nil, err } - awsInstances, err := client.DiscoverInstances(ctx, env) + client, err := daws.NewClient(ctx, region) + if err != nil { + return nil, err + } + awsInstances, err := client.DiscoverInstances(ctx, cfg.Env) if err != nil { return nil, fmt.Errorf("discover instances: %w", err) } @@ -195,7 +199,11 @@ func runInventoryShow(cmd *cobra.Command, args []string) error { return err } - fmt.Printf("Inventory: %s (env=%s, region=%s)\n\n", parsed.FilePath, cfg.Env, parsed.Region()) + displayRegion := parsed.Region() + if displayRegion == "" { + displayRegion = "(not set in inventory)" + } + fmt.Printf("Inventory: %s (env=%s, region=%s)\n\n", parsed.FilePath, cfg.Env, displayRegion) fmt.Printf("%-8s %-24s %-10s %-10s %s\n", "HOST", "INSTANCE ID", "DICT_KEY", "DNS_DOMAIN", "GROUPS") fmt.Println(strings.Repeat("-", 80)) @@ -235,7 +243,11 @@ func generateInstanceMapping(ctx context.Context, outputPath string) error { outputPath = filepath.Join("/tmp", fmt.Sprintf("aws_instance_mapping_%s.json", cfg.Env)) } - client, err := daws.NewClient(ctx, parsed.Region()) + region, err := cfg.ResolveRegionWithInventory(parsed) + if err != nil { + return err + } + client, err := daws.NewClient(ctx, region) if err != nil { return err } diff --git a/cli/cmd/lab.go b/cli/cmd/lab.go index 1384c92c..16031a02 100644 --- a/cli/cmd/lab.go +++ b/cli/cmd/lab.go @@ -79,9 +79,9 @@ func runLabStatus(cmd *cobra.Command, args []string) error { } ctx := context.Background() - region := cfg.Region - if region == "" { - region = "us-west-1" + region, err := cfg.ResolveRegion() + if err != nil { + return err } client, err := daws.NewClient(ctx, region) @@ -118,9 +118,9 @@ func runLabAction(action string) func(*cobra.Command, []string) error { } ctx := context.Background() - region := cfg.Region - if region == "" { - region = "us-west-1" + region, err := cfg.ResolveRegion() + if err != nil { + return err } client, err := daws.NewClient(ctx, region) @@ -221,9 +221,9 @@ func runVMAction(action string) func(*cobra.Command, []string) error { } ctx := context.Background() - region := cfg.Region - if region == "" { - region = "us-west-1" + region, err := cfg.ResolveRegion() + if err != nil { + return err } client, err := daws.NewClient(ctx, region) diff --git a/cli/cmd/root.go b/cli/cmd/root.go index be3d5f92..990e63e2 100644 --- a/cli/cmd/root.go +++ b/cli/cmd/root.go @@ -49,7 +49,7 @@ func Execute() error { func init() { rootCmd.PersistentFlags().StringP("env", "e", "staging", "Target environment (dev, staging, prod)") - rootCmd.PersistentFlags().String("region", "", "AWS region (default: from inventory)") + rootCmd.PersistentFlags().String("region", "", "AWS region (required for AWS commands; can also be set via --region, dreadgoad.yaml, DREADGOAD_REGION, or inventory ansible_aws_ssm_region where supported)") rootCmd.PersistentFlags().Bool("debug", false, "Enable debug/verbose output") rootCmd.PersistentFlags().String("config", "", "Config file path") diff --git a/cli/cmd/ssm.go b/cli/cmd/ssm.go index a9ce7fad..a18baa78 100644 --- a/cli/cmd/ssm.go +++ b/cli/cmd/ssm.go @@ -75,7 +75,11 @@ func runSSMStatus(cmd *cobra.Command, args []string) error { return fmt.Errorf("parse inventory: %w", err) } - client, err := daws.NewClient(ctx, inv.Region()) + region, err := cfg.ResolveRegionWithInventory(inv) + if err != nil { + return err + } + client, err := daws.NewClient(ctx, region) if err != nil { return err } @@ -120,7 +124,11 @@ func runSSMCleanup(cmd *cobra.Command, args []string) error { return fmt.Errorf("parse inventory: %w", err) } - client, err := daws.NewClient(ctx, inv.Region()) + region, err := cfg.ResolveRegionWithInventory(inv) + if err != nil { + return err + } + client, err := daws.NewClient(ctx, region) if err != nil { return err } @@ -157,7 +165,10 @@ func runSSMConnect(cmd *cobra.Command, args []string) error { return fmt.Errorf("host %q not found in inventory", args[0]) } - region := inv.Region() + region, err := cfg.ResolveRegionWithInventory(inv) + if err != nil { + return err + } fmt.Printf("Starting SSM session to %s (%s) in %s...\n", host.Name, host.InstanceID, region) // Ignore SIGINT in the parent so Ctrl+C is forwarded to the SSM @@ -186,10 +197,18 @@ func runSSMRun(cmd *cobra.Command, args []string) error { hostsFlag, _ := cmd.Flags().GetString("hosts") psCmd, _ := cmd.Flags().GetString("cmd") - // Determine region - prefer flag, then inventory - region := cfg.Region - if region == "" { - region = "us-west-1" + // Parse the inventory so the region resolves consistently with the other + // ssm subcommands: prefer the inventory's ansible_aws_ssm_region, fall back + // to cfg.Region. Without this, a user with region in the inventory but not + // in dreadgoad.yaml would have ssm status/cleanup/connect work and only + // ssm run fail with "AWS region not configured". + inv, err := inventory.Parse(cfg.InventoryPath()) + if err != nil { + return fmt.Errorf("parse inventory: %w", err) + } + region, err := cfg.ResolveRegionWithInventory(inv) + if err != nil { + return err } client, err := daws.NewClient(ctx, region) diff --git a/cli/internal/ansible/retry.go b/cli/internal/ansible/retry.go index 6d1c2080..2ff737af 100644 --- a/cli/internal/ansible/retry.go +++ b/cli/internal/ansible/retry.go @@ -237,7 +237,12 @@ func CleanupSSMSessions(ctx context.Context, env string, log *slog.Logger) { return } - client, err := daws.NewClient(ctx, inv.Region()) + region, err := cfg.ResolveRegionWithInventory(inv) + if err != nil { + log.Warn("could not resolve AWS region for SSM cleanup", "error", err) + return + } + client, err := daws.NewClient(ctx, region) if err != nil { log.Warn("could not create AWS client for SSM cleanup", "error", err) return @@ -269,7 +274,12 @@ func fixSSMUsers(ctx context.Context, env string, failedHosts []string, log *slo return } - client, err := daws.NewClient(ctx, inv.Region()) + region, err := cfg.ResolveRegionWithInventory(inv) + if err != nil { + log.Warn("could not resolve AWS region for ssm-user fix", "error", err) + return + } + client, err := daws.NewClient(ctx, region) if err != nil { log.Warn("could not create AWS client for ssm-user fix", "error", err) return diff --git a/cli/internal/config/config.go b/cli/internal/config/config.go index f26c1db8..37beba6e 100644 --- a/cli/internal/config/config.go +++ b/cli/internal/config/config.go @@ -7,6 +7,7 @@ import ( "path/filepath" "sync" + "github.com/dreadnode/dreadgoad/internal/inventory" "github.com/spf13/viper" ) @@ -239,6 +240,29 @@ func (c *Config) VpcCIDR(envName string) string { return fmt.Sprintf("10.%d.0.0/16", octet) } +// 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. +func (c *Config) ResolveRegion() (string, error) { + if c.Region == "" { + return "", fmt.Errorf("AWS region not configured: set 'region' in dreadgoad.yaml, export DREADGOAD_REGION, or pass --region") + } + return c.Region, nil +} + +// ResolveRegionWithInventory resolves the AWS region for talking to a deployed +// lab, preferring the parsed Ansible inventory's own region (most authoritative +// — the lab knows where it lives) and falling back to ResolveRegion. +func (c *Config) ResolveRegionWithInventory(inv *inventory.Inventory) (string, error) { + if inv != nil { + if r := inv.Region(); r != "" { + return r, nil + } + } + return c.ResolveRegion() +} + // InfraBasePath returns the base path for a deployment's infra directory. func (c *Config) InfraBasePath() string { return filepath.Join(c.ProjectRoot, "infra", c.Infra.Deployment) @@ -246,17 +270,21 @@ func (c *Config) InfraBasePath() string { // InfraWorkDir returns the working directory for terragrunt operations // at the region level: infra/{deployment}/{env}/{region}/ -func (c *Config) InfraWorkDir() string { - region := c.Region - if region == "" { - region = "us-west-1" +func (c *Config) InfraWorkDir() (string, error) { + region, err := c.ResolveRegion() + if err != nil { + return "", err } - return filepath.Join(c.InfraBasePath(), c.Env, region) + return filepath.Join(c.InfraBasePath(), c.Env, region), nil } // InfraModulePath returns the path for a specific module within the infra working directory. -func (c *Config) InfraModulePath(module string) string { - return filepath.Join(c.InfraWorkDir(), module) +func (c *Config) InfraModulePath(module string) (string, error) { + workDir, err := c.InfraWorkDir() + if err != nil { + return "", err + } + return filepath.Join(workDir, module), nil } func findProjectRoot() (string, error) { diff --git a/cli/internal/config/config_test.go b/cli/internal/config/config_test.go index eb561840..38ff7d7b 100644 --- a/cli/internal/config/config_test.go +++ b/cli/internal/config/config_test.go @@ -5,8 +5,90 @@ import ( "path/filepath" "strings" "testing" + + "github.com/dreadnode/dreadgoad/internal/inventory" ) +func TestResolveRegion(t *testing.T) { + t.Run("returns configured region", func(t *testing.T) { + c := &Config{Region: "eu-west-1"} + got, err := c.ResolveRegion() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != "eu-west-1" { + t.Errorf("ResolveRegion() = %q, want %q", got, "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") + } + if !strings.Contains(err.Error(), "region") { + t.Errorf("error should mention region, got: %v", err) + } + }) +} + +func TestResolveRegionWithInventory(t *testing.T) { + t.Run("prefers inventory region", func(t *testing.T) { + c := &Config{Region: "us-west-1"} + inv := &inventory.Inventory{ + Vars: map[string]string{"ansible_aws_ssm_region": "ap-southeast-1"}, + } + got, err := c.ResolveRegionWithInventory(inv) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != "ap-southeast-1" { + t.Errorf("ResolveRegionWithInventory() = %q, want %q", got, "ap-southeast-1") + } + }) + + t.Run("falls back to config when inventory has no region", func(t *testing.T) { + c := &Config{Region: "us-east-2"} + inv := &inventory.Inventory{Vars: map[string]string{}} + got, err := c.ResolveRegionWithInventory(inv) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != "us-east-2" { + t.Errorf("ResolveRegionWithInventory() = %q, want %q", got, "us-east-2") + } + }) + + t.Run("falls back to config when inventory is nil", func(t *testing.T) { + c := &Config{Region: "eu-central-1"} + got, err := c.ResolveRegionWithInventory(nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != "eu-central-1" { + t.Errorf("ResolveRegionWithInventory() = %q, want %q", got, "eu-central-1") + } + }) + + t.Run("errors when both inventory and config have no region", func(t *testing.T) { + c := &Config{Region: ""} + inv := &inventory.Inventory{Vars: map[string]string{}} + _, err := c.ResolveRegionWithInventory(inv) + if err == nil { + t.Fatal("expected error when no region available, got nil") + } + }) + + t.Run("errors when nil inventory and config has no region", func(t *testing.T) { + c := &Config{Region: ""} + _, err := c.ResolveRegionWithInventory(nil) + if err == nil { + t.Fatal("expected error when no region available, got nil") + } + }) +} + func TestConfigInventoryPath(t *testing.T) { c := &Config{ProjectRoot: "/opt/goad", Env: "dev"} got := c.InventoryPath() diff --git a/cli/internal/inventory/parser.go b/cli/internal/inventory/parser.go index 000dae8b..00042c20 100644 --- a/cli/internal/inventory/parser.go +++ b/cli/internal/inventory/parser.go @@ -128,12 +128,16 @@ func (inv *Inventory) InstanceIDs() []string { return ids } -// Region returns the AWS SSM region from inventory vars. +// Region returns the AWS SSM region from inventory vars, or an empty string +// if the inventory does not specify one. Callers should fall back to +// config.Config.ResolveRegion() rather than hardcoding a default — silently +// picking a region for the user causes confusing "no instances found" errors +// when they're actually deployed in a different region. func (inv *Inventory) Region() string { if r, ok := inv.Vars["ansible_aws_ssm_region"]; ok { return r } - return "us-west-2" + return "" } // HostByName returns a host by its name (case-insensitive). diff --git a/cli/internal/inventory/parser_test.go b/cli/internal/inventory/parser_test.go index dacde940..8c60a5ff 100644 --- a/cli/internal/inventory/parser_test.go +++ b/cli/internal/inventory/parser_test.go @@ -175,11 +175,11 @@ func TestRegion(t *testing.T) { } }) - t.Run("default fallback", func(t *testing.T) { + t.Run("missing returns empty", func(t *testing.T) { path := writeTestInventory(t, "[default]\n") inv, _ := Parse(path) - if got := inv.Region(); got != "us-west-2" { - t.Errorf("Region() = %q, want %q", got, "us-west-2") + if got := inv.Region(); got != "" { + t.Errorf("Region() = %q, want empty string (no silent fallback)", got) } }) }