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.

Sign up for GitHub

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

Jump to bottom

resource_device_authorization: update delete #160

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

resource_device_authorization: update delete #160

wants to merge 1 commit into from

Conversation

defo89
Copy link

@defo89 defo89 commented Oct 10, 2022

deletes the device on authorization resouce deletion

What this PR does / why we need it: Ability to delete device with provider

Which issue this PR fixes: fixes #68

@defo89
resource_device_authorization: update delete
2a0eaeb 
deletes the device on authorization resouce deletion
@defo89
Copy link
Author

defo89 commented Oct 10, 2022

Worth to mention: if multiple resources are created/destroyed in a single run, users may face an error that some resources failed to be destroyed (due to device being removed already).

data "tailscale_device" "device1" {
  name     = "device1"
  wait_for = "60s"
}

resource "tailscale_device_authorization" "authorization" {
  device_id  = data.tailscale_device.device1.id
  authorized = true
}

resource "tailscale_device_key" "key" {
  device_id           = data.tailscale_device.device1.id
  key_expiry_disabled = true
}

resource "tailscale_device_subnet_routes" "routes" {
  device_id = data.tailscale_device.device1.id
  routes = local.routes
}

› terraform destroy -auto-approve
---snip---
Error: Failed to set device subnet routes
internal server error (500)

Error: failed to update device key
internal server error (500)

User will have to make sure that device deletion happens last.

davidsbond
davidsbond reviewed Oct 10, 2022
View reviewed changes
Copy link
Collaborator

@davidsbond davidsbond left a comment

Choose a reason for hiding this comment

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

I'm still of the opinion that if the terraform provider is not in charge of creating devices then it seems like not the right place for deleting them.

However, should this PR be accepted we should probably update the documentation for this resource as part of this pull request to inform users of the behaviour if they delete the device_authorization resource.

IMO we should modify the description on line 14 to include this information, you'll also need to run go generate ./... after you modify it.

@davidsbond
Copy link
Collaborator

if multiple resources are created/destroyed in a single run, users may face an error that some resources failed to be destroyed (due to device being removed already).

This is part of the reason I don't like us managing deletion without creation, the UX suffers in a non-obvious way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davidsbond davidsbond davidsbond left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

FR: add ability to delete devices
2 participants
@defo89 @davidsbond