tailscale/terraform-provider-tailscale

tailscale_acl creation deletes already present ACL without any warning

stephan242 opened this issue · 2 comments

stephan242 commented

Describe the bug
If a new tailscale_acl resource is getting created, it will delete and override an already existing ACL. This is the worst possible behaviour imaginable for a resource and can mess people up big time, especially since the Tailscale WebUI doesn't offer revision control of the ACL to restore previous ACLs. I would even suggest to pull this resource from circulation in an emergency release until this issue is fixed, since it is dangerous and doesn't heed the most basic concepts of a terraform resource one would expect. A "create" action should never ever override existing items.

So 2 things: If this resource continues to override the ACL wholesale then it should only do so after a successful (so far apparently unimplemented) import of the resource, and otherwise error. However, such a resource will be pretty much unusable for all but the most simple use cases, since usually all the Subnet Routers will live in different VPCs of different projects, and will be implemented individually as part of these projects. Resources aren't separated by provider in Terraform, but by organisational structures within a company.

Therefore the proper fix would be to make this resource merge its policy items into an already existing ACL.

To Reproduce
Steps to reproduce the behaviour:

  1. Create an ACL in tailscale WebUI manually
  2. Create a different tailscale_acl via terraform, and apply it against the same tailnet.
  3. The manually created ACL will get overridden

Expected behaviour
Never ever delete existing infrastructure/config without a lot of warning!

Desktop (please complete the following information):

  • OS: Linux
  • Terraform Version 1.2.9
  • Provider Version 0.13.5
knyar commented

Thank you for reporting this! I don't think there is a particularly good solution here, since Terraform is designed to make imperative APIs configurable in a declarative way, while Tailscale ACL is already a declarative API, which breaks some built-in assumptions in Terraform. Managing singleton resources that get created automatically for all users seems to be a well known pain point.

I have a few improvements in mind that will hopefully make this easier to use:

  1. Document that the tailscale_acl resource will completely override the ACL file contents.
  2. Implement terraform import for the ACL resource.
  3. Make resource creation fail if ACL contents for the given tailnet has ever been modified.

As a result, users who never modified the default ACL will be able to simply create a new one via Terraform, while those who have made changes manually in the UI will need to explicitly import tailscale_acl into their Terraform state first.

colans commented

Thee fix for this was incomplete. See #426 for details.