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

feat[mysql_info]: add 'users_info' filter #580

Conversation

laurent-indermuehle
Copy link
Collaborator

@laurent-indermuehle laurent-indermuehle commented Oct 12, 2023

SUMMARY

Add the filter users_info to the mysql_info module.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

mysql_info

ADDITIONAL INFORMATION

This filter returns information about users accounts. Useful when migrating accounts to a new server or to help exporting accounts in YAML that can be placed in an Ansible inventory.
To migrate accounts, feed the output of mysql_info directly to mysql_user.
To export the accounts definition, use a playbook that writes the output of mysql_info into a file or just ansible.builtin.debug if you have very few accounts.

It doesn't return information about password_option and lock_option but since the mysql_user module doesn't support them, I didn't implemented them.

Doesn't support sha256_password and caching_sha2_password authentication plugins. Even if you use print_identified_with_as_hex to convert the password into a hash. I think this is because PyMySQL and
mysqlclient wrap the password in quote: CREATE USER... IDENTIFIED AS '0x1234' instead of CREATE USER... IDENTIFIED AS 0x1234. Sorry to leave that unfinished but this issue is too much work for one who only works on MariaDB with mysql_native_password.

@codecov
Copy link

codecov bot commented Oct 12, 2023

Codecov Report

Attention: 14 lines in your changes are missing coverage. Please review.

Comparison is base (033b4c7) 76.35% compared to head (77b0831) 74.77%.
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #580      +/-   ##
==========================================
- Coverage   76.35%   74.77%   -1.59%     
==========================================
  Files          28       18      -10     
  Lines        2394     2319      -75     
  Branches      584      583       -1     
==========================================
- Hits         1828     1734      -94     
- Misses        390      405      +15     
- Partials      176      180       +4     
Flag Coverage Δ
integration 74.16% <73.33%> (+0.33%) ⬆️
sanity ?
units ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
plugins/module_utils/user.py 85.56% <69.56%> (+0.27%) ⬆️
plugins/modules/mysql_info.py 79.91% <81.08%> (-1.28%) ⬇️

... and 18 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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.

@laurent-indermuehle thanks a lot for the PR!

Causes issues when authentications plugins C(sha256_password) and C(caching_sha2_password).

What kind of issues? Will the module not fail? Or you mean issues for migration?
Anyway, if i have such questions, I think it's worth elaborating a bit more on the issues in the doc;)

UPDATE: Also how about renaming the key to user_accounts?

plugins/modules/mysql_info.py Outdated Show resolved Hide resolved
plugins/modules/mysql_info.py Outdated Show resolved Hide resolved
@laurent-indermuehle
Copy link
Collaborator Author

@laurent-indermuehle thanks a lot for the PR!

Causes issues when authentications plugins C(sha256_password) and C(caching_sha2_password).

What kind of issues? Will the module not fail? Or you mean issues for migration? Anyway, if i have such questions, I think it's worth elaborating a bit more on the issues in the doc;)

UPDATE: Also how about renaming the key to user_accounts?

You're right, I was vague. I've pushed an explanation along with your suggestion of using a list in the description instead of a long sentence.

user_accounts is a far better name than users_privs (user_ or users_ thought?) I'll need to rename everything. I'll do this soon.

@laurent-indermuehle laurent-indermuehle changed the title feat[mysql_info]: add 'users_privs' filter feat[mysql_info]: add 'user_accounts' filter Oct 12, 2023
@Andersson007
Copy link
Collaborator

Andersson007 commented Oct 13, 2023

user_accounts is a far better name than users_privs (user_ or users_ thought?) I'll need to rename everything. I'll do this soon.

@laurent-indermuehle how about users if it's not occupied already? If it's occupied users_accounts i think
UPDATE: or maybe users_info if users is busy?

@laurent-indermuehle
Copy link
Collaborator Author

laurent-indermuehle commented Oct 13, 2023

@laurent-indermuehle how about users if it's not occupied already? If it's occupied users_accounts i think UPDATE: or > maybe users_info if users is busy?

Users is already used.
I see you prefer the plural. I do too now I see it. Ok then, I re-re-name to users_info?
users_information would be too long. And users_accounts is kind of synonymous.

@Andersson007
Copy link
Collaborator

users_info SGTM

@laurent-indermuehle laurent-indermuehle changed the title feat[mysql_info]: add 'user_accounts' filter feat[mysql_info]: add 'users_info' filter Oct 13, 2023
@laurent-indermuehle
Copy link
Collaborator Author

I added a description for the old users filter. Otherwise it was weird:

users:
  description: Users informations
users_info:
  description: Information about users accounts...

If someone knows a use case for this filter, we could add it to the description.

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.

SGTM

@laurent-indermuehle laurent-indermuehle merged commit 3ef9bda into ansible-collections:main Oct 23, 2023
40 of 41 checks passed
@laurent-indermuehle laurent-indermuehle deleted the lie_mysql_info_users_privs branch October 23, 2023 09:27
@Andersson007
Copy link
Collaborator

@laurent-indermuehle 🎉 thanks!

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.

None yet

2 participants