[kafka] branch 1.1 updated: MINOR: Fix encoder config to make DynamicBrokerReconfigurationTest stable (#4764)

[rs...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

rsivaram pushed a commit to branch 1.1
in repository https://gitbox.apache.org/repos/asf/kafka.git


The following commit(s) were added to refs/heads/1.1 by this push:
     new 3c05c44  MINOR: Fix encoder config to make DynamicBrokerReconfigurationTest stable (#4764)
3c05c44 is described below

commit 3c05c44f2277aad613a0ecece6cccb91f724bc39
Author: Rajini Sivaram <ra...@googlemail.com>
AuthorDate: Fri Mar 23 17:01:32 2018 +0000

    MINOR: Fix encoder config to make DynamicBrokerReconfigurationTest stable (#4764)
    
    DynamicBrokerReconfigurationTest currently assumes that passwords encoded with one secret will fail with an exception if decoded with another secret and configures an old.secret in setUp. This could potentially cause test failures if a password was incorrectly decoded with the wrong secret, since the test writes passwords encoded with the new secret directly to ZooKeeper. Since old.secret is only used in one test for verifying secret rotation, this config can be moved to that test to  [...]
    
    Reviewers: Ismael Juma <is...@juma.me.uk>
---
 .../org/apache/kafka/common/security/ssl/SslFactory.java | 16 ++++++++++------
 .../kafka/server/DynamicBrokerReconfigurationTest.scala  |  6 +++---
 2 files changed, 13 insertions(+), 9 deletions(-)

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 c54260d..bebd691 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
@@ -287,17 +287,21 @@ public class SslFactory implements Reconfigurable {
             this.keyPassword = keyPassword;
         }
 
-        KeyStore load() throws GeneralSecurityException, IOException {
-            FileInputStream in = null;
-            try {
+        /**
+         * Loads this keystore
+         * @return the keystore
+         * @throws KafkaException if the file could not be read or if the keystore could not be loaded
+         *   using the specified configs (e.g. if the password or keystore type is invalid)
+         */
+        KeyStore load() {
+            try (FileInputStream in = new FileInputStream(path)) {
                 KeyStore ks = KeyStore.getInstance(type);
-                in = new FileInputStream(path);
                 // If a password is not set access to the truststore is still available, but integrity checking is disabled.
                 char[] passwordChars = password != null ? password.value().toCharArray() : null;
                 ks.load(in, passwordChars);
                 return ks;
-            } finally {
-                if (in != null) in.close();
+            } catch (GeneralSecurityException | IOException e) {
+                throw new KafkaException("Failed to load SSL keystore " + path + " of type " + type, e);
             }
         }
     }
diff --git a/core/src/test/scala/integration/kafka/server/DynamicBrokerReconfigurationTest.scala b/core/src/test/scala/integration/kafka/server/DynamicBrokerReconfigurationTest.scala
index e207de8..e0ab55c 100644
--- a/core/src/test/scala/integration/kafka/server/DynamicBrokerReconfigurationTest.scala
+++ b/core/src/test/scala/integration/kafka/server/DynamicBrokerReconfigurationTest.scala
@@ -111,7 +111,6 @@ class DynamicBrokerReconfigurationTest extends ZooKeeperTestHarness with SaslSet
       props.put(KafkaConfig.NumReplicaFetchersProp, "2") // greater than one to test reducing threads
       props.put(KafkaConfig.ProducerQuotaBytesPerSecondDefaultProp, "10000000") // non-default value to trigger a new metric
       props.put(KafkaConfig.PasswordEncoderSecretProp, "dynamic-config-secret")
-      props.put(KafkaConfig.PasswordEncoderOldSecretProp, "old-dynamic-config-secret") // for testing secret rotation
 
       props ++= sslProperties1
       addKeystoreWithListenerPrefix(sslProperties1, props, SecureInternal)
@@ -670,7 +669,8 @@ class DynamicBrokerReconfigurationTest extends ZooKeeperTestHarness with SaslSet
       val propsEncodedWithOldSecret = props.clone().asInstanceOf[Properties]
       val config = server.config
       val secret = config.passwordEncoderSecret.getOrElse(throw new IllegalStateException("Password encoder secret not configured"))
-      val oldSecret = config.passwordEncoderOldSecret.getOrElse(throw new IllegalStateException("Password encoder old secret not configured"))
+      val oldSecret = "old-dynamic-config-secret"
+      config.dynamicConfig.staticBrokerConfigs.put(KafkaConfig.PasswordEncoderOldSecretProp, oldSecret)
       val passwordConfigs = props.asScala.filterKeys(DynamicBrokerConfig.isPasswordConfig)
       assertTrue("Password configs not found", passwordConfigs.nonEmpty)
       val passwordDecoder = new PasswordEncoder(secret,
@@ -678,7 +678,7 @@ class DynamicBrokerReconfigurationTest extends ZooKeeperTestHarness with SaslSet
         config.passwordEncoderCipherAlgorithm,
         config.passwordEncoderKeyLength,
         config.passwordEncoderIterations)
-      val passwordEncoder = new PasswordEncoder(oldSecret,
+      val passwordEncoder = new PasswordEncoder(new Password(oldSecret),
         config.passwordEncoderKeyFactoryAlgorithm,
         config.passwordEncoderCipherAlgorithm,
         config.passwordEncoderKeyLength,

-- 
To stop receiving notification emails like this one, please contact
rsivaram@apache.org.