Skip to content

Commit df22929

Browse files
nvazqueznvazquez
authored
Fix Static NAT rules naming (apache#83)
1 parent efa985d commit df22929

File tree

6 files changed

+30
-21
lines changed

6 files changed

+30
-21
lines changed

‎api/src/main/java/com/cloud/network/netris/NetrisService.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,9 @@ public interface NetrisService {
4646

4747
boolean updateVpcSourceNatIp(Vpc vpc, IpAddress address);
4848

49-
boolean createStaticNatRule(long zoneId, long accountId, long domainId, String networkResourceName, Long networkResourceId, boolean isForVpc, String vpcCidr, String staticNatIp, String vmIp);
49+
boolean createStaticNatRule(long zoneId, long accountId, long domainId, String networkResourceName, Long networkResourceId, boolean isForVpc, String vpcCidr, String staticNatIp, String vmIp, long vmId);
5050

51-
boolean deleteStaticNatRule(long zoneId, long accountId, long domainId, String networkResourceName, Long networkResourceId, boolean isForVpc, String staticNatIp);
51+
boolean deleteStaticNatRule(long zoneId, long accountId, long domainId, String networkResourceName, Long networkResourceId, boolean isForVpc, String staticNatIp, long vmId);
5252

5353
boolean addFirewallRules(Network network, List<NetrisNetworkRule> firewallRules);
5454
boolean deleteFirewallRules(Network network, List<NetrisNetworkRule> firewallRules);

‎plugins/network-elements/netris/src/main/java/org/apache/cloudstack/resource/NetrisResourceObjectUtils.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ public static String retrieveNetrisResourceObjectName(NetrisCommand cmd, NetrisO
7272
suffixes = new String[0];
7373
break;
7474
case STATICNAT:
75-
stringBuilder.append(String.format("%s%s-%s", prefix, suffixes[0], "STATICNAT"));
75+
stringBuilder.append(String.format("%s%s-VM%s-%s", prefix, suffixes[0], suffixes[1], "STATICNAT"));
7676
suffixes = new String[0];
7777
break;
7878
case DNAT:

‎plugins/network-elements/netris/src/main/java/org/apache/cloudstack/service/NetrisElement.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -676,6 +676,7 @@ public boolean applyStaticNats(Network config, List<? extends StaticNat> rules)
676676
if (vm == null || networkModel.getNicInNetworkIncludingRemoved(vm.getId(), config.getId()) == null) {
677677
continue;
678678
}
679+
long vmId = vm.getId();
679680
Pair<VpcVO, NetworkVO> vpcOrNetwork = getVpcOrNetwork(config.getVpcId(), config.getId());
680681
VpcVO vpc = vpcOrNetwork.first();
681682
NetworkVO network = vpcOrNetwork.second();
@@ -685,10 +686,10 @@ public boolean applyStaticNats(Network config, List<? extends StaticNat> rules)
685686
if (!staticNat.isForRevoke()) {
686687
return netrisService.createStaticNatRule(config.getDataCenterId(), config.getAccountId(), config.getDomainId(),
687688
networkResourceName, networkResourceId, isVpcResource, vpc.getCidr(),
688-
ipAddressVO.getAddress().addr(), staticNat.getDestIpAddress());
689+
ipAddressVO.getAddress().addr(), staticNat.getDestIpAddress(), vmId);
689690
} else {
690691
return netrisService.deleteStaticNatRule(config.getDataCenterId(), config.getAccountId(), config.getDomainId(),
691-
networkResourceName, networkResourceId, isVpcResource, ipAddressVO.getAddress().addr());
692+
networkResourceName, networkResourceId, isVpcResource, ipAddressVO.getAddress().addr(), vmId);
692693
}
693694
}
694695
return false;

‎plugins/network-elements/netris/src/main/java/org/apache/cloudstack/service/NetrisServiceImpl.java‎

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,7 @@ public boolean updateVpcSourceNatIp(Vpc vpc, IpAddress address) {
332332
}
333333

