This is an automated email from the ASF dual-hosted git repository.
cmccabe pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/kafka.git
The following commit(s) were added to refs/heads/trunk by this push:
new d0f71486c20 MINOR: Improve the logging of IllegalStateException
exceptions thrown from SslFactory (#16346)
d0f71486c20 is described below
commit d0f71486c20f6fb18a1901db49086ddb678689e2
Author: Alyssa Huang <[email protected]>
AuthorDate: Tue Jul 16 11:01:53 2024 -0700
MINOR: Improve the logging of IllegalStateException exceptions thrown from
SslFactory (#16346)
Reviewers: Colin P. McCabe <[email protected]>, Rajini Sivaram
<[email protected]>
---
.../kafka/common/network/SaslChannelBuilder.java | 9 +++++++--
.../kafka/common/security/ssl/SslFactory.java | 8 ++++++--
.../kafka/common/security/ssl/SslFactoryTest.java | 21 +++++++++++++--------
3 files changed, 26 insertions(+), 12 deletions(-)
diff --git
a/clients/src/main/java/org/apache/kafka/common/network/SaslChannelBuilder.java
b/clients/src/main/java/org/apache/kafka/common/network/SaslChannelBuilder.java
index c8c621698c0..4c73e4e3e4c 100644
---
a/clients/src/main/java/org/apache/kafka/common/network/SaslChannelBuilder.java
+++
b/clients/src/main/java/org/apache/kafka/common/network/SaslChannelBuilder.java
@@ -17,6 +17,7 @@
package org.apache.kafka.common.network;
import org.apache.kafka.common.KafkaException;
+import org.apache.kafka.common.config.ConfigException;
import org.apache.kafka.common.config.SaslConfigs;
import org.apache.kafka.common.config.SslConfigs;
import org.apache.kafka.common.config.internals.BrokerSecurityConfigs;
@@ -193,9 +194,13 @@ public class SaslChannelBuilder implements ChannelBuilder,
ListenerReconfigurabl
}
@Override
- public void validateReconfiguration(Map<String, ?> configs) {
+ public void validateReconfiguration(Map<String, ?> configs) throws
ConfigException {
if (this.securityProtocol == SecurityProtocol.SASL_SSL)
- sslFactory.validateReconfiguration(configs);
+ try {
+ sslFactory.validateReconfiguration(configs);
+ } catch (IllegalStateException e) {
+ throw new ConfigException("SASL reconfiguration failed due to
" + e);
+ }
}
@Override
diff --git
a/clients/src/main/java/org/apache/kafka/common/security/ssl/SslFactory.java
b/clients/src/main/java/org/apache/kafka/common/security/ssl/SslFactory.java
index 361e1c23439..9f9d23686d4 100644
--- a/clients/src/main/java/org/apache/kafka/common/security/ssl/SslFactory.java
+++ b/clients/src/main/java/org/apache/kafka/common/security/ssl/SslFactory.java
@@ -115,8 +115,12 @@ public class SslFactory implements Reconfigurable,
Closeable {
}
@Override
- public void validateReconfiguration(Map<String, ?> newConfigs) {
- createNewSslEngineFactory(newConfigs);
+ public void validateReconfiguration(Map<String, ?> newConfigs) throws
ConfigException {
+ try {
+ createNewSslEngineFactory(newConfigs);
+ } catch (IllegalStateException e) {
+ throw new ConfigException("SSL reconfiguration failed due to " +
e);
+ }
}
@Override
diff --git
a/clients/src/test/java/org/apache/kafka/common/security/ssl/SslFactoryTest.java
b/clients/src/test/java/org/apache/kafka/common/security/ssl/SslFactoryTest.java
index 846425a5048..41d6bed942c 100644
---
a/clients/src/test/java/org/apache/kafka/common/security/ssl/SslFactoryTest.java
+++
b/clients/src/test/java/org/apache/kafka/common/security/ssl/SslFactoryTest.java
@@ -204,6 +204,11 @@ public abstract class SslFactoryTest {
.createNewTrustStore(trustStoreFile)
.build();
SslFactory sslFactory = new SslFactory(ConnectionMode.SERVER);
+
+ // Verify that we'll throw an exception if validateReconfiguration is
called before sslFactory is configured
+ Exception e = assertThrows(ConfigException.class, () ->
sslFactory.validateReconfiguration(sslConfig));
+ assertEquals("SSL reconfiguration failed due to
java.lang.IllegalStateException: SslFactory has not been configured.",
e.getMessage());
+
sslFactory.configure(sslConfig);
SslEngineFactory sslEngineFactory = sslFactory.sslEngineFactory();
assertNotNull(sslEngineFactory, "SslEngineFactory not created");
@@ -215,37 +220,37 @@ public abstract class SslFactoryTest {
// Verify that the SslEngineFactory is recreated on reconfigure() if
config is changed
trustStoreFile = TestUtils.tempFile("truststore", ".jks");
- sslConfig = sslConfigsBuilder(ConnectionMode.SERVER)
+ Map<String, Object> newSslConfig =
sslConfigsBuilder(ConnectionMode.SERVER)
.createNewTrustStore(trustStoreFile)
.build();
- sslFactory.reconfigure(sslConfig);
+ sslFactory.reconfigure(newSslConfig);
assertNotSame(sslEngineFactory, sslFactory.sslEngineFactory(),
"SslEngineFactory not recreated");
sslEngineFactory = sslFactory.sslEngineFactory();
// Verify that builder is recreated on reconfigure() if config is not
changed, but truststore file was modified
trustStoreFile.setLastModified(System.currentTimeMillis() + 10000);
- sslFactory.reconfigure(sslConfig);
+ sslFactory.reconfigure(newSslConfig);
assertNotSame(sslEngineFactory, sslFactory.sslEngineFactory(),
"SslEngineFactory not recreated");
sslEngineFactory = sslFactory.sslEngineFactory();
// Verify that builder is recreated on reconfigure() if config is not
changed, but keystore file was modified
- File keyStoreFile = new File((String)
sslConfig.get(SslConfigs.SSL_KEYSTORE_LOCATION_CONFIG));
+ File keyStoreFile = new File((String)
newSslConfig.get(SslConfigs.SSL_KEYSTORE_LOCATION_CONFIG));
keyStoreFile.setLastModified(System.currentTimeMillis() + 10000);
- sslFactory.reconfigure(sslConfig);
+ sslFactory.reconfigure(newSslConfig);
assertNotSame(sslEngineFactory, sslFactory.sslEngineFactory(),
"SslEngineFactory not recreated");
sslEngineFactory = sslFactory.sslEngineFactory();
// Verify that builder is recreated after validation on reconfigure()
if config is not changed, but keystore file was modified
keyStoreFile.setLastModified(System.currentTimeMillis() + 15000);
- sslFactory.validateReconfiguration(sslConfig);
- sslFactory.reconfigure(sslConfig);
+ sslFactory.validateReconfiguration(newSslConfig);
+ sslFactory.reconfigure(newSslConfig);
assertNotSame(sslEngineFactory, sslFactory.sslEngineFactory(),
"SslEngineFactory not recreated");
sslEngineFactory = sslFactory.sslEngineFactory();
// Verify that the builder is not recreated if modification time
cannot be determined
keyStoreFile.setLastModified(System.currentTimeMillis() + 20000);
Files.delete(keyStoreFile.toPath());
- sslFactory.reconfigure(sslConfig);
+ sslFactory.reconfigure(newSslConfig);
assertSame(sslEngineFactory, sslFactory.sslEngineFactory(),
"SslEngineFactory recreated unnecessarily");
}
