xiaoyuyao commented on a change in pull request #1829: HDFS-14743. Enhance
INodeAttributeProvider/ AccessControlEnforcer Interface in HDFS to support
Authorization of mkdir, rm, rmdir, copy, move etc...
URL: https://github.com/apache/hadoop/pull/1829#discussion_r377977917
##########
File path:
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeAttributeProvider.java
##########
@@ -17,19 +17,227 @@
*/
package org.apache.hadoop.hdfs.server.namenode;
+import com.google.common.annotations.VisibleForTesting;
import org.apache.commons.lang3.StringUtils;
import org.apache.hadoop.classification.InterfaceAudience;
import org.apache.hadoop.classification.InterfaceStability;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.fs.permission.FsAction;
import org.apache.hadoop.hdfs.DFSUtil;
+import org.apache.hadoop.ipc.CallerContext;
import org.apache.hadoop.security.AccessControlException;
import org.apache.hadoop.security.UserGroupInformation;
@InterfaceAudience.Public
@InterfaceStability.Unstable
public abstract class INodeAttributeProvider {
+ public static class AuthorizationContext {
+ public String fsOwner;
+ public String supergroup;
+ public UserGroupInformation callerUgi;
+ public INodeAttributes[] inodeAttrs;
+ public INode[] inodes;
+ public byte[][] pathByNameArr;
+ public int snapshotId;
+ public String path;
+ public int ancestorIndex;
+ public boolean doCheckOwner;
+ public FsAction ancestorAccess;
+ public FsAction parentAccess;
+ public FsAction access;
+ public FsAction subAccess;
+ public boolean ignoreEmptyDir;
+ public String operationName;
+ public CallerContext callerContext;
+
+ public static class Builder {
+ public String fsOwner;
+ public String supergroup;
+ public UserGroupInformation callerUgi;
+ public INodeAttributes[] inodeAttrs;
+ public INode[] inodes;
+ public byte[][] pathByNameArr;
+ public int snapshotId;
+ public String path;
+ public int ancestorIndex;
+ public boolean doCheckOwner;
+ public FsAction ancestorAccess;
+ public FsAction parentAccess;
+ public FsAction access;
+ public FsAction subAccess;
+ public boolean ignoreEmptyDir;
+ public String operationName;
+ public CallerContext callerContext;
+
+ public AuthorizationContext build() {
+ return new AuthorizationContext(this);
+ }
+
+ public Builder fsOwner(String val) {
+ this.fsOwner = val;
+ return this;
+ }
+
+ public Builder supergroup(String val) {
+ this.supergroup = val;
+ return this;
+ }
+
+ public Builder callerUgi(UserGroupInformation val) {
+ this.callerUgi = val;
+ return this;
+ }
+
+ public Builder inodeAttrs(INodeAttributes[] val) {
+ this.inodeAttrs = val;
+ return this;
+ }
+
+ public Builder inodes(INode[] val) {
+ this.inodes = val;
+ return this;
+ }
+
+ public Builder pathByNameArr(byte[][] val) {
+ this.pathByNameArr = val;
+ return this;
+ }
+
+ public Builder snapshotId(int val) {
+ this.snapshotId = val;
+ return this;
+ }
+
+ public Builder path(String val) {
+ this.path = val;
+ return this;
+ }
+
+ public Builder ancestorIndex(int val) {
+ this.ancestorIndex = val;
+ return this;
+ }
+
+ public Builder doCheckOwner(boolean val) {
+ this.doCheckOwner = val;
+ return this;
+ }
+
+ public Builder ancestorAccess(FsAction val) {
+ this.ancestorAccess = val;
+ return this;
+ }
+
+ public Builder parentAccess(FsAction val) {
+ this.parentAccess = val;
+ return this;
+ }
+
+ public Builder access(FsAction val) {
+ this.access = val;
+ return this;
+ }
+
+ public Builder subAccess(FsAction val) {
+ this.subAccess = val;
+ return this;
+ }
+
+ public Builder ignoreEmptyDir(boolean val) {
+ this.ignoreEmptyDir = val;
+ return this;
+ }
+
+ public Builder operationName(String val) {
+ this.operationName = val;
+ return this;
+ }
+
+ public Builder callerContext(CallerContext val) {
+ this.callerContext = val;
+ return this;
+ }
+ }
+
+ public AuthorizationContext(
+ String fsOwner,
+ String supergroup,
+ UserGroupInformation callerUgi,
+ INodeAttributes[] inodeAttrs,
+ INode[] inodes,
+ byte[][] pathByNameArr,
+ int snapshotId,
+ String path,
+ int ancestorIndex,
+ boolean doCheckOwner,
+ FsAction ancestorAccess,
+ FsAction parentAccess,
+ FsAction access,
+ FsAction subAccess,
+ boolean ignoreEmptyDir) {
+ this.fsOwner = fsOwner;
+ this.supergroup = supergroup;
+ this.callerUgi = callerUgi;
+ this.inodeAttrs = inodeAttrs;
+ this.inodes = inodes;
+ this.pathByNameArr = pathByNameArr;
+ this.snapshotId = snapshotId;
+ this.path = path;
+ this.ancestorIndex = ancestorIndex;
+ this.doCheckOwner = doCheckOwner;
+ this.ancestorAccess = ancestorAccess;
+ this.parentAccess = parentAccess;
+ this.access = access;
+ this.subAccess = subAccess;
+ this.ignoreEmptyDir = ignoreEmptyDir;
+ }
+
+ public AuthorizationContext(
+ String fsOwner,
+ String supergroup,
+ UserGroupInformation callerUgi,
+ INodeAttributes[] inodeAttrs,
+ INode[] inodes,
+ byte[][] pathByNameArr,
+ int snapshotId,
+ String path,
+ int ancestorIndex,
+ boolean doCheckOwner,
+ FsAction ancestorAccess,
+ FsAction parentAccess,
+ FsAction access,
+ FsAction subAccess,
+ boolean ignoreEmptyDir,
+ String operationName,
+ CallerContext callerContext) {
+ this(fsOwner, supergroup, callerUgi, inodeAttrs, inodes,
+ pathByNameArr, snapshotId, path, ancestorIndex, doCheckOwner,
+ ancestorAccess, parentAccess, access, subAccess, ignoreEmptyDir);
+ this.operationName = operationName;
+ this.callerContext = callerContext;
+ }
+
+ public AuthorizationContext(Builder builder) {
+ this(builder.fsOwner, builder.supergroup, builder.callerUgi,
+ builder.inodeAttrs, builder.inodes, builder.pathByNameArr,
+ builder.snapshotId, builder.path, builder.ancestorIndex,
+ builder.doCheckOwner, builder.ancestorAccess, builder.parentAccess,
+ builder.access, builder.subAccess, builder.ignoreEmptyDir);
+ this.operationName = builder.operationName;
+ this.callerContext = builder.callerContext;
+ }
+
+ @VisibleForTesting
+ @Override
+ public boolean equals(Object obj) {
+ if (!(obj instanceof AuthorizationContext)) {
+ return false;
+ }
+ return true;
Review comment:
I don't think it is right to return always return true for two different
AuthorizationContext instances.
----------------------------------------------------------------
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:
us...@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
