From 705182c39334a598f200d0e1a968650550630c36 Mon Sep 17 00:00:00 2001 From: Ferdinando Formica Date: Thu, 27 Jun 2024 16:41:32 +0100 Subject: [PATCH] Fix regions update --- CHANGELOG.md | 4 + ns1/config.go | 2 +- ns1/resource_record.go | 24 ++--- ns1/resource_record_test.go | 172 +++++++++++++++++++++++++++++++++--- 4 files changed, 173 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6259ce7b..122a9284 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/ns1/config.go b/ns1/config.go index babacd30..794c4324 100644 --- a/ns1/config.go +++ b/ns1/config.go @@ -19,7 +19,7 @@ import ( ) var ( - clientVersion = "2.3.0" + clientVersion = "2.3.1" providerUserAgent = "tf-ns1" + "/" + clientVersion defaultRetryMax = 3 ) diff --git a/ns1/resource_record.go b/ns1/resource_record.go index deb805df..042bb518 100644 --- a/ns1/resource_record.go +++ b/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 diff --git a/ns1/resource_record_test.go b/ns1/resource_record_test.go index eb800e73..b6b08131 100644 --- a/ns1/resource_record_test.go +++ b/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(`