K8SPSMDB-1537: connection string secrets#2397
Open
pooknull wants to merge 15 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds support for generating and maintaining Kubernetes Secrets containing MongoDB connection strings (standard and SRV) for the database admin user and for custom users, alongside some naming/URI-building refactors to support this.
Changes:
- Introduces reconciliation logic to create/update per-user “connection string” Secrets (system databaseAdmin + custom users) and extends E2E coverage to validate the secrets work.
- Refactors Mongo client configuration to support
authSourceand to generatemongodb://andmongodb+srv://URIs from a shared config object. - Centralizes Service/Secret naming helpers and updates service discovery to accommodate mongos “service-per-pod” addressing.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/psmdb/service.go | Uses naming helpers, adjusts mongos address lookup signature, minor refactors. |
| pkg/psmdb/mongo/mongo.go | Adds URI/SRVURI builders and Config.Options(); refactors Dial to use Options(). |
| pkg/psmdb/client.go | Splits config building from dialing; adds AuthSource plumbing and MongosConfig/MongoConfig helpers. |
| pkg/psmdb/client_test.go | Adds unit tests validating URI/SRVURI generation across DNS modes and mongos exposure modes. |
| pkg/naming/service.go | Adds service naming helper functions for replsets and mongos services. |
| pkg/naming/secret.go | Adds naming functions for new connection-string Secrets. |
| pkg/controller/perconaservermongodb/users.go | Ensures database admin connection string Secret is created during user reconciliation. |
| pkg/controller/perconaservermongodb/status.go | Updates mongos address lookup to pass service-per-pod mode. |
| pkg/controller/perconaservermongodb/service.go | Uses naming helpers for mongos service creation/removal. |
| pkg/controller/perconaservermongodb/secrets.go | Refactors credential lookup; adds ensureConnectionStringSecret() helper for conn-str Secrets. |
| pkg/controller/perconaservermongodb/secrets_test.go | Adds unit tests for ensureConnectionStringSecret() and reconcileUsers conn-str behavior. |
| pkg/controller/perconaservermongodb/custom_users.go | Creates per-custom-user connection string Secret (non-external users). |
| pkg/apis/psmdb/v1/psmdb_types.go | Adds role→secret-key mapping helpers for system user creds. |
| e2e-tests/custom-users-roles-sharded/run | Extends E2E to verify generated connection strings authenticate successfully. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
pooknull
requested review from
egegunes,
eleo007,
gkech,
hors,
mayankshah1607,
nmarukovich,
oksana-grishchenko and
valmiranogueira
as code owners
Comment on lines
+99
to
+106
| if rs.Expose.Enabled { | ||
| cfg, err := psmdb.MongoConfig(ctx, cl, cr, rs, cred, true) | ||
| if err != nil { | ||
| return errors.Wrap(err, "mongo config") | ||
| } | ||
| if exposedConnStr := cfg.URI(); exposedConnStr != connStr { | ||
| connStrSecret.Data[key+"_connectionStringExposed"] = []byte(exposedConnStr) | ||
| } |
egegunes
previously approved these changes
Jun 15, 2026
| } | ||
|
|
||
| _, err := controllerutil.CreateOrUpdate(ctx, cl, connStrSecret, func() error { | ||
| connStrSecret.Data = make(map[string][]byte) |
Member
There was a problem hiding this comment.
If we're resetting the data here, doesn't it mean that in case of custom users, only the last one in the list is present in the secret, and all others before it get wiped out? Did you test it? I think the e2e test does not assert anything about the prev users in the Secret no?
nmarukovich
reviewed
Jun 16, 2026
|
|
||
| func SecretCustomUserConnStrName(cr *api.PerconaServerMongoDB, user *api.User) string { | ||
| return user.SecretName(cr) + "-conn-str" | ||
| } |
Contributor
There was a problem hiding this comment.
if we have several custom users and don't use for them passwordSecretRef
in my-cluster-name-custom-user-secret-conn-str we keep only last user.
users:
- name: alice
db: admin
roles:
- name: clusterAdmin
db: admin
- name: userAdminAnyDatabase
db: admin
- name: bob
db: admin
roles:
- name: read
db: admin
kubectl get secret my-cluster-name-custom-user-secret-conn-str \
-o jsonpath='{.data}' | python3 -c \
"import sys,json; [print(k) for k in json.load(sys.stdin).keys()]"
bob_mongos_connectionString
Contributor
|
I think that @nmarukovich and @mayankshah1607 comments underline the same issue because If we reset Data, only the last user is kept |
Comment on lines
+261
to
263
| func GetMongosAddrs(ctx context.Context, cl client.Client, cr *api.PerconaServerMongoDB, useInternalAddr bool, servicePerPod bool) ([]string, error) { | ||
| if !servicePerPod { | ||
| host, err := MongosHost(ctx, cl, cr, nil, useInternalAddr) |
Comment on lines
+184
to
+188
| mongosCfg, err := psmdb.MongosConfig(ctx, cl, cr, cred, true, servicePerPod) | ||
| if err != nil { | ||
| return errors.Wrap(err, "mongos config") | ||
| } | ||
| data[keyPrefix+"_mongos_connectionString"] = []byte(mongosCfg.URI()) |
Comment on lines
+38
to
46
| func getCredentials(secret *corev1.Secret, role api.SystemUserRole) (psmdb.Credentials, error) { | ||
| creds := psmdb.Credentials{} | ||
| usersSecret, err := getUserSecret(ctx, cl, cr, name) | ||
| if err != nil { | ||
| return creds, errors.Wrap(err, "failed to get user secret") | ||
| } | ||
|
|
||
| switch role { | ||
| case api.RoleDatabaseAdmin: | ||
| creds.Username = string(usersSecret.Data[api.EnvMongoDBDatabaseAdminUser]) | ||
| creds.Password = string(usersSecret.Data[api.EnvMongoDBDatabaseAdminPassword]) | ||
| case api.RoleClusterAdmin: | ||
| creds.Username = string(usersSecret.Data[api.EnvMongoDBClusterAdminUser]) | ||
| creds.Password = string(usersSecret.Data[api.EnvMongoDBClusterAdminPassword]) | ||
| case api.RoleUserAdmin: | ||
| creds.Username = string(usersSecret.Data[api.EnvMongoDBUserAdminUser]) | ||
| creds.Password = string(usersSecret.Data[api.EnvMongoDBUserAdminPassword]) | ||
| case api.RoleClusterMonitor: | ||
| creds.Username = string(usersSecret.Data[api.EnvMongoDBClusterMonitorUser]) | ||
| creds.Password = string(usersSecret.Data[api.EnvMongoDBClusterMonitorPassword]) | ||
| case api.RoleBackup: | ||
| creds.Username = string(usersSecret.Data[api.EnvMongoDBBackupUser]) | ||
| creds.Password = string(usersSecret.Data[api.EnvMongoDBBackupPassword]) | ||
| case api.RoleSearch: | ||
| creds.Username = string(usersSecret.Data[api.EnvMongoDBSearchUser]) | ||
| creds.Password = string(usersSecret.Data[api.EnvMongoDBSearchPassword]) | ||
| default: | ||
| return creds, errors.Errorf("not implemented for role: %s", role) | ||
| envKeyUser, envKeyPass := role.EnvKeyUsername(), role.EnvKeyPassword() | ||
| if envKeyUser == "" || envKeyPass == "" { | ||
| return creds, errors.Errorf("invalid role %s", string(role)) | ||
| } | ||
| creds.Username = string(secret.Data[envKeyUser]) | ||
| creds.Password = string(secret.Data[envKeyPass]) | ||
|
|
Comment on lines
+342
to
348
| svcName := naming.MongosServiceName(cr) | ||
| if cr.Spec.Sharding.Mongos.Expose.ServicePerPod { | ||
| if pod == nil { | ||
| return "", errors.New("mongos pod is required for service-per-pod exposure") | ||
| } | ||
| svcName = pod.Name | ||
| } |
gkech
previously approved these changes
Jun 22, 2026
Comment on lines
+154
to
+159
| if includeReplsets { | ||
| for _, rs := range cr.GetAllReplsets() { | ||
| cfg, err := psmdb.MongoConfig(ctx, cl, cr, cr.Spec.ClusterServiceDNSMode, rs, cred, false) | ||
| if err != nil { | ||
| return errors.Wrap(err, "mongo config") | ||
| } |
Collaborator
commit: b574baa |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
https://perconadev.atlassian.net/browse/K8SPSMDB-1537
CHANGE DESCRIPTION
This PR creates the following secrets:
<cluster>-databaseadmin-conn-strfor thedatabaseAdminuser<custom-user-secret-name>-conn-strfor each non-external custom user<cluster>-databaseadmin-conn-strcan contain:<user>_<replset>_connectionString:mongodb://URI using internal replica-set addresses.<user>_<replset>_connectionStringSrv:mongodb+srv://URI using the replica-set service hostname.<user>_<replset>_connectionStringExposed: created when the replica set is exposed and its exposed URI differs from its internal URI.<user>_mongos_connectionString: created for sharded clusters using internal mongos addresses.<user>_mongos_connectionStringExposed: created for sharded clusters when mongos servicePerPod is enabled.<replset>keys are generated for every replica set, including thecfgreplset<custom-user-secret-name>-conn-strcan contain:For non-sharded clusters:
<user>_<replset>_connectionString<user>_<replset>_connectionStringSrv<user>_<replset>_connectionStringExposed, when applicableFor sharded clusters:
<user>_mongos_connectionString<user>_mongos_connectionStringExposed, when mongos servicePerPod is enabledCHECKLIST
Jira
Needs Doc) and QA (Needs QA)?Tests
compare/*-oc.yml)?Config/Logging/Testability