diff --git a/plugin/portForwarding/src/main/java/org/zstack/network/service/portforwarding/PortForwardingManagerImpl.java b/plugin/portForwarding/src/main/java/org/zstack/network/service/portforwarding/PortForwardingManagerImpl.java index 87dd4c1539c..1758ae99a39 100755 --- a/plugin/portForwarding/src/main/java/org/zstack/network/service/portforwarding/PortForwardingManagerImpl.java +++ b/plugin/portForwarding/src/main/java/org/zstack/network/service/portforwarding/PortForwardingManagerImpl.java @@ -19,6 +19,7 @@ import org.zstack.header.core.NopeCompletion; import org.zstack.header.core.workflow.*; import org.zstack.header.errorcode.ErrorCode; +import org.zstack.header.errorcode.OperationFailureException; import org.zstack.header.errorcode.SysErrors; import org.zstack.header.exception.CloudRuntimeException; import org.zstack.header.identity.APIChangeResourceOwnerMsg; @@ -404,8 +405,16 @@ private void handle(APIDetachPortForwardingRuleMsg msg) { PortForwardingRuleInventory inv = PortForwardingRuleInventory.valueOf(vo); final PortForwardingStruct struct = makePortForwardingStruct(inv); struct.setReleaseVmNicInfoWhenDetaching(true); - final NetworkServiceProviderType providerType = nwServiceMgr.getTypeOfNetworkServiceProviderForService(struct.getGuestL3Network().getUuid(), - NetworkServiceType.PortForwarding); + final NetworkServiceProviderType providerType = getPortForwardingProviderTypeIfEnabled(struct.getGuestL3Network().getUuid()); + if (providerType == null) { + logger.warn(String.format("port forwarding rule[uuid:%s] is attached to vm nic[uuid:%s] on L3[uuid:%s] without PortForwarding service, clean stale binding only", + vo.getUuid(), vo.getVmNicUuid(), struct.getGuestL3Network().getUuid())); + detachPortForwardingRuleInDb(vo); + PortForwardingRuleVO prvo = dbf.reload(vo); + evt.setInventory(PortForwardingRuleInventory.valueOf(prvo)); + bus.publish(evt); + return; + } detachPortForwardingRule(struct, providerType.toString(), new Completion(msg) { @Override @@ -429,10 +438,44 @@ private VmInstanceState getVmStateFromVmNicUuid(String vmNicUuid) { VmInstanceState.class).param("nicUuid", vmNicUuid).find(); } + private NetworkServiceProviderType getPortForwardingProviderType(String l3NetworkUuid) { + return nwServiceMgr.getTypeOfNetworkServiceProviderForService(l3NetworkUuid, NetworkServiceType.PortForwarding); + } + + private NetworkServiceProviderType getPortForwardingProviderTypeIfEnabled(String l3NetworkUuid) { + try { + return getPortForwardingProviderType(l3NetworkUuid); + } catch (OperationFailureException e) { + if (!isPortForwardingServiceMissing(e.getErrorCode())) { + throw e; + } + return null; + } + } + + private boolean isPortForwardingServiceMissing(ErrorCode errorCode) { + return errorCode != null && (ORG_ZSTACK_NETWORK_SERVICE_10005.equals(errorCode.getGlobalErrorCode()) + || ORG_ZSTACK_NETWORK_SERVICE_10005.equals(errorCode.getCode())); + } + + private void detachPortForwardingRuleInDb(PortForwardingRuleVO vo) { + SQL.New(PortForwardingRuleVO.class).eq(PortForwardingRuleVO_.uuid, vo.getUuid()) + .set(PortForwardingRuleVO_.vmNicUuid, null) + .set(PortForwardingRuleVO_.guestIp, null).update(); + } + private void handle(final APIAttachPortForwardingRuleMsg msg) { final APIAttachPortForwardingRuleEvent evt = new APIAttachPortForwardingRuleEvent(msg.getId()); PortForwardingRuleVO vo = dbf.findByUuid(msg.getRuleUuid(), PortForwardingRuleVO.class); VmNicVO nicvo = dbf.findByUuid(msg.getVmNicUuid(), VmNicVO.class); + final NetworkServiceProviderType providerType; + try { + providerType = getPortForwardingProviderType(nicvo.getL3NetworkUuid()); + } catch (OperationFailureException e) { + evt.setError(e.getErrorCode()); + bus.publish(evt); + return; + } vo.setVmNicUuid(nicvo.getUuid()); vo.setGuestIp(nicvo.getIp()); L3NetworkVO nicL3Vo = dbf.findByUuid(nicvo.getL3NetworkUuid(), L3NetworkVO.class); @@ -445,9 +488,6 @@ private void handle(final APIAttachPortForwardingRuleMsg msg) { ModifyVipAttributesStruct struct = new ModifyVipAttributesStruct(); struct.setUseFor(PortForwardingConstant.PORTFORWARDING_NETWORK_SERVICE_TYPE); struct.setServiceUuid(vo.getUuid()); - final NetworkServiceProviderType providerType = - nwServiceMgr.getTypeOfNetworkServiceProviderForService( - nicvo.getL3NetworkUuid(), NetworkServiceType.PortForwarding); struct.setServiceProvider(providerType.toString()); struct.setPeerL3NetworkUuid(nicvo.getL3NetworkUuid()); vip.setStruct(struct); @@ -460,6 +500,7 @@ public void success() { @Override public void fail(ErrorCode errorCode) { + detachPortForwardingRuleInDb(vo); evt.setError(errorCode); bus.publish(evt); } @@ -468,7 +509,6 @@ public void fail(ErrorCode errorCode) { } final PortForwardingStruct struct = makePortForwardingStruct(inv); - final NetworkServiceProviderType providerType = nwServiceMgr.getTypeOfNetworkServiceProviderForService(struct.getGuestL3Network().getUuid(), NetworkServiceType.PortForwarding); attachPortForwardingRule(struct, providerType.toString(), new Completion(msg) { @Override public void success() { @@ -519,8 +559,29 @@ public void fail(ErrorCode errorCode) { } final PortForwardingStruct struct = makePortForwardingStruct(inv); - final NetworkServiceProviderType providerType = nwServiceMgr.getTypeOfNetworkServiceProviderForService(struct.getGuestL3Network().getUuid(), - NetworkServiceType.PortForwarding); + final NetworkServiceProviderType providerType = getPortForwardingProviderTypeIfEnabled(struct.getGuestL3Network().getUuid()); + if (providerType == null) { + logger.warn(String.format("port forwarding rule[uuid:%s] is attached to vm nic[uuid:%s] on L3[uuid:%s] without PortForwarding service, release VIP service and delete stale binding", + vo.getUuid(), vo.getVmNicUuid(), struct.getGuestL3Network().getUuid())); + ModifyVipAttributesStruct vipStruct = new ModifyVipAttributesStruct(); + vipStruct.setUseFor(PortForwardingConstant.PORTFORWARDING_NETWORK_SERVICE_TYPE); + vipStruct.setServiceUuid(vo.getUuid()); + Vip v = new Vip(inv.getVipUuid()); + v.setStruct(vipStruct); + v.release(new Completion(complete) { + @Override + public void success() { + dbf.remove(vo); + complete.success(); + } + + @Override + public void fail(ErrorCode errorCode) { + complete.fail(errorCode); + } + }); + return; + } for (RevokePortForwardingRuleExtensionPoint extp : revokeRuleExts) { try { @@ -576,9 +637,6 @@ public void run(FlowTrigger trigger, Map data) { vipStruct.setUseFor(PortForwardingConstant.PORTFORWARDING_NETWORK_SERVICE_TYPE); vipStruct.setServiceUuid(vo.getUuid()); if (struct.getGuestL3Network() != null) { - final NetworkServiceProviderType providerType = - nwServiceMgr.getTypeOfNetworkServiceProviderForService( - struct.getGuestL3Network().getUuid(), NetworkServiceType.PortForwarding); vipStruct.setServiceProvider(providerType.toString()); vipStruct.setPeerL3NetworkUuid(struct.getGuestL3Network().getUuid()); } @@ -746,6 +804,20 @@ private void doCreatePortForwardingRule(APICreatePortForwardingRuleMsg msg, Sync } VipVO vip = dbf.findByUuid(msg.getVipUuid(), VipVO.class); + VmNicVO vmNic = msg.getVmNicUuid() == null ? null : dbf.findByUuid(msg.getVmNicUuid(), VmNicVO.class); + final NetworkServiceProviderType providerType; + if (vmNic == null) { + providerType = null; + } else { + try { + providerType = getPortForwardingProviderType(vmNic.getL3NetworkUuid()); + } catch (OperationFailureException e) { + evt.setError(e.getErrorCode()); + bus.publish(evt); + syncChain.next(); + return; + } + } final PortForwardingRuleVO vo = new PortForwardingRuleVO(); if (msg.getResourceUuid() != null) { vo.setUuid(msg.getResourceUuid()); @@ -802,7 +874,6 @@ public void fail(ErrorCode errorCode) { return; } - VmNicVO vmNic = dbf.findByUuid(msg.getVmNicUuid(), VmNicVO.class); SimpleQuery q = dbf.createQuery(VmInstanceVO.class); q.select(VmInstanceVO_.state); q.add(VmInstanceVO_.uuid, Op.EQ, vmNic.getVmInstanceUuid()); @@ -845,9 +916,6 @@ public void setup() { flow(new NoRollbackFlow() { @Override public void run(FlowTrigger trigger, Map data) { - final NetworkServiceProviderType providerType = nwServiceMgr.getTypeOfNetworkServiceProviderForService(vmNic.getL3NetworkUuid(), - NetworkServiceType.PortForwarding); - for (AttachPortForwardingRuleExtensionPoint extp : attachRuleExts) { try { extp.preAttachPortForwardingRule(ruleInv, providerType); @@ -993,8 +1061,15 @@ public void run(SyncTaskChain chain) { } PortForwardingStruct struct = makePortForwardingStruct(PortForwardingRuleInventory.valueOf(pf)); - final NetworkServiceProviderType providerType = nwServiceMgr.getTypeOfNetworkServiceProviderForService(struct.getGuestL3Network().getUuid(), - NetworkServiceType.PortForwarding); + final NetworkServiceProviderType providerType = getPortForwardingProviderTypeIfEnabled(struct.getGuestL3Network().getUuid()); + if (providerType == null) { + logger.warn(String.format("port forwarding rule[uuid:%s] is attached to vm nic[uuid:%s] on L3[uuid:%s] without PortForwarding service, release stale VIP service directly", + pf.getUuid(), pf.getVmNicUuid(), struct.getGuestL3Network().getUuid())); + dbf.remove(pf); + completion.success(); + chain.next(); + return; + } PortForwardingBackend bkd = getPortForwardingBackend(providerType); bkd.revokePortForwardingRule(struct, new Completion(completion) { @Override diff --git a/test/src/test/groovy/org/zstack/test/integration/networkservice/provider/virtualrouter/portforwarding/PortForwardingInvalidNicBindingCase.groovy b/test/src/test/groovy/org/zstack/test/integration/networkservice/provider/virtualrouter/portforwarding/PortForwardingInvalidNicBindingCase.groovy new file mode 100644 index 00000000000..df189cf3e1b --- /dev/null +++ b/test/src/test/groovy/org/zstack/test/integration/networkservice/provider/virtualrouter/portforwarding/PortForwardingInvalidNicBindingCase.groovy @@ -0,0 +1,263 @@ +package org.zstack.test.integration.networkservice.provider.virtualrouter.portforwarding + +import org.zstack.core.db.Q +import org.zstack.core.db.SQL +import org.zstack.header.network.service.NetworkServiceType +import org.zstack.network.service.eip.EipConstant +import org.zstack.network.service.lb.LoadBalancerConstants +import org.zstack.network.service.portforwarding.PortForwardingConstant +import org.zstack.network.service.portforwarding.PortForwardingProtocolType +import org.zstack.network.service.portforwarding.PortForwardingRuleVO +import org.zstack.network.service.portforwarding.PortForwardingRuleVO_ +import org.zstack.network.service.virtualrouter.vyos.VyosConstants +import org.zstack.sdk.L3NetworkInventory +import org.zstack.sdk.PortForwardingRuleInventory +import org.zstack.sdk.VipInventory +import org.zstack.sdk.VmInstanceInventory +import org.zstack.sdk.VmNicInventory +import org.zstack.test.integration.networkservice.provider.NetworkServiceProviderTest +import org.zstack.testlib.EnvSpec +import org.zstack.testlib.SubCase +import org.zstack.utils.data.SizeUnit + +import static org.zstack.utils.clouderrorcode.CloudOperationsErrorCode.ORG_ZSTACK_NETWORK_SERVICE_10005 + +class PortForwardingInvalidNicBindingCase extends SubCase { + EnvSpec env + + @Override + void clean() { + env.delete() + } + + @Override + void setup() { + useSpring(NetworkServiceProviderTest.springSpec) + } + + @Override + void environment() { + env = env { + instanceOffering { + name = "instanceOffering" + memory = SizeUnit.GIGABYTE.toByte(8) + cpu = 4 + } + + sftpBackupStorage { + name = "sftp" + url = "/sftp" + username = "root" + password = "password" + hostname = "localhost" + + image { + name = "image" + url = "http://zstack.org/download/test.qcow2" + } + + image { + name = "vr" + url = "http://zstack.org/download/vr.qcow2" + } + } + + zone { + name = "zone" + description = "test" + + cluster { + name = "cluster" + hypervisorType = "KVM" + + kvm { + name = "kvm" + managementIp = "localhost" + username = "root" + password = "password" + } + + attachPrimaryStorage("local") + attachL2Network("l2") + } + + localPrimaryStorage { + name = "local" + url = "/local_ps" + } + + l2NoVlanNetwork { + name = "l2" + physicalInterface = "eth0" + + l3Network { + name = "l3" + + service { + provider = VyosConstants.VYOS_ROUTER_PROVIDER_TYPE + types = [NetworkServiceType.DHCP.toString(), + NetworkServiceType.DNS.toString(), + NetworkServiceType.SNAT.toString(), + PortForwardingConstant.PORTFORWARDING_NETWORK_SERVICE_TYPE, + LoadBalancerConstants.LB_NETWORK_SERVICE_TYPE_STRING, + EipConstant.EIP_NETWORK_SERVICE_TYPE] + } + + ip { + startIp = "192.168.100.10" + endIp = "192.168.100.100" + netmask = "255.255.255.0" + gateway = "192.168.100.1" + } + } + + l3Network { + name = "pubL3" + + ip { + startIp = "11.168.100.10" + endIp = "11.168.100.100" + netmask = "255.255.255.0" + gateway = "11.168.100.1" + } + } + } + + attachBackupStorage("sftp") + + virtualRouterOffering { + name = "vro" + memory = SizeUnit.MEGABYTE.toByte(512) + cpu = 2 + useManagementL3Network("pubL3") + usePublicL3Network("pubL3") + useImage("vr") + } + } + + vm { + name = "vm" + useImage("image") + useL3Networks("l3") + useInstanceOffering("instanceOffering") + } + } + } + + @Override + void test() { + env.create { + testInvalidNicDoesNotLeaveStaleBinding() + testStaleInvalidNicBindingCanBeDetachedOrDeleted() + } + } + + PortForwardingRuleInventory createRule(VipInventory vip, int port) { + return createPortForwardingRule { + name = "pf-${port}" + vipUuid = vip.uuid + vipPortStart = port + vipPortEnd = port + privatePortStart = port + privatePortEnd = port + protocolType = PortForwardingProtocolType.TCP.toString() + } + } + + VmNicInventory publicNicOfVm(VmInstanceInventory vm, String publicL3Uuid) { + return vm.vmNics.find { VmNicInventory nic -> nic.l3NetworkUuid == publicL3Uuid } + } + + void setStaleBinding(PortForwardingRuleInventory rule, VmNicInventory wrongNic) { + SQL.New(PortForwardingRuleVO.class) + .eq(PortForwardingRuleVO_.uuid, rule.uuid) + .set(PortForwardingRuleVO_.vmNicUuid, wrongNic.uuid) + .set(PortForwardingRuleVO_.guestIp, wrongNic.ip) + .update() + } + + void assertRuleDetached(String ruleUuid) { + assert Q.New(PortForwardingRuleVO.class) + .eq(PortForwardingRuleVO_.uuid, ruleUuid) + .select(PortForwardingRuleVO_.vmNicUuid) + .findValue() == null + assert Q.New(PortForwardingRuleVO.class) + .eq(PortForwardingRuleVO_.uuid, ruleUuid) + .select(PortForwardingRuleVO_.guestIp) + .findValue() == null + } + + void testInvalidNicDoesNotLeaveStaleBinding() { + L3NetworkInventory l3 = env.inventoryByName("l3") + L3NetworkInventory pubL3 = env.inventoryByName("pubL3") + VmInstanceInventory vm = env.inventoryByName("vm") + vm = attachL3NetworkToVm { + vmInstanceUuid = vm.uuid + l3NetworkUuid = pubL3.uuid + } + VmNicInventory publicNic = publicNicOfVm(vm, pubL3.uuid) + assert publicNic != null + + VipInventory vip = createVip { + name = "vip" + l3NetworkUuid = l3.uuid + } + PortForwardingRuleInventory rule = createRule(vip, 80) + + expectApiFailure { + attachPortForwardingRule { + vmNicUuid = publicNic.uuid + ruleUuid = rule.uuid + } + } { + assert globalErrorCode == ORG_ZSTACK_NETWORK_SERVICE_10005 + } + + assertRuleDetached(rule.uuid) + + long count = Q.New(PortForwardingRuleVO.class).count() + expectApiFailure { + createPortForwardingRule { + name = "invalid-pf" + vipUuid = vip.uuid + vipPortStart = 81 + vipPortEnd = 81 + privatePortStart = 81 + privatePortEnd = 81 + protocolType = PortForwardingProtocolType.TCP.toString() + vmNicUuid = publicNic.uuid + } + } { + assert globalErrorCode == ORG_ZSTACK_NETWORK_SERVICE_10005 + } + assert Q.New(PortForwardingRuleVO.class).count() == count + } + + void testStaleInvalidNicBindingCanBeDetachedOrDeleted() { + L3NetworkInventory l3 = env.inventoryByName("l3") + L3NetworkInventory pubL3 = env.inventoryByName("pubL3") + VmInstanceInventory vm = queryVmInstance { + conditions = ["name=vm"] + }[0] + VmNicInventory publicNic = publicNicOfVm(vm, pubL3.uuid) + assert publicNic != null + + VipInventory vip = createVip { + name = "vip-stale" + l3NetworkUuid = l3.uuid + } + PortForwardingRuleInventory rule = createRule(vip, 90) + + setStaleBinding(rule, publicNic) + detachPortForwardingRule { + uuid = rule.uuid + } + assertRuleDetached(rule.uuid) + + setStaleBinding(rule, publicNic) + deletePortForwardingRule { + uuid = rule.uuid + } + assert !Q.New(PortForwardingRuleVO.class).eq(PortForwardingRuleVO_.uuid, rule.uuid).isExists() + } +}