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

Jason Lowe commented on YARN-6820:
----------------------------------

bq. DEFAULT_TIMELINE_SERVICE_READ_ALLOWED_USERS should be star *. It is empty 
now.

I do not agree.  The whole point of this JIRA is to block all users from seeing 
the data in the ATS.  The feature already has a master enable that defaults to 
off, so by default all users can read the data.  If a user bothers to flip the 
master enable to on, it should not have zero effect by default.  IMHO once the 
master enable is turned on, it should only allow the configured YARN admins to 
read the data by default, and the config needs to be explicitly updated to 
allow any other users to read.  Therefore I believe an empty value for this 
default is correct.

Speaking of which, the following code can NPE:
{code}
      String adminAclListStr =
          conf.getInitParameter(YarnConfiguration.YARN_ADMIN_ACL);
      if (StringUtils.isEmpty(adminAclListStr)) {
        adminAclList = new AccessControlList(
            YarnConfiguration.DEFAULT_TIMELINE_SERVICE_READ_ALLOWED_USERS);
        LOG.info("adminAclList not set, hence setting it to "
            + " YarnConfiguration.DEFAULT_TIMELINE_SERVICE_READ_ALLOWED_USERS");
      }
      adminAclList = new AccessControlList(adminAclListStr);
{code}
because adminAclListStr is always passed to AccessControlList and could be 
null.  It also doesn't make sense to log a message that references code symbols 
for property values since users won't be familiar with those.  We also 
shouldn't assume that the whitelist reader default makes a good admin default.  
Even if it wasn't empty, we shouldn't assume the default reader list should be 
a default admin list.  Therefore I think it should be simplified to something 
like:
{code}
      String adminAclListStr =
          conf.getInitParameter(YarnConfiguration.YARN_ADMIN_ACL);
      if (StringUtils.isEmpty(adminAclListStr)) {
        adminAclListStr = "";
      }
      adminAclList = new AccessControlList(adminAclListStr);
{code}

Same comment applies to the code where we initialize the filter config.  We 
should explicitly set it to "" (or a static final String property specific to 
this filter that has that value) rather than assume default read allowed makes 
a good default admin value.

bq.  HttpServeletRequest#remoteuser will be always null when we access from 
browsers. I doubt that in normal browser, we get always AuthorizationException. 
How ever it is expected if user is not authenticated. But my doubt is should we 
get user from principle name?

RMWebServices gets the user name from the principal, and I think we would need 
to do the same here.

> Restrict read access to timelineservice v2 data 
> ------------------------------------------------
>
>                 Key: YARN-6820
>                 URL: https://issues.apache.org/jira/browse/YARN-6820
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: timelineserver
>            Reporter: Vrushali C
>            Assignee: Vrushali C
>              Labels: yarn-5355-merge-blocker
>         Attachments: YARN-6820-YARN-5355.0001.patch, 
> YARN-6820-YARN-5355.002.patch, YARN-6820-YARN-5355.003.patch
>
>
> Need to provide a way to restrict read access in ATSv2. Not all users should 
> be able to read all entities. On the flip side, some folks may not need any 
> read restrictions, so we need to provide a way to disable this access 
> restriction as well. 
> Initially this access restriction could be done in a simple way via a 
> whitelist of users allowed to read data. That set of users can read all data, 
> no other user can read any data. Can be turned off for all users to read all 
> data.
> Could be stored in a "domain" table in hbase perhaps. Or a configuration 
> setting for the cluster. Or something else that's simple enough. ATSv1 has a 
> concept of domain for isolating users for reading. Would be good to keep that 
> in consideration. 
> In ATSv1, domain offers a namespace for Timeline server allowing users to 
> host multiple entities, isolating them from other users and applications. A 
> “Domain” in ATSV1 primarily stores owner info, read and& write ACL 
> information, created and modified time stamp information. Each Domain is 
> identified by an ID which must be unique across all users in the YARN cluster.



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

---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org

Reply via email to