feat(cmd): add database user password update #1036

curzolapierre · 2024-02-09T17:16:02Z

Add a changelog entry in the section "To Be Released" of CHANGELOG.md
Need feat(database): add database user password update route go-scalingo#373 to be released

Examples

Max retry reached

➜ scalingo --app my-app --addon mysql database-users-update-password my_user
Password (Will be generated if left empty): 
Password Confirmation: 
 !     Password confirmation doesn't match. Remaining retries: 2
Password (Will be generated if left empty): 
Password Confirmation: 
 !     Password confirmation doesn't match. Remaining retries: 1
Password (Will be generated if left empty): 
Password Confirmation: 
 !     Password confirmation doesn't match. Too many retries

User does not exists

➜ scalingo --app my-app --addon mysql database-users-update-password user_does_not_exists
 !     An error occurred:
       User "user_does_not_exists" does not exist

It works after a retry

➜ scalingo --app my-app --addon mysql database-users-update-password my_user
Password (Will be generated if left empty): 
Password Confirmation: 
 !     Password confirmation doesn't match. Remaining retries: 2
Password (Will be generated if left empty): 
Password Confirmation: 
User "my_user" password updated.

It works generates a password

➜ scalingo --app my-app --addon mysql database-users-update-password my_user
Password (Will be generated if left empty): 
Password Confirmation: 
User "my_user" updated with password "this_is_a_generated_password".

Fixes #1034

notion-workspace · 2024-02-21T09:31:28Z

🗣️ User can update a database user password

ipfaze

Some suggestions otherwise LGTM

ipfaze · 2024-02-23T08:46:15Z

db/users/update.go

+		return nil
+	}
+
+	password, confirmedPassword, err := askForPassword(ctx)


suggestion: allow client to retry 3 times to set his own password

Yep why not

ipfaze · 2024-02-23T08:48:08Z

db/users/update.go

+
+	isPasswordGenerated := false
+	if password == "" {
+		isPasswordGenerated = true


suggestion: display a message that password will be generated because left empty or max retry attempts reached

It's already handled by askForPassword or you're looking for something else?

Message in askForPassword just say that if the client let it empty it will be generated but if he tries 3 times and fail, we will generate it for him without any message to inform him.
Do you see what I mean?

I'm not really sure what you means.

But I'm not sure we want to change the behavior as it is what the API expects actually, empty password and confirmation to make it generate one: https://developers.scalingo.com/databases/users

I've updated the description of this PR with an example of generated password. It is more clear and/or ok for you?

Yup, sorry misread your code 🙁

No problem :)

ipfaze · 2024-02-23T08:54:31Z

cmd/databases.go

+			&addonFlag,
+		},
+		Description: CommandDescription{
+			Description: `Update a non-protected database user's password.


I think this sentence may be misunderstood, like the "database is non-protected" when it's the user who is non-protected.

I suggest:

Suggested change

Description: `Update a non-protected database user's password.

Description: `Update password for unprotected database user.

WDYT?

ipfaze · 2024-02-23T08:56:14Z

db/users/create.go

@@ -10,14 +10,14 @@ import (
 	"github.com/Scalingo/cli/config"
 	"github.com/Scalingo/cli/io"
 	"github.com/Scalingo/go-scalingo/v6"
-	scErrors "github.com/Scalingo/go-utils/errors/v2"


praise: thanks for handling this 🙏

ipfaze

One remaining suggestion otherwise LGTM 👍

ipfaze

LGTM 👍

ipfaze · 2024-02-23T13:50:56Z

db/users/update.go

+
+	isPasswordGenerated := false
+	if password == "" {
+		isPasswordGenerated = true


Yup, sorry misread your code 🙁

curzolapierre self-assigned this Feb 9, 2024

curzolapierre changed the title ~~fix(style): correct errorCtx variable~~ feat(cmd): add database user password update Feb 12, 2024

fix(style): correct errorCtx variable

0026672

curzolapierre force-pushed the feat/1034/update_database_user_password branch 2 times, most recently from d5e057e to e50d2ea Compare February 20, 2024 17:22

curzolapierre marked this pull request as ready for review February 23, 2024 08:36

curzolapierre requested review from ipfaze and leo-scalingo February 23, 2024 08:40

leo-scalingo approved these changes Feb 23, 2024

View reviewed changes

ipfaze reviewed Feb 23, 2024

View reviewed changes

curzolapierre added 3 commits February 23, 2024 12:04

feat(cmd): add database user password update

20765a3

chore(deps): upgrade go-scalingo from v6.7.6 to v6.7.7

435af4a

feat(cmd): add a retry when ask for password

f18a31c

curzolapierre force-pushed the feat/1034/update_database_user_password branch from 7ca53ff to f18a31c Compare February 23, 2024 11:08

curzolapierre requested a review from ipfaze February 23, 2024 11:09

fix(cmd): database user create checks if user already exists

23fbd87

ipfaze reviewed Feb 23, 2024

View reviewed changes

curzolapierre requested a review from ipfaze February 23, 2024 13:44

ipfaze approved these changes Feb 23, 2024

View reviewed changes

db/users/update.go

isPasswordGenerated := false

if password == "" {

isPasswordGenerated = true

Copy link

Contributor

ipfaze Feb 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, sorry misread your code 🙁

curzolapierre merged commit 2b8d414 into master Feb 23, 2024
6 checks passed

curzolapierre deleted the feat/1034/update_database_user_password branch February 23, 2024 13:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(cmd): add database user password update #1036

feat(cmd): add database user password update #1036

curzolapierre commented Feb 9, 2024 •

edited

Loading

notion-workspace bot commented Feb 21, 2024

ipfaze left a comment

ipfaze Feb 23, 2024

curzolapierre Feb 23, 2024

ipfaze Feb 23, 2024

curzolapierre Feb 23, 2024

ipfaze Feb 23, 2024

curzolapierre Feb 23, 2024

ipfaze Feb 23, 2024

curzolapierre Feb 23, 2024 •

edited

Loading

ipfaze Feb 23, 2024

ipfaze Feb 23, 2024

ipfaze left a comment

ipfaze left a comment

ipfaze Feb 23, 2024

	Description: `Update a non-protected database user's password.
	Description: `Update password for unprotected database user.

feat(cmd): add database user password update #1036

feat(cmd): add database user password update #1036

Conversation

curzolapierre commented Feb 9, 2024 • edited Loading

Examples

notion-workspace bot commented Feb 21, 2024

ipfaze left a 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

curzolapierre Feb 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ipfaze left a comment

Choose a reason for hiding this comment

ipfaze left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

curzolapierre commented Feb 9, 2024 •

edited

Loading

curzolapierre Feb 23, 2024 •

edited

Loading