make suffix configurable and add deprecated field to pgUser struct

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"

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

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"
 

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.

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

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"

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"

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

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",
 							},

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

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

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

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)

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)

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 {

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"`

pkg/util/constants/roles.go

@@ -18,6 +18,5 @@ const (
 	ReaderRoleNameSuffix        = "_reader"
 	WriterRoleNameSuffix        = "_writer"
 	UserRoleNameSuffix          = "_user"
-	RoleRenameSuffix            = "_delete_me"
 	DefaultSearchPath           = "\"$user\""
 )

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 {