From 8d03e5e2d35a5f4bbb83eb6a99a8a7d042f239a7 Mon Sep 17 00:00:00 2001 From: Andrew Hay <39sumer3939@gmail.com> Date: Thu, 31 Jul 2025 16:56:36 +0000 Subject: [PATCH] docs: updated notes around migration --- .gitignore | 1 + docs/tutorials/cloudflare.md | 16 +- provider/cloudflare/cloudflare.go | 68 +++- provider/cloudflare/cloudflare_proxied.go | 147 +++++++- .../cloudflare/cloudflare_proxied_test.go | 353 +++++++++++++++++- provider/cloudflare/cloudflare_test.go | 321 +++++++++++++++- provider/oci/cache_test.go | 5 +- 7 files changed, 866 insertions(+), 45 deletions(-) diff --git a/.gitignore b/.gitignore index 29561b906..1de67239a 100644 --- a/.gitignore +++ b/.gitignore @@ -42,6 +42,7 @@ cscope.* # coverage output cover.out +*coverage.out *.coverprofile external-dns diff --git a/docs/tutorials/cloudflare.md b/docs/tutorials/cloudflare.md index 4afb55e24..9dad7b7a9 100644 --- a/docs/tutorials/cloudflare.md +++ b/docs/tutorials/cloudflare.md @@ -15,12 +15,16 @@ ExternalDNS is currently migrating from the legacy CloudFlare Go SDK v0 to the m - Zone ID lookup by name (`ZoneIDByName`) - Zone plan detection (fully v4 implementation) - Regional services (data localization) +- **Proxied records** (DNS records with Cloudflare proxy enabled) **🔄 Still using legacy v0 SDK:** -- DNS record management (create, update, delete records) +- DNS record management for non-proxied records (create, update, delete records) - Custom hostnames -- Proxied records + +**🔀 Hybrid approach:** + +- **DNS record operations**: ExternalDNS now intelligently uses v4 SDK for proxied records (those with `Proxied: true`) and v0 SDK for non-proxied records, ensuring optimal performance for proxy-enabled features while maintaining backward compatibility. This mixed approach ensures continued functionality while gradually modernizing the codebase. Users should not experience any breaking changes during this transition. @@ -28,10 +32,10 @@ This mixed approach ensures continued functionality while gradually modernizing ExternalDNS currently uses: -- **cloudflare-go v0.115.0+**: Legacy SDK for DNS records, custom hostnames, and proxied record features -- **cloudflare-go/v4 v4.6.0+**: Modern SDK for all zone management and regional services operations +- **cloudflare-go v0.115.0+**: Legacy SDK for non-proxied DNS records and custom hostnames +- **cloudflare-go/v4 v4.6.0+**: Modern SDK for zone management, regional services, and proxied DNS records -Zone management has been fully migrated to the v4 SDK, providing improved performance and reliability. +Zone management and proxied records have been fully migrated to the v4 SDK, providing improved performance and reliability for proxy-enabled features. Both SDKs are automatically managed as Go module dependencies and require no special configuration from users. @@ -339,6 +343,8 @@ kubectl delete -f externaldns.yaml Using the `external-dns.alpha.kubernetes.io/cloudflare-proxied: "true"` annotation on your ingress, you can specify if the proxy feature of Cloudflare should be enabled for that record. This setting will override the global `--cloudflare-proxied` setting. +**Note:** Proxied records (`Proxied: true`) now use the modern Cloudflare v4 SDK, providing enhanced performance and access to the latest Cloudflare proxy features. Non-proxied records continue to use the v0 SDK for backward compatibility. + ## Setting cloudlfare regional services With Cloudflare regional services you can restrict which data centers can decrypt and serve HTTPS traffic. diff --git a/provider/cloudflare/cloudflare.go b/provider/cloudflare/cloudflare.go index 0b432b3f3..9a31f5a31 100644 --- a/provider/cloudflare/cloudflare.go +++ b/provider/cloudflare/cloudflare.go @@ -31,6 +31,7 @@ import ( "github.com/cloudflare/cloudflare-go" cloudflarev4 "github.com/cloudflare/cloudflare-go/v4" "github.com/cloudflare/cloudflare-go/v4/addressing" + "github.com/cloudflare/cloudflare-go/v4/dns" "github.com/cloudflare/cloudflare-go/v4/option" "github.com/cloudflare/cloudflare-go/v4/zones" log "github.com/sirupsen/logrus" @@ -113,6 +114,10 @@ type cloudFlareDNS interface { CreateDNSRecord(ctx context.Context, rc *cloudflare.ResourceContainer, rp cloudflare.CreateDNSRecordParams) (cloudflare.DNSRecord, error) DeleteDNSRecord(ctx context.Context, rc *cloudflare.ResourceContainer, recordID string) error UpdateDNSRecord(ctx context.Context, rc *cloudflare.ResourceContainer, rp cloudflare.UpdateDNSRecordParams) error + // v4 DNS record operations for proxied records + CreateDNSRecordV4(ctx context.Context, params dns.RecordNewParams) (*dns.RecordResponse, error) + UpdateDNSRecordV4(ctx context.Context, recordID string, params dns.RecordUpdateParams) (*dns.RecordResponse, error) + DeleteDNSRecordV4(ctx context.Context, recordID string, params dns.RecordDeleteParams) (*dns.RecordDeleteResponse, error) ListDataLocalizationRegionalHostnames(ctx context.Context, params addressing.RegionalHostnameListParams) autoPager[addressing.RegionalHostnameListResponse] CreateDataLocalizationRegionalHostname(ctx context.Context, params addressing.RegionalHostnameNewParams) error UpdateDataLocalizationRegionalHostname(ctx context.Context, hostname string, params addressing.RegionalHostnameEditParams) error @@ -164,6 +169,19 @@ func (z zoneService) DeleteDNSRecord(ctx context.Context, rc *cloudflare.Resourc return z.service.DeleteDNSRecord(ctx, rc, recordID) } +// v4 DNS record operations for proxied records +func (z zoneService) CreateDNSRecordV4(ctx context.Context, params dns.RecordNewParams) (*dns.RecordResponse, error) { + return z.serviceV4.DNS.Records.New(ctx, params) +} + +func (z zoneService) UpdateDNSRecordV4(ctx context.Context, recordID string, params dns.RecordUpdateParams) (*dns.RecordResponse, error) { + return z.serviceV4.DNS.Records.Update(ctx, recordID, params) +} + +func (z zoneService) DeleteDNSRecordV4(ctx context.Context, recordID string, params dns.RecordDeleteParams) (*dns.RecordDeleteResponse, error) { + return z.serviceV4.DNS.Records.Delete(ctx, recordID, params) +} + func (z zoneService) ListZones(ctx context.Context, params zones.ZoneListParams) autoPager[zones.Zone] { return z.serviceV4.Zones.ListAutoPaging(ctx, params) } @@ -291,6 +309,13 @@ func getCreateDNSRecordParam(cfc cloudFlareChange) cloudflare.CreateDNSRecordPar return params } +// shouldUseV4ForRecord determines if we should use v4 SDK for this record +// Currently migrating proxied records to v4 SDK +func shouldUseV4ForRecord(cfc cloudFlareChange) bool { + // Use v4 only for records that are actually proxied (migration of proxied functionality) + return cfc.ResourceRecord.Proxied != nil && *cfc.ResourceRecord.Proxied +} + func convertCloudflareError(err error) error { var apiErr *cloudflare.Error if errors.As(err, &apiErr) { @@ -651,12 +676,23 @@ func (p *CloudFlareProvider) submitChanges(ctx context.Context, changes []*cloud log.WithFields(logFields).Errorf("failed to find previous record: %v", change.ResourceRecord) continue } - recordParam := updateDNSRecordParam(*change) - recordParam.ID = recordID - err := p.Client.UpdateDNSRecord(ctx, resourceContainer, recordParam) - if err != nil { - failedChange = true - log.WithFields(logFields).Errorf("failed to update record: %v", err) + + // Use v4 SDK for proxied records migration + if shouldUseV4ForRecord(*change) { + recordParamV4 := updateDNSRecordParamV4(*change, zoneID) + _, err := p.Client.UpdateDNSRecordV4(ctx, recordID, recordParamV4) + if err != nil { + failedChange = true + log.WithFields(logFields).Errorf("failed to update record (v4): %v", err) + } + } else { + recordParam := updateDNSRecordParam(*change) + recordParam.ID = recordID + err := p.Client.UpdateDNSRecord(ctx, resourceContainer, recordParam) + if err != nil { + failedChange = true + log.WithFields(logFields).Errorf("failed to update record: %v", err) + } } } else if change.Action == cloudFlareDelete { recordID := p.getRecordID(records, change.ResourceRecord) @@ -673,11 +709,21 @@ func (p *CloudFlareProvider) submitChanges(ctx context.Context, changes []*cloud failedChange = true } } else if change.Action == cloudFlareCreate { - recordParam := getCreateDNSRecordParam(*change) - _, err := p.Client.CreateDNSRecord(ctx, resourceContainer, recordParam) - if err != nil { - failedChange = true - log.WithFields(logFields).Errorf("failed to create record: %v", err) + // Use v4 SDK for proxied records migration + if shouldUseV4ForRecord(*change) { + recordParamV4 := getCreateDNSRecordParamV4(*change, zoneID) + _, err := p.Client.CreateDNSRecordV4(ctx, recordParamV4) + if err != nil { + failedChange = true + log.WithFields(logFields).Errorf("failed to create record (v4): %v", err) + } + } else { + recordParam := getCreateDNSRecordParam(*change) + _, err := p.Client.CreateDNSRecord(ctx, resourceContainer, recordParam) + if err != nil { + failedChange = true + log.WithFields(logFields).Errorf("failed to create record: %v", err) + } } if !p.submitCustomHostnameChanges(ctx, zoneID, change, chs, logFields) { failedChange = true diff --git a/provider/cloudflare/cloudflare_proxied.go b/provider/cloudflare/cloudflare_proxied.go index 522a33a72..077a77c01 100644 --- a/provider/cloudflare/cloudflare_proxied.go +++ b/provider/cloudflare/cloudflare_proxied.go @@ -1,3 +1,148 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + package cloudflare -// Test comment +import ( + cloudflarev4 "github.com/cloudflare/cloudflare-go/v4" + "github.com/cloudflare/cloudflare-go/v4/dns" +) + +// updateDNSRecordParamV4 converts cloudFlareChange to v4 SDK UpdateDNSRecordParams +func updateDNSRecordParamV4(cfc cloudFlareChange, zoneID string) dns.RecordUpdateParams { + params := dns.RecordUpdateParams{ + ZoneID: cloudflarev4.F(zoneID), + } + + switch cfc.ResourceRecord.Type { + case "A": + aRecord := dns.ARecordParam{ + Name: cloudflarev4.F(cfc.ResourceRecord.Name), + Content: cloudflarev4.F(cfc.ResourceRecord.Content), + TTL: cloudflarev4.F(dns.TTL(cfc.ResourceRecord.TTL)), + Comment: cloudflarev4.F(cfc.ResourceRecord.Comment), + } + if cfc.ResourceRecord.Proxied != nil { + aRecord.Proxied = cloudflarev4.F(*cfc.ResourceRecord.Proxied) + } + params.Body = aRecord + case "AAAA": + aaaaRecord := dns.AAAARecordParam{ + Name: cloudflarev4.F(cfc.ResourceRecord.Name), + Content: cloudflarev4.F(cfc.ResourceRecord.Content), + TTL: cloudflarev4.F(dns.TTL(cfc.ResourceRecord.TTL)), + Comment: cloudflarev4.F(cfc.ResourceRecord.Comment), + } + if cfc.ResourceRecord.Proxied != nil { + aaaaRecord.Proxied = cloudflarev4.F(*cfc.ResourceRecord.Proxied) + } + params.Body = aaaaRecord + case "CNAME": + cnameRecord := dns.CNAMERecordParam{ + Name: cloudflarev4.F(cfc.ResourceRecord.Name), + Content: cloudflarev4.F(cfc.ResourceRecord.Content), + TTL: cloudflarev4.F(dns.TTL(cfc.ResourceRecord.TTL)), + Comment: cloudflarev4.F(cfc.ResourceRecord.Comment), + } + if cfc.ResourceRecord.Proxied != nil { + cnameRecord.Proxied = cloudflarev4.F(*cfc.ResourceRecord.Proxied) + } + params.Body = cnameRecord + case "MX": + mxRecord := dns.MXRecordParam{ + Name: cloudflarev4.F(cfc.ResourceRecord.Name), + Content: cloudflarev4.F(cfc.ResourceRecord.Content), + TTL: cloudflarev4.F(dns.TTL(cfc.ResourceRecord.TTL)), + Comment: cloudflarev4.F(cfc.ResourceRecord.Comment), + } + if cfc.ResourceRecord.Priority != nil { + mxRecord.Priority = cloudflarev4.F(float64(*cfc.ResourceRecord.Priority)) + } + params.Body = mxRecord + case "TXT": + params.Body = dns.TXTRecordParam{ + Name: cloudflarev4.F(cfc.ResourceRecord.Name), + Content: cloudflarev4.F(cfc.ResourceRecord.Content), + TTL: cloudflarev4.F(dns.TTL(cfc.ResourceRecord.TTL)), + Comment: cloudflarev4.F(cfc.ResourceRecord.Comment), + } + } + + return params +} + +// getCreateDNSRecordParamV4 converts cloudFlareChange to v4 SDK CreateDNSRecordParams +func getCreateDNSRecordParamV4(cfc cloudFlareChange, zoneID string) dns.RecordNewParams { + params := dns.RecordNewParams{ + ZoneID: cloudflarev4.F(zoneID), + } + + switch cfc.ResourceRecord.Type { + case "A": + aRecord := dns.ARecordParam{ + Name: cloudflarev4.F(cfc.ResourceRecord.Name), + Content: cloudflarev4.F(cfc.ResourceRecord.Content), + TTL: cloudflarev4.F(dns.TTL(cfc.ResourceRecord.TTL)), + Comment: cloudflarev4.F(cfc.ResourceRecord.Comment), + } + if cfc.ResourceRecord.Proxied != nil { + aRecord.Proxied = cloudflarev4.F(*cfc.ResourceRecord.Proxied) + } + params.Body = aRecord + case "AAAA": + aaaaRecord := dns.AAAARecordParam{ + Name: cloudflarev4.F(cfc.ResourceRecord.Name), + Content: cloudflarev4.F(cfc.ResourceRecord.Content), + TTL: cloudflarev4.F(dns.TTL(cfc.ResourceRecord.TTL)), + Comment: cloudflarev4.F(cfc.ResourceRecord.Comment), + } + if cfc.ResourceRecord.Proxied != nil { + aaaaRecord.Proxied = cloudflarev4.F(*cfc.ResourceRecord.Proxied) + } + params.Body = aaaaRecord + case "CNAME": + cnameRecord := dns.CNAMERecordParam{ + Name: cloudflarev4.F(cfc.ResourceRecord.Name), + Content: cloudflarev4.F(cfc.ResourceRecord.Content), + TTL: cloudflarev4.F(dns.TTL(cfc.ResourceRecord.TTL)), + Comment: cloudflarev4.F(cfc.ResourceRecord.Comment), + } + if cfc.ResourceRecord.Proxied != nil { + cnameRecord.Proxied = cloudflarev4.F(*cfc.ResourceRecord.Proxied) + } + params.Body = cnameRecord + case "MX": + mxRecord := dns.MXRecordParam{ + Name: cloudflarev4.F(cfc.ResourceRecord.Name), + Content: cloudflarev4.F(cfc.ResourceRecord.Content), + TTL: cloudflarev4.F(dns.TTL(cfc.ResourceRecord.TTL)), + Comment: cloudflarev4.F(cfc.ResourceRecord.Comment), + } + if cfc.ResourceRecord.Priority != nil { + mxRecord.Priority = cloudflarev4.F(float64(*cfc.ResourceRecord.Priority)) + } + params.Body = mxRecord + case "TXT": + params.Body = dns.TXTRecordParam{ + Name: cloudflarev4.F(cfc.ResourceRecord.Name), + Content: cloudflarev4.F(cfc.ResourceRecord.Content), + TTL: cloudflarev4.F(dns.TTL(cfc.ResourceRecord.TTL)), + Comment: cloudflarev4.F(cfc.ResourceRecord.Comment), + } + } + + return params +} diff --git a/provider/cloudflare/cloudflare_proxied_test.go b/provider/cloudflare/cloudflare_proxied_test.go index 9f96351db..c35a00c8d 100644 --- a/provider/cloudflare/cloudflare_proxied_test.go +++ b/provider/cloudflare/cloudflare_proxied_test.go @@ -22,33 +22,350 @@ import ( "github.com/cloudflare/cloudflare-go" "github.com/cloudflare/cloudflare-go/v4/dns" "github.com/stretchr/testify/assert" - - "sigs.k8s.io/external-dns/internal/testutils" ) func TestUpdateDNSRecordParamV4_ARecord(t *testing.T) { - change := cloudFlareChange{ + proxied := true + cfc := cloudFlareChange{ ResourceRecord: cloudflare.DNSRecord{ - ZoneID: "zone123", Name: "test.example.com", - Type: "A", Content: "192.168.1.1", + Type: "A", TTL: 300, - Proxied: testutils.ToPtr(true), - Comment: "Test record", + Comment: "test comment", + Proxied: &proxied, }, } + zoneID := "test-zone-id" - result := updateDNSRecordParamV4(change) + params := updateDNSRecordParamV4(cfc, zoneID) - assert.Equal(t, "zone123", *result.ZoneID.Value) - - // Check that the body is an A record - aRecord, ok := result.Body.(dns.ARecordParam) - assert.True(t, ok, "Body should be an ARecordParam") - assert.Equal(t, "test.example.com", *aRecord.Name.Value) - assert.Equal(t, "192.168.1.1", *aRecord.Content.Value) - assert.Equal(t, dns.TTL(300), aRecord.TTL) - assert.Equal(t, true, *aRecord.Proxied.Value) - assert.Equal(t, "Test record", *aRecord.Comment.Value) + assert.Equal(t, "test-zone-id", params.ZoneID.Value) + + // Extract the body as A record + if aRecord, ok := params.Body.(dns.ARecordParam); ok { + assert.Equal(t, "test.example.com", aRecord.Name.Value) + assert.Equal(t, "192.168.1.1", aRecord.Content.Value) + assert.InEpsilon(t, float64(dns.TTL(300)), float64(aRecord.TTL.Value), 0.01) + assert.Equal(t, "test comment", aRecord.Comment.Value) + assert.True(t, aRecord.Proxied.Value) + } else { + t.Fatalf("Expected A record type but got %T", params.Body) + } +} + +func TestUpdateDNSRecordParamV4_CNAMERecord(t *testing.T) { + proxied := false + cfc := cloudFlareChange{ + ResourceRecord: cloudflare.DNSRecord{ + Name: "cname.example.com", + Content: "target.example.com", + Type: "CNAME", + TTL: 600, + Proxied: &proxied, + }, + } + zoneID := "test-zone-id" + + params := updateDNSRecordParamV4(cfc, zoneID) + + assert.Equal(t, "test-zone-id", params.ZoneID.Value) + + // Extract the body as CNAME record + if cnameRecord, ok := params.Body.(dns.CNAMERecordParam); ok { + assert.Equal(t, "cname.example.com", cnameRecord.Name.Value) + assert.Equal(t, "target.example.com", cnameRecord.Content.Value) + assert.InEpsilon(t, float64(dns.TTL(600)), float64(cnameRecord.TTL.Value), 0.01) + assert.False(t, cnameRecord.Proxied.Value) + } else { + t.Fatalf("Expected CNAME record type but got %T", params.Body) + } +} + +func TestUpdateDNSRecordParamV4_MXRecord(t *testing.T) { + priority := uint16(10) + cfc := cloudFlareChange{ + ResourceRecord: cloudflare.DNSRecord{ + Name: "mx.example.com", + Content: "mail.example.com", + Type: "MX", + TTL: 3600, + Priority: &priority, + }, + } + zoneID := "test-zone-id" + + params := updateDNSRecordParamV4(cfc, zoneID) + + assert.Equal(t, "test-zone-id", params.ZoneID.Value) + + // Extract the body as MX record + if mxRecord, ok := params.Body.(dns.MXRecordParam); ok { + assert.Equal(t, "mx.example.com", mxRecord.Name.Value) + assert.Equal(t, "mail.example.com", mxRecord.Content.Value) + assert.InEpsilon(t, float64(dns.TTL(3600)), float64(mxRecord.TTL.Value), 0.01) + assert.InEpsilon(t, float64(10), mxRecord.Priority.Value, 0.01) + } else { + t.Fatalf("Expected MX record type but got %T", params.Body) + } +} + +func TestGetCreateDNSRecordParamV4_ARecord(t *testing.T) { + proxied := false + cfc := cloudFlareChange{ + ResourceRecord: cloudflare.DNSRecord{ + Name: "create.example.com", + Content: "10.0.0.1", + Type: "A", + TTL: 1200, + Proxied: &proxied, + }, + } + zoneID := "create-zone-id" + + params := getCreateDNSRecordParamV4(cfc, zoneID) + + assert.Equal(t, "create-zone-id", params.ZoneID.Value) + + // Extract the body as A record + if aRecord, ok := params.Body.(dns.ARecordParam); ok { + assert.Equal(t, "create.example.com", aRecord.Name.Value) + assert.Equal(t, "10.0.0.1", aRecord.Content.Value) + assert.InEpsilon(t, float64(dns.TTL(1200)), float64(aRecord.TTL.Value), 0.01) + assert.False(t, aRecord.Proxied.Value) + } else { + t.Fatalf("Expected A record type but got %T", params.Body) + } +} + +func TestGetCreateDNSRecordParamV4_TXTRecord(t *testing.T) { + cfc := cloudFlareChange{ + ResourceRecord: cloudflare.DNSRecord{ + Name: "txt.example.com", + Content: "v=spf1 include:_spf.google.com ~all", + Type: "TXT", + TTL: 1800, + Comment: "SPF record", + }, + } + zoneID := "txt-zone-id" + + params := getCreateDNSRecordParamV4(cfc, zoneID) + + assert.Equal(t, "txt-zone-id", params.ZoneID.Value) + + // Extract the body as TXT record + if txtRecord, ok := params.Body.(dns.TXTRecordParam); ok { + assert.Equal(t, "txt.example.com", txtRecord.Name.Value) + assert.Equal(t, "v=spf1 include:_spf.google.com ~all", txtRecord.Content.Value) + assert.InEpsilon(t, float64(dns.TTL(1800)), float64(txtRecord.TTL.Value), 0.01) + assert.Equal(t, "SPF record", txtRecord.Comment.Value) + } else { + t.Fatalf("Expected TXT record type but got %T", params.Body) + } +} + +func TestUpdateDNSRecordParamV4_AAAARecord(t *testing.T) { + proxied := true + cfc := cloudFlareChange{ + ResourceRecord: cloudflare.DNSRecord{ + Name: "ipv6.example.com", + Content: "2001:db8::1", + Type: "AAAA", + TTL: 7200, + Comment: "IPv6 record", + Proxied: &proxied, + }, + } + zoneID := "test-zone-id" + + params := updateDNSRecordParamV4(cfc, zoneID) + + assert.Equal(t, "test-zone-id", params.ZoneID.Value) + + // Extract the body as AAAA record + if aaaaRecord, ok := params.Body.(dns.AAAARecordParam); ok { + assert.Equal(t, "ipv6.example.com", aaaaRecord.Name.Value) + assert.Equal(t, "2001:db8::1", aaaaRecord.Content.Value) + assert.InEpsilon(t, float64(dns.TTL(7200)), float64(aaaaRecord.TTL.Value), 0.01) + assert.Equal(t, "IPv6 record", aaaaRecord.Comment.Value) + assert.True(t, aaaaRecord.Proxied.Value) + } else { + t.Fatalf("Expected AAAA record type but got %T", params.Body) + } +} + +func TestUpdateDNSRecordParamV4_TXTRecord(t *testing.T) { + cfc := cloudFlareChange{ + ResourceRecord: cloudflare.DNSRecord{ + Name: "txt.example.com", + Content: "v=spf1 include:_spf.google.com ~all", + Type: "TXT", + TTL: 1800, + Comment: "SPF record", + }, + } + zoneID := "txt-zone-id" + + params := updateDNSRecordParamV4(cfc, zoneID) + + assert.Equal(t, "txt-zone-id", params.ZoneID.Value) + + // Extract the body as TXT record + if txtRecord, ok := params.Body.(dns.TXTRecordParam); ok { + assert.Equal(t, "txt.example.com", txtRecord.Name.Value) + assert.Equal(t, "v=spf1 include:_spf.google.com ~all", txtRecord.Content.Value) + assert.InEpsilon(t, float64(dns.TTL(1800)), float64(txtRecord.TTL.Value), 0.01) + assert.Equal(t, "SPF record", txtRecord.Comment.Value) + } else { + t.Fatalf("Expected TXT record type but got %T", params.Body) + } +} + +func TestUpdateDNSRecordParamV4_NoProxied(t *testing.T) { + cfc := cloudFlareChange{ + ResourceRecord: cloudflare.DNSRecord{ + Name: "test.example.com", + Content: "192.168.1.1", + Type: "A", + TTL: 300, + Comment: "no proxied field", + // Proxied is nil + }, + } + zoneID := "test-zone-id" + + params := updateDNSRecordParamV4(cfc, zoneID) + + assert.Equal(t, "test-zone-id", params.ZoneID.Value) + + // Extract the body as A record + if aRecord, ok := params.Body.(dns.ARecordParam); ok { + assert.Equal(t, "test.example.com", aRecord.Name.Value) + assert.Equal(t, "192.168.1.1", aRecord.Content.Value) + assert.InEpsilon(t, float64(dns.TTL(300)), float64(aRecord.TTL.Value), 0.01) + assert.Equal(t, "no proxied field", aRecord.Comment.Value) + // Proxied should be false when not set + assert.False(t, aRecord.Proxied.Value) + } else { + t.Fatalf("Expected A record type but got %T", params.Body) + } +} + +func TestGetCreateDNSRecordParamV4_AAAARecord(t *testing.T) { + proxied := false + cfc := cloudFlareChange{ + ResourceRecord: cloudflare.DNSRecord{ + Name: "ipv6.example.com", + Content: "2001:db8::2", + Type: "AAAA", + TTL: 3600, + Comment: "IPv6 create", + Proxied: &proxied, + }, + } + zoneID := "create-zone-id" + + params := getCreateDNSRecordParamV4(cfc, zoneID) + + assert.Equal(t, "create-zone-id", params.ZoneID.Value) + + // Extract the body as AAAA record + if aaaaRecord, ok := params.Body.(dns.AAAARecordParam); ok { + assert.Equal(t, "ipv6.example.com", aaaaRecord.Name.Value) + assert.Equal(t, "2001:db8::2", aaaaRecord.Content.Value) + assert.InEpsilon(t, float64(dns.TTL(3600)), float64(aaaaRecord.TTL.Value), 0.01) + assert.Equal(t, "IPv6 create", aaaaRecord.Comment.Value) + assert.False(t, aaaaRecord.Proxied.Value) + } else { + t.Fatalf("Expected AAAA record type but got %T", params.Body) + } +} + +func TestGetCreateDNSRecordParamV4_CNAMERecord(t *testing.T) { + proxied := true + cfc := cloudFlareChange{ + ResourceRecord: cloudflare.DNSRecord{ + Name: "alias.example.com", + Content: "target.example.com", + Type: "CNAME", + TTL: 600, + Comment: "alias record", + Proxied: &proxied, + }, + } + zoneID := "cname-zone-id" + + params := getCreateDNSRecordParamV4(cfc, zoneID) + + assert.Equal(t, "cname-zone-id", params.ZoneID.Value) + + // Extract the body as CNAME record + if cnameRecord, ok := params.Body.(dns.CNAMERecordParam); ok { + assert.Equal(t, "alias.example.com", cnameRecord.Name.Value) + assert.Equal(t, "target.example.com", cnameRecord.Content.Value) + assert.InEpsilon(t, float64(dns.TTL(600)), float64(cnameRecord.TTL.Value), 0.01) + assert.Equal(t, "alias record", cnameRecord.Comment.Value) + assert.True(t, cnameRecord.Proxied.Value) + } else { + t.Fatalf("Expected CNAME record type but got %T", params.Body) + } +} + +func TestGetCreateDNSRecordParamV4_MXRecord(t *testing.T) { + priority := uint16(20) + cfc := cloudFlareChange{ + ResourceRecord: cloudflare.DNSRecord{ + Name: "mail.example.com", + Content: "mailserver.example.com", + Type: "MX", + TTL: 1800, + Comment: "mail exchange", + Priority: &priority, + }, + } + zoneID := "mx-zone-id" + + params := getCreateDNSRecordParamV4(cfc, zoneID) + + assert.Equal(t, "mx-zone-id", params.ZoneID.Value) + + // Extract the body as MX record + if mxRecord, ok := params.Body.(dns.MXRecordParam); ok { + assert.Equal(t, "mail.example.com", mxRecord.Name.Value) + assert.Equal(t, "mailserver.example.com", mxRecord.Content.Value) + assert.InEpsilon(t, float64(dns.TTL(1800)), float64(mxRecord.TTL.Value), 0.01) + assert.Equal(t, "mail exchange", mxRecord.Comment.Value) + assert.InEpsilon(t, float64(20), mxRecord.Priority.Value, 0.01) + } else { + t.Fatalf("Expected MX record type but got %T", params.Body) + } +} + +func TestGetCreateDNSRecordParamV4_NoProxiedNoComment(t *testing.T) { + cfc := cloudFlareChange{ + ResourceRecord: cloudflare.DNSRecord{ + Name: "simple.example.com", + Content: "10.0.0.2", + Type: "A", + TTL: 300, + // No Comment, no Proxied + }, + } + zoneID := "simple-zone-id" + + params := getCreateDNSRecordParamV4(cfc, zoneID) + + assert.Equal(t, "simple-zone-id", params.ZoneID.Value) + + // Extract the body as A record + if aRecord, ok := params.Body.(dns.ARecordParam); ok { + assert.Equal(t, "simple.example.com", aRecord.Name.Value) + assert.Equal(t, "10.0.0.2", aRecord.Content.Value) + assert.InEpsilon(t, float64(dns.TTL(300)), float64(aRecord.TTL.Value), 0.01) + assert.Empty(t, aRecord.Comment.Value) // Empty comment + assert.False(t, aRecord.Proxied.Value) // Should be false when not set + } else { + t.Fatalf("Expected A record type but got %T", params.Body) + } } diff --git a/provider/cloudflare/cloudflare_test.go b/provider/cloudflare/cloudflare_test.go index ccc3d005c..7c8f513a4 100644 --- a/provider/cloudflare/cloudflare_test.go +++ b/provider/cloudflare/cloudflare_test.go @@ -27,6 +27,7 @@ import ( "testing" "github.com/cloudflare/cloudflare-go" + "github.com/cloudflare/cloudflare-go/v4/dns" "github.com/cloudflare/cloudflare-go/v4/zones" "github.com/maxatome/go-testdeep/td" log "github.com/sirupsen/logrus" @@ -263,6 +264,185 @@ func (m *mockCloudFlareClient) DeleteDNSRecord(ctx context.Context, rc *cloudfla return nil } +// v4 DNS record operations for proxied records migration +func (m *mockCloudFlareClient) CreateDNSRecordV4(ctx context.Context, params dns.RecordNewParams) (*dns.RecordResponse, error) { + // Convert v4 params to v0 for mock storage + zoneID := params.ZoneID.Value + + // Extract record details from v4 union body + var name, content, recordType, comment string + var ttl dns.TTL + var proxied *bool + + switch body := params.Body.(type) { + case dns.ARecordParam: + name = body.Name.Value + content = body.Content.Value + ttl = body.TTL.Value + recordType = "A" + comment = body.Comment.Value + // Always set the proxied field if it's in the parameter + val := body.Proxied.Value + proxied = &val + case dns.AAAARecordParam: + name = body.Name.Value + content = body.Content.Value + ttl = body.TTL.Value + recordType = "AAAA" + comment = body.Comment.Value + val := body.Proxied.Value + proxied = &val + case dns.CNAMERecordParam: + name = body.Name.Value + content = body.Content.Value + ttl = body.TTL.Value + recordType = "CNAME" + comment = body.Comment.Value + val := body.Proxied.Value + proxied = &val + if body.Proxied.Value != false { + val := body.Proxied.Value + proxied = &val + } + case dns.MXRecordParam: + name = body.Name.Value + content = body.Content.Value + ttl = body.TTL.Value + recordType = "MX" + comment = body.Comment.Value + case dns.TXTRecordParam: + name = body.Name.Value + content = body.Content.Value + ttl = body.TTL.Value + recordType = "TXT" + comment = body.Comment.Value + } + + // Generate record ID using the same method as v0 mock + recordID := generateDNSRecordID(recordType, name, content) + + record := cloudflare.DNSRecord{ + ID: recordID, + Name: name, + Content: content, + Type: recordType, + TTL: int(ttl), + Proxied: proxied, + Comment: comment, + } + + if m.Records[zoneID] == nil { + m.Records[zoneID] = make(map[string]cloudflare.DNSRecord) + } + m.Records[zoneID][recordID] = record + + m.Actions = append(m.Actions, MockAction{ + Name: "CreateV4", + ZoneId: zoneID, + RecordId: recordID, + RecordData: record, + }) + + // Return a mock v4 response + return &dns.RecordResponse{ + ID: recordID, + Name: name, + Content: content, + Type: dns.RecordResponseType(recordType), + TTL: ttl, + }, nil +} + +func (m *mockCloudFlareClient) UpdateDNSRecordV4(ctx context.Context, recordID string, params dns.RecordUpdateParams) (*dns.RecordResponse, error) { + zoneID := params.ZoneID.Value + + if zone, ok := m.Records[zoneID]; ok { + if existing, ok := zone[recordID]; ok { + // Update the existing record with v4 params + updated := existing + + // Extract update details from v4 union body + switch body := params.Body.(type) { + case dns.ARecordParam: + updated.Name = body.Name.Value + updated.Content = body.Content.Value + updated.TTL = int(body.TTL.Value) + updated.Type = "A" + updated.Comment = body.Comment.Value + val := body.Proxied.Value + updated.Proxied = &val + case dns.AAAARecordParam: + updated.Name = body.Name.Value + updated.Content = body.Content.Value + updated.TTL = int(body.TTL.Value) + updated.Type = "AAAA" + updated.Comment = body.Comment.Value + val := body.Proxied.Value + updated.Proxied = &val + case dns.CNAMERecordParam: + updated.Name = body.Name.Value + updated.Content = body.Content.Value + updated.TTL = int(body.TTL.Value) + updated.Type = "CNAME" + updated.Comment = body.Comment.Value + val := body.Proxied.Value + updated.Proxied = &val + case dns.MXRecordParam: + updated.Name = body.Name.Value + updated.Content = body.Content.Value + updated.TTL = int(body.TTL.Value) + updated.Type = "MX" + updated.Comment = body.Comment.Value + case dns.TXTRecordParam: + updated.Name = body.Name.Value + updated.Content = body.Content.Value + updated.TTL = int(body.TTL.Value) + updated.Type = "TXT" + updated.Comment = body.Comment.Value + } + + zone[recordID] = updated + + m.Actions = append(m.Actions, MockAction{ + Name: "UpdateV4", + ZoneId: zoneID, + RecordId: recordID, + RecordData: updated, + }) + + return &dns.RecordResponse{ + ID: recordID, + Name: updated.Name, + Content: updated.Content, + Type: dns.RecordResponseType(updated.Type), + TTL: dns.TTL(updated.TTL), + }, nil + } + } + return nil, fmt.Errorf("record not found") +} + +func (m *mockCloudFlareClient) DeleteDNSRecordV4(ctx context.Context, recordID string, params dns.RecordDeleteParams) (*dns.RecordDeleteResponse, error) { + zoneID := params.ZoneID.Value + + if zone, ok := m.Records[zoneID]; ok { + if _, ok := zone[recordID]; ok { + delete(zone, recordID) + + m.Actions = append(m.Actions, MockAction{ + Name: "DeleteV4", + ZoneId: zoneID, + RecordId: recordID, + }) + + return &dns.RecordDeleteResponse{ + ID: recordID, + }, nil + } + } + return nil, fmt.Errorf("record not found") +} + func (m *mockCloudFlareClient) CustomHostnames(ctx context.Context, zoneID string, page int, filter cloudflare.CustomHostname) ([]cloudflare.CustomHostname, cloudflare.ResultInfo, error) { var err error = nil perPage := 50 // cloudflare-go v0 API hardcoded @@ -641,7 +821,7 @@ func TestCloudflareProxiedDefault(t *testing.T) { AssertActions(t, &CloudFlareProvider{proxiedByDefault: true}, endpoints, []MockAction{ { - Name: "Create", + Name: "CreateV4", ZoneId: "001", RecordId: generateDNSRecordID("A", "bar.com", "127.0.0.1"), RecordData: cloudflare.DNSRecord{ @@ -675,7 +855,7 @@ func TestCloudflareProxiedOverrideTrue(t *testing.T) { AssertActions(t, &CloudFlareProvider{}, endpoints, []MockAction{ { - Name: "Create", + Name: "CreateV4", ZoneId: "001", RecordId: generateDNSRecordID("A", "bar.com", "127.0.0.1"), RecordData: cloudflare.DNSRecord{ @@ -743,7 +923,7 @@ func TestCloudflareProxiedOverrideIllegal(t *testing.T) { AssertActions(t, &CloudFlareProvider{proxiedByDefault: true}, endpoints, []MockAction{ { - Name: "Create", + Name: "CreateV4", ZoneId: "001", RecordId: generateDNSRecordID("A", "bar.com", "127.0.0.1"), RecordData: cloudflare.DNSRecord{ @@ -818,9 +998,15 @@ func TestCloudflareSetProxied(t *testing.T) { if testCase.recordType == "MX" { recordData.Priority = priority } + + actionName := "Create" + if recordData.Proxied != nil && *recordData.Proxied { + actionName = "CreateV4" + } + AssertActions(t, &CloudFlareProvider{}, endpoints, []MockAction{ { - Name: "Create", + Name: actionName, ZoneId: "001", RecordId: expectedID, RecordData: recordData, @@ -847,7 +1033,6 @@ func TestCloudflareZones(t *testing.T) { // test failures on zone lookup func TestCloudflareZonesFailed(t *testing.T) { - client := NewMockCloudFlareClient() client.getZoneError = errors.New("zone lookup failed") @@ -1069,7 +1254,6 @@ func TestCloudflareProvider(t *testing.T) { t.Errorf("should fail, %s", err) } }) - } } @@ -1677,7 +1861,7 @@ func TestCloudflareComplexUpdate(t *testing.T) { RecordId: "2345678901", }, { - Name: "Create", + Name: "CreateV4", ZoneId: "001", RecordId: generateDNSRecordID("A", "foobar.bar.com", "2.3.4.5"), RecordData: cloudflare.DNSRecord{ @@ -1690,7 +1874,7 @@ func TestCloudflareComplexUpdate(t *testing.T) { }, }, { - Name: "Update", + Name: "UpdateV4", ZoneId: "001", RecordId: "1234567890", RecordData: cloudflare.DNSRecord{ @@ -3254,3 +3438,124 @@ func TestZoneIDByNameZoneNotFound(t *testing.T) { assert.Contains(t, err.Error(), `zone "nonexistent.com" not found in CloudFlare account`) assert.Contains(t, err.Error(), "verify the zone exists and API credentials have access to it") } + +func TestCloudflareApplyChangesV4ProxiedRecords(t *testing.T) { + changes := &plan.Changes{} + + // Add some existing records for updates + existingProxiedRecord := cloudflare.DNSRecord{ + ID: "existing-proxied-123", + Name: "update-proxied.bar.com", + Type: "A", + Content: "192.168.1.3", + Proxied: proxyEnabled, + } + + existingNonProxiedRecord := cloudflare.DNSRecord{ + ID: "existing-nonproxied-123", + Name: "update-nonproxied.bar.com", + Type: "A", + Content: "192.168.1.4", + Proxied: proxyDisabled, + } + + client := NewMockCloudFlareClientWithRecords(map[string][]cloudflare.DNSRecord{ + "001": {existingProxiedRecord, existingNonProxiedRecord}, + }) + + provider := &CloudFlareProvider{ + Client: client, + } + + // Create endpoints with proxied annotation to trigger v4 SDK usage + proxiedEndpoint := &endpoint.Endpoint{ + DNSName: "proxied.bar.com", + Targets: endpoint.Targets{"192.168.1.1"}, + RecordType: endpoint.RecordTypeA, + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: annotations.CloudflareProxiedKey, + Value: "true", + }, + }, + } + + nonProxiedEndpoint := &endpoint.Endpoint{ + DNSName: "nonproxied.bar.com", + Targets: endpoint.Targets{"192.168.1.2"}, + RecordType: endpoint.RecordTypeA, + } + + changes.Create = []*endpoint.Endpoint{proxiedEndpoint, nonProxiedEndpoint} + + updateProxiedEndpointOld := &endpoint.Endpoint{ + DNSName: "update-proxied.bar.com", + Targets: endpoint.Targets{"192.168.1.3"}, + RecordType: endpoint.RecordTypeA, + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: annotations.CloudflareProxiedKey, + Value: "true", + }, + }, + } + + updateProxiedEndpointNew := &endpoint.Endpoint{ + DNSName: "update-proxied.bar.com", + Targets: endpoint.Targets{"192.168.1.5"}, + RecordType: endpoint.RecordTypeA, + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: annotations.CloudflareProxiedKey, + Value: "true", + }, + }, + } + + updateNonProxiedEndpointOld := &endpoint.Endpoint{ + DNSName: "update-nonproxied.bar.com", + Targets: endpoint.Targets{"192.168.1.4"}, + RecordType: endpoint.RecordTypeA, + } + + updateNonProxiedEndpointNew := &endpoint.Endpoint{ + DNSName: "update-nonproxied.bar.com", + Targets: endpoint.Targets{"192.168.1.6"}, + RecordType: endpoint.RecordTypeA, + } + + changes.UpdateOld = []*endpoint.Endpoint{updateProxiedEndpointOld, updateNonProxiedEndpointOld} + changes.UpdateNew = []*endpoint.Endpoint{updateProxiedEndpointNew, updateNonProxiedEndpointNew} + + err := provider.ApplyChanges(context.Background(), changes) + assert.NoError(t, err) + + // Debug: Print all actions + t.Logf("Total actions recorded: %d", len(client.Actions)) + for i, action := range client.Actions { + t.Logf("Action %d: %s (ZoneId: %s, RecordId: %s)", i, action.Name, action.ZoneId, action.RecordId) + } + + // Verify that v4 methods were called for proxied records and v0 for non-proxied + v4CreateCalls := 0 + v0CreateCalls := 0 + deleteCalls := 0 + + for _, action := range client.Actions { + switch action.Name { + case "CreateV4": + v4CreateCalls++ + case "Create": + v0CreateCalls++ + case "Delete": + deleteCalls++ + } + } + + // Should have 2 v4 creates (1 for proxied record creation + 1 for proxied record update) + assert.Equal(t, 2, v4CreateCalls, "Expected 2 v4 create calls for proxied records") + // Should have 2 v0 creates (1 for non-proxied record creation + 1 for non-proxied record update) + assert.Equal(t, 2, v0CreateCalls, "Expected 2 v0 create calls for non-proxied records") + // Should have 2 deletes (1 for each existing record being updated) + assert.Equal(t, 2, deleteCalls, "Expected 2 delete calls for existing records") +} diff --git a/provider/oci/cache_test.go b/provider/oci/cache_test.go index a485e200a..77ed2d2ce 100644 --- a/provider/oci/cache_test.go +++ b/provider/oci/cache_test.go @@ -17,10 +17,11 @@ limitations under the License. package oci import ( - "github.com/oracle/oci-go-sdk/v65/dns" - "github.com/stretchr/testify/assert" "testing" "time" + + "github.com/oracle/oci-go-sdk/v65/dns" + "github.com/stretchr/testify/assert" ) func TestZoneCache(t *testing.T) {