Skip to content

Commit 7bb2713

Browse files
winterhazelwinterhazel
committed
Address updatePortForwardingRule API
1 parent b5eaa5c commit 7bb2713

File tree

8 files changed

+93
-25
lines changed

8 files changed

+93
-25
lines changed

‎api/src/main/java/com/cloud/network/rules/RulesService.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import com.cloud.user.Account;
2727
import com.cloud.utils.Pair;
2828
import com.cloud.utils.net.Ip;
29+
import org.apache.cloudstack.api.command.user.firewall.UpdatePortForwardingRuleCmd;
2930

3031
public interface RulesService {
3132
Pair<List<? extends FirewallRule>, Integer> searchStaticNatRules(Long ipId, Long id, Long vmId, Long start, Long size, String accountName, Long domainId,
@@ -81,6 +82,8 @@ Pair<List<? extends FirewallRule>, Integer> searchStaticNatRules(Long ipId, Long
8182

8283
boolean disableStaticNat(long ipId) throws ResourceUnavailableException, NetworkRuleConflictException, InsufficientAddressCapacityException;
8384

84-
PortForwardingRule updatePortForwardingRule(long id, Integer privatePort, Integer privateEndPort, Long virtualMachineId, Ip vmGuestIp, String customId, Boolean forDisplay);
85+
PortForwardingRule updatePortForwardingRule(UpdatePortForwardingRuleCmd cmd);
86+
87+
void validatePortForwardingSourceCidrList(List<String> sourceCidrList);
8588

8689
}

‎api/src/main/java/org/apache/cloudstack/api/command/user/firewall/CreatePortForwardingRuleCmd.java‎

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@
3636
import org.apache.cloudstack.api.response.NetworkResponse;
3737
import org.apache.cloudstack.api.response.UserVmResponse;
3838
import org.apache.cloudstack.context.CallContext;
39-
import org.apache.commons.collections.CollectionUtils;
4039

4140
import com.cloud.event.EventTypes;
4241
import com.cloud.exception.InvalidParameterValueException;
@@ -112,7 +111,7 @@ public class CreatePortForwardingRuleCmd extends BaseAsyncCreateCmd implements P
112111
type = CommandType.LIST,
113112
collectionType = CommandType.STRING,
114113
description = " the source CIDR list to allow traffic from; all other CIDRs will be blocked. " +
115-
"Multiple entries must be separated by a single comma character (,). This param will be used only for VPC's networks. By default, all CIDRs are allowed.")
114+
"Multiple entries must be separated by a single comma character (,). This param will be used only for VPC tiers. By default, all CIDRs are allowed.")
116115
private List<String> sourceCidrList;
117116

118117
@Parameter(name = ApiConstants.OPEN_FIREWALL, type = CommandType.BOOLEAN, description = "if true, firewall rule for source/end public port is automatically created; "
@@ -342,7 +341,7 @@ public void create() {
342341
}
343342
}
344343

345-
validateSourceCidrList();
344+
_rulesService.validatePortForwardingSourceCidrList(sourceCidrList);
346345

347346
try {
348347
PortForwardingRule result = _rulesService.createPortForwardingRule(this, virtualMachineId, privateIp, getOpenFirewall(), isDisplay());
@@ -443,16 +442,4 @@ public String getName() {
443442
return null;
444443
}
445444

446-
public void validateSourceCidrList() {
447-
if (CollectionUtils.isEmpty(sourceCidrList)) {
448-
return;
449-
}
450-
451-
for (String cidr : sourceCidrList) {
452-
if (!NetUtils.isValidCidrList(cidr) && !NetUtils.isValidIp6Cidr(cidr)) {
453-
throw new InvalidParameterValueException(String.format("The given source CIDR [%s] is invalid.", cidr));
454-
}
455-
}
456-
}
457-
458445
}

‎api/src/main/java/org/apache/cloudstack/api/command/user/firewall/UpdatePortForwardingRuleCmd.java‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@
3333
import com.cloud.user.Account;
3434
import com.cloud.utils.net.Ip;
3535

