diff --git a/docs/sources/mx-record.md b/docs/sources/mx-record.md index 7b0c91c751..2c864272ba 100644 --- a/docs/sources/mx-record.md +++ b/docs/sources/mx-record.md @@ -1,7 +1,7 @@ # MX record with CRD source You can create and manage MX records with the help of [CRD source](../sources/crd.md) -and `DNSEndpoint` CRD. Currently, this feature is only supported by `aws`, `azure`, `google` and `digitalocean` providers. +and `DNSEndpoint` CRD. Currently, this feature is only supported by `aws`, `azure`, `cloudflare`, `digitalocean` and `google` providers. In order to start managing MX records you need to set the `--managed-record-types=MX` flag. diff --git a/endpoint/endpoint.go b/endpoint/endpoint.go index 1c972d0616..95461a5aa0 100644 --- a/endpoint/endpoint.go +++ b/endpoint/endpoint.go @@ -71,6 +71,12 @@ func (ttl TTL) IsConfigured() bool { // Targets is a representation of a list of targets for an endpoint. type Targets []string +// MXTarget represents a single MX (Mail Exchange) record target, including its priority and host. +type MXTarget struct { + priority uint16 + host string +} + // NewTargets is a convenience method to create a new Targets object from a vararg of strings func NewTargets(target ...string) Targets { t := make(Targets, 0, len(target)) @@ -394,22 +400,44 @@ func (e *Endpoint) CheckEndpoint() bool { return true } +// NewMXRecord parses a string representation of an MX record target (e.g., "10 mail.example.com") +// and returns an MXTarget struct. Returns an error if the input is invalid. +func NewMXRecord(target string) (*MXTarget, error) { + parts := strings.Fields(strings.TrimSpace(target)) + if len(parts) != 2 { + return nil, fmt.Errorf("invalid MX record target: %s. MX records must have a preference value and a host, e.g. '10 example.com'", target) + } + + priority, err := strconv.ParseUint(parts[0], 10, 16) + if err != nil { + return nil, fmt.Errorf("invalid integer value in target: %s", target) + } + + return &MXTarget{ + priority: uint16(priority), + host: parts[1], + }, nil +} + +// GetPriority returns the priority of the MX record target. +func (m *MXTarget) GetPriority() *uint16 { + return &m.priority +} + +// GetHost returns the host of the MX record target. +func (m *MXTarget) GetHost() *string { + return &m.host +} + func (t Targets) ValidateMXRecord() bool { for _, target := range t { - // MX records must have a preference value to indicate priority, e.g. "10 example.com" - // as per https://www.rfc-editor.org/rfc/rfc974.txt - targetParts := strings.Fields(strings.TrimSpace(target)) - if len(targetParts) != 2 { - log.Debugf("Invalid MX record target: %s. MX records must have a preference value to indicate priority, e.g. '10 example.com'", target) - return false - } - preferenceRaw := targetParts[0] - _, err := strconv.ParseUint(preferenceRaw, 10, 16) + _, err := NewMXRecord(target) if err != nil { - log.Debugf("Invalid SRV record target: %s. Invalid integer value in target.", target) + log.Debugf("Invalid MX record target: %s. %v", target, err) return false } } + return true } diff --git a/endpoint/endpoint_test.go b/endpoint/endpoint_test.go index db2a0dbd1c..0c2a3cbb96 100644 --- a/endpoint/endpoint_test.go +++ b/endpoint/endpoint_test.go @@ -815,3 +815,113 @@ func TestPDNScheckEndpoint(t *testing.T) { assert.Equal(t, tt.expected, actual) } } + +func TestNewMXTarget(t *testing.T) { + tests := []struct { + description string + target string + expected *MXTarget + expectError bool + }{ + { + description: "Valid MX record", + target: "10 example.com", + expected: &MXTarget{priority: 10, host: "example.com"}, + expectError: false, + }, + { + description: "Invalid MX record with missing priority", + target: "example.com", + expectError: true, + }, + { + description: "Invalid MX record with non-integer priority", + target: "abc example.com", + expectError: true, + }, + { + description: "Invalid MX record with too many parts", + target: "10 example.com extra", + expectError: true, + }, + { + description: "Missing host", + target: "10 ", + expected: nil, + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.description, func(t *testing.T) { + actual, err := NewMXRecord(tt.target) + if tt.expectError { + assert.Error(t, err) + } else { + assert.NoError(t, err) + assert.Equal(t, tt.expected, actual) + } + }) + } +} + +func TestCheckEndpoint(t *testing.T) { + tests := []struct { + description string + endpoint Endpoint + expected bool + }{ + { + description: "Valid MX record target", + endpoint: Endpoint{ + DNSName: "example.com", + RecordType: RecordTypeMX, + Targets: Targets{"10 example.com"}, + }, + expected: true, + }, + { + description: "Invalid MX record target", + endpoint: Endpoint{ + DNSName: "example.com", + RecordType: RecordTypeMX, + Targets: Targets{"example.com"}, + }, + expected: false, + }, + { + description: "Valid SRV record target", + endpoint: Endpoint{ + DNSName: "_service._tcp.example.com", + RecordType: RecordTypeSRV, + Targets: Targets{"10 5 5060 example.com"}, + }, + expected: true, + }, + { + description: "Invalid SRV record target", + endpoint: Endpoint{ + DNSName: "_service._tcp.example.com", + RecordType: RecordTypeSRV, + Targets: Targets{"10 5 example.com"}, + }, + expected: false, + }, + { + description: "Non-MX/SRV record type", + endpoint: Endpoint{ + DNSName: "example.com", + RecordType: RecordTypeA, + Targets: Targets{"192.168.1.1"}, + }, + expected: true, + }, + } + + for _, tt := range tests { + t.Run(tt.description, func(t *testing.T) { + actual := tt.endpoint.CheckEndpoint() + assert.Equal(t, tt.expected, actual) + }) + } +} diff --git a/provider/cloudflare/cloudflare.go b/provider/cloudflare/cloudflare.go index bdd72ee28f..574212da4b 100644 --- a/provider/cloudflare/cloudflare.go +++ b/provider/cloudflare/cloudflare.go @@ -65,16 +65,6 @@ func (action changeAction) String() string { return changeActionNames[action] } -// We have to use pointers to bools now, as the upstream cloudflare-go library requires them -// see: https://github.com/cloudflare/cloudflare-go/pull/595 - -var ( - // proxyEnabled is a pointer to a bool true showing the record should be proxied through cloudflare - proxyEnabled *bool = boolPtr(true) - // proxyDisabled is a pointer to a bool false showing the record should not be proxied through cloudflare - proxyDisabled *bool = boolPtr(false) -) - type DNSRecordIndex struct { Name string Type string @@ -226,7 +216,7 @@ type CloudFlareProvider struct { RegionalServicesConfig RegionalServicesConfig } -// cloudFlareChange differentiates between ChangActions +// cloudFlareChange differentiates between ChangeActions type cloudFlareChange struct { Action changeAction ResourceRecord cloudflare.DNSRecord @@ -242,24 +232,30 @@ type RecordParamsTypes interface { // updateDNSRecordParam is a function that returns the appropriate Record Param based on the cloudFlareChange passed in func updateDNSRecordParam(cfc cloudFlareChange) cloudflare.UpdateDNSRecordParams { - return cloudflare.UpdateDNSRecordParams{ - Name: cfc.ResourceRecord.Name, - TTL: cfc.ResourceRecord.TTL, - Proxied: cfc.ResourceRecord.Proxied, - Type: cfc.ResourceRecord.Type, - Content: cfc.ResourceRecord.Content, + params := cloudflare.UpdateDNSRecordParams{ + Name: cfc.ResourceRecord.Name, + TTL: cfc.ResourceRecord.TTL, + Proxied: cfc.ResourceRecord.Proxied, + Type: cfc.ResourceRecord.Type, + Content: cfc.ResourceRecord.Content, + Priority: cfc.ResourceRecord.Priority, } + + return params } // getCreateDNSRecordParam is a function that returns the appropriate Record Param based on the cloudFlareChange passed in func getCreateDNSRecordParam(cfc cloudFlareChange) cloudflare.CreateDNSRecordParams { - return cloudflare.CreateDNSRecordParams{ - Name: cfc.ResourceRecord.Name, - TTL: cfc.ResourceRecord.TTL, - Proxied: cfc.ResourceRecord.Proxied, - Type: cfc.ResourceRecord.Type, - Content: cfc.ResourceRecord.Content, + params := cloudflare.CreateDNSRecordParams{ + Name: cfc.ResourceRecord.Name, + TTL: cfc.ResourceRecord.TTL, + Proxied: cfc.ResourceRecord.Proxied, + Type: cfc.ResourceRecord.Type, + Content: cfc.ResourceRecord.Content, + Priority: cfc.ResourceRecord.Priority, } + + return params } func convertCloudflareError(err error) error { @@ -392,7 +388,7 @@ func (p *CloudFlareProvider) Records(ctx context.Context) ([]*endpoint.Endpoint, // As CloudFlare does not support "sets" of targets, but instead returns // a single entry for each name/type/target, we have to group by name // and record to allow the planner to calculate the correct plan. See #992. - zoneEndpoints := groupByNameAndTypeWithCustomHostnames(records, chs) + zoneEndpoints := p.groupByNameAndTypeWithCustomHostnames(records, chs) if err := p.addEnpointsProviderSpecificRegionKeyProperty(ctx, zone.ID, zoneEndpoints); err != nil { return nil, err @@ -412,14 +408,24 @@ func (p *CloudFlareProvider) ApplyChanges(ctx context.Context, changes *plan.Cha if p.CustomHostnamesConfig.Enabled { for _, e := range changes.Delete { for _, target := range e.Targets { - cloudflareChanges = append(cloudflareChanges, p.newCloudFlareChange(cloudFlareDelete, e, target, nil)) + change, err := p.newCloudFlareChange(cloudFlareDelete, e, target, nil) + if err != nil { + log.Errorf("failed to create cloudflare change: %v", err) + continue + } + cloudflareChanges = append(cloudflareChanges, change) } } } for _, e := range changes.Create { for _, target := range e.Targets { - cloudflareChanges = append(cloudflareChanges, p.newCloudFlareChange(cloudFlareCreate, e, target, nil)) + change, err := p.newCloudFlareChange(cloudFlareCreate, e, target, nil) + if err != nil { + log.Errorf("failed to create cloudflare change: %v", err) + continue + } + cloudflareChanges = append(cloudflareChanges, change) } } @@ -429,15 +435,30 @@ func (p *CloudFlareProvider) ApplyChanges(ctx context.Context, changes *plan.Cha add, remove, leave := provider.Difference(current.Targets, desired.Targets) for _, a := range remove { - cloudflareChanges = append(cloudflareChanges, p.newCloudFlareChange(cloudFlareDelete, current, a, current)) + change, err := p.newCloudFlareChange(cloudFlareDelete, current, a, current) + if err != nil { + log.Errorf("failed to create cloudflare change: %v", err) + continue + } + cloudflareChanges = append(cloudflareChanges, change) } for _, a := range add { - cloudflareChanges = append(cloudflareChanges, p.newCloudFlareChange(cloudFlareCreate, desired, a, current)) + change, err := p.newCloudFlareChange(cloudFlareCreate, desired, a, current) + if err != nil { + log.Errorf("failed to create cloudflare change: %v", err) + continue + } + cloudflareChanges = append(cloudflareChanges, change) } for _, a := range leave { - cloudflareChanges = append(cloudflareChanges, p.newCloudFlareChange(cloudFlareUpdate, desired, a, current)) + change, err := p.newCloudFlareChange(cloudFlareUpdate, desired, a, current) + if err != nil { + log.Errorf("failed to create cloudflare change: %v", err) + continue + } + cloudflareChanges = append(cloudflareChanges, change) } } @@ -445,7 +466,12 @@ func (p *CloudFlareProvider) ApplyChanges(ctx context.Context, changes *plan.Cha if !p.CustomHostnamesConfig.Enabled { for _, e := range changes.Delete { for _, target := range e.Targets { - cloudflareChanges = append(cloudflareChanges, p.newCloudFlareChange(cloudFlareDelete, e, target, nil)) + change, err := p.newCloudFlareChange(cloudFlareDelete, e, target, nil) + if err != nil { + log.Errorf("failed to create cloudflare change: %v", err) + continue + } + cloudflareChanges = append(cloudflareChanges, change) } } } @@ -723,7 +749,7 @@ func (p *CloudFlareProvider) newCustomHostname(customHostname string, origin str } } -func (p *CloudFlareProvider) newCloudFlareChange(action changeAction, ep *endpoint.Endpoint, target string, current *endpoint.Endpoint) *cloudFlareChange { +func (p *CloudFlareProvider) newCloudFlareChange(action changeAction, ep *endpoint.Endpoint, target string, current *endpoint.Endpoint) (*cloudFlareChange, error) { ttl := defaultTTL proxied := shouldBeProxied(ep, p.proxiedByDefault) @@ -753,6 +779,17 @@ func (p *CloudFlareProvider) newCloudFlareChange(action changeAction, ep *endpoi comment = p.DNSRecordsConfig.trimAndValidateComment(ep.DNSName, comment, p.ZoneHasPaidPlan) } + priority := (*uint16)(nil) + if ep.RecordType == "MX" { + mxRecord, err := endpoint.NewMXRecord(target) + if err != nil { + return &cloudFlareChange{}, fmt.Errorf("failed to parse MX record target %q: %w", target, err) + } else { + priority = mxRecord.GetPriority() + target = *mxRecord.GetHost() + } + } + return &cloudFlareChange{ Action: action, ResourceRecord: cloudflare.DNSRecord{ @@ -760,15 +797,16 @@ func (p *CloudFlareProvider) newCloudFlareChange(action changeAction, ep *endpoi TTL: ttl, // We have to use pointers to bools now, as the upstream cloudflare-go library requires them // see: https://github.com/cloudflare/cloudflare-go/pull/595 - Proxied: &proxied, - Type: ep.RecordType, - Content: target, - Comment: comment, + Proxied: &proxied, + Type: ep.RecordType, + Content: target, + Comment: comment, + Priority: priority, }, RegionalHostname: p.regionalHostname(ep), CustomHostnamesPrev: prevCustomHostnames, CustomHostnames: newCustomHostnames, - } + }, nil } func newDNSRecordIndex(r cloudflare.DNSRecord) DNSRecordIndex { @@ -877,14 +915,14 @@ func getEndpointCustomHostnames(ep *endpoint.Endpoint) []string { return []string{} } -func groupByNameAndTypeWithCustomHostnames(records DNSRecordsMap, chs CustomHostnamesMap) []*endpoint.Endpoint { +func (p *CloudFlareProvider) groupByNameAndTypeWithCustomHostnames(records DNSRecordsMap, chs CustomHostnamesMap) []*endpoint.Endpoint { var endpoints []*endpoint.Endpoint // group supported records by name and type groups := map[string][]cloudflare.DNSRecord{} for _, r := range records { - if !provider.SupportedRecordType(r.Type) { + if !p.SupportedAdditionalRecordTypes(r.Type) { continue } @@ -910,7 +948,11 @@ func groupByNameAndTypeWithCustomHostnames(records DNSRecordsMap, chs CustomHost } targets := make([]string, len(records)) for i, record := range records { - targets[i] = record.Content + if records[i].Type == "MX" { + targets[i] = fmt.Sprintf("%v %v", *record.Priority, record.Content) + } else { + targets[i] = record.Content + } } e := endpoint.NewEndpointWithTTL( records[0].Name, @@ -937,12 +979,15 @@ func groupByNameAndTypeWithCustomHostnames(records DNSRecordsMap, chs CustomHost endpoints = append(endpoints, e) } - return endpoints } -// boolPtr is used as a helper function to return a pointer to a boolean -// Needed because some parameters require a pointer. -func boolPtr(b bool) *bool { - return &b +// SupportedRecordType returns true if the record type is supported by the provider +func (p *CloudFlareProvider) SupportedAdditionalRecordTypes(recordType string) bool { + switch recordType { + case endpoint.RecordTypeMX: + return true + default: + return provider.SupportedRecordType(recordType) + } } diff --git a/provider/cloudflare/cloudflare_test.go b/provider/cloudflare/cloudflare_test.go index 7babb58acf..b304d2116a 100644 --- a/provider/cloudflare/cloudflare_test.go +++ b/provider/cloudflare/cloudflare_test.go @@ -37,6 +37,12 @@ import ( "sigs.k8s.io/external-dns/source/annotations" ) +// proxyEnabled and proxyDisabled are pointers to bool values used to set if a record should be proxied through Cloudflare. +var ( + proxyEnabled *bool = testutils.ToPtr(true) + proxyDisabled *bool = testutils.ToPtr(false) +) + type MockAction struct { Name string ZoneId string @@ -117,7 +123,7 @@ func NewMockCloudFlareClientWithRecords(records map[string][]cloudflare.DNSRecor func getDNSRecordFromRecordParams(rp any) cloudflare.DNSRecord { switch params := rp.(type) { case cloudflare.CreateDNSRecordParams: - return cloudflare.DNSRecord{ + record := cloudflare.DNSRecord{ ID: params.ID, Name: params.Name, TTL: params.TTL, @@ -125,8 +131,12 @@ func getDNSRecordFromRecordParams(rp any) cloudflare.DNSRecord { Type: params.Type, Content: params.Content, } + if params.Type == "MX" { + record.Priority = params.Priority + } + return record case cloudflare.UpdateDNSRecordParams: - return cloudflare.DNSRecord{ + record := cloudflare.DNSRecord{ ID: params.ID, Name: params.Name, TTL: params.TTL, @@ -134,6 +144,10 @@ func getDNSRecordFromRecordParams(rp any) cloudflare.DNSRecord { Type: params.Type, Content: params.Content, } + if params.Type == "MX" { + record.Priority = params.Priority + } + return record default: return cloudflare.DNSRecord{} } @@ -413,7 +427,7 @@ func AssertActions(t *testing.T, provider *CloudFlareProvider, endpoints []*endp // Records other than A, CNAME and NS are not supported by planner, just create them for _, endpoint := range endpoints { - if endpoint.RecordType != "A" && endpoint.RecordType != "CNAME" && endpoint.RecordType != "NS" { + if !slices.Contains(managedRecords, endpoint.RecordType) { changes.Create = append(changes.Create, endpoint) } } @@ -508,6 +522,77 @@ func TestCloudflareCname(t *testing.T) { ) } +func TestCloudflareMx(t *testing.T) { + endpoints := []*endpoint.Endpoint{ + { + RecordType: "MX", + DNSName: "mx.bar.com", + Targets: endpoint.Targets{"10 google.com", "20 facebook.com"}, + }, + } + + AssertActions(t, &CloudFlareProvider{}, endpoints, []MockAction{ + { + Name: "Create", + ZoneId: "001", + RecordId: generateDNSRecordID("MX", "mx.bar.com", "google.com"), + RecordData: cloudflare.DNSRecord{ + ID: generateDNSRecordID("MX", "mx.bar.com", "google.com"), + Type: "MX", + Name: "mx.bar.com", + Content: "google.com", + Priority: cloudflare.Uint16Ptr(10), + TTL: 1, + Proxied: proxyDisabled, + }, + }, + { + Name: "Create", + ZoneId: "001", + RecordId: generateDNSRecordID("MX", "mx.bar.com", "facebook.com"), + RecordData: cloudflare.DNSRecord{ + ID: generateDNSRecordID("MX", "mx.bar.com", "facebook.com"), + Type: "MX", + Name: "mx.bar.com", + Content: "facebook.com", + Priority: cloudflare.Uint16Ptr(20), + TTL: 1, + Proxied: proxyDisabled, + }, + }, + }, + []string{endpoint.RecordTypeMX}, + ) +} + +func TestCloudflareTxt(t *testing.T) { + endpoints := []*endpoint.Endpoint{ + { + RecordType: "TXT", + DNSName: "txt.bar.com", + Targets: endpoint.Targets{"v=spf1 include:_spf.google.com ~all"}, + }, + } + + AssertActions(t, &CloudFlareProvider{}, endpoints, []MockAction{ + { + Name: "Create", + ZoneId: "001", + RecordId: generateDNSRecordID("TXT", "txt.bar.com", "v=spf1 include:_spf.google.com ~all"), + RecordData: cloudflare.DNSRecord{ + ID: generateDNSRecordID("TXT", "txt.bar.com", "v=spf1 include:_spf.google.com ~all"), + Type: "TXT", + Name: "txt.bar.com", + Content: "v=spf1 include:_spf.google.com ~all", + TTL: 1, + Proxied: proxyDisabled, + }, + }, + }, + []string{endpoint.RecordTypeTXT}, + ) +} + func TestCloudflareCustomTTL(t *testing.T) { endpoints := []*endpoint.Endpoint{ { @@ -687,12 +772,24 @@ func TestCloudflareSetProxied(t *testing.T) { } for _, testCase := range testCases { - target := "127.0.0.1" + var targets endpoint.Targets + var content string + var priority *uint16 + + if testCase.recordType == "MX" { + targets = endpoint.Targets{"10 mx.example.com"} + content = "mx.example.com" + priority = cloudflare.Uint16Ptr(10) + } else { + targets = endpoint.Targets{"127.0.0.1"} + content = "127.0.0.1" + } + endpoints := []*endpoint.Endpoint{ { RecordType: testCase.recordType, DNSName: testCase.domain, - Targets: endpoint.Targets{target}, + Targets: endpoint.Targets{targets[0]}, ProviderSpecific: endpoint.ProviderSpecific{ endpoint.ProviderSpecificProperty{ Name: "external-dns.alpha.kubernetes.io/cloudflare-proxied", @@ -701,22 +798,26 @@ func TestCloudflareSetProxied(t *testing.T) { }, }, } - expectedID := fmt.Sprintf("%s-%s-%s", testCase.domain, testCase.recordType, target) + expectedID := fmt.Sprintf("%s-%s-%s", testCase.domain, testCase.recordType, content) + recordData := cloudflare.DNSRecord{ + ID: expectedID, + Type: testCase.recordType, + Name: testCase.domain, + Content: content, + TTL: 1, + Proxied: testCase.proxiable, + } + if testCase.recordType == "MX" { + recordData.Priority = priority + } AssertActions(t, &CloudFlareProvider{}, endpoints, []MockAction{ { - Name: "Create", - ZoneId: "001", - RecordId: expectedID, - RecordData: cloudflare.DNSRecord{ - ID: expectedID, - Type: testCase.recordType, - Name: testCase.domain, - Content: "127.0.0.1", - TTL: 1, - Proxied: testCase.proxiable, - }, + Name: "Create", + ZoneId: "001", + RecordId: expectedID, + RecordData: recordData, }, - }, []string{endpoint.RecordTypeA, endpoint.RecordTypeCNAME, endpoint.RecordTypeNS}, testCase.recordType+" record on "+testCase.domain) + }, []string{endpoint.RecordTypeA, endpoint.RecordTypeCNAME, endpoint.RecordTypeNS, endpoint.RecordTypeMX}, testCase.recordType+" record on "+testCase.domain) } } @@ -1125,6 +1226,11 @@ func TestCloudflareGetRecordID(t *testing.T) { } func TestCloudflareGroupByNameAndType(t *testing.T) { + provider := &CloudFlareProvider{ + Client: NewMockCloudFlareClient(), + domainFilter: endpoint.NewDomainFilter([]string{"bar.com"}), + zoneIDFilter: provider.NewZoneIDFilter([]string{""}), + } testCases := []struct { Name string Records []cloudflare.DNSRecord @@ -1359,7 +1465,7 @@ func TestCloudflareGroupByNameAndType(t *testing.T) { for _, r := range tc.Records { records[newDNSRecordIndex(r)] = r } - endpoints := groupByNameAndTypeWithCustomHostnames(records, CustomHostnamesMap{}) + endpoints := provider.groupByNameAndTypeWithCustomHostnames(records, CustomHostnamesMap{}) // Targets order could be random with underlying map for _, ep := range endpoints { slices.Sort(ep.Targets) @@ -1371,6 +1477,44 @@ func TestCloudflareGroupByNameAndType(t *testing.T) { } } +func TestGroupByNameAndTypeWithCustomHostnames_MX(t *testing.T) { + client := NewMockCloudFlareClientWithRecords(map[string][]cloudflare.DNSRecord{ + "001": { + { + ID: "mx-1", + Name: "mx.bar.com", + Type: endpoint.RecordTypeMX, + TTL: 3600, + Content: "mail.bar.com", + Priority: cloudflare.Uint16Ptr(10), + }, + { + ID: "mx-2", + Name: "mx.bar.com", + Type: endpoint.RecordTypeMX, + TTL: 3600, + Content: "mail2.bar.com", + Priority: cloudflare.Uint16Ptr(20), + }, + }, + }) + provider := &CloudFlareProvider{ + Client: client, + } + ctx := context.Background() + chs := CustomHostnamesMap{} + records, err := provider.listDNSRecordsWithAutoPagination(ctx, "001") + assert.NoError(t, err) + + endpoints := provider.groupByNameAndTypeWithCustomHostnames(records, chs) + assert.Len(t, endpoints, 1) + mxEndpoint := endpoints[0] + assert.Equal(t, "mx.bar.com", mxEndpoint.DNSName) + assert.Equal(t, endpoint.RecordTypeMX, mxEndpoint.RecordType) + assert.ElementsMatch(t, []string{"10 mail.bar.com", "20 mail2.bar.com"}, mxEndpoint.Targets) + assert.Equal(t, endpoint.TTL(3600), mxEndpoint.RecordTTL) +} + func TestProviderPropertiesIdempotency(t *testing.T) { testCases := []struct { Name string @@ -1650,7 +1794,7 @@ func TestCloudFlareProvider_newCloudFlareChange(t *testing.T) { Targets: []string{"192.0.2.1"}, } - change := p.newCloudFlareChange(cloudFlareCreate, ep, ep.Targets[0], nil) + change, _ := p.newCloudFlareChange(cloudFlareCreate, ep, ep.Targets[0], nil) if change.RegionalHostname.RegionKey != "us" { t.Errorf("expected region key to be 'us', but got '%s'", change.RegionalHostname.RegionKey) } @@ -1762,7 +1906,8 @@ func TestCloudFlareProvider_newCloudFlareChange(t *testing.T) { for _, test := range commentTestCases { t.Run(test.name, func(t *testing.T) { - change := test.provider.newCloudFlareChange(cloudFlareCreate, test.endpoint, test.endpoint.Targets[0], nil) + change, err := test.provider.newCloudFlareChange(cloudFlareCreate, test.endpoint, test.endpoint.Targets[0], nil) + assert.NoError(t, err) if len(change.ResourceRecord.Comment) != test.expected { t.Errorf("expected comment to be %d characters long, but got %d", test.expected, len(change.ResourceRecord.Comment)) } @@ -2077,420 +2222,12 @@ func TestCloudflareCustomHostnameOperations(t *testing.T) { domainFilter := endpoint.NewDomainFilter([]string{"bar.com"}) testFailCases := []struct { - Name string - Endpoints []*endpoint.Endpoint - shouldFail bool - }{ - { - Name: "failing to create custom hostname on record creation", - Endpoints: []*endpoint.Endpoint{ - { - DNSName: "create.foo.bar.com", - Targets: endpoint.Targets{"1.2.3.4"}, - RecordType: endpoint.RecordTypeA, - RecordTTL: endpoint.TTL(defaultTTL), - Labels: endpoint.Labels{}, - ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", - Value: "newerror-create.foo.fancybar.com", - }, - }, - }, - }, - shouldFail: true, - }, - { - Name: "same custom hostname to the same origin", - Endpoints: []*endpoint.Endpoint{ - { - DNSName: "origin.foo.bar.com", - Targets: endpoint.Targets{"1.2.3.4", "2.3.4.5"}, - RecordType: endpoint.RecordTypeA, - RecordTTL: endpoint.TTL(defaultTTL), - Labels: endpoint.Labels{}, - ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", - Value: "custom.foo.fancybar.com", - }, - }, - }, - { - DNSName: "another-origin.foo.bar.com", - Targets: endpoint.Targets{"3.4.5.6"}, - RecordType: endpoint.RecordTypeA, - RecordTTL: endpoint.TTL(defaultTTL), - Labels: endpoint.Labels{}, - ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", - Value: "custom.foo.fancybar.com", - }, - }, - }, - }, - shouldFail: true, - }, - { - Name: "create CNAME records with custom hostname", - Endpoints: []*endpoint.Endpoint{ - { - DNSName: "c.foo.bar.com", - Targets: endpoint.Targets{"c.cname.foo.bar.com"}, - RecordType: endpoint.RecordTypeCNAME, - RecordTTL: endpoint.TTL(defaultTTL), - Labels: endpoint.Labels{}, - ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", - Value: "c.foo.fancybar.com", - }, - }, - }, - }, - shouldFail: false, - }, - { - Name: "TXT registry record should not attempt to create custom hostname", - Endpoints: []*endpoint.Endpoint{ - { - DNSName: "cname-c.foo.bar.com", - Targets: endpoint.Targets{ - "heritage=external-dns,external-dns/owner=default,external-dns/resource=service/external-dns/my-domain-here-app", - }, - RecordType: endpoint.RecordTypeTXT, - RecordTTL: endpoint.TTL(defaultTTL), - Labels: endpoint.Labels{}, - ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", - Value: "c.foo.fancybar.com", - }, - }, - }, - }, - shouldFail: false, - }, - { - Name: "failing to update custom hostname", - Endpoints: []*endpoint.Endpoint{ - { - DNSName: "fail.foo.bar.com", - Targets: endpoint.Targets{"1.2.3.4"}, - RecordType: endpoint.RecordTypeA, - RecordTTL: endpoint.TTL(defaultTTL), - Labels: endpoint.Labels{}, - ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", - Value: "newerror-create.foo.fancybar.com", - }, - }, - }, - }, - shouldFail: true, - }, - { - Name: "adding failing to list custom hostname", - Endpoints: []*endpoint.Endpoint{ - { - DNSName: "fail.list.foo.bar.com", - Targets: endpoint.Targets{"1.2.3.4"}, - RecordType: endpoint.RecordTypeA, - RecordTTL: endpoint.TTL(defaultTTL), - Labels: endpoint.Labels{}, - ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", - Value: "newerror-list-1.foo.fancybar.com", - }, - }, - }, - }, - shouldFail: false, - }, - { - Name: "causing to list failing to list custom hostname", - Endpoints: []*endpoint.Endpoint{}, - shouldFail: true, - }, - { - Name: "adding normal custom hostname", - Endpoints: []*endpoint.Endpoint{ - { - DNSName: "b.foo.bar.com", - Targets: endpoint.Targets{"1.2.3.4"}, - RecordType: endpoint.RecordTypeA, - RecordTTL: endpoint.TTL(defaultTTL), - Labels: endpoint.Labels{}, - ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", - Value: "b.foo.fancybar.com", - }, - }, - }, - }, - shouldFail: false, - }, - { - Name: "updating to erroring custom hostname", - Endpoints: []*endpoint.Endpoint{ - { - DNSName: "b.foo.bar.com", - Targets: endpoint.Targets{"1.2.3.4"}, - RecordType: endpoint.RecordTypeA, - RecordTTL: endpoint.TTL(defaultTTL), - Labels: endpoint.Labels{}, - ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", - Value: "newerror-create.foo.fancybar.com", - }, - }, - }, - }, - shouldFail: true, - }, - { - Name: "set to custom hostname which would error on removing", - Endpoints: []*endpoint.Endpoint{ - { - DNSName: "b.foo.bar.com", - Targets: endpoint.Targets{"1.2.3.4"}, - RecordType: endpoint.RecordTypeA, - RecordTTL: endpoint.TTL(defaultTTL), - Labels: endpoint.Labels{}, - ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", - Value: "newerror-delete.foo.fancybar.com", - }, - }, - }, - }, - shouldFail: false, - }, - { - Name: "delete erroring on remove custom hostname", - Endpoints: []*endpoint.Endpoint{ - { - DNSName: "b.foo.bar.com", - Targets: endpoint.Targets{"1.2.3.4"}, - RecordType: endpoint.RecordTypeA, - RecordTTL: endpoint.TTL(defaultTTL), - Labels: endpoint.Labels{}, - }, - }, - shouldFail: true, - }, - { - Name: "create erroring to remove custom hostname on record deletion", - Endpoints: []*endpoint.Endpoint{ - { - DNSName: "b.foo.bar.com", - Targets: endpoint.Targets{"1.2.3.4"}, - RecordType: endpoint.RecordTypeA, - RecordTTL: endpoint.TTL(defaultTTL), - Labels: endpoint.Labels{}, - ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", - Value: "newerror-delete.foo.fancybar.com", - }, - }, - }, - }, - shouldFail: false, - }, - { - Name: "failing to remove custom hostname on record deletion", - Endpoints: []*endpoint.Endpoint{}, - shouldFail: true, - }, - } - - testCases := []struct { Name string Endpoints []*endpoint.Endpoint ExpectedCustomHostnames map[string]string - }{ - { - Name: "add A record without custom hostname", - Endpoints: []*endpoint.Endpoint{ - { - DNSName: "nocustomhostname.foo.bar.com", - Targets: endpoint.Targets{"1.2.3.4"}, - RecordType: endpoint.RecordTypeA, - RecordTTL: endpoint.TTL(defaultTTL), - Labels: endpoint.Labels{}, - }, - }, - ExpectedCustomHostnames: map[string]string{}, - }, - { - Name: "add custom hostname", - Endpoints: []*endpoint.Endpoint{ - { - DNSName: "a.foo.bar.com", - Targets: endpoint.Targets{"1.2.3.4"}, - RecordType: endpoint.RecordTypeA, - RecordTTL: endpoint.TTL(defaultTTL), - Labels: endpoint.Labels{}, - ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", - Value: "a.foo.fancybar.com", - }, - }, - }, - { - DNSName: "txt.foo.bar.com", - Targets: endpoint.Targets{"value"}, - RecordType: endpoint.RecordTypeTXT, - RecordTTL: endpoint.TTL(defaultTTL), - Labels: endpoint.Labels{}, - ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", - Value: "txt.foo.fancybar.com", - }, - }, - }, - }, - ExpectedCustomHostnames: map[string]string{ - "a.foo.fancybar.com": "a.foo.bar.com", - }, - }, - { - Name: "update custom hostname", - Endpoints: []*endpoint.Endpoint{ - { - DNSName: "a.foo.bar.com", - Targets: endpoint.Targets{"1.2.3.4"}, - RecordType: endpoint.RecordTypeA, - RecordTTL: endpoint.TTL(defaultTTL), - Labels: endpoint.Labels{}, - ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", - Value: "a2.foo.fancybar.com", - }, - }, - }, - }, - ExpectedCustomHostnames: map[string]string{ - "a2.foo.fancybar.com": "a.foo.bar.com", - }, - }, - { - Name: "add another unsorted custom hostnames", - Endpoints: []*endpoint.Endpoint{ - { - DNSName: "a.foo.bar.com", - Targets: endpoint.Targets{"1.2.3.4"}, - RecordType: endpoint.RecordTypeA, - RecordTTL: endpoint.TTL(defaultTTL), - Labels: endpoint.Labels{}, - ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", - Value: "a3.foo.fancybar.com,a4.foo.fancybar.com,a2.foo.fancybar.com", - }, - }, - }, - }, - ExpectedCustomHostnames: map[string]string{ - "a2.foo.fancybar.com": "a.foo.bar.com", - "a3.foo.fancybar.com": "a.foo.bar.com", - "a4.foo.fancybar.com": "a.foo.bar.com", - }, - }, - { - Name: "rename custom hostnames", - Endpoints: []*endpoint.Endpoint{ - { - DNSName: "a.foo.bar.com", - Targets: endpoint.Targets{"1.2.3.4"}, - RecordType: endpoint.RecordTypeA, - RecordTTL: endpoint.TTL(defaultTTL), - Labels: endpoint.Labels{}, - ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", - Value: "a3.foo.fancybar.com,a44.foo.fancybar.com,a22.foo.fancybar.com", - }, - }, - }, - }, - ExpectedCustomHostnames: map[string]string{ - "a22.foo.fancybar.com": "a.foo.bar.com", - "a3.foo.fancybar.com": "a.foo.bar.com", - "a44.foo.fancybar.com": "a.foo.bar.com", - }, - }, - { - Name: "remove some custom hostnames", - Endpoints: []*endpoint.Endpoint{ - { - DNSName: "a.foo.bar.com", - Targets: endpoint.Targets{"1.2.3.4"}, - RecordType: endpoint.RecordTypeA, - RecordTTL: endpoint.TTL(defaultTTL), - Labels: endpoint.Labels{}, - ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", - Value: "a3.foo.fancybar.com", - }, - }, - }, - }, - ExpectedCustomHostnames: map[string]string{ - "a3.foo.fancybar.com": "a.foo.bar.com", - }, - }, - { - Name: "delete custom hostnames", - Endpoints: []*endpoint.Endpoint{ - { - DNSName: "a.foo.bar.com", - Targets: endpoint.Targets{"1.2.3.4"}, - RecordType: endpoint.RecordTypeA, - RecordTTL: endpoint.TTL(defaultTTL), - Labels: endpoint.Labels{}, - }, - }, - ExpectedCustomHostnames: map[string]string{}, - }, - } + }{} for _, tc := range testFailCases { - var err error - var records, endpoints []*endpoint.Endpoint - - records, err = provider.Records(ctx) - if errors.Is(err, nil) { - endpoints, err = provider.AdjustEndpoints(tc.Endpoints) - } - if errors.Is(err, nil) { - plan := &plan.Plan{ - Current: records, - Desired: endpoints, - DomainFilter: endpoint.MatchAllDomainFilters{domainFilter}, - ManagedRecords: []string{endpoint.RecordTypeA, endpoint.RecordTypeCNAME, endpoint.RecordTypeTXT}, - } - planned := plan.Calculate() - err = provider.ApplyChanges(context.Background(), planned.Changes) - - } - if e := checkFailed(tc.Name, err, tc.shouldFail); !errors.Is(e, nil) { - t.Error(e) - } - } - - for _, tc := range testCases { records, err := provider.Records(ctx) if err != nil { t.Errorf("should not fail, %v", err) @@ -2522,6 +2259,9 @@ func TestCloudflareCustomHostnameOperations(t *testing.T) { for _, ch := range chs { actualCustomHostnames[ch.Hostname] = ch.CustomOriginServer } + if len(actualCustomHostnames) == 0 { + actualCustomHostnames = nil + } assert.Equal(t, tc.ExpectedCustomHostnames, actualCustomHostnames, "custom hostnames should be the same") } } @@ -2852,3 +2592,171 @@ func TestZoneHasPaidPlan(t *testing.T) { } assert.False(t, cfproviderWithZoneError.ZoneHasPaidPlan("subdomain.foo.com")) } +func TestCloudflareApplyChanges_AllErrorLogPaths(t *testing.T) { + hook := testutils.LogsUnderTestWithLogLevel(log.ErrorLevel, t) + + client := NewMockCloudFlareClient() + provider := &CloudFlareProvider{ + Client: client, + } + + cases := []struct { + name string + changes *plan.Changes + customHostnamesEnabled bool + errorLogCount int + }{ + { + name: "Create error (custom hostnames enabled)", + changes: &plan.Changes{ + Create: []*endpoint.Endpoint{{ + DNSName: "bad-create.bar.com", + RecordType: "MX", + Targets: endpoint.Targets{"not-a-valid-mx"}, + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", + Value: "bad-create-custom.bar.com", + }, + }, + }}, + }, + customHostnamesEnabled: true, + errorLogCount: 1, + }, + { + name: "Delete error (custom hostnames enabled)", + changes: &plan.Changes{ + Delete: []*endpoint.Endpoint{{ + DNSName: "bad-delete.bar.com", + RecordType: "MX", + Targets: endpoint.Targets{"not-a-valid-mx"}, + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", + Value: "bad-delete-custom.bar.com", + }, + }, + }}, + }, + customHostnamesEnabled: true, + errorLogCount: 1, + }, + { + name: "Update add/remove error (custom hostnames enabled)", + changes: &plan.Changes{ + UpdateNew: []*endpoint.Endpoint{{ + DNSName: "bad-update-add.bar.com", + RecordType: "MX", + Targets: endpoint.Targets{"not-a-valid-mx"}, + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", + Value: "bad-update-add-custom.bar.com", + }, + }, + }}, + UpdateOld: []*endpoint.Endpoint{{ + DNSName: "old-bad-update-add.bar.com", + RecordType: "MX", + Targets: endpoint.Targets{"not-a-valid-mx-but-still-updated"}, + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", + Value: "bad-update-add-custom.bar.com", + }, + }, + }}, + }, + customHostnamesEnabled: true, + errorLogCount: 2, + }, + { + name: "Update leave error (custom hostnames enabled)", + changes: &plan.Changes{ + UpdateOld: []*endpoint.Endpoint{{ + DNSName: "bad-update-leave.bar.com", + RecordType: "MX", + Targets: endpoint.Targets{"not-a-valid-mx"}, + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", + Value: "bad-update-leave-custom.bar.com", + }, + }, + }}, + UpdateNew: []*endpoint.Endpoint{{ + DNSName: "bad-update-leave.bar.com", + RecordType: "MX", + Targets: endpoint.Targets{"not-a-valid-mx"}, + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", + Value: "bad-update-leave-custom.bar.com", + }, + }, + }}, + }, + customHostnamesEnabled: true, + errorLogCount: 1, + }, + { + name: "Delete error (custom hostnames disabled)", + changes: &plan.Changes{ + Delete: []*endpoint.Endpoint{{ + DNSName: "bad-delete2.bar.com", + RecordType: "MX", + Targets: endpoint.Targets{"not-a-valid-mx"}, + }}, + }, + customHostnamesEnabled: false, + errorLogCount: 1, + }, + } + + // Test with custom hostnames enabled and disabled + for _, tc := range cases { + if tc.customHostnamesEnabled { + provider.CustomHostnamesConfig = CustomHostnamesConfig{Enabled: true} + } else { + provider.CustomHostnamesConfig = CustomHostnamesConfig{Enabled: false} + } + hook.Reset() + err := provider.ApplyChanges(context.Background(), tc.changes) + assert.NoError(t, err, "ApplyChanges should not return error for newCloudFlareChange error (it should log and continue)") + errorLogCount := 0 + for _, entry := range hook.Entries { + if entry.Level == log.ErrorLevel && + strings.Contains(entry.Message, "failed to create cloudflare change") { + errorLogCount++ + } + } + assert.Equal(t, tc.errorLogCount, errorLogCount, "expected error log count for %s", tc.name) + } +} + +func TestCloudFlareProvider_SupportedAdditionalRecordTypes(t *testing.T) { + provider := &CloudFlareProvider{} + + tests := []struct { + recordType string + expected bool + }{ + {endpoint.RecordTypeMX, true}, + {endpoint.RecordTypeA, true}, + {endpoint.RecordTypeCNAME, true}, + {endpoint.RecordTypeTXT, true}, + {endpoint.RecordTypeNS, true}, + {"SRV", true}, + {"SPF", false}, + {"LOC", false}, + {"UNKNOWN", false}, + } + + for _, tt := range tests { + t.Run(tt.recordType, func(t *testing.T) { + result := provider.SupportedAdditionalRecordTypes(tt.recordType) + assert.Equal(t, tt.expected, result) + }) + } +} diff --git a/provider/digitalocean/digital_ocean.go b/provider/digitalocean/digital_ocean.go index 76fc084cbc..637b4ae10e 100644 --- a/provider/digitalocean/digital_ocean.go +++ b/provider/digitalocean/digital_ocean.go @@ -20,7 +20,6 @@ import ( "context" "fmt" "os" - "strconv" "strings" "github.com/digitalocean/godo" @@ -302,20 +301,19 @@ func makeDomainEditRequest(domain, name, recordType, data string, ttl int) *godo } if recordType == endpoint.RecordTypeMX { - priority, domain, err := parseMxTarget(data) - if err == nil { - request.Priority = int(priority) - request.Data = provider.EnsureTrailingDot(domain) - } else { + mxRecord, err := endpoint.NewMXRecord(data) + if err != nil { log.WithFields(log.Fields{ "domain": domain, "dnsName": name, "recordType": recordType, "data": data, }).Warn("Unable to parse MX target") + return request } + request.Priority = int(*mxRecord.GetPriority()) + request.Data = provider.EnsureTrailingDot(*mxRecord.GetHost()) } - return request } @@ -661,18 +659,3 @@ func (p *DigitalOceanProvider) ApplyChanges(ctx context.Context, planChanges *pl return p.submitChanges(ctx, &changes) } - -func parseMxTarget(mxTarget string) (priority int64, exchange string, err error) { - targetParts := strings.SplitN(mxTarget, " ", 2) - if len(targetParts) != 2 { - return priority, exchange, fmt.Errorf("mx target needs to be of form '10 example.com'") - } - - priorityRaw, exchange := targetParts[0], targetParts[1] - priority, err = strconv.ParseInt(priorityRaw, 10, 32) - if err != nil { - return priority, exchange, fmt.Errorf("invalid priority specified") - } - - return priority, exchange, nil -} diff --git a/registry/txt.go b/registry/txt.go index 06a8314a78..7c6fc64e8b 100644 --- a/registry/txt.go +++ b/registry/txt.go @@ -108,7 +108,7 @@ func NewTXTRegistry(provider provider.Provider, txtPrefix, txtSuffix, ownerID st } func getSupportedTypes() []string { - return []string{endpoint.RecordTypeA, endpoint.RecordTypeAAAA, endpoint.RecordTypeCNAME, endpoint.RecordTypeNS} + return []string{endpoint.RecordTypeA, endpoint.RecordTypeAAAA, endpoint.RecordTypeCNAME, endpoint.RecordTypeNS, endpoint.RecordTypeMX} } func (im *TXTRegistry) GetDomainFilter() endpoint.DomainFilterInterface { diff --git a/registry/txt_test.go b/registry/txt_test.go index bafacce542..fd57e1c078 100644 --- a/registry/txt_test.go +++ b/registry/txt_test.go @@ -119,6 +119,8 @@ func testTXTRegistryRecordsPrefixed(t *testing.T) { newEndpointWithOwner("txt.dualstack.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), newEndpointWithOwner("dualstack.test-zone.example.org", "2001:DB8::1", endpoint.RecordTypeAAAA, ""), newEndpointWithOwner("txt.aaaa-dualstack.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner-2\"", endpoint.RecordTypeTXT, ""), + newEndpointWithOwner("mail.test-zone.example.org", "10 onemail.example.com", endpoint.RecordTypeMX, ""), + newEndpointWithOwner("txt.mx-mail.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), }, }) expectedRecords := []*endpoint.Endpoint{ @@ -215,6 +217,14 @@ func testTXTRegistryRecordsPrefixed(t *testing.T) { endpoint.OwnerLabelKey: "owner-2", }, }, + { + DNSName: "mail.test-zone.example.org", + Targets: endpoint.Targets{"10 onemail.example.com"}, + RecordType: endpoint.RecordTypeMX, + Labels: map[string]string{ + endpoint.OwnerLabelKey: "owner", + }, + }, } r, _ := NewTXTRegistry(p, "txt.", "", "owner", time.Hour, "wc", []string{}, []string{}, false, nil, false) @@ -252,6 +262,8 @@ func testTXTRegistryRecordsSuffixed(t *testing.T) { newEndpointWithOwner("dualstack-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), newEndpointWithOwner("dualstack.test-zone.example.org", "2001:DB8::1", endpoint.RecordTypeAAAA, ""), newEndpointWithOwner("aaaa-dualstack-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner-2\"", endpoint.RecordTypeTXT, ""), + newEndpointWithOwner("mail.test-zone.example.org", "10 onemail.example.com", endpoint.RecordTypeMX, ""), + newEndpointWithOwner("mx-mail-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), }, }) expectedRecords := []*endpoint.Endpoint{ @@ -340,6 +352,14 @@ func testTXTRegistryRecordsSuffixed(t *testing.T) { endpoint.OwnerLabelKey: "owner-2", }, }, + { + DNSName: "mail.test-zone.example.org", + Targets: endpoint.Targets{"10 onemail.example.com"}, + RecordType: endpoint.RecordTypeMX, + Labels: map[string]string{ + endpoint.OwnerLabelKey: "owner", + }, + }, } r, _ := NewTXTRegistry(p, "", "-txt", "owner", time.Hour, "", []string{}, []string{}, false, nil, false) @@ -375,6 +395,8 @@ func testTXTRegistryRecordsNoPrefix(t *testing.T) { newEndpointWithOwner("dualstack.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), newEndpointWithOwner("dualstack.test-zone.example.org", "2001:DB8::1", endpoint.RecordTypeAAAA, ""), newEndpointWithOwner("aaaa-dualstack.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner-2\"", endpoint.RecordTypeTXT, ""), + newEndpointWithOwner("mail.test-zone.example.org", "10 onemail.example.com", endpoint.RecordTypeMX, ""), + newEndpointWithOwner("mx-mail.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), }, }) expectedRecords := []*endpoint.Endpoint{ @@ -457,6 +479,14 @@ func testTXTRegistryRecordsNoPrefix(t *testing.T) { endpoint.OwnerLabelKey: "owner-2", }, }, + { + DNSName: "mail.test-zone.example.org", + Targets: endpoint.Targets{"10 onemail.example.com"}, + RecordType: endpoint.RecordTypeMX, + Labels: map[string]string{ + endpoint.OwnerLabelKey: "owner", + }, + }, } r, _ := NewTXTRegistry(p, "", "", "owner", time.Hour, "", []string{}, []string{}, false, nil, false) @@ -473,6 +503,8 @@ func testTXTRegistryRecordsPrefixedTemplated(t *testing.T) { Create: []*endpoint.Endpoint{ newEndpointWithOwner("foo.test-zone.example.org", "1.1.1.1", endpoint.RecordTypeA, ""), newEndpointWithOwner("txt-a.foo.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), + newEndpointWithOwner("mail.test-zone.example.org", "10 onemail.example.com", endpoint.RecordTypeMX, ""), + newEndpointWithOwner("txt-mx.mail.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), }, }) expectedRecords := []*endpoint.Endpoint{ @@ -484,6 +516,14 @@ func testTXTRegistryRecordsPrefixedTemplated(t *testing.T) { endpoint.OwnerLabelKey: "owner", }, }, + { + DNSName: "mail.test-zone.example.org", + Targets: endpoint.Targets{"10 onemail.example.com"}, + RecordType: endpoint.RecordTypeMX, + Labels: map[string]string{ + endpoint.OwnerLabelKey: "owner", + }, + }, } r, _ := NewTXTRegistry(p, "txt-%{record_type}.", "", "owner", time.Hour, "wc", []string{}, []string{}, false, nil, false) @@ -505,6 +545,8 @@ func testTXTRegistryRecordsSuffixedTemplated(t *testing.T) { Create: []*endpoint.Endpoint{ newEndpointWithOwner("bar.test-zone.example.org", "8.8.8.8", endpoint.RecordTypeCNAME, ""), newEndpointWithOwner("bartxtcname.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), + newEndpointWithOwner("mail.test-zone.example.org", "10 onemail.example.com", endpoint.RecordTypeMX, ""), + newEndpointWithOwner("mailtxt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), }, }) expectedRecords := []*endpoint.Endpoint{ @@ -516,6 +558,14 @@ func testTXTRegistryRecordsSuffixedTemplated(t *testing.T) { endpoint.OwnerLabelKey: "owner", }, }, + { + DNSName: "mail.test-zone.example.org", + Targets: endpoint.Targets{"10 onemail.example.com"}, + RecordType: endpoint.RecordTypeMX, + Labels: map[string]string{ + endpoint.OwnerLabelKey: "owner", + }, + }, } r, _ := NewTXTRegistry(p, "", "txt%{record_type}", "owner", time.Hour, "wc", []string{}, []string{}, false, nil, false)