From 82f84c1ee8507fb9880bad4379fd3c6b582a6e8c Mon Sep 17 00:00:00 2001
From: Moritz Wirth <27090954+mowirth@users.noreply.github.com>
Date: Mon, 16 Oct 2023 08:59:48 +0200
Subject: [PATCH] =?UTF-8?q?Fix=20issue=20where=20sealed=20secrets=20status?=
 =?UTF-8?q?=20is=20not=20updated=20if=20sealed=20secret=E2=80=A6=20(#1295)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Moritz Wirth <mw@flanga.io>
---
 pkg/controller/controller.go      | 25 ++++++++--------
 pkg/controller/controller_test.go | 47 +++++++++++++++++++++++++++++++
 2 files changed, 61 insertions(+), 11 deletions(-)

diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go
index 8ed389aa81..22655895c3 100644
--- a/pkg/controller/controller.go
+++ b/pkg/controller/controller.go
@@ -412,20 +412,18 @@ func (c *Controller) updateSealedSecretStatus(ssecret *ssv1alpha1.SealedSecret,
 		ssecret.Status = &ssv1alpha1.SealedSecretStatus{}
 	}
 
-	// No need to update the status if we already have observed it from the
-	// current generation of the resource.
-	if ssecret.Status.ObservedGeneration == ssecret.ObjectMeta.Generation {
-		return nil
-	}
-
 	ssecret.Status.ObservedGeneration = ssecret.ObjectMeta.Generation
-	updateSealedSecretsStatusConditions(ssecret.Status, unsealError)
+	updatedRequired := updateSealedSecretsStatusConditions(ssecret.Status, unsealError)
+	if updatedRequired {
+		_, err := c.ssclient.SealedSecrets(ssecret.GetObjectMeta().GetNamespace()).UpdateStatus(context.Background(), ssecret, metav1.UpdateOptions{})
+		return err
+	}
 
-	_, err := c.ssclient.SealedSecrets(ssecret.GetObjectMeta().GetNamespace()).UpdateStatus(context.Background(), ssecret, metav1.UpdateOptions{})
-	return err
+	return nil
 }
 
-func updateSealedSecretsStatusConditions(st *ssv1alpha1.SealedSecretStatus, unsealError error) {
+func updateSealedSecretsStatusConditions(st *ssv1alpha1.SealedSecretStatus, unsealError error) bool {
+	var updateRequired bool
 	cond := func() *ssv1alpha1.SealedSecretCondition {
 		for i := range st.Conditions {
 			if st.Conditions[i].Type == ssv1alpha1.SealedSecretSynced {
@@ -446,11 +444,16 @@ func updateSealedSecretsStatusConditions(st *ssv1alpha1.SealedSecretStatus, unse
 		status = corev1.ConditionFalse
 		cond.Message = unsealError.Error()
 	}
-	cond.LastUpdateTime = metav1.Now()
+
+	// Status has changed, update the transition time and signal that an update is required
 	if cond.Status != status {
 		cond.LastTransitionTime = cond.LastUpdateTime
 		cond.Status = status
+		cond.LastUpdateTime = metav1.Now()
+		updateRequired = true
 	}
+
+	return updateRequired
 }
 
 func isAnnotatedToBeManaged(secret *corev1.Secret) bool {
diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go
index 4046717d43..f7356c07a7 100644
--- a/pkg/controller/controller_test.go
+++ b/pkg/controller/controller_test.go
@@ -156,6 +156,53 @@ func TestSkipRecreateConfigDoesSkipIt(t *testing.T) {
 	}
 }
 
+func TestEmptyStatusSendsUpdate(t *testing.T) {
+	updateRequired := updateSealedSecretsStatusConditions(&ssv1alpha1.SealedSecretStatus{}, nil)
+
+	if !updateRequired {
+		t.Fatalf("expected status update, but no update was send")
+	}
+}
+
+func TestStatusUpdateSendsUpdate(t *testing.T) {
+	updateRequired := updateSealedSecretsStatusConditions(&ssv1alpha1.SealedSecretStatus{
+		Conditions: []ssv1alpha1.SealedSecretCondition{{
+			Status: "False",
+			Type:   ssv1alpha1.SealedSecretSynced,
+		}},
+	}, nil)
+
+	if !updateRequired {
+		t.Fatalf("expected status update, but no update was send")
+	}
+}
+
+func TestSameStatusNoUpdate(t *testing.T) {
+	updateRequired := updateSealedSecretsStatusConditions(&ssv1alpha1.SealedSecretStatus{
+		Conditions: []ssv1alpha1.SealedSecretCondition{{
+			Type:   ssv1alpha1.SealedSecretSynced,
+			Status: "False",
+		}},
+	}, errors.New("testerror"))
+
+	if updateRequired {
+		t.Fatalf("expected no status update, but update was send")
+	}
+}
+
+func TestSyncedSecretWithErrorSendsUpdate(t *testing.T) {
+	updateRequired := updateSealedSecretsStatusConditions(&ssv1alpha1.SealedSecretStatus{
+		Conditions: []ssv1alpha1.SealedSecretCondition{{
+			Type:   ssv1alpha1.SealedSecretSynced,
+			Status: "True",
+		}},
+	}, errors.New("testerror"))
+
+	if !updateRequired {
+		t.Fatalf("expected status update, but no update was send")
+	}
+}
+
 func testKeyRegister(t *testing.T, ctx context.Context, clientset kubernetes.Interface, ns string) *KeyRegistry {
 	t.Helper()