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

tailscale: add dns_split_nameservers resource #359

Merged
merged 1 commit into from
Apr 30, 2024

Conversation

mpminardi
Copy link
Contributor

Add resource_dns_split_nameservers to allow for controlling split DNS settings for a given tailnet.

Updates https://github.com/tailscale/corp/issues/19483

@mpminardi mpminardi self-assigned this Apr 24, 2024
@mpminardi mpminardi force-pushed the mpminardi/split-dns-resource branch from b7e65d1 to 8cb204b Compare April 24, 2024 18:29
go.mod Outdated Show resolved Hide resolved
tailscale/resource_dns_split_nameservers.go Show resolved Hide resolved
return diagnosticsError(err, "Failed to fetch split DNS configs")
}

nameservers := splitDNS[d.Get("domain").(string)]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might need something like this to consider the resource deleted when there are no split DNS servers configured for a given domain.

Suggested change
nameservers := splitDNS[d.Get("domain").(string)]
nameservers, ok := splitDNS[d.Get("domain").(string)]
if !ok {
d.SetId("")
return
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Digging into this a bit more but making this change was breaking import.

Copy link
Contributor Author

@mpminardi mpminardi Apr 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ended up changing to use d.Id() as this is what was breaking the import and is mentioned as necessary here if using schema.ImportStatePassthrough.

This seems to operate fine without the ok check in the scenarios I've tested (namely creation, deletion, import, update of split DNS nameservers) so I've left it out for now. The endpoint this is hitting also should not return nil values for map entries.

tailscale/resource_dns_split_nameservers.go Outdated Show resolved Hide resolved
@mpminardi mpminardi force-pushed the mpminardi/split-dns-resource branch 3 times, most recently from f43a9ea to 9999944 Compare April 26, 2024 19:38
@mpminardi mpminardi marked this pull request as ready for review April 26, 2024 21:10
@mpminardi mpminardi force-pushed the mpminardi/split-dns-resource branch 3 times, most recently from 9c4087f to 310da05 Compare April 29, 2024 20:56
@mpminardi mpminardi force-pushed the mpminardi/split-dns-resource branch from 310da05 to 856bf8d Compare April 29, 2024 21:37
@aiell0 aiell0 mentioned this pull request Apr 30, 2024
@aiell0
Copy link

aiell0 commented Apr 30, 2024

Just chiming in to say I love using Tailscale and this is a feature I have been anticipating for a while! Good work @mpminardi

Add `resource_dns_split_nameservers` to allow for controlling split DNS
settings for a given tailnet.

Updates tailscale/corp#19483

Signed-off-by: Mario Minardi <mario@tailscale.com>
@mpminardi mpminardi force-pushed the mpminardi/split-dns-resource branch from 856bf8d to 50149d2 Compare April 30, 2024 16:10
@mpminardi mpminardi merged commit 79561c1 into main Apr 30, 2024
3 checks passed
@mpminardi mpminardi deleted the mpminardi/split-dns-resource branch April 30, 2024 16:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants