copy rolconfig during password rotation (#2183)

* copy rolconfig during password rotation

Co-authored-by: idanovinda <idanovinda@gmail.com>

e2e/tests/test_e2e.py

@@ -932,7 +932,8 @@ class EndToEndTestCase(unittest.TestCase):
                             "AdminRole": "",
                             "Origin": 2,
                             "IsDbOwner": False,
-                            "Deleted": False
+                            "Deleted": False,
+                            "Rotated": False
                         })
                         return True
                 except:
@@ -1472,9 +1473,9 @@ class EndToEndTestCase(unittest.TestCase):
         # create fake rotation users that should be removed by operator
         # but have one that would still fit into the retention period
         create_fake_rotation_user = """
-            CREATE ROLE foo_user201031 IN ROLE foo_user;
+            CREATE USER foo_user201031 IN ROLE foo_user;
-            CREATE ROLE foo_user211031 IN ROLE foo_user;
+            CREATE USER foo_user211031 IN ROLE foo_user;
-            CREATE ROLE foo_user"""+(today-timedelta(days=40)).strftime("%y%m%d")+""" IN ROLE foo_user;
+            CREATE USER foo_user"""+(today-timedelta(days=40)).strftime("%y%m%d")+""" IN ROLE foo_user;
         """
         self.query_database(leader.metadata.name, "postgres", create_fake_rotation_user)
 
@@ -1491,6 +1492,12 @@ class EndToEndTestCase(unittest.TestCase):
             namespace="default",
             body=secret_fake_rotation)
 
+        # update rolconfig for foo_user that will be copied for new rotation user
+        alter_foo_user_search_path = """
+            ALTER ROLE foo_user SET search_path TO data;
+        """
+        self.query_database(leader.metadata.name, "postgres", alter_foo_user_search_path)
+
         # enable password rotation for all other users (foo_user)
         # this will force a sync of secrets for further assertions
         enable_password_rotation = {
@@ -1526,6 +1533,18 @@ class EndToEndTestCase(unittest.TestCase):
         self.eventuallyEqual(lambda: len(self.query_database(leader.metadata.name, "postgres", user_query)), 3,
             "Found incorrect number of rotation users", 10, 5)
 
+        # check if rolconfig was passed from foo_user to foo_user+today
+        # and that no foo_user has been deprecated (can still login)
+        user_query = """
+            SELECT rolname
+              FROM pg_catalog.pg_roles
+             WHERE rolname LIKE 'foo_user%'
+               AND rolconfig = ARRAY['search_path=data']::text[]
+               AND rolcanlogin;
+        """
+        self.eventuallyEqual(lambda: len(self.query_database(leader.metadata.name, "postgres", user_query)), 2,
+            "Rolconfig not applied to new rotation user", 10, 5)
+
         # test that rotation_user can connect to the database
         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)
