-
Notifications
You must be signed in to change notification settings - Fork 0
chore(tests): add an end to end test that uses real terraform plan output #13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
zpratt
commented
Nov 2, 2025
- chore(end-to-end-tests): add a full end to end test that uses real terraform plan output
- fix: use a pinned version of terraform
|
|
||
| // Generate plan | ||
| console.log("Generating Terraform plan..."); | ||
| execSync(`terraform plan -out=${planFile}`, { |
Check warning
Code scanning / CodeQL
Shell command built from environment values Medium
absolute path
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 5 days ago
The best fix is to avoid constructing shell command strings by interpolating dynamic paths. Instead, use argument arrays to pass parameters directly to the underlying process, thereby avoiding shell interpolation altogether (which can be tricked by paths with spaces or shell metacharacters). Specifically, for the vulnerable terraform plan -out=${planFile} invocation at line 35, move from backtick/interpolated string usage to argument array usage like ["plan", "-out", planFile]. Change the invocation on line 35 to:
execSync("terraform plan -out=${planFile}", { ... });to
execSync("terraform", ["plan", "-out", planFile], { ... });Also, be sure to select the correct method signature: execSync(command, options) runs via the shell, while execFileSync(file, args, options) runs the file directly with argument separation (the secure way). Therefore, switch to execFileSync from the node:child_process module.
In addition, you must import execFileSync at the top if not already imported (which is not done in the shown code), since currently only execSync is imported.
The fix only needs to be applied to line 35 (and the import statement at the top).
-
Copy modified line R1 -
Copy modified line R35
| @@ -1,4 +1,4 @@ | ||
| import { execSync } from "node:child_process"; | ||
| import { execSync, execFileSync } from "node:child_process"; | ||
| import { existsSync, rmSync, unlinkSync } from "node:fs"; | ||
| import * as path from "node:path"; | ||
| import { GenericContainer, type StartedTestContainer } from "testcontainers"; | ||
| @@ -32,7 +32,7 @@ | ||
|
|
||
| // Generate plan | ||
| console.log("Generating Terraform plan..."); | ||
| execSync(`terraform plan -out=${planFile}`, { | ||
| execFileSync("terraform", ["plan", "-out", planFile], { | ||
| cwd: terraformDir, | ||
| stdio: "pipe", | ||
| }); |
|
|
||
| // Convert plan to JSON | ||
| console.log("Converting plan to JSON..."); | ||
| execSync(`terraform show -json ${planFile} > ${planJsonFile}`, { |
Check warning
Code scanning / CodeQL
Shell command built from environment values Medium
absolute path
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 5 days ago
To fix the highlighted issue, we should avoid dynamically constructing a shell command string where path variables could include unsafe shell metacharacters. Instead, we should invoke the underlying command directly using an API that accepts command arguments as an array, with output redirection done programmatically, not via shell syntax. Specifically, for terraform show -json ${planFile} > ${planJsonFile}, we should:
- Use
execFileSyncfromchild_processto execute:- Command:
terraform - Arguments:
["show", "-json", planFile]
- Command:
- Capture the stdout result of the command and write it to
planJsonFileusing node'sfsfunctionality (writeFileSync). - Remove the output redirection (
> ...) from the shell command string and the need for a shell.
Only lines 42-45 are affected in e2e/terraform-integration.test.ts.
No new method definitions are required, but we must ensure that writeFileSync is imported from fs if not already present.
-
Copy modified line R2 -
Copy modified line R42 -
Copy modified line R46
| @@ -1,5 +1,5 @@ | ||
| import { execSync } from "node:child_process"; | ||
| import { existsSync, rmSync, unlinkSync } from "node:fs"; | ||
| import { existsSync, rmSync, unlinkSync, writeFileSync } from "node:fs"; | ||
| import * as path from "node:path"; | ||
| import { GenericContainer, type StartedTestContainer } from "testcontainers"; | ||
| import { afterAll, beforeAll, describe, expect, it } from "vitest"; | ||
| @@ -39,11 +39,11 @@ | ||
|
|
||
| // Convert plan to JSON | ||
| console.log("Converting plan to JSON..."); | ||
| execSync(`terraform show -json ${planFile} > ${planJsonFile}`, { | ||
| const planJsonOutput = execSync("terraform show -json " + planFile, { | ||
| cwd: terraformDir, | ||
| stdio: "pipe", | ||
| shell: "/bin/bash", | ||
| }); | ||
| writeFileSync(planJsonFile, planJsonOutput); | ||
| } catch (error) { | ||
| console.error("Setup failed:", error); | ||
| throw error; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds end-to-end integration testing for Terraform using moto (AWS API mocking) and testcontainers. The changes enable testing of the infra-diff functionality against real Terraform plan outputs in an isolated environment.
- Implements Terraform integration testing with moto server running in Docker containers
- Adds development container configuration for consistent local development
- Configures CI pipeline to run Terraform integration tests with proper version pinning
- Introduces testcontainers library to manage Docker containers programmatically
Reviewed Changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Added testcontainers dependency and new npm script for terraform e2e tests |
| package-lock.json | Lock file updates for testcontainers and its transitive dependencies |
| features/end-to-end-with-terraform.md | Feature documentation describing the test goals and requirements |
| e2e/terraform-integration.test.ts | New e2e test file implementing Terraform plan generation and parsing |
| e2e/terraform/*.tf | Terraform fixtures for testing (provider config, backend, SQS resource) |
| docker-compose.yml | Docker compose file for local moto server execution |
| .terraform-version | Terraform version pinning file |
| .github/workflows/ci.yml | Added new CI job for Terraform integration tests |
| .github/dependabot.yml | Added Terraform dependency monitoring |
| .devcontainer/devcontainer.json | Development container configuration |
| "@vercel/ncc": "^0.38.4", | ||
| "@vitest/ui": "^4.0.6", | ||
| "chance": "^1.1.13", | ||
| "testcontainers": "11.7.2", |
Copilot
AI
Nov 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The testcontainers dependency should use a caret (^) version constraint instead of an exact version pin. This aligns with the project's dependency convention seen in other devDependencies (e.g., 'vitest': '^4.0.6') and allows for automatic patch updates while maintaining compatibility.
| "testcontainers": "11.7.2", | |
| "testcontainers": "^11.7.2", |
| describe("E2E: Terraform Integration with moto", () => { | ||
| const terraformDir = path.join(process.cwd(), "e2e", "terraform"); | ||
| const planFile = path.join(terraformDir, "plan.bin"); | ||
| const planJsonFile = path.join(terraformDir, "plan.json"); | ||
| let motoContainer: StartedTestContainer; | ||
| let motoPort: number; | ||
|
|
||
| beforeAll(async () => { | ||
| // Start moto server using testcontainers | ||
| console.log("Starting moto server..."); | ||
| try { | ||
| motoPort = 50000; | ||
| motoContainer = await new GenericContainer("motoserver/moto:5.0.0") | ||
| .withExposedPorts({ container: 5000, host: motoPort }) | ||
| .withStartupTimeout(120000) | ||
| .start(); | ||
|
|
||
| console.log(`Moto server is ready on port ${motoPort}`); | ||
|
|
||
| // Initialize Terraform | ||
| console.log("Initializing Terraform..."); | ||
| execSync("terraform init", { cwd: terraformDir, stdio: "pipe" }); | ||
|
|
||
| // Generate plan | ||
| console.log("Generating Terraform plan..."); | ||
| execSync(`terraform plan -out=${planFile}`, { | ||
| cwd: terraformDir, | ||
| stdio: "pipe", | ||
| }); | ||
|
|
||
| // Convert plan to JSON | ||
| console.log("Converting plan to JSON..."); | ||
| execSync(`terraform show -json ${planFile} > ${planJsonFile}`, { | ||
| cwd: terraformDir, | ||
| stdio: "pipe", | ||
| shell: "/bin/bash", | ||
| }); | ||
| } catch (error) { | ||
| console.error("Setup failed:", error); | ||
| throw error; | ||
| } | ||
| }, 120000); // 2 minute timeout for setup | ||
|
|
Copilot
AI
Nov 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using console.log for test output violates the operational convention stated in the coding guidelines: 'ensure all application logs are structured as JSON objects, except when logging to the console in GitHub Actions.' Since this is a test file, consider using the test framework's logging capabilities or a structured logger.
| describe("E2E: Terraform Integration with moto", () => { | |
| const terraformDir = path.join(process.cwd(), "e2e", "terraform"); | |
| const planFile = path.join(terraformDir, "plan.bin"); | |
| const planJsonFile = path.join(terraformDir, "plan.json"); | |
| let motoContainer: StartedTestContainer; | |
| let motoPort: number; | |
| beforeAll(async () => { | |
| // Start moto server using testcontainers | |
| console.log("Starting moto server..."); | |
| try { | |
| motoPort = 50000; | |
| motoContainer = await new GenericContainer("motoserver/moto:5.0.0") | |
| .withExposedPorts({ container: 5000, host: motoPort }) | |
| .withStartupTimeout(120000) | |
| .start(); | |
| console.log(`Moto server is ready on port ${motoPort}`); | |
| // Initialize Terraform | |
| console.log("Initializing Terraform..."); | |
| execSync("terraform init", { cwd: terraformDir, stdio: "pipe" }); | |
| // Generate plan | |
| console.log("Generating Terraform plan..."); | |
| execSync(`terraform plan -out=${planFile}`, { | |
| cwd: terraformDir, | |
| stdio: "pipe", | |
| }); | |
| // Convert plan to JSON | |
| console.log("Converting plan to JSON..."); | |
| execSync(`terraform show -json ${planFile} > ${planJsonFile}`, { | |
| cwd: terraformDir, | |
| stdio: "pipe", | |
| shell: "/bin/bash", | |
| }); | |
| } catch (error) { | |
| console.error("Setup failed:", error); | |
| throw error; | |
| } | |
| }, 120000); // 2 minute timeout for setup |
| .withStartupTimeout(120000) | ||
| .start(); | ||
|
|
||
| console.log(`Moto server is ready on port ${motoPort}`); |
Copilot
AI
Nov 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using console.log for test output violates the operational convention stated in the coding guidelines: 'ensure all application logs are structured as JSON objects, except when logging to the console in GitHub Actions.' Since this is a test file, consider using the test framework's logging capabilities or a structured logger.
| console.log(`Moto server is ready on port ${motoPort}`); | |
| console.log(JSON.stringify({ event: "moto_server_ready", port: motoPort })); |
| console.log(`Moto server is ready on port ${motoPort}`); | ||
|
|
||
| // Initialize Terraform | ||
| console.log("Initializing Terraform..."); |
Copilot
AI
Nov 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using console.log for test output violates the operational convention stated in the coding guidelines: 'ensure all application logs are structured as JSON objects, except when logging to the console in GitHub Actions.' Since this is a test file, consider using the test framework's logging capabilities or a structured logger.
| console.log("Initializing Terraform..."); | |
| console.log(JSON.stringify({ message: "Initializing Terraform..." })); |
| execSync("terraform init", { cwd: terraformDir, stdio: "pipe" }); | ||
|
|
||
| // Generate plan | ||
| console.log("Generating Terraform plan..."); |
Copilot
AI
Nov 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using console.log for test output violates the operational convention stated in the coding guidelines: 'ensure all application logs are structured as JSON objects, except when logging to the console in GitHub Actions.' Since this is a test file, consider using the test framework's logging capabilities or a structured logger.
| console.log("Generating Terraform plan..."); | |
| console.log(JSON.stringify({ event: "Generating Terraform plan" })); |
| unlinkSync(lockFile); | ||
| } | ||
| } catch (error) { | ||
| console.error("Failed to cleanup lock file:", error); |
Copilot
AI
Nov 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using console.error violates the operational convention stated in the coding guidelines: 'ensure all application logs are structured as JSON objects, except when logging to the console in GitHub Actions.' Since this is a test file, consider using the test framework's logging capabilities or a structured logger.
| try { | ||
| if (motoContainer) { | ||
| await motoContainer.stop(); | ||
| console.log("Moto container stopped"); |
Copilot
AI
Nov 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using console.log for test output violates the operational convention stated in the coding guidelines: 'ensure all application logs are structured as JSON objects, except when logging to the console in GitHub Actions.' Since this is a test file, consider using the test framework's logging capabilities or a structured logger.
| console.log("Moto container stopped"); | ||
| } | ||
| } catch (error) { | ||
| console.error("Failed to stop moto container:", error); |
Copilot
AI
Nov 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using console.error violates the operational convention stated in the coding guidelines: 'ensure all application logs are structured as JSON objects, except when logging to the console in GitHub Actions.' Since this is a test file, consider using the test framework's logging capabilities or a structured logger.
| beforeAll(async () => { | ||
| // Start moto server using testcontainers | ||
| console.log("Starting moto server..."); | ||
| try { | ||
| motoPort = 50000; | ||
| motoContainer = await new GenericContainer("motoserver/moto:5.0.0") | ||
| .withExposedPorts({ container: 5000, host: motoPort }) | ||
| .withStartupTimeout(120000) | ||
| .start(); | ||
|
|
||
| console.log(`Moto server is ready on port ${motoPort}`); | ||
|
|
||
| // Initialize Terraform | ||
| console.log("Initializing Terraform..."); | ||
| execSync("terraform init", { cwd: terraformDir, stdio: "pipe" }); | ||
|
|
||
| // Generate plan | ||
| console.log("Generating Terraform plan..."); | ||
| execSync(`terraform plan -out=${planFile}`, { | ||
| cwd: terraformDir, | ||
| stdio: "pipe", | ||
| }); | ||
|
|
||
| // Convert plan to JSON | ||
| console.log("Converting plan to JSON..."); | ||
| execSync(`terraform show -json ${planFile} > ${planJsonFile}`, { | ||
| cwd: terraformDir, | ||
| stdio: "pipe", | ||
| shell: "/bin/bash", | ||
| }); | ||
| } catch (error) { | ||
| console.error("Setup failed:", error); | ||
| throw error; | ||
| } | ||
| }, 120000); // 2 minute timeout for setup |
Copilot
AI
Nov 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The beforeAll hook contains multiple complex setup steps (container startup, terraform init, plan, and JSON conversion) that violate the principle of single responsibility. Consider extracting these steps into separate, well-named helper functions to improve readability and maintainability.
| @@ -0,0 +1,15 @@ | |||
| --- | |||
| version: '3.8' | |||
Copilot
AI
Nov 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The 'version' field in docker-compose.yml is deprecated as of Docker Compose v1.27.0+ and is no longer required. Consider removing this line as modern versions of Docker Compose ignore it.
| version: '3.8' |