smengcl commented on code in PR #3131:
URL: https://github.com/apache/ozone/pull/3131#discussion_r871750668
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OMMultiTenantManagerImpl.java:
##########
@@ -260,57 +321,75 @@ public void removeTenantAccessFromAuthorizer(Tenant
tenant) throws Exception {
*/
@Override
public String assignUserToTenant(BasicUserPrincipal principal,
- String tenantId,
- String accessId) throws IOException {
- ImmutablePair<String, String> userAccessIdPair =
- new ImmutablePair<>(principal.getName(), accessId);
+ String tenantId, String accessId) throws IOException {
+
+ final CachedAccessIdInfo cacheEntry =
+ new CachedAccessIdInfo(principal.getName(), false);
+
try {
- controlPathLock.writeLock().lock();
-
- LOG.info("Adding user '{}' to tenant '{}' in-memory state.",
- principal.getName(), tenantId);
- CachedTenantState cachedTenantState =
- tenantCache.getOrDefault(tenantId,
- new CachedTenantState(tenantId));
- cachedTenantState.getTenantUsers().add(userAccessIdPair);
-
- final OzoneTenantRolePrincipal roleTenantAllUsers =
- OzoneTenantRolePrincipal.getUserRole(tenantId);
- String roleJsonStr = authorizer.getRole(roleTenantAllUsers);
- String roleId = authorizer.assignUser(principal, roleJsonStr, false);
+ tenantCacheLock.writeLock().lock();
+
+ CachedTenantState cachedTenantState = tenantCache.get(tenantId);
+ Preconditions.checkNotNull(cachedTenantState,
+ "Cache entry for tenant '" + tenantId + "' does not exist");
+
+ LOG.info("Adding user '{}' access ID '{}' to tenant '{}' in-memory
cache",
+ principal.getName(), accessId, tenantId);
+ cachedTenantState.getAccessIdInfoMap().put(accessId, cacheEntry);
+
+ final String tenantUserRoleName =
+ tenantCache.get(tenantId).getTenantUserRoleName();
+ final OzoneTenantRolePrincipal tenantUserRolePrincipal =
+ new OzoneTenantRolePrincipal(tenantUserRoleName);
+ String roleJsonStr = authorizer.getRole(tenantUserRolePrincipal);
+ final String roleId =
+ authorizer.assignUserToRole(principal, roleJsonStr, false);
return roleId;
- } catch (Exception e) {
+ } catch (IOException e) {
+ // Clean up
revokeUserAccessId(accessId);
- tenantCache.get(tenantId).getTenantUsers().remove(userAccessIdPair);
+ tenantCache.get(tenantId).getAccessIdInfoMap().remove(accessId);
+
throw new OMException(e.getMessage(), TENANT_AUTHORIZER_ERROR);
} finally {
- controlPathLock.writeLock().unlock();
+ tenantCacheLock.writeLock().unlock();
}
}
@Override
- public void revokeUserAccessId(String accessID) throws IOException {
+ public void revokeUserAccessId(String accessId) throws IOException {
Review Comment:
Good catch. Same as the comment above.
--
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]
