Add terraform test coverage for vpc + quilt modules#116
Conversation
Plan-only, provider-mocked tests — no AWS credentials, no infrastructure: - modules/vpc: characterize the new-vs-existing network input validation — valid new/existing configs plan cleanly; contradictory configs trip the configuration_error precondition. - modules/quilt: smoke-test the public module end-to-end (vpc + db + search + CloudFormation) through a wrapper root that re-exposes only the non-sensitive stack name. Run both in a new CI `test` job; bump CI Terraform 1.5.0 -> 1.10.0 (mock_provider requires >= 1.7). No module behavior changes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds initial automated Terraform module tests (plan-only, AWS provider mocked) and wires them into CI to validate module logic beyond fmt/validate, especially around VPC input validation and the public quilt module wiring.
Changes:
- Add
terraform testcharacterization tests formodules/vpcnetwork-input validation behavior. - Add a smoke-test wrapper root + fixture to exercise
modules/quiltend-to-end in a mocked, plan-only test. - Add a new CI
testjob and bump CI Terraform from 1.5.0 to 1.10.0.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| modules/vpc/tests/validation.tftest.hcl | Adds mocked-provider plan tests covering valid/invalid new-vs-existing VPC input combinations. |
| modules/quilt/tests/smoke/smoke.tftest.hcl | Adds mocked-provider smoke tests asserting the wrapper exposes expected non-sensitive output. |
| modules/quilt/tests/smoke/main.tf | Adds a wrapper root module for testing modules/quilt without exposing sensitive outputs. |
| modules/quilt/tests/smoke/fixtures/quilt.yaml | Adds a minimal CloudFormation template fixture so plan-time template/file checks resolve. |
| .gitignore | Ignores .terraform.lock.hcl. |
| .github/workflows/ci.yml | Bumps Terraform version and adds a CI job to run terraform test in selected directories. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
sir-sigurd
left a comment
There was a problem hiding this comment.
LGTM. Design: sound — scope is coherent (the TF 1.5.0 → 1.10.0 bump is a real prerequisite, since mock_provider needs ≥ 1.7, and it matches the >= 1.10.0 floor examples/main.tf already declares), and the test-framework wiring checks out: I ran the suite locally and confirmed the mocked provider attaches across the whole tree (vpc + db + search + CFN), the required_providers-at-root block is what makes that attachment happen, expect_failures trips on the precondition rather than some unrelated error, and both layouts discover their .tftest.hcl files.
As groundwork the coverage is deliberately a first pass, so these are notes for the #115 refactor rather than asks on this PR:
- An
internal = truesmoke run would exercise the most conditional wiring inquilt/main.tf— thePublicSubnets/UserSubnetsnull-coalescing and thevar.internal-gatedapiendpoint — which is the surface the Transit Gateway work is most likely to touch. The vpc-level test already coversinternal = true; this is just the end-to-end path. - The current tests pin "valid plans clean / contradictory fails fast" but not which requirement failed (the
expect_failuresonoutput.configuration_errorcollapses all rows into one boolean) or that the requirement checklist stays complete (a silently dropped row would still pass!strcontains(..., "❌")). Localizing the failing row needs a precondition-free output to assert against, so it fits naturally alongside #115 once the row structure settles.
Two small things worth doing here:
- The README "Terraform cheat sheet" has Init/Lint/Validate/Plan but no Test entry, and nothing in the repo records that running the tests needs Terraform ≥ 1.7 (for
mock_provider). A one-line "Test" entry (terraform init -backend=false && terraform testfrom a module/wrapper dir) would give a contributor editing the vpc precondition or quilt wiring a documented way to reproduce CI. - A one-line caution in
modules/quilt/tests/smoke/main.tfthat the wrapper must not outputmodule.quilt.stackor any*_password(the whole point of re-exposing onlystack.name), and that newquiltinputs need threading through here or smoke coverage silently narrows.
Reviewed against 51eae3b
- Pin the quilt smoke wrapper's AWS provider to ~> 6.0 for deterministic CI (the terraform-aws-modules/vpc ~> 6.0 module requires aws >= 6.28; the ~> 5.0 Copilot suggested would fail init). - Assert new_vpc_internal_alb is checked against the new-network requirement set, matching new_vpc_external_alb, so a regression that evaluated the wrong set can't pass silently. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- README cheat sheet: add a Test entry documenting the plan-only/mocked, credential-free workflow and the Terraform >= 1.7 requirement. - quilt smoke wrapper: comment that new quilt inputs must be threaded through (or coverage silently narrows) and that the root must re-expose only the non-sensitive stack name. - Add an internal = true new-VPC smoke run, exercising the PublicSubnets/ UserSubnets coalescing and the internal-gated api endpoint. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@greptileai please re-review |
- Thread api_endpoint + user_subnets through the quilt smoke wrapper and add an existing-VPC + internal-ALB smoke run, which exercises quilt's pass-through of those inputs and the internal-gated CloudFormation wiring. - Make existing_vpc_internal_alb symmetric with existing_vpc_external_alb: assert the existing-network requirement set was evaluated and that vpc_id is surfaced unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@greptileai please re-review |
Description
Pre-factoring PR: adds the repo's first automated module tests, so logic is verified beyond
fmt/validate. This is groundwork for safely changing thevpcmodule's network-input validation (e.g. the Transit Gateway work in #115) — a pure-logic validation bug thatvalidatecan't catch now fails a test instead.Plan-only and provider-mocked: no AWS credentials, no infrastructure.
modules/vpc— characterizes new-vs-existing network input validation: valid new/existing configs (internal + external ALB) plan cleanly; contradictory inputs trip theconfiguration_errorprecondition.modules/quilt— smoke-tests the public module end-to-end (vpc + db + search + CloudFormation) via a thin wrapper root that re-exposes only the non-sensitive stack name.Run in a new CI
testjob. CI Terraform is bumped 1.5.0 → 1.10.0 (the mocked-provider test framework needs ≥ 1.7; 1.10.0 matches the floorexamples/main.tfalready declares).No module behavior changes — additions are test-only plus the CI/Terraform-version bump, so there is no user-facing impact.
TODO
Greptile Summary
This PR adds the repository's first automated module tests for
modules/vpcandmodules/quilt, using Terraform's plan-only mocked-provider framework. No module behavior is changed — all additions are test infrastructure plus a Terraform version bump (1.5.0 → 1.10.0) required by themock_providerfeature.modules/vpc/tests/validation.tftest.hcl— characterises four valid VPC-config paths (new/existing × external/internal ALB) and two explicit rejection cases that exercise theconfiguration_errorprecondition.modules/quilt/tests/smoke/— a thin wrapper root (needed to avoid the sensitive-output restriction onmodule.quilt.stack) plus four smoke-test runs covering the same VPC-path combinations end-to-end.testmatrix job runsterraform init -backend=false && terraform testfor both test roots;.terraform.lock.hclis added to.gitignore.Confidence Score: 5/5
Safe to merge — all changes are test infrastructure and CI configuration with no modifications to any deployable module.
The PR only adds test files, a fixture YAML, CI job configuration, and a README section. No production module logic is touched. The new tests are plan-only with a mocked AWS provider, so they carry no risk of infrastructure side-effects. The one finding is a missing required_version guard in the smoke test wrapper, which is a developer-experience nit with no impact on correctness or CI.
No files require special attention.
Important Files Changed
testmatrix job runningterraform init -backend=false && terraform testformodules/vpcandmodules/quilt/tests/smoke; bumps Terraform from 1.5.0 to 1.10.0 in all jobs..terraform.lock.hclto the ignore list, formalising the existing convention of not committing generated lock files.stack_nameoutput, enablingterraform testto run against thequiltmodule without hitting the sensitive-output restriction. Missingrequired_version = ">= 1.7"that would guard against opaque failures with older Terraform.template_filepath for plan-timefilemd5evaluation; clearly documented as non-deployable.Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD CI["CI: test job\n(matrix)"] VPC_DIR["modules/vpc\n(terraform test)"] SMOKE_DIR["modules/quilt/tests/smoke\n(terraform test)"] CI --> VPC_DIR CI --> SMOKE_DIR VPC_DIR --> VTESTS["validation.tftest.hcl\n4 valid + 2 rejection runs"] VTESTS --> VPC_MOD["modules/vpc\n(mocked aws provider)"] SMOKE_DIR --> STESTS["smoke.tftest.hcl\n4 plan runs"] SMOKE_DIR --> WRAPPER["main.tf wrapper root\nre-exposes stack_name only"] STESTS --> WRAPPER WRAPPER --> QUILT_MOD["modules/quilt"] QUILT_MOD --> VPC_MOD2["modules/vpc\n(mocked aws provider)"] QUILT_MOD --> DB_MOD["modules/db"] QUILT_MOD --> SEARCH_MOD["modules/search"] QUILT_MOD --> CFN["aws_cloudformation_stack\n(fixture: quilt.yaml)"]Reviews (3): Last reviewed commit: "Address review: existing-VPC + internal-..." | Re-trigger Greptile