fix: google_compute_instance with user-managed service account and empty scopes results in no service account assignment (#10358) (#18521)

[upstream:a00db19f538bab3f8f8d42b35d033a7703e19e18]

Signed-off-by: Modular Magician <magic-modules@google.com>

Author: modular-magician

.changelog/10358.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+compute: fixed google_compute_instance with `service_account.email` but no `service_account.scopes`
+```

google/services/compute/resource_compute_instance.go

@@ -2252,7 +2252,7 @@ func resourceComputeInstanceUpdate(d *schema.ResourceData, meta interface{}) err
 		if d.HasChange("service_account.0.email") || scopesChange {
 			sa := d.Get("service_account").([]interface{})
 			req := &compute.InstancesSetServiceAccountRequest{ForceSendFields: []string{"email"}}
-			if len(sa) > 0 && sa[0] != nil {
+			if !isEmptyServiceAccountBlock(d) && len(sa) > 0 && sa[0] != nil {
 				saMap := sa[0].(map[string]interface{})
 				req.Email = saMap["email"].(string)
 				req.Scopes = tpgresource.CanonicalizeServiceScopes(tpgresource.ConvertStringSet(saMap["scopes"].(*schema.Set)))
@@ -2862,6 +2862,11 @@ func serviceAccountDiffSuppress(k, old, new string, d *schema.ResourceData) bool
 	// suppress changes between { } and {scopes:[]}
 	if l[0] != nil {
 		contents := l[0].(map[string]interface{})
+		email := contents["email"]
+		if email != "" {
+			// if email is non empty, don't suppress the diff
+			return false
+		}
 		if scopes, ok := contents["scopes"]; ok {
 			a := scopes.(*schema.Set).List()
 			if a != nil && len(a) > 0 {
@@ -2871,3 +2876,42 @@ func serviceAccountDiffSuppress(k, old, new string, d *schema.ResourceData) bool
 	}
 	return true
 }
+
+// isEmptyServiceAccountBlock is used to work around an issue when updating
+// service accounts. Creating the instance with some scopes but without
+// specifying a service account email, assigns default compute service account
+// to the instance:
+//
+//	service_account {
+//	   scopes = ["some-scope"]
+//	}
+//
+// Then when updating the instance with empty service account:
+//
+//	service_account {
+//	   scopes = []
+//	}
+//
+// the default Terraform behavior is to clear scopes without clearing the
+// email. The email was previously computed to be the default service account
+// and has not been modified, so the default plan is to leave it unchanged.
+// However, when creating a new instance:
+//
+//	service_account {
+//	   scopes = []
+//	}
+//
+// indicates an instance without any service account set.
+// isEmptyServiceAccountBlock is used to detect empty service_account block
+// and if it is, it is interpreted as no service account and no scopes.
+func isEmptyServiceAccountBlock(d *schema.ResourceData) bool {
+	serviceAccountsConfig := d.GetRawConfig().GetAttr("service_account")
+	if serviceAccountsConfig.IsNull() || len(serviceAccountsConfig.AsValueSlice()) == 0 {
+		return true
+	}
+	serviceAccount := serviceAccountsConfig.AsValueSlice()[0]
+	if serviceAccount.GetAttr("email").IsNull() && len(serviceAccount.GetAttr("scopes").AsValueSlice()) == 0 {
+		return true
+	}
+	return false
+}

google/services/compute/resource_compute_instance_test.go

@@ -1101,6 +1101,54 @@ func TestAccComputeInstance_serviceAccount(t *testing.T) {
 	})
 }
 
+func TestAccComputeInstance_noServiceAccount(t *testing.T) {
+	t.Parallel()
+
+	var instance compute.Instance
+	var instanceName = fmt.Sprintf("tf-test-%s", acctest.RandString(t, 10))
+
+	acctest.VcrTest(t, resource.TestCase{
+		PreCheck:                 func() { acctest.AccTestPreCheck(t) },
+		ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t),
+		CheckDestroy:             testAccCheckComputeInstanceDestroyProducer(t),
+		Steps: []resource.TestStep{
+			{
+				Config: testAccComputeInstance_noServiceAccount(instanceName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckComputeInstanceExists(
+						t, "google_compute_instance.foobar", &instance),
+					testAccCheckComputeInstanceNoServiceAccount(&instance),
+				),
+			},
+			computeInstanceImportStep("us-central1-a", instanceName, []string{}),
+		},
+	})
+}
+
+func TestAccComputeInstance_serviceAccountEmail_0scopes(t *testing.T) {
+	t.Parallel()
+
+	var instance compute.Instance
+	var instanceName = fmt.Sprintf("tf-test-%s", acctest.RandString(t, 10))
+
+	acctest.VcrTest(t, resource.TestCase{
+		PreCheck:                 func() { acctest.AccTestPreCheck(t) },
+		ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t),
+		CheckDestroy:             testAccCheckComputeInstanceDestroyProducer(t),
+		Steps: []resource.TestStep{
+			{
+				Config: testAccComputeInstance_serviceAccountEmail_0scopes(instanceName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckComputeInstanceExists(
+						t, "google_compute_instance.foobar", &instance),
+					testAccCheckComputeInstanceMatchServiceAccount(&instance, "\\d+-compute@developer.gserviceaccount.com"),
+				),
+			},
+			computeInstanceImportStep("us-central1-a", instanceName, []string{}),
+		},
+	})
+}
+
 func TestAccComputeInstance_serviceAccount_updated(t *testing.T) {
 	t.Parallel()
 
@@ -1117,6 +1165,7 @@ func TestAccComputeInstance_serviceAccount_updated(t *testing.T) {
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckComputeInstanceExists(
 						t, "google_compute_instance.foobar", &instance),
+					testAccCheckComputeInstanceNoServiceAccount(&instance),
 					testAccCheckComputeInstanceScopes(&instance, 0),
 				),
 			},
@@ -1126,6 +1175,7 @@ func TestAccComputeInstance_serviceAccount_updated(t *testing.T) {
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckComputeInstanceExists(
 						t, "google_compute_instance.foobar", &instance),
+					testAccCheckComputeInstanceNoServiceAccount(&instance),
 					testAccCheckComputeInstanceScopes(&instance, 0),
 				),
 			},
@@ -1135,6 +1185,7 @@ func TestAccComputeInstance_serviceAccount_updated(t *testing.T) {
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckComputeInstanceExists(
 						t, "google_compute_instance.foobar", &instance),
+					testAccCheckComputeInstanceMatchServiceAccount(&instance, "\\d+-compute@developer.gserviceaccount.com"),
 					testAccCheckComputeInstanceScopes(&instance, 0),
 				),
 			},
@@ -1144,6 +1195,7 @@ func TestAccComputeInstance_serviceAccount_updated(t *testing.T) {
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckComputeInstanceExists(
 						t, "google_compute_instance.foobar", &instance),
+					testAccCheckComputeInstanceMatchServiceAccount(&instance, "\\d+-compute@developer.gserviceaccount.com"),
 					testAccCheckComputeInstanceScopes(&instance, 3),
 				),
 			},
@@ -1168,6 +1220,7 @@ func TestAccComputeInstance_serviceAccount_updated0to1to0scopes(t *testing.T) {
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckComputeInstanceExists(
 						t, "google_compute_instance.foobar", &instance),
+					testAccCheckComputeInstanceNoServiceAccount(&instance),
 					testAccCheckComputeInstanceScopes(&instance, 0),
 				),
 			},
@@ -1177,6 +1230,7 @@ func TestAccComputeInstance_serviceAccount_updated0to1to0scopes(t *testing.T) {
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckComputeInstanceExists(
 						t, "google_compute_instance.foobar", &instance),
+					testAccCheckComputeInstanceMatchServiceAccount(&instance, "\\d+-compute@developer.gserviceaccount.com"),
 					testAccCheckComputeInstanceScopes(&instance, 1),
 				),
 			},
@@ -1186,6 +1240,7 @@ func TestAccComputeInstance_serviceAccount_updated0to1to0scopes(t *testing.T) {
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckComputeInstanceExists(
 						t, "google_compute_instance.foobar", &instance),
+					testAccCheckComputeInstanceNoServiceAccount(&instance),
 					testAccCheckComputeInstanceScopes(&instance, 0),
 				),
 			},
@@ -3306,6 +3361,30 @@ func testAccCheckComputeInstanceServiceAccount(instance *compute.Instance, scope
 	}
 }
 
+func testAccCheckComputeInstanceNoServiceAccount(instance *compute.Instance) resource.TestCheckFunc {
+	return func(s *terraform.State) error {
+		if count := len(instance.ServiceAccounts); count != 0 {
+			return fmt.Errorf("Wrong number of ServiceAccounts: expected 0, got %d", count)
+		}
+		return nil
+	}
+}
+
+func testAccCheckComputeInstanceMatchServiceAccount(instance *compute.Instance, serviceAcctRegexp string) resource.TestCheckFunc {
+	return func(s *terraform.State) error {
+		if count := len(instance.ServiceAccounts); count != 1 {
+			return fmt.Errorf("Wrong number of ServiceAccounts: expected 1, got %d", count)
+		}
+
+		email := instance.ServiceAccounts[0].Email
+		if !regexp.MustCompile(serviceAcctRegexp).MatchString(email) {
+			return fmt.Errorf("ServiceAccount email didn't match:\"%s\", got \"%s\"", serviceAcctRegexp, email)
+		}
+
+		return nil
+	}
+}
+
 func testAccCheckComputeInstanceScopes(instance *compute.Instance, scopeCount int) resource.TestCheckFunc {
 	return func(s *terraform.State) error {
 
@@ -5277,6 +5356,70 @@ resource "google_compute_instance" "foobar" {
 `, instance)
 }
 
+func testAccComputeInstance_noServiceAccount(instance string) string {
+	return fmt.Sprintf(`
+data "google_compute_image" "my_image" {
+  family  = "debian-11"
+  project = "debian-cloud"
+}
+
+resource "google_compute_instance" "foobar" {
+  name         = "%s"
+  machine_type = "e2-medium"
+  zone         = "us-central1-a"
+
+  boot_disk {
+    initialize_params {
+      image = data.google_compute_image.my_image.self_link
+    }
+  }
+
+  network_interface {
+    network = "default"
+  }
+
+  service_account {
+	  scopes = []
+  }
+}
+`, instance)
+}
+
+func testAccComputeInstance_serviceAccountEmail_0scopes(instance string) string {
+	return fmt.Sprintf(`
+data "google_project" "project" {}
+
+data "google_compute_image" "my_image" {
+  family  = "debian-11"
+  project = "debian-cloud"
+}
+
+resource "google_compute_instance" "foobar" {
+  name         = "%s"
+  machine_type = "e2-medium"
+  zone         = "us-central1-a"
+
+  boot_disk {
+    initialize_params {
+      image = data.google_compute_image.my_image.self_link
+    }
+  }
+
+  network_interface {
+    network = "default"
+  }
+
+  service_account {
+    email = data.google_compute_default_service_account.default.email
+    scopes = []
+  }
+}
+
+data "google_compute_default_service_account" "default" {
+}
+`, instance)
+}
+
 func testAccComputeInstance_serviceAccount_update0(instance string) string {
 	return fmt.Sprintf(`
 data "google_compute_image" "my_image" {