You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2020/12/15 08:29:43 UTC

[GitHub] [ozone] ChenSammi opened a new pull request #1701: HDDS-4585. Support bucket acl operation in S3g

ChenSammi opened a new pull request #1701:
URL: https://github.com/apache/ozone/pull/1701


   https://issues.apache.org/jira/browse/HDDS-4585 
   
   
   Design document:
   https://issues.apache.org/jira/browse/HDDS-4550


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] ChenSammi commented on a change in pull request #1701: HDDS-4585. Support bucket acl operation in S3g

Posted by GitBox <gi...@apache.org>.
ChenSammi commented on a change in pull request #1701:
URL: https://github.com/apache/ozone/pull/1701#discussion_r565897304



##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneVolume.java
##########
@@ -261,7 +270,37 @@ public Instant getModificationTime() {
    * @return aclMap
    */
   public List<OzoneAcl> getAcls() {
-    return acls;
+    return (ArrayList)((ArrayList)acls).clone();
+  }
+
+   /**
+   * Adds ACLs to the volume.
+   * @param addAcl ACL to be added
+   * @return true - if acl is successfully added, false if acl already exists
+   * for the bucket.
+   * @throws IOException
+   */
+  public boolean addAcl(OzoneAcl addAcl) throws IOException {

Review comment:
       Yes.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] ChenSammi commented on a change in pull request #1701: HDDS-4585. Support bucket acl operation in S3g

Posted by GitBox <gi...@apache.org>.
ChenSammi commented on a change in pull request #1701:
URL: https://github.com/apache/ozone/pull/1701#discussion_r565906482



##########
File path: hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/S3Acl.java
##########
@@ -0,0 +1,388 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+package org.apache.hadoop.ozone.s3.endpoint;
+
+import org.apache.hadoop.ozone.OzoneAcl;
+import org.apache.hadoop.ozone.s3.endpoint.S3BucketAcl.Grant;
+import org.apache.hadoop.ozone.s3.endpoint.S3BucketAcl.Grantee;
+import org.apache.hadoop.ozone.s3.exception.OS3Exception;
+import org.apache.hadoop.ozone.s3.exception.S3ErrorTable;
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.ArrayList;
+import java.util.BitSet;
+import java.util.List;
+
+import static org.apache.hadoop.ozone.s3.exception.S3ErrorTable.INVALID_ARGUMENT;
+import static org.apache.hadoop.ozone.s3.exception.S3ErrorTable.NOT_IMPLEMENTED;
+
+public final class S3Acl {
+  private static final Logger LOG = LoggerFactory.getLogger(S3Acl.class);
+
+  // ACL put related headers
+  public static final String grantRead = "x-amz-grant-read";
+  public static final String grantWrite = "x-amz-grant-write";
+  public static final String grantReadACP = "x-amz-grant-read-acp";
+  public static final String grantWriteACP = "x-amz-grant-write-acp";
+  public static final String grantFullControl = "x-amz-grant-full-control";
+
+  // Not supported headers at current stage, may support it in future
+  public static final String cannedAclHeader = "x-amz-acl";
+
+  /**
+   * https://docs.aws.amazon.com/AmazonS3/latest/dev/acl-overview.html.
+   */
+  enum ACLType {
+    // Allows grantee to list the objects in the bucket
+    READ("READ"),
+    // Allows grantee to create, overwrite, and delete any object in the bucket
+    WRITE("WRITE"),
+    // Allows grantee to write the bucket ACL
+    READ_ACP("READ_ACP"),
+    // Allows grantee to write the ACL for the applicable bucket
+    WRITE_ACP("WRITE_ACP"),
+    // Allows grantee above all permissions on the bucket
+    FULL_CONTROL("FULL_CONTROL");
+
+    public String getValue() {
+      return value;
+    }
+    /**
+     * String value for this Enum.
+     */
+    private final String value;
+
+    /**
+     * @param val String type for this enum.
+     */
+    ACLType(String val) {
+      value = val;
+    }
+
+
+    public static ACLType getType(String typeStr) {
+      for(ACLType type: ACLType.values()) {
+        if (type.getValue().equals(typeStr)) {
+          return type;
+        }
+      }
+      return null;
+    }
+  }
+
+  enum ACLIdentityType {
+    USER("CanonicalUser", true, "id"),
+    GROUP("Group", false, "url"),
+    USER_BY_EMAIL("AmazonCustomerByEmail", false, "emailAddress");
+
+    public String getGranteeType() {
+      return granteeType;
+    }
+
+    public String getHeaderType() {
+      return granteeInHeader;
+    }
+
+    /**
+     *  Grantee type in body XML.
+     */
+    private final String granteeType;
+
+    /**
+     * Is this type supported or not.
+     */
+    private final boolean supported;
+
+    /**
+     * Grantee type in header.
+     */
+    private final String granteeInHeader;
+
+    /**
+     * Init OzoneACLtypes enum.
+     *
+     * @param val String type for this enum.
+     */
+    ACLIdentityType(String val, boolean support, String headerType) {
+      granteeType = val;
+      supported = support;
+      granteeInHeader = headerType;
+    }
+
+    boolean isSupported() {
+      return supported;
+    }
+
+    public static ACLIdentityType getTypeFromGranteeType(String typeStr) {
+      for(ACLIdentityType type: ACLIdentityType.values()) {
+        if (type.getGranteeType().equals(typeStr)) {
+          return type;
+        }
+      }
+      return null;
+    }
+
+    public static ACLIdentityType getTypeFromHeaderType(String typeStr) {
+      for(ACLIdentityType type: ACLIdentityType.values()) {
+        if (type.getHeaderType().equals(typeStr)) {
+          return type;
+        }
+      }
+      return null;
+    }
+  }
+
+  public static boolean isGranteeTypeSupported(String typeStr) {
+    ACLIdentityType type =  ACLIdentityType.getTypeFromGranteeType(typeStr);
+    return type == null ? false : type.isSupported();
+  }
+
+  public static boolean isHeaderTypeSupported(String typeStr) {
+    ACLIdentityType type =  ACLIdentityType.getTypeFromHeaderType(typeStr);
+    return type == null ? false : type.isSupported();
+  }
+
+  public static List<Grant> ozoneNativeAclToS3Acl(OzoneAcl ozoneAcl) {
+    // Since currently only "CanonicalUser" is supported, which maps to Ozone
+    // "USER"
+    List<Grant> grantList = new ArrayList<>();
+    if (ozoneAcl.getType() != IAccessAuthorizer.ACLIdentityType.USER) {
+      return grantList;
+    }
+
+    Grantee grantee = new Grantee();
+    grantee.setDisplayName(ozoneAcl.getName());
+    grantee.setId(ozoneAcl.getName());
+
+    List<IAccessAuthorizer.ACLType> acls = ozoneAcl.getAclList();
+    if (acls.contains(IAccessAuthorizer.ACLType.ALL)) {
+      Grant grant = new Grant();
+      grant.setGrantee(grantee);
+      grant.setPermission(ACLType.FULL_CONTROL.toString());
+      grantList.add(grant);
+      return grantList;
+    } else if (acls.contains(IAccessAuthorizer.ACLType.WRITE_ACL)) {

Review comment:
       If I understand your question correctly,  when else is removed, then we will have a grant list with both FULL_CONTROL and WRITE_ACL grant for the same user, which is kind of information duplicate.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] ChenSammi commented on pull request #1701: HDDS-4585. Support bucket acl operation in S3g

Posted by GitBox <gi...@apache.org>.
ChenSammi commented on pull request #1701:
URL: https://github.com/apache/ozone/pull/1701#issuecomment-805565913


   Hey,@xiaoyuyao do you have time to take another look? 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] xiaoyuyao commented on a change in pull request #1701: HDDS-4585. Support bucket acl operation in S3g

Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #1701:
URL: https://github.com/apache/ozone/pull/1701#discussion_r562380251



##########
File path: hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/S3Acl.java
##########
@@ -0,0 +1,388 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+package org.apache.hadoop.ozone.s3.endpoint;
+
+import org.apache.hadoop.ozone.OzoneAcl;
+import org.apache.hadoop.ozone.s3.endpoint.S3BucketAcl.Grant;
+import org.apache.hadoop.ozone.s3.endpoint.S3BucketAcl.Grantee;
+import org.apache.hadoop.ozone.s3.exception.OS3Exception;
+import org.apache.hadoop.ozone.s3.exception.S3ErrorTable;
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.ArrayList;
+import java.util.BitSet;
+import java.util.List;
+
+import static org.apache.hadoop.ozone.s3.exception.S3ErrorTable.INVALID_ARGUMENT;
+import static org.apache.hadoop.ozone.s3.exception.S3ErrorTable.NOT_IMPLEMENTED;
+
+public final class S3Acl {
+  private static final Logger LOG = LoggerFactory.getLogger(S3Acl.class);
+
+  // ACL put related headers
+  public static final String grantRead = "x-amz-grant-read";
+  public static final String grantWrite = "x-amz-grant-write";
+  public static final String grantReadACP = "x-amz-grant-read-acp";
+  public static final String grantWriteACP = "x-amz-grant-write-acp";
+  public static final String grantFullControl = "x-amz-grant-full-control";
+
+  // Not supported headers at current stage, may support it in future
+  public static final String cannedAclHeader = "x-amz-acl";
+
+  /**
+   * https://docs.aws.amazon.com/AmazonS3/latest/dev/acl-overview.html.
+   */
+  enum ACLType {
+    // Allows grantee to list the objects in the bucket
+    READ("READ"),
+    // Allows grantee to create, overwrite, and delete any object in the bucket
+    WRITE("WRITE"),
+    // Allows grantee to write the bucket ACL
+    READ_ACP("READ_ACP"),
+    // Allows grantee to write the ACL for the applicable bucket
+    WRITE_ACP("WRITE_ACP"),
+    // Allows grantee above all permissions on the bucket
+    FULL_CONTROL("FULL_CONTROL");
+
+    public String getValue() {
+      return value;
+    }
+    /**
+     * String value for this Enum.
+     */
+    private final String value;
+
+    /**
+     * @param val String type for this enum.
+     */
+    ACLType(String val) {
+      value = val;
+    }
+
+
+    public static ACLType getType(String typeStr) {
+      for(ACLType type: ACLType.values()) {
+        if (type.getValue().equals(typeStr)) {
+          return type;
+        }
+      }
+      return null;
+    }
+  }
+
+  enum ACLIdentityType {
+    USER("CanonicalUser", true, "id"),
+    GROUP("Group", false, "url"),
+    USER_BY_EMAIL("AmazonCustomerByEmail", false, "emailAddress");
+
+    public String getGranteeType() {
+      return granteeType;
+    }
+
+    public String getHeaderType() {
+      return granteeInHeader;
+    }
+
+    /**
+     *  Grantee type in body XML.
+     */
+    private final String granteeType;
+
+    /**
+     * Is this type supported or not.
+     */
+    private final boolean supported;
+
+    /**
+     * Grantee type in header.
+     */
+    private final String granteeInHeader;
+
+    /**
+     * Init OzoneACLtypes enum.
+     *
+     * @param val String type for this enum.
+     */
+    ACLIdentityType(String val, boolean support, String headerType) {
+      granteeType = val;
+      supported = support;
+      granteeInHeader = headerType;
+    }
+
+    boolean isSupported() {
+      return supported;
+    }
+
+    public static ACLIdentityType getTypeFromGranteeType(String typeStr) {
+      for(ACLIdentityType type: ACLIdentityType.values()) {
+        if (type.getGranteeType().equals(typeStr)) {
+          return type;
+        }
+      }
+      return null;
+    }
+
+    public static ACLIdentityType getTypeFromHeaderType(String typeStr) {
+      for(ACLIdentityType type: ACLIdentityType.values()) {
+        if (type.getHeaderType().equals(typeStr)) {
+          return type;
+        }
+      }
+      return null;
+    }
+  }
+
+  public static boolean isGranteeTypeSupported(String typeStr) {
+    ACLIdentityType type =  ACLIdentityType.getTypeFromGranteeType(typeStr);
+    return type == null ? false : type.isSupported();
+  }
+
+  public static boolean isHeaderTypeSupported(String typeStr) {
+    ACLIdentityType type =  ACLIdentityType.getTypeFromHeaderType(typeStr);
+    return type == null ? false : type.isSupported();
+  }
+
+  public static List<Grant> ozoneNativeAclToS3Acl(OzoneAcl ozoneAcl) {
+    // Since currently only "CanonicalUser" is supported, which maps to Ozone
+    // "USER"
+    List<Grant> grantList = new ArrayList<>();
+    if (ozoneAcl.getType() != IAccessAuthorizer.ACLIdentityType.USER) {
+      return grantList;
+    }
+
+    Grantee grantee = new Grantee();
+    grantee.setDisplayName(ozoneAcl.getName());
+    grantee.setId(ozoneAcl.getName());
+
+    List<IAccessAuthorizer.ACLType> acls = ozoneAcl.getAclList();
+    if (acls.contains(IAccessAuthorizer.ACLType.ALL)) {
+      Grant grant = new Grant();
+      grant.setGrantee(grantee);
+      grant.setPermission(ACLType.FULL_CONTROL.toString());
+      grantList.add(grant);
+      return grantList;
+    } else if (acls.contains(IAccessAuthorizer.ACLType.WRITE_ACL)) {

Review comment:
       should we allow other combinations here? such as READ+WRITE?  I feel if we remove "else" here, the 1-1 mapping between Ozone AclType to the S3 ACLType can be added. 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] xiaoyuyao commented on a change in pull request #1701: HDDS-4585. Support bucket acl operation in S3g

Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #1701:
URL: https://github.com/apache/ozone/pull/1701#discussion_r612649256



##########
File path: hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java
##########
@@ -353,6 +373,204 @@ public MultiDeleteResponse multiDelete(@PathParam("bucket") String bucketName,
     return result;
   }
 
+  /**
+   * Implement acl get.
+   * <p>
+   * see: https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketAcl.html
+   */
+  public S3BucketAcl getAcl(String bucketName)
+      throws OS3Exception, IOException {
+    S3BucketAcl result = new S3BucketAcl();
+    try {
+      OzoneBucket bucket = getBucket(bucketName);
+      OzoneVolume volume = getVolume();
+      // TODO: use bucket owner instead of volume owner here once bucket owner
+      // TODO: is supported.
+      S3Owner owner = new S3Owner(volume.getOwner(), volume.getOwner());
+      result.setOwner(owner);
+
+      // TODO: remove this duplication avoid logic when ACCESS and DEFAULT scope
+      // TODO: are merged.
+      // Use set to remove ACLs with different scopes(ACCESS and DEFAULT)
+      Set<Grant> grantSet = new HashSet<>();
+      // Return ACL list
+      for (OzoneAcl acl : bucket.getAcls()) {
+        List<Grant> grants = S3Acl.ozoneNativeAclToS3Acl(acl);
+        grantSet.addAll(grants);
+      }
+      ArrayList<Grant> grantList = new ArrayList<>();
+      grantList.addAll(grantSet);
+      result.setAclList(
+          new S3BucketAcl.AccessControlList(grantList));
+      return result;
+    } catch (OMException ex) {
+      if (ex.getResult() == ResultCodes.BUCKET_NOT_FOUND) {
+        throw S3ErrorTable.newError(S3ErrorTable
+            .NO_SUCH_BUCKET, bucketName);
+      } else if (ex.getResult() == ResultCodes.PERMISSION_DENIED) {
+        throw S3ErrorTable.newError(S3ErrorTable
+            .ACCESS_DENIED, bucketName);
+      } else {
+        LOG.error("Failed to get acl of Bucket " + bucketName, ex);
+        throw S3ErrorTable.newError(S3ErrorTable.INTERNAL_ERROR, bucketName);
+      }
+    }
+  }
+
+  /**
+   * Implement acl put.
+   * <p>
+   * see: https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutBucketAcl.html
+   */
+  public Response putAcl(String bucketName, HttpHeaders httpHeaders,
+      InputStream body) throws IOException, OS3Exception {
+    String grantReads = httpHeaders.getHeaderString(S3Acl.grantRead);
+    String grantWrites = httpHeaders.getHeaderString(S3Acl.grantWrite);
+    String grantReadACP = httpHeaders.getHeaderString(S3Acl.grantReadACP);
+    String grantWriteACP = httpHeaders.getHeaderString(S3Acl.grantWriteACP);
+    String grantFull = httpHeaders.getHeaderString(S3Acl.grantFullControl);
+
+    try {
+      OzoneBucket bucket = getBucket(bucketName);
+      OzoneVolume volume = getVolume();
+
+      List<OzoneAcl> ozoneAclListOnBucket = new ArrayList<>();
+      List<OzoneAcl> ozoneAclListOnVolume = new ArrayList<>();
+
+      if (grantReads == null && grantWrites == null && grantReadACP == null
+          && grantWriteACP == null && grantFull == null) {
+        S3BucketAcl putBucketAclRequest =
+            new PutBucketAclRequestUnmarshaller().readFrom(
+            null, null, null, null, null, body);
+        // Handle grants in body
+        ozoneAclListOnBucket.addAll(
+            S3Acl.s3AclToOzoneNativeAclOnBucket(putBucketAclRequest));
+        ozoneAclListOnVolume.addAll(
+            S3Acl.s3AclToOzoneNativeAclOnVolume(putBucketAclRequest));
+      } else {
+
+        // Handle grants in headers
+        if (grantReads != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantReads,
+              S3Acl.ACLType.READ.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantReads,
+              S3Acl.ACLType.READ.getValue()));
+        }
+        if (grantWrites != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantWrites,
+              S3Acl.ACLType.WRITE.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantWrites,
+              S3Acl.ACLType.WRITE.getValue()));
+        }
+        if (grantReadACP != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantReadACP,
+              S3Acl.ACLType.READ_ACP.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantReadACP,
+              S3Acl.ACLType.READ_ACP.getValue()));
+        }
+        if (grantWriteACP != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantWriteACP,
+              S3Acl.ACLType.WRITE_ACP.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantWriteACP,
+              S3Acl.ACLType.WRITE_ACP.getValue()));
+        }
+        if (grantFull != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantFull,
+              S3Acl.ACLType.FULL_CONTROL.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantFull,
+              S3Acl.ACLType.FULL_CONTROL.getValue()));
+        }
+      }
+
+      // A put request will reset all previous ACLs
+      bucket.setAcl(ozoneAclListOnBucket);
+      volume.setAcl(ozoneAclListOnVolume);

Review comment:
       Should we read the existing volume acl, add the new ones necessary and set it back instead of overwrite the existing ACL on the s3 volume?

##########
File path: hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/S3Acl.java
##########
@@ -0,0 +1,390 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+package org.apache.hadoop.ozone.s3.endpoint;
+
+import org.apache.hadoop.ozone.OzoneAcl;
+import org.apache.hadoop.ozone.s3.endpoint.S3BucketAcl.Grant;
+import org.apache.hadoop.ozone.s3.endpoint.S3BucketAcl.Grantee;
+import org.apache.hadoop.ozone.s3.exception.OS3Exception;
+import org.apache.hadoop.ozone.s3.exception.S3ErrorTable;
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.ArrayList;
+import java.util.BitSet;
+import java.util.List;
+
+import static org.apache.hadoop.ozone.s3.exception.S3ErrorTable.INVALID_ARGUMENT;
+import static org.apache.hadoop.ozone.s3.exception.S3ErrorTable.NOT_IMPLEMENTED;
+
+public final class S3Acl {
+  private static final Logger LOG = LoggerFactory.getLogger(S3Acl.class);
+
+  // ACL put related headers
+  public static final String GRANT_READ = "x-amz-grant-read";
+  public static final String GRANT_WRITE = "x-amz-grant-write";
+  public static final String GRANT_READ_CAP = "x-amz-grant-read-acp";
+  public static final String GRANT_WRITE_CAP = "x-amz-grant-write-acp";
+  public static final String GRANT_FULL_CONTROL = "x-amz-grant-full-control";
+
+  // Not supported headers at current stage, may support it in future
+  public static final String CANNED_ACL_HEADER = "x-amz-acl";
+
+  /**
+   * https://docs.aws.amazon.com/AmazonS3/latest/dev/acl-overview.html.
+   */
+  enum ACLType {
+    // Allows grantee to list the objects in the bucket
+    READ("READ"),
+    // Allows grantee to create, overwrite, and delete any object in the bucket
+    WRITE("WRITE"),
+    // Allows grantee to write the bucket ACL
+    READ_ACP("READ_ACP"),
+    // Allows grantee to write the ACL for the applicable bucket
+    WRITE_ACP("WRITE_ACP"),
+    // Allows grantee above all permissions on the bucket
+    FULL_CONTROL("FULL_CONTROL");
+
+    public String getValue() {
+      return value;
+    }
+    /**
+     * String value for this Enum.
+     */
+    private final String value;
+
+    /**
+     * @param val String type for this enum.
+     */
+    ACLType(String val) {
+      value = val;
+    }
+
+
+    public static ACLType getType(String typeStr) {
+      for(ACLType type: ACLType.values()) {
+        if (type.getValue().equals(typeStr)) {
+          return type;
+        }
+      }
+      return null;
+    }
+  }
+
+  enum ACLIdentityType {
+    USER("CanonicalUser", true, "id"),
+    GROUP("Group", false, "url"),
+    USER_BY_EMAIL("AmazonCustomerByEmail", false, "emailAddress");
+
+    public String getGranteeType() {
+      return granteeType;
+    }
+
+    public String getHeaderType() {
+      return granteeInHeader;
+    }
+
+    /**
+     *  Grantee type in body XML.
+     */
+    private final String granteeType;
+
+    /**
+     * Is this type supported or not.
+     */
+    private final boolean supported;
+
+    /**
+     * Grantee type in header.
+     */
+    private final String granteeInHeader;
+
+    /**
+     * Init OzoneACLtypes enum.
+     *
+     * @param val String type for this enum.
+     */
+    ACLIdentityType(String val, boolean support, String headerType) {
+      granteeType = val;
+      supported = support;
+      granteeInHeader = headerType;
+    }
+
+    boolean isSupported() {
+      return supported;
+    }
+
+    public static ACLIdentityType getTypeFromGranteeType(String typeStr) {
+      for(ACLIdentityType type: ACLIdentityType.values()) {
+        if (type.getGranteeType().equals(typeStr)) {
+          return type;
+        }
+      }
+      return null;
+    }
+
+    public static ACLIdentityType getTypeFromHeaderType(String typeStr) {
+      for(ACLIdentityType type: ACLIdentityType.values()) {
+        if (type.getHeaderType().equals(typeStr)) {
+          return type;
+        }
+      }
+      return null;
+    }
+  }
+
+  private S3Acl() {
+  }
+
+  public static boolean isGranteeTypeSupported(String typeStr) {
+    ACLIdentityType type =  ACLIdentityType.getTypeFromGranteeType(typeStr);
+    return type == null ? false : type.isSupported();
+  }
+
+  public static boolean isHeaderTypeSupported(String typeStr) {
+    ACLIdentityType type =  ACLIdentityType.getTypeFromHeaderType(typeStr);
+    return type == null ? false : type.isSupported();
+  }
+
+  public static List<Grant> ozoneNativeAclToS3Acl(OzoneAcl ozoneAcl) {
+    // Since currently only "CanonicalUser" is supported, which maps to Ozone
+    // "USER"
+    List<Grant> grantList = new ArrayList<>();
+    if (ozoneAcl.getType() != IAccessAuthorizer.ACLIdentityType.USER) {
+      return grantList;
+    }
+
+    Grantee grantee = new Grantee();
+    grantee.setDisplayName(ozoneAcl.getName());
+    grantee.setId(ozoneAcl.getName());
+
+    List<IAccessAuthorizer.ACLType> acls = ozoneAcl.getAclList();
+    if (acls.contains(IAccessAuthorizer.ACLType.ALL)) {
+      Grant grant = new Grant();
+      grant.setGrantee(grantee);
+      grant.setPermission(ACLType.FULL_CONTROL.toString());
+      grantList.add(grant);
+    } else if (acls.contains(IAccessAuthorizer.ACLType.WRITE_ACL)) {
+      Grant grant = new Grant();
+      grant.setGrantee(grantee);
+      grant.setPermission(ACLType.WRITE_ACP.toString());
+      grantList.add(grant);
+    } else if (acls.contains(IAccessAuthorizer.ACLType.READ_ACL)) {
+      Grant grant = new Grant();
+      grant.setGrantee(grantee);
+      grant.setPermission(ACLType.READ_ACP.toString());
+      grantList.add(grant);
+    } else if (acls.contains(IAccessAuthorizer.ACLType.WRITE) &&
+        acls.contains(IAccessAuthorizer.ACLType.DELETE) &&
+        acls.contains(IAccessAuthorizer.ACLType.CREATE)) {
+      Grant grant = new Grant();
+      grant.setGrantee(grantee);
+      grant.setPermission(ACLType.WRITE.toString());
+      grantList.add(grant);
+    } else if (acls.contains(IAccessAuthorizer.ACLType.READ) &&
+        acls.contains(IAccessAuthorizer.ACLType.LIST)) {
+      Grant grant = new Grant();
+      grant.setGrantee(grantee);
+      grant.setPermission(ACLType.READ.toString());
+      grantList.add(grant);
+    } else {
+      LOG.error("Cannot find a good mapping for Ozone ACL {} to S3",
+          ozoneAcl.toString());
+    }
+    return grantList;
+  }
+
+  public static List<OzoneAcl> s3AclToOzoneNativeAclOnBucket(
+      S3BucketAcl bucketAcl) throws OS3Exception {
+    List<OzoneAcl> ozoneAclList = new ArrayList<>();
+    List<Grant> grantList = bucketAcl.getAclList().getGrantList();
+    for (Grant grant : grantList) {
+      //  Only "CanonicalUser" is supported, which maps to Ozone "USER"
+      ACLIdentityType identityType = ACLIdentityType.getTypeFromGranteeType(
+          grant.getGrantee().getXsiType());
+      if (identityType != null && identityType.isSupported()) {
+        String permission = grant.getPermission();
+        BitSet acls = getOzoneAclOnBucketFromS3Permission(permission);
+        OzoneAcl defaultOzoneAcl = new OzoneAcl(
+            IAccessAuthorizer.ACLIdentityType.USER,
+            grant.getGrantee().getId(), acls,
+            OzoneAcl.AclScope.DEFAULT);
+        OzoneAcl accessOzoneAcl = new OzoneAcl(
+            IAccessAuthorizer.ACLIdentityType.USER,
+            grant.getGrantee().getId(), acls,
+            OzoneAcl.AclScope.ACCESS);
+        ozoneAclList.add(defaultOzoneAcl);
+        ozoneAclList.add(accessOzoneAcl);
+      } else {
+        LOG.error("Grantee type {} is not supported",
+            grant.getGrantee().getXsiType());
+        throw S3ErrorTable.newError(NOT_IMPLEMENTED,
+            grant.getGrantee().getXsiType());
+      }
+    }
+    return ozoneAclList;
+  }
+
+  public static BitSet getOzoneAclOnBucketFromS3Permission(String permission)
+      throws OS3Exception {
+    ACLType permissionType = ACLType.getType(permission);
+    if (permissionType == null) {
+      throw S3ErrorTable.newError(S3ErrorTable.INVALID_ARGUMENT, permission);
+    }
+    BitSet acls = new BitSet(IAccessAuthorizer.ACLType.getNoOfAcls());
+    switch (permissionType) {
+    case FULL_CONTROL:
+      acls.set(IAccessAuthorizer.ACLType.ALL.ordinal());
+      break;
+    case WRITE_ACP:
+      acls.set(IAccessAuthorizer.ACLType.WRITE_ACL.ordinal());
+      break;
+    case READ_ACP:
+      acls.set(IAccessAuthorizer.ACLType.READ_ACL.ordinal());
+      break;
+    case WRITE:
+      acls.set(IAccessAuthorizer.ACLType.WRITE.ordinal());
+      acls.set(IAccessAuthorizer.ACLType.DELETE.ordinal());
+      acls.set(IAccessAuthorizer.ACLType.CREATE.ordinal());
+      break;
+    case READ:
+      acls.set(IAccessAuthorizer.ACLType.READ.ordinal());
+      acls.set(IAccessAuthorizer.ACLType.LIST.ordinal());
+      break;
+    default:
+      LOG.error("Failed to recognize S3 permission {}", permission);
+      throw S3ErrorTable.newError(INVALID_ARGUMENT, permission);
+    }
+    return acls;
+  }
+
+  public static List<OzoneAcl> s3AclToOzoneNativeAclOnVolume(
+      S3BucketAcl bucketAcl) throws OS3Exception {
+    List<OzoneAcl> ozoneAclList = new ArrayList<>();
+    List<Grant> grantList = bucketAcl.getAclList().getGrantList();
+    for (Grant grant : grantList) {
+      //  Only "CanonicalUser" is supported, which maps to Ozone "USER"
+      ACLIdentityType identityType = ACLIdentityType.getTypeFromGranteeType(
+          grant.getGrantee().getXsiType());
+      if (identityType != null && identityType.isSupported()) {
+        String permission = grant.getPermission();
+        BitSet acls = getOzoneAclOnVolumeFromS3Permission(permission);
+        OzoneAcl defaultOzoneAcl = new OzoneAcl(
+            IAccessAuthorizer.ACLIdentityType.USER,
+            grant.getGrantee().getId(), acls,
+            OzoneAcl.AclScope.DEFAULT);
+        OzoneAcl accessOzoneAcl = new OzoneAcl(
+            IAccessAuthorizer.ACLIdentityType.USER,
+            grant.getGrantee().getId(), acls,
+            OzoneAcl.AclScope.ACCESS);
+        ozoneAclList.add(defaultOzoneAcl);
+        ozoneAclList.add(accessOzoneAcl);
+      } else {
+        LOG.error("Grantee type {} is not supported",
+            grant.getGrantee().getXsiType());
+        throw S3ErrorTable.newError(NOT_IMPLEMENTED,
+            grant.getGrantee().getXsiType());
+      }
+    }
+    return ozoneAclList;
+  }
+
+  public static BitSet getOzoneAclOnVolumeFromS3Permission(String permission)
+      throws OS3Exception {
+    BitSet acls = new BitSet(IAccessAuthorizer.ACLType.getNoOfAcls());
+    ACLType permissionType = ACLType.getType(permission);
+    if (permissionType == null) {
+      throw S3ErrorTable.newError(S3ErrorTable.INVALID_ARGUMENT, permission);
+    }
+    switch (permissionType) {
+    case FULL_CONTROL:
+      acls.set(IAccessAuthorizer.ACLType.ALL.ordinal());
+      break;
+    case WRITE_ACP:
+      acls.set(IAccessAuthorizer.ACLType.WRITE.ordinal());
+      break;
+    case READ_ACP:
+      acls.set(IAccessAuthorizer.ACLType.READ.ordinal());
+      break;
+    case WRITE:
+      acls.set(IAccessAuthorizer.ACLType.WRITE.ordinal());
+      break;
+    case READ:

Review comment:
       In the design doc, the bucket read acl is mapped to read/list on both volume and bucket. The code mapped to read only on the volume. Can you confirm if the doc needs to be changed? https://docs.google.com/document/d/18loKS6qbMKh8kKkpPvma9X-fNIcxBVAcHulAQxZpI1c/edit#




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] ChenSammi commented on pull request #1701: HDDS-4585. Support bucket acl operation in S3g

Posted by GitBox <gi...@apache.org>.
ChenSammi commented on pull request #1701:
URL: https://github.com/apache/ozone/pull/1701#issuecomment-751221225


   AWS CLI console output with the patch, 
   
   Get ACL
   [root@]# aws s3api --profile root --endpoint-url http://host:9878  get-bucket-acl --bucket ozone
   {
       "Owner": {
           "DisplayName": "root", 
           "ID": "root"
       }, 
       "Grants": [
           {
               "Grantee": {
                   "Type": "CanonicalUser", 
                   "DisplayName": "apple", 
                   "ID": "apple"
               }, 
               "Permission": "FULL_CONTROL"
           }
       ]
   }
   
   Put ACL 
   Form1 
   [root@]#aws s3api --profile root  --endpoint-url http://host:9878 put-bucket-acl --bucket=ozone --grant-read id=one,id=two,id=three --grant-full-control id=tom,id=jerry
   
   Form2 
   [root@]#aws s3api --profile root  --endpoint-url http://host:9878 put-bucket-acl --bucket=ozone --access-control-policy '{"Grants": [ {"Grantee": {"DisplayName": "root", "ID": "root", "Type": "CanonicalUser"},"Permission": "FULL_CONTROL"} ], "Owner": {"DisplayName": "root","ID": "root"}}'
   
   
   
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] xiaoyuyao merged pull request #1701: HDDS-4585. Support bucket acl operation in S3g

Posted by GitBox <gi...@apache.org>.
xiaoyuyao merged pull request #1701:
URL: https://github.com/apache/ozone/pull/1701


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] xiaoyuyao commented on a change in pull request #1701: HDDS-4585. Support bucket acl operation in S3g

Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #1701:
URL: https://github.com/apache/ozone/pull/1701#discussion_r552910908



