Skip to content

Commit

Permalink
Merge pull request #209 from Kuadrant/gh-202
Browse files Browse the repository at this point in the history
add domain owners to dnsrecord
  • Loading branch information
maleck13 authored Aug 13, 2024
2 parents ee92883 + 9492fdb commit 99185ba
Show file tree
Hide file tree
Showing 10 changed files with 78 additions and 22 deletions.
3 changes: 3 additions & 0 deletions api/v1alpha1/dnsrecord_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,9 @@ type DNSRecordStatus struct {

// ownerID is a unique string used to identify the owner of this record.
OwnerID string `json:"ownerID,omitempty"`

// DomainOwners is a list of all the owners working against the root domain of this record
DomainOwners []string `json:"domainOwners,omitempty"`
}

//+kubebuilder:object:root=true
Expand Down
5 changes: 5 additions & 0 deletions api/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion bundle/manifests/dns-operator.clusterserviceversion.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ metadata:
capabilities: Basic Install
categories: Integration & Delivery
containerImage: quay.io/kuadrant/dns-operator:latest
createdAt: "2024-06-28T14:24:36Z"
createdAt: "2024-08-12T10:16:02Z"
description: A Kubernetes Operator to manage the lifecycle of DNS resources
operators.operatorframework.io/builder: operator-sdk-v1.33.0
operators.operatorframework.io/project_layout: go.kubebuilder.io/v4
Expand Down
6 changes: 6 additions & 0 deletions bundle/manifests/kuadrant.io_dnsrecords.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,12 @@ spec:
- type
type: object
type: array
domainOwners:
description: DomainOwners is a list of all the owners working against
the root domain of this record
items:
type: string
type: array
endpoints:
description: |-
endpoints are the last endpoints that were successfully published by the provider
Expand Down
6 changes: 6 additions & 0 deletions config/crd/bases/kuadrant.io_dnsrecords.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,12 @@ spec:
- type
type: object
type: array
domainOwners:
description: DomainOwners is a list of all the owners working against
the root domain of this record
items:
type: string
type: array
endpoints:
description: |-
endpoints are the last endpoints that were successfully published by the provider
Expand Down
4 changes: 2 additions & 2 deletions internal/controller/dnsrecord_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,7 @@ func (r *DNSRecordReconciler) getDNSProvider(ctx context.Context, managedZone *v
func (r *DNSRecordReconciler) applyChanges(ctx context.Context, dnsRecord *v1alpha1.DNSRecord, managedZone *v1alpha1.ManagedZone, isDelete bool) (bool, error) {
logger := log.FromContext(ctx)
zoneDomainName, _ := strings.CutPrefix(managedZone.Spec.DomainName, v1alpha1.WildcardPrefix)
rootDomainName, _ := strings.CutPrefix(dnsRecord.Spec.RootHost, v1alpha1.WildcardPrefix)
rootDomainName := dnsRecord.Spec.RootHost
zoneDomainFilter := externaldnsendpoint.NewDomainFilter([]string{zoneDomainName})
managedDNSRecordTypes := []string{externaldnsendpoint.RecordTypeA, externaldnsendpoint.RecordTypeAAAA, externaldnsendpoint.RecordTypeCNAME}
var excludeDNSRecordTypes []string
Expand Down Expand Up @@ -465,10 +465,10 @@ func (r *DNSRecordReconciler) applyChanges(ctx context.Context, dnsRecord *v1alp
)

plan = plan.Calculate()

if err = plan.Error(); err != nil {
return false, err
}
dnsRecord.Status.DomainOwners = plan.Owners
if plan.Changes.HasChanges() {
logger.Info("Applying changes")
err = registry.ApplyChanges(ctx, plan.Changes)
Expand Down
28 changes: 20 additions & 8 deletions internal/controller/dnsrecord_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,14 @@ import (
)

var _ = Describe("DNSRecordReconciler", func() {
var dnsRecord *v1alpha1.DNSRecord
var dnsRecord2 *v1alpha1.DNSRecord
var dnsProviderSecret *v1.Secret
var managedZone *v1alpha1.ManagedZone
var brokenZone *v1alpha1.ManagedZone
var testNamespace string
var (
dnsRecord *v1alpha1.DNSRecord
dnsRecord2 *v1alpha1.DNSRecord
dnsProviderSecret *v1.Secret
managedZone *v1alpha1.ManagedZone
brokenZone *v1alpha1.ManagedZone
testNamespace string
)

BeforeEach(func() {
CreateNamespace(&testNamespace)
Expand Down Expand Up @@ -169,6 +171,7 @@ var _ = Describe("DNSRecordReconciler", func() {
"ObservedGeneration": Equal(dnsRecord.Generation),
})),
)
g.Expect(dnsRecord.Status.DomainOwners).To(ConsistOf(dnsRecord.GetUIDHash()))
}, TestTimeoutMedium, time.Second).Should(Succeed())

dnsRecord2 = &v1alpha1.DNSRecord{
Expand Down Expand Up @@ -258,6 +261,7 @@ var _ = Describe("DNSRecordReconciler", func() {
})),
)
g.Expect(dnsRecord.Finalizers).To(ContainElement(DNSRecordFinalizer))
g.Expect(dnsRecord.Status.DomainOwners).To(ConsistOf(dnsRecord.GetUIDHash()))
}, TestTimeoutMedium, time.Second).Should(Succeed())
})

