From 310195cbe25ac49160713e511d75525a16c9364b Mon Sep 17 00:00:00 2001 From: Rohit Yadav Date: Tue, 22 Aug 2017 10:22:27 +0200 Subject: [PATCH] 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 --- .../cloud/upgrade/dao/Upgrade481to490.java | 45 +++++++++---------- .../cloudstack/acl/RoleManagerImpl.java | 43 +++++++++--------- .../java/com/cloud/utils/PropertiesUtil.java | 4 -- 3 files changed, 41 insertions(+), 51 deletions(-) diff --git a/engine/schema/src/com/cloud/upgrade/dao/Upgrade481to490.java b/engine/schema/src/com/cloud/upgrade/dao/Upgrade481to490.java index 1c6ce38bd71..29e6534105b 100644 --- a/engine/schema/src/com/cloud/upgrade/dao/Upgrade481to490.java +++ b/engine/schema/src/com/cloud/upgrade/dao/Upgrade481to490.java @@ -17,13 +17,6 @@ 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; @@ -31,7 +24,13 @@ 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 @@ public class Upgrade481to490 implements DbUpgrade { migrateAccountsToDefaultRoles(conn); - final Map 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); } } diff --git a/server/src/org/apache/cloudstack/acl/RoleManagerImpl.java b/server/src/org/apache/cloudstack/acl/RoleManagerImpl.java index 27cb3d0238a..053ec582ab7 100644 --- a/server/src/org/apache/cloudstack/acl/RoleManagerImpl.java +++ b/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.context.CallContext; 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 @@ public class RoleManagerImpl extends ManagerBase implements RoleService, Configu @Override public boolean isEnabled() { - File apiCmdFile = PropertiesUtil.findConfigFile(PropertiesUtil.getDefaultApiCommandsFileName()); - return RoleService.EnableDynamicApiChecker.value() && (apiCmdFile == null || !apiCmdFile.exists()); + return RoleService.EnableDynamicApiChecker.value(); } @Override diff --git a/utils/src/main/java/com/cloud/utils/PropertiesUtil.java b/utils/src/main/java/com/cloud/utils/PropertiesUtil.java index c0da87a192d..4cb89f7531d 100644 --- a/utils/src/main/java/com/cloud/utils/PropertiesUtil.java +++ b/utils/src/main/java/com/cloud/utils/PropertiesUtil.java @@ -34,10 +34,6 @@ import org.apache.log4j.Logger; 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.