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 import logic for remaining resources #445

Merged
merged 1 commit into from
Oct 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions docs/resources/logstream_configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,12 @@ resource "tailscale_logstream_configuration" "sample_logstream_configuration" {
### Read-Only

- `id` (String) The ID of this resource.

## Import

Import is supported using the following syntax:

```shell
# Logstream configuration can be imported using the logstream configuration id, e.g.,
terraform import tailscale_logstream_configuration.sample_logstream_configuration 123456789
```
9 changes: 9 additions & 0 deletions docs/resources/posture_integration.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,12 @@ resource "tailscale_posture_integration" "sample_posture_integration" {
### Read-Only

- `id` (String) The ID of this resource.

## Import

Import is supported using the following syntax:

```shell
# Posture integration can be imported using the posture integration id, e.g.,
terraform import tailscale_posture_integration.sample_posture_integration 123456789
```
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# Logstream configuration can be imported using the logstream configuration id, e.g.,
terraform import tailscale_logstream_configuration.sample_logstream_configuration 123456789
2 changes: 2 additions & 0 deletions examples/resources/tailscale_posture_integration/import.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# Posture integration can be imported using the posture integration id, e.g.,
terraform import tailscale_posture_integration.sample_posture_integration 123456789
5 changes: 5 additions & 0 deletions tailscale/resource_device_key_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,11 @@ func TestAccTailscaleDeviceKey(t *testing.T) {
resource.TestCheckResourceAttr(resourceName, "key_expiry_disabled", "true"),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
},
})
}
15 changes: 12 additions & 3 deletions tailscale/resource_device_subnet_routes.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,17 @@ func resourceDeviceSubnetRoutes() *schema.Resource {
UpdateContext: resourceDeviceSubnetRoutesUpdate,
DeleteContext: resourceDeviceSubnetRoutesDelete,
Importer: &schema.ResourceImporter{
StateContext: schema.ImportStatePassthroughContext,
StateContext: func(ctx context.Context, d *schema.ResourceData, meta any) ([]*schema.ResourceData, error) {
// We can't do a simple passthrough here as the ID used for this resource is a
// randomly generated UUID and we need to instead fetch based on the device_id.
//
// TODO(mpminardi): investigate changing the ID in state to be the device_id instead
// in an eventual major version bump.
d.Set("device_id", d.Id())
d.SetId(createUUID())
Copy link
Contributor

@reegnz reegnz Oct 9, 2024

Choose a reason for hiding this comment

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

Interesting.
What were the issues with just using plain device id? I didn't experience any problems with importing the subnet just by passthrough.
This logic seems a bit unnecessary but maybe I'm not understanding something in this context.

The UUID creation is also present in the other device resources as well, so I'm not following why it has to be UUID for this particular resource but not the other device resources.

I mean I get why it has to be UUID when creating the resource, but when importing it can straight be the device_id, nothing assumes this has to be UUID tbqh.

Anyway, still works, was just curious to see this changed for only device_subnet, and not for device_key, device_tags, device_authorization.

Maybe it needs to change for those as well?
@mpminardi

EDIT: I think the other resources I broke backward compatibility with read, you probably need to change device_key, device_tags, device_authorization in a similar manner.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hey @reegnz!

The rest of the device* resources are using device_id as the id in state upon resource creation (and were before the import changes), so in those cases the passthrough can be used and works as expected (device_key, device_tags, and device_authorization for reference).

For this resource the change here would end up breaking existing users of the resource as the Id of existing device_subnet resources are a UUID in state, meaning that we would try and fail to fetch the device using the UUID instead of the device Id for non-imported resources.

This is why we need to do the more complex import logic in this case: so that we can properly read both when doing the import and when doing a read operation for an existing resource. Doing a fresh import would not have surfaced this issue as the read would be using the passthrough Id (e.g., the device_id) and work properly for that scenario.

This resource should also use the device_id as its id, but since we have released versions where it uses a UUID instead we need to keep this in UUID format to stay within what we can release without doing a major version revision as per the HashiCorp versioning best practices.

Apologies as well for not catching this on the original review! I had forgotten that this resource was using a UUID for its Id and completely missed that this would cause issues originally.


return []*schema.ResourceData{d}, nil
},
},
Schema: map[string]*schema.Schema{
"device_id": {
Expand All @@ -49,14 +59,13 @@ func resourceDeviceSubnetRoutes() *schema.Resource {

func resourceDeviceSubnetRoutesRead(ctx context.Context, d *schema.ResourceData, m interface{}) diag.Diagnostics {
client := m.(*tsclient.Client)
deviceID := d.Id()
deviceID := d.Get("device_id").(string)

routes, err := client.Devices().SubnetRoutes(ctx, deviceID)
if err != nil {
return diagnosticsError(err, "Failed to fetch device subnet routes")
}

d.Set("device_id", deviceID)
if err = d.Set("routes", routes.Enabled); err != nil {
return diag.FromErr(err)
}
Expand Down
63 changes: 63 additions & 0 deletions tailscale/resource_device_subnet_routes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"fmt"
"os"
"reflect"
"strings"
"testing"

"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
Expand Down Expand Up @@ -62,6 +63,7 @@ func TestAccTailscaleDeviceSubnetRoutes(t *testing.T) {
}
}

var deviceId string
resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
ProviderFactories: testAccProviderFactories(t),
Expand All @@ -84,6 +86,67 @@ func TestAccTailscaleDeviceSubnetRoutes(t *testing.T) {
resource.TestCheckTypeSetElemAttr(resourceName, "routes.*", "2.0.0.0/24"),
),
},
{
ResourceName: resourceName,
ImportState: true,
// Need import state ID func to dynamically grab device_id for import.
ImportStateIdFunc: func(state *terraform.State) (string, error) {
rs, ok := state.RootModule().Resources[resourceName]
if !ok {
return "", fmt.Errorf("resource not found: %s", resourceName)

}

deviceId = rs.Primary.Attributes["device_id"]

return deviceId, nil
},
// Need a custom import state check due to the fact that the ID for this
// resource is re-generated on import.
ImportStateCheck: func(states []*terraform.InstanceState) error {
if len(states) != 1 {
return fmt.Errorf("expected 1 state: %+v", states)
}

rs := states[0]
elemCheck := func(attr string, value string) bool {
attrParts := strings.Split(attr, ".")
for stateKey, stateValue := range rs.Attributes {
if stateValue == value {
stateKeyParts := strings.Split(stateKey, ".")
if len(stateKeyParts) == len(attrParts) {
for i := range attrParts {
if attrParts[i] != stateKeyParts[i] && attrParts[i] != "*" {
break
}
if i == len(attrParts)-1 {
return true
}
}
}
}
}

return false
}

if rs.Attributes["device_id"] != deviceId {
return fmt.Errorf("expected device_id to be %q but was: %q", deviceId, rs.Attributes["device_id"])
}

if !elemCheck("routes.*", "10.0.1.0/24") {
return fmt.Errorf("expected routes to contain '10.0.1.0/24': %#v", rs.Attributes)
}
if !elemCheck("routes.*", "1.2.0.0/16") {
return fmt.Errorf("expected routes to contain '1.2.0.0/16': %#v", rs.Attributes)
}
if !elemCheck("routes.*", "2.0.0.0/24") {
return fmt.Errorf("expected routes to contain '2.0.0.0/24': %#v", rs.Attributes)
}

return nil
},
},
},
})
}
5 changes: 5 additions & 0 deletions tailscale/resource_device_tags_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,11 @@ func TestAccTailscaleDeviceTags(t *testing.T) {
resource.TestCheckTypeSetElemAttr(resourceName, "tags.*", "tag:c"),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
},
})
}
5 changes: 5 additions & 0 deletions tailscale/resource_dns_nameservers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,11 @@ func TestAccTailscaleDNSNameservers(t *testing.T) {
resource.TestCheckTypeSetElemAttr(resourceName, "nameservers.*", "1.1.1.1"),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
},
})
}
5 changes: 5 additions & 0 deletions tailscale/resource_dns_search_paths_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,11 @@ func TestAccTailscaleDNSSearchPaths(t *testing.T) {
resource.TestCheckTypeSetElemAttr(resourceName, "search_paths.*", "example.com"),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
},
})
}
3 changes: 3 additions & 0 deletions tailscale/resource_logstream_configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ func resourceLogstreamConfiguration() *schema.Resource {
CreateContext: resourceLogstreamConfigurationCreate,
UpdateContext: resourceLogstreamUpdate,
DeleteContext: resourceLogstreamDelete,
Importer: &schema.ResourceImporter{
StateContext: schema.ImportStatePassthroughContext,
},
Schema: map[string]*schema.Schema{
"log_type": {
Type: schema.TypeString,
Expand Down
6 changes: 6 additions & 0 deletions tailscale/resource_logstream_configuration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,12 @@ func TestAccTailscaleLogstreamConfiguration_basic(t *testing.T) {
resource.TestCheckResourceAttr(resourceName, "token", "some-token"),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"token"},
},
},
})
}
3 changes: 3 additions & 0 deletions tailscale/resource_posture_integration.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ func resourcePostureIntegration() *schema.Resource {
CreateContext: resourcePostureIntegrationCreate,
UpdateContext: resourcePostureIntegrationUpdate,
DeleteContext: resourcePostureIntegrationDelete,
Importer: &schema.ResourceImporter{
StateContext: schema.ImportStatePassthroughContext,
},
Schema: map[string]*schema.Schema{
"posture_provider": {
Type: schema.TypeString,
Expand Down
6 changes: 6 additions & 0 deletions tailscale/resource_posture_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,12 @@ func TestAccTailscalePostureIntegration(t *testing.T) {
resource.TestCheckResourceAttr(resourceName, "client_secret", "test-secret3"),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"client_secret"},
},
},
})
}
Loading