##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneVolume.java
##########
@@ -261,7 +270,37 @@ public Instant getModificationTime() {
    * @return aclMap
    */
   public List<OzoneAcl> getAcls() {
-    return acls;
+    return (ArrayList)((ArrayList)acls).clone();
+  }
+
+   /**
+   * Adds ACLs to the volume.
+   * @param addAcl ACL to be added
+   * @return true - if acl is successfully added, false if acl already exists
+   * for the bucket.
+   * @throws IOException
+   */
+  public boolean addAcl(OzoneAcl addAcl) throws IOException {

Review comment:
       Do we need volume acl APIs for s3g?

##########
File path: hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java
##########
@@ -75,7 +87,7 @@
   @GET
   @SuppressFBWarnings
   @SuppressWarnings("parameternumber")
-  public Response list(
+  public Response get(

Review comment:
       Does this change the semantics of the get/list for bucket endpoint?

##########
File path: hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java
##########
@@ -353,6 +375,215 @@ public MultiDeleteResponse multiDelete(@PathParam("bucket") String bucketName,
     return result;
   }
 
+  /**
+   * Implement acl get.
+   * <p>
+   * see: https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketAcl.html
+   */
+  public S3BucketAcl getAcl(String bucketName)
+      throws OS3Exception, IOException {
+    S3BucketAcl result = new S3BucketAcl();
+    try {
+      OzoneBucket bucket = getBucket(bucketName);
+      OzoneVolume volume = getVolume();
+      S3Owner owner = new S3Owner(volume.getOwner(), volume.getOwner());

Review comment:
       Can you add a TODO to support bucket owner here?

##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneVolume.java
##########
@@ -261,7 +270,37 @@ public Instant getModificationTime() {
    * @return aclMap
    */
   public List<OzoneAcl> getAcls() {
-    return acls;
+    return (ArrayList)((ArrayList)acls).clone();

Review comment:
       NIT: can we use apache common ListUtils.unmodifiableList here?

##########
File path: hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java
##########
@@ -353,6 +375,215 @@ public MultiDeleteResponse multiDelete(@PathParam("bucket") String bucketName,
     return result;
   }
 
+  /**
+   * Implement acl get.
+   * <p>
+   * see: https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketAcl.html
+   */
+  public S3BucketAcl getAcl(String bucketName)
+      throws OS3Exception, IOException {
+    S3BucketAcl result = new S3BucketAcl();
+    try {
+      OzoneBucket bucket = getBucket(bucketName);
+      OzoneVolume volume = getVolume();
+      S3Owner owner = new S3Owner(volume.getOwner(), volume.getOwner());
+      result.setOwner(owner);
+      // Use set to remove ACLs with different scopes(ACCESS and DEFAULT)
+      Set<Grant> grantSet = new HashSet<>();
+      // Return ACL list
+      for (OzoneAcl acl : bucket.getAcls()) {
+        List<Grant> grants = S3Acl.ozoneNativeAclToS3Acl(acl);

Review comment:
       The Ozone Bucket acl should not have duplicate entries. Can we use the list directly with the the list->set->list conversion?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] ChenSammi commented on a change in pull request #1701: HDDS-4585. Support bucket acl operation in S3g

Posted by GitBox <gi...@apache.org>.
ChenSammi commented on a change in pull request #1701:
URL: https://github.com/apache/ozone/pull/1701#discussion_r565916883



##########
File path: hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java
##########
@@ -353,6 +375,215 @@ public MultiDeleteResponse multiDelete(@PathParam("bucket") String bucketName,
     return result;
   }
 
+  /**
+   * Implement acl get.
+   * <p>
+   * see: https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketAcl.html
+   */
+  public S3BucketAcl getAcl(String bucketName)
+      throws OS3Exception, IOException {
+    S3BucketAcl result = new S3BucketAcl();
+    try {
+      OzoneBucket bucket = getBucket(bucketName);
+      OzoneVolume volume = getVolume();
+      S3Owner owner = new S3Owner(volume.getOwner(), volume.getOwner());
+      result.setOwner(owner);
+      // Use set to remove ACLs with different scopes(ACCESS and DEFAULT)
+      Set<Grant> grantSet = new HashSet<>();
+      // Return ACL list
+      for (OzoneAcl acl : bucket.getAcls()) {
+        List<Grant> grants = S3Acl.ozoneNativeAclToS3Acl(acl);

Review comment:
       Yes, we don't have duplicate entries in individual ACCESS and DEFAULT scope, but we may have two entries with the same content besides the scope field. So I still need this set to cover this case, to avoid add two same entries. 
   
   In future, when ACCESS and DEFAULT scope is merged, we can refactor this part of code. I will add a TODO here. 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] prashantpogde commented on a change in pull request #1701: HDDS-4585. Support bucket acl operation in S3g

Posted by GitBox <gi...@apache.org>.
prashantpogde commented on a change in pull request #1701:
URL: https://github.com/apache/ozone/pull/1701#discussion_r568937379



##########
File path: hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java
##########
@@ -353,6 +373,204 @@ public MultiDeleteResponse multiDelete(@PathParam("bucket") String bucketName,
     return result;
   }
 
+  /**
+   * Implement acl get.
+   * <p>
+   * see: https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketAcl.html
+   */
+  public S3BucketAcl getAcl(String bucketName)
+      throws OS3Exception, IOException {
+    S3BucketAcl result = new S3BucketAcl();
+    try {
+      OzoneBucket bucket = getBucket(bucketName);
+      OzoneVolume volume = getVolume();
+      // TODO: use bucket owner instead of volume owner here once bucket owner
+      // TODO: is supported.
+      S3Owner owner = new S3Owner(volume.getOwner(), volume.getOwner());
+      result.setOwner(owner);
+
+      // TODO: remove this duplication avoid logic when ACCESS and DEFAULT scope
+      // TODO: are merged.
+      // Use set to remove ACLs with different scopes(ACCESS and DEFAULT)
+      Set<Grant> grantSet = new HashSet<>();
+      // Return ACL list
+      for (OzoneAcl acl : bucket.getAcls()) {
+        List<Grant> grants = S3Acl.ozoneNativeAclToS3Acl(acl);
+        grantSet.addAll(grants);
+      }
+      ArrayList<Grant> grantList = new ArrayList<>();
+      grantList.addAll(grantSet);
+      result.setAclList(
+          new S3BucketAcl.AccessControlList(grantList));
+      return result;
+    } catch (OMException ex) {
+      if (ex.getResult() == ResultCodes.BUCKET_NOT_FOUND) {
+        throw S3ErrorTable.newError(S3ErrorTable
+            .NO_SUCH_BUCKET, bucketName);
+      } else if (ex.getResult() == ResultCodes.PERMISSION_DENIED) {
+        throw S3ErrorTable.newError(S3ErrorTable
+            .ACCESS_DENIED, bucketName);
+      } else {
+        LOG.error("Failed to get acl of Bucket " + bucketName, ex);
+        throw S3ErrorTable.newError(S3ErrorTable.INTERNAL_ERROR, bucketName);
+      }
+    }
+  }
+
+  /**
+   * Implement acl put.
+   * <p>
+   * see: https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutBucketAcl.html
+   */
+  public Response putAcl(String bucketName, HttpHeaders httpHeaders,
+      InputStream body) throws IOException, OS3Exception {
+    String grantReads = httpHeaders.getHeaderString(S3Acl.grantRead);
+    String grantWrites = httpHeaders.getHeaderString(S3Acl.grantWrite);
+    String grantReadACP = httpHeaders.getHeaderString(S3Acl.grantReadACP);
+    String grantWriteACP = httpHeaders.getHeaderString(S3Acl.grantWriteACP);
+    String grantFull = httpHeaders.getHeaderString(S3Acl.grantFullControl);
+
+    try {
+      OzoneBucket bucket = getBucket(bucketName);
+      OzoneVolume volume = getVolume();
+
+      List<OzoneAcl> ozoneAclListOnBucket = new ArrayList<>();
+      List<OzoneAcl> ozoneAclListOnVolume = new ArrayList<>();
+
+      if (grantReads == null && grantWrites == null && grantReadACP == null
+          && grantWriteACP == null && grantFull == null) {
+        S3BucketAcl putBucketAclRequest =
+            new PutBucketAclRequestUnmarshaller().readFrom(
+            null, null, null, null, null, body);
+        // Handle grants in body
+        ozoneAclListOnBucket.addAll(
+            S3Acl.s3AclToOzoneNativeAclOnBucket(putBucketAclRequest));
+        ozoneAclListOnVolume.addAll(
+            S3Acl.s3AclToOzoneNativeAclOnVolume(putBucketAclRequest));
+      } else {
+
+        // Handle grants in headers
+        if (grantReads != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantReads,
+              S3Acl.ACLType.READ.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantReads,
+              S3Acl.ACLType.READ.getValue()));
+        }
+        if (grantWrites != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantWrites,
+              S3Acl.ACLType.WRITE.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantWrites,
+              S3Acl.ACLType.WRITE.getValue()));
+        }
+        if (grantReadACP != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantReadACP,
+              S3Acl.ACLType.READ_ACP.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantReadACP,
+              S3Acl.ACLType.READ_ACP.getValue()));
+        }
+        if (grantWriteACP != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantWriteACP,
+              S3Acl.ACLType.WRITE_ACP.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantWriteACP,
+              S3Acl.ACLType.WRITE_ACP.getValue()));
+        }
+        if (grantFull != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantFull,
+              S3Acl.ACLType.FULL_CONTROL.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantFull,
+              S3Acl.ACLType.FULL_CONTROL.getValue()));
+        }
+      }
+
+      // A put request will reset all previous ACLs
+      bucket.setAcl(ozoneAclListOnBucket);
+      volume.setAcl(ozoneAclListOnVolume);

Review comment:
       Do we need to setup ACL on a volume here ? couldn't this lead to manipulating permissions on a bucket where the user doesn't have a permission ?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] xiaoyuyao commented on a change in pull request #1701: HDDS-4585. Support bucket acl operation in S3g

Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #1701:
URL: https://github.com/apache/ozone/pull/1701#discussion_r612643425



##########
File path: hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/S3Acl.java
##########
@@ -0,0 +1,390 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+package org.apache.hadoop.ozone.s3.endpoint;
+
+import org.apache.hadoop.ozone.OzoneAcl;
+import org.apache.hadoop.ozone.s3.endpoint.S3BucketAcl.Grant;
+import org.apache.hadoop.ozone.s3.endpoint.S3BucketAcl.Grantee;
+import org.apache.hadoop.ozone.s3.exception.OS3Exception;
+import org.apache.hadoop.ozone.s3.exception.S3ErrorTable;
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.ArrayList;
+import java.util.BitSet;
+import java.util.List;
+
+import static org.apache.hadoop.ozone.s3.exception.S3ErrorTable.INVALID_ARGUMENT;
+import static org.apache.hadoop.ozone.s3.exception.S3ErrorTable.NOT_IMPLEMENTED;
+
+public final class S3Acl {
+  private static final Logger LOG = LoggerFactory.getLogger(S3Acl.class);
+
+  // ACL put related headers
+  public static final String GRANT_READ = "x-amz-grant-read";
+  public static final String GRANT_WRITE = "x-amz-grant-write";
+  public static final String GRANT_READ_CAP = "x-amz-grant-read-acp";
+  public static final String GRANT_WRITE_CAP = "x-amz-grant-write-acp";
+  public static final String GRANT_FULL_CONTROL = "x-amz-grant-full-control";
+
+  // Not supported headers at current stage, may support it in future
+  public static final String CANNED_ACL_HEADER = "x-amz-acl";
+
+  /**
+   * https://docs.aws.amazon.com/AmazonS3/latest/dev/acl-overview.html.
+   */
+  enum ACLType {
+    // Allows grantee to list the objects in the bucket
+    READ("READ"),
+    // Allows grantee to create, overwrite, and delete any object in the bucket
+    WRITE("WRITE"),
+    // Allows grantee to write the bucket ACL
+    READ_ACP("READ_ACP"),
+    // Allows grantee to write the ACL for the applicable bucket
+    WRITE_ACP("WRITE_ACP"),
+    // Allows grantee above all permissions on the bucket
+    FULL_CONTROL("FULL_CONTROL");
+
+    public String getValue() {
+      return value;
+    }
+    /**
+     * String value for this Enum.
+     */
+    private final String value;
+
+    /**
+     * @param val String type for this enum.
+     */
+    ACLType(String val) {
+      value = val;
+    }
+
+
+    public static ACLType getType(String typeStr) {
+      for(ACLType type: ACLType.values()) {
+        if (type.getValue().equals(typeStr)) {
+          return type;
+        }
+      }
+      return null;
+    }
+  }
+
+  enum ACLIdentityType {
+    USER("CanonicalUser", true, "id"),
+    GROUP("Group", false, "url"),
+    USER_BY_EMAIL("AmazonCustomerByEmail", false, "emailAddress");
+
+    public String getGranteeType() {
+      return granteeType;
+    }
+
+    public String getHeaderType() {
+      return granteeInHeader;
+    }
+
+    /**
+     *  Grantee type in body XML.
+     */
+    private final String granteeType;
+
+    /**
+     * Is this type supported or not.
+     */
+    private final boolean supported;
+
+    /**
+     * Grantee type in header.
+     */
+    private final String granteeInHeader;
+
+    /**
+     * Init OzoneACLtypes enum.
+     *
+     * @param val String type for this enum.
+     */
+    ACLIdentityType(String val, boolean support, String headerType) {
+      granteeType = val;
+      supported = support;
+      granteeInHeader = headerType;
+    }
+
+    boolean isSupported() {
+      return supported;
+    }
+
+    public static ACLIdentityType getTypeFromGranteeType(String typeStr) {
+      for(ACLIdentityType type: ACLIdentityType.values()) {
+        if (type.getGranteeType().equals(typeStr)) {
+          return type;
+        }
+      }
+      return null;
+    }
+
+    public static ACLIdentityType getTypeFromHeaderType(String typeStr) {
+      for(ACLIdentityType type: ACLIdentityType.values()) {
+        if (type.getHeaderType().equals(typeStr)) {
+          return type;
+        }
+      }
+      return null;
+    }
+  }
+
+  private S3Acl() {
+  }
+
+  public static boolean isGranteeTypeSupported(String typeStr) {
+    ACLIdentityType type =  ACLIdentityType.getTypeFromGranteeType(typeStr);
+    return type == null ? false : type.isSupported();
+  }
+
+  public static boolean isHeaderTypeSupported(String typeStr) {
+    ACLIdentityType type =  ACLIdentityType.getTypeFromHeaderType(typeStr);
+    return type == null ? false : type.isSupported();
+  }
+
+  public static List<Grant> ozoneNativeAclToS3Acl(OzoneAcl ozoneAcl) {
+    // Since currently only "CanonicalUser" is supported, which maps to Ozone
+    // "USER"
+    List<Grant> grantList = new ArrayList<>();
+    if (ozoneAcl.getType() != IAccessAuthorizer.ACLIdentityType.USER) {
+      return grantList;
+    }
+
+    Grantee grantee = new Grantee();
+    grantee.setDisplayName(ozoneAcl.getName());
+    grantee.setId(ozoneAcl.getName());
+
+    List<IAccessAuthorizer.ACLType> acls = ozoneAcl.getAclList();
+    if (acls.contains(IAccessAuthorizer.ACLType.ALL)) {
+      Grant grant = new Grant();
+      grant.setGrantee(grantee);
+      grant.setPermission(ACLType.FULL_CONTROL.toString());
+      grantList.add(grant);
+    } else if (acls.contains(IAccessAuthorizer.ACLType.WRITE_ACL)) {
+      Grant grant = new Grant();
+      grant.setGrantee(grantee);
+      grant.setPermission(ACLType.WRITE_ACP.toString());
+      grantList.add(grant);
+    } else if (acls.contains(IAccessAuthorizer.ACLType.READ_ACL)) {
+      Grant grant = new Grant();
+      grant.setGrantee(grantee);
+      grant.setPermission(ACLType.READ_ACP.toString());
+      grantList.add(grant);
+    } else if (acls.contains(IAccessAuthorizer.ACLType.WRITE) &&
+        acls.contains(IAccessAuthorizer.ACLType.DELETE) &&
+        acls.contains(IAccessAuthorizer.ACLType.CREATE)) {
+      Grant grant = new Grant();
+      grant.setGrantee(grantee);
+      grant.setPermission(ACLType.WRITE.toString());
+      grantList.add(grant);
+    } else if (acls.contains(IAccessAuthorizer.ACLType.READ) &&
+        acls.contains(IAccessAuthorizer.ACLType.LIST)) {
+      Grant grant = new Grant();
+      grant.setGrantee(grantee);
+      grant.setPermission(ACLType.READ.toString());
+      grantList.add(grant);
+    } else {
+      LOG.error("Cannot find a good mapping for Ozone ACL {} to S3",
+          ozoneAcl.toString());
+    }
+    return grantList;
+  }
+
+  public static List<OzoneAcl> s3AclToOzoneNativeAclOnBucket(
+      S3BucketAcl bucketAcl) throws OS3Exception {
+    List<OzoneAcl> ozoneAclList = new ArrayList<>();
+    List<Grant> grantList = bucketAcl.getAclList().getGrantList();
+    for (Grant grant : grantList) {
+      //  Only "CanonicalUser" is supported, which maps to Ozone "USER"
+      ACLIdentityType identityType = ACLIdentityType.getTypeFromGranteeType(
+          grant.getGrantee().getXsiType());
+      if (identityType != null && identityType.isSupported()) {
+        String permission = grant.getPermission();
+        BitSet acls = getOzoneAclOnBucketFromS3Permission(permission);
+        OzoneAcl defaultOzoneAcl = new OzoneAcl(
+            IAccessAuthorizer.ACLIdentityType.USER,
+            grant.getGrantee().getId(), acls,
+            OzoneAcl.AclScope.DEFAULT);
+        OzoneAcl accessOzoneAcl = new OzoneAcl(
+            IAccessAuthorizer.ACLIdentityType.USER,
+            grant.getGrantee().getId(), acls,
+            OzoneAcl.AclScope.ACCESS);
+        ozoneAclList.add(defaultOzoneAcl);
+        ozoneAclList.add(accessOzoneAcl);
+      } else {
+        LOG.error("Grantee type {} is not supported",
+            grant.getGrantee().getXsiType());
+        throw S3ErrorTable.newError(NOT_IMPLEMENTED,
+            grant.getGrantee().getXsiType());
+      }
+    }
+    return ozoneAclList;
+  }
+
+  public static BitSet getOzoneAclOnBucketFromS3Permission(String permission)
+      throws OS3Exception {
+    ACLType permissionType = ACLType.getType(permission);
+    if (permissionType == null) {
+      throw S3ErrorTable.newError(S3ErrorTable.INVALID_ARGUMENT, permission);
+    }
+    BitSet acls = new BitSet(IAccessAuthorizer.ACLType.getNoOfAcls());
+    switch (permissionType) {
+    case FULL_CONTROL:
+      acls.set(IAccessAuthorizer.ACLType.ALL.ordinal());
+      break;
+    case WRITE_ACP:
+      acls.set(IAccessAuthorizer.ACLType.WRITE_ACL.ordinal());
+      break;
+    case READ_ACP:
+      acls.set(IAccessAuthorizer.ACLType.READ_ACL.ordinal());
+      break;
+    case WRITE:
+      acls.set(IAccessAuthorizer.ACLType.WRITE.ordinal());
+      acls.set(IAccessAuthorizer.ACLType.DELETE.ordinal());
+      acls.set(IAccessAuthorizer.ACLType.CREATE.ordinal());
+      break;
+    case READ:
+      acls.set(IAccessAuthorizer.ACLType.READ.ordinal());
+      acls.set(IAccessAuthorizer.ACLType.LIST.ordinal());
+      break;
+    default:
+      LOG.error("Failed to recognize S3 permission {}", permission);
+      throw S3ErrorTable.newError(INVALID_ARGUMENT, permission);
+    }
+    return acls;
+  }
+
+  public static List<OzoneAcl> s3AclToOzoneNativeAclOnVolume(
+      S3BucketAcl bucketAcl) throws OS3Exception {
+    List<OzoneAcl> ozoneAclList = new ArrayList<>();
+    List<Grant> grantList = bucketAcl.getAclList().getGrantList();
+    for (Grant grant : grantList) {
+      //  Only "CanonicalUser" is supported, which maps to Ozone "USER"
+      ACLIdentityType identityType = ACLIdentityType.getTypeFromGranteeType(
+          grant.getGrantee().getXsiType());
+      if (identityType != null && identityType.isSupported()) {
+        String permission = grant.getPermission();
+        BitSet acls = getOzoneAclOnVolumeFromS3Permission(permission);
+        OzoneAcl defaultOzoneAcl = new OzoneAcl(
+            IAccessAuthorizer.ACLIdentityType.USER,
+            grant.getGrantee().getId(), acls,
+            OzoneAcl.AclScope.DEFAULT);
+        OzoneAcl accessOzoneAcl = new OzoneAcl(
+            IAccessAuthorizer.ACLIdentityType.USER,
+            grant.getGrantee().getId(), acls,
+            OzoneAcl.AclScope.ACCESS);
+        ozoneAclList.add(defaultOzoneAcl);
+        ozoneAclList.add(accessOzoneAcl);
+      } else {
+        LOG.error("Grantee type {} is not supported",
+            grant.getGrantee().getXsiType());
+        throw S3ErrorTable.newError(NOT_IMPLEMENTED,
+            grant.getGrantee().getXsiType());
+      }
+    }
+    return ozoneAclList;
+  }
+
+  public static BitSet getOzoneAclOnVolumeFromS3Permission(String permission)
+      throws OS3Exception {
+    BitSet acls = new BitSet(IAccessAuthorizer.ACLType.getNoOfAcls());
+    ACLType permissionType = ACLType.getType(permission);
+    if (permissionType == null) {
+      throw S3ErrorTable.newError(S3ErrorTable.INVALID_ARGUMENT, permission);
+    }
+    switch (permissionType) {
+    case FULL_CONTROL:
+      acls.set(IAccessAuthorizer.ACLType.ALL.ordinal());
+      break;
+    case WRITE_ACP:
+      acls.set(IAccessAuthorizer.ACLType.WRITE.ordinal());
+      break;
+    case READ_ACP:

Review comment:
       In the design doc, the bucket read acl is mapped to read/list on both volume and bucket. The code mapped to read only on the volume. Can you confirm if the doc needs to be changed? https://docs.google.com/document/d/18loKS6qbMKh8kKkpPvma9X-fNIcxBVAcHulAQxZpI1c/edit#




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] ChenSammi commented on pull request #1701: HDDS-4585. Support bucket acl operation in S3g

Posted by GitBox <gi...@apache.org>.
ChenSammi commented on pull request #1701:
URL: https://github.com/apache/ozone/pull/1701#issuecomment-769595310


   > 
   > 
   > > misc acceptance exit with error code 1, while there is no obvious error.
   > 
   > I'm looking into it: [HDDS-4760](https://issues.apache.org/jira/browse/HDDS-4760).
   
   @adoroszlai ,thanks for the info.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] ChenSammi commented on a change in pull request #1701: HDDS-4585. Support bucket acl operation in S3g

Posted by GitBox <gi...@apache.org>.
ChenSammi commented on a change in pull request #1701:
URL: https://github.com/apache/ozone/pull/1701#discussion_r616444648



##########
File path: hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/S3Acl.java
##########
@@ -0,0 +1,390 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+package org.apache.hadoop.ozone.s3.endpoint;
+
+import org.apache.hadoop.ozone.OzoneAcl;
+import org.apache.hadoop.ozone.s3.endpoint.S3BucketAcl.Grant;
+import org.apache.hadoop.ozone.s3.endpoint.S3BucketAcl.Grantee;
+import org.apache.hadoop.ozone.s3.exception.OS3Exception;
+import org.apache.hadoop.ozone.s3.exception.S3ErrorTable;
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.ArrayList;
+import java.util.BitSet;
+import java.util.List;
+
+import static org.apache.hadoop.ozone.s3.exception.S3ErrorTable.INVALID_ARGUMENT;
+import static org.apache.hadoop.ozone.s3.exception.S3ErrorTable.NOT_IMPLEMENTED;
+
+public final class S3Acl {
+  private static final Logger LOG = LoggerFactory.getLogger(S3Acl.class);
+
+  // ACL put related headers
+  public static final String GRANT_READ = "x-amz-grant-read";
+  public static final String GRANT_WRITE = "x-amz-grant-write";
+  public static final String GRANT_READ_CAP = "x-amz-grant-read-acp";
+  public static final String GRANT_WRITE_CAP = "x-amz-grant-write-acp";
+  public static final String GRANT_FULL_CONTROL = "x-amz-grant-full-control";
+
+  // Not supported headers at current stage, may support it in future
+  public static final String CANNED_ACL_HEADER = "x-amz-acl";
+
+  /**
+   * https://docs.aws.amazon.com/AmazonS3/latest/dev/acl-overview.html.
+   */
+  enum ACLType {
+    // Allows grantee to list the objects in the bucket
+    READ("READ"),
+    // Allows grantee to create, overwrite, and delete any object in the bucket
+    WRITE("WRITE"),
+    // Allows grantee to write the bucket ACL
+    READ_ACP("READ_ACP"),
+    // Allows grantee to write the ACL for the applicable bucket
+    WRITE_ACP("WRITE_ACP"),
+    // Allows grantee above all permissions on the bucket
+    FULL_CONTROL("FULL_CONTROL");
+
+    public String getValue() {
+      return value;
+    }
+    /**
+     * String value for this Enum.
+     */
+    private final String value;
+
+    /**
+     * @param val String type for this enum.
+     */
+    ACLType(String val) {
+      value = val;
+    }
+
+
+    public static ACLType getType(String typeStr) {
+      for(ACLType type: ACLType.values()) {
+        if (type.getValue().equals(typeStr)) {
+          return type;
+        }
+      }
+      return null;
+    }
+  }
+
+  enum ACLIdentityType {
+    USER("CanonicalUser", true, "id"),
+    GROUP("Group", false, "url"),
+    USER_BY_EMAIL("AmazonCustomerByEmail", false, "emailAddress");
+
+    public String getGranteeType() {
+      return granteeType;
+    }
+
+    public String getHeaderType() {
+      return granteeInHeader;
+    }
+
+    /**
+     *  Grantee type in body XML.
+     */
+    private final String granteeType;
+
+    /**
+     * Is this type supported or not.
+     */
+    private final boolean supported;
+
+    /**
+     * Grantee type in header.
+     */
+    private final String granteeInHeader;
+
+    /**
+     * Init OzoneACLtypes enum.
+     *
+     * @param val String type for this enum.
+     */
+    ACLIdentityType(String val, boolean support, String headerType) {
+      granteeType = val;
+      supported = support;
+      granteeInHeader = headerType;
+    }
+
+    boolean isSupported() {
+      return supported;
+    }
+
+    public static ACLIdentityType getTypeFromGranteeType(String typeStr) {
+      for(ACLIdentityType type: ACLIdentityType.values()) {
+        if (type.getGranteeType().equals(typeStr)) {
+          return type;
+        }
+      }
+      return null;
+    }
+
+    public static ACLIdentityType getTypeFromHeaderType(String typeStr) {
+      for(ACLIdentityType type: ACLIdentityType.values()) {
+        if (type.getHeaderType().equals(typeStr)) {
+          return type;
+        }
+      }
+      return null;
+    }
+  }
+
+  private S3Acl() {
+  }
+
+  public static boolean isGranteeTypeSupported(String typeStr) {
+    ACLIdentityType type =  ACLIdentityType.getTypeFromGranteeType(typeStr);
+    return type == null ? false : type.isSupported();
+  }
+
+  public static boolean isHeaderTypeSupported(String typeStr) {
+    ACLIdentityType type =  ACLIdentityType.getTypeFromHeaderType(typeStr);
+    return type == null ? false : type.isSupported();
+  }
+
+  public static List<Grant> ozoneNativeAclToS3Acl(OzoneAcl ozoneAcl) {
+    // Since currently only "CanonicalUser" is supported, which maps to Ozone
+    // "USER"
+    List<Grant> grantList = new ArrayList<>();
+    if (ozoneAcl.getType() != IAccessAuthorizer.ACLIdentityType.USER) {
+      return grantList;
+    }
+
+    Grantee grantee = new Grantee();
+    grantee.setDisplayName(ozoneAcl.getName());
+    grantee.setId(ozoneAcl.getName());
+
+    List<IAccessAuthorizer.ACLType> acls = ozoneAcl.getAclList();
+    if (acls.contains(IAccessAuthorizer.ACLType.ALL)) {
+      Grant grant = new Grant();
+      grant.setGrantee(grantee);
+      grant.setPermission(ACLType.FULL_CONTROL.toString());
+      grantList.add(grant);
+    } else if (acls.contains(IAccessAuthorizer.ACLType.WRITE_ACL)) {
+      Grant grant = new Grant();
+      grant.setGrantee(grantee);
+      grant.setPermission(ACLType.WRITE_ACP.toString());
+      grantList.add(grant);
+    } else if (acls.contains(IAccessAuthorizer.ACLType.READ_ACL)) {
+      Grant grant = new Grant();
+      grant.setGrantee(grantee);
+      grant.setPermission(ACLType.READ_ACP.toString());
+      grantList.add(grant);
+    } else if (acls.contains(IAccessAuthorizer.ACLType.WRITE) &&
+        acls.contains(IAccessAuthorizer.ACLType.DELETE) &&
+        acls.contains(IAccessAuthorizer.ACLType.CREATE)) {
+      Grant grant = new Grant();
+      grant.setGrantee(grantee);
+      grant.setPermission(ACLType.WRITE.toString());
+      grantList.add(grant);
+    } else if (acls.contains(IAccessAuthorizer.ACLType.READ) &&
+        acls.contains(IAccessAuthorizer.ACLType.LIST)) {
+      Grant grant = new Grant();
+      grant.setGrantee(grantee);
+      grant.setPermission(ACLType.READ.toString());
+      grantList.add(grant);
+    } else {
+      LOG.error("Cannot find a good mapping for Ozone ACL {} to S3",
+          ozoneAcl.toString());
+    }
+    return grantList;
+  }
+
+  public static List<OzoneAcl> s3AclToOzoneNativeAclOnBucket(
+      S3BucketAcl bucketAcl) throws OS3Exception {
+    List<OzoneAcl> ozoneAclList = new ArrayList<>();
+    List<Grant> grantList = bucketAcl.getAclList().getGrantList();
+    for (Grant grant : grantList) {
+      //  Only "CanonicalUser" is supported, which maps to Ozone "USER"
+      ACLIdentityType identityType = ACLIdentityType.getTypeFromGranteeType(
+          grant.getGrantee().getXsiType());
+      if (identityType != null && identityType.isSupported()) {
+        String permission = grant.getPermission();
+        BitSet acls = getOzoneAclOnBucketFromS3Permission(permission);
+        OzoneAcl defaultOzoneAcl = new OzoneAcl(
+            IAccessAuthorizer.ACLIdentityType.USER,
+            grant.getGrantee().getId(), acls,
+            OzoneAcl.AclScope.DEFAULT);
+        OzoneAcl accessOzoneAcl = new OzoneAcl(
+            IAccessAuthorizer.ACLIdentityType.USER,
+            grant.getGrantee().getId(), acls,
+            OzoneAcl.AclScope.ACCESS);
+        ozoneAclList.add(defaultOzoneAcl);
+        ozoneAclList.add(accessOzoneAcl);
+      } else {
+        LOG.error("Grantee type {} is not supported",
+            grant.getGrantee().getXsiType());
+        throw S3ErrorTable.newError(NOT_IMPLEMENTED,
+            grant.getGrantee().getXsiType());
+      }
+    }
+    return ozoneAclList;
+  }
+
+  public static BitSet getOzoneAclOnBucketFromS3Permission(String permission)
+      throws OS3Exception {
+    ACLType permissionType = ACLType.getType(permission);
+    if (permissionType == null) {
+      throw S3ErrorTable.newError(S3ErrorTable.INVALID_ARGUMENT, permission);
+    }
+    BitSet acls = new BitSet(IAccessAuthorizer.ACLType.getNoOfAcls());
+    switch (permissionType) {
+    case FULL_CONTROL:
+      acls.set(IAccessAuthorizer.ACLType.ALL.ordinal());
+      break;
+    case WRITE_ACP:
+      acls.set(IAccessAuthorizer.ACLType.WRITE_ACL.ordinal());
+      break;
+    case READ_ACP:
+      acls.set(IAccessAuthorizer.ACLType.READ_ACL.ordinal());
+      break;
+    case WRITE:
+      acls.set(IAccessAuthorizer.ACLType.WRITE.ordinal());
+      acls.set(IAccessAuthorizer.ACLType.DELETE.ordinal());
+      acls.set(IAccessAuthorizer.ACLType.CREATE.ordinal());
+      break;
+    case READ:
+      acls.set(IAccessAuthorizer.ACLType.READ.ordinal());
+      acls.set(IAccessAuthorizer.ACLType.LIST.ordinal());
+      break;
+    default:
+      LOG.error("Failed to recognize S3 permission {}", permission);
+      throw S3ErrorTable.newError(INVALID_ARGUMENT, permission);
+    }
+    return acls;
+  }
+
+  public static List<OzoneAcl> s3AclToOzoneNativeAclOnVolume(
+      S3BucketAcl bucketAcl) throws OS3Exception {
+    List<OzoneAcl> ozoneAclList = new ArrayList<>();
+    List<Grant> grantList = bucketAcl.getAclList().getGrantList();
+    for (Grant grant : grantList) {
+      //  Only "CanonicalUser" is supported, which maps to Ozone "USER"
+      ACLIdentityType identityType = ACLIdentityType.getTypeFromGranteeType(
+          grant.getGrantee().getXsiType());
+      if (identityType != null && identityType.isSupported()) {
+        String permission = grant.getPermission();
+        BitSet acls = getOzoneAclOnVolumeFromS3Permission(permission);
+        OzoneAcl defaultOzoneAcl = new OzoneAcl(
+            IAccessAuthorizer.ACLIdentityType.USER,
+            grant.getGrantee().getId(), acls,
+            OzoneAcl.AclScope.DEFAULT);
+        OzoneAcl accessOzoneAcl = new OzoneAcl(
+            IAccessAuthorizer.ACLIdentityType.USER,
+            grant.getGrantee().getId(), acls,
+            OzoneAcl.AclScope.ACCESS);
+        ozoneAclList.add(defaultOzoneAcl);
+        ozoneAclList.add(accessOzoneAcl);
+      } else {
+        LOG.error("Grantee type {} is not supported",
+            grant.getGrantee().getXsiType());
+        throw S3ErrorTable.newError(NOT_IMPLEMENTED,
+            grant.getGrantee().getXsiType());
+      }
+    }
+    return ozoneAclList;
+  }
+
+  public static BitSet getOzoneAclOnVolumeFromS3Permission(String permission)
+      throws OS3Exception {
+    BitSet acls = new BitSet(IAccessAuthorizer.ACLType.getNoOfAcls());
+    ACLType permissionType = ACLType.getType(permission);
+    if (permissionType == null) {
+      throw S3ErrorTable.newError(S3ErrorTable.INVALID_ARGUMENT, permission);
+    }
+    switch (permissionType) {
+    case FULL_CONTROL:
+      acls.set(IAccessAuthorizer.ACLType.ALL.ordinal());
+      break;
+    case WRITE_ACP:
+      acls.set(IAccessAuthorizer.ACLType.WRITE.ordinal());
+      break;
+    case READ_ACP:
+      acls.set(IAccessAuthorizer.ACLType.READ.ordinal());
+      break;
+    case WRITE:
+      acls.set(IAccessAuthorizer.ACLType.WRITE.ordinal());
+      break;
+    case READ:

Review comment:
       Sure. I have updated the document and the implement based on test result. 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] ChenSammi commented on a change in pull request #1701: HDDS-4585. Support bucket acl operation in S3g

Posted by GitBox <gi...@apache.org>.
ChenSammi commented on a change in pull request #1701:
URL: https://github.com/apache/ozone/pull/1701#discussion_r565915206



##########
File path: hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/S3Acl.java
##########
@@ -0,0 +1,388 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+package org.apache.hadoop.ozone.s3.endpoint;
+
+import org.apache.hadoop.ozone.OzoneAcl;
+import org.apache.hadoop.ozone.s3.endpoint.S3BucketAcl.Grant;
+import org.apache.hadoop.ozone.s3.endpoint.S3BucketAcl.Grantee;
+import org.apache.hadoop.ozone.s3.exception.OS3Exception;
+import org.apache.hadoop.ozone.s3.exception.S3ErrorTable;
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.ArrayList;
+import java.util.BitSet;
+import java.util.List;
+
+import static org.apache.hadoop.ozone.s3.exception.S3ErrorTable.INVALID_ARGUMENT;
+import static org.apache.hadoop.ozone.s3.exception.S3ErrorTable.NOT_IMPLEMENTED;
+
+public final class S3Acl {
+  private static final Logger LOG = LoggerFactory.getLogger(S3Acl.class);
+
+  // ACL put related headers
+  public static final String grantRead = "x-amz-grant-read";
+  public static final String grantWrite = "x-amz-grant-write";
+  public static final String grantReadACP = "x-amz-grant-read-acp";
+  public static final String grantWriteACP = "x-amz-grant-write-acp";
+  public static final String grantFullControl = "x-amz-grant-full-control";
+
+  // Not supported headers at current stage, may support it in future
+  public static final String cannedAclHeader = "x-amz-acl";
+
+  /**
+   * https://docs.aws.amazon.com/AmazonS3/latest/dev/acl-overview.html.
+   */
+  enum ACLType {
+    // Allows grantee to list the objects in the bucket
+    READ("READ"),
+    // Allows grantee to create, overwrite, and delete any object in the bucket
+    WRITE("WRITE"),
+    // Allows grantee to write the bucket ACL
+    READ_ACP("READ_ACP"),
+    // Allows grantee to write the ACL for the applicable bucket
+    WRITE_ACP("WRITE_ACP"),
+    // Allows grantee above all permissions on the bucket
+    FULL_CONTROL("FULL_CONTROL");
+
+    public String getValue() {
+      return value;
+    }
+    /**
+     * String value for this Enum.
+     */
+    private final String value;
+
+    /**
+     * @param val String type for this enum.
+     */
+    ACLType(String val) {
+      value = val;
+    }
+
+
+    public static ACLType getType(String typeStr) {
+      for(ACLType type: ACLType.values()) {
+        if (type.getValue().equals(typeStr)) {
+          return type;
+        }
+      }
+      return null;
+    }
+  }
+
+  enum ACLIdentityType {
+    USER("CanonicalUser", true, "id"),
+    GROUP("Group", false, "url"),
+    USER_BY_EMAIL("AmazonCustomerByEmail", false, "emailAddress");
+
+    public String getGranteeType() {
+      return granteeType;
+    }
+
+    public String getHeaderType() {
+      return granteeInHeader;
+    }
+
+    /**
+     *  Grantee type in body XML.
+     */
+    private final String granteeType;
+
+    /**
+     * Is this type supported or not.
+     */
+    private final boolean supported;
+
+    /**
+     * Grantee type in header.
+     */
+    private final String granteeInHeader;
+
+    /**
+     * Init OzoneACLtypes enum.
+     *
+     * @param val String type for this enum.
+     */
+    ACLIdentityType(String val, boolean support, String headerType) {
+      granteeType = val;
+      supported = support;
+      granteeInHeader = headerType;
+    }
+
+    boolean isSupported() {
+      return supported;
+    }
+
+    public static ACLIdentityType getTypeFromGranteeType(String typeStr) {
+      for(ACLIdentityType type: ACLIdentityType.values()) {
+        if (type.getGranteeType().equals(typeStr)) {
+          return type;
+        }
+      }
+      return null;
+    }
+
+    public static ACLIdentityType getTypeFromHeaderType(String typeStr) {
+      for(ACLIdentityType type: ACLIdentityType.values()) {
+        if (type.getHeaderType().equals(typeStr)) {
+          return type;
+        }
+      }
+      return null;
+    }
+  }
+
+  public static boolean isGranteeTypeSupported(String typeStr) {
+    ACLIdentityType type =  ACLIdentityType.getTypeFromGranteeType(typeStr);
+    return type == null ? false : type.isSupported();
+  }
+
+  public static boolean isHeaderTypeSupported(String typeStr) {
+    ACLIdentityType type =  ACLIdentityType.getTypeFromHeaderType(typeStr);
+    return type == null ? false : type.isSupported();
+  }
+
+  public static List<Grant> ozoneNativeAclToS3Acl(OzoneAcl ozoneAcl) {
+    // Since currently only "CanonicalUser" is supported, which maps to Ozone
+    // "USER"
+    List<Grant> grantList = new ArrayList<>();
+    if (ozoneAcl.getType() != IAccessAuthorizer.ACLIdentityType.USER) {
+      return grantList;
+    }
+
+    Grantee grantee = new Grantee();
+    grantee.setDisplayName(ozoneAcl.getName());
+    grantee.setId(ozoneAcl.getName());
+
+    List<IAccessAuthorizer.ACLType> acls = ozoneAcl.getAclList();
+    if (acls.contains(IAccessAuthorizer.ACLType.ALL)) {
+      Grant grant = new Grant();
+      grant.setGrantee(grantee);
+      grant.setPermission(ACLType.FULL_CONTROL.toString());
+      grantList.add(grant);
+      return grantList;
+    } else if (acls.contains(IAccessAuthorizer.ACLType.WRITE_ACL)) {
+      Grant grant = new Grant();
+      grant.setGrantee(grantee);
+      grant.setPermission(ACLType.WRITE_ACP.toString());
+      grantList.add(grant);
+    } else if (acls.contains(IAccessAuthorizer.ACLType.READ_ACL)) {
+      Grant grant = new Grant();
+      grant.setGrantee(grantee);
+      grant.setPermission(ACLType.READ_ACP.toString());
+      grantList.add(grant);
+    } else if (acls.contains(IAccessAuthorizer.ACLType.WRITE) &&
+        acls.contains(IAccessAuthorizer.ACLType.DELETE) &&
+        acls.contains(IAccessAuthorizer.ACLType.CREATE)) {
+      Grant grant = new Grant();
+      grant.setGrantee(grantee);
+      grant.setPermission(ACLType.WRITE.toString());
+      grantList.add(grant);
+    } else if (acls.contains(IAccessAuthorizer.ACLType.READ) &&
+        acls.contains(IAccessAuthorizer.ACLType.LIST)) {
+      Grant grant = new Grant();
+      grant.setGrantee(grantee);
+      grant.setPermission(ACLType.READ.toString());
+      grantList.add(grant);
+    } else {
+      LOG.error("Cannot find a good mapping for Ozone ACL {} to S3",
+          ozoneAcl.toString());
+    }
+    return grantList;
+  }
+
+  public static List<OzoneAcl> s3AclToOzoneNativeAclOnBucket(
+      S3BucketAcl bucketAcl) throws OS3Exception {
+    List<OzoneAcl> ozoneAclList = new ArrayList<>();
+    List<Grant> grantList = bucketAcl.getAclList().getGrantList();
+    for (Grant grant : grantList) {
+      //  Only "CanonicalUser" is supported, which maps to Ozone "USER"
+      ACLIdentityType identityType = ACLIdentityType.getTypeFromGranteeType(
+          grant.getGrantee().getXsiType());
+      if (identityType != null && identityType.isSupported()) {
+        String permission = grant.getPermission();
+        BitSet acls = getOzoneAclOnBucketFromS3Permission(permission);
+        OzoneAcl defaultOzoneAcl = new OzoneAcl(
+            IAccessAuthorizer.ACLIdentityType.USER,
+            grant.getGrantee().getId(), acls,
+            OzoneAcl.AclScope.DEFAULT);
+        OzoneAcl accessOzoneAcl = new OzoneAcl(
+            IAccessAuthorizer.ACLIdentityType.USER,
+            grant.getGrantee().getId(), acls,
+            OzoneAcl.AclScope.ACCESS);
+        ozoneAclList.add(defaultOzoneAcl);
+        ozoneAclList.add(accessOzoneAcl);
+      } else {
+        LOG.error("Grantee type {} is not supported",
+            grant.getGrantee().getXsiType());
+        throw S3ErrorTable.newError(NOT_IMPLEMENTED,
+            grant.getGrantee().getXsiType());
+      }
+    }
+    return ozoneAclList;
+  }
+
+  public static BitSet getOzoneAclOnBucketFromS3Permission(String permission)
+      throws OS3Exception {
+    ACLType permissionType = ACLType.getType(permission);
+    if (permissionType == null) {
+      throw S3ErrorTable.newError(S3ErrorTable.INVALID_ARGUMENT, permission);
+    }
+    BitSet acls = new BitSet(IAccessAuthorizer.ACLType.getNoOfAcls());
+    switch (permissionType) {
+    case FULL_CONTROL:
+      acls.set(IAccessAuthorizer.ACLType.ALL.ordinal());
+      break;
+    case WRITE_ACP:
+      acls.set(IAccessAuthorizer.ACLType.WRITE_ACL.ordinal());
+      break;
+    case READ_ACP:
+      acls.set(IAccessAuthorizer.ACLType.READ_ACL.ordinal());
+      break;
+    case WRITE:
+      acls.set(IAccessAuthorizer.ACLType.WRITE.ordinal());
+      acls.set(IAccessAuthorizer.ACLType.DELETE.ordinal());
+      acls.set(IAccessAuthorizer.ACLType.CREATE.ordinal());
+      break;
+    case READ:
+      acls.set(IAccessAuthorizer.ACLType.READ.ordinal());
+      acls.set(IAccessAuthorizer.ACLType.LIST.ordinal());
+      break;
+    default:
+      LOG.error("Failed to recognize S3 permission {}", permission);
+      throw S3ErrorTable.newError(INVALID_ARGUMENT, permission);
+    }
+    return acls;
+  }
+
+  public static List<OzoneAcl> s3AclToOzoneNativeAclOnVolume(
+      S3BucketAcl bucketAcl) throws OS3Exception {
+    List<OzoneAcl> ozoneAclList = new ArrayList<>();
+    List<Grant> grantList = bucketAcl.getAclList().getGrantList();
+    for (Grant grant : grantList) {
+      //  Only "CanonicalUser" is supported, which maps to Ozone "USER"
+      ACLIdentityType identityType = ACLIdentityType.getTypeFromGranteeType(
+          grant.getGrantee().getXsiType());
+      if (identityType != null && identityType.isSupported()) {
+        String permission = grant.getPermission();
+        BitSet acls = getOzoneAclOnVolumeFromS3Permission(permission);
+        OzoneAcl defaultOzoneAcl = new OzoneAcl(
+            IAccessAuthorizer.ACLIdentityType.USER,

Review comment:
       It's a good point. By going through the code, I think we can leverage the om side addAcl logic which will merge the BitSet if the user already has an ACL entry.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] ChenSammi commented on a change in pull request #1701: HDDS-4585. Support bucket acl operation in S3g

Posted by GitBox <gi...@apache.org>.
ChenSammi commented on a change in pull request #1701:
URL: https://github.com/apache/ozone/pull/1701#discussion_r565893835



##########
File path: hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java
##########
@@ -353,6 +375,215 @@ public MultiDeleteResponse multiDelete(@PathParam("bucket") String bucketName,
     return result;
   }
 
+  /**
+   * Implement acl get.
+   * <p>
+   * see: https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketAcl.html
+   */
+  public S3BucketAcl getAcl(String bucketName)
+      throws OS3Exception, IOException {
+    S3BucketAcl result = new S3BucketAcl();
+    try {
+      OzoneBucket bucket = getBucket(bucketName);
+      OzoneVolume volume = getVolume();
+      S3Owner owner = new S3Owner(volume.getOwner(), volume.getOwner());
+      result.setOwner(owner);
+      // Use set to remove ACLs with different scopes(ACCESS and DEFAULT)
+      Set<Grant> grantSet = new HashSet<>();
+      // Return ACL list
+      for (OzoneAcl acl : bucket.getAcls()) {
+        List<Grant> grants = S3Acl.ozoneNativeAclToS3Acl(acl);
+        grantSet.addAll(grants);
+      }
+      ArrayList<Grant> grantList = new ArrayList<>();
+      grantList.addAll(grantSet);
+      result.setAclList(
+          new S3BucketAcl.AccessControlList(grantList));
+      return result;
+    } catch (OMException ex) {
+      if (ex.getResult() == ResultCodes.BUCKET_NOT_FOUND) {
+        throw S3ErrorTable.newError(S3ErrorTable
+            .NO_SUCH_BUCKET, bucketName);
+      } else if (ex.getResult() == ResultCodes.PERMISSION_DENIED) {
+        throw S3ErrorTable.newError(S3ErrorTable
+            .ACCESS_DENIED, bucketName);
+      } else {
+        LOG.error("Failed to get acl of Bucket " + bucketName, ex);
+        throw S3ErrorTable.newError(S3ErrorTable.INTERNAL_ERROR, bucketName);
+      }
+    }
+  }
+
+  /**
+   * Implement acl put.
+   * <p>
+   * see: https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutBucketAcl.html
+   */
+  public Response putAcl(String bucketName, HttpHeaders httpHeaders,
+      InputStream body) throws IOException, OS3Exception {
+    String grantReads = httpHeaders.getHeaderString(S3Acl.grantRead);
+    String grantWrites = httpHeaders.getHeaderString(S3Acl.grantWrite);
+    String grantReadACP = httpHeaders.getHeaderString(S3Acl.grantReadACP);
+    String grantWriteACP = httpHeaders.getHeaderString(S3Acl.grantWriteACP);
+    String grantFull = httpHeaders.getHeaderString(S3Acl.grantFullControl);
+
+    try {
+      OzoneBucket bucket = getBucket(bucketName);
+      OzoneVolume volume = getVolume();
+
+      List<OzoneAcl> ozoneAclListOnBucket = new ArrayList<>();
+      List<OzoneAcl> ozoneAclListOnVolume = new ArrayList<>();
+
+      if (grantReads == null && grantWrites == null && grantReadACP == null
+          && grantWriteACP == null && grantFull == null) {
+        S3BucketAcl putBucketAclRequest =
+            new PutBucketAclRequestUnmarshaller().readFrom(
+            null, null, null, null, null, body);
+        // Handle grants in body
+        ozoneAclListOnBucket.addAll(
+            S3Acl.s3AclToOzoneNativeAclOnBucket(putBucketAclRequest));
+        ozoneAclListOnVolume.addAll(
+            S3Acl.s3AclToOzoneNativeAclOnVolume(putBucketAclRequest));
+      } else {
+
+        // Handle grants in headers
+        if (grantReads != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantReads,
+              S3Acl.ACLType.READ.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantReads,
+              S3Acl.ACLType.READ.getValue()));
+        }
+        if (grantWrites != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantWrites,
+              S3Acl.ACLType.WRITE.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantWrites,
+              S3Acl.ACLType.WRITE.getValue()));
+        }
+        if (grantReadACP != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantReadACP,
+              S3Acl.ACLType.READ_ACP.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantReadACP,
+              S3Acl.ACLType.READ_ACP.getValue()));
+        }
+        if (grantWriteACP != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantWriteACP,
+              S3Acl.ACLType.WRITE_ACP.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantWriteACP,
+              S3Acl.ACLType.WRITE_ACP.getValue()));
+        }
+        if (grantFull != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantFull,
+              S3Acl.ACLType.FULL_CONTROL.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantFull,
+              S3Acl.ACLType.FULL_CONTROL.getValue()));
+        }
+      }
+
+      List<OzoneAcl> oldBucketAcls = bucket.getAcls();
+      List<OzoneAcl> oldVolumeAcls =
+          S3Acl.getVolumeAclFromBucketAcl(oldBucketAcls);
+
+      // Add new ACLs
+      for (OzoneAcl addAcl : ozoneAclListOnBucket) {
+        bucket.addAcl(addAcl);
+      }
+      for (OzoneAcl addAcl : ozoneAclListOnVolume) {
+        volume.addAcl(addAcl);
+      }
+
+      // A put request will reset all previous ACLs
+      for (OzoneAcl removeAcl : oldBucketAcls) {
+        bucket.removeAcl(removeAcl);
+      }
+      for (OzoneAcl removeAcl : oldVolumeAcls) {
+        volume.removeAcl(removeAcl);
+      }
+    } catch (OMException exception) {
+      LOG.error("Error in set ACL Request for bucket: {}", bucketName,
+          exception);
+      if (exception.getResult() == ResultCodes.BUCKET_NOT_FOUND) {
+        throw S3ErrorTable.newError(S3ErrorTable.NO_SUCH_BUCKET,
+            bucketName);
+      } else if (exception.getResult() == ResultCodes.PERMISSION_DENIED) {
+        throw S3ErrorTable.newError(S3ErrorTable
+            .ACCESS_DENIED, bucketName);
+      }
+      throw exception;
+    }
+    return Response.status(HttpStatus.SC_OK).build();
+  }
+
+  /**
+   *  Example: x-amz-grant-write: \
+   *  uri="http://acs.amazonaws.com/groups/s3/LogDelivery", id="111122223333", \
+   *  id="555566667777".
+   */
+  private List<OzoneAcl> getAndConvertAclOnBucket(String value,
+      String permission) throws OS3Exception {
+    List<OzoneAcl> ozoneAclList = new ArrayList<>();
+    if (StringUtils.isEmpty(value)) {
+      return ozoneAclList;
+    }
+    String[] subValues = value.split(",");
+    for (String acl: subValues) {
+      String[] part = acl.split("=");
+      if (part.length != 2) {
+        throw S3ErrorTable.newError(S3ErrorTable.INVALID_ARGUMENT, acl);
+      }
+      S3Acl.ACLIdentityType type =
+          S3Acl.ACLIdentityType.getTypeFromHeaderType(part[0]);
+      if (type == null || !type.isSupported()) {
+        LOG.warn("S3 grantee {} is null or not supported", part[0]);
+        throw S3ErrorTable.newError(NOT_IMPLEMENTED, part[0]);
+      }
+      // Build ACL on Bucket
+      BitSet aclsOnBucket =
+          S3Acl.getOzoneAclOnBucketFromS3Permission(permission);
+      OzoneAcl defaultOzoneAcl = new OzoneAcl(
+          IAccessAuthorizer.ACLIdentityType.USER, part[1], aclsOnBucket,
+          OzoneAcl.AclScope.DEFAULT);
+      OzoneAcl accessOzoneAcl = new OzoneAcl(
+          IAccessAuthorizer.ACLIdentityType.USER, part[1], aclsOnBucket,
+          OzoneAcl.AclScope.ACCESS);
+      ozoneAclList.add(defaultOzoneAcl);
+      ozoneAclList.add(accessOzoneAcl);
+    }
+    return ozoneAclList;
+  }
+
+  private List<OzoneAcl> getAndConvertAclOnVolume(String value,
+      String permission) throws OS3Exception {
+    List<OzoneAcl> ozoneAclList = new ArrayList<>();
+    if (StringUtils.isEmpty(value)) {
+      return ozoneAclList;
+    }
+    String[] subValues = value.split(",");
+    for (String acl: subValues) {
+      String[] part = acl.split("=");
+      if (part.length != 2) {
+        throw S3ErrorTable.newError(S3ErrorTable.INVALID_ARGUMENT, acl);
+      }
+      S3Acl.ACLIdentityType type =
+          S3Acl.ACLIdentityType.getTypeFromHeaderType(part[0]);
+      if (type == null || !type.isSupported()) {
+        LOG.warn("S3 grantee {} is null or not supported", part[0]);
+        throw S3ErrorTable.newError(NOT_IMPLEMENTED, part[0]);
+      }
+      // Build ACL on Bucket

Review comment:
       Right. 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] ChenSammi commented on pull request #1701: HDDS-4585. Support bucket acl operation in S3g

Posted by GitBox <gi...@apache.org>.
ChenSammi commented on pull request #1701:
URL: https://github.com/apache/ozone/pull/1701#issuecomment-769544034


   misc acceptance exit with error code 1, while there is no obvious error.  Can anyone tell me how to do a further investigation. 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] ChenSammi commented on a change in pull request #1701: HDDS-4585. Support bucket acl operation in S3g

Posted by GitBox <gi...@apache.org>.
ChenSammi commented on a change in pull request #1701:
URL: https://github.com/apache/ozone/pull/1701#discussion_r569247353



##########
File path: hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java
##########
@@ -353,6 +373,204 @@ public MultiDeleteResponse multiDelete(@PathParam("bucket") String bucketName,
     return result;
   }
 
+  /**
+   * Implement acl get.
+   * <p>
+   * see: https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketAcl.html
+   */
+  public S3BucketAcl getAcl(String bucketName)
+      throws OS3Exception, IOException {
+    S3BucketAcl result = new S3BucketAcl();
+    try {
+      OzoneBucket bucket = getBucket(bucketName);
+      OzoneVolume volume = getVolume();
+      // TODO: use bucket owner instead of volume owner here once bucket owner
+      // TODO: is supported.
+      S3Owner owner = new S3Owner(volume.getOwner(), volume.getOwner());
+      result.setOwner(owner);
+
+      // TODO: remove this duplication avoid logic when ACCESS and DEFAULT scope
+      // TODO: are merged.
+      // Use set to remove ACLs with different scopes(ACCESS and DEFAULT)
+      Set<Grant> grantSet = new HashSet<>();
+      // Return ACL list
+      for (OzoneAcl acl : bucket.getAcls()) {
+        List<Grant> grants = S3Acl.ozoneNativeAclToS3Acl(acl);
+        grantSet.addAll(grants);
+      }
+      ArrayList<Grant> grantList = new ArrayList<>();
+      grantList.addAll(grantSet);
+      result.setAclList(
+          new S3BucketAcl.AccessControlList(grantList));
+      return result;
+    } catch (OMException ex) {
+      if (ex.getResult() == ResultCodes.BUCKET_NOT_FOUND) {
+        throw S3ErrorTable.newError(S3ErrorTable
+            .NO_SUCH_BUCKET, bucketName);
+      } else if (ex.getResult() == ResultCodes.PERMISSION_DENIED) {
+        throw S3ErrorTable.newError(S3ErrorTable
+            .ACCESS_DENIED, bucketName);
+      } else {
+        LOG.error("Failed to get acl of Bucket " + bucketName, ex);
+        throw S3ErrorTable.newError(S3ErrorTable.INTERNAL_ERROR, bucketName);
+      }
+    }
+  }
+
+  /**
+   * Implement acl put.
+   * <p>
+   * see: https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutBucketAcl.html
+   */
+  public Response putAcl(String bucketName, HttpHeaders httpHeaders,
+      InputStream body) throws IOException, OS3Exception {
+    String grantReads = httpHeaders.getHeaderString(S3Acl.grantRead);
+    String grantWrites = httpHeaders.getHeaderString(S3Acl.grantWrite);
+    String grantReadACP = httpHeaders.getHeaderString(S3Acl.grantReadACP);
+    String grantWriteACP = httpHeaders.getHeaderString(S3Acl.grantWriteACP);
+    String grantFull = httpHeaders.getHeaderString(S3Acl.grantFullControl);
+
+    try {
+      OzoneBucket bucket = getBucket(bucketName);
+      OzoneVolume volume = getVolume();
+
+      List<OzoneAcl> ozoneAclListOnBucket = new ArrayList<>();
+      List<OzoneAcl> ozoneAclListOnVolume = new ArrayList<>();
+
+      if (grantReads == null && grantWrites == null && grantReadACP == null
+          && grantWriteACP == null && grantFull == null) {
+        S3BucketAcl putBucketAclRequest =
+            new PutBucketAclRequestUnmarshaller().readFrom(
+            null, null, null, null, null, body);
+        // Handle grants in body
+        ozoneAclListOnBucket.addAll(
+            S3Acl.s3AclToOzoneNativeAclOnBucket(putBucketAclRequest));
+        ozoneAclListOnVolume.addAll(
+            S3Acl.s3AclToOzoneNativeAclOnVolume(putBucketAclRequest));
+      } else {
+
+        // Handle grants in headers
+        if (grantReads != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantReads,
+              S3Acl.ACLType.READ.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantReads,
+              S3Acl.ACLType.READ.getValue()));
+        }
+        if (grantWrites != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantWrites,
+              S3Acl.ACLType.WRITE.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantWrites,
+              S3Acl.ACLType.WRITE.getValue()));
+        }
+        if (grantReadACP != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantReadACP,
+              S3Acl.ACLType.READ_ACP.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantReadACP,
+              S3Acl.ACLType.READ_ACP.getValue()));
+        }
+        if (grantWriteACP != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantWriteACP,
+              S3Acl.ACLType.WRITE_ACP.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantWriteACP,
+              S3Acl.ACLType.WRITE_ACP.getValue()));
+        }
+        if (grantFull != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantFull,
+              S3Acl.ACLType.FULL_CONTROL.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantFull,
+              S3Acl.ACLType.FULL_CONTROL.getValue()));
+        }
+      }
+
+      // A put request will reset all previous ACLs
+      bucket.setAcl(ozoneAclListOnBucket);
+      volume.setAcl(ozoneAclListOnVolume);

