From bd9bdad3666b8d9553efa0c2310cb5483a80d25e Mon Sep 17 00:00:00 2001
From: Julien Duchesne <julien.duchesne@grafana.com>
Date: Fri, 19 Apr 2024 11:36:12 -0400
Subject: [PATCH] `grafana_service_account`: Fix updates on v11 (#1503)

Issue: https://github.com/grafana/terraform-provider-grafana/issues/1493
Seems to be a regression in Grafana 11 where not passing the Role in an SA update results in a 500 error
This should fix it, we can pass all the fields all the time in TF
---
 .../grafana/resource_service_account.go       | 13 ++-----
 .../grafana/resource_service_account_test.go  | 38 +++++++++++++++----
 2 files changed, 35 insertions(+), 16 deletions(-)

diff --git a/internal/resources/grafana/resource_service_account.go b/internal/resources/grafana/resource_service_account.go
index 8fc34a78d..700a7970d 100644
--- a/internal/resources/grafana/resource_service_account.go
+++ b/internal/resources/grafana/resource_service_account.go
@@ -132,15 +132,10 @@ func UpdateServiceAccount(ctx context.Context, d *schema.ResourceData, meta inte
 		return diag.FromErr(err)
 	}
 
-	updateRequest := models.UpdateServiceAccountForm{}
-	if d.HasChange("name") {
-		updateRequest.Name = d.Get("name").(string)
-	}
-	if d.HasChange("role") {
-		updateRequest.Role = d.Get("role").(string)
-	}
-	if d.HasChange("is_disabled") {
-		updateRequest.IsDisabled = d.Get("is_disabled").(bool)
+	updateRequest := models.UpdateServiceAccountForm{
+		Name:       d.Get("name").(string),
+		Role:       d.Get("role").(string),
+		IsDisabled: d.Get("is_disabled").(bool),
 	}
 
 	params := service_accounts.NewUpdateServiceAccountParams().
diff --git a/internal/resources/grafana/resource_service_account_test.go b/internal/resources/grafana/resource_service_account_test.go
index f14be426d..a377a4d06 100644
--- a/internal/resources/grafana/resource_service_account_test.go
+++ b/internal/resources/grafana/resource_service_account_test.go
@@ -26,7 +26,7 @@ func TestAccServiceAccount_basic(t *testing.T) {
 		CheckDestroy:             serviceAccountCheckExists.destroyed(&updatedSA, nil),
 		Steps: []resource.TestStep{
 			{
-				Config: testServiceAccountConfig(name, "Editor"),
+				Config: testServiceAccountConfig(name, "Editor", false),
 				Check: resource.ComposeTestCheckFunc(
 					serviceAccountCheckExists.exists("grafana_service_account.test", &sa),
 					resource.TestCheckResourceAttr("grafana_service_account.test", "name", name),
@@ -38,7 +38,7 @@ func TestAccServiceAccount_basic(t *testing.T) {
 			},
 			// Change the name. Check that the ID stays the same.
 			{
-				Config: testServiceAccountConfig(name+"-updated", "Editor"),
+				Config: testServiceAccountConfig(name+"-updated", "Editor", false),
 				Check: resource.ComposeTestCheckFunc(
 					serviceAccountCheckExists.exists("grafana_service_account.test", &updatedSA),
 					func(s *terraform.State) error {
@@ -54,6 +54,12 @@ func TestAccServiceAccount_basic(t *testing.T) {
 					resource.TestMatchResourceAttr("grafana_service_account.test", "id", defaultOrgIDRegexp),
 				),
 			},
+			// Import test
+			{
+				ResourceName:      "grafana_service_account.test",
+				ImportState:       true,
+				ImportStateVerify: true,
+			},
 		},
 	})
 }
@@ -69,7 +75,7 @@ func TestAccServiceAccount_NoneRole(t *testing.T) {
 		CheckDestroy:             serviceAccountCheckExists.destroyed(&sa, nil),
 		Steps: []resource.TestStep{
 			{
-				Config: testServiceAccountConfig(name, "None"),
+				Config: testServiceAccountConfig(name, "None", false),
 				Check: resource.ComposeTestCheckFunc(
 					serviceAccountCheckExists.exists("grafana_service_account.test", &sa),
 					resource.TestCheckResourceAttr("grafana_service_account.test", "name", name),
@@ -79,6 +85,24 @@ func TestAccServiceAccount_NoneRole(t *testing.T) {
 					resource.TestMatchResourceAttr("grafana_service_account.test", "id", defaultOrgIDRegexp),
 				),
 			},
+			// Disable the SA
+			{
+				Config: testServiceAccountConfig(name, "None", true),
+				Check: resource.ComposeTestCheckFunc(
+					serviceAccountCheckExists.exists("grafana_service_account.test", &sa),
+					resource.TestCheckResourceAttr("grafana_service_account.test", "name", name),
+					resource.TestCheckResourceAttr("grafana_service_account.test", "org_id", "1"),
+					resource.TestCheckResourceAttr("grafana_service_account.test", "role", "None"),
+					resource.TestCheckResourceAttr("grafana_service_account.test", "is_disabled", "true"),
+					resource.TestMatchResourceAttr("grafana_service_account.test", "id", defaultOrgIDRegexp),
+				),
+			},
+			// Import test
+			{
+				ResourceName:      "grafana_service_account.test",
+				ImportState:       true,
+				ImportStateVerify: true,
+			},
 		},
 	})
 }
@@ -121,7 +145,7 @@ func TestAccServiceAccount_invalid_role(t *testing.T) {
 		Steps: []resource.TestStep{
 			{
 				ExpectError: regexp.MustCompile(`.*expected role to be one of \[.+\], got InvalidRole`),
-				Config:      testServiceAccountConfig("any", "InvalidRole"),
+				Config:      testServiceAccountConfig("any", "InvalidRole", false),
 			},
 		},
 	})
@@ -143,11 +167,11 @@ func testManyServiceAccountsConfig(prefix string, count int) string {
 	return config
 }
 
-func testServiceAccountConfig(name, role string) string {
+func testServiceAccountConfig(name, role string, disabled bool) string {
 	return fmt.Sprintf(`
 resource "grafana_service_account" "test" {
 	name        = "%[1]s"
 	role        = "%[2]s"
-	is_disabled = false
-}`, name, role)
+	is_disabled = %[3]t
+}`, name, role, disabled)
 }