From 11497c601fdd91fac6f58377fcdbbfd5fb02b753 Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Tue, 27 Aug 2024 14:40:28 +0530 Subject: [PATCH 1/4] [VMware] Update data disk controller same as the root disk controller type when it is not set in the VM detail (#9433) --- .../java/com/cloud/vm/UserVmManagerImpl.java | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 1176fb31a3e..3b48378b985 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -4650,17 +4650,24 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir vm.setDetail(VmDetailConstants.DATA_DISK_CONTROLLER, dataDiskControllerSetting); } - String controllerSetting = StringUtils.defaultIfEmpty(_configDao.getValue(Config.VmwareRootDiskControllerType.key()), - Config.VmwareRootDiskControllerType.getDefaultValue()); - // Don't override if VM already has root/data disk controller detail if (vm.getDetail(VmDetailConstants.ROOT_DISK_CONTROLLER) == null) { - vm.setDetail(VmDetailConstants.ROOT_DISK_CONTROLLER, controllerSetting); + String vmwareRootDiskControllerTypeFromSetting = StringUtils.defaultIfEmpty(_configDao.getValue(Config.VmwareRootDiskControllerType.key()), + Config.VmwareRootDiskControllerType.getDefaultValue()); + vm.setDetail(VmDetailConstants.ROOT_DISK_CONTROLLER, vmwareRootDiskControllerTypeFromSetting); } + if (vm.getDetail(VmDetailConstants.DATA_DISK_CONTROLLER) == null) { - if (controllerSetting.equalsIgnoreCase("scsi")) { - vm.setDetail(VmDetailConstants.DATA_DISK_CONTROLLER, "scsi"); + String finalRootDiskController = vm.getDetail(VmDetailConstants.ROOT_DISK_CONTROLLER); + // Set the data disk controller detail same as the final scsi root disk controller if VM doesn't have data disk controller detail + // This is to ensure the disk controller is available for the data disks, as all the SCSI controllers are created with same controller type + String scsiControllerPattern = "(?i)\\b(scsi|lsilogic|lsilogicsas|lsisas1068|buslogic|pvscsi)\\b"; + if (finalRootDiskController.matches(scsiControllerPattern)) { + s_logger.info(String.format("Data disk controller was not defined, but root disk is using SCSI controller [%s]." + + "To ensure disk controllers are available for the data disks, the data disk controller is updated to match the root disk controller.", finalRootDiskController)); + vm.setDetail(VmDetailConstants.DATA_DISK_CONTROLLER, finalRootDiskController); } else { + s_logger.info("Data disk controller was not defined; defaulting to 'osdefault'."); vm.setDetail(VmDetailConstants.DATA_DISK_CONTROLLER, "osdefault"); } } From 6c0492366c0212b68487c631c1a824b9b81a46af Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Tue, 27 Aug 2024 14:41:01 +0530 Subject: [PATCH 2/4] [VMware] Disconnect/Detach config drive ISO (if exists) on stop VM (#9468) --- .../vmware/resource/VmwareResource.java | 30 ++++++++++++++++++- .../vmware/mo/VirtualMachineMO.java | 8 +++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java index 157ed75c9d0..732d40da307 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java @@ -55,6 +55,7 @@ import com.vmware.vim25.FileQueryFlags; import com.vmware.vim25.FolderFileInfo; import com.vmware.vim25.HostDatastoreBrowserSearchResults; import com.vmware.vim25.HostDatastoreBrowserSearchSpec; +import com.vmware.vim25.VirtualCdromIsoBackingInfo; import com.vmware.vim25.VirtualMachineConfigSummary; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.backup.PrepareForBackupRestorationCommand; @@ -2738,8 +2739,9 @@ public class VmwareResource extends ServerResourceBase implements StoragePoolRes private DiskTO[] getDisks(DiskTO[] sortedDisks) { return Arrays.stream(sortedDisks).filter(vol -> ((vol.getPath() != null && - vol.getPath().contains("configdrive"))) || (vol.getType() != Volume.Type.ISO)).toArray(DiskTO[]::new); + vol.getPath().contains(ConfigDrive.CONFIGDRIVEDIR))) || (vol.getType() != Volume.Type.ISO)).toArray(DiskTO[]::new); } + private void configureIso(VmwareHypervisorHost hyperHost, VirtualMachineMO vmMo, DiskTO vol, VirtualDeviceConfigSpec[] deviceConfigSpecArray, int ideUnitNumber, int i) throws Exception { TemplateObjectTO iso = (TemplateObjectTO) vol.getData(); @@ -4448,6 +4450,8 @@ public class VmwareResource extends ServerResourceBase implements StoragePoolRes msg = "Have problem in powering off VM " + cmd.getVmName() + ", let the process continue"; s_logger.warn(msg); } + + disconnectConfigDriveIsoIfExists(vmMo); return new StopAnswer(cmd, msg, true); } @@ -4466,6 +4470,30 @@ public class VmwareResource extends ServerResourceBase implements StoragePoolRes } } + private void disconnectConfigDriveIsoIfExists(VirtualMachineMO vmMo) { + try { + List isoDevices = vmMo.getIsoDevices(); + if (CollectionUtils.isEmpty(isoDevices)) { + return; + } + + for (VirtualDevice isoDevice : isoDevices) { + if (!(isoDevice.getBacking() instanceof VirtualCdromIsoBackingInfo)) { + continue; + } + String isoFilePath = ((VirtualCdromIsoBackingInfo)isoDevice.getBacking()).getFileName(); + if (!isoFilePath.contains(ConfigDrive.CONFIGDRIVEDIR)) { + continue; + } + s_logger.info(String.format("Disconnecting config drive at location: %s", isoFilePath)); + vmMo.detachIso(isoFilePath, true); + return; + } + } catch (Exception e) { + s_logger.warn(String.format("Couldn't check/disconnect config drive, error: %s", e.getMessage()), e); + } + } + protected Answer execute(RebootRouterCommand cmd) { RebootAnswer answer = (RebootAnswer) execute((RebootCommand) cmd); diff --git a/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java b/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java index d21d20b791c..742f33575f5 100644 --- a/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java +++ b/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java @@ -3203,6 +3203,14 @@ public class VirtualMachineMO extends BaseMO { return null; } + public List getIsoDevices() throws Exception { + List devices = _context.getVimClient().getDynamicProperty(_mor, "config.hardware.device"); + if (CollectionUtils.isEmpty(devices)) { + return new ArrayList<>(); + } + return devices.stream().filter(device -> device instanceof VirtualCdrom).collect(Collectors.toList()); + } + public VirtualDevice getIsoDevice(int key) throws Exception { List devices = _context.getVimClient().getDynamicProperty(_mor, "config.hardware.device"); if (devices != null && devices.size() > 0) { From 674129cd588e9be67923fc06d652170f405f2d07 Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Tue, 27 Aug 2024 16:07:28 +0530 Subject: [PATCH 3/4] Update project account for all the events with project account owner, except for create project event (#9572) --- .../user/account/AddUserToProjectCmd.java | 2 +- .../user/account/DeleteUserFromProjectCmd.java | 1 - .../com/cloud/event/ActionEventInterceptor.java | 4 ++-- .../java/com/cloud/event/ActionEventUtils.java | 11 +++++++++-- .../com/cloud/projects/ProjectManagerImpl.java | 16 ++++++++-------- 5 files changed, 20 insertions(+), 14 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/account/AddUserToProjectCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/account/AddUserToProjectCmd.java index d38ae057f05..9cd845c774c 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/account/AddUserToProjectCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/account/AddUserToProjectCmd.java @@ -103,7 +103,7 @@ public class AddUserToProjectCmd extends BaseAsyncCmd { @Override public String getEventDescription() { - return "Adding user "+getUsername()+" to Project "+getProjectId(); + return "Adding user " + getUsername() + " to project: " + getProjectId(); } ///////////////////////////////////////////////////// diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/account/DeleteUserFromProjectCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/account/DeleteUserFromProjectCmd.java index 596fb876008..fbcffb7332c 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/account/DeleteUserFromProjectCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/account/DeleteUserFromProjectCmd.java @@ -83,7 +83,6 @@ public class DeleteUserFromProjectCmd extends BaseAsyncCmd { return "Removing user " + userId + " from project: " + projectId; } - @Override public long getEntityOwnerId() { Project project = _projectService.getProject(projectId); diff --git a/server/src/main/java/com/cloud/event/ActionEventInterceptor.java b/server/src/main/java/com/cloud/event/ActionEventInterceptor.java index ee025c825f5..9554ae34cb6 100644 --- a/server/src/main/java/com/cloud/event/ActionEventInterceptor.java +++ b/server/src/main/java/com/cloud/event/ActionEventInterceptor.java @@ -88,13 +88,13 @@ public class ActionEventInterceptor implements ComponentMethodInterceptor, Metho for (ActionEvent actionEvent : getActionEvents(method)) { CallContext ctx = CallContext.current(); long userId = ctx.getCallingUserId(); - long accountId = ctx.getProject() != null ? ctx.getProject().getProjectAccountId() : ctx.getCallingAccountId(); //This should be the entity owner id rather than the Calling User Account Id. long startEventId = ctx.getStartEventId(); String eventDescription = getEventDescription(actionEvent, ctx); Long eventResourceId = getEventResourceId(actionEvent, ctx); String eventResourceType = getEventResourceType(actionEvent, ctx); String eventType = getEventType(actionEvent, ctx); boolean isEventDisplayEnabled = ctx.isEventDisplayEnabled(); + long accountId = ActionEventUtils.getOwnerAccountId(ctx, eventType, ctx.getCallingAccountId()); if (eventType.equals("")) return; @@ -118,13 +118,13 @@ public class ActionEventInterceptor implements ComponentMethodInterceptor, Metho for (ActionEvent actionEvent : getActionEvents(method)) { CallContext ctx = CallContext.current(); long userId = ctx.getCallingUserId(); - long accountId = ctx.getCallingAccountId(); long startEventId = ctx.getStartEventId(); String eventDescription = getEventDescription(actionEvent, ctx); Long eventResourceId = getEventResourceId(actionEvent, ctx); String eventResourceType = getEventResourceType(actionEvent, ctx); String eventType = getEventType(actionEvent, ctx); boolean isEventDisplayEnabled = ctx.isEventDisplayEnabled(); + long accountId = ActionEventUtils.getOwnerAccountId(ctx, eventType, ctx.getCallingAccountId()); if (eventType.equals("")) return; diff --git a/server/src/main/java/com/cloud/event/ActionEventUtils.java b/server/src/main/java/com/cloud/event/ActionEventUtils.java index 36461d20e42..87dd4ef8af9 100644 --- a/server/src/main/java/com/cloud/event/ActionEventUtils.java +++ b/server/src/main/java/com/cloud/event/ActionEventUtils.java @@ -22,6 +22,7 @@ import java.lang.reflect.Method; import java.text.SimpleDateFormat; import java.util.Date; import java.util.HashMap; +import java.util.List; import java.util.Map; import javax.annotation.PostConstruct; @@ -110,6 +111,8 @@ public class ActionEventUtils { */ public static Long onScheduledActionEvent(Long userId, Long accountId, String type, String description, Long resourceId, String resourceType, boolean eventDisplayEnabled, long startEventId) { Ternary resourceDetails = getResourceDetails(resourceId, resourceType, type); + CallContext ctx = CallContext.current(); + accountId = getOwnerAccountId(ctx, type, accountId); publishOnEventBus(userId, accountId, EventCategory.ACTION_EVENT.getName(), type, com.cloud.event.Event.State.Scheduled, description, resourceDetails.second(), resourceDetails.third()); Event event = persistActionEvent(userId, accountId, null, null, type, Event.State.Scheduled, eventDisplayEnabled, description, resourceDetails.first(), resourceDetails.third(), startEventId); return event.getId(); @@ -123,7 +126,7 @@ public class ActionEventUtils { public static void onStartedActionEventFromContext(String eventType, String eventDescription, Long resourceId, String resourceType, boolean eventDisplayEnabled) { CallContext ctx = CallContext.current(); long userId = ctx.getCallingUserId(); - long accountId = ctx.getProject() != null ? ctx.getProject().getProjectAccountId() : ctx.getCallingAccountId(); //This should be the entity owner id rather than the Calling User Account Id. + long accountId = getOwnerAccountId(ctx, eventType, ctx.getCallingAccountId()); long startEventId = ctx.getStartEventId(); if (!eventType.equals("")) @@ -393,7 +396,11 @@ public class ActionEventUtils { s_logger.trace("Caught exception while populating first class entities for event bus, moving on"); } } - } + public static long getOwnerAccountId(CallContext ctx, String eventType, long callingAccountId) { + List mainProjectEvents = List.of(EventTypes.EVENT_PROJECT_CREATE, EventTypes.EVENT_PROJECT_UPDATE, EventTypes.EVENT_PROJECT_DELETE); + long accountId = ctx.getProject() != null && !mainProjectEvents.stream().anyMatch(eventType::equalsIgnoreCase) ? ctx.getProject().getProjectAccountId() : callingAccountId; //This should be the entity owner id rather than the Calling User Account Id. + return accountId; + } } diff --git a/server/src/main/java/com/cloud/projects/ProjectManagerImpl.java b/server/src/main/java/com/cloud/projects/ProjectManagerImpl.java index 19776d4993f..abdd48cd4ae 100644 --- a/server/src/main/java/com/cloud/projects/ProjectManagerImpl.java +++ b/server/src/main/java/com/cloud/projects/ProjectManagerImpl.java @@ -279,16 +279,16 @@ public class ProjectManagerImpl extends ManagerBase implements ProjectManager, C assignAccountToProject(project, ownerFinal.getId(), ProjectAccount.Role.Admin, Optional.ofNullable(finalUser).map(User::getId).orElse(null), null); - if (project != null) { - CallContext.current().setEventDetails("Project id=" + project.getId()); - CallContext.current().putContextParameter(Project.class, project.getUuid()); - } + if (project != null) { + CallContext.current().setEventDetails("Project id=" + project.getId()); + CallContext.current().putContextParameter(Project.class, project.getUuid()); + } - //Increment resource count + //Increment resource count _resourceLimitMgr.incrementResourceCount(ownerFinal.getId(), ResourceType.project); - return project; - } + return project; + } }); messageBus.publish(_name, ProjectManager.MESSAGE_CREATE_TUNGSTEN_PROJECT_EVENT, PublishScope.LOCAL, project); @@ -1275,7 +1275,7 @@ public class ProjectManagerImpl extends ManagerBase implements ProjectManager, C } @Override - @ActionEvent(eventType = EventTypes.EVENT_PROJECT_ACTIVATE, eventDescription = "activating project") + @ActionEvent(eventType = EventTypes.EVENT_PROJECT_ACTIVATE, eventDescription = "activating project", async = true) @DB public Project activateProject(final long projectId) { Account caller = CallContext.current().getCallingAccount(); From 48e745cad28d4ff146b57c62501bbcb908308082 Mon Sep 17 00:00:00 2001 From: Harikrishna Date: Wed, 28 Aug 2024 15:06:44 +0530 Subject: [PATCH 4/4] Add certificate validation to check headers (#9255) --- .../security/keystore/KeystoreManager.java | 3 ++- .../keystore/KeystoreManagerImpl.java | 23 +++++++++------- .../cloud/server/ManagementServerImpl.java | 27 ++++++++++++++++--- 3 files changed, 39 insertions(+), 14 deletions(-) diff --git a/framework/security/src/main/java/org/apache/cloudstack/framework/security/keystore/KeystoreManager.java b/framework/security/src/main/java/org/apache/cloudstack/framework/security/keystore/KeystoreManager.java index c44347c85ef..18b840e7a8c 100644 --- a/framework/security/src/main/java/org/apache/cloudstack/framework/security/keystore/KeystoreManager.java +++ b/framework/security/src/main/java/org/apache/cloudstack/framework/security/keystore/KeystoreManager.java @@ -18,6 +18,7 @@ package org.apache.cloudstack.framework.security.keystore; import com.cloud.agent.api.LogLevel; import com.cloud.agent.api.LogLevel.Log4jLevel; +import com.cloud.utils.Pair; import com.cloud.utils.component.Manager; public interface KeystoreManager extends Manager { @@ -59,7 +60,7 @@ public interface KeystoreManager extends Manager { } } - boolean validateCertificate(String certificate, String key, String domainSuffix); + Pair validateCertificate(String certificate, String key, String domainSuffix); void saveCertificate(String name, String certificate, String key, String domainSuffix); diff --git a/framework/security/src/main/java/org/apache/cloudstack/framework/security/keystore/KeystoreManagerImpl.java b/framework/security/src/main/java/org/apache/cloudstack/framework/security/keystore/KeystoreManagerImpl.java index 03a91fecc8e..f89b6eebf9b 100644 --- a/framework/security/src/main/java/org/apache/cloudstack/framework/security/keystore/KeystoreManagerImpl.java +++ b/framework/security/src/main/java/org/apache/cloudstack/framework/security/keystore/KeystoreManagerImpl.java @@ -30,6 +30,7 @@ import java.util.regex.Pattern; import javax.inject.Inject; +import com.cloud.utils.Pair; import org.apache.commons.lang3.StringUtils; import org.apache.log4j.Logger; import org.springframework.stereotype.Component; @@ -47,24 +48,28 @@ public class KeystoreManagerImpl extends ManagerBase implements KeystoreManager private KeystoreDao _ksDao; @Override - public boolean validateCertificate(String certificate, String key, String domainSuffix) { + public Pair validateCertificate(String certificate, String key, String domainSuffix) { + String errMsg = null; if (StringUtils.isAnyEmpty(certificate, key, domainSuffix)) { - s_logger.error("Invalid parameter found in (certificate, key, domainSuffix) tuple for domain: " + domainSuffix); - return false; + errMsg = String.format("Invalid parameter found in (certificate, key, domainSuffix) tuple for domain: %s", domainSuffix); + s_logger.error(errMsg); + return new Pair<>(false, errMsg); } try { String ksPassword = "passwordForValidation"; byte[] ksBits = CertificateHelper.buildAndSaveKeystore(domainSuffix, certificate, getKeyContent(key), ksPassword); KeyStore ks = CertificateHelper.loadKeystore(ksBits, ksPassword); - if (ks != null) - return true; - - s_logger.error("Unabled to construct keystore for domain: " + domainSuffix); + if (ks != null) { + return new Pair<>(true, errMsg); + } + errMsg = String.format("Unable to construct keystore for domain: %s", domainSuffix); + s_logger.error(errMsg); } catch (Exception e) { - s_logger.error("Certificate validation failed due to exception for domain: " + domainSuffix, e); + errMsg = String.format("Certificate validation failed due to exception for domain: %s", domainSuffix); + s_logger.error(errMsg, e); } - return false; + return new Pair<>(false, errMsg); } @Override diff --git a/server/src/main/java/com/cloud/server/ManagementServerImpl.java b/server/src/main/java/com/cloud/server/ManagementServerImpl.java index 9d69a52cfb8..5ac34fba1d5 100644 --- a/server/src/main/java/com/cloud/server/ManagementServerImpl.java +++ b/server/src/main/java/com/cloud/server/ManagementServerImpl.java @@ -17,6 +17,7 @@ package com.cloud.server; import java.lang.reflect.Field; +import java.security.cert.CertificateException; import java.util.ArrayList; import java.util.Arrays; import java.util.Calendar; @@ -43,6 +44,7 @@ import javax.crypto.spec.SecretKeySpec; import javax.inject.Inject; import javax.naming.ConfigurationException; +import com.cloud.utils.security.CertificateHelper; import org.apache.cloudstack.acl.ControlledEntity; import org.apache.cloudstack.acl.SecurityChecker; import org.apache.cloudstack.affinity.AffinityGroupProcessor; @@ -4484,13 +4486,12 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe final String certificate = cmd.getCertificate(); final String key = cmd.getPrivateKey(); + String domainSuffix = cmd.getDomainSuffix(); - if (cmd.getPrivateKey() != null && !_ksMgr.validateCertificate(certificate, key, cmd.getDomainSuffix())) { - throw new InvalidParameterValueException("Failed to pass certificate validation check"); - } + validateCertificate(certificate, key, domainSuffix); if (cmd.getPrivateKey() != null) { - _ksMgr.saveCertificate(ConsoleProxyManager.CERTIFICATE_NAME, certificate, key, cmd.getDomainSuffix()); + _ksMgr.saveCertificate(ConsoleProxyManager.CERTIFICATE_NAME, certificate, key, domainSuffix); // Reboot ssvm here since private key is present - meaning server cert being passed final List alreadyRunning = _secStorageVmDao.getSecStorageVmListInStates(null, State.Running, State.Migrating, State.Starting); @@ -4507,6 +4508,24 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe + "please give a few minutes for console access and storage services service to be up and working again"; } + private void validateCertificate(String certificate, String key, String domainSuffix) { + if (key != null) { + Pair result = _ksMgr.validateCertificate(certificate, key, domainSuffix); + if (!result.first()) { + throw new InvalidParameterValueException(String.format("Failed to pass certificate validation check with error: %s", result.second())); + } + } else { + try { + s_logger.debug(String.format("Trying to validate the root certificate format")); + CertificateHelper.buildCertificate(certificate); + } catch (CertificateException e) { + String errorMsg = String.format("Failed to pass certificate validation check with error: Certificate validation failed due to exception: %s", e.getMessage()); + s_logger.error(errorMsg); + throw new InvalidParameterValueException(errorMsg); + } + } + } + @Override public List getHypervisors(final Long zoneId) { final List result = new ArrayList();