add on-cluster healtcheck test and prevent enforced false on not healhy probes

api/v1/dnspolicy_types.go

@@ -322,9 +322,9 @@ func (p *DNSPolicy) WithTargetGatewayListener(gwName string, lName string) *DNSP
 func (p *DNSPolicy) WithHealthCheckFor(endpoint string, port int, protocol string, failureThreshold int) *DNSPolicy {
 	return p.WithHealthCheck(dnsv1alpha1.HealthCheckSpec{
 		Path:             endpoint,
-		Port:             &port,
+		Port:             port,
 		Protocol:         dnsv1alpha1.Protocol(protocol),
-		FailureThreshold: &failureThreshold,
+		FailureThreshold: failureThreshold,
 	})
 }
 

controllers/dns_workflow.go

@@ -1,6 +1,7 @@
 package controllers
 
 import (
+	"errors"
 	"fmt"
 	"sync"
 
@@ -19,13 +20,17 @@
 	"github.com/kuadrant/policy-machinery/machinery"
 
 	kuadrantv1 "github.com/kuadrant/kuadrant-operator/api/v1"
+	"github.com/kuadrant/kuadrant-operator/pkg/library/kuadrant"
 	"github.com/kuadrant/kuadrant-operator/pkg/library/utils"
 )
 
 const (
 	DNSRecordKind             = "DNSRecord"
 	StateDNSPolicyAcceptedKey = "DNSPolicyValid"
 	StateDNSPolicyErrorsKey   = "DNSPolicyErrors"
+
+	PolicyConditionSubResourcesHealthy gatewayapiv1alpha2.PolicyConditionType   = "SubResourcesHealthy"
+	PolicyReasonSubResourcesHealthy    gatewayapiv1alpha2.PolicyConditionReason = "SubResourcesHealthy"
 )
 
 var (
@@ -138,3 +143,27 @@
 		return p, ok
 	}
 }
+
+func dnsPolicyHealthyCondition(policy kuadrant.Policy, err kuadrant.PolicyError) *metav1.Condition {
+	cond := &metav1.Condition{
+		Type:    string(PolicyConditionSubResourcesHealthy),
+		Status:  metav1.ConditionTrue,
+		Reason:  string(PolicyReasonSubResourcesHealthy),
+		Message: fmt.Sprintf("all subresources of %s are healthy", policy.Kind()),
+	}
+
+	if err == nil {
+		return cond
+	}
+
+	// Wrap error into a PolicyError if it is not this type
+	var policyErr kuadrant.PolicyError
+	if !errors.As(err, &policyErr) {
+		policyErr = kuadrant.NewErrUnknown(policy.Kind(), err)
+	}
+	cond.Status = metav1.ConditionFalse
+	cond.Message = policyErr.Error()
+	cond.Reason = string(policyErr.Reason())
+
+	return cond
+}

controllers/dnspolicy_status_updater.go

@@ -93,6 +93,13 @@
 			}
 			meta.SetStatusCondition(&newStatus.Conditions, *enforcedCond)
 
+			if policy.Spec.HealthCheck != nil {
+				healthyCond := healthyCondition(policyRecords, policy)
+				meta.SetStatusCondition(&newStatus.Conditions, *healthyCond)
+			} else {
+				meta.RemoveStatusCondition(&newStatus.Conditions, string(PolicyConditionSubResourcesHealthy))
+			}
+
 			propagateRecordConditions(policyRecords, newStatus)
 
 			newStatus.TotalRecords = int32(len(policyRecords))
@@ -155,6 +162,36 @@
 	return kuadrant.EnforcedCondition(dnsPolicy, nil, true)
 }
 
