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.

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

Merged
merged 63 commits into from
Oct 11, 2023
Merged

Get acls subcommand #150

merged 63 commits into from
Oct 11, 2023

Conversation

petedannemann
Copy link
Contributor

@petedannemann petedannemann commented Sep 15, 2023

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:

$ topicctl get acls --cluster-config examples/local-cluster/cluster.yaml --help
Displays information for ACLs in the cluster. Supports filtering with flags.

Usage:
  topicctl get acls [flags]

Examples:
List all acls
$ topicctl get acls

List read acls for topic my-topic
$ topicctl get acls --resource-type topic --resource-name my-topic --operations read

List acls for user Alice with permission allow
$ topicctl get acls --principal User:alice --permission-type allow

List acls for host 198.51.100.0
$ topicctl get acls --host 198.51.100.0


Flags:
  -h, --help                                help for acls
      --host string                         The host to filter on. (e.g. 198.51.100.0)
      --operation ACLOperationType          The operation that is being allowed or denied to filter on. allowed: "any", "all", "read", "write", "create", "delete", "alter", "describe", "clusteraction", "describeconfigs", "alterconfigs" or "idempotentwrite" (default any)
      --permission-type ACLPermissionType   The permission type to filter on. allowed: "any", "allow", or "deny" (default any)
      --principal string                    The principal to filter on in principalType:name format (e.g. User:alice).
      --resource-name string                The resource name to filter on. (e.g. my-topic)
      --resource-pattern-type PatternType   The type of the resource pattern or filter. allowed: "any", "match", "literal", "prefixed". "any" will match any pattern type (literal or prefixed), but will match the resource name exactly, where as "match" will perform pattern matching to list all acls that affect the supplied resource(s). (default any)
      --resource-type ResourceType          The type of resource to filter on. allowed: "any", "topic", "group", "cluster", "transactionalid", "delegationtoken" (default any)
$ topicctl get acls --cluster-config examples/local-cluster/cluster.yaml
[2023-09-15 18:28:46]  INFO ACLs:
----------------+--------------+---------------+------------+---------+-----------+------------------
  RESOURCE TYPE | PATTERN TYPE | RESOURCE NAME | PRINCIPAL  |  HOST   | OPERATION | PERMISSION TYPE
----------------+--------------+---------------+------------+---------+-----------+------------------
  group         | literal      | junk3         | User:foo5  | 1.2.3.4 | all       | deny
  group         | literal      | junk3         | User:foo5  | 1.2.3.7 | all       | deny
  group         | literal      | junk3         | User:foo2  | 1.2.3.4 | read      | allow
  topic         | prefixed     | team1_        | User:team2 | *       | all       | deny
  topic         | literal      | junk          | User:foo5  | 1.2.3.4 | all       | deny
  topic         | literal      | junk          | User:foo2  | 1.2.3.4 | read      | allow
  topic         | literal      | junk          | User:foo2  | 1.2.3.4 | describe  | allow
  topic         | literal      | junk          | User:foo5  | 1.2.3.7 | all       | deny
  topic         | literal      | foo           | User:alice | *       | describe  | allow
  topic         | literal      | foo           | User:alice | *       | create    | allow
  topic         | literal      | foo           | User:alice | *       | write     | allow
----------------+--------------+---------------+------------+---------+-----------+------------------
$ topicctl get acls --cluster-config examples/local-cluster/cluster.yaml --resource-type topic --resource-pattern-type literal --resource-name junk --permission-type allow --operation describe
[2023-09-15 18:28:30]  INFO ACLs:
----------------+--------------+---------------+-----------+---------+-----------+------------------
  RESOURCE TYPE | PATTERN TYPE | RESOURCE NAME | PRINCIPAL |  HOST   | OPERATION | PERMISSION TYPE
----------------+--------------+---------------+-----------+---------+-----------+------------------
  topic         | literal      | junk          | User:foo2 | 1.2.3.4 | describe  | allow
----------------+--------------+---------------+-----------+---------+-----------+------------------

@petedannemann petedannemann marked this pull request as ready for review September 15, 2023 20:15
@petedannemann petedannemann requested a review from a team as a code owner September 15, 2023 20:15
// 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
Copy link
Contributor Author

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

Copy link
Contributor

@hhahn-tw hhahn-tw left a 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]

@petedannemann
Copy link
Contributor Author

Need to add this to the repl

@petedannemann
Copy link
Contributor Author

@hhahn-tw since your last review I added repl support, bumped to the released version of kafka-go and changed CreateACL to CreateACLs as that will be more useful in later PRs

Copy link
Contributor

@hhahn-tw hhahn-tw left a 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",
Copy link
Contributor

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, fixed

@petedannemann petedannemann merged commit bd46e16 into master Oct 11, 2023
14 checks passed
@petedannemann petedannemann deleted the feat/get-acls branch October 11, 2023 17:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants