diff --git a/DOCUMENTATION.md b/DOCUMENTATION.md index da60d0c1..d89aaf33 100644 --- a/DOCUMENTATION.md +++ b/DOCUMENTATION.md @@ -284,6 +284,8 @@ flowchart TD | `step.iac_provider_destroy` | Destroys resources via an `iac.provider` plugin | platform | | `step.iac_provider_drift` | Detects drift via an `iac.provider` (optional `IaCProviderDriftDetector`; `supported:false` fallback) | platform | | `step.iac_secret_reachability` | Pre-flight gate: checks whether `secret://` refs in plan specs are reachable from the chosen exec-env. The verdict is provider-level (one `CheckAccess` probe; the same result is reported per distinct ref); returns `all_reachable` bool. Fail-safe for remote exec-envs (host-local backends unverifiable per ADR 0017, unknown backends, and probe failure → unreachable) | platform | +| `step.iac_commit_back` | Serialises authored specs via `iac/specgen.SpecToYAML` and commits back to git after a full-success apply; `{committed:false, reason:"partial-apply"}` on partial; `{state_diverged:true}` (HTTP 207) when apply succeeded but git failed; `secret://` refs survive verbatim | platform | +| `step.iac_provider_reconcile` | Drift → import → approximate cloud-snapshot YAML → draft PR; APPROXIMATE (not via SpecToYAML); mandatory disclaimer: "imported from cloud; approximate; does NOT reconstruct your `secret://` refs — review before merge" | platform | | `step.tofu_init` | Initializes an OpenTofu working directory | platform | | `step.tofu_plan` | Creates an OpenTofu execution plan | platform | | `step.tofu_apply` | Applies OpenTofu changes to infrastructure | platform | diff --git a/module/pipeline_step_iac_commit_back.go b/module/pipeline_step_iac_commit_back.go new file mode 100644 index 00000000..31633205 --- /dev/null +++ b/module/pipeline_step_iac_commit_back.go @@ -0,0 +1,281 @@ +package module + +import ( + "context" + "encoding/json" + "fmt" + "os" + "path/filepath" + + "github.com/GoCodeAlone/modular" + "github.com/GoCodeAlone/workflow/iac/specgen" + "github.com/GoCodeAlone/workflow/iac/specparse" + "github.com/GoCodeAlone/workflow/interfaces" +) + +// GitExecFn executes a git/gh command and returns its combined output. +// +// argv is the COMPLETE argument vector including the binary as argv[0] (e.g. +// {"git","commit","-m","msg"} or {"gh","pr","create","--fill",...}); the prod +// implementation runs argv[0] directly with no shell and no entrypoint prefix. +// env carries extra environment variables (merged over the host environment so +// GH_TOKEN/GITHUB_TOKEN are forwarded automatically). workDir is the git +// working directory the command runs in (the step's repo_dir). +// +// The prod implementation in plugins/platform/plugin.go runs host-native via +// os/exec — the engine committing to its own repo is not untrusted-code +// execution. Tests inject a stub. +type GitExecFn func(ctx context.Context, argv []string, env map[string]string, workDir string) (string, error) + +// ─── step.iac_commit_back ──────────────────────────────────────────────────── + +// IaCCommitBackStep serialises the authored specs to YAML and commits the +// result back to a git branch — but ONLY when the preceding apply step +// completed with full success (no errors + action_count matches the plan). +// +// Partial apply → {committed:false, reason:"partial-apply"} (no commit). +// Full success but git failure → {state_diverged:true, reason:...} +// (route maps to HTTP 207; the apply already happened). +type IaCCommitBackStep struct { + name string + specs []interfaces.ResourceSpec + specsFrom string // dotted context path; mutually exclusive with specs + applyResultFrom string // dotted context path to the upstream apply_result + branch string + message string + target string // "branch-push" (default) or "gh-pr" + repoDir string // git working directory / sandbox mount root + gitFn GitExecFn +} + +const ( + defaultApplyResultFrom = "steps.apply.apply_result" + defaultTarget = "branch-push" + targetBranchPush = "branch-push" + targetGHPR = "gh-pr" + specsYAMLFilename = "resources.yaml" +) + +// resolveTarget validates a configured publish target. An empty value defaults +// to branch-push; any value other than "branch-push" or "gh-pr" is rejected +// (so a typo silently falling back to branch-push can't push to an unintended +// place). Shared by step.iac_commit_back and step.iac_provider_reconcile. +func resolveTarget(raw string) (string, error) { + switch raw { + case "": + return defaultTarget, nil + case targetBranchPush, targetGHPR: + return raw, nil + default: + return "", fmt.Errorf("invalid target %q (must be %q or %q)", raw, targetBranchPush, targetGHPR) + } +} + +// NewIaCCommitBackStepFactory returns a StepFactory for step.iac_commit_back. +// gitFn is the git executor — pass the prod impl from plugins/platform/plugin.go +// or inject a stub in tests. The factory panics if gitFn is nil (mirrors +// NewIaCProviderApplyStepFactory). +func NewIaCCommitBackStepFactory(gitFn GitExecFn) StepFactory { + if gitFn == nil { + panic("NewIaCCommitBackStepFactory: gitFn must not be nil") + } + return func(name string, cfg map[string]any, app modular.Application) (PipelineStep, error) { + branch, _ := cfg["branch"].(string) + if branch == "" { + return nil, fmt.Errorf("iac_commit_back step %q: 'branch' is required", name) + } + repoDir, _ := cfg["repo_dir"].(string) + if repoDir == "" { + return nil, fmt.Errorf("iac_commit_back step %q: 'repo_dir' is required", name) + } + message, _ := cfg["message"].(string) + if message == "" { + message = "chore: commit back applied infrastructure specs" + } + rawTarget, _ := cfg["target"].(string) + target, err := resolveTarget(rawTarget) + if err != nil { + return nil, fmt.Errorf("iac_commit_back step %q: %w", name, err) + } + applyResultFrom, _ := cfg["apply_result_from"].(string) + if applyResultFrom == "" { + applyResultFrom = defaultApplyResultFrom + } + + specsFrom, _ := cfg["specs_from"].(string) + _, hasStaticSpecs := cfg["specs"] + if specsFrom != "" && hasStaticSpecs { + return nil, fmt.Errorf("iac_commit_back step %q: 'specs' and 'specs_from' are mutually exclusive", name) + } + + var specs []interfaces.ResourceSpec + if hasStaticSpecs { + specs, err = parseResourceSpecs(cfg["specs"]) + if err != nil { + return nil, fmt.Errorf("iac_commit_back step %q: parse specs: %w", name, err) + } + } + + return &IaCCommitBackStep{ + name: name, + specs: specs, + specsFrom: specsFrom, + applyResultFrom: applyResultFrom, + branch: branch, + message: message, + target: target, + repoDir: repoDir, + gitFn: gitFn, + }, nil + } +} + +func (s *IaCCommitBackStep) Name() string { return s.name } + +func (s *IaCCommitBackStep) Execute(ctx context.Context, pc *PipelineContext) (*StepResult, error) { + // 1. Resolve specs. + specs := s.specs + if s.specsFrom != "" { + raw := resolveBodyFrom(s.specsFrom, pc) + var err error + specs, err = specparse.ParseResourceSpecs(raw) + if err != nil { + return nil, fmt.Errorf("iac_commit_back step %q: resolve specs_from %q: %w", s.name, s.specsFrom, err) + } + if len(specs) == 0 { + return nil, fmt.Errorf("iac_commit_back step %q: specs_from %q resolved to empty/zero specs", s.name, s.specsFrom) + } + } + + // 2. Read apply_result from context. + rawApplyResult := resolveBodyFrom(s.applyResultFrom, pc) + // Also read action_count — it is a sibling of apply_result in the apply step output. + // action_count path: replace the last segment "apply_result" with "action_count". + actionCountFrom := replaceLastSegment(s.applyResultFrom, "action_count") + rawActionCount := resolveBodyFrom(actionCountFrom, pc) + + // 3. Determine full success. + if !isFullSuccess(rawApplyResult, rawActionCount) { + return &StepResult{Output: map[string]any{ + "committed": false, + "reason": "partial-apply", + }}, nil + } + + // 4. Full success: serialise specs to YAML via specgen.SpecToYAML. + yamlBytes, err := specgen.SpecToYAML(specs) + if err != nil { + return nil, fmt.Errorf("iac_commit_back step %q: SpecToYAML: %w", s.name, err) + } + + // 5. Write YAML into repo_dir. + outPath := filepath.Join(s.repoDir, specsYAMLFilename) + if err := os.WriteFile(outPath, yamlBytes, 0o600); err != nil { + return nil, fmt.Errorf("iac_commit_back step %q: write specs YAML: %w", s.name, err) + } + + // 6. Run git commands via the injected executor. Each command is a COMPLETE + // argv with the binary as argv[0] so the host-native executor runs it + // directly (no entrypoint double-prefix). If ANY git operation fails → + // state_diverged:true (the apply already happened; 207, not 5xx). + var gitErr error + var ref string + + _, gitErr = s.gitFn(ctx, []string{"git", "checkout", "-b", s.branch}, nil, s.repoDir) + if gitErr == nil { + _, gitErr = s.gitFn(ctx, []string{"git", "add", "-A"}, nil, s.repoDir) + } + if gitErr == nil { + _, gitErr = s.gitFn(ctx, []string{"git", "commit", "-m", s.message}, nil, s.repoDir) + } + if gitErr == nil { + switch s.target { + case "gh-pr": + ref, gitErr = s.gitFn(ctx, []string{"gh", "pr", "create", "--fill", "--head", s.branch}, nil, s.repoDir) + default: // "branch-push" + ref, gitErr = s.gitFn(ctx, []string{"git", "push", "--set-upstream", "origin", s.branch}, nil, s.repoDir) + } + } + + if gitErr != nil { + return &StepResult{Output: map[string]any{ + "committed": false, + "state_diverged": true, + "reason": fmt.Sprintf("git executor error: %v", gitErr), + }}, nil + } + + out := map[string]any{ + "committed": true, + } + if ref != "" { + out["ref"] = ref + } + return &StepResult{Output: out}, nil +} + +// isFullSuccess returns true iff the apply result has no errors AND the number +// of recorded action outcomes matches a PRESENT, numeric action_count. +// +// action_count MUST be present and numeric: a missing/non-numeric action_count +// is treated as NOT full success. Otherwise a malformed or empty apply_result +// (no action_count, no actions) would degrade to 0 == 0 → "full success" and +// commit on garbage input — a destructive-empty hazard. +func isFullSuccess(rawApplyResult any, rawActionCount any) bool { + if rawApplyResult == nil { + return false + } + m, ok := rawApplyResult.(map[string]any) + if !ok { + return false + } + // Check Errors field — absent or empty slice means no errors. + if errs, ok := m["errors"]; ok && errs != nil { + if errList, ok := errs.([]any); ok && len(errList) > 0 { + return false + } + } + // action_count is the number of planned actions; the Actions slice in the + // result must match. Require action_count present + numeric — never infer 0. + actionCount, ok := toFloat64(rawActionCount) + if !ok { + return false + } + actions, _ := m["actions"].([]any) + return len(actions) == int(actionCount) +} + +// replaceLastSegment replaces the last dot-separated segment of path with newSeg. +// E.g. "steps.apply.apply_result" → "steps.apply.action_count". +func replaceLastSegment(path, newSeg string) string { + for i := len(path) - 1; i >= 0; i-- { + if path[i] == '.' { + return path[:i+1] + newSeg + } + } + return newSeg +} + +// toFloat64 converts a JSON-decoded numeric value (float64 from json.Unmarshal, +// json.Number, or int/int64/float32 from direct Go construction) to float64. +// The second return is false when v is nil or not a numeric type, so callers +// can distinguish "absent/non-numeric" from a legitimate zero. +func toFloat64(v any) (float64, bool) { + switch n := v.(type) { + case float64: + return n, true + case float32: + return float64(n), true + case int: + return float64(n), true + case int64: + return float64(n), true + case json.Number: + f, err := n.Float64() + if err != nil { + return 0, false + } + return f, true + } + return 0, false +} diff --git a/module/pipeline_step_iac_commit_back_test.go b/module/pipeline_step_iac_commit_back_test.go new file mode 100644 index 00000000..47080aee --- /dev/null +++ b/module/pipeline_step_iac_commit_back_test.go @@ -0,0 +1,630 @@ +package module_test + +import ( + "context" + "encoding/json" + "errors" + "os" + "path/filepath" + "testing" + + "github.com/GoCodeAlone/workflow/interfaces" + "github.com/GoCodeAlone/workflow/module" +) + +// ─── helpers ───────────────────────────────────────────────────────────────── + +// buildFullApplyResult returns an apply_result any value (as it appears in the +// pipeline context) representing a FULL success: no errors + action_count +// actions all succeeded. +func buildFullApplyResult(t *testing.T, actionCount int) any { + t.Helper() + actions := make([]interfaces.ActionOutcome, actionCount) + for i := range actions { + actions[i] = interfaces.ActionOutcome{Status: interfaces.ActionStatusSuccess} + } + ar := interfaces.ApplyResult{ + PlanID: "plan-test", + Actions: actions, + // Errors is nil → full success + } + b, err := json.Marshal(ar) + if err != nil { + t.Fatalf("buildFullApplyResult: marshal: %v", err) + } + var out any + if err := json.Unmarshal(b, &out); err != nil { + t.Fatalf("buildFullApplyResult: unmarshal: %v", err) + } + return out +} + +// buildPartialApplyResult returns an apply_result with errors (partial failure). +func buildPartialApplyResult(t *testing.T) any { + t.Helper() + ar := interfaces.ApplyResult{ + PlanID: "plan-partial", + Errors: []interfaces.ActionError{ + {Resource: "db", Action: "create", Error: "timeout"}, + }, + Actions: []interfaces.ActionOutcome{ + {Status: interfaces.ActionStatusSuccess}, + {Status: interfaces.ActionStatusError}, + }, + } + b, err := json.Marshal(ar) + if err != nil { + t.Fatalf("buildPartialApplyResult: marshal: %v", err) + } + var out any + if err := json.Unmarshal(b, &out); err != nil { + t.Fatalf("buildPartialApplyResult: unmarshal: %v", err) + } + return out +} + +// commitBackPCWithApplyResult builds a PipelineContext that holds an apply +// result at the default path "steps.apply.apply_result" with the given +// action_count. +func commitBackPCWithApplyResult(applyResult any, actionCount int) *module.PipelineContext { + return &module.PipelineContext{ + StepOutputs: map[string]map[string]any{ + "apply": { + "apply_result": applyResult, + "action_count": float64(actionCount), + }, + }, + } +} + +// stubGitExecFn returns a GitExecFn stub that records every git invocation and +// returns the specified error on the nth call (1-indexed, 0 means never error). +func stubGitExecFn(t *testing.T, failOnCall int, captured *[][]string) module.GitExecFn { + t.Helper() + call := 0 + return func(_ context.Context, args []string, _ map[string]string, _ string) (string, error) { + call++ + if captured != nil { + *captured = append(*captured, append([]string{}, args...)) + } + if failOnCall > 0 && call == failOnCall { + return "", errors.New("git push: remote rejected") + } + return "ok", nil + } +} + +// ─── step.iac_commit_back tests ────────────────────────────────────────────── + +// TestIaCCommitBack_FullSuccess_Commits verifies that on a full-success apply +// (no errors + action_count matches) the step serialises the specs with +// SpecToYAML, writes into repo_dir, calls git and reports committed:true. +func TestIaCCommitBack_FullSuccess_Commits(t *testing.T) { + dir := t.TempDir() + + var calls [][]string + gitFn := stubGitExecFn(t, 0, &calls) + + factory := module.NewIaCCommitBackStepFactory(gitFn) + step, err := factory("cb", map[string]any{ + "repo_dir": dir, + "branch": "infra/auto-commit", + "message": "chore: commit back applied specs", + "target": "branch-push", + "specs": []any{ + map[string]any{"name": "db", "type": "infra.database"}, + }, + }, nil) + if err != nil { + t.Fatalf("factory error: %v", err) + } + + applyResult := buildFullApplyResult(t, 1) + pc := commitBackPCWithApplyResult(applyResult, 1) + + result, err := step.Execute(context.Background(), pc) + if err != nil { + t.Fatalf("Execute error: %v", err) + } + + // Must report committed:true + if committed, _ := result.Output["committed"].(bool); !committed { + t.Errorf("expected committed:true, got %v", result.Output["committed"]) + } + + // Git commands must have been invoked + if len(calls) == 0 { + t.Error("expected at least one git command call") + } + + // Every git/gh invocation must carry the FULL argv (binary as args[0]) so + // host-native exec runs args[0] directly — no entrypoint double-prefix. + for i, call := range calls { + if len(call) == 0 { + t.Fatalf("call %d is empty", i) + } + if call[0] != "git" && call[0] != "gh" { + t.Errorf("call %d must start with the binary name (git/gh), got: %v", i, call) + } + } + // The branch-push path must include the canonical command sequence. + assertCallPresent(t, calls, []string{"git", "checkout", "-b", "infra/auto-commit"}) + assertCallPresent(t, calls, []string{"git", "add", "-A"}) + assertCallPresent(t, calls, []string{"git", "commit", "-m", "chore: commit back applied specs"}) + assertCallPresent(t, calls, []string{"git", "push", "--set-upstream", "origin", "infra/auto-commit"}) + + // The specs YAML file must exist in repo_dir + entries, err := os.ReadDir(dir) + if err != nil { + t.Fatalf("ReadDir: %v", err) + } + if len(entries) == 0 { + t.Error("expected SpecToYAML file written to repo_dir") + } +} + +// assertCallPresent fails the test if no recorded call equals want exactly. +func assertCallPresent(t *testing.T, calls [][]string, want []string) { + t.Helper() + for _, c := range calls { + if equalArgs(c, want) { + return + } + } + t.Errorf("expected git call %v, got calls: %v", want, calls) +} + +func equalArgs(a, b []string) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if a[i] != b[i] { + return false + } + } + return true +} + +// TestIaCCommitBack_WorkDirForwarded verifies that the step passes repo_dir as +// the workDir argument to the git executor on every call. +func TestIaCCommitBack_WorkDirForwarded(t *testing.T) { + dir := t.TempDir() + + var workDirs []string + gitFn := func(_ context.Context, _ []string, _ map[string]string, workDir string) (string, error) { + workDirs = append(workDirs, workDir) + return "ok", nil + } + + factory := module.NewIaCCommitBackStepFactory(gitFn) + step, err := factory("cb", map[string]any{ + "repo_dir": dir, + "branch": "infra/auto", + "message": "chore: commit back", + "specs": []any{ + map[string]any{"name": "db", "type": "infra.database"}, + }, + }, nil) + if err != nil { + t.Fatalf("factory error: %v", err) + } + + applyResult := buildFullApplyResult(t, 0) + pc := commitBackPCWithApplyResult(applyResult, 0) + + if _, err := step.Execute(context.Background(), pc); err != nil { + t.Fatalf("Execute error: %v", err) + } + + if len(workDirs) == 0 { + t.Fatal("expected at least one git call carrying a workDir") + } + for i, wd := range workDirs { + if wd != dir { + t.Errorf("call %d workDir = %q, want %q", i, wd, dir) + } + } +} + +// TestIaCCommitBack_PartialApplyByCount_NoCommit verifies that an apply_result +// with empty errors but action_count > len(actions) is treated as a partial +// apply (NOT full success) — no commit is produced. +func TestIaCCommitBack_PartialApplyByCount_NoCommit(t *testing.T) { + dir := t.TempDir() + + var calls [][]string + gitFn := stubGitExecFn(t, 0, &calls) + + factory := module.NewIaCCommitBackStepFactory(gitFn) + step, err := factory("cb", map[string]any{ + "repo_dir": dir, + "branch": "infra/auto", + "message": "chore: commit back", + "specs": []any{ + map[string]any{"name": "db", "type": "infra.database"}, + }, + }, nil) + if err != nil { + t.Fatalf("factory error: %v", err) + } + + // Apply result has NO errors but only 1 action outcome recorded while the + // plan had 3 actions (action_count=3) → not full success. + applyResult := buildFullApplyResult(t, 1) + pc := commitBackPCWithApplyResult(applyResult, 3) + + result, err := step.Execute(context.Background(), pc) + if err != nil { + t.Fatalf("Execute must not error on partial-by-count, got: %v", err) + } + + if committed, _ := result.Output["committed"].(bool); committed { + t.Error("expected committed:false when action_count > len(actions)") + } + reason, _ := result.Output["reason"].(string) + if reason != "partial-apply" { + t.Errorf("expected reason 'partial-apply', got %q", reason) + } + if len(calls) > 0 { + t.Errorf("git must not be called on partial-by-count, got %d calls", len(calls)) + } +} + +// TestIaCCommitBack_MissingActionCount_NoCommit is the destructive-empty +// safety guard: an apply_result that is present with NO errors but where +// action_count is MISSING must NOT be treated as full success. Without the +// guard, a missing count degrades to 0 and an absent/empty actions slice makes +// 0 == 0 → "full success" → it would COMMIT on a malformed/empty apply_result. +func TestIaCCommitBack_MissingActionCount_NoCommit(t *testing.T) { + dir := t.TempDir() + + var calls [][]string + gitFn := stubGitExecFn(t, 0, &calls) + + factory := module.NewIaCCommitBackStepFactory(gitFn) + step, err := factory("cb", map[string]any{ + "repo_dir": dir, + "branch": "infra/auto", + "message": "chore: commit back", + "specs": []any{ + map[string]any{"name": "db", "type": "infra.database"}, + }, + }, nil) + if err != nil { + t.Fatalf("factory error: %v", err) + } + + // apply_result present, errors empty, NO actions — but action_count is + // entirely ABSENT from the apply step output. + applyResult := buildFullApplyResult(t, 0) + pc := &module.PipelineContext{ + StepOutputs: map[string]map[string]any{ + "apply": { + "apply_result": applyResult, + // action_count intentionally omitted + }, + }, + } + + result, err := step.Execute(context.Background(), pc) + if err != nil { + t.Fatalf("Execute must not error on missing action_count, got: %v", err) + } + + if committed, _ := result.Output["committed"].(bool); committed { + t.Error("expected committed:false when action_count is missing (destructive-empty guard)") + } + reason, _ := result.Output["reason"].(string) + if reason != "partial-apply" { + t.Errorf("expected reason 'partial-apply', got %q", reason) + } + if len(calls) > 0 { + t.Errorf("git must not be called when action_count is missing, got %d calls", len(calls)) + } +} + +// TestIaCCommitBack_NonNumericActionCount_NoCommit asserts that a present but +// non-numeric action_count (e.g. a string) is also rejected as not-full-success. +func TestIaCCommitBack_NonNumericActionCount_NoCommit(t *testing.T) { + dir := t.TempDir() + + var calls [][]string + gitFn := stubGitExecFn(t, 0, &calls) + + factory := module.NewIaCCommitBackStepFactory(gitFn) + step, err := factory("cb", map[string]any{ + "repo_dir": dir, + "branch": "infra/auto", + "message": "chore: commit back", + "specs": []any{ + map[string]any{"name": "db", "type": "infra.database"}, + }, + }, nil) + if err != nil { + t.Fatalf("factory error: %v", err) + } + + applyResult := buildFullApplyResult(t, 0) + pc := &module.PipelineContext{ + StepOutputs: map[string]map[string]any{ + "apply": { + "apply_result": applyResult, + "action_count": "not-a-number", + }, + }, + } + + result, err := step.Execute(context.Background(), pc) + if err != nil { + t.Fatalf("Execute must not error on non-numeric action_count, got: %v", err) + } + if committed, _ := result.Output["committed"].(bool); committed { + t.Error("expected committed:false when action_count is non-numeric") + } + if len(calls) > 0 { + t.Errorf("git must not be called when action_count is non-numeric, got %d calls", len(calls)) + } +} + +// TestIaCCommitBack_Factory_InvalidTarget verifies the factory rejects an +// unknown target value rather than silently falling back to branch-push. +func TestIaCCommitBack_Factory_InvalidTarget(t *testing.T) { + gitFn := stubGitExecFn(t, 0, nil) + factory := module.NewIaCCommitBackStepFactory(gitFn) + _, err := factory("cb", map[string]any{ + "repo_dir": "/tmp", + "branch": "infra/auto", + "target": "force-push-to-main", // not a valid target + "specs": []any{}, + }, nil) + if err == nil { + t.Fatal("expected error for invalid target, got nil") + } + if !containsString(err.Error(), "invalid target") { + t.Errorf("expected 'invalid target' error, got: %v", err) + } +} + +// TestIaCCommitBack_PartialApply_NoCommit verifies that a partial apply +// (Errors non-empty) causes the step to skip committing and return +// committed:false with reason:"partial-apply". +func TestIaCCommitBack_PartialApply_NoCommit(t *testing.T) { + dir := t.TempDir() + + var calls [][]string + gitFn := stubGitExecFn(t, 0, &calls) + + factory := module.NewIaCCommitBackStepFactory(gitFn) + step, err := factory("cb", map[string]any{ + "repo_dir": dir, + "branch": "infra/auto", + "message": "chore: commit back", + "specs": []any{ + map[string]any{"name": "db", "type": "infra.database"}, + }, + }, nil) + if err != nil { + t.Fatalf("factory error: %v", err) + } + + applyResult := buildPartialApplyResult(t) + // action_count is 2 but Errors is non-empty → partial + pc := commitBackPCWithApplyResult(applyResult, 2) + + result, err := step.Execute(context.Background(), pc) + if err != nil { + t.Fatalf("Execute must not error on partial-apply, got: %v", err) + } + + if committed, _ := result.Output["committed"].(bool); committed { + t.Error("expected committed:false on partial apply") + } + reason, _ := result.Output["reason"].(string) + if reason != "partial-apply" { + t.Errorf("expected reason 'partial-apply', got %q", reason) + } + if len(calls) > 0 { + t.Errorf("git must not be called on partial apply, got %d calls", len(calls)) + } +} + +// TestIaCCommitBack_GitFails_StateDiverged verifies that when the apply +// succeeded but the git executor errors, the step returns +// state_diverged:true (not a hard error) so callers can map to HTTP 207. +func TestIaCCommitBack_GitFails_StateDiverged(t *testing.T) { + dir := t.TempDir() + + var calls [][]string + // Fail on the very first git call (e.g., git checkout -b) + gitFn := stubGitExecFn(t, 1, &calls) + + factory := module.NewIaCCommitBackStepFactory(gitFn) + step, err := factory("cb", map[string]any{ + "repo_dir": dir, + "branch": "infra/auto", + "message": "chore: commit back", + "specs": []any{ + map[string]any{"name": "db", "type": "infra.database"}, + }, + }, nil) + if err != nil { + t.Fatalf("factory error: %v", err) + } + + applyResult := buildFullApplyResult(t, 0) + pc := commitBackPCWithApplyResult(applyResult, 0) + + result, err := step.Execute(context.Background(), pc) + // A git executor failure MUST NOT be a pipeline error; it returns + // state_diverged:true so the route can map to HTTP 207. + if err != nil { + t.Fatalf("Execute must not hard-error on git failure, got: %v", err) + } + + if sd, _ := result.Output["state_diverged"].(bool); !sd { + t.Errorf("expected state_diverged:true on git failure, got %v", result.Output) + } + if _, ok := result.Output["reason"]; !ok { + t.Error("expected reason field when state_diverged") + } +} + +// TestIaCCommitBack_SecretRefSurvival verifies that authored specs containing +// secret:// refs are serialised verbatim — the literal ref appears in the +// YAML, not an expanded value. +func TestIaCCommitBack_SecretRefSurvival(t *testing.T) { + dir := t.TempDir() + + var calls [][]string + gitFn := stubGitExecFn(t, 0, &calls) + + factory := module.NewIaCCommitBackStepFactory(gitFn) + step, err := factory("cb", map[string]any{ + "repo_dir": dir, + "branch": "infra/auto", + "message": "chore: commit back", + "specs": []any{ + map[string]any{ + "name": "secret-db", + "type": "infra.database", + "config": map[string]any{ + "password": "secret://vault/my-db-pw", + }, + }, + }, + }, nil) + if err != nil { + t.Fatalf("factory error: %v", err) + } + + applyResult := buildFullApplyResult(t, 0) + pc := commitBackPCWithApplyResult(applyResult, 0) + + result, err := step.Execute(context.Background(), pc) + if err != nil { + t.Fatalf("Execute error: %v", err) + } + if committed, _ := result.Output["committed"].(bool); !committed { + t.Errorf("expected committed:true, got %v", result.Output) + } + + // Read the written YAML file and assert the secret:// ref is present verbatim. + entries, _ := os.ReadDir(dir) + if len(entries) == 0 { + t.Fatal("no file written to repo_dir") + } + yamlBytes, err := os.ReadFile(filepath.Join(dir, entries[0].Name())) + if err != nil { + t.Fatalf("ReadFile: %v", err) + } + yamlStr := string(yamlBytes) + if !containsString(yamlStr, "secret://vault/my-db-pw") { + t.Errorf("expected literal secret:// ref in YAML, got:\n%s", yamlStr) + } +} + +// TestIaCCommitBack_Factory_RequiresBranch verifies factory rejects missing branch. +func TestIaCCommitBack_Factory_RequiresBranch(t *testing.T) { + gitFn := stubGitExecFn(t, 0, nil) + factory := module.NewIaCCommitBackStepFactory(gitFn) + _, err := factory("cb", map[string]any{ + "repo_dir": "/tmp", + "message": "msg", + "specs": []any{}, + }, nil) + if err == nil { + t.Fatal("expected error when 'branch' missing") + } +} + +// TestIaCCommitBack_Factory_RequiresRepoDir verifies factory rejects missing repo_dir. +func TestIaCCommitBack_Factory_RequiresRepoDir(t *testing.T) { + gitFn := stubGitExecFn(t, 0, nil) + factory := module.NewIaCCommitBackStepFactory(gitFn) + _, err := factory("cb", map[string]any{ + "branch": "infra/auto", + "message": "msg", + "specs": []any{}, + }, nil) + if err == nil { + t.Fatal("expected error when 'repo_dir' missing") + } +} + +// TestIaCCommitBack_Factory_PanicOnNilGitFn verifies the factory panics when +// gitFn is nil (mirrors the apply step pattern). +func TestIaCCommitBack_Factory_PanicOnNilGitFn(t *testing.T) { + defer func() { + if r := recover(); r == nil { + t.Error("expected panic when gitFn is nil") + } + }() + module.NewIaCCommitBackStepFactory(nil) //nolint:staticcheck // intentional panic test +} + +// TestIaCCommitBack_GhPR_Target verifies that target=gh-pr calls gh instead of git push. +func TestIaCCommitBack_GhPR_Target(t *testing.T) { + dir := t.TempDir() + + var calls [][]string + gitFn := stubGitExecFn(t, 0, &calls) + + factory := module.NewIaCCommitBackStepFactory(gitFn) + step, err := factory("cb", map[string]any{ + "repo_dir": dir, + "branch": "infra/auto", + "message": "chore: commit back", + "target": "gh-pr", + "specs": []any{ + map[string]any{"name": "db", "type": "infra.database"}, + }, + }, nil) + if err != nil { + t.Fatalf("factory error: %v", err) + } + + applyResult := buildFullApplyResult(t, 0) + pc := commitBackPCWithApplyResult(applyResult, 0) + + result, err := step.Execute(context.Background(), pc) + if err != nil { + t.Fatalf("Execute error: %v", err) + } + if committed, _ := result.Output["committed"].(bool); !committed { + t.Errorf("expected committed:true, got %v", result.Output) + } + + // The gh-pr path must NOT push and MUST invoke gh pr create with full argv. + foundGh := false + for _, call := range calls { + if len(call) > 0 && call[0] == "gh" { + foundGh = true + // gh pr create must carry --head and --fill (no --draft for commit_back). + if len(call) < 3 || call[1] != "pr" || call[2] != "create" { + t.Errorf("gh call must be 'gh pr create ...', got: %v", call) + } + if !argvContains(call, "--head") || !argvContains(call, "infra/auto") { + t.Errorf("gh call must include '--head infra/auto', got: %v", call) + } + if !argvContains(call, "--fill") { + t.Errorf("gh call must include '--fill', got: %v", call) + } + } + if len(call) > 0 && argvContains(call, "push") { + t.Errorf("gh-pr target must not git push, got call: %v", call) + } + } + if !foundGh { + t.Errorf("expected a 'gh' call for target=gh-pr, got calls: %v", calls) + } +} + +func argvContains(argv []string, want string) bool { + for _, a := range argv { + if a == want { + return true + } + } + return false +} diff --git a/module/pipeline_step_iac_provider_reconcile.go b/module/pipeline_step_iac_provider_reconcile.go new file mode 100644 index 00000000..a8671aa3 --- /dev/null +++ b/module/pipeline_step_iac_provider_reconcile.go @@ -0,0 +1,290 @@ +package module + +import ( + "context" + "fmt" + "os" + "path/filepath" + + "github.com/GoCodeAlone/modular" + "github.com/GoCodeAlone/workflow/interfaces" + "gopkg.in/yaml.v3" +) + +// ─── step.iac_provider_reconcile ───────────────────────────────────────────── + +// IaCProviderReconcileStep performs a drift → import → approximate-YAML → +// DRAFT commit cycle. It is explicitly approximate: the YAML it emits is a +// cloud snapshot, NOT a faithful reconstruction of any authored spec (no +// SpecToYAML call on authored data). The draft PR/commit body carries a +// mandatory disclaimer warning the reviewer that secret:// refs are absent +// and the YAML must be reviewed before merge. +// +// Output shape: +// +// { +// "draft": bool — true iff a draft commit/PR was actually produced +// "ref": string — optional (branch/PR ref when draft=true) +// "warning": string — the disclaimer +// "count": int — number of drifted resources detected +// "state_diverged": bool — true when drift was found but git failed (no PR produced) +// "reason": string — set when state_diverged=true +// } +// +// On git failure draft is FALSE (no commit/PR exists); the caller maps +// state_diverged to HTTP 207. +const reconcileWarning = "imported from cloud; approximate; does NOT reconstruct your secret:// refs — review before merge" + +// IaCProviderReconcileStep implements step.iac_provider_reconcile. +type IaCProviderReconcileStep struct { + name string + provider string + branch string + target string // "branch-push" (default) or "gh-pr" + repoDir string + gitFn GitExecFn + app modular.Application +} + +// NewIaCProviderReconcileStepFactory returns a StepFactory for +// step.iac_provider_reconcile. gitFn is the git executor (same pattern as +// NewIaCCommitBackStepFactory). The factory panics if gitFn is nil. +func NewIaCProviderReconcileStepFactory(gitFn GitExecFn) StepFactory { + if gitFn == nil { + panic("NewIaCProviderReconcileStepFactory: gitFn must not be nil") + } + return func(name string, cfg map[string]any, app modular.Application) (PipelineStep, error) { + providerName, _ := cfg["provider"].(string) + if providerName == "" { + return nil, fmt.Errorf("iac_provider_reconcile step %q: 'provider' is required", name) + } + branch, _ := cfg["branch"].(string) + if branch == "" { + branch = "infra/reconcile" + } + rawTarget, _ := cfg["target"].(string) + target, err := resolveTarget(rawTarget) + if err != nil { + return nil, fmt.Errorf("iac_provider_reconcile step %q: %w", name, err) + } + repoDir, _ := cfg["repo_dir"].(string) + if repoDir == "" { + return nil, fmt.Errorf("iac_provider_reconcile step %q: 'repo_dir' is required", name) + } + return &IaCProviderReconcileStep{ + name: name, + provider: providerName, + branch: branch, + target: target, + repoDir: repoDir, + gitFn: gitFn, + app: app, + }, nil + } +} + +func (s *IaCProviderReconcileStep) Name() string { return s.name } + +func (s *IaCProviderReconcileStep) Execute(ctx context.Context, _ *PipelineContext) (*StepResult, error) { + provider, err := resolveIaCProvider(s.app, s.provider, s.name, "iac_provider_reconcile") + if err != nil { + return nil, err + } + + // Step 1: detect drift using DetectDrift (existence-only; no authored specs + // available at reconcile time). + statuses, err := provider.Status(ctx, nil) + if err != nil { + return nil, fmt.Errorf("iac_provider_reconcile step %q: Status: %w", s.name, err) + } + + // Build refs from current statuses, and index them by Name+Type so a drift + // result can be matched back to its ProviderID regardless of the order or + // count the provider returns DetectDrift results in. + refs := make([]statusRef, 0, len(statuses)) + refByKey := make(map[string]statusRef, len(statuses)) + for _, st := range statuses { + r := statusRef{ + Name: st.Name, + Type: st.Type, + ProviderID: st.ProviderID, + } + refs = append(refs, r) + refByKey[driftKey(st.Name, st.Type)] = r + } + + drifts, err := provider.DetectDrift(ctx, statusRefsToResourceRefs(refs)) + if err != nil { + return nil, fmt.Errorf("iac_provider_reconcile step %q: DetectDrift: %w", s.name, err) + } + + // Step 2: collect drifted resources by matching each drift result back to a + // status ref via Name+Type — NOT by positional index, which assumes the + // provider preserves order+length (a wrong-ProviderID / out-of-range hazard). + var drifted []statusRef + for _, d := range drifts { + if !d.Drifted { + continue + } + if r, ok := refByKey[driftKey(d.Name, d.Type)]; ok { + drifted = append(drifted, r) + } else { + // Drift reported for a resource Status didn't list — carry the + // drift's own identity (no ProviderID available to import by). + drifted = append(drifted, statusRef{Name: d.Name, Type: d.Type}) + } + } + + if len(drifted) == 0 { + // No drift — nothing to reconcile. + return &StepResult{Output: map[string]any{ + "draft": false, + "warning": "", + "count": 0, + }}, nil + } + + // Step 3: for each drifted resource, call Import to get a cloud snapshot. + snapshots := make([]map[string]any, 0, len(drifted)) + for _, r := range drifted { + state, importErr := provider.Import(ctx, r.ProviderID, r.Type) + if importErr != nil { + // Import errors are non-fatal for the reconcile step — record what we + // can and skip this resource rather than aborting the whole run. + snapshots = append(snapshots, map[string]any{ + "name": r.Name, + "type": r.Type, + "provider_id": r.ProviderID, + "import_error": importErr.Error(), + }) + continue + } + entry := map[string]any{ + "name": r.Name, + "type": r.Type, + "provider_id": r.ProviderID, + } + if state != nil { + if state.Outputs != nil { + entry["outputs"] = state.Outputs + } + if state.AppliedConfig != nil { + entry["config"] = state.AppliedConfig + } + } + snapshots = append(snapshots, entry) + } + + // Step 4: build an APPROXIMATE YAML cloud-snapshot. This is NOT SpecToYAML + // (which is for authored specs). We emit a plainly-labeled cloud-snapshot + // YAML prefixed with a comment block carrying the mandatory disclaimer. + approxYAML, err := buildApproximateYAML(snapshots) + if err != nil { + return nil, fmt.Errorf("iac_provider_reconcile step %q: build approximate YAML: %w", s.name, err) + } + + // Write the approximate YAML to repo_dir. + outPath := filepath.Join(s.repoDir, "reconcile-snapshot.yaml") + if err := os.WriteFile(outPath, approxYAML, 0o600); err != nil { + return nil, fmt.Errorf("iac_provider_reconcile step %q: write snapshot: %w", s.name, err) + } + + // Step 5: create a draft commit. Each command is a COMPLETE argv (binary as + // argv[0]) run host-native in repo_dir. + commitMessage := fmt.Sprintf("chore(reconcile): import drift snapshot — %s", reconcileWarning) + + var gitErr error + var ref string + + _, gitErr = s.gitFn(ctx, []string{"git", "checkout", "-b", s.branch}, nil, s.repoDir) + if gitErr == nil { + _, gitErr = s.gitFn(ctx, []string{"git", "add", "-A"}, nil, s.repoDir) + } + if gitErr == nil { + _, gitErr = s.gitFn(ctx, []string{"git", "commit", "-m", commitMessage}, nil, s.repoDir) + } + if gitErr == nil { + switch s.target { + case "gh-pr": + ref, gitErr = s.gitFn(ctx, []string{"gh", "pr", "create", + "--head", s.branch, + "--title", "reconcile: drift snapshot (approximate; review required)", + "--body", reconcileWarning, + "--draft", + }, nil, s.repoDir) + default: // "branch-push" + ref, gitErr = s.gitFn(ctx, []string{"git", "push", "--set-upstream", "origin", s.branch}, nil, s.repoDir) + } + } + + if gitErr != nil { + // Git failure on the reconcile path — NO commit/PR was produced, so + // draft MUST be false (claiming draft:true would be a lie). state_diverged + // signals the caller to surface a 207. + return &StepResult{Output: map[string]any{ + "draft": false, + "state_diverged": true, + "warning": reconcileWarning, + "count": len(drifted), + "reason": fmt.Sprintf("git executor error: %v", gitErr), + }}, nil + } + + out := map[string]any{ + "draft": true, + "warning": reconcileWarning, + "count": len(drifted), + } + if ref != "" { + out["ref"] = ref + } + return &StepResult{Output: out}, nil +} + +// statusRef is a minimal struct holding drift-detection identifiers for a +// resource. Using a bespoke type avoids importing the full ResourceRef struct +// while still carrying the ProviderID needed for Import. +type statusRef struct { + Name string + Type string + ProviderID string +} + +// driftKey is the identity used to match a DriftResult back to its status ref. +// Name+Type together identify a resource within a provider's namespace; the +// NUL separator avoids ambiguity between e.g. ("ab","c") and ("a","bc"). +func driftKey(name, typ string) string { + return name + "\x00" + typ +} + +// statusRefsToResourceRefs converts []statusRef to []interfaces.ResourceRef. +func statusRefsToResourceRefs(refs []statusRef) []interfaces.ResourceRef { + out := make([]interfaces.ResourceRef, len(refs)) + for i, r := range refs { + out[i] = interfaces.ResourceRef{ + Name: r.Name, + Type: r.Type, + ProviderID: r.ProviderID, + } + } + return out +} + +// buildApproximateYAML produces a YAML document from cloud-import snapshots. +// The result is clearly labeled as approximate via a header comment; it does +// NOT follow the SpecToYAML authoring schema. +func buildApproximateYAML(snapshots []map[string]any) ([]byte, error) { + header := "# APPROXIMATE CLOUD SNAPSHOT — imported from cloud state\n" + + "# " + reconcileWarning + "\n" + + "# This file was auto-generated by step.iac_provider_reconcile.\n" + + "# Do NOT use this as a source of truth without review.\n\n" + + body, err := yaml.Marshal(snapshots) + if err != nil { + return nil, err + } + return append([]byte(header), body...), nil +} + +// Ensure IaCProviderReconcileStep satisfies PipelineStep at compile time. +var _ PipelineStep = (*IaCProviderReconcileStep)(nil) diff --git a/module/pipeline_step_iac_provider_reconcile_test.go b/module/pipeline_step_iac_provider_reconcile_test.go new file mode 100644 index 00000000..f17bd206 --- /dev/null +++ b/module/pipeline_step_iac_provider_reconcile_test.go @@ -0,0 +1,462 @@ +package module_test + +import ( + "context" + "testing" + + "github.com/GoCodeAlone/workflow/interfaces" + "github.com/GoCodeAlone/workflow/module" +) + +// stubDriftImportProvider extends stubIaCProvider with controllable Import and +// DetectDrift responses for reconcile tests. +type stubDriftImportProvider struct { + stubIaCProvider + importResult *interfaces.ResourceState + importErr error +} + +func (s *stubDriftImportProvider) Import(_ context.Context, _ string, _ string) (*interfaces.ResourceState, error) { + return s.importResult, s.importErr +} + +// compile-time check +var _ interfaces.IaCProvider = (*stubDriftImportProvider)(nil) + +// capturingImportProvider records each (cloudID, resourceType) Import was called +// with and returns a ResourceState echoing that cloudID, so a test can assert +// the step imported each drifted resource by its CORRECT ProviderID — proving +// drift→ref matching is identity-based, not positional. +type capturingImportProvider struct { + stubIaCProvider + importedCloudIDs []string +} + +func (s *capturingImportProvider) Import(_ context.Context, cloudID string, resourceType string) (*interfaces.ResourceState, error) { + s.importedCloudIDs = append(s.importedCloudIDs, cloudID) + return &interfaces.ResourceState{ProviderID: cloudID, Type: resourceType}, nil +} + +var _ interfaces.IaCProvider = (*capturingImportProvider)(nil) + +// ─── step.iac_provider_reconcile tests ─────────────────────────────────────── + +// TestIaCProviderReconcile_DriftedProducesDraftCommit verifies that when the +// provider reports drifted resources the step produces a draft commit/PR with +// draft:true and a non-empty warning string containing the required disclaimer. +func TestIaCProviderReconcile_DriftedProducesDraftCommit(t *testing.T) { + dir := t.TempDir() + + var calls [][]string + gitFn := stubGitExecFn(t, 0, &calls) + + app := module.NewMockApplication() + provider := &stubDriftImportProvider{ + stubIaCProvider: stubIaCProvider{ + driftResult: []interfaces.DriftResult{ + { + Name: "db", + Type: "infra.database", + Drifted: true, + Class: interfaces.DriftClassConfig, + }, + }, + statusResult: []interfaces.ResourceStatus{ + {Name: "db", Type: "infra.database", ProviderID: "pid-1"}, + }, + }, + importResult: &interfaces.ResourceState{ + Name: "db", + Type: "infra.database", + ProviderID: "pid-1", + }, + } + if err := app.RegisterService("my-provider", provider); err != nil { + t.Fatal(err) + } + + factory := module.NewIaCProviderReconcileStepFactory(gitFn) + step, err := factory("reconcile", map[string]any{ + "provider": "my-provider", + "branch": "infra/reconcile-drift", + "repo_dir": dir, + }, app) + if err != nil { + t.Fatalf("factory error: %v", err) + } + + result, err := step.Execute(context.Background(), &module.PipelineContext{}) + if err != nil { + t.Fatalf("Execute error: %v", err) + } + + // Must be a draft + if draft, _ := result.Output["draft"].(bool); !draft { + t.Errorf("expected draft:true, got %v", result.Output["draft"]) + } + + // Warning must contain the required disclaimer + warning, _ := result.Output["warning"].(string) + if warning == "" { + t.Error("expected non-empty warning string") + } + if !containsString(warning, "approximate") { + t.Errorf("warning must contain 'approximate', got: %q", warning) + } + if !containsString(warning, "secret://") { + t.Errorf("warning must mention secret:// refs, got: %q", warning) + } + + // Git must have been called (branch + commit) + if len(calls) == 0 { + t.Error("expected git calls for draft commit") + } + + // Every invocation must carry the FULL argv (binary as args[0]). + for i, call := range calls { + if len(call) == 0 { + t.Fatalf("call %d is empty", i) + } + if call[0] != "git" && call[0] != "gh" { + t.Errorf("call %d must start with the binary name (git/gh), got: %v", i, call) + } + } + // Default target is branch-push → must push the branch, must NOT call gh. + assertCallPresent(t, calls, []string{"git", "checkout", "-b", "infra/reconcile-drift"}) + assertCallPresent(t, calls, []string{"git", "add", "-A"}) + assertCallPresent(t, calls, []string{"git", "push", "--set-upstream", "origin", "infra/reconcile-drift"}) + for _, call := range calls { + if len(call) > 0 && call[0] == "gh" { + t.Errorf("default branch-push target must NOT call gh, got call: %v", call) + } + } +} + +// TestIaCProviderReconcile_GhPRTarget verifies that target=gh-pr opens a DRAFT +// PR (gh pr create --draft) instead of a plain branch push. +func TestIaCProviderReconcile_GhPRTarget(t *testing.T) { + dir := t.TempDir() + + var calls [][]string + gitFn := stubGitExecFn(t, 0, &calls) + + app := module.NewMockApplication() + provider := &stubDriftImportProvider{ + stubIaCProvider: stubIaCProvider{ + driftResult: []interfaces.DriftResult{ + {Name: "db", Type: "infra.database", Drifted: true}, + }, + statusResult: []interfaces.ResourceStatus{ + {Name: "db", Type: "infra.database", ProviderID: "pid-1"}, + }, + }, + importResult: &interfaces.ResourceState{Name: "db", Type: "infra.database", ProviderID: "pid-1"}, + } + if err := app.RegisterService("my-provider", provider); err != nil { + t.Fatal(err) + } + + factory := module.NewIaCProviderReconcileStepFactory(gitFn) + step, err := factory("reconcile", map[string]any{ + "provider": "my-provider", + "branch": "infra/reconcile", + "repo_dir": dir, + "target": "gh-pr", + }, app) + if err != nil { + t.Fatalf("factory error: %v", err) + } + + result, err := step.Execute(context.Background(), &module.PipelineContext{}) + if err != nil { + t.Fatalf("Execute error: %v", err) + } + if draft, _ := result.Output["draft"].(bool); !draft { + t.Errorf("expected draft:true, got %v", result.Output) + } + + foundGhDraft := false + for _, call := range calls { + if len(call) >= 3 && call[0] == "gh" && call[1] == "pr" && call[2] == "create" { + foundGhDraft = true + if !argvContains(call, "--draft") { + t.Errorf("reconcile gh pr create must include --draft, got: %v", call) + } + } + } + if !foundGhDraft { + t.Errorf("expected 'gh pr create --draft' for target=gh-pr, got calls: %v", calls) + } +} + +// TestIaCProviderReconcile_NoDrift_NothingCommitted verifies that when there is +// no drift the step does NOT produce a commit and returns draft:false with +// count:0. +func TestIaCProviderReconcile_NoDrift_NothingCommitted(t *testing.T) { + dir := t.TempDir() + + var calls [][]string + gitFn := stubGitExecFn(t, 0, &calls) + + app := module.NewMockApplication() + provider := &stubDriftImportProvider{ + stubIaCProvider: stubIaCProvider{ + driftResult: []interfaces.DriftResult{ + {Name: "db", Type: "infra.database", Drifted: false}, + }, + statusResult: []interfaces.ResourceStatus{ + {Name: "db", Type: "infra.database", ProviderID: "pid-1"}, + }, + }, + importResult: &interfaces.ResourceState{ + Name: "db", Type: "infra.database", ProviderID: "pid-1", + }, + } + if err := app.RegisterService("my-provider", provider); err != nil { + t.Fatal(err) + } + + factory := module.NewIaCProviderReconcileStepFactory(gitFn) + step, err := factory("reconcile", map[string]any{ + "provider": "my-provider", + "branch": "infra/reconcile-drift", + "repo_dir": dir, + }, app) + if err != nil { + t.Fatalf("factory error: %v", err) + } + + result, err := step.Execute(context.Background(), &module.PipelineContext{}) + if err != nil { + t.Fatalf("Execute error: %v", err) + } + + if draft, _ := result.Output["draft"].(bool); draft { + t.Error("expected draft:false when no drift") + } + if len(calls) > 0 { + t.Errorf("git must not be called when no drift, got %d calls", len(calls)) + } +} + +// TestIaCProviderReconcile_OutputIsApproximate verifies that the reconcile step +// does NOT claim to faithfully reconstruct authored specs — the YAML output is +// labeled as approximate / cloud-snapshot, NOT produced via iac/specgen's +// SpecToYAML on authored specs. +func TestIaCProviderReconcile_OutputIsApproximate(t *testing.T) { + dir := t.TempDir() + + var calls [][]string + gitFn := stubGitExecFn(t, 0, &calls) + + app := module.NewMockApplication() + provider := &stubDriftImportProvider{ + stubIaCProvider: stubIaCProvider{ + driftResult: []interfaces.DriftResult{ + {Name: "db", Type: "infra.database", Drifted: true}, + }, + statusResult: []interfaces.ResourceStatus{ + {Name: "db", Type: "infra.database", ProviderID: "pid-1"}, + }, + }, + importResult: &interfaces.ResourceState{ + Name: "db", Type: "infra.database", ProviderID: "pid-1", + }, + } + if err := app.RegisterService("my-provider", provider); err != nil { + t.Fatal(err) + } + + factory := module.NewIaCProviderReconcileStepFactory(gitFn) + step, err := factory("reconcile", map[string]any{ + "provider": "my-provider", + "branch": "infra/reconcile", + "repo_dir": dir, + }, app) + if err != nil { + t.Fatalf("factory error: %v", err) + } + + result, err := step.Execute(context.Background(), &module.PipelineContext{}) + if err != nil { + t.Fatalf("Execute error: %v", err) + } + + // The warning must include the standard disclaimer text. + warning, _ := result.Output["warning"].(string) + requiredPhrases := []string{ + "imported from cloud", + "approximate", + "secret://", + "review before merge", + } + for _, phrase := range requiredPhrases { + if !containsString(warning, phrase) { + t.Errorf("warning missing required phrase %q; full warning: %q", phrase, warning) + } + } +} + +// TestIaCProviderReconcile_GitFails_StateDiverged verifies that when drift was +// detected but the git executor fails, the step returns draft:false (NO commit +// was produced) + state_diverged:true + a reason — NOT draft:true (which would +// falsely claim a PR exists). +func TestIaCProviderReconcile_GitFails_StateDiverged(t *testing.T) { + dir := t.TempDir() + + var calls [][]string + // Fail on the first git call (e.g. git checkout -b). + gitFn := stubGitExecFn(t, 1, &calls) + + app := module.NewMockApplication() + provider := &stubDriftImportProvider{ + stubIaCProvider: stubIaCProvider{ + driftResult: []interfaces.DriftResult{ + {Name: "db", Type: "infra.database", Drifted: true}, + }, + statusResult: []interfaces.ResourceStatus{ + {Name: "db", Type: "infra.database", ProviderID: "pid-1"}, + }, + }, + importResult: &interfaces.ResourceState{Name: "db", Type: "infra.database", ProviderID: "pid-1"}, + } + if err := app.RegisterService("my-provider", provider); err != nil { + t.Fatal(err) + } + + factory := module.NewIaCProviderReconcileStepFactory(gitFn) + step, err := factory("reconcile", map[string]any{ + "provider": "my-provider", + "branch": "infra/reconcile", + "repo_dir": dir, + }, app) + if err != nil { + t.Fatalf("factory error: %v", err) + } + + result, err := step.Execute(context.Background(), &module.PipelineContext{}) + // Git failure must NOT be a hard pipeline error. + if err != nil { + t.Fatalf("Execute must not hard-error on git failure, got: %v", err) + } + + // draft MUST be false — no commit/PR was produced. + if draft, _ := result.Output["draft"].(bool); draft { + t.Errorf("expected draft:false on git failure (no PR produced), got %v", result.Output) + } + if sd, _ := result.Output["state_diverged"].(bool); !sd { + t.Errorf("expected state_diverged:true on git failure, got %v", result.Output) + } + if _, ok := result.Output["reason"]; !ok { + t.Error("expected reason field when state_diverged") + } + // Must not claim a ref. + if _, ok := result.Output["ref"]; ok { + t.Errorf("must not claim a ref on git failure, got %v", result.Output["ref"]) + } +} + +// TestIaCProviderReconcile_DriftOrderIndependent proves that drift results are +// matched back to their status ref by Name+Type identity, NOT by positional +// index: DetectDrift returns results in a DIFFERENT order than the refs, yet +// each drifted resource is imported by its CORRECT ProviderID (no swap, no +// out-of-range panic). +func TestIaCProviderReconcile_DriftOrderIndependent(t *testing.T) { + dir := t.TempDir() + + var calls [][]string + gitFn := stubGitExecFn(t, 0, &calls) + + app := module.NewMockApplication() + provider := &capturingImportProvider{ + stubIaCProvider: stubIaCProvider{ + // Status lists web (pid-web) then db (pid-db). + statusResult: []interfaces.ResourceStatus{ + {Name: "web", Type: "infra.app", ProviderID: "pid-web"}, + {Name: "db", Type: "infra.database", ProviderID: "pid-db"}, + }, + // DetectDrift returns them in the REVERSE order, with only db drifted. + driftResult: []interfaces.DriftResult{ + {Name: "db", Type: "infra.database", Drifted: true}, + {Name: "web", Type: "infra.app", Drifted: false}, + }, + }, + } + if err := app.RegisterService("my-provider", provider); err != nil { + t.Fatal(err) + } + + factory := module.NewIaCProviderReconcileStepFactory(gitFn) + step, err := factory("reconcile", map[string]any{ + "provider": "my-provider", + "branch": "infra/reconcile", + "repo_dir": dir, + }, app) + if err != nil { + t.Fatalf("factory error: %v", err) + } + + result, err := step.Execute(context.Background(), &module.PipelineContext{}) + if err != nil { + t.Fatalf("Execute error: %v", err) + } + + if draft, _ := result.Output["draft"].(bool); !draft { + t.Errorf("expected draft:true, got %v", result.Output) + } + if count, _ := result.Output["count"].(int); count != 1 { + t.Errorf("expected count:1 (only db drifted), got %v", result.Output["count"]) + } + + // Only db drifted → Import must be called exactly once, with pid-db (NOT + // pid-web, which positional indexing would have wrongly selected). + if len(provider.importedCloudIDs) != 1 { + t.Fatalf("expected exactly 1 Import call, got %d: %v", len(provider.importedCloudIDs), provider.importedCloudIDs) + } + if provider.importedCloudIDs[0] != "pid-db" { + t.Errorf("expected Import with correct ProviderID 'pid-db', got %q (positional matching bug)", provider.importedCloudIDs[0]) + } +} + +// TestIaCProviderReconcile_Factory_InvalidTarget verifies the factory rejects +// an unknown target value rather than silently defaulting to branch-push. +func TestIaCProviderReconcile_Factory_InvalidTarget(t *testing.T) { + gitFn := stubGitExecFn(t, 0, nil) + factory := module.NewIaCProviderReconcileStepFactory(gitFn) + _, err := factory("reconcile", map[string]any{ + "provider": "my-provider", + "branch": "infra/reconcile", + "repo_dir": "/tmp", + "target": "yolo-merge", + }, module.NewMockApplication()) + if err == nil { + t.Fatal("expected error for invalid target, got nil") + } + if !containsString(err.Error(), "invalid target") { + t.Errorf("expected 'invalid target' error, got: %v", err) + } +} + +// TestIaCProviderReconcile_Factory_RequiresProvider verifies factory rejects missing provider. +func TestIaCProviderReconcile_Factory_RequiresProvider(t *testing.T) { + gitFn := stubGitExecFn(t, 0, nil) + factory := module.NewIaCProviderReconcileStepFactory(gitFn) + _, err := factory("reconcile", map[string]any{ + "branch": "infra/auto", + "repo_dir": "/tmp", + }, nil) + if err == nil { + t.Fatal("expected error when 'provider' missing") + } +} + +// TestIaCProviderReconcile_Factory_PanicOnNilGitFn verifies the factory panics when +// gitFn is nil. +func TestIaCProviderReconcile_Factory_PanicOnNilGitFn(t *testing.T) { + defer func() { + if r := recover(); r == nil { + t.Error("expected panic when gitFn is nil") + } + }() + module.NewIaCProviderReconcileStepFactory(nil) //nolint:staticcheck // intentional panic test +} diff --git a/plugins/platform/plugin.go b/plugins/platform/plugin.go index 9884ea01..7183354c 100644 --- a/plugins/platform/plugin.go +++ b/plugins/platform/plugin.go @@ -5,6 +5,10 @@ package platform import ( "context" + "fmt" + "os" + "os/exec" + "strings" "github.com/GoCodeAlone/modular" "github.com/GoCodeAlone/workflow/handlers" @@ -23,6 +27,53 @@ func iacProviderApplyFn(ctx context.Context, p interfaces.IaCProvider, plan *int return wfctlhelpers.ApplyPlanWithHooks(ctx, p, plan, wfctlhelpers.ApplyPlanHooks{}) } +// gitExecFn is the production GitExecFn passed to NewIaCCommitBackStepFactory +// and NewIaCProviderReconcileStepFactory. It runs git/gh commands HOST-NATIVE +// via os/exec: the engine committing back to its own repository is not +// untrusted-code execution, and a per-call ephemeral sandbox cannot preserve +// git working-tree state across commands or see the YAML the step wrote into +// repo_dir. (The standard-profile Docker sandbox is retained elsewhere for the +// remote-runner's arbitrary commands.) +// +// argv is the complete argument vector with the binary as argv[0]; it is run +// directly with no shell. The command inherits the host environment (so +// GH_TOKEN/GITHUB_TOKEN are forwarded automatically) with env merged over it. +// workDir is the git working directory. +func gitExecFn(ctx context.Context, argv []string, env map[string]string, workDir string) (string, error) { + if len(argv) == 0 { + return "", fmt.Errorf("gitExecFn: empty argv") + } + // argv[0] is always a literal binary name ("git"/"gh") set by the step code, + // and the remaining args are step-controlled and run with no shell, so there + // is no command-injection surface. The engine committing to its own repo is + // trusted; this is the explicit host-native design (see comment above). + cmd := exec.CommandContext(ctx, argv[0], argv[1:]...) //nolint:gosec // G204: trusted literal binary + no shell; see func doc + cmd.Dir = workDir + // Build the environment from a map so that explicit overrides reliably win: + // appending env over a copy of os.Environ() would leave DUPLICATE KEY= + // entries, and some platforms honor the FIRST occurrence — meaning a + // GH_TOKEN override might not take effect. Materializing from a map yields + // exactly one entry per key with the override applied last. + envMap := make(map[string]string) + for _, kv := range os.Environ() { + if eq := strings.IndexByte(kv, '='); eq >= 0 { + envMap[kv[:eq]] = kv[eq+1:] + } + } + for k, v := range env { + envMap[k] = v + } + cmd.Env = make([]string, 0, len(envMap)) + for k, v := range envMap { + cmd.Env = append(cmd.Env, k+"="+v) + } + out, err := cmd.CombinedOutput() + if err != nil { + return "", fmt.Errorf("gitExecFn: %v: %w: %s", argv, err, strings.TrimSpace(string(out))) + } + return strings.TrimSpace(string(out)), nil +} + // Plugin is the platform EnginePlugin. type Plugin struct { plugin.BaseEnginePlugin @@ -44,7 +95,7 @@ func New() *Plugin { Description: "Platform infrastructure modules, workflow handler, reconciliation trigger, and template step", Tier: plugin.TierCore, ModuleTypes: []string{"platform.provider", "platform.resource", "platform.context", "platform.kubernetes", "platform.dns", "platform.region", "platform.region_router", "iac.state", "app.container", "argo.workflows"}, - StepTypes: []string{"step.platform_template", "step.k8s_plan", "step.k8s_apply", "step.k8s_status", "step.k8s_destroy", "step.iac_plan", "step.iac_apply", "step.iac_status", "step.iac_destroy", "step.iac_drift_detect", "step.iac_provider_list", "step.iac_provider_catalog", "step.iac_provider_plan", "step.iac_provider_apply", "step.iac_provider_destroy", "step.iac_provider_drift", "step.iac_secret_reachability", "step.dns_plan", "step.dns_apply", "step.dns_status", "step.app_deploy", "step.app_status", "step.app_rollback", "step.region_deploy", "step.region_promote", "step.region_failover", "step.region_status", "step.region_weight", "step.region_sync", "step.argo_submit", "step.argo_status", "step.argo_logs", "step.argo_delete", "step.argo_list"}, + StepTypes: []string{"step.platform_template", "step.k8s_plan", "step.k8s_apply", "step.k8s_status", "step.k8s_destroy", "step.iac_plan", "step.iac_apply", "step.iac_status", "step.iac_destroy", "step.iac_drift_detect", "step.iac_provider_list", "step.iac_provider_catalog", "step.iac_provider_plan", "step.iac_provider_apply", "step.iac_provider_destroy", "step.iac_provider_drift", "step.iac_secret_reachability", "step.iac_commit_back", "step.iac_provider_reconcile", "step.dns_plan", "step.dns_apply", "step.dns_status", "step.app_deploy", "step.app_status", "step.app_rollback", "step.region_deploy", "step.region_promote", "step.region_failover", "step.region_status", "step.region_weight", "step.region_sync", "step.argo_submit", "step.argo_status", "step.argo_logs", "step.argo_delete", "step.argo_list"}, TriggerTypes: []string{"reconciliation"}, WorkflowTypes: []string{"platform"}, }, @@ -155,6 +206,12 @@ func (p *Plugin) StepFactories() map[string]plugin.StepFactory { "step.iac_secret_reachability": func(name string, cfg map[string]any, app modular.Application) (any, error) { return module.NewIaCSecretReachabilityStepFactory()(name, cfg, app) }, + "step.iac_commit_back": func(name string, cfg map[string]any, app modular.Application) (any, error) { + return module.NewIaCCommitBackStepFactory(gitExecFn)(name, cfg, app) + }, + "step.iac_provider_reconcile": func(name string, cfg map[string]any, app modular.Application) (any, error) { + return module.NewIaCProviderReconcileStepFactory(gitExecFn)(name, cfg, app) + }, "step.dns_plan": func(name string, cfg map[string]any, app modular.Application) (any, error) { return module.NewDNSPlanStepFactory()(name, cfg, app) }, diff --git a/plugins/platform/plugin_test.go b/plugins/platform/plugin_test.go index 8d29559b..48e1f67c 100644 --- a/plugins/platform/plugin_test.go +++ b/plugins/platform/plugin_test.go @@ -48,6 +48,8 @@ func TestStepFactories(t *testing.T) { "step.iac_provider_destroy", "step.iac_provider_drift", "step.iac_secret_reachability", + "step.iac_commit_back", + "step.iac_provider_reconcile", "step.dns_plan", "step.dns_apply", "step.dns_status", diff --git a/schema/module_schema.go b/schema/module_schema.go index cabdd1c4..463dbdfd 100644 --- a/schema/module_schema.go +++ b/schema/module_schema.go @@ -2849,6 +2849,7 @@ func (r *ModuleSchemaRegistry) registerBuiltins() { {"step.gitlab_pipeline_status", "GitLab Pipeline Status", "Gets the status of a GitLab pipeline"}, {"step.gitlab_trigger_pipeline", "GitLab Trigger Pipeline", "Triggers a GitLab CI/CD pipeline"}, {"step.iac_apply", "IaC Apply", "Applies infrastructure changes"}, + {"step.iac_commit_back", "IaC Commit Back", "Commits serialised authored specs back to git after a full-success apply"}, {"step.iac_destroy", "IaC Destroy", "Destroys IaC-managed infrastructure"}, {"step.iac_drift_detect", "IaC Drift Detect", "Detects IaC configuration drift"}, {"step.iac_plan", "IaC Plan", "Plans infrastructure changes without applying"}, @@ -2858,6 +2859,7 @@ func (r *ModuleSchemaRegistry) registerBuiltins() { {"step.iac_provider_drift", "IaC Provider Drift", "Detects drift via IaCProvider service"}, {"step.iac_provider_list", "IaC Provider List", "Lists resource statuses from IaCProvider service"}, {"step.iac_provider_plan", "IaC Provider Plan", "Plans infrastructure changes via IaCProvider service"}, + {"step.iac_provider_reconcile", "IaC Provider Reconcile", "Drift → import → approximate cloud-snapshot YAML → draft PR (review-required)"}, {"step.iac_secret_reachability", "IaC Secret Reachability", "Pre-flight gate: reports whether a plan's secret:// refs are reachable from the chosen exec-env"}, {"step.iac_status", "IaC Status", "Gets IaC provisioning status"}, {"step.k8s_apply", "K8s Apply", "Applies Kubernetes manifests"}, diff --git a/schema/schema.go b/schema/schema.go index 06a7f7af..89867a22 100644 --- a/schema/schema.go +++ b/schema/schema.go @@ -309,6 +309,7 @@ var coreModuleTypes = []string{ "step.http_call", "step.http_proxy", "step.iac_apply", + "step.iac_commit_back", "step.iac_destroy", "step.iac_drift_detect", "step.iac_plan", @@ -318,6 +319,7 @@ var coreModuleTypes = []string{ "step.iac_provider_drift", "step.iac_provider_list", "step.iac_provider_plan", + "step.iac_provider_reconcile", "step.iac_secret_reachability", "step.iac_status", "step.jq", diff --git a/schema/step_schema_builtins.go b/schema/step_schema_builtins.go index 7689e228..7b37751c 100644 --- a/schema/step_schema_builtins.go +++ b/schema/step_schema_builtins.go @@ -2233,6 +2233,51 @@ func (r *StepSchemaRegistry) registerBuiltins() { }, }) + // ---- IaC Commit Back ---- + + r.Register(&StepSchema{ + Type: "step.iac_commit_back", + Plugin: "platform", + Description: "Serialises the authored resource specs to YAML via iac/specgen.SpecToYAML and commits the result back to a git branch after a full-success apply. On partial apply returns {committed:false, reason:\"partial-apply\"} without committing. On apply-succeeded-but-git-failed returns {state_diverged:true} so the caller can surface HTTP 207. secret:// refs survive verbatim in the serialised YAML.", + ConfigFields: []ConfigFieldDef{ + {Key: "repo_dir", Type: FieldTypeString, Description: "Git working directory the commands run in", Required: true}, + {Key: "branch", Type: FieldTypeString, Description: "Branch name to create and push", Required: true}, + {Key: "message", Type: FieldTypeString, Description: "Git commit message (default: chore: commit back applied infrastructure specs)"}, + {Key: "target", Type: FieldTypeString, Description: "Publish target: 'branch-push' (default; git push) or 'gh-pr' (gh pr create --fill)"}, + {Key: "apply_result_from", Type: FieldTypeString, Description: "Context path to the upstream apply step result (default: steps.apply.apply_result)"}, + {Key: "specs", Type: FieldTypeArray, Description: "Static authored specs to serialise (mutually exclusive with specs_from)"}, + {Key: "specs_from", Type: FieldTypeString, Description: "Context path to the specs (mutually exclusive with specs)"}, + }, + Outputs: []StepOutputDef{ + {Key: "committed", Type: "boolean", Description: "Whether a commit was produced"}, + {Key: "ref", Type: "string", Description: "Branch or PR reference (set when committed:true)"}, + {Key: "state_diverged", Type: "boolean", Description: "True when apply succeeded but git failed (HTTP 207 scenario)"}, + {Key: "reason", Type: "string", Description: "Reason when committed:false or state_diverged:true"}, + }, + }) + + // ---- IaC Provider Reconcile ---- + + r.Register(&StepSchema{ + Type: "step.iac_provider_reconcile", + Plugin: "platform", + Description: "Drift → import → approximate cloud-snapshot YAML → draft PR. APPROXIMATE: the YAML is a cloud snapshot, NOT a faithful reconstruction of authored specs (no SpecToYAML). The draft PR body carries the mandatory disclaimer: 'imported from cloud; approximate; does NOT reconstruct your secret:// refs — review before merge'. On git failure returns {draft:false, state_diverged:true}. Use step.iac_commit_back for authoritative spec commits.", + ConfigFields: []ConfigFieldDef{ + {Key: "provider", Type: FieldTypeString, Description: "Name of the registered IaCProvider service", Required: true}, + {Key: "branch", Type: FieldTypeString, Description: "Branch name for the draft commit (default: infra/reconcile)"}, + {Key: "target", Type: FieldTypeString, Description: "Publish target: 'branch-push' (default; git push) or 'gh-pr' (gh pr create --draft)"}, + {Key: "repo_dir", Type: FieldTypeString, Description: "Git working directory the commands run in", Required: true}, + }, + Outputs: []StepOutputDef{ + {Key: "draft", Type: "boolean", Description: "True iff a draft commit/PR was produced (false when no drift detected or git failed)"}, + {Key: "ref", Type: "string", Description: "Branch or PR reference (set when draft:true)"}, + {Key: "warning", Type: "string", Description: "The mandatory disclaimer string"}, + {Key: "count", Type: "number", Description: "Number of drifted resources detected"}, + {Key: "state_diverged", Type: "boolean", Description: "True when drift was detected but the git commit/push failed"}, + {Key: "reason", Type: "string", Description: "Reason when state_diverged:true"}, + }, + }) + // ---- Kubernetes Apply ---- r.Register(&StepSchema{ diff --git a/schema/testdata/editor-schemas.golden.json b/schema/testdata/editor-schemas.golden.json index fab7b3e0..0b5b1c7d 100644 --- a/schema/testdata/editor-schemas.golden.json +++ b/schema/testdata/editor-schemas.golden.json @@ -5992,6 +5992,13 @@ "description": "Applies infrastructure changes", "configFields": [] }, + "step.iac_commit_back": { + "type": "step.iac_commit_back", + "label": "IaC Commit Back", + "category": "pipeline", + "description": "Commits serialised authored specs back to git after a full-success apply", + "configFields": [] + }, "step.iac_destroy": { "type": "step.iac_destroy", "label": "IaC Destroy", @@ -6055,6 +6062,13 @@ "description": "Plans infrastructure changes via IaCProvider service", "configFields": [] }, + "step.iac_provider_reconcile": { + "type": "step.iac_provider_reconcile", + "label": "IaC Provider Reconcile", + "category": "pipeline", + "description": "Drift → import → approximate cloud-snapshot YAML → draft PR (review-required)", + "configFields": [] + }, "step.iac_secret_reachability": { "type": "step.iac_secret_reachability", "label": "IaC Secret Reachability",