Fix Issue 227: Generate new PostgreSQL password for role after restoration

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.

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.

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

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

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 {

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")