added allow_overwrite option to resource_acl

[markwellis]

so resource_acl doesn't need to be imported first, as this is a manual task that breaks our workflow.

by default it is false, so will require import unless you read the docs and learn about the allow_overwite option

Fixes #229

Special notes for your reviewer:
It's my first time writing go, sorry if it's not best practice.

[knyar]

Thank you for preparing a PR for this. Before adding more options, I'd like to understand what problem this solves and make sure this is something that other users will find helpful as well. Could you please share a bit more on why this is necessary?

resource_acl should support terraform import, so if you have a non-default policy, you should be able to import it before changing. For tailnets that never had their ACL changed from the default, this check should pass and allow setting the policy even without importing.

[markwellis]

We have many terraform modules that modify the tailscale ACL

e.g.

• a module for our aws org to create new aws accounts. This adds tags and groups to the tailscale ACL
• a module to create a VPC. This adds hosts to the tailscale ACL
• modules for RDS etc that add ACLs for access

All of these fail every time as we always forget to first import the ACL. It's annoying, and adding this as an option will streamline the process for us massively, whilst preserving the current behaviour of it not being overwritten by default.

It's also a little strange that this resource requires importing before it can be controlled with terraform, I can't think of a single other thing that works that way. I can see why you've done it, but for advanced users it's more of a hindrance than a help and having a way to opt out seems like a reasonable work around

Hope that clears it up, happy to provide any info :)

There is also #196 which relates to this

[markwellis]

I just realised with import blocks added in tf 1.5 there is a work around for this already

data "tailscale_acl" "acl" {
}

locals {
  acl = jsondecode(data.tailscale_acl.acl.json)
}

import {
  to = tailscale_acl.acl
  id = "acl"
}
resource "tailscale_acl" "acl" {
  acl = jsonencode(merge(local.acl, {groups = merge(local.acl.groups, {"group:foo" = ["${plantimestamp()}@example.com"]})}))
}

I don't feel this is as nice as having allow_overwrite but it does work.

So with that said, there is a solution to the initial problem we were facing without needing to add more options

[knyar]

We have many terraform modules that modify the tailscale ACL

Sorry, but I still don't quite understand how this works. Terraform is designed around the idea of each Terraform resource "owning" management of a particular cloud resource, keeping local state. If you have multiple resources in multiple modules pointing to the same tailnet's ACL, won't they keep overwriting each other's changes? This seems like a use case for an automation tool that's imperative in nature (making individual changes) rather than declarative (converging on desired state) like Terraform is.

[markwellis]

Yes it is a little strange, but the way we do it is to manage parts of the ACL in places, by loading the existing ACL, and then updating certain groups etc. It is riddled with race conditions, but it works better for us than having one place that manages the ACL.

What would be best would be if the tailscale API allowed us to mange groups/acls/hosts etc individually, rather than as a single json document. But there's nothing I can do about that

E.g. what we do is add a host for a new VPC's network, a new group with the VPC name in, and an ACL that allows the group access to the host. Doing that when we create the VPC is neater for us than doing it in a single place. It means the ACL for accessing the VPC lives with the VPC terraform itself, rather than somewhere else. Even though this isn't how the tailscale API actually works

it would be awesome if we could do something like

resource "aws_vpc" "example" {
  ...
}

resource "tailscale_acl_host" "foo" {
  name = aws_vpc.example.tags.name
  ip_address = aws_vpc.example.cidr_block
}

resource "tailscale_acl_group" "foo"  {
  name = aws_vpc.example.tags.name
  members = [] #managed in the tailscale admin or scim in the future
}

resource "tailscale_acl_acl" "foo" {
  action = "accept"
  src = tailscale_acl_group.foo.name
  dst = tailscale_acl_host.foo.name
}

but we can't so instead we do something like

resource "aws_vpc" "example" {
  ...
}

data "tailscale_acl" "acl" {
}

locals {
  acl = jsondecode(data.tailscale_acl.acl.json)
}

import {
  to = tailscale_acl.acl
  id = "acl"
}
resource "tailscale_acl" "acl" {
  acl = jsonencode(
    merge(local.acl, {
      groups = merge(local.acl.groups, {"group:foo" = []}),
      hosts = merge(local.acl.hosts, {"hosts": {
        aws_vpc.example.tags.name  = aws_vpc.example.cidr_block
      }},
....
    })
  )
}

where we abuse merge to overwrite just parts of the json. it works, but it's not great, but is still better for us than managing the acl in one place

hope this clears up our abuseuse case

[markwellis]

I'm going to close this as i can use the import block as a work around

[markwellis]

reopening as import blocks cannot be used in modules, see hashicorp/terraform#33474

Because of that issue I would still like to be able to set allow_overwrite on the tailscale_acl resource to not require importing it before terraform can manage the ACL

[knyar]

Thank you for your patience here. Just a couple comments from me.

[knyar]

Thank you!

[colans]

This shouldn't be necessary. See #426 for details.