+func healthyCondition(records []*kuadrantdnsv1alpha1.DNSRecord, dnsPolicy *kuadrantv1.DNSPolicy) *metav1.Condition {
+	// if we don't have records - consider healthy
+	if len(records) == 0 {
+		cond := dnsPolicyHealthyCondition(dnsPolicy, nil)
+		cond.Message = "No sub-resources present"
+		return cond
+	}
+
+	// filter not healthy records
+	notHealthyRecords := utils.Filter(records, func(record *kuadrantdnsv1alpha1.DNSRecord) bool {
+		return meta.IsStatusConditionFalse(record.Status.Conditions, string(kuadrantdnsv1alpha1.ConditionTypeHealthy))
+	})
+
+	// all records are healthy
+	if len(notHealthyRecords) == 0 {
+		cond := dnsPolicyHealthyCondition(dnsPolicy, nil)
+		cond.Message = "All sub-resources are healthy"
+		return cond
+	}
+
+	cond := dnsPolicyHealthyCondition(dnsPolicy, kuadrant.NewErrUnknown(kuadrantv1.DNSPolicyGroupKind.Kind, errors.New("not all sub-resources of policy are passing the policy defined health check")))
+	additionalMessage := ". Not healthy DNSRecords are: "
+	for _, record := range notHealthyRecords {
+		additionalMessage += fmt.Sprintf("%s ", record.Name)
+	}
+	cond.Message += additionalMessage
+
+	return cond
+}
+
 var NegativePolarityConditions []string
 
 func propagateRecordConditions(records []*kuadrantdnsv1alpha1.DNSRecord, policyStatus *kuadrantv1.DNSPolicyStatus) {

go.mod

@@ -11,7 +11,7 @@ require (
 	github.com/google/go-cmp v0.6.0
 	github.com/kuadrant/authorino v0.19.0
 	github.com/kuadrant/authorino-operator v0.11.1
-	github.com/kuadrant/dns-operator v0.0.0-20241018131559-f2ce8b6aaaef
+	github.com/kuadrant/dns-operator v0.0.0-20241108101824-a0cca97b2ef8
 	github.com/kuadrant/limitador-operator v0.9.0
 	github.com/kuadrant/policy-machinery v0.6.4
 	github.com/martinlindhe/base36 v1.1.1

go.sum

@@ -258,8 +258,8 @@ github.com/kuadrant/authorino v0.19.0 h1:vPkDWVgQPi5A91MfIyWe06rLW0ZaACd+8j2SZPK
 github.com/kuadrant/authorino v0.19.0/go.mod h1:J9B/8aZMSyim6CxdmSlS2kRfQt3xMrZub2V/qHnkNeA=
 github.com/kuadrant/authorino-operator v0.11.1 h1:jndTZhiHMU+2Dk0NU+KP2+MUSfvclrn+YtTCQDJj+1s=
 github.com/kuadrant/authorino-operator v0.11.1/go.mod h1:TeFFdX477vUTMushCojaHpvwPLga4DpErGI2oQbqFIs=
-github.com/kuadrant/dns-operator v0.0.0-20241018131559-f2ce8b6aaaef h1:6P2pC1kOPcrT/22N23Mr3xr3CTHmQQkj3jmOlUEJvO0=
-github.com/kuadrant/dns-operator v0.0.0-20241018131559-f2ce8b6aaaef/go.mod h1:LGG4R3KEz93Ep0CV1/tziCmRk+VtojWUHR9mXkOHZks=
+github.com/kuadrant/dns-operator v0.0.0-20241108101824-a0cca97b2ef8 h1:z+pdlUIPjLCeytxmL5/NcORm3elZDAP8i+MLIpMba9U=
+github.com/kuadrant/dns-operator v0.0.0-20241108101824-a0cca97b2ef8/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.4 h1:UMdZ2p7WyUdOKcWlJA2w2MzJnB8/Nn4dT6hE9cUcbeg=

tests/common/dnspolicy/dnspolicy_controller_test.go

@@ -6,13 +6,13 @@ import (
 	"fmt"
 	"time"
 
+	"github.com/kuadrant/kuadrant-operator/controllers"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	. "github.com/onsi/gomega/gstruct"
 	"github.com/onsi/gomega/types"
 	"k8s.io/apimachinery/pkg/util/rand"
 
-	kuadrantdnsv1alpha1 "github.com/kuadrant/dns-operator/api/v1alpha1"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/utils/ptr"
@@ -21,6 +21,7 @@ import (
 	gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1"
 	gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2"
 
+	kuadrantdnsv1alpha1 "github.com/kuadrant/dns-operator/api/v1alpha1"
 	kuadrantv1 "github.com/kuadrant/kuadrant-operator/api/v1"
 	"github.com/kuadrant/kuadrant-operator/pkg/common"
 	"github.com/kuadrant/kuadrant-operator/pkg/library/kuadrant"
@@ -1244,6 +1245,143 @@ var _ = Describe("DNSPolicy controller", func() {
 
 	})
 
+	// there is no need to replicate cases form the "valid target and valid gateway status" context
+	// from the policy pov healthchecks can only affect the "SubResourcesHealthy" condition
+	Context("valid target and valid gateway status with healthchecks", func() {
+		BeforeEach(func(ctx SpecContext) {
+			gateway = tests.NewGatewayBuilder(tests.GatewayName, gatewayClass.Name, testNamespace).
+				WithHTTPListener(tests.ListenerNameOne, tests.HostOne(domain)).
+				WithHTTPListener(tests.ListenerNameWildcard, tests.HostWildcard(domain)).
+				Gateway
+			dnsPolicy = tests.NewDNSPolicy("test-dns-policy", testNamespace).
+				WithProviderSecret(*dnsProviderSecret).
+				WithTargetGateway(tests.GatewayName).
+				WithLoadBalancingFor(100, "foo", true).
+				WithHealthCheckFor("/health", 80, string(kuadrantdnsv1alpha1.HttpProtocol), 1)
+
+			Expect(k8sClient.Create(ctx, gateway)).To(Succeed())
+			Expect(k8sClient.Create(ctx, dnsPolicy)).To(Succeed())
+
+			Eventually(func(g Gomega) {
+				g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(gateway), gateway)).To(Succeed())
+				gateway.Status.Addresses = []gatewayapiv1.GatewayStatusAddress{
+					{
+						Type:  ptr.To(gatewayapiv1.IPAddressType),
+						Value: tests.IPAddressOne,
+					},
+					{
+						Type:  ptr.To(gatewayapiv1.IPAddressType),
+						Value: tests.IPAddressTwo,
+					},
+				}
+				gateway.Status.Listeners = []gatewayapiv1.ListenerStatus{
+					{
+						Name:           tests.ListenerNameOne,
+						SupportedKinds: []gatewayapiv1.RouteGroupKind{},
+						AttachedRoutes: 1,
+						Conditions:     []metav1.Condition{},
+					},
+					{
+						Name:           tests.ListenerNameWildcard,
+						SupportedKinds: []gatewayapiv1.RouteGroupKind{},
+						AttachedRoutes: 1,
+						Conditions:     []metav1.Condition{},
+					},
+				}
+				g.Expect(k8sClient.Status().Update(ctx, gateway)).To(Succeed())
+			}, tests.TimeoutMedium, tests.RetryIntervalMedium).Should(Succeed())
+
+			recordName = fmt.Sprintf("%s-%s", tests.GatewayName, tests.ListenerNameOne)
+			wildcardRecordName = fmt.Sprintf("%s-%s", tests.GatewayName, tests.ListenerNameWildcard)
+		})
+
+		It("should create records with enforced and not healthy status", func(ctx SpecContext) {
+			Eventually(func(g Gomega) {
+				//Check records
+				recordList := &kuadrantdnsv1alpha1.DNSRecordList{}
+				err := k8sClient.List(ctx, recordList, &client.ListOptions{Namespace: testNamespace})
+				g.Expect(err).NotTo(HaveOccurred())
+				g.Expect(recordList.Items).To(HaveLen(2))
+
+				// This record should not be ready - we are not publishing unhealthy EPs
+				dnsRecord := &kuadrantdnsv1alpha1.DNSRecord{}
+				g.Expect(k8sClient.Get(ctx, client.ObjectKey{Name: recordName, Namespace: testNamespace}, dnsRecord)).To(Succeed())
+				g.Expect(dnsRecord.Status.Conditions).To(
+					ContainElements(
+						MatchFields(IgnoreExtras, Fields{
+							"Type":    Equal(string(kuadrantdnsv1alpha1.ConditionTypeReady)),
+							"Status":  Equal(metav1.ConditionFalse),
+							"Reason":  Equal(string(kuadrantdnsv1alpha1.ConditionReasonUnhealthy)),
+							"Message": Equal("Not publishing unhealthy records"),
+						}),
+						MatchFields(IgnoreExtras, Fields{
+							"Type":   Equal(string(kuadrantdnsv1alpha1.ConditionTypeHealthy)),
+							"Status": Equal(metav1.ConditionFalse),
+							"Reason": Equal(string(kuadrantdnsv1alpha1.ConditionReasonUnhealthy)),
+							"Message": And(
+								ContainSubstring("Not healthy addresses"),
+								ContainSubstring(tests.IPAddressOne),
+								ContainSubstring(tests.IPAddressTwo)),
+						}),
+					),
+				)
+
+				// This record should be ready - we are not creating checks for wildcards
+				wildcardDnsRecord := &kuadrantdnsv1alpha1.DNSRecord{}
+				g.Expect(k8sClient.Get(ctx, client.ObjectKey{Name: wildcardRecordName, Namespace: testNamespace}, wildcardDnsRecord)).To(Succeed())
+				g.Expect(wildcardDnsRecord.Status.Conditions).To(
+					And(
+						ContainElement(
+							MatchFields(IgnoreExtras, Fields{
+								"Type":    Equal(string(kuadrantdnsv1alpha1.ConditionTypeReady)),
+								"Status":  Equal(metav1.ConditionTrue),
+								"Reason":  Equal(string(kuadrantdnsv1alpha1.ConditionReasonProviderSuccess)),
+								"Message": Equal("Provider ensured the dns record"),
+							}),
+						),
+						Not(ContainElement(
+							MatchFields(IgnoreExtras, Fields{
+								"Type": Equal(string(kuadrantdnsv1alpha1.ConditionTypeHealthy)),
+							}),
+						)),
+					),
+				)
+
+				//Check policy status
+				err = k8sClient.Get(ctx, client.ObjectKeyFromObject(dnsPolicy), dnsPolicy)
+				g.Expect(err).NotTo(HaveOccurred())
+				g.Expect(dnsPolicy.Status.Conditions).To(
+					ContainElements(
+						MatchFields(IgnoreExtras, Fields{
+							"Type":    Equal(string(gatewayapiv1alpha2.PolicyConditionAccepted)),
+							"Status":  Equal(metav1.ConditionTrue),
+							"Reason":  Equal(string(gatewayapiv1alpha2.PolicyConditionAccepted)),
+							"Message": Equal("DNSPolicy has been accepted"),
+						}),
+						MatchFields(IgnoreExtras, Fields{
+							"Type":   Equal(string(kuadrant.PolicyConditionEnforced)),
+							"Status": Equal(metav1.ConditionTrue),
+							"Reason": Equal(string(kuadrant.PolicyReasonEnforced)),
+							"Message": And(
+								ContainSubstring("DNSPolicy has been partially enforced. Not ready DNSRecords are:"),
+								ContainSubstring(recordName),
+								Not(ContainSubstring(wildcardRecordName))),
+						}),
+						MatchFields(IgnoreExtras, Fields{
+							"Type":   Equal(string(controllers.PolicyConditionSubResourcesHealthy)),
+							"Status": Equal(metav1.ConditionFalse),
+							"Reason": Equal(string(kuadrant.PolicyReasonUnknown)),
+							"Message": And(
+								ContainSubstring("DNSPolicy has encountered some issues: not all sub-resources of policy are passing the policy defined health check. Not healthy DNSRecords are:"),
+								ContainSubstring(recordName),
+								Not(ContainSubstring(wildcardRecordName))), // explicitly make sure that we have no probes for the wildcard record
+						})),
+				)
+				g.Expect(dnsPolicy.Status.TotalRecords).To(Equal(int32(2)))
+			}, tests.TimeoutLong, tests.RetryIntervalMedium, ctx).Should(Succeed())
+		}, testTimeOut)
+	})
+
 	Context("cel validation", func() {
 		It("should error targeting invalid group", func(ctx SpecContext) {
 			p := tests.NewDNSPolicy("test-dns-policy", testNamespace).