Skip to content

Commit

Permalink
Merge pull request #214 from Kuadrant/gh-213
Browse files Browse the repository at this point in the history
dont proceed if a host is an apex domain
  • Loading branch information
maleck13 authored Aug 22, 2024
2 parents 8a6812d + 55fb806 commit df3d1e9
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 3 deletions.
39 changes: 38 additions & 1 deletion internal/controller/dnsrecord_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())

Expand Down Expand Up @@ -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").
Expand Down
17 changes: 15 additions & 2 deletions internal/provider/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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.
Expand All @@ -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)
Expand Down
15 changes: 15 additions & 0 deletions internal/provider/provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit df3d1e9

Please sign in to comment.