adoroszlai commented on code in PR #4809:
URL: https://github.com/apache/ozone/pull/4809#discussion_r1212605650
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/HddsDatanodeService.java:
##########
@@ -669,18 +662,7 @@ public void saveNewCertId(String newCertId) {
*/
public void checkAdminUserPrivilege(UserGroupInformation ugi)
throws IOException {
- if (ugi != null && !isAdmin(ugi)) {
- throw new AccessControlException("Access denied for user "
- + ugi.getUserName() + ". Superuser privilege is required.");
- }
- }
-
- /**
- * Return true if a UserGroupInformation is admin, false otherwise.
- * @param callerUgi Caller UserGroupInformation
- */
- public boolean isAdmin(UserGroupInformation callerUgi) {
- return callerUgi != null && admins.isAdmin(callerUgi);
+ admins.isAdmin(ugi);
Review Comment:
`isAdmin` only returns `true` or `false`, we should call
`checkAdminUserPrivilege` here to preserve behavior.
```suggestion
admins.checkAdminUserPrivilege(ugi);
```
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java:
##########
@@ -642,19 +642,15 @@ private OzoneManager(OzoneConfiguration conf,
StartupOption startupOption)
perfMetrics = OMPerformanceMetrics.register();
// Get admin list
omStarterUser = UserGroupInformation.getCurrentUser().getShortUserName();
- Collection<String> omAdminUsernames =
- OzoneConfigUtil.getOzoneAdminsFromConfig(configuration, omStarterUser);
- Collection<String> omAdminGroups =
- OzoneConfigUtil.getOzoneAdminsGroupsFromConfig(configuration);
- LOG.info("OM start with adminUsers: {}", omAdminUsernames);
- omAdmins = new OzoneAdmins(omAdminUsernames, omAdminGroups);
+ omAdmins = OzoneAdmins.getOzoneAdmins(omStarterUser, conf);
+ LOG.info("OM start with adminUsers: {}", omAdmins.getAdminUsernames());
// Get read only admin list
Collection<String> omReadOnlyAdmins =
- OzoneConfigUtil.getOzoneReadOnlyAdminsFromConfig(
+ OzoneAdmins.getOzoneReadOnlyAdminsFromConfig(
configuration);
Collection<String> omReadOnlyAdminsGroups =
- OzoneConfigUtil.getOzoneReadOnlyAdminsGroupsFromConfig(
+ OzoneAdmins.getOzoneReadOnlyAdminsGroupsFromConfig(
configuration);
readOnlyAdmins = new OzoneAdmins(omReadOnlyAdmins,
Review Comment:
We could create another factory method for read-only admins, similar to
https://github.com/apache/ozone/blob/b186b306ff435f1fecd7078548562e3f2f31372d/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/server/OzoneAdmins.java#L74-L81
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
