[ 
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

Reply via email to