draft for add an excludeAddresses option in DNSPolicy

Signed-off-by: craig <[email protected]>

rh-pre-commit.version: 2.2.0
rh-pre-commit.check-secrets: ENABLED

fix lint

api/v1alpha1/dnspolicy_types.go

@@ -68,6 +68,11 @@ type DNSPolicySpec struct {
 	// +kubebuilder:validation:MaxItems=1
 	// +kubebuilder:validation:MinItems=1
 	ProviderRefs []dnsv1alpha1.ProviderRef `json:"providerRefs"`
+
+	// 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"`
 }
 
 type LoadBalancingSpec struct {

bundle/manifests/kuadrant-operator.clusterserviceversion.yaml

@@ -106,7 +106,7 @@ metadata:
     capabilities: Basic Install
     categories: Integration & Delivery
     containerImage: quay.io/kuadrant/kuadrant-operator:latest
-    createdAt: "2024-09-17T13:54:51Z"
+    createdAt: "2024-09-23T14:42:42Z"
     operators.operatorframework.io/builder: operator-sdk-v1.32.0
     operators.operatorframework.io/project_layout: go.kubebuilder.io/v3
     repository: https://github.com/Kuadrant/kuadrant-operator

bundle/manifests/kuadrant.io_dnspolicies.yaml

@@ -66,6 +66,15 @@ spec:
           spec:
             description: DNSPolicySpec defines the desired state of DNSPolicy
             properties:
+              excludeAddresses:
+                description: 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
+                items:
+                  type: string
+                maxItems: 20
+                type: array
               healthCheck:
                 description: |-
                   HealthCheckSpec configures health checks in the DNS provider.

config/crd/bases/kuadrant.io_dnspolicies.yaml

@@ -65,6 +65,15 @@ spec:
           spec:
             description: DNSPolicySpec defines the desired state of DNSPolicy
             properties:
+              excludeAddresses:
+                description: 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
+                items:
+                  type: string
+                maxItems: 20
+                type: array
               healthCheck:
                 description: |-
                   HealthCheckSpec configures health checks in the DNS provider.

controllers/dnspolicy_controller.go

@@ -133,7 +133,7 @@ func (r *DNSPolicyReconciler) reconcileResources(ctx context.Context, dnsPolicy
 	}
 
 	if err = r.reconcileDNSRecords(ctx, dnsPolicy, gatewayDiffObj); err != nil {
-		return fmt.Errorf("reconcile DNSRecords error %w", err)
+		return fmt.Errorf("error reconciling DNSRecords %w", err)
 	}
 
 	// set direct back ref - i.e. claim the target network object as taken asap

controllers/dnspolicy_dnsrecords.go

@@ -35,7 +35,7 @@ func (r *DNSPolicyReconciler) reconcileDNSRecords(ctx context.Context, dnsPolicy
 	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("error reconciling dns records for gateway %v: %w", gw.Gateway.Name, err)
+			return fmt.Errorf("reconciling dns records for gateway %v: error %w", gw.Gateway.Name, err)
 		}
 	}
 	return nil
@@ -52,6 +52,12 @@ func (r *DNSPolicyReconciler) reconcileGatewayDNSRecords(ctx context.Context, gw
 		return err
 	}
 
