bdbyrne commented on a change in pull request #9628:
URL: https://github.com/apache/kafka/pull/9628#discussion_r532909153
##########
File path:
clients/src/main/java/org/apache/kafka/common/quota/ClientQuotaEntity.java
##########
@@ -32,6 +32,7 @@
*/
public static final String USER = "user";
public static final String CLIENT_ID = "client-id";
+ public static final String IP = "IP";
Review comment:
Consider making lower-case for consistency, but otherwise not a big
issue, as I see that's how it was proposed in the KIP.
##########
File path: core/src/main/scala/kafka/server/AdminManager.scala
##########
@@ -807,23 +807,25 @@ class AdminManager(val config: KafkaConfig,
case name => Sanitizer.desanitize(name)
}
- private def entityToSanitizedUserClientId(entity: ClientQuotaEntity):
(Option[String], Option[String]) = {
+ private def entityToSanitizedUserClientId(entity: ClientQuotaEntity):
(Option[String], Option[String], Option[String]) = {
Review comment:
May want to rename (generalize) since this now includes IP
##########
File path: core/src/main/scala/kafka/server/AdminManager.scala
##########
@@ -833,45 +835,76 @@ class AdminManager(val config: KafkaConfig,
def describeClientQuotas(filter: ClientQuotaFilter): Map[ClientQuotaEntity,
Map[String, Double]] = {
var userComponent: Option[ClientQuotaFilterComponent] = None
var clientIdComponent: Option[ClientQuotaFilterComponent] = None
+ var ipComponent: Option[ClientQuotaFilterComponent] = None
filter.components.forEach { component =>
component.entityType match {
case ClientQuotaEntity.USER =>
if (userComponent.isDefined)
- throw new InvalidRequestException(s"Duplicate user filter
component entity type");
+ throw new InvalidRequestException(s"Duplicate user filter
component entity type")
userComponent = Some(component)
case ClientQuotaEntity.CLIENT_ID =>
if (clientIdComponent.isDefined)
- throw new InvalidRequestException(s"Duplicate client filter
component entity type");
+ throw new InvalidRequestException(s"Duplicate client filter
component entity type")
clientIdComponent = Some(component)
+ case ClientQuotaEntity.IP =>
+ if (ipComponent.isDefined)
+ throw new InvalidRequestException(s"Duplicate ip filter component
entity type")
+ ipComponent = Some(component)
case "" =>
throw new InvalidRequestException(s"Unexpected empty filter
component entity type")
case et =>
// Supplying other entity types is not yet supported.
throw new UnsupportedVersionException(s"Custom entity type '${et}'
not supported")
}
}
- handleDescribeClientQuotas(userComponent, clientIdComponent, filter.strict)
+ if ((userComponent.isDefined || clientIdComponent.isDefined) &&
ipComponent.isDefined)
+ throw new InvalidRequestException(s"Invalid entity filter component
combination")
Review comment:
May want to provide a little more detail in the response
##########
File path: core/src/test/scala/unit/kafka/utils/TestUtils.scala
##########
@@ -1529,6 +1530,16 @@ object TestUtils extends Logging {
adminClient.incrementalAlterConfigs(configs)
}
+ def alterClientQuotas(adminClient: Admin, request: Map[ClientQuotaEntity,
Map[String, Option[Double]]]): AlterClientQuotasResult = {
+ val entries = request.map { case (entity, alter) =>
+ val ops = alter.map { case (key, value) =>
+ new ClientQuotaAlteration.Op(key,
value.map(Double.box).getOrElse(null))
+ }.asJavaCollection
+ new ClientQuotaAlteration(entity, ops)
Review comment:
nit: indenting
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
