You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ozone.apache.org by xy...@apache.org on 2021/04/19 19:30:01 UTC
[ozone] branch master updated: HDDS-5087. Ozone RPC client leaks
KeyProvider instances. (#2144)
This is an automated email from the ASF dual-hosted git repository.
xyao pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ozone.git
The following commit(s) were added to refs/heads/master by this push:
new cda430a HDDS-5087. Ozone RPC client leaks KeyProvider instances. (#2144)
cda430a is described below
commit cda430aac7a8afd64fe184ed250c6fa7482e2279
Author: Xiaoyu Yao <xy...@apache.org>
AuthorDate: Mon Apr 19 12:29:40 2021 -0700
HDDS-5087. Ozone RPC client leaks KeyProvider instances. (#2144)
---
.../org/apache/hadoop/ozone/OzoneConfigKeys.java | 7 +++
.../common/src/main/resources/ozone-default.xml | 9 ++++
.../apache/hadoop/ozone/client/rpc/RpcClient.java | 63 +++++++++++++++++++---
.../client/rpc/TestOzoneAtRestEncryption.java | 25 +++++++++
4 files changed, 98 insertions(+), 6 deletions(-)
diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java
index e9843bb..1992426 100644
--- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java
+++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java
@@ -28,6 +28,8 @@ import org.apache.hadoop.http.HttpConfig;
import org.apache.ratis.proto.RaftProtos.ReplicationLevel;
import org.apache.ratis.util.TimeDuration;
+import java.util.concurrent.TimeUnit;
+
/**
* This class contains constants for configuration keys used in Ozone.
*/
@@ -440,6 +442,11 @@ public final class OzoneConfigKeys {
public static final boolean OZONE_OM_KEYNAME_CHARACTER_CHECK_ENABLED_DEFAULT =
false;
+ public static final String OZONE_CLIENT_KEY_PROVIDER_CACHE_EXPIRY =
+ "ozone.client.key.provider.cache.expiry";
+ public static final long OZONE_CLIENT_KEY_PROVIDER_CACHE_EXPIRY_DEFAULT =
+ TimeUnit.DAYS.toMillis(10); // 10 days
+
/**
* There is no need to instantiate this class.
*/
diff --git a/hadoop-hdds/common/src/main/resources/ozone-default.xml b/hadoop-hdds/common/src/main/resources/ozone-default.xml
index ee18441..b2fd609 100644
--- a/hadoop-hdds/common/src/main/resources/ozone-default.xml
+++ b/hadoop-hdds/common/src/main/resources/ozone-default.xml
@@ -2770,4 +2770,13 @@
during OzoneManager init/SCM bootstrap.
</description>
</property>
+
+ <property>
+ <name>ozone.client.key.provider.cache.expiry</name>
+ <tag>OZONE, CLIENT, SECURITY</tag>
+ <value>10d</value>
+ <description>Ozone client security key provider cache expiration time.
+ </description>
+ </property>
+
</configuration>
diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
index 0974682..f7f9cc7 100644
--- a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
+++ b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
@@ -18,6 +18,7 @@
package org.apache.hadoop.ozone.client.rpc;
+import javax.annotation.Nonnull;
import javax.crypto.Cipher;
import javax.crypto.CipherInputStream;
import javax.crypto.CipherOutputStream;
@@ -32,6 +33,8 @@ import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.UUID;
+import java.util.concurrent.Callable;
+import java.util.concurrent.TimeUnit;
import java.util.function.Function;
import java.util.stream.Collectors;
@@ -113,8 +116,14 @@ import org.apache.hadoop.security.token.Token;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
+import com.google.common.cache.Cache;
+import com.google.common.cache.CacheBuilder;
+import com.google.common.cache.RemovalListener;
+import com.google.common.cache.RemovalNotification;
import static org.apache.hadoop.ozone.OzoneAcl.AclScope.ACCESS;
+import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_CLIENT_KEY_PROVIDER_CACHE_EXPIRY;
+import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_CLIENT_KEY_PROVIDER_CACHE_EXPIRY_DEFAULT;
import static org.apache.hadoop.ozone.OzoneConsts.OLD_QUOTA_DEFAULT;
import org.apache.logging.log4j.util.Strings;
@@ -146,6 +155,7 @@ public class RpcClient implements ClientProtocol {
private final boolean topologyAwareReadEnabled;
private final boolean checkKeyNameEnabled;
private final OzoneClientConfig clientConfig;
+ private final Cache<URI, KeyProvider> keyProviderCache;
/**
* Creates RpcClient instance with the given configuration.
@@ -194,7 +204,7 @@ public class RpcClient implements ClientProtocol {
int configuredChunkSize = (int) conf
.getStorageSize(ScmConfigKeys.OZONE_SCM_CHUNK_SIZE_KEY,
ScmConfigKeys.OZONE_SCM_CHUNK_SIZE_DEFAULT, StorageUnit.BYTES);
- if(configuredChunkSize > OzoneConsts.OZONE_SCM_CHUNK_MAX_SIZE) {
+ if (configuredChunkSize > OzoneConsts.OZONE_SCM_CHUNK_MAX_SIZE) {
LOG.warn("The chunk size ({}) is not allowed to be more than"
+ " the maximum size ({}),"
+ " resetting to the maximum size.",
@@ -208,15 +218,34 @@ public class RpcClient implements ClientProtocol {
OzoneConfigKeys.OZONE_SCM_BLOCK_SIZE_DEFAULT, StorageUnit.BYTES);
unsafeByteBufferConversion = conf.getBoolean(
- OzoneConfigKeys.OZONE_UNSAFEBYTEOPERATIONS_ENABLED,
- OzoneConfigKeys.OZONE_UNSAFEBYTEOPERATIONS_ENABLED_DEFAULT);
+ OzoneConfigKeys.OZONE_UNSAFEBYTEOPERATIONS_ENABLED,
+ OzoneConfigKeys.OZONE_UNSAFEBYTEOPERATIONS_ENABLED_DEFAULT);
topologyAwareReadEnabled = conf.getBoolean(
OzoneConfigKeys.OZONE_NETWORK_TOPOLOGY_AWARE_READ_KEY,
OzoneConfigKeys.OZONE_NETWORK_TOPOLOGY_AWARE_READ_DEFAULT);
checkKeyNameEnabled = conf.getBoolean(
- OMConfigKeys.OZONE_OM_KEYNAME_CHARACTER_CHECK_ENABLED_KEY,
- OMConfigKeys.OZONE_OM_KEYNAME_CHARACTER_CHECK_ENABLED_DEFAULT);
+ OMConfigKeys.OZONE_OM_KEYNAME_CHARACTER_CHECK_ENABLED_KEY,
+ OMConfigKeys.OZONE_OM_KEYNAME_CHARACTER_CHECK_ENABLED_DEFAULT);
+
+ long keyProviderCacheExpiryMs = conf.getTimeDuration(
+ OZONE_CLIENT_KEY_PROVIDER_CACHE_EXPIRY,
+ OZONE_CLIENT_KEY_PROVIDER_CACHE_EXPIRY_DEFAULT, TimeUnit.MILLISECONDS);
+ keyProviderCache = CacheBuilder.newBuilder()
+ .expireAfterAccess(keyProviderCacheExpiryMs, TimeUnit.MILLISECONDS)
+ .removalListener(new RemovalListener<URI, KeyProvider>() {
+ @Override
+ public void onRemoval(
+ @Nonnull RemovalNotification<URI, KeyProvider> notification) {
+ try {
+ assert notification.getValue() != null;
+ notification.getValue().close();
+ } catch (Throwable t) {
+ LOG.error("Error closing KeyProvider with uri [" +
+ notification.getKey() + "]", t);
+ }
+ }
+ }).build();
}
@Override
@@ -879,6 +908,8 @@ public class RpcClient implements ClientProtocol {
@Override
public void close() throws IOException {
IOUtils.cleanupWithLogger(LOG, ozoneManagerClient, xceiverClientManager);
+ keyProviderCache.invalidateAll();
+ keyProviderCache.cleanUp();
}
@Override
@@ -1315,7 +1346,22 @@ public class RpcClient implements ClientProtocol {
@Override
public KeyProvider getKeyProvider() throws IOException {
- return OzoneKMSUtil.getKeyProvider(conf, getKeyProviderUri());
+ URI kmsUri = getKeyProviderUri();
+ if (kmsUri == null) {
+ return null;
+ }
+
+ try {
+ return keyProviderCache.get(kmsUri, new Callable<KeyProvider>() {
+ @Override
+ public KeyProvider call() throws Exception {
+ return OzoneKMSUtil.getKeyProvider(conf, kmsUri);
+ }
+ });
+ } catch (Exception e) {
+ LOG.error("Can't create KeyProvider for Ozone RpcClient.", e);
+ return null;
+ }
}
@Override
@@ -1335,4 +1381,9 @@ public class RpcClient implements ClientProtocol {
public OzoneManagerProtocol getOzoneManagerClient() {
return ozoneManagerClient;
}
+
+ @VisibleForTesting
+ public Cache<URI, KeyProvider> getKeyProviderCache() {
+ return keyProviderCache;
+ }
}
diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneAtRestEncryption.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneAtRestEncryption.java
index 676c096..8a5bfea 100644
--- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneAtRestEncryption.java
+++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneAtRestEncryption.java
@@ -19,6 +19,7 @@ package org.apache.hadoop.ozone.client.rpc;
import java.io.File;
import java.io.IOException;
+import java.net.URI;
import java.nio.charset.StandardCharsets;
import java.security.NoSuchAlgorithmException;
import java.time.Instant;
@@ -31,6 +32,7 @@ import java.util.Random;
import java.util.TreeMap;
import java.util.UUID;
+import com.google.common.cache.Cache;
import org.apache.hadoop.conf.StorageUnit;
import org.apache.hadoop.crypto.key.KeyProvider;
import org.apache.hadoop.crypto.key.kms.KMSClientProvider;
@@ -76,6 +78,7 @@ import org.junit.AfterClass;
import org.junit.Assert;
import org.junit.BeforeClass;
import org.junit.Test;
+import org.mockito.Mockito;
/**
* This class is to test all the public facing APIs of Ozone Client.
@@ -493,4 +496,26 @@ public class TestOzoneAtRestEncryption {
offset + " and length " + readData.length,
inputDataForComparison, readData);
}
+
+ @Test
+ public void testGetKeyProvider() throws Exception {
+ KeyProvider kp1 = store.getKeyProvider();
+ KeyProvider kpSpy = Mockito.spy(kp1);
+ Assert.assertNotEquals(kpSpy, kp1);
+ Cache<URI, KeyProvider> cacheSpy =
+ ((RpcClient)store.getClientProxy()).getKeyProviderCache();
+ cacheSpy.put(store.getKeyProviderUri(), kpSpy);
+ KeyProvider kp2 = store.getKeyProvider();
+ Assert.assertEquals(kpSpy, kp2);
+
+ // Verify the spied key provider is closed upon ozone client close
+ ozClient.close();
+ Mockito.verify(kpSpy).close();
+
+ KeyProvider kp3 = ozClient.getObjectStore().getKeyProvider();
+ Assert.assertNotEquals(kp3, kpSpy);
+ // Restore ozClient and store
+ TestOzoneRpcClient.setOzClient(OzoneClientFactory.getRpcClient(conf));
+ TestOzoneRpcClient.setStore(ozClient.getObjectStore());
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@ozone.apache.org
For additional commands, e-mail: commits-help@ozone.apache.org