Skip to content
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

Fix terraform clean bugs #870

Open
wants to merge 24 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions internal/exec/help.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,24 @@ func processHelp(
u.PrintMessage(fmt.Sprintf("atmos %s <subcommand> <component> -s <stack> [options]", componentType))
u.PrintMessage(fmt.Sprintf("atmos %s <subcommand> <component> --stack <stack> [options]", componentType))
u.PrintMessage(fmt.Sprintf("\nFor more details, execute '%s --help'\n", componentType))
} else if componentType == "terraform" && command == "clean" {
u.PrintMessage("\n'atmos terraform clean' command deletes the following folders and files from the component's directory:\n\n" +
" - '.terraform' folder\n" +
" - folder that the 'TF_DATA_DIR' ENV var points to\n" +
" - '.terraform.lock.hcl' file\n" +
" - generated varfile for the component in the stack\n" +
" - generated planfile for the component in the stack\n" +
" - generated 'backend.tf.json' file\n" +
" - 'terraform.tfstate.d' folder (if '--everything' flag is used)\n\n" +
"Usage: atmos terraform clean <component> -s <stack> <flags>\n\n" +
"Use '--everything' flag to also delete the Terraform state files and and directories with confirm message.\n\n" +
"Use --force to forcefully delete Terraform state files and directories for the component.\n\n" +
"- If no component is specified, the command will apply to all components and stacks.\n" +
"- If no stack is specified, the command will apply to all stacks for the specified component.\n" +
"Use '--skip-lock-file' flag to skip deleting the '.terraform.lock.hcl' file.\n\n" +
"If no component or stack is specified, the clean operation will apply globally to all components.\n\n" +
"For more details refer to https://atmos.tools/cli/commands/terraform/clean\n")

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we're regressing here. This was heavily refactored in #914

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should go away from this file. Is there something we are missing that the code handle comes here?

haitham911 marked this conversation as resolved.
Show resolved Hide resolved
}

return nil
Expand Down
7 changes: 2 additions & 5 deletions internal/exec/terraform.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ const (
outFlag = "-out"
varFileFlag = "-var-file"
skipTerraformLockFileFlag = "--skip-lock-file"
everythingFlag = "--everything"
forceFlag = "--force"
)

