From b55dbdf10aacc672e1315325ef037d7ceab92517 Mon Sep 17 00:00:00 2001 From: Michael Nairn Date: Mon, 21 Oct 2024 17:13:14 +0100 Subject: [PATCH] sotw dnspolicy: dnspolicies reconciliation Signed-off-by: Michael Nairn --- controllers/dns_helper.go | 77 +------ controllers/dns_workflow.go | 5 +- controllers/dnspolicy_controller.go | 156 +------------ controllers/dnspolicy_dnsrecords.go | 140 ++---------- .../effective_dnspolicies_reconciler.go | 207 ++++++++++++++++-- controllers/state_of_the_world.go | 2 +- controllers/test_common.go | 13 -- main.go | 14 -- pkg/library/utils/k8s_utils.go | 8 +- ...nspolicy_controller_single_cluster_test.go | 12 +- .../dnspolicy/dnspolicy_controller_test.go | 53 ++--- 11 files changed, 241 insertions(+), 446 deletions(-) diff --git a/controllers/dns_helper.go b/controllers/dns_helper.go index fbafe72f2..175b78048 100644 --- a/controllers/dns_helper.go +++ b/controllers/dns_helper.go @@ -1,16 +1,12 @@ package controllers import ( - "context" "fmt" "net" "strings" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "sigs.k8s.io/controller-runtime/pkg/client" gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" - kuadrantdnsv1alpha1 "github.com/kuadrant/dns-operator/api/v1alpha1" "github.com/kuadrant/dns-operator/pkg/builder" "github.com/kuadrant/kuadrant-operator/api/v1alpha1" @@ -18,84 +14,13 @@ import ( const ( LabelGatewayReference = "kuadrant.io/gateway" - LabelGatewayNSRef = "kuadrant.io/gateway-namespace" LabelListenerReference = "kuadrant.io/listener-name" ) -type dnsHelper struct { - client.Client -} - -func commonDNSRecordLabels(gwKey client.ObjectKey, p *v1alpha1.DNSPolicy) map[string]string { - commonLabels := CommonLabels() - for k, v := range policyDNSRecordLabels(p) { - commonLabels[k] = v - } - for k, v := range gatewayDNSRecordLabels(gwKey) { - commonLabels[k] = v - } - return commonLabels -} - -func policyDNSRecordLabels(p *v1alpha1.DNSPolicy) map[string]string { - return map[string]string{ - p.DirectReferenceAnnotationName(): p.Name, - fmt.Sprintf("%s-namespace", p.DirectReferenceAnnotationName()): p.Namespace, - } -} - -func gatewayDNSRecordLabels(gwKey client.ObjectKey) map[string]string { - return map[string]string{ - LabelGatewayNSRef: gwKey.Namespace, - LabelGatewayReference: gwKey.Name, - } -} - -// removeDNSForDeletedListeners remove any DNSRecords that are associated with listeners that no longer exist in this gateway -func (dh *dnsHelper) removeDNSForDeletedListeners(ctx context.Context, upstreamGateway *gatewayapiv1.Gateway) error { - dnsList := &kuadrantdnsv1alpha1.DNSRecordList{} - //List all dns records that belong to this gateway - labelSelector := &client.MatchingLabels{ - LabelGatewayReference: upstreamGateway.Name, - } - if err := dh.List(ctx, dnsList, labelSelector, &client.ListOptions{Namespace: upstreamGateway.Namespace}); err != nil { - return err - } - - for i, dnsRecord := range dnsList.Items { - listenerExists := false - rootHostMatches := false - for _, listener := range upstreamGateway.Spec.Listeners { - if listener.Name == gatewayapiv1.SectionName(dnsRecord.Labels[LabelListenerReference]) { - listenerExists = true - rootHostMatches = string(*listener.Hostname) == dnsRecord.Spec.RootHost - break - } - } - if !listenerExists || !rootHostMatches { - if err := dh.Delete(ctx, &dnsList.Items[i], &client.DeleteOptions{}); client.IgnoreNotFound(err) != nil { - return err - } - } - } - return nil -} - func dnsRecordName(gatewayName, listenerName string) string { return fmt.Sprintf("%s-%s", gatewayName, listenerName) } -func (dh *dnsHelper) deleteDNSRecordForListener(ctx context.Context, owner metav1.Object, listener gatewayapiv1.Listener) error { - recordName := dnsRecordName(owner.GetName(), string(listener.Name)) - dnsRecord := kuadrantdnsv1alpha1.DNSRecord{ - ObjectMeta: metav1.ObjectMeta{ - Name: recordName, - Namespace: owner.GetNamespace(), - }, - } - return dh.Delete(ctx, &dnsRecord, &client.DeleteOptions{}) -} - // GatewayWrapper is a wrapper for gateway to implement interface form the builder type GatewayWrapper struct { *gatewayapiv1.Gateway @@ -106,7 +31,7 @@ func NewGatewayWrapper(gateway *gatewayapiv1.Gateway) *GatewayWrapper { return &GatewayWrapper{Gateway: gateway} } -func (g GatewayWrapper) GetAddresses() []builder.TargetAddress { +func (g *GatewayWrapper) GetAddresses() []builder.TargetAddress { addresses := make([]builder.TargetAddress, len(g.Status.Addresses)) for i, address := range g.Status.Addresses { addresses[i] = builder.TargetAddress{ diff --git a/controllers/dns_workflow.go b/controllers/dns_workflow.go index 80d0a0fc0..e4a954b4a 100644 --- a/controllers/dns_workflow.go +++ b/controllers/dns_workflow.go @@ -8,6 +8,7 @@ import ( "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/dynamic" gwapiv1 "sigs.k8s.io/gateway-api/apis/v1" @@ -40,11 +41,11 @@ var ( //+kubebuilder:rbac:groups=kuadrant.io,resources=dnsrecords,verbs=get;list;watch;create;update;patch;delete //+kubebuilder:rbac:groups=kuadrant.io,resources=dnsrecords/status,verbs=get -func NewDNSWorkflow(client *dynamic.DynamicClient) *controller.Workflow { +func NewDNSWorkflow(client *dynamic.DynamicClient, scheme *runtime.Scheme) *controller.Workflow { return &controller.Workflow{ Precondition: NewDNSPoliciesValidator().Subscription().Reconcile, Tasks: []controller.ReconcileFunc{ - NewEffectiveDNSPoliciesReconciler(client).Subscription().Reconcile, + NewEffectiveDNSPoliciesReconciler(client, scheme).Subscription().Reconcile, }, Postcondition: NewDNSPolicyStatusUpdater(client).Subscription().Reconcile, } diff --git a/controllers/dnspolicy_controller.go b/controllers/dnspolicy_controller.go index 57541f4f5..fcb0c85c3 100644 --- a/controllers/dnspolicy_controller.go +++ b/controllers/dnspolicy_controller.go @@ -17,165 +17,11 @@ limitations under the License. package controllers import ( - "context" - "fmt" - "github.com/prometheus/client_golang/prometheus" - "sigs.k8s.io/controller-runtime/pkg/metrics" - - apierrors "k8s.io/apimachinery/pkg/api/errors" - 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" - crlog "sigs.k8s.io/controller-runtime/pkg/log" - gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" - kuadrantdnsv1alpha1 "github.com/kuadrant/dns-operator/api/v1alpha1" - - "github.com/kuadrant/kuadrant-operator/api/v1alpha1" - kuadrantgatewayapi "github.com/kuadrant/kuadrant-operator/pkg/library/gatewayapi" - "github.com/kuadrant/kuadrant-operator/pkg/library/kuadrant" - "github.com/kuadrant/kuadrant-operator/pkg/library/mappers" - "github.com/kuadrant/kuadrant-operator/pkg/library/reconcilers" + "sigs.k8s.io/controller-runtime/pkg/metrics" ) -const DNSPolicyFinalizer = "kuadrant.io/dns-policy" - -type DNSPolicyRefsConfig struct{} - -// DNSPolicyReconciler reconciles a DNSPolicy object -type DNSPolicyReconciler struct { - *reconcilers.BaseReconciler - TargetRefReconciler reconcilers.TargetRefReconciler - dnsHelper dnsHelper -} - -func (r *DNSPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { - log := r.Logger().WithValues("DNSPolicy", req.NamespacedName) - log.Info("Reconciling DNSPolicy") - ctx = crlog.IntoContext(ctx, log) - - previous := &v1alpha1.DNSPolicy{} - if err := r.Client().Get(ctx, req.NamespacedName, previous); err != nil { - log.Info("error getting dns policy", "error", err) - return ctrl.Result{}, client.IgnoreNotFound(err) - } - - dnsPolicy := previous.DeepCopy() - log.V(3).Info("DNSPolicyReconciler Reconcile", "dnsPolicy", dnsPolicy) - - markedForDeletion := dnsPolicy.GetDeletionTimestamp() != nil - - targetNetworkObject, err := reconcilers.FetchTargetRefObject(ctx, r.Client(), dnsPolicy.GetTargetRef(), dnsPolicy.Namespace, dnsPolicy.TargetProgrammedGatewaysOnly()) - if err != nil { - if !markedForDeletion { - if apierrors.IsNotFound(err) { - log.V(3).Info("Network object not found. Cleaning up") - delResErr := r.deleteResources(ctx, dnsPolicy, nil) - if delResErr == nil { - delResErr = err - } - return ctrl.Result{}, kuadrant.NewErrTargetNotFound(dnsPolicy.Kind(), dnsPolicy.GetTargetRef(), delResErr) - } - return ctrl.Result{}, err - } - targetNetworkObject = nil // we need the object set to nil when there's an error, otherwise deleting the resources (when marked for deletion) will panic - } - - if markedForDeletion { - log.V(3).Info("cleaning up dns policy") - if controllerutil.ContainsFinalizer(dnsPolicy, DNSPolicyFinalizer) { - if err := r.deleteResources(ctx, dnsPolicy, targetNetworkObject); err != nil { - return ctrl.Result{}, err - } - if err := r.RemoveFinalizer(ctx, dnsPolicy, DNSPolicyFinalizer); err != nil { - return ctrl.Result{}, err - } - } - - return ctrl.Result{}, nil - } - - // add finalizer to the dnsPolicy - if !controllerutil.ContainsFinalizer(dnsPolicy, DNSPolicyFinalizer) { - if err := r.AddFinalizer(ctx, dnsPolicy, DNSPolicyFinalizer); client.IgnoreNotFound(err) != nil { - return ctrl.Result{Requeue: true}, err - } else if apierrors.IsNotFound(err) { - return ctrl.Result{}, err - } - } - - specErr := r.reconcileResources(ctx, dnsPolicy, targetNetworkObject) - - return ctrl.Result{}, specErr -} - -func (r *DNSPolicyReconciler) reconcileResources(ctx context.Context, dnsPolicy *v1alpha1.DNSPolicy, targetNetworkObject client.Object) error { - // reconcile based on gateway diffs - gatewayDiffObj, err := reconcilers.ComputeGatewayDiffs(ctx, r.Client(), dnsPolicy, targetNetworkObject) - if err != nil { - return err - } - - if err = r.reconcileDNSRecords(ctx, dnsPolicy, gatewayDiffObj); err != nil { - return fmt.Errorf("error reconciling DNSRecords %w", err) - } - - // set direct back ref - i.e. claim the target network object as taken asap - if err = r.TargetRefReconciler.ReconcileTargetBackReference(ctx, dnsPolicy, targetNetworkObject, dnsPolicy.DirectReferenceAnnotationName()); err != nil { - return fmt.Errorf("reconcile TargetBackReference error %w", err) - } - - // set annotation of policies affecting the gateway - if err := r.TargetRefReconciler.ReconcileGatewayPolicyReferences(ctx, dnsPolicy, gatewayDiffObj); err != nil { - return fmt.Errorf("ReconcileGatewayPolicyReferences error %w", err) - } - - return nil -} - -func (r *DNSPolicyReconciler) deleteResources(ctx context.Context, dnsPolicy *v1alpha1.DNSPolicy, targetNetworkObject client.Object) error { - // remove direct back ref - if targetNetworkObject != nil { - if err := r.TargetRefReconciler.DeleteTargetBackReference(ctx, targetNetworkObject, dnsPolicy.DirectReferenceAnnotationName()); err != nil { - return err - } - } - - gatewayDiffObj, err := reconcilers.ComputeGatewayDiffs(ctx, r.Client(), dnsPolicy, targetNetworkObject) - if err != nil { - return err - } - - // update annotation of policies affecting the gateway - return r.TargetRefReconciler.ReconcileGatewayPolicyReferences(ctx, dnsPolicy, gatewayDiffObj) -} - -func (r *DNSPolicyReconciler) SetupWithManager(mgr ctrl.Manager) error { - ok, err := kuadrantgatewayapi.IsGatewayAPIInstalled(mgr.GetRESTMapper()) - if err != nil { - return err - } - if !ok { - r.Logger().Info("DNSPolicy controller disabled. GatewayAPI was not found") - return nil - } - - gatewayEventMapper := mappers.NewGatewayEventMapper( - v1alpha1.NewDNSPolicyType(), - mappers.WithLogger(r.Logger().WithName("gateway.mapper")), - mappers.WithClient(mgr.GetClient()), - ) - - r.dnsHelper = dnsHelper{Client: r.Client()} - ctrlr := ctrl.NewControllerManagedBy(mgr). - For(&v1alpha1.DNSPolicy{}). - Owns(&kuadrantdnsv1alpha1.DNSRecord{}). - Watches(&gatewayapiv1.Gateway{}, handler.EnqueueRequestsFromMapFunc(gatewayEventMapper.Map)) - return ctrlr.Complete(r) -} - const ( dnsPolicyNameLabel = "dns_policy_name" dnsPolicyNamespaceLabel = "dns_policy_namespace" diff --git a/controllers/dnspolicy_dnsrecords.go b/controllers/dnspolicy_dnsrecords.go index 144e4356b..0daca897f 100644 --- a/controllers/dnspolicy_dnsrecords.go +++ b/controllers/dnspolicy_dnsrecords.go @@ -1,14 +1,9 @@ package controllers import ( - "context" "fmt" - "reflect" - apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "sigs.k8s.io/controller-runtime/pkg/client" - crlog "sigs.k8s.io/controller-runtime/pkg/log" externaldns "sigs.k8s.io/external-dns/endpoint" gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" @@ -16,8 +11,6 @@ import ( "github.com/kuadrant/dns-operator/pkg/builder" "github.com/kuadrant/kuadrant-operator/api/v1alpha1" - reconcilerutils "github.com/kuadrant/kuadrant-operator/pkg/library/reconcilers" - "github.com/kuadrant/kuadrant-operator/pkg/library/utils" ) var ( @@ -25,105 +18,7 @@ var ( ErrNoAddresses = fmt.Errorf("no valid status addresses to use on gateway") ) -func (r *DNSPolicyReconciler) reconcileDNSRecords(ctx context.Context, dnsPolicy *v1alpha1.DNSPolicy, gwDiffObj *reconcilerutils.GatewayDiffs) error { - log := crlog.FromContext(ctx) - log.V(3).Info("reconciling dns records") - - // Reconcile DNSRecords for each gateway directly referred by the policy (existing and new) - for _, gw := range append(gwDiffObj.GatewaysWithValidPolicyRef, gwDiffObj.GatewaysMissingPolicyRef...) { - log.V(1).Info("reconcileDNSRecords: gateway with valid or missing policy ref", "key", gw.Key()) - if err := r.reconcileGatewayDNSRecords(ctx, gw.Gateway, dnsPolicy); err != nil { - return fmt.Errorf("reconciling dns records for gateway %v: error %w", gw.Gateway.Name, err) - } - } - return nil -} - -func (r *DNSPolicyReconciler) reconcileGatewayDNSRecords(ctx context.Context, gateway *gatewayapiv1.Gateway, dnsPolicy *v1alpha1.DNSPolicy) error { - log := crlog.FromContext(ctx) - clusterID, err := utils.GetClusterUID(ctx, r.Client()) - if err != nil { - return fmt.Errorf("failed to generate cluster ID: %w", err) - } - gw := gateway.DeepCopy() - gatewayWrapper := NewGatewayWrapper(gw) - // modify the status addresses based on any that need to be excluded - if err := gatewayWrapper.RemoveExcludedStatusAddresses(dnsPolicy); err != nil { - return fmt.Errorf("failed to reconcile gateway dns records error: %w ", err) - } - - if err = r.dnsHelper.removeDNSForDeletedListeners(ctx, gw); err != nil { - log.V(3).Info("error removing DNS for deleted listeners") - return err - } - - log.V(3).Info("checking gateway for attached routes ", "gateway", gw.Name) - var totalPolicyRecords int32 - var gatewayHasAttachedRoutes = false - - if len(gw.Status.Addresses) == 0 { - return ErrNoAddresses - } - - for _, listener := range gw.Spec.Listeners { - if listener.Hostname == nil || *listener.Hostname == "" { - log.Info("skipping listener no hostname assigned", "listener", listener.Name, "in ns ", gateway.Namespace) - continue - } - - hasAttachedRoute := false - for _, statusListener := range gateway.Status.Listeners { - if string(listener.Name) == string(statusListener.Name) { - hasAttachedRoute = statusListener.AttachedRoutes > 0 - } - } - - if hasAttachedRoute { - gatewayHasAttachedRoutes = true - } - if !hasAttachedRoute { - // delete record - log.V(1).Info("no cluster gateways, deleting DNS record", " for listener ", listener.Name) - if err := r.dnsHelper.deleteDNSRecordForListener(ctx, gw, listener); client.IgnoreNotFound(err) != nil { - return fmt.Errorf("failed to delete dns record for listener %s : %w", listener.Name, err) - } - continue - } - - dnsRecord, err := r.desiredDNSRecord(gw, clusterID, dnsPolicy, listener) - if err != nil { - return err - } - - err = r.SetOwnerReference(dnsPolicy, dnsRecord) - if err != nil { - return err - } - - if len(dnsRecord.Spec.Endpoints) == 0 { - log.V(1).Info("no endpoint addresses for DNSRecord ", "removing any records for listener", listener) - if err := r.dnsHelper.deleteDNSRecordForListener(ctx, gatewayWrapper, listener); client.IgnoreNotFound(err) != nil { - return err - } - //return fmt.Errorf("no valid addresses for DNSRecord endpoints. Check allowedAddresses") - continue - } - - err = r.ReconcileResource(ctx, &kuadrantdnsv1alpha1.DNSRecord{}, dnsRecord, dnsRecordBasicMutator) - if err != nil && !apierrors.IsAlreadyExists(err) { - log.Error(err, "ReconcileResource failed to create/update DNSRecord resource") - return err - } - totalPolicyRecords++ - } - dnsPolicy.Status.TotalRecords = totalPolicyRecords - if !gatewayHasAttachedRoutes { - return ErrNoRoutes - } - return nil -} - -func (r *DNSPolicyReconciler) desiredDNSRecord(gateway *gatewayapiv1.Gateway, clusterID string, dnsPolicy *v1alpha1.DNSPolicy, targetListener gatewayapiv1.Listener) (*kuadrantdnsv1alpha1.DNSRecord, error) { +func desiredDNSRecord(gateway *gatewayapiv1.Gateway, clusterID string, dnsPolicy *v1alpha1.DNSPolicy, targetListener gatewayapiv1.Listener) (*kuadrantdnsv1alpha1.DNSRecord, error) { rootHost := string(*targetListener.Hostname) var healthCheckSpec *kuadrantdnsv1alpha1.HealthCheckSpec @@ -141,7 +36,11 @@ func (r *DNSPolicyReconciler) desiredDNSRecord(gateway *gatewayapiv1.Gateway, cl ObjectMeta: metav1.ObjectMeta{ Name: dnsRecordName(gateway.Name, string(targetListener.Name)), Namespace: dnsPolicy.Namespace, - Labels: commonDNSRecordLabels(client.ObjectKeyFromObject(gateway), dnsPolicy), + Labels: CommonLabels(), + }, + TypeMeta: metav1.TypeMeta{ + Kind: DNSRecordKind, + APIVersion: kuadrantdnsv1alpha1.GroupVersion.String(), }, Spec: kuadrantdnsv1alpha1.DNSRecordSpec{ RootHost: rootHost, @@ -162,27 +61,14 @@ func (r *DNSPolicyReconciler) desiredDNSRecord(gateway *gatewayapiv1.Gateway, cl return dnsRecord, nil } -func dnsRecordBasicMutator(existingObj, desiredObj client.Object) (bool, error) { - existing, ok := existingObj.(*kuadrantdnsv1alpha1.DNSRecord) - if !ok { - return false, fmt.Errorf("%T is not an *kuadrantdnsv1alpha1.DNSRecord", existingObj) - } - desired, ok := desiredObj.(*kuadrantdnsv1alpha1.DNSRecord) - if !ok { - return false, fmt.Errorf("%T is not an *kuadrantdnsv1alpha1.DNSRecord", desiredObj) - } - - if reflect.DeepEqual(existing.Spec, desired.Spec) { - return false, nil - } - - existing.Spec = desired.Spec - - return true, nil -} - func buildEndpoints(clusterID, hostname string, gateway *gatewayapiv1.Gateway, policy *v1alpha1.DNSPolicy) ([]*externaldns.Endpoint, error) { - endpointBuilder := builder.NewEndpointsBuilder(NewGatewayWrapper(gateway), hostname) + gw := gateway.DeepCopy() + gatewayWrapper := NewGatewayWrapper(gw) + // modify the status addresses based on any that need to be excluded + if err := gatewayWrapper.RemoveExcludedStatusAddresses(policy); err != nil { + return nil, fmt.Errorf("failed to reconcile gateway dns records error: %w ", err) + } + endpointBuilder := builder.NewEndpointsBuilder(gatewayWrapper, hostname) if policy.Spec.LoadBalancing != nil { endpointBuilder.WithLoadBalancingFor( diff --git a/controllers/effective_dnspolicies_reconciler.go b/controllers/effective_dnspolicies_reconciler.go index 59c4cb612..9ca2ddec4 100644 --- a/controllers/effective_dnspolicies_reconciler.go +++ b/controllers/effective_dnspolicies_reconciler.go @@ -2,27 +2,36 @@ package controllers import ( "context" + "fmt" + "reflect" "sync" "github.com/samber/lo" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/dynamic" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" kuadrantdnsv1alpha1 "github.com/kuadrant/dns-operator/api/v1alpha1" "github.com/kuadrant/policy-machinery/controller" "github.com/kuadrant/policy-machinery/machinery" kuadrantv1alpha1 "github.com/kuadrant/kuadrant-operator/api/v1alpha1" + "github.com/kuadrant/kuadrant-operator/pkg/library/utils" ) -func NewEffectiveDNSPoliciesReconciler(client *dynamic.DynamicClient) *EffectiveDNSPoliciesReconciler { - return &EffectiveDNSPoliciesReconciler{client: client} +func NewEffectiveDNSPoliciesReconciler(client *dynamic.DynamicClient, scheme *runtime.Scheme) *EffectiveDNSPoliciesReconciler { + return &EffectiveDNSPoliciesReconciler{ + client: client, + scheme: scheme, + } } type EffectiveDNSPoliciesReconciler struct { client *dynamic.DynamicClient + scheme *runtime.Scheme } func (r *EffectiveDNSPoliciesReconciler) Subscription() controller.Subscription { @@ -36,14 +45,177 @@ func (r *EffectiveDNSPoliciesReconciler) Subscription() controller.Subscription } } -func (r *EffectiveDNSPoliciesReconciler) reconcile(ctx context.Context, _ []controller.ResourceEvent, topology *machinery.Topology, _ error, _ *sync.Map) error { - //ToDo Implement DNSRecord reconcile - return r.deleteOrphanDNSRecords(ctx, topology) +type dnsPolicyTypeFilterFunc func(item machinery.Policy, index int) (*kuadrantv1alpha1.DNSPolicy, bool) + +func (r *EffectiveDNSPoliciesReconciler) reconcile(ctx context.Context, _ []controller.ResourceEvent, topology *machinery.Topology, _ error, state *sync.Map) error { + logger := controller.LoggerFromContext(ctx).WithName("EffectiveDNSPoliciesReconciler") + + policyTypeFilterFunc := func(item machinery.Policy, index int) (*kuadrantv1alpha1.DNSPolicy, bool) { + p, ok := item.(*kuadrantv1alpha1.DNSPolicy) + return p, ok + } + + policies := lo.FilterMap(topology.Policies().Items(), policyTypeFilterFunc) + + policyAcceptedFunc := dnsPolicyAcceptedStatusFunc(state) + + logger.V(1).Info("updating dns policies", "policies", len(policies)) + + for _, policy := range policies { + pLogger := logger.WithValues("policy", policy.Name) + if policy.GetDeletionTimestamp() != nil { + pLogger.V(1).Info("policy marked for deletion, skipping") + continue + } + + if accepted, _ := policyAcceptedFunc(policy); !accepted { + pLogger.V(1).Info("policy not accepted, skipping") + continue + } + + listeners := r.listenersForPolicy(ctx, topology, policy, policyTypeFilterFunc) + + if logger.V(1).Enabled() { + listenerLocators := lo.Map(listeners, func(item *machinery.Listener, _ int) string { + return item.GetLocator() + }) + pLogger.V(1).Info("reconciling policy for gateway listeners", "listeners", listenerLocators) + } + + var gatewayHasAttachedRoutes = false + + clusterID, err := utils.GetClusterUID(ctx, r.client) + if err != nil { + return fmt.Errorf("failed to generate cluster ID: %w", err) + } + + for _, listener := range listeners { + lLogger := pLogger.WithValues("listener", listener.GetLocator()) + + gateway := listener.Gateway + if listener.Hostname == nil || *listener.Hostname == "" { + lLogger.Info("skipping listener no hostname assigned") + continue + } + + hasAttachedRoute := false + for _, statusListener := range gateway.Status.Listeners { + if string(listener.Name) == string(statusListener.Name) { + hasAttachedRoute = statusListener.AttachedRoutes > 0 + } + } + if hasAttachedRoute { + gatewayHasAttachedRoutes = true + } + + desiredRecord, err := desiredDNSRecord(gateway.Gateway, clusterID, policy, *listener.Listener) + if err != nil { + lLogger.Error(err, "failed to build desired dns record") + continue + } + if err = controllerutil.SetControllerReference(policy, desiredRecord, r.scheme); err != nil { + lLogger.Error(err, "failed to set owner reference on desired record") + continue + } + obj, err := controller.Destruct(desiredRecord) + if err != nil { + lLogger.Error(err, "unable to destruct dns record") // should never happen + continue + } + resource := r.client.Resource(DNSRecordResource).Namespace(desiredRecord.GetNamespace()) + + existingRecordObj, ok := lo.Find(topology.Objects().Children(listener), func(o machinery.Object) bool { + _, ok := o.(*controller.RuntimeObject).Object.(*kuadrantdnsv1alpha1.DNSRecord) + return ok && o.GetNamespace() == listener.GetNamespace() && o.GetName() == dnsRecordName(listener.Gateway.Name, string(listener.Name)) + }) + + //Update + if ok { + existingRecord := existingRecordObj.(*controller.RuntimeObject).Object.(*kuadrantdnsv1alpha1.DNSRecord) + + //Deal with the potential deletion of a record first + if !hasAttachedRoute || len(desiredRecord.Spec.Endpoints) == 0 { + if logger.V(1).Enabled() { + if !hasAttachedRoute { + lLogger.V(1).Info("listener has no attached routes, deleting DNS record for listener") + } else { + lLogger.V(1).Info("no endpoint addresses for DNSRecord, deleting DNS record for listener") + } + } + r.deleteRecord(ctx, existingRecordObj) + continue + } + + if desiredRecord.Spec.RootHost != existingRecord.Spec.RootHost { + lLogger.V(1).Info("listener hostname has changed, deleting DNS record for listener") + r.deleteRecord(ctx, existingRecordObj) + //Break to allow it to try the creation of the desired record + break + } + + if reflect.DeepEqual(existingRecord.Spec, desiredRecord.Spec) && + reflect.DeepEqual(existingRecord.OwnerReferences, desiredRecord.OwnerReferences) { + lLogger.V(1).Info("dns record is up to date, nothing to do") + continue + } + + lLogger.V(1).Info("updating DNS record for listener") + _, err = resource.Update(ctx, obj, metav1.UpdateOptions{}) + if err != nil { + lLogger.Error(err, "unable to update dns record") + } + continue + } + + if !hasAttachedRoute || len(desiredRecord.Spec.Endpoints) == 0 { + continue + } + + //Create + lLogger.V(1).Info("creating DNS record for listener") + _, err = resource.Create(ctx, obj, metav1.CreateOptions{}) + if err != nil && !apierrors.IsAlreadyExists(err) { + lLogger.Error(err, "unable to create dns record") + } + } + + if !gatewayHasAttachedRoutes { + pLogger.V(1).Info("gateway has no attached routes") + //return ErrNoRoutes + } + } + + return r.deleteOrphanDNSRecords(controller.LoggerIntoContext(ctx, logger), topology) +} + +// listenersForPolicy returns an array of listeners that are targeted by the given policy. +// If the target is a Listener a single element array containing that listener is returned. +// If the target is a Gateway all listeners that do not have a DNS policy explicitly attached are returned. +func (r *EffectiveDNSPoliciesReconciler) listenersForPolicy(_ context.Context, topology *machinery.Topology, policy machinery.Policy, policyTypeFilterFunc dnsPolicyTypeFilterFunc) []*machinery.Listener { + return lo.Flatten(lo.FilterMap(topology.Targetables().Items(), func(t machinery.Targetable, _ int) ([]*machinery.Listener, bool) { + pTarget := lo.ContainsBy(t.Policies(), func(item machinery.Policy) bool { + return item.GetLocator() == policy.GetLocator() + }) + if pTarget { + if l, ok := t.(*machinery.Listener); ok { + return []*machinery.Listener{l}, true + } + if g, ok := t.(*machinery.Gateway); ok { + listeners := lo.FilterMap(topology.Targetables().Children(g), func(t machinery.Targetable, _ int) (*machinery.Listener, bool) { + l, lok := t.(*machinery.Listener) + lPolicies := lo.FilterMap(l.Policies(), policyTypeFilterFunc) + return l, lok && len(lPolicies) == 0 + }) + return listeners, true + } + } + return nil, false + })) } // deleteOrphanDNSRecords deletes any DNSRecord resources that exist in the topology but have no parent targettable, policy or path back to the policy. func (r *EffectiveDNSPoliciesReconciler) deleteOrphanDNSRecords(ctx context.Context, topology *machinery.Topology) error { - logger := controller.LoggerFromContext(ctx).WithName("EffectiveDNSPoliciesReconciler") + logger := controller.LoggerFromContext(ctx).WithName("deleteOrphanDNSRecords") orphanRecords := lo.Filter(topology.Objects().Items(), func(item machinery.Object, _ int) bool { if item.GroupVersionKind().GroupKind() == DNSRecordGroupKind { @@ -81,16 +253,21 @@ func (r *EffectiveDNSPoliciesReconciler) deleteOrphanDNSRecords(ctx context.Cont }) for _, obj := range orphanRecords { - record := obj.(*controller.RuntimeObject).Object.(*kuadrantdnsv1alpha1.DNSRecord) - if record.GetDeletionTimestamp() != nil { - continue - } - logger.Info("deleting orphan dns record", "record", obj.GetLocator()) - resource := r.client.Resource(DNSRecordResource).Namespace(record.GetNamespace()) - if err := resource.Delete(ctx, record.GetName(), metav1.DeleteOptions{}); err != nil && !apierrors.IsNotFound(err) { - logger.Error(err, "failed to delete DNSRecord", "record", obj.GetLocator()) - } + r.deleteRecord(ctx, obj) } return nil } + +func (r *EffectiveDNSPoliciesReconciler) deleteRecord(ctx context.Context, obj machinery.Object) { + logger := controller.LoggerFromContext(ctx) + + record := obj.(*controller.RuntimeObject).Object.(*kuadrantdnsv1alpha1.DNSRecord) + if record.GetDeletionTimestamp() != nil { + return + } + resource := r.client.Resource(DNSRecordResource).Namespace(record.GetNamespace()) + if err := resource.Delete(ctx, record.GetName(), metav1.DeleteOptions{}); err != nil && !apierrors.IsNotFound(err) { + logger.Error(err, "failed to delete DNSRecord", "record", obj.GetLocator()) + } +} diff --git a/controllers/state_of_the_world.go b/controllers/state_of_the_world.go index 11111d1ed..5b07ae1db 100644 --- a/controllers/state_of_the_world.go +++ b/controllers/state_of_the_world.go @@ -324,7 +324,7 @@ func (b *BootOptionsBuilder) Reconciler() controller.ReconcileFunc { Tasks: []controller.ReconcileFunc{ NewAuthorinoReconciler(b.client).Subscription().Reconcile, NewLimitadorReconciler(b.client).Subscription().Reconcile, - NewDNSWorkflow(b.client).Run, + NewDNSWorkflow(b.client, b.manager.GetScheme()).Run, NewTLSWorkflow(b.client, b.manager.GetScheme(), b.isCertManagerInstalled).Run, NewAuthWorkflow().Run, NewRateLimitWorkflow().Run, diff --git a/controllers/test_common.go b/controllers/test_common.go index b6eb306d0..bcd460d43 100644 --- a/controllers/test_common.go +++ b/controllers/test_common.go @@ -99,19 +99,6 @@ func SetupKuadrantOperatorForTest(s *runtime.Scheme, cfg *rest.Config) { Expect(err).NotTo(HaveOccurred()) - dnsPolicyBaseReconciler := reconcilers.NewBaseReconciler( - mgr.GetClient(), mgr.GetScheme(), mgr.GetAPIReader(), - log.Log.WithName("dnspolicy"), - mgr.GetEventRecorderFor("DNSPolicy"), - ) - - err = (&DNSPolicyReconciler{ - BaseReconciler: dnsPolicyBaseReconciler, - TargetRefReconciler: reconcilers.TargetRefReconciler{Client: mgr.GetClient()}, - }).SetupWithManager(mgr) - - Expect(err).NotTo(HaveOccurred()) - kuadrantBaseReconciler := reconcilers.NewBaseReconciler( mgr.GetClient(), mgr.GetScheme(), mgr.GetAPIReader(), log.Log.WithName("kuadrant-controller"), diff --git a/main.go b/main.go index 3d06d59de..221cf11ee 100644 --- a/main.go +++ b/main.go @@ -200,20 +200,6 @@ func main() { os.Exit(1) } - dnsPolicyBaseReconciler := reconcilers.NewBaseReconciler( - mgr.GetClient(), mgr.GetScheme(), mgr.GetAPIReader(), - log.Log.WithName("dnspolicy"), - mgr.GetEventRecorderFor("DNSPolicy"), - ) - - if err = (&controllers.DNSPolicyReconciler{ - BaseReconciler: dnsPolicyBaseReconciler, - TargetRefReconciler: reconcilers.TargetRefReconciler{Client: mgr.GetClient()}, - }).SetupWithManager(mgr); err != nil { - setupLog.Error(err, "unable to create controller", "controller", "DNSPolicy") - os.Exit(1) - } - limitadorClusterEnvoyFilterBaseReconciler := reconcilers.NewBaseReconciler( mgr.GetClient(), mgr.GetScheme(), mgr.GetAPIReader(), log.Log.WithName("ratelimitpolicy").WithName("envoyfilter"), diff --git a/pkg/library/utils/k8s_utils.go b/pkg/library/utils/k8s_utils.go index 192aa8676..ea59b350b 100644 --- a/pkg/library/utils/k8s_utils.go +++ b/pkg/library/utils/k8s_utils.go @@ -26,6 +26,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/dynamic" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -158,17 +159,16 @@ func GetLabel(obj metav1.Object, key string) string { return obj.GetLabels()[key] } -func GetClusterUID(ctx context.Context, c client.Client) (string, error) { +func GetClusterUID(ctx context.Context, c *dynamic.DynamicClient) (string, error) { //Already calculated? return it if clusterUID != "" { return clusterUID, nil } - ns := &corev1.Namespace{} - err := c.Get(ctx, client.ObjectKey{Name: clusterIDNamespace}, ns) + un, err := c.Resource(corev1.SchemeGroupVersion.WithResource("namespaces")).Get(ctx, clusterIDNamespace, metav1.GetOptions{}) if err != nil { return "", err } - clusterUID = string(ns.UID) + clusterUID = string(un.GetUID()) return clusterUID, nil } diff --git a/tests/common/dnspolicy/dnspolicy_controller_single_cluster_test.go b/tests/common/dnspolicy/dnspolicy_controller_single_cluster_test.go index a724fa61b..d9e60e847 100644 --- a/tests/common/dnspolicy/dnspolicy_controller_single_cluster_test.go +++ b/tests/common/dnspolicy/dnspolicy_controller_single_cluster_test.go @@ -27,6 +27,16 @@ import ( "github.com/kuadrant/kuadrant-operator/tests" ) +func getClusterUID(ctx context.Context, c client.Client) (string, error) { + ns := &corev1.Namespace{} + err := c.Get(ctx, client.ObjectKey{Name: "kube-system"}, ns) + if err != nil { + return "", err + } + + return string(ns.UID), nil +} + var _ = Describe("DNSPolicy Single Cluster", func() { const ( testTimeOut = SpecTimeout(1 * time.Minute) @@ -45,7 +55,7 @@ var _ = Describe("DNSPolicy Single Cluster", func() { testNamespace = tests.CreateNamespace(ctx, testClient()) var err error - clusterUID, err := utils.GetClusterUID(ctx, k8sClient) + clusterUID, err := getClusterUID(ctx, k8sClient) Expect(err).To(BeNil()) gatewayClass = tests.BuildGatewayClass("gwc-"+testNamespace, "default", "kuadrant.io/bar") diff --git a/tests/common/dnspolicy/dnspolicy_controller_test.go b/tests/common/dnspolicy/dnspolicy_controller_test.go index 4e167d569..628df6444 100644 --- a/tests/common/dnspolicy/dnspolicy_controller_test.go +++ b/tests/common/dnspolicy/dnspolicy_controller_test.go @@ -3,7 +3,6 @@ package dnspolicy import ( - "encoding/json" "fmt" "time" @@ -21,7 +20,6 @@ import ( gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" "github.com/kuadrant/kuadrant-operator/api/v1alpha1" - "github.com/kuadrant/kuadrant-operator/controllers" "github.com/kuadrant/kuadrant-operator/pkg/library/kuadrant" "github.com/kuadrant/kuadrant-operator/tests" ) @@ -365,7 +363,6 @@ var _ = Describe("DNSPolicy controller", func() { }, testTimeOut) It("should have partially enforced policy if one of the records is not ready", func(ctx SpecContext) { - // setting up two gateways that have the same host gateway1 := tests.NewGatewayBuilder("test-gateway1", gatewayClass.Name, testNamespace). WithHTTPListener(tests.ListenerNameOne, tests.HostOne(domain)).Gateway @@ -425,6 +422,20 @@ var _ = Describe("DNSPolicy controller", func() { WithTargetGateway("test-gateway1") Expect(k8sClient.Create(ctx, dnsPolicy1)).To(Succeed()) + // policy1 should succeed + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(dnsPolicy1), dnsPolicy1)).To(Succeed()) + // check that policy is enforced with a correct message + g.Expect(dnsPolicy1.Status.Conditions).To( + ContainElements( + MatchFields(IgnoreExtras, Fields{ + "Type": Equal(string(kuadrant.PolicyConditionEnforced)), + "Status": Equal(metav1.ConditionTrue), + "Reason": Equal(string(kuadrant.PolicyReasonEnforced)), + "Message": Equal("DNSPolicy has been successfully enforced"), + }))) + }, tests.TimeoutLong, tests.RetryIntervalMedium).Should(Succeed()) + // create policy2 targeting gateway2 with the load-balanced strategy dnsPolicy2 := tests.NewDNSPolicy("test-dns-policy2", testNamespace). WithProviderSecret(*dnsProviderSecret). @@ -516,8 +527,6 @@ var _ = Describe("DNSPolicy controller", func() { }) Context("valid target and valid gateway status", func() { - var policiesBackRefValue, policyBackRefValue string - BeforeEach(func(ctx SpecContext) { gateway = tests.NewGatewayBuilder(tests.GatewayName, gatewayClass.Name, testNamespace). WithHTTPListener(tests.ListenerNameOne, tests.HostOne(domain)). @@ -561,10 +570,6 @@ var _ = Describe("DNSPolicy controller", func() { recordName = fmt.Sprintf("%s-%s", tests.GatewayName, tests.ListenerNameOne) wildcardRecordName = fmt.Sprintf("%s-%s", tests.GatewayName, tests.ListenerNameWildcard) - - policyBackRefValue = testNamespace + "/" + dnsPolicy.Name - refs, _ := json.Marshal([]client.ObjectKey{{Name: dnsPolicy.Name, Namespace: testNamespace}}) - policiesBackRefValue = string(refs) }) It("should create dns records and have correct policy status", func(ctx SpecContext) { @@ -584,7 +589,6 @@ var _ = Describe("DNSPolicy controller", func() { //Check policy status err = k8sClient.Get(ctx, client.ObjectKeyFromObject(dnsPolicy), dnsPolicy) g.Expect(err).NotTo(HaveOccurred()) - g.Expect(dnsPolicy.Finalizers).To(ContainElement(controllers.DNSPolicyFinalizer)) g.Expect(dnsPolicy.Status.Conditions).To( ContainElements( MatchFields(IgnoreExtras, Fields{ @@ -601,12 +605,6 @@ var _ = Describe("DNSPolicy controller", func() { })), ) g.Expect(dnsPolicy.Status.TotalRecords).To(Equal(int32(2))) - - //Check gateway back reference" - err = k8sClient.Get(ctx, client.ObjectKeyFromObject(gateway), gateway) - g.Expect(err).NotTo(HaveOccurred()) - g.Expect(gateway.Annotations).To(HaveKeyWithValue(v1alpha1.DNSPolicyDirectReferenceAnnotationName, policyBackRefValue)) - g.Expect(gateway.Annotations).To(HaveKeyWithValue(v1alpha1.DNSPolicyBackReferenceAnnotationName, policiesBackRefValue)) }, tests.TimeoutLong, tests.RetryIntervalMedium, ctx).Should(Succeed()) }, testTimeOut) @@ -664,12 +662,9 @@ var _ = Describe("DNSPolicy controller", func() { err := k8sClient.Get(ctx, client.ObjectKeyFromObject(gateway), gateway) g.Expect(err).NotTo(HaveOccurred()) - g.Expect(gateway.Annotations).To(HaveKeyWithValue(v1alpha1.DNSPolicyDirectReferenceAnnotationName, policyBackRefValue)) - g.Expect(gateway.Annotations).To(HaveKeyWithValue(v1alpha1.DNSPolicyBackReferenceAnnotationName, policiesBackRefValue)) err = k8sClient.Get(ctx, client.ObjectKeyFromObject(dnsPolicy), dnsPolicy) g.Expect(err).NotTo(HaveOccurred()) - g.Expect(dnsPolicy.Finalizers).To(ContainElement(controllers.DNSPolicyFinalizer)) }, tests.TimeoutMedium, time.Second).Should(Succeed()) By("deleting the dns policy") @@ -681,8 +676,6 @@ var _ = Describe("DNSPolicy controller", func() { err := k8sClient.Get(ctx, client.ObjectKeyFromObject(gateway), gateway) g.Expect(err).NotTo(HaveOccurred()) - g.Expect(gateway.Annotations).ToNot(HaveKey(v1alpha1.DNSPolicyDirectReferenceAnnotationName)) - g.Expect(gateway.Annotations).ToNot(HaveKeyWithValue(v1alpha1.DNSPolicyBackReferenceAnnotationName, policiesBackRefValue)) }, tests.TimeoutMedium, time.Second).Should(Succeed()) }, testTimeOut) @@ -723,8 +716,6 @@ var _ = Describe("DNSPolicy controller", func() { err := k8sClient.Get(ctx, client.ObjectKeyFromObject(gateway), gateway) g.Expect(err).NotTo(HaveOccurred()) - g.Expect(gateway.Annotations).To(HaveKeyWithValue(v1alpha1.DNSPolicyDirectReferenceAnnotationName, policyBackRefValue)) - g.Expect(gateway.Annotations).To(HaveKeyWithValue(v1alpha1.DNSPolicyBackReferenceAnnotationName, policiesBackRefValue)) }, tests.TimeoutMedium, tests.RetryIntervalMedium).Should(Succeed()) By("changing the policy target ref") @@ -773,8 +764,6 @@ var _ = Describe("DNSPolicy controller", func() { err := k8sClient.Get(ctx, client.ObjectKeyFromObject(gateway), gateway) g.Expect(err).NotTo(HaveOccurred()) - g.Expect(gateway.Annotations).To(HaveKeyWithValue(v1alpha1.DNSPolicyDirectReferenceAnnotationName, policyBackRefValue)) - g.Expect(gateway.Annotations).To(HaveKeyWithValue(v1alpha1.DNSPolicyBackReferenceAnnotationName, policiesBackRefValue)) }, tests.TimeoutMedium, tests.RetryIntervalMedium).Should(Succeed()) testGateway2Name := "test-gateway-2" @@ -822,20 +811,8 @@ var _ = Describe("DNSPolicy controller", func() { g.Expect(k8sClient.Get(ctx, client.ObjectKey{Name: recordName, Namespace: testNamespace}, &kuadrantdnsv1alpha1.DNSRecord{})).Should(MatchError(ContainSubstring("not found"))) g.Expect(k8sClient.Get(ctx, client.ObjectKey{Name: wildcardRecordName, Namespace: testNamespace}, &kuadrantdnsv1alpha1.DNSRecord{})).Should(MatchError(ContainSubstring("not found"))) - //Old gateway target has gateway back references removed - err := k8sClient.Get(ctx, client.ObjectKeyFromObject(gateway), gateway) - g.Expect(err).NotTo(HaveOccurred()) - g.Expect(gateway.Annotations).ToNot(HaveKey(v1alpha1.DNSPolicyDirectReferenceAnnotationName)) - g.Expect(gateway.Annotations).ToNot(HaveKeyWithValue(v1alpha1.DNSPolicyBackReferenceAnnotationName, policiesBackRefValue)) - - //New gateway target has gateway back references added - err = k8sClient.Get(ctx, client.ObjectKeyFromObject(gateway2), gateway2) - g.Expect(err).NotTo(HaveOccurred()) - g.Expect(gateway2.Annotations).To(HaveKeyWithValue(v1alpha1.DNSPolicyDirectReferenceAnnotationName, policyBackRefValue)) - g.Expect(gateway2.Annotations).To(HaveKeyWithValue(v1alpha1.DNSPolicyBackReferenceAnnotationName, policiesBackRefValue)) - //Check policy status - err = k8sClient.Get(ctx, client.ObjectKeyFromObject(dnsPolicy), dnsPolicy) + err := k8sClient.Get(ctx, client.ObjectKeyFromObject(dnsPolicy), dnsPolicy) g.Expect(err).NotTo(HaveOccurred()) g.Expect(dnsPolicy.Status.Conditions).To( ContainElements(