From 89801ef30a68643fcf44fb0cdde0296f83f2b33d Mon Sep 17 00:00:00 2001 From: Felix Kunde Date: Wed, 2 Mar 2022 18:45:48 +0100 Subject: [PATCH 1/5] grant db owners to cron_admin --- .../crds/operatorconfigurations.yaml | 2 ++ charts/postgres-operator/values.yaml | 8 +++++ docs/reference/operator_parameters.md | 7 ++++ manifests/configmap.yaml | 3 +- manifests/operatorconfiguration.crd.yaml | 2 ++ ...gresql-operator-default-configuration.yaml | 2 ++ pkg/apis/acid.zalan.do/v1/crds.go | 12 +++++++ .../v1/operator_configuration_type.go | 1 + pkg/cluster/cluster.go | 36 +++++++++++++++++++ pkg/cluster/k8sres.go | 2 +- pkg/controller/operator_config.go | 3 +- pkg/spec/types.go | 1 + pkg/util/config/config.go | 3 +- 13 files changed, 78 insertions(+), 4 deletions(-) diff --git a/charts/postgres-operator/crds/operatorconfigurations.yaml b/charts/postgres-operator/crds/operatorconfigurations.yaml index f510e08f5..b801ed331 100644 --- a/charts/postgres-operator/crds/operatorconfigurations.yaml +++ b/charts/postgres-operator/crds/operatorconfigurations.yaml @@ -130,6 +130,8 @@ spec: users: type: object properties: + cron_admin_username: + type: string enable_password_rotation: type: boolean default: false diff --git a/charts/postgres-operator/values.yaml b/charts/postgres-operator/values.yaml index 288efe763..f78a358c6 100644 --- a/charts/postgres-operator/values.yaml +++ b/charts/postgres-operator/values.yaml @@ -59,6 +59,14 @@ configGeneral: # parameters describing Postgres users configUsers: + # username used to grant rights to set up and maintain cron jobs to database owners + cron_admin_username: 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 diff --git a/docs/reference/operator_parameters.md b/docs/reference/operator_parameters.md index f3d9be88f..0059bd103 100644 --- a/docs/reference/operator_parameters.md +++ b/docs/reference/operator_parameters.md @@ -177,6 +177,13 @@ under the `users` key. Postgres username used for replication between instances. The default is `standby`. +* **cron_admin_username** + Specifies the role that owns rights to set up cron jobs with `pg_cron` + extension inside the `postgres` database. The must be pre-configured in the + docker image. In Spilo this role is called `cron_admin`. This role will + become a member of all database owners so that they can set up cron jobs + e.g. as part of a migration script. 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 diff --git a/manifests/configmap.yaml b/manifests/configmap.yaml index b3aaa3c66..a3c033745 100644 --- a/manifests/configmap.yaml +++ b/manifests/configmap.yaml @@ -23,6 +23,7 @@ data: # connection_pooler_schema: "pooler" # connection_pooler_user: "pooler" crd_categories: "all" + # cron_admin_username: "cron_admin" # custom_service_annotations: "keyx:valuez,keya:valuea" # custom_pod_annotations: "keya:valuea,keyb:valueb" db_hosted_zone: db.example.com @@ -109,7 +110,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 index d086998cf..ff99a6e7b 100644 --- a/manifests/operatorconfiguration.crd.yaml +++ b/manifests/operatorconfiguration.crd.yaml @@ -128,6 +128,8 @@ spec: users: type: object properties: + cron_admin_username: + type: string enable_password_rotation: type: boolean default: false diff --git a/manifests/postgresql-operator-default-configuration.yaml b/manifests/postgresql-operator-default-configuration.yaml index 87b5436d5..28aaabb21 100644 --- a/manifests/postgresql-operator-default-configuration.yaml +++ b/manifests/postgresql-operator-default-configuration.yaml @@ -31,6 +31,7 @@ configuration: password_rotation_user_retention: 180 replication_username: standby super_username: postgres + # cron_admin_username: cron_admin major_version_upgrade: major_version_upgrade_mode: "off" # major_version_upgrade_team_allow_list: @@ -163,6 +164,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 index 9dc3d167e..043c8aaea 100644 --- a/pkg/apis/acid.zalan.do/v1/crds.go +++ b/pkg/apis/acid.zalan.do/v1/crds.go @@ -1142,6 +1142,18 @@ var OperatorConfigCRDResourceValidation = apiextv1.CustomResourceValidation{ "users": { Type: "object", Properties: map[string]apiextv1.JSONSchemaProps{ + "cron_admin_username": { + 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 index 1298c6834..7fbd5f9de 100644 --- a/pkg/apis/acid.zalan.do/v1/operator_configuration_type.go +++ b/pkg/apis/acid.zalan.do/v1/operator_configuration_type.go @@ -39,6 +39,7 @@ type OperatorConfigurationList struct { type PostgresUsersConfiguration struct { SuperUsername string `json:"super_username,omitempty"` ReplicationUsername string `json:"replication_username,omitempty"` + CronAdminUsername string `json:"cron_admin_username,omitempty"` EnablePasswordRotation bool `json:"enable_password_rotation,omitempty"` PasswordRotationInterval uint32 `json:"password_rotation_interval,omitempty"` PasswordRotationUserRetention uint32 `json:"password_rotation_user_retention,omitempty"` diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index 2afacde99..8ac0ab1ff 100644 --- 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.initCronAdmin() + return nil } @@ -1297,6 +1299,40 @@ func (c *Cluster) initRobotUsers() error { return nil } +func (c *Cluster) initCronAdmin() { + cronAdminName := c.OpConfig.CronAdminUsername + if cronAdminName == "" { + return + } + memberOf := make([]string, 0) + for username, pgUser := range c.pgUsers { + if pgUser.IsDbOwner { + memberOf = append(memberOf, username) + } + } + + if len(memberOf) > 1 { + namespace := c.Namespace + adminRole := "" + if c.OpConfig.EnableAdminRoleForUsers && cronAdminName != c.OpConfig.TeamAdminRole { + adminRole = c.OpConfig.TeamAdminRole + } + cronAdmin := spec.PgUser{ + Origin: spec.RoleOriginSpilo, + MemberOf: memberOf, + Name: cronAdminName, + Namespace: namespace, + Flags: []string{constants.RoleFlagNoLogin}, + AdminRole: adminRole, + } + if currentRole, present := c.pgUsers[cronAdminName]; present { + c.pgUsers[cronAdminName] = c.resolveNameConflict(¤tRole, &cronAdmin) + } else { + c.pgUsers[cronAdminName] = cronAdmin + } + } +} + func (c *Cluster) initTeamMembers(teamID string, isPostgresSuperuserTeam bool) error { teamMembers, err := c.getTeamMembers(teamID) diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index e741c6dc4..3f558820e 100644 --- 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 index fbf12bfb9..ccf2bc424 100644 --- a/pkg/controller/operator_config.go +++ b/pkg/controller/operator_config.go @@ -55,6 +55,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.CronAdminUsername = fromCRD.PostgresUsersConfiguration.CronAdminUsername result.EnablePasswordRotation = fromCRD.PostgresUsersConfiguration.EnablePasswordRotation result.PasswordRotationInterval = util.CoalesceUInt32(fromCRD.PostgresUsersConfiguration.PasswordRotationInterval, 90) result.PasswordRotationUserRetention = util.CoalesceUInt32(fromCRD.PostgresUsersConfiguration.DeepCopy().PasswordRotationUserRetention, 180) @@ -186,7 +187,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 index 202e0fa6f..428bcbdc6 100644 --- 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 index 0dc1004a7..139a0810e 100644 --- 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"` + CronAdminUsername string `name:"cron_admin_username" 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"` @@ -210,7 +211,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"` From 78eaae2efc3a02508b49827c30d2f35cfbd71c34 Mon Sep 17 00:00:00 2001 From: Felix Kunde Date: Fri, 4 Mar 2022 11:01:07 +0100 Subject: [PATCH 2/5] allow specifiying more extra owner roles --- .../crds/operatorconfigurations.yaml | 8 ++- charts/postgres-operator/values.yaml | 7 ++- docs/reference/operator_parameters.md | 16 +++--- manifests/configmap.yaml | 2 +- manifests/operatorconfiguration.crd.yaml | 8 ++- ...gresql-operator-default-configuration.yaml | 3 +- pkg/apis/acid.zalan.do/v1/crds.go | 10 +++- .../v1/operator_configuration_type.go | 12 ++--- .../acid.zalan.do/v1/zz_generated.deepcopy.go | 7 ++- pkg/cluster/cluster.go | 51 ++++++++----------- pkg/util/config/config.go | 2 +- pkg/util/users/users.go | 22 +++++--- 12 files changed, 86 insertions(+), 62 deletions(-) diff --git a/charts/postgres-operator/crds/operatorconfigurations.yaml b/charts/postgres-operator/crds/operatorconfigurations.yaml index b801ed331..c4b20c5e0 100644 --- a/charts/postgres-operator/crds/operatorconfigurations.yaml +++ b/charts/postgres-operator/crds/operatorconfigurations.yaml @@ -130,8 +130,11 @@ spec: users: type: object properties: - cron_admin_username: - type: string + additional_owner_roles: + type: array + nullable: true + items: + type: string enable_password_rotation: type: boolean default: false @@ -502,6 +505,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 index f78a358c6..38e389a8c 100644 --- a/charts/postgres-operator/values.yaml +++ b/charts/postgres-operator/values.yaml @@ -59,8 +59,10 @@ configGeneral: # parameters describing Postgres users configUsers: - # username used to grant rights to set up and maintain cron jobs to database owners - cron_admin_username: cron_admin + # 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 @@ -346,6 +348,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 index 0059bd103..136155b8a 100644 --- a/docs/reference/operator_parameters.md +++ b/docs/reference/operator_parameters.md @@ -177,12 +177,14 @@ under the `users` key. Postgres username used for replication between instances. The default is `standby`. -* **cron_admin_username** - Specifies the role that owns rights to set up cron jobs with `pg_cron` - extension inside the `postgres` database. The must be pre-configured in the - docker image. In Spilo this role is called `cron_admin`. This role will - become a member of all database owners so that they can set up cron jobs - e.g. as part of a migration script. Default is `empty`. +* **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 @@ -755,7 +757,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/manifests/configmap.yaml b/manifests/configmap.yaml index a3c033745..faef2eb1b 100644 --- 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" @@ -23,7 +24,6 @@ data: # connection_pooler_schema: "pooler" # connection_pooler_user: "pooler" crd_categories: "all" - # cron_admin_username: "cron_admin" # custom_service_annotations: "keyx:valuez,keya:valuea" # custom_pod_annotations: "keya:valuea,keyb:valueb" db_hosted_zone: db.example.com diff --git a/manifests/operatorconfiguration.crd.yaml b/manifests/operatorconfiguration.crd.yaml index ff99a6e7b..3c5ad3548 100644 --- a/manifests/operatorconfiguration.crd.yaml +++ b/manifests/operatorconfiguration.crd.yaml @@ -128,8 +128,11 @@ spec: users: type: object properties: - cron_admin_username: - type: string + additional_owner_roles: + type: array + nullable: true + items: + type: string enable_password_rotation: type: boolean default: false @@ -500,6 +503,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 index 28aaabb21..518b4dd49 100644 --- a/manifests/postgresql-operator-default-configuration.yaml +++ b/manifests/postgresql-operator-default-configuration.yaml @@ -26,12 +26,13 @@ configuration: # protocol: TCP workers: 8 users: + # additional_owner_roles: + # - cron_admin enable_password_rotation: false password_rotation_interval: 90 password_rotation_user_retention: 180 replication_username: standby super_username: postgres - # cron_admin_username: cron_admin major_version_upgrade: major_version_upgrade_mode: "off" # major_version_upgrade_team_allow_list: diff --git a/pkg/apis/acid.zalan.do/v1/crds.go b/pkg/apis/acid.zalan.do/v1/crds.go index 043c8aaea..fb4678a19 100644 --- a/pkg/apis/acid.zalan.do/v1/crds.go +++ b/pkg/apis/acid.zalan.do/v1/crds.go @@ -1142,8 +1142,14 @@ var OperatorConfigCRDResourceValidation = apiextv1.CustomResourceValidation{ "users": { Type: "object", Properties: map[string]apiextv1.JSONSchemaProps{ - "cron_admin_username": { - Type: "string", + "additional_owner_roles": { + Type: "array", + Nullable: true, + Items: &apiextv1.JSONSchemaPropsOrArray{ + Schema: &apiextv1.JSONSchemaProps{ + Type: "string", + }, + }, }, "enable_password_rotation": { Type: "boolean", 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 7fbd5f9de..d2a0e8969 100644 --- a/pkg/apis/acid.zalan.do/v1/operator_configuration_type.go +++ b/pkg/apis/acid.zalan.do/v1/operator_configuration_type.go @@ -37,12 +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"` - CronAdminUsername string `json:"cron_admin_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"` + AddtionalOwnerRoles []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 index c2298fada..4b791293d 100644 --- a/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go +++ b/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go @@ -401,7 +401,7 @@ func (in *OperatorConfigurationData) DeepCopyInto(out *OperatorConfigurationData (*in)[i].DeepCopyInto(&(*out)[i]) } } - out.PostgresUsersConfiguration = in.PostgresUsersConfiguration + in.PostgresUsersConfiguration.DeepCopyInto(&out.PostgresUsersConfiguration) in.MajorVersionUpgrade.DeepCopyInto(&out.MajorVersionUpgrade) in.Kubernetes.DeepCopyInto(&out.Kubernetes) out.PostgresPodResources = in.PostgresPodResources @@ -916,6 +916,11 @@ func (in *PostgresTeamSpec) DeepCopy() *PostgresTeamSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PostgresUsersConfiguration) DeepCopyInto(out *PostgresUsersConfiguration) { *out = *in + if in.AddtionalOwnerRoles != nil { + in, out := &in.AddtionalOwnerRoles, &out.AddtionalOwnerRoles + *out = make([]string, len(*in)) + copy(*out, *in) + } return } diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index 8ac0ab1ff..6567e6c9c 100644 --- a/pkg/cluster/cluster.go +++ b/pkg/cluster/cluster.go @@ -228,7 +228,7 @@ func (c *Cluster) initUsers() error { return fmt.Errorf("could not init human users: %v", err) } - c.initCronAdmin() + c.initAdditionalOwnerRoles() return nil } @@ -1299,36 +1299,29 @@ func (c *Cluster) initRobotUsers() error { return nil } -func (c *Cluster) initCronAdmin() { - cronAdminName := c.OpConfig.CronAdminUsername - if cronAdminName == "" { - return - } - memberOf := make([]string, 0) - for username, pgUser := range c.pgUsers { - if pgUser.IsDbOwner { - memberOf = append(memberOf, username) +func (c *Cluster) initAdditionalOwnerRoles() { + for _, additionalOwner := range c.OpConfig.AddtionalOwnerRoles { + // 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 - adminRole := "" - if c.OpConfig.EnableAdminRoleForUsers && cronAdminName != c.OpConfig.TeamAdminRole { - adminRole = c.OpConfig.TeamAdminRole - } - cronAdmin := spec.PgUser{ - Origin: spec.RoleOriginSpilo, - MemberOf: memberOf, - Name: cronAdminName, - Namespace: namespace, - Flags: []string{constants.RoleFlagNoLogin}, - AdminRole: adminRole, - } - if currentRole, present := c.pgUsers[cronAdminName]; present { - c.pgUsers[cronAdminName] = c.resolveNameConflict(¤tRole, &cronAdmin) - } else { - c.pgUsers[cronAdminName] = cronAdmin + 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(¤tRole, &additionalOwnerPgUser) + } else { + c.pgUsers[additionalOwner] = additionalOwnerPgUser + } } } } diff --git a/pkg/util/config/config.go b/pkg/util/config/config.go index 139a0810e..9619403f7 100644 --- a/pkg/util/config/config.go +++ b/pkg/util/config/config.go @@ -101,7 +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"` - CronAdminUsername string `name:"cron_admin_username" default:""` + AddtionalOwnerRoles []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"` diff --git a/pkg/util/users/users.go b/pkg/util/users/users.go index 6bc31f6da..5007268d2 100644 --- a/pkg/util/users/users.go +++ b/pkg/util/users/users.go @@ -53,25 +53,31 @@ func (strategy DefaultUserSyncStrategy) ProduceSyncRequests(dbUsers spec.PgUserM } } else { r := spec.PgSyncUserRequest{} + r.User = dbUser newMD5Password := util.NewEncryptor(strategy.PasswordEncryption).PGUserPassword(newUser) - if dbUser.Password != newMD5Password { - r.User.Password = newMD5Password - r.Kind = spec.PGsyncUserAlter + // 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 + } + if addNewFlags, equal := util.SubstractStringSlices(newUser.Flags, dbUser.Flags); !equal { + 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 } - if addNewFlags, equal := util.SubstractStringSlices(newUser.Flags, dbUser.Flags); !equal { - r.User.Flags = addNewFlags - r.Kind = spec.PGsyncUserAlter - } if r.Kind == spec.PGsyncUserAlter { r.User.Name = newUser.Name reqs = append(reqs, r) } - if len(newUser.Parameters) > 0 && !reflect.DeepEqual(dbUser.Parameters, newUser.Parameters) { + if newUser.Origin != spec.RoleOriginSpilo && + len(newUser.Parameters) > 0 && + !reflect.DeepEqual(dbUser.Parameters, newUser.Parameters) { reqs = append(reqs, spec.PgSyncUserRequest{Kind: spec.PGSyncAlterSet, User: newUser}) } } From 585fd07c43dc69d14921d3be4c48c6ef652f960b Mon Sep 17 00:00:00 2001 From: Felix Kunde Date: Fri, 4 Mar 2022 11:03:35 +0100 Subject: [PATCH 3/5] fix typo addtional --- pkg/apis/acid.zalan.do/v1/operator_configuration_type.go | 2 +- pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go | 4 ++-- pkg/cluster/cluster.go | 2 +- pkg/controller/operator_config.go | 2 +- pkg/util/config/config.go | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) 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 d2a0e8969..7066729b8 100644 --- a/pkg/apis/acid.zalan.do/v1/operator_configuration_type.go +++ b/pkg/apis/acid.zalan.do/v1/operator_configuration_type.go @@ -39,7 +39,7 @@ type OperatorConfigurationList struct { type PostgresUsersConfiguration struct { SuperUsername string `json:"super_username,omitempty"` ReplicationUsername string `json:"replication_username,omitempty"` - AddtionalOwnerRoles []string `json:"additional_owner_roles,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"` diff --git a/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go b/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go index 4b791293d..7f22f5444 100644 --- a/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go +++ b/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go @@ -916,8 +916,8 @@ func (in *PostgresTeamSpec) DeepCopy() *PostgresTeamSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PostgresUsersConfiguration) DeepCopyInto(out *PostgresUsersConfiguration) { *out = *in - if in.AddtionalOwnerRoles != nil { - in, out := &in.AddtionalOwnerRoles, &out.AddtionalOwnerRoles + if in.AdditionalOwnerRoles != nil { + in, out := &in.AdditionalOwnerRoles, &out.AdditionalOwnerRoles *out = make([]string, len(*in)) copy(*out, *in) } diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index 6567e6c9c..80c59f872 100644 --- a/pkg/cluster/cluster.go +++ b/pkg/cluster/cluster.go @@ -1300,7 +1300,7 @@ func (c *Cluster) initRobotUsers() error { } func (c *Cluster) initAdditionalOwnerRoles() { - for _, additionalOwner := range c.OpConfig.AddtionalOwnerRoles { + 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 { diff --git a/pkg/controller/operator_config.go b/pkg/controller/operator_config.go index ccf2bc424..a511bde10 100644 --- a/pkg/controller/operator_config.go +++ b/pkg/controller/operator_config.go @@ -55,7 +55,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.CronAdminUsername = fromCRD.PostgresUsersConfiguration.CronAdminUsername + 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) diff --git a/pkg/util/config/config.go b/pkg/util/config/config.go index 9619403f7..eef22ca9f 100644 --- a/pkg/util/config/config.go +++ b/pkg/util/config/config.go @@ -101,7 +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"` - AddtionalOwnerRoles []string `name:"additional_owner_roles" default:""` + 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"` From 1d86f1dc4d270a5473fe037b165638a8450e64df Mon Sep 17 00:00:00 2001 From: Felix Kunde Date: Fri, 4 Mar 2022 19:25:31 +0100 Subject: [PATCH 4/5] add unit test for InitAdditionalOwnerRoles --- pkg/cluster/cluster_test.go | 48 +++++++++++++++++++++++++++++++++---- 1 file changed, 44 insertions(+), 4 deletions(-) diff --git a/pkg/cluster/cluster_test.go b/pkg/cluster/cluster_test.go index d06cc21e1..48ecdbc8a 100644 --- a/pkg/cluster/cluster_test.go +++ b/pkg/cluster/cluster_test.go @@ -33,10 +33,11 @@ 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/*"}, @@ -44,7 +45,13 @@ var cl = New( }, }, 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 { } From dafacbfd11d02057524ad581c59dec9aed9cf22b Mon Sep 17 00:00:00 2001 From: Felix Kunde Date: Wed, 16 Mar 2022 12:46:55 +0100 Subject: [PATCH 5/5] add e2e test --- e2e/tests/test_e2e.py | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/e2e/tests/test_e2e.py b/e2e/tests/test_e2e.py index c4d104069..aaeae7afe 100644 --- 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): '''