password_expire support for mysql_user

[tompal3]

SUMMARY

Add new feature to support password expire policy for users as discussed in: #513

ISSUE TYPE

• Feature Pull Request

COMPONENT NAME

• mysql_user

ADDITIONAL INFORMATION

new features will include setting password_lifetime policy for creating or altering user:

• expire password in custom number of days.
• expire password using global variable default_password_lifetime.
• expire password now.
• never expire password.

[laurent-indermuehle]

Hi @tompal3 . Great start! Thanks for your contribution.

When I started to contribute to c.mysql I didn't knew how to find the error messages from the failing tests. So I paste it here in case you didn't saw it already:

TASK [test_mysql_role : Create role0 in check_mode again] **********************
fatal: [testhost]: FAILED! => {"changed": false, "msg": "user_mod() missing 2 required positional arguments: 'password_expire' and 'password_expire_interval'"}

To find it, I searched for "FAILED" and hit enter until I saw an error without the light blue text "...ignoring"

Do you know why this is failing?

[tompal3]

Thanks for the tip @laurent-indermuehle. Yes I have already fixed it, but now I'm trying to setup ansible integration tests on my wsl ubuntu instance to reduce number of commits and to write some tests for my PR.

[laurent-indermuehle]

In that case you'll need to adapt the Makefile that I wrote for podman. I think docker will work best in your case. Feel free to ask questions here or in matrix #mysql:ansible.com if you need help.

[codecov]

Codecov Report

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

Comparison is base (32718ca) 77.04% compared to head (1c25ab5) 75.53%.
Report is 1 commits behind head on main.

Files	Patch %	Lines
plugins/module_utils/user.py	87.80%	2 Missing and 3 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #598      +/-   ##
==========================================
- Coverage   77.04%   75.53%   -1.52%     
==========================================
  Files          28       18      -10     
  Lines        2501     2420      -81     
  Branches      618      610       -8     
==========================================
- Hits         1927     1828      -99     
- Misses        388      407      +19     
+ Partials      186      185       -1     

Flag	Coverage Δ
integration	74.79% <88.23%> (+0.28%)	⬆️
sanity	?
units	?

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

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

[laurent-indermuehle]

@tompal3 as you mention on Matrix, our scheduled integration tests for the plugins fails with latest Ansible due to jinja templating been forbidden in the assertions. I'm fixing this now. You'll be able to rebase your fork on this to avoid clutter in your patch when I'm done. Thanks for your message.

[laurent-indermuehle]

I'm quite rusty on the Python part. Also I may have introduced some regression so I apologise in advance if that happen.

As a rule of thumb, if an assertion has only one test, I prefer to use failed_when. This offer a cleaner output (searching for a failed task is easier when you don't have thousand of "failed... ignored" in the log.

[tompal3]

I'm quite rusty on the Python part. Also I may have introduced some regression so I apologise in advance if that happen.

As a rule of thumb, if an assertion has only one test, I prefer to use failed_when. This offer a cleaner output (searching for a failed task is easier when you don't have thousand of "failed... ignored" in the log.

Thanks for the code review, committed all your suggestions.

[Andersson007]

@tompal3 hi, thanks for the great improvement!
Could you please add a changelog fragment to let users know about the added feature?
Any questions? Just ask
Thanks!

[Andersson007]

Suggested change

	password_expire=dict(type='str', choices=['now', 'never', 'default', 'interval'], no_log=True),
	password_expire_interval=dict(type='int', no_log=True),
	password_expire=dict(type='str', choices=['now', 'never', 'default', 'interval'], no_log=True),
	password_expire_interval=dict(type='int', no_log=True),

why do we no_log them?

[tompal3]

no reason why we should, will remove that one.

[Andersson007]

hello, any updates on this?

[tompal3]

hello, any updates on this?

Hi, sorry missed your previous comment, will check it this week.

[Andersson007]

@tompal3 could you also please rebase the PR, fix the conflicts, and force-push?

[Andersson007]

@tompal3 there's a sanity failure:

ERROR: Found 1 pep8 issue(s) which need to be resolved:
ERROR: plugins/module_utils/user.py:1071:1: E302: expected 2 blank lines, found 1 (50%)

needs to be fixed before merging

[tompal3]

@tompal3 there's a sanity failure:

ERROR: Found 1 pep8 issue(s) which need to be resolved:
ERROR: plugins/module_utils/user.py:1071:1: E302: expected 2 blank lines, found 1 (50%)

needs to be fixed before merging

@Andersson007 ok, I have remembered now, sanity checks fails if variable containing password key word is not set to no_log=True. So I will leave that one there.

[laurent-indermuehle]

Only way I found to get rid of an old request for changes I did was to validate the whole PR. Thought I'm too busy now to do a full review, so please ignore my vote on this one (I skimmed, it looks not bad at all thought). You can ping me if necessary.

[Andersson007]

@tompal3 thanks for the contributing!
@laurent-indermuehle thanks for the review!

@tompal3 FYI we have a forum group and a matrix channel, see the communication guide for details.