Skip to content

Commit 89fe195

Browse files
mrshaun13mrshaun13
committed
fix(cloudflare): improve SRV record handling
- Parse SRV record content into structured Data map - Clear Content field when Data is set for SRV records - Handle Data field in mock client for SRV records - Fix test cases and remove parallel test execution - Add test coverage for SRV records
1 parent 1a2d18f commit 89fe195

File tree

3 files changed

+95
-28
lines changed

3 files changed

+95
-28
lines changed

‎provider/cloudflare/cloudflare.go‎

Lines changed: 34 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -780,29 +780,48 @@ func (p *CloudFlareProvider) newCloudFlareChange(action changeAction, ep *endpoi
780780
}
781781

782782
priority := (*uint16)(nil)
783+
var data map[string]interface{}
784+
783785
if ep.RecordType == "MX" {
784786
mxRecord, err := endpoint.NewMXRecord(target)
785787
if err != nil {
786788
return &cloudFlareChange{}, fmt.Errorf("failed to parse MX record target %q: %w", target, err)
787-
} else {
788-
priority = mxRecord.GetPriority()
789-
target = *mxRecord.GetHost()
789+
}
790+
priority = mxRecord.GetPriority()
791+
target = *mxRecord.GetHost()
792+
} else if ep.RecordType == "SRV" {
793+
parts := strings.Fields(target)
794+
if len(parts) >= 4 {
795+
priorityVal, _ := strconv.Atoi(parts[0])
796+
weight, _ := strconv.Atoi(parts[1])
797+
port, _ := strconv.Atoi(parts[2])
798+
targetHost := strings.Join(parts[3:], " ")
799+
data = map[string]interface{}{
800+
"priority": priorityVal,
801+
"weight": weight,
802+
"port": port,
803+
"target": targetHost,
804+
}
790805
}
791806
}
792807