Review comment:
       Yes, we need to setup ACL on a volume for S3 bucket.  Here we grant the user least privilege on volume in order to make sure user can access the bucket. If a user has full control of the bucket while he doesn't have any permission on volume, user will fail to execute some operations, such as bucket list. 
   
   The thing is object hierarchy of Ozone is /volume/bucket/key.  ACL on Ozone has the hierarchical characteristic.  ACL of parent will impact the accessibility of it's child.  
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] xiaoyuyao commented on a change in pull request #1701: HDDS-4585. Support bucket acl operation in S3g

Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #1701:
URL: https://github.com/apache/ozone/pull/1701#discussion_r562382043



##########
File path: hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/S3Acl.java
##########
@@ -0,0 +1,388 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+package org.apache.hadoop.ozone.s3.endpoint;
+
+import org.apache.hadoop.ozone.OzoneAcl;
+import org.apache.hadoop.ozone.s3.endpoint.S3BucketAcl.Grant;
+import org.apache.hadoop.ozone.s3.endpoint.S3BucketAcl.Grantee;
+import org.apache.hadoop.ozone.s3.exception.OS3Exception;
+import org.apache.hadoop.ozone.s3.exception.S3ErrorTable;
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.ArrayList;
+import java.util.BitSet;
+import java.util.List;
+
+import static org.apache.hadoop.ozone.s3.exception.S3ErrorTable.INVALID_ARGUMENT;
+import static org.apache.hadoop.ozone.s3.exception.S3ErrorTable.NOT_IMPLEMENTED;
+
+public final class S3Acl {
+  private static final Logger LOG = LoggerFactory.getLogger(S3Acl.class);
+
+  // ACL put related headers
+  public static final String grantRead = "x-amz-grant-read";
+  public static final String grantWrite = "x-amz-grant-write";
+  public static final String grantReadACP = "x-amz-grant-read-acp";
+  public static final String grantWriteACP = "x-amz-grant-write-acp";
+  public static final String grantFullControl = "x-amz-grant-full-control";
+
+  // Not supported headers at current stage, may support it in future
+  public static final String cannedAclHeader = "x-amz-acl";
+
+  /**
+   * https://docs.aws.amazon.com/AmazonS3/latest/dev/acl-overview.html.
+   */
+  enum ACLType {
+    // Allows grantee to list the objects in the bucket
+    READ("READ"),
+    // Allows grantee to create, overwrite, and delete any object in the bucket
+    WRITE("WRITE"),
+    // Allows grantee to write the bucket ACL
+    READ_ACP("READ_ACP"),
+    // Allows grantee to write the ACL for the applicable bucket
+    WRITE_ACP("WRITE_ACP"),
+    // Allows grantee above all permissions on the bucket
+    FULL_CONTROL("FULL_CONTROL");
+
+    public String getValue() {
+      return value;
+    }
+    /**
+     * String value for this Enum.
+     */
+    private final String value;
+
+    /**
+     * @param val String type for this enum.
+     */
+    ACLType(String val) {
+      value = val;
+    }
+
+
+    public static ACLType getType(String typeStr) {
+      for(ACLType type: ACLType.values()) {
+        if (type.getValue().equals(typeStr)) {
+          return type;
+        }
+      }
+      return null;
+    }
+  }
+
+  enum ACLIdentityType {
+    USER("CanonicalUser", true, "id"),
+    GROUP("Group", false, "url"),
+    USER_BY_EMAIL("AmazonCustomerByEmail", false, "emailAddress");
+
+    public String getGranteeType() {
+      return granteeType;
+    }
+
+    public String getHeaderType() {
+      return granteeInHeader;
+    }
+
+    /**
+     *  Grantee type in body XML.
+     */
+    private final String granteeType;
+
+    /**
+     * Is this type supported or not.
+     */
+    private final boolean supported;
+
+    /**
+     * Grantee type in header.
+     */
+    private final String granteeInHeader;
+
+    /**
+     * Init OzoneACLtypes enum.
+     *
+     * @param val String type for this enum.
+     */
+    ACLIdentityType(String val, boolean support, String headerType) {
+      granteeType = val;
+      supported = support;
+      granteeInHeader = headerType;
+    }
+
+    boolean isSupported() {
+      return supported;
+    }
+
+    public static ACLIdentityType getTypeFromGranteeType(String typeStr) {
+      for(ACLIdentityType type: ACLIdentityType.values()) {
+        if (type.getGranteeType().equals(typeStr)) {
+          return type;
+        }
+      }
+      return null;
+    }
+
+    public static ACLIdentityType getTypeFromHeaderType(String typeStr) {
+      for(ACLIdentityType type: ACLIdentityType.values()) {
+        if (type.getHeaderType().equals(typeStr)) {
+          return type;
+        }
+      }
+      return null;
+    }
+  }
+
+  public static boolean isGranteeTypeSupported(String typeStr) {
+    ACLIdentityType type =  ACLIdentityType.getTypeFromGranteeType(typeStr);
+    return type == null ? false : type.isSupported();
+  }
+
+  public static boolean isHeaderTypeSupported(String typeStr) {
+    ACLIdentityType type =  ACLIdentityType.getTypeFromHeaderType(typeStr);
+    return type == null ? false : type.isSupported();
+  }
+
+  public static List<Grant> ozoneNativeAclToS3Acl(OzoneAcl ozoneAcl) {
+    // Since currently only "CanonicalUser" is supported, which maps to Ozone
+    // "USER"
+    List<Grant> grantList = new ArrayList<>();
+    if (ozoneAcl.getType() != IAccessAuthorizer.ACLIdentityType.USER) {
+      return grantList;
+    }
+
+    Grantee grantee = new Grantee();
+    grantee.setDisplayName(ozoneAcl.getName());
+    grantee.setId(ozoneAcl.getName());
+
+    List<IAccessAuthorizer.ACLType> acls = ozoneAcl.getAclList();
+    if (acls.contains(IAccessAuthorizer.ACLType.ALL)) {
+      Grant grant = new Grant();
+      grant.setGrantee(grantee);
+      grant.setPermission(ACLType.FULL_CONTROL.toString());
+      grantList.add(grant);
+      return grantList;
+    } else if (acls.contains(IAccessAuthorizer.ACLType.WRITE_ACL)) {
+      Grant grant = new Grant();
+      grant.setGrantee(grantee);
+      grant.setPermission(ACLType.WRITE_ACP.toString());
+      grantList.add(grant);
+    } else if (acls.contains(IAccessAuthorizer.ACLType.READ_ACL)) {
+      Grant grant = new Grant();
+      grant.setGrantee(grantee);
+      grant.setPermission(ACLType.READ_ACP.toString());
+      grantList.add(grant);
+    } else if (acls.contains(IAccessAuthorizer.ACLType.WRITE) &&
+        acls.contains(IAccessAuthorizer.ACLType.DELETE) &&
+        acls.contains(IAccessAuthorizer.ACLType.CREATE)) {
+      Grant grant = new Grant();
+      grant.setGrantee(grantee);
+      grant.setPermission(ACLType.WRITE.toString());
+      grantList.add(grant);
+    } else if (acls.contains(IAccessAuthorizer.ACLType.READ) &&
+        acls.contains(IAccessAuthorizer.ACLType.LIST)) {
+      Grant grant = new Grant();
+      grant.setGrantee(grantee);
+      grant.setPermission(ACLType.READ.toString());
+      grantList.add(grant);
+    } else {
+      LOG.error("Cannot find a good mapping for Ozone ACL {} to S3",
+          ozoneAcl.toString());
+    }
+    return grantList;
+  }
+
+  public static List<OzoneAcl> s3AclToOzoneNativeAclOnBucket(
+      S3BucketAcl bucketAcl) throws OS3Exception {
+    List<OzoneAcl> ozoneAclList = new ArrayList<>();
+    List<Grant> grantList = bucketAcl.getAclList().getGrantList();
+    for (Grant grant : grantList) {
+      //  Only "CanonicalUser" is supported, which maps to Ozone "USER"
+      ACLIdentityType identityType = ACLIdentityType.getTypeFromGranteeType(
+          grant.getGrantee().getXsiType());
+      if (identityType != null && identityType.isSupported()) {
+        String permission = grant.getPermission();
+        BitSet acls = getOzoneAclOnBucketFromS3Permission(permission);
+        OzoneAcl defaultOzoneAcl = new OzoneAcl(
+            IAccessAuthorizer.ACLIdentityType.USER,
+            grant.getGrantee().getId(), acls,
+            OzoneAcl.AclScope.DEFAULT);
+        OzoneAcl accessOzoneAcl = new OzoneAcl(
+            IAccessAuthorizer.ACLIdentityType.USER,
+            grant.getGrantee().getId(), acls,
+            OzoneAcl.AclScope.ACCESS);
+        ozoneAclList.add(defaultOzoneAcl);
+        ozoneAclList.add(accessOzoneAcl);
+      } else {
+        LOG.error("Grantee type {} is not supported",
+            grant.getGrantee().getXsiType());
+        throw S3ErrorTable.newError(NOT_IMPLEMENTED,
+            grant.getGrantee().getXsiType());
+      }
+    }
+    return ozoneAclList;
+  }
+
+  public static BitSet getOzoneAclOnBucketFromS3Permission(String permission)
+      throws OS3Exception {
+    ACLType permissionType = ACLType.getType(permission);
+    if (permissionType == null) {
+      throw S3ErrorTable.newError(S3ErrorTable.INVALID_ARGUMENT, permission);
+    }
+    BitSet acls = new BitSet(IAccessAuthorizer.ACLType.getNoOfAcls());
+    switch (permissionType) {
+    case FULL_CONTROL:
+      acls.set(IAccessAuthorizer.ACLType.ALL.ordinal());
+      break;
+    case WRITE_ACP:
+      acls.set(IAccessAuthorizer.ACLType.WRITE_ACL.ordinal());
+      break;
+    case READ_ACP:
+      acls.set(IAccessAuthorizer.ACLType.READ_ACL.ordinal());
+      break;
+    case WRITE:
+      acls.set(IAccessAuthorizer.ACLType.WRITE.ordinal());
+      acls.set(IAccessAuthorizer.ACLType.DELETE.ordinal());
+      acls.set(IAccessAuthorizer.ACLType.CREATE.ordinal());
+      break;
+    case READ:
+      acls.set(IAccessAuthorizer.ACLType.READ.ordinal());
+      acls.set(IAccessAuthorizer.ACLType.LIST.ordinal());
+      break;
+    default:
+      LOG.error("Failed to recognize S3 permission {}", permission);
+      throw S3ErrorTable.newError(INVALID_ARGUMENT, permission);
+    }
+    return acls;
+  }
+
+  public static List<OzoneAcl> s3AclToOzoneNativeAclOnVolume(
+      S3BucketAcl bucketAcl) throws OS3Exception {
+    List<OzoneAcl> ozoneAclList = new ArrayList<>();
+    List<Grant> grantList = bucketAcl.getAclList().getGrantList();
+    for (Grant grant : grantList) {
+      //  Only "CanonicalUser" is supported, which maps to Ozone "USER"
+      ACLIdentityType identityType = ACLIdentityType.getTypeFromGranteeType(
+          grant.getGrantee().getXsiType());
+      if (identityType != null && identityType.isSupported()) {
+        String permission = grant.getPermission();
+        BitSet acls = getOzoneAclOnVolumeFromS3Permission(permission);
+        OzoneAcl defaultOzoneAcl = new OzoneAcl(
+            IAccessAuthorizer.ACLIdentityType.USER,

Review comment:
       s3 acl may have multiple grants for the same user. When it is converted to OzoneAcl, we should combine to take the advantage of BitSet.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] ChenSammi commented on a change in pull request #1701: HDDS-4585. Support bucket acl operation in S3g

Posted by GitBox <gi...@apache.org>.
ChenSammi commented on a change in pull request #1701:
URL: https://github.com/apache/ozone/pull/1701#discussion_r616444975



##########
File path: hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java
##########
@@ -353,6 +373,204 @@ public MultiDeleteResponse multiDelete(@PathParam("bucket") String bucketName,
     return result;
   }
 
+  /**
+   * Implement acl get.
+   * <p>
+   * see: https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketAcl.html
+   */
+  public S3BucketAcl getAcl(String bucketName)
+      throws OS3Exception, IOException {
+    S3BucketAcl result = new S3BucketAcl();
+    try {
+      OzoneBucket bucket = getBucket(bucketName);
+      OzoneVolume volume = getVolume();
+      // TODO: use bucket owner instead of volume owner here once bucket owner
+      // TODO: is supported.
+      S3Owner owner = new S3Owner(volume.getOwner(), volume.getOwner());
+      result.setOwner(owner);
+
+      // TODO: remove this duplication avoid logic when ACCESS and DEFAULT scope
+      // TODO: are merged.
+      // Use set to remove ACLs with different scopes(ACCESS and DEFAULT)
+      Set<Grant> grantSet = new HashSet<>();
+      // Return ACL list
+      for (OzoneAcl acl : bucket.getAcls()) {
+        List<Grant> grants = S3Acl.ozoneNativeAclToS3Acl(acl);
+        grantSet.addAll(grants);
+      }
+      ArrayList<Grant> grantList = new ArrayList<>();
+      grantList.addAll(grantSet);
+      result.setAclList(
+          new S3BucketAcl.AccessControlList(grantList));
+      return result;
+    } catch (OMException ex) {
+      if (ex.getResult() == ResultCodes.BUCKET_NOT_FOUND) {
+        throw S3ErrorTable.newError(S3ErrorTable
+            .NO_SUCH_BUCKET, bucketName);
+      } else if (ex.getResult() == ResultCodes.PERMISSION_DENIED) {
+        throw S3ErrorTable.newError(S3ErrorTable
+            .ACCESS_DENIED, bucketName);
+      } else {
+        LOG.error("Failed to get acl of Bucket " + bucketName, ex);
+        throw S3ErrorTable.newError(S3ErrorTable.INTERNAL_ERROR, bucketName);
+      }
+    }
+  }
+
+  /**
+   * Implement acl put.
+   * <p>
+   * see: https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutBucketAcl.html
+   */
+  public Response putAcl(String bucketName, HttpHeaders httpHeaders,
+      InputStream body) throws IOException, OS3Exception {
+    String grantReads = httpHeaders.getHeaderString(S3Acl.GRANT_READ);
+    String grantWrites = httpHeaders.getHeaderString(S3Acl.GRANT_WRITE);
+    String grantReadACP = httpHeaders.getHeaderString(S3Acl.GRANT_READ_CAP);
+    String grantWriteACP = httpHeaders.getHeaderString(S3Acl.GRANT_WRITE_CAP);
+    String grantFull = httpHeaders.getHeaderString(S3Acl.GRANT_FULL_CONTROL);
+
+    try {
+      OzoneBucket bucket = getBucket(bucketName);
+      OzoneVolume volume = getVolume();
+
+      List<OzoneAcl> ozoneAclListOnBucket = new ArrayList<>();
+      List<OzoneAcl> ozoneAclListOnVolume = new ArrayList<>();
+
+      if (grantReads == null && grantWrites == null && grantReadACP == null
+          && grantWriteACP == null && grantFull == null) {
+        S3BucketAcl putBucketAclRequest =
+            new PutBucketAclRequestUnmarshaller().readFrom(
+            null, null, null, null, null, body);
+        // Handle grants in body
+        ozoneAclListOnBucket.addAll(
+            S3Acl.s3AclToOzoneNativeAclOnBucket(putBucketAclRequest));
+        ozoneAclListOnVolume.addAll(
+            S3Acl.s3AclToOzoneNativeAclOnVolume(putBucketAclRequest));
+      } else {
+
+        // Handle grants in headers
+        if (grantReads != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantReads,
+              S3Acl.ACLType.READ.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantReads,
+              S3Acl.ACLType.READ.getValue()));
+        }
+        if (grantWrites != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantWrites,
+              S3Acl.ACLType.WRITE.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantWrites,
+              S3Acl.ACLType.WRITE.getValue()));
+        }
+        if (grantReadACP != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantReadACP,
+              S3Acl.ACLType.READ_ACP.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantReadACP,
+              S3Acl.ACLType.READ_ACP.getValue()));
+        }
+        if (grantWriteACP != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantWriteACP,
+              S3Acl.ACLType.WRITE_ACP.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantWriteACP,
+              S3Acl.ACLType.WRITE_ACP.getValue()));
+        }
+        if (grantFull != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantFull,
+              S3Acl.ACLType.FULL_CONTROL.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantFull,
+              S3Acl.ACLType.FULL_CONTROL.getValue()));
+        }
+      }
+
+      // A put request will reset all previous ACLs
+      bucket.setAcl(ozoneAclListOnBucket);
+      volume.setAcl(ozoneAclListOnVolume);
+    } catch (OMException exception) {
+      LOG.error("Error in set ACL Request for bucket: {}", bucketName,
+          exception);
+      if (exception.getResult() == ResultCodes.BUCKET_NOT_FOUND) {
+        throw S3ErrorTable.newError(S3ErrorTable.NO_SUCH_BUCKET,
+            bucketName);
+      } else if (exception.getResult() == ResultCodes.PERMISSION_DENIED) {
+        throw S3ErrorTable.newError(S3ErrorTable
+            .ACCESS_DENIED, bucketName);
+      }
+      throw exception;
+    }
+    return Response.status(HttpStatus.SC_OK).build();
+  }
+
+  /**
+   *  Example: x-amz-grant-write: \
+   *  uri="http://acs.amazonaws.com/groups/s3/LogDelivery", id="111122223333", \
+   *  id="555566667777".
+   */
+  private List<OzoneAcl> getAndConvertAclOnBucket(String value,
+      String permission) throws OS3Exception {
+    List<OzoneAcl> ozoneAclList = new ArrayList<>();
+    if (StringUtils.isEmpty(value)) {
+      return ozoneAclList;
+    }
+    String[] subValues = value.split(",");
+    for (String acl: subValues) {
+      String[] part = acl.split("=");
+      if (part.length != 2) {
+        throw S3ErrorTable.newError(S3ErrorTable.INVALID_ARGUMENT, acl);
+      }
+      S3Acl.ACLIdentityType type =
+          S3Acl.ACLIdentityType.getTypeFromHeaderType(part[0]);
+      if (type == null || !type.isSupported()) {
+        LOG.warn("S3 grantee {} is null or not supported", part[0]);
+        throw S3ErrorTable.newError(NOT_IMPLEMENTED, part[0]);
+      }
+      // Build ACL on Bucket
+      BitSet aclsOnBucket =
+          S3Acl.getOzoneAclOnBucketFromS3Permission(permission);
+      OzoneAcl defaultOzoneAcl = new OzoneAcl(
+          IAccessAuthorizer.ACLIdentityType.USER, part[1], aclsOnBucket,
+          OzoneAcl.AclScope.DEFAULT);
+      OzoneAcl accessOzoneAcl = new OzoneAcl(
+          IAccessAuthorizer.ACLIdentityType.USER, part[1], aclsOnBucket,
+          OzoneAcl.AclScope.ACCESS);
+      ozoneAclList.add(defaultOzoneAcl);
+      ozoneAclList.add(accessOzoneAcl);
+    }
+    return ozoneAclList;
+  }
+
+  private List<OzoneAcl> getAndConvertAclOnVolume(String value,
+      String permission) throws OS3Exception {
+    List<OzoneAcl> ozoneAclList = new ArrayList<>();
+    if (StringUtils.isEmpty(value)) {
+      return ozoneAclList;
+    }
+    String[] subValues = value.split(",");
+    for (String acl: subValues) {
+      String[] part = acl.split("=");
+      if (part.length != 2) {
+        throw S3ErrorTable.newError(S3ErrorTable.INVALID_ARGUMENT, acl);
+      }
+      S3Acl.ACLIdentityType type =
+          S3Acl.ACLIdentityType.getTypeFromHeaderType(part[0]);
+      if (type == null || !type.isSupported()) {
+        LOG.warn("S3 grantee {} is null or not supported", part[0]);
+        throw S3ErrorTable.newError(NOT_IMPLEMENTED, part[0]);
+      }
+      // Build ACL on Volume
+      BitSet aclsOnVolume =
+          S3Acl.getOzoneAclOnVolumeFromS3Permission(permission);
+      OzoneAcl defaultOzoneAcl = new OzoneAcl(
+          IAccessAuthorizer.ACLIdentityType.USER, part[1], aclsOnVolume,

Review comment:
       Good catch.  DEFAULT ACL on volume is not necessary. 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] ChenSammi commented on a change in pull request #1701: HDDS-4585. Support bucket acl operation in S3g

Posted by GitBox <gi...@apache.org>.
ChenSammi commented on a change in pull request #1701:
URL: https://github.com/apache/ozone/pull/1701#discussion_r565916883



##########
File path: hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java
##########
@@ -353,6 +375,215 @@ public MultiDeleteResponse multiDelete(@PathParam("bucket") String bucketName,
     return result;
   }
 
+  /**
+   * Implement acl get.
+   * <p>
+   * see: https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketAcl.html
+   */
+  public S3BucketAcl getAcl(String bucketName)
+      throws OS3Exception, IOException {
+    S3BucketAcl result = new S3BucketAcl();
+    try {
+      OzoneBucket bucket = getBucket(bucketName);
+      OzoneVolume volume = getVolume();
+      S3Owner owner = new S3Owner(volume.getOwner(), volume.getOwner());
+      result.setOwner(owner);
+      // Use set to remove ACLs with different scopes(ACCESS and DEFAULT)
+      Set<Grant> grantSet = new HashSet<>();
+      // Return ACL list
+      for (OzoneAcl acl : bucket.getAcls()) {
+        List<Grant> grants = S3Acl.ozoneNativeAclToS3Acl(acl);

Review comment:
       Yes, with the duplicate ACL issue fixed, I don't need this set to list conversion now. 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] ChenSammi commented on pull request #1701: HDDS-4585. Support bucket acl operation in S3g

Posted by GitBox <gi...@apache.org>.
ChenSammi commented on pull request #1701:
URL: https://github.com/apache/ozone/pull/1701#issuecomment-833190259


   Thanks @xiaoyuyao  for the code review.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] xiaoyuyao commented on a change in pull request #1701: HDDS-4585. Support bucket acl operation in S3g

Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #1701:
URL: https://github.com/apache/ozone/pull/1701#discussion_r612630356



##########
File path: hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java
##########
@@ -353,6 +373,204 @@ public MultiDeleteResponse multiDelete(@PathParam("bucket") String bucketName,
     return result;
   }
 
+  /**
+   * Implement acl get.
+   * <p>
+   * see: https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketAcl.html
+   */
+  public S3BucketAcl getAcl(String bucketName)
+      throws OS3Exception, IOException {
+    S3BucketAcl result = new S3BucketAcl();
+    try {
+      OzoneBucket bucket = getBucket(bucketName);
+      OzoneVolume volume = getVolume();
+      // TODO: use bucket owner instead of volume owner here once bucket owner
+      // TODO: is supported.
+      S3Owner owner = new S3Owner(volume.getOwner(), volume.getOwner());
+      result.setOwner(owner);
+
+      // TODO: remove this duplication avoid logic when ACCESS and DEFAULT scope
+      // TODO: are merged.
+      // Use set to remove ACLs with different scopes(ACCESS and DEFAULT)
+      Set<Grant> grantSet = new HashSet<>();
+      // Return ACL list
+      for (OzoneAcl acl : bucket.getAcls()) {
+        List<Grant> grants = S3Acl.ozoneNativeAclToS3Acl(acl);
+        grantSet.addAll(grants);
+      }
+      ArrayList<Grant> grantList = new ArrayList<>();
+      grantList.addAll(grantSet);
+      result.setAclList(
+          new S3BucketAcl.AccessControlList(grantList));
+      return result;
+    } catch (OMException ex) {
+      if (ex.getResult() == ResultCodes.BUCKET_NOT_FOUND) {
+        throw S3ErrorTable.newError(S3ErrorTable
+            .NO_SUCH_BUCKET, bucketName);
+      } else if (ex.getResult() == ResultCodes.PERMISSION_DENIED) {
+        throw S3ErrorTable.newError(S3ErrorTable
+            .ACCESS_DENIED, bucketName);
+      } else {
+        LOG.error("Failed to get acl of Bucket " + bucketName, ex);
+        throw S3ErrorTable.newError(S3ErrorTable.INTERNAL_ERROR, bucketName);
+      }
+    }
+  }
+
+  /**
+   * Implement acl put.
+   * <p>
+   * see: https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutBucketAcl.html
+   */
+  public Response putAcl(String bucketName, HttpHeaders httpHeaders,
+      InputStream body) throws IOException, OS3Exception {
+    String grantReads = httpHeaders.getHeaderString(S3Acl.GRANT_READ);
+    String grantWrites = httpHeaders.getHeaderString(S3Acl.GRANT_WRITE);
+    String grantReadACP = httpHeaders.getHeaderString(S3Acl.GRANT_READ_CAP);
+    String grantWriteACP = httpHeaders.getHeaderString(S3Acl.GRANT_WRITE_CAP);
+    String grantFull = httpHeaders.getHeaderString(S3Acl.GRANT_FULL_CONTROL);
+
+    try {
+      OzoneBucket bucket = getBucket(bucketName);
+      OzoneVolume volume = getVolume();
+
+      List<OzoneAcl> ozoneAclListOnBucket = new ArrayList<>();
+      List<OzoneAcl> ozoneAclListOnVolume = new ArrayList<>();
+
+      if (grantReads == null && grantWrites == null && grantReadACP == null
+          && grantWriteACP == null && grantFull == null) {
+        S3BucketAcl putBucketAclRequest =
+            new PutBucketAclRequestUnmarshaller().readFrom(
+            null, null, null, null, null, body);
+        // Handle grants in body
+        ozoneAclListOnBucket.addAll(
+            S3Acl.s3AclToOzoneNativeAclOnBucket(putBucketAclRequest));
+        ozoneAclListOnVolume.addAll(
+            S3Acl.s3AclToOzoneNativeAclOnVolume(putBucketAclRequest));
+      } else {
+
+        // Handle grants in headers
+        if (grantReads != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantReads,
+              S3Acl.ACLType.READ.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantReads,
+              S3Acl.ACLType.READ.getValue()));
+        }
+        if (grantWrites != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantWrites,
+              S3Acl.ACLType.WRITE.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantWrites,
+              S3Acl.ACLType.WRITE.getValue()));
+        }
+        if (grantReadACP != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantReadACP,
+              S3Acl.ACLType.READ_ACP.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantReadACP,
+              S3Acl.ACLType.READ_ACP.getValue()));
+        }
+        if (grantWriteACP != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantWriteACP,
+              S3Acl.ACLType.WRITE_ACP.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantWriteACP,
+              S3Acl.ACLType.WRITE_ACP.getValue()));
+        }
+        if (grantFull != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantFull,
+              S3Acl.ACLType.FULL_CONTROL.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantFull,
+              S3Acl.ACLType.FULL_CONTROL.getValue()));
+        }
+      }
+
+      // A put request will reset all previous ACLs
+      bucket.setAcl(ozoneAclListOnBucket);
+      volume.setAcl(ozoneAclListOnVolume);
+    } catch (OMException exception) {
+      LOG.error("Error in set ACL Request for bucket: {}", bucketName,
+          exception);
+      if (exception.getResult() == ResultCodes.BUCKET_NOT_FOUND) {
+        throw S3ErrorTable.newError(S3ErrorTable.NO_SUCH_BUCKET,
+            bucketName);
+      } else if (exception.getResult() == ResultCodes.PERMISSION_DENIED) {
+        throw S3ErrorTable.newError(S3ErrorTable
+            .ACCESS_DENIED, bucketName);
+      }
+      throw exception;
+    }
+    return Response.status(HttpStatus.SC_OK).build();
+  }
+
+  /**
+   *  Example: x-amz-grant-write: \
+   *  uri="http://acs.amazonaws.com/groups/s3/LogDelivery", id="111122223333", \
+   *  id="555566667777".
+   */
+  private List<OzoneAcl> getAndConvertAclOnBucket(String value,
+      String permission) throws OS3Exception {
+    List<OzoneAcl> ozoneAclList = new ArrayList<>();
+    if (StringUtils.isEmpty(value)) {
+      return ozoneAclList;
+    }
+    String[] subValues = value.split(",");
+    for (String acl: subValues) {
+      String[] part = acl.split("=");
+      if (part.length != 2) {
+        throw S3ErrorTable.newError(S3ErrorTable.INVALID_ARGUMENT, acl);
+      }
+      S3Acl.ACLIdentityType type =
+          S3Acl.ACLIdentityType.getTypeFromHeaderType(part[0]);
+      if (type == null || !type.isSupported()) {
+        LOG.warn("S3 grantee {} is null or not supported", part[0]);
+        throw S3ErrorTable.newError(NOT_IMPLEMENTED, part[0]);
+      }
+      // Build ACL on Bucket
+      BitSet aclsOnBucket =
+          S3Acl.getOzoneAclOnBucketFromS3Permission(permission);
+      OzoneAcl defaultOzoneAcl = new OzoneAcl(
+          IAccessAuthorizer.ACLIdentityType.USER, part[1], aclsOnBucket,
+          OzoneAcl.AclScope.DEFAULT);
+      OzoneAcl accessOzoneAcl = new OzoneAcl(
+          IAccessAuthorizer.ACLIdentityType.USER, part[1], aclsOnBucket,
+          OzoneAcl.AclScope.ACCESS);
+      ozoneAclList.add(defaultOzoneAcl);
+      ozoneAclList.add(accessOzoneAcl);
+    }
+    return ozoneAclList;
+  }
+
+  private List<OzoneAcl> getAndConvertAclOnVolume(String value,
+      String permission) throws OS3Exception {
+    List<OzoneAcl> ozoneAclList = new ArrayList<>();
+    if (StringUtils.isEmpty(value)) {
+      return ozoneAclList;
+    }
+    String[] subValues = value.split(",");
+    for (String acl: subValues) {
+      String[] part = acl.split("=");
+      if (part.length != 2) {
+        throw S3ErrorTable.newError(S3ErrorTable.INVALID_ARGUMENT, acl);
+      }
+      S3Acl.ACLIdentityType type =
+          S3Acl.ACLIdentityType.getTypeFromHeaderType(part[0]);
+      if (type == null || !type.isSupported()) {
+        LOG.warn("S3 grantee {} is null or not supported", part[0]);
+        throw S3ErrorTable.newError(NOT_IMPLEMENTED, part[0]);
+      }
+      // Build ACL on Volume
+      BitSet aclsOnVolume =
+          S3Acl.getOzoneAclOnVolumeFromS3Permission(permission);
+      OzoneAcl defaultOzoneAcl = new OzoneAcl(
+          IAccessAuthorizer.ACLIdentityType.USER, part[1], aclsOnVolume,

Review comment:
       I don't this we should populate DEFAULT ACL on Volume with the same permission. This will affect all new buckets create after that. 
   
   For example if user **foo** change its bucket acl to allow full control of user **bar** on bucket1, but as a result of that, the default acl for user **bar** with full control was added on volume. user **bar**  will will gain full control on all volume/buckets created subsequently. 
   
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] ChenSammi commented on a change in pull request #1701: HDDS-4585. Support bucket acl operation in S3g

Posted by GitBox <gi...@apache.org>.
ChenSammi commented on a change in pull request #1701:
URL: https://github.com/apache/ozone/pull/1701#discussion_r565915429



##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneVolume.java
##########
@@ -261,7 +270,37 @@ public Instant getModificationTime() {
    * @return aclMap
    */
   public List<OzoneAcl> getAcls() {
-    return acls;
+    return (ArrayList)((ArrayList)acls).clone();

Review comment:
       Sure.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] ChenSammi commented on a change in pull request #1701: HDDS-4585. Support bucket acl operation in S3g

Posted by GitBox <gi...@apache.org>.
ChenSammi commented on a change in pull request #1701:
URL: https://github.com/apache/ozone/pull/1701#discussion_r565902941



##########
File path: hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java
##########
@@ -353,6 +375,215 @@ public MultiDeleteResponse multiDelete(@PathParam("bucket") String bucketName,
     return result;
   }
 
+  /**
+   * Implement acl get.
+   * <p>
+   * see: https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketAcl.html
+   */
+  public S3BucketAcl getAcl(String bucketName)
+      throws OS3Exception, IOException {
+    S3BucketAcl result = new S3BucketAcl();
+    try {
+      OzoneBucket bucket = getBucket(bucketName);
+      OzoneVolume volume = getVolume();
+      S3Owner owner = new S3Owner(volume.getOwner(), volume.getOwner());

Review comment:
       Sure.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] adoroszlai commented on pull request #1701: HDDS-4585. Support bucket acl operation in S3g

Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #1701:
URL: https://github.com/apache/ozone/pull/1701#issuecomment-782214286


   > The remaining checkstyle warnings are mainly XML content string used for test which I don't plan to fix.
   
   These should be trivial to fix:
   
   ```
   hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/S3Acl.java
    38: Utility classes should not have a public or default constructor.
    42: Name &apos;grantRead&apos; must match pattern &apos;^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$&apos;.
    43: Name &apos;grantWrite&apos; must match pattern &apos;^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$&apos;.
    44: Name &apos;grantReadACP&apos; must match pattern &apos;^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$&apos;.
    45: Name &apos;grantWriteACP&apos; must match pattern &apos;^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$&apos;.
    46: Name &apos;grantFullControl&apos; must match pattern &apos;^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$&apos;.
    49: Name &apos;cannedAclHeader&apos; must match pattern &apos;^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$&apos;.
   hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestBucketAcl.java
    53: Name &apos;bucketName&apos; must match pattern &apos;^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$&apos;.
    60: Name &apos;aclMarker&apos; must match pattern &apos;^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$&apos;.
   ```
   
   and these can be fixed by moving XML content to external file(s) under `hadoop-ozone/s3gateway/src/test/resources`:
   
   ```
   hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestBucketAcl.java
    224: Line is longer than 80 characters (found 90).
    226: Line is longer than 80 characters (found 100).
    231: Line is longer than 80 characters (found 120).
    232: Line is longer than 80 characters (found 104).
    238: Line is longer than 80 characters (found 112).
    239: Line is longer than 80 characters (found 99).
    244: Line is longer than 80 characters (found 112).
    245: Line is longer than 80 characters (found 98).
    250: Line is longer than 80 characters (found 128).
    251: Line is longer than 80 characters (found 84).
    256: Line is longer than 80 characters (found 120).
    257: Line is longer than 80 characters (found 115).
    272: Line is longer than 80 characters (found 90).
    274: Line is longer than 80 characters (found 100).
    279: Line is longer than 80 characters (found 120).
    280: Line is longer than 80 characters (found 104).
    286: Line is longer than 80 characters (found 120).
    287: Line is longer than 80 characters (found 115).
   ```


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] ChenSammi commented on a change in pull request #1701: HDDS-4585. Support bucket acl operation in S3g

Posted by GitBox <gi...@apache.org>.
ChenSammi commented on a change in pull request #1701:
URL: https://github.com/apache/ozone/pull/1701#discussion_r565900409



##########
File path: hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java
##########
@@ -75,7 +87,7 @@
   @GET
   @SuppressFBWarnings
   @SuppressWarnings("parameternumber")
-  public Response list(
+  public Response get(

Review comment:
       The annotation @GET defines that this get or list function is called when HTTP GET on bucket is called.  
   There are plenty of HTTP GET operations on bucket, such as list object, get ACL, each with different parameters. 
   
   Before this patch, the GET operation on the bucket only supports bucket list. With this patch, get ACL and list bucket are both supported.  In the future, when we need to support another GET operation, we just need to add more logic to this bucket endpoint get function. 
     




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] prashantpogde commented on a change in pull request #1701: HDDS-4585. Support bucket acl operation in S3g

Posted by GitBox <gi...@apache.org>.
prashantpogde commented on a change in pull request #1701:
URL: https://github.com/apache/ozone/pull/1701#discussion_r568936411



##########
File path: hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java
##########
@@ -353,6 +373,204 @@ public MultiDeleteResponse multiDelete(@PathParam("bucket") String bucketName,
     return result;
   }
 
+  /**
+   * Implement acl get.
+   * <p>
+   * see: https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketAcl.html
+   */
+  public S3BucketAcl getAcl(String bucketName)
+      throws OS3Exception, IOException {
+    S3BucketAcl result = new S3BucketAcl();
+    try {
+      OzoneBucket bucket = getBucket(bucketName);
+      OzoneVolume volume = getVolume();
+      // TODO: use bucket owner instead of volume owner here once bucket owner
+      // TODO: is supported.
+      S3Owner owner = new S3Owner(volume.getOwner(), volume.getOwner());
+      result.setOwner(owner);
+
+      // TODO: remove this duplication avoid logic when ACCESS and DEFAULT scope
+      // TODO: are merged.
+      // Use set to remove ACLs with different scopes(ACCESS and DEFAULT)
+      Set<Grant> grantSet = new HashSet<>();
+      // Return ACL list
+      for (OzoneAcl acl : bucket.getAcls()) {
+        List<Grant> grants = S3Acl.ozoneNativeAclToS3Acl(acl);
+        grantSet.addAll(grants);
+      }
+      ArrayList<Grant> grantList = new ArrayList<>();
+      grantList.addAll(grantSet);
+      result.setAclList(
+          new S3BucketAcl.AccessControlList(grantList));
+      return result;
+    } catch (OMException ex) {
+      if (ex.getResult() == ResultCodes.BUCKET_NOT_FOUND) {
+        throw S3ErrorTable.newError(S3ErrorTable
+            .NO_SUCH_BUCKET, bucketName);
+      } else if (ex.getResult() == ResultCodes.PERMISSION_DENIED) {
+        throw S3ErrorTable.newError(S3ErrorTable
+            .ACCESS_DENIED, bucketName);
+      } else {
+        LOG.error("Failed to get acl of Bucket " + bucketName, ex);
+        throw S3ErrorTable.newError(S3ErrorTable.INTERNAL_ERROR, bucketName);
+      }
+    }
+  }
+
+  /**
+   * Implement acl put.
+   * <p>
+   * see: https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutBucketAcl.html
+   */
+  public Response putAcl(String bucketName, HttpHeaders httpHeaders,
+      InputStream body) throws IOException, OS3Exception {
+    String grantReads = httpHeaders.getHeaderString(S3Acl.grantRead);
+    String grantWrites = httpHeaders.getHeaderString(S3Acl.grantWrite);
+    String grantReadACP = httpHeaders.getHeaderString(S3Acl.grantReadACP);
+    String grantWriteACP = httpHeaders.getHeaderString(S3Acl.grantWriteACP);
+    String grantFull = httpHeaders.getHeaderString(S3Acl.grantFullControl);
+
+    try {
+      OzoneBucket bucket = getBucket(bucketName);
+      OzoneVolume volume = getVolume();
+
+      List<OzoneAcl> ozoneAclListOnBucket = new ArrayList<>();
+      List<OzoneAcl> ozoneAclListOnVolume = new ArrayList<>();
+
+      if (grantReads == null && grantWrites == null && grantReadACP == null
+          && grantWriteACP == null && grantFull == null) {
+        S3BucketAcl putBucketAclRequest =
+            new PutBucketAclRequestUnmarshaller().readFrom(
+            null, null, null, null, null, body);
+        // Handle grants in body
+        ozoneAclListOnBucket.addAll(
+            S3Acl.s3AclToOzoneNativeAclOnBucket(putBucketAclRequest));
+        ozoneAclListOnVolume.addAll(
+            S3Acl.s3AclToOzoneNativeAclOnVolume(putBucketAclRequest));
+      } else {
+
+        // Handle grants in headers
+        if (grantReads != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantReads,
+              S3Acl.ACLType.READ.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantReads,
+              S3Acl.ACLType.READ.getValue()));
+        }
+        if (grantWrites != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantWrites,
+              S3Acl.ACLType.WRITE.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantWrites,
+              S3Acl.ACLType.WRITE.getValue()));
+        }
+        if (grantReadACP != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantReadACP,
+              S3Acl.ACLType.READ_ACP.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantReadACP,
+              S3Acl.ACLType.READ_ACP.getValue()));
+        }
+        if (grantWriteACP != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantWriteACP,
+              S3Acl.ACLType.WRITE_ACP.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantWriteACP,
+              S3Acl.ACLType.WRITE_ACP.getValue()));
+        }
+        if (grantFull != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantFull,
+              S3Acl.ACLType.FULL_CONTROL.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantFull,
+              S3Acl.ACLType.FULL_CONTROL.getValue()));
+        }
+      }
+
+      // A put request will reset all previous ACLs
+      bucket.setAcl(ozoneAclListOnBucket);
+      volume.setAcl(ozoneAclListOnVolume);

