From 68ee6d776627743b0ba79c330d609a1e90b361bb Mon Sep 17 00:00:00 2001 From: Jevgeni Fenko Date: Mon, 23 Mar 2026 09:39:41 +0200 Subject: [PATCH 1/4] trigger new password generation on Role spec.passwordReset boolean change Signed-off-by: Jevgeni Fenko --- .../cluster/postgresql/v1alpha1/role_types.go | 10 +++ .../v1alpha1/zz_generated.deepcopy.go | 10 +++ .../postgresql/v1alpha1/role_types.go | 10 +++ .../v1alpha1/zz_generated.deepcopy.go | 10 +++ .../cluster/postgresql/role/reconciler.go | 10 +++ .../cluster/postgresql/role/utils.go | 63 +++++++++++-------- .../namespaced/postgresql/role/reconciler.go | 10 +++ .../namespaced/postgresql/role/utils.go | 63 +++++++++++-------- 8 files changed, 132 insertions(+), 54 deletions(-) diff --git a/apis/cluster/postgresql/v1alpha1/role_types.go b/apis/cluster/postgresql/v1alpha1/role_types.go index f00803dd..62381ace 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"` + + // ResetPassword when set to true triggers a one-time password reset for the current generation. + // Useful after restoring a database from a snapshot, where roles already exist in the DB but + // the connection secret may be empty. Will not trigger again until the spec is changed. + // +optional + ResetPassword *bool `json:"resetPassword,omitempty"` } // RoleConfigurationParameter is a role configuration parameter. @@ -105,6 +111,10 @@ type RoleObservation struct { PrivilegesAsClauses []string `json:"privilegesAsClauses,omitempty"` // ConfigurationParameters represents the applied configuration parameters for the PostgreSQL role. ConfigurationParameters *[]RoleConfigurationParameter `json:"configurationParameters,omitempty"` + // LastPasswordResetGeneration tracks the resource generation at which the last password reset was applied. + // Used to prevent repeated resets when spec.forProvider.resetPassword is true. + // +optional + LastPasswordResetGeneration *int64 `json:"lastPasswordResetGeneration,omitempty"` } // +kubebuilder:object:root=true diff --git a/apis/cluster/postgresql/v1alpha1/zz_generated.deepcopy.go b/apis/cluster/postgresql/v1alpha1/zz_generated.deepcopy.go index 599a6d62..32b85b08 100644 --- a/apis/cluster/postgresql/v1alpha1/zz_generated.deepcopy.go +++ b/apis/cluster/postgresql/v1alpha1/zz_generated.deepcopy.go @@ -910,6 +910,11 @@ func (in *RoleObservation) DeepCopyInto(out *RoleObservation) { copy(*out, *in) } } + if in.LastPasswordResetGeneration != nil { + in, out := &in.LastPasswordResetGeneration, &out.LastPasswordResetGeneration + *out = new(int64) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RoleObservation. @@ -945,6 +950,11 @@ func (in *RoleParameters) DeepCopyInto(out *RoleParameters) { copy(*out, *in) } } + if in.ResetPassword != nil { + in, out := &in.ResetPassword, &out.ResetPassword + *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..eec9a2a8 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"` + + // ResetPassword when set to true triggers a one-time password reset for the current generation. + // Useful after restoring a database from a snapshot, where roles already exist in the DB but + // the connection secret may be empty. Will not trigger again until the spec is changed. + // +optional + ResetPassword *bool `json:"resetPassword,omitempty"` } // RoleConfigurationParameter is a role configuration parameter. @@ -106,6 +112,10 @@ type RoleObservation struct { PrivilegesAsClauses []string `json:"privilegesAsClauses,omitempty"` // ConfigurationParameters represents the applied configuration parameters for the PostgreSQL role. ConfigurationParameters *[]RoleConfigurationParameter `json:"configurationParameters,omitempty"` + // LastPasswordResetGeneration tracks the resource generation at which the last password reset was applied. + // Used to prevent repeated resets when spec.forProvider.resetPassword is true. + // +optional + LastPasswordResetGeneration *int64 `json:"lastPasswordResetGeneration,omitempty"` } // +kubebuilder:object:root=true diff --git a/apis/namespaced/postgresql/v1alpha1/zz_generated.deepcopy.go b/apis/namespaced/postgresql/v1alpha1/zz_generated.deepcopy.go index 3a65605b..cb94c29d 100644 --- a/apis/namespaced/postgresql/v1alpha1/zz_generated.deepcopy.go +++ b/apis/namespaced/postgresql/v1alpha1/zz_generated.deepcopy.go @@ -1018,6 +1018,11 @@ func (in *RoleObservation) DeepCopyInto(out *RoleObservation) { copy(*out, *in) } } + if in.LastPasswordResetGeneration != nil { + in, out := &in.LastPasswordResetGeneration, &out.LastPasswordResetGeneration + *out = new(int64) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RoleObservation. @@ -1053,6 +1058,11 @@ func (in *RoleParameters) DeepCopyInto(out *RoleParameters) { copy(*out, *in) } } + if in.ResetPassword != nil { + in, out := &in.ResetPassword, &out.ResetPassword + *out = new(bool) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RoleParameters. diff --git a/pkg/controller/cluster/postgresql/role/reconciler.go b/pkg/controller/cluster/postgresql/role/reconciler.go index f666a6cf..d7414923 100644 --- a/pkg/controller/cluster/postgresql/role/reconciler.go +++ b/pkg/controller/cluster/postgresql/role/reconciler.go @@ -341,11 +341,21 @@ 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 { return managed.ExternalUpdate{}, errors.Wrap(err, errUpdateRole) } + if mg.Spec.ForProvider.ResetPassword != nil && *mg.Spec.ForProvider.ResetPassword { + gen := mg.GetGeneration() + mg.Status.AtProvider.LastPasswordResetGeneration = &gen + } } privs := privilegesToClauses(mg.Spec.ForProvider.Privileges) diff --git a/pkg/controller/cluster/postgresql/role/utils.go b/pkg/controller/cluster/postgresql/role/utils.go index 94d89f05..4a3e6d42 100644 --- a/pkg/controller/cluster/postgresql/role/utils.go +++ b/pkg/controller/cluster/postgresql/role/utils.go @@ -31,36 +31,45 @@ 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) - } - newPwd = string(s.Data[role.Spec.ForProvider.PasswordSecretRef.Key]) + 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 - } + 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]) - nn = types.NamespacedName{ - Name: role.Spec.WriteConnectionSecretToReference.Name, - Namespace: role.Spec.WriteConnectionSecretToReference.Namespace, + return newPwd, changed, nil } - 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 + + // resetPassword triggers a forced password reset for the current generation. + if role.Spec.ForProvider.ResetPassword != nil && *role.Spec.ForProvider.ResetPassword { + gen := role.GetGeneration() + if role.Status.AtProvider.LastPasswordResetGeneration == nil || *role.Status.AtProvider.LastPasswordResetGeneration != gen { + return "", true, nil + } } - // 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 "", false, nil } diff --git a/pkg/controller/namespaced/postgresql/role/reconciler.go b/pkg/controller/namespaced/postgresql/role/reconciler.go index 95430342..88680adb 100644 --- a/pkg/controller/namespaced/postgresql/role/reconciler.go +++ b/pkg/controller/namespaced/postgresql/role/reconciler.go @@ -325,11 +325,21 @@ 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 { return managed.ExternalUpdate{}, errors.Wrap(err, errUpdateRole) } + if mg.Spec.ForProvider.ResetPassword != nil && *mg.Spec.ForProvider.ResetPassword { + gen := mg.GetGeneration() + mg.Status.AtProvider.LastPasswordResetGeneration = &gen + } } privs := privilegesToClauses(mg.Spec.ForProvider.Privileges) diff --git a/pkg/controller/namespaced/postgresql/role/utils.go b/pkg/controller/namespaced/postgresql/role/utils.go index 4eb7cd5e..335aed61 100644 --- a/pkg/controller/namespaced/postgresql/role/utils.go +++ b/pkg/controller/namespaced/postgresql/role/utils.go @@ -31,36 +31,45 @@ 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) - } - newPwd = string(s.Data[role.Spec.ForProvider.PasswordSecretRef.Key]) + 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 - } + 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]) - nn = types.NamespacedName{ - Name: role.Spec.WriteConnectionSecretToReference.Name, - Namespace: role.Namespace, + return newPwd, changed, nil } - 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 + + // resetPassword triggers a forced password reset for the current generation. + if role.Spec.ForProvider.ResetPassword != nil && *role.Spec.ForProvider.ResetPassword { + gen := role.GetGeneration() + if role.Status.AtProvider.LastPasswordResetGeneration == nil || *role.Status.AtProvider.LastPasswordResetGeneration != gen { + return "", true, nil + } } - // 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 "", false, nil } From 278322cc1334dcbbc08f0826cf1f90e340378b13 Mon Sep 17 00:00:00 2001 From: Jevgeni Fenko Date: Mon, 23 Mar 2026 09:59:59 +0200 Subject: [PATCH 2/4] trigger new password generation on Role spec.passwordResetToken. This prevents unwanted generation due to any spec change. Signed-off-by: Jevgeni Fenko --- apis/cluster/postgresql/v1alpha1/role_types.go | 12 ++++++------ .../postgresql/v1alpha1/zz_generated.deepcopy.go | 12 ++++++------ apis/namespaced/postgresql/v1alpha1/role_types.go | 12 ++++++------ .../postgresql/v1alpha1/zz_generated.deepcopy.go | 12 ++++++------ package/crds/postgresql.sql.crossplane.io_roles.yaml | 11 +++++++++++ .../crds/postgresql.sql.m.crossplane.io_roles.yaml | 11 +++++++++++ pkg/controller/cluster/postgresql/role/reconciler.go | 6 +++--- pkg/controller/cluster/postgresql/role/utils.go | 11 +++++------ .../namespaced/postgresql/role/reconciler.go | 6 +++--- pkg/controller/namespaced/postgresql/role/utils.go | 11 +++++------ 10 files changed, 62 insertions(+), 42 deletions(-) diff --git a/apis/cluster/postgresql/v1alpha1/role_types.go b/apis/cluster/postgresql/v1alpha1/role_types.go index 62381ace..5269e7a9 100644 --- a/apis/cluster/postgresql/v1alpha1/role_types.go +++ b/apis/cluster/postgresql/v1alpha1/role_types.go @@ -91,11 +91,11 @@ type RoleParameters struct { // +optional ConfigurationParameters *[]RoleConfigurationParameter `json:"configurationParameters,omitempty"` - // ResetPassword when set to true triggers a one-time password reset for the current generation. + // PasswordResetToken can be set to any string to trigger a one-time password reset. // Useful after restoring a database from a snapshot, where roles already exist in the DB but - // the connection secret may be empty. Will not trigger again until the spec is changed. + // the connection secret may be empty. Change the token value to trigger another reset. // +optional - ResetPassword *bool `json:"resetPassword,omitempty"` + PasswordResetToken *string `json:"passwordResetToken,omitempty"` } // RoleConfigurationParameter is a role configuration parameter. @@ -111,10 +111,10 @@ type RoleObservation struct { PrivilegesAsClauses []string `json:"privilegesAsClauses,omitempty"` // ConfigurationParameters represents the applied configuration parameters for the PostgreSQL role. ConfigurationParameters *[]RoleConfigurationParameter `json:"configurationParameters,omitempty"` - // LastPasswordResetGeneration tracks the resource generation at which the last password reset was applied. - // Used to prevent repeated resets when spec.forProvider.resetPassword is true. + // LastPasswordResetToken tracks the passwordResetToken value at which the last password reset was applied. + // Used to prevent repeated resets when spec.forProvider.passwordResetToken is set. // +optional - LastPasswordResetGeneration *int64 `json:"lastPasswordResetGeneration,omitempty"` + LastPasswordResetToken *string `json:"lastPasswordResetToken,omitempty"` } // +kubebuilder:object:root=true diff --git a/apis/cluster/postgresql/v1alpha1/zz_generated.deepcopy.go b/apis/cluster/postgresql/v1alpha1/zz_generated.deepcopy.go index 32b85b08..02e5cf46 100644 --- a/apis/cluster/postgresql/v1alpha1/zz_generated.deepcopy.go +++ b/apis/cluster/postgresql/v1alpha1/zz_generated.deepcopy.go @@ -910,9 +910,9 @@ func (in *RoleObservation) DeepCopyInto(out *RoleObservation) { copy(*out, *in) } } - if in.LastPasswordResetGeneration != nil { - in, out := &in.LastPasswordResetGeneration, &out.LastPasswordResetGeneration - *out = new(int64) + if in.LastPasswordResetToken != nil { + in, out := &in.LastPasswordResetToken, &out.LastPasswordResetToken + *out = new(string) **out = **in } } @@ -950,9 +950,9 @@ func (in *RoleParameters) DeepCopyInto(out *RoleParameters) { copy(*out, *in) } } - if in.ResetPassword != nil { - in, out := &in.ResetPassword, &out.ResetPassword - *out = new(bool) + if in.PasswordResetToken != nil { + in, out := &in.PasswordResetToken, &out.PasswordResetToken + *out = new(string) **out = **in } } diff --git a/apis/namespaced/postgresql/v1alpha1/role_types.go b/apis/namespaced/postgresql/v1alpha1/role_types.go index eec9a2a8..7ab9b1a3 100644 --- a/apis/namespaced/postgresql/v1alpha1/role_types.go +++ b/apis/namespaced/postgresql/v1alpha1/role_types.go @@ -92,11 +92,11 @@ type RoleParameters struct { // +optional ConfigurationParameters *[]RoleConfigurationParameter `json:"configurationParameters,omitempty"` - // ResetPassword when set to true triggers a one-time password reset for the current generation. + // PasswordResetToken can be set to any string to trigger a one-time password reset. // Useful after restoring a database from a snapshot, where roles already exist in the DB but - // the connection secret may be empty. Will not trigger again until the spec is changed. + // the connection secret may be empty. Change the token value to trigger another reset. // +optional - ResetPassword *bool `json:"resetPassword,omitempty"` + PasswordResetToken *string `json:"passwordResetToken,omitempty"` } // RoleConfigurationParameter is a role configuration parameter. @@ -112,10 +112,10 @@ type RoleObservation struct { PrivilegesAsClauses []string `json:"privilegesAsClauses,omitempty"` // ConfigurationParameters represents the applied configuration parameters for the PostgreSQL role. ConfigurationParameters *[]RoleConfigurationParameter `json:"configurationParameters,omitempty"` - // LastPasswordResetGeneration tracks the resource generation at which the last password reset was applied. - // Used to prevent repeated resets when spec.forProvider.resetPassword is true. + // LastPasswordResetToken tracks the passwordResetToken value at which the last password reset was applied. + // Used to prevent repeated resets when spec.forProvider.passwordResetToken is set. // +optional - LastPasswordResetGeneration *int64 `json:"lastPasswordResetGeneration,omitempty"` + LastPasswordResetToken *string `json:"lastPasswordResetToken,omitempty"` } // +kubebuilder:object:root=true diff --git a/apis/namespaced/postgresql/v1alpha1/zz_generated.deepcopy.go b/apis/namespaced/postgresql/v1alpha1/zz_generated.deepcopy.go index cb94c29d..f050149c 100644 --- a/apis/namespaced/postgresql/v1alpha1/zz_generated.deepcopy.go +++ b/apis/namespaced/postgresql/v1alpha1/zz_generated.deepcopy.go @@ -1018,9 +1018,9 @@ func (in *RoleObservation) DeepCopyInto(out *RoleObservation) { copy(*out, *in) } } - if in.LastPasswordResetGeneration != nil { - in, out := &in.LastPasswordResetGeneration, &out.LastPasswordResetGeneration - *out = new(int64) + if in.LastPasswordResetToken != nil { + in, out := &in.LastPasswordResetToken, &out.LastPasswordResetToken + *out = new(string) **out = **in } } @@ -1058,9 +1058,9 @@ func (in *RoleParameters) DeepCopyInto(out *RoleParameters) { copy(*out, *in) } } - if in.ResetPassword != nil { - in, out := &in.ResetPassword, &out.ResetPassword - *out = new(bool) + if in.PasswordResetToken != nil { + in, out := &in.PasswordResetToken, &out.PasswordResetToken + *out = new(string) **out = **in } } diff --git a/package/crds/postgresql.sql.crossplane.io_roles.yaml b/package/crds/postgresql.sql.crossplane.io_roles.yaml index 1a7a2729..97c84ae5 100644 --- a/package/crds/postgresql.sql.crossplane.io_roles.yaml +++ b/package/crds/postgresql.sql.crossplane.io_roles.yaml @@ -94,6 +94,12 @@ spec: description: ConnectionLimit to be applied to the role. format: int32 type: integer + passwordResetToken: + description: |- + PasswordResetToken can be set to any string to trigger a one-time password reset. + Useful after restoring a database from a snapshot, where roles already exist in the DB but + the connection secret may be empty. Change the token value to trigger another reset. + type: string passwordSecretRef: description: |- PasswordSecretRef references the secret that contains the password used @@ -252,6 +258,11 @@ spec: type: string type: object type: array + lastPasswordResetToken: + description: |- + LastPasswordResetToken tracks the passwordResetToken value at which the last password reset was applied. + Used to prevent repeated resets when spec.forProvider.passwordResetToken is set. + type: string privilegesAsClauses: description: |- PrivilegesAsClauses represents the applied privileges state, taking into account diff --git a/package/crds/postgresql.sql.m.crossplane.io_roles.yaml b/package/crds/postgresql.sql.m.crossplane.io_roles.yaml index 2f4209f8..158717c7 100644 --- a/package/crds/postgresql.sql.m.crossplane.io_roles.yaml +++ b/package/crds/postgresql.sql.m.crossplane.io_roles.yaml @@ -80,6 +80,12 @@ spec: description: ConnectionLimit to be applied to the role. format: int32 type: integer + passwordResetToken: + description: |- + PasswordResetToken can be set to any string to trigger a one-time password reset. + Useful after restoring a database from a snapshot, where roles already exist in the DB but + the connection secret may be empty. Change the token value to trigger another reset. + type: string passwordSecretRef: description: |- PasswordSecretRef references the secret that contains the password used @@ -205,6 +211,11 @@ spec: type: string type: object type: array + lastPasswordResetToken: + description: |- + LastPasswordResetToken tracks the passwordResetToken value at which the last password reset was applied. + Used to prevent repeated resets when spec.forProvider.passwordResetToken is set. + type: string privilegesAsClauses: description: |- PrivilegesAsClauses represents the applied privileges state, taking into account diff --git a/pkg/controller/cluster/postgresql/role/reconciler.go b/pkg/controller/cluster/postgresql/role/reconciler.go index d7414923..b697bc36 100644 --- a/pkg/controller/cluster/postgresql/role/reconciler.go +++ b/pkg/controller/cluster/postgresql/role/reconciler.go @@ -352,9 +352,9 @@ func (c *external) Update(ctx context.Context, mg *v1alpha1.Role) (managed.Exter }); err != nil { return managed.ExternalUpdate{}, errors.Wrap(err, errUpdateRole) } - if mg.Spec.ForProvider.ResetPassword != nil && *mg.Spec.ForProvider.ResetPassword { - gen := mg.GetGeneration() - mg.Status.AtProvider.LastPasswordResetGeneration = &gen + if mg.Spec.ForProvider.PasswordResetToken != nil { + token := *mg.Spec.ForProvider.PasswordResetToken + mg.Status.AtProvider.LastPasswordResetToken = &token } } diff --git a/pkg/controller/cluster/postgresql/role/utils.go b/pkg/controller/cluster/postgresql/role/utils.go index 4a3e6d42..76306510 100644 --- a/pkg/controller/cluster/postgresql/role/utils.go +++ b/pkg/controller/cluster/postgresql/role/utils.go @@ -63,12 +63,11 @@ func (c *external) getPassword(ctx context.Context, role *v1alpha1.Role) (newPwd return newPwd, changed, nil } - // resetPassword triggers a forced password reset for the current generation. - if role.Spec.ForProvider.ResetPassword != nil && *role.Spec.ForProvider.ResetPassword { - gen := role.GetGeneration() - if role.Status.AtProvider.LastPasswordResetGeneration == nil || *role.Status.AtProvider.LastPasswordResetGeneration != gen { - return "", true, nil - } + // passwordResetToken triggers a forced password reset when the token changes. + if role.Spec.ForProvider.PasswordResetToken != nil && + (role.Status.AtProvider.LastPasswordResetToken == nil || + *role.Spec.ForProvider.PasswordResetToken != *role.Status.AtProvider.LastPasswordResetToken) { + return "", true, nil } return "", false, nil diff --git a/pkg/controller/namespaced/postgresql/role/reconciler.go b/pkg/controller/namespaced/postgresql/role/reconciler.go index 88680adb..d01e43cf 100644 --- a/pkg/controller/namespaced/postgresql/role/reconciler.go +++ b/pkg/controller/namespaced/postgresql/role/reconciler.go @@ -336,9 +336,9 @@ func (c *external) Update(ctx context.Context, mg *namespacedv1alpha1.Role) (man }); err != nil { return managed.ExternalUpdate{}, errors.Wrap(err, errUpdateRole) } - if mg.Spec.ForProvider.ResetPassword != nil && *mg.Spec.ForProvider.ResetPassword { - gen := mg.GetGeneration() - mg.Status.AtProvider.LastPasswordResetGeneration = &gen + if mg.Spec.ForProvider.PasswordResetToken != nil { + token := *mg.Spec.ForProvider.PasswordResetToken + mg.Status.AtProvider.LastPasswordResetToken = &token } } diff --git a/pkg/controller/namespaced/postgresql/role/utils.go b/pkg/controller/namespaced/postgresql/role/utils.go index 335aed61..3b2a9f92 100644 --- a/pkg/controller/namespaced/postgresql/role/utils.go +++ b/pkg/controller/namespaced/postgresql/role/utils.go @@ -63,12 +63,11 @@ func (c *external) getPassword(ctx context.Context, role *v1alpha1.Role) (newPwd return newPwd, changed, nil } - // resetPassword triggers a forced password reset for the current generation. - if role.Spec.ForProvider.ResetPassword != nil && *role.Spec.ForProvider.ResetPassword { - gen := role.GetGeneration() - if role.Status.AtProvider.LastPasswordResetGeneration == nil || *role.Status.AtProvider.LastPasswordResetGeneration != gen { - return "", true, nil - } + // passwordResetToken triggers a forced password reset when the token changes. + if role.Spec.ForProvider.PasswordResetToken != nil && + (role.Status.AtProvider.LastPasswordResetToken == nil || + *role.Spec.ForProvider.PasswordResetToken != *role.Status.AtProvider.LastPasswordResetToken) { + return "", true, nil } return "", false, nil From 133108a54746cb0b2f00b9841923c09b88609b09 Mon Sep 17 00:00:00 2001 From: Jevgeni Fenko Date: Mon, 23 Mar 2026 10:30:43 +0200 Subject: [PATCH 3/4] added tests Signed-off-by: Jevgeni Fenko --- .../postgresql/role/reconciler_test.go | 233 +++++++++++++++++ .../postgresql/role/reconciler_test.go | 239 ++++++++++++++++++ 2 files changed, 472 insertions(+) diff --git a/pkg/controller/cluster/postgresql/role/reconciler_test.go b/pkg/controller/cluster/postgresql/role/reconciler_test.go index 23248982..42b20ba9 100644 --- a/pkg/controller/cluster/postgresql/role/reconciler_test.go +++ b/pkg/controller/cluster/postgresql/role/reconciler_test.go @@ -1043,6 +1043,239 @@ func TestUpdate(t *testing.T) { } } +func TestGetPassword(t *testing.T) { + type args struct { + ctx context.Context + role *v1alpha1.Role + } + type want struct { + pwd string + changed bool + err error + } + + cases := map[string]struct { + reason string + args args + want want + }{ + "NoSecretRefNoToken": { + reason: "No password secret ref and no reset token should return unchanged", + args: args{ + role: &v1alpha1.Role{}, + }, + want: want{pwd: "", changed: false}, + }, + "NewPasswordResetToken": { + reason: "A new password reset token with no last token should trigger a reset", + args: args{ + role: &v1alpha1.Role{ + Spec: v1alpha1.RoleSpec{ + ForProvider: v1alpha1.RoleParameters{ + PasswordResetToken: ptr.To("token-1"), + }, + }, + }, + }, + want: want{pwd: "", changed: true}, + }, + "PasswordResetTokenUnchanged": { + reason: "An unchanged password reset token should not trigger a reset", + args: args{ + role: &v1alpha1.Role{ + Spec: v1alpha1.RoleSpec{ + ForProvider: v1alpha1.RoleParameters{ + PasswordResetToken: ptr.To("token-1"), + }, + }, + Status: v1alpha1.RoleStatus{ + AtProvider: v1alpha1.RoleObservation{ + LastPasswordResetToken: ptr.To("token-1"), + }, + }, + }, + }, + want: want{pwd: "", changed: false}, + }, + "PasswordResetTokenChanged": { + reason: "A changed password reset token should trigger a reset", + args: args{ + role: &v1alpha1.Role{ + Spec: v1alpha1.RoleSpec{ + ForProvider: v1alpha1.RoleParameters{ + PasswordResetToken: ptr.To("token-2"), + }, + }, + Status: v1alpha1.RoleStatus{ + AtProvider: v1alpha1.RoleObservation{ + LastPasswordResetToken: ptr.To("token-1"), + }, + }, + }, + }, + want: want{pwd: "", changed: true}, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + e := external{kube: nil} + 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 TestUpdatePasswordResetToken(t *testing.T) { + errBoom := errors.New("boom") + + type fields struct { + execQuery *string + } + + type args struct { + ctx context.Context + mg *v1alpha1.Role + } + + type want struct { + err error + lastPasswordResetToken *string + passwordGenerated bool + } + + cases := map[string]struct { + reason string + fields fields + args args + want want + }{ + "NewTokenGeneratesAndSetsPassword": { + reason: "A new password reset token should trigger password generation and update LastPasswordResetToken", + fields: fields{execQuery: new(string)}, + args: args{ + mg: &v1alpha1.Role{ + ObjectMeta: v1.ObjectMeta{ + Annotations: map[string]string{ + meta.AnnotationKeyExternalName: "example", + }, + }, + Spec: v1alpha1.RoleSpec{ + ForProvider: v1alpha1.RoleParameters{ + PasswordResetToken: ptr.To("token-1"), + }, + }, + }, + }, + want: want{ + err: nil, + lastPasswordResetToken: ptr.To("token-1"), + passwordGenerated: true, + }, + }, + "UnchangedTokenNoPasswordReset": { + reason: "An unchanged password reset token 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{ + PasswordResetToken: ptr.To("token-1"), + }, + }, + Status: v1alpha1.RoleStatus{ + AtProvider: v1alpha1.RoleObservation{ + LastPasswordResetToken: ptr.To("token-1"), + }, + }, + }, + }, + want: want{ + err: nil, + lastPasswordResetToken: ptr.To("token-1"), + passwordGenerated: false, + }, + }, + "ChangedTokenGeneratesNewPassword": { + reason: "A changed password reset token should generate a new password and update LastPasswordResetToken", + fields: fields{execQuery: new(string)}, + args: args{ + mg: &v1alpha1.Role{ + ObjectMeta: v1.ObjectMeta{ + Annotations: map[string]string{ + meta.AnnotationKeyExternalName: "example", + }, + }, + Spec: v1alpha1.RoleSpec{ + ForProvider: v1alpha1.RoleParameters{ + PasswordResetToken: ptr.To("token-2"), + }, + }, + Status: v1alpha1.RoleStatus{ + AtProvider: v1alpha1.RoleObservation{ + LastPasswordResetToken: ptr.To("token-1"), + }, + }, + }, + }, + want: want{ + err: nil, + lastPasswordResetToken: ptr.To("token-2"), + passwordGenerated: true, + }, + }, + } + + 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: &test.MockClient{}, + } + 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 diff := cmp.Diff(tc.want.lastPasswordResetToken, tc.args.mg.Status.AtProvider.LastPasswordResetToken); diff != "" { + t.Errorf("\n%s\ne.Update(...): -want lastPasswordResetToken, +got lastPasswordResetToken:\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/namespaced/postgresql/role/reconciler_test.go b/pkg/controller/namespaced/postgresql/role/reconciler_test.go index b73c9eee..f3976a24 100644 --- a/pkg/controller/namespaced/postgresql/role/reconciler_test.go +++ b/pkg/controller/namespaced/postgresql/role/reconciler_test.go @@ -1106,6 +1106,245 @@ func TestUpdate(t *testing.T) { } } +func TestGetPassword(t *testing.T) { + type args struct { + ctx context.Context + role *v1alpha1.Role + } + type want struct { + pwd string + changed bool + err error + } + + cases := map[string]struct { + reason string + args args + want want + }{ + "NoSecretRefNoToken": { + reason: "No password secret ref and no reset token should return unchanged", + args: args{ + role: &v1alpha1.Role{}, + }, + want: want{pwd: "", changed: false}, + }, + "NewPasswordResetToken": { + reason: "A new password reset token with no last token should trigger a reset", + args: args{ + role: &v1alpha1.Role{ + Spec: v1alpha1.RoleSpec{ + ForProvider: v1alpha1.RoleParameters{ + PasswordResetToken: ptr.To("token-1"), + }, + }, + }, + }, + want: want{pwd: "", changed: true}, + }, + "PasswordResetTokenUnchanged": { + reason: "An unchanged password reset token should not trigger a reset", + args: args{ + role: &v1alpha1.Role{ + Spec: v1alpha1.RoleSpec{ + ForProvider: v1alpha1.RoleParameters{ + PasswordResetToken: ptr.To("token-1"), + }, + }, + Status: v1alpha1.RoleStatus{ + AtProvider: v1alpha1.RoleObservation{ + LastPasswordResetToken: ptr.To("token-1"), + }, + }, + }, + }, + want: want{pwd: "", changed: false}, + }, + "PasswordResetTokenChanged": { + reason: "A changed password reset token should trigger a reset", + args: args{ + role: &v1alpha1.Role{ + Spec: v1alpha1.RoleSpec{ + ForProvider: v1alpha1.RoleParameters{ + PasswordResetToken: ptr.To("token-2"), + }, + }, + Status: v1alpha1.RoleStatus{ + AtProvider: v1alpha1.RoleObservation{ + LastPasswordResetToken: ptr.To("token-1"), + }, + }, + }, + }, + want: want{pwd: "", changed: true}, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + e := external{kube: nil} + 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 TestUpdatePasswordResetToken(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 + } + + type want struct { + err error + lastPasswordResetToken *string + // 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 + }{ + "NewTokenGeneratesAndSetsPassword": { + reason: "A new password reset token should trigger password generation and update LastPasswordResetToken", + fields: fields{execQuery: new(string)}, + args: args{ + mg: &v1alpha1.Role{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + meta.AnnotationKeyExternalName: "example", + }, + }, + Spec: v1alpha1.RoleSpec{ + ForProvider: v1alpha1.RoleParameters{ + PasswordResetToken: ptr.To("token-1"), + }, + }, + }, + }, + want: want{ + err: nil, + lastPasswordResetToken: ptr.To("token-1"), + passwordGenerated: true, + }, + }, + "UnchangedTokenNoPasswordReset": { + reason: "An unchanged password reset token 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{ + PasswordResetToken: ptr.To("token-1"), + }, + }, + Status: v1alpha1.RoleStatus{ + AtProvider: v1alpha1.RoleObservation{ + LastPasswordResetToken: ptr.To("token-1"), + }, + }, + }, + }, + want: want{ + err: nil, + lastPasswordResetToken: ptr.To("token-1"), + passwordGenerated: false, + }, + }, + "ChangedTokenGeneratesNewPassword": { + reason: "A changed password reset token should generate a new password and update LastPasswordResetToken", + fields: fields{execQuery: new(string)}, + args: args{ + mg: &v1alpha1.Role{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + meta.AnnotationKeyExternalName: "example", + }, + }, + Spec: v1alpha1.RoleSpec{ + ForProvider: v1alpha1.RoleParameters{ + PasswordResetToken: ptr.To("token-2"), + }, + }, + Status: v1alpha1.RoleStatus{ + AtProvider: v1alpha1.RoleObservation{ + LastPasswordResetToken: ptr.To("token-1"), + }, + }, + }, + }, + want: want{ + err: nil, + lastPasswordResetToken: ptr.To("token-2"), + passwordGenerated: true, + }, + }, + } + + 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: &test.MockClient{}, + } + 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 diff := cmp.Diff(tc.want.lastPasswordResetToken, tc.args.mg.Status.AtProvider.LastPasswordResetToken); diff != "" { + t.Errorf("\n%s\ne.Update(...): -want lastPasswordResetToken, +got lastPasswordResetToken:\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") From 0d912620ae32c38424986c8b63d442b4d8868d42 Mon Sep 17 00:00:00 2001 From: Jevgeni Fenko Date: Thu, 26 Mar 2026 12:54:53 +0200 Subject: [PATCH 4/4] changed to boolean field, which indicates provider behavior after databases and roles restoration Signed-off-by: Jevgeni Fenko --- .../cluster/postgresql/v1alpha1/role_types.go | 12 +- .../v1alpha1/zz_generated.deepcopy.go | 11 +- .../postgresql/v1alpha1/role_types.go | 12 +- .../v1alpha1/zz_generated.deepcopy.go | 11 +- .../postgresql.sql.crossplane.io_roles.yaml | 14 +- .../postgresql.sql.m.crossplane.io_roles.yaml | 14 +- .../cluster/postgresql/role/reconciler.go | 4 - .../postgresql/role/reconciler_test.go | 153 +++++++++++------- .../cluster/postgresql/role/utils.go | 29 +++- .../namespaced/postgresql/role/reconciler.go | 4 - .../postgresql/role/reconciler_test.go | 147 ++++++++++------- .../namespaced/postgresql/role/utils.go | 29 +++- 12 files changed, 247 insertions(+), 193 deletions(-) diff --git a/apis/cluster/postgresql/v1alpha1/role_types.go b/apis/cluster/postgresql/v1alpha1/role_types.go index 5269e7a9..5d2c2e3a 100644 --- a/apis/cluster/postgresql/v1alpha1/role_types.go +++ b/apis/cluster/postgresql/v1alpha1/role_types.go @@ -91,11 +91,11 @@ type RoleParameters struct { // +optional ConfigurationParameters *[]RoleConfigurationParameter `json:"configurationParameters,omitempty"` - // PasswordResetToken can be set to any string to trigger a one-time password reset. - // Useful after restoring a database from a snapshot, where roles already exist in the DB but - // the connection secret may be empty. Change the token value to trigger another reset. + // 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 - PasswordResetToken *string `json:"passwordResetToken,omitempty"` + PasswordReset *bool `json:"passwordReset,omitempty"` } // RoleConfigurationParameter is a role configuration parameter. @@ -111,10 +111,6 @@ type RoleObservation struct { PrivilegesAsClauses []string `json:"privilegesAsClauses,omitempty"` // ConfigurationParameters represents the applied configuration parameters for the PostgreSQL role. ConfigurationParameters *[]RoleConfigurationParameter `json:"configurationParameters,omitempty"` - // LastPasswordResetToken tracks the passwordResetToken value at which the last password reset was applied. - // Used to prevent repeated resets when spec.forProvider.passwordResetToken is set. - // +optional - LastPasswordResetToken *string `json:"lastPasswordResetToken,omitempty"` } // +kubebuilder:object:root=true diff --git a/apis/cluster/postgresql/v1alpha1/zz_generated.deepcopy.go b/apis/cluster/postgresql/v1alpha1/zz_generated.deepcopy.go index 02e5cf46..7784a6a0 100644 --- a/apis/cluster/postgresql/v1alpha1/zz_generated.deepcopy.go +++ b/apis/cluster/postgresql/v1alpha1/zz_generated.deepcopy.go @@ -910,11 +910,6 @@ func (in *RoleObservation) DeepCopyInto(out *RoleObservation) { copy(*out, *in) } } - if in.LastPasswordResetToken != nil { - in, out := &in.LastPasswordResetToken, &out.LastPasswordResetToken - *out = new(string) - **out = **in - } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RoleObservation. @@ -950,9 +945,9 @@ func (in *RoleParameters) DeepCopyInto(out *RoleParameters) { copy(*out, *in) } } - if in.PasswordResetToken != nil { - in, out := &in.PasswordResetToken, &out.PasswordResetToken - *out = new(string) + if in.PasswordReset != nil { + in, out := &in.PasswordReset, &out.PasswordReset + *out = new(bool) **out = **in } } diff --git a/apis/namespaced/postgresql/v1alpha1/role_types.go b/apis/namespaced/postgresql/v1alpha1/role_types.go index 7ab9b1a3..a81bc51b 100644 --- a/apis/namespaced/postgresql/v1alpha1/role_types.go +++ b/apis/namespaced/postgresql/v1alpha1/role_types.go @@ -92,11 +92,11 @@ type RoleParameters struct { // +optional ConfigurationParameters *[]RoleConfigurationParameter `json:"configurationParameters,omitempty"` - // PasswordResetToken can be set to any string to trigger a one-time password reset. - // Useful after restoring a database from a snapshot, where roles already exist in the DB but - // the connection secret may be empty. Change the token value to trigger another reset. + // 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 - PasswordResetToken *string `json:"passwordResetToken,omitempty"` + PasswordReset *bool `json:"passwordReset,omitempty"` } // RoleConfigurationParameter is a role configuration parameter. @@ -112,10 +112,6 @@ type RoleObservation struct { PrivilegesAsClauses []string `json:"privilegesAsClauses,omitempty"` // ConfigurationParameters represents the applied configuration parameters for the PostgreSQL role. ConfigurationParameters *[]RoleConfigurationParameter `json:"configurationParameters,omitempty"` - // LastPasswordResetToken tracks the passwordResetToken value at which the last password reset was applied. - // Used to prevent repeated resets when spec.forProvider.passwordResetToken is set. - // +optional - LastPasswordResetToken *string `json:"lastPasswordResetToken,omitempty"` } // +kubebuilder:object:root=true diff --git a/apis/namespaced/postgresql/v1alpha1/zz_generated.deepcopy.go b/apis/namespaced/postgresql/v1alpha1/zz_generated.deepcopy.go index f050149c..a4cd4c32 100644 --- a/apis/namespaced/postgresql/v1alpha1/zz_generated.deepcopy.go +++ b/apis/namespaced/postgresql/v1alpha1/zz_generated.deepcopy.go @@ -1018,11 +1018,6 @@ func (in *RoleObservation) DeepCopyInto(out *RoleObservation) { copy(*out, *in) } } - if in.LastPasswordResetToken != nil { - in, out := &in.LastPasswordResetToken, &out.LastPasswordResetToken - *out = new(string) - **out = **in - } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RoleObservation. @@ -1058,9 +1053,9 @@ func (in *RoleParameters) DeepCopyInto(out *RoleParameters) { copy(*out, *in) } } - if in.PasswordResetToken != nil { - in, out := &in.PasswordResetToken, &out.PasswordResetToken - *out = new(string) + if in.PasswordReset != nil { + in, out := &in.PasswordReset, &out.PasswordReset + *out = new(bool) **out = **in } } diff --git a/package/crds/postgresql.sql.crossplane.io_roles.yaml b/package/crds/postgresql.sql.crossplane.io_roles.yaml index 97c84ae5..78d40b37 100644 --- a/package/crds/postgresql.sql.crossplane.io_roles.yaml +++ b/package/crds/postgresql.sql.crossplane.io_roles.yaml @@ -94,12 +94,11 @@ spec: description: ConnectionLimit to be applied to the role. format: int32 type: integer - passwordResetToken: + passwordReset: description: |- - PasswordResetToken can be set to any string to trigger a one-time password reset. - Useful after restoring a database from a snapshot, where roles already exist in the DB but - the connection secret may be empty. Change the token value to trigger another reset. - type: string + 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 @@ -258,11 +257,6 @@ spec: type: string type: object type: array - lastPasswordResetToken: - description: |- - LastPasswordResetToken tracks the passwordResetToken value at which the last password reset was applied. - Used to prevent repeated resets when spec.forProvider.passwordResetToken is set. - type: string privilegesAsClauses: description: |- PrivilegesAsClauses represents the applied privileges state, taking into account diff --git a/package/crds/postgresql.sql.m.crossplane.io_roles.yaml b/package/crds/postgresql.sql.m.crossplane.io_roles.yaml index 158717c7..46c8ef9c 100644 --- a/package/crds/postgresql.sql.m.crossplane.io_roles.yaml +++ b/package/crds/postgresql.sql.m.crossplane.io_roles.yaml @@ -80,12 +80,11 @@ spec: description: ConnectionLimit to be applied to the role. format: int32 type: integer - passwordResetToken: + passwordReset: description: |- - PasswordResetToken can be set to any string to trigger a one-time password reset. - Useful after restoring a database from a snapshot, where roles already exist in the DB but - the connection secret may be empty. Change the token value to trigger another reset. - type: string + 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 @@ -211,11 +210,6 @@ spec: type: string type: object type: array - lastPasswordResetToken: - description: |- - LastPasswordResetToken tracks the passwordResetToken value at which the last password reset was applied. - Used to prevent repeated resets when spec.forProvider.passwordResetToken is set. - type: string privilegesAsClauses: description: |- PrivilegesAsClauses represents the applied privileges state, taking into account diff --git a/pkg/controller/cluster/postgresql/role/reconciler.go b/pkg/controller/cluster/postgresql/role/reconciler.go index b697bc36..0be0b0c2 100644 --- a/pkg/controller/cluster/postgresql/role/reconciler.go +++ b/pkg/controller/cluster/postgresql/role/reconciler.go @@ -352,10 +352,6 @@ func (c *external) Update(ctx context.Context, mg *v1alpha1.Role) (managed.Exter }); err != nil { return managed.ExternalUpdate{}, errors.Wrap(err, errUpdateRole) } - if mg.Spec.ForProvider.PasswordResetToken != nil { - token := *mg.Spec.ForProvider.PasswordResetToken - mg.Status.AtProvider.LastPasswordResetToken = &token - } } privs := privilegesToClauses(mg.Spec.ForProvider.Privileges) diff --git a/pkg/controller/cluster/postgresql/role/reconciler_test.go b/pkg/controller/cluster/postgresql/role/reconciler_test.go index 42b20ba9..23525e51 100644 --- a/pkg/controller/cluster/postgresql/role/reconciler_test.go +++ b/pkg/controller/cluster/postgresql/role/reconciler_test.go @@ -1047,6 +1047,7 @@ func TestGetPassword(t *testing.T) { type args struct { ctx context.Context role *v1alpha1.Role + kube client.Client } type want struct { pwd string @@ -1059,67 +1060,92 @@ func TestGetPassword(t *testing.T) { args args want want }{ - "NoSecretRefNoToken": { - reason: "No password secret ref and no reset token should return unchanged", + "NoSecretRefNoPasswordReset": { + reason: "No password secret ref and no passwordReset should return unchanged", args: args{ role: &v1alpha1.Role{}, }, want: want{pwd: "", changed: false}, }, - "NewPasswordResetToken": { - reason: "A new password reset token with no last token should trigger a reset", + "PasswordResetFalse": { + reason: "passwordReset=false should not trigger a reset", args: args{ role: &v1alpha1.Role{ Spec: v1alpha1.RoleSpec{ ForProvider: v1alpha1.RoleParameters{ - PasswordResetToken: ptr.To("token-1"), + PasswordReset: ptr.To(false), }, }, }, }, - want: want{pwd: "", changed: true}, + want: want{pwd: "", changed: false}, }, - "PasswordResetTokenUnchanged": { - reason: "An unchanged password reset token should not trigger a reset", + "PasswordResetTrueNoConnectionSecretRef": { + reason: "passwordReset=true with no WriteConnectionSecretToReference should not trigger a reset", args: args{ role: &v1alpha1.Role{ Spec: v1alpha1.RoleSpec{ ForProvider: v1alpha1.RoleParameters{ - PasswordResetToken: ptr.To("token-1"), - }, - }, - Status: v1alpha1.RoleStatus{ - AtProvider: v1alpha1.RoleObservation{ - LastPasswordResetToken: ptr.To("token-1"), + PasswordReset: ptr.To(true), }, }, }, }, want: want{pwd: "", changed: false}, }, - "PasswordResetTokenChanged": { - reason: "A changed password reset token should trigger a reset", + "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{ - PasswordResetToken: ptr.To("token-2"), + PasswordReset: ptr.To(true), }, }, - Status: v1alpha1.RoleStatus{ - AtProvider: v1alpha1.RoleObservation{ - LastPasswordResetToken: ptr.To("token-1"), + }, + 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: true}, + want: want{pwd: "", changed: false}, }, } for name, tc := range cases { t.Run(name, func(t *testing.T) { - e := external{kube: nil} + 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) @@ -1134,7 +1160,7 @@ func TestGetPassword(t *testing.T) { } } -func TestUpdatePasswordResetToken(t *testing.T) { +func TestUpdatePasswordReset(t *testing.T) { errBoom := errors.New("boom") type fields struct { @@ -1142,14 +1168,14 @@ func TestUpdatePasswordResetToken(t *testing.T) { } type args struct { - ctx context.Context - mg *v1alpha1.Role + ctx context.Context + mg *v1alpha1.Role + kube client.Client } type want struct { - err error - lastPasswordResetToken *string - passwordGenerated bool + err error + passwordGenerated bool } cases := map[string]struct { @@ -1158,8 +1184,8 @@ func TestUpdatePasswordResetToken(t *testing.T) { args args want want }{ - "NewTokenGeneratesAndSetsPassword": { - reason: "A new password reset token should trigger password generation and update LastPasswordResetToken", + "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{ @@ -1169,20 +1195,26 @@ func TestUpdatePasswordResetToken(t *testing.T) { }, }, Spec: v1alpha1.RoleSpec{ + ResourceSpec: xpv1.ResourceSpec{ + WriteConnectionSecretToReference: &xpv1.SecretReference{ + Name: "connection-secret", + Namespace: "default", + }, + }, ForProvider: v1alpha1.RoleParameters{ - PasswordResetToken: ptr.To("token-1"), + PasswordReset: ptr.To(true), }, }, }, + kube: &test.MockClient{MockGet: test.NewMockGetFn(nil)}, }, want: want{ - err: nil, - lastPasswordResetToken: ptr.To("token-1"), - passwordGenerated: true, + err: nil, + passwordGenerated: true, }, }, - "UnchangedTokenNoPasswordReset": { - reason: "An unchanged password reset token should not trigger any database call", + "PasswordResetTruePasswordExistsNoReset": { + reason: "passwordReset=true with existing password in connection secret should not reset", fields: fields{execQuery: nil}, args: args{ mg: &v1alpha1.Role{ @@ -1192,26 +1224,35 @@ func TestUpdatePasswordResetToken(t *testing.T) { }, }, Spec: v1alpha1.RoleSpec{ + ResourceSpec: xpv1.ResourceSpec{ + WriteConnectionSecretToReference: &xpv1.SecretReference{ + Name: "connection-secret", + Namespace: "default", + }, + }, ForProvider: v1alpha1.RoleParameters{ - PasswordResetToken: ptr.To("token-1"), + PasswordReset: ptr.To(true), }, }, - Status: v1alpha1.RoleStatus{ - AtProvider: v1alpha1.RoleObservation{ - LastPasswordResetToken: ptr.To("token-1"), - }, + }, + 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, - lastPasswordResetToken: ptr.To("token-1"), - passwordGenerated: false, + err: nil, + passwordGenerated: false, }, }, - "ChangedTokenGeneratesNewPassword": { - reason: "A changed password reset token should generate a new password and update LastPasswordResetToken", - fields: fields{execQuery: new(string)}, + "PasswordResetFalseNoReset": { + reason: "passwordReset=false should not trigger any database call", + fields: fields{execQuery: nil}, args: args{ mg: &v1alpha1.Role{ ObjectMeta: v1.ObjectMeta{ @@ -1221,20 +1262,15 @@ func TestUpdatePasswordResetToken(t *testing.T) { }, Spec: v1alpha1.RoleSpec{ ForProvider: v1alpha1.RoleParameters{ - PasswordResetToken: ptr.To("token-2"), - }, - }, - Status: v1alpha1.RoleStatus{ - AtProvider: v1alpha1.RoleObservation{ - LastPasswordResetToken: ptr.To("token-1"), + PasswordReset: ptr.To(false), }, }, }, + kube: &test.MockClient{}, }, want: want{ - err: nil, - lastPasswordResetToken: ptr.To("token-2"), - passwordGenerated: true, + err: nil, + passwordGenerated: false, }, }, } @@ -1253,15 +1289,12 @@ func TestUpdatePasswordResetToken(t *testing.T) { } e := external{ db: db, - kube: &test.MockClient{}, + 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 diff := cmp.Diff(tc.want.lastPasswordResetToken, tc.args.mg.Status.AtProvider.LastPasswordResetToken); diff != "" { - t.Errorf("\n%s\ne.Update(...): -want lastPasswordResetToken, +got lastPasswordResetToken:\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) diff --git a/pkg/controller/cluster/postgresql/role/utils.go b/pkg/controller/cluster/postgresql/role/utils.go index 76306510..9bb89745 100644 --- a/pkg/controller/cluster/postgresql/role/utils.go +++ b/pkg/controller/cluster/postgresql/role/utils.go @@ -63,12 +63,27 @@ func (c *external) getPassword(ctx context.Context, role *v1alpha1.Role) (newPwd return newPwd, changed, nil } - // passwordResetToken triggers a forced password reset when the token changes. - if role.Spec.ForProvider.PasswordResetToken != nil && - (role.Status.AtProvider.LastPasswordResetToken == nil || - *role.Spec.ForProvider.PasswordResetToken != *role.Status.AtProvider.LastPasswordResetToken) { - return "", true, nil - } + changed, err = c.shouldResetPassword(ctx, role) + return "", changed, err +} - return "", false, nil +// 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 false, nil + } + nn := types.NamespacedName{ + Name: role.Spec.WriteConnectionSecretToReference.Name, + Namespace: role.Spec.WriteConnectionSecretToReference.Namespace, + } + s := &corev1.Secret{} + if err := c.kube.Get(ctx, nn, s); resource.IgnoreNotFound(err) != nil { + return false, err + } + 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 d01e43cf..b79999db 100644 --- a/pkg/controller/namespaced/postgresql/role/reconciler.go +++ b/pkg/controller/namespaced/postgresql/role/reconciler.go @@ -336,10 +336,6 @@ func (c *external) Update(ctx context.Context, mg *namespacedv1alpha1.Role) (man }); err != nil { return managed.ExternalUpdate{}, errors.Wrap(err, errUpdateRole) } - if mg.Spec.ForProvider.PasswordResetToken != nil { - token := *mg.Spec.ForProvider.PasswordResetToken - mg.Status.AtProvider.LastPasswordResetToken = &token - } } privs := privilegesToClauses(mg.Spec.ForProvider.Privileges) diff --git a/pkg/controller/namespaced/postgresql/role/reconciler_test.go b/pkg/controller/namespaced/postgresql/role/reconciler_test.go index f3976a24..aa922096 100644 --- a/pkg/controller/namespaced/postgresql/role/reconciler_test.go +++ b/pkg/controller/namespaced/postgresql/role/reconciler_test.go @@ -1110,6 +1110,7 @@ func TestGetPassword(t *testing.T) { type args struct { ctx context.Context role *v1alpha1.Role + kube client.Client } type want struct { pwd string @@ -1122,67 +1123,90 @@ func TestGetPassword(t *testing.T) { args args want want }{ - "NoSecretRefNoToken": { - reason: "No password secret ref and no reset token should return unchanged", + "NoSecretRefNoPasswordReset": { + reason: "No password secret ref and no passwordReset should return unchanged", args: args{ role: &v1alpha1.Role{}, }, want: want{pwd: "", changed: false}, }, - "NewPasswordResetToken": { - reason: "A new password reset token with no last token should trigger a reset", + "PasswordResetFalse": { + reason: "passwordReset=false should not trigger a reset", args: args{ role: &v1alpha1.Role{ Spec: v1alpha1.RoleSpec{ ForProvider: v1alpha1.RoleParameters{ - PasswordResetToken: ptr.To("token-1"), + PasswordReset: ptr.To(false), }, }, }, }, - want: want{pwd: "", changed: true}, + want: want{pwd: "", changed: false}, }, - "PasswordResetTokenUnchanged": { - reason: "An unchanged password reset token should not trigger a reset", + "PasswordResetTrueNoConnectionSecretRef": { + reason: "passwordReset=true with no WriteConnectionSecretToReference should not trigger a reset", args: args{ role: &v1alpha1.Role{ Spec: v1alpha1.RoleSpec{ ForProvider: v1alpha1.RoleParameters{ - PasswordResetToken: ptr.To("token-1"), - }, - }, - Status: v1alpha1.RoleStatus{ - AtProvider: v1alpha1.RoleObservation{ - LastPasswordResetToken: ptr.To("token-1"), + PasswordReset: ptr.To(true), }, }, }, }, want: want{pwd: "", changed: false}, }, - "PasswordResetTokenChanged": { - reason: "A changed password reset token should trigger a reset", + "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{ - PasswordResetToken: ptr.To("token-2"), + PasswordReset: ptr.To(true), }, }, - Status: v1alpha1.RoleStatus{ - AtProvider: v1alpha1.RoleObservation{ - LastPasswordResetToken: ptr.To("token-1"), + }, + 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: true}, + want: want{pwd: "", changed: false}, }, } for name, tc := range cases { t.Run(name, func(t *testing.T) { - e := external{kube: nil} + 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) @@ -1197,7 +1221,7 @@ func TestGetPassword(t *testing.T) { } } -func TestUpdatePasswordResetToken(t *testing.T) { +func TestUpdatePasswordReset(t *testing.T) { errBoom := errors.New("boom") type fields struct { @@ -1206,13 +1230,13 @@ func TestUpdatePasswordResetToken(t *testing.T) { } type args struct { - ctx context.Context - mg *v1alpha1.Role + ctx context.Context + mg *v1alpha1.Role + kube client.Client } type want struct { - err error - lastPasswordResetToken *string + 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 @@ -1224,8 +1248,8 @@ func TestUpdatePasswordResetToken(t *testing.T) { args args want want }{ - "NewTokenGeneratesAndSetsPassword": { - reason: "A new password reset token should trigger password generation and update LastPasswordResetToken", + "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{ @@ -1235,20 +1259,25 @@ func TestUpdatePasswordResetToken(t *testing.T) { }, }, Spec: v1alpha1.RoleSpec{ + ManagedResourceSpec: xpv2.ManagedResourceSpec{ + WriteConnectionSecretToReference: &common.LocalSecretReference{ + Name: "connection-secret", + }, + }, ForProvider: v1alpha1.RoleParameters{ - PasswordResetToken: ptr.To("token-1"), + PasswordReset: ptr.To(true), }, }, }, + kube: &test.MockClient{MockGet: test.NewMockGetFn(nil)}, }, want: want{ - err: nil, - lastPasswordResetToken: ptr.To("token-1"), - passwordGenerated: true, + err: nil, + passwordGenerated: true, }, }, - "UnchangedTokenNoPasswordReset": { - reason: "An unchanged password reset token should not trigger any database call", + "PasswordResetTruePasswordExistsNoReset": { + reason: "passwordReset=true with existing password in connection secret should not reset", fields: fields{execQuery: nil}, args: args{ mg: &v1alpha1.Role{ @@ -1258,26 +1287,34 @@ func TestUpdatePasswordResetToken(t *testing.T) { }, }, Spec: v1alpha1.RoleSpec{ + ManagedResourceSpec: xpv2.ManagedResourceSpec{ + WriteConnectionSecretToReference: &common.LocalSecretReference{ + Name: "connection-secret", + }, + }, ForProvider: v1alpha1.RoleParameters{ - PasswordResetToken: ptr.To("token-1"), + PasswordReset: ptr.To(true), }, }, - Status: v1alpha1.RoleStatus{ - AtProvider: v1alpha1.RoleObservation{ - LastPasswordResetToken: ptr.To("token-1"), - }, + }, + 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, - lastPasswordResetToken: ptr.To("token-1"), - passwordGenerated: false, + err: nil, + passwordGenerated: false, }, }, - "ChangedTokenGeneratesNewPassword": { - reason: "A changed password reset token should generate a new password and update LastPasswordResetToken", - fields: fields{execQuery: new(string)}, + "PasswordResetFalseNoReset": { + reason: "passwordReset=false should not trigger any database call", + fields: fields{execQuery: nil}, args: args{ mg: &v1alpha1.Role{ ObjectMeta: metav1.ObjectMeta{ @@ -1287,20 +1324,15 @@ func TestUpdatePasswordResetToken(t *testing.T) { }, Spec: v1alpha1.RoleSpec{ ForProvider: v1alpha1.RoleParameters{ - PasswordResetToken: ptr.To("token-2"), - }, - }, - Status: v1alpha1.RoleStatus{ - AtProvider: v1alpha1.RoleObservation{ - LastPasswordResetToken: ptr.To("token-1"), + PasswordReset: ptr.To(false), }, }, }, + kube: &test.MockClient{}, }, want: want{ - err: nil, - lastPasswordResetToken: ptr.To("token-2"), - passwordGenerated: true, + err: nil, + passwordGenerated: false, }, }, } @@ -1320,15 +1352,12 @@ func TestUpdatePasswordResetToken(t *testing.T) { } e := external{ db: db, - kube: &test.MockClient{}, + 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 diff := cmp.Diff(tc.want.lastPasswordResetToken, tc.args.mg.Status.AtProvider.LastPasswordResetToken); diff != "" { - t.Errorf("\n%s\ne.Update(...): -want lastPasswordResetToken, +got lastPasswordResetToken:\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 == "" { diff --git a/pkg/controller/namespaced/postgresql/role/utils.go b/pkg/controller/namespaced/postgresql/role/utils.go index 3b2a9f92..aa5eae26 100644 --- a/pkg/controller/namespaced/postgresql/role/utils.go +++ b/pkg/controller/namespaced/postgresql/role/utils.go @@ -63,12 +63,27 @@ func (c *external) getPassword(ctx context.Context, role *v1alpha1.Role) (newPwd return newPwd, changed, nil } - // passwordResetToken triggers a forced password reset when the token changes. - if role.Spec.ForProvider.PasswordResetToken != nil && - (role.Status.AtProvider.LastPasswordResetToken == nil || - *role.Spec.ForProvider.PasswordResetToken != *role.Status.AtProvider.LastPasswordResetToken) { - return "", true, nil - } + changed, err = c.shouldResetPassword(ctx, role) + return "", changed, err +} - return "", false, nil +// 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 false, nil + } + nn := types.NamespacedName{ + Name: role.Spec.WriteConnectionSecretToReference.Name, + Namespace: role.Namespace, + } + s := &corev1.Secret{} + if err := c.kube.Get(ctx, nn, s); resource.IgnoreNotFound(err) != nil { + return false, err + } + return len(s.Data[xpv1.ResourceCredentialsSecretPasswordKey]) == 0, nil }