cmccabe commented on a change in pull request #9032:
URL: https://github.com/apache/kafka/pull/9032#discussion_r465378122
##########
File path: core/src/main/scala/kafka/admin/ConfigCommand.scala
##########
@@ -365,45 +368,90 @@ object ConfigCommand extends Config {
adminClient.incrementalAlterConfigs(Map(configResource ->
alterLogLevelEntries).asJava, alterOptions).all().get(60, TimeUnit.SECONDS)
case ConfigType.User | ConfigType.Client =>
- val nonQuotaConfigsToAdd =
configsToBeAdded.keys.filterNot(QuotaConfigs.isQuotaConfig)
- if (nonQuotaConfigsToAdd.nonEmpty)
- throw new IllegalArgumentException(s"Only quota configs can be added
for '$entityTypeHead' using --bootstrap-server. Unexpected config names:
$nonQuotaConfigsToAdd")
- val nonQuotaConfigsToDelete =
configsToBeDeleted.filterNot(QuotaConfigs.isQuotaConfig)
- if (nonQuotaConfigsToDelete.nonEmpty)
- throw new IllegalArgumentException(s"Only quota configs can be
deleted for '$entityTypeHead' using --bootstrap-server. Unexpected config
names: $nonQuotaConfigsToDelete")
-
-
- val oldConfig = getClientQuotasConfig(adminClient, entityTypes,
entityNames)
-
- val invalidConfigs = configsToBeDeleted.filterNot(oldConfig.contains)
- if (invalidConfigs.nonEmpty)
- throw new InvalidConfigurationException(s"Invalid config(s):
${invalidConfigs.mkString(",")}")
-
- val alterEntityTypes = entityTypes.map { entType =>
- entType match {
- case ConfigType.User => ClientQuotaEntity.USER
- case ConfigType.Client => ClientQuotaEntity.CLIENT_ID
- case _ => throw new IllegalArgumentException(s"Unexpected entity
type: ${entType}")
+ val hasQuotaConfigsToAdd =
configsToBeAdded.keys.exists(QuotaConfigs.isQuotaConfig)
+ val scramConfigsToAddMap = configsToBeAdded.filter(entry =>
ScramMechanism.isScram(entry._1))
+ val unknownConfigsToAdd = configsToBeAdded.keys.filterNot(key =>
ScramMechanism.isScram(key) || QuotaConfigs.isQuotaConfig(key))
+ val hasQuotaConfigsToDelete =
configsToBeDeleted.exists(QuotaConfigs.isQuotaConfig)
+ val scramConfigsToDelete =
configsToBeDeleted.filter(ScramMechanism.isScram)
+ val unknownConfigsToDelete = configsToBeDeleted.filterNot(key =>
ScramMechanism.isScram(key) || QuotaConfigs.isQuotaConfig(key))
+ if (entityTypeHead == ConfigType.Client || entityTypes.size == 2) { //
size==2 for case where users is specified first on the command line, before
clients
+ // either just a client or both a user and a client
+ if (unknownConfigsToAdd.nonEmpty || scramConfigsToAddMap.nonEmpty)
+ throw new IllegalArgumentException(s"Only quota configs can be
added for '${ConfigType.Client}' using --bootstrap-server. Unexpected config
names: ${unknownConfigsToAdd ++ scramConfigsToAddMap.keys}")
+ if (unknownConfigsToDelete.nonEmpty || scramConfigsToDelete.nonEmpty)
+ throw new IllegalArgumentException(s"Only quota configs can be
deleted for '${ConfigType.Client}' using --bootstrap-server. Unexpected config
names: ${unknownConfigsToDelete ++ scramConfigsToDelete}")
+ } else { // ConfigType.User
+ if (unknownConfigsToAdd.nonEmpty)
+ throw new IllegalArgumentException(s"Only quota and SCRAM
credential configs can be added for '${ConfigType.User}' using
--bootstrap-server. Unexpected config names: $unknownConfigsToAdd")
+ if (unknownConfigsToDelete.nonEmpty)
+ throw new IllegalArgumentException(s"Only quota and SCRAM
credential configs can be deleted for '${ConfigType.User}' using
--bootstrap-server. Unexpected config names: $unknownConfigsToDelete")
+ if (scramConfigsToAddMap.nonEmpty || scramConfigsToDelete.nonEmpty) {
+ if (entityNames.exists(_.isEmpty)) // either --entity-type users
--entity-default or --user-defaults
+ throw new IllegalArgumentException("The use of --entity-default
or --user-defaults is not allowed with User SCRAM Credentials using
--bootstrap-server.")
+ if (hasQuotaConfigsToAdd || hasQuotaConfigsToDelete)
+ throw new IllegalArgumentException(s"Cannot alter both quota and
SCRAM credential configs simultaneously for '${ConfigType.User}' using
--bootstrap-server.")
}
}
- val alterEntityNames = entityNames.map(en => if (en.nonEmpty) en else
null)
- // Explicitly populate a HashMap to ensure nulls are recorded properly.
- val alterEntityMap = new java.util.HashMap[String, String]
- alterEntityTypes.zip(alterEntityNames).foreach { case (k, v) =>
alterEntityMap.put(k, v) }
- val entity = new ClientQuotaEntity(alterEntityMap)
+ if (hasQuotaConfigsToAdd || hasQuotaConfigsToDelete) {
+ // handle altering client/user quota configs
+ val oldConfig = getClientQuotasConfig(adminClient, entityTypes,
entityNames)
- val alterOptions = new AlterClientQuotasOptions().validateOnly(false)
- val alterOps = (configsToBeAddedMap.map { case (key, value) =>
- val doubleValue = try value.toDouble catch {
- case _: NumberFormatException =>
- throw new IllegalArgumentException(s"Cannot parse quota
configuration value for ${key}: ${value}")
+ val invalidConfigs = configsToBeDeleted.filterNot(oldConfig.contains)
+ if (invalidConfigs.nonEmpty)
+ throw new InvalidConfigurationException(s"Invalid config(s):
${invalidConfigs.mkString(",")}")
+
+ val alterEntityTypes = entityTypes.map { entType =>
+ entType match {
+ case ConfigType.User => ClientQuotaEntity.USER
+ case ConfigType.Client => ClientQuotaEntity.CLIENT_ID
+ case _ => throw new IllegalArgumentException(s"Unexpected entity
type: ${entType}")
+ }
+ }
+ val alterEntityNames = entityNames.map(en => if (en.nonEmpty) en
else null)
+
+ // Explicitly populate a HashMap to ensure nulls are recorded
properly.
+ val alterEntityMap = new java.util.HashMap[String, String]
+ alterEntityTypes.zip(alterEntityNames).foreach { case (k, v) =>
alterEntityMap.put(k, v) }
+ val entity = new ClientQuotaEntity(alterEntityMap)
+
+ val alterOptions = new AlterClientQuotasOptions().validateOnly(false)
+ val alterOps = (configsToBeAddedMap.map { case (key, value) =>
+ val doubleValue = try value.toDouble catch {
+ case _: NumberFormatException =>
+ throw new IllegalArgumentException(s"Cannot parse quota
configuration value for ${key}: ${value}")
+ }
+ new ClientQuotaAlteration.Op(key, doubleValue)
+ } ++ configsToBeDeleted.map(key => new ClientQuotaAlteration.Op(key,
null))).asJavaCollection
+
+ adminClient.alterClientQuotas(Collections.singleton(new
ClientQuotaAlteration(entity, alterOps)), alterOptions)
+ .all().get(60, TimeUnit.SECONDS)
+ } else {
+ // handle altering user SCRAM credential configs
Review comment:
it would be good to split this off into its own function (after
verifying that we weren't trying to do both SCRAM + quota) or multiple entity
names
----------------------------------------------------------------
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:
[email protected]
