From 11958ec46a1f88d4cd4f811e528312c8d31911cd Mon Sep 17 00:00:00 2001 From: "Jorge Rodriguez (A.K.A. Tiriel)" Date: Tue, 16 Mar 2021 10:15:19 +0200 Subject: [PATCH] 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 --- .../103-mysql_and_mariadb_divergence.yml | 2 + .../implementations/mariadb/replication.py | 10 ++++ .../implementations/mariadb/user.py | 15 +++++ .../implementations/mysql/replication.py | 10 ++++ .../implementations/mysql/user.py | 16 ++++++ plugins/modules/mysql_replication.py | 26 +++------ plugins/modules/mysql_user.py | 56 +++++-------------- .../module_utils/test_mariadb_replication.py | 24 ++++++++ .../test_mariadb_user_implementation.py | 29 ++++++++++ .../test_mysql_replication.py | 6 +- .../test_mysql_user_implementation.py | 33 +++++++++++ tests/unit/plugins/modules/test_mysql_user.py | 27 --------- 12 files changed, 162 insertions(+), 92 deletions(-) create mode 100644 changelogs/fragments/103-mysql_and_mariadb_divergence.yml create mode 100644 plugins/module_utils/implementations/mariadb/replication.py create mode 100644 plugins/module_utils/implementations/mariadb/user.py create mode 100644 plugins/module_utils/implementations/mysql/replication.py create mode 100644 plugins/module_utils/implementations/mysql/user.py create mode 100644 tests/unit/plugins/module_utils/test_mariadb_replication.py create mode 100644 tests/unit/plugins/module_utils/test_mariadb_user_implementation.py rename tests/unit/plugins/{modules => module_utils}/test_mysql_replication.py (74%) create mode 100644 tests/unit/plugins/module_utils/test_mysql_user_implementation.py diff --git a/changelogs/fragments/103-mysql_and_mariadb_divergence.yml b/changelogs/fragments/103-mysql_and_mariadb_divergence.yml new file mode 100644 index 0000000..039c654 --- /dev/null +++ b/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). diff --git a/plugins/module_utils/implementations/mariadb/replication.py b/plugins/module_utils/implementations/mariadb/replication.py new file mode 100644 index 0000000..7968e92 --- /dev/null +++ b/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') diff --git a/plugins/module_utils/implementations/mariadb/user.py b/plugins/module_utils/implementations/mariadb/user.py new file mode 100644 index 0000000..fa2cac6 --- /dev/null +++ b/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 diff --git a/plugins/module_utils/implementations/mysql/replication.py b/plugins/module_utils/implementations/mysql/replication.py new file mode 100644 index 0000000..2fd3d6c --- /dev/null +++ b/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') diff --git a/plugins/module_utils/implementations/mysql/user.py b/plugins/module_utils/implementations/mysql/user.py new file mode 100644 index 0000000..ce7f5b8 --- /dev/null +++ b/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") diff --git a/plugins/modules/mysql_replication.py b/plugins/modules/mysql_replication.py index 9aea9fd..f46721a 100644 --- a/plugins/modules/mysql_replication.py +++ b/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.', diff --git a/plugins/modules/mysql_user.py b/plugins/modules/mysql_user.py index 773f4b9..bb0f5a0 100644 --- a/plugins/modules/mysql_user.py +++ b/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) diff --git a/tests/unit/plugins/module_utils/test_mariadb_replication.py b/tests/unit/plugins/module_utils/test_mariadb_replication.py new file mode 100644 index 0000000..deb3099 --- /dev/null +++ b/tests/unit/plugins/module_utils/test_mariadb_replication.py @@ -0,0 +1,24 @@ +# -*- coding: utf-8 -*- +# Copyright: (c) 2020, Andrew Klychkov (@Andersson007) + +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 diff --git a/tests/unit/plugins/module_utils/test_mariadb_user_implementation.py b/tests/unit/plugins/module_utils/test_mariadb_user_implementation.py new file mode 100644 index 0000000..a6fbff9 --- /dev/null +++ b/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 diff --git a/tests/unit/plugins/modules/test_mysql_replication.py b/tests/unit/plugins/module_utils/test_mysql_replication.py similarity index 74% rename from tests/unit/plugins/modules/test_mysql_replication.py rename to tests/unit/plugins/module_utils/test_mysql_replication.py index 27473b7..96d4d9a 100644 --- a/tests/unit/plugins/modules/test_mysql_replication.py +++ b/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): diff --git a/tests/unit/plugins/module_utils/test_mysql_user_implementation.py b/tests/unit/plugins/module_utils/test_mysql_user_implementation.py new file mode 100644 index 0000000..c1fe2ee --- /dev/null +++ b/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 diff --git a/tests/unit/plugins/modules/test_mysql_user.py b/tests/unit/plugins/modules/test_mysql_user.py index 2ab9267..fe5c853 100644 --- a/tests/unit/plugins/modules/test_mysql_user.py +++ b/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', [