gharris1727 commented on a change in pull request #9987:
URL: https://github.com/apache/kafka/pull/9987#discussion_r565694653
##########
File path:
connect/basic-auth-extension/src/main/java/org/apache/kafka/connect/rest/basic/auth/extension/BasicAuthSecurityRestExtension.java
##########
@@ -80,7 +95,8 @@ public void close() throws IOException {
@Override
public void configure(Map<String, ?> configs) {
-
+ // If we failed to retrieve a JAAS configuration during startup, throw
that exception now
+ CONFIGURATION.get();
Review comment:
Could you add a test which confirms that we're propagating the exception
here? At the moment, the test verifies that the wrapping method works, but
doesn't verify that it's used by the rest extension during the loading phase.
##########
File path:
connect/basic-auth-extension/src/test/java/org/apache/kafka/connect/rest/basic/auth/extension/BasicAuthSecurityRestExtensionTest.java
##########
@@ -63,4 +69,25 @@ public void testJaasConfigurationNotOverwritten() {
assertNotEquals(overwrittenConfiguration,
jaasFilter.getValue().configuration,
"Overwritten JAAS configuration should not be used by basic auth
REST extension");
}
+
+ @Test
+ public void testBadJaasConfiguration() {
+ SecurityException jaasConfigurationException = new
SecurityException(new IOException("Bad JAAS config is bad"));
+ Supplier<Configuration> configuration =
BasicAuthSecurityRestExtension.initializeConfiguration(() -> {
+ throw jaasConfigurationException;
+ });
+
+ ConnectException thrownException =
assertThrows(ConnectException.class, configuration::get);
+ assertEquals(jaasConfigurationException, thrownException.getCause());
+ }
+
+ @Test
+ public void testGoodJaasConfiguration() {
+ Configuration mockConfiguration = EasyMock.mock(Configuration.class);
Review comment:
The identity function could pass this test, but wouldn't have the
behavior we need in the BasicAuthSecurityRestExtension. I wonder if there's a
way to confirm that the mockConfiguration has been evaluated prior to calling
`get()` on the returned supplier.
----------------------------------------------------------------
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
