diff --git a/api/v1alpha1/dnspolicy_types.go b/api/v1alpha1/dnspolicy_types.go index 2ca52fd3f..bac852131 100644 --- a/api/v1alpha1/dnspolicy_types.go +++ b/api/v1alpha1/dnspolicy_types.go @@ -18,6 +18,9 @@ package v1alpha1 import ( "context" + "fmt" + "net" + "strings" dnsv1alpha1 "github.com/kuadrant/dns-operator/api/v1alpha1" corev1 "k8s.io/api/core/v1" @@ -70,8 +73,23 @@ type DNSPolicySpec struct { // ExcludeAddresses is a list of addresses (either hostnames, CIDR or IPAddresses) that DNSPolicy should not use as values in the configured DNS provider records. The default is to allow all addresses configured in the Gateway DNSPolicy is targeting // +optional - // +kubebuilder:validation:MaxItems=20 - ExcludeAddresses []string `json:"excludeAddresses,omitempty"` + ExcludeAddresses ExcludeAddresses `json:"excludeAddresses,omitempty"` +} + +// +kubebuilder:validation:MaxItems=20 +type ExcludeAddresses []string + +func (ea ExcludeAddresses) Validate() error { + for _, exclude := range ea { + //Only a CIDR will have / in the address so attempt to parse fail if not valid + if strings.Contains(exclude, "/") { + _, _, err := net.ParseCIDR(exclude) + if err != nil { + return fmt.Errorf("could not parse the CIDR from the excludeAddresses field %w", err) + } + } + } + return nil } type LoadBalancingSpec struct { @@ -159,6 +177,10 @@ type DNSPolicy struct { Status DNSPolicyStatus `json:"status,omitempty"` } +func (p *DNSPolicy) Validate() error { + return p.Spec.ExcludeAddresses.Validate() +} + func (p *DNSPolicy) GetWrappedNamespace() gatewayapiv1.Namespace { return gatewayapiv1.Namespace(p.Namespace) } diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 53a868b6c..1ca43e956 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -148,7 +148,7 @@ func (in *DNSPolicySpec) DeepCopyInto(out *DNSPolicySpec) { } if in.ExcludeAddresses != nil { in, out := &in.ExcludeAddresses, &out.ExcludeAddresses - *out = make([]string, len(*in)) + *out = make(ExcludeAddresses, len(*in)) copy(*out, *in) } } @@ -208,6 +208,25 @@ func (in *DNSPolicyStatus) DeepCopy() *DNSPolicyStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in ExcludeAddresses) DeepCopyInto(out *ExcludeAddresses) { + { + in := &in + *out = make(ExcludeAddresses, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ExcludeAddresses. +func (in ExcludeAddresses) DeepCopy() ExcludeAddresses { + if in == nil { + return nil + } + out := new(ExcludeAddresses) + in.DeepCopyInto(out) + return *out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *LoadBalancingSpec) DeepCopyInto(out *LoadBalancingSpec) { *out = *in diff --git a/controllers/common.go b/controllers/common.go new file mode 100644 index 000000000..a11fb358d --- /dev/null +++ b/controllers/common.go @@ -0,0 +1,21 @@ +package controllers + +const ( + KuadrantAppName = "kuadrant" +) + +var ( + AppLabelKey = "app" + AppLabelValue = KuadrantAppName +) + +func CommonLabels() map[string]string { + return map[string]string{ + AppLabelKey: AppLabelValue, + "app.kubernetes.io/component": KuadrantAppName, + "app.kubernetes.io/managed-by": "kuadrant-operator", + "app.kubernetes.io/instance": KuadrantAppName, + "app.kubernetes.io/name": KuadrantAppName, + "app.kubernetes.io/part-of": KuadrantAppName, + } +} diff --git a/controllers/dns_helper.go b/controllers/dns_helper.go index 152194945..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 := map[string]string{} - 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 99787a34c..055dfb350 100644 --- a/controllers/dns_workflow.go +++ b/controllers/dns_workflow.go @@ -1,7 +1,141 @@ package controllers -import "github.com/kuadrant/policy-machinery/controller" +import ( + "fmt" + "sync" -func NewDNSWorkflow() *controller.Workflow { - return &controller.Workflow{} + "github.com/samber/lo" + + "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" + gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" + + "github.com/kuadrant/policy-machinery/controller" + "github.com/kuadrant/policy-machinery/machinery" + + kuadrantdnsv1alpha1 "github.com/kuadrant/dns-operator/api/v1alpha1" + + "github.com/kuadrant/kuadrant-operator/api/v1alpha1" + "github.com/kuadrant/kuadrant-operator/pkg/library/utils" +) + +const ( + DNSRecordKind = "DNSRecord" + StateDNSPolicyAcceptedKey = "DNSPolicyValid" + StateDNSPolicyErrorsKey = "DNSPolicyErrors" +) + +var ( + DNSRecordResource = kuadrantdnsv1alpha1.GroupVersion.WithResource("dnsrecords") + DNSRecordGroupKind = schema.GroupKind{Group: kuadrantdnsv1alpha1.GroupVersion.Group, Kind: DNSRecordKind} +) + +//+kubebuilder:rbac:groups=core,resources=namespaces,verbs=get;list;watch +//+kubebuilder:rbac:groups=kuadrant.io,resources=dnspolicies,verbs=get;list;watch;update;patch;delete +//+kubebuilder:rbac:groups=kuadrant.io,resources=dnspolicies/status,verbs=get;update;patch +//+kubebuilder:rbac:groups=kuadrant.io,resources=dnspolicies/finalizers,verbs=update + +//+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, scheme *runtime.Scheme) *controller.Workflow { + return &controller.Workflow{ + Precondition: NewDNSPoliciesValidator().Subscription().Reconcile, + Tasks: []controller.ReconcileFunc{ + NewEffectiveDNSPoliciesReconciler(client, scheme).Subscription().Reconcile, + }, + Postcondition: NewDNSPolicyStatusUpdater(client).Subscription().Reconcile, + } +} + +func LinkListenerToDNSRecord(objs controller.Store) machinery.LinkFunc { + gateways := lo.Map(objs.FilterByGroupKind(machinery.GatewayGroupKind), controller.ObjectAs[*gwapiv1.Gateway]) + listeners := lo.FlatMap(lo.Map(gateways, func(g *gwapiv1.Gateway, _ int) *machinery.Gateway { + return &machinery.Gateway{Gateway: g} + }), machinery.ListenersFromGatewayFunc) + + return machinery.LinkFunc{ + From: machinery.ListenerGroupKind, + To: DNSRecordGroupKind, + Func: func(child machinery.Object) []machinery.Object { + return lo.FilterMap(listeners, func(l *machinery.Listener, _ int) (machinery.Object, bool) { + if dnsRecord, ok := child.(*controller.RuntimeObject).Object.(*kuadrantdnsv1alpha1.DNSRecord); ok { + return l, l.GetNamespace() == dnsRecord.GetNamespace() && + dnsRecord.GetName() == dnsRecordName(l.Gateway.Name, string(l.Name)) + } + return nil, false + }) + }, + } +} + +func LinkDNSPolicyToDNSRecord(objs controller.Store) machinery.LinkFunc { + policies := lo.Map(objs.FilterByGroupKind(v1alpha1.DNSPolicyGroupKind), controller.ObjectAs[*v1alpha1.DNSPolicy]) + + return machinery.LinkFunc{ + From: v1alpha1.DNSPolicyGroupKind, + To: DNSRecordGroupKind, + Func: func(child machinery.Object) []machinery.Object { + if dnsRecord, ok := child.(*controller.RuntimeObject).Object.(*kuadrantdnsv1alpha1.DNSRecord); ok { + return lo.FilterMap(policies, func(dnsPolicy *v1alpha1.DNSPolicy, _ int) (machinery.Object, bool) { + return dnsPolicy, utils.IsOwnedBy(dnsRecord, dnsPolicy) + }) + } + return nil + }, + } +} + +func dnsPolicyAcceptedStatusFunc(state *sync.Map) func(policy machinery.Policy) (bool, error) { + validatedPolicies, validated := state.Load(StateDNSPolicyAcceptedKey) + if !validated { + return dnsPolicyAcceptedStatus + } + validatedPoliciesMap := validatedPolicies.(map[string]error) + return func(policy machinery.Policy) (bool, error) { + err, pValidated := validatedPoliciesMap[policy.GetLocator()] + if pValidated { + return err == nil, err + } + return dnsPolicyAcceptedStatus(policy) + } +} + +func dnsPolicyAcceptedStatus(policy machinery.Policy) (accepted bool, err error) { + p, ok := policy.(*v1alpha1.DNSPolicy) + if !ok { + return + } + if condition := meta.FindStatusCondition(p.Status.Conditions, string(gatewayapiv1alpha2.PolicyConditionAccepted)); condition != nil { + accepted = condition.Status == metav1.ConditionTrue + if !accepted { + err = fmt.Errorf(condition.Message) + } + return + } + return +} + +func dnsPolicyErrorFunc(state *sync.Map) func(policy machinery.Policy) error { + var policyErrorsMap map[string]error + policyErrors, exists := state.Load(StateDNSPolicyErrorsKey) + if exists { + policyErrorsMap = policyErrors.(map[string]error) + } + return func(policy machinery.Policy) error { + return policyErrorsMap[policy.GetLocator()] + } +} + +type dnsPolicyTypeFilter func(item machinery.Policy, index int) (*v1alpha1.DNSPolicy, bool) + +func dnsPolicyTypeFilterFunc() func(item machinery.Policy, _ int) (*v1alpha1.DNSPolicy, bool) { + return func(item machinery.Policy, _ int) (*v1alpha1.DNSPolicy, bool) { + p, ok := item.(*v1alpha1.DNSPolicy) + return p, ok + } } diff --git a/controllers/dnspolicies_validator.go b/controllers/dnspolicies_validator.go new file mode 100644 index 000000000..cbe77d42d --- /dev/null +++ b/controllers/dnspolicies_validator.go @@ -0,0 +1,52 @@ +package controllers + +import ( + "context" + "sync" + + "github.com/samber/lo" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + + "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/kuadrant" +) + +func NewDNSPoliciesValidator() *DNSPoliciesValidator { + return &DNSPoliciesValidator{} +} + +type DNSPoliciesValidator struct{} + +func (r *DNSPoliciesValidator) Subscription() controller.Subscription { + return controller.Subscription{ + ReconcileFunc: r.validate, + Events: []controller.ResourceEventMatcher{ + {Kind: &machinery.GatewayGroupKind}, + {Kind: &kuadrantv1alpha1.DNSPolicyGroupKind}, + }, + } +} + +func (r *DNSPoliciesValidator) validate(ctx context.Context, _ []controller.ResourceEvent, topology *machinery.Topology, _ error, state *sync.Map) error { + logger := controller.LoggerFromContext(ctx).WithName("DNSPoliciesValidator") + + policies := lo.FilterMap(topology.Policies().Items(), dnsPolicyTypeFilterFunc()) + + logger.V(1).Info("validating dns policies", "policies", len(policies)) + + state.Store(StateDNSPolicyAcceptedKey, lo.SliceToMap(policies, func(policy *kuadrantv1alpha1.DNSPolicy) (string, error) { + if len(policy.GetTargetRefs()) == 0 || len(topology.Targetables().Children(policy)) == 0 { + return policy.GetLocator(), kuadrant.NewErrTargetNotFound(policy.Kind(), policy.GetTargetRef(), + apierrors.NewNotFound(kuadrantv1alpha1.DNSPoliciesResource.GroupResource(), policy.GetName())) + } + return policy.GetLocator(), policy.Validate() + })) + + logger.V(1).Info("finished validating dns policies") + + return nil +} diff --git a/controllers/dnspolicy_controller.go b/controllers/dnspolicy_controller.go index 749c6efc6..fcb0c85c3 100644 --- a/controllers/dnspolicy_controller.go +++ b/controllers/dnspolicy_controller.go @@ -17,184 +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 -} - -//+kubebuilder:rbac:groups=core,resources=namespaces,verbs=get;list;watch -//+kubebuilder:rbac:groups=kuadrant.io,resources=dnspolicies,verbs=get;list;watch;update;patch;delete -//+kubebuilder:rbac:groups=kuadrant.io,resources=dnspolicies/status,verbs=get;update;patch -//+kubebuilder:rbac:groups=kuadrant.io,resources=dnspolicies/finalizers,verbs=update - -//+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 (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 r.reconcileStatus(ctx, dnsPolicy, 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) - - statusResult, statusErr := r.reconcileStatus(ctx, dnsPolicy, specErr) - - if specErr != nil { - return ctrl.Result{}, specErr - } - - return statusResult, statusErr -} - -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 { - // delete based on gateway diffs - if err := r.deleteDNSRecords(ctx, dnsPolicy); err != nil { - return err - } - - // 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 6c542505a..0daca897f 100644 --- a/controllers/dnspolicy_dnsrecords.go +++ b/controllers/dnspolicy_dnsrecords.go @@ -1,15 +1,9 @@ package controllers import ( - "context" "fmt" - "reflect" - apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/labels" - "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" @@ -17,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 ( @@ -26,112 +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") - for _, gw := range gwDiffObj.GatewaysWithInvalidPolicyRef { - log.V(1).Info("reconcileDNSRecords: gateway with invalid policy ref", "key", gw.Key()) - if err := r.deleteGatewayDNSRecords(ctx, gw.Gateway, dnsPolicy); err != nil { - return fmt.Errorf("error deleting dns records for gw %v: %w", gw.Gateway.Name, err) - } - } - - // 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 @@ -149,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, @@ -170,53 +61,14 @@ func (r *DNSPolicyReconciler) desiredDNSRecord(gateway *gatewayapiv1.Gateway, cl return dnsRecord, nil } -func (r *DNSPolicyReconciler) deleteGatewayDNSRecords(ctx context.Context, gateway *gatewayapiv1.Gateway, dnsPolicy *v1alpha1.DNSPolicy) error { - return r.deleteDNSRecordsWithLabels(ctx, commonDNSRecordLabels(client.ObjectKeyFromObject(gateway), dnsPolicy), dnsPolicy.Namespace) -} - -func (r *DNSPolicyReconciler) deleteDNSRecords(ctx context.Context, dnsPolicy *v1alpha1.DNSPolicy) error { - return r.deleteDNSRecordsWithLabels(ctx, policyDNSRecordLabels(dnsPolicy), dnsPolicy.Namespace) -} - -func (r *DNSPolicyReconciler) deleteDNSRecordsWithLabels(ctx context.Context, lbls map[string]string, namespace string) error { - log := crlog.FromContext(ctx) - - listOptions := &client.ListOptions{LabelSelector: labels.SelectorFromSet(lbls), Namespace: namespace} - recordsList := &kuadrantdnsv1alpha1.DNSRecordList{} - if err := r.Client().List(ctx, recordsList, listOptions); err != nil { - return err - } - - for i := range recordsList.Items { - if err := r.DeleteResource(ctx, &recordsList.Items[i]); client.IgnoreNotFound(err) != nil { - log.Error(err, "failed to delete DNSRecord") - return err - } - } - return 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/dnspolicy_status.go b/controllers/dnspolicy_status.go index 6c1147232..ac42f8605 100644 --- a/controllers/dnspolicy_status.go +++ b/controllers/dnspolicy_status.go @@ -17,20 +17,13 @@ limitations under the License. package controllers import ( - "context" "errors" "fmt" "slices" "strings" - "k8s.io/apimachinery/pkg/api/equality" - apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/reconcile" - "sigs.k8s.io/gateway-api/apis/v1alpha2" kuadrantdnsv1alpha1 "github.com/kuadrant/dns-operator/api/v1alpha1" @@ -41,37 +34,7 @@ import ( var NegativePolarityConditions []string -func (r *DNSPolicyReconciler) reconcileStatus(ctx context.Context, dnsPolicy *v1alpha1.DNSPolicy, specErr error) (ctrl.Result, error) { - newStatus := r.calculateStatus(ctx, dnsPolicy, specErr) - - equalStatus := equality.Semantic.DeepEqual(newStatus, dnsPolicy.Status) - if equalStatus && dnsPolicy.Generation == dnsPolicy.Status.ObservedGeneration { - return reconcile.Result{}, nil - } - - newStatus.ObservedGeneration = dnsPolicy.Generation - - dnsPolicy.Status = *newStatus - updateErr := r.Client().Status().Update(ctx, dnsPolicy) - if updateErr != nil { - // Ignore conflicts, resource might just be outdated. - if apierrors.IsConflict(updateErr) { - return ctrl.Result{Requeue: true}, nil - } - return ctrl.Result{}, updateErr - } - - // policy updated in API, emit metrics based on status conditions - r.emitConditionMetrics(dnsPolicy) - - if kuadrant.IsTargetNotFound(specErr) { - return ctrl.Result{Requeue: true}, nil - } - - return ctrl.Result{}, nil -} - -func (r *DNSPolicyReconciler) emitConditionMetrics(dnsPolicy *v1alpha1.DNSPolicy) { +func emitConditionMetrics(dnsPolicy *v1alpha1.DNSPolicy) { readyStatus := meta.FindStatusCondition(dnsPolicy.Status.Conditions, ReadyConditionType) if readyStatus == nil { dnsPolicyReady.WithLabelValues(dnsPolicy.Name, dnsPolicy.Namespace, "true").Set(0) @@ -88,81 +51,21 @@ func (r *DNSPolicyReconciler) emitConditionMetrics(dnsPolicy *v1alpha1.DNSPolicy } } -func (r *DNSPolicyReconciler) calculateStatus(ctx context.Context, dnsPolicy *v1alpha1.DNSPolicy, specErr error) *v1alpha1.DNSPolicyStatus { - newStatus := &v1alpha1.DNSPolicyStatus{ - // Copy initial conditions. Otherwise, status will always be updated - Conditions: slices.Clone(dnsPolicy.Status.Conditions), - ObservedGeneration: dnsPolicy.Status.ObservedGeneration, - TotalRecords: dnsPolicy.Status.TotalRecords, - } - acceptedCond := kuadrant.AcceptedCondition(dnsPolicy, nil) - if !(errors.Is(specErr, ErrNoAddresses) || errors.Is(specErr, ErrNoRoutes)) { - acceptedCond = kuadrant.AcceptedCondition(dnsPolicy, specErr) - } - - meta.SetStatusCondition(&newStatus.Conditions, *acceptedCond) - - // Do not set enforced condition if Accepted condition is false - if meta.IsStatusConditionFalse(newStatus.Conditions, string(v1alpha2.PolicyConditionAccepted)) { - meta.RemoveStatusCondition(&newStatus.Conditions, string(kuadrant.PolicyConditionEnforced)) - return newStatus - } - var enforcedCondition = kuadrant.EnforcedCondition(dnsPolicy, nil, true) - recordList, err := r.filteredRecordList(ctx, dnsPolicy) - if err != nil { - enforcedCondition = kuadrant.EnforcedCondition(dnsPolicy, kuadrant.NewErrUnknown("DNSPolicy", err), false) - meta.SetStatusCondition(&newStatus.Conditions, *enforcedCondition) - return newStatus - } - - enforcedCondition = r.enforcedCondition(recordList, dnsPolicy) - - // add some additional user friendly context - if errors.Is(specErr, ErrNoAddresses) && !strings.Contains(enforcedCondition.Message, ErrNoAddresses.Error()) { - enforcedCondition.Message = fmt.Sprintf("%s : %s", enforcedCondition.Message, ErrNoAddresses.Error()) - } - if errors.Is(specErr, ErrNoRoutes) && !strings.Contains(enforcedCondition.Message, ErrNoRoutes.Error()) { - enforcedCondition.Message = fmt.Sprintf("%s : %s", enforcedCondition.Message, ErrNoRoutes) - } - - meta.SetStatusCondition(&newStatus.Conditions, *enforcedCondition) - propagateRecordConditions(recordList, newStatus) - - return newStatus -} - -func (r *DNSPolicyReconciler) filteredRecordList(ctx context.Context, dnsPolicy *v1alpha1.DNSPolicy) (*kuadrantdnsv1alpha1.DNSRecordList, error) { - recordsList := &kuadrantdnsv1alpha1.DNSRecordList{} - if err := r.Client().List(ctx, recordsList, &client.ListOptions{Namespace: dnsPolicy.Namespace}); err != nil { - return nil, err - } - // filter down to records controlled by the policy - recordsList.Items = utils.Filter(recordsList.Items, func(record kuadrantdnsv1alpha1.DNSRecord) bool { - for _, reference := range record.GetOwnerReferences() { - if reference.Controller != nil && *reference.Controller && reference.Name == dnsPolicy.Name && reference.UID == dnsPolicy.UID { - return true - } - } - return false - }) - return recordsList, nil -} - -func (r *DNSPolicyReconciler) enforcedCondition(recordsList *kuadrantdnsv1alpha1.DNSRecordList, dnsPolicy *v1alpha1.DNSPolicy) *metav1.Condition { +func enforcedCondition(records []*kuadrantdnsv1alpha1.DNSRecord, dnsPolicy *v1alpha1.DNSPolicy) *metav1.Condition { // there are no controlled DNS records present - if len(recordsList.Items) == 0 { + if len(records) == 0 { cond := kuadrant.EnforcedCondition(dnsPolicy, nil, true) cond.Message = "DNSPolicy has been successfully enforced : no DNSRecords created based on policy and gateway configuration" return cond } // filter not ready records - notReadyRecords := utils.Filter(recordsList.Items, func(record kuadrantdnsv1alpha1.DNSRecord) bool { + notReadyRecords := utils.Filter(records, func(record *kuadrantdnsv1alpha1.DNSRecord) bool { return meta.IsStatusConditionFalse(record.Status.Conditions, string(kuadrantdnsv1alpha1.ConditionTypeReady)) }) // if there are records and none of the records are ready - if len(recordsList.Items) > 0 && len(notReadyRecords) == len(recordsList.Items) { + if len(records) > 0 && len(notReadyRecords) == len(records) { return kuadrant.EnforcedCondition(dnsPolicy, kuadrant.NewErrUnknown(dnsPolicy.Kind(), errors.New("policy is not enforced on any DNSRecord: not a single DNSRecord is ready")), false) } @@ -180,11 +83,11 @@ func (r *DNSPolicyReconciler) enforcedCondition(recordsList *kuadrantdnsv1alpha1 return kuadrant.EnforcedCondition(dnsPolicy, nil, true) } -func propagateRecordConditions(records *kuadrantdnsv1alpha1.DNSRecordList, policyStatus *v1alpha1.DNSPolicyStatus) { +func propagateRecordConditions(records []*kuadrantdnsv1alpha1.DNSRecord, policyStatus *v1alpha1.DNSPolicyStatus) { //reset conditions policyStatus.RecordConditions = map[string][]metav1.Condition{} - for _, record := range records.Items { + for _, record := range records { var allConditions []metav1.Condition allConditions = append(allConditions, record.Status.Conditions...) if record.Status.HealthCheck != nil { diff --git a/controllers/dnspolicy_status_test.go b/controllers/dnspolicy_status_test.go index bdc9d59cc..b8de6bcdf 100644 --- a/controllers/dnspolicy_status_test.go +++ b/controllers/dnspolicy_status_test.go @@ -3,17 +3,14 @@ package controllers import ( - "context" - "errors" "reflect" "testing" - kuadrantdnsv1alpha1 "github.com/kuadrant/dns-operator/api/v1alpha1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" + + kuadrantdnsv1alpha1 "github.com/kuadrant/dns-operator/api/v1alpha1" "github.com/kuadrant/kuadrant-operator/api/v1alpha1" - "github.com/kuadrant/kuadrant-operator/pkg/library/kuadrant" ) func TestPropagateRecordConditions(t *testing.T) { @@ -67,28 +64,26 @@ func TestPropagateRecordConditions(t *testing.T) { tests := []struct { Name string PolicyStatus *v1alpha1.DNSPolicyStatus - Records *kuadrantdnsv1alpha1.DNSRecordList + Records []*kuadrantdnsv1alpha1.DNSRecord Validate func(*testing.T, *v1alpha1.DNSPolicyStatus) }{ { Name: "Healthy conditions not propagated", - Records: &kuadrantdnsv1alpha1.DNSRecordList{ - Items: []kuadrantdnsv1alpha1.DNSRecord{ - { - Spec: kuadrantdnsv1alpha1.DNSRecordSpec{RootHost: rootHost}, - Status: kuadrantdnsv1alpha1.DNSRecordStatus{ + Records: []*kuadrantdnsv1alpha1.DNSRecord{ + { + Spec: kuadrantdnsv1alpha1.DNSRecordSpec{RootHost: rootHost}, + Status: kuadrantdnsv1alpha1.DNSRecordStatus{ + Conditions: []metav1.Condition{ + healthyProviderCondition, + }, + HealthCheck: &kuadrantdnsv1alpha1.HealthCheckStatus{ Conditions: []metav1.Condition{ - healthyProviderCondition, + healthyProbesCondition, }, - HealthCheck: &kuadrantdnsv1alpha1.HealthCheckStatus{ - Conditions: []metav1.Condition{ - healthyProbesCondition, - }, - Probes: []kuadrantdnsv1alpha1.HealthCheckStatusProbe{ - { - Conditions: []metav1.Condition{ - healthyProbeCondition, - }, + Probes: []kuadrantdnsv1alpha1.HealthCheckStatusProbe{ + { + Conditions: []metav1.Condition{ + healthyProbeCondition, }, }, }, @@ -105,23 +100,21 @@ func TestPropagateRecordConditions(t *testing.T) { }, { Name: "Unhealthy conditions are propagated", - Records: &kuadrantdnsv1alpha1.DNSRecordList{ - Items: []kuadrantdnsv1alpha1.DNSRecord{ - { - Spec: kuadrantdnsv1alpha1.DNSRecordSpec{RootHost: rootHost}, - Status: kuadrantdnsv1alpha1.DNSRecordStatus{ + Records: []*kuadrantdnsv1alpha1.DNSRecord{ + { + Spec: kuadrantdnsv1alpha1.DNSRecordSpec{RootHost: rootHost}, + Status: kuadrantdnsv1alpha1.DNSRecordStatus{ + Conditions: []metav1.Condition{ + healthyProviderCondition, + }, + HealthCheck: &kuadrantdnsv1alpha1.HealthCheckStatus{ Conditions: []metav1.Condition{ - healthyProviderCondition, + unhealthyProbesCondition, }, - HealthCheck: &kuadrantdnsv1alpha1.HealthCheckStatus{ - Conditions: []metav1.Condition{ - unhealthyProbesCondition, - }, - Probes: []kuadrantdnsv1alpha1.HealthCheckStatusProbe{ - { - Conditions: []metav1.Condition{ - unhealthyProbeCondition, - }, + Probes: []kuadrantdnsv1alpha1.HealthCheckStatusProbe{ + { + Conditions: []metav1.Condition{ + unhealthyProbeCondition, }, }, }, @@ -153,59 +146,3 @@ func TestPropagateRecordConditions(t *testing.T) { }) } } - -func TestDNSPolicyReconciler_calculateStatus(t *testing.T) { - type args struct { - ctx context.Context - dnsPolicy *v1alpha1.DNSPolicy - specErr error - } - tests := []struct { - name string - args args - want *v1alpha1.DNSPolicyStatus - }{ - { - name: "Enforced status block removed if policy not Accepted. (Regression test)", // https://github.com/Kuadrant/kuadrant-operator/issues/588 - args: args{ - dnsPolicy: &v1alpha1.DNSPolicy{ - Status: v1alpha1.DNSPolicyStatus{ - Conditions: []metav1.Condition{ - { - Message: "not accepted", - Type: string(gatewayapiv1alpha2.PolicyConditionAccepted), - Status: metav1.ConditionFalse, - Reason: string(gatewayapiv1alpha2.PolicyReasonTargetNotFound), - }, - { - Message: "DNSPolicy has been successfully enforced", - Type: string(kuadrant.PolicyConditionEnforced), - Status: metav1.ConditionTrue, - Reason: string(kuadrant.PolicyConditionEnforced), - }, - }, - }, - }, - specErr: kuadrant.NewErrInvalid("DNSPolicy", errors.New("policy Error")), - }, - want: &v1alpha1.DNSPolicyStatus{ - Conditions: []metav1.Condition{ - { - Message: "DNSPolicy target is invalid: policy Error", - Type: string(gatewayapiv1alpha2.PolicyConditionAccepted), - Status: metav1.ConditionFalse, - Reason: string(gatewayapiv1alpha2.PolicyReasonInvalid), - }, - }, - }, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - r := &DNSPolicyReconciler{} - if got := r.calculateStatus(tt.args.ctx, tt.args.dnsPolicy, tt.args.specErr); !reflect.DeepEqual(got, tt.want) { - t.Errorf("calculateStatus() = %v, want %v", got, tt.want) - } - }) - } -} diff --git a/controllers/dnspolicy_status_updater.go b/controllers/dnspolicy_status_updater.go new file mode 100644 index 000000000..e792e4915 --- /dev/null +++ b/controllers/dnspolicy_status_updater.go @@ -0,0 +1,121 @@ +package controllers + +import ( + "context" + "fmt" + "slices" + "sync" + + "github.com/samber/lo" + + "k8s.io/apimachinery/pkg/api/equality" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/dynamic" + + 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/kuadrant" +) + +func NewDNSPolicyStatusUpdater(client *dynamic.DynamicClient) *DNSPolicyStatusUpdater { + return &DNSPolicyStatusUpdater{client: client} +} + +type DNSPolicyStatusUpdater struct { + client *dynamic.DynamicClient +} + +func (r *DNSPolicyStatusUpdater) Subscription() controller.Subscription { + return controller.Subscription{ + ReconcileFunc: r.updateStatus, + Events: []controller.ResourceEventMatcher{ + {Kind: &machinery.GatewayGroupKind}, + {Kind: &kuadrantv1alpha1.DNSPolicyGroupKind}, + {Kind: &DNSRecordGroupKind}, + }, + } +} + +func (r *DNSPolicyStatusUpdater) updateStatus(ctx context.Context, _ []controller.ResourceEvent, topology *machinery.Topology, _ error, state *sync.Map) error { + logger := controller.LoggerFromContext(ctx).WithName("DNSPolicyStatusUpdater") + + policyTypeFilterFunc := dnsPolicyTypeFilterFunc() + policyAcceptedFunc := dnsPolicyAcceptedStatusFunc(state) + policyErrorFunc := dnsPolicyErrorFunc(state) + + policies := lo.FilterMap(topology.Policies().Items(), policyTypeFilterFunc) + + logger.V(1).Info("updating dns policy statuses", "policies", len(policies)) + + for _, policy := range policies { + pLogger := logger.WithValues("policy", policy.GetLocator()) + + pLogger.V(1).Info("updating dns policy status") + + if policy.GetDeletionTimestamp() != nil { + pLogger.V(1).Info("policy marked for deletion, skipping") + continue + } + + // copy initial conditions, otherwise status will always be updated + newStatus := &kuadrantv1alpha1.DNSPolicyStatus{ + Conditions: slices.Clone(policy.Status.Conditions), + ObservedGeneration: policy.Status.ObservedGeneration, + } + + accepted, err := policyAcceptedFunc(policy) + meta.SetStatusCondition(&newStatus.Conditions, *kuadrant.AcceptedCondition(policy, err)) + + // do not set enforced condition if Accepted condition is false + if !accepted { + meta.RemoveStatusCondition(&newStatus.Conditions, string(kuadrant.PolicyConditionEnforced)) + } else { + policyRecords := lo.FilterMap(topology.Objects().Children(policy), func(item machinery.Object, _ int) (*kuadrantdnsv1alpha1.DNSRecord, bool) { + if rObj, isObj := item.(*controller.RuntimeObject); isObj { + if record, isRec := rObj.Object.(*kuadrantdnsv1alpha1.DNSRecord); isRec { + return record, true + } + } + return nil, false + }) + + enforcedCond := enforcedCondition(policyRecords, policy) + if pErr := policyErrorFunc(policy); pErr != nil { + pLogger.V(1).Info("adding contextual error to policy enforced status", "err", pErr) + enforcedCond.Message = fmt.Sprintf("%s : %s", enforcedCond.Message, pErr.Error()) + } + meta.SetStatusCondition(&newStatus.Conditions, *enforcedCond) + + propagateRecordConditions(policyRecords, newStatus) + + newStatus.TotalRecords = int32(len(policyRecords)) + } + + equalStatus := equality.Semantic.DeepEqual(newStatus, policy.Status) + if equalStatus && policy.Generation == policy.Status.ObservedGeneration { + pLogger.V(1).Info("policy status unchanged, skipping update") + continue + } + newStatus.ObservedGeneration = policy.Generation + policy.Status = *newStatus + + obj, err := controller.Destruct(policy) + if err != nil { + pLogger.Error(err, "unable to destruct policy") // should never happen + continue + } + + _, err = r.client.Resource(kuadrantv1alpha1.DNSPoliciesResource).Namespace(policy.GetNamespace()).UpdateStatus(ctx, obj, metav1.UpdateOptions{}) + if err != nil { + pLogger.Error(err, "unable to update status for policy") + } + + emitConditionMetrics(policy) + } + + return nil +} diff --git a/controllers/effective_dnspolicies_reconciler.go b/controllers/effective_dnspolicies_reconciler.go new file mode 100644 index 000000000..0b70cc579 --- /dev/null +++ b/controllers/effective_dnspolicies_reconciler.go @@ -0,0 +1,302 @@ +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, 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 { + return controller.Subscription{ + ReconcileFunc: r.reconcile, + Events: []controller.ResourceEventMatcher{ + {Kind: &machinery.GatewayGroupKind}, + {Kind: &kuadrantv1alpha1.DNSPolicyGroupKind}, + {Kind: &DNSRecordGroupKind}, + }, + } +} + +func (r *EffectiveDNSPoliciesReconciler) reconcile(ctx context.Context, _ []controller.ResourceEvent, topology *machinery.Topology, _ error, state *sync.Map) error { + logger := controller.LoggerFromContext(ctx).WithName("EffectiveDNSPoliciesReconciler") + + policyTypeFilterFunc := dnsPolicyTypeFilterFunc() + policyAcceptedFunc := dnsPolicyAcceptedStatusFunc(state) + + policies := lo.FilterMap(topology.Policies().Items(), policyTypeFilterFunc) + + policyErrors := map[string]error{} + + logger.V(1).Info("updating dns policies", "policies", len(policies)) + + clusterID, err := utils.GetClusterUID(ctx, r.client) + if err != nil { + return fmt.Errorf("failed to generate cluster ID: %w", err) + } + + for _, policy := range policies { + pLogger := logger.WithValues("policy", policy.GetLocator()) + + 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 + var gatewayHasAddresses = false + + for _, listener := range listeners { + lLogger := pLogger.WithValues("listener", listener.GetLocator()) + + gateway := listener.Gateway + if listener.Hostname == nil || *listener.Hostname == "" { + lLogger.Info("listener has no hostname assigned, skipping") + continue + } + + if len(gateway.Status.Addresses) > 0 { + gatewayHasAddresses = true + } + + hasAttachedRoute := false + for _, statusListener := range gateway.Status.Listeners { + if string(listener.Name) == string(statusListener.Name) { + hasAttachedRoute = statusListener.AttachedRoutes > 0 + break + } + } + 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 + } + + resource := r.client.Resource(DNSRecordResource).Namespace(desiredRecord.GetNamespace()) + + existingRecordObj, recordExists := 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)) + }) + + if len(desiredRecord.Spec.Endpoints) == 0 { + policyErrors[policy.GetLocator()] = ErrNoAddresses + } + + //Update + if recordExists { + rLogger := lLogger.WithValues("record", existingRecordObj.GetLocator()) + + existingRecord := existingRecordObj.(*controller.RuntimeObject).Object.(*kuadrantdnsv1alpha1.DNSRecord) + + //Deal with the potential deletion of a record first + if !hasAttachedRoute || len(desiredRecord.Spec.Endpoints) == 0 { + if !hasAttachedRoute { + rLogger.V(1).Info("listener has no attached routes, deleting record for listener") + } else { + rLogger.V(1).Info("no endpoint addresses for DNSRecord, deleting record for listener") + } + r.deleteRecord(ctx, existingRecordObj) + continue + } + + if desiredRecord.Spec.RootHost != existingRecord.Spec.RootHost { + rLogger.V(1).Info("listener hostname has changed, deleting 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) { + rLogger.V(1).Info("dns record is up to date, nothing to do") + continue + } + existingRecord.Spec = desiredRecord.Spec + existingRecord.OwnerReferences = desiredRecord.OwnerReferences + + un, err := controller.Destruct(existingRecord) + if err != nil { + lLogger.Error(err, "unable to destruct dns record") + continue + } + + rLogger.V(1).Info("updating DNS record for listener") + if _, uErr := resource.Update(ctx, un, metav1.UpdateOptions{}); uErr != nil { + rLogger.Error(uErr, "unable to update dns record") + } + continue + } + + if !hasAttachedRoute { + lLogger.V(1).Info("listener has no attached routes, skipping record create for listener") + continue + } + + if len(desiredRecord.Spec.Endpoints) == 0 { + lLogger.V(1).Info("record for listener has no addresses, skipping record create for listener") + continue + } + + un, err := controller.Destruct(desiredRecord) + if err != nil { + lLogger.Error(err, "unable to destruct dns record") + continue + } + + //Create + lLogger.V(1).Info("creating DNS record for listener") + if _, cErr := resource.Create(ctx, un, metav1.CreateOptions{}); cErr != nil && !apierrors.IsAlreadyExists(cErr) { + lLogger.Error(cErr, "unable to create dns record") + } + } + + if !gatewayHasAddresses { + pLogger.V(1).Info("gateway has no addresses") + policyErrors[policy.GetLocator()] = ErrNoAddresses + } else if !gatewayHasAttachedRoutes { + pLogger.V(1).Info("gateway has no attached routes") + policyErrors[policy.GetLocator()] = ErrNoRoutes + } + } + + state.Store(StateDNSPolicyErrorsKey, policyErrors) + + 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 dnsPolicyTypeFilter) []*machinery.Listener { + return lo.Flatten(lo.FilterMap(topology.Targetables().Children(policy), func(t machinery.Targetable, _ int) ([]*machinery.Listener, bool) { + 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("deleteOrphanDNSRecords") + + orphanRecords := lo.Filter(topology.Objects().Items(), func(item machinery.Object, _ int) bool { + if item.GroupVersionKind().GroupKind() == DNSRecordGroupKind { + rLogger := logger.WithValues("record", item.GetLocator()) + + pTargettables := topology.Targetables().Parents(item) + pPolicies := topology.Policies().Parents(item) + + if logger.V(1).Enabled() { + pPoliciesLocs := lo.Map(pPolicies, func(item machinery.Policy, _ int) string { + return item.GetLocator() + }) + pTargetablesLocs := lo.Map(pTargettables, func(item machinery.Targetable, _ int) string { + return item.GetLocator() + }) + rLogger.V(1).Info("dns record parents", "targetables", pTargetablesLocs, "polices", pPoliciesLocs) + } + + //Target removed from topology + if len(pTargettables) == 0 { + rLogger.Info("dns record has not parent targetable, deleting") + return true + } + + //Policy removed from topology + if len(pPolicies) == 0 { + rLogger.Info("dns record has not parent policy, deleting") + return true + } + + //Policy target ref changes + if len(topology.All().Paths(pPolicies[0], item)) == 1 { //There will always be at least one DNSPolicy -> DNSRecord + rLogger.Info("dns record has no path through a targetable to the policy, deleting", "policy", pPolicies[0]) + return true + } + + return false + } + return false + }) + + for _, obj := range orphanRecords { + 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 + } + logger.Info("deleting 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()) + } +} diff --git a/controllers/state_of_the_world.go b/controllers/state_of_the_world.go index 0f3174ef1..dfa703a3e 100644 --- a/controllers/state_of_the_world.go +++ b/controllers/state_of_the_world.go @@ -9,6 +9,7 @@ import ( egv1alpha1 "github.com/envoyproxy/gateway/api/v1alpha1" "github.com/go-logr/logr" authorinov1beta1 "github.com/kuadrant/authorino-operator/api/v1beta1" + kuadrantdnsv1alpha1 "github.com/kuadrant/dns-operator/api/v1alpha1" limitadorv1alpha1 "github.com/kuadrant/limitador-operator/api/v1alpha1" "github.com/kuadrant/policy-machinery/controller" "github.com/kuadrant/policy-machinery/machinery" @@ -169,6 +170,7 @@ func (b *BootOptionsBuilder) getOptions() []controller.ControllerOption { opts = append(opts, b.getEnvoyGatewayOptions()...) opts = append(opts, b.getCertManagerOptions()...) opts = append(opts, b.getConsolePluginOptions()...) + opts = append(opts, b.getDNSOperatorOptions()...) return opts } @@ -315,13 +317,31 @@ func (b *BootOptionsBuilder) getConsolePluginOptions() []controller.ControllerOp return opts } +func (b *BootOptionsBuilder) getDNSOperatorOptions() []controller.ControllerOption { + var opts []controller.ControllerOption + opts = append(opts, + controller.WithRunnable("dnsrecord watcher", controller.Watch( + &kuadrantdnsv1alpha1.DNSRecord{}, DNSRecordResource, metav1.NamespaceAll, + controller.FilterResourcesByLabel[*kuadrantdnsv1alpha1.DNSRecord](fmt.Sprintf("%s=%s", AppLabelKey, AppLabelValue)))), + controller.WithObjectKinds( + DNSRecordGroupKind, + ), + controller.WithObjectLinks( + LinkListenerToDNSRecord, + LinkDNSPolicyToDNSRecord, + ), + ) + + return opts +} + func (b *BootOptionsBuilder) Reconciler() controller.ReconcileFunc { mainWorkflow := &controller.Workflow{ Precondition: initWorkflow(b.client).Run, Tasks: []controller.ReconcileFunc{ NewAuthorinoReconciler(b.client).Subscription().Reconcile, NewLimitadorReconciler(b.client).Subscription().Reconcile, - NewDNSWorkflow().Run, + NewDNSWorkflow(b.client, b.manager.GetScheme()).Run, NewTLSWorkflow(b.client, b.manager.GetScheme(), b.isCertManagerInstalled).Run, NewAuthWorkflow().Run, NewRateLimitWorkflow(b.client, b.isIstioInstalled, b.isEnvoyGatewayInstalled).Run, diff --git a/controllers/test_common.go b/controllers/test_common.go index 0f7c67693..c4a9e3efb 100644 --- a/controllers/test_common.go +++ b/controllers/test_common.go @@ -87,20 +87,6 @@ func SetupKuadrantOperatorForTest(s *runtime.Scheme, cfg *rest.Config) { }).SetupWithManager(mgr) Expect(err).NotTo(HaveOccurred()) - dnsPolicyBaseReconciler := reconcilers.NewBaseReconciler( - mgr.GetClient(), - mgr.GetScheme(), - mgr.GetAPIReader(), - log.Log.WithName("dnspolicy"), - ) - - err = (&DNSPolicyReconciler{ - BaseReconciler: dnsPolicyBaseReconciler, - TargetRefReconciler: reconcilers.TargetRefReconciler{Client: mgr.GetClient()}, - }).SetupWithManager(mgr) - - Expect(err).NotTo(HaveOccurred()) - kuadrantBaseReconciler := reconcilers.NewBaseReconciler( mgr.GetClient(), mgr.GetScheme(), diff --git a/go.mod b/go.mod index 04d157da7..699b43ce5 100644 --- a/go.mod +++ b/go.mod @@ -14,7 +14,7 @@ require ( github.com/kuadrant/authorino-operator v0.11.1 github.com/kuadrant/dns-operator v0.0.0-20241018131559-f2ce8b6aaaef github.com/kuadrant/limitador-operator v0.9.0 - github.com/kuadrant/policy-machinery v0.6.0 + github.com/kuadrant/policy-machinery v0.6.1 github.com/martinlindhe/base36 v1.1.1 github.com/onsi/ginkgo/v2 v2.20.2 github.com/onsi/gomega v1.34.1 diff --git a/go.sum b/go.sum index 5c6e9f278..895ed1ec2 100644 --- a/go.sum +++ b/go.sum @@ -262,8 +262,8 @@ github.com/kuadrant/dns-operator v0.0.0-20241018131559-f2ce8b6aaaef h1:6P2pC1kOP github.com/kuadrant/dns-operator v0.0.0-20241018131559-f2ce8b6aaaef/go.mod h1:LGG4R3KEz93Ep0CV1/tziCmRk+VtojWUHR9mXkOHZks= github.com/kuadrant/limitador-operator v0.9.0 h1:hTQ6CFPayf/sL7cIzwWjCoU8uTn6fzWdsJgKbDlnFts= github.com/kuadrant/limitador-operator v0.9.0/go.mod h1:DQOlg9qFOcnWPrwO529JRCMLLOEXJQxkmOes952S/Hw= -github.com/kuadrant/policy-machinery v0.6.0 h1:sgvZ+EENZ+azvJ8uVsgOuWGH0z1gjCnXbknrvHdoQxQ= -github.com/kuadrant/policy-machinery v0.6.0/go.mod h1:ZV4xS0CCxPgu/Xg6gz+YUaS9zqEXKOiAj33bZ67B6Lo= +github.com/kuadrant/policy-machinery v0.6.1 h1:w43DyD/yljzz0T6PNYXmuuhLxrF+IhaFB2rUqrwvGGk= +github.com/kuadrant/policy-machinery v0.6.1/go.mod h1:ZV4xS0CCxPgu/Xg6gz+YUaS9zqEXKOiAj33bZ67B6Lo= github.com/kylelemons/godebug v1.1.0 h1:RPNrshWIDI6G2gRW9EHilWtl7Z6Sb1BR0xunSBf0SNc= github.com/kylelemons/godebug v1.1.0/go.mod h1:9/0rRGxNHcop5bhtWyNeEfOS8JIWk580+fNqagV/RAw= github.com/lann/builder v0.0.0-20180802200727-47ae307949d0 h1:SOEGU9fKiNWd/HOJuq6+3iTQz8KNCLtVX6idSoTLdUw= diff --git a/main.go b/main.go index 229af861b..1df960668 100644 --- a/main.go +++ b/main.go @@ -184,19 +184,6 @@ func main() { os.Exit(1) } - dnsPolicyBaseReconciler := reconcilers.NewBaseReconciler( - mgr.GetClient(), mgr.GetScheme(), mgr.GetAPIReader(), - log.Log.WithName("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) - } - gatewayKuadrantBaseReconciler := reconcilers.NewBaseReconciler( mgr.GetClient(), mgr.GetScheme(), mgr.GetAPIReader(), log.Log.WithName("kuadrant").WithName("gateway"), diff --git a/pkg/library/utils/k8s_utils.go b/pkg/library/utils/k8s_utils.go index 6c42e85ff..913347277 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" ) @@ -86,6 +87,10 @@ func StatusConditionsMarshalJSON(input []metav1.Condition) ([]byte, error) { // The version of the owner reference is not checked in this implementation. // Returns true if the owned object is owned by the owner object, false otherwise. func IsOwnedBy(owned, owner client.Object) bool { + if owned.GetNamespace() != owner.GetNamespace() { + return false + } + ownerGVK := owner.GetObjectKind().GroupVersionKind() for _, o := range owned.GetOwnerReferences() { @@ -154,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.Interface) (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/pkg/library/utils/k8s_utils_test.go b/pkg/library/utils/k8s_utils_test.go index fcc291ead..7d36a48a3 100644 --- a/pkg/library/utils/k8s_utils_test.go +++ b/pkg/library/utils/k8s_utils_test.go @@ -13,7 +13,10 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/intstr" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" + dfake "k8s.io/client-go/dynamic/fake" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -466,6 +469,58 @@ func TestIsOwnedBy(t *testing.T) { }, expected: false, }, + { + name: "when owned object has owner reference and in same namespace then return true", + owned: &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "ns1", + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "v1", + Kind: "Deployment", + Name: "my-deployment", + }, + }, + }, + }, + owner: &appsv1.Deployment{ + TypeMeta: metav1.TypeMeta{ + Kind: "Deployment", + APIVersion: "v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "my-deployment", + Namespace: "ns1", + }, + }, + expected: true, + }, + { + name: "when owned object has owner reference but in different namespace then return false", + owned: &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "ns1", + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "v1", + Kind: "Deployment", + Name: "my-deployment", + }, + }, + }, + }, + owner: &appsv1.Deployment{ + TypeMeta: metav1.TypeMeta{ + Kind: "Deployment", + APIVersion: "v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "my-deployment", + Namespace: "ns2", + }, + }, + expected: false, + }, } for _, tc := range testCases { @@ -849,14 +904,17 @@ func TestGetLabel(t *testing.T) { } func TestGetClusterUID(t *testing.T) { + var tScheme = runtime.NewScheme() + utilruntime.Must(corev1.AddToScheme(tScheme)) + var testCases = []struct { Name string - Objects []client.Object + Objects []runtime.Object Validation func(t *testing.T, e error, id string) }{ { Name: "an absent namespace generates an error", - Objects: []client.Object{}, + Objects: []runtime.Object{}, Validation: func(t *testing.T, e error, id string) { if !errors.IsNotFound(e) { t.Errorf("expected not found error, got '%v'", e) @@ -865,7 +923,7 @@ func TestGetClusterUID(t *testing.T) { }, { Name: "a UID generates a valid deterministic cluster ID", - Objects: []client.Object{ + Objects: []runtime.Object{ &corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{ Name: clusterIDNamespace, @@ -887,7 +945,7 @@ func TestGetClusterUID(t *testing.T) { for _, testCase := range testCases { t.Run(testCase.Name, func(t *testing.T) { - fc := fake.NewClientBuilder().WithObjects(testCase.Objects...).Build() + fc := dfake.NewSimpleDynamicClient(tScheme, testCase.Objects...) id, err := GetClusterUID(context.Background(), fc) testCase.Validation(t, err, id) }) 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 1f6dfa675..1e0b7b362 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). @@ -515,8 +526,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)). @@ -560,10 +569,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) { @@ -583,7 +588,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{ @@ -600,12 +604,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) @@ -663,12 +661,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") @@ -680,8 +675,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) @@ -722,8 +715,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") @@ -772,8 +763,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" @@ -821,20 +810,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( @@ -1238,14 +1215,18 @@ var _ = Describe("DNSPolicy controller", func() { g.Expect(err).NotTo(HaveOccurred()) g.Expect(dnsPolicy.Status.Conditions).To( ContainElement(MatchFields(IgnoreExtras, Fields{ - "Type": Equal(string(gatewayapiv1alpha2.PolicyConditionAccepted)), - "Status": Equal(metav1.ConditionTrue), + "Type": Equal(string(gatewayapiv1alpha2.PolicyConditionAccepted)), + "Status": Equal(metav1.ConditionTrue), + "Reason": Equal(string(gatewayapiv1alpha2.PolicyConditionAccepted)), + "Message": Equal("DNSPolicy has been accepted"), })), ) g.Expect(dnsPolicy.Status.Conditions).To( ContainElement(MatchFields(IgnoreExtras, Fields{ - "Type": Equal(string(kuadrant.PolicyConditionEnforced)), - "Status": Equal(metav1.ConditionTrue), + "Type": Equal(string(kuadrant.PolicyConditionEnforced)), + "Status": Equal(metav1.ConditionTrue), + "Reason": Equal(string(kuadrant.PolicyReasonEnforced)), + "Message": ContainSubstring("DNSPolicy has been successfully enforced : no DNSRecords created based on policy and gateway configuration : no valid status addresses to use on gateway"), })), ) g.Expect(int(dnsPolicy.Status.TotalRecords)).To(Equal(0))