Fix NPE on updating security groups for an instance (apache#10493)

* Fix NPE on updating security groups for an instance

* addressed review comments

* Method refactoring

---------

Co-authored-by: Harikrishna Patnala <[email protected]>

Author: 2 people

server/src/main/java/com/cloud/vm/UserVmManagerImpl.java

@@ -3236,11 +3236,29 @@ public UserVm updateVirtualMachine(long id, String displayName, String group, Bo
                     .getUuid(), nic.getId(), extraDhcpOptionsMap);
         }
 
+        checkAndUpdateSecurityGroupForVM(securityGroupIdList, vm, networks);
+
+        _vmDao.updateVM(id, displayName, ha, osTypeId, userData, userDataId,
+                userDataDetails, isDisplayVmEnabled, isDynamicallyScalable,
+                deleteProtection, customId, hostName, instanceName);
+
+        if (updateUserdata) {
+            updateUserData(vm);
+        }
+
+        if (State.Running == vm.getState()) {
+            updateDns(vm, hostName);
+        }
+
+        return _vmDao.findById(id);
+    }
+
+    private void checkAndUpdateSecurityGroupForVM(List<Long> securityGroupIdList, UserVmVO vm, List<NetworkVO> networks) {
         boolean isVMware = (vm.getHypervisorType() == HypervisorType.VMware);
 
         if (securityGroupIdList != null && isVMware) {
-            throw new InvalidParameterValueException("Security group feature is not supported for vmWare hypervisor");
-        } else if (securityGroupIdList != null){
+            throw new InvalidParameterValueException("Security group feature is not supported for VMware hypervisor");
+        } else if (securityGroupIdList != null) {
             DataCenterVO zone = _dcDao.findById(vm.getDataCenterId());
             List<Long> networkIds = new ArrayList<>();
             try {
@@ -3264,20 +3282,6 @@ public UserVm updateVirtualMachine(long id, String displayName, String group, Bo
                 updateSecurityGroup(vm, securityGroupIdList);
             }
         }
-
-        _vmDao.updateVM(id, displayName, ha, osTypeId, userData, userDataId,
-                userDataDetails, isDisplayVmEnabled, isDynamicallyScalable,
-                deleteProtection, customId, hostName, instanceName);
-
-        if (updateUserdata) {
-            updateUserData(vm);
-        }
-
-        if (State.Running == vm.getState()) {
-            updateDns(vm, hostName);
-        }
-
-        return _vmDao.findById(id);
     }
 
     private void updateSecurityGroup(UserVmVO vm, List<Long> securityGroupIdList) {
@@ -3287,7 +3291,7 @@ private void updateSecurityGroup(UserVmVO vm, List<Long> securityGroupIdList) {
             // Add instance in provided groups
             _securityGroupMgr.addInstanceToGroups(vm, securityGroupIdList);
         } else {
-            throw new InvalidParameterValueException("Virtual machine must be stopped prior to update security groups ");
+            throw new InvalidParameterValueException(String.format("VM %s must be stopped prior to update security groups", vm.getUuid()));
         }
     }
 
@@ -3831,7 +3835,7 @@ public UserVm createBasicSecurityGroupVirtualMachine(DataCenter zone, ServiceOff
         boolean isVmWare = (template.getHypervisorType() == HypervisorType.VMware || (hypervisor != null && hypervisor == HypervisorType.VMware));
 
         if (securityGroupIdList != null && isVmWare) {
-            throw new InvalidParameterValueException("Security group feature is not supported for vmWare hypervisor");
+            throw new InvalidParameterValueException("Security group feature is not supported for VMware hypervisor");
         } else if (!isVmWare && _networkModel.isSecurityGroupSupportedInNetwork(defaultNetwork) && _networkModel.canAddDefaultSecurityGroup()) {
             //add the default securityGroup only if no security group is specified
             if (securityGroupIdList == null || securityGroupIdList.isEmpty()) {
@@ -3891,7 +3895,7 @@ public UserVm createAdvancedSecurityGroupVirtualMachine(DataCenter zone, Service
 
         } else if (securityGroupIdList != null && !securityGroupIdList.isEmpty()) {
             if (isVmWare) {
-                throw new InvalidParameterValueException("Security group feature is not supported for vmWare hypervisor");
+                throw new InvalidParameterValueException("Security group feature is not supported for VMware hypervisor");
             }
             // Only one network can be specified, and it should be security group enabled
             if (networkIdList.size() > 1 && template.getHypervisorType() != HypervisorType.KVM && hypervisor != HypervisorType.KVM) {

ui/src/views/compute/EditVM.vue

@@ -207,27 +207,6 @@ export default {
       }).then(response => {
         const zone = response?.listzonesresponse?.zone || []
         this.securityGroupsEnabled = zone?.[0]?.securitygroupsenabled || this.$store.getters.showSecurityGroups
-        if (this.securityGroupsEnabled) {
-          api('listNetworks', { supportedservices: 'SecurityGroup' }).then(json => {
-            if (json.listnetworksresponse && json.listnetworksresponse.network) {
-              for (const net of json.listnetworksresponse.network) {
-                if (this.securityGroupNetworkProviderUseThisVM) {
-                  break
-                }
-                const listVmParams = {
-                  id: this.resource.id,
-                  networkid: net.id,
-                  listall: true
-                }
-                api('listVirtualMachines', listVmParams).then(json => {
-                  if (json.listvirtualmachinesresponse && json.listvirtualmachinesresponse?.virtualmachine?.length > 0) {
-                    this.securityGroupNetworkProviderUseThisVM = true
-                  }
-                })
-              }
-            }
-          })
-        }
       })
     },
     fetchSecurityGroups () {

ui/src/views/compute/InstanceTab.vue

@@ -249,6 +249,7 @@ export default {
       vm: {},
       totalStorage: 0,
       currentTab: 'details',
+      showUpdateSecurityGroupsModal: false,
       showAddVolumeModal: false,
       showUpdateSecurityGroupsModal: false,
       diskOfferings: [],