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 xy...@apache.org on 2019/10/15 20:43:00 UTC
[hadoop-ozone] 01/01: Revert "HDDS 2181. Ozone Manager should send
correct ACL type in ACL requests to Authorizer"
This is an automated email from the ASF dual-hosted git repository.
xyao pushed a commit to branch revert-24-HDDS-2181
in repository https://gitbox.apache.org/repos/asf/hadoop-ozone.git
commit e6111d4d2b77ed664e8ac57545c34b22279fac56
Author: Xiaoyu Yao <xy...@apache.org>
AuthorDate: Tue Oct 15 13:42:47 2019 -0700
Revert "HDDS 2181. Ozone Manager should send correct ACL type in ACL requests to Authorizer"
---
...OzoneManagerProtocolClientSideTranslatorPB.java | 2 +-
.../ozone/security/acl/IAccessAuthorizer.java | 2 +-
.../org/apache/hadoop/ozone/om/TestOmAcls.java | 6 +--
.../security/acl/TestOzoneNativeAuthorizer.java | 23 ++--------
.../org/apache/hadoop/ozone/om/KeyManagerImpl.java | 39 ++++++++---------
.../org/apache/hadoop/ozone/om/OzoneManager.java | 10 -----
.../om/request/bucket/OMBucketCreateRequest.java | 2 +-
.../om/request/bucket/OMBucketDeleteRequest.java | 2 +-
.../om/request/file/OMDirectoryCreateRequest.java | 5 +--
.../ozone/om/request/file/OMFileCreateRequest.java | 5 +--
.../om/request/key/OMAllocateBlockRequest.java | 4 +-
.../ozone/om/request/key/OMKeyCommitRequest.java | 17 +++-----
.../ozone/om/request/key/OMKeyCreateRequest.java | 5 +--
.../ozone/om/request/key/OMKeyDeleteRequest.java | 5 +--
.../ozone/om/request/key/OMKeyRenameRequest.java | 10 +----
.../hadoop/ozone/om/request/key/OMKeyRequest.java | 39 +++--------------
.../ozone/security/acl/OzoneNativeAuthorizer.java | 50 +++++-----------------
.../ozone/om/request/key/TestOMKeyRequest.java | 1 -
18 files changed, 55 insertions(+), 172 deletions(-)
diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java
index ee9e19a..c9dc8ec 100644
--- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java
+++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java
@@ -340,7 +340,7 @@ public final class OzoneManagerProtocolClientSideTranslatorPB
if (omResponse.hasLeaderOMNodeId() && omFailoverProxyProvider != null) {
String leaderOmId = omResponse.getLeaderOMNodeId();
- // Failover to the OM node returned by OMResponse leaderOMNodeId if
+ // Failover to the OM node returned by OMReponse leaderOMNodeId if
// current proxy is not pointing to that node.
omFailoverProxyProvider.performFailoverIfRequired(leaderOmId);
}
diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/security/acl/IAccessAuthorizer.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/security/acl/IAccessAuthorizer.java
index 939f2c1..d8a2660 100644
--- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/security/acl/IAccessAuthorizer.java
+++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/security/acl/IAccessAuthorizer.java
@@ -64,7 +64,7 @@ public interface IAccessAuthorizer {
public static ACLType getAclTypeFromOrdinal(int ordinal) {
if (ordinal > length - 1 && ordinal > -1) {
- throw new IllegalArgumentException("Ordinal greater than array length" +
+ throw new IllegalArgumentException("Ordinal greater than array lentgh" +
". ordinal:" + ordinal);
}
return vals[ordinal];
diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmAcls.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmAcls.java
index ebf5964..c75e365 100644
--- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmAcls.java
+++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmAcls.java
@@ -118,7 +118,7 @@ public class TestOmAcls {
() -> volume.createBucket(bucketName));
assertTrue(logCapturer.getOutput()
- .contains("doesn't have CREATE permission to access bucket"));
+ .contains("doesn't have CREATE permission to access volume"));
}
@Test
@@ -133,8 +133,8 @@ public class TestOmAcls {
OzoneTestUtils.expectOmException(ResultCodes.PERMISSION_DENIED,
() -> TestDataUtil.createKey(bucket, "testKey", "testcontent"));
- assertTrue(logCapturer.getOutput().contains("doesn't have CREATE " +
- "permission to access key"));
+ assertTrue(logCapturer.getOutput().contains("doesn't have WRITE " +
+ "permission to access bucket"));
}
/**
diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/security/acl/TestOzoneNativeAuthorizer.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/security/acl/TestOzoneNativeAuthorizer.java
index c7bda9b..43ce679 100644
--- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/security/acl/TestOzoneNativeAuthorizer.java
+++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/security/acl/TestOzoneNativeAuthorizer.java
@@ -69,8 +69,6 @@ import static org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLIdentity
import static org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLIdentityType.USER;
import static org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLIdentityType.WORLD;
import static org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLType.ALL;
-import static org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLType.CREATE;
-import static org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLType.WRITE;
import static org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLType.NONE;
import static org.apache.hadoop.ozone.security.acl.OzoneObj.ResourceType.BUCKET;
import static org.apache.hadoop.ozone.security.acl.OzoneObj.ResourceType.KEY;
@@ -109,7 +107,6 @@ public class TestOzoneNativeAuthorizer {
private static OzoneObj buckObj;
private static OzoneObj keyObj;
private static OzoneObj prefixObj;
- private static long keySessionId;
@Parameterized.Parameters
public static Collection<Object[]> data() {
@@ -195,7 +192,6 @@ public class TestOzoneNativeAuthorizer {
keySession.getKeyInfo().getLatestVersionLocations()
.getLocationList());
keyManager.commitKey(keyArgs, keySession.getId());
- keySessionId = keySession.getId();
}
keyObj = new OzoneObjInfo.Builder()
@@ -362,21 +358,10 @@ public class TestOzoneNativeAuthorizer {
List<ACLType> aclsToBeAdded =
Arrays.stream(ACLType.values()).collect(Collectors.toList());
aclsToBeValidated.remove(NONE);
- // Do not validate "WRITE" since write acl type requires object to be
- // present in OpenKeyTable.
- aclsToBeValidated.remove(WRITE);
aclsToBeValidated.remove(a1);
aclsToBeAdded.remove(NONE);
aclsToBeAdded.remove(ALL);
- // AclType "CREATE" is skipped from access check on objects
- // since the object will not exist during access check.
- aclsToBeAdded.remove(CREATE);
- // AclType "WRITE" is removed from being tested here,
- // because object must always be present in OpenKeyTable for write
- // acl requests. But, here the objects are already committed
- // and will move to keyTable.
- aclsToBeAdded.remove(WRITE);
// Fetch acls again.
for (ACLType a2 : aclsToBeAdded) {
@@ -385,7 +370,7 @@ public class TestOzoneNativeAuthorizer {
acls = aclImplementor.getAcl(obj);
List right = acls.stream().map(a -> a.getAclList()).collect(
Collectors.toList());
- assertFalse("Did not expect client to have " + a2 + " acl. " +
+ assertFalse("Do not expected client to have " + a2 + " acl. " +
"Current acls found:" + right + ". Type:" + accessType + ","
+ " name:" + (accessType == USER ? user : group),
nativeAuthorizer.checkAccess(obj,
@@ -425,7 +410,7 @@ public class TestOzoneNativeAuthorizer {
builder.setAclRights(a2).build()));
aclsToBeValidated.remove(a2);
for (ACLType a3 : aclsToBeValidated) {
- if (!a3.equals(a1) && !a3.equals(a2) && !a3.equals(CREATE)) {
+ if (!a3.equals(a1) && !a3.equals(a2)) {
assertFalse("User shouldn't have right " + a3 + ". " +
"Current acl rights for user:" + a1 + "," + a2,
nativeAuthorizer.checkAccess(obj,
@@ -435,6 +420,7 @@ public class TestOzoneNativeAuthorizer {
}
}
}
+
}
private String getAclName(ACLIdentityType identityType) {
@@ -476,9 +462,6 @@ public class TestOzoneNativeAuthorizer {
builder) throws OMException {
List<ACLType> allAcls = new ArrayList<>(Arrays.asList(ACLType.values()));
allAcls.remove(NONE);
- // Removing CREATE, WRITE since they need special handling.
- allAcls.remove(CREATE);
- allAcls.remove(WRITE);
for (ACLType a : allAcls) {
assertFalse("User shouldn't have right " + a + ".",
nativeAuthorizer.checkAccess(obj, builder.setAclRights(a).build()));
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 ea9bb82..20b7fdf 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
@@ -89,7 +89,6 @@ import org.apache.hadoop.ozone.om.helpers.OzoneFileStatus;
import org.apache.hadoop.ozone.om.helpers.RepeatedOmKeyInfo;
import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.PartKeyInfo;
import org.apache.hadoop.ozone.security.OzoneBlockTokenSecretManager;
-import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
import org.apache.hadoop.ozone.security.acl.OzoneObj;
import org.apache.hadoop.ozone.security.acl.RequestContext;
import org.apache.hadoop.security.SecurityUtil;
@@ -1655,30 +1654,28 @@ public class KeyManagerImpl implements KeyManager {
metadataManager.getLock().acquireReadLock(BUCKET_LOCK, volume, bucket);
try {
validateBucket(volume, bucket);
- OmKeyInfo keyInfo;
-
- // For Acl Type "WRITE", the key can only be found in
- // OpenKeyTable since appends to existing keys are not supported.
- if (context.getAclRights() == IAccessAuthorizer.ACLType.WRITE) {
- keyInfo = metadataManager.getOpenKeyTable().get(objectKey);
- } else {
- try {
- OzoneFileStatus fileStatus = getFileStatus(args);
- keyInfo = fileStatus.getKeyInfo();
- } catch (IOException e) {
- throw new OMException("Key not found, checkAccess failed. Key:" +
- objectKey, KEY_NOT_FOUND);
+ OmKeyInfo keyInfo = null;
+ try {
+ OzoneFileStatus fileStatus = getFileStatus(args);
+ keyInfo = fileStatus.getKeyInfo();
+ 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
+ if (LOG.isDebugEnabled()) {
+ 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 (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
- if (LOG.isDebugEnabled()) {
- LOG.debug("key:{} is non-existent parent, permit access to user:{}",
- keyName, context.getClientUgi());
- }
- return true;
+ throw new OMException("Key not found, checkAccess failed. Key:" +
+ objectKey, KEY_NOT_FOUND);
}
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 294b74e..0cd087e 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,8 +301,6 @@ 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);
@@ -475,7 +473,6 @@ 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);
@@ -3295,11 +3292,4 @@ 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/bucket/OMBucketCreateRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java
index 33a6310..2b2448d 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java
@@ -143,7 +143,7 @@ public class OMBucketCreateRequest extends OMClientRequest {
try {
// check Acl
if (ozoneManager.getAclsEnabled()) {
- checkAcls(ozoneManager, OzoneObj.ResourceType.BUCKET,
+ checkAcls(ozoneManager, OzoneObj.ResourceType.VOLUME,
OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.CREATE,
volumeName, bucketName, null);
}
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketDeleteRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketDeleteRequest.java
index 5444a33..9469f88 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketDeleteRequest.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketDeleteRequest.java
@@ -95,7 +95,7 @@ public class OMBucketDeleteRequest extends OMClientRequest {
// check Acl
if (ozoneManager.getAclsEnabled()) {
checkAcls(ozoneManager, OzoneObj.ResourceType.BUCKET,
- OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.DELETE,
+ OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.WRITE,
volumeName, bucketName, null);
}
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMDirectoryCreateRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMDirectoryCreateRequest.java
index aaac874..4b591db 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMDirectoryCreateRequest.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMDirectoryCreateRequest.java
@@ -32,8 +32,6 @@ import org.apache.hadoop.ozone.om.helpers.OmKeyLocationInfoGroup;
import org.apache.hadoop.ozone.om.helpers.OzoneAclUtil;
import org.apache.hadoop.ozone.om.helpers.OzoneFSUtils;
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;
@@ -129,8 +127,7 @@ public class OMDirectoryCreateRequest extends OMKeyRequest {
OMClientResponse omClientResponse = null;
try {
// check Acl
- checkKeyAcls(ozoneManager, volumeName, bucketName, keyName,
- IAccessAuthorizer.ACLType.CREATE, OzoneObj.ResourceType.KEY);
+ checkBucketAcls(ozoneManager, volumeName, bucketName, keyName);
// Check if this is the root of the filesystem.
if (keyName.length() == 0) {
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileCreateRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileCreateRequest.java
index 52af0a3..20b5174 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileCreateRequest.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileCreateRequest.java
@@ -31,8 +31,6 @@ import javax.annotation.Nonnull;
import com.google.common.base.Optional;
import com.google.common.base.Preconditions;
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;
@@ -179,8 +177,7 @@ public class OMFileCreateRequest extends OMKeyRequest {
OMClientResponse omClientResponse = null;
try {
// check Acl
- checkKeyAcls(ozoneManager, volumeName, bucketName, keyName,
- IAccessAuthorizer.ACLType.CREATE, OzoneObj.ResourceType.KEY);
+ checkBucketAcls(ozoneManager, volumeName, bucketName, keyName);
// acquire lock
acquiredLock = omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK,
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 7bc8738..e800927 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
@@ -26,7 +26,6 @@ import java.util.Map;
import com.google.common.base.Optional;
import com.google.common.base.Preconditions;
import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
-import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
import org.apache.hadoop.util.Time;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -170,8 +169,7 @@ public class OMAllocateBlockRequest extends OMKeyRequest {
OmKeyInfo omKeyInfo = null;
try {
// check Acl
- checkKeyAclsInOpenKeyTable(ozoneManager, volumeName, bucketName, keyName,
- IAccessAuthorizer.ACLType.WRITE, allocateBlockRequest.getClientID());
+ checkBucketAcls(ozoneManager, volumeName, bucketName, keyName);
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 811ecf7..196d61c 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
@@ -26,7 +26,6 @@ import java.util.stream.Collectors;
import com.google.common.base.Optional;
import com.google.common.base.Preconditions;
import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper;
-import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -112,14 +111,11 @@ public class OMKeyCommitRequest extends OMKeyRequest {
IOException exception = null;
OmKeyInfo omKeyInfo = null;
OMClientResponse omClientResponse = null;
- boolean bucketLockAcquired = false;
OMMetadataManager omMetadataManager = ozoneManager.getMetadataManager();
try {
// check Acl
- checkKeyAclsInOpenKeyTable(ozoneManager, volumeName, bucketName,
- keyName, IAccessAuthorizer.ACLType.WRITE,
- commitKeyRequest.getClientID());
+ checkBucketAcls(ozoneManager, volumeName, bucketName, keyName);
List<OmKeyLocationInfo> locationInfoList = commitKeyArgs
.getKeyLocationsList().stream()
@@ -131,8 +127,8 @@ public class OMKeyCommitRequest extends OMKeyRequest {
String dbOpenKey = omMetadataManager.getOpenKey(volumeName, bucketName,
keyName, commitKeyRequest.getClientID());
- bucketLockAcquired = omMetadataManager.getLock()
- .acquireWriteLock(BUCKET_LOCK, volumeName, bucketName);
+ omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK, volumeName,
+ bucketName);
validateBucketAndVolume(omMetadataManager, volumeName, bucketName);
omKeyInfo = omMetadataManager.getOpenKeyTable().get(dbOpenKey);
@@ -170,11 +166,8 @@ public class OMKeyCommitRequest extends OMKeyRequest {
ozoneManagerDoubleBufferHelper.add(omClientResponse,
transactionLogIndex));
}
-
- if(bucketLockAcquired) {
- omMetadataManager.getLock().releaseWriteLock(BUCKET_LOCK, volumeName,
- bucketName);
- }
+ omMetadataManager.getLock().releaseWriteLock(BUCKET_LOCK, volumeName,
+ bucketName);
}
// Performing audit logging outside of the lock.
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequest.java
index 9681b20..baa13ad 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequest.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequest.java
@@ -26,8 +26,6 @@ import java.util.stream.Collectors;
import com.google.common.base.Optional;
import com.google.common.base.Preconditions;
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;
@@ -164,8 +162,7 @@ public class OMKeyCreateRequest extends OMKeyRequest {
OMClientResponse omClientResponse = null;
try {
// check Acl
- checkKeyAcls(ozoneManager, volumeName, bucketName, keyName,
- IAccessAuthorizer.ACLType.CREATE, OzoneObj.ResourceType.KEY);
+ checkBucketAcls(ozoneManager, volumeName, bucketName, keyName);
acquireLock = omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK,
volumeName, bucketName);
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java
index 28dfaa5..ee4b9b2 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java
@@ -23,8 +23,6 @@ import java.util.Map;
import com.google.common.base.Optional;
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;
@@ -111,8 +109,7 @@ public class OMKeyDeleteRequest extends OMKeyRequest {
OMClientResponse omClientResponse = null;
try {
// check Acl
- checkKeyAcls(ozoneManager, volumeName, bucketName, keyName,
- IAccessAuthorizer.ACLType.DELETE, OzoneObj.ResourceType.KEY);
+ checkKeyAcls(ozoneManager, volumeName, bucketName, keyName);
String objectKey = omMetadataManager.getOzoneKey(
volumeName, bucketName, keyName);
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRenameRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRenameRequest.java
index 6f7ff60..526473c 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRenameRequest.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRenameRequest.java
@@ -24,8 +24,6 @@ import java.util.Map;
import com.google.common.base.Optional;
import com.google.common.base.Preconditions;
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;
@@ -119,12 +117,8 @@ public class OMKeyRenameRequest extends OMKeyRequest {
throw new OMException("Key name is empty",
OMException.ResultCodes.INVALID_KEY_NAME);
}
- // check Acls to see if user has access to perform delete operation on
- // old key and create operation on new key
- checkKeyAcls(ozoneManager, volumeName, bucketName, fromKeyName,
- IAccessAuthorizer.ACLType.DELETE, OzoneObj.ResourceType.KEY);
- checkKeyAcls(ozoneManager, volumeName, bucketName, toKeyName,
- IAccessAuthorizer.ACLType.CREATE, OzoneObj.ResourceType.KEY);
+ // check Acl
+ checkKeyAcls(ozoneManager, volumeName, bucketName, fromKeyName);
acquiredLock = omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK,
volumeName, bucketName);
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 eb93018..8e1e760 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
@@ -507,11 +507,10 @@ public abstract class OMKeyRequest extends OMClientRequest {
* @throws IOException
*/
protected void checkBucketAcls(OzoneManager ozoneManager, String volume,
- String bucket, String key, IAccessAuthorizer.ACLType aclType)
- throws IOException {
+ String bucket, String key) throws IOException {
if (ozoneManager.getAclsEnabled()) {
checkAcls(ozoneManager, OzoneObj.ResourceType.BUCKET,
- OzoneObj.StoreType.OZONE, aclType,
+ OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.WRITE,
volume, bucket, key);
}
}
@@ -523,43 +522,15 @@ 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,
- String bucket, String key, IAccessAuthorizer.ACLType aclType,
- OzoneObj.ResourceType resourceType)
- throws IOException {
+ String bucket, String key) throws IOException {
if (ozoneManager.getAclsEnabled()) {
- checkAcls(ozoneManager, resourceType, OzoneObj.StoreType.OZONE, aclType,
+ checkAcls(ozoneManager, OzoneObj.ResourceType.KEY,
+ OzoneObj.StoreType.OZONE, IAccessAuthorizer.ACLType.WRITE,
volume, bucket, key);
}
}
- /**
- * 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 {
- String keyNameForAclCheck = key;
- // 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()) {
- keyNameForAclCheck = key + "/" + clientId;
- }
-
- checkKeyAcls(ozoneManager, volume, bucket, keyNameForAclCheck,
- aclType, OzoneObj.ResourceType.KEY);
- }
}
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/acl/OzoneNativeAuthorizer.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/acl/OzoneNativeAuthorizer.java
index c2a96bd..0b7c51a 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/acl/OzoneNativeAuthorizer.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/acl/OzoneNativeAuthorizer.java
@@ -69,9 +69,6 @@ public class OzoneNativeAuthorizer implements IAccessAuthorizer {
Objects.requireNonNull(ozObject);
Objects.requireNonNull(context);
OzoneObjInfo objInfo;
- RequestContext parentContext;
- boolean isACLTypeCreate = (context.getAclRights() == ACLType.CREATE);
- boolean isACLTypeDelete = (context.getAclRights() == ACLType.DELETE);
if (ozObject instanceof OzoneObjInfo) {
objInfo = (OzoneObjInfo) ozObject;
@@ -80,52 +77,25 @@ public class OzoneNativeAuthorizer implements IAccessAuthorizer {
"configured to work with OzoneObjInfo type only.", INVALID_REQUEST);
}
- // For CREATE and DELETE acl requests, the parents need to be checked
- // for WRITE acl. If Key create request is received, then we need to
- // check if user has WRITE acl set on Bucket and Volume. In all other cases
- // the parents also need to be checked for the same acl type.
- if (isACLTypeCreate || isACLTypeDelete) {
- parentContext = RequestContext.newBuilder()
- .setClientUgi(context.getClientUgi())
- .setIp(context.getIp())
- .setAclType(context.getAclType())
- .setAclRights(ACLType.WRITE)
- .build();
- } else {
- parentContext = context;
- }
-
switch (objInfo.getResourceType()) {
case VOLUME:
LOG.trace("Checking access for volume: {}", objInfo);
return volumeManager.checkAccess(objInfo, context);
case BUCKET:
LOG.trace("Checking access for bucket: {}", objInfo);
- // Skip bucket access check for CREATE acl since
- // bucket will not exist at the time of creation
- boolean bucketAccess = isACLTypeCreate
- || bucketManager.checkAccess(objInfo, context);
- return (bucketAccess
- && volumeManager.checkAccess(objInfo, parentContext));
+ return (bucketManager.checkAccess(objInfo, context)
+ && volumeManager.checkAccess(objInfo, context));
case KEY:
LOG.trace("Checking access for Key: {}", objInfo);
- // Skip key access check for CREATE acl since
- // key will not exist at the time of creation
- boolean keyAccess = isACLTypeCreate
- || keyManager.checkAccess(objInfo, context);
- return (keyAccess
- && prefixManager.checkAccess(objInfo, parentContext)
- && bucketManager.checkAccess(objInfo, parentContext)
- && volumeManager.checkAccess(objInfo, parentContext));
+ return (keyManager.checkAccess(objInfo, context)
+ && prefixManager.checkAccess(objInfo, context)
+ && bucketManager.checkAccess(objInfo, context)
+ && volumeManager.checkAccess(objInfo, context));
case PREFIX:
- LOG.trace("Checking access for Prefix: {}", objInfo);
- // Skip prefix access check for CREATE acl since
- // prefix will not exist at the time of creation
- boolean prefixAccess = isACLTypeCreate
- || prefixManager.checkAccess(objInfo, context);
- return (prefixAccess
- && bucketManager.checkAccess(objInfo, parentContext)
- && volumeManager.checkAccess(objInfo, parentContext));
+ LOG.trace("Checking access for Prefix: {]", objInfo);
+ return (prefixManager.checkAccess(objInfo, context)
+ && bucketManager.checkAccess(objInfo, context)
+ && volumeManager.checkAccess(objInfo, context));
default:
throw new OMException("Unexpected object type:" +
objInfo.getResourceType(), INVALID_REQUEST);
diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyRequest.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyRequest.java
index f634ff3..92d6cdb 100644
--- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyRequest.java
+++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyRequest.java
@@ -100,7 +100,6 @@ public class TestOMKeyRequest {
omMetadataManager = new OmMetadataManagerImpl(ozoneConfiguration);
when(ozoneManager.getMetrics()).thenReturn(omMetrics);
when(ozoneManager.getMetadataManager()).thenReturn(omMetadataManager);
- when(ozoneManager.getConfiguration()).thenReturn(ozoneConfiguration);
auditLogger = Mockito.mock(AuditLogger.class);
when(ozoneManager.getAuditLogger()).thenReturn(auditLogger);
Mockito.doNothing().when(auditLogger).logWrite(any(AuditMessage.class));
---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-commits-unsubscribe@hadoop.apache.org
For additional commands, e-mail: hdfs-commits-help@hadoop.apache.org