[
https://issues.apache.org/jira/browse/YARN-5956?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15863254#comment-15863254
]
Sunil G commented on YARN-5956:
-------------------------------
I think there is a bit confusion. Lemme give a detailed concern from my end.
*verifyUserAccessForRMApp* is defined as below.
{code}
private RMApp verifyUserAccessForRMApp(ApplicationId applicationId,
UserGroupInformation callerUGI, String operation) throws YarnException {
RMApp application = this.rmContext.getRMApps().get(applicationId);
if (application == null) {
RMAuditLogger.logFailure(callerUGI.getUserName(), operation, "UNKNOWN",
"ClientRMService",
"Trying to " + operation + " of an absent application",
applicationId);
throw new ApplicationNotFoundException("Trying to " + operation
+ " of an absent application " + applicationId);
}
if (!checkAccess(callerUGI, application.getUser(),
ApplicationAccessType.MODIFY_APP, application)) {
RMAuditLogger.logFailure(callerUGI.getShortUserName(), operation,
"User doesn't have permissions to "
+ ApplicationAccessType.MODIFY_APP.toString(),
"ClientRMService", AuditConstants.UNAUTHORIZED_USER, applicationId);
throw RPCUtil.getRemoteException(new AccessControlException("User "
+ callerUGI.getShortUserName() + " cannot perform operation "
+ ApplicationAccessType.MODIFY_APP.name() + " on " + applicationId));
}
return application;
}
{code}
Now lets see each of the api.
1. In {{getApplicationAttemptReport}}, {{getApplicationAttempts}},
{{getContainerReport}} and {{getContaines}}, below code is added.
{code}
UserGroupInformation callerUGI = getCallerUgi(applicationId,
AuditConstants.GET_APP_ATTEMPT_REPORT);
RMApp application = verifyUserAccessForRMApp(applicationId, callerUGI,
AuditConstants.GET_APP_ATTEMPT_REPORT);
boolean allowAccess = checkAccess(callerUGI, application.getUser(),
ApplicationAccessType.VIEW_APP, application);
{code}
Inside {{verifyUserAccessForRMApp}}, we do a {{checkAccess}} for *MODIFY_APP*.
Then in each of this apis ,we do one more time for *VIEW_APP* (see 3rd line in
above code snippet). This may cause issue because for any getter api, we just
need *VIEW_APP* permission. This change enforce a *MODIFY_APP* and will break
many valid use cases and breaks compatibility.
2. Change done to {{failApplicationAttempt}} looks correct to me as you avoided
a repeated call to {{checkAccess}}
> Refactor ClientRMService
> ------------------------
>
> Key: YARN-5956
> URL: https://issues.apache.org/jira/browse/YARN-5956
> Project: Hadoop YARN
> Issue Type: Improvement
> Components: resourcemanager
> Affects Versions: 3.0.0-alpha2
> Reporter: Kai Sasaki
> Assignee: Kai Sasaki
> Priority: Minor
> Attachments: YARN-5956.01.patch, YARN-5956.02.patch,
> YARN-5956.03.patch, YARN-5956.04.patch, YARN-5956.05.patch,
> YARN-5956.06.patch, YARN-5956.07.patch, YARN-5956.08.patch
>
>
> Some refactoring can be done in {{ClientRMService}}.
> - Remove redundant variable declaration
> - Fill in missing javadocs
> - Proper variable access modifier
> - Fix some typos in method name and exception messages
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