Review comment:
       Do we need to set up the ACL on volume ?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] adoroszlai commented on pull request #1701: HDDS-4585. Support bucket acl operation in S3g

Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #1701:
URL: https://github.com/apache/ozone/pull/1701#issuecomment-769571151


   > misc acceptance exit with error code 1, while there is no obvious error.
   
   I'm looking into it: HDDS-4760.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] xiaoyuyao commented on a change in pull request #1701: HDDS-4585. Support bucket acl operation in S3g

Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #1701:
URL: https://github.com/apache/ozone/pull/1701#discussion_r562384811



##########
File path: hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java
##########
@@ -353,6 +375,215 @@ public MultiDeleteResponse multiDelete(@PathParam("bucket") String bucketName,
     return result;
   }
 
+  /**
+   * Implement acl get.
+   * <p>
+   * see: https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketAcl.html
+   */
+  public S3BucketAcl getAcl(String bucketName)
+      throws OS3Exception, IOException {
+    S3BucketAcl result = new S3BucketAcl();
+    try {
+      OzoneBucket bucket = getBucket(bucketName);
+      OzoneVolume volume = getVolume();
+      S3Owner owner = new S3Owner(volume.getOwner(), volume.getOwner());
+      result.setOwner(owner);
+      // Use set to remove ACLs with different scopes(ACCESS and DEFAULT)
+      Set<Grant> grantSet = new HashSet<>();
+      // Return ACL list
+      for (OzoneAcl acl : bucket.getAcls()) {
+        List<Grant> grants = S3Acl.ozoneNativeAclToS3Acl(acl);
+        grantSet.addAll(grants);
+      }
+      ArrayList<Grant> grantList = new ArrayList<>();
+      grantList.addAll(grantSet);
+      result.setAclList(
+          new S3BucketAcl.AccessControlList(grantList));
+      return result;
+    } catch (OMException ex) {
+      if (ex.getResult() == ResultCodes.BUCKET_NOT_FOUND) {
+        throw S3ErrorTable.newError(S3ErrorTable
+            .NO_SUCH_BUCKET, bucketName);
+      } else if (ex.getResult() == ResultCodes.PERMISSION_DENIED) {
+        throw S3ErrorTable.newError(S3ErrorTable
+            .ACCESS_DENIED, bucketName);
+      } else {
+        LOG.error("Failed to get acl of Bucket " + bucketName, ex);
+        throw S3ErrorTable.newError(S3ErrorTable.INTERNAL_ERROR, bucketName);
+      }
+    }
+  }
+
+  /**
+   * Implement acl put.
+   * <p>
+   * see: https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutBucketAcl.html
+   */
+  public Response putAcl(String bucketName, HttpHeaders httpHeaders,
+      InputStream body) throws IOException, OS3Exception {
+    String grantReads = httpHeaders.getHeaderString(S3Acl.grantRead);
+    String grantWrites = httpHeaders.getHeaderString(S3Acl.grantWrite);
+    String grantReadACP = httpHeaders.getHeaderString(S3Acl.grantReadACP);
+    String grantWriteACP = httpHeaders.getHeaderString(S3Acl.grantWriteACP);
+    String grantFull = httpHeaders.getHeaderString(S3Acl.grantFullControl);
+
+    try {
+      OzoneBucket bucket = getBucket(bucketName);
+      OzoneVolume volume = getVolume();
+
+      List<OzoneAcl> ozoneAclListOnBucket = new ArrayList<>();
+      List<OzoneAcl> ozoneAclListOnVolume = new ArrayList<>();
+
+      if (grantReads == null && grantWrites == null && grantReadACP == null
+          && grantWriteACP == null && grantFull == null) {
+        S3BucketAcl putBucketAclRequest =
+            new PutBucketAclRequestUnmarshaller().readFrom(
+            null, null, null, null, null, body);
+        // Handle grants in body
+        ozoneAclListOnBucket.addAll(
+            S3Acl.s3AclToOzoneNativeAclOnBucket(putBucketAclRequest));
+        ozoneAclListOnVolume.addAll(
+            S3Acl.s3AclToOzoneNativeAclOnVolume(putBucketAclRequest));
+      } else {
+
+        // Handle grants in headers
+        if (grantReads != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantReads,
+              S3Acl.ACLType.READ.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantReads,
+              S3Acl.ACLType.READ.getValue()));
+        }
+        if (grantWrites != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantWrites,
+              S3Acl.ACLType.WRITE.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantWrites,
+              S3Acl.ACLType.WRITE.getValue()));
+        }
+        if (grantReadACP != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantReadACP,
+              S3Acl.ACLType.READ_ACP.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantReadACP,
+              S3Acl.ACLType.READ_ACP.getValue()));
+        }
+        if (grantWriteACP != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantWriteACP,
+              S3Acl.ACLType.WRITE_ACP.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantWriteACP,
+              S3Acl.ACLType.WRITE_ACP.getValue()));
+        }
+        if (grantFull != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantFull,
+              S3Acl.ACLType.FULL_CONTROL.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantFull,
+              S3Acl.ACLType.FULL_CONTROL.getValue()));
+        }
+      }
+
+      List<OzoneAcl> oldBucketAcls = bucket.getAcls();
+      List<OzoneAcl> oldVolumeAcls =
+          S3Acl.getVolumeAclFromBucketAcl(oldBucketAcls);
+
+      // Add new ACLs
+      for (OzoneAcl addAcl : ozoneAclListOnBucket) {
+        bucket.addAcl(addAcl);

Review comment:
       why don't we call volume/bucket setAcl() directly instead of add new/remove old?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] xiaoyuyao commented on pull request #1701: HDDS-4585. Support bucket acl operation in S3g

Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on pull request #1701:
URL: https://github.com/apache/ozone/pull/1701#issuecomment-827931126


   Thanks @ChenSammi  for the update. The latest change LGTM. +1. I will merge the PR if no further comments by EOD today. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] xiaoyuyao commented on a change in pull request #1701: HDDS-4585. Support bucket acl operation in S3g

Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #1701:
URL: https://github.com/apache/ozone/pull/1701#discussion_r566579914



