Support subnetwork IP CIDR range expansion (#945)

* Vendor schema/helper/customdiff

* Support subnetwork IP CIDR range expansion

* Change wording

google/resource_compute_subnetwork.go

@@ -6,9 +6,12 @@ import (
 
 	"strings"
 
+	"github.com/apparentlymart/go-cidr/cidr"
+	"github.com/hashicorp/terraform/helper/customdiff"
 	"github.com/hashicorp/terraform/helper/schema"
 	computeBeta "google.golang.org/api/compute/v0.beta"
 	"google.golang.org/api/compute/v1"
+	"net"
 	"time"
 )
 
@@ -37,9 +40,10 @@ func resourceComputeSubnetwork() *schema.Resource {
 
 		Schema: map[string]*schema.Schema{
 			"ip_cidr_range": &schema.Schema{
-				Type:     schema.TypeString,
-				Required: true,
-				ForceNew: true,
+				Type:         schema.TypeString,
+				Required:     true,
+				ValidateFunc: validateIpCidrRange,
+				// ForceNew only if it shrinks the CIDR range, this is set in CustomizeDiff below.
 			},
 
 			"name": &schema.Schema{
@@ -113,6 +117,10 @@ func resourceComputeSubnetwork() *schema.Resource {
 				Computed: true,
 			},
 		},
+
+		CustomizeDiff: customdiff.All(
+			customdiff.ForceNewIfChange("ip_cidr_range", isShrinkageIpCidr),
+		),
 	}
 }
 
@@ -279,6 +287,25 @@ func resourceComputeSubnetworkUpdate(d *schema.ResourceData, meta interface{}) e
 		d.SetPartial("private_ip_google_access")
 	}
 
