diff --git a/.claude/agents/learning-reviewer.md b/.claude/agents/learning-reviewer.md new file mode 100644 index 0000000..e3c9db6 --- /dev/null +++ b/.claude/agents/learning-reviewer.md @@ -0,0 +1,19 @@ +--- +name: learning-reviewer +description: Verifies the Terraform workflow actually teaches, not just automates — checks just-in-time moments, ExplanationDepth gating, and evidence recording. Use after any diff touching src/devops_learn/workflows/terraform_flow.py, docs/terraform-state.md, docs/learning-model.md, docs/roadmap.md, or docs/safety.md. +tools: Read, Grep, Glob +--- + +You evaluate whether this milestone genuinely closes the user's Terraform/Azure knowledge gaps, per `docs/learning-model.md`, or merely automates the work. You are read-only: report findings, do not edit anything. + +Check specifically: + +- `workflows/terraform_flow.py` calls `_teach_state_concepts` before `fmt`, `_teach_provider_concept` before `init`, and the post-plan "what to check" explanation after a successful plan — confirm these land at the point where the concept is actually relevant, not front-loaded before anything runs. +- Teaching moments use `LearningMoment`/`Explanation` via `ExplanationService.render`/`render_learning_moment`, gated by `ExplanationDepth`, not hardcoded to always or never show — confirm the depth-gating logic in `explanations/service.py` is genuinely exercised here (mode/depth aren't hardcoded to force max verbosity every time). +- A `terraform plan` failure due to missing Azure credentials produces a real diagnosis (`_diagnose_plan_failure`) explaining `az login`/`ARM_CLIENT_ID`/`ARM_CLIENT_SECRET`/`ARM_TENANT_ID`/`ARM_SUBSCRIPTION_ID` as the WHY, not just a bare failure message — this is the expected outcome on a credential-less machine and should be a genuine learning moment, not treated as an error to hide. +- `ExperienceTracker.record` calls in `terraform_flow.py` use only existing `CompetencyArea` (`terraform`, `azure`) and `ExperienceState` (`introduced`, `practiced`, `demonstrated`, etc.) vocabulary — no invented competency areas or states. +- `docs/terraform-state.md` actually covers: desired vs. actual state, the state file, provider, resource address, dependencies, drift, local vs. remote state, locking, state sensitivity, why deleting state ≠ deleting infrastructure, why hand-editing state is dangerous. +- `docs/terraform-state.md` is cross-linked from `docs/learning-model.md`'s "Just-in-time learning" section. +- `docs/roadmap.md`/`docs/safety.md`/README updates describe only what is genuinely implemented this milestone (real `fmt`/`init`/`validate`/`plan` against the bundled sample project; no real Azure execution, no `apply`/`destroy`) — flag any language that overstates real Azure deployment having happened. + +Report findings ranked BLOCKING/HIGH/MEDIUM/LOW with the specific file and line. diff --git a/.claude/agents/security-reviewer.md b/.claude/agents/security-reviewer.md new file mode 100644 index 0000000..0c6df89 --- /dev/null +++ b/.claude/agents/security-reviewer.md @@ -0,0 +1,20 @@ +--- +name: security-reviewer +description: Reviews for secret leakage, subprocess safety, and approval-gate integrity across RealTerraformTool, CI, and Claude Code hooks. Use after any diff touching src/devops_learn/tools/terraform_tool.py, src/devops_learn/tools/_subprocess_safety.py, .github/workflows/ci.yml, or .claude/settings.json. +tools: Read, Grep, Glob, Bash +--- + +You review this repo for credential handling, subprocess safety, and approval-gate integrity. You are read-only: report findings, do not fix them yourself. + +Check specifically: + +- `tools/_subprocess_safety.py`'s `redact()`/`run_safely()` are actually applied to every stdout/stderr that lands in a `ToolResult.details` from `RealTerraformTool` — grep for any `result.stdout`/`result.stderr` usage that bypasses `_subprocess_safety`. +- `RealTerraformTool.plan()`'s parsing of `terraform show -json` (`_details_from_plan_json`) never extracts `resource_changes[].change.before`/`.after` — only `{address, action}`. This is the primary defense against leaking sensitive plan data; confirm no future edit reintroduced raw attribute access. +- Every subprocess call in `terraform_tool.py` goes through `run_safely(..., timeout=...)` with an explicit timeout — none should call `subprocess.run` directly. +- `ToolOperationSpec` invariants hold: any operation with `risk_level=RiskLevel.DESTRUCTIVE` has `requires_approval=True` and `supports_dry_run=False` (enforced by `__post_init__`, but confirm no new operation tries to work around it, e.g. via a different risk level for something that is actually destructive). +- `.github/workflows/ci.yml`'s Terraform steps never reference `ARM_CLIENT_ID`/`ARM_CLIENT_SECRET`/`ARM_TENANT_ID`/`ARM_SUBSCRIPTION_ID` or any Azure credential, and never run `terraform plan`/`apply` (only `fmt -check`/`init -backend=false`/`validate`, none of which need credentials). +- `.gitignore` covers `.terraform/`, `*.tfstate*`, and crash logs; `.terraform.lock.hcl` is NOT ignored. +- `.claude/settings.json`'s `PreToolUse` hook (`.claude/hooks/confirm_destructive_command.py`) actually matches `terraform apply`, `terraform destroy`, `terraform state rm`, `az group delete`, `az resource delete`, `az role assignment (create|delete)`, `rm terraform.tfstate*`, `rm -rf .terraform`, and `git push --force` — test it yourself with sample stdin JSON rather than trusting the regex by inspection alone. +- No API keys, tokens, `.env` contents, or Terraform state ever appear in a commit, test fixture, or doc in this diff — grep for anything secret-shaped. + +Report findings ranked BLOCKING/HIGH/MEDIUM/LOW with the specific file and line. diff --git a/.claude/agents/terraform-reviewer.md b/.claude/agents/terraform-reviewer.md new file mode 100644 index 0000000..14a7e25 --- /dev/null +++ b/.claude/agents/terraform-reviewer.md @@ -0,0 +1,20 @@ +--- +name: terraform-reviewer +description: Reviews changes to *.tf files and RealTerraformTool for correctness and idiomatic Terraform. Use after any diff touching projects/api_platform/infra/terraform/ or src/devops_learn/tools/terraform_tool.py. +tools: Read, Grep, Glob, Bash +--- + +You review Terraform configuration and the RealTerraformTool implementation in this repo. You are read-only: report findings, do not rewrite the implementation. + +Check specifically: + +- Provider version is pinned (`~> X.Y`, never unpinned) in `providers.tf`. +- Resource naming and tags are consistent with `main.tf` (`project`, `environment`, `managed-by`, `purpose`). +- `.gitignore` correctly excludes `.terraform/` and `*.tfstate*` while `.terraform.lock.hcl` stays committed (verify it is not accidentally ignored). +- The `providers.tf`/`variables.tf`/`main.tf`/`outputs.tf` split in `projects/api_platform/infra/terraform/` stays a naming convention only — Terraform merges all `.tf` files in the directory, so nothing should assume module-style isolation between them. +- `RealTerraformTool` (`src/devops_learn/tools/terraform_tool.py`) only declares `fmt`/`init`/`validate`/`plan` operations, all `RiskLevel.SAFE` with `requires_approval=False` — flag immediately if `apply_approved_plan`/`destroy_approved_environment` are ever added to it (that's Milestone 3 territory and must go through `ApprovalGate`, not this tool). +- `terraform show -json` action-mapping (`_details_from_plan_json`) correctly classifies `("create",)`/`("update",)`/`("delete","create")` or `("create","delete")`/`("delete",)` into create/change/replace/destroy, and ignores `("no-op",)`/`("read",)`. +- No `shell=True` anywhere; every subprocess call is an argument list through `_subprocess_safety.run_safely`. +- You can run `terraform fmt -check -recursive` and `terraform validate` (after `terraform init -backend=false`) yourself against `projects/api_platform/infra/terraform/` to verify claims rather than reading code only. + +Report findings ranked BLOCKING/HIGH/MEDIUM/LOW with the specific file and line. diff --git a/.claude/hooks/confirm_destructive_command.py b/.claude/hooks/confirm_destructive_command.py new file mode 100755 index 0000000..b422c84 --- /dev/null +++ b/.claude/hooks/confirm_destructive_command.py @@ -0,0 +1,70 @@ +#!/usr/bin/env python3 +"""PreToolUse hook: forces explicit user confirmation before a Bash command +that could destroy infrastructure or discard history runs, instead of +letting it execute silently. + +This is a defense-in-depth backstop, not the platform's real approval +mechanism -- the platform's own Terraform/Azure/Docker execution always goes +through ToolService/ApprovalGate regardless of this hook (see +docs/adr/0004-controlled-tool-execution.md). This hook only matters when a +command bypasses that layer entirely, e.g. an agent running `terraform +apply` directly via the Bash tool instead of through RealTerraformTool. + +Reads the PreToolUse JSON payload from stdin, and if tool_input.command +matches a dangerous pattern, prints a permissionDecision "ask" response so +the normal permission prompt fires instead of silent execution. Any other +command is left untouched (no stdout, exit 0) -- this must not add friction +to routine commands like `terraform fmt`/`terraform plan`/`pytest`. +""" +from __future__ import annotations + +import json +import re +import sys + +_DANGEROUS_PATTERNS = ( + r"\bterraform\s+apply\b", + r"\bterraform\s+destroy\b", + r"\bterraform\s+state\s+rm\b", + r"\baz\s+group\s+delete\b", + r"\baz\s+resource\s+delete\b", + r"\baz\s+role\s+assignment\s+(create|delete)\b", + r"\brm\s+.*terraform\.tfstate", + r"\brm\s+-rf\s+.*\.terraform\b", + r"\bgit\s+push\b.*(--force\b|(? int: + try: + payload = json.load(sys.stdin) + except (json.JSONDecodeError, ValueError): + return 0 + + tool_input = payload.get("tool_input") if isinstance(payload, dict) else None + command = tool_input.get("command", "") if isinstance(tool_input, dict) else "" + command = str(command) + if not _COMPILED.search(command): + return 0 + + print( + json.dumps( + { + "hookSpecificOutput": { + "hookEventName": "PreToolUse", + "permissionDecision": "ask", + "permissionDecisionReason": ( + "This command can destroy infrastructure or discard history " + "outside the platform's own ToolService approval gate. " + "Confirm you intend to run it before proceeding." + ), + } + } + ) + ) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/.claude/settings.json b/.claude/settings.json new file mode 100644 index 0000000..993f542 --- /dev/null +++ b/.claude/settings.json @@ -0,0 +1,15 @@ +{ + "hooks": { + "PreToolUse": [ + { + "matcher": "Bash", + "hooks": [ + { + "type": "command", + "command": "python3 \"${CLAUDE_PROJECT_DIR}/.claude/hooks/confirm_destructive_command.py\"" + } + ] + } + ] + } +} diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cd2b278..5c2a466 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -23,3 +23,14 @@ jobs: run: cd projects/api_platform && mypy app --strict - name: Test run: pytest + - name: Setup Terraform + uses: hashicorp/setup-terraform@v3 + with: + terraform_version: "1.15.8" + terraform_wrapper: false + - name: Terraform fmt check + run: terraform fmt -check -recursive projects/api_platform/infra/terraform + - name: Terraform init (no backend, no credentials) + run: terraform -chdir=projects/api_platform/infra/terraform init -backend=false -input=false + - name: Terraform validate + run: terraform -chdir=projects/api_platform/infra/terraform validate diff --git a/.gitignore b/.gitignore index e6c3749..01305ff 100644 --- a/.gitignore +++ b/.gitignore @@ -13,3 +13,20 @@ htmlcov/ *.sqlite3 .env .devops_learn/ + +# Terraform working-directory artifacts (see projects/api_platform/infra/terraform/). +# .terraform.lock.hcl is deliberately NOT listed here: it pins exact provider +# versions/hashes so `terraform init` is reproducible across machines and CI, +# which is normal, recommended Terraform practice -- unlike .terraform/ (a +# regenerable local plugin cache) and *.tfstate (can contain sensitive values, +# and since this milestone never applies, would only ever hold stale/empty +# state if committed). +**/.terraform/ +*.tfstate +*.tfstate.* +crash.log +crash.*.log +override.tf +override.tf.json +*_override.tf +*_override.tf.json diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..285966d --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,90 @@ +# DevOps-Learn-by-Doing + +## Product + +This is an AI-assisted DevOps mastery platform, not an automation tool and +not an anti-AI exercise. The goal is real engineering work performed +together with AI in a way that deliberately closes the user's knowledge +gaps. AI automation and learner understanding are two separate dimensions: +running a command for the user is not the same as the user understanding +why it ran or what it did. See `docs/learning-model.md`. Never equate manual +typing with expertise, and never let AI execution stand in for competency +evidence — evidence lives in `ExperienceTracker`/`ExperienceState` +(`experience/tracker.py`, `domain/enums.py`). + +## Architecture + +Modular monolith (`docs/architecture.md`, `docs/adr/0001`). `bootstrap.py` +is the single composition root — all concrete implementations are wired +there via constructor injection, never a service locator. `domain/` depends +on nothing; `tools/`/`cloud/`/`ai/` depend only on `domain/`; +`workflows/`/`bootstrap.py` depend on everything; `cli/` depends only on +`bootstrap.py`/`workflows/`/`domain/`. Preserve this layering unless there +is a strong reason to change it. + +Every capability the platform can execute is a `Tool` (`tools/base.py`). +`ToolService.invoke` (`tools/service.py`) is the only caller of +`Tool.execute` anywhere in the codebase — this is what makes "destructive +operations require human approval" structural rather than a convention any +one caller has to remember. Never bypass `ToolService` for convenience, and +never call `Tool.execute` directly outside it. + +LLMs explain and reason; deterministic code decides. `LLMProvider` +(`ai/provider.py`) has no method that invokes a tool or makes a decision — +it only produces freeform explanation text. Recommendations, risk +classification, and plans are typed dataclasses produced by deterministic +services (`docs/adr/0008-structured-ai-output.md`), never freeform LLM +output parsed at runtime. + +## Safety + +- Never fabricate successful execution. Every real result is explicitly + labeled `(real)`/`(real, failed)`; every simulated result says + `(simulated)`. See `docs/safety.md` for the current real-vs-simulated + boundary — keep it accurate as implementation changes. +- `ToolOperationSpec` (`tools/base.py`) enforces at construction time that a + `RiskLevel.DESTRUCTIVE` operation must require approval and must never + support a dry-run bypass of that approval. Do not work around this. +- `ApprovalGate`/`CliApprovalGate` (`tools/approval.py`) is the only path to + approving a HIGH/DESTRUCTIVE operation; `AutoApproveApprovalGate`/ + `AutoDenyApprovalGate` exist only for tests, never for the real CLI. +- Never commit credentials, Terraform state, or `.env` files. Real + subprocess-backed tools must redact secret-shaped output before it lands + in a `ToolResult` (see `tools/_subprocess_safety.py`) and must set an + explicit timeout on every subprocess call. +- Never expose unrestricted arbitrary shell execution through a `Tool`. Real + tools shell out to a narrow, allow-listed set of operations via argument + arrays (never `shell=True`) — see `tools/docker_tool.py`, + `tools/python_tool.py`, `tools/terraform_tool.py`. + +## Product direction + +- Azure first; Terraform depth before AWS/GCP breadth (`docs/cloud-model.md`). +- Do not add Kubernetes/AKS until a real cloud deployment path is proven + reliable without it — not because Kubernetes is technically harder, but + because cluster-level complexity should sit on top of a working real + deployment path, not substitute for one (`docs/roadmap.md`). +- Prefer understandable learning infrastructure over enterprise complexity — + no module registries, landing zones, Terragrunt, or multi-subscription + setups unless a milestone genuinely requires them. +- See `docs/roadmap.md` for current milestone status before proposing new + scope. + +## Quality + +Before considering any change complete, all of the following must pass: + +``` +pytest +flake8 src tests projects/api_platform/app projects/api_platform/tests +mypy +cd projects/api_platform && mypy app --strict +``` + +Update `docs/roadmap.md`/`docs/safety.md` when implementation reality +changes — this repo's credibility depends on never claiming more than what +is genuinely implemented (e.g. do not describe real Azure execution as +working unless it has actually been verified against a real subscription). + +Do not put task-specific notes, implementation plans, or milestone journals +in this file — it is for durable project knowledge only. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 8674c22..a15439b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -29,6 +29,11 @@ existing `CloudConcept`. See `docs/cloud-model.md`. ## Adding a tool -All tool operations are simulated in V1. Implement the `Tool` interface in -`src/devops_learn/tools/`, declare accurate `risk_level`/`is_destructive` metadata, and -never bypass `ToolService.invoke`. See `docs/safety.md`. +Implement the `Tool` interface in `src/devops_learn/tools/`, declare accurate +`risk_level`/`is_destructive` metadata, and never bypass `ToolService.invoke`. Provide a +`Simulated*Tool` for tests/offline use and a `Real*Tool` that shells out to the actual binary +via an argument list (never `shell=True`), an explicit timeout, and +`tools/_subprocess_safety.py`'s redaction. See `docs/safety.md` and `docs/development.md`. + +Any test needing real Azure credentials must be gated behind `RUN_AZURE_INTEGRATION_TESTS=1` so +normal CI never attempts to reach Azure. diff --git a/README.md b/README.md index 43882cc..ed3a6c7 100644 --- a/README.md +++ b/README.md @@ -24,8 +24,14 @@ Project -> Analysis -> Requirements -> Questions -> Recommendations -> Architect questions that cannot be inferred safely. - **Real local execution** (`devops-learn local`) runs actual `pytest`, `flake8`, `docker build`, `docker run`, and HTTP health checks against the project. -- **Simulated cloud/Terraform/Kubernetes** execution for safe learning without - credentials or cost (`devops-learn analyze` without `--real-tools`). +- **Real Terraform execution** (`devops-learn terraform`) runs actual + `terraform fmt`/`init`/`validate`/`plan` against a real, committed Azure + configuration, with structured `terraform show -json` parsing feeding a + deterministic risk classifier. No `apply`/`destroy` yet; `plan` requires + Azure authentication and fails cleanly, with an explanation, without it. +- **Simulated cloud/Kubernetes** execution for safe learning without + credentials or cost (`devops-learn analyze` without `--real-tools`; + Terraform in `analyze`/`review` also stays simulated). - **Controlled tool execution**: every capability is a `Tool` with declared risk level, dry-run support, and human approval; `ToolService` is the only caller of `Tool.execute`. @@ -53,6 +59,10 @@ devops-learn init projects/api_platform # Run a real local vertical slice: test -> lint -> docker build -> run -> verify devops-learn local projects/api_platform +# Run a real Terraform vertical slice: fmt -> init -> validate -> plan -> risk analysis +# (plan requires Azure credentials; fails cleanly with an explanation without them) +devops-learn terraform + # Simulate the full cloud workflow (no credentials, no cost) devops-learn analyze projects/api_platform --mode collaborative --depth learning diff --git a/docs/development.md b/docs/development.md index b150976..01c5247 100644 --- a/docs/development.md +++ b/docs/development.md @@ -35,6 +35,11 @@ devops-learn init projects/api_platform # Real local vertical slice: test -> lint -> docker build -> run -> verify devops-learn local projects/api_platform +# Real Terraform vertical slice: fmt -> init -> validate -> plan -> risk analysis +# (plan requires Azure credentials -- az login, or ARM_CLIENT_ID/ARM_CLIENT_SECRET/ +# ARM_TENANT_ID/ARM_SUBSCRIPTION_ID -- and fails cleanly, with an explanation, without them) +devops-learn terraform + # Simulate the full cloud workflow (no credentials, no cost) devops-learn analyze projects/api_platform --mode collaborative --depth learning @@ -84,7 +89,17 @@ declaration at construction time. Provide a `Simulated*Tool` for tests and environments where the real dependency is unavailable, and a `Real*Tool` that shells out to the actual binary. See `tools/python_tool.py` and -`tools/docker_tool.py` for the pattern. +`tools/docker_tool.py` for the base pattern, and `tools/terraform_tool.py` for a `Real*Tool` +that also needs an explicit subprocess timeout and output redaction — reuse +`tools/_subprocess_safety.py`'s `run_safely()`/`redact()` rather than calling `subprocess.run` +directly, so secret-shaped output can never accidentally reach a `ToolResult` unredacted. + +Any future test that requires real Azure credentials must be gated behind +`RUN_AZURE_INTEGRATION_TESTS=1` (e.g. +`@pytest.mark.skipif(os.environ.get("RUN_AZURE_INTEGRATION_TESTS") != "1", reason="requires az +login")`), so ordinary CI and `pytest` runs never attempt to reach Azure. Nothing in the +platform needs this yet — it is the convention Milestone 3 (`docs/roadmap.md`) will use as soon +as real Azure calls exist. ## Adding a workflow command diff --git a/docs/learning-model.md b/docs/learning-model.md index b2ca996..1484e5a 100644 --- a/docs/learning-model.md +++ b/docs/learning-model.md @@ -92,6 +92,12 @@ course. When the platform generates or runs Terraform, it explains: The depth of each of those points depends on the learner profile and the current explanation depth. +`devops-learn terraform` applies this to Terraform state specifically: +`workflows/terraform_flow.py` surfaces 2-3 state/provider concepts per run, +tied to the command about to execute, rather than the full picture at once. +See `docs/terraform-state.md` for the complete reference those moments link +back to. + ## AI-generated work must be reviewable Before significant execution the platform shows: diff --git a/docs/roadmap.md b/docs/roadmap.md index 9c479d1..3680643 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -26,15 +26,20 @@ allow-listed subprocess wrapper, gated by `ToolService.invoke`. offline/no-Docker environments. The `devops-learn local` command runs the full local vertical slice end to end and teaches Docker concepts in context. -## Milestone 2: real Terraform workflow - -Add `RealTerraformTool` shelling out to a real `terraform` binary for -`fmt`/`validate`/`plan` against generated configuration, with structured -parsing of `terraform show -json` feeding the existing -`validation/terraform_plan_analysis.analyze()` risk classifier unchanged. No -`apply` and no Azure credentials required yet; -`apply_approved_plan`/`destroy_approved_environment` stay simulated until -Milestone 3. +## Milestone 2: real Terraform workflow (done) + +`RealTerraformTool` (`tools/terraform_tool.py`) shells out to a real +`terraform` binary for `fmt`/`init`/`validate`/`plan` against a real, +committed configuration (`projects/api_platform/infra/terraform/`), with +structured parsing of `terraform show -json` feeding the existing +`validation/terraform_plan_analysis.analyze()` risk classifier unchanged. +`devops-learn terraform` runs the full vertical slice (fmt -> init -> +validate -> plan -> risk analysis) and teaches Terraform state, provider, +and resource-address concepts just-in-time (`docs/terraform-state.md`). No +`apply`; `apply_approved_plan`/`destroy_approved_environment` are not even +declared on `RealTerraformTool` and stay simulated until Milestone 3. `plan` +requires Azure authentication and fails cleanly, with an explanation, on a +machine without it — `init`/`validate`/`fmt` need no credentials. ## Milestone 3: first Azure vertical slice diff --git a/docs/safety.md b/docs/safety.md index a5547fa..eb15dea 100644 --- a/docs/safety.md +++ b/docs/safety.md @@ -10,6 +10,13 @@ - `devops-learn local ` runs real `pytest`, `flake8`, `docker build`, `docker run`, `docker logs`, and `docker stop` against the project you provide, and performs a real HTTP health check against the running container. +- `devops-learn terraform` runs real `terraform fmt`, `init`, `validate`, and `plan` (including + `terraform show -json`) against `projects/api_platform/infra/terraform/`. It never applies or + destroys anything. `init` downloads the `azurerm` provider plugin (network access, no Azure + credentials required); `plan` requires Azure authentication (`az login`, or + `ARM_CLIENT_ID`/`ARM_CLIENT_SECRET`/`ARM_TENANT_ID`/`ARM_SUBSCRIPTION_ID`) to build the + provider client and fails cleanly, with an explanation, without it — see "Real Terraform + execution" below. - The platform's own persistence (SQLite), audit log, decisions, experience tracking, and learner profile are real. - The reference Dockerfile, Terraform configuration, Kubernetes manifest, and GitHub Actions @@ -60,6 +67,27 @@ resources, applies Terraform, or modifies remote state. It: - stops the container at the end of the workflow, - clearly labels every real result with `(real)`. +## Real Terraform execution + +`devops-learn terraform` is the only command that uses `RealTerraformTool` +(`tools/terraform_tool.py`) — `devops-learn analyze --real-tools` and `devops-learn local` both +still use `SimulatedTerraformTool` for the `terraform` tool name; only `python`/`docker` become +real for those commands. `RealTerraformTool` declares only `fmt`/`init`/`validate`/`plan` — +`apply_approved_plan`/`destroy_approved_environment` are not implemented on it at all, so +invoking either raises `KeyError` rather than silently falling back to simulated behavior. Real +apply/destroy stay simulated until Milestone 3 (`docs/roadmap.md`). + +Every subprocess call goes through `tools/_subprocess_safety.py::run_safely`, which sets an +explicit timeout and always redacts secret-shaped `KEY=value` text and truncates long output +before it reaches a `ToolResult`. `plan()`'s parsing of `terraform show -json` never extracts +raw resource attribute values (`before`/`after`) — only `{address, action}` per resource change +— which is the primary defense against leaking sensitive plan data; free-text redaction is +defense in depth on top of that. + +A `terraform plan` failure due to missing Azure credentials is an expected, honest outcome on a +machine with no `az login`/`ARM_*` environment variables configured — `workflows/terraform_flow.py` +diagnoses it explicitly rather than treating it as a bug, and never fabricates a successful plan. + ## Terraform plan risk `validation/terraform_plan_analysis.analyze()` classifies a plan's `create`/`change`/`replace`/ diff --git a/docs/terraform-state.md b/docs/terraform-state.md new file mode 100644 index 0000000..d384451 --- /dev/null +++ b/docs/terraform-state.md @@ -0,0 +1,94 @@ +# Terraform state + +`devops-learn terraform` (`workflows/terraform_flow.py`) surfaces a subset of +these concepts just-in-time, right before the command that makes them +relevant -- not as a front-loaded course. This doc is the fuller reference +those in-flow moments link back to. + +## Desired state vs. actual infrastructure vs. Terraform state + +Three things, easy to conflate: + +- **Desired state**: what your `.tf` files declare should exist. +- **Actual infrastructure**: what really exists in Azure right now. +- **Terraform state**: Terraform's own record of what it believes exists, + stored in `terraform.tfstate`. This is neither of the other two -- it's + Terraform's memory, built from the last successful apply (or import). + +`terraform plan` diffs desired state against Terraform's state file, then +confirms that against the real provider API. All three can disagree with +each other; when state and actual infrastructure disagree, that's drift. + +## Provider + +A provider is the plugin Terraform uses to talk to a specific platform -- +`azurerm` for Azure. `terraform init` downloads it. Every resource type's +prefix (`azurerm_resource_group`, `azurerm_container_registry`) names the +provider that owns it. + +## Resource address + +A resource's address (e.g. `azurerm_resource_group.main`) combines its type +and a local name. Terraform uses addresses as keys into the state file and to +build the dependency graph between resources -- e.g. the container registry +in `main.tf` implicitly depends on the resource group because it references +`azurerm_resource_group.main.name`. + +## Dependencies + +Terraform builds a dependency graph from resource references (not file +order) and applies/destroys in graph order: dependencies before dependents on +create, dependents before dependencies on destroy. + +## Drift + +Drift is when the real infrastructure no longer matches what Terraform's +state file says it should be -- usually because someone changed a resource +outside Terraform (Azure Portal, `az` CLI, another tool). `terraform plan` +detects drift by refreshing state against the real provider before diffing, +so a plan can show changes even though nothing in your `.tf` files changed. + +## Local vs. remote state, and locking + +This milestone only uses **local state**: `terraform.tfstate` sits in the +working directory. That's fine solo, but two people (or two CI runs) applying +concurrently against local state can corrupt it. **Remote state** (e.g. an +Azure Storage blob) centralizes the file and typically adds **locking**, so +only one `apply` can run against a given state file at a time. Remote state +and locking are out of scope for this milestone; see `docs/roadmap.md`. + +## State sensitivity + +State can contain resource attribute values, including ones marked sensitive +by a provider (connection strings, generated passwords, keys). This is why +`terraform.tfstate*` is gitignored (see `.gitignore`) -- committing it would +commit those values in plaintext into version control history. + +## Why deleting state does not delete infrastructure + +`terraform.tfstate` is Terraform's memory, not a control plane. Deleting it +does not touch Azure at all -- it only makes Terraform forget every resource +it was tracking. Run `terraform plan` afterward and every real resource would +show up as something Terraform wants to *create*, because as far as +Terraform now knows, nothing exists yet. The real resources are still there, +now unmanaged and orphaned from Terraform's perspective. + +## Why hand-editing state is dangerous + +State's internal schema is version-specific and includes fields (resource +dependency ordering, provider-specific metadata) that aren't meant to be +edited by hand. A manual edit that doesn't match reality can cause Terraform +to plan a destructive "fix" -- e.g. destroying and recreating a resource +Terraform now believes doesn't match its record, even though the real +resource is fine. Use `terraform state` subcommands (`mv`, `rm`, `import`) +instead of editing the file directly; even those require care and are +DESTRUCTIVE-adjacent operations in this platform's risk model (see +`docs/safety.md`). + +## Real vs. simulated in this platform + +`devops-learn terraform` runs real `fmt`/`init`/`validate`/`plan` against +`projects/api_platform/infra/terraform/` and never applies or destroys. +`apply_approved_plan`/`destroy_approved_environment` -- and therefore any +real state file this platform would ever write -- stay simulated until +Milestone 3. See `docs/safety.md` and `docs/roadmap.md`. diff --git a/projects/api_platform/infra/terraform/.terraform.lock.hcl b/projects/api_platform/infra/terraform/.terraform.lock.hcl new file mode 100644 index 0000000..eb0286e --- /dev/null +++ b/projects/api_platform/infra/terraform/.terraform.lock.hcl @@ -0,0 +1,22 @@ +# This file is maintained automatically by "terraform init". +# Manual edits may be lost in future updates. + +provider "registry.terraform.io/hashicorp/azurerm" { + version = "3.117.1" + constraints = "~> 3.0" + hashes = [ + "h1:3c9iOEtBMnHrpJLlhbQ0sCZPWhE/2dvEPcL8KkXAh7w=", + "zh:0c513676836e3c50d004ece7d2624a8aff6faac14b833b96feeac2e4bc2c1c12", + "zh:50ea01ada95bae2f187db9e926e463f45d860767a85ebc59160414e00e76c35d", + "zh:52c2a9edacc06b3f72153f5ef6daca0761c6292158815961fe37f60bc576a3d7", + "zh:618eed2a06b19b1a025b45b05891846d570a6a1cca4d23f4942f5a99e1f747ae", + "zh:61cde5d3165d7e5ec311d5d89486819cd605c1b2d54611b5c97bd4e97dba2762", + "zh:6a873358d5031fc222f5e05f029d1237f3dce8345c767665f393283dfa2627f6", + "zh:afdd80064b2a04da311856feb4ed45f77ff4df6c356e8c2b10afb51fe7e61c70", + "zh:b09113df7e0e8c8959539bd22bae6c39faeb269ba3c4cd948e742f5cf58c35fb", + "zh:d340db7973109761cfc27d52aa02560363337c908b2c99b3628adc5a70a99d5b", + "zh:d5a577226ebc8c65e8f19384878a86acc4b51ede4b4a82d37c3b331b0efcd4a7", + "zh:e2962b147f9e71732df8dbc74940c10d20906f3c003cbfaa1eb9fabbf601a9f0", + "zh:f569b65999264a9416862bca5cd2a6177d94ccb0424f3a4ef424428912b9cb3c", + ] +} diff --git a/projects/api_platform/infra/terraform/README.md b/projects/api_platform/infra/terraform/README.md new file mode 100644 index 0000000..d2b808f --- /dev/null +++ b/projects/api_platform/infra/terraform/README.md @@ -0,0 +1,34 @@ +# infra/terraform + +This is a real, valid Terraform configuration for a minimal Azure footprint: +a resource group, a container registry, and a Log Analytics workspace. + +## Files in this directory + +Terraform merges every `*.tf` file in a directory into a single +configuration. The split here (`providers.tf` / `variables.tf` / `main.tf` / +`outputs.tf`) is a naming convention for readability, not a module boundary +-- Terraform does not treat these as separate modules, and there is no +requirement to name files this way. + +## What runs for real + +`devops-learn terraform` runs real `fmt`, `init`, `validate`, and `plan` +against this directory via `RealTerraformTool` +(`src/devops_learn/tools/terraform_tool.py`). `init` downloads the `azurerm` +provider plugin (network access, no Azure credentials needed). `plan` +requires Azure authentication (`az login`, or `ARM_CLIENT_ID` / +`ARM_CLIENT_SECRET` / `ARM_TENANT_ID` / `ARM_SUBSCRIPTION_ID` environment +variables) to build the provider client -- without it, `plan` fails cleanly +and the workflow explains why rather than faking a result. + +`apply`/`destroy` are not implemented against this directory yet; see +`docs/roadmap.md` (Milestone 3) and `docs/terraform-state.md`. + +## Local state + +Running `init`/`plan` here creates `.terraform/` and, if this configuration +is ever applied, `terraform.tfstate*` -- both are gitignored (see +`docs/terraform-state.md` for why state isn't committed). +`.terraform.lock.hcl` is committed on purpose: it pins the exact provider +version/hash so `init` is reproducible on every machine. diff --git a/projects/api_platform/infra/terraform/main.tf b/projects/api_platform/infra/terraform/main.tf new file mode 100644 index 0000000..1ec84b1 --- /dev/null +++ b/projects/api_platform/infra/terraform/main.tf @@ -0,0 +1,42 @@ +# The resources this configuration declares: everything the learner sees +# planned/applied against Azure by `devops-learn terraform`. +# +# Tagged consistently so every resource this project creates is obvious in +# the Azure portal and easy to find and delete. + +locals { + tags = { + project = "devops-learn-by-doing" + environment = "learning" + managed-by = "terraform" + purpose = "devops-learning" + } + + # Azure Container Registry names must be alphanumeric only (no hyphens), + # unlike most other Azure resource names -- strip them here rather than + # relaxing var.project_name itself. + registry_name = replace(var.project_name, "-", "") +} + +resource "azurerm_resource_group" "main" { + name = "${var.project_name}-rg" + location = var.location + tags = local.tags +} + +resource "azurerm_container_registry" "main" { + name = "${local.registry_name}acr" + resource_group_name = azurerm_resource_group.main.name + location = azurerm_resource_group.main.location + sku = "Basic" + tags = local.tags +} + +resource "azurerm_log_analytics_workspace" "main" { + name = "${var.project_name}-logs" + resource_group_name = azurerm_resource_group.main.name + location = azurerm_resource_group.main.location + sku = "PerGB2018" + retention_in_days = 30 + tags = local.tags +} diff --git a/projects/api_platform/infra/terraform/outputs.tf b/projects/api_platform/infra/terraform/outputs.tf new file mode 100644 index 0000000..ea34d4c --- /dev/null +++ b/projects/api_platform/infra/terraform/outputs.tf @@ -0,0 +1,7 @@ +output "resource_group_name" { + value = azurerm_resource_group.main.name +} + +output "container_registry_login_server" { + value = azurerm_container_registry.main.login_server +} diff --git a/projects/api_platform/infra/terraform/providers.tf b/projects/api_platform/infra/terraform/providers.tf new file mode 100644 index 0000000..10afbfc --- /dev/null +++ b/projects/api_platform/infra/terraform/providers.tf @@ -0,0 +1,18 @@ +# Which platform Terraform talks to, and which version of it. +# +# Terraform merges every *.tf file in this directory into a single +# configuration -- providers.tf / variables.tf / main.tf / outputs.tf is a +# naming convention for readability, not a module boundary. See README.md. + +terraform { + required_providers { + azurerm = { + source = "hashicorp/azurerm" + version = "~> 3.0" + } + } +} + +provider "azurerm" { + features {} +} diff --git a/projects/api_platform/infra/terraform/variables.tf b/projects/api_platform/infra/terraform/variables.tf new file mode 100644 index 0000000..93feebe --- /dev/null +++ b/projects/api_platform/infra/terraform/variables.tf @@ -0,0 +1,11 @@ +variable "project_name" { + description = "Short name used to prefix every resource this configuration creates." + type = string + default = "api-platform" +} + +variable "location" { + description = "Azure region to create resources in." + type = string + default = "eastus" +} diff --git a/src/devops_learn/cli/commands/terraform.py b/src/devops_learn/cli/commands/terraform.py new file mode 100644 index 0000000..f48c5e3 --- /dev/null +++ b/src/devops_learn/cli/commands/terraform.py @@ -0,0 +1,44 @@ +"""`devops-learn terraform [path]`: run the real Terraform vertical slice. + +Runs real fmt/init/validate/plan against /infra/terraform via +RealTerraformTool. This milestone ships real execution only against the +bundled projects/api_platform sample; generating configuration for arbitrary +projects is future work (see docs/roadmap.md). +""" + +from __future__ import annotations + +import argparse +from pathlib import Path + +from devops_learn.bootstrap import Platform +from devops_learn.cli.terminal_ui import TerminalUi +from devops_learn.workflows.terraform_flow import TerraformOptions, run_terraform_flow + + +def register(subparsers: "argparse._SubParsersAction[argparse.ArgumentParser]") -> None: + parser = subparsers.add_parser( + "terraform", + help="Run the real Terraform vertical slice: fmt, init, validate, plan, risk analysis", + ) + parser.add_argument( + "path", + nargs="?", + default="projects/api_platform", + help="Project containing infra/terraform/ (defaults to the bundled sample project)", + ) + parser.set_defaults(handler=run) + + +def run(args: argparse.Namespace, platform: Platform) -> None: + infra_root = f"{args.path}/infra/terraform" + if not Path(infra_root).is_dir(): + print(f"No Terraform configuration found at {infra_root}.") + print( + "This milestone ships real Terraform execution only against the bundled " + "projects/api_platform sample; generating configuration for arbitrary projects " + "is future work (see docs/roadmap.md)." + ) + return + options = TerraformOptions(project_root=args.path, infra_root=infra_root) + run_terraform_flow(platform, TerminalUi(), options) diff --git a/src/devops_learn/cli/main.py b/src/devops_learn/cli/main.py index 9c669ee..80b7289 100644 --- a/src/devops_learn/cli/main.py +++ b/src/devops_learn/cli/main.py @@ -7,21 +7,30 @@ from devops_learn.ai.anthropic_provider import AnthropicProvider from devops_learn.bootstrap import build_platform -from devops_learn.cli.commands import analyze, explain, history, init, local, profile, review +from devops_learn.cli.commands import ( + analyze, + explain, + history, + init, + local, + profile, + review, + terraform, +) from devops_learn.config.settings import load_settings from devops_learn.domain.enums import ExecutionMode from devops_learn.learning.persistence.connection import connect from devops_learn.tools.approval import ApprovalGate, CliApprovalGate, ThresholdApprovalGate from devops_learn.tools.base import RiskLevel, Tool -from devops_learn.tools.docker_tool import RealDockerTool +from devops_learn.tools.docker_tool import RealDockerTool, SimulatedDockerTool from devops_learn.tools.git_tool import SimulatedGitTool from devops_learn.tools.kubernetes_tool import SimulatedKubernetesTool -from devops_learn.tools.python_tool import RealPythonTool -from devops_learn.tools.terraform_tool import SimulatedTerraformTool +from devops_learn.tools.python_tool import RealPythonTool, SimulatedPythonTool +from devops_learn.tools.terraform_tool import RealTerraformTool, SimulatedTerraformTool from devops_learn.tools.validation_tool import SimulatedValidationTool from devops_learn.tools.cloud_tool import SimulatedCloudTool -_COMMAND_MODULES = (analyze, review, history, explain, profile, init, local) +_COMMAND_MODULES = (analyze, review, history, explain, profile, init, local, terraform) def build_parser() -> argparse.ArgumentParser: @@ -69,6 +78,18 @@ def _approval_gate_for_args(args: argparse.Namespace) -> ApprovalGate: def _tools_for_args(args: argparse.Namespace) -> dict[str, Tool] | None: + if args.command == "terraform": + # This command only ever invokes the "terraform" tool -- keep the rest + # simulated rather than provisioning real python/docker tools it never uses. + return { + "python": SimulatedPythonTool(), + "git": SimulatedGitTool(), + "docker": SimulatedDockerTool(), + "terraform": RealTerraformTool(), + "kubernetes": SimulatedKubernetesTool(), + "cloud": SimulatedCloudTool(), + "validation": SimulatedValidationTool(), + } if getattr(args, "real_tools", False) or args.command == "local": return { "python": RealPythonTool(), diff --git a/src/devops_learn/explanations/service.py b/src/devops_learn/explanations/service.py index fb921e4..6654109 100644 --- a/src/devops_learn/explanations/service.py +++ b/src/devops_learn/explanations/service.py @@ -48,4 +48,6 @@ def render_learning_moment( lines.append(moment.why_it_matters) if depth >= ExplanationDepth.DEEP: lines.append(moment.deep_explanation) + if moment.related_artifact and depth >= ExplanationDepth.LEARNING: + lines.append(f"See: {moment.related_artifact}") return "\n".join(lines) diff --git a/src/devops_learn/tools/_subprocess_safety.py b/src/devops_learn/tools/_subprocess_safety.py new file mode 100644 index 0000000..f5d8e97 --- /dev/null +++ b/src/devops_learn/tools/_subprocess_safety.py @@ -0,0 +1,86 @@ +"""Shared subprocess-safety helpers for Real*Tool implementations. + +Extracted because RealTerraformTool needs both an explicit timeout and output +redaction, neither of which RealDockerTool/RealPythonTool have today: those +two tools never set a subprocess timeout and put stdout/stderr into +ToolResult.details verbatim. That gap matters more for Terraform, since +provider auth errors and init/plan output can contain secret-shaped values. +Deliberately generic (no Terraform-specific knowledge) so those two tools +could adopt it later; retrofitting them is out of scope for this milestone. +""" + +from __future__ import annotations + +import re +import subprocess +from dataclasses import dataclass + +DEFAULT_TIMEOUT_SECONDS = 120 +MAX_OUTPUT_CHARS = 20_000 + +_SECRET_PATTERN = re.compile( + r"(?i)\b([A-Z0-9_]*(?:SECRET|TOKEN|PASSWORD|API_KEY)[A-Z0-9_]*)\s*[=:]\s*(\S+)" +) + + +@dataclass(frozen=True) +class SafeRunResult: + returncode: int + stdout: str + stderr: str + timed_out: bool + + +def redact(text: str) -> str: + """Masks KEY=value-shaped secrets in free text. + + Defense in depth only. The primary defense against leaking sensitive + Terraform data is never extracting attribute values from structured + output in the first place -- see the module docstring in + tools/terraform_tool.py for why plan parsing never touches + resource_changes[].change.before/after. + """ + return _SECRET_PATTERN.sub(lambda m: f"{m.group(1)}=", text) + + +def _truncate(text: str) -> str: + if len(text) <= MAX_OUTPUT_CHARS: + return text + hidden = len(text) - MAX_OUTPUT_CHARS + return text[:MAX_OUTPUT_CHARS] + f"\n... [truncated, {hidden} more characters] ...\n" + + +def _sanitize(text: str) -> str: + return _truncate(redact(text)) + + +def run_safely( + command: list[str], *, cwd: str | None, timeout: int = DEFAULT_TIMEOUT_SECONDS +) -> SafeRunResult: + """Runs `command` as an argument array (never shell=True) with a timeout, + and always redacts + truncates stdout/stderr before returning, so a call + site can never forget to sanitize output before it reaches a ToolResult. + """ + try: + completed = subprocess.run( + command, + cwd=cwd, + capture_output=True, + text=True, + check=False, + timeout=timeout, + ) + except subprocess.TimeoutExpired as exc: + partial_stdout = exc.stdout if isinstance(exc.stdout, str) else "" + return SafeRunResult( + returncode=-1, + stdout=_sanitize(partial_stdout), + stderr=_sanitize(f"Command timed out after {timeout}s: {' '.join(command)}"), + timed_out=True, + ) + return SafeRunResult( + returncode=completed.returncode, + stdout=_sanitize(completed.stdout), + stderr=_sanitize(completed.stderr), + timed_out=False, + ) diff --git a/src/devops_learn/tools/terraform_tool.py b/src/devops_learn/tools/terraform_tool.py index 330b3dd..4d65bea 100644 --- a/src/devops_learn/tools/terraform_tool.py +++ b/src/devops_learn/tools/terraform_tool.py @@ -1,16 +1,32 @@ -"""Simulated TerraformTool. No real terraform binary invoked in V1; see docs/safety.md. +"""TerraformTool implementations: simulated (no binary invoked, used by +`analyze`/`review`) and real (used by `devops-learn terraform`). -plan() derives its resource count from templates/terraform/main.tf.reference -instead of a hardcoded string, so the learner's interpretation question stays -meaningful and the reference config can change without a second edit site. +SimulatedTerraformTool.plan() derives its resource count from +templates/terraform/main.tf.reference instead of a hardcoded string, so the +learner's interpretation question stays meaningful and the reference config +can change without a second edit site. + +RealTerraformTool shells out to a real `terraform` binary for fmt/init/ +validate/plan only -- see its class docstring for why apply/destroy are +deliberately not declared here. Its plan parsing never extracts raw resource +attribute values (before/after) from `terraform show -json`, only +{address, action} pairs: this is the primary defense against leaking +sensitive plan data, since Terraform's own "sensitive" markers can vary by +provider/version and are not a defense to rely on alone. Free-text +stdout/stderr from every real subprocess call is still redacted and +truncated via tools/_subprocess_safety.py as defense in depth. """ from __future__ import annotations +import json import re +import shutil +import tempfile from pathlib import Path from typing import Any, Mapping +from devops_learn.tools import _subprocess_safety from devops_learn.tools.approval import ApprovalRecord from devops_learn.tools.base import RiskLevel, Tool, ToolOperationSpec, ToolResult @@ -126,3 +142,260 @@ def execute( was_dry_run=dry_run, approval=approval, ) + + +_REAL_OPERATIONS = ( + ToolOperationSpec( + name="fmt", + risk_level=RiskLevel.SAFE, + supports_dry_run=False, + requires_approval=False, + is_destructive=False, + ), + ToolOperationSpec( + name="init", + risk_level=RiskLevel.SAFE, + supports_dry_run=False, + requires_approval=False, + is_destructive=False, + ), + ToolOperationSpec( + name="validate", + risk_level=RiskLevel.SAFE, + supports_dry_run=False, + requires_approval=False, + is_destructive=False, + ), + ToolOperationSpec( + name="plan", + risk_level=RiskLevel.SAFE, + supports_dry_run=False, + requires_approval=False, + is_destructive=False, + ), +) + + +def _terraform_command() -> str: + command = shutil.which("terraform") + if command is None: + raise FileNotFoundError( + "Terraform CLI not found. Install Terraform or use simulation mode." + ) + return command + + +def _details_from_plan_json(plan_json: Mapping[str, Any]) -> dict[str, Any]: + """Extracts only {address, action} pairs from a real `terraform show -json` + plan -- never the resource_changes[].change.before/after attribute maps, + which can contain sensitive values. See the module docstring.""" + create = change = replace = destroy = 0 + resources: list[dict[str, str]] = [] + for resource_change in plan_json.get("resource_changes", []): + actions = tuple(resource_change.get("change", {}).get("actions", ())) + if actions == ("create",): + create += 1 + action = "create" + elif actions == ("update",): + change += 1 + action = "update" + elif actions in (("delete", "create"), ("create", "delete")): + replace += 1 + action = "replace" + elif actions == ("delete",): + destroy += 1 + action = "destroy" + else: + continue # ("no-op",) / ("read",): not a change + address = str(resource_change.get("address", "unknown")) + resources.append({"address": address, "action": action}) + return { + "create": create, + "change": change, + "replace": replace, + "destroy": destroy, + "resources": resources, + } + + +class RealTerraformTool(Tool): + """Runs real terraform fmt/init/validate/plan via subprocess. + + Deliberately does not declare apply_approved_plan or + destroy_approved_environment: those stay simulated until Milestone 3, per + docs/roadmap.md. A caller invoking either operation on this tool gets a + KeyError from Tool.spec_for rather than a result that looks real but + silently isn't -- apply/destroy remain real capabilities of the + "terraform" tool name only via SimulatedTerraformTool, exactly how + cli/main.py already selects real-vs-simulated per command today. See + docs/adr/0004-controlled-tool-execution.md. + """ + + @property + def name(self) -> str: + return "terraform" + + @property + def operations(self) -> tuple[ToolOperationSpec, ...]: + return _REAL_OPERATIONS + + def execute( + self, + operation: str, + params: Mapping[str, Any], + *, + dry_run: bool, + approval: ApprovalRecord | None, + ) -> ToolResult: + spec = self.spec_for(operation) + assert not spec.requires_approval or dry_run or (approval is not None and approval.granted) + + if dry_run: + return ToolResult( + success=True, + summary=f"Would run terraform {operation} (dry run)", + details={"dry_run": True}, + risk_level=spec.risk_level, + was_dry_run=dry_run, + approval=approval, + ) + + working_dir = params.get("path") + timeout_override = params.get("timeout_seconds") + + def _timeout(default: int) -> int: + return int(timeout_override) if timeout_override is not None else default + + try: + terraform = _terraform_command() + if operation == "fmt": + result = _subprocess_safety.run_safely( + [terraform, "fmt", "-check", "-diff", "-no-color"], + cwd=working_dir, + timeout=_timeout(30), + ) + success = result.returncode == 0 + summary = "Configuration already formatted" if success else ( + "Configuration needs formatting" + ) + details: dict[str, Any] = { + "returncode": result.returncode, + "stdout": result.stdout, + "stderr": result.stderr, + } + elif operation == "init": + result = _subprocess_safety.run_safely( + [terraform, "init", "-input=false", "-no-color"], + cwd=working_dir, + timeout=_timeout(120), + ) + success = result.returncode == 0 + success_lines = [ + line + for line in result.stdout.strip().splitlines() + if "successfully initialized" in line.lower() + ] + if success_lines: + summary = success_lines[0].strip() + elif success: + summary = "Terraform has been initialized" + else: + summary = "terraform init failed" + details = { + "returncode": result.returncode, + "stdout": result.stdout, + "stderr": result.stderr, + } + elif operation == "validate": + result = _subprocess_safety.run_safely( + [terraform, "validate", "-json"], cwd=working_dir, timeout=_timeout(30) + ) + try: + parsed = json.loads(result.stdout) if result.stdout else {} + except json.JSONDecodeError: + parsed = {} + diagnostics = parsed.get("diagnostics", []) + success = result.returncode == 0 and bool(parsed.get("valid", False)) + summary = ( + "Success! The configuration is valid." + if success + else f"Validation failed: {len(diagnostics)} diagnostic(s)" + ) + details = { + "returncode": result.returncode, + "valid": parsed.get("valid"), + "diagnostic_count": len(diagnostics), + "stderr": result.stderr, + } + else: # plan + with tempfile.TemporaryDirectory() as tmp_dir: + plan_path = str(Path(tmp_dir) / "tfplan.out") + plan_result = _subprocess_safety.run_safely( + [terraform, "plan", "-input=false", "-no-color", f"-out={plan_path}"], + cwd=working_dir, + timeout=_timeout(180), + ) + if plan_result.returncode != 0: + return ToolResult( + success=False, + summary="(real, failed) terraform plan failed", + details={ + "returncode": plan_result.returncode, + "stdout": plan_result.stdout, + "stderr": plan_result.stderr, + }, + risk_level=spec.risk_level, + was_dry_run=dry_run, + approval=approval, + ) + show_result = _subprocess_safety.run_safely( + [terraform, "show", "-json", plan_path], cwd=working_dir, timeout=30 + ) + if show_result.returncode != 0: + return ToolResult( + success=False, + summary="(real, failed) terraform show -json failed", + details={ + "returncode": show_result.returncode, + "stderr": show_result.stderr, + }, + risk_level=spec.risk_level, + was_dry_run=dry_run, + approval=approval, + ) + try: + plan_json = json.loads(show_result.stdout) + except json.JSONDecodeError: + return ToolResult( + success=False, + summary="(real, failed) Could not parse terraform show -json output", + details={"error": "malformed JSON from terraform show -json"}, + risk_level=spec.risk_level, + was_dry_run=dry_run, + approval=approval, + ) + details = _details_from_plan_json(plan_json) + success = True + summary = ( + f"Plan: {details['create']} to add, {details['change']} to change, " + f"{details['replace']} to replace, {details['destroy']} to destroy." + ) + except (OSError, FileNotFoundError) as exc: + return ToolResult( + success=False, + summary=f"(real, failed) {exc}", + details={"error": str(exc)}, + risk_level=spec.risk_level, + was_dry_run=dry_run, + approval=approval, + ) + + prefix = "(real) " if success else "(real, failed) " + return ToolResult( + success=success, + summary=prefix + summary, + details=details, + risk_level=spec.risk_level, + was_dry_run=dry_run, + approval=approval, + ) diff --git a/src/devops_learn/workflows/terraform_flow.py b/src/devops_learn/workflows/terraform_flow.py new file mode 100644 index 0000000..b6330f1 --- /dev/null +++ b/src/devops_learn/workflows/terraform_flow.py @@ -0,0 +1,360 @@ +"""Real Terraform vertical slice: fmt -> init -> validate -> plan -> risk analysis. + +Runs RealTerraformTool against a real, committed Terraform working directory +(projects/api_platform/infra/terraform/ by default). Never applies or +destroys -- see docs/roadmap.md (Milestone 3) and docs/safety.md. A plan +failure due to missing Azure credentials is treated as an expected, honest +outcome to teach from, not something to hide. +""" + +from __future__ import annotations + +from dataclasses import dataclass +from datetime import datetime, timezone +from typing import Any + +from devops_learn.bootstrap import Platform +from devops_learn.domain.enums import ( + AuditEventType, + CloudProviderKind, + CostPriority, + EnvironmentKind, + ExecutionMode, + ExperienceState, + ExplanationDepth, +) +from devops_learn.domain.explanation_models import Explanation, LearningMoment +from devops_learn.domain.learner_profile_models import CompetencyArea +from devops_learn.domain.plan_models import TerraformPlanSummary +from devops_learn.domain.session_models import EngagementSession +from devops_learn.tools.approval import RiskLevel +from devops_learn.tools.base import ToolResult +from devops_learn.validation import terraform_plan_analysis +from devops_learn.workflows import presenters +from devops_learn.workflows.ui import Ui + + +@dataclass(frozen=True) +class TerraformOptions: + project_root: str + infra_root: str + + +def run_terraform_flow( + platform: Platform, ui: Ui, options: TerraformOptions +) -> EngagementSession: + session = platform.session_service.start( + project_root=options.project_root, + mode=ExecutionMode.COLLABORATIVE, + explanation_depth=ExplanationDepth.LEARNING, + cloud=CloudProviderKind.AZURE, + environment=EnvironmentKind.DEV, + cost_priority=CostPriority.BALANCED, + simulation_mode=False, + ) + assert session.id is not None + session_id = session.id + + _audit(platform, session_id, AuditEventType.SESSION_STARTED, "Terraform workflow started") + + fmt_result = _run_terraform_step( + platform, ui, session_id, "fmt", options.infra_root, "Terraform fmt" + ) + if not fmt_result.success: + ui.present( + f"Configuration is not formatted. Run `terraform fmt` in {options.infra_root} " + "to fix this before continuing." + ) + + _teach_provider_concept(platform, ui, session_id) + init_result = _run_terraform_step( + platform, ui, session_id, "init", options.infra_root, "Terraform init" + ) + if not init_result.success: + _diagnose_and_stop(platform, ui, session_id, "terraform init failed") + platform.session_service.complete(session) + return session + + validate_result = _run_terraform_step( + platform, ui, session_id, "validate", options.infra_root, "Terraform validate" + ) + if not validate_result.success: + _diagnose_and_stop(platform, ui, session_id, "terraform validate failed") + platform.session_service.complete(session) + return session + + _teach_state_concepts(platform, ui, session_id) + _audit(platform, session_id, AuditEventType.TERRAFORM_PLAN_STARTED, "Terraform plan started") + plan_result = _run_terraform_step( + platform, ui, session_id, "plan", options.infra_root, "Terraform plan" + ) + if not plan_result.success: + _diagnose_plan_failure(platform, ui, session_id, plan_result) + platform.session_service.complete(session) + return session + + summary = terraform_plan_analysis.analyze(plan_result.details) + ui.present(presenters.render_terraform_plan(summary)) + _audit(platform, session_id, AuditEventType.TERRAFORM_PLAN_COMPLETED, summary.reason) + _explain(platform, ui, "What to check", _what_to_check_explanation(summary)) + + platform.experience_tracker.record( + session_id, "Terraform", "Ran real fmt/init/validate/plan", ExperienceState.PRACTICED + ) + platform.experience_tracker.record( + session_id, "Azure", "Reviewed a real Azure resource plan", ExperienceState.INTRODUCED + ) + + _print_history_and_experience(platform, ui, session_id) + platform.session_service.complete(session) + return session + + +def _audit( + platform: Platform, + session_id: int, + event_type: AuditEventType, + summary: str, + payload: dict[str, Any] | None = None, +) -> None: + platform.audit_service.record( + session_id=session_id, + event_type=event_type, + occurred_at=_now(), + summary=summary, + payload=payload or {}, + ) + + +def _run_terraform_step( + platform: Platform, + ui: Ui, + session_id: int, + operation: str, + infra_root: str, + label: str, +) -> ToolResult: + _explain(platform, ui, label, _terraform_explanation(operation)) + result = platform.tool_service.invoke("terraform", operation, {"path": infra_root}) + ui.present(result.summary) + _audit( + platform, + session_id, + AuditEventType.TOOL_INVOKED, + f"terraform.{operation}", + {"success": result.success}, + ) + return result + + +def _diagnose_and_stop( + platform: Platform, ui: Ui, session_id: int, context: str +) -> None: + ui.present(f"{context}. Diagnosis starts from the output above.") + _audit(platform, session_id, AuditEventType.DEPLOYMENT_FAILED, context) + platform.experience_tracker.record( + session_id, "Troubleshooting", f"Investigated {context}", ExperienceState.INTRODUCED + ) + + +def _diagnose_plan_failure( + platform: Platform, ui: Ui, session_id: int, result: ToolResult +) -> None: + stderr = str(result.details.get("stderr", "")).lower() + is_auth_failure = "az login" in stderr or "authoriz" in stderr or "authent" in stderr + if is_auth_failure: + ui.present( + "DIAGNOSIS: terraform plan failed because Terraform could not authenticate to " + "Azure. The azurerm provider needs credentials to build API requests, even for a " + "read-only plan.\n\n" + "Fix by doing ONE of:\n" + " - `az login` (uses your interactive Azure CLI session), or\n" + " - setting ARM_CLIENT_ID, ARM_CLIENT_SECRET, ARM_TENANT_ID, and " + "ARM_SUBSCRIPTION_ID for a service principal.\n\n" + "This is an expected, honest failure on a machine with no Azure credentials -- " + "not a bug. See docs/safety.md." + ) + else: + ui.present("terraform plan failed. Diagnosis starts from the output above.") + _audit(platform, session_id, AuditEventType.DEPLOYMENT_FAILED, "terraform plan failed") + platform.experience_tracker.record( + session_id, + "Troubleshooting", + "Diagnosed a terraform plan authentication failure" + if is_auth_failure + else "Diagnosed a terraform plan failure", + ExperienceState.PRACTICED, + ) + + +def _should_teach(platform: Platform) -> bool: + profile = platform.learner_profile_service.load() + return not profile.is_strong(CompetencyArea.TERRAFORM) + + +def _teach_state_concepts(platform: Platform, ui: Ui, session_id: int) -> None: + if not _should_teach(platform): + return + moment = LearningMoment( + concept="Terraform state", + trigger="before running terraform plan", + summary=( + "Terraform tracks the infrastructure it manages in a state file. State is " + "Terraform's record of what it believes exists; the real Azure resources are the " + "ground truth, and the two can drift apart." + ), + deep_explanation=( + "Every managed resource gets an address (e.g. azurerm_resource_group.main) " + "recorded in state alongside the real Azure resource ID it maps to. Terraform " + "diffs your configuration against state, then confirms that against Azure during " + "plan/apply. State can contain resource attribute values, including sensitive " + "ones, which is why it is never committed to git (see .gitignore) -- and why " + "deleting a state file does NOT delete the real Azure resources; it only makes " + "Terraform forget about them." + ), + why_it_matters=( + "Understanding state is the difference between reading a `terraform plan` and " + "actually knowing what it will do." + ), + related_artifact="docs/terraform-state.md", + ) + _render_learning_moment(platform, ui, moment) + platform.experience_tracker.record( + session_id, + "Terraform state", + "Explained desired vs. actual state", + ExperienceState.INTRODUCED, + ) + + +def _teach_provider_concept(platform: Platform, ui: Ui, session_id: int) -> None: + if not _should_teach(platform): + return + moment = LearningMoment( + concept="Terraform provider", + trigger="before running terraform init", + summary=( + "A provider is the plugin Terraform uses to talk to a specific platform -- here, " + "the azurerm provider talks to Azure's Resource Manager API. `terraform init` " + "downloads it." + ), + deep_explanation=( + "A resource block's type prefix (azurerm_) tells Terraform which provider owns it " + "and which real API calls to make. A resource address like " + "azurerm_resource_group.main combines the resource type and a local name; " + "Terraform uses addresses to track dependencies between resources, such as the " + "container registry needing the resource group to exist first." + ), + why_it_matters=( + "init failing almost always means a provider or network problem, not a mistake in " + "your .tf files -- a different failure mode than validate or plan." + ), + related_artifact="docs/terraform-state.md", + ) + _render_learning_moment(platform, ui, moment) + platform.experience_tracker.record( + session_id, + "Terraform", + "Explained provider and resource address", + ExperienceState.INTRODUCED, + ) + + +def _render_learning_moment(platform: Platform, ui: Ui, moment: LearningMoment) -> None: + rendered = platform.explanation_service.render_learning_moment( + moment, mode=ExecutionMode.COLLABORATIVE, depth=ExplanationDepth.LEARNING + ) + if rendered: + ui.present(rendered) + + +def _explain(platform: Platform, ui: Ui, label: str, explanation: Explanation) -> None: + rendered = platform.explanation_service.render( + explanation, mode=ExecutionMode.COLLABORATIVE, depth=ExplanationDepth.LEARNING + ) + ui.present(rendered) + + +def _terraform_explanation(operation: str) -> Explanation: + if operation == "fmt": + return Explanation( + action="Checking Terraform formatting.", + why="Consistent formatting makes configuration easy to review and diff.", + ) + if operation == "init": + return Explanation( + action="Initializing the Terraform working directory.", + why=( + "Terraform must download the azurerm provider before it can validate, plan, " + "or apply anything." + ), + what_to_understand=( + "init also creates a local .terraform/ plugin cache, which is never committed " + "to git -- it is regenerated by running init again." + ), + ) + if operation == "validate": + return Explanation( + action="Validating the configuration.", + why="Validate catches syntax and type errors locally, before Terraform talks to Azure.", + ) + return Explanation( + action="Running terraform plan.", + why="Plan shows exactly what Terraform would change, without changing anything yet.", + what_to_understand=( + "Plan compares your configuration against Terraform's state file, then confirms " + "that against Azure. It requires Azure authentication even though it makes no " + "changes." + ), + ) + + +def _what_to_check_explanation(summary: TerraformPlanSummary) -> Explanation: + if summary.risk_level is RiskLevel.DESTRUCTIVE: + result = ( + "This plan would destroy real resources. Never approve an apply from a plan like " + "this without independently confirming that destruction is intended." + ) + elif summary.risk_level is RiskLevel.HIGH: + result = ( + "Terraform must destroy and recreate at least one resource. Confirm you " + "understand *why* before ever approving an apply: is the resource stateful, " + "would its address or endpoint change, and would that cause downtime?" + ) + elif summary.risk_level is RiskLevel.LOW: + result = "Confirm the in-place changes match what you intended to modify." + else: + result = ( + "Confirm the resource names in this plan won't collide with anything already in " + "the target subscription." + ) + return Explanation( + action="Reviewing what this plan means before any apply.", + what_to_understand=( + "Terraform state can drift from real infrastructure if something changes " + "resources outside Terraform. A plan is only trustworthy if state and Azure " + "haven't drifted apart." + ), + result=result, + ) + + +def _print_history_and_experience(platform: Platform, ui: Ui, session_id: int) -> None: + events = platform.audit_service.history(session_id) + lines = ["AUDIT HISTORY", ""] + for event in events: + lines.append(f"{event.sequence_no:>3}. {event.event_type.value}: {event.summary}") + ui.present("\n".join(lines)) + + grouped = platform.experience_tracker.summary(session_id) + lines = ["EXPERIENCE SUMMARY", ""] + for concept, entries in grouped.items(): + lines.append(concept) + for entry in entries: + lines.append(f" ✓ {entry.item}") + lines.append("") + ui.present("\n".join(lines).rstrip()) + + +def _now() -> datetime: + return datetime.now(timezone.utc) diff --git a/tests/analysis/test_project_analyzer.py b/tests/analysis/test_project_analyzer.py index a8f95cc..059298f 100644 --- a/tests/analysis/test_project_analyzer.py +++ b/tests/analysis/test_project_analyzer.py @@ -14,7 +14,9 @@ def test_analyzes_the_bundled_example_project() -> None: assert assessment.containerization_status is MaturityStatus.GOOD assert assessment.healthcheck_status is MaturityStatus.GOOD assert assessment.ci_cd_status is MaturityStatus.MISSING - assert assessment.iac_status is MaturityStatus.MISSING + # projects/api_platform/infra/terraform/ is real Terraform config as of Milestone 2 + # (docs/roadmap.md) -- ProjectAnalyzer correctly detects it. + assert assessment.iac_status is MaturityStatus.GOOD def test_detects_missing_everything_for_an_empty_project(tmp_path: Path) -> None: diff --git a/tests/explanations/test_service.py b/tests/explanations/test_service.py new file mode 100644 index 0000000..e69d3ef --- /dev/null +++ b/tests/explanations/test_service.py @@ -0,0 +1,49 @@ +from devops_learn.domain.enums import ExecutionMode, ExplanationDepth +from devops_learn.domain.explanation_models import LearningMoment +from devops_learn.explanations.service import ExplanationService + + +def _moment(**overrides: object) -> LearningMoment: + defaults: dict[str, object] = dict( + concept="Terraform state", + trigger="before init", + summary="State tracks what Terraform believes exists.", + deep_explanation="Full detail about state internals.", + why_it_matters="Understanding state matters for trustworthy plans.", + related_artifact="docs/terraform-state.md", + ) + defaults.update(overrides) + return LearningMoment(**defaults) # type: ignore[arg-type] + + +def test_related_artifact_is_rendered_at_learning_depth() -> None: + rendered = ExplanationService().render_learning_moment( + _moment(), mode=ExecutionMode.COLLABORATIVE, depth=ExplanationDepth.LEARNING + ) + assert rendered is not None + assert "docs/terraform-state.md" in rendered + + +def test_related_artifact_is_omitted_below_learning_depth() -> None: + rendered = ExplanationService().render_learning_moment( + _moment(), mode=ExecutionMode.COLLABORATIVE, depth=ExplanationDepth.NORMAL + ) + assert rendered is not None + assert "docs/terraform-state.md" not in rendered + + +def test_related_artifact_absent_when_not_set() -> None: + rendered = ExplanationService().render_learning_moment( + _moment(related_artifact=None), + mode=ExecutionMode.COLLABORATIVE, + depth=ExplanationDepth.DEEP, + ) + assert rendered is not None + assert "See:" not in rendered + + +def test_learning_moment_suppressed_in_autonomous_mode_below_deep_depth() -> None: + rendered = ExplanationService().render_learning_moment( + _moment(), mode=ExecutionMode.AUTONOMOUS, depth=ExplanationDepth.LEARNING + ) + assert rendered is None diff --git a/tests/fixtures/fake_terraform/terraform b/tests/fixtures/fake_terraform/terraform new file mode 100755 index 0000000..329d163 --- /dev/null +++ b/tests/fixtures/fake_terraform/terraform @@ -0,0 +1,130 @@ +#!/usr/bin/env python3 +"""A fake `terraform` binary for tests/tools/test_real_terraform_tool.py. + +Behavior is selected by the FAKE_TERRAFORM_SCENARIO env var so a single +script covers every scenario instead of one file per case. Never touches +real infrastructure; only prints canned output and writes a placeholder +plan file when asked to `-out=`. +""" +from __future__ import annotations + +import json +import os +import sys +import time + +SCENARIO = os.environ.get("FAKE_TERRAFORM_SCENARIO", "success") + + +def _plan_out_path(args: list[str]) -> str | None: + for arg in args: + if arg.startswith("-out="): + return arg[len("-out="):] + return None + + +def main() -> int: + args = sys.argv[1:] + if not args: + return 1 + subcommand = args[0] + + if SCENARIO == "timeout": + time.sleep(5) + return 0 + + if subcommand == "fmt": + if SCENARIO == "fmt_needs_formatting": + print("main.tf\n+ location = \"eastus\"", file=sys.stdout) + return 3 + return 0 + + if subcommand == "init": + if SCENARIO == "init_failure": + print("Error: Failed to install provider", file=sys.stderr) + return 1 + print("Terraform has been successfully initialized!") + return 0 + + if subcommand == "validate": + if SCENARIO == "validate_failure": + print(json.dumps({ + "valid": False, + "diagnostics": [{"severity": "error", "summary": "Missing required argument"}], + })) + return 1 + print(json.dumps({"valid": True, "diagnostics": []})) + return 0 + + if subcommand == "plan": + out_path = _plan_out_path(args) + if SCENARIO == "plan_failure_with_secret": + # Mirrors real `terraform plan` output observed with no Azure credentials: + # a genuine "az login"/"Authorizer" auth failure, with a secret-shaped + # value mixed in to also exercise redaction. + print( + "Error: building AzureRM Client: obtain a token using " + "ARM_CLIENT_SECRET=super-secret-value-123 failed: could not configure " + "AzureCli Authorizer: obtaining tenant ID: running Azure CLI: exit status 1: " + "ERROR: Please run 'az login' to setup account.", + file=sys.stderr, + ) + return 1 + if out_path: + with open(out_path, "wb") as handle: + handle.write(b"fake-plan-binary-content") + return 0 + + if subcommand == "show": + if SCENARIO == "show_json_malformed": + print("{not valid json") + return 0 + if SCENARIO == "plan_with_sensitive_attribute": + print(json.dumps({ + "resource_changes": [ + { + "address": "azurerm_resource_group.main", + "change": {"actions": ["create"]}, + }, + { + "address": "azurerm_key_vault_secret.admin", + "change": { + "actions": ["create"], + "after": {"admin_password": "hunter2"}, + }, + }, + ] + })) + return 0 + if SCENARIO == "plan_mixed_actions": + print(json.dumps({ + "resource_changes": [ + {"address": "azurerm_resource_group.main", + "change": {"actions": ["create"]}}, + {"address": "azurerm_container_registry.main", + "change": {"actions": ["update"]}}, + {"address": "azurerm_log_analytics_workspace.main", + "change": {"actions": ["delete", "create"]}}, + {"address": "azurerm_storage_account.old", + "change": {"actions": ["delete"]}}, + {"address": "azurerm_resource_group.untouched", + "change": {"actions": ["no-op"]}}, + ] + })) + return 0 + # default "success" scenario: two creates + print(json.dumps({ + "resource_changes": [ + {"address": "azurerm_resource_group.main", + "change": {"actions": ["create"]}}, + {"address": "azurerm_container_registry.main", + "change": {"actions": ["create"]}}, + ] + })) + return 0 + + return 1 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tests/tools/test_real_terraform_tool.py b/tests/tools/test_real_terraform_tool.py new file mode 100644 index 0000000..24ec66a --- /dev/null +++ b/tests/tools/test_real_terraform_tool.py @@ -0,0 +1,189 @@ +import json +import os +from pathlib import Path + +import pytest + +from devops_learn.tools.base import RiskLevel +from devops_learn.tools.terraform_tool import RealTerraformTool +from devops_learn.validation import terraform_plan_analysis + +FIXTURES_DIR = Path(__file__).resolve().parents[1] / "fixtures" / "fake_terraform" + + +def _use_fake_terraform(monkeypatch, scenario: str = "success") -> None: + monkeypatch.setenv("PATH", f"{FIXTURES_DIR}{os.pathsep}{os.environ['PATH']}") + monkeypatch.setenv("FAKE_TERRAFORM_SCENARIO", scenario) + + +def test_real_terraform_tool_has_safe_operations() -> None: + tool = RealTerraformTool() + assert tool.name == "terraform" + names = {spec.name for spec in tool.operations} + assert names == {"fmt", "init", "validate", "plan"} + for spec in tool.operations: + assert spec.risk_level is RiskLevel.SAFE + assert not spec.requires_approval + + +def test_real_terraform_tool_does_not_expose_apply_or_destroy() -> None: + tool = RealTerraformTool() + with pytest.raises(KeyError): + tool.spec_for("apply_approved_plan") + with pytest.raises(KeyError): + tool.spec_for("destroy_approved_environment") + + +def test_missing_binary_reports_failure(monkeypatch, tmp_path) -> None: + monkeypatch.setenv("PATH", str(tmp_path / "empty-path")) + (tmp_path / "empty-path").mkdir() + result = RealTerraformTool().execute( + "fmt", {"path": str(tmp_path)}, dry_run=False, approval=None + ) + assert not result.success + assert "Terraform CLI not found" in result.summary + + +def test_fmt_success(monkeypatch, tmp_path) -> None: + _use_fake_terraform(monkeypatch) + result = RealTerraformTool().execute( + "fmt", {"path": str(tmp_path)}, dry_run=False, approval=None + ) + assert result.success + assert "(real)" in result.summary + + +def test_fmt_needs_formatting(monkeypatch, tmp_path) -> None: + _use_fake_terraform(monkeypatch, "fmt_needs_formatting") + result = RealTerraformTool().execute( + "fmt", {"path": str(tmp_path)}, dry_run=False, approval=None + ) + assert not result.success + assert "needs formatting" in result.summary.lower() + + +def test_init_success(monkeypatch, tmp_path) -> None: + _use_fake_terraform(monkeypatch) + result = RealTerraformTool().execute( + "init", {"path": str(tmp_path)}, dry_run=False, approval=None + ) + assert result.success + assert "(real)" in result.summary + + +def test_init_failure(monkeypatch, tmp_path) -> None: + _use_fake_terraform(monkeypatch, "init_failure") + result = RealTerraformTool().execute( + "init", {"path": str(tmp_path)}, dry_run=False, approval=None + ) + assert not result.success + assert "(real, failed)" in result.summary + + +def test_validate_success(monkeypatch, tmp_path) -> None: + _use_fake_terraform(monkeypatch) + result = RealTerraformTool().execute( + "validate", {"path": str(tmp_path)}, dry_run=False, approval=None + ) + assert result.success + assert "valid" in result.summary.lower() + + +def test_validate_failure(monkeypatch, tmp_path) -> None: + _use_fake_terraform(monkeypatch, "validate_failure") + result = RealTerraformTool().execute( + "validate", {"path": str(tmp_path)}, dry_run=False, approval=None + ) + assert not result.success + assert result.details["diagnostic_count"] == 1 + + +def test_plan_success_produces_classifier_shaped_details(monkeypatch, tmp_path) -> None: + """Proves the real plan output needs zero changes to + validation/terraform_plan_analysis.analyze().""" + _use_fake_terraform(monkeypatch) + result = RealTerraformTool().execute( + "plan", {"path": str(tmp_path)}, dry_run=False, approval=None + ) + assert result.success + assert result.details.keys() >= {"create", "change", "replace", "destroy"} + assert result.details["create"] == 2 + + summary = terraform_plan_analysis.analyze(result.details) + assert summary.risk_level is RiskLevel.SAFE + + +def test_plan_mixed_actions_classified_correctly(monkeypatch, tmp_path) -> None: + _use_fake_terraform(monkeypatch, "plan_mixed_actions") + result = RealTerraformTool().execute( + "plan", {"path": str(tmp_path)}, dry_run=False, approval=None + ) + assert result.success + assert result.details["create"] == 1 + assert result.details["change"] == 1 + assert result.details["replace"] == 1 + assert result.details["destroy"] == 1 + + summary = terraform_plan_analysis.analyze(result.details) + assert summary.risk_level is RiskLevel.DESTRUCTIVE + + +def test_plan_failure_is_reported_and_secret_is_redacted(monkeypatch, tmp_path) -> None: + _use_fake_terraform(monkeypatch, "plan_failure_with_secret") + result = RealTerraformTool().execute( + "plan", {"path": str(tmp_path)}, dry_run=False, approval=None + ) + assert not result.success + dumped = json.dumps(result.details) + assert "super-secret-value-123" not in dumped + assert "ARM_CLIENT_SECRET=" in dumped + + +def test_plan_show_json_malformed_is_a_clean_failure(monkeypatch, tmp_path) -> None: + _use_fake_terraform(monkeypatch, "show_json_malformed") + result = RealTerraformTool().execute( + "plan", {"path": str(tmp_path)}, dry_run=False, approval=None + ) + assert not result.success + assert "Could not parse" in result.summary + + +def test_plan_details_never_include_raw_attribute_values(monkeypatch, tmp_path) -> None: + _use_fake_terraform(monkeypatch, "plan_with_sensitive_attribute") + result = RealTerraformTool().execute( + "plan", {"path": str(tmp_path)}, dry_run=False, approval=None + ) + assert result.success + assert "hunter2" not in json.dumps(result.details) + + +def test_timeout_is_reported_not_hung(monkeypatch, tmp_path) -> None: + _use_fake_terraform(monkeypatch, "timeout") + result = RealTerraformTool().execute( + "fmt", + {"path": str(tmp_path), "timeout_seconds": 1}, + dry_run=False, + approval=None, + ) + assert not result.success + assert "timed out" in result.details.get("stderr", "").lower() + + +def test_invalid_working_directory_reports_failure(monkeypatch, tmp_path) -> None: + _use_fake_terraform(monkeypatch) + result = RealTerraformTool().execute( + "fmt", + {"path": str(tmp_path / "does-not-exist")}, + dry_run=False, + approval=None, + ) + assert not result.success + + +def test_dry_run_never_invokes_subprocess(monkeypatch, tmp_path) -> None: + monkeypatch.setenv("PATH", str(tmp_path)) # empty dir: would fail loudly if invoked + result = RealTerraformTool().execute( + "plan", {"path": str(tmp_path)}, dry_run=True, approval=None + ) + assert result.success + assert "dry run" in result.summary.lower() diff --git a/tests/tools/test_subprocess_safety.py b/tests/tools/test_subprocess_safety.py new file mode 100644 index 0000000..af9e108 --- /dev/null +++ b/tests/tools/test_subprocess_safety.py @@ -0,0 +1,52 @@ +from devops_learn.tools import _subprocess_safety + + +def test_redact_masks_secret_shaped_key_value_pairs() -> None: + text = "Error: ARM_CLIENT_SECRET=abc123XYZ failed to authenticate" + redacted = _subprocess_safety.redact(text) + assert "abc123XYZ" not in redacted + assert "ARM_CLIENT_SECRET=" in redacted + + +def test_redact_leaves_non_secret_text_untouched() -> None: + text = "Plan: 2 to add, 0 to change, 0 to destroy." + assert _subprocess_safety.redact(text) == text + + +def test_truncate_caps_long_output_with_a_visible_marker() -> None: + long_text = "x" * (_subprocess_safety.MAX_OUTPUT_CHARS + 500) + result = _subprocess_safety._truncate(long_text) + assert len(result) < len(long_text) + assert "truncated" in result + + +def test_run_safely_returns_sanitized_result_on_success() -> None: + result = _subprocess_safety.run_safely( + ["python3", "-c", "print('hello')"], cwd=None, timeout=10 + ) + assert result.returncode == 0 + assert "hello" in result.stdout + assert not result.timed_out + + +def test_run_safely_reports_timeout_without_hanging() -> None: + result = _subprocess_safety.run_safely( + ["python3", "-c", "import time; time.sleep(5)"], cwd=None, timeout=1 + ) + assert result.timed_out + assert result.returncode != 0 + assert "timed out" in result.stderr.lower() + + +def test_run_safely_redacts_secret_shaped_stderr() -> None: + result = _subprocess_safety.run_safely( + [ + "python3", + "-c", + "import sys; sys.stderr.write('API_KEY=super-secret-value')", + ], + cwd=None, + timeout=10, + ) + assert "super-secret-value" not in result.stderr + assert "API_KEY=" in result.stderr diff --git a/tests/workflows/test_terraform_flow.py b/tests/workflows/test_terraform_flow.py new file mode 100644 index 0000000..e8f7f9f --- /dev/null +++ b/tests/workflows/test_terraform_flow.py @@ -0,0 +1,165 @@ +import os +import sqlite3 +from pathlib import Path + +from devops_learn.bootstrap import build_platform +from devops_learn.domain.enums import AuditEventType +from devops_learn.domain.learner_profile_models import ( + CompetencyArea, + LearnerProfile, + ProficiencyLevel, +) +from devops_learn.domain.question_models import ClarifyingQuestion +from devops_learn.tools.approval import AutoApproveApprovalGate +from devops_learn.tools.terraform_tool import RealTerraformTool +from devops_learn.workflows.terraform_flow import TerraformOptions, run_terraform_flow +from devops_learn.workflows.ui import Ui + +FIXTURES_DIR = Path(__file__).resolve().parents[1] / "fixtures" / "fake_terraform" + + +class ScriptedUi(Ui): + def __init__(self) -> None: + self.presented: list[str] = [] + + def present(self, text: str) -> None: + self.presented.append(text) + + def ask_choice(self, question: ClarifyingQuestion) -> str: + return question.options[0] if question.options else "yes" + + def confirm(self, prompt: str, *, default: bool = False) -> bool: + return True + + +def _platform(conn: sqlite3.Connection): + return build_platform( + conn, tools={"terraform": RealTerraformTool()}, approval_gate=AutoApproveApprovalGate() + ) + + +def _use_fake_terraform(monkeypatch, scenario: str) -> None: + monkeypatch.setenv("PATH", f"{FIXTURES_DIR}{os.pathsep}{os.environ['PATH']}") + monkeypatch.setenv("FAKE_TERRAFORM_SCENARIO", scenario) + + +def test_successful_plan_reaches_risk_analysis_and_records_experience( + conn: sqlite3.Connection, monkeypatch, tmp_path +) -> None: + _use_fake_terraform(monkeypatch, "success") + platform = _platform(conn) + ui = ScriptedUi() + session = run_terraform_flow( + platform, ui, TerraformOptions(project_root=str(tmp_path), infra_root=str(tmp_path)) + ) + assert session.id is not None + + joined = "\n".join(ui.presented) + assert "TERRAFORM PLAN" in joined + assert "Risk: SAFE" in joined + + events = {e.event_type for e in platform.audit_service.history(session.id)} + assert AuditEventType.TERRAFORM_PLAN_COMPLETED in events + assert AuditEventType.DEPLOYMENT_FAILED not in events + + experience = platform.experience_tracker.summary(session.id) + assert "Terraform" in experience + assert "Azure" in experience + + +def test_plan_auth_failure_produces_credential_diagnosis( + conn: sqlite3.Connection, monkeypatch, tmp_path +) -> None: + _use_fake_terraform(monkeypatch, "plan_failure_with_secret") + platform = _platform(conn) + ui = ScriptedUi() + session = run_terraform_flow( + platform, ui, TerraformOptions(project_root=str(tmp_path), infra_root=str(tmp_path)) + ) + assert session.id is not None + + joined = "\n".join(ui.presented) + assert "az login" in joined + assert "ARM_CLIENT_SECRET" in joined + assert "super-secret-value-123" not in joined # redacted before ever reaching the UI + + events = {e.event_type for e in platform.audit_service.history(session.id)} + assert AuditEventType.DEPLOYMENT_FAILED in events + assert AuditEventType.TERRAFORM_PLAN_COMPLETED not in events + + +def test_generic_plan_failure_does_not_claim_an_auth_problem( + conn: sqlite3.Connection, monkeypatch, tmp_path +) -> None: + _use_fake_terraform(monkeypatch, "show_json_malformed") + platform = _platform(conn) + ui = ScriptedUi() + run_terraform_flow( + platform, ui, TerraformOptions(project_root=str(tmp_path), infra_root=str(tmp_path)) + ) + + joined = "\n".join(ui.presented) + assert "az login" not in joined + assert "Diagnosis starts from the output above" in joined + + +def test_init_failure_stops_before_plan( + conn: sqlite3.Connection, monkeypatch, tmp_path +) -> None: + _use_fake_terraform(monkeypatch, "init_failure") + platform = _platform(conn) + ui = ScriptedUi() + session = run_terraform_flow( + platform, ui, TerraformOptions(project_root=str(tmp_path), infra_root=str(tmp_path)) + ) + assert session.id is not None + + events = {e.event_type for e in platform.audit_service.history(session.id)} + assert AuditEventType.DEPLOYMENT_FAILED in events + assert AuditEventType.TERRAFORM_PLAN_STARTED not in events + + +def test_beginner_learner_sees_teaching_moments( + conn: sqlite3.Connection, monkeypatch, tmp_path +) -> None: + _use_fake_terraform(monkeypatch, "success") + platform = _platform(conn) + ui = ScriptedUi() + run_terraform_flow( + platform, ui, TerraformOptions(project_root=str(tmp_path), infra_root=str(tmp_path)) + ) + joined = "\n".join(ui.presented) + assert "LEARNING MOMENT: Terraform provider" in joined + assert "LEARNING MOMENT: Terraform state" in joined + + +def test_learner_strong_in_terraform_sees_no_teaching_moments( + conn: sqlite3.Connection, monkeypatch, tmp_path +) -> None: + _use_fake_terraform(monkeypatch, "success") + platform = _platform(conn) + platform.learner_profile_service.save( + LearnerProfile( + proficiencies={CompetencyArea.TERRAFORM: ProficiencyLevel.STRONG}, + learning_focus=(), + ) + ) + ui = ScriptedUi() + run_terraform_flow( + platform, ui, TerraformOptions(project_root=str(tmp_path), infra_root=str(tmp_path)) + ) + joined = "\n".join(ui.presented) + assert "LEARNING MOMENT" not in joined + + +def test_mixed_actions_plan_is_classified_high_risk( + conn: sqlite3.Connection, monkeypatch, tmp_path +) -> None: + _use_fake_terraform(monkeypatch, "plan_mixed_actions") + platform = _platform(conn) + ui = ScriptedUi() + run_terraform_flow( + platform, ui, TerraformOptions(project_root=str(tmp_path), infra_root=str(tmp_path)) + ) + joined = "\n".join(ui.presented) + assert "Risk: DESTRUCTIVE" in joined