diff --git a/internal/controller/managedzone_controller.go b/internal/controller/managedzone_controller.go index f6ee3383..3fa18429 100644 --- a/internal/controller/managedzone_controller.go +++ b/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 diff --git a/internal/controller/managedzone_controller_test.go b/internal/controller/managedzone_controller_test.go index 9766c322..b12989eb 100644 --- a/internal/controller/managedzone_controller_test.go +++ b/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()) + }) }) }) diff --git a/internal/provider/aws/aws.go b/internal/provider/aws/aws.go index 29c5ef45..554dd684 100644 --- a/internal/provider/aws/aws.go +++ b/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 } diff --git a/internal/provider/google/google.go b/internal/provider/google/google.go index 4b00fe54..1ea99fe1 100644 --- a/internal/provider/google/google.go +++ b/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) diff --git a/internal/provider/inmemory/inmemory.go b/internal/provider/inmemory/inmemory.go index d027a549..159e4c05 100644 --- a/internal/provider/inmemory/inmemory.go +++ b/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 { diff --git a/internal/provider/provider.go b/internal/provider/provider.go index 0a79505c..dcac11d0 100644 --- a/internal/provider/provider.go +++ b/internal/provider/provider.go @@ -49,6 +49,7 @@ type ProviderSpecificLabels struct { type ManagedZoneOutput struct { ID string + DNSName string NameServers []*string RecordCount int64 }