Avoid overwriting critical users. (#172)
* Avoid overwriting critical users. Disallow defining new users either in the cluster manifest, teams API or infrastructure roles with the names mentioned in the new protected_role_names parameter (list of comma-separated names) Additionally, forbid defining a user with the name matching either super_username or replication_username, so that we don't overwrite system roles required for correct working of the operator itself. Also, clear PostgreSQL roles on each sync first in order to avoid using the old definitions that are no longer present in the current manifest, infrastructure roles secret or the teams API.
This commit is contained in:
		
							parent
							
								
									022ce29314
								
							
						
					
					
						commit
						1fb8cf7ea0
					
				|  | @ -208,6 +208,8 @@ The following steps will get you the docker image built and deployed. | ||||||
| of configuration parameters applied to the roles fetched from the API. | of configuration parameters applied to the roles fetched from the API. | ||||||
| For instance, `team_api_role_configuration: log_statement:all,search_path:'public,"$user"'`. | For instance, `team_api_role_configuration: log_statement:all,search_path:'public,"$user"'`. | ||||||
| By default is set to *"log_statement:all"*. See [PostgreSQL documentation on ALTER ROLE .. SET](https://www.postgresql.org/docs/current/static/sql-alterrole.html) for to learn about the available options. | By default is set to *"log_statement:all"*. See [PostgreSQL documentation on ALTER ROLE .. SET](https://www.postgresql.org/docs/current/static/sql-alterrole.html) for to learn about the available options. | ||||||
|  | * protected_role_names - a list of role names that should be forbidden as the manifest, infrastructure and teams API roles. | ||||||
|  | The default value is `admin`. Operator will also disallow superuser and replication roles to be redefined. | ||||||
| 
 | 
 | ||||||
| 
 | 
 | ||||||
| ### Debugging the operator itself | ### Debugging the operator itself | ||||||
|  |  | ||||||
|  | @ -168,6 +168,11 @@ func (c *Cluster) setStatus(status spec.PostgresStatus) { | ||||||
| // initUsers populates c.systemUsers and c.pgUsers maps.
 | // initUsers populates c.systemUsers and c.pgUsers maps.
 | ||||||
| func (c *Cluster) initUsers() error { | func (c *Cluster) initUsers() error { | ||||||
| 	c.setProcessName("initializing users") | 	c.setProcessName("initializing users") | ||||||
|  | 
 | ||||||
|  | 	// clear our the previous state of the cluster users (in case we are running a sync).
 | ||||||
|  | 	c.systemUsers = map[string]spec.PgUser{} | ||||||
|  | 	c.pgUsers = map[string]spec.PgUser{} | ||||||
|  | 
 | ||||||
| 	c.initSystemUsers() | 	c.initSystemUsers() | ||||||
| 
 | 
 | ||||||
| 	if err := c.initInfrastructureRoles(); err != nil { | 	if err := c.initInfrastructureRoles(); err != nil { | ||||||
|  | @ -615,6 +620,9 @@ func (c *Cluster) initRobotUsers() error { | ||||||
| 			return fmt.Errorf("invalid username: %q", username) | 			return fmt.Errorf("invalid username: %q", username) | ||||||
| 		} | 		} | ||||||
| 
 | 
 | ||||||
|  | 		if c.shouldAvoidProtectedOrSystemRole(username, "manifest robot role") { | ||||||
|  | 			continue | ||||||
|  | 		} | ||||||
| 		flags, err := normalizeUserFlags(userFlags) | 		flags, err := normalizeUserFlags(userFlags) | ||||||
| 		if err != nil { | 		if err != nil { | ||||||
| 			return fmt.Errorf("invalid flags for user %q: %v", username, err) | 			return fmt.Errorf("invalid flags for user %q: %v", username, err) | ||||||
|  | @ -648,6 +656,9 @@ func (c *Cluster) initHumanUsers() error { | ||||||
| 		flags := []string{constants.RoleFlagLogin} | 		flags := []string{constants.RoleFlagLogin} | ||||||
| 		memberOf := []string{c.OpConfig.PamRoleName} | 		memberOf := []string{c.OpConfig.PamRoleName} | ||||||
| 
 | 
 | ||||||
|  | 		if c.shouldAvoidProtectedOrSystemRole(username, "API role") { | ||||||
|  | 			continue | ||||||
|  | 		} | ||||||
| 		if c.OpConfig.EnableTeamSuperuser { | 		if c.OpConfig.EnableTeamSuperuser { | ||||||
| 			flags = append(flags, constants.RoleFlagSuperuser) | 			flags = append(flags, constants.RoleFlagSuperuser) | ||||||
| 		} else { | 		} else { | ||||||
|  | @ -677,6 +688,9 @@ func (c *Cluster) initInfrastructureRoles() error { | ||||||
| 		if !isValidUsername(username) { | 		if !isValidUsername(username) { | ||||||
| 			return fmt.Errorf("invalid username: '%v'", username) | 			return fmt.Errorf("invalid username: '%v'", username) | ||||||
| 		} | 		} | ||||||
|  | 		if c.shouldAvoidProtectedOrSystemRole(username, "infrastructure role") { | ||||||
|  | 			continue | ||||||
|  | 		} | ||||||
| 		flags, err := normalizeUserFlags(data.Flags) | 		flags, err := normalizeUserFlags(data.Flags) | ||||||
| 		if err != nil { | 		if err != nil { | ||||||
| 			return fmt.Errorf("invalid flags for user '%v': %v", username, err) | 			return fmt.Errorf("invalid flags for user '%v': %v", username, err) | ||||||
|  | @ -687,6 +701,18 @@ func (c *Cluster) initInfrastructureRoles() error { | ||||||
| 	return nil | 	return nil | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
|  | 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 | ||||||
|  | 	} | ||||||
|  | 	if c.isSystemUsername(username) { | ||||||
|  | 		c.logger.Warnf("cannot initialize a new %s with the name of the system user %q", purpose, username) | ||||||
|  | 		return true | ||||||
|  | 	} | ||||||
|  | 	return false | ||||||
|  | } | ||||||
|  | 
 | ||||||
| // GetCurrentProcess provides name of the last process of the cluster
 | // GetCurrentProcess provides name of the last process of the cluster
 | ||||||
| func (c *Cluster) GetCurrentProcess() spec.Process { | func (c *Cluster) GetCurrentProcess() spec.Process { | ||||||
| 	c.processMu.RLock() | 	c.processMu.RLock() | ||||||
|  |  | ||||||
|  | @ -4,6 +4,7 @@ import ( | ||||||
| 	"fmt" | 	"fmt" | ||||||
| 	"github.com/Sirupsen/logrus" | 	"github.com/Sirupsen/logrus" | ||||||
| 	"github.com/zalando-incubator/postgres-operator/pkg/spec" | 	"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/k8sutil" | ||||||
| 	"github.com/zalando-incubator/postgres-operator/pkg/util/teams" | 	"github.com/zalando-incubator/postgres-operator/pkg/util/teams" | ||||||
| 	"reflect" | 	"reflect" | ||||||
|  | @ -11,7 +12,10 @@ import ( | ||||||
| ) | ) | ||||||
| 
 | 
 | ||||||
| var logger = logrus.New().WithField("test", "cluster") | 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) { | func TestInitRobotUsers(t *testing.T) { | ||||||
| 	testName := "TestInitRobotUsers" | 	testName := "TestInitRobotUsers" | ||||||
|  | @ -47,6 +51,12 @@ func TestInitRobotUsers(t *testing.T) { | ||||||
| 			err: fmt.Errorf(`invalid flags for user "foobar": ` + | 			err: fmt.Errorf(`invalid flags for user "foobar": ` + | ||||||
| 				`conflicting user flags: "NOINHERIT" and "INHERIT"`), | 				`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 { | 	for _, tt := range tests { | ||||||
| 		cl.Spec.Users = tt.manifestUsers | 		cl.Spec.Users = tt.manifestUsers | ||||||
|  | @ -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"}}, | 			result: map[string]spec.PgUser{"foo": {Name: "foo", MemberOf: []string{cl.OpConfig.PamRoleName}, Flags: []string{"LOGIN", "SUPERUSER"}}, | ||||||
| 				"bar": {Name: "bar", Flags: []string{"NOLOGIN"}}}, | 				"bar": {Name: "bar", Flags: []string{"NOLOGIN"}}}, | ||||||
| 		}, | 		}, | ||||||
|  | 		{ | ||||||
|  | 			existingRoles: map[string]spec.PgUser{}, | ||||||
|  | 			teamRoles:     []string{"admin", "standby"}, | ||||||
|  | 			result:        map[string]spec.PgUser{}, | ||||||
|  | 		}, | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
| 	for _, tt := range tests { | 	for _, tt := range tests { | ||||||
|  |  | ||||||
|  | @ -60,6 +60,19 @@ func isValidUsername(username string) bool { | ||||||
| 	return userRegexp.MatchString(username) | 	return userRegexp.MatchString(username) | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
|  | func (c *Cluster) isProtectedUsername(username string) bool { | ||||||
|  | 	for _, protected := range c.OpConfig.ProtectedRoles { | ||||||
|  | 		if username == protected { | ||||||
|  | 			return true | ||||||
|  | 		} | ||||||
|  | 	} | ||||||
|  | 	return false | ||||||
|  | } | ||||||
|  | 
 | ||||||
|  | func (c *Cluster) isSystemUsername(username string) bool { | ||||||
|  | 	return (username == c.OpConfig.SuperUsername || username == c.OpConfig.ReplicationUsername) | ||||||
|  | } | ||||||
|  | 
 | ||||||
| func isValidFlag(flag string) bool { | func isValidFlag(flag string) bool { | ||||||
| 	for _, validFlag := range []string{constants.RoleFlagSuperuser, constants.RoleFlagLogin, constants.RoleFlagCreateDB, | 	for _, validFlag := range []string{constants.RoleFlagSuperuser, constants.RoleFlagLogin, constants.RoleFlagCreateDB, | ||||||
| 		constants.RoleFlagInherit, constants.RoleFlagReplication, constants.RoleFlagByPassRLS} { | 		constants.RoleFlagInherit, constants.RoleFlagReplication, constants.RoleFlagByPassRLS} { | ||||||
|  |  | ||||||
|  | @ -74,6 +74,7 @@ type Config struct { | ||||||
| 	ClusterHistoryEntries    int               `name:"cluster_history_entries" default:"1000"` | 	ClusterHistoryEntries    int               `name:"cluster_history_entries" default:"1000"` | ||||||
| 	TeamAPIRoleConfiguration map[string]string `name:"team_api_role_configuration" default:"log_statement:all"` | 	TeamAPIRoleConfiguration map[string]string `name:"team_api_role_configuration" default:"log_statement:all"` | ||||||
| 	PodTerminateGracePeriod  time.Duration     `name:"pod_terminate_grace_period" default:"5m"` | 	PodTerminateGracePeriod  time.Duration     `name:"pod_terminate_grace_period" default:"5m"` | ||||||
|  | 	ProtectedRoles           []string          `name:"protected_role_names" default:"admin"` | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| // MustMarshal marshals the config or panics
 | // MustMarshal marshals the config or panics
 | ||||||
|  |  | ||||||
		Loading…
	
		Reference in New Issue