##########
File path: hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java
##########
@@ -75,7 +87,7 @@
   @GET
   @SuppressFBWarnings
   @SuppressWarnings("parameternumber")
-  public Response list(
+  public Response get(

Review comment:
       Thanks for the details. Make sense to me. 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] xiaoyuyao commented on a change in pull request #1701: HDDS-4585. Support bucket acl operation in S3g

Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #1701:
URL: https://github.com/apache/ozone/pull/1701#discussion_r612630356



##########
File path: hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java
##########
@@ -353,6 +373,204 @@ public MultiDeleteResponse multiDelete(@PathParam("bucket") String bucketName,
     return result;
   }
 
+  /**
+   * Implement acl get.
+   * <p>
+   * see: https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketAcl.html
+   */
+  public S3BucketAcl getAcl(String bucketName)
+      throws OS3Exception, IOException {
+    S3BucketAcl result = new S3BucketAcl();
+    try {
+      OzoneBucket bucket = getBucket(bucketName);
+      OzoneVolume volume = getVolume();
+      // TODO: use bucket owner instead of volume owner here once bucket owner
+      // TODO: is supported.
+      S3Owner owner = new S3Owner(volume.getOwner(), volume.getOwner());
+      result.setOwner(owner);
+
+      // TODO: remove this duplication avoid logic when ACCESS and DEFAULT scope
+      // TODO: are merged.
+      // Use set to remove ACLs with different scopes(ACCESS and DEFAULT)
+      Set<Grant> grantSet = new HashSet<>();
+      // Return ACL list
+      for (OzoneAcl acl : bucket.getAcls()) {
+        List<Grant> grants = S3Acl.ozoneNativeAclToS3Acl(acl);
+        grantSet.addAll(grants);
+      }
+      ArrayList<Grant> grantList = new ArrayList<>();
+      grantList.addAll(grantSet);
+      result.setAclList(
+          new S3BucketAcl.AccessControlList(grantList));
+      return result;
+    } catch (OMException ex) {
+      if (ex.getResult() == ResultCodes.BUCKET_NOT_FOUND) {
+        throw S3ErrorTable.newError(S3ErrorTable
+            .NO_SUCH_BUCKET, bucketName);
+      } else if (ex.getResult() == ResultCodes.PERMISSION_DENIED) {
+        throw S3ErrorTable.newError(S3ErrorTable
+            .ACCESS_DENIED, bucketName);
+      } else {
+        LOG.error("Failed to get acl of Bucket " + bucketName, ex);
+        throw S3ErrorTable.newError(S3ErrorTable.INTERNAL_ERROR, bucketName);
+      }
+    }
+  }
+
+  /**
+   * Implement acl put.
+   * <p>
+   * see: https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutBucketAcl.html
+   */
+  public Response putAcl(String bucketName, HttpHeaders httpHeaders,
+      InputStream body) throws IOException, OS3Exception {
+    String grantReads = httpHeaders.getHeaderString(S3Acl.GRANT_READ);
+    String grantWrites = httpHeaders.getHeaderString(S3Acl.GRANT_WRITE);
+    String grantReadACP = httpHeaders.getHeaderString(S3Acl.GRANT_READ_CAP);
+    String grantWriteACP = httpHeaders.getHeaderString(S3Acl.GRANT_WRITE_CAP);
+    String grantFull = httpHeaders.getHeaderString(S3Acl.GRANT_FULL_CONTROL);
+
+    try {
+      OzoneBucket bucket = getBucket(bucketName);
+      OzoneVolume volume = getVolume();
+
+      List<OzoneAcl> ozoneAclListOnBucket = new ArrayList<>();
+      List<OzoneAcl> ozoneAclListOnVolume = new ArrayList<>();
+
+      if (grantReads == null && grantWrites == null && grantReadACP == null
+          && grantWriteACP == null && grantFull == null) {
+        S3BucketAcl putBucketAclRequest =
+            new PutBucketAclRequestUnmarshaller().readFrom(
+            null, null, null, null, null, body);
+        // Handle grants in body
+        ozoneAclListOnBucket.addAll(
+            S3Acl.s3AclToOzoneNativeAclOnBucket(putBucketAclRequest));
+        ozoneAclListOnVolume.addAll(
+            S3Acl.s3AclToOzoneNativeAclOnVolume(putBucketAclRequest));
+      } else {
+
+        // Handle grants in headers
+        if (grantReads != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantReads,
+              S3Acl.ACLType.READ.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantReads,
+              S3Acl.ACLType.READ.getValue()));
+        }
+        if (grantWrites != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantWrites,
+              S3Acl.ACLType.WRITE.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantWrites,
+              S3Acl.ACLType.WRITE.getValue()));
+        }
+        if (grantReadACP != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantReadACP,
+              S3Acl.ACLType.READ_ACP.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantReadACP,
+              S3Acl.ACLType.READ_ACP.getValue()));
+        }
+        if (grantWriteACP != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantWriteACP,
+              S3Acl.ACLType.WRITE_ACP.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantWriteACP,
+              S3Acl.ACLType.WRITE_ACP.getValue()));
+        }
+        if (grantFull != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantFull,
+              S3Acl.ACLType.FULL_CONTROL.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantFull,
+              S3Acl.ACLType.FULL_CONTROL.getValue()));
+        }
+      }
+
+      // A put request will reset all previous ACLs
+      bucket.setAcl(ozoneAclListOnBucket);
+      volume.setAcl(ozoneAclListOnVolume);
+    } catch (OMException exception) {
+      LOG.error("Error in set ACL Request for bucket: {}", bucketName,
+          exception);
+      if (exception.getResult() == ResultCodes.BUCKET_NOT_FOUND) {
+        throw S3ErrorTable.newError(S3ErrorTable.NO_SUCH_BUCKET,
+            bucketName);
+      } else if (exception.getResult() == ResultCodes.PERMISSION_DENIED) {
+        throw S3ErrorTable.newError(S3ErrorTable
+            .ACCESS_DENIED, bucketName);
+      }
+      throw exception;
+    }
+    return Response.status(HttpStatus.SC_OK).build();
+  }
+
+  /**
+   *  Example: x-amz-grant-write: \
+   *  uri="http://acs.amazonaws.com/groups/s3/LogDelivery", id="111122223333", \
+   *  id="555566667777".
+   */
+  private List<OzoneAcl> getAndConvertAclOnBucket(String value,
+      String permission) throws OS3Exception {
+    List<OzoneAcl> ozoneAclList = new ArrayList<>();
+    if (StringUtils.isEmpty(value)) {
+      return ozoneAclList;
+    }
+    String[] subValues = value.split(",");
+    for (String acl: subValues) {
+      String[] part = acl.split("=");
+      if (part.length != 2) {
+        throw S3ErrorTable.newError(S3ErrorTable.INVALID_ARGUMENT, acl);
+      }
+      S3Acl.ACLIdentityType type =
+          S3Acl.ACLIdentityType.getTypeFromHeaderType(part[0]);
+      if (type == null || !type.isSupported()) {
+        LOG.warn("S3 grantee {} is null or not supported", part[0]);
+        throw S3ErrorTable.newError(NOT_IMPLEMENTED, part[0]);
+      }
+      // Build ACL on Bucket
+      BitSet aclsOnBucket =
+          S3Acl.getOzoneAclOnBucketFromS3Permission(permission);
+      OzoneAcl defaultOzoneAcl = new OzoneAcl(
+          IAccessAuthorizer.ACLIdentityType.USER, part[1], aclsOnBucket,
+          OzoneAcl.AclScope.DEFAULT);
+      OzoneAcl accessOzoneAcl = new OzoneAcl(
+          IAccessAuthorizer.ACLIdentityType.USER, part[1], aclsOnBucket,
+          OzoneAcl.AclScope.ACCESS);
+      ozoneAclList.add(defaultOzoneAcl);
+      ozoneAclList.add(accessOzoneAcl);
+    }
+    return ozoneAclList;
+  }
+
+  private List<OzoneAcl> getAndConvertAclOnVolume(String value,
+      String permission) throws OS3Exception {
+    List<OzoneAcl> ozoneAclList = new ArrayList<>();
+    if (StringUtils.isEmpty(value)) {
+      return ozoneAclList;
+    }
+    String[] subValues = value.split(",");
+    for (String acl: subValues) {
+      String[] part = acl.split("=");
+      if (part.length != 2) {
+        throw S3ErrorTable.newError(S3ErrorTable.INVALID_ARGUMENT, acl);
+      }
+      S3Acl.ACLIdentityType type =
+          S3Acl.ACLIdentityType.getTypeFromHeaderType(part[0]);
+      if (type == null || !type.isSupported()) {
+        LOG.warn("S3 grantee {} is null or not supported", part[0]);
+        throw S3ErrorTable.newError(NOT_IMPLEMENTED, part[0]);
+      }
+      // Build ACL on Volume
+      BitSet aclsOnVolume =
+          S3Acl.getOzoneAclOnVolumeFromS3Permission(permission);
+      OzoneAcl defaultOzoneAcl = new OzoneAcl(
+          IAccessAuthorizer.ACLIdentityType.USER, part[1], aclsOnVolume,

Review comment:
       I don't think we should populate DEFAULT ACL on Volume with the same permission. This will affect all new buckets create after that. 
   
   For example if user **foo** change its bucket acl to allow full control of user **bar** on bucket1, but as a result of that, the default acl for user **bar** with full control was added on volume. user **bar**  will will gain full control on all volume/buckets created subsequently. 
   
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] ChenSammi commented on pull request #1701: HDDS-4585. Support bucket acl operation in S3g

Posted by GitBox <gi...@apache.org>.
ChenSammi commented on pull request #1701:
URL: https://github.com/apache/ozone/pull/1701#issuecomment-782011294


   @xiaoyuyao  @prashantpogde , I just rebased the patch to the master branch,  do you have time to take another look? 
   
   The remaining checkstyle warnings are mainly XML content string used for test which I don't plan to fix. 
   The failed secure acceptance test seems irrelevant. 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] ChenSammi commented on a change in pull request #1701: HDDS-4585. Support bucket acl operation in S3g

Posted by GitBox <gi...@apache.org>.
ChenSammi commented on a change in pull request #1701:
URL: https://github.com/apache/ozone/pull/1701#discussion_r565893240



##########
File path: hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java
##########
@@ -353,6 +375,215 @@ public MultiDeleteResponse multiDelete(@PathParam("bucket") String bucketName,
     return result;
   }
 
+  /**
+   * Implement acl get.
+   * <p>
+   * see: https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketAcl.html
+   */
+  public S3BucketAcl getAcl(String bucketName)
+      throws OS3Exception, IOException {
+    S3BucketAcl result = new S3BucketAcl();
+    try {
+      OzoneBucket bucket = getBucket(bucketName);
+      OzoneVolume volume = getVolume();
+      S3Owner owner = new S3Owner(volume.getOwner(), volume.getOwner());
+      result.setOwner(owner);
+      // Use set to remove ACLs with different scopes(ACCESS and DEFAULT)
+      Set<Grant> grantSet = new HashSet<>();
+      // Return ACL list
+      for (OzoneAcl acl : bucket.getAcls()) {
+        List<Grant> grants = S3Acl.ozoneNativeAclToS3Acl(acl);
+        grantSet.addAll(grants);
+      }
+      ArrayList<Grant> grantList = new ArrayList<>();
+      grantList.addAll(grantSet);
+      result.setAclList(
+          new S3BucketAcl.AccessControlList(grantList));
+      return result;
+    } catch (OMException ex) {
+      if (ex.getResult() == ResultCodes.BUCKET_NOT_FOUND) {
+        throw S3ErrorTable.newError(S3ErrorTable
+            .NO_SUCH_BUCKET, bucketName);
+      } else if (ex.getResult() == ResultCodes.PERMISSION_DENIED) {
+        throw S3ErrorTable.newError(S3ErrorTable
+            .ACCESS_DENIED, bucketName);
+      } else {
+        LOG.error("Failed to get acl of Bucket " + bucketName, ex);
+        throw S3ErrorTable.newError(S3ErrorTable.INTERNAL_ERROR, bucketName);
+      }
+    }
+  }
+
+  /**
+   * Implement acl put.
+   * <p>
+   * see: https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutBucketAcl.html
+   */
+  public Response putAcl(String bucketName, HttpHeaders httpHeaders,
+      InputStream body) throws IOException, OS3Exception {
+    String grantReads = httpHeaders.getHeaderString(S3Acl.grantRead);
+    String grantWrites = httpHeaders.getHeaderString(S3Acl.grantWrite);
+    String grantReadACP = httpHeaders.getHeaderString(S3Acl.grantReadACP);
+    String grantWriteACP = httpHeaders.getHeaderString(S3Acl.grantWriteACP);
+    String grantFull = httpHeaders.getHeaderString(S3Acl.grantFullControl);
+
+    try {
+      OzoneBucket bucket = getBucket(bucketName);
+      OzoneVolume volume = getVolume();
+
+      List<OzoneAcl> ozoneAclListOnBucket = new ArrayList<>();
+      List<OzoneAcl> ozoneAclListOnVolume = new ArrayList<>();
+
+      if (grantReads == null && grantWrites == null && grantReadACP == null
+          && grantWriteACP == null && grantFull == null) {
+        S3BucketAcl putBucketAclRequest =
+            new PutBucketAclRequestUnmarshaller().readFrom(
+            null, null, null, null, null, body);
+        // Handle grants in body
+        ozoneAclListOnBucket.addAll(
+            S3Acl.s3AclToOzoneNativeAclOnBucket(putBucketAclRequest));
+        ozoneAclListOnVolume.addAll(
+            S3Acl.s3AclToOzoneNativeAclOnVolume(putBucketAclRequest));
+      } else {
+
+        // Handle grants in headers
+        if (grantReads != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantReads,
+              S3Acl.ACLType.READ.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantReads,
+              S3Acl.ACLType.READ.getValue()));
+        }
+        if (grantWrites != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantWrites,
+              S3Acl.ACLType.WRITE.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantWrites,
+              S3Acl.ACLType.WRITE.getValue()));
+        }
+        if (grantReadACP != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantReadACP,
+              S3Acl.ACLType.READ_ACP.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantReadACP,
+              S3Acl.ACLType.READ_ACP.getValue()));
+        }
+        if (grantWriteACP != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantWriteACP,
+              S3Acl.ACLType.WRITE_ACP.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantWriteACP,
+              S3Acl.ACLType.WRITE_ACP.getValue()));
+        }
+        if (grantFull != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantFull,
+              S3Acl.ACLType.FULL_CONTROL.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantFull,
+              S3Acl.ACLType.FULL_CONTROL.getValue()));
+        }
+      }
+
+      List<OzoneAcl> oldBucketAcls = bucket.getAcls();
+      List<OzoneAcl> oldVolumeAcls =
+          S3Acl.getVolumeAclFromBucketAcl(oldBucketAcls);
+
+      // Add new ACLs
+      for (OzoneAcl addAcl : ozoneAclListOnBucket) {
+        bucket.addAcl(addAcl);

Review comment:
       Good point.  Currently setAcl is not exposed in OzoneBucket and OzoneVolume, I will add it first. 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] prashantpogde commented on a change in pull request #1701: HDDS-4585. Support bucket acl operation in S3g

