Skip to content

mysql_user: added flush privileges to write dynamic privs into db #338

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.

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

Merged
merged 3 commits into from
Apr 29, 2022
Merged

Conversation

bigo8525
Copy link
Contributor

Fixes #120

SUMMARY

Added Flush Privileges after the revoke all privileges to write consitent the dynamic privileges which are required for cluster creation

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME
  • mysql_user.py
  • user.py
  • privileges_revoke function
ADDITIONAL INFORMATION

Without this fix a user with all privileges is only on every second run allowed to create a cluster or replicaset on the host.
Mysqlsh with dba.checkInstanceConfiguration() is failing because these rights are missing:
CLONE_ADMIN, CONNECTION_ADMIN, GROUP_REPLICATION_ADMIN, PERSIST_RO_VARIABLES_ADMIN, REPLICATION_APPLIER, REPLICATION_SLAVE_ADMIN, ROLE_ADMIN, SYSTEM_VARIABLES_ADMIN
After a new run with priv: "*.*:ALL,GRANT" the dba.checkInstanceConfiguration() check is successfull.
More Details could be found in the issue #120

@bigo8525 bigo8525 changed the title added flush privileges to write dynamic privs into db mysql_user: added flush privileges to write dynamic privs into db Apr 27, 2022
@codecov
Copy link

codecov bot commented Apr 27, 2022

Codecov Report

Merging #338 (a896b5d) into main (4aab8ac) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #338   +/-   ##
=======================================
  Coverage   78.26%   78.27%           
=======================================
  Files          27       27           
  Lines        2250     2251    +1     
  Branches      543      543           
=======================================
+ Hits         1761     1762    +1     
  Misses        333      333           
  Partials      156      156           
Impacted Files Coverage Δ
plugins/module_utils/user.py 87.00% <100.00%> (+0.03%) ⬆️

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 4aab8ac...a896b5d. Read the comment docs.

@Andersson007
Copy link
Collaborator

@bigo8525 hi, thanks for the issue and the PR! LGTM
Could you please add a changelog fragment?

@rsicart @Jorge-Rodriguez @bmalynovytch i wounder if there are possible pitfalls?

@Andersson007
Copy link
Collaborator

@bigo8525 thanks! Let's wait for other maintainers' feedback for a couple of days.

@Andersson007
Copy link
Collaborator

@rsicart yep, the statement doesn't seem dangerous to me to, approved.
If you're OK with the change, feel free to merge and backport it to stable-1 and stable-2

@rsicart rsicart merged commit 1dcc5ec into ansible-collections:main Apr 29, 2022
@patchback
Copy link

patchback bot commented Apr 29, 2022

Backport to stable-2: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-2/1dcc5ec086434e707d0ad122ffd9b612187b1132/pr-338

Backported as #339

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

@patchback
Copy link

patchback bot commented Apr 29, 2022

Backport to stable-1: 💔 cherry-picking failed — conflicts found

❌ Failed to cleanly apply 1dcc5ec on top of patchback/backports/stable-1/1dcc5ec086434e707d0ad122ffd9b612187b1132/pr-338

Backporting merged PR #338 into main

  1. Ensure you have a local repo clone of your fork. Unless you cloned it
    from the upstream, this would be your origin remote.
  2. Make sure you have an upstream repo added as a remote too. In these
    instructions you'll refer to it by the name upstream. If you don't
    have it, here's how you can add it:
    $ git remote add upstream https://github.com/ansible-collections/community.mysql.git
  3. Ensure you have the latest copy of upstream and prepare a branch
    that will hold the backported code:
    $ git fetch upstream
    $ git checkout -b patchback/backports/stable-1/1dcc5ec086434e707d0ad122ffd9b612187b1132/pr-338 upstream/stable-1
  4. Now, cherry-pick PR mysql_user: added flush privileges to write dynamic privs into db #338 contents into that branch:
    $ git cherry-pick -x 1dcc5ec086434e707d0ad122ffd9b612187b1132
    If it'll yell at you with something like fatal: Commit 1dcc5ec086434e707d0ad122ffd9b612187b1132 is a merge but no -m option was given., add -m 1 as follows intead:
    $ git cherry-pick -m1 -x 1dcc5ec086434e707d0ad122ffd9b612187b1132
  5. At this point, you'll probably encounter some merge conflicts. You must
    resolve them in to preserve the patch from PR mysql_user: added flush privileges to write dynamic privs into db #338 as close to the
    original as possible.
  6. Push this branch to your fork on GitHub:
    $ git push origin patchback/backports/stable-1/1dcc5ec086434e707d0ad122ffd9b612187b1132/pr-338
  7. Create a PR, ensure that the CI is green. If it's not — update it so that
    the tests and any other checks pass. This is it!
    Now relax and wait for the maintainers to process your pull request
    when they have some cycles to do reviews. Don't worry — they'll tell you if
    any improvements are necessary when the time comes!

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback bot pushed a commit that referenced this pull request Apr 29, 2022
* added flush privileges to write dynamic privs into db
Fixes #120

* added changelog fragment

* Update changelogs/fragments/338-mysql_user_fix_missing_dynamic_privileges.yml

Co-authored-by: Andrew Klychkov <[email protected]>

Co-authored-by: Andrew Klychkov <[email protected]>
(cherry picked from commit 1dcc5ec)
@Andersson007
Copy link
Collaborator

@bigo8525 thanks for the contribution!:) Hope you'll continue to help make the collection better!
@rsicart thanks for reviewing and merging!

Andersson007 pushed a commit that referenced this pull request Apr 29, 2022
…) (#339)

* added flush privileges to write dynamic privs into db
Fixes #120

* added changelog fragment

* Update changelogs/fragments/338-mysql_user_fix_missing_dynamic_privileges.yml

Co-authored-by: Andrew Klychkov <[email protected]>

Co-authored-by: Andrew Klychkov <[email protected]>
(cherry picked from commit 1dcc5ec)

Co-authored-by: bigo8525 <[email protected]>
@Andersson007
Copy link
Collaborator

all backports have been merged
I'm not sure when i can release the 3 patch versions (1., 2., 3.*) containing the fix as it's pretty time consuming and there's only one fix (this one) made since the last release
will see

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

mysql_user: Grant all privileges result in missing dynamic privileges in every second run in MySQL 8
3 participants