Skip to content

fix: remove unneeded flush privileges call for mysql #141 #147

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.

Sign up for GitHub

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

Closed
mariusziemke wants to merge 4 commits into from
Closed

fix: remove unneeded flush privileges call for mysql #141 #147

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e09254a
fix: remove unneeded flush privileges call for mysql
mariusziemke Aug 22, 2023
665d20b
Merge branch 'master' into remove-flush-privileges
mariusziemke Aug 30, 2023
02eba1d
Merge branch 'master' into remove-flush-privileges
mariusziemke Sep 18, 2023
0c1558d
chore: merge master
mariusziemke Jun 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 2 additions & 17 deletions pkg/clients/mysql/mysql.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import (
const (
errNotSupported = "%s not supported by mysql client"
errSetSQLLogBin = "cannot set sql_log_bin = 0"
errFlushPriv = "cannot flush privileges"
)

type mySQLDB struct {
Expand Down Expand Up @@ -145,20 +144,14 @@ type ExecQuery struct {
type ExecOptions struct {
// Binlog defines whether storing binlogs will be disabled before executing the query. Defaults to true
Binlog *bool
// Flush defines whether privileges will be flushed after executing the query. Defaults to true
Flush *bool
}

// ExecWithBinlogAndFlush is a wrapper function for xsql.DB.Exec() that allows the execution of optional queries before and after the provided query
func ExecWithBinlogAndFlush(ctx context.Context, db xsql.DB, query ExecQuery, options ExecOptions) error {
// ExecWrapper is a wrapper function for xsql.DB.Exec() that allows the execution of optional queries before and after the provided query
func ExecWrapper(ctx context.Context, db xsql.DB, query ExecQuery, options ExecOptions) error {
if options.Binlog == nil {
options.Binlog = ptr.To(true)
}

if options.Flush == nil {
options.Flush = ptr.To(true)
}

if !*options.Binlog {
if err := db.Exec(ctx, xsql.Query{
String: "SET sql_log_bin = 0",
Expand All @@ -173,13 +166,5 @@ func ExecWithBinlogAndFlush(ctx context.Context, db xsql.DB, query ExecQuery, op
return errors.Wrap(err, query.ErrorValue)
}

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

return nil
}
5 changes: 2 additions & 3 deletions pkg/controller/mysql/database/reconciler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"github.com/pkg/errors"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/utils/ptr"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller"
Expand Down Expand Up @@ -151,7 +150,7 @@ func (c *external) Create(ctx context.Context, mg resource.Managed) (managed.Ext
binlog := cr.Spec.ForProvider.BinLog
query := "CREATE DATABASE " + mysql.QuoteIdentifier(meta.GetExternalName(cr))

if err := mysql.ExecWithBinlogAndFlush(ctx, c.db, mysql.ExecQuery{Query: query, ErrorValue: errCreateDB}, mysql.ExecOptions{Binlog: binlog, Flush: ptr.To(false)}); err != nil {
if err := mysql.ExecWrapper(ctx, c.db, mysql.ExecQuery{Query: query, ErrorValue: errCreateDB}, mysql.ExecOptions{Binlog: binlog}); err != nil {
return managed.ExternalCreation{}, err
}

Expand All @@ -172,7 +171,7 @@ func (c *external) Delete(ctx context.Context, mg resource.Managed) error {
binlog := cr.Spec.ForProvider.BinLog
query := "DROP DATABASE IF EXISTS " + mysql.QuoteIdentifier(meta.GetExternalName(cr))

if err := mysql.ExecWithBinlogAndFlush(ctx, c.db, mysql.ExecQuery{Query: query, ErrorValue: errDropDB}, mysql.ExecOptions{Binlog: binlog, Flush: ptr.To(false)}); err != nil {
if err := mysql.ExecWrapper(ctx, c.db, mysql.ExecQuery{Query: query, ErrorValue: errDropDB}, mysql.ExecOptions{Binlog: binlog}); err != nil {
return err
}

Expand Down
8 changes: 4 additions & 4 deletions pkg/controller/mysql/grant/reconciler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ func (c *external) Create(ctx context.Context, mg resource.Managed) (managed.Ext
binlog := cr.Spec.ForProvider.BinLog
query := createGrantQuery(privileges, dbname, username, table, grantOption)

if err := mysql.ExecWithBinlogAndFlush(ctx, c.db, mysql.ExecQuery{Query: query, ErrorValue: errCreateGrant}, mysql.ExecOptions{Binlog: binlog}); err != nil {
if err := mysql.ExecWrapper(ctx, c.db, mysql.ExecQuery{Query: query, ErrorValue: errCreateGrant}, mysql.ExecOptions{Binlog: binlog}); err != nil {
return managed.ExternalCreation{}, err
}
return managed.ExternalCreation{}, nil
Expand All @@ -287,7 +287,7 @@ func (c *external) Update(ctx context.Context, mg resource.Managed) (managed.Ext
sort.Strings(toRevoke)
privileges, grantOption := getPrivilegesString(toRevoke)
query := createRevokeQuery(privileges, dbname, username, table, grantOption)
if err := mysql.ExecWithBinlogAndFlush(ctx, c.db,
if err := mysql.ExecWrapper(ctx, c.db,
mysql.ExecQuery{
Query: query, ErrorValue: errRevokeGrant,
}, mysql.ExecOptions{
Expand All @@ -300,7 +300,7 @@ func (c *external) Update(ctx context.Context, mg resource.Managed) (managed.Ext
sort.Strings(toGrant)
privileges, grantOption := getPrivilegesString(toGrant)
query := createGrantQuery(privileges, dbname, username, table, grantOption)
if err := mysql.ExecWithBinlogAndFlush(ctx, c.db,
if err := mysql.ExecWrapper(ctx, c.db,
mysql.ExecQuery{
Query: query, ErrorValue: errCreateGrant,
}, mysql.ExecOptions{
Expand Down Expand Up @@ -374,7 +374,7 @@ func (c *external) Delete(ctx context.Context, mg resource.Managed) error {
privileges, grantOption := getPrivilegesString(cr.Spec.ForProvider.Privileges.ToStringSlice())
query := createRevokeQuery(privileges, dbname, username, table, grantOption)

if err := mysql.ExecWithBinlogAndFlush(ctx, c.db, mysql.ExecQuery{Query: query, ErrorValue: errRevokeGrant}, mysql.ExecOptions{Binlog: binlog}); err != nil {
if err := mysql.ExecWrapper(ctx, c.db, mysql.ExecQuery{Query: query, ErrorValue: errRevokeGrant}, mysql.ExecOptions{Binlog: binlog}); err != nil {
var myErr *mysqldriver.MySQLError
if errors.As(err, &myErr) && myErr.Number == errCodeNoSuchGrant {
// MySQL automatically deletes related grants if the user has been deleted
Expand Down
8 changes: 4 additions & 4 deletions pkg/controller/mysql/user/reconciler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ func (c *external) executeCreateUserQuery(ctx context.Context, username string,
resourceOptions,
)

if err := mysql.ExecWithBinlogAndFlush(ctx, c.db, mysql.ExecQuery{Query: query, ErrorValue: errCreateUser}, mysql.ExecOptions{Binlog: binlog}); err != nil {
if err := mysql.ExecWrapper(ctx, c.db, mysql.ExecQuery{Query: query, ErrorValue: errCreateUser}, mysql.ExecOptions{Binlog: binlog}); err != nil {
return err
}

Expand Down Expand Up @@ -310,7 +310,7 @@ func (c *external) Update(ctx context.Context, mg resource.Managed) (managed.Ext
mysql.QuoteValue(host),
resourceOptions,
)
if err := mysql.ExecWithBinlogAndFlush(ctx, c.db, mysql.ExecQuery{Query: query, ErrorValue: errUpdateUser}, mysql.ExecOptions{Binlog: binlog}); err != nil {
if err := mysql.ExecWrapper(ctx, c.db, mysql.ExecQuery{Query: query, ErrorValue: errUpdateUser}, mysql.ExecOptions{Binlog: binlog}); err != nil {
return managed.ExternalUpdate{}, err
}

Expand Down Expand Up @@ -338,7 +338,7 @@ func (c *external) UpdatePassword(ctx context.Context, cr *v1alpha1.User, userna
if pwchanged {
binlog := cr.Spec.ForProvider.BinLog
query := fmt.Sprintf("ALTER USER %s@%s IDENTIFIED BY %s", mysql.QuoteValue(username), mysql.QuoteValue(host), mysql.QuoteValue(pw))
if err := mysql.ExecWithBinlogAndFlush(ctx, c.db, mysql.ExecQuery{Query: query, ErrorValue: errUpdateUser}, mysql.ExecOptions{Binlog: binlog}); err != nil {
if err := mysql.ExecWrapper(ctx, c.db, mysql.ExecQuery{Query: query, ErrorValue: errUpdateUser}, mysql.ExecOptions{Binlog: binlog}); err != nil {
return managed.ConnectionDetails{}, err
}

Expand All @@ -360,7 +360,7 @@ func (c *external) Delete(ctx context.Context, mg resource.Managed) error {

binlog := cr.Spec.ForProvider.BinLog
query := fmt.Sprintf("DROP USER IF EXISTS %s@%s", mysql.QuoteValue(username), mysql.QuoteValue(host))
if err := mysql.ExecWithBinlogAndFlush(ctx, c.db, mysql.ExecQuery{Query: query, ErrorValue: errDropUser}, mysql.ExecOptions{Binlog: binlog}); err != nil {
if err := mysql.ExecWrapper(ctx, c.db, mysql.ExecQuery{Query: query, ErrorValue: errDropUser}, mysql.ExecOptions{Binlog: binlog}); err != nil {
return err
}

Expand Down