Use a set for service account scopes. Fix #1759

builtin/providers/google/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)

builtin/providers/google/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 <N> 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
+}

builtin/providers/google/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 {

builtin/providers/google/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",
+		]
+	}
+}`