You are viewing a plain text version of this content. The canonical link for it is here.
Posted to hdfs-commits@hadoop.apache.org by el...@apache.org on 2019/10/13 06:51:07 UTC
[hadoop-ozone] 15/18: Fix review comments
This is an automated email from the ASF dual-hosted git repository.
elek pushed a commit to branch HDDS-2181
in repository https://gitbox.apache.org/repos/asf/hadoop-ozone.git
commit 90ca1247b36fa384ae17ddd2dd11a7a5603ae72d
Author: Vivek Ratnavel Subramanian <vi...@gmail.com>
AuthorDate: Wed Oct 9 17:11:55 2019 -0700
Fix review comments
---
.../main/java/org/apache/hadoop/ozone/OmUtils.java | 14 ----------
.../org/apache/hadoop/ozone/om/KeyManagerImpl.java | 32 ++++++++--------------
.../org/apache/hadoop/ozone/om/OzoneManager.java | 10 +++++++
.../om/request/key/OMAllocateBlockRequest.java | 21 ++------------
.../ozone/om/request/key/OMKeyCommitRequest.java | 22 ++-------------
.../hadoop/ozone/om/request/key/OMKeyRequest.java | 31 +++++++++++++++++++++
6 files changed, 57 insertions(+), 73 deletions(-)
diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OmUtils.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OmUtils.java
index ad33cae..8e129c9 100644
--- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OmUtils.java
+++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OmUtils.java
@@ -57,8 +57,6 @@ import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
import static org.apache.hadoop.hdds.HddsUtils.getHostNameFromConfigKeys;
import static org.apache.hadoop.hdds.HddsUtils.getPortNumberFromConfigKeys;
-import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_ACL_AUTHORIZER_CLASS;
-import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_ACL_AUTHORIZER_CLASS_NATIVE;
import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_ADDRESS_KEY;
import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_BIND_HOST_DEFAULT;
import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_HTTPS_ADDRESS_KEY;
@@ -527,16 +525,4 @@ public final class OmUtils {
return repeatedOmKeyInfo;
}
-
- /**
- * Returns true if OzoneNativeAuthorizer is configured in the configuration.
- * @param configuration ozone configuration
- * @return true if OzoneNativeAuthorizer is configured in the configuration;
- * else false.
- */
- public static boolean isNativeAuthorizerEnabled(Configuration configuration) {
- String authorizer = configuration.get(OZONE_ACL_AUTHORIZER_CLASS);
- return authorizer != null &&
- authorizer.equals(OZONE_ACL_AUTHORIZER_CLASS_NATIVE);
- }
}
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
index faa65bb..d0be40b 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
@@ -1655,31 +1655,21 @@ public class KeyManagerImpl implements KeyManager {
metadataManager.getLock().acquireReadLock(BUCKET_LOCK, volume, bucket);
try {
validateBucket(volume, bucket);
- OmKeyInfo keyInfo = null;
- try {
- if (ozObject.getResourceType() == OPEN_KEY) {
- keyInfo = metadataManager.getOpenKeyTable().get(objectKey);
- } else {
- OzoneFileStatus fileStatus = getFileStatus(args);
- keyInfo = fileStatus.getKeyInfo();
- }
+ OmKeyInfo keyInfo;
- if (keyInfo == null) {
- // the key does not exist, but it is a parent "dir" of some key
- // let access be determined based on volume/bucket/prefix ACL
- LOG.debug("key:{} is non-existent parent, permit access to user:{}",
- keyName, context.getClientUgi());
- return true;
- }
- } catch (OMException e) {
- if (e.getResult() == FILE_NOT_FOUND) {
- keyInfo = metadataManager.getOpenKeyTable().get(objectKey);
- }
+ if (ozObject.getResourceType() == OPEN_KEY) {
+ keyInfo = metadataManager.getOpenKeyTable().get(objectKey);
+ } else {
+ OzoneFileStatus fileStatus = getFileStatus(args);
+ keyInfo = fileStatus.getKeyInfo();
}
if (keyInfo == null) {
- throw new OMException("Key not found, checkAccess failed. Key:" +
- objectKey, KEY_NOT_FOUND);
+ // the key does not exist, but it is a parent "dir" of some key
+ // let access be determined based on volume/bucket/prefix ACL
+ LOG.debug("key:{} is non-existent parent, permit access to user:{}",
+ keyName, context.getClientUgi());
+ return true;
}
boolean hasAccess = OzoneAclUtil.checkAclRight(
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
index a6503d7..ba157bc 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
@@ -301,6 +301,8 @@ public final class OzoneManager extends ServiceRuntimeInfoImpl
private final boolean grpcBlockTokenEnabled;
private final boolean useRatisForReplication;
+ private boolean isNativeAuthorizerEnabled;
+
private OzoneManager(OzoneConfiguration conf) throws IOException,
AuthenticationException {
super(OzoneVersionInfo.OZONE_VERSION_INFO);
@@ -473,6 +475,7 @@ public final class OzoneManager extends ServiceRuntimeInfoImpl
if (accessAuthorizer instanceof OzoneNativeAuthorizer) {
OzoneNativeAuthorizer authorizer =
(OzoneNativeAuthorizer) accessAuthorizer;
+ isNativeAuthorizerEnabled = true;
authorizer.setVolumeManager(volumeManager);
authorizer.setBucketManager(bucketManager);
authorizer.setKeyManager(keyManager);
@@ -3290,4 +3293,11 @@ public final class OzoneManager extends ServiceRuntimeInfoImpl
return ozAdmins;
}
+ /**
+ * Returns true if OzoneNativeAuthorizer is enabled and false if otherwise.
+ * @return if native authorizer is enabled.
+ */
+ public boolean isNativeAuthorizerEnabled() {
+ return isNativeAuthorizerEnabled;
+ }
}
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMAllocateBlockRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMAllocateBlockRequest.java
index ef2af6d..7bc8738 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMAllocateBlockRequest.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMAllocateBlockRequest.java
@@ -25,11 +25,8 @@ import java.util.Map;
import com.google.common.base.Optional;
import com.google.common.base.Preconditions;
-import org.apache.hadoop.conf.Configuration;
-import org.apache.hadoop.ozone.OmUtils;
import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
-import org.apache.hadoop.ozone.security.acl.OzoneObj;
import org.apache.hadoop.util.Time;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -173,22 +170,8 @@ public class OMAllocateBlockRequest extends OMKeyRequest {
OmKeyInfo omKeyInfo = null;
try {
// check Acl
- // Native authorizer requires client id as part of keyname to check
- // write ACL on key. Add client id to key name if ozone native
- // authorizer is configured.
- Configuration config = ozoneManager.getConfiguration();
- if (OmUtils.isNativeAuthorizerEnabled(config)) {
- String keyNameForAclCheck =
- keyName + "/" + allocateBlockRequest.getClientID();
- // During allocate block request, it is possible that key is
- // not present in the key table and hence setting the resource type
- // to OPEN_KEY to check the openKeyTable.
- checkKeyAcls(ozoneManager, volumeName, bucketName, keyNameForAclCheck,
- IAccessAuthorizer.ACLType.WRITE, OzoneObj.ResourceType.OPEN_KEY);
- } else {
- checkKeyAcls(ozoneManager, volumeName, bucketName, keyName,
- IAccessAuthorizer.ACLType.WRITE, OzoneObj.ResourceType.KEY);
- }
+ checkKeyAclsInOpenKeyTable(ozoneManager, volumeName, bucketName, keyName,
+ IAccessAuthorizer.ACLType.WRITE, allocateBlockRequest.getClientID());
OMMetadataManager omMetadataManager = ozoneManager.getMetadataManager();
validateBucketAndVolume(omMetadataManager, volumeName,
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java
index 63ea5a0..811ecf7 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java
@@ -25,11 +25,8 @@ import java.util.stream.Collectors;
import com.google.common.base.Optional;
import com.google.common.base.Preconditions;
-import org.apache.hadoop.conf.Configuration;
-import org.apache.hadoop.ozone.OmUtils;
import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
-import org.apache.hadoop.ozone.security.acl.OzoneObj;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -120,22 +117,9 @@ public class OMKeyCommitRequest extends OMKeyRequest {
OMMetadataManager omMetadataManager = ozoneManager.getMetadataManager();
try {
// check Acl
- // Native authorizer requires client id as part of keyname to check
- // write ACL on key. Add client id to key name if ozone native
- // authorizer is configured.
- Configuration config = ozoneManager.getConfiguration();
- if (OmUtils.isNativeAuthorizerEnabled(config)) {
- String keyNameForAclCheck =
- keyName + "/" + commitKeyRequest.getClientID();
- // During key commit request, it is possible that key is
- // not present in the key table and hence setting the resource type
- // to OPEN_KEY to check the openKeyTable.
- checkKeyAcls(ozoneManager, volumeName, bucketName, keyNameForAclCheck,
- IAccessAuthorizer.ACLType.WRITE, OzoneObj.ResourceType.OPEN_KEY);
- } else {
- checkKeyAcls(ozoneManager, volumeName, bucketName, keyName,
- IAccessAuthorizer.ACLType.WRITE, OzoneObj.ResourceType.KEY);
- }
+ checkKeyAclsInOpenKeyTable(ozoneManager, volumeName, bucketName,
+ keyName, IAccessAuthorizer.ACLType.WRITE,
+ commitKeyRequest.getClientID());
List<OmKeyLocationInfo> locationInfoList = commitKeyArgs
.getKeyLocationsList().stream()
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java
index 16e97e8..73753d8 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java
@@ -523,6 +523,8 @@ public abstract class OMKeyRequest extends OMClientRequest {
* @param volume
* @param bucket
* @param key
+ * @param aclType
+ * @param resourceType
* @throws IOException
*/
protected void checkKeyAcls(OzoneManager ozoneManager, String volume,
@@ -535,4 +537,33 @@ public abstract class OMKeyRequest extends OMClientRequest {
}
}
+ /**
+ * Check ACLs for Ozone Key in OpenKey table
+ * if ozone native authorizer is enabled.
+ * @param ozoneManager
+ * @param volume
+ * @param bucket
+ * @param key
+ * @param aclType
+ * @param clientId
+ * @throws IOException
+ */
+ protected void checkKeyAclsInOpenKeyTable(OzoneManager ozoneManager,
+ String volume, String bucket, String key,
+ IAccessAuthorizer.ACLType aclType, long clientId) throws IOException {
+ // Native authorizer requires client id as part of key name to check
+ // write ACL on key. Add client id to key name if ozone native
+ // authorizer is configured.
+ if (ozoneManager.isNativeAuthorizerEnabled()) {
+ String keyNameForAclCheck = key + "/" + clientId;
+ // During key commit request, it is possible that key is
+ // not present in the key table and hence setting the resource type
+ // to OPEN_KEY to check the openKeyTable.
+ checkKeyAcls(ozoneManager, volume, bucket, keyNameForAclCheck,
+ aclType, OzoneObj.ResourceType.OPEN_KEY);
+ } else {
+ checkKeyAcls(ozoneManager, volume, bucket, key,
+ aclType, OzoneObj.ResourceType.KEY);
+ }
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-commits-unsubscribe@hadoop.apache.org
For additional commands, e-mail: hdfs-commits-help@hadoop.apache.org