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.

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

[mysql] FLUSH PRIVILEGES is only required after making manual edits to tables such as mysql.user, not after CREATE, ALTER or DROP USER #141

Open
alereca opened this issue Apr 17, 2023 · 4 comments · May be fixed by #147
Labels
enhancement New feature or request

Comments

@alereca
Copy link
Contributor

alereca commented Apr 17, 2023

What problem are you facing?

Why every time a CREATE, ALTER or DROP USER statement is executed, FLUSH PRIVILEGES is called after? It's also present after GRANT and REVOKE statements.

According to mysql documentation executing FLUSH PRIVILEGES is only required If you modify the grant tables directly using statements such as INSERT, UPDATE, or DELETE (which is not recommended), the changes have no effect on privilege checking until you either tell the server to reload the tables or restart it. (https://dev.mysql.com/doc/refman/8.0/en/privilege-changes.html). Other references:

How could Crossplane help solve your problem?

Remove FLUSH PRIVILEGES statements from user and grant controllers, for example:

if err := c.db.Exec(ctx, xsql.Query{
		String: "FLUSH PRIVILEGES",
	}); err != nil {
		return managed.ExternalCreation{}, errors.Wrap(err, errFlushPriv)
	}

https://github.com/crossplane-contrib/provider-sql/blob/master/pkg/controller/mysql/user/reconciler.go#L271

I would like to work in a pr if this is considered as desirable

@alereca alereca added the enhancement New feature or request label Apr 17, 2023
@mariusziemke
Copy link

We are using a percona xtradb cluster and the FLUSH PRIVILEGES call brought down the whole cluster due to some bugs in the galera replication.. We created a version of the operator without the FLUSH PRIVILEGES queries and everything worked as expected.

Not sure why it was implemented, but maybe we could add a configuration flag which allows disabling the queries.

@alereca I could help with a PR :-) Otherwise we will probably "maintain" a fork.

@alereca
Copy link
Contributor Author

alereca commented Jul 27, 2023

@mariusziemke it's great to know that the provider can work without the flush statements. Bearing that in mind and what I mentioned from MySQL documentation I would stick to removing them, as they bring no value from my view point.

If you like to work on a PR, taking advantage that you already implemented it in your fork, it will be highly appreciated.

@Duologic
Copy link
Member

Duologic commented Aug 9, 2023

I have no problem with this, looking forward to a PR.

@mariusziemke
Copy link

Awesome :-) I will talk to my team and we will figure out when we can fit it into our schedule.

@mariusziemke mariusziemke linked a pull request Aug 22, 2023 that will close this issue
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants