From ce74e3d8f04194c5ddbdebbf299db0d2849e1748 Mon Sep 17 00:00:00 2001
From: Paul Hinze <phinze@phinze.com>
Date: Mon, 22 Feb 2016 15:05:55 -0600
Subject: [PATCH] provider/aws: improve vpc cidr_block err message

Pull CIDR block validation into a shared func ready to be used elsewhere

Example of new err message:

```
Errors:

  * aws_vpc.foo: "cidr_block" must contain a valid network CIDR,
    expected "10.0.0.0/16", got "10.0.1.0/16"
```
---
 builtin/providers/aws/resource_aws_vpc.go | 18 +++----------
 builtin/providers/aws/validators.go       | 21 +++++++++++++++
 builtin/providers/aws/validators_test.go  | 31 +++++++++++++++++++++++
 3 files changed, 56 insertions(+), 14 deletions(-)

diff --git a/builtin/providers/aws/resource_aws_vpc.go b/builtin/providers/aws/resource_aws_vpc.go
index 768d96aac..5a71138b8 100644
--- a/builtin/providers/aws/resource_aws_vpc.go
+++ b/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{
diff --git a/builtin/providers/aws/validators.go b/builtin/providers/aws/validators.go
index 101ab8bd1..6ff09a4a3 100644
--- a/builtin/providers/aws/validators.go
+++ b/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
+}
diff --git a/builtin/providers/aws/validators_test.go b/builtin/providers/aws/validators_test.go
index 4f8cfd481..0cca92c4f 100644
--- a/builtin/providers/aws/validators_test.go
+++ b/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)
+			}
+		}
+	}
+}