add terraform plan-diff command

[mcalhoun]

what

Create a new command that shows what has changed between two plan files

why

Helps validate that an "approved plan" hasn't changed prior to deployment

example output

Diff Output
===========

Variables:
----------
~ location: Stockholm => New Jersey

Resources:
-----------

data.http.weather
  ~ id: https://wttr.in/Stockholm?0&format=&lang=se&u=m => https://wttr.in/New+Jersey?0&format=&lang=se&u=m
  ~ url: https://wttr.in/Stockholm?0&format=&lang=se&u=m => https://wttr.in/New+Jersey?0&format=&lang=se&u=m
  ~ response_body: Weather report: Stockholm

     \  /       Partly cloudy
   _ /"".-.     -1(-4) °C      
     \_(   ).   ↓ 8 km/h       
     /(___(__)  10 km          
                0.0 mm         
 => Weather report: New+Jersey

                Overcast
       .--.     +8(4) °C       
    .-(    ).   ↙ 22 km/h      
   (___.__)__)  16 km          
                0.0 mm         

  ~ response_headers: {
    ~ Content-Length: 350 => 304
    ~ Date: Thu, 13 Mar 2025 19:28:58 GMT => Thu, 13 Mar 2025 19:29:17 GMT
}
  ~ body: Weather report: Stockholm

     \  /       Partly cloudy
   _ /"".-.     -1(-4) °C      
     \_(   ).   ↓ 8 km/h       
     /(___(__)  10 km          
                0.0 mm         
 => Weather report: New+Jersey

                Overcast
       .--.     +8(4) °C       
    .-(    ).   ↙ 22 km/h      
   (___.__)__)  16 km          
                0.0 mm         

local_file.cache
  ~ content: Weather report: Stockholm

     \  /       Partly cloudy
   _ /"".-.     -1(-4) °C      
     \_(   ).   ↓ 8 km/h       
     /(___(__)  10 km          
                0.0 mm         
 => Weather report: New+Jersey

                Overcast
       .--.     +8(4) °C       
    .-(    ).   ↙ 22 km/h      
   (___.__)__)  16 km          
                0.0 mm         


Outputs:
--------
~ url: https://wttr.in/Stockholm?0&format=&lang=se&u=m => https://wttr.in/New+Jersey?0&format=&lang=se&u=m
~ weather: Weather report: Stockholm

     \  /       Partly cloudy
   _ /"".-.     -1(-4) °C      
     \_(   ).   ↓ 8 km/h       
     /(___(__)  10 km          
                0.0 mm         
 => Weather report: New+Jersey

                Overcast
       .--.     +8(4) °C       
    .-(    ).   ↙ 22 km/h      
   (___.__)__)  16 km          
                0.0 mm         

~ location: Stockholm => (sensitive value)

Windows-Specific Logic

This PR includes some error handling to deal with two distinct Windows-specific issues that caused intermittent test failures:

Process Termination & Goroutine Synchronization

Windows handles process termination differently than Unix systems. When intercepting os.Exit() calls in tests, Windows may not properly schedule the goroutine to completion, causing deadlocks when waiting for both exit codes and goroutine completion signals.

 // Handle Windows specially - just wait for exit code  
   if runtime.GOOS == "windows" {
       return <-exitCodeCh
   }

This specialized Windows path ensures we only wait for the exit code without expecting the goroutine to signal completion, avoiding deadlocks specific to Windows process termination behavior.

File System Timing Issues

Windows file systems sometimes experience timing inconsistencies where file handles aren't fully released or contents aren't completely flushed before subsequent operations start.

// Add a small delay to ensure Windows file operations are complete
   time.Sleep(500 * time.Millisecond)

This small delay before the on-the-fly plan-diff ensures previous Terraform operations fully complete their file I/O before starting the next test phase, preventing race conditions in file access.

These changes significantly improve test reliability on Windows environments while preserving the original test behavior on Unix platforms.

Summary by CodeRabbit

Summary by CodeRabbit

• New Features

  • Introduced a new “plan-diff” command that compares two Terraform plans, highlighting differences in variables, resources, and outputs.
  • Improved error reporting that now provides distinct exit statuses when plan differences are detected.

• Documentation

  • Updated CLI help output and online guides to include detailed usage instructions, flag requirements, and examples for the new plan-diff functionality.

[mergify]

Warning

This PR exceeds the recommended limit of 1,000 lines.

Large PRs are difficult to review and may be rejected due to their size.

Please verify that this PR does not address multiple issues.
Consider refactoring it into smaller, more focused PRs to facilitate a smoother review process.

[github-advanced-security]

golangci-lint found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (2)

main_plan_diff_integration_test.go (1)

120-121: Consider replacing static sleep with a robust check.

A 500ms sleep may not always suffice, especially on congested systems or slow environments. Consider a loop-based wait or a file lock mechanism for Windows-specific file timing issues.

- time.Sleep(500 * time.Millisecond)
// E.g., poll until file operations complete or a max timeout is reached
+ for i := 0; i < 10; i++ {
+   if checkAllFilesAreClosedOrReady() {
+       break
+   }
+   time.Sleep(100 * time.Millisecond)
+ }

internal/exec/terraform_plan_diff.go (1)

149-160: Avoid forced exit within library code.

Calling u.OsExit(2) here precludes upstream callers from handling differences in a customized way. Consider returning an error or a sentinel value to let callers decide how to exit or continue.

- u.OsExit(2)
- return nil // This line will never be reached
+ return errors.New("terraform plans differ -- exit code 2 recommended")

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e74a4a9 and 9f9402a.

📒 Files selected for processing (2)

• internal/exec/terraform_plan_diff.go (1 hunks)
• main_plan_diff_integration_test.go (1 hunks)

⏰ Context from checks skipped due to timeout of 90000ms (3)

• GitHub Check: Acceptance Tests (macos-latest, macos)
• GitHub Check: Acceptance Tests (windows-latest, windows)
• GitHub Check: Summary

🔇 Additional comments (3)

main_plan_diff_integration_test.go (2)

13-27: Well-structured approach to capturing exit codes.

This technique helps keep the test process under control by avoiding an actual os.Exit, ensuring integration tests can robustly verify different exit conditions. Great work!

---

54-67:

Details

❓ Verification inconclusive

Request concurrency consideration.

Modifying global state like runtime.GOOS checks or intercepting u.OsExit can be risky if tests run in parallel. Confirm no concurrency issues arise or mark this test as serial if needed.

Please verify with your CI or test runner that these tests do not run concurrently, or otherwise provide a concurrency-safe approach (e.g., using sync primitives).

---

🏁 Script executed:

#!/bin/bash
# Attempt to locate any concurrency or parallel annotation in the test suite
rg -A 3 't\.Parallel\(\)'

Length of output: 27

---

Concurrency Check: Verify Serial Execution of Affected Tests

The search for t.Parallel() didn’t return any output, so we can’t be sure that these tests aren’t running concurrently. Please manually verify with your CI or test runner that tests—especially those manipulating global state (like runtime.GOOS and intercepting u.OsExit)—aren’t executed in parallel. If there’s a risk of concurrent execution, consider marking this test as serial or introducing explicit synchronization.

internal/exec/terraform_plan_diff.go (1)

33-45: Good use of a temporary directory.

You are properly isolating intermediate files and cleaning them up after use, preserving the host environment’s cleanliness.

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (3)

internal/exec/terraform_plan_diff.go (3)

31-69: Consider returning errors instead of calling os.Exit.

Forcing process termination at line 159 with u.OsExit(2) can surprise callers and complicate testing. It may be more flexible to propagate an error indicating a plan diff was detected, so the calling code can handle that gracefully.

---

72-98: Double-check handling of --orig and --new flags.

The flag parsing logic looks correct. As a small preventive measure, consider validating that each flag’s argument does not begin with -- to avoid flag collisions (e.g., --orig --new something).

---

115-165: Avoid overshadowing stdout for diff messages.

The diff is printed to os.Stdout before calling u.OsExit(2). If other logs or data are being printed concurrently, it might be interleaved. Consider capturing output separately and printing in a controlled manner.

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9f9402a and 1ab52fe.

📒 Files selected for processing (1)

• internal/exec/terraform_plan_diff.go (1 hunks)

🧰 Additional context used

🧬 Code Definitions (1)

internal/exec/terraform_plan_diff.go (5)

internal/exec/terraform_plan_diff_preparation.go (2) (2)

• err (18-18)
• prepareNewPlanFile (15-34)

internal/exec/terraform_plan_diff_comparison.go (8) (8)

• sortMapKeys (11-30)
• diff (54-54)
• diff (85-85)
• diff (121-121)
• diff (140-140)
• diff (295-295)
• diff (323-323)
• generatePlanDiff (53-76)

pkg/errors/terraform_errors.go (1) (1)

• ErrPlanHasDiff (6-6)

pkg/utils/log_utils.go (1) (1)

• OsExit (23-23)

internal/exec/constants.go (1) (1)

• planFileMode (18-18)

⏰ Context from checks skipped due to timeout of 90000ms (2)

• GitHub Check: Build (windows-latest, windows)
• GitHub Check: Summary

🔇 Additional comments (1)

internal/exec/terraform_plan_diff.go (1)

167-191: Revisit potential large plan file outputs.

This logic calls runTerraformShow, which can generate large JSON outputs. Ensure there’s enough buffer capacity or streaming approach to avoid choking the pipe if Terraform outputs large data.

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (6)

internal/exec/terraform_plan_diff.go (6)

40-43: Consider adding test coverage for error handling when creating the temporary directory.
This block is not covered by tests according to the static analysis and is critical for robust error scenarios.

🧰 Tools

🪛 GitHub Check: codecov/patch

[warning] 42-43: internal/exec/terraform_plan_diff.go#L42-L43
Added lines #L42 - L43 were not covered by tests

---

63-65: Add negative test scenarios to exercise prepareNewPlanFile failures.
If prepareNewPlanFile returns an error, these lines remain untested. Consider adding coverage to confirm correct handling of initialization failures.

🧰 Tools

🪛 GitHub Check: codecov/patch

[warning] 64-65: internal/exec/terraform_plan_diff.go#L64-L65
Added lines #L64 - L65 were not covered by tests

---

124-127: Add coverage for the error branch during retrieval of the new plan JSON.
Simulate a situation in which getTerraformPlanJSON fails, ensuring proper error propagation is tested.

🧰 Tools

🪛 GitHub Check: codecov/patch

[warning] 124-127: internal/exec/terraform_plan_diff.go#L124-L127
Added lines #L124 - L127 were not covered by tests

---

130-139: Test invalid or malformed plan JSON scenarios.
These lines that parse JSON remain untested for error conditions. Include coverage for corrupted or empty JSON to solidify error handling.

🧰 Tools

🪛 GitHub Check: codecov/patch

[warning] 130-134: internal/exec/terraform_plan_diff.go#L130-L134
Added lines #L130 - L134 were not covered by tests

---

[warning] 136-139: internal/exec/terraform_plan_diff.go#L136-L139
Added lines #L136 - L139 were not covered by tests

---

208-234: Expand testing to cover plan file copying logic.
Simulate a scenario where the source file is missing or the destination is unwritable, ensuring each step of the copy flow is validated.

🧰 Tools

🪛 GitHub Check: codecov/patch

[warning] 208-234: internal/exec/terraform_plan_diff.go#L208-L234
Added lines #L208 - L234 were not covered by tests

---

299-310: Increase test coverage for runTerraformInit logic.
Verifying the -reconfigure flag scenario on different platforms ensures consistent initialization.

🧰 Tools

🪛 GitHub Check: codecov/patch

[warning] 299-299: internal/exec/terraform_plan_diff.go#L299
Added line #L299 was not covered by tests

---

[warning] 310-310: internal/exec/terraform_plan_diff.go#L310
Added line #L310 was not covered by tests

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1ab52fe and e586e6d.

📒 Files selected for processing (1)

• internal/exec/terraform_plan_diff.go (1 hunks)

🧰 Additional context used

🧬 Code Definitions (1)

