Replies: 4 comments 2 replies
-
|
@parseword hi, thanks for reporting this! Would you like to create a pull request yourself? (If you don't know how to do it, we have a short quick-start guide. If you don't want, we could. |
Beta Was this translation helpful? Give feedback.
-
|
@Andersson007 I've been looking at the code this issue refers to and while the concern is valid, we seem to be doing something fairly stupid, namely:
I argue that rather than patching the loop as @parseword has suggested, we should rework the privileges handling logic and call a fixed version of |
Beta Was this translation helpful? Give feedback.
-
|
Good spot! I added the dict form of passing privs to the module long time after someone added the initial string implementation, so didn't want to touch code parts that were not necessary to touch for the sake of safety:) |
Beta Was this translation helpful? Give feedback.
-
|
@Andersson007 @parseword In light of the reason for closing PR #137 I've migrated this issue into a discussion to flesh out an action plan regarding my comment above and so that we can create a dedicated issue if we feel that's necessary. |
Beta Was this translation helpful? Give feedback.
-
|
BTW, On further inspection, it would seem that the offending lines were: output[pieces[0]] = pieces[1].upper().split(',')
privs = output[pieces[0]]Where we are asigning a list to another, which is a recipe for disaster because we are assigning two variables to the same memory address opening the door for all sorts of nasty side effects: >>> a = []
>>> b = []
>>> a = b
>>> a
[]
>>> b
[]
>>> a.append(1)
>>> a
[1]
>>> b
[1]
>>> b.append(2)
>>> a
[1, 2]
>>> b
[1, 2] |
Beta Was this translation helpful? Give feedback.
-
|
Good catch! |
Beta Was this translation helpful? Give feedback.
-
SUMMARY
In some scenarios,
privileges_unpack()callsprivs.append()inside a loop without first emptying or reinitializing theprivslist from the prior iteration. This can result in an invalidGRANTstatement, which incorrectly includes privileges from a previously-builtGRANTstatement.In my environment, the bug is triggered when the
privspecification from an Ansible task contains both of the following:ALLprivilegeDuring execution of
privileges_unpack(), the privilege lists for multiple grants become commingled. This results in the module building a bogus statement likeGRANT ALL,SELECT,INSERT,UPDATE ON [...]which throws a MySQL syntax error.I've traced the problem to here. At the time
privs.append()is called,privsstill contains whatever the last iteration of thefor item in priv.strip().split('/'):loop put into it.In my environment, I resolved the problem by moving the initialization of
privsinto the outer loop so it gets cleared on each pass.Here's a giant patch: 8ceec70
ISSUE TYPE
COMPONENT NAME
community.mysql.mysql_userANSIBLE VERSION
CONFIGURATION
OS / ENVIRONMENT
STEPS TO REPRODUCE
Set up a MySQL test environment:
Add a user and grant some mixed privileges, including column privileges:
Inspect the grants:
Get the user's hashed password for the Ansible task:
Drop the user, to simulate adding it as a new user in the Ansible task:
Now attempt to create the user through this Ansible task. (Assume that
/root/.my.cnfhas already been configured to allow root access to MySQL.)EXPECTED RESULTS
The user is added with all of the privileges specified in the
privdictionary, and the task succeeds.ACTUAL RESULTS
The task ultimately fails. It does manage to add the user and some of the privileges, by successfully running these queries against MySQL:
But then it attempts to run an invalid query:
That query throws a syntax error due to specifying both
ALLand granular privileges, instead of justALL. The MySQL error causes the remainder of the task to bomb out.Ansible output:
Note that the Ansible output doesn't show the origin of the problem, or the full failed query text. I tracked it down to
privileges_unpack()using some messy debugging (I don't know python so I stuck a bunch of print statements throughoutmysql_user.py).TESTING THE PATCH
After applying the patch at 8ceec70, and dropping the MySQL user again to simulate a clean environment, the task succeeds and MySQL shows it ran the correct set of
GRANTqueries:Ansible output:
MySQL query log:
MySQL grant info:
On subsequent runs, the patched module exhibits idempotent behavior:
Ansible output:
MySQL query log:
Beta Was this translation helpful? Give feedback.
All reactions