id of instance templates now relies on the unique id of the resource

This is a security improvement to prevent instance templates being replaced before a follow-up operation (e.g. MIG creation) is started.

mmv1/third_party/terraform/resources/resource_compute_instance_from_template.go.erb

@@ -121,7 +121,8 @@ func resourceComputeInstanceFromTemplateCreate(d *schema.ResourceData, meta inte
 		return err
 	}
 
-	tpl, err := ParseInstanceTemplateFieldValue(d.Get("source_instance_template").(string), d, config)
+	sourceTemplate:= ConvertToUniqueIdWhenPresent(d.Get("source_instance_template").(string))
+	tpl, err := ParseInstanceTemplateFieldValue(sourceTemplate, d, config)
 	if err != nil {
 		return err
 	}

mmv1/third_party/terraform/resources/resource_compute_instance_group_manager.go.erb

@@ -55,7 +55,7 @@ func ResourceComputeInstanceGroupManager() *schema.Resource {
 						"instance_template": {
 							Type:             schema.TypeString,
 							Required:         true,
-							DiffSuppressFunc: compareSelfLinkRelativePaths,
+							DiffSuppressFunc: compareSelfLinkRelativePathsIgnoreParams,
 							Description:      `The full URL to an instance template from which all new instances of this version will be created.`,
 						},
 
@@ -495,6 +495,33 @@ func ResourceComputeInstanceGroupManager() *schema.Resource {
 	}
 }
 
