Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

mysql_user idempotency issue with REQUIRESSL and ALL privileges #89

Closed
kkeane opened this issue Jan 14, 2021 · 12 comments · Fixed by #132
Closed

mysql_user idempotency issue with REQUIRESSL and ALL privileges #89

kkeane opened this issue Jan 14, 2021 · 12 comments · Fixed by #132

Comments

@kkeane
Copy link

kkeane commented Jan 14, 2021

SUMMARY

There is an idempotency mismatch for certain privilege specifications. This probably happens because the names of certain privileges do not exactly match the names used by the database. See Ansible bug #29625 (which is not actually resolved for these cases).

Specific examples I found (for MariaDB 10.5)

Specify privileges as "db.*:ALL,REQUIRESSL"
Specify privileges as "db.*:ALL"
ISSUE TYPE
  • Bug Report
COMPONENT NAME

mysql_user

ANSIBLE VERSION
ansible 2.9.16
  config file = /home/kkeane/Documents/dev/ansible/ansible.cfg
  configured module search path = [u'/home/kkeane/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python2.7/site-packages/ansible
  executable location = /usr/bin/ansible
  python version = 2.7.5 (default, Aug 13 2020, 02:51:10) [GCC 4.8.5 20150623 (Red Hat 4.8.5-39)]
CONFIGURATION
ANSIBLE_NOCOWS(env: ANSIBLE_NOCOWS) = True
ANSIBLE_PIPELINING(/home/kkeane/Documents/dev/ansible/ansible.cfg) = True
DEFAULT_FORCE_HANDLERS(/home/kkeane/Documents/dev/ansible/ansible.cfg) = True
DEFAULT_FORKS(/home/kkeane/Documents/dev/ansible/ansible.cfg) = 10
DEFAULT_HOST_LIST(/home/kkeane/Documents/dev/ansible/ansible.cfg) = [u'/home/kkeane/Documents/dev/ansible/inventory/linux.yaml']
DEFAULT_MANAGED_STR(/home/kkeane/Documents/dev/ansible/ansible.cfg) = This file is managed by Ansible.%n
Do not edit this file. Changes will be reverted.%n
template: {file}
user: {uid}
host: {host}
DEFAULT_PRIVATE_KEY_FILE(/home/kkeane/Documents/dev/ansible/ansible.cfg) = /.{{ lookup('env','HOME') }}/.ssh/ansible_rsa
DEFAULT_REMOTE_USER(/home/kkeane/Documents/dev/ansible/ansible.cfg) = svc-ansible
DEFAULT_ROLES_PATH(/home/kkeane/Documents/dev/ansible/ansible.cfg) = [u'/home/kkeane/Documents/dev/ansible/roles', u'/home/kkeane/Documents/dev/ansible/download-roles', u'/usr/share/ansible/roles
DEFAULT_TIMEOUT(/home/kkeane/Documents/dev/ansible/ansible.cfg) = 300
DEFAULT_VAULT_PASSWORD_FILE(/home/kkeane/Documents/dev/ansible/ansible.cfg) = /home/kkeane/Documents/dev/ansible/vault-passwords
DISPLAY_SKIPPED_HOSTS(/home/kkeane/Documents/dev/ansible/ansible.cfg) = False
INVENTORY_ENABLED(/home/kkeane/Documents/dev/ansible/ansible.cfg) = [u'yaml', u'script', u'ini']
OS / ENVIRONMENT

Target OS: RHEL 8.3. MariaDB 10.5.8, installed from the MariaDB repository.

Also observed on some older versions of mysql.

Python 3.6.8 on the target.

STEPS TO REPRODUCE

(Note: derived from the actual playbook, but not tested verbatim)

- name: Create user for this DB
  mysql_user:
    name: 'dbuser'
    host: 'fqdn.example.com'
    priv: "sampledb.*:ALL,REQUIRESSL"
    login_user: root
    login_host: localhost
    config_file: /root/.my.cnf
  become: True
EXPECTED RESULTS

On first run, the task may report changed.
On subsequent runs, the task should report unchanged.

After changing "REQUIRESSL" to "REQUIRE SSL", the same behavior should occur.

ACTUAL RESULTS

The task reports as changed on every run.
The task produces an error when changing REQUIRESSL to "REQUIRE SSL"

Escalation succeeded
<redacted.example.com> (0, '\n{"changed": true, "user": "redacted_user", "msg": "Privileges updated", "invocation": {"module_args": {"config_file": "/root/.my.cnf", "name": "redacted_user", "login_user": "root", "login_host": "localhost", "host": "redacted.example.com", "priv": "redactedb.*:ALL,REQUIRESSL", "user": "redacted_user", "login_port": 3306, "encrypted": false,
"host_all": false, "state": "present", "append_privs": false, "check_implicit_admin": false, "update_password": "always", "connect_timeout": 30, "sql_log_bin": true, "login_password": null, "login_unix_socket": null, "password": null, "client_cert": null, "client_key": null, "ca_cert": null}}, "warnings": ["Module did not set no_log for update_password"]}\n', 'OpenSSH_7.4p1, OpenSSL 1.0.2k-fips  26 Jan 2017\r\ndebug1: Reading configuration data /home/kkeane/.ssh/config\r\ndebug2: checking match for \'user svc-ansible\' host redacted.example.com originally redacted.example.com\r\ndebug3: /home/kkeane/.ssh/config line 1: matched \'user "svc-ansible"\' \r\ndebug2: match found\r\ndebug1: /home/kkeane/.ssh/config line 71: Applying options for *.example.com\r\ndebug1: Reading configuration data /etc/ssh/ssh_config\r\ndebug1: /etc/ssh/ssh_config line 58: Applying options for *\r\ndebug1: auto-mux: Trying existing master\r\ndebug2: fd 4 setting O_NONBLOCK\r\
ndebug2: mux_client_hello_exchange: master version 4\r\ndebug3: mux_client_forwards: request forwardings: 0 local, 0 remote\r\ndebug3: mux_client_request_session: entering\r\ndebug3: mux_client_request_alive: entering\r\ndebug3: mux_client_request_alive: done pid = 31162\r\ndebug3: mux_client_request_session: session request sent\r\ndebug1: mux_client_request_session: master session id:
2\r\ndebug3: mux_client_read_packet: read header failed: Broken pipe\r\ndebug2: Received exit status from master 0\r\n')
changed: [redacted.example.com] => (item=redacted.example.com) => {
    "ansible_loop_var": "item",
    "changed": true,
    "invocation": {
        "module_args": {
            "append_privs": false,
            "ca_cert": null,
            "check_implicit_admin": false,
            "client_cert": null,
            "client_key": null,
            "config_file": "/root/.my.cnf",
            "connect_timeout": 30,
            "encrypted": false,
            "host": "redacted.example.com",
            "host_all": false,
            "login_host": "localhost",
            "login_password": null,
            "login_port": 3306,
            "login_unix_socket": null,
            "login_user": "root",
            "name": "redacted.example.com",
            "password": null,
            "priv": "redacteddb.*:ALL,REQUIRESSL",
            "sql_log_bin": true,
            "state": "present",
            "update_password": "always",
            "user": "redacted_user"
        }
    },
    "item": "redacted.sandiego.edu",
    "msg": "Privileges updated",
    "user": "redacted_user",
    "warnings": [
        "Module did not set no_log for update_password"
    ]
}

