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

@@ -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,

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

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{

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 {

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 {

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

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