feat(resources/servicevcl): support importing a specific service vers…

…ion (#69)

* refactor: separate each test case into separate test functions

* ci: use latest golangci-lint

* test(resources/servicevcl): validate import of specific service version

* fix(resources/servicevcl): correctly handle import scenarios

* refactor(resources/servicevcl): reduce number of statements in read method

.github/workflows/test.yml

@@ -29,7 +29,7 @@ jobs:
       - name: Run linters
         uses: golangci/golangci-lint-action@v3
         with:
-          version: v1.54.2 # switch to "latest" once new release (post 1.55.0) is published.
+          version: latest
           args: "--verbose"
   generate:
     name: Generate

internal/provider/resources/servicevcl/process_read.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"net/http"
 
+	"github.com/fastly/fastly-go/fastly"
 	"github.com/hashicorp/terraform-plugin-framework/resource"
 	"github.com/hashicorp/terraform-plugin-framework/types"
 	"github.com/hashicorp/terraform-plugin-log/tflog"
@@ -53,26 +54,7 @@ func (r *Resource) Read(ctx context.Context, req resource.ReadRequest, resp *res
 		return
 	}
 
-	// NOTE: When importing a service there is no prior 'serviceVersion' in the state.
-	// So we presume the user wants to import the last active service serviceVersion.
-	// Which we retrieve from the GetServiceDetail call.
-	var (
-		foundActive    bool
-		serviceVersion int64
-	)
-	versions := clientResp.GetVersions()
-	for _, version := range versions {
-		if version.GetActive() {
-			serviceVersion = int64(version.GetNumber())
-			foundActive = true
-			break
-		}
-	}
-
-	if !foundActive {
-		// Use latest version if the user imports a service with no active versions.
-		serviceVersion = int64(versions[0].GetNumber())
-	}
+	serviceVersion := readServiceVersion(state, clientResp)
 
 	api := helpers.API{
 		Client:    r.client,
@@ -114,6 +96,45 @@ func (r *Resource) Read(ctx context.Context, req resource.ReadRequest, resp *res
 	tflog.Debug(ctx, "Read", map[string]any{"state": fmt.Sprintf("%#v", state)})
 }
 
+// readServiceVersion returns the service version.
+//
+// The returned value depends on if we're in an import scenario.
+//
+// When importing a service there might be no prior 'serviceVersion' in state.
+// If the user imports using the `ID@VERSION` syntax, then there will be.
+// This is because `ImportState()` in ./resource.go makes sure it's set.
+//
+// So we check if the attribute is null or not.
+//
+// If it's null, then we'll presume the user wants the last active version.
+// Which we retrieve from the GetServiceDetail call.
+// We fallback to the latest version if there is no prior active version.
+//
+// Otherwise we'll use whatever version they specified in their import.
+func readServiceVersion(state *models.ServiceVCL, clientResp *fastly.ServiceDetail) int64 {
+	var serviceVersion int64
+
+	if state.Version.IsNull() {
+		var foundActive bool
+		versions := clientResp.GetVersions()
+		for _, version := range versions {
+			if version.GetActive() {
+				serviceVersion = int64(version.GetNumber())
+				foundActive = true
+				break
+			}
+		}
+		if !foundActive {
+			// Use latest version if the user imports a service with no active versions.
+			serviceVersion = int64(versions[0].GetNumber())
+		}
+	} else {
+		serviceVersion = state.Version.ValueInt64()
+	}
+
+	return serviceVersion
+}
+
 func readSettings(ctx context.Context, state *models.ServiceVCL, resp *resource.ReadResponse, api helpers.API) error {
 	serviceID := state.ID.ValueString()
 	serviceVersion := int32(state.Version.ValueInt64())

internal/provider/resources/servicevcl/resource.go

@@ -4,6 +4,8 @@ import (
 	"context"
 	_ "embed"
 	"fmt"
+	"strconv"
+	"strings"
 
 	"github.com/fastly/fastly-go/fastly"
 	"github.com/hashicorp/terraform-plugin-framework-validators/resourcevalidator"
@@ -12,6 +14,7 @@ import (
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema"
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema/booldefault"
 	"github.com/hashicorp/terraform-plugin-framework/resource/schema/int64default"
+	"github.com/hashicorp/terraform-plugin-framework/types"
 	"github.com/hashicorp/terraform-plugin-go/tftypes"
 	"github.com/hashicorp/terraform-plugin-log/tflog"
 
@@ -139,16 +142,15 @@ func (r *Resource) Configure(_ context.Context, req resource.ConfigureRequest, r
 // The service resource then iterates over all nested resources populating the
 // state for each nested resource.
 func (r *Resource) ImportState(ctx context.Context, req resource.ImportStateRequest, resp *resource.ImportStateResponse) {
-	// TODO: req.ID needs to be checked for format.
-	// Typically just a Service ID but can also be <service id>@<service version>
-	// If the @<service_version> format is provided, then we need to parse the
-	// version and set it into the `version` attribute as well as `last_active`.
-
-	// The ImportStatePassthroughID() call is a small helper function that simply
-	// checks for an empty ID value passed (and errors accordingly) and if there
-	// is no error it calls `resp.State.SetAttribute()` passing in the ADDRESS
-	// (which we hardcode to the `id` attribute) and the user provided ID value.
-	resource.ImportStatePassthroughID(ctx, path.Root("id"), req, resp)
+	id, version, found := strings.Cut(req.ID, "@")
+	if found {
+		v, err := strconv.ParseInt(version, 10, 64)
+		if err != nil {
+			return
+		}
+		resp.Diagnostics.Append(resp.State.SetAttribute(ctx, path.Root("version"), types.Int64Value(v))...)
+	}
+	resp.Diagnostics.Append(resp.State.SetAttribute(ctx, path.Root("id"), id)...)
 
 	var state map[string]tftypes.Value
 	err := resp.State.Raw.As(&state)

internal/provider/tests/resources/service_vcl_test.go

@@ -14,30 +14,22 @@ import (
 	"github.com/integralist/terraform-provider-fastly-framework/internal/provider"
 )
 
+// The following test validates the standard service behaviours.
+// e.g. creating/updating the resource and nested resources.
 func TestAccResourceServiceVCL(t *testing.T) {
 	serviceName := fmt.Sprintf("tf-test-%s", acctest.RandString(10))
 	domain1Name := fmt.Sprintf("%s-tpff-1.integralist.co.uk", serviceName)
 	domain1CommentAdded := "a random updated comment"
 	domain2Name := fmt.Sprintf("%s-tpff-2.integralist.co.uk", serviceName)
 	domain2NameUpdated := fmt.Sprintf("%s-tpff-2-updated.integralist.co.uk", serviceName)
 
-	// Create a service with two domains.
-	// Also set `force_destroy = false`.
-	configCreate := fmt.Sprintf(`
-    resource "fastly_service_vcl" "test" {
-      name = "%s"
-      force_destroy = false
-
-      domains = {
-        "example-1" = {
-          name = "%s"
-        },
-        "example-2" = {
-          name = "%s"
-        },
-      }
-    }
-  `, serviceName, domain1Name, domain2Name)
+	configCreate := configServiceVCLCreate(configServiceVCLCreateOpts{
+		activate:     true,
+		forceDestroy: false,
+		serviceName:  serviceName,
+		domain1Name:  domain1Name,
+		domain2Name:  domain2Name,
+	})
 
 	// Update the first domain's comment + second domain's name (force_destroy = true).
 	// We also change the order of the domains so the second is now first.
@@ -155,9 +147,23 @@ func TestAccResourceServiceVCL(t *testing.T) {
 			// Delete testing automatically occurs in TestCase
 		},
 	})
+}
+
+// The following test validates the service deleted_at behaviour.
+// i.e. if deleted_at is not empty, then remove the service resource.
+func TestAccResourceServiceVCLDeletedAtCheck(t *testing.T) {
+	serviceName := fmt.Sprintf("tf-test-%s", acctest.RandString(10))
+	domain1Name := fmt.Sprintf("%s-tpff-1.integralist.co.uk", serviceName)
+	domain2Name := fmt.Sprintf("%s-tpff-2.integralist.co.uk", serviceName)
+
+	configCreate := configServiceVCLCreate(configServiceVCLCreateOpts{
+		activate:     true,
+		forceDestroy: true,
+		serviceName:  serviceName,
+		domain1Name:  domain1Name,
+		domain2Name:  domain2Name,
+	})
 
-	// NOTE: The following test validates the service deleted_at behaviour.
-	// i.e. if deleted_at is not empty, then remove the service resource.
 	resource.Test(t, resource.TestCase{
 		PreCheck:                 func() { provider.TestAccPreCheck(t) },
 		ProtoV6ProviderFactories: provider.TestAccProtoV6ProviderFactories,
@@ -172,37 +178,27 @@ func TestAccResourceServiceVCL(t *testing.T) {
 					resource.TestCheckResourceAttr("fastly_service_vcl.test", "domains.%", "2"),
 					resource.TestCheckResourceAttr("fastly_service_vcl.test", "domains.example-1.name", domain1Name),
 					resource.TestCheckResourceAttr("fastly_service_vcl.test", "domains.example-2.name", domain2Name),
-					resource.TestCheckResourceAttr("fastly_service_vcl.test", "force_destroy", "false"),
+					resource.TestCheckResourceAttr("fastly_service_vcl.test", "force_destroy", "true"),
 					resource.TestCheckResourceAttr("fastly_service_vcl.test", "stale_if_error", "false"),
 					resource.TestCheckResourceAttr("fastly_service_vcl.test", "stale_if_error_ttl", "43200"),
 					resource.TestCheckNoResourceAttr("fastly_service_vcl.test", "domains.example-1.comment"),
 					resource.TestCheckNoResourceAttr("fastly_service_vcl.test", "domains.example-2.comment"),
 				),
 			},
-			// Update and Read testing
-			{
-				Config: configUpdate,
-				Check: resource.ComposeAggregateTestCheckFunc(
-					resource.TestCheckResourceAttr("fastly_service_vcl.test", "force_destroy", "true"),
-					resource.TestCheckResourceAttr("fastly_service_vcl.test", "domains.example-1.comment", domain1CommentAdded),
-					resource.TestCheckResourceAttr("fastly_service_vcl.test", "domains.example-2.name", domain2NameUpdated),
-					resource.TestCheckNoResourceAttr("fastly_service_vcl.test", "domains.example-2.comment"),
-				),
-			},
 			// Trigger side-effect of deleting resource outside of Terraform.
 			// We use the same config as previous TestStep (so no config changes).
 			//
 			// Because Terraform executes a refresh/plan after each test case, we
 			// validate that the final plan is not empty using `ExpectNonEmptyPlan`.
 			{
-				Config: configUpdate,
+				Config: configCreate,
 				Check: resource.ComposeAggregateTestCheckFunc(
 					func(s *terraform.State) error {
 						if r, ok := s.RootModule().Resources["fastly_service_vcl.test"]; ok {
 							if id, ok := r.Primary.Attributes["id"]; ok {
 								apiClient := fastly.NewAPIClient(fastly.NewConfiguration())
 								ctx := fastly.NewAPIKeyContextFromEnv(helpers.APIKeyEnv)
-								version := int32(2)
+								version := int32(1)
 								deactivateReq := apiClient.VersionAPI.DeactivateServiceVersion(ctx, id, version)
 								_, httpResp, err := deactivateReq.Execute()
 								if err != nil {
@@ -225,29 +221,31 @@ func TestAccResourceServiceVCL(t *testing.T) {
 			// Delete testing automatically occurs in TestCase
 		},
 	})
+}
+
+// The following test validates the service type import behaviour.
+// i.e. when importing a service, check the service type matches the resource.
+// e.g. importing a Compute service ID into a VCL service resource.
+func TestAccResourceServiceVCLImportServiceTypeCheck(t *testing.T) {
+	serviceName := fmt.Sprintf("tf-test-%s", acctest.RandString(10))
+	domain1Name := fmt.Sprintf("%s-tpff-1.integralist.co.uk", serviceName)
+	domain2Name := fmt.Sprintf("%s-tpff-2.integralist.co.uk", serviceName)
+
+	configCreate := configServiceVCLCreate(configServiceVCLCreateOpts{
+		activate:     false,
+		forceDestroy: true,
+		serviceName:  serviceName,
+		domain1Name:  domain1Name,
+		domain2Name:  domain2Name,
+	})
 
-	// NOTE: The following test validates the service type import behaviour.
-	// i.e. when importing a service, check the service type matches the resource.
-	// e.g. importing a Compute service ID into a VCL service resource.
 	resource.Test(t, resource.TestCase{
 		PreCheck:                 func() { provider.TestAccPreCheck(t) },
 		ProtoV6ProviderFactories: provider.TestAccProtoV6ProviderFactories,
 		Steps: []resource.TestStep{
 			// We need a resource to be created by Terraform so we can import into it.
 			{
-				Config: fmt.Sprintf(`
-          resource "fastly_service_vcl" "test" {
-            name = "%s"
-            activate = false
-            force_destroy = true
-
-            domains = {
-              "example" = {
-                name = "%s"
-              },
-            }
-          }
-        `, serviceName, domain1Name),
+				Config: configCreate,
 			},
 			// ImportState testing
 			{
@@ -269,3 +267,89 @@ func TestAccResourceServiceVCL(t *testing.T) {
 		},
 	})
 }
+
+// The following test validates importing a specific service version.
+// e.g. terraform import fastly_service_vcl.test xxxxxxxxxxxxxxxxxxxx@2.
+func TestAccResourceServiceVCLImportServiceVersion(t *testing.T) {
+	serviceName := fmt.Sprintf("tf-test-%s", acctest.RandString(10))
+	domain1Name := fmt.Sprintf("%s-tpff-1.integralist.co.uk", serviceName)
+	domain2Name := fmt.Sprintf("%s-tpff-2.integralist.co.uk", serviceName)
+
+	configCreate := configServiceVCLCreate(configServiceVCLCreateOpts{
+		activate:     true,
+		forceDestroy: true,
+		serviceName:  serviceName,
+		domain1Name:  domain1Name,
+		domain2Name:  domain2Name,
+	})
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:                 func() { provider.TestAccPreCheck(t) },
+		ProtoV6ProviderFactories: provider.TestAccProtoV6ProviderFactories,
+		Steps: []resource.TestStep{
+			// We need a resource to be created by Terraform so we can import into it.
+			{
+				Config: configCreate,
+			},
+			// Clone the service version and return the import ID to use.
+			{
+				ResourceName: "fastly_service_vcl.test",
+				ImportState:  true,
+				ImportStateIdFunc: func(s *terraform.State) (string, error) {
+					if r, ok := s.RootModule().Resources["fastly_service_vcl.test"]; ok {
+						if id, ok := r.Primary.Attributes["id"]; ok {
+							apiClient := fastly.NewAPIClient(fastly.NewConfiguration())
+							ctx := fastly.NewAPIKeyContextFromEnv(helpers.APIKeyEnv)
+							req := apiClient.VersionAPI.CloneServiceVersion(ctx, id, 1)
+							_, _, err := req.Execute()
+							if err != nil {
+								return "", fmt.Errorf("failed to clone service version: %w", err)
+							}
+							return id + "@2", nil
+						}
+					}
+					return "", nil
+				},
+				ImportStateCheck: func(is []*terraform.InstanceState) error {
+					for _, s := range is {
+						serviceVersion := s.Attributes["version"]
+						if serviceVersion != "2" {
+							return fmt.Errorf("import failed: unexpected service version found: got %s, want 2", serviceVersion)
+						}
+					}
+					return nil
+				},
+			},
+			// Delete testing automatically occurs in TestCase
+		},
+	})
+}
+
+type configServiceVCLCreateOpts struct {
+	activate, forceDestroy                bool
+	serviceName, domain1Name, domain2Name string
+}
+
+// configServiceVCLCreate returns a TF config that consists of a VCL service
+// with two domains + configurable `activate` and `force_destroy` attributes.
+//
+// NOTE: We use this config for a lot of the tests.
+// But occasionally we need to tweak the force_destroy attribute.
+func configServiceVCLCreate(opts configServiceVCLCreateOpts) string {
+	return fmt.Sprintf(`
+    resource "fastly_service_vcl" "test" {
+      activate = %t
+      force_destroy = %t
+      name = "%s"
+
+      domains = {
+        "example-1" = {
+          name = "%s"
+        },
+        "example-2" = {
+          name = "%s"
+        },
+      }
+    }
+  `, opts.activate, opts.forceDestroy, opts.serviceName, opts.domain1Name, opts.domain2Name)
+}