Destroying ACL configuration does not restore default configuration

[colans]

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:

• Failed to set ACL #229
• ACL behavior in integration tests #196

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]

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.

[timduhenchanter]

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.

Is the provider on any roadmaps internally as of yet? The terraform provider could use some TLC and would be great since there is such a large userbase for Terraform and operations love to homogenize their tooling as much as possible.

[mpminardi]

@colans @timduhenchanter we've got time blocked off internally to work on the provider and address some outstanding issues (this one included!) in February.