internal/exec/terraform_plan_diff.go (5)

internal/exec/terraform_plan_diff_preparation.go (2) (2)

• err (18-18)
• prepareNewPlanFile (15-34)

internal/exec/terraform_plan_diff_comparison.go (8) (8)

• sortMapKeys (11-30)
• diff (54-54)
• diff (85-85)
• diff (121-121)
• diff (140-140)
• diff (295-295)
• diff (323-323)
• generatePlanDiff (53-76)

pkg/errors/terraform_errors.go (1) (1)

• ErrPlanHasDiff (6-6)

pkg/utils/log_utils.go (1) (1)

• OsExit (23-23)

internal/exec/constants.go (1) (1)

• planFileMode (18-18)

🪛 GitHub Check: codecov/patch

internal/exec/terraform_plan_diff.go

[warning] 42-43: internal/exec/terraform_plan_diff.go#L42-L43
Added lines #L42 - L43 were not covered by tests

---

[warning] 64-65: internal/exec/terraform_plan_diff.go#L64-L65
Added lines #L64 - L65 were not covered by tests

---

[warning] 124-127: internal/exec/terraform_plan_diff.go#L124-L127
Added lines #L124 - L127 were not covered by tests

---

[warning] 130-134: internal/exec/terraform_plan_diff.go#L130-L134
Added lines #L130 - L134 were not covered by tests

---

[warning] 136-139: internal/exec/terraform_plan_diff.go#L136-L139
Added lines #L136 - L139 were not covered by tests

---

[warning] 142-161: internal/exec/terraform_plan_diff.go#L142-L161
Added lines #L142 - L161 were not covered by tests

---

[warning] 163-164: internal/exec/terraform_plan_diff.go#L163-L164
Added lines #L163 - L164 were not covered by tests

---

[warning] 175-181: internal/exec/terraform_plan_diff.go#L175-L181
Added lines #L175 - L181 were not covered by tests

---

[warning] 184-187: internal/exec/terraform_plan_diff.go#L184-L187
Added lines #L184 - L187 were not covered by tests

---

[warning] 190-190: internal/exec/terraform_plan_diff.go#L190
Added line #L190 was not covered by tests

---

[warning] 208-234: internal/exec/terraform_plan_diff.go#L208-L234
Added lines #L208 - L234 were not covered by tests

---

[warning] 237-240: internal/exec/terraform_plan_diff.go#L237-L240
Added lines #L237 - L240 were not covered by tests

---

[warning] 243-243: internal/exec/terraform_plan_diff.go#L243
Added line #L243 was not covered by tests

---

[warning] 247-252: internal/exec/terraform_plan_diff.go#L247-L252
Added lines #L247 - L252 were not covered by tests

---

[warning] 255-282: internal/exec/terraform_plan_diff.go#L255-L282
Added lines #L255 - L282 were not covered by tests

---

[warning] 284-284: internal/exec/terraform_plan_diff.go#L284
Added line #L284 was not covered by tests

---

[warning] 299-299: internal/exec/terraform_plan_diff.go#L299
Added line #L299 was not covered by tests

---

[warning] 310-310: internal/exec/terraform_plan_diff.go#L310
Added line #L310 was not covered by tests

⏰ Context from checks skipped due to timeout of 90000ms (3)

• GitHub Check: Acceptance Tests (macos-latest, macos)
• GitHub Check: Acceptance Tests (windows-latest, windows)
• GitHub Check: Summary

🔇 Additional comments (1)

internal/exec/terraform_plan_diff.go (1)

247-285: Capture stderr output for enhanced debugging.
Terraform may generate errors on stderr, so capturing only stdout can hamper troubleshooting.

🧰 Tools

🪛 GitHub Check: codecov/patch

[warning] 247-252: internal/exec/terraform_plan_diff.go#L247-L252
Added lines #L247 - L252 were not covered by tests

---

[warning] 255-282: internal/exec/terraform_plan_diff.go#L255-L282
Added lines #L255 - L282 were not covered by tests

---

[warning] 284-284: internal/exec/terraform_plan_diff.go#L284
Added line #L284 was not covered by tests