Changing the string "REQUIRESSL" to "REQUIRE SSL" results in:

failed: [redacted.example.com] (item=redacted.example.com) => {"ansible_loop_var": "item", "changed": false, "item": "redacted.example.com", "msg": "invalid privileges string: Invalid privileges specified: frozenset({'REQUIRE SSL'})"}

The underlying problem here is probably that the SHOW GRANTS and GRANT USAGE commands use the syntax "REQUIRE SSL" while the module only accepts "REQUIRESSL".

MariaDB [(none)]> show grants for redacteduser@'redacted.example.com';
+---------------------------------------------------------------------------------------------------------------------------------------------------+
| Grants for [email protected]                                                                                               |
+---------------------------------------------------------------------------------------------------------------------------------------------------+
| GRANT USAGE ON *.* TO `redacteduser`@`redacted.example.com` IDENTIFIED BY PASSWORD '*redacted' REQUIRE SSL |
| GRANT ALL PRIVILEGES ON `redacteddb`.* TO `redacted_user`@`redacted.example.com`                                                            |
+---------------------------------------------------------------------------------------------------------------------------------------------------+

Modified test case: changing the privileges to "redactedb.*:ALL" (without REQUIRESSL) also results in the task reporting "changed" on every run.
@Andersson007
Copy link
Collaborator

