This is an automated email from the ASF dual-hosted git repository. rakeshr pushed a commit to branch HDDS-2939 in repository https://gitbox.apache.org/repos/asf/ozone.git
commit 5ed34dfc4142d959e0a6ae73e9dd04a9e8ff6fbb Author: Rakesh Radhakrishnan <[email protected]> AuthorDate: Sat Mar 27 08:30:13 2021 +0530 HDDS-4932. [FSO] Provide list subpaths function to perform recursive ACL check during delete and rename op (#2008) --- .../apache/hadoop/ozone/security/acl/OzoneObj.java | 2 + .../hadoop/ozone/security/acl/OzoneObjInfo.java | 20 ++- .../hadoop/ozone/security/acl/OzonePrefixPath.java | 67 +++++++++ .../hadoop/fs/ozone/TestOzoneFileSystem.java | 62 ++++++++ .../org/apache/hadoop/ozone/om/KeyManagerImpl.java | 42 ++---- .../org/apache/hadoop/ozone/om/OzoneManager.java | 17 +++ .../hadoop/ozone/om/OzonePrefixPathImpl.java | 161 +++++++++++++++++++++ .../hadoop/ozone/om/request/OMClientRequest.java | 48 ++++++ .../ozone/om/request/file/OMFileRequest.java | 36 +++++ .../ozone/om/request/key/OMKeyDeleteRequestV1.java | 8 +- .../ozone/om/request/key/OMKeyRenameRequestV1.java | 17 ++- .../om/request/key/TestOMKeyDeleteRequestV1.java | 68 +++++++++ .../hadoop/ozone/security/acl/TestOzoneObj.java | 77 ++++++++++ .../ozone/security/acl/TestRequestContext.java | 7 +- 14 files changed, 587 insertions(+), 45 deletions(-) diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/security/acl/OzoneObj.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/security/acl/OzoneObj.java index 4a95e55..1916d25 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/security/acl/OzoneObj.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/security/acl/OzoneObj.java @@ -73,6 +73,8 @@ public abstract class OzoneObj implements IOzoneObj { public abstract String getKeyName(); + public abstract OzonePrefixPath getOzonePrefixPathViewer(); + /** * Get PrefixName. * A prefix name is like a key name under the bucket but diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/security/acl/OzoneObjInfo.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/security/acl/OzoneObjInfo.java index 42ddbb9..76fb76a 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/security/acl/OzoneObjInfo.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/security/acl/OzoneObjInfo.java @@ -37,6 +37,8 @@ public final class OzoneObjInfo extends OzoneObj { private final String bucketName; private final String name; + private OzonePrefixPath ozonePrefixPath; + /** * * @param resType @@ -46,11 +48,13 @@ public final class OzoneObjInfo extends OzoneObj { * @param name - keyName/PrefixName */ private OzoneObjInfo(ResourceType resType, StoreType storeType, - String volumeName, String bucketName, String name) { + String volumeName, String bucketName, String name, + OzonePrefixPath ozonePrefixPath) { super(resType, storeType); this.volumeName = volumeName; this.bucketName = bucketName; this.name = name; + this.ozonePrefixPath = ozonePrefixPath; } @Override @@ -95,6 +99,10 @@ public final class OzoneObjInfo extends OzoneObj { return name; } + @Override + public OzonePrefixPath getOzonePrefixPathViewer() { + return ozonePrefixPath; + } public static OzoneObjInfo fromProtobuf(OzoneManagerProtocolProtos.OzoneObj proto) { @@ -154,6 +162,7 @@ public final class OzoneObjInfo extends OzoneObj { private String volumeName; private String bucketName; private String name; + private OzonePrefixPath ozonePrefixPath; public static Builder newBuilder() { return new Builder(); @@ -207,8 +216,15 @@ public final class OzoneObjInfo extends OzoneObj { return this; } + public Builder setOzonePrefixPath(OzonePrefixPath ozonePrefixPathViewer) { + this.ozonePrefixPath = ozonePrefixPathViewer; + return this; + } + + public OzoneObjInfo build() { - return new OzoneObjInfo(resType, storeType, volumeName, bucketName, name); + return new OzoneObjInfo(resType, storeType, volumeName, bucketName, + name, ozonePrefixPath); } } } diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/security/acl/OzonePrefixPath.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/security/acl/OzonePrefixPath.java new file mode 100644 index 0000000..4e91d5a --- /dev/null +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/security/acl/OzonePrefixPath.java @@ -0,0 +1,67 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with this + * work for additional information regarding copyright ownership. The ASF + * licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package org.apache.hadoop.ozone.security.acl; + +import org.apache.hadoop.ozone.om.helpers.OzoneFileStatus; + +import java.io.IOException; +import java.util.Iterator; + +/** + * Interface used to lists immediate children(sub-paths) for a given keyPrefix. + */ +public interface OzonePrefixPath { + + /** + * Returns file status for the given key path. + * + * @return OzoneFileStatus for the given path. + */ + OzoneFileStatus getOzoneFileStatus(); + + /** + * Lists immediate children(files or a directories) of the given keyPrefix. + * It won't do recursive traversal. The given keyPrefix parameter should be a + * directory type. + * + * Assume following is the Ozone FS tree structure. + * + * buck-1 + * | + * a + * | + * ----------------------------------- + * | | | + * b1 b2 b3 + * ----- -------- ---------- + * | | | | | | | | + * c1 c2 d1 d2 d3 e1 e2 e3 + * | | + * -------- | + * | | | + * d21.txt d22.txt e11.txt + * + * Say, KeyPrefix = "a" will return immediate children [a/b1, a/b2, a/b3]. + * Say, KeyPrefix = "a/b2" will return children [a/b2/d1, a/b2/d2, a/b2/d3]. + * + * @param keyPrefix keyPrefix name + * @return list of immediate files or directories under the given keyPrefix. + * @throws IOException + */ + Iterator<? extends OzoneFileStatus> getChildren(String keyPrefix) + throws IOException; +} diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystem.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystem.java index a8e89e1..3d25e72 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystem.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystem.java @@ -25,6 +25,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.Iterator; import java.util.Set; import java.util.TreeSet; @@ -48,9 +49,12 @@ import org.apache.hadoop.ozone.TestDataUtil; import org.apache.hadoop.ozone.client.OzoneBucket; import org.apache.hadoop.ozone.client.OzoneKeyDetails; import org.apache.hadoop.ozone.om.OMConfigKeys; +import org.apache.hadoop.ozone.om.OzonePrefixPathImpl; import org.apache.hadoop.ozone.om.TrashPolicyOzone; +import org.apache.hadoop.ozone.om.exceptions.OMException; import org.apache.hadoop.ozone.om.helpers.OmKeyArgs; import org.apache.hadoop.ozone.om.helpers.OpenKeySession; +import org.apache.hadoop.ozone.om.helpers.OzoneFileStatus; import org.apache.hadoop.ozone.om.request.TestOMRequestUtils; import org.apache.hadoop.security.UserGroupInformation; import org.apache.hadoop.test.GenericTestUtils; @@ -63,6 +67,7 @@ import static org.apache.hadoop.fs.FileSystem.TRASH_PREFIX; import static org.apache.hadoop.fs.ozone.Constants.LISTING_PAGE_SIZE; import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_ACL_ENABLED; import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_FS_ITERATE_BATCH_SIZE; +import static org.apache.hadoop.ozone.OzoneConsts.OM_KEY_PREFIX; import static org.apache.hadoop.ozone.OzoneConsts.OZONE_URI_DELIMITER; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -1265,4 +1270,61 @@ public class TestOzoneFileSystem { } }, 1000, 120000); } + + @Test + public void testListStatusOnLargeDirectoryForACLCheck() throws Exception { + String keyName = "dir1/dir2/testListStatusOnLargeDirectoryForACLCheck"; + Path root = new Path(OZONE_URI_DELIMITER, keyName); + Set<String> paths = new TreeSet<>(); + int numDirs = LISTING_PAGE_SIZE + LISTING_PAGE_SIZE / 2; + for (int i = 0; i < numDirs; i++) { + Path p = new Path(root, String.valueOf(i)); + getFs().mkdirs(p); + paths.add(keyName + OM_KEY_PREFIX + p.getName()); + } + + // unknown keyname + try { + new OzonePrefixPathImpl(getVolumeName(), getBucketName(), "invalidKey", + cluster.getOzoneManager().getKeyManager()); + Assert.fail("Non-existent key name!"); + } catch (OMException ome) { + Assert.assertEquals(OMException.ResultCodes.KEY_NOT_FOUND, + ome.getResult()); + } + + OzonePrefixPathImpl ozonePrefixPath = + new OzonePrefixPathImpl(getVolumeName(), getBucketName(), keyName, + cluster.getOzoneManager().getKeyManager()); + + OzoneFileStatus status = ozonePrefixPath.getOzoneFileStatus(); + Assert.assertNotNull(status); + Assert.assertEquals(keyName, status.getTrimmedName()); + Assert.assertTrue(status.isDirectory()); + + Iterator<? extends OzoneFileStatus> pathItr = + ozonePrefixPath.getChildren(keyName); + Assert.assertTrue("Failed to list keyPath:" + keyName, pathItr.hasNext()); + + Set<String> actualPaths = new TreeSet<>(); + while (pathItr.hasNext()) { + String pathname = pathItr.next().getTrimmedName(); + actualPaths.add(pathname); + + // no subpaths, expected an empty list + Iterator<? extends OzoneFileStatus> subPathItr = + ozonePrefixPath.getChildren(pathname); + Assert.assertNotNull(subPathItr); + Assert.assertFalse("Failed to list keyPath: " + pathname, + subPathItr.hasNext()); + } + + Assert.assertEquals("ListStatus failed", paths.size(), + actualPaths.size()); + + for (String pathname : actualPaths) { + paths.remove(pathname); + } + Assert.assertTrue("ListStatus failed:" + paths, paths.isEmpty()); + } } 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 b2ff866..422a915 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 @@ -270,28 +270,6 @@ public class KeyManagerImpl implements KeyManager { return metadataManager.getBucketTable().get(bucketKey); } - private void validateBucket(String volumeName, String bucketName) - throws IOException { - String bucketKey = metadataManager.getBucketKey(volumeName, bucketName); - // Check if bucket exists - if (metadataManager.getBucketTable().get(bucketKey) == null) { - String volumeKey = metadataManager.getVolumeKey(volumeName); - // If the volume also does not exist, we should throw volume not found - // exception - if (metadataManager.getVolumeTable().get(volumeKey) == null) { - LOG.error("volume not found: {}", volumeName); - throw new OMException("Volume not found", - VOLUME_NOT_FOUND); - } - - // if the volume exists but bucket does not exist, throw bucket not found - // exception - LOG.error("bucket not found: {}/{} ", volumeName, bucketName); - throw new OMException("Bucket not found", - BUCKET_NOT_FOUND); - } - } - /** * Check S3 bucket exists or not. * @param volumeName @@ -322,7 +300,7 @@ public class KeyManagerImpl implements KeyManager { String volumeName = args.getVolumeName(); String bucketName = args.getBucketName(); String keyName = args.getKeyName(); - validateBucket(volumeName, bucketName); + OMFileRequest.validateBucket(metadataManager, volumeName, bucketName); String openKey = metadataManager.getOpenKey( volumeName, bucketName, keyName, clientID); @@ -431,7 +409,7 @@ public class KeyManagerImpl implements KeyManager { String volumeName = args.getVolumeName(); String bucketName = args.getBucketName(); String keyName = args.getKeyName(); - validateBucket(volumeName, bucketName); + OMFileRequest.validateBucket(metadataManager, volumeName, bucketName); long currentTime = UniqueId.next(); OmKeyInfo keyInfo; @@ -615,7 +593,7 @@ public class KeyManagerImpl implements KeyManager { try { metadataManager.getLock().acquireWriteLock(BUCKET_LOCK, volumeName, bucketName); - validateBucket(volumeName, bucketName); + OMFileRequest.validateBucket(metadataManager, volumeName, bucketName); OmKeyInfo keyInfo = metadataManager.getOpenKeyTable().get(openKey); if (keyInfo == null) { throw new OMException("Failed to commit key, as " + openKey + "entry " + @@ -1577,7 +1555,7 @@ public class KeyManagerImpl implements KeyManager { metadataManager.getLock().acquireWriteLock(BUCKET_LOCK, volume, bucket); try { - validateBucket(volume, bucket); + OMFileRequest.validateBucket(metadataManager, volume, bucket); String objectKey = metadataManager.getOzoneKey(volume, bucket, keyName); OmKeyInfo keyInfo = metadataManager.getKeyTable().get(objectKey); if (keyInfo == null) { @@ -1621,7 +1599,7 @@ public class KeyManagerImpl implements KeyManager { metadataManager.getLock().acquireWriteLock(BUCKET_LOCK, volume, bucket); try { - validateBucket(volume, bucket); + OMFileRequest.validateBucket(metadataManager, volume, bucket); String objectKey = metadataManager.getOzoneKey(volume, bucket, keyName); OmKeyInfo keyInfo = metadataManager.getKeyTable().get(objectKey); if (keyInfo == null) { @@ -1662,7 +1640,7 @@ public class KeyManagerImpl implements KeyManager { metadataManager.getLock().acquireWriteLock(BUCKET_LOCK, volume, bucket); try { - validateBucket(volume, bucket); + OMFileRequest.validateBucket(metadataManager, volume, bucket); String objectKey = metadataManager.getOzoneKey(volume, bucket, keyName); OmKeyInfo keyInfo = metadataManager.getKeyTable().get(objectKey); if (keyInfo == null) { @@ -1701,7 +1679,7 @@ public class KeyManagerImpl implements KeyManager { OmKeyInfo keyInfo; metadataManager.getLock().acquireReadLock(BUCKET_LOCK, volume, bucket); try { - validateBucket(volume, bucket); + OMFileRequest.validateBucket(metadataManager, volume, bucket); String objectKey = metadataManager.getOzoneKey(volume, bucket, keyName); if (OzoneManagerRatisUtils.isBucketFSOptimized()) { keyInfo = getOmKeyInfoV1(volume, bucket, keyName); @@ -1750,7 +1728,7 @@ public class KeyManagerImpl implements KeyManager { metadataManager.getLock().acquireReadLock(BUCKET_LOCK, volume, bucket); try { - validateBucket(volume, bucket); + OMFileRequest.validateBucket(metadataManager, volume, bucket); OmKeyInfo keyInfo; // For Acl Type "WRITE", the key can only be found in @@ -1885,7 +1863,7 @@ public class KeyManagerImpl implements KeyManager { try { // Check if this is the root of the filesystem. if (keyName.length() == 0) { - validateBucket(volumeName, bucketName); + OMFileRequest.validateBucket(metadataManager, volumeName, bucketName); return new OzoneFileStatus(); } @@ -1942,7 +1920,7 @@ public class KeyManagerImpl implements KeyManager { try { // Check if this is the root of the filesystem. if (keyName.length() == 0) { - validateBucket(volumeName, bucketName); + OMFileRequest.validateBucket(metadataManager, volumeName, bucketName); return new OzoneFileStatus(); } 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 2b4484e..d50ff3d 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 @@ -1771,6 +1771,21 @@ public final class OzoneManager extends ServiceRuntimeInfoImpl .setAclRights(aclType) .setOwnerName(volumeOwner) .build(); + + return checkAcls(obj, context, throwIfPermissionDenied); + } + + /** + * CheckAcls for the ozone object. + * + * @return true if permission granted, false if permission denied. + * @throws OMException ResultCodes.PERMISSION_DENIED if permission denied + * and throwOnPermissionDenied set to true. + */ + public boolean checkAcls(OzoneObj obj, RequestContext context, + boolean throwIfPermissionDenied) + throws OMException { + if (!accessAuthorizer.checkAccess(obj, context)) { if (throwIfPermissionDenied) { LOG.warn("User {} doesn't have {} permission to access {} /{}/{}/{}", @@ -1790,6 +1805,8 @@ public final class OzoneManager extends ServiceRuntimeInfoImpl } } + + /** * Return true if Ozone acl's are enabled, else false. * diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzonePrefixPathImpl.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzonePrefixPathImpl.java new file mode 100644 index 0000000..c30d5dd --- /dev/null +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzonePrefixPathImpl.java @@ -0,0 +1,161 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with this + * work for additional information regarding copyright ownership. The ASF + * licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package org.apache.hadoop.ozone.om; + +import org.apache.commons.lang3.StringUtils; +import org.apache.hadoop.ozone.om.exceptions.OMException; +import org.apache.hadoop.ozone.om.helpers.OmKeyArgs; +import org.apache.hadoop.ozone.om.helpers.OzoneFileStatus; +import org.apache.hadoop.ozone.security.acl.OzonePrefixPath; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.io.IOException; +import java.util.Iterator; +import java.util.List; +import java.util.NoSuchElementException; + +import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.KEY_NOT_FOUND; + +public class OzonePrefixPathImpl implements OzonePrefixPath { + private static final Logger LOG = + LoggerFactory.getLogger(OzonePrefixPathImpl.class); + private String volumeName; + private String bucketName; + private KeyManager keyManager; + // TODO: based on need can make batchSize configurable. + private int batchSize = 1000; + private OzoneFileStatus pathStatus; + + public OzonePrefixPathImpl(String volumeName, String bucketName, + String keyPrefix, KeyManager keyManagerImpl) throws IOException { + this.volumeName = volumeName; + this.bucketName = bucketName; + this.keyManager = keyManagerImpl; + + OmKeyArgs omKeyArgs = new OmKeyArgs.Builder() + .setVolumeName(volumeName) + .setBucketName(bucketName) + .setKeyName(keyPrefix) + .setRefreshPipeline(false) + .build(); + try { + pathStatus = keyManager.getFileStatus(omKeyArgs); + } catch (OMException ome) { + // In existing code non-FSO code, ozone client delete and rename expects + // KNF error code. So converting FNF to KEY_NOT_FOUND error code. + if (ome.getResult() == OMException.ResultCodes.FILE_NOT_FOUND) { + throw new OMException(ome.getMessage(), KEY_NOT_FOUND); + } + throw ome; + } + } + + @Override + public OzoneFileStatus getOzoneFileStatus() { + return pathStatus; + } + + @Override + public Iterator<? extends OzoneFileStatus> getChildren(String keyPrefix) + throws IOException { + + return new PathIterator(keyPrefix); + } + + class PathIterator implements Iterator<OzoneFileStatus> { + private Iterator<OzoneFileStatus> currentIterator; + private String keyPrefix; + private OzoneFileStatus currentValue; + + /** + * Creates an Iterator to iterate over all sub paths of the given keyPrefix. + * + * @param keyPrefix + */ + PathIterator(String keyPrefix) throws IOException { + this.keyPrefix = keyPrefix; + this.currentValue = null; + List<OzoneFileStatus> statuses = getNextListOfKeys(""); + if (statuses.size() == 1) { + OzoneFileStatus keyStatus = statuses.get(0); + if (keyStatus.isFile() && StringUtils.equals(keyPrefix, + keyStatus.getTrimmedName())) { + throw new OMException("Invalid keyPrefix: " + keyPrefix + + ", file type is not allowed, expected directory type.", + OMException.ResultCodes.INVALID_KEY_NAME); + } + } + this.currentIterator = statuses.iterator(); + } + + @Override + public boolean hasNext() { + if (!currentIterator.hasNext() && currentValue != null) { + String keyName = ""; + try { + keyName = currentValue.getTrimmedName(); + currentIterator = + getNextListOfKeys(keyName).iterator(); + } catch (IOException e) { + if (LOG.isDebugEnabled()) { + LOG.debug("Exception while listing keys, keyName:" + keyName, e); + } + return false; + } + } + return currentIterator.hasNext(); + } + + @Override + public OzoneFileStatus next() { + if (hasNext()) { + currentValue = currentIterator.next(); + return currentValue; + } + throw new NoSuchElementException(); + } + + /** + * Gets the next set of key list using keyManager OM interface. + * + * @param prevKey + * @return {@code List<OzoneFileStatus>} + */ + List<OzoneFileStatus> getNextListOfKeys(String prevKey) throws + IOException { + + OmKeyArgs omKeyArgs = new OmKeyArgs.Builder() + .setVolumeName(volumeName) + .setBucketName(bucketName) + .setKeyName(keyPrefix) + .setRefreshPipeline(false) + .build(); + + List<OzoneFileStatus> statuses = keyManager.listStatus(omKeyArgs, false, + prevKey, batchSize); + + // ListStatuses with non-null startKey will add startKey as first element + // in the resultList. Remove startKey element as it is duplicated one. + if (!statuses.isEmpty() && StringUtils.equals(prevKey, + statuses.get(0).getTrimmedName())) { + statuses.remove(0); + } + return statuses; + } + } +} diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java index 828c9e9..56fff9f 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/OMClientRequest.java @@ -29,6 +29,7 @@ import org.apache.hadoop.ozone.audit.AuditEventStatus; import org.apache.hadoop.ozone.audit.AuditLogger; import org.apache.hadoop.ozone.audit.AuditMessage; import org.apache.hadoop.ozone.om.OzoneManager; +import org.apache.hadoop.ozone.om.OzonePrefixPathImpl; import org.apache.hadoop.ozone.om.exceptions.OMException; import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper; import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerRatisUtils; @@ -38,6 +39,8 @@ import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMReque import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse; import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer; import org.apache.hadoop.ozone.security.acl.OzoneObj; +import org.apache.hadoop.ozone.security.acl.OzoneObjInfo; +import org.apache.hadoop.ozone.security.acl.RequestContext; import org.apache.hadoop.security.UserGroupInformation; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -187,6 +190,51 @@ public abstract class OMClientRequest implements RequestAuditor { } /** + * Check Acls for the ozone key. + * @param ozoneManager + * @param volumeName + * @param bucketName + * @param keyName + * @throws IOException + */ + protected void checkACLs(OzoneManager ozoneManager, String volumeName, + String bucketName, String keyName, IAccessAuthorizer.ACLType aclType) + throws IOException { + + // TODO: Presently not populating sub-paths under a single bucket + // lock. Need to revisit this to handle any concurrent operations + // along with this. + OzonePrefixPathImpl pathViewer = new OzonePrefixPathImpl(volumeName, + bucketName, keyName, ozoneManager.getKeyManager()); + + OzoneObj obj = OzoneObjInfo.Builder.newBuilder() + .setResType(OzoneObj.ResourceType.KEY) + .setStoreType(OzoneObj.StoreType.OZONE) + .setVolumeName(volumeName) + .setBucketName(bucketName) + .setKeyName(keyName) + .setOzonePrefixPath(pathViewer).build(); + + boolean isDirectory = pathViewer.getOzoneFileStatus().isDirectory(); + + RequestContext.Builder contextBuilder = RequestContext.newBuilder() + .setAclRights(aclType) + .setRecursiveAccessCheck(isDirectory); // recursive checks for a dir + + // check Acl + if (ozoneManager.getAclsEnabled()) { + String volumeOwner = ozoneManager.getVolumeOwner(obj.getVolumeName(), + contextBuilder.getAclRights(), obj.getResourceType()); + contextBuilder.setClientUgi(createUGI()); + contextBuilder.setIp(getRemoteAddress()); + contextBuilder.setHost(getHostName()); + contextBuilder.setAclType(IAccessAuthorizer.ACLIdentityType.USER); + contextBuilder.setOwnerName(volumeOwner); + ozoneManager.checkAcls(obj, contextBuilder.build(), true); + } + } + + /** * Check Acls of ozone object with volOwner given. * @param ozoneManager * @param resType diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileRequest.java index e42bc6b..27eda71 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileRequest.java @@ -52,8 +52,10 @@ import org.slf4j.LoggerFactory; import javax.annotation.Nonnull; +import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.BUCKET_NOT_FOUND; import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.KEY_NOT_FOUND; import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.NOT_A_FILE; +import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.VOLUME_NOT_FOUND; /** * Base class for file requests. @@ -607,6 +609,8 @@ public final class OMFileRequest { OMMetadataManager omMetadataMgr, String volumeName, String bucketName, String keyName, long scmBlockSize) throws IOException { + OMFileRequest.validateBucket(omMetadataMgr, volumeName, bucketName); + Path keyPath = Paths.get(keyName); Iterator<Path> elements = keyPath.iterator(); String bucketKey = omMetadataMgr.getBucketKey(volumeName, bucketName); @@ -932,4 +936,36 @@ public final class OMFileRequest { return lastKnownParentId; } + + /** + * Validates volume and bucket existence. + * + * @param metadataManager + * @param volumeName + * @param bucketName + * @throws IOException + */ + public static void validateBucket(OMMetadataManager metadataManager, + String volumeName, String bucketName) + throws IOException { + + String bucketKey = metadataManager.getBucketKey(volumeName, bucketName); + // Check if bucket exists + if (metadataManager.getBucketTable().get(bucketKey) == null) { + String volumeKey = metadataManager.getVolumeKey(volumeName); + // If the volume also does not exist, we should throw volume not found + // exception + if (metadataManager.getVolumeTable().get(volumeKey) == null) { + LOG.error("volume not found: {}", volumeName); + throw new OMException("Volume not found", + VOLUME_NOT_FOUND); + } + + // if the volume exists but bucket does not exist, throw bucket not found + // exception + LOG.error("bucket not found: {}/{} ", volumeName, bucketName); + throw new OMException("Bucket not found", + BUCKET_NOT_FOUND); + } + } } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequestV1.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequestV1.java index af5c4df..dbf5645 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequestV1.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequestV1.java @@ -29,7 +29,6 @@ import org.apache.hadoop.ozone.om.OzoneManager; import org.apache.hadoop.ozone.om.exceptions.OMException; import org.apache.hadoop.ozone.om.helpers.OmBucketInfo; import org.apache.hadoop.ozone.om.helpers.OmKeyInfo; -import org.apache.hadoop.ozone.om.helpers.OmVolumeArgs; import org.apache.hadoop.ozone.om.helpers.OzoneFileStatus; import org.apache.hadoop.ozone.om.helpers.OzoneFSUtils; import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerDoubleBufferHelper; @@ -43,7 +42,6 @@ import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.DeleteK import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse; import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer; -import org.apache.hadoop.ozone.security.acl.OzoneObj; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -94,16 +92,14 @@ public class OMKeyDeleteRequestV1 extends OMKeyDeleteRequest { boolean acquiredLock = false; OMClientResponse omClientResponse = null; Result result = null; - OmVolumeArgs omVolumeArgs = null; OmBucketInfo omBucketInfo = null; try { keyArgs = resolveBucketLink(ozoneManager, keyArgs, auditMap); volumeName = keyArgs.getVolumeName(); bucketName = keyArgs.getBucketName(); - // check Acl - checkKeyAcls(ozoneManager, volumeName, bucketName, keyName, - IAccessAuthorizer.ACLType.DELETE, OzoneObj.ResourceType.KEY); + checkACLs(ozoneManager, volumeName, bucketName, keyName, + IAccessAuthorizer.ACLType.DELETE); 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/OMKeyRenameRequestV1.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRenameRequestV1.java index 56dcd6b..4ab9d3a 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRenameRequestV1.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRenameRequestV1.java @@ -38,7 +38,14 @@ import org.apache.hadoop.ozone.om.request.file.OMFileRequest; import org.apache.hadoop.ozone.om.request.util.OmResponseUtil; import org.apache.hadoop.ozone.om.response.OMClientResponse; import org.apache.hadoop.ozone.om.response.key.OMKeyRenameResponseV1; -import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.*; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos + .KeyArgs; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos + .RenameKeyRequest; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos + .RenameKeyResponse; import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer; import org.apache.hadoop.ozone.security.acl.OzoneObj; import org.slf4j.Logger; @@ -102,8 +109,12 @@ public class OMKeyRenameRequestV1 extends OMKeyRenameRequest { // 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); + + // check Acl fromKeyName + checkACLs(ozoneManager, volumeName, bucketName, fromKeyName, + IAccessAuthorizer.ACLType.DELETE); + + // check Acl toKeyName checkKeyAcls(ozoneManager, volumeName, bucketName, toKeyName, IAccessAuthorizer.ACLType.CREATE, OzoneObj.ResourceType.KEY); diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyDeleteRequestV1.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyDeleteRequestV1.java index 2c43d51..7d58d08 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyDeleteRequestV1.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyDeleteRequestV1.java @@ -20,11 +20,21 @@ package org.apache.hadoop.ozone.om.request.key; import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.hdds.protocol.proto.HddsProtos; +import org.apache.hadoop.ozone.om.OzonePrefixPathImpl; +import org.apache.hadoop.ozone.om.exceptions.OMException; import org.apache.hadoop.ozone.om.helpers.OmKeyInfo; +import org.apache.hadoop.ozone.om.helpers.OzoneFileStatus; import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerRatisUtils; import org.apache.hadoop.ozone.om.request.TestOMRequestUtils; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest; +import org.apache.hadoop.ozone.security.acl.OzonePrefixPath; import org.apache.hadoop.util.Time; +import org.junit.Assert; +import org.junit.Test; + +import java.io.IOException; +import java.util.Iterator; +import java.util.NoSuchElementException; /** * Tests OmKeyDelete request layout version V1. @@ -52,6 +62,7 @@ public class TestOMKeyDeleteRequestV1 extends TestOMKeyDeleteRequest { HddsProtos.ReplicationFactor.ONE, parentId + 1, parentId, 100, Time.now()); + omKeyInfo.setKeyName(fileName); TestOMRequestUtils.addFileToKeyTable(false, false, fileName, omKeyInfo, -1, 50, omMetadataManager); return omKeyInfo.getPath(); @@ -66,4 +77,61 @@ public class TestOMKeyDeleteRequestV1 extends TestOMKeyDeleteRequest { OzoneManagerRatisUtils.setBucketFSOptimized(true); return config; } + + @Test + public void testOzonePrefixPathViewer() throws Exception { + // Add volume, bucket and key entries to OM DB. + TestOMRequestUtils.addVolumeAndBucketToDB(volumeName, bucketName, + omMetadataManager); + + String ozoneKey = addKeyToTable(); + + OmKeyInfo omKeyInfo = omMetadataManager.getKeyTable().get(ozoneKey); + + // As we added manually to key table. + Assert.assertNotNull(omKeyInfo); + + // OzonePrefixPathImpl on a directory + OzonePrefixPathImpl ozonePrefixPath = new OzonePrefixPathImpl(volumeName, + bucketName, "c", keyManager); + OzoneFileStatus status = ozonePrefixPath.getOzoneFileStatus(); + Assert.assertNotNull(status); + Assert.assertEquals("c", status.getTrimmedName()); + Assert.assertTrue(status.isDirectory()); + verifyPath(ozonePrefixPath, "c", "c/d"); + verifyPath(ozonePrefixPath, "c/d", "c/d/e"); + verifyPath(ozonePrefixPath, "c/d/e", "c/d/e/file1"); + + try { + ozonePrefixPath.getChildren("c/d/e/file1"); + Assert.fail("Should throw INVALID_KEY_NAME as the given path is a file."); + } catch (OMException ome) { + Assert.assertEquals(OMException.ResultCodes.INVALID_KEY_NAME, + ome.getResult()); + } + + // OzonePrefixPathImpl on a file + ozonePrefixPath = new OzonePrefixPathImpl(volumeName, + bucketName, "c/d/e/file1", keyManager); + status = ozonePrefixPath.getOzoneFileStatus(); + Assert.assertNotNull(status); + Assert.assertEquals("c/d/e/file1", status.getTrimmedName()); + Assert.assertEquals("c/d/e/file1", status.getKeyInfo().getKeyName()); + Assert.assertTrue(status.isFile()); + } + + private void verifyPath(OzonePrefixPath ozonePrefixPath, String pathName, + String expectedPath) + throws IOException { + Iterator<? extends OzoneFileStatus> pathItr = ozonePrefixPath.getChildren( + pathName); + Assert.assertTrue("Failed to list keyPaths", pathItr.hasNext()); + Assert.assertEquals(expectedPath, pathItr.next().getTrimmedName()); + try{ + pathItr.next(); + Assert.fail("Reached end of the list!"); + } catch (NoSuchElementException nse){ + // expected + } + } } diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/acl/TestOzoneObj.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/acl/TestOzoneObj.java new file mode 100644 index 0000000..ab4e60f --- /dev/null +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/acl/TestOzoneObj.java @@ -0,0 +1,77 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with this + * work for additional information regarding copyright ownership. The ASF + * licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package org.apache.hadoop.ozone.security.acl; + +import org.apache.hadoop.ozone.om.KeyManager; +import org.apache.hadoop.ozone.om.OzonePrefixPathImpl; +import org.junit.Test; + +import java.io.IOException; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.mockito.Mockito.mock; + +public class TestOzoneObj { + + + private OzoneObjInfo objInfo; + private OzoneObjInfo.Builder builder; + private String volume = "vol1"; + private String bucket = "bucket1"; + private String key = "key1"; + private static final OzoneObj.StoreType STORE = OzoneObj.StoreType.OZONE; + + @Test + public void testGetPathViewer() throws IOException { + + builder = getBuilder(volume, bucket, key); + objInfo = builder.build(); + assertEquals(objInfo.getVolumeName(), volume); + assertNotNull("unexpected path accessor", + objInfo.getOzonePrefixPathViewer()); + + objInfo = getBuilder(null, null, null).build(); + assertEquals(objInfo.getVolumeName(), null); + assertNotNull("unexpected path accessor", + objInfo.getOzonePrefixPathViewer()); + + objInfo = getBuilder(volume, null, null).build(); + assertEquals(objInfo.getVolumeName(), volume); + assertNotNull("unexpected path accessor", + objInfo.getOzonePrefixPathViewer()); + + } + + private OzoneObjInfo.Builder getBuilder(String withVolume, + String withBucket, String withKey) throws IOException { + + KeyManager mockKeyManager = mock(KeyManager.class); + OzonePrefixPath prefixPathViewer = new OzonePrefixPathImpl("vol1", + "buck1", "file", mockKeyManager); + + return OzoneObjInfo.Builder.newBuilder() + .setResType(OzoneObj.ResourceType.VOLUME) + .setStoreType(STORE) + .setVolumeName(withVolume) + .setBucketName(withBucket) + .setKeyName(withKey) + .setOzonePrefixPath(prefixPathViewer); + } + +} + diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/acl/TestRequestContext.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/acl/TestRequestContext.java index b8b0363..5e76e09 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/acl/TestRequestContext.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/acl/TestRequestContext.java @@ -20,13 +20,15 @@ import org.apache.hadoop.security.UserGroupInformation; import org.junit.Assert; import org.junit.Test; +import java.io.IOException; + /** * Test request context. */ public class TestRequestContext { @Test - public void testRecursiveAccessFlag() { + public void testRecursiveAccessFlag() throws IOException { RequestContext context = getUserRequestContext("om", IAccessAuthorizer.ACLType.CREATE, false, "volume1", true); @@ -78,7 +80,8 @@ public class TestRequestContext { private RequestContext getUserRequestContext(String username, IAccessAuthorizer.ACLType type, boolean isOwner, String ownerName, - boolean recursiveAccessCheck) { + boolean recursiveAccessCheck) throws IOException { + return RequestContext.getBuilder( UserGroupInformation.createRemoteUser(username), null, null, type, ownerName, recursiveAccessCheck).build(); --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
