-
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
feat(cli): Extend CLI with v1 authorization commands #19829
Conversation
@gavincabbage the team-assigner gods picked you! |
e6a2ab6
to
af239cd
Compare
@gavincabbage I'll request a review when this is ready, as we need to make a few additional changes |
af239cd
to
c68c46d
Compare
This commit add a series of new subcommands to the `influx` CLI tool. * `influx v1` to host any InfluxDB v1 specific commands * `influx v1 auth` for v1 authorization token management. The commands provide access to the private authorization APIs used to manage authorization tokens for v1 compatibility APIs, including: * `/write` * `/query` Closes #19765
Remove remnants of previous token implementation * As of #19840, V1 API tokens will be authorized using bcrypt passwords
* Added capability to HTTP API to search for tokens by ID or Token
c68c46d
to
eb33c0f
Compare
@gavincabbage this PR is ready for review |
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.
Looks good Stuart- just wanted to ask, would it be appropriate to add some tests for all the business logic in v1_authorization.go
?
I'm not really sure we have established a way to test CLI commands and I don't see precedent for testing any of the other ones. We do test the HTTP APIs that the CLI commands call, however. |
Well, it looks like influxdb/cmd/influx/bucket_test.go Line 33 in 6bc4158
|
I'll take a closer look at the bucket tests and verify what they are testing – if they add value, I'll do the same. |
@gavincabbage I'm going to add a follow up task to investigate refactoring these commands so they are testable. The v1_authorization command was copied from |
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.
Great, sounds good @stuartcarnie. I just made a similar call to skip testing something difficult to test until I refactor it to make it testable, so I know how that goes 😅
Closes #19765
This commit add a series of new subcommands to the
influx
CLI tool. The specific commands are:influx v1
to host any InfluxDB v1 specific commandsinflux v1 auth
for v1 authorization token management.influx v1 auth set-password
to assign a password to a token for V1 API authorizationThe
v1 auth
commands enable access to the private authorization APIs (see #19815 and #19837) used to manage authorization tokens for v1 compatibility APIs, including:/write
/query
The final issue, #19768 will update the authorization handlers to use these new tokens and their assigned passwords.