Expand All @@ -38,12 +37,10 @@ func shouldProcessStacks(info *schema.ConfigAndStacksInfo) (bool, bool) {
shouldCheckStack := true

if info.SubCommand == "clean" &&
(u.SliceContainsString(info.AdditionalArgsAndFlags, everythingFlag) ||
u.SliceContainsString(info.AdditionalArgsAndFlags, forceFlag)) {
u.SliceContainsString(info.AdditionalArgsAndFlags, forceFlag) {
if info.ComponentFromArg == "" {
shouldProcessStacks = false
}

shouldCheckStack = info.Stack != ""

}
Expand Down Expand Up @@ -128,7 +125,7 @@ func ExecuteTerraform(info schema.ConfigAndStacksInfo) error {
}
}

if !info.ComponentIsEnabled {
if !info.ComponentIsEnabled && info.SubCommand != "clean" {
u.LogInfo(atmosConfig, fmt.Sprintf("component '%s' is not enabled and skipped", info.ComponentFromArg))
return nil
}
Expand Down
9 changes: 4 additions & 5 deletions internal/exec/terraform_clean.go
Original file line number Diff line number Diff line change
Expand Up @@ -347,8 +347,8 @@ func handleTFDataDir(componentPath string, relativePath string, atmosConfig sche
}

}
func initializeFilesToClear(info schema.ConfigAndStacksInfo, atmosConfig schema.AtmosConfiguration, everything bool) []string {
if everything && info.Stack == "" {
func initializeFilesToClear(info schema.ConfigAndStacksInfo, atmosConfig schema.AtmosConfiguration) []string {
if info.ComponentFromArg == "" {
return []string{".terraform", ".terraform.lock.hcl", "*.tfvar.json", "terraform.tfstate.d"}
}
varFile := constructTerraformComponentVarfileName(info)
Expand Down Expand Up @@ -407,8 +407,7 @@ func handleCleanSubCommand(info schema.ConfigAndStacksInfo, componentPath string
}

force := u.SliceContainsString(info.AdditionalArgsAndFlags, forceFlag)
everything := u.SliceContainsString(info.AdditionalArgsAndFlags, everythingFlag)
filesToClear := initializeFilesToClear(info, atmosConfig, everything)
filesToClear := initializeFilesToClear(info, atmosConfig)
folders, err := CollectDirectoryObjects(cleanPath, filesToClear)
if err != nil {
u.LogTrace(atmosConfig, fmt.Errorf("error collecting folders and files: %v", err).Error())
Expand Down Expand Up @@ -456,7 +455,7 @@ func handleCleanSubCommand(info schema.ConfigAndStacksInfo, componentPath string
u.PrintMessage(fmt.Sprintf("Do you want to delete the folder '%s'? ", tfDataDir))
}
var message string
if everything && info.ComponentFromArg == "" {
if info.ComponentFromArg == "" {
message = fmt.Sprintf("This will delete %v local terraform state files affecting all components", total)
} else if info.Component != "" && info.Stack != "" {
message = fmt.Sprintf("This will delete %v local terraform state files for component '%s' in stack '%s'", total, info.Component, info.Stack)
Expand Down
7 changes: 5 additions & 2 deletions internal/exec/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -678,9 +678,12 @@ func processArgsAndFlags(
var additionalArgsAndFlags []string
var globalOptions []string
var indexesToRemove []int
if len(inputArgsAndFlags) == 1 && inputArgsAndFlags[0] == "clean" {
info.SubCommand = inputArgsAndFlags[0]
}

// For commands like `atmos terraform clean` and `atmos terraform plan`, show the command help
if len(inputArgsAndFlags) == 1 && inputArgsAndFlags[0] != "version" {
// For commands like `atmos terraform plan`, show the command help
if len(inputArgsAndFlags) == 1 && inputArgsAndFlags[0] != "version" && info.SubCommand == "" {
info.SubCommand = inputArgsAndFlags[0]
info.NeedHelp = true
return info, nil
Expand Down
143 changes: 143 additions & 0 deletions tests/cli_terraform_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
package tests

import (
"bytes"
"errors"
"fmt"
"os"
"os/exec"
"path/filepath"
"testing"
)

func TestCLITerraformClean(t *testing.T) {
// Capture the starting working directory
startingDir, err := os.Getwd()
if err != nil {
t.Fatalf("Failed to get the current working directory: %v", err)
}

// Initialize PathManager and update PATH
pathManager := NewPathManager()
pathManager.Prepend("../build", "..")
err = pathManager.Apply()
if err != nil {
t.Fatalf("Failed to apply updated PATH: %v", err)
}
fmt.Printf("Updated PATH: %s\n", pathManager.GetPath())
defer func() {
// Change back to the original working directory after the test
if err := os.Chdir(startingDir); err != nil {
t.Fatalf("Failed to change back to the starting directory: %v", err)
}
}()

// Define the work directory and change to it
workDir := "../examples/quick-start-simple"
if err := os.Chdir(workDir); err != nil {
t.Fatalf("Failed to change directory to %q: %v", workDir, err)
}

// Find the binary path for "atmos"
binaryPath, err := exec.LookPath("atmos")
if err != nil {
t.Fatalf("Binary not found: %s. Current PATH: %s", "atmos", pathManager.GetPath())
}
// Run terraform apply for prod environment
runTerraformApply(t, binaryPath, "prod")
verifyStateFilesExist(t, []string{"./components/terraform/weather/terraform.tfstate.d/prod-station"})
runCLITerraformCleanComponent(t, binaryPath, "prod")
verifyStateFilesDeleted(t, []string{"./components/terraform/weather/terraform.tfstate.d/prod-station"})

// Run terraform apply for dev environment
runTerraformApply(t, binaryPath, "dev")

// Run terraform apply for prod environment
runTerraformApply(t, binaryPath, "prod")
haitham911 marked this conversation as resolved.
Show resolved Hide resolved

// Verify if state files exist before cleaning
stateFiles := []string{
"./components/terraform/weather/.terraform",
"./components/terraform/weather/terraform.tfstate.d",
"./components/terraform/weather/.terraform.lock.hcl",
}
verifyStateFilesExist(t, stateFiles)

// Run terraform clean
runTerraformClean(t, binaryPath)

// Verify if state files have been deleted after clean
verifyStateFilesDeleted(t, stateFiles)

}
haitham911 marked this conversation as resolved.
Show resolved Hide resolved

// runTerraformApply runs the terraform apply command for a given environment.
func runTerraformApply(t *testing.T, binaryPath, environment string) {
cmd := exec.Command(binaryPath, "terraform", "apply", "station", "-s", environment)
envVars := os.Environ()
envVars = append(envVars, "ATMOS_COMPONENTS_TERRAFORM_APPLY_AUTO_APPROVE=true")
cmd.Env = envVars

var stdout, stderr bytes.Buffer
cmd.Stdout = &stdout
cmd.Stderr = &stderr
err := cmd.Run()
t.Log(stdout.String())
if err != nil {
t.Fatalf("Failed to run terraform apply station -s %s: %v", environment, stderr.String())
}
}

// verifyStateFilesExist checks if the state files exist before cleaning.
func verifyStateFilesExist(t *testing.T, stateFiles []string) {
for _, file := range stateFiles {
fileAbs, err := filepath.Abs(file)
if err != nil {
t.Fatalf("Failed to resolve absolute path for %q: %v", file, err)
}
if _, err := os.Stat(fileAbs); errors.Is(err, os.ErrNotExist) {
t.Errorf("Expected file to exist before cleaning: %q", fileAbs)
}
}
}

// runTerraformClean runs the terraform clean command.
func runTerraformClean(t *testing.T, binaryPath string) {
cmd := exec.Command(binaryPath, "terraform", "clean", "--force")
var stdout, stderr bytes.Buffer
cmd.Stdout = &stdout
cmd.Stderr = &stderr
err := cmd.Run()
t.Logf("Clean command output:\n%s", stdout.String())
if err != nil {
t.Fatalf("Failed to run terraform clean: %v", stderr.String())
}
}
osterman marked this conversation as resolved.
Show resolved Hide resolved

// verifyStateFilesDeleted checks if the state files have been deleted after cleaning.
func verifyStateFilesDeleted(t *testing.T, stateFiles []string) {
for _, file := range stateFiles {
fileAbs, err := filepath.Abs(file)
if err != nil {
t.Fatalf("Failed to resolve absolute path for %q: %v", file, err)
}
_, err = os.Stat(fileAbs)
if err == nil {
t.Errorf("Expected Terraform state file to be deleted: %q", fileAbs)
} else if !errors.Is(err, os.ErrNotExist) {
t.Errorf("Unexpected error checking file %q: %v", fileAbs, err)
}
}
}

func runCLITerraformCleanComponent(t *testing.T, binaryPath, environment string) {
cmd := exec.Command(binaryPath, "terraform", "clean", "station", "-s", environment, "--force")
var stdout, stderr bytes.Buffer
cmd.Stdout = &stdout
cmd.Stderr = &stderr
err := cmd.Run()
t.Logf("Clean command output:\n%s", stdout.String())
if err != nil {
t.Fatalf("Failed to run terraform clean: %v", stderr.String())
}
}
7 changes: 3 additions & 4 deletions website/docs/cli/commands/terraform/terraform-clean.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ Execute the `terraform clean` command like this:
atmos terraform clean <component> -s <stack> [--skip-lock-file] [--everything] [--force]

:::warning
The `--everything` flag will delete all Terraform-related files including state files. The `--force` flag will bypass confirmation prompts.
The `clean` default behavior and will delete all Terraform-related files including state files, with a confirmation prompt before proceeding. The `--force` flag will bypass confirmation prompts.
haitham911 marked this conversation as resolved.
Show resolved Hide resolved
Use these flags with extreme caution as they can lead to irreversible data loss.
:::
```
Expand All @@ -36,10 +36,9 @@ Run `atmos terraform clean --help` to see all the available options

```shell
# Delete all Terraform-related files for all components (with confirmation)
atmos terraform clean --everything

atmos terraform clean
# Force delete all Terraform-related files for all components (no confirmation)
atmos terraform clean --everything --force
atmos terraform clean --force
atmos terraform clean top-level-component1 -s tenant1-ue2-dev
atmos terraform clean infra/vpc -s tenant1-ue2-staging
atmos terraform clean infra/vpc -s tenant1-ue2-staging --skip-lock-file
Expand Down
14 changes: 7 additions & 7 deletions website/docs/cli/commands/terraform/usage.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,11 @@ HCL-based domain-specific language and its interpreter. Atmos works with [OpenTo

- `atmos terraform clean` command deletes the `.terraform` folder, `.terraform.lock.hcl` lock file, and the previously generated `planfile`
and `varfile` for the specified component and stack. Use the `--skip-lock-file` flag to skip deleting the `.terraform.lock.hcl` file.
Use the `--everything` flag to delete all the local Terraform state files and directories (including `terraform.tfstate.d`) for all components and stacks.
Use the `--force` flag to bypass the safety confirmation prompt and force the deletion (use with caution).
It deletes all local Terraform state files and directories (including `terraform.tfstate.d/`) for all components and stacks.
The `--force` flag bypasses the safety confirmation prompt and forces the deletion. Use with caution.

:::warning
The `--everything` flag performs destructive operations that can lead to permanent state loss. Always ensure you have remote state configured in your components before proceeding.
The `clean` performs destructive operations that can lead to permanent state loss. Always ensure you have remote state configured in your components before proceeding.
haitham911 marked this conversation as resolved.
Show resolved Hide resolved
:::

- `atmos terraform workspace` command first runs `terraform init -reconfigure`, then `terraform workspace select`, and if the workspace was not
Expand Down Expand Up @@ -113,16 +113,16 @@ atmos terraform destroy test/test-component-override -s tenant1-ue2-dev --redire
atmos terraform init test/test-component-override-3 -s tenant1-ue2-dev

# Clean all components (with confirmation)
atmos terraform clean --everything
atmos terraform clean

# Clean a specific component
atmos terraform clean vpc --everything
atmos terraform clean vpc

# Clean a specific component in a stack
atmos terraform clean vpc --stack dev --everything
atmos terraform clean vpc --stack dev

# Clean without confirmation prompt
atmos terraform clean --everything --force
atmos terraform clean --force
atmos terraform clean test/test-component-override-3 -s tenant1-ue2-dev

atmos terraform workspace test/test-component-override-3 -s tenant1-ue2-dev
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading