From 1e2dbe4712da62e8f938cbf888bc7dac2c1ca6e9 Mon Sep 17 00:00:00 2001 From: Felix Kunde Date: Fri, 23 Apr 2021 09:57:34 +0200 Subject: [PATCH] make suffix configurable and add deprecated field to pgUser struct --- .../crds/operatorconfigurations.yaml | 3 ++ charts/postgres-operator/values-crd.yaml | 4 +++ charts/postgres-operator/values.yaml | 3 ++ docs/reference/operator_parameters.md | 7 +++++ docs/user.md | 11 +++++++ manifests/configmap.yaml | 1 + manifests/operatorconfiguration.crd.yaml | 3 ++ ...gresql-operator-default-configuration.yaml | 1 + pkg/apis/acid.zalan.do/v1/crds.go | 3 ++ .../v1/operator_configuration_type.go | 1 + pkg/cluster/cluster.go | 25 +++++++++------- pkg/cluster/database.go | 7 ++++- pkg/cluster/sync.go | 30 ++++++++++++------- pkg/controller/operator_config.go | 1 + pkg/spec/types.go | 1 + pkg/util/config/config.go | 1 + pkg/util/constants/roles.go | 1 - pkg/util/users/users.go | 16 +++++----- 18 files changed, 89 insertions(+), 30 deletions(-) diff --git a/charts/postgres-operator/crds/operatorconfigurations.yaml b/charts/postgres-operator/crds/operatorconfigurations.yaml index 0ac5651c3..d9923c1b5 100644 --- a/charts/postgres-operator/crds/operatorconfigurations.yaml +++ b/charts/postgres-operator/crds/operatorconfigurations.yaml @@ -465,6 +465,9 @@ spec: type: string default: - admin + role_deprecation_suffix: + type: string + default: "_delete_me" team_admin_role: type: string default: "admin" diff --git a/charts/postgres-operator/values-crd.yaml b/charts/postgres-operator/values-crd.yaml index 6110fe8bc..44f9166a9 100644 --- a/charts/postgres-operator/values-crd.yaml +++ b/charts/postgres-operator/values-crd.yaml @@ -312,6 +312,10 @@ configTeamsApi: # List of roles that cannot be overwritten by an application, team or infrastructure role protected_role_names: - admin + + # Suffix to add if members are removed from TeamsAPI or PostgresTeam CRD + # role_deprecation_suffix: "_delete_me" + # role name to grant to team members created from the Teams API team_admin_role: admin # postgres config parameters to apply to each team member role diff --git a/charts/postgres-operator/values.yaml b/charts/postgres-operator/values.yaml index 1cf84ca87..90d5dca47 100644 --- a/charts/postgres-operator/values.yaml +++ b/charts/postgres-operator/values.yaml @@ -304,6 +304,9 @@ configTeamsApi: # List of roles that cannot be overwritten by an application, team or infrastructure role # protected_role_names: "admin" + # Suffix to add if members are removed from TeamsAPI or PostgresTeam CRD + # role_deprecation_suffix: "_delete_me" + # role name to grant to team members created from the Teams API # team_admin_role: "admin" diff --git a/docs/reference/operator_parameters.md b/docs/reference/operator_parameters.md index 80d0e5b8c..a6e89640a 100644 --- a/docs/reference/operator_parameters.md +++ b/docs/reference/operator_parameters.md @@ -704,6 +704,13 @@ key. cluster to administer Postgres and maintain infrastructure built around it. The default is empty. +* **role_deprecation_suffix** + defines a suffix that will be appended to database role names of team members + that were removed from either PostgresTeam CRDs (additionalMembers) or from + the team in the teams API. When readded to the manifest, the operator will + rename roles with the defined suffix back to the original role name. + The default is `_delete_me`. + * **enable_postgres_team_crd** toggle to make the operator watch for created or updated `PostgresTeam` CRDs and create roles for specified additional teams and members. diff --git a/docs/user.md b/docs/user.md index 3342913c8..89d180071 100644 --- a/docs/user.md +++ b/docs/user.md @@ -407,6 +407,17 @@ spec: - "briggs" ``` +#### Removed members + +The Postgres Operator does not delete database roles when users are removed +from manifests. But, when using the PostgresTeam CRD or Teams API it is very +easy to (accidently) add roles to many clusters. Manually reverting such a +change is cumbersome. Therefore, if members are removed from the team CRD or +teams API the operator will rename roles appending a configured suffix to the +name (see `role_deprecation_suffix` option) so that old members cannot login +anymore. When a role is readded to the manifest the operator will check for +roles with the configured suffix and rename the role back to the original name. + ## Prepared databases with roles and default privileges The `users` section in the manifests only allows for creating database roles diff --git a/manifests/configmap.yaml b/manifests/configmap.yaml index 29390a1f0..4b95a96b1 100644 --- a/manifests/configmap.yaml +++ b/manifests/configmap.yaml @@ -111,6 +111,7 @@ data: resource_check_timeout: 10m resync_period: 30m ring_log_lines: "100" + # role_deprecation_suffix: "_delete_me" secret_name_template: "{username}.{cluster}.credentials" # sidecar_docker_images: "" # set_memory_request_to_limit: "false" diff --git a/manifests/operatorconfiguration.crd.yaml b/manifests/operatorconfiguration.crd.yaml index 47244e5d7..fc708674e 100644 --- a/manifests/operatorconfiguration.crd.yaml +++ b/manifests/operatorconfiguration.crd.yaml @@ -461,6 +461,9 @@ spec: type: string default: - admin + role_deprecation_suffix: + type: string + default: "_delete_me" team_admin_role: type: string default: "admin" diff --git a/manifests/postgresql-operator-default-configuration.yaml b/manifests/postgresql-operator-default-configuration.yaml index 3a8a79c8d..a3d120ce4 100644 --- a/manifests/postgresql-operator-default-configuration.yaml +++ b/manifests/postgresql-operator-default-configuration.yaml @@ -149,6 +149,7 @@ configuration: # - postgres_superusers protected_role_names: - admin + # role_deprecation_suffix: "_delete_me" team_admin_role: admin team_api_role_configuration: log_statement: all diff --git a/pkg/apis/acid.zalan.do/v1/crds.go b/pkg/apis/acid.zalan.do/v1/crds.go index 89d71eef5..f3090c370 100644 --- a/pkg/apis/acid.zalan.do/v1/crds.go +++ b/pkg/apis/acid.zalan.do/v1/crds.go @@ -1405,6 +1405,9 @@ var OperatorConfigCRDResourceValidation = apiextv1.CustomResourceValidation{ }, }, }, + "role_deprecation_suffix": { + Type: "string", + }, "team_admin_role": { Type: "string", }, diff --git a/pkg/apis/acid.zalan.do/v1/operator_configuration_type.go b/pkg/apis/acid.zalan.do/v1/operator_configuration_type.go index 78b618b78..ddcac1c45 100644 --- a/pkg/apis/acid.zalan.do/v1/operator_configuration_type.go +++ b/pkg/apis/acid.zalan.do/v1/operator_configuration_type.go @@ -159,6 +159,7 @@ type TeamsAPIConfiguration struct { PostgresSuperuserTeams []string `json:"postgres_superuser_teams,omitempty"` EnablePostgresTeamCRD bool `json:"enable_postgres_team_crd,omitempty"` EnablePostgresTeamCRDSuperusers bool `json:"enable_postgres_team_crd_superusers,omitempty"` + RoleDeprecationSuffix string `json:"role_deprecation_suffix,omitempty"` } // LoggingRESTAPIConfiguration defines Logging API conf diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index 00d753888..b5d15da3f 100644 --- a/pkg/cluster/cluster.go +++ b/pkg/cluster/cluster.go @@ -130,7 +130,9 @@ func New(cfg Config, kubeClient k8sutil.KubernetesClient, pgSpec acidv1.Postgres Secrets: make(map[types.UID]*v1.Secret), Services: make(map[PostgresRole]*v1.Service), Endpoints: make(map[PostgresRole]*v1.Endpoints)}, - userSyncStrategy: users.DefaultUserSyncStrategy{PasswordEncryption: passwordEncryption}, + userSyncStrategy: users.DefaultUserSyncStrategy{ + PasswordEncryption: passwordEncryption, + RoleDeprecationSuffix: cfg.OpConfig.RoleDeprecationSuffix}, deleteOptions: metav1.DeleteOptions{PropagationPolicy: &deletePropagationPolicy}, podEventsQueue: podEventsQueue, KubeClient: kubeClient, @@ -191,6 +193,14 @@ func (c *Cluster) isNewCluster() bool { func (c *Cluster) initUsers() error { c.setProcessName("initializing users") + // save current state of pgUsers to check for deleted roles later + c.pgUsersCache = map[string]spec.PgUser{} + for k, v := range c.pgUsers { + if v.Origin == spec.RoleOriginTeamsAPI { + c.pgUsersCache[k] = v + } + } + // clear our the previous state of the cluster users (in case we are // running a sync). c.systemUsers = map[string]spec.PgUser{} @@ -644,12 +654,6 @@ func (c *Cluster) Update(oldSpec, newSpec *acidv1.Postgresql) error { needReplicaConnectionPoolerWorker(&newSpec.Spec) if !sameUsers || needConnectionPooler { c.logger.Debugf("initialize users") - // save current state of pgUsers to check for deleted roles later - c.pgUsersCache = map[string]spec.PgUser{} - for k, v := range c.pgUsers { - c.pgUsersCache[k] = v - } - if err := c.initUsers(); err != nil { c.logger.Errorf("could not init users: %v", err) updateFailed = true @@ -1140,6 +1144,7 @@ func (c *Cluster) initTeamMembers(teamID string, isPostgresSuperuserTeam bool) e Flags: flags, MemberOf: memberOf, Parameters: c.OpConfig.TeamAPIRoleConfiguration, + Deprecated: false, } if currentRole, present := c.pgUsers[username]; present { @@ -1170,7 +1175,7 @@ func (c *Cluster) initHumanUsers() error { for _, superuserTeam := range superuserTeams { err := c.initTeamMembers(superuserTeam, true) if err != nil { - return fmt.Errorf("Cannot initialize members for team %q of Postgres superusers: %v", superuserTeam, err) + return fmt.Errorf("cannot initialize members for team %q of Postgres superusers: %v", superuserTeam, err) } if superuserTeam == c.Spec.TeamID { clusterIsOwnedBySuperuserTeam = true @@ -1183,7 +1188,7 @@ func (c *Cluster) initHumanUsers() error { if !(util.SliceContains(superuserTeams, additionalTeam)) { err := c.initTeamMembers(additionalTeam, false) if err != nil { - return fmt.Errorf("Cannot initialize members for additional team %q for cluster owned by %q: %v", additionalTeam, c.Spec.TeamID, err) + return fmt.Errorf("cannot initialize members for additional team %q for cluster owned by %q: %v", additionalTeam, c.Spec.TeamID, err) } } } @@ -1196,7 +1201,7 @@ func (c *Cluster) initHumanUsers() error { err := c.initTeamMembers(c.Spec.TeamID, false) if err != nil { - return fmt.Errorf("Cannot initialize members for team %q who owns the Postgres cluster: %v", c.Spec.TeamID, err) + return fmt.Errorf("cannot initialize members for team %q who owns the Postgres cluster: %v", c.Spec.TeamID, err) } return nil diff --git a/pkg/cluster/database.go b/pkg/cluster/database.go index 760b68d72..6a5fa9bd4 100644 --- a/pkg/cluster/database.go +++ b/pkg/cluster/database.go @@ -198,6 +198,7 @@ func (c *Cluster) readPgUsersFromDatabase(userNames []string) (users spec.PgUser rolname, rolpassword string rolsuper, rolinherit, rolcreaterole, rolcreatedb, rolcanlogin bool roloptions, memberof []string + roldeprecated bool ) err := rows.Scan(&rolname, &rolpassword, &rolsuper, &rolinherit, &rolcreaterole, &rolcreatedb, &rolcanlogin, pq.Array(&roloptions), pq.Array(&memberof)) @@ -216,7 +217,11 @@ func (c *Cluster) readPgUsersFromDatabase(userNames []string) (users spec.PgUser parameters[fields[0]] = fields[1] } - users[rolname] = spec.PgUser{Name: rolname, Password: rolpassword, Flags: flags, MemberOf: memberof, Parameters: parameters} + if strings.HasSuffix(rolname, c.OpConfig.RoleDeprecationSuffix) { + roldeprecated = true + } + + users[rolname] = spec.PgUser{Name: rolname, Password: rolpassword, Flags: flags, MemberOf: memberof, Parameters: parameters, Deprecated: roldeprecated} } return users, nil diff --git a/pkg/cluster/sync.go b/pkg/cluster/sync.go index 9ccb109f9..a5551b203 100644 --- a/pkg/cluster/sync.go +++ b/pkg/cluster/sync.go @@ -37,12 +37,6 @@ func (c *Cluster) Sync(newSpec *acidv1.Postgresql) error { } }() - // save current state of pgUsers to check for deleted roles later - c.pgUsersCache = map[string]spec.PgUser{} - for k, v := range c.pgUsers { - c.pgUsersCache[k] = v - } - if err = c.initUsers(); err != nil { err = fmt.Errorf("could not init users: %v", err) return err @@ -582,25 +576,29 @@ func (c *Cluster) syncRoles() (err error) { } }() + // mapping between deprecated and original role name + deprecatedUsers := map[string]string{} + + // create list of database roles to query for _, u := range c.pgUsers { userNames = append(userNames, u.Name) // add team member role name with rename suffix in case we need to rename it back if u.Origin == spec.RoleOriginTeamsAPI { - userNames = append(userNames, u.Name+constants.RoleRenameSuffix) + deprecatedUsers[u.Name+c.OpConfig.RoleDeprecationSuffix] = u.Name + userNames = append(userNames, u.Name+c.OpConfig.RoleDeprecationSuffix) } } - // add removed additional team members from cache + // add team members that exist only in cache // to trigger a rename of the role in ProduceSyncRequests for _, cachedUser := range c.pgUsersCache { - if cachedUser.Origin != spec.RoleOriginTeamsAPI { - continue - } if _, exists := c.pgUsers[cachedUser.Name]; !exists { userNames = append(userNames, cachedUser.Name) } } + // add pooler user to list of pgUsers, too + // to check if the pooler user exists or has to be created if needMasterConnectionPooler(&c.Spec) || needReplicaConnectionPooler(&c.Spec) { connectionPoolerUser := c.systemUsers[constants.ConnectionPoolerUserKeyName] userNames = append(userNames, connectionPoolerUser.Name) @@ -615,6 +613,16 @@ func (c *Cluster) syncRoles() (err error) { return fmt.Errorf("error getting users from the database: %v", err) } + // update pgUsers where a deprecated role was found + // so that they are skipped in ProduceSyncRequests + for _, dbUser := range dbUsers { + if originalUser, exists := deprecatedUsers[dbUser.Name]; exists { + recreatedUser := c.pgUsers[originalUser] + recreatedUser.Deprecated = true + c.pgUsers[originalUser] = recreatedUser + } + } + pgSyncRequests := c.userSyncStrategy.ProduceSyncRequests(dbUsers, c.pgUsers) if err = c.userSyncStrategy.ExecuteSyncRequests(pgSyncRequests, c.pgDb); err != nil { return fmt.Errorf("error executing sync statements: %v", err) diff --git a/pkg/controller/operator_config.go b/pkg/controller/operator_config.go index 3c0302cab..1496e6572 100644 --- a/pkg/controller/operator_config.go +++ b/pkg/controller/operator_config.go @@ -180,6 +180,7 @@ func (c *Controller) importConfigurationFromCRD(fromCRD *acidv1.OperatorConfigur result.PostgresSuperuserTeams = fromCRD.TeamsAPI.PostgresSuperuserTeams result.EnablePostgresTeamCRD = fromCRD.TeamsAPI.EnablePostgresTeamCRD result.EnablePostgresTeamCRDSuperusers = fromCRD.TeamsAPI.EnablePostgresTeamCRDSuperusers + result.RoleDeprecationSuffix = util.Coalesce(fromCRD.TeamsAPI.RoleDeprecationSuffix, "_delete_me") // logging REST API config result.APIPort = util.CoalesceInt(fromCRD.LoggingRESTAPI.APIPort, 8080) diff --git a/pkg/spec/types.go b/pkg/spec/types.go index 69f742e42..db73cc896 100644 --- a/pkg/spec/types.go +++ b/pkg/spec/types.go @@ -54,6 +54,7 @@ type PgUser struct { MemberOf []string `yaml:"inrole"` Parameters map[string]string `yaml:"db_parameters"` AdminRole string `yaml:"admin_role"` + Deprecated bool `yaml:"deprecated"` } func (user *PgUser) Valid() bool { diff --git a/pkg/util/config/config.go b/pkg/util/config/config.go index a5d144051..30d5fb2ca 100644 --- a/pkg/util/config/config.go +++ b/pkg/util/config/config.go @@ -176,6 +176,7 @@ type Config struct { EnableTeamsAPI bool `name:"enable_teams_api" default:"true"` EnableTeamSuperuser bool `name:"enable_team_superuser" default:"false"` TeamAdminRole string `name:"team_admin_role" default:"admin"` + RoleDeprecationSuffix string `name:"role_deprecation_suffix,omitempty" default:"_delete_me"` EnableAdminRoleForUsers bool `name:"enable_admin_role_for_users" default:"true"` EnablePostgresTeamCRD bool `name:"enable_postgres_team_crd" default:"false"` EnablePostgresTeamCRDSuperusers bool `name:"enable_postgres_team_crd_superusers" default:"false"` diff --git a/pkg/util/constants/roles.go b/pkg/util/constants/roles.go index 4230028e1..dd906fe80 100644 --- a/pkg/util/constants/roles.go +++ b/pkg/util/constants/roles.go @@ -18,6 +18,5 @@ const ( ReaderRoleNameSuffix = "_reader" WriterRoleNameSuffix = "_writer" UserRoleNameSuffix = "_user" - RoleRenameSuffix = "_delete_me" DefaultSearchPath = "\"$user\"" ) diff --git a/pkg/util/users/users.go b/pkg/util/users/users.go index 461a45567..63f230ce5 100644 --- a/pkg/util/users/users.go +++ b/pkg/util/users/users.go @@ -9,7 +9,6 @@ import ( "github.com/zalando/postgres-operator/pkg/spec" "github.com/zalando/postgres-operator/pkg/util" - "github.com/zalando/postgres-operator/pkg/util/constants" ) const ( @@ -30,7 +29,8 @@ const ( // an existing roles of another role membership, nor it removes the already assigned flag // (except for the NOLOGIN). TODO: process other NOflags, i.e. NOSUPERUSER correctly. type DefaultUserSyncStrategy struct { - PasswordEncryption string + PasswordEncryption string + RoleDeprecationSuffix string } // ProduceSyncRequests figures out the types of changes that need to happen with the given users. @@ -39,9 +39,11 @@ func (strategy DefaultUserSyncStrategy) ProduceSyncRequests(dbUsers spec.PgUserM var reqs []spec.PgSyncUserRequest for name, newUser := range newUsers { + if newUser.Deprecated { + continue + } dbUser, exists := dbUsers[name] - _, existsRenamed := dbUsers[name+constants.RoleRenameSuffix] - if !exists && !existsRenamed { + if !exists { reqs = append(reqs, spec.PgSyncUserRequest{Kind: spec.PGSyncUserAdd, User: newUser}) if len(newUser.Parameters) > 0 { reqs = append(reqs, spec.PgSyncUserRequest{Kind: spec.PGSyncAlterSet, User: newUser}) @@ -141,11 +143,11 @@ func (strategy DefaultUserSyncStrategy) alterPgUserSet(user spec.PgUser, db *sql func (strategy DefaultUserSyncStrategy) alterPgUserRename(user spec.PgUser, db *sql.DB) error { var query string - if strings.HasSuffix(user.Name, constants.RoleRenameSuffix) { - newName := strings.TrimSuffix(user.Name, constants.RoleRenameSuffix) + if user.Deprecated { + newName := strings.TrimSuffix(user.Name, strategy.RoleDeprecationSuffix) query = fmt.Sprintf(alterUserRenameSQL, user.Name, newName, "") } else { - query = fmt.Sprintf(alterUserRenameSQL, user.Name, user.Name, constants.RoleRenameSuffix) + query = fmt.Sprintf(alterUserRenameSQL, user.Name, user.Name, strategy.RoleDeprecationSuffix) } if _, err := db.Exec(query); err != nil {