From e2a8f4220eac1cd3cd91d8a563cc139367e4e7e2 Mon Sep 17 00:00:00 2001
From: Mark McDonnell <Integralist@users.noreply.github.com>
Date: Thu, 9 Nov 2023 16:28:55 +0000
Subject: [PATCH] test(resources/servicevcl): validate versionless attributes
 (#81)

---
 .../resources/servicevcl/process_update.go    |  4 +-
 .../tests/resources/service_vcl_test.go       | 63 +++++++++++++++++++
 2 files changed, 66 insertions(+), 1 deletion(-)

diff --git a/internal/provider/resources/servicevcl/process_update.go b/internal/provider/resources/servicevcl/process_update.go
index b57c443..1ee529b 100644
--- a/internal/provider/resources/servicevcl/process_update.go
+++ b/internal/provider/resources/servicevcl/process_update.go
@@ -93,7 +93,9 @@ func (r *Resource) Update(ctx context.Context, req resource.UpdateRequest, resp
 	}
 
 	// NOTE: The service attributes (Name, Comment) are 'versionless'.
-	// So we update them once the service itself has been activated.
+	// In the old Terraform provider implementation we only updated if `activate`
+	// was set to `true` but it's unclear why as recent testing shows that it
+	// works regardless of whether the service is active or not.
 	err = updateServiceAttributes(ctx, plan, resp, api, state)
 	if err != nil {
 		return
diff --git a/internal/provider/tests/resources/service_vcl_test.go b/internal/provider/tests/resources/service_vcl_test.go
index 4b4b4bf..0c73d67 100644
--- a/internal/provider/tests/resources/service_vcl_test.go
+++ b/internal/provider/tests/resources/service_vcl_test.go
@@ -155,6 +155,69 @@ func TestAccResourceServiceVCLStandardBehaviours(t *testing.T) {
 	})
 }
 
+// The following test validates that versionless attributes can be updated even
+// when the service itself is inactive.
+func TestAccResourceServiceVCLInactiveVersionlessAttributeUpdates(t *testing.T) {
+	serviceName := fmt.Sprintf("tf-test-%s", acctest.RandString(10))
+	serviceNameUpdated := serviceName + "-updated"
+	serviceComment := "an updated service comment"
+	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,
+	})
+
+	// Update the service name and the comment from its default value.
+	configUpdate := fmt.Sprintf(`
+    resource "fastly_service_vcl" "test" {
+      activate = false
+      comment = "%s"
+      name = "%s"
+      force_destroy = true
+
+      domains = {
+        "example-1" = {
+          name = "%s"
+        },
+        "example-2" = {
+          name = "%s"
+        },
+      }
+    }
+    `, serviceComment, serviceNameUpdated, domain1Name, domain2Name)
+
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck:                 func() { provider.TestAccPreCheck(t) },
+		ProtoV6ProviderFactories: provider.TestAccProtoV6ProviderFactories,
+		Steps: []resource.TestStep{
+			// Create and Read testing
+			{
+				Config: configCreate,
+				Check: resource.ComposeAggregateTestCheckFunc(
+					resource.TestCheckResourceAttr("fastly_service_vcl.test", "activate", "false"),
+					resource.TestCheckResourceAttr("fastly_service_vcl.test", "comment", "Managed by Terraform"),
+					resource.TestCheckResourceAttr("fastly_service_vcl.test", "name", serviceName),
+				),
+			},
+			// Update and Read testing
+			{
+				Config: configUpdate,
+				Check: resource.ComposeAggregateTestCheckFunc(
+					resource.TestCheckResourceAttr("fastly_service_vcl.test", "activate", "false"),
+					resource.TestCheckResourceAttr("fastly_service_vcl.test", "comment", serviceComment),
+					resource.TestCheckResourceAttr("fastly_service_vcl.test", "name", serviceNameUpdated),
+				),
+			},
+			// Delete testing automatically occurs at the end of the 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) {