From c9749cf1d591751b919a14a5c5061e6a80992a9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Jandre?= <48719461+JoaoJandre@users.noreply.github.com> Date: Fri, 27 Jan 2023 16:57:37 -0300 Subject: [PATCH 1/5] Fix log bugs and vulnerabilities --- .../resource/LibvirtComputingResource.java | 2 +- .../kvm/storage/LibvirtStorageAdaptor.java | 2 +- .../cloud/ovm/hypervisor/OvmResourceBase.java | 2 +- .../vmware/resource/VmwareResource.java | 5 + .../resource/XenServerConnectionPool.java | 2 +- .../cloud/alert/ConsoleProxyAlertAdapter.java | 170 +++++++++-------- .../alert/SecondaryStorageVmAlertAdapter.java | 178 +++++++++--------- .../main/java/com/cloud/api/ApiServer.java | 6 +- ...ExternalLoadBalancerDeviceManagerImpl.java | 8 +- .../cloud/servlet/ConsoleProxyServlet.java | 6 +- .../java/com/cloud/vm/UserVmManagerImpl.java | 2 +- .../OutOfBandManagementServiceImpl.java | 2 +- .../resource/NfsSecondaryStorageResource.java | 2 +- 13 files changed, 205 insertions(+), 182 deletions(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java index 11f19142833f..9a5228af8ee9 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java @@ -1706,7 +1706,7 @@ private boolean checkOvsNetwork(final String networkName) { } public boolean passCmdLine(final String vmName, final String cmdLine) throws InternalErrorException { - final Script command = new Script(_patchScriptPath, 300 * 1000, s_logger); + final Script command = new Script(_patchScriptPath, 300000, s_logger); String result; command.add("-n", vmName); command.add("-c", cmdLine); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java index 4f228ac9e2da..46d64b1d4bdc 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java @@ -1427,7 +1427,7 @@ public KVMPhysicalDisk copyPhysicalDisk(KVMPhysicalDisk disk, String name, KVMSt r.ioCtxDestroy(io); } catch (QemuImgException | LibvirtException e) { - s_logger.error("Failed to convert from " + srcFile.getFileName() + " to " + destFile.getFileName() + " the error was: " + e.getMessage()); + s_logger.error(String.format("Failed to convert from %s to %s the error was: " + e.getMessage(), srcFile != null ? srcFile.getFileName() : null, destFile != null ? destFile.getFileName() : null)); newDisk = null; } catch (RadosException e) { s_logger.error("A Ceph RADOS operation failed (" + e.getReturnValue() + "). The error was: " + e.getMessage()); diff --git a/plugins/hypervisors/ovm/src/main/java/com/cloud/ovm/hypervisor/OvmResourceBase.java b/plugins/hypervisors/ovm/src/main/java/com/cloud/ovm/hypervisor/OvmResourceBase.java index f24783190d46..cf2f1fbed64d 100644 --- a/plugins/hypervisors/ovm/src/main/java/com/cloud/ovm/hypervisor/OvmResourceBase.java +++ b/plugins/hypervisors/ovm/src/main/java/com/cloud/ovm/hypervisor/OvmResourceBase.java @@ -258,7 +258,7 @@ public boolean configure(String name, Map params) throws Configu _canBridgeFirewall = false; - s_logger.debug(_canBridgeFirewall ? "OVM host supports security groups." : "OVM host doesn't support security groups."); + s_logger.debug("OVM host doesn't support security groups."); return true; } diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java index 5be98d4c43ca..aa26f63b3a96 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java @@ -7541,6 +7541,11 @@ private List relocateVirtualMachine(final VmwareHypervisorHost h // prepare network on the host prepareNetworkFromNicInfo((HostMO)targetHyperHost, nic, false, vmTo.getType()); } + + if (targetHyperHost == null) { + throw new CloudRuntimeException(String.format("Trying to relocate VM [%s], but target hyper host is null.", vmTo.getUuid())); + } + // Ensure secondary storage mounted on target host VmwareManager mgr = targetHyperHost.getContext().getStockObject(VmwareManager.CONTEXT_STOCK_NAME); Pair secStoreUrlAndId = mgr.getSecondaryStorageStoreUrlAndId(Long.parseLong(_dcId)); diff --git a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/XenServerConnectionPool.java b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/XenServerConnectionPool.java index 9bc8d9e8bf05..5d7f3da5c43d 100644 --- a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/XenServerConnectionPool.java +++ b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/XenServerConnectionPool.java @@ -210,7 +210,7 @@ public Connection connect(String hostUuid, String poolUuid, String ipAddress, throw e; } catch (Exception e) { if (s_logger.isDebugEnabled()) { - s_logger.debug("connect through IP(" + mConn.getIp() + " for pool(" + poolUuid + ") is broken due to " + e.toString()); + s_logger.debug("connect through IP(" + (mConn != null ? mConn.getIp() : null) + ") for pool(" + poolUuid + ") is broken due to " + e.toString()); } removeConnect(poolUuid); mConn = null; diff --git a/server/src/main/java/com/cloud/alert/ConsoleProxyAlertAdapter.java b/server/src/main/java/com/cloud/alert/ConsoleProxyAlertAdapter.java index 72dd0d7fb0bb..2d209e546dc7 100644 --- a/server/src/main/java/com/cloud/alert/ConsoleProxyAlertAdapter.java +++ b/server/src/main/java/com/cloud/alert/ConsoleProxyAlertAdapter.java @@ -60,87 +60,97 @@ public void onProxyAlert(Object sender, ConsoleProxyAlertEventArgs args) { throw new CloudRuntimeException("Invalid alert arguments, proxy must be set"); } + String proxyHostName = ""; + String proxyPublicIpAddress = ""; + String proxyPrivateIpAddress = "N/A"; + Long proxyPodIdToDeployIn = null; + + if (proxy != null) { + proxyHostName = proxy.getHostName(); + proxyPublicIpAddress = proxy.getPublicIpAddress(); + proxyPrivateIpAddress = proxy.getPrivateIpAddress() == null ? "N/A" : proxy.getPrivateIpAddress(); + proxyPodIdToDeployIn = proxy.getPodIdToDeployIn(); + } + switch (args.getType()) { - case ConsoleProxyAlertEventArgs.PROXY_CREATED: - if (s_logger.isDebugEnabled()) - s_logger.debug("New console proxy created, zone: " + dc.getName() + ", proxy: " + proxy.getHostName() + ", public IP: " + proxy.getPublicIpAddress() + - ", private IP: " + proxy.getPrivateIpAddress()); - break; - - case ConsoleProxyAlertEventArgs.PROXY_UP: - if (s_logger.isDebugEnabled()) - s_logger.debug("Console proxy is up, zone: " + dc.getName() + ", proxy: " + proxy.getHostName() + ", public IP: " + proxy.getPublicIpAddress() + - ", private IP: " + proxy.getPrivateIpAddress()); - - _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_CONSOLE_PROXY, args.getZoneId(), proxy.getPodIdToDeployIn(), - "Console proxy up in zone: " + dc.getName() + - ", proxy: " + proxy.getHostName() + ", public IP: " + proxy.getPublicIpAddress() + ", private IP: " + - (proxy.getPrivateIpAddress() == null ? "N/A" : proxy.getPrivateIpAddress()), "Console proxy up (zone " + dc.getName() + ")"); - break; - - case ConsoleProxyAlertEventArgs.PROXY_DOWN: - if (s_logger.isDebugEnabled()) - s_logger.debug("Console proxy is down, zone: " + dc.getName() + ", proxy: " + proxy.getHostName() + ", public IP: " + proxy.getPublicIpAddress() + - ", private IP: " + (proxy.getPrivateIpAddress() == null ? "N/A" : proxy.getPrivateIpAddress())); - - _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_CONSOLE_PROXY, args.getZoneId(), proxy.getPodIdToDeployIn(), - "Console proxy down in zone: " + dc.getName() + - ", proxy: " + proxy.getHostName() + ", public IP: " + proxy.getPublicIpAddress() + ", private IP: " + - (proxy.getPrivateIpAddress() == null ? "N/A" : proxy.getPrivateIpAddress()), "Console proxy down (zone " + dc.getName() + ")"); - break; - - case ConsoleProxyAlertEventArgs.PROXY_REBOOTED: - if (s_logger.isDebugEnabled()) - s_logger.debug("Console proxy is rebooted, zone: " + dc.getName() + ", proxy: " + proxy.getHostName() + ", public IP: " + proxy.getPublicIpAddress() + - ", private IP: " + (proxy.getPrivateIpAddress() == null ? "N/A" : proxy.getPrivateIpAddress())); - - _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_CONSOLE_PROXY, args.getZoneId(), proxy.getPodIdToDeployIn(), - "Console proxy rebooted in zone: " + dc.getName() + ", proxy: " + proxy.getHostName() + ", public IP: " + proxy.getPublicIpAddress() + - ", private IP: " + (proxy.getPrivateIpAddress() == null ? "N/A" : proxy.getPrivateIpAddress()), "Console proxy rebooted (zone " + dc.getName() + - ")"); - break; - - case ConsoleProxyAlertEventArgs.PROXY_CREATE_FAILURE: - if (s_logger.isDebugEnabled()) - s_logger.debug("Console proxy creation failure, zone: " + dc.getName()); - _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_CONSOLE_PROXY, args.getZoneId(), null, - "Console proxy creation failure. zone: " + dc.getName() + ", error details: " + args.getMessage(), - "Console proxy creation failure (zone " + dc.getName() + ")"); - break; - - case ConsoleProxyAlertEventArgs.PROXY_START_FAILURE: - if (s_logger.isDebugEnabled()) - s_logger.debug("Console proxy startup failure, zone: " + dc.getName() + ", proxy: " + proxy.getHostName() + ", public IP: " + - proxy.getPublicIpAddress() + ", private IP: " + (proxy.getPrivateIpAddress() == null ? "N/A" : proxy.getPrivateIpAddress())); - - _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_CONSOLE_PROXY, args.getZoneId(), proxy.getPodIdToDeployIn(), - "Console proxy startup failure. zone: " + dc.getName() + ", proxy: " + proxy.getHostName() + ", public IP: " + proxy.getPublicIpAddress() + - ", private IP: " + (proxy.getPrivateIpAddress() == null ? "N/A" : proxy.getPrivateIpAddress()) + ", error details: " + args.getMessage(), - "Console proxy startup failure (zone " + dc.getName() + ")"); - break; - - case ConsoleProxyAlertEventArgs.PROXY_FIREWALL_ALERT: - if (s_logger.isDebugEnabled()) - s_logger.debug("Console proxy firewall alert, zone: " + dc.getName() + ", proxy: " + proxy.getHostName() + ", public IP: " + - proxy.getPublicIpAddress() + ", private IP: " + (proxy.getPrivateIpAddress() == null ? "N/A" : proxy.getPrivateIpAddress())); - - _alertMgr.sendAlert( - AlertManager.AlertType.ALERT_TYPE_CONSOLE_PROXY, - args.getZoneId(), - proxy.getPodIdToDeployIn(), - "Failed to open console proxy firewall port. zone: " + dc.getName() + ", proxy: " + proxy.getHostName() + ", public IP: " + - proxy.getPublicIpAddress() + ", private IP: " + (proxy.getPrivateIpAddress() == null ? "N/A" : proxy.getPrivateIpAddress()), - "Console proxy alert (zone " + dc.getName() + ")"); - break; - - case ConsoleProxyAlertEventArgs.PROXY_STORAGE_ALERT: - if (s_logger.isDebugEnabled()) - s_logger.debug("Console proxy storage alert, zone: " + dc.getName() + ", proxy: " + proxy.getHostName() + ", public IP: " + - proxy.getPublicIpAddress() + ", private IP: " + proxy.getPrivateIpAddress() + ", message: " + args.getMessage()); - - _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_STORAGE_MISC, args.getZoneId(), proxy.getPodIdToDeployIn(), - "Console proxy storage issue. zone: " + dc.getName() + ", message: " + args.getMessage(), "Console proxy alert (zone " + dc.getName() + ")"); - break; + case ConsoleProxyAlertEventArgs.PROXY_CREATED: + if (s_logger.isDebugEnabled()) { + s_logger.debug("New console proxy created, zone: " + dc.getName() + ", proxy: " + proxyHostName + ", public IP: " + proxyPublicIpAddress + ", private IP: " + + proxyPrivateIpAddress); + } + break; + + case ConsoleProxyAlertEventArgs.PROXY_UP: + if (s_logger.isDebugEnabled()) { + s_logger.debug("Console proxy is up, zone: " + dc.getName() + ", proxy: " + proxyHostName + ", public IP: " + proxyPublicIpAddress + ", private IP: " + proxyPrivateIpAddress); + } + + _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_CONSOLE_PROXY, args.getZoneId(), proxyPodIdToDeployIn, "Console proxy up in zone: " + dc.getName() + + ", proxy: " + proxyHostName + ", public IP: " + proxyPublicIpAddress + ", private IP: " + proxyPrivateIpAddress, "Console proxy up (zone " + dc.getName() + + ")"); + break; + + case ConsoleProxyAlertEventArgs.PROXY_DOWN: + if (s_logger.isDebugEnabled()) { + s_logger.debug("Console proxy is down, zone: " + dc.getName() + ", proxy: " + proxyHostName + ", public IP: " + proxyPublicIpAddress + ", private IP: " + + proxyPrivateIpAddress); + } + + _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_CONSOLE_PROXY, args.getZoneId(), proxyPodIdToDeployIn, "Console proxy down in zone: " + dc.getName() + + ", proxy: " + proxyHostName + ", public IP: " + proxyPublicIpAddress + ", private IP: " + proxyPrivateIpAddress, "Console proxy down (zone " + dc.getName() + + ")"); + break; + + case ConsoleProxyAlertEventArgs.PROXY_REBOOTED: + if (s_logger.isDebugEnabled()) { + s_logger.debug("Console proxy is rebooted, zone: " + dc.getName() + ", proxy: " + proxyHostName + ", public IP: " + proxyPublicIpAddress + ", private IP: " + + proxyPrivateIpAddress); + } + + _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_CONSOLE_PROXY, args.getZoneId(), proxyPodIdToDeployIn, "Console proxy rebooted in zone: " + dc.getName() + + ", proxy: " + proxyHostName + ", public IP: " + proxyPublicIpAddress + ", private IP: " + proxyPrivateIpAddress, "Console proxy rebooted (zone " + + dc.getName() + ")"); + break; + + case ConsoleProxyAlertEventArgs.PROXY_CREATE_FAILURE: + if (s_logger.isDebugEnabled()) { + s_logger.debug("Console proxy creation failure, zone: " + dc.getName()); + } + + _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_CONSOLE_PROXY, args.getZoneId(), null, "Console proxy creation failure. zone: " + dc.getName() + + ", error details: " + args.getMessage(), "Console proxy creation failure (zone " + dc.getName() + ")"); + break; + + case ConsoleProxyAlertEventArgs.PROXY_START_FAILURE: + if (s_logger.isDebugEnabled()) { + s_logger.debug("Console proxy startup failure, zone: " + dc.getName() + ", proxy: " + proxyHostName + ", public IP: " + proxyPublicIpAddress + ", private IP: " + + proxyPrivateIpAddress); + } + + _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_CONSOLE_PROXY, args.getZoneId(), proxyPodIdToDeployIn, "Console proxy startup failure. zone: " + dc.getName() + + ", proxy: " + proxyHostName + ", public IP: " + proxyPublicIpAddress + ", private IP: " + proxyPrivateIpAddress + ", error details: " + + args.getMessage(), "Console proxy startup failure (zone " + dc.getName() + ")"); + break; + + case ConsoleProxyAlertEventArgs.PROXY_FIREWALL_ALERT: + if (s_logger.isDebugEnabled()) { + s_logger.debug("Console proxy firewall alert, zone: " + dc.getName() + ", proxy: " + proxyHostName + ", public IP: " + proxyPublicIpAddress + ", private IP: " + + proxyPrivateIpAddress); + } + + _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_CONSOLE_PROXY, args.getZoneId(), proxyPodIdToDeployIn, "Failed to open console proxy firewall port. zone: " + + dc.getName() + ", proxy: " + proxyHostName + ", public IP: " + proxyPublicIpAddress + ", private IP: " + proxyPrivateIpAddress, "Console proxy alert" + + " (zone " + dc.getName() + ")"); + break; + + case ConsoleProxyAlertEventArgs.PROXY_STORAGE_ALERT: + if (s_logger.isDebugEnabled()) { + s_logger.debug("Console proxy storage alert, zone: " + dc.getName() + ", proxy: " + proxyHostName + ", public IP: " + proxyPublicIpAddress + ", private IP: " + + proxyPrivateIpAddress + ", message: " + args.getMessage()); + } + _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_STORAGE_MISC, args.getZoneId(), proxyPodIdToDeployIn, "Console proxy storage issue. zone: " + dc.getName() + + ", message: " + args.getMessage(), "Console proxy alert (zone " + dc.getName() + ")"); + break; } } diff --git a/server/src/main/java/com/cloud/alert/SecondaryStorageVmAlertAdapter.java b/server/src/main/java/com/cloud/alert/SecondaryStorageVmAlertAdapter.java index c29b982a5c44..33f817a04fb9 100644 --- a/server/src/main/java/com/cloud/alert/SecondaryStorageVmAlertAdapter.java +++ b/server/src/main/java/com/cloud/alert/SecondaryStorageVmAlertAdapter.java @@ -59,92 +59,100 @@ public void onSSVMAlert(Object sender, SecStorageVmAlertEventArgs args) { throw new CloudRuntimeException("Invalid alert arguments, secStorageVm must be set"); } + String secStorageVmHostName = ""; + String secStorageVmPublicIpAddress = ""; + String secStorageVmPrivateIpAddress = "N/A"; + Long secStorageVmPodIdToDeployIn = null; + + if (secStorageVm != null) { + secStorageVmHostName = secStorageVm.getHostName(); + secStorageVmPublicIpAddress = secStorageVm.getPublicIpAddress(); + secStorageVmPrivateIpAddress = secStorageVm.getPrivateIpAddress() == null ? "N/A" : secStorageVm.getPrivateIpAddress(); + secStorageVmPodIdToDeployIn = secStorageVm.getPodIdToDeployIn(); + } + switch (args.getType()) { - case SecStorageVmAlertEventArgs.SSVM_CREATED: - if (s_logger.isDebugEnabled()) - s_logger.debug("New secondary storage vm created, zone: " + dc.getName() + ", secStorageVm: " + secStorageVm.getHostName() + ", public IP: " + - secStorageVm.getPublicIpAddress() + ", private IP: " + secStorageVm.getPrivateIpAddress()); - break; - - case SecStorageVmAlertEventArgs.SSVM_UP: - if (s_logger.isDebugEnabled()) - s_logger.debug("Secondary Storage Vm is up, zone: " + dc.getName() + ", secStorageVm: " + secStorageVm.getHostName() + ", public IP: " + - secStorageVm.getPublicIpAddress() + ", private IP: " + secStorageVm.getPrivateIpAddress()); - - _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_SSVM, args.getZoneId(), secStorageVm.getPodIdToDeployIn(), "Secondary Storage Vm up in zone: " + - dc.getName() + ", secStorageVm: " + secStorageVm.getHostName() + ", public IP: " + secStorageVm.getPublicIpAddress() + ", private IP: " + - (secStorageVm.getPrivateIpAddress() == null ? "N/A" : secStorageVm.getPrivateIpAddress()), "Secondary Storage Vm up (zone " + dc.getName() + ")"); - break; - - case SecStorageVmAlertEventArgs.SSVM_DOWN: - if (s_logger.isDebugEnabled()) - s_logger.debug("Secondary Storage Vm is down, zone: " + dc.getName() + ", secStorageVm: " + secStorageVm.getHostName() + ", public IP: " + - secStorageVm.getPublicIpAddress() + ", private IP: " + (secStorageVm.getPrivateIpAddress() == null ? "N/A" : secStorageVm.getPrivateIpAddress())); - - _alertMgr.sendAlert( - AlertManager.AlertType.ALERT_TYPE_SSVM, - args.getZoneId(), - secStorageVm.getPodIdToDeployIn(), - "Secondary Storage Vm down in zone: " + dc.getName() + ", secStorageVm: " + secStorageVm.getHostName() + ", public IP: " + - secStorageVm.getPublicIpAddress() + ", private IP: " + (secStorageVm.getPrivateIpAddress() == null ? "N/A" : secStorageVm.getPrivateIpAddress()), - "Secondary Storage Vm down (zone " + dc.getName() + ")"); - break; - - case SecStorageVmAlertEventArgs.SSVM_REBOOTED: - if (s_logger.isDebugEnabled()) - s_logger.debug("Secondary Storage Vm is rebooted, zone: " + dc.getName() + ", secStorageVm: " + secStorageVm.getHostName() + ", public IP: " + - secStorageVm.getPublicIpAddress() + ", private IP: " + (secStorageVm.getPrivateIpAddress() == null ? "N/A" : secStorageVm.getPrivateIpAddress())); - - _alertMgr.sendAlert( - AlertManager.AlertType.ALERT_TYPE_SSVM, - args.getZoneId(), - secStorageVm.getPodIdToDeployIn(), - "Secondary Storage Vm rebooted in zone: " + dc.getName() + ", secStorageVm: " + secStorageVm.getHostName() + ", public IP: " + - secStorageVm.getPublicIpAddress() + ", private IP: " + (secStorageVm.getPrivateIpAddress() == null ? "N/A" : secStorageVm.getPrivateIpAddress()), - "Secondary Storage Vm rebooted (zone " + dc.getName() + ")"); - break; - - case SecStorageVmAlertEventArgs.SSVM_CREATE_FAILURE: - if (s_logger.isDebugEnabled()) - s_logger.debug("Secondary Storage Vm creation failure, zone: " + dc.getName()); - - _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_SSVM, args.getZoneId(), null, - "Secondary Storage Vm creation failure. zone: " + dc.getName() + ", error details: " + args.getMessage(), - "Secondary Storage Vm creation failure (zone " + dc.getName() + ")"); - break; - - case SecStorageVmAlertEventArgs.SSVM_START_FAILURE: - if (s_logger.isDebugEnabled()) - s_logger.debug("Secondary Storage Vm startup failure, zone: " + dc.getName() + ", secStorageVm: " + secStorageVm.getHostName() + ", public IP: " + - secStorageVm.getPublicIpAddress() + ", private IP: " + (secStorageVm.getPrivateIpAddress() == null ? "N/A" : secStorageVm.getPrivateIpAddress())); - - _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_SSVM, args.getZoneId(), secStorageVm.getPodIdToDeployIn(), - "Secondary Storage Vm startup failure. zone: " + - dc.getName() + ", secStorageVm: " + secStorageVm.getHostName() + ", public IP: " + secStorageVm.getPublicIpAddress() + ", private IP: " + - (secStorageVm.getPrivateIpAddress() == null ? "N/A" : secStorageVm.getPrivateIpAddress()) + ", error details: " + args.getMessage(), - "Secondary Storage Vm startup failure (zone " + dc.getName() + ")"); - break; - - case SecStorageVmAlertEventArgs.SSVM_FIREWALL_ALERT: - if (s_logger.isDebugEnabled()) - s_logger.debug("Secondary Storage Vm firewall alert, zone: " + dc.getName() + ", secStorageVm: " + secStorageVm.getHostName() + ", public IP: " + - secStorageVm.getPublicIpAddress() + ", private IP: " + (secStorageVm.getPrivateIpAddress() == null ? "N/A" : secStorageVm.getPrivateIpAddress())); - - _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_SSVM, args.getZoneId(), secStorageVm.getPodIdToDeployIn(), - "Failed to open secondary storage vm firewall port. zone: " + dc.getName() + ", secStorageVm: " + secStorageVm.getHostName() + ", public IP: " + - secStorageVm.getPublicIpAddress() + ", private IP: " + (secStorageVm.getPrivateIpAddress() == null ? "N/A" : secStorageVm.getPrivateIpAddress()), - "Secondary Storage Vm alert (zone " + dc.getName() + ")"); - break; - - case SecStorageVmAlertEventArgs.SSVM_STORAGE_ALERT: - if (s_logger.isDebugEnabled()) - s_logger.debug("Secondary Storage Vm storage alert, zone: " + dc.getName() + ", secStorageVm: " + secStorageVm.getHostName() + ", public IP: " + - secStorageVm.getPublicIpAddress() + ", private IP: " + secStorageVm.getPrivateIpAddress() + ", message: " + args.getMessage()); - - _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_STORAGE_MISC, args.getZoneId(), secStorageVm.getPodIdToDeployIn(), - "Secondary Storage Vm storage issue. zone: " + dc.getName() + ", message: " + args.getMessage(), "Secondary Storage Vm alert (zone " + dc.getName() + - ")"); - break; + case SecStorageVmAlertEventArgs.SSVM_CREATED: + if (s_logger.isDebugEnabled()) { + s_logger.debug("New secondary storage vm created, zone: " + dc.getName() + ", secStorageVm: " + secStorageVmHostName + ", public IP: " + + secStorageVmPublicIpAddress + ", private IP: " + secStorageVmPrivateIpAddress); + } + break; + + case SecStorageVmAlertEventArgs.SSVM_UP: + if (s_logger.isDebugEnabled()) { + s_logger.debug("Secondary Storage Vm is up, zone: " + dc.getName() + ", secStorageVm: " + secStorageVmHostName + ", public IP: " + secStorageVmPublicIpAddress + + ", private IP: " + secStorageVmPrivateIpAddress); + } + + _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_SSVM, args.getZoneId(), secStorageVmPodIdToDeployIn, "Secondary Storage Vm up in zone: " + + dc.getName() + ", secStorageVm: " + secStorageVmHostName + ", public IP: " + secStorageVmPublicIpAddress + ", private IP: " + secStorageVmPrivateIpAddress, + "Secondary Storage Vm up (zone " + dc.getName() + ")"); + break; + + case SecStorageVmAlertEventArgs.SSVM_DOWN: + if (s_logger.isDebugEnabled()) { + s_logger.debug("Secondary Storage Vm is down, zone: " + dc.getName() + ", secStorageVm: " + secStorageVmHostName + ", public IP: " + secStorageVmPublicIpAddress + + ", private IP: " + secStorageVmPrivateIpAddress); + } + + _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_SSVM, args.getZoneId(), secStorageVmPodIdToDeployIn, "Secondary Storage Vm down in zone: " + + dc.getName() + ", secStorageVm: " + secStorageVmHostName + ", public IP: " + secStorageVmPublicIpAddress + ", private IP: " + secStorageVmPrivateIpAddress, + "Secondary Storage Vm down (zone " + dc.getName() + ")"); + break; + + case SecStorageVmAlertEventArgs.SSVM_REBOOTED: + if (s_logger.isDebugEnabled()) { + s_logger.debug("Secondary Storage Vm is rebooted, zone: " + dc.getName() + ", secStorageVm: " + secStorageVmHostName + ", public IP: " + + secStorageVmPublicIpAddress + ", private IP: " + secStorageVmPrivateIpAddress); + } + + _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_SSVM, args.getZoneId(), secStorageVmPodIdToDeployIn, "Secondary Storage Vm rebooted in zone: " + dc.getName() + + ", secStorageVm: " + secStorageVmHostName + ", public IP: " + secStorageVmPublicIpAddress + ", private IP: " + secStorageVmPrivateIpAddress, + "Secondary Storage Vm rebooted (zone " + dc.getName() + ")"); + break; + + case SecStorageVmAlertEventArgs.SSVM_CREATE_FAILURE: + if (s_logger.isDebugEnabled()) { + s_logger.debug("Secondary Storage Vm creation failure, zone: " + dc.getName()); + } + + _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_SSVM, args.getZoneId(), null, "Secondary Storage Vm creation failure. zone: " + dc.getName() + + ", error details: " + args.getMessage(), "Secondary Storage Vm creation failure (zone " + dc.getName() + ")"); + break; + + case SecStorageVmAlertEventArgs.SSVM_START_FAILURE: + if (s_logger.isDebugEnabled()) { + s_logger.debug("Secondary Storage Vm startup failure, zone: " + dc.getName() + ", secStorageVm: " + secStorageVmHostName + ", public IP: " + + secStorageVmPublicIpAddress + ", private IP: " + secStorageVmPrivateIpAddress); + } + + _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_SSVM, args.getZoneId(), secStorageVmPodIdToDeployIn, "Secondary Storage Vm startup failure. zone: " + + dc.getName() + ", secStorageVm: " + secStorageVmHostName + ", public IP: " + secStorageVmPublicIpAddress + ", private IP: " + + secStorageVmPrivateIpAddress + ", error details: " + args.getMessage(), "Secondary Storage Vm startup failure (zone " + dc.getName() + ")"); + break; + + case SecStorageVmAlertEventArgs.SSVM_FIREWALL_ALERT: + if (s_logger.isDebugEnabled()) { + s_logger.debug("Secondary Storage Vm firewall alert, zone: " + dc.getName() + ", secStorageVm: " + secStorageVmHostName + ", public IP: " + + secStorageVmPublicIpAddress + ", private IP: " + secStorageVmPrivateIpAddress); + } + + _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_SSVM, args.getZoneId(), secStorageVmPodIdToDeployIn, "Failed to open secondary storage vm firewall port. " + + "zone: " + dc.getName() + ", secStorageVm: " + secStorageVmHostName + ", public IP: " + secStorageVmPublicIpAddress + ", private IP: " + + secStorageVmPrivateIpAddress, "Secondary Storage Vm alert (zone " + dc.getName() + ")"); + break; + + case SecStorageVmAlertEventArgs.SSVM_STORAGE_ALERT: + if (s_logger.isDebugEnabled()) { + s_logger.debug("Secondary Storage Vm storage alert, zone: " + dc.getName() + ", secStorageVm: " + secStorageVmHostName + ", public IP: " + + secStorageVmPublicIpAddress + ", private IP: " + secStorageVmPrivateIpAddress + ", message: " + args.getMessage()); + } + + _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_STORAGE_MISC, args.getZoneId(), secStorageVmPodIdToDeployIn, + "Secondary Storage Vm storage issue. zone: " + dc.getName() + ", message: " + args.getMessage(), "Secondary Storage Vm alert (zone " + dc.getName() + + ")"); + break; } } diff --git a/server/src/main/java/com/cloud/api/ApiServer.java b/server/src/main/java/com/cloud/api/ApiServer.java index 136cd624325d..ce0472f65b77 100644 --- a/server/src/main/java/com/cloud/api/ApiServer.java +++ b/server/src/main/java/com/cloud/api/ApiServer.java @@ -918,7 +918,7 @@ public boolean verifyRequest(final Map requestParameters, fina if ("3".equals(signatureVersion)) { // New signature authentication. Check for expire parameter and its validity if (expires == null) { - s_logger.debug("Missing Expires parameter -- ignoring request. Signature: " + signature + ", apiKey: " + apiKey); + s_logger.debug("Missing Expires parameter -- ignoring request."); return false; } @@ -931,7 +931,7 @@ public boolean verifyRequest(final Map requestParameters, fina final Date now = new Date(System.currentTimeMillis()); if (expiresTS.before(now)) { - s_logger.debug("Request expired -- ignoring ...sig: " + signature + ", apiKey: " + apiKey); + s_logger.debug("Request expired -- ignoring"); return false; } } @@ -978,7 +978,7 @@ public boolean verifyRequest(final Map requestParameters, fina final boolean equalSig = ConstantTimeComparator.compareStrings(signature, computedSignature); if (!equalSig) { - s_logger.info("User signature: " + signature + " is not equaled to computed signature: " + computedSignature); + s_logger.info("User signature is not equaled to computed signature: " + computedSignature); } else { CallContext.register(user, account); } diff --git a/server/src/main/java/com/cloud/network/ExternalLoadBalancerDeviceManagerImpl.java b/server/src/main/java/com/cloud/network/ExternalLoadBalancerDeviceManagerImpl.java index 34f1c4b2bcae..c44dfa54eea6 100644 --- a/server/src/main/java/com/cloud/network/ExternalLoadBalancerDeviceManagerImpl.java +++ b/server/src/main/java/com/cloud/network/ExternalLoadBalancerDeviceManagerImpl.java @@ -720,10 +720,10 @@ public ExternalLoadBalancerDeviceVO doInTransaction(TransactionStatus status) { DestroyLoadBalancerApplianceAnswer answer = null; try { answer = (DestroyLoadBalancerApplianceAnswer)_agentMgr.easySend(lbDevice.getParentHostId(), lbDeleteCmd); - if (answer == null || !answer.getResult()) { - s_logger.warn("Failed to destoy load balancer appliance used by the network" - + guestConfig.getId() + " due to " + answer == null ? "communication error with agent" - : answer.getDetails()); + if (answer == null) { + s_logger.warn(String.format("Failed to destroy load balancer appliance used by the network [%s] due to a communication error with agent.", guestConfig.getId())); + } else if (!answer.getResult()) { + s_logger.warn(String.format("Failed to destroy load balancer appliance used by the network [%s] due to [%s].", guestConfig.getId(), answer.getDetails())); } } catch (Exception e) { s_logger.warn("Failed to destroy load balancer appliance used by the network" + guestConfig.getId() + " due to " + e.getMessage()); diff --git a/server/src/main/java/com/cloud/servlet/ConsoleProxyServlet.java b/server/src/main/java/com/cloud/servlet/ConsoleProxyServlet.java index 595299440fe5..1e4094c5d360 100644 --- a/server/src/main/java/com/cloud/servlet/ConsoleProxyServlet.java +++ b/server/src/main/java/com/cloud/servlet/ConsoleProxyServlet.java @@ -154,7 +154,7 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) { String cmd = req.getParameter("cmd"); if (cmd == null || !isValidCmd(cmd)) { - s_logger.debug("invalid console servlet command: " + cmd); + s_logger.debug("invalid console servlet command."); sendResponse(resp, ""); return; } @@ -162,7 +162,7 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) { String vmIdString = req.getParameter("vm"); VirtualMachine vm = _entityMgr.findByUuid(VirtualMachine.class, vmIdString); if (vm == null) { - s_logger.info("invalid console servlet command parameter: " + vmIdString); + s_logger.info("invalid console servlet command vm parameter."); sendResponse(resp, ""); return; } @@ -262,7 +262,7 @@ private void handleAuthRequest(HttpServletRequest req, HttpServletResponse resp, String sid = req.getParameter("sid"); if (sid == null || !sid.equals(vm.getVncPassword())) { - s_logger.warn("sid " + sid + " in url does not match stored sid."); + s_logger.warn("sid in url does not match stored sid."); sendResponse(resp, "failed"); return; } diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 8815ea65eb0e..0d595abe2393 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -7298,7 +7298,7 @@ public void doInTransactionWithoutResult(TransactionStatus status) { _securityGroupMgr.addInstanceToGroups(vm.getId(), securityGroupIdList); - s_logger.debug("AssignVM: Basic zone, adding security groups no " + securityGroupIdList.size() + " to " + vm.getInstanceName()); + s_logger.debug("AssignVM: Basic zone, adding security groups no " + (securityGroupIdList != null ? securityGroupIdList.size() : 0) + " to " + vm.getInstanceName()); } else { Set applicableNetworks = new LinkedHashSet<>(); Map requestedIPv4ForNics = new HashMap<>(); diff --git a/server/src/main/java/org/apache/cloudstack/outofbandmanagement/OutOfBandManagementServiceImpl.java b/server/src/main/java/org/apache/cloudstack/outofbandmanagement/OutOfBandManagementServiceImpl.java index 39cea17d9f05..8ec9dd653bd4 100644 --- a/server/src/main/java/org/apache/cloudstack/outofbandmanagement/OutOfBandManagementServiceImpl.java +++ b/server/src/main/java/org/apache/cloudstack/outofbandmanagement/OutOfBandManagementServiceImpl.java @@ -255,7 +255,7 @@ private boolean isOutOfBandManagementEnabledForHost(Long hostId) { Host host = hostDao.findById(hostId); if (host == null || host.getResourceState() == ResourceState.Degraded) { - LOG.debug(String.format("Host [id=%s, state=] was removed or placed in Degraded state by the Admin.", hostId, host.getResourceState())); + LOG.debug(String.format("Host [id=%s, state=%s] was removed or placed in Degraded state by the Admin.", hostId, host != null ? host.getResourceState() : null)); return false; } diff --git a/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java b/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java index e32e2455e09a..3300207a2801 100644 --- a/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java +++ b/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java @@ -865,7 +865,7 @@ protected Answer copySnapshotToTemplateFromNfsToNfsXenserver(CopyCommand cmd, Sn String templateUuid = UUID.randomUUID().toString(); String templateName = templateUuid + ".vhd"; - Script command = new Script(createTemplateFromSnapshotXenScript, cmd.getWait() * 1000, s_logger); + Script command = new Script(createTemplateFromSnapshotXenScript, cmd.getWait() * 1000L, s_logger); command.add("-p", snapshotPath); command.add("-s", snapshotName); command.add("-n", templateName); From 71c8bdc81ad60fed6f4187f581b0fb6c1db310dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Jandre?= <48719461+JoaoJandre@users.noreply.github.com> Date: Mon, 30 Jan 2023 14:51:32 -0300 Subject: [PATCH 2/5] Address reviews --- .../kvm/storage/LibvirtStorageAdaptor.java | 3 +- .../cloud/alert/ConsoleProxyAlertAdapter.java | 65 +++++++++---------- .../alert/SecondaryStorageVmAlertAdapter.java | 65 +++++++++---------- 3 files changed, 64 insertions(+), 69 deletions(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java index 46d64b1d4bdc..d9e15c3d6621 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java @@ -1427,7 +1427,8 @@ public KVMPhysicalDisk copyPhysicalDisk(KVMPhysicalDisk disk, String name, KVMSt r.ioCtxDestroy(io); } catch (QemuImgException | LibvirtException e) { - s_logger.error(String.format("Failed to convert from %s to %s the error was: " + e.getMessage(), srcFile != null ? srcFile.getFileName() : null, destFile != null ? destFile.getFileName() : null)); + s_logger.error(String.format("Failed to convert from %s to %s the error was: %s", srcFile != null ? srcFile.getFileName() : null, + destFile != null ? destFile.getFileName() : null, e.getMessage())); newDisk = null; } catch (RadosException e) { s_logger.error("A Ceph RADOS operation failed (" + e.getReturnValue() + "). The error was: " + e.getMessage()); diff --git a/server/src/main/java/com/cloud/alert/ConsoleProxyAlertAdapter.java b/server/src/main/java/com/cloud/alert/ConsoleProxyAlertAdapter.java index 2d209e546dc7..cdcf68b10fdc 100644 --- a/server/src/main/java/com/cloud/alert/ConsoleProxyAlertAdapter.java +++ b/server/src/main/java/com/cloud/alert/ConsoleProxyAlertAdapter.java @@ -21,6 +21,7 @@ import javax.inject.Inject; import javax.naming.ConfigurationException; +import org.apache.cloudstack.alert.AlertService; import org.apache.log4j.Logger; import org.springframework.stereotype.Component; @@ -72,84 +73,82 @@ public void onProxyAlert(Object sender, ConsoleProxyAlertEventArgs args) { proxyPodIdToDeployIn = proxy.getPodIdToDeployIn(); } + String message = ""; + String zoneProxyPublicAndPrivateIp = String.format("zone [%s], proxy [%s], public IP [%s], private IP [%s].", dc.getName(), proxyHostName, proxyPublicIpAddress, + proxyPrivateIpAddress); + String zone = String.format("(zone %s)", dc.getName()); + String errorDetails = " Error details: " + args.getMessage(); + + switch (args.getType()) { case ConsoleProxyAlertEventArgs.PROXY_CREATED: if (s_logger.isDebugEnabled()) { - s_logger.debug("New console proxy created, zone: " + dc.getName() + ", proxy: " + proxyHostName + ", public IP: " + proxyPublicIpAddress + ", private IP: " + - proxyPrivateIpAddress); + s_logger.debug("New console proxy created, " + zoneProxyPublicAndPrivateIp); } break; case ConsoleProxyAlertEventArgs.PROXY_UP: + message = "Console proxy up in " + zoneProxyPublicAndPrivateIp; if (s_logger.isDebugEnabled()) { - s_logger.debug("Console proxy is up, zone: " + dc.getName() + ", proxy: " + proxyHostName + ", public IP: " + proxyPublicIpAddress + ", private IP: " + proxyPrivateIpAddress); + s_logger.debug(message); } - _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_CONSOLE_PROXY, args.getZoneId(), proxyPodIdToDeployIn, "Console proxy up in zone: " + dc.getName() + - ", proxy: " + proxyHostName + ", public IP: " + proxyPublicIpAddress + ", private IP: " + proxyPrivateIpAddress, "Console proxy up (zone " + dc.getName() + - ")"); + _alertMgr.sendAlert(AlertService.AlertType.ALERT_TYPE_CONSOLE_PROXY, args.getZoneId(), proxyPodIdToDeployIn, message, "Console proxy up " + zone); break; case ConsoleProxyAlertEventArgs.PROXY_DOWN: + message = "Console proxy is down in " + zoneProxyPublicAndPrivateIp; if (s_logger.isDebugEnabled()) { - s_logger.debug("Console proxy is down, zone: " + dc.getName() + ", proxy: " + proxyHostName + ", public IP: " + proxyPublicIpAddress + ", private IP: " + - proxyPrivateIpAddress); + s_logger.debug(message); } - _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_CONSOLE_PROXY, args.getZoneId(), proxyPodIdToDeployIn, "Console proxy down in zone: " + dc.getName() + - ", proxy: " + proxyHostName + ", public IP: " + proxyPublicIpAddress + ", private IP: " + proxyPrivateIpAddress, "Console proxy down (zone " + dc.getName() - + ")"); + _alertMgr.sendAlert(AlertService.AlertType.ALERT_TYPE_CONSOLE_PROXY, args.getZoneId(), proxyPodIdToDeployIn, message, "Console proxy down " + zone); break; case ConsoleProxyAlertEventArgs.PROXY_REBOOTED: + message = "Console proxy is rebooted in " + zoneProxyPublicAndPrivateIp; if (s_logger.isDebugEnabled()) { - s_logger.debug("Console proxy is rebooted, zone: " + dc.getName() + ", proxy: " + proxyHostName + ", public IP: " + proxyPublicIpAddress + ", private IP: " + - proxyPrivateIpAddress); + s_logger.debug(message); } - _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_CONSOLE_PROXY, args.getZoneId(), proxyPodIdToDeployIn, "Console proxy rebooted in zone: " + dc.getName() + - ", proxy: " + proxyHostName + ", public IP: " + proxyPublicIpAddress + ", private IP: " + proxyPrivateIpAddress, "Console proxy rebooted (zone " + - dc.getName() + ")"); + _alertMgr.sendAlert(AlertService.AlertType.ALERT_TYPE_CONSOLE_PROXY, args.getZoneId(), proxyPodIdToDeployIn, message, "Console proxy rebooted " + zone); break; case ConsoleProxyAlertEventArgs.PROXY_CREATE_FAILURE: + message = String.format("Console proxy creation failure. Zone [%s].", dc.getName()); if (s_logger.isDebugEnabled()) { - s_logger.debug("Console proxy creation failure, zone: " + dc.getName()); + s_logger.debug(message); } - _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_CONSOLE_PROXY, args.getZoneId(), null, "Console proxy creation failure. zone: " + dc.getName() + - ", error details: " + args.getMessage(), "Console proxy creation failure (zone " + dc.getName() + ")"); + _alertMgr.sendAlert(AlertService.AlertType.ALERT_TYPE_CONSOLE_PROXY, args.getZoneId(), null, message + errorDetails, "Console proxy creation failure " + zone); break; case ConsoleProxyAlertEventArgs.PROXY_START_FAILURE: + message = "Console proxy startup failure in " + zoneProxyPublicAndPrivateIp; if (s_logger.isDebugEnabled()) { - s_logger.debug("Console proxy startup failure, zone: " + dc.getName() + ", proxy: " + proxyHostName + ", public IP: " + proxyPublicIpAddress + ", private IP: " - + proxyPrivateIpAddress); + s_logger.debug(message); } - _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_CONSOLE_PROXY, args.getZoneId(), proxyPodIdToDeployIn, "Console proxy startup failure. zone: " + dc.getName() - + ", proxy: " + proxyHostName + ", public IP: " + proxyPublicIpAddress + ", private IP: " + proxyPrivateIpAddress + ", error details: " + - args.getMessage(), "Console proxy startup failure (zone " + dc.getName() + ")"); + _alertMgr.sendAlert(AlertService.AlertType.ALERT_TYPE_CONSOLE_PROXY, args.getZoneId(), proxyPodIdToDeployIn, message + errorDetails, + "Console proxy startup failure " + zone); break; case ConsoleProxyAlertEventArgs.PROXY_FIREWALL_ALERT: if (s_logger.isDebugEnabled()) { - s_logger.debug("Console proxy firewall alert, zone: " + dc.getName() + ", proxy: " + proxyHostName + ", public IP: " + proxyPublicIpAddress + ", private IP: " - + proxyPrivateIpAddress); + s_logger.debug("Console proxy firewall alert, " + zoneProxyPublicAndPrivateIp); } - _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_CONSOLE_PROXY, args.getZoneId(), proxyPodIdToDeployIn, "Failed to open console proxy firewall port. zone: " - + dc.getName() + ", proxy: " + proxyHostName + ", public IP: " + proxyPublicIpAddress + ", private IP: " + proxyPrivateIpAddress, "Console proxy alert" - + " (zone " + dc.getName() + ")"); + _alertMgr.sendAlert(AlertService.AlertType.ALERT_TYPE_CONSOLE_PROXY, args.getZoneId(), proxyPodIdToDeployIn, "Failed to open console proxy firewall port. " + + zoneProxyPublicAndPrivateIp, "Console proxy alert " + zone); break; case ConsoleProxyAlertEventArgs.PROXY_STORAGE_ALERT: + message = zoneProxyPublicAndPrivateIp + ", message: " + args.getMessage(); if (s_logger.isDebugEnabled()) { - s_logger.debug("Console proxy storage alert, zone: " + dc.getName() + ", proxy: " + proxyHostName + ", public IP: " + proxyPublicIpAddress + ", private IP: " + - proxyPrivateIpAddress + ", message: " + args.getMessage()); + s_logger.debug("Console proxy storage alert, " + message); } - _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_STORAGE_MISC, args.getZoneId(), proxyPodIdToDeployIn, "Console proxy storage issue. zone: " + dc.getName() + - ", message: " + args.getMessage(), "Console proxy alert (zone " + dc.getName() + ")"); + _alertMgr.sendAlert(AlertService.AlertType.ALERT_TYPE_STORAGE_MISC, args.getZoneId(), proxyPodIdToDeployIn, "Console proxy storage issue. " + message, + "Console proxy alert " + zone); break; } } diff --git a/server/src/main/java/com/cloud/alert/SecondaryStorageVmAlertAdapter.java b/server/src/main/java/com/cloud/alert/SecondaryStorageVmAlertAdapter.java index 33f817a04fb9..5edc3ccd8f8b 100644 --- a/server/src/main/java/com/cloud/alert/SecondaryStorageVmAlertAdapter.java +++ b/server/src/main/java/com/cloud/alert/SecondaryStorageVmAlertAdapter.java @@ -21,6 +21,7 @@ import javax.inject.Inject; import javax.naming.ConfigurationException; +import org.apache.cloudstack.alert.AlertService; import org.apache.log4j.Logger; import org.springframework.stereotype.Component; @@ -70,88 +71,82 @@ public void onSSVMAlert(Object sender, SecStorageVmAlertEventArgs args) { secStorageVmPrivateIpAddress = secStorageVm.getPrivateIpAddress() == null ? "N/A" : secStorageVm.getPrivateIpAddress(); secStorageVmPodIdToDeployIn = secStorageVm.getPodIdToDeployIn(); } + String message = ""; + String zoneSecStorageVmPrivateAndPublicIp = String.format("zone [%s], secStorageVm [%s], public IP [%s], private IP [%s].", dc.getName(), secStorageVmHostName, + secStorageVmPublicIpAddress, secStorageVmPrivateIpAddress); + String errorDetails = " Error details: " + args.getMessage(); + String zone = String.format("(zone %s)", dc.getName()); switch (args.getType()) { case SecStorageVmAlertEventArgs.SSVM_CREATED: if (s_logger.isDebugEnabled()) { - s_logger.debug("New secondary storage vm created, zone: " + dc.getName() + ", secStorageVm: " + secStorageVmHostName + ", public IP: " + - secStorageVmPublicIpAddress + ", private IP: " + secStorageVmPrivateIpAddress); + s_logger.debug("New secondary storage vm created in " + zoneSecStorageVmPrivateAndPublicIp); } break; case SecStorageVmAlertEventArgs.SSVM_UP: + message = "Secondary Storage Vm is up in " + zoneSecStorageVmPrivateAndPublicIp; if (s_logger.isDebugEnabled()) { - s_logger.debug("Secondary Storage Vm is up, zone: " + dc.getName() + ", secStorageVm: " + secStorageVmHostName + ", public IP: " + secStorageVmPublicIpAddress + - ", private IP: " + secStorageVmPrivateIpAddress); + s_logger.debug(message); } - _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_SSVM, args.getZoneId(), secStorageVmPodIdToDeployIn, "Secondary Storage Vm up in zone: " + - dc.getName() + ", secStorageVm: " + secStorageVmHostName + ", public IP: " + secStorageVmPublicIpAddress + ", private IP: " + secStorageVmPrivateIpAddress, - "Secondary Storage Vm up (zone " + dc.getName() + ")"); + _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_SSVM, args.getZoneId(), secStorageVmPodIdToDeployIn, message, "Secondary Storage Vm up " + zone); break; case SecStorageVmAlertEventArgs.SSVM_DOWN: + message = "Secondary Storage Vm is down in " + zoneSecStorageVmPrivateAndPublicIp; if (s_logger.isDebugEnabled()) { - s_logger.debug("Secondary Storage Vm is down, zone: " + dc.getName() + ", secStorageVm: " + secStorageVmHostName + ", public IP: " + secStorageVmPublicIpAddress - + ", private IP: " + secStorageVmPrivateIpAddress); + s_logger.debug(message); } - _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_SSVM, args.getZoneId(), secStorageVmPodIdToDeployIn, "Secondary Storage Vm down in zone: " + - dc.getName() + ", secStorageVm: " + secStorageVmHostName + ", public IP: " + secStorageVmPublicIpAddress + ", private IP: " + secStorageVmPrivateIpAddress, - "Secondary Storage Vm down (zone " + dc.getName() + ")"); + _alertMgr.sendAlert(AlertService.AlertType.ALERT_TYPE_SSVM, args.getZoneId(), secStorageVmPodIdToDeployIn, message, "Secondary Storage Vm down " + zone); break; case SecStorageVmAlertEventArgs.SSVM_REBOOTED: + message = "Secondary Storage Vm rebooted in " + zoneSecStorageVmPrivateAndPublicIp; if (s_logger.isDebugEnabled()) { - s_logger.debug("Secondary Storage Vm is rebooted, zone: " + dc.getName() + ", secStorageVm: " + secStorageVmHostName + ", public IP: " + - secStorageVmPublicIpAddress + ", private IP: " + secStorageVmPrivateIpAddress); + s_logger.debug(message); } - _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_SSVM, args.getZoneId(), secStorageVmPodIdToDeployIn, "Secondary Storage Vm rebooted in zone: " + dc.getName() - + ", secStorageVm: " + secStorageVmHostName + ", public IP: " + secStorageVmPublicIpAddress + ", private IP: " + secStorageVmPrivateIpAddress, - "Secondary Storage Vm rebooted (zone " + dc.getName() + ")"); + _alertMgr.sendAlert(AlertService.AlertType.ALERT_TYPE_SSVM, args.getZoneId(), secStorageVmPodIdToDeployIn, message, "Secondary Storage Vm rebooted " + zone); break; case SecStorageVmAlertEventArgs.SSVM_CREATE_FAILURE: + message = String.format("Secondary Storage Vm creation failure in zone [%s].", dc.getName()); if (s_logger.isDebugEnabled()) { - s_logger.debug("Secondary Storage Vm creation failure, zone: " + dc.getName()); + s_logger.debug(message); } - _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_SSVM, args.getZoneId(), null, "Secondary Storage Vm creation failure. zone: " + dc.getName() + - ", error details: " + args.getMessage(), "Secondary Storage Vm creation failure (zone " + dc.getName() + ")"); + _alertMgr.sendAlert(AlertService.AlertType.ALERT_TYPE_SSVM, args.getZoneId(), null, message + errorDetails, + "Secondary Storage Vm creation failure " + zone); break; case SecStorageVmAlertEventArgs.SSVM_START_FAILURE: + message = "Secondary Storage Vm startup failure in " + zoneSecStorageVmPrivateAndPublicIp; if (s_logger.isDebugEnabled()) { - s_logger.debug("Secondary Storage Vm startup failure, zone: " + dc.getName() + ", secStorageVm: " + secStorageVmHostName + ", public IP: " + - secStorageVmPublicIpAddress + ", private IP: " + secStorageVmPrivateIpAddress); + s_logger.debug(message); } - _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_SSVM, args.getZoneId(), secStorageVmPodIdToDeployIn, "Secondary Storage Vm startup failure. zone: " + - dc.getName() + ", secStorageVm: " + secStorageVmHostName + ", public IP: " + secStorageVmPublicIpAddress + ", private IP: " + - secStorageVmPrivateIpAddress + ", error details: " + args.getMessage(), "Secondary Storage Vm startup failure (zone " + dc.getName() + ")"); + _alertMgr.sendAlert(AlertService.AlertType.ALERT_TYPE_SSVM, args.getZoneId(), secStorageVmPodIdToDeployIn, message + errorDetails, + "Secondary Storage Vm startup failure " + zone); break; case SecStorageVmAlertEventArgs.SSVM_FIREWALL_ALERT: if (s_logger.isDebugEnabled()) { - s_logger.debug("Secondary Storage Vm firewall alert, zone: " + dc.getName() + ", secStorageVm: " + secStorageVmHostName + ", public IP: " + - secStorageVmPublicIpAddress + ", private IP: " + secStorageVmPrivateIpAddress); + s_logger.debug("Secondary Storage Vm firewall alert, " + zoneSecStorageVmPrivateAndPublicIp); } - _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_SSVM, args.getZoneId(), secStorageVmPodIdToDeployIn, "Failed to open secondary storage vm firewall port. " - + "zone: " + dc.getName() + ", secStorageVm: " + secStorageVmHostName + ", public IP: " + secStorageVmPublicIpAddress + ", private IP: " + - secStorageVmPrivateIpAddress, "Secondary Storage Vm alert (zone " + dc.getName() + ")"); + _alertMgr.sendAlert(AlertService.AlertType.ALERT_TYPE_SSVM, args.getZoneId(), secStorageVmPodIdToDeployIn, "Failed to open secondary storage vm firewall port. " + + zoneSecStorageVmPrivateAndPublicIp, "Secondary Storage Vm alert " + zone); break; case SecStorageVmAlertEventArgs.SSVM_STORAGE_ALERT: if (s_logger.isDebugEnabled()) { - s_logger.debug("Secondary Storage Vm storage alert, zone: " + dc.getName() + ", secStorageVm: " + secStorageVmHostName + ", public IP: " + - secStorageVmPublicIpAddress + ", private IP: " + secStorageVmPrivateIpAddress + ", message: " + args.getMessage()); + s_logger.debug("Secondary Storage Vm storage alert, " + zoneSecStorageVmPrivateAndPublicIp + ", message: " + args.getMessage()); } - _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_STORAGE_MISC, args.getZoneId(), secStorageVmPodIdToDeployIn, - "Secondary Storage Vm storage issue. zone: " + dc.getName() + ", message: " + args.getMessage(), "Secondary Storage Vm alert (zone " + dc.getName() + - ")"); + _alertMgr.sendAlert(AlertService.AlertType.ALERT_TYPE_STORAGE_MISC, args.getZoneId(), secStorageVmPodIdToDeployIn, + "Secondary Storage Vm storage issue. zone: " + dc.getName() + ", message: " + args.getMessage(), "Secondary Storage Vm alert " + zone); break; } } From 820ad50536031e99a8e0088df82eaea0ec60a5e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Jandre?= <48719461+JoaoJandre@users.noreply.github.com> Date: Mon, 30 Jan 2023 16:08:57 -0300 Subject: [PATCH 3/5] Fix last code smell --- .../java/com/cloud/alert/SecondaryStorageVmAlertAdapter.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/com/cloud/alert/SecondaryStorageVmAlertAdapter.java b/server/src/main/java/com/cloud/alert/SecondaryStorageVmAlertAdapter.java index 5edc3ccd8f8b..c7d7c5c4fefa 100644 --- a/server/src/main/java/com/cloud/alert/SecondaryStorageVmAlertAdapter.java +++ b/server/src/main/java/com/cloud/alert/SecondaryStorageVmAlertAdapter.java @@ -90,7 +90,7 @@ public void onSSVMAlert(Object sender, SecStorageVmAlertEventArgs args) { s_logger.debug(message); } - _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_SSVM, args.getZoneId(), secStorageVmPodIdToDeployIn, message, "Secondary Storage Vm up " + zone); + _alertMgr.sendAlert(AlertService.AlertType.ALERT_TYPE_SSVM, args.getZoneId(), secStorageVmPodIdToDeployIn, message, "Secondary Storage Vm up " + zone); break; case SecStorageVmAlertEventArgs.SSVM_DOWN: From 6ca187bec7d6fd907ba494d20b4e558b657461e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Jandre?= <48719461+JoaoJandre@users.noreply.github.com> Date: Thu, 9 Feb 2023 11:11:11 -0300 Subject: [PATCH 4/5] addres reviews --- .../kvm/storage/LibvirtStorageAdaptor.java | 5 +++-- .../resource/XenServerConnectionPool.java | 3 ++- server/src/main/java/com/cloud/api/ApiServer.java | 7 +++++-- .../com/cloud/servlet/ConsoleProxyServlet.java | 15 ++++++++++++--- .../main/java/com/cloud/vm/UserVmManagerImpl.java | 3 ++- .../OutOfBandManagementServiceImpl.java | 3 ++- 6 files changed, 26 insertions(+), 10 deletions(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java index d9e15c3d6621..183a36446cf2 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java @@ -1427,8 +1427,9 @@ public KVMPhysicalDisk copyPhysicalDisk(KVMPhysicalDisk disk, String name, KVMSt r.ioCtxDestroy(io); } catch (QemuImgException | LibvirtException e) { - s_logger.error(String.format("Failed to convert from %s to %s the error was: %s", srcFile != null ? srcFile.getFileName() : null, - destFile != null ? destFile.getFileName() : null, e.getMessage())); + String srcFilename = srcFile != null ? srcFile.getFileName() : null; + String destFilename = destFile != null ? destFile.getFileName() : null; + s_logger.error(String.format("Failed to convert from %s to %s the error was: %s", srcFilename, destFilename, e.getMessage())); newDisk = null; } catch (RadosException e) { s_logger.error("A Ceph RADOS operation failed (" + e.getReturnValue() + "). The error was: " + e.getMessage()); diff --git a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/XenServerConnectionPool.java b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/XenServerConnectionPool.java index 5d7f3da5c43d..2f27b1376fdb 100644 --- a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/XenServerConnectionPool.java +++ b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/XenServerConnectionPool.java @@ -210,7 +210,8 @@ public Connection connect(String hostUuid, String poolUuid, String ipAddress, throw e; } catch (Exception e) { if (s_logger.isDebugEnabled()) { - s_logger.debug("connect through IP(" + (mConn != null ? mConn.getIp() : null) + ") for pool(" + poolUuid + ") is broken due to " + e.toString()); + String ip = mConn != null ? mConn.getIp() : null; + s_logger.debug("connect through IP(" + ip + ") for pool(" + poolUuid + ") is broken due to " + e.toString()); } removeConnect(poolUuid); mConn = null; diff --git a/server/src/main/java/com/cloud/api/ApiServer.java b/server/src/main/java/com/cloud/api/ApiServer.java index ce0472f65b77..2806125dae54 100644 --- a/server/src/main/java/com/cloud/api/ApiServer.java +++ b/server/src/main/java/com/cloud/api/ApiServer.java @@ -931,7 +931,9 @@ public boolean verifyRequest(final Map requestParameters, fina final Date now = new Date(System.currentTimeMillis()); if (expiresTS.before(now)) { - s_logger.debug("Request expired -- ignoring"); + signature = signature.replaceAll("[\n\r]", "_"); + apiKey = apiKey.replaceAll("[\n\r]", "_"); + s_logger.debug(String.format("Request expired -- ignoring ...sig [%s], apiKey [%s].", signature, apiKey)); return false; } } @@ -978,7 +980,8 @@ public boolean verifyRequest(final Map requestParameters, fina final boolean equalSig = ConstantTimeComparator.compareStrings(signature, computedSignature); if (!equalSig) { - s_logger.info("User signature is not equaled to computed signature: " + computedSignature); + signature = signature.replaceAll("[\n\r]", "_"); + s_logger.info(String.format("User signature [%s] is not equaled to computed signature [%s].", signature, computedSignature)); } else { CallContext.register(user, account); } diff --git a/server/src/main/java/com/cloud/servlet/ConsoleProxyServlet.java b/server/src/main/java/com/cloud/servlet/ConsoleProxyServlet.java index 1e4094c5d360..8f317a7291a5 100644 --- a/server/src/main/java/com/cloud/servlet/ConsoleProxyServlet.java +++ b/server/src/main/java/com/cloud/servlet/ConsoleProxyServlet.java @@ -154,7 +154,10 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) { String cmd = req.getParameter("cmd"); if (cmd == null || !isValidCmd(cmd)) { - s_logger.debug("invalid console servlet command."); + if (cmd != null) { + cmd = cmd.replaceAll("[\n\r]", "_"); + } + s_logger.debug(String.format("invalid console servlet command [%s].", cmd)); sendResponse(resp, ""); return; } @@ -162,7 +165,10 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) { String vmIdString = req.getParameter("vm"); VirtualMachine vm = _entityMgr.findByUuid(VirtualMachine.class, vmIdString); if (vm == null) { - s_logger.info("invalid console servlet command vm parameter."); + if (vmIdString != null) { + vmIdString = vmIdString.replaceAll("[\n\r]", "_"); + } + s_logger.info(String.format("invalid console servlet command vm parameter[%s].", vmIdString)); sendResponse(resp, ""); return; } @@ -262,7 +268,10 @@ private void handleAuthRequest(HttpServletRequest req, HttpServletResponse resp, String sid = req.getParameter("sid"); if (sid == null || !sid.equals(vm.getVncPassword())) { - s_logger.warn("sid in url does not match stored sid."); + if(sid != null) { + sid = sid.replaceAll("[\n\r]", "_"); + } + s_logger.warn(String.format("sid [%s] in url does not match stored sid.", sid)); sendResponse(resp, "failed"); return; } diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 0d595abe2393..0ba095f79b90 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -7298,7 +7298,8 @@ public void doInTransactionWithoutResult(TransactionStatus status) { _securityGroupMgr.addInstanceToGroups(vm.getId(), securityGroupIdList); - s_logger.debug("AssignVM: Basic zone, adding security groups no " + (securityGroupIdList != null ? securityGroupIdList.size() : 0) + " to " + vm.getInstanceName()); + int securityIdList = securityGroupIdList != null ? securityGroupIdList.size() : 0; + s_logger.debug("AssignVM: Basic zone, adding security groups no " + securityIdList + " to " + vm.getInstanceName()); } else { Set applicableNetworks = new LinkedHashSet<>(); Map requestedIPv4ForNics = new HashMap<>(); diff --git a/server/src/main/java/org/apache/cloudstack/outofbandmanagement/OutOfBandManagementServiceImpl.java b/server/src/main/java/org/apache/cloudstack/outofbandmanagement/OutOfBandManagementServiceImpl.java index 8ec9dd653bd4..302765aa2873 100644 --- a/server/src/main/java/org/apache/cloudstack/outofbandmanagement/OutOfBandManagementServiceImpl.java +++ b/server/src/main/java/org/apache/cloudstack/outofbandmanagement/OutOfBandManagementServiceImpl.java @@ -255,7 +255,8 @@ private boolean isOutOfBandManagementEnabledForHost(Long hostId) { Host host = hostDao.findById(hostId); if (host == null || host.getResourceState() == ResourceState.Degraded) { - LOG.debug(String.format("Host [id=%s, state=%s] was removed or placed in Degraded state by the Admin.", hostId, host != null ? host.getResourceState() : null)); + String state = host != null ? String.valueOf(host.getResourceState()) : null; + LOG.debug(String.format("Host [id=%s, state=%s] was removed or placed in Degraded state by the Admin.", hostId, state)); return false; } From a721520377b44b3728e3467c83f025302401fd2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Jandre?= <48719461+JoaoJandre@users.noreply.github.com> Date: Fri, 10 Feb 2023 13:57:04 -0300 Subject: [PATCH 5/5] fix new code smells --- .../main/java/com/cloud/api/ApiServer.java | 8 ++++--- .../cloud/servlet/ConsoleProxyServlet.java | 23 ++++++++++++++----- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/server/src/main/java/com/cloud/api/ApiServer.java b/server/src/main/java/com/cloud/api/ApiServer.java index 2806125dae54..67f3f964e65a 100644 --- a/server/src/main/java/com/cloud/api/ApiServer.java +++ b/server/src/main/java/com/cloud/api/ApiServer.java @@ -187,6 +187,8 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer private static final Logger s_logger = Logger.getLogger(ApiServer.class.getName()); private static final Logger s_accessLogger = Logger.getLogger("apiserver." + ApiServer.class.getName()); + private static final String SANITIZATION_REGEX = "[\n\r]"; + private static boolean encodeApiResponse = false; /** @@ -931,8 +933,8 @@ public boolean verifyRequest(final Map requestParameters, fina final Date now = new Date(System.currentTimeMillis()); if (expiresTS.before(now)) { - signature = signature.replaceAll("[\n\r]", "_"); - apiKey = apiKey.replaceAll("[\n\r]", "_"); + signature = signature.replaceAll(SANITIZATION_REGEX, "_"); + apiKey = apiKey.replaceAll(SANITIZATION_REGEX, "_"); s_logger.debug(String.format("Request expired -- ignoring ...sig [%s], apiKey [%s].", signature, apiKey)); return false; } @@ -980,7 +982,7 @@ public boolean verifyRequest(final Map requestParameters, fina final boolean equalSig = ConstantTimeComparator.compareStrings(signature, computedSignature); if (!equalSig) { - signature = signature.replaceAll("[\n\r]", "_"); + signature = signature.replaceAll(SANITIZATION_REGEX, "_"); s_logger.info(String.format("User signature [%s] is not equaled to computed signature [%s].", signature, computedSignature)); } else { CallContext.register(user, account); diff --git a/server/src/main/java/com/cloud/servlet/ConsoleProxyServlet.java b/server/src/main/java/com/cloud/servlet/ConsoleProxyServlet.java index 8f317a7291a5..83c359a96f94 100644 --- a/server/src/main/java/com/cloud/servlet/ConsoleProxyServlet.java +++ b/server/src/main/java/com/cloud/servlet/ConsoleProxyServlet.java @@ -71,6 +71,8 @@ public class ConsoleProxyServlet extends HttpServlet { private static final int DEFAULT_THUMBNAIL_WIDTH = 144; private static final int DEFAULT_THUMBNAIL_HEIGHT = 110; + private static final String SANITIZATION_REGEX = "[\n\r]"; + @Inject AccountManager _accountMgr; @Inject @@ -155,9 +157,12 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) { String cmd = req.getParameter("cmd"); if (cmd == null || !isValidCmd(cmd)) { if (cmd != null) { - cmd = cmd.replaceAll("[\n\r]", "_"); + cmd = cmd.replaceAll(SANITIZATION_REGEX, "_"); + s_logger.debug(String.format("invalid console servlet command [%s].", cmd)); + } else { + s_logger.debug("Null console servlet command."); } - s_logger.debug(String.format("invalid console servlet command [%s].", cmd)); + sendResponse(resp, ""); return; } @@ -166,9 +171,12 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) { VirtualMachine vm = _entityMgr.findByUuid(VirtualMachine.class, vmIdString); if (vm == null) { if (vmIdString != null) { - vmIdString = vmIdString.replaceAll("[\n\r]", "_"); + vmIdString = vmIdString.replaceAll(SANITIZATION_REGEX, "_"); + s_logger.info(String.format("invalid console servlet command vm parameter[%s].", vmIdString)); + } else { + s_logger.info("Null console servlet command VM parameter."); } - s_logger.info(String.format("invalid console servlet command vm parameter[%s].", vmIdString)); + sendResponse(resp, ""); return; } @@ -269,9 +277,12 @@ 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("[\n\r]", "_"); + sid = sid.replaceAll(SANITIZATION_REGEX, "_"); + s_logger.warn(String.format("sid [%s] in url does not match stored sid.", sid)); + } else { + s_logger.warn("Null sid in URL."); } - s_logger.warn(String.format("sid [%s] in url does not match stored sid.", sid)); + sendResponse(resp, "failed"); return; }