Get rid of privs comparison (#243)

* Remove all code related to VALID_PRIVS and get_valid_privs()

* Add tests to update user with invalid privs

* Re-raise InvalidPrivsError when granting privileges

* Fix: compatibility with python2

* More explicit assertions as commented by Andersson007

* Add changelog fragment

changelogs/fragments/243-get-rid-of-privs-comparison.yml

@@ -0,0 +1,2 @@
+breaking_changes:
+  - mysql_user - validate privileges using database engine directly (https://github.com/ansible-collections/community.mysql/issues/234 https://github.com/ansible-collections/community.mysql/pull/243). Do not validate privileges in this module anymore.

plugins/module_utils/user.py

@@ -19,49 +19,6 @@ from ansible_collections.community.mysql.plugins.module_utils.mysql import (
 )
 
 
-EXTRA_PRIVS = ['ALL', 'ALL PRIVILEGES', 'GRANT', 'REQUIRESSL']
-
-# This list is kept for backwards compatibility after release 2.3.0,
-# see https://github.com/ansible-collections/community.mysql/issues/232 for details
-VALID_PRIVS = [
-    'CREATE', 'DROP', 'GRANT', 'GRANT OPTION',
-    'LOCK TABLES', 'REFERENCES', 'EVENT', 'ALTER',
-    'DELETE', 'INDEX', 'INSERT', 'SELECT', 'UPDATE',
-    'CREATE TEMPORARY TABLES', 'TRIGGER', 'CREATE VIEW',
-    'SHOW VIEW', 'ALTER ROUTINE', 'CREATE ROUTINE',
-    'EXECUTE', 'FILE', 'CREATE TABLESPACE', 'CREATE USER',
-    'PROCESS', 'PROXY', 'RELOAD', 'REPLICATION CLIENT',
-    'REPLICATION SLAVE', 'SHOW DATABASES', 'SHUTDOWN',
-    'SUPER', 'ALL', 'ALL PRIVILEGES', 'USAGE',
-    'REQUIRESSL',  # Deprecated, to be removed in version 3.0.0
-    'CREATE ROLE', 'DROP ROLE', 'APPLICATION_PASSWORD_ADMIN',
-    'AUDIT_ADMIN', 'BACKUP_ADMIN', 'BINLOG_ADMIN',
-    'BINLOG_ENCRYPTION_ADMIN', 'CLONE_ADMIN', 'CONNECTION_ADMIN',
-    'ENCRYPTION_KEY_ADMIN', 'FIREWALL_ADMIN', 'FIREWALL_USER',
-    'GROUP_REPLICATION_ADMIN', 'INNODB_REDO_LOG_ARCHIVE',
-    'NDB_STORED_USER', 'PERSIST_RO_VARIABLES_ADMIN',
-    'REPLICATION_APPLIER', 'REPLICATION_SLAVE_ADMIN',
-    'RESOURCE_GROUP_ADMIN', 'RESOURCE_GROUP_USER',
-    'ROLE_ADMIN', 'SESSION_VARIABLES_ADMIN', 'SET_USER_ID',
-    'SYSTEM_USER', 'SYSTEM_VARIABLES_ADMIN', 'SYSTEM_USER',
-    'TABLE_ENCRYPTION_ADMIN', 'VERSION_TOKEN_ADMIN',
-    'XA_RECOVER_ADMIN', 'LOAD FROM S3', 'SELECT INTO S3',
-    'INVOKE LAMBDA',
-    'ALTER ROUTINE',
-    'BINLOG ADMIN',
-    'BINLOG MONITOR',
-    'BINLOG REPLAY',
-    'CONNECTION ADMIN',
-    'READ_ONLY ADMIN',
-    'REPLICATION MASTER ADMIN',
-    'REPLICATION SLAVE ADMIN',
-    'SET USER',
-    'SHOW_ROUTINE',
-    'SLAVE MONITOR',
-    'REPLICA MONITOR',
-]
-
-
 class InvalidPrivsError(Exception):
     pass
 
@@ -147,14 +104,6 @@ def get_tls_requires(cursor, user, host):
         return requires or None
 
 
-def get_valid_privs(cursor):
-    cursor.execute("SHOW PRIVILEGES")
-    show_privs = [priv[0].upper() for priv in cursor.fetchall()]
-    # See the comment above VALID_PRIVS declaration
-    all_privs = show_privs + EXTRA_PRIVS + VALID_PRIVS
-    return frozenset(all_privs)
-
-
 def get_grants(cursor, user, host):
     cursor.execute("SHOW GRANTS FOR %s@%s", (user, host))
     grants_line = list(filter(lambda x: "ON *.*" in x[0], cursor.fetchall()))[0]
@@ -597,7 +546,7 @@ def sort_column_order(statement):
     return '%s(%s)' % (priv_name, ', '.join(columns))
 
 
-def privileges_unpack(priv, mode, valid_privs):
+def privileges_unpack(priv, mode):
     """ Take a privileges string, typically passed as a parameter, and unserialize
     it into a dictionary, the same format as privileges_get() above. We have this
     custom format to avoid using YAML/JSON strings inside YAML playbooks. Example
@@ -643,10 +592,6 @@ def privileges_unpack(priv, mode, valid_privs):
         # Handle cases when there's privs like GRANT SELECT (colA, ...) in privs.
         output[pieces[0]] = normalize_col_grants(output[pieces[0]])
 
-        new_privs = frozenset(privs)
-        if not new_privs.issubset(valid_privs):
-            raise InvalidPrivsError('Invalid privileges specified: %s' % new_privs.difference(valid_privs))
-
     if '*.*' not in output:
         output['*.*'] = ['USAGE']
 
@@ -699,7 +644,10 @@ def privileges_grant(cursor, user, host, db_table, priv, tls_requires, maria_rol
     if 'GRANT' in priv:
         query.append("WITH GRANT OPTION")
     query = ' '.join(query)
+    try:
         cursor.execute(query, params)
+    except (mysql_driver.ProgrammingError, mysql_driver.OperationalError, mysql_driver.InternalError) as e:
+        raise InvalidPrivsError("Error granting privileges, invalid priv string: %s" % priv_string)
 
 
 def convert_priv_dict_to_str(priv):

plugins/modules/mysql_role.py

@@ -250,7 +250,6 @@ from ansible_collections.community.mysql.plugins.module_utils.user import (
     get_mode,
     user_mod,
     privileges_grant,
-    get_valid_privs,
     privileges_unpack,
 )
 from ansible.module_utils._text import to_native
@@ -1014,8 +1013,7 @@ def main():
             module.fail_json(msg=to_native(e))
 
         try:
-            valid_privs = get_valid_privs(cursor)
-            priv = privileges_unpack(priv, mode, valid_privs)
+            priv = privileges_unpack(priv, mode)
         except Exception as e:
             module.fail_json(msg='Invalid privileges string: %s' % to_native(e))
 

plugins/modules/mysql_user.py

@@ -318,7 +318,6 @@ from ansible_collections.community.mysql.plugins.module_utils.user import (
     handle_requiressl_in_priv_string,
     InvalidPrivsError,
     limit_resources,
-    get_valid_privs,
     privileges_unpack,
     sanitize_requires,
     user_add,
@@ -421,11 +420,7 @@ def main():
             mode = get_mode(cursor)
         except Exception as e:
             module.fail_json(msg=to_native(e))
-        try:
-            valid_privs = get_valid_privs(cursor)
-            priv = privileges_unpack(priv, mode, valid_privs)
-        except Exception as e:
-            module.fail_json(msg="invalid privileges string: %s" % to_native(e))
+        priv = privileges_unpack(priv, mode)
 
     if state == "present":
         if user_exists(cursor, user, host, host_all):

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

@@ -96,6 +96,25 @@
           - "'GRANT SELECT, DELETE ON `data2`.*' in result.stdout"
       when: enable_check_mode == 'yes'
 
+    - name: Try to append invalid privileges
+      mysql_user:
+        <<: *mysql_params
+        name: '{{ user_name_4 }}'
+        password: '{{ user_password_4 }}'
+        priv: 'data1.*:INVALID/data2.*:SELECT'
+        append_privs: yes
+        state: present
+      check_mode: '{{ enable_check_mode }}'
+      register: result
+      ignore_errors: true
+
+    - name: Assert that there wasn't a change in privileges if check_mode is set to 'no'
+      assert:
+        that:
+          - result is failed
+          - "'Error granting privileges' in result.msg"
+      when: enable_check_mode == 'no'
+
     ##########
     # Clean up
     - name: Drop test databases

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

@@ -178,6 +178,23 @@
         that:
           - "result.changed == false"
 
+    # ============================================================
+    - name: update user with invalid privileges
+      mysql_user:
+        <<: *mysql_params
+        name: '{{ user_name_2 }}'
+        password: '{{ user_password_2 }}'
+        priv: '*.*:INVALID'
+        state: present
+      register: result
+      ignore_errors: yes
+
+    - name: Assert that priv did not change
+      assert:
+        that:
+          - result is failed
+          - "'Error granting privileges' in result.msg"
+
     - name: remove username
       mysql_user:
         <<: *mysql_params