From 2215fbf4c9e62f97a84eb7db3db5ee708903cd65 Mon Sep 17 00:00:00 2001 From: Jayapal Date: Wed, 19 Mar 2014 16:58:58 +0530 Subject: [PATCH] CLOUDSTACK-6240 Fixed updating advanced SG rules for vm nic secondary ip --- .../api/command/user/vm/AddIpToVmNicCmd.java | 8 +++- .../command/user/vm/RemoveIpFromVmNicCmd.java | 8 +++- .../security/SecurityGroupManagerImpl.java | 38 +++++++++---------- 3 files changed, 32 insertions(+), 22 deletions(-) diff --git a/api/src/org/apache/cloudstack/api/command/user/vm/AddIpToVmNicCmd.java b/api/src/org/apache/cloudstack/api/command/user/vm/AddIpToVmNicCmd.java index b5e223993de..c55d4315659 100644 --- a/api/src/org/apache/cloudstack/api/command/user/vm/AddIpToVmNicCmd.java +++ b/api/src/org/apache/cloudstack/api/command/user/vm/AddIpToVmNicCmd.java @@ -112,6 +112,12 @@ public class AddIpToVmNicCmd extends BaseAsyncCmd { return dc.getNetworkType(); } + private boolean isZoneSGEnabled() { + Network ntwk = _entityMgr.findById(Network.class, getNetworkId()); + DataCenter dc = _entityMgr.findById(DataCenter.class, ntwk.getDataCenterId()); + return dc.isSecurityGroupEnabled(); + } + @Override public long getEntityOwnerId() { Account caller = CallContext.current().getCallingAccount(); @@ -164,7 +170,7 @@ public class AddIpToVmNicCmd extends BaseAsyncCmd { if (result != null) { secondaryIp = result.getIp4Address(); - if (getNetworkType() == NetworkType.Basic) { + if (isZoneSGEnabled()) { // add security group rules for the secondary ip addresses boolean success = false; success = _securityGroupService.securityGroupRulesForVmSecIp(getNicId(), getNetworkId(), secondaryIp, (boolean) true); diff --git a/api/src/org/apache/cloudstack/api/command/user/vm/RemoveIpFromVmNicCmd.java b/api/src/org/apache/cloudstack/api/command/user/vm/RemoveIpFromVmNicCmd.java index 199cf2e772c..0fbf8607eb6 100644 --- a/api/src/org/apache/cloudstack/api/command/user/vm/RemoveIpFromVmNicCmd.java +++ b/api/src/org/apache/cloudstack/api/command/user/vm/RemoveIpFromVmNicCmd.java @@ -129,6 +129,12 @@ public class RemoveIpFromVmNicCmd extends BaseAsyncCmd { return null; } + private boolean isZoneSGEnabled() { + Network ntwk = _entityMgr.findById(Network.class, getNetworkId()); + DataCenter dc = _entityMgr.findById(DataCenter.class, ntwk.getDataCenterId()); + return dc.isSecurityGroupEnabled(); + } + @Override public void execute() throws InvalidParameterValueException { CallContext.current().setEventDetails("Ip Id: " + id); @@ -138,7 +144,7 @@ public class RemoveIpFromVmNicCmd extends BaseAsyncCmd { throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Invalid IP id is passed"); } - if (getNetworkType() == NetworkType.Basic) { + if (isZoneSGEnabled()) { //remove the security group rules for this secondary ip boolean success = false; success = _securityGroupService.securityGroupRulesForVmSecIp(nicSecIp.getNicId(), nicSecIp.getNetworkId(),nicSecIp.getIp4Address(), false); diff --git a/server/src/com/cloud/network/security/SecurityGroupManagerImpl.java b/server/src/com/cloud/network/security/SecurityGroupManagerImpl.java index 51c93b7f178..b46e15accec 100755 --- a/server/src/com/cloud/network/security/SecurityGroupManagerImpl.java +++ b/server/src/com/cloud/network/security/SecurityGroupManagerImpl.java @@ -1341,6 +1341,7 @@ public class SecurityGroupManagerImpl extends ManagerBase implements SecurityGro @Override public boolean securityGroupRulesForVmSecIp(Long nicId, Long networkId, String secondaryIp, boolean ruleAction) { + Account caller = CallContext.current().getCallingAccount(); String vmMac = null; String vmName = null; @@ -1351,36 +1352,33 @@ public class SecurityGroupManagerImpl extends ManagerBase implements SecurityGro NicVO nic = _nicDao.findById(nicId); Long vmId = nic.getInstanceId(); + UserVm vm = _userVMDao.findById(vmId); + if (vm == null || vm.getType() != VirtualMachine.Type.User) { + throw new InvalidParameterValueException("Can't configure the SG ipset, arprules rules for the non existing or non user vm"); + } + // Verify permissions + _accountMgr.checkAccess(caller, null, false, vm); // Validate parameters List vmSgGrps = getSecurityGroupsForVm(vmId); - if (vmSgGrps == null) { + if (vmSgGrps.isEmpty()) { s_logger.debug("Vm is not in any Security group "); return true; } - Account caller = CallContext.current().getCallingAccount(); - - for (SecurityGroupVO securityGroup: vmSgGrps) { - Account owner = _accountMgr.getAccount(securityGroup.getAccountId()); - if (owner == null) { - throw new InvalidParameterValueException("Unable to find security group owner by id=" + securityGroup.getAccountId()); - } - // Verify permissions - _accountMgr.checkAccess(caller, null, true, securityGroup); + //If network does not support SG service, no need add SG rules for secondary ip + Network network = _networkModel.getNetwork(nic.getNetworkId()); + if (!_networkModel.isSecurityGroupSupportedInNetwork(network)) { + s_logger.debug("Network " + network + " is not enabled with security group service, "+ + "so not applying SG rules for secondary ip"); + return true; } - UserVm vm = _userVMDao.findById(vmId); - if (vm.getType() != VirtualMachine.Type.User) { - throw new InvalidParameterValueException("Can't configure the SG ipset, arprules rules for the non user vm"); - } - if (vm != null) { - vmMac = vm.getPrivateMacAddress(); - vmName = vm.getInstanceName(); - if (vmMac == null || vmName == null) { - throw new InvalidParameterValueException("vm name or vm mac can't be null"); - } + vmMac = vm.getPrivateMacAddress(); + vmName = vm.getInstanceName(); + if (vmMac == null || vmName == null) { + throw new InvalidParameterValueException("vm name or vm mac can't be null"); } //create command for the to add ip in ipset and arptables rules