dajac commented on a change in pull request #9628: URL: https://github.com/apache/kafka/pull/9628#discussion_r537615920
########## 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: Thanks for the explanation. That makes sense. ---------------------------------------------------------------- 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