From 941a5852c48859460f8686b3f2265e584e854d79 Mon Sep 17 00:00:00 2001 From: amold1 Date: Thu, 22 Aug 2024 12:39:33 -0400 Subject: [PATCH] [fix] fix dns record ownership (#475) * fix dns record ownership * fix mock tests --- cloud/services/domains.go | 45 ++++++++++++++++----- cloud/services/domains_test.go | 26 +++++++++++- controller/linodecluster_controller_test.go | 29 +++++++++++++ 3 files changed, 90 insertions(+), 10 deletions(-) diff --git a/cloud/services/domains.go b/cloud/services/domains.go index 134375d40..aceef0706 100644 --- a/cloud/services/domains.go +++ b/cloud/services/domains.go @@ -103,7 +103,7 @@ func EnsureAkamaiDNSEntries(ctx context.Context, cscope *scope.ClusterScope, ope // if operation is delete and we got the record, delete it if operation == "delete" { - return deleteAkamaiEntry(ctx, akaDNSClient, recordBody, dnsEntry, rootDomain) + return deleteAkamaiEntry(ctx, cscope, recordBody, dnsEntry) } // if operation is create and we got the record, update it // Check if the target already exists in the target list @@ -136,7 +136,20 @@ func createAkamaiEntry(ctx context.Context, client clients.AkamClient, dnsEntry ) } -func deleteAkamaiEntry(ctx context.Context, client clients.AkamClient, recordBody *dns.RecordBody, dnsEntry DNSOptions, rootDomain string) error { +func deleteAkamaiEntry(ctx context.Context, cscope *scope.ClusterScope, recordBody *dns.RecordBody, dnsEntry DNSOptions) error { + linodeCluster := cscope.LinodeCluster + linodeClusterNetworkSpec := linodeCluster.Spec.Network + rootDomain := linodeClusterNetworkSpec.DNSRootDomain + // If record is A/AAAA type, verify ownership + if dnsEntry.DNSRecordType != linodego.RecordTypeTXT { + isOwner, err := IsAkamaiDomainRecordOwner(ctx, cscope) + if err != nil { + return err + } + if !isOwner { + return fmt.Errorf("the domain record is not owned by this entity. wont delete") + } + } switch { case len(recordBody.Target) > 1: recordBody.Target = removeElement( @@ -145,9 +158,9 @@ func deleteAkamaiEntry(ctx context.Context, client clients.AkamClient, recordBod // So we need to match that strings.Replace(dnsEntry.Target, "::", ":0:0:", 8), //nolint:mnd // 8 for 8 octest ) - return client.UpdateRecord(ctx, recordBody, rootDomain) + return cscope.AkamaiDomainsClient.UpdateRecord(ctx, recordBody, rootDomain) default: - return client.DeleteRecord(ctx, recordBody, rootDomain) + return cscope.AkamaiDomainsClient.DeleteRecord(ctx, recordBody, rootDomain) } } @@ -190,8 +203,8 @@ func (d *DNSEntries) getDNSEntriesToEnsure(cscope *scope.ClusterScope) ([]DNSOpt if len(d.options) == 0 { continue } - d.options = append(d.options, DNSOptions{subDomain, eachMachine.Name, linodego.RecordTypeTXT, dnsTTLSec}) } + d.options = append(d.options, DNSOptions{subDomain, cscope.LinodeCluster.Name, linodego.RecordTypeTXT, dnsTTLSec}) return d.options, nil } @@ -263,7 +276,7 @@ func DeleteDomainRecord(ctx context.Context, cscope *scope.ClusterScope, domainI // If record is A/AAAA type, verify ownership if dnsEntry.DNSRecordType != linodego.RecordTypeTXT { - isOwner, err := IsDomainRecordOwner(ctx, cscope, dnsEntry.Hostname, domainID) + isOwner, err := IsLinodeDomainRecordOwner(ctx, cscope, dnsEntry.Hostname, domainID) if err != nil { return err } @@ -276,9 +289,9 @@ func DeleteDomainRecord(ctx context.Context, cscope *scope.ClusterScope, domainI return cscope.LinodeDomainsClient.DeleteDomainRecord(ctx, domainID, domainRecords[0].ID) } -func IsDomainRecordOwner(ctx context.Context, cscope *scope.ClusterScope, hostname string, domainID int) (bool, error) { +func IsLinodeDomainRecordOwner(ctx context.Context, cscope *scope.ClusterScope, hostname string, domainID int) (bool, error) { // Check if domain record exists - filter, err := json.Marshal(map[string]interface{}{"name": hostname, "target": cscope.LinodeCluster.Name, "type": linodego.RecordTypeTXT}) + filter, err := json.Marshal(map[string]interface{}{"name": hostname, "type": linodego.RecordTypeTXT}) if err != nil { return false, err } @@ -290,7 +303,21 @@ func IsDomainRecordOwner(ctx context.Context, cscope *scope.ClusterScope, hostna // If record exists, update it if len(domainRecords) == 0 { - return false, fmt.Errorf("no txt record %s found with value %s for machine %s", hostname, cscope.LinodeCluster.Name, cscope.LinodeCluster.Name) + return false, fmt.Errorf("no txt record %s found", hostname) + } + + return true, nil +} + +func IsAkamaiDomainRecordOwner(ctx context.Context, cscope *scope.ClusterScope) (bool, error) { + linodeCluster := cscope.LinodeCluster + linodeClusterNetworkSpec := linodeCluster.Spec.Network + rootDomain := linodeClusterNetworkSpec.DNSRootDomain + akaDNSClient := cscope.AkamaiDomainsClient + fqdn := getSubDomain(cscope) + "." + rootDomain + recordBody, err := akaDNSClient.GetRecord(ctx, rootDomain, fqdn, string(linodego.RecordTypeTXT)) + if err != nil || recordBody == nil { + return false, fmt.Errorf("no txt record %s found", fqdn) } return true, nil diff --git a/cloud/services/domains_test.go b/cloud/services/domains_test.go index b04d19449..291626463 100644 --- a/cloud/services/domains_test.go +++ b/cloud/services/domains_test.go @@ -731,6 +731,14 @@ func TestAddIPToDNS(t *testing.T) { Domain: "lkedevs.net", }, }, nil).AnyTimes() + mockClient.EXPECT().ListDomainRecords(gomock.Any(), gomock.Any(), gomock.Any()).Return([]linodego.DomainRecord{ + { + ID: 1234, + Type: "A", + Name: "test-cluster", + TTLSec: 30, + }, + }, nil).AnyTimes() }, expectedError: nil, expectK8sClient: func(mockK8sClient *mock.MockK8sClient) { @@ -1003,7 +1011,23 @@ func TestDeleteIPFromDNS(t *testing.T) { }, }, }, - expects: func(mockClient *mock.MockLinodeClient) {}, + expects: func(mockClient *mock.MockLinodeClient) { + mockClient.EXPECT().ListDomains(gomock.Any(), gomock.Any()).Return([]linodego.Domain{ + { + ID: 1, + Domain: "lkedevs.net", + }, + }, nil).AnyTimes() + mockClient.EXPECT().ListDomainRecords(gomock.Any(), gomock.Any(), gomock.Any()).Return([]linodego.DomainRecord{ + { + ID: 1234, + Type: "A", + Name: "test-cluster", + TTLSec: 30, + }, + }, nil).AnyTimes() + mockClient.EXPECT().DeleteDomainRecord(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes() + }, expectedError: nil, expectK8sClient: func(mockK8sClient *mock.MockK8sClient) { mockK8sClient.EXPECT().Scheme().Return(nil).AnyTimes() diff --git a/controller/linodecluster_controller_test.go b/controller/linodecluster_controller_test.go index 1b6c63fb8..d0fc95042 100644 --- a/controller/linodecluster_controller_test.go +++ b/controller/linodecluster_controller_test.go @@ -338,6 +338,21 @@ var _ = Describe("cluster-lifecycle-dns", Ordered, Label("cluster", "cluster-lif Path( Call("cluster with dns loadbalancing is created", func(ctx context.Context, mck Mock) { cScope.LinodeClient = mck.LinodeClient + cScope.LinodeDomainsClient = mck.LinodeClient + mck.LinodeClient.EXPECT().ListDomains(gomock.Any(), gomock.Any()).Return([]linodego.Domain{ + { + ID: 1, + Domain: "lkedevs.net", + }, + }, nil).AnyTimes() + mck.LinodeClient.EXPECT().ListDomainRecords(gomock.Any(), gomock.Any(), gomock.Any()).Return([]linodego.DomainRecord{ + { + ID: 1234, + Type: "A", + Name: "test-cluster", + TTLSec: 30, + }, + }, nil).AnyTimes() }), Result("cluster created", func(ctx context.Context, mck Mock) { reconciler.Client = k8sClient @@ -529,6 +544,20 @@ var _ = Describe("dns-override-endpoint", Ordered, Label("cluster", "dns-overrid } Expect(k8sClient.Create(ctx, &linodeMachine)).To(Succeed()) cScope.LinodeMachines = linodeMachines + mck.LinodeClient.EXPECT().ListDomains(gomock.Any(), gomock.Any()).Return([]linodego.Domain{ + { + ID: 1, + Domain: "lkedevs.net", + }, + }, nil).AnyTimes() + mck.LinodeClient.EXPECT().ListDomainRecords(gomock.Any(), gomock.Any(), gomock.Any()).Return([]linodego.DomainRecord{ + { + ID: 1234, + Type: "A", + Name: "test-cluster", + TTLSec: 30, + }, + }, nil).AnyTimes() }), Result("cluster created", func(ctx context.Context, mck Mock) { reconciler.Client = k8sClient