community.mysql
community.mysql copied to clipboard
mysql_user: Adding statements output
SUMMARY
Adding a statements list to the output of mysql_user could make the module more transparent and easier to debug. Ideally, users would be able to run in check mode and see exactly which statements would be executed to get to the desired state.
ISSUE TYPE
- Feature Idea
COMPONENT NAME
mysql_user
ADDITIONAL INFORMATION
The mysql_user module currently returns changed, user, and msg. This proposal would add statements to include the SQL statements associated with the actual user or permission changes. An example of the proposed output may be seen below:
"statements": [
"CREATE USER 'db_user2'@'localhost' IDENTIFIED WITH mysql_native_password AS '[redacted]'",
"GRANT EXECUTE ON FUNCTION `foo`.`function` TO 'db_user2'@'localhost'",
"GRANT SELECT ON `foo`.* TO 'db_user2'@'localhost'",
"GRANT EXECUTE ON PROCEDURE `bar`.`procedure` TO 'db_user2'@'localhost'",
"GRANT USAGE ON *.* TO 'db_user2'@'localhost'"
],
An initial version (not ready for PR) can be seen here: https://github.com/steveteahan/community.mysql/commit/245f5f77a57b236fc926ad533a2a8fc4b7a26a0f
Providing the output of the SQL statements is something that the postgresql collection is doing today:
- https://github.com/ansible-collections/community.postgresql/blob/main/plugins/modules/postgresql_privs.py#L1168
- https://github.com/ansible-collections/community.postgresql/blob/main/plugins/modules/postgresql_user.py#L988
In that collection, they call it queries which I think is acceptable is well, it just seemed the statements was possibly more accurate.
Looking for some initial feedback about whether the maintainers are open to this change, or if it was already considered and avoided. Some other general thoughts about this:
- Most of the functions in the module return early in check_mode, before the query is ever built, so additional refactoring would be required to make this work in check mode.
- Check mode is arguably where this functionality would be most useful, so I'll take a look at the feasibility of moving the check mode conditionals around
cursor.execute, but it's possible it wouldn't be that easy.
@steveteahan thanks for the feature idea!
I'd suggest to name it queries because now it's implemented in mysql_variables module.
As far as i remember, we can use cursor.mogrify() to get final statements without real execution.
I took a quick look at the pre-pr.
Are tls related changes really necessary there?
If not, better to put them in a separate PR and merge before of after the PR about logging.
Also, would be cool to see the implementation as simple as possible (maybe your approach is, i don't know now, just a general recommendation).
Thank you, @Andersson007. To respond to your feedback:
- queries works for me
- Agreed on
cursor.mogrify(), that's what I ended up using only wrapping it in a method to avoidmogrify()andappend()calls everywhere - I'm happy to handle some of the refactoring in separate PRs to keep this change as straight-forward as possible
I'll finish up my changes, get the docs and tests in order, and then submit a PR.
@steveteahan cool, thanks:) Don't forget about a changelog fragment (minor_changes:) and integration tests