K8SPSMDB-1236 Check for custom users name uniqueness #1813
Conversation
gkech
requested review from
hors,
egegunes,
pooknull and
nmarukovich
as code owners
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| api "github.com/percona/percona-server-mongodb-operator/pkg/apis/psmdb/v1" | ||
| "github.com/percona/percona-server-mongodb-operator/pkg/psmdb/mongo" |
There was a problem hiding this comment.
[goimports-reviser] reported by reviewdog 🐶
| api "github.com/percona/percona-server-mongodb-operator/pkg/apis/psmdb/v1" | |
| "github.com/percona/percona-server-mongodb-operator/pkg/psmdb/mongo" |
| "github.com/percona/percona-server-mongodb-operator/pkg/psmdb/mongo" | ||
| "github.com/pkg/errors" | ||
| "github.com/stretchr/testify/assert" | ||
| ) |
There was a problem hiding this comment.
[goimports-reviser] reported by reviewdog 🐶
| ) | |
| api "github.com/percona/percona-server-mongodb-operator/pkg/apis/psmdb/v1" | |
| "github.com/percona/percona-server-mongodb-operator/pkg/psmdb/mongo" | |
| ) |
| log := logf.FromContext(ctx) | ||
|
|
||
| if sysUserNames == nil || uniqueUserNames == nil { | ||
| log.Error(nil, "sys or unique usernames are nil") |
There was a problem hiding this comment.
do we need these logs for errors? don't the error we return will be logged as reconciler error?
There was a problem hiding this comment.
The error logs that we see here are only added because they existed also in the existing implementation. More specifically, we used to do something like this
if _, ok := sysUserNames[user.Name]; ok {
log.Error(nil, "creating user with reserved user name is forbidden", "user", user.Name)
continue
}
but now we do this:
err := validateUser(ctx, &user, systemUserNames, uniqueUserNames)
if err != nil {
continue
}
So the answer is that the errors returned are not logged.
We can either keep the implementation as is, or we can add more details to the errors retuned and log a single time when we handle the validateUser error. Maybe the latter is more clean.
There was a problem hiding this comment.
Running this with errors, looks like this with the PR's implementation:
commit: 12fe253 |
CHANGE DESCRIPTION
Problem:
With this PR, we are improving the user creation flow and we validate on creation time that the users created will be unique in terms of name.
Cause:
Short explanation of the root cause of the issue if applicable.
Solution:
Short explanation of the solution we are providing with this PR.
CHECKLIST
Jira
Needs Doc) and QA (Needs QA)?Tests
compare/*-oc.yml)?Config/Logging/Testability