[ 
https://issues.apache.org/jira/browse/YARN-3100?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14297828#comment-14297828
 ] 

Zhijie Shen commented on YARN-3100:
-----------------------------------

bq. I would like to have YARN ready first and then considering merging the 
common part into a common interface.

+1 for staring from YARN first.

Here're some comments about the patch.

1. Should {{AccessType}} be the first class citizen too like 
{{PrivilegedEntity}}? And there are 1-to-many relationship between 
{{PrivilegedEntity}} and {{AccessType}}? For example, Queue -> \{ADD, DELETE, 
MODIFY, VIEW\}, App -> 
\{SUBMIT, VIEW\}.

2. Do we need to include application as another entity type?

3. No need in {{AdminService.java}} any more.
{code}
private AccessControlList adminAcl;
{code}

4. YarnAuthorizationProvider needs to be annotated? The following java doc is 
not as complete as the other methods.
{code}
          /**
           * Check if the user is an admin.
           */
   public abstract boolean isAdmin(UserGroupInformation ugi);
{code}

5. According to the name, signature and implemenation, the method seems not to 
be just for verify admin access, but generich. However, it's only used in 
AdminService now. After the code change, the method reduces the use case to 
only check if it is admin. We may want to avoid the change or refactor the 
method name.
{code}
  public static UserGroupInformation verifyAccess(
      AccessControlList acl, String method, final Log LOG)
      throws IOException {
{code}

6. For YarnAuthorizationProvider, it's better to call init() in getInstance(), 
such that the user doesn't need to struggle to make sure init() is just called 
once and at the first reference. Just think out loudly. Is the singleton 
friendly to MiniYARNCluster, where different components reside in a single 
process?

> Make YARN authorization pluggable
> ---------------------------------
>
>                 Key: YARN-3100
>                 URL: https://issues.apache.org/jira/browse/YARN-3100
>             Project: Hadoop YARN
>          Issue Type: Bug
>            Reporter: Jian He
>            Assignee: Jian He
>         Attachments: YARN-3100.1.patch, YARN-3100.2.patch
>
>
> The goal is to have YARN acl model pluggable so as to integrate other 
> authorization tool such as Apache Ranger, Sentry.
> Currently, we have 
> - admin ACL
> - queue ACL
> - application ACL
> - time line domain ACL
> - service ACL
> The proposal is to create a YarnAuthorizationProvider interface. Current 
> implementation will be the default implementation. Ranger or Sentry plug-in 
> can implement  this interface.
> Benefit:
> -  Unify the code base. With the default implementation, we can get rid of 
> each specific ACL manager such as AdminAclManager, ApplicationACLsManager, 
> QueueAclsManager etc.
> - Enable Ranger, Sentry to do authorization for YARN. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to