36+
import java.util.List;
37+
3638
@APICommand(name = "updatePortForwardingRule",
3739
responseObject = FirewallRuleResponse.class,
3840
description = "Updates a port forwarding rule. Only the private port and the virtual machine can be updated.", entityType = {PortForwardingRule.class},
@@ -65,6 +67,13 @@ public class UpdatePortForwardingRuleCmd extends BaseAsyncCustomIdCmd {
6567
@Parameter(name = ApiConstants.FOR_DISPLAY, type = CommandType.BOOLEAN, description = "an optional field, whether to the display the rule to the end user or not", since = "4.4", authorized = {RoleType.Admin})
6668
private Boolean display;
6769

70+
@Parameter(name = ApiConstants.CIDR_LIST,
71+
type = CommandType.LIST,
72+
collectionType = CommandType.STRING,
73+
description = " the source CIDR list to allow traffic from; all other CIDRs will be blocked. " +
74+
"Multiple entries must be separated by a single comma character (,). This param will be used only for VPC tiers. By default, all CIDRs are allowed.")
75+
private List<String> sourceCidrList;
76+
6877
/////////////////////////////////////////////////////
6978
/////////////////// Accessors ///////////////////////
7079
/////////////////////////////////////////////////////
@@ -96,6 +105,10 @@ public Boolean getDisplay() {
96105
return display;
97106
}
98107

108+
public List<String> getSourceCidrList() {
109+
return sourceCidrList;
110+
}
111+
99112
/////////////////////////////////////////////////////
100113
/////////////// API Implementation///////////////////
101114
/////////////////////////////////////////////////////
@@ -132,7 +145,7 @@ public void checkUuid() {
132145

133146
@Override
134147
public void execute() {
135-
PortForwardingRule rule = _rulesService.updatePortForwardingRule(getId(), getPrivatePort(), getPrivateEndPort(), getVirtualMachineId(), getVmGuestIp(), getCustomId(), getDisplay());
148+
PortForwardingRule rule = _rulesService.updatePortForwardingRule(this);
136149
FirewallRuleResponse fwResponse = new FirewallRuleResponse();
137150
if (rule != null) {
138151
fwResponse = _responseGenerator.createPortForwardingRuleResponse(rule);

‎engine/schema/src/main/java/com/cloud/network/dao/FirewallRulesCidrsDao.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,4 +29,6 @@ public interface FirewallRulesCidrsDao extends GenericDao<FirewallRulesCidrsVO,
2929

3030
@DB
3131
List<FirewallRulesCidrsVO> listByFirewallRuleId(long firewallRuleId);
32+
33+
void updateSourceCidrsForRule(Long firewallRuleId, List<String> sourceCidrList);
3234
}

‎engine/schema/src/main/java/com/cloud/network/dao/FirewallRulesCidrsDaoImpl.java‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import java.util.List;
2121

2222

23+
import org.apache.commons.collections4.CollectionUtils;
2324
import org.apache.log4j.Logger;
2425
import org.springframework.stereotype.Component;
2526

@@ -65,9 +66,27 @@ public List<FirewallRulesCidrsVO> listByFirewallRuleId(long firewallRuleId) {
6566
return results;
6667
}
6768

69+
@Override
70+
public void updateSourceCidrsForRule(Long firewallRuleId, List<String> sourceCidrList) {
71+
TransactionLegacy txn = TransactionLegacy.currentTxn();
72+
txn.start();
73+
74+
SearchCriteria<FirewallRulesCidrsVO> sc = CidrsSearch.create();
75+
sc.setParameters("firewallRuleId", firewallRuleId);
76+
remove(sc);
77+
78+
persist(firewallRuleId, sourceCidrList);
79+
80+
txn.commit();
81+
}
82+
6883
@Override
6984
@DB
7085
public void persist(long firewallRuleId, List<String> sourceCidrs) {
86+
if (CollectionUtils.isEmpty(sourceCidrs)) {
87+
return;
88+
}
89+
7190
TransactionLegacy txn = TransactionLegacy.currentTxn();
7291

7392
txn.start();

‎engine/schema/src/main/java/com/cloud/network/dao/FirewallRulesDaoImpl.java‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -243,9 +243,6 @@ public FirewallRuleVO persist(FirewallRuleVO firewallRule) {
243243
}
244244

245245
public void saveSourceCidrs(FirewallRuleVO firewallRule, List<String> cidrList) {
246-
if (cidrList == null) {
247-
return;
248-
}
249246
_firewallRulesCidrsDao.persist(firewallRule.getId(), cidrList);
250247
}
251248

‎engine/schema/src/main/java/com/cloud/network/rules/dao/PortForwardingRulesDaoImpl.java‎

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222

2323
import com.cloud.utils.db.Transaction;
2424
import com.cloud.utils.db.TransactionCallback;
25+
import com.cloud.utils.db.TransactionLegacy;
2526
import org.apache.commons.collections.CollectionUtils;
2627
import org.springframework.stereotype.Component;
2728

@@ -178,13 +179,29 @@ public PortForwardingRuleVO findByIdAndIp(long id, String secondaryIp) {
178179
public PortForwardingRuleVO persist(PortForwardingRuleVO portForwardingRule) {
179180
return Transaction.execute((TransactionCallback<PortForwardingRuleVO>) transactionStatus -> {
180181
PortForwardingRuleVO dbPfRule = super.persist(portForwardingRule);
181-
if (CollectionUtils.isNotEmpty(portForwardingRule.getSourceCidrList())) {
182-
portForwardingRulesCidrsDao.persist(portForwardingRule.getId(), portForwardingRule.getSourceCidrList());
183-
}
182+
183+
portForwardingRulesCidrsDao.persist(portForwardingRule.getId(), portForwardingRule.getSourceCidrList());
184184
List<String> cidrList = portForwardingRulesCidrsDao.getSourceCidrs(portForwardingRule.getId());
185185
portForwardingRule.setSourceCidrList(cidrList);
186+
186187
return dbPfRule;
187188
});
188189

189190
}
191+
192+
@Override
193+
public boolean update(Long id, PortForwardingRuleVO entity) {
194+
TransactionLegacy txn = TransactionLegacy.currentTxn();
195+
txn.start();
196+
197+
boolean success = super.update(id, entity);
198+
if (!success) {
199+
return false;
200+
}
201+
202+
portForwardingRulesCidrsDao.updateSourceCidrsForRule(entity.getId(), entity.getSourceCidrList());
203+
txn.commit();
204+
205+
return true;
206+
}
190207
}

‎server/src/main/java/com/cloud/network/rules/RulesManagerImpl.java‎

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import com.cloud.vm.VirtualMachineProfileImpl;
3434
import org.apache.cloudstack.acl.SecurityChecker;
3535
import org.apache.cloudstack.api.command.user.firewall.ListPortForwardingRulesCmd;
36+
import org.apache.cloudstack.api.command.user.firewall.UpdatePortForwardingRuleCmd;
3637
import org.apache.cloudstack.context.CallContext;
3738
import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService;
3839
import org.apache.log4j.Logger;
@@ -103,6 +104,7 @@
103104
import com.cloud.vm.dao.NicSecondaryIpVO;
104105
import com.cloud.vm.dao.UserVmDao;
105106
import com.cloud.vm.dao.VMInstanceDao;
107+
import org.apache.commons.collections.CollectionUtils;
106108

107109
public class RulesManagerImpl extends ManagerBase implements RulesManager, RulesService {
108110
private static final Logger s_logger = Logger.getLogger(RulesManagerImpl.class);
@@ -1578,12 +1580,22 @@ public List<FirewallRuleVO> listAssociatedRulesForGuestNic(Nic nic) {
15781580

15791581
@Override
15801582
@ActionEvent(eventType = EventTypes.EVENT_NET_RULE_MODIFY, eventDescription = "updating forwarding rule", async = true)
1581-
public PortForwardingRule updatePortForwardingRule(long id, Integer privatePort, Integer privateEndPort, Long virtualMachineId, Ip vmGuestIp, String customId, Boolean forDisplay) {
1582-
Account caller = CallContext.current().getCallingAccount();
1583+
public PortForwardingRule updatePortForwardingRule(UpdatePortForwardingRuleCmd cmd) {
1584+
long id = cmd.getId();
1585+
Integer privatePort = cmd.getPrivatePort();
1586+
Integer privateEndPort = cmd.getPrivateEndPort();
1587+
Long virtualMachineId = cmd.getVirtualMachineId();
1588+
Ip vmGuestIp = cmd.getVmGuestIp();
1589+
String customId = cmd.getCustomId();
1590+
Boolean forDisplay = cmd.getDisplay();
1591+
List<String> sourceCidrList = cmd.getSourceCidrList();
1592+
15831593
PortForwardingRuleVO rule = _portForwardingDao.findById(id);
15841594
if (rule == null) {
15851595
throw new InvalidParameterValueException("Unable to find " + id);
15861596
}
1597+
1598+
Account caller = CallContext.current().getCallingAccount();
15871599
_accountMgr.checkAccess(caller, null, true, rule);
15881600

15891601
if (customId != null) {
@@ -1671,6 +1683,11 @@ public PortForwardingRule updatePortForwardingRule(long id, Integer privatePort,
16711683
rule.setVirtualMachineId(virtualMachineId);
16721684
rule.setDestinationIpAddress(dstIp);
16731685
}
1686+
1687+
if (CollectionUtils.isNotEmpty(sourceCidrList)) {
1688+
rule.setSourceCidrList(sourceCidrList);
1689+
}
1690+
16741691
_portForwardingDao.update(id, rule);
16751692

16761693
//apply new rules
@@ -1680,4 +1697,17 @@ public PortForwardingRule updatePortForwardingRule(long id, Integer privatePort,
16801697

16811698
return _portForwardingDao.findById(id);
16821699
}
1700+
1701+
@Override
1702+
public void validatePortForwardingSourceCidrList(List<String> sourceCidrList) {
1703+
if (CollectionUtils.isEmpty(sourceCidrList)) {
1704+
return;
1705+
}
1706+
1707+
for (String cidr : sourceCidrList) {
1708+
if (!NetUtils.isValidCidrList(cidr) && !NetUtils.isValidIp6Cidr(cidr)) {
1709+
throw new InvalidParameterValueException(String.format("The given source CIDR [%s] is invalid.", cidr));
1710+
}
1711+
}
1712+
}
16831713
}

0 commit comments

Comments
 (0)