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 <x...@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-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-commits-h...@hadoop.apache.org