From ebb37ae7a3b126603cfe4066aa69e3e9c7cc93e7 Mon Sep 17 00:00:00 2001
From: Soledad208 <luongminhtuan208@gmail.com>
Date: Thu, 7 Nov 2024 15:56:31 +0700
Subject: [PATCH] sql_mode can be set in session, therefore we should look for
 ANSI_QUOTES in session variable instead of global variable (#677)

* issue-671: get ASNI_QUOTES from session sql_mode instead of GLOBAL sql_mode
---
 .../fragments/671-modules_util_user.yml       |  12 ++
 plugins/module_utils/user.py                  |   2 +-
 .../test_mysql_user/tasks/issue-671.yaml      | 112 ++++++++++++++++++
 .../targets/test_mysql_user/tasks/main.yml    |   6 +
 4 files changed, 131 insertions(+), 1 deletion(-)
 create mode 100644 changelogs/fragments/671-modules_util_user.yml
 create mode 100644 tests/integration/targets/test_mysql_user/tasks/issue-671.yaml

diff --git a/changelogs/fragments/671-modules_util_user.yml b/changelogs/fragments/671-modules_util_user.yml
new file mode 100644
index 0000000..a913651
--- /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/plugins/module_utils/user.py b/plugins/module_utils/user.py
index 7b6914f..307ef6e 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 0000000..3696cf0
--- /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
+      community.mysql.mysql_db:
+        <<: *mysql_params
+        name: "{{ item }}"
+        state: absent
+      loop:
+        - foo
+        - bar
+
+    - name: Issue-671| test setup | create database
+      community.mysql.mysql_db:
+        <<: *mysql_params
+        name: "{{ item }}"
+        state: present
+      loop:
+        - foo
+        - bar
+
+    - name: Issue-671| test setup | get value of GLOBAL.sql_mode
+      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
+      ansible.builtin.assert:
+        that:
+          - sql_mode_orig.query_result[0][0].sql_mode != None  
+
+    - name: Issue-671| enable sql_mode ANSI_QUOTES
+      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
+      ansible.builtin.copy:
+        src: "{{ item }}"
+        dest: "{{ remote_tmp_dir }}/{{ item | basename }}"
+      loop:
+        - create-function.sql
+        - create-procedure.sql
+
+    - name: Issue-671| Create function for test
+      ansible.builtin.shell:
+        cmd: "{{ mysql_command }} < {{ remote_tmp_dir }}/create-function.sql"
+
+    - name: Issue-671| Create procedure for test
+      ansible.builtin.shell:
+        cmd: "{{ mysql_command }} < {{ remote_tmp_dir }}/create-procedure.sql"
+
+    - name: Issue-671| Create user with FUNCTION and PROCEDURE privileges
+      community.mysql.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
+      community.mysql.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
+      failed_when:
+        - result is failed or result is changed
+
+    - name: Issue-671| Test teardown | cleanup databases
+      community.mysql.mysql_db:
+        <<: *mysql_params
+        name: "{{ item }}"
+        state: absent
+      loop:
+        - foo
+        - bar
+
+    - name: Issue-671| set sql_mode back to original value
+      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 %}"
+
+    - name: Issue-671| Teardown user_name_2
+      ansible.builtin.include_tasks:
+        file: utils/remove_user.yml
+      vars:
+        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 e77c443..9244570 100644
--- a/tests/integration/targets/test_mysql_user/tasks/main.yml
+++ b/tests/integration/targets/test_mysql_user/tasks/main.yml
@@ -282,6 +282,12 @@
     - import_tasks: issue-64560.yaml
       tags:
         - issue-64560
+    
+    - name: Test ANSI_QUOTES
+      ansible.builtin.import_tasks:
+        file: 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)