reverse membership for additional owner roles

remove type RoleOriginSpilo

docs/reference/operator_parameters.md

@@ -178,8 +178,8 @@ under the `users` key.
   `standby`.
 
 * **additional_owner_roles**
-  Specifies database roles that will become members of all database owners.
+  Specifies database roles that will granted to all database owners. Owners
-  Then owners can use `SET ROLE` to obtain privileges of these roles to e.g.
+  can then use `SET ROLE` to obtain privileges of these roles to e.g.
   create/update functionality from extensions as part of a migration script.
   Note, that roles listed here should be preconfigured in the docker image
   and already exist in the database cluster on startup. One such role can be

e2e/tests/test_e2e.py

@@ -161,7 +161,7 @@ class EndToEndTestCase(unittest.TestCase):
     @timeout_decorator.timeout(TEST_TIMEOUT_SEC)
     def test_additional_owner_roles(self):
         '''
-           Test adding additional member roles to existing database owner roles
+           Test granting additional roles to existing database owners
         '''
         k8s = self.k8s
 
@@ -181,10 +181,10 @@ class EndToEndTestCase(unittest.TestCase):
               FROM pg_catalog.pg_authid a
               JOIN pg_catalog.pg_auth_members am
                 ON a.oid = am.member
-               AND a.rolname = 'cron_admin'
+               AND a.rolname IN ('zalando', 'bar_owner', 'bar_data_owner')
               JOIN pg_catalog.pg_authid a2
                 ON a2.oid = am.roleid
