diff --git a/apis/cluster/postgresql/v1alpha1/role_types.go b/apis/cluster/postgresql/v1alpha1/role_types.go index f00803dd..5d2c2e3a 100644 --- a/apis/cluster/postgresql/v1alpha1/role_types.go +++ b/apis/cluster/postgresql/v1alpha1/role_types.go @@ -90,6 +90,12 @@ type RoleParameters struct { // See https://www.postgresql.org/docs/current/runtime-config-client.html for some available configuration parameters. // +optional ConfigurationParameters *[]RoleConfigurationParameter `json:"configurationParameters,omitempty"` + + // PasswordReset controls behaviour when the role exists in the database but the connection + // secret has no password. + // When true, a new password is generated and written to the connection secret. + // +optional + PasswordReset *bool `json:"passwordReset,omitempty"` } // RoleConfigurationParameter is a role configuration parameter. diff --git a/apis/cluster/postgresql/v1alpha1/zz_generated.deepcopy.go b/apis/cluster/postgresql/v1alpha1/zz_generated.deepcopy.go index 599a6d62..7784a6a0 100644 --- a/apis/cluster/postgresql/v1alpha1/zz_generated.deepcopy.go +++ b/apis/cluster/postgresql/v1alpha1/zz_generated.deepcopy.go @@ -945,6 +945,11 @@ func (in *RoleParameters) DeepCopyInto(out *RoleParameters) { copy(*out, *in) } } + if in.PasswordReset != nil { + in, out := &in.PasswordReset, &out.PasswordReset + *out = new(bool) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RoleParameters. diff --git a/apis/namespaced/postgresql/v1alpha1/role_types.go b/apis/namespaced/postgresql/v1alpha1/role_types.go index cd5e8afc..a81bc51b 100644 --- a/apis/namespaced/postgresql/v1alpha1/role_types.go +++ b/apis/namespaced/postgresql/v1alpha1/role_types.go @@ -91,6 +91,12 @@ type RoleParameters struct { // See https://www.postgresql.org/docs/current/runtime-config-client.html for some available configuration parameters. // +optional ConfigurationParameters *[]RoleConfigurationParameter `json:"configurationParameters,omitempty"` + + // PasswordReset controls behaviour when the role exists in the database but the connection + // secret has no password. + // When true, a new password is generated and written to the connection secret. + // +optional + PasswordReset *bool `json:"passwordReset,omitempty"` } // RoleConfigurationParameter is a role configuration parameter. diff --git a/apis/namespaced/postgresql/v1alpha1/zz_generated.deepcopy.go b/apis/namespaced/postgresql/v1alpha1/zz_generated.deepcopy.go index 3a65605b..a4cd4c32 100644 --- a/apis/namespaced/postgresql/v1alpha1/zz_generated.deepcopy.go +++ b/apis/namespaced/postgresql/v1alpha1/zz_generated.deepcopy.go @@ -1053,6 +1053,11 @@ func (in *RoleParameters) DeepCopyInto(out *RoleParameters) { copy(*out, *in) } } + if in.PasswordReset != nil { + in, out := &in.PasswordReset, &out.PasswordReset + *out = new(bool) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RoleParameters. diff --git a/package/crds/postgresql.sql.crossplane.io_roles.yaml b/package/crds/postgresql.sql.crossplane.io_roles.yaml index 1a7a2729..78d40b37 100644 --- a/package/crds/postgresql.sql.crossplane.io_roles.yaml +++ b/package/crds/postgresql.sql.crossplane.io_roles.yaml @@ -94,6 +94,11 @@ spec: description: ConnectionLimit to be applied to the role. format: int32 type: integer + passwordReset: + description: |- + PasswordReset controls behaviour when the role exists in the database but the connection + secret has no password. When true, a new password is generated and written to the connection secret. + type: boolean passwordSecretRef: description: |- PasswordSecretRef references the secret that contains the password used diff --git a/package/crds/postgresql.sql.m.crossplane.io_roles.yaml b/package/crds/postgresql.sql.m.crossplane.io_roles.yaml index 2f4209f8..46c8ef9c 100644 --- a/package/crds/postgresql.sql.m.crossplane.io_roles.yaml +++ b/package/crds/postgresql.sql.m.crossplane.io_roles.yaml @@ -80,6 +80,11 @@ spec: description: ConnectionLimit to be applied to the role. format: int32 type: integer + passwordReset: + description: |- + PasswordReset controls behaviour when the role exists in the database but the connection + secret has no password. When true, a new password is generated and written to the connection secret. + type: boolean passwordSecretRef: description: |- PasswordSecretRef references the secret that contains the password used diff --git a/pkg/controller/cluster/postgresql/role/reconciler.go b/pkg/controller/cluster/postgresql/role/reconciler.go index f666a6cf..0be0b0c2 100644 --- a/pkg/controller/cluster/postgresql/role/reconciler.go +++ b/pkg/controller/cluster/postgresql/role/reconciler.go @@ -341,6 +341,12 @@ func (c *external) Update(ctx context.Context, mg *v1alpha1.Role) (managed.Exter crn := pq.QuoteIdentifier(meta.GetExternalName(mg)) if pwchanged { + if pw == "" { + pw, err = password.Generate() + if err != nil { + return managed.ExternalUpdate{}, err + } + } if err := c.db.Exec(ctx, xsql.Query{ String: fmt.Sprintf("ALTER ROLE %s PASSWORD %s", crn, pq.QuoteLiteral(pw)), }); err != nil { diff --git a/pkg/controller/cluster/postgresql/role/reconciler_test.go b/pkg/controller/cluster/postgresql/role/reconciler_test.go index 23248982..23525e51 100644 --- a/pkg/controller/cluster/postgresql/role/reconciler_test.go +++ b/pkg/controller/cluster/postgresql/role/reconciler_test.go @@ -1043,6 +1043,272 @@ func TestUpdate(t *testing.T) { } } +func TestGetPassword(t *testing.T) { + type args struct { + ctx context.Context + role *v1alpha1.Role + kube client.Client + } + type want struct { + pwd string + changed bool + err error + } + + cases := map[string]struct { + reason string + args args + want want + }{ + "NoSecretRefNoPasswordReset": { + reason: "No password secret ref and no passwordReset should return unchanged", + args: args{ + role: &v1alpha1.Role{}, + }, + want: want{pwd: "", changed: false}, + }, + "PasswordResetFalse": { + reason: "passwordReset=false should not trigger a reset", + args: args{ + role: &v1alpha1.Role{ + Spec: v1alpha1.RoleSpec{ + ForProvider: v1alpha1.RoleParameters{ + PasswordReset: ptr.To(false), + }, + }, + }, + }, + want: want{pwd: "", changed: false}, + }, + "PasswordResetTrueNoConnectionSecretRef": { + reason: "passwordReset=true with no WriteConnectionSecretToReference should not trigger a reset", + args: args{ + role: &v1alpha1.Role{ + Spec: v1alpha1.RoleSpec{ + ForProvider: v1alpha1.RoleParameters{ + PasswordReset: ptr.To(true), + }, + }, + }, + }, + want: want{pwd: "", changed: false}, + }, + "PasswordResetTrueEmptySecret": { + reason: "passwordReset=true with no password in connection secret should trigger a reset", + args: args{ + role: &v1alpha1.Role{ + Spec: v1alpha1.RoleSpec{ + ResourceSpec: xpv1.ResourceSpec{ + WriteConnectionSecretToReference: &xpv1.SecretReference{ + Name: "connection-secret", + Namespace: "default", + }, + }, + ForProvider: v1alpha1.RoleParameters{ + PasswordReset: ptr.To(true), + }, + }, + }, + kube: &test.MockClient{MockGet: test.NewMockGetFn(nil)}, + }, + want: want{pwd: "", changed: true}, + }, + "PasswordResetTruePasswordExists": { + reason: "passwordReset=true with a password already in connection secret should not trigger a reset", + args: args{ + role: &v1alpha1.Role{ + Spec: v1alpha1.RoleSpec{ + ResourceSpec: xpv1.ResourceSpec{ + WriteConnectionSecretToReference: &xpv1.SecretReference{ + Name: "connection-secret", + Namespace: "default", + }, + }, + ForProvider: v1alpha1.RoleParameters{ + PasswordReset: ptr.To(true), + }, + }, + }, + kube: &test.MockClient{ + MockGet: func(_ context.Context, _ client.ObjectKey, obj client.Object) error { + s := obj.(*corev1.Secret) + s.Data = map[string][]byte{ + xpv1.ResourceCredentialsSecretPasswordKey: []byte("existing-password"), + } + return nil + }, + }, + }, + want: want{pwd: "", changed: false}, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + e := external{kube: tc.args.kube} + pwd, changed, err := e.getPassword(tc.args.ctx, tc.args.role) + if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" { + t.Errorf("\n%s\ne.getPassword(...): -want error, +got error:\n%s\n", tc.reason, diff) + } + if diff := cmp.Diff(tc.want.pwd, pwd); diff != "" { + t.Errorf("\n%s\ne.getPassword(...): -want pwd, +got pwd:\n%s\n", tc.reason, diff) + } + if diff := cmp.Diff(tc.want.changed, changed); diff != "" { + t.Errorf("\n%s\ne.getPassword(...): -want changed, +got changed:\n%s\n", tc.reason, diff) + } + }) + } +} + +func TestUpdatePasswordReset(t *testing.T) { + errBoom := errors.New("boom") + + type fields struct { + execQuery *string + } + + type args struct { + ctx context.Context + mg *v1alpha1.Role + kube client.Client + } + + type want struct { + err error + passwordGenerated bool + } + + cases := map[string]struct { + reason string + fields fields + args args + want want + }{ + "PasswordResetTrueEmptySecretGeneratesPassword": { + reason: "passwordReset=true with no password in connection secret should generate a new password", + fields: fields{execQuery: new(string)}, + args: args{ + mg: &v1alpha1.Role{ + ObjectMeta: v1.ObjectMeta{ + Annotations: map[string]string{ + meta.AnnotationKeyExternalName: "example", + }, + }, + Spec: v1alpha1.RoleSpec{ + ResourceSpec: xpv1.ResourceSpec{ + WriteConnectionSecretToReference: &xpv1.SecretReference{ + Name: "connection-secret", + Namespace: "default", + }, + }, + ForProvider: v1alpha1.RoleParameters{ + PasswordReset: ptr.To(true), + }, + }, + }, + kube: &test.MockClient{MockGet: test.NewMockGetFn(nil)}, + }, + want: want{ + err: nil, + passwordGenerated: true, + }, + }, + "PasswordResetTruePasswordExistsNoReset": { + reason: "passwordReset=true with existing password in connection secret should not reset", + fields: fields{execQuery: nil}, + args: args{ + mg: &v1alpha1.Role{ + ObjectMeta: v1.ObjectMeta{ + Annotations: map[string]string{ + meta.AnnotationKeyExternalName: "example", + }, + }, + Spec: v1alpha1.RoleSpec{ + ResourceSpec: xpv1.ResourceSpec{ + WriteConnectionSecretToReference: &xpv1.SecretReference{ + Name: "connection-secret", + Namespace: "default", + }, + }, + ForProvider: v1alpha1.RoleParameters{ + PasswordReset: ptr.To(true), + }, + }, + }, + kube: &test.MockClient{ + MockGet: func(_ context.Context, _ client.ObjectKey, obj client.Object) error { + s := obj.(*corev1.Secret) + s.Data = map[string][]byte{ + xpv1.ResourceCredentialsSecretPasswordKey: []byte("existing-password"), + } + return nil + }, + }, + }, + want: want{ + err: nil, + passwordGenerated: false, + }, + }, + "PasswordResetFalseNoReset": { + reason: "passwordReset=false should not trigger any database call", + fields: fields{execQuery: nil}, + args: args{ + mg: &v1alpha1.Role{ + ObjectMeta: v1.ObjectMeta{ + Annotations: map[string]string{ + meta.AnnotationKeyExternalName: "example", + }, + }, + Spec: v1alpha1.RoleSpec{ + ForProvider: v1alpha1.RoleParameters{ + PasswordReset: ptr.To(false), + }, + }, + }, + kube: &test.MockClient{}, + }, + want: want{ + err: nil, + passwordGenerated: false, + }, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + execQuery := tc.fields.execQuery + db := &mockDB{ + MockExec: func(ctx context.Context, q xsql.Query) error { + if execQuery == nil { + return errBoom + } + *execQuery = q.String + return nil + }, + } + e := external{ + db: db, + kube: tc.args.kube, + } + got, err := e.Update(tc.args.ctx, tc.args.mg) + if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" { + t.Errorf("\n%s\ne.Update(...): -want error, +got error:\n%s\n", tc.reason, diff) + } + if tc.want.passwordGenerated { + if execQuery == nil || *execQuery == "" { + t.Errorf("\n%s\ne.Update(...): expected ALTER ROLE PASSWORD query to be executed\n", tc.reason) + } else if *execQuery == fmt.Sprintf("ALTER ROLE %s PASSWORD ''", pq.QuoteIdentifier("example")) { + t.Errorf("\n%s\ne.Update(...): ALTER ROLE PASSWORD query contained an empty password\n", tc.reason) + } + if pw := got.ConnectionDetails[xpv1.ResourceCredentialsSecretPasswordKey]; len(pw) == 0 { + t.Errorf("\n%s\ne.Update(...): expected non-empty password in ConnectionDetails\n", tc.reason) + } + } + }) + } +} + func TestDelete(t *testing.T) { errBoom := errors.New("boom") diff --git a/pkg/controller/cluster/postgresql/role/utils.go b/pkg/controller/cluster/postgresql/role/utils.go index 94d89f05..9bb89745 100644 --- a/pkg/controller/cluster/postgresql/role/utils.go +++ b/pkg/controller/cluster/postgresql/role/utils.go @@ -31,36 +31,59 @@ import ( ) func (c *external) getPassword(ctx context.Context, role *v1alpha1.Role) (newPwd string, changed bool, err error) { - if role.Spec.ForProvider.PasswordSecretRef == nil { - return "", false, nil - } - nn := types.NamespacedName{ - Name: role.Spec.ForProvider.PasswordSecretRef.Name, - Namespace: role.Spec.ForProvider.PasswordSecretRef.Namespace, - } - s := &corev1.Secret{} - if err := c.kube.Get(ctx, nn, s); err != nil { - return "", false, errors.Wrap(err, errGetPasswordSecretFailed) + if role.Spec.ForProvider.PasswordSecretRef != nil { + nn := types.NamespacedName{ + Name: role.Spec.ForProvider.PasswordSecretRef.Name, + Namespace: role.Spec.ForProvider.PasswordSecretRef.Namespace, + } + s := &corev1.Secret{} + if err := c.kube.Get(ctx, nn, s); err != nil { + return "", false, errors.Wrap(err, errGetPasswordSecretFailed) + } + newPwd = string(s.Data[role.Spec.ForProvider.PasswordSecretRef.Key]) + + if role.Spec.WriteConnectionSecretToReference == nil { + return newPwd, false, nil + } + + nn = types.NamespacedName{ + Name: role.Spec.WriteConnectionSecretToReference.Name, + Namespace: role.Spec.WriteConnectionSecretToReference.Namespace, + } + s = &corev1.Secret{} + // the output secret may not exist yet, so we can skip returning an + // error if the error is NotFound + if err := c.kube.Get(ctx, nn, s); resource.IgnoreNotFound(err) != nil { + return "", false, err + } + // if newPwd was set to some value, compare value in output secret with + // newPwd + changed = newPwd != "" && newPwd != string(s.Data[xpv1.ResourceCredentialsSecretPasswordKey]) + + return newPwd, changed, nil } - newPwd = string(s.Data[role.Spec.ForProvider.PasswordSecretRef.Key]) + changed, err = c.shouldResetPassword(ctx, role) + return "", changed, err +} + +// shouldResetPassword returns true if passwordReset=true and the connection secret has no password. +// This handles the case where the role exists in the DB after restoration from a snapshot +// but the connection secret is empty. When false, the provider acts as before. +func (c *external) shouldResetPassword(ctx context.Context, role *v1alpha1.Role) (bool, error) { + if role.Spec.ForProvider.PasswordReset == nil || !*role.Spec.ForProvider.PasswordReset { + return false, nil + } if role.Spec.WriteConnectionSecretToReference == nil { - return newPwd, false, nil + return false, nil } - - nn = types.NamespacedName{ + nn := types.NamespacedName{ Name: role.Spec.WriteConnectionSecretToReference.Name, Namespace: role.Spec.WriteConnectionSecretToReference.Namespace, } - s = &corev1.Secret{} - // the output secret may not exist yet, so we can skip returning an - // error if the error is NotFound + s := &corev1.Secret{} if err := c.kube.Get(ctx, nn, s); resource.IgnoreNotFound(err) != nil { - return "", false, err + return false, err } - // if newPwd was set to some value, compare value in output secret with - // newPwd - changed = newPwd != "" && newPwd != string(s.Data[xpv1.ResourceCredentialsSecretPasswordKey]) - - return newPwd, changed, nil + return len(s.Data[xpv1.ResourceCredentialsSecretPasswordKey]) == 0, nil } diff --git a/pkg/controller/namespaced/postgresql/role/reconciler.go b/pkg/controller/namespaced/postgresql/role/reconciler.go index 95430342..b79999db 100644 --- a/pkg/controller/namespaced/postgresql/role/reconciler.go +++ b/pkg/controller/namespaced/postgresql/role/reconciler.go @@ -325,6 +325,12 @@ func (c *external) Update(ctx context.Context, mg *namespacedv1alpha1.Role) (man crn := pq.QuoteIdentifier(meta.GetExternalName(mg)) if pwchanged { + if pw == "" { + pw, err = password.Generate() + if err != nil { + return managed.ExternalUpdate{}, err + } + } if err := c.db.Exec(ctx, xsql.Query{ String: fmt.Sprintf("ALTER ROLE %s PASSWORD %s", crn, pq.QuoteLiteral(pw)), }); err != nil { diff --git a/pkg/controller/namespaced/postgresql/role/reconciler_test.go b/pkg/controller/namespaced/postgresql/role/reconciler_test.go index b73c9eee..aa922096 100644 --- a/pkg/controller/namespaced/postgresql/role/reconciler_test.go +++ b/pkg/controller/namespaced/postgresql/role/reconciler_test.go @@ -1106,6 +1106,274 @@ func TestUpdate(t *testing.T) { } } +func TestGetPassword(t *testing.T) { + type args struct { + ctx context.Context + role *v1alpha1.Role + kube client.Client + } + type want struct { + pwd string + changed bool + err error + } + + cases := map[string]struct { + reason string + args args + want want + }{ + "NoSecretRefNoPasswordReset": { + reason: "No password secret ref and no passwordReset should return unchanged", + args: args{ + role: &v1alpha1.Role{}, + }, + want: want{pwd: "", changed: false}, + }, + "PasswordResetFalse": { + reason: "passwordReset=false should not trigger a reset", + args: args{ + role: &v1alpha1.Role{ + Spec: v1alpha1.RoleSpec{ + ForProvider: v1alpha1.RoleParameters{ + PasswordReset: ptr.To(false), + }, + }, + }, + }, + want: want{pwd: "", changed: false}, + }, + "PasswordResetTrueNoConnectionSecretRef": { + reason: "passwordReset=true with no WriteConnectionSecretToReference should not trigger a reset", + args: args{ + role: &v1alpha1.Role{ + Spec: v1alpha1.RoleSpec{ + ForProvider: v1alpha1.RoleParameters{ + PasswordReset: ptr.To(true), + }, + }, + }, + }, + want: want{pwd: "", changed: false}, + }, + "PasswordResetTrueEmptySecret": { + reason: "passwordReset=true with no password in connection secret should trigger a reset", + args: args{ + role: &v1alpha1.Role{ + Spec: v1alpha1.RoleSpec{ + ManagedResourceSpec: xpv2.ManagedResourceSpec{ + WriteConnectionSecretToReference: &common.LocalSecretReference{ + Name: "connection-secret", + }, + }, + ForProvider: v1alpha1.RoleParameters{ + PasswordReset: ptr.To(true), + }, + }, + }, + kube: &test.MockClient{MockGet: test.NewMockGetFn(nil)}, + }, + want: want{pwd: "", changed: true}, + }, + "PasswordResetTruePasswordExists": { + reason: "passwordReset=true with a password already in connection secret should not trigger a reset", + args: args{ + role: &v1alpha1.Role{ + Spec: v1alpha1.RoleSpec{ + ManagedResourceSpec: xpv2.ManagedResourceSpec{ + WriteConnectionSecretToReference: &common.LocalSecretReference{ + Name: "connection-secret", + }, + }, + ForProvider: v1alpha1.RoleParameters{ + PasswordReset: ptr.To(true), + }, + }, + }, + kube: &test.MockClient{ + MockGet: func(_ context.Context, _ client.ObjectKey, obj client.Object) error { + s := obj.(*corev1.Secret) + s.Data = map[string][]byte{ + xpv1.ResourceCredentialsSecretPasswordKey: []byte("existing-password"), + } + return nil + }, + }, + }, + want: want{pwd: "", changed: false}, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + e := external{kube: tc.args.kube} + pwd, changed, err := e.getPassword(tc.args.ctx, tc.args.role) + if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" { + t.Errorf("\n%s\ne.getPassword(...): -want error, +got error:\n%s\n", tc.reason, diff) + } + if diff := cmp.Diff(tc.want.pwd, pwd); diff != "" { + t.Errorf("\n%s\ne.getPassword(...): -want pwd, +got pwd:\n%s\n", tc.reason, diff) + } + if diff := cmp.Diff(tc.want.changed, changed); diff != "" { + t.Errorf("\n%s\ne.getPassword(...): -want changed, +got changed:\n%s\n", tc.reason, diff) + } + }) + } +} + +func TestUpdatePasswordReset(t *testing.T) { + errBoom := errors.New("boom") + + type fields struct { + // execQuery receives the SQL query string from MockExec, if called. + execQuery *string + } + + type args struct { + ctx context.Context + mg *v1alpha1.Role + kube client.Client + } + + type want struct { + err error + // passwordGenerated asserts that ALTER ROLE was called with a non-empty + // generated password and that ConnectionDetails carry a non-empty password. + passwordGenerated bool + } + + cases := map[string]struct { + reason string + fields fields + args args + want want + }{ + "PasswordResetTrueEmptySecretGeneratesPassword": { + reason: "passwordReset=true with no password in connection secret should generate a new password", + fields: fields{execQuery: new(string)}, + args: args{ + mg: &v1alpha1.Role{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + meta.AnnotationKeyExternalName: "example", + }, + }, + Spec: v1alpha1.RoleSpec{ + ManagedResourceSpec: xpv2.ManagedResourceSpec{ + WriteConnectionSecretToReference: &common.LocalSecretReference{ + Name: "connection-secret", + }, + }, + ForProvider: v1alpha1.RoleParameters{ + PasswordReset: ptr.To(true), + }, + }, + }, + kube: &test.MockClient{MockGet: test.NewMockGetFn(nil)}, + }, + want: want{ + err: nil, + passwordGenerated: true, + }, + }, + "PasswordResetTruePasswordExistsNoReset": { + reason: "passwordReset=true with existing password in connection secret should not reset", + fields: fields{execQuery: nil}, + args: args{ + mg: &v1alpha1.Role{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + meta.AnnotationKeyExternalName: "example", + }, + }, + Spec: v1alpha1.RoleSpec{ + ManagedResourceSpec: xpv2.ManagedResourceSpec{ + WriteConnectionSecretToReference: &common.LocalSecretReference{ + Name: "connection-secret", + }, + }, + ForProvider: v1alpha1.RoleParameters{ + PasswordReset: ptr.To(true), + }, + }, + }, + kube: &test.MockClient{ + MockGet: func(_ context.Context, _ client.ObjectKey, obj client.Object) error { + s := obj.(*corev1.Secret) + s.Data = map[string][]byte{ + xpv1.ResourceCredentialsSecretPasswordKey: []byte("existing-password"), + } + return nil + }, + }, + }, + want: want{ + err: nil, + passwordGenerated: false, + }, + }, + "PasswordResetFalseNoReset": { + reason: "passwordReset=false should not trigger any database call", + fields: fields{execQuery: nil}, + args: args{ + mg: &v1alpha1.Role{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + meta.AnnotationKeyExternalName: "example", + }, + }, + Spec: v1alpha1.RoleSpec{ + ForProvider: v1alpha1.RoleParameters{ + PasswordReset: ptr.To(false), + }, + }, + }, + kube: &test.MockClient{}, + }, + want: want{ + err: nil, + passwordGenerated: false, + }, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + execQuery := tc.fields.execQuery + db := &mockDB{ + MockExec: func(ctx context.Context, q xsql.Query) error { + if execQuery == nil { + // No DB call expected. + return errBoom + } + *execQuery = q.String + return nil + }, + } + e := external{ + db: db, + kube: tc.args.kube, + } + got, err := e.Update(tc.args.ctx, tc.args.mg) + if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" { + t.Errorf("\n%s\ne.Update(...): -want error, +got error:\n%s\n", tc.reason, diff) + } + if tc.want.passwordGenerated { + // Verify the ALTER ROLE query was issued with a non-empty password. + if execQuery == nil || *execQuery == "" { + t.Errorf("\n%s\ne.Update(...): expected ALTER ROLE PASSWORD query to be executed\n", tc.reason) + } else if *execQuery == fmt.Sprintf("ALTER ROLE %s PASSWORD ''", pq.QuoteIdentifier("example")) { + t.Errorf("\n%s\ne.Update(...): ALTER ROLE PASSWORD query contained an empty password\n", tc.reason) + } + // Verify the returned ConnectionDetails carry a non-empty password. + if pw := got.ConnectionDetails[xpv1.ResourceCredentialsSecretPasswordKey]; len(pw) == 0 { + t.Errorf("\n%s\ne.Update(...): expected non-empty password in ConnectionDetails\n", tc.reason) + } + } + }) + } +} + func TestDelete(t *testing.T) { errBoom := errors.New("boom") diff --git a/pkg/controller/namespaced/postgresql/role/utils.go b/pkg/controller/namespaced/postgresql/role/utils.go index 4eb7cd5e..aa5eae26 100644 --- a/pkg/controller/namespaced/postgresql/role/utils.go +++ b/pkg/controller/namespaced/postgresql/role/utils.go @@ -31,36 +31,59 @@ import ( ) func (c *external) getPassword(ctx context.Context, role *v1alpha1.Role) (newPwd string, changed bool, err error) { - if role.Spec.ForProvider.PasswordSecretRef == nil { - return "", false, nil - } - nn := types.NamespacedName{ - Name: role.Spec.ForProvider.PasswordSecretRef.Name, - Namespace: role.Namespace, - } - s := &corev1.Secret{} - if err := c.kube.Get(ctx, nn, s); err != nil { - return "", false, errors.Wrap(err, errGetPasswordSecretFailed) + if role.Spec.ForProvider.PasswordSecretRef != nil { + nn := types.NamespacedName{ + Name: role.Spec.ForProvider.PasswordSecretRef.Name, + Namespace: role.Namespace, + } + s := &corev1.Secret{} + if err := c.kube.Get(ctx, nn, s); err != nil { + return "", false, errors.Wrap(err, errGetPasswordSecretFailed) + } + newPwd = string(s.Data[role.Spec.ForProvider.PasswordSecretRef.Key]) + + if role.Spec.WriteConnectionSecretToReference == nil { + return newPwd, false, nil + } + + nn = types.NamespacedName{ + Name: role.Spec.WriteConnectionSecretToReference.Name, + Namespace: role.Namespace, + } + s = &corev1.Secret{} + // the output secret may not exist yet, so we can skip returning an + // error if the error is NotFound + if err := c.kube.Get(ctx, nn, s); resource.IgnoreNotFound(err) != nil { + return "", false, err + } + // if newPwd was set to some value, compare value in output secret with + // newPwd + changed = newPwd != "" && newPwd != string(s.Data[xpv1.ResourceCredentialsSecretPasswordKey]) + + return newPwd, changed, nil } - newPwd = string(s.Data[role.Spec.ForProvider.PasswordSecretRef.Key]) + changed, err = c.shouldResetPassword(ctx, role) + return "", changed, err +} + +// shouldResetPassword returns true if passwordReset=true and the connection secret has no password. +// This handles the case where the role exists in the DB after restoration from a snapshot +// but the connection secret is empty. When false, the provider acts as before. +func (c *external) shouldResetPassword(ctx context.Context, role *v1alpha1.Role) (bool, error) { + if role.Spec.ForProvider.PasswordReset == nil || !*role.Spec.ForProvider.PasswordReset { + return false, nil + } if role.Spec.WriteConnectionSecretToReference == nil { - return newPwd, false, nil + return false, nil } - - nn = types.NamespacedName{ + nn := types.NamespacedName{ Name: role.Spec.WriteConnectionSecretToReference.Name, Namespace: role.Namespace, } - s = &corev1.Secret{} - // the output secret may not exist yet, so we can skip returning an - // error if the error is NotFound + s := &corev1.Secret{} if err := c.kube.Get(ctx, nn, s); resource.IgnoreNotFound(err) != nil { - return "", false, err + return false, err } - // if newPwd was set to some value, compare value in output secret with - // newPwd - changed = newPwd != "" && newPwd != string(s.Data[xpv1.ResourceCredentialsSecretPasswordKey]) - - return newPwd, changed, nil + return len(s.Data[xpv1.ResourceCredentialsSecretPasswordKey]) == 0, nil }