Allow the "%" character in database name

[koleo]

The naming rules for MySQL/MariaDB identifiers, when quoted, allow the % character.

However, currently, the use of the % character in database names results in mismatch or missing databases.

SUMMARY

• Rewrite query to identify the databases in the catalog using information_schema instead of SHOW DATABASES LIKE
• Escape the % character in CREATE DATABASE query (because of the Python format string)

ISSUE TYPE

• Bugfix Pull Request

COMPONENT NAME

mysql_db

ADDITIONAL INFORMATION

• Test playbook

# test_db_present.yaml
---
- hosts: localhost
  connection: local
  tasks:
    - mysql_db:
        name: "db%"
        state: present
        login_unix_socket: /var/run/mysqld/mysqld.sock

• Actual behavior (before this PR)

$ ansible-playbook test_db_present.yaml

PLAY [localhost] *******************************************************************************************************************************************************************************************

TASK [Gathering Facts] *************************************************************************************************************************************************************************************
ok: [localhost]

TASK [mysql_db] ********************************************************************************************************************************************************************************************
An exception occurred during task execution. To see the full traceback, use -vvv. The error was: ValueError: unsupported format character '`' (0x60) at index 20
fatal: [localhost]: FAILED! => {"changed": false, "msg": "error creating database: unsupported format character '`' (0x60) at index 20"}

PLAY RECAP *************************************************************************************************************************************************************************************************
localhost                  : ok=1    changed=0    unreachable=0    failed=1    skipped=0    rescued=0    ignored=0

• New behavior (after this PR)

$ ansible-playbook test_db_present.yaml

PLAY [localhost] *******************************************************************************************************************************************************************************************

TASK [Gathering Facts] *************************************************************************************************************************************************************************************
ok: [localhost]

TASK [mysql_db] ********************************************************************************************************************************************************************************************
changed: [localhost]

PLAY RECAP *************************************************************************************************************************************************************************************************
localhost                  : ok=2    changed=1    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0

Check database list

$ mysql -e "SHOW DATABASES"
+--------------------+
| Database           |
+--------------------+
| db%                |
| information_schema |
| mysql              |
| performance_schema |
+--------------------+

[Andersson007]

@koleo hi, thanks for the PR and welcome to the project!
Could you please

1. add a changelog fragment https://docs.ansible.com/ansible/latest/community/development_process.html#creating-a-changelog-fragment
2. add integration tests to tests/integration/targets/test_mysql_db/*, i.e.:

• add a task that add a db with a name containing % and then
• a check with mysql_query module that the DB is actually present

3. if any questions appear, feel free to ask. BTW there's info how to run integration tests locally if needed https://github.com/ansible/community-docs/blob/main/create_pr_quick_start_guide.rst#test-your-changes

[codecov]

Codecov Report

Merging #227 (1c5a23b) into main (f47d463) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #227   +/-   ##
=======================================
  Coverage   77.60%   77.60%           
=======================================
  Files          24       24           
  Lines        2170     2170           
  Branches      510      510           
=======================================
  Hits         1684     1684           
  Misses        319      319           
  Partials      167      167           

Impacted Files	Coverage Δ
plugins/modules/mysql_db.py	74.56% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f47d463...1c5a23b. Read the comment docs.

[Andersson007]

@koleo could you also please rebase the PR? I've just updated the test matrix, would be nice to test it in the updated CI

[koleo]

@Andersson007 thanks for the tips. I will complete the missing steps and rebase soon.

[Andersson007]

@koleo cool, thanks:) When the PR is ready for review again, please let me know

[koleo]

Hi @Andersson007 , so I pushed an update with Fragment, tests and rebase.

The test file is inspired by the existing create/drop database tests from main.yml with a specific db_name containing the % character. This is a duplication of code but refactoring the current test files would be a hard work and should be part of a new PR IMHO.

[Andersson007]

@koleo hi, i think

1. we should do similar changes (i.e. replacement) for the other states (absent, dump, import) and test them. I tried to drop a database without an additional % and it failed. All the states should support this kind of name.
2. we could use the mysql_query module instead of command (though I'm not strictly against checking with command, just a fyi)

[Andersson007]

i think it would be much faster if you run the tests locally before pushing using this guide

There are also sanity issues (you can see them by clicking "Details" in the test report box near failed tasks).

ERROR: Found 2 yamllint issue(s) which need to be resolved:
ERROR: tests/integration/targets/test_mysql_db/tasks/db_create_special.yml:86:1: empty-lines: too many blank lines (1 > 0)
ERROR: tests/integration/targets/test_mysql_db/tasks/main.yml:329:1: empty-lines: too many blank lines (1 > 0)

[koleo]

@Andersson007 it's ready for a review.
My last changes in tests folder:

• Move the main.yml content to state_present_absent.yml leaving only the mysql_command and the include parts.
• include state_present_absent.yml (with default db_name)
• include state_present_absent.yml for db_name with % char
• include state_dump_import.yml for db_name with % char
• Fix a few things in original main code (now in state_present_absent.yml) like quoting dbname in SHOW CREATE DATABASE commands or escaping special chars in LIKE queries (=> escape chars %, _, \)

[Andersson007]

@koleo thanks, i'll try to review it tomorrow

@bmalynovytch @Jorge-Rodriguez could you please take a look at this PR too

[Andersson007]

@koleo thanks for working on this! Let's wait a couple of days for @bmalynovytch and @Jorge-Rodriguez 's feedback.
If no objections, I'll merge the PR max on Monday. Thank you

[bmalynovytch]

Changes related to the described PR look good 👍
I didn't deep dive in the tests though.

[Andersson007]

@koleo thanks for the contribution! @bmalynovytch thanks for reviewing!