diff --git a/ad/GOAD-variant-1/.dreadgoad-variant-complete b/ad/GOAD-variant-1/.dreadgoad-variant-complete new file mode 100644 index 00000000..c591978b --- /dev/null +++ b/ad/GOAD-variant-1/.dreadgoad-variant-complete @@ -0,0 +1 @@ +complete diff --git a/cli/cmd/extension.go b/cli/cmd/extension.go index 29ced910..a740bb93 100644 --- a/cli/cmd/extension.go +++ b/cli/cmd/extension.go @@ -49,8 +49,8 @@ func init() { extensionListCmd.Flags().String("lab", "", "Filter by lab compatibility (e.g. GOAD, GOAD-Light)") extensionProvisionCmd.Flags().String("limit", "", "Limit execution to specific hosts") - extensionProvisionCmd.Flags().Int("max-retries", 0, "Max retry attempts (default: from config)") - extensionProvisionCmd.Flags().Int("retry-delay", 0, "Delay between retries in seconds") + extensionProvisionCmd.Flags().Int("max-retries", 0, "Max retry attempts (default: from config; 0 disables retries)") + extensionProvisionCmd.Flags().Int("retry-delay", 0, "Delay between retries in seconds (default: from config; 0 disables delay)") } func runExtensionList(cmd *cobra.Command, args []string) error { @@ -111,10 +111,12 @@ func runExtensionProvision(cmd *cobra.Command, args []string) error { } limit, _ := cmd.Flags().GetString("limit") - maxRetries, _ := cmd.Flags().GetInt("max-retries") - retryDelay, _ := cmd.Flags().GetInt("retry-delay") + retry, err := retryOverridesFromFlags(cmd) + if err != nil { + return err + } - return provisionExtension(cfg, name, ext, limit, maxRetries, retryDelay) + return provisionExtension(cfg, name, ext, limit, retry) } func runExtensionProvisionAll(cmd *cobra.Command, args []string) error { @@ -141,7 +143,7 @@ func runExtensionProvisionAll(cmd *cobra.Command, args []string) error { if !ok { return fmt.Errorf("enabled extension %q not found in config", name) } - if err := provisionExtension(cfg, name, ext, "", 0, 0); err != nil { + if err := provisionExtension(cfg, name, ext, "", retryOverrides{}); err != nil { return fmt.Errorf("extension %s failed: %w", name, err) } } @@ -150,7 +152,7 @@ func runExtensionProvisionAll(cmd *cobra.Command, args []string) error { return nil } -func provisionExtension(cfg *config.Config, name string, ext config.ExtensionConfig, limit string, maxRetries, retryDelay int) error { +func provisionExtension(cfg *config.Config, name string, ext config.ExtensionConfig, limit string, retry retryOverrides) error { ctx := context.Background() _ = os.MkdirAll(cfg.LogDir, 0o755) @@ -198,12 +200,7 @@ func provisionExtension(cfg *config.Config, name string, ext config.ExtensionCon Debug: cfg.Debug, LogFile: logFile, } - if maxRetries > 0 { - opts.MaxRetries = maxRetries - } - if retryDelay > 0 { - opts.RetryDelay = time.Duration(retryDelay) * time.Second - } + retry.apply(&opts) if err := ansible.RunPlaybookWithRetry(ctx, opts); err != nil { return err diff --git a/cli/cmd/infra_cmd.go b/cli/cmd/infra_cmd.go index 800979b0..f814af87 100644 --- a/cli/cmd/infra_cmd.go +++ b/cli/cmd/infra_cmd.go @@ -2,7 +2,9 @@ package cmd import ( "context" + "errors" "fmt" + "log/slog" "os" "os/exec" "path/filepath" @@ -106,7 +108,11 @@ func init() { func materializeLabConfig(cfg *config.Config) error { resolved, err := cfg.ResolvedLabConfigPath() if err != nil { - return nil // no config to materialize -- let terragrunt surface the error + if errors.Is(err, config.ErrLabConfigNotFound) { + slog.Debug("no lab config to materialize; continuing for standalone infrastructure", "error", err) + return nil + } + return fmt.Errorf("resolve lab config: %w", err) } dataDir := filepath.Join(cfg.ProjectRoot, "ad", "GOAD", "data") @@ -121,7 +127,13 @@ func materializeLabConfig(cfg *config.Config) error { if err != nil { return fmt.Errorf("read resolved config: %w", err) } - return os.WriteFile(expected, data, 0o644) + if err := os.MkdirAll(dataDir, 0o755); err != nil { + return fmt.Errorf("create lab config directory: %w", err) + } + if err := os.WriteFile(expected, data, 0o644); err != nil { + return fmt.Errorf("write lab config: %w", err) + } + return nil } func runInfraAction(action string) func(*cobra.Command, []string) error { diff --git a/cli/cmd/infra_cmd_test.go b/cli/cmd/infra_cmd_test.go new file mode 100644 index 00000000..911d77fe --- /dev/null +++ b/cli/cmd/infra_cmd_test.go @@ -0,0 +1,168 @@ +package cmd + +import ( + "errors" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/dreadnode/dreadgoad/internal/config" +) + +func TestMaterializeLabConfigAllowsMissingOptionalConfig(t *testing.T) { + cfg := &config.Config{ProjectRoot: t.TempDir(), Env: "dev"} + + if err := materializeLabConfig(cfg); err != nil { + t.Fatalf("materializeLabConfig() error = %v, want nil", err) + } +} + +func TestMaterializeLabConfigSurfacesResolutionFailure(t *testing.T) { + root := t.TempDir() + dataDir := filepath.Join(root, "ad", "GOAD", "data") + if err := os.MkdirAll(dataDir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dataDir, "config.json"), []byte(`{"base":true}`), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dataDir, "dev-overlay.json"), []byte(`{"broken":`), 0o644); err != nil { + t.Fatal(err) + } + + err := materializeLabConfig(&config.Config{ProjectRoot: root, Env: "dev"}) + if err == nil || !strings.Contains(err.Error(), "resolve lab config: merge config") { + t.Fatalf("materializeLabConfig() error = %v, want merge resolution error", err) + } + if errors.Is(err, config.ErrLabConfigNotFound) { + t.Fatalf("malformed config was misclassified as missing: %v", err) + } +} + +func TestMaterializeLabConfigRejectsOverlayWithoutBase(t *testing.T) { + root := t.TempDir() + dataDir := filepath.Join(root, "ad", "GOAD", "data") + if err := os.MkdirAll(dataDir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dataDir, "dev-overlay.json"), []byte(`{"present":true}`), 0o644); err != nil { + t.Fatal(err) + } + + err := materializeLabConfig(&config.Config{ProjectRoot: root, Env: "dev"}) + if err == nil || !strings.Contains(err.Error(), "overlay") || !strings.Contains(err.Error(), "requires base config") { + t.Fatalf("materializeLabConfig() error = %v, want missing base config error", err) + } + if errors.Is(err, config.ErrLabConfigNotFound) { + t.Fatalf("orphaned overlay was misclassified as missing: %v", err) + } +} + +func TestMaterializeLabConfigCreatesDestinationDirectory(t *testing.T) { + root := t.TempDir() + variantData := filepath.Join(root, "ad", "custom-variant", "data") + if err := os.MkdirAll(variantData, 0o755); err != nil { + t.Fatal(err) + } + want := []byte(`{"variant":true}`) + if err := os.WriteFile(filepath.Join(variantData, "config.json"), want, 0o644); err != nil { + t.Fatal(err) + } + cfg := &config.Config{ + ProjectRoot: root, + Env: "dev", + Environments: map[string]config.EnvironmentConfig{ + "dev": {Variant: true, VariantTarget: "ad/custom-variant"}, + }, + } + + if err := materializeLabConfig(cfg); err != nil { + t.Fatalf("materializeLabConfig() error: %v", err) + } + destination := filepath.Join(root, "ad", "GOAD", "data", "dev-config.json") + got, err := os.ReadFile(destination) + if err != nil { + t.Fatalf("read materialized config: %v", err) + } + if string(got) != string(want) { + t.Errorf("materialized config = %s, want %s", got, want) + } +} + +func TestMaterializeLabConfigLeavesLegacyDestinationUntouched(t *testing.T) { + root := t.TempDir() + dataDir := filepath.Join(root, "ad", "GOAD", "data") + if err := os.MkdirAll(dataDir, 0o755); err != nil { + t.Fatal(err) + } + destination := filepath.Join(dataDir, "dev-config.json") + want := []byte(`{"legacy":true}`) + if err := os.WriteFile(destination, want, 0o644); err != nil { + t.Fatal(err) + } + + if err := materializeLabConfig(&config.Config{ProjectRoot: root, Env: "dev"}); err != nil { + t.Fatalf("materializeLabConfig() error: %v", err) + } + got, err := os.ReadFile(destination) + if err != nil { + t.Fatal(err) + } + if string(got) != string(want) { + t.Errorf("legacy config changed: got %s, want %s", got, want) + } +} + +func TestMaterializeLabConfigReportsDirectoryCreationFailure(t *testing.T) { + root := t.TempDir() + variantData := filepath.Join(root, "ad", "custom-variant", "data") + if err := os.MkdirAll(variantData, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(variantData, "config.json"), []byte(`{}`), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, "ad", "GOAD"), []byte("not a directory"), 0o644); err != nil { + t.Fatal(err) + } + cfg := &config.Config{ + ProjectRoot: root, + Env: "dev", + Environments: map[string]config.EnvironmentConfig{ + "dev": {Variant: true, VariantTarget: "ad/custom-variant"}, + }, + } + + err := materializeLabConfig(cfg) + if err == nil || !strings.Contains(err.Error(), "create lab config directory") { + t.Fatalf("materializeLabConfig() error = %v, want directory creation error", err) + } +} + +func TestMaterializeLabConfigReportsWriteFailure(t *testing.T) { + root := t.TempDir() + variantData := filepath.Join(root, "ad", "custom-variant", "data") + if err := os.MkdirAll(variantData, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(variantData, "config.json"), []byte(`{}`), 0o644); err != nil { + t.Fatal(err) + } + destination := filepath.Join(root, "ad", "GOAD", "data", "dev-config.json") + if err := os.MkdirAll(destination, 0o755); err != nil { + t.Fatal(err) + } + cfg := &config.Config{ + ProjectRoot: root, + Env: "dev", + Environments: map[string]config.EnvironmentConfig{ + "dev": {Variant: true, VariantTarget: "ad/custom-variant"}, + }, + } + + err := materializeLabConfig(cfg) + if err == nil || !strings.Contains(err.Error(), "write lab config") { + t.Fatalf("materializeLabConfig() error = %v, want write error", err) + } +} diff --git a/cli/cmd/lab_reset.go b/cli/cmd/lab_reset.go index cbd16f5e..24914760 100644 --- a/cli/cmd/lab_reset.go +++ b/cli/cmd/lab_reset.go @@ -249,8 +249,8 @@ func init() { labResetCmd.Flags().Bool("skip-provision", false, "Skip the AD-state playbook stage") labResetCmd.Flags().String("plays", "", "Comma-separated playbooks (default: AD-state set)") labResetCmd.Flags().String("limit", "", "Limit playbook execution to specific hosts") - labResetCmd.Flags().Int("max-retries", 0, "Max retry attempts (default: from config)") - labResetCmd.Flags().Int("retry-delay", 0, "Delay between retries in seconds (default: from config)") + labResetCmd.Flags().Int("max-retries", 0, "Max retry attempts (default: from config; 0 disables retries)") + labResetCmd.Flags().Int("retry-delay", 0, "Delay between retries in seconds (default: from config; 0 disables delay)") labResetCmd.Flags().Bool("skip-creator-check", false, "Skip the admin creator-SID safety belt during purge") labResetCmd.Flags().StringArrayP("extra-vars", "E", nil, extraVarsUsage) } @@ -496,8 +496,10 @@ func runLabReset(cmd *cobra.Command, args []string) error { skipProvision, _ := cmd.Flags().GetBool("skip-provision") playsFlag, _ := cmd.Flags().GetString("plays") limit, _ := cmd.Flags().GetString("limit") - maxRetries, _ := cmd.Flags().GetInt("max-retries") - retryDelay, _ := cmd.Flags().GetInt("retry-delay") + retry, err := retryOverridesFromFlags(cmd) + if err != nil { + return err + } skipCreator, _ := cmd.Flags().GetBool("skip-creator-check") extraVars, err := parseExtraVars(cmd) if err != nil { @@ -524,7 +526,7 @@ func runLabReset(cmd *cobra.Command, args []string) error { if !skipProvision { fmt.Println("--- Stage 2: restore AD baseline state ---") - if err := provisionPlaybooks(ctx, cfg, playbooks, limit, maxRetries, retryDelay, extraVars); err != nil { + if err := provisionPlaybooks(ctx, cfg, playbooks, limit, retry, extraVars); err != nil { return err } } diff --git a/cli/cmd/provision.go b/cli/cmd/provision.go index fafd7abd..62b35e77 100644 --- a/cli/cmd/provision.go +++ b/cli/cmd/provision.go @@ -2,6 +2,7 @@ package cmd import ( "context" + "errors" "fmt" "log/slog" "os" @@ -62,15 +63,15 @@ func init() { provisionCmd.Flags().String("plays", "", "Comma-separated playbooks to run (default: all)") provisionCmd.Flags().String("from", "", "Resume provisioning from this playbook onward") provisionCmd.Flags().String("limit", "", "Limit execution to specific hosts") - provisionCmd.Flags().Int("max-retries", 0, "Max retry attempts (default: from config)") - provisionCmd.Flags().Int("retry-delay", 0, "Delay between retries in seconds (default: from config)") + provisionCmd.Flags().Int("max-retries", 0, "Max retry attempts (default: from config; 0 disables retries)") + provisionCmd.Flags().Int("retry-delay", 0, "Delay between retries in seconds (default: from config; 0 disables delay)") provisionCmd.Flags().StringArrayP("extra-vars", "E", nil, extraVarsUsage) provisionCmd.MarkFlagsMutuallyExclusive("plays", "from") adUsersCmd.Flags().String("plays", "ad-data.yml", "Playbooks to run") adUsersCmd.Flags().String("limit", "", "Limit execution to specific hosts") - adUsersCmd.Flags().Int("max-retries", 0, "Max retry attempts") - adUsersCmd.Flags().Int("retry-delay", 0, "Delay between retries in seconds") + adUsersCmd.Flags().Int("max-retries", 0, "Max retry attempts (0 disables retries)") + adUsersCmd.Flags().Int("retry-delay", 0, "Delay between retries in seconds (0 disables delay)") adUsersCmd.Flags().StringArrayP("extra-vars", "E", nil, extraVarsUsage) } @@ -112,10 +113,24 @@ func ensureVariant(cfg *config.Config) error { if variantName == "" { variantName = "variant-1" } - if _, err := os.Stat(target); !os.IsNotExist(err) { + info, err := os.Stat(target) + if err == nil { + if !info.IsDir() { + return fmt.Errorf("variant target exists but is not a directory: %s", target) + } + complete, err := variant.IsComplete(target) + if err != nil { + return fmt.Errorf("inspect variant target %s: %w", target, err) + } + if !complete { + return fmt.Errorf("variant directory is incomplete (missing %s): %s; move or remove it, then rerun provisioning", variant.CompletionMarkerName, target) + } slog.Info("Variant directory already exists, skipping generation", "target", target) return nil } + if !errors.Is(err, os.ErrNotExist) { + return fmt.Errorf("inspect variant target %s: %w", target, err) + } fmt.Printf("Environment %q has variant=true, generating variant...\n", cfg.Env) gen := variant.NewGenerator(source, target, variantName) if err := gen.Run(); err != nil { @@ -337,14 +352,58 @@ func runProvision(cmd *cobra.Command, args []string) error { } limit, _ := cmd.Flags().GetString("limit") - maxRetries, _ := cmd.Flags().GetInt("max-retries") - retryDelay, _ := cmd.Flags().GetInt("retry-delay") + retry, err := retryOverridesFromFlags(cmd) + if err != nil { + return err + } extraVars, err := parseExtraVars(cmd) if err != nil { return err } - return provisionPlaybooks(ctx, cfg, playbooks, limit, maxRetries, retryDelay, extraVars) + return provisionPlaybooks(ctx, cfg, playbooks, limit, retry, extraVars) +} + +type retryOverrides struct { + maxRetries *int + retryDelay *int +} + +func retryOverridesFromFlags(cmd *cobra.Command) (retryOverrides, error) { + maxRetries, err := optionalNonNegativeIntFlag(cmd, "max-retries") + if err != nil { + return retryOverrides{}, err + } + retryDelay, err := optionalNonNegativeIntFlag(cmd, "retry-delay") + if err != nil { + return retryOverrides{}, err + } + return retryOverrides{maxRetries: maxRetries, retryDelay: retryDelay}, nil +} + +func optionalNonNegativeIntFlag(cmd *cobra.Command, name string) (*int, error) { + if !cmd.Flags().Changed(name) { + return nil, nil + } + value, err := cmd.Flags().GetInt(name) + if err != nil { + return nil, fmt.Errorf("read --%s: %w", name, err) + } + if value < 0 { + return nil, fmt.Errorf("--%s must be zero or greater", name) + } + return &value, nil +} + +func (r retryOverrides) apply(opts *ansible.RetryOptions) { + if r.maxRetries != nil { + opts.MaxRetries = *r.maxRetries + opts.MaxRetriesSet = true + } + if r.retryDelay != nil { + opts.RetryDelay = time.Duration(*r.retryDelay) * time.Second + opts.RetryDelaySet = true + } } // parseExtraVars reads the repeatable --extra-vars flag into the map the @@ -400,9 +459,23 @@ func sortedPairs(m map[string]string) []string { return out } +type provisionFailure struct { + Playbook string + LogFile string + Err error +} + +func (e *provisionFailure) Error() string { + return fmt.Sprintf("provisioning failed at %s: %v\n see full log: %s", e.Playbook, e.Err, e.LogFile) +} + +func (e *provisionFailure) Unwrap() error { + return e.Err +} + // provisionPlaybooks runs preflight checks then executes the given playbooks // with retry logic. Shared between `provision` and `lab reset`. -func provisionPlaybooks(ctx context.Context, cfg *config.Config, playbooks []string, limit string, maxRetries, retryDelay int, extraVars map[string]string) error { +func provisionPlaybooks(ctx context.Context, cfg *config.Config, playbooks []string, limit string, retry retryOverrides, extraVars map[string]string) error { _ = os.MkdirAll(cfg.LogDir, 0o755) logFile := filepath.Join(cfg.LogDir, fmt.Sprintf("%s-dreadgoad-%s.log", cfg.Env, time.Now().Format("20060102_150405"))) @@ -461,16 +534,11 @@ func provisionPlaybooks(ctx context.Context, cfg *config.Config, playbooks []str LogFile: logFile, ExtraVars: runVars, } - if maxRetries > 0 { - opts.MaxRetries = maxRetries - } - if retryDelay > 0 { - opts.RetryDelay = time.Duration(retryDelay) * time.Second - } + retry.apply(&opts) if err := ansible.RunPlaybookWithRetry(ctx, opts); err != nil { log.Error("provisioning failed", "playbook", playbook, "log_file", logFile, "error", err) - return fmt.Errorf("provisioning failed at %s: %w\n see full log: %s", playbook, err, logFile) + return &provisionFailure{Playbook: playbook, LogFile: logFile, Err: err} } // Between playbooks: clean up accumulated SSM sessions and wait diff --git a/cli/cmd/provision_test.go b/cli/cmd/provision_test.go index 003459dd..3c7c984b 100644 --- a/cli/cmd/provision_test.go +++ b/cli/cmd/provision_test.go @@ -1,11 +1,201 @@ package cmd import ( + "errors" + "fmt" + "os" + "path/filepath" + "strings" "testing" + "time" + "github.com/dreadnode/dreadgoad/internal/ansible" + "github.com/dreadnode/dreadgoad/internal/config" + "github.com/dreadnode/dreadgoad/internal/variant" "github.com/spf13/cobra" ) +func TestProvisionFailureCarriesResumeDetails(t *testing.T) { + cause := errors.New("ansible failed") + err := &provisionFailure{ + Playbook: "ad-data.yml", + LogFile: "/tmp/provision.log", + Err: cause, + } + + want := "provisioning failed at ad-data.yml: ansible failed\n see full log: /tmp/provision.log" + if err.Error() != want { + t.Errorf("Error() = %q, want %q", err.Error(), want) + } + if !errors.Is(err, cause) { + t.Error("provisionFailure does not unwrap to its cause") + } + var got *provisionFailure + if !errors.As(fmt.Errorf("outer: %w", err), &got) || got.Playbook != "ad-data.yml" || got.LogFile != "/tmp/provision.log" { + t.Errorf("errors.As() = %#v, want structured failure details", got) + } +} + +func retryFlagsCommand() *cobra.Command { + cmd := &cobra.Command{} + cmd.Flags().Int("max-retries", 0, "") + cmd.Flags().Int("retry-delay", 0, "") + return cmd +} + +func TestRetryOverridesDistinguishOmittedAndExplicitZero(t *testing.T) { + omitted, err := retryOverridesFromFlags(retryFlagsCommand()) + if err != nil { + t.Fatalf("omitted flags: %v", err) + } + if omitted.maxRetries != nil || omitted.retryDelay != nil { + t.Fatalf("omitted flags produced overrides: %#v", omitted) + } + + cmd := retryFlagsCommand() + for _, name := range []string{"max-retries", "retry-delay"} { + if err := cmd.Flags().Set(name, "0"); err != nil { + t.Fatalf("set --%s: %v", name, err) + } + } + explicit, err := retryOverridesFromFlags(cmd) + if err != nil { + t.Fatalf("explicit zero flags: %v", err) + } + if explicit.maxRetries == nil || *explicit.maxRetries != 0 || explicit.retryDelay == nil || *explicit.retryDelay != 0 { + t.Fatalf("explicit zero flags lost: %#v", explicit) + } + + var opts ansible.RetryOptions + explicit.apply(&opts) + if opts.MaxRetries != 0 || !opts.MaxRetriesSet { + t.Errorf("MaxRetries = %d, set=%v; want 0,true", opts.MaxRetries, opts.MaxRetriesSet) + } + if opts.RetryDelay != 0 || !opts.RetryDelaySet { + t.Errorf("RetryDelay = %s, set=%v; want 0,true", opts.RetryDelay, opts.RetryDelaySet) + } +} + +func TestRetryOverridesForwardPositiveValues(t *testing.T) { + cmd := retryFlagsCommand() + if err := cmd.Flags().Set("max-retries", "5"); err != nil { + t.Fatal(err) + } + if err := cmd.Flags().Set("retry-delay", "12"); err != nil { + t.Fatal(err) + } + retry, err := retryOverridesFromFlags(cmd) + if err != nil { + t.Fatal(err) + } + + var opts ansible.RetryOptions + retry.apply(&opts) + if opts.MaxRetries != 5 || !opts.MaxRetriesSet || opts.RetryDelay != 12*time.Second || !opts.RetryDelaySet { + t.Errorf("applied retry options = %#v", opts) + } +} + +func TestRetryOverridesRejectNegativeValues(t *testing.T) { + for _, name := range []string{"max-retries", "retry-delay"} { + t.Run(name, func(t *testing.T) { + cmd := retryFlagsCommand() + if err := cmd.Flags().Set(name, "-1"); err != nil { + t.Fatal(err) + } + _, err := retryOverridesFromFlags(cmd) + if err == nil || !strings.Contains(err.Error(), "must be zero or greater") { + t.Errorf("error = %v, want non-negative validation", err) + } + }) + } +} + +func variantTestConfig(root, source, target string) *config.Config { + return &config.Config{ + ProjectRoot: root, + Env: "dev", + Environments: map[string]config.EnvironmentConfig{ + "dev": { + Variant: true, + VariantSource: source, + VariantTarget: target, + }, + }, + } +} + +func TestEnsureVariantReusesCompleteTarget(t *testing.T) { + target := t.TempDir() + if err := os.WriteFile(filepath.Join(target, variant.CompletionMarkerName), []byte("complete\n"), 0o644); err != nil { + t.Fatal(err) + } + + if err := ensureVariant(variantTestConfig(t.TempDir(), "unused", target)); err != nil { + t.Fatalf("ensureVariant() error: %v", err) + } +} + +func TestEnsureVariantRejectsIncompleteTarget(t *testing.T) { + target := t.TempDir() + + err := ensureVariant(variantTestConfig(t.TempDir(), "unused", target)) + if err == nil || !strings.Contains(err.Error(), "variant directory is incomplete") || + !strings.Contains(err.Error(), variant.CompletionMarkerName) { + t.Fatalf("ensureVariant() error = %v, want incomplete variant error", err) + } +} + +func TestEnsureVariantRejectsNonDirectoryTarget(t *testing.T) { + root := t.TempDir() + target := filepath.Join(root, "variant") + if err := os.WriteFile(target, []byte("not a directory"), 0o644); err != nil { + t.Fatal(err) + } + + err := ensureVariant(variantTestConfig(root, "unused", target)) + if err == nil || !strings.Contains(err.Error(), "not a directory") { + t.Fatalf("ensureVariant() error = %v, want non-directory error", err) + } +} + +func TestEnsureVariantReturnsTargetInspectionError(t *testing.T) { + root := t.TempDir() + parent := filepath.Join(root, "not-a-directory") + if err := os.WriteFile(parent, []byte("file"), 0o644); err != nil { + t.Fatal(err) + } + target := filepath.Join(parent, "variant") + + err := ensureVariant(variantTestConfig(root, "unused", target)) + if err == nil || !strings.Contains(err.Error(), "inspect variant target") { + t.Fatalf("ensureVariant() error = %v, want inspection error", err) + } +} + +func TestEnsureVariantGeneratesMissingTargetAndMarksComplete(t *testing.T) { + root := t.TempDir() + source := filepath.Join(root, "source") + target := filepath.Join(root, "target") + if err := os.MkdirAll(filepath.Join(source, "data"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(source, "data", "config.json"), []byte(`{"lab":{"hosts":{},"domains":{}}}`), 0o644); err != nil { + t.Fatal(err) + } + + if err := ensureVariant(variantTestConfig(root, source, target)); err != nil { + t.Fatalf("ensureVariant() error: %v", err) + } + complete, err := variant.IsComplete(target) + if err != nil { + t.Fatalf("check completion marker: %v", err) + } + if !complete { + t.Fatal("generated variant has no completion marker") + } +} + func extraVarsCmd(t *testing.T, args ...string) *cobra.Command { t.Helper() c := &cobra.Command{Use: "test", RunE: func(*cobra.Command, []string) error { return nil }} diff --git a/cli/cmd/up.go b/cli/cmd/up.go index cb8a4af7..cf8e8ce6 100644 --- a/cli/cmd/up.go +++ b/cli/cmd/up.go @@ -1,6 +1,8 @@ package cmd import ( + "context" + "errors" "fmt" "strconv" "strings" @@ -19,6 +21,7 @@ var ( upPlays string upMaxRetries int upRetryDelay int + upFromPlaybook string upInfraModule string upInfraExclude string ) @@ -40,6 +43,7 @@ to restart from a specific point. The recommended new-user flow is: Example: ` dreadgoad up dreadgoad up --skip-doctor dreadgoad up --from provision + dreadgoad up --from provision --from-playbook ad-data.yml dreadgoad up --limit dc01`, RunE: runUp, } @@ -51,8 +55,9 @@ func init() { upCmd.Flags().StringVar(&upFromStep, "from", "", "Resume from this step (doctor, infra, provision, health-check)") upCmd.Flags().StringVar(&upLimit, "limit", "", "Limit provisioning to specific hosts") upCmd.Flags().StringVar(&upPlays, "plays", "", "Comma-separated playbooks to run (default: all)") - upCmd.Flags().IntVar(&upMaxRetries, "max-retries", 0, "Max retry attempts for provisioning") - upCmd.Flags().IntVar(&upRetryDelay, "retry-delay", 0, "Delay between retries in seconds") + upCmd.Flags().IntVar(&upMaxRetries, "max-retries", 0, "Max retry attempts for provisioning (0 disables retries)") + upCmd.Flags().IntVar(&upRetryDelay, "retry-delay", 0, "Delay between retries in seconds (0 disables delay)") + upCmd.Flags().StringVar(&upFromPlaybook, "from-playbook", "", "Resume provisioning from this playbook onward") upCmd.Flags().StringVar(&upInfraModule, "module", "", "Target a specific infra module (default: all)") upCmd.Flags().StringVar(&upInfraExclude, "exclude", "", "Exclude infra modules (comma-separated)") } @@ -90,6 +95,10 @@ func runUp(cmd *cobra.Command, args []string) error { } else if upSkipDoctor { steps = steps[1:] } + if err := validateUpProvisionResume(steps, upPlays, upFromPlaybook); err != nil { + return err + } + resumeOptions := currentUpResumeOptions(cmd) total := len(steps) start := time.Now() @@ -98,7 +107,7 @@ func runUp(cmd *cobra.Command, args []string) error { if err := step.run(cmd, args); err != nil { fmt.Println() color.Red("✗ %s failed: %v", step.name, err) - color.Yellow(" Resume with: dreadgoad up --from %s", step.id) + color.Yellow(" Resume with: %s", upResumeCommand(step.id, err, resumeOptions)) return err } } @@ -109,6 +118,103 @@ func runUp(cmd *cobra.Command, args []string) error { return nil } +func validateUpProvisionResume(steps []upStep, plays, fromPlaybook string) error { + if fromPlaybook == "" { + return nil + } + if plays != "" { + return fmt.Errorf("--from-playbook cannot be combined with --plays") + } + for _, step := range steps { + if step.id == "provision" { + return nil + } + } + return fmt.Errorf("--from-playbook cannot be used when the provision step is skipped") +} + +type upResumeOptions struct { + plays string + fromPlaybook string + limit string + infraModule string + infraExclude string + retry retryOverrides +} + +func currentUpResumeOptions(cmd *cobra.Command) upResumeOptions { + opts := upResumeOptions{ + plays: upPlays, + fromPlaybook: upFromPlaybook, + limit: upLimit, + infraModule: upInfraModule, + infraExclude: upInfraExclude, + } + if cmd.Flags().Changed("max-retries") { + value := upMaxRetries + opts.retry.maxRetries = &value + } + if cmd.Flags().Changed("retry-delay") { + value := upRetryDelay + opts.retry.retryDelay = &value + } + return opts +} + +func upResumeCommand(stepID string, err error, opts upResumeOptions) string { + command := fmt.Sprintf("dreadgoad up --from %s", stepID) + if stepID == "health-check" { + return command + } + + if stepID == "doctor" || stepID == "infra" { + if opts.infraModule != "" { + command += " --module " + shellQuoteResumeArg(opts.infraModule) + } + if opts.infraExclude != "" { + command += " --exclude " + shellQuoteResumeArg(opts.infraExclude) + } + } + + var failure *provisionFailure + switch { + case stepID == "provision" && errors.As(err, &failure) && failure.Playbook != "": + if opts.plays != "" { + command += " --plays " + shellQuoteResumeArg(remainingPlaybookSelection(opts.plays, failure.Playbook)) + } else { + command += " --from-playbook " + shellQuoteResumeArg(failure.Playbook) + } + case opts.plays != "": + command += " --plays " + shellQuoteResumeArg(opts.plays) + case opts.fromPlaybook != "": + command += " --from-playbook " + shellQuoteResumeArg(opts.fromPlaybook) + } + if opts.limit != "" { + command += " --limit " + shellQuoteResumeArg(opts.limit) + } + if opts.retry.maxRetries != nil { + command += " --max-retries " + strconv.Itoa(*opts.retry.maxRetries) + } + if opts.retry.retryDelay != nil { + command += " --retry-delay " + strconv.Itoa(*opts.retry.retryDelay) + } + return command +} + +func remainingPlaybookSelection(selected, failed string) string { + playbooks := strings.Split(selected, ",") + for i, playbook := range playbooks { + if playbook == failed { + return strings.Join(playbooks[i:], ",") + } + } + return selected +} + +func shellQuoteResumeArg(value string) string { + return "'" + strings.ReplaceAll(value, "'", `'"'"'`) + "'" +} + func printUpHeader(step, total int, name string) { line := strings.Repeat("━", 60) fmt.Println() @@ -141,11 +247,15 @@ func runUpDoctor(cmd *cobra.Command, _ []string) error { }, }) if failed := doctor.PrintResults(results); failed > 0 { - return fmt.Errorf("%d pre-flight check(s) failed (re-run 'dreadgoad doctor' for details, or pass --skip-doctor to bypass)", failed) + return upDoctorFailure(failed) } return nil } +func upDoctorFailure(failed int) error { + return fmt.Errorf("%d pre-flight check(s) failed; run 'dreadgoad doctor' for details, fix the reported issues, then retry 'dreadgoad up'", failed) +} + // runUpInfraApply invokes `infra apply` with auto-approve. We build a // synthetic cobra.Command so the inner action sees only the flags we want // (auto-approve=true, module/exclude pass-through) without conflating with @@ -161,29 +271,78 @@ func runUpInfraApply(cmd *cobra.Command, args []string) error { return runInfraAction("apply")(infraCmd, args) } -// runUpProvision calls runProvision via a synthetic command so up's --from -// (which is the step name) is not mistakenly read as the playbook-resume -// flag of the provision subcommand. -func runUpProvision(cmd *cobra.Command, args []string) error { +type upProvisionOptions struct { + plays string + fromPlaybook string + limit string + retry retryOverrides +} + +// newUpProvisionCommand builds the synthetic command that `up` uses to invoke +// provisioning. Every flag read by runProvision must be registered here so a +// missing flag cannot silently turn into its zero value. The structural test in +// up_test.go keeps this flag set aligned with the real provision command. +func newUpProvisionCommand(ctx context.Context, opts upProvisionOptions) (*cobra.Command, error) { provCmd := &cobra.Command{} provCmd.Flags().String("plays", "", "") provCmd.Flags().String("from", "", "") provCmd.Flags().String("limit", "", "") provCmd.Flags().Int("max-retries", 0, "") provCmd.Flags().Int("retry-delay", 0, "") - if upPlays != "" { - _ = provCmd.Flags().Set("plays", upPlays) + provCmd.Flags().StringArray("extra-vars", nil, "") + + setFlag := func(name, value string) error { + if err := provCmd.Flags().Set(name, value); err != nil { + return fmt.Errorf("configure synthetic provision flag --%s: %w", name, err) + } + return nil + } + if opts.plays != "" { + if err := setFlag("plays", opts.plays); err != nil { + return nil, err + } + } + if opts.fromPlaybook != "" { + if err := setFlag("from", opts.fromPlaybook); err != nil { + return nil, err + } } - if upLimit != "" { - _ = provCmd.Flags().Set("limit", upLimit) + if opts.limit != "" { + if err := setFlag("limit", opts.limit); err != nil { + return nil, err + } + } + if opts.retry.maxRetries != nil { + if err := setFlag("max-retries", strconv.Itoa(*opts.retry.maxRetries)); err != nil { + return nil, err + } + } + if opts.retry.retryDelay != nil { + if err := setFlag("retry-delay", strconv.Itoa(*opts.retry.retryDelay)); err != nil { + return nil, err + } } - if upMaxRetries > 0 { - _ = provCmd.Flags().Set("max-retries", strconv.Itoa(upMaxRetries)) + provCmd.SetContext(ctx) + return provCmd, nil +} + +// runUpProvision calls runProvision via a synthetic command so up's --from +// (which is the step name) is not mistakenly read as the playbook-resume +// flag of the provision subcommand. +func runUpProvision(cmd *cobra.Command, args []string) error { + retry, err := retryOverridesFromFlags(cmd) + if err != nil { + return err } - if upRetryDelay > 0 { - _ = provCmd.Flags().Set("retry-delay", strconv.Itoa(upRetryDelay)) + provCmd, err := newUpProvisionCommand(cmd.Context(), upProvisionOptions{ + plays: upPlays, + fromPlaybook: upFromPlaybook, + limit: upLimit, + retry: retry, + }) + if err != nil { + return err } - provCmd.SetContext(cmd.Context()) return runProvision(provCmd, args) } diff --git a/cli/cmd/up_test.go b/cli/cmd/up_test.go new file mode 100644 index 00000000..d31b250f --- /dev/null +++ b/cli/cmd/up_test.go @@ -0,0 +1,264 @@ +package cmd + +import ( + "context" + "errors" + "fmt" + "strings" + "testing" + + "github.com/dreadnode/dreadgoad/internal/config" + "github.com/spf13/cobra" + "github.com/spf13/pflag" +) + +func TestUpProvisionCommandRegistersEveryProvisionFlag(t *testing.T) { + synth, err := newUpProvisionCommand(context.Background(), upProvisionOptions{}) + if err != nil { + t.Fatalf("newUpProvisionCommand() error: %v", err) + } + + provisionCmd.Flags().VisitAll(func(flag *pflag.Flag) { + if synth.Flags().Lookup(flag.Name) == nil { + t.Errorf("up does not register provision flag --%s", flag.Name) + } + }) +} + +func TestUpProvisionCommandForwardsValues(t *testing.T) { + maxRetries, retryDelay := 4, 45 + synth, err := newUpProvisionCommand(context.Background(), upProvisionOptions{ + fromPlaybook: "ad-data.yml", + limit: "dc01", + retry: retryOverrides{ + maxRetries: &maxRetries, + retryDelay: &retryDelay, + }, + }) + if err != nil { + t.Fatalf("newUpProvisionCommand() error: %v", err) + } + + assertStringFlag(t, synth, "plays", "") + assertStringFlag(t, synth, "from", "ad-data.yml") + assertStringFlag(t, synth, "limit", "dc01") + assertIntFlag(t, synth, "max-retries", 4) + assertIntFlag(t, synth, "retry-delay", 45) + + extraVars, err := synth.Flags().GetStringArray("extra-vars") + if err != nil { + t.Fatalf("get --extra-vars: %v", err) + } + if len(extraVars) != 0 { + t.Errorf("--extra-vars = %v, want empty", extraVars) + } +} + +func TestUpProvisionCommandForwardsExplicitZeroRetryFlags(t *testing.T) { + zero := 0 + synth, err := newUpProvisionCommand(context.Background(), upProvisionOptions{ + retry: retryOverrides{maxRetries: &zero, retryDelay: &zero}, + }) + if err != nil { + t.Fatalf("newUpProvisionCommand() error: %v", err) + } + + for _, name := range []string{"max-retries", "retry-delay"} { + assertIntFlag(t, synth, name, 0) + if !synth.Flags().Changed(name) { + t.Errorf("--%s explicit zero was not marked as set", name) + } + } +} + +func TestUpProvisionResumeResolvesPlaybookSuffix(t *testing.T) { + synth, err := newUpProvisionCommand(context.Background(), upProvisionOptions{ + fromPlaybook: "ad-data.yml", + }) + if err != nil { + t.Fatalf("newUpProvisionCommand() error: %v", err) + } + + from, err := synth.Flags().GetString("from") + if err != nil { + t.Fatalf("get --from: %v", err) + } + cfg := &config.Config{ + ProjectRoot: t.TempDir(), + Playbooks: []string{"build.yml", "ad-servers.yml", "ad-data.yml", "vulnerabilities.yml"}, + } + got, err := resolvePlaybooks(cfg, "", from) + if err != nil { + t.Fatalf("resolvePlaybooks() error: %v", err) + } + want := []string{"ad-data.yml", "vulnerabilities.yml"} + if fmt.Sprint(got) != fmt.Sprint(want) { + t.Errorf("resolved playbooks = %v, want %v", got, want) + } +} + +func TestValidateUpProvisionResume(t *testing.T) { + withProvision := []upStep{{id: "provision"}, {id: "health-check"}} + withoutProvision := []upStep{{id: "health-check"}} + + if err := validateUpProvisionResume(withProvision, "", "ad-data.yml"); err != nil { + t.Errorf("valid resume rejected: %v", err) + } + if err := validateUpProvisionResume(withoutProvision, "", ""); err != nil { + t.Errorf("empty --from-playbook should be ignored: %v", err) + } + + tests := []struct { + name string + steps []upStep + plays string + want string + }{ + { + name: "plays conflict", + steps: withProvision, + plays: "ad-data.yml", + want: "cannot be combined with --plays", + }, + { + name: "provision skipped", + steps: withoutProvision, + want: "provision step is skipped", + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + err := validateUpProvisionResume(tc.steps, tc.plays, "ad-data.yml") + if err == nil || !strings.Contains(err.Error(), tc.want) { + t.Errorf("error = %v, want text %q", err, tc.want) + } + }) + } +} + +func TestUpResumeCommandNamesFailedPlaybook(t *testing.T) { + cause := errors.New("ansible failed") + failure := &provisionFailure{ + Playbook: "ad-data.yml", + LogFile: "/tmp/provision.log", + Err: cause, + } + + got := upResumeCommand("provision", fmt.Errorf("wrapped: %w", failure), upResumeOptions{}) + want := "dreadgoad up --from provision --from-playbook 'ad-data.yml'" + if got != want { + t.Errorf("resume command = %q, want %q", got, want) + } + + if got := upResumeCommand("provision", cause, upResumeOptions{}); got != "dreadgoad up --from provision" { + t.Errorf("generic provision resume command = %q", got) + } + if got := upResumeCommand("infra", failure, upResumeOptions{}); got != "dreadgoad up --from infra" { + t.Errorf("infra resume command = %q", got) + } +} + +func TestUpResumeCommandPreservesRemainingCustomPlaybooks(t *testing.T) { + failure := &provisionFailure{ + Playbook: "custom-data.yml", + LogFile: "/tmp/provision.log", + Err: errors.New("ansible failed"), + } + + got := upResumeCommand( + "provision", + failure, + upResumeOptions{plays: "bootstrap.yml,custom-data.yml,custom vulnerabilities.yml"}, + ) + want := "dreadgoad up --from provision --plays 'custom-data.yml,custom vulnerabilities.yml'" + if got != want { + t.Errorf("resume command = %q, want %q", got, want) + } +} + +func TestUpResumeCommandPreservesExecutionOverrides(t *testing.T) { + zero, delay := 0, 12 + failure := &provisionFailure{ + Playbook: "ad-data.yml", + LogFile: "/tmp/provision.log", + Err: errors.New("ansible failed"), + } + opts := upResumeOptions{ + limit: "dc01,DC 02", + retry: retryOverrides{ + maxRetries: &zero, + retryDelay: &delay, + }, + } + + got := upResumeCommand("provision", failure, opts) + want := "dreadgoad up --from provision --from-playbook 'ad-data.yml' --limit 'dc01,DC 02' --max-retries 0 --retry-delay 12" + if got != want { + t.Errorf("resume command = %q, want %q", got, want) + } +} + +func TestUpResumeCommandPreservesOverridesBeforeProvisioning(t *testing.T) { + zero := 0 + opts := upResumeOptions{ + plays: "build.yml,ad-data.yml", + limit: "dc01", + infraModule: "network", + infraExclude: "bastion,monitoring", + retry: retryOverrides{maxRetries: &zero}, + } + + got := upResumeCommand("infra", errors.New("terraform failed"), opts) + want := "dreadgoad up --from infra --module 'network' --exclude 'bastion,monitoring' --plays 'build.yml,ad-data.yml' --limit 'dc01' --max-retries 0" + if got != want { + t.Errorf("resume command = %q, want %q", got, want) + } + + got = upResumeCommand("health-check", errors.New("check failed"), opts) + if want := "dreadgoad up --from health-check"; got != want { + t.Errorf("health-check resume command = %q, want %q", got, want) + } +} + +func TestShellQuoteResumeArg(t *testing.T) { + got := shellQuoteResumeArg("first.yml,operator's.yml") + want := `'first.yml,operator'"'"'s.yml'` + if got != want { + t.Errorf("shellQuoteResumeArg() = %q, want %q", got, want) + } +} + +func TestUpDoctorFailureDoesNotRecommendBypass(t *testing.T) { + err := upDoctorFailure(2) + message := err.Error() + for _, want := range []string{"2 pre-flight check(s) failed", "dreadgoad doctor", "retry 'dreadgoad up'"} { + if !strings.Contains(message, want) { + t.Errorf("error %q does not contain %q", message, want) + } + } + if strings.Contains(message, "skip-doctor") || strings.Contains(message, "bypass") { + t.Errorf("doctor failure recommends bypassing checks: %q", message) + } +} + +func assertStringFlag(t *testing.T, cmd *cobra.Command, name, want string) { + t.Helper() + got, err := cmd.Flags().GetString(name) + if err != nil { + t.Fatalf("get --%s: %v", name, err) + } + if got != want { + t.Errorf("--%s = %q, want %q", name, got, want) + } +} + +func assertIntFlag(t *testing.T, cmd *cobra.Command, name string, want int) { + t.Helper() + got, err := cmd.Flags().GetInt(name) + if err != nil { + t.Fatalf("get --%s: %v", name, err) + } + if got != want { + t.Errorf("--%s = %d, want %d", name, got, want) + } +} diff --git a/cli/go.mod b/cli/go.mod index b40c41f3..5216fb10 100644 --- a/cli/go.mod +++ b/cli/go.mod @@ -19,6 +19,7 @@ require ( github.com/fatih/color v1.19.0 github.com/masterzen/winrm v0.0.0-20260407182533-5570be7f80cf github.com/spf13/cobra v1.10.2 + github.com/spf13/pflag v1.0.10 github.com/spf13/viper v1.21.0 github.com/stretchr/testify v1.11.1 go.yaml.in/yaml/v3 v3.0.5 @@ -99,7 +100,6 @@ require ( github.com/sirupsen/logrus v1.9.4 // indirect github.com/spf13/afero v1.15.0 // indirect github.com/spf13/cast v1.10.0 // indirect - github.com/spf13/pflag v1.0.10 // indirect github.com/subosito/gotenv v1.6.0 // indirect github.com/tidwall/transform v0.0.0-20201103190739-32f242e2dbde // indirect github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e // indirect diff --git a/cli/internal/ansible/retry.go b/cli/internal/ansible/retry.go index a15b6f86..679eb4db 100644 --- a/cli/internal/ansible/retry.go +++ b/cli/internal/ansible/retry.go @@ -16,18 +16,21 @@ import ( // RetryOptions configures the retry behavior for a [RunPlaybookWithRetry] call. // MaxRetries and RetryDelay default to the values from the global [config.Config] -// when left as zero. +// when left as zero and their corresponding Set field is false. MaxRetries=0 +// with MaxRetriesSet=true means run once without retrying. type RetryOptions struct { - Playbook string - Env string - Inventories []string // additional inventory paths - ExtraVars map[string]string // extra variables passed to ansible-playbook - Limit string - Debug bool - MaxRetries int - RetryDelay time.Duration - LogFile string - Log *slog.Logger // optional; falls back to slog.Default() + Playbook string + Env string + Inventories []string // additional inventory paths + ExtraVars map[string]string // extra variables passed to ansible-playbook + Limit string + Debug bool + MaxRetries int + MaxRetriesSet bool + RetryDelay time.Duration + RetryDelaySet bool + LogFile string + Log *slog.Logger // optional; falls back to slog.Default() } func (o *RetryOptions) logger() *slog.Logger { @@ -37,6 +40,8 @@ func (o *RetryOptions) logger() *slog.Logger { return slog.Default() } +var runPlaybookAttempt = RunPlaybook + // RunPlaybookWithRetry runs an Ansible playbook with error-specific retry logic. // On each failure it classifies the error via [DetectErrorType] and applies a // targeted recovery strategy (e.g. SSM session cleanup, host reboots) before @@ -49,12 +54,10 @@ func RunPlaybookWithRetry(ctx context.Context, opts RetryOptions) error { } log := opts.logger() - if opts.MaxRetries == 0 { - opts.MaxRetries = cfg.MaxRetries - } - if opts.RetryDelay == 0 { - opts.RetryDelay = time.Duration(cfg.RetryDelay) * time.Second - } + var retriesDisabled bool + opts.MaxRetries, opts.RetryDelay, retriesDisabled = resolveRetrySettings( + opts, cfg.MaxRetries, time.Duration(cfg.RetryDelay)*time.Second, + ) retryForks := 2 // limit SSM concurrency to avoid session saturation for attempt := range opts.MaxRetries { @@ -78,7 +81,7 @@ func RunPlaybookWithRetry(ctx context.Context, opts RetryOptions) error { log.Info("starting playbook", "playbook", opts.Playbook, "attempt", attempt+1, "max", opts.MaxRetries) - result := RunPlaybook(ctx, RunOptions{ + result := runPlaybookAttempt(ctx, RunOptions{ Playbook: opts.Playbook, Env: opts.Env, Inventories: opts.Inventories, @@ -104,6 +107,9 @@ func RunPlaybookWithRetry(ctx context.Context, opts RetryOptions) error { log.Warn("playbook failed", "playbook", opts.Playbook, "error_type", result.ErrorType, "detail", result.ErrorDetail, "failed_hosts", result.FailedHosts) + if retriesDisabled { + continue + } retryResult := retryWithErrorStrategy(ctx, opts, result, log) if retryResult != nil && retryResult.Success { @@ -115,6 +121,25 @@ func RunPlaybookWithRetry(ctx context.Context, opts RetryOptions) error { return fmt.Errorf("playbook %s failed after %d attempts", opts.Playbook, opts.MaxRetries) } +func resolveRetrySettings(opts RetryOptions, configuredMaxRetries int, configuredRetryDelay time.Duration) (int, time.Duration, bool) { + maxAttempts := opts.MaxRetries + if !opts.MaxRetriesSet && maxAttempts == 0 { + maxAttempts = configuredMaxRetries + } + retriesDisabled := maxAttempts <= 0 + // Zero retries still means one initial playbook attempt. Treat a negative + // configured value the same way; CLI negatives are rejected before here. + if retriesDisabled { + maxAttempts = 1 + } + + retryDelay := opts.RetryDelay + if !opts.RetryDelaySet && retryDelay == 0 { + retryDelay = configuredRetryDelay + } + return maxAttempts, retryDelay, retriesDisabled +} + func retryWithErrorStrategy(ctx context.Context, opts RetryOptions, failResult *RunResult, log *slog.Logger) *RunResult { failedHostsStr := strings.Join(failResult.FailedHosts, ",") limit := buildRetryLimit(opts.Limit, failedHostsStr) @@ -140,7 +165,7 @@ func retryWithErrorStrategy(ctx context.Context, opts RetryOptions, failResult * baseOpts.ExtraEnv = map[string]string{ "ANSIBLE_GATHERING": "explicit", } - return RunPlaybook(ctx, baseOpts) + return runPlaybookAttempt(ctx, baseOpts) case ErrNetworkAdapter: log.Info("retrying with network adapter fix") @@ -148,7 +173,7 @@ func retryWithErrorStrategy(ctx context.Context, opts RetryOptions, failResult * "skip_network_adapter_config": "true", "bypass_ethernet3_check": "true", }) - return RunPlaybook(ctx, baseOpts) + return runPlaybookAttempt(ctx, baseOpts) case ErrSSMTransfer: log.Info("SSM transfer error - fixing ssm-user accounts") @@ -166,7 +191,7 @@ func retryWithErrorStrategy(ctx context.Context, opts RetryOptions, failResult * "ansible_aws_ssm_timeout": "300", }) baseOpts.ExtraEnv = map[string]string{"ANSIBLE_TIMEOUT": "300"} - return RunPlaybook(ctx, baseOpts) + return runPlaybookAttempt(ctx, baseOpts) case ErrSSMReconnection: log.Info("SSM reconnection needed - waiting for systems to reboot") @@ -184,7 +209,7 @@ func retryWithErrorStrategy(ctx context.Context, opts RetryOptions, failResult * "ansible_facts_gathering_timeout": "60", }) baseOpts.ExtraEnv = map[string]string{"ANSIBLE_TIMEOUT": "180"} - return RunPlaybook(ctx, baseOpts) + return runPlaybookAttempt(ctx, baseOpts) case ErrPowerShell: log.Info("retrying with PowerShell interactive mode fix") @@ -193,7 +218,7 @@ func retryWithErrorStrategy(ctx context.Context, opts RetryOptions, failResult * "force_ps_module": "true", "ansible_ps_version": "5.1", }) - return RunPlaybook(ctx, baseOpts) + return runPlaybookAttempt(ctx, baseOpts) case ErrSSMUserAccount: log.Info("SSM user account issue - recreating as domain account") @@ -208,7 +233,7 @@ func retryWithErrorStrategy(ctx context.Context, opts RetryOptions, failResult * "ansible_aws_ssm_timeout": "300", }) baseOpts.ExtraEnv = map[string]string{"ANSIBLE_TIMEOUT": "180"} - return RunPlaybook(ctx, baseOpts) + return runPlaybookAttempt(ctx, baseOpts) case ErrMSIInstaller: log.Info("MSI installer error - rebooting failed hosts before retry") @@ -216,7 +241,7 @@ func retryWithErrorStrategy(ctx context.Context, opts RetryOptions, failResult * time.Sleep(30 * time.Second) baseOpts.Forks = 1 - return RunPlaybook(ctx, baseOpts) + return runPlaybookAttempt(ctx, baseOpts) case ErrWUACOM: log.Info("WUA COM corruption - rebooting to clear pending registry deletions") @@ -224,7 +249,7 @@ func retryWithErrorStrategy(ctx context.Context, opts RetryOptions, failResult * time.Sleep(30 * time.Second) baseOpts.Forks = 1 - return RunPlaybook(ctx, baseOpts) + return runPlaybookAttempt(ctx, baseOpts) default: log.Info("retrying with general robust settings") @@ -233,7 +258,7 @@ func retryWithErrorStrategy(ctx context.Context, opts RetryOptions, failResult * "ANSIBLE_SSH_RETRIES": "5", "ANSIBLE_TIMEOUT": "120", } - return RunPlaybook(ctx, baseOpts) + return runPlaybookAttempt(ctx, baseOpts) } } diff --git a/cli/internal/ansible/retry_test.go b/cli/internal/ansible/retry_test.go index 596dae5f..545e6cc2 100644 --- a/cli/internal/ansible/retry_test.go +++ b/cli/internal/ansible/retry_test.go @@ -4,11 +4,103 @@ import ( "bytes" "context" "errors" + "io" "log/slog" "strings" "testing" + "time" ) +func TestResolveRetrySettings(t *testing.T) { + tests := []struct { + name string + opts RetryOptions + configuredRetries int + configuredDelay time.Duration + wantAttempts int + wantDelay time.Duration + wantDisabled bool + }{ + { + name: "omitted uses config", + configuredRetries: 3, + configuredDelay: 30 * time.Second, + wantAttempts: 3, + wantDelay: 30 * time.Second, + }, + { + name: "explicit zero runs once without retry or delay", + opts: RetryOptions{ + MaxRetriesSet: true, + RetryDelaySet: true, + }, + configuredRetries: 3, + configuredDelay: 30 * time.Second, + wantAttempts: 1, + wantDelay: 0, + wantDisabled: true, + }, + { + name: "zero in config still runs initial attempt", + configuredRetries: 0, + configuredDelay: 0, + wantAttempts: 1, + wantDelay: 0, + wantDisabled: true, + }, + { + name: "explicit positive overrides config", + opts: RetryOptions{ + MaxRetries: 5, + MaxRetriesSet: true, + RetryDelay: 12 * time.Second, + RetryDelaySet: true, + }, + configuredRetries: 3, + configuredDelay: 30 * time.Second, + wantAttempts: 5, + wantDelay: 12 * time.Second, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + attempts, delay, disabled := resolveRetrySettings(tc.opts, tc.configuredRetries, tc.configuredDelay) + if attempts != tc.wantAttempts || delay != tc.wantDelay || disabled != tc.wantDisabled { + t.Errorf("resolveRetrySettings() = (%d, %s, disabled=%v), want (%d, %s, disabled=%v)", + attempts, delay, disabled, tc.wantAttempts, tc.wantDelay, tc.wantDisabled) + } + }) + } +} + +func TestRunPlaybookWithRetryExplicitZeroRunsExactlyOnce(t *testing.T) { + original := runPlaybookAttempt + t.Cleanup(func() { runPlaybookAttempt = original }) + + attempts := 0 + runPlaybookAttempt = func(context.Context, RunOptions) *RunResult { + attempts++ + return &RunResult{ExitCode: 1, ErrorType: ErrUnclassified, ErrorDetail: "fixture failure"} + } + + err := RunPlaybookWithRetry(context.Background(), RetryOptions{ + Playbook: "fixture.yml", + Env: "test", + MaxRetries: 0, + MaxRetriesSet: true, + RetryDelay: 0, + RetryDelaySet: true, + Log: slog.New(slog.NewTextHandler(io.Discard, nil)), + }) + if err == nil || !strings.Contains(err.Error(), "failed after 1 attempts") { + t.Fatalf("error = %v, want one-attempt failure", err) + } + if attempts != 1 { + t.Errorf("ansible attempts = %d, want exactly 1", attempts) + } +} + // TestRunPlaybookWithRetryStopsOnCancelledContext pins the cancellation check // at the top of the retry loop. Wiring `provision` to the root's signal-aware // context means an interrupt now surfaces as an ordinary playbook failure, and diff --git a/cli/internal/config/config.go b/cli/internal/config/config.go index 181c8dfb..1b276ea0 100644 --- a/cli/internal/config/config.go +++ b/cli/internal/config/config.go @@ -121,6 +121,12 @@ var ( regionOverride string ) +// ErrLabConfigNotFound indicates that no base, overlay, or legacy lab config +// exists. Callers may treat this as optional for infrastructure modules that do +// not consume the GOAD lab config, while still surfacing other resolution +// failures such as malformed overlays or cache write errors. +var ErrLabConfigNotFound = errors.New("lab config not found") + // 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. @@ -242,7 +248,12 @@ func (c *Config) ResolvedLabConfigPath() (string, error) { overlayPath := filepath.Join(dataDir, c.Env+"-overlay.json") basePath := filepath.Join(dataDir, "config.json") - if fileExists(overlayPath) && fileExists(basePath) { + overlayExists := fileExists(overlayPath) + baseExists := fileExists(basePath) + if overlayExists && !baseExists { + return "", fmt.Errorf("lab config overlay %s requires base config %s", overlayPath, basePath) + } + if overlayExists { return c.mergedConfigPath(basePath, overlayPath) } @@ -253,11 +264,11 @@ func (c *Config) ResolvedLabConfigPath() (string, error) { } // Fallback: base config.json. - if fileExists(basePath) { + if baseExists { return basePath, nil } - return "", fmt.Errorf("no lab config found in %s", dataDir) + return "", fmt.Errorf("%w in %s", ErrLabConfigNotFound, dataDir) } // labConfigDataDir returns the data directory for the active environment's diff --git a/cli/internal/config/config_test.go b/cli/internal/config/config_test.go index c4d63cd9..3c6201cd 100644 --- a/cli/internal/config/config_test.go +++ b/cli/internal/config/config_test.go @@ -1,6 +1,7 @@ package config import ( + "errors" "os" "path/filepath" "reflect" @@ -283,6 +284,15 @@ func TestLabConfigPath(t *testing.T) { }) } +func TestResolvedLabConfigPathMarksMissingConfig(t *testing.T) { + c := &Config{ProjectRoot: resolveSymlinks(t, t.TempDir()), Env: "dev"} + + _, err := c.ResolvedLabConfigPath() + if !errors.Is(err, ErrLabConfigNotFound) { + t.Fatalf("ResolvedLabConfigPath() error = %v, want ErrLabConfigNotFound", err) + } +} + func TestConfigInstanceProfile(t *testing.T) { t.Run("field accessible on struct", func(t *testing.T) { c := &Config{InstanceProfile: "WarpgateImageBuilderInstanceProfile"} diff --git a/cli/internal/terragrunt/runner.go b/cli/internal/terragrunt/runner.go index 9336c7fb..06b41b7b 100644 --- a/cli/internal/terragrunt/runner.go +++ b/cli/internal/terragrunt/runner.go @@ -8,10 +8,20 @@ import ( "os" "os/exec" "path/filepath" + "regexp" "sort" "strings" ) +const outputTailLimit = 64 * 1024 + +var ( + ansiEscapeRE = regexp.MustCompile(`\x1b\[[0-?]*[ -/]*[@-~]`) + stateLockErrorRE = regexp.MustCompile(`(?i)(error acquiring the state lock|failed to acquire state lock)`) + stateLockIDRE = regexp.MustCompile(`(?im)\bID:\s*([A-Za-z0-9][A-Za-z0-9._:/-]*)(?:\s|$|│)`) + shellSafeArgRE = regexp.MustCompile(`^[A-Za-z0-9_./:-]+$`) +) + type Options struct { Action string WorkDir string @@ -54,11 +64,12 @@ func Run(ctx context.Context, opts Options) error { } defer cleanup() - cmd.Stdout = writer - cmd.Stderr = writer + tail := newTailBuffer(outputTailLimit) + cmd.Stdout = io.MultiWriter(writer, tail) + cmd.Stderr = cmd.Stdout if err := cmd.Run(); err != nil { - return fmt.Errorf("terragrunt %s failed: %w", opts.Action, err) + return commandError(fmt.Sprintf("terragrunt %s failed", opts.Action), err, tail.String(), opts.TerragruntBinary) } return nil } @@ -98,11 +109,12 @@ func RunAll(ctx context.Context, opts Options) error { } defer cleanup() - cmd.Stdout = writer - cmd.Stderr = writer + tail := newTailBuffer(outputTailLimit) + cmd.Stdout = io.MultiWriter(writer, tail) + cmd.Stderr = cmd.Stdout if err := cmd.Run(); err != nil { - return fmt.Errorf("terragrunt run --all %s failed: %w", opts.Action, err) + return commandError(fmt.Sprintf("terragrunt run --all %s failed", opts.Action), err, tail.String(), opts.TerragruntBinary) } return nil } @@ -208,6 +220,75 @@ func buildEnv(opts Options) []string { return env } +type tailBuffer struct { + limit int + buf []byte +} + +func newTailBuffer(limit int) *tailBuffer { + return &tailBuffer{limit: limit} +} + +func (b *tailBuffer) Write(p []byte) (int, error) { + written := len(p) + if b.limit <= 0 { + return written, nil + } + if len(p) >= b.limit { + b.buf = append(b.buf[:0], p[len(p)-b.limit:]...) + return written, nil + } + + overflow := len(b.buf) + len(p) - b.limit + if overflow > 0 { + copy(b.buf, b.buf[overflow:]) + b.buf = b.buf[:len(b.buf)-overflow] + } + b.buf = append(b.buf, p...) + return written, nil +} + +func (b *tailBuffer) String() string { + return string(b.buf) +} + +func commandError(message string, runErr error, output, terragruntBinary string) error { + lockID := stateLockID(output) + if lockID == "" { + return fmt.Errorf("%s: %w", message, runErr) + } + if terragruntBinary == "" { + terragruntBinary = "terragrunt" + } + return fmt.Errorf("%s: %w\nTerraform state lock detected (ID: %s). "+ + "After confirming no other operation is running, run this from the locked module directory:\n %s force-unlock %s", + message, runErr, lockID, shellQuote(terragruntBinary), lockID) +} + +func shellQuote(value string) string { + if shellSafeArgRE.MatchString(value) { + return value + } + return "'" + strings.ReplaceAll(value, "'", `'"'"'`) + "'" +} + +func stateLockID(output string) string { + clean := ansiEscapeRE.ReplaceAllString(output, "") + lockAt := stateLockErrorRE.FindStringIndex(clean) + if lockAt == nil { + return "" + } + lockOutput := clean[lockAt[0]:] + if infoAt := strings.Index(strings.ToLower(lockOutput), "lock info:"); infoAt >= 0 { + lockOutput = lockOutput[infoAt:] + } + match := stateLockIDRE.FindStringSubmatch(lockOutput) + if len(match) != 2 { + return "" + } + return match[1] +} + func outputWriter(logFile string) (io.Writer, func(), error) { if logFile == "" { return os.Stdout, func() {}, nil diff --git a/cli/internal/terragrunt/runner_test.go b/cli/internal/terragrunt/runner_test.go index cf242027..69e3bea0 100644 --- a/cli/internal/terragrunt/runner_test.go +++ b/cli/internal/terragrunt/runner_test.go @@ -1,7 +1,11 @@ package terragrunt import ( + "context" + "errors" "os" + "path/filepath" + "runtime" "strings" "testing" ) @@ -152,3 +156,135 @@ func TestOutputWriter_InvalidDir(t *testing.T) { t.Fatal("expected error for invalid log path, got nil") } } + +func TestStateLockID(t *testing.T) { + tests := []struct { + name string + output string + want string + }{ + { + name: "Terraform lock info", + output: `Error: Error acquiring the state lock + +Lock Info: + ID: 2f9a3fa7-14a9-4e74-a2ef-235a43f1bf00 + Path: state/prod.tfstate`, + want: "2f9a3fa7-14a9-4e74-a2ef-235a43f1bf00", + }, + { + name: "OpenTofu output with Terragrunt prefix and ANSI", + output: "\x1b[31mERROR\x1b[0m [goad/dc01] Failed to acquire state lock\n" + + "[goad/dc01] Lock Info:\n[goad/dc01] │ ID: azure-lease-123 │\n", + want: "azure-lease-123", + }, + { + name: "unrelated ID is ignored without lock error", + output: "request failed\nID: request-123\n", + }, + { + name: "lock error without ID", + output: "Error acquiring the state lock: backend unavailable", + }, + { + name: "unsafe partial ID is rejected", + output: "Error acquiring the state lock\nLock Info:\n ID: abc;rm", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + if got := stateLockID(tc.output); got != tc.want { + t.Errorf("stateLockID() = %q, want %q", got, tc.want) + } + }) + } +} + +func TestTailBufferRetainsBoundedSuffix(t *testing.T) { + tail := newTailBuffer(5) + for _, chunk := range []string{"abc", "def", "gh"} { + if n, err := tail.Write([]byte(chunk)); err != nil || n != len(chunk) { + t.Fatalf("Write(%q) = (%d, %v)", chunk, n, err) + } + } + if got := tail.String(); got != "defgh" { + t.Errorf("tail = %q, want %q", got, "defgh") + } + + if _, err := tail.Write([]byte("0123456789")); err != nil { + t.Fatalf("large Write() error: %v", err) + } + if got := tail.String(); got != "56789" { + t.Errorf("tail after large write = %q, want %q", got, "56789") + } +} + +func TestCommandErrorAddsSafeUnlockHint(t *testing.T) { + cause := errors.New("exit status 1") + output := "Error acquiring the state lock\nLock Info:\n ID: lock-123\n" + err := commandError("terragrunt apply failed", cause, output, "/opt/terragrunt tools/terragrunt") + + if !errors.Is(err, cause) { + t.Error("command error does not unwrap to process failure") + } + for _, want := range []string{ + "Terraform state lock detected (ID: lock-123)", + "confirming no other operation is running", + "'/opt/terragrunt tools/terragrunt' force-unlock lock-123", + } { + if !strings.Contains(err.Error(), want) { + t.Errorf("error %q does not contain %q", err, want) + } + } + + plain := commandError("terragrunt apply failed", cause, "ordinary failure", "terragrunt") + if plain.Error() != "terragrunt apply failed: exit status 1" { + t.Errorf("ordinary error changed: %q", plain) + } +} + +func TestShellQuote(t *testing.T) { + tests := map[string]string{ + "/opt/homebrew/bin/terragrunt": "/opt/homebrew/bin/terragrunt", + "/opt/terragrunt tools/tg": "'/opt/terragrunt tools/tg'", + "/tmp/operator's/tg": `'/tmp/operator'"'"'s/tg'`, + } + for input, want := range tests { + if got := shellQuote(input); got != want { + t.Errorf("shellQuote(%q) = %q, want %q", input, got, want) + } + } +} + +func TestRunnersSurfaceStateLockHint(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("test uses a POSIX shell fixture") + } + dir := t.TempDir() + binary := filepath.Join(dir, "fake-terragrunt") + script := "#!/bin/sh\n" + + "printf '%s\\n' 'Error acquiring the state lock' >&2\n" + + "printf '%s\\n' 'Lock Info:' ' ID: integration-lock-456' >&2\n" + + "exit 1\n" + if err := os.WriteFile(binary, []byte(script), 0o755); err != nil { + t.Fatalf("write fixture: %v", err) + } + + opts := Options{Action: "apply", WorkDir: dir, TerragruntBinary: binary} + for _, tc := range []struct { + name string + run func(context.Context, Options) error + }{ + {name: "single module", run: Run}, + {name: "run all", run: RunAll}, + } { + t.Run(tc.name, func(t *testing.T) { + err := tc.run(context.Background(), opts) + want := binary + " force-unlock integration-lock-456" + if err == nil || !strings.Contains(err.Error(), want) { + t.Fatalf("runner error = %v, want state-lock recovery hint", err) + } + }) + } +} diff --git a/cli/internal/variant/generator.go b/cli/internal/variant/generator.go index b8a7f6f1..9bbeef93 100644 --- a/cli/internal/variant/generator.go +++ b/cli/internal/variant/generator.go @@ -3,6 +3,7 @@ package variant import ( "bytes" "encoding/json" + "errors" "fmt" "io/fs" "os" @@ -13,6 +14,43 @@ import ( "unicode/utf8" ) +// CompletionMarkerName is written only after a variant generation run reaches +// the end successfully. Its absence means an existing target may be partial. +const CompletionMarkerName = ".dreadgoad-variant-complete" + +const completionMarkerContent = "complete\n" + +// IsComplete reports whether target contains a valid completion marker. +func IsComplete(target string) (bool, error) { + marker := filepath.Join(target, CompletionMarkerName) + data, err := os.ReadFile(marker) + if errors.Is(err, os.ErrNotExist) { + return false, nil + } + if err != nil { + return false, fmt.Errorf("inspect completion marker: %w", err) + } + if string(data) != completionMarkerContent { + return false, fmt.Errorf("completion marker has invalid contents: %s", marker) + } + return true, nil +} + +func writeCompletionMarker(target string) error { + marker := filepath.Join(target, CompletionMarkerName) + tmp := marker + ".tmp" + if err := os.WriteFile(tmp, []byte(completionMarkerContent), 0o644); err != nil { + return err + } + if err := os.Rename(tmp, marker); err != nil { + if removeErr := os.Remove(tmp); removeErr != nil { + return fmt.Errorf("rename marker: %w; cleanup: %v", err, removeErr) + } + return fmt.Errorf("rename marker: %w", err) + } + return nil +} + // LabConfig is the top-level structure of a GOAD config.json. // All known fields are modeled; if a config adds new top-level keys they // must be added here to survive the transform round-trip in transformFile. @@ -243,6 +281,9 @@ func (g *Generator) Run() error { g.generateMappings(config) g.buildOrderedReplacements() + if err := createFreshTarget(g.TargetPath); err != nil { + return err + } if err := g.copyAndTransform(); err != nil { return fmt.Errorf("transform: %w", err) @@ -252,20 +293,36 @@ func (g *Generator) Run() error { return fmt.Errorf("save mappings: %w", err) } - valid := g.validate() - g.createDocumentation() + if err := g.validate(); err != nil { + return fmt.Errorf("variant validation failed: %w", err) + } + if err := g.createDocumentation(); err != nil { + return fmt.Errorf("create documentation: %w", err) + } + if err := writeCompletionMarker(g.TargetPath); err != nil { + return fmt.Errorf("write completion marker: %w", err) + } fmt.Printf("\n%s\n", strings.Repeat("=", 60)) - if valid { - fmt.Println("Variant generation complete and validated!") - } else { - fmt.Println("Variant generated but validation found issues") - } + fmt.Println("Variant generation complete and validated!") fmt.Printf("%s\n\n", strings.Repeat("=", 60)) return nil } +func createFreshTarget(target string) error { + if err := os.MkdirAll(filepath.Dir(target), 0o755); err != nil { + return fmt.Errorf("create variant target parent: %w", err) + } + if err := os.Mkdir(target, 0o755); err != nil { + if errors.Is(err, os.ErrExist) { + return fmt.Errorf("variant target already exists: %s; move or remove it before generating", target) + } + return fmt.Errorf("create variant target: %w", err) + } + return nil +} + // loadConfig reads the source GOAD config.json. func (g *Generator) loadConfig() (*LabConfig, error) { data, err := os.ReadFile(filepath.Join(g.SourcePath, "data", "config.json")) @@ -956,7 +1013,7 @@ var textFilenames = map[string]bool{ } // transformFile transforms a single file with replacements and writes to target. -func (g *Generator) transformFile(srcPath, relPath string) (transformed bool) { +func (g *Generator) transformFile(srcPath, relPath string) (transformed bool, err error) { // Rename file paths based on entity mappings (e.g., arya.txt -> thomas.txt). newRelPath := g.applyReplacements(relPath) ext := filepath.Ext(srcPath) @@ -964,25 +1021,23 @@ func (g *Generator) transformFile(srcPath, relPath string) (transformed bool) { targetFile := filepath.Join(g.TargetPath, newRelPath) if err := os.MkdirAll(filepath.Dir(targetFile), 0o755); err != nil { - fmt.Printf("Warning: mkdir failed for %s: %v\n", relPath, err) - return false + return false, fmt.Errorf("create target directory for %s: %w", relPath, err) } isText := textExtensions[ext] || textFilenames[base] || (ext == "" && g.isTextFile(srcPath)) if !isText { if err := copyFile(srcPath, targetFile); err != nil { - fmt.Printf("Warning: Could not copy %s: %v\n", relPath, err) + return false, err } - return false + return false, nil } content, err := os.ReadFile(srcPath) if err != nil { - fmt.Printf("Warning: Could not read %s: %v\n", relPath, err) if cpErr := copyFile(srcPath, targetFile); cpErr != nil { - fmt.Printf("Warning: fallback copy also failed: %v\n", cpErr) + return false, fmt.Errorf("read %s: %v; fallback copy: %w", srcPath, err, cpErr) } - return false + return false, nil } newContent := g.applyReplacements(string(content)) @@ -992,10 +1047,9 @@ func (g *Generator) transformFile(srcPath, relPath string) (transformed bool) { newContent = g.transformConfigJSON(base, newContent) if err := os.WriteFile(targetFile, []byte(newContent), 0o644); err != nil { - fmt.Printf("Warning: Could not write %s: %v\n", relPath, err) - return false + return false, fmt.Errorf("write %s: %w", targetFile, err) } - return true + return true, nil } // copyAndTransform copies the source directory, transforming text files. @@ -1025,10 +1079,6 @@ func (g *Generator) transformConfigJSON(base, content string) string { func (g *Generator) copyAndTransform() error { fmt.Println("\n=== Copying and Transforming Files ===") - if err := os.MkdirAll(g.TargetPath, 0o755); err != nil { - return err - } - var total, transformed, copied int err := filepath.WalkDir(g.SourcePath, func(path string, d fs.DirEntry, err error) error { @@ -1042,14 +1092,24 @@ func (g *Generator) copyAndTransform() error { return nil } - // Skip .git files - rel, _ := filepath.Rel(g.SourcePath, path) + // Skip .git files and completion markers inherited from a variant source. + rel, err := filepath.Rel(g.SourcePath, path) + if err != nil { + return fmt.Errorf("resolve relative path for %s: %w", path, err) + } if strings.Contains(rel, ".git") { return nil } + if d.Name() == CompletionMarkerName || d.Name() == CompletionMarkerName+".tmp" { + return nil + } total++ - if g.transformFile(path, rel) { + didTransform, err := g.transformFile(path, rel) + if err != nil { + return fmt.Errorf("process %s: %w", rel, err) + } + if didTransform { transformed++ } else { copied++ @@ -1095,25 +1155,29 @@ type violation struct { } // validate checks that no original GOAD names appear in variant files. -func (g *Generator) validate() bool { +func (g *Generator) validate() error { fmt.Println("\n=== Validating Variant ===") - violations, filesChecked := g.findNameViolations() + violations, filesChecked, err := g.findNameViolations() + if err != nil { + return fmt.Errorf("scan generated files: %w", err) + } fmt.Printf("Checked %d text files\n", filesChecked) printViolations(violations) + if len(violations) > 0 { + return fmt.Errorf("found %d original-name violations", len(violations)) + } fmt.Println("\nValidating structure...") - g.validateStructureCounts() - - return len(violations) == 0 + return g.validateStructureCounts() } -func (g *Generator) findNameViolations() ([]violation, int) { +func (g *Generator) findNameViolations() ([]violation, int, error) { var violations []violation filesChecked := 0 skipFiles := map[string]bool{"mapping.json": true, "README.md": true} - if err := filepath.WalkDir(g.TargetPath, func(path string, d fs.DirEntry, err error) error { + err := filepath.WalkDir(g.TargetPath, func(path string, d fs.DirEntry, err error) error { if err != nil || d.IsDir() { return err } @@ -1127,10 +1191,13 @@ func (g *Generator) findNameViolations() ([]violation, int) { filesChecked++ content, err := os.ReadFile(path) if err != nil { - return nil + return fmt.Errorf("read %s: %w", path, err) } lower := strings.ToLower(string(content)) - rel, _ := filepath.Rel(g.TargetPath, path) + rel, err := filepath.Rel(g.TargetPath, path) + if err != nil { + return fmt.Errorf("resolve relative path for %s: %w", path, err) + } for _, name := range originalNames { if strings.Contains(lower, name) { re, err := regexp.Compile(`\b` + regexp.QuoteMeta(name) + `\b`) @@ -1140,10 +1207,8 @@ func (g *Generator) findNameViolations() ([]violation, int) { } } return nil - }); err != nil { - fmt.Printf("Warning: error walking variant directory: %v\n", err) - } - return violations, filesChecked + }) + return violations, filesChecked, err } func printViolations(violations []violation) { @@ -1164,18 +1229,18 @@ func printViolations(violations []violation) { } } -func (g *Generator) validateStructureCounts() { +func (g *Generator) validateStructureCounts() error { origConfig, err := g.loadConfig() if err != nil { - return + return fmt.Errorf("load source structure: %w", err) } varData, err := os.ReadFile(filepath.Join(g.TargetPath, "data", "config.json")) if err != nil { - return + return fmt.Errorf("read generated structure: %w", err) } var varConfig LabConfig - if json.Unmarshal(varData, &varConfig) != nil { - return + if err := json.Unmarshal(varData, &varConfig); err != nil { + return fmt.Errorf("parse generated structure: %w", err) } origHosts := len(origConfig.Lab.Hosts) varHosts := len(varConfig.Lab.Hosts) @@ -1190,10 +1255,14 @@ func (g *Generator) validateStructureCounts() { } fmt.Printf(" Hosts: %d -> %d %s\n", origHosts, varHosts, checkMark(origHosts, varHosts)) fmt.Printf(" Domains: %d -> %d %s\n", origDomains, varDomains, checkMark(origDomains, varDomains)) + if origHosts != varHosts || origDomains != varDomains { + return fmt.Errorf("structure count mismatch: hosts %d -> %d, domains %d -> %d", origHosts, varHosts, origDomains, varDomains) + } + return nil } // createDocumentation generates a README for the variant. -func (g *Generator) createDocumentation() { +func (g *Generator) createDocumentation() error { readme := fmt.Sprintf(`# GOAD %s This is a graph-isomorphic variant of the GOAD (Game of Active Directory) lab environment. @@ -1241,10 +1310,10 @@ Generated by GOAD Variant Generator readmePath := filepath.Join(g.TargetPath, "README.md") if err := os.WriteFile(readmePath, []byte(readme), 0o644); err != nil { - fmt.Printf("Warning: failed to write documentation %s: %v\n", readmePath, err) - return + return fmt.Errorf("write %s: %w", readmePath, err) } fmt.Printf("Documentation created at %s\n", readmePath) + return nil } func copyFile(src, dst string) error { diff --git a/cli/internal/variant/generator_test.go b/cli/internal/variant/generator_test.go index 449e56a0..8415dc12 100644 --- a/cli/internal/variant/generator_test.go +++ b/cli/internal/variant/generator_test.go @@ -188,6 +188,139 @@ func TestGeneratorEndToEnd(t *testing.T) { if _, err := os.Stat(filepath.Join(targetDir, "README.md")); err != nil { t.Fatal("README.md not created") } + complete, err := IsComplete(targetDir) + if err != nil { + t.Fatalf("check completion marker: %v", err) + } + if !complete { + t.Fatal("completion marker not created") + } +} + +func TestGeneratorFailureLeavesNoCompletionMarker(t *testing.T) { + sourceDir, targetDir := setupTestSource(t) + if err := os.Symlink(filepath.Join(sourceDir, "missing.bin"), filepath.Join(sourceDir, "broken.bin")); err != nil { + t.Skipf("create broken symlink fixture: %v", err) + } + + err := NewGenerator(sourceDir, targetDir, "test-failure").Run() + if err == nil || !strings.Contains(err.Error(), "process broken.bin") { + t.Fatalf("Run() error = %v, want target write failure", err) + } + complete, checkErr := IsComplete(targetDir) + if checkErr != nil { + t.Fatalf("check completion marker: %v", checkErr) + } + if complete { + t.Fatal("failed generation retained a stale completion marker") + } +} + +func TestGeneratorValidationFailureLeavesNoCompletionMarker(t *testing.T) { + sourceDir, targetDir := setupTestSource(t) + if err := os.WriteFile(filepath.Join(sourceDir, "scripts", "unmapped.txt"), []byte("tywin\n"), 0o644); err != nil { + t.Fatal(err) + } + + err := NewGenerator(sourceDir, targetDir, "test-validation-failure").Run() + if err == nil || !strings.Contains(err.Error(), "variant validation failed") { + t.Fatalf("Run() error = %v, want validation failure", err) + } + complete, checkErr := IsComplete(targetDir) + if checkErr != nil { + t.Fatalf("check completion marker: %v", checkErr) + } + if complete { + t.Fatal("failed validation retained a completion marker") + } +} + +func TestGeneratorDocumentationFailureLeavesNoCompletionMarker(t *testing.T) { + sourceDir, targetDir := setupTestSource(t) + readmeDir := filepath.Join(sourceDir, "README.md") + if err := os.MkdirAll(readmeDir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(readmeDir, "blocker"), []byte("block README creation"), 0o644); err != nil { + t.Fatal(err) + } + + err := NewGenerator(sourceDir, targetDir, "test-documentation-failure").Run() + if err == nil || !strings.Contains(err.Error(), "create documentation") { + t.Fatalf("Run() error = %v, want documentation failure", err) + } + complete, checkErr := IsComplete(targetDir) + if checkErr != nil { + t.Fatalf("check completion marker: %v", checkErr) + } + if complete { + t.Fatal("documentation failure left a completion marker") + } +} + +func TestGeneratorRejectsExistingTargetWithoutModification(t *testing.T) { + sourceDir, targetDir := setupTestSource(t) + if err := os.MkdirAll(targetDir, 0o755); err != nil { + t.Fatal(err) + } + marker := filepath.Join(targetDir, CompletionMarkerName) + if err := os.WriteFile(marker, []byte("complete\n"), 0o644); err != nil { + t.Fatal(err) + } + sentinel := filepath.Join(targetDir, "keep.txt") + if err := os.WriteFile(sentinel, []byte("unchanged"), 0o644); err != nil { + t.Fatal(err) + } + + err := NewGenerator(sourceDir, targetDir, "test-existing-target").Run() + if err == nil || !strings.Contains(err.Error(), "variant target already exists") { + t.Fatalf("Run() error = %v, want existing-target rejection", err) + } + data, readErr := os.ReadFile(sentinel) + if readErr != nil || string(data) != "unchanged" { + t.Fatalf("existing target was modified: data=%q error=%v", data, readErr) + } + complete, checkErr := IsComplete(targetDir) + if checkErr != nil || !complete { + t.Fatalf("existing completion marker changed: complete=%v error=%v", complete, checkErr) + } +} + +func TestValidateRejectsScanFailure(t *testing.T) { + missingTarget := filepath.Join(t.TempDir(), "missing") + gen := NewGenerator(t.TempDir(), missingTarget, "test-scan-failure") + + err := gen.validate() + if err == nil || !strings.Contains(err.Error(), "scan generated files") { + t.Fatalf("validate() error = %v, want scan failure", err) + } +} + +func TestValidateRejectsStructureMismatch(t *testing.T) { + sourceDir, targetDir := setupTestSource(t) + if err := os.MkdirAll(filepath.Join(targetDir, "data"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(targetDir, "data", "config.json"), []byte(`{"lab":{"hosts":{},"domains":{}}}`), 0o644); err != nil { + t.Fatal(err) + } + + err := NewGenerator(sourceDir, targetDir, "test-structure-mismatch").validate() + if err == nil || !strings.Contains(err.Error(), "structure count mismatch") { + t.Fatalf("validate() error = %v, want structure mismatch", err) + } +} + +func TestIsCompleteRejectsInvalidMarker(t *testing.T) { + target := t.TempDir() + if err := os.WriteFile(filepath.Join(target, CompletionMarkerName), []byte("partial"), 0o644); err != nil { + t.Fatal(err) + } + + complete, err := IsComplete(target) + if err == nil || !strings.Contains(err.Error(), "invalid contents") { + t.Fatalf("IsComplete() = %v, %v; want invalid marker error", complete, err) + } } func TestPasswordInDescriptionPreserved(t *testing.T) { diff --git a/docs/cli.md b/docs/cli.md index 45165ab6..ae4878ea 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -175,11 +175,14 @@ that preserve all structural relationships and vulnerabilities. - **`dreadgoad provision`**: When the active environment has `variant: true`, provisioning automatically generates the variant if the target directory - doesn't exist yet. Subsequent runs skip generation. + doesn't exist yet. The generator writes `.dreadgoad-variant-complete` only + after all required files are produced. Subsequent runs skip generation only + when that marker is valid; an unmarked directory is rejected as incomplete. - **`dreadgoad variant generate`**: Reads defaults from the active environment's config. Explicit flags (`--source`, `--target`, `--name`) - override the config values. + override the config values. Generation refuses an existing target so files + from different randomized runs cannot be mixed. - **Regenerating**: Delete the variant target directory and re-run `dreadgoad provision` or `dreadgoad variant generate` to get fresh diff --git a/docs/mkdocs/docs/cli-reference.md b/docs/mkdocs/docs/cli-reference.md index 00f9a366..bee48ee2 100644 --- a/docs/mkdocs/docs/cli-reference.md +++ b/docs/mkdocs/docs/cli-reference.md @@ -36,6 +36,7 @@ Deploy the lab end-to-end: doctor → infra apply → provision → health-check ```bash dreadgoad up # full pipeline dreadgoad up --from provision # resume from a step +dreadgoad up --from provision --from-playbook ad-data.yml dreadgoad up --skip-doctor # bypass pre-flight checks dreadgoad up --limit dc01 # narrow provisioning to one host ``` @@ -43,11 +44,12 @@ dreadgoad up --limit dc01 # narrow provisioning to one host | Flag | Description | |------|-------------| | `--from string` | Resume from this step (`doctor`, `infra`, `provision`, `health-check`) | +| `--from-playbook string` | Resume provisioning from this playbook onward | | `--skip-doctor` | Skip the doctor pre-flight checks | | `--limit string` | Limit provisioning to specific hosts | | `--plays string` | Comma-separated playbooks to run (default: all) | -| `--max-retries int` | Max retry attempts for provisioning | -| `--retry-delay int` | Delay between retries in seconds | +| `--max-retries int` | Max retry attempts for provisioning (`0` disables retries) | +| `--retry-delay int` | Delay between retries in seconds (`0` disables delay) | | `--module string` | Target a specific infra module | | `--exclude string` | Exclude infra modules (comma-separated) | @@ -235,9 +237,9 @@ Runs Ansible playbooks to provision Active Directory infrastructure with error-s |------|-------------| | `--from string` | Resume provisioning from this playbook onward | | `--limit string` | Limit execution to specific hosts | -| `--max-retries int` | Max retry attempts | +| `--max-retries int` | Max retry attempts (`0` disables retries) | | `--plays string` | Comma-separated playbooks to run (default: all) | -| `--retry-delay int` | Delay between retries in seconds | +| `--retry-delay int` | Delay between retries in seconds (`0` disables delay) | ```bash # Run all provisioning playbooks @@ -251,6 +253,9 @@ dreadgoad provision --plays "ad-groups.yml,ad-acl.yml" # Limit to specific hosts with retries dreadgoad provision --limit dc01 --max-retries 5 + +# Run once with no automatic retry +dreadgoad provision --max-retries 0 ``` ### ad-users @@ -262,9 +267,9 @@ Shortcut for `provision --plays ad-data.yml`. | Flag | Description | |------|-------------| | `--limit string` | Limit execution to specific hosts | -| `--max-retries int` | Max retry attempts | +| `--max-retries int` | Max retry attempts (`0` disables retries) | | `--plays string` | Comma-separated playbooks to run | -| `--retry-delay int` | Delay between retries in seconds | +| `--retry-delay int` | Delay between retries in seconds (`0` disables delay) | ```bash dreadgoad ad-users diff --git a/docs/mkdocs/docs/provisioning.md b/docs/mkdocs/docs/provisioning.md index 9e25fe6c..9ce53e63 100644 --- a/docs/mkdocs/docs/provisioning.md +++ b/docs/mkdocs/docs/provisioning.md @@ -168,6 +168,9 @@ Configure retry behavior with: ```bash dreadgoad provision --max-retries 5 --retry-delay 60 + +# Run the playbook once without automatic retries +dreadgoad provision --max-retries 0 ``` ### When to stop and fix manually