fix(domain): fix SRV domain duplication bug (#3287)

* fix(domain): fix SRV domain duplication bug

* fix lint:

* add unit test and better help messages

Author: jremy42

internal/services/domain/helpers.go

@@ -32,17 +32,29 @@ func getRecordFromTypeAndData(dnsType domain.RecordType, data string, records []
 	var currentRecord *domain.Record
 
 	for _, r := range records {
-		flattedData := flattenDomainData(strings.ToLower(r.Data), r.Type).(string)
-		flattenCurrentData := flattenDomainData(strings.ToLower(data), r.Type).(string)
+		flattedData := FlattenDomainData(strings.ToLower(r.Data), r.Type).(string)
+		flattenCurrentData := FlattenDomainData(strings.ToLower(data), r.Type).(string)
 
-		if strings.HasPrefix(flattedData, flattenCurrentData) && r.Type == dnsType {
-			if currentRecord != nil {
-				return nil, errors.New("multiple records found with same type and data")
+		if dnsType == domain.RecordTypeSRV {
+			if flattedData == flattenCurrentData {
+				if currentRecord != nil {
+					return nil, fmt.Errorf("multiple records found with same type and data: existing record %s (ID: %s) conflicts with new record data %s", currentRecord.Data, currentRecord.ID, data)
+				}
+
+				currentRecord = r
+
+				break
 			}
+		} else {
+			if strings.HasPrefix(flattedData, flattenCurrentData) && r.Type == dnsType {
+				if currentRecord != nil {
+					return nil, fmt.Errorf("multiple records found with same type and data: existing record %s (ID: %s) conflicts with new record data %s", currentRecord.Data, currentRecord.ID, data)
+				}
 
-			currentRecord = r
+				currentRecord = r
 
-			break
+				break
+			}
 		}
 	}
 

internal/services/domain/record.go

@@ -296,7 +296,7 @@ func resourceRecordCreate(ctx context.Context, d *schema.ResourceData, m any) di
 		return diag.FromErr(err)
 	}
 
-	currentRecord, err := getRecordFromTypeAndData(recordType, recordData, dnsZoneData.Records)
+	currentRecord, err := getRecordFromTypeAndData(recordType, FlattenDomainData(recordData, recordType).(string), dnsZoneData.Records)
 	if err != nil {
 		return diag.FromErr(err)
 	}
@@ -408,7 +408,7 @@ func resourceDomainRecordRead(ctx context.Context, d *schema.ResourceData, m any
 	_ = d.Set("dns_zone", dnsZone)
 	_ = d.Set("name", record.Name)
 	_ = d.Set("type", record.Type.String())
-	_ = d.Set("data", flattenDomainData(record.Data, record.Type).(string))
+	_ = d.Set("data", FlattenDomainData(record.Data, record.Type).(string))
 	_ = d.Set("ttl", int(record.TTL))
 	_ = d.Set("priority", int(record.Priority))
 	_ = d.Set("geo_ip", flattenDomainGeoIP(record.GeoIPConfig))

internal/services/domain/record_test.go

@@ -692,6 +692,72 @@ func TestAccDomainRecord_SRVZone(t *testing.T) {
 	})
 }
 
+func TestAccDomainRecord_SRVWithDomainDuplication(t *testing.T) {
+	tt := acctest.NewTestTools(t)
+	defer tt.Cleanup()
+
+	testDNSZone := "test-srv-duplication." + acctest.TestDomain
+	logging.L.Debugf("TestAccDomainRecord_SRVWithDomainDuplication: test dns zone: %s", testDNSZone)
+
+	name := "_test_srv_bug"
+	recordType := "SRV"
+	data := "0 0 1234 foo.example.com."
+	ttl := 60
+	priority := 0
+
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck:          func() { acctest.PreCheck(t) },
+		ProviderFactories: tt.ProviderFactories,
+		CheckDestroy:      testAccCheckDomainRecordDestroy(tt),
+		Steps: []resource.TestStep{
+			{
+				Config: fmt.Sprintf(`
+					resource "scaleway_domain_record" "srv_test" {
+						dns_zone = "%[1]s"
+						name     = "%[2]s"
+						type     = "%[3]s"
+						data     = "%[4]s"
+						priority = %[5]d
+						ttl      = %[6]d
+					}
+				`, testDNSZone, name, recordType, data, priority, ttl),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckDomainRecordExists(tt, "scaleway_domain_record.srv_test"),
+					resource.TestCheckResourceAttr("scaleway_domain_record.srv_test", "dns_zone", testDNSZone),
+					resource.TestCheckResourceAttr("scaleway_domain_record.srv_test", "name", name),
+					resource.TestCheckResourceAttr("scaleway_domain_record.srv_test", "type", recordType),
+					resource.TestCheckResourceAttr("scaleway_domain_record.srv_test", "data", "0 0 1234 foo.example.com."),
+					resource.TestCheckResourceAttr("scaleway_domain_record.srv_test", "ttl", strconv.Itoa(ttl)),
+					resource.TestCheckResourceAttr("scaleway_domain_record.srv_test", "priority", strconv.Itoa(priority)),
+					acctest.CheckResourceAttrUUID("scaleway_domain_record.srv_test", "id"),
+				),
+			},
+			{
+				Config: fmt.Sprintf(`
+					resource "scaleway_domain_record" "srv_test" {
+						dns_zone = "%[1]s"
+						name     = "%[2]s"
+						type     = "%[3]s"
+						data     = "10 0 5678 bar.example.com."
+						priority = 10
+						ttl      = 300
+					}
+				`, testDNSZone, name, recordType),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckDomainRecordExists(tt, "scaleway_domain_record.srv_test"),
+					resource.TestCheckResourceAttr("scaleway_domain_record.srv_test", "dns_zone", testDNSZone),
+					resource.TestCheckResourceAttr("scaleway_domain_record.srv_test", "name", name),
+					resource.TestCheckResourceAttr("scaleway_domain_record.srv_test", "type", recordType),
+					resource.TestCheckResourceAttr("scaleway_domain_record.srv_test", "data", "10 0 5678 bar.example.com."),
+					resource.TestCheckResourceAttr("scaleway_domain_record.srv_test", "ttl", "300"),
+					resource.TestCheckResourceAttr("scaleway_domain_record.srv_test", "priority", "10"),
+					acctest.CheckResourceAttrUUID("scaleway_domain_record.srv_test", "id"),
+				),
+			},
+		},
+	})
+}
+
 func testAccCheckDomainRecordExists(tt *acctest.TestTools, n string) resource.TestCheckFunc {
 	return func(s *terraform.State) error {
 		rs, ok := s.RootModule().Resources[n]