+	//ensure only approved addresses are considered for DNS records
+	if err := gatewayWrapper.SetValidStatusAddresses(dnsPolicy); err != nil {
+		log.V(3).Info("error setting valid addresses based on DNSPolicy")
+		return err
+	}
+
 	if err := r.dnsHelper.removeDNSForDeletedListeners(ctx, gatewayWrapper.Gateway); err != nil {
 		log.V(3).Info("error removing DNS for deleted listeners")
 		return err
@@ -98,6 +104,14 @@ func (r *DNSPolicyReconciler) reconcileGatewayDNSRecords(ctx context.Context, gw
 			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")
+		}
+
 		err = r.ReconcileResource(ctx, &kuadrantdnsv1alpha1.DNSRecord{}, dnsRecord, dnsRecordBasicMutator)
 		if err != nil && !apierrors.IsAlreadyExists(err) {
 			log.Error(err, "ReconcileResource failed to create/update DNSRecord resource")

examples/dnspolicy/dnspolicy-exclude-address.yaml

@@ -0,0 +1,20 @@
+apiVersion: kuadrant.io/v1alpha1
+kind: DNSPolicy
+metadata:
+  name: prod-web
+  namespace: ${DNSPOLICY_NAMESPACE}
+spec:
+  targetRef:
+    name: prod-web-istio
+    group: gateway.networking.k8s.io
+    kind: Gateway
+  providerRefs:
+    - name: aws-credentials
+  loadBalancing:
+    weight: 120
+    geo: EU
+    defaultGeo: true
+  excludeAddresses:
+    - "10.89.0.0/16"
+    - "some.local.domain"
+    - "127.0.0.1"

pkg/multicluster/gateway_wrapper.go

@@ -2,10 +2,13 @@ package multicluster
 
 import (
 	"fmt"
+	"net"
 	"strings"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1"
+
+	"github.com/kuadrant/kuadrant-operator/api/v1alpha1"
 )
 
 const (
@@ -24,6 +27,37 @@ func NewGatewayWrapper(g *gatewayapiv1.Gateway, clusterID string) *GatewayWrappe
 	return &GatewayWrapper{Gateway: g, ClusterID: clusterID}
 }
 
+func (g *GatewayWrapper) SetValidStatusAddresses(p *v1alpha1.DNSPolicy) error {
+	newAddresses := []gatewayapiv1.GatewayStatusAddress{}
+	for _, address := range g.Gateway.Status.Addresses {
+		found := false
+		for _, exclude := range p.Spec.ExcludeAddresses {
+			if strings.LastIndex(exclude, "/") == len(exclude)-3 {
+				_, network, err := net.ParseCIDR(exclude)
+				if err != nil {
+					return fmt.Errorf("could not parse the CIDR from the excludeAddresses field %w", err)
+				}
+				ip := net.ParseIP(address.Value)
+				// only check addresses that are actually IPs
+				if ip != nil && network.Contains(ip) {
+					found = true
+					break
+				}
+			}
+			if exclude == address.Value {
+				found = true
+				break
+			}
+		}
+		if !found {
+			newAddresses = append(newAddresses, address)
+		}
+	}
+	// setting this in memory only wont be saved to actual gateway
+	g.Status.Addresses = newAddresses
+	return nil
+}
+
 func isMultiClusterAddressType(addressType gatewayapiv1.AddressType) bool {
 	return addressType == MultiClusterIPAddressType || addressType == MultiClusterHostnameAddressType
 }

pkg/multicluster/gateway_wrapper_test.go

@@ -0,0 +1,132 @@
+package multicluster_test
+
+import (
+	"testing"
+
+	gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1"
+
+	"github.com/kuadrant/kuadrant-operator/api/v1alpha1"
+	"github.com/kuadrant/kuadrant-operator/pkg/multicluster"
+)
+
+func TestSetValidStatusAddresses(t *testing.T) {
+	ipaddress := gatewayapiv1.IPAddressType
+	hostaddress := gatewayapiv1.HostnameAddressType
+	testCases := []struct {
+		Name      string
+		Gateway   *gatewayapiv1.Gateway
+		DNSPolicy *v1alpha1.DNSPolicy
+		Validate  func(t *testing.T, g *gatewayapiv1.GatewayStatus)
+	}{
+		{
+			Name: "ensure addresses in ingore are are removed from status",
+			Gateway: &gatewayapiv1.Gateway{
+				Status: gatewayapiv1.GatewayStatus{
+					Addresses: []gatewayapiv1.GatewayStatusAddress{
+						{
+							Type:  &ipaddress,
+							Value: "1.1.1.1",
+						},
+						{
+							Type:  &hostaddress,
+							Value: "example.com",
+						},
+					},
+				},
+			},
+			DNSPolicy: &v1alpha1.DNSPolicy{
+				Spec: v1alpha1.DNSPolicySpec{
+					ExcludeAddresses: []string{
+						"1.1.1.1",
+					},
+				},
+			},
+			Validate: func(t *testing.T, g *gatewayapiv1.GatewayStatus) {
+				if len(g.Addresses) != 1 {
+					t.Fatalf("expected a single address but got %v ", len(g.Addresses))
+				}
+				for _, addr := range g.Addresses {
+					if addr.Value == "1.1.1.1" {
+						t.Fatalf("did not expect address %s to be present", "1.1.1.1")
+					}
+				}
+			},
+		},
+		{
+			Name: "ensure all addresses if nothing ignored",
+			Gateway: &gatewayapiv1.Gateway{
+				Status: gatewayapiv1.GatewayStatus{
+					Addresses: []gatewayapiv1.GatewayStatusAddress{
+						{
+							Type:  &ipaddress,
+							Value: "1.1.1.1",
+						},
+						{
+							Type:  &hostaddress,
+							Value: "example.com",
+						},
+					},
+				},
+			},
+			DNSPolicy: &v1alpha1.DNSPolicy{
+				Spec: v1alpha1.DNSPolicySpec{
+					ExcludeAddresses: []string{},
+				},
+			},
+			Validate: func(t *testing.T, g *gatewayapiv1.GatewayStatus) {
+				if len(g.Addresses) != 2 {
+					t.Fatalf("expected a both address but got %v ", len(g.Addresses))
+				}
+			},
+		},
+		{
+			Name: "ensure addresses removed if CIDR is set and hostname",
+			Gateway: &gatewayapiv1.Gateway{
+				Status: gatewayapiv1.GatewayStatus{
+					Addresses: []gatewayapiv1.GatewayStatusAddress{
+						{
+							Type:  &ipaddress,
+							Value: "1.1.1.1",
+						},
+						{
+							Type:  &hostaddress,
+							Value: "example.com",
+						},
+						{
+							Type:  &ipaddress,
+							Value: "81.17.21.22",
+						},
+					},
+				},
+			},
+			DNSPolicy: &v1alpha1.DNSPolicy{
+				Spec: v1alpha1.DNSPolicySpec{
+					ExcludeAddresses: []string{
+						"1.1.0.0/16",
+						"10.0.0.1/32",
+						"example.com",
+					},
+				},
+			},
+			Validate: func(t *testing.T, g *gatewayapiv1.GatewayStatus) {
+				if len(g.Addresses) != 1 {
+					t.Fatalf("expected only a single address but got %v %v ", len(g.Addresses), g.Addresses)
+				}
+				if g.Addresses[0].Value != "81.17.21.22" {
+					t.Fatalf("expected the only remaining address to be 81.17.21.22 but got %s", g.Addresses[0].Value)
+				}
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.Name, func(t *testing.T) {
+			gw := multicluster.NewGatewayWrapper(tc.Gateway, "id")
+			err := gw.SetValidStatusAddresses(tc.DNSPolicy)
+			if err != nil {
+				t.Fatalf("unexpected error %s", err)
+			}
+			tc.Validate(t, &gw.Status)
+		})
+	}
+}