fix(salt-user): fix setting grants for the salt user #261
Conversation
javierbertoli
left a comment
javierbertoli
left a comment
There was a problem hiding this comment.
@pprkut good catch!
as you're in it, I propose you a couple of changes that I think makes sense and would be great if you apply them.
|
@javierbertoli Of course! I can't see them, however :-) |
aaargh! It seems that when I clicked to add that comment, didn't save the other text 🤦♂️ |
mysql/salt-user.sls
Outdated
| - mysql --user {{ mysql_salt_user }} --password='{{ mysql_salt_pass|replace("'", "'\"'\"'") }}' -h {{ mysql_host }} --execute="SELECT 1; 2>&1" | ||
| - VALUE=$(mysql --user {{ mysql_salt_user }} --password='{{ mysql_salt_pass|replace("'", "'\"'\"'") }}' -ss -e "SELECT Grant_priv FROM mysql.user WHERE user = '{{ mysql_salt_user }}' AND host = '{{ host }}';" 2>&1); if [ "$VALUE" = 'Y' ]; then /bin/true; else /bin/false; fi | ||
| - mysql --user {{ mysql_salt_user }} --password='{{ mysql_salt_pass|replace("'", "'\"'\"'") }}' -h {{ mysql_host }} --execute="SELECT 1;" 2>&1 | ||
| - VALUE=$(mysql --user {{ mysql_salt_user }} --password='{{ mysql_salt_pass|replace("'", "'\"'\"'") }}' -ss -e "SELECT Grant_priv FROM mysql.user WHERE user = '{{ mysql_salt_user }}' AND host = '{{ host }}';" 2>/dev/null); if [ "$VALUE" = 'Y' ]; then /bin/true; else /bin/false; fi |
There was a problem hiding this comment.
I think that these two changes might be useful:
- For the sake of consistency, redirect stderr to
/dev/nullin all the cases - For the sake of readability, replace those
replace()s withyaml_dquote, like
- mysql --user {{ mysql_salt_user }} --password={{ mysql_salt_pass | yaml_dquote }} -h {{ mysql_host }} --execute="SELECT 1;" 2>&1which should take care of properly escaping and quoting the password string.
Wdyt?
There was a problem hiding this comment.
Hey! Sorry, this took so long :-(
I made the changes, but I've only been able to verify them on one host so far, and salt crapped out on it so I didn't see the full job output. It worked, but I'd be more comfortable with a bit more testing.
m2mobi-mirror
force-pushed
the
fix_salt_user_grants
branch
from
cc6bb96 to
346633d
Compare
javierbertoli
left a comment
javierbertoli
left a comment
There was a problem hiding this comment.
@pprkut They look good to me. Thanks!
|
🎉 This PR is included in version 0.55.2 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
3 participants
PR progress checklist (to be filled in by reviewers)
What type of PR is this?
Primary type
[build]Changes related to the build system[chore]Changes to the build process or auxiliary tools and libraries such as documentation generation[ci]Changes to the continuous integration configuration[feat]A new feature[fix]A bug fix[perf]A code change that improves performance[refactor]A code change that neither fixes a bug nor adds a feature[revert]A change used to revert a previous commit[style]Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc.)Secondary type
[docs]Documentation changes[test]Adding missing or correcting existing testsDoes this PR introduce a
BREAKING CHANGE?No.
Related issues and/or pull requests
Follow up to #260
More complete fix for #230
Describe the changes you're proposing
This is embarrassing :(
I swear I tested the previous pull requests, but I just finished setting up a couple more servers and ran into these issues...
Pillar / config required to test the proposed changes
Debug log showing how the proposed changes work
Documentation checklist
README(e.g.Available states).pillar.example.Testing checklist
state_top).Additional context