Merge pull request #11993 from hashicorp/b-fix-cloudflare-validator

provider/cloudflare: Fix record validation

builtin/providers/cloudflare/resource_cloudflare_record.go

@@ -18,51 +18,50 @@ func resourceCloudFlareRecord() *schema.Resource {
 		SchemaVersion: 1,
 		MigrateState:  resourceCloudFlareRecordMigrateState,
 		Schema: map[string]*schema.Schema{
-			"domain": &schema.Schema{
+			"domain": {
 				Type:     schema.TypeString,
 				Required: true,
 			},
 
-			"name": &schema.Schema{
+			"name": {
 				Type:     schema.TypeString,
 				Required: true,
 			},
 
-			"hostname": &schema.Schema{
+			"hostname": {
 				Type:     schema.TypeString,
 				Computed: true,
 			},
 
-			"type": &schema.Schema{
-				Type:         schema.TypeString,
-				Required:     true,
-				ForceNew:     true,
-				ValidateFunc: validateRecordType,
+			"type": {
+				Type:     schema.TypeString,
+				Required: true,
+				ForceNew: true,
 			},
 
-			"value": &schema.Schema{
+			"value": {
 				Type:     schema.TypeString,
 				Required: true,
 			},
 
-			"ttl": &schema.Schema{
+			"ttl": {
 				Type:     schema.TypeInt,
 				Optional: true,
 				Computed: true,
 			},
 
-			"priority": &schema.Schema{
+			"priority": {
 				Type:     schema.TypeInt,
 				Optional: true,
 			},
 
-			"proxied": &schema.Schema{
+			"proxied": {
 				Default:  false,
 				Optional: true,
 				Type:     schema.TypeBool,
 			},
 
-			"zone_id": &schema.Schema{
+			"zone_id": {
 				Type:     schema.TypeString,
 				Computed: true,
 			},
@@ -94,6 +93,11 @@ func resourceCloudFlareRecordCreate(d *schema.ResourceData, meta interface{}) er
 		return fmt.Errorf("Error validating record name %q: %s", newRecord.Name, err)
 	}
 
+	// Validate type
+	if err := validateRecordType(newRecord.Type, newRecord.Proxied); err != nil {
+		return fmt.Errorf("Error validating record type %q: %s", newRecord.Type, err)
+	}
+
 	zoneId, err := client.ZoneIDByName(newRecord.ZoneName)
 	if err != nil {
 		return fmt.Errorf("Error finding zone %q: %s", newRecord.ZoneName, err)

builtin/providers/cloudflare/validators.go

@@ -7,26 +7,44 @@ import (
 )
 
 // validateRecordType ensures that the cloudflare record type is valid
-func validateRecordType(v interface{}, k string) (ws []string, errors []error) {
-	value := v.(string)
-
-	validTypes := map[string]struct{}{
-		"A":     {},
-		"AAAA":  {},
-		"CNAME": {},
-		"TXT":   {},
-		"SRV":   {},
-		"LOC":   {},
-		"MX":    {},
-		"NS":    {},
-		"SPF":   {},
+func validateRecordType(t string, proxied bool) error {
+	switch t {
+	case "A":
+		return nil
+	case "AAAA":
+		return nil
+	case "CNAME":
+		return nil
+	case "TXT":
+		if !proxied {
+			return nil
+		}
+	case "SRV":
+		if !proxied {
+			return nil
+		}
+	case "LOC":
+		if !proxied {
+			return nil
+		}
+	case "MX":
+		if !proxied {
+			return nil
+		}
+	case "NS":
+		if !proxied {
+			return nil
+		}
+	case "SPF":
+		if !proxied {
+			return nil
+		}
+	default:
+		return fmt.Errorf(
+			`Invalid type %q. Valid types are "A", "AAAA", "CNAME", "TXT", "SRV", "LOC", "MX", "NS" or "SPF"`, t)
 	}
 
-	if _, ok := validTypes[value]; !ok {
-		errors = append(errors, fmt.Errorf(
-			`%q contains an invalid type %q. Valid types are "A", "AAAA", "CNAME", "TXT", "SRV", "LOC", "MX", "NS" or "SPF"`, k, value))
-	}
-	return
+	return fmt.Errorf("Type %q cannot be proxied", t)
 }
 
 // validateRecordName ensures that based on supplied record type, the name content matches

builtin/providers/cloudflare/validators_test.go

@@ -3,36 +3,38 @@ package cloudflare
 import "testing"
 
 func TestValidateRecordType(t *testing.T) {
-	validTypes := []string{
-		"A",
-		"AAAA",
-		"CNAME",
-		"TXT",
-		"SRV",
-		"LOC",
-		"MX",
-		"NS",
-		"SPF",
+	validTypes := map[string]bool{
+		"A":     true,
+		"AAAA":  true,
+		"CNAME": true,
+		"TXT":   false,
+		"SRV":   false,
+		"LOC":   false,
+		"MX":    false,
+		"NS":    false,
+		"SPF":   false,
 	}
-	for _, v := range validTypes {
-		_, errors := validateRecordType(v, "type")
-		if len(errors) != 0 {
-			t.Fatalf("%q should be a valid record type: %q", v, errors)
+	for k, v := range validTypes {
+		err := validateRecordType(k, v)
+		if err != nil {
+			t.Fatalf("%s should be a valid record type: %s", k, err)
 		}
 	}
 
-	invalidTypes := []string{
-		"a",
-		"cName",
-		"txt",
-		"SRv",
-		"foo",
-		"bar",
+	invalidTypes := map[string]bool{
+		"a":     false,
+		"cName": false,
+		"txt":   false,
+		"SRv":   false,
+		"foo":   false,
+		"bar":   false,
+		"TXT":   true,
+		"SRV":   true,
+		"SPF":   true,
 	}
-	for _, v := range invalidTypes {
-		_, errors := validateRecordType(v, "type")
-		if len(errors) == 0 {
-			t.Fatalf("%q should be an invalid record type", v)
+	for k, v := range invalidTypes {
+		if err := validateRecordType(k, v); err == nil {
+			t.Fatalf("%s should be an invalid record type", k)
 		}
 	}
 }