From 1f6725c71ab1ba00bd8bf765b4701fd7220a7d93 Mon Sep 17 00:00:00 2001 From: Alejandro Recalde Date: Thu, 16 Feb 2023 16:29:36 -0300 Subject: [PATCH 1/8] Use auth plugin to create users and set its password if plugin requires it Signed-off-by: Alejandro Recalde --- apis/mysql/v1alpha1/user_types.go | 5 +++++ pkg/controller/mysql/user/reconciler.go | 11 +++++++++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/apis/mysql/v1alpha1/user_types.go b/apis/mysql/v1alpha1/user_types.go index f34af6f4..e04cc00d 100644 --- a/apis/mysql/v1alpha1/user_types.go +++ b/apis/mysql/v1alpha1/user_types.go @@ -64,6 +64,11 @@ type ResourceOptions struct { // MaxUserConnections sets The number of simultaneous connections to the server by an account // +optional MaxUserConnections *int `json:"maxUserConnections,omitempty"` + + // AuthPlugin sets the mysql authentication plugin, defaults to mysql_native_password + // +optional + // +kubebuilder:validation:Pattern:=^([a-z]+_)+[a-z]+$ + AuthPlugin *string `json:"authPlugin,omitempty" default:"mysql_native_password"` } // A UserObservation represents the observed state of a MySQL user. diff --git a/pkg/controller/mysql/user/reconciler.go b/pkg/controller/mysql/user/reconciler.go index 8b3fff2d..0f0cb20c 100644 --- a/pkg/controller/mysql/user/reconciler.go +++ b/pkg/controller/mysql/user/reconciler.go @@ -240,6 +240,7 @@ func (c *external) Create(ctx context.Context, mg resource.Managed) (managed.Ext cr.SetConditions(xpv1.Creating()) username, host := mysql.SplitUserHost(meta.GetExternalName(cr)) + plugin := *cr.Spec.ForProvider.ResourceOptions.AuthPlugin pw, _, err := c.getPassword(ctx, cr) if err != nil { return managed.ExternalCreation{}, err @@ -257,11 +258,17 @@ func (c *external) Create(ctx context.Context, mg resource.Managed) (managed.Ext resourceOptions = fmt.Sprintf(" WITH %s", strings.Join(ro, " ")) } + password_section := "" + if plugin == "mysql_native_password" || plugin == "caching_sha2_password" { + password_section = fmt.Sprintf("AS %s", mysql.QuoteValue(pw)) + } + query := fmt.Sprintf( - "CREATE USER %s@%s IDENTIFIED BY %s%s", + "CREATE USER %s@%s IDENTIFIED WITH %s%s%s", mysql.QuoteValue(username), mysql.QuoteValue(host), - mysql.QuoteValue(pw), + mysql.QuoteValue(plugin), + password_section, resourceOptions, ) if err := c.db.Exec(ctx, xsql.Query{ From bc1e3ea088cef798eed1634bc59eb09bc1870bed Mon Sep 17 00:00:00 2001 From: Alejandro Recalde Date: Wed, 22 Feb 2023 16:15:47 -0300 Subject: [PATCH 2/8] Define default auth plugin and use password values in order to keep previous behavior working Signed-off-by: Alejandro Recalde --- apis/mysql/v1alpha1/user_types.go | 14 +- apis/mysql/v1alpha1/zz_generated.deepcopy.go | 10 ++ .../crds/mysql.sql.crossplane.io_users.yaml | 9 ++ pkg/controller/mysql/user/reconciler.go | 125 +++++++++++++----- pkg/controller/mysql/user/reconciler_test.go | 56 ++++++++ 5 files changed, 174 insertions(+), 40 deletions(-) diff --git a/apis/mysql/v1alpha1/user_types.go b/apis/mysql/v1alpha1/user_types.go index e04cc00d..e22425f2 100644 --- a/apis/mysql/v1alpha1/user_types.go +++ b/apis/mysql/v1alpha1/user_types.go @@ -45,6 +45,15 @@ type UserParameters struct { // See https://dev.mysql.com/doc/refman/8.0/en/user-resources.html // +optional ResourceOptions *ResourceOptions `json:"resourceOptions,omitempty"` + + // AuthPlugin sets the mysql authentication plugin, defaults to mysql_native_password + // +optional + // +kubebuilder:validation:Pattern:=^([a-z]+_)+[a-z]+$ + AuthPlugin *string `json:"authPlugin,omitempty" default:"mysql_native_password"` + + // UsePassword indicate whether the provided AuthPlugin requires setting a password, defaults to true + // +optional + UsePassword *bool `json:"usePassword,omitempty" default:"true"` } // ResourceOptions define the account specific resource limits. @@ -64,11 +73,6 @@ type ResourceOptions struct { // MaxUserConnections sets The number of simultaneous connections to the server by an account // +optional MaxUserConnections *int `json:"maxUserConnections,omitempty"` - - // AuthPlugin sets the mysql authentication plugin, defaults to mysql_native_password - // +optional - // +kubebuilder:validation:Pattern:=^([a-z]+_)+[a-z]+$ - AuthPlugin *string `json:"authPlugin,omitempty" default:"mysql_native_password"` } // A UserObservation represents the observed state of a MySQL user. diff --git a/apis/mysql/v1alpha1/zz_generated.deepcopy.go b/apis/mysql/v1alpha1/zz_generated.deepcopy.go index 1ef6357d..63820c64 100644 --- a/apis/mysql/v1alpha1/zz_generated.deepcopy.go +++ b/apis/mysql/v1alpha1/zz_generated.deepcopy.go @@ -584,6 +584,16 @@ func (in *UserParameters) DeepCopyInto(out *UserParameters) { *out = new(ResourceOptions) (*in).DeepCopyInto(*out) } + if in.AuthPlugin != nil { + in, out := &in.AuthPlugin, &out.AuthPlugin + *out = new(string) + **out = **in + } + if in.UsePassword != nil { + in, out := &in.UsePassword, &out.UsePassword + *out = new(bool) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new UserParameters. diff --git a/package/crds/mysql.sql.crossplane.io_users.yaml b/package/crds/mysql.sql.crossplane.io_users.yaml index 06bfd10c..ae561238 100644 --- a/package/crds/mysql.sql.crossplane.io_users.yaml +++ b/package/crds/mysql.sql.crossplane.io_users.yaml @@ -62,6 +62,11 @@ spec: description: UserParameters define the desired state of a MySQL user instance. properties: + authPlugin: + description: AuthPlugin sets the mysql authentication plugin, + defaults to mysql_native_password + pattern: ^([a-z]+_)+[a-z]+$ + type: string passwordSecretRef: description: PasswordSecretRef references the secret that contains the password used for this user. If no reference is given, a @@ -102,6 +107,10 @@ spec: connections to the server by an account type: integer type: object + usePassword: + description: UsePassword indicate whether the provided AuthPlugin + requires setting a password, defaults to true + type: boolean type: object providerConfigRef: default: diff --git a/pkg/controller/mysql/user/reconciler.go b/pkg/controller/mysql/user/reconciler.go index 0f0cb20c..17201d62 100644 --- a/pkg/controller/mysql/user/reconciler.go +++ b/pkg/controller/mysql/user/reconciler.go @@ -240,55 +240,70 @@ func (c *external) Create(ctx context.Context, mg resource.Managed) (managed.Ext cr.SetConditions(xpv1.Creating()) username, host := mysql.SplitUserHost(meta.GetExternalName(cr)) - plugin := *cr.Spec.ForProvider.ResourceOptions.AuthPlugin - pw, _, err := c.getPassword(ctx, cr) - if err != nil { - return managed.ExternalCreation{}, err + plugin := defaultAuthPlugin(cr.Spec.ForProvider.AuthPlugin) + + var resourceOptions string + ro := resourceOptionsToClauses(cr.Spec.ForProvider.ResourceOptions) + if len(ro) != 0 { + resourceOptions = fmt.Sprintf(" WITH %s", strings.Join(ro, " ")) } - if pw == "" { - pw, err = password.Generate() + + if checkUsePassword(cr.Spec.ForProvider.UsePassword) { + pw, _, err := c.getPassword(ctx, cr) if err != nil { return managed.ExternalCreation{}, err } + if pw == "" { + pw, err = password.Generate() + if err != nil { + return managed.ExternalCreation{}, err + } + } + + if err := c.executeCreateUserQuery(ctx, username, host, plugin, resourceOptions, &pw); err != nil { + return managed.ExternalCreation{}, err + } + + return managed.ExternalCreation{ + ConnectionDetails: c.db.GetConnectionDetails(username, pw), + }, nil } - var resourceOptions string - ro := resourceOptionsToClauses(cr.Spec.ForProvider.ResourceOptions) - if len(ro) != 0 { - resourceOptions = fmt.Sprintf(" WITH %s", strings.Join(ro, " ")) + if err := c.executeCreateUserQuery(ctx, username, host, plugin, resourceOptions, nil); err != nil { + return managed.ExternalCreation{}, err } - password_section := "" - if plugin == "mysql_native_password" || plugin == "caching_sha2_password" { - password_section = fmt.Sprintf("AS %s", mysql.QuoteValue(pw)) + return managed.ExternalCreation{}, nil +} + +func (c *external) executeCreateUserQuery(ctx context.Context, username string, host string, plugin string, resourceOptions string, pw *string) error { + passwordSection := "" + if pw != nil { + passwordSection = fmt.Sprintf(" AS %s", mysql.QuoteValue(*pw)) } query := fmt.Sprintf( "CREATE USER %s@%s IDENTIFIED WITH %s%s%s", mysql.QuoteValue(username), mysql.QuoteValue(host), - mysql.QuoteValue(plugin), - password_section, + plugin, + passwordSection, resourceOptions, ) + if err := c.db.Exec(ctx, xsql.Query{ String: query, }); err != nil { - return managed.ExternalCreation{}, errors.Wrap(err, errCreateUser) + return errors.Wrap(err, errCreateUser) } + if err := c.db.Exec(ctx, xsql.Query{ String: "FLUSH PRIVILEGES", }); err != nil { - return managed.ExternalCreation{}, errors.Wrap(err, errFlushPriv) - } - - if len(ro) != 0 { - cr.Status.AtProvider.ResourceOptionsAsClauses = ro + return errors.Wrap(err, errFlushPriv) } - return managed.ExternalCreation{ - ConnectionDetails: c.db.GetConnectionDetails(username, pw), - }, nil + return nil } func (c *external) Update(ctx context.Context, mg resource.Managed) (managed.ExternalUpdate, error) { @@ -298,10 +313,6 @@ func (c *external) Update(ctx context.Context, mg resource.Managed) (managed.Ext } username, host := mysql.SplitUserHost(meta.GetExternalName(cr)) - pw, pwchanged, err := c.getPassword(ctx, cr) - if err != nil { - return managed.ExternalUpdate{}, err - } ro := resourceOptionsToClauses(cr.Spec.ForProvider.ResourceOptions) rochanged, err := changedResourceOptions(cr.Status.AtProvider.ResourceOptionsAsClauses, ro) @@ -332,24 +343,68 @@ func (c *external) Update(ctx context.Context, mg resource.Managed) (managed.Ext cr.Status.AtProvider.ResourceOptionsAsClauses = ro } + if checkUsePassword(cr.Spec.ForProvider.UsePassword) { + connectionDetails, err := c.UpdatePassword(ctx, cr, username, host) + + if err != nil { + return managed.ExternalUpdate{}, err + } + + if len(connectionDetails) > 0 { + return managed.ExternalUpdate{ConnectionDetails: connectionDetails}, nil + } + } + + return managed.ExternalUpdate{}, nil +} + +func checkUsePassword(usePassword *bool) bool { + if usePassword == nil { + return true + } + + return *usePassword +} + +func (c *external) UpdatePassword(ctx context.Context, cr *v1alpha1.User, username, host string) (managed.ConnectionDetails, error) { + pw, pwchanged, err := c.getPassword(ctx, cr) + if err != nil { + return managed.ConnectionDetails{}, err + } + if pwchanged { - query := fmt.Sprintf("ALTER USER %s@%s IDENTIFIED BY %s", mysql.QuoteValue(username), mysql.QuoteValue(host), mysql.QuoteValue(pw)) + plugin := defaultAuthPlugin(cr.Spec.ForProvider.AuthPlugin) + query := fmt.Sprintf("ALTER USER %s@%s IDENTIFIED WITH %s AS %s", + mysql.QuoteValue(username), + mysql.QuoteValue(host), + plugin, + mysql.QuoteValue(pw), + ) + if err := c.db.Exec(ctx, xsql.Query{ String: query, }); err != nil { - return managed.ExternalUpdate{}, errors.Wrap(err, errUpdateUser) + return managed.ConnectionDetails{}, errors.Wrap(err, errUpdateUser) } + if err := c.db.Exec(ctx, xsql.Query{ String: "FLUSH PRIVILEGES", }); err != nil { - return managed.ExternalUpdate{}, errors.Wrap(err, errFlushPriv) + return managed.ConnectionDetails{}, errors.Wrap(err, errFlushPriv) } - return managed.ExternalUpdate{ - ConnectionDetails: c.db.GetConnectionDetails(username, pw), - }, nil + return c.db.GetConnectionDetails(username, pw), nil } - return managed.ExternalUpdate{}, nil + + return managed.ConnectionDetails{}, nil +} + +func defaultAuthPlugin(authPlugin *string) string { + if authPlugin == nil { + return "mysql_native_password" + } + + return *authPlugin } func (c *external) Delete(ctx context.Context, mg resource.Managed) error { diff --git a/pkg/controller/mysql/user/reconciler_test.go b/pkg/controller/mysql/user/reconciler_test.go index da0374be..99c5c1e8 100644 --- a/pkg/controller/mysql/user/reconciler_test.go +++ b/pkg/controller/mysql/user/reconciler_test.go @@ -27,6 +27,7 @@ import ( "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/pointer" "sigs.k8s.io/controller-runtime/pkg/client" xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1" @@ -494,6 +495,34 @@ func TestCreate(t *testing.T) { }, }, }, + "UserWithAnAuthPluginThatNotRequiresPassword": { + reason: "A user that uses an authentication plugin that does not require password should not receive connection details and no error should happen", + comparePw: true, + fields: fields{ + db: &mockDB{ + MockExec: func(ctx context.Context, q xsql.Query) error { return nil }, + }, + }, + args: args{ + mg: &v1alpha1.User{ + ObjectMeta: v1.ObjectMeta{ + Annotations: map[string]string{ + meta.AnnotationKeyExternalName: "example", + }, + }, + Spec: v1alpha1.UserSpec{ + ForProvider: v1alpha1.UserParameters{ + AuthPlugin: pointer.StringPtr("authentication_ldap_simple"), + UsePassword: pointer.BoolPtr(false), + }, + }, + }, + }, + want: want{ + err: nil, + c: managed.ExternalCreation{}, + }, + }, } for name, tc := range cases { @@ -768,6 +797,33 @@ func TestUpdate(t *testing.T) { err: nil, }, }, + "UserWithAnAuthPluginThatNotRequiresPassword": { + reason: "A user that uses an authentication plugin that does not require password should not receive connection details and no error should happen", + fields: fields{ + db: &mockDB{ + MockExec: func(ctx context.Context, q xsql.Query) error { return nil }, + }, + }, + args: args{ + mg: &v1alpha1.User{ + ObjectMeta: v1.ObjectMeta{ + Annotations: map[string]string{ + meta.AnnotationKeyExternalName: "example", + }, + }, + Spec: v1alpha1.UserSpec{ + ForProvider: v1alpha1.UserParameters{ + AuthPlugin: pointer.StringPtr("authentication_ldap_simple"), + UsePassword: pointer.BoolPtr(false), + }, + }, + }, + }, + want: want{ + err: nil, + c: managed.ExternalUpdate{}, + }, + }, } for name, tc := range cases { From 0ab2e2285557741a02b1834822623a9e335a17a5 Mon Sep 17 00:00:00 2001 From: Alejandro Recalde Date: Wed, 22 Feb 2023 18:32:58 -0300 Subject: [PATCH 3/8] Add missing ResourceOptionsAsClauses cr assignation in user create Signed-off-by: Alejandro Recalde --- pkg/controller/mysql/user/reconciler.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/controller/mysql/user/reconciler.go b/pkg/controller/mysql/user/reconciler.go index 17201d62..47c7cde8 100644 --- a/pkg/controller/mysql/user/reconciler.go +++ b/pkg/controller/mysql/user/reconciler.go @@ -246,6 +246,7 @@ func (c *external) Create(ctx context.Context, mg resource.Managed) (managed.Ext ro := resourceOptionsToClauses(cr.Spec.ForProvider.ResourceOptions) if len(ro) != 0 { resourceOptions = fmt.Sprintf(" WITH %s", strings.Join(ro, " ")) + cr.Status.AtProvider.ResourceOptionsAsClauses = ro } if checkUsePassword(cr.Spec.ForProvider.UsePassword) { From c9e65f8a3f32c62e05863bbe28c85bc1a8841e4e Mon Sep 17 00:00:00 2001 From: Alejandro Recalde Date: Fri, 24 Feb 2023 17:13:55 -0300 Subject: [PATCH 4/8] Replace as with by in create and alter user queries Signed-off-by: Alejandro Recalde --- pkg/controller/mysql/user/reconciler.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/controller/mysql/user/reconciler.go b/pkg/controller/mysql/user/reconciler.go index 47c7cde8..1a45c1a4 100644 --- a/pkg/controller/mysql/user/reconciler.go +++ b/pkg/controller/mysql/user/reconciler.go @@ -280,7 +280,7 @@ func (c *external) Create(ctx context.Context, mg resource.Managed) (managed.Ext func (c *external) executeCreateUserQuery(ctx context.Context, username string, host string, plugin string, resourceOptions string, pw *string) error { passwordSection := "" if pw != nil { - passwordSection = fmt.Sprintf(" AS %s", mysql.QuoteValue(*pw)) + passwordSection = fmt.Sprintf(" BY %s", mysql.QuoteValue(*pw)) } query := fmt.Sprintf( @@ -375,7 +375,7 @@ func (c *external) UpdatePassword(ctx context.Context, cr *v1alpha1.User, userna if pwchanged { plugin := defaultAuthPlugin(cr.Spec.ForProvider.AuthPlugin) - query := fmt.Sprintf("ALTER USER %s@%s IDENTIFIED WITH %s AS %s", + query := fmt.Sprintf("ALTER USER %s@%s IDENTIFIED WITH %s BY %s", mysql.QuoteValue(username), mysql.QuoteValue(host), plugin, From 2c6ba733b8e0347c4e3bb8c4818ee4ff6925dedb Mon Sep 17 00:00:00 2001 From: Alejandro Recalde Date: Fri, 24 Feb 2023 22:39:10 -0300 Subject: [PATCH 5/8] Set AuthPlugin as an user observation to be included in user status Signed-off-by: Alejandro Recalde --- apis/mysql/v1alpha1/user_types.go | 3 +++ apis/mysql/v1alpha1/zz_generated.deepcopy.go | 5 +++++ package/crds/mysql.sql.crossplane.io_users.yaml | 4 ++++ 3 files changed, 12 insertions(+) diff --git a/apis/mysql/v1alpha1/user_types.go b/apis/mysql/v1alpha1/user_types.go index e22425f2..f554b764 100644 --- a/apis/mysql/v1alpha1/user_types.go +++ b/apis/mysql/v1alpha1/user_types.go @@ -79,6 +79,9 @@ type ResourceOptions struct { type UserObservation struct { // ResourceOptionsAsClauses represents the applied resource options ResourceOptionsAsClauses []string `json:"resourceOptionsAsClauses,omitempty"` + + // AuthPlugin represents the applied mysql authentication plugin + AuthPlugin *string `json:"authPlugin,omitempty"` } // +kubebuilder:object:root=true diff --git a/apis/mysql/v1alpha1/zz_generated.deepcopy.go b/apis/mysql/v1alpha1/zz_generated.deepcopy.go index 63820c64..e095088c 100644 --- a/apis/mysql/v1alpha1/zz_generated.deepcopy.go +++ b/apis/mysql/v1alpha1/zz_generated.deepcopy.go @@ -559,6 +559,11 @@ func (in *UserObservation) DeepCopyInto(out *UserObservation) { *out = make([]string, len(*in)) copy(*out, *in) } + if in.AuthPlugin != nil { + in, out := &in.AuthPlugin, &out.AuthPlugin + *out = new(string) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new UserObservation. diff --git a/package/crds/mysql.sql.crossplane.io_users.yaml b/package/crds/mysql.sql.crossplane.io_users.yaml index ae561238..98a0b74a 100644 --- a/package/crds/mysql.sql.crossplane.io_users.yaml +++ b/package/crds/mysql.sql.crossplane.io_users.yaml @@ -290,6 +290,10 @@ spec: description: A UserObservation represents the observed state of a MySQL user. properties: + authPlugin: + description: AuthPlugin represents the applied mysql authentication + plugin + type: string resourceOptionsAsClauses: description: ResourceOptionsAsClauses represents the applied resource options From 3e710a694ba2581f59bbb14d1ddec33987675012 Mon Sep 17 00:00:00 2001 From: Alejandro Recalde Date: Sat, 25 Feb 2023 11:36:35 -0300 Subject: [PATCH 6/8] Include AuthPlugin in observed fields and apply alter query only of user spec field diverged Signed-off-by: Alejandro Recalde --- pkg/controller/mysql/user/reconciler.go | 163 ++++++++++++------- pkg/controller/mysql/user/reconciler_test.go | 136 +++++++++++++++- 2 files changed, 237 insertions(+), 62 deletions(-) diff --git a/pkg/controller/mysql/user/reconciler.go b/pkg/controller/mysql/user/reconciler.go index 1a45c1a4..d7550777 100644 --- a/pkg/controller/mysql/user/reconciler.go +++ b/pkg/controller/mysql/user/reconciler.go @@ -187,6 +187,7 @@ func (c *external) Observe(ctx context.Context, mg resource.Managed) (managed.Ex username, host := mysql.SplitUserHost(meta.GetExternalName(cr)) observed := &v1alpha1.UserParameters{ + AuthPlugin: new(string), ResourceOptions: &v1alpha1.ResourceOptions{}, } @@ -195,6 +196,7 @@ func (c *external) Observe(ctx context.Context, mg resource.Managed) (managed.Ex "max_updates, " + "max_connections, " + "max_user_connections " + + "plugin" + "FROM mysql.user WHERE User = ? AND Host = ?" err := c.db.Scan(ctx, xsql.Query{ @@ -208,6 +210,7 @@ func (c *external) Observe(ctx context.Context, mg resource.Managed) (managed.Ex &observed.ResourceOptions.MaxUpdatesPerHour, &observed.ResourceOptions.MaxConnectionsPerHour, &observed.ResourceOptions.MaxUserConnections, + &observed.AuthPlugin, ) if xsql.IsNoRows(err) { return managed.ExternalObservation{ResourceExists: false}, nil @@ -222,6 +225,7 @@ func (c *external) Observe(ctx context.Context, mg resource.Managed) (managed.Ex } cr.Status.AtProvider.ResourceOptionsAsClauses = resourceOptionsToClauses(observed.ResourceOptions) + cr.Status.AtProvider.AuthPlugin = observed.AuthPlugin cr.SetConditions(xpv1.Available()) @@ -242,14 +246,12 @@ func (c *external) Create(ctx context.Context, mg resource.Managed) (managed.Ext username, host := mysql.SplitUserHost(meta.GetExternalName(cr)) plugin := defaultAuthPlugin(cr.Spec.ForProvider.AuthPlugin) - var resourceOptions string ro := resourceOptionsToClauses(cr.Spec.ForProvider.ResourceOptions) if len(ro) != 0 { - resourceOptions = fmt.Sprintf(" WITH %s", strings.Join(ro, " ")) cr.Status.AtProvider.ResourceOptionsAsClauses = ro } - if checkUsePassword(cr.Spec.ForProvider.UsePassword) { + if checkUsePassword(cr) { pw, _, err := c.getPassword(ctx, cr) if err != nil { return managed.ExternalCreation{}, err @@ -261,7 +263,7 @@ func (c *external) Create(ctx context.Context, mg resource.Managed) (managed.Ext } } - if err := c.executeCreateUserQuery(ctx, username, host, plugin, resourceOptions, &pw); err != nil { + if err := c.executeCreateUserQuery(ctx, username, host, plugin, ro, &pw); err != nil { return managed.ExternalCreation{}, err } @@ -270,19 +272,24 @@ func (c *external) Create(ctx context.Context, mg resource.Managed) (managed.Ext }, nil } - if err := c.executeCreateUserQuery(ctx, username, host, plugin, resourceOptions, nil); err != nil { + if err := c.executeCreateUserQuery(ctx, username, host, plugin, ro, nil); err != nil { return managed.ExternalCreation{}, err } return managed.ExternalCreation{}, nil } -func (c *external) executeCreateUserQuery(ctx context.Context, username string, host string, plugin string, resourceOptions string, pw *string) error { +func (c *external) executeCreateUserQuery(ctx context.Context, username string, host string, plugin string, resourceOptionsClauses []string, pw *string) error { passwordSection := "" if pw != nil { passwordSection = fmt.Sprintf(" BY %s", mysql.QuoteValue(*pw)) } + resourceOptions := "" + if len(resourceOptionsClauses) != 0 { + resourceOptions = fmt.Sprintf(" WITH %s", strings.Join(resourceOptionsClauses, " ")) + } + query := fmt.Sprintf( "CREATE USER %s@%s IDENTIFIED WITH %s%s%s", mysql.QuoteValue(username), @@ -314,90 +321,124 @@ func (c *external) Update(ctx context.Context, mg resource.Managed) (managed.Ext } username, host := mysql.SplitUserHost(meta.GetExternalName(cr)) + plugin := defaultAuthPlugin(cr.Spec.ForProvider.AuthPlugin) - ro := resourceOptionsToClauses(cr.Spec.ForProvider.ResourceOptions) - rochanged, err := changedResourceOptions(cr.Status.AtProvider.ResourceOptionsAsClauses, ro) + roToAlter, err := getResourceOptionsToAlter(cr) if err != nil { - return managed.ExternalUpdate{}, errors.Wrap(err, errUpdateUser) + return managed.ExternalUpdate{}, err } - if len(rochanged) > 0 { - resourceOptions := fmt.Sprintf("WITH %s", strings.Join(ro, " ")) + password, passwordChanged, err := getPassword(ctx, cr, c) + if err != nil { + return managed.ExternalUpdate{}, err + } - query := fmt.Sprintf( - "ALTER USER %s@%s %s", - mysql.QuoteValue(username), - mysql.QuoteValue(host), - resourceOptions, - ) - if err := c.db.Exec(ctx, xsql.Query{ - String: query, - }); err != nil { - return managed.ExternalUpdate{}, errors.Wrap(err, errUpdateUser) - } - if err := c.db.Exec(ctx, xsql.Query{ - String: "FLUSH PRIVILEGES", - }); err != nil { - return managed.ExternalUpdate{}, errors.Wrap(err, errFlushPriv) + return c.applyAlterUserIfSomeFieldChanged(ctx, cr, passwordChanged, roToAlter, username, host, plugin, password) +} + +func (c *external) applyAlterUserIfSomeFieldChanged(ctx context.Context, cr *v1alpha1.User, passwordChanged bool, roToAlter []string, username string, host string, plugin string, password string) (managed.ExternalUpdate, error) { + if (checkUsePassword(cr) && passwordChanged) || checkAuthPluginChanged(cr) || checkResourceOptionsChanged(roToAlter) { + if err := c.executeAlterUserQuery(ctx, username, host, plugin, roToAlter, password); err != nil { + return managed.ExternalUpdate{}, err } + } - cr.Status.AtProvider.ResourceOptionsAsClauses = ro + if checkUsePassword(cr) && passwordChanged { + return managed.ExternalUpdate{ConnectionDetails: c.db.GetConnectionDetails(username, password)}, nil } - if checkUsePassword(cr.Spec.ForProvider.UsePassword) { - connectionDetails, err := c.UpdatePassword(ctx, cr, username, host) + return managed.ExternalUpdate{}, nil +} +func getPassword(ctx context.Context, cr *v1alpha1.User, c *external) (string, bool, error) { + password := "" + passwordChanged := false + if checkUsePassword(cr) { + pw, pwdChanged, err := c.getPassword(ctx, cr) if err != nil { - return managed.ExternalUpdate{}, err + return pw, pwdChanged, err } - if len(connectionDetails) > 0 { - return managed.ExternalUpdate{ConnectionDetails: connectionDetails}, nil - } + password = pw + passwordChanged = pwdChanged } - return managed.ExternalUpdate{}, nil + return password, passwordChanged, nil } -func checkUsePassword(usePassword *bool) bool { - if usePassword == nil { +func getResourceOptionsToAlter(cr *v1alpha1.User) ([]string, error) { + roToAlter := []string{} + + ro := resourceOptionsToClauses(cr.Spec.ForProvider.ResourceOptions) + roChanged, err := changedResourceOptions(cr.Status.AtProvider.ResourceOptionsAsClauses, ro) + if err != nil { + return roToAlter, errors.Wrap(err, errUpdateUser) + } + + if len(roChanged) > 0 { + cr.Status.AtProvider.ResourceOptionsAsClauses = ro + roToAlter = ro + } + + return roToAlter, nil +} + +func checkUsePassword(cr *v1alpha1.User) bool { + if cr.Spec.ForProvider.UsePassword == nil { return true } - return *usePassword + return *cr.Spec.ForProvider.UsePassword } -func (c *external) UpdatePassword(ctx context.Context, cr *v1alpha1.User, username, host string) (managed.ConnectionDetails, error) { - pw, pwchanged, err := c.getPassword(ctx, cr) - if err != nil { - return managed.ConnectionDetails{}, err +func checkResourceOptionsChanged(roToAlter []string) bool { + return len(roToAlter) > 0 +} + +func checkAuthPluginChanged(cr *v1alpha1.User) bool { + if cr.Status.AtProvider.AuthPlugin == nil { + return true } - if pwchanged { - plugin := defaultAuthPlugin(cr.Spec.ForProvider.AuthPlugin) - query := fmt.Sprintf("ALTER USER %s@%s IDENTIFIED WITH %s BY %s", - mysql.QuoteValue(username), - mysql.QuoteValue(host), - plugin, - mysql.QuoteValue(pw), - ) + if *cr.Status.AtProvider.AuthPlugin != defaultAuthPlugin(cr.Spec.ForProvider.AuthPlugin) { + return true + } - if err := c.db.Exec(ctx, xsql.Query{ - String: query, - }); err != nil { - return managed.ConnectionDetails{}, errors.Wrap(err, errUpdateUser) - } + return false +} - if err := c.db.Exec(ctx, xsql.Query{ - String: "FLUSH PRIVILEGES", - }); err != nil { - return managed.ConnectionDetails{}, errors.Wrap(err, errFlushPriv) - } +func (c *external) executeAlterUserQuery(ctx context.Context, username string, host string, plugin string, resourceOptionsClauses []string, pw string) error { + passwordSection := "" + if pw != "" { + passwordSection = fmt.Sprintf(" BY %s", mysql.QuoteValue(pw)) + } - return c.db.GetConnectionDetails(username, pw), nil + resourceOptions := "" + if len(resourceOptionsClauses) != 0 { + resourceOptions = fmt.Sprintf(" WITH %s", strings.Join(resourceOptionsClauses, " ")) } - return managed.ConnectionDetails{}, nil + query := fmt.Sprintf("ALTER USER %s@%s IDENTIFIED WITH %s%s%s", + mysql.QuoteValue(username), + mysql.QuoteValue(host), + plugin, + passwordSection, + resourceOptions, + ) + + if err := c.db.Exec(ctx, xsql.Query{ + String: query, + }); err != nil { + return errors.Wrap(err, errUpdateUser) + } + + if err := c.db.Exec(ctx, xsql.Query{ + String: "FLUSH PRIVILEGES", + }); err != nil { + return errors.Wrap(err, errFlushPriv) + } + + return nil } func defaultAuthPlugin(authPlugin *string) string { diff --git a/pkg/controller/mysql/user/reconciler_test.go b/pkg/controller/mysql/user/reconciler_test.go index 99c5c1e8..6ff29d38 100644 --- a/pkg/controller/mysql/user/reconciler_test.go +++ b/pkg/controller/mysql/user/reconciler_test.go @@ -603,6 +603,11 @@ func TestUpdate(t *testing.T) { }, }, }, + Status: v1alpha1.UserStatus{ + AtProvider: v1alpha1.UserObservation{ + AuthPlugin: pointer.StringPtr(defaultAuthPlugin(nil)), + }, + }, }, kube: &test.MockClient{ MockGet: func(_ context.Context, key client.ObjectKey, obj client.Object) error { @@ -633,6 +638,11 @@ func TestUpdate(t *testing.T) { meta.AnnotationKeyExternalName: "example", }, }, + Status: v1alpha1.UserStatus{ + AtProvider: v1alpha1.UserObservation{ + AuthPlugin: pointer.StringPtr(defaultAuthPlugin(nil)), + }, + }, }, }, want: want{ @@ -662,6 +672,11 @@ func TestUpdate(t *testing.T) { }, }, }, + Status: v1alpha1.UserStatus{ + AtProvider: v1alpha1.UserObservation{ + AuthPlugin: pointer.StringPtr(defaultAuthPlugin(nil)), + }, + }, }, kube: &test.MockClient{ MockGet: func(_ context.Context, key client.ObjectKey, obj client.Object) error { @@ -705,6 +720,11 @@ func TestUpdate(t *testing.T) { }, }, }, + Status: v1alpha1.UserStatus{ + AtProvider: v1alpha1.UserObservation{ + AuthPlugin: pointer.StringPtr(defaultAuthPlugin(nil)), + }, + }, }, kube: &test.MockClient{ MockGet: func(_ context.Context, key client.ObjectKey, obj client.Object) error { @@ -741,6 +761,65 @@ func TestUpdate(t *testing.T) { }, }, }, + "UpdatedResourceOptions": { + reason: "We should execute an SQL query if the resource options are not synced.", + fields: fields{ + db: &mockDB{ + MockExec: func(ctx context.Context, q xsql.Query) error { + return nil + }, + }, + }, + args: args{ + mg: &v1alpha1.User{ + ObjectMeta: v1.ObjectMeta{ + Annotations: map[string]string{ + meta.AnnotationKeyExternalName: "example", + }, + }, + Spec: v1alpha1.UserSpec{ + ForProvider: v1alpha1.UserParameters{ + PasswordSecretRef: &xpv1.SecretKeySelector{ + SecretReference: xpv1.SecretReference{ + Name: "connection-secret", + }, + Key: xpv1.ResourceCredentialsSecretPasswordKey, + }, + ResourceOptions: &v1alpha1.ResourceOptions{ + MaxQueriesPerHour: pointer.IntPtr(10), + MaxUpdatesPerHour: pointer.IntPtr(10), + MaxConnectionsPerHour: pointer.IntPtr(10), + MaxUserConnections: pointer.IntPtr(10), + }, + }, + }, + Status: v1alpha1.UserStatus{ + AtProvider: v1alpha1.UserObservation{ + ResourceOptionsAsClauses: []string{ + "MAX_QUERIES_PER_HOUR 20", // default ResourceOptions values + "MAX_UPDATES_PER_HOUR 20", + "MAX_CONNECTIONS_PER_HOUR 20", + "MAX_USER_CONNECTIONS 20", + }, + AuthPlugin: pointer.StringPtr(defaultAuthPlugin(nil)), // default AuthPlugin value + }, + }, + }, + kube: &test.MockClient{ + MockGet: func(_ context.Context, key client.ObjectKey, obj client.Object) error { + secret := corev1.Secret{ + Data: map[string][]byte{}, + } + secret.Data[xpv1.ResourceCredentialsSecretPasswordKey] = []byte("samesame") + secret.DeepCopyInto(obj.(*corev1.Secret)) + return nil + }, + }, + }, + want: want{ + err: nil, + }, + }, "NoUpdateQueryUnchangedResourceOptions": { reason: "We should not execute an SQL query if the resource options are unchanged.", fields: fields{ @@ -774,11 +853,12 @@ func TestUpdate(t *testing.T) { Status: v1alpha1.UserStatus{ AtProvider: v1alpha1.UserObservation{ ResourceOptionsAsClauses: []string{ - "MAX_QUERIES_PER_HOUR 0", + "MAX_QUERIES_PER_HOUR 0", // default ResourceOptions values "MAX_UPDATES_PER_HOUR 0", "MAX_CONNECTIONS_PER_HOUR 0", "MAX_USER_CONNECTIONS 0", }, + AuthPlugin: pointer.StringPtr(defaultAuthPlugin(nil)), // default AuthPlugin value }, }, }, @@ -817,6 +897,60 @@ func TestUpdate(t *testing.T) { UsePassword: pointer.BoolPtr(false), }, }, + Status: v1alpha1.UserStatus{ + AtProvider: v1alpha1.UserObservation{ + AuthPlugin: pointer.StringPtr("authentication_ldap_simple"), + }, + }, + }, + }, + want: want{ + err: nil, + c: managed.ExternalUpdate{}, + }, + }, + "UpdatedAuthPlugin": { + reason: "We should execute an SQL query if the auth plugin is not synced.", + fields: fields{ + db: &mockDB{ + MockExec: func(ctx context.Context, q xsql.Query) error { + return nil + }, + }, + }, + args: args{ + mg: &v1alpha1.User{ + ObjectMeta: v1.ObjectMeta{ + Annotations: map[string]string{ + meta.AnnotationKeyExternalName: "example", + }, + }, + Spec: v1alpha1.UserSpec{ + ForProvider: v1alpha1.UserParameters{ + PasswordSecretRef: &xpv1.SecretKeySelector{ + SecretReference: xpv1.SecretReference{ + Name: "connection-secret", + }, + Key: xpv1.ResourceCredentialsSecretPasswordKey, + }, + AuthPlugin: pointer.StringPtr(defaultAuthPlugin(nil)), + }, + }, + Status: v1alpha1.UserStatus{ + AtProvider: v1alpha1.UserObservation{ + AuthPlugin: pointer.StringPtr("authentication_ldap_simple"), + }, + }, + }, + kube: &test.MockClient{ + MockGet: func(_ context.Context, key client.ObjectKey, obj client.Object) error { + secret := corev1.Secret{ + Data: map[string][]byte{}, + } + secret.Data[xpv1.ResourceCredentialsSecretPasswordKey] = []byte("samesame") + secret.DeepCopyInto(obj.(*corev1.Secret)) + return nil + }, }, }, want: want{ From 6b453fe2574030b2c283c1a81dba529b277ee5b5 Mon Sep 17 00:00:00 2001 From: Alejandro Recalde Date: Mon, 27 Feb 2023 16:24:07 -0300 Subject: [PATCH 7/8] Add missing comma in grant select query (observe method) Signed-off-by: Alejandro Recalde --- pkg/controller/mysql/user/reconciler.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/controller/mysql/user/reconciler.go b/pkg/controller/mysql/user/reconciler.go index d7550777..c2044827 100644 --- a/pkg/controller/mysql/user/reconciler.go +++ b/pkg/controller/mysql/user/reconciler.go @@ -195,7 +195,7 @@ func (c *external) Observe(ctx context.Context, mg resource.Managed) (managed.Ex "max_questions, " + "max_updates, " + "max_connections, " + - "max_user_connections " + + "max_user_connections, " + "plugin" + "FROM mysql.user WHERE User = ? AND Host = ?" err := c.db.Scan(ctx, From 6529b9ea4a3b244ef926b0a00722a2b2b1e775f9 Mon Sep 17 00:00:00 2001 From: Alejandro Recalde Date: Mon, 27 Feb 2023 16:45:59 -0300 Subject: [PATCH 8/8] Add space in grant select query (observe method) Signed-off-by: Alejandro Recalde --- pkg/controller/mysql/user/reconciler.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/controller/mysql/user/reconciler.go b/pkg/controller/mysql/user/reconciler.go index c2044827..5377d8ed 100644 --- a/pkg/controller/mysql/user/reconciler.go +++ b/pkg/controller/mysql/user/reconciler.go @@ -196,7 +196,7 @@ func (c *external) Observe(ctx context.Context, mg resource.Managed) (managed.Ex "max_updates, " + "max_connections, " + "max_user_connections, " + - "plugin" + + "plugin " + "FROM mysql.user WHERE User = ? AND Host = ?" err := c.db.Scan(ctx, xsql.Query{