[ https://issues.apache.org/jira/browse/HDFS-12895?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16289556#comment-16289556 ]
Íñigo Goiri edited comment on HDFS-12895 at 12/13/17 7:53 PM: -------------------------------------------------------------- Thanks [~linyiqun] for taking care of the comments. A couple more based on [^HDFS-12895.005.patch]: * It seems we are defaulting to nulls when nothing there. The entries created early still show without permissions. We probably want to check for empty {{getMode()}} at {{MountTableImpl}}. Not sure about user and group; show a default one? Super user? This is what I currently get for old entries: {code} Mount Table Entries: Source Destinations Owner Group Mode / ns0->/ null null --------- /ns0 ns0->/ null null --------- /ns1 ns1->/ null null --------- /ns2 ns2->/ null null --------- /ns3 ns3->/ null null --------- {code} * {{removeMountTableEntry()}} could initialize {{deleteEntry}} right away without the null: {{final MountTable deleteEntry = getDriver().get(getRecordClass(), query);}} * For consistency, {{removeMountTableEntry()}} could do the same order of {{if}} as {{addMountTableEntry()}} and {{updateMountTableEntry()}} and avoid the {{pc}} init if no entry. In addition, the check status could use this if structure: {code} boolean status = false; if (deleteEntry != null) { RouterPermissionChecker pc = RouterAdminServer.getPermissionChecker(); if (pc != null) { pc.checkPermission(deleteEntry, FsAction.WRITE); } status = getDriver().remove(deleteEntry); } {code} * Typo in the name {{testMountTableDefalutACL}} Other than this, this is ready to go. was (Author: elgoiri): Thanks [~linyiqun] for taking care of the comments. A couple more based on [^HDFS-12895.005.patch]: * It seems we are defaulting to nulls when nothing there. The entries created early still show without permissions. We probably want to check for empty {{getMode()}} at {{MountTableImpl}}. Not sure about user and group; show a default one? Super user? This is what I currently get for old entries: {code} Mount Table Entries: Source Destinations Owner Group Mode / ns0->/ null null --------- /ns0 ns0->/ null null --------- /ns1 ns1->/ null null --------- /ns2 ns2->/ null null --------- /ns3 ns3->/ null null --------- {code} * {{removeMountTableEntry()}} could initialize {{deleteEntry}} right away without the null: {{final MountTable deleteEntry = getDriver().get(getRecordClass(), query);}} * For consistency, {{removeMountTableEntry()}} could do the same order of {{if}} as {{addMountTableEntry()}} and {{updateMountTableEntry()}} and avoid the {{pc}} init if no entry. In addition, the check status could use this if structure: {code} boolean status = false; if (deleteEntry != null) { RouterPermissionChecker pc = RouterAdminServer.getPermissionChecker(); if (pc != null) { pc.checkPermission(deleteEntry, FsAction.WRITE); } status = getDriver().remove(deleteEntry); } {code} Other than this, this is ready to go. > RBF: Add ACL support for mount table > ------------------------------------ > > Key: HDFS-12895 > URL: https://issues.apache.org/jira/browse/HDFS-12895 > Project: Hadoop HDFS > Issue Type: Sub-task > Affects Versions: 3.0.0-alpha3 > Reporter: Yiqun Lin > Assignee: Yiqun Lin > Labels: RBF > Attachments: HDFS-12895.001.patch, HDFS-12895.002.patch, > HDFS-12895.003.patch, HDFS-12895.004.patch, HDFS-12895.005.patch > > > Adding ACL support for the Mount Table management. Following is the initial > design of ACL control for the mount table management. > Each mount table has its owner, group name and permission. > The mount table permissions (FsPermission), here we use > {{org.apache.hadoop.fs.permission.FsPermission}} to do the access check: > # READ permission: you can read the mount table info. > # WRITE permission: you can add remove or update this mount table info. > # EXECUTE permission: This won't be used. > The add command of mount table will be extended like this > {noformat} > $HADOOP_HOME/bin/hdfs dfsrouteradmin [-add <source> <nameservice> > <destination> [-owner <owner>] [-group <group>] [-mode <mode>]] > {noformat} > *<mode> is UNIX-style permissions for the mount table. Permissions are > specified in octal, e.g. 0755. By default, this is set to 0755*. > If we want update the ACL info of specfied mount table, just execute add > command again. This command not only adding for new mount talle but also > updating mount table once it finds given mount table is existed. -- This message was sent by Atlassian JIRA (v6.4.14#64029) --------------------------------------------------------------------- To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org