Expand Down Expand Up @@ -307,8 +311,6 @@ var _ = Describe("DNSRecordReconciler", func() {
Eventually(func(g Gomega) {
err := k8sClient.Get(ctx, client.ObjectKeyFromObject(dnsRecord), dnsRecord)
g.Expect(err).NotTo(HaveOccurred())
g.Expect(dnsRecord.Spec.OwnerID).To(BeEmpty())
g.Expect(dnsRecord.Status.OwnerID).To(Equal(dnsRecord.GetUIDHash()))
g.Expect(dnsRecord.Status.Conditions).To(
ContainElement(MatchFields(IgnoreExtras, Fields{
"Type": Equal(string(v1alpha1.ConditionTypeReady)),
Expand All @@ -320,6 +322,7 @@ var _ = Describe("DNSRecordReconciler", func() {
)
g.Expect(dnsRecord.Finalizers).To(ContainElement(DNSRecordFinalizer))
g.Expect(dnsRecord.Status.WriteCounter).To(BeZero())
g.Expect(dnsRecord.Status.DomainOwners).To(ConsistOf(dnsRecord.GetUIDHash()))
}, TestTimeoutMedium, time.Second).Should(Succeed())
})

Expand All @@ -341,6 +344,7 @@ var _ = Describe("DNSRecordReconciler", func() {
)
g.Expect(dnsRecord.Finalizers).To(ContainElement(DNSRecordFinalizer))
g.Expect(dnsRecord.Status.WriteCounter).To(BeZero())
g.Expect(dnsRecord.Status.DomainOwners).To(ConsistOf(dnsRecord.GetUIDHash()))
}, TestTimeoutMedium, time.Second).Should(Succeed())

//Does not allow ownerID to change once set
Expand All @@ -356,6 +360,7 @@ var _ = Describe("DNSRecordReconciler", func() {
err = k8sClient.Get(ctx, client.ObjectKeyFromObject(dnsRecord), dnsRecord)
g.Expect(err).NotTo(HaveOccurred())
g.Expect(dnsRecord.Status.OwnerID).To(Equal(dnsRecord.GetUIDHash()))
g.Expect(dnsRecord.Status.DomainOwners).To(ConsistOf(dnsRecord.GetUIDHash()))

}, TestTimeoutMedium, time.Second).Should(Succeed())
})
Expand Down Expand Up @@ -391,6 +396,7 @@ var _ = Describe("DNSRecordReconciler", func() {
})),
)
g.Expect(dnsRecord.Finalizers).To(ContainElement(DNSRecordFinalizer))
g.Expect(dnsRecord.Status.DomainOwners).To(ConsistOf("owner1"))
}, TestTimeoutMedium, time.Second).Should(Succeed())

//Does not allow ownerID to change once set
Expand All @@ -410,6 +416,7 @@ var _ = Describe("DNSRecordReconciler", func() {
err = k8sClient.Get(ctx, client.ObjectKeyFromObject(dnsRecord), dnsRecord)
g.Expect(err).NotTo(HaveOccurred())
g.Expect(dnsRecord.Status.OwnerID).To(Equal("owner1"))
g.Expect(dnsRecord.Status.DomainOwners).To(ConsistOf("owner1"))
}, TestTimeoutMedium, time.Second).Should(Succeed())
})

Expand Down Expand Up @@ -457,6 +464,7 @@ var _ = Describe("DNSRecordReconciler", func() {
})),
)
g.Expect(dnsRecord.Finalizers).To(ContainElement(DNSRecordFinalizer))
g.Expect(dnsRecord.Status.DomainOwners).To(ConsistOf(dnsRecord.GetUIDHash()))
}, TestTimeoutMedium, time.Second).Should(Succeed())

By("creating dnsrecord " + dnsRecord2.Name + " with endpoint dnsName: `foo.example.com` and target: `127.0.0.2`")
Expand All @@ -476,6 +484,7 @@ var _ = Describe("DNSRecordReconciler", func() {
})),
)
g.Expect(dnsRecord.Status.WriteCounter).To(BeNumerically(">", int64(1)))
g.Expect(dnsRecord.Status.DomainOwners).To(ConsistOf(dnsRecord.GetUIDHash(), dnsRecord2.GetUIDHash()))

err = k8sClient.Get(ctx, client.ObjectKeyFromObject(dnsRecord2), dnsRecord2)
g.Expect(err).NotTo(HaveOccurred())
Expand All @@ -489,6 +498,7 @@ var _ = Describe("DNSRecordReconciler", func() {
})),
)
g.Expect(dnsRecord2.Status.WriteCounter).To(BeNumerically(">", int64(1)))
g.Expect(dnsRecord2.Status.DomainOwners).To(ConsistOf(dnsRecord.GetUIDHash(), dnsRecord2.GetUIDHash()))
}, TestTimeoutLong, time.Second).Should(Succeed())

By("fixing conflict with dnsrecord " + dnsRecord2.Name + " with endpoint dnsName: `foo.example.com` and target: `127.0.0.1`")
Expand All @@ -511,6 +521,7 @@ var _ = Describe("DNSRecordReconciler", func() {
"Message": Equal("Provider ensured the dns record"),
})),
)
g.Expect(dnsRecord.Status.DomainOwners).To(ConsistOf(dnsRecord.GetUIDHash(), dnsRecord2.GetUIDHash()))

err = k8sClient.Get(ctx, client.ObjectKeyFromObject(dnsRecord2), dnsRecord2)
g.Expect(err).NotTo(HaveOccurred())
Expand All @@ -522,6 +533,7 @@ var _ = Describe("DNSRecordReconciler", func() {
"Message": Equal("Provider ensured the dns record"),
})),
)
g.Expect(dnsRecord2.Status.DomainOwners).To(ConsistOf(dnsRecord.GetUIDHash(), dnsRecord2.GetUIDHash()))
}, TestTimeoutLong, time.Second).Should(Succeed())
})

Expand Down
15 changes: 14 additions & 1 deletion internal/external-dns/plan/plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ import (

"sigs.k8s.io/external-dns/endpoint"
externaldnsplan "sigs.k8s.io/external-dns/plan"

"github.com/kuadrant/dns-operator/api/v1alpha1"
)

var (
Expand Down Expand Up @@ -65,6 +67,9 @@ type Plan struct {
Errors []error
// RootHost the host dns name being managed by the set of records in the plan.
RootHost *string
// Owners list of owners ids contributing to this record set.
// Populated after calling Calculate()
Owners []string

logger logr.Logger
}
Expand Down Expand Up @@ -208,7 +213,8 @@ func (p *Plan) Calculate() *Plan {
}

if p.RootHost != nil {
rootDomainFilter = endpoint.NewDomainFilter([]string{*p.RootHost})
rootDomainName, _ := strings.CutPrefix(*p.RootHost, v1alpha1.WildcardPrefix)
rootDomainFilter = endpoint.NewDomainFilter([]string{rootDomainName})
p.DomainFilter = append(p.DomainFilter, &rootDomainFilter)
}

Expand Down Expand Up @@ -367,6 +373,13 @@ func (p *Plan) Calculate() *Plan {
ManagedRecords: []string{endpoint.RecordTypeA, endpoint.RecordTypeAAAA, endpoint.RecordTypeCNAME},
}

if p.RootHost != nil {
p.logger.V(1).Info("plan", "record dnsOwners", managedChanges.dnsNameOwners, "record rootHost", *p.RootHost, "record normalized", normalizeDNSName(*p.RootHost))

plan.Owners = managedChanges.dnsNameOwners[normalizeDNSName(*p.RootHost)]

}

return plan
}

Expand Down
27 changes: 18 additions & 9 deletions test/e2e/multi_record_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,8 @@ var _ = Describe("Multi Record Test", Labels{"multi_record"}, func() {
}

By(fmt.Sprintf("checking all dns records become ready within %s", recordsReadyMaxDuration))
var allOwners = []string{}
var allTargetIps = []string{}
Eventually(func(g Gomega, ctx context.Context) {
for _, tr := range testRecords {
err := tr.cluster.k8sClient.Get(ctx, client.ObjectKeyFromObject(tr.record), tr.record)
Expand All @@ -129,7 +131,12 @@ var _ = Describe("Multi Record Test", Labels{"multi_record"}, func() {
"Status": Equal(metav1.ConditionTrue),
})),
)
allOwners = append(allOwners, tr.record.GetUIDHash())
allTargetIps = append(allTargetIps, tr.config.testTargetIP)
g.Expect(tr.record.Status.DomainOwners).NotTo(BeEmpty())
g.Expect(tr.record.Status.DomainOwners).To(ContainElement(tr.record.GetUIDHash()))
}
g.Expect(len(allOwners)).To(Equal(len(testRecords)))
}, recordsReadyMaxDuration, 5*time.Second, ctx).Should(Succeed())

By("checking provider zone records are created as expected")
Expand All @@ -140,11 +147,9 @@ var _ = Describe("Multi Record Test", Labels{"multi_record"}, func() {
Expect(err).NotTo(HaveOccurred())
Expect(zoneEndpoints).To(HaveLen(2))

var allOwners = []string{}
var allTargetIps = []string{}
for i := range testRecords {
allOwners = append(allOwners, testRecords[i].record.Status.OwnerID)
allTargetIps = append(allTargetIps, testRecords[i].config.testTargetIP)
By("checking each record has all owners present")
for _, tr := range testRecords {
Expect(tr.record.Status.DomainOwners).To(ConsistOf(allOwners))
}

By("checking all target ips are present")
Expand Down Expand Up @@ -376,6 +381,7 @@ var _ = Describe("Multi Record Test", Labels{"multi_record"}, func() {
}

By(fmt.Sprintf("checking all dns records become ready within %s", recordsReadyMaxDuration))
var allOwners = []string{}
Eventually(func(g Gomega, ctx context.Context) {
for _, tr := range testRecords {
err := tr.cluster.k8sClient.Get(ctx, client.ObjectKeyFromObject(tr.record), tr.record)
Expand All @@ -386,7 +392,10 @@ var _ = Describe("Multi Record Test", Labels{"multi_record"}, func() {
"Status": Equal(metav1.ConditionTrue),
})),
)
allOwners = append(allOwners, tr.record.GetUIDHash())
g.Expect(tr.record.Status.DomainOwners).To(Not(BeEmpty()))
}
g.Expect(len(allOwners)).To(Equal(len(testRecords)))
}, recordsReadyMaxDuration, 5*time.Second, ctx).Should(Succeed())

By("checking provider zone records are created as expected")
Expand All @@ -405,18 +414,18 @@ var _ = Describe("Multi Record Test", Labels{"multi_record"}, func() {
}

var totalEndpointsChecked = 0
var allOwners = []string{}

var allOwnerMatcher = []types.GomegaMatcher{
ContainSubstring("heritage=external-dns,external-dns/owner="),
}
var geoOwners = map[string][]string{}
var geoKlbHostname = map[string]string{}
var geoOwnerMatcher = map[string][]types.GomegaMatcher{}
for i := range testRecords {
ownerID := testRecords[i].record.Status.OwnerID
allOwners = append(allOwners, ownerID)
underTest := testRecords[i]
ownerID := underTest.record.Status.OwnerID
allOwnerMatcher = append(allOwnerMatcher, ContainSubstring(ownerID))

Expect(underTest.record.Status.DomainOwners).To(ConsistOf(allOwners))
geoCode := testRecords[i].config.testGeoCode
geoOwners[geoCode] = append(geoOwners[geoCode], ownerID)
geoKlbHostname[geoCode] = testRecords[i].config.hostnames.geoKlb
Expand Down
4 changes: 3 additions & 1 deletion test/e2e/single_record_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,8 @@ var _ = Describe("Single Record Test", func() {
"Status": Equal(metav1.ConditionTrue),
})),
)
g.Expect(dnsRecord.Status.DomainOwners).To(ConsistOf(dnsRecord.GetUIDHash()))
}, time.Minute, 10*time.Second, ctx).Should(Succeed())

By("checking " + dnsRecord.Name + " ownerID is set correctly")
Expect(dnsRecord.Spec.OwnerID).To(BeEmpty())
Expect(dnsRecord.Status.OwnerID).ToNot(BeEmpty())
Expand Down Expand Up @@ -194,6 +194,7 @@ var _ = Describe("Single Record Test", func() {
"Status": Equal(metav1.ConditionTrue),
})),
)
g.Expect(dnsRecord.Status.DomainOwners).To(ConsistOf(dnsRecord.GetUIDHash()))
}, time.Minute, 10*time.Second, ctx).Should(Succeed())

By("checking " + dnsRecord.Name + " ownerID is set correctly")
Expand Down Expand Up @@ -337,6 +338,7 @@ var _ = Describe("Single Record Test", func() {
"Status": Equal(metav1.ConditionTrue),
})),
)
g.Expect(dnsRecord.Status.DomainOwners).To(ConsistOf(dnsRecord.GetUIDHash()))
}, time.Minute, 10*time.Second, ctx).Should(Succeed())

By("checking " + dnsRecord.Name + " ownerID is set correctly")
Expand Down

0 comments on commit 99185ba

Please sign in to comment.