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

add integration test for excludeAddresses

updates post rebase

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 {
@@ -251,6 +256,13 @@ func (p *DNSPolicy) WithProviderSecret(s corev1.Secret) *DNSPolicy {
 	})
 }
 
+//excludeAddresses
+
+func (p *DNSPolicy) WithExcludeAddresses(excluded []string) *DNSPolicy {
+	p.Spec.ExcludeAddresses = excluded
+	return p
+}
+
 //TargetRef
 
 func (p *DNSPolicy) WithTargetGateway(gwName string) *DNSPolicy {

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/dns_helper.go

@@ -3,6 +3,8 @@ package controllers
 import (
 	"context"
 	"fmt"
+	"net"
+	"strings"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -97,6 +99,7 @@ func (dh *dnsHelper) deleteDNSRecordForListener(ctx context.Context, owner metav
 // GatewayWrapper is a wrapper for gateway to implement interface form the builder
 type GatewayWrapper struct {
 	*gatewayapiv1.Gateway
+	excludedAddresses []string
 }
 
 func NewGatewayWrapper(gateway *gatewayapiv1.Gateway) *GatewayWrapper {
@@ -113,3 +116,36 @@ func (g GatewayWrapper) GetAddresses() []builder.TargetAddress {
 	}
 	return addresses
 }
+
+func (g *GatewayWrapper) RemoveExcludedStatusAddresses(p *v1alpha1.DNSPolicy) error {
+	g.excludedAddresses = p.Spec.ExcludeAddresses
+	newAddresses := []gatewayapiv1.GatewayStatusAddress{}
+	for _, address := range g.Gateway.Status.Addresses {
+		found := false
+		for _, exclude := range p.Spec.ExcludeAddresses {
+			//Only a CIDR will have  / in the address so attempt to parse fail if not valid
+			if strings.Contains(exclude, "/") {
+				_, 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
+}

controllers/dns_helper_test.go

@@ -0,0 +1,167 @@
+package controllers_test
+
+import (
+	"testing"
+
+	gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1"
+
+	"github.com/kuadrant/kuadrant-operator/api/v1alpha1"
+	"github.com/kuadrant/kuadrant-operator/controllers"
+)
+
+func TestRemoveExcludedStatusAddresses(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)
+		ExpectErr bool
+	}{
+		{
+			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)
+				}
+			},
+		},
+		{
+			Name:      "ensure invalid CIDR causes error",
+			ExpectErr: true,
+			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/161",
+						"example.com",
+					},
+				},
+			},
+			Validate: func(t *testing.T, g *gatewayapiv1.GatewayStatus) {},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.Name, func(t *testing.T) {
+			gw := controllers.NewGatewayWrapper(tc.Gateway)
+			err := gw.RemoveExcludedStatusAddresses(tc.DNSPolicy)
+			if err != nil && !tc.ExpectErr {
+				t.Fatalf("unexpected error %s", err)
+			}
+			if tc.ExpectErr && err == nil {
+				t.Fatalf("expected an error but got none")
+			}
+			tc.Validate(t, &gw.Status)
+		})
+	}
+}

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

@@ -36,7 +36,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
@@ -48,6 +48,11 @@ func (r *DNSPolicyReconciler) reconcileGatewayDNSRecords(ctx context.Context, ga
 	if err != nil {
 		return fmt.Errorf("failed to generate cluster ID: %w", err)
 	}
+	gatewayWrapper := NewGatewayWrapper(gateway)
+	// 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, gateway); err != nil {
 		log.V(3).Info("error removing DNS for deleted listeners")
@@ -88,6 +93,14 @@ func (r *DNSPolicyReconciler) reconcileGatewayDNSRecords(ctx context.Context, ga
 			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"