+	if d.HasChange("ip_cidr_range") {
+		r := &compute.SubnetworksExpandIpCidrRangeRequest{
+			IpCidrRange: d.Get("ip_cidr_range").(string),
+		}
+
+		op, err := config.clientCompute.Subnetworks.ExpandIpCidrRange(project, region, d.Get("name").(string), r).Do()
+
+		if err != nil {
+			return fmt.Errorf("Error expanding the ip cidr range: %s", err)
+		}
+
+		err = computeSharedOperationWaitTime(config.clientCompute, op, project, int(d.Timeout(schema.TimeoutUpdate).Minutes()), "Expanding Subnetwork IP CIDR range")
+		if err != nil {
+			return err
+		}
+
+		d.SetPartial("ip_cidr_range")
+	}
+
 	if d.HasChange("secondary_ip_range") && computeApiVersion == v0beta {
 		v0BetaSubnetwork := &computeBeta.Subnetwork{
 			SecondaryIpRanges: expandSecondaryRangesV0Beta(d.Get("secondary_ip_range").([]interface{})),
@@ -415,3 +442,23 @@ func flattenSecondaryRangesV0Beta(secondaryRanges []*computeBeta.SubnetworkSecon
 	}
 	return secondaryRangesSchema
 }
+
+// Whether the IP CIDR change shrinks the block.
+func isShrinkageIpCidr(old, new, _ interface{}) bool {
+	_, oldCidr, oldErr := net.ParseCIDR(old.(string))
+	_, newCidr, newErr := net.ParseCIDR(new.(string))
+
+	if oldErr != nil || newErr != nil {
+		// This should never happen. The ValidateFunc on the field ensures it.
+		return false
+	}
+
+	oldStart, oldEnd := cidr.AddressRange(oldCidr)
+
+	if newCidr.Contains(oldStart) && newCidr.Contains(oldEnd) {
+		// This is a CIDR range expansion, no need to ForceNew, we have an update method for it.
+		return false
+	}
+
+	return true
+}

google/resource_compute_subnetwork_test.go

@@ -10,6 +10,44 @@ import (
 	"google.golang.org/api/compute/v1"
 )
 
+// Unit tests
+
+func TestIsShrinkageIpCidr(t *testing.T) {
+	cases := map[string]struct {
+		Old, New  string
+		Shrinkage bool
+	}{
+		"Expansion same network ip": {
+			Old:       "10.0.0.0/24",
+			New:       "10.0.0.0/16",
+			Shrinkage: false,
+		},
+		"Expansion different network ip": {
+			Old:       "10.0.1.0/24",
+			New:       "10.0.0.0/16",
+			Shrinkage: false,
+		},
+		"Shrinkage same network ip": {
+			Old:       "10.0.0.0/16",
+			New:       "10.0.0.0/24",
+			Shrinkage: true,
+		},
+		"Shrinkage different network ip": {
+			Old:       "10.0.0.0/16",
+			New:       "10.1.0.0/16",
+			Shrinkage: true,
+		},
+	}
+
+	for tn, tc := range cases {
+		if isShrinkageIpCidr(tc.Old, tc.New, nil) != tc.Shrinkage {
+			t.Errorf("%s failed: Shrinkage should be %t", tn, tc.Shrinkage)
+		}
+	}
+}
+
+// Acceptance tests
+
 func TestAccComputeSubnetwork_basic(t *testing.T) {
 	t.Parallel()
 
@@ -63,14 +101,23 @@ func TestAccComputeSubnetwork_update(t *testing.T) {
 		CheckDestroy: testAccCheckComputeSubnetworkDestroy,
 		Steps: []resource.TestStep{
 			resource.TestStep{
-				Config: testAccComputeSubnetwork_update1(cnName, subnetworkName),
+				Config: testAccComputeSubnetwork_update1(cnName, "10.2.0.0/24", subnetworkName),
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckComputeSubnetworkExists(
 						"google_compute_subnetwork.network-with-private-google-access", &subnetwork),
 				),
 			},
 			resource.TestStep{
-				Config: testAccComputeSubnetwork_update2(cnName, subnetworkName),
+				// Expand IP CIDR range and update private_ip_google_access
+				Config: testAccComputeSubnetwork_update2(cnName, "10.2.0.0/16", subnetworkName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckComputeSubnetworkExists(
+						"google_compute_subnetwork.network-with-private-google-access", &subnetwork),
+				),
+			},
+			resource.TestStep{
+				// Shrink IP CIDR range and update private_ip_google_access
+				Config: testAccComputeSubnetwork_update2(cnName, "10.2.0.0/24", subnetworkName),
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckComputeSubnetworkExists(
 						"google_compute_subnetwork.network-with-private-google-access", &subnetwork),
@@ -234,7 +281,7 @@ resource "google_compute_subnetwork" "network-with-private-google-access" {
 `, cnName, subnetwork1Name, subnetwork2Name, subnetwork3Name)
 }
 
-func testAccComputeSubnetwork_update1(cnName, subnetworkName string) string {
+func testAccComputeSubnetwork_update1(cnName, cidrRange, subnetworkName string) string {
 	return fmt.Sprintf(`
 resource "google_compute_network" "custom-test" {
 	name = "%s"
@@ -243,15 +290,15 @@ resource "google_compute_network" "custom-test" {
 
 resource "google_compute_subnetwork" "network-with-private-google-access" {
 	name = "%s"
-	ip_cidr_range = "10.2.0.0/16"
+	ip_cidr_range = "%s"
 	region = "us-central1"
 	network = "${google_compute_network.custom-test.self_link}"
 	private_ip_google_access = true
 }
-`, cnName, subnetworkName)
+`, cnName, subnetworkName, cidrRange)
 }
 
-func testAccComputeSubnetwork_update2(cnName, subnetworkName string) string {
+func testAccComputeSubnetwork_update2(cnName, cidrRange, subnetworkName string) string {
 	return fmt.Sprintf(`
 resource "google_compute_network" "custom-test" {
 	name = "%s"
@@ -260,11 +307,11 @@ resource "google_compute_network" "custom-test" {
 
 resource "google_compute_subnetwork" "network-with-private-google-access" {
 	name = "%s"
-	ip_cidr_range = "10.2.0.0/16"
+	ip_cidr_range = "%s"
 	region = "us-central1"
 	network = "${google_compute_network.custom-test.self_link}"
 }
-`, cnName, subnetworkName)
+`, cnName, subnetworkName, cidrRange)
 }
 
 func testAccComputeSubnetwork_secondaryIpRanges_update1(cnName, subnetworkName string) string {

google/validation.go

@@ -107,3 +107,11 @@ func validateRFC1035Name(min, max int) schema.SchemaValidateFunc {
 
 	return validateRegexp(fmt.Sprintf("^"+RFC1035NameTemplate+"$", min-2, max-2))
 }
+
+func validateIpCidrRange(v interface{}, k string) (warnings []string, errors []error) {
+	_, _, err := net.ParseCIDR(v.(string))
+	if err != nil {
+		errors = append(errors, fmt.Errorf("%q is not a valid IP CIDR range: %s", k, err))
+	}
+	return
+}

vendor/github.com/hashicorp/terraform/helper/customdiff/compose.go

@@ -0,0 +1,72 @@
+package customdiff
+
+import (
+	"github.com/hashicorp/go-multierror"
+	"github.com/hashicorp/terraform/helper/schema"
+)
+
+// All returns a CustomizeDiffFunc that runs all of the given
+// CustomizeDiffFuncs and returns all of the errors produced.
+//
+// If one function produces an error, functions after it are still run.
+// If this is not desirable, use function Sequence instead.
+//
+// If multiple functions returns errors, the result is a multierror.
+//
+// For example:
+//
+//     &schema.Resource{
+//         // ...
+//         CustomizeDiff: customdiff.All(
+//             customdiff.ValidateChange("size", func (old, new, meta interface{}) error {
+//                 // If we are increasing "size" then the new value must be
+//                 // a multiple of the old value.
+//                 if new.(int) <= old.(int) {
+//                     return nil
+//                 }
+//                 if (new.(int) % old.(int)) != 0 {
+//                     return fmt.Errorf("new size value must be an integer multiple of old value %d", old.(int))
+//                 }
+//                 return nil
+//             }),
+//             customdiff.ForceNewIfChange("size", func (old, new, meta interface{}) bool {
+//                 // "size" can only increase in-place, so we must create a new resource
+//                 // if it is decreased.
+//                 return new.(int) < old.(int)
+//             }),
+//             customdiff.ComputedIf("version_id", func (d *schema.ResourceDiff, meta interface{}) bool {
+//                 // Any change to "content" causes a new "version_id" to be allocated.
+//                 return d.HasChange("content")
+//             }),
+//         ),
+//     }
+//
+func All(funcs ...schema.CustomizeDiffFunc) schema.CustomizeDiffFunc {
+	return func(d *schema.ResourceDiff, meta interface{}) error {
+		var err error
+		for _, f := range funcs {
+			thisErr := f(d, meta)
+			if thisErr != nil {
+				err = multierror.Append(err, thisErr)
+			}
+		}
+		return err
+	}
+}
+
+// Sequence returns a CustomizeDiffFunc that runs all of the given
+// CustomizeDiffFuncs in sequence, stopping at the first one that returns
+// an error and returning that error.
+//
+// If all functions succeed, the combined function also succeeds.
+func Sequence(funcs ...schema.CustomizeDiffFunc) schema.CustomizeDiffFunc {
+	return func(d *schema.ResourceDiff, meta interface{}) error {
+		for _, f := range funcs {
+			err := f(d, meta)
+			if err != nil {
+				return err
+			}
+		}
+		return nil
+	}
+}

vendor/github.com/hashicorp/terraform/helper/customdiff/computed.go

@@ -0,0 +1,16 @@
+package customdiff
+
+import (
+	"github.com/hashicorp/terraform/helper/schema"
+)
+
+// ComputedIf returns a CustomizeDiffFunc that sets the given key's new value
+// as computed if the given condition function returns true.
+func ComputedIf(key string, f ResourceConditionFunc) schema.CustomizeDiffFunc {
+	return func(d *schema.ResourceDiff, meta interface{}) error {
+		if f(d, meta) {
+			d.SetNewComputed(key)
+		}
+		return nil
+	}
+}

vendor/github.com/hashicorp/terraform/helper/customdiff/condition.go

@@ -0,0 +1,60 @@
+package customdiff
+
+import (
+	"github.com/hashicorp/terraform/helper/schema"
+)
+
+// ResourceConditionFunc is a function type that makes a boolean decision based
+// on an entire resource diff.
+type ResourceConditionFunc func(d *schema.ResourceDiff, meta interface{}) bool
+
+// ValueChangeConditionFunc is a function type that makes a boolean decision
+// by comparing two values.
+type ValueChangeConditionFunc func(old, new, meta interface{}) bool
+
+// ValueConditionFunc is a function type that makes a boolean decision based
+// on a given value.
+type ValueConditionFunc func(value, meta interface{}) bool
+
+// If returns a CustomizeDiffFunc that calls the given condition
+// function and then calls the given CustomizeDiffFunc only if the condition
+// function returns true.
+//
+// This can be used to include conditional customizations when composing
+// customizations using All and Sequence, but should generally be used only in
+// simple scenarios. Prefer directly writing a CustomizeDiffFunc containing
+// a conditional branch if the given CustomizeDiffFunc is already a
+// locally-defined function, since this avoids obscuring the control flow.
+func If(cond ResourceConditionFunc, f schema.CustomizeDiffFunc) schema.CustomizeDiffFunc {
+	return func(d *schema.ResourceDiff, meta interface{}) error {
+		if cond(d, meta) {
+			return f(d, meta)
+		}
+		return nil
+	}
+}
+
+// IfValueChange returns a CustomizeDiffFunc that calls the given condition
+// function with the old and new values of the given key and then calls the
+// given CustomizeDiffFunc only if the condition function returns true.
+func IfValueChange(key string, cond ValueChangeConditionFunc, f schema.CustomizeDiffFunc) schema.CustomizeDiffFunc {
+	return func(d *schema.ResourceDiff, meta interface{}) error {
+		old, new := d.GetChange(key)
+		if cond(old, new, meta) {
+			return f(d, meta)
+		}
+		return nil
+	}
+}
+
+// IfValue returns a CustomizeDiffFunc that calls the given condition
+// function with the new values of the given key and then calls the
+// given CustomizeDiffFunc only if the condition function returns true.
+func IfValue(key string, cond ValueConditionFunc, f schema.CustomizeDiffFunc) schema.CustomizeDiffFunc {
+	return func(d *schema.ResourceDiff, meta interface{}) error {
+		if cond(d.Get(key), meta) {
+			return f(d, meta)
+		}
+		return nil
+	}
+}

vendor/github.com/hashicorp/terraform/helper/customdiff/doc.go

@@ -0,0 +1,11 @@
+// Package customdiff provides a set of reusable and composable functions
+// to enable more "declarative" use of the CustomizeDiff mechanism available
+// for resources in package helper/schema.
+//
+// The intent of these helpers is to make the intent of a set of diff
+// customizations easier to see, rather than lost in a sea of Go function
+// boilerplate. They should _not_ be used in situations where they _obscure_
+// intent, e.g. by over-using the composition functions where a single
+// function containing normal Go control flow statements would be more
+// straightforward.
+package customdiff

vendor/github.com/hashicorp/terraform/helper/customdiff/force_new.go

@@ -0,0 +1,40 @@
+package customdiff
+
+import (
+	"github.com/hashicorp/terraform/helper/schema"
+)
+
+// ForceNewIf returns a CustomizeDiffFunc that flags the given key as
+// requiring a new resource if the given condition function returns true.
+//
+// The return value of the condition function is ignored if the old and new
+// values of the field compare equal, since no attribute diff is generated in
+// that case.
+func ForceNewIf(key string, f ResourceConditionFunc) schema.CustomizeDiffFunc {
+	return func(d *schema.ResourceDiff, meta interface{}) error {
+		if f(d, meta) {
+			d.ForceNew(key)
+		}
+		return nil
+	}
+}
+
+// ForceNewIfChange returns a CustomizeDiffFunc that flags the given key as
+// requiring a new resource if the given condition function returns true.
+//
+// The return value of the condition function is ignored if the old and new
+// values compare equal, since no attribute diff is generated in that case.
+//
+// This function is similar to ForceNewIf but provides the condition function
+// only the old and new values of the given key, which leads to more compact
+// and explicit code in the common case where the decision can be made with
+// only the specific field value.
+func ForceNewIfChange(key string, f ValueChangeConditionFunc) schema.CustomizeDiffFunc {
+	return func(d *schema.ResourceDiff, meta interface{}) error {
+		old, new := d.GetChange(key)
+		if f(old, new, meta) {
+			d.ForceNew(key)
+		}
+		return nil
+	}
+}

vendor/github.com/hashicorp/terraform/helper/customdiff/validate.go

@@ -0,0 +1,38 @@
+package customdiff
+
+import (
+	"github.com/hashicorp/terraform/helper/schema"
+)
+
+// ValueChangeValidationFunc is a function type that validates the difference
+// (or lack thereof) between two values, returning an error if the change
+// is invalid.
+type ValueChangeValidationFunc func(old, new, meta interface{}) error
+
+// ValueValidationFunc is a function type that validates a particular value,
+// returning an error if the value is invalid.
+type ValueValidationFunc func(value, meta interface{}) error
+
+// ValidateChange returns a CustomizeDiffFunc that applies the given validation
+// function to the change for the given key, returning any error produced.
+func ValidateChange(key string, f ValueChangeValidationFunc) schema.CustomizeDiffFunc {
+	return func(d *schema.ResourceDiff, meta interface{}) error {
+		old, new := d.GetChange(key)
+		return f(old, new, meta)
+	}
+}
+
+// ValidateValue returns a CustomizeDiffFunc that applies the given validation
+// function to value of the given key, returning any error produced.
+//
+// This should generally not be used since it is functionally equivalent to
+// a validation function applied directly to the schema attribute in question,
+// but is provided for situations where composing multiple CustomizeDiffFuncs
+// together makes intent clearer than spreading that validation across the
+// schema.
+func ValidateValue(key string, f ValueValidationFunc) schema.CustomizeDiffFunc {
+	return func(d *schema.ResourceDiff, meta interface{}) error {
+		val := d.Get(key)
+		return f(val, meta)
+	}
+}

vendor/vendor.json

@@ -600,6 +600,14 @@
 			"version": "v0.11.2",
 			"versionExact": "v0.11.2"
 		},
+		{
+			"checksumSHA1": "qVmQPoZmJ2w2OnaxIheWfuwun6g=",
+			"path": "github.com/hashicorp/terraform/helper/customdiff",
+			"revision": "a6008b8a48a749c7c167453b9cf55ffd572b9a5d",
+			"revisionTime": "2018-01-09T23:13:33Z",
+			"version": "v0.11.2",
+			"versionExact": "v0.11.2"
+		},
 		{
 			"checksumSHA1": "FH5eOEHfHgdxPC/JnfmCeSBk66U=",
 			"path": "github.com/hashicorp/terraform/helper/encryption",