crossplane-contrib / provider-sql

An SQL provider for @crossplane
https://crossplane.io
Apache License 2.0
100 stars 57 forks source link

[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 1 year ago

alereca commented 1 year ago

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

mariusziemke commented 11 months ago

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 commented 11 months ago

@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 commented 11 months ago

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

mariusziemke commented 11 months ago

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