From 5656b94e6d15f07e094b5129f9d3cda41e2effaf Mon Sep 17 00:00:00 2001 From: craig Date: Wed, 21 Aug 2024 16:04:36 +0100 Subject: [PATCH 1/2] dont proceed if a host is an apex domain Signed-off-by: craig --- internal/provider/provider.go | 17 +++++++++++++++-- internal/provider/provider_test.go | 15 +++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/internal/provider/provider.go b/internal/provider/provider.go index ccc986a1..f577ef1a 100644 --- a/internal/provider/provider.go +++ b/internal/provider/provider.go @@ -79,6 +79,16 @@ func FindDNSZoneForHost(ctx context.Context, host string, zones []DNSZone) (*DNS return z, err } +func isApexDomain(host string, zones []DNSZone) (string, bool) { + for _, z := range zones { + if z.DNSName == host { + return z.ID, true + } + } + return "", false +} + +// findDNSZoneForHost will take a host and look for a zone that patches the immediate parent of that host and will continue to step through parents until it either finds a zone or fails. Example *.example.com will look for example.com and other.domain.example.com will step through each subdomain until it hits example.com. func findDNSZoneForHost(originalHost, host string, zones []DNSZone) (*DNSZone, string, error) { if len(zones) == 0 { return nil, "", fmt.Errorf("%w : %s", ErrNoZoneForHost, host) @@ -92,12 +102,16 @@ func findDNSZoneForHost(originalHost, host string, zones []DNSZone) (*DNSZone, s return nil, "", fmt.Errorf("no valid zone found for host: %v", originalHost) } + // We do not currently support creating records for Apex domains, and a DNSZone represents an Apex domain we cannot setup dns for the host + if id, is := isApexDomain(originalHost, zones); is { + return nil, "", fmt.Errorf("host %s is an apex domain with zone id %s. Cannot configure DNS for apex domain as apex domains only support A records", originalHost, id) + } + hostParts := strings.SplitN(host, ".", 2) if len(hostParts) < 2 { return nil, "", fmt.Errorf("no valid zone found for host: %s", originalHost) } parentDomain := hostParts[1] - // We do not currently support creating records for Apex domains, and a DNSZone represents an Apex domain, as such // we should never be trying to find a zone that matches the `originalHost` exactly. Instead, we just continue // on to the next possible valid host to try i.e. the parent domain. @@ -108,7 +122,6 @@ func findDNSZoneForHost(originalHost, host string, zones []DNSZone) (*DNSZone, s matches := slices.DeleteFunc(slices.Clone(zones), func(zone DNSZone) bool { return strings.ToLower(zone.DNSName) != host }) - if len(matches) > 0 { if len(matches) > 1 { return nil, "", fmt.Errorf("multiple zones found for host: %s", originalHost) diff --git a/internal/provider/provider_test.go b/internal/provider/provider_test.go index 32e2baea..1b7f485a 100644 --- a/internal/provider/provider_test.go +++ b/internal/provider/provider_test.go @@ -186,6 +186,21 @@ func Test_findDNSZoneForHost(t *testing.T) { want1: "", wantErr: true, }, + { + name: "apex domain", + host: "test.example.com", + zones: []DNSZone{ + { + DNSName: "example.com", + }, + { + DNSName: "test.example.com", + }, + }, + want: "", + want1: "", + wantErr: true, + }, } for _, tt := range testCases { t.Run(tt.name, func(t *testing.T) { From 55fb8068b333305f042deccf53c33bf563b92619 Mon Sep 17 00:00:00 2001 From: craig Date: Wed, 21 Aug 2024 16:21:47 +0100 Subject: [PATCH 2/2] e2e test Signed-off-by: craig --- .../controller/dnsrecord_controller_test.go | 39 ++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/internal/controller/dnsrecord_controller_test.go b/internal/controller/dnsrecord_controller_test.go index dab81629..cb73bc45 100644 --- a/internal/controller/dnsrecord_controller_test.go +++ b/internal/controller/dnsrecord_controller_test.go @@ -654,7 +654,7 @@ var _ = Describe("DNSRecordReconciler", func() { It("should assign the most suitable zone for the provider", func(ctx SpecContext) { pSecret = pBuilder. - WithZonesInitialisedFor("example.com", "foo.example.com", "bar.foo.example.com"). + WithZonesInitialisedFor("example.com", "foo.example.com"). Build() Expect(k8sClient.Create(ctx, pSecret)).To(Succeed()) @@ -717,6 +717,43 @@ var _ = Describe("DNSRecordReconciler", func() { }, TestTimeoutMedium, time.Second).Should(Succeed()) }) + It("should report an error when an apex domain is used", func(ctx SpecContext) { + pSecret = pBuilder. + WithZonesInitialisedFor("example.com"). + Build() + Expect(k8sClient.Create(ctx, pSecret)).To(Succeed()) + + dnsRecord = &v1alpha1.DNSRecord{ + ObjectMeta: metav1.ObjectMeta{ + Name: "example.com", + Namespace: testNamespace, + }, + Spec: v1alpha1.DNSRecordSpec{ + RootHost: "example.com", + ProviderRef: v1alpha1.ProviderRef{ + Name: pSecret.Name, + }, + Endpoints: getTestEndpoints("example.com", "127.0.0.1"), + }, + } + Expect(k8sClient.Create(ctx, dnsRecord)).To(Succeed()) + Eventually(func(g Gomega) { + err := k8sClient.Get(ctx, client.ObjectKeyFromObject(dnsRecord), dnsRecord) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(dnsRecord.Status.ZoneID).To(BeEmpty()) + g.Expect(dnsRecord.Status.ZoneDomainName).To(BeEmpty()) + g.Expect(dnsRecord.Status.Conditions).To( + ContainElement(MatchFields(IgnoreExtras, Fields{ + "Type": Equal(string(v1alpha1.ConditionTypeReady)), + "Status": Equal(metav1.ConditionFalse), + "Reason": Equal("DNSProviderError"), + "Message": ContainSubstring("is an apex domain"), + "ObservedGeneration": Equal(dnsRecord.Generation), + })), + ) + }, TestTimeoutMedium, time.Second).Should(Succeed()) + }) + It("should update broken record when provider is updated", func(ctx SpecContext) { pSecret = pBuilder. WithZonesInitialisedFor("example.com").