Mailing-List: contact hdfs-issues-help@hadoop.apache.org; run by ezmlm
Precedence: bulk
Date: Wed, 13 Dec 2017 19:57:00 +0000 (UTC)
From: =?utf-8?Q?=C3=8D=C3=B1igo_Goiri_=28JIRA=29?= <jira@apache.org>
To: hdfs-issues@hadoop.apache.org
Message-ID: <JIRA.13123069.1512526518000.451521.1513195020239@Atlassian.JIRA>
In-Reply-To: <JIRA.13123069.1512526518000@Atlassian.JIRA>
References: <JIRA.13123069.1512526518000@Atlassian.JIRA> <JIRA.13123069.1512526518184@jira-lw-us.apache.org>
Subject: [jira] [Comment Edited] (HDFS-12895) RBF: Add ACL support for mount
 table
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
archived-at: Wed, 13 Dec 2017 19:57:05 -0000


    [ https://issues.apache.org/jira/browse/HDFS-12895?page=3Dcom.atlassian=
.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=3D1628=
9556#comment-16289556 ]=20

=C3=8D=C3=B1igo Goiri edited comment on HDFS-12895 at 12/13/17 7:56 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 creat=
ed early still show without permissions. We probably want to check for empt=
y {{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 w=
ithout the null: {{final MountTable deleteEntry =3D getDriver().get(getReco=
rdClass(), 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 =3D false;
if (deleteEntry !=3D null) {
  RouterPermissionChecker pc =3D RouterAdminServer.getPermissionChecker();
  if (pc !=3D null) {
    pc.checkPermission(deleteEntry, FsAction.WRITE);
  }
  status =3D getDriver().remove(deleteEntry);
}
{code}
* Typo in the name {{testMountTableDefalutACL}}
* Internally this fails {{assertEquals(ugi.getShortUserName(), mountTable.g=
etGroupName());}}, I think it shouldn't be {{getShortUserName()}} to compar=
e with the group.

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 creat=
ed early still show without permissions. We probably want to check for empt=
y {{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 w=
ithout the null: {{final MountTable deleteEntry =3D getDriver().get(getReco=
rdClass(), 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 =3D false;
if (deleteEntry !=3D null) {
  RouterPermissionChecker pc =3D RouterAdminServer.getPermissionChecker();
  if (pc !=3D null) {
    pc.checkPermission(deleteEntry, FsAction.WRITE);
  }
  status =3D getDriver().remove(deleteEntry);
}
{code}
* Typo in the name {{testMountTableDefalutACL}}

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-128=
95.003.patch, HDFS-12895.004.patch, HDFS-12895.005.patch
>
>
> Adding ACL support for the Mount Table management. Following is the initi=
al 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.hado=
op.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> <destin=
ation> [-owner <owner>] [-group <group>] [-mode <mode>]]
> {noformat}
> *<mode> is UNIX-style permissions for the mount table. Permissions are sp=
ecified 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 up=
dating mount table once it finds given mount table is existed.=20


--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

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