Handle divergences between MySQL and MariaDB (#103)

* Initial attempt

* First functional approach

* Remove unused imports

* Add dychotomy handling for mysql_replication

* Fix cursor lookup

* Fix sanity tests

* Cleanup implementation conditional import

* Fix unit tests

* Fix conditional import to satisfy both sanity and integration tests

* Add changelog fragment

changelogs/fragments/103-mysql_and_mariadb_divergence.yml

@@ -0,0 +1,2 @@
+minor_changes:
+- mysql_collection - introduce codebabse split to handle divergences between MySQL and MariaDB (https://github.com/ansible-collections/community.mysql/pull/103).

plugins/module_utils/implementations/mariadb/replication.py

@@ -0,0 +1,10 @@
+from __future__ import (absolute_import, division, print_function)
+__metaclass__ = type
+
+from ansible_collections.community.mysql.plugins.module_utils.mysql import get_server_version
+from distutils.version import LooseVersion
+
+
+def uses_replica_terminology(cursor):
+    """Checks if REPLICA must be used instead of SLAVE"""
+    return LooseVersion(get_server_version(cursor)) >= LooseVersion('10.5.1')

plugins/module_utils/implementations/mariadb/user.py

@@ -0,0 +1,15 @@
+from __future__ import (absolute_import, division, print_function)
+__metaclass__ = type
+
+from distutils.version import LooseVersion
+from ansible_collections.community.mysql.plugins.module_utils.mysql import get_server_version
+
+
+def use_old_user_mgmt(cursor):
+    version = get_server_version(cursor)
+
+    return LooseVersion(version) < LooseVersion("10.2")
+
+
+def supports_identified_by_password(cursor):
+    return True

plugins/module_utils/implementations/mysql/replication.py

@@ -0,0 +1,10 @@
+from __future__ import (absolute_import, division, print_function)
+__metaclass__ = type
+
+from ansible_collections.community.mysql.plugins.module_utils.mysql import get_server_version
+from distutils.version import LooseVersion
+
+
+def uses_replica_terminology(cursor):
+    """Checks if REPLICA must be used instead of SLAVE"""
+    return LooseVersion(get_server_version(cursor)) >= LooseVersion('8.0.22')

plugins/module_utils/implementations/mysql/user.py

@@ -0,0 +1,16 @@
+from __future__ import (absolute_import, division, print_function)
+__metaclass__ = type
+
+from distutils.version import LooseVersion
+from ansible_collections.community.mysql.plugins.module_utils.mysql import get_server_version
+
+
+def use_old_user_mgmt(cursor):
+    version = get_server_version(cursor)
+
+    return LooseVersion(version) < LooseVersion("5.7")
+
+
+def supports_identified_by_password(cursor):
+    version = get_server_version(cursor)
+    return LooseVersion(version) < LooseVersion("8")

plugins/modules/mysql_replication.py

@@ -277,24 +277,6 @@ from distutils.version import LooseVersion
 executed_queries = []
 
 
-def uses_replica_terminology(cursor):
-    """Checks if REPLICA must be used instead of SLAVE"""
-    cursor.execute("SELECT VERSION() AS version")
-    result = cursor.fetchone()
-
-    if isinstance(result, dict):
-        version_str = result['version']
-    else:
-        version_str = result[0]
-
-    version = LooseVersion(version_str)
-
-    if 'mariadb' in version_str.lower():
-        return version >= LooseVersion('10.5.1')
-    else:
-        return version >= LooseVersion('8.0.22')
-
-
 def get_master_status(cursor):
     cursor.execute("SHOW MASTER STATUS")
     masterstatus = cursor.fetchone()
@@ -520,9 +502,15 @@ def main():
         else:
             module.fail_json(msg="unable to find %s. Exception message: %s" % (config_file, to_native(e)))
 
+    cursor.execute("SELECT VERSION()")
+    if 'mariadb' in cursor.fetchone()["VERSION()"].lower():
+        from ansible_collections.community.mysql.plugins.module_utils.implementations.mariadb import replication as impl
+    else:
+        from ansible_collections.community.mysql.plugins.module_utils.implementations.mysql import replication as impl
+
     # Since MySQL 8.0.22 and MariaDB 10.5.1,
     # "REPLICA" must be used instead of "SLAVE"
-    if uses_replica_terminology(cursor):
+    if impl.uses_replica_terminology(cursor):
         replica_term = 'REPLICA'
         if master_use_gtid == 'slave_pos':
             module.deprecate('master_use_gtid "slave_pos" value is deprecated, use "replica_pos" instead.',

plugins/modules/mysql_user.py

@@ -298,12 +298,11 @@ RETURN = '''#'''
 
 import re
 import string
-from distutils.version import LooseVersion
 
 from ansible.module_utils.basic import AnsibleModule
 from ansible_collections.community.mysql.plugins.module_utils.database import SQLParseError
 from ansible_collections.community.mysql.plugins.module_utils.mysql import (
-    mysql_connect, mysql_driver, mysql_driver_fail_msg, mysql_common_argument_spec, get_server_version
+    mysql_connect, mysql_driver, mysql_driver_fail_msg, mysql_common_argument_spec
 )
 from ansible.module_utils.six import iteritems
 from ansible.module_utils._text import to_native
@@ -353,40 +352,6 @@ class InvalidPrivsError(Exception):
 #
 
 
-# User Authentication Management changed in MySQL 5.7 and MariaDB 10.2.0
-def use_old_user_mgmt(cursor):
-    cursor.execute("SELECT VERSION()")
-    result = cursor.fetchone()
-    version_str = result[0]
-    version = version_str.split('.')
-
-    if 'mariadb' in version_str.lower():
-        # Prior to MariaDB 10.2
-        if int(version[0]) * 1000 + int(version[1]) < 10002:
-            return True
-        else:
-            return False
-    else:
-        # Prior to MySQL 5.7
-        if int(version[0]) * 1000 + int(version[1]) < 5007:
-            return True
-        else:
-            return False
-
-
-def supports_identified_by_password(cursor):
-    """
-    Determines whether the 'CREATE USER %s@%s IDENTIFIED BY PASSWORD %s' syntax is supported. This was dropped in
-    MySQL 8.0.
-    """
-    version_str = get_server_version(cursor)
-
-    if 'mariadb' in version_str.lower():
-        return True
-    else:
-        return LooseVersion(version_str) < LooseVersion('8')
-
-
 def get_mode(cursor):
     cursor.execute('SELECT @@GLOBAL.sql_mode')
     result = cursor.fetchone()
@@ -445,7 +410,7 @@ def do_not_mogrify_requires(query, params, tls_requires):
 
 def get_tls_requires(cursor, user, host):
     if user:
-        if not use_old_user_mgmt(cursor):
+        if not impl.use_old_user_mgmt(cursor):
             query = "SHOW CREATE USER '%s'@'%s'" % (user, host)
         else:
             query = "SHOW GRANTS for '%s'@'%s'" % (user, host)
@@ -487,12 +452,12 @@ def user_add(cursor, user, host, host_all, password, encrypted,
         return True
 
     # Determine what user management method server uses
-    old_user_mgmt = use_old_user_mgmt(cursor)
+    old_user_mgmt = impl.use_old_user_mgmt(cursor)
 
     mogrify = do_not_mogrify_requires if old_user_mgmt else mogrify_requires
 
     if password and encrypted:
-        if supports_identified_by_password(cursor):
+        if impl.supports_identified_by_password(cursor):
             query_with_args = "CREATE USER %s@%s IDENTIFIED BY PASSWORD %s", (user, host, password)
         else:
             query_with_args = "CREATE USER %s@%s IDENTIFIED WITH mysql_native_password AS %s", (user, host, password)
@@ -539,7 +504,7 @@ def user_mod(cursor, user, host, host_all, password, encrypted,
     grant_option = False
 
     # Determine what user management method server uses
-    old_user_mgmt = use_old_user_mgmt(cursor)
+    old_user_mgmt = impl.use_old_user_mgmt(cursor)
 
     if host_all:
         hostnames = user_get_hostnames(cursor, user)
@@ -985,7 +950,7 @@ def privileges_grant(cursor, user, host, db_table, priv, tls_requires):
     query = ["GRANT %s ON %s" % (priv_string, db_table)]
     query.append("TO %s@%s")
     params = (user, host)
-    if tls_requires and use_old_user_mgmt(cursor):
+    if tls_requires and impl.use_old_user_mgmt(cursor):
         query, params = mogrify_requires(" ".join(query), params, tls_requires)
         query = [query]
     if 'REQUIRESSL' in priv and not tls_requires:
@@ -1224,6 +1189,15 @@ def main():
     if not sql_log_bin:
         cursor.execute("SET SQL_LOG_BIN=0;")
 
+    global impl
+    cursor.execute("SELECT VERSION()")
+    if 'mariadb' in cursor.fetchone()[0].lower():
+        from ansible_collections.community.mysql.plugins.module_utils.implementations.mariadb import user as mysqluser
+        impl = mysqluser
+    else:
+        from ansible_collections.community.mysql.plugins.module_utils.implementations.mysql import user as mariauser
+        impl = mariauser
+
     if priv is not None:
         try:
             mode = get_mode(cursor)

tests/unit/plugins/module_utils/test_mariadb_replication.py

@@ -0,0 +1,24 @@
+# -*- coding: utf-8 -*-
+# Copyright: (c) 2020, Andrew Klychkov (@Andersson007) <aaklychkov@mail.ru>
+
+from __future__ import (absolute_import, division, print_function)
+__metaclass__ = type
+
+import pytest
+
+from ansible_collections.community.mysql.plugins.module_utils.implementations.mariadb.replication import uses_replica_terminology
+from ..utils import dummy_cursor_class
+
+
+@pytest.mark.parametrize(
+    'f_output,c_output,c_ret_type',
+    [
+        (False, '10.5.0-mariadb', 'dict'),
+        (True, '10.5.1-mariadb', 'dict'),
+        (True, '10.6.0-mariadb', 'dict'),
+        (True, '11.5.1-mariadb', 'dict'),
+    ]
+)
+def test_uses_replica_terminology(f_output, c_output, c_ret_type):
+    cursor = dummy_cursor_class(c_output, c_ret_type)
+    assert uses_replica_terminology(cursor) == f_output

tests/unit/plugins/module_utils/test_mariadb_user_implementation.py

@@ -0,0 +1,29 @@
+# -*- coding: utf-8 -*-
+
+from __future__ import (absolute_import, division, print_function)
+__metaclass__ = type
+
+import pytest
+
+from ansible_collections.community.mysql.plugins.module_utils.implementations.mariadb.user import (
+    supports_identified_by_password,
+)
+from ..utils import dummy_cursor_class
+
+
+@pytest.mark.parametrize(
+    'function_return,cursor_output,cursor_ret_type',
+    [
+        (True, '10.5.0-mariadb', 'dict'),
+        (True, '10.5.1-mariadb', 'dict'),
+        (True, '10.6.0-mariadb', 'dict'),
+        (True, '11.5.1-mariadb', 'dict'),
+    ]
+)
+def test_supports_identified_by_password(function_return, cursor_output, cursor_ret_type):
+    """
+    Tests whether 'CREATE USER %s@%s IDENTIFIED BY PASSWORD %s' is supported,
+    which is currently supported by everything besides MySQL >= 8.0.
+    """
+    cursor = dummy_cursor_class(cursor_output, cursor_ret_type)
+    assert supports_identified_by_password(cursor) == function_return

tests/unit/plugins/module_utils/test_mysql_replication.py

@@ -6,7 +6,7 @@ __metaclass__ = type
 
 import pytest
 
-from ansible_collections.community.mysql.plugins.modules.mysql_replication import uses_replica_terminology
+from ansible_collections.community.mysql.plugins.module_utils.implementations.mysql.replication import uses_replica_terminology
 from ..utils import dummy_cursor_class
 
 
@@ -18,13 +18,9 @@ from ..utils import dummy_cursor_class
         (False, '8.0.0-mysql', 'list'),
         (False, '8.0.11-mysql', 'dict'),
         (False, '8.0.21-mysql', 'list'),
-        (False, '10.5.0-mariadb', 'dict'),
         (True, '8.0.22-mysql', 'list'),
         (True, '8.1.2-mysql', 'dict'),
         (True, '9.0.0-mysql', 'list'),
-        (True, '10.5.1-mariadb', 'dict'),
-        (True, '10.6.0-mariadb', 'dict'),
-        (True, '11.5.1-mariadb', 'dict'),
     ]
 )
 def test_uses_replica_terminology(f_output, c_output, c_ret_type):

tests/unit/plugins/module_utils/test_mysql_user_implementation.py

@@ -0,0 +1,33 @@
+# -*- coding: utf-8 -*-
+
+from __future__ import (absolute_import, division, print_function)
+__metaclass__ = type
+
+import pytest
+
+from ansible_collections.community.mysql.plugins.module_utils.implementations.mysql.user import (
+    supports_identified_by_password,
+)
+from ..utils import dummy_cursor_class
+
+
+@pytest.mark.parametrize(
+    'function_return,cursor_output,cursor_ret_type',
+    [
+        (True, '5.5.1-mysql', 'list'),
+        (True, '5.7.0-mysql', 'dict'),
+        (False, '8.0.22-mysql', 'list'),
+        (False, '8.1.2-mysql', 'dict'),
+        (False, '9.0.0-mysql', 'list'),
+        (False, '8.0.0-mysql', 'list'),
+        (False, '8.0.11-mysql', 'dict'),
+        (False, '8.0.21-mysql', 'list'),
+    ]
+)
+def test_supports_identified_by_password(function_return, cursor_output, cursor_ret_type):
+    """
+    Tests whether 'CREATE USER %s@%s IDENTIFIED BY PASSWORD %s' is supported,
+    which is currently supported by everything besides MySQL >= 8.0.
+    """
+    cursor = dummy_cursor_class(cursor_output, cursor_ret_type)
+    assert supports_identified_by_password(cursor) == function_return

tests/unit/plugins/modules/test_mysql_user.py

@@ -10,37 +10,10 @@ from ansible_collections.community.mysql.plugins.modules.mysql_user import (
     has_grant_on_col,
     normalize_col_grants,
     sort_column_order,
-    supports_identified_by_password,
 )
 from ..utils import dummy_cursor_class
 
 
-@pytest.mark.parametrize(
-    'function_return,cursor_output,cursor_ret_type',
-    [
-        (True, '5.5.1-mysql', 'list'),
-        (True, '5.7.0-mysql', 'dict'),
-        (True, '10.5.0-mariadb', 'dict'),
-        (True, '10.5.1-mariadb', 'dict'),
-        (True, '10.6.0-mariadb', 'dict'),
-        (True, '11.5.1-mariadb', 'dict'),
-        (False, '8.0.22-mysql', 'list'),
-        (False, '8.1.2-mysql', 'dict'),
-        (False, '9.0.0-mysql', 'list'),
-        (False, '8.0.0-mysql', 'list'),
-        (False, '8.0.11-mysql', 'dict'),
-        (False, '8.0.21-mysql', 'list'),
-    ]
-)
-def test_supports_identified_by_password(function_return, cursor_output, cursor_ret_type):
-    """
-    Tests whether 'CREATE USER %s@%s IDENTIFIED BY PASSWORD %s' is supported,
-    which is currently supported by everything besides MySQL >= 8.0.
-    """
-    cursor = dummy_cursor_class(cursor_output, cursor_ret_type)
-    assert supports_identified_by_password(cursor) == function_return
-
-
 @pytest.mark.parametrize(
     'input_list,grant,output_tuple',
     [