rpk: add client quotas support to rpk

[r-vasquez]

This PR adds rpk support to DescribeClientQuotas and AlterClientQuotas under the rpk cluster quotas command.

Examples:

Creating a Quota

$ rpk cluster quotas alter --add consumer_byte_rate=12000 --name client-id=foo --default user
ENTITY                        STATUS
client-id=foo,user=<default>  OK

Describing Quotas

$ rpk cluster quotas describe --name client-id=testclient                                    
ENTITY                               QUOTA-TYPE          QUOTA-VALUE
client-id=testclient,user=user2      producer_byte_rate  130000
client-id=testclient,user=<default>  producer_byte_rate  140000

All new commands now include support for --format json|yaml flag.

Backports Required

• none - not a bug fix
• none - this is a backport
• none - issue does not exist in previous branches
• none - papercut/not impactful enough to backport
• v24.1.x
• v23.3.x
• v23.2.x

Release Notes

Features

• rpk: now you can manage client quotas using rpk cluster quotas.

[vbotbuildovich]

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/49661#018fc728-0975-4894-96e0-cf9cccd185bb

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/49661#018fc72a-84c9-4703-8ab7-56fa56d33845

[r-vasquez]

Last Force Push:

• Refactor the describe output
• Add ducktape handler for rpk cluster quotas in 1b7fc43

[pgellert]

Thanks, I've skimmed through the PR and it pretty much looks good to me. I might come back for a deeper review later. But let me add a bit of context in the meantime:

For the time being, Redpanda is only going to support a subset of the quotas that Kafka supports:

• Only entity=client quotas and only for the consumer_byte_rate, producer_byte_rate and controller_mutation_rate quota types
• We won’t yet support user entity types or ip entity types. We also won’t support connection_creation_rate and request_percentage quotas.
  I’m wondering if it would be confusing for customers if rpk supports everything but redpanda doesn’t. I think it’s probably fine as long as we propagate the errors returned by redpanda to the user and redpanda handles the validation of which quota combinations are supported. I think that’s already the case, but I just wanted to call this out for context.

Also, redpanda has a client-id-prefix entity type that we’ve introduced. Long story short, redpanda has supported this client quota type from a while ago and we want to be able to configure them through the Kafka API as well. I’m not sure if franz-go would already support this (ie. if it validates the entity type string at all), but if it’s easy, it would be great if rpk could start supporting the client-id-prefix entity type as well. It would make testing easier in redpanda.

[pgellert]

Thanks so much for setting this up!

[Feediver1]

Left some suggestions for clarity, consistency with docs.

[r-vasquez]

@pgellert How does a client-id-prefix entity work along with client-id? how does a default for client-id-prefix work?

Is an entity composed of {client-id=id, client-id-prefix=prefix} or is it like IP, where the prefix is a single entity.

Also, out of curiosity, what's the order of precedence? say I have:

client-id=foo-bar
consumer_byte_rate=200

client-id=<default>
consumer_byte_rate=100

client-id-prefix=foo-
consumer_byte_rate=300

[pgellert]

@r-vasquez:

How does a client-id-prefix entity work along with client-id? how does a default for client-id-prefix work?

The priority order is:

1. Explicit match: (client-id, <client-id>)
2. Prefix match: (client-id-prefix, <client-id-prefix>)
3. Client default: (client-id, <default>)

There's no specific default for client-id-prefix, (client-id-prefix, <default>) should be "not supported" and we should error out either in rpk/redpanda.

Is an entity composed of {client-id=id, client-id-prefix=prefix} or is it like IP, where the prefix is a single entity.

The client-id and client-id-prefix should be mutually exclusive, but we should allow both {client-id=id, user=user} and {client-id-prefix=prefix, user=user} pairs (at least once redpanda supports user-based quotas). So {client-id-prefix=prefix} should be usable whereever {client-id=id} is used.

Also, out of curiosity, what's the order of precedence? say I have:

client-id=foo-bar
consumer_byte_rate=200

client-id=<default>
consumer_byte_rate=100

client-id-prefix=foo-
consumer_byte_rate=300

Based on the priority order from above, foo-bar would get 200 (explicit match wins over prefix match), foo-abc would get 300 (from the prefix match) and baz would get 100 (from the default).

[r-vasquez]

Last 2 force pushes: 1 and 2

• Removed any reference to user and ip from the help text.
• Applied the suggested doc changes
• Added client-id-prefix to the examples.
• Added an error message when the describe command returns an empty list. (text format)

@pgellert Thanks for the context, I changed the PR to meet what Redpanda will support, the validation happens in Redpanda so we should be fine.

(For example, this is using Kafka:)

// client-id-prefix is not valid in Kafka
$ rpk cluster quotas alter --add producer_byte_rate=12123123 --name client-id-prefix=foo-
ENTITY                 STATUS
client-id-prefix=foo-  Error: Unhandled client quota entity type: client-id-prefix

// From Kafka Logs:
[2024-06-05 15:21:02,385] INFO [Admin Manager on Broker 1]: Error encountered while updating client quotas (kafka.server.ZkAdminManager)
org.apache.kafka.common.errors.InvalidRequestException: Unhandled client quota entity type: client-id-prefix

Please review the examples again.

[pgellert]

There's one error message which is misleading but apart from that lgtm.

[pgellert]

The error message says that --default client-id-prefix is allowed even though it's not supported. I think there should probably be a separate validTypes for any and default. Alternatively, we could handle the validation in redpanda and rpk will just show the error but at least this error message should be changed.

[r-vasquez]

I added your suggestion in a new commit, thanks! 0ac43ee