Skip to content

Commit

Permalink
fix: google_compute_instance with user-managed service account and em…
Browse files Browse the repository at this point in the history 
…pty scopes results in no service account assignment (#10358) (#18521)

[upstream:a00db19f538bab3f8f8d42b35d033a7703e19e18]

Signed-off-by: Modular Magician <[email protected]>
  • Loading branch information
@modular-magician
modular-magician authored Jun 22, 2024
1 parent 1e7fb46 commit 3463a5b
Show file tree
Hide file tree
Showing 3 changed files with 191 additions and 1 deletion.
3 changes: 3 additions & 0 deletions .changelog/10358.txt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
compute: fixed google_compute_instance with `service_account.email` but no `service_account.scopes`
```
46 changes: 45 additions & 1 deletion google/services/compute/resource_compute_instance.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)))
Expand Down Expand Up @@ -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 {
Expand All @@ -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
}
143 changes: 143 additions & 0 deletions google/services/compute/resource_compute_instance_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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, "\\[email protected]"),
),
},
computeInstanceImportStep("us-central1-a", instanceName, []string{}),
},
})
}

func TestAccComputeInstance_serviceAccount_updated(t *testing.T) {
t.Parallel()

Expand All @@ -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),
),
},
Expand All @@ -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),
),
},
Expand All @@ -1135,6 +1185,7 @@ func TestAccComputeInstance_serviceAccount_updated(t *testing.T) {
Check: resource.ComposeTestCheckFunc(
testAccCheckComputeInstanceExists(
t, "google_compute_instance.foobar", &instance),
testAccCheckComputeInstanceMatchServiceAccount(&instance, "\\[email protected]"),
testAccCheckComputeInstanceScopes(&instance, 0),
),
},
Expand All @@ -1144,6 +1195,7 @@ func TestAccComputeInstance_serviceAccount_updated(t *testing.T) {
Check: resource.ComposeTestCheckFunc(
testAccCheckComputeInstanceExists(
t, "google_compute_instance.foobar", &instance),
testAccCheckComputeInstanceMatchServiceAccount(&instance, "\\[email protected]"),
testAccCheckComputeInstanceScopes(&instance, 3),
),
},
Expand All @@ -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),
),
},
Expand All @@ -1177,6 +1230,7 @@ func TestAccComputeInstance_serviceAccount_updated0to1to0scopes(t *testing.T) {
Check: resource.ComposeTestCheckFunc(
testAccCheckComputeInstanceExists(
t, "google_compute_instance.foobar", &instance),
testAccCheckComputeInstanceMatchServiceAccount(&instance, "\\[email protected]"),
testAccCheckComputeInstanceScopes(&instance, 1),
),
},
Expand All @@ -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),
),
},
Expand Down Expand Up @@ -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 {

Expand Down Expand Up @@ -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" {
Expand Down

0 comments on commit 3463a5b

Please sign in to comment.