Posted by GitBox <gi...@apache.org>.
prashantpogde commented on a change in pull request #1701:
URL: https://github.com/apache/ozone/pull/1701#discussion_r568936411



##########
File path: hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java
##########
@@ -353,6 +373,204 @@ public MultiDeleteResponse multiDelete(@PathParam("bucket") String bucketName,
     return result;
   }
 
+  /**
+   * Implement acl get.
+   * <p>
+   * see: https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketAcl.html
+   */
+  public S3BucketAcl getAcl(String bucketName)
+      throws OS3Exception, IOException {
+    S3BucketAcl result = new S3BucketAcl();
+    try {
+      OzoneBucket bucket = getBucket(bucketName);
+      OzoneVolume volume = getVolume();
+      // TODO: use bucket owner instead of volume owner here once bucket owner
+      // TODO: is supported.
+      S3Owner owner = new S3Owner(volume.getOwner(), volume.getOwner());
+      result.setOwner(owner);
+
+      // TODO: remove this duplication avoid logic when ACCESS and DEFAULT scope
+      // TODO: are merged.
+      // Use set to remove ACLs with different scopes(ACCESS and DEFAULT)
+      Set<Grant> grantSet = new HashSet<>();
+      // Return ACL list
+      for (OzoneAcl acl : bucket.getAcls()) {
+        List<Grant> grants = S3Acl.ozoneNativeAclToS3Acl(acl);
+        grantSet.addAll(grants);
+      }
+      ArrayList<Grant> grantList = new ArrayList<>();
+      grantList.addAll(grantSet);
+      result.setAclList(
+          new S3BucketAcl.AccessControlList(grantList));
+      return result;
+    } catch (OMException ex) {
+      if (ex.getResult() == ResultCodes.BUCKET_NOT_FOUND) {
+        throw S3ErrorTable.newError(S3ErrorTable
+            .NO_SUCH_BUCKET, bucketName);
+      } else if (ex.getResult() == ResultCodes.PERMISSION_DENIED) {
+        throw S3ErrorTable.newError(S3ErrorTable
+            .ACCESS_DENIED, bucketName);
+      } else {
+        LOG.error("Failed to get acl of Bucket " + bucketName, ex);
+        throw S3ErrorTable.newError(S3ErrorTable.INTERNAL_ERROR, bucketName);
+      }
+    }
+  }
+
+  /**
+   * Implement acl put.
+   * <p>
+   * see: https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutBucketAcl.html
+   */
+  public Response putAcl(String bucketName, HttpHeaders httpHeaders,
+      InputStream body) throws IOException, OS3Exception {
+    String grantReads = httpHeaders.getHeaderString(S3Acl.grantRead);
+    String grantWrites = httpHeaders.getHeaderString(S3Acl.grantWrite);
+    String grantReadACP = httpHeaders.getHeaderString(S3Acl.grantReadACP);
+    String grantWriteACP = httpHeaders.getHeaderString(S3Acl.grantWriteACP);
+    String grantFull = httpHeaders.getHeaderString(S3Acl.grantFullControl);
+
+    try {
+      OzoneBucket bucket = getBucket(bucketName);
+      OzoneVolume volume = getVolume();
+
+      List<OzoneAcl> ozoneAclListOnBucket = new ArrayList<>();
+      List<OzoneAcl> ozoneAclListOnVolume = new ArrayList<>();
+
+      if (grantReads == null && grantWrites == null && grantReadACP == null
+          && grantWriteACP == null && grantFull == null) {
+        S3BucketAcl putBucketAclRequest =
+            new PutBucketAclRequestUnmarshaller().readFrom(
+            null, null, null, null, null, body);
+        // Handle grants in body
+        ozoneAclListOnBucket.addAll(
+            S3Acl.s3AclToOzoneNativeAclOnBucket(putBucketAclRequest));
+        ozoneAclListOnVolume.addAll(
+            S3Acl.s3AclToOzoneNativeAclOnVolume(putBucketAclRequest));
+      } else {
+
+        // Handle grants in headers
+        if (grantReads != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantReads,
+              S3Acl.ACLType.READ.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantReads,
+              S3Acl.ACLType.READ.getValue()));
+        }
+        if (grantWrites != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantWrites,
+              S3Acl.ACLType.WRITE.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantWrites,
+              S3Acl.ACLType.WRITE.getValue()));
+        }
+        if (grantReadACP != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantReadACP,
+              S3Acl.ACLType.READ_ACP.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantReadACP,
+              S3Acl.ACLType.READ_ACP.getValue()));
+        }
+        if (grantWriteACP != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantWriteACP,
+              S3Acl.ACLType.WRITE_ACP.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantWriteACP,
+              S3Acl.ACLType.WRITE_ACP.getValue()));
+        }
+        if (grantFull != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantFull,
+              S3Acl.ACLType.FULL_CONTROL.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantFull,
+              S3Acl.ACLType.FULL_CONTROL.getValue()));
+        }
+      }
+
+      // A put request will reset all previous ACLs
+      bucket.setAcl(ozoneAclListOnBucket);
+      volume.setAcl(ozoneAclListOnVolume);

