make dynamicapichecker cache confgurable, fix test

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
This commit is contained in:
Abhishek Kumar 2024-10-23 13:02:33 +05:30
parent fd424d55f4
commit 3dccebce77
4 changed files with 43 additions and 11 deletions

View File

@ -30,6 +30,11 @@ public interface RoleService {
ConfigKey<Boolean> EnableDynamicApiChecker = new ConfigKey<>("Advanced", Boolean.class, "dynamic.apichecker.enabled", "false",
"If set to true, this enables the dynamic role-based api access checker and disables the default static role-based api access checker.", true);
ConfigKey<Integer> DynamicApiCheckerCachePeriod = new ConfigKey<>("Advanced", Integer.class,
"dynamic.apichecker.cache.period", "0",
"Defines the expiration time in seconds for the Dynamic API Checker cache, determining how long cached data is retained before being refreshed. If set to zero then caching will be disabled",
false);
boolean isEnabled();
/**

View File

@ -51,8 +51,9 @@ public class DynamicRoleBasedAPIAccessChecker extends AdapterBase implements API
private List<PluggableService> services;
private Map<RoleType, Set<String>> annotationRoleBasedApisMap = new HashMap<RoleType, Set<String>>();
final private LazyCache<Long, Account> accountCache;
final private LazyCache<Long, Pair<Role, List<RolePermission>>> rolePermissionsCache;
private LazyCache<Long, Account> accountCache;
private LazyCache<Long, Pair<Role, List<RolePermission>>> rolePermissionsCache;
private int cachePeriod;
private static final Logger LOGGER = Logger.getLogger(DynamicRoleBasedAPIAccessChecker.class.getName());
@ -61,10 +62,6 @@ public class DynamicRoleBasedAPIAccessChecker extends AdapterBase implements API
for (RoleType roleType : RoleType.values()) {
annotationRoleBasedApisMap.put(roleType, new HashSet<String>());
}
accountCache = new LazyCache<>(32, 60,
this::getAccountFromId);
rolePermissionsCache = new LazyCache<>(32, 60,
this::getRolePermissions);
}
@Override
@ -127,16 +124,30 @@ public class DynamicRoleBasedAPIAccessChecker extends AdapterBase implements API
return new Pair<>(accountRole, roleService.findAllPermissionsBy(accountRole.getId()));
}
protected Pair<Role, List<RolePermission>> getRolePermissionsUsingCache(long roleId) {
if (cachePeriod > 0) {
return rolePermissionsCache.get(roleId);
}
return getRolePermissions(roleId);
}
protected Account getAccountFromIdUsingCache(long accountId) {
if (cachePeriod > 0) {
return accountCache.get(accountId);
}
return getAccountFromId(accountId);
}
@Override
public boolean checkAccess(User user, String commandName) throws PermissionDeniedException {
if (!isEnabled()) {
return true;
}
Account account = accountCache.get(user.getAccountId());
Account account = getAccountFromIdUsingCache(user.getAccountId());
if (account == null) {
throw new PermissionDeniedException(String.format("Account for user id [%s] cannot be found", user.getUuid()));
}
Pair<Role, List<RolePermission>> roleAndPermissions = rolePermissionsCache.get(account.getRoleId());
Pair<Role, List<RolePermission>> roleAndPermissions = getRolePermissionsUsingCache(account.getRoleId());
final Role accountRole = roleAndPermissions.first();
if (accountRole == null) {
throw new PermissionDeniedException(String.format("Account role for user id [%s] cannot be found.", user.getUuid()));
@ -153,7 +164,7 @@ public class DynamicRoleBasedAPIAccessChecker extends AdapterBase implements API
}
public boolean checkAccess(Account account, String commandName) {
Pair<Role, List<RolePermission>> roleAndPermissions = rolePermissionsCache.get(account.getRoleId());
Pair<Role, List<RolePermission>> roleAndPermissions = getRolePermissionsUsingCache(account.getRoleId());
final Role accountRole = roleAndPermissions.first();
if (accountRole == null) {
throw new PermissionDeniedException(String.format("The account [%s] has role null or unknown.", account));
@ -198,6 +209,9 @@ public class DynamicRoleBasedAPIAccessChecker extends AdapterBase implements API
@Override
public boolean configure(String name, Map<String, Object> params) throws ConfigurationException {
super.configure(name, params);
cachePeriod = Math.max(0, RoleService.DynamicApiCheckerCachePeriod.value());
accountCache = new LazyCache<>(32, cachePeriod, this::getAccountFromId);
rolePermissionsCache = new LazyCache<>(32, cachePeriod, this::getRolePermissions);
return true;
}

View File

@ -486,7 +486,7 @@ public class RoleManagerImpl extends ManagerBase implements RoleService, Configu
@Override
public ConfigKey<?>[] getConfigKeys() {
return new ConfigKey<?>[] {RoleService.EnableDynamicApiChecker};
return new ConfigKey<?>[] {RoleService.EnableDynamicApiChecker, RoleService.DynamicApiCheckerCachePeriod};
}
@Override

View File

@ -18,7 +18,7 @@
from marvin.cloudstackAPI import *
from marvin.cloudstackTestCase import cloudstackTestCase
from marvin.cloudstackException import CloudstackAPIException
from marvin.lib.base import Account, Role, RolePermission
from marvin.lib.base import Account, Role, RolePermission, Configurations
from marvin.lib.utils import cleanup_resources
from nose.plugins.attrib import attr
from random import shuffle
@ -26,6 +26,7 @@ from random import shuffle
import copy
import random
import re
import time
class TestData(object):
@ -109,6 +110,14 @@ class TestDynamicRoles(cloudstackTestCase):
self.testdata["account"],
roleid=self.role.id
)
cache_period_config = Configurations.list(
self.apiclient,
name='dynamic.apichecker.cache.period'
)[0]
self.cache_period = int(cache_period_config.value)
self.cleanup = [
self.account,
self.rolepermission,
@ -623,6 +632,8 @@ class TestDynamicRoles(cloudstackTestCase):
testdata
)
time.sleep(self.cache_period + 5)
userApiClient = self.getUserApiClient(self.account.name, domain=self.account.domain, role_type=self.account.roletype)
# Perform listApis check
@ -645,6 +656,8 @@ class TestDynamicRoles(cloudstackTestCase):
self.dbclient.execute("insert into role_permissions (uuid, role_id, rule, permission, sort_order) values (UUID(), %d, '%s', '%s', %d)" % (roleId, rule, perm.upper(), sortOrder))
sortOrder += 1
time.sleep(self.cache_period + 5)
userApiClient = self.getUserApiClient(self.account.name, domain=self.account.domain, role_type=self.account.roletype)
# Perform listApis check