Skip to content

Commit

Permalink
Minor refactoring, add tests.
Browse files Browse the repository at this point in the history
Address the code review by Sergey.
  • Loading branch information
alexeyklyukin committed Dec 5, 2017
1 parent 570c930 commit 6ad08d3
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 5 deletions.
8 changes: 4 additions & 4 deletions pkg/cluster/cluster.go
Expand Up @@ -620,7 +620,7 @@ func (c *Cluster) initRobotUsers() error {
return fmt.Errorf("invalid username: %q", username)
}

if c.avoidProtectedOrSystemRole(username, "manifest robot role") {
if c.shouldAvoidProtectedOrSystemRole(username, "manifest robot role") {
continue
}
flags, err := normalizeUserFlags(userFlags)
Expand Down Expand Up @@ -656,7 +656,7 @@ func (c *Cluster) initHumanUsers() error {
flags := []string{constants.RoleFlagLogin}
memberOf := []string{c.OpConfig.PamRoleName}

if c.avoidProtectedOrSystemRole(username, "API role") {
if c.shouldAvoidProtectedOrSystemRole(username, "API role") {
continue
}
if c.OpConfig.EnableTeamSuperuser {
Expand Down Expand Up @@ -688,7 +688,7 @@ func (c *Cluster) initInfrastructureRoles() error {
if !isValidUsername(username) {
return fmt.Errorf("invalid username: '%v'", username)
}
if c.avoidProtectedOrSystemRole(username, "infrastructure role") {
if c.shouldAvoidProtectedOrSystemRole(username, "infrastructure role") {
continue
}
flags, err := normalizeUserFlags(data.Flags)
Expand All @@ -701,7 +701,7 @@ func (c *Cluster) initInfrastructureRoles() error {
return nil
}

func (c *Cluster) avoidProtectedOrSystemRole(username, purpose string) bool {
func (c *Cluster) shouldAvoidProtectedOrSystemRole(username, purpose string) bool {
if c.isProtectedUsername(username) {
c.logger.Warnf("cannot initialize a new %s with the name of the protected user %q", purpose, username)
return true
Expand Down
17 changes: 16 additions & 1 deletion pkg/cluster/cluster_test.go
Expand Up @@ -4,14 +4,18 @@ import (
"fmt"
"github.com/Sirupsen/logrus"
"github.com/zalando-incubator/postgres-operator/pkg/spec"
"github.com/zalando-incubator/postgres-operator/pkg/util/config"
"github.com/zalando-incubator/postgres-operator/pkg/util/k8sutil"
"github.com/zalando-incubator/postgres-operator/pkg/util/teams"
"reflect"
"testing"
)

var logger = logrus.New().WithField("test", "cluster")
var cl = New(Config{}, k8sutil.KubernetesClient{}, spec.Postgresql{}, logger)
var cl = New(Config{OpConfig: config.Config{ProtectedRoles: []string{"admin"},
Auth: config.Auth{SuperUsername: "postgres",
ReplicationUsername: "standby"}}},
k8sutil.KubernetesClient{}, spec.Postgresql{}, logger)

func TestInitRobotUsers(t *testing.T) {
testName := "TestInitRobotUsers"
Expand Down Expand Up @@ -47,6 +51,12 @@ func TestInitRobotUsers(t *testing.T) {
err: fmt.Errorf(`invalid flags for user "foobar": ` +
`conflicting user flags: "NOINHERIT" and "INHERIT"`),
},
{
manifestUsers: map[string]spec.UserFlags{"admin": {"superuser"}, "postgres": {"createdb"}},
infraRoles: map[string]spec.PgUser{},
result: map[string]spec.PgUser{},
err: nil,
},
}
for _, tt := range tests {
cl.Spec.Users = tt.manifestUsers
Expand Down Expand Up @@ -109,6 +119,11 @@ func TestInitHumanUsers(t *testing.T) {
result: map[string]spec.PgUser{"foo": {Name: "foo", MemberOf: []string{cl.OpConfig.PamRoleName}, Flags: []string{"LOGIN", "SUPERUSER"}},
"bar": {Name: "bar", Flags: []string{"NOLOGIN"}}},
},
{
existingRoles: map[string]spec.PgUser{},
teamRoles: []string{"admin", "standby"},
result: map[string]spec.PgUser{},
},
}

for _, tt := range tests {
Expand Down

0 comments on commit 6ad08d3

Please sign in to comment.