diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/protocol/ClientProtocol.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/protocol/ClientProtocol.java index 863a109b843b..cc9335506d70 100644 --- a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/protocol/ClientProtocol.java +++ b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/protocol/ClientProtocol.java @@ -125,6 +125,7 @@ OzoneVolume getVolumeDetails(String volumeName) * This is possible for owners of the volume and admin users * @throws IOException */ + @Deprecated boolean checkVolumeAccess(String volumeName, OzoneAcl acl) throws IOException; @@ -228,6 +229,7 @@ void deleteBucket(String volumeName, String bucketName) * @param bucketName Name of the Bucket * @throws IOException */ + @Deprecated void checkBucketAccess(String volumeName, String bucketName) throws IOException; diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java index ed85a32adb13..e74d5b406326 100644 --- a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java +++ b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java @@ -111,6 +111,7 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; + import static org.apache.hadoop.ozone.OzoneAcl.AclScope.ACCESS; import static org.apache.hadoop.ozone.OzoneConsts.OLD_QUOTA_DEFAULT; @@ -247,7 +248,7 @@ public void createVolume(String volumeName, VolumeArgs volArgs) List listOfAcls = new ArrayList<>(); //User ACL listOfAcls.add(new OzoneAcl(ACLIdentityType.USER, - owner, userRights, ACCESS)); + owner, userRights, ACCESS)); //Group ACLs of the User List userGroups = Arrays.asList(UserGroupInformation .createRemoteUser(owner).getGroupNames()); @@ -270,7 +271,7 @@ public void createVolume(String volumeName, VolumeArgs volArgs) //Remove duplicates and add ACLs for (OzoneAcl ozoneAcl : listOfAcls.stream().distinct().collect(Collectors.toList())) { - builder.addOzoneAcls(OzoneAcl.toProtobuf(ozoneAcl)); + builder.addOzoneAcls(ozoneAcl); } if (volArgs.getQuotaInBytes() == 0) { @@ -324,8 +325,7 @@ public OzoneVolume getVolumeDetails(String volumeName) volume.getUsedNamespace(), volume.getCreationTime(), volume.getModificationTime(), - volume.getAclMap().ozoneAclGetProtobuf().stream(). - map(OzoneAcl::fromProtobuf).collect(Collectors.toList()), + volume.getAcls(), volume.getMetadata()); } @@ -359,8 +359,7 @@ public List listVolumes(String volumePrefix, String prevVolume, volume.getUsedNamespace(), volume.getCreationTime(), volume.getModificationTime(), - volume.getAclMap().ozoneAclGetProtobuf().stream(). - map(OzoneAcl::fromProtobuf).collect(Collectors.toList()))) + volume.getAcls())) .collect(Collectors.toList()); } @@ -382,8 +381,7 @@ public List listVolumes(String user, String volumePrefix, volume.getUsedNamespace(), volume.getCreationTime(), volume.getModificationTime(), - volume.getAclMap().ozoneAclGetProtobuf().stream(). - map(OzoneAcl::fromProtobuf).collect(Collectors.toList()), + volume.getAcls(), volume.getMetadata())) .collect(Collectors.toList()); } diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketInfo.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketInfo.java index ca4bdb0ffef5..895d2b0d25ea 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketInfo.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketInfo.java @@ -397,6 +397,10 @@ public Builder setAcls(List listOfAcls) { return this; } + public List getAcls() { + return acls; + } + public Builder addAcl(OzoneAcl ozoneAcl) { if (ozoneAcl != null) { this.acls.add(ozoneAcl); diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmOzoneAclMap.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmOzoneAclMap.java deleted file mode 100644 index d9fe23aeac4e..000000000000 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmOzoneAclMap.java +++ /dev/null @@ -1,357 +0,0 @@ -/* - * 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.helpers; - -import static org.apache.hadoop.ozone.OzoneAcl.ZERO_BITSET; -import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.INVALID_REQUEST; -import static org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLType.ALL; -import static org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLType.NONE; - -import com.google.protobuf.ByteString; -import java.util.ArrayList; -import java.util.BitSet; -import java.util.Collection; -import java.util.HashMap; -import java.util.LinkedList; -import java.util.List; -import java.util.Map; -import java.util.Objects; -import java.util.stream.Collectors; -import org.apache.hadoop.ozone.OzoneAcl; -import org.apache.hadoop.ozone.om.exceptions.OMException; -import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OzoneAclInfo; -import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OzoneAclInfo.OzoneAclScope; -import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OzoneAclInfo.OzoneAclType; -import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLIdentityType; -import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLType; -import org.apache.hadoop.security.UserGroupInformation; - -/** - * This helper class keeps a map of all user and their permissions. - */ -@SuppressWarnings("ProtocolBufferOrdinal") -public class OmOzoneAclMap { - // per Acl Type user:rights map - private ArrayList> accessAclMap; - private List defaultAclList; - - OmOzoneAclMap() { - accessAclMap = new ArrayList<>(); - defaultAclList = new ArrayList<>(); - for (OzoneAclType aclType : OzoneAclType.values()) { - accessAclMap.add(aclType.ordinal(), new HashMap<>()); - } - } - - OmOzoneAclMap(List defaultAclList, - ArrayList> accessAclMap) { - this.defaultAclList = defaultAclList; - this.accessAclMap = accessAclMap; - } - - private Map getAccessAclMap(OzoneAclType type) { - return accessAclMap.get(type.ordinal()); - } - - // For a given acl type and user, get the stored acl - private BitSet getAcl(OzoneAclType type, String user) { - return getAccessAclMap(type).get(user); - } - - public List getAcl() { - List acls = new ArrayList<>(); - - acls.addAll(getAccessAcls()); - acls.addAll(defaultAclList.stream().map(a -> - OzoneAcl.fromProtobuf(a)).collect(Collectors.toList())); - return acls; - } - - private Collection getAccessAcls() { - List acls = new ArrayList<>(); - for (OzoneAclType type : OzoneAclType.values()) { - accessAclMap.get(type.ordinal()).entrySet().stream(). - forEach(entry -> acls.add(new OzoneAcl(ACLIdentityType. - valueOf(type.name()), entry.getKey(), entry.getValue(), - OzoneAcl.AclScope.ACCESS))); - } - return acls; - } - - // Add a new acl to the map - public void addAcl(OzoneAcl acl) throws OMException { - Objects.requireNonNull(acl, "Acl should not be null."); - OzoneAclType aclType = OzoneAclType.valueOf(acl.getType().name()); - if (acl.getAclScope().equals(OzoneAcl.AclScope.DEFAULT)) { - addDefaultAcl(acl); - return; - } - - if (!getAccessAclMap(aclType).containsKey(acl.getName())) { - getAccessAclMap(aclType).put(acl.getName(), acl.getAclBitSet()); - } else { - BitSet curBitSet = getAccessAclMap(aclType).get(acl.getName()); - BitSet bitSet = checkAndGet(acl, curBitSet); - getAccessAclMap(aclType).replace(acl.getName(), bitSet); - } - } - - private void addDefaultAcl(OzoneAcl acl) throws OMException { - OzoneAclInfo ozoneAclInfo = OzoneAcl.toProtobuf(acl); - if (defaultAclList.contains(ozoneAclInfo)) { - aclExistsError(acl); - } else { - for (int i = 0; i < defaultAclList.size(); i++) { - OzoneAclInfo old = defaultAclList.get(i); - if (old.getType() == ozoneAclInfo.getType() && old.getName().equals( - ozoneAclInfo.getName())) { - BitSet curBitSet = BitSet.valueOf(old.getRights().toByteArray()); - BitSet bitSet = checkAndGet(acl, curBitSet); - ozoneAclInfo = OzoneAclInfo.newBuilder(ozoneAclInfo).setRights( - ByteString.copyFrom(bitSet.toByteArray())).build(); - defaultAclList.remove(i); - defaultAclList.add(ozoneAclInfo); - return; - } - } - } - defaultAclList.add(ozoneAclInfo); - } - - private void aclExistsError(OzoneAcl acl) throws OMException { - // throw exception if acl is already added. - throw new OMException("Acl " + acl + " already exist.", INVALID_REQUEST); - } - - private BitSet checkAndGet(OzoneAcl acl, BitSet curBitSet) - throws OMException { - // Check if we are adding new rights to existing acl. - BitSet temp = (BitSet) acl.getAclBitSet().clone(); - BitSet curRights = (BitSet) curBitSet.clone(); - temp.or(curRights); - if (temp.equals(curRights)) { - aclExistsError(acl); - } - return temp; - } - - // Add a new acl to the map - public void setAcls(List acls) throws OMException { - Objects.requireNonNull(acls, "Acls should not be null."); - // Remove all Acls. - for (OzoneAclType type : OzoneAclType.values()) { - accessAclMap.get(type.ordinal()).clear(); - } - // Add acls. - for (OzoneAcl acl : acls) { - addAcl(acl); - } - } - - // Add a new acl to the map - public void removeAcl(OzoneAcl acl) throws OMException { - Objects.requireNonNull(acl, "Acl should not be null."); - if (acl.getAclScope().equals(OzoneAcl.AclScope.DEFAULT)) { - defaultAclList.remove(OzoneAcl.toProtobuf(acl)); - return; - } - - OzoneAclType aclType = OzoneAclType.valueOf(acl.getType().name()); - if (getAccessAclMap(aclType).containsKey(acl.getName())) { - BitSet aclRights = getAccessAclMap(aclType).get(acl.getName()); - BitSet bits = (BitSet) acl.getAclBitSet().clone(); - bits.and(aclRights); - - if (bits.equals(ZERO_BITSET)) { - // throw exception if acl doesn't exist. - throw new OMException("Acl [" + acl + "] doesn't exist.", - INVALID_REQUEST); - } - - acl.getAclBitSet().and(aclRights); - aclRights.xor(acl.getAclBitSet()); - - // Remove the acl as all rights are already set to 0. - if (aclRights.equals(ZERO_BITSET)) { - getAccessAclMap(aclType).remove(acl.getName()); - } - } else { - // throw exception if acl doesn't exist. - throw new OMException("Acl [" + acl + "] doesn't exist.", - INVALID_REQUEST); - } - } - - // Add a new acl to the map - public void addAcl(OzoneAclInfo acl) throws OMException { - Objects.requireNonNull(acl, "Acl should not be null."); - if (acl.getAclScope().equals(OzoneAclInfo.OzoneAclScope.DEFAULT)) { - addDefaultAcl(OzoneAcl.fromProtobuf(acl)); - return; - } - - if (!getAccessAclMap(acl.getType()).containsKey(acl.getName())) { - BitSet acls = BitSet.valueOf(acl.getRights().toByteArray()); - getAccessAclMap(acl.getType()).put(acl.getName(), acls); - } else { - aclExistsError(OzoneAcl.fromProtobuf(acl)); - } - } - - // for a given acl, check if the user has access rights - public boolean hasAccess(OzoneAclInfo acl) { - if (acl == null) { - return false; - } - - BitSet aclBitSet = getAcl(acl.getType(), acl.getName()); - if (aclBitSet == null) { - return false; - } - BitSet result = BitSet.valueOf(acl.getRights().toByteArray()); - result.and(aclBitSet); - return (!result.equals(ZERO_BITSET) || aclBitSet.get(ALL.ordinal())) - && !aclBitSet.get(NONE.ordinal()); - } - - /** - * For a given acl, check if the user has access rights. - * Acl's are checked in followoing order: - * 1. Acls for USER. - * 2. Acls for GROUPS. - * 3. Acls for WORLD. - * 4. Acls for ANONYMOUS. - * @param acl - * @param ugi - * - * @return true if given ugi has acl set, else false. - * */ - public boolean hasAccess(ACLType acl, UserGroupInformation ugi) { - if (acl == null) { - return false; - } - if (ugi == null) { - return false; - } - - // Check acls in user acl list. - return checkAccessForOzoneAclType(OzoneAclType.USER, acl, ugi) - || checkAccessForOzoneAclType(OzoneAclType.GROUP, acl, ugi) - || checkAccessForOzoneAclType(OzoneAclType.WORLD, acl, ugi) - || checkAccessForOzoneAclType(OzoneAclType.ANONYMOUS, acl, ugi); - } - - /** - * Helper function to check acl access for OzoneAclType. - * */ - private boolean checkAccessForOzoneAclType(OzoneAclType identityType, - ACLType acl, UserGroupInformation ugi) { - - switch (identityType) { - case USER: - return OzoneAclUtil.checkIfAclBitIsSet(acl, getAcl(identityType, - ugi.getUserName())); - case GROUP: - // Check access for user groups. - for (String userGroup : ugi.getGroupNames()) { - if (OzoneAclUtil.checkIfAclBitIsSet(acl, getAcl(identityType, - userGroup))) { - // Return true if any user group has required permission. - return true; - } - } - break; - default: - // For type WORLD and ANONYMOUS we set acl type as name. - if(OzoneAclUtil.checkIfAclBitIsSet(acl, getAcl(identityType, - identityType.name()))) { - return true; - } - - } - return false; - } - - // Convert this map to OzoneAclInfo Protobuf List - public List ozoneAclGetProtobuf() { - List aclList = new LinkedList<>(); - for (OzoneAclType type : OzoneAclType.values()) { - for (Map.Entry entry : - accessAclMap.get(type.ordinal()).entrySet()) { - OzoneAclInfo.Builder builder = OzoneAclInfo.newBuilder() - .setName(entry.getKey()) - .setType(type) - .setAclScope(OzoneAclScope.ACCESS) - .setRights(ByteString.copyFrom(entry.getValue().toByteArray())); - - aclList.add(builder.build()); - } - } - aclList.addAll(defaultAclList); - return aclList; - } - - // Create map from list of OzoneAclInfos - public static OmOzoneAclMap ozoneAclGetFromProtobuf( - List aclList) throws OMException { - OmOzoneAclMap aclMap = new OmOzoneAclMap(); - for (OzoneAclInfo acl : aclList) { - aclMap.addAcl(acl); - } - return aclMap; - } - - public Collection getAclsByScope(OzoneAclScope scope) { - if (scope.equals(OzoneAclScope.DEFAULT)) { - return defaultAclList.stream().map(a -> - OzoneAcl.fromProtobuf(a)).collect(Collectors.toList()); - } else { - return getAcl(); - } - } - - public List getDefaultAclList() { - return defaultAclList; - } - - /** - * Return a new copy of the object. - */ - public OmOzoneAclMap copyObject() { - ArrayList> accessMap = new ArrayList<>(); - - // Initialize. - for (OzoneAclType aclType : OzoneAclType.values()) { - accessMap.add(aclType.ordinal(), new HashMap<>()); - } - - // Add from original accessAclMap to accessMap. - for (OzoneAclType aclType : OzoneAclType.values()) { - final int ordinal = aclType.ordinal(); - accessAclMap.get(ordinal).forEach((k, v) -> - accessMap.get(ordinal).put(k, (BitSet) v.clone())); - } - - // We can do shallow copy here, as OzoneAclInfo is immutable structure. - ArrayList defaultList = new ArrayList<>(); - defaultList.addAll(defaultAclList); - - return new OmOzoneAclMap(defaultList, accessMap); - } -} diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmVolumeArgs.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmVolumeArgs.java index 559eebcc7d72..85165d6e5b5e 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmVolumeArgs.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmVolumeArgs.java @@ -18,6 +18,8 @@ package org.apache.hadoop.ozone.om.helpers; import java.io.IOException; +import java.util.ArrayList; +import java.util.BitSet; import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; @@ -27,7 +29,6 @@ import org.apache.hadoop.ozone.OzoneAcl; import org.apache.hadoop.ozone.OzoneConsts; import org.apache.hadoop.ozone.audit.Auditable; -import org.apache.hadoop.ozone.om.exceptions.OMException; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OzoneAclInfo; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.VolumeInfo; @@ -46,7 +47,7 @@ public final class OmVolumeArgs extends WithObjectID implements Auditable { private long quotaInBytes; private long quotaInNamespace; private long usedNamespace; - private final OmOzoneAclMap aclMap; + private List acls; /** * Private constructor, constructed via builder. @@ -57,7 +58,7 @@ public final class OmVolumeArgs extends WithObjectID implements Auditable { * @param quotaInNamespace - Volume Quota in counts. * @param usedNamespace - Volume Namespace Quota Usage in counts. * @param metadata - metadata map for custom key/value data. - * @param aclMap - User to access rights map. + * @param acls - list of volume acls. * @param creationTime - Volume creation time. * @param objectID - ID of this object. * @param updateID - A sequence number that denotes the last update on this @@ -67,7 +68,7 @@ public final class OmVolumeArgs extends WithObjectID implements Auditable { "builder."}) private OmVolumeArgs(String adminName, String ownerName, String volume, long quotaInBytes, long quotaInNamespace, long usedNamespace, - Map metadata, OmOzoneAclMap aclMap, long creationTime, + Map metadata, List acls, long creationTime, long modificationTime, long objectID, long updateID) { this.adminName = adminName; this.ownerName = ownerName; @@ -76,7 +77,7 @@ private OmVolumeArgs(String adminName, String ownerName, String volume, this.quotaInNamespace = quotaInNamespace; this.usedNamespace = usedNamespace; this.metadata = metadata; - this.aclMap = aclMap; + this.acls = acls; this.creationTime = creationTime; this.modificationTime = modificationTime; this.objectID = objectID; @@ -104,16 +105,16 @@ public void setModificationTime(long time) { this.modificationTime = time; } - public void addAcl(OzoneAcl acl) throws OMException { - this.aclMap.addAcl(acl); + public boolean addAcl(OzoneAcl ozoneAcl) { + return OzoneAclUtil.addAcl(acls, ozoneAcl); } - public void setAcls(List acls) throws OMException { - this.aclMap.setAcls(acls); + public boolean setAcls(List ozoneAcls) { + return OzoneAclUtil.setAcl(acls, ozoneAcls); } - public void removeAcl(OzoneAcl acl) throws OMException { - this.aclMap.removeAcl(acl); + public boolean removeAcl(OzoneAcl ozoneAcl) { + return OzoneAclUtil.removeAcl(acls, ozoneAcl); } /** @@ -172,8 +173,18 @@ public long getQuotaInNamespace() { return quotaInNamespace; } - public OmOzoneAclMap getAclMap() { - return aclMap; + public List getAcls() { + return acls; + } + + public List getDefaultAcls() { + List defaultAcls = new ArrayList<>(); + for (OzoneAcl acl : acls) { + if (acl.getAclScope() == OzoneAcl.AclScope.DEFAULT) { + defaultAcls.add(acl); + } + } + return defaultAcls; } /** @@ -249,7 +260,7 @@ public static class Builder { private long quotaInNamespace; private long usedNamespace; private Map metadata; - private OmOzoneAclMap aclMap; + private List acls; private long objectID; private long updateID; @@ -279,7 +290,7 @@ public Builder setUpdateID(long id) { */ public Builder() { metadata = new HashMap<>(); - aclMap = new OmOzoneAclMap(); + acls = new ArrayList(); quotaInBytes = OzoneConsts.QUOTA_RESET; quotaInNamespace = OzoneConsts.QUOTA_RESET; } @@ -336,8 +347,8 @@ public Builder addAllMetadata(Map additionalMetaData) { return this; } - public Builder addOzoneAcls(OzoneAclInfo acl) throws IOException { - aclMap.addAcl(acl); + public Builder addOzoneAcls(OzoneAcl acl) throws IOException { + OzoneAclUtil.addAcl(acls, acl); return this; } @@ -350,14 +361,14 @@ public OmVolumeArgs build() { Preconditions.checkNotNull(ownerName); Preconditions.checkNotNull(volume); return new OmVolumeArgs(adminName, ownerName, volume, quotaInBytes, - quotaInNamespace, usedNamespace, metadata, aclMap, creationTime, + quotaInNamespace, usedNamespace, metadata, acls, creationTime, modificationTime, objectID, updateID); } } public VolumeInfo getProtobuf() { - List aclList = aclMap.ozoneAclGetProtobuf(); + List aclList = OzoneAclUtil.toProtobuf(acls); return VolumeInfo.newBuilder() .setAdminName(adminName) .setOwnerName(ownerName) @@ -375,10 +386,9 @@ public VolumeInfo getProtobuf() { .build(); } - public static OmVolumeArgs getFromProtobuf(VolumeInfo volInfo) - throws OMException { - OmOzoneAclMap aclMap = - OmOzoneAclMap.ozoneAclGetFromProtobuf(volInfo.getVolumeAclsList()); + public static OmVolumeArgs getFromProtobuf(VolumeInfo volInfo) { + List acls = OzoneAclUtil.fromProtobuf( + volInfo.getVolumeAclsList()); return new OmVolumeArgs( volInfo.getAdminName(), volInfo.getOwnerName(), @@ -387,7 +397,7 @@ public static OmVolumeArgs getFromProtobuf(VolumeInfo volInfo) volInfo.getQuotaInNamespace(), volInfo.getUsedNamespace(), KeyValueUtil.getFromProtobuf(volInfo.getMetadataList()), - aclMap, + acls, volInfo.getCreationTime(), volInfo.getModificationTime(), volInfo.getObjectID(), @@ -415,10 +425,14 @@ public OmVolumeArgs copyObject() { metadata.forEach((k, v) -> cloneMetadata.put(k, v)); } - OmOzoneAclMap cloneAclMap = aclMap.copyObject(); + List cloneAcls = new ArrayList(acls.size()); + + acls.forEach(acl -> cloneAcls.add(new OzoneAcl(acl.getType(), + acl.getName(), (BitSet) acl.getAclBitSet().clone(), + acl.getAclScope()))); return new OmVolumeArgs(adminName, ownerName, volume, quotaInBytes, - quotaInNamespace, usedNamespace, cloneMetadata, cloneAclMap, + quotaInNamespace, usedNamespace, cloneMetadata, cloneAcls, creationTime, modificationTime, objectID, updateID); } } diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OzoneAclUtil.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OzoneAclUtil.java index 5b099d83d38c..0c15d79fcf6d 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OzoneAclUtil.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OzoneAclUtil.java @@ -175,7 +175,7 @@ public static boolean inheritDefaultAcls(List acls, inheritedAcls = parentAcls.stream() .filter(a -> a.getAclScope() == DEFAULT) .map(acl -> new OzoneAcl(acl.getType(), acl.getName(), - acl.getAclBitSet(), OzoneAcl.AclScope.ACCESS)) + acl.getAclBitSet(), ACCESS)) .collect(Collectors.toList()); } if (inheritedAcls != null && !inheritedAcls.isEmpty()) { diff --git a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmOzoneAclMap.java b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmOzoneAclMap.java deleted file mode 100644 index 6d8f685d735d..000000000000 --- a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmOzoneAclMap.java +++ /dev/null @@ -1,56 +0,0 @@ -/** - * 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.helpers; - -import org.apache.hadoop.ozone.OzoneAcl; -import org.junit.Assert; -import org.junit.Test; - -/** - * Class to test {@link OmOzoneAclMap}. - */ -public class TestOmOzoneAclMap { - - @Test - public void testAddAcl() throws Exception { - OmOzoneAclMap map = new OmOzoneAclMap(); - map.addAcl(OzoneAcl.parseAcl("user:masstter:rx[DEFAULT]")); - map.addAcl(OzoneAcl.parseAcl("user:masstter:rw[DEFAULT]")); - - //[user:masstter:rwx[DEFAULT]] - Assert.assertEquals(1, map.getAcl().size()); - Assert.assertEquals(1, map.getDefaultAclList().size()); - - map = new OmOzoneAclMap(); - map.addAcl(OzoneAcl.parseAcl("user:masstter:rx")); - map.addAcl(OzoneAcl.parseAcl("user:masstter:rw[ACCESS]")); - - //[user:masstter:rwx[ACCESS]] - Assert.assertEquals(1, map.getAcl().size()); - Assert.assertEquals(0, map.getDefaultAclList().size()); - - map = new OmOzoneAclMap(); - map.addAcl(OzoneAcl.parseAcl("user:masstter:rwx[DEFAULT]")); - map.addAcl(OzoneAcl.parseAcl("user:masstter:rwx[ACCESS]")); - - //[user:masstter:rwx[ACCESS], user:masstter:rwx[DEFAULT]] - Assert.assertEquals(2, map.getAcl().size()); - Assert.assertEquals(1, map.getDefaultAclList().size()); - - } -} diff --git a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmVolumeArgs.java b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmVolumeArgs.java index c03f519d1a5d..1e3135e841c8 100644 --- a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmVolumeArgs.java +++ b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmVolumeArgs.java @@ -43,9 +43,9 @@ public void testClone() throws Exception { .setAdminName(admin).setCreationTime(Time.now()).setOwnerName(owner) .setObjectID(1L).setUpdateID(1L).setQuotaInBytes(Long.MAX_VALUE) .addMetadata("key1", "value1").addMetadata("key2", "value2") - .addOzoneAcls(OzoneAcl.toProtobuf( + .addOzoneAcls( new OzoneAcl(IAccessAuthorizer.ACLIdentityType.USER, "user1", - IAccessAuthorizer.ACLType.READ, ACCESS))).build(); + IAccessAuthorizer.ACLType.READ, ACCESS)).build(); OmVolumeArgs cloneVolumeArgs = omVolumeArgs.copyObject(); @@ -57,31 +57,31 @@ public void testClone() throws Exception { IAccessAuthorizer.ACLType.WRITE, ACCESS)); // Now check clone acl - Assert.assertNotEquals(cloneVolumeArgs.getAclMap().getAcl().get(0), - omVolumeArgs.getAclMap().getAcl().get(0)); + Assert.assertNotEquals(cloneVolumeArgs.getAcls().get(0), + omVolumeArgs.getAcls().get(0)); // Set user acl to Write_ACL. omVolumeArgs.setAcls(Collections.singletonList(new OzoneAcl( IAccessAuthorizer.ACLIdentityType.USER, "user1", IAccessAuthorizer.ACLType.WRITE_ACL, ACCESS))); - Assert.assertNotEquals(cloneVolumeArgs.getAclMap().getAcl().get(0), - omVolumeArgs.getAclMap().getAcl().get(0)); + Assert.assertNotEquals(cloneVolumeArgs.getAcls().get(0), + omVolumeArgs.getAcls().get(0)); // Now clone and check. It should have same as original acl. cloneVolumeArgs = (OmVolumeArgs) omVolumeArgs.copyObject(); Assert.assertEquals(omVolumeArgs, cloneVolumeArgs); - Assert.assertEquals(cloneVolumeArgs.getAclMap().getAcl().get(0), - omVolumeArgs.getAclMap().getAcl().get(0)); + Assert.assertEquals(cloneVolumeArgs.getAcls().get(0), + omVolumeArgs.getAcls().get(0)); omVolumeArgs.removeAcl(new OzoneAcl( IAccessAuthorizer.ACLIdentityType.USER, "user1", IAccessAuthorizer.ACLType.WRITE_ACL, ACCESS)); // Removing acl, in original omVolumeArgs it should have no acls. - Assert.assertEquals(0, omVolumeArgs.getAclMap().getAcl().size()); - Assert.assertEquals(1, cloneVolumeArgs.getAclMap().getAcl().size()); + Assert.assertEquals(0, omVolumeArgs.getAcls().size()); + Assert.assertEquals(1, cloneVolumeArgs.getAcls().size()); } } diff --git a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOzoneAclUtil.java b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOzoneAclUtil.java index 6e7c05233a74..5b1ca3355d11 100644 --- a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOzoneAclUtil.java +++ b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOzoneAclUtil.java @@ -22,6 +22,7 @@ import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLType; import org.apache.hadoop.ozone.security.acl.OzoneAclConfig; import org.apache.hadoop.security.UserGroupInformation; +import org.junit.Assert; import org.junit.Test; import java.io.IOException; @@ -32,6 +33,7 @@ import static org.apache.hadoop.hdds.conf.OzoneConfiguration.newInstanceOf; import static org.apache.hadoop.ozone.OzoneAcl.AclScope.ACCESS; +import static org.apache.hadoop.ozone.OzoneAcl.AclScope.DEFAULT; import static org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLIdentityType.GROUP; import static org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLIdentityType.USER; import static org.junit.Assert.assertEquals; @@ -188,4 +190,41 @@ private static List getDefaultAcls() { new OzoneAcl(GROUP, group, groupRights, ACCESS))); return ozoneAcls; } + + // Ported from tests added with HDDS-4606 for OmOzoneAclMap + @Test + public void testAddDefaultAcl() { + List ozoneAcls = new ArrayList<>(); + OzoneAclUtil.addAcl(ozoneAcls, + OzoneAcl.parseAcl("user:masstter:rx[DEFAULT]")); + OzoneAclUtil.addAcl(ozoneAcls, + OzoneAcl.parseAcl("user:masstter:rw[DEFAULT]")); + + //[user:masstter:rwx[DEFAULT]] + Assert.assertEquals(1, ozoneAcls.size()); + Assert.assertEquals(DEFAULT, ozoneAcls.get(0).getAclScope()); + + ozoneAcls = new ArrayList<>(); + OzoneAclUtil.addAcl(ozoneAcls, + OzoneAcl.parseAcl("user:masstter:rx")); + OzoneAclUtil.addAcl(ozoneAcls, + OzoneAcl.parseAcl("user:masstter:rw[ACCESS]")); + + //[user:masstter:rwx[ACCESS]] + Assert.assertEquals(1, ozoneAcls.size()); + Assert.assertEquals(ACCESS, ozoneAcls.get(0).getAclScope()); + + ozoneAcls = new ArrayList<>(); + OzoneAclUtil.addAcl(ozoneAcls, + OzoneAcl.parseAcl("user:masstter:rwx[DEFAULT]")); + OzoneAclUtil.addAcl(ozoneAcls, + OzoneAcl.parseAcl("user:masstter:rwx[ACCESS]")); + + //[user:masstter:rwx[ACCESS], user:masstter:rwx[DEFAULT]] + Assert.assertEquals(2, ozoneAcls.size()); + Assert.assertNotEquals(ozoneAcls.get(0).getAclScope(), + ozoneAcls.get(1).getAclScope()); + Assert.assertEquals(ozoneAcls.get(0).getAclBitSet(), + ozoneAcls.get(1).getAclBitSet()); + } } diff --git a/hadoop-ozone/dist/src/main/smoketest/basic/ozone-shell-lib.robot b/hadoop-ozone/dist/src/main/smoketest/basic/ozone-shell-lib.robot index 793b553a74b0..54a12b362ddc 100644 --- a/hadoop-ozone/dist/src/main/smoketest/basic/ozone-shell-lib.robot +++ b/hadoop-ozone/dist/src/main/smoketest/basic/ozone-shell-lib.robot @@ -97,7 +97,7 @@ Test Volume Acls ${result} = Execute ozone sh volume removeacl ${protocol}${server}/${volume} -a user:superuser1:xy ${result} = Execute ozone sh volume getacl ${protocol}${server}/${volume} Should Match Regexp ${result} \"type\" : \"USER\",\n.*\"name\" : \"superuser1\",\n.*\"aclScope\" : \"DEFAULT\",\n.*\"aclList\" : . \"READ\", \"WRITE\", \"READ_ACL\", \"WRITE_ACL\" . - ${result} = Execute ozone sh volume setacl ${protocol}${server}/${volume} -al user:superuser1:rwxy,group:superuser1:a,user:testuser/${SCM}@EXAMPLE.COM:rwxyc,group:superuser1:a[DEFAULT] + ${result} = Execute ozone sh volume setacl ${protocol}${server}/${volume} -al user:superuser1:rwxy[DEFAULT],group:superuser1:a,user:testuser/${SCM}@EXAMPLE.COM:rwxyc,group:superuser1:a[DEFAULT] ${result} = Execute ozone sh volume getacl ${protocol}${server}/${volume} Should Match Regexp ${result} \"type\" : \"USER\",\n.*\"name\" : \"superuser1*\",\n.*\"aclScope\" : \"DEFAULT\",\n.*\"aclList\" : . \"READ\", \"WRITE\", \"READ_ACL\", \"WRITE_ACL\" . Should Match Regexp ${result} \"type\" : \"GROUP\",\n.*\"name\" : \"superuser1\",\n.*\"aclScope\" : \"DEFAULT\",\n.*\"aclList\" : . \"ALL\" . diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java index 581dc2f0583c..277359cfbb3a 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java @@ -3093,7 +3093,7 @@ private void validateOzoneAccessAcl(OzoneObj ozObj) throws IOException { if(expectedAcls.size()>0) { OzoneAcl oldAcl = expectedAcls.get(0); OzoneAcl newAcl = new OzoneAcl(oldAcl.getType(), oldAcl.getName(), - ACLType.READ_ACL, ACCESS); + ACLType.READ_ACL, oldAcl.getAclScope()); // Verify that operation successful. assertTrue(store.addAcl(ozObj, newAcl)); diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerImpl.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerImpl.java index 493bf9a53b16..96a0f7c137e4 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerImpl.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerImpl.java @@ -609,7 +609,7 @@ public void testInvalidPrefixAcl() throws IOException { .setStoreType(OzoneObj.StoreType.OZONE) .build(); - + prefixManager.addAcl(ozPrefix1, ozAcl1); List ozAclGet = prefixManager.getAcl(ozPrefix1); Assert.assertEquals(1, ozAclGet.size()); Assert.assertEquals(ozAcl1, ozAclGet.get(0)); @@ -617,7 +617,7 @@ public void testInvalidPrefixAcl() throws IOException { // get acl with invalid prefix name exception.expect(OMException.class); exception.expectMessage("Invalid prefix name"); - Assert.assertEquals(null, ozAcl1); + prefixManager.getAcl(ozInvalidPrefix); // set acl with invalid prefix name List ozoneAcls = new ArrayList(); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/BucketManagerImpl.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/BucketManagerImpl.java index a93ebe61b28a..cdb18ea25e15 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/BucketManagerImpl.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/BucketManagerImpl.java @@ -33,7 +33,6 @@ import org.apache.hadoop.ozone.om.helpers.OmBucketInfo; import org.apache.hadoop.ozone.om.helpers.OmVolumeArgs; import org.apache.hadoop.ozone.om.helpers.OzoneAclUtil; -import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos; import org.apache.hadoop.ozone.security.acl.OzoneObj; import org.apache.hadoop.ozone.security.acl.RequestContext; import org.apache.hadoop.util.StringUtils; @@ -160,11 +159,8 @@ public void createBucket(OmBucketInfo bucketInfo) throws IOException { OmBucketInfo.Builder omBucketInfoBuilder = bucketInfo.toBuilder() .setCreationTime(Time.now()); - List defaultAclList = - volumeArgs.getAclMap().getDefaultAclList(); - for (OzoneManagerProtocolProtos.OzoneAclInfo a : defaultAclList) { - omBucketInfoBuilder.addAcl(OzoneAcl.fromProtobufWithAccessType(a)); - } + OzoneAclUtil.inheritDefaultAcls(omBucketInfoBuilder.getAcls(), + volumeArgs.getDefaultAcls()); if (bekb != null) { omBucketInfoBuilder.setBucketEncryptionKey(bekb.build()); 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 abc02830b063..c0461ad34a90 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 @@ -3758,7 +3758,7 @@ private OmVolumeArgs createS3VolumeInfo(String s3Volume, // Add ACLs for (OzoneAcl ozoneAcl : listOfAcls) { - omVolumeArgs.addOzoneAcls(OzoneAcl.toProtobuf(ozoneAcl)); + omVolumeArgs.addOzoneAcls(ozoneAcl); } return omVolumeArgs.build(); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/PrefixManagerImpl.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/PrefixManagerImpl.java index 5e4ab5f7e358..367bbf3ebd82 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/PrefixManagerImpl.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/PrefixManagerImpl.java @@ -247,15 +247,12 @@ public boolean checkAccess(OzoneObj ozObject, RequestContext context) context.getClientUgi(), ozObject, hasAccess); } return hasAccess; - } else { - return true; } - } else { - return true; } } finally { metadataManager.getLock().releaseReadLock(PREFIX_LOCK, prefixPath); } + return true; } @Override @@ -311,6 +308,10 @@ public void validateOzoneObj(OzoneObj obj) throws OMException { public OMPrefixAclOpResult addAcl(OzoneObj ozoneObj, OzoneAcl ozoneAcl, OmPrefixInfo prefixInfo, long transactionLogIndex) throws IOException { + // No explicit prefix create API, both add/set Acl can get new prefix + // created. When new prefix is created, it should inherit parent prefix + // or bucket default ACLs. + boolean newPrefix = false; if (prefixInfo == null) { OmPrefixInfo.Builder prefixInfoBuilder = new OmPrefixInfo.Builder() @@ -321,10 +322,14 @@ public OMPrefixAclOpResult addAcl(OzoneObj ozoneObj, OzoneAcl ozoneAcl, prefixInfoBuilder.setUpdateID(transactionLogIndex); } prefixInfo = prefixInfoBuilder.build(); + newPrefix = true; } boolean changed = prefixInfo.addAcl(ozoneAcl); if (changed) { + if (newPrefix) { + inheritParentAcl(ozoneObj, prefixInfo); + } // update the in-memory prefix tree prefixTree.insert(ozoneObj.getPath(), prefixInfo); @@ -360,6 +365,35 @@ public OMPrefixAclOpResult removeAcl(OzoneObj ozoneObj, OzoneAcl ozoneAcl, return new OMPrefixAclOpResult(prefixInfo, removed); } + private void inheritParentAcl(OzoneObj ozoneObj, OmPrefixInfo prefixInfo) + throws IOException { + List aclsToBeSet = prefixInfo.getAcls(); + // Inherit DEFAULT acls from prefix. + boolean prefixParentFound = false; + List prefixList = getLongestPrefixPathHelper( + prefixTree.getLongestPrefix(ozoneObj.getPath())); + + if (prefixList.size() > 0) { + // Add all acls from direct parent to key. + OmPrefixInfo parentPrefixInfo = prefixList.get(prefixList.size() - 1); + if (parentPrefixInfo != null) { + prefixParentFound = OzoneAclUtil.inheritDefaultAcls( + aclsToBeSet, parentPrefixInfo.getAcls()); + } + } + + // If no parent prefix is found inherit DEFAULT acls from bucket. + if (!prefixParentFound) { + String bucketKey = metadataManager.getBucketKey(ozoneObj + .getVolumeName(), ozoneObj.getBucketName()); + OmBucketInfo bucketInfo = metadataManager.getBucketTable(). + get(bucketKey); + if (bucketInfo != null) { + OzoneAclUtil.inheritDefaultAcls(aclsToBeSet, bucketInfo.getAcls()); + } + } + } + public OMPrefixAclOpResult setAcl(OzoneObj ozoneObj, List ozoneAcls, OmPrefixInfo prefixInfo, long transactionLogIndex) throws IOException { if (prefixInfo == null) { @@ -376,32 +410,7 @@ public OMPrefixAclOpResult setAcl(OzoneObj ozoneObj, List ozoneAcls, boolean changed = prefixInfo.setAcls(ozoneAcls); if (changed) { - List aclsToBeSet = prefixInfo.getAcls(); - // Inherit DEFAULT acls from prefix. - boolean prefixParentFound = false; - List prefixList = getLongestPrefixPathHelper( - prefixTree.getLongestPrefix(ozoneObj.getPath())); - - if (prefixList.size() > 0) { - // Add all acls from direct parent to key. - OmPrefixInfo parentPrefixInfo = prefixList.get(prefixList.size() - 1); - if (parentPrefixInfo != null) { - prefixParentFound = OzoneAclUtil.inheritDefaultAcls(aclsToBeSet, - parentPrefixInfo.getAcls()); - } - } - - // If no parent prefix is found inherit DEFAULT acls from bucket. - if (!prefixParentFound) { - String bucketKey = metadataManager.getBucketKey(ozoneObj - .getVolumeName(), ozoneObj.getBucketName()); - OmBucketInfo bucketInfo = metadataManager.getBucketTable(). - get(bucketKey); - if (bucketInfo != null) { - OzoneAclUtil.inheritDefaultAcls(aclsToBeSet, bucketInfo.getAcls()); - } - } - + inheritParentAcl(ozoneObj, prefixInfo); prefixTree.insert(ozoneObj.getPath(), prefixInfo); if (!isRatisEnabled) { metadataManager.getPrefixTable().put(ozoneObj.getPath(), prefixInfo); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/VolumeManagerImpl.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/VolumeManagerImpl.java index e3367b24f3fd..fe50a6cc9f36 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/VolumeManagerImpl.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/VolumeManagerImpl.java @@ -26,6 +26,7 @@ import org.apache.hadoop.ozone.OzoneConfigKeys; import org.apache.hadoop.ozone.om.exceptions.OMException; import org.apache.hadoop.ozone.om.helpers.OmVolumeArgs; +import org.apache.hadoop.ozone.om.helpers.OzoneAclUtil; import org.apache.hadoop.ozone.protocol.proto .OzoneManagerProtocolProtos.OzoneAclInfo; import org.apache.hadoop.ozone.security.acl.OzoneObj; @@ -36,6 +37,7 @@ import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_USER_MAX_VOLUME; import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_USER_MAX_VOLUME_DEFAULT; import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes; +import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.NOT_SUPPORTED_OPERATION; import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.USER_LOCK; import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.VOLUME_LOCK; @@ -381,28 +383,7 @@ public boolean checkVolumeAccess(String volume, OzoneAclInfo userAcl) throws IOException { Preconditions.checkNotNull(volume); Preconditions.checkNotNull(userAcl); - metadataManager.getLock().acquireReadLock(VOLUME_LOCK, volume); - try { - String dbVolumeKey = metadataManager.getVolumeKey(volume); - OmVolumeArgs volumeArgs = - metadataManager.getVolumeTable().get(dbVolumeKey); - if (volumeArgs == null) { - LOG.debug("volume:{} does not exist", volume); - throw new OMException("Volume " + volume + " is not found", - ResultCodes.VOLUME_NOT_FOUND); - } - - Preconditions.checkState(volume.equals(volumeArgs.getVolume())); - return volumeArgs.getAclMap().hasAccess(userAcl); - } catch (IOException ex) { - if (!(ex instanceof OMException)) { - LOG.error("Check volume access failed for volume:{} user:{} rights:{}", - volume, userAcl.getName(), userAcl.getRights().toString(), ex); - } - throw ex; - } finally { - metadataManager.getLock().releaseReadLock(VOLUME_LOCK, volume); - } + throw new OMException(NOT_SUPPORTED_OPERATION); } /** @@ -446,18 +427,10 @@ public boolean addAcl(OzoneObj obj, OzoneAcl acl) throws IOException { throw new OMException("Volume " + volume + " is not found", ResultCodes.VOLUME_NOT_FOUND); } - try { - volumeArgs.addAcl(acl); - } catch (OMException ex) { - if (LOG.isDebugEnabled()) { - LOG.debug("Add acl failed.", ex); - } - return false; + if (volumeArgs.addAcl(acl)) { + metadataManager.getVolumeTable().put(dbVolumeKey, volumeArgs); + return true; } - metadataManager.getVolumeTable().put(dbVolumeKey, volumeArgs); - - Preconditions.checkState(volume.equals(volumeArgs.getVolume())); - //return volumeArgs.getAclMap().hasAccess(userAcl); } catch (IOException ex) { if (!(ex instanceof OMException)) { LOG.error("Add acl operation failed for volume:{} acl:{}", @@ -468,7 +441,7 @@ public boolean addAcl(OzoneObj obj, OzoneAcl acl) throws IOException { metadataManager.getLock().releaseWriteLock(VOLUME_LOCK, volume); } - return true; + return false; } /** @@ -498,15 +471,10 @@ public boolean removeAcl(OzoneObj obj, OzoneAcl acl) throws IOException { throw new OMException("Volume " + volume + " is not found", ResultCodes.VOLUME_NOT_FOUND); } - try { - volumeArgs.removeAcl(acl); - } catch (OMException ex) { - if (LOG.isDebugEnabled()) { - LOG.debug("Remove acl failed.", ex); - } - return false; + if (volumeArgs.removeAcl(acl)) { + metadataManager.getVolumeTable().put(dbVolumeKey, volumeArgs); + return true; } - metadataManager.getVolumeTable().put(dbVolumeKey, volumeArgs); Preconditions.checkState(volume.equals(volumeArgs.getVolume())); //return volumeArgs.getAclMap().hasAccess(userAcl); @@ -520,7 +488,7 @@ public boolean removeAcl(OzoneObj obj, OzoneAcl acl) throws IOException { metadataManager.getLock().releaseWriteLock(VOLUME_LOCK, volume); } - return true; + return false; } /** @@ -596,7 +564,7 @@ public List getAcl(OzoneObj obj) throws IOException { } Preconditions.checkState(volume.equals(volumeArgs.getVolume())); - return volumeArgs.getAclMap().getAcl(); + return volumeArgs.getAcls(); } catch (IOException ex) { if (!(ex instanceof OMException)) { LOG.error("Get acl operation failed for volume:{}", volume, ex); @@ -633,8 +601,8 @@ public boolean checkAccess(OzoneObj ozObject, RequestContext context) } Preconditions.checkState(volume.equals(volumeArgs.getVolume())); - boolean hasAccess = volumeArgs.getAclMap().hasAccess( - context.getAclRights(), context.getClientUgi()); + boolean hasAccess = OzoneAclUtil.checkAclRights( + volumeArgs.getAcls(), context); if (LOG.isDebugEnabled()) { LOG.debug("user:{} has access rights for volume:{} :{} ", context.getClientUgi(), ozObject.getVolumeName(), hasAccess); 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 1130f94e097f..3f81f40656c6 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 @@ -21,7 +21,6 @@ import java.io.IOException; import java.util.ArrayList; import java.util.List; -import java.util.stream.Collectors; import com.google.common.base.Optional; @@ -263,17 +262,15 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, */ private void addDefaultAcls(OmBucketInfo omBucketInfo, OmVolumeArgs omVolumeArgs) { - // Add default acls from volume. + // Add default acls for bucket creator. List acls = new ArrayList<>(); if (omBucketInfo.getAcls() != null) { acls.addAll(omBucketInfo.getAcls()); } - List defaultVolumeAclList = omVolumeArgs.getAclMap() - .getDefaultAclList().stream().map(OzoneAcl::fromProtobuf) - .collect(Collectors.toList()); - - OzoneAclUtil.inheritDefaultAcls(acls, defaultVolumeAclList); + // Add default acls from volume. + List defaultVolumeAcls = omVolumeArgs.getDefaultAcls(); + OzoneAclUtil.inheritDefaultAcls(acls, defaultVolumeAcls); omBucketInfo.setAcls(acls); } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeCreateRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeCreateRequest.java index 1edd035c1616..ca2a8ff82a63 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeCreateRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeCreateRequest.java @@ -19,7 +19,6 @@ package org.apache.hadoop.ozone.om.request.volume; import java.io.IOException; -import java.util.HashMap; import java.util.Map; import com.google.common.annotations.VisibleForTesting; @@ -118,7 +117,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, IOException exception = null; OMClientResponse omClientResponse = null; OmVolumeArgs omVolumeArgs = null; - Map auditMap = new HashMap<>(); + Map auditMap = null; try { omVolumeArgs = OmVolumeArgs.getFromProtobuf(volumeInfo); // when you create a volume, we set both Object ID and update ID. diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/volume/acl/TestOMVolumeAddAclRequest.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/volume/acl/TestOMVolumeAddAclRequest.java index 017d59f0e3fc..28a5ce17cdde 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/volume/acl/TestOMVolumeAddAclRequest.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/volume/acl/TestOMVolumeAddAclRequest.java @@ -19,7 +19,6 @@ package org.apache.hadoop.ozone.om.request.volume.acl; import org.apache.hadoop.ozone.OzoneAcl; -import org.apache.hadoop.ozone.om.helpers.OmOzoneAclMap; import org.apache.hadoop.ozone.om.helpers.OmVolumeArgs; import org.apache.hadoop.ozone.om.request.TestOMRequestUtils; import org.apache.hadoop.ozone.om.request.volume.TestOMVolumeRequest; @@ -31,6 +30,7 @@ import org.junit.Assert; import org.junit.Test; +import java.util.List; import java.util.UUID; /** @@ -95,12 +95,12 @@ public void testValidateAndUpdateCacheSuccess() throws Exception { Assert.assertEquals(OzoneManagerProtocolProtos.Status.OK, omResponse.getStatus()); - OmOzoneAclMap aclMapAfterSet = omMetadataManager - .getVolumeTable().get(volumeKey).getAclMap(); + List aclsAfterSet = omMetadataManager + .getVolumeTable().get(volumeKey).getAcls(); // acl is added to aclMapAfterSet - Assert.assertEquals(1, aclMapAfterSet.getAcl().size()); - Assert.assertEquals(acl, aclMapAfterSet.getAcl().get(0)); + Assert.assertEquals(1, aclsAfterSet.size()); + Assert.assertEquals(acl, aclsAfterSet.get(0)); } @Test diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/volume/acl/TestOMVolumeRemoveAclRequest.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/volume/acl/TestOMVolumeRemoveAclRequest.java index 390d547b0c91..b6ef38178e80 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/volume/acl/TestOMVolumeRemoveAclRequest.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/volume/acl/TestOMVolumeRemoveAclRequest.java @@ -19,7 +19,6 @@ package org.apache.hadoop.ozone.om.request.volume.acl; import org.apache.hadoop.ozone.OzoneAcl; -import org.apache.hadoop.ozone.om.helpers.OmOzoneAclMap; import org.apache.hadoop.ozone.om.helpers.OmVolumeArgs; import org.apache.hadoop.ozone.om.request.TestOMRequestUtils; import org.apache.hadoop.ozone.om.request.volume.TestOMVolumeRequest; @@ -30,6 +29,7 @@ import org.junit.Assert; import org.junit.Test; +import java.util.List; import java.util.UUID; /** @@ -97,8 +97,8 @@ public void testValidateAndUpdateCacheSuccess() throws Exception { omMetadataManager.getVolumeTable().get(volumeKey); // As request is valid volume table should have entry. Assert.assertNotNull(omVolumeArgs); - OmOzoneAclMap aclMapBeforeRemove = omVolumeArgs.getAclMap(); - Assert.assertEquals(acl, aclMapBeforeRemove.getAcl().get(0)); + List aclsBeforeRemove = omVolumeArgs.getAcls(); + Assert.assertEquals(acl, aclsBeforeRemove.get(0)); OMClientResponse omClientRemoveResponse = omVolumeRemoveAclRequest.validateAndUpdateCache(ozoneManager, 2, @@ -110,9 +110,9 @@ public void testValidateAndUpdateCacheSuccess() throws Exception { omRemoveAclResponse.getStatus()); // acl is removed from aclMapAfterSet - OmOzoneAclMap aclMapAfterRemove = omMetadataManager - .getVolumeTable().get(volumeKey).getAclMap(); - Assert.assertEquals(0, aclMapAfterRemove.getAcl().size()); + List aclsAfterRemove = omMetadataManager + .getVolumeTable().get(volumeKey).getAcls(); + Assert.assertEquals(0, aclsAfterRemove.size()); } @Test diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/volume/acl/TestOMVolumeSetAclRequest.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/volume/acl/TestOMVolumeSetAclRequest.java index 3c49e6dd38c9..0bd052cc495e 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/volume/acl/TestOMVolumeSetAclRequest.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/volume/acl/TestOMVolumeSetAclRequest.java @@ -20,7 +20,6 @@ import com.google.common.collect.Lists; import org.apache.hadoop.ozone.OzoneAcl; -import org.apache.hadoop.ozone.om.helpers.OmOzoneAclMap; import org.apache.hadoop.ozone.om.helpers.OmVolumeArgs; import org.apache.hadoop.ozone.om.request.TestOMRequestUtils; import org.apache.hadoop.ozone.om.request.volume.TestOMVolumeRequest; @@ -34,9 +33,6 @@ import java.util.List; import java.util.UUID; -import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OzoneAclInfo.OzoneAclScope.ACCESS; -import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OzoneAclInfo.OzoneAclScope.DEFAULT; - /** * Tests volume setAcl request. */ @@ -104,15 +100,15 @@ public void testValidateAndUpdateCacheSuccess() throws Exception { Assert.assertEquals(OzoneManagerProtocolProtos.Status.OK, omResponse.getStatus()); - OmOzoneAclMap aclMapAfterSet = omMetadataManager - .getVolumeTable().get(volumeKey).getAclMap(); + List aclsAfterSet = omMetadataManager + .getVolumeTable().get(volumeKey).getAcls(); // Acl is added to aclMapAfterSet - Assert.assertEquals(2, aclMapAfterSet.getAcl().size()); - Assert.assertTrue("Default Acl should be set.", - aclMapAfterSet.getAclsByScope(ACCESS).contains(userAccessAcl)); + Assert.assertEquals(2, aclsAfterSet.size()); + Assert.assertTrue("Access Acl should be set.", + aclsAfterSet.contains(userAccessAcl)); Assert.assertTrue("Default Acl should be set.", - aclMapAfterSet.getAclsByScope(DEFAULT).contains(groupDefaultAcl)); + aclsAfterSet.contains(groupDefaultAcl)); } @Test diff --git a/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/containergenerator/GeneratorOm.java b/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/containergenerator/GeneratorOm.java index f03be6444536..bd31c84e5c45 100644 --- a/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/containergenerator/GeneratorOm.java +++ b/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/containergenerator/GeneratorOm.java @@ -151,12 +151,12 @@ private void writeOmBucketVolume() throws IOException { .setObjectID(1L) .setUpdateID(1L) .setQuotaInBytes(100L) - .addOzoneAcls(OzoneAcl.toProtobuf( + .addOzoneAcls( new OzoneAcl(IAccessAuthorizer.ACLIdentityType.WORLD, "", - IAccessAuthorizer.ACLType.ALL, ACCESS))) - .addOzoneAcls(OzoneAcl.toProtobuf( - new OzoneAcl(IAccessAuthorizer.ACLIdentityType.USER, getUserId(), IAccessAuthorizer.ACLType.ALL, ACCESS)) + .addOzoneAcls( + new OzoneAcl(IAccessAuthorizer.ACLIdentityType.USER, getUserId(), + IAccessAuthorizer.ACLType.ALL, ACCESS) ).build(); volTable.put("/" + volumeName, omVolumeArgs);