From 7a8e3ab6a65d973bd5f3d14f17e0e80edae140f9 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Tue, 9 Sep 2025 19:11:26 +0530 Subject: [PATCH 01/14] extension/proxmox: add console access for instances Add console access support for the instances deployed using inbuilt Proxmox Orchestrator extension. Underlying CloudStack queries Proxmox API for console and then passes the ticket and host to the CPVM for the zone. During the flow it is assumed CPVM will be able to access the instance (Proxmox) host. Signed-off-by: Abhishek Kumar --- .../agent/api/GetExternalConsoleAnswer.java | 54 +++++ .../agent/api/GetExternalConsoleCommand.java | 53 +++++ .../agent/api/RunCustomActionCommand.java | 12 +- .../cloud/hypervisor/ExternalProvisioner.java | 4 + extensions/Proxmox/proxmox.sh | 85 ++++++++ .../extensions/manager/ExtensionsManager.java | 4 + .../manager/ExtensionsManagerImpl.java | 31 ++- .../ExternalPathPayloadProvisioner.java | 77 +++++-- .../external/resource/ExternalResource.java | 11 + .../ExternalPathPayloadProvisionerTest.java | 13 -- .../external/provisioner/provisioner.sh | 11 + .../com/cloud/consoleproxy/AgentHookBase.java | 9 +- .../com/cloud/server/ManagementServer.java | 4 + .../cloud/server/ManagementServerImpl.java | 27 +-- .../cloud/servlet/ConsoleProxyServlet.java | 19 +- .../ConsoleAccessManagerImpl.java | 196 +++++++++++++----- ui/src/components/view/ActionButton.vue | 4 +- 17 files changed, 503 insertions(+), 111 deletions(-) create mode 100644 core/src/main/java/com/cloud/agent/api/GetExternalConsoleAnswer.java create mode 100644 core/src/main/java/com/cloud/agent/api/GetExternalConsoleCommand.java diff --git a/core/src/main/java/com/cloud/agent/api/GetExternalConsoleAnswer.java b/core/src/main/java/com/cloud/agent/api/GetExternalConsoleAnswer.java new file mode 100644 index 000000000000..f2f6f2aa9ce1 --- /dev/null +++ b/core/src/main/java/com/cloud/agent/api/GetExternalConsoleAnswer.java @@ -0,0 +1,54 @@ +// 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.agent.api; + +public class GetExternalConsoleAnswer extends Answer { + + private String host; + private int port; + private String password; + private String protocol; + + public GetExternalConsoleAnswer(Command command, String details) { + super(command, false, details); + } + + public GetExternalConsoleAnswer(Command command, String host, int port, String password, String protocol) { + super(command, true, ""); + this.host = host; + this.port = port; + this.password = password; + this.protocol = protocol; + } + + public String getHost() { + return host; + } + + public int getPort() { + return port; + } + + public String getPassword() { + return password; + } + + public String getProtocol() { + return protocol; + } +} diff --git a/core/src/main/java/com/cloud/agent/api/GetExternalConsoleCommand.java b/core/src/main/java/com/cloud/agent/api/GetExternalConsoleCommand.java new file mode 100644 index 000000000000..fc2134f631fb --- /dev/null +++ b/core/src/main/java/com/cloud/agent/api/GetExternalConsoleCommand.java @@ -0,0 +1,53 @@ +// 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.agent.api; + +import com.cloud.agent.api.to.VirtualMachineTO; + +public class GetExternalConsoleCommand extends Command { + String vmName; + VirtualMachineTO vm; + protected boolean executeInSequence; + + public GetExternalConsoleCommand(String vmName, VirtualMachineTO vm) { + this.vmName = vmName; + this.vm = vm; + this.executeInSequence = false; + } + + public String getVmName() { + return this.vmName; + } + + public void setVirtualMachine(VirtualMachineTO vm) { + this.vm = vm; + } + + public VirtualMachineTO getVirtualMachine() { + return vm; + } + + @Override + public boolean executeInSequence() { + return executeInSequence; + } + + public void setExecuteInSequence(boolean executeInSequence) { + this.executeInSequence = executeInSequence; + } +} diff --git a/core/src/main/java/com/cloud/agent/api/RunCustomActionCommand.java b/core/src/main/java/com/cloud/agent/api/RunCustomActionCommand.java index 36489ad4fa5f..113073ac5ee5 100644 --- a/core/src/main/java/com/cloud/agent/api/RunCustomActionCommand.java +++ b/core/src/main/java/com/cloud/agent/api/RunCustomActionCommand.java @@ -21,10 +21,12 @@ import java.util.Map; +import com.cloud.agent.api.to.VirtualMachineTO; + public class RunCustomActionCommand extends Command { String actionName; - Long vmId; + VirtualMachineTO vmTO; Map parameters; public RunCustomActionCommand(String actionName) { @@ -36,12 +38,12 @@ public String getActionName() { return actionName; } - public Long getVmId() { - return vmId; + public VirtualMachineTO getVmTO() { + return vmTO; } - public void setVmId(Long vmId) { - this.vmId = vmId; + public void setVmTO(VirtualMachineTO vmTO) { + this.vmTO = vmTO; } public Map getParameters() { diff --git a/engine/components-api/src/main/java/com/cloud/hypervisor/ExternalProvisioner.java b/engine/components-api/src/main/java/com/cloud/hypervisor/ExternalProvisioner.java index a22ea4211132..c574a8be0175 100644 --- a/engine/components-api/src/main/java/com/cloud/hypervisor/ExternalProvisioner.java +++ b/engine/components-api/src/main/java/com/cloud/hypervisor/ExternalProvisioner.java @@ -18,6 +18,8 @@ import java.util.Map; +import com.cloud.agent.api.GetExternalConsoleAnswer; +import com.cloud.agent.api.GetExternalConsoleCommand; import com.cloud.agent.api.HostVmStateReportEntry; import com.cloud.agent.api.PrepareExternalProvisioningAnswer; import com.cloud.agent.api.PrepareExternalProvisioningCommand; @@ -57,5 +59,7 @@ public interface ExternalProvisioner extends Manager { Map getHostVmStateReport(long hostId, String extensionName, String extensionRelativePath); + GetExternalConsoleAnswer getInstanceConsole(String hostGuid, String extensionName, String extensionRelativePath, GetExternalConsoleCommand cmd); + RunCustomActionAnswer runCustomAction(String hostGuid, String extensionName, String extensionRelativePath, RunCustomActionCommand cmd); } diff --git a/extensions/Proxmox/proxmox.sh b/extensions/Proxmox/proxmox.sh index 7f363a6b9a0d..a6d7a91e9376 100755 --- a/extensions/Proxmox/proxmox.sh +++ b/extensions/Proxmox/proxmox.sh @@ -285,6 +285,88 @@ status() { echo "{\"status\": \"success\", \"power_state\": \"$powerstate\"}" } +get_node_host() { + check_required_fields node + local net_json host + + if ! net_json="$(call_proxmox_api GET "/nodes/${node}/network")"; then + echo "" + return 1 + fi + + # Prefer a static non-bridge IP + host="$(echo "$net_json" | jq -r ' + .data + | map(select( + (.type // "") != "bridge" and + (.type // "") != "bond" and + (.method // "") == "static" and + ((.address // .cidr // "") != "") + )) + | map(.address // (.cidr | split("/")[0])) + | .[0] // empty + ' 2>/dev/null)" + + # Fallback: first interface with a CIDR + if [[ -z "$host" ]]; then + host="$(echo "$net_json" | jq -r ' + .data + | map(select((.cidr // "") != "")) + | map(.cidr | split("/")[0]) + | .[0] // empty + ' 2>/dev/null)" + fi + + echo "$host" + } + + get_console() { + check_required_fields node vmid + + # Request VNC proxy from Proxmox + local api_resp port ticket + if ! api_resp="$(call_proxmox_api POST "/nodes/${node}/qemu/${vmid}/vncproxy")"; then + jq -n --arg msg "API call failed for node=$node vmid=$vmid" \ + '{status:"error", message:$msg, code:"API_CALL_FAILED"}' + return 1 + fi + + port="$(echo "$api_resp" | jq -re '.data.port // empty' 2>/dev/null || true)" + ticket="$(echo "$api_resp" | jq -re '.data.ticket // empty' 2>/dev/null || true)" + + if [[ -z "$port" || -z "$ticket" ]]; then + jq -n --arg msg "Proxmox response missing port/ticket" \ + --arg raw "$api_resp" \ + '{status:"error", message:$msg, code:"BAD_UPSTREAM_RESPONSE", upstream:$raw}' + return 1 + fi + + # Derive host from node’s network info + local host + host="$(get_node_host)" + if [[ -z "$host" ]]; then + jq -n --arg msg "Could not determine host IP for node $node" \ + '{status:"error", message:$msg, code:"HOST_RESOLUTION_ERROR"}' + return 1 + fi + + # Success JSON to stdout + jq -n \ + --arg host "$host" \ + --arg port "$port" \ + --arg password "$ticket" \ + '{ + status: "success", + message: "Console retrieved", + console: { + host: $host, + port: $port, + password: $password, + protocol: "vnc" + } + }' + } + list_snapshots() { snapshot_response=$(call_proxmox_api GET "/nodes/${node}/qemu/${vmid}/snapshot") echo "$snapshot_response" | jq ' @@ -396,6 +478,9 @@ case $action in status) status ;; + getconsole) + get_console "$parameters" + ;; ListSnapshots) list_snapshots ;; diff --git a/framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManager.java b/framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManager.java index 82174872e87d..1b1a175c5975 100644 --- a/framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManager.java +++ b/framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManager.java @@ -44,10 +44,12 @@ import org.apache.cloudstack.framework.extensions.api.UpdateExtensionCmd; import org.apache.cloudstack.framework.extensions.command.ExtensionServerActionBaseCommand; +import com.cloud.agent.api.Answer; import com.cloud.host.Host; import com.cloud.org.Cluster; import com.cloud.utils.Pair; import com.cloud.utils.component.Manager; +import com.cloud.vm.VirtualMachine; public interface ExtensionsManager extends Manager { @@ -93,4 +95,6 @@ Pair extensionResourceMapDetailsNeedUpdate(final final ExtensionResourceMap.ResourceType resourceType, final Map details); void updateExtensionResourceMapDetails(final long extensionResourceMapId, final Map details); + + Answer getInstanceConsole(VirtualMachine vm, Host host); } diff --git a/framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImpl.java b/framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImpl.java index 5abf0f424a7c..b30cbe668bf6 100644 --- a/framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImpl.java +++ b/framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImpl.java @@ -104,8 +104,10 @@ import com.cloud.agent.AgentManager; import com.cloud.agent.api.Answer; import com.cloud.agent.api.Command; +import com.cloud.agent.api.GetExternalConsoleCommand; import com.cloud.agent.api.RunCustomActionAnswer; import com.cloud.agent.api.RunCustomActionCommand; +import com.cloud.agent.api.to.VirtualMachineTO; import com.cloud.alert.AlertManager; import com.cloud.cluster.ClusterManager; import com.cloud.cluster.ManagementServerHostVO; @@ -141,6 +143,8 @@ import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.vm.VirtualMachine; import com.cloud.vm.VirtualMachineManager; +import com.cloud.vm.VirtualMachineProfile; +import com.cloud.vm.VirtualMachineProfileImpl; import com.cloud.vm.VmDetailConstants; import com.cloud.vm.dao.VMInstanceDao; @@ -1323,11 +1327,13 @@ public CustomActionResultResponse runCustomAction(RunCustomActionCmd cmd) { clusterId = host.getClusterId(); } else if (entity instanceof VirtualMachine) { VirtualMachine virtualMachine = (VirtualMachine)entity; - runCustomActionCommand.setVmId(virtualMachine.getId()); if (!Hypervisor.HypervisorType.External.equals(virtualMachine.getHypervisorType())) { logger.error("Invalid {} specified as VM resource for running {}", entity, customActionVO); throw new InvalidParameterValueException(error); } + VirtualMachineProfile vmProfile = new VirtualMachineProfileImpl(virtualMachine); + VirtualMachineTO virtualMachineTO = virtualMachineManager.toVmTO(vmProfile); + runCustomActionCommand.setVmTO(virtualMachineTO); Pair clusterAndHostId = virtualMachineManager.findClusterAndHostIdForVm(virtualMachine, false); clusterId = clusterAndHostId.first(); hostId = clusterAndHostId.second(); @@ -1369,6 +1375,13 @@ public CustomActionResultResponse runCustomAction(RunCustomActionCmd cmd) { actionResourceType, entity)); Map> externalDetails = getExternalAccessDetails(allDetails.first(), hostId, extensionResource); + Map vmExternalDetails = null; + if (runCustomActionCommand.getVmTO() != null) { + vmExternalDetails = runCustomActionCommand.getVmTO().getExternalDetails(); + } + if (MapUtils.isNotEmpty(vmExternalDetails)) { + externalDetails.put(ApiConstants.VIRTUAL_MACHINE, vmExternalDetails); + } runCustomActionCommand.setParameters(parameters); runCustomActionCommand.setExternalDetails(externalDetails); runCustomActionCommand.setWait(customActionVO.getTimeout()); @@ -1517,6 +1530,22 @@ public void updateExtensionResourceMapDetails(long extensionResourceMapId, Map> externalAccessDetails = + getExternalAccessDetails(host, virtualMachineTO.getExternalDetails()); + cmd.setExternalDetails(externalAccessDetails); + return agentMgr.easySend(host.getId(), cmd); + } + @Override public Long getExtensionIdForCluster(long clusterId) { ExtensionResourceMapVO map = extensionResourceMapDao.findByResourceIdAndType(clusterId, diff --git a/plugins/hypervisors/external/src/main/java/org/apache/cloudstack/hypervisor/external/provisioner/ExternalPathPayloadProvisioner.java b/plugins/hypervisors/external/src/main/java/org/apache/cloudstack/hypervisor/external/provisioner/ExternalPathPayloadProvisioner.java index 5a1632ce9777..d8f2709b8b68 100644 --- a/plugins/hypervisors/external/src/main/java/org/apache/cloudstack/hypervisor/external/provisioner/ExternalPathPayloadProvisioner.java +++ b/plugins/hypervisors/external/src/main/java/org/apache/cloudstack/hypervisor/external/provisioner/ExternalPathPayloadProvisioner.java @@ -54,8 +54,11 @@ import org.apache.cloudstack.utils.security.DigestHelper; import org.apache.commons.collections.CollectionUtils; import org.apache.commons.collections.MapUtils; +import org.apache.commons.lang3.ObjectUtils; import com.cloud.agent.AgentManager; +import com.cloud.agent.api.GetExternalConsoleAnswer; +import com.cloud.agent.api.GetExternalConsoleCommand; import com.cloud.agent.api.HostVmStateReportEntry; import com.cloud.agent.api.PrepareExternalProvisioningAnswer; import com.cloud.agent.api.PrepareExternalProvisioningCommand; @@ -85,7 +88,6 @@ import com.cloud.utils.json.JsonMergeUtil; import com.cloud.utils.script.Script; import com.cloud.vm.UserVmVO; -import com.cloud.vm.VMInstanceVO; import com.cloud.vm.VirtualMachine; import com.cloud.vm.VirtualMachineProfile; import com.cloud.vm.VirtualMachineProfileImpl; @@ -208,6 +210,15 @@ protected void createOrCheckExtensionsDataDirectory() throws ConfigurationExcept logger.info("Extensions data directory path: {}", extensionsDataDirectory); } + protected VirtualMachineTO getVirtualMachineTO(VirtualMachine vm) { + if (vm == null) { + return null; + } + final HypervisorGuru hvGuru = hypervisorGuruManager.getGuru(Hypervisor.HypervisorType.External); + VirtualMachineProfile profile = new VirtualMachineProfileImpl(vm); + return hvGuru.implement(profile); + } + private String getServerProperty(String name) { Properties props = propertiesRef.get(); if (props == null) { @@ -393,7 +404,6 @@ public RebootAnswer rebootInstance(String hostGuid, String extensionName, String if (StringUtils.isEmpty(extensionPath)) { return new RebootAnswer(cmd, "Extension not configured", false); } - logger.debug("Executing reboot command using IPMI in the external provisioner"); VirtualMachineTO virtualMachineTO = cmd.getVirtualMachine(); String vmUUID = virtualMachineTO.getUuid(); logger.debug("Executing reboot command in the external system for the VM {}", vmUUID); @@ -455,6 +465,49 @@ public Map getHostVmStateReport(long hostId, Str return vmStates; } + @Override + public GetExternalConsoleAnswer getInstanceConsole(String hostGuid, String extensionName, + String extensionRelativePath, GetExternalConsoleCommand cmd) { + String extensionPath = getExtensionCheckedPath(extensionName, extensionRelativePath); + if (StringUtils.isEmpty(extensionPath)) { + return new GetExternalConsoleAnswer(cmd, "Extension not configured"); + } + VirtualMachineTO virtualMachineTO = cmd.getVirtualMachine(); + String vmUUID = virtualMachineTO.getUuid(); + logger.debug("Executing getconsole command in the external system for the VM {}", vmUUID); + Map accessDetails = loadAccessDetails(cmd.getExternalDetails(), virtualMachineTO); + Pair result = getInstanceConsoleOnExternalSystem(extensionName, extensionPath, vmUUID, + accessDetails, cmd.getWait()); + if (result == null) { + return new GetExternalConsoleAnswer(cmd, "No response from external system"); + } + String output = result.second(); + if (!result.first()) { + return new GetExternalConsoleAnswer(cmd, output); + } + logger.debug("Received console details from the external system: {}", output); + try { + JsonObject jsonObj = JsonParser.parseString(output).getAsJsonObject(); + JsonObject consoleObj = jsonObj.has("console") ? jsonObj.getAsJsonObject("console") : null; + if (consoleObj == null) { + logger.error("Missing console object in external console output: {}", output); + return new GetExternalConsoleAnswer(cmd, "Missing console object in output"); + } + String host = consoleObj.has("host") ? consoleObj.get("host").getAsString() : null; + Integer port = consoleObj.has("port") ? Integer.valueOf(consoleObj.get("port").getAsString()) : null; + String password = consoleObj.has("password") ? consoleObj.get("password").getAsString() : null; + String protocol = consoleObj.has("protocol") ? consoleObj.get("protocol").getAsString() : null; + if (ObjectUtils.anyNull(host, port, password)) { + logger.error("Missing required fields in external console output: {}", output); + return new GetExternalConsoleAnswer(cmd, "Missing required fields in output"); + } + return new GetExternalConsoleAnswer(cmd, host, port, password, protocol); + } catch (RuntimeException e) { + logger.error("Failed to parse output for getInstanceConsole: {}", e.getMessage(), e); + return new GetExternalConsoleAnswer(cmd, "Failed to parse output"); + } + } + @Override public RunCustomActionAnswer runCustomAction(String hostGuid, String extensionName, String extensionRelativePath, RunCustomActionCommand cmd) { @@ -464,16 +517,8 @@ public RunCustomActionAnswer runCustomAction(String hostGuid, String extensionNa } final String actionName = cmd.getActionName(); final Map parameters = cmd.getParameters(); - logger.debug("Executing custom action '{}' in the external provisioner", actionName); - VirtualMachineTO virtualMachineTO = null; - if (cmd.getVmId() != null) { - VMInstanceVO vm = vmInstanceDao.findById(cmd.getVmId()); - final HypervisorGuru hvGuru = hypervisorGuruManager.getGuru(Hypervisor.HypervisorType.External); - VirtualMachineProfile profile = new VirtualMachineProfileImpl(vm); - virtualMachineTO = hvGuru.implement(profile); - } logger.debug("Executing custom action '{}' in the external system", actionName); - Map accessDetails = loadAccessDetails(cmd.getExternalDetails(), virtualMachineTO); + Map accessDetails = loadAccessDetails(cmd.getExternalDetails(), cmd.getVmTO()); accessDetails.put(ApiConstants.ACTION, actionName); if (MapUtils.isNotEmpty(parameters)) { accessDetails.put(ApiConstants.PARAMETERS, parameters); @@ -659,9 +704,7 @@ protected VirtualMachine.PowerState parsePowerStateFromResponse(UserVmVO userVmV private VirtualMachine.PowerState getVmPowerState(UserVmVO userVmVO, Map> accessDetails, String extensionName, String extensionPath) { - final HypervisorGuru hvGuru = hypervisorGuruManager.getGuru(Hypervisor.HypervisorType.External); - VirtualMachineProfile profile = new VirtualMachineProfileImpl(userVmVO); - VirtualMachineTO virtualMachineTO = hvGuru.implement(profile); + VirtualMachineTO virtualMachineTO = getVirtualMachineTO(userVmVO); accessDetails.put(ApiConstants.VIRTUAL_MACHINE, virtualMachineTO.getExternalDetails()); Map modifiedDetails = loadAccessDetails(accessDetails, virtualMachineTO); String vmUUID = userVmVO.getUuid(); @@ -718,6 +761,12 @@ public Pair getInstanceStatusOnExternalSystem(String extensionN String.format("Failed to get the instance power status %s on external system", vmUUID), filename); } + public Pair getInstanceConsoleOnExternalSystem(String extensionName, String filename, + String vmUUID, Map accessDetails, int wait) { + return executeExternalCommand(extensionName, "getconsole", accessDetails, wait, + String.format("Failed to get the instance console %s on external system", vmUUID), filename); + } + public Pair executeExternalCommand(String extensionName, String action, Map accessDetails, int wait, String errorLogPrefix, String file) { try { diff --git a/plugins/hypervisors/external/src/main/java/org/apache/cloudstack/hypervisor/external/resource/ExternalResource.java b/plugins/hypervisors/external/src/main/java/org/apache/cloudstack/hypervisor/external/resource/ExternalResource.java index ab70c880a81f..02747350dd67 100644 --- a/plugins/hypervisors/external/src/main/java/org/apache/cloudstack/hypervisor/external/resource/ExternalResource.java +++ b/plugins/hypervisors/external/src/main/java/org/apache/cloudstack/hypervisor/external/resource/ExternalResource.java @@ -35,6 +35,8 @@ import com.cloud.agent.api.CheckNetworkCommand; import com.cloud.agent.api.CleanupNetworkRulesCmd; import com.cloud.agent.api.Command; +import com.cloud.agent.api.GetExternalConsoleAnswer; +import com.cloud.agent.api.GetExternalConsoleCommand; import com.cloud.agent.api.GetHostStatsAnswer; import com.cloud.agent.api.GetHostStatsCommand; import com.cloud.agent.api.GetVmStatsCommand; @@ -162,6 +164,8 @@ public Answer executeRequest(Command cmd) { return execute((StopCommand) cmd); } else if (cmd instanceof RebootCommand) { return execute((RebootCommand) cmd); + } else if (cmd instanceof GetExternalConsoleCommand) { + return execute((GetExternalConsoleCommand) cmd); } else if (cmd instanceof PrepareExternalProvisioningCommand) { return execute((PrepareExternalProvisioningCommand) cmd); } else if (cmd instanceof GetHostStatsCommand) { @@ -273,6 +277,13 @@ public RebootAnswer execute(RebootCommand cmd) { return externalProvisioner.rebootInstance(guid, extensionName, extensionRelativePath, cmd); } + public GetExternalConsoleAnswer execute(GetExternalConsoleCommand cmd) { + if (isExtensionDisconnected() || isExtensionNotEnabled() || isExtensionPathNotReady()) { + return new GetExternalConsoleAnswer(cmd, logAndGetExtensionNotConnectedOrDisabledError()); + } + return externalProvisioner.getInstanceConsole(guid, extensionName, extensionRelativePath, cmd); + } + public PrepareExternalProvisioningAnswer execute(PrepareExternalProvisioningCommand cmd) { if (isExtensionDisconnected() || isExtensionNotEnabled() || isExtensionPathNotReady()) { return new PrepareExternalProvisioningAnswer(cmd, false, logAndGetExtensionNotConnectedOrDisabledError()); diff --git a/plugins/hypervisors/external/src/test/java/org/apache/cloudstack/hypervisor/external/provisioner/ExternalPathPayloadProvisionerTest.java b/plugins/hypervisors/external/src/test/java/org/apache/cloudstack/hypervisor/external/provisioner/ExternalPathPayloadProvisionerTest.java index 8c63a20fa314..a90c13aeed5c 100644 --- a/plugins/hypervisors/external/src/test/java/org/apache/cloudstack/hypervisor/external/provisioner/ExternalPathPayloadProvisionerTest.java +++ b/plugins/hypervisors/external/src/test/java/org/apache/cloudstack/hypervisor/external/provisioner/ExternalPathPayloadProvisionerTest.java @@ -88,11 +88,9 @@ import com.cloud.utils.PropertiesUtil; import com.cloud.utils.script.Script; import com.cloud.vm.UserVmVO; -import com.cloud.vm.VMInstanceVO; import com.cloud.vm.VirtualMachine; import com.cloud.vm.VirtualMachineProfile; import com.cloud.vm.dao.UserVmDao; -import com.cloud.vm.dao.VMInstanceDao; @RunWith(MockitoJUnitRunner.class) public class ExternalPathPayloadProvisionerTest { @@ -107,9 +105,6 @@ public class ExternalPathPayloadProvisionerTest { @Mock private HostDao hostDao; - @Mock - private VMInstanceDao vmInstanceDao; - @Mock private HypervisorGuruManager hypervisorGuruManager; @@ -489,14 +484,6 @@ public void testRunCustomAction() { when(cmd.getParameters()).thenReturn(new HashMap<>()); when(cmd.getExternalDetails()).thenReturn(new HashMap<>()); when(cmd.getWait()).thenReturn(30); - when(cmd.getVmId()).thenReturn(1L); - - VMInstanceVO vm = mock(VMInstanceVO.class); - when(vmInstanceDao.findById(anyLong())).thenReturn(vm); - - when(hypervisorGuruManager.getGuru(Hypervisor.HypervisorType.External)).thenReturn(hypervisorGuru); - VirtualMachineTO vmTO = mock(VirtualMachineTO.class); - when(hypervisorGuru.implement(any(VirtualMachineProfile.class))).thenReturn(vmTO); doReturn(new Pair<>(true, "success")).when(provisioner) .executeExternalCommand(anyString(), anyString(), anyMap(), anyInt(), anyString(), anyString()); diff --git a/scripts/vm/hypervisor/external/provisioner/provisioner.sh b/scripts/vm/hypervisor/external/provisioner/provisioner.sh index 63d07653c0fc..507001cdf424 100755 --- a/scripts/vm/hypervisor/external/provisioner/provisioner.sh +++ b/scripts/vm/hypervisor/external/provisioner/provisioner.sh @@ -99,6 +99,14 @@ status() { echo '{"status": "success", "power_state": "poweron"}' } +get_console() { + parse_json "$1" || exit 1 + local response + response=$(jq -n \ + '{status: "success", message: "Console retrieved"}') + echo "$response" +} + action=$1 parameters_file="$2" wait_time="$3" @@ -138,6 +146,9 @@ case $action in status) status "$parameters" ;; + GetConsole) + get_console "$parameters" + ;; *) echo '{"error":"Invalid action"}' exit 1 diff --git a/server/src/main/java/com/cloud/consoleproxy/AgentHookBase.java b/server/src/main/java/com/cloud/consoleproxy/AgentHookBase.java index 93cf1e3f689b..a67e56f6ad23 100644 --- a/server/src/main/java/com/cloud/consoleproxy/AgentHookBase.java +++ b/server/src/main/java/com/cloud/consoleproxy/AgentHookBase.java @@ -48,6 +48,7 @@ import com.cloud.host.HostVO; import com.cloud.host.Status; import com.cloud.host.dao.HostDao; +import com.cloud.hypervisor.Hypervisor; import com.cloud.servlet.ConsoleProxyPasswordBasedEncryptor; import com.cloud.servlet.ConsoleProxyServlet; import com.cloud.utils.Ternary; @@ -161,8 +162,12 @@ public AgentControlAnswer onConsoleAccessAuthentication(ConsoleAccessAuthenticat String sid = cmd.getSid(); if (sid == null || !sid.equals(vm.getVncPassword())) { - logger.warn("sid " + sid + " in url does not match stored sid."); - return new ConsoleAccessAuthenticationAnswer(cmd, false); + if (Hypervisor.HypervisorType.External.equals(vm.getHypervisorType())) { + logger.debug("{} is on External hypervisor, skip checking sid", vm.getHypervisorType()); + } else { + logger.warn("sid {} in url does not match stored sid.", sid); + return new ConsoleAccessAuthenticationAnswer(cmd, false); + } } if (cmd.isReauthenticating()) { diff --git a/server/src/main/java/com/cloud/server/ManagementServer.java b/server/src/main/java/com/cloud/server/ManagementServer.java index 611ba9b4200f..d89a9197d17f 100644 --- a/server/src/main/java/com/cloud/server/ManagementServer.java +++ b/server/src/main/java/com/cloud/server/ManagementServer.java @@ -16,7 +16,9 @@ // under the License. package com.cloud.server; +import com.cloud.agent.api.Answer; import com.cloud.host.DetailVO; +import com.cloud.host.Host; import com.cloud.host.HostVO; import com.cloud.storage.GuestOSHypervisorVO; import com.cloud.storage.GuestOSVO; @@ -74,4 +76,6 @@ public interface ManagementServer extends ManagementService, PluggableService { Pair updateSystemVM(VMInstanceVO systemVM, boolean forced); + Answer getExternalConsole(VirtualMachine vm, Host host); + } diff --git a/server/src/main/java/com/cloud/server/ManagementServerImpl.java b/server/src/main/java/com/cloud/server/ManagementServerImpl.java index 3245acfbbf2c..88f9a98eb844 100644 --- a/server/src/main/java/com/cloud/server/ManagementServerImpl.java +++ b/server/src/main/java/com/cloud/server/ManagementServerImpl.java @@ -44,18 +44,6 @@ import javax.inject.Inject; import javax.naming.ConfigurationException; -import com.cloud.cpu.CPU; -import com.cloud.dc.VlanDetailsVO; -import com.cloud.dc.dao.VlanDetailsDao; -import com.cloud.network.dao.NetrisProviderDao; -import com.cloud.network.dao.NsxProviderDao; - -import com.cloud.utils.security.CertificateHelper; -import com.cloud.api.query.dao.ManagementServerJoinDao; -import com.cloud.api.query.vo.ManagementServerJoinVO; -import com.cloud.gpu.VgpuProfileVO; -import com.cloud.gpu.dao.VgpuProfileDao; -import com.cloud.offering.ServiceOffering; import org.apache.cloudstack.acl.ControlledEntity; import org.apache.cloudstack.acl.SecurityChecker; import org.apache.cloudstack.affinity.AffinityGroupProcessor; @@ -695,7 +683,9 @@ import com.cloud.alert.AlertVO; import com.cloud.alert.dao.AlertDao; import com.cloud.api.ApiDBUtils; +import com.cloud.api.query.dao.ManagementServerJoinDao; import com.cloud.api.query.dao.StoragePoolJoinDao; +import com.cloud.api.query.vo.ManagementServerJoinVO; import com.cloud.api.query.vo.StoragePoolJoinVO; import com.cloud.capacity.Capacity; import com.cloud.capacity.CapacityVO; @@ -706,6 +696,7 @@ import com.cloud.configuration.ConfigurationManagerImpl; import com.cloud.consoleproxy.ConsoleProxyManagementState; import com.cloud.consoleproxy.ConsoleProxyManager; +import com.cloud.cpu.CPU; import com.cloud.dc.AccountVlanMapVO; import com.cloud.dc.ClusterVO; import com.cloud.dc.DataCenterVO; @@ -715,6 +706,7 @@ import com.cloud.dc.PodVlanMapVO; import com.cloud.dc.Vlan; import com.cloud.dc.Vlan.VlanType; +import com.cloud.dc.VlanDetailsVO; import com.cloud.dc.VlanVO; import com.cloud.dc.dao.AccountVlanMapDao; import com.cloud.dc.dao.ClusterDao; @@ -723,6 +715,7 @@ import com.cloud.dc.dao.HostPodDao; import com.cloud.dc.dao.PodVlanMapDao; import com.cloud.dc.dao.VlanDao; +import com.cloud.dc.dao.VlanDetailsDao; import com.cloud.deploy.DataCenterDeployment; import com.cloud.deploy.DeploymentPlanner; import com.cloud.deploy.DeploymentPlanner.ExcludeList; @@ -744,6 +737,8 @@ import com.cloud.exception.ResourceUnavailableException; import com.cloud.exception.VirtualMachineMigrationException; import com.cloud.gpu.GPU; +import com.cloud.gpu.VgpuProfileVO; +import com.cloud.gpu.dao.VgpuProfileDao; import com.cloud.ha.HighAvailabilityManager; import com.cloud.host.DetailVO; import com.cloud.host.Host; @@ -770,13 +765,16 @@ import com.cloud.network.dao.IPAddressVO; import com.cloud.network.dao.LoadBalancerDao; import com.cloud.network.dao.LoadBalancerVO; +import com.cloud.network.dao.NetrisProviderDao; import com.cloud.network.dao.NetworkAccountDao; import com.cloud.network.dao.NetworkAccountVO; import com.cloud.network.dao.NetworkDao; import com.cloud.network.dao.NetworkDomainDao; import com.cloud.network.dao.NetworkDomainVO; import com.cloud.network.dao.NetworkVO; +import com.cloud.network.dao.NsxProviderDao; import com.cloud.network.vpc.dao.VpcDao; +import com.cloud.offering.ServiceOffering; import com.cloud.org.Cluster; import com.cloud.org.Grouping.AllocationState; import com.cloud.projects.Project; @@ -850,6 +848,7 @@ import com.cloud.utils.fsm.StateMachine2; import com.cloud.utils.net.MacAddress; import com.cloud.utils.net.NetUtils; +import com.cloud.utils.security.CertificateHelper; import com.cloud.utils.ssh.SSHKeysHelper; import com.cloud.vm.ConsoleProxyVO; import com.cloud.vm.DiskProfile; @@ -5813,4 +5812,8 @@ public boolean removeManagementServer(RemoveManagementServerCmd cmd) { } + @Override + public Answer getExternalConsole(VirtualMachine vm, Host host) { + return extensionsManager.getInstanceConsole(vm, host); + } } diff --git a/server/src/main/java/com/cloud/servlet/ConsoleProxyServlet.java b/server/src/main/java/com/cloud/servlet/ConsoleProxyServlet.java index 2b786a8f1efe..265a975af245 100644 --- a/server/src/main/java/com/cloud/servlet/ConsoleProxyServlet.java +++ b/server/src/main/java/com/cloud/servlet/ConsoleProxyServlet.java @@ -43,6 +43,7 @@ import org.springframework.web.context.support.SpringBeanAutowiringSupport; +import com.cloud.hypervisor.Hypervisor; import com.google.gson.Gson; import com.google.gson.GsonBuilder; @@ -277,15 +278,19 @@ private void handleAuthRequest(HttpServletRequest req, HttpServletResponse resp, String sid = req.getParameter("sid"); if (sid == null || !sid.equals(vm.getVncPassword())) { - if(sid != null) { - sid = sid.replaceAll(SANITIZATION_REGEX, "_"); - LOGGER.warn(String.format("sid [%s] in url does not match stored sid.", sid)); + if (Hypervisor.HypervisorType.External.equals(vm.getHypervisorType())) { + LOGGER.debug("{} is on External hypervisor, skip checking sid", vm.getHypervisorType()); } else { - LOGGER.warn("Null sid in URL."); - } + if (sid != null) { + sid = sid.replaceAll(SANITIZATION_REGEX, "_"); + LOGGER.warn(String.format("sid [%s] in url does not match stored sid.", sid)); + } else { + LOGGER.warn("Null sid in URL."); + } - sendResponse(resp, "failed"); - return; + sendResponse(resp, "failed"); + return; + } } sendResponse(resp, "success"); diff --git a/server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java b/server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java index 2edb6bab001f..5421edc69583 100644 --- a/server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java @@ -22,15 +22,15 @@ import java.util.List; import java.util.Map; import java.util.UUID; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; import javax.crypto.Mac; import javax.crypto.spec.SecretKeySpec; import javax.inject.Inject; import javax.naming.ConfigurationException; -import com.cloud.domain.Domain; -import com.cloud.domain.dao.DomainDao; -import com.cloud.exception.InvalidParameterValueException; import org.apache.cloudstack.api.ResponseGenerator; import org.apache.cloudstack.api.ResponseObject; import org.apache.cloudstack.api.command.user.consoleproxy.ConsoleEndpoint; @@ -38,20 +38,29 @@ import org.apache.cloudstack.api.response.ConsoleSessionResponse; import org.apache.cloudstack.api.response.ListResponse; import org.apache.cloudstack.context.CallContext; +import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.cloudstack.framework.security.keys.KeysManager; +import org.apache.cloudstack.managed.context.ManagedContextRunnable; import org.apache.commons.codec.binary.Base64; import org.apache.commons.lang3.ArrayUtils; import org.apache.commons.lang3.ObjectUtils; import org.apache.commons.lang3.StringUtils; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.joda.time.DateTime; import com.cloud.agent.AgentManager; import com.cloud.agent.api.Answer; +import com.cloud.agent.api.GetExternalConsoleAnswer; import com.cloud.agent.api.GetVmVncTicketAnswer; import com.cloud.agent.api.GetVmVncTicketCommand; import com.cloud.consoleproxy.ConsoleProxyManager; import com.cloud.dc.DataCenter; import com.cloud.dc.dao.DataCenterDao; +import com.cloud.domain.Domain; +import com.cloud.domain.dao.DomainDao; import com.cloud.exception.AgentUnavailableException; +import com.cloud.exception.InvalidParameterValueException; import com.cloud.exception.OperationTimedoutException; import com.cloud.exception.PermissionDeniedException; import com.cloud.host.HostVO; @@ -80,15 +89,6 @@ import com.cloud.vm.dao.VMInstanceDetailsDao; import com.google.gson.Gson; import com.google.gson.GsonBuilder; -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; -import org.apache.cloudstack.framework.config.ConfigKey; -import org.apache.cloudstack.managed.context.ManagedContextRunnable; -import org.joda.time.DateTime; - -import java.util.concurrent.Executors; -import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.TimeUnit; public class ConsoleAccessManagerImpl extends ManagerBase implements ConsoleAccessManager { @@ -293,12 +293,6 @@ public ConsoleEndpoint generateConsoleEndpoint(Long vmId, String extraSecurityTo return new ConsoleEndpoint(false, null, "Cannot find VM with ID " + vmId); } - if (Hypervisor.HypervisorType.External.equals(vm.getHypervisorType())) { - logger.error("Console access for {} cannot be provided it is {} hypervisor instance", vm, - Hypervisor.HypervisorType.External); - return new ConsoleEndpoint(false, null, "Console access to this instance cannot be provided"); - } - if (!checkSessionPermission(vm, account)) { return new ConsoleEndpoint(false, null, "Permission denied"); } @@ -427,6 +421,112 @@ private ConsoleEndpoint generateAccessEndpoint(Long vmId, String sessionUuid, St return consoleEndpoint; } + private static class Result { + String host; + int port; + String sid; + String locale; + String tunnelUrl = null; + String tunnelSession = null; + boolean usingRDP; + + Result(String host, int port, String sid, String locale) { + this.host = host; + this.port = port; + this.sid = sid; + this.locale = locale; + } + + public String getHost() { + return host; + } + + public int getPort() { + return port; + } + + public String getSid() { + return sid; + } + + public String getLocale() { + return locale; + } + + public String getTunnelUrl() { + return tunnelUrl; + } + + public void setTunnelUrl(String tunnelUrl) { + this.tunnelUrl = tunnelUrl; + } + + public String getTunnelSession() { + return tunnelSession; + } + + public void setTunnelSession(String tunnelSession) { + this.tunnelSession = tunnelSession; + } + + public boolean isUsingRDP() { + return usingRDP; + } + + public void setUsingRDP(boolean usingRDP) { + this.usingRDP = usingRDP; + } + } + + protected Result getConsoleDetails(VirtualMachine vm, HostVO host, Pair portInfo) { + String hostInfo = null; + String locale = null; + VMInstanceDetailVO details = vmInstanceDetailsDao.findDetail(vm.getId(), VmDetailConstants.KEYBOARD); + if (details != null) { + locale = details.getValue(); + } + if (Hypervisor.HypervisorType.External.equals(host.getHypervisorType())) { + Answer answer = managementServer.getExternalConsole(vm, host); + if (answer == null) { + logger.error("Unable to get console access details for external {} on {}: answer is null.", vm, host); + return null; + } + if (!answer.getResult()) { + logger.error("Unable to get console access details for external {} on {}: answer result is false. Reason: {}", vm, host, answer.getDetails()); + return null; + } + if (!(answer instanceof GetExternalConsoleAnswer)) { + logger.error("Unable to get console access details for external {} on {}: answer is not of type GetExternalConsoleAnswer.", vm, host); + return null; + } + GetExternalConsoleAnswer getExternalConsoleAnswer = (GetExternalConsoleAnswer) answer; + return new Result(getExternalConsoleAnswer.getHost(), getExternalConsoleAnswer.getPort(), getExternalConsoleAnswer.getPassword(), locale); + } + + int port = -1; + if (portInfo == null) { + portInfo = managementServer.getVncPort(vm); + hostInfo = portInfo.first(); + port = portInfo.second(); + } + logger.debug("Host and port info :[{}, {}]", hostInfo, port); + Ternary parsedHostInfo = parseHostInfo(portInfo.first()); + boolean usingRDP = false; + if (portInfo.second() == -9) { + //for hyperv + usingRDP = true; + port = Integer.parseInt(managementServer.findDetail(host.getId(), "rdp.server.port").getValue()); + } else { + port = portInfo.second(); + } + String sid = vm.getVncPassword(); + Result result = new Result(parsedHostInfo.first(), port, sid, locale); + result.setTunnelUrl(parsedHostInfo.second()); + result.setTunnelSession(parsedHostInfo.third()); + result.setUsingRDP(usingRDP); + return result; + } + private ConsoleEndpoint composeConsoleAccessEndpoint(String rootUrl, VirtualMachine vm, HostVO hostVo, String addr, String sessionUuid, String extraSecurityToken) { String host = hostVo.getPrivateIpAddress(); @@ -444,41 +544,27 @@ private ConsoleEndpoint composeConsoleAccessEndpoint(String rootUrl, VirtualMach "no VNC Address/Port was available. Falling back to default one from MS."); } } - - if (portInfo == null) { - portInfo = managementServer.getVncPort(vm); - } - - if (logger.isDebugEnabled()) - logger.debug("Port info " + portInfo.first()); - - Ternary parsedHostInfo = parseHostInfo(portInfo.first()); - - int port = -1; - if (portInfo.second() == -9) { - //for hyperv - port = Integer.parseInt(managementServer.findDetail(hostVo.getId(), "rdp.server.port").getValue()); - } else { - port = portInfo.second(); + Result result = getConsoleDetails(vm, hostVo, portInfo); + if (result == null) { + return new ConsoleEndpoint(false, null, "Console access to this instance cannot be provided"); } - String sid = vm.getVncPassword(); - VMInstanceDetailVO details = vmInstanceDetailsDao.findDetail(vm.getId(), VmDetailConstants.KEYBOARD); - String tag = vm.getUuid(); String displayName = vm.getHostName(); if (vm instanceof UserVm) { displayName = ((UserVm) vm).getDisplayName(); } - String ticket = genAccessTicket(parsedHostInfo.first(), String.valueOf(port), sid, tag, sessionUuid); + String ticket = genAccessTicket(result.getHost(), String.valueOf(result.getPort()), result.getSid(), tag, sessionUuid); ConsoleProxyPasswordBasedEncryptor encryptor = new ConsoleProxyPasswordBasedEncryptor(getEncryptorPassword()); - ConsoleProxyClientParam param = generateConsoleProxyClientParam(parsedHostInfo, port, sid, tag, ticket, - sessionUuid, addr, extraSecurityToken, vm, hostVo, details, portInfo, host, displayName); + ConsoleProxyClientParam param = generateConsoleProxyClientParam( + new Ternary<>(result.getHost(), result.getTunnelUrl(), result.getTunnelSession()), result.getPort(), + result.getSid(), tag, ticket, sessionUuid, addr, extraSecurityToken, vm, hostVo, result.getLocale(), result.isUsingRDP(), + host, displayName); String token = encryptor.encryptObject(ConsoleProxyClientParam.class, param); int vncPort = consoleProxyManager.getVncPort(); - String url = generateConsoleAccessUrl(rootUrl, param, token, vncPort, vm, hostVo, details); + String url = generateConsoleAccessUrl(rootUrl, param, token, vncPort, vm, hostVo, result.getLocale()); logger.debug("Adding allowed session: " + sessionUuid); persistConsoleSession(sessionUuid, vm.getId(), hostVo.getId(), addr); @@ -510,22 +596,22 @@ protected void persistConsoleSession(String sessionUuid, long instanceId, long h } private String generateConsoleAccessUrl(String rootUrl, ConsoleProxyClientParam param, String token, int vncPort, - VirtualMachine vm, HostVO hostVo, VMInstanceDetailVO details) { + VirtualMachine vm, HostVO hostVo, String locale) { StringBuilder sb = new StringBuilder(rootUrl); if (param.getHypervHost() != null || !ConsoleProxyManager.NoVncConsoleDefault.value()) { - sb.append("/ajax?token=" + token); + sb.append("/ajax?token=").append(token); } else { sb.append("/resource/noVNC/vnc.html") .append("?autoconnect=true&show_dot=true") - .append("&port=" + vncPort) - .append("&token=" + token); - if (requiresVncOverWebSocketConnection(vm, hostVo) && details != null && details.getValue() != null) { - sb.append("&language=" + details.getValue()); + .append("&port=").append(vncPort) + .append("&token=").append(token); + if (requiresVncOverWebSocketConnection(vm, hostVo) && StringUtils.isNotBlank(locale)) { + sb.append("&language=").append(locale); } } if (StringUtils.isNotBlank(param.getExtraSecurityToken())) { - sb.append("&extra=" + param.getExtraSecurityToken()); + sb.append("&extra=").append(param.getExtraSecurityToken()); } // for console access, we need guest OS type to help implement keyboard @@ -535,7 +621,7 @@ private String generateConsoleAccessUrl(String rootUrl, ConsoleProxyClientParam sb.append("&guest=windows"); if (logger.isDebugEnabled()) { - logger.debug("Compose console url: " + sb); + logger.debug("Compose console url: {}", sb); } return sb.toString().startsWith("https") ? sb.toString() : "http:" + sb; } @@ -544,8 +630,8 @@ private ConsoleProxyClientParam generateConsoleProxyClientParam(Ternary portInfo, String host, + HostVO hostVo, String locale, + boolean usingRDP, String host, String displayName) { ConsoleProxyClientParam param = new ConsoleProxyClientParam(); param.setClientHostAddress(parsedHostInfo.first()); @@ -566,12 +652,12 @@ private ConsoleProxyClientParam generateConsoleProxyClientParam(Ternary - + @@ -29,7 +29,7 @@ icon="code" /> - + From c5a727e295e5f95108daaadacc30d73964da21dd Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Wed, 10 Sep 2025 14:15:25 +0530 Subject: [PATCH 02/14] match response Signed-off-by: Abhishek Kumar --- scripts/vm/hypervisor/external/provisioner/provisioner.sh | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/scripts/vm/hypervisor/external/provisioner/provisioner.sh b/scripts/vm/hypervisor/external/provisioner/provisioner.sh index 507001cdf424..950f3aee74bf 100755 --- a/scripts/vm/hypervisor/external/provisioner/provisioner.sh +++ b/scripts/vm/hypervisor/external/provisioner/provisioner.sh @@ -102,9 +102,9 @@ status() { get_console() { parse_json "$1" || exit 1 local response - response=$(jq -n \ - '{status: "success", message: "Console retrieved"}') - echo "$response" + jq -n --arg msg "Operation not supported" \ + '{status:"error", message:$msg, code:"OPERATION_NOT_SUPPORTED"}' + return 1 } action=$1 @@ -146,7 +146,7 @@ case $action in status) status "$parameters" ;; - GetConsole) + getconsole) get_console "$parameters" ;; *) From 85deee8a076bc228c6642d54f0a312e8adc65b89 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Wed, 10 Sep 2025 14:33:54 +0530 Subject: [PATCH 03/14] tests and changes Signed-off-by: Abhishek Kumar --- .../agent/api/GetExternalConsoleAnswer.java | 1 + .../manager/ExtensionsManagerImplTest.java | 74 ++- .../ExternalPathPayloadProvisioner.java | 18 +- .../ExternalPathPayloadProvisionerTest.java | 213 ++++++++ .../com/cloud/server/ManagementServer.java | 2 +- .../cloud/server/ManagementServerImpl.java | 2 +- .../ConsoleAccessManagerImpl.java | 325 +++++++------ .../server/ManagementServerImplTest.java | 14 + .../ConsoleAccessManagerImplTest.java | 458 +++++++++++++++++- 9 files changed, 934 insertions(+), 173 deletions(-) diff --git a/core/src/main/java/com/cloud/agent/api/GetExternalConsoleAnswer.java b/core/src/main/java/com/cloud/agent/api/GetExternalConsoleAnswer.java index f2f6f2aa9ce1..ead4709bdcff 100644 --- a/core/src/main/java/com/cloud/agent/api/GetExternalConsoleAnswer.java +++ b/core/src/main/java/com/cloud/agent/api/GetExternalConsoleAnswer.java @@ -21,6 +21,7 @@ public class GetExternalConsoleAnswer extends Answer { private String host; private int port; + @LogLevel(LogLevel.Log4jLevel.Off) private String password; private String protocol; diff --git a/framework/extensions/src/test/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImplTest.java b/framework/extensions/src/test/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImplTest.java index fcceb16523e8..37297654a533 100644 --- a/framework/extensions/src/test/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImplTest.java +++ b/framework/extensions/src/test/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImplTest.java @@ -92,7 +92,6 @@ import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.MockedStatic; -import org.mockito.Mockito; import org.mockito.MockitoAnnotations; import org.mockito.Spy; import org.mockito.junit.MockitoJUnitRunner; @@ -102,6 +101,7 @@ import com.cloud.agent.api.Answer; import com.cloud.agent.api.Command; import com.cloud.agent.api.RunCustomActionAnswer; +import com.cloud.agent.api.to.VirtualMachineTO; import com.cloud.alert.AlertManager; import com.cloud.cluster.ClusterManager; import com.cloud.cluster.ManagementServerHostVO; @@ -111,6 +111,7 @@ import com.cloud.exception.AgentUnavailableException; import com.cloud.exception.InvalidParameterValueException; import com.cloud.exception.OperationTimedoutException; +import com.cloud.host.Host; import com.cloud.host.dao.HostDao; import com.cloud.host.dao.HostDetailsDao; import com.cloud.hypervisor.ExternalProvisioner; @@ -1281,7 +1282,7 @@ public void deleteCustomAction_RemoveFails() { } private void mockCallerRole(RoleType roleType) { - CallContext callContextMock = Mockito.mock(CallContext.class); + CallContext callContextMock = mock(CallContext.class); when(CallContext.current()).thenReturn(callContextMock); Account accountMock = mock(Account.class); when(accountMock.getRoleId()).thenReturn(1L); @@ -1882,4 +1883,73 @@ public void getExtensionForCluster_WhenNoMappingExists_ReturnsNull() { Extension result = extensionsManager.getExtensionForCluster(clusterId); assertNull(result); } + + @Test + public void getInstanceConsole_whenValid() { + Extension extension = mock(Extension.class); + when(extension.getType()).thenReturn(Extension.Type.Orchestrator); + when(extension.getState()).thenReturn(Extension.State.Enabled); + when(extensionsManager.getExtensionForCluster(anyLong())).thenReturn(extension); + VirtualMachine vm = mock(VirtualMachine.class); + Host host = mock(Host.class); + when(host.getClusterId()).thenReturn(1L); + Answer expectedAnswer = mock(Answer.class); + when(virtualMachineManager.toVmTO(any())).thenReturn(mock(VirtualMachineTO.class)); + when(agentMgr.easySend(anyLong(), any())).thenReturn(expectedAnswer); + Answer result = extensionsManager.getInstanceConsole(vm, host); + assertNotNull(result); + assertEquals(expectedAnswer, result); + } + + @Test + public void getInstanceConsole_whenNullExtension() { + when(extensionsManager.getExtensionForCluster(anyLong())).thenReturn(null); + VirtualMachine vm = mock(VirtualMachine.class); + Host host = mock(Host.class); + when(host.getClusterId()).thenReturn(1L); + Answer result = extensionsManager.getInstanceConsole(vm, host); + assertNotNull(result); + assertFalse(result.getResult()); + } + + @Test + public void getInstanceConsole_whenNullExtensionNotOrchestrator() { + Extension extension = mock(Extension.class); + when(extensionsManager.getExtensionForCluster(anyLong())).thenReturn(extension); + VirtualMachine vm = mock(VirtualMachine.class); + Host host = mock(Host.class); + when(host.getClusterId()).thenReturn(1L); + Answer result = extensionsManager.getInstanceConsole(vm, host); + assertNotNull(result); + assertFalse(result.getResult()); + } + + @Test + public void getInstanceConsole_whenNullExtensionNotEnabled() { + Extension extension = mock(Extension.class); + when(extension.getType()).thenReturn(Extension.Type.Orchestrator); + when(extension.getState()).thenReturn(Extension.State.Disabled); + when(extensionsManager.getExtensionForCluster(anyLong())).thenReturn(extension); + VirtualMachine vm = mock(VirtualMachine.class); + Host host = mock(Host.class); + when(host.getClusterId()).thenReturn(1L); + Answer result = extensionsManager.getInstanceConsole(vm, host); + assertNotNull(result); + assertFalse(result.getResult()); + } + + @Test + public void getInstanceConsole_whenAgentManagerFails() { + Extension extension = mock(Extension.class); + when(extension.getType()).thenReturn(Extension.Type.Orchestrator); + when(extension.getState()).thenReturn(Extension.State.Enabled); + when(extensionsManager.getExtensionForCluster(anyLong())).thenReturn(extension); + VirtualMachine vm = mock(VirtualMachine.class); + Host host = mock(Host.class); + when(host.getClusterId()).thenReturn(1L); + when(virtualMachineManager.toVmTO(any())).thenReturn(mock(VirtualMachineTO.class)); + when(agentMgr.easySend(anyLong(), any())).thenReturn(null); + Answer result = extensionsManager.getInstanceConsole(vm, host); + assertNull(result); + } } diff --git a/plugins/hypervisors/external/src/main/java/org/apache/cloudstack/hypervisor/external/provisioner/ExternalPathPayloadProvisioner.java b/plugins/hypervisors/external/src/main/java/org/apache/cloudstack/hypervisor/external/provisioner/ExternalPathPayloadProvisioner.java index d8f2709b8b68..c1632030fa76 100644 --- a/plugins/hypervisors/external/src/main/java/org/apache/cloudstack/hypervisor/external/provisioner/ExternalPathPayloadProvisioner.java +++ b/plugins/hypervisors/external/src/main/java/org/apache/cloudstack/hypervisor/external/provisioner/ExternalPathPayloadProvisioner.java @@ -219,6 +219,13 @@ protected VirtualMachineTO getVirtualMachineTO(VirtualMachine vm) { return hvGuru.implement(profile); } + protected String getSanitizedJsonStringForLog(String json) { + if (StringUtils.isBlank(json)) { + return json; + } + return json.replaceAll("(\"password\"\\s*:\\s*\")([^\"]*)(\")", "$1****$3"); + } + private String getServerProperty(String name) { Properties props = propertiesRef.get(); if (props == null) { @@ -485,20 +492,23 @@ public GetExternalConsoleAnswer getInstanceConsole(String hostGuid, String exten if (!result.first()) { return new GetExternalConsoleAnswer(cmd, output); } - logger.debug("Received console details from the external system: {}", output); + logger.debug("Received console details from the external system: {}", + getSanitizedJsonStringForLog(output)); try { JsonObject jsonObj = JsonParser.parseString(output).getAsJsonObject(); JsonObject consoleObj = jsonObj.has("console") ? jsonObj.getAsJsonObject("console") : null; if (consoleObj == null) { - logger.error("Missing console object in external console output: {}", output); + logger.error("Missing console object in external console output: {}", + getSanitizedJsonStringForLog(output)); return new GetExternalConsoleAnswer(cmd, "Missing console object in output"); } String host = consoleObj.has("host") ? consoleObj.get("host").getAsString() : null; Integer port = consoleObj.has("port") ? Integer.valueOf(consoleObj.get("port").getAsString()) : null; String password = consoleObj.has("password") ? consoleObj.get("password").getAsString() : null; String protocol = consoleObj.has("protocol") ? consoleObj.get("protocol").getAsString() : null; - if (ObjectUtils.anyNull(host, port, password)) { - logger.error("Missing required fields in external console output: {}", output); + if (ObjectUtils.anyNull(host, port)) { + logger.error("Missing required fields in external console output: {}", + getSanitizedJsonStringForLog(output)); return new GetExternalConsoleAnswer(cmd, "Missing required fields in output"); } return new GetExternalConsoleAnswer(cmd, host, port, password, protocol); diff --git a/plugins/hypervisors/external/src/test/java/org/apache/cloudstack/hypervisor/external/provisioner/ExternalPathPayloadProvisionerTest.java b/plugins/hypervisors/external/src/test/java/org/apache/cloudstack/hypervisor/external/provisioner/ExternalPathPayloadProvisionerTest.java index a90c13aeed5c..665eaa5bfb0f 100644 --- a/plugins/hypervisors/external/src/test/java/org/apache/cloudstack/hypervisor/external/provisioner/ExternalPathPayloadProvisionerTest.java +++ b/plugins/hypervisors/external/src/test/java/org/apache/cloudstack/hypervisor/external/provisioner/ExternalPathPayloadProvisionerTest.java @@ -65,6 +65,8 @@ import org.mockito.junit.MockitoJUnitRunner; import org.springframework.test.util.ReflectionTestUtils; +import com.cloud.agent.api.GetExternalConsoleAnswer; +import com.cloud.agent.api.GetExternalConsoleCommand; import com.cloud.agent.api.HostVmStateReportEntry; import com.cloud.agent.api.PrepareExternalProvisioningAnswer; import com.cloud.agent.api.PrepareExternalProvisioningCommand; @@ -734,4 +736,215 @@ public void parsePowerStateFromResponseReturnsPowerStateForPlainTextResponse() { VirtualMachine.PowerState result = provisioner.parsePowerStateFromResponse(vm, response); assertEquals(VirtualMachine.PowerState.PowerOn, result); } + + @Test + public void getVirtualMachineTOReturnsNullWhenVmIsNull() { + VirtualMachineTO result = provisioner.getVirtualMachineTO(null); + assertNull(result); + } + + @Test + public void getVirtualMachineTOReturnsValidTOWhenVmIsNotNull() { + VirtualMachine vm = mock(VirtualMachine.class); + VirtualMachineTO vmTO = mock(VirtualMachineTO.class); + when(hypervisorGuruManager.getGuru(Hypervisor.HypervisorType.External)).thenReturn(hypervisorGuru); + when(hypervisorGuru.implement(any(VirtualMachineProfile.class))).thenReturn(vmTO); + VirtualMachineTO result = provisioner.getVirtualMachineTO(vm); + assertNotNull(result); + assertEquals(vmTO, result); + Mockito.verify(hypervisorGuruManager).getGuru(Hypervisor.HypervisorType.External); + Mockito.verify(hypervisorGuru).implement(any(VirtualMachineProfile.class)); + } + + @Test + public void getInstanceConsoleReturnsAnswerWhenConsoleDetailsAreValid() { + GetExternalConsoleCommand cmd = mock(GetExternalConsoleCommand.class); + VirtualMachineTO vmTO = mock(VirtualMachineTO.class); + when(cmd.getVirtualMachine()).thenReturn(vmTO); + when(vmTO.getUuid()).thenReturn("test-uuid"); + + Map accessDetails = new HashMap<>(); + when(provisioner.loadAccessDetails(any(), eq(vmTO))).thenReturn(accessDetails); + + String validOutput = "{\"console\":{\"host\":\"127.0.0.1\",\"port\":5900,\"password\":\"pass\",\"protocol\":\"vnc\"}}"; + doReturn(new Pair<>(true, validOutput)).when(provisioner) + .getInstanceConsoleOnExternalSystem(anyString(), anyString(), anyString(), anyMap(), anyInt()); + + GetExternalConsoleAnswer result = provisioner.getInstanceConsole("host-guid", "test-extension", "test-extension.sh", cmd); + + assertNotNull(result); + assertEquals("127.0.0.1", result.getHost()); + assertEquals(5900, result.getPort()); + assertEquals("pass", result.getPassword()); + assertEquals("vnc", result.getProtocol()); + } + + @Test + public void getInstanceConsoleReturnsErrorWhenExtensionNotConfigured() { + GetExternalConsoleCommand cmd = mock(GetExternalConsoleCommand.class); + when(provisioner.getExtensionCheckedPath(anyString(), anyString())).thenReturn(null); + + GetExternalConsoleAnswer result = provisioner.getInstanceConsole("host-guid", + "test-extension", "test-extension.sh", cmd); + + assertNotNull(result); + assertEquals("Extension not configured", result.getDetails()); + } + + @Test + public void getInstanceConsoleReturnsErrorWhenExternalSystemFails() { + GetExternalConsoleCommand cmd = mock(GetExternalConsoleCommand.class); + VirtualMachineTO vmTO = mock(VirtualMachineTO.class); + when(cmd.getVirtualMachine()).thenReturn(vmTO); + when(vmTO.getUuid()).thenReturn("test-uuid"); + + doReturn(new Pair<>(false, "External system error")).when(provisioner) + .getInstanceConsoleOnExternalSystem(anyString(), anyString(), anyString(), anyMap(), anyInt()); + + GetExternalConsoleAnswer result = provisioner.getInstanceConsole("host-guid", "test-extension", "test-extension.sh", cmd); + + assertNotNull(result); + assertEquals("External system error", result.getDetails()); + } + + @Test + public void getInstanceConsoleReturnsErrorWhenConsoleObjectIsMissing() { + GetExternalConsoleCommand cmd = mock(GetExternalConsoleCommand.class); + VirtualMachineTO vmTO = mock(VirtualMachineTO.class); + when(cmd.getVirtualMachine()).thenReturn(vmTO); + when(vmTO.getUuid()).thenReturn("test-uuid"); + + String invalidOutput = "{\"invalid_key\":\"value\"}"; + doReturn(new Pair<>(true, invalidOutput)).when(provisioner) + .getInstanceConsoleOnExternalSystem(anyString(), anyString(), anyString(), anyMap(), anyInt()); + + GetExternalConsoleAnswer result = provisioner.getInstanceConsole("host-guid", "test-extension", "test-extension.sh", cmd); + + assertNotNull(result); + assertEquals("Missing console object in output", result.getDetails()); + } + + @Test + public void getInstanceConsoleReturnsErrorWhenRequiredFieldsAreMissing() { + GetExternalConsoleCommand cmd = mock(GetExternalConsoleCommand.class); + VirtualMachineTO vmTO = mock(VirtualMachineTO.class); + when(cmd.getVirtualMachine()).thenReturn(vmTO); + when(vmTO.getUuid()).thenReturn("test-uuid"); + + String incompleteOutput = "{\"console\":{\"host\":\"127.0.0.1\"}}"; + doReturn(new Pair<>(true, incompleteOutput)).when(provisioner) + .getInstanceConsoleOnExternalSystem(anyString(), anyString(), anyString(), anyMap(), anyInt()); + + GetExternalConsoleAnswer result = provisioner.getInstanceConsole("host-guid", "test-extension", "test-extension.sh", cmd); + + assertNotNull(result); + assertEquals("Missing required fields in output", result.getDetails()); + } + + @Test + public void getInstanceConsoleReturnsErrorWhenOutputParsingFails() { + GetExternalConsoleCommand cmd = mock(GetExternalConsoleCommand.class); + VirtualMachineTO vmTO = mock(VirtualMachineTO.class); + when(cmd.getVirtualMachine()).thenReturn(vmTO); + when(vmTO.getUuid()).thenReturn("test-uuid"); + + String malformedOutput = "{console:invalid}"; + doReturn(new Pair<>(true, malformedOutput)).when(provisioner) + .getInstanceConsoleOnExternalSystem(anyString(), anyString(), anyString(), anyMap(), anyInt()); + + GetExternalConsoleAnswer result = provisioner.getInstanceConsole("host-guid", "test-extension", "test-extension.sh", cmd); + + assertNotNull(result); + assertEquals("Failed to parse output", result.getDetails()); + } + + @Test + public void getInstanceConsoleOnExternalSystemReturnsSuccessWhenCommandExecutesSuccessfully() { + String extensionName = "test-extension"; + String filename = "test-script.sh"; + String vmUUID = "test-vm-uuid"; + Map accessDetails = new HashMap<>(); + int wait = 30; + + doReturn(new Pair<>(true, "Console details")).when(provisioner) + .executeExternalCommand(eq(extensionName), eq("getconsole"), eq(accessDetails), eq(wait), anyString(), eq(filename)); + + Pair result = provisioner.getInstanceConsoleOnExternalSystem(extensionName, filename, vmUUID, accessDetails, wait); + + assertTrue(result.first()); + assertEquals("Console details", result.second()); + } + + @Test + public void getInstanceConsoleOnExternalSystemReturnsFailureWhenCommandFails() { + String extensionName = "test-extension"; + String filename = "test-script.sh"; + String vmUUID = "test-vm-uuid"; + Map accessDetails = new HashMap<>(); + int wait = 30; + + doReturn(new Pair<>(false, "Failed to get console")).when(provisioner) + .executeExternalCommand(eq(extensionName), eq("getconsole"), eq(accessDetails), eq(wait), anyString(), eq(filename)); + + Pair result = provisioner.getInstanceConsoleOnExternalSystem(extensionName, filename, vmUUID, accessDetails, wait); + + assertFalse(result.first()); + assertEquals("Failed to get console", result.second()); + } + + @Test + public void getInstanceConsoleOnExternalSystemHandlesNullResponseGracefully() { + String extensionName = "test-extension"; + String filename = "test-script.sh"; + String vmUUID = "test-vm-uuid"; + Map accessDetails = new HashMap<>(); + int wait = 30; + + doReturn(null).when(provisioner) + .executeExternalCommand(eq(extensionName), eq("getconsole"), eq(accessDetails), eq(wait), anyString(), eq(filename)); + + Pair result = provisioner.getInstanceConsoleOnExternalSystem(extensionName, filename, vmUUID, accessDetails, wait); + + assertNull(result); + } + + @Test + public void getSanitizedJsonStringForLogReturnsNullWhenInputIsNull() { + String result = provisioner.getSanitizedJsonStringForLog(null); + assertNull(result); + } + + @Test + public void getSanitizedJsonStringForLogReturnsEmptyWhenInputIsEmpty() { + String result = provisioner.getSanitizedJsonStringForLog(""); + assertEquals("", result); + } + + @Test + public void getSanitizedJsonStringForLogReturnsSameStringWhenNoPasswordField() { + String json = "{\"key\":\"value\"}"; + String result = provisioner.getSanitizedJsonStringForLog(json); + assertEquals(json, result); + } + + @Test + public void getSanitizedJsonStringForLogMasksPasswordField() { + String json = "{\"password\":\"secret\"}"; + String result = provisioner.getSanitizedJsonStringForLog(json); + assertEquals("{\"password\":\"****\"}", result); + } + + @Test + public void getSanitizedJsonStringForLogHandlesMultiplePasswordFields() { + String json = "{\"password\":\"secret\",\"nested\":{\"password\":\"anotherSecret\"}}"; + String result = provisioner.getSanitizedJsonStringForLog(json); + assertEquals("{\"password\":\"****\",\"nested\":{\"password\":\"****\"}}", result); + } + + @Test + public void getSanitizedJsonStringForLogHandlesMalformedJsonGracefully() { + String json = "{password:\"secret\""; + String result = provisioner.getSanitizedJsonStringForLog(json); + assertEquals("{password:\"secret\"", result); + } } diff --git a/server/src/main/java/com/cloud/server/ManagementServer.java b/server/src/main/java/com/cloud/server/ManagementServer.java index d89a9197d17f..3932006c2924 100644 --- a/server/src/main/java/com/cloud/server/ManagementServer.java +++ b/server/src/main/java/com/cloud/server/ManagementServer.java @@ -76,6 +76,6 @@ public interface ManagementServer extends ManagementService, PluggableService { Pair updateSystemVM(VMInstanceVO systemVM, boolean forced); - Answer getExternalConsole(VirtualMachine vm, Host host); + Answer getExternalVmConsole(VirtualMachine vm, Host host); } diff --git a/server/src/main/java/com/cloud/server/ManagementServerImpl.java b/server/src/main/java/com/cloud/server/ManagementServerImpl.java index 88f9a98eb844..43f6a8a5b87e 100644 --- a/server/src/main/java/com/cloud/server/ManagementServerImpl.java +++ b/server/src/main/java/com/cloud/server/ManagementServerImpl.java @@ -5813,7 +5813,7 @@ public boolean removeManagementServer(RemoveManagementServerCmd cmd) { } @Override - public Answer getExternalConsole(VirtualMachine vm, Host host) { + public Answer getExternalVmConsole(VirtualMachine vm, Host host) { return extensionsManager.getInstanceConsole(vm, host); } } diff --git a/server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java b/server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java index 5421edc69583..9cf9cf42afc7 100644 --- a/server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java @@ -81,7 +81,6 @@ import com.cloud.utils.db.GlobalLock; import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.vm.ConsoleSessionVO; -import com.cloud.vm.VMInstanceDetailVO; import com.cloud.vm.VirtualMachine; import com.cloud.vm.VirtualMachineManager; import com.cloud.vm.VmDetailConstants; @@ -128,6 +127,11 @@ public class ConsoleAccessManagerImpl extends ManagerBase implements ConsoleAcce VirtualMachine.State.Stopped, VirtualMachine.State.Restoring, VirtualMachine.State.Error, VirtualMachine.State.Destroyed ); + protected static final List MAINTENANCE_RESOURCE_STATES = new ArrayList<>(Arrays.asList( + ResourceState.ErrorInMaintenance, ResourceState.ErrorInPrepareForMaintenance + )); + protected static final String WEB_SOCKET_PATH= "websockify"; + @Override public boolean configure(String name, Map params) throws ConfigurationException { ConsoleAccessManagerImpl.secretKeysManager = keysManager; @@ -421,159 +425,109 @@ private ConsoleEndpoint generateAccessEndpoint(Long vmId, String sessionUuid, St return consoleEndpoint; } - private static class Result { - String host; - int port; - String sid; - String locale; - String tunnelUrl = null; - String tunnelSession = null; - boolean usingRDP; - - Result(String host, int port, String sid, String locale) { - this.host = host; - this.port = port; - this.sid = sid; - this.locale = locale; - } - - public String getHost() { - return host; - } - - public int getPort() { - return port; - } - - public String getSid() { - return sid; - } - - public String getLocale() { - return locale; + protected ConsoleConnectionDetails getConsoleConnectionDetailsFoxExternalVm(ConsoleConnectionDetails details, + VirtualMachine vm, HostVO host) { + Answer answer = managementServer.getExternalVmConsole(vm, host); + if (answer == null) { + logger.error("Unable to get console access details for external {} on {}: answer is null.", vm, host); + return null; } - - public String getTunnelUrl() { - return tunnelUrl; + if (!answer.getResult()) { + logger.error("Unable to get console access details for external {} on {}: answer result is false. Reason: {}", vm, host, answer.getDetails()); + return null; } - - public void setTunnelUrl(String tunnelUrl) { - this.tunnelUrl = tunnelUrl; + if (!(answer instanceof GetExternalConsoleAnswer)) { + logger.error("Unable to get console access details for external {} on {}: answer is not of type GetExternalConsoleAnswer.", vm, host); + return null; } - - public String getTunnelSession() { - return tunnelSession; + GetExternalConsoleAnswer getExternalConsoleAnswer = (GetExternalConsoleAnswer) answer; + details.setHost(getExternalConsoleAnswer.getHost()); + details.setPort(getExternalConsoleAnswer.getPort()); + if (StringUtils.isNotBlank(getExternalConsoleAnswer.getPassword())) { + details.setSid(getExternalConsoleAnswer.getPassword()); } + return details; + } - public void setTunnelSession(String tunnelSession) { - this.tunnelSession = tunnelSession; + protected Pair getHostAndPortForKVMMaintenanceHostIfNeeded(HostVO host, + Map vmDetails) { + if (!Hypervisor.HypervisorType.KVM.equals(host.getHypervisorType())) { + return null; } - - public boolean isUsingRDP() { - return usingRDP; + if(!MAINTENANCE_RESOURCE_STATES.contains(host.getResourceState())) { + return null; } - - public void setUsingRDP(boolean usingRDP) { - this.usingRDP = usingRDP; + String address = vmDetails.get(VmDetailConstants.KVM_VNC_ADDRESS); + String port = vmDetails.get(VmDetailConstants.KVM_VNC_PORT); + if (ObjectUtils.allNotNull(address, port)) { + return new Pair<>(address, Integer.valueOf(port)); } + logger.warn("KVM Host in ErrorInMaintenance/ErrorInPrepareForMaintenance but " + + "no VNC Address/Port was available. Falling back to default one from MS."); + return null; } - protected Result getConsoleDetails(VirtualMachine vm, HostVO host, Pair portInfo) { - String hostInfo = null; + protected ConsoleConnectionDetails getConsoleConnectionDetails(VirtualMachine vm, HostVO host) { String locale = null; - VMInstanceDetailVO details = vmInstanceDetailsDao.findDetail(vm.getId(), VmDetailConstants.KEYBOARD); - if (details != null) { - locale = details.getValue(); - } - if (Hypervisor.HypervisorType.External.equals(host.getHypervisorType())) { - Answer answer = managementServer.getExternalConsole(vm, host); - if (answer == null) { - logger.error("Unable to get console access details for external {} on {}: answer is null.", vm, host); - return null; - } - if (!answer.getResult()) { - logger.error("Unable to get console access details for external {} on {}: answer result is false. Reason: {}", vm, host, answer.getDetails()); - return null; - } - if (!(answer instanceof GetExternalConsoleAnswer)) { - logger.error("Unable to get console access details for external {} on {}: answer is not of type GetExternalConsoleAnswer.", vm, host); - return null; - } - GetExternalConsoleAnswer getExternalConsoleAnswer = (GetExternalConsoleAnswer) answer; - return new Result(getExternalConsoleAnswer.getHost(), getExternalConsoleAnswer.getPort(), getExternalConsoleAnswer.getPassword(), locale); - } - - int port = -1; - if (portInfo == null) { - portInfo = managementServer.getVncPort(vm); - hostInfo = portInfo.first(); - port = portInfo.second(); - } - logger.debug("Host and port info :[{}, {}]", hostInfo, port); - Ternary parsedHostInfo = parseHostInfo(portInfo.first()); - boolean usingRDP = false; - if (portInfo.second() == -9) { - //for hyperv - usingRDP = true; - port = Integer.parseInt(managementServer.findDetail(host.getId(), "rdp.server.port").getValue()); - } else { - port = portInfo.second(); - } - String sid = vm.getVncPassword(); - Result result = new Result(parsedHostInfo.first(), port, sid, locale); - result.setTunnelUrl(parsedHostInfo.second()); - result.setTunnelSession(parsedHostInfo.third()); - result.setUsingRDP(usingRDP); - return result; - } - - private ConsoleEndpoint composeConsoleAccessEndpoint(String rootUrl, VirtualMachine vm, HostVO hostVo, String addr, - String sessionUuid, String extraSecurityToken) { - String host = hostVo.getPrivateIpAddress(); - - Pair portInfo = null; - if (hostVo.getHypervisorType() == Hypervisor.HypervisorType.KVM && - (hostVo.getResourceState().equals(ResourceState.ErrorInMaintenance) || - hostVo.getResourceState().equals(ResourceState.ErrorInPrepareForMaintenance))) { - VMInstanceDetailVO detailAddress = vmInstanceDetailsDao.findDetail(vm.getId(), VmDetailConstants.KVM_VNC_ADDRESS); - VMInstanceDetailVO detailPort = vmInstanceDetailsDao.findDetail(vm.getId(), VmDetailConstants.KVM_VNC_PORT); - if (detailAddress != null && detailPort != null) { - portInfo = new Pair<>(detailAddress.getValue(), Integer.valueOf(detailPort.getValue())); - } else { - logger.warn("KVM Host in ErrorInMaintenance/ErrorInPrepareForMaintenance but " + - "no VNC Address/Port was available. Falling back to default one from MS."); - } - } - Result result = getConsoleDetails(vm, hostVo, portInfo); - if (result == null) { - return new ConsoleEndpoint(false, null, "Console access to this instance cannot be provided"); - } - String tag = vm.getUuid(); String displayName = vm.getHostName(); if (vm instanceof UserVm) { displayName = ((UserVm) vm).getDisplayName(); } + Map vmDetails = vmInstanceDetailsDao.listDetailsKeyPairs(vm.getId(), + List.of(VmDetailConstants.KEYBOARD, VmDetailConstants.KVM_VNC_ADDRESS, VmDetailConstants.KVM_VNC_PORT)); + if (vmDetails.get(VmDetailConstants.KEYBOARD) != null) { + locale = vmDetails.get(VmDetailConstants.KEYBOARD); + } + ConsoleConnectionDetails details = new ConsoleConnectionDetails(vm.getVncPassword(), locale, tag, displayName); + if (Hypervisor.HypervisorType.External.equals(host.getHypervisorType())) { + return getConsoleConnectionDetailsFoxExternalVm(details, vm, host); + } + Pair hostPortInfo = getHostAndPortForKVMMaintenanceHostIfNeeded(host, vmDetails); + if (hostPortInfo == null) { + hostPortInfo = managementServer.getVncPort(vm); + } + logger.debug("Retrieved VNC host and port info :[{}, {}] for {} on {}", hostPortInfo.first(), + hostPortInfo.second(), vm, host); + Ternary parsedHostInfo = parseHostInfo(hostPortInfo.first()); + details.setHost(parsedHostInfo.first()); + details.setTunnelUrl(parsedHostInfo.second()); + details.setTunnelSession(parsedHostInfo.third()); + details.setPort(hostPortInfo.second()); + if (hostPortInfo.second() == -9) { + details.setUsingRDP(true); + details.setPort(Integer.parseInt(managementServer.findDetail(host.getId(), "rdp.server.port") + .getValue())); + logger.debug("HyperV RDP port for {} on {} is: {}", vm, host, details.getPort()); + } + return details; + } - String ticket = genAccessTicket(result.getHost(), String.valueOf(result.getPort()), result.getSid(), tag, sessionUuid); + protected ConsoleEndpoint composeConsoleAccessEndpoint(String rootUrl, VirtualMachine vm, HostVO hostVo, String addr, + String sessionUuid, String extraSecurityToken) { + ConsoleConnectionDetails result = getConsoleConnectionDetails(vm, hostVo); + if (result == null) { + return new ConsoleEndpoint(false, null, "Console access to this instance cannot be provided"); + } + + String ticket = genAccessTicket(result.getHost(), String.valueOf(result.getPort()), result.getSid(), + result.getTag(), sessionUuid); ConsoleProxyPasswordBasedEncryptor encryptor = new ConsoleProxyPasswordBasedEncryptor(getEncryptorPassword()); - ConsoleProxyClientParam param = generateConsoleProxyClientParam( - new Ternary<>(result.getHost(), result.getTunnelUrl(), result.getTunnelSession()), result.getPort(), - result.getSid(), tag, ticket, sessionUuid, addr, extraSecurityToken, vm, hostVo, result.getLocale(), result.isUsingRDP(), - host, displayName); + ConsoleProxyClientParam param = generateConsoleProxyClientParam(result, ticket, sessionUuid, addr, + extraSecurityToken, vm, hostVo); String token = encryptor.encryptObject(ConsoleProxyClientParam.class, param); int vncPort = consoleProxyManager.getVncPort(); String url = generateConsoleAccessUrl(rootUrl, param, token, vncPort, vm, hostVo, result.getLocale()); - logger.debug("Adding allowed session: " + sessionUuid); + logger.debug("Adding allowed session: {}", sessionUuid); persistConsoleSession(sessionUuid, vm.getId(), hostVo.getId(), addr); managementServer.setConsoleAccessForVm(vm.getId(), sessionUuid); ConsoleEndpoint consoleEndpoint = new ConsoleEndpoint(true, url); consoleEndpoint.setWebsocketHost(managementServer.getConsoleAccessAddress(vm.getId())); consoleEndpoint.setWebsocketPort(String.valueOf(vncPort)); - consoleEndpoint.setWebsocketPath("websockify"); + consoleEndpoint.setWebsocketPath(WEB_SOCKET_PATH); consoleEndpoint.setWebsocketToken(token); if (StringUtils.isNotBlank(param.getExtraSecurityToken())) { consoleEndpoint.setWebsocketExtra(param.getExtraSecurityToken()); @@ -595,7 +549,7 @@ protected void persistConsoleSession(String sessionUuid, long instanceId, long h consoleSessionDao.persist(consoleSessionVo); } - private String generateConsoleAccessUrl(String rootUrl, ConsoleProxyClientParam param, String token, int vncPort, + protected String generateConsoleAccessUrl(String rootUrl, ConsoleProxyClientParam param, String token, int vncPort, VirtualMachine vm, HostVO hostVo, String locale) { StringBuilder sb = new StringBuilder(rootUrl); if (param.getHypervHost() != null || !ConsoleProxyManager.NoVncConsoleDefault.value()) { @@ -626,19 +580,14 @@ private String generateConsoleAccessUrl(String rootUrl, ConsoleProxyClientParam return sb.toString().startsWith("https") ? sb.toString() : "http:" + sb; } - private ConsoleProxyClientParam generateConsoleProxyClientParam(Ternary parsedHostInfo, - int port, String sid, String tag, String ticket, - String sessionUuid, String addr, - String extraSecurityToken, VirtualMachine vm, - HostVO hostVo, String locale, - boolean usingRDP, String host, - String displayName) { + protected ConsoleProxyClientParam generateConsoleProxyClientParam(ConsoleConnectionDetails details, String ticket, + String sessionUuid, String addr, String extraSecurityToken, VirtualMachine vm, HostVO host) { ConsoleProxyClientParam param = new ConsoleProxyClientParam(); - param.setClientHostAddress(parsedHostInfo.first()); - param.setClientHostPort(port); - param.setClientHostPassword(sid); - param.setClientTag(tag); - param.setClientDisplayName(displayName); + param.setClientHostAddress(details.getHost()); + param.setClientHostPort(details.getPort()); + param.setClientHostPassword(details.getSid()); + param.setClientTag(details.getTag()); + param.setClientDisplayName(details.getDisplayName()); param.setTicket(ticket); param.setSessionUuid(sessionUuid); param.setSourceIP(addr); @@ -648,23 +597,23 @@ private ConsoleProxyClientParam generateConsoleProxyClientParam(Ternary result = consoleAccessManager.getHostAndPortForKVMMaintenanceHostIfNeeded(host, Map.of()); + + Assert.assertNull(result); + } + + @Test + public void returnsNullForNonMaintenanceResourceState() { + HostVO host = Mockito.mock(HostVO.class); + Mockito.when(host.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.KVM); + Mockito.when(host.getResourceState()).thenReturn(ResourceState.Enabled); + + Pair result = consoleAccessManager.getHostAndPortForKVMMaintenanceHostIfNeeded(host, Map.of()); + + Assert.assertNull(result); + } + + @Test + public void returnsHostAndPortForValidKVMInMaintenance() { + HostVO host = Mockito.mock(HostVO.class); + Mockito.when(host.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.KVM); + Mockito.when(host.getResourceState()).thenReturn(ResourceState.ErrorInMaintenance); + + String address = "192.168.1.100"; + int port = 5901; + Map vmDetails = Map.of( + VmDetailConstants.KVM_VNC_ADDRESS, address, + VmDetailConstants.KVM_VNC_PORT, String.valueOf(port) + ); + + Pair result = consoleAccessManager.getHostAndPortForKVMMaintenanceHostIfNeeded(host, vmDetails); + + Assert.assertNotNull(result); + Assert.assertEquals(address, result.first()); + Assert.assertEquals(port, (int) result.second()); + } + + @Test + public void returnsNullWhenVncAddressOrPortIsMissing() { + HostVO host = Mockito.mock(HostVO.class); + Mockito.when(host.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.KVM); + Mockito.when(host.getResourceState()).thenReturn(ResourceState.ErrorInMaintenance); + + Map vmDetails = Map.of(VmDetailConstants.KVM_VNC_ADDRESS, "192.168.1.100"); + + Pair result = consoleAccessManager.getHostAndPortForKVMMaintenanceHostIfNeeded(host, vmDetails); + + Assert.assertNull(result); + } + + @Test + public void returnsDetailsForExternalHypervisor() { + VirtualMachine vm = Mockito.mock(VirtualMachine.class); + HostVO host = Mockito.mock(HostVO.class); + ConsoleConnectionDetails details = Mockito.mock(ConsoleConnectionDetails.class); + + Mockito.when(vm.getUuid()).thenReturn("vm-uuid"); + Mockito.when(vm.getHostName()).thenReturn("vm-hostname"); + Mockito.when(vm.getVncPassword()).thenReturn("vnc-password"); + Mockito.when(host.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.External); + Mockito.when(vmInstanceDetailsDao.listDetailsKeyPairs(Mockito.anyLong(), Mockito.anyList())).thenReturn(Map.of()); + + Mockito.doReturn(details).when(consoleAccessManager).getConsoleConnectionDetailsFoxExternalVm(Mockito.any(), Mockito.eq(vm), Mockito.eq(host)); + + ConsoleConnectionDetails result = consoleAccessManager.getConsoleConnectionDetails(vm, host); + + Assert.assertNotNull(result); + Mockito.verify(consoleAccessManager).getConsoleConnectionDetailsFoxExternalVm(Mockito.any(), Mockito.eq(vm), Mockito.eq(host)); + } + + @Test + public void returnsDetailsForKVMHypervisor() { + VirtualMachine vm = Mockito.mock(VirtualMachine.class); + HostVO host = Mockito.mock(HostVO.class); + String hostAddress = "192.168.1.100"; + int port = 5900; + String vmUuid = "vm-uuid"; + String vmHostName = "vm-hostname"; + String vncPassword = "vnc-password"; + + Pair hostPortInfo = new Pair<>(hostAddress, port); + + Mockito.when(vm.getUuid()).thenReturn(vmUuid); + Mockito.when(vm.getHostName()).thenReturn(vmHostName); + Mockito.when(vm.getVncPassword()).thenReturn(vncPassword); + Mockito.when(host.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.KVM); + Mockito.when(vmInstanceDetailsDao.listDetailsKeyPairs(Mockito.anyLong(), Mockito.anyList())).thenReturn(Map.of()); + Mockito.when(managementServer.getVncPort(vm)).thenReturn(hostPortInfo); + Mockito.doReturn(new Ternary<>(hostAddress, null, null)).when(consoleAccessManager).parseHostInfo(Mockito.anyString()); + + ConsoleConnectionDetails result = consoleAccessManager.getConsoleConnectionDetails(vm, host); + + Assert.assertNotNull(result); + Assert.assertEquals(hostAddress, result.getHost()); + Assert.assertEquals(port, result.getPort()); + } + + @Test + public void returnsDetailsWithRDPForHyperV() { + VirtualMachine vm = Mockito.mock(VirtualMachine.class); + HostVO host = Mockito.mock(HostVO.class); + String hostAddress = "192.168.1.100"; + Pair hostPortInfo = new Pair<>(hostAddress, -9); + + Mockito.when(vm.getUuid()).thenReturn("vm-uuid"); + Mockito.when(vm.getHostName()).thenReturn("vm-hostname"); + Mockito.when(vm.getVncPassword()).thenReturn("vnc-password"); + Mockito.when(host.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.Hyperv); + Mockito.when(vmInstanceDetailsDao.listDetailsKeyPairs(Mockito.anyLong(), Mockito.anyList())).thenReturn(Map.of()); + Mockito.when(managementServer.getVncPort(vm)).thenReturn(hostPortInfo); + int port = 3389; + DetailVO detailVO = Mockito.mock(DetailVO.class); + Mockito.when(detailVO.getValue()).thenReturn(String.valueOf(port)); + Mockito.when(managementServer.findDetail(Mockito.anyLong(), Mockito.eq("rdp.server.port"))).thenReturn(detailVO); + Mockito.doReturn(new Ternary<>(hostAddress, null, null)).when(consoleAccessManager).parseHostInfo(Mockito.anyString()); + + ConsoleConnectionDetails result = consoleAccessManager.getConsoleConnectionDetails(vm, host); + + Assert.assertNotNull(result); + Assert.assertTrue(result.isUsingRDP()); + Assert.assertEquals(port, result.getPort()); + } + + @Test + public void returnsNullHostInvalidPortWhenVncPortInfoIsMissing() { + VirtualMachine vm = Mockito.mock(VirtualMachine.class); + HostVO host = Mockito.mock(HostVO.class); + + Mockito.when(vm.getUuid()).thenReturn("vm-uuid"); + Mockito.when(vm.getHostName()).thenReturn("vm-hostname"); + Mockito.when(vm.getVncPassword()).thenReturn("vnc-password"); + Mockito.when(host.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.KVM); + Mockito.when(vmInstanceDetailsDao.listDetailsKeyPairs(Mockito.anyLong(), Mockito.anyList())).thenReturn(Map.of()); + Mockito.when(managementServer.getVncPort(vm)).thenReturn(new Pair<>(null, -1)); + + ConsoleConnectionDetails result = consoleAccessManager.getConsoleConnectionDetails(vm, host); + + Assert.assertNull(result.getHost()); + Assert.assertEquals(-1, result.getPort()); + } + + @Test + public void setsLocaleWhenKeyboardDetailIsPresent() { + VirtualMachine vm = Mockito.mock(VirtualMachine.class); + HostVO host = Mockito.mock(HostVO.class); + String hostAddress = "192.168.1.100"; + Pair hostPortInfo = new Pair<>(hostAddress, 5900); + + Mockito.when(vm.getUuid()).thenReturn("vm-uuid"); + Mockito.when(vm.getHostName()).thenReturn("vm-hostname"); + Mockito.when(vm.getVncPassword()).thenReturn("vnc-password"); + Mockito.when(host.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.KVM); + Mockito.when(vmInstanceDetailsDao.listDetailsKeyPairs(Mockito.anyLong(), Mockito.anyList())).thenReturn(Map.of(VmDetailConstants.KEYBOARD, "en-us")); + Mockito.when(managementServer.getVncPort(vm)).thenReturn(hostPortInfo); + Mockito.doReturn(new Ternary<>(hostAddress, null, null)).when(consoleAccessManager).parseHostInfo(Mockito.anyString()); + + ConsoleConnectionDetails result = consoleAccessManager.getConsoleConnectionDetails(vm, host); + + Assert.assertNotNull(result); + Assert.assertEquals("en-us", result.getLocale()); + } + + @Test + public void setsBasicDetailsCorrectly() { + VirtualMachine vm = Mockito.mock(VirtualMachine.class); + HostVO host = Mockito.mock(HostVO.class); + String hostAddress = "192.168.1.100"; + int port = 5902; + String sid = "sid"; + String tag = "tag"; + String displayName = "displayName"; + String ticket = "ticket"; + String sessionUuid = "sessionUuid"; + String sourceIp = "127.0.0.1"; + ConsoleConnectionDetails details = new ConsoleConnectionDetails(sid, null, tag, displayName); + details.setHost(hostAddress); + details.setPort(port); + + ConsoleProxyClientParam param = consoleAccessManager.generateConsoleProxyClientParam(details, ticket, sessionUuid, sourceIp, null, vm, host); + + Assert.assertEquals(hostAddress, param.getClientHostAddress()); + Assert.assertEquals(port, param.getClientHostPort()); + Assert.assertEquals(sid, param.getClientHostPassword()); + Assert.assertEquals(tag, param.getClientTag()); + Assert.assertEquals(displayName, param.getClientDisplayName()); + Assert.assertEquals(ticket, param.getTicket()); + Assert.assertEquals(sessionUuid, param.getSessionUuid()); + Assert.assertEquals(sourceIp, param.getSourceIP()); + Assert.assertNull(param.getLocale()); + Assert.assertNull(param.getExtraSecurityToken()); + } + + @Test + public void setsExtraSecurityTokenWhenProvided() { + VirtualMachine vm = Mockito.mock(VirtualMachine.class); + HostVO host = Mockito.mock(HostVO.class); + ConsoleConnectionDetails details = new ConsoleConnectionDetails("password", null, null, null); + + ConsoleProxyClientParam param = consoleAccessManager.generateConsoleProxyClientParam(details, "ticket", "sessionUuid", "127.0.0.1", "extraToken", vm, host); + + Assert.assertEquals("extraToken", param.getExtraSecurityToken()); + } + + @Test + public void setsLocaleWhenProvided() { + HostVO host = Mockito.mock(HostVO.class); + VirtualMachine vm = Mockito.mock(VirtualMachine.class); + ConsoleConnectionDetails details = new ConsoleConnectionDetails(null, "fr-fr", null, null); + + ConsoleProxyClientParam param = consoleAccessManager.generateConsoleProxyClientParam(details, "ticket", "sessionUuid", "127.0.0.1", null, vm, host); + + Assert.assertEquals("fr-fr", param.getLocale()); + } + + @Test + public void setsRdpDetailsForHyperV() { + long hostId = 1L; + String username = "admin"; + String password = "adminPass"; + HostVO host = Mockito.mock(HostVO.class); + Mockito.when(host.getId()).thenReturn(hostId); + VirtualMachine vm = Mockito.mock(VirtualMachine.class); + ConsoleConnectionDetails details = new ConsoleConnectionDetails(null, null, null, null); + details.setUsingRDP(true); + String ip = "10.0.0.1"; + Mockito.when(host.getPrivateIpAddress()).thenReturn(ip); + Mockito.when(managementServer.findDetail(host.getId(), "username")).thenReturn(new DetailVO(hostId, "username", username)); + Mockito.when(managementServer.findDetail(host.getId(), "password")).thenReturn(new DetailVO(hostId, "password", password)); + + ConsoleProxyClientParam param = consoleAccessManager.generateConsoleProxyClientParam(details, "ticket", "sessionUuid", "127.0.0.1", null, vm, host); + + Assert.assertEquals(ip, param.getHypervHost()); + Assert.assertEquals(username, param.getUsername()); + Assert.assertEquals(password, param.getPassword()); + } + + @Test + public void setsTunnelDetailsWhenProvided() { + HostVO host = Mockito.mock(HostVO.class); + VirtualMachine vm = Mockito.mock(VirtualMachine.class); + ConsoleConnectionDetails details = new ConsoleConnectionDetails(null, null, null, null); + details.setTunnelUrl("tunnelUrl"); + details.setTunnelSession("tunnelSession"); + + ConsoleProxyClientParam param = consoleAccessManager.generateConsoleProxyClientParam(details, "ticket", "sessionUuid", "127.0.0.1", null, vm, host); + + Assert.assertEquals("tunnelUrl", param.getClientTunnelUrl()); + Assert.assertEquals("tunnelSession", param.getClientTunnelSession()); + } + + @Test + public void returnsNullWhenConsoleConnectionDetailsAreNull() { + VirtualMachine vm = Mockito.mock(VirtualMachine.class); + HostVO host = Mockito.mock(HostVO.class); + Mockito.doReturn(null).when(consoleAccessManager).getConsoleConnectionDetails(vm, host); + + ConsoleEndpoint result = consoleAccessManager.composeConsoleAccessEndpoint("rootUrl", vm, host, "addr", "sessionUuid", "extraToken"); + + Assert.assertNotNull(result); + Assert.assertFalse(result.isResult()); + Assert.assertNull(result.getUrl()); + Assert.assertEquals("Console access to this instance cannot be provided", result.getDetails()); + } + + @Test + public void returnsNullWhenConsoleConnectionDetailsAreValid() { + String locale = "en"; + String hostStr = "192.168.1.100"; + int port = 5900; + String sid = "SID"; + String sessionUuid = UUID.randomUUID().toString(); + String ticket = UUID.randomUUID().toString(); + String addr = "addr"; + String extraToken = "extraToken"; + String rootUrl = "rootUrl"; + int vncPort = 443; + long vmId = 100L; + long hostId = 1L; + String url = "url"; + String consoleAddress = "127.0.0.1"; + VirtualMachine vm = Mockito.mock(VirtualMachine.class); + Mockito.when(vm.getId()).thenReturn(vmId); + HostVO host = Mockito.mock(HostVO.class); + Mockito.when(host.getId()).thenReturn(hostId); + String tag = UUID.randomUUID().toString(); + ConsoleConnectionDetails details = new ConsoleConnectionDetails("password", locale, tag, null); + details.setHost(hostStr); + details.setPort(port); + details.setSid(sid); + Mockito.doReturn(details).when(consoleAccessManager).getConsoleConnectionDetails(vm, host); + Mockito.when(consoleProxyManager.getVncPort()).thenReturn(vncPort); + ConsoleProxyPasswordBasedEncryptor.KeyIVPair keyIvPair = new ConsoleProxyPasswordBasedEncryptor.KeyIVPair("key", "iv"); + Mockito.doReturn(GsonHelper.getGson().toJson(keyIvPair)).when(consoleAccessManager).getEncryptorPassword(); + Mockito.doReturn(ticket).when(consoleAccessManager).genAccessTicket(hostStr, String.valueOf(port), sid, tag, sessionUuid); + ConsoleProxyClientParam param = Mockito.mock(ConsoleProxyClientParam.class); + Mockito.when(param.getExtraSecurityToken()).thenReturn(extraToken); + Mockito.doReturn(param).when(consoleAccessManager).generateConsoleProxyClientParam(details, ticket, sessionUuid, addr, extraToken, vm, host); + Mockito.doReturn(url).when(consoleAccessManager).generateConsoleAccessUrl(Mockito.eq(rootUrl), + Mockito.eq(param), Mockito.anyString(), Mockito.eq(vncPort), Mockito.eq(vm), Mockito.eq(host), + Mockito.eq(locale)); + Mockito.doNothing().when(consoleAccessManager).persistConsoleSession(sessionUuid, vmId, hostId, addr); + Mockito.when(managementServer.getConsoleAccessAddress(vmId)).thenReturn(consoleAddress); + + ConsoleEndpoint endpoint = consoleAccessManager.composeConsoleAccessEndpoint(rootUrl, vm, host, addr, sessionUuid, extraToken); + + Mockito.verify(consoleAccessManager).persistConsoleSession(sessionUuid, vmId, hostId, addr); + Mockito.verify(managementServer).setConsoleAccessForVm(vmId, sessionUuid); + Assert.assertEquals(url, endpoint.getUrl()); + Assert.assertEquals(ConsoleAccessManagerImpl.WEB_SOCKET_PATH, endpoint.getWebsocketPath()); + Assert.assertEquals(extraToken, endpoint.getWebsocketExtra()); + Assert.assertEquals(consoleAddress, endpoint.getWebsocketHost()); + } } From 0617ed4abe780a4dbde18e3ff4c6ef2a3ea9e4d6 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Wed, 10 Sep 2025 16:19:06 +0530 Subject: [PATCH 04/14] fix typo Signed-off-by: Abhishek Kumar --- .../consoleproxy/ConsoleAccessManagerImpl.java | 6 +++--- .../consoleproxy/ConsoleAccessManagerImplTest.java | 14 +++++++------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java b/server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java index 9cf9cf42afc7..4ac56fefeb91 100644 --- a/server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java @@ -425,8 +425,8 @@ private ConsoleEndpoint generateAccessEndpoint(Long vmId, String sessionUuid, St return consoleEndpoint; } - protected ConsoleConnectionDetails getConsoleConnectionDetailsFoxExternalVm(ConsoleConnectionDetails details, - VirtualMachine vm, HostVO host) { + protected ConsoleConnectionDetails getConsoleConnectionDetailsForExternalVm(ConsoleConnectionDetails details, + VirtualMachine vm, HostVO host) { Answer answer = managementServer.getExternalVmConsole(vm, host); if (answer == null) { logger.error("Unable to get console access details for external {} on {}: answer is null.", vm, host); @@ -481,7 +481,7 @@ protected ConsoleConnectionDetails getConsoleConnectionDetails(VirtualMachine vm } ConsoleConnectionDetails details = new ConsoleConnectionDetails(vm.getVncPassword(), locale, tag, displayName); if (Hypervisor.HypervisorType.External.equals(host.getHypervisorType())) { - return getConsoleConnectionDetailsFoxExternalVm(details, vm, host); + return getConsoleConnectionDetailsForExternalVm(details, vm, host); } Pair hostPortInfo = getHostAndPortForKVMMaintenanceHostIfNeeded(host, vmDetails); if (hostPortInfo == null) { diff --git a/server/src/test/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImplTest.java b/server/src/test/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImplTest.java index 798b23fed857..27fca6fac1c8 100644 --- a/server/src/test/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImplTest.java +++ b/server/src/test/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImplTest.java @@ -339,7 +339,7 @@ public void returnsNullWhenAnswerIsNull() { Mockito.when(managementServer.getExternalVmConsole(vm, host)).thenReturn(null); - ConsoleConnectionDetails result = consoleAccessManager.getConsoleConnectionDetailsFoxExternalVm(details, vm, host); + ConsoleConnectionDetails result = consoleAccessManager.getConsoleConnectionDetailsForExternalVm(details, vm, host); Assert.assertNull(result); } @@ -355,7 +355,7 @@ public void returnsNullWhenAnswerResultIsFalse() { Mockito.when(answer.getDetails()).thenReturn("Error details"); Mockito.when(managementServer.getExternalVmConsole(vm, host)).thenReturn(answer); - ConsoleConnectionDetails result = consoleAccessManager.getConsoleConnectionDetailsFoxExternalVm(details, vm, host); + ConsoleConnectionDetails result = consoleAccessManager.getConsoleConnectionDetailsForExternalVm(details, vm, host); Assert.assertNull(result); } @@ -370,7 +370,7 @@ public void returnsNullWhenAnswerIsNotOfTypeGetExternalConsoleAnswer() { Mockito.when(answer.getResult()).thenReturn(true); Mockito.when(managementServer.getExternalVmConsole(vm, host)).thenReturn(answer); - ConsoleConnectionDetails result = consoleAccessManager.getConsoleConnectionDetailsFoxExternalVm(details, vm, host); + ConsoleConnectionDetails result = consoleAccessManager.getConsoleConnectionDetailsForExternalVm(details, vm, host); Assert.assertNull(result); } @@ -392,7 +392,7 @@ public void setsDetailsWhenAnswerIsValid() { Mockito.when(answer.getPassword()).thenReturn(expectedPassword); Mockito.when(managementServer.getExternalVmConsole(vm, host)).thenReturn(answer); - ConsoleConnectionDetails result = consoleAccessManager.getConsoleConnectionDetailsFoxExternalVm(details, vm, host); + ConsoleConnectionDetails result = consoleAccessManager.getConsoleConnectionDetailsForExternalVm(details, vm, host); Assert.assertNotNull(result); Assert.assertEquals(expectedHost, result.getHost()); @@ -413,7 +413,7 @@ public void doesNotSetSidWhenPasswordIsBlank() { Mockito.when(answer.getPassword()).thenReturn(""); Mockito.when(managementServer.getExternalVmConsole(vm, host)).thenReturn(answer); - ConsoleConnectionDetails result = consoleAccessManager.getConsoleConnectionDetailsFoxExternalVm(details, vm, host); + ConsoleConnectionDetails result = consoleAccessManager.getConsoleConnectionDetailsForExternalVm(details, vm, host); Assert.assertNotNull(result); Assert.assertEquals("10.0.0.1", result.getHost()); @@ -487,12 +487,12 @@ public void returnsDetailsForExternalHypervisor() { Mockito.when(host.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.External); Mockito.when(vmInstanceDetailsDao.listDetailsKeyPairs(Mockito.anyLong(), Mockito.anyList())).thenReturn(Map.of()); - Mockito.doReturn(details).when(consoleAccessManager).getConsoleConnectionDetailsFoxExternalVm(Mockito.any(), Mockito.eq(vm), Mockito.eq(host)); + Mockito.doReturn(details).when(consoleAccessManager).getConsoleConnectionDetailsForExternalVm(Mockito.any(), Mockito.eq(vm), Mockito.eq(host)); ConsoleConnectionDetails result = consoleAccessManager.getConsoleConnectionDetails(vm, host); Assert.assertNotNull(result); - Mockito.verify(consoleAccessManager).getConsoleConnectionDetailsFoxExternalVm(Mockito.any(), Mockito.eq(vm), Mockito.eq(host)); + Mockito.verify(consoleAccessManager).getConsoleConnectionDetailsForExternalVm(Mockito.any(), Mockito.eq(vm), Mockito.eq(host)); } @Test From d5c642a2e884ad76199f7bd8cc89b97c052ada67 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Thu, 11 Sep 2025 11:29:41 +0530 Subject: [PATCH 05/14] improve log, error Signed-off-by: Abhishek Kumar --- .../manager/ExtensionsManagerImpl.java | 5 +- .../ExternalPathPayloadProvisioner.java | 40 +++++++------ .../ExternalPathPayloadProvisionerTest.java | 59 +++++++++++++------ 3 files changed, 67 insertions(+), 37 deletions(-) diff --git a/framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImpl.java b/framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImpl.java index b30cbe668bf6..4900a1acf62b 100644 --- a/framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImpl.java +++ b/framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImpl.java @@ -1535,7 +1535,10 @@ public Answer getInstanceConsole(VirtualMachine vm, Host host) { Extension extension = getExtensionForCluster(host.getClusterId()); if (extension == null || !Extension.Type.Orchestrator.equals(extension.getType()) || !Extension.State.Enabled.equals(extension.getState())) { - return new Answer(null, false, "No enabled orchestrator extension found for the host"); + logger.error("No enabled orchestrator {} found for the {} while trying to get console for {}", + extension == null ? "extension" : extension, host, vm); + return new Answer(null, false, + String.format("No enabled orchestrator extension found for the host: %s", host.getName())); } VirtualMachineProfile vmProfile = new VirtualMachineProfileImpl(vm); VirtualMachineTO virtualMachineTO = virtualMachineManager.toVmTO(vmProfile); diff --git a/plugins/hypervisors/external/src/main/java/org/apache/cloudstack/hypervisor/external/provisioner/ExternalPathPayloadProvisioner.java b/plugins/hypervisors/external/src/main/java/org/apache/cloudstack/hypervisor/external/provisioner/ExternalPathPayloadProvisioner.java index c1632030fa76..a2f117214719 100644 --- a/plugins/hypervisors/external/src/main/java/org/apache/cloudstack/hypervisor/external/provisioner/ExternalPathPayloadProvisioner.java +++ b/plugins/hypervisors/external/src/main/java/org/apache/cloudstack/hypervisor/external/provisioner/ExternalPathPayloadProvisioner.java @@ -93,7 +93,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.gson.JsonObject; import com.google.gson.JsonParser; @@ -116,9 +115,6 @@ public class ExternalPathPayloadProvisioner extends ManagerBase implements Exter @Inject HostDao hostDao; - @Inject - VMInstanceDao vmInstanceDao; - @Inject HypervisorGuruManager hypervisorGuruManager; @@ -307,12 +303,20 @@ public String getChecksumForExtensionPath(String extensionName, String relativeP } } + protected String getExtensionConfigureError(String extensionName, String hostName) { + StringBuilder sb = new StringBuilder("Extension: ").append(extensionName).append(" not configured"); + if (StringUtils.isNotBlank(hostName)) { + sb.append(" for host: ").append(hostName); + } + return sb.toString(); + } + @Override - public PrepareExternalProvisioningAnswer prepareExternalProvisioning(String hostGuid, + public PrepareExternalProvisioningAnswer prepareExternalProvisioning(String hostName, String extensionName, String extensionRelativePath, PrepareExternalProvisioningCommand cmd) { String extensionPath = getExtensionCheckedPath(extensionName, extensionRelativePath); if (StringUtils.isEmpty(extensionPath)) { - return new PrepareExternalProvisioningAnswer(cmd, false, "Extension not configured"); + return new PrepareExternalProvisioningAnswer(cmd, false, getExtensionConfigureError(extensionName, hostName)); } VirtualMachineTO vmTO = cmd.getVirtualMachineTO(); String vmUUID = vmTO.getUuid(); @@ -340,11 +344,11 @@ public PrepareExternalProvisioningAnswer prepareExternalProvisioning(String host } @Override - public StartAnswer startInstance(String hostGuid, String extensionName, String extensionRelativePath, + public StartAnswer startInstance(String hostName, String extensionName, String extensionRelativePath, StartCommand cmd) { String extensionPath = getExtensionCheckedPath(extensionName, extensionRelativePath); if (StringUtils.isEmpty(extensionPath)) { - return new StartAnswer(cmd, "Extension not configured"); + return new StartAnswer(cmd, getExtensionConfigureError(extensionName, hostName)); } VirtualMachineTO virtualMachineTO = cmd.getVirtualMachine(); Map accessDetails = loadAccessDetails(cmd.getExternalDetails(), virtualMachineTO); @@ -384,11 +388,11 @@ private Pair executeStartCommandOnExternalSystem(String extensi } @Override - public StopAnswer stopInstance(String hostGuid, String extensionName, String extensionRelativePath, + public StopAnswer stopInstance(String hostName, String extensionName, String extensionRelativePath, StopCommand cmd) { String extensionPath = getExtensionCheckedPath(extensionName, extensionRelativePath); if (StringUtils.isEmpty(extensionPath)) { - return new StopAnswer(cmd, "Extension not configured", false); + return new StopAnswer(cmd, getExtensionConfigureError(extensionName, hostName), false); } logger.debug("Executing stop command on the external provisioner"); VirtualMachineTO virtualMachineTO = cmd.getVirtualMachine(); @@ -405,11 +409,11 @@ public StopAnswer stopInstance(String hostGuid, String extensionName, String ext } @Override - public RebootAnswer rebootInstance(String hostGuid, String extensionName, String extensionRelativePath, + public RebootAnswer rebootInstance(String hostName, String extensionName, String extensionRelativePath, RebootCommand cmd) { String extensionPath = getExtensionCheckedPath(extensionName, extensionRelativePath); if (StringUtils.isEmpty(extensionPath)) { - return new RebootAnswer(cmd, "Extension not configured", false); + return new RebootAnswer(cmd, getExtensionConfigureError(extensionName, hostName), false); } VirtualMachineTO virtualMachineTO = cmd.getVirtualMachine(); String vmUUID = virtualMachineTO.getUuid(); @@ -425,11 +429,11 @@ public RebootAnswer rebootInstance(String hostGuid, String extensionName, String } @Override - public StopAnswer expungeInstance(String hostGuid, String extensionName, String extensionRelativePath, + public StopAnswer expungeInstance(String hostName, String extensionName, String extensionRelativePath, StopCommand cmd) { String extensionPath = getExtensionCheckedPath(extensionName, extensionRelativePath); if (StringUtils.isEmpty(extensionPath)) { - return new StopAnswer(cmd, "Extension not configured", false); + return new StopAnswer(cmd, getExtensionConfigureError(extensionName, hostName), false); } VirtualMachineTO virtualMachineTO = cmd.getVirtualMachine(); String vmUUID = virtualMachineTO.getUuid(); @@ -473,11 +477,11 @@ public Map getHostVmStateReport(long hostId, Str } @Override - public GetExternalConsoleAnswer getInstanceConsole(String hostGuid, String extensionName, + public GetExternalConsoleAnswer getInstanceConsole(String hostName, String extensionName, String extensionRelativePath, GetExternalConsoleCommand cmd) { String extensionPath = getExtensionCheckedPath(extensionName, extensionRelativePath); if (StringUtils.isEmpty(extensionPath)) { - return new GetExternalConsoleAnswer(cmd, "Extension not configured"); + return new GetExternalConsoleAnswer(cmd, getExtensionConfigureError(extensionName, hostName)); } VirtualMachineTO virtualMachineTO = cmd.getVirtualMachine(); String vmUUID = virtualMachineTO.getUuid(); @@ -519,11 +523,11 @@ public GetExternalConsoleAnswer getInstanceConsole(String hostGuid, String exten } @Override - public RunCustomActionAnswer runCustomAction(String hostGuid, String extensionName, + public RunCustomActionAnswer runCustomAction(String hostName, String extensionName, String extensionRelativePath, RunCustomActionCommand cmd) { String extensionPath = getExtensionCheckedPath(extensionName, extensionRelativePath); if (StringUtils.isEmpty(extensionPath)) { - return new RunCustomActionAnswer(cmd, false, "Extension not configured"); + return new RunCustomActionAnswer(cmd, false, getExtensionConfigureError(extensionName, hostName)); } final String actionName = cmd.getActionName(); final Map parameters = cmd.getParameters(); diff --git a/plugins/hypervisors/external/src/test/java/org/apache/cloudstack/hypervisor/external/provisioner/ExternalPathPayloadProvisionerTest.java b/plugins/hypervisors/external/src/test/java/org/apache/cloudstack/hypervisor/external/provisioner/ExternalPathPayloadProvisionerTest.java index 665eaa5bfb0f..db38cb5da3e8 100644 --- a/plugins/hypervisors/external/src/test/java/org/apache/cloudstack/hypervisor/external/provisioner/ExternalPathPayloadProvisionerTest.java +++ b/plugins/hypervisors/external/src/test/java/org/apache/cloudstack/hypervisor/external/provisioner/ExternalPathPayloadProvisionerTest.java @@ -314,7 +314,7 @@ public void testPrepareExternalProvisioning() { .executeExternalCommand(anyString(), anyString(), anyMap(), anyInt(), anyString(), anyString()); PrepareExternalProvisioningAnswer answer = provisioner.prepareExternalProvisioning( - "host-guid", "test-extension", "test-extension.sh", cmd); + "host-name", "test-extension", "test-extension.sh", cmd); assertTrue(answer.getResult()); assertEquals("test-net-uuid", answer.getVirtualMachineTO().getNics()[0].getNetworkUuid()); @@ -326,11 +326,14 @@ public void testPrepareExternalProvisioning() { public void testPrepareExternalProvisioning_ExtensionNotConfigured() { PrepareExternalProvisioningCommand cmd = mock(PrepareExternalProvisioningCommand.class); + String extensionName = "test-extension"; + String hostName = "host-name"; PrepareExternalProvisioningAnswer answer = provisioner.prepareExternalProvisioning( - "host-guid", "test-extension", "nonexistent.sh", cmd); + hostName, extensionName, "nonexistent.sh", cmd); assertFalse(answer.getResult()); - assertEquals("Extension not configured", answer.getDetails()); + assertNotNull(answer); + assertEquals(String.format("Extension: %s not configured for host: %s", extensionName, hostName), answer.getDetails()); } @Test @@ -345,7 +348,7 @@ public void testStartInstance() { doReturn(new Pair<>(true, "{\"status\": \"success\", \"message\": \"Instance started\"}")).when(provisioner) .executeExternalCommand(anyString(), anyString(), anyMap(), anyInt(), anyString(), anyString()); - StartAnswer answer = provisioner.startInstance("host-guid", "test-extension", "test-extension.sh", cmd); + StartAnswer answer = provisioner.startInstance("host-name", "test-extension", "test-extension.sh", cmd); assertTrue(answer.getResult()); Mockito.verify(logger).debug("Starting VM test-uuid on the external system"); @@ -366,7 +369,7 @@ public void testStartInstanceDeploy() { doReturn(new Pair<>(true, "{\"status\": \"success\", \"message\": \"Instance started\"}")).when(provisioner) .executeExternalCommand(anyString(), anyString(), anyMap(), anyInt(), anyString(), anyString()); - StartAnswer answer = provisioner.startInstance("host-guid", "test-extension", "test-extension.sh", cmd); + StartAnswer answer = provisioner.startInstance("host-name", "test-extension", "test-extension.sh", cmd); assertTrue(answer.getResult()); Mockito.verify(logger).debug("Deploying VM test-uuid on the external system"); @@ -384,7 +387,7 @@ public void testStartInstanceError() { doReturn(new Pair<>(false, "{\"error\": \"Instance failed to start\"}")).when(provisioner) .executeExternalCommand(anyString(), anyString(), anyMap(), anyInt(), anyString(), anyString()); - StartAnswer answer = provisioner.startInstance("host-guid", "test-extension", "test-extension.sh", cmd); + StartAnswer answer = provisioner.startInstance("host-name", "test-extension", "test-extension.sh", cmd); assertFalse(answer.getResult()); assertEquals("{\"error\": \"Instance failed to start\"}", answer.getDetails()); @@ -403,7 +406,7 @@ public void testStopInstance() { doReturn(new Pair<>(true, "success")).when(provisioner) .executeExternalCommand(anyString(), anyString(), anyMap(), anyInt(), anyString(), anyString()); - StopAnswer answer = provisioner.stopInstance("host-guid", "test-extension", "test-extension.sh", cmd); + StopAnswer answer = provisioner.stopInstance("host-name", "test-extension", "test-extension.sh", cmd); assertTrue(answer.getResult()); } @@ -420,7 +423,7 @@ public void testRebootInstance() { doReturn(new Pair<>(true, "success")).when(provisioner) .executeExternalCommand(anyString(), anyString(), anyMap(), anyInt(), anyString(), anyString()); - RebootAnswer answer = provisioner.rebootInstance("host-guid", "test-extension", "test-extension.sh", cmd); + RebootAnswer answer = provisioner.rebootInstance("host-name", "test-extension", "test-extension.sh", cmd); assertTrue(answer.getResult()); } @@ -437,7 +440,7 @@ public void testExpungeInstance() { doReturn(new Pair<>(true, "success")).when(provisioner) .executeExternalCommand(anyString(), anyString(), anyMap(), anyInt(), anyString(), anyString()); - StopAnswer answer = provisioner.expungeInstance("host-guid", "test-extension", "test-extension.sh", cmd); + StopAnswer answer = provisioner.expungeInstance("host-name", "test-extension", "test-extension.sh", cmd); assertTrue(answer.getResult()); } @@ -490,7 +493,7 @@ public void testRunCustomAction() { doReturn(new Pair<>(true, "success")).when(provisioner) .executeExternalCommand(anyString(), anyString(), anyMap(), anyInt(), anyString(), anyString()); - RunCustomActionAnswer answer = provisioner.runCustomAction("host-guid", "test-extension", "test-extension.sh", cmd); + RunCustomActionAnswer answer = provisioner.runCustomAction("host-name", "test-extension", "test-extension.sh", cmd); assertTrue(answer.getResult()); Mockito.verify(logger).debug("Executing custom action '{}' in the external system", "test-action"); @@ -770,7 +773,7 @@ public void getInstanceConsoleReturnsAnswerWhenConsoleDetailsAreValid() { doReturn(new Pair<>(true, validOutput)).when(provisioner) .getInstanceConsoleOnExternalSystem(anyString(), anyString(), anyString(), anyMap(), anyInt()); - GetExternalConsoleAnswer result = provisioner.getInstanceConsole("host-guid", "test-extension", "test-extension.sh", cmd); + GetExternalConsoleAnswer result = provisioner.getInstanceConsole("host-name", "test-extension", "test-extension.sh", cmd); assertNotNull(result); assertEquals("127.0.0.1", result.getHost()); @@ -784,11 +787,13 @@ public void getInstanceConsoleReturnsErrorWhenExtensionNotConfigured() { GetExternalConsoleCommand cmd = mock(GetExternalConsoleCommand.class); when(provisioner.getExtensionCheckedPath(anyString(), anyString())).thenReturn(null); - GetExternalConsoleAnswer result = provisioner.getInstanceConsole("host-guid", - "test-extension", "test-extension.sh", cmd); + String extensionName = "test-extension"; + String hostName = "host-name"; + GetExternalConsoleAnswer result = provisioner.getInstanceConsole(hostName, + extensionName, "test-extension.sh", cmd); assertNotNull(result); - assertEquals("Extension not configured", result.getDetails()); + assertEquals(String.format("Extension: %s not configured for host: %s", extensionName, hostName), result.getDetails()); } @Test @@ -801,7 +806,7 @@ public void getInstanceConsoleReturnsErrorWhenExternalSystemFails() { doReturn(new Pair<>(false, "External system error")).when(provisioner) .getInstanceConsoleOnExternalSystem(anyString(), anyString(), anyString(), anyMap(), anyInt()); - GetExternalConsoleAnswer result = provisioner.getInstanceConsole("host-guid", "test-extension", "test-extension.sh", cmd); + GetExternalConsoleAnswer result = provisioner.getInstanceConsole("host-name", "test-extension", "test-extension.sh", cmd); assertNotNull(result); assertEquals("External system error", result.getDetails()); @@ -818,7 +823,7 @@ public void getInstanceConsoleReturnsErrorWhenConsoleObjectIsMissing() { doReturn(new Pair<>(true, invalidOutput)).when(provisioner) .getInstanceConsoleOnExternalSystem(anyString(), anyString(), anyString(), anyMap(), anyInt()); - GetExternalConsoleAnswer result = provisioner.getInstanceConsole("host-guid", "test-extension", "test-extension.sh", cmd); + GetExternalConsoleAnswer result = provisioner.getInstanceConsole("host-name", "test-extension", "test-extension.sh", cmd); assertNotNull(result); assertEquals("Missing console object in output", result.getDetails()); @@ -835,7 +840,7 @@ public void getInstanceConsoleReturnsErrorWhenRequiredFieldsAreMissing() { doReturn(new Pair<>(true, incompleteOutput)).when(provisioner) .getInstanceConsoleOnExternalSystem(anyString(), anyString(), anyString(), anyMap(), anyInt()); - GetExternalConsoleAnswer result = provisioner.getInstanceConsole("host-guid", "test-extension", "test-extension.sh", cmd); + GetExternalConsoleAnswer result = provisioner.getInstanceConsole("host-name", "test-extension", "test-extension.sh", cmd); assertNotNull(result); assertEquals("Missing required fields in output", result.getDetails()); @@ -852,7 +857,7 @@ public void getInstanceConsoleReturnsErrorWhenOutputParsingFails() { doReturn(new Pair<>(true, malformedOutput)).when(provisioner) .getInstanceConsoleOnExternalSystem(anyString(), anyString(), anyString(), anyMap(), anyInt()); - GetExternalConsoleAnswer result = provisioner.getInstanceConsole("host-guid", "test-extension", "test-extension.sh", cmd); + GetExternalConsoleAnswer result = provisioner.getInstanceConsole("host-name", "test-extension", "test-extension.sh", cmd); assertNotNull(result); assertEquals("Failed to parse output", result.getDetails()); @@ -947,4 +952,22 @@ public void getSanitizedJsonStringForLogHandlesMalformedJsonGracefully() { String result = provisioner.getSanitizedJsonStringForLog(json); assertEquals("{password:\"secret\"", result); } + + @Test + public void getExtensionConfigureErrorReturnsMessageWhenHostNameIsNotBlank() { + String result = provisioner.getExtensionConfigureError("test-extension", "test-host"); + assertEquals("Extension: test-extension not configured for host: test-host", result); + } + + @Test + public void getExtensionConfigureErrorReturnsMessageWhenHostNameIsBlank() { + String result = provisioner.getExtensionConfigureError("test-extension", ""); + assertEquals("Extension: test-extension not configured", result); + } + + @Test + public void getExtensionConfigureErrorReturnsMessageWhenHostNameIsNull() { + String result = provisioner.getExtensionConfigureError("test-extension", null); + assertEquals("Extension: test-extension not configured", result); + } } From 63f5f945d97c1616216c13e0ca8a9055c4779288 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Thu, 11 Sep 2025 14:54:44 +0530 Subject: [PATCH 06/14] server,extension: allow returning direct console url Signed-off-by: Abhishek Kumar --- .../CreateConsoleEndpointCmd.java | 5 ++ .../agent/api/GetExternalConsoleAnswer.java | 12 ++- .../ExternalPathPayloadProvisioner.java | 5 +- .../ExternalPathPayloadProvisionerTest.java | 3 +- .../ConsoleAccessManagerImpl.java | 58 +++++++++--- .../ConsoleAccessManagerImplTest.java | 90 ++++++++++++++----- 6 files changed, 137 insertions(+), 36 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/consoleproxy/CreateConsoleEndpointCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/consoleproxy/CreateConsoleEndpointCmd.java index 63b47e163b6b..b84f8ce34899 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/consoleproxy/CreateConsoleEndpointCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/consoleproxy/CreateConsoleEndpointCmd.java @@ -35,6 +35,7 @@ import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.utils.consoleproxy.ConsoleAccessUtils; import org.apache.commons.collections.MapUtils; +import org.apache.commons.lang3.ObjectUtils; import javax.inject.Inject; import java.util.Map; @@ -86,6 +87,10 @@ private CreateConsoleEndpointResponse createResponse(ConsoleEndpoint endpoint) { } private ConsoleEndpointWebsocketResponse createWebsocketResponse(ConsoleEndpoint endpoint) { + if (ObjectUtils.allNull(endpoint.getWebsocketHost(), endpoint.getWebsocketPort(), endpoint.getWebsocketPath(), + endpoint.getWebsocketToken(), endpoint.getWebsocketExtra())) { + return null; + } ConsoleEndpointWebsocketResponse wsResponse = new ConsoleEndpointWebsocketResponse(); wsResponse.setHost(endpoint.getWebsocketHost()); wsResponse.setPort(endpoint.getWebsocketPort()); diff --git a/core/src/main/java/com/cloud/agent/api/GetExternalConsoleAnswer.java b/core/src/main/java/com/cloud/agent/api/GetExternalConsoleAnswer.java index ead4709bdcff..792cd50642fa 100644 --- a/core/src/main/java/com/cloud/agent/api/GetExternalConsoleAnswer.java +++ b/core/src/main/java/com/cloud/agent/api/GetExternalConsoleAnswer.java @@ -19,8 +19,9 @@ public class GetExternalConsoleAnswer extends Answer { + private String url; private String host; - private int port; + private Integer port; @LogLevel(LogLevel.Log4jLevel.Off) private String password; private String protocol; @@ -29,19 +30,24 @@ public GetExternalConsoleAnswer(Command command, String details) { super(command, false, details); } - public GetExternalConsoleAnswer(Command command, String host, int port, String password, String protocol) { + public GetExternalConsoleAnswer(Command command, String url, String host, Integer port, String password, String protocol) { super(command, true, ""); + this.url = url; this.host = host; this.port = port; this.password = password; this.protocol = protocol; } + public String getUrl() { + return url; + } + public String getHost() { return host; } - public int getPort() { + public Integer getPort() { return port; } diff --git a/plugins/hypervisors/external/src/main/java/org/apache/cloudstack/hypervisor/external/provisioner/ExternalPathPayloadProvisioner.java b/plugins/hypervisors/external/src/main/java/org/apache/cloudstack/hypervisor/external/provisioner/ExternalPathPayloadProvisioner.java index a2f117214719..5eff5d814d6e 100644 --- a/plugins/hypervisors/external/src/main/java/org/apache/cloudstack/hypervisor/external/provisioner/ExternalPathPayloadProvisioner.java +++ b/plugins/hypervisors/external/src/main/java/org/apache/cloudstack/hypervisor/external/provisioner/ExternalPathPayloadProvisioner.java @@ -506,16 +506,17 @@ public GetExternalConsoleAnswer getInstanceConsole(String hostName, String exten getSanitizedJsonStringForLog(output)); return new GetExternalConsoleAnswer(cmd, "Missing console object in output"); } + String url = consoleObj.has("url") ? consoleObj.get("url").getAsString() : null; String host = consoleObj.has("host") ? consoleObj.get("host").getAsString() : null; Integer port = consoleObj.has("port") ? Integer.valueOf(consoleObj.get("port").getAsString()) : null; String password = consoleObj.has("password") ? consoleObj.get("password").getAsString() : null; String protocol = consoleObj.has("protocol") ? consoleObj.get("protocol").getAsString() : null; - if (ObjectUtils.anyNull(host, port)) { + if (url == null && ObjectUtils.anyNull(host, port)) { logger.error("Missing required fields in external console output: {}", getSanitizedJsonStringForLog(output)); return new GetExternalConsoleAnswer(cmd, "Missing required fields in output"); } - return new GetExternalConsoleAnswer(cmd, host, port, password, protocol); + return new GetExternalConsoleAnswer(cmd, url, host, port, password, protocol); } catch (RuntimeException e) { logger.error("Failed to parse output for getInstanceConsole: {}", e.getMessage(), e); return new GetExternalConsoleAnswer(cmd, "Failed to parse output"); diff --git a/plugins/hypervisors/external/src/test/java/org/apache/cloudstack/hypervisor/external/provisioner/ExternalPathPayloadProvisionerTest.java b/plugins/hypervisors/external/src/test/java/org/apache/cloudstack/hypervisor/external/provisioner/ExternalPathPayloadProvisionerTest.java index db38cb5da3e8..3c99fcd03683 100644 --- a/plugins/hypervisors/external/src/test/java/org/apache/cloudstack/hypervisor/external/provisioner/ExternalPathPayloadProvisionerTest.java +++ b/plugins/hypervisors/external/src/test/java/org/apache/cloudstack/hypervisor/external/provisioner/ExternalPathPayloadProvisionerTest.java @@ -777,7 +777,8 @@ public void getInstanceConsoleReturnsAnswerWhenConsoleDetailsAreValid() { assertNotNull(result); assertEquals("127.0.0.1", result.getHost()); - assertEquals(5900, result.getPort()); + Integer port = 5900; + assertEquals(port, result.getPort()); assertEquals("pass", result.getPassword()); assertEquals("vnc", result.getProtocol()); } diff --git a/server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java b/server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java index 4ac56fefeb91..d41b388fbe66 100644 --- a/server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java @@ -441,8 +441,12 @@ protected ConsoleConnectionDetails getConsoleConnectionDetailsForExternalVm(Cons return null; } GetExternalConsoleAnswer getExternalConsoleAnswer = (GetExternalConsoleAnswer) answer; + details.setModeFromExternalProtocol(getExternalConsoleAnswer.getProtocol()); + details.setDirectUrl(getExternalConsoleAnswer.getUrl()); details.setHost(getExternalConsoleAnswer.getHost()); - details.setPort(getExternalConsoleAnswer.getPort()); + if (getExternalConsoleAnswer.getPort() != null) { + details.setPort(getExternalConsoleAnswer.getPort()); + } if (StringUtils.isNotBlank(getExternalConsoleAnswer.getPassword())) { details.setSid(getExternalConsoleAnswer.getPassword()); } @@ -510,6 +514,11 @@ protected ConsoleEndpoint composeConsoleAccessEndpoint(String rootUrl, VirtualMa return new ConsoleEndpoint(false, null, "Console access to this instance cannot be provided"); } + if (ConsoleConnectionDetails.Mode.Direct.equals(result.getMode())) { + persistConsoleSession(sessionUuid, vm.getId(), hostVo.getId(), addr); + return new ConsoleEndpoint(true, result.getDirectUrl()); + } + String ticket = genAccessTicket(result.getHost(), String.valueOf(result.getPort()), result.getSid(), result.getTag(), sessionUuid); ConsoleProxyPasswordBasedEncryptor encryptor = new ConsoleProxyPasswordBasedEncryptor(getEncryptorPassword()); @@ -736,15 +745,22 @@ private String acquireVncTicketForVmwareVm(VirtualMachine vm) { } protected static class ConsoleConnectionDetails { - String host; - int port; - String sid; - String locale; - String tag; - String displayName; - String tunnelUrl = null; - String tunnelSession = null; - boolean usingRDP; + public enum Mode { + ConsoleProxy, + Direct + } + + private Mode mode = Mode.ConsoleProxy; + private String host; + private int port = -1; + private String sid; + private String locale; + private String tag; + private String displayName; + private String tunnelUrl = null; + private String tunnelSession = null; + private boolean usingRDP; + private String directUrl; ConsoleConnectionDetails(String sid, String locale, String tag, String displayName) { this.sid = sid; @@ -753,6 +769,20 @@ protected static class ConsoleConnectionDetails { this.displayName = displayName; } + public Mode getMode() { + return mode; + } + + public void setModeFromExternalProtocol(String protocol) { + this.mode = Mode.ConsoleProxy; + if (StringUtils.isBlank(protocol)) { + return; + } + if (List.of("link", "url", "direct").contains(protocol.toLowerCase())) { + this.mode = Mode.Direct; + } + } + public String getHost() { return host; } @@ -812,5 +842,13 @@ public boolean isUsingRDP() { public void setUsingRDP(boolean usingRDP) { this.usingRDP = usingRDP; } + + public String getDirectUrl() { + return directUrl; + } + + public void setDirectUrl(String directUrl) { + this.directUrl = directUrl; + } } } diff --git a/server/src/test/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImplTest.java b/server/src/test/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImplTest.java index 27fca6fac1c8..1a4ef5561107 100644 --- a/server/src/test/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImplTest.java +++ b/server/src/test/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImplTest.java @@ -332,7 +332,7 @@ public void listConsoleSessionByIdTestShouldCallDbLayer() { } @Test - public void returnsNullWhenAnswerIsNull() { + public void getConsoleConnectionDetailsForExternalVmReturnsNullWhenAnswerIsNull() { VirtualMachine vm = Mockito.mock(VirtualMachine.class); HostVO host = Mockito.mock(HostVO.class); ConsoleConnectionDetails details = new ConsoleConnectionDetails("sid", "en", "tag", "displayName"); @@ -345,7 +345,7 @@ public void returnsNullWhenAnswerIsNull() { } @Test - public void returnsNullWhenAnswerResultIsFalse() { + public void getConsoleConnectionDetailsForExternalVmReturnsNullWhenAnswerResultIsFalse() { VirtualMachine vm = Mockito.mock(VirtualMachine.class); HostVO host = Mockito.mock(HostVO.class); ConsoleConnectionDetails details = new ConsoleConnectionDetails("sid", "en", "tag", "displayName"); @@ -361,7 +361,7 @@ public void returnsNullWhenAnswerResultIsFalse() { } @Test - public void returnsNullWhenAnswerIsNotOfTypeGetExternalConsoleAnswer() { + public void getConsoleConnectionDetailsForExternalVmReturnsNullWhenAnswerIsNotOfTypeGetExternalConsoleAnswer() { VirtualMachine vm = Mockito.mock(VirtualMachine.class); HostVO host = Mockito.mock(HostVO.class); ConsoleConnectionDetails details = new ConsoleConnectionDetails("sid", "en", "tag", "displayName"); @@ -376,7 +376,7 @@ public void returnsNullWhenAnswerIsNotOfTypeGetExternalConsoleAnswer() { } @Test - public void setsDetailsWhenAnswerIsValid() { + public void getConsoleConnectionDetailsForExternalVmSetsDetailsWhenAnswerIsValid() { VirtualMachine vm = Mockito.mock(VirtualMachine.class); HostVO host = Mockito.mock(HostVO.class); ConsoleConnectionDetails details = new ConsoleConnectionDetails("sid", "en", "tag", "displayName"); @@ -395,13 +395,36 @@ public void setsDetailsWhenAnswerIsValid() { ConsoleConnectionDetails result = consoleAccessManager.getConsoleConnectionDetailsForExternalVm(details, vm, host); Assert.assertNotNull(result); + Assert.assertEquals(ConsoleConnectionDetails.Mode.ConsoleProxy, result.getMode()); Assert.assertEquals(expectedHost, result.getHost()); Assert.assertEquals(expectedPort, result.getPort()); Assert.assertEquals(expectedPassword, result.getSid()); + Assert.assertNull(result.getDirectUrl()); } @Test - public void doesNotSetSidWhenPasswordIsBlank() { + public void getConsoleConnectionDetailsForExternalVmSetsDetailsWhenAnswerIsValidDirect() { + VirtualMachine vm = Mockito.mock(VirtualMachine.class); + HostVO host = Mockito.mock(HostVO.class); + ConsoleConnectionDetails details = new ConsoleConnectionDetails("sid", "en", "tag", "displayName"); + GetExternalConsoleAnswer answer = Mockito.mock(GetExternalConsoleAnswer.class); + + String url = "url"; + + Mockito.when(answer.getResult()).thenReturn(true); + Mockito.when(answer.getUrl()).thenReturn(url); + Mockito.when(answer.getProtocol()).thenReturn("url"); + Mockito.when(managementServer.getExternalVmConsole(vm, host)).thenReturn(answer); + + ConsoleConnectionDetails result = consoleAccessManager.getConsoleConnectionDetailsForExternalVm(details, vm, host); + + Assert.assertNotNull(result); + Assert.assertEquals(ConsoleConnectionDetails.Mode.Direct, result.getMode()); + Assert.assertEquals(url, result.getDirectUrl()); + } + + @Test + public void getConsoleConnectionDetailsForExternalVmDoesNotSetSidWhenPasswordIsBlank() { VirtualMachine vm = Mockito.mock(VirtualMachine.class); HostVO host = Mockito.mock(HostVO.class); ConsoleConnectionDetails details = new ConsoleConnectionDetails("sid", "en", "tag", "displayName"); @@ -422,7 +445,7 @@ public void doesNotSetSidWhenPasswordIsBlank() { } @Test - public void returnsNullForNonKVMHypervisor() { + public void getHostAndPortForKVMMaintenanceHostIfNeededReturnsNullForNonKVMHypervisor() { HostVO host = Mockito.mock(HostVO.class); Mockito.when(host.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.XenServer); @@ -432,7 +455,7 @@ public void returnsNullForNonKVMHypervisor() { } @Test - public void returnsNullForNonMaintenanceResourceState() { + public void getHostAndPortForKVMMaintenanceHostIfNeededReturnsNullForNonMaintenanceResourceState() { HostVO host = Mockito.mock(HostVO.class); Mockito.when(host.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.KVM); Mockito.when(host.getResourceState()).thenReturn(ResourceState.Enabled); @@ -443,7 +466,7 @@ public void returnsNullForNonMaintenanceResourceState() { } @Test - public void returnsHostAndPortForValidKVMInMaintenance() { + public void getHostAndPortForKVMMaintenanceHostIfNeededReturnsHostAndPortForValidKVMInMaintenance() { HostVO host = Mockito.mock(HostVO.class); Mockito.when(host.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.KVM); Mockito.when(host.getResourceState()).thenReturn(ResourceState.ErrorInMaintenance); @@ -463,7 +486,7 @@ public void returnsHostAndPortForValidKVMInMaintenance() { } @Test - public void returnsNullWhenVncAddressOrPortIsMissing() { + public void getHostAndPortForKVMMaintenanceHostIfNeededReturnsNullWhenVncAddressOrPortIsMissing() { HostVO host = Mockito.mock(HostVO.class); Mockito.when(host.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.KVM); Mockito.when(host.getResourceState()).thenReturn(ResourceState.ErrorInMaintenance); @@ -476,7 +499,7 @@ public void returnsNullWhenVncAddressOrPortIsMissing() { } @Test - public void returnsDetailsForExternalHypervisor() { + public void getConsoleConnectionDetailsReturnsDetailsForExternalHypervisor() { VirtualMachine vm = Mockito.mock(VirtualMachine.class); HostVO host = Mockito.mock(HostVO.class); ConsoleConnectionDetails details = Mockito.mock(ConsoleConnectionDetails.class); @@ -496,7 +519,7 @@ public void returnsDetailsForExternalHypervisor() { } @Test - public void returnsDetailsForKVMHypervisor() { + public void getConsoleConnectionDetailsReturnsDetailsForKVMHypervisor() { VirtualMachine vm = Mockito.mock(VirtualMachine.class); HostVO host = Mockito.mock(HostVO.class); String hostAddress = "192.168.1.100"; @@ -523,7 +546,7 @@ public void returnsDetailsForKVMHypervisor() { } @Test - public void returnsDetailsWithRDPForHyperV() { + public void getConsoleConnectionDetailsReturnsDetailsWithRDPForHyperV() { VirtualMachine vm = Mockito.mock(VirtualMachine.class); HostVO host = Mockito.mock(HostVO.class); String hostAddress = "192.168.1.100"; @@ -549,7 +572,7 @@ public void returnsDetailsWithRDPForHyperV() { } @Test - public void returnsNullHostInvalidPortWhenVncPortInfoIsMissing() { + public void getConsoleConnectionDetailsReturnsNullHostInvalidPortWhenVncPortInfoIsMissing() { VirtualMachine vm = Mockito.mock(VirtualMachine.class); HostVO host = Mockito.mock(HostVO.class); @@ -567,7 +590,7 @@ public void returnsNullHostInvalidPortWhenVncPortInfoIsMissing() { } @Test - public void setsLocaleWhenKeyboardDetailIsPresent() { + public void getConsoleConnectionDetailsSetsLocaleWhenKeyboardDetailIsPresent() { VirtualMachine vm = Mockito.mock(VirtualMachine.class); HostVO host = Mockito.mock(HostVO.class); String hostAddress = "192.168.1.100"; @@ -588,7 +611,7 @@ public void setsLocaleWhenKeyboardDetailIsPresent() { } @Test - public void setsBasicDetailsCorrectly() { + public void generateConsoleProxyClientParamSetsBasicDetailsCorrectly() { VirtualMachine vm = Mockito.mock(VirtualMachine.class); HostVO host = Mockito.mock(HostVO.class); String hostAddress = "192.168.1.100"; @@ -618,7 +641,7 @@ public void setsBasicDetailsCorrectly() { } @Test - public void setsExtraSecurityTokenWhenProvided() { + public void generateConsoleProxyClientParamSetsExtraSecurityTokenWhenProvided() { VirtualMachine vm = Mockito.mock(VirtualMachine.class); HostVO host = Mockito.mock(HostVO.class); ConsoleConnectionDetails details = new ConsoleConnectionDetails("password", null, null, null); @@ -629,7 +652,7 @@ public void setsExtraSecurityTokenWhenProvided() { } @Test - public void setsLocaleWhenProvided() { + public void generateConsoleProxyClientParamSetsLocaleWhenProvided() { HostVO host = Mockito.mock(HostVO.class); VirtualMachine vm = Mockito.mock(VirtualMachine.class); ConsoleConnectionDetails details = new ConsoleConnectionDetails(null, "fr-fr", null, null); @@ -640,7 +663,7 @@ public void setsLocaleWhenProvided() { } @Test - public void setsRdpDetailsForHyperV() { + public void generateConsoleProxyClientParamSetsRdpDetailsForHyperV() { long hostId = 1L; String username = "admin"; String password = "adminPass"; @@ -662,7 +685,7 @@ public void setsRdpDetailsForHyperV() { } @Test - public void setsTunnelDetailsWhenProvided() { + public void generateConsoleProxyClientParamSetsTunnelDetailsWhenProvided() { HostVO host = Mockito.mock(HostVO.class); VirtualMachine vm = Mockito.mock(VirtualMachine.class); ConsoleConnectionDetails details = new ConsoleConnectionDetails(null, null, null, null); @@ -690,7 +713,7 @@ public void returnsNullWhenConsoleConnectionDetailsAreNull() { } @Test - public void returnsNullWhenConsoleConnectionDetailsAreValid() { + public void composeConsoleAccessEndpointReturnsConsoleEndpointWhenConsoleConnectionDetailsAreValid() { String locale = "en"; String hostStr = "192.168.1.100"; int port = 5900; @@ -737,4 +760,31 @@ public void returnsNullWhenConsoleConnectionDetailsAreValid() { Assert.assertEquals(extraToken, endpoint.getWebsocketExtra()); Assert.assertEquals(consoleAddress, endpoint.getWebsocketHost()); } + + @Test + public void composeConsoleAccessEndpointReturnsWithoutPersistWhenConsoleConnectionDetailsAreValidDirect() { + String url = "url"; + long vmId = 100L; + long hostId = 1L; + String sessionUuid = UUID.randomUUID().toString(); + String addr = "addr"; + ConsoleConnectionDetails details = new ConsoleConnectionDetails("password", "en", "tag", null); + details.setDirectUrl(url); + details.setModeFromExternalProtocol("url"); + VirtualMachine vm = Mockito.mock(VirtualMachine.class); + Mockito.when(vm.getId()).thenReturn(vmId); + HostVO host = Mockito.mock(HostVO.class); + Mockito.when(host.getId()).thenReturn(hostId); + Mockito.doReturn(details).when(consoleAccessManager).getConsoleConnectionDetails(vm, host); + Mockito.doNothing().when(consoleAccessManager).persistConsoleSession(sessionUuid, vmId, hostId, addr); + + ConsoleEndpoint endpoint = consoleAccessManager.composeConsoleAccessEndpoint("url", vm, host, addr, sessionUuid, ""); + + Mockito.verify(consoleAccessManager).persistConsoleSession(sessionUuid, vmId, hostId, addr); + Mockito.verify(managementServer, Mockito.never()).setConsoleAccessForVm(Mockito.anyLong(), Mockito.anyString()); + Assert.assertEquals(url, endpoint.getUrl()); + Assert.assertNull(endpoint.getWebsocketPath()); + Assert.assertNull(endpoint.getWebsocketExtra()); + Assert.assertNull(endpoint.getWebsocketHost()); + } } From e46f5cccc5f15ca31b2a714e8e7d8ccaf004902b Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Thu, 11 Sep 2025 23:42:22 +0530 Subject: [PATCH 07/14] address review, scripts consistency Signed-off-by: Abhishek Kumar --- extensions/HyperV/hyperv.py | 6 +- extensions/Proxmox/proxmox.sh | 174 +++++++++--------- .../external/provisioner/provisioner.sh | 13 +- 3 files changed, 101 insertions(+), 92 deletions(-) diff --git a/extensions/HyperV/hyperv.py b/extensions/HyperV/hyperv.py index 8ae2c7ff7979..c9b1d4da77e3 100755 --- a/extensions/HyperV/hyperv.py +++ b/extensions/HyperV/hyperv.py @@ -25,7 +25,7 @@ def fail(message): - print(json.dumps({"error": message})) + print(json.dumps({"status": "error", "error": message})) sys.exit(1) @@ -220,6 +220,9 @@ def delete(self): fail(str(e)) succeed({"status": "success", "message": "Instance deleted"}) + def get_console(self): + fail("Operation not supported") + def suspend(self): self.run_ps(f'Suspend-VM -Name "{self.data["vmname"]}"') succeed({"status": "success", "message": "Instance suspended"}) @@ -283,6 +286,7 @@ def main(): "reboot": manager.reboot, "delete": manager.delete, "status": manager.status, + "getconsole": manager.get_console, "suspend": manager.suspend, "resume": manager.resume, "listsnapshots": manager.list_snapshots, diff --git a/extensions/Proxmox/proxmox.sh b/extensions/Proxmox/proxmox.sh index a6d7a91e9376..0982040b71bc 100755 --- a/extensions/Proxmox/proxmox.sh +++ b/extensions/Proxmox/proxmox.sh @@ -18,7 +18,7 @@ parse_json() { local json_string="$1" - echo "$json_string" | jq '.' > /dev/null || { echo '{"error":"Invalid JSON input"}'; exit 1; } + echo "$json_string" | jq '.' > /dev/null || { echo '{"status": "error", "error": "Invalid JSON input"}'; exit 1; } local -A details while IFS="=" read -r key value; do @@ -112,9 +112,14 @@ call_proxmox_api() { curl_opts+=(-d "$data") fi - #echo curl "${curl_opts[@]}" "https://${url}:8006/api2/json${path}" >&2 response=$(curl "${curl_opts[@]}" "https://${url}:8006/api2/json${path}") + local status=$? + if [[ $status -ne 0 ]]; then + echo "{\"errors\":{\"curl\":\"API call failed with status $status: $(echo "$response" | jq -Rsa . | jq -r .)\"}}" + return $status + fi echo "$response" + return 0 } wait_for_proxmox_task() { @@ -129,7 +134,7 @@ wait_for_proxmox_task() { local now now=$(date +%s) if (( now - start_time > timeout )); then - echo '{"error":"Timeout while waiting for async task"}' + echo '{"status": "error", "error":"Timeout while waiting for async task"}' exit 1 fi @@ -139,7 +144,7 @@ wait_for_proxmox_task() { if [[ -z "$status_response" || "$status_response" == *'"errors":'* ]]; then local msg msg=$(echo "$status_response" | jq -r '.message // "Unknown error"') - echo "{\"error\":\"$msg\"}" + echo "{\"status\": \"error\", \"error\": \"$msg\"}" exit 1 fi @@ -286,85 +291,81 @@ status() { } get_node_host() { - check_required_fields node - local net_json host - - if ! net_json="$(call_proxmox_api GET "/nodes/${node}/network")"; then - echo "" - return 1 - fi - - # Prefer a static non-bridge IP - host="$(echo "$net_json" | jq -r ' - .data - | map(select( - (.type // "") != "bridge" and - (.type // "") != "bond" and - (.method // "") == "static" and - ((.address // .cidr // "") != "") - )) - | map(.address // (.cidr | split("/")[0])) - | .[0] // empty - ' 2>/dev/null)" - - # Fallback: first interface with a CIDR - if [[ -z "$host" ]]; then - host="$(echo "$net_json" | jq -r ' - .data - | map(select((.cidr // "") != "")) - | map(.cidr | split("/")[0]) - | .[0] // empty - ' 2>/dev/null)" - fi - - echo "$host" - } + check_required_fields node + local net_json host + + if ! net_json="$(call_proxmox_api GET "/nodes/${node}/network")"; then + echo "" + return 1 + fi + + # Prefer a static non-bridge IP + host="$(echo "$net_json" | jq -r ' + .data + | map(select( + (.type // "") != "bridge" and + (.type // "") != "bond" and + (.method // "") == "static" and + ((.address // .cidr // "") != "") + )) + | map(.address // (.cidr | split("/")[0])) + | .[0] // empty + ' 2>/dev/null)" + + # Fallback: first interface with a CIDR + if [[ -z "$host" ]]; then + host="$(echo "$net_json" | jq -r ' + .data + | map(select((.cidr // "") != "")) + | map(.cidr | split("/")[0]) + | .[0] // empty + ' 2>/dev/null)" + fi + + echo "$host" +} get_console() { - check_required_fields node vmid - - # Request VNC proxy from Proxmox - local api_resp port ticket - if ! api_resp="$(call_proxmox_api POST "/nodes/${node}/qemu/${vmid}/vncproxy")"; then - jq -n --arg msg "API call failed for node=$node vmid=$vmid" \ - '{status:"error", message:$msg, code:"API_CALL_FAILED"}' - return 1 - fi - - port="$(echo "$api_resp" | jq -re '.data.port // empty' 2>/dev/null || true)" - ticket="$(echo "$api_resp" | jq -re '.data.ticket // empty' 2>/dev/null || true)" - - if [[ -z "$port" || -z "$ticket" ]]; then - jq -n --arg msg "Proxmox response missing port/ticket" \ - --arg raw "$api_resp" \ - '{status:"error", message:$msg, code:"BAD_UPSTREAM_RESPONSE", upstream:$raw}' - return 1 - fi - - # Derive host from node’s network info - local host - host="$(get_node_host)" - if [[ -z "$host" ]]; then - jq -n --arg msg "Could not determine host IP for node $node" \ - '{status:"error", message:$msg, code:"HOST_RESOLUTION_ERROR"}' - return 1 - fi - - # Success JSON to stdout - jq -n \ - --arg host "$host" \ - --arg port "$port" \ - --arg password "$ticket" \ - '{ - status: "success", - message: "Console retrieved", - console: { - host: $host, - port: $port, - password: $password, - protocol: "vnc" - } - }' + check_required_fields node vmid + + local api_resp port ticket + if ! api_resp="$(call_proxmox_api POST "/nodes/${node}/qemu/${vmid}/vncproxy")"; then + echo "$api_resp" | jq -c '{status:"error", error:(.errors.curl // (.errors|tostring))}' + exit 1 + fi + + port="$(echo "$api_resp" | jq -re '.data.port // empty' 2>/dev/null || true)" + ticket="$(echo "$api_resp" | jq -re '.data.ticket // empty' 2>/dev/null || true)" + + if [[ -z "$port" || -z "$ticket" ]]; then + jq -n --arg raw "$api_resp" \ + '{status:"error", error:"Proxmox response missing port/ticket", upstream:$raw}' + exit 1 + fi + + # Derive host from node’s network info + local host + host="$(get_node_host)" + if [[ -z "$host" ]]; then + jq -n --arg msg "Could not determine host IP for node $node" \ + '{status:"error", error:$msg}' + exit 1 + fi + + jq -n \ + --arg host "$host" \ + --arg port "$port" \ + --arg password "$ticket" \ + '{ + status: "success", + message: "Console retrieved", + console: { + host: $host, + port: $port, + password: $password, + protocol: "vnc" + } + }' } list_snapshots() { @@ -438,7 +439,12 @@ parameters_file="$2" wait_time=$3 if [[ -z "$action" || -z "$parameters_file" ]]; then - echo '{"error":"Missing required arguments"}' + echo '{"status": "error", "error": "Missing required arguments"}' + exit 1 +fi + +if [[ ! -r "$parameters_file" ]]; then + echo '{"status": "error", "error": "File not found or unreadable"}' exit 1 fi @@ -479,7 +485,7 @@ case $action in status ;; getconsole) - get_console "$parameters" + get_console ;; ListSnapshots) list_snapshots @@ -494,7 +500,7 @@ case $action in delete_snapshot ;; *) - echo '{"error":"Invalid action"}' + echo '{"status": "error", "error": "Invalid action"}' exit 1 ;; esac diff --git a/scripts/vm/hypervisor/external/provisioner/provisioner.sh b/scripts/vm/hypervisor/external/provisioner/provisioner.sh index 950f3aee74bf..f067d892f1f3 100755 --- a/scripts/vm/hypervisor/external/provisioner/provisioner.sh +++ b/scripts/vm/hypervisor/external/provisioner/provisioner.sh @@ -18,7 +18,7 @@ parse_json() { local json_string=$1 - echo "$json_string" | jq '.' > /dev/null || { echo '{"error":"Invalid JSON input"}'; exit 1; } + echo "$json_string" | jq '.' > /dev/null || { echo '{"status": "error", "error": "Invalid JSON input"}'; exit 1; } } generate_random_mac() { @@ -102,9 +102,8 @@ status() { get_console() { parse_json "$1" || exit 1 local response - jq -n --arg msg "Operation not supported" \ - '{status:"error", message:$msg, code:"OPERATION_NOT_SUPPORTED"}' - return 1 + jq -n '{status:"error", error: "Operation not supported"}' + exit 1 } action=$1 @@ -112,12 +111,12 @@ parameters_file="$2" wait_time="$3" if [[ -z "$action" || -z "$parameters_file" ]]; then - echo '{"error":"Missing required arguments"}' + echo '{"status": "error", "error": "Missing required arguments"}' exit 1 fi if [[ ! -r "$parameters_file" ]]; then - echo '{"error":"File not found or unreadable"}' + echo '{"status": "error", "error": "File not found or unreadable"}' exit 1 fi @@ -150,7 +149,7 @@ case $action in get_console "$parameters" ;; *) - echo '{"error":"Invalid action"}' + echo '{"status": "error", "error": "Invalid action"}' exit 1 ;; esac From 5f95f77c5997f755dbd0595078f69237a0868831 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Thu, 18 Sep 2025 13:34:03 +0530 Subject: [PATCH 08/14] change for one-time ticket as password Signed-off-by: Abhishek Kumar --- .../cloud/agent/api/GetExternalConsoleAnswer.java | 9 ++++++++- extensions/Proxmox/proxmox.sh | 2 ++ .../ExternalPathPayloadProvisioner.java | 4 +++- .../com/cloud/servlet/ConsoleProxyClientParam.java | 6 ++++++ .../consoleproxy/ConsoleAccessManagerImpl.java | 11 +++++++++++ .../consoleproxy/ConsoleProxyClientParam.java | 14 +++++++++++++- 6 files changed, 43 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/com/cloud/agent/api/GetExternalConsoleAnswer.java b/core/src/main/java/com/cloud/agent/api/GetExternalConsoleAnswer.java index 792cd50642fa..e913d6f0d3a1 100644 --- a/core/src/main/java/com/cloud/agent/api/GetExternalConsoleAnswer.java +++ b/core/src/main/java/com/cloud/agent/api/GetExternalConsoleAnswer.java @@ -25,17 +25,20 @@ public class GetExternalConsoleAnswer extends Answer { @LogLevel(LogLevel.Log4jLevel.Off) private String password; private String protocol; + private boolean passwordOneTimeUseOnly; public GetExternalConsoleAnswer(Command command, String details) { super(command, false, details); } - public GetExternalConsoleAnswer(Command command, String url, String host, Integer port, String password, String protocol) { + public GetExternalConsoleAnswer(Command command, String url, String host, Integer port, String password, + boolean passwordOneTimeUseOnly, String protocol) { super(command, true, ""); this.url = url; this.host = host; this.port = port; this.password = password; + this.passwordOneTimeUseOnly = passwordOneTimeUseOnly; this.protocol = protocol; } @@ -58,4 +61,8 @@ public String getPassword() { public String getProtocol() { return protocol; } + + public boolean isPasswordOneTimeUseOnly() { + return passwordOneTimeUseOnly; + } } diff --git a/extensions/Proxmox/proxmox.sh b/extensions/Proxmox/proxmox.sh index 0982040b71bc..23f30311e2b7 100755 --- a/extensions/Proxmox/proxmox.sh +++ b/extensions/Proxmox/proxmox.sh @@ -356,6 +356,7 @@ get_node_host() { --arg host "$host" \ --arg port "$port" \ --arg password "$ticket" \ + --argjson passwordonetimeuseonly true \ '{ status: "success", message: "Console retrieved", @@ -363,6 +364,7 @@ get_node_host() { host: $host, port: $port, password: $password, + passwordonetimeuseonly: $passwordonetimeuseonly, protocol: "vnc" } }' diff --git a/plugins/hypervisors/external/src/main/java/org/apache/cloudstack/hypervisor/external/provisioner/ExternalPathPayloadProvisioner.java b/plugins/hypervisors/external/src/main/java/org/apache/cloudstack/hypervisor/external/provisioner/ExternalPathPayloadProvisioner.java index 5eff5d814d6e..4bfd7fae5c17 100644 --- a/plugins/hypervisors/external/src/main/java/org/apache/cloudstack/hypervisor/external/provisioner/ExternalPathPayloadProvisioner.java +++ b/plugins/hypervisors/external/src/main/java/org/apache/cloudstack/hypervisor/external/provisioner/ExternalPathPayloadProvisioner.java @@ -510,13 +510,15 @@ public GetExternalConsoleAnswer getInstanceConsole(String hostName, String exten String host = consoleObj.has("host") ? consoleObj.get("host").getAsString() : null; Integer port = consoleObj.has("port") ? Integer.valueOf(consoleObj.get("port").getAsString()) : null; String password = consoleObj.has("password") ? consoleObj.get("password").getAsString() : null; + boolean passwordOneTimeUseOnly = consoleObj.has("passwordonetimeuseonly") && + consoleObj.get("passwordonetimeuseonly").getAsBoolean(); String protocol = consoleObj.has("protocol") ? consoleObj.get("protocol").getAsString() : null; if (url == null && ObjectUtils.anyNull(host, port)) { logger.error("Missing required fields in external console output: {}", getSanitizedJsonStringForLog(output)); return new GetExternalConsoleAnswer(cmd, "Missing required fields in output"); } - return new GetExternalConsoleAnswer(cmd, url, host, port, password, protocol); + return new GetExternalConsoleAnswer(cmd, url, host, port, password, passwordOneTimeUseOnly, protocol); } catch (RuntimeException e) { logger.error("Failed to parse output for getInstanceConsole: {}", e.getMessage(), e); return new GetExternalConsoleAnswer(cmd, "Failed to parse output"); diff --git a/server/src/main/java/com/cloud/servlet/ConsoleProxyClientParam.java b/server/src/main/java/com/cloud/servlet/ConsoleProxyClientParam.java index b416ab98288b..b923d71bfd18 100644 --- a/server/src/main/java/com/cloud/servlet/ConsoleProxyClientParam.java +++ b/server/src/main/java/com/cloud/servlet/ConsoleProxyClientParam.java @@ -34,6 +34,8 @@ public class ConsoleProxyClientParam { private String username; private String password; + private boolean sessionRequiresNewViewer = false; + /** * IP that has generated the console endpoint */ @@ -218,4 +220,8 @@ public String getClientIp() { public void setClientIp(String clientIp) { this.clientIp = clientIp; } + + public void setSessionRequiresNewViewer(boolean sessionRequiresNewViewer) { + this.sessionRequiresNewViewer = sessionRequiresNewViewer; + } } diff --git a/server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java b/server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java index d41b388fbe66..5031b61cf0db 100644 --- a/server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java @@ -449,6 +449,7 @@ protected ConsoleConnectionDetails getConsoleConnectionDetailsForExternalVm(Cons } if (StringUtils.isNotBlank(getExternalConsoleAnswer.getPassword())) { details.setSid(getExternalConsoleAnswer.getPassword()); + details.setSessionRequiresNewViewer(getExternalConsoleAnswer.isPasswordOneTimeUseOnly()); } return details; } @@ -600,6 +601,7 @@ protected ConsoleProxyClientParam generateConsoleProxyClientParam(ConsoleConnect param.setTicket(ticket); param.setSessionUuid(sessionUuid); param.setSourceIP(addr); + param.setSessionRequiresNewViewer(details.isSessionRequiresNewViewer()); if (StringUtils.isNotBlank(extraSecurityToken)) { param.setExtraSecurityToken(extraSecurityToken); @@ -761,6 +763,7 @@ public enum Mode { private String tunnelSession = null; private boolean usingRDP; private String directUrl; + private boolean sessionRequiresNewViewer = false; ConsoleConnectionDetails(String sid, String locale, String tag, String displayName) { this.sid = sid; @@ -850,5 +853,13 @@ public String getDirectUrl() { public void setDirectUrl(String directUrl) { this.directUrl = directUrl; } + + public boolean isSessionRequiresNewViewer() { + return sessionRequiresNewViewer; + } + + public void setSessionRequiresNewViewer(boolean sessionRequiresNewViewer) { + this.sessionRequiresNewViewer = sessionRequiresNewViewer; + } } } diff --git a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyClientParam.java b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyClientParam.java index 01c4fa6480e8..a6da576306ee 100644 --- a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyClientParam.java +++ b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyClientParam.java @@ -16,6 +16,8 @@ // under the License. package com.cloud.consoleproxy; +import org.apache.commons.lang3.StringUtils; + /** * * Data object to store parameter info needed by client to connect to its host @@ -39,6 +41,8 @@ public class ConsoleProxyClientParam { private String password; private String websocketUrl; + private boolean sessionRequiresNewViewer; + /** * IP that has generated the console endpoint */ @@ -143,8 +147,12 @@ public void setLocale(String locale) { } public String getClientMapKey() { - if (clientTag != null && !clientTag.isEmpty()) + if (sessionRequiresNewViewer && StringUtils.isNotBlank(sessionUuid)) { + return sessionUuid; + } + if (StringUtils.isNotBlank(clientTag)) { return clientTag; + } return clientHostAddress + ":" + clientHostPort; } @@ -220,4 +228,8 @@ public String getClientIp() { public void setClientIp(String clientIp) { this.clientIp = clientIp; } + + public boolean isSessionRequiresNewViewer() { + return sessionRequiresNewViewer; + } } From c3a0d4689659869b4aab0ffbd556606df48d4ac9 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Thu, 18 Sep 2025 13:57:35 +0530 Subject: [PATCH 09/14] main merge test fix Signed-off-by: Abhishek Kumar --- .../cloudstack/consoleproxy/ConsoleAccessManagerImplTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/test/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImplTest.java b/server/src/test/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImplTest.java index 1a4ef5561107..aca5d8832818 100644 --- a/server/src/test/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImplTest.java +++ b/server/src/test/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImplTest.java @@ -738,7 +738,7 @@ public void composeConsoleAccessEndpointReturnsConsoleEndpointWhenConsoleConnect details.setPort(port); details.setSid(sid); Mockito.doReturn(details).when(consoleAccessManager).getConsoleConnectionDetails(vm, host); - Mockito.when(consoleProxyManager.getVncPort()).thenReturn(vncPort); + Mockito.when(consoleProxyManager.getVncPort(Mockito.anyLong())).thenReturn(vncPort); ConsoleProxyPasswordBasedEncryptor.KeyIVPair keyIvPair = new ConsoleProxyPasswordBasedEncryptor.KeyIVPair("key", "iv"); Mockito.doReturn(GsonHelper.getGson().toJson(keyIvPair)).when(consoleAccessManager).getEncryptorPassword(); Mockito.doReturn(ticket).when(consoleAccessManager).genAccessTicket(hostStr, String.valueOf(port), sid, tag, sessionUuid); From f82ee7d4334abf6872055b91b397f10bc2c71788 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Thu, 18 Sep 2025 15:17:20 +0530 Subject: [PATCH 10/14] fix on cpvm Signed-off-by: Abhishek Kumar --- .../src/main/java/com/cloud/consoleproxy/ConsoleProxy.java | 7 ++++--- .../com/cloud/consoleproxy/ConsoleProxyClientParam.java | 4 ++++ .../cloud/consoleproxy/ConsoleProxyHttpHandlerHelper.java | 3 +++ .../com/cloud/consoleproxy/ConsoleProxyNoVNCHandler.java | 2 ++ 4 files changed, 13 insertions(+), 3 deletions(-) diff --git a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxy.java b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxy.java index 0c46de2a4ac6..a25abac981b9 100644 --- a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxy.java +++ b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxy.java @@ -36,6 +36,8 @@ import org.apache.commons.lang3.ArrayUtils; import org.apache.commons.lang3.StringUtils; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.core.config.Configurator; import org.eclipse.jetty.websocket.api.Session; @@ -43,9 +45,6 @@ import com.google.gson.Gson; import com.sun.net.httpserver.HttpServer; -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; - /** * * ConsoleProxy, singleton class that manages overall activities in console proxy process. To make legacy code work, we still @@ -598,6 +597,8 @@ public static ConsoleProxyNoVncClient getNoVncViewer(ConsoleProxyClientParam par Session session) throws AuthenticationException { boolean reportLoadChange = false; String clientKey = param.getClientMapKey(); + LOGGER.debug("Getting NoVNC viewer for {}. Session requires new viewer: {}, client tag: {}. session UUID: {}", + clientKey, param.isSessionRequiresNewViewer(), param.getClientTag(), param.getSessionUuid()); synchronized (connectionMap) { ConsoleProxyClient viewer = connectionMap.get(clientKey); if (viewer == null || viewer.getClass() != ConsoleProxyNoVncClient.class) { diff --git a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyClientParam.java b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyClientParam.java index a6da576306ee..79c6b8c2a951 100644 --- a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyClientParam.java +++ b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyClientParam.java @@ -232,4 +232,8 @@ public void setClientIp(String clientIp) { public boolean isSessionRequiresNewViewer() { return sessionRequiresNewViewer; } + + public void setSessionRequiresNewViewer(boolean sessionRequiresNewViewer) { + this.sessionRequiresNewViewer = sessionRequiresNewViewer; + } } diff --git a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyHttpHandlerHelper.java b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyHttpHandlerHelper.java index 48ac5f44ff23..483b90db05e5 100644 --- a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyHttpHandlerHelper.java +++ b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyHttpHandlerHelper.java @@ -110,6 +110,9 @@ public static Map getQueryMap(String query) { if (param.getExtraSecurityToken() != null) { map.put("extraSecurityToken", param.getExtraSecurityToken()); } + if (param.isSessionRequiresNewViewer()) { + map.put("sessionRequiresNewViewer", Boolean.TRUE.toString()); + } } else { LOGGER.error("Unable to decode token"); } diff --git a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVNCHandler.java b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVNCHandler.java index a9639d0b32e3..a148b988e40d 100644 --- a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVNCHandler.java +++ b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVNCHandler.java @@ -93,6 +93,7 @@ public void onConnect(final Session session) throws IOException, InterruptedExce String websocketUrl = queryMap.get("websocketUrl"); String sessionUuid = queryMap.get("sessionUuid"); String clientIp = session.getRemoteAddress().getAddress().getHostAddress(); + boolean sessionRequiresNewViewer = Boolean.parseBoolean(queryMap.get("sessionRequiresNewViewer")); if (tag == null) tag = ""; @@ -141,6 +142,7 @@ public void onConnect(final Session session) throws IOException, InterruptedExce param.setSessionUuid(sessionUuid); param.setSourceIP(sourceIP); param.setClientIp(clientIp); + param.setSessionRequiresNewViewer(sessionRequiresNewViewer); if (queryMap.containsKey("extraSecurityToken")) { param.setExtraSecurityToken(queryMap.get("extraSecurityToken")); From 19814b0f0b3e94544660dd1710af8988cb2c441d Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Tue, 23 Sep 2025 16:00:07 +0530 Subject: [PATCH 11/14] console-proxy: close session if unable to connect to VNC server Signed-off-by: Abhishek Kumar --- .../consoleproxy/ConsoleProxyNoVncClient.java | 11 ++++-- .../cloud/consoleproxy/vnc/NoVncClient.java | 5 +-- .../consoleproxy/vnc/network/NioSocket.java | 36 +++++++++---------- 3 files changed, 29 insertions(+), 23 deletions(-) diff --git a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVncClient.java b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVncClient.java index 85a2e5c541f3..36dce8b8554c 100644 --- a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVncClient.java +++ b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVncClient.java @@ -109,7 +109,12 @@ public void run() { String tunnelSession = param.getClientTunnelSession(); String websocketUrl = param.getWebsocketUrl(); - connectClientToVNCServer(tunnelUrl, tunnelSession, websocketUrl); + if (!connectClientToVNCServer(tunnelUrl, tunnelSession, websocketUrl)) { + logger.error("Failed to connect to VNC server, will close connection with client [{}] [IP: {}].", clientId, clientSourceIp); + connectionAlive = false; + session.close(); + return; + } authenticateToVNCServer(clientSourceIp); // Track consecutive iterations with no data and sleep accordingly. Only used for NIO socket connections. @@ -313,7 +318,7 @@ protected static byte[] rewriteServerNameInServerInit(byte[] serverInitBytes, St * - When websocketUrl is not empty -> connect to websocket * - Otherwise -> connect to TCP port on host directly */ - private void connectClientToVNCServer(String tunnelUrl, String tunnelSession, String websocketUrl) { + private boolean connectClientToVNCServer(String tunnelUrl, String tunnelSession, String websocketUrl) { try { if (StringUtils.isNotBlank(websocketUrl)) { logger.info(String.format("Connect to VNC over websocket URL: %s", websocketUrl)); @@ -337,7 +342,9 @@ private void connectClientToVNCServer(String tunnelUrl, String tunnelSession, St logger.info("Connection to VNC server has been established successfully."); } catch (Throwable e) { logger.error("Unexpected exception while connecting to VNC server.", e); + return false; } + return true; } private void setClientParam(ConsoleProxyClientParam param) { diff --git a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/NoVncClient.java b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/NoVncClient.java index 493c22879318..ca7577d2bfcb 100644 --- a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/NoVncClient.java +++ b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/NoVncClient.java @@ -88,15 +88,16 @@ public void connectTo(String host, int port, String path, String session, boolea setTunnelSocketStreams(); } - public void connectTo(String host, int port) { + public void connectTo(String host, int port) throws IOException { // Connect to server logger.info("Connecting to VNC server {}:{} ...", host, port); try { NioSocket nioSocket = new NioSocket(host, port); this.nioSocketConnection = new NioSocketHandlerImpl(nioSocket); - } catch (Exception e) { + } catch (IOException e) { logger.error(String.format("Cannot create socket to host: %s and port %s: %s", host, port, e.getMessage()), e); + throw e; } } diff --git a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/network/NioSocket.java b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/network/NioSocket.java index 9bd2a10e6f07..4ab88ea9fc72 100644 --- a/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/network/NioSocket.java +++ b/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/network/NioSocket.java @@ -36,7 +36,7 @@ public class NioSocket { private static final int CONNECTION_TIMEOUT_MILLIS = 3000; protected Logger logger = LogManager.getLogger(getClass()); - private void initializeSocket() { + private void initializeSocket() throws IOException { try { socketChannel = SocketChannel.open(); socketChannel.configureBlocking(false); @@ -49,30 +49,27 @@ private void initializeSocket() { socketChannel.register(readSelector, SelectionKey.OP_READ); } catch (IOException e) { logger.error("Could not initialize NioSocket: " + e.getMessage(), e); + throw e; } } - private void waitForSocketSelectorConnected(Selector selector) { - try { - while (selector.select(CONNECTION_TIMEOUT_MILLIS) <= 0) { - logger.debug("Waiting for ready operations to connect to the socket"); - } - Set keys = selector.selectedKeys(); - for (SelectionKey selectionKey: keys) { - if (selectionKey.isConnectable()) { - if (socketChannel.isConnectionPending()) { - socketChannel.finishConnect(); - } - logger.debug("Connected to the socket"); - break; + private void waitForSocketSelectorConnected(Selector selector) throws IOException { + while (selector.select(CONNECTION_TIMEOUT_MILLIS) <= 0) { + logger.debug("Waiting for ready operations to connect to the socket"); + } + Set keys = selector.selectedKeys(); + for (SelectionKey selectionKey: keys) { + if (selectionKey.isConnectable()) { + if (socketChannel.isConnectionPending()) { + socketChannel.finishConnect(); } + logger.debug("Connected to the socket"); + break; } - } catch (IOException e) { - logger.error(String.format("Error waiting for socket selector ready: %s", e.getMessage()), e); } } - private void connectSocket(String host, int port) { + private void connectSocket(String host, int port) throws IOException { try { socketChannel.connect(new InetSocketAddress(host, port)); Selector selector = Selector.open(); @@ -80,11 +77,12 @@ private void connectSocket(String host, int port) { waitForSocketSelectorConnected(selector); } catch (IOException e) { - logger.error(String.format("Error creating NioSocket to %s:%s: %s", host, port, e.getMessage()), e); + logger.error("Error connecting NioSocket to {}:{}: {}", host, port, e.getMessage(), e); + throw e; } } - public NioSocket(String host, int port) { + public NioSocket(String host, int port) throws IOException { initializeSocket(); connectSocket(host, port); } From ff4ab2d5c75a9e3664e69b2f7a23117a07860c0a Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Thu, 25 Sep 2025 12:31:00 +0530 Subject: [PATCH 12/14] review changes Signed-off-by: Abhishek Kumar --- .../apache/cloudstack/api/ApiConstants.java | 1 + .../manager/ExtensionsManagerImpl.java | 27 ++++++ .../manager/ExtensionsManagerImplTest.java | 94 ++++++++++++++++++- .../ConsoleAccessManagerImpl.java | 2 +- .../ConsoleAccessManagerImplTest.java | 4 +- 5 files changed, 120 insertions(+), 8 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java index 489d737b5bb9..1d991ac1a904 100644 --- a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java +++ b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java @@ -80,6 +80,7 @@ public class ApiConstants { public static final String BYTES_WRITE_RATE_MAX = "byteswriteratemax"; public static final String BYTES_WRITE_RATE_MAX_LENGTH = "byteswriteratemaxlength"; public static final String BYPASS_VLAN_OVERLAP_CHECK = "bypassvlanoverlapcheck"; + public static final String CALLER = "caller"; public static final String CAPACITY = "capacity"; public static final String CATEGORY = "category"; public static final String CAN_REVERT = "canrevert"; diff --git a/framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImpl.java b/framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImpl.java index 4900a1acf62b..9af5cb69739f 100644 --- a/framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImpl.java +++ b/framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImpl.java @@ -476,6 +476,29 @@ protected void updateAllExtensionHosts(Extension extension, Long clusterId, bool executorService.shutdown(); } + protected Map getCallerDetails() { + Account caller = CallContext.current().getCallingAccount(); + if (caller == null) { + return null; + } + Map callerDetails = new HashMap<>(); + callerDetails.put(ApiConstants.ID, caller.getUuid()); + callerDetails.put(ApiConstants.NAME, caller.getAccountName()); + if (caller.getType() != null) { + callerDetails.put(ApiConstants.TYPE, caller.getType().name()); + } + Role role = roleService.findRole(caller.getRoleId()); + if (role == null) { + return callerDetails; + } + callerDetails.put(ApiConstants.ROLE_ID, role.getUuid()); + callerDetails.put(ApiConstants.ROLE_NAME, role.getName()); + if (role.getRoleType() != null) { + callerDetails.put(ApiConstants.ROLE_TYPE, role.getRoleType().name()); + } + return callerDetails; + } + protected Map> getExternalAccessDetails(Map actionDetails, long hostId, ExtensionResourceMap resourceMap) { Map> externalDetails = new HashMap<>(); @@ -497,6 +520,10 @@ protected Map> getExternalAccessDetails(Map callerDetails = getCallerDetails(); + if (MapUtils.isNotEmpty(callerDetails)) { + externalDetails.put(ApiConstants.CALLER, callerDetails); + } return externalDetails; } diff --git a/framework/extensions/src/test/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImplTest.java b/framework/extensions/src/test/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImplTest.java index 37297654a533..bee597550a07 100644 --- a/framework/extensions/src/test/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImplTest.java +++ b/framework/extensions/src/test/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImplTest.java @@ -121,6 +121,7 @@ import com.cloud.storage.dao.VMTemplateDao; import com.cloud.user.Account; import com.cloud.utils.Pair; +import com.cloud.utils.UuidUtils; import com.cloud.utils.db.EntityManager; import com.cloud.utils.db.SearchBuilder; import com.cloud.utils.db.SearchCriteria; @@ -525,11 +526,15 @@ public void getExternalAccessDetailsReturnsMapWithHostAndExtension() { when(hostDetailsDao.findDetails(hostId)).thenReturn(null); when(extensionResourceMapDetailsDao.listDetailsKeyPairs(2L, true)).thenReturn(Collections.emptyMap()); when(extensionDetailsDao.listDetailsKeyPairs(3L, true)).thenReturn(map); - Map> result = extensionsManager.getExternalAccessDetails(map, hostId, resourceMap); - assertTrue(result.containsKey(ApiConstants.ACTION)); - assertFalse(result.containsKey(ApiConstants.HOST)); - assertFalse(result.containsKey(ApiConstants.RESOURCE_MAP)); - assertTrue(result.containsKey(ApiConstants.EXTENSION)); + try (MockedStatic ignored = mockStatic(CallContext.class)) { + mockCallerRole(RoleType.Admin); + Map> result = extensionsManager.getExternalAccessDetails(map, hostId, resourceMap); + assertTrue(result.containsKey(ApiConstants.ACTION)); + assertFalse(result.containsKey(ApiConstants.HOST)); + assertFalse(result.containsKey(ApiConstants.RESOURCE_MAP)); + assertTrue(result.containsKey(ApiConstants.EXTENSION)); + assertTrue(result.containsKey(ApiConstants.CALLER)); + } } @Test(expected = CloudRuntimeException.class) @@ -1285,9 +1290,14 @@ private void mockCallerRole(RoleType roleType) { CallContext callContextMock = mock(CallContext.class); when(CallContext.current()).thenReturn(callContextMock); Account accountMock = mock(Account.class); + when(accountMock.getAccountName()).thenReturn("testAccount"); + when(accountMock.getUuid()).thenReturn(UUID.randomUUID().toString()); + when(accountMock.getType()).thenReturn(RoleType.Admin.equals(roleType) ? Account.Type.ADMIN : Account.Type.NORMAL); when(accountMock.getRoleId()).thenReturn(1L); Role role = mock(Role.class); when(role.getRoleType()).thenReturn(roleType); + when(role.getUuid()).thenReturn("role-uuid-1"); + when(role.getName()).thenReturn(roleType.name() + "Role"); when(roleService.findRole(1L)).thenReturn(role); when(callContextMock.getCallingAccount()).thenReturn(accountMock); } @@ -1952,4 +1962,78 @@ public void getInstanceConsole_whenAgentManagerFails() { Answer result = extensionsManager.getInstanceConsole(vm, host); assertNull(result); } + + @Test + public void getExternalAccessDetailsReturnsExpectedDetails() { + Host host = mock(Host.class); + when(host.getId()).thenReturn(100L); + when(host.getClusterId()).thenReturn(1L); + Map vmDetails = Map.of("key1", "value1", "key2", "value2"); + ExtensionResourceMapVO resourceMapVO = mock(ExtensionResourceMapVO.class); + when(extensionResourceMapDao.findByResourceIdAndType(1L, ExtensionResourceMap.ResourceType.Cluster)) + .thenReturn(resourceMapVO); + doReturn(new HashMap<>()).when(extensionsManager).getExternalAccessDetails(null, 100L, resourceMapVO); + Map> result = extensionsManager.getExternalAccessDetails(host, vmDetails); + assertNotNull(result); + assertNotNull(result.get(ApiConstants.VIRTUAL_MACHINE)); + assertEquals(vmDetails, result.get(ApiConstants.VIRTUAL_MACHINE)); + } + + @Test + public void getExternalAccessDetailsReturnsExpectedNullDetails() { + Host host = mock(Host.class); + when(host.getId()).thenReturn(101L); + when(host.getClusterId()).thenReturn(1L); + Map vmDetails = null; + ExtensionResourceMapVO resourceMapVO = mock(ExtensionResourceMapVO.class); + when(extensionResourceMapDao.findByResourceIdAndType(1L, ExtensionResourceMap.ResourceType.Cluster)) + .thenReturn(resourceMapVO); + doReturn(new HashMap<>()).when(extensionsManager).getExternalAccessDetails(null, 101L, resourceMapVO); + Map> result = extensionsManager.getExternalAccessDetails(host, vmDetails); + assertNotNull(result); + assertNull(result.get(ApiConstants.VIRTUAL_MACHINE)); + } + + @Test + public void getCallerDetailsReturnsExpectedDetailsForValidCaller() { + try (MockedStatic ignored = mockStatic(CallContext.class)) { + mockCallerRole(RoleType.Admin); + Map result = extensionsManager.getCallerDetails(); + assertNotNull(result); + assertTrue(UuidUtils.isUuid(result.get(ApiConstants.ID))); + assertEquals("testAccount", result.get(ApiConstants.NAME)); + assertEquals("ADMIN", result.get(ApiConstants.TYPE)); + assertEquals("role-uuid-1", result.get(ApiConstants.ROLE_ID)); + assertEquals("AdminRole", result.get(ApiConstants.ROLE_NAME)); + assertEquals("Admin", result.get(ApiConstants.ROLE_TYPE)); + } + } + + @Test + public void getCallerDetailsReturnsNullWhenCallerIsNull() { + CallContext callContext = mock(CallContext.class); + when(callContext.getCallingAccount()).thenReturn(null); + try (MockedStatic mockedCallContext = mockStatic(CallContext.class)) { + mockedCallContext.when(CallContext::current).thenReturn(callContext); + Map result = extensionsManager.getCallerDetails(); + assertNull(result); + } + } + + @Test + public void getCallerDetailsReturnsDetailsWithoutRoleWhenRoleIsNull() { + try (MockedStatic ignored = mockStatic(CallContext.class)) { + mockCallerRole(RoleType.User); + when(roleService.findRole(1L)).thenReturn(null); + Map result = extensionsManager.getCallerDetails(); + assertNotNull(result); + assertTrue(UuidUtils.isUuid(result.get(ApiConstants.ID))); + assertEquals("testAccount", result.get(ApiConstants.NAME)); + assertEquals("NORMAL", result.get(ApiConstants.TYPE)); + assertNull(result.get(ApiConstants.ROLE_ID)); + assertNull(result.get(ApiConstants.ROLE_NAME)); + assertNull(result.get(ApiConstants.ROLE_TYPE)); + } + } + } diff --git a/server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java b/server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java index 22f87540e042..fde937764e7b 100644 --- a/server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java @@ -781,7 +781,7 @@ public void setModeFromExternalProtocol(String protocol) { if (StringUtils.isBlank(protocol)) { return; } - if (List.of("link", "url", "direct").contains(protocol.toLowerCase())) { + if (Mode.Direct.name().toLowerCase().equalsIgnoreCase(protocol)) { this.mode = Mode.Direct; } } diff --git a/server/src/test/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImplTest.java b/server/src/test/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImplTest.java index aca5d8832818..7c9a4208b58f 100644 --- a/server/src/test/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImplTest.java +++ b/server/src/test/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImplTest.java @@ -770,7 +770,7 @@ public void composeConsoleAccessEndpointReturnsWithoutPersistWhenConsoleConnecti String addr = "addr"; ConsoleConnectionDetails details = new ConsoleConnectionDetails("password", "en", "tag", null); details.setDirectUrl(url); - details.setModeFromExternalProtocol("url"); + details.setModeFromExternalProtocol("direct"); VirtualMachine vm = Mockito.mock(VirtualMachine.class); Mockito.when(vm.getId()).thenReturn(vmId); HostVO host = Mockito.mock(HostVO.class); @@ -778,7 +778,7 @@ public void composeConsoleAccessEndpointReturnsWithoutPersistWhenConsoleConnecti Mockito.doReturn(details).when(consoleAccessManager).getConsoleConnectionDetails(vm, host); Mockito.doNothing().when(consoleAccessManager).persistConsoleSession(sessionUuid, vmId, hostId, addr); - ConsoleEndpoint endpoint = consoleAccessManager.composeConsoleAccessEndpoint("url", vm, host, addr, sessionUuid, ""); + ConsoleEndpoint endpoint = consoleAccessManager.composeConsoleAccessEndpoint("rootUrl", vm, host, addr, sessionUuid, ""); Mockito.verify(consoleAccessManager).persistConsoleSession(sessionUuid, vmId, hostId, addr); Mockito.verify(managementServer, Mockito.never()).setConsoleAccessForVm(Mockito.anyLong(), Mockito.anyString()); From e06cbf9db0888da365a81a5c8ec724ad6d0317f3 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Thu, 25 Sep 2025 12:50:05 +0530 Subject: [PATCH 13/14] fix test Signed-off-by: Abhishek Kumar --- .../cloudstack/consoleproxy/ConsoleAccessManagerImplTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/test/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImplTest.java b/server/src/test/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImplTest.java index 7c9a4208b58f..30aec5ebb874 100644 --- a/server/src/test/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImplTest.java +++ b/server/src/test/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImplTest.java @@ -413,7 +413,7 @@ public void getConsoleConnectionDetailsForExternalVmSetsDetailsWhenAnswerIsValid Mockito.when(answer.getResult()).thenReturn(true); Mockito.when(answer.getUrl()).thenReturn(url); - Mockito.when(answer.getProtocol()).thenReturn("url"); + Mockito.when(answer.getProtocol()).thenReturn("direct"); Mockito.when(managementServer.getExternalVmConsole(vm, host)).thenReturn(answer); ConsoleConnectionDetails result = consoleAccessManager.getConsoleConnectionDetailsForExternalVm(details, vm, host); From 1e911a912371ad6e1542d0516deea99b3d8f934f Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Thu, 25 Sep 2025 16:00:39 +0530 Subject: [PATCH 14/14] change Signed-off-by: Abhishek Kumar --- .../ExternalPathPayloadProvisioner.java | 4 ++++ .../ExternalPathPayloadProvisionerTest.java | 15 +++++++++++++++ .../ConsoleAccessManagerImplTest.java | 2 +- 3 files changed, 20 insertions(+), 1 deletion(-) diff --git a/plugins/hypervisors/external/src/main/java/org/apache/cloudstack/hypervisor/external/provisioner/ExternalPathPayloadProvisioner.java b/plugins/hypervisors/external/src/main/java/org/apache/cloudstack/hypervisor/external/provisioner/ExternalPathPayloadProvisioner.java index 4bfd7fae5c17..6cec5181de63 100644 --- a/plugins/hypervisors/external/src/main/java/org/apache/cloudstack/hypervisor/external/provisioner/ExternalPathPayloadProvisioner.java +++ b/plugins/hypervisors/external/src/main/java/org/apache/cloudstack/hypervisor/external/provisioner/ExternalPathPayloadProvisioner.java @@ -138,6 +138,10 @@ public String getName() { protected Map loadAccessDetails(Map> externalDetails, VirtualMachineTO virtualMachineTO) { Map modifiedDetails = new HashMap<>(); + if (MapUtils.isNotEmpty(externalDetails) && externalDetails.containsKey(ApiConstants.CALLER)) { + modifiedDetails.put(ApiConstants.CALLER, externalDetails.get(ApiConstants.CALLER)); + externalDetails.remove(ApiConstants.CALLER); + } if (MapUtils.isNotEmpty(externalDetails)) { modifiedDetails.put(ApiConstants.EXTERNAL_DETAILS, externalDetails); } diff --git a/plugins/hypervisors/external/src/test/java/org/apache/cloudstack/hypervisor/external/provisioner/ExternalPathPayloadProvisionerTest.java b/plugins/hypervisors/external/src/test/java/org/apache/cloudstack/hypervisor/external/provisioner/ExternalPathPayloadProvisionerTest.java index 3c99fcd03683..d0a396f7a94c 100644 --- a/plugins/hypervisors/external/src/test/java/org/apache/cloudstack/hypervisor/external/provisioner/ExternalPathPayloadProvisionerTest.java +++ b/plugins/hypervisors/external/src/test/java/org/apache/cloudstack/hypervisor/external/provisioner/ExternalPathPayloadProvisionerTest.java @@ -92,6 +92,7 @@ import com.cloud.vm.UserVmVO; import com.cloud.vm.VirtualMachine; import com.cloud.vm.VirtualMachineProfile; +import com.cloud.vm.VmDetailConstants; import com.cloud.vm.dao.UserVmDao; @RunWith(MockitoJUnitRunner.class) @@ -205,6 +206,20 @@ public void testLoadAccessDetailsWithNullExternalDetails() { assertEquals("test-vm", result.get(ApiConstants.VIRTUAL_MACHINE_NAME)); } + @Test + public void testLoadAccessDetails_WithCaller() { + Map> externalDetails = new HashMap<>(); + externalDetails.put(ApiConstants.EXTENSION, Map.of("key1", "value1")); + externalDetails.put(ApiConstants.CALLER, Map.of("key2", "value2")); + Map result = provisioner.loadAccessDetails(externalDetails, null); + + assertNotNull(result); + assertNotNull(result.get(ApiConstants.EXTERNAL_DETAILS)); + assertNotNull(((Map) result.get(ApiConstants.EXTERNAL_DETAILS)).get(ApiConstants.EXTENSION)); + assertNotNull(result.get(ApiConstants.CALLER)); + assertNull(result.get(VmDetailConstants.CLOUDSTACK_VM_DETAILS)); + } + @Test public void testGetExtensionCheckedPathValidFile() { String result = provisioner.getExtensionCheckedPath("test-extension", "test-extension.sh"); diff --git a/server/src/test/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImplTest.java b/server/src/test/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImplTest.java index 30aec5ebb874..97e6295da1a5 100644 --- a/server/src/test/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImplTest.java +++ b/server/src/test/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImplTest.java @@ -413,7 +413,7 @@ public void getConsoleConnectionDetailsForExternalVmSetsDetailsWhenAnswerIsValid Mockito.when(answer.getResult()).thenReturn(true); Mockito.when(answer.getUrl()).thenReturn(url); - Mockito.when(answer.getProtocol()).thenReturn("direct"); + Mockito.when(answer.getProtocol()).thenReturn(ConsoleConnectionDetails.Mode.Direct.name()); Mockito.when(managementServer.getExternalVmConsole(vm, host)).thenReturn(answer); ConsoleConnectionDetails result = consoleAccessManager.getConsoleConnectionDetailsForExternalVm(details, vm, host);