From 3ebb6284ccf23bf5e4a5a34365bbb99bbe510e0f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Charles-Edouard=20Br=C3=A9t=C3=A9ch=C3=A9?=
 <charled.breteche@gmail.com>
Date: Mon, 17 Oct 2022 07:52:54 +0200
Subject: [PATCH] refactor: add update status helper (#4985)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>
Co-authored-by: shuting <shuting@nirmata.com>
---
 pkg/controllers/webhook/controller.go | 58 +++++++++++++++------------
 pkg/utils/controller/utils.go         | 18 +++++++++
 2 files changed, 51 insertions(+), 25 deletions(-)

diff --git a/pkg/controllers/webhook/controller.go b/pkg/controllers/webhook/controller.go
index 63fef3f0b5..1feef8b3a9 100644
--- a/pkg/controllers/webhook/controller.go
+++ b/pkg/controllers/webhook/controller.go
@@ -412,7 +412,7 @@ func (c *controller) updatePolicyStatuses(ctx context.Context) error {
 	if err != nil {
 		return err
 	}
-	for _, policy := range policies {
+	updateStatusFunc := func(policy kyvernov1.PolicyInterface) error {
 		policyKey, err := cache.MetaNamespaceKeyFunc(policy)
 		if err != nil {
 			return err
@@ -421,35 +421,43 @@ func (c *controller) updatePolicyStatuses(ctx context.Context) error {
 		for _, set := range c.policyState {
 			if !set.Has(policyKey) {
 				ready = false
+				break
 			}
 		}
-		if policy.IsReady() != ready {
-			policy = policy.CreateDeepCopy()
-			status := policy.GetStatus()
-			status.SetReady(ready)
-			if toggle.AutogenInternals.Enabled() {
-				var rules []kyvernov1.Rule
-				for _, rule := range autogen.ComputeRules(policy) {
-					if strings.HasPrefix(rule.Name, "autogen-") {
-						rules = append(rules, rule)
-					}
-				}
-				status.Autogen.Rules = rules
-			} else {
-				status.Autogen.Rules = nil
-			}
-			if policy.GetNamespace() == "" {
-				_, err := c.kyvernoClient.KyvernoV1().ClusterPolicies().UpdateStatus(ctx, policy.(*kyvernov1.ClusterPolicy), metav1.UpdateOptions{})
-				if err != nil {
-					return err
-				}
-			} else {
-				_, err := c.kyvernoClient.KyvernoV1().Policies(policy.GetNamespace()).UpdateStatus(ctx, policy.(*kyvernov1.Policy), metav1.UpdateOptions{})
-				if err != nil {
-					return err
+		status := policy.GetStatus()
+		status.SetReady(ready)
+		status.Autogen.Rules = nil
+		if toggle.AutogenInternals.Enabled() {
+			for _, rule := range autogen.ComputeRules(policy) {
+				if strings.HasPrefix(rule.Name, "autogen-") {
+					status.Autogen.Rules = append(status.Autogen.Rules, rule)
 				}
 			}
 		}
+		return nil
+	}
+	for _, policy := range policies {
+		if policy.GetNamespace() == "" {
+			_, err := controllerutils.UpdateStatus(
+				ctx,
+				policy.(*kyvernov1.ClusterPolicy),
+				c.kyvernoClient.KyvernoV1().ClusterPolicies(),
+				func(policy *kyvernov1.ClusterPolicy) error {
+					return updateStatusFunc(policy)
+				},
+			)
+			return err
+		} else {
+			_, err := controllerutils.UpdateStatus(
+				ctx,
+				policy.(*kyvernov1.Policy),
+				c.kyvernoClient.KyvernoV1().Policies(policy.GetNamespace()),
+				func(policy *kyvernov1.Policy) error {
+					return updateStatusFunc(policy)
+				},
+			)
+			return err
+		}
 	}
 	return nil
 }
diff --git a/pkg/utils/controller/utils.go b/pkg/utils/controller/utils.go
index e590b10da7..1adf751c44 100644
--- a/pkg/utils/controller/utils.go
+++ b/pkg/utils/controller/utils.go
@@ -148,6 +148,24 @@ func Update[T interface {
 	}
 }
 
+func UpdateStatus[T interface {
+	metav1.Object
+	DeepCopy[T]
+}, S StatusClient[T]](ctx context.Context, obj T, setter S, build func(T) error,
+) (T, error) {
+	mutated := obj.DeepCopy()
+	if err := build(mutated); err != nil {
+		var d T
+		return d, err
+	} else {
+		if reflect.DeepEqual(obj, mutated) {
+			return mutated, nil
+		} else {
+			return setter.UpdateStatus(ctx, mutated, metav1.UpdateOptions{})
+		}
+	}
+}
+
 func Cleanup[T any, R Object[T]](ctx context.Context, actual []R, expected []R, deleter Deleter) error {
 	keep := sets.NewString()
 	for _, obj := range expected {