Add support for alias_ip_range in google_compute_instance network interface (#375)

google/api_versions.go

@@ -2,6 +2,8 @@ package google
 
 import (
 	"encoding/json"
+	"fmt"
+	"strings"
 )
 
 type ComputeApiVersion uint8
@@ -71,7 +73,16 @@ func getComputeApiVersionUpdate(d TerraformResourceData, resourceVersion Compute
 // A field of a resource and the version of the Compute API required to use it.
 type Feature struct {
 	Version ComputeApiVersion
-	Item    string
+	// Path to the beta field.
+	//
+	// The feature is considered to be in-use if the field referenced by "Item" is set in the state.
+	// The path can reference:
+	// - a beta field at the top-level (e.g. "min_cpu_platform").
+	// - a beta field nested inside a list (e.g. "network_interface.*.alias_ip_range" is considered to be
+	// 		in-use if the "alias_ip_range" field is set in the state for any of the network interfaces).
+	//
+	// Note: beta field nested inside a SET are NOT supported at the moment.
+	Item string
 }
 
 // Returns true when a feature has been modified.
@@ -84,8 +95,34 @@ func (s Feature) HasChangeBy(d TerraformResourceData) bool {
 
 // Return true when a feature appears in schema or has been modified.
 func (s Feature) InUseBy(d TerraformResourceData) bool {
-	_, ok := d.GetOk(s.Item)
+	return inUseBy(d, s.Item)
-	return ok || s.HasChangeBy(d)
+}
+
+func inUseBy(d TerraformResourceData, path string) bool {
+	pos := strings.Index(path, "*")
+	if pos == -1 {
+		_, ok := d.GetOk(path)
+		return ok || d.HasChange(path)
+	}
+
+	prefix := path[0:pos]
+	suffix := path[pos+1:]
+
+	v, ok := d.GetOk(prefix + "#")
+
+	if !ok {
+		return false
+	}
+
+	count := v.(int)
+	for i := 0; i < count; i++ {
+		nestedPath := fmt.Sprintf("%s%d%s", prefix, i, suffix)
+		if inUseBy(d, nestedPath) {
+			return true
+		}
+	}
+
+	return false
 }
 
 func maxVersion(versionsInUse map[ComputeApiVersion]struct{}) ComputeApiVersion {

google/api_versions_test.go

@@ -4,7 +4,9 @@ import "testing"
 
 func TestResourceWithOnlyBaseVersionFields(t *testing.T) {
 	d := &ResourceDataMock{
-		FieldsInSchema: []string{"normal_field"},
+		FieldsInSchema: map[string]interface{}{
+			"normal_field": "foo",
+		},
 	}
 
 	resourceVersion := v1
@@ -22,7 +24,10 @@ func TestResourceWithOnlyBaseVersionFields(t *testing.T) {
 func TestResourceWithBetaFields(t *testing.T) {
 	resourceVersion := v1
 	d := &ResourceDataMock{
-		FieldsInSchema: []string{"normal_field", "beta_field"},
+		FieldsInSchema: map[string]interface{}{
+			"normal_field": "foo",
+			"beta_field":   "bar",
+		},
 	}
 
 	expectedVersion := v0beta
@@ -40,7 +45,9 @@ func TestResourceWithBetaFields(t *testing.T) {
 func TestResourceWithBetaFieldsNotInSchema(t *testing.T) {
 	resourceVersion := v1
 	d := &ResourceDataMock{
-		FieldsInSchema: []string{"normal_field"},
+		FieldsInSchema: map[string]interface{}{
+			"normal_field": "foo",
+		},
 	}
 
 	expectedVersion := v1
@@ -58,7 +65,10 @@ func TestResourceWithBetaFieldsNotInSchema(t *testing.T) {
 func TestResourceWithBetaUpdateFields(t *testing.T) {
 	resourceVersion := v1
 	d := &ResourceDataMock{
-		FieldsInSchema:      []string{"normal_field", "beta_update_field"},
+		FieldsInSchema: map[string]interface{}{
+			"normal_field": "foo",
+			"beta_field":   "bar",
+		},
 		FieldsWithHasChange: []string{"beta_update_field"},
 	}
 
@@ -73,11 +83,76 @@ func TestResourceWithBetaUpdateFields(t *testing.T) {
 	if computeApiVersion != expectedVersion {
 		t.Errorf("Expected to see version: %v. Saw version: %v.", expectedVersion, computeApiVersion)
 	}
+}
 
+func TestResourceWithOnlyBaseNestedFields(t *testing.T) {
+	resourceVersion := v1
+	d := &ResourceDataMock{
+		FieldsInSchema: map[string]interface{}{
+			"list_field.#":              2,
+			"list_field.0.normal_field": "foo",
+			"list_field.1.normal_field": "bar",
+		},
+	}
+
+	computeApiVersion := getComputeApiVersion(d, resourceVersion, []Feature{})
+	if computeApiVersion != resourceVersion {
+		t.Errorf("Expected to see version: %v. Saw version: %v.", resourceVersion, computeApiVersion)
+	}
+
+	computeApiVersion = getComputeApiVersionUpdate(d, resourceVersion, []Feature{}, []Feature{{Version: resourceVersion, Item: "list_field.*.beta_nested_field"}})
+	if computeApiVersion != resourceVersion {
+		t.Errorf("Expected to see version: %v. Saw version: %v.", resourceVersion, computeApiVersion)
+	}
+}
+
+func TestResourceWithBetaNestedFields(t *testing.T) {
+	resourceVersion := v1
+	d := &ResourceDataMock{
+		FieldsInSchema: map[string]interface{}{
+			"list_field.#":                   2,
+			"list_field.0.normal_field":      "foo",
+			"list_field.1.normal_field":      "bar",
+			"list_field.1.beta_nested_field": "baz",
+		},
+	}
+
+	expectedVersion := v0beta
+	computeApiVersion := getComputeApiVersion(d, resourceVersion, []Feature{{Version: expectedVersion, Item: "list_field.*.beta_nested_field"}})
+	if computeApiVersion != expectedVersion {
+		t.Errorf("Expected to see version: %v. Saw version: %v.", expectedVersion, computeApiVersion)
+	}
+
+	computeApiVersion = getComputeApiVersionUpdate(d, resourceVersion, []Feature{{Version: expectedVersion, Item: "list_field.*.beta_nested_field"}}, []Feature{})
+	if computeApiVersion != expectedVersion {
+		t.Errorf("Expected to see version: %v. Saw version: %v.", expectedVersion, computeApiVersion)
+	}
+}
+
+func TestResourceWithBetaDoubleNestedFields(t *testing.T) {
+	resourceVersion := v1
+	d := &ResourceDataMock{
+		FieldsInSchema: map[string]interface{}{
+			"list_field.#":                                       1,
+			"list_field.0.nested_list_field.#":                   1,
+			"list_field.0.nested_list_field.0.beta_nested_field": "foo",
+		},
+	}
+
+	expectedVersion := v0beta
+	computeApiVersion := getComputeApiVersion(d, resourceVersion, []Feature{{Version: expectedVersion, Item: "list_field.*.nested_list_field.*.beta_nested_field"}})
+	if computeApiVersion != expectedVersion {
+		t.Errorf("Expected to see version: %v. Saw version: %v.", expectedVersion, computeApiVersion)
+	}
+
+	computeApiVersion = getComputeApiVersionUpdate(d, resourceVersion, []Feature{{Version: expectedVersion, Item: "list_field.*.nested_list_field.*.beta_nested_field"}}, []Feature{})
+	if computeApiVersion != expectedVersion {
+		t.Errorf("Expected to see version: %v. Saw version: %v.", expectedVersion, computeApiVersion)
+	}
 }
 
 type ResourceDataMock struct {
-	FieldsInSchema      []string
+	FieldsInSchema      map[string]interface{}
 	FieldsWithHasChange []string
 }
 
@@ -93,13 +168,11 @@ func (d *ResourceDataMock) HasChange(key string) bool {
 }
 
 func (d *ResourceDataMock) GetOk(key string) (interface{}, bool) {
-	exists := false
+	for k, v := range d.FieldsInSchema {
-	for _, val := range d.FieldsInSchema {
+		if key == k {
-		if key == val {
+			return v, true
-			exists = true
 		}
-
 	}
 
-	return nil, exists
+	return nil, false
 }

google/resource_compute_instance.go

@@ -26,6 +26,10 @@ var InstanceVersionedFeatures = []Feature{
 		Version: v0beta,
 		Item:    "min_cpu_platform",
 	},
+	{
+		Version: v0beta,
+		Item:    "network_interface.*.alias_ip_range",
+	},
 }
 
 func stringScopeHashcode(v interface{}) int {
@@ -293,7 +297,7 @@ func resourceComputeInstance() *schema.Resource {
 							Optional:         true,
 							Computed:         true,
 							ForceNew:         true,
-							DiffSuppressFunc: linkDiffSuppress,
+							DiffSuppressFunc: compareSelfLinkOrResourceName,
 						},
 
 						"subnetwork": &schema.Schema{
@@ -301,7 +305,7 @@ func resourceComputeInstance() *schema.Resource {
 							Optional:         true,
 							Computed:         true,
 							ForceNew:         true,
-							DiffSuppressFunc: linkDiffSuppress,
+							DiffSuppressFunc: compareSelfLinkOrResourceName,
 						},
 
 						"subnetwork_project": &schema.Schema{
@@ -340,6 +344,27 @@ func resourceComputeInstance() *schema.Resource {
 								},
 							},
 						},
+
+						"alias_ip_range": &schema.Schema{
+							Type:     schema.TypeList,
+							Optional: true,
+							ForceNew: true,
+							Elem: &schema.Resource{
+								Schema: map[string]*schema.Schema{
+									"ip_cidr_range": &schema.Schema{
+										Type:             schema.TypeString,
+										Required:         true,
+										ForceNew:         true,
+										DiffSuppressFunc: ipCidrRangeDiffSuppress,
+									},
+									"subnetwork_range_name": &schema.Schema{
+										Type:     schema.TypeString,
+										Optional: true,
+										ForceNew: true,
+									},
+								},
+							},
+						},
 					},
 				},
 			},
@@ -796,6 +821,7 @@ func resourceComputeInstanceCreate(d *schema.ResourceData, meta interface{}) err
 			iface.Network = networkLink
 			iface.Subnetwork = subnetworkLink
 			iface.NetworkIP = address
+			iface.AliasIpRanges = expandAliasIpRanges(d.Get(prefix + ".alias_ip_range").([]interface{}))
 
 			// Handle access_config structs
 			accessConfigsCount := d.Get(prefix + ".access_config.#").(int)
@@ -1038,6 +1064,7 @@ func resourceComputeInstanceRead(d *schema.ResourceData, meta interface{}) error
 				"subnetwork":         iface.Subnetwork,
 				"subnetwork_project": getProjectFromSubnetworkLink(iface.Subnetwork),
 				"access_config":      accessConfigs,
+				"alias_ip_range":     flattenAliasIpRange(iface.AliasIpRanges),
 			})
 		}
 	}
@@ -1580,6 +1607,18 @@ func expandGuestAccelerators(zone string, configs []interface{}) []*computeBeta.
 	return guestAccelerators
 }
 
+func expandAliasIpRanges(ranges []interface{}) []*computeBeta.AliasIpRange {
+	ipRanges := make([]*computeBeta.AliasIpRange, 0, len(ranges))
+	for _, raw := range ranges {
+		data := raw.(map[string]interface{})
+		ipRanges = append(ipRanges, &computeBeta.AliasIpRange{
+			IpCidrRange:         data["ip_cidr_range"].(string),
+			SubnetworkRangeName: data["subnetwork_range_name"].(string),
+		})
+	}
+	return ipRanges
+}
+
 func flattenGuestAccelerators(zone string, accelerators []*computeBeta.AcceleratorConfig) []map[string]interface{} {
 	acceleratorsSchema := make([]map[string]interface{}, 0, len(accelerators))
 	for _, accelerator := range accelerators {
@@ -1612,6 +1651,17 @@ func flattenBetaScheduling(scheduling *computeBeta.Scheduling) []map[string]inte
 	return result
 }
 
+func flattenAliasIpRange(ranges []*computeBeta.AliasIpRange) []map[string]interface{} {
+	rangesSchema := make([]map[string]interface{}, 0, len(ranges))
+	for _, ipRange := range ranges {
+		rangesSchema = append(rangesSchema, map[string]interface{}{
+			"ip_cidr_range":         ipRange.IpCidrRange,
+			"subnetwork_range_name": ipRange.SubnetworkRangeName,
+		})
+	}
+	return rangesSchema
+}
+
 func getProjectFromSubnetworkLink(subnetwork string) string {
 	r := regexp.MustCompile(SubnetworkLinkRegex)
 	if !r.MatchString(subnetwork) {

google/resource_compute_instance_test.go

@@ -749,7 +749,46 @@ func TestAccComputeInstance_minCpuPlatform(t *testing.T) {
 			},
 		},
 	})
+}
 
+func TestAccComputeInstance_primaryAliasIpRange(t *testing.T) {
+	var instance computeBeta.Instance
+	instanceName := fmt.Sprintf("terraform-test-%s", acctest.RandString(10))
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckComputeInstanceDestroy,
+		Steps: []resource.TestStep{
+			resource.TestStep{
+				Config: testAccComputeInstance_primaryAliasIpRange(instanceName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckComputeBetaInstanceExists("google_compute_instance.foobar", &instance),
+					testAccCheckComputeInstanceHasAliasIpRange(&instance, "", "/24"),
+				),
+			},
+		},
+	})
+}
+
+func TestAccComputeInstance_secondaryAliasIpRange(t *testing.T) {
+	var instance computeBeta.Instance
+	instanceName := fmt.Sprintf("terraform-test-%s", acctest.RandString(10))
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckComputeInstanceDestroy,
+		Steps: []resource.TestStep{
+			resource.TestStep{
+				Config: testAccComputeInstance_secondaryAliasIpRange(instanceName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckComputeBetaInstanceExists("google_compute_instance.foobar", &instance),
+					testAccCheckComputeInstanceHasAliasIpRange(&instance, "inst-test-secondary", "172.16.0.0/24"),
+				),
+			},
+		},
+	})
 }
 
 func testAccCheckComputeInstanceUpdateMachineType(n string) resource.TestCheckFunc {
@@ -1186,6 +1225,20 @@ func testAccCheckComputeInstanceHasMinCpuPlatform(instance *computeBeta.Instance
 	}
 }
 
