Skip to content
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.

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

mysql_user: prevent password getting set for existing users on on_cre… #342

Conversation

hubiongithub
Copy link
Contributor

…ate when plugin is used

SUMMARY

if update_password is set to on_create the code did not update "password" but update "plugin,plugin_auth_string,plugin_hash_string"

From my point of view "on_create" should never update a password related value.

This change has the side effect that a change of the plugin (lets say caching_sha2_password -> auth_pam (which requires no plugin_auth_string at all) will not happen.

Fixes #334

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

mysql_user

ADDITIONAL INFORMATION

@hubiongithub
Copy link
Contributor Author

I canvnot view the result from this third-party-check status: failure
I could not get the tests working here behind an outgoing proxy as apt runs inside (the ansible-test -docker) containers and apt does not accept HTTP_PROXY environment and I would have to set these proxy parameter inside /etc/apt/apt-conf.d/.00_proxy.conf in all containers which ansible-test generates and running apt add key ... in. And I don't know how to do this.

@codecov
Copy link

codecov bot commented May 6, 2022

Codecov Report

Merging #342 (f7cc26d) into main (c489cf1) will decrease coverage by 0.13%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #342      +/-   ##
==========================================
- Coverage   78.23%   78.10%   -0.14%     
==========================================
  Files          27       27              
  Lines        2270     2279       +9     
  Branches      551      552       +1     
==========================================
+ Hits         1776     1780       +4     
- Misses        335      340       +5     
  Partials      159      159              
Impacted Files Coverage Δ
plugins/modules/mysql_user.py 81.44% <ø> (ø)
plugins/modules/mysql_role.py 85.66% <0.00%> (-1.72%) ⬇️
plugins/modules/mysql_query.py 88.17% <0.00%> (+1.58%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c489cf1...f7cc26d. Read the comment docs.

@Andersson007
Copy link
Collaborator

@hubiongithub thanks for the PR!
I'll try to take a look at the changes and your comment at the beginning of next week.

Could you please also add a changelog gragment?

Comment on lines 2 to 4
- "mysql_user - fix logicwhen update_password is set to on_create for users using plugin* arguments (https://github.com/ansible-collections/community.mysql/issues/334)."


Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
- "mysql_user - fix logicwhen update_password is set to on_create for users using plugin* arguments (https://github.com/ansible-collections/community.mysql/issues/334)."
- "mysql_user - fix logic when ``update_password`` is set to ``on_create`` for users using ``plugin*`` arguments (https://github.com/ansible-collections/community.mysql/issues/334)."

@Andersson007
Copy link
Collaborator

@hubiongithub sorry for the late reply, busy days.., could you please rebase the PR and fix the changelog fragment? About rebasing, here's the guide. As it's for ansible core, it uses the devel branch. Use main instead. Thanks!

@betanummeric @rsicart @Jorge-Rodriguez do you have experience in using these arguments? I'd be great to hear opinions of folks who use this feature

@betanummeric
Copy link
Member

betanummeric commented May 25, 2022

I have not used the plugin, plugin_auth_string, plugin_hash_string arguments yet, but I might start soon, for which I would need this PR too. Regardless of my potential usecase, this PR looks good to me. Some integration tests could be added.

@hubiongithub hubiongithub force-pushed the issue334_fix_logic_on_oncreate branch from f004806 to bcbb620 Compare May 25, 2022 07:37
@hubiongithub
Copy link
Contributor Author

Rebase of PR to main: done
Format fixing of changelog fragment: done and pushed

@Andersson007
Copy link
Collaborator

Andersson007 commented May 25, 2022

@hubiongithub Could you also please clarify the documentation for corresponding arguments according to the change? (i would also suggest elaborating a bit on details in the changelog fragment's entry)

…_auth_string options, format fix on changelog
Copy link
Collaborator

@Andersson007 Andersson007 left a comment

Choose a reason for hiding this comment

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

you can just click commit suggestion

plugins/modules/mysql_user.py Outdated Show resolved Hide resolved
plugins/modules/mysql_user.py Outdated Show resolved Hide resolved
plugins/modules/mysql_user.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@Andersson007 Andersson007 left a comment

Choose a reason for hiding this comment

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

another linting issue

plugins/modules/mysql_user.py Outdated Show resolved Hide resolved
@Andersson007 Andersson007 merged commit 51a3884 into ansible-collections:main May 31, 2022
@Andersson007
Copy link
Collaborator

@hubiongithub thanks for the contribution! The community will be happy to see more from you!
@betanummeric thanks for reviewing!

@Andersson007
Copy link
Collaborator

@hubiongithub there are conflicts which need to be solved, so the automatic backporting is not possible. Would you like to backport your changes manually to stable-2 and stable-1 branches? See this guide.
If you have no time, please let me know, i'll do it myself, np.

@ansible-collections ansible-collections deleted a comment from patchback bot May 31, 2022
@ansible-collections ansible-collections deleted a comment from patchback bot May 31, 2022
@hubiongithub
Copy link
Contributor Author

I followed the steps from the patchback instructions above, I hope I didn't missed anything.

@hubiongithub hubiongithub deleted the issue334_fix_logic_on_oncreate branch May 31, 2022 08:29
@Andersson007
Copy link
Collaborator

@hubiongithub i replied in #379 (comment)

@Andersson007
Copy link
Collaborator

@hubiongithub what i usually do #379 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

mysql.user tries to set password on second run with update_password = "on_create"
3 participants