Merge pull request #132 from mikenairn/add_mz_validation

fix: Validate MZ id and domainName on create

internal/controller/managedzone_controller.go

@@ -41,7 +41,8 @@ const (
 )
 
 var (
-	ErrProvider = errors.New("ProviderError")
+	ErrProvider       = errors.New("ProviderError")
+	ErrZoneValidation = errors.New("ZoneValidationError")
 )
 
 // ManagedZoneReconciler reconciles a ManagedZone object
@@ -111,6 +112,10 @@ func (r *ManagedZoneReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 			reason = ErrProvider.Error()
 			message = err.Error()
 		}
+		if errors.Is(err, ErrZoneValidation) {
+			reason = ErrZoneValidation.Error()
+			message = err.Error()
+		}
 		setManagedZoneCondition(managedZone, string(v1alpha1.ConditionTypeReady), metav1.ConditionFalse, reason, message)
 		statusUpdateErr := r.Status().Update(ctx, managedZone)
 		if statusUpdateErr != nil {
@@ -169,12 +174,19 @@ func (r *ManagedZoneReconciler) publishManagedZone(ctx context.Context, managedZ
 	if err != nil {
 		return fmt.Errorf("failed to get provider for the zone: %v", provider.SanitizeError(err))
 	}
+
 	mzResp, err := dnsProvider.EnsureManagedZone(managedZone)
+	if err != nil {
+		err = fmt.Errorf("%w, The DNS provider failed to ensure the managed zone: %v", ErrProvider, provider.SanitizeError(err))
+	} else if managedZone.Spec.ID != "" && mzResp.DNSName != managedZone.Spec.DomainName {
+		err = fmt.Errorf("%w, zone DNS name '%s' and managed zone domain name '%s' do not match for zone id '%s'", ErrZoneValidation, mzResp.DNSName, managedZone.Spec.DomainName, managedZone.Spec.ID)
+	}
+
 	if err != nil {
 		managedZone.Status.ID = ""
 		managedZone.Status.RecordCount = 0
 		managedZone.Status.NameServers = nil
-		return fmt.Errorf("%w, The DNS provider failed to ensure the managed zone: %v", ErrProvider, provider.SanitizeError(err))
+		return err
 	}
 
 	managedZone.Status.ID = mzResp.ID

internal/controller/managedzone_controller_test.go

@@ -168,5 +168,65 @@ var _ = Describe("ManagedZoneReconciler", func() {
 				return nil
 			}, TestTimeoutMedium, time.Second).Should(Succeed())
 		})
+
+		It("managed zone should not become ready with a spec.DomainName and spec.ID that do no match provider zone", func() {
+			//Create a managed zone with no spec.ID
+			Expect(k8sClient.Create(ctx, managedZone)).To(Succeed())
+			Eventually(func(g Gomega) {
+				err := k8sClient.Get(ctx, client.ObjectKeyFromObject(managedZone), managedZone)
+				g.Expect(err).NotTo(HaveOccurred())
+				g.Expect(managedZone.Status.ID).To(Equal("example.com"))
+				g.Expect(managedZone.Status.Conditions).To(
+					ContainElement(MatchFields(IgnoreExtras, Fields{
+						"Type":               Equal(string(v1alpha1.ConditionTypeReady)),
+						"Status":             Equal(metav1.ConditionTrue),
+						"ObservedGeneration": Equal(managedZone.Generation),
+					})),
+				)
+			}, TestTimeoutMedium, time.Second).Should(Succeed())
+
+			//Create a second managed zone with spec.ID of known existing managed zone (managedZone.Status.ID) but with a different domainName i.e. !example.com
+			mz := testBuildManagedZone("mz-example-com-2", testNamespace, "foo.example.com", dnsProviderSecret.Name)
+			mz.Spec.ID = managedZone.Status.ID
+			Expect(k8sClient.Create(ctx, mz)).To(Succeed())
+			Eventually(func(g Gomega) error {
+				err := k8sClient.Get(ctx, client.ObjectKey{Namespace: mz.Namespace, Name: mz.Name}, mz)
+				g.Expect(err).NotTo(HaveOccurred())
+				g.Expect(mz.Status.Conditions).To(
+					ContainElement(MatchFields(IgnoreExtras, Fields{
+						"Type":    Equal(string(v1alpha1.ConditionTypeReady)),
+						"Status":  Equal(metav1.ConditionFalse),
+						"Reason":  Equal("ZoneValidationError"),
+						"Message": Equal("ZoneValidationError, zone DNS name 'example.com' and managed zone domain name 'foo.example.com' do not match for zone id 'example.com'"),
+					})),
+				)
+				g.Expect(mz.Finalizers).To(ContainElement(ManagedZoneFinalizer))
+				return nil
+			}, TestTimeoutMedium, time.Second).Should(Succeed())
+
+			//Update second managed zone to use the known existing managed zone domainName (managedZone.Spec.DomainName)
+			Eventually(func(g Gomega) {
+				err := k8sClient.Get(ctx, client.ObjectKeyFromObject(mz), mz)
+				g.Expect(err).NotTo(HaveOccurred())
+				mz.Spec.DomainName = managedZone.Spec.DomainName
+				err = k8sClient.Update(ctx, mz)
+				g.Expect(err).NotTo(HaveOccurred())
+			}, TestTimeoutMedium, time.Second).Should(Succeed())
+
+			Eventually(func(g Gomega) error {
+				err := k8sClient.Get(ctx, client.ObjectKey{Namespace: mz.Namespace, Name: mz.Name}, mz)
+				Expect(err).NotTo(HaveOccurred())
+				g.Expect(mz.Status.Conditions).To(
+					ContainElement(MatchFields(IgnoreExtras, Fields{
+						"Type":               Equal(string(v1alpha1.ConditionTypeReady)),
+						"Status":             Equal(metav1.ConditionTrue),
+						"Reason":             Equal("ProviderSuccess"),
+						"ObservedGeneration": Equal(mz.Generation),
+					})),
+				)
+				g.Expect(mz.Finalizers).To(ContainElement(ManagedZoneFinalizer))
+				return nil
+			}, TestTimeoutMedium, time.Second).Should(Succeed())
+		})
 	})
 })

