K8SPSMDB-1164: Allow creating user with $external database #1690
Conversation
inelpandzic
requested review from
tplavcic,
nmarukovich,
ptankov,
jvpasinatto,
eleo007,
hors,
egegunes and
pooknull
as code owners
inelpandzic
changed the title
There was a problem hiding this comment.
@inelpandzic please add test case for https://github.com/percona/percona-server-mongodb-operator/blob/main/e2e-tests/ldap-tls/run test as well. We need to be sure that it works
@hors I was thinking about this but now I'm positive we don't need to do it since it will not provide any value. The way that you enable external authentication ability is simply by creating a user with If we need to add this test as well to make sure it works, then we would need to cover Kerberos and other external auth providers. |
| if err != nil && name != defaultName { | ||
| return nil, errors.Wrap(err, "failed to get user secret") | ||
| } |
There was a problem hiding this comment.
Not sure what do you mean by "user just wants to customize its name", but how this behaves is like this:
- If in the spec the user sets
passwordSecretRef.namewe will look for that secret. If we don't find it we will fail creating that user. - If the user does not set
passwordSecretRef.name, we will create secret{cluster-name}-custom-user-secret, generate a password for the user and set it by the key named after user name.
And yes, I'll add this to the spec as well.
|
@inelpandzic looks like now you need to fix role order in e2e test 😄 |
Yeah, I know... :) |
inelpandzic
dismissed stale reviews from nmarukovich, egegunes, and pooknull
via
9a99799
pull-request-size
bot
added
size/XL
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| echo "$cmd" | ||
| } | ||
|
|
||
|
|
There was a problem hiding this comment.
[shfmt] reported by reviewdog 🐶
|
|
||
| desc 'check role recreated after deleted from DB' | ||
| run_mongos \ | ||
| 'use admin\n db.dropRole("role-one")' \ | ||
| "$mongosUri" | ||
| sleep 15 | ||
| compare 'admin' 'db.getRole("role-one", {showPrivileges: true, showAuthenticationRestrictions: true})' \ | ||
| "$mongosUri" "role-one" | ||
| compare 'admin' "$(get_role_cmd \"role-one\" )" "$mongosUri" "role-one" |
There was a problem hiding this comment.
[shfmt] reported by reviewdog 🐶
| compare 'admin' "$(get_role_cmd \"role-one\" )" "$mongosUri" "role-one" | |
| compare 'admin' "$(get_role_cmd \"role-one\")" "$mongosUri" "role-one" |
| compare 'admin' "$(get_role_cmd \"role-one\" )" "$mongosUri" "role-one" | ||
| compare 'admin' "$(get_role_cmd \"role-two\" )" "$mongosUri" "role-two" |
There was a problem hiding this comment.
[shfmt] reported by reviewdog 🐶
| compare 'admin' "$(get_role_cmd \"role-one\" )" "$mongosUri" "role-one" | |
| compare 'admin' "$(get_role_cmd \"role-two\" )" "$mongosUri" "role-two" | |
| compare 'admin' "$(get_role_cmd \"role-one\")" "$mongosUri" "role-one" | |
| compare 'admin' "$(get_role_cmd \"role-two\")" "$mongosUri" "role-two" |
| @@ -282,16 +324,14 @@ kubectl_bin patch psmdb ${cluster} --type=merge --patch '{ | |||
| }}' | |||
| wait_for_running $cluster-rs0 3 | |||
|
|
|||
| compare 'admin' 'db.getRole("role-two", {showPrivileges: true, showAuthenticationRestrictions: true})' \ | |||
| "$mongosUri" "role-two-updated" | |||
| compare 'admin' "$(get_role_cmd \"role-two\" )" "$mongosUri" "role-two-updated" | |||
There was a problem hiding this comment.
[shfmt] reported by reviewdog 🐶
| compare 'admin' "$(get_role_cmd \"role-two\" )" "$mongosUri" "role-two-updated" | |
| compare 'admin' "$(get_role_cmd \"role-two\")" "$mongosUri" "role-two-updated" |
|
|
||
| desc 'check role update from DB' | ||
| run_mongos \ | ||
| 'use admin\n db.updateRole( "role-two",{privileges:[{resource: {db:"config", collection:"" }, actions: ["find", "update"]}]})' \ | ||
| "$mongosUri" | ||
| sleep 15 | ||
| compare 'admin' 'db.getRole("role-two", {showPrivileges: true, showAuthenticationRestrictions: true})' \ | ||
| "$mongosUri" "role-two-updated" | ||
| compare 'admin' "$(get_role_cmd \"role-two\" )" "$mongosUri" "role-two-updated" |
There was a problem hiding this comment.
[shfmt] reported by reviewdog 🐶
| compare 'admin' "$(get_role_cmd \"role-two\" )" "$mongosUri" "role-two-updated" | |
| compare 'admin' "$(get_role_cmd \"role-two\")" "$mongosUri" "role-two-updated" |
| @@ -315,8 +355,7 @@ kubectl_bin patch psmdb ${cluster} --type=merge --patch '{ | |||
| }}' | |||
| wait_for_running $cluster-rs0 3 | |||
|
|
|||
| compare 'admin' 'db.getRole("role-three", {showPrivileges: true, showAuthenticationRestrictions: true})' \ | |||
| "$mongosUri" "role-three" | |||
| compare 'admin' "$(get_role_cmd \"role-three\" )" "$mongosUri" "role-three" | |||
There was a problem hiding this comment.
[shfmt] reported by reviewdog 🐶
| compare 'admin' "$(get_role_cmd \"role-three\" )" "$mongosUri" "role-three" | |
| compare 'admin' "$(get_role_cmd \"role-three\")" "$mongosUri" "role-three" |
| compare 'testAdmin1' "$(get_role_cmd \"role-four\" )" "$mongosUri" "role-four" | ||
| compare 'testAdmin2' "$(get_role_cmd \"role-five\" )" "$mongosUri" "role-five" |
There was a problem hiding this comment.
[shfmt] reported by reviewdog 🐶
| compare 'testAdmin1' "$(get_role_cmd \"role-four\" )" "$mongosUri" "role-four" | |
| compare 'testAdmin2' "$(get_role_cmd \"role-five\" )" "$mongosUri" "role-five" | |
| compare 'testAdmin1' "$(get_role_cmd \"role-four\")" "$mongosUri" "role-four" | |
| compare 'testAdmin2' "$(get_role_cmd \"role-five\")" "$mongosUri" "role-five" |
| @@ -18,7 +18,44 @@ compare() { | |||
| | sed '/"userId"/d' \ | |||
| >$tmp_dir/${target} | |||
|
|
|||
| diff ${test_dir}/compare/${target}.json $tmp_dir/${target} | |||
| diff ${test_dir}/compare/${target}.json $tmp_dir/${target} | |||
There was a problem hiding this comment.
[shfmt] reported by reviewdog 🐶
| diff ${test_dir}/compare/${target}.json $tmp_dir/${target} | |
| diff ${test_dir}/compare/${target}.json $tmp_dir/${target} |
| echo "$cmd" | ||
| } | ||
|
|
||
|
|
There was a problem hiding this comment.
[shfmt] reported by reviewdog 🐶
commit: 6394cce |
CHANGE DESCRIPTION
Problem:
There was no ability to add a user to
$externaldatabase because settinguser.PasswordSecretRefwas mandatory and for a user with$externaldatabase we don't provide user credentials, since they are handled by the external provider.Solution:
Add a support to create user with
$externaldatabase.Note:
This PR also adds support for generating user pass/secret if it is not set. Task https://perconadev.atlassian.net/browse/K8SPSMDB-1171
Also covers task: https://perconadev.atlassian.net/browse/K8SPSMDB-1162
CHECKLIST
Jira
Needs Doc) and QA (Needs QA)?Tests
compare/*-oc.yml)?Config/Logging/Testability