Merge pull request #629 from mikenairn/simple_dnspolicy_strategy

Add DNSPolicy Routing Strategy

bundle/manifests/kuadrant.io_dnspolicies.yaml

@@ -156,6 +156,12 @@ spec:
                         type: integer
                     type: object
                 type: object
+              routingStrategy:
+                default: loadbalanced
+                enum:
+                - simple
+                - loadbalanced
+                type: string
               targetRef:
                 description: PolicyTargetReference identifies an API object to apply
                   policy to. This should be used as part of Policy resources that
@@ -194,6 +200,7 @@ spec:
                 - name
                 type: object
             required:
+            - routingStrategy
             - targetRef
             type: object
           status:

bundle/manifests/multicluster-gateway-controller.clusterserviceversion.yaml

@@ -4,7 +4,7 @@ metadata:
   annotations:
     alm-examples: '[]'
     capabilities: Basic Install
-    createdAt: "2023-11-09T08:49:14Z"
+    createdAt: "2023-11-10T09:41:24Z"
     operators.operatorframework.io/builder: operator-sdk-v1.28.0
     operators.operatorframework.io/project_layout: go.kubebuilder.io/v3
   name: multicluster-gateway-controller.v0.0.0

config/policy-controller/crd/bases/kuadrant.io_dnspolicies.yaml

@@ -155,6 +155,12 @@ spec:
                         type: integer
                     type: object
                 type: object
+              routingStrategy:
+                default: loadbalanced
+                enum:
+                - simple
+                - loadbalanced
+                type: string
               targetRef:
                 description: PolicyTargetReference identifies an API object to apply
                   policy to. This should be used as part of Policy resources that
@@ -193,6 +199,7 @@ spec:
                 - name
                 type: object
             required:
+            - routingStrategy
             - targetRef
             type: object
           status:

go.mod

