From b39460bd62191b61b324113f1b4ac6a8fe3e6630 Mon Sep 17 00:00:00 2001 From: Dave Cunningham Date: Thu, 30 Apr 2015 21:21:21 -0400 Subject: [PATCH] Use a set for service account scopes. Fix #1759 --- resource_compute_instance.go | 32 +++++----- resource_compute_instance_migrate.go | 71 ++++++++++++++++++++++- resource_compute_instance_migrate_test.go | 21 +++++++ resource_compute_instance_test.go | 64 ++++++++++++++++++++ 4 files changed, 174 insertions(+), 14 deletions(-) diff --git a/resource_compute_instance.go b/resource_compute_instance.go index 565d2f2c..255718e5 100644 --- a/resource_compute_instance.go +++ b/resource_compute_instance.go @@ -11,6 +11,15 @@ import ( "google.golang.org/api/googleapi" ) +func stringHashcode(v interface{}) int { + return hashcode.String(v.(string)) +} + +func stringScopeHashcode(v interface{}) int { + v = canonicalizeServiceScope(v.(string)) + return hashcode.String(v.(string)) +} + func resourceComputeInstance() *schema.Resource { return &schema.Resource{ Create: resourceComputeInstanceCreate, @@ -18,7 +27,7 @@ func resourceComputeInstance() *schema.Resource { Update: resourceComputeInstanceUpdate, Delete: resourceComputeInstanceDelete, - SchemaVersion: 1, + SchemaVersion: 2, MigrateState: resourceComputeInstanceMigrateState, Schema: map[string]*schema.Schema{ @@ -195,7 +204,7 @@ func resourceComputeInstance() *schema.Resource { }, "scopes": &schema.Schema{ - Type: schema.TypeList, + Type: schema.TypeSet, Required: true, ForceNew: true, Elem: &schema.Schema{ @@ -204,6 +213,7 @@ func resourceComputeInstance() *schema.Resource { return canonicalizeServiceScope(v.(string)) }, }, + Set: stringScopeHashcode, }, }, }, @@ -213,9 +223,7 @@ func resourceComputeInstance() *schema.Resource { Type: schema.TypeSet, Optional: true, Elem: &schema.Schema{Type: schema.TypeString}, - Set: func(v interface{}) int { - return hashcode.String(v.(string)) - }, + Set: stringHashcode, }, "metadata_fingerprint": &schema.Schema{ @@ -434,11 +442,10 @@ func resourceComputeInstanceCreate(d *schema.ResourceData, meta interface{}) err for i := 0; i < serviceAccountsCount; i++ { prefix := fmt.Sprintf("service_account.%d", i) - scopesCount := d.Get(prefix + ".scopes.#").(int) - scopes := make([]string, 0, scopesCount) - for j := 0; j < scopesCount; j++ { - scope := d.Get(fmt.Sprintf(prefix+".scopes.%d", j)).(string) - scopes = append(scopes, canonicalizeServiceScope(scope)) + scopesSet := d.Get(prefix + ".scopes").(*schema.Set) + scopes := make([]string, scopesSet.Len()) + for i, v := range scopesSet.List() { + scopes[i] = canonicalizeServiceScope(v.(string)) } serviceAccount := &compute.ServiceAccount{ @@ -504,14 +511,13 @@ func resourceComputeInstanceRead(d *schema.ResourceData, meta interface{}) error // Set the service accounts serviceAccounts := make([]map[string]interface{}, 0, 1) for _, serviceAccount := range instance.ServiceAccounts { - scopes := make([]string, len(serviceAccount.Scopes)) + scopes := make([]interface{}, len(serviceAccount.Scopes)) for i, scope := range serviceAccount.Scopes { scopes[i] = scope } - serviceAccounts = append(serviceAccounts, map[string]interface{}{ "email": serviceAccount.Email, - "scopes": scopes, + "scopes": schema.NewSet(stringScopeHashcode, scopes), }) } d.Set("service_account", serviceAccounts) diff --git a/resource_compute_instance_migrate.go b/resource_compute_instance_migrate.go index dd883f0f..749839ad 100644 --- a/resource_compute_instance_migrate.go +++ b/resource_compute_instance_migrate.go @@ -6,6 +6,7 @@ import ( "strconv" "strings" + "github.com/hashicorp/terraform/helper/hashcode" "github.com/hashicorp/terraform/terraform" ) @@ -19,7 +20,18 @@ func resourceComputeInstanceMigrateState( switch v { case 0: log.Println("[INFO] Found Compute Instance State v0; migrating to v1") - return migrateStateV0toV1(is) + is, err := migrateStateV0toV1(is) + if err != nil { + return is, err + } + fallthrough + case 1: + log.Println("[INFO] Found Compute Instance State v1; migrating to v2") + is, err := migrateStateV1toV2(is) + if err != nil { + return is, err + } + return is, nil default: return is, fmt.Errorf("Unexpected schema version: %d", v) } @@ -70,3 +82,60 @@ func migrateStateV0toV1(is *terraform.InstanceState) (*terraform.InstanceState, log.Printf("[DEBUG] Attributes after migration: %#v", is.Attributes) return is, nil } + +func migrateStateV1toV2(is *terraform.InstanceState) (*terraform.InstanceState, error) { + log.Printf("[DEBUG] Attributes before migration: %#v", is.Attributes) + + // Maps service account index to list of scopes for that sccount + newScopesMap := make(map[string][]string) + + for k, v := range is.Attributes { + if !strings.HasPrefix(k, "service_account.") { + continue + } + + if k == "service_account.#" { + continue + } + + if strings.HasSuffix(k, ".scopes.#") { + continue + } + + if strings.HasSuffix(k, ".email") { + continue + } + + // Key is now of the form service_account.%d.scopes.%d + kParts := strings.Split(k, ".") + + // Sanity check: all three parts should be there and should be a number + badFormat := false + if len(kParts) != 4 { + badFormat = true + } else if _, err := strconv.Atoi(kParts[1]); err != nil { + badFormat = true + } + + if badFormat { + return is, fmt.Errorf( + "migration error: found scope key in unexpected format: %s", k) + } + + newScopesMap[kParts[1]] = append(newScopesMap[kParts[1]], v) + + delete(is.Attributes, k) + } + + + for service_acct_index, newScopes := range newScopesMap { + for _, newScope := range newScopes { + hash := hashcode.String(canonicalizeServiceScope(newScope)) + newKey := fmt.Sprintf("service_account.%s.scopes.%d", service_acct_index, hash) + is.Attributes[newKey] = newScope + } + } + + log.Printf("[DEBUG] Attributes after migration: %#v", is.Attributes) + return is, nil +} diff --git a/resource_compute_instance_migrate_test.go b/resource_compute_instance_migrate_test.go index 2bf01ff6..9272761c 100644 --- a/resource_compute_instance_migrate_test.go +++ b/resource_compute_instance_migrate_test.go @@ -27,6 +27,27 @@ func TestComputeInstanceMigrateState(t *testing.T) { "metadata.with.dots": "should.work", }, }, + "change scope from list to set": { + StateVersion: 1, + Attributes: map[string]string{ + "service_account.#": "1", + "service_account.0.email": "xxxxxx-compute@developer.gserviceaccount.com", + "service_account.0.scopes.#": "4", + "service_account.0.scopes.0": "https://www.googleapis.com/auth/compute", + "service_account.0.scopes.1": "https://www.googleapis.com/auth/datastore", + "service_account.0.scopes.2": "https://www.googleapis.com/auth/devstorage.full_control", + "service_account.0.scopes.3": "https://www.googleapis.com/auth/logging.write", + }, + Expected: map[string]string{ + "service_account.#": "1", + "service_account.0.email": "xxxxxx-compute@developer.gserviceaccount.com", + "service_account.0.scopes.#": "4", + "service_account.0.scopes.1693978638": "https://www.googleapis.com/auth/devstorage.full_control", + "service_account.0.scopes.172152165": "https://www.googleapis.com/auth/logging.write", + "service_account.0.scopes.299962681": "https://www.googleapis.com/auth/compute", + "service_account.0.scopes.3435931483": "https://www.googleapis.com/auth/datastore", + }, + }, } for tn, tc := range cases { diff --git a/resource_compute_instance_test.go b/resource_compute_instance_test.go index d6ee96a2..9dabc2fb 100644 --- a/resource_compute_instance_test.go +++ b/resource_compute_instance_test.go @@ -227,6 +227,31 @@ func TestAccComputeInstance_update(t *testing.T) { }) } +func TestAccComputeInstance_service_account(t *testing.T) { + var instance compute.Instance + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckComputeInstanceDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: testAccComputeInstance_service_account, + Check: resource.ComposeTestCheckFunc( + testAccCheckComputeInstanceExists( + "google_compute_instance.foobar", &instance), + testAccCheckComputeInstanceServiceAccount(&instance, + "https://www.googleapis.com/auth/compute.readonly"), + testAccCheckComputeInstanceServiceAccount(&instance, + "https://www.googleapis.com/auth/devstorage.read_only"), + testAccCheckComputeInstanceServiceAccount(&instance, + "https://www.googleapis.com/auth/userinfo.email"), + ), + }, + }, + }) +} + func testAccCheckComputeInstanceDestroy(s *terraform.State) error { config := testAccProvider.Meta().(*Config) @@ -356,6 +381,22 @@ func testAccCheckComputeInstanceTag(instance *compute.Instance, n string) resour } } +func testAccCheckComputeInstanceServiceAccount(instance *compute.Instance, scope 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) + } + + for _, val := range instance.ServiceAccounts[0].Scopes { + if val == scope { + return nil + } + } + + return fmt.Errorf("Scope not found: %s", scope) + } +} + const testAccComputeInstance_basic_deprecated_network = ` resource "google_compute_instance" "foobar" { name = "terraform-test" @@ -567,3 +608,26 @@ resource "google_compute_instance" "foobar" { foo = "bar" } }` + +const testAccComputeInstance_service_account = ` +resource "google_compute_instance" "foobar" { + name = "terraform-test" + machine_type = "n1-standard-1" + zone = "us-central1-a" + + disk { + image = "debian-7-wheezy-v20140814" + } + + network_interface { + network = "default" + } + + service_account { + scopes = [ + "userinfo-email", + "compute-ro", + "storage-ro", + ] + } +}`