cmccabe commented on code in PR #13116:
URL: https://github.com/apache/kafka/pull/13116#discussion_r1127186242
##########
core/src/test/scala/unit/kafka/server/DynamicBrokerConfigTest.scala:
##########
@@ -479,6 +489,42 @@ class DynamicBrokerConfigTest {
assertEquals("User:admin", authorizer.superUsers)
}
+ @Test
+ def testCombinedControllerAuthorizerConfig(): Unit = {
+ val props = TestUtils.createCombinedControllerConfig(0, port = 9092)
+ val oldConfig = KafkaConfig.fromProps(props)
+ oldConfig.dynamicConfig.initialize(None)
+
+ val controllerServer: ControllerServer =
mock(classOf[kafka.server.ControllerServer])
+
+ val authorizer = new TestAuthorizer
+ when(controllerServer.config).thenReturn(oldConfig)
+ when(controllerServer.authorizer).thenReturn(Some(authorizer))
+ // We are only testing authorizer reconfiguration, ignore any exceptions
due to incomplete mock
+ assertThrows(classOf[Throwable], () =>
controllerServer.config.dynamicConfig.addReconfigurables(controllerServer))
Review Comment:
I sympathize with not wanting to debug the mocking mess here. There are just
so many functions that get called, and having to do thenReturn for all of them
is a huge pain. Still, just putting an `assertThrows(classOf[Throwable], ...)`
seems like it could hide errors over time.
Should we just add the authorizer as a reconfigurable directly? Or is that
too hard to do here?
##########
core/src/test/scala/unit/kafka/server/DynamicBrokerConfigTest.scala:
##########
@@ -479,6 +489,42 @@ class DynamicBrokerConfigTest {
assertEquals("User:admin", authorizer.superUsers)
}
+ @Test
+ def testCombinedControllerAuthorizerConfig(): Unit = {
+ val props = TestUtils.createCombinedControllerConfig(0, port = 9092)
+ val oldConfig = KafkaConfig.fromProps(props)
+ oldConfig.dynamicConfig.initialize(None)
+
+ val controllerServer: ControllerServer =
mock(classOf[kafka.server.ControllerServer])
+
+ val authorizer = new TestAuthorizer
+ when(controllerServer.config).thenReturn(oldConfig)
+ when(controllerServer.authorizer).thenReturn(Some(authorizer))
+ // We are only testing authorizer reconfiguration, ignore any exceptions
due to incomplete mock
+ assertThrows(classOf[Throwable], () =>
controllerServer.config.dynamicConfig.addReconfigurables(controllerServer))
+ props.put("super.users", "User:admin")
+ controllerServer.config.dynamicConfig.updateBrokerConfig(0, props)
+ assertEquals("User:admin", authorizer.superUsers)
+ }
+
+ @Test
+ def testIsolatedControllerAuthorizerConfig(): Unit = {
+ val props = TestUtils.createIsolatedControllerConfig(0, port = 9092)
+ val oldConfig = KafkaConfig.fromProps(props)
+ oldConfig.dynamicConfig.initialize(None)
+
+ val controllerServer: ControllerServer =
mock(classOf[kafka.server.ControllerServer])
+
+ val authorizer = new TestAuthorizer
+ when(controllerServer.config).thenReturn(oldConfig)
+ when(controllerServer.authorizer).thenReturn(Some(authorizer))
+ // We are only testing authorizer reconfiguration, ignore any exceptions
due to incomplete mock
+ assertThrows(classOf[Throwable], () =>
controllerServer.config.dynamicConfig.addReconfigurables(controllerServer))
Review Comment:
Same question as above
--
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.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
