grant db owners to cron_admin (#1805)

FxKu · web-flow · commit 2719d411c322 · 2022-03-18T12:36:12.000+01:00
* grant db owners to cron_admin
* allow specifiying more extra owner roles
* add unit test for InitAdditionalOwnerRoles
* add e2e test
diff --git a/charts/postgres-operator/crds/operatorconfigurations.yaml b/charts/postgres-operator/crds/operatorconfigurations.yaml
@@ -130,6 +130,11 @@ spec:
               users:
                 type: object
                 properties:
+                  additional_owner_roles:
+                    type: array
+                    nullable: true
+                    items:
+                      type: string
                   enable_password_rotation:
                     type: boolean
                     default: false
@@ -514,6 +519,7 @@ spec:
                       type: string
                     default:
                     - admin
+                    - cron_admin
                   role_deletion_suffix:
                     type: string
                     default: "_deleted"
diff --git a/charts/postgres-operator/values.yaml b/charts/postgres-operator/values.yaml
@@ -59,6 +59,16 @@ configGeneral:
 
 # parameters describing Postgres users
 configUsers:
+  # roles to be granted to database owners
+  # additional_owner_roles:
+  # - cron_admin
+
+  # enable password rotation for app users that are not database owners
+  enable_password_rotation: false
+  # rotation interval for updating credentials in K8s secrets of app users
+  password_rotation_interval: 90
+  # retention interval to keep rotation users
+  password_rotation_user_retention: 180
   # postgres username used for replication between instances
   replication_username: standby
   # postgres superuser name to be created by initdb
@@ -348,6 +358,7 @@ configTeamsApi:
   # List of roles that cannot be overwritten by an application, team or infrastructure role
   protected_role_names:
     - admin
+    - cron_admin
   # Suffix to add if members are removed from TeamsAPI or PostgresTeam CRD
   role_deletion_suffix: "_deleted"
   # role name to grant to team members created from the Teams API
diff --git a/docs/reference/operator_parameters.md b/docs/reference/operator_parameters.md
@@ -177,6 +177,15 @@ under the `users` key.
   Postgres username used for replication between instances. The default is
   `standby`.
 
+* **additional_owner_roles**
+  Specifies database roles that will become members of all database owners.
+  Then owners can 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
+  `cron_admin` which is provided by the Spilo docker image to set up cron
+  jobs inside the `postgres` database. Default is `empty`.
+
 * **enable_password_rotation**
   For all `LOGIN` roles that are not database owners the operator can rotate
   credentials in the corresponding K8s secrets by replacing the username and
@@ -770,7 +779,7 @@ key.
 
 * **protected_role_names**
   List of roles that cannot be overwritten by an application, team or
-  infrastructure role. The default is `admin`.
+  infrastructure role. The default list is `admin` and `cron_admin`.
 
 * **postgres_superuser_teams**
   List of teams which members need the superuser role in each PG database
diff --git a/e2e/tests/test_e2e.py b/e2e/tests/test_e2e.py
@@ -158,6 +158,37 @@ def setUpClass(cls):
             print('Operator log: {}'.format(k8s.get_operator_log()))
             raise
 
+    @timeout_decorator.timeout(TEST_TIMEOUT_SEC)
+    def test_additional_owner_roles(self):
+        '''
+           Test adding additional member roles to existing database owner roles
+        '''
+        k8s = self.k8s
+
+        # enable PostgresTeam CRD and lower resync
+        owner_roles = {
+            "data": {
+                "additional_owner_roles": "cron_admin",
+            },
+        }
+        k8s.update_config(owner_roles)
+        self.eventuallyEqual(lambda: k8s.get_operator_state(), {"0": "idle"},
+                             "Operator does not get in sync")
+
+        leader = k8s.get_cluster_leader_pod()
+        owner_query = """
+            SELECT a2.rolname
+              FROM pg_catalog.pg_authid a
+              JOIN pg_catalog.pg_auth_members am
+                ON a.oid = am.member
+               AND a.rolname = 'cron_admin'
+              JOIN pg_catalog.pg_authid a2
+                ON a2.oid = am.roleid
+             WHERE a2.rolname IN ('zalando', 'bar_owner', 'bar_data_owner');
+        """
+        self.eventuallyEqual(lambda: len(self.query_database(leader.metadata.name, "postgres", owner_query)), 3,
+            "Not all additional users found in database", 10, 5)
+
     @timeout_decorator.timeout(TEST_TIMEOUT_SEC)
     def test_additional_pod_capabilities(self):
         '''
diff --git a/manifests/configmap.yaml b/manifests/configmap.yaml
@@ -3,6 +3,7 @@ kind: ConfigMap
 metadata:
   name: postgres-operator
 data:
+  # additional_owner_roles: "cron_admin"
   # additional_pod_capabilities: "SYS_NICE"
   # additional_secret_mount: "some-secret-name"
   # additional_secret_mount_path: "/some/dir"
@@ -114,7 +115,7 @@ data:
   # pod_service_account_role_binding_definition: ""
   pod_terminate_grace_period: 5m
   # postgres_superuser_teams: "postgres_superusers"
-  # protected_role_names: "admin"
+  # protected_role_names: "admin,cron_admin"
   ready_wait_interval: 3s
   ready_wait_timeout: 30s
   repair_period: 5m
diff --git a/manifests/operatorconfiguration.crd.yaml b/manifests/operatorconfiguration.crd.yaml
@@ -128,6 +128,11 @@ spec:
               users:
                 type: object
                 properties:
+                  additional_owner_roles:
+                    type: array
+                    nullable: true
+                    items:
+                      type: string
                   enable_password_rotation:
                     type: boolean
                     default: false
@@ -512,6 +517,7 @@ spec:
                       type: string
                     default:
                     - admin
+                    - cron_admin
                   role_deletion_suffix:
                     type: string
                     default: "_deleted"
diff --git a/manifests/postgresql-operator-default-configuration.yaml b/manifests/postgresql-operator-default-configuration.yaml
@@ -26,6 +26,8 @@ configuration:
   #     protocol: TCP
   workers: 8
   users:
+    # additional_owner_roles: 
+    # - cron_admin
     enable_password_rotation: false
     password_rotation_interval: 90
     password_rotation_user_retention: 180
@@ -168,6 +170,7 @@ configuration:
     # - postgres_superusers
     protected_role_names:
     - admin
+    - cron_admin
     role_deletion_suffix: "_deleted"
     team_admin_role: admin
     team_api_role_configuration:
diff --git a/pkg/apis/acid.zalan.do/v1/crds.go b/pkg/apis/acid.zalan.do/v1/crds.go
@@ -1148,6 +1148,24 @@ var OperatorConfigCRDResourceValidation = apiextv1.CustomResourceValidation{
 					"users": {
 						Type: "object",
 						Properties: map[string]apiextv1.JSONSchemaProps{
+							"additional_owner_roles": {
+								Type:     "array",
+								Nullable: true,
+								Items: &apiextv1.JSONSchemaPropsOrArray{
+									Schema: &apiextv1.JSONSchemaProps{
+										Type: "string",
+									},
+								},
+							},
+							"enable_password_rotation": {
+								Type: "boolean",
+							},
+							"password_rotation_interval": {
+								Type: "integer",
+							},
+							"password_rotation_user_retention": {
+								Type: "integer",
+							},
 							"replication_username": {
 								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
@@ -37,11 +37,12 @@ type OperatorConfigurationList struct {
 
 // PostgresUsersConfiguration defines the system users of Postgres.
 type PostgresUsersConfiguration struct {
-	SuperUsername                 string `json:"super_username,omitempty"`
-	ReplicationUsername           string `json:"replication_username,omitempty"`
-	EnablePasswordRotation        bool   `json:"enable_password_rotation,omitempty"`
-	PasswordRotationInterval      uint32 `json:"password_rotation_interval,omitempty"`
-	PasswordRotationUserRetention uint32 `json:"password_rotation_user_retention,omitempty"`
+	SuperUsername                 string   `json:"super_username,omitempty"`
+	ReplicationUsername           string   `json:"replication_username,omitempty"`
+	AdditionalOwnerRoles          []string `json:"additional_owner_roles,omitempty"`
+	EnablePasswordRotation        bool     `json:"enable_password_rotation,omitempty"`
+	PasswordRotationInterval      uint32   `json:"password_rotation_interval,omitempty"`
+	PasswordRotationUserRetention uint32   `json:"password_rotation_user_retention,omitempty"`
 }
 
 // MajorVersionUpgradeConfiguration defines how to execute major version upgrades of Postgres.
diff --git a/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go b/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go
diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go
@@ -228,6 +228,8 @@ func (c *Cluster) initUsers() error {
 		return fmt.Errorf("could not init human users: %v", err)
 	}
 
+	c.initAdditionalOwnerRoles()
+
 	return nil
 }
 
@@ -1297,6 +1299,33 @@ func (c *Cluster) initRobotUsers() error {
 	return nil
 }
 
+func (c *Cluster) initAdditionalOwnerRoles() {
+	for _, additionalOwner := range c.OpConfig.AdditionalOwnerRoles {
+		// fetch all database owners the additional should become a member of
+		memberOf := make([]string, 0)
+		for username, pgUser := range c.pgUsers {
+			if pgUser.IsDbOwner {
+				memberOf = append(memberOf, username)
+			}
+		}
+
+		if len(memberOf) > 1 {
+			namespace := c.Namespace
+			additionalOwnerPgUser := spec.PgUser{
+				Origin:    spec.RoleOriginSpilo,
+				MemberOf:  memberOf,
+				Name:      additionalOwner,
+				Namespace: namespace,
+			}
+			if currentRole, present := c.pgUsers[additionalOwner]; present {
+				c.pgUsers[additionalOwner] = c.resolveNameConflict(&currentRole, &additionalOwnerPgUser)
+			} else {
+				c.pgUsers[additionalOwner] = additionalOwnerPgUser
+			}
+		}
+	}
+}
+
 func (c *Cluster) initTeamMembers(teamID string, isPostgresSuperuserTeam bool) error {
 	teamMembers, err := c.getTeamMembers(teamID)
 
diff --git a/pkg/cluster/cluster_test.go b/pkg/cluster/cluster_test.go
@@ -33,18 +33,25 @@ var cl = New(
 	Config{
 		OpConfig: config.Config{
 			PodManagementPolicy: "ordered_ready",
-			ProtectedRoles:      []string{"admin"},
+			ProtectedRoles:      []string{"admin", "cron_admin", "part_man"},
 			Auth: config.Auth{
-				SuperUsername:       superUserName,
-				ReplicationUsername: replicationUserName,
+				SuperUsername:        superUserName,
+				ReplicationUsername:  replicationUserName,
+				AdditionalOwnerRoles: []string{"cron_admin", "part_man"},
 			},
 			Resources: config.Resources{
 				DownscalerAnnotations: []string{"downscaler/*"},
 			},
 		},
 	},
 	k8sutil.NewMockKubernetesClient(),
-	acidv1.Postgresql{ObjectMeta: metav1.ObjectMeta{Name: "acid-test", Namespace: "test", Annotations: map[string]string{"downscaler/downtime_replicas": "0"}}},
+	acidv1.Postgresql{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:        "acid-test",
+			Namespace:   "test",
+			Annotations: map[string]string{"downscaler/downtime_replicas": "0"},
+		},
+	},
 	logger,
 	eventRecorder,
 )
@@ -132,6 +139,39 @@ func TestInitRobotUsers(t *testing.T) {
 	}
 }
 
+func TestInitAdditionalOwnerRoles(t *testing.T) {
+	testName := "TestInitAdditionalOwnerRoles"
+
+	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},
+		"bar_owner":  {Origin: spec.RoleOriginManifest, Name: "bar_owner", Namespace: cl.Namespace, Password: "b123", Flags: []string{"LOGIN"}, IsDbOwner: true},
+		"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"}
+	cl.Spec.Users = manifestUsers
+
+	// this should set IsDbOwner field for manifest users
+	if err := cl.initRobotUsers(); err != nil {
+		t.Errorf("%s could not init manifest users", testName)
+	}
+
+	// update passwords to compare with result
+	for manifestUser := range manifestUsers {
+		pgUser := cl.pgUsers[manifestUser]
+		pgUser.Password = manifestUser[0:1] + "123"
+		cl.pgUsers[manifestUser] = pgUser
+	}
+
+	cl.initAdditionalOwnerRoles()
+	if !reflect.DeepEqual(cl.pgUsers, expectedUsers) {
+		t.Errorf("%s expected: %#v, got %#v", testName, expectedUsers, cl.pgUsers)
+	}
+}
+
 type mockOAuthTokenGetter struct {
 }
 
diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go
@@ -1622,7 +1622,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 {
+		if pgUser.Origin != spec.RoleOriginTeamsAPI && pgUser.Origin != spec.RoleOriginSpilo {
 			c.logger.Warningf("could not generate secret for a non-teamsAPI role %q: role has no password",
 				pgUser.Name)
 		}
diff --git a/pkg/controller/operator_config.go b/pkg/controller/operator_config.go
@@ -52,6 +52,7 @@ func (c *Controller) importConfigurationFromCRD(fromCRD *acidv1.OperatorConfigur
 	// user config
 	result.SuperUsername = util.Coalesce(fromCRD.PostgresUsersConfiguration.SuperUsername, "postgres")
 	result.ReplicationUsername = util.Coalesce(fromCRD.PostgresUsersConfiguration.ReplicationUsername, "standby")
+	result.AdditionalOwnerRoles = fromCRD.PostgresUsersConfiguration.AdditionalOwnerRoles
 	result.EnablePasswordRotation = fromCRD.PostgresUsersConfiguration.EnablePasswordRotation
 	result.PasswordRotationInterval = util.CoalesceUInt32(fromCRD.PostgresUsersConfiguration.PasswordRotationInterval, 90)
 	result.PasswordRotationUserRetention = util.CoalesceUInt32(fromCRD.PostgresUsersConfiguration.DeepCopy().PasswordRotationUserRetention, 180)
@@ -188,7 +189,7 @@ func (c *Controller) importConfigurationFromCRD(fromCRD *acidv1.OperatorConfigur
 	result.TeamAdminRole = fromCRD.TeamsAPI.TeamAdminRole
 	result.PamRoleName = util.Coalesce(fromCRD.TeamsAPI.PamRoleName, "zalandos")
 	result.PamConfiguration = util.Coalesce(fromCRD.TeamsAPI.PamConfiguration, "https://info.example.com/oauth2/tokeninfo?access_token= uid realm=/employees")
-	result.ProtectedRoles = util.CoalesceStrArr(fromCRD.TeamsAPI.ProtectedRoles, []string{"admin"})
+	result.ProtectedRoles = util.CoalesceStrArr(fromCRD.TeamsAPI.ProtectedRoles, []string{"admin", "cron_admin"})
 	result.PostgresSuperuserTeams = fromCRD.TeamsAPI.PostgresSuperuserTeams
 	result.EnablePostgresTeamCRD = fromCRD.TeamsAPI.EnablePostgresTeamCRD
 	result.EnablePostgresTeamCRDSuperusers = fromCRD.TeamsAPI.EnablePostgresTeamCRDSuperusers
diff --git a/pkg/spec/types.go b/pkg/spec/types.go
@@ -30,6 +30,7 @@ const (
 	RoleOriginManifest
 	RoleOriginInfrastructure
 	RoleOriginTeamsAPI
+	RoleOriginSpilo
 	RoleOriginSystem
 	RoleOriginBootstrap
 	RoleConnectionPooler
diff --git a/pkg/util/config/config.go b/pkg/util/config/config.go
@@ -101,6 +101,7 @@ type Auth struct {
 	InfrastructureRolesDefs       string                `name:"infrastructure_roles_secrets"`
 	SuperUsername                 string                `name:"super_username" default:"postgres"`
 	ReplicationUsername           string                `name:"replication_username" default:"standby"`
+	AdditionalOwnerRoles          []string              `name:"additional_owner_roles" default:""`
 	EnablePasswordRotation        bool                  `name:"enable_password_rotation" default:"false"`
 	PasswordRotationInterval      uint32                `name:"password_rotation_interval" default:"90"`
 	PasswordRotationUserRetention uint32                `name:"password_rotation_user_retention" default:"180"`
@@ -213,7 +214,7 @@ type Config struct {
 	TeamAPIRoleConfiguration               map[string]string `name:"team_api_role_configuration" default:"log_statement:all"`
 	PodTerminateGracePeriod                time.Duration     `name:"pod_terminate_grace_period" default:"5m"`
 	PodManagementPolicy                    string            `name:"pod_management_policy" default:"ordered_ready"`
-	ProtectedRoles                         []string          `name:"protected_role_names" default:"admin"`
+	ProtectedRoles                         []string          `name:"protected_role_names" default:"admin,cron_admin"`
 	PostgresSuperuserTeams                 []string          `name:"postgres_superuser_teams" default:""`
 	SetMemoryRequestToLimit                bool              `name:"set_memory_request_to_limit" default:"false"`
 	EnableLazySpiloUpgrade                 bool              `name:"enable_lazy_spilo_upgrade" default:"false"`
diff --git a/pkg/util/users/users.go b/pkg/util/users/users.go
