diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneBucket.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneBucket.java index 7843ad36038..fbb50e212e3 100644 --- a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneBucket.java +++ b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneBucket.java @@ -35,6 +35,8 @@ import org.apache.hadoop.ozone.om.helpers.OmMultipartInfo; import org.apache.hadoop.ozone.om.helpers.OmMultipartUploadCompleteInfo; import org.apache.hadoop.ozone.om.helpers.OzoneFileStatus; import org.apache.hadoop.ozone.om.helpers.WithMetadata; +import org.apache.hadoop.ozone.security.acl.OzoneObj; +import org.apache.hadoop.ozone.security.acl.OzoneObjInfo; import java.io.IOException; import java.util.HashMap; @@ -70,10 +72,6 @@ public class OzoneBucket extends WithMetadata { * Default replication type to be used while creating keys. */ private final ReplicationType defaultReplicationType; - /** - * Bucket ACLs. - */ - private List acls; /** * Type of storage to be used for this bucket. @@ -101,28 +99,47 @@ public class OzoneBucket extends WithMetadata { */ private String encryptionKeyName; - @SuppressWarnings("parameternumber") - public OzoneBucket(Configuration conf, ClientProtocol proxy, - String volumeName, String bucketName, - List acls, StorageType storageType, - Boolean versioning, long creationTime, - Map metadata, - String encryptionKeyName) { + private OzoneObj ozoneObj; + + + private OzoneBucket(Configuration conf, String volumeName, + String bucketName, ReplicationFactor defaultReplication, + ReplicationType defaultReplicationType, ClientProtocol proxy) { Preconditions.checkNotNull(proxy, "Client proxy is not set."); - this.proxy = proxy; this.volumeName = volumeName; this.name = bucketName; - this.acls = acls; + if (defaultReplication == null) { + this.defaultReplication = ReplicationFactor.valueOf(conf.getInt( + OzoneConfigKeys.OZONE_REPLICATION, + OzoneConfigKeys.OZONE_REPLICATION_DEFAULT)); + } else { + this.defaultReplication = defaultReplication; + } + + if (defaultReplicationType == null) { + this.defaultReplicationType = ReplicationType.valueOf(conf.get( + OzoneConfigKeys.OZONE_REPLICATION_TYPE, + OzoneConfigKeys.OZONE_REPLICATION_TYPE_DEFAULT)); + } else { + this.defaultReplicationType = defaultReplicationType; + } + this.proxy = proxy; + this.ozoneObj = OzoneObjInfo.Builder.newBuilder() + .setBucketName(bucketName) + .setVolumeName(volumeName) + .setResType(OzoneObj.ResourceType.BUCKET) + .setStoreType(OzoneObj.StoreType.OZONE).build(); + } + @SuppressWarnings("parameternumber") + public OzoneBucket(Configuration conf, ClientProtocol proxy, + String volumeName, String bucketName, StorageType storageType, + Boolean versioning, long creationTime, Map metadata, + String encryptionKeyName) { + this(conf, volumeName, bucketName, null, null, proxy); this.storageType = storageType; this.versioning = versioning; this.listCacheSize = HddsClientUtils.getListCacheSize(conf); this.creationTime = creationTime; - this.defaultReplication = ReplicationFactor.valueOf(conf.getInt( - OzoneConfigKeys.OZONE_REPLICATION, - OzoneConfigKeys.OZONE_REPLICATION_DEFAULT)); - this.defaultReplicationType = ReplicationType.valueOf(conf.get( - OzoneConfigKeys.OZONE_REPLICATION_TYPE, - OzoneConfigKeys.OZONE_REPLICATION_TYPE_DEFAULT)); this.metadata = metadata; this.encryptionKeyName = encryptionKeyName; } @@ -133,32 +150,19 @@ public class OzoneBucket extends WithMetadata { * @param proxy ClientProtocol proxy. * @param volumeName Name of the volume the bucket belongs to. * @param bucketName Name of the bucket. - * @param acls ACLs associated with the bucket. * @param storageType StorageType of the bucket. * @param versioning versioning status of the bucket. * @param creationTime creation time of the bucket. */ @SuppressWarnings("parameternumber") public OzoneBucket(Configuration conf, ClientProtocol proxy, - String volumeName, String bucketName, - List acls, StorageType storageType, - Boolean versioning, long creationTime, - Map metadata) { - Preconditions.checkNotNull(proxy, "Client proxy is not set."); - this.proxy = proxy; - this.volumeName = volumeName; - this.name = bucketName; - this.acls = acls; + String volumeName, String bucketName, StorageType storageType, + Boolean versioning, long creationTime, Map metadata) { + this(conf, volumeName, bucketName, null, null, proxy); this.storageType = storageType; this.versioning = versioning; this.listCacheSize = HddsClientUtils.getListCacheSize(conf); this.creationTime = creationTime; - this.defaultReplication = ReplicationFactor.valueOf(conf.getInt( - OzoneConfigKeys.OZONE_REPLICATION, - OzoneConfigKeys.OZONE_REPLICATION_DEFAULT)); - this.defaultReplicationType = ReplicationType.valueOf(conf.get( - OzoneConfigKeys.OZONE_REPLICATION_TYPE, - OzoneConfigKeys.OZONE_REPLICATION_TYPE_DEFAULT)); this.metadata = metadata; } @@ -166,20 +170,24 @@ public class OzoneBucket extends WithMetadata { @SuppressWarnings("parameternumber") OzoneBucket(String volumeName, String name, ReplicationFactor defaultReplication, - ReplicationType defaultReplicationType, - List acls, StorageType storageType, Boolean versioning, - long creationTime) { + ReplicationType defaultReplicationType, StorageType storageType, + Boolean versioning, long creationTime) { this.proxy = null; this.volumeName = volumeName; this.name = name; this.defaultReplication = defaultReplication; this.defaultReplicationType = defaultReplicationType; - this.acls = acls; this.storageType = storageType; this.versioning = versioning; this.creationTime = creationTime; + this.ozoneObj = OzoneObjInfo.Builder.newBuilder() + .setBucketName(name) + .setVolumeName(volumeName) + .setResType(OzoneObj.ResourceType.BUCKET) + .setStoreType(OzoneObj.StoreType.OZONE).build(); } + /** * Returns Volume Name. * @@ -203,8 +211,8 @@ public class OzoneBucket extends WithMetadata { * * @return acls */ - public List getAcls() { - return acls; + public List getAcls() throws IOException { + return proxy.getAcl(ozoneObj); } /** @@ -244,23 +252,23 @@ public class OzoneBucket extends WithMetadata { /** * Adds ACLs to the Bucket. - * @param addAcls ACLs to be added + * @param addAcl ACL to be added + * @return true - if acl is successfully added, false if acl already exists + * for the bucket. * @throws IOException */ - public void addAcls(List addAcls) throws IOException { - proxy.addBucketAcls(volumeName, name, addAcls); - addAcls.stream().filter(acl -> !acls.contains(acl)).forEach( - acls::add); + public boolean addAcls(OzoneAcl addAcl) throws IOException { + return proxy.addAcl(ozoneObj, addAcl); } /** * Removes ACLs from the bucket. - * @param removeAcls ACLs to be removed + * @return true - if acl is successfully removed, false if acl to be + * removed does not exist for the bucket. * @throws IOException */ - public void removeAcls(List removeAcls) throws IOException { - proxy.removeBucketAcls(volumeName, name, removeAcls); - acls.removeAll(removeAcls); + public boolean removeAcls(OzoneAcl removeAcl) throws IOException { + return proxy.removeAcl(ozoneObj, removeAcl); } /** diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneClientUtils.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneClientUtils.java index 38c0ba76840..3da45cfc902 100644 --- a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneClientUtils.java +++ b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneClientUtils.java @@ -17,6 +17,7 @@ */ package org.apache.hadoop.ozone.client; +import java.io.IOException; import java.util.ArrayList; import java.util.List; import java.util.concurrent.TimeUnit; @@ -45,7 +46,7 @@ public final class OzoneClientUtils { * be created. * @return BucketInfo instance */ - public static BucketInfo asBucketInfo(OzoneBucket bucket) { + public static BucketInfo asBucketInfo(OzoneBucket bucket) throws IOException { BucketInfo bucketInfo = new BucketInfo(bucket.getVolumeName(), bucket.getName()); bucketInfo @@ -53,7 +54,6 @@ public final class OzoneClientUtils { bucketInfo.setStorageType(bucket.getStorageType()); bucketInfo.setVersioning( OzoneConsts.Versioning.getVersioning(bucket.getVersioning())); - bucketInfo.setAcls(bucket.getAcls()); bucketInfo.setEncryptionKeyName( bucket.getEncryptionKeyName()==null? "N/A" : bucket.getEncryptionKeyName()); 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 d5bac34eefd..efc0b8afc99 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 @@ -174,28 +174,6 @@ public interface ClientProtocol { BucketArgs bucketArgs) throws IOException; - /** - * Adds ACLs to the Bucket. - * @param volumeName Name of the Volume - * @param bucketName Name of the Bucket - * @param addAcls ACLs to be added - * @throws IOException - */ - void addBucketAcls(String volumeName, String bucketName, - List addAcls) - throws IOException; - - /** - * Removes ACLs from a Bucket. - * @param volumeName Name of the Volume - * @param bucketName Name of the Bucket - * @param removeAcls ACLs to be removed - * @throws IOException - */ - void removeBucketAcls(String volumeName, String bucketName, - List removeAcls) - throws IOException; - /** * Enables or disables Bucket Versioning. diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rest/RestClient.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rest/RestClient.java index 7b37e498e2d..e01c2c3eec5 100644 --- a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rest/RestClient.java +++ b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rest/RestClient.java @@ -446,54 +446,6 @@ public class RestClient implements ClientProtocol { } } - @Override - public void addBucketAcls( - String volumeName, String bucketName, List addAcls) - throws IOException { - try { - HddsClientUtils.verifyResourceName(volumeName, bucketName); - Preconditions.checkNotNull(addAcls); - URIBuilder builder = new URIBuilder(ozoneRestUri); - - builder.setPath(PATH_SEPARATOR + volumeName + - PATH_SEPARATOR + bucketName); - HttpPut httpPut = new HttpPut(builder.build()); - addOzoneHeaders(httpPut); - - for (OzoneAcl acl : addAcls) { - httpPut.addHeader( - Header.OZONE_ACLS, Header.OZONE_ACL_ADD + " " + acl.toString()); - } - EntityUtils.consume(executeHttpRequest(httpPut)); - } catch (URISyntaxException e) { - throw new IOException(e); - } - } - - @Override - public void removeBucketAcls( - String volumeName, String bucketName, List removeAcls) - throws IOException { - try { - HddsClientUtils.verifyResourceName(volumeName, bucketName); - Preconditions.checkNotNull(removeAcls); - URIBuilder builder = new URIBuilder(ozoneRestUri); - - builder.setPath(PATH_SEPARATOR + volumeName + - PATH_SEPARATOR + bucketName); - HttpPut httpPut = new HttpPut(builder.build()); - addOzoneHeaders(httpPut); - - for (OzoneAcl acl : removeAcls) { - httpPut.addHeader( - Header.OZONE_ACLS, Header.OZONE_ACL_REMOVE + " " + acl.toString()); - } - EntityUtils.consume(executeHttpRequest(httpPut)); - } catch (URISyntaxException e) { - throw new IOException(e); - } - } - @Override public void setBucketVersioning( String volumeName, String bucketName, Boolean versioning) @@ -578,7 +530,6 @@ public class RestClient implements ClientProtocol { this, bucketInfo.getVolumeName(), bucketInfo.getBucketName(), - bucketInfo.getAcls(), bucketInfo.getStorageType(), getBucketVersioningFlag(bucketInfo.getVersioning()), HddsClientUtils.formatDateTime(bucketInfo.getCreatedOn()), @@ -619,11 +570,9 @@ public class RestClient implements ClientProtocol { LOG.warn("Parse exception in getting creation time for volume", e); } return new OzoneBucket(conf, this, volumeName, - bucketInfo.getBucketName(), bucketInfo.getAcls(), - bucketInfo.getStorageType(), + bucketInfo.getBucketName(), bucketInfo.getStorageType(), getBucketVersioningFlag(bucketInfo.getVersioning()), creationTime, - new HashMap<>(), bucketInfo - .getEncryptionKeyName()); + new HashMap<>(), bucketInfo.getEncryptionKeyName()); }).collect(Collectors.toList()); } catch (URISyntaxException e) { throw new IOException(e); 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 bd41adac047..fbb488ecd46 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 @@ -388,7 +388,9 @@ public class RpcClient implements ClientProtocol { @Override public void createBucket(String volumeName, String bucketName) throws IOException { - createBucket(volumeName, bucketName, BucketArgs.newBuilder().build()); + // Set acls of current user. + createBucket(volumeName, bucketName, + BucketArgs.newBuilder().build()); } @Override @@ -442,32 +444,6 @@ public class RpcClient implements ClientProtocol { userRights, groupRights); } - @Override - public void addBucketAcls( - String volumeName, String bucketName, List addAcls) - throws IOException { - HddsClientUtils.verifyResourceName(volumeName, bucketName); - Preconditions.checkNotNull(addAcls); - OmBucketArgs.Builder builder = OmBucketArgs.newBuilder(); - builder.setVolumeName(volumeName) - .setBucketName(bucketName) - .setAddAcls(addAcls); - ozoneManagerClient.setBucketProperty(builder.build()); - } - - @Override - public void removeBucketAcls( - String volumeName, String bucketName, List removeAcls) - throws IOException { - HddsClientUtils.verifyResourceName(volumeName, bucketName); - Preconditions.checkNotNull(removeAcls); - OmBucketArgs.Builder builder = OmBucketArgs.newBuilder(); - builder.setVolumeName(volumeName) - .setBucketName(bucketName) - .setRemoveAcls(removeAcls); - ozoneManagerClient.setBucketProperty(builder.build()); - } - /** * Get a valid Delegation Token. * @@ -586,7 +562,6 @@ public class RpcClient implements ClientProtocol { this, bucketInfo.getVolumeName(), bucketInfo.getBucketName(), - bucketInfo.getAcls(), bucketInfo.getStorageType(), bucketInfo.getIsVersionEnabled(), bucketInfo.getCreationTime(), @@ -607,7 +582,6 @@ public class RpcClient implements ClientProtocol { this, bucket.getVolumeName(), bucket.getBucketName(), - bucket.getAcls(), bucket.getStorageType(), bucket.getIsVersionEnabled(), bucket.getCreationTime(), @@ -794,7 +768,6 @@ public class RpcClient implements ClientProtocol { this, bucket.getVolumeName(), bucket.getBucketName(), - bucket.getAcls(), bucket.getStorageType(), bucket.getIsVersionEnabled(), bucket.getCreationTime(), diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketArgs.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketArgs.java index 6ff29de2354..8a938a95460 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketArgs.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketArgs.java @@ -18,12 +18,9 @@ package org.apache.hadoop.ozone.om.helpers; import java.util.LinkedHashMap; -import java.util.List; import java.util.Map; -import java.util.stream.Collectors; import org.apache.hadoop.hdds.protocol.StorageType; -import org.apache.hadoop.ozone.OzoneAcl; import org.apache.hadoop.ozone.OzoneConsts; import org.apache.hadoop.ozone.audit.Auditable; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.BucketArgs; @@ -42,14 +39,6 @@ public final class OmBucketArgs extends WithMetadata implements Auditable { * Name of the bucket. */ private final String bucketName; - /** - * ACL's that are to be added for the bucket. - */ - private List addAcls; - /** - * ACL's that are to be removed from the bucket. - */ - private List removeAcls; /** * Bucket Version flag. */ @@ -64,19 +53,14 @@ public final class OmBucketArgs extends WithMetadata implements Auditable { * Private constructor, constructed via builder. * @param volumeName - Volume name. * @param bucketName - Bucket name. - * @param addAcls - ACL's to be added. - * @param removeAcls - ACL's to be removed. * @param isVersionEnabled - Bucket version flag. * @param storageType - Storage type to be used. */ private OmBucketArgs(String volumeName, String bucketName, - List addAcls, List removeAcls, Boolean isVersionEnabled, StorageType storageType, Map metadata) { this.volumeName = volumeName; this.bucketName = bucketName; - this.addAcls = addAcls; - this.removeAcls = removeAcls; this.isVersionEnabled = isVersionEnabled; this.storageType = storageType; this.metadata = metadata; @@ -98,22 +82,6 @@ public final class OmBucketArgs extends WithMetadata implements Auditable { return bucketName; } - /** - * Returns the ACL's that are to be added. - * @return {@literal List} - */ - public List getAddAcls() { - return addAcls; - } - - /** - * Returns the ACL's that are to be removed. - * @return {@literal List} - */ - public List getRemoveAcls() { - return removeAcls; - } - /** * Returns true if bucket version is enabled, else false. * @return isVersionEnabled @@ -144,12 +112,6 @@ public final class OmBucketArgs extends WithMetadata implements Auditable { Map auditMap = new LinkedHashMap<>(); auditMap.put(OzoneConsts.VOLUME, this.volumeName); auditMap.put(OzoneConsts.BUCKET, this.bucketName); - if(this.addAcls != null){ - auditMap.put(OzoneConsts.ADD_ACLS, this.addAcls.toString()); - } - if(this.removeAcls != null){ - auditMap.put(OzoneConsts.REMOVE_ACLS, this.removeAcls.toString()); - } auditMap.put(OzoneConsts.IS_VERSION_ENABLED, String.valueOf(this.isVersionEnabled)); if(this.storageType != null){ @@ -164,8 +126,6 @@ public final class OmBucketArgs extends WithMetadata implements Auditable { public static class Builder { private String volumeName; private String bucketName; - private List addAcls; - private List removeAcls; private Boolean isVersionEnabled; private StorageType storageType; private Map metadata; @@ -180,16 +140,6 @@ public final class OmBucketArgs extends WithMetadata implements Auditable { return this; } - public Builder setAddAcls(List acls) { - this.addAcls = acls; - return this; - } - - public Builder setRemoveAcls(List acls) { - this.removeAcls = acls; - return this; - } - public Builder setIsVersionEnabled(Boolean versionFlag) { this.isVersionEnabled = versionFlag; return this; @@ -212,8 +162,8 @@ public final class OmBucketArgs extends WithMetadata implements Auditable { public OmBucketArgs build() { Preconditions.checkNotNull(volumeName); Preconditions.checkNotNull(bucketName); - return new OmBucketArgs(volumeName, bucketName, addAcls, - removeAcls, isVersionEnabled, storageType, metadata); + return new OmBucketArgs(volumeName, bucketName, isVersionEnabled, + storageType, metadata); } } @@ -224,14 +174,6 @@ public final class OmBucketArgs extends WithMetadata implements Auditable { BucketArgs.Builder builder = BucketArgs.newBuilder(); builder.setVolumeName(volumeName) .setBucketName(bucketName); - if(addAcls != null && !addAcls.isEmpty()) { - builder.addAllAddAcls(addAcls.stream().map( - OzoneAcl::toProtobuf).collect(Collectors.toList())); - } - if(removeAcls != null && !removeAcls.isEmpty()) { - builder.addAllRemoveAcls(removeAcls.stream().map( - OzoneAcl::toProtobuf).collect(Collectors.toList())); - } if(isVersionEnabled != null) { builder.setIsVersionEnabled(isVersionEnabled); } @@ -249,10 +191,6 @@ public final class OmBucketArgs extends WithMetadata implements Auditable { public static OmBucketArgs getFromProtobuf(BucketArgs bucketArgs) { return new OmBucketArgs(bucketArgs.getVolumeName(), bucketArgs.getBucketName(), - bucketArgs.getAddAclsList().stream().map( - OzoneAcl::fromProtobuf).collect(Collectors.toList()), - bucketArgs.getRemoveAclsList().stream().map( - OzoneAcl::fromProtobuf).collect(Collectors.toList()), bucketArgs.hasIsVersionEnabled() ? bucketArgs.getIsVersionEnabled() : null, bucketArgs.hasStorageType() ? StorageType.valueOf( diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/web/handlers/BucketArgs.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/web/handlers/BucketArgs.java index a1b787cf208..63860dad80f 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/web/handlers/BucketArgs.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/web/handlers/BucketArgs.java @@ -18,20 +18,14 @@ package org.apache.hadoop.ozone.web.handlers; import org.apache.hadoop.hdds.protocol.StorageType; -import org.apache.hadoop.ozone.OzoneAcl; import org.apache.hadoop.ozone.OzoneConsts; -import java.util.LinkedList; -import java.util.List; - /** * BucketArgs packages all bucket related arguments to * file system calls. */ public class BucketArgs extends VolumeArgs { private final String bucketName; - private List addAcls; - private List removeAcls; private OzoneConsts.Versioning versioning; private StorageType storageType; @@ -70,8 +64,6 @@ public class BucketArgs extends VolumeArgs { */ public BucketArgs(BucketArgs args) { this(args.getBucketName(), args); - this.setAddAcls(args.getAddAcls()); - this.setRemoveAcls(args.getRemoveAcls()); } /** @@ -83,78 +75,6 @@ public class BucketArgs extends VolumeArgs { return bucketName; } - /** - * Returns Additive ACLs for the Bucket if specified. - * - * @return acls - */ - public List getAddAcls() { - return addAcls; - } - - /** - * Set Additive ACLs. - * - * @param acl - ACL - */ - public void setAddAcls(List acl) { - this.addAcls = acl; - } - - /** - * Returns remove ACLs for the Bucket if specified. - * - * @return acls - */ - public List getRemoveAcls() { - return removeAcls; - } - - /** - * Takes an ACL and sets the ACL object to ACL represented by the String. - * - * @param aclString - aclString - */ - public void addAcls(List aclString) throws IllegalArgumentException { - if (aclString == null) { - throw new IllegalArgumentException("ACLs cannot be null"); - } - if (this.addAcls == null) { - this.addAcls = new LinkedList<>(); - } - for (String s : aclString) { - this.addAcls.add(OzoneAcl.parseAcl(s)); - } - } - - /** - * Takes an ACL and sets the ACL object to ACL represented by the String. - * - * @param aclString - aclString - */ - public void removeAcls(List aclString) - throws IllegalArgumentException { - if (aclString == null) { - throw new IllegalArgumentException("ACLs cannot be null"); - } - if (this.removeAcls == null) { - this.removeAcls = new LinkedList<>(); - } - for (String s : aclString) { - this.removeAcls.add(OzoneAcl.parseAcl(s)); - } - } - - /** - * Set remove ACLs. - * - * @param acl - ACL - */ - public void setRemoveAcls(List acl) { - this.removeAcls = acl; - } - - /** * Returns Versioning Info. * diff --git a/hadoop-ozone/common/src/main/proto/OzoneManagerProtocol.proto b/hadoop-ozone/common/src/main/proto/OzoneManagerProtocol.proto index 6f2107cc4cb..1f40f7c60b7 100644 --- a/hadoop-ozone/common/src/main/proto/OzoneManagerProtocol.proto +++ b/hadoop-ozone/common/src/main/proto/OzoneManagerProtocol.proto @@ -478,8 +478,6 @@ message DataEncryptionKeyProto { message BucketArgs { required string volumeName = 1; required string bucketName = 2; - repeated OzoneAclInfo addAcls = 3; - repeated OzoneAclInfo removeAcls = 4; optional bool isVersionEnabled = 5; optional StorageTypeProto storageType = 6; repeated hadoop.hdds.KeyValue metadata = 7; diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestStorageContainerManagerHelper.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestStorageContainerManagerHelper.java index 505ee5d7ea8..bb0a3e69207 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestStorageContainerManagerHelper.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestStorageContainerManagerHelper.java @@ -44,7 +44,6 @@ import org.apache.hadoop.ozone.container.common.utils.ReferenceCountedDB; import java.io.IOException; import java.io.OutputStream; -import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.Set; @@ -82,8 +81,6 @@ public class TestStorageContainerManagerHelper { storageHandler.createVolume(createVolumeArgs); BucketArgs bucketArgs = new BucketArgs(bucket, createVolumeArgs); - bucketArgs.setAddAcls(new ArrayList<>()); - bucketArgs.setRemoveAcls(new ArrayList<>()); bucketArgs.setStorageType(StorageType.DISK); storageHandler.createBucket(bucketArgs); 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 6ed4eae0de6..ca506c63861 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 @@ -516,7 +516,9 @@ public abstract class TestOzoneRpcClientAbstract { List acls = new ArrayList<>(); acls.add(new OzoneAcl(USER, "test", ACLType.ALL, ACCESS)); OzoneBucket bucket = volume.getBucket(bucketName); - bucket.addAcls(acls); + for (OzoneAcl acl : acls) { + assertTrue(bucket.addAcls(acl)); + } OzoneBucket newBucket = volume.getBucket(bucketName); Assert.assertEquals(bucketName, newBucket.getName()); Assert.assertTrue(bucket.getAcls().contains(acls.get(0))); @@ -537,7 +539,9 @@ public abstract class TestOzoneRpcClientAbstract { builder.setAcls(acls); volume.createBucket(bucketName, builder.build()); OzoneBucket bucket = volume.getBucket(bucketName); - bucket.removeAcls(acls); + for (OzoneAcl acl : acls) { + assertTrue(bucket.removeAcls(acl)); + } OzoneBucket newBucket = volume.getBucket(bucketName); Assert.assertEquals(bucketName, newBucket.getName()); Assert.assertTrue(!bucket.getAcls().contains(acls.get(0))); @@ -590,6 +594,28 @@ public abstract class TestOzoneRpcClientAbstract { Assert.assertEquals(true, newBucket.getVersioning()); } + @Test + public void testAclsAfterCallingSetBucketProperty() throws Exception { + String volumeName = UUID.randomUUID().toString(); + String bucketName = UUID.randomUUID().toString(); + store.createVolume(volumeName); + OzoneVolume volume = store.getVolume(volumeName); + volume.createBucket(bucketName); + + OzoneBucket ozoneBucket = volume.getBucket(bucketName); + List currentAcls = ozoneBucket.getAcls(); + + ozoneBucket.setVersioning(true); + + OzoneBucket newBucket = volume.getBucket(bucketName); + Assert.assertEquals(bucketName, newBucket.getName()); + Assert.assertEquals(true, newBucket.getVersioning()); + + List aclsAfterSet = newBucket.getAcls(); + Assert.assertEquals(currentAcls, aclsAfterSet); + + } + @Test public void testSetBucketStorageType() throws IOException, OzoneException { diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestMultipleContainerReadWrite.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestMultipleContainerReadWrite.java index 08e41304866..328d1be2a11 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestMultipleContainerReadWrite.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestMultipleContainerReadWrite.java @@ -42,7 +42,6 @@ import org.junit.rules.ExpectedException; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -import java.util.LinkedList; import static org.apache.hadoop.test.MetricsAsserts.assertCounter; import static org.apache.hadoop.test.MetricsAsserts.getMetrics; @@ -104,8 +103,6 @@ public class TestMultipleContainerReadWrite { storageHandler.createVolume(createVolumeArgs); BucketArgs bucketArgs = new BucketArgs(bucketName, createVolumeArgs); - bucketArgs.setAddAcls(new LinkedList<>()); - bucketArgs.setRemoveAcls(new LinkedList<>()); bucketArgs.setStorageType(StorageType.DISK); storageHandler.createBucket(bucketArgs); @@ -148,8 +145,6 @@ public class TestMultipleContainerReadWrite { storageHandler.createVolume(createVolumeArgs); BucketArgs bucketArgs = new BucketArgs(bucketName, createVolumeArgs); - bucketArgs.setAddAcls(new LinkedList<>()); - bucketArgs.setRemoveAcls(new LinkedList<>()); bucketArgs.setStorageType(StorageType.DISK); storageHandler.createBucket(bucketArgs); @@ -183,8 +178,6 @@ public class TestMultipleContainerReadWrite { storageHandler.createVolume(createVolumeArgs); BucketArgs bucketArgs = new BucketArgs(bucketName, createVolumeArgs); - bucketArgs.setAddAcls(new LinkedList<>()); - bucketArgs.setRemoveAcls(new LinkedList<>()); bucketArgs.setStorageType(StorageType.DISK); storageHandler.createBucket(bucketArgs); diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmAcls.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmAcls.java index c4375641063..3b6389b8c82 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmAcls.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmAcls.java @@ -16,7 +16,6 @@ */ package org.apache.hadoop.ozone.om; -import java.util.LinkedList; import java.util.UUID; import org.apache.hadoop.hdds.conf.OzoneConfiguration; @@ -127,8 +126,6 @@ public class TestOmAcls { "authorized to create Ozone")); BucketArgs bucketArgs = new BucketArgs("bucket1", createVolumeArgs); - bucketArgs.setAddAcls(new LinkedList<>()); - bucketArgs.setRemoveAcls(new LinkedList<>()); bucketArgs.setStorageType(StorageType.DISK); OzoneTestUtils.expectOmException(ResultCodes.PERMISSION_DENIED, () -> storageHandler.createBucket(bucketArgs)); @@ -146,8 +143,6 @@ public class TestOmAcls { createVolumeArgs.setAdminName(adminName); createVolumeArgs.setQuota(new OzoneQuota(100, OzoneQuota.Units.GB)); BucketArgs bucketArgs = new BucketArgs("bucket1", createVolumeArgs); - bucketArgs.setAddAcls(new LinkedList<>()); - bucketArgs.setRemoveAcls(new LinkedList<>()); bucketArgs.setStorageType(StorageType.DISK); logCapturer.clearOutput(); diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmBlockVersioning.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmBlockVersioning.java index 6ec843eda56..0db78c75e14 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmBlockVersioning.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmBlockVersioning.java @@ -46,7 +46,6 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; import java.util.ArrayList; -import java.util.LinkedList; import java.util.List; import static org.junit.Assert.assertEquals; @@ -108,8 +107,6 @@ public class TestOmBlockVersioning { storageHandler.createVolume(createVolumeArgs); BucketArgs bucketArgs = new BucketArgs(bucketName, createVolumeArgs); - bucketArgs.setAddAcls(new LinkedList<>()); - bucketArgs.setRemoveAcls(new LinkedList<>()); bucketArgs.setStorageType(StorageType.DISK); storageHandler.createBucket(bucketArgs); @@ -209,8 +206,6 @@ public class TestOmBlockVersioning { storageHandler.createVolume(createVolumeArgs); BucketArgs bucketArgs = new BucketArgs(bucketName, createVolumeArgs); - bucketArgs.setAddAcls(new LinkedList<>()); - bucketArgs.setRemoveAcls(new LinkedList<>()); bucketArgs.setStorageType(StorageType.DISK); storageHandler.createBucket(bucketArgs); diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManager.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManager.java index 3b99c6f9efc..a1b023e1aec 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManager.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManager.java @@ -23,7 +23,6 @@ import java.net.InetSocketAddress; import java.nio.file.Path; import java.nio.file.Paths; import java.text.ParseException; -import java.util.LinkedList; import java.util.List; import java.util.Random; import java.util.Set; @@ -534,8 +533,6 @@ public class TestOzoneManager { storageHandler.createVolume(createVolumeArgs); BucketArgs bucketArgs = new BucketArgs(bucketName, createVolumeArgs); - bucketArgs.setAddAcls(new LinkedList<>()); - bucketArgs.setRemoveAcls(new LinkedList<>()); bucketArgs.setStorageType(StorageType.DISK); storageHandler.createBucket(bucketArgs); @@ -578,8 +575,6 @@ public class TestOzoneManager { storageHandler.createVolume(createVolumeArgs); BucketArgs bucketArgs = new BucketArgs(bucketName, createVolumeArgs); - bucketArgs.setAddAcls(new LinkedList<>()); - bucketArgs.setRemoveAcls(new LinkedList<>()); bucketArgs.setStorageType(StorageType.DISK); storageHandler.createBucket(bucketArgs); @@ -623,8 +618,6 @@ public class TestOzoneManager { storageHandler.createVolume(createVolumeArgs); BucketArgs bucketArgs = new BucketArgs(bucketName, createVolumeArgs); - bucketArgs.setAddAcls(new LinkedList<>()); - bucketArgs.setRemoveAcls(new LinkedList<>()); bucketArgs.setStorageType(StorageType.DISK); storageHandler.createBucket(bucketArgs); @@ -934,8 +927,6 @@ public class TestOzoneManager { storageHandler.createVolume(createVolumeArgs); BucketArgs bucketArgs = new BucketArgs(bucketName, createVolumeArgs); - bucketArgs.setAddAcls(new LinkedList<>()); - bucketArgs.setRemoveAcls(new LinkedList<>()); bucketArgs.setStorageType(StorageType.DISK); storageHandler.createBucket(bucketArgs); @@ -1126,8 +1117,6 @@ public class TestOzoneManager { storageHandler.createVolume(createVolumeArgs); BucketArgs bucketArgs = new BucketArgs(bucketName, createVolumeArgs); - bucketArgs.setAddAcls(new LinkedList<>()); - bucketArgs.setRemoveAcls(new LinkedList<>()); bucketArgs.setStorageType(StorageType.DISK); storageHandler.createBucket(bucketArgs); @@ -1171,8 +1160,6 @@ public class TestOzoneManager { storageHandler.createVolume(createVolumeArgs); BucketArgs bucketArgs = new BucketArgs(bucketName, createVolumeArgs); - bucketArgs.setAddAcls(new LinkedList<>()); - bucketArgs.setRemoveAcls(new LinkedList<>()); bucketArgs.setStorageType(StorageType.DISK); storageHandler.createBucket(bucketArgs); @@ -1232,8 +1219,6 @@ public class TestOzoneManager { storageHandler.createVolume(createVolumeArgs); BucketArgs bucketArgs = new BucketArgs(bucketName, createVolumeArgs); - bucketArgs.setAddAcls(new LinkedList<>()); - bucketArgs.setRemoveAcls(new LinkedList<>()); bucketArgs.setStorageType(StorageType.DISK); storageHandler.createBucket(bucketArgs); diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/web/client/TestBuckets.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/web/client/TestBuckets.java index 1578bfa3c86..1bfd640297c 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/web/client/TestBuckets.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/web/client/TestBuckets.java @@ -32,6 +32,7 @@ import org.apache.hadoop.ozone.web.request.OzoneQuota; import org.apache.hadoop.ozone.web.utils.OzoneUtils; import org.apache.hadoop.util.Time; import org.junit.AfterClass; +import org.junit.Assert; import org.junit.Before; import org.junit.BeforeClass; import org.junit.Rule; @@ -54,6 +55,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import static org.junit.Assume.assumeFalse; /** * Test Ozone Bucket Lifecycle. @@ -177,6 +179,8 @@ public class TestBuckets { @Test public void testAddBucketAcls() throws Exception { + assumeFalse("Rest Client does not support ACL", + clientProtocol.equals(RestClient.class)); runTestAddBucketAcls(client); } @@ -194,11 +198,14 @@ public class TestBuckets { String bucketName = OzoneUtils.getRequestID().toLowerCase(); vol.createBucket(bucketName); OzoneBucket bucket = vol.getBucket(bucketName); + List aclList = Arrays.stream(acls).map(acl -> OzoneAcl.parseAcl(acl)) .collect(Collectors.toList()); int numAcls = bucket.getAcls().size(); - bucket.addAcls(aclList); + for (OzoneAcl ozoneAcl : aclList) { + Assert.assertTrue(bucket.addAcls(ozoneAcl)); + } OzoneBucket updatedBucket = vol.getBucket(bucketName); assertEquals(updatedBucket.getAcls().size(), 2 + numAcls); // verify if the creation time is missing after update operation @@ -209,6 +216,8 @@ public class TestBuckets { @Test public void testRemoveBucketAcls() throws Exception { + assumeFalse("Rest Client does not support ACL", + clientProtocol.equals(RestClient.class)); runTestRemoveBucketAcls(client); } @@ -230,9 +239,13 @@ public class TestBuckets { vol.createBucket(bucketName); OzoneBucket bucket = vol.getBucket(bucketName); int numAcls = bucket.getAcls().size(); - bucket.addAcls(aclList); + for (OzoneAcl ozoneAcl : aclList) { + Assert.assertTrue(bucket.addAcls(ozoneAcl)); + } assertEquals(bucket.getAcls().size(), 2 + numAcls); - bucket.removeAcls(aclList); + for (OzoneAcl ozoneAcl : aclList) { + Assert.assertTrue(bucket.removeAcls(ozoneAcl)); + } OzoneBucket updatedBucket = vol.getBucket(bucketName); // We removed all acls diff --git a/hadoop-ozone/objectstore-service/src/main/java/org/apache/hadoop/ozone/web/handlers/BucketHandler.java b/hadoop-ozone/objectstore-service/src/main/java/org/apache/hadoop/ozone/web/handlers/BucketHandler.java index 5d26f69554f..3c73fce7781 100644 --- a/hadoop-ozone/objectstore-service/src/main/java/org/apache/hadoop/ozone/web/handlers/BucketHandler.java +++ b/hadoop-ozone/objectstore-service/src/main/java/org/apache/hadoop/ozone/web/handlers/BucketHandler.java @@ -67,7 +67,6 @@ public class BucketHandler implements Bucket { public Response doProcess(BucketArgs args) throws OzoneException, IOException { StorageHandler fs = StorageHandlerBuilder.getStorageHandler(); - getAclsFromHeaders(args, false); args.setVersioning(getVersioning(args)); args.setStorageType(getStorageType(args)); fs.createBucket(args); @@ -103,14 +102,8 @@ public class BucketHandler implements Bucket { public Response doProcess(BucketArgs args) throws OzoneException, IOException { StorageHandler fs = StorageHandlerBuilder.getStorageHandler(); - getAclsFromHeaders(args, true); args.setVersioning(getVersioning(args)); args.setStorageType(getStorageType(args)); - - if ((args.getAddAcls() != null) || (args.getRemoveAcls() != null)) { - fs.setBucketAcls(args); - } - if (args.getVersioning() != OzoneConsts.Versioning.NOT_DEFINED) { fs.setBucketVersioning(args); } diff --git a/hadoop-ozone/objectstore-service/src/main/java/org/apache/hadoop/ozone/web/handlers/BucketProcessTemplate.java b/hadoop-ozone/objectstore-service/src/main/java/org/apache/hadoop/ozone/web/handlers/BucketProcessTemplate.java index 8f68cc403f4..7dea7167497 100644 --- a/hadoop-ozone/objectstore-service/src/main/java/org/apache/hadoop/ozone/web/handlers/BucketProcessTemplate.java +++ b/hadoop-ozone/objectstore-service/src/main/java/org/apache/hadoop/ozone/web/handlers/BucketProcessTemplate.java @@ -108,35 +108,6 @@ public abstract class BucketProcessTemplate { return null; } - /** - * Reads ACLs from headers and throws appropriate exception if needed. - * - * @param args - bucketArgs - * - * @throws OzoneException - */ - void getAclsFromHeaders(BucketArgs args, boolean parseRemoveACL) - throws OzoneException { - try { - List acls = getAcls(args, Header.OZONE_ACL_REMOVE); - if (acls != null && !acls.isEmpty()) { - args.removeAcls(acls); - } - if ((!parseRemoveACL) && args.getRemoveAcls() != null) { - OzoneException ex = ErrorTable.newError(ErrorTable.MALFORMED_ACL, args); - ex.setMessage("Invalid Remove ACLs"); - throw ex; - } - - acls = getAcls(args, Header.OZONE_ACL_ADD); - if (acls != null && !acls.isEmpty()) { - args.addAcls(acls); - } - } catch (IllegalArgumentException ex) { - throw ErrorTable.newError(ErrorTable.MALFORMED_ACL, args, ex); - } - } - /** * Converts FileSystem IO exceptions to OZONE exceptions. * diff --git a/hadoop-ozone/objectstore-service/src/main/java/org/apache/hadoop/ozone/web/interfaces/StorageHandler.java b/hadoop-ozone/objectstore-service/src/main/java/org/apache/hadoop/ozone/web/interfaces/StorageHandler.java index 836c03ab0e6..91481d227b7 100644 --- a/hadoop-ozone/objectstore-service/src/main/java/org/apache/hadoop/ozone/web/interfaces/StorageHandler.java +++ b/hadoop-ozone/objectstore-service/src/main/java/org/apache/hadoop/ozone/web/interfaces/StorageHandler.java @@ -143,15 +143,6 @@ public interface StorageHandler extends Closeable{ */ void createBucket(BucketArgs args) throws IOException, OzoneException; - /** - * Adds or Removes ACLs from a Bucket. - * - * @param args - BucketArgs - * - * @throws IOException - */ - void setBucketAcls(BucketArgs args) throws IOException, OzoneException; - /** * Enables or disables Bucket Versioning. * diff --git a/hadoop-ozone/objectstore-service/src/main/java/org/apache/hadoop/ozone/web/storage/DistributedStorageHandler.java b/hadoop-ozone/objectstore-service/src/main/java/org/apache/hadoop/ozone/web/storage/DistributedStorageHandler.java index ea017e1de2d..fbbd3bed85c 100644 --- a/hadoop-ozone/objectstore-service/src/main/java/org/apache/hadoop/ozone/web/storage/DistributedStorageHandler.java +++ b/hadoop-ozone/objectstore-service/src/main/java/org/apache/hadoop/ozone/web/storage/DistributedStorageHandler.java @@ -291,9 +291,6 @@ public final class DistributedStorageHandler implements StorageHandler { OmBucketInfo.Builder builder = OmBucketInfo.newBuilder(); builder.setVolumeName(args.getVolumeName()) .setBucketName(args.getBucketName()); - if(args.getAddAcls() != null) { - builder.setAcls(args.getAddAcls()); - } if(args.getStorageType() != null) { builder.setStorageType(args.getStorageType()); } @@ -325,25 +322,6 @@ public final class DistributedStorageHandler implements StorageHandler { return false; } - @Override - public void setBucketAcls(BucketArgs args) - throws IOException, OzoneException { - List removeAcls = args.getRemoveAcls(); - List addAcls = args.getAddAcls(); - if(removeAcls != null || addAcls != null) { - OmBucketArgs.Builder builder = OmBucketArgs.newBuilder(); - builder.setVolumeName(args.getVolumeName()) - .setBucketName(args.getBucketName()); - if(removeAcls != null && !removeAcls.isEmpty()) { - builder.setRemoveAcls(args.getRemoveAcls()); - } - if(addAcls != null && !addAcls.isEmpty()) { - builder.setAddAcls(args.getAddAcls()); - } - ozoneManagerClient.setBucketProperty(builder.build()); - } - } - @Override public void setBucketVersioning(BucketArgs args) throws IOException, OzoneException { 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 530280b21cb..8837c2d1e57 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 @@ -272,16 +272,6 @@ public class BucketManagerImpl implements BucketManager { .setBucketName(oldBucketInfo.getBucketName()); bucketInfoBuilder.addAllMetadata(args.getMetadata()); - //Check ACLs to update - if (args.getAddAcls() != null || args.getRemoveAcls() != null) { - bucketInfoBuilder.setAcls(getUpdatedAclList(oldBucketInfo.getAcls(), - args.getRemoveAcls(), args.getAddAcls())); - LOG.debug("Updating ACLs for bucket: {} in volume: {}", - bucketName, volumeName); - } else { - bucketInfoBuilder.setAcls(oldBucketInfo.getAcls()); - } - //Check StorageType to update StorageType storageType = args.getStorageType(); if (storageType != null) { @@ -304,7 +294,14 @@ public class BucketManagerImpl implements BucketManager { } bucketInfoBuilder.setCreationTime(oldBucketInfo.getCreationTime()); + // Set acls from oldBucketInfo if it has any. + if (oldBucketInfo.getAcls() != null) { + bucketInfoBuilder.setAcls(oldBucketInfo.getAcls()); + } + OmBucketInfo omBucketInfo = bucketInfoBuilder.build(); + + commitBucketInfoToDB(omBucketInfo); } catch (IOException | DBException ex) { if (!(ex instanceof OMException)) { @@ -318,27 +315,6 @@ public class BucketManagerImpl implements BucketManager { } } - /** - * Updates the existing ACL list with remove and add ACLs that are passed. - * Remove is done before Add. - * - * @param existingAcls - old ACL list. - * @param removeAcls - ACLs to be removed. - * @param addAcls - ACLs to be added. - * @return updated ACL list. - */ - private List getUpdatedAclList(List existingAcls, - List removeAcls, List addAcls) { - if (removeAcls != null && !removeAcls.isEmpty()) { - existingAcls.removeAll(removeAcls); - } - if (addAcls != null && !addAcls.isEmpty()) { - addAcls.stream().filter(acl -> !existingAcls.contains(acl)).forEach( - existingAcls::add); - } - return existingAcls; - } - /** * Deletes an existing empty bucket from volume. * diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetPropertyRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetPropertyRequest.java index ada5c04c8e4..85e80818b4f 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetPropertyRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetPropertyRequest.java @@ -19,7 +19,6 @@ package org.apache.hadoop.ozone.om.request.bucket; import java.io.IOException; -import java.util.List; import com.google.common.base.Optional; import com.google.common.base.Preconditions; @@ -35,7 +34,6 @@ import org.apache.hadoop.ozone.security.acl.OzoneObj; import org.apache.hadoop.ozone.om.request.OMClientRequest; import org.apache.hadoop.hdds.protocol.StorageType; -import org.apache.hadoop.ozone.OzoneAcl; import org.apache.hadoop.ozone.om.OMMetadataManager; import org.apache.hadoop.ozone.om.OMMetrics; import org.apache.hadoop.ozone.om.OzoneManager; @@ -134,17 +132,6 @@ public class OMBucketSetPropertyRequest extends OMClientRequest { bucketInfoBuilder.addAllMetadata(KeyValueUtil .getFromProtobuf(bucketArgs.getMetadataList())); - //Check ACLs to update - if (omBucketArgs.getAddAcls() != null || - omBucketArgs.getRemoveAcls() != null) { - bucketInfoBuilder.setAcls(getUpdatedAclList(oldBucketInfo.getAcls(), - omBucketArgs.getRemoveAcls(), omBucketArgs.getAddAcls())); - LOG.debug("Updating ACLs for bucket: {} in volume: {}", - bucketName, volumeName); - } else { - bucketInfoBuilder.setAcls(oldBucketInfo.getAcls()); - } - //Check StorageType to update StorageType storageType = omBucketArgs.getStorageType(); if (storageType != null) { @@ -165,8 +152,14 @@ public class OMBucketSetPropertyRequest extends OMClientRequest { bucketInfoBuilder .setIsVersionEnabled(oldBucketInfo.getIsVersionEnabled()); } + bucketInfoBuilder.setCreationTime(oldBucketInfo.getCreationTime()); + // Set acls from oldBucketInfo if it has any. + if (oldBucketInfo.getAcls() != null) { + bucketInfoBuilder.setAcls(oldBucketInfo.getAcls()); + } + omBucketInfo = bucketInfoBuilder.build(); // Update table cache. @@ -210,25 +203,4 @@ public class OMBucketSetPropertyRequest extends OMClientRequest { return omClientResponse; } } - - /** - * Updates the existing ACL list with remove and add ACLs that are passed. - * Remove is done before Add. - * - * @param existingAcls - old ACL list. - * @param removeAcls - ACLs to be removed. - * @param addAcls - ACLs to be added. - * @return updated ACL list. - */ - private List< OzoneAcl > getUpdatedAclList(List existingAcls, - List removeAcls, List addAcls) { - if (removeAcls != null && !removeAcls.isEmpty()) { - existingAcls.removeAll(removeAcls); - } - if (addAcls != null && !addAcls.isEmpty()) { - addAcls.stream().filter(acl -> !existingAcls.contains(acl)).forEach( - existingAcls::add); - } - return existingAcls; - } } diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestBucketManagerImpl.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestBucketManagerImpl.java index 4d652c34edd..d4e6eb70f0f 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestBucketManagerImpl.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestBucketManagerImpl.java @@ -18,8 +18,6 @@ package org.apache.hadoop.ozone.om; import java.io.File; import java.io.IOException; -import java.util.LinkedList; -import java.util.List; import org.apache.hadoop.crypto.key.KeyProvider; import org.apache.hadoop.crypto.key.KeyProviderCryptoExtension; @@ -28,7 +26,6 @@ import org.apache.hadoop.hdds.protocol.StorageType; import org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationFactor; import org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationType; import org.apache.hadoop.hdds.server.ServerUtils; -import org.apache.hadoop.ozone.OzoneAcl; import org.apache.hadoop.ozone.om.exceptions.OMException; import org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes; import org.apache.hadoop.ozone.om.helpers.*; @@ -42,9 +39,6 @@ import org.junit.runner.RunWith; import org.mockito.Mockito; import org.mockito.runners.MockitoJUnitRunner; -import static org.apache.hadoop.ozone.OzoneAcl.AclScope.ACCESS; -import static org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.*; - /** * Tests BucketManagerImpl, mocks OMMetadataManager for testing. */ @@ -215,83 +209,6 @@ public class TestBucketManagerImpl { metaMgr.getStore().close(); } - @Test - public void testSetBucketPropertyAddACL() throws Exception { - OmMetadataManagerImpl metaMgr = createSampleVol(); - - List acls = new LinkedList<>(); - OzoneAcl ozoneAcl = new OzoneAcl(ACLIdentityType.USER, - "root", ACLType.READ, ACCESS); - acls.add(ozoneAcl); - BucketManager bucketManager = new BucketManagerImpl(metaMgr); - OmBucketInfo bucketInfo = OmBucketInfo.newBuilder() - .setVolumeName("sampleVol") - .setBucketName("bucketOne") - .setAcls(acls) - .setStorageType(StorageType.DISK) - .setIsVersionEnabled(false) - .build(); - bucketManager.createBucket(bucketInfo); - OmBucketInfo result = bucketManager.getBucketInfo( - "sampleVol", "bucketOne"); - Assert.assertEquals("sampleVol", result.getVolumeName()); - Assert.assertEquals("bucketOne", result.getBucketName()); - Assert.assertEquals(1, result.getAcls().size()); - List addAcls = new LinkedList<>(); - OzoneAcl newAcl = new OzoneAcl(ACLIdentityType.USER, - "ozone", ACLType.READ, ACCESS); - addAcls.add(newAcl); - OmBucketArgs bucketArgs = OmBucketArgs.newBuilder() - .setVolumeName("sampleVol") - .setBucketName("bucketOne") - .setAddAcls(addAcls) - .build(); - bucketManager.setBucketProperty(bucketArgs); - OmBucketInfo updatedResult = bucketManager.getBucketInfo( - "sampleVol", "bucketOne"); - Assert.assertEquals(2, updatedResult.getAcls().size()); - Assert.assertTrue(updatedResult.getAcls().contains(newAcl)); - metaMgr.getStore().close(); - } - - @Test - public void testSetBucketPropertyRemoveACL() throws Exception { - OmMetadataManagerImpl metaMgr = createSampleVol(); - - List acls = new LinkedList<>(); - OzoneAcl aclOne = new OzoneAcl(ACLIdentityType.USER, - "root", ACLType.READ, ACCESS); - OzoneAcl aclTwo = new OzoneAcl(ACLIdentityType.USER, - "ozone", ACLType.READ, ACCESS); - acls.add(aclOne); - acls.add(aclTwo); - BucketManager bucketManager = new BucketManagerImpl(metaMgr); - OmBucketInfo bucketInfo = OmBucketInfo.newBuilder() - .setVolumeName("sampleVol") - .setBucketName("bucketOne") - .setAcls(acls) - .setStorageType(StorageType.DISK) - .setIsVersionEnabled(false) - .build(); - bucketManager.createBucket(bucketInfo); - OmBucketInfo result = bucketManager.getBucketInfo( - "sampleVol", "bucketOne"); - Assert.assertEquals(2, result.getAcls().size()); - List removeAcls = new LinkedList<>(); - removeAcls.add(aclTwo); - OmBucketArgs bucketArgs = OmBucketArgs.newBuilder() - .setVolumeName("sampleVol") - .setBucketName("bucketOne") - .setRemoveAcls(removeAcls) - .build(); - bucketManager.setBucketProperty(bucketArgs); - OmBucketInfo updatedResult = bucketManager.getBucketInfo( - "sampleVol", "bucketOne"); - Assert.assertEquals(1, updatedResult.getAcls().size()); - Assert.assertFalse(updatedResult.getAcls().contains(aclTwo)); - metaMgr.getStore().close(); - } - @Test public void testSetBucketPropertyChangeStorageType() throws Exception { OmMetadataManagerImpl metaMgr = createSampleVol(); diff --git a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/OzoneBucketStub.java b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/OzoneBucketStub.java index 399504c57eb..9f962668f56 100644 --- a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/OzoneBucketStub.java +++ b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/OzoneBucketStub.java @@ -35,7 +35,6 @@ import org.apache.commons.codec.digest.DigestUtils; import org.apache.hadoop.hdds.client.ReplicationFactor; import org.apache.hadoop.hdds.client.ReplicationType; import org.apache.hadoop.hdds.protocol.StorageType; -import org.apache.hadoop.ozone.OzoneAcl; import org.apache.hadoop.ozone.client.io.OzoneInputStream; import org.apache.hadoop.ozone.client.io.OzoneOutputStream; import org.apache.hadoop.ozone.client.OzoneMultipartUploadPartListParts.PartInfo; @@ -63,7 +62,6 @@ public class OzoneBucketStub extends OzoneBucket { * * @param volumeName Name of the volume the bucket belongs to. * @param bucketName Name of the bucket. - * @param acls ACLs associated with the bucket. * @param storageType StorageType of the bucket. * @param versioning versioning status of the bucket. * @param creationTime creation time of the bucket. @@ -71,14 +69,12 @@ public class OzoneBucketStub extends OzoneBucket { public OzoneBucketStub( String volumeName, String bucketName, - List acls, StorageType storageType, Boolean versioning, long creationTime) { super(volumeName, bucketName, ReplicationFactor.ONE, ReplicationType.STAND_ALONE, - acls, storageType, versioning, creationTime); diff --git a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/OzoneVolumeStub.java b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/OzoneVolumeStub.java index c7317a4771f..89972601e9b 100644 --- a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/OzoneVolumeStub.java +++ b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/OzoneVolumeStub.java @@ -56,7 +56,6 @@ public class OzoneVolumeStub extends OzoneVolume { buckets.put(bucketName, new OzoneBucketStub( getName(), bucketName, - bucketArgs.getAcls(), bucketArgs.getStorageType(), bucketArgs.getVersioning(), System.currentTimeMillis()));