mysql_user, mysql_role: add option to revoke privileges explicitly

[betanummeric]

SUMMARY

When updating a role, its privileges are either added or replaced, depending on the append_privs argument. I would like to add an argument to revoke privileges explicitly.

USECASE

I read the desired privileges from a dynamic source. To ensure no privileges are revoked in case the dynamic source accidentally returns incomplete information, I need to revoke privileges explicitly.

ISSUE TYPE

• Feature Idea

COMPONENT NAME

modules mysql_role, mysql_user

IMPLEMENTATION IDEAS

I propose 3 alternative ways to implement that:

1) add argument subtract_privs

Add a new boolean argument subtract_privs (default no, mutual conflict with append_privs). If enabled, the module would revoke the privileges specified by priv (without granting any privileges).

2) add argument revoke_privs

Add a new argument revoke_privs which takes the same format as privs. If set, all privileges in revoke_privs that are not in privs will be revoked.
privs and append_privs continue to work as before.

3) add argument privilege_state, deprecate option append_privs

Add a new argument privilege_state with three possible values:

• exact (default, like previous behavior with append_privs: no)
• granted (like previous behaviour with append_privs: yes)
• revoked (revoke all privileges specified by priv)

This options should mutually conflict with append_privs. privilege_state: granted should be used instead of append_privs: yes.

What implementation idea would you prefer?

[Andersson007]

@betanummeric great issue, thanks for bringing it up and for the options!

I personally like number 1 (number 3 sounds also interesting but as it'll introduce a breaking change - which would be painful especially for mysql_user's users because the module is old and widely used, i would prefer number 1).

So +1 to option 1

@betanummeric which option do you prefer?

@rsicart @bmalynovytch @Jorge-Rodriguez your preferences?

[betanummeric]

@Andersson007

My favorite is option 2, because revoke_privs and privs could be used together to revoke and grant privileges explicitly in one step.

I think option 3 would be the most intuitive interface, but after a deprecation phase this would be a breaking change, which I understand you want to avoid.

When we decide on the options, I can submit a PR.

[rsicart]

Hi, I go with option 1 too, seems the simplest to implement and maintain.

[Jorge-Rodriguez]

While the completeness feature of option 2 outlined by @betanummeric is quite interesting, I think the syntax is a bit clunky and the semantics confusing (what happens when a privilege appears in both parameters? And so on).

I like option 1 because it maintains the existing syntax and semantics of append_privs.

In a counter argument to option 2 for setting and revoking privileges in one step, that should be the behaviour of the privs parameter when both append_privs and substract_privs are false.

If this counter argument is true in our implementation, it would have the same meaning as option 3 in exact mode and we would be covering all methods of operation with option 1, so I think we should also verify that privs with append and substract set to false does define the actual state of privileges, no more, no less.

Unfortunately, dynamic privileges have been historically breaking our efforts towards idempotence.

[betanummeric]

@Jorge-Rodriguez All options allow to grant and revoke privileges in one step (by keeping the current behavior with append_privs: no available). Option 2 is the only option where you could keep not-specified privileges while granting and revoking specified privileges in one step.
With option 2, privileges appearing in both parameters should result in an error or a documented precedence.

All options declare a desired state, so I see no issue with idempotence.

Nevertheless, option 1 looks like a winner.

[Andersson007]

Nevertheless, option 1 looks like a winner.

Yep, thanks everyone for sharing your opinions!
Let's proceed with option 1 then.
@betanummeric are you OK with raising a PR?
If yes, remember to add:

• a changelog fragment
• to cover the changes with integration tests (including with check_mode: yes). Running integration tests locally before pushing can save time. See https://docs.ansible.com/ansible/devel/community/collection_contributors/collection_integration_tests.html for details.

If any questions/you need advice or whatever, just ask.
In case you have no time to complete this, please let us know.

Thank you!