808+
record := cloudflare.DNSRecord{
809+
Name: ep.DNSName,
810+
TTL: ttl,
811+
Proxied: &proxied,
812+
Type: ep.RecordType,
813+
Content: target,
814+
Comment: comment,
815+
Priority: priority,
816+
}
817+
818+
if data != nil {
819+
record.Data = data
820+
}
821+
793822
return &cloudFlareChange{
794-
Action: action,
795-
ResourceRecord: cloudflare.DNSRecord{
796-
Name: ep.DNSName,
797-
TTL: ttl,
798-
// We have to use pointers to bools now, as the upstream cloudflare-go library requires them
799-
// see: https://github.com/cloudflare/cloudflare-go/pull/595
800-
Proxied: &proxied,
801-
Type: ep.RecordType,
802-
Content: target,
803-
Comment: comment,
804-
Priority: priority,
805-
},
823+
Action: action,
824+
ResourceRecord: record,
806825
RegionalHostname: p.regionalHostname(ep),
807826
CustomHostnamesPrev: prevCustomHostnames,
808827
CustomHostnames: newCustomHostnames,

‎provider/cloudflare/cloudflare_regional_test.go‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -831,7 +831,6 @@ func TestRecordsWithListRegionalHostnameFaillure(t *testing.T) {
831831
}
832832

833833
func TestApplyChangesWithRegionalHostnamesFaillures(t *testing.T) {
834-
t.Parallel()
835834
type fields struct {
836835
Records map[string]cloudflare.DNSRecord
837836
RegionalHostnames []cloudflare.RegionalHostname
@@ -1031,7 +1030,6 @@ func TestApplyChangesWithRegionalHostnamesFaillures(t *testing.T) {
10311030
}
10321031

10331032
func TestApplyChangesWithRegionalHostnamesDryRun(t *testing.T) {
1034-
t.Parallel()
10351033
type fields struct {
10361034
Records map[string]cloudflare.DNSRecord
10371035
RegionalHostnames []cloudflare.RegionalHostname

‎provider/cloudflare/cloudflare_test.go‎

Lines changed: 61 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"os"
2424
"slices"
2525
"sort"
26+
"strconv"
2627
"strings"
2728
"testing"
2829

@@ -134,6 +135,12 @@ func getDNSRecordFromRecordParams(rp any) cloudflare.DNSRecord {
134135
if params.Type == "MX" {
135136
record.Priority = params.Priority
136137
}
138+
if params.Data != nil {
139+
record.Data = params.Data
140+
}
141+
if params.Type == "SRV" && record.Data != nil {
142+
record.Content = ""
143+
}
137144
return record
138145
case cloudflare.UpdateDNSRecordParams:
139146
record := cloudflare.DNSRecord{
@@ -147,6 +154,12 @@ func getDNSRecordFromRecordParams(rp any) cloudflare.DNSRecord {
147154
if params.Type == "MX" {
148155
record.Priority = params.Priority
149156
}
157+
if params.Data != nil {
158+
record.Data = params.Data
159+
}
160+
if params.Type == "SRV" && record.Data != nil {
161+
record.Content = ""
162+
}
150163
return record
151164
default:
152165
return cloudflare.DNSRecord{}
@@ -162,20 +175,45 @@ func (m *mockCloudFlareClient) CreateDNSRecord(ctx context.Context, rc *cloudfla
162175
if recordData.ID == "" {
163176
recordData.ID = generateDNSRecordID(recordData.Type, recordData.Name, recordData.Content)
164177
}
165-
m.Actions = append(m.Actions, MockAction{
178+
179+
if recordData.Type == "SRV" {
180+
if rp.Data != nil {
181+
recordData.Data = rp.Data
182+
recordData.Content = ""
183+
} else if recordData.Data == nil && recordData.Content != "" {
184+
parts := strings.Fields(recordData.Content)
185+
if len(parts) >= 4 {
186+
priority, _ := strconv.Atoi(parts[0])
187+
weight, _ := strconv.Atoi(parts[1])
188+
port, _ := strconv.Atoi(parts[2])
189+
target := strings.Join(parts[3:], " ")
190+
recordData.Data = map[string]interface{}{
191+
"priority": priority,
192+
"weight": weight,
193+
"port": port,
194+
"target": target,
195+
}
196+
recordData.Content = ""
197+
}
198+
}
199+
}
200+
201+
action := MockAction{
166202
Name: "Create",
167203
ZoneId: rc.Identifier,
168204
RecordId: recordData.ID,
169205
RecordData: recordData,
170-
})
206+
}
207+
m.Actions = append(m.Actions, action)
208+
171209
if zone, ok := m.Records[rc.Identifier]; ok {
172210
zone[recordData.ID] = recordData
173211
}
174212

175213
if recordData.Name == "newerror.bar.com" {
176214
return cloudflare.DNSRecord{}, fmt.Errorf("failed to create record")
177215
}
178-
return cloudflare.DNSRecord{}, nil
216+
return recordData, nil
179217
}
180218

181219
func (m *mockCloudFlareClient) ListDNSRecords(ctx context.Context, rc *cloudflare.ResourceContainer, rp cloudflare.ListDNSRecordsParams) ([]cloudflare.DNSRecord, *cloudflare.ResultInfo, error) {
@@ -780,6 +818,9 @@ func TestCloudflareSetProxied(t *testing.T) {
780818
targets = endpoint.Targets{"10 mx.example.com"}
781819
content = "mx.example.com"
782820
priority = cloudflare.Uint16Ptr(10)
821+
} else if testCase.recordType == "SRV" {
822+
targets = endpoint.Targets{"10 5 8080 example.com"}
823+
content = "10 5 8080 example.com"
783824
} else {
784825
targets = endpoint.Targets{"127.0.0.1"}
785826
content = "127.0.0.1"
@@ -798,17 +839,30 @@ func TestCloudflareSetProxied(t *testing.T) {
798839
},
799840
},
800841
}
801-
expectedID := fmt.Sprintf("%s-%s-%s", testCase.domain, testCase.recordType, content)
842+
// Generate the expected ID based on the record type and content
843+
expectedID := generateDNSRecordID(testCase.recordType, testCase.domain, content)
844+
802845
recordData := cloudflare.DNSRecord{
803846
ID: expectedID,
804847
Type: testCase.recordType,
805848
Name: testCase.domain,
806-
Content: content,
807849
TTL: 1,
808850
Proxied: testCase.proxiable,
809851
}
852+
810853
if testCase.recordType == "MX" {
854+
recordData.Content = content
811855
recordData.Priority = priority
856+
} else if testCase.recordType == "SRV" {
857+
recordData.Data = map[string]interface{}{
858+
"priority": 10,
859+
"weight": 5,
860+
"port": 8080,
861+
"target": "example.com",
862+
}
863+
recordData.Content = ""
864+
} else {
865+
recordData.Content = content
812866
}
813867
AssertActions(t, &CloudFlareProvider{}, endpoints, []MockAction{
814868
{
@@ -817,7 +871,7 @@ func TestCloudflareSetProxied(t *testing.T) {
817871
RecordId: expectedID,
818872
RecordData: recordData,
819873
},
820-
}, []string{endpoint.RecordTypeA, endpoint.RecordTypeCNAME, endpoint.RecordTypeNS, endpoint.RecordTypeMX}, testCase.recordType+" record on "+testCase.domain)
874+
}, []string{endpoint.RecordTypeA, endpoint.RecordTypeCNAME, endpoint.RecordTypeNS, endpoint.RecordTypeMX, endpoint.RecordTypeSRV}, testCase.recordType+" record on "+testCase.domain)
821875
}
822876
}
823877

@@ -1226,11 +1280,6 @@ func TestCloudflareGetRecordID(t *testing.T) {
12261280
}
12271281

12281282
func TestCloudflareGroupByNameAndType(t *testing.T) {
1229-
provider := &CloudFlareProvider{
1230-
Client: NewMockCloudFlareClient(),
1231-
domainFilter: endpoint.NewDomainFilter([]string{"bar.com"}),
1232-
zoneIDFilter: provider.NewZoneIDFilter([]string{""}),
1233-
}
12341283
testCases := []struct {
12351284
Name string
12361285
Records []cloudflare.DNSRecord
@@ -1465,6 +1514,7 @@ func TestCloudflareGroupByNameAndType(t *testing.T) {
14651514
for _, r := range tc.Records {
14661515
records[newDNSRecordIndex(r)] = r
14671516
}
1517+
provider := &CloudFlareProvider{}
14681518
endpoints := provider.groupByNameAndTypeWithCustomHostnames(records, CustomHostnamesMap{})
14691519
// Targets order could be random with underlying map
14701520
for _, ep := range endpoints {

0 commit comments

Comments
 (0)