HDDS-1913. Fix OzoneBucket and RpcClient APIS for acl. (#1257)

(cherry picked from commit a46ba03d150f9376a13ef91c818a5779ce9cfb4e)

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<OzoneAcl> 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<OzoneAcl> acls, StorageType storageType,
-                     Boolean versioning, long creationTime,
-                     Map<String, String> 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<String, String> 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<OzoneAcl> acls, StorageType storageType,
-                     Boolean versioning, long creationTime,
-                     Map<String, String> 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<String, String> 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<OzoneAcl> 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<OzoneAcl> getAcls() {
-    return acls;
+  public List<OzoneAcl> 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<OzoneAcl> 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<OzoneAcl> removeAcls) throws IOException {
-    proxy.removeBucketAcls(volumeName, name, removeAcls);
-    acls.removeAll(removeAcls);
+  public boolean removeAcls(OzoneAcl removeAcl) throws IOException {
+    return proxy.removeAcl(ozoneObj, removeAcl);
   }
 
   /**

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());

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<OzoneAcl> 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<OzoneAcl> removeAcls)
-      throws IOException;
-
 
   /**
    * Enables or disables Bucket Versioning.

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<OzoneAcl> 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<OzoneAcl> 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);

hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java

@@ -384,7 +384,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
@@ -438,32 +440,6 @@ public class RpcClient implements ClientProtocol {
         userRights, groupRights);
   }
 
-  @Override
-  public void addBucketAcls(
-      String volumeName, String bucketName, List<OzoneAcl> 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<OzoneAcl> 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.
    *
@@ -582,7 +558,6 @@ public class RpcClient implements ClientProtocol {
         this,
         bucketInfo.getVolumeName(),
         bucketInfo.getBucketName(),
-        bucketInfo.getAcls(),
         bucketInfo.getStorageType(),
         bucketInfo.getIsVersionEnabled(),
         bucketInfo.getCreationTime(),
@@ -603,7 +578,6 @@ public class RpcClient implements ClientProtocol {
         this,
         bucket.getVolumeName(),
         bucket.getBucketName(),
-        bucket.getAcls(),
         bucket.getStorageType(),
         bucket.getIsVersionEnabled(),
         bucket.getCreationTime(),
@@ -788,7 +762,6 @@ public class RpcClient implements ClientProtocol {
         this,
         bucket.getVolumeName(),
         bucket.getBucketName(),
-        bucket.getAcls(),
         bucket.getStorageType(),
         bucket.getIsVersionEnabled(),
         bucket.getCreationTime(),

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<OzoneAcl> addAcls;
-  /**
-   * ACL's that are to be removed from the bucket.
-   */
-  private List<OzoneAcl> 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<OzoneAcl> addAcls, List<OzoneAcl> removeAcls,
       Boolean isVersionEnabled, StorageType storageType,
       Map<String, String> 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<OzoneAclInfo>}
-   */
-  public List<OzoneAcl> getAddAcls() {
-    return addAcls;
-  }
-
-  /**
-   * Returns the ACL's that are to be removed.
-   * @return {@literal List<OzoneAclInfo>}
-   */
-  public List<OzoneAcl> 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<String, String> 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<OzoneAcl> addAcls;
-    private List<OzoneAcl> removeAcls;
     private Boolean isVersionEnabled;
     private StorageType storageType;
     private Map<String, String> metadata;
@@ -180,16 +140,6 @@ public final class OmBucketArgs extends WithMetadata implements Auditable {
       return this;
     }
 
-    public Builder setAddAcls(List<OzoneAcl> acls) {
-      this.addAcls = acls;
-      return this;
-    }
-
-    public Builder setRemoveAcls(List<OzoneAcl> 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(

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<OzoneAcl> addAcls;
-  private List<OzoneAcl> 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<OzoneAcl> getAddAcls() {
-    return addAcls;
-  }
-
-  /**
-   * Set Additive ACLs.
-   *
-   * @param acl - ACL
-   */
-  public void setAddAcls(List<OzoneAcl> acl) {
-    this.addAcls = acl;
-  }
-
-  /**
-   * Returns remove ACLs for the Bucket if specified.
-   *
-   * @return acls
-   */
-  public List<OzoneAcl> getRemoveAcls() {
-    return removeAcls;
-  }
-
-  /**
-   * Takes an ACL and sets the ACL object to ACL represented by the String.
-   *
-   * @param aclString - aclString
-   */
-  public void addAcls(List<String> 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<String> 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<OzoneAcl> acl) {
-    this.removeAcls = acl;
-  }
-
-
   /**
    * Returns Versioning Info.
    *

hadoop-ozone/common/src/main/proto/OzoneManagerProtocol.proto

@@ -464,8 +464,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;

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);
 

hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java

@@ -518,7 +518,9 @@ public abstract class TestOzoneRpcClientAbstract {
     List<OzoneAcl> 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)));
@@ -539,7 +541,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)));
@@ -592,6 +596,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<OzoneAcl> currentAcls = ozoneBucket.getAcls();
+
+    ozoneBucket.setVersioning(true);
+
+    OzoneBucket newBucket = volume.getBucket(bucketName);
+    Assert.assertEquals(bucketName, newBucket.getName());
+    Assert.assertEquals(true, newBucket.getVersioning());
+
+    List<OzoneAcl> aclsAfterSet = newBucket.getAcls();
+    Assert.assertEquals(currentAcls, aclsAfterSet);
+
+  }
+
   @Test
   public void testSetBucketStorageType()
       throws IOException, OzoneException {

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);
 

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;
@@ -128,8 +127,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));
@@ -147,8 +144,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();
 

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);
 

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;
@@ -532,8 +531,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);
 
@@ -576,8 +573,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);
 
@@ -621,8 +616,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);
 
@@ -932,8 +925,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);
 
@@ -1124,8 +1115,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);
 
@@ -1169,8 +1158,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);
 
@@ -1230,8 +1217,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);
 

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<OzoneAcl> 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

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);
         }

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<String> 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.
    *

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.
    *

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<OzoneAcl> removeAcls = args.getRemoveAcls();
-    List<OzoneAcl> 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 {

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<OzoneAcl> getUpdatedAclList(List<OzoneAcl> existingAcls,
-      List<OzoneAcl> removeAcls, List<OzoneAcl> 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.
    *

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;
@@ -34,7 +33,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;
@@ -142,17 +140,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) {
@@ -173,8 +160,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.
@@ -208,25 +201,4 @@ public class OMBucketSetPropertyRequest extends OMClientRequest {
           createErrorOMResponse(omResponse, exception));
     }
   }
-
-  /**
-   * 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<OzoneAcl> existingAcls,
-      List<OzoneAcl> removeAcls, List<OzoneAcl> 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;
-  }
 }

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<OzoneAcl> 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<OzoneAcl> 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<OzoneAcl> 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<OzoneAcl> 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();

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<OzoneAcl> acls,
       StorageType storageType, Boolean versioning,
       long creationTime) {
     super(volumeName,
         bucketName,
         ReplicationFactor.ONE,
         ReplicationType.STAND_ALONE,
-        acls,
         storageType,
         versioning,
         creationTime);

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()));