rondagostino commented on a change in pull request #9032: URL: https://github.com/apache/kafka/pull/9032#discussion_r472530253
########## File path: core/src/test/scala/unit/kafka/admin/ConfigCommandTest.scala ########## @@ -486,7 +486,9 @@ class ConfigCommandTest extends ZooKeeperTestHarness with Logging { } @Test - def shouldNotAlterNonQuotaClientConfigUsingBootstrapServer(): Unit = { + def shouldNotAlterNonQuotaNonScramUserOrClientConfigUsingBootstrapServer(): Unit = { + // when using --bootstrap-server, it should be illegal to alter anything that is not a quota and not a SCRAM credential + // for both user and client entities Review comment: @cmccabe Good question, actually. There is already a check to make sure a non-existent config cannot be **deleted** via `--zookeeper`: `shouldNotUpdateConfigIfNonExistingConfigIsDeletedUsingZookeper()`. This test passes, of course. However, there is no check to make sure an unrecognized config can be **added**, and in fact if I add that test it fails; the code will gladly go ahead and add anything we wish (and it will gladly go ahead and delete it if we wish as well -- the above test is only checking that something that doesn't exist can't be deleted). The next question, of course, is whether we should "fix" this or not. What do you think? To fix it we would need the full set of allowed configs at the User, Client, Topic, and Broker levels and then insert code to check accordingly. Since the ZooKeeper update path is going away due to KIP-500, I'm wondering if we can just leave it alone? ---------------------------------------------------------------- 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