-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
fix: Improve the UI for the influx v1 auth commands #19935
Conversation
The enhancements and fixes to the user experience are as follows: * Rename active and inactive commands to the actions set-active and set-inactive respectively to clarify their behavior To set a token to active: ``` influx v1 auth set-active ``` To set a token to inactive, preventing its use: ``` influx v1 auth set-inactive ``` * allow `--id` or `--username` anywhere a single authentication token is required * use separate vars to store the flag state of each command * factor out the behavior to find a single authentication token into a shared function, `v1FindOneAuthorization`
cmd/influx/v1_authorization.go
Outdated
@@ -479,9 +519,78 @@ func v1AuthorizationInactiveF(cmd *cobra.Command, args []string) error { | |||
}) | |||
} | |||
|
|||
var ( | |||
errMultipleMarchingAuthorizations = errors.New("multiple authorizations found") |
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.
errMultipleMarchingAuthorizations = errors.New("multiple authorizations found") | |
errMultipleMatchingAuthorizations = errors.New("multiple authorizations found") |
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.
Thanks! I am not used to typing on this laptop keyboard – one of the joys of being in quarantine in Australia right now, and not having access to my external keyboard 🤣
cmd/influx/v1_authorization.go
Outdated
func (f *v1AuthLookupFlags) validate() error { | ||
switch { | ||
case f.id.Valid() && f.username != "": | ||
return errors.New("specify id or username") |
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.
Could we reword the error to make it more clear that it's an XOR? At first glance I wasn't sure why this was an error.
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.
How about
specify id or username, not both
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.
LGTM
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.
assuming CI passes
Closes #19933
This PR improves the
v1 auth
commands of theinflux
CLI tool, and includes some minor improvements to thekit/cli
package. The enhancements and fixes to the user experience are as follows:Rename
--auth-token
to--username
for thev1 auth create
command to clarify its purpose in the context of v1 requestsv1 auth create
now requires a password when creating a new authorisation. It can be optionally disabled with the--no-password
flag, however, the user much set a password using thev1 auth set-password
command prior to using the token.Rename
v1 auth active
andv1 auth inactive
commands to the actionsset-active
andset-inactive
respectively to clarify their behaviour.For example, to set a token to active:
To set a token to inactive, preventing its use in subsequent requests:
Universally allow the
--id
or--username
flags anywhere a single authentication token isrequired
Use separate global variables to store the flag state of each v1 auth command
factor out the behavior to find a single authentication token into
a shared function,
v1FindOneAuthorization