make dynamicapichecker cache confgurable, fix test

Signed-off-by: Abhishek Kumar <[email protected]>

Author: shwstppr

api/src/main/java/org/apache/cloudstack/acl/RoleService.java

@@ -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();
 
     /**

plugins/acl/dynamic-role-based/src/main/java/org/apache/cloudstack/acl/DynamicRoleBasedAPIAccessChecker.java

@@ -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 @@ protected DynamicRoleBasedAPIAccessChecker() {
         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 @@ protected Pair<Role, List<RolePermission>> getRolePermissions(long roleId) {
         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 boolean checkAccess(User user, String commandName) throws PermissionDenie
     }
 
     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 void addApiToRoleBasedAnnotationsMap(final RoleType roleType, final Strin
     @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;
     }
 

server/src/main/java/org/apache/cloudstack/acl/RoleManagerImpl.java

@@ -486,7 +486,7 @@ public String getConfigComponentName() {
 
     @Override
     public ConfigKey<?>[] getConfigKeys() {
-        return new ConfigKey<?>[] {RoleService.EnableDynamicApiChecker};
+        return new ConfigKey<?>[] {RoleService.EnableDynamicApiChecker, RoleService.DynamicApiCheckerCachePeriod};
     }
 
     @Override

test/integration/smoke/test_dynamicroles.py

@@ -18,14 +18,15 @@
 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
 
 import copy
 import random
 import re
+import time
 
 
 class TestData(object):
@@ -109,6 +110,14 @@ def setUp(self):
             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 @@ def test_role_account_acls(self):
                 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 @@ def test_role_account_acls_multiple_mgmt_servers(self):
             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