From 4741b3f7340f5007935697695d88468f95282f3e Mon Sep 17 00:00:00 2001 From: Felix Kunde Date: Wed, 25 Jan 2023 10:48:23 +0100 Subject: [PATCH] copy rolconfig during password rotation (#2183) * copy rolconfig during password rotation Co-authored-by: idanovinda --- e2e/tests/test_e2e.py | 29 +++++++++++++++---- pkg/cluster/database.go | 2 +- pkg/cluster/sync.go | 26 ++++++++++++++--- pkg/cluster/sync_test.go | 3 +- .../clientset/versioned/fake/register.go | 14 ++++----- .../clientset/versioned/scheme/register.go | 14 ++++----- pkg/spec/types.go | 1 + pkg/util/constants/roles.go | 1 + 8 files changed, 65 insertions(+), 25 deletions(-) diff --git a/e2e/tests/test_e2e.py b/e2e/tests/test_e2e.py index 6b46dd7db..8b087b909 100644 --- a/e2e/tests/test_e2e.py +++ b/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 ROLE 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_user201031 IN ROLE foo_user; + CREATE USER foo_user211031 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): diff --git a/pkg/cluster/database.go b/pkg/cluster/database.go index f1ea736ce..3ec36bb67 100644 --- a/pkg/cluster/database.go +++ b/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 diff --git a/pkg/cluster/sync.go b/pkg/cluster/sync.go index 75d4e8a65..797a07256 100644 --- a/pkg/cluster/sync.go +++ b/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 - // so that they are skipped in ProduceSyncRequests +DBUSERS: 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] diff --git a/pkg/cluster/sync_test.go b/pkg/cluster/sync_test.go index 3cd3d3f28..f7f8ad9c7 100644 --- a/pkg/cluster/sync_test.go +++ b/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) } diff --git a/pkg/generated/clientset/versioned/fake/register.go b/pkg/generated/clientset/versioned/fake/register.go index 19d48b0d2..a156f8f52 100644 --- a/pkg/generated/clientset/versioned/fake/register.go +++ b/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 ( -// "k8s.io/client-go/kubernetes" -// clientsetscheme "k8s.io/client-go/kubernetes/scheme" -// aggregatorclientsetscheme "k8s.io/kube-aggregator/pkg/client/clientset_generated/clientset/scheme" -// ) +// import ( +// "k8s.io/client-go/kubernetes" +// clientsetscheme "k8s.io/client-go/kubernetes/scheme" +// aggregatorclientsetscheme "k8s.io/kube-aggregator/pkg/client/clientset_generated/clientset/scheme" +// ) // -// kclientset, _ := kubernetes.NewForConfig(c) -// _ = aggregatorclientsetscheme.AddToScheme(clientsetscheme.Scheme) +// kclientset, _ := kubernetes.NewForConfig(c) +// _ = aggregatorclientsetscheme.AddToScheme(clientsetscheme.Scheme) // // After this, RawExtensions in Kubernetes types will serialize kube-aggregator types // correctly. diff --git a/pkg/generated/clientset/versioned/scheme/register.go b/pkg/generated/clientset/versioned/scheme/register.go index b3ac16d07..b1509deb9 100644 --- a/pkg/generated/clientset/versioned/scheme/register.go +++ b/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 ( -// "k8s.io/client-go/kubernetes" -// clientsetscheme "k8s.io/client-go/kubernetes/scheme" -// aggregatorclientsetscheme "k8s.io/kube-aggregator/pkg/client/clientset_generated/clientset/scheme" -// ) +// import ( +// "k8s.io/client-go/kubernetes" +// clientsetscheme "k8s.io/client-go/kubernetes/scheme" +// aggregatorclientsetscheme "k8s.io/kube-aggregator/pkg/client/clientset_generated/clientset/scheme" +// ) // -// kclientset, _ := kubernetes.NewForConfig(c) -// _ = aggregatorclientsetscheme.AddToScheme(clientsetscheme.Scheme) +// kclientset, _ := kubernetes.NewForConfig(c) +// _ = aggregatorclientsetscheme.AddToScheme(clientsetscheme.Scheme) // // After this, RawExtensions in Kubernetes types will serialize kube-aggregator types // correctly. diff --git a/pkg/spec/types.go b/pkg/spec/types.go index 66f26a312..023f9660f 100644 --- a/pkg/spec/types.go +++ b/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 { diff --git a/pkg/util/constants/roles.go b/pkg/util/constants/roles.go index 8c81e2f04..34f1d0737 100644 --- a/pkg/util/constants/roles.go +++ b/pkg/util/constants/roles.go @@ -20,4 +20,5 @@ const ( WriterRoleNameSuffix = "_writer" UserRoleNameSuffix = "_user" DefaultSearchPath = "\"$user\"" + RotationUserDateFormat = "060102" )