[
https://issues.apache.org/jira/browse/YARN-9445?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16810922#comment-16810922
]
Szilard Nemeth commented on YARN-9445:
--------------------------------------
Hi [~shuzirra]!
Some comments:
In TestFairSchedulerACLAuthorizer:
1. Please extract the username strings to constants so you can re-use them in
the alloc file config strings and you don't need to repeat the strings.
2. Please extract the queue names as well, so that you can reuse the constants
in the testcases.
3. In generateAllocation: Pls use camelcase names for TEST_DIR / ALLOC_FILE.
Checkstyle will also complain about this.
4. For the user fields: All can be private. Pls also use javadoc instead of
simple comment to document these. I guess checkstyle will also complain about
this.
5. I would use as restrcited visibility as possible for helper methods like
assertHasAccess, as this class is not very likely to be subclassed, but this is
just my personal preference.
6. In assertHasAccess, you have assertTrue / assertFalse calls:
Please add some assertion message.
7. A javadoc could be added to the class and to the method generateAllocation:
As per our offline discussion, I know why you have 4 queues, but probably some
explanation javadoc could make the code more easy to understand for other
readers.
8. I would get rid of the following comments, these are just adding noise:
{code:java}
//enabling ACL
//setting scheduler to fair
//setting the queue allocation
//Setting the "global" YARN admin
{code}
I like the rest of the comments in the testcases.
9. I would introduce one method that checks a queue for both ACLs (admin /
submit) and receives a map: keys could be ACL type, values could be lists of
users. This would give much more readable code.
> yarn.admin.acl is futile
> ------------------------
>
> Key: YARN-9445
> URL: https://issues.apache.org/jira/browse/YARN-9445
> Project: Hadoop YARN
> Issue Type: Bug
> Components: security
> Affects Versions: 3.3.0
> Reporter: Peter Simon
> Assignee: Gergely Pollak
> Priority: Major
> Attachments: YARN-9445.001.patch
>
>
> * Define a queue with restrictive administerApps settings (e.g. yarn)
> * Set yarn.admin.acl to "*".
> * Try to submit an application with user yarn, it is denied.
> This way my expected behaviour would be that while everyone is admin, I can
> submit to whatever pool.
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
