Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/10358.txt
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
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
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, "\\d+-compute@developer.gserviceaccount.com"),
),
},
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, "\\d+-compute@developer.gserviceaccount.com"),
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, "\\d+-compute@developer.gserviceaccount.com"),
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, "\\d+-compute@developer.gserviceaccount.com"),
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