@kkeane thanks for reporting this!
Could you please try this using Ansible 2.10.5?

There was significant work done related to SSL during the recent months by @Jorge-Rodriguez.

Alternatively, 2.9 supports collections. You can get the latest collection tarbal from Galaxy. How to install and use it, see this (official) guide.

Waiting for your feedback!

@Jorge-Rodriguez
Copy link
Contributor

@kkeane the REQUIRESSL privilege is kept for backwards compatibility only and is bound to have certain minor bugs. The tls_requires parameter should be used instead.

Waiting for your feedback.

@Jorge-Rodriguez
Copy link
Contributor

@Andersson007 we've seen other bugs related to three REQUIRESSL privilege. I'm wondering if we shouldn't handle that syntax as an alias for the tls_requires parameter and get rid of the legacy code and the problematic conditional branches.

@Andersson007
Copy link
Collaborator

@Jorge-Rodriguez good question:) I''m not a great user of mysql and ssl related options. Does it impliy any breaking changes?
Would be interesting to hear @bmalynovytch 's openion.

@Jorge-Rodriguez
Copy link
Contributor

@Jorge-Rodriguez good question:) I''m not a great user of mysql and ssl related options. Does it impliy any breaking changes?
Would be interesting to hear @bmalynovytch 's openion.

Done carefully, it shouldn't be a breaking change, although personally, I'd like to deprecate the old REQUIRESSL especially now that the support for SSL options has grown far beyond what REQUIRESSL can offer.

@kkeane
Copy link
Author

kkeane commented Feb 12, 2021 via email

@Andersson007
Copy link
Collaborator

Andersson007 commented Feb 12, 2021 •
edited
Loading

@kkeane thanks for the feedback!
For information: you can be sure that your playbook uses community.mysql by using a fully qualified collection name (FQCN), i.e. change mysql_user: to community.mysql.mysql_user: (and other modules invocations similarly).

As far as I remember there's a similar issue related to reporting changed=True when using ALL, #77 , the issue's author explains why.
If anyone decides to fix this (if it's technically possible), I'd be happy to review.

@kkeane
Copy link
Author

kkeane commented Feb 12, 2021 via email

@Jorge-Rodriguez
Copy link
Contributor

@kkeane thank you so much for the info, it's very valuable.
The bad news is I won't be taking this issue immediately. Since it seems to involve discrepancies with MariaDB, I feel it makes more sense to handle the MariaDB refactor first.

This is still an important issue and it's high on my priority list, but I won't be providing a fix just yet.

I'm the meanwhile, whatever extra information you can gather is greatly appreciated.

@kkeane
Copy link
Author

kkeane commented Feb 12, 2021 via email

@kkeane
Copy link
Author

kkeane commented Feb 12, 2021 via email

This was referenced Feb 25, 2021
Draft
Open
@Jorge-Rodriguez
Copy link
Contributor

@kkeane we're dropping support for REQUIRESSL altogether, I'll have a PR ready soon for that. The support will be dropped in two stages, the first of which will effectively turn the REQUIRESSL privilege into an alias for {"tls_requires": {"ssl": true}}.

I would appreciate if you could verify whether this bug appears on your setup with the new code. I'll tag you and this issue on the PR.

@Jorge-Rodriguez Jorge-Rodriguez mentioned this issue Mar 30, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Deprecate REQUIRESSL privilege Jorge-Rodriguez/community.mysql
3 participants
@kkeane @Jorge-Rodriguez @Andersson007