From 6d4fc3c24f05aff258b5f4e30c5743e901d55a2d Mon Sep 17 00:00:00 2001 From: SoledaD208 Date: Thu, 10 Oct 2024 23:18:14 +0700 Subject: [PATCH 1/4] issue-671: get ASNI_QUOTES from session sql_mode instead of GLOBAL sql_mode --- plugins/module_utils/user.py | 2 +- .../test_mysql_user/tasks/issue-671.yaml | 112 ++++++++++++++++++ .../targets/test_mysql_user/tasks/main.yml | 4 + 3 files changed, 117 insertions(+), 1 deletion(-) create mode 100644 tests/integration/targets/test_mysql_user/tasks/issue-671.yaml diff --git a/plugins/module_utils/user.py b/plugins/module_utils/user.py index 7b6914f2..307ef6e6 100644 --- a/plugins/module_utils/user.py +++ b/plugins/module_utils/user.py @@ -32,7 +32,7 @@ class InvalidPrivsError(Exception): def get_mode(cursor): - cursor.execute('SELECT @@GLOBAL.sql_mode') + cursor.execute('SELECT @@sql_mode') result = cursor.fetchone() mode_str = result[0] if 'ANSI' in mode_str: diff --git a/tests/integration/targets/test_mysql_user/tasks/issue-671.yaml b/tests/integration/targets/test_mysql_user/tasks/issue-671.yaml new file mode 100644 index 00000000..3f6cec5d --- /dev/null +++ b/tests/integration/targets/test_mysql_user/tasks/issue-671.yaml @@ -0,0 +1,112 @@ +--- +# Due to https://bugs.mysql.com/bug.php?id=115953, in Mysql 8, if ANSI_QUOTES is enabled, +# backticks will be used instead of double quotes to quote functions or procedures name. +# As a consequence, mysql_user and mysql_roles will always report "changed" for functions +# and procedures no matter the privileges are granted or not. +# Workaround for the mysql bug 116953 is removing ANSI_QUOTES from the module's session +# sql_mode. But because issue 671, ANSI_QUOTES is always got from GLOBAL sql_mode, thus +# this workaround can't work. Even without the Mysql bug, because sql_mode in session +# precedes GLOBAL sql_mode. we should check for sql_mode in session variable instead of +# the GLOBAL one. +- vars: + mysql_parameters: &mysql_params + login_user: '{{ mysql_user }}' + login_password: '{{ mysql_password }}' + login_host: '{{ mysql_host }}' + login_port: '{{ mysql_primary_port }}' + + block: + - name: Issue-671| test setup | drop database + mysql_db: + <<: *mysql_params + name: "{{ item }}" + state: absent + loop: + - foo + - bar + + - name: Issue-671| test setup | create database + mysql_db: + <<: *mysql_params + name: "{{ item }}" + state: present + loop: + - foo + - bar + + - name: Issue-671| test setup | get value of GLOBAL.sql_mode + mysql_query: + <<: *mysql_params + query: 'select @@GLOBAL.sql_mode AS sql_mode' + register: sql_mode_orig + + - name: Issue-671| enable sql_mode ANSI_QUOTES + mysql_variables: + <<: *mysql_params + variable: sql_mode + value: '{{ sql_mode_orig.query_result[0][0].sql_mode }},ANSI_QUOTES' + mode: persist + + - name: Issue-671| test setup | get value of GLOBAL.sql_mode + mysql_query: + <<: *mysql_params + query: 'select @@GLOBAL.sql_mode' + + - name: Issue-671| Copy SQL scripts to remote + copy: + src: "{{ item }}" + dest: "{{ remote_tmp_dir }}/{{ item | basename }}" + with_items: + - create-function.sql + - create-procedure.sql + + - name: Issue-671| Create function for test + shell: "{{ mysql_command }} < {{ remote_tmp_dir }}/create-function.sql" + + - name: Issue-671| Create procedure for test + shell: "{{ mysql_command }} < {{ remote_tmp_dir }}/create-procedure.sql" + + - name: Issue-671| Create user with FUNCTION and PROCEDURE privileges + mysql_user: + <<: *mysql_params + name: '{{ user_name_2 }}' + password: '{{ user_password_2 }}' + state: present + priv: 'FUNCTION foo.function:EXECUTE/foo.*:SELECT/PROCEDURE bar.procedure:EXECUTE' + + - name: Issue-671| Grant the privileges again, remove ANSI_QUOTES from the session variable + mysql_user: + <<: *mysql_params + session_vars: + sql_mode: "" + name: '{{ user_name_2 }}' + password: '{{ user_password_2 }}' + state: present + priv: 'FUNCTION foo.function:EXECUTE/foo.*:SELECT/PROCEDURE bar.procedure:EXECUTE' + register: result + + - name: Issue-671| Assert Create user with FUNCTION and PROCEDURE privileges + assert: + that: + - result is success + - result is not changed + + - name: Issue-671| Test teardown | cleanup databases + mysql_db: + <<: *mysql_params + name: "{{ item }}" + state: absent + loop: + - foo + - bar + + - name: Issue-671| set sql_mode back to original value + mysql_variables: + <<: *mysql_params + variable: sql_mode + value: '{{ sql_mode_orig.query_result[0][0].sql_mode }}' + mode: persist + + - include_tasks: utils/remove_user.yml + vars: + user_name: "{{ user_name_2 }}" diff --git a/tests/integration/targets/test_mysql_user/tasks/main.yml b/tests/integration/targets/test_mysql_user/tasks/main.yml index e77c4436..4e9046f3 100644 --- a/tests/integration/targets/test_mysql_user/tasks/main.yml +++ b/tests/integration/targets/test_mysql_user/tasks/main.yml @@ -282,6 +282,10 @@ - import_tasks: issue-64560.yaml tags: - issue-64560 + + - import_tasks: issue-671.yaml + tags: + - issue-671 # Test that mysql_user still works with force_context enabled (database set to "mysql") # (https://github.com/ansible-collections/community.mysql/issues/265) From 8ef32e5eff3f93fc4b1f5804e704911f1e60572b Mon Sep 17 00:00:00 2001 From: SoledaD208 Date: Fri, 11 Oct 2024 00:10:57 +0700 Subject: [PATCH 2/4] issue 671: improve test playbook --- .../integration/targets/test_mysql_user/tasks/issue-671.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integration/targets/test_mysql_user/tasks/issue-671.yaml b/tests/integration/targets/test_mysql_user/tasks/issue-671.yaml index 3f6cec5d..384c3ce5 100644 --- a/tests/integration/targets/test_mysql_user/tasks/issue-671.yaml +++ b/tests/integration/targets/test_mysql_user/tasks/issue-671.yaml @@ -45,7 +45,7 @@ <<: *mysql_params variable: sql_mode value: '{{ sql_mode_orig.query_result[0][0].sql_mode }},ANSI_QUOTES' - mode: persist + mode: "{% if db_engine == 'mariadb' %}global{% else %}persist{% endif %}" - name: Issue-671| test setup | get value of GLOBAL.sql_mode mysql_query: @@ -105,7 +105,7 @@ <<: *mysql_params variable: sql_mode value: '{{ sql_mode_orig.query_result[0][0].sql_mode }}' - mode: persist + mode: "{% if db_engine == 'mariadb' %}global{% else %}persist{% endif %}" - include_tasks: utils/remove_user.yml vars: From 596743374b7f6c74a9deb08de23971b964f8c15a Mon Sep 17 00:00:00 2001 From: SoledaD208 Date: Mon, 21 Oct 2024 22:20:35 +0700 Subject: [PATCH 3/4] improve the test file and add fragment files for issue 671 --- changelogs/fragments/671-modules_util_user.yml | 12 ++++++++++++ .../targets/test_mysql_user/tasks/issue-671.yaml | 10 +++++----- 2 files changed, 17 insertions(+), 5 deletions(-) create mode 100644 changelogs/fragments/671-modules_util_user.yml diff --git a/changelogs/fragments/671-modules_util_user.yml b/changelogs/fragments/671-modules_util_user.yml new file mode 100644 index 00000000..a9136516 --- /dev/null +++ b/changelogs/fragments/671-modules_util_user.yml @@ -0,0 +1,12 @@ +bugfixes: + - mysql_user,mysql_role - The sql_mode ANSI_QUOTES affects how the modules mysql_user + and mysql_role compare the existing privileges with the configured privileges, + as well as decide whether double quotes or backticks should be used in the GRANT + statements. Pointing out in issue 671, the modules mysql_user and mysql_role allow + users to enable/disable ANSI_QUOTES in session variable (within a DB session, the + session variable always overwrites the global one). But due to the issue, the modules + do not check for ANSI_MODE in the session variable, instead, they only check in the + GLOBAL one.That behavior is not only limiting the users' flexibility, but also not + allowing users to explicitly disable ANSI_MODE to work around such bugs like + https://bugs.mysql.com/bug.php?id=115953. + (https://github.com/ansible-collections/community.mysql/issues/671) \ No newline at end of file diff --git a/tests/integration/targets/test_mysql_user/tasks/issue-671.yaml b/tests/integration/targets/test_mysql_user/tasks/issue-671.yaml index 384c3ce5..fa3d3213 100644 --- a/tests/integration/targets/test_mysql_user/tasks/issue-671.yaml +++ b/tests/integration/targets/test_mysql_user/tasks/issue-671.yaml @@ -40,6 +40,11 @@ query: 'select @@GLOBAL.sql_mode AS sql_mode' register: sql_mode_orig + - name: Issue-671| Assert sql_mode_orig + assert: + that: + - sql_mode_orig.query_result[0][0].sql_mode != None + - name: Issue-671| enable sql_mode ANSI_QUOTES mysql_variables: <<: *mysql_params @@ -47,11 +52,6 @@ value: '{{ sql_mode_orig.query_result[0][0].sql_mode }},ANSI_QUOTES' mode: "{% if db_engine == 'mariadb' %}global{% else %}persist{% endif %}" - - name: Issue-671| test setup | get value of GLOBAL.sql_mode - mysql_query: - <<: *mysql_params - query: 'select @@GLOBAL.sql_mode' - - name: Issue-671| Copy SQL scripts to remote copy: src: "{{ item }}" From 24bf6397c9f986f66fa5be366164cd99c0a38e1c Mon Sep 17 00:00:00 2001 From: SoledaD208 Date: Thu, 31 Oct 2024 22:35:42 +0700 Subject: [PATCH 4/4] improve tests --- .../test_mysql_user/tasks/issue-671.yaml | 42 +++++++++---------- .../targets/test_mysql_user/tasks/main.yml | 6 ++- 2 files changed, 25 insertions(+), 23 deletions(-) diff --git a/tests/integration/targets/test_mysql_user/tasks/issue-671.yaml b/tests/integration/targets/test_mysql_user/tasks/issue-671.yaml index fa3d3213..3696cf00 100644 --- a/tests/integration/targets/test_mysql_user/tasks/issue-671.yaml +++ b/tests/integration/targets/test_mysql_user/tasks/issue-671.yaml @@ -17,7 +17,7 @@ block: - name: Issue-671| test setup | drop database - mysql_db: + community.mysql.mysql_db: <<: *mysql_params name: "{{ item }}" state: absent @@ -26,7 +26,7 @@ - bar - name: Issue-671| test setup | create database - mysql_db: + community.mysql.mysql_db: <<: *mysql_params name: "{{ item }}" state: present @@ -35,39 +35,41 @@ - bar - name: Issue-671| test setup | get value of GLOBAL.sql_mode - mysql_query: + community.mysql.mysql_query: <<: *mysql_params query: 'select @@GLOBAL.sql_mode AS sql_mode' register: sql_mode_orig - name: Issue-671| Assert sql_mode_orig - assert: + ansible.builtin.assert: that: - sql_mode_orig.query_result[0][0].sql_mode != None - name: Issue-671| enable sql_mode ANSI_QUOTES - mysql_variables: + community.mysql.mysql_variables: <<: *mysql_params variable: sql_mode value: '{{ sql_mode_orig.query_result[0][0].sql_mode }},ANSI_QUOTES' mode: "{% if db_engine == 'mariadb' %}global{% else %}persist{% endif %}" - name: Issue-671| Copy SQL scripts to remote - copy: + ansible.builtin.copy: src: "{{ item }}" dest: "{{ remote_tmp_dir }}/{{ item | basename }}" - with_items: + loop: - create-function.sql - create-procedure.sql - name: Issue-671| Create function for test - shell: "{{ mysql_command }} < {{ remote_tmp_dir }}/create-function.sql" + ansible.builtin.shell: + cmd: "{{ mysql_command }} < {{ remote_tmp_dir }}/create-function.sql" - name: Issue-671| Create procedure for test - shell: "{{ mysql_command }} < {{ remote_tmp_dir }}/create-procedure.sql" + ansible.builtin.shell: + cmd: "{{ mysql_command }} < {{ remote_tmp_dir }}/create-procedure.sql" - name: Issue-671| Create user with FUNCTION and PROCEDURE privileges - mysql_user: + community.mysql.mysql_user: <<: *mysql_params name: '{{ user_name_2 }}' password: '{{ user_password_2 }}' @@ -75,7 +77,7 @@ priv: 'FUNCTION foo.function:EXECUTE/foo.*:SELECT/PROCEDURE bar.procedure:EXECUTE' - name: Issue-671| Grant the privileges again, remove ANSI_QUOTES from the session variable - mysql_user: + community.mysql.mysql_user: <<: *mysql_params session_vars: sql_mode: "" @@ -84,15 +86,11 @@ state: present priv: 'FUNCTION foo.function:EXECUTE/foo.*:SELECT/PROCEDURE bar.procedure:EXECUTE' register: result - - - name: Issue-671| Assert Create user with FUNCTION and PROCEDURE privileges - assert: - that: - - result is success - - result is not changed + failed_when: + - result is failed or result is changed - name: Issue-671| Test teardown | cleanup databases - mysql_db: + community.mysql.mysql_db: <<: *mysql_params name: "{{ item }}" state: absent @@ -101,12 +99,14 @@ - bar - name: Issue-671| set sql_mode back to original value - mysql_variables: + community.mysql.mysql_variables: <<: *mysql_params variable: sql_mode value: '{{ sql_mode_orig.query_result[0][0].sql_mode }}' mode: "{% if db_engine == 'mariadb' %}global{% else %}persist{% endif %}" - - include_tasks: utils/remove_user.yml + - name: Issue-671| Teardown user_name_2 + ansible.builtin.include_tasks: + file: utils/remove_user.yml vars: - user_name: "{{ user_name_2 }}" + user_name: "{{ user_name_2 }}" \ No newline at end of file diff --git a/tests/integration/targets/test_mysql_user/tasks/main.yml b/tests/integration/targets/test_mysql_user/tasks/main.yml index 4e9046f3..9244570f 100644 --- a/tests/integration/targets/test_mysql_user/tasks/main.yml +++ b/tests/integration/targets/test_mysql_user/tasks/main.yml @@ -283,9 +283,11 @@ tags: - issue-64560 - - import_tasks: issue-671.yaml + - name: Test ANSI_QUOTES + ansible.builtin.import_tasks: + file: issue-671.yaml tags: - - issue-671 + - issue-671 # Test that mysql_user still works with force_context enabled (database set to "mysql") # (https://github.com/ansible-collections/community.mysql/issues/265)