+func parseUniqueId(s string) (string, string) {
+	splits:= strings.SplitN(s, "?uniqueId=", 2)
+	if len(splits) == 2 {
+		return splits[0], splits[1]
+	}
+	return s, ""
+}
+
+func compareSelfLinkRelativePathsIgnoreParams(_unused1, old, new string, _unused2 *schema.ResourceData) bool {
+	oldName, oldUniqueId:= parseUniqueId(old)
+	newName, newUniqueId:= parseUniqueId(new)
+	if oldUniqueId != "" && newUniqueId != "" && oldUniqueId != newUniqueId {
+		return false
+	}
+	return compareSelfLinkRelativePaths(_unused1, oldName, newName, _unused2)
+}
+
+func ConvertToUniqueIdWhenPresent(s string) string {
+	original, uniqueId:= parseUniqueId(s)
+	if uniqueId != "" {
+		splits:= strings.Split(original, "/")
+		splits[len(splits)-1] = uniqueId
+		return strings.Join(splits, "/")
+	}
+	return s
+}
+
 func getNamedPorts(nps []interface{}) []*compute.NamedPort {
 	namedPorts := make([]*compute.NamedPort, 0, len(nps))
 	for _, v := range nps {
@@ -1160,7 +1187,7 @@ func expandVersions(configured []interface{}) []*compute.InstanceGroupManagerVer
 
 		version := compute.InstanceGroupManagerVersion{
 			Name:             data["name"].(string),
-			InstanceTemplate: data["instance_template"].(string),
+			InstanceTemplate: ConvertToUniqueIdWhenPresent(data["instance_template"].(string)),
 			TargetSize:       expandFixedOrPercent(data["target_size"].([]interface{})),
 		}
 

mmv1/third_party/terraform/resources/resource_compute_instance_template.go.erb

@@ -1243,7 +1243,7 @@ func resourceComputeInstanceTemplateCreate(d *schema.ResourceData, meta interfac
 	}
 
 	// Store the ID now
-	d.SetId(fmt.Sprintf("projects/%s/global/instanceTemplates/%s", project, instanceTemplate.Name))
+	d.SetId(fmt.Sprintf("projects/%s/global/instanceTemplates/%v?uniqueId=%v", project, itName, op.TargetId))
 
 	err = computeOperationWaitTime(config, op, project, "Creating Instance Template", userAgent, d.Timeout(schema.TimeoutCreate))
 	if err != nil {
@@ -1486,12 +1486,19 @@ func resourceComputeInstanceTemplateRead(d *schema.ResourceData, meta interface{
 		return err
 	}
 
-	splits := strings.Split(d.Id(), "/")
-	instanceTemplate, err := config.NewComputeClient(userAgent).InstanceTemplates.Get(project, splits[len(splits)-1]).Do()
+	id:= ConvertToUniqueIdWhenPresent(d.Id())
+	splits := strings.Split(id, "/")
+	template:= splits[len(splits)-1]
+
+	instanceTemplate, err := config.NewComputeClient(userAgent).InstanceTemplates.Get(project, template).Do()
 	if err != nil {
 		return handleNotFoundError(err, d, fmt.Sprintf("Instance Template %q", d.Get("name").(string)))
 	}
 
+	// fixing the terraform id to refer to this resource through its unique numeric id
+	uniqueIdSuffix:= fmt.Sprintf("?uniqueId=%v", instanceTemplate.Id)
+	d.SetId(fmt.Sprintf("projects/%s/global/instanceTemplates/%v%v", project, instanceTemplate.Name, uniqueIdSuffix))
+
 	// Set the metadata fingerprint if there is one.
 	if instanceTemplate.Properties.Metadata != nil {
 		if err = d.Set("metadata_fingerprint", instanceTemplate.Properties.Metadata.Fingerprint); err != nil {
@@ -1530,7 +1537,7 @@ func resourceComputeInstanceTemplateRead(d *schema.ResourceData, meta interface{
 			return fmt.Errorf("Error setting labels: %s", err)
 		}
 	}
-	if err = d.Set("self_link", instanceTemplate.SelfLink); err != nil {
+	if err = d.Set("self_link", instanceTemplate.SelfLink + uniqueIdSuffix); err != nil {
 		return fmt.Errorf("Error setting self_link: %s", err)
 	}
 	if err = d.Set("name", instanceTemplate.Name); err != nil {
@@ -1661,9 +1668,12 @@ func resourceComputeInstanceTemplateDelete(d *schema.ResourceData, meta interfac
 		return err
 	}
 
-	splits := strings.Split(d.Id(), "/")
+	id:= ConvertToUniqueIdWhenPresent(d.Id())
+	splits := strings.Split(id, "/")
+	template:= splits[len(splits)-1]
+
 	op, err := config.NewComputeClient(userAgent).InstanceTemplates.Delete(
-		project, splits[len(splits)-1]).Do()
+		project, template).Do()
 	if err != nil {
 		return fmt.Errorf("Error deleting instance template: %s", err)
 	}

mmv1/third_party/terraform/resources/resource_compute_region_instance_group_manager.go.erb

@@ -56,7 +56,7 @@ func ResourceComputeRegionInstanceGroupManager() *schema.Resource {
 						"instance_template": {
 							Type:             schema.TypeString,
 							Required:         true,
-							DiffSuppressFunc: compareSelfLinkRelativePaths,
+							DiffSuppressFunc: compareSelfLinkRelativePathsIgnoreParams,
 							Description:      `The full URL to an instance template from which all new instances of this version will be created.`,
 						},
 

mmv1/third_party/terraform/tests/resource_compute_instance_group_manager_test.go.erb

@@ -68,6 +68,76 @@ func testSweepComputeInstanceGroupManager(region string) error {
 	return nil
 }
 
+func TestInstanceGroupManager_parseUniqueId(t *testing.T) {
+	expectations:= map[string][]string {
+		"projects/imre-test/global/instanceTemplates/example-template-custom?uniqueId=123": []string{"projects/imre-test/global/instanceTemplates/example-template-custom", "123"},
+		"https://www.googleapis.com/compute/v1/projects/imre-test/global/instanceTemplates/example-template-custom?uniqueId=123": []string{"https://www.googleapis.com/compute/v1/projects/imre-test/global/instanceTemplates/example-template-custom", "123"},
+		"projects/imre-test/global/instanceTemplates/example-template-custom": []string{"projects/imre-test/global/instanceTemplates/example-template-custom", ""},
+		"https://www.googleapis.com/compute/v1/projects/imre-test/global/instanceTemplates/example-template-custom": []string{"https://www.googleapis.com/compute/v1/projects/imre-test/global/instanceTemplates/example-template-custom", ""},
+		"example-template-custom?uniqueId=123": []string{"example-template-custom", "123"},
+	}
+
+	for k, v:= range expectations {
+		aName, aUniqueId := parseUniqueId(k)
+		if v[0] != aName {
+			t.Errorf("parseUniqueId failed; name of %v should be %v, not %v", k, v[0], aName)
+		}
+		if v[1] != aUniqueId {
+			t.Errorf("parseUniqueId failed; uniqueId of %v should be %v, not %v", k, v[1], aUniqueId)
+		}
+	}
+}
+
+func TestInstanceGroupManager_compareInstanceTemplate(t *testing.T) {
+	shouldAllMatch := []string {
+		// uniqueId not present
+		"https://www.googleapis.com/compute/v1/projects/imre-test/global/instanceTemplates/example-template-custom",
+		"projects/imre-test/global/instanceTemplates/example-template-custom",
+		// uniqueId present
+		"https://www.googleapis.com/compute/v1/projects/imre-test/global/instanceTemplates/example-template-custom?uniqueId=123",
+		"projects/imre-test/global/instanceTemplates/example-template-custom?uniqueId=123",
+	}
+	shouldNotMatch := map[string]string {
+		// mismatching name
+		"https://www.googleapis.com/compute/v1/projects/imre-test/global/instanceTemplates/example-template-custom": "projects/imre-test/global/instanceTemplates/example-template-custom2",
+		"projects/imre-test/global/instanceTemplates/example-template-custom": "https://www.googleapis.com/compute/v1/projects/imre-test/global/instanceTemplates/example-template-custom2",
+		// matching name, but mismatching uniqueId
+		"https://www.googleapis.com/compute/v1/projects/imre-test/global/instanceTemplates/example-template-custom?uniqueId=123": "projects/imre-test/global/instanceTemplates/example-template-custom?uniqueId=1234",
+		"projects/imre-test/global/instanceTemplates/example-template-custom?uniqueId=123": "https://www.googleapis.com/compute/v1/projects/imre-test/global/instanceTemplates/example-template-custom?uniqueId=1234",
+	}
+	for _, v1 := range shouldAllMatch {
+		for _, v2:= range shouldAllMatch {
+			if !compareSelfLinkRelativePathsIgnoreParams("", v1, v2, nil) {
+				t.Fatalf("compareSelfLinkRelativePathsIgnoreParams did not match (and should have) %v and %v", v1, v2)
+			}
+		}
+	}
+
+	for v1, v2 := range shouldNotMatch {
+		if compareSelfLinkRelativePathsIgnoreParams("", v1, v2, nil) {
+			t.Fatalf("compareSelfLinkRelativePathsIgnoreParams did match (and shouldn't) %v and %v", v1, v2)
+		}
+	}
+}
+
+func TestInstanceGroupManager_convertUniqueId(t *testing.T) {
+	matches:= map[string]string {
+		// uniqueId not present (should return the same)
+		"https://www.googleapis.com/compute/v1/projects/imre-test/global/instanceTemplates/example-template-custom": "https://www.googleapis.com/compute/v1/projects/imre-test/global/instanceTemplates/example-template-custom",
+		"projects/imre-test/global/instanceTemplates/example-template-custom": "projects/imre-test/global/instanceTemplates/example-template-custom",
+		// uniqueId present (should return the last component replaced)
+		"https://www.googleapis.com/compute/v1/projects/imre-test/global/instanceTemplates/example-template-custom?uniqueId=123": "https://www.googleapis.com/compute/v1/projects/imre-test/global/instanceTemplates/123",
+		"projects/imre-test/global/instanceTemplates/example-template-custom?uniqueId=123": "projects/imre-test/global/instanceTemplates/123",
+		"tf-test-igm-8amncgtq22?uniqueId=8361222501423044003": "8361222501423044003",
+	}
+	for input, expected := range matches {
+		actual:= ConvertToUniqueIdWhenPresent(input)
+		if actual != expected {
+			t.Fatalf("invalid return value by ConvertToUniqueIdWhenPresent for input %v; expected: %v, actual: %v", input, expected, actual)
+		}
+	}
+}
+
 func TestAccInstanceGroupManager_basic(t *testing.T) {
 	t.Parallel()
 

mmv1/third_party/terraform/tests/resource_compute_instance_template_test.go.erb

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"reflect"
 	"regexp"
+	"strconv"
 	"strings"
 	"testing"
 	"time"
@@ -1302,7 +1303,10 @@ func testAccCheckComputeInstanceTemplateExistsInProject(t *testing.T, n, p strin
 
 		config := googleProviderConfig(t)
 
-		splits := strings.Split(rs.Primary.ID, "/")
+		// old syntax: projects/imre-test/global/instanceTemplates/something
+		// new syntax: projects/imre-test/global/instanceTemplates/something?uniqueId=123123
+		originalId, uniqueId:= parseUniqueId(rs.Primary.ID)
+		splits := strings.Split(originalId, "/")
 		templateName := splits[len(splits)-1]
 		found, err := config.NewComputeClient(config.userAgent).InstanceTemplates.Get(
 			p, templateName).Do()
@@ -1311,7 +1315,16 @@ func testAccCheckComputeInstanceTemplateExistsInProject(t *testing.T, n, p strin
 		}
 
 		if found.Name != templateName {
-			return fmt.Errorf("Instance template not found")
+			return fmt.Errorf("Instance template: mismatching name")
+		}
+		foundId:= strconv.FormatUint(found.Id, 10)
+		if foundId != uniqueId {
+			return fmt.Errorf("Instance template: mismatching unique ID")
+		}
+
+		selfLink:= rs.Primary.Attributes["self_link"]
+		if !strings.HasSuffix(selfLink, "?uniqueId="+foundId) {
+			return fmt.Errorf("unique ID is not present in the self_link")
 		}
 
 		*instanceTemplate = *found

mmv1/third_party/terraform/website/docs/d/compute_instance_template.html.markdown

@@ -297,7 +297,7 @@ The `disk_encryption_key` block supports:
 
 ---
 
-* `id` - an identifier for the resource with format `projects/{{project}}/global/instanceTemplates/{{name}}`
+* `id` - an identifier for the resource with format `projects/{{project}}/global/instanceTemplates/{{name}}?uniqueId={{uniqueId}}`
 
 * `metadata_fingerprint` - The unique fingerprint of the metadata.
 

mmv1/third_party/terraform/website/docs/r/compute_instance_template.html.markdown

@@ -635,7 +635,7 @@ The `specific_reservation` block supports:
 In addition to the arguments listed above, the following computed attributes are
 exported:
 
-* `id` - an identifier for the resource with format `projects/{{project}}/global/instanceTemplates/{{name}}`
+* `id` - an identifier for the resource with format `projects/{{project}}/global/instanceTemplates/{{name}}?uniqueId={{uniqueId}}`
 
 * `metadata_fingerprint` - The unique fingerprint of the metadata.