Review comment:
       Do we need to set up the ACL on volume ?

##########
File path: hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java
##########
@@ -353,6 +373,204 @@ public MultiDeleteResponse multiDelete(@PathParam("bucket") String bucketName,
     return result;
   }
 
+  /**
+   * Implement acl get.
+   * <p>
+   * see: https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketAcl.html
+   */
+  public S3BucketAcl getAcl(String bucketName)
+      throws OS3Exception, IOException {
+    S3BucketAcl result = new S3BucketAcl();
+    try {
+      OzoneBucket bucket = getBucket(bucketName);
+      OzoneVolume volume = getVolume();
+      // TODO: use bucket owner instead of volume owner here once bucket owner
+      // TODO: is supported.
+      S3Owner owner = new S3Owner(volume.getOwner(), volume.getOwner());
+      result.setOwner(owner);
+
+      // TODO: remove this duplication avoid logic when ACCESS and DEFAULT scope
+      // TODO: are merged.
+      // Use set to remove ACLs with different scopes(ACCESS and DEFAULT)
+      Set<Grant> grantSet = new HashSet<>();
+      // Return ACL list
+      for (OzoneAcl acl : bucket.getAcls()) {
+        List<Grant> grants = S3Acl.ozoneNativeAclToS3Acl(acl);
+        grantSet.addAll(grants);
+      }
+      ArrayList<Grant> grantList = new ArrayList<>();
+      grantList.addAll(grantSet);
+      result.setAclList(
+          new S3BucketAcl.AccessControlList(grantList));
+      return result;
+    } catch (OMException ex) {
+      if (ex.getResult() == ResultCodes.BUCKET_NOT_FOUND) {
+        throw S3ErrorTable.newError(S3ErrorTable
+            .NO_SUCH_BUCKET, bucketName);
+      } else if (ex.getResult() == ResultCodes.PERMISSION_DENIED) {
+        throw S3ErrorTable.newError(S3ErrorTable
+            .ACCESS_DENIED, bucketName);
+      } else {
+        LOG.error("Failed to get acl of Bucket " + bucketName, ex);
+        throw S3ErrorTable.newError(S3ErrorTable.INTERNAL_ERROR, bucketName);
+      }
+    }
+  }
+
+  /**
+   * Implement acl put.
+   * <p>
+   * see: https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutBucketAcl.html
+   */
+  public Response putAcl(String bucketName, HttpHeaders httpHeaders,
+      InputStream body) throws IOException, OS3Exception {
+    String grantReads = httpHeaders.getHeaderString(S3Acl.grantRead);
+    String grantWrites = httpHeaders.getHeaderString(S3Acl.grantWrite);
+    String grantReadACP = httpHeaders.getHeaderString(S3Acl.grantReadACP);
+    String grantWriteACP = httpHeaders.getHeaderString(S3Acl.grantWriteACP);
+    String grantFull = httpHeaders.getHeaderString(S3Acl.grantFullControl);
+
+    try {
+      OzoneBucket bucket = getBucket(bucketName);
+      OzoneVolume volume = getVolume();
+
+      List<OzoneAcl> ozoneAclListOnBucket = new ArrayList<>();
+      List<OzoneAcl> ozoneAclListOnVolume = new ArrayList<>();
+
+      if (grantReads == null && grantWrites == null && grantReadACP == null
+          && grantWriteACP == null && grantFull == null) {
+        S3BucketAcl putBucketAclRequest =
+            new PutBucketAclRequestUnmarshaller().readFrom(
+            null, null, null, null, null, body);
+        // Handle grants in body
+        ozoneAclListOnBucket.addAll(
+            S3Acl.s3AclToOzoneNativeAclOnBucket(putBucketAclRequest));
+        ozoneAclListOnVolume.addAll(
+            S3Acl.s3AclToOzoneNativeAclOnVolume(putBucketAclRequest));
+      } else {
+
+        // Handle grants in headers
+        if (grantReads != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantReads,
+              S3Acl.ACLType.READ.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantReads,
+              S3Acl.ACLType.READ.getValue()));
+        }
+        if (grantWrites != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantWrites,
+              S3Acl.ACLType.WRITE.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantWrites,
+              S3Acl.ACLType.WRITE.getValue()));
+        }
+        if (grantReadACP != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantReadACP,
+              S3Acl.ACLType.READ_ACP.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantReadACP,
+              S3Acl.ACLType.READ_ACP.getValue()));
+        }
+        if (grantWriteACP != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantWriteACP,
+              S3Acl.ACLType.WRITE_ACP.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantWriteACP,
+              S3Acl.ACLType.WRITE_ACP.getValue()));
+        }
+        if (grantFull != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantFull,
+              S3Acl.ACLType.FULL_CONTROL.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantFull,
+              S3Acl.ACLType.FULL_CONTROL.getValue()));
+        }
+      }
+
+      // A put request will reset all previous ACLs
+      bucket.setAcl(ozoneAclListOnBucket);
+      volume.setAcl(ozoneAclListOnVolume);

Review comment:
       Do we need to setup ACL on a volume here ? couldn't this lead to manipulating permissions on a bucket where the user doesn't have a permission ?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] ChenSammi closed pull request #1701: HDDS-4585. Support bucket acl operation in S3g

Posted by GitBox <gi...@apache.org>.
ChenSammi closed pull request #1701:
URL: https://github.com/apache/ozone/pull/1701


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] xiaoyuyao commented on a change in pull request #1701: HDDS-4585. Support bucket acl operation in S3g

Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #1701:
URL: https://github.com/apache/ozone/pull/1701#discussion_r612649256



##########
File path: hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java
##########
@@ -353,6 +373,204 @@ public MultiDeleteResponse multiDelete(@PathParam("bucket") String bucketName,
     return result;
   }
 
+  /**
+   * Implement acl get.
+   * <p>
+   * see: https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketAcl.html
+   */
+  public S3BucketAcl getAcl(String bucketName)
+      throws OS3Exception, IOException {
+    S3BucketAcl result = new S3BucketAcl();
+    try {
+      OzoneBucket bucket = getBucket(bucketName);
+      OzoneVolume volume = getVolume();
+      // TODO: use bucket owner instead of volume owner here once bucket owner
+      // TODO: is supported.
+      S3Owner owner = new S3Owner(volume.getOwner(), volume.getOwner());
+      result.setOwner(owner);
+
+      // TODO: remove this duplication avoid logic when ACCESS and DEFAULT scope
+      // TODO: are merged.
+      // Use set to remove ACLs with different scopes(ACCESS and DEFAULT)
+      Set<Grant> grantSet = new HashSet<>();
+      // Return ACL list
+      for (OzoneAcl acl : bucket.getAcls()) {
+        List<Grant> grants = S3Acl.ozoneNativeAclToS3Acl(acl);
+        grantSet.addAll(grants);
+      }
+      ArrayList<Grant> grantList = new ArrayList<>();
+      grantList.addAll(grantSet);
+      result.setAclList(
+          new S3BucketAcl.AccessControlList(grantList));
+      return result;
+    } catch (OMException ex) {
+      if (ex.getResult() == ResultCodes.BUCKET_NOT_FOUND) {
+        throw S3ErrorTable.newError(S3ErrorTable
+            .NO_SUCH_BUCKET, bucketName);
+      } else if (ex.getResult() == ResultCodes.PERMISSION_DENIED) {
+        throw S3ErrorTable.newError(S3ErrorTable
+            .ACCESS_DENIED, bucketName);
+      } else {
+        LOG.error("Failed to get acl of Bucket " + bucketName, ex);
+        throw S3ErrorTable.newError(S3ErrorTable.INTERNAL_ERROR, bucketName);
+      }
+    }
+  }
+
+  /**
+   * Implement acl put.
+   * <p>
+   * see: https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutBucketAcl.html
+   */
+  public Response putAcl(String bucketName, HttpHeaders httpHeaders,
+      InputStream body) throws IOException, OS3Exception {
+    String grantReads = httpHeaders.getHeaderString(S3Acl.grantRead);
+    String grantWrites = httpHeaders.getHeaderString(S3Acl.grantWrite);
+    String grantReadACP = httpHeaders.getHeaderString(S3Acl.grantReadACP);
+    String grantWriteACP = httpHeaders.getHeaderString(S3Acl.grantWriteACP);
+    String grantFull = httpHeaders.getHeaderString(S3Acl.grantFullControl);
+
+    try {
+      OzoneBucket bucket = getBucket(bucketName);
+      OzoneVolume volume = getVolume();
+
+      List<OzoneAcl> ozoneAclListOnBucket = new ArrayList<>();
+      List<OzoneAcl> ozoneAclListOnVolume = new ArrayList<>();
+
+      if (grantReads == null && grantWrites == null && grantReadACP == null
+          && grantWriteACP == null && grantFull == null) {
+        S3BucketAcl putBucketAclRequest =
+            new PutBucketAclRequestUnmarshaller().readFrom(
+            null, null, null, null, null, body);
+        // Handle grants in body
+        ozoneAclListOnBucket.addAll(
+            S3Acl.s3AclToOzoneNativeAclOnBucket(putBucketAclRequest));
+        ozoneAclListOnVolume.addAll(
+            S3Acl.s3AclToOzoneNativeAclOnVolume(putBucketAclRequest));
+      } else {
+
+        // Handle grants in headers
+        if (grantReads != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantReads,
+              S3Acl.ACLType.READ.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantReads,
+              S3Acl.ACLType.READ.getValue()));
+        }
+        if (grantWrites != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantWrites,
+              S3Acl.ACLType.WRITE.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantWrites,
+              S3Acl.ACLType.WRITE.getValue()));
+        }
+        if (grantReadACP != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantReadACP,
+              S3Acl.ACLType.READ_ACP.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantReadACP,
+              S3Acl.ACLType.READ_ACP.getValue()));
+        }
+        if (grantWriteACP != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantWriteACP,
+              S3Acl.ACLType.WRITE_ACP.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantWriteACP,
+              S3Acl.ACLType.WRITE_ACP.getValue()));
+        }
+        if (grantFull != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantFull,
+              S3Acl.ACLType.FULL_CONTROL.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantFull,
+              S3Acl.ACLType.FULL_CONTROL.getValue()));
+        }
+      }
+
+      // A put request will reset all previous ACLs
+      bucket.setAcl(ozoneAclListOnBucket);
+      volume.setAcl(ozoneAclListOnVolume);

Review comment:
       Should we read the existing volume acl, add the new ones necessary and set it back instead of overwrite the existing ACL on the s3 volume?
   
   @xiaoyuyao , I updated the implementation. Now it will remove all the ACLs of import users from Volume first, then apply the new ACLs on Volume. 
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] ChenSammi commented on a change in pull request #1701: HDDS-4585. Support bucket acl operation in S3g

Posted by GitBox <gi...@apache.org>.
ChenSammi commented on a change in pull request #1701:
URL: https://github.com/apache/ozone/pull/1701#discussion_r565900409



##########
File path: hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java
##########
@@ -75,7 +87,7 @@
   @GET
   @SuppressFBWarnings
   @SuppressWarnings("parameternumber")
