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

Destroying ACL configuration does not restore default configuration #426

Open
colans opened this issue Sep 4, 2024 · 1 comment
Open
Labels
bug Something isn't working

Comments

@colans
Copy link

colans commented Sep 4, 2024

Describe the bug

If managing ACLs via a tailscale_acl resource in Terraform that was created and destroyed at least once, terraform apply fails with:

You seem to be trying to overwrite a non-default ACL with a tailscale_acl resource. Before doing this, please import your existing ACL into Terraform state using: terraform import $(this_resource) acl

But the non-default configuration should have been removed when terraform destroy was last run so there shouldn't be any non-default configuration anymore. So there's no reason to stop the next build.

To Reproduce

Steps to reproduce the behaviour:

  1. Add some ACL configuration to a tailscale_acl resource.
  2. Run terraform apply
  3. Run terraform destroy
  4. Run terraform apply again.
  5. The build fails with the above error, forcing users to run the import command before continuing with it (unless they use the overwrite_existing_content workaround, which shouldn't be necessary).

Expected behaviour

The build does not fail with the above error.

To make this happen, terraform destroy should remove any custom configuration (this isn't happening now), and restore defaults. So the next time apply is run, it will only see defaults that can be overwritten (allowing the build to proceed).

Desktop (please complete the following information):

  • OS: Pop!_OS (Ubuntu) 22.04 LTS
  • Terraform: v1.9.5 on linux_amd64
  • Provider Version: v0.16.2

Additional context

The root cause of this problem is an incomplete resolution to #182. It was a good idea to check for non-default configuration before apply, but non-default configuration must be destroyed (i.e. defaults restored) on destroy for this to work properly. Basically, the PR (#186) should not have been merged without also doing this. Let's do it now to finally get this working properly.

Leaving this unresolved is causing other problems:

A "fix" for this was merged in #303, but that's treating a symptom of breaking the spirit of how Terraform is supposed to work. It does not resolve the root cause. It shouldn't be necessary for such workarounds if destroy works properly.

CC @markwellis @knyar @DentonGentry @AaronFriel @timduhenchanter

@mpminardi
Copy link
Member

Thanks for flagging this @colans !

Agree that this ends up being an awkward and unexpected flow when managing the ACL via Terraform.

I think in the short term we can potentially add an optional reset_acl_on_destroy (or something similar) property to reset to default on destroy. This is a bit of an awkward workaround, but would allow us to support this behaviour without unexpectedly breaking users who are now accustomed to the existing behaviour.

When we do a major version bump of the provider I think having the destroy behaviour reset the ACL to the default and removing reset_acl_on_destroy as an option would make sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants