From 8cf76d8372e07aebe332af4f748f41edadb88682 Mon Sep 17 00:00:00 2001 From: Rafia Sabih Date: Fri, 14 May 2021 17:10:51 +0200 Subject: [PATCH 01/21] Create cross namespace secrets --- pkg/cluster/cluster.go | 13 ++++++++++++- pkg/cluster/k8sres.go | 5 +++-- pkg/spec/types.go | 1 + 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index ff3a33af9..5271ed90b 100644 --- a/pkg/cluster/cluster.go +++ b/pkg/cluster/cluster.go @@ -1089,6 +1089,16 @@ func (c *Cluster) initRobotUsers() error { if c.shouldAvoidProtectedOrSystemRole(username, "manifest robot role") { continue } + name := username + namespace := "default" + + if strings.Contains(username, ".") { + splits := strings.Split(username, ".") + name = splits[1] + namespace = splits[0] + username = name + } + flags, err := normalizeUserFlags(userFlags) if err != nil { return fmt.Errorf("invalid flags for user %q: %v", username, err) @@ -1099,7 +1109,8 @@ func (c *Cluster) initRobotUsers() error { } newRole := spec.PgUser{ Origin: spec.RoleOriginManifest, - Name: username, + Name: name, + Namespace: namespace, Password: util.RandomPassword(constants.PasswordLength), Flags: flags, AdminRole: adminRole, diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index 9e4b045ab..98f64449b 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -1541,10 +1541,11 @@ func (c *Cluster) generateUserSecrets() map[string]*v1.Secret { namespace := c.Namespace for username, pgUser := range c.pgUsers { //Skip users with no password i.e. human users (they'll be authenticated using pam) - secret := c.generateSingleUserSecret(namespace, pgUser) + secret := c.generateSingleUserSecret(pgUser.Namespace, pgUser) if secret != nil { secrets[username] = secret } + namespace = pgUser.Namespace } /* special case for the system user */ for _, systemUser := range c.systemUsers { @@ -1584,7 +1585,7 @@ func (c *Cluster) generateSingleUserSecret(namespace string, pgUser spec.PgUser) secret := v1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: c.credentialSecretName(username), - Namespace: namespace, + Namespace: pgUser.Namespace, Labels: lbls, Annotations: c.annotationsSet(nil), }, diff --git a/pkg/spec/types.go b/pkg/spec/types.go index 78c79e1b3..06203bd46 100644 --- a/pkg/spec/types.go +++ b/pkg/spec/types.go @@ -48,6 +48,7 @@ const ( type PgUser struct { Origin RoleOrigin `yaml:"-"` Name string `yaml:"-"` + Namespace string `yaml:"."` Password string `yaml:"-"` Flags []string `yaml:"user_flags"` MemberOf []string `yaml:"inrole"` From 620502010e2381f0d624b9198d7caed623441b9e Mon Sep 17 00:00:00 2001 From: Rafia Sabih Date: Wed, 19 May 2021 05:19:23 +0200 Subject: [PATCH 02/21] add test cases --- e2e/tests/k8s_api.py | 3 ++ e2e/tests/test_e2e.py | 27 +++++++++++++--- pkg/cluster/cluster_test.go | 63 +++++++++++++++++++++++++++++++++++++ 3 files changed, 89 insertions(+), 4 deletions(-) diff --git a/e2e/tests/k8s_api.py b/e2e/tests/k8s_api.py index 85bcb6245..6c816524f 100644 --- a/e2e/tests/k8s_api.py +++ b/e2e/tests/k8s_api.py @@ -168,6 +168,9 @@ def count_endpoints_with_label(self, labels, namespace='default'): def count_secrets_with_label(self, labels, namespace='default'): return len(self.api.core_v1.list_namespaced_secret(namespace, label_selector=labels).items) + def count_secrets_in_namespace(self, labels, namespace): + return len(self.api.core_v1.list_namespaced_secret(namespace).items) + def count_statefulsets_with_label(self, labels, namespace='default'): return len(self.api.apps_v1.list_namespaced_stateful_set(namespace, label_selector=labels).items) diff --git a/e2e/tests/test_e2e.py b/e2e/tests/test_e2e.py index 114f881c4..422fec991 100644 --- a/e2e/tests/test_e2e.py +++ b/e2e/tests/test_e2e.py @@ -203,7 +203,7 @@ def test_additional_teams_and_members(self): self.k8s.update_config(enable_postgres_team_crd) self.eventuallyEqual(lambda: self.k8s.get_operator_state(), {"0": "idle"}, "Operator does not get in sync") - + self.k8s.api.custom_objects_api.patch_namespaced_custom_object( 'acid.zalan.do', 'v1', 'default', 'postgresteams', 'custom-team-membership', @@ -232,7 +232,7 @@ def test_additional_teams_and_members(self): WHERE usename IN ('elephant', 'kind'); """ users = self.query_database(leader.metadata.name, "postgres", user_query) - self.eventuallyEqual(lambda: len(users), 2, + self.eventuallyEqual(lambda: len(users), 2, "Not all additional users found in database: {}".format(users)) # revert config change @@ -414,7 +414,7 @@ def test_enable_disable_connection_pooler(self): db_list = self.list_databases(leader.metadata.name) for db in db_list: - self.eventuallyNotEqual(lambda: len(self.query_database(leader.metadata.name, db, schemas_query)), 0, + self.eventuallyNotEqual(lambda: len(self.query_database(leader.metadata.name, db, schemas_query)), 0, "Pooler schema not found in database {}".format(db)) # remove config section to make test work next time @@ -542,6 +542,25 @@ def verify_role(): print('Operator log: {}'.format(k8s.get_operator_log())) raise + @timeout_decorator.timeout(TEST_TIMEOUT_SEC) + def test_cross_namespace_secrets(self): + ''' + Test secrets in different namespace + ''' + k8s = self.k8s + k8s.api.custom_objects_api.patch_namespaced_custom_object( + 'acid.zalan.do', 'v1', 'default', + 'postgresqls', 'acid-minimal-cluster', + { + 'spec': { + 'users':{ + 'appspace.db_user': [], + } + } + }) + self.eventuallyEqual(lambda: k8s.count_secrets_in_namespace('appspace'), + 1, "Secret not created in user namespace") + @timeout_decorator.timeout(TEST_TIMEOUT_SEC) def test_lazy_spilo_upgrade(self): ''' @@ -744,7 +763,7 @@ def test_min_resource_limits(self): } } k8s.api.custom_objects_api.patch_namespaced_custom_object( - "acid.zalan.do", "v1", "default", "postgresqls", "acid-minimal-cluster", pg_patch_resources) + "acid.zalan.do", "v1", "default", "postgresqls", "acid-minimal-cluster", pg_patch_resources) self.eventuallyEqual(lambda: k8s.get_operator_state(), {"0": "idle"}, "Operator does not get in sync") self.eventuallyEqual(lambda: k8s.count_running_pods(), 2, "No two pods running after lazy rolling upgrade") diff --git a/pkg/cluster/cluster_test.go b/pkg/cluster/cluster_test.go index 1f6510e65..b4c497522 100644 --- a/pkg/cluster/cluster_test.go +++ b/pkg/cluster/cluster_test.go @@ -7,12 +7,14 @@ import ( "github.com/sirupsen/logrus" acidv1 "github.com/zalando/postgres-operator/pkg/apis/acid.zalan.do/v1" + fakeacidv1 "github.com/zalando/postgres-operator/pkg/generated/clientset/versioned/fake" "github.com/zalando/postgres-operator/pkg/spec" "github.com/zalando/postgres-operator/pkg/util/config" "github.com/zalando/postgres-operator/pkg/util/constants" "github.com/zalando/postgres-operator/pkg/util/k8sutil" "github.com/zalando/postgres-operator/pkg/util/teams" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes/fake" "k8s.io/client-go/tools/record" ) @@ -845,3 +847,64 @@ func TestPreparedDatabases(t *testing.T) { } } } + +func TestCrossNamespacedSecrets(t *testing.T) { + testName := "test secrets in different namespace" + clientSet := fake.NewSimpleClientset() + acidClientSet := fakeacidv1.NewSimpleClientset() + namespace := "default" + + client := k8sutil.KubernetesClient{ + StatefulSetsGetter: clientSet.AppsV1(), + ServicesGetter: clientSet.CoreV1(), + DeploymentsGetter: clientSet.AppsV1(), + PostgresqlsGetter: acidClientSet.AcidV1(), + SecretsGetter: clientSet.CoreV1(), + } + pg := acidv1.Postgresql{ + ObjectMeta: metav1.ObjectMeta{ + Name: "acid-fake-cluster", + Namespace: namespace, + }, + Spec: acidv1.PostgresSpec{ + Volume: acidv1.Volume{ + Size: "1Gi", + }, + Users: map[string]acidv1.UserFlags{ + "appspace.db_user": {}, + }, + }, + } + + var cluster = New( + Config{ + OpConfig: config.Config{ + ConnectionPooler: config.ConnectionPooler{ + ConnectionPoolerDefaultCPURequest: "100m", + ConnectionPoolerDefaultCPULimit: "100m", + ConnectionPoolerDefaultMemoryRequest: "100Mi", + ConnectionPoolerDefaultMemoryLimit: "100Mi", + NumberOfInstances: int32ToPointer(1), + }, + PodManagementPolicy: "ordered_ready", + Resources: config.Resources{ + ClusterLabels: map[string]string{"application": "spilo"}, + ClusterNameLabel: "cluster-name", + DefaultCPURequest: "300m", + DefaultCPULimit: "300m", + DefaultMemoryRequest: "300Mi", + DefaultMemoryLimit: "300Mi", + PodRoleLabel: "spilo-role", + }, + }, + }, client, pg, logger, eventRecorder) + + err := cluster.initRobotUsers() + if err != nil { + t.Errorf("%s Could not create namespaced users with error: %s", testName, err) + } + + if cluster.pgUsers["db_user"].Namespace == cluster.Namespace { + t.Errorf("%s: Could not create namespaced users", testName) + } +} From af719c07fe4373fb85e0907d73c3568833cef598 Mon Sep 17 00:00:00 2001 From: Rafia Sabih Date: Wed, 19 May 2021 17:39:56 +0200 Subject: [PATCH 03/21] fixes --- e2e/tests/test_e2e.py | 4 +++- pkg/cluster/cluster.go | 32 ++++++++++++++++++++------------ pkg/cluster/sync.go | 5 ++++- 3 files changed, 27 insertions(+), 14 deletions(-) diff --git a/e2e/tests/test_e2e.py b/e2e/tests/test_e2e.py index 422fec991..b6c36f3e5 100644 --- a/e2e/tests/test_e2e.py +++ b/e2e/tests/test_e2e.py @@ -547,7 +547,9 @@ def test_cross_namespace_secrets(self): ''' Test secrets in different namespace ''' + app_namespace = "appspace" k8s = self.k8s + k8s.api.core_v1.create_namespace(app_namespace) k8s.api.custom_objects_api.patch_namespaced_custom_object( 'acid.zalan.do', 'v1', 'default', 'postgresqls', 'acid-minimal-cluster', @@ -558,7 +560,7 @@ def test_cross_namespace_secrets(self): } } }) - self.eventuallyEqual(lambda: k8s.count_secrets_in_namespace('appspace'), + self.eventuallyEqual(lambda: k8s.count_secrets_in_namespace(app_namespace), 1, "Secret not created in user namespace") @timeout_decorator.timeout(TEST_TIMEOUT_SEC) diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index 5271ed90b..205f61c9a 100644 --- a/pkg/cluster/cluster.go +++ b/pkg/cluster/cluster.go @@ -924,14 +924,16 @@ 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), + Origin: spec.RoleOriginSystem, + Name: c.OpConfig.SuperUsername, + Namespace: c.Namespace, + Password: util.RandomPassword(constants.PasswordLength), } c.systemUsers[constants.ReplicationUserKeyName] = spec.PgUser{ - Origin: spec.RoleOriginSystem, - Name: c.OpConfig.ReplicationUsername, - Password: util.RandomPassword(constants.PasswordLength), + Origin: spec.RoleOriginSystem, + Name: c.OpConfig.ReplicationUsername, + Namespace: c.Namespace, + Password: util.RandomPassword(constants.PasswordLength), } // Connection pooler user is an exception, if requested it's going to be @@ -959,10 +961,11 @@ func (c *Cluster) initSystemUsers() { // connection pooler application should be able to login with this role connectionPoolerUser := spec.PgUser{ - Origin: spec.RoleConnectionPooler, - Name: username, - Flags: []string{constants.RoleFlagLogin}, - Password: util.RandomPassword(constants.PasswordLength), + Origin: spec.RoleConnectionPooler, + Name: username, + Namespace: c.Namespace, + Flags: []string{constants.RoleFlagLogin}, + Password: util.RandomPassword(constants.PasswordLength), } if _, exists := c.pgUsers[username]; !exists { @@ -1065,6 +1068,7 @@ func (c *Cluster) initDefaultRoles(defaultRoles map[string]string, admin, prefix newRole := spec.PgUser{ Origin: spec.RoleOriginBootstrap, Name: roleName, + Namespace: c.Namespace, Password: util.RandomPassword(constants.PasswordLength), Flags: flags, MemberOf: memberOf, @@ -1090,12 +1094,14 @@ func (c *Cluster) initRobotUsers() error { continue } name := username - namespace := "default" + namespace := c.Namespace if strings.Contains(username, ".") { splits := strings.Split(username, ".") name = splits[1] - namespace = splits[0] + if splits[0] != "" { + namespace = splits[0] + } username = name } @@ -1149,6 +1155,7 @@ func (c *Cluster) initTeamMembers(teamID string, isPostgresSuperuserTeam bool) e newRole := spec.PgUser{ Origin: spec.RoleOriginTeamsAPI, Name: username, + Namespace: c.Namespace, Flags: flags, MemberOf: memberOf, Parameters: c.OpConfig.TeamAPIRoleConfiguration, @@ -1228,6 +1235,7 @@ func (c *Cluster) initInfrastructureRoles() error { return fmt.Errorf("invalid flags for user '%v': %v", username, err) } newRole.Flags = flags + newRole.Namespace = c.Namespace if currentRole, present := c.pgUsers[username]; present { c.pgUsers[username] = c.resolveNameConflict(¤tRole, &newRole) diff --git a/pkg/cluster/sync.go b/pkg/cluster/sync.go index 3036a9942..8d4241c37 100644 --- a/pkg/cluster/sync.go +++ b/pkg/cluster/sync.go @@ -481,6 +481,9 @@ func (c *Cluster) syncSecrets() error { secrets := c.generateUserSecrets() for secretUsername, secretSpec := range secrets { + if len(secretSpec.Namespace) < 0 { + c.logger.Warningf("found empty namespace for user %s", secretUsername) + } if secret, err = c.KubeClient.Secrets(secretSpec.Namespace).Create(context.TODO(), secretSpec, metav1.CreateOptions{}); err == nil { c.Secrets[secret.UID] = secret c.logger.Debugf("created new secret %s, uid: %s", util.NameFromMeta(secret.ObjectMeta), secret.UID) @@ -521,7 +524,7 @@ func (c *Cluster) syncSecrets() error { userMap[secretUsername] = pwdUser } } else { - return fmt.Errorf("could not create secret for user %s: %v", secretUsername, err) + return fmt.Errorf("could not create secret for user %s: in namespace %s: %v", secretUsername, secretSpec.Namespace, err) } } From 43154baf39ae676357748b327d5fc061d6ddd877 Mon Sep 17 00:00:00 2001 From: Rafia Sabih Date: Thu, 20 May 2021 19:22:00 +0200 Subject: [PATCH 04/21] Fixes - include namespace in secret name only when namespace is provided - use username.namespace as key to pgUsers only when namespace is provided - avoid conflict in the role creation in db by checking namespace alongwith the username --- pkg/cluster/cluster.go | 6 +++++- pkg/cluster/k8sres.go | 7 +++++-- pkg/cluster/resources.go | 2 +- pkg/cluster/sync.go | 11 ++++++----- pkg/spec/types.go | 6 +++--- 5 files changed, 20 insertions(+), 12 deletions(-) diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index 205f61c9a..42366781f 100644 --- a/pkg/cluster/cluster.go +++ b/pkg/cluster/cluster.go @@ -1122,7 +1122,11 @@ func (c *Cluster) initRobotUsers() error { AdminRole: adminRole, } if currentRole, present := c.pgUsers[username]; present { - c.pgUsers[username] = c.resolveNameConflict(¤tRole, &newRole) + if namespace == c.pgUsers[username].Namespace { + c.pgUsers[username] = c.resolveNameConflict(¤tRole, &newRole) + } else { + c.pgUsers[username+"."+namespace] = newRole + } } else { c.pgUsers[username] = newRole } diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index 98f64449b..af01bd1b9 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -1581,10 +1581,13 @@ func (c *Cluster) generateSingleUserSecret(namespace string, pgUser spec.PgUser) if username == constants.ConnectionPoolerUserName { lbls = c.connectionPoolerLabels("", false).MatchLabels } - + secret_name := username + if pgUser.Namespace != c.Namespace { + secret_name = username + "." + pgUser.Namespace + } secret := v1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Name: c.credentialSecretName(username), + Name: c.credentialSecretName(secret_name), Namespace: pgUser.Namespace, Labels: lbls, Annotations: c.annotationsSet(nil), diff --git a/pkg/cluster/resources.go b/pkg/cluster/resources.go index 48b17f532..f078c6434 100644 --- a/pkg/cluster/resources.go +++ b/pkg/cluster/resources.go @@ -32,7 +32,7 @@ func (c *Cluster) listResources() error { } for _, obj := range c.Secrets { - c.logger.Infof("found secret: %q (uid: %q)", util.NameFromMeta(obj.ObjectMeta), obj.UID) + c.logger.Infof("found secret: %q (uid: %q) namesapce: %s", util.NameFromMeta(obj.ObjectMeta), obj.UID, obj.ObjectMeta.Namespace) } for role, endpoint := range c.Endpoints { diff --git a/pkg/cluster/sync.go b/pkg/cluster/sync.go index 8d4241c37..112a0cf1f 100644 --- a/pkg/cluster/sync.go +++ b/pkg/cluster/sync.go @@ -481,12 +481,9 @@ func (c *Cluster) syncSecrets() error { secrets := c.generateUserSecrets() for secretUsername, secretSpec := range secrets { - if len(secretSpec.Namespace) < 0 { - c.logger.Warningf("found empty namespace for user %s", secretUsername) - } if secret, err = c.KubeClient.Secrets(secretSpec.Namespace).Create(context.TODO(), secretSpec, metav1.CreateOptions{}); err == nil { c.Secrets[secret.UID] = secret - c.logger.Debugf("created new secret %s, uid: %s", util.NameFromMeta(secret.ObjectMeta), secret.UID) + c.logger.Debugf("created new secret %s, namespace: %s, uid: %s", util.NameFromMeta(secret.ObjectMeta), secretSpec.Namespace, secret.UID) continue } if k8sutil.ResourceAlreadyExists(err) { @@ -555,7 +552,11 @@ func (c *Cluster) syncRoles() (err error) { }() for _, u := range c.pgUsers { - userNames = append(userNames, u.Name) + if u.Namespace != c.Namespace { + userNames = append(userNames, u.Name+"."+"u.Namespace") + } else { + userNames = append(userNames, u.Name) + } } if needMasterConnectionPooler(&c.Spec) || needReplicaConnectionPooler(&c.Spec) { diff --git a/pkg/spec/types.go b/pkg/spec/types.go index 06203bd46..66b4465cb 100644 --- a/pkg/spec/types.go +++ b/pkg/spec/types.go @@ -46,9 +46,9 @@ const ( // PgUser contains information about a single user. type PgUser struct { - Origin RoleOrigin `yaml:"-"` - Name string `yaml:"-"` - Namespace string `yaml:"."` + Origin RoleOrigin `yaml:"-"` + Name string `yaml:"-"` + Namespace string Password string `yaml:"-"` Flags []string `yaml:"user_flags"` MemberOf []string `yaml:"inrole"` From 188e812e406a176fc2fa2fb1bdd85acbea8e0a68 Mon Sep 17 00:00:00 2001 From: Rafia Sabih Date: Thu, 20 May 2021 20:28:01 +0200 Subject: [PATCH 05/21] Update unit tests --- pkg/cluster/cluster.go | 3 +-- pkg/cluster/cluster_test.go | 20 ++++++++++++++++---- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index 42366781f..9937deff3 100644 --- a/pkg/cluster/cluster.go +++ b/pkg/cluster/cluster.go @@ -1099,7 +1099,7 @@ func (c *Cluster) initRobotUsers() error { if strings.Contains(username, ".") { splits := strings.Split(username, ".") name = splits[1] - if splits[0] != "" { + if len(splits[0]) > 0 { namespace = splits[0] } username = name @@ -1159,7 +1159,6 @@ func (c *Cluster) initTeamMembers(teamID string, isPostgresSuperuserTeam bool) e newRole := spec.PgUser{ Origin: spec.RoleOriginTeamsAPI, Name: username, - Namespace: c.Namespace, Flags: flags, MemberOf: memberOf, Parameters: c.OpConfig.TeamAPIRoleConfiguration, diff --git a/pkg/cluster/cluster_test.go b/pkg/cluster/cluster_test.go index b4c497522..63f63d382 100644 --- a/pkg/cluster/cluster_test.go +++ b/pkg/cluster/cluster_test.go @@ -81,8 +81,8 @@ func TestInitRobotUsers(t *testing.T) { }{ { manifestUsers: map[string]acidv1.UserFlags{"foo": {"superuser", "createdb"}}, - infraRoles: map[string]spec.PgUser{"foo": {Origin: spec.RoleOriginInfrastructure, Name: "foo", Password: "bar"}}, - result: map[string]spec.PgUser{"foo": {Origin: spec.RoleOriginInfrastructure, Name: "foo", Password: "bar"}}, + infraRoles: map[string]spec.PgUser{"foo": {Origin: spec.RoleOriginInfrastructure, Name: "foo", Namespace: cl.Namespace, Password: "bar"}}, + result: map[string]spec.PgUser{"foo": {Origin: spec.RoleOriginInfrastructure, Name: "foo", Namespace: cl.Namespace, Password: "bar"}}, err: nil, }, { @@ -872,6 +872,7 @@ func TestCrossNamespacedSecrets(t *testing.T) { }, Users: map[string]acidv1.UserFlags{ "appspace.db_user": {}, + "db_user": {}, }, }, } @@ -899,12 +900,23 @@ func TestCrossNamespacedSecrets(t *testing.T) { }, }, client, pg, logger, eventRecorder) + userNamespaceMap := map[string]string{ + cluster.Namespace: "db_user", + "appspace": "db_user", + } + err := cluster.initRobotUsers() if err != nil { t.Errorf("%s Could not create namespaced users with error: %s", testName, err) } - if cluster.pgUsers["db_user"].Namespace == cluster.Namespace { - t.Errorf("%s: Could not create namespaced users", testName) + for _, u := range cluster.pgUsers { + if u.Name != userNamespaceMap[u.Namespace] { + t.Errorf("%s: Could not create namespaced user in its correct namespaces", testName) + } + } + err = cluster.syncRoles() + if err != nil { + t.Errorf("%s Could not create namespaced users with error: %s", testName, err) } } From f57204721db66ace92f71740792886b3fd9e8c48 Mon Sep 17 00:00:00 2001 From: Rafia Sabih Date: Fri, 21 May 2021 07:49:44 +0200 Subject: [PATCH 06/21] Fix test case --- pkg/cluster/cluster_test.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/pkg/cluster/cluster_test.go b/pkg/cluster/cluster_test.go index 63f63d382..7ccad19f1 100644 --- a/pkg/cluster/cluster_test.go +++ b/pkg/cluster/cluster_test.go @@ -915,8 +915,4 @@ func TestCrossNamespacedSecrets(t *testing.T) { t.Errorf("%s: Could not create namespaced user in its correct namespaces", testName) } } - err = cluster.syncRoles() - if err != nil { - t.Errorf("%s Could not create namespaced users with error: %s", testName, err) - } } From fd5edea670f977c9eac11f5cf648d4a4d12bd7a7 Mon Sep 17 00:00:00 2001 From: Rafia Sabih Date: Thu, 27 May 2021 13:25:54 +0200 Subject: [PATCH 07/21] Fixes - update regular expression for usernames - add test to allow check for valid usernames - create pg roles with namespace (if any) appended in rolename --- pkg/cluster/cluster.go | 18 +++++------------- pkg/cluster/cluster_test.go | 16 ++++++++++++++-- pkg/cluster/k8sres.go | 7 ++----- pkg/cluster/sync.go | 8 +++++--- 4 files changed, 26 insertions(+), 23 deletions(-) diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index 9937deff3..17e8c6747 100644 --- a/pkg/cluster/cluster.go +++ b/pkg/cluster/cluster.go @@ -41,7 +41,7 @@ import ( var ( alphaNumericRegexp = regexp.MustCompile("^[a-zA-Z][a-zA-Z0-9]*$") databaseNameRegexp = regexp.MustCompile("^[a-zA-Z_][a-zA-Z0-9_]*$") - userRegexp = regexp.MustCompile(`^[a-z0-9]([-_a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-_a-z0-9]*[a-z0-9])?)*$`) + userRegexp = regexp.MustCompile(`^[a-z0-9,]+\.?[-_a-z0-9,]+[a-z0-9,]$`) patroniObjectSuffixes = []string{"config", "failover", "sync"} ) @@ -1093,16 +1093,12 @@ func (c *Cluster) initRobotUsers() error { if c.shouldAvoidProtectedOrSystemRole(username, "manifest robot role") { continue } - name := username namespace := c.Namespace + //more than one dot in the username is reported invalid by regexp if strings.Contains(username, ".") { splits := strings.Split(username, ".") - name = splits[1] - if len(splits[0]) > 0 { - namespace = splits[0] - } - username = name + namespace = splits[0] } flags, err := normalizeUserFlags(userFlags) @@ -1115,18 +1111,14 @@ func (c *Cluster) initRobotUsers() error { } newRole := spec.PgUser{ Origin: spec.RoleOriginManifest, - Name: name, + Name: username, Namespace: namespace, Password: util.RandomPassword(constants.PasswordLength), Flags: flags, AdminRole: adminRole, } if currentRole, present := c.pgUsers[username]; present { - if namespace == c.pgUsers[username].Namespace { - c.pgUsers[username] = c.resolveNameConflict(¤tRole, &newRole) - } else { - c.pgUsers[username+"."+namespace] = newRole - } + c.pgUsers[username] = c.resolveNameConflict(¤tRole, &newRole) } else { c.pgUsers[username] = newRole } diff --git a/pkg/cluster/cluster_test.go b/pkg/cluster/cluster_test.go index 7ccad19f1..df46a38f3 100644 --- a/pkg/cluster/cluster_test.go +++ b/pkg/cluster/cluster_test.go @@ -902,7 +902,7 @@ func TestCrossNamespacedSecrets(t *testing.T) { userNamespaceMap := map[string]string{ cluster.Namespace: "db_user", - "appspace": "db_user", + "appspace": "appspace.db_user", } err := cluster.initRobotUsers() @@ -912,7 +912,19 @@ func TestCrossNamespacedSecrets(t *testing.T) { for _, u := range cluster.pgUsers { if u.Name != userNamespaceMap[u.Namespace] { - t.Errorf("%s: Could not create namespaced user in its correct namespaces", testName) + t.Errorf("%s: Could not create namespaced user in its correct namespaces for user %s in namespace %s", testName, u.Name, u.Namespace) + } + } +} + +func TestValidUsernames(t *testing.T) { + testName := "test username validity" + + invalidUsernames := []string{"_", ".", ".user", "appspace.", "appspace.user.extra", "user_", "_user", "-user", "user-"} + + for _, username := range invalidUsernames { + if isValidUsername(username) { + t.Errorf("%s Invalid username is allowed: %s", testName, username) } } } diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index af01bd1b9..98f64449b 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -1581,13 +1581,10 @@ func (c *Cluster) generateSingleUserSecret(namespace string, pgUser spec.PgUser) if username == constants.ConnectionPoolerUserName { lbls = c.connectionPoolerLabels("", false).MatchLabels } - secret_name := username - if pgUser.Namespace != c.Namespace { - secret_name = username + "." + pgUser.Namespace - } + secret := v1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Name: c.credentialSecretName(secret_name), + Name: c.credentialSecretName(username), Namespace: pgUser.Namespace, Labels: lbls, Annotations: c.annotationsSet(nil), diff --git a/pkg/cluster/sync.go b/pkg/cluster/sync.go index 112a0cf1f..43e56d14c 100644 --- a/pkg/cluster/sync.go +++ b/pkg/cluster/sync.go @@ -552,11 +552,13 @@ func (c *Cluster) syncRoles() (err error) { }() for _, u := range c.pgUsers { + pg_role := u.Name if u.Namespace != c.Namespace { - userNames = append(userNames, u.Name+"."+"u.Namespace") - } else { - userNames = append(userNames, u.Name) + // to avoid the conflict of having multiple users of same name + // but each in different namespace. + pg_role = fmt.Sprintf("%s.%s", u.Name, u.Namespace) } + userNames = append(userNames, pg_role) } if needMasterConnectionPooler(&c.Spec) || needReplicaConnectionPooler(&c.Spec) { From 25a6417d479b6319b307135e2aa22d3f7e3f5694 Mon Sep 17 00:00:00 2001 From: Rafia Sabih Date: Thu, 27 May 2021 14:28:57 +0200 Subject: [PATCH 08/21] add more test cases for valid usernames --- pkg/cluster/cluster_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cluster/cluster_test.go b/pkg/cluster/cluster_test.go index df46a38f3..c118e82a4 100644 --- a/pkg/cluster/cluster_test.go +++ b/pkg/cluster/cluster_test.go @@ -920,7 +920,7 @@ func TestCrossNamespacedSecrets(t *testing.T) { func TestValidUsernames(t *testing.T) { testName := "test username validity" - invalidUsernames := []string{"_", ".", ".user", "appspace.", "appspace.user.extra", "user_", "_user", "-user", "user-"} + invalidUsernames := []string{"_", ".", ".user", "appspace.", "appspace.user.extra", "user_", "_user", "-user", "user-", ",", ",user", "user,", "namespace,user"} for _, username := range invalidUsernames { if isValidUsername(username) { From 0f31b5bea68021166eb69cdd4f430f9f01b75788 Mon Sep 17 00:00:00 2001 From: Rafia Sabih Date: Thu, 27 May 2021 14:43:49 +0200 Subject: [PATCH 09/21] update docs --- charts/postgres-operator/values-crd.yaml | 2 +- charts/postgres-operator/values.yaml | 2 +- docs/reference/operator_parameters.md | 15 ++++++++------- manifests/configmap.yaml | 2 +- ...postgresql-operator-default-configuration.yaml | 2 +- pkg/cluster/cluster.go | 2 +- 6 files changed, 13 insertions(+), 12 deletions(-) diff --git a/charts/postgres-operator/values-crd.yaml b/charts/postgres-operator/values-crd.yaml index bd563a636..9c482e0a4 100644 --- a/charts/postgres-operator/values-crd.yaml +++ b/charts/postgres-operator/values-crd.yaml @@ -146,7 +146,7 @@ configKubernetes: # Postgres pods are terminated forcefully after this timeout pod_terminate_grace_period: 5m # template for database user secrets generated by the operator - secret_name_template: "{username}.{cluster}.credentials.{tprkind}.{tprgroup}" + secret_name_template: "{namespace}.{username}.{cluster}.credentials.{tprkind}.{tprgroup}" # set user and group for the spilo container (required to run Spilo as non-root process) # spilo_runasuser: "101" # spilo_runasgroup: "103" diff --git a/charts/postgres-operator/values.yaml b/charts/postgres-operator/values.yaml index 15a53a00d..3c6349bcc 100644 --- a/charts/postgres-operator/values.yaml +++ b/charts/postgres-operator/values.yaml @@ -138,7 +138,7 @@ configKubernetes: # Postgres pods are terminated forcefully after this timeout pod_terminate_grace_period: 5m # template for database user secrets generated by the operator - secret_name_template: "{username}.{cluster}.credentials.{tprkind}.{tprgroup}" + secret_name_template: "{namespace}.{username}.{cluster}.credentials.{tprkind}.{tprgroup}" # set user and group for the spilo container (required to run Spilo as non-root process) # spilo_runasuser: "101" # spilo_runasgroup: "103" diff --git a/docs/reference/operator_parameters.md b/docs/reference/operator_parameters.md index b0d982943..980e27c4c 100644 --- a/docs/reference/operator_parameters.md +++ b/docs/reference/operator_parameters.md @@ -172,11 +172,11 @@ under the `users` key. ## Major version upgrades -Parameters configuring automatic major version upgrades. In a +Parameters configuring automatic major version upgrades. In a CRD-configuration, they are grouped under the `major_version_upgrade` key. * **major_version_upgrade_mode** - Postgres Operator supports [in-place major version upgrade](../administrator.md#in-place-major-version-upgrade) + Postgres Operator supports [in-place major version upgrade](../administrator.md#in-place-major-version-upgrade) with three different modes: `"off"` = no upgrade by the operator, `"manual"` = manifest triggers action, @@ -275,11 +275,12 @@ configuration they are grouped under the `kubernetes` key. * **secret_name_template** a template for the name of the database user secrets generated by the - operator. `{username}` is replaced with name of the secret, `{cluster}` with - the name of the cluster, `{tprkind}` with the kind of CRD (formerly known as - TPR) and `{tprgroup}` with the group of the CRD. No other placeholders are - allowed. The default is - `{username}.{cluster}.credentials.{tprkind}.{tprgroup}`. + operator. `{namesapce}` is replaced with name of the namespace (if any, + otherwise empty), `{username}` is replaced with name of the secret, + `{cluster}` with the name of the cluster, `{tprkind}` with the kind of CRD + (formerly known as TPR) and `{tprgroup}` with the group of the CRD. + No other placeholders are allowed. The default is + `{namesapce}.{username}.{cluster}.credentials.{tprkind}.{tprgroup}`. * **cluster_domain** defines the default DNS domain for the kubernetes cluster the operator is diff --git a/manifests/configmap.yaml b/manifests/configmap.yaml index b379975eb..7898735bd 100644 --- a/manifests/configmap.yaml +++ b/manifests/configmap.yaml @@ -113,7 +113,7 @@ data: resync_period: 30m ring_log_lines: "100" role_deletion_suffix: "_deleted" - secret_name_template: "{username}.{cluster}.credentials" + secret_name_template: "{namespace}.{username}.{cluster}.credentials" # sidecar_docker_images: "" # set_memory_request_to_limit: "false" spilo_allow_privilege_escalation: "true" diff --git a/manifests/postgresql-operator-default-configuration.yaml b/manifests/postgresql-operator-default-configuration.yaml index 65dfd6ce4..1af4a9ce4 100644 --- a/manifests/postgresql-operator-default-configuration.yaml +++ b/manifests/postgresql-operator-default-configuration.yaml @@ -78,7 +78,7 @@ configuration: pod_service_account_name: postgres-pod # pod_service_account_role_binding_definition: "" pod_terminate_grace_period: 5m - secret_name_template: "{username}.{cluster}.credentials.{tprkind}.{tprgroup}" + secret_name_template: "{namespace}.{username}.{cluster}.credentials.{tprkind}.{tprgroup}" spilo_allow_privilege_escalation: true # spilo_runasuser: 101 # spilo_runasgroup: 103 diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index 59c4975e8..ff5fd994b 100644 --- a/pkg/cluster/cluster.go +++ b/pkg/cluster/cluster.go @@ -41,7 +41,7 @@ import ( var ( alphaNumericRegexp = regexp.MustCompile("^[a-zA-Z][a-zA-Z0-9]*$") databaseNameRegexp = regexp.MustCompile("^[a-zA-Z_][a-zA-Z0-9_]*$") - userRegexp = regexp.MustCompile(`^[a-z0-9,]+\.?[-_a-z0-9,]+[a-z0-9,]$`) + userRegexp = regexp.MustCompile(`^[a-z0-9]+\.?[-_a-z0-9]+[a-z0-9]$`) patroniObjectSuffixes = []string{"config", "failover", "sync"} ) From 09039e8730797db5508cf701ed144eb6a28e64b4 Mon Sep 17 00:00:00 2001 From: Rafia Sabih Date: Mon, 31 May 2021 16:07:30 +0200 Subject: [PATCH 10/21] fixes as per review comments --- charts/postgres-operator/values-crd.yaml | 6 ++++-- charts/postgres-operator/values.yaml | 6 ++++-- docs/reference/operator_parameters.md | 7 ++++--- e2e/tests/test_e2e.py | 10 ++++++++-- manifests/configmap.yaml | 2 +- ...gresql-operator-default-configuration.yaml | 2 +- pkg/cluster/cluster_test.go | 4 ++-- pkg/cluster/sync.go | 19 ++++++++++++++++--- 8 files changed, 40 insertions(+), 16 deletions(-) diff --git a/charts/postgres-operator/values-crd.yaml b/charts/postgres-operator/values-crd.yaml index 9c482e0a4..d14c9d335 100644 --- a/charts/postgres-operator/values-crd.yaml +++ b/charts/postgres-operator/values-crd.yaml @@ -145,8 +145,10 @@ configKubernetes: # Postgres pods are terminated forcefully after this timeout pod_terminate_grace_period: 5m - # template for database user secrets generated by the operator - secret_name_template: "{namespace}.{username}.{cluster}.credentials.{tprkind}.{tprgroup}" + # template for database user secrets generated by the operator, + # here username contains the namespace in the format namespace.username + # if the user is in different namespace than cluster + secret_name_template: "{username}.{cluster}.credentials.{tprkind}.{tprgroup}" # set user and group for the spilo container (required to run Spilo as non-root process) # spilo_runasuser: "101" # spilo_runasgroup: "103" diff --git a/charts/postgres-operator/values.yaml b/charts/postgres-operator/values.yaml index 3c6349bcc..8827a13f1 100644 --- a/charts/postgres-operator/values.yaml +++ b/charts/postgres-operator/values.yaml @@ -137,8 +137,10 @@ configKubernetes: # Postgres pods are terminated forcefully after this timeout pod_terminate_grace_period: 5m - # template for database user secrets generated by the operator - secret_name_template: "{namespace}.{username}.{cluster}.credentials.{tprkind}.{tprgroup}" + # template for database user secrets generated by the operator, + # here username contains the namespace in the format namespace.username + # if the user is in different namespace than cluster + secret_name_template: "{username}.{cluster}.credentials.{tprkind}.{tprgroup}" # set user and group for the spilo container (required to run Spilo as non-root process) # spilo_runasuser: "101" # spilo_runasgroup: "103" diff --git a/docs/reference/operator_parameters.md b/docs/reference/operator_parameters.md index 980e27c4c..59dd4297d 100644 --- a/docs/reference/operator_parameters.md +++ b/docs/reference/operator_parameters.md @@ -275,12 +275,13 @@ configuration they are grouped under the `kubernetes` key. * **secret_name_template** a template for the name of the database user secrets generated by the - operator. `{namesapce}` is replaced with name of the namespace (if any, - otherwise empty), `{username}` is replaced with name of the secret, + operator. `{namespace}` is replaced with name of the namespace (if any, + otherwise the secret is in cluster's namespace and in that case it is not + present in secret name), `{username}` is replaced with name of the secret, `{cluster}` with the name of the cluster, `{tprkind}` with the kind of CRD (formerly known as TPR) and `{tprgroup}` with the group of the CRD. No other placeholders are allowed. The default is - `{namesapce}.{username}.{cluster}.credentials.{tprkind}.{tprgroup}`. + `{namespace}.{username}.{cluster}.credentials.{tprkind}.{tprgroup}`. * **cluster_domain** defines the default DNS domain for the kubernetes cluster the operator is diff --git a/e2e/tests/test_e2e.py b/e2e/tests/test_e2e.py index 7a414379f..bd3566d5f 100644 --- a/e2e/tests/test_e2e.py +++ b/e2e/tests/test_e2e.py @@ -594,7 +594,13 @@ def test_cross_namespace_secrets(self): ''' app_namespace = "appspace" k8s = self.k8s - k8s.api.core_v1.create_namespace(app_namespace) + v1_appnamespace = client.V1Namespace(metadata=client.V1ObjectMeta(name=app_namespace)) + try: + k8s.api.core_v1.create_namespace(v1_appnamespace) + except timeout_decorator.TimeoutError: + print('Operator log: {}'.format(k8s.get_operator_log())) + raise + k8s.api.custom_objects_api.patch_namespaced_custom_object( 'acid.zalan.do', 'v1', 'default', 'postgresqls', 'acid-minimal-cluster', @@ -606,7 +612,7 @@ def test_cross_namespace_secrets(self): } }) self.eventuallyEqual(lambda: k8s.count_secrets_in_namespace(app_namespace), - 1, "Secret not created in user namespace") + 1, "Secret not created for user in namespace", app_namespace) @timeout_decorator.timeout(TEST_TIMEOUT_SEC) def test_lazy_spilo_upgrade(self): diff --git a/manifests/configmap.yaml b/manifests/configmap.yaml index 7898735bd..b379975eb 100644 --- a/manifests/configmap.yaml +++ b/manifests/configmap.yaml @@ -113,7 +113,7 @@ data: resync_period: 30m ring_log_lines: "100" role_deletion_suffix: "_deleted" - secret_name_template: "{namespace}.{username}.{cluster}.credentials" + secret_name_template: "{username}.{cluster}.credentials" # sidecar_docker_images: "" # set_memory_request_to_limit: "false" spilo_allow_privilege_escalation: "true" diff --git a/manifests/postgresql-operator-default-configuration.yaml b/manifests/postgresql-operator-default-configuration.yaml index 1af4a9ce4..65dfd6ce4 100644 --- a/manifests/postgresql-operator-default-configuration.yaml +++ b/manifests/postgresql-operator-default-configuration.yaml @@ -78,7 +78,7 @@ configuration: pod_service_account_name: postgres-pod # pod_service_account_role_binding_definition: "" pod_terminate_grace_period: 5m - secret_name_template: "{namespace}.{username}.{cluster}.credentials.{tprkind}.{tprgroup}" + secret_name_template: "{username}.{cluster}.credentials.{tprkind}.{tprgroup}" spilo_allow_privilege_escalation: true # spilo_runasuser: 101 # spilo_runasgroup: 103 diff --git a/pkg/cluster/cluster_test.go b/pkg/cluster/cluster_test.go index c118e82a4..8859d2730 100644 --- a/pkg/cluster/cluster_test.go +++ b/pkg/cluster/cluster_test.go @@ -907,7 +907,7 @@ func TestCrossNamespacedSecrets(t *testing.T) { err := cluster.initRobotUsers() if err != nil { - t.Errorf("%s Could not create namespaced users with error: %s", testName, err) + t.Errorf("Could not create secret for namespaced users with error: %s", err) } for _, u := range cluster.pgUsers { @@ -920,7 +920,7 @@ func TestCrossNamespacedSecrets(t *testing.T) { func TestValidUsernames(t *testing.T) { testName := "test username validity" - invalidUsernames := []string{"_", ".", ".user", "appspace.", "appspace.user.extra", "user_", "_user", "-user", "user-", ",", ",user", "user,", "namespace,user"} + invalidUsernames := []string{"_", ".", ".user", "appspace.", "appspace.user.extra", "user_", "_user", "-user", "user-", ",", "-", ",user", "user,", "namespace,user"} for _, username := range invalidUsernames { if isValidUsername(username) { diff --git a/pkg/cluster/sync.go b/pkg/cluster/sync.go index b8068b6e9..ac64747be 100644 --- a/pkg/cluster/sync.go +++ b/pkg/cluster/sync.go @@ -556,13 +556,26 @@ func (c *Cluster) syncRoles() (err error) { // create list of database roles to query for _, u := range c.pgUsers { - pg_role := u.Name + pgRole := u.Name if u.Namespace != c.Namespace { // to avoid the conflict of having multiple users of same name // but each in different namespace. - pg_role = fmt.Sprintf("%s.%s", u.Name, u.Namespace) + pgRole = fmt.Sprintf("%s.%s", u.Name, u.Namespace) + } + userNames = append(userNames, pgRole) + // add team member role name with rename suffix in case we need to rename it back + if u.Origin == spec.RoleOriginTeamsAPI && c.OpConfig.EnableTeamMemberDeprecation { + deletedUsers[u.Name+c.OpConfig.RoleDeletionSuffix] = u.Name + userNames = append(userNames, u.Name+c.OpConfig.RoleDeletionSuffix) + } + } + + // add team members that exist only in cache + // to trigger a rename of the role in ProduceSyncRequests + for _, cachedUser := range c.pgUsersCache { + if _, exists := c.pgUsers[cachedUser.Name]; !exists { + userNames = append(userNames, cachedUser.Name) } - userNames = append(userNames, pg_role) } // add pooler user to list of pgUsers, too From c0bfca9225095c0084995846a4f37808c85cde51 Mon Sep 17 00:00:00 2001 From: Rafia Sabih Date: Mon, 31 May 2021 17:04:58 +0200 Subject: [PATCH 11/21] update e2e --- e2e/tests/k8s_api.py | 3 --- e2e/tests/test_e2e.py | 3 +-- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/e2e/tests/k8s_api.py b/e2e/tests/k8s_api.py index 6c816524f..85bcb6245 100644 --- a/e2e/tests/k8s_api.py +++ b/e2e/tests/k8s_api.py @@ -168,9 +168,6 @@ def count_endpoints_with_label(self, labels, namespace='default'): def count_secrets_with_label(self, labels, namespace='default'): return len(self.api.core_v1.list_namespaced_secret(namespace, label_selector=labels).items) - def count_secrets_in_namespace(self, labels, namespace): - return len(self.api.core_v1.list_namespaced_secret(namespace).items) - def count_statefulsets_with_label(self, labels, namespace='default'): return len(self.api.apps_v1.list_namespaced_stateful_set(namespace, label_selector=labels).items) diff --git a/e2e/tests/test_e2e.py b/e2e/tests/test_e2e.py index bd3566d5f..342e62838 100644 --- a/e2e/tests/test_e2e.py +++ b/e2e/tests/test_e2e.py @@ -322,7 +322,6 @@ def test_overwrite_pooler_deployment(self): self.eventuallyEqual(lambda: self.k8s.count_running_pods("connection-pooler=acid-minimal-cluster-pooler"), 0, "Pooler pods not scaled down") - @timeout_decorator.timeout(TEST_TIMEOUT_SEC) def test_enable_disable_connection_pooler(self): ''' @@ -611,7 +610,7 @@ def test_cross_namespace_secrets(self): } } }) - self.eventuallyEqual(lambda: k8s.count_secrets_in_namespace(app_namespace), + self.eventuallyEqual(lambda: k8s.count_secrets_with_label("cluster_name=acid-minimal-cluster,application=spilo", app_namespace), 1, "Secret not created for user in namespace", app_namespace) @timeout_decorator.timeout(TEST_TIMEOUT_SEC) From f0472fd4431b2acdf7aeef32ccacb113fbae3f1b Mon Sep 17 00:00:00 2001 From: Rafia Sabih Date: Fri, 4 Jun 2021 16:53:06 +0200 Subject: [PATCH 12/21] fixes --- charts/postgres-operator/values-crd.yaml | 2 +- e2e/tests/k8s_api.py | 11 +++++++++++ e2e/tests/test_e2e.py | 7 ++----- 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/charts/postgres-operator/values-crd.yaml b/charts/postgres-operator/values-crd.yaml index 4adcb8b06..b5436fafd 100644 --- a/charts/postgres-operator/values-crd.yaml +++ b/charts/postgres-operator/values-crd.yaml @@ -147,7 +147,7 @@ configKubernetes: pod_terminate_grace_period: 5m # template for database user secrets generated by the operator, # here username contains the namespace in the format namespace.username - # if the user is in different namespace than cluster + # if the secret is in different namespace than cluster secret_name_template: "{username}.{cluster}.credentials.{tprkind}.{tprgroup}" # set user and group for the spilo container (required to run Spilo as non-root process) # spilo_runasuser: "101" diff --git a/e2e/tests/k8s_api.py b/e2e/tests/k8s_api.py index 85bcb6245..3333636ff 100644 --- a/e2e/tests/k8s_api.py +++ b/e2e/tests/k8s_api.py @@ -362,6 +362,17 @@ def wait_for_pod_start(self, pod_labels, namespace='default'): time.sleep(self.RETRY_TIMEOUT_SEC) + def wait_for_namespace_creation(self, namespace='default'): + ns_found = False + while ns_found != True: + ns = self.api.core_v1.list_namespaces() + for n in ns: + if n == "appspace": + ns_found = True + break + + time.sleep(self.RETRY_TIMEOUT_SEC) + def get_service_type(self, svc_labels, namespace='default'): svc_type = '' svcs = self.api.core_v1.list_namespaced_service(namespace, label_selector=svc_labels, limit=1).items diff --git a/e2e/tests/test_e2e.py b/e2e/tests/test_e2e.py index 342e62838..bf52ba670 100644 --- a/e2e/tests/test_e2e.py +++ b/e2e/tests/test_e2e.py @@ -594,11 +594,8 @@ def test_cross_namespace_secrets(self): app_namespace = "appspace" k8s = self.k8s v1_appnamespace = client.V1Namespace(metadata=client.V1ObjectMeta(name=app_namespace)) - try: - k8s.api.core_v1.create_namespace(v1_appnamespace) - except timeout_decorator.TimeoutError: - print('Operator log: {}'.format(k8s.get_operator_log())) - raise + k8s.api.core_v1.create_namespace(v1_appnamespace) + k8s.wait_for_namespace_creation(app_namespace) k8s.api.custom_objects_api.patch_namespaced_custom_object( 'acid.zalan.do', 'v1', 'default', From a992494fbff6a58cba73ed139eae65a1bafa255b Mon Sep 17 00:00:00 2001 From: Rafia Sabih Date: Sun, 6 Jun 2021 13:03:31 +0200 Subject: [PATCH 13/21] Add toggle to allow namespaced secrets --- .../postgres-operator/crds/postgresqls.yaml | 2 + e2e/tests/k8s_api.py | 21 +++++---- e2e/tests/test_e2e.py | 15 ++++--- manifests/complete-postgres-manifest.yaml | 3 ++ pkg/apis/acid.zalan.do/v1/crds.go | 3 ++ pkg/apis/acid.zalan.do/v1/postgresql_type.go | 43 ++++++++++--------- pkg/cluster/cluster.go | 13 +++--- pkg/cluster/cluster_test.go | 10 ++++- 8 files changed, 64 insertions(+), 46 deletions(-) diff --git a/charts/postgres-operator/crds/postgresqls.yaml b/charts/postgres-operator/crds/postgresqls.yaml index aead7fe69..eb628863d 100644 --- a/charts/postgres-operator/crds/postgresqls.yaml +++ b/charts/postgres-operator/crds/postgresqls.yaml @@ -515,6 +515,8 @@ spec: type: integer useLoadBalancer: # deprecated type: boolean + enableNamespacedSecret: + type: boolean users: type: object additionalProperties: diff --git a/e2e/tests/k8s_api.py b/e2e/tests/k8s_api.py index 3333636ff..d28ea69ad 100644 --- a/e2e/tests/k8s_api.py +++ b/e2e/tests/k8s_api.py @@ -197,6 +197,16 @@ def wait_for_pod_failover(self, failover_targets, labels, namespace='default'): pod_phase = pods[0].status.phase time.sleep(self.RETRY_TIMEOUT_SEC) + def wait_for_namespace_creation(self, namespace='default'): + ns_found = False + while ns_found != True: + ns = self.api.core_v1.list_namespace().items + for n in ns: + if n.metadata.name == namespace: + ns_found = True + break + time.sleep(self.RETRY_TIMEOUT_SEC) + def get_logical_backup_job(self, namespace='default'): return self.api.batch_v1_beta1.list_namespaced_cron_job(namespace, label_selector="application=spilo") @@ -362,17 +372,6 @@ def wait_for_pod_start(self, pod_labels, namespace='default'): time.sleep(self.RETRY_TIMEOUT_SEC) - def wait_for_namespace_creation(self, namespace='default'): - ns_found = False - while ns_found != True: - ns = self.api.core_v1.list_namespaces() - for n in ns: - if n == "appspace": - ns_found = True - break - - time.sleep(self.RETRY_TIMEOUT_SEC) - def get_service_type(self, svc_labels, namespace='default'): svc_type = '' svcs = self.api.core_v1.list_namespaced_service(namespace, label_selector=svc_labels, limit=1).items diff --git a/e2e/tests/test_e2e.py b/e2e/tests/test_e2e.py index bf52ba670..bd1b2b6a1 100644 --- a/e2e/tests/test_e2e.py +++ b/e2e/tests/test_e2e.py @@ -314,7 +314,7 @@ def test_overwrite_pooler_deployment(self): 'postgresqls', 'acid-minimal-cluster', { 'spec': { - 'enableConnectionPooler': False + c } }) @@ -592,23 +592,24 @@ def test_cross_namespace_secrets(self): Test secrets in different namespace ''' app_namespace = "appspace" - k8s = self.k8s + # k8s = self.k8s v1_appnamespace = client.V1Namespace(metadata=client.V1ObjectMeta(name=app_namespace)) - k8s.api.core_v1.create_namespace(v1_appnamespace) - k8s.wait_for_namespace_creation(app_namespace) + self.k8s.api.core_v1.create_namespace(v1_appnamespace) + self.k8s.wait_for_namespace_creation(app_namespace) - k8s.api.custom_objects_api.patch_namespaced_custom_object( + self.k8s.api.custom_objects_api.patch_namespaced_custom_object( 'acid.zalan.do', 'v1', 'default', 'postgresqls', 'acid-minimal-cluster', { 'spec': { + 'enableNamespacedSecret': True, 'users':{ 'appspace.db_user': [], } } }) - self.eventuallyEqual(lambda: k8s.count_secrets_with_label("cluster_name=acid-minimal-cluster,application=spilo", app_namespace), - 1, "Secret not created for user in namespace", app_namespace) + self.eventuallyEqual(lambda: self.k8s.count_secrets_with_label("cluster_name=acid-minimal-cluster,application=spilo", app_namespace), + 1, "Secret not created for user in namespace") @timeout_decorator.timeout(TEST_TIMEOUT_SEC) def test_lazy_spilo_upgrade(self): diff --git a/manifests/complete-postgres-manifest.yaml b/manifests/complete-postgres-manifest.yaml index 6e2acbdd3..a474482e9 100644 --- a/manifests/complete-postgres-manifest.yaml +++ b/manifests/complete-postgres-manifest.yaml @@ -12,7 +12,10 @@ spec: dockerImage: registry.opensource.zalan.do/acid/spilo-13:2.0-p7 teamId: "acid" numberOfInstances: 2 + enableNamespacedSecret: True users: # Application/Robot users + appspace.db_user: [] + appspace.db_user.with.dots: [] zalando: - superuser - createdb diff --git a/pkg/apis/acid.zalan.do/v1/crds.go b/pkg/apis/acid.zalan.do/v1/crds.go index 83e7273e4..ae91a9f38 100644 --- a/pkg/apis/acid.zalan.do/v1/crds.go +++ b/pkg/apis/acid.zalan.do/v1/crds.go @@ -730,6 +730,9 @@ var PostgresCRDResourceValidation = apiextv1.CustomResourceValidation{ Type: "boolean", Description: "Deprecated", }, + "enableNamespacedSecret": { + Type: "boolean", + }, "users": { Type: "object", AdditionalProperties: &apiextv1.JSONSchemaPropsOrBool{ diff --git a/pkg/apis/acid.zalan.do/v1/postgresql_type.go b/pkg/apis/acid.zalan.do/v1/postgresql_type.go index 7346fb0e5..1787f5b4e 100644 --- a/pkg/apis/acid.zalan.do/v1/postgresql_type.go +++ b/pkg/apis/acid.zalan.do/v1/postgresql_type.go @@ -53,27 +53,28 @@ type PostgresSpec struct { // load balancers' source ranges are the same for master and replica services AllowedSourceRanges []string `json:"allowedSourceRanges"` - NumberOfInstances int32 `json:"numberOfInstances"` - Users map[string]UserFlags `json:"users,omitempty"` - MaintenanceWindows []MaintenanceWindow `json:"maintenanceWindows,omitempty"` - Clone *CloneDescription `json:"clone,omitempty"` - ClusterName string `json:"-"` - Databases map[string]string `json:"databases,omitempty"` - PreparedDatabases map[string]PreparedDatabase `json:"preparedDatabases,omitempty"` - SchedulerName *string `json:"schedulerName,omitempty"` - NodeAffinity *v1.NodeAffinity `json:"nodeAffinity,omitempty"` - Tolerations []v1.Toleration `json:"tolerations,omitempty"` - Sidecars []Sidecar `json:"sidecars,omitempty"` - InitContainers []v1.Container `json:"initContainers,omitempty"` - PodPriorityClassName string `json:"podPriorityClassName,omitempty"` - ShmVolume *bool `json:"enableShmVolume,omitempty"` - EnableLogicalBackup bool `json:"enableLogicalBackup,omitempty"` - LogicalBackupSchedule string `json:"logicalBackupSchedule,omitempty"` - StandbyCluster *StandbyDescription `json:"standby,omitempty"` - PodAnnotations map[string]string `json:"podAnnotations,omitempty"` - ServiceAnnotations map[string]string `json:"serviceAnnotations,omitempty"` - TLS *TLSDescription `json:"tls,omitempty"` - AdditionalVolumes []AdditionalVolume `json:"additionalVolumes,omitempty"` + NumberOfInstances int32 `json:"numberOfInstances"` + EnableNamespacedSecret *bool `json:"enableNamespacedSecret,omitempty"` + Users map[string]UserFlags `json:"users,omitempty"` + MaintenanceWindows []MaintenanceWindow `json:"maintenanceWindows,omitempty"` + Clone *CloneDescription `json:"clone,omitempty"` + ClusterName string `json:"-"` + Databases map[string]string `json:"databases,omitempty"` + PreparedDatabases map[string]PreparedDatabase `json:"preparedDatabases,omitempty"` + SchedulerName *string `json:"schedulerName,omitempty"` + NodeAffinity *v1.NodeAffinity `json:"nodeAffinity,omitempty"` + Tolerations []v1.Toleration `json:"tolerations,omitempty"` + Sidecars []Sidecar `json:"sidecars,omitempty"` + InitContainers []v1.Container `json:"initContainers,omitempty"` + PodPriorityClassName string `json:"podPriorityClassName,omitempty"` + ShmVolume *bool `json:"enableShmVolume,omitempty"` + EnableLogicalBackup bool `json:"enableLogicalBackup,omitempty"` + LogicalBackupSchedule string `json:"logicalBackupSchedule,omitempty"` + StandbyCluster *StandbyDescription `json:"standby,omitempty"` + PodAnnotations map[string]string `json:"podAnnotations,omitempty"` + ServiceAnnotations map[string]string `json:"serviceAnnotations,omitempty"` + TLS *TLSDescription `json:"tls,omitempty"` + AdditionalVolumes []AdditionalVolume `json:"additionalVolumes,omitempty"` // deprecated json tags InitContainersOld []v1.Container `json:"init_containers,omitempty"` diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index 2051dd3ef..ced184877 100644 --- a/pkg/cluster/cluster.go +++ b/pkg/cluster/cluster.go @@ -41,7 +41,7 @@ import ( var ( alphaNumericRegexp = regexp.MustCompile("^[a-zA-Z][a-zA-Z0-9]*$") databaseNameRegexp = regexp.MustCompile("^[a-zA-Z_][a-zA-Z0-9_]*$") - userRegexp = regexp.MustCompile(`^[a-z0-9]+\.?[-_a-z0-9]+[a-z0-9]$`) + userRegexp = regexp.MustCompile(`^[a-z0-9]([-_a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-_a-z0-9]*[a-z0-9])?)*$`) patroniObjectSuffixes = []string{"config", "failover", "sync"} ) @@ -1111,10 +1111,13 @@ func (c *Cluster) initRobotUsers() error { } namespace := c.Namespace - //more than one dot in the username is reported invalid by regexp - if strings.Contains(username, ".") { - splits := strings.Split(username, ".") - namespace = splits[0] + //if namespaced secrets are allowed + if c.Postgresql.Spec.EnableNamespacedSecret != nil && + *c.Postgresql.Spec.EnableNamespacedSecret { + if strings.Contains(username, ".") { + splits := strings.Split(username, ".") + namespace = splits[0] + } } flags, err := normalizeUserFlags(userFlags) diff --git a/pkg/cluster/cluster_test.go b/pkg/cluster/cluster_test.go index 8859d2730..cb9356f47 100644 --- a/pkg/cluster/cluster_test.go +++ b/pkg/cluster/cluster_test.go @@ -870,6 +870,7 @@ func TestCrossNamespacedSecrets(t *testing.T) { Volume: acidv1.Volume{ Size: "1Gi", }, + EnableNamespacedSecret: boolToPointer(true), Users: map[string]acidv1.UserFlags{ "appspace.db_user": {}, "db_user": {}, @@ -920,11 +921,16 @@ func TestCrossNamespacedSecrets(t *testing.T) { func TestValidUsernames(t *testing.T) { testName := "test username validity" - invalidUsernames := []string{"_", ".", ".user", "appspace.", "appspace.user.extra", "user_", "_user", "-user", "user-", ",", "-", ",user", "user,", "namespace,user"} - + invalidUsernames := []string{"_", ".", ".user", "appspace.", "user_", "_user", "-user", "user-", ",", "-", ",user", "user,", "namespace,user"} + validUsernames := []string{"user", "appspace.user", "appspace.dot.user", "user_name", "app_space.user_name"} for _, username := range invalidUsernames { if isValidUsername(username) { t.Errorf("%s Invalid username is allowed: %s", testName, username) } } + for _, username := range validUsernames { + if !isValidUsername(username) { + t.Errorf("%s Valid username is not allowed: %s", testName, username) + } + } } From 9e1d906c4fa83bd461d234df29f45c4d3ea75a96 Mon Sep 17 00:00:00 2001 From: Rafia Sabih Date: Mon, 7 Jun 2021 11:33:53 +0200 Subject: [PATCH 14/21] update docs --- charts/postgres-operator/values.yaml | 3 ++- docs/reference/operator_parameters.md | 13 +++++++------ docs/user.md | 21 ++++++++++++++++++++- 3 files changed, 29 insertions(+), 8 deletions(-) diff --git a/charts/postgres-operator/values.yaml b/charts/postgres-operator/values.yaml index b156b1c26..ec5e6d6f2 100644 --- a/charts/postgres-operator/values.yaml +++ b/charts/postgres-operator/values.yaml @@ -139,7 +139,8 @@ configKubernetes: pod_terminate_grace_period: 5m # template for database user secrets generated by the operator, # here username contains the namespace in the format namespace.username - # if the user is in different namespace than cluster + # if the user is in different namespace than cluster and cross namespace secrets + # are enabled via EnableNamespacedSecret flag. secret_name_template: "{username}.{cluster}.credentials.{tprkind}.{tprgroup}" # set user and group for the spilo container (required to run Spilo as non-root process) # spilo_runasuser: "101" diff --git a/docs/reference/operator_parameters.md b/docs/reference/operator_parameters.md index ed8ab3259..1b1ae852e 100644 --- a/docs/reference/operator_parameters.md +++ b/docs/reference/operator_parameters.md @@ -275,12 +275,13 @@ configuration they are grouped under the `kubernetes` key. * **secret_name_template** a template for the name of the database user secrets generated by the - operator. `{namespace}` is replaced with name of the namespace (if any, - otherwise the secret is in cluster's namespace and in that case it is not - present in secret name), `{username}` is replaced with name of the secret, - `{cluster}` with the name of the cluster, `{tprkind}` with the kind of CRD - (formerly known as TPR) and `{tprgroup}` with the group of the CRD. - No other placeholders are allowed. The default is + operator. `{namespace}` is replaced with name of the namespace (if cross + namespace secrets are enabled via EnableNamespacedSecret flag, otherwise the + secret is in cluster's namespace and in that case it is not present in secret + name), `{username}` is replaced with name of the secret, `{cluster}` with the + name of the cluster, `{tprkind}` with the kind of CRD (formerly known as TPR) + and `{tprgroup}` with the group of the CRD. No other placeholders are allowed. + The default is `{namespace}.{username}.{cluster}.credentials.{tprkind}.{tprgroup}`. * **cluster_domain** diff --git a/docs/user.md b/docs/user.md index 8e406ec00..8194d2ced 100644 --- a/docs/user.md +++ b/docs/user.md @@ -139,6 +139,25 @@ secret, without ever sharing it outside of the cluster. At the moment it is not possible to define membership of the manifest role in other roles. +To define the secrets for the users in a different namespace than that of the cluster, +one can use the flag `EnableNamespacedSecret` and declare the namespace for the +secrets in the manifest in the following manner, + +```yaml +spec: + users: + #users with secret in dfferent namespace + appspace.db_user: + - createdb +``` +Here, anything before the first dot is taken as the namespace and the text after +the first dot is the username. Also, the postgres roles of these usernames would +be in the form of `namespace.username`. + +For such usernames, the secret is created in the given namespace and its name is +of the following form, +`{namespace}.{username}.{team}-{clustername}.credentials.postgresql.acid.zalan.do` + ### Infrastructure roles An infrastructure role is a role that should be present on every PostgreSQL @@ -330,7 +349,7 @@ spec: This creates roles for members of the `c-team` team not only in all clusters owned by `a-team`, but as well in cluster owned by `b-team`, as `a-team` is -an `additionalTeam` to `b-team` +an `additionalTeam` to `b-team` Not, you can also define `additionalSuperuserTeams` in the `PostgresTeam` manifest. By default, this option is disabled and must be configured with From 2f893540e884250f88be5051b0ad0bfe5606db0d Mon Sep 17 00:00:00 2001 From: Rafia Sabih Date: Mon, 7 Jun 2021 16:57:52 +0200 Subject: [PATCH 15/21] comment update --- charts/postgres-operator/values-crd.yaml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/charts/postgres-operator/values-crd.yaml b/charts/postgres-operator/values-crd.yaml index b5436fafd..b9659d91e 100644 --- a/charts/postgres-operator/values-crd.yaml +++ b/charts/postgres-operator/values-crd.yaml @@ -147,7 +147,8 @@ configKubernetes: pod_terminate_grace_period: 5m # template for database user secrets generated by the operator, # here username contains the namespace in the format namespace.username - # if the secret is in different namespace than cluster + # if the secret is in different namespace than cluster and cross namespace secrets + # are enabled via EnableNamespacedSecret flag. secret_name_template: "{username}.{cluster}.credentials.{tprkind}.{tprgroup}" # set user and group for the spilo container (required to run Spilo as non-root process) # spilo_runasuser: "101" From 1ed916ff5261c4e47a4f2166e44a37e2aaaff4ba Mon Sep 17 00:00:00 2001 From: Felix Kunde Date: Tue, 8 Jun 2021 18:49:44 +0200 Subject: [PATCH 16/21] Update e2e/tests/test_e2e.py --- e2e/tests/test_e2e.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2e/tests/test_e2e.py b/e2e/tests/test_e2e.py index bd1b2b6a1..c0319f6de 100644 --- a/e2e/tests/test_e2e.py +++ b/e2e/tests/test_e2e.py @@ -314,7 +314,7 @@ def test_overwrite_pooler_deployment(self): 'postgresqls', 'acid-minimal-cluster', { 'spec': { - c + 'enableConnectionPooler': False } }) From fcd9d27f18f9b04c33ad0fed464031f4da30dbb7 Mon Sep 17 00:00:00 2001 From: Felix Kunde Date: Wed, 9 Jun 2021 10:22:25 +0200 Subject: [PATCH 17/21] few minor fixes --- pkg/cluster/cluster.go | 1 + pkg/cluster/database.go | 2 +- pkg/cluster/sync.go | 4 ++-- pkg/spec/types.go | 6 +++--- 4 files changed, 7 insertions(+), 6 deletions(-) diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index ced184877..8f5f047d8 100644 --- a/pkg/cluster/cluster.go +++ b/pkg/cluster/cluster.go @@ -1170,6 +1170,7 @@ func (c *Cluster) initTeamMembers(teamID string, isPostgresSuperuserTeam bool) e newRole := spec.PgUser{ Origin: spec.RoleOriginTeamsAPI, Name: username, + Namespace: c.Namespace, Flags: flags, MemberOf: memberOf, Parameters: c.OpConfig.TeamAPIRoleConfiguration, diff --git a/pkg/cluster/database.go b/pkg/cluster/database.go index 829c2e5c7..3d4216dfb 100644 --- a/pkg/cluster/database.go +++ b/pkg/cluster/database.go @@ -221,7 +221,7 @@ func (c *Cluster) readPgUsersFromDatabase(userNames []string) (users spec.PgUser roldeleted = true } - users[rolname] = spec.PgUser{Name: rolname, Password: rolpassword, Flags: flags, MemberOf: memberof, Parameters: parameters, Deleted: roldeleted} + users[rolname] = spec.PgUser{Name: rolname, Namespace: c.Namespace, Password: rolpassword, Flags: flags, MemberOf: memberof, Parameters: parameters, Deleted: roldeleted} } return users, nil diff --git a/pkg/cluster/sync.go b/pkg/cluster/sync.go index 3fa0648c0..dcf577abb 100644 --- a/pkg/cluster/sync.go +++ b/pkg/cluster/sync.go @@ -565,8 +565,8 @@ func (c *Cluster) syncRoles() (err error) { userNames = append(userNames, pgRole) // add team member role name with rename suffix in case we need to rename it back if u.Origin == spec.RoleOriginTeamsAPI && c.OpConfig.EnableTeamMemberDeprecation { - deletedUsers[u.Name+c.OpConfig.RoleDeletionSuffix] = u.Name - userNames = append(userNames, u.Name+c.OpConfig.RoleDeletionSuffix) + deletedUsers[pgRole+c.OpConfig.RoleDeletionSuffix] = pgRole + userNames = append(userNames, pgRole+c.OpConfig.RoleDeletionSuffix) } } diff --git a/pkg/spec/types.go b/pkg/spec/types.go index 4e7306294..533aae79f 100644 --- a/pkg/spec/types.go +++ b/pkg/spec/types.go @@ -47,9 +47,9 @@ const ( // PgUser contains information about a single user. type PgUser struct { - Origin RoleOrigin `yaml:"-"` - Name string `yaml:"-"` - Namespace string + Origin RoleOrigin `yaml:"-"` + Name string `yaml:"-"` + Namespace string `yaml:"-"` Password string `yaml:"-"` Flags []string `yaml:"user_flags"` MemberOf []string `yaml:"inrole"` From 27c6ac256533daaac41e54ac43c0ea3324f74d8c Mon Sep 17 00:00:00 2001 From: Felix Kunde Date: Wed, 9 Jun 2021 10:37:08 +0200 Subject: [PATCH 18/21] fix unit tests --- pkg/cluster/cluster.go | 1 - pkg/cluster/database.go | 2 +- pkg/cluster/sync.go | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index 8f5f047d8..ced184877 100644 --- a/pkg/cluster/cluster.go +++ b/pkg/cluster/cluster.go @@ -1170,7 +1170,6 @@ func (c *Cluster) initTeamMembers(teamID string, isPostgresSuperuserTeam bool) e newRole := spec.PgUser{ Origin: spec.RoleOriginTeamsAPI, Name: username, - Namespace: c.Namespace, Flags: flags, MemberOf: memberOf, Parameters: c.OpConfig.TeamAPIRoleConfiguration, diff --git a/pkg/cluster/database.go b/pkg/cluster/database.go index 3d4216dfb..829c2e5c7 100644 --- a/pkg/cluster/database.go +++ b/pkg/cluster/database.go @@ -221,7 +221,7 @@ func (c *Cluster) readPgUsersFromDatabase(userNames []string) (users spec.PgUser roldeleted = true } - users[rolname] = spec.PgUser{Name: rolname, Namespace: c.Namespace, Password: rolpassword, Flags: flags, MemberOf: memberof, Parameters: parameters, Deleted: roldeleted} + users[rolname] = spec.PgUser{Name: rolname, Password: rolpassword, Flags: flags, MemberOf: memberof, Parameters: parameters, Deleted: roldeleted} } return users, nil diff --git a/pkg/cluster/sync.go b/pkg/cluster/sync.go index dcf577abb..79dceedd5 100644 --- a/pkg/cluster/sync.go +++ b/pkg/cluster/sync.go @@ -557,7 +557,7 @@ func (c *Cluster) syncRoles() (err error) { // create list of database roles to query for _, u := range c.pgUsers { pgRole := u.Name - if u.Namespace != c.Namespace { + if u.Namespace != c.Namespace && u.Namespace != "" { // to avoid the conflict of having multiple users of same name // but each in different namespace. pgRole = fmt.Sprintf("%s.%s", u.Name, u.Namespace) From 8f183c210703b553ccfef948fc31a78f3160c6de Mon Sep 17 00:00:00 2001 From: Rafia Sabih Date: Wed, 9 Jun 2021 16:08:17 +0200 Subject: [PATCH 19/21] fix e2e --- e2e/tests/test_e2e.py | 2 +- manifests/complete-postgres-manifest.yaml | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/e2e/tests/test_e2e.py b/e2e/tests/test_e2e.py index c0319f6de..c1eb14d61 100644 --- a/e2e/tests/test_e2e.py +++ b/e2e/tests/test_e2e.py @@ -608,7 +608,7 @@ def test_cross_namespace_secrets(self): } } }) - self.eventuallyEqual(lambda: self.k8s.count_secrets_with_label("cluster_name=acid-minimal-cluster,application=spilo", app_namespace), + self.eventuallyEqual(lambda: self.k8s.count_secrets_with_label("cluster-name=acid-minimal-cluster,application=spilo", app_namespace), 1, "Secret not created for user in namespace") @timeout_decorator.timeout(TEST_TIMEOUT_SEC) diff --git a/manifests/complete-postgres-manifest.yaml b/manifests/complete-postgres-manifest.yaml index a474482e9..5f995de15 100644 --- a/manifests/complete-postgres-manifest.yaml +++ b/manifests/complete-postgres-manifest.yaml @@ -12,10 +12,8 @@ spec: dockerImage: registry.opensource.zalan.do/acid/spilo-13:2.0-p7 teamId: "acid" numberOfInstances: 2 - enableNamespacedSecret: True + enableNamespacedSecret: False users: # Application/Robot users - appspace.db_user: [] - appspace.db_user.with.dots: [] zalando: - superuser - createdb From 880c8a8e18cce2e079a470db4087d62ba5c1c67e Mon Sep 17 00:00:00 2001 From: Rafia Sabih Date: Wed, 9 Jun 2021 17:15:16 +0200 Subject: [PATCH 20/21] fix e2e attempt 2 --- e2e/tests/test_e2e.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/e2e/tests/test_e2e.py b/e2e/tests/test_e2e.py index c1eb14d61..d0c56258e 100644 --- a/e2e/tests/test_e2e.py +++ b/e2e/tests/test_e2e.py @@ -587,12 +587,12 @@ def verify_role(): raise @timeout_decorator.timeout(TEST_TIMEOUT_SEC) - def test_cross_namespace_secrets(self): + def test_zz_cross_namespace_secrets(self): ''' Test secrets in different namespace ''' app_namespace = "appspace" - # k8s = self.k8s + v1_appnamespace = client.V1Namespace(metadata=client.V1ObjectMeta(name=app_namespace)) self.k8s.api.core_v1.create_namespace(v1_appnamespace) self.k8s.wait_for_namespace_creation(app_namespace) @@ -611,6 +611,16 @@ def test_cross_namespace_secrets(self): self.eventuallyEqual(lambda: self.k8s.count_secrets_with_label("cluster-name=acid-minimal-cluster,application=spilo", app_namespace), 1, "Secret not created for user in namespace") + #reset the flag + self.k8s.api.custom_objects_api.patch_namespaced_custom_object( + 'acid.zalan.do', 'v1', 'default', + 'postgresqls', 'acid-minimal-cluster', + { + 'spec': { + 'enableNamespacedSecret': False, + } + }) + @timeout_decorator.timeout(TEST_TIMEOUT_SEC) def test_lazy_spilo_upgrade(self): ''' From 4ca2c5667d43f2321caa3dab58a669b71f92cc94 Mon Sep 17 00:00:00 2001 From: Rafia Sabih Date: Thu, 10 Jun 2021 12:55:43 +0200 Subject: [PATCH 21/21] fix e2e --- e2e/tests/test_e2e.py | 1 + 1 file changed, 1 insertion(+) diff --git a/e2e/tests/test_e2e.py b/e2e/tests/test_e2e.py index d0c56258e..fcac70e10 100644 --- a/e2e/tests/test_e2e.py +++ b/e2e/tests/test_e2e.py @@ -567,6 +567,7 @@ def verify_role(): role.pop("Password", None) self.assertDictEqual(role, { "Name": "robot_zmon_acid_monitoring_new", + "Namespace":"", "Flags": None, "MemberOf": ["robot_zmon"], "Parameters": None,