From 60908ad19725f5a0fb9a9c826438b9e084bf4f21 Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Thu, 22 Oct 2026 11:35:36 +0200 Subject: [PATCH 1/2] use ConfigKeys for UCS settings and remaining getValue() literals Part of the split of #13884 (phase out the Config enum, #10752). Co-Authored-By: Claude Opus 5.5 --- .../domain/dao/DomainDetailsDaoImpl.java | 13 ++- .../com/cloud/user/AccountDetailsDaoImpl.java | 20 ++-- ...GarbageCollectingStoragePoolAllocator.java | 7 +- .../quota/QuotaAlertManagerImpl.java | 5 +- .../presetvariables/PresetVariableHelper.java | 12 ++- .../com/cloud/ucs/manager/UcsManagerImpl.java | 3 +- .../KubernetesClusterActionWorker.java | 15 ++- .../KubernetesClusterStartWorker.java | 6 +- .../api/query/dao/TemplateJoinDaoImpl.java | 3 - .../com/cloud/consoleproxy/AgentHookBase.java | 2 +- .../StaticConsoleProxyManager.java | 10 +- .../cloud/network/SshKeysDistriMonitor.java | 7 +- .../apache/cloudstack/ca/CAManagerImpl.java | 6 +- .../response/ApiResponseSerializerTest.java | 100 ++++++++++++++++++ .../resource/ResourceManagerImplTest.java | 8 +- .../server/ManagementServerImplTest.java | 31 ++++++ .../cloudstack/ca/CAManagerImplTest.java | 7 +- 17 files changed, 195 insertions(+), 60 deletions(-) create mode 100644 server/src/test/java/com/cloud/api/response/ApiResponseSerializerTest.java diff --git a/engine/schema/src/main/java/com/cloud/domain/dao/DomainDetailsDaoImpl.java b/engine/schema/src/main/java/com/cloud/domain/dao/DomainDetailsDaoImpl.java index 5b4e4c591ffc..6276c5c0217f 100644 --- a/engine/schema/src/main/java/com/cloud/domain/dao/DomainDetailsDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/domain/dao/DomainDetailsDaoImpl.java @@ -21,10 +21,10 @@ import java.util.Map; import javax.inject.Inject; +import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.cloudstack.framework.config.ConfigKey.Scope; import org.apache.cloudstack.framework.config.ScopedConfigStorage; -import org.apache.cloudstack.framework.config.dao.ConfigurationDao; import com.cloud.domain.DomainDetailVO; import com.cloud.domain.DomainVO; @@ -40,8 +40,12 @@ public class DomainDetailsDaoImpl extends ResourceDetailsDaoBase @Inject protected DomainDao _domainDao; - @Inject - private ConfigurationDao _configDao; + + // Registered/owned by com.cloud.configuration.ConfigurationManagerImpl#ENABLE_DOMAIN_SETTINGS_FOR_CHILD_DOMAIN; + // declared here too so this module can read it without a reverse dependency on the server module. + private static final ConfigKey EnableDomainSettingsForChildDomain = new ConfigKey<>("Advanced", Boolean.class, "enable.domain.settings.for.child.domain", "false", + "Indicates whether the settings of parent domain should be applied for child domain. If true, the child domain will get value from parent domain if its not configured in child domain else global value is taken.", + true); protected DomainDetailsDaoImpl() { domainSearch = createSearchBuilder(); @@ -113,8 +117,7 @@ public Scope getScope() { @Override public String getConfigValue(long id, String key) { DomainDetailVO vo = null; - String enableDomainSettingsForChildDomain = _configDao.getValue("enable.domain.settings.for.child.domain"); - if (!Boolean.parseBoolean(enableDomainSettingsForChildDomain)) { + if (!EnableDomainSettingsForChildDomain.value()) { vo = findDetail(id, key); return vo == null ? null : getActualValue(vo); } diff --git a/engine/schema/src/main/java/com/cloud/user/AccountDetailsDaoImpl.java b/engine/schema/src/main/java/com/cloud/user/AccountDetailsDaoImpl.java index cbacf9af5721..33ae2e32575b 100644 --- a/engine/schema/src/main/java/com/cloud/user/AccountDetailsDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/user/AccountDetailsDaoImpl.java @@ -26,7 +26,6 @@ import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.cloudstack.framework.config.ConfigKey.Scope; import org.apache.cloudstack.framework.config.ScopedConfigStorage; -import org.apache.cloudstack.framework.config.dao.ConfigurationDao; import com.cloud.domain.DomainDetailVO; import com.cloud.domain.DomainVO; @@ -50,8 +49,17 @@ public class AccountDetailsDaoImpl extends ResourceDetailsDaoBase EnableAccountSettingsForDomain = new ConfigKey<>("Advanced", Boolean.class, "enable.account.settings.for.domain", "false", + "Indicates whether to add Account settings for domain. If True, Account settings will be added to domain settings, all Accounts in the domain will inherit the domain setting if Account setting is not set.", true); + + // Registered/owned by com.cloud.configuration.ConfigurationManagerImpl#ENABLE_DOMAIN_SETTINGS_FOR_CHILD_DOMAIN; + // declared here too so this module can read it without a reverse dependency on the server module. + private static final ConfigKey EnableDomainSettingsForChildDomain = new ConfigKey<>("Advanced", Boolean.class, "enable.domain.settings.for.child.domain", "false", + "Indicates whether the settings of parent domain should be applied for child domain. If true, the child domain will get value from parent domain if its not configured in child domain else global value is taken.", + true); protected AccountDetailsDaoImpl() { accountSearch = createSearchBuilder(); @@ -131,14 +139,12 @@ public String getConfigValue(long id, String key) { // if account level setting is not configured then check if // we can take value from domain - String enableAccountSettingsForDomain = _configDao.getValue("enable.account.settings.for.domain"); - if (! Boolean.parseBoolean(enableAccountSettingsForDomain)) { + if (!EnableAccountSettingsForDomain.value()) { return null; } // check if we can traverse till ROOT domain to get the value - String enableDomainSettingsForChildDomain = _configDao.getValue("enable.domain.settings.for.child.domain"); - if (Boolean.parseBoolean(enableDomainSettingsForChildDomain)) { + if (EnableDomainSettingsForChildDomain.value()) { Optional account = Optional.ofNullable(_accountDao.findById(id)); if (account.isPresent()) { DomainVO domain = _domainDao.findById(account.get().getDomainId()); diff --git a/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/GarbageCollectingStoragePoolAllocator.java b/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/GarbageCollectingStoragePoolAllocator.java index 39c29bd2c6e6..eef8f6d66206 100644 --- a/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/GarbageCollectingStoragePoolAllocator.java +++ b/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/GarbageCollectingStoragePoolAllocator.java @@ -22,9 +22,7 @@ import javax.inject.Inject; import javax.naming.ConfigurationException; - import org.apache.cloudstack.engine.subsystem.api.storage.StoragePoolAllocator; -import org.apache.cloudstack.framework.config.dao.ConfigurationDao; import com.cloud.deploy.DeploymentPlan; import com.cloud.deploy.DeploymentPlanner.ExcludeList; @@ -40,8 +38,6 @@ public class GarbageCollectingStoragePoolAllocator extends AbstractStoragePoolAl StoragePoolAllocator _localStoragePoolAllocator; @Inject StorageManager storageMgr; - @Inject - ConfigurationDao _configDao; boolean _storagePoolCleanupEnabled; @Override @@ -78,8 +74,7 @@ public boolean configure(String name, Map params) throws Configu _localStoragePoolAllocator = ComponentContext.inject(LocalStoragePoolAllocator.class); _localStoragePoolAllocator.configure("GCLocalStoragePoolAllocator", params); - String storagePoolCleanupEnabled = _configDao.getValue("storage.pool.cleanup.enabled"); - _storagePoolCleanupEnabled = (storagePoolCleanupEnabled == null) ? true : Boolean.parseBoolean(storagePoolCleanupEnabled); + _storagePoolCleanupEnabled = StorageManager.StoragePoolCleanupEnabled.value(); return true; } diff --git a/framework/quota/src/main/java/org/apache/cloudstack/quota/QuotaAlertManagerImpl.java b/framework/quota/src/main/java/org/apache/cloudstack/quota/QuotaAlertManagerImpl.java index b26b3171f5b5..ef9c78ead951 100644 --- a/framework/quota/src/main/java/org/apache/cloudstack/quota/QuotaAlertManagerImpl.java +++ b/framework/quota/src/main/java/org/apache/cloudstack/quota/QuotaAlertManagerImpl.java @@ -42,7 +42,6 @@ import org.apache.commons.lang.StringEscapeUtils; import org.apache.commons.lang.text.StrSubstitutor; import org.apache.commons.lang3.StringUtils; -import org.apache.commons.lang3.ObjectUtils; import org.springframework.stereotype.Component; import com.cloud.domain.DomainVO; @@ -262,7 +261,7 @@ public void sendQuotaAlert(DeferredQuotaEmail emailToBeSent) { userNames = userNames.substring(0, userNames.length() - 1); } - String currencySymbol = ObjectUtils.defaultIfNull(_configDao.getValue(QuotaConfig.QuotaCurrencySymbol.key()), QuotaConfig.QuotaCurrencySymbol.defaultValue()); + String currencySymbol = QuotaConfig.QuotaCurrencySymbol.value(); NumberFormat localeFormat = getLocaleFormatIfCurrencyLocaleNotNull(); String balanceStr = String.format("%s %s", currencySymbol, NumbersUtil.formatBigDecimalAccordingToNumberFormat(balance, localeFormat)); @@ -298,7 +297,7 @@ public void sendQuotaAlert(DeferredQuotaEmail emailToBeSent) { } private NumberFormat getLocaleFormatIfCurrencyLocaleNotNull() { - String currencyLocale = _configDao.getValue(QuotaConfig.QuotaCurrencyLocale.key()); + String currencyLocale = QuotaConfig.QuotaCurrencyLocale.value(); NumberFormat localeFormat = null; if (currencyLocale != null) { Locale locale = Locale.forLanguageTag(currencyLocale); 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 23020292027c..8bff1e059cc4 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 @@ -41,7 +41,7 @@ import org.apache.cloudstack.backup.BackupOfferingVO; import org.apache.cloudstack.backup.dao.BackupOfferingDao; import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo; -import org.apache.cloudstack.framework.config.dao.ConfigurationDao; +import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.cloudstack.quota.constant.QuotaTypes; import org.apache.cloudstack.quota.dao.NetworkDao; import org.apache.cloudstack.quota.dao.VmTemplateDao; @@ -188,12 +188,14 @@ public class PresetVariableHelper { @Inject VpcDao vpcDao; - @Inject - ConfigurationDao configDao; - @Inject ClusterDetailsDao clusterDetailsDao; + // Registered/owned by HighAvailabilityManager (engine/components-api); declared here too so this module + // can read it without a dependency on that module. + private static final ConfigKey ForceHA = new ConfigKey<>("Advanced", Boolean.class, "force.ha", "false", + "Force High-Availability to happen even if the VM says no.", true); + @Inject VpcOfferingDao vpcOfferingDao; @@ -315,7 +317,7 @@ protected void setForceHaInConfiguration(Configuration configuration, UsageVO us if (forceHa != null) { forceHaValue = forceHa.getValue(); } else { - forceHaValue = configDao.getValue("force.ha"); + forceHaValue = String.valueOf(ForceHA.value()); } configuration.setForceHa((Boolean.parseBoolean(forceHaValue))); diff --git a/plugins/hypervisors/ucs/src/main/java/com/cloud/ucs/manager/UcsManagerImpl.java b/plugins/hypervisors/ucs/src/main/java/com/cloud/ucs/manager/UcsManagerImpl.java index 2a5699f6bc30..568216b29d9a 100644 --- a/plugins/hypervisors/ucs/src/main/java/com/cloud/ucs/manager/UcsManagerImpl.java +++ b/plugins/hypervisors/ucs/src/main/java/com/cloud/ucs/manager/UcsManagerImpl.java @@ -46,7 +46,6 @@ import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.LogManager; -import com.cloud.configuration.Config; import com.cloud.dc.ClusterDetailsDao; import com.cloud.dc.DataCenterVO; import com.cloud.dc.dao.ClusterDao; @@ -175,7 +174,7 @@ public boolean configure(String name, Map params) throws Configu @Override public boolean start() { try { - syncBladeInterval = Integer.parseInt(configDao.getValue(Config.UCSSyncBladeInterval.key())); + syncBladeInterval = UCSSyncBladeInterval.value(); } catch (NumberFormatException e) { syncBladeInterval = 600; } diff --git a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterActionWorker.java b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterActionWorker.java index 07c9f12a50e2..9dd98bcb92ed 100644 --- a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterActionWorker.java +++ b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterActionWorker.java @@ -75,7 +75,7 @@ import org.apache.cloudstack.config.ApiServiceConfiguration; import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService; -import org.apache.cloudstack.framework.config.dao.ConfigurationDao; +import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.cloudstack.userdata.UserDataManager; import org.apache.commons.codec.binary.Base64; import org.apache.commons.collections.CollectionUtils; @@ -167,8 +167,13 @@ public class KubernetesClusterActionWorker { @Inject protected CAManager caManager; - @Inject - protected ConfigurationDao configurationDao; + + // Registered/owned by ManagementServer (server module); declared here too so this module can read them + // without a reverse dependency on the server module. + protected static final ConfigKey DeveloperMode = new ConfigKey<>("Advanced", Boolean.class, "developer", "false", + "Whether the management server is running in developer mode.", false); + protected static final ConfigKey SshPublicKey = new ConfigKey<>("Hidden", String.class, "ssh.publickey", null, + "Public key for the entire CloudStack, generated once and stored on first management server start.", false); @Inject protected DataCenterDao dataCenterDao; @Inject @@ -408,7 +413,7 @@ protected void logAndThrow(final Level logLevel, final String message, final Exc } protected File getManagementServerSshPublicKeyFile() { - boolean devel = Boolean.parseBoolean(configurationDao.getValue("developer")); + boolean devel = DeveloperMode.value(); String keyFile = String.format("%s/.ssh/id_rsa", System.getProperty("user.home")); if (devel) { keyFile += ".cloud"; @@ -1049,7 +1054,7 @@ public String getKubernetesNodeConfig(final String joinIp, final boolean ejectIs routerIp = routerNicOnNetwork.getIPv4Address(); } } - String pubKey = "- \"" + configurationDao.getValue("ssh.publickey") + "\""; + String pubKey = "- \"" + SshPublicKey.value() + "\""; String sshKeyPair = kubernetesCluster.getKeyPair(); if (StringUtils.isNotEmpty(sshKeyPair)) { SSHKeyPairVO sshkp = sshKeyPairDao.findByName(owner.getAccountId(), owner.getDomainId(), sshKeyPair); diff --git a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterStartWorker.java b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterStartWorker.java index a15086c60d6c..47e92562e81d 100644 --- a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterStartWorker.java +++ b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterStartWorker.java @@ -209,7 +209,7 @@ private Pair getKubernetesControlNodeConfig(final String control k8sControlNodeConfig = k8sControlNodeConfig.replace(apiServerCert, tlsClientCert.replace("\n", "\n ")); k8sControlNodeConfig = k8sControlNodeConfig.replace(apiServerKey, tlsPrivateKey.replace("\n", "\n ")); k8sControlNodeConfig = k8sControlNodeConfig.replace(caCert, tlsCaCert.replace("\n", "\n ")); - String pubKey = "- \"" + configurationDao.getValue("ssh.publickey") + "\""; + String pubKey = "- \"" + SshPublicKey.value() + "\""; String sshKeyPair = kubernetesCluster.getKeyPair(); if (StringUtils.isNotEmpty(sshKeyPair)) { SSHKeyPairVO sshkp = sshKeyPairDao.findByName(owner.getAccountId(), owner.getDomainId(), sshKeyPair); @@ -341,7 +341,7 @@ private String getKubernetesAdditionalControlNodeConfig(final String joinIp, fin final Long waitTime = KubernetesClusterService.KubernetesControlNodeInstallAttemptWait.value(); final Long reattempts = KubernetesClusterService.KubernetesControlNodeInstallReattempts.value(); - String pubKey = "- \"" + configurationDao.getValue("ssh.publickey") + "\""; + String pubKey = "- \"" + SshPublicKey.value() + "\""; String sshKeyPair = kubernetesCluster.getKeyPair(); if (StringUtils.isNotEmpty(sshKeyPair)) { SSHKeyPairVO sshkp = sshKeyPairDao.findByName(owner.getAccountId(), owner.getDomainId(), sshKeyPair); @@ -412,7 +412,7 @@ private String getEtcdNodeConfig(final List ipAddresses, final List implements TemplateJoinDao { - @Inject - private ConfigurationDao _configDao; @Inject private AccountService _accountService; @Inject diff --git a/server/src/main/java/com/cloud/consoleproxy/AgentHookBase.java b/server/src/main/java/com/cloud/consoleproxy/AgentHookBase.java index fd8c535a630b..7a94eaa7c3f3 100644 --- a/server/src/main/java/com/cloud/consoleproxy/AgentHookBase.java +++ b/server/src/main/java/com/cloud/consoleproxy/AgentHookBase.java @@ -236,7 +236,7 @@ public void startAgentHttpHandlerInVM(StartupProxyCommand startupCmd) { cmd = new StartConsoleProxyAgentHttpHandlerCommand(ksBits, storePassword); cmd.setEncryptorPassword(getEncryptorPassword()); - cmd.setIsSourceIpCheckEnabled(Boolean.parseBoolean(_configDao.getValue(ConsoleProxyManager.NoVncConsoleSourceIpCheckEnabled.key()))); + cmd.setIsSourceIpCheckEnabled(ConsoleProxyManager.NoVncConsoleSourceIpCheckEnabled.value()); if (consoleProxyHost != null) { Answer answer = _agentMgr.send(consoleProxyHost.getId(), cmd); diff --git a/server/src/main/java/com/cloud/consoleproxy/StaticConsoleProxyManager.java b/server/src/main/java/com/cloud/consoleproxy/StaticConsoleProxyManager.java index 558bff3fdc87..9dd764b979c0 100644 --- a/server/src/main/java/com/cloud/consoleproxy/StaticConsoleProxyManager.java +++ b/server/src/main/java/com/cloud/consoleproxy/StaticConsoleProxyManager.java @@ -22,8 +22,6 @@ import javax.inject.Inject; import javax.naming.ConfigurationException; -import org.apache.cloudstack.framework.config.dao.ConfigurationDao; - import com.cloud.agent.api.StartupCommand; import com.cloud.agent.api.StartupProxyCommand; import com.cloud.host.Host.Type; @@ -33,7 +31,6 @@ import com.cloud.resource.ResourceStateAdapter; import com.cloud.resource.ServerResource; import com.cloud.resource.UnableDeleteHostException; -import com.cloud.utils.NumbersUtil; import com.cloud.vm.VMInstanceVO; import com.cloud.vm.dao.ConsoleProxyDao; @@ -43,8 +40,6 @@ public class StaticConsoleProxyManager extends AgentBasedConsoleProxyManager imp ConsoleProxyDao _proxyDao; @Inject ResourceManager _resourceMgr; - @Inject - ConfigurationDao _configDao; private String _ip = null; public StaticConsoleProxyManager() { @@ -67,7 +62,7 @@ public ConsoleProxyInfo assignProxy(long dataCenterId, VMInstanceVO userVm) { @Override public boolean configure(String name, Map params) throws ConfigurationException { super.configure(name, params); - _ip = _configDao.getValue("consoleproxy.static.publicIp"); + _ip = ConsoleProxyStaticPublicIp.value(); if (_ip == null) { _ip = "127.0.0.1"; } @@ -79,7 +74,8 @@ public boolean configure(String name, Map params) throws Configu int defaultPort = 8088; if (_sslEnabled) defaultPort = 8443; - _consoleProxyUrlPort = NumbersUtil.parseInt(_configDao.getValue("consoleproxy.static.port"), defaultPort); + Integer configuredPort = ConsoleProxyStaticPort.value(); + _consoleProxyUrlPort = configuredPort != null ? configuredPort : defaultPort; _resourceMgr.registerResourceStateAdapter(this.getClass().getSimpleName(), this); diff --git a/server/src/main/java/com/cloud/network/SshKeysDistriMonitor.java b/server/src/main/java/com/cloud/network/SshKeysDistriMonitor.java index 373eb80349f5..4e5685c55efe 100644 --- a/server/src/main/java/com/cloud/network/SshKeysDistriMonitor.java +++ b/server/src/main/java/com/cloud/network/SshKeysDistriMonitor.java @@ -37,15 +37,14 @@ import com.cloud.host.Status; import com.cloud.host.dao.HostDao; import com.cloud.hypervisor.Hypervisor.HypervisorType; +import com.cloud.server.ManagementServer; public class SshKeysDistriMonitor implements Listener { protected Logger logger = LogManager.getLogger(getClass()); AgentManager _agentMgr; - private ConfigurationDao _configDao; public SshKeysDistriMonitor(AgentManager mgr, HostDao host, ConfigurationDao config) { _agentMgr = mgr; - _configDao = config; } @Override @@ -93,8 +92,8 @@ public void processConnect(Host host, StartupCommand cmd, boolean forRebalance) ((StartupRoutingCommand)cmd).getHypervisorType() == HypervisorType.LXC) { /*TODO: Get the private/public keys here*/ - String pubKey = _configDao.getValue("ssh.publickey"); - String prvKey = _configDao.getValue("ssh.privatekey"); + String pubKey = ManagementServer.SshPublicKey.value(); + String prvKey = ManagementServer.SshPrivateKey.value(); try { ModifySshKeysCommand cmds = new ModifySshKeysCommand(pubKey, prvKey); diff --git a/server/src/main/java/org/apache/cloudstack/ca/CAManagerImpl.java b/server/src/main/java/org/apache/cloudstack/ca/CAManagerImpl.java index 73ff79301fb7..17eb8c1eb60c 100644 --- a/server/src/main/java/org/apache/cloudstack/ca/CAManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/ca/CAManagerImpl.java @@ -48,8 +48,8 @@ import com.trilead.ssh2.Connection; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService; -import org.apache.cloudstack.framework.config.dao.ConfigurationDao; import com.cloud.host.HostVO; +import com.cloud.server.ManagementServer; import com.cloud.utils.PasswordGenerator; import com.cloud.utils.ssh.SSHCmdHelper; import com.cloud.vm.VMInstanceVO; @@ -103,8 +103,6 @@ public class CAManagerImpl extends ManagerBase implements CAManager { @Inject private NetworkOrchestrationService networkOrchestrationService; @Inject - private ConfigurationDao configDao; - @Inject private AgentManager agentManager; @Inject private BackgroundPollManager backgroundPollManager; @@ -315,7 +313,7 @@ private boolean provisionKvmHostViaSsh(Host host, String caProvider) { sshConnection = new Connection(hostIp, port); sshConnection.connect(null, 60000, 60000); - String privateKey = configDao.getValue("ssh.privatekey"); + String privateKey = ManagementServer.SshPrivateKey.value(); if (!SSHCmdHelper.acquireAuthorizedConnectionWithPublicKey(sshConnection, username, privateKey)) { if (StringUtils.isEmpty(password) || !sshConnection.authenticateWithPassword(username, password)) { throw new CloudRuntimeException("Failed to authenticate to host via SSH for forced provisioning: " + hostIp); diff --git a/server/src/test/java/com/cloud/api/response/ApiResponseSerializerTest.java b/server/src/test/java/com/cloud/api/response/ApiResponseSerializerTest.java new file mode 100644 index 000000000000..18ca88e4739a --- /dev/null +++ b/server/src/test/java/com/cloud/api/response/ApiResponseSerializerTest.java @@ -0,0 +1,100 @@ +// 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 com.cloud.api.response; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.lang.reflect.Field; + +import org.apache.cloudstack.api.response.SuccessResponse; +import org.apache.cloudstack.context.CallContext; +import org.apache.cloudstack.framework.config.ConfigKey; +import org.junit.After; +import org.junit.Test; + +import com.cloud.api.ApiDBUtils; +import com.cloud.server.ManagementServer; +import com.cloud.user.Account; +import com.cloud.user.User; + +public class ApiResponseSerializerTest { + + @After + public void tearDown() throws Exception { + CallContext.unregister(); + setManagementServer(null); + } + + private void overrideDefaultConfigValue(ConfigKey configKey, String value) throws NoSuchFieldException, IllegalAccessException { + Field f = ConfigKey.class.getDeclaredField("_defaultValue"); + f.setAccessible(true); + f.set(configKey, value); + } + + private void setManagementServer(ManagementServer ms) throws Exception { + Field smsField = ApiDBUtils.class.getDeclaredField("s_ms"); + smsField.setAccessible(true); + smsField.set(null, ms); + } + + private void registerAuthenticatedCallContext() { + Account account = mock(Account.class); + when(account.getId()).thenReturn(Account.ACCOUNT_ID_SYSTEM + 1); + CallContext.register(mock(User.class), account); + } + + @Test + public void toXmlSerializedStringIncludesVersionWhenExposedAndAuthenticated() throws Exception { + overrideDefaultConfigValue(ManagementServer.exposeCloudStackVersionInApiXmlResponse, "true"); + ManagementServer ms = mock(ManagementServer.class); + when(ms.getVersion()).thenReturn("1.2.3-test"); + setManagementServer(ms); + registerAuthenticatedCallContext(); + + String xml = ApiResponseSerializer.toSerializedString(new SuccessResponse("success"), "xml"); + + assertTrue(xml.contains("cloud-stack-version=\"1.2.3-test\"")); + } + + @Test + public void toXmlSerializedStringOmitsVersionWhenNotExposed() throws Exception { + overrideDefaultConfigValue(ManagementServer.exposeCloudStackVersionInApiXmlResponse, "false"); + registerAuthenticatedCallContext(); + + String xml = ApiResponseSerializer.toSerializedString(new SuccessResponse("success"), "xml"); + + assertFalse(xml.contains("cloud-stack-version=")); + } + + @Test + public void toXmlSerializedStringOmitsVersionForSystemAccountEvenWhenExposed() throws Exception { + overrideDefaultConfigValue(ManagementServer.exposeCloudStackVersionInApiXmlResponse, "true"); + ManagementServer ms = mock(ManagementServer.class); + when(ms.getVersion()).thenReturn("1.2.3-test"); + setManagementServer(ms); + Account systemAccount = mock(Account.class); + when(systemAccount.getId()).thenReturn(Account.ACCOUNT_ID_SYSTEM); + CallContext.register(mock(User.class), systemAccount); + + String xml = ApiResponseSerializer.toSerializedString(new SuccessResponse("success"), "xml"); + + assertFalse(xml.contains("cloud-stack-version=")); + } +} diff --git a/server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java b/server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java index b9da7390566c..98ff7625345f 100644 --- a/server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java +++ b/server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java @@ -36,6 +36,7 @@ import com.cloud.host.Status; import com.cloud.host.dao.HostDao; import com.cloud.hypervisor.Hypervisor; +import com.cloud.server.ManagementServer; import com.cloud.storage.ScopeType; import com.cloud.storage.StorageManager; import com.cloud.storage.StoragePoolHostVO; @@ -198,7 +199,7 @@ public void setup() throws Exception { when(hostDao.findById(hostId)).thenReturn(host); when(host.getDetail("username")).thenReturn(hostUsername); when(host.getDetail("password")).thenReturn(hostPassword); - when(configurationDao.getValue("ssh.privatekey")).thenReturn(hostPrivateKey); + overrideDefaultConfigValue(ManagementServer.SshPrivateKey, "_value", hostPrivateKey); when(host.getStatus()).thenReturn(Status.Up); when(host.getPrivateIpAddress()).thenReturn(hostPrivateIp); when(vm1.getId()).thenReturn(vm1Id); @@ -242,6 +243,7 @@ public void setup() throws Exception { @After public void tearDown() throws Exception { + overrideDefaultConfigValue(ManagementServer.SshPrivateKey, "_value", null); sshHelperMocked.close(); actionEventUtilsMocked.close(); getVncPortCommandMockedConstruction.close(); @@ -380,9 +382,9 @@ public void testConfigureVncAccessForKVMHostFailedMigrations() { } @Test(expected = CloudRuntimeException.class) - public void testGetHostCredentialsMissingParameter() { + public void testGetHostCredentialsMissingParameter() throws Exception { when(host.getDetail("password")).thenReturn(null); - when(configurationDao.getValue("ssh.privatekey")).thenReturn(null); + overrideDefaultConfigValue(ManagementServer.SshPrivateKey, "_value", null); resourceManager.getHostCredentials(host); } diff --git a/server/src/test/java/com/cloud/server/ManagementServerImplTest.java b/server/src/test/java/com/cloud/server/ManagementServerImplTest.java index d2c12b6c99dd..e1cd21f87d14 100644 --- a/server/src/test/java/com/cloud/server/ManagementServerImplTest.java +++ b/server/src/test/java/com/cloud/server/ManagementServerImplTest.java @@ -42,8 +42,11 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; import org.apache.cloudstack.annotation.dao.AnnotationDao; import org.apache.cloudstack.affinity.dao.AffinityGroupVMMapDao; @@ -2493,4 +2496,32 @@ public void createDeploymentPlanForMigrationListingTestAllocatesInSourceClusterW Assert.assertEquals(5L, (long) deploymentPlan.getPodId()); Assert.assertEquals(4L, (long) deploymentPlan.getClusterId()); } + + @Test + public void configureSchedulesAlertPurgeTaskWhenAlertPurgeDelayIsNonZero() throws Exception { + when(configDao.getConfiguration()).thenReturn(new HashMap<>()); + overrideDefaultConfigValue(spy.AlertPurgeInterval, "_defaultValue", "10"); + overrideDefaultConfigValue(spy.AlertPurgeDelay, "_defaultValue", "5"); + ScheduledExecutorService alertExecutor = mock(ScheduledExecutorService.class); + ReflectionTestUtils.setField(spy, "_alertExecutor", alertExecutor); + + spy.configure("management-server", new HashMap<>()); + + Mockito.verify(alertExecutor).scheduleAtFixedRate(any(Runnable.class), Mockito.eq(10L), Mockito.eq(10L), Mockito.eq(TimeUnit.SECONDS)); + Assert.assertEquals(5, ReflectionTestUtils.getField(spy, "_alertPurgeDelay")); + } + + @Test + public void configureDoesNotScheduleAlertPurgeTaskWhenAlertPurgeDelayIsZero() throws Exception { + when(configDao.getConfiguration()).thenReturn(new HashMap<>()); + overrideDefaultConfigValue(spy.AlertPurgeInterval, "_defaultValue", "10"); + overrideDefaultConfigValue(spy.AlertPurgeDelay, "_defaultValue", "0"); + ScheduledExecutorService alertExecutor = mock(ScheduledExecutorService.class); + ReflectionTestUtils.setField(spy, "_alertExecutor", alertExecutor); + + spy.configure("management-server", new HashMap<>()); + + Mockito.verifyNoInteractions(alertExecutor); + Assert.assertEquals(0, ReflectionTestUtils.getField(spy, "_alertPurgeDelay")); + } } diff --git a/server/src/test/java/org/apache/cloudstack/ca/CAManagerImplTest.java b/server/src/test/java/org/apache/cloudstack/ca/CAManagerImplTest.java index 2d60833d35c7..28f88748f84d 100644 --- a/server/src/test/java/org/apache/cloudstack/ca/CAManagerImplTest.java +++ b/server/src/test/java/org/apache/cloudstack/ca/CAManagerImplTest.java @@ -50,12 +50,13 @@ import java.util.Map; import java.util.HashMap; - import org.mockito.MockedStatic; import org.mockito.MockedConstruction; import com.cloud.utils.ssh.SSHCmdHelper; import com.cloud.host.HostVO; import com.cloud.vm.VMInstanceVO; +import com.cloud.server.ManagementServer; +import org.springframework.test.util.ReflectionTestUtils; import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService; import org.apache.cloudstack.framework.config.dao.ConfigurationDao; import com.cloud.vm.dao.VMInstanceDao; @@ -272,7 +273,7 @@ public void testProvisionKvmHostViaSsh() throws Exception { Mockito.when(host.getDetail(ApiConstants.USERNAME)).thenReturn("root"); Mockito.when(host.getDetail(ApiConstants.PASSWORD)).thenReturn("password"); - Mockito.when(configDao.getValue("ssh.privatekey")).thenReturn("privatekey"); + ReflectionTestUtils.setField(ManagementServer.SshPrivateKey, "_value", "privatekey"); try (MockedConstruction ignored = Mockito.mockConstruction(Connection.class, (mock, context) -> { @@ -292,6 +293,8 @@ public void testProvisionKvmHostViaSsh() throws Exception { Mockito.verify(caManager, Mockito.times(1)).provisionCertificateViaSsh(Mockito.any(Connection.class), Mockito.eq("192.168.1.1"), Mockito.eq("host1"), Mockito.eq("root")); sshCmdHelperMock.verify(() -> SSHCmdHelper.sshExecuteCmd(Mockito.any(Connection.class), Mockito.eq("systemctl restart libvirtd")), Mockito.times(1)); sshCmdHelperMock.verify(() -> SSHCmdHelper.sshExecuteCmd(Mockito.any(Connection.class), Mockito.eq("systemctl restart cloudstack-agent")), Mockito.times(1)); + } finally { + ReflectionTestUtils.setField(ManagementServer.SshPrivateKey, "_value", null); } } From a7fff8332367a2c3a95f8824f1faea6158b81d0d Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Fri, 23 Oct 2026 11:57:06 +0200 Subject: [PATCH 2/2] read quota settings from the DB in code that runs in the usage server Also hand the ResourceManagerImplTest and ManagementServerImplTest changes to the PR that changes the code they test. Co-Authored-By: Claude Opus 5.5 --- .../quota/QuotaAlertManagerImpl.java | 5 +-- .../presetvariables/PresetVariableHelper.java | 12 +++---- .../resource/ResourceManagerImplTest.java | 8 ++--- .../server/ManagementServerImplTest.java | 31 ------------------- 4 files changed, 11 insertions(+), 45 deletions(-) diff --git a/framework/quota/src/main/java/org/apache/cloudstack/quota/QuotaAlertManagerImpl.java b/framework/quota/src/main/java/org/apache/cloudstack/quota/QuotaAlertManagerImpl.java index ef9c78ead951..b26b3171f5b5 100644 --- a/framework/quota/src/main/java/org/apache/cloudstack/quota/QuotaAlertManagerImpl.java +++ b/framework/quota/src/main/java/org/apache/cloudstack/quota/QuotaAlertManagerImpl.java @@ -42,6 +42,7 @@ import org.apache.commons.lang.StringEscapeUtils; import org.apache.commons.lang.text.StrSubstitutor; import org.apache.commons.lang3.StringUtils; +import org.apache.commons.lang3.ObjectUtils; import org.springframework.stereotype.Component; import com.cloud.domain.DomainVO; @@ -261,7 +262,7 @@ public void sendQuotaAlert(DeferredQuotaEmail emailToBeSent) { userNames = userNames.substring(0, userNames.length() - 1); } - String currencySymbol = QuotaConfig.QuotaCurrencySymbol.value(); + String currencySymbol = ObjectUtils.defaultIfNull(_configDao.getValue(QuotaConfig.QuotaCurrencySymbol.key()), QuotaConfig.QuotaCurrencySymbol.defaultValue()); NumberFormat localeFormat = getLocaleFormatIfCurrencyLocaleNotNull(); String balanceStr = String.format("%s %s", currencySymbol, NumbersUtil.formatBigDecimalAccordingToNumberFormat(balance, localeFormat)); @@ -297,7 +298,7 @@ public void sendQuotaAlert(DeferredQuotaEmail emailToBeSent) { } private NumberFormat getLocaleFormatIfCurrencyLocaleNotNull() { - String currencyLocale = QuotaConfig.QuotaCurrencyLocale.value(); + String currencyLocale = _configDao.getValue(QuotaConfig.QuotaCurrencyLocale.key()); NumberFormat localeFormat = null; if (currencyLocale != null) { Locale locale = Locale.forLanguageTag(currencyLocale); 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 8bff1e059cc4..23020292027c 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 @@ -41,7 +41,7 @@ import org.apache.cloudstack.backup.BackupOfferingVO; import org.apache.cloudstack.backup.dao.BackupOfferingDao; import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo; -import org.apache.cloudstack.framework.config.ConfigKey; +import org.apache.cloudstack.framework.config.dao.ConfigurationDao; import org.apache.cloudstack.quota.constant.QuotaTypes; import org.apache.cloudstack.quota.dao.NetworkDao; import org.apache.cloudstack.quota.dao.VmTemplateDao; @@ -189,12 +189,10 @@ public class PresetVariableHelper { VpcDao vpcDao; @Inject - ClusterDetailsDao clusterDetailsDao; + ConfigurationDao configDao; - // Registered/owned by HighAvailabilityManager (engine/components-api); declared here too so this module - // can read it without a dependency on that module. - private static final ConfigKey ForceHA = new ConfigKey<>("Advanced", Boolean.class, "force.ha", "false", - "Force High-Availability to happen even if the VM says no.", true); + @Inject + ClusterDetailsDao clusterDetailsDao; @Inject VpcOfferingDao vpcOfferingDao; @@ -317,7 +315,7 @@ protected void setForceHaInConfiguration(Configuration configuration, UsageVO us if (forceHa != null) { forceHaValue = forceHa.getValue(); } else { - forceHaValue = String.valueOf(ForceHA.value()); + forceHaValue = configDao.getValue("force.ha"); } configuration.setForceHa((Boolean.parseBoolean(forceHaValue))); diff --git a/server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java b/server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java index 98ff7625345f..b9da7390566c 100644 --- a/server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java +++ b/server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java @@ -36,7 +36,6 @@ import com.cloud.host.Status; import com.cloud.host.dao.HostDao; import com.cloud.hypervisor.Hypervisor; -import com.cloud.server.ManagementServer; import com.cloud.storage.ScopeType; import com.cloud.storage.StorageManager; import com.cloud.storage.StoragePoolHostVO; @@ -199,7 +198,7 @@ public void setup() throws Exception { when(hostDao.findById(hostId)).thenReturn(host); when(host.getDetail("username")).thenReturn(hostUsername); when(host.getDetail("password")).thenReturn(hostPassword); - overrideDefaultConfigValue(ManagementServer.SshPrivateKey, "_value", hostPrivateKey); + when(configurationDao.getValue("ssh.privatekey")).thenReturn(hostPrivateKey); when(host.getStatus()).thenReturn(Status.Up); when(host.getPrivateIpAddress()).thenReturn(hostPrivateIp); when(vm1.getId()).thenReturn(vm1Id); @@ -243,7 +242,6 @@ public void setup() throws Exception { @After public void tearDown() throws Exception { - overrideDefaultConfigValue(ManagementServer.SshPrivateKey, "_value", null); sshHelperMocked.close(); actionEventUtilsMocked.close(); getVncPortCommandMockedConstruction.close(); @@ -382,9 +380,9 @@ public void testConfigureVncAccessForKVMHostFailedMigrations() { } @Test(expected = CloudRuntimeException.class) - public void testGetHostCredentialsMissingParameter() throws Exception { + public void testGetHostCredentialsMissingParameter() { when(host.getDetail("password")).thenReturn(null); - overrideDefaultConfigValue(ManagementServer.SshPrivateKey, "_value", null); + when(configurationDao.getValue("ssh.privatekey")).thenReturn(null); resourceManager.getHostCredentials(host); } diff --git a/server/src/test/java/com/cloud/server/ManagementServerImplTest.java b/server/src/test/java/com/cloud/server/ManagementServerImplTest.java index e1cd21f87d14..d2c12b6c99dd 100644 --- a/server/src/test/java/com/cloud/server/ManagementServerImplTest.java +++ b/server/src/test/java/com/cloud/server/ManagementServerImplTest.java @@ -42,11 +42,8 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; -import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.TimeUnit; import org.apache.cloudstack.annotation.dao.AnnotationDao; import org.apache.cloudstack.affinity.dao.AffinityGroupVMMapDao; @@ -2496,32 +2493,4 @@ public void createDeploymentPlanForMigrationListingTestAllocatesInSourceClusterW Assert.assertEquals(5L, (long) deploymentPlan.getPodId()); Assert.assertEquals(4L, (long) deploymentPlan.getClusterId()); } - - @Test - public void configureSchedulesAlertPurgeTaskWhenAlertPurgeDelayIsNonZero() throws Exception { - when(configDao.getConfiguration()).thenReturn(new HashMap<>()); - overrideDefaultConfigValue(spy.AlertPurgeInterval, "_defaultValue", "10"); - overrideDefaultConfigValue(spy.AlertPurgeDelay, "_defaultValue", "5"); - ScheduledExecutorService alertExecutor = mock(ScheduledExecutorService.class); - ReflectionTestUtils.setField(spy, "_alertExecutor", alertExecutor); - - spy.configure("management-server", new HashMap<>()); - - Mockito.verify(alertExecutor).scheduleAtFixedRate(any(Runnable.class), Mockito.eq(10L), Mockito.eq(10L), Mockito.eq(TimeUnit.SECONDS)); - Assert.assertEquals(5, ReflectionTestUtils.getField(spy, "_alertPurgeDelay")); - } - - @Test - public void configureDoesNotScheduleAlertPurgeTaskWhenAlertPurgeDelayIsZero() throws Exception { - when(configDao.getConfiguration()).thenReturn(new HashMap<>()); - overrideDefaultConfigValue(spy.AlertPurgeInterval, "_defaultValue", "10"); - overrideDefaultConfigValue(spy.AlertPurgeDelay, "_defaultValue", "0"); - ScheduledExecutorService alertExecutor = mock(ScheduledExecutorService.class); - ReflectionTestUtils.setField(spy, "_alertExecutor", alertExecutor); - - spy.configure("management-server", new HashMap<>()); - - Mockito.verifyNoInteractions(alertExecutor); - Assert.assertEquals(0, ReflectionTestUtils.getField(spy, "_alertPurgeDelay")); - } }