-
Notifications
You must be signed in to change notification settings - Fork 59
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
Get acls subcommand #150
Get acls subcommand #150
Conversation
This reverts commit 7b8ee42.
// We need to subtype this to be able to define methods to | ||
// satisfy the Value interface from Cobra so we can use it | ||
// as a Cobra flag. | ||
type ResourceType kafka.ResourceType |
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.
These types allow us to convert between kafka-go types (most of which are int8s that represent something in the Kafka API) and human read able forms of the types for the user to provide through the CLI or see displayed as output
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. A cosmetic suggestion for your help strings that list allowed values: quoting each makes it harder to read in my opinion, how about something like this instead?
The permission type to filter on. allowed: [any, allow, deny]
Need to add this to the repl |
@hhahn-tw since your last review I added repl support, bumped to the released version of kafka-go and changed |
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.
Text style change request "acl" -> "ACL" in info output to user
pkg/cli/repl.go
Outdated
@@ -39,6 +39,10 @@ var ( | |||
} | |||
|
|||
getSuggestions = []prompt.Suggest{ | |||
{ | |||
Text: "acls", | |||
Description: "Get all acls", |
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.
"acls" should be capitalized here and below at line 471 for consistency with other text output
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.
Good catch, fixed
This PR adds the
get acls
subcommand. It's behavior was based heavily on Kafka's built in kafka-acls.sh --list script.It supports filtering on various fields so that users can answer questions related to which services have access to which
resources.
This PR also fixes flaky test failures like this one by adding some sleep time in a test
Examples: