From 919e76da1d2262ac6d8398d6296ec8e4a604835b Mon Sep 17 00:00:00 2001 From: Zhou Zhiqiang Date: Fri, 30 Jun 2023 20:17:10 +0800 Subject: [PATCH] fix: only delete dns record related to managed tunnel (#21) --- pkg/cloudflare-controller/bootstrap.go | 3 +- pkg/cloudflare-controller/dns.go | 18 ++++++---- pkg/cloudflare-controller/dns_test.go | 38 +++++++++++++--------- pkg/cloudflare-controller/tunnel-client.go | 20 +++++++----- 4 files changed, 48 insertions(+), 31 deletions(-) diff --git a/pkg/cloudflare-controller/bootstrap.go b/pkg/cloudflare-controller/bootstrap.go index 257b7f0..afc13c2 100644 --- a/pkg/cloudflare-controller/bootstrap.go +++ b/pkg/cloudflare-controller/bootstrap.go @@ -4,6 +4,7 @@ import ( "context" "crypto/rand" "fmt" + "github.com/cloudflare/cloudflare-go" "github.com/go-logr/logr" "github.com/pkg/errors" @@ -16,7 +17,7 @@ func BootstrapTunnelClientWithTunnelName(ctx context.Context, logger logr.Logger return nil, errors.Wrapf(err, "get tunnel id from tunnel name %s", tunnelName) } logger.V(3).Info("tunnel id fetched", "tunnel-id", tunnelId, "tunnel-name", tunnelName, "account-id", accountId) - return NewTunnelClient(logger, cfClient, accountId, tunnelId), nil + return NewTunnelClient(logger, cfClient, accountId, tunnelId, tunnelName), nil } func GetTunnelIdFromTunnelName(ctx context.Context, logger logr.Logger, cfClient *cloudflare.API, tunnelName string, accountId string) (string, error) { diff --git a/pkg/cloudflare-controller/dns.go b/pkg/cloudflare-controller/dns.go index 7de2fbd..5c16254 100644 --- a/pkg/cloudflare-controller/dns.go +++ b/pkg/cloudflare-controller/dns.go @@ -2,12 +2,13 @@ package cloudflarecontroller import ( "fmt" + "strings" + "github.com/STRRL/cloudflare-tunnel-ingress-controller/pkg/exposure" "github.com/cloudflare/cloudflare-go" - "strings" ) -const ManagedCNAMERecordCommentMark = "managed by cloudflare-tunnel-ingress-controller" +const ManagedCNAMERecordCommentMarkFormat = "managed by strrl.dev/cloudflare-tunnel-ingress-controller, tunnel [%s]" type DNSOperationCreate struct { Hostname string @@ -27,7 +28,7 @@ type DNSOperationDelete struct { OldRecord cloudflare.DNSRecord } -func syncDNSRecord(exposures []exposure.Exposure, existedCNAMERecords []cloudflare.DNSRecord, tunnelId string) ([]DNSOperationCreate, []DNSOperationUpdate, []DNSOperationDelete, error) { +func syncDNSRecord(exposures []exposure.Exposure, existedCNAMERecords []cloudflare.DNSRecord, tunnelId string, tunnelName string) ([]DNSOperationCreate, []DNSOperationUpdate, []DNSOperationDelete, error) { var effectiveExposures []exposure.Exposure for _, item := range exposures { if !item.IsDeleted { @@ -46,14 +47,14 @@ func syncDNSRecord(exposures []exposure.Exposure, existedCNAMERecords []cloudfla OldRecord: old, Type: "CNAME", Content: tunnelDomain(tunnelId), - Comment: ManagedCNAMERecordCommentMark, + Comment: renderDNSRecordComment(tunnelName), }) } else { toCreate = append(toCreate, DNSOperationCreate{ Hostname: item.Hostname, Type: "CNAME", Content: tunnelDomain(tunnelId), - Comment: ManagedCNAMERecordCommentMark, + Comment: renderDNSRecordComment(tunnelName), }) } } @@ -62,7 +63,7 @@ func syncDNSRecord(exposures []exposure.Exposure, existedCNAMERecords []cloudfla for _, item := range existedCNAMERecords { contains, _ := exposureContainsHostname(effectiveExposures, item.Name) if !contains { - if item.Comment == ManagedCNAMERecordCommentMark { + if item.Comment == renderDNSRecordComment(tunnelName) { toDelete = append(toDelete, DNSOperationDelete{ OldRecord: item, }) @@ -96,3 +97,8 @@ const WellKnownTunnelDomainFormat = "%s.cfargotunnel.com" func tunnelDomain(tunnelId string) string { return strings.ToLower(fmt.Sprintf(WellKnownTunnelDomainFormat, tunnelId)) } + +func renderDNSRecordComment(tunnelName string) string { + // TODO: comment has a limitation with max 100 char, maybe use TXT record in the future? + return fmt.Sprintf(ManagedCNAMERecordCommentMarkFormat, tunnelName) +} diff --git a/pkg/cloudflare-controller/dns_test.go b/pkg/cloudflare-controller/dns_test.go index 9ab4049..47fc3a0 100644 --- a/pkg/cloudflare-controller/dns_test.go +++ b/pkg/cloudflare-controller/dns_test.go @@ -1,10 +1,11 @@ package cloudflarecontroller import ( - "github.com/STRRL/cloudflare-tunnel-ingress-controller/pkg/exposure" - "github.com/cloudflare/cloudflare-go" "reflect" "testing" + + "github.com/STRRL/cloudflare-tunnel-ingress-controller/pkg/exposure" + "github.com/cloudflare/cloudflare-go" ) const WhateverTunnelId = "whatever" @@ -15,6 +16,7 @@ func Test_syncDNSRecord(t *testing.T) { exposures []exposure.Exposure existedRecords []cloudflare.DNSRecord tunnelId string + tunnelName string } var tests = []struct { name string @@ -49,13 +51,14 @@ func Test_syncDNSRecord(t *testing.T) { }, existedRecords: nil, tunnelId: WhateverTunnelId, + tunnelName: "tunnel-in-test", }, wantCreate: []DNSOperationCreate{ { Hostname: "test.example.com", Type: "CNAME", Content: WhateverTunnelDomain, - Comment: ManagedCNAMERecordCommentMark, + Comment: "managed by strrl.dev/cloudflare-tunnel-ingress-controller, tunnel [tunnel-in-test]", }, }, wantUpdate: nil, @@ -81,13 +84,14 @@ func Test_syncDNSRecord(t *testing.T) { }, existedRecords: nil, tunnelId: WhateverTunnelId, + tunnelName: "tunnel-in-test", }, wantCreate: []DNSOperationCreate{ { Hostname: "test2.example.com", Type: "CNAME", Content: WhateverTunnelDomain, - Comment: ManagedCNAMERecordCommentMark, + Comment: "managed by strrl.dev/cloudflare-tunnel-ingress-controller, tunnel [tunnel-in-test]", }, }, wantUpdate: nil, @@ -112,7 +116,8 @@ func Test_syncDNSRecord(t *testing.T) { Comment: "", }, }, - tunnelId: "", + tunnelId: "", + tunnelName: "", }, wantCreate: nil, wantUpdate: nil, @@ -138,7 +143,8 @@ func Test_syncDNSRecord(t *testing.T) { Comment: "", }, }, - tunnelId: WhateverTunnelId, + tunnelId: WhateverTunnelId, + tunnelName: "tunnel-in-test", }, wantCreate: nil, wantUpdate: []DNSOperationUpdate{ @@ -151,7 +157,7 @@ func Test_syncDNSRecord(t *testing.T) { }, Type: "CNAME", Content: WhateverTunnelDomain, - Comment: ManagedCNAMERecordCommentMark, + Comment: "managed by strrl.dev/cloudflare-tunnel-ingress-controller, tunnel [tunnel-in-test]", }, }, wantDelete: nil, @@ -173,10 +179,11 @@ func Test_syncDNSRecord(t *testing.T) { Name: "test.example.com", Type: "A", Content: "1.2.3.4", - Comment: ManagedCNAMERecordCommentMark, + Comment: "managed by strrl.dev/cloudflare-tunnel-ingress-controller, tunnel [tunnel-in-test]", }, }, - tunnelId: WhateverTunnelId, + tunnelId: WhateverTunnelId, + tunnelName: "tunnel-in-test", }, wantCreate: nil, wantUpdate: nil, @@ -186,7 +193,7 @@ func Test_syncDNSRecord(t *testing.T) { Name: "test.example.com", Type: "A", Content: "1.2.3.4", - Comment: ManagedCNAMERecordCommentMark, + Comment: "managed by strrl.dev/cloudflare-tunnel-ingress-controller, tunnel [tunnel-in-test]", }, }, }, @@ -208,10 +215,11 @@ func Test_syncDNSRecord(t *testing.T) { Name: "test.example.com", Type: "CNAME", Content: WhateverTunnelDomain, - Comment: ManagedCNAMERecordCommentMark, + Comment: "managed by strrl.dev/cloudflare-tunnel-ingress-controller, tunnel [tunnel-in-test]", }, }, - tunnelId: WhateverTunnelId, + tunnelId: WhateverTunnelId, + tunnelName: "tunnel-in-test", }, wantCreate: nil, wantUpdate: []DNSOperationUpdate{ @@ -220,11 +228,11 @@ func Test_syncDNSRecord(t *testing.T) { Name: "test.example.com", Type: "CNAME", Content: WhateverTunnelDomain, - Comment: ManagedCNAMERecordCommentMark, + Comment: "managed by strrl.dev/cloudflare-tunnel-ingress-controller, tunnel [tunnel-in-test]", }, Type: "CNAME", Content: WhateverTunnelDomain, - Comment: ManagedCNAMERecordCommentMark, + Comment: "managed by strrl.dev/cloudflare-tunnel-ingress-controller, tunnel [tunnel-in-test]", }, }, wantDelete: nil, @@ -233,7 +241,7 @@ func Test_syncDNSRecord(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - gotCreate, gotUpdate, gotDelete, err := syncDNSRecord(tt.args.exposures, tt.args.existedRecords, tt.args.tunnelId) + gotCreate, gotUpdate, gotDelete, err := syncDNSRecord(tt.args.exposures, tt.args.existedRecords, tt.args.tunnelId, tt.args.tunnelName) if (err != nil) != tt.wantErr { t.Errorf("syncDNSRecord() error = %v, wantErr %v", err, tt.wantErr) return diff --git a/pkg/cloudflare-controller/tunnel-client.go b/pkg/cloudflare-controller/tunnel-client.go index da8af66..72abff3 100644 --- a/pkg/cloudflare-controller/tunnel-client.go +++ b/pkg/cloudflare-controller/tunnel-client.go @@ -2,22 +2,24 @@ package cloudflarecontroller import ( "context" + "strings" + "github.com/STRRL/cloudflare-tunnel-ingress-controller/pkg/exposure" "github.com/cloudflare/cloudflare-go" "github.com/go-logr/logr" "github.com/pkg/errors" - "strings" ) type TunnelClient struct { - logger logr.Logger - cfClient *cloudflare.API - accountId string - tunnelId string + logger logr.Logger + cfClient *cloudflare.API + accountId string + tunnelId string + tunnelName string } -func NewTunnelClient(logger logr.Logger, cfClient *cloudflare.API, accountId string, tunnelId string) *TunnelClient { - return &TunnelClient{logger: logger, cfClient: cfClient, accountId: accountId, tunnelId: tunnelId} +func NewTunnelClient(logger logr.Logger, cfClient *cloudflare.API, accountId string, tunnelId string, tunnelName string) *TunnelClient { + return &TunnelClient{logger: logger, cfClient: cfClient, accountId: accountId, tunnelId: tunnelId, tunnelName: tunnelName} } func (t *TunnelClient) PutExposures(ctx context.Context, exposures []exposure.Exposure) error { @@ -117,7 +119,7 @@ func (t *TunnelClient) updateDNSCNAMERecordForZone(ctx context.Context, exposure if err != nil { return errors.Wrapf(err, "list DNS records for zone %s", zone.Name) } - toCreate, toUpdate, toDelete, err := syncDNSRecord(exposures, cnameDnsRecords, t.tunnelId) + toCreate, toUpdate, toDelete, err := syncDNSRecord(exposures, cnameDnsRecords, t.tunnelId, t.tunnelName) if err != nil { return errors.Wrap(err, "sync DNS records") } @@ -140,7 +142,7 @@ func (t *TunnelClient) updateDNSCNAMERecordForZone(ctx context.Context, exposure for _, item := range toUpdate { - if item.OldRecord.Comment != ManagedCNAMERecordCommentMark { + if item.OldRecord.Comment != renderDNSRecordComment(t.tunnelName) { t.logger.Info("WARNING, the origin DNS record is not managed by this controller, it would be changed to managed record", "origin-record", item.OldRecord, )