Skip to content

Create cross namespace secrets #1490

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 24 commits into from
Jun 11, 2021
Merged

Create cross namespace secrets #1490

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
8cf76d8
Create cross namespace secrets
May 14, 2021
6205020
add test cases
May 19, 2021
af719c0
fixes
May 19, 2021
43154ba
Fixes
May 20, 2021
188e812
Update unit tests
May 20, 2021
f572047
Fix test case
May 21, 2021
fd5edea
Fixes
May 27, 2021
4e9816b
merge master
May 27, 2021
25a6417
add more test cases for valid usernames
May 27, 2021
0f31b5b
update docs
May 27, 2021
09039e8
fixes as per review comments
May 31, 2021
c0bfca9
update e2e
May 31, 2021
b2171be
Merge branch 'master' of https://github.com/zalando/postgres-operator…
May 31, 2021
f0472fd
fixes
Jun 4, 2021
a992494
Add toggle to allow namespaced secrets
Jun 6, 2021
9e1d906
update docs
Jun 7, 2021
2f89354
comment update
Jun 7, 2021
1ed916f
Update e2e/tests/test_e2e.py
FxKu Jun 8, 2021
775aa34
update codegen and merge with master
FxKu Jun 8, 2021
fcd9d27
few minor fixes
FxKu Jun 9, 2021
27c6ac2
fix unit tests
FxKu Jun 9, 2021
8f183c2
fix e2e
Jun 9, 2021
880c8a8
fix e2e attempt 2
Jun 9, 2021
4ca2c56
fix e2e
Jun 10, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions e2e/tests/k8s_api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
27 changes: 23 additions & 4 deletions e2e/tests/test_e2e.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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")
Copy link
Member

@sdudoladov sdudoladov May 27, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To make it more informative, please log the name of the secret and the namespace


@timeout_decorator.timeout(TEST_TIMEOUT_SEC)
def test_lazy_spilo_upgrade(self):
'''
Expand Down Expand Up @@ -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")
Expand Down
13 changes: 12 additions & 1 deletion pkg/cluster/cluster.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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, ".") {
Copy link
Member

@Jan-M Jan-M May 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs some attention.

Also include update to regex in manifest schema.

splits := strings.Split(username, ".")
name = splits[1]
namespace = splits[0]
Copy link
Member

@FxKu FxKu May 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what if the username is .user? Then namespace is "" and should become the default namespace

Copy link
Contributor Author

@RafiaSabih RafiaSabih May 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right now, such a username is considered invalid username and I think this should continue. I don't see a reason why this should be allowed, if no namespace is provided then username only should be provided.

username = name
}

flags, err := normalizeUserFlags(userFlags)
if err != nil {
return fmt.Errorf("invalid flags for user %q: %v", username, err)
Expand All @@ -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,
Expand Down
63 changes: 63 additions & 0 deletions pkg/cluster/cluster_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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)
Copy link
Member

@sdudoladov sdudoladov May 27, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Does namespaced user here really mean namespaced secret ?
  2. I doubt there is a need to log a test name, the golang test framework likely reports it automatically

}

if cluster.pgUsers["db_user"].Namespace == cluster.Namespace {
t.Errorf("%s: Could not create namespaced users", testName)
}
}
5 changes: 3 additions & 2 deletions pkg/cluster/k8sres.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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),
},
Expand Down
1 change: 1 addition & 0 deletions pkg/spec/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down