Fix regions update (#322)

Author: fformica

CHANGELOG.md

@@ -1,3 +1,7 @@
+## 2.3.1 (June 28, 2024)
+BUGFIX
+* Fix issue with updating regions in record resource
+
 ## 2.3.0 (June 18, 2024)
 ENHANCEMENTS
 * Adds support for redirect endpoints

ns1/config.go

@@ -19,7 +19,7 @@ import (
 )
 
 var (
-	clientVersion     = "2.3.0"
+	clientVersion     = "2.3.1"
 	providerUserAgent = "tf-ns1" + "/" + clientVersion
 	defaultRetryMax   = 3
 )

ns1/resource_record.go

@@ -449,9 +449,15 @@ func resourceDataToRecord(r *dns.Record, d *schema.ResourceData) error {
 		}
 		r.Filters = filters
 	}
-	if regions := d.Get("regions").(*schema.Set); regions.Len() > 0 {
+	if regions := d.Get("regions").(*schema.Set); regions != nil {
+		r.Regions = data.Regions{}
 		for _, regionRaw := range regions.List() {
 			region := regionRaw.(map[string]interface{})
+			name := region["name"].(string)
+			if name == "" {
+				// silently ignore - the name is required but TF can append empty entries in the set
+				continue
+			}
 			ns1R := data.Region{
 				Meta: data.Meta{},
 			}
@@ -465,21 +471,7 @@ func resourceDataToRecord(r *dns.Record, d *schema.ResourceData) error {
 
 				ns1R.Meta = *meta
 			}
-			r.Regions[region["name"].(string)] = ns1R
-		}
-	}
-
-	// Even though blocked_tags are not evaluated on GET, dual update logic is currently enforced on POST
-	if _, tagsExist := d.GetOk("tags"); tagsExist == true {
-		if _, blockedTagsExist := d.GetOk("blocked_tags"); blockedTagsExist == false {
-			r.BlockedTags = []string{}
-			log.Println("empty 'blocked tags' key added")
-		}
-	}
-	if _, blockedTagsExist := d.GetOk("blocked_tags"); blockedTagsExist == true {
-		if _, tagsExist := d.GetOk("tags"); tagsExist == false {
-			r.Tags = map[string]string{}
-			log.Println("empty 'tags' key added")
+			r.Regions[name] = ns1R
 		}
 	}
 	return nil

ns1/resource_record_test.go

@@ -502,6 +502,79 @@ func TestAccRecord_updatedWithTags(t *testing.T) {
 	})
 }
 
