Merge pull request #5255 from hashicorp/phinze/improve-vpc-cidr-errmsg

provider/aws: improve vpc cidr_block err message

builtin/providers/aws/resource_aws_vpc.go

@@ -3,7 +3,6 @@ package aws
 import (
 	"fmt"
 	"log"
-	"net"
 	"time"
 
 	"github.com/aws/aws-sdk-go/aws"
@@ -22,19 +21,10 @@ func resourceAwsVpc() *schema.Resource {
 
 		Schema: map[string]*schema.Schema{
 			"cidr_block": &schema.Schema{
-				Type:     schema.TypeString,
-				Required: true,
-				ForceNew: true,
-				ValidateFunc: func(v interface{}, k string) (ws []string, errors []error) {
-					value := v.(string)
-					_, ipnet, err := net.ParseCIDR(value)
-
-					if err != nil || ipnet == nil || value != ipnet.String() {
-						errors = append(errors, fmt.Errorf(
-							"%q must contain a valid CIDR", k))
-					}
-					return
-				},
+				Type:         schema.TypeString,
+				Required:     true,
+				ForceNew:     true,
+				ValidateFunc: validateCIDRNetworkAddress,
 			},
 
 			"instance_tenancy": &schema.Schema{

builtin/providers/aws/validators.go

@@ -2,6 +2,7 @@ package aws
 
 import (
 	"fmt"
+	"net"
 	"regexp"
 	"time"
 
@@ -278,3 +279,23 @@ func validatePolicyStatementId(v interface{}, k string) (ws []string, errors []e
 
 	return
 }
+
+// validateCIDRNetworkAddress ensures that the string value is a valid CIDR that
+// represents a network address - it adds an error otherwise
+func validateCIDRNetworkAddress(v interface{}, k string) (ws []string, errors []error) {
+	value := v.(string)
+	_, ipnet, err := net.ParseCIDR(value)
+	if err != nil {
+		errors = append(errors, fmt.Errorf(
+			"%q must contain a valid CIDR, got error parsing: %s", k, err))
+		return
+	}
+
+	if ipnet == nil || value != ipnet.String() {
+		errors = append(errors, fmt.Errorf(
+			"%q must contain a valid network CIDR, expected %q, got %q",
+			k, ipnet, value))
+	}
+
+	return
+}

builtin/providers/aws/validators_test.go

@@ -1,6 +1,7 @@
 package aws
 
 import (
+	"strings"
 	"testing"
 )
 
@@ -243,3 +244,33 @@ func TestValidatePolicyStatementId(t *testing.T) {
 		}
 	}
 }
+
+func TestValidateCIDRNetworkAddress(t *testing.T) {
+	cases := []struct {
+		CIDR              string
+		ExpectedErrSubstr string
+	}{
+		{"notacidr", `must contain a valid CIDR`},
+		{"10.0.1.0/16", `must contain a valid network CIDR`},
+		{"10.0.1.0/24", ``},
+	}
+
+	for i, tc := range cases {
+		_, errs := validateCIDRNetworkAddress(tc.CIDR, "foo")
+		if tc.ExpectedErrSubstr == "" {
+			if len(errs) != 0 {
+				t.Fatalf("%d/%d: Expected no error, got errs: %#v",
+					i+1, len(cases), errs)
+			}
+		} else {
+			if len(errs) != 1 {
+				t.Fatalf("%d/%d: Expected 1 err containing %q, got %d errs",
+					i+1, len(cases), tc.ExpectedErrSubstr, len(errs))
+			}
+			if !strings.Contains(errs[0].Error(), tc.ExpectedErrSubstr) {
+				t.Fatalf("%d/%d: Expected err: %q, to include %q",
+					i+1, len(cases), errs[0], tc.ExpectedErrSubstr)
+			}
+		}
+	}
+}