diff --git a/.github/workflows/merge-conflict-checker.yml b/.github/workflows/merge-conflict-checker.yml index 860e1c1b561..a997cb94ccc 100644 --- a/.github/workflows/merge-conflict-checker.yml +++ b/.github/workflows/merge-conflict-checker.yml @@ -18,8 +18,8 @@ name: "PR Merge Conflict Check" on: push: - pull_request_target: - types: [synchronize] + pull_request: + types: [opened, synchronize, reopened] permissions: # added using https://github.com/step-security/secure-workflows contents: read diff --git a/api/src/main/java/com/cloud/host/Host.java b/api/src/main/java/com/cloud/host/Host.java index a3b6ccadc01..4672b302776 100644 --- a/api/src/main/java/com/cloud/host/Host.java +++ b/api/src/main/java/com/cloud/host/Host.java @@ -59,6 +59,9 @@ public interface Host extends StateObject, Identity, Partition, HAResour String HOST_INSTANCE_CONVERSION = "host.instance.conversion"; String HOST_OVFTOOL_VERSION = "host.ovftool.version"; String HOST_VIRTV2V_VERSION = "host.virtv2v.version"; + String HOST_SSH_PORT = "host.ssh.port"; + + int DEFAULT_SSH_PORT = 22; /** * @return name of the machine. diff --git a/api/src/main/java/org/apache/cloudstack/api/ApiServerService.java b/api/src/main/java/org/apache/cloudstack/api/ApiServerService.java index cbbcdc3bda4..a422129ad21 100644 --- a/api/src/main/java/org/apache/cloudstack/api/ApiServerService.java +++ b/api/src/main/java/org/apache/cloudstack/api/ApiServerService.java @@ -48,4 +48,6 @@ public interface ApiServerService { boolean forgotPassword(UserAccount userAccount, Domain domain); boolean resetPassword(UserAccount userAccount, String token, String password); + + String getDomainId(Map params); } diff --git a/api/src/main/java/org/apache/cloudstack/api/BaseUpdateTemplateOrIsoCmd.java b/api/src/main/java/org/apache/cloudstack/api/BaseUpdateTemplateOrIsoCmd.java index 696a500860e..54a398d756f 100644 --- a/api/src/main/java/org/apache/cloudstack/api/BaseUpdateTemplateOrIsoCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/BaseUpdateTemplateOrIsoCmd.java @@ -145,8 +145,8 @@ public abstract class BaseUpdateTemplateOrIsoCmd extends BaseCmd { return (Map) (paramsCollection.toArray())[0]; } - public boolean isCleanupDetails(){ - return cleanupDetails == null ? false : cleanupDetails.booleanValue(); + public boolean isCleanupDetails() { + return cleanupDetails != null && cleanupDetails; } public CPU.CPUArch getCPUArch() { diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/host/AddHostCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/host/AddHostCmd.java index 6c8eded2618..d91828c89db 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/host/AddHostCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/host/AddHostCmd.java @@ -60,7 +60,8 @@ public class AddHostCmd extends BaseCmd { @Parameter(name = ApiConstants.POD_ID, type = CommandType.UUID, entityType = PodResponse.class, required = true, description = "The Pod ID for the host") private Long podId; - @Parameter(name = ApiConstants.URL, type = CommandType.STRING, required = true, description = "The host URL") + @Parameter(name = ApiConstants.URL, type = CommandType.STRING, required = true, description = "The host URL, optionally add ssh port (format: 'host:port') for KVM hosts," + + " otherwise falls back to the port defined at the config 'kvm.host.discovery.ssh.port'") private String url; @Parameter(name = ApiConstants.ZONE_ID, type = CommandType.UUID, entityType = ZoneResponse.class, required = true, description = "The Zone ID for the host") diff --git a/api/src/main/java/org/apache/cloudstack/backup/BackupManager.java b/api/src/main/java/org/apache/cloudstack/backup/BackupManager.java index 78d189c3bf1..0090b4f6b16 100644 --- a/api/src/main/java/org/apache/cloudstack/backup/BackupManager.java +++ b/api/src/main/java/org/apache/cloudstack/backup/BackupManager.java @@ -26,6 +26,7 @@ import org.apache.cloudstack.api.command.user.backup.DeleteBackupScheduleCmd; import org.apache.cloudstack.api.command.user.backup.ListBackupOfferingsCmd; import org.apache.cloudstack.api.command.user.backup.ListBackupsCmd; import org.apache.cloudstack.framework.config.ConfigKey; +import org.apache.cloudstack.framework.config.ValidatedConfigKey; import org.apache.cloudstack.framework.config.Configurable; import com.cloud.utils.Pair; @@ -42,10 +43,11 @@ public interface BackupManager extends BackupService, Configurable, PluggableSer "false", "Is backup and recovery framework enabled.", false, ConfigKey.Scope.Zone); - ConfigKey BackupProviderPlugin = new ConfigKey<>("Advanced", String.class, + ConfigKey BackupProviderPlugin = new ValidatedConfigKey<>("Advanced", String.class, "backup.framework.provider.plugin", "dummy", - "The backup and recovery provider plugin.", true, ConfigKey.Scope.Zone, BackupFrameworkEnabled.key()); + "The backup and recovery provider plugin. Valid plugin values: dummy, veeam, networker and nas", + true, ConfigKey.Scope.Zone, BackupFrameworkEnabled.key(), value -> validateBackupProviderConfig((String)value)); ConfigKey BackupSyncPollingInterval = new ConfigKey<>("Advanced", Long.class, "backup.framework.sync.interval", @@ -148,4 +150,14 @@ public interface BackupManager extends BackupService, Configurable, PluggableSer boolean deleteBackup(final Long backupId, final Boolean forced); BackupOffering updateBackupOffering(UpdateBackupOfferingCmd updateBackupOfferingCmd); + + static void validateBackupProviderConfig(String value) { + if (value != null && (value.contains(",") || value.trim().contains(" "))) { + throw new IllegalArgumentException("Multiple backup provider plugins are not supported. Please provide a single plugin value."); + } + List validPlugins = List.of("dummy", "veeam", "networker", "nas"); + if (value != null && !validPlugins.contains(value)) { + throw new IllegalArgumentException("Invalid backup provider plugin: " + value + ". Valid plugin values are: " + String.join(", ", validPlugins)); + } + } } diff --git a/core/src/main/java/com/cloud/agent/api/ModifyStoragePoolAnswer.java b/core/src/main/java/com/cloud/agent/api/ModifyStoragePoolAnswer.java index 552ffb85aaf..9f7b1059512 100644 --- a/core/src/main/java/com/cloud/agent/api/ModifyStoragePoolAnswer.java +++ b/core/src/main/java/com/cloud/agent/api/ModifyStoragePoolAnswer.java @@ -46,6 +46,10 @@ public class ModifyStoragePoolAnswer extends Answer { templateInfo = tInfo; } + public ModifyStoragePoolAnswer(final Command command, final boolean success, final String details) { + super(command, success, details); + } + public void setPoolInfo(StoragePoolInfo poolInfo) { this.poolInfo = poolInfo; } diff --git a/core/src/main/java/org/apache/cloudstack/backup/RestoreBackupCommand.java b/core/src/main/java/org/apache/cloudstack/backup/RestoreBackupCommand.java index 7228e35147a..0bc6865d9e5 100644 --- a/core/src/main/java/org/apache/cloudstack/backup/RestoreBackupCommand.java +++ b/core/src/main/java/org/apache/cloudstack/backup/RestoreBackupCommand.java @@ -31,9 +31,9 @@ public class RestoreBackupCommand extends Command { private String backupRepoType; private String backupRepoAddress; private List volumePaths; + private List backupFiles; private String diskType; private Boolean vmExists; - private String restoreVolumeUUID; private VirtualMachine.State vmState; protected RestoreBackupCommand() { @@ -80,6 +80,14 @@ public class RestoreBackupCommand extends Command { this.volumePaths = volumePaths; } + public List getBackupFiles() { + return backupFiles; + } + + public void setBackupFiles(List backupFiles) { + this.backupFiles = backupFiles; + } + public Boolean isVmExists() { return vmExists; } @@ -104,14 +112,6 @@ public class RestoreBackupCommand extends Command { this.mountOptions = mountOptions; } - public String getRestoreVolumeUUID() { - return restoreVolumeUUID; - } - - public void setRestoreVolumeUUID(String restoreVolumeUUID) { - this.restoreVolumeUUID = restoreVolumeUUID; - } - public VirtualMachine.State getVmState() { return vmState; } diff --git a/engine/components-api/src/main/java/com/cloud/agent/AgentManager.java b/engine/components-api/src/main/java/com/cloud/agent/AgentManager.java index b29eb38395f..f70ab494fdc 100644 --- a/engine/components-api/src/main/java/com/cloud/agent/AgentManager.java +++ b/engine/components-api/src/main/java/com/cloud/agent/AgentManager.java @@ -54,6 +54,10 @@ public interface AgentManager { "This timeout overrides the wait global config. This holds a comma separated key value pairs containing timeout (in seconds) for specific commands. " + "For example: DhcpEntryCommand=600, SavePasswordCommand=300, VmDataCommand=300", false); + ConfigKey KVMHostDiscoverySshPort = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, Integer.class, + "kvm.host.discovery.ssh.port", String.valueOf(Host.DEFAULT_SSH_PORT), "SSH port used for KVM host discovery and any other operations on host (using SSH)." + + " Please note that this is applicable when port is not defined through host url while adding the KVM host.", true, ConfigKey.Scope.Cluster); + enum TapAgentsAction { Add, Del, Contains, } @@ -170,4 +174,6 @@ public interface AgentManager { void notifyMonitorsOfRemovedHost(long hostId, long clusterId); void propagateChangeToAgents(Map params); + + int getHostSshPort(HostVO host); } diff --git a/engine/components-api/src/main/java/com/cloud/resource/ResourceManager.java b/engine/components-api/src/main/java/com/cloud/resource/ResourceManager.java index 936e8b3448e..059578ab2ca 100755 --- a/engine/components-api/src/main/java/com/cloud/resource/ResourceManager.java +++ b/engine/components-api/src/main/java/com/cloud/resource/ResourceManager.java @@ -154,6 +154,8 @@ public interface ResourceManager extends ResourceService, Configurable { public HostVO findHostByGuid(String guid); + HostVO findHostByGuidPrefix(String guid); + public HostVO findHostByName(String name); HostStats getHostStatistics(Host host); diff --git a/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java b/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java index 2b5e81eb3f9..ebe0465e3f0 100644 --- a/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java @@ -40,6 +40,7 @@ import java.util.stream.Collectors; import javax.inject.Inject; import javax.naming.ConfigurationException; +import com.cloud.utils.StringUtils; import org.apache.cloudstack.agent.lb.IndirectAgentLB; import org.apache.cloudstack.ca.CAManager; import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService; @@ -55,7 +56,6 @@ import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToSt import org.apache.commons.collections.MapUtils; import org.apache.commons.lang3.BooleanUtils; import org.apache.commons.lang3.ObjectUtils; -import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.ThreadContext; import com.cloud.agent.AgentManager; @@ -1977,7 +1977,7 @@ public class AgentManagerImpl extends ManagerBase implements AgentManager, Handl return new ConfigKey[] { CheckTxnBeforeSending, Workers, Port, Wait, AlertWait, DirectAgentLoadSize, DirectAgentPoolSize, DirectAgentThreadCap, EnableKVMAutoEnableDisable, ReadyCommandWait, GranularWaitTimeForCommands, RemoteAgentSslHandshakeTimeout, RemoteAgentMaxConcurrentNewConnections, - RemoteAgentNewConnectionsMonitorInterval }; + RemoteAgentNewConnectionsMonitorInterval, KVMHostDiscoverySshPort }; } protected class SetHostParamsListener implements Listener { @@ -2093,6 +2093,25 @@ public class AgentManagerImpl extends ManagerBase implements AgentManager, Handl } } + @Override + public int getHostSshPort(HostVO host) { + if (host == null) { + return KVMHostDiscoverySshPort.value(); + } + + if (host.getHypervisorType() != HypervisorType.KVM) { + return Host.DEFAULT_SSH_PORT; + } + + _hostDao.loadDetails(host); + String hostPort = host.getDetail(Host.HOST_SSH_PORT); + if (StringUtils.isBlank(hostPort)) { + return KVMHostDiscoverySshPort.valueIn(host.getClusterId()); + } + + return Integer.parseInt(hostPort); + } + private GlobalLock getHostJoinLock(Long hostId) { return GlobalLock.getInternLock(String.format("%s-%s", "Host-Join", hostId)); } diff --git a/engine/orchestration/src/test/java/com/cloud/agent/manager/AgentManagerImplTest.java b/engine/orchestration/src/test/java/com/cloud/agent/manager/AgentManagerImplTest.java index 52b7ed77533..293a988dd2c 100644 --- a/engine/orchestration/src/test/java/com/cloud/agent/manager/AgentManagerImplTest.java +++ b/engine/orchestration/src/test/java/com/cloud/agent/manager/AgentManagerImplTest.java @@ -22,9 +22,11 @@ import com.cloud.agent.api.ReadyCommand; import com.cloud.agent.api.StartupCommand; import com.cloud.agent.api.StartupRoutingCommand; import com.cloud.exception.ConnectionException; +import com.cloud.host.Host; import com.cloud.host.HostVO; import com.cloud.host.Status; import com.cloud.host.dao.HostDao; +import com.cloud.hypervisor.Hypervisor; import com.cloud.utils.Pair; import org.junit.Assert; import org.junit.Before; @@ -103,4 +105,36 @@ public class AgentManagerImplTest { Assert.assertEquals(50, result); } + + @Test + public void testGetHostSshPortWithHostNull() { + int hostSshPort = mgr.getHostSshPort(null); + Assert.assertEquals(22, hostSshPort); + } + + @Test + public void testGetHostSshPortWithNonKVMHost() { + HostVO host = Mockito.mock(HostVO.class); + Mockito.when(host.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.XenServer); + int hostSshPort = mgr.getHostSshPort(host); + Assert.assertEquals(22, hostSshPort); + } + + @Test + public void testGetHostSshPortWithKVMHostDefaultPort() { + HostVO host = Mockito.mock(HostVO.class); + Mockito.when(host.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.KVM); + Mockito.when(host.getClusterId()).thenReturn(1L); + int hostSshPort = mgr.getHostSshPort(host); + Assert.assertEquals(22, hostSshPort); + } + + @Test + public void testGetHostSshPortWithKVMHostCustomPort() { + HostVO host = Mockito.mock(HostVO.class); + Mockito.when(host.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.KVM); + Mockito.when(host.getDetail(Host.HOST_SSH_PORT)).thenReturn(String.valueOf(3922)); + int hostSshPort = mgr.getHostSshPort(host); + Assert.assertEquals(3922, hostSshPort); + } } diff --git a/engine/schema/src/main/java/com/cloud/dc/ClusterDetailsDaoImpl.java b/engine/schema/src/main/java/com/cloud/dc/ClusterDetailsDaoImpl.java index 7650b40dd2f..f7d5c23c32c 100644 --- a/engine/schema/src/main/java/com/cloud/dc/ClusterDetailsDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/dc/ClusterDetailsDaoImpl.java @@ -163,7 +163,7 @@ public class ClusterDetailsDaoImpl extends ResourceDetailsDaoBase implements HostDao SearchCriteria sc = TypeClusterStatusSearch.create(); sc.setParameters("type", Host.Type.Routing); sc.setParameters("cluster", clusterId); - List list = listBy(sc, new Filter(1)); + List list = listBy(sc, new Filter(1, true)); return list.isEmpty() ? null : list.get(0); } diff --git a/engine/schema/src/main/java/com/cloud/storage/dao/VMTemplateDaoImpl.java b/engine/schema/src/main/java/com/cloud/storage/dao/VMTemplateDaoImpl.java index 98859789f82..45dffea7621 100644 --- a/engine/schema/src/main/java/com/cloud/storage/dao/VMTemplateDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/storage/dao/VMTemplateDaoImpl.java @@ -459,7 +459,7 @@ public class VMTemplateDaoImpl extends GenericDaoBase implem if (detailsStr == null) { return; } - List details = new ArrayList(); + List details = new ArrayList<>(); for (String key : detailsStr.keySet()) { VMTemplateDetailVO detail = new VMTemplateDetailVO(tmpl.getId(), key, detailsStr.get(key), true); details.add(detail); @@ -481,7 +481,7 @@ public class VMTemplateDaoImpl extends GenericDaoBase implem } if (tmplt.getDetails() != null) { - List details = new ArrayList(); + List details = new ArrayList<>(); for (String key : tmplt.getDetails().keySet()) { details.add(new VMTemplateDetailVO(tmplt.getId(), key, tmplt.getDetails().get(key), true)); } diff --git a/engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java b/engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java index a72b4a25845..d480ce6a0b8 100644 --- a/engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java @@ -382,7 +382,7 @@ public class VolumeDaoImpl extends GenericDaoBase implements Vol public VolumeDaoImpl() { AllFieldsSearch = createSearchBuilder(); - AllFieldsSearch.and("state", AllFieldsSearch.entity().getState(), Op.EQ); + AllFieldsSearch.and("state", AllFieldsSearch.entity().getState(), Op.IN); AllFieldsSearch.and("accountId", AllFieldsSearch.entity().getAccountId(), Op.EQ); AllFieldsSearch.and("dcId", AllFieldsSearch.entity().getDataCenterId(), Op.EQ); AllFieldsSearch.and("pod", AllFieldsSearch.entity().getPodId(), Op.EQ); @@ -579,17 +579,16 @@ public class VolumeDaoImpl extends GenericDaoBase implements Vol @Override public List listVolumesToBeDestroyed() { - SearchCriteria sc = AllFieldsSearch.create(); - sc.setParameters("state", Volume.State.Destroy); - - return listBy(sc); + return listVolumesToBeDestroyed(null); } @Override public List listVolumesToBeDestroyed(Date date) { SearchCriteria sc = AllFieldsSearch.create(); - sc.setParameters("state", Volume.State.Destroy); - sc.setParameters("updateTime", date); + sc.setParameters("state", Volume.State.Destroy, Volume.State.Expunging); + if (date != null) { + sc.setParameters("updateTime", date); + } return listBy(sc); } diff --git a/engine/schema/src/main/java/com/cloud/user/UserVO.java b/engine/schema/src/main/java/com/cloud/user/UserVO.java index 6e355e102e6..d74aa7ed41b 100644 --- a/engine/schema/src/main/java/com/cloud/user/UserVO.java +++ b/engine/schema/src/main/java/com/cloud/user/UserVO.java @@ -123,8 +123,8 @@ public class UserVO implements User, Identity, InternalIdentity { } public UserVO(long id) { + this(); this.id = id; - this.uuid = UUID.randomUUID().toString(); } public UserVO(long accountId, String username, String password, String firstName, String lastName, String email, String timezone, String uuid, Source source) { diff --git a/engine/schema/src/main/java/com/cloud/user/dao/AccountDao.java b/engine/schema/src/main/java/com/cloud/user/dao/AccountDao.java index dae5f3a3467..67b70571cb4 100644 --- a/engine/schema/src/main/java/com/cloud/user/dao/AccountDao.java +++ b/engine/schema/src/main/java/com/cloud/user/dao/AccountDao.java @@ -21,13 +21,11 @@ import java.util.List; import com.cloud.user.Account; import com.cloud.user.AccountVO; -import com.cloud.user.User; import com.cloud.utils.Pair; import com.cloud.utils.db.Filter; import com.cloud.utils.db.GenericDao; public interface AccountDao extends GenericDao { - Pair findUserAccountByApiKey(String apiKey); List findAccountsLike(String accountName); diff --git a/engine/schema/src/main/java/com/cloud/user/dao/AccountDaoImpl.java b/engine/schema/src/main/java/com/cloud/user/dao/AccountDaoImpl.java index f5f95d5da1f..48b29fac45e 100644 --- a/engine/schema/src/main/java/com/cloud/user/dao/AccountDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/user/dao/AccountDaoImpl.java @@ -16,8 +16,6 @@ // under the License. package com.cloud.user.dao; -import java.sql.PreparedStatement; -import java.sql.ResultSet; import java.util.Date; import java.util.List; @@ -27,10 +25,7 @@ import org.springframework.stereotype.Component; import com.cloud.user.Account; import com.cloud.user.Account.State; import com.cloud.user.AccountVO; -import com.cloud.user.User; -import com.cloud.user.UserVO; import com.cloud.utils.Pair; -import com.cloud.utils.crypt.DBEncryptionUtil; import com.cloud.utils.db.Filter; import com.cloud.utils.db.GenericDaoBase; import com.cloud.utils.db.GenericSearchBuilder; @@ -38,13 +33,9 @@ import com.cloud.utils.db.SearchBuilder; import com.cloud.utils.db.SearchCriteria; import com.cloud.utils.db.SearchCriteria.Func; import com.cloud.utils.db.SearchCriteria.Op; -import com.cloud.utils.db.TransactionLegacy; @Component public class AccountDaoImpl extends GenericDaoBase implements AccountDao { - private static final String FIND_USER_ACCOUNT_BY_API_KEY = "SELECT u.id, u.username, u.account_id, u.secret_key, u.state, u.api_key_access, " - + "a.id, a.account_name, a.type, a.role_id, a.domain_id, a.state, a.api_key_access " + "FROM `cloud`.`user` u, `cloud`.`account` a " - + "WHERE u.account_id = a.id AND u.api_key = ? and u.removed IS NULL"; protected final SearchBuilder AllFieldsSearch; protected final SearchBuilder AccountTypeSearch; @@ -132,51 +123,6 @@ public class AccountDaoImpl extends GenericDaoBase implements A return listBy(sc); } - @Override - public Pair findUserAccountByApiKey(String apiKey) { - TransactionLegacy txn = TransactionLegacy.currentTxn(); - PreparedStatement pstmt = null; - Pair userAcctPair = null; - try { - String sql = FIND_USER_ACCOUNT_BY_API_KEY; - pstmt = txn.prepareAutoCloseStatement(sql); - pstmt.setString(1, apiKey); - ResultSet rs = pstmt.executeQuery(); - // TODO: make sure we don't have more than 1 result? ApiKey had better be unique - if (rs.next()) { - User u = new UserVO(rs.getLong(1)); - u.setUsername(rs.getString(2)); - u.setAccountId(rs.getLong(3)); - u.setSecretKey(DBEncryptionUtil.decrypt(rs.getString(4))); - u.setState(State.getValueOf(rs.getString(5))); - boolean apiKeyAccess = rs.getBoolean(6); - if (rs.wasNull()) { - u.setApiKeyAccess(null); - } else { - u.setApiKeyAccess(apiKeyAccess); - } - - AccountVO a = new AccountVO(rs.getLong(7)); - a.setAccountName(rs.getString(8)); - a.setType(Account.Type.getFromValue(rs.getInt(9))); - a.setRoleId(rs.getLong(10)); - a.setDomainId(rs.getLong(11)); - a.setState(State.getValueOf(rs.getString(12))); - apiKeyAccess = rs.getBoolean(13); - if (rs.wasNull()) { - a.setApiKeyAccess(null); - } else { - a.setApiKeyAccess(apiKeyAccess); - } - - userAcctPair = new Pair(u, a); - } - } catch (Exception e) { - logger.warn("Exception finding user/acct by api key: " + apiKey, e); - } - return userAcctPair; - } - @Override public List findAccountsLike(String accountName) { return findAccountsLike(accountName, null).first(); @@ -341,11 +287,9 @@ public class AccountDaoImpl extends GenericDaoBase implements A domain_id = account_vo.getDomainId(); } catch (Exception e) { - logger.warn("getDomainIdForGivenAccountId: Exception :" + e.getMessage()); - } - finally { - return domain_id; + logger.warn("Can not get DomainId for the given AccountId; exception message : {}", e.getMessage()); } + return domain_id; } @Override diff --git a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/ImageStoreDaoImpl.java b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/ImageStoreDaoImpl.java index 4cb40b5eaf6..5f32a350232 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/ImageStoreDaoImpl.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/ImageStoreDaoImpl.java @@ -202,7 +202,7 @@ public class ImageStoreDaoImpl extends GenericDaoBase implem sc.setParameters("dataCenterId", dataCenterId); sc.setParameters("protocol", protocol); sc.setParameters("role", DataStoreRole.Image); - Filter filter = new Filter(1); + Filter filter = new Filter(1, true); List results = listBy(sc, filter); return results.size() == 0 ? null : results.get(0); } diff --git a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java index 2c70e48706d..3a7c9491273 100644 --- a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java +++ b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java @@ -434,6 +434,9 @@ public class VolumeServiceImpl implements VolumeService { // no need to change state in volumes table volume.processEventOnly(Event.DestroyRequested); } else if (volume.getDataStore().getRole() == DataStoreRole.Primary) { + if (vol.getState() == Volume.State.Expunging) { + logger.info("Volume {} is already in Expunging, retrying", volume); + } volume.processEvent(Event.ExpungeRequested); } diff --git a/framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/ResourceTest.java b/framework/config/src/main/java/org/apache/cloudstack/framework/config/ValidatedConfigKey.java similarity index 53% rename from framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/ResourceTest.java rename to framework/config/src/main/java/org/apache/cloudstack/framework/config/ValidatedConfigKey.java index cdcfc87cd4e..4fcbe4151d3 100644 --- a/framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/ResourceTest.java +++ b/framework/config/src/main/java/org/apache/cloudstack/framework/config/ValidatedConfigKey.java @@ -14,27 +14,25 @@ // KIND, either express or implied. See the License for the // specific language governing permissions and limitations // under the License. +package org.apache.cloudstack.framework.config; -package org.apache.cloudstack.quota.activationrule.presetvariables; +import java.util.function.Consumer; -import org.apache.commons.lang3.builder.ToStringBuilder; -import org.apache.commons.lang3.builder.ToStringStyle; -import org.junit.Assert; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.junit.MockitoJUnitRunner; +public class ValidatedConfigKey extends ConfigKey { + private final Consumer validator; -@RunWith(MockitoJUnitRunner.class) -public class ResourceTest { - - @Test - public void toStringTestReturnAJson() { - Resource variable = new Resource(); - - String expected = ToStringBuilder.reflectionToString(variable, ToStringStyle.JSON_STYLE); - String result = variable.toString(); - - Assert.assertEquals(expected, result); + public ValidatedConfigKey(String category, Class type, String name, String defaultValue, String description, boolean dynamic, Scope scope, String parent, Consumer validator) { + super(category, type, name, defaultValue, description, dynamic, scope, parent); + this.validator = validator; } + public Consumer getValidator() { + return validator; + } + + public void validateValue(String value) { + if (validator != null) { + validator.accept((T) value); + } + } } diff --git a/framework/db/src/main/java/com/cloud/utils/db/Filter.java b/framework/db/src/main/java/com/cloud/utils/db/Filter.java index 90e42952a99..375e508c55f 100644 --- a/framework/db/src/main/java/com/cloud/utils/db/Filter.java +++ b/framework/db/src/main/java/com/cloud/utils/db/Filter.java @@ -57,7 +57,18 @@ public class Filter { } public Filter(long limit) { - _orderBy = " ORDER BY RAND()"; + this(limit, false); + } + + /** + * Constructor for creating a filter with random ordering + * @param limit the maximum number of results to return + * @param randomize if true, orders results randomly + */ + public Filter(long limit, boolean randomize) { + if (randomize) { + _orderBy = " ORDER BY RAND()" ; + } _limit = limit; } diff --git a/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java b/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java index fedc7d7cd7a..e6a4bb8f9b9 100644 --- a/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java +++ b/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java @@ -347,7 +347,7 @@ public abstract class GenericDaoBase extends Compone @Override @DB() public T lockOneRandomRow(final SearchCriteria sc, final boolean exclusive) { - final Filter filter = new Filter(1); + final Filter filter = new Filter(1, true); final List beans = search(sc, filter, exclusive, true); return beans.isEmpty() ? null : beans.get(0); } @@ -927,7 +927,7 @@ public abstract class GenericDaoBase extends Compone @DB() protected T findOneIncludingRemovedBy(final SearchCriteria sc) { - Filter filter = new Filter(1); + Filter filter = new Filter(1, true); List results = searchIncludingRemoved(sc, filter, null, false); assert results.size() <= 1 : "Didn't the limiting worked?"; return results.size() == 0 ? null : results.get(0); @@ -1324,7 +1324,7 @@ public abstract class GenericDaoBase extends Compone Filter filter = null; final long batchSizeFinal = ObjectUtils.defaultIfNull(batchSize, 0L); if (batchSizeFinal > 0) { - filter = new Filter(null, batchSizeFinal); + filter = new Filter(batchSizeFinal); } int expunged = 0; int currentExpunged = 0; diff --git a/framework/db/src/test/java/com/cloud/utils/db/FilterTest.java b/framework/db/src/test/java/com/cloud/utils/db/FilterTest.java index 079611ab69f..9f040e7a5e3 100644 --- a/framework/db/src/test/java/com/cloud/utils/db/FilterTest.java +++ b/framework/db/src/test/java/com/cloud/utils/db/FilterTest.java @@ -41,4 +41,62 @@ public class FilterTest { Assert.assertTrue(filter.getOrderBy().split(",").length == 3); Assert.assertTrue(filter.getOrderBy().split(",")[2].trim().toLowerCase().equals("test.fld_int asc")); } + + + @Test + public void testFilterWithLimitOnly() { + Filter filter = new Filter(5); + + Assert.assertEquals(Long.valueOf(5), filter.getLimit()); + Assert.assertNull(filter.getOrderBy()); + Assert.assertNull(filter.getOffset()); + } + + @Test + public void testFilterWithLimitAndRandomizeFalse() { + Filter filter = new Filter(10, false); + + Assert.assertEquals(Long.valueOf(10), filter.getLimit()); + Assert.assertNull(filter.getOrderBy()); + Assert.assertNull(filter.getOffset()); + } + + @Test + public void testFilterWithLimitAndRandomizeTrue() { + Filter filter = new Filter(3, true); + + Assert.assertNull(filter.getLimit()); + Assert.assertNotNull(filter.getOrderBy()); + Assert.assertTrue(filter.getOrderBy().contains("ORDER BY RAND()")); + Assert.assertTrue(filter.getOrderBy().contains("LIMIT 3")); + Assert.assertEquals(" ORDER BY RAND() LIMIT 3", filter.getOrderBy()); + } + + @Test + public void testFilterRandomizeWithDifferentLimits() { + Filter filter1 = new Filter(1, true); + Filter filter10 = new Filter(10, true); + Filter filter100 = new Filter(100, true); + + Assert.assertEquals(" ORDER BY RAND() LIMIT 1", filter1.getOrderBy()); + Assert.assertEquals(" ORDER BY RAND() LIMIT 10", filter10.getOrderBy()); + Assert.assertEquals(" ORDER BY RAND() LIMIT 100", filter100.getOrderBy()); + } + + @Test + public void testFilterConstructorBackwardsCompatibility() { + // Test that Filter(long) behaves differently now (no ORDER BY RAND()) + // compared to Filter(long, true) which preserves old behavior + Filter simpleLimitFilter = new Filter(1); + Filter randomFilter = new Filter(1, true); + + // Simple limit filter should just set limit + Assert.assertEquals(Long.valueOf(1), simpleLimitFilter.getLimit()); + Assert.assertNull(simpleLimitFilter.getOrderBy()); + + // Random filter should set orderBy with RAND() + Assert.assertNull(randomFilter.getLimit()); + Assert.assertNotNull(randomFilter.getOrderBy()); + Assert.assertTrue(randomFilter.getOrderBy().contains("RAND()")); + } } diff --git a/framework/db/src/test/java/com/cloud/utils/db/GenericDaoBaseTest.java b/framework/db/src/test/java/com/cloud/utils/db/GenericDaoBaseTest.java index 308600341c3..ebf514f532f 100644 --- a/framework/db/src/test/java/com/cloud/utils/db/GenericDaoBaseTest.java +++ b/framework/db/src/test/java/com/cloud/utils/db/GenericDaoBaseTest.java @@ -20,6 +20,7 @@ import java.sql.ResultSet; import java.sql.SQLException; import java.util.ArrayList; import java.util.Collection; +import java.util.List; import org.junit.Assert; import org.junit.Before; @@ -263,4 +264,71 @@ public class GenericDaoBaseTest { " INNER JOIN tableA tableA2Alias ON tableC.column3=tableA2Alias.column2 " + " INNER JOIN tableA tableA3Alias ON tableD.column4=tableA3Alias.column3 AND tableD.column5=? ", joinString.toString()); } + + + @Test + public void testLockOneRandomRowUsesRandomFilter() { + // Create a mock DAO to test lockOneRandomRow behavior + GenericDaoBase testDao = Mockito.mock(GenericDaoBase.class); + + // Capture the filter passed to the search method + final Filter[] capturedFilter = new Filter[1]; + + Mockito.when(testDao.lockOneRandomRow(Mockito.any(SearchCriteria.class), Mockito.anyBoolean())) + .thenCallRealMethod(); + + Mockito.when(testDao.search(Mockito.any(SearchCriteria.class), Mockito.any(Filter.class), + Mockito.anyBoolean(), Mockito.anyBoolean())) + .thenAnswer(invocation -> { + capturedFilter[0] = invocation.getArgument(1); + return new ArrayList(); + }); + + SearchCriteria sc = Mockito.mock(SearchCriteria.class); + testDao.lockOneRandomRow(sc, true); + + // Verify that the filter uses random ordering + Assert.assertNotNull(capturedFilter[0]); + Assert.assertNotNull(capturedFilter[0].getOrderBy()); + Assert.assertTrue(capturedFilter[0].getOrderBy().contains("ORDER BY RAND()")); + Assert.assertTrue(capturedFilter[0].getOrderBy().contains("LIMIT 1")); + } + + @Test + public void testLockOneRandomRowReturnsNullOnEmptyResult() { + GenericDaoBase testDao = Mockito.mock(GenericDaoBase.class); + + Mockito.when(testDao.lockOneRandomRow(Mockito.any(SearchCriteria.class), Mockito.anyBoolean())) + .thenCallRealMethod(); + + Mockito.when(testDao.search(Mockito.any(SearchCriteria.class), Mockito.any(Filter.class), + Mockito.anyBoolean(), Mockito.anyBoolean())) + .thenReturn(new ArrayList()); + + SearchCriteria sc = Mockito.mock(SearchCriteria.class); + DbTestVO result = testDao.lockOneRandomRow(sc, true); + + Assert.assertNull(result); + } + + @Test + public void testLockOneRandomRowReturnsFirstElement() { + GenericDaoBase testDao = Mockito.mock(GenericDaoBase.class); + DbTestVO expectedResult = new DbTestVO(); + List resultList = new ArrayList<>(); + resultList.add(expectedResult); + + Mockito.when(testDao.lockOneRandomRow(Mockito.any(SearchCriteria.class), Mockito.anyBoolean())) + .thenCallRealMethod(); + + Mockito.when(testDao.search(Mockito.any(SearchCriteria.class), Mockito.any(Filter.class), + Mockito.anyBoolean(), Mockito.anyBoolean())) + .thenReturn(resultList); + + SearchCriteria sc = Mockito.mock(SearchCriteria.class); + DbTestVO result = testDao.lockOneRandomRow(sc, true); + + Assert.assertNotNull(result); + Assert.assertEquals(expectedResult, result); + } } diff --git a/framework/quota/src/main/java/org/apache/cloudstack/quota/QuotaManagerImpl.java b/framework/quota/src/main/java/org/apache/cloudstack/quota/QuotaManagerImpl.java index 99181f80c29..7c3eaead63f 100644 --- a/framework/quota/src/main/java/org/apache/cloudstack/quota/QuotaManagerImpl.java +++ b/framework/quota/src/main/java/org/apache/cloudstack/quota/QuotaManagerImpl.java @@ -428,7 +428,7 @@ public class QuotaManagerImpl extends ManagerBase implements QuotaManager { } injectPresetVariablesIntoJsInterpreter(jsInterpreter, presetVariables); - jsInterpreter.injectVariable("lastTariffs", lastAppliedTariffsList.toString()); + jsInterpreter.injectVariable("lastTariffs", lastAppliedTariffsList); String scriptResult = jsInterpreter.executeScript(activationRule).toString(); @@ -458,18 +458,18 @@ public class QuotaManagerImpl extends ManagerBase implements QuotaManager { protected void injectPresetVariablesIntoJsInterpreter(JsInterpreter jsInterpreter, PresetVariables presetVariables) { jsInterpreter.discardCurrentVariables(); - jsInterpreter.injectVariable("account", presetVariables.getAccount().toString()); - jsInterpreter.injectVariable("domain", presetVariables.getDomain().toString()); + jsInterpreter.injectVariable("account", presetVariables.getAccount()); + jsInterpreter.injectVariable("domain", presetVariables.getDomain()); GenericPresetVariable project = presetVariables.getProject(); if (project != null) { - jsInterpreter.injectVariable("project", project.toString()); + jsInterpreter.injectVariable("project", project); } jsInterpreter.injectVariable("resourceType", presetVariables.getResourceType()); - jsInterpreter.injectVariable("value", presetVariables.getValue().toString()); - jsInterpreter.injectVariable("zone", presetVariables.getZone().toString()); + jsInterpreter.injectVariable("value", presetVariables.getValue()); + jsInterpreter.injectVariable("zone", presetVariables.getZone()); } /** diff --git a/framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/Account.java b/framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/Account.java index 37c90ab0bcd..289958fe447 100644 --- a/framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/Account.java +++ b/framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/Account.java @@ -28,7 +28,6 @@ public class Account extends GenericPresetVariable { public void setRole(Role role) { this.role = role; - fieldNamesToIncludeInToString.add("role"); } } diff --git a/framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/BackupOffering.java b/framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/BackupOffering.java index d8457d294ec..e3927d967f7 100644 --- a/framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/BackupOffering.java +++ b/framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/BackupOffering.java @@ -29,6 +29,5 @@ public class BackupOffering extends GenericPresetVariable { public void setExternalId(String externalId) { this.externalId = externalId; - fieldNamesToIncludeInToString.add("externalId"); } } diff --git a/framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/ComputeOffering.java b/framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/ComputeOffering.java index 1d294276d47..9f9575052d3 100644 --- a/framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/ComputeOffering.java +++ b/framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/ComputeOffering.java @@ -27,7 +27,6 @@ public class ComputeOffering extends GenericPresetVariable { public void setCustomized(boolean customized) { this.customized = customized; - fieldNamesToIncludeInToString.add("customized"); } } diff --git a/framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/Domain.java b/framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/Domain.java index 6d83da4cd8f..cbdfa3e4bb4 100644 --- a/framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/Domain.java +++ b/framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/Domain.java @@ -27,7 +27,6 @@ public class Domain extends GenericPresetVariable { public void setPath(String path) { this.path = path; - fieldNamesToIncludeInToString.add("path"); } } diff --git a/framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/GenericPresetVariable.java b/framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/GenericPresetVariable.java index 7073d2760d7..4db099ca479 100644 --- a/framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/GenericPresetVariable.java +++ b/framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/GenericPresetVariable.java @@ -17,10 +17,8 @@ package org.apache.cloudstack.quota.activationrule.presetvariables; -import java.util.HashSet; -import java.util.Set; - -import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils; +import org.apache.commons.lang3.builder.ToStringBuilder; +import org.apache.commons.lang3.builder.ToStringStyle; public class GenericPresetVariable { @PresetVariableDefinition(description = "ID of the resource.") @@ -29,15 +27,12 @@ public class GenericPresetVariable { @PresetVariableDefinition(description = "Name of the resource.") private String name; - protected transient Set fieldNamesToIncludeInToString = new HashSet<>(); - public String getId() { return id; } public void setId(String id) { this.id = id; - fieldNamesToIncludeInToString.add("id"); } public String getName() { @@ -46,15 +41,10 @@ public class GenericPresetVariable { public void setName(String name) { this.name = name; - fieldNamesToIncludeInToString.add("name"); } - /*** - * Converts the preset variable into a valid JSON object that will be injected into the JS interpreter. - * This method should not be overridden or changed. - */ @Override - public final String toString() { - return ReflectionToStringBuilderUtils.reflectOnlySelectedFields(this, fieldNamesToIncludeInToString.toArray(new String[0])); + public String toString() { + return ToStringBuilder.reflectionToString(this, ToStringStyle.JSON_STYLE); } } diff --git a/framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/Host.java b/framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/Host.java index 4a0fd2f5a07..6d54a143834 100644 --- a/framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/Host.java +++ b/framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/Host.java @@ -32,7 +32,6 @@ public class Host extends GenericPresetVariable { public void setTags(List tags) { this.tags = tags; - fieldNamesToIncludeInToString.add("tags"); } public Boolean getIsTagARule() { @@ -41,6 +40,5 @@ public class Host extends GenericPresetVariable { public void setIsTagARule(Boolean isTagARule) { this.isTagARule = isTagARule; - fieldNamesToIncludeInToString.add("isTagARule"); } } diff --git a/framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/PresetVariableHelper.java b/framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/PresetVariableHelper.java index d5df3ae8a91..918cf78b4e6 100644 --- a/framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/PresetVariableHelper.java +++ b/framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/PresetVariableHelper.java @@ -243,7 +243,7 @@ public class PresetVariableHelper { Role role = new Role(); role.setId(roleVo.getUuid()); role.setName(roleVo.getName()); - role.setType(roleVo.getRoleType()); + role.setType(roleVo.getRoleType().toString()); return role; } @@ -490,7 +490,7 @@ public class PresetVariableHelper { value.setDiskOffering(getPresetVariableValueDiskOffering(volumeVo.getDiskOfferingId())); value.setId(volumeVo.getUuid()); value.setName(volumeVo.getName()); - value.setProvisioningType(volumeVo.getProvisioningType()); + value.setProvisioningType(volumeVo.getProvisioningType().toString()); Long poolId = volumeVo.getPoolId(); if (poolId == null) { @@ -533,7 +533,7 @@ public class PresetVariableHelper { storage = new Storage(); storage.setId(storagePoolVo.getUuid()); storage.setName(storagePoolVo.getName()); - storage.setScope(storagePoolVo.getScope()); + storage.setScope(storagePoolVo.getScope().toString()); List storagePoolTagVOList = storagePoolTagsDao.findStoragePoolTags(storageId); List storageTags = new ArrayList<>(); boolean isTagARule = false; @@ -602,7 +602,7 @@ public class PresetVariableHelper { value.setId(snapshotVo.getUuid()); value.setName(snapshotVo.getName()); value.setSize(ByteScaleUtils.bytesToMebibytes(snapshotVo.getSize())); - value.setSnapshotType(Snapshot.Type.values()[snapshotVo.getSnapshotType()]); + value.setSnapshotType(Snapshot.Type.values()[snapshotVo.getSnapshotType()].toString()); value.setStorage(getPresetVariableValueStorage(getSnapshotDataStoreId(snapshotId, usageRecord.getZoneId()), usageType)); value.setTags(getPresetVariableValueResourceTags(snapshotId, ResourceObjectType.Snapshot)); Hypervisor.HypervisorType hypervisorType = snapshotVo.getHypervisorType(); @@ -671,7 +671,7 @@ public class PresetVariableHelper { value.setId(vmSnapshotVo.getUuid()); value.setName(vmSnapshotVo.getName()); value.setTags(getPresetVariableValueResourceTags(vmSnapshotId, ResourceObjectType.VMSnapshot)); - value.setVmSnapshotType(vmSnapshotVo.getType()); + value.setVmSnapshotType(vmSnapshotVo.getType().toString()); VMInstanceVO vmVo = vmInstanceDao.findByIdIncludingRemoved(vmSnapshotVo.getVmId()); if (vmVo != null && vmVo.getHypervisorType() != null) { diff --git a/framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/Role.java b/framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/Role.java index 3f953b3a4ff..3c61786cb0a 100644 --- a/framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/Role.java +++ b/framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/Role.java @@ -17,19 +17,16 @@ package org.apache.cloudstack.quota.activationrule.presetvariables; -import org.apache.cloudstack.acl.RoleType; - public class Role extends GenericPresetVariable { @PresetVariableDefinition(description = "Role type of the resource's owner.") - private RoleType type; + private String type; - public RoleType getType() { + public String getType() { return type; } - public void setType(RoleType type) { + public void setType(String type) { this.type = type; - fieldNamesToIncludeInToString.add("type"); } } diff --git a/framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/Storage.java b/framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/Storage.java index 9b6cfb31092..8ddae82f383 100644 --- a/framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/Storage.java +++ b/framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/Storage.java @@ -19,8 +19,6 @@ package org.apache.cloudstack.quota.activationrule.presetvariables; import java.util.List; -import com.cloud.storage.ScopeType; - public class Storage extends GenericPresetVariable { @PresetVariableDefinition(description = "List of string representing the tags of the storage where the volume is (i.e.: [\"a\", \"b\"]).") private List tags; @@ -29,7 +27,7 @@ public class Storage extends GenericPresetVariable { private Boolean isTagARule; @PresetVariableDefinition(description = "Scope of the storage where the volume is. Values can be: ZONE, CLUSTER or HOST. Applicable only for primary storages.") - private ScopeType scope; + private String scope; public List getTags() { return tags; @@ -37,7 +35,6 @@ public class Storage extends GenericPresetVariable { public void setTags(List tags) { this.tags = tags; - fieldNamesToIncludeInToString.add("tags"); } public Boolean getIsTagARule() { @@ -46,16 +43,14 @@ public class Storage extends GenericPresetVariable { public void setIsTagARule(Boolean isTagARule) { this.isTagARule = isTagARule; - fieldNamesToIncludeInToString.add("isTagARule"); } - public ScopeType getScope() { + public String getScope() { return scope; } - public void setScope(ScopeType scope) { + public void setScope(String scope) { this.scope = scope; - fieldNamesToIncludeInToString.add("scope"); } } diff --git a/framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/Tariff.java b/framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/Tariff.java index 3703820a1a4..9414908b3a2 100644 --- a/framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/Tariff.java +++ b/framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/Tariff.java @@ -28,6 +28,5 @@ public class Tariff extends GenericPresetVariable { public void setValue(BigDecimal value) { this.value = value; - fieldNamesToIncludeInToString.add("value"); } } diff --git a/framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/Value.java b/framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/Value.java index d87146d8798..98f9c2678a8 100644 --- a/framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/Value.java +++ b/framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/Value.java @@ -20,9 +20,6 @@ package org.apache.cloudstack.quota.activationrule.presetvariables; import java.util.List; import java.util.Map; -import com.cloud.storage.Snapshot; -import com.cloud.storage.Storage.ProvisioningType; -import com.cloud.vm.snapshot.VMSnapshot; import org.apache.cloudstack.quota.constant.QuotaTypes; public class Value extends GenericPresetVariable { @@ -60,13 +57,13 @@ public class Value extends GenericPresetVariable { private Long virtualSize; @PresetVariableDefinition(description = "Provisioning type of the resource. Values can be: thin, sparse or fat.", supportedTypes = {QuotaTypes.VOLUME}) - private ProvisioningType provisioningType; + private String provisioningType; @PresetVariableDefinition(description = "Type of the snapshot. Values can be: MANUAL, RECURRING, HOURLY, DAILY, WEEKLY and MONTHLY.", supportedTypes = {QuotaTypes.SNAPSHOT}) - private Snapshot.Type snapshotType; + private String snapshotType; @PresetVariableDefinition(description = "Type of the VM snapshot. Values can be: Disk or DiskAndMemory.", supportedTypes = {QuotaTypes.VM_SNAPSHOT}) - private VMSnapshot.Type vmSnapshotType; + private String vmSnapshotType; @PresetVariableDefinition(description = "Computing offering of the VM.", supportedTypes = {QuotaTypes.RUNNING_VM, QuotaTypes.ALLOCATED_VM}) private ComputeOffering computeOffering; @@ -101,7 +98,6 @@ public class Value extends GenericPresetVariable { public void setHost(Host host) { this.host = host; - fieldNamesToIncludeInToString.add("host"); } public String getOsName() { @@ -110,7 +106,6 @@ public class Value extends GenericPresetVariable { public void setOsName(String osName) { this.osName = osName; - fieldNamesToIncludeInToString.add("osName"); } public List getAccountResources() { @@ -119,7 +114,6 @@ public class Value extends GenericPresetVariable { public void setAccountResources(List accountResources) { this.accountResources = accountResources; - fieldNamesToIncludeInToString.add("accountResources"); } public Map getTags() { @@ -128,7 +122,6 @@ public class Value extends GenericPresetVariable { public void setTags(Map tags) { this.tags = tags; - fieldNamesToIncludeInToString.add("tags"); } public String getTag() { @@ -137,7 +130,6 @@ public class Value extends GenericPresetVariable { public void setTag(String tag) { this.tag = tag; - fieldNamesToIncludeInToString.add("tag"); } public Long getSize() { @@ -146,34 +138,30 @@ public class Value extends GenericPresetVariable { public void setSize(Long size) { this.size = size; - fieldNamesToIncludeInToString.add("size"); } - public ProvisioningType getProvisioningType() { + public String getProvisioningType() { return provisioningType; } - public void setProvisioningType(ProvisioningType provisioningType) { + public void setProvisioningType(String provisioningType) { this.provisioningType = provisioningType; - fieldNamesToIncludeInToString.add("provisioningType"); } - public Snapshot.Type getSnapshotType() { + public String getSnapshotType() { return snapshotType; } - public void setSnapshotType(Snapshot.Type snapshotType) { + public void setSnapshotType(String snapshotType) { this.snapshotType = snapshotType; - fieldNamesToIncludeInToString.add("snapshotType"); } - public VMSnapshot.Type getVmSnapshotType() { + public String getVmSnapshotType() { return vmSnapshotType; } - public void setVmSnapshotType(VMSnapshot.Type vmSnapshotType) { + public void setVmSnapshotType(String vmSnapshotType) { this.vmSnapshotType = vmSnapshotType; - fieldNamesToIncludeInToString.add("vmSnapshotType"); } public ComputeOffering getComputeOffering() { @@ -182,7 +170,6 @@ public class Value extends GenericPresetVariable { public void setComputeOffering(ComputeOffering computeOffering) { this.computeOffering = computeOffering; - fieldNamesToIncludeInToString.add("computeOffering"); } public GenericPresetVariable getTemplate() { @@ -191,7 +178,6 @@ public class Value extends GenericPresetVariable { public void setTemplate(GenericPresetVariable template) { this.template = template; - fieldNamesToIncludeInToString.add("template"); } public GenericPresetVariable getDiskOffering() { @@ -200,7 +186,6 @@ public class Value extends GenericPresetVariable { public void setDiskOffering(GenericPresetVariable diskOffering) { this.diskOffering = diskOffering; - fieldNamesToIncludeInToString.add("diskOffering"); } public Storage getStorage() { @@ -209,7 +194,6 @@ public class Value extends GenericPresetVariable { public void setStorage(Storage storage) { this.storage = storage; - fieldNamesToIncludeInToString.add("storage"); } public ComputingResources getComputingResources() { @@ -218,7 +202,6 @@ public class Value extends GenericPresetVariable { public void setComputingResources(ComputingResources computingResources) { this.computingResources = computingResources; - fieldNamesToIncludeInToString.add("computingResources"); } public Long getVirtualSize() { @@ -227,7 +210,6 @@ public class Value extends GenericPresetVariable { public void setVirtualSize(Long virtualSize) { this.virtualSize = virtualSize; - fieldNamesToIncludeInToString.add("virtualSize"); } public BackupOffering getBackupOffering() { @@ -236,12 +218,10 @@ public class Value extends GenericPresetVariable { public void setBackupOffering(BackupOffering backupOffering) { this.backupOffering = backupOffering; - fieldNamesToIncludeInToString.add("backupOffering"); } public void setHypervisorType(String hypervisorType) { this.hypervisorType = hypervisorType; - fieldNamesToIncludeInToString.add("hypervisorType"); } public String getHypervisorType() { @@ -250,7 +230,6 @@ public class Value extends GenericPresetVariable { public void setVolumeFormat(String volumeFormat) { this.volumeFormat = volumeFormat; - fieldNamesToIncludeInToString.add("volumeFormat"); } public String getVolumeFormat() { @@ -263,6 +242,5 @@ public class Value extends GenericPresetVariable { public void setState(String state) { this.state = state; - fieldNamesToIncludeInToString.add("state"); } } diff --git a/framework/quota/src/test/java/org/apache/cloudstack/quota/QuotaManagerImplTest.java b/framework/quota/src/test/java/org/apache/cloudstack/quota/QuotaManagerImplTest.java index 3b2ea54e86d..a33faa054de 100644 --- a/framework/quota/src/test/java/org/apache/cloudstack/quota/QuotaManagerImplTest.java +++ b/framework/quota/src/test/java/org/apache/cloudstack/quota/QuotaManagerImplTest.java @@ -267,12 +267,12 @@ public class QuotaManagerImplTest { quotaManagerImplSpy.injectPresetVariablesIntoJsInterpreter(jsInterpreterMock, presetVariablesMock); - Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("account"), Mockito.anyString()); - Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("domain"), Mockito.anyString()); - Mockito.verify(jsInterpreterMock, Mockito.never()).injectVariable(Mockito.eq("project"), Mockito.anyString()); - Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("resourceType"), Mockito.anyString()); - Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("value"), Mockito.anyString()); - Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("zone"), Mockito.anyString()); + Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("account"), Mockito.any()); + Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("domain"), Mockito.any()); + Mockito.verify(jsInterpreterMock, Mockito.never()).injectVariable(Mockito.eq("project"), Mockito.any()); + Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("resourceType"), Mockito.any()); + Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("value"), Mockito.any()); + Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("zone"), Mockito.any()); } @Test @@ -288,12 +288,12 @@ public class QuotaManagerImplTest { quotaManagerImplSpy.injectPresetVariablesIntoJsInterpreter(jsInterpreterMock, presetVariablesMock); - Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("account"), Mockito.anyString()); - Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("domain"), Mockito.anyString()); - Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("project"), Mockito.anyString()); - Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("resourceType"), Mockito.anyString()); - Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("value"), Mockito.anyString()); - Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("zone"), Mockito.anyString()); + Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("account"), Mockito.any()); + Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("domain"), Mockito.any()); + Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("project"), Mockito.any()); + Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("resourceType"), Mockito.any()); + Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("value"), Mockito.any()); + Mockito.verify(jsInterpreterMock).injectVariable(Mockito.eq("zone"), Mockito.any()); } @Test diff --git a/framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/AccountTest.java b/framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/AccountTest.java deleted file mode 100644 index 1e62235e71c..00000000000 --- a/framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/AccountTest.java +++ /dev/null @@ -1,34 +0,0 @@ -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you under the Apache License, Version 2.0 (the -// "License"); you may not use this file except in compliance -// with the License. You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. - -package org.apache.cloudstack.quota.activationrule.presetvariables; - -import org.junit.Assert; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.junit.MockitoJUnitRunner; - -@RunWith(MockitoJUnitRunner.class) -public class AccountTest { - - @Test - public void setRoleTestAddFieldRoleToCollection() { - Account variable = new Account(); - variable.setRole(null); - Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("role")); - } -} diff --git a/framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/BackupOfferingTest.java b/framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/BackupOfferingTest.java deleted file mode 100644 index 57c18f936f2..00000000000 --- a/framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/BackupOfferingTest.java +++ /dev/null @@ -1,36 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -package org.apache.cloudstack.quota.activationrule.presetvariables; - -import org.junit.Assert; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.junit.MockitoJUnitRunner; - -@RunWith(MockitoJUnitRunner.class) -public class BackupOfferingTest { - @Test - public void setExternalIdTestAddFieldExternalIdToCollection() { - BackupOffering backupOffering = new BackupOffering(); - backupOffering.setExternalId("any-external-id"); - Assert.assertTrue(backupOffering.fieldNamesToIncludeInToString.contains("externalId")); - } - -} diff --git a/framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/ComputeOfferingTest.java b/framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/ComputeOfferingTest.java deleted file mode 100644 index 5fbcbe76476..00000000000 --- a/framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/ComputeOfferingTest.java +++ /dev/null @@ -1,35 +0,0 @@ -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you under the Apache License, Version 2.0 (the -// "License"); you may not use this file except in compliance -// with the License. You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. - -package org.apache.cloudstack.quota.activationrule.presetvariables; - -import org.junit.Assert; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.junit.MockitoJUnitRunner; - -@RunWith(MockitoJUnitRunner.class) -public class ComputeOfferingTest { - - @Test - public void setCustomizedTestAddFieldCustomizedToCollection() { - ComputeOffering variable = new ComputeOffering(); - variable.setCustomized(true); - Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("customized")); - } - -} diff --git a/framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/ComputingResourcesTest.java b/framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/ComputingResourcesTest.java deleted file mode 100644 index f7978f16e04..00000000000 --- a/framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/ComputingResourcesTest.java +++ /dev/null @@ -1,40 +0,0 @@ -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you under the Apache License, Version 2.0 (the -// "License"); you may not use this file except in compliance -// with the License. You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. - -package org.apache.cloudstack.quota.activationrule.presetvariables; - -import org.apache.commons.lang3.builder.ToStringBuilder; -import org.apache.commons.lang3.builder.ToStringStyle; -import org.junit.Assert; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.junit.MockitoJUnitRunner; - -@RunWith(MockitoJUnitRunner.class) -public class ComputingResourcesTest { - - @Test - public void toStringTestReturnAJson() { - ComputingResources variable = new ComputingResources(); - - String expected = ToStringBuilder.reflectionToString(variable, ToStringStyle.JSON_STYLE); - String result = variable.toString(); - - Assert.assertEquals(expected, result); - } - -} diff --git a/framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/DomainTest.java b/framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/DomainTest.java deleted file mode 100644 index f245b4637e6..00000000000 --- a/framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/DomainTest.java +++ /dev/null @@ -1,35 +0,0 @@ -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you under the Apache License, Version 2.0 (the -// "License"); you may not use this file except in compliance -// with the License. You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. - -package org.apache.cloudstack.quota.activationrule.presetvariables; - -import org.junit.Assert; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.junit.MockitoJUnitRunner; - -@RunWith(MockitoJUnitRunner.class) -public class DomainTest { - - @Test - public void setPathTestAddFieldPathToCollection() { - Domain variable = new Domain(); - variable.setPath("test path"); - Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("path")); - } - -} diff --git a/framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/GenericPresetVariableTest.java b/framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/GenericPresetVariableTest.java deleted file mode 100644 index 4f594ee5d00..00000000000 --- a/framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/GenericPresetVariableTest.java +++ /dev/null @@ -1,73 +0,0 @@ -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you under the Apache License, Version 2.0 (the -// "License"); you may not use this file except in compliance -// with the License. You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. - -package org.apache.cloudstack.quota.activationrule.presetvariables; - -import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils; -import org.junit.Assert; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.junit.MockitoJUnitRunner; - -@RunWith(MockitoJUnitRunner.class) -public class GenericPresetVariableTest { - - @Test - public void setIdTestAddFieldIdToCollection() { - GenericPresetVariable variable = new GenericPresetVariable(); - variable.setId("test"); - Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("id")); - } - - @Test - public void setNameTestAddFieldNameToCollection() { - GenericPresetVariable variable = new GenericPresetVariable(); - variable.setName("test"); - Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("name")); - } - - @Test - public void toStringTestSetAllFieldsAndReturnAJson() { - GenericPresetVariable variable = new GenericPresetVariable(); - variable.setId("test id"); - variable.setName("test name"); - - String expected = ReflectionToStringBuilderUtils.reflectOnlySelectedFields(variable, "id", "name"); - String result = variable.toString(); - - Assert.assertEquals(expected, result); - } - - @Test - public void toStringTestSetSomeFieldsAndReturnAJson() { - GenericPresetVariable variable = new GenericPresetVariable(); - variable.setId("test id"); - - String expected = ReflectionToStringBuilderUtils.reflectOnlySelectedFields(variable, "id"); - String result = variable.toString(); - - Assert.assertEquals(expected, result); - - variable = new GenericPresetVariable(); - variable.setName("test name"); - - expected = ReflectionToStringBuilderUtils.reflectOnlySelectedFields(variable, "name"); - result = variable.toString(); - - Assert.assertEquals(expected, result); - } -} diff --git a/framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/HostTest.java b/framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/HostTest.java deleted file mode 100644 index 87aae7788e2..00000000000 --- a/framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/HostTest.java +++ /dev/null @@ -1,34 +0,0 @@ -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you under the Apache License, Version 2.0 (the -// "License"); you may not use this file except in compliance -// with the License. You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. - -package org.apache.cloudstack.quota.activationrule.presetvariables; - -import org.junit.Assert; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.junit.MockitoJUnitRunner; - -@RunWith(MockitoJUnitRunner.class) -public class HostTest { - - @Test - public void setTagsTestAddFieldTagsToCollection() { - Host variable = new Host(); - variable.setTags(null); - Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("tags")); - } -} diff --git a/framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/PresetVariableHelperTest.java b/framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/PresetVariableHelperTest.java index 45af4b8a29a..095ab422ee7 100644 --- a/framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/PresetVariableHelperTest.java +++ b/framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/PresetVariableHelperTest.java @@ -209,12 +209,12 @@ public class PresetVariableHelperTest { value.setTags(Collections.singletonMap("tag1", "value1")); value.setTemplate(getGenericPresetVariableForTests()); value.setDiskOffering(getGenericPresetVariableForTests()); - value.setProvisioningType(ProvisioningType.THIN); + value.setProvisioningType(ProvisioningType.THIN.toString()); value.setStorage(getStorageForTests()); value.setSize(ByteScaleUtils.GiB); - value.setSnapshotType(Snapshot.Type.HOURLY); + value.setSnapshotType(Snapshot.Type.HOURLY.toString()); value.setTag("tag_test"); - value.setVmSnapshotType(VMSnapshot.Type.Disk); + value.setVmSnapshotType(VMSnapshot.Type.Disk.toString()); value.setComputingResources(getComputingResourcesForTests()); return value; } @@ -256,7 +256,7 @@ public class PresetVariableHelperTest { storage.setId("storage_id"); storage.setName("storage_name"); storage.setTags(Arrays.asList("tag1", "tag2")); - storage.setScope(ScopeType.ZONE); + storage.setScope(ScopeType.ZONE.toString()); return storage; } @@ -293,13 +293,6 @@ public class PresetVariableHelperTest { Assert.assertEquals(expected.getName(), result.getName()); } - private void validateFieldNamesToIncludeInToString(List expected, GenericPresetVariable resultObject) { - List result = new ArrayList<>(resultObject.fieldNamesToIncludeInToString); - Collections.sort(expected); - Collections.sort(result); - Assert.assertEquals(expected, result); - } - private BackupOffering getBackupOfferingForTests() { BackupOffering backupOffering = new BackupOffering(); backupOffering.setId("backup_offering_id"); @@ -362,7 +355,6 @@ public class PresetVariableHelperTest { Assert.assertNotNull(result.getProject()); assertPresetVariableIdAndName(account, result.getProject()); - validateFieldNamesToIncludeInToString(Arrays.asList("id", "name"), result.getProject()); } @Test @@ -379,7 +371,6 @@ public class PresetVariableHelperTest { Account result = presetVariableHelperSpy.getPresetVariableAccount(1l); assertPresetVariableIdAndName(account, result); - validateFieldNamesToIncludeInToString(Arrays.asList("id", "name"), result); } @Test @@ -409,18 +400,16 @@ public class PresetVariableHelperTest { Role role = new Role(); role.setId("test_id"); role.setName("test_name"); - role.setType(roleType); + role.setType(roleType.toString()); Mockito.doReturn(role.getId()).when(roleVoMock).getUuid(); Mockito.doReturn(role.getName()).when(roleVoMock).getName(); - Mockito.doReturn(role.getType()).when(roleVoMock).getRoleType(); + Mockito.doReturn(RoleType.fromString(role.getType())).when(roleVoMock).getRoleType(); Role result = presetVariableHelperSpy.getPresetVariableRole(1l); assertPresetVariableIdAndName(role, result); Assert.assertEquals(role.getType(), result.getType()); - - validateFieldNamesToIncludeInToString(Arrays.asList("id", "name", "type"), result); }); } @@ -439,8 +428,6 @@ public class PresetVariableHelperTest { assertPresetVariableIdAndName(domain, result); Assert.assertEquals(domain.getPath(), result.getPath()); - - validateFieldNamesToIncludeInToString(Arrays.asList("id", "name", "path"), result); } @Test @@ -456,7 +443,6 @@ public class PresetVariableHelperTest { GenericPresetVariable result = presetVariableHelperSpy.getPresetVariableZone(1l); assertPresetVariableIdAndName(expected, result); - validateFieldNamesToIncludeInToString(Arrays.asList("id", "name"), result); } @Test @@ -477,7 +463,6 @@ public class PresetVariableHelperTest { Value result = presetVariableHelperSpy.getPresetVariableValue(usageVoMock); Assert.assertEquals(resources, result.getAccountResources()); - validateFieldNamesToIncludeInToString(Arrays.asList("accountResources"), result); } @Test @@ -536,8 +521,6 @@ public class PresetVariableHelperTest { Assert.assertEquals(expected.getTags(), result.getTags()); Assert.assertEquals(expected.getTemplate(), result.getTemplate()); Assert.assertEquals(hypervisorType.name(), result.getHypervisorType()); - - validateFieldNamesToIncludeInToString(Arrays.asList("id", "name", "osName", "tags", "template", "hypervisorType"), result); }); } @@ -569,7 +552,6 @@ public class PresetVariableHelperTest { assertPresetVariableIdAndName(expectedHost, result.getHost()); Assert.assertEquals(expectedHost.getTags(), result.getHost().getTags()); - validateFieldNamesToIncludeInToString(Arrays.asList("host"), result); } @Test @@ -588,7 +570,6 @@ public class PresetVariableHelperTest { assertPresetVariableIdAndName(expected, result); Assert.assertEquals(expected.getTags(), result.getTags()); - validateFieldNamesToIncludeInToString(Arrays.asList("id", "isTagARule", "name", "tags"), result); } @Test @@ -608,7 +589,6 @@ public class PresetVariableHelperTest { assertPresetVariableIdAndName(expected, result); Assert.assertEquals(new ArrayList<>(), result.getTags()); Assert.assertTrue(result.getIsTagARule()); - validateFieldNamesToIncludeInToString(Arrays.asList("id", "isTagARule", "name", "tags"), result); } @Test @@ -636,7 +616,6 @@ public class PresetVariableHelperTest { assertPresetVariableIdAndName(expected, result); Assert.assertEquals(expected.isCustomized(), result.isCustomized()); - validateFieldNamesToIncludeInToString(Arrays.asList("id", "name", "customized"), result); } @Test @@ -652,7 +631,6 @@ public class PresetVariableHelperTest { GenericPresetVariable result = presetVariableHelperSpy.getPresetVariableValueTemplate(1l); assertPresetVariableIdAndName(expected, result); - validateFieldNamesToIncludeInToString(Arrays.asList("id", "name"), result); } @Test @@ -696,7 +674,7 @@ public class PresetVariableHelperTest { Mockito.doReturn(expected.getId()).when(volumeVoMock).getUuid(); Mockito.doReturn(expected.getName()).when(volumeVoMock).getName(); Mockito.doReturn(expected.getDiskOffering()).when(presetVariableHelperSpy).getPresetVariableValueDiskOffering(Mockito.anyLong()); - Mockito.doReturn(expected.getProvisioningType()).when(volumeVoMock).getProvisioningType(); + Mockito.doReturn(ProvisioningType.getProvisioningType(expected.getProvisioningType())).when(volumeVoMock).getProvisioningType(); Mockito.doReturn(expected.getStorage()).when(presetVariableHelperSpy).getPresetVariableValueStorage(Mockito.anyLong(), Mockito.anyInt()); Mockito.doReturn(expected.getTags()).when(presetVariableHelperSpy).getPresetVariableValueResourceTags(Mockito.anyLong(), Mockito.any(ResourceObjectType.class)); Mockito.doReturn(expected.getSize()).when(volumeVoMock).getSize(); @@ -716,8 +694,6 @@ public class PresetVariableHelperTest { Assert.assertEquals(expected.getTags(), result.getTags()); Assert.assertEquals(expectedSize, result.getSize()); Assert.assertEquals(imageFormat.name(), result.getVolumeFormat()); - - validateFieldNamesToIncludeInToString(Arrays.asList("id", "name", "diskOffering", "provisioningType", "storage", "tags", "size", "volumeFormat"), result); } Mockito.verify(presetVariableHelperSpy, Mockito.times(ImageFormat.values().length)).getPresetVariableValueResourceTags(Mockito.anyLong(), @@ -738,7 +714,7 @@ public class PresetVariableHelperTest { Mockito.doReturn(expected.getId()).when(volumeVoMock).getUuid(); Mockito.doReturn(expected.getName()).when(volumeVoMock).getName(); Mockito.doReturn(expected.getDiskOffering()).when(presetVariableHelperSpy).getPresetVariableValueDiskOffering(Mockito.anyLong()); - Mockito.doReturn(expected.getProvisioningType()).when(volumeVoMock).getProvisioningType(); + Mockito.doReturn(ProvisioningType.getProvisioningType(expected.getProvisioningType())).when(volumeVoMock).getProvisioningType(); Mockito.doReturn(expected.getTags()).when(presetVariableHelperSpy).getPresetVariableValueResourceTags(Mockito.anyLong(), Mockito.any(ResourceObjectType.class)); Mockito.doReturn(expected.getSize()).when(volumeVoMock).getSize(); Mockito.doReturn(imageFormat).when(volumeVoMock).getFormat(); @@ -757,8 +733,6 @@ public class PresetVariableHelperTest { Assert.assertEquals(expected.getTags(), result.getTags()); Assert.assertEquals(expectedSize, result.getSize()); Assert.assertEquals(imageFormat.name(), result.getVolumeFormat()); - - validateFieldNamesToIncludeInToString(Arrays.asList("id", "name", "diskOffering", "provisioningType", "tags", "size", "volumeFormat"), result); } Mockito.verify(presetVariableHelperSpy, Mockito.times(ImageFormat.values().length)).getPresetVariableValueResourceTags(Mockito.anyLong(), @@ -778,7 +752,6 @@ public class PresetVariableHelperTest { GenericPresetVariable result = presetVariableHelperSpy.getPresetVariableValueDiskOffering(1l); assertPresetVariableIdAndName(expected, result); - validateFieldNamesToIncludeInToString(Arrays.asList("id", "name"), result); } @Test @@ -804,7 +777,7 @@ public class PresetVariableHelperTest { Mockito.doReturn(expected.getId()).when(storagePoolVoMock).getUuid(); Mockito.doReturn(expected.getName()).when(storagePoolVoMock).getName(); - Mockito.doReturn(expected.getScope()).when(storagePoolVoMock).getScope(); + Mockito.doReturn(ScopeType.validateAndGetScopeType(expected.getScope())).when(storagePoolVoMock).getScope(); Mockito.doReturn(storageTagVOListMock).when(storagePoolTagsDaoMock).findStoragePoolTags(Mockito.anyLong()); Storage result = presetVariableHelperSpy.getPresetVariableValueStorage(1l, 2); @@ -812,8 +785,6 @@ public class PresetVariableHelperTest { assertPresetVariableIdAndName(expected, result); Assert.assertEquals(expected.getScope(), result.getScope()); Assert.assertEquals(expected.getTags(), result.getTags()); - - validateFieldNamesToIncludeInToString(Arrays.asList("id", "isTagARule", "name", "scope", "tags"), result); } @Test @@ -828,7 +799,7 @@ public class PresetVariableHelperTest { Mockito.doReturn(expected.getId()).when(storagePoolVoMock).getUuid(); Mockito.doReturn(expected.getName()).when(storagePoolVoMock).getName(); - Mockito.doReturn(expected.getScope()).when(storagePoolVoMock).getScope(); + Mockito.doReturn(ScopeType.validateAndGetScopeType(expected.getScope())).when(storagePoolVoMock).getScope(); Mockito.doReturn(storageTagVOListMock).when(storagePoolTagsDaoMock).findStoragePoolTags(Mockito.anyLong()); Storage result = presetVariableHelperSpy.getPresetVariableValueStorage(1l, 2); @@ -837,8 +808,6 @@ public class PresetVariableHelperTest { Assert.assertEquals(expected.getScope(), result.getScope()); Assert.assertEquals(new ArrayList<>(), result.getTags()); Assert.assertTrue(result.getIsTagARule()); - - validateFieldNamesToIncludeInToString(Arrays.asList("id", "isTagARule", "name", "scope", "tags"), result); } @Test @@ -874,7 +843,6 @@ public class PresetVariableHelperTest { Storage result = presetVariableHelperSpy.getSecondaryStorageForSnapshot(1l, UsageTypes.SNAPSHOT); assertPresetVariableIdAndName(expected, result); - validateFieldNamesToIncludeInToString(Arrays.asList("id", "name"), result); } @Test @@ -915,8 +883,6 @@ public class PresetVariableHelperTest { Assert.assertEquals(expected.getOsName(), result.getOsName()); Assert.assertEquals(expected.getTags(), result.getTags()); Assert.assertEquals(expectedSize, result.getSize()); - - validateFieldNamesToIncludeInToString(Arrays.asList("id", "name", "osName", "tags", "size"), result); }); Mockito.verify(presetVariableHelperSpy).getPresetVariableValueResourceTags(Mockito.anyLong(), Mockito.eq(ResourceObjectType.Template)); @@ -967,8 +933,6 @@ public class PresetVariableHelperTest { Assert.assertEquals(expected.getTags(), result.getTags()); Assert.assertEquals(expectedSize, result.getSize()); Assert.assertEquals(hypervisorType.name(), result.getHypervisorType()); - - validateFieldNamesToIncludeInToString(Arrays.asList("id", "name", "snapshotType", "storage", "tags", "size", "hypervisorType"), result); } Mockito.verify(presetVariableHelperSpy, Mockito.times(Hypervisor.HypervisorType.values().length)).getPresetVariableValueResourceTags(Mockito.anyLong(), @@ -1053,8 +1017,6 @@ public class PresetVariableHelperTest { assertPresetVariableIdAndName(expected, result); Assert.assertEquals(expected.getTag(), result.getTag()); - - validateFieldNamesToIncludeInToString(Arrays.asList("id", "name", "tag"), result); } @Test @@ -1079,7 +1041,7 @@ public class PresetVariableHelperTest { Mockito.doReturn(expected.getId()).when(vmSnapshotVoMock).getUuid(); Mockito.doReturn(expected.getName()).when(vmSnapshotVoMock).getName(); Mockito.doReturn(expected.getTags()).when(presetVariableHelperSpy).getPresetVariableValueResourceTags(Mockito.anyLong(), Mockito.any(ResourceObjectType.class)); - Mockito.doReturn(expected.getVmSnapshotType()).when(vmSnapshotVoMock).getType(); + Mockito.doReturn(VMSnapshot.Type.valueOf(expected.getVmSnapshotType())).when(vmSnapshotVoMock).getType(); Mockito.doReturn(UsageTypes.VM_SNAPSHOT).when(usageVoMock).getUsageType(); @@ -1090,8 +1052,6 @@ public class PresetVariableHelperTest { Assert.assertEquals(expected.getTags(), result.getTags()); Assert.assertEquals(expected.getVmSnapshotType(), result.getVmSnapshotType()); - validateFieldNamesToIncludeInToString(Arrays.asList("id", "name", "tags", "vmSnapshotType"), result); - Mockito.verify(presetVariableHelperSpy).getPresetVariableValueResourceTags(Mockito.anyLong(), Mockito.eq(ResourceObjectType.VMSnapshot)); } @@ -1124,9 +1084,6 @@ public class PresetVariableHelperTest { if (typeInt == UsageTypes.RUNNING_VM) { Assert.assertEquals(expected.getComputingResources(), result.getComputingResources()); - validateFieldNamesToIncludeInToString(Arrays.asList("computeOffering", "computingResources"), result); - } else { - validateFieldNamesToIncludeInToString(Arrays.asList("computeOffering"), result); } }); } @@ -1219,8 +1176,6 @@ public class PresetVariableHelperTest { Assert.assertEquals(expected.getVirtualSize(), result.getVirtualSize()); Assert.assertEquals(expected.getBackupOffering(), result.getBackupOffering()); - validateFieldNamesToIncludeInToString(Arrays.asList("size", "virtualSize", "backupOffering"), result); - Mockito.verify(presetVariableHelperSpy).getPresetVariableValueBackupOffering(Mockito.anyLong()); } @@ -1239,7 +1194,6 @@ public class PresetVariableHelperTest { assertPresetVariableIdAndName(expected, result); Assert.assertEquals(expected.getExternalId(), result.getExternalId()); - validateFieldNamesToIncludeInToString(Arrays.asList("id", "name", "externalId"), result); } @Test diff --git a/framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/RoleTest.java b/framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/RoleTest.java deleted file mode 100644 index 88265ee4e55..00000000000 --- a/framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/RoleTest.java +++ /dev/null @@ -1,34 +0,0 @@ -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you under the Apache License, Version 2.0 (the -// "License"); you may not use this file except in compliance -// with the License. You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. - -package org.apache.cloudstack.quota.activationrule.presetvariables; - -import org.junit.Assert; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.junit.MockitoJUnitRunner; - -@RunWith(MockitoJUnitRunner.class) -public class RoleTest { - - @Test - public void setTagsTestAddFieldTagsToCollection() { - Role variable = new Role(); - variable.setType(null); - Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("type")); - } -} diff --git a/framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/StorageTest.java b/framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/StorageTest.java deleted file mode 100644 index f36d5c49581..00000000000 --- a/framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/StorageTest.java +++ /dev/null @@ -1,41 +0,0 @@ -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you under the Apache License, Version 2.0 (the -// "License"); you may not use this file except in compliance -// with the License. You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. - -package org.apache.cloudstack.quota.activationrule.presetvariables; - -import org.junit.Assert; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.junit.MockitoJUnitRunner; - -@RunWith(MockitoJUnitRunner.class) -public class StorageTest { - - @Test - public void setTagsTestAddFieldTagsToCollection() { - Storage variable = new Storage(); - variable.setTags(null); - Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("tags")); - } - - @Test - public void setScopeTestAddFieldScopeToCollection() { - Storage variable = new Storage(); - variable.setScope(null);; - Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("scope")); - } -} diff --git a/framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/ValueTest.java b/framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/ValueTest.java deleted file mode 100644 index bad33da8836..00000000000 --- a/framework/quota/src/test/java/org/apache/cloudstack/quota/activationrule/presetvariables/ValueTest.java +++ /dev/null @@ -1,175 +0,0 @@ -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you under the Apache License, Version 2.0 (the -// "License"); you may not use this file except in compliance -// with the License. You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. - -package org.apache.cloudstack.quota.activationrule.presetvariables; - -import org.junit.Assert; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.junit.MockitoJUnitRunner; - -@RunWith(MockitoJUnitRunner.class) -public class ValueTest { - - @Test - public void setIdTestAddFieldIdToCollection() { - Value variable = new Value(); - variable.setId(null); - Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("id")); - } - - @Test - public void setNameTestAddFieldNameToCollection() { - Value variable = new Value(); - variable.setName(null); - Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("name")); - } - - @Test - public void setHostTestAddFieldHostToCollection() { - Value variable = new Value(); - variable.setHost(null); - Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("host")); - } - - @Test - public void setOsNameTestAddFieldOsNameToCollection() { - Value variable = new Value(); - variable.setOsName(null); - Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("osName")); - } - - @Test - public void setAccountResourcesTestAddFieldAccountResourcesToCollection() { - Value variable = new Value(); - variable.setAccountResources(null); - Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("accountResources")); - } - - @Test - public void setTagsTestAddFieldTagsToCollection() { - Value variable = new Value(); - variable.setTags(null); - Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("tags")); - } - - @Test - public void setTagTestAddFieldTagToCollection() { - Value variable = new Value(); - variable.setTag(null); - Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("tag")); - } - - @Test - public void setSizeTestAddFieldSizeToCollection() { - Value variable = new Value(); - variable.setSize(null); - Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("size")); - } - - @Test - public void setProvisioningTypeTestAddFieldProvisioningTypeToCollection() { - Value variable = new Value(); - variable.setProvisioningType(null); - Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("provisioningType")); - } - - @Test - public void setSnapshotTypeTestAddFieldSnapshotTypeToCollection() { - Value variable = new Value(); - variable.setSnapshotType(null); - Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("snapshotType")); - } - - @Test - public void setVmSnapshotTypeTestAddFieldVmSnapshotTypeToCollection() { - Value variable = new Value(); - variable.setVmSnapshotType(null); - Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("vmSnapshotType")); - } - - @Test - public void setComputeOfferingTestAddFieldComputeOfferingToCollection() { - Value variable = new Value(); - variable.setComputeOffering(null); - Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("computeOffering")); - } - - @Test - public void setTemplateTestAddFieldTemplateToCollection() { - Value variable = new Value(); - variable.setTemplate(null); - Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("template")); - } - - @Test - public void setDiskOfferingTestAddFieldDiskOfferingToCollection() { - Value variable = new Value(); - variable.setDiskOffering(null); - Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("diskOffering")); - } - - @Test - public void setStorageTestAddFieldStorageToCollection() { - Value variable = new Value(); - variable.setStorage(null); - Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("storage")); - } - - @Test - public void setComputingResourcesTestAddFieldComputingResourcesToCollection() { - Value variable = new Value(); - variable.setComputingResources(null); - Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("computingResources")); - } - - @Test - public void setVirtualSizeTestAddFieldVirtualSizeToCollection() { - Value variable = new Value(); - variable.setVirtualSize(null); - Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("virtualSize")); - } - - @Test - public void setBackupOfferingTestAddFieldBackupOfferingToCollection() { - Value variable = new Value(); - variable.setBackupOffering(null); - Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("backupOffering")); - } - - @Test - public void setHypervisorTypeTestAddFieldHypervisorTypeToCollection() { - Value variable = new Value(); - variable.setHypervisorType(null); - Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("hypervisorType")); - } - - @Test - public void setVolumeFormatTestAddFieldVolumeFormatToCollection() { - Value variable = new Value(); - variable.setVolumeFormat(null); - Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("volumeFormat")); - } - - @Test - public void setStateTestAddFieldStateToCollection() { - Value variable = new Value(); - variable.setState(null); - Assert.assertTrue(variable.fieldNamesToIncludeInToString.contains("state")); - } - -} diff --git a/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java b/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java index 9b5672e228f..565ea29acf8 100644 --- a/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java +++ b/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java @@ -58,7 +58,6 @@ import java.util.Locale; import java.util.Map; import java.util.HashMap; import java.util.Objects; -import java.util.Optional; import java.util.UUID; import java.util.stream.Collectors; @@ -230,6 +229,7 @@ public class NASBackupProvider extends AdapterBase implements BackupProvider, Co restoreCommand.setMountOptions(backupRepository.getMountOptions()); restoreCommand.setVmName(vm.getName()); restoreCommand.setVolumePaths(getVolumePaths(volumes)); + restoreCommand.setBackupFiles(getBackupFiles(backedVolumes)); restoreCommand.setVmExists(vm.getRemoved() == null); restoreCommand.setVmState(vm.getState()); @@ -244,6 +244,14 @@ public class NASBackupProvider extends AdapterBase implements BackupProvider, Co return answer.getResult(); } + private List getBackupFiles(List backedVolumes) { + List backupFiles = new ArrayList<>(); + for (Backup.VolumeInfo backedVolume : backedVolumes) { + backupFiles.add(backedVolume.getPath()); + } + return backupFiles; + } + private List getVolumePaths(List volumes) { List volumePaths = new ArrayList<>(); for (VolumeVO volume : volumes) { @@ -271,8 +279,11 @@ public class NASBackupProvider extends AdapterBase implements BackupProvider, Co final StoragePoolHostVO dataStore = storagePoolHostDao.findByUuid(dataStoreUuid); final HostVO hostVO = hostDao.findByIp(hostIp); - Optional matchingVolume = getBackedUpVolumeInfo(backupSourceVm.getBackupVolumeList(), volumeUuid); - Long backedUpVolumeSize = matchingVolume.isPresent() ? matchingVolume.get().getSize() : 0L; + Backup.VolumeInfo matchingVolume = getBackedUpVolumeInfo(backup.getBackedUpVolumes(), volumeUuid); + if (matchingVolume == null) { + throw new CloudRuntimeException(String.format("Unable to find volume %s in the list of backed up volumes for backup %s, cannot proceed with restore", volumeUuid, backup)); + } + Long backedUpVolumeSize = matchingVolume.getSize(); LOG.debug("Restoring vm volume {} from backup {} on the NAS Backup Provider", volume, backup); BackupRepository backupRepository = getBackupRepository(backupSourceVm, backup); @@ -300,11 +311,11 @@ public class NASBackupProvider extends AdapterBase implements BackupProvider, Co restoreCommand.setBackupRepoAddress(backupRepository.getAddress()); restoreCommand.setVmName(vmNameAndState.first()); restoreCommand.setVolumePaths(Collections.singletonList(String.format("%s/%s", dataStore.getLocalPath(), volumeUUID))); + restoreCommand.setBackupFiles(Collections.singletonList(matchingVolume.getPath())); restoreCommand.setDiskType(volume.getVolumeType().name().toLowerCase(Locale.ROOT)); restoreCommand.setMountOptions(backupRepository.getMountOptions()); restoreCommand.setVmExists(null); restoreCommand.setVmState(vmNameAndState.second()); - restoreCommand.setRestoreVolumeUUID(volumeUuid); BackupAnswer answer = null; try { @@ -339,10 +350,11 @@ public class NASBackupProvider extends AdapterBase implements BackupProvider, Co return backupRepository; } - private Optional getBackedUpVolumeInfo(List backedUpVolumes, String volumeUuid) { + private Backup.VolumeInfo getBackedUpVolumeInfo(List backedUpVolumes, String volumeUuid) { return backedUpVolumes.stream() .filter(v -> v.getUuid().equals(volumeUuid)) - .findFirst(); + .findFirst() + .orElse(null); } @Override diff --git a/plugins/backup/networker/src/main/java/org/apache/cloudstack/backup/NetworkerBackupProvider.java b/plugins/backup/networker/src/main/java/org/apache/cloudstack/backup/NetworkerBackupProvider.java index 393e2911ac3..3f14ab259a0 100644 --- a/plugins/backup/networker/src/main/java/org/apache/cloudstack/backup/NetworkerBackupProvider.java +++ b/plugins/backup/networker/src/main/java/org/apache/cloudstack/backup/NetworkerBackupProvider.java @@ -16,6 +16,7 @@ // under the License. package org.apache.cloudstack.backup; +import com.cloud.agent.AgentManager; import com.cloud.dc.dao.ClusterDao; import com.cloud.host.HostVO; import com.cloud.host.Status; @@ -117,6 +118,9 @@ public class NetworkerBackupProvider extends AdapterBase implements BackupProvid @Inject private VMInstanceDao vmInstanceDao; + @Inject + private AgentManager agentMgr; + private static String getUrlDomain(String url) throws URISyntaxException { URI uri; try { @@ -229,8 +233,13 @@ public class NetworkerBackupProvider extends AdapterBase implements BackupProvid String nstRegex = "\\bcompleted savetime=([0-9]{10})"; Pattern saveTimePattern = Pattern.compile(nstRegex); + if (host == null) { + LOG.warn("Unable to take backup, host is null"); + return null; + } + try { - Pair response = SshHelper.sshExecute(host.getPrivateIpAddress(), 22, + Pair response = SshHelper.sshExecute(host.getPrivateIpAddress(), agentMgr.getHostSshPort(host), username, null, password, command, 120000, 120000, 3600000); if (!response.first()) { LOG.error(String.format("Backup Script failed on HYPERVISOR %s due to: %s", host, response.second())); @@ -249,9 +258,13 @@ public class NetworkerBackupProvider extends AdapterBase implements BackupProvid return null; } private boolean executeRestoreCommand(HostVO host, String username, String password, String command) { + if (host == null) { + LOG.warn("Unable to restore backup, host is null"); + return false; + } try { - Pair response = SshHelper.sshExecute(host.getPrivateIpAddress(), 22, + Pair response = SshHelper.sshExecute(host.getPrivateIpAddress(), agentMgr.getHostSshPort(host), username, null, password, command, 120000, 120000, 3600000); if (!response.first()) { diff --git a/plugins/event-bus/rabbitmq/src/main/java/org/apache/cloudstack/mom/rabbitmq/RabbitMQEventBus.java b/plugins/event-bus/rabbitmq/src/main/java/org/apache/cloudstack/mom/rabbitmq/RabbitMQEventBus.java index e8067e75b40..2851ef3498e 100644 --- a/plugins/event-bus/rabbitmq/src/main/java/org/apache/cloudstack/mom/rabbitmq/RabbitMQEventBus.java +++ b/plugins/event-bus/rabbitmq/src/main/java/org/apache/cloudstack/mom/rabbitmq/RabbitMQEventBus.java @@ -492,7 +492,7 @@ public class RabbitMQEventBus extends ManagerBase implements EventBus { @Override public synchronized boolean stop() { - if (s_connection.isOpen()) { + if (s_connection != null && s_connection.isOpen()) { for (String subscriberId : s_subscribers.keySet()) { Ternary subscriberDetails = s_subscribers.get(subscriberId); Channel channel = subscriberDetails.second(); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java index 8abc359250c..47b903c47a7 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java @@ -31,7 +31,6 @@ import org.apache.cloudstack.backup.BackupAnswer; import org.apache.cloudstack.backup.RestoreBackupCommand; import org.apache.commons.lang3.RandomStringUtils; -import java.io.File; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Paths; @@ -59,20 +58,21 @@ public class LibvirtRestoreBackupCommandWrapper extends CommandWrapper volumePaths = command.getVolumePaths(); - String restoreVolumeUuid = command.getRestoreVolumeUUID(); + List backupFiles = command.getBackupFiles(); String newVolumeId = null; try { if (Objects.isNull(vmExists)) { String volumePath = volumePaths.get(0); + String backupFile = backupFiles.get(0); int lastIndex = volumePath.lastIndexOf("/"); newVolumeId = volumePath.substring(lastIndex + 1); - restoreVolume(backupPath, backupRepoType, backupRepoAddress, volumePath, diskType, restoreVolumeUuid, + restoreVolume(backupPath, backupRepoType, backupRepoAddress, volumePath, diskType, backupFile, new Pair<>(vmName, command.getVmState()), mountOptions); } else if (Boolean.TRUE.equals(vmExists)) { - restoreVolumesOfExistingVM(volumePaths, backupPath, backupRepoType, backupRepoAddress, mountOptions); + restoreVolumesOfExistingVM(volumePaths, backupPath, backupFiles, backupRepoType, backupRepoAddress, mountOptions); } else { - restoreVolumesOfDestroyedVMs(volumePaths, vmName, backupPath, backupRepoType, backupRepoAddress, mountOptions); + restoreVolumesOfDestroyedVMs(volumePaths, vmName, backupPath, backupFiles, backupRepoType, backupRepoAddress, mountOptions); } } catch (CloudRuntimeException e) { String errorMessage = "Failed to restore backup for VM: " + vmName + "."; @@ -86,17 +86,18 @@ public class LibvirtRestoreBackupCommandWrapper extends CommandWrapper volumePaths, String backupPath, + private void restoreVolumesOfExistingVM(List volumePaths, String backupPath, List backupFiles, String backupRepoType, String backupRepoAddress, String mountOptions) { String diskType = "root"; String mountDirectory = mountBackupDirectory(backupRepoAddress, backupRepoType, mountOptions); try { for (int idx = 0; idx < volumePaths.size(); idx++) { String volumePath = volumePaths.get(idx); - Pair bkpPathAndVolUuid = getBackupPath(mountDirectory, volumePath, backupPath, diskType, null); + String backupFile = backupFiles.get(idx); + String bkpPath = getBackupPath(mountDirectory, backupPath, backupFile, diskType); diskType = "datadisk"; - if (!replaceVolumeWithBackup(volumePath, bkpPathAndVolUuid.first())) { - throw new CloudRuntimeException(String.format("Unable to restore backup for volume [%s].", bkpPathAndVolUuid.second())); + if (!replaceVolumeWithBackup(volumePath, bkpPath)) { + throw new CloudRuntimeException(String.format("Unable to restore backup from volume [%s].", volumePath)); } } } finally { @@ -106,17 +107,18 @@ public class LibvirtRestoreBackupCommandWrapper extends CommandWrapper volumePaths, String vmName, String backupPath, + private void restoreVolumesOfDestroyedVMs(List volumePaths, String vmName, String backupPath, List backupFiles, String backupRepoType, String backupRepoAddress, String mountOptions) { String mountDirectory = mountBackupDirectory(backupRepoAddress, backupRepoType, mountOptions); String diskType = "root"; try { - for (int i = 0; i < volumePaths.size(); i++) { - String volumePath = volumePaths.get(i); - Pair bkpPathAndVolUuid = getBackupPath(mountDirectory, volumePath, backupPath, diskType, null); + for (int idx = 0; idx < volumePaths.size(); idx++) { + String volumePath = volumePaths.get(idx); + String backupFile = backupFiles.get(idx); + String bkpPath = getBackupPath(mountDirectory, backupPath, backupFile, diskType); diskType = "datadisk"; - if (!replaceVolumeWithBackup(volumePath, bkpPathAndVolUuid.first())) { - throw new CloudRuntimeException(String.format("Unable to restore backup for volume [%s].", bkpPathAndVolUuid.second())); + if (!replaceVolumeWithBackup(volumePath, bkpPath)) { + throw new CloudRuntimeException(String.format("Unable to restore backup from volume [%s].", volumePath)); } } } finally { @@ -126,13 +128,13 @@ public class LibvirtRestoreBackupCommandWrapper extends CommandWrapper vmNameAndState, String mountOptions) { + String diskType, String backupFile, Pair vmNameAndState, String mountOptions) { String mountDirectory = mountBackupDirectory(backupRepoAddress, backupRepoType, mountOptions); - Pair bkpPathAndVolUuid; + String bkpPath; try { - bkpPathAndVolUuid = getBackupPath(mountDirectory, volumePath, backupPath, diskType, volumeUUID); - if (!replaceVolumeWithBackup(volumePath, bkpPathAndVolUuid.first())) { - throw new CloudRuntimeException(String.format("Unable to restore backup for volume [%s].", bkpPathAndVolUuid.second())); + bkpPath = getBackupPath(mountDirectory, backupPath, backupFile, diskType); + if (!replaceVolumeWithBackup(volumePath, bkpPath)) { + throw new CloudRuntimeException(String.format("Unable to restore backup from volume [%s].", volumePath)); } if (VirtualMachine.State.Running.equals(vmNameAndState.second())) { if (!attachVolumeToVm(vmNameAndState.first(), volumePath)) { @@ -188,13 +190,11 @@ public class LibvirtRestoreBackupCommandWrapper extends CommandWrapper getBackupPath(String mountDirectory, String volumePath, String backupPath, String diskType, String volumeUuid) { + private String getBackupPath(String mountDirectory, String backupPath, String backupFile, String diskType) { String bkpPath = String.format(FILE_PATH_PLACEHOLDER, mountDirectory, backupPath); - int lastIndex = volumePath.lastIndexOf(File.separator); - String volUuid = Objects.isNull(volumeUuid) ? volumePath.substring(lastIndex + 1) : volumeUuid; - String backupFileName = String.format("%s.%s.qcow2", diskType.toLowerCase(Locale.ROOT), volUuid); + String backupFileName = String.format("%s.%s.qcow2", diskType.toLowerCase(Locale.ROOT), backupFile); bkpPath = String.format(FILE_PATH_PLACEHOLDER, bkpPath, backupFileName); - return new Pair<>(bkpPath, volUuid); + return bkpPath; } private boolean replaceVolumeWithBackup(String volumePath, String backupPath) { diff --git a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixModifyStoragePoolCommandWrapper.java b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixModifyStoragePoolCommandWrapper.java index 63cb675c614..2878998d622 100644 --- a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixModifyStoragePoolCommandWrapper.java +++ b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixModifyStoragePoolCommandWrapper.java @@ -59,7 +59,7 @@ public final class CitrixModifyStoragePoolCommandWrapper extends CommandWrapper< if (capacity == -1) { final String msg = "Pool capacity is -1! pool: " + pool.getHost() + pool.getPath(); logger.warn(msg); - return new Answer(command, false, msg); + return new ModifyStoragePoolAnswer(command, false, msg); } final Map tInfo = new HashMap(); final ModifyStoragePoolAnswer answer = new ModifyStoragePoolAnswer(command, capacity, available, tInfo); @@ -68,12 +68,12 @@ public final class CitrixModifyStoragePoolCommandWrapper extends CommandWrapper< final String msg = "ModifyStoragePoolCommand add XenAPIException:" + e.toString() + " host:" + citrixResourceBase.getHost().getUuid() + " pool: " + pool.getHost() + pool.getPath(); logger.warn(msg, e); - return new Answer(command, false, msg); + return new ModifyStoragePoolAnswer(command, false, msg); } catch (final Exception e) { final String msg = "ModifyStoragePoolCommand add XenAPIException:" + e.getMessage() + " host:" + citrixResourceBase.getHost().getUuid() + " pool: " + pool.getHost() + pool.getPath(); logger.warn(msg, e); - return new Answer(command, false, msg); + return new ModifyStoragePoolAnswer(command, false, msg); } } else { try { @@ -85,17 +85,17 @@ public final class CitrixModifyStoragePoolCommandWrapper extends CommandWrapper< if (result == null || !result.split("#")[1].equals("0")) { throw new CloudRuntimeException("Unable to remove heartbeat file entry for SR " + srUuid + " due to " + result); } - return new Answer(command, true, "success"); + return new ModifyStoragePoolAnswer(command, true, "success"); } catch (final XenAPIException e) { final String msg = "ModifyStoragePoolCommand remove XenAPIException:" + e.toString() + " host:" + citrixResourceBase.getHost().getUuid() + " pool: " + pool.getHost() + pool.getPath(); logger.warn(msg, e); - return new Answer(command, false, msg); + return new ModifyStoragePoolAnswer(command, false, msg); } catch (final Exception e) { final String msg = "ModifyStoragePoolCommand remove XenAPIException:" + e.getMessage() + " host:" + citrixResourceBase.getHost().getUuid() + " pool: " + pool.getHost() + pool.getPath(); logger.warn(msg, e); - return new Answer(command, false, msg); + return new ModifyStoragePoolAnswer(command, false, msg); } } } diff --git a/plugins/integrations/prometheus/src/main/java/org/apache/cloudstack/metrics/PrometheusExporterImpl.java b/plugins/integrations/prometheus/src/main/java/org/apache/cloudstack/metrics/PrometheusExporterImpl.java index 32ec2f53211..b49f11c7774 100644 --- a/plugins/integrations/prometheus/src/main/java/org/apache/cloudstack/metrics/PrometheusExporterImpl.java +++ b/plugins/integrations/prometheus/src/main/java/org/apache/cloudstack/metrics/PrometheusExporterImpl.java @@ -17,6 +17,7 @@ package org.apache.cloudstack.metrics; import java.math.BigDecimal; +import java.security.cert.X509Certificate; import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; @@ -26,6 +27,7 @@ import java.util.stream.Collectors; import javax.inject.Inject; +import org.apache.cloudstack.ca.CAManager; import org.apache.cloudstack.engine.subsystem.api.storage.ZoneScope; import org.apache.cloudstack.storage.datastore.db.ImageStoreDao; import org.apache.commons.lang3.StringUtils; @@ -133,6 +135,8 @@ public class PrometheusExporterImpl extends ManagerBase implements PrometheusExp private ResourceCountDao _resourceCountDao; @Inject private HostTagsDao _hostTagsDao; + @Inject + private CAManager caManager; public PrometheusExporterImpl() { super(); @@ -216,6 +220,9 @@ public class PrometheusExporterImpl extends ManagerBase implements PrometheusExp } metricsList.add(new ItemHostVM(zoneName, zoneUuid, host.getName(), host.getUuid(), host.getPrivateIpAddress(), vmDao.listByHostId(host.getId()).size())); + + addSSLCertificateExpirationMetrics(metricsList, zoneName, zoneUuid, host); + final CapacityVO coreCapacity = capacityDao.findByHostIdType(host.getId(), Capacity.CAPACITY_TYPE_CPU_CORE); if (coreCapacity == null && !host.isInMaintenanceStates()){ @@ -253,6 +260,18 @@ public class PrometheusExporterImpl extends ManagerBase implements PrometheusExp addHostTagsMetrics(metricsList, dcId, zoneName, zoneUuid, totalHosts, upHosts, downHosts, total, up, down); } + private void addSSLCertificateExpirationMetrics(List metricsList, String zoneName, String zoneUuid, HostVO host) { + if (caManager == null || caManager.getActiveCertificatesMap() == null) { + return; + } + X509Certificate cert = caManager.getActiveCertificatesMap().getOrDefault(host.getPrivateIpAddress(), null); + if (cert == null) { + return; + } + long certExpiryEpoch = cert.getNotAfter().getTime() / 1000; // Convert to epoch seconds + metricsList.add(new ItemHostCertExpiry(zoneName, zoneUuid, host.getName(), host.getUuid(), host.getPrivateIpAddress(), certExpiryEpoch)); + } + private String markTagMaps(HostVO host, Map totalHosts, Map upHosts, Map downHosts) { List hostTagVOS = _hostTagsDao.getHostTags(host.getId()); List hostTags = new ArrayList<>(); @@ -1049,4 +1068,28 @@ public class PrometheusExporterImpl extends ManagerBase implements PrometheusExp return String.format("%s{zone=\"%s\",cpu=\"%d\",memory=\"%d\"} %d", name, zoneName, cpu, memory, total); } } + + class ItemHostCertExpiry extends Item { + String zoneName; + String zoneUuid; + String hostName; + String hostUuid; + String hostIp; + long expiryTimestamp; + + public ItemHostCertExpiry(final String zoneName, final String zoneUuid, final String hostName, final String hostUuid, final String hostIp, final long expiry) { + super("cloudstack_host_cert_expiry_timestamp"); + this.zoneName = zoneName; + this.zoneUuid = zoneUuid; + this.hostName = hostName; + this.hostUuid = hostUuid; + this.hostIp = hostIp; + this.expiryTimestamp = expiry; + } + + @Override + public String toMetricsString() { + return String.format("%s{zone=\"%s\",hostname=\"%s\",ip=\"%s\"} %d", name, zoneName, hostName, hostIp, expiryTimestamp); + } + } } diff --git a/plugins/integrations/prometheus/src/test/java/org/apache/cloudstack/metrics/PrometheusExporterImplTest.java b/plugins/integrations/prometheus/src/test/java/org/apache/cloudstack/metrics/PrometheusExporterImplTest.java new file mode 100644 index 00000000000..40490c46f56 --- /dev/null +++ b/plugins/integrations/prometheus/src/test/java/org/apache/cloudstack/metrics/PrometheusExporterImplTest.java @@ -0,0 +1,108 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package org.apache.cloudstack.metrics; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +import org.junit.Test; + +public class PrometheusExporterImplTest { + + private static final String TEST_ZONE_NAME = "zone1"; + private static final String TEST_ZONE_UUID = "zone-uuid-1"; + private static final String TEST_HOST_NAME = "host1"; + private static final String TEST_HOST_UUID = "host-uuid-1"; + private static final String TEST_HOST_IP = "192.168.1.10"; + private static final long CERT_EXPIRY_TIME = 1735689600000L; // 2025-01-01 00:00:00 UTC + private static final long CERT_EXPIRY_EPOCH = CERT_EXPIRY_TIME / 1000; + + @Test + public void testItemHostCertExpiryFormat() { + PrometheusExporterImpl exporter = new PrometheusExporterImpl(); + PrometheusExporterImpl.ItemHostCertExpiry item = exporter.new ItemHostCertExpiry( + TEST_ZONE_NAME, + TEST_ZONE_UUID, + TEST_HOST_NAME, + TEST_HOST_UUID, + TEST_HOST_IP, + CERT_EXPIRY_EPOCH + ); + + String metricsString = item.toMetricsString(); + String expected = String.format( + "cloudstack_host_cert_expiry_timestamp{zone=\"%s\",hostname=\"%s\",ip=\"%s\"} %d", + TEST_ZONE_NAME, + TEST_HOST_NAME, + TEST_HOST_IP, + CERT_EXPIRY_EPOCH + ); + assertEquals("Certificate expiry metric format should match expected format", expected, metricsString); + } + + @Test + public void testItemHostCertExpiryContainsCorrectMetricName() { + PrometheusExporterImpl exporter = new PrometheusExporterImpl(); + PrometheusExporterImpl.ItemHostCertExpiry item = exporter.new ItemHostCertExpiry( + TEST_ZONE_NAME, + TEST_ZONE_UUID, + TEST_HOST_NAME, + TEST_HOST_UUID, + TEST_HOST_IP, + CERT_EXPIRY_EPOCH + ); + + String metricsString = item.toMetricsString(); + assertTrue("Metric should contain correct metric name", + metricsString.contains("cloudstack_host_cert_expiry_timestamp")); + } + + @Test + public void testItemHostCertExpiryContainsAllLabels() { + PrometheusExporterImpl exporter = new PrometheusExporterImpl(); + PrometheusExporterImpl.ItemHostCertExpiry item = exporter.new ItemHostCertExpiry( + TEST_ZONE_NAME, + TEST_ZONE_UUID, + TEST_HOST_NAME, + TEST_HOST_UUID, + TEST_HOST_IP, + CERT_EXPIRY_EPOCH + ); + + String metricsString = item.toMetricsString(); + assertTrue("Metric should contain zone label", metricsString.contains("zone=\"" + TEST_ZONE_NAME + "\"")); + assertTrue("Metric should contain hostname label", metricsString.contains("hostname=\"" + TEST_HOST_NAME + "\"")); + assertTrue("Metric should contain ip label", metricsString.contains("ip=\"" + TEST_HOST_IP + "\"")); + } + + @Test + public void testItemHostCertExpiryContainsTimestampValue() { + PrometheusExporterImpl exporter = new PrometheusExporterImpl(); + PrometheusExporterImpl.ItemHostCertExpiry item = exporter.new ItemHostCertExpiry( + TEST_ZONE_NAME, + TEST_ZONE_UUID, + TEST_HOST_NAME, + TEST_HOST_UUID, + TEST_HOST_IP, + CERT_EXPIRY_EPOCH + ); + + String metricsString = item.toMetricsString(); + assertTrue("Metric should contain correct timestamp value", + metricsString.endsWith(" " + CERT_EXPIRY_EPOCH)); + } +} diff --git a/plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/api/command/OauthLoginAPIAuthenticatorCmd.java b/plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/api/command/OauthLoginAPIAuthenticatorCmd.java index f9a1d10d352..e273358e83e 100644 --- a/plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/api/command/OauthLoginAPIAuthenticatorCmd.java +++ b/plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/api/command/OauthLoginAPIAuthenticatorCmd.java @@ -176,18 +176,14 @@ public class OauthLoginAPIAuthenticatorCmd extends BaseCmd implements APIAuthent } protected Long getDomainIdFromParams(Map params, StringBuilder auditTrailSb, String responseType) { - String[] domainIdArr = (String[])params.get(ApiConstants.DOMAIN_ID); - - if (domainIdArr == null) { - domainIdArr = (String[])params.get(ApiConstants.DOMAIN__ID); - } + String domainIdStr = _apiServer.getDomainId(params); Long domainId = null; - if ((domainIdArr != null) && (domainIdArr.length > 0)) { + if (StringUtils.isNotEmpty(domainIdStr)) { try { //check if UUID is passed in for domain - domainId = _apiServer.fetchDomainId(domainIdArr[0]); + domainId = _apiServer.fetchDomainId(domainIdStr); if (domainId == null) { - domainId = Long.parseLong(domainIdArr[0]); + domainId = Long.parseLong(domainIdStr); } auditTrailSb.append(" domainid=" + domainId);// building the params for POST call } catch (final NumberFormatException e) { diff --git a/plugins/user-authenticators/oauth2/src/test/java/org/apache/cloudstack/oauth2/api/command/OauthLoginAPIAuthenticatorCmdTest.java b/plugins/user-authenticators/oauth2/src/test/java/org/apache/cloudstack/oauth2/api/command/OauthLoginAPIAuthenticatorCmdTest.java index ccbb53cfc80..962ffefd5ce 100644 --- a/plugins/user-authenticators/oauth2/src/test/java/org/apache/cloudstack/oauth2/api/command/OauthLoginAPIAuthenticatorCmdTest.java +++ b/plugins/user-authenticators/oauth2/src/test/java/org/apache/cloudstack/oauth2/api/command/OauthLoginAPIAuthenticatorCmdTest.java @@ -85,10 +85,29 @@ public class OauthLoginAPIAuthenticatorCmdTest { ApiServer apiServer = mock(ApiServer.class); cmd._apiServer = apiServer; when(apiServer.fetchDomainId("1234")).thenReturn(5678L); + when(apiServer.getDomainId(params)).thenCallRealMethod(); Long domainId = cmd.getDomainIdFromParams(params, auditTrailSb, responseType); assertEquals(Long.valueOf(5678), domainId); assertEquals(" domainid=5678", auditTrailSb.toString()); } + + @Test + public void testGetDomainIdFromCamelCaseParam() { + StringBuilder auditTrailSb = new StringBuilder(); + String responseType = "json"; + Map params = new HashMap<>(); + params.put(ApiConstants.DOMAIN_ID, null); + params.put(ApiConstants.DOMAIN__ID, new String[]{"5678"}); + ApiServer apiServer = mock(ApiServer.class); + cmd._apiServer = apiServer; + when(apiServer.fetchDomainId("5678")).thenReturn(1234L); + when(apiServer.getDomainId(params)).thenCallRealMethod(); + + Long domainId = cmd.getDomainIdFromParams(params, auditTrailSb, responseType); + + assertEquals(Long.valueOf(1234), domainId); + assertEquals(" domainid=1234", auditTrailSb.toString()); + } } diff --git a/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmd.java b/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmd.java index bfd47922142..584f2463754 100644 --- a/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmd.java +++ b/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmd.java @@ -158,11 +158,17 @@ public class SAML2LoginAPIAuthenticatorCmd extends BaseCmd implements APIAuthent String domainPath = null; if (params.containsKey(ApiConstants.IDP_ID)) { - idpId = ((String[])params.get(ApiConstants.IDP_ID))[0]; + String[] idpIds = (String[])params.get(ApiConstants.IDP_ID); + if (idpIds != null && idpIds.length > 0) { + idpId = idpIds[0]; + } } if (params.containsKey(ApiConstants.DOMAIN)) { - domainPath = ((String[])params.get(ApiConstants.DOMAIN))[0]; + String[] domainPaths = (String[])params.get(ApiConstants.DOMAIN); + if (domainPaths != null && domainPaths.length > 0) { + domainPath = domainPaths[0]; + } } if (domainPath != null && !domainPath.isEmpty()) { diff --git a/pom.xml b/pom.xml index 6985108302d..e57d6dfc46a 100644 --- a/pom.xml +++ b/pom.xml @@ -63,20 +63,20 @@ 1.8 3.0.0 - 3.1.0 + 3.6.0 0.8.11 3.8.1 - 3.1.1 + 3.9.0 2.22.2 3.1.12 3.1.12.2 3.2.0 - 3.12.0 + 3.28.0 3.0.0 7.4.4 2.5.3 3.1.0 - 3.8.2 + 3.21.0 2.22.2 4.4.1 3.2.0 diff --git a/server/src/main/java/com/cloud/api/ApiResponseHelper.java b/server/src/main/java/com/cloud/api/ApiResponseHelper.java index 3331053dff2..2b7ed1b1100 100644 --- a/server/src/main/java/com/cloud/api/ApiResponseHelper.java +++ b/server/src/main/java/com/cloud/api/ApiResponseHelper.java @@ -4364,7 +4364,7 @@ public class ApiResponseHelper implements ResponseGenerator { } else if (usageRecord.getUsageType() == UsageTypes.BACKUP) { resourceType = ResourceObjectType.Backup; final StringBuilder builder = new StringBuilder(); - builder.append("Backup usage of size ").append(usageRecord.getUsageDisplay()); + builder.append("Backup usage"); if (vmInstance != null) { resourceId = vmInstance.getId(); usageRecResponse.setResourceName(vmInstance.getInstanceName()); @@ -4377,9 +4377,12 @@ public class ApiResponseHelper implements ResponseGenerator { .append(" (").append(backupOffering.getUuid()).append(", user ad-hoc/scheduled backup allowed: ") .append(backupOffering.isUserDrivenBackupAllowed()).append(")"); } - } + builder.append(" with size ").append(toHumanReadableSize(usageRecord.getSize())); + builder.append(" and with virtual size ").append(toHumanReadableSize(usageRecord.getVirtualSize())); usageRecResponse.setDescription(builder.toString()); + usageRecResponse.setSize(usageRecord.getSize()); + usageRecResponse.setVirtualSize(usageRecord.getVirtualSize()); } else if (usageRecord.getUsageType() == UsageTypes.VM_SNAPSHOT) { resourceType = ResourceObjectType.VMSnapshot; VMSnapshotVO vmSnapshotVO = null; diff --git a/server/src/main/java/com/cloud/api/ApiServer.java b/server/src/main/java/com/cloud/api/ApiServer.java index 85d58ec0d53..3a89c7bde23 100644 --- a/server/src/main/java/com/cloud/api/ApiServer.java +++ b/server/src/main/java/com/cloud/api/ApiServer.java @@ -115,6 +115,7 @@ import org.apache.cloudstack.framework.messagebus.MessageHandler; import org.apache.cloudstack.managed.context.ManagedContextRunnable; import org.apache.cloudstack.user.UserPasswordResetManager; import org.apache.commons.codec.binary.Base64; +import org.apache.commons.collections.MapUtils; import org.apache.commons.lang3.EnumUtils; import org.apache.http.ConnectionClosedException; import org.apache.http.HttpException; @@ -1354,6 +1355,25 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer return userPasswordResetManager.validateAndResetPassword(userAccount, token, password); } + @Override + public String getDomainId(Map params) { + if (MapUtils.isEmpty(params)) { + return null; + } + + String[] domainIdArr = (String[])params.get(ApiConstants.DOMAIN_ID); + if (domainIdArr == null) { + // Fallback to support clients using the camelCase parameter name "domainId" + domainIdArr = (String[])params.get(ApiConstants.DOMAIN__ID); + } + + if (domainIdArr == null || domainIdArr.length == 0) { + return null; + } + + return domainIdArr[0]; + } + private void checkCommandAvailable(final User user, final String commandName, final InetAddress remoteAddress) throws PermissionDeniedException { if (user == null) { throw new PermissionDeniedException("User is null for role based API access check for command" + commandName); diff --git a/server/src/main/java/com/cloud/api/ApiServlet.java b/server/src/main/java/com/cloud/api/ApiServlet.java index 4994c42bb4d..01cb21681b0 100644 --- a/server/src/main/java/com/cloud/api/ApiServlet.java +++ b/server/src/main/java/com/cloud/api/ApiServlet.java @@ -34,6 +34,7 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpSession; +import com.cloud.api.auth.DefaultForgotPasswordAPIAuthenticatorCmd; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.ApiErrorCode; import org.apache.cloudstack.api.ApiServerService; @@ -164,7 +165,6 @@ public class ApiServlet extends HttpServlet { LOGGER.warn(message); } }); - } void processRequestInContext(final HttpServletRequest req, final HttpServletResponse resp) { @@ -226,7 +226,6 @@ public class ApiServlet extends HttpServlet { } if (command != null && !command.equals(ValidateUserTwoFactorAuthenticationCodeCmd.APINAME)) { - APIAuthenticator apiAuthenticator = authManager.getAPIAuthenticator(command); if (apiAuthenticator != null) { auditTrailSb.append("command="); @@ -262,7 +261,9 @@ public class ApiServlet extends HttpServlet { } catch (ServerApiException e) { httpResponseCode = e.getErrorCode().getHttpCode(); responseString = e.getMessage(); - LOGGER.debug("Authentication failure: " + e.getMessage()); + if (!DefaultForgotPasswordAPIAuthenticatorCmd.APINAME.equalsIgnoreCase(command) || StringUtils.isNotBlank(username)) { + LOGGER.debug("Authentication failure: {}", e.getMessage()); + } } if (apiAuthenticator.getAPIType() == APIAuthenticationType.LOGOUT_API) { @@ -330,7 +331,7 @@ public class ApiServlet extends HttpServlet { } } - if (! requestChecksoutAsSane(resp, auditTrailSb, responseType, params, session, command, userId, account, accountObj)) + if (!requestChecksoutAsSane(resp, auditTrailSb, responseType, params, session, command, userId, account, accountObj)) return; } else { CallContext.register(accountMgr.getSystemUser(), accountMgr.getSystemAccount()); @@ -360,7 +361,6 @@ public class ApiServlet extends HttpServlet { apiServer.getSerializedApiError(HttpServletResponse.SC_UNAUTHORIZED, "unable to verify user credentials and/or request signature", params, responseType); HttpUtils.writeHttpResponse(resp, serializedResponse, HttpServletResponse.SC_UNAUTHORIZED, responseType, ApiServer.JSONcontentType.value()); - } } catch (final ServerApiException se) { final String serializedResponseText = apiServer.getSerializedApiError(se, params, responseType); @@ -550,6 +550,9 @@ public class ApiServlet extends HttpServlet { if (LOGGER.isTraceEnabled()) { LOGGER.trace(msg); } + if (session == null) { + return; + } session.invalidate(); } catch (final IllegalStateException ise) { if (LOGGER.isTraceEnabled()) { diff --git a/server/src/main/java/com/cloud/api/auth/DefaultForgotPasswordAPIAuthenticatorCmd.java b/server/src/main/java/com/cloud/api/auth/DefaultForgotPasswordAPIAuthenticatorCmd.java index 1e90b43c5e8..46a9dd9bfe3 100644 --- a/server/src/main/java/com/cloud/api/auth/DefaultForgotPasswordAPIAuthenticatorCmd.java +++ b/server/src/main/java/com/cloud/api/auth/DefaultForgotPasswordAPIAuthenticatorCmd.java @@ -44,13 +44,13 @@ import java.net.InetAddress; import java.util.List; import java.util.Map; -@APICommand(name = "forgotPassword", +@APICommand(name = DefaultForgotPasswordAPIAuthenticatorCmd.APINAME, description = "Sends an email to the user with a token to reset the password using resetPassword command.", since = "4.20.0.0", requestHasSensitiveInfo = true, responseObject = SuccessResponse.class) public class DefaultForgotPasswordAPIAuthenticatorCmd extends BaseCmd implements APIAuthenticator { - + public static final String APINAME = "forgotPassword"; ///////////////////////////////////////////////////// //////////////// API parameters ///////////////////// @@ -108,10 +108,12 @@ public class DefaultForgotPasswordAPIAuthenticatorCmd extends BaseCmd implements if (userDomain != null) { domainId = userDomain.getId(); } else { + logger.debug("Unable to find the domain from the path {}", domain); throw new ServerApiException(ApiErrorCode.PARAM_ERROR, String.format("Unable to find the domain from the path %s", domain)); } final UserAccount userAccount = _accountService.getActiveUserAccount(username[0], domainId); if (userAccount != null && List.of(User.Source.SAML2, User.Source.OAUTH2, User.Source.LDAP).contains(userAccount.getSource())) { + logger.debug("Forgot Password is not allowed for the user {} from source {}", username[0], userAccount.getSource()); throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Forgot Password is not allowed for this user"); } boolean success = _apiServer.forgotPassword(userAccount, userDomain); diff --git a/server/src/main/java/com/cloud/api/auth/DefaultLoginAPIAuthenticatorCmd.java b/server/src/main/java/com/cloud/api/auth/DefaultLoginAPIAuthenticatorCmd.java index c9b03a85f4c..41174c37be2 100644 --- a/server/src/main/java/com/cloud/api/auth/DefaultLoginAPIAuthenticatorCmd.java +++ b/server/src/main/java/com/cloud/api/auth/DefaultLoginAPIAuthenticatorCmd.java @@ -20,6 +20,7 @@ import com.cloud.api.ApiServlet; import com.cloud.domain.Domain; import com.cloud.user.User; import com.cloud.user.UserAccount; +import com.cloud.utils.StringUtils; import org.apache.cloudstack.api.ApiServerService; import com.cloud.api.response.ApiResponseSerializer; import com.cloud.exception.CloudAuthenticationException; @@ -47,7 +48,6 @@ import java.net.InetAddress; @APICommand(name = "login", description = "Logs a user into the CloudStack. A successful login attempt will generate a JSESSIONID cookie value that can be passed in subsequent Query command calls until the \"logout\" command has been issued or the session has expired.", requestHasSensitiveInfo = true, responseObject = LoginCmdResponse.class, entityType = {}) public class DefaultLoginAPIAuthenticatorCmd extends BaseCmd implements APIAuthenticator { - ///////////////////////////////////////////////////// //////////////// API parameters ///////////////////// ///////////////////////////////////////////////////// @@ -107,22 +107,18 @@ public class DefaultLoginAPIAuthenticatorCmd extends BaseCmd implements APIAuthe if (HTTPMethod.valueOf(req.getMethod()) != HTTPMethod.POST) { throw new ServerApiException(ApiErrorCode.METHOD_NOT_ALLOWED, "Please use HTTP POST to authenticate using this API"); } + // FIXME: ported from ApiServlet, refactor and cleanup final String[] username = (String[])params.get(ApiConstants.USERNAME); final String[] password = (String[])params.get(ApiConstants.PASSWORD); - String[] domainIdArr = (String[])params.get(ApiConstants.DOMAIN_ID); - - if (domainIdArr == null) { - domainIdArr = (String[])params.get(ApiConstants.DOMAIN__ID); - } - final String[] domainName = (String[])params.get(ApiConstants.DOMAIN); + String domainIdStr = _apiServer.getDomainId(params); Long domainId = null; - if ((domainIdArr != null) && (domainIdArr.length > 0)) { + if (StringUtils.isNotEmpty(domainIdStr)) { try { //check if UUID is passed in for domain - domainId = _apiServer.fetchDomainId(domainIdArr[0]); + domainId = _apiServer.fetchDomainId(domainIdStr); if (domainId == null) { - domainId = Long.parseLong(domainIdArr[0]); + domainId = Long.parseLong(domainIdStr); } auditTrailSb.append(" domainid=" + domainId);// building the params for POST call } catch (final NumberFormatException e) { @@ -135,6 +131,7 @@ public class DefaultLoginAPIAuthenticatorCmd extends BaseCmd implements APIAuthe } String domain = null; + final String[] domainName = (String[])params.get(ApiConstants.DOMAIN); domain = getDomainName(auditTrailSb, domainName, domain); String serializedResponse = null; diff --git a/server/src/main/java/com/cloud/api/auth/DefaultResetPasswordAPIAuthenticatorCmd.java b/server/src/main/java/com/cloud/api/auth/DefaultResetPasswordAPIAuthenticatorCmd.java index 077efdee087..810b5ebefcf 100644 --- a/server/src/main/java/com/cloud/api/auth/DefaultResetPasswordAPIAuthenticatorCmd.java +++ b/server/src/main/java/com/cloud/api/auth/DefaultResetPasswordAPIAuthenticatorCmd.java @@ -53,7 +53,6 @@ import java.util.Map; responseObject = SuccessResponse.class) public class DefaultResetPasswordAPIAuthenticatorCmd extends BaseCmd implements APIAuthenticator { - ///////////////////////////////////////////////////// //////////////// API parameters ///////////////////// ///////////////////////////////////////////////////// diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java index 88d64198559..45c98b35fb5 100644 --- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java @@ -107,6 +107,7 @@ import org.apache.cloudstack.engine.subsystem.api.storage.ZoneScope; import org.apache.cloudstack.framework.config.ConfigDepot; import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.cloudstack.framework.config.Configurable; +import org.apache.cloudstack.framework.config.ValidatedConfigKey; import org.apache.cloudstack.framework.config.dao.ConfigurationDao; import org.apache.cloudstack.framework.config.dao.ConfigurationGroupDao; import org.apache.cloudstack.framework.config.dao.ConfigurationSubGroupDao; @@ -716,6 +717,12 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati throw new InvalidParameterValueException(validationMsg); } + ConfigKey configKey = _configDepot.get(name); + if (configKey instanceof ValidatedConfigKey) { + ValidatedConfigKey validatedConfigKey = (ValidatedConfigKey) configKey; + validatedConfigKey.validateValue(value); + } + // If scope of the parameter is given then it needs to be updated in the // corresponding details table, // if scope is mentioned as global or not mentioned then it is normal diff --git a/server/src/main/java/com/cloud/hypervisor/kvm/discoverer/LibvirtServerDiscoverer.java b/server/src/main/java/com/cloud/hypervisor/kvm/discoverer/LibvirtServerDiscoverer.java index 8f7bf21dff2..da3ca47ae9c 100644 --- a/server/src/main/java/com/cloud/hypervisor/kvm/discoverer/LibvirtServerDiscoverer.java +++ b/server/src/main/java/com/cloud/hypervisor/kvm/discoverer/LibvirtServerDiscoverer.java @@ -272,7 +272,12 @@ public abstract class LibvirtServerDiscoverer extends DiscovererBase implements } } - sshConnection = new Connection(agentIp, 22); + int port = uri.getPort(); + if (port <= 0) { + port = AgentManager.KVMHostDiscoverySshPort.valueIn(clusterId); + } + + sshConnection = new Connection(agentIp, port); sshConnection.connect(null, 60000, 60000); @@ -380,6 +385,9 @@ public abstract class LibvirtServerDiscoverer extends DiscovererBase implements Map hostDetails = connectedHost.getDetails(); hostDetails.put("password", password); hostDetails.put("username", username); + if (uri.getPort() > 0) { + hostDetails.put(Host.HOST_SSH_PORT, String.valueOf(uri.getPort())); + } _hostDao.saveDetails(connectedHost); return resources; } catch (DiscoveredWithErrorException e) { diff --git a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java index 12ceac21322..cc789bf5650 100755 --- a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java +++ b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java @@ -776,7 +776,6 @@ public class ResourceManagerImpl extends ManagerBase implements ResourceManager, _clusterDetailsDao.persist(cluster_cpu_detail); _clusterDetailsDao.persist(cluster_memory_detail); } - } try { @@ -871,7 +870,6 @@ public class ResourceManagerImpl extends ManagerBase implements ResourceManager, hosts.add(host); } discoverer.postDiscovery(hosts, _nodeId); - } logger.info("server resources successfully discovered by " + discoverer.getName()); return hosts; @@ -2261,15 +2259,26 @@ public class ResourceManagerImpl extends ManagerBase implements ResourceManager, private HostVO getNewHost(StartupCommand[] startupCommands) { StartupCommand startupCommand = startupCommands[0]; - HostVO host = findHostByGuid(startupCommand.getGuid()); + String fullGuid = startupCommand.getGuid(); + logger.debug(String.format("Trying to find Host by guid %s", fullGuid)); + HostVO host = findHostByGuid(fullGuid); if (host != null) { + logger.debug(String.format("Found Host by guid %s: %s", fullGuid, host)); return host; } - host = findHostByGuid(startupCommand.getGuidWithoutResource()); + String guidPrefix = startupCommand.getGuidWithoutResource(); + logger.debug(String.format("Trying to find Host by guid prefix %s", guidPrefix)); + host = findHostByGuidPrefix(guidPrefix); - return host; // even when host == null! + if (host != null) { + logger.debug(String.format("Found Host by guid prefix %s: %s", guidPrefix, host)); + return host; + } + + logger.debug(String.format("Could not find Host by guid %s", fullGuid)); + return null; } protected HostVO createHostVO(final StartupCommand[] cmds, final ServerResource resource, final Map details, List hostTags, @@ -2949,7 +2958,7 @@ public class ResourceManagerImpl extends ManagerBase implements ResourceManager, */ protected void connectAndRestartAgentOnHost(HostVO host, String username, String password, String privateKey) { final com.trilead.ssh2.Connection connection = SSHCmdHelper.acquireAuthorizedConnection( - host.getPrivateIpAddress(), 22, username, password, privateKey); + host.getPrivateIpAddress(), _agentMgr.getHostSshPort(host), username, password, privateKey); if (connection == null) { throw new CloudRuntimeException(String.format("SSH to agent is enabled, but failed to connect to %s via IP address [%s].", host, host.getPrivateIpAddress())); } @@ -3296,6 +3305,15 @@ public class ResourceManagerImpl extends ManagerBase implements ResourceManager, public HostVO findHostByGuid(final String guid) { final QueryBuilder sc = QueryBuilder.create(HostVO.class); sc.and(sc.entity().getGuid(), Op.EQ, guid); + sc.and(sc.entity().getRemoved(), Op.NULL); + return sc.find(); + } + + @Override + public HostVO findHostByGuidPrefix(String guid) { + final QueryBuilder sc = QueryBuilder.create(HostVO.class); + sc.and(sc.entity().getGuid(), Op.LIKE, guid + "%"); + sc.and(sc.entity().getRemoved(), Op.NULL); return sc.find(); } @@ -3303,6 +3321,7 @@ public class ResourceManagerImpl extends ManagerBase implements ResourceManager, public HostVO findHostByName(final String name) { final QueryBuilder sc = QueryBuilder.create(HostVO.class); sc.and(sc.entity().getName(), Op.EQ, name); + sc.and(sc.entity().getRemoved(), Op.NULL); return sc.find(); } diff --git a/server/src/main/java/com/cloud/server/ManagementServerImpl.java b/server/src/main/java/com/cloud/server/ManagementServerImpl.java index 347b9b26ac4..71913467ced 100644 --- a/server/src/main/java/com/cloud/server/ManagementServerImpl.java +++ b/server/src/main/java/com/cloud/server/ManagementServerImpl.java @@ -44,6 +44,7 @@ import javax.crypto.spec.SecretKeySpec; import javax.inject.Inject; import javax.naming.ConfigurationException; +import com.cloud.network.vpc.VpcVO; import org.apache.cloudstack.acl.ControlledEntity; import org.apache.cloudstack.acl.SecurityChecker; import org.apache.cloudstack.affinity.AffinityGroupProcessor; @@ -2580,12 +2581,21 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe } if (associatedNetworkId != null) { - _accountMgr.checkAccess(caller, null, false, networkDao.findById(associatedNetworkId)); - sc.setParameters("associatedNetworkIdEq", associatedNetworkId); + NetworkVO associatedNetwork = networkDao.findById(associatedNetworkId); + + if (associatedNetwork != null) { + _accountMgr.checkAccess(caller, null, false, associatedNetwork); + sc.setParameters("associatedNetworkIdEq", associatedNetworkId); + } } + if (vpcId != null) { - _accountMgr.checkAccess(caller, null, false, _vpcDao.findById(vpcId)); - sc.setParameters("vpcId", vpcId); + VpcVO vpc = _vpcDao.findById(vpcId); + + if (vpc != null) { + _accountMgr.checkAccess(caller, null, false, vpc); + sc.setParameters("vpcId", vpcId); + } } addrs = _publicIpAddressDao.search(sc, searchFilter); // Allocated @@ -2602,13 +2612,16 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe } if (associatedNetworkId != null) { NetworkVO guestNetwork = networkDao.findById(associatedNetworkId); - if (zoneId == null) { - zoneId = guestNetwork.getDataCenterId(); - } else if (zoneId != guestNetwork.getDataCenterId()) { - InvalidParameterValueException ex = new InvalidParameterValueException("Please specify a valid associated network id in the specified zone."); - throw ex; + + if (guestNetwork != null) { + if (zoneId == null) { + zoneId = guestNetwork.getDataCenterId(); + } else if (zoneId != guestNetwork.getDataCenterId()) { + InvalidParameterValueException ex = new InvalidParameterValueException("Please specify a valid associated network id in the specified zone."); + throw ex; + } + owner = _accountDao.findById(guestNetwork.getAccountId()); } - owner = _accountDao.findById(guestNetwork.getAccountId()); } List dcList = new ArrayList<>(); if (zoneId == null){ diff --git a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java index 78265021c0a..bf470205107 100755 --- a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java +++ b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java @@ -219,7 +219,6 @@ import com.cloud.vm.VirtualMachineProfileImpl; import com.cloud.vm.VmDetailConstants; import com.cloud.vm.dao.UserVmDao; import com.cloud.vm.dao.VMInstanceDao; -import com.google.common.base.Joiner; import com.google.gson.Gson; import com.google.gson.GsonBuilder; @@ -1375,9 +1374,16 @@ public class TemplateManagerImpl extends ManagerBase implements TemplateManager, else { vmInstanceVOList = _vmInstanceDao.listNonExpungedByTemplate(templateId); } - if(!cmd.isForced() && CollectionUtils.isNotEmpty(vmInstanceVOList)) { - final String message = String.format("Unable to delete Template: %s because Instance: [%s] are using it.", template, Joiner.on(",").join(vmInstanceVOList)); - logger.warn(message); + if (!cmd.isForced() && CollectionUtils.isNotEmpty(vmInstanceVOList)) { + String message = String.format("Unable to delete template [%s] because there are [%d] VM instances using it.", template, vmInstanceVOList.size()); + String instancesListMessage = String.format(" Instances list: [%s].", StringUtils.join(vmInstanceVOList, ",")); + + logger.warn("{}{}", message, instancesListMessage); + + if (_accountMgr.isRootAdmin(caller.getAccountId())) { + message += instancesListMessage; + } + throw new InvalidParameterValueException(message); } @@ -2210,7 +2216,7 @@ public class TemplateManagerImpl extends ManagerBase implements TemplateManager, templateType == null && templateTag == null && arch == null && - (! cleanupDetails && details == null) //update details in every case except this one + (!cleanupDetails && details == null) //update details in every case except this one ); if (!updateNeeded) { return template; @@ -2308,8 +2314,7 @@ public class TemplateManagerImpl extends ManagerBase implements TemplateManager, if (cleanupDetails) { template.setDetails(null); _tmpltDetailsDao.removeDetails(id); - } - else if (details != null && !details.isEmpty()) { + } else if (details != null && !details.isEmpty()) { template.setDetails(details); _tmpltDao.saveDetails(template); } diff --git a/server/src/main/java/com/cloud/user/AccountManagerImpl.java b/server/src/main/java/com/cloud/user/AccountManagerImpl.java index d4c23e8d62b..1f6e8d5b49e 100644 --- a/server/src/main/java/com/cloud/user/AccountManagerImpl.java +++ b/server/src/main/java/com/cloud/user/AccountManagerImpl.java @@ -85,7 +85,6 @@ import org.apache.cloudstack.webhook.WebhookHelper; import org.apache.commons.codec.binary.Base64; import org.apache.commons.collections.CollectionUtils; import org.apache.commons.lang3.BooleanUtils; -import org.apache.commons.lang3.StringUtils; import org.jetbrains.annotations.NotNull; import org.springframework.beans.factory.NoSuchBeanDefinitionException; @@ -176,6 +175,7 @@ import com.cloud.utils.ConstantTimeComparator; import com.cloud.utils.NumbersUtil; import com.cloud.utils.Pair; import com.cloud.utils.Ternary; +import com.cloud.utils.StringUtils; import com.cloud.utils.component.ComponentContext; import com.cloud.utils.component.Manager; import com.cloud.utils.component.ManagerBase; @@ -223,7 +223,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M @Inject private InstanceGroupDao _vmGroupDao; @Inject - private UserAccountDao _userAccountDao; + private UserAccountDao userAccountDao; @Inject private VolumeDao _volumeDao; @Inject @@ -585,11 +585,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M if (acct == null) { return false; //account is deleted or does not exist } - if ((isRootAdmin(accountId)) || (isDomainAdmin(accountId)) || (isResourceDomainAdmin(accountId))) { - return true; - } else if (acct.getType() == Account.Type.READ_ONLY_ADMIN) { - return true; - } + return (isRootAdmin(accountId)) || (isDomainAdmin(accountId)) || (isResourceDomainAdmin(accountId)) || (acct.getType() == Account.Type.READ_ONLY_ADMIN); } return false; @@ -644,10 +640,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M @Override public boolean isNormalUser(long accountId) { AccountVO acct = _accountDao.findById(accountId); - if (acct != null && acct.getType() == Account.Type.NORMAL) { - return true; - } - return false; + return acct != null && acct.getType() == Account.Type.NORMAL; } @Override @@ -678,10 +671,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M if (account == null) { return false; //account is deleted or does not exist } - if (isRootAdmin(accountId) || (account.getType() == Account.Type.ADMIN)) { - return true; - } - return false; + return isRootAdmin(accountId) || (account.getType() == Account.Type.ADMIN); } @Override @@ -712,7 +702,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M for (ControlledEntity entity : entities) { if (ownerId == null) { ownerId = entity.getAccountId(); - } else if (ownerId.longValue() != entity.getAccountId()) { + } else if (! ownerId.equals(entity.getAccountId())) { throw new PermissionDeniedException("Entity " + entity + " and entity " + prevEntity + " belong to different accounts"); } prevEntity = entity; @@ -738,7 +728,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M domainId = account != null ? account.getDomainId() : -1; } if (entity.getAccountId() != -1 && domainId != -1 && !(entity instanceof VirtualMachineTemplate) - && !(entity instanceof Network && accessType != null && (accessType == AccessType.UseEntry || accessType == AccessType.OperateEntry)) + && !(entity instanceof Network && (accessType == AccessType.UseEntry || accessType == AccessType.OperateEntry)) && !(entity instanceof AffinityGroup) && !(entity instanceof VirtualRouter)) { List toBeChecked = domains.get(entity.getDomainId()); // for templates, we don't have to do cross domains check @@ -821,7 +811,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M // Currently just for resource domain admin List dcList = _dcDao.findZonesByDomainId(account.getDomainId()); - if (dcList != null && dcList.size() != 0) { + if (CollectionUtils.isNotEmpty(dcList)) { return dcList.get(0).getId(); } else { throw new CloudRuntimeException("Failed to find any private zone for Resource domain admin."); @@ -836,23 +826,23 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M @Override public void doInTransactionWithoutResult(TransactionStatus status) { UserAccountVO user = null; - user = _userAccountDao.lockRow(id, true); + user = userAccountDao.lockRow(id, true); user.setLoginAttempts(attempts); if (toDisable) { user.setState(State.DISABLED.toString()); } - _userAccountDao.update(id, user); + userAccountDao.update(id, user); } }); } catch (Exception e) { - logger.error("Failed to update login attempts for user {}", () -> _userAccountDao.findById(id)); + logger.error("Failed to update login attempts for user {}", () -> userAccountDao.findById(id)); } } private boolean doSetUserStatus(long userId, State state) { UserVO userForUpdate = _userDao.createForUpdate(); userForUpdate.setState(state); - return _userDao.update(Long.valueOf(userId), userForUpdate); + return _userDao.update(userId, userForUpdate); } @Override @@ -861,7 +851,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M AccountVO acctForUpdate = _accountDao.createForUpdate(); acctForUpdate.setState(State.ENABLED); acctForUpdate.setNeedsCleanup(false); - success = _accountDao.update(Long.valueOf(accountId), acctForUpdate); + success = _accountDao.update(accountId, acctForUpdate); return success; } @@ -874,7 +864,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M } else if (account.getState().equals(State.ENABLED)) { AccountVO acctForUpdate = _accountDao.createForUpdate(); acctForUpdate.setState(State.LOCKED); - success = _accountDao.update(Long.valueOf(accountId), acctForUpdate); + success = _accountDao.update(accountId, acctForUpdate); } else { if (logger.isInfoEnabled()) { logger.info("Attempting to lock a non-enabled account {}, current state is {}, locking failed.", account, account.getState()); @@ -988,7 +978,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M } // Destroy VM Snapshots - List vmSnapshots = _vmSnapshotDao.listByAccountId(Long.valueOf(accountId)); + List vmSnapshots = _vmSnapshotDao.listByAccountId(accountId); for (VMSnapshot vmSnapshot : vmSnapshots) { try { _vmSnapshotMgr.deleteVMSnapshot(vmSnapshot.getId()); @@ -1010,8 +1000,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M try { _vmMgr.destroyVm(vm.getId(), false); } catch (Exception e) { - e.printStackTrace(); - logger.warn("Failed destroying instance {} as part of account deletion.", vm); + logger.warn("Failed destroying instance {} as part of account deletion.", vm, e); } } // no need to catch exception at this place as expunging vm @@ -1069,7 +1058,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M logger.debug("Deleting networks for account {}", account); List networks = _networkDao.listByOwner(accountId); if (networks != null) { - Collections.sort(networks, new Comparator<>() { + networks.sort(new Comparator<>() { @Override public int compare(NetworkVO network1, NetworkVO network2) { if (network1.getGuestType() != network2.getGuestType() && Network.GuestType.Isolated.equals(network2.getGuestType())) { @@ -1237,7 +1226,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M } else { AccountVO acctForUpdate = _accountDao.createForUpdate(); acctForUpdate.setState(State.DISABLED); - success = _accountDao.update(Long.valueOf(accountId), acctForUpdate); + success = _accountDao.update(accountId, acctForUpdate); if (success) { boolean disableAccountResult = false; @@ -1331,11 +1320,11 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M // Check permissions checkAccess(getCurrentCallingAccount(), domain); - if (!userAllowMultipleAccounts.valueInDomain(domainId) && !_userAccountDao.validateUsernameInDomain(userName, domainId)) { + if (!userAllowMultipleAccounts.valueInDomain(domainId) && !userAccountDao.validateUsernameInDomain(userName, domainId)) { throw new InvalidParameterValueException(String.format("The user %s already exists in domain %s", userName, domain)); } - if (networkDomain != null && networkDomain.length() > 0) { + if (StringUtils.isNotEmpty(networkDomain)) { if (!NetUtils.verifyDomainName(networkDomain)) { throw new InvalidParameterValueException( "Invalid network domain. Total length shouldn't exceed 190 chars. Each domain label must be between 1 and 63 characters long, can contain ASCII letters 'a' through 'z', the digits '0' through '9', " @@ -1387,7 +1376,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M CallContext.current().putContextParameter(User.class, userId); // check success - return _userAccountDao.findById(userId); + return userAccountDao.findById(userId); } /* @@ -1525,7 +1514,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M throw new PermissionDeniedException(String.format("Account: %s is a system account, can't add a user to it", account)); } - if (!userAllowMultipleAccounts.valueInDomain(domainId) && !_userAccountDao.validateUsernameInDomain(userName, domainId)) { + if (!userAllowMultipleAccounts.valueInDomain(domainId) && !userAccountDao.validateUsernameInDomain(userName, domainId)) { throw new CloudRuntimeException("The user " + userName + " already exists in domain " + domainId); } List duplicatedUsers = _userDao.findUsersByName(userName); @@ -1579,7 +1568,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M user.setUser2faEnabled(true); } _userDao.update(user.getId(), user); - return _userAccountDao.findById(user.getId()); + return userAccountDao.findById(user.getId()); } @Override @@ -1861,10 +1850,9 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M if (isApiKeyBlank && isSecretKeyBlank) { return; } - Pair apiKeyOwner = _accountDao.findUserAccountByApiKey(apiKey); + UserAccount apiKeyOwner = userAccountDao.getUserByApiKey(apiKey); if (apiKeyOwner != null) { - User userThatHasTheProvidedApiKey = apiKeyOwner.first(); - if (userThatHasTheProvidedApiKey.getId() != user.getId()) { + if (apiKeyOwner.getId() != user.getId()) { throw new InvalidParameterValueException(String.format("The API key [%s] already exists in the system. Please provide a unique key.", apiKey)); } } @@ -1952,7 +1940,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M CallContext.current().putContextParameter(User.class, user.getUuid()); // user successfully disabled - return _userAccountDao.findById(userId); + return userAccountDao.findById(userId); } else { throw new CloudRuntimeException(String.format("Unable to disable user %s", user)); } @@ -2006,7 +1994,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M CallContext.current().putContextParameter(User.class, user.getUuid()); - return _userAccountDao.findById(userId); + return userAccountDao.findById(userId); } else { throw new CloudRuntimeException(String.format("Unable to enable user %s", user)); } @@ -2047,7 +2035,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M boolean success; if (user.getState().equals(State.LOCKED)) { // already locked...no-op - return _userAccountDao.findById(userId); + return userAccountDao.findById(userId); } else if (user.getState().equals(State.ENABLED)) { success = doSetUserStatus(user.getId(), State.LOCKED); @@ -2074,7 +2062,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M CallContext.current().putContextParameter(User.class, user.getUuid()); - return _userAccountDao.findById(userId); + return userAccountDao.findById(userId); } else { throw new CloudRuntimeException(String.format("Unable to lock user %s", user)); } @@ -2602,7 +2590,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M return owner; } else if (!isAdmin(caller.getId()) && accountName != null && domainId != null) { - if (!accountName.equals(caller.getAccountName()) || domainId.longValue() != caller.getDomainId()) { + if (!accountName.equals(caller.getAccountName()) || domainId != caller.getDomainId()) { throw new PermissionDeniedException("Can't create/list resources for account " + accountName + " in domain " + domainId + ", permission denied"); } else { return caller; @@ -2627,12 +2615,12 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M @Override public UserAccount getActiveUserAccount(String username, Long domainId) { - return _userAccountDao.getUserAccount(username, domainId); + return userAccountDao.getUserAccount(username, domainId); } @Override public List getActiveUserAccountByEmail(String email, Long domainId) { - List userAccountByEmail = _userAccountDao.getUserAccountByEmail(email, domainId); + List userAccountByEmail = userAccountDao.getUserAccountByEmail(email, domainId); List userAccounts = userAccountByEmail.stream() .map(userAccountVO -> (UserAccount) userAccountVO) .collect(Collectors.toList()); @@ -2676,7 +2664,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M public void markUserRegistered(long userId) { UserVO userForUpdate = _userDao.createForUpdate(); userForUpdate.setRegistered(true); - _userDao.update(Long.valueOf(userId), userForUpdate); + _userDao.update(userId, userForUpdate); } @Override @@ -2731,7 +2719,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M throw new CloudRuntimeException(String.format("Failed to create account name %s in domain id=%s", accountName, _domainMgr.getDomain(domainId))); } - Long accountId = account.getId(); + long accountId = account.getId(); if (details != null) { _accountDetailsDao.persist(accountId, details); @@ -2780,7 +2768,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M @Override public void logoutUser(long userId) { - UserAccount userAcct = _userAccountDao.findById(userId); + UserAccount userAcct = userAccountDao.findById(userId); if (userAcct != null) { ActionEventUtils.onActionEvent(userId, userAcct.getAccountId(), userAcct.getDomainId(), EventTypes.EVENT_USER_LOGOUT, "user has logged out", userId, ApiCommandResourceType.User.toString()); } // else log some kind of error event? This likely means the user doesn't exist, or has been deleted... @@ -2822,11 +2810,11 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M final Boolean ApiSourceCidrChecksEnabled = ApiServiceConfiguration.ApiSourceCidrChecksEnabled.value(); if (ApiSourceCidrChecksEnabled) { - logger.debug("CIDRs from which account '" + account.toString() + "' is allowed to perform API calls: " + accessAllowedCidrs); + logger.debug("CIDRs from which account '{}' is allowed to perform API calls: {}", account.toString(), accessAllowedCidrs); // Block when is not in the list of allowed IPs if (!NetUtils.isIpInCidrList(loginIpAddress, accessAllowedCidrs.split(","))) { - logger.warn("Request by account '" + account.toString() + "' was denied since " + loginIpAddress.toString().replace("/", "") + " does not match " + accessAllowedCidrs); + logger.warn("Request by account '{}' was denied since {} does not match {}", account.toString(), loginIpAddress.toString().replace("/", ""), accessAllowedCidrs); throw new CloudAuthenticationException("Failed to authenticate user '" + username + "' in domain '" + domain.getPath() + "' from ip " + loginIpAddress.toString().replace("/", "") + "; please provide valid credentials"); } @@ -2858,6 +2846,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M try { Thread.sleep(waitTimeDurationInMs); } catch (final InterruptedException e) { + // ignored } } @@ -2869,7 +2858,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M if (logger.isDebugEnabled()) { logger.debug("Attempting to log in user: " + username + " in domain " + domainId); } - UserAccount userAccount = _userAccountDao.getUserAccount(username, domainId); + UserAccount userAccount = userAccountDao.getUserAccount(username, domainId); boolean authenticated = false; HashSet actionsOnFailedAuthenticaion = new HashSet<>(); @@ -2899,11 +2888,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M if (authenticated) { Domain domain = _domainMgr.getDomain(domainId); - String domainName = null; - if (domain != null) { - domainName = domain.getName(); - } - userAccount = _userAccountDao.getUserAccount(username, domainId); + userAccount = userAccountDao.getUserAccount(username, domainId); if (!userAccount.getState().equalsIgnoreCase(Account.State.ENABLED.toString()) || !userAccount.getAccountState().equalsIgnoreCase(Account.State.ENABLED.toString())) { if (logger.isInfoEnabled()) { @@ -2963,11 +2948,9 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M // - build a request string with sorted params, make sure it's all lowercase // - sign the request, verify the signature is the same - List parameterNames = new ArrayList<>(); - for (Object paramNameObj : requestParameters.keySet()) { - parameterNames.add((String)paramNameObj); // put the name in a list that we'll sort later - } + // put the name in a list that we'll sort later + List parameterNames = new ArrayList<>(requestParameters.keySet()); Collections.sort(parameterNames); @@ -2999,7 +2982,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M if (unsignedRequestBuffer.length() != 0) { unsignedRequestBuffer.append("&"); } - unsignedRequestBuffer.append(paramName).append("=").append(URLEncoder.encode(paramValue, "UTF-8")); + unsignedRequestBuffer.append(paramName).append("=").append(URLEncoder.encode(paramValue, com.cloud.utils.StringUtils.getPreferredCharset())); } } @@ -3022,7 +3005,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M if (!equalSig) { logger.info("User signature: " + signature + " is not equaled to computed signature: " + computedSignature); } else { - user = _userAccountDao.getUserAccount(username, domainId); + user = userAccountDao.getUserAccount(username, domainId); } } catch (Exception ex) { logger.error("Exception authenticating user", ex); @@ -3050,7 +3033,14 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M @Override public Pair findUserByApiKey(String apiKey) { - return _accountDao.findUserAccountByApiKey(apiKey); + UserAccount userAccount = userAccountDao.getUserByApiKey(apiKey); + if (userAccount != null) { + User user = _userDao.getUser(userAccount.getId()); + Account account = _accountDao.findById(userAccount.getAccountId()); + return new Pair<>(user, account); + } else { + return null; + } } @Override @@ -3184,14 +3174,14 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M UserVO updatedUser = _userDao.createForUpdate(); String encodedKey; - Pair userAcct; + UserAccount userAcct; int retryLimit = 10; do { // FIXME: what algorithm should we use for API keys? KeyGenerator generator = KeyGenerator.getInstance("HmacSHA1"); SecretKey key = generator.generateKey(); encodedKey = Base64.encodeBase64URLSafeString(key.getEncoded()); - userAcct = _accountDao.findUserAccountByApiKey(encodedKey); + userAcct = userAccountDao.getUserByApiKey(encodedKey); retryLimit--; } while ((userAcct != null) && (retryLimit >= 0)); @@ -3202,7 +3192,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M _userDao.update(userId, updatedUser); return encodedKey; } catch (NoSuchAlgorithmException ex) { - logger.error("error generating secret key for user {}", _userAccountDao.findById(userId), ex); + logger.error("error generating secret key for user {}", userAccountDao.findById(userId), ex); } return null; } @@ -3229,7 +3219,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M _userDao.update(userId, updatedUser); return encodedKey; } catch (NoSuchAlgorithmException ex) { - logger.error("error generating secret key for user {}", _userAccountDao.findById(userId), ex); + logger.error("error generating secret key for user {}", userAccountDao.findById(userId), ex); } return null; } @@ -3440,12 +3430,12 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M @Override public UserAccount getUserByApiKey(String apiKey) { - return _userAccountDao.getUserByApiKey(apiKey); + return userAccountDao.getUserByApiKey(apiKey); } @Override public List listAclGroupsByAccount(Long accountId) { - if (_querySelectors == null || _querySelectors.size() == 0) { + if (CollectionUtils.isEmpty(_querySelectors)) { return new ArrayList<>(); } @@ -3500,7 +3490,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M @Override public UserAccount getUserAccountById(Long userId) { - UserAccount userAccount = _userAccountDao.findById(userId); + UserAccount userAccount = userAccountDao.findById(userId); Map details = _userDetailsDao.listDetailsKeyPairs(userId); userAccount.setDetails(details); @@ -3674,7 +3664,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M } protected UserTwoFactorAuthenticationSetupResponse enableTwoFactorAuthentication(Long userId, String providerName) { - UserAccountVO userAccount = _userAccountDao.findById(userId); + UserAccountVO userAccount = userAccountDao.findById(userId); UserVO userVO = _userDao.findById(userId); Long domainId = userAccount.getDomainId(); if (Boolean.FALSE.equals(enableUserTwoFactorAuthentication.valueIn(domainId)) && Boolean.FALSE.equals(mandateUserTwoFactorAuthentication.valueIn(domainId))) { @@ -3766,11 +3756,11 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M if (userDetailVO != null) { _userDetailsDao.remove(userDetailVO.getId()); } - UserAccountVO userAccountVO = _userAccountDao.findById(user.getId()); + UserAccountVO userAccountVO = userAccountDao.findById(user.getId()); userAccountVO.setUser2faEnabled(false); userAccountVO.setUser2faProvider(null); userAccountVO.setKeyFor2fa(null); - _userAccountDao.update(user.getId(), userAccountVO); + userAccountDao.update(user.getId(), userAccountVO); return userAccountVO; }); } diff --git a/server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java b/server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java index 43270e6a029..cdbb7119e9e 100644 --- a/server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java @@ -23,6 +23,7 @@ import java.util.Date; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Set; import javax.inject.Inject; import javax.naming.ConfigurationException; @@ -182,6 +183,11 @@ public class VMSnapshotManagerImpl extends MutualExclusiveIdsManagerBase impleme Long.class, "vm.job.check.interval", "3000", "Interval in milliseconds to check if the job is complete", false); + private static final Set VM_SNAPSHOT_CUSTOM_SERVICE_OFFERING_DETAILS = Set.of( + VmDetailConstants.CPU_NUMBER.toLowerCase(), + VmDetailConstants.CPU_SPEED.toLowerCase(), + VmDetailConstants.MEMORY.toLowerCase()); + @Override public boolean configure(String name, Map params) throws ConfigurationException { _name = name; @@ -473,7 +479,8 @@ public class VMSnapshotManagerImpl extends MutualExclusiveIdsManagerBase impleme } /** - * Add entries on vm_snapshot_details if service offering is dynamic. This will allow setting details when revert to vm snapshot + * Add entries about cpu, cpu_speed and memory in vm_snapshot_details if service offering is dynamic. + * This will allow setting details when revert to vm snapshot. * @param vmId vm id * @param serviceOfferingId service offering id * @param vmSnapshotId vm snapshot id @@ -484,7 +491,7 @@ public class VMSnapshotManagerImpl extends MutualExclusiveIdsManagerBase impleme List vmDetails = _userVmDetailsDao.listDetails(vmId); List vmSnapshotDetails = new ArrayList(); for (UserVmDetailVO detail : vmDetails) { - if(detail.getName().equalsIgnoreCase(VmDetailConstants.CPU_NUMBER) || detail.getName().equalsIgnoreCase(VmDetailConstants.CPU_SPEED) || detail.getName().equalsIgnoreCase(VmDetailConstants.MEMORY)) { + if (VM_SNAPSHOT_CUSTOM_SERVICE_OFFERING_DETAILS.contains(detail.getName().toLowerCase())) { vmSnapshotDetails.add(new VMSnapshotDetailsVO(vmSnapshotId, detail.getName(), detail.getValue(), detail.isDisplay())); } } @@ -931,7 +938,7 @@ public class VMSnapshotManagerImpl extends MutualExclusiveIdsManagerBase impleme Transaction.execute(new TransactionCallbackWithExceptionNoReturn() { @Override public void doInTransactionWithoutResult(TransactionStatus status) throws CloudRuntimeException { - revertUserVmDetailsFromVmSnapshot(userVm, vmSnapshotVo); + revertCustomServiceOfferingDetailsFromVmSnapshot(userVm, vmSnapshotVo); updateUserVmServiceOffering(userVm, vmSnapshotVo); } }); @@ -943,19 +950,19 @@ public class VMSnapshotManagerImpl extends MutualExclusiveIdsManagerBase impleme } /** - * Update or add user vm details from vm snapshot for vms with custom service offerings + * Update or add user vm details (cpu, cpu_speed and memory) from vm snapshot for vms with custom service offerings * @param userVm user vm * @param vmSnapshotVo vm snapshot */ - protected void revertUserVmDetailsFromVmSnapshot(UserVmVO userVm, VMSnapshotVO vmSnapshotVo) { + protected void revertCustomServiceOfferingDetailsFromVmSnapshot(UserVmVO userVm, VMSnapshotVO vmSnapshotVo) { ServiceOfferingVO serviceOfferingVO = _serviceOfferingDao.findById(vmSnapshotVo.getServiceOfferingId()); if (serviceOfferingVO.isDynamic()) { List vmSnapshotDetails = _vmSnapshotDetailsDao.listDetails(vmSnapshotVo.getId()); - List userVmDetails = new ArrayList(); for (VMSnapshotDetailsVO detail : vmSnapshotDetails) { - userVmDetails.add(new UserVmDetailVO(userVm.getId(), detail.getName(), detail.getValue(), detail.isDisplay())); + if (VM_SNAPSHOT_CUSTOM_SERVICE_OFFERING_DETAILS.contains(detail.getName().toLowerCase())) { + _userVmDetailsDao.addDetail(userVm.getId(), detail.getName(), detail.getValue(), detail.isDisplay()); + } } - _userVmDetailsDao.saveDetails(userVmDetails); } } diff --git a/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java b/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java index dc2677a507f..b3b22e98e45 100644 --- a/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java @@ -826,7 +826,7 @@ public class BackupManagerImpl extends ManagerBase implements BackupManager { throw new CloudRuntimeException(String.format("Error restoring volume [%s] of VM [%s] to host [%s] using backup provider [%s] due to: [%s].", backedUpVolumeUuid, vm.getUuid(), host.getUuid(), backupProvider.getName(), result.second())); } - if (!attachVolumeToVM(vm.getDataCenterId(), result.second(), vmFromBackup.getBackupVolumeList(), + if (!attachVolumeToVM(vm.getDataCenterId(), result.second(), backup.getBackedUpVolumes(), backedUpVolumeUuid, vm, datastore.getUuid(), backup)) { throw new CloudRuntimeException(String.format("Error attaching volume [%s] to VM [%s]." + backedUpVolumeUuid, vm.getUuid())); } @@ -972,10 +972,10 @@ public class BackupManagerImpl extends ManagerBase implements BackupManager { if (StringUtils.isEmpty(name)) { throw new CloudRuntimeException("Invalid backup provider name provided"); } - if (!backupProvidersMap.containsKey(name)) { - throw new CloudRuntimeException("Failed to find backup provider by the name: " + name); - } - return backupProvidersMap.get(name); + if (!backupProvidersMap.containsKey(name)) { + throw new CloudRuntimeException("Failed to find backup provider by the name: " + name); + } + return backupProvidersMap.get(name); } @Override diff --git a/server/src/main/java/org/apache/cloudstack/storage/heuristics/HeuristicRuleHelper.java b/server/src/main/java/org/apache/cloudstack/storage/heuristics/HeuristicRuleHelper.java index 2e0780e7fe8..beecf90d2b8 100644 --- a/server/src/main/java/org/apache/cloudstack/storage/heuristics/HeuristicRuleHelper.java +++ b/server/src/main/java/org/apache/cloudstack/storage/heuristics/HeuristicRuleHelper.java @@ -139,23 +139,23 @@ public class HeuristicRuleHelper { * @param presetVariables used for injecting in the JS interpreter. */ protected void injectPresetVariables(JsInterpreter jsInterpreter, PresetVariables presetVariables) { - jsInterpreter.injectVariable("secondaryStorages", presetVariables.getSecondaryStorages().toString()); + jsInterpreter.injectVariable("secondaryStorages", presetVariables.getSecondaryStorages()); if (presetVariables.getTemplate() != null) { - jsInterpreter.injectVariable("template", presetVariables.getTemplate().toString()); - jsInterpreter.injectVariable("iso", presetVariables.getTemplate().toString()); + jsInterpreter.injectVariable("template", presetVariables.getTemplate()); + jsInterpreter.injectVariable("iso", presetVariables.getTemplate()); } if (presetVariables.getSnapshot() != null) { - jsInterpreter.injectVariable("snapshot", presetVariables.getSnapshot().toString()); + jsInterpreter.injectVariable("snapshot", presetVariables.getSnapshot()); } if (presetVariables.getVolume() != null) { - jsInterpreter.injectVariable("volume", presetVariables.getVolume().toString()); + jsInterpreter.injectVariable("volume", presetVariables.getVolume()); } if (presetVariables.getAccount() != null) { - jsInterpreter.injectVariable("account", presetVariables.getAccount().toString()); + jsInterpreter.injectVariable("account", presetVariables.getAccount()); } } @@ -185,8 +185,8 @@ public class HeuristicRuleHelper { Template template = new Template(); template.setName(templateVO.getName()); - template.setFormat(templateVO.getFormat()); - template.setHypervisorType(templateVO.getHypervisorType()); + template.setFormat(templateVO.getFormat().toString()); + template.setHypervisorType(templateVO.getHypervisorType().toString()); return template; } @@ -195,7 +195,7 @@ public class HeuristicRuleHelper { Volume volumePresetVariable = new Volume(); volumePresetVariable.setName(volumeVO.getName()); - volumePresetVariable.setFormat(volumeVO.getFormat()); + volumePresetVariable.setFormat(volumeVO.getFormat().toString()); volumePresetVariable.setSize(volumeVO.getSize()); return volumePresetVariable; @@ -206,7 +206,7 @@ public class HeuristicRuleHelper { snapshot.setName(snapshotInfo.getName()); snapshot.setSize(snapshotInfo.getSize()); - snapshot.setHypervisorType(snapshotInfo.getHypervisorType()); + snapshot.setHypervisorType(snapshotInfo.getHypervisorType().toString()); return snapshot; } diff --git a/server/src/main/java/org/apache/cloudstack/storage/heuristics/presetvariables/Account.java b/server/src/main/java/org/apache/cloudstack/storage/heuristics/presetvariables/Account.java index 67750e8ec5c..f4652d87e7b 100644 --- a/server/src/main/java/org/apache/cloudstack/storage/heuristics/presetvariables/Account.java +++ b/server/src/main/java/org/apache/cloudstack/storage/heuristics/presetvariables/Account.java @@ -27,7 +27,6 @@ public class Account extends GenericHeuristicPresetVariable { public void setId(String id) { this.id = id; - fieldNamesToIncludeInToString.add("id"); } public Domain getDomain() { @@ -36,6 +35,5 @@ public class Account extends GenericHeuristicPresetVariable { public void setDomain(Domain domain) { this.domain = domain; - fieldNamesToIncludeInToString.add("domain"); } } diff --git a/server/src/main/java/org/apache/cloudstack/storage/heuristics/presetvariables/Domain.java b/server/src/main/java/org/apache/cloudstack/storage/heuristics/presetvariables/Domain.java index 704cbf4373e..0b01604e673 100644 --- a/server/src/main/java/org/apache/cloudstack/storage/heuristics/presetvariables/Domain.java +++ b/server/src/main/java/org/apache/cloudstack/storage/heuristics/presetvariables/Domain.java @@ -25,6 +25,5 @@ public class Domain extends GenericHeuristicPresetVariable { public void setId(String id) { this.id = id; - fieldNamesToIncludeInToString.add("id"); } } diff --git a/server/src/main/java/org/apache/cloudstack/storage/heuristics/presetvariables/GenericHeuristicPresetVariable.java b/server/src/main/java/org/apache/cloudstack/storage/heuristics/presetvariables/GenericHeuristicPresetVariable.java index b85b7763eee..56c7d48e68a 100644 --- a/server/src/main/java/org/apache/cloudstack/storage/heuristics/presetvariables/GenericHeuristicPresetVariable.java +++ b/server/src/main/java/org/apache/cloudstack/storage/heuristics/presetvariables/GenericHeuristicPresetVariable.java @@ -16,15 +16,11 @@ // under the License. package org.apache.cloudstack.storage.heuristics.presetvariables; -import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils; - -import java.util.HashSet; -import java.util.Set; +import org.apache.commons.lang3.builder.ToStringBuilder; +import org.apache.commons.lang3.builder.ToStringStyle; public class GenericHeuristicPresetVariable { - protected transient Set fieldNamesToIncludeInToString = new HashSet<>(); - private String name; public String getName() { @@ -33,15 +29,10 @@ public class GenericHeuristicPresetVariable { public void setName(String name) { this.name = name; - fieldNamesToIncludeInToString.add("name"); } - /*** - * Converts the preset variable into a valid JSON object that will be injected into the JS interpreter. - * This method should not be overridden or changed. - */ @Override - public final String toString() { - return ReflectionToStringBuilderUtils.reflectOnlySelectedFields(this, fieldNamesToIncludeInToString.toArray(new String[0])); + public String toString() { + return ToStringBuilder.reflectionToString(this, ToStringStyle.JSON_STYLE); } } diff --git a/server/src/main/java/org/apache/cloudstack/storage/heuristics/presetvariables/SecondaryStorage.java b/server/src/main/java/org/apache/cloudstack/storage/heuristics/presetvariables/SecondaryStorage.java index ad7058d8336..e3a8dac43c6 100644 --- a/server/src/main/java/org/apache/cloudstack/storage/heuristics/presetvariables/SecondaryStorage.java +++ b/server/src/main/java/org/apache/cloudstack/storage/heuristics/presetvariables/SecondaryStorage.java @@ -32,7 +32,6 @@ public class SecondaryStorage extends GenericHeuristicPresetVariable { public void setId(String id) { this.id = id; - fieldNamesToIncludeInToString.add("id"); } public Long getUsedDiskSize() { @@ -41,7 +40,6 @@ public class SecondaryStorage extends GenericHeuristicPresetVariable { public void setUsedDiskSize(Long usedDiskSize) { this.usedDiskSize = usedDiskSize; - fieldNamesToIncludeInToString.add("usedDiskSize"); } public Long getTotalDiskSize() { @@ -50,7 +48,6 @@ public class SecondaryStorage extends GenericHeuristicPresetVariable { public void setTotalDiskSize(Long totalDiskSize) { this.totalDiskSize = totalDiskSize; - fieldNamesToIncludeInToString.add("totalDiskSize"); } public String getProtocol() { @@ -59,6 +56,5 @@ public class SecondaryStorage extends GenericHeuristicPresetVariable { public void setProtocol(String protocol) { this.protocol = protocol; - fieldNamesToIncludeInToString.add("protocol"); } } diff --git a/server/src/main/java/org/apache/cloudstack/storage/heuristics/presetvariables/Snapshot.java b/server/src/main/java/org/apache/cloudstack/storage/heuristics/presetvariables/Snapshot.java index 34acd394dbe..404db3cdebc 100644 --- a/server/src/main/java/org/apache/cloudstack/storage/heuristics/presetvariables/Snapshot.java +++ b/server/src/main/java/org/apache/cloudstack/storage/heuristics/presetvariables/Snapshot.java @@ -16,13 +16,11 @@ // under the License. package org.apache.cloudstack.storage.heuristics.presetvariables; -import com.cloud.hypervisor.Hypervisor; - public class Snapshot extends GenericHeuristicPresetVariable { private Long size; - private Hypervisor.HypervisorType hypervisorType; + private String hypervisorType; public Long getSize() { return size; @@ -30,15 +28,13 @@ public class Snapshot extends GenericHeuristicPresetVariable { public void setSize(Long size) { this.size = size; - fieldNamesToIncludeInToString.add("size"); } - public Hypervisor.HypervisorType getHypervisorType() { + public String getHypervisorType() { return hypervisorType; } - public void setHypervisorType(Hypervisor.HypervisorType hypervisorType) { + public void setHypervisorType(String hypervisorType) { this.hypervisorType = hypervisorType; - fieldNamesToIncludeInToString.add("hypervisorType"); } } diff --git a/server/src/main/java/org/apache/cloudstack/storage/heuristics/presetvariables/Template.java b/server/src/main/java/org/apache/cloudstack/storage/heuristics/presetvariables/Template.java index 297c95fad9a..c6df349010f 100644 --- a/server/src/main/java/org/apache/cloudstack/storage/heuristics/presetvariables/Template.java +++ b/server/src/main/java/org/apache/cloudstack/storage/heuristics/presetvariables/Template.java @@ -16,41 +16,35 @@ // under the License. package org.apache.cloudstack.storage.heuristics.presetvariables; -import com.cloud.hypervisor.Hypervisor; -import com.cloud.storage.Storage; - public class Template extends GenericHeuristicPresetVariable { - private Hypervisor.HypervisorType hypervisorType; + private String hypervisorType; - private Storage.ImageFormat format; + private String format; - private Storage.TemplateType templateType; + private String templateType; - public Hypervisor.HypervisorType getHypervisorType() { + public String getHypervisorType() { return hypervisorType; } - public void setHypervisorType(Hypervisor.HypervisorType hypervisorType) { + public void setHypervisorType(String hypervisorType) { this.hypervisorType = hypervisorType; - fieldNamesToIncludeInToString.add("hypervisorType"); } - public Storage.ImageFormat getFormat() { + public String getFormat() { return format; } - public void setFormat(Storage.ImageFormat format) { + public void setFormat(String format) { this.format = format; - fieldNamesToIncludeInToString.add("format"); } - public Storage.TemplateType getTemplateType() { + public String getTemplateType() { return templateType; } - public void setTemplateType(Storage.TemplateType templateType) { + public void setTemplateType(String templateType) { this.templateType = templateType; - fieldNamesToIncludeInToString.add("templateType"); } } diff --git a/server/src/main/java/org/apache/cloudstack/storage/heuristics/presetvariables/Volume.java b/server/src/main/java/org/apache/cloudstack/storage/heuristics/presetvariables/Volume.java index 4e5e81b117f..8f571a57209 100644 --- a/server/src/main/java/org/apache/cloudstack/storage/heuristics/presetvariables/Volume.java +++ b/server/src/main/java/org/apache/cloudstack/storage/heuristics/presetvariables/Volume.java @@ -16,13 +16,11 @@ // under the License. package org.apache.cloudstack.storage.heuristics.presetvariables; -import com.cloud.storage.Storage; - public class Volume extends GenericHeuristicPresetVariable { private Long size; - private Storage.ImageFormat format; + private String format; public Long getSize() { return size; @@ -30,15 +28,13 @@ public class Volume extends GenericHeuristicPresetVariable { public void setSize(Long size) { this.size = size; - fieldNamesToIncludeInToString.add("size"); } - public Storage.ImageFormat getFormat() { + public String getFormat() { return format; } - public void setFormat(Storage.ImageFormat format) { + public void setFormat(String format) { this.format = format; - fieldNamesToIncludeInToString.add("format"); } } diff --git a/server/src/test/java/com/cloud/resource/MockResourceManagerImpl.java b/server/src/test/java/com/cloud/resource/MockResourceManagerImpl.java index b7bb2238334..48ac105a320 100755 --- a/server/src/test/java/com/cloud/resource/MockResourceManagerImpl.java +++ b/server/src/test/java/com/cloud/resource/MockResourceManagerImpl.java @@ -460,6 +460,11 @@ public class MockResourceManagerImpl extends ManagerBase implements ResourceMana return null; } + @Override + public HostVO findHostByGuidPrefix(String guid) { + return null; + } + /* (non-Javadoc) * @see com.cloud.resource.ResourceManager#findHostByName(java.lang.String) */ diff --git a/server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java b/server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java index 1669d7a47d9..91e4bf7a47b 100644 --- a/server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java +++ b/server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java @@ -360,12 +360,14 @@ public class ResourceManagerImplTest { @Test public void testConnectAndRestartAgentOnHost() { + when(agentManager.getHostSshPort(any())).thenReturn(22); resourceManager.connectAndRestartAgentOnHost(host, hostUsername, hostPassword, hostPrivateKey); } @Test public void testHandleAgentSSHEnabledNotConnectedAgent() { when(host.getStatus()).thenReturn(Status.Disconnected); + when(agentManager.getHostSshPort(any())).thenReturn(22); resourceManager.handleAgentIfNotConnected(host, false); verify(resourceManager).getHostCredentials(eq(host)); verify(resourceManager).connectAndRestartAgentOnHost(eq(host), eq(hostUsername), eq(hostPassword), eq(hostPrivateKey)); diff --git a/server/src/test/java/com/cloud/user/AccountManagetImplTestBase.java b/server/src/test/java/com/cloud/user/AccountManagentImplTestBase.java similarity index 98% rename from server/src/test/java/com/cloud/user/AccountManagetImplTestBase.java rename to server/src/test/java/com/cloud/user/AccountManagentImplTestBase.java index 98f152088ed..71878143f24 100644 --- a/server/src/test/java/com/cloud/user/AccountManagetImplTestBase.java +++ b/server/src/test/java/com/cloud/user/AccountManagentImplTestBase.java @@ -84,7 +84,7 @@ import java.util.HashMap; import java.util.Map; @RunWith(MockitoJUnitRunner.class) -public class AccountManagetImplTestBase { +public class AccountManagentImplTestBase { @Mock AccountDao _accountDao; @@ -99,7 +99,7 @@ public class AccountManagetImplTestBase { @Mock InstanceGroupDao _vmGroupDao; @Mock - UserAccountDao userAccountDaoMock; + UserAccountDao userAccountDao; @Mock VolumeDao _volumeDao; @Mock @@ -210,9 +210,6 @@ public class AccountManagetImplTestBase { @Mock RoutedIpv4Manager routedIpv4Manager; - @Mock - Account accountMock; - @Before public void setup() { accountManagerImpl.setUserAuthenticators(Arrays.asList(userAuthenticator)); @@ -228,7 +225,6 @@ public class AccountManagetImplTestBase { @Test public void test() { - return; } public static Map getInheritedFields(Class type) { diff --git a/server/src/test/java/com/cloud/user/AccountManagerImplTest.java b/server/src/test/java/com/cloud/user/AccountManagerImplTest.java index 2aeb43469d1..6f5fbb0fdc1 100644 --- a/server/src/test/java/com/cloud/user/AccountManagerImplTest.java +++ b/server/src/test/java/com/cloud/user/AccountManagerImplTest.java @@ -47,13 +47,11 @@ import org.apache.cloudstack.webhook.WebhookHelper; import org.junit.Assert; import org.junit.Before; import org.junit.Test; -import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; import org.mockito.InOrder; import org.mockito.Mock; import org.mockito.MockedStatic; import org.mockito.Mockito; -import org.mockito.junit.MockitoJUnitRunner; import org.springframework.beans.factory.NoSuchBeanDefinitionException; import com.cloud.acl.DomainChecker; @@ -76,8 +74,7 @@ import com.cloud.vm.UserVmVO; import com.cloud.vm.VMInstanceVO; import com.cloud.vm.snapshot.VMSnapshotVO; -@RunWith(MockitoJUnitRunner.class) -public class AccountManagerImplTest extends AccountManagetImplTestBase { +public class AccountManagerImplTest extends AccountManagentImplTestBase { @Mock private UserVmManagerImpl _vmMgr; @@ -100,11 +97,11 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase { @Mock private UpdateAccountCmd UpdateAccountCmdMock; - private long userVoIdMock = 111l; + private final long userVoIdMock = 111L; @Mock private UserVO userVoMock; - private long accountMockId = 100l; + private final long accountMockId = 100L; @Mock private Account accountMock; @@ -154,7 +151,7 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase { @Test public void disableAccountNotexisting() throws ConcurrentOperationException, ResourceUnavailableException { - Mockito.when(_accountDao.findById(42l)).thenReturn(null); + Mockito.when(_accountDao.findById(42L)).thenReturn(null); Assert.assertTrue(accountManagerImpl.disableAccount(42)); } @@ -162,7 +159,7 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase { public void disableAccountDisabled() throws ConcurrentOperationException, ResourceUnavailableException { AccountVO disabledAccount = new AccountVO(); disabledAccount.setState(State.DISABLED); - Mockito.when(_accountDao.findById(42l)).thenReturn(disabledAccount); + Mockito.when(_accountDao.findById(42L)).thenReturn(disabledAccount); Assert.assertTrue(accountManagerImpl.disableAccount(42)); } @@ -170,22 +167,22 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase { public void disableAccount() throws ConcurrentOperationException, ResourceUnavailableException { AccountVO account = new AccountVO(); account.setState(State.ENABLED); - Mockito.when(_accountDao.findById(42l)).thenReturn(account); + Mockito.when(_accountDao.findById(42L)).thenReturn(account); Mockito.when(_accountDao.createForUpdate()).thenReturn(new AccountVO()); - Mockito.when(_accountDao.update(Mockito.eq(42l), Mockito.any(AccountVO.class))).thenReturn(true); - Mockito.when(_vmDao.listByAccountId(42l)).thenReturn(Arrays.asList(Mockito.mock(VMInstanceVO.class))); + Mockito.when(_accountDao.update(Mockito.eq(42L), Mockito.any(AccountVO.class))).thenReturn(true); + Mockito.when(_vmDao.listByAccountId(42L)).thenReturn(Arrays.asList(Mockito.mock(VMInstanceVO.class))); Assert.assertTrue(accountManagerImpl.disableAccount(42)); - Mockito.verify(_accountDao, Mockito.atLeastOnce()).update(Mockito.eq(42l), Mockito.any(AccountVO.class)); + Mockito.verify(_accountDao, Mockito.atLeastOnce()).update(Mockito.eq(42L), Mockito.any(AccountVO.class)); } @Test public void deleteUserAccount() { AccountVO account = new AccountVO(); - account.setId(42l); + account.setId(42L); DomainVO domain = new DomainVO(); - Mockito.when(_accountDao.findById(42l)).thenReturn(account); + Mockito.when(_accountDao.findById(42L)).thenReturn(account); Mockito.doNothing().when(accountManagerImpl).checkAccess(Mockito.any(Account.class), Mockito.isNull(), Mockito.anyBoolean(), Mockito.any(Account.class)); - Mockito.when(_accountDao.remove(42l)).thenReturn(true); + Mockito.when(_accountDao.remove(42L)).thenReturn(true); Mockito.when(_configMgr.releaseAccountSpecificVirtualRanges(account)).thenReturn(true); Mockito.lenient().when(_domainMgr.getDomain(Mockito.anyLong())).thenReturn(domain); Mockito.lenient().when(securityChecker.checkAccess(Mockito.any(Account.class), Mockito.any(Domain.class))).thenReturn(true); @@ -194,7 +191,7 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase { List sshkeyList = new ArrayList(); SSHKeyPairVO sshkey = new SSHKeyPairVO(); - sshkey.setId(1l); + sshkey.setId(1L); sshkeyList.add(sshkey); Mockito.when(_sshKeyPairDao.listKeyPairs(Mockito.anyLong(), Mockito.anyLong())).thenReturn(sshkeyList); Mockito.when(_sshKeyPairDao.remove(Mockito.anyLong())).thenReturn(true); @@ -202,30 +199,30 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase { Mockito.doNothing().when(accountManagerImpl).deleteWebhooksForAccount(Mockito.anyLong()); Mockito.doNothing().when(accountManagerImpl).verifyCallerPrivilegeForUserOrAccountOperations((Account) any()); - Assert.assertTrue(accountManagerImpl.deleteUserAccount(42l)); + Assert.assertTrue(accountManagerImpl.deleteUserAccount(42L)); // assert that this was a clean delete - Mockito.verify(_accountDao, Mockito.never()).markForCleanup(Mockito.eq(42l)); + Mockito.verify(_accountDao, Mockito.never()).markForCleanup(Mockito.eq(42L)); } @Test public void deleteUserAccountCleanup() { AccountVO account = new AccountVO(); - account.setId(42l); + account.setId(42L); DomainVO domain = new DomainVO(); - Mockito.when(_accountDao.findById(42l)).thenReturn(account); + Mockito.when(_accountDao.findById(42L)).thenReturn(account); Mockito.doNothing().when(accountManagerImpl).checkAccess(Mockito.any(Account.class), Mockito.isNull(), Mockito.anyBoolean(), Mockito.any(Account.class)); - Mockito.when(_accountDao.remove(42l)).thenReturn(true); + Mockito.when(_accountDao.remove(42L)).thenReturn(true); Mockito.when(_configMgr.releaseAccountSpecificVirtualRanges(account)).thenReturn(true); - Mockito.when(_userVmDao.listByAccountId(42l)).thenReturn(Arrays.asList(Mockito.mock(UserVmVO.class))); + Mockito.when(_userVmDao.listByAccountId(42L)).thenReturn(Arrays.asList(Mockito.mock(UserVmVO.class))); Mockito.when(_vmMgr.expunge(Mockito.any(UserVmVO.class))).thenReturn(false); Mockito.lenient().when(_domainMgr.getDomain(Mockito.anyLong())).thenReturn(domain); Mockito.lenient().when(securityChecker.checkAccess(Mockito.any(Account.class), Mockito.any(Domain.class))).thenReturn(true); Mockito.doNothing().when(accountManagerImpl).deleteWebhooksForAccount(Mockito.anyLong()); Mockito.doNothing().when(accountManagerImpl).verifyCallerPrivilegeForUserOrAccountOperations((Account) any()); - Assert.assertTrue(accountManagerImpl.deleteUserAccount(42l)); + Assert.assertTrue(accountManagerImpl.deleteUserAccount(42L)); // assert that this was NOT a clean delete - Mockito.verify(_accountDao, Mockito.atLeastOnce()).markForCleanup(Mockito.eq(42l)); + Mockito.verify(_accountDao, Mockito.atLeastOnce()).markForCleanup(Mockito.eq(42L)); } @Test (expected = InvalidParameterValueException.class) @@ -308,7 +305,7 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase { UserAccountVO userAccountVO = new UserAccountVO(); userAccountVO.setSource(User.Source.UNKNOWN); userAccountVO.setState(Account.State.DISABLED.toString()); - Mockito.when(userAccountDaoMock.getUserAccount("test", 1L)).thenReturn(userAccountVO); + Mockito.when(userAccountDao.getUserAccount("test", 1L)).thenReturn(userAccountVO); Mockito.when(userAuthenticator.authenticate("test", "fail", 1L, new HashMap<>())).thenReturn(failureAuthenticationPair); Mockito.lenient().when(userAuthenticator.authenticate("test", null, 1L, new HashMap<>())).thenReturn(successAuthenticationPair); Mockito.lenient().when(userAuthenticator.authenticate("test", "", 1L, new HashMap<>())).thenReturn(successAuthenticationPair); @@ -337,7 +334,7 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase { CallContext.register(callingUser, callingAccount); // Calling account is user account i.e normal account Mockito.when(_listkeyscmd.getID()).thenReturn(1L); Mockito.when(accountManagerImpl.getActiveUser(1L)).thenReturn(userVoMock); - Mockito.when(userAccountDaoMock.findById(1L)).thenReturn(userAccountVO); + Mockito.when(userAccountDao.findById(1L)).thenReturn(userAccountVO); Mockito.when(userAccountVO.getAccountId()).thenReturn(1L); Mockito.lenient().when(accountManagerImpl.getAccount(Mockito.anyLong())).thenReturn(accountMock); // Queried account - admin account @@ -355,7 +352,7 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase { CallContext.register(callingUser, callingAccount); Mockito.when(_listkeyscmd.getID()).thenReturn(2L); Mockito.when(accountManagerImpl.getActiveUser(2L)).thenReturn(userVoMock); - Mockito.when(userAccountDaoMock.findById(2L)).thenReturn(userAccountVO); + Mockito.when(userAccountDao.findById(2L)).thenReturn(userAccountVO); Mockito.when(userAccountVO.getAccountId()).thenReturn(2L); Mockito.when(userDetailsDaoMock.listDetailsKeyPairs(Mockito.anyLong())).thenReturn(null); @@ -442,14 +439,14 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase { Mockito.doNothing().when(accountManagerImpl).validateUserPasswordAndUpdateIfNeeded(Mockito.anyString(), Mockito.eq(userVoMock), Mockito.anyString(), Mockito.eq(false)); Mockito.doReturn(true).when(userDaoMock).update(Mockito.anyLong(), Mockito.eq(userVoMock)); - Mockito.doReturn(Mockito.mock(UserAccountVO.class)).when(userAccountDaoMock).findById(Mockito.anyLong()); + Mockito.doReturn(Mockito.mock(UserAccountVO.class)).when(userAccountDao).findById(Mockito.anyLong()); Mockito.doNothing().when(accountManagerImpl).checkAccess(nullable(User.class), nullable(Account.class)); accountManagerImpl.updateUser(UpdateUserCmdMock); Mockito.lenient().doNothing().when(accountManagerImpl).checkRoleEscalation(accountMock, accountMock); - InOrder inOrder = Mockito.inOrder(userVoMock, accountManagerImpl, userDaoMock, userAccountDaoMock); + InOrder inOrder = Mockito.inOrder(userVoMock, accountManagerImpl, userDaoMock, userAccountDao); inOrder.verify(accountManagerImpl).retrieveAndValidateUser(UpdateUserCmdMock); inOrder.verify(accountManagerImpl).retrieveAndValidateAccount(userVoMock); @@ -464,7 +461,7 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase { inOrder.verify(userVoMock, Mockito.times(numberOfExpectedCallsForSetEmailAndSetTimeZone)).setTimezone(Mockito.anyString()); inOrder.verify(userDaoMock).update(Mockito.anyLong(), Mockito.eq(userVoMock)); - inOrder.verify(userAccountDaoMock).findById(Mockito.anyLong()); + inOrder.verify(userAccountDao).findById(Mockito.anyLong()); } @Test(expected = InvalidParameterValueException.class) @@ -487,7 +484,7 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase { public void validateAndUpdatApiAndSecretKeyIfNeededTestNoKeys() { accountManagerImpl.validateAndUpdateApiAndSecretKeyIfNeeded(UpdateUserCmdMock, userVoMock); - Mockito.verify(_accountDao, Mockito.times(0)).findUserAccountByApiKey(Mockito.anyString()); + Mockito.verify(userAccountDao, Mockito.times(0)).getUserByApiKey(Mockito.anyString()); } @Test(expected = InvalidParameterValueException.class) @@ -513,10 +510,9 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase { Mockito.doReturn(1L).when(userVoMock).getId(); User otherUserMock = Mockito.mock(User.class); - Mockito.doReturn(2L).when(otherUserMock).getId(); - Pair pairUserAccountMock = new Pair(otherUserMock, Mockito.mock(Account.class)); - Mockito.doReturn(pairUserAccountMock).when(_accountDao).findUserAccountByApiKey(apiKey); + UserAccount UserAccountMock = Mockito.mock(UserAccount.class); + Mockito.doReturn(UserAccountMock).when(userAccountDao).getUserByApiKey(apiKey); accountManagerImpl.validateAndUpdateApiAndSecretKeyIfNeeded(UpdateUserCmdMock, userVoMock); } @@ -529,17 +525,13 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase { String secretKey = "secretKey"; Mockito.doReturn(secretKey).when(UpdateUserCmdMock).getSecretKey(); - Mockito.doReturn(1L).when(userVoMock).getId(); - User otherUserMock = Mockito.mock(User.class); - Mockito.doReturn(1L).when(otherUserMock).getId(); - Pair pairUserAccountMock = new Pair(otherUserMock, Mockito.mock(Account.class)); - Mockito.doReturn(pairUserAccountMock).when(_accountDao).findUserAccountByApiKey(apiKey); + Mockito.doReturn(null).when(userAccountDao).getUserByApiKey(apiKey); accountManagerImpl.validateAndUpdateApiAndSecretKeyIfNeeded(UpdateUserCmdMock, userVoMock); - Mockito.verify(_accountDao).findUserAccountByApiKey(apiKey); + Mockito.verify(userAccountDao).getUserByApiKey(apiKey); Mockito.verify(userVoMock).setApiKey(apiKey); Mockito.verify(userVoMock).setSecretKey(secretKey); } @@ -693,18 +685,18 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase { @Test(expected = InvalidParameterValueException.class) public void validateAndUpdateUsernameIfNeededTestDuplicatedUserSameDomainThisUser() { - long domanIdCurrentUser = 22l; + long domanIdCurrentUser = 22L; String userName = "username"; Mockito.doReturn(userName).when(UpdateUserCmdMock).getUsername(); Mockito.lenient().doReturn(userName).when(userVoMock).getUsername(); Mockito.doReturn(domanIdCurrentUser).when(accountMock).getDomainId(); - long userVoDuplicatedMockId = 67l; + long userVoDuplicatedMockId = 67L; UserVO userVoDuplicatedMock = Mockito.mock(UserVO.class); Mockito.doReturn(userVoDuplicatedMockId).when(userVoDuplicatedMock).getId(); - long accountIdUserDuplicated = 98l; + long accountIdUserDuplicated = 98L; Mockito.doReturn(accountIdUserDuplicated).when(userVoDuplicatedMock).getAccountId(); @@ -728,24 +720,24 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase { @Test public void validateAndUpdateUsernameIfNeededTestDuplicatedUserButInDifferentDomains() { - long domanIdCurrentUser = 22l; + long domanIdCurrentUser = 22L; String userName = "username"; Mockito.doReturn(userName).when(UpdateUserCmdMock).getUsername(); Mockito.lenient().doReturn(userName).when(userVoMock).getUsername(); Mockito.doReturn(domanIdCurrentUser).when(accountMock).getDomainId(); - long userVoDuplicatedMockId = 67l; + long userVoDuplicatedMockId = 67L; UserVO userVoDuplicatedMock = Mockito.mock(UserVO.class); Mockito.lenient().doReturn(userName).when(userVoDuplicatedMock).getUsername(); Mockito.doReturn(userVoDuplicatedMockId).when(userVoDuplicatedMock).getId(); - long accountIdUserDuplicated = 98l; + long accountIdUserDuplicated = 98L; Mockito.doReturn(accountIdUserDuplicated).when(userVoDuplicatedMock).getAccountId(); Account accountUserDuplicatedMock = Mockito.mock(AccountVO.class); Mockito.lenient().doReturn(accountIdUserDuplicated).when(accountUserDuplicatedMock).getId(); - Mockito.doReturn(45l).when(accountUserDuplicatedMock).getDomainId(); + Mockito.doReturn(45L).when(accountUserDuplicatedMock).getDomainId(); List usersWithSameUserName = new ArrayList<>(); usersWithSameUserName.add(userVoMock); @@ -763,7 +755,7 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase { @Test public void validateAndUpdateUsernameIfNeededTestNoDuplicatedUserNames() { - long domanIdCurrentUser = 22l; + long domanIdCurrentUser = 22L; String userName = "username"; Mockito.doReturn(userName).when(UpdateUserCmdMock).getUsername(); @@ -961,7 +953,7 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase { @Test public void validateCurrentPasswordTestUserAuthenticatedWithProvidedCurrentPasswordViaFirstAuthenticator() { AccountVO accountVoMock = Mockito.mock(AccountVO.class); - long domainId = 14l; + long domainId = 14L; Mockito.doReturn(domainId).when(accountVoMock).getDomainId(); Mockito.doReturn(accountVoMock).when(_accountDao).findById(accountMockId); @@ -990,7 +982,7 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase { @Test public void validateCurrentPasswordTestUserAuthenticatedWithProvidedCurrentPasswordViaSecondAuthenticator() { AccountVO accountVoMock = Mockito.mock(AccountVO.class); - long domainId = 14l; + long domainId = 14L; Mockito.doReturn(domainId).when(accountVoMock).getDomainId(); Mockito.doReturn(accountVoMock).when(_accountDao).findById(accountMockId); @@ -1051,7 +1043,7 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase { UserAccountVO userAccount = Mockito.mock(UserAccountVO.class); UserVO userVO = Mockito.mock(UserVO.class); - Mockito.when(userAccountDaoMock.findById(userId)).thenReturn(userAccount); + Mockito.when(userAccountDao.findById(userId)).thenReturn(userAccount); Mockito.when(userDaoMock.findById(userId)).thenReturn(userVO); Mockito.when(userAccount.getDomainId()).thenReturn(1L); @@ -1070,7 +1062,7 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase { UserAccountVO userAccount = Mockito.mock(UserAccountVO.class); UserVO userVO = Mockito.mock(UserVO.class); - Mockito.when(userAccountDaoMock.findById(userId)).thenReturn(userAccount); + Mockito.when(userAccountDao.findById(userId)).thenReturn(userAccount); Mockito.when(userDaoMock.findById(userId)).thenReturn(userVO); Mockito.when(userAccount.getDomainId()).thenReturn(1L); @@ -1099,7 +1091,7 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase { UserAccountVO userAccount = Mockito.mock(UserAccountVO.class); UserVO userVO = Mockito.mock(UserVO.class); - Mockito.when(userAccountDaoMock.findById(userId)).thenReturn(userAccount); + Mockito.when(userAccountDao.findById(userId)).thenReturn(userAccount); Mockito.when(userDaoMock.findById(userId)).thenReturn(userVO); Mockito.when(userAccount.getDomainId()).thenReturn(1L); @@ -1205,7 +1197,7 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase { Mockito.when(callingUser.getId()).thenReturn(1L); CallContext.register(callingUser, callingAccount); // Calling account is user account i.e normal account Mockito.lenient().when(_accountService.getActiveAccountById(1L)).thenReturn(accountMock); - Mockito.when(userAccountDaoMock.findById(1L)).thenReturn(userAccountVO); + Mockito.when(userAccountDao.findById(1L)).thenReturn(userAccountVO); Mockito.when(userDaoMock.findById(1L)).thenReturn(userVoMock); Mockito.when(userAccountVO.getDomainId()).thenReturn(1L); Mockito.when(enableUserTwoFactorAuthenticationMock.valueIn(1L)).thenReturn(true); @@ -1231,7 +1223,7 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase { List userAccountVOList = new ArrayList<>(); UserAccountVO userAccountVO = new UserAccountVO(); userAccountVOList.add(userAccountVO); - Mockito.when(userAccountDaoMock.getUserAccountByEmail(email, domainId)).thenReturn(userAccountVOList); + Mockito.when(userAccountDao.getUserAccountByEmail(email, domainId)).thenReturn(userAccountVOList); List userAccounts = accountManagerImpl.getActiveUserAccountByEmail(email, domainId); Assert.assertEquals(userAccountVOList.size(), userAccounts.size()); Assert.assertEquals(userAccountVOList.get(0), userAccounts.get(0)); @@ -1406,7 +1398,7 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase { Mockito.when(user.getUser2faProvider()).thenReturn(null); UserAccount result = accountManagerImpl.clearUserTwoFactorAuthenticationInSetupStateOnLogin(user); Assert.assertSame(user, result); - Mockito.verifyNoInteractions(userDetailsDaoMock, userAccountDaoMock); + Mockito.verifyNoInteractions(userDetailsDaoMock, userAccountDao); } @Test @@ -1420,7 +1412,7 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase { UserAccount result = accountManagerImpl.clearUserTwoFactorAuthenticationInSetupStateOnLogin(user); Assert.assertSame(user, result); Mockito.verify(userDetailsDaoMock).findDetail(1L, UserDetailVO.Setup2FADetail); - Mockito.verifyNoMoreInteractions(userDetailsDaoMock, userAccountDaoMock); + Mockito.verifyNoMoreInteractions(userDetailsDaoMock, userAccountDao); } @Test @@ -1433,16 +1425,16 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase { UserAccountVO userAccountVO = new UserAccountVO(); userAccountVO.setId(1L); Mockito.when(userDetailsDaoMock.findDetail(1L, UserDetailVO.Setup2FADetail)).thenReturn(userDetail); - Mockito.when(userAccountDaoMock.findById(1L)).thenReturn(userAccountVO); + Mockito.when(userAccountDao.findById(any())).thenReturn(userAccountVO); UserAccount result = accountManagerImpl.clearUserTwoFactorAuthenticationInSetupStateOnLogin(user); Assert.assertNotNull(result); Assert.assertFalse(result.isUser2faEnabled()); Assert.assertNull(result.getUser2faProvider()); Mockito.verify(userDetailsDaoMock).findDetail(1L, UserDetailVO.Setup2FADetail); Mockito.verify(userDetailsDaoMock).remove(Mockito.anyLong()); - Mockito.verify(userAccountDaoMock).findById(1L); + Mockito.verify(userAccountDao).findById(1L); ArgumentCaptor captor = ArgumentCaptor.forClass(UserAccountVO.class); - Mockito.verify(userAccountDaoMock).update(Mockito.eq(1L), captor.capture()); + Mockito.verify(userAccountDao).update(Mockito.eq(1L), captor.capture()); UserAccountVO updatedUser = captor.getValue(); Assert.assertFalse(updatedUser.isUser2faEnabled()); Assert.assertNull(updatedUser.getUser2faProvider()); diff --git a/server/src/test/java/com/cloud/user/AccountManagerImplVolumeDeleteEventTest.java b/server/src/test/java/com/cloud/user/AccountManagerImplVolumeDeleteEventTest.java index 6d69890c9a5..3f13d9dd024 100644 --- a/server/src/test/java/com/cloud/user/AccountManagerImplVolumeDeleteEventTest.java +++ b/server/src/test/java/com/cloud/user/AccountManagerImplVolumeDeleteEventTest.java @@ -63,7 +63,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @RunWith(MockitoJUnitRunner.class) -public class AccountManagerImplVolumeDeleteEventTest extends AccountManagetImplTestBase { +public class AccountManagerImplVolumeDeleteEventTest extends AccountManagentImplTestBase { private static final Long ACCOUNT_ID = 1l; private static final String VOLUME_UUID = "vol-111111"; diff --git a/server/src/test/java/com/cloud/vm/snapshot/VMSnapshotManagerTest.java b/server/src/test/java/com/cloud/vm/snapshot/VMSnapshotManagerTest.java index a666d0aff7c..a5a452dd2dc 100644 --- a/server/src/test/java/com/cloud/vm/snapshot/VMSnapshotManagerTest.java +++ b/server/src/test/java/com/cloud/vm/snapshot/VMSnapshotManagerTest.java @@ -51,6 +51,7 @@ import com.cloud.vm.UserVmManager; import com.cloud.vm.UserVmVO; import com.cloud.vm.VirtualMachine.State; import com.cloud.vm.VirtualMachineManager; +import com.cloud.vm.VmDetailConstants; import com.cloud.vm.dao.UserVmDao; import com.cloud.vm.dao.UserVmDetailsDao; import com.cloud.vm.dao.VMInstanceDao; @@ -67,7 +68,6 @@ import org.junit.Before; import org.junit.Test; import org.mockito.ArgumentCaptor; import org.mockito.Captor; -import org.mockito.ArgumentMatchers; import org.mockito.Mock; import org.mockito.MockitoAnnotations; import org.mockito.Spy; @@ -79,13 +79,18 @@ import java.util.List; import java.util.Map; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -225,13 +230,13 @@ public class VMSnapshotManagerTest { when(_serviceOfferingDao.findById(SERVICE_OFFERING_ID)).thenReturn(serviceOffering); for (ResourceDetail detail : Arrays.asList(userVmDetailCpuNumber, vmSnapshotDetailCpuNumber)) { - when(detail.getName()).thenReturn("cpuNumber"); + when(detail.getName()).thenReturn(VmDetailConstants.CPU_NUMBER); when(detail.getValue()).thenReturn("2"); when(detail.isDisplay()).thenReturn(true); } for (ResourceDetail detail : Arrays.asList(userVmDetailMemory, vmSnapshotDetailMemory)) { - when(detail.getName()).thenReturn("memory"); + when(detail.getName()).thenReturn(VmDetailConstants.MEMORY); when(detail.getValue()).thenReturn("2048"); when(detail.isDisplay()).thenReturn(true); } @@ -348,12 +353,12 @@ public class VMSnapshotManagerTest { @Test public void testUpdateUserVmServiceOfferingDifferentServiceOffering() throws ConcurrentOperationException, ResourceUnavailableException, ManagementServerException, VirtualMachineMigrationException { when(userVm.getServiceOfferingId()).thenReturn(SERVICE_OFFERING_DIFFERENT_ID); - when(_userVmManager.upgradeVirtualMachine(ArgumentMatchers.eq(TEST_VM_ID), ArgumentMatchers.eq(SERVICE_OFFERING_ID), mapDetailsCaptor.capture())).thenReturn(true); + when(_userVmManager.upgradeVirtualMachine(eq(TEST_VM_ID), eq(SERVICE_OFFERING_ID), mapDetailsCaptor.capture())).thenReturn(true); _vmSnapshotMgr.updateUserVmServiceOffering(userVm, vmSnapshotVO); verify(_vmSnapshotMgr).changeUserVmServiceOffering(userVm, vmSnapshotVO); verify(_vmSnapshotMgr).getVmMapDetails(userVm); - verify(_vmSnapshotMgr).upgradeUserVmServiceOffering(ArgumentMatchers.eq(userVm), ArgumentMatchers.eq(SERVICE_OFFERING_ID), mapDetailsCaptor.capture()); + verify(_vmSnapshotMgr).upgradeUserVmServiceOffering(eq(userVm), eq(SERVICE_OFFERING_ID), mapDetailsCaptor.capture()); } @Test @@ -368,18 +373,18 @@ public class VMSnapshotManagerTest { @Test public void testChangeUserVmServiceOffering() throws ConcurrentOperationException, ResourceUnavailableException, ManagementServerException, VirtualMachineMigrationException { - when(_userVmManager.upgradeVirtualMachine(ArgumentMatchers.eq(TEST_VM_ID), ArgumentMatchers.eq(SERVICE_OFFERING_ID), mapDetailsCaptor.capture())).thenReturn(true); + when(_userVmManager.upgradeVirtualMachine(eq(TEST_VM_ID), eq(SERVICE_OFFERING_ID), mapDetailsCaptor.capture())).thenReturn(true); _vmSnapshotMgr.changeUserVmServiceOffering(userVm, vmSnapshotVO); verify(_vmSnapshotMgr).getVmMapDetails(userVm); - verify(_vmSnapshotMgr).upgradeUserVmServiceOffering(ArgumentMatchers.eq(userVm), ArgumentMatchers.eq(SERVICE_OFFERING_ID), mapDetailsCaptor.capture()); + verify(_vmSnapshotMgr).upgradeUserVmServiceOffering(eq(userVm), eq(SERVICE_OFFERING_ID), mapDetailsCaptor.capture()); } @Test(expected=CloudRuntimeException.class) public void testChangeUserVmServiceOfferingFailOnUpgradeVMServiceOffering() throws ConcurrentOperationException, ResourceUnavailableException, ManagementServerException, VirtualMachineMigrationException { - when(_userVmManager.upgradeVirtualMachine(ArgumentMatchers.eq(TEST_VM_ID), ArgumentMatchers.eq(SERVICE_OFFERING_ID), mapDetailsCaptor.capture())).thenReturn(false); + when(_userVmManager.upgradeVirtualMachine(eq(TEST_VM_ID), eq(SERVICE_OFFERING_ID), mapDetailsCaptor.capture())).thenReturn(false); _vmSnapshotMgr.changeUserVmServiceOffering(userVm, vmSnapshotVO); verify(_vmSnapshotMgr).getVmMapDetails(userVm); - verify(_vmSnapshotMgr).upgradeUserVmServiceOffering(ArgumentMatchers.eq(userVm), ArgumentMatchers.eq(SERVICE_OFFERING_ID), mapDetailsCaptor.capture()); + verify(_vmSnapshotMgr).upgradeUserVmServiceOffering(eq(userVm), eq(SERVICE_OFFERING_ID), mapDetailsCaptor.capture()); } @Test @@ -395,16 +400,27 @@ public class VMSnapshotManagerTest { @Test public void testRevertUserVmDetailsFromVmSnapshotNotDynamicServiceOffering() { - _vmSnapshotMgr.revertUserVmDetailsFromVmSnapshot(vmMock, vmSnapshotVO); + _vmSnapshotMgr.revertCustomServiceOfferingDetailsFromVmSnapshot(vmMock, vmSnapshotVO); verify(_vmSnapshotDetailsDao, never()).listDetails(anyLong()); } @Test public void testRevertUserVmDetailsFromVmSnapshotDynamicServiceOffering() { when(serviceOffering.isDynamic()).thenReturn(true); - _vmSnapshotMgr.revertUserVmDetailsFromVmSnapshot(vmMock, vmSnapshotVO); - verify(_vmSnapshotDetailsDao).listDetails(VM_SNAPSHOT_ID); - verify(_userVmDetailsDao).saveDetails(listUserVmDetailsCaptor.capture()); - } + VMSnapshotDetailsVO uefiSnapshotDetail = new VMSnapshotDetailsVO(VM_SNAPSHOT_ID, "UEFI", "SECURE", true); + List snapshotDetailsWithUefi = Arrays.asList( + vmSnapshotDetailCpuNumber, vmSnapshotDetailMemory, uefiSnapshotDetail); + when(_vmSnapshotDetailsDao.listDetails(VM_SNAPSHOT_ID)).thenReturn(snapshotDetailsWithUefi); + _vmSnapshotMgr.revertCustomServiceOfferingDetailsFromVmSnapshot(vmMock, vmSnapshotVO); + + verify(_vmSnapshotDetailsDao).listDetails(VM_SNAPSHOT_ID); + verify(_userVmDetailsDao, never()).saveDetails(any()); + ArgumentCaptor detailNameCaptor = ArgumentCaptor.forClass(String.class); + verify(_userVmDetailsDao, times(2)).addDetail(eq(TEST_VM_ID), detailNameCaptor.capture(), anyString(), anyBoolean()); + List appliedNames = detailNameCaptor.getAllValues(); + assertTrue(appliedNames.contains(VmDetailConstants.CPU_NUMBER)); + assertTrue(appliedNames.contains(VmDetailConstants.MEMORY)); + assertFalse("UEFI must not be applied from snapshot so that existing UEFI setting is preserved", appliedNames.contains("UEFI")); + } } diff --git a/server/src/test/java/org/apache/cloudstack/storage/heuristics/HeuristicRuleHelperTest.java b/server/src/test/java/org/apache/cloudstack/storage/heuristics/HeuristicRuleHelperTest.java index 272e79fea49..032e947fdce 100644 --- a/server/src/test/java/org/apache/cloudstack/storage/heuristics/HeuristicRuleHelperTest.java +++ b/server/src/test/java/org/apache/cloudstack/storage/heuristics/HeuristicRuleHelperTest.java @@ -16,6 +16,8 @@ // under the License. package org.apache.cloudstack.storage.heuristics; +import com.cloud.hypervisor.Hypervisor; +import com.cloud.storage.Storage; import com.cloud.storage.VMTemplateVO; import com.cloud.storage.VolumeVO; import com.cloud.utils.exception.CloudRuntimeException; @@ -29,6 +31,7 @@ import org.apache.cloudstack.storage.heuristics.presetvariables.PresetVariables; import org.apache.cloudstack.utils.jsinterpreter.JsInterpreter; import org.apache.logging.log4j.Logger; import org.junit.Assert; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.InjectMocks; @@ -68,6 +71,19 @@ public class HeuristicRuleHelperTest { @InjectMocks HeuristicRuleHelper heuristicRuleHelperSpy = new HeuristicRuleHelper(); + @Before + public void setUp() { + Mockito.doReturn("template-name").when(vmTemplateVOMock).getName(); + Mockito.doReturn(Storage.ImageFormat.QCOW2).when(vmTemplateVOMock).getFormat(); + Mockito.doReturn(Hypervisor.HypervisorType.KVM).when(vmTemplateVOMock).getHypervisorType(); + Mockito.doReturn("snapshot-name").when(snapshotInfoMock).getName(); + Mockito.doReturn(1024L).when(snapshotInfoMock).getSize(); + Mockito.doReturn(Hypervisor.HypervisorType.VMware).when(snapshotInfoMock).getHypervisorType(); + Mockito.doReturn("volume-name").when(volumeVOMock).getName(); + Mockito.doReturn(Storage.ImageFormat.RAW).when(volumeVOMock).getFormat(); + Mockito.doReturn(2048L).when(volumeVOMock).getSize(); + } + @Test public void getImageStoreIfThereIsHeuristicRuleTestZoneDoesNotHaveHeuristicRuleShouldReturnNull() { Long zoneId = 1L; diff --git a/server/src/test/java/org/apache/cloudstack/storage/heuristics/presetvariables/AccountTest.java b/server/src/test/java/org/apache/cloudstack/storage/heuristics/presetvariables/AccountTest.java deleted file mode 100644 index f7610438b78..00000000000 --- a/server/src/test/java/org/apache/cloudstack/storage/heuristics/presetvariables/AccountTest.java +++ /dev/null @@ -1,46 +0,0 @@ -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you under the Apache License, Version 2.0 (the -// "License"); you may not use this file except in compliance -// with the License. You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. - -package org.apache.cloudstack.storage.heuristics.presetvariables; - -import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils; -import org.junit.Assert; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.junit.MockitoJUnitRunner; - -@RunWith(MockitoJUnitRunner.class) -public class AccountTest { - - @Test - public void toStringTestReturnsValidJson() { - Account variable = new Account(); - variable.setName("test name"); - variable.setId("test id"); - - Domain domainVariable = new Domain(); - domainVariable.setId("domain id"); - domainVariable.setName("domain name"); - variable.setDomain(domainVariable); - - String expected = ReflectionToStringBuilderUtils.reflectOnlySelectedFields(variable, "name", "id", "domain"); - String result = variable.toString(); - - Assert.assertEquals(expected, result); - } - -} diff --git a/server/src/test/java/org/apache/cloudstack/storage/heuristics/presetvariables/DomainTest.java b/server/src/test/java/org/apache/cloudstack/storage/heuristics/presetvariables/DomainTest.java deleted file mode 100644 index a1ec6854ecc..00000000000 --- a/server/src/test/java/org/apache/cloudstack/storage/heuristics/presetvariables/DomainTest.java +++ /dev/null @@ -1,41 +0,0 @@ -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you under the Apache License, Version 2.0 (the -// "License"); you may not use this file except in compliance -// with the License. You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. - -package org.apache.cloudstack.storage.heuristics.presetvariables; - -import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils; -import org.junit.Assert; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.junit.MockitoJUnitRunner; - -@RunWith(MockitoJUnitRunner.class) -public class DomainTest { - - @Test - public void toStringTestReturnsValidJson() { - Domain variable = new Domain(); - variable.setName("test name"); - variable.setId("test id"); - - String expected = ReflectionToStringBuilderUtils.reflectOnlySelectedFields(variable, "name", "id"); - String result = variable.toString(); - - Assert.assertEquals(expected, result); - } - -} diff --git a/server/src/test/java/org/apache/cloudstack/storage/heuristics/presetvariables/GenericHeuristicPresetVariableTest.java b/server/src/test/java/org/apache/cloudstack/storage/heuristics/presetvariables/GenericHeuristicPresetVariableTest.java deleted file mode 100644 index cd295e92caf..00000000000 --- a/server/src/test/java/org/apache/cloudstack/storage/heuristics/presetvariables/GenericHeuristicPresetVariableTest.java +++ /dev/null @@ -1,40 +0,0 @@ -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you under the Apache License, Version 2.0 (the -// "License"); you may not use this file except in compliance -// with the License. You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. - -package org.apache.cloudstack.storage.heuristics.presetvariables; - -import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils; -import org.junit.Assert; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.junit.MockitoJUnitRunner; - -@RunWith(MockitoJUnitRunner.class) -public class GenericHeuristicPresetVariableTest { - - @Test - public void toStringTestReturnsValidJson() { - GenericHeuristicPresetVariable variable = new GenericHeuristicPresetVariable(); - variable.setName("test name"); - - String expected = ReflectionToStringBuilderUtils.reflectOnlySelectedFields(variable, "name"); - String result = variable.toString(); - - Assert.assertEquals(expected, result); - } - -} diff --git a/server/src/test/java/org/apache/cloudstack/storage/heuristics/presetvariables/SecondaryStorageTest.java b/server/src/test/java/org/apache/cloudstack/storage/heuristics/presetvariables/SecondaryStorageTest.java deleted file mode 100644 index a09386789cf..00000000000 --- a/server/src/test/java/org/apache/cloudstack/storage/heuristics/presetvariables/SecondaryStorageTest.java +++ /dev/null @@ -1,45 +0,0 @@ -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you under the Apache License, Version 2.0 (the -// "License"); you may not use this file except in compliance -// with the License. You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. - -package org.apache.cloudstack.storage.heuristics.presetvariables; - -import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils; -import org.junit.Assert; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.junit.MockitoJUnitRunner; - -@RunWith(MockitoJUnitRunner.class) -public class SecondaryStorageTest { - - @Test - public void toStringTestReturnsValidJson() { - SecondaryStorage variable = new SecondaryStorage(); - variable.setName("test name"); - variable.setId("test id"); - variable.setProtocol("test protocol"); - variable.setUsedDiskSize(1L); - variable.setTotalDiskSize(2L); - - String expected = ReflectionToStringBuilderUtils.reflectOnlySelectedFields(variable, "name", "id", - "protocol", "usedDiskSize", "totalDiskSize"); - String result = variable.toString(); - - Assert.assertEquals(expected, result); - } - -} diff --git a/server/src/test/java/org/apache/cloudstack/storage/heuristics/presetvariables/SnapshotTest.java b/server/src/test/java/org/apache/cloudstack/storage/heuristics/presetvariables/SnapshotTest.java deleted file mode 100644 index b8476cd8e46..00000000000 --- a/server/src/test/java/org/apache/cloudstack/storage/heuristics/presetvariables/SnapshotTest.java +++ /dev/null @@ -1,44 +0,0 @@ -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you under the Apache License, Version 2.0 (the -// "License"); you may not use this file except in compliance -// with the License. You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. - -package org.apache.cloudstack.storage.heuristics.presetvariables; - -import com.cloud.hypervisor.Hypervisor; -import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils; -import org.junit.Assert; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.junit.MockitoJUnitRunner; - -@RunWith(MockitoJUnitRunner.class) -public class SnapshotTest { - - @Test - public void toStringTestReturnsValidJson() { - Snapshot variable = new Snapshot(); - variable.setName("test name"); - variable.setSize(1L); - variable.setHypervisorType(Hypervisor.HypervisorType.KVM); - - String expected = ReflectionToStringBuilderUtils.reflectOnlySelectedFields(variable, "name", "size", - "hypervisorType"); - String result = variable.toString(); - - Assert.assertEquals(expected, result); - } - -} diff --git a/server/src/test/java/org/apache/cloudstack/storage/heuristics/presetvariables/TemplateTest.java b/server/src/test/java/org/apache/cloudstack/storage/heuristics/presetvariables/TemplateTest.java deleted file mode 100644 index 2c1582befb2..00000000000 --- a/server/src/test/java/org/apache/cloudstack/storage/heuristics/presetvariables/TemplateTest.java +++ /dev/null @@ -1,46 +0,0 @@ -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you under the Apache License, Version 2.0 (the -// "License"); you may not use this file except in compliance -// with the License. You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. - -package org.apache.cloudstack.storage.heuristics.presetvariables; - -import com.cloud.hypervisor.Hypervisor; -import com.cloud.storage.Storage; -import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils; -import org.junit.Assert; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.junit.MockitoJUnitRunner; - -@RunWith(MockitoJUnitRunner.class) -public class TemplateTest { - - @Test - public void toStringTestReturnsValidJson() { - Template variable = new Template(); - variable.setName("test name"); - variable.setTemplateType(Storage.TemplateType.USER); - variable.setHypervisorType(Hypervisor.HypervisorType.KVM); - variable.setFormat(Storage.ImageFormat.QCOW2); - - String expected = ReflectionToStringBuilderUtils.reflectOnlySelectedFields(variable, "name", "templateType", - "hypervisorType", "format"); - String result = variable.toString(); - - Assert.assertEquals(expected, result); - } - -} diff --git a/server/src/test/java/org/apache/cloudstack/storage/heuristics/presetvariables/VolumeTest.java b/server/src/test/java/org/apache/cloudstack/storage/heuristics/presetvariables/VolumeTest.java deleted file mode 100644 index e74ddc93ec5..00000000000 --- a/server/src/test/java/org/apache/cloudstack/storage/heuristics/presetvariables/VolumeTest.java +++ /dev/null @@ -1,44 +0,0 @@ -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you under the Apache License, Version 2.0 (the -// "License"); you may not use this file except in compliance -// with the License. You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. - -package org.apache.cloudstack.storage.heuristics.presetvariables; - -import com.cloud.storage.Storage; -import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils; -import org.junit.Assert; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.junit.MockitoJUnitRunner; - -@RunWith(MockitoJUnitRunner.class) -public class VolumeTest { - - @Test - public void toStringTestReturnsValidJson() { - Volume variable = new Volume(); - variable.setName("test name"); - variable.setFormat(Storage.ImageFormat.QCOW2); - variable.setSize(1L); - - String expected = ReflectionToStringBuilderUtils.reflectOnlySelectedFields(variable, "name", "format", - "size"); - String result = variable.toString(); - - Assert.assertEquals(expected, result); - } - -} diff --git a/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/template/UploadManagerImpl.java b/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/template/UploadManagerImpl.java index 828f61f89dc..e98791822d0 100644 --- a/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/template/UploadManagerImpl.java +++ b/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/template/UploadManagerImpl.java @@ -16,7 +16,10 @@ // under the License. package org.apache.cloudstack.storage.template; +import static com.cloud.utils.NumbersUtil.toHumanReadableSize; + import java.io.File; +import java.io.IOException; import java.net.URI; import java.net.URISyntaxException; import java.nio.file.Path; @@ -30,10 +33,10 @@ import java.util.concurrent.Executors; import javax.naming.ConfigurationException; -import com.cloud.agent.api.Answer; - import org.apache.cloudstack.storage.resource.SecondaryStorageResource; +import org.apache.commons.lang3.StringUtils; +import com.cloud.agent.api.Answer; import com.cloud.agent.api.storage.CreateEntityDownloadURLAnswer; import com.cloud.agent.api.storage.CreateEntityDownloadURLCommand; import com.cloud.agent.api.storage.DeleteEntityDownloadURLCommand; @@ -48,15 +51,18 @@ import com.cloud.storage.template.FtpTemplateUploader; import com.cloud.storage.template.TemplateUploader; import com.cloud.storage.template.TemplateUploader.Status; import com.cloud.storage.template.TemplateUploader.UploadCompleteCallback; +import com.cloud.utils.FileUtil; import com.cloud.utils.NumbersUtil; +import com.cloud.utils.UuidUtils; import com.cloud.utils.component.ManagerBase; import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.utils.script.Script; -import static com.cloud.utils.NumbersUtil.toHumanReadableSize; - public class UploadManagerImpl extends ManagerBase implements UploadManager { + protected static final String EXTRACT_USERDATA_DIR = "userdata"; + protected static final String BASE_EXTRACT_PATH = String.format("/var/www/html/%s/", EXTRACT_USERDATA_DIR); + public class Completion implements UploadCompleteCallback { private final String jobId; @@ -266,7 +272,7 @@ public class UploadManagerImpl extends ManagerBase implements UploadManager { return new CreateEntityDownloadURLAnswer(errorString, CreateEntityDownloadURLAnswer.RESULT_FAILURE); } // Create the directory structure so that its visible under apache server root - String extractDir = "/var/www/html/userdata/"; + String extractDir = BASE_EXTRACT_PATH; extractDir = extractDir + cmd.getFilepathInExtractURL() + File.separator; Script command = new Script("/bin/su", logger); command.add("-s"); @@ -330,12 +336,20 @@ public class UploadManagerImpl extends ManagerBase implements UploadManager { String extractUrl = cmd.getExtractUrl(); String result; if (extractUrl != null) { - command.add("unlink /var/www/html/userdata/" + extractUrl.substring(extractUrl.lastIndexOf(File.separator) + 1)); + URI uri = URI.create(extractUrl); + String uriPath = uri.getPath(); + String marker = String.format("/%s/", EXTRACT_USERDATA_DIR); + String linkPath = uriPath.startsWith(marker) + ? uriPath.substring(marker.length()) + : uriPath.substring(uriPath.indexOf(marker) + marker.length()); + command.add("unlink " + BASE_EXTRACT_PATH + linkPath); result = command.execute(); if (result != null) { // FIXME - Ideally should bail out if you can't delete symlink. Not doing it right now. // This is because the ssvm might already be destroyed and the symlinks do not exist. logger.warn("Error in deleting symlink :" + result); + } else { + deleteEntitySymlinkRootDirectoryIfNeeded(cmd, linkPath); } } @@ -356,6 +370,30 @@ public class UploadManagerImpl extends ManagerBase implements UploadManager { return new Answer(cmd, true, ""); } + protected void deleteEntitySymlinkRootDirectoryIfNeeded(DeleteEntityDownloadURLCommand cmd, String linkPath) { + if (StringUtils.isEmpty(linkPath)) { + return; + } + String[] parts = linkPath.split("/"); + if (parts.length == 0) { + return; + } + String rootDir = parts[0]; + if (StringUtils.isEmpty(rootDir) || !UuidUtils.isUuid(rootDir)) { + return; + } + logger.info("Deleting symlink root directory: {} for {}", rootDir, cmd.getExtractUrl()); + Path rootDirPath = Path.of(BASE_EXTRACT_PATH, rootDir); + String failMsg = "Failed to delete symlink root directory: {} for {}"; + try { + if (!FileUtil.deleteRecursively(rootDirPath)) { + logger.warn(failMsg, rootDir, cmd.getExtractUrl()); + } + } catch (IOException e) { + logger.warn(failMsg, rootDir, cmd.getExtractUrl(), e); + } + } + private String getInstallPath(String jobId) { // TODO Auto-generated method stub return null; diff --git a/services/secondary-storage/server/src/test/java/org/apache/cloudstack/storage/template/UploadManagerImplTest.java b/services/secondary-storage/server/src/test/java/org/apache/cloudstack/storage/template/UploadManagerImplTest.java new file mode 100644 index 00000000000..9038098e235 --- /dev/null +++ b/services/secondary-storage/server/src/test/java/org/apache/cloudstack/storage/template/UploadManagerImplTest.java @@ -0,0 +1,85 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package org.apache.cloudstack.storage.template; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; + +import java.nio.file.Path; + +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.InjectMocks; +import org.mockito.MockedStatic; +import org.mockito.Mockito; +import org.mockito.junit.MockitoJUnitRunner; + +import com.cloud.agent.api.storage.DeleteEntityDownloadURLCommand; +import com.cloud.utils.FileUtil; + +@RunWith(MockitoJUnitRunner.class) +public class UploadManagerImplTest { + + @InjectMocks + UploadManagerImpl uploadManager; + + MockedStatic fileUtilMock; + + @Before + public void setup() { + fileUtilMock = mockStatic(FileUtil.class, Mockito.CALLS_REAL_METHODS); + fileUtilMock.when(() -> FileUtil.deleteRecursively(any(Path.class))).thenReturn(true); + } + + @After + public void tearDown() { + fileUtilMock.close(); + } + + @Test + public void doesNotDeleteWhenLinkPathIsEmpty() { + String emptyLinkPath = ""; + uploadManager.deleteEntitySymlinkRootDirectoryIfNeeded(mock(DeleteEntityDownloadURLCommand.class), emptyLinkPath); + fileUtilMock.verify(() -> FileUtil.deleteRecursively(any(Path.class)), never()); + } + + @Test + public void doesNotDeleteWhenRootDirIsNotUuid() { + String invalidLinkPath = "invalidRootDir/file"; + uploadManager.deleteEntitySymlinkRootDirectoryIfNeeded(mock(DeleteEntityDownloadURLCommand.class), invalidLinkPath); + fileUtilMock.verify(() -> FileUtil.deleteRecursively(any(Path.class)), never()); + } + + @Test + public void deletesSymlinkRootDirectoryWhenValidUuid() { + String validLinkPath = "123e4567-e89b-12d3-a456-426614174000/file"; + uploadManager.deleteEntitySymlinkRootDirectoryIfNeeded(mock(DeleteEntityDownloadURLCommand.class), validLinkPath); + fileUtilMock.verify(() -> FileUtil.deleteRecursively(any(Path.class)), times(1)); + } + + @Test + public void deletesSymlinkRootDirectoryWhenNoFile() { + String validLinkPath = "123e4567-e89b-12d3-a456-426614174000"; + uploadManager.deleteEntitySymlinkRootDirectoryIfNeeded(mock(DeleteEntityDownloadURLCommand.class), validLinkPath); + fileUtilMock.verify(() -> FileUtil.deleteRecursively(any(Path.class)), times(1)); + } +} diff --git a/tools/checkstyle/src/main/resources/cloud-pmd.xml b/tools/checkstyle/src/main/resources/cloud-pmd.xml index 66a4ec08294..78d394ea15e 100644 --- a/tools/checkstyle/src/main/resources/cloud-pmd.xml +++ b/tools/checkstyle/src/main/resources/cloud-pmd.xml @@ -19,11 +19,8 @@ under the License. --> - + Ruleset that brings all the rulesets we want from the pmd jar, because @@ -31,16 +28,16 @@ to add our own future rulesets, if any. - - - - - + + + + + - + @@ -50,35 +47,35 @@ - + - + - - - - - - - + + + + + + + - + - - - - - - - + + + + + + + diff --git a/ui/public/locales/en.json b/ui/public/locales/en.json index 99873820d53..673f6da0ad1 100644 --- a/ui/public/locales/en.json +++ b/ui/public/locales/en.json @@ -935,6 +935,7 @@ "label.endpoint": "Endpoint", "label.endport": "End port", "label.enter.account.name": "Enter the account name", +"label.enter.domain.name": "Enter the domain name", "label.enter.code": "Enter 2FA code to verify", "label.enter.static.pin": "Enter static PIN to verify", "label.enter.token": "Enter token", @@ -2973,6 +2974,9 @@ "message.delete.account.processing": "Deleting account", "message.delete.account.success": "Successfully deleted account", "message.delete.account.warning": "Deleting this account will delete all of the instances, volumes and snapshots associated with the account.", +"message.delete.domain.confirm": "Please confirm that you want to delete this domain by entering the name of the domain below.", +"message.delete.domain.warning": "All associated accounts, users, VMs, and sub-domains will be permanently deleted. This action cannot be undone.", +"message.delete.domain.failed": "Delete domain failed", "message.delete.acl.processing": "Removing ACL rule...", "message.delete.acl.rule": "Remove ACL rule", "message.delete.acl.rule.failed": "Failed to remove ACL rule.", diff --git a/ui/src/components/view/DomainDeleteConfirm.vue b/ui/src/components/view/DomainDeleteConfirm.vue new file mode 100644 index 00000000000..4fdb9b658ad --- /dev/null +++ b/ui/src/components/view/DomainDeleteConfirm.vue @@ -0,0 +1,155 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + + + + + diff --git a/ui/src/config/section/user.js b/ui/src/config/section/user.js index 60a55973f8c..d26901aecca 100644 --- a/ui/src/config/section/user.js +++ b/ui/src/config/section/user.js @@ -69,6 +69,10 @@ export default { label: 'label.action.change.password', dataView: true, popup: true, + show: (record, store) => { + return (['Admin', 'DomainAdmin'].includes(store.userInfo.roletype) || store.userInfo.id === record.id) && + ['native'].includes(record.usersource) && record.state === 'enabled' + }, component: shallowRef(defineAsyncComponent(() => import('@/views/iam/ChangeUserPassword.vue'))) }, { diff --git a/ui/src/views/iam/DomainView.vue b/ui/src/views/iam/DomainView.vue index ed875151a13..2b75d836b13 100644 --- a/ui/src/views/iam/DomainView.vue +++ b/ui/src/views/iam/DomainView.vue @@ -74,6 +74,11 @@ :resource="resource" :action="action"/> + @@ -87,6 +92,7 @@ import ActionButton from '@/components/view/ActionButton' import TreeView from '@/components/view/TreeView' import DomainActionForm from '@/views/iam/DomainActionForm' import ResourceView from '@/components/view/ResourceView' +import DomainDeleteConfirm from '@/components/view/DomainDeleteConfirm' import eventBus from '@/config/eventBus' export default { @@ -96,7 +102,8 @@ export default { ActionButton, TreeView, DomainActionForm, - ResourceView + ResourceView, + DomainDeleteConfirm }, mixins: [mixinDevice], data () { @@ -111,7 +118,9 @@ export default { action: {}, dataView: false, domainStore: {}, - treeDeletedKey: null + treeDeletedKey: null, + showDeleteConfirm: false, + deleteDomainResource: null } }, computed: { @@ -205,7 +214,12 @@ export default { }) }, execAction (action) { - this.treeDeletedKey = action.api === 'deleteDomain' ? this.resource.key : null + if (action.api === 'deleteDomain') { + this.deleteDomainResource = this.resource + this.showDeleteConfirm = true + return + } + this.treeDeletedKey = null this.actionData = [] this.action = action this.action.params = store.getters.apis[this.action.api].params @@ -319,6 +333,42 @@ export default { closeAction () { this.showAction = false }, + confirmDeleteDomain () { + const domain = this.deleteDomainResource + const params = { id: domain.id, cleanup: true } + + api('deleteDomain', params).then(json => { + const jobId = json.deletedomainresponse.jobid + + this.$pollJob({ + jobId, + title: this.$t('label.action.delete.domain'), + description: domain.name, + loadingMessage: `${this.$t('label.action.delete.domain')} ${domain.name}`, + successMessage: `${this.$t('label.action.delete.domain')} ${domain.name}`, + catchMessage: this.$t('error.fetching.async.job.result'), + successMethod: () => { + this.$router.replace({ path: '/domain' }) + this.resource = {} + this.treeSelected = {} + this.treeDeletedKey = null + this.treeViewKey += 1 + this.$nextTick(() => { + this.fetchData() + }) + } + }) + }).catch(error => { + this.$notification.error({ + message: this.$t('message.request.failed'), + description: error.response?.headers['x-description'] || this.$t('message.request.failed') + }) + }).finally(() => { + this.showDeleteConfirm = false + this.deleteDomainResource = null + this.treeDeletedKey = null + }) + }, forceRerender () { this.treeViewKey += 1 } diff --git a/ui/src/views/image/UpdateTemplate.vue b/ui/src/views/image/UpdateTemplate.vue index 7db402cdd5b..590e8c233c8 100644 --- a/ui/src/views/image/UpdateTemplate.vue +++ b/ui/src/views/image/UpdateTemplate.vue @@ -245,7 +245,9 @@ export default { userdataid: null, userdatapolicy: null, userdatapolicylist: {}, - architectureTypes: {} + architectureTypes: {}, + detailsFields: [], + details: {} } }, beforeCreate () { @@ -295,17 +297,10 @@ export default { } } } - const resourceDetailsFields = [] if (this.resource.hypervisor === 'KVM') { - resourceDetailsFields.push('rootDiskController') + this.detailsFields.push('rootDiskController') } else if (this.resource.hypervisor === 'VMware' && !this.resource.deployasis) { - resourceDetailsFields.push(...['rootDiskController', 'nicAdapter', 'keyboard']) - } - for (var detailsField of resourceDetailsFields) { - var detailValue = this.resource?.details?.[detailsField] || null - if (detailValue) { - this.form[detailValue] = fieldValue - } + this.detailsFields.push(...['rootDiskController', 'nicAdapter', 'keyboard']) } }, fetchData () { @@ -316,6 +311,7 @@ export default { this.fetchKeyboardTypes() this.fetchUserdata() this.fetchUserdataPolicy() + this.fetchDetails() }, isValidValueForKey (obj, key) { if (this.emptyAllowedFields.includes(key) && obj[key] === '') { @@ -360,6 +356,10 @@ export default { id: 'virtio', description: 'virtio' }) + controller.push({ + id: 'virtio-blk', + description: 'virtio-blk' + }) } else if (hyperVisor === 'VMware') { controller.push({ id: '', @@ -486,6 +486,25 @@ export default { this.userdata.loading = false }) }, + fetchDetails () { + const params = {} + params.id = this.resource.id + params.templatefilter = 'all' + + api('listTemplates', params).then(response => { + if (response?.listtemplatesresponse?.template?.length > 0) { + this.details = response.listtemplatesresponse.template[0].details + if (this.details) { + for (var detailsField of this.detailsFields) { + var detailValue = this.details?.[detailsField] || null + if (detailValue) { + this.form[detailsField] = detailValue + } + } + } + } + }) + }, handleSubmit (e) { e.preventDefault() if (this.loading) return @@ -495,10 +514,14 @@ export default { const params = { id: this.resource.id } - const detailsField = ['rootDiskController', 'nicAdapter', 'keyboard'] + if (this.details) { + Object.keys(this.details).forEach((detail, index) => { + params['details[0].' + detail] = this.details[detail] + }) + } for (const key in values) { if (!this.isValidValueForKey(values, key)) continue - if (detailsField.includes(key)) { + if (this.detailsFields.includes(key)) { params['details[0].' + key] = values[key] continue } diff --git a/usage/src/main/java/com/cloud/usage/UsageManagerImpl.java b/usage/src/main/java/com/cloud/usage/UsageManagerImpl.java index 9da64889fc3..2db91ab0e35 100644 --- a/usage/src/main/java/com/cloud/usage/UsageManagerImpl.java +++ b/usage/src/main/java/com/cloud/usage/UsageManagerImpl.java @@ -31,6 +31,7 @@ import java.util.concurrent.Executors; import java.util.concurrent.Future; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; import com.cloud.network.Network; import com.cloud.usage.dao.UsageNetworksDao; @@ -192,6 +193,7 @@ public class UsageManagerImpl extends ManagerBase implements UsageManager, Runna private final List usageVmDisks = new ArrayList(); private final ScheduledExecutorService _executor = Executors.newSingleThreadScheduledExecutor(new NamedThreadFactory("Usage-Job")); + private final AtomicBoolean isParsingJobRunning = new AtomicBoolean(false); private final ScheduledExecutorService _heartbeatExecutor = Executors.newSingleThreadScheduledExecutor(new NamedThreadFactory("Usage-HB")); private final ScheduledExecutorService _sanityExecutor = Executors.newSingleThreadScheduledExecutor(new NamedThreadFactory("Usage-Sanity")); private Future _scheduledFuture = null; @@ -367,7 +369,12 @@ public class UsageManagerImpl extends ManagerBase implements UsageManager, Runna (new ManagedContextRunnable() { @Override protected void runInContext() { - runInContextInternal(); + isParsingJobRunning.set(true); + try { + runInContextInternal(); + } finally { + isParsingJobRunning.set(false); + } } }).run(); } @@ -2267,9 +2274,14 @@ public class UsageManagerImpl extends ManagerBase implements UsageManager, Runna if ((timeSinceLastSuccessJob > 0) && (timeSinceLastSuccessJob > (aggregationDurationMillis - 100))) { if (timeToJob > (aggregationDurationMillis / 2)) { - logger.debug("it's been {} ms since last usage job and {} ms until next job, scheduling an immediate job to catch up (aggregation duration is {} minutes)" - , timeSinceLastSuccessJob, timeToJob, _aggregationDuration); - scheduleParse(); + logger.debug("Heartbeat: it's been {} ms since last finished usage job and {} ms until next job (aggregation duration is {} minutes)", + timeSinceLastSuccessJob, timeToJob, _aggregationDuration); + if (isParsingJobRunning.get()) { + logger.debug("Heartbeat: A parsing job is already running"); + } else { + logger.debug("Heartbeat: Scheduling an immediate job to catch up"); + scheduleParse(); + } } } diff --git a/usage/src/main/java/com/cloud/usage/parser/BackupUsageParser.java b/usage/src/main/java/com/cloud/usage/parser/BackupUsageParser.java index 82370fcae3e..257bc468302 100644 --- a/usage/src/main/java/com/cloud/usage/parser/BackupUsageParser.java +++ b/usage/src/main/java/com/cloud/usage/parser/BackupUsageParser.java @@ -84,12 +84,11 @@ public class BackupUsageParser { DecimalFormat dFormat = new DecimalFormat("#.######"); String usageDisplay = dFormat.format(usage); - final Double rawUsage = (double) usageBackup.getSize(); final String description = String.format("Backup usage VM ID: %d, backup offering: %d", vmId, offeringId); final UsageVO usageRecord = new UsageVO(zoneId, account.getAccountId(), account.getDomainId(), description, usageDisplay + " Hrs", - UsageTypes.BACKUP, new Double(usage), vmId, null, offeringId, null, vmId, + UsageTypes.BACKUP, (double) usage, vmId, null, offeringId, null, vmId, usageBackup.getSize(), usageBackup.getProtectedSize(), startDate, endDate); s_usageDao.persist(usageRecord); } diff --git a/usage/src/main/java/com/cloud/usage/parser/VMSnapshotOnPrimaryParser.java b/usage/src/main/java/com/cloud/usage/parser/VMSnapshotOnPrimaryParser.java index fb48bbde4be..252f661569d 100644 --- a/usage/src/main/java/com/cloud/usage/parser/VMSnapshotOnPrimaryParser.java +++ b/usage/src/main/java/com/cloud/usage/parser/VMSnapshotOnPrimaryParser.java @@ -125,7 +125,7 @@ public class VMSnapshotOnPrimaryParser { String usageDesc = "VMSnapshot Id: " + vmSnapshotId + " On Primary Usage: VM Id: " + vmId; usageDesc += " Size: " + toHumanReadableSize(virtualSize); - UsageVO usageRecord = new UsageVO(zoneId, account.getId(), account.getDomainId(), usageDesc, usageDisplay + " Hrs", usageType, new Double(usage), vmId, name, null, null, + UsageVO usageRecord = new UsageVO(zoneId, account.getId(), account.getDomainId(), usageDesc, usageDisplay + " Hrs", usageType, (double) usage, vmId, name, null, null, vmSnapshotId, physicalSize, virtualSize, startDate, endDate); s_usageDao.persist(usageRecord); } diff --git a/utils/src/main/java/com/cloud/utils/FileUtil.java b/utils/src/main/java/com/cloud/utils/FileUtil.java index eea7c0f8561..8a1d3d32ec1 100644 --- a/utils/src/main/java/com/cloud/utils/FileUtil.java +++ b/utils/src/main/java/com/cloud/utils/FileUtil.java @@ -160,4 +160,19 @@ public class FileUtil { public static String readResourceFile(String resource) throws IOException { return IOUtils.toString(Objects.requireNonNull(Thread.currentThread().getContextClassLoader().getResourceAsStream(resource)), com.cloud.utils.StringUtils.getPreferredCharset()); } + + public static boolean deleteRecursively(Path path) throws IOException { + LOGGER.debug("Deleting path: {}", path); + if (Files.isDirectory(path)) { + try (Stream entries = Files.list(path)) { + List list = entries.collect(Collectors.toList()); + for (Path entry : list) { + if (!deleteRecursively(entry)) { + return false; + } + } + } + } + return Files.deleteIfExists(path); + } } diff --git a/utils/src/main/java/com/cloud/utils/ssh/SSHCmdHelper.java b/utils/src/main/java/com/cloud/utils/ssh/SSHCmdHelper.java index dd1c17aa3c0..944f63391a9 100644 --- a/utils/src/main/java/com/cloud/utils/ssh/SSHCmdHelper.java +++ b/utils/src/main/java/com/cloud/utils/ssh/SSHCmdHelper.java @@ -77,7 +77,7 @@ public class SSHCmdHelper { } public static com.trilead.ssh2.Connection acquireAuthorizedConnection(String ip, int port, String username, String password) { - return acquireAuthorizedConnection(ip, 22, username, password, null); + return acquireAuthorizedConnection(ip, port, username, password, null); } public static boolean acquireAuthorizedConnectionWithPublicKey(final com.trilead.ssh2.Connection sshConnection, final String username, final String privateKey) { diff --git a/utils/src/main/java/org/apache/cloudstack/utils/jsinterpreter/JsInterpreter.java b/utils/src/main/java/org/apache/cloudstack/utils/jsinterpreter/JsInterpreter.java index 3126da50bca..6e6ef2bbe59 100644 --- a/utils/src/main/java/org/apache/cloudstack/utils/jsinterpreter/JsInterpreter.java +++ b/utils/src/main/java/org/apache/cloudstack/utils/jsinterpreter/JsInterpreter.java @@ -24,7 +24,6 @@ import java.util.Arrays; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; -import java.util.Objects; import java.util.concurrent.Callable; import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; @@ -44,6 +43,7 @@ import javax.script.SimpleBindings; import javax.script.SimpleScriptContext; import org.apache.commons.collections.MapUtils; +import org.apache.commons.lang3.ObjectUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.openjdk.nashorn.api.scripting.ClassFilter; @@ -67,7 +67,7 @@ public class JsInterpreter implements Closeable { protected ScriptEngine interpreter; protected String interpreterName; - private final String injectingLogMessage = "Injecting variable [%s] with value [%s] into the JS interpreter."; + private final String injectingLogMessage = "Injecting variable [{}] with value [{}] into the JS interpreter."; protected ExecutorService executor; private TimeUnit defaultTimeUnit = TimeUnit.MILLISECONDS; private long timeout; @@ -107,7 +107,7 @@ public class JsInterpreter implements Closeable { NashornScriptEngineFactory factory = new NashornScriptEngineFactory(); this.interpreterName = factory.getEngineName(); - logger.trace(String.format("Initiating JS interpreter: %s.", interpreterName)); + logger.trace("Initiating JS interpreter: {}.", interpreterName); setScriptEngineDisablingJavaLanguage(factory); } @@ -136,36 +136,25 @@ public class JsInterpreter implements Closeable { */ public void injectVariable(String key, Object value) { if (key == null) return; - logger.trace(String.format(injectingLogMessage, key, String.valueOf(value))); + logger.trace(injectingLogMessage, key, value); variables.put(key, value); } - /** - * @deprecated Not needed when using Bindings; kept for source compatibility. - * Prefer {@link #injectVariable(String, Object)}. - */ - @Deprecated - public void injectStringVariable(String key, String value) { - if (value == null) { - logger.trace(String.format("Not injecting [%s] because its value is null.", key)); - return; - } - injectVariable(key, value); - } - /** * Injects the variables via Bindings and executes the script with a fresh context. * @param script Code to be executed. * @return The result of the executed script. */ public Object executeScript(String script) { - Objects.requireNonNull(script, "script"); + if (script == null) { + throw new CloudRuntimeException("Script injected into the JavaScript interpreter must not be null."); + } - logger.debug(String.format("Executing script [%s].", script)); + logger.debug("Executing script [{}].", script); Object result = executeScriptInThread(script); - logger.debug(String.format("The script [%s] had the following result: [%s].", script, result)); + logger.debug("The script [{}] had the following result: [{}].", script, result); return result; } @@ -193,7 +182,7 @@ public class JsInterpreter implements Closeable { } return result; } catch (ScriptException se) { - String msg = se.getMessage() == null ? "Script error" : se.getMessage(); + String msg = ObjectUtils.defaultIfNull(se.getMessage(), "Script error"); throw new ScriptException("Script error: " + msg, se.getFileName(), se.getLineNumber(), se.getColumnNumber()); } }; @@ -213,7 +202,7 @@ public class JsInterpreter implements Closeable { logger.error(message, e); throw new CloudRuntimeException(message, e); } catch (ExecutionException e) { - Throwable cause = e.getCause() == null ? e : e.getCause(); + Throwable cause = ObjectUtils.defaultIfNull(e.getCause(), e); String message = String.format("Unable to execute script [%s] due to [%s]", script, cause.getMessage()); logger.error(message, cause); throw new CloudRuntimeException(message, cause); diff --git a/utils/src/main/java/org/apache/cloudstack/utils/jsinterpreter/TagAsRuleHelper.java b/utils/src/main/java/org/apache/cloudstack/utils/jsinterpreter/TagAsRuleHelper.java index da3da612a22..b3013567352 100644 --- a/utils/src/main/java/org/apache/cloudstack/utils/jsinterpreter/TagAsRuleHelper.java +++ b/utils/src/main/java/org/apache/cloudstack/utils/jsinterpreter/TagAsRuleHelper.java @@ -17,7 +17,11 @@ package org.apache.cloudstack.utils.jsinterpreter; import java.io.IOException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import com.cloud.utils.StringUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -27,24 +31,25 @@ public class TagAsRuleHelper { protected static Logger LOGGER = LogManager.getLogger(TagAsRuleHelper.class); - private static final String PARSE_TAGS = "tags = tags ? tags.split(',') : [];"; - - public static boolean interpretTagAsRule(String rule, String tags, long timeout) { - String script = PARSE_TAGS + rule; + List tagsPresetVariable = new ArrayList<>(); + if (!StringUtils.isEmpty(tags)) { + tagsPresetVariable.addAll(Arrays.asList(tags.split(","))); + } + try (JsInterpreter jsInterpreter = new JsInterpreter(timeout)) { - jsInterpreter.injectVariable("tags", tags); - Object scriptReturn = jsInterpreter.executeScript(script); + jsInterpreter.injectVariable("tags", tagsPresetVariable); + Object scriptReturn = jsInterpreter.executeScript(rule); if (scriptReturn instanceof Boolean) { return (Boolean)scriptReturn; } } catch (IOException ex) { - String message = String.format("Error while executing script [%s].", script); + String message = String.format("Error while executing script [%s].", rule); LOGGER.error(message, ex); throw new CloudRuntimeException(message, ex); } - LOGGER.debug(String.format("Result of tag rule [%s] was not a boolean, returning false.", script)); + LOGGER.debug("Result of tag rule [{}] was not a boolean, returning false.", rule); return false; } diff --git a/utils/src/test/java/org/apache/cloudstack/utils/jsinterpreter/JsInterpreterTest.java b/utils/src/test/java/org/apache/cloudstack/utils/jsinterpreter/JsInterpreterTest.java index a78ee7b908e..e6ef43f2fc1 100644 --- a/utils/src/test/java/org/apache/cloudstack/utils/jsinterpreter/JsInterpreterTest.java +++ b/utils/src/test/java/org/apache/cloudstack/utils/jsinterpreter/JsInterpreterTest.java @@ -159,24 +159,6 @@ public class JsInterpreterTest { Mockito.any(ClassLoader.class), Mockito.any(ClassFilter.class)); } - @Test - public void injectStringVariableTestNullValueDoNothing() { - jsInterpreterSpy.variables = new LinkedHashMap<>(); - - jsInterpreterSpy.injectStringVariable("a", null); - - Assert.assertTrue(jsInterpreterSpy.variables.isEmpty()); - } - - @Test - public void injectStringVariableTestNotNullValueSurroundWithDoubleQuotes() { - jsInterpreterSpy.variables = new LinkedHashMap<>(); - - jsInterpreterSpy.injectStringVariable("a", "b"); - - Assert.assertEquals(jsInterpreterSpy.variables.get("a"), "b"); - } - @Test public void executeScriptTestValidScriptShouldPassWithMixedVariables() { try (JsInterpreter jsInterpreter = new JsInterpreter(1000)) {