From 2a4be1cb397e70d641a40ab20a05537b0e90d42c Mon Sep 17 00:00:00 2001
From: Felix Kunde <felix-kunde@gmx.de>
Date: Fri, 14 Feb 2025 09:44:09 +0100
Subject: [PATCH] fix creating secrets for rotation users (#2863)

* fix creating secrets for rotation users
* rework annotation comparison on update to decide on when to call syncSecrets
---
 e2e/tests/test_e2e.py  | 25 +++++++++++++++++++++++--
 pkg/cluster/cluster.go | 17 ++++++++++++-----
 2 files changed, 35 insertions(+), 7 deletions(-)

diff --git a/e2e/tests/test_e2e.py b/e2e/tests/test_e2e.py
index febf4a374..b9a2a27d4 100644
--- a/e2e/tests/test_e2e.py
+++ b/e2e/tests/test_e2e.py
@@ -1752,9 +1752,13 @@ def test_password_rotation(self):
            Test password rotation and removal of users due to retention policy
         '''
         k8s = self.k8s
+        cluster_label = 'application=spilo,cluster-name=acid-minimal-cluster'
         leader = k8s.get_cluster_leader_pod()
         today = date.today()
 
+        # remember number of secrets to make sure it stays the same
+        secret_count = k8s.count_secrets_with_label(cluster_label)
+
         # enable password rotation for owner of foo database
         pg_patch_rotation_single_users = {
             "spec": {
@@ -1810,6 +1814,7 @@ def test_password_rotation(self):
         enable_password_rotation = {
             "data": {
                 "enable_password_rotation": "true",
+                "inherited_annotations": "environment",
                 "password_rotation_interval": "30",
                 "password_rotation_user_retention": "30",  # should be set to 60 
             },
@@ -1856,13 +1861,29 @@ def test_password_rotation(self):
         self.eventuallyEqual(lambda: len(self.query_database_with_user(leader.metadata.name, "postgres", "SELECT 1", "foo_user")), 1,
             "Could not connect to the database with rotation user {}".format(rotation_user), 10, 5)
 
+        # add annotation which triggers syncSecrets call
+        pg_annotation_patch = {
+            "metadata": {
+                "annotations": {
+                    "environment": "test",
+                }
+            }
+        }
+        k8s.api.custom_objects_api.patch_namespaced_custom_object(
+            "acid.zalan.do", "v1", "default", "postgresqls", "acid-minimal-cluster", pg_annotation_patch)
+        self.eventuallyEqual(lambda: k8s.get_operator_state(), {"0": "idle"}, "Operator does not get in sync")
+        time.sleep(10)
+        self.eventuallyEqual(lambda: k8s.count_secrets_with_label(cluster_label), secret_count, "Unexpected number of secrets")
+
         # check if rotation has been ignored for user from test_cross_namespace_secrets test
         db_user_secret = k8s.get_secret(username="test.db_user", namespace="test")
         secret_username = str(base64.b64decode(db_user_secret.data["username"]), 'utf-8')
-
         self.assertEqual("test.db_user", secret_username,
                         "Unexpected username in secret of test.db_user: expected {}, got {}".format("test.db_user", secret_username))
 
+        # check if annotation for secret has been updated
+        self.assertTrue("environment" in db_user_secret.metadata.annotations, "Added annotation was not propagated to secret")
+
         # disable password rotation for all other users (foo_user)
         # and pick smaller intervals to see if the third fake rotation user is dropped 
         enable_password_rotation = {
@@ -2100,7 +2121,7 @@ def test_statefulset_annotation_propagation(self):
         patch_sset_propagate_annotations = {
             "data": {
                 "downscaler_annotations": "deployment-time,downscaler/*",
-                "inherited_annotations": "owned-by",
+                "inherited_annotations": "environment,owned-by",
             }
         }
         k8s.update_config(patch_sset_propagate_annotations)
diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go
index e2b53a7ce..a839397b2 100644
--- a/pkg/cluster/cluster.go
+++ b/pkg/cluster/cluster.go
@@ -1034,10 +1034,18 @@ func (c *Cluster) Update(oldSpec, newSpec *acidv1.Postgresql) error {
 		// only when streams were not specified in oldSpec but in newSpec
 		needStreamUser := len(oldSpec.Spec.Streams) == 0 && len(newSpec.Spec.Streams) > 0
 
-		annotationsChanged, _ := c.compareAnnotations(oldSpec.Annotations, newSpec.Annotations, nil)
-
 		initUsers := !sameUsers || !sameRotatedUsers || needPoolerUser || needStreamUser
-		if initUsers {
+
+		// if inherited annotations differ secrets have to be synced on update
+		newAnnotations := c.annotationsSet(nil)
+		oldAnnotations := make(map[string]string)
+		for _, secret := range c.Secrets {
+			oldAnnotations = secret.ObjectMeta.Annotations
+			break
+		}
+		annotationsChanged, _ := c.compareAnnotations(oldAnnotations, newAnnotations, nil)
+
+		if initUsers || annotationsChanged {
 			c.logger.Debug("initialize users")
 			if err := c.initUsers(); err != nil {
 				c.logger.Errorf("could not init users - skipping sync of secrets and databases: %v", err)
@@ -1045,8 +1053,7 @@ func (c *Cluster) Update(oldSpec, newSpec *acidv1.Postgresql) error {
 				updateFailed = true
 				return
 			}
-		}
-		if initUsers || annotationsChanged {
+
 			c.logger.Debug("syncing secrets")
 			//TODO: mind the secrets of the deleted/new users
 			if err := c.syncSecrets(); err != nil {