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.

Sign up for GitHub

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 9 commits into
base: main
Choose a base branch
from
+12 −10
Open

fix terraform clean bugs #870

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
6832ddd
fix clean cmd
haitham911 Dec 19, 2024
0c90df5
feat --everything is the default behavior
haitham911 Dec 19, 2024
a2b820e
remove debug print
haitham911 Dec 19, 2024
1649cd9
Merge branch 'main' into DEV-2859
osterman Dec 19, 2024
4a9bde6
Update website/docs/cli/commands/terraform/usage.mdx
osterman Dec 26, 2024
755e5c6
Update website/docs/cli/commands/terraform/usage.mdx
osterman Dec 26, 2024
13126b2
Apply suggestions from code review
osterman Dec 26, 2024
7e2f054
Update internal/exec/help.go
osterman Dec 26, 2024
f4a81b3
Merge branch 'main' into DEV-2859
osterman Dec 26, 2024
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
2 changes: 1 addition & 1 deletion internal/exec/help.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ func processHelp(
" - 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 the `--everything` flag to remove all Terraform state files and directories, prompting you for confirmation before proceeding. This is the default behavior.\n\n" +
Copy link
Member

Choose a reason for hiding this comment

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

This is the default behavior

from the description it's not clear what is the default behavior - using the --everything flag, or that it will prompt the user for confirmation.

Please update it and make it more clear why do we need the flag, what is the default behavior, and also how --force flag is related to the --everything flag (they should be used together).

"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" +
Expand Down
7 changes: 2 additions & 5 deletions internal/exec/terraform.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,13 +75,10 @@ func ExecuteTerraform(info schema.ConfigAndStacksInfo) error {
shouldCheckStack := true
// Skip stack processing when cleaning with --everything or --force flags to allow
// cleaning without requiring stack configuration
if info.SubCommand == "clean" &&
(u.SliceContainsString(info.AdditionalArgsAndFlags, everythingFlag) ||
u.SliceContainsString(info.AdditionalArgsAndFlags, forceFlag)) {
if info.SubCommand == "clean" {
if info.ComponentFromArg == "" {
shouldProcessStacks = false
}

shouldCheckStack = info.Stack != ""

}
Expand All @@ -96,7 +93,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
7 changes: 6 additions & 1 deletion internal/exec/utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -657,8 +657,13 @@ func processArgsAndFlags(componentType string, inputArgsAndFlags []string) (sche
var additionalArgsAndFlags []string
var globalOptions []string
var indexesToRemove []int
if len(inputArgsAndFlags) > 1 && inputArgsAndFlags[0] == "clean" {
if !u.SliceContainsString(inputArgsAndFlags, everythingFlag) {
inputArgsAndFlags = append(inputArgsAndFlags, everythingFlag)
}
}

// For commands like `atmos terraform clean` and `atmos terraform plan`, show the command help
// For commands like `atmos terraform plan`, show the command help
if len(inputArgsAndFlags) == 1 && inputArgsAndFlags[0] != "version" {
info.SubCommand = inputArgsAndFlags[0]
info.NeedHelp = true
Expand Down
2 changes: 1 addition & 1 deletion website/docs/cli/commands/terraform/terraform-clean.mdx
View file
Open in desktop
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 `--everything` flag is the 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.
Copy link
Member

Choose a reason for hiding this comment

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

The --everything flag is the default behavior

@haitham911 please review/update the docs and help.

If --everything flag is the default behavior, then why do we need it?
It's prob not a default behavior, the default behavior is not to use the flag, but to specify a component and a stack for which to perform clean.

Copy link
Member

Choose a reason for hiding this comment

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

We have changed it so atmos terraform clean with no arguments, cleans everything but prompts the user.

The previous behavior was to error, and print usage.

We added the --everything flag, but then decided to default it. I guess we can remove the flag.

Use these flags with extreme caution as they can lead to irreversible data loss.
:::
```
Expand Down
4 changes: 2 additions & 2 deletions website/docs/cli/commands/terraform/usage.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@ 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).
The `--everything` flag is the default behavior. 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.
Expand Down
Loading