+func testAccCheckComputeInstanceHasAliasIpRange(instance *computeBeta.Instance, subnetworkRangeName, iPCidrRange string) resource.TestCheckFunc {
+	return func(s *terraform.State) error {
+		for _, networkInterface := range instance.NetworkInterfaces {
+			for _, aliasIpRange := range networkInterface.AliasIpRanges {
+				if aliasIpRange.SubnetworkRangeName == subnetworkRangeName && (aliasIpRange.IpCidrRange == iPCidrRange || ipCidrRangeDiffSuppress("ip_cidr_range", aliasIpRange.IpCidrRange, iPCidrRange, nil)) {
+					return nil
+				}
+			}
+		}
+
+		return fmt.Errorf("Alias ip range with name %s and cidr %s not present", subnetworkRangeName, iPCidrRange)
+	}
+}
+
 func testAccComputeInstance_basic_deprecated_network(instance string) string {
 	return fmt.Sprintf(`
 resource "google_compute_instance" "foobar" {
@@ -2109,3 +2162,63 @@ resource "google_compute_instance" "foobar" {
   min_cpu_platform = "Intel Haswell"
 }`, instance)
 }
+
+func testAccComputeInstance_primaryAliasIpRange(instance string) string {
+	return fmt.Sprintf(`
+resource "google_compute_instance" "foobar" {
+  name = "%s"
+  machine_type = "n1-standard-1"
+  zone = "us-east1-d"
+
+  boot_disk {
+    initialize_params {
+      image = "debian-8-jessie-v20160803"
+    }
+  }
+
+  network_interface {
+    network = "default"
+
+    alias_ip_range {
+      ip_cidr_range = "/24"
+    }
+  }
+}`, instance)
+}
+
+func testAccComputeInstance_secondaryAliasIpRange(instance string) string {
+	return fmt.Sprintf(`
+resource "google_compute_network" "inst-test-network" {
+	name = "inst-test-network-%s"
+}
+resource "google_compute_subnetwork" "inst-test-subnetwork" {
+	name          = "inst-test-subnetwork-%s"
+	ip_cidr_range = "10.0.0.0/16"
+	region        = "us-east1"
+	network       = "${google_compute_network.inst-test-network.self_link}"
+	secondary_ip_range {
+		range_name = "inst-test-secondary"
+		ip_cidr_range = "172.16.0.0/20"
+	}
+}
+resource "google_compute_instance" "foobar" {
+  name = "%s"
+  machine_type = "n1-standard-1"
+  zone = "us-east1-d"
+
+  boot_disk {
+    initialize_params {
+      image = "debian-8-jessie-v20160803"
+    }
+  }
+
+  network_interface {
+    subnetwork = "${google_compute_subnetwork.inst-test-subnetwork.self_link}"
+
+    alias_ip_range {
+      subnetwork_range_name = "${google_compute_subnetwork.inst-test-subnetwork.secondary_ip_range.0.range_name}"
+      ip_cidr_range = "172.16.0.0/24"
+    }
+  }
+}`, acctest.RandString(10), acctest.RandString(10), instance)
+}

