splett2 commented on a change in pull request #9628:
URL: https://github.com/apache/kafka/pull/9628#discussion_r536240414



##########
File path: core/src/main/scala/kafka/server/AdminManager.scala
##########
@@ -920,32 +954,51 @@ class AdminManager(val config: KafkaConfig,
         !name.isDefined || !strict
     }
 
-    def fromProps(props: Map[String, String]): Map[String, Double] = {
-      props.map { case (key, value) =>
-        val doubleValue = try value.toDouble catch {
-          case _: NumberFormatException =>
-            throw new IllegalStateException(s"Unexpected client quota 
configuration value: $key -> $value")
-        }
-        key -> doubleValue
-      }
-    }
-
-    (userEntries ++ clientIdEntries ++ bothEntries).map { case ((u, c), p) =>
+    (userEntries ++ clientIdEntries ++ bothEntries).flatMap { case ((u, c), p) 
=>
       val quotaProps = p.asScala.filter { case (key, _) => 
QuotaConfigs.isQuotaConfig(key) }
       if (quotaProps.nonEmpty && matches(userComponent, u) && 
matches(clientIdComponent, c))
         Some(userClientIdToEntity(u, c) -> fromProps(quotaProps))
       else
         None
-    }.flatten.toMap
+    }.toMap
+  }
+
+  def handleDescribeIpQuotas(ipComponent: Option[ClientQuotaFilterComponent], 
strict: Boolean): Map[ClientQuotaEntity, Map[String, Double]] = {
+    val ip = ipComponent.flatMap(c => toOption(c.`match`))
+    val exactIp = wantExact(ipComponent)
+    val allIps = ipComponent.exists(_.`match` == null) || (ipComponent.isEmpty 
&& !strict)
+    val ipEntries = if (exactIp)
+      Map(Some(ip.get) -> adminZkClient.fetchEntityConfig(ConfigType.Ip, 
sanitized(ip)))
+    else if (allIps)
+      adminZkClient.fetchAllEntityConfigs(ConfigType.Ip).map { case (name, 
props) =>
+        Some(desanitizeEntityName(name)) -> props
+      }
+    else
+      Map.empty
+
+    def ipToQuotaEntity(ip: Option[String]): ClientQuotaEntity = {
+      new ClientQuotaEntity(ip.map(ipName => ClientQuotaEntity.IP -> 
ipName).toMap.asJava)
+    }
+
+    ipEntries.flatMap { case (ip, props) =>
+      val ipQuotaProps = props.asScala.filter { case (key, _) => 
DynamicConfig.Ip.names.contains(key) }
+      if (ipQuotaProps.nonEmpty)
+        Some(ipToQuotaEntity(ip) -> fromProps(ipQuotaProps))
+      else
+        None
+    }
   }
 
   def alterClientQuotas(entries: Seq[ClientQuotaAlteration], validateOnly: 
Boolean): Map[ClientQuotaEntity, ApiError] = {
     def alterEntityQuotas(entity: ClientQuotaEntity, ops: 
Iterable[ClientQuotaAlteration.Op]): Unit = {
-      val (path, configType, configKeys) = 
entityToSanitizedUserClientId(entity) match {
-        case (Some(user), Some(clientId)) => (user + "/clients/" + clientId, 
ConfigType.User, DynamicConfig.User.configKeys)
-        case (Some(user), None) => (user, ConfigType.User, 
DynamicConfig.User.configKeys)
-        case (None, Some(clientId)) => (clientId, ConfigType.Client, 
DynamicConfig.Client.configKeys)
-        case _ => throw new InvalidRequestException("Invalid empty client 
quota entity")
+      val (path, configType, configKeys) = parseAndSanitizeQuotaEntity(entity) 
match {
+        case (Some(user), Some(clientId), None) => (user + "/clients/" + 
clientId, ConfigType.User, DynamicConfig.User.configKeys)
+        case (Some(user), None, None) => (user, ConfigType.User, 
DynamicConfig.User.configKeys)
+        case (None, Some(clientId), None) => (clientId, ConfigType.Client, 
DynamicConfig.Client.configKeys)
+        case (None, None, Some(ip)) => (ip, ConfigType.Ip, 
DynamicConfig.Ip.configKeys)
+        case (_, _, Some(_)) => throw new InvalidRequestException(s"Invalid 
quota entity combination, " +

Review comment:
       i used `(_, _, Some(_))` for this case, because we want to throw the 
more verbose exception in all of these cases:
   
   `(Some(_), None, Some(_))` => specifying a user and IP, but no client
   `(None, Some, Some)` => specifying client and IP, but no user
   `(Some, Some, Some)` => specifying client, user and IP
   
   basically any time there is a client OR user specified along with IP. I felt 
like adding all three cases would be pretty verbose, so I relied on the 
ordering of the cases instead.
   




----------------------------------------------------------------
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


Reply via email to