CLOUDSTACK-10052: Simplify dynamic roles enable checking (#2241)

This fixes issue of enabling dynamic roles based on the global setting
only. This also fixes application of the default role/permissions mapping
on upgrade from 4.8 and previous versions to 4.9+.

Previously, it would make additional check to ensure commands.properties
is not in the classpath however this creates confusion for admins who
may skip/skim through the rn/docs and assume that mere changing the
global settings was not enough.

Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>

Author: yadvr

engine/schema/src/com/cloud/upgrade/dao/Upgrade481to490.java

@@ -17,21 +17,20 @@
 
 package com.cloud.upgrade.dao;
 
-import com.cloud.utils.PropertiesUtil;
-import com.cloud.utils.db.ScriptRunner;
-import com.cloud.utils.exception.CloudRuntimeException;
-import com.cloud.utils.script.Script;
-import org.apache.cloudstack.acl.RoleType;
-import org.apache.log4j.Logger;
-
 import java.io.File;
 import java.io.FileReader;
 import java.io.IOException;
 import java.sql.Connection;
 import java.sql.PreparedStatement;
 import java.sql.ResultSet;
 import java.sql.SQLException;
-import java.util.Map;
+
+import org.apache.cloudstack.acl.RoleType;
+import org.apache.log4j.Logger;
+
+import com.cloud.utils.db.ScriptRunner;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.utils.script.Script;
 
 public class Upgrade481to490 implements DbUpgrade {
     final static Logger s_logger = Logger.getLogger(Upgrade481to490.class);
@@ -115,23 +114,19 @@ private void setupRolesAndPermissionsForDynamicChecker(final Connection conn) {
 
         migrateAccountsToDefaultRoles(conn);
 
-        final Map<String, String> apiMap = PropertiesUtil.processConfigFile(new String[] { PropertiesUtil.getDefaultApiCommandsFileName() });
-        if (apiMap == null || apiMap.isEmpty()) {
-            if (s_logger.isDebugEnabled()) {
-                s_logger.debug("The commands.properties file and default role permissions were not found. " +
-                        "Assuming new installation, configuring default role-api mappings.");
-            }
-            String script = Script.findScript("", "db/create-default-role-api-mappings.sql");
-            if (script == null) {
-                s_logger.error("Unable to find default role-api mapping sql file, please configure api per role manually");
-                return;
-            }
-            try(final FileReader reader = new FileReader(new File(script))) {
-                ScriptRunner runner = new ScriptRunner(conn, false, true);
-                runner.runScript(reader);
-            } catch (SQLException | IOException e) {
-                s_logger.error("Unable to insert default api-role mappings from file: " + script + ". Please configure api per role manually, giving up!", e);
-            }
+        if (s_logger.isDebugEnabled()) {
+            s_logger.debug("Configuring default role-api mappings, use migrate-dynamicroles.py instead if you want to migrate rules from an existing commands.properties file");
+        }
+        String script = Script.findScript("", "db/create-default-role-api-mappings.sql");
+        if (script == null) {
+            s_logger.error("Unable to find default role-api mapping sql file, please configure api per role manually");
+            return;
+        }
+        try(final FileReader reader = new FileReader(new File(script))) {
+            ScriptRunner runner = new ScriptRunner(conn, false, true);
+            runner.runScript(reader);
+        } catch (SQLException | IOException e) {
+            s_logger.error("Unable to insert default api-role mappings from file: " + script + ". Please configure api per role manually, giving up!", e);
         }
     }
 

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

@@ -16,19 +16,14 @@
 // under the License.
 package org.apache.cloudstack.acl;
 
-import com.cloud.event.ActionEvent;
-import com.cloud.event.EventTypes;
-import com.cloud.exception.PermissionDeniedException;
-import com.cloud.user.Account;
-import com.cloud.user.dao.AccountDao;
-import com.cloud.utils.ListUtils;
-import com.cloud.utils.PropertiesUtil;
-import com.cloud.utils.component.ManagerBase;
-import com.cloud.utils.component.PluggableService;
-import com.cloud.utils.db.Transaction;
-import com.cloud.utils.db.TransactionCallback;
-import com.cloud.utils.db.TransactionStatus;
-import com.google.common.base.Strings;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Date;
+import java.util.List;
+
+import javax.ejb.Local;
+import javax.inject.Inject;
+
 import org.apache.cloudstack.acl.dao.RoleDao;
 import org.apache.cloudstack.acl.dao.RolePermissionsDao;
 import org.apache.cloudstack.api.ApiErrorCode;
@@ -45,13 +40,18 @@
 import org.apache.cloudstack.framework.config.ConfigKey;
 import org.apache.cloudstack.framework.config.Configurable;
 
-import javax.ejb.Local;
-import javax.inject.Inject;
-import java.io.File;
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.Date;
-import java.util.List;
+import com.cloud.event.ActionEvent;
+import com.cloud.event.EventTypes;
+import com.cloud.exception.PermissionDeniedException;
+import com.cloud.user.Account;
+import com.cloud.user.dao.AccountDao;
+import com.cloud.utils.ListUtils;
+import com.cloud.utils.component.ManagerBase;
+import com.cloud.utils.component.PluggableService;
+import com.cloud.utils.db.Transaction;
+import com.cloud.utils.db.TransactionCallback;
+import com.cloud.utils.db.TransactionStatus;
+import com.google.common.base.Strings;
 
 @Local(value = {RoleService.class})
 public class RoleManagerImpl extends ManagerBase implements RoleService, Configurable, PluggableService {
@@ -78,8 +78,7 @@ private void checkCallerAccess() {
 
     @Override
     public boolean isEnabled() {
-        File apiCmdFile = PropertiesUtil.findConfigFile(PropertiesUtil.getDefaultApiCommandsFileName());
-        return RoleService.EnableDynamicApiChecker.value() && (apiCmdFile == null || !apiCmdFile.exists());
+        return RoleService.EnableDynamicApiChecker.value();
     }
 
     @Override

utils/src/main/java/com/cloud/utils/PropertiesUtil.java

@@ -34,10 +34,6 @@
 public class PropertiesUtil {
     private static final Logger s_logger = Logger.getLogger(PropertiesUtil.class);
 
-    public static String getDefaultApiCommandsFileName() {
-        return "commands.properties";
-    }
-
     /**
      * Searches the class path and local paths to find the config file.
      * @param path path to find.  if it starts with / then it's absolute path.