google/resource_compute_subnetwork.go

@@ -46,7 +46,7 @@ func resourceComputeSubnetwork() *schema.Resource {
 				Type:             schema.TypeString,
 				Required:         true,
 				ForceNew:         true,
-				DiffSuppressFunc: compareGlobalSelfLinkOrResourceName,
+				DiffSuppressFunc: compareSelfLinkOrResourceName,
 			},
 
 			"description": &schema.Schema{

google/self_link_helpers.go

@@ -28,15 +28,21 @@ func compareSelfLinkRelativePaths(k, old, new string, d *schema.ResourceData) bo
 	return false
 }
 
-// Use this method when the field accepts either a name or a self_link referencing a global resource.
+// Use this method when the field accepts either a name or a self_link referencing a resource.
-func compareGlobalSelfLinkOrResourceName(k, old, new string, d *schema.ResourceData) bool {
+// The value we store (i.e. `old` in this method), must be a self_link.
-	oldParts := strings.Split(old, "/")
+func compareSelfLinkOrResourceName(k, old, new string, d *schema.ResourceData) bool {
+	oldParts := strings.Split(old, "/") // always a self_link
 	newParts := strings.Split(new, "/")
 
-	if oldParts[len(oldParts)-1] == newParts[len(newParts)-1] {
+	if len(newParts) == 1 {
-		return true
+		// The `new` string is a name
+		if oldParts[len(oldParts)-1] == newParts[0] {
+			return true
+		}
 	}
-	return false
+
+	// The `new` string is a self_link
+	return compareSelfLinkRelativePaths(k, old, new, d)
 }
 
 // Hash the relative path of a self link.

google/utils.go

@@ -258,6 +258,30 @@ func linkDiffSuppress(k, old, new string, d *schema.ResourceData) bool {
 	return false
 }
 
+func ipCidrRangeDiffSuppress(k, old, new string, d *schema.ResourceData) bool {
+	// The range may be a:
+	// A) single IP address (e.g. 10.2.3.4)
+	// B) CIDR format string (e.g. 10.1.2.0/24)
+	// C) netmask (e.g. /24)
+	//
+	// For A) and B), no diff to suppress, they have to match completely.
+	// For C), The API picks a network IP address and this creates a diff of the form:
+	// network_interface.0.alias_ip_range.0.ip_cidr_range: "10.128.1.0/24" => "/24"
+	// We should only compare the mask portion for this case.
+	if len(new) > 0 && new[0] == '/' {
+		oldNetmaskStartPos := strings.LastIndex(old, "/")
+
+		if oldNetmaskStartPos != -1 {
+			oldNetmask := old[strings.LastIndex(old, "/"):]
+			if oldNetmask == new {
+				return true
+			}
+		}
+	}
+
+	return false
+}
+
 // expandLabels pulls the value of "labels" out of a schema.ResourceData as a map[string]string.
 func expandLabels(d *schema.ResourceData) map[string]string {
 	return expandStringMap(d, "labels")

google/utils_test.go

@@ -0,0 +1,47 @@
+package google
+
+import "testing"
+
+func TestIpCidrRangeDiffSuppress(t *testing.T) {
+	cases := map[string]struct {
+		Old, New          string
+		ExpectDiffSupress bool
+	}{
+		"single ip address": {
+			Old:               "10.2.3.4",
+			New:               "10.2.3.5",
+			ExpectDiffSupress: false,
+		},
+		"cidr format string": {
+			Old:               "10.1.2.0/24",
+			New:               "10.1.3.0/24",
+			ExpectDiffSupress: false,
+		},
+		"netmask same mask": {
+			Old:               "10.1.2.0/24",
+			New:               "/24",
+			ExpectDiffSupress: true,
+		},
+		"netmask different mask": {
+			Old:               "10.1.2.0/24",
+			New:               "/32",
+			ExpectDiffSupress: false,
+		},
+		"add netmask": {
+			Old:               "",
+			New:               "/24",
+			ExpectDiffSupress: false,
+		},
+		"remove netmask": {
+			Old:               "/24",
+			New:               "",
+			ExpectDiffSupress: false,
+		},
+	}
+
+	for tn, tc := range cases {
+		if ipCidrRangeDiffSuppress("ip_cidr_range", tc.Old, tc.New, nil) != tc.ExpectDiffSupress {
+			t.Fatalf("bad: %s, '%s' => '%s' expect %t", tn, tc.Old, tc.New, tc.ExpectDiffSupress)
+		}
+	}
+}

website/docs/r/compute_instance.html.markdown

@@ -229,11 +229,26 @@ The `network_interface` block supports:
     on that network). This block can be repeated multiple times. Structure
     documented below.
 
+* `alias_ip_range` - (Optional, [Beta](/docs/providers/google/index.html#beta-features)) An
+    array of alias IP ranges for this network interface. Can only be specified for network
+    interfaces on subnet-mode networks. Structure documented below.
+
 The `access_config` block supports:
 
 * `nat_ip` - (Optional) The IP address that will be 1:1 mapped to the instance's
     network ip. If not given, one will be generated.
 
+The `alias_ip_range` block supports:
+
+* `ip_cidr_range` - The IP CIDR range represented by this alias IP range. This IP CIDR range
+    must belong to the specified subnetwork and cannot contain IP addresses reserved by
+    system or used by other network interfaces. This range may be a single IP address
+    (e.g. 10.2.3.4), a netmask (e.g. /24) or a CIDR format string (e.g. 10.1.2.0/24).
+
+* `subnetwork_range_name` - (Optional) The subnetwork secondary range name specifying
+    the secondary range from which to allocate the IP CIDR range for this alias IP
+    range. If left unspecified, the primary range of the subnetwork will be used.
+
 The `service_account` block supports:
 
 * `email` - (Optional) The service account e-mail address. If not given, the