+func TestAccRecord_updatedWithRegions(t *testing.T) {
+	var record dns.Record
+	rString := acctest.RandStringFromCharSet(15, acctest.CharSetAlphaNum)
+	zoneName := fmt.Sprintf("terraform-test-%s.io", rString)
+	domainName := fmt.Sprintf("test.%s", zoneName)
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckRecordDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccRecordBasic(rString),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckRecordExists("ns1_record.it", &record),
+					testAccCheckRecordDomain(&record, domainName),
+					testAccCheckRecordTTL(&record, 60),
+					testAccCheckRecordUseClientSubnet(&record, true),
+					testAccCheckRecordRegionName(&record, []string{"cal"}),
+					testAccCheckRecordAnswerRdata(
+						t, &record, 0, []string{fmt.Sprintf("test1.%s", zoneName)},
+					),
+				),
+			},
+			{
+				Config: testAccRecordUpdatedWithRegionWeight(rString),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckRecordExists("ns1_record.it", &record),
+					testAccCheckRecordDomain(&record, domainName),
+					testAccCheckRecordTTL(&record, 60),
+					testAccCheckRecordUseClientSubnet(&record, true),
+					testAccCheckRecordRegionName(&record, []string{"cal"}),
+					testAccCheckRecordAnswerRdata(
+						t, &record, 0, []string{fmt.Sprintf("test1.%s", zoneName)},
+					),
+				),
+			},
+			{
+				Config: testAccRecordUpdatedWithRegions(rString),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckRecordExists("ns1_record.it", &record),
+					testAccCheckRecordDomain(&record, domainName),
+					testAccCheckRecordTTL(&record, 60),
+					testAccCheckRecordUseClientSubnet(&record, true),
+					testAccCheckRecordRegionName(&record, []string{"ny", "cal"}),
+					testAccCheckRecordAnswerRdata(
+						t, &record, 0, []string{fmt.Sprintf("test1.%s", zoneName)},
+					),
+				),
+			},
+			{
+				Config: testAccRecordUpdatedWithRegionWeight(rString),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckRecordExists("ns1_record.it", &record),
+					testAccCheckRecordDomain(&record, domainName),
+					testAccCheckRecordTTL(&record, 60),
+					testAccCheckRecordUseClientSubnet(&record, true),
+					testAccCheckRecordRegionName(&record, []string{"cal"}),
+					testAccCheckRecordAnswerRdata(
+						t, &record, 0, []string{fmt.Sprintf("test1.%s", zoneName)},
+					),
+				),
+			},
+			{
+				ResourceName:      "ns1_record.it",
+				ImportState:       true,
+				ImportStateId:     fmt.Sprintf("%s/%s/CNAME", zoneName, domainName),
+				ImportStateVerify: true,
+			},
+		},
+	})
+}
+
 func TestAccRecord_validationError(t *testing.T) {
 	rString := acctest.RandStringFromCharSet(15, acctest.CharSetAlphaNum)
 
@@ -1038,18 +1111,6 @@ func mapToSlice(m map[string]interface{}) []string {
 	return sliceString
 }
 
-func testAccCheckRecordAnswerMetaWeight(r *dns.Record, expected float64) resource.TestCheckFunc {
-	return func(s *terraform.State) error {
-		recordAnswer := r.Answers[0]
-		recordMetas := recordAnswer.Meta
-		weight := recordMetas.Weight.(float64)
-		if weight != expected {
-			return fmt.Errorf("r.Answers[0].Meta.Weight: got: %#v want: %#v", weight, expected)
-		}
-		return nil
-	}
-}
-
 func testAccCheckRecordAnswerMetaIPPrefixes(r *dns.Record, expected []string) resource.TestCheckFunc {
 	return func(s *terraform.State) error {
 		recordAnswer := r.Answers[0]
@@ -1637,6 +1698,93 @@ resource "ns1_zone" "test" {
 `, rString)
 }
 
+func testAccRecordUpdatedWithRegionWeight(rString string) string {
+	return fmt.Sprintf(`
+resource "ns1_record" "it" {
+  zone              = "${ns1_zone.test.zone}"
+  domain            = "test.${ns1_zone.test.zone}"
+  type              = "CNAME"
+  ttl               = 60
+
+  answers {
+    answer = "test1.${ns1_zone.test.zone}"
+    region = "cal"
+  }
+
+  regions {
+    name = "cal"
+    meta = {
+      weight = 100
+    }
+  }
+
+  filters {
+    filter = "geotarget_country"
+  }
+
+  filters {
+    filter = "select_first_n"
+    config = {N=1}
+  }
+
+  filters {
+    filter = "up"
+  }
+}
+
+resource "ns1_zone" "test" {
+  zone = "terraform-test-%s.io"
+}
+`, rString)
+}
+
+func testAccRecordUpdatedWithRegions(rString string) string {
+	return fmt.Sprintf(`
+resource "ns1_record" "it" {
+  zone              = "${ns1_zone.test.zone}"
+  domain            = "test.${ns1_zone.test.zone}"
+  type              = "CNAME"
+  ttl               = 60
+
+  answers {
+    answer = "test1.${ns1_zone.test.zone}"
+    region = "cal"
+  }
+
+  regions {
+    name = "cal"
+    meta = {
+      weight = 90
+    }
+  }
+
+  regions {
+    name = "ny"
+    meta = {
+      weight = 10
+    }
+  }
+
+  filters {
+    filter = "geotarget_country"
+  }
+
+  filters {
+    filter = "select_first_n"
+    config = {N=1}
+  }
+
+  filters {
+    filter = "up"
+  }
+}
+
+resource "ns1_zone" "test" {
+  zone = "terraform-test-%s.io"
+}
+`, rString)
+}
+
 // zone and domain have leading and trailing dots and should fail validation.
 func testAccRecordInvalid(rString string) string {
 	return fmt.Sprintf(`