-
Notifications
You must be signed in to change notification settings - Fork 9.5k
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
Change -force to -auto-approve when destroying #17218
Change -force to -auto-approve when destroying #17218
Conversation
Hi @surminus! Thanks for working on this. The choice of As you noted, the Given the reasoning above, what do you think about making the opposite change here, so that |
Hi @apparentlymart! Thanks for getting back to me and providing the context on this. Appreciate ambiguity of the |
Since an early version of Terraform, the `destroy` command has always had the `-force` flag to allow an auto approval of the interactive prompt. 0.11 introduced `-auto-approve` as default to `false` when using the `apply` command. The `-auto-approve` flag was introduced to reduce ambiguity of it's function, but the `-force` flag was never updated for a destroy. People often use wrappers when automating commands in Terraform, and the inconsistency between `apply` and `destroy` means that additional logic must be added to the wrappers to do similar functions. Both commands are more or less able to run with similar syntax, and also heavily share their code. This commit updates the command in `destroy` to use the `-auto-approve` flag making working with the Terraform CLI a more consistent experience. We leave in `-force` in `destroy` for the time-being and flag it as deprecated to ensure a safe switchover period.
3ae9b8c
to
6e1e614
Compare
@apparentlymart I've pushed a new commit with the changes - I actually had to understand the code a little bit this time rather than just a search and replace! I couldn't find a specific alias function in the flag library used for CLI options, so I added in a bit of logic that can be removed in the future instead. |
Hi @surminus! Sorry for the silence here. This is definitely on our list to review or merge, but the team is currently focused in a different area (the configuration loader) and so we've been holding off on merging this until we're able to context-switch back into "CLI land". I'm sorry we haven't been able to get to this sooner, but we'll take a look at it once we are able to take a breather from our current configuration-related projects. |
No problem, thanks for letting me know! |
Eager for this change - the asymmetry is really poor CLI design, very annoying during development. |
Thanks for your patience here, @surminus! This change looks great to me... thanks again for working on it. It'll be included in the next release. |
Great thanks @apparentlymart, pleased I could contribute! |
I know this may sound weird, but Just yesterday I accidentally did a cmd+r and typed appro Had this flag existed on my current version, I would have accidentally destroyed my environment with no option to validate. Yes, it would have been my fault for not being careful, but tooling should also prevent us from accidently executing potentially damaging actions. Thanks |
It would be great if you could add something like I really don't want anyone able to run something against prod with auto-approve per @FernandoMiguel 's comment above. |
@FernandoMiguel @unixorn FWIW some people (including us internally) use Terraform within automation and they always need non-interactive ways of running it, so that capability is a requirement. You could always alias terraform to a function that strips that argument off if you wanted. |
The tooling does protect you from accidentally running damaging actions - you have to explicitly set the |
@paultyng if the flag is the same as the apply, i find it to be risky. I'm not opposed to automation, but I would rather have a different flag for apply and destroy, even if that is a small burden on users to remember/search |
I think that would be a poor CLI API design choice. |
Fixes hashicorp#17671 This `auto-approve` flag was in `apply -help` on Terraform v0.11.3, but it was deleted by mistake in hashicorp#17218. This fix restores it.
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
Since an early version of Terraform, the
destroy
command has always had the-force
flag to allow an auto approval of the interactive prompt. 0.11 introduced-auto-approve
as default tofalse
when using theapply
command.People often use wrappers when automating commands in Terraform, and the inconsistency between
apply
anddestroy
means that additional logic must be added to the wrappers to do similar functions. Both commands are more or less able to run with similar syntax, and I could not find a reason for the inconsistency between the two commands despite the functionality being the same.This commit updates the command in
apply
to use the-force
flag making working with the Terraform CLI a more consistent experience.I searched PRs and issues and could not find any context on why this was implemented the way that it was, or if there was a specific reason why the differences exist between
apply
anddestroy
. I also appreciate this is a breaking change.