internal/provider/aws/aws.go

@@ -19,6 +19,7 @@ package aws
 import (
 	"context"
 	"fmt"
+	"strings"
 	"time"
 
 	"github.com/aws/aws-sdk-go/aws"
@@ -163,13 +164,6 @@ func (p *Route53DNSProvider) EnsureManagedZone(zone *v1alpha1.ManagedZone) (prov
 			return managedZoneOutput, err
 		}
 
-		_, err = p.route53Client.UpdateHostedZoneComment(&route53.UpdateHostedZoneCommentInput{
-			Comment: &zone.Spec.Description,
-			Id:      &zoneID,
-		})
-		if err != nil {
-			log.Log.Error(err, "failed to update hosted zone comment")
-		}
 		if getResp.HostedZone == nil {
 			err = fmt.Errorf("aws zone issue. No hosted zone info in response")
 			log.Log.Error(err, "unexpected response")
@@ -179,8 +173,22 @@ func (p *Route53DNSProvider) EnsureManagedZone(zone *v1alpha1.ManagedZone) (prov
 			err = fmt.Errorf("aws zone issue. No hosted zone id in response")
 			return managedZoneOutput, err
 		}
+		if getResp.HostedZone.Name == nil {
+			err = fmt.Errorf("aws zone issue. No hosted zone name in response")
+			return managedZoneOutput, err
+		}
+
+		_, err = p.route53Client.UpdateHostedZoneComment(&route53.UpdateHostedZoneCommentInput{
+			Comment: &zone.Spec.Description,
+			Id:      &zoneID,
+		})
+		if err != nil {
+			log.Log.Error(err, "failed to update hosted zone comment")
+		}
 
 		managedZoneOutput.ID = *getResp.HostedZone.Id
+		managedZoneOutput.DNSName = strings.ToLower(strings.TrimSuffix(*getResp.HostedZone.Name, "."))
+
 		if getResp.HostedZone.ResourceRecordSetCount != nil {
 			managedZoneOutput.RecordCount = *getResp.HostedZone.ResourceRecordSetCount
 		}
@@ -218,11 +226,18 @@ func (p *Route53DNSProvider) EnsureManagedZone(zone *v1alpha1.ManagedZone) (prov
 		err = fmt.Errorf("aws zone creation issue. No hosted zone id in response")
 		return managedZoneOutput, err
 	}
+	if createResp.HostedZone.Name == nil {
+		err = fmt.Errorf("aws zone creation issue. No hosted zone name in response")
+		return managedZoneOutput, err
+	}
+
 	managedZoneOutput.ID = *createResp.HostedZone.Id
+	managedZoneOutput.DNSName = strings.ToLower(strings.TrimSuffix(*createResp.HostedZone.Name, "."))
 
 	if createResp.HostedZone.ResourceRecordSetCount != nil {
 		managedZoneOutput.RecordCount = *createResp.HostedZone.ResourceRecordSetCount
 	}
+
 	if createResp.DelegationSet != nil {
 		managedZoneOutput.NameServers = createResp.DelegationSet.NameServers
 	}

internal/provider/google/google.go

@@ -688,6 +688,7 @@ func (p *GoogleDNSProvider) toManagedZoneOutput(mz *dnsv1.ManagedZone) (provider
 		nameservers = append(nameservers, &mz.NameServers[i])
 	}
 	managedZoneOutput.ID = zoneID
+	managedZoneOutput.DNSName = strings.ToLower(strings.TrimSuffix(mz.DnsName, "."))
 	managedZoneOutput.NameServers = nameservers
 
 	currentRecords, err := p.getResourceRecordSets(p.ctx, zoneID)

internal/provider/inmemory/inmemory.go

@@ -53,19 +53,27 @@ func (i InMemoryDNSProvider) EnsureManagedZone(mz *v1alpha1.ManagedZone) (provid
 	}
 
 	if zoneID != "" {
-		_, err := i.GetZone(zoneID)
+		z, err := i.GetZone(zoneID)
+		if err != nil {
+			return provider.ManagedZoneOutput{}, err
+		}
 		return provider.ManagedZoneOutput{
 			ID:          zoneID,
+			DNSName:     zoneID,
 			NameServers: nil,
-			RecordCount: 0,
-		}, err
+			RecordCount: int64(len(z)),
+		}, nil
 	}
 	err := i.CreateZone(mz.Spec.DomainName)
+	if err != nil {
+		return provider.ManagedZoneOutput{}, err
+	}
 	return provider.ManagedZoneOutput{
 		ID:          mz.Spec.DomainName,
+		DNSName:     mz.Spec.DomainName,
 		NameServers: nil,
 		RecordCount: 0,
-	}, err
+	}, nil
 }
 
 func (i InMemoryDNSProvider) DeleteManagedZone(managedZone *v1alpha1.ManagedZone) error {

internal/provider/provider.go

@@ -49,6 +49,7 @@ type ProviderSpecificLabels struct {
 
 type ManagedZoneOutput struct {
 	ID          string
+	DNSName     string
 	NameServers []*string
 	RecordCount int64
 }