validate + test lb with duplicate pool locations

cloudflare/resource_cloudflare_load_balancer.go

@@ -100,7 +100,6 @@ func resourceCloudFlareLoadBalancer() *schema.Resource {
 						},
 					},
 				},
-				Set: HashByMapKey("pop"),
 			},
 
 			"region_pools": {
@@ -161,11 +160,19 @@ func resourceCloudFlareLoadBalancerCreate(d *schema.ResourceData, meta interface
 	}
 
 	if regionPools, ok := d.GetOk("region_pools"); ok {
-		newLoadBalancer.RegionPools = expandGeoPools(regionPools, "region")
+		expandedRegionPools, err := expandGeoPools(regionPools, "region")
+		if err != nil {
+			return err
+		}
+		newLoadBalancer.RegionPools = expandedRegionPools
 	}
 
 	if popPools, ok := d.GetOk("pop_pools"); ok {
-		newLoadBalancer.PopPools = expandGeoPools(popPools, "pop")
+		expandedPopPools, err := expandGeoPools(popPools, "pop")
+		if err != nil {
+			return err
+		}
+		newLoadBalancer.PopPools = expandedPopPools
 	}
 
 	zoneName := d.Get("zone").(string)
@@ -212,11 +219,19 @@ func resourceCloudFlareLoadBalancerUpdate(d *schema.ResourceData, meta interface
 	}
 
 	if regionPools, ok := d.GetOk("region_pools"); ok {
-		loadBalancer.RegionPools = expandGeoPools(regionPools, "region")
+		expandedRegionPools, err := expandGeoPools(regionPools, "region")
+		if err != nil {
+			return err
+		}
+		loadBalancer.RegionPools = expandedRegionPools
 	}
 
 	if popPools, ok := d.GetOk("pop_pools"); ok {
-		loadBalancer.PopPools = expandGeoPools(popPools, "pop")
+		expandedPopPools, err := expandGeoPools(popPools, "pop")
+		if err != nil {
+			return err
+		}
+		loadBalancer.PopPools = expandedPopPools
 	}
 
 	log.Printf("[INFO] Updating CloudFlare Load Balancer from struct: %+v", loadBalancer)
@@ -229,15 +244,20 @@ func resourceCloudFlareLoadBalancerUpdate(d *schema.ResourceData, meta interface
 	return resourceCloudFlareLoadBalancerRead(d, meta)
 }
 
-func expandGeoPools(pool interface{}, geoType string) map[string][]string {
+func expandGeoPools(pool interface{}, geoType string) (map[string][]string, error) {
 	cfg := pool.(*schema.Set).List()
 	expanded := make(map[string][]string)
 	for _, v := range cfg {
 		locationConfig := v.(map[string]interface{})
-		// assume for now that lists are of type interface{} by default
-		expanded[locationConfig[geoType].(string)] = expandInterfaceToStringList(locationConfig["pool_ids"])
+		// lists are of type interface{} by default
+		location := locationConfig[geoType].(string)
+		if _, present := expanded[location]; !present {
+			expanded[location] = expandInterfaceToStringList(locationConfig["pool_ids"])
+		} else {
+			return nil, fmt.Errorf("duplicate entry specified for %s pool in location %q. each location must only be specified once", geoType, location)
+		}
 	}
-	return expanded
+	return expanded, nil
 }
 
 func resourceCloudFlareLoadBalancerRead(d *schema.ResourceData, meta interface{}) error {

cloudflare/resource_cloudflare_load_balancer_test.go

@@ -13,6 +13,7 @@ import (
 	"github.com/hashicorp/terraform/helper/acctest"
 	"github.com/hashicorp/terraform/helper/resource"
 	"github.com/hashicorp/terraform/terraform"
+	"regexp"
 )
 
 func TestAccCloudFlareLoadBalancer_Basic(t *testing.T) {
@@ -77,6 +78,24 @@ func TestAccCloudFlareLoadBalancer_GeoBalanced(t *testing.T) {
 	})
 }
 
+func TestAccCloudFlareLoadBalancer_DuplicatePool(t *testing.T) {
+	t.Parallel()
+	zone := os.Getenv("CLOUDFLARE_DOMAIN")
+	rnd := acctest.RandString(10)
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckCloudFlareLoadBalancerDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config:      testAccCheckCloudFlareLoadBalancerConfigDuplicatePool(zone, rnd),
+				ExpectError: regexp.MustCompile(regexp.QuoteMeta("duplicate entry specified for pop pool in location \"LAX\". each location must only be specified once")),
+			},
+		},
+	})
+}
+
 /**
 Any change to a load balancer  results in a new resource
 Although the API client contains a modify method, this always results in 405 status
@@ -299,3 +318,21 @@ resource "cloudflare_load_balancer" "%[2]s" {
   }
 }`, zone, id)
 }
+
+func testAccCheckCloudFlareLoadBalancerConfigDuplicatePool(zone, id string) string {
+	return testAccCheckCloudFlareLoadBalancerPoolConfigBasic(id) + fmt.Sprintf(`
+resource "cloudflare_load_balancer" "%[2]s" {
+  zone = "%[1]s"
+  name = "tf-testacc-lb-%[2]s"
+  fallback_pool_id = "${cloudflare_load_balancer_pool.%[2]s.id}"
+  default_pool_ids = ["${cloudflare_load_balancer_pool.%[2]s.id}"]
+  pop_pools {
+    pop = "LAX"
+    pool_ids = ["i_am_an_invalid_pool_id"]
+  }
+  pop_pools {
+    pop = "LAX"
+    pool_ids = ["${cloudflare_load_balancer_pool.%[2]s.id}"]
+  }
+}`, zone, id)
+}

website/docs/r/load_balancer.html.markdown

@@ -55,16 +55,16 @@ The following arguments are supported:
 * `ttl` - (Optional) Time to live (TTL) of this load balancer's DNS `name`. Conflicts with `proxied` - this cannot be set for proxied load balancers. Default is `30`.
 * `proxied` - (Optional) Whether the hostname gets Cloudflare's origin protection. Defaults to `false`.
 * `region_pools` - (Optional) A set containing mappings of region/country codes to a list of pool IDs (ordered by their failover priority) for the given region. Fields documented below.
-* `pop_pools` - (Optional) A set containing mappings of Cloudflare Point-of-Presence identifiers to a list of pool IDs (ordered by their failover priority) for the PoP (datacenter). This feature is only available to enterprise customers. Fields documented below.
+* `pop_pools` - (Optional) A set containing mappings of Cloudflare Point-of-Presence (PoP) identifiers to a list of pool IDs (ordered by their failover priority) for the PoP (datacenter). This feature is only available to enterprise customers. Fields documented below.
 
 **region_pools** requires the following:
 
-* `region` - (Required) A region code which must be in the list defined [here](https://support.cloudflare.com/hc/en-us/articles/115000540888-Load-Balancing-Geographic-Regions).
+* `region` - (Required) A region code which must be in the list defined [here](https://support.cloudflare.com/hc/en-us/articles/115000540888-Load-Balancing-Geographic-Regions). Multiple entries should not be specified with the same region.
 * `pool_ids` - (Required) A list of pool IDs in failover priority to use in the given region.
 
 **pop_pools** requires the following:
 
-* `pop` - (Required) A 3-letter code for the PoP. Allowed values can be found in the list of datacenters on the [status page](https://www.cloudflarestatus.com/).
+* `pop` - (Required) A 3-letter code for the Point-of-Presence. Allowed values can be found in the list of datacenters on the [status page](https://www.cloudflarestatus.com/). Multiple entries should not be specified with the same PoP.
 * `pool_ids` - (Required) A list of pool IDs in failover priority to use for traffic reaching the given PoP.
 
 ## Attributes Reference