Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

grafana-cli: Add ability to read password from stdin to reset admin password #26016

Merged
merged 7 commits into from
Jul 2, 2020
Merged

grafana-cli: Add ability to read password from stdin to reset admin password #26016

merged 7 commits into from
Jul 2, 2020

Conversation

nabokihms
Copy link
Contributor

@nabokihms nabokihms commented Jul 2, 2020 •
edited by aknuds1
Loading

Signed-off-by: m.nabokikh maksim.nabokikh@flant.com

What this PR does / why we need it:
This PR adds the ability to securely read the password from stdin, either typing or sending it via the pipe. It helps to use the reset-admin-password command in scripts.

Which issue(s) this PR fixes:
Fixes #14076

Special notes for your reviewer:
I understand that the issue is 2 years old, but in my opinion, it deserves to be fixed.

This PR also covers cases to read the password from a file or from an env variable:

echo $MY_PASSWORD | grafana-cli admin reset-admin-password --passwordFromStdin
cat ./passwords.txt | grafana-cli admin reset-admin-password --passwordFromStdin

@nabokihms
grafana-cli: Add ability to read password from stdin to reset admin p…
e37912f 
…assword

Signed-off-by: m.nabokikh <maksim.nabokikh@flant.com>
@nabokihms nabokihms requested a review from a team as a code owner July 2, 2020 11:54
@nabokihms nabokihms requested review from papagian and removed request for a team July 2, 2020 11:54
@marefr marefr added area/backend pr/external This PR is from external contributor labels Jul 2, 2020
@marefr marefr requested a review from aknuds1 July 2, 2020 11:57
@nabokihms @aknuds1
Update pkg/cmd/grafana-cli/commands/reset_password_command.go
45ba1cf 
Co-authored-by: Arve Knudsen <arve.knudsen@gmail.com>
@bergquist
Copy link
Contributor

Sounds good. Previous work on this have been introducing breaking changes. This doesn't. 👍
While it should be the other way around I don't think it's worth a breaking change.

@aknuds1
Copy link
Contributor

aknuds1 commented Jul 2, 2020 •
edited
Loading

When I try your branch, I don't see the new option? What am I missing?

$ ./bin/grafana-cli admin reset-admin-password -h                 
NAME:
   Grafana CLI admin reset-admin-password - reset-admin-password <new password>

USAGE:
   Grafana CLI admin reset-admin-password [command options] [arguments...]

OPTIONS:
   --help, -h  show help (default: false)

aknuds1
aknuds1 requested changes Jul 2, 2020
View reviewed changes
pkg/cmd/grafana-cli/commands/commands.go Outdated Show resolved Hide resolved
pkg/cmd/grafana-cli/commands/reset_password_command.go Outdated Show resolved Hide resolved
@nabokihms
Copy link
Contributor Author

Oh, maybe it is me who missed something...

I tested grafana-cli by running:

make build-cli
./bin/darwin-amd64/grafana-cli admin reset-admin-password --help

NAME:
   Grafana CLI admin reset-admin-password - reset-admin-password <new password>

USAGE:
   Grafana CLI admin reset-admin-password [command options] [arguments...]

OPTIONS:
   --passwordFromStdin  Read the password from stdin (default: false)
   --help, -h           show help (default: false)

@aknuds1
Copy link
Contributor

aknuds1 commented Jul 2, 2020

Oh no, the mistake was actually on my side. For some reason, some make commands produce bin/grafana-* and others bin/linux-amd64/grafana-*. It's the latter I should have been using. I can see your flag now.

nabokihms and others added 2 commits July 2, 2020 17:37
@nabokihms @aknuds1
Update pkg/cmd/grafana-cli/commands/commands.go
75cd161 
Co-authored-by: Arve Knudsen <arve.knudsen@gmail.com>
@nabokihms @aknuds1
Update pkg/cmd/grafana-cli/commands/reset_password_command.go
70576d6 
Co-authored-by: Arve Knudsen <arve.knudsen@gmail.com>
nabokihms and others added 2 commits July 2, 2020 17:49
@nabokihms @aknuds1
Update pkg/cmd/grafana-cli/commands/reset_password_command.go
dca5499 
Co-authored-by: Arve Knudsen <arve.knudsen@gmail.com>
@nabokihms
Migrate to bufio.NewScanner
8e693ef 
Signed-off-by: m.nabokikh <maksim.nabokikh@flant.com>
aknuds1
aknuds1 requested changes Jul 2, 2020
View reviewed changes
Copy link
Contributor

@aknuds1 aknuds1 left a comment

Choose a reason for hiding this comment

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

Sorry, last one I promise. I just realized the correlation between scanner.Scan() returning false and scanner.Err()

pkg/cmd/grafana-cli/commands/reset_password_command.go Outdated Show resolved Hide resolved
@nabokihms @aknuds1
Update pkg/cmd/grafana-cli/commands/reset_password_command.go
b4f4000 
Co-authored-by: Arve Knudsen <arve.knudsen@gmail.com>
aknuds1
aknuds1 approved these changes Jul 2, 2020
View reviewed changes
Copy link
Contributor

@aknuds1 aknuds1 left a comment

Choose a reason for hiding this comment

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

LGTM!

@aknuds1 aknuds1 added this to the 7.2 milestone Jul 2, 2020
@aknuds1 aknuds1 merged commit 3720c25 into grafana:master Jul 2, 2020
@aknuds1
Copy link
Contributor

aknuds1 commented Jul 2, 2020

Thanks for contributing to Grafana!

@mckn mckn mentioned this pull request Sep 9, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aknuds1 aknuds1 aknuds1 approved these changes

@papagian papagian Awaiting requested review from papagian

Assignees
No one assigned
Labels
add to changelog area/backend pr/external This PR is from external contributor
Projects
None yet
Milestone
7.2.0-beta1
Development

Successfully merging this pull request may close these issues.

grafana-cli admin password reset should get the password from stdin
4 participants
@nabokihms @bergquist @aknuds1 @marefr