internal/exec/terraform_utils.go (1)

45-69: Consider handling file permission errors explicitly.

While the error handling is good, it would be helpful to distinguish between permission errors and other types of errors when checking and deleting files.

Here's a suggested improvement:

 	if _, err := os.Stat(filePath); err == nil {
 		l.Debug("Terraform environment file found. Proceeding with deletion.",
 			"file", filePath,
 		)
 		if err := os.Remove(filePath); err != nil {
+			if os.IsPermission(err) {
+				l.Debug("Permission denied when deleting Terraform environment file.",
+					"file", filePath,
+					"error", err,
+				)
+			} else {
 				l.Debug("Failed to delete Terraform environment file.",
 					"file", filePath,
 					"error", err,
 				)
+			}
 		} else {
 			l.Debug("Successfully deleted Terraform environment file.",
 				"file", filePath,
 			)
 		}
 	} else if os.IsNotExist(err) {
 		l.Debug("Terraform environment file not found. No action needed.",
 			"file", filePath,
 		)
+	} else if os.IsPermission(err) {
+		l.Debug("Permission denied when checking Terraform environment file.",
+			"file", filePath,
+			"error", err,
+		)
 	} else {
 		l.Debug("Error checking Terraform environment file.",
 			"file", filePath,
 			"error", err,
 		)
 	}

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

Learnt from: aknysh
PR: cloudposse/atmos#759
File: internal/exec/terraform.go:366-368
Timestamp: 2024-11-12T05:52:05.088Z
Learning: In `internal/exec/terraform.go`, the workspace cleaning code under both the general execution path and within the `case "init":` block is intentionally duplicated because the code execution paths are different. The `.terraform/environment` file should be deleted before executing `terraform init` in both scenarios to ensure a clean state.

internal/exec/terraform_outputs.go (2)

83-83: Consider adding more context to debug logs.

While the logging statements are clear, they could benefit from additional context fields.

Consider enhancing the debug logs with more structured fields:

-l.Debug("Writing the backend config to file:", "file", backendFileName)
+l.Debug("Writing the backend config to file:", "file", backendFileName, "component", component, "stack", stack)

-l.Debug("Wrote the backend config to file:", "file", backendFileName)
+l.Debug("Wrote the backend config to file:", "file", backendFileName, "component", component, "stack", stack)

-l.Debug("Writing the provider overrides to file:", "file", providerOverrideFileName)
+l.Debug("Writing the provider overrides to file:", "file", providerOverrideFileName, "component", component, "stack", stack)

-l.Debug("Wrote the provider overrides to file:", "file", providerOverrideFileName)
+l.Debug("Wrote the provider overrides to file:", "file", providerOverrideFileName, "component", component, "stack", stack)

Also applies to: 105-105, 114-114, 122-122

---

259-262: Consider using a helper function for error handling.

The error handling pattern is repeated in multiple places. Consider extracting it into a helper function.

Create a helper function:

func handleTerraformError(p *tea.Program, spinnerDone chan struct{}, message string, err error, component string, stack string) {
    p.Quit()
    <-spinnerDone
    fmt.Printf("\r✗ %s\n", message)
    l.Error("Terraform operation failed", "component", component, "stack", stack, "error", err)
    os.Exit(1)
}

Then use it in all three places:

-p.Quit()
-<-spinnerDone
-fmt.Printf("\r✗ %s\n", message)
-l.Error("Failed to describe the component", "component", component, "stack", stack, "error", err)
-os.Exit(1)
+handleTerraformError(p, spinnerDone, message, err, component, stack)

Also applies to: 270-273, 286-289

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