334334
@Override
335-
public boolean createStaticNatRule(long zoneId, long accountId, long domainId, String networkResourceName, Long networkResourceId, boolean isForVpc, String vpcCidr, String staticNatIp, String vmIp) {
335+
public boolean createStaticNatRule(long zoneId, long accountId, long domainId, String networkResourceName, Long networkResourceId, boolean isForVpc, String vpcCidr, String staticNatIp, String vmIp, long vmId) {
336336
String vpcName = null;
337337
String networkName = null;
338338
Long vpcId = null;
@@ -348,15 +348,15 @@ public boolean createStaticNatRule(long zoneId, long accountId, long domainId, S
348348
cmd.setNatRuleType("STATICNAT");
349349
cmd.setNatIp(staticNatIp);
350350
cmd.setVmIp(vmIp);
351-
String suffix = getResourceSuffix(vpcId, networkId, isForVpc);
352-
String dnatRuleName = NetrisResourceObjectUtils.retrieveNetrisResourceObjectName(cmd, NetrisResourceObjectUtils.NetrisObjectType.STATICNAT, suffix);
351+
String[] suffixes = getStaticNatResourceSuffixes(vpcId, networkId, isForVpc, vmId);
352+
String dnatRuleName = NetrisResourceObjectUtils.retrieveNetrisResourceObjectName(cmd, NetrisResourceObjectUtils.NetrisObjectType.STATICNAT, suffixes);
353353
cmd.setNatRuleName(dnatRuleName);
354354
NetrisAnswer answer = sendNetrisCommand(cmd, zoneId);
355355
return answer.getResult();
356356
}
357357

358358
@Override
359-
public boolean deleteStaticNatRule(long zoneId, long accountId, long domainId, String networkResourceName, Long networkResourceId, boolean isForVpc, String staticNatIp) {
359+
public boolean deleteStaticNatRule(long zoneId, long accountId, long domainId, String networkResourceName, Long networkResourceId, boolean isForVpc, String staticNatIp, long vmId) {
360360
String vpcName = null;
361361
String networkName = null;
362362
Long vpcId = null;
@@ -369,8 +369,8 @@ public boolean deleteStaticNatRule(long zoneId, long accountId, long domainId, S
369369
networkId = networkResourceId;
370370
}
371371
DeleteNetrisNatRuleCommand cmd = new DeleteNetrisNatRuleCommand(zoneId, accountId, domainId, vpcName, vpcId, networkName, networkId, isForVpc);
372-
String suffix = getResourceSuffix(vpcId, networkId, isForVpc);
373-
String dnatRuleName = NetrisResourceObjectUtils.retrieveNetrisResourceObjectName(cmd, NetrisResourceObjectUtils.NetrisObjectType.STATICNAT, suffix);
372+
String suffixes[] = getStaticNatResourceSuffixes(vpcId, networkId, isForVpc, vmId);
373+
String dnatRuleName = NetrisResourceObjectUtils.retrieveNetrisResourceObjectName(cmd, NetrisResourceObjectUtils.NetrisObjectType.STATICNAT, suffixes);
374374
cmd.setNatRuleName(dnatRuleName);
375375
cmd.setNatRuleType("STATICNAT");
376376
cmd.setNatIp(staticNatIp);
@@ -510,13 +510,10 @@ public boolean deleteLbRule(NetrisNetworkRule rule) {
510510
return answer.getResult();
511511
}
512512

513-
private String getResourceSuffix(Long vpcId, Long networkId, boolean isForVpc) {
514-
String suffix;
515-
if (isForVpc) {
516-
suffix = String.valueOf(vpcId); // D1-A1-Z1-V25-STATICNAT or D1-A1-Z1-V25-SNAT
517-
} else {
518-
suffix = String.valueOf(networkId); // D1-A1-Z1-N25-STATICNAT or D1-A1-Z1-N25-SNAT
519-
}
520-
return suffix;
513+
public static String[] getStaticNatResourceSuffixes(Long vpcId, Long networkId, boolean isForVpc, long vmId) {
514+
String[] suffixes = new String[2];
515+
suffixes[0] = isForVpc ? String.valueOf(vpcId) : String.valueOf(networkId);
516+
suffixes[1] = String.valueOf(vmId);
517+
return suffixes;
521518
}
522519
}

‎plugins/network-elements/netris/src/test/java/org/apache/cloudstack/resource/NetrisResourceObjectUtilsTest.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import org.apache.cloudstack.agent.api.CreateNetrisVpcCommand;
2121
import org.apache.cloudstack.agent.api.CreateOrUpdateNetrisNatCommand;
2222
import org.apache.cloudstack.agent.api.DeleteNetrisVpcCommand;
23+
import org.apache.cloudstack.service.NetrisServiceImpl;
2324
import org.junit.Assert;
2425
import org.junit.Test;
2526

@@ -95,4 +96,14 @@ public void testSourceNatName() {
9596
String expectedName = String.format("D%s-A%s-Z%s-V%s-SNAT", domainId, accountId, zoneId, vpcId);
9697
Assert.assertEquals(expectedName, snatRuleName);
9798
}
99+
100+
@Test
101+
public void testStaticNatName() {
102+
long vmId = 1234L;
103+
CreateOrUpdateNetrisNatCommand cmd = new CreateOrUpdateNetrisNatCommand(zoneId, accountId, domainId, vpcName, vpcId, null, null, true, vpcCidr);
104+
String[] suffixes = NetrisServiceImpl.getStaticNatResourceSuffixes(vpcId, null, true, vmId);
105+
String staticNatRuleName = NetrisResourceObjectUtils.retrieveNetrisResourceObjectName(cmd, NetrisResourceObjectUtils.NetrisObjectType.STATICNAT, suffixes);
106+
String expectedName = String.format("D%s-A%s-Z%s-V%s-VM%s-STATICNAT", domainId, accountId, zoneId, vpcId, vmId);
107+
Assert.assertEquals(expectedName, staticNatRuleName);
108+
}
98109
}

‎server/src/test/java/org/apache/cloudstack/service/NetrisServiceMockTest.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,12 +82,12 @@ public boolean updateVpcSourceNatIp(Vpc vpc, IpAddress address) {
8282
}
8383

8484
@Override
85-
public boolean createStaticNatRule(long zoneId, long accountId, long domainId, String networkResourceName, Long networkResourceId, boolean isForVpc, String vpcCidr, String staticNatIp, String vmIp) {
85+
public boolean createStaticNatRule(long zoneId, long accountId, long domainId, String networkResourceName, Long networkResourceId, boolean isForVpc, String vpcCidr, String staticNatIp, String vmIp, long vmId) {
8686
return true;
8787
}
8888

8989
@Override
90-
public boolean deleteStaticNatRule(long zoneId, long accountId, long domainId, String networkResourceName, Long networkResourceId, boolean isForVpc, String staticNatIp) {
90+
public boolean deleteStaticNatRule(long zoneId, long accountId, long domainId, String networkResourceName, Long networkResourceId, boolean isForVpc, String staticNatIp, long vmId) {
9191
return true;
9292
}
9393

0 commit comments

Comments
 (0)