-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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.
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
Perform token validation as part of 'k6 cloud login' #3930
Conversation
@@ -90,6 +104,9 @@ func (c *cmdCloudLogin) run(cmd *cobra.Command, _ []string) error { | |||
newCloudConf.Token = null.StringFromPtr(nil) | |||
printToStdout(c.globalState, " token reset\n") | |||
case show.Bool: | |||
valueColor := getColor(c.globalState.Flags.NoColor || !c.globalState.Stdout.IsTTY, color.FgCyan) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I moved these lines here, from the bottom of the file, as I think it only makes sense to print the token when providing the -s
flag, as also discussed at #3886.
Otherwise, it doesn't, because:
- When resetting, with
-r
, there's no token. - When providing it with
-t
, it's already there. - When using the form, we define it as a sensitive field (so it's not displayed while being written).
If we want to show it in the last scenario, I'd just switch the form input type to regular text, otherwise (as it was until now) it is a bit incoherent.
a7064cc
to
2324996
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I share most of the comments brought up so far, but consider those potential improvements rather than blockers.
632f80c
to
b3778a3
Compare
b3778a3
to
cdbca39
Compare
d6ce425
What?
It performs token validation after as part of
k6 cloud login
process.Why?
As discussed in #3886, it's nice to give immediate feedback to users when "logging-in", instead of waiting until they use any other cloud command.
Checklist
make lint
) and all checks pass.make tests
) and all tests pass.Related PR(s)/Issue(s)