Skip to content

HDDS-1927. Consolidate add/remove Acl into OzoneAclUtil class. Contri… #1263

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

HDDS-1927. Consolidate add/remove Acl into OzoneAclUtil class. Contri… #1263

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5ca1ebd
HDDS-1927. Consolidate add/remove Acl into OzoneAclUtil class. Contri…
xiaoyuyao Aug 19, 2019
33b807f
remove unused helper functions
xiaoyuyao Aug 21, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,10 @@
import org.apache.hadoop.ozone.om.helpers.OmPartInfo;
import org.apache.hadoop.ozone.om.helpers.OmVolumeArgs;
import org.apache.hadoop.ozone.om.helpers.OpenKeySession;
import org.apache.hadoop.ozone.om.helpers.OzoneAclUtil;
import org.apache.hadoop.ozone.om.helpers.OzoneFileStatus;
import org.apache.hadoop.ozone.om.helpers.S3SecretValue;
import org.apache.hadoop.ozone.om.helpers.ServiceInfo;
import org.apache.hadoop.ozone.om.helpers.OzoneFileStatus;
import org.apache.hadoop.ozone.om.protocol.OzoneManagerProtocol;
import org.apache.hadoop.ozone.om.protocolPB
.OzoneManagerProtocolClientSideTranslatorPB;
Expand Down Expand Up @@ -440,7 +441,7 @@ public void createBucket(
* @return listOfAcls
* */
private List<OzoneAcl> getAclList() {
return OzoneUtils.getAclList(ugi.getUserName(), ugi.getGroups(),
return OzoneAclUtil.getAclList(ugi.getUserName(), ugi.getGroups(),
userRights, groupRights);
}

Expand Down
5 changes: 5 additions & 0 deletions hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OzoneAcl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,11 @@ public boolean equals(Object obj) {
otherAcl.getAclScope().equals(this.getAclScope());
}

public OzoneAcl setAclScope(AclScope scope) {
this.aclScope = scope;
return this;
}

/**
* Scope of ozone acl.
* */
Expand Down
76 changes: 7 additions & 69 deletions hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketInfo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
package org.apache.hadoop.ozone.om.helpers;


import java.util.BitSet;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.LinkedList;
Expand All @@ -37,8 +36,6 @@

import com.google.common.base.Preconditions;

import static org.apache.hadoop.ozone.OzoneAcl.ZERO_BITSET;

/**
* A class that encapsulates Bucket Info.
*/
Expand Down Expand Up @@ -135,36 +132,7 @@ public List<OzoneAcl> getAcls() {
* already existing in the acl list.
*/
public boolean addAcl(OzoneAcl ozoneAcl) {
// Case 1: When we are adding more rights to existing user/group.
boolean addToExistingAcl = false;
for(OzoneAcl existingAcl: getAcls()) {
if(existingAcl.getName().equals(ozoneAcl.getName()) &&
existingAcl.getType().equals(ozoneAcl.getType())) {

BitSet bits = (BitSet) ozoneAcl.getAclBitSet().clone();

// We need to do "or" before comparision because think of a case like
// existing acl is 777 and newly added acl is 444, we have already
// that acl set. In this case if we do direct check they will not
// be equal, but if we do or and then check, we shall know it
// has acl's already set or not.
bits.or(existingAcl.getAclBitSet());

if (bits.equals(existingAcl.getAclBitSet())) {
return false;
} else {
existingAcl.getAclBitSet().or(ozoneAcl.getAclBitSet());
addToExistingAcl = true;
break;
}
}
}

// Case 2: When a completely new acl is added.
if(!addToExistingAcl) {
getAcls().add(ozoneAcl);
}
return true;
return OzoneAclUtil.addAcl(acls, ozoneAcl);
}

/**
Expand All @@ -174,36 +142,7 @@ public boolean addAcl(OzoneAcl ozoneAcl) {
* to that acl is not in the existing acl list.
*/
public boolean removeAcl(OzoneAcl ozoneAcl) {
boolean removed = false;

// When we are removing subset of rights from existing acl.
for(OzoneAcl existingAcl: getAcls()) {
if (existingAcl.getName().equals(ozoneAcl.getName()) &&
existingAcl.getType().equals(ozoneAcl.getType())) {
BitSet bits = (BitSet) ozoneAcl.getAclBitSet().clone();
bits.and(existingAcl.getAclBitSet());

// This happens when the acl bitset is not existing for current name
// and type.
// Like a case we have 444 permission, 333 is asked to removed.
if (bits.equals(ZERO_BITSET)) {
return false;
}

// We have some matching. Remove them.
existingAcl.getAclBitSet().xor(bits);

// If existing acl has same bitset as passed acl bitset, remove that
// acl from the list
if (existingAcl.getAclBitSet().equals(ZERO_BITSET)) {
getAcls().remove(existingAcl);
}
removed = true;
break;
}
}

return removed;
return OzoneAclUtil.removeAcl(acls, ozoneAcl);
}

/**
Expand All @@ -212,9 +151,7 @@ public boolean removeAcl(OzoneAcl ozoneAcl) {
* @return true - if successfully able to reset.
*/
public boolean setAcls(List<OzoneAcl> ozoneAcls) {
this.acls.clear();
this.acls = ozoneAcls;
return true;
return OzoneAclUtil.setAcl(acls, ozoneAcls);
}

/**
Expand Down Expand Up @@ -307,7 +244,9 @@ public Builder setBucketName(String bucket) {
}

public Builder setAcls(List<OzoneAcl> listOfAcls) {
this.acls = listOfAcls;
if (listOfAcls != null) {
this.acls.addAll(listOfAcls);
}
return this;
}

Expand Down Expand Up @@ -367,8 +306,7 @@ public BucketInfo getProtobuf() {
BucketInfo.Builder bib = BucketInfo.newBuilder()
.setVolumeName(volumeName)
.setBucketName(bucketName)
.addAllAcls(acls.stream().map(OzoneAcl::toProtobuf)
.collect(Collectors.toList()))
.addAllAcls(OzoneAclUtil.toProtobuf(acls))
.setIsVersionEnabled(isVersionEnabled)
.setStorageType(storageType.toProto())
.setCreationTime(creationTime)
Expand Down
147 changes: 22 additions & 125 deletions hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,25 +19,21 @@

import java.io.IOException;
import java.util.ArrayList;
import java.util.BitSet;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.stream.Collectors;

import com.google.protobuf.ByteString;
import org.apache.hadoop.fs.FileEncryptionInfo;
import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
import org.apache.hadoop.ozone.OzoneAcl;
import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.KeyInfo;
import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OzoneAclInfo;
import org.apache.hadoop.ozone.protocolPB.OMPBHelper;
import org.apache.hadoop.util.Time;

import com.google.common.base.Preconditions;

import static org.apache.hadoop.ozone.OzoneAcl.ZERO_BITSET;

/**
* Args for key block. The block instance for the key requested in putKey.
* This is returned from OM to client, and client use class to talk to
Expand All @@ -58,7 +54,7 @@ public final class OmKeyInfo extends WithMetadata {
/**
* ACL Information.
*/
private List<OzoneAclInfo> acls;
private List<OzoneAcl> acls;

@SuppressWarnings("parameternumber")
OmKeyInfo(String volumeName, String bucketName, String keyName,
Expand All @@ -67,7 +63,7 @@ public final class OmKeyInfo extends WithMetadata {
HddsProtos.ReplicationType type,
HddsProtos.ReplicationFactor factor,
Map<String, String> metadata,
FileEncryptionInfo encInfo, List<OzoneAclInfo> acls) {
FileEncryptionInfo encInfo, List<OzoneAcl> acls) {
this.volumeName = volumeName;
this.bucketName = bucketName;
this.keyName = keyName;
Expand Down Expand Up @@ -235,123 +231,22 @@ public FileEncryptionInfo getFileEncryptionInfo() {
return encInfo;
}

public List<OzoneAclInfo> getAcls() {
public List<OzoneAcl> getAcls() {
return acls;
}

/**
* Add an ozoneAcl to list of existing Acl set.
* @param ozoneAcl
* @return true - if successfully added, false if not added or acl is
* already existing in the acl list.
*/
public boolean addAcl(OzoneAclInfo ozoneAcl) {
// Case 1: When we are adding more rights to existing user/group.
boolean addToExistingAcl = false;
for(OzoneAclInfo existingAcl: getAcls()) {
if(existingAcl.getName().equals(ozoneAcl.getName()) &&
existingAcl.getType().equals(ozoneAcl.getType())) {

// We need to do "or" before comparision because think of a case like
// existing acl is 777 and newly added acl is 444, we have already
// that acl set. In this case if we do direct check they will not
// be equal, but if we do or and then check, we shall know it
// has acl's already set or not.
BitSet newAclBits = BitSet.valueOf(
existingAcl.getRights().toByteArray());

newAclBits.or(BitSet.valueOf(ozoneAcl.getRights().toByteArray()));

if (newAclBits.equals(BitSet.valueOf(
existingAcl.getRights().toByteArray()))) {
return false;
} else {
OzoneAclInfo newAcl = OzoneAclInfo.newBuilder()
.setType(ozoneAcl.getType())
.setName(ozoneAcl.getName())
.setAclScope(ozoneAcl.getAclScope())
.setRights(ByteString.copyFrom(newAclBits.toByteArray()))
.build();
getAcls().remove(existingAcl);
getAcls().add(newAcl);
addToExistingAcl = true;
break;
}
}
}

// Case 2: When a completely new acl is added.
if(!addToExistingAcl) {
getAcls().add(ozoneAcl);
}
return true;
public boolean addAcl(OzoneAcl acl) {
return OzoneAclUtil.addAcl(acls, acl);
}

/**
* Remove acl from existing acl list.
* @param ozoneAcl
* @return true - if successfully removed, false if not able to remove due
* to that acl is not in the existing acl list.
*/
public boolean removeAcl(OzoneAclInfo ozoneAcl) {
boolean removed = false;

// When we are removing subset of rights from existing acl.
for(OzoneAclInfo existingAcl: getAcls()) {
if (existingAcl.getName().equals(ozoneAcl.getName()) &&
existingAcl.getType().equals(ozoneAcl.getType())) {

BitSet bits = BitSet.valueOf(ozoneAcl.getRights().toByteArray());
BitSet existingAclBits =
BitSet.valueOf(existingAcl.getRights().toByteArray());
bits.and(existingAclBits);

// This happens when the acl bitset asked to remove is not set for
// matched name and type.
// Like a case we have 444 permission, 333 is asked to removed.
if (bits.equals(ZERO_BITSET)) {
return false;
}

// We have some matching. Remove them.
bits.xor(existingAclBits);

// If existing acl has same bitset as passed acl bitset, remove that
// acl from the list
if (bits.equals(ZERO_BITSET)) {
getAcls().remove(existingAcl);
} else {
// Remove old acl and add new acl.
OzoneAclInfo newAcl = OzoneAclInfo.newBuilder()
.setType(ozoneAcl.getType())
.setName(ozoneAcl.getName())
.setAclScope(ozoneAcl.getAclScope())
.setRights(ByteString.copyFrom(bits.toByteArray()))
.build();
getAcls().remove(existingAcl);
getAcls().add(newAcl);
}
removed = true;
break;
}
}

return removed;
public boolean removeAcl(OzoneAcl acl) {
return OzoneAclUtil.removeAcl(acls, acl);
}

/**
* Reset the existing acl list.
* @param ozoneAcls
* @return true - if successfully able to reset.
*/
public boolean setAcls(List<OzoneAclInfo> ozoneAcls) {
this.acls.clear();
this.acls = ozoneAcls;
return true;
public boolean setAcls(List<OzoneAcl> newAcls) {
return OzoneAclUtil.setAcl(acls, newAcls);
}



/**
* Builder of OmKeyInfo.
*/
Expand All @@ -368,11 +263,12 @@ public static class Builder {
private HddsProtos.ReplicationFactor factor;
private Map<String, String> metadata;
private FileEncryptionInfo encInfo;
private List<OzoneAclInfo> acls;
private List<OzoneAcl> acls;

public Builder() {
this.metadata = new HashMap<>();
omKeyLocationInfoGroups = new ArrayList<>();
acls = new ArrayList<>();
}

public Builder setVolumeName(String volume) {
Expand Down Expand Up @@ -436,9 +332,10 @@ public Builder setFileEncryptionInfo(FileEncryptionInfo feInfo) {
return this;
}

public Builder setAcls(List<OzoneAclInfo> listOfAcls) {
this.acls = new ArrayList<>();
this.acls.addAll(listOfAcls);
public Builder setAcls(List<OzoneAcl> listOfAcls) {
if (listOfAcls != null) {
this.acls.addAll(listOfAcls);
}
return this;
}

Expand Down Expand Up @@ -466,13 +363,11 @@ public KeyInfo getProtobuf() {
.setLatestVersion(latestVersion)
.setCreationTime(creationTime)
.setModificationTime(modificationTime)
.addAllMetadata(KeyValueUtil.toProtobuf(metadata));
.addAllMetadata(KeyValueUtil.toProtobuf(metadata))
.addAllAcls(OzoneAclUtil.toProtobuf(acls));
if (encInfo != null) {
kb.setFileEncryptionInfo(OMPBHelper.convert(encInfo));
}
if(acls != null) {
kb.addAllAcls(acls);
}
return kb.build();
}

Expand All @@ -492,7 +387,8 @@ public static OmKeyInfo getFromProtobuf(KeyInfo keyInfo) {
.addAllMetadata(KeyValueUtil.getFromProtobuf(keyInfo.getMetadataList()))
.setFileEncryptionInfo(keyInfo.hasFileEncryptionInfo() ?
OMPBHelper.convert(keyInfo.getFileEncryptionInfo()): null)
.setAcls(keyInfo.getAclsList()).build();
.setAcls(OzoneAclUtil.fromProtobuf(keyInfo.getAclsList()))
.build();
}

@Override
Expand All @@ -514,7 +410,8 @@ public boolean equals(Object o) {
.equals(keyLocationVersions, omKeyInfo.keyLocationVersions) &&
type == omKeyInfo.type &&
factor == omKeyInfo.factor &&
Objects.equals(metadata, omKeyInfo.metadata);
Objects.equals(metadata, omKeyInfo.metadata) &&
Objects.equals(acls, omKeyInfo.acls);
}

@Override
Expand Down
Loading