@@ -8,7 +8,6 @@ require (
 	github.com/google/uuid v1.3.0
 	github.com/goombaio/namegenerator v0.0.0-20181006234301-989e774b106e
 	github.com/jetstack/cert-manager v1.7.1
-	github.com/kuadrant/authorino v0.10.0
 	github.com/kuadrant/kuadrant-operator v0.1.1-0.20230323151616-58593d01833a
 	github.com/martinlindhe/base36 v1.1.1
 	github.com/onsi/ginkgo/v2 v2.11.0
@@ -66,6 +65,7 @@ require (
 	github.com/jmespath/go-jmespath v0.4.0 // indirect
 	github.com/josharian/intern v1.0.0 // indirect
 	github.com/json-iterator/go v1.1.12 // indirect
+	github.com/kuadrant/authorino v0.10.0 // indirect
 	github.com/kuadrant/authorino-operator v0.4.1 // indirect
 	github.com/kuadrant/limitador-operator v0.4.0 // indirect
 	github.com/mailru/easyjson v0.7.7 // indirect

pkg/apis/v1alpha1/dnspolicy_types.go

@@ -25,6 +25,13 @@ import (
 	gatewayv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1"
 )
 
+type RoutingStrategy string
+
+const (
+	SimpleRoutingStrategy       RoutingStrategy = "simple"
+	LoadBalancedRoutingStrategy RoutingStrategy = "loadbalanced"
+)
+
 // DNSPolicySpec defines the desired state of DNSPolicy
 type DNSPolicySpec struct {
 
@@ -37,6 +44,11 @@ type DNSPolicySpec struct {
 
 	// +optional
 	LoadBalancing *LoadBalancingSpec `json:"loadBalancing"`
+
+	// +required
+	// +kubebuilder:validation:Enum=simple;loadbalanced
+	// +kubebuilder:default=loadbalanced
+	RoutingStrategy RoutingStrategy `json:"routingStrategy"`
 }
 
 type LoadBalancingSpec struct {

pkg/controllers/dnspolicy/dns_helper.go

@@ -33,8 +33,9 @@ const (
 )
 
 var (
-	ErrNoManagedZoneForHost = fmt.Errorf("no managed zone for host")
-	ErrAlreadyAssigned      = fmt.Errorf("managed host already assigned")
+	ErrUnknownRoutingStrategy = fmt.Errorf("unknown routing strategy")
+	ErrNoManagedZoneForHost   = fmt.Errorf("no managed zone for host")
+	ErrAlreadyAssigned        = fmt.Errorf("managed host already assigned")
 )
 
 type dnsHelper struct {
@@ -147,7 +148,74 @@ func withGatewayListener[T metav1.Object](gateway common.GatewayWrapper, listene
 	return obj
 }
 
-// setEndpoints sets the endpoints for the given MultiClusterGatewayTarget
+func (dh *dnsHelper) setEndpoints(ctx context.Context, mcgTarget *dns.MultiClusterGatewayTarget, dnsRecord *v1alpha1.DNSRecord, listener gatewayv1beta1.Listener, strategy v1alpha1.RoutingStrategy) error {
+	old := dnsRecord.DeepCopy()
+	gwListenerHost := string(*listener.Hostname)
+	var endpoints []*v1alpha1.Endpoint
+
+	//Health Checks currently modify endpoints so we have to keep existing ones in order to not lose health check ids
+	currentEndpoints := make(map[string]*v1alpha1.Endpoint, len(dnsRecord.Spec.Endpoints))
+	for _, endpoint := range dnsRecord.Spec.Endpoints {
+		currentEndpoints[endpoint.SetID()] = endpoint
+	}
+
+	switch strategy {
+	case v1alpha1.SimpleRoutingStrategy:
+		endpoints = dh.getSimpleEndpoints(mcgTarget, gwListenerHost, currentEndpoints)
+	case v1alpha1.LoadBalancedRoutingStrategy:
+		endpoints = dh.getLoadBalancedEndpoints(mcgTarget, gwListenerHost, currentEndpoints)
+	default:
+		return fmt.Errorf("%w : %s", ErrUnknownRoutingStrategy, strategy)
+	}
+
+	sort.Slice(endpoints, func(i, j int) bool {
+		return endpoints[i].SetID() < endpoints[j].SetID()
+	})
+
+	dnsRecord.Spec.Endpoints = endpoints
+
+	if !equality.Semantic.DeepEqual(old, dnsRecord) {
+		return dh.Update(ctx, dnsRecord)
+	}
+
+	return nil
+}
+
+// getSimpleEndpoints returns the endpoints for the given MultiClusterGatewayTarget using the simple routing strategy
+
+func (dh *dnsHelper) getSimpleEndpoints(mcgTarget *dns.MultiClusterGatewayTarget, hostname string, currentEndpoints map[string]*v1alpha1.Endpoint) []*v1alpha1.Endpoint {
+
+	var (
+		endpoints  []*v1alpha1.Endpoint
+		ipValues   []string
+		hostValues []string
+	)
+
+	for _, cgwTarget := range mcgTarget.ClusterGatewayTargets {
+		for _, gwa := range cgwTarget.GatewayAddresses {
+			if *gwa.Type == gatewayv1beta1.IPAddressType {
+				ipValues = append(ipValues, gwa.Value)
+			} else {
+				hostValues = append(hostValues, gwa.Value)
+			}
+		}
+	}
+
+	if len(ipValues) > 0 {
+		endpoint := createOrUpdateEndpoint(hostname, ipValues, v1alpha1.ARecordType, "", dns.DefaultTTL, currentEndpoints)
+		endpoints = append(endpoints, endpoint)
+	}
+
+	//ToDO This could possibly result in an invalid record since you can't have multiple CNAME target values https://github.com/Kuadrant/multicluster-gateway-controller/issues/663
+	if len(hostValues) > 0 {
+		endpoint := createOrUpdateEndpoint(hostname, hostValues, v1alpha1.CNAMERecordType, "", dns.DefaultTTL, currentEndpoints)
+		endpoints = append(endpoints, endpoint)
+	}
+
+	return endpoints
+}
+
+// getLoadBalancedEndpoints returns the endpoints for the given MultiClusterGatewayTarget using the loadbalanced routing strategy
 //
 // Builds an array of v1alpha1.Endpoint resources and sets them on the given DNSRecord. The endpoints expected are calculated
 // from the MultiClusterGatewayTarget using the target Gateway (MultiClusterGatewayTarget.Gateway), the LoadBalancing Spec
@@ -186,23 +254,15 @@ func withGatewayListener[T metav1.Object](gateway common.GatewayWrapper, listene
 // ab2.lb-a1b2.shop.example.com A 192.22.2.3
 // ab3.lb-a1b2.shop.example.com A 192.22.2.4
 
-func (dh *dnsHelper) setEndpoints(ctx context.Context, mcgTarget *dns.MultiClusterGatewayTarget, dnsRecord *v1alpha1.DNSRecord, listener gatewayv1beta1.Listener) error {
+func (dh *dnsHelper) getLoadBalancedEndpoints(mcgTarget *dns.MultiClusterGatewayTarget, hostname string, currentEndpoints map[string]*v1alpha1.Endpoint) []*v1alpha1.Endpoint {
 
-	old := dnsRecord.DeepCopy()
-	gwListenerHost := string(*listener.Hostname)
-	cnameHost := gwListenerHost
-	if isWildCardListener(listener) {
-		cnameHost = strings.Replace(gwListenerHost, "*.", "", -1)
-	}
-
-	//Health Checks currently modify endpoints so we have to keep existing ones in order to not lose health check ids
-	currentEndpoints := make(map[string]*v1alpha1.Endpoint, len(dnsRecord.Spec.Endpoints))
-	for _, endpoint := range dnsRecord.Spec.Endpoints {
-		currentEndpoints[endpoint.SetID()] = endpoint
+	cnameHost := hostname
+	if isWildCardHost(hostname) {
+		cnameHost = strings.Replace(hostname, "*.", "", -1)
 	}
 
 	var (
-		newEndpoints    []*v1alpha1.Endpoint
+		endpoints       []*v1alpha1.Endpoint
 		endpoint        *v1alpha1.Endpoint
 		defaultEndpoint *v1alpha1.Endpoint
 	)
@@ -239,7 +299,7 @@ func (dh *dnsHelper) setEndpoints(ctx context.Context, mcgTarget *dns.MultiClust
 		if len(clusterEndpoints) == 0 {
 			continue
 		}
-		newEndpoints = append(newEndpoints, clusterEndpoints...)
+		endpoints = append(endpoints, clusterEndpoints...)
 
 		//Create lbName CNAME (lb-a1b2.shop.example.com -> default.lb-a1b2.shop.example.com)
 		endpoint = createOrUpdateEndpoint(lbName, []string{geoLbName}, v1alpha1.CNAMERecordType, string(geoCode), dns.DefaultCnameTTL, currentEndpoints)
@@ -256,28 +316,19 @@ func (dh *dnsHelper) setEndpoints(ctx context.Context, mcgTarget *dns.MultiClust
 
 		endpoint.SetProviderSpecific(dns.ProviderSpecificGeoCode, string(geoCode))
 
-		newEndpoints = append(newEndpoints, endpoint)
+		endpoints = append(endpoints, endpoint)
 	}
 
-	if len(newEndpoints) > 0 {
-		// Add the `defaultEndpoint`, this should always be set by this point if `newEndpoints` isn't empty
+	if len(endpoints) > 0 {
+		// Add the `defaultEndpoint`, this should always be set by this point if `endpoints` isn't empty
 		defaultEndpoint.SetProviderSpecific(dns.ProviderSpecificGeoCode, string(dns.WildcardGeo))
-		newEndpoints = append(newEndpoints, defaultEndpoint)
+		endpoints = append(endpoints, defaultEndpoint)
 		//Create gwListenerHost CNAME (shop.example.com -> lb-a1b2.shop.example.com)
-		endpoint = createOrUpdateEndpoint(gwListenerHost, []string{lbName}, v1alpha1.CNAMERecordType, "", dns.DefaultCnameTTL, currentEndpoints)
-		newEndpoints = append(newEndpoints, endpoint)
+		endpoint = createOrUpdateEndpoint(hostname, []string{lbName}, v1alpha1.CNAMERecordType, "", dns.DefaultCnameTTL, currentEndpoints)
+		endpoints = append(endpoints, endpoint)
 	}
 
-	sort.Slice(newEndpoints, func(i, j int) bool {
-		return newEndpoints[i].SetID() < newEndpoints[j].SetID()
-	})
-
-	dnsRecord.Spec.Endpoints = newEndpoints
-
-	if !equality.Semantic.DeepEqual(old, dnsRecord) {
-		return dh.Update(ctx, dnsRecord)
-	}
-	return nil
+	return endpoints
 }
 
 func createOrUpdateEndpoint(dnsName string, targets v1alpha1.Targets, recordType v1alpha1.DNSRecordType, setIdentifier string,
@@ -374,8 +425,8 @@ func (dh *dnsHelper) deleteDNSRecordForListener(ctx context.Context, owner metav
 	return dh.Delete(ctx, &dnsRecord, &client.DeleteOptions{})
 }
 
-func isWildCardListener(l gatewayv1beta1.Listener) bool {
-	return strings.HasPrefix(string(*l.Hostname), "*")
+func isWildCardHost(host string) bool {
+	return strings.HasPrefix(host, "*")
 }
 
 func (dh *dnsHelper) getDNSHealthCheckProbes(ctx context.Context, gateway *gatewayv1beta1.Gateway, dnsPolicy *v1alpha1.DNSPolicy) ([]*v1alpha1.DNSHealthCheckProbe, error) {

pkg/controllers/dnspolicy/dns_helper_test.go

@@ -975,7 +975,7 @@ func Test_dnsHelper_setEndpoints(t *testing.T) {
 		t.Run(testCase.name, func(t *testing.T) {
 			f := fake.NewClientBuilder().WithScheme(testScheme(t)).WithObjects(testCase.dnsRecord).Build()
 			s := dnsHelper{Client: f}
-			if err := s.setEndpoints(context.TODO(), testCase.mcgTarget, testCase.dnsRecord, testCase.listener); (err != nil) != testCase.wantErr {
+			if err := s.setEndpoints(context.TODO(), testCase.mcgTarget, testCase.dnsRecord, testCase.listener, v1alpha1.LoadBalancedRoutingStrategy); (err != nil) != testCase.wantErr {
 				t.Errorf("SetEndpoints() error = %v, wantErr %v", err, testCase.wantErr)
 			}
 

pkg/controllers/dnspolicy/dnspolicy_controller.go

@@ -22,7 +22,6 @@ import (
 	"fmt"
 	"reflect"
 
-	"github.com/kuadrant/authorino/pkg/log"
 	clusterv1 "open-cluster-management.io/api/cluster/v1"
 
 	"k8s.io/apimachinery/pkg/api/equality"
@@ -194,7 +193,6 @@ func (r *DNSPolicyReconciler) deleteResources(ctx context.Context, dnsPolicy *v1
 	// delete based on gateway diffs
 
 	if err := r.deleteDNSRecords(ctx, dnsPolicy); err != nil {
-		log.V(3).Info("error reconciling DNS records from delete, returning", "error", err)
 		return err
 	}
 

pkg/controllers/dnspolicy/dnspolicy_dnsrecords.go

@@ -53,9 +53,9 @@ func (r *DNSPolicyReconciler) reconcileGatewayDNSRecords(ctx context.Context, ga
 		return err
 	}
 
-	clusterAddresses := getClusterGatewayAddresses(gateway)
+	clusterGatewayAddresses := getClusterGatewayAddresses(gateway)
 
-	log.V(3).Info("checking gateway for attached routes ", "gateway", gateway.Name, "clusters", clusterAddresses)
+	log.V(3).Info("checking gateway for attached routes ", "gateway", gateway.Name, "clusters", clusterGatewayAddresses)
 
 	for _, listener := range gateway.Spec.Listeners {
 		var clusterGateways []dns.ClusterGateway
@@ -68,19 +68,19 @@ func (r *DNSPolicyReconciler) reconcileGatewayDNSRecords(ctx context.Context, ga
 			log.Info("skipping listener no hostname assigned", listener.Name, "in ns ", gateway.Namespace)
 			continue
 		}
-		for clusterName, clusterAddress := range clusterAddresses {
+		for clusterName, gatewayAddresses := range clusterGatewayAddresses {
 			// Only consider host for dns if there's at least 1 attached route to the listener for this host in *any* gateway
 
 			log.V(3).Info("checking downstream", "listener ", listener.Name)
-			attached := listenerTotalAttachedRoutes(gateway, clusterName, listener, clusterAddress)
+			attached := listenerTotalAttachedRoutes(gateway, clusterName, listener, gatewayAddresses)
 
 			if attached == 0 {
 				log.V(1).Info("no attached routes for ", "listener", listener, "cluster ", clusterName)
 				continue
 			}
 			log.V(3).Info("hostHasAttachedRoutes", "host", listener.Name, "hostHasAttachedRoutes", attached)
 
-			cg, err := r.buildClusterGateway(ctx, clusterName, clusterAddress, gateway)
+			cg, err := r.buildClusterGateway(ctx, clusterName, gatewayAddresses, gateway)
 			if err != nil {
 				return fmt.Errorf("get cluster gateway failed: %s", err)
 			}
@@ -118,7 +118,7 @@ func (r *DNSPolicyReconciler) reconcileGatewayDNSRecords(ctx context.Context, ga
 			return err
 		}
 		mcgTarget.RemoveUnhealthyGatewayAddresses(probes, listener)
-		if err := r.dnsHelper.setEndpoints(ctx, mcgTarget, dnsRecord, listener); err != nil {
+		if err := r.dnsHelper.setEndpoints(ctx, mcgTarget, dnsRecord, listener, dnsPolicy.Spec.RoutingStrategy); err != nil {
 			return fmt.Errorf("failed to add dns record dnsTargets %s %v", err, mcgTarget)
 		}
 	}
@@ -151,22 +151,22 @@ func (r *DNSPolicyReconciler) deleteDNSRecordsWithLabels(ctx context.Context, lb
 	return nil
 }
 
-func (r *DNSPolicyReconciler) buildClusterGateway(ctx context.Context, downstreamClusterName string, clusterAddress []gatewayv1beta1.GatewayAddress, targetGW *gatewayv1beta1.Gateway) (dns.ClusterGateway, error) {
+func (r *DNSPolicyReconciler) buildClusterGateway(ctx context.Context, clusterName string, gatewayAddresses []gatewayv1beta1.GatewayAddress, targetGW *gatewayv1beta1.Gateway) (dns.ClusterGateway, error) {
 	var target dns.ClusterGateway
-	singleClusterAddresses := make([]gatewayv1beta1.GatewayAddress, len(clusterAddress))
+	singleClusterAddresses := make([]gatewayv1beta1.GatewayAddress, len(gatewayAddresses))
 
 	var metaObj client.Object
-	if downstreamClusterName != singleCluster {
+	if clusterName != singleCluster {
 		mc := &clusterv1.ManagedCluster{}
-		if err := r.Client().Get(ctx, client.ObjectKey{Name: downstreamClusterName}, mc, &client.GetOptions{}); err != nil {
+		if err := r.Client().Get(ctx, client.ObjectKey{Name: clusterName}, mc, &client.GetOptions{}); err != nil {
 			return target, err
 		}
 		metaObj = mc
 	} else {
 		metaObj = targetGW
 	}
 
-	for i, addr := range clusterAddress {
+	for i, addr := range gatewayAddresses {
 		addrType := *addr.Type
 		if addrType == gateway.MultiClusterHostnameAddressType {
 			addrType = gatewayv1beta1.HostnameAddressType
@@ -189,8 +189,6 @@ func getClusterGatewayAddresses(gw *gatewayv1beta1.Gateway) map[string][]gateway
 	clusterAddrs := make(map[string][]gatewayv1beta1.GatewayAddress, len(gw.Status.Addresses))
 
 	for _, address := range gw.Status.Addresses {
-		var gatewayAddresses []gatewayv1beta1.GatewayAddress
-
 		//Default to Single Cluster (Normal Gateway Status)
 		cluster := singleCluster
 		addressValue := address.Value
@@ -205,11 +203,14 @@ func getClusterGatewayAddresses(gw *gatewayv1beta1.Gateway) map[string][]gateway
 			}
 		}
 
-		gatewayAddresses = append(gatewayAddresses, gatewayv1beta1.GatewayAddress{
+		if _, ok := clusterAddrs[cluster]; !ok {
+			clusterAddrs[cluster] = []gatewayv1beta1.GatewayAddress{}
+		}
+
+		clusterAddrs[cluster] = append(clusterAddrs[cluster], gatewayv1beta1.GatewayAddress{
 			Type:  address.Type,
 			Value: addressValue,
 		})
-		clusterAddrs[cluster] = gatewayAddresses
 	}
 
 	return clusterAddrs