-  public Response list(
+  public Response get(

Review comment:
       The function name doesn't matter.  It's the annotation @GET which defines that this get or list function is called when HTTP GET against bucket is called.  
   
   There are plenty of HTTP GET operations for a bucket, such as list object, get ACL, each with different parameters. 
   
   Before this patch, bucket endpoint only supports bucket list GET. With this patch, bucket ACL get and bucket list are both supported.  
   In future, when we need to support another GET operation, we just need to add more logic to this bucket endpoint get function. 
     




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] xiaoyuyao commented on a change in pull request #1701: HDDS-4585. Support bucket acl operation in S3g

Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #1701:
URL: https://github.com/apache/ozone/pull/1701#discussion_r562385231



##########
File path: hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java
##########
@@ -353,6 +375,215 @@ public MultiDeleteResponse multiDelete(@PathParam("bucket") String bucketName,
     return result;
   }
 
+  /**
+   * Implement acl get.
+   * <p>
+   * see: https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketAcl.html
+   */
+  public S3BucketAcl getAcl(String bucketName)
+      throws OS3Exception, IOException {
+    S3BucketAcl result = new S3BucketAcl();
+    try {
+      OzoneBucket bucket = getBucket(bucketName);
+      OzoneVolume volume = getVolume();
+      S3Owner owner = new S3Owner(volume.getOwner(), volume.getOwner());
+      result.setOwner(owner);
+      // Use set to remove ACLs with different scopes(ACCESS and DEFAULT)
+      Set<Grant> grantSet = new HashSet<>();
+      // Return ACL list
+      for (OzoneAcl acl : bucket.getAcls()) {
+        List<Grant> grants = S3Acl.ozoneNativeAclToS3Acl(acl);
+        grantSet.addAll(grants);
+      }
+      ArrayList<Grant> grantList = new ArrayList<>();
+      grantList.addAll(grantSet);
+      result.setAclList(
+          new S3BucketAcl.AccessControlList(grantList));
+      return result;
+    } catch (OMException ex) {
+      if (ex.getResult() == ResultCodes.BUCKET_NOT_FOUND) {
+        throw S3ErrorTable.newError(S3ErrorTable
+            .NO_SUCH_BUCKET, bucketName);
+      } else if (ex.getResult() == ResultCodes.PERMISSION_DENIED) {
+        throw S3ErrorTable.newError(S3ErrorTable
+            .ACCESS_DENIED, bucketName);
+      } else {
+        LOG.error("Failed to get acl of Bucket " + bucketName, ex);
+        throw S3ErrorTable.newError(S3ErrorTable.INTERNAL_ERROR, bucketName);
+      }
+    }
+  }
+
+  /**
+   * Implement acl put.
+   * <p>
+   * see: https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutBucketAcl.html
+   */
+  public Response putAcl(String bucketName, HttpHeaders httpHeaders,
+      InputStream body) throws IOException, OS3Exception {
+    String grantReads = httpHeaders.getHeaderString(S3Acl.grantRead);
+    String grantWrites = httpHeaders.getHeaderString(S3Acl.grantWrite);
+    String grantReadACP = httpHeaders.getHeaderString(S3Acl.grantReadACP);
+    String grantWriteACP = httpHeaders.getHeaderString(S3Acl.grantWriteACP);
+    String grantFull = httpHeaders.getHeaderString(S3Acl.grantFullControl);
+
+    try {
+      OzoneBucket bucket = getBucket(bucketName);
+      OzoneVolume volume = getVolume();
+
+      List<OzoneAcl> ozoneAclListOnBucket = new ArrayList<>();
+      List<OzoneAcl> ozoneAclListOnVolume = new ArrayList<>();
+
+      if (grantReads == null && grantWrites == null && grantReadACP == null
+          && grantWriteACP == null && grantFull == null) {
+        S3BucketAcl putBucketAclRequest =
+            new PutBucketAclRequestUnmarshaller().readFrom(
+            null, null, null, null, null, body);
+        // Handle grants in body
+        ozoneAclListOnBucket.addAll(
+            S3Acl.s3AclToOzoneNativeAclOnBucket(putBucketAclRequest));
+        ozoneAclListOnVolume.addAll(
+            S3Acl.s3AclToOzoneNativeAclOnVolume(putBucketAclRequest));
+      } else {
+
+        // Handle grants in headers
+        if (grantReads != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantReads,
+              S3Acl.ACLType.READ.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantReads,
+              S3Acl.ACLType.READ.getValue()));
+        }
+        if (grantWrites != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantWrites,
+              S3Acl.ACLType.WRITE.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantWrites,
+              S3Acl.ACLType.WRITE.getValue()));
+        }
+        if (grantReadACP != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantReadACP,
+              S3Acl.ACLType.READ_ACP.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantReadACP,
+              S3Acl.ACLType.READ_ACP.getValue()));
+        }
+        if (grantWriteACP != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantWriteACP,
+              S3Acl.ACLType.WRITE_ACP.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantWriteACP,
+              S3Acl.ACLType.WRITE_ACP.getValue()));
+        }
+        if (grantFull != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantFull,
+              S3Acl.ACLType.FULL_CONTROL.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantFull,
+              S3Acl.ACLType.FULL_CONTROL.getValue()));
+        }
+      }
+
+      List<OzoneAcl> oldBucketAcls = bucket.getAcls();
+      List<OzoneAcl> oldVolumeAcls =
+          S3Acl.getVolumeAclFromBucketAcl(oldBucketAcls);
+
+      // Add new ACLs
+      for (OzoneAcl addAcl : ozoneAclListOnBucket) {
+        bucket.addAcl(addAcl);
+      }
+      for (OzoneAcl addAcl : ozoneAclListOnVolume) {
+        volume.addAcl(addAcl);
+      }
+
+      // A put request will reset all previous ACLs
+      for (OzoneAcl removeAcl : oldBucketAcls) {
+        bucket.removeAcl(removeAcl);
+      }
+      for (OzoneAcl removeAcl : oldVolumeAcls) {
+        volume.removeAcl(removeAcl);
+      }
+    } catch (OMException exception) {
+      LOG.error("Error in set ACL Request for bucket: {}", bucketName,
+          exception);
+      if (exception.getResult() == ResultCodes.BUCKET_NOT_FOUND) {
+        throw S3ErrorTable.newError(S3ErrorTable.NO_SUCH_BUCKET,
+            bucketName);
+      } else if (exception.getResult() == ResultCodes.PERMISSION_DENIED) {
+        throw S3ErrorTable.newError(S3ErrorTable
+            .ACCESS_DENIED, bucketName);
+      }
+      throw exception;
+    }
+    return Response.status(HttpStatus.SC_OK).build();
+  }
+
+  /**
+   *  Example: x-amz-grant-write: \
+   *  uri="http://acs.amazonaws.com/groups/s3/LogDelivery", id="111122223333", \
+   *  id="555566667777".
+   */
+  private List<OzoneAcl> getAndConvertAclOnBucket(String value,
+      String permission) throws OS3Exception {
+    List<OzoneAcl> ozoneAclList = new ArrayList<>();
+    if (StringUtils.isEmpty(value)) {
+      return ozoneAclList;
+    }
+    String[] subValues = value.split(",");
+    for (String acl: subValues) {
+      String[] part = acl.split("=");
+      if (part.length != 2) {
+        throw S3ErrorTable.newError(S3ErrorTable.INVALID_ARGUMENT, acl);
+      }
+      S3Acl.ACLIdentityType type =
+          S3Acl.ACLIdentityType.getTypeFromHeaderType(part[0]);
+      if (type == null || !type.isSupported()) {
+        LOG.warn("S3 grantee {} is null or not supported", part[0]);
+        throw S3ErrorTable.newError(NOT_IMPLEMENTED, part[0]);
+      }
+      // Build ACL on Bucket
+      BitSet aclsOnBucket =
+          S3Acl.getOzoneAclOnBucketFromS3Permission(permission);
+      OzoneAcl defaultOzoneAcl = new OzoneAcl(
+          IAccessAuthorizer.ACLIdentityType.USER, part[1], aclsOnBucket,
+          OzoneAcl.AclScope.DEFAULT);
+      OzoneAcl accessOzoneAcl = new OzoneAcl(
+          IAccessAuthorizer.ACLIdentityType.USER, part[1], aclsOnBucket,
+          OzoneAcl.AclScope.ACCESS);
+      ozoneAclList.add(defaultOzoneAcl);
+      ozoneAclList.add(accessOzoneAcl);
+    }
+    return ozoneAclList;
+  }
+
+  private List<OzoneAcl> getAndConvertAclOnVolume(String value,
+      String permission) throws OS3Exception {
+    List<OzoneAcl> ozoneAclList = new ArrayList<>();
+    if (StringUtils.isEmpty(value)) {
+      return ozoneAclList;
+    }
+    String[] subValues = value.split(",");
+    for (String acl: subValues) {
+      String[] part = acl.split("=");
+      if (part.length != 2) {
+        throw S3ErrorTable.newError(S3ErrorTable.INVALID_ARGUMENT, acl);
+      }
+      S3Acl.ACLIdentityType type =
+          S3Acl.ACLIdentityType.getTypeFromHeaderType(part[0]);
+      if (type == null || !type.isSupported()) {
+        LOG.warn("S3 grantee {} is null or not supported", part[0]);
+        throw S3ErrorTable.newError(NOT_IMPLEMENTED, part[0]);
+      }
+      // Build ACL on Bucket

Review comment:
       NIT: you mean volume?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] xiaoyuyao commented on a change in pull request #1701: HDDS-4585. Support bucket acl operation in S3g

Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #1701:
URL: https://github.com/apache/ozone/pull/1701#discussion_r562380251



##########
File path: hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/S3Acl.java
##########
@@ -0,0 +1,388 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+package org.apache.hadoop.ozone.s3.endpoint;
+
+import org.apache.hadoop.ozone.OzoneAcl;
+import org.apache.hadoop.ozone.s3.endpoint.S3BucketAcl.Grant;
+import org.apache.hadoop.ozone.s3.endpoint.S3BucketAcl.Grantee;
+import org.apache.hadoop.ozone.s3.exception.OS3Exception;
+import org.apache.hadoop.ozone.s3.exception.S3ErrorTable;
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.ArrayList;
+import java.util.BitSet;
+import java.util.List;
+
+import static org.apache.hadoop.ozone.s3.exception.S3ErrorTable.INVALID_ARGUMENT;
+import static org.apache.hadoop.ozone.s3.exception.S3ErrorTable.NOT_IMPLEMENTED;
+
+public final class S3Acl {
+  private static final Logger LOG = LoggerFactory.getLogger(S3Acl.class);
+
+  // ACL put related headers
+  public static final String grantRead = "x-amz-grant-read";
+  public static final String grantWrite = "x-amz-grant-write";
+  public static final String grantReadACP = "x-amz-grant-read-acp";
+  public static final String grantWriteACP = "x-amz-grant-write-acp";
+  public static final String grantFullControl = "x-amz-grant-full-control";
+
+  // Not supported headers at current stage, may support it in future
+  public static final String cannedAclHeader = "x-amz-acl";
+
+  /**
+   * https://docs.aws.amazon.com/AmazonS3/latest/dev/acl-overview.html.
+   */
+  enum ACLType {
+    // Allows grantee to list the objects in the bucket
+    READ("READ"),
+    // Allows grantee to create, overwrite, and delete any object in the bucket
+    WRITE("WRITE"),
+    // Allows grantee to write the bucket ACL
+    READ_ACP("READ_ACP"),
+    // Allows grantee to write the ACL for the applicable bucket
+    WRITE_ACP("WRITE_ACP"),
+    // Allows grantee above all permissions on the bucket
+    FULL_CONTROL("FULL_CONTROL");
+
+    public String getValue() {
+      return value;
+    }
+    /**
+     * String value for this Enum.
+     */
+    private final String value;
+
+    /**
+     * @param val String type for this enum.
+     */
+    ACLType(String val) {
+      value = val;
+    }
+
+
+    public static ACLType getType(String typeStr) {
+      for(ACLType type: ACLType.values()) {
+        if (type.getValue().equals(typeStr)) {
+          return type;
+        }
+      }
+      return null;
+    }
+  }
+
+  enum ACLIdentityType {
+    USER("CanonicalUser", true, "id"),
+    GROUP("Group", false, "url"),
+    USER_BY_EMAIL("AmazonCustomerByEmail", false, "emailAddress");
+
+    public String getGranteeType() {
+      return granteeType;
+    }
+
+    public String getHeaderType() {
+      return granteeInHeader;
+    }
+
+    /**
+     *  Grantee type in body XML.
+     */
+    private final String granteeType;
+
+    /**
+     * Is this type supported or not.
+     */
+    private final boolean supported;
+
+    /**
+     * Grantee type in header.
+     */
+    private final String granteeInHeader;
+
+    /**
+     * Init OzoneACLtypes enum.
+     *
+     * @param val String type for this enum.
+     */
+    ACLIdentityType(String val, boolean support, String headerType) {
+      granteeType = val;
+      supported = support;
+      granteeInHeader = headerType;
+    }
+
+    boolean isSupported() {
+      return supported;
+    }
+
+    public static ACLIdentityType getTypeFromGranteeType(String typeStr) {
+      for(ACLIdentityType type: ACLIdentityType.values()) {
+        if (type.getGranteeType().equals(typeStr)) {
+          return type;
+        }
+      }
+      return null;
+    }
+
+    public static ACLIdentityType getTypeFromHeaderType(String typeStr) {
+      for(ACLIdentityType type: ACLIdentityType.values()) {
+        if (type.getHeaderType().equals(typeStr)) {
+          return type;
+        }
+      }
+      return null;
+    }
+  }
+
+  public static boolean isGranteeTypeSupported(String typeStr) {
+    ACLIdentityType type =  ACLIdentityType.getTypeFromGranteeType(typeStr);
+    return type == null ? false : type.isSupported();
+  }
+
+  public static boolean isHeaderTypeSupported(String typeStr) {
+    ACLIdentityType type =  ACLIdentityType.getTypeFromHeaderType(typeStr);
+    return type == null ? false : type.isSupported();
+  }
+
+  public static List<Grant> ozoneNativeAclToS3Acl(OzoneAcl ozoneAcl) {
+    // Since currently only "CanonicalUser" is supported, which maps to Ozone
+    // "USER"
+    List<Grant> grantList = new ArrayList<>();
+    if (ozoneAcl.getType() != IAccessAuthorizer.ACLIdentityType.USER) {
+      return grantList;
+    }
+
+    Grantee grantee = new Grantee();
+    grantee.setDisplayName(ozoneAcl.getName());
+    grantee.setId(ozoneAcl.getName());
+
+    List<IAccessAuthorizer.ACLType> acls = ozoneAcl.getAclList();
+    if (acls.contains(IAccessAuthorizer.ACLType.ALL)) {
+      Grant grant = new Grant();
+      grant.setGrantee(grantee);
+      grant.setPermission(ACLType.FULL_CONTROL.toString());
+      grantList.add(grant);
+      return grantList;
+    } else if (acls.contains(IAccessAuthorizer.ACLType.WRITE_ACL)) {

Review comment:
       should we allow other combinations here? such as READ+WRITE?  I feel if we remove "else" here, the 1-1 mapping between Ozone AclType to the S3 ACLType can be added. 

##########
File path: hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/S3Acl.java
##########
@@ -0,0 +1,388 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+package org.apache.hadoop.ozone.s3.endpoint;
+
+import org.apache.hadoop.ozone.OzoneAcl;
+import org.apache.hadoop.ozone.s3.endpoint.S3BucketAcl.Grant;
+import org.apache.hadoop.ozone.s3.endpoint.S3BucketAcl.Grantee;
+import org.apache.hadoop.ozone.s3.exception.OS3Exception;
+import org.apache.hadoop.ozone.s3.exception.S3ErrorTable;
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.ArrayList;
+import java.util.BitSet;
+import java.util.List;
+
+import static org.apache.hadoop.ozone.s3.exception.S3ErrorTable.INVALID_ARGUMENT;
+import static org.apache.hadoop.ozone.s3.exception.S3ErrorTable.NOT_IMPLEMENTED;
+
+public final class S3Acl {
+  private static final Logger LOG = LoggerFactory.getLogger(S3Acl.class);
+
+  // ACL put related headers
+  public static final String grantRead = "x-amz-grant-read";
+  public static final String grantWrite = "x-amz-grant-write";
+  public static final String grantReadACP = "x-amz-grant-read-acp";
+  public static final String grantWriteACP = "x-amz-grant-write-acp";
+  public static final String grantFullControl = "x-amz-grant-full-control";
+
+  // Not supported headers at current stage, may support it in future
+  public static final String cannedAclHeader = "x-amz-acl";
+
+  /**
+   * https://docs.aws.amazon.com/AmazonS3/latest/dev/acl-overview.html.
+   */
+  enum ACLType {
+    // Allows grantee to list the objects in the bucket
+    READ("READ"),
+    // Allows grantee to create, overwrite, and delete any object in the bucket
+    WRITE("WRITE"),
+    // Allows grantee to write the bucket ACL
+    READ_ACP("READ_ACP"),
+    // Allows grantee to write the ACL for the applicable bucket
+    WRITE_ACP("WRITE_ACP"),
+    // Allows grantee above all permissions on the bucket
+    FULL_CONTROL("FULL_CONTROL");
+
+    public String getValue() {
+      return value;
+    }
+    /**
+     * String value for this Enum.
+     */
+    private final String value;
+
+    /**
+     * @param val String type for this enum.
+     */
+    ACLType(String val) {
+      value = val;
+    }
+
+
+    public static ACLType getType(String typeStr) {
+      for(ACLType type: ACLType.values()) {
+        if (type.getValue().equals(typeStr)) {
+          return type;
+        }
+      }
+      return null;
+    }
+  }
+
+  enum ACLIdentityType {
+    USER("CanonicalUser", true, "id"),
+    GROUP("Group", false, "url"),
+    USER_BY_EMAIL("AmazonCustomerByEmail", false, "emailAddress");
+
+    public String getGranteeType() {
+      return granteeType;
+    }
+
+    public String getHeaderType() {
+      return granteeInHeader;
+    }
+
+    /**
+     *  Grantee type in body XML.
+     */
+    private final String granteeType;
+
+    /**
+     * Is this type supported or not.
+     */
+    private final boolean supported;
+
+    /**
+     * Grantee type in header.
+     */
+    private final String granteeInHeader;
+
+    /**
+     * Init OzoneACLtypes enum.
+     *
+     * @param val String type for this enum.
+     */
+    ACLIdentityType(String val, boolean support, String headerType) {
+      granteeType = val;
+      supported = support;
+      granteeInHeader = headerType;
+    }
+
+    boolean isSupported() {
+      return supported;
+    }
+
+    public static ACLIdentityType getTypeFromGranteeType(String typeStr) {
+      for(ACLIdentityType type: ACLIdentityType.values()) {
+        if (type.getGranteeType().equals(typeStr)) {
+          return type;
+        }
+      }
+      return null;
+    }
+
+    public static ACLIdentityType getTypeFromHeaderType(String typeStr) {
+      for(ACLIdentityType type: ACLIdentityType.values()) {
+        if (type.getHeaderType().equals(typeStr)) {
+          return type;
+        }
+      }
+      return null;
+    }
+  }
+
+  public static boolean isGranteeTypeSupported(String typeStr) {
+    ACLIdentityType type =  ACLIdentityType.getTypeFromGranteeType(typeStr);
+    return type == null ? false : type.isSupported();
+  }
+
+  public static boolean isHeaderTypeSupported(String typeStr) {
+    ACLIdentityType type =  ACLIdentityType.getTypeFromHeaderType(typeStr);
+    return type == null ? false : type.isSupported();
+  }
+
+  public static List<Grant> ozoneNativeAclToS3Acl(OzoneAcl ozoneAcl) {
+    // Since currently only "CanonicalUser" is supported, which maps to Ozone
+    // "USER"
+    List<Grant> grantList = new ArrayList<>();
+    if (ozoneAcl.getType() != IAccessAuthorizer.ACLIdentityType.USER) {
+      return grantList;
+    }
+
+    Grantee grantee = new Grantee();
+    grantee.setDisplayName(ozoneAcl.getName());
+    grantee.setId(ozoneAcl.getName());
+
+    List<IAccessAuthorizer.ACLType> acls = ozoneAcl.getAclList();
+    if (acls.contains(IAccessAuthorizer.ACLType.ALL)) {
+      Grant grant = new Grant();
+      grant.setGrantee(grantee);
+      grant.setPermission(ACLType.FULL_CONTROL.toString());
+      grantList.add(grant);
+      return grantList;
+    } else if (acls.contains(IAccessAuthorizer.ACLType.WRITE_ACL)) {
+      Grant grant = new Grant();
+      grant.setGrantee(grantee);
+      grant.setPermission(ACLType.WRITE_ACP.toString());
+      grantList.add(grant);
+    } else if (acls.contains(IAccessAuthorizer.ACLType.READ_ACL)) {
+      Grant grant = new Grant();
+      grant.setGrantee(grantee);
+      grant.setPermission(ACLType.READ_ACP.toString());
+      grantList.add(grant);
+    } else if (acls.contains(IAccessAuthorizer.ACLType.WRITE) &&
+        acls.contains(IAccessAuthorizer.ACLType.DELETE) &&
+        acls.contains(IAccessAuthorizer.ACLType.CREATE)) {
+      Grant grant = new Grant();
+      grant.setGrantee(grantee);
+      grant.setPermission(ACLType.WRITE.toString());
+      grantList.add(grant);
+    } else if (acls.contains(IAccessAuthorizer.ACLType.READ) &&
+        acls.contains(IAccessAuthorizer.ACLType.LIST)) {
+      Grant grant = new Grant();
+      grant.setGrantee(grantee);
+      grant.setPermission(ACLType.READ.toString());
+      grantList.add(grant);
+    } else {
+      LOG.error("Cannot find a good mapping for Ozone ACL {} to S3",
+          ozoneAcl.toString());
+    }
+    return grantList;
+  }
+
+  public static List<OzoneAcl> s3AclToOzoneNativeAclOnBucket(
+      S3BucketAcl bucketAcl) throws OS3Exception {
+    List<OzoneAcl> ozoneAclList = new ArrayList<>();
+    List<Grant> grantList = bucketAcl.getAclList().getGrantList();
+    for (Grant grant : grantList) {
+      //  Only "CanonicalUser" is supported, which maps to Ozone "USER"
+      ACLIdentityType identityType = ACLIdentityType.getTypeFromGranteeType(
+          grant.getGrantee().getXsiType());
+      if (identityType != null && identityType.isSupported()) {
+        String permission = grant.getPermission();
+        BitSet acls = getOzoneAclOnBucketFromS3Permission(permission);
+        OzoneAcl defaultOzoneAcl = new OzoneAcl(
+            IAccessAuthorizer.ACLIdentityType.USER,
+            grant.getGrantee().getId(), acls,
+            OzoneAcl.AclScope.DEFAULT);
+        OzoneAcl accessOzoneAcl = new OzoneAcl(
+            IAccessAuthorizer.ACLIdentityType.USER,
+            grant.getGrantee().getId(), acls,
+            OzoneAcl.AclScope.ACCESS);
+        ozoneAclList.add(defaultOzoneAcl);
+        ozoneAclList.add(accessOzoneAcl);
+      } else {
+        LOG.error("Grantee type {} is not supported",
+            grant.getGrantee().getXsiType());
+        throw S3ErrorTable.newError(NOT_IMPLEMENTED,
+            grant.getGrantee().getXsiType());
+      }
+    }
+    return ozoneAclList;
+  }
+
+  public static BitSet getOzoneAclOnBucketFromS3Permission(String permission)
+      throws OS3Exception {
+    ACLType permissionType = ACLType.getType(permission);
+    if (permissionType == null) {
+      throw S3ErrorTable.newError(S3ErrorTable.INVALID_ARGUMENT, permission);
+    }
+    BitSet acls = new BitSet(IAccessAuthorizer.ACLType.getNoOfAcls());
+    switch (permissionType) {
+    case FULL_CONTROL:
+      acls.set(IAccessAuthorizer.ACLType.ALL.ordinal());
+      break;
+    case WRITE_ACP:
+      acls.set(IAccessAuthorizer.ACLType.WRITE_ACL.ordinal());
+      break;
+    case READ_ACP:
+      acls.set(IAccessAuthorizer.ACLType.READ_ACL.ordinal());
+      break;
+    case WRITE:
+      acls.set(IAccessAuthorizer.ACLType.WRITE.ordinal());
+      acls.set(IAccessAuthorizer.ACLType.DELETE.ordinal());
+      acls.set(IAccessAuthorizer.ACLType.CREATE.ordinal());
+      break;
+    case READ:
+      acls.set(IAccessAuthorizer.ACLType.READ.ordinal());
+      acls.set(IAccessAuthorizer.ACLType.LIST.ordinal());
+      break;
+    default:
+      LOG.error("Failed to recognize S3 permission {}", permission);
+      throw S3ErrorTable.newError(INVALID_ARGUMENT, permission);
+    }
+    return acls;
+  }
+
+  public static List<OzoneAcl> s3AclToOzoneNativeAclOnVolume(
+      S3BucketAcl bucketAcl) throws OS3Exception {
+    List<OzoneAcl> ozoneAclList = new ArrayList<>();
+    List<Grant> grantList = bucketAcl.getAclList().getGrantList();
+    for (Grant grant : grantList) {
+      //  Only "CanonicalUser" is supported, which maps to Ozone "USER"
+      ACLIdentityType identityType = ACLIdentityType.getTypeFromGranteeType(
+          grant.getGrantee().getXsiType());
+      if (identityType != null && identityType.isSupported()) {
+        String permission = grant.getPermission();
+        BitSet acls = getOzoneAclOnVolumeFromS3Permission(permission);
+        OzoneAcl defaultOzoneAcl = new OzoneAcl(
+            IAccessAuthorizer.ACLIdentityType.USER,

Review comment:
       s3 acl may have multiple grants for the same user. When it is converted to OzoneAcl, we should combine to take the advantage of BitSet.

##########
File path: hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java
##########
@@ -353,6 +375,215 @@ public MultiDeleteResponse multiDelete(@PathParam("bucket") String bucketName,
     return result;
   }
 
+  /**
+   * Implement acl get.
+   * <p>
+   * see: https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketAcl.html
+   */
+  public S3BucketAcl getAcl(String bucketName)
+      throws OS3Exception, IOException {
+    S3BucketAcl result = new S3BucketAcl();
+    try {
+      OzoneBucket bucket = getBucket(bucketName);
+      OzoneVolume volume = getVolume();
+      S3Owner owner = new S3Owner(volume.getOwner(), volume.getOwner());
+      result.setOwner(owner);
+      // Use set to remove ACLs with different scopes(ACCESS and DEFAULT)
+      Set<Grant> grantSet = new HashSet<>();
+      // Return ACL list
+      for (OzoneAcl acl : bucket.getAcls()) {
+        List<Grant> grants = S3Acl.ozoneNativeAclToS3Acl(acl);
+        grantSet.addAll(grants);
+      }
+      ArrayList<Grant> grantList = new ArrayList<>();
+      grantList.addAll(grantSet);
+      result.setAclList(
+          new S3BucketAcl.AccessControlList(grantList));
+      return result;
+    } catch (OMException ex) {
+      if (ex.getResult() == ResultCodes.BUCKET_NOT_FOUND) {
+        throw S3ErrorTable.newError(S3ErrorTable
+            .NO_SUCH_BUCKET, bucketName);
+      } else if (ex.getResult() == ResultCodes.PERMISSION_DENIED) {
+        throw S3ErrorTable.newError(S3ErrorTable
+            .ACCESS_DENIED, bucketName);
+      } else {
+        LOG.error("Failed to get acl of Bucket " + bucketName, ex);
+        throw S3ErrorTable.newError(S3ErrorTable.INTERNAL_ERROR, bucketName);
+      }
+    }
+  }
+
+  /**
+   * Implement acl put.
+   * <p>
+   * see: https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutBucketAcl.html
+   */
+  public Response putAcl(String bucketName, HttpHeaders httpHeaders,
+      InputStream body) throws IOException, OS3Exception {
+    String grantReads = httpHeaders.getHeaderString(S3Acl.grantRead);
+    String grantWrites = httpHeaders.getHeaderString(S3Acl.grantWrite);
+    String grantReadACP = httpHeaders.getHeaderString(S3Acl.grantReadACP);
+    String grantWriteACP = httpHeaders.getHeaderString(S3Acl.grantWriteACP);
+    String grantFull = httpHeaders.getHeaderString(S3Acl.grantFullControl);
+
+    try {
+      OzoneBucket bucket = getBucket(bucketName);
+      OzoneVolume volume = getVolume();
+
+      List<OzoneAcl> ozoneAclListOnBucket = new ArrayList<>();
+      List<OzoneAcl> ozoneAclListOnVolume = new ArrayList<>();
+
+      if (grantReads == null && grantWrites == null && grantReadACP == null
+          && grantWriteACP == null && grantFull == null) {
+        S3BucketAcl putBucketAclRequest =
+            new PutBucketAclRequestUnmarshaller().readFrom(
+            null, null, null, null, null, body);
+        // Handle grants in body
+        ozoneAclListOnBucket.addAll(
+            S3Acl.s3AclToOzoneNativeAclOnBucket(putBucketAclRequest));
+        ozoneAclListOnVolume.addAll(
+            S3Acl.s3AclToOzoneNativeAclOnVolume(putBucketAclRequest));
+      } else {
+
+        // Handle grants in headers
+        if (grantReads != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantReads,
+              S3Acl.ACLType.READ.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantReads,
+              S3Acl.ACLType.READ.getValue()));
+        }
+        if (grantWrites != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantWrites,
+              S3Acl.ACLType.WRITE.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantWrites,
+              S3Acl.ACLType.WRITE.getValue()));
+        }
+        if (grantReadACP != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantReadACP,
+              S3Acl.ACLType.READ_ACP.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantReadACP,
+              S3Acl.ACLType.READ_ACP.getValue()));
+        }
+        if (grantWriteACP != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantWriteACP,
+              S3Acl.ACLType.WRITE_ACP.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantWriteACP,
+              S3Acl.ACLType.WRITE_ACP.getValue()));
+        }
+        if (grantFull != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantFull,
+              S3Acl.ACLType.FULL_CONTROL.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantFull,
+              S3Acl.ACLType.FULL_CONTROL.getValue()));
+        }
+      }
+
+      List<OzoneAcl> oldBucketAcls = bucket.getAcls();
+      List<OzoneAcl> oldVolumeAcls =
+          S3Acl.getVolumeAclFromBucketAcl(oldBucketAcls);
+
+      // Add new ACLs
+      for (OzoneAcl addAcl : ozoneAclListOnBucket) {
+        bucket.addAcl(addAcl);

Review comment:
       why don't we call volume/bucket setAcl() directly instead of add new/remove old?

##########
File path: hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java
##########
@@ -353,6 +375,215 @@ public MultiDeleteResponse multiDelete(@PathParam("bucket") String bucketName,
     return result;
   }
 
+  /**
+   * Implement acl get.
+   * <p>
+   * see: https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketAcl.html
+   */
+  public S3BucketAcl getAcl(String bucketName)
+      throws OS3Exception, IOException {
+    S3BucketAcl result = new S3BucketAcl();
+    try {
+      OzoneBucket bucket = getBucket(bucketName);
+      OzoneVolume volume = getVolume();
+      S3Owner owner = new S3Owner(volume.getOwner(), volume.getOwner());
+      result.setOwner(owner);
+      // Use set to remove ACLs with different scopes(ACCESS and DEFAULT)
+      Set<Grant> grantSet = new HashSet<>();
+      // Return ACL list
+      for (OzoneAcl acl : bucket.getAcls()) {
+        List<Grant> grants = S3Acl.ozoneNativeAclToS3Acl(acl);
+        grantSet.addAll(grants);
+      }
+      ArrayList<Grant> grantList = new ArrayList<>();
+      grantList.addAll(grantSet);
+      result.setAclList(
+          new S3BucketAcl.AccessControlList(grantList));
+      return result;
+    } catch (OMException ex) {
+      if (ex.getResult() == ResultCodes.BUCKET_NOT_FOUND) {
+        throw S3ErrorTable.newError(S3ErrorTable
+            .NO_SUCH_BUCKET, bucketName);
+      } else if (ex.getResult() == ResultCodes.PERMISSION_DENIED) {
+        throw S3ErrorTable.newError(S3ErrorTable
+            .ACCESS_DENIED, bucketName);
+      } else {
+        LOG.error("Failed to get acl of Bucket " + bucketName, ex);
+        throw S3ErrorTable.newError(S3ErrorTable.INTERNAL_ERROR, bucketName);
+      }
+    }
+  }
+
+  /**
+   * Implement acl put.
+   * <p>
+   * see: https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutBucketAcl.html
+   */
+  public Response putAcl(String bucketName, HttpHeaders httpHeaders,
+      InputStream body) throws IOException, OS3Exception {
+    String grantReads = httpHeaders.getHeaderString(S3Acl.grantRead);
+    String grantWrites = httpHeaders.getHeaderString(S3Acl.grantWrite);
+    String grantReadACP = httpHeaders.getHeaderString(S3Acl.grantReadACP);
+    String grantWriteACP = httpHeaders.getHeaderString(S3Acl.grantWriteACP);
+    String grantFull = httpHeaders.getHeaderString(S3Acl.grantFullControl);
+
+    try {
+      OzoneBucket bucket = getBucket(bucketName);
+      OzoneVolume volume = getVolume();
+
+      List<OzoneAcl> ozoneAclListOnBucket = new ArrayList<>();
+      List<OzoneAcl> ozoneAclListOnVolume = new ArrayList<>();
+
+      if (grantReads == null && grantWrites == null && grantReadACP == null
+          && grantWriteACP == null && grantFull == null) {
+        S3BucketAcl putBucketAclRequest =
+            new PutBucketAclRequestUnmarshaller().readFrom(
+            null, null, null, null, null, body);
+        // Handle grants in body
+        ozoneAclListOnBucket.addAll(
+            S3Acl.s3AclToOzoneNativeAclOnBucket(putBucketAclRequest));
+        ozoneAclListOnVolume.addAll(
+            S3Acl.s3AclToOzoneNativeAclOnVolume(putBucketAclRequest));
+      } else {
+
+        // Handle grants in headers
+        if (grantReads != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantReads,
+              S3Acl.ACLType.READ.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantReads,
+              S3Acl.ACLType.READ.getValue()));
+        }
+        if (grantWrites != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantWrites,
+              S3Acl.ACLType.WRITE.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantWrites,
+              S3Acl.ACLType.WRITE.getValue()));
+        }
+        if (grantReadACP != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantReadACP,
+              S3Acl.ACLType.READ_ACP.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantReadACP,
+              S3Acl.ACLType.READ_ACP.getValue()));
+        }
+        if (grantWriteACP != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantWriteACP,
+              S3Acl.ACLType.WRITE_ACP.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantWriteACP,
+              S3Acl.ACLType.WRITE_ACP.getValue()));
+        }
+        if (grantFull != null) {
+          ozoneAclListOnBucket.addAll(getAndConvertAclOnBucket(grantFull,
+              S3Acl.ACLType.FULL_CONTROL.getValue()));
+          ozoneAclListOnVolume.addAll(getAndConvertAclOnVolume(grantFull,
+              S3Acl.ACLType.FULL_CONTROL.getValue()));
+        }
+      }
+
+      List<OzoneAcl> oldBucketAcls = bucket.getAcls();
+      List<OzoneAcl> oldVolumeAcls =
+          S3Acl.getVolumeAclFromBucketAcl(oldBucketAcls);
+
+      // Add new ACLs
+      for (OzoneAcl addAcl : ozoneAclListOnBucket) {
+        bucket.addAcl(addAcl);
+      }
+      for (OzoneAcl addAcl : ozoneAclListOnVolume) {
+        volume.addAcl(addAcl);
+      }
+
+      // A put request will reset all previous ACLs
+      for (OzoneAcl removeAcl : oldBucketAcls) {
+        bucket.removeAcl(removeAcl);
+      }
+      for (OzoneAcl removeAcl : oldVolumeAcls) {
+        volume.removeAcl(removeAcl);
+      }
+    } catch (OMException exception) {
+      LOG.error("Error in set ACL Request for bucket: {}", bucketName,
+          exception);
+      if (exception.getResult() == ResultCodes.BUCKET_NOT_FOUND) {
+        throw S3ErrorTable.newError(S3ErrorTable.NO_SUCH_BUCKET,
+            bucketName);
+      } else if (exception.getResult() == ResultCodes.PERMISSION_DENIED) {
+        throw S3ErrorTable.newError(S3ErrorTable
+            .ACCESS_DENIED, bucketName);
+      }
+      throw exception;
+    }
+    return Response.status(HttpStatus.SC_OK).build();
+  }
+
+  /**
+   *  Example: x-amz-grant-write: \
+   *  uri="http://acs.amazonaws.com/groups/s3/LogDelivery", id="111122223333", \
+   *  id="555566667777".
+   */
+  private List<OzoneAcl> getAndConvertAclOnBucket(String value,
+      String permission) throws OS3Exception {
+    List<OzoneAcl> ozoneAclList = new ArrayList<>();
+    if (StringUtils.isEmpty(value)) {
+      return ozoneAclList;
+    }
+    String[] subValues = value.split(",");
+    for (String acl: subValues) {
+      String[] part = acl.split("=");
+      if (part.length != 2) {
+        throw S3ErrorTable.newError(S3ErrorTable.INVALID_ARGUMENT, acl);
+      }
+      S3Acl.ACLIdentityType type =
+          S3Acl.ACLIdentityType.getTypeFromHeaderType(part[0]);
+      if (type == null || !type.isSupported()) {
+        LOG.warn("S3 grantee {} is null or not supported", part[0]);
+        throw S3ErrorTable.newError(NOT_IMPLEMENTED, part[0]);
+      }
+      // Build ACL on Bucket
+      BitSet aclsOnBucket =
+          S3Acl.getOzoneAclOnBucketFromS3Permission(permission);
+      OzoneAcl defaultOzoneAcl = new OzoneAcl(
+          IAccessAuthorizer.ACLIdentityType.USER, part[1], aclsOnBucket,
+          OzoneAcl.AclScope.DEFAULT);
+      OzoneAcl accessOzoneAcl = new OzoneAcl(
+          IAccessAuthorizer.ACLIdentityType.USER, part[1], aclsOnBucket,
+          OzoneAcl.AclScope.ACCESS);
+      ozoneAclList.add(defaultOzoneAcl);
+      ozoneAclList.add(accessOzoneAcl);
+    }
+    return ozoneAclList;
+  }
+
+  private List<OzoneAcl> getAndConvertAclOnVolume(String value,
+      String permission) throws OS3Exception {
+    List<OzoneAcl> ozoneAclList = new ArrayList<>();
+    if (StringUtils.isEmpty(value)) {
+      return ozoneAclList;
+    }
+    String[] subValues = value.split(",");
+    for (String acl: subValues) {
+      String[] part = acl.split("=");
+      if (part.length != 2) {
+        throw S3ErrorTable.newError(S3ErrorTable.INVALID_ARGUMENT, acl);
+      }
+      S3Acl.ACLIdentityType type =
+          S3Acl.ACLIdentityType.getTypeFromHeaderType(part[0]);
+      if (type == null || !type.isSupported()) {
+        LOG.warn("S3 grantee {} is null or not supported", part[0]);
+        throw S3ErrorTable.newError(NOT_IMPLEMENTED, part[0]);
+      }
+      // Build ACL on Bucket

Review comment:
       NIT: you mean volume?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org