issue-135, saving and updating the PostgreSQL default password were implemented #447

DoodgeMatvey · 2023-06-29T11:18:13Z

Closes #446
We need to get default user password and store it in secret to be able to create new PostgreSQL user.
In this Pr, I updated the current logic for creating secret with default user password and username when cluster is creating.
Also, fixed and improved logic for updating default password when secret is updated.

RostislavPorohnya · 2023-06-29T11:26:24Z

apis/clusters/v1alpha1/postgresql_types.go

 }

 func (pg *PostgreSQL) GetUserSecret(ctx context.Context, k8sClient client.Client) (*k8sCore.Secret, error) {
 	userSecret := &k8sCore.Secret{}
 	userSecretNamespacedName := types.NamespacedName{
-		Name:      pg.Status.DefaultUserSecretName,
+		Name:      models.DefaultUserSecretPrefix + pg.Name,


What if user already has secret with this name that is not related to pg cluster?

We work only with secrets that have DefaultSecretLabel == "true", I described it in findSecretObject method

And I don't think there will be such name default-user-password-$postgresClusterName

RostislavPorohnya · 2023-06-29T11:27:09Z

apis/clusters/v1alpha1/postgresql_types.go

@@ -356,9 +355,15 @@ func (pg *PostgreSQL) NewUserSecret() *k8sCore.Secret {
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      models.DefaultUserSecretPrefix + pg.Name,


If user already has a secret with the same name it can be an issue

I answered in the previous comment, I handled it

RostislavPorohnya · 2023-06-29T11:34:58Z

controllers/clusters/postgresql_controller.go

+	err = r.createDefaultPassword(pg, logger)
+	if err != nil {
+		logger.Error(err, "Cannot create default password for PostgreSQL",
+			"cluster name", pg.Spec.Name,
+			"clusterID", pg.Status.ID,
+		)
+
+		return models.ReconcileRequeue
 	}

 	return models.ExitReconcile


You set UpdatingEvent in the createPassword(), but use exitReconcile.
I suggest you either don't set any events in the createPassword() or set an updatedEvent

RostislavPorohnya · 2023-06-29T11:37:58Z

controllers/clusters/postgresql_controller.go

+	case models.SecretEvent:
+		return r.handleUpdateDefaultUserPassword(ctx, pg, logger), nil


Please move the handleUpdatePassword() to the handleUpdate() to prevent interuption of the secret event by update event

We need to update secret only if it is secret event I think

RostislavPorohnya · 2023-06-29T11:43:02Z

controllers/clusters/postgresql_controller.go

+	if s.Labels[models.DefaultSecretLabel] != "true" {
+		return []reconcile.Request{}
+	}


I think it is better to make this label resource-related, e.g. pgDefaultSecretLabel, so this condition can filter secrets from other resources

We won't use or store default passwords for other entities, this logic will be only for postgresql

RostislavPorohnya · 2023-06-29T11:44:22Z

pkg/models/postgresql_apiv2.go

@@ -22,6 +22,7 @@ type PGCluster struct {
 	PostgreSQLVersion             string             `json:"postgresqlVersion"`
 	DataCentres                   []*PGDataCentre    `json:"dataCentres"`
 	SynchronousModeStrict         bool               `json:"synchronousModeStrict"`
+	DefaultUserPassword           string             `json:"defaultUserPassword,omitempty"`


Don't store password in the spec

Agree, fixed

testisnullus · 2023-06-29T17:28:20Z

controllers/clusters/postgresql_controller.go

+		logger.Error(err, "Cannot create default password for PostgreSQL",
+			"cluster name", pg.Spec.Name,
+			"clusterID", pg.Status.ID,
+		)


Let's add an event (r.EventRecorder.Eventf) message here

Agree, added for all such cases

testisnullus · 2023-06-29T17:31:28Z

controllers/clusters/postgresql_controller.go

+		logger.Error(err, "PostgreSQL cannot get user secret",
+			"cluster name", pg.Spec.Name,
+			"cluster ID", pg.Status.ID,
+		)


Let's change the error message here, like PostgreSQL cannot get user secret -> Cannot get the user secret for the PostgreSQL cluster
and add an event

controllers/clusters/postgresql_controller.go

testisnullus · 2023-06-29T17:41:49Z

controllers/clusters/postgresql_controller.go

+
+	defaultUserPassword, err := pg.DefaultPasswordFromInstAPI(iData)
+	if err != nil {
+		l.Error(err, "Cannot convert PostgreSQL cluster status from Instaclustr",


Cannot get default user creds for PostgreSQL cluster from the Instaclustr API maybe?

ribaraka · 2023-06-30T11:27:48Z

controllers/clusters/postgresql_controller.go

+	}
+
+	if secret != nil {
+		return nil


I suggest you add here a log info that a secret already exists.

RostislavPorohnya · 2023-07-04T14:46:35Z

apis/clusters/v1beta1/postgresql_types.go

 }

 func (pg *PostgreSQL) GetUserSecret(ctx context.Context, k8sClient client.Client) (*k8sCore.Secret, error) {
 	userSecret := &k8sCore.Secret{}
 	userSecretNamespacedName := types.NamespacedName{
-		Name:      pg.Status.DefaultUserSecretName,
+		Name:      models.DefaultUserSecretPrefix + pg.Name,


Use printf and some constant format to have the same name in different parts of code

…mplemented

DoodgeMatvey added the enhancement New feature or request label Jun 29, 2023

DoodgeMatvey requested review from RostislavPorohnya, ribaraka and testisnullus June 29, 2023 11:18

DoodgeMatvey self-assigned this Jun 29, 2023

DoodgeMatvey mentioned this pull request Jun 19, 2023

Support access to Instaclustr applications #430

Closed

7 tasks

RostislavPorohnya requested changes Jun 29, 2023

View reviewed changes

DoodgeMatvey force-pushed the issue-135 branch from 9f58395 to 844d5c1 Compare June 29, 2023 12:10

DoodgeMatvey requested a review from RostislavPorohnya June 29, 2023 12:15

testisnullus requested changes Jun 29, 2023

View reviewed changes

DoodgeMatvey force-pushed the issue-135 branch from 844d5c1 to 56f79f3 Compare June 30, 2023 09:04

DoodgeMatvey requested a review from testisnullus June 30, 2023 09:12

ribaraka reviewed Jun 30, 2023

View reviewed changes

ribaraka approved these changes Jun 30, 2023

View reviewed changes

DoodgeMatvey force-pushed the issue-135 branch from 56f79f3 to 28900ad Compare June 30, 2023 11:46

testisnullus approved these changes Jun 30, 2023

View reviewed changes

DoodgeMatvey force-pushed the issue-135 branch from 28900ad to 3868bc2 Compare July 4, 2023 13:52

RostislavPorohnya reviewed Jul 4, 2023

View reviewed changes

issue-135, saving and updating the PostgreSQL default password were i…

7bc87d9

…mplemented

DoodgeMatvey force-pushed the issue-135 branch from 3868bc2 to 7bc87d9 Compare July 4, 2023 14:58

taaraora requested a review from RostislavPorohnya July 5, 2023 09:40

taaraora approved these changes Jul 5, 2023

View reviewed changes

taaraora merged commit 582cf36 into main Jul 5, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

issue-135, saving and updating the PostgreSQL default password were implemented #447

issue-135, saving and updating the PostgreSQL default password were implemented #447

DoodgeMatvey commented Jun 29, 2023 •

edited

Loading

RostislavPorohnya Jun 29, 2023

DoodgeMatvey Jun 29, 2023

DoodgeMatvey Jun 29, 2023

RostislavPorohnya Jun 29, 2023

DoodgeMatvey Jun 29, 2023

RostislavPorohnya Jun 29, 2023

DoodgeMatvey Jun 29, 2023

RostislavPorohnya Jun 29, 2023

DoodgeMatvey Jun 29, 2023

RostislavPorohnya Jun 29, 2023

DoodgeMatvey Jun 29, 2023

RostislavPorohnya Jun 29, 2023

DoodgeMatvey Jun 29, 2023

testisnullus Jun 29, 2023

DoodgeMatvey Jun 30, 2023

testisnullus Jun 29, 2023

DoodgeMatvey Jun 30, 2023

testisnullus Jun 29, 2023

DoodgeMatvey Jun 30, 2023

ribaraka Jun 30, 2023

DoodgeMatvey Jun 30, 2023

RostislavPorohnya Jul 4, 2023

DoodgeMatvey Jul 4, 2023

		@@ -356,9 +355,15 @@ func (pg PostgreSQL) NewUserSecret() k8sCore.Secret {
		ObjectMeta: metav1.ObjectMeta{
		Name: models.DefaultUserSecretPrefix + pg.Name,

		case models.SecretEvent:
		return r.handleUpdateDefaultUserPassword(ctx, pg, logger), nil

issue-135, saving and updating the PostgreSQL default password were implemented #447

issue-135, saving and updating the PostgreSQL default password were implemented #447

Conversation

DoodgeMatvey commented Jun 29, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DoodgeMatvey commented Jun 29, 2023 •

edited

Loading