-             WHERE a2.rolname IN ('zalando', 'bar_owner', 'bar_data_owner');
+             WHERE a2.rolname = 'cron_admin';
         """
         self.eventuallyEqual(lambda: len(self.query_database(leader.metadata.name, "postgres", owner_query)), 3,
             "Not all additional users found in database", 10, 5)

pkg/cluster/cluster.go

@@ -1308,28 +1308,15 @@ func (c *Cluster) initRobotUsers() error {
 }
 
 func (c *Cluster) initAdditionalOwnerRoles() {
-	for _, additionalOwner := range c.OpConfig.AdditionalOwnerRoles {
+	if len(c.OpConfig.AdditionalOwnerRoles) == 0 {
-		// fetch all database owners the additional should become a member of
+		return
-		memberOf := make([]string, 0)
-		for username, pgUser := range c.pgUsers {
-			if pgUser.IsDbOwner {
-				memberOf = append(memberOf, username)
-			}
 	}
 
-		if len(memberOf) > 0 {
+	// fetch database owners and assign additional owner roles
-			namespace := c.Namespace
+	for username, pgUser := range c.pgUsers {
-			additionalOwnerPgUser := spec.PgUser{
+		if pgUser.IsDbOwner {
-				Origin:    spec.RoleOriginSpilo,
+			pgUser.MemberOf = append(pgUser.MemberOf, c.OpConfig.AdditionalOwnerRoles...)
-				MemberOf:  memberOf,
+			c.pgUsers[username] = pgUser
-				Name:      additionalOwner,
-				Namespace: namespace,
-			}
-			if currentRole, present := c.pgUsers[additionalOwner]; present {
-				c.pgUsers[additionalOwner] = c.resolveNameConflict(&currentRole, &additionalOwnerPgUser)
-			} else {
-				c.pgUsers[additionalOwner] = additionalOwnerPgUser
-			}
 		}
 	}
 }

pkg/cluster/cluster_test.go

@@ -148,11 +148,9 @@ func TestInitAdditionalOwnerRoles(t *testing.T) {
 
 	manifestUsers := map[string]acidv1.UserFlags{"foo_owner": {}, "bar_owner": {}, "app_user": {}}
 	expectedUsers := map[string]spec.PgUser{
-		"foo_owner":  {Origin: spec.RoleOriginManifest, Name: "foo_owner", Namespace: cl.Namespace, Password: "f123", Flags: []string{"LOGIN"}, IsDbOwner: true},
+		"foo_owner": {Origin: spec.RoleOriginManifest, Name: "foo_owner", Namespace: cl.Namespace, Password: "f123", Flags: []string{"LOGIN"}, IsDbOwner: true, MemberOf: []string{"cron_admin", "part_man"}},
-		"bar_owner":  {Origin: spec.RoleOriginManifest, Name: "bar_owner", Namespace: cl.Namespace, Password: "b123", Flags: []string{"LOGIN"}, IsDbOwner: true},
+		"bar_owner": {Origin: spec.RoleOriginManifest, Name: "bar_owner", Namespace: cl.Namespace, Password: "b123", Flags: []string{"LOGIN"}, IsDbOwner: true, MemberOf: []string{"cron_admin", "part_man"}},
 		"app_user":  {Origin: spec.RoleOriginManifest, Name: "app_user", Namespace: cl.Namespace, Password: "a123", Flags: []string{"LOGIN"}, IsDbOwner: false},
-		"cron_admin": {Origin: spec.RoleOriginSpilo, Name: "cron_admin", Namespace: cl.Namespace, MemberOf: []string{"foo_owner", "bar_owner"}},
-		"part_man":   {Origin: spec.RoleOriginSpilo, Name: "part_man", Namespace: cl.Namespace, MemberOf: []string{"foo_owner", "bar_owner"}},
 	}
 
 	cl.Spec.Databases = map[string]string{"foo_db": "foo_owner", "bar_db": "bar_owner"}
@@ -163,24 +161,15 @@ func TestInitAdditionalOwnerRoles(t *testing.T) {
 		t.Errorf("%s could not init manifest users", testName)
 	}
 
-	// update passwords to compare with result
+	// now assign additional roles to owners
-	for manifestUser := range manifestUsers {
-		pgUser := cl.pgUsers[manifestUser]
-		pgUser.Password = manifestUser[0:1] + "123"
-		cl.pgUsers[manifestUser] = pgUser
-	}
-
 	cl.initAdditionalOwnerRoles()
 
-	for _, additionalOwnerRole := range cl.Config.OpConfig.AdditionalOwnerRoles {
+	// update passwords to compare with result
-		expectedPgUser := expectedUsers[additionalOwnerRole]
+	for username, existingPgUser := range cl.pgUsers {
-		existingPgUser, exists := cl.pgUsers[additionalOwnerRole]
+		expectedPgUser := expectedUsers[username]
-		if !exists {
-			t.Errorf("%s additional owner role %q not initilaized", testName, additionalOwnerRole)
-		}
 		if !util.IsEqualIgnoreOrder(expectedPgUser.MemberOf, existingPgUser.MemberOf) {
-			t.Errorf("%s unexpected membership of additional owner role %q: expected member of %#v, got member of %#v",
+			t.Errorf("%s unexpected membership of user %q: expected member of %#v, got member of %#v",
-				testName, additionalOwnerRole, expectedPgUser.MemberOf, existingPgUser.MemberOf)
+				testName, username, expectedPgUser.MemberOf, existingPgUser.MemberOf)
 		}
 	}
 }

pkg/cluster/k8sres.go

@@ -1649,7 +1649,7 @@ func (c *Cluster) generateUserSecrets() map[string]*v1.Secret {
 func (c *Cluster) generateSingleUserSecret(namespace string, pgUser spec.PgUser) *v1.Secret {
 	//Skip users with no password i.e. human users (they'll be authenticated using pam)
 	if pgUser.Password == "" {
-		if pgUser.Origin != spec.RoleOriginTeamsAPI && pgUser.Origin != spec.RoleOriginSpilo {
+		if pgUser.Origin != spec.RoleOriginTeamsAPI {
 			c.logger.Warningf("could not generate secret for a non-teamsAPI role %q: role has no password",
 				pgUser.Name)
 		}

pkg/spec/types.go

@@ -30,7 +30,6 @@ const (
 	RoleOriginManifest
 	RoleOriginInfrastructure
 	RoleOriginTeamsAPI
-	RoleOriginSpilo
 	RoleOriginSystem
 	RoleOriginBootstrap
 	RoleConnectionPooler

pkg/util/users/users.go

@@ -57,7 +57,6 @@ func (strategy DefaultUserSyncStrategy) ProduceSyncRequests(dbUsers spec.PgUserM
 			newMD5Password := util.NewEncryptor(strategy.PasswordEncryption).PGUserPassword(newUser)
 
 			// do not compare for roles coming from docker image
-			if newUser.Origin != spec.RoleOriginSpilo {
 			if dbUser.Password != newMD5Password {
 				r.User.Password = newMD5Password
 				r.Kind = spec.PGsyncUserAlter
@@ -66,7 +65,6 @@ func (strategy DefaultUserSyncStrategy) ProduceSyncRequests(dbUsers spec.PgUserM
 				r.User.Flags = addNewFlags
 				r.Kind = spec.PGsyncUserAlter
 			}
-			}
 			if addNewRoles, equal := util.SubstractStringSlices(newUser.MemberOf, dbUser.MemberOf); !equal {
 				r.User.MemberOf = addNewRoles
 				r.Kind = spec.PGsyncUserAlter
@@ -75,8 +73,7 @@ func (strategy DefaultUserSyncStrategy) ProduceSyncRequests(dbUsers spec.PgUserM
 				r.User.Name = newUser.Name
 				reqs = append(reqs, r)
 			}
-			if newUser.Origin != spec.RoleOriginSpilo &&
+			if len(newUser.Parameters) > 0 &&
-				len(newUser.Parameters) > 0 &&
 				!reflect.DeepEqual(dbUser.Parameters, newUser.Parameters) {
 				reqs = append(reqs, spec.PgSyncUserRequest{Kind: spec.PGSyncAlterSet, User: newUser})
 			}