diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/kms/KMSClientProvider.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/kms/KMSClientProvider.java index 2b6ae9ea4d..f184ea790d 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/kms/KMSClientProvider.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/kms/KMSClientProvider.java @@ -173,14 +173,20 @@ public long renew(Token token, Configuration conf) throws IOException { LOG.debug("Renewing delegation token {}", token); KeyProvider keyProvider = KMSUtil.createKeyProvider(conf, KeyProviderFactory.KEY_PROVIDER_PATH); - if (!(keyProvider instanceof - KeyProviderDelegationTokenExtension.DelegationTokenExtension)) { - LOG.warn("keyProvider {} cannot renew dt.", keyProvider == null ? - "null" : keyProvider.getClass()); - return 0; + try { + if (!(keyProvider instanceof + KeyProviderDelegationTokenExtension.DelegationTokenExtension)) { + LOG.warn("keyProvider {} cannot renew dt.", keyProvider == null ? + "null" : keyProvider.getClass()); + return 0; + } + return ((KeyProviderDelegationTokenExtension.DelegationTokenExtension) + keyProvider).renewDelegationToken(token); + } finally { + if (keyProvider != null) { + keyProvider.close(); + } } - return ((KeyProviderDelegationTokenExtension.DelegationTokenExtension) - keyProvider).renewDelegationToken(token); } @Override @@ -188,14 +194,20 @@ public void cancel(Token token, Configuration conf) throws IOException { LOG.debug("Canceling delegation token {}", token); KeyProvider keyProvider = KMSUtil.createKeyProvider(conf, KeyProviderFactory.KEY_PROVIDER_PATH); - if (!(keyProvider instanceof - KeyProviderDelegationTokenExtension.DelegationTokenExtension)) { - LOG.warn("keyProvider {} cannot cancel dt.", keyProvider == null ? - "null" : keyProvider.getClass()); - return; + try { + if (!(keyProvider instanceof + KeyProviderDelegationTokenExtension.DelegationTokenExtension)) { + LOG.warn("keyProvider {} cannot cancel dt.", keyProvider == null ? + "null" : keyProvider.getClass()); + return; + } + ((KeyProviderDelegationTokenExtension.DelegationTokenExtension) + keyProvider).cancelDelegationToken(token); + } finally { + if (keyProvider != null) { + keyProvider.close(); + } } - ((KeyProviderDelegationTokenExtension.DelegationTokenExtension) - keyProvider).cancelDelegationToken(token); } } @@ -1072,6 +1084,7 @@ public void close() throws IOException { } finally { if (sslFactory != null) { sslFactory.destroy(); + sslFactory = null; } } } diff --git a/hadoop-common-project/hadoop-kms/src/test/java/org/apache/hadoop/crypto/key/kms/server/TestKMS.java b/hadoop-common-project/hadoop-kms/src/test/java/org/apache/hadoop/crypto/key/kms/server/TestKMS.java index dac91e0841..aaab1721c6 100644 --- a/hadoop-common-project/hadoop-kms/src/test/java/org/apache/hadoop/crypto/key/kms/server/TestKMS.java +++ b/hadoop-common-project/hadoop-kms/src/test/java/org/apache/hadoop/crypto/key/kms/server/TestKMS.java @@ -17,6 +17,7 @@ */ package org.apache.hadoop.crypto.key.kms.server; +import com.google.common.base.Supplier; import org.apache.curator.test.TestingServer; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.crypto.key.KeyProviderFactory; @@ -74,12 +75,16 @@ import java.util.List; import java.util.Map; import java.util.Properties; +import java.util.Set; import java.util.UUID; import java.util.concurrent.Callable; public class TestKMS { private static final Logger LOG = LoggerFactory.getLogger(TestKMS.class); + private static final String SSL_RELOADER_THREAD_NAME = + "Truststore reloader thread"; + @Rule public final Timeout testTimeout = new Timeout(180000); @@ -348,7 +353,7 @@ public Void call() throws Exception { Thread reloaderThread = null; for (Thread thread : threads) { if ((thread.getName() != null) - && (thread.getName().contains("Truststore reloader thread"))) { + && (thread.getName().contains(SSL_RELOADER_THREAD_NAME))) { reloaderThread = thread; } } @@ -378,6 +383,7 @@ public Void run() throws Exception { .addDelegationTokens("myuser", new Credentials()); Assert.assertEquals(1, tokens.length); Assert.assertEquals("kms-dt", tokens[0].getKind().toString()); + kp.close(); return null; } }); @@ -393,6 +399,7 @@ public Void run() throws Exception { .addDelegationTokens("myuser", new Credentials()); Assert.assertEquals(1, tokens.length); Assert.assertEquals("kms-dt", tokens[0].getKind().toString()); + kp.close(); } return null; } @@ -1755,34 +1762,63 @@ public Void run() throws Exception { }); } - @Test - public void testDelegationTokensOpsSimple() throws Exception { - final Configuration conf = new Configuration(); - testDelegationTokensOps(conf, false); - } - - @Test - public void testDelegationTokensOpsKerberized() throws Exception { - final Configuration conf = new Configuration(); + private Configuration setupConfForKerberos(File confDir) throws Exception { + final Configuration conf = createBaseKMSConf(confDir, null); conf.set("hadoop.security.authentication", "kerberos"); - testDelegationTokensOps(conf, true); + conf.set("hadoop.kms.authentication.type", "kerberos"); + conf.set("hadoop.kms.authentication.kerberos.keytab", + keytab.getAbsolutePath()); + conf.set("hadoop.kms.authentication.kerberos.principal", + "HTTP/localhost"); + conf.set("hadoop.kms.authentication.kerberos.name.rules", "DEFAULT"); + return conf; } - private void testDelegationTokensOps(Configuration conf, - final boolean useKrb) throws Exception { - File confDir = getTestDir(); - conf = createBaseKMSConf(confDir, conf); - if (useKrb) { - conf.set("hadoop.kms.authentication.type", "kerberos"); - conf.set("hadoop.kms.authentication.kerberos.keytab", - keytab.getAbsolutePath()); - conf.set("hadoop.kms.authentication.kerberos.principal", - "HTTP/localhost"); - conf.set("hadoop.kms.authentication.kerberos.name.rules", "DEFAULT"); + @Test + public void testDelegationTokensOpsHttpPseudo() throws Exception { + testDelegationTokensOps(false, false); + } + + @Test + public void testDelegationTokensOpsHttpKerberized() throws Exception { + testDelegationTokensOps(false, true); + } + + @Test + public void testDelegationTokensOpsHttpsPseudo() throws Exception { + testDelegationTokensOps(true, false); + } + + @Test + public void testDelegationTokensOpsHttpsKerberized() throws Exception { + testDelegationTokensOps(true, true); + } + + private void testDelegationTokensOps(final boolean ssl, final boolean kerb) + throws Exception { + final File confDir = getTestDir(); + final Configuration conf; + if (kerb) { + conf = setupConfForKerberos(confDir); + } else { + conf = createBaseKMSConf(confDir, null); + } + + final String keystore; + final String password; + if (ssl) { + final String sslConfDir = KeyStoreTestUtil.getClasspathDir(TestKMS.class); + KeyStoreTestUtil.setupSSLConfig(confDir.getAbsolutePath(), sslConfDir, + conf, false); + keystore = confDir.getAbsolutePath() + "/serverKS.jks"; + password = "serverP"; + } else { + keystore = null; + password = null; } writeConf(confDir, conf); - runServer(null, null, confDir, new KMSCallable() { + runServer(keystore, password, confDir, new KMSCallable() { @Override public Void call() throws Exception { final Configuration clientConf = new Configuration(); @@ -1830,7 +1866,7 @@ public Void run() throws Exception { } final UserGroupInformation otherUgi; - if (useKrb) { + if (kerb) { UserGroupInformation .loginUserFromKeytab("client1", keytab.getAbsolutePath()); otherUgi = UserGroupInformation.getLoginUser(); @@ -1885,6 +1921,9 @@ public Void run() throws Exception { return null; } }); + // Close the client provider. We will verify all providers' + // Truststore reloader threads are closed later. + kp.close(); return null; } finally { otherUgi.logoutUserFromKeytab(); @@ -1894,6 +1933,22 @@ public Void run() throws Exception { return null; } }); + + // verify that providers created by KMSTokenRenewer are closed. + if (ssl) { + GenericTestUtils.waitFor(new Supplier() { + @Override + public Boolean get() { + final Set threadSet = Thread.getAllStackTraces().keySet(); + for (Thread t : threadSet) { + if (t.getName().contains(SSL_RELOADER_THREAD_NAME)) { + return false; + } + } + return true; + } + }, 1000, 10000); + } } @Test diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java index 06a0564fa7..c78ef465a9 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java @@ -4569,6 +4569,13 @@ void shutdown() { if (blockManager != null) { blockManager.shutdown(); } + if (provider != null) { + try { + provider.close(); + } catch (IOException e) { + LOG.error("Failed to close provider.", e); + } + } } @Override // FSNamesystemMBean