@@ -1559,7 +1578,7 @@ class EndToEndTestCase(unittest.TestCase):
              WHERE rolname LIKE 'foo_user%';
         """
         self.eventuallyEqual(lambda: len(self.query_database(leader.metadata.name, "postgres", user_query)), 2,
-            "Found incorrect number of rotation users", 10, 5)
+            "Found incorrect number of rotation users after disabling password rotation", 10, 5)
 
     @timeout_decorator.timeout(TEST_TIMEOUT_SEC)
     def test_rolling_update_flag(self):

pkg/cluster/database.go

@@ -284,7 +284,7 @@ func (c *Cluster) cleanupRotatedUsers(rotatedUsers []string, db *sql.DB) error {
 	retentionDate := time.Now().AddDate(0, 0, int(retenionDays)*-1)
 
 	for rotatedUser, dateSuffix := range extraUsers {
-		userCreationDate, err := time.Parse("060102", dateSuffix)
+		userCreationDate, err := time.Parse(constants.RotationUserDateFormat, dateSuffix)
 		if err != nil {
 			c.logger.Errorf("could not parse creation date suffix of user %q: %v", rotatedUser, err)
 			continue

pkg/cluster/sync.go

@@ -656,7 +656,6 @@ func (c *Cluster) checkAndSetGlobalPostgreSQLConfiguration(pod *v1.Pod, effectiv
 }
 
 func (c *Cluster) syncSecrets() error {
-
 	c.logger.Info("syncing secrets")
 	c.setProcessName("syncing secrets")
 	generatedSecrets := c.generateUserSecrets()
@@ -792,6 +791,7 @@ func (c *Cluster) updateSecret(
 		pwdUser.Password = string(secret.Data["password"])
 		// update membership if we deal with a rotation user
 		if secretUsername != pwdUser.Name {
+			pwdUser.Rotated = true
 			pwdUser.MemberOf = []string{secretUsername}
 		}
 		userMap[userKey] = pwdUser
@@ -842,7 +842,7 @@ func (c *Cluster) rotatePasswordInSecret(
 	if currentTime.After(nextRotationDate) {
 		// create rotation user if role is not listed for in-place password update
 		if !util.SliceContains(c.Spec.UsersWithInPlaceSecretRotation, secretUsername) {
-			rotationUsername := fmt.Sprintf("%s%s", secretUsername, currentTime.Format("060102"))
+			rotationUsername := fmt.Sprintf("%s%s", secretUsername, currentTime.Format(constants.RotationUserDateFormat))
 			secret.Data["username"] = []byte(rotationUsername)
 			c.logger.Infof("updating username in secret %s and creating rotation user %s in the database", secretName, rotationUsername)
 			// whenever there is a rotation, check if old rotation users can be deleted
@@ -924,6 +924,12 @@ func (c *Cluster) syncRoles() (err error) {
 	for _, u := range c.pgUsers {
 		pgRole := u.Name
 		userNames = append(userNames, pgRole)
+
+		// when a rotation happened add group role to query its rolconfig
+		if u.Rotated {
+			userNames = append(userNames, u.MemberOf[0])
+		}
+
 		// add team member role name with rename suffix in case we need to rename it back
 		if u.Origin == spec.RoleOriginTeamsAPI && c.OpConfig.EnableTeamMemberDeprecation {
 			deletedUsers[pgRole+c.OpConfig.RoleDeletionSuffix] = pgRole
@@ -950,9 +956,21 @@ func (c *Cluster) syncRoles() (err error) {
 		return fmt.Errorf("error getting users from the database: %v", err)
 	}
 
-	// update pgUsers where a deleted role was found
+DBUSERS:
-	// so that they are skipped in ProduceSyncRequests
 	for _, dbUser := range dbUsers {
+		// copy rolconfig to rotation users
+		for pgUserName, pgUser := range c.pgUsers {
+			if pgUser.Rotated && pgUser.MemberOf[0] == dbUser.Name {
+				pgUser.Parameters = dbUser.Parameters
+				c.pgUsers[pgUserName] = pgUser
+				// remove group role from dbUsers to not count as deleted role
+				delete(dbUsers, dbUser.Name)
+				continue DBUSERS
+			}
+		}
+
+		// update pgUsers where a deleted role was found
+		// so that they are skipped in ProduceSyncRequests
 		originalUsername, foundDeletedUser := deletedUsers[dbUser.Name]
 		// check if original user does not exist in dbUsers
 		_, originalUserAlreadyExists := dbUsers[originalUsername]

pkg/cluster/sync_test.go

@@ -22,6 +22,7 @@ import (
 	"github.com/zalando/postgres-operator/pkg/spec"
 	"github.com/zalando/postgres-operator/pkg/util"
 	"github.com/zalando/postgres-operator/pkg/util/config"
+	"github.com/zalando/postgres-operator/pkg/util/constants"
 	"github.com/zalando/postgres-operator/pkg/util/k8sutil"
 	"github.com/zalando/postgres-operator/pkg/util/patroni"
 	"k8s.io/client-go/kubernetes/fake"
@@ -593,7 +594,7 @@ func TestUpdateSecret(t *testing.T) {
 				t.Errorf("%s: username differs in updated secret: expected %s, got %s", testName, username, secretUsername)
 			}
 		} else {
-			rotatedUsername := username + dayAfterTomorrow.Format("060102")
+			rotatedUsername := username + dayAfterTomorrow.Format(constants.RotationUserDateFormat)
 			if secretUsername != rotatedUsername {
 				t.Errorf("%s: updated secret does not contain correct username: expected %s, got %s", testName, rotatedUsername, secretUsername)
 			}

pkg/generated/clientset/versioned/fake/register.go

@@ -45,14 +45,14 @@ var localSchemeBuilder = runtime.SchemeBuilder{
 // AddToScheme adds all types of this clientset into the given scheme. This allows composition
 // of clientsets, like in:
 //
-//   import (
+//	import (
-//     "k8s.io/client-go/kubernetes"
+//	  "k8s.io/client-go/kubernetes"
-//     clientsetscheme "k8s.io/client-go/kubernetes/scheme"
+//	  clientsetscheme "k8s.io/client-go/kubernetes/scheme"
-//     aggregatorclientsetscheme "k8s.io/kube-aggregator/pkg/client/clientset_generated/clientset/scheme"
+//	  aggregatorclientsetscheme "k8s.io/kube-aggregator/pkg/client/clientset_generated/clientset/scheme"
-//   )
+//	)
 //
-//   kclientset, _ := kubernetes.NewForConfig(c)
+//	kclientset, _ := kubernetes.NewForConfig(c)
-//   _ = aggregatorclientsetscheme.AddToScheme(clientsetscheme.Scheme)
+//	_ = aggregatorclientsetscheme.AddToScheme(clientsetscheme.Scheme)
 //
 // After this, RawExtensions in Kubernetes types will serialize kube-aggregator types
 // correctly.

pkg/generated/clientset/versioned/scheme/register.go

@@ -45,14 +45,14 @@ var localSchemeBuilder = runtime.SchemeBuilder{
 // AddToScheme adds all types of this clientset into the given scheme. This allows composition
 // of clientsets, like in:
 //
-//   import (
+//	import (
-//     "k8s.io/client-go/kubernetes"
+//	  "k8s.io/client-go/kubernetes"
-//     clientsetscheme "k8s.io/client-go/kubernetes/scheme"
+//	  clientsetscheme "k8s.io/client-go/kubernetes/scheme"
-//     aggregatorclientsetscheme "k8s.io/kube-aggregator/pkg/client/clientset_generated/clientset/scheme"
+//	  aggregatorclientsetscheme "k8s.io/kube-aggregator/pkg/client/clientset_generated/clientset/scheme"
-//   )
+//	)
 //
-//   kclientset, _ := kubernetes.NewForConfig(c)
+//	kclientset, _ := kubernetes.NewForConfig(c)
-//   _ = aggregatorclientsetscheme.AddToScheme(clientsetscheme.Scheme)
+//	_ = aggregatorclientsetscheme.AddToScheme(clientsetscheme.Scheme)
 //
 // After this, RawExtensions in Kubernetes types will serialize kube-aggregator types
 // correctly.

pkg/spec/types.go

@@ -58,6 +58,7 @@ type PgUser struct {
 	AdminRole  string            `yaml:"admin_role"`
 	IsDbOwner  bool              `yaml:"is_db_owner"`
 	Deleted    bool              `yaml:"deleted"`
+	Rotated    bool              `yaml:"rotated"`
 }
 
 func (user *PgUser) Valid() bool {

pkg/util/constants/roles.go

@@ -20,4 +20,5 @@ const (
 	WriterRoleNameSuffix        = "_writer"
 	UserRoleNameSuffix          = "_user"
 	DefaultSearchPath           = "\"$user\""
+	RotationUserDateFormat      = "060102"
 )