diff --git a/internal/common/helper.go b/internal/common/helper.go index 3a72c9af..3fdc36b5 100644 --- a/internal/common/helper.go +++ b/internal/common/helper.go @@ -3,6 +3,7 @@ package common import ( "time" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/rand" ) @@ -23,3 +24,12 @@ func RandomizeDuration(variance float64, duration time.Duration) time.Duration { int64(lowerLimit), int64(upperLimit))) } + +func Owns(owner, object metav1.Object) bool { + for _, ref := range object.GetOwnerReferences() { + if ref.UID == owner.GetUID() { + return true + } + } + return false +} diff --git a/internal/common/helper_test.go b/internal/common/helper_test.go index 50a9961d..0353abf2 100644 --- a/internal/common/helper_test.go +++ b/internal/common/helper_test.go @@ -5,6 +5,13 @@ package common import ( "testing" "time" + + . "github.com/onsi/gomega" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" + + "github.com/kuadrant/dns-operator/api/v1alpha1" ) func TestRandomizeDuration(t *testing.T) { @@ -41,3 +48,133 @@ func isValidVariance(duration, randomizedDuration time.Duration, variance float6 return float64(randomizedDuration.Milliseconds()) >= lowerLimmit && float64(randomizedDuration.Milliseconds()) < upperLimit } + +func TestOwns(t *testing.T) { + RegisterTestingT(t) + testCases := []struct { + Name string + Object metav1.Object + Owner metav1.Object + Verify func(t *testing.T, result bool) + }{ + { + Name: "object is owned", + Object: &v1alpha1.DNSRecord{ + ObjectMeta: metav1.ObjectMeta{ + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "v1beta1", + Kind: "ManagedZone", + Name: "test-zone", + UID: "unique-uid", + BlockOwnerDeletion: ptr.To(true), + }, + }, + }, + }, + Owner: &v1alpha1.ManagedZone{ + ObjectMeta: metav1.ObjectMeta{ + UID: "unique-uid", + }, + }, + Verify: func(t *testing.T, result bool) { + Expect(result).To(BeTrue()) + }, + }, { + Name: "object is owned by multiple", + Object: &v1alpha1.DNSRecord{ + ObjectMeta: metav1.ObjectMeta{ + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "v1beta1", + Kind: "ManagedZone", + Name: "test-zone-other", + UID: "unique-uid-other", + BlockOwnerDeletion: ptr.To(true), + }, + { + APIVersion: "v1beta1", + Kind: "ManagedZone", + Name: "test-zone", + UID: "unique-uid", + BlockOwnerDeletion: ptr.To(true), + }, + { + APIVersion: "v1beta1", + Kind: "ManagedZone", + Name: "test-zone-other2", + UID: "unique-uid-other2", + BlockOwnerDeletion: ptr.To(true), + }, + }, + }, + }, + Owner: &v1alpha1.ManagedZone{ + ObjectMeta: metav1.ObjectMeta{ + UID: "unique-uid", + }, + }, + Verify: func(t *testing.T, result bool) { + Expect(result).To(BeTrue()) + }, + }, { + Name: "object is not owned", + Object: &v1alpha1.DNSRecord{ + ObjectMeta: metav1.ObjectMeta{ + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "v1beta1", + Kind: "ManagedZone", + Name: "test-zone-other", + UID: "unique-uid-other", + BlockOwnerDeletion: ptr.To(false), + }, + }, + }, + }, + Owner: &v1alpha1.ManagedZone{ + ObjectMeta: metav1.ObjectMeta{ + UID: "unique-uid", + }, + }, + Verify: func(t *testing.T, result bool) { + Expect(result).To(BeFalse()) + }, + }, { + Name: "object is not owned multiple", + Object: &v1alpha1.DNSRecord{ + ObjectMeta: metav1.ObjectMeta{ + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "v1beta1", + Kind: "ManagedZone", + Name: "test-zone-other", + UID: "unique-uid-other", + BlockOwnerDeletion: ptr.To(true), + }, { + APIVersion: "v1beta1", + Kind: "ManagedZone", + Name: "test-zone-other2", + UID: "unique-uid-other2", + BlockOwnerDeletion: ptr.To(false), + }, + }, + }, + }, + Owner: &v1alpha1.ManagedZone{ + ObjectMeta: metav1.ObjectMeta{ + UID: "unique-uid", + }, + }, + Verify: func(t *testing.T, result bool) { + Expect(result).To(BeFalse()) + }, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.Name, func(t *testing.T) { + testCase.Verify(t, Owns(testCase.Owner, testCase.Object)) + }) + } +} diff --git a/internal/controller/dnsrecord_controller.go b/internal/controller/dnsrecord_controller.go index 126928f2..1ee1aa1b 100644 --- a/internal/controller/dnsrecord_controller.go +++ b/internal/controller/dnsrecord_controller.go @@ -40,6 +40,7 @@ import ( "github.com/kuadrant/dns-operator/internal/common" externaldnsplan "github.com/kuadrant/dns-operator/internal/external-dns/plan" externaldnsregistry "github.com/kuadrant/dns-operator/internal/external-dns/registry" + "github.com/kuadrant/dns-operator/internal/metrics" "github.com/kuadrant/dns-operator/internal/provider" ) @@ -96,11 +97,25 @@ func (r *DNSRecordReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( } dnsRecord := previous.DeepCopy() + managedZone := &v1alpha1.ManagedZone{ + ObjectMeta: metav1.ObjectMeta{ + Name: dnsRecord.Spec.ManagedZoneRef.Name, + Namespace: dnsRecord.Namespace, + }, + } + err = r.Get(ctx, client.ObjectKeyFromObject(managedZone), managedZone, &client.GetOptions{}) + if err != nil { + reason := "ManagedZoneError" + message := fmt.Sprintf("The managedZone could not be loaded: %v", err) + setDNSRecordCondition(dnsRecord, string(v1alpha1.ConditionTypeReady), metav1.ConditionFalse, reason, message) + return r.updateStatus(ctx, previous, dnsRecord, false, err) + } + if dnsRecord.DeletionTimestamp != nil && !dnsRecord.DeletionTimestamp.IsZero() { - if err = r.ReconcileHealthChecks(ctx, dnsRecord); client.IgnoreNotFound(err) != nil { + if err = r.ReconcileHealthChecks(ctx, dnsRecord, managedZone); client.IgnoreNotFound(err) != nil { return ctrl.Result{}, err } - hadChanges, err := r.deleteRecord(ctx, dnsRecord) + hadChanges, err := r.deleteRecord(ctx, dnsRecord, managedZone) if err != nil { logger.Error(err, "Failed to delete DNSRecord") return ctrl.Result{}, err @@ -134,6 +149,14 @@ func (r *DNSRecordReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( return ctrl.Result{RequeueAfter: randomizedValidationRequeue}, nil } + if !common.Owns(managedZone, dnsRecord) { + err = controllerutil.SetOwnerReference(managedZone, dnsRecord, r.Scheme) + if err != nil { + return ctrl.Result{}, err + } + err = r.Client.Update(ctx, dnsRecord) + return ctrl.Result{}, err + } var reason, message string err = dnsRecord.Validate() if err != nil { @@ -144,7 +167,7 @@ func (r *DNSRecordReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( } // Publish the record - hadChanges, err := r.publishRecord(ctx, dnsRecord) + hadChanges, err := r.publishRecord(ctx, dnsRecord, managedZone) if err != nil { reason = "ProviderError" message = fmt.Sprintf("The DNS provider failed to ensure the record: %v", provider.SanitizeError(err)) @@ -152,7 +175,7 @@ func (r *DNSRecordReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( return r.updateStatus(ctx, previous, dnsRecord, hadChanges, err) } - if err = r.ReconcileHealthChecks(ctx, dnsRecord); err != nil { + if err = r.ReconcileHealthChecks(ctx, dnsRecord, managedZone); err != nil { return ctrl.Result{}, err } @@ -185,7 +208,7 @@ func (r *DNSRecordReconciler) updateStatus(ctx context.Context, previous, curren // implies that they were overridden - bump write counter if !generationChanged(current) { current.Status.WriteCounter++ - writeCounter.WithLabelValues(current.Name, current.Namespace).Inc() + metrics.WriteCounter.WithLabelValues(current.Name, current.Namespace).Inc() logger.V(1).Info("Changes needed on the same generation of record") } requeueTime = randomizedValidationRequeue @@ -208,7 +231,7 @@ func (r *DNSRecordReconciler) updateStatus(ctx context.Context, previous, curren // reset the counter on the gen change regardless of having changes in the plan if generationChanged(current) { current.Status.WriteCounter = 0 - writeCounter.WithLabelValues(current.Name, current.Namespace).Set(0) + metrics.WriteCounter.WithLabelValues(current.Name, current.Namespace).Set(0) logger.V(1).Info("Resetting write counter on the generation change") } @@ -239,15 +262,15 @@ func (r *DNSRecordReconciler) SetupWithManager(mgr ctrl.Manager, maxRequeue, val For(&v1alpha1.DNSRecord{}). Watches(&v1alpha1.ManagedZone{}, handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, o client.Object) []reconcile.Request { logger := log.FromContext(ctx) - toReconcile := []reconcile.Request{} - // list dns records in the maanagedzone namespace as they will be in the same namespace as the zone + var toReconcile []reconcile.Request + // list dns records in the managedzone namespace as they will be in the same namespace as the zone records := &v1alpha1.DNSRecordList{} if err := mgr.GetClient().List(ctx, records, &client.ListOptions{Namespace: o.GetNamespace()}); err != nil { logger.Error(err, "failed to list dnsrecords ", "namespace", o.GetNamespace()) return toReconcile } for _, record := range records.Items { - if record.Spec.ManagedZoneRef.Name == o.GetName() { + if common.Owns(o, &record) { logger.Info("managed zone updated", "managedzone", o.GetNamespace()+"/"+o.GetName(), "enqueuing dnsrecord ", record.GetName()) toReconcile = append(toReconcile, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(&record)}) } @@ -259,20 +282,9 @@ func (r *DNSRecordReconciler) SetupWithManager(mgr ctrl.Manager, maxRequeue, val // deleteRecord deletes record(s) in the DNSPRovider(i.e. route53) configured by the ManagedZone assigned to this // DNSRecord (dnsRecord.Status.ParentManagedZone). -func (r *DNSRecordReconciler) deleteRecord(ctx context.Context, dnsRecord *v1alpha1.DNSRecord) (bool, error) { +func (r *DNSRecordReconciler) deleteRecord(ctx context.Context, dnsRecord *v1alpha1.DNSRecord, managedZone *v1alpha1.ManagedZone) (bool, error) { logger := log.FromContext(ctx) - managedZone := &v1alpha1.ManagedZone{ - ObjectMeta: metav1.ObjectMeta{ - Name: dnsRecord.Spec.ManagedZoneRef.Name, - Namespace: dnsRecord.Namespace, - }, - } - err := r.Get(ctx, client.ObjectKeyFromObject(managedZone), managedZone, &client.GetOptions{}) - if err != nil { - // If the Managed Zone isn't found, just continue - return false, client.IgnoreNotFound(err) - } managedZoneReady := meta.IsStatusConditionTrue(managedZone.Status.Conditions, "Ready") if !managedZoneReady { @@ -297,18 +309,9 @@ func (r *DNSRecordReconciler) deleteRecord(ctx context.Context, dnsRecord *v1alp // publishRecord publishes record(s) to the DNSPRovider(i.e. route53) configured by the ManagedZone assigned to this // DNSRecord (dnsRecord.Status.ParentManagedZone). -func (r *DNSRecordReconciler) publishRecord(ctx context.Context, dnsRecord *v1alpha1.DNSRecord) (bool, error) { +func (r *DNSRecordReconciler) publishRecord(ctx context.Context, dnsRecord *v1alpha1.DNSRecord, managedZone *v1alpha1.ManagedZone) (bool, error) { logger := log.FromContext(ctx) - managedZone := &v1alpha1.ManagedZone{ - ObjectMeta: metav1.ObjectMeta{ - Name: dnsRecord.Spec.ManagedZoneRef.Name, - Namespace: dnsRecord.Namespace, - }, - } - err := r.Get(ctx, client.ObjectKeyFromObject(managedZone), managedZone, &client.GetOptions{}) - if err != nil { - return false, err - } + managedZoneReady := meta.IsStatusConditionTrue(managedZone.Status.Conditions, "Ready") if !managedZoneReady { @@ -355,14 +358,14 @@ func exponentialRequeueTime(lastRequeueTime string) time.Duration { return randomizedValidationRequeue } // double the duration. Return the max timeout if overshoot - newReqeueue := lastRequeue * 2 - if newReqeueue > defaultRequeueTime { + newRequeue := lastRequeue * 2 + if newRequeue > defaultRequeueTime { return defaultRequeueTime } - return newReqeueue + return newRequeue } -// setDNSRecordCondition adds or updates a given condition in the DNSRecord status.. +// setDNSRecordCondition adds or updates a given condition in the DNSRecord status. func setDNSRecordCondition(dnsRecord *v1alpha1.DNSRecord, conditionType string, status metav1.ConditionStatus, reason, message string) { cond := metav1.Condition{ Type: conditionType, @@ -374,18 +377,7 @@ func setDNSRecordCondition(dnsRecord *v1alpha1.DNSRecord, conditionType string, meta.SetStatusCondition(&dnsRecord.Status.Conditions, cond) } -func (r *DNSRecordReconciler) getDNSProvider(ctx context.Context, dnsRecord *v1alpha1.DNSRecord) (provider.Provider, error) { - managedZone := &v1alpha1.ManagedZone{ - ObjectMeta: metav1.ObjectMeta{ - Name: dnsRecord.Spec.ManagedZoneRef.Name, - Namespace: dnsRecord.Namespace, - }, - } - err := r.Get(ctx, client.ObjectKeyFromObject(managedZone), managedZone, &client.GetOptions{}) - if err != nil { - return nil, err - } - +func (r *DNSRecordReconciler) getDNSProvider(ctx context.Context, managedZone *v1alpha1.ManagedZone) (provider.Provider, error) { providerConfig := provider.Config{ DomainFilter: externaldnsendpoint.NewDomainFilter([]string{managedZone.Spec.DomainName}), ZoneTypeFilter: externaldnsprovider.NewZoneTypeFilter(""), @@ -403,9 +395,8 @@ func (r *DNSRecordReconciler) applyChanges(ctx context.Context, dnsRecord *v1alp rootDomainName, _ := strings.CutPrefix(dnsRecord.Spec.RootHost, v1alpha1.WildcardPrefix) zoneDomainFilter := externaldnsendpoint.NewDomainFilter([]string{zoneDomainName}) managedDNSRecordTypes := []string{externaldnsendpoint.RecordTypeA, externaldnsendpoint.RecordTypeAAAA, externaldnsendpoint.RecordTypeCNAME} - excludeDNSRecordTypes := []string{} - - dnsProvider, err := r.getDNSProvider(ctx, dnsRecord) + var excludeDNSRecordTypes []string + dnsProvider, err := r.getDNSProvider(ctx, managedZone) if err != nil { return false, err } diff --git a/internal/controller/dnsrecord_controller_test.go b/internal/controller/dnsrecord_controller_test.go index 9198deda..0bd5f3a1 100644 --- a/internal/controller/dnsrecord_controller_test.go +++ b/internal/controller/dnsrecord_controller_test.go @@ -32,6 +32,7 @@ import ( externaldnsendpoint "sigs.k8s.io/external-dns/endpoint" "github.com/kuadrant/dns-operator/api/v1alpha1" + "github.com/kuadrant/dns-operator/internal/common" ) var _ = Describe("DNSRecordReconciler", func() { @@ -140,7 +141,6 @@ var _ = Describe("DNSRecordReconciler", func() { "ObservedGeneration": Equal(dnsRecord.Generation), })), ) - g.Expect(dnsRecord.Finalizers).To(ContainElement(DNSRecordFinalizer)) }, TestTimeoutMedium, time.Second).Should(Succeed()) fixedZone := brokenZone.DeepCopy() @@ -162,8 +162,7 @@ var _ = Describe("DNSRecordReconciler", func() { g.Expect(dnsRecord.Finalizers).To(ContainElement(DNSRecordFinalizer)) }, TestTimeoutMedium, time.Second).Should(Succeed()) }) - - It("can delete a record with an invalid managed zone", func(ctx SpecContext) { + It("maintains the finalizer on a deleting DNS record with a deleted managed zone", func(ctx SpecContext) { dnsRecord = &v1alpha1.DNSRecord{ ObjectMeta: metav1.ObjectMeta{ Name: "foo.example.com", @@ -173,13 +172,29 @@ var _ = Describe("DNSRecordReconciler", func() { OwnerID: "owner1", RootHost: "foo.example.com", ManagedZoneRef: &v1alpha1.ManagedZoneReference{ - Name: "doesnotexist", + Name: managedZone.Name, }, Endpoints: getDefaultTestEndpoints(), HealthCheck: nil, }, } 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.Conditions).To( + ContainElement(MatchFields(IgnoreExtras, Fields{ + "Type": Equal(string(v1alpha1.ConditionTypeReady)), + "Status": Equal(metav1.ConditionTrue), + "Reason": Equal("ProviderSuccess"), + "Message": Equal("Provider ensured the dns record"), + "ObservedGeneration": Equal(dnsRecord.Generation), + })), + ) + g.Expect(dnsRecord.Finalizers).To(ContainElement(DNSRecordFinalizer)) + }, TestTimeoutMedium, time.Second).Should(Succeed()) + + Expect(k8sClient.Delete(ctx, managedZone)).To(Succeed()) Eventually(func(g Gomega) { err := k8sClient.Get(ctx, client.ObjectKeyFromObject(dnsRecord), dnsRecord) @@ -188,24 +203,64 @@ var _ = Describe("DNSRecordReconciler", func() { ContainElement(MatchFields(IgnoreExtras, Fields{ "Type": Equal(string(v1alpha1.ConditionTypeReady)), "Status": Equal(metav1.ConditionFalse), - "Reason": Equal("ProviderError"), - "Message": Equal("The DNS provider failed to ensure the record: ManagedZone.kuadrant.io \"doesnotexist\" not found"), + "Reason": Equal("ManagedZoneError"), + "Message": Equal("The managedZone could not be loaded: ManagedZone.kuadrant.io \"mz-example-com\" not found"), "ObservedGeneration": Equal(dnsRecord.Generation), })), ) + g.Expect(common.Owns(managedZone, dnsRecord)).To(BeTrue()) g.Expect(dnsRecord.Finalizers).To(ContainElement(DNSRecordFinalizer)) }, TestTimeoutMedium, time.Second).Should(Succeed()) err := k8sClient.Delete(ctx, dnsRecord) Expect(err).ToNot(HaveOccurred()) + Consistently(func(g Gomega, ctx context.Context) { + err := k8sClient.Get(ctx, client.ObjectKeyFromObject(dnsRecord), dnsRecord) + g.Expect(err).ToNot(HaveOccurred()) + }, TestTimeoutMedium, time.Second, ctx).Should(Succeed()) + }) + It("can delete a record with an valid managed zone", func(ctx SpecContext) { + dnsRecord = &v1alpha1.DNSRecord{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo.example.com", + Namespace: testNamespace, + }, + Spec: v1alpha1.DNSRecordSpec{ + OwnerID: "owner1", + RootHost: "foo.example.com", + ManagedZoneRef: &v1alpha1.ManagedZoneReference{ + Name: managedZone.Name, + }, + Endpoints: getDefaultTestEndpoints(), + HealthCheck: nil, + }, + } + 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.Conditions).To( + ContainElement(MatchFields(IgnoreExtras, Fields{ + "Type": Equal(string(v1alpha1.ConditionTypeReady)), + "Status": Equal(metav1.ConditionTrue), + "ObservedGeneration": Equal(dnsRecord.Generation), + })), + ) + g.Expect(dnsRecord.Finalizers).To(ContainElement(DNSRecordFinalizer)) + g.Expect(common.Owns(managedZone, dnsRecord)).To(BeTrue()) + }, TestTimeoutMedium, time.Second).Should(Succeed()) + + err := k8sClient.Delete(ctx, dnsRecord) + Expect(err).ToNot(HaveOccurred()) + Eventually(func(g Gomega, ctx context.Context) { err := k8sClient.Get(ctx, client.ObjectKeyFromObject(dnsRecord), dnsRecord) g.Expect(err).To(HaveOccurred()) g.Expect(err).To(MatchError(ContainSubstring("not found"))) }, 5*time.Second, time.Second, ctx).Should(Succeed()) }) - It("should have ready condition with status true", func() { Expect(k8sClient.Create(ctx, dnsRecord)).To(Succeed()) Eventually(func(g Gomega) { diff --git a/internal/controller/dnsrecord_healthchecks.go b/internal/controller/dnsrecord_healthchecks.go index 00cd01a1..a4df4803 100644 --- a/internal/controller/dnsrecord_healthchecks.go +++ b/internal/controller/dnsrecord_healthchecks.go @@ -23,11 +23,11 @@ type healthChecksConfig struct { Protocol *provider.HealthCheckProtocol } -func (r *DNSRecordReconciler) ReconcileHealthChecks(ctx context.Context, dnsRecord *v1alpha1.DNSRecord) error { +func (r *DNSRecordReconciler) ReconcileHealthChecks(ctx context.Context, dnsRecord *v1alpha1.DNSRecord, managedZone *v1alpha1.ManagedZone) error { var results []provider.HealthCheckResult var err error - dnsProvider, err := r.getDNSProvider(ctx, dnsRecord) + dnsProvider, err := r.getDNSProvider(ctx, managedZone) if err != nil { return err } diff --git a/internal/controller/managedzone_controller.go b/internal/controller/managedzone_controller.go index b802e291..00263977 100644 --- a/internal/controller/managedzone_controller.go +++ b/internal/controller/managedzone_controller.go @@ -22,6 +22,7 @@ import ( "fmt" "strings" + v1 "k8s.io/api/core/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -29,10 +30,13 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/reconcile" externaldns "sigs.k8s.io/external-dns/endpoint" "github.com/kuadrant/dns-operator/api/v1alpha1" + "github.com/kuadrant/dns-operator/internal/metrics" "github.com/kuadrant/dns-operator/internal/provider" ) @@ -41,8 +45,9 @@ const ( ) var ( - ErrProvider = errors.New("ProviderError") - ErrZoneValidation = errors.New("ZoneValidationError") + ErrProvider = errors.New("ProviderError") + ErrZoneValidation = errors.New("ZoneValidationError") + ErrProviderSecretMissing = errors.New("ProviderSecretMissing") ) // ManagedZoneReconciler reconciles a ManagedZone object @@ -71,6 +76,7 @@ func (r *ManagedZoneReconciler) Reconcile(ctx context.Context, req ctrl.Request) return ctrl.Result{}, err } } + managedZone := previous.DeepCopy() if managedZone.DeletionTimestamp != nil && !managedZone.DeletionTimestamp.IsZero() { @@ -117,6 +123,17 @@ func (r *ManagedZoneReconciler) Reconcile(ctx context.Context, req ctrl.Request) reason = ErrZoneValidation.Error() message = err.Error() } + if errors.Is(err, ErrProviderSecretMissing) { + metrics.SecretMissing.WithLabelValues(managedZone.Name, managedZone.Namespace, managedZone.Spec.SecretRef.Name).Set(1) + reason = "DNSProviderSecretNotFound" + message = fmt.Sprintf( + "Could not find secret: %v/%v for managedzone: %v/%v", + managedZone.Namespace, managedZone.Spec.SecretRef.Name, + managedZone.Namespace, managedZone.Name) + } else { + metrics.SecretMissing.WithLabelValues(managedZone.Name, managedZone.Namespace, managedZone.Spec.SecretRef.Name).Set(0) + } + setManagedZoneCondition(managedZone, string(v1alpha1.ConditionTypeReady), metav1.ConditionFalse, reason, message) statusUpdateErr := r.Status().Update(ctx, managedZone) if statusUpdateErr != nil { @@ -166,6 +183,24 @@ func (r *ManagedZoneReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). For(&v1alpha1.ManagedZone{}). Owns(&v1alpha1.ManagedZone{}). + Owns(&v1alpha1.DNSRecord{}). + Watches(&v1.Secret{}, handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, o client.Object) []reconcile.Request { + logger := log.FromContext(ctx) + var toReconcile []reconcile.Request + + zones := &v1alpha1.ManagedZoneList{} + if err := mgr.GetClient().List(ctx, zones, &client.ListOptions{Namespace: o.GetNamespace()}); err != nil { + logger.Error(err, "failed to list zones ", "namespace", o.GetNamespace()) + return toReconcile + } + for _, zone := range zones.Items { + if zone.Spec.SecretRef.Name == o.GetName() { + logger.Info("managed zone secret updated", "secret", o.GetNamespace()+"/"+o.GetName(), "enqueuing zone ", zone.GetName()) + toReconcile = append(toReconcile, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(&zone)}) + } + } + return toReconcile + })). Complete(r) } @@ -173,7 +208,13 @@ func (r *ManagedZoneReconciler) publishManagedZone(ctx context.Context, managedZ dnsProvider, err := r.ProviderFactory.ProviderFor(ctx, managedZone, provider.Config{}) if err != nil { - return fmt.Errorf("failed to get provider for the zone: %v", provider.SanitizeError(err)) + if k8serrors.IsNotFound(err) { + return fmt.Errorf("%w, the secret '%s/%s', referenced in the managedZone '%s/%s' does not exist", + ErrProviderSecretMissing, + managedZone.Namespace, managedZone.Spec.SecretRef.Name, + managedZone.Namespace, managedZone.Name) + } + return fmt.Errorf("failed to get provider for the zone: %w", provider.SanitizeError(err)) } mzResp, err := dnsProvider.EnsureManagedZone(managedZone) diff --git a/internal/controller/managedzone_controller_test.go b/internal/controller/managedzone_controller_test.go index b12989eb..0937333d 100644 --- a/internal/controller/managedzone_controller_test.go +++ b/internal/controller/managedzone_controller_test.go @@ -228,5 +228,23 @@ var _ = Describe("ManagedZoneReconciler", func() { return nil }, TestTimeoutMedium, time.Second).Should(Succeed()) }) + It("reports an error when managed zone secret is absent", func(ctx SpecContext) { + badSecretMZ := testBuildManagedZone("mz-example-com", testNamespace, "example.com", "badSecretName") + Expect(k8sClient.Create(ctx, badSecretMZ)).To(Succeed()) + + Eventually(func(g Gomega) { + err := k8sClient.Get(ctx, client.ObjectKeyFromObject(badSecretMZ), badSecretMZ) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(badSecretMZ.Status.Conditions).To( + ContainElement(MatchFields(IgnoreExtras, Fields{ + "Type": Equal(string(v1alpha1.ConditionTypeReady)), + "Status": Equal(metav1.ConditionFalse), + "ObservedGeneration": Equal(badSecretMZ.Generation), + "Reason": Equal("DNSProviderSecretNotFound"), + })), + ) + }, TestTimeoutMedium, time.Second).Should(Succeed()) + + }) }) }) diff --git a/internal/controller/metrics.go b/internal/controller/metrics.go deleted file mode 100644 index 9697656f..00000000 --- a/internal/controller/metrics.go +++ /dev/null @@ -1,25 +0,0 @@ -package controller - -import ( - "github.com/prometheus/client_golang/prometheus" - - "sigs.k8s.io/controller-runtime/pkg/metrics" -) - -const ( - dnsRecordNameLabel = "dns_record_name" - dnsRecordNamespaceLabel = "dns_record_namespace" -) - -var ( - writeCounter = prometheus.NewGaugeVec( - prometheus.GaugeOpts{ - Name: "dns_provider_write_counter", - Help: "Counts DNS provider write operations for a current generation of the DNS record", - }, - []string{dnsRecordNameLabel, dnsRecordNamespaceLabel}) -) - -func init() { - metrics.Registry.MustRegister(writeCounter) -} diff --git a/internal/metrics/metrics.go b/internal/metrics/metrics.go new file mode 100644 index 00000000..03ceb6e8 --- /dev/null +++ b/internal/metrics/metrics.go @@ -0,0 +1,35 @@ +package metrics + +import ( + "github.com/prometheus/client_golang/prometheus" + + "sigs.k8s.io/controller-runtime/pkg/metrics" +) + +const ( + dnsRecordNameLabel = "dns_record_name" + dnsRecordNamespaceLabel = "dns_record_namespace" + mzRecordNameLabel = "managed_zone_name" + mzRecordNamespaceLabel = "managed_zone_namespace" + mzSecretNameLabel = "managed_zone_secret_name" +) + +var ( + WriteCounter = prometheus.NewGaugeVec( + prometheus.GaugeOpts{ + Name: "dns_provider_write_counter", + Help: "Counts DNS provider write operations for a current generation of the DNS record", + }, + []string{dnsRecordNameLabel, dnsRecordNamespaceLabel}) + SecretMissing = prometheus.NewGaugeVec( + prometheus.GaugeOpts{ + Name: "dns_provider_secret_absent", + Help: "Emits one when provider secret is found to be absent, or zero when expected secrets exist", + }, + []string{mzRecordNameLabel, mzRecordNamespaceLabel, mzSecretNameLabel}) +) + +func init() { + metrics.Registry.MustRegister(WriteCounter) + metrics.Registry.MustRegister(SecretMissing) +}