Merge pull request #1877 from officialasishkumar/issues/1876

fix: restore TLSPolicy reconciliation on issuer readiness changes

Author: thomasmaas

internal/controller/state_of_the_world.go

@@ -785,25 +785,13 @@ func certManagerControllerOpts() []controller.ControllerOption {
 			&certmanagerv1.Issuer{},
 			CertManagerIssuersResource,
 			metav1.NamespaceAll,
-			controller.WithPredicates(ctrlruntimepredicate.TypedFuncs[*certmanagerv1.Issuer]{
-				UpdateFunc: func(e event.TypedUpdateEvent[*certmanagerv1.Issuer]) bool {
-					oldStatus := e.ObjectOld.GetStatus()
-					newStatus := e.ObjectOld.GetStatus()
-					return !reflect.DeepEqual(oldStatus, newStatus)
-				},
-			})),
+			controller.WithPredicates(issuerStatusChangedPredicate())),
 		),
 		controller.WithRunnable("clusterissuers watcher", controller.Watch(
 			&certmanagerv1.ClusterIssuer{},
 			CertMangerClusterIssuersResource,
 			metav1.NamespaceAll,
-			controller.WithPredicates(ctrlruntimepredicate.TypedFuncs[*certmanagerv1.ClusterIssuer]{
-				UpdateFunc: func(e event.TypedUpdateEvent[*certmanagerv1.ClusterIssuer]) bool {
-					oldStatus := e.ObjectOld.GetStatus()
-					newStatus := e.ObjectOld.GetStatus()
-					return !reflect.DeepEqual(oldStatus, newStatus)
-				},
-			})),
+			controller.WithPredicates(clusterIssuerStatusChangedPredicate())),
 		),
 		controller.WithObjectKinds(
 			CertManagerCertificateKind,
@@ -818,6 +806,22 @@ func certManagerControllerOpts() []controller.ControllerOption {
 	}
 }
 
+func issuerStatusChangedPredicate() ctrlruntimepredicate.TypedPredicate[*certmanagerv1.Issuer] {
+	return ctrlruntimepredicate.TypedFuncs[*certmanagerv1.Issuer]{
+		UpdateFunc: func(e event.TypedUpdateEvent[*certmanagerv1.Issuer]) bool {
+			return !reflect.DeepEqual(e.ObjectOld.GetStatus(), e.ObjectNew.GetStatus())
+		},
+	}
+}
+
+func clusterIssuerStatusChangedPredicate() ctrlruntimepredicate.TypedPredicate[*certmanagerv1.ClusterIssuer] {
+	return ctrlruntimepredicate.TypedFuncs[*certmanagerv1.ClusterIssuer]{
+		UpdateFunc: func(e event.TypedUpdateEvent[*certmanagerv1.ClusterIssuer]) bool {
+			return !reflect.DeepEqual(e.ObjectOld.GetStatus(), e.ObjectNew.GetStatus())
+		},
+	}
+}
+
 func initWorkflow(client *dynamic.DynamicClient) *controller.Workflow {
 	return &controller.Workflow{
 		Precondition: traceReconcileFunc("init.event_logger", NewEventLogger().Log),

internal/controller/state_of_the_world_test.go

@@ -7,8 +7,11 @@ import (
 	"testing"
 	"time"
 
+	certmanagerv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1"
+	certmanmetav1 "github.com/cert-manager/cert-manager/pkg/apis/meta/v1"
 	"github.com/kuadrant/policy-machinery/machinery"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"sigs.k8s.io/controller-runtime/pkg/event"
 
 	kuadrantv1beta1 "github.com/kuadrant/kuadrant-operator/api/v1beta1"
 )
@@ -109,3 +112,101 @@ func TestGetKuadrant(t *testing.T) {
 		})
 	}
 }
+
+func TestIssuerStatusChangedPredicate(t *testing.T) {
+	ready := certmanagerv1.IssuerStatus{
+		Conditions: []certmanagerv1.IssuerCondition{{
+			Type:   certmanagerv1.IssuerConditionReady,
+			Status: certmanmetav1.ConditionTrue,
+		}},
+	}
+	notReady := certmanagerv1.IssuerStatus{
+		Conditions: []certmanagerv1.IssuerCondition{{
+			Type:   certmanagerv1.IssuerConditionReady,
+			Status: certmanmetav1.ConditionFalse,
+		}},
+	}
+
+	predicate := issuerStatusChangedPredicate()
+
+	tests := []struct {
+		name string
+		old  certmanagerv1.IssuerStatus
+		new  certmanagerv1.IssuerStatus
+		want bool
+	}{
+		{
+			name: "status change triggers reconciliation",
+			old:  notReady,
+			new:  ready,
+			want: true,
+		},
+		{
+			name: "unchanged status is ignored",
+			old:  ready,
+			new:  ready,
+			want: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := predicate.Update(event.TypedUpdateEvent[*certmanagerv1.Issuer]{
+				ObjectOld: &certmanagerv1.Issuer{Status: tt.old},
+				ObjectNew: &certmanagerv1.Issuer{Status: tt.new},
+			})
+			if got != tt.want {
+				t.Fatalf("issuerStatusChangedPredicate().Update() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestClusterIssuerStatusChangedPredicate(t *testing.T) {
+	ready := certmanagerv1.IssuerStatus{
+		Conditions: []certmanagerv1.IssuerCondition{{
+			Type:   certmanagerv1.IssuerConditionReady,
+			Status: certmanmetav1.ConditionTrue,
+		}},
+	}
+	notReady := certmanagerv1.IssuerStatus{
+		Conditions: []certmanagerv1.IssuerCondition{{
+			Type:   certmanagerv1.IssuerConditionReady,
+			Status: certmanmetav1.ConditionFalse,
+		}},
+	}
+
+	predicate := clusterIssuerStatusChangedPredicate()
+
+	tests := []struct {
+		name string
+		old  certmanagerv1.IssuerStatus
+		new  certmanagerv1.IssuerStatus
+		want bool
+	}{
+		{
+			name: "status change triggers reconciliation",
+			old:  notReady,
+			new:  ready,
+			want: true,
+		},
+		{
+			name: "unchanged status is ignored",
+			old:  ready,
+			new:  ready,
+			want: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := predicate.Update(event.TypedUpdateEvent[*certmanagerv1.ClusterIssuer]{
+				ObjectOld: &certmanagerv1.ClusterIssuer{Status: tt.old},
+				ObjectNew: &certmanagerv1.ClusterIssuer{Status: tt.new},
+			})
+			if got != tt.want {
+				t.Fatalf("clusterIssuerStatusChangedPredicate().Update() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}