mysql_user, mysql_role: add argument subtract_privs to revoke privileges explicitly (#333)

* add option subtract_privs to mysql_role and mysql_user

see https://github.com/ansible-collections/community.mysql/issues/331

* add integration tests for subtract_privs for mysql_role and mysql_user

* add changelog fragment for PR #333

* mysql_role, mysql_user: when subtract_privileges, don't grant unwanted privileges and don't revoke USAGE implicitly

* fix integration tests

* mysql_role, mysql_user: invalid privileges are ignored when subtract_privs is true -> document that and fix integration tests

* fix mysql_role integration tests

* fix mysql_role, mysql_user integration tests

* formatting

make the PEP8 check happy

* mysql_user and mysql_role: fix granting privileges when only the GRANT OPTION needs to be added

* mysql_user and mysql_role: log some updated privileges; explain integration test blind spot

* mysql_user and mysql_role: don't grant too much privileges

If only the grant option needs to be granted, at least one privilege needs to be granted to get valid syntax. USAGE is better for that than the existing privileges, because unwanted privileges would be re-added after revokation.

* mysql_user and mysql_role: fix type error

* Update changelogs/fragments/333-mysql_user-mysql_role-add-subtract_privileges-argument.yml

Co-authored-by: Andrew Klychkov <aaklychkov@mail.ru>

* Update plugins/modules/mysql_role.py

Co-authored-by: Andrew Klychkov <aaklychkov@mail.ru>

* Update plugins/modules/mysql_user.py

Co-authored-by: Andrew Klychkov <aaklychkov@mail.ru>

Co-authored-by: Felix Hamme <felix.hamme@ionos.com>
Co-authored-by: Andrew Klychkov <aaklychkov@mail.ru>

tests/integration/targets/test_mysql_role/defaults/main.yml

@@ -14,3 +14,4 @@ nonexistent: user3
 
 role0: role0
 role1: role1
+role2: role2

tests/integration/targets/test_mysql_role/tasks/main.yml

@@ -3,5 +3,15 @@
 # and should not be used as examples of how to write Ansible roles #
 ####################################################################
 
+- name: alias mysql command to include default options
+  set_fact:
+    mysql_command: "mysql -u{{ mysql_user }} -p{{ mysql_password }} -P{{ mysql_primary_port }} --protocol=tcp"
+
+
 # mysql_role module initial CI tests
 - import_tasks: mysql_role_initial.yml
+
+# Test that subtract_privs will only revoke the grants given by priv
+# (https://github.com/ansible-collections/community.mysql/issues/331)
+- include: test_priv_subtract.yml enable_check_mode=no
+- include: test_priv_subtract.yml enable_check_mode=yes

tests/integration/targets/test_mysql_role/tasks/test_priv_subtract.yml

@@ -0,0 +1,168 @@
+# Test code to ensure that subtracting privileges will not result in unnecessary changes.
+- vars:
+    mysql_parameters: &mysql_params
+      login_user: '{{ mysql_user }}'
+      login_password: '{{ mysql_password }}'
+      login_host: 127.0.0.1
+      login_port: '{{ mysql_primary_port }}'
+
+  block:
+
+    - name: Create test databases
+      mysql_db:
+        <<: *mysql_params
+        name: '{{ item }}'
+        state: present
+      loop:
+        - data1
+
+    - name: Create a role with an initial set of privileges
+      mysql_role:
+        <<: *mysql_params
+        name: '{{ role2 }}'
+        priv: 'data1.*:SELECT,INSERT'
+        state: present
+
+    - name: Run command to show privileges for role (expect privileges in stdout)
+      command: "{{ mysql_command }} -e \"SHOW GRANTS FOR '{{ role2 }}'\""
+      register: result
+
+    - name: Assert that the initial set of privileges matches what is expected
+      assert:
+        that:
+          - "'GRANT SELECT, INSERT ON `data1`.*' in result.stdout"
+
+    - name: Subtract privileges that are not in the current privileges, which should be a no-op
+      mysql_role:
+        <<: *mysql_params
+        name: '{{ role2 }}'
+        priv: 'data1.*:DELETE'
+        subtract_privs: yes
+        state: present
+      check_mode: '{{ enable_check_mode }}'
+      register: result
+
+    - name: Assert that there wasn't a change in permissions
+      assert:
+        that:
+          - "result.changed == false"
+
+    - name: Run command to show privileges for role (expect privileges in stdout)
+      command: "{{ mysql_command }} -e \"SHOW GRANTS FOR '{{ role2 }}'\""
+      register: result
+
+    - name: Assert that the permissions still match what was originally granted
+      assert:
+        that:
+          - "'GRANT SELECT, INSERT ON `data1`.*' in result.stdout"
+
+    - name: Subtract existing and not-existing privileges, but not all
+      mysql_role:
+        <<: *mysql_params
+        name: '{{ role2 }}'
+        priv: 'data1.*:INSERT,DELETE'
+        subtract_privs: yes
+        state: present
+      check_mode: '{{ enable_check_mode }}'
+      register: result
+
+    - name: Assert that there was a change because permissions were/would be revoked on data1.*
+      assert:
+        that:
+          - "result.changed == true"
+
+    - name: Run command to show privileges for role (expect privileges in stdout)
+      command: "{{ mysql_command }} -e \"SHOW GRANTS FOR '{{ role2 }}'\""
+      register: result
+
+    - name: Assert that the permissions were not changed if check_mode is set to 'yes'
+      assert:
+        that:
+          - "'GRANT SELECT, INSERT ON `data1`.*' in result.stdout"
+      when: enable_check_mode == 'yes'
+
+    - name: Assert that only DELETE was revoked if check_mode is set to 'no'
+      assert:
+        that:
+          - "'GRANT SELECT ON `data1`.*' in result.stdout"
+      when: enable_check_mode == 'no'
+
+    - name: Try to subtract invalid privileges
+      mysql_role:
+        <<: *mysql_params
+        name: '{{ role2 }}'
+        priv: 'data1.*:INVALID'
+        subtract_privs: yes
+        state: present
+      check_mode: '{{ enable_check_mode }}'
+      register: result
+
+    - name: Assert that there was no change because invalid permissions are ignored
+      assert:
+        that:
+          - "result.changed == false"
+
+    - name: Run command to show privileges for role (expect privileges in stdout)
+      command: "{{ mysql_command }} -e \"SHOW GRANTS FOR '{{ role2 }}'\""
+      register: result
+
+    - name: Assert that the permissions were not changed with check_mode=='yes'
+      assert:
+        that:
+          - "'GRANT SELECT, INSERT ON `data1`.*' in result.stdout"
+      when: enable_check_mode == 'yes'
+
+    - name: Assert that the permissions were not changed with check_mode=='no'
+      assert:
+        that:
+          - "'GRANT SELECT ON `data1`.*' in result.stdout"
+      when: enable_check_mode == 'no'
+
+    - name: trigger failure by trying to subtract and append privileges at the same time
+      mysql_role:
+        <<: *mysql_params
+        name: '{{ role2 }}'
+        priv: 'data1.*:SELECT'
+        subtract_privs: yes
+        append_privs: yes
+        state: present
+      check_mode: '{{ enable_check_mode }}'
+      register: result
+      ignore_errors: true
+
+    - name: Assert the previous execution failed
+      assert:
+        that:
+          - result is failed
+
+    - name: Run command to show privileges for role (expect privileges in stdout)
+      command: "{{ mysql_command }} -e \"SHOW GRANTS FOR '{{ role2 }}'\""
+      register: result
+
+    - name: Assert that the permissions stayed the same, with check_mode=='yes'
+      assert:
+        that:
+          - "'GRANT SELECT, INSERT ON `data1`.*' in result.stdout"
+      when: enable_check_mode == 'yes'
+
+    - name: Assert that the permissions stayed the same, with check_mode=='no'
+      assert:
+        that:
+          - "'GRANT SELECT ON `data1`.*' in result.stdout"
+      when: enable_check_mode == 'no'
+
+    ##########
+    # Clean up
+    - name: Drop test databases
+      mysql_db:
+        <<: *mysql_params
+        name: '{{ item }}'
+        state: present
+      loop:
+        - data1
+
+    - name: Drop test role
+      mysql_role:
+        <<: *mysql_params
+        name: '{{ role2 }}'
+        state: absent

tests/integration/targets/test_mysql_user/tasks/main.yml

@@ -274,6 +274,11 @@
     - include: test_priv_append.yml enable_check_mode=no
     - include: test_priv_append.yml enable_check_mode=yes
 
+    # Test that subtract_privs will only revoke the grants given by priv
+    # (https://github.com/ansible-collections/community.mysql/issues/331)
+    - include: test_priv_subtract.yml enable_check_mode=no
+    - include: test_priv_subtract.yml enable_check_mode=yes
+
     # Tests for the TLS requires dictionary
     - include: tls_requirements.yml
 

tests/integration/targets/test_mysql_user/tasks/test_priv_subtract.yml

@@ -0,0 +1,173 @@
+# Test code to ensure that subtracting privileges will not result in unnecessary changes.
+- vars:
+    mysql_parameters: &mysql_params
+      login_user: '{{ mysql_user }}'
+      login_password: '{{ mysql_password }}'
+      login_host: 127.0.0.1
+      login_port: '{{ mysql_primary_port }}'
+
+  block:
+
+    - name: Create test databases
+      mysql_db:
+        <<: *mysql_params
+        name: '{{ item }}'
+        state: present
+      loop:
+      - data1
+
+    - name: Create a user with an initial set of privileges
+      mysql_user:
+        <<: *mysql_params
+        name: '{{ user_name_4 }}'
+        password: '{{ user_password_4 }}'
+        priv: 'data1.*:SELECT,INSERT'
+        state: present
+
+    - name: Run command to show privileges for user (expect privileges in stdout)
+      command: "{{ mysql_command }} -e \"SHOW GRANTS FOR '{{ user_name_4 }}'@'localhost'\""
+      register: result
+
+    - name: Assert that the initial set of privileges matches what is expected
+      assert:
+        that:
+          - "'GRANT SELECT, INSERT ON `data1`.*' in result.stdout"
+
+    - name: Subtract privileges that are not in the current privileges, which should be a no-op
+      mysql_user:
+        <<: *mysql_params
+        name: '{{ user_name_4 }}'
+        password: '{{ user_password_4 }}'
+        priv: 'data1.*:DELETE'
+        subtract_privs: yes
+        state: present
+      check_mode: '{{ enable_check_mode }}'
+      register: result
+
+    - name: Assert that there wasn't a change in permissions
+      assert:
+        that:
+          - "result.changed == false"
+
+    - name: Run command to show privileges for user (expect privileges in stdout)
+      command: "{{ mysql_command }} -e \"SHOW GRANTS FOR '{{ user_name_4 }}'@'localhost'\""
+      register: result
+
+    - name: Assert that the permissions still match what was originally granted
+      assert:
+        that:
+          - "'GRANT SELECT, INSERT ON `data1`.*' in result.stdout"
+
+    - name: Subtract existing and not-existing privileges, but not all
+      mysql_user:
+        <<: *mysql_params
+        name: '{{ user_name_4 }}'
+        password: '{{ user_password_4 }}'
+        priv: 'data1.*:INSERT,DELETE'
+        subtract_privs: yes
+        state: present
+      check_mode: '{{ enable_check_mode }}'
+      register: result
+
+    - name: Assert that there was a change because permissions were/would be revoked on data1.*
+      assert:
+        that:
+          - "result.changed == true"
+
+    - name: Run command to show privileges for user (expect privileges in stdout)
+      command: "{{ mysql_command }} -e \"SHOW GRANTS FOR '{{ user_name_4 }}'@'localhost'\""
+      register: result
+
+    - name: Assert that the permissions were not changed if check_mode is set to 'yes'
+      assert:
+        that:
+          - "'GRANT SELECT, INSERT ON `data1`.*' in result.stdout"
+      when: enable_check_mode == 'yes'
+
+    - name: Assert that only DELETE was revoked if check_mode is set to 'no'
+      assert:
+        that:
+          - "'GRANT SELECT ON `data1`.*' in result.stdout"
+      when: enable_check_mode == 'no'
+
+    - name: Try to subtract invalid privileges
+      mysql_user:
+        <<: *mysql_params
+        name: '{{ user_name_4 }}'
+        password: '{{ user_password_4 }}'
+        priv: 'data1.*:INVALID'
+        subtract_privs: yes
+        state: present
+      check_mode: '{{ enable_check_mode }}'
+      register: result
+
+    - name: Assert that there was no change because invalid permissions are ignored
+      assert:
+        that:
+          - "result.changed == false"
+
+    - name: Run command to show privileges for user (expect privileges in stdout)
+      command: "{{ mysql_command }} -e \"SHOW GRANTS FOR '{{ user_name_4 }}'@'localhost'\""
+      register: result
+
+    - name: Assert that the permissions were not changed with check_mode=='yes'
+      assert:
+        that:
+          - "'GRANT SELECT, INSERT ON `data1`.*' in result.stdout"
+      when: enable_check_mode == 'yes'
+
+    - name: Assert that the permissions were not changed with check_mode=='no'
+      assert:
+        that:
+          - "'GRANT SELECT ON `data1`.*' in result.stdout"
+      when: enable_check_mode == 'no'
+
+    - name: trigger failure by trying to subtract and append privileges at the same time
+      mysql_user:
+        <<: *mysql_params
+        name: '{{ user_name_4 }}'
+        password: '{{ user_password_4 }}'
+        priv: 'data1.*:SELECT'
+        subtract_privs: yes
+        append_privs: yes
+        state: present
+      check_mode: '{{ enable_check_mode }}'
+      register: result
+      ignore_errors: true
+
+    - name: Assert the previous execution failed
+      assert:
+        that:
+          - result is failed
+
+    - name: Run command to show privileges for user (expect privileges in stdout)
+      command: "{{ mysql_command }} -e \"SHOW GRANTS FOR '{{ user_name_4 }}'@'localhost'\""
+      register: result
+
+    - name: Assert that the permissions stayed the same, with check_mode=='yes'
+      assert:
+        that:
+          - "'GRANT SELECT, INSERT ON `data1`.*' in result.stdout"
+      when: enable_check_mode == 'yes'
+
+    - name: Assert that the permissions stayed the same, with check_mode=='no'
+      assert:
+        that:
+          - "'GRANT SELECT ON `data1`.*' in result.stdout"
+      when: enable_check_mode == 'no'
+
+    ##########
+    # Clean up
+    - name: Drop test databases
+      mysql_db:
+        <<: *mysql_params
+        name: '{{ item }}'
+        state: present
+      loop:
+      - data1
+
+    - name: Drop test user
+      mysql_user:
+        <<: *mysql_params
+        name: '{{ user_name_4 }}'
+        state: absent

tests/integration/targets/test_mysql_user/tasks/test_privs.yml

@@ -173,7 +173,7 @@
         state: present
       register: result
 
-    # FIXME: on mariadb 10.5 there's always a change
+    # FIXME: on mariadb >=10.5.2 there's always a change because the REPLICATION CLIENT privilege was renamed to BINLOG MONITOR
     - name: Assert that priv did not change
       assert:
         that: