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

Please make it clear that the terraform state which exists will be nuked and replaced with the new local infrastructure state. #62

Open
creikey opened this issue Oct 2, 2024 · 11 comments

Comments

@creikey
Copy link

creikey commented Oct 2, 2024

No description provided.

@lakkeger
Copy link
Contributor

lakkeger commented Oct 2, 2024

Hi @creikey,

Sorry to hear you had problem with tflocal.

Let me share my 2 cents on the topic even though I'm not a maintainer of the repo anymore:

Terraform itself is nuking your state if you provide another account's credentials. Period. That's the way it works and that's why you have the plan stage before any applys. Hashicorp emphasizes the vulnerability of the state file in its docs many times.

So by default it's the user's responsibility to isolate an environment ie into workspaces or into a different folder layout and double check if the changes are correct.

I personally can't think of a scenario where you'd be able to check an already existing state programmatically and tell if it was run against LocalStack or a valid AWS account as the stored resource ARNs could contain valid account numbers not just mumbo jumbo accounts.

The best LocalStack could do is provide in the docs and in the README a warning for terraform and any other tool that manages a separately stored statefile(ie Pulumi), but in the end everything comes down to the users and their actions.

@creikey
Copy link
Author

creikey commented Oct 2, 2024

@lakkeger
Yeah I agree I certainly made a mistake procrastinating more robust state management (will probably move it to a row in dynamo or an s3 file after this), I think tflocal as a tool should simply look for a state file, check if it's already referring to localstack infrastructure or not, and if it exists and isn't localstack add an extra Are you sure you want to proceed? You will lose current state before the apply.

I had assumed this tool used state that was separate from the existing local state files in the directory, based on some other language in the documentation

@lakkeger
Copy link
Contributor

lakkeger commented Oct 3, 2024

@creikey The problem with the suggested solution is, terraform has many options where to store your state, ie gitlab state storage or hashi cloud and that certainly would fall through with similar issues here.

Here the solution could be to list the resources in the background with terraform state list.

However, thinking a bit more on the issue the prompt would be a breaking change for anyone who would run terraform-local against LocalStack multiple times as it would prompt every single time for input an extra time if you plan or apply something. 🤔 As a result any existing CI would break instantly that uses tflocal, which could be worked around by introducing a non-interactive flag/env variable, but this won't be the default. In case it's default this change would lose purpose.
As a major version this could be an option, but imo the UX for any normal user would become annoying by prompting twice for confirmation every single time.
Simply showing a warning makes no sense either as your plan stage with a bigger change rolls over this message in the console history. Showing it after the apply prompt makes no sense either.

So all in context of the above your best bet is better docs to raise awareness.

/cc @simonrw @MarcelStranak

@creikey
Copy link
Author

creikey commented Oct 3, 2024

@lakkeger What do you mean? I'm suggesting this tool reads the tfstate it's about to modify, and if and only if the existing state points to non-tflocal resources, there is an explicit confirmation step.

Under none of the environments you listed, CI, running against local stack multiple times, etc does this warning get triggered, because in those cases, there are no non-tflocal resources in the state. Normal users are not bothered because they aren't prompted twice, because the resources are seen to be tflocal by this tool.

@creikey
Copy link
Author

creikey commented Oct 3, 2024

To detect if a tfstate is managed by tflocal or not, it seems like the best bet would be to check for all zeros in the ARN:
"arn": "arn:aws:dynamodb:us-west-2:000000000000:table/TestingUsers", (it's only like this with tflocal state)

Although this is a bit hacky

@creikey
Copy link
Author

creikey commented Oct 3, 2024

image

Also, the problem with the current UX is that, if you run tflocal apply, during terraform's confirmation step, there is absolutely no mention of the previous state being lost at all, everything is green and it seems as if local stack infrastructure is all being provisioned as plan (when what has just happened is all of your state has just been lost, that was there before)

@creikey
Copy link
Author

creikey commented Oct 3, 2024

Alternatively, ideally, you actually just make it so tflocal doesn't use the terraform.tfstate file, using the -state=path argument, instead please make it named local_terraform.tfstate or something

@lakkeger
Copy link
Contributor

lakkeger commented Oct 3, 2024

To detect if a tfstate is managed by tflocal or not, it seems like the best bet would be to check for all zeros in the ARN:
"arn": "arn:aws:dynamodb:us-west-2:000000000000:table/TestingUsers", (it's only like this with tflocal state)

Although this is a bit hacky

You can set LocalStack to not deploy to the all 0 account so this is not a fool proof solution.

@lakkeger
Copy link
Contributor

lakkeger commented Oct 3, 2024

image

Also, the problem with the current UX is that, if you run tflocal apply, during terraform's confirmation step, there is absolutely no mention of the previous state being lost at all, everything is green and it seems as if local stack infrastructure is all being provisioned as plan (when what has just happened is all of your state has just been lost, that was there before)

Yes, this is the case as the state planned is done by terraform, terraform-local is not a fork of it but a wrapper if the plan is long, as I said the warning would roll out of your consoles buffer and you miss the message.
And since resources are not in LocalStack obviously you won't get the being removed differences, all this is how terraform behaves, not tflocal.

@lakkeger
Copy link
Contributor

lakkeger commented Oct 3, 2024

@lakkeger What do you mean? I'm suggesting this tool reads the tfstate it's about to modify, and if and only if the existing state points to non-tflocal resources, there is an explicit confirmation step.

Under none of the environments you listed, CI, running against local stack multiple times, etc does this warning get triggered, because in those cases, there are no non-tflocal resources in the state. Normal users are not bothered because they aren't prompted twice, because the resources are seen to be tflocal by this tool.

You can't tell just from the state if it's LocalStack or other resources being involved.

@lakkeger
Copy link
Contributor

lakkeger commented Oct 3, 2024

Alternatively, ideally, you actually just make it so tflocal doesn't use the terraform.tfstate file, using the -state=path argument, instead please make it named local_terraform.tfstate or something

With this one you are right though, I didn't consider this option, however with a configured remote backend the issue still persist.
This can work only for local states, only command parsing has to be added to the tool which currently is not a thing it does.
I'm not sure if it worth the effort though as ideally you are not storing states locally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants