From 2bb7e98268f8da2343a97c4fb46b2eb1ce1a786a Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Fri, 23 Feb 2018 17:24:04 +0100 Subject: [PATCH] update individual role secrets from infrastructure roles (#206) * Track origin of roles. * Propagate changes on infrastructure roles to corresponding secrets. When the password in the infrastructure role is updated, re-generate the secret for that role. Previously, the password for an infrastructure role was always fetched from the secret, making any updates to such role a no-op after the corresponding secret had been generated. --- pkg/cluster/cluster.go | 4 ++++ pkg/cluster/cluster_test.go | 13 +++++++------ pkg/cluster/k8sres.go | 5 +++++ pkg/cluster/sync.go | 17 +++++++++++++++-- pkg/controller/util.go | 2 +- pkg/controller/util_test.go | 1 + pkg/spec/types.go | 11 +++++++++++ 7 files changed, 44 insertions(+), 9 deletions(-) diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index 13db2b75e..e2bb86d57 100644 --- a/pkg/cluster/cluster.go +++ b/pkg/cluster/cluster.go @@ -656,10 +656,12 @@ func (c *Cluster) initSystemUsers() { // secrets, therefore, setting flags like SUPERUSER or REPLICATION // is not necessary here c.systemUsers[constants.SuperuserKeyName] = spec.PgUser{ + Origin: spec.RoleOriginSystem, Name: c.OpConfig.SuperUsername, Password: util.RandomPassword(constants.PasswordLength), } c.systemUsers[constants.ReplicationUserKeyName] = spec.PgUser{ + Origin: spec.RoleOriginSystem, Name: c.OpConfig.ReplicationUsername, Password: util.RandomPassword(constants.PasswordLength), } @@ -680,6 +682,7 @@ func (c *Cluster) initRobotUsers() error { } if _, present := c.pgUsers[username]; !present { c.pgUsers[username] = spec.PgUser{ + Origin: spec.RoleOriginManifest, Name: username, Password: util.RandomPassword(constants.PasswordLength), Flags: flags, @@ -723,6 +726,7 @@ func (c *Cluster) initHumanUsers() error { } c.pgUsers[username] = spec.PgUser{ + Origin: spec.RoleOriginTeamsAPI, Name: username, Flags: flags, MemberOf: memberOf, diff --git a/pkg/cluster/cluster_test.go b/pkg/cluster/cluster_test.go index 141355281..9b7b5de12 100644 --- a/pkg/cluster/cluster_test.go +++ b/pkg/cluster/cluster_test.go @@ -33,9 +33,9 @@ func TestInitRobotUsers(t *testing.T) { }{ { manifestUsers: map[string]spec.UserFlags{"foo": {"superuser", "createdb"}}, - infraRoles: map[string]spec.PgUser{"foo": {Name: "foo", Password: "bar"}}, - result: map[string]spec.PgUser{"foo": {Name: "foo", Password: "bar", - Flags: []string{"CREATEDB", "LOGIN", "SUPERUSER"}}}, + infraRoles: map[string]spec.PgUser{"foo": {Origin: spec.RoleOriginManifest, Name: "foo", Password: "bar"}}, + result: map[string]spec.PgUser{"foo": {Origin: spec.RoleOriginManifest, + Name: "foo", Password: "bar", Flags: []string{"CREATEDB", "LOGIN", "SUPERUSER"}}}, err: nil, }, { @@ -119,10 +119,11 @@ func TestInitHumanUsers(t *testing.T) { result map[string]spec.PgUser }{ { - existingRoles: map[string]spec.PgUser{"foo": {Name: "foo", Flags: []string{"NOLOGIN"}}, - "bar": {Name: "bar", Flags: []string{"NOLOGIN"}}}, + existingRoles: map[string]spec.PgUser{"foo": {Name: "foo", Origin: spec.RoleOriginTeamsAPI, + Flags: []string{"NOLOGIN"}}, "bar": {Name: "bar", Flags: []string{"NOLOGIN"}}}, teamRoles: []string{"foo"}, - result: map[string]spec.PgUser{"foo": {Name: "foo", MemberOf: []string{cl.OpConfig.PamRoleName}, Flags: []string{"LOGIN", "SUPERUSER"}}, + result: map[string]spec.PgUser{"foo": {Name: "foo", Origin: spec.RoleOriginTeamsAPI, + MemberOf: []string{cl.OpConfig.PamRoleName}, Flags: []string{"LOGIN", "SUPERUSER"}}, "bar": {Name: "bar", Flags: []string{"NOLOGIN"}}}, }, { diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index 52fbfeb17..562214375 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -646,8 +646,13 @@ func (c *Cluster) generateUserSecrets() (secrets 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 { + c.logger.Warningf("could not generate secret for a non-teamsAPI role %q: role has no password", + pgUser.Name) + } return nil } + username := pgUser.Name secret := v1.Secret{ ObjectMeta: metav1.ObjectMeta{ diff --git a/pkg/cluster/sync.go b/pkg/cluster/sync.go index b6865b7b6..955253f1a 100644 --- a/pkg/cluster/sync.go +++ b/pkg/cluster/sync.go @@ -322,6 +322,10 @@ func (c *Cluster) syncSecrets() error { if err2 != nil { return fmt.Errorf("could not get current secret: %v", err2) } + if secretUsername != string(curSecret.Data["username"]) { + c.logger.Warningf("secret %q does not contain the role %q", secretSpec.Name, secretUsername) + continue + } c.logger.Debugf("secret %q already exists, fetching its password", util.NameFromMeta(curSecret.ObjectMeta)) if secretUsername == c.systemUsers[constants.SuperuserKeyName].Name { secretUsername = constants.SuperuserKeyName @@ -333,8 +337,17 @@ func (c *Cluster) syncSecrets() error { userMap = c.pgUsers } pwdUser := userMap[secretUsername] - pwdUser.Password = string(curSecret.Data["password"]) - userMap[secretUsername] = pwdUser + // if this secret belongs to the infrastructure role and the password has changed - replace it in the secret + if pwdUser.Password != string(curSecret.Data["password"]) && pwdUser.Origin == spec.RoleOriginInfrastructure { + c.logger.Debugf("updating the secret %q from the infrastructure roles", secretSpec.Name) + if _, err := c.KubeClient.Secrets(secretSpec.Namespace).Update(secretSpec); err != nil { + return fmt.Errorf("could not update infrastructure role secret for role %q: %v", secretUsername, err) + } + } else { + // for non-infrastructure role - update the role with the password from the secret + pwdUser.Password = string(curSecret.Data["password"]) + userMap[secretUsername] = pwdUser + } continue } else { diff --git a/pkg/controller/util.go b/pkg/controller/util.go index 806573df8..fc310d5e5 100644 --- a/pkg/controller/util.go +++ b/pkg/controller/util.go @@ -116,7 +116,7 @@ Users: // in worst case we would have one line per user for i := 1; i <= len(data); i++ { properties := []string{"user", "password", "inrole"} - t := spec.PgUser{} + t := spec.PgUser{Origin: spec.RoleOriginInfrastructure} for _, p := range properties { key := fmt.Sprintf("%s%d", p, i) if val, present := data[key]; !present { diff --git a/pkg/controller/util_test.go b/pkg/controller/util_test.go index 41b8e2fe1..8da9621ba 100644 --- a/pkg/controller/util_test.go +++ b/pkg/controller/util_test.go @@ -131,6 +131,7 @@ func TestGetInfrastructureRoles(t *testing.T) { map[string]spec.PgUser{ "testrole": { Name: "testrole", + Origin: spec.RoleOriginInfrastructure, Password: "testpassword", MemberOf: []string{"testinrole"}, }, diff --git a/pkg/spec/types.go b/pkg/spec/types.go index a4fadf368..8f213948b 100644 --- a/pkg/spec/types.go +++ b/pkg/spec/types.go @@ -32,6 +32,16 @@ const ( fileWithNamespace = "/var/run/secrets/kubernetes.io/serviceaccount/namespace" ) +type RoleOrigin int + +const ( + RoleOriginUnknown = iota + RoleOriginInfrastructure + RoleOriginManifest + RoleOriginTeamsAPI + RoleOriginSystem +) + // ClusterEvent carries the payload of the Cluster TPR events. type ClusterEvent struct { EventTime time.Time @@ -62,6 +72,7 @@ type PodEvent struct { // PgUser contains information about a single user. type PgUser struct { + Origin RoleOrigin Name string Password string Flags []string