Closed markwellis closed 6 months ago
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.
We have many terraform modules that modify the tailscale ACL
e.g.
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
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
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.
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 ~abuse~use case
I'm going to close this as i can use the import
block as a work around
reopening as import blocks cannot be used in modules, see https://github.com/hashicorp/terraform/issues/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
This shouldn't be necessary. See https://github.com/tailscale/terraform-provider-tailscale/issues/426 for details.
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
optionFixes #229
Special notes for your reviewer: It's my first time writing go, sorry if it's not best practice.