This is an automated email from the ASF dual-hosted git repository.
dahn pushed a commit to branch 4.19
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/4.19 by this push:
new 4189bac8e0d enable to create VPC portfowarding rules with source cidr
(#7081)
4189bac8e0d is described below
commit 4189bac8e0d755a65c10ab07868686bba0128bac
Author: Rodrigo D. Lopez <[email protected]>
AuthorDate: Thu Nov 28 13:53:07 2024 -0300
enable to create VPC portfowarding rules with source cidr (#7081)
Co-authored-by: Lopez <[email protected]>
Co-authored-by: Fabricio Duarte <[email protected]>
---
.../cloud/agent/api/to/PortForwardingRuleTO.java | 13 +++
.../java/com/cloud/network/rules/RulesService.java | 5 +-
.../org/apache/cloudstack/api/ApiConstants.java | 1 +
.../user/firewall/CreatePortForwardingRuleCmd.java | 22 ++---
.../user/firewall/UpdatePortForwardingRuleCmd.java | 15 +++-
.../loadbalancer/CreateLoadBalancerRuleCmd.java | 2 +-
.../facade/SetPortForwardingRulesConfigItem.java | 2 +-
.../virtualnetwork/model/ForwardingRule.java | 9 +-
.../cloud/network/dao/FirewallRulesCidrsDao.java | 2 +
.../network/dao/FirewallRulesCidrsDaoImpl.java | 19 ++++
.../cloud/network/dao/FirewallRulesDaoImpl.java | 3 -
.../cloud/network/rules/PortForwardingRuleVO.java | 23 +++--
.../rules/dao/PortForwardingRulesDaoImpl.java | 45 +++++++++-
...ernetesClusterResourceModifierActionWorker.java | 2 +-
.../network/firewall/FirewallManagerImpl.java | 75 ++++++++++++----
.../cloud/network/router/CommandSetupHelper.java | 2 +
.../com/cloud/network/rules/RulesManagerImpl.java | 52 +++++++++--
.../network/firewall/FirewallManagerTest.java | 100 ++++++++++++++++++++-
systemvm/debian/opt/cloud/bin/configure.py | 10 ++-
systemvm/debian/opt/cloud/bin/cs/CsNetfilter.py | 2 +-
.../debian/opt/cloud/bin/cs_forwardingrules.py | 22 ++++-
ui/public/locales/en.json | 1 +
ui/public/locales/pt_BR.json | 3 +-
ui/src/views/network/LoadBalancing.vue | 6 +-
ui/src/views/network/PortForwarding.vue | 27 +++++-
25 files changed, 396 insertions(+), 67 deletions(-)
diff --git a/api/src/main/java/com/cloud/agent/api/to/PortForwardingRuleTO.java
b/api/src/main/java/com/cloud/agent/api/to/PortForwardingRuleTO.java
index 76d6d952814..d43625c09a9 100644
--- a/api/src/main/java/com/cloud/agent/api/to/PortForwardingRuleTO.java
+++ b/api/src/main/java/com/cloud/agent/api/to/PortForwardingRuleTO.java
@@ -19,6 +19,9 @@ package com.cloud.agent.api.to;
import com.cloud.network.rules.FirewallRule;
import com.cloud.network.rules.PortForwardingRule;
import com.cloud.utils.net.NetUtils;
+import org.apache.commons.lang3.StringUtils;
+
+import java.util.List;
/**
* PortForwardingRuleTO specifies one port forwarding rule.
@@ -29,6 +32,8 @@ public class PortForwardingRuleTO extends FirewallRuleTO {
String dstIp;
int[] dstPortRange;
+ List<String> sourceCidrList;
+
protected PortForwardingRuleTO() {
super();
}
@@ -37,6 +42,7 @@ public class PortForwardingRuleTO extends FirewallRuleTO {
super(rule, srcVlanTag, srcIp);
this.dstIp = rule.getDestinationIpAddress().addr();
this.dstPortRange = new int[] {rule.getDestinationPortStart(),
rule.getDestinationPortEnd()};
+ this.sourceCidrList = rule.getSourceCidrList();
}
public PortForwardingRuleTO(long id, String srcIp, int srcPortStart, int
srcPortEnd, String dstIp, int dstPortStart, int dstPortEnd, String protocol,
@@ -58,4 +64,11 @@ public class PortForwardingRuleTO extends FirewallRuleTO {
return NetUtils.portRangeToString(dstPortRange);
}
+ public String getSourceCidrListAsString() {
+ if (sourceCidrList != null) {
+ return StringUtils.join(sourceCidrList, ",");
+ }
+ return null;
+ }
+
}
diff --git a/api/src/main/java/com/cloud/network/rules/RulesService.java
b/api/src/main/java/com/cloud/network/rules/RulesService.java
index 0b4afeef945..547d4ab51e0 100644
--- a/api/src/main/java/com/cloud/network/rules/RulesService.java
+++ b/api/src/main/java/com/cloud/network/rules/RulesService.java
@@ -26,6 +26,7 @@ import com.cloud.exception.ResourceUnavailableException;
import com.cloud.user.Account;
import com.cloud.utils.Pair;
import com.cloud.utils.net.Ip;
+import
org.apache.cloudstack.api.command.user.firewall.UpdatePortForwardingRuleCmd;
public interface RulesService {
Pair<List<? extends FirewallRule>, Integer> searchStaticNatRules(Long
ipId, Long id, Long vmId, Long start, Long size, String accountName, Long
domainId,
@@ -81,6 +82,8 @@ public interface RulesService {
boolean disableStaticNat(long ipId) throws ResourceUnavailableException,
NetworkRuleConflictException, InsufficientAddressCapacityException;
- PortForwardingRule updatePortForwardingRule(long id, Integer privatePort,
Integer privateEndPort, Long virtualMachineId, Ip vmGuestIp, String customId,
Boolean forDisplay);
+ PortForwardingRule updatePortForwardingRule(UpdatePortForwardingRuleCmd
cmd);
+
+ void validatePortForwardingSourceCidrList(List<String> sourceCidrList);
}
diff --git a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java
b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java
index b9a4938b5b2..b2042c116a7 100644
--- a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java
+++ b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java
@@ -427,6 +427,7 @@ public class ApiConstants {
public static final String SNAPSHOT_TYPE = "snapshottype";
public static final String SNAPSHOT_QUIESCEVM = "quiescevm";
public static final String SUPPORTS_STORAGE_SNAPSHOT =
"supportsstoragesnapshot";
+ public static final String SOURCE_CIDR_LIST = "sourcecidrlist";
public static final String SOURCE_ZONE_ID = "sourcezoneid";
public static final String START_DATE = "startdate";
public static final String START_ID = "startid";
diff --git
a/api/src/main/java/org/apache/cloudstack/api/command/user/firewall/CreatePortForwardingRuleCmd.java
b/api/src/main/java/org/apache/cloudstack/api/command/user/firewall/CreatePortForwardingRuleCmd.java
index 5e136210150..05d627fe577 100644
---
a/api/src/main/java/org/apache/cloudstack/api/command/user/firewall/CreatePortForwardingRuleCmd.java
+++
b/api/src/main/java/org/apache/cloudstack/api/command/user/firewall/CreatePortForwardingRuleCmd.java
@@ -107,8 +107,12 @@ public class CreatePortForwardingRuleCmd extends
BaseAsyncCreateCmd implements P
description = "the ID of the virtual machine for the port
forwarding rule")
private Long virtualMachineId;
- @Parameter(name = ApiConstants.CIDR_LIST, type = CommandType.LIST,
collectionType = CommandType.STRING, description = "the cidr list to forward
traffic from. Multiple entries must be separated by a single comma character
(,). This parameter is deprecated. Do not use.")
- private List<String> cidrlist;
+ @Parameter(name = ApiConstants.CIDR_LIST,
+ type = CommandType.LIST,
+ collectionType = CommandType.STRING,
+ description = " the source CIDR list to allow traffic from; all
other CIDRs will be blocked. " +
+ "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.")
+ private List<String> sourceCidrList;
@Parameter(name = ApiConstants.OPEN_FIREWALL, type = CommandType.BOOLEAN,
description = "if true, firewall rule for source/end public port is
automatically created; "
+ "if false - firewall rule has to be created explicitly. If not
specified 1) defaulted to false when PF"
@@ -157,11 +161,7 @@ public class CreatePortForwardingRuleCmd extends
BaseAsyncCreateCmd implements P
@Override
public List<String> getSourceCidrList() {
- if (cidrlist != null) {
- throw new InvalidParameterValueException("Parameter cidrList is
deprecated; if you need to open firewall "
- + "rule for the specific cidr, please refer to
createFirewallRule command");
- }
- return null;
+ return sourceCidrList;
}
public Boolean getOpenFirewall() {
@@ -334,12 +334,6 @@ public class CreatePortForwardingRuleCmd extends
BaseAsyncCreateCmd implements P
@Override
public void create() {
- // cidr list parameter is deprecated
- if (cidrlist != null) {
- throw new InvalidParameterValueException(
- "Parameter cidrList is deprecated; if you need to open
firewall rule for the specific cidr, please refer to createFirewallRule
command");
- }
-
Ip privateIp = getVmSecondaryIp();
if (privateIp != null) {
if (!NetUtils.isValidIp4(privateIp.toString())) {
@@ -347,6 +341,8 @@ public class CreatePortForwardingRuleCmd extends
BaseAsyncCreateCmd implements P
}
}
+ _rulesService.validatePortForwardingSourceCidrList(sourceCidrList);
+
try {
PortForwardingRule result =
_rulesService.createPortForwardingRule(this, virtualMachineId, privateIp,
getOpenFirewall(), isDisplay());
setEntityId(result.getId());
diff --git
a/api/src/main/java/org/apache/cloudstack/api/command/user/firewall/UpdatePortForwardingRuleCmd.java
b/api/src/main/java/org/apache/cloudstack/api/command/user/firewall/UpdatePortForwardingRuleCmd.java
index 2afc0bb66ad..da02aa4b174 100644
---
a/api/src/main/java/org/apache/cloudstack/api/command/user/firewall/UpdatePortForwardingRuleCmd.java
+++
b/api/src/main/java/org/apache/cloudstack/api/command/user/firewall/UpdatePortForwardingRuleCmd.java
@@ -33,6 +33,8 @@ import com.cloud.network.rules.PortForwardingRule;
import com.cloud.user.Account;
import com.cloud.utils.net.Ip;
+import java.util.List;
+
@APICommand(name = "updatePortForwardingRule",
responseObject = FirewallRuleResponse.class,
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 {
@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})
private Boolean display;
+ @Parameter(name = ApiConstants.CIDR_LIST,
+ type = CommandType.LIST,
+ collectionType = CommandType.STRING,
+ description = " the source CIDR list to allow traffic from; all
other CIDRs will be blocked. " +
+ "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.")
+ private List<String> sourceCidrList;
+
/////////////////////////////////////////////////////
/////////////////// Accessors ///////////////////////
/////////////////////////////////////////////////////
@@ -96,6 +105,10 @@ public class UpdatePortForwardingRuleCmd extends
BaseAsyncCustomIdCmd {
return display;
}
+ public List<String> getSourceCidrList() {
+ return sourceCidrList;
+ }
+
/////////////////////////////////////////////////////
/////////////// API Implementation///////////////////
/////////////////////////////////////////////////////
@@ -132,7 +145,7 @@ public class UpdatePortForwardingRuleCmd extends
BaseAsyncCustomIdCmd {
@Override
public void execute() {
- PortForwardingRule rule =
_rulesService.updatePortForwardingRule(getId(), getPrivatePort(),
getPrivateEndPort(), getVirtualMachineId(), getVmGuestIp(), getCustomId(),
getDisplay());
+ PortForwardingRule rule = _rulesService.updatePortForwardingRule(this);
FirewallRuleResponse fwResponse = new FirewallRuleResponse();
if (rule != null) {
fwResponse =
_responseGenerator.createPortForwardingRuleResponse(rule);
diff --git
a/api/src/main/java/org/apache/cloudstack/api/command/user/loadbalancer/CreateLoadBalancerRuleCmd.java
b/api/src/main/java/org/apache/cloudstack/api/command/user/loadbalancer/CreateLoadBalancerRuleCmd.java
index ef9e46f3a76..b741310cb16 100644
---
a/api/src/main/java/org/apache/cloudstack/api/command/user/loadbalancer/CreateLoadBalancerRuleCmd.java
+++
b/api/src/main/java/org/apache/cloudstack/api/command/user/loadbalancer/CreateLoadBalancerRuleCmd.java
@@ -106,7 +106,7 @@ public class CreateLoadBalancerRuleCmd extends
BaseAsyncCreateCmd /*implements L
@Parameter(name = ApiConstants.DOMAIN_ID, type = CommandType.UUID,
entityType = DomainResponse.class, description = "the domain ID associated with
the load balancer")
private Long domainId;
- @Parameter(name = ApiConstants.CIDR_LIST, type = CommandType.LIST,
collectionType = CommandType.STRING, since = "4.18.0.0", description = "the
CIDR list to allow traffic, "
+ @Parameter(name = ApiConstants.CIDR_LIST, type = CommandType.LIST,
collectionType = CommandType.STRING, since = "4.18.0.0", description = "the
source CIDR list to allow traffic from; "
+ "all other CIDRs will be blocked. Multiple entries must be
separated by a single comma character (,). By default, all CIDRs are allowed.")
private List<String> cidrlist;
diff --git
a/core/src/main/java/com/cloud/agent/resource/virtualnetwork/facade/SetPortForwardingRulesConfigItem.java
b/core/src/main/java/com/cloud/agent/resource/virtualnetwork/facade/SetPortForwardingRulesConfigItem.java
index c9d0e74e2e4..4daef64ed8a 100644
---
a/core/src/main/java/com/cloud/agent/resource/virtualnetwork/facade/SetPortForwardingRulesConfigItem.java
+++
b/core/src/main/java/com/cloud/agent/resource/virtualnetwork/facade/SetPortForwardingRulesConfigItem.java
@@ -41,7 +41,7 @@ public class SetPortForwardingRulesConfigItem extends
AbstractConfigItemFacade {
for (final PortForwardingRuleTO rule : command.getRules()) {
final ForwardingRule fwdRule = new ForwardingRule(rule.revoked(),
rule.getProtocol().toLowerCase(), rule.getSrcIp(),
rule.getStringSrcPortRange(), rule.getDstIp(),
- rule.getStringDstPortRange());
+ rule.getStringDstPortRange(),
rule.getSourceCidrListAsString());
rules.add(fwdRule);
}
diff --git
a/core/src/main/java/com/cloud/agent/resource/virtualnetwork/model/ForwardingRule.java
b/core/src/main/java/com/cloud/agent/resource/virtualnetwork/model/ForwardingRule.java
index cf3e43d1c01..1dc1cc855c4 100644
---
a/core/src/main/java/com/cloud/agent/resource/virtualnetwork/model/ForwardingRule.java
+++
b/core/src/main/java/com/cloud/agent/resource/virtualnetwork/model/ForwardingRule.java
@@ -26,18 +26,21 @@ public class ForwardingRule {
private String sourcePortRange;
private String destinationIpAddress;
private String destinationPortRange;
+ private String sourceCidrList;
public ForwardingRule() {
// Empty constructor for (de)serialization
}
- public ForwardingRule(boolean revoke, String protocol, String
sourceIpAddress, String sourcePortRange, String destinationIpAddress, String
destinationPortRange) {
+ public ForwardingRule(boolean revoke, String protocol, String
sourceIpAddress, String sourcePortRange, String destinationIpAddress, String
destinationPortRange,
+ String sourceCidrList) {
this.revoke = revoke;
this.protocol = protocol;
this.sourceIpAddress = sourceIpAddress;
this.sourcePortRange = sourcePortRange;
this.destinationIpAddress = destinationIpAddress;
this.destinationPortRange = destinationPortRange;
+ this.sourceCidrList = sourceCidrList;
}
public boolean isRevoke() {
@@ -88,4 +91,8 @@ public class ForwardingRule {
this.destinationPortRange = destinationPortRange;
}
+ public String getSourceCidrList() {
+ return sourceCidrList;
+ }
+
}
diff --git
a/engine/schema/src/main/java/com/cloud/network/dao/FirewallRulesCidrsDao.java
b/engine/schema/src/main/java/com/cloud/network/dao/FirewallRulesCidrsDao.java
index 55c454860ef..df5b6b5d647 100644
---
a/engine/schema/src/main/java/com/cloud/network/dao/FirewallRulesCidrsDao.java
+++
b/engine/schema/src/main/java/com/cloud/network/dao/FirewallRulesCidrsDao.java
@@ -29,4 +29,6 @@ public interface FirewallRulesCidrsDao extends
GenericDao<FirewallRulesCidrsVO,
@DB
List<FirewallRulesCidrsVO> listByFirewallRuleId(long firewallRuleId);
+
+ void updateSourceCidrsForRule(Long firewallRuleId, List<String>
sourceCidrList);
}
diff --git
a/engine/schema/src/main/java/com/cloud/network/dao/FirewallRulesCidrsDaoImpl.java
b/engine/schema/src/main/java/com/cloud/network/dao/FirewallRulesCidrsDaoImpl.java
index f6185309972..4630925ddee 100644
---
a/engine/schema/src/main/java/com/cloud/network/dao/FirewallRulesCidrsDaoImpl.java
+++
b/engine/schema/src/main/java/com/cloud/network/dao/FirewallRulesCidrsDaoImpl.java
@@ -20,6 +20,7 @@ import java.util.ArrayList;
import java.util.List;
+import org.apache.commons.collections4.CollectionUtils;
import org.apache.log4j.Logger;
import org.springframework.stereotype.Component;
@@ -65,9 +66,27 @@ public class FirewallRulesCidrsDaoImpl extends
GenericDaoBase<FirewallRulesCidrs
return results;
}
+ @Override
+ public void updateSourceCidrsForRule(Long firewallRuleId, List<String>
sourceCidrList) {
+ TransactionLegacy txn = TransactionLegacy.currentTxn();
+ txn.start();
+
+ SearchCriteria<FirewallRulesCidrsVO> sc = CidrsSearch.create();
+ sc.setParameters("firewallRuleId", firewallRuleId);
+ remove(sc);
+
+ persist(firewallRuleId, sourceCidrList);
+
+ txn.commit();
+ }
+
@Override
@DB
public void persist(long firewallRuleId, List<String> sourceCidrs) {
+ if (CollectionUtils.isEmpty(sourceCidrs)) {
+ return;
+ }
+
TransactionLegacy txn = TransactionLegacy.currentTxn();
txn.start();
diff --git
a/engine/schema/src/main/java/com/cloud/network/dao/FirewallRulesDaoImpl.java
b/engine/schema/src/main/java/com/cloud/network/dao/FirewallRulesDaoImpl.java
index 3ac860b08c5..f18fb98f3de 100644
---
a/engine/schema/src/main/java/com/cloud/network/dao/FirewallRulesDaoImpl.java
+++
b/engine/schema/src/main/java/com/cloud/network/dao/FirewallRulesDaoImpl.java
@@ -243,9 +243,6 @@ public class FirewallRulesDaoImpl extends
GenericDaoBase<FirewallRuleVO, Long> i
}
public void saveSourceCidrs(FirewallRuleVO firewallRule, List<String>
cidrList) {
- if (cidrList == null) {
- return;
- }
_firewallRulesCidrsDao.persist(firewallRule.getId(), cidrList);
}
diff --git
a/engine/schema/src/main/java/com/cloud/network/rules/PortForwardingRuleVO.java
b/engine/schema/src/main/java/com/cloud/network/rules/PortForwardingRuleVO.java
index e1a698881f3..28d12fb24b4 100644
---
a/engine/schema/src/main/java/com/cloud/network/rules/PortForwardingRuleVO.java
+++
b/engine/schema/src/main/java/com/cloud/network/rules/PortForwardingRuleVO.java
@@ -25,6 +25,7 @@ import javax.persistence.EnumType;
import javax.persistence.Enumerated;
import javax.persistence.PrimaryKeyJoinColumn;
import javax.persistence.Table;
+import javax.persistence.Transient;
import com.cloud.utils.net.Ip;
@@ -47,21 +48,20 @@ public class PortForwardingRuleVO extends FirewallRuleVO
implements PortForwardi
@Column(name = "instance_id")
private long virtualMachineId;
+ @Transient
+ List<String> sourceCidrs;
+
public PortForwardingRuleVO() {
}
public PortForwardingRuleVO(String xId, long srcIpId, int srcPortStart,
int srcPortEnd, Ip dstIp, int dstPortStart, int dstPortEnd, String protocol,
long networkId,
- long accountId, long domainId, long instanceId) {
- super(xId, srcIpId, srcPortStart, srcPortEnd, protocol, networkId,
accountId, domainId, Purpose.PortForwarding, null, null, null, null, null);
+ long accountId, long domainId, long
instanceId, List<String> sourceCidrs) {
+ super(xId, srcIpId, srcPortStart, srcPortEnd, protocol, networkId,
accountId, domainId, Purpose.PortForwarding, sourceCidrs, null, null, null,
null);
this.destinationIpAddress = dstIp;
this.virtualMachineId = instanceId;
this.destinationPortStart = dstPortStart;
this.destinationPortEnd = dstPortEnd;
- }
-
- public PortForwardingRuleVO(String xId, long srcIpId, int srcPort, Ip
dstIp, int dstPort, String protocol, List<String> sourceCidrs, long networkId,
long accountId,
- long domainId, long instanceId) {
- this(xId, srcIpId, srcPort, srcPort, dstIp, dstPort, dstPort,
protocol.toLowerCase(), networkId, accountId, domainId, instanceId);
+ this.sourceCidrs = sourceCidrs;
}
@Override
@@ -106,4 +106,13 @@ public class PortForwardingRuleVO extends FirewallRuleVO
implements PortForwardi
return null;
}
+ public void setSourceCidrList(List<String> sourceCidrs) {
+ this.sourceCidrs = sourceCidrs;
+ }
+
+ @Override
+ public List<String> getSourceCidrList() {
+ return sourceCidrs;
+ }
+
}
diff --git
a/engine/schema/src/main/java/com/cloud/network/rules/dao/PortForwardingRulesDaoImpl.java
b/engine/schema/src/main/java/com/cloud/network/rules/dao/PortForwardingRulesDaoImpl.java
index 29cba516d72..1ddecaa8a17 100644
---
a/engine/schema/src/main/java/com/cloud/network/rules/dao/PortForwardingRulesDaoImpl.java
+++
b/engine/schema/src/main/java/com/cloud/network/rules/dao/PortForwardingRulesDaoImpl.java
@@ -20,6 +20,9 @@ import java.util.List;
import javax.inject.Inject;
+import com.cloud.utils.db.Transaction;
+import com.cloud.utils.db.TransactionCallback;
+import com.cloud.utils.db.TransactionLegacy;
import org.springframework.stereotype.Component;
import com.cloud.network.dao.FirewallRulesCidrsDao;
@@ -41,7 +44,7 @@ public class PortForwardingRulesDaoImpl extends
GenericDaoBase<PortForwardingRul
protected final SearchBuilder<PortForwardingRuleVO>
ActiveRulesSearchByAccount;
@Inject
- protected FirewallRulesCidrsDao _portForwardingRulesCidrsDao;
+ protected FirewallRulesCidrsDao portForwardingRulesCidrsDao;
protected PortForwardingRulesDaoImpl() {
super();
@@ -170,4 +173,44 @@ public class PortForwardingRulesDaoImpl extends
GenericDaoBase<PortForwardingRul
sc.setParameters("dstIp", secondaryIp);
return findOneBy(sc);
}
+
+ @Override
+ public PortForwardingRuleVO persist(PortForwardingRuleVO
portForwardingRule) {
+ return Transaction.execute((TransactionCallback<PortForwardingRuleVO>)
transactionStatus -> {
+ PortForwardingRuleVO dbPfRule = super.persist(portForwardingRule);
+
+ portForwardingRulesCidrsDao.persist(portForwardingRule.getId(),
portForwardingRule.getSourceCidrList());
+ List<String> cidrList =
portForwardingRulesCidrsDao.getSourceCidrs(portForwardingRule.getId());
+ portForwardingRule.setSourceCidrList(cidrList);
+
+ return dbPfRule;
+ });
+
+ }
+
+ @Override
+ public boolean update(Long id, PortForwardingRuleVO entity) {
+ TransactionLegacy txn = TransactionLegacy.currentTxn();
+ txn.start();
+
+ boolean success = super.update(id, entity);
+ if (!success) {
+ return false;
+ }
+
+ portForwardingRulesCidrsDao.updateSourceCidrsForRule(entity.getId(),
entity.getSourceCidrList());
+ txn.commit();
+
+ return true;
+ }
+
+ @Override
+ public PortForwardingRuleVO findById(Long id) {
+ PortForwardingRuleVO rule = super.findById(id);
+
+ List<String> sourceCidrList =
portForwardingRulesCidrsDao.getSourceCidrs(id);
+ rule.setSourceCidrList(sourceCidrList);
+
+ return rule;
+ }
}
diff --git
a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorker.java
b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorker.java
index d721db4b6b4..60cd9a2dff4 100644
---
a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorker.java
+++
b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorker.java
@@ -472,7 +472,7 @@ public class KubernetesClusterResourceModifierActionWorker
extends KubernetesClu
sourcePort, sourcePort,
vmIp,
destPort, destPort,
- "tcp", networkId, accountId, domainId, vmId);
+ "tcp", networkId, accountId, domainId, vmId, null);
newRule.setDisplay(true);
newRule.setState(FirewallRule.State.Add);
newRule = portForwardingRulesDao.persist(newRule);
diff --git
a/server/src/main/java/com/cloud/network/firewall/FirewallManagerImpl.java
b/server/src/main/java/com/cloud/network/firewall/FirewallManagerImpl.java
index 4ed480ea68b..4d4a8b30a56 100644
--- a/server/src/main/java/com/cloud/network/firewall/FirewallManagerImpl.java
+++ b/server/src/main/java/com/cloud/network/firewall/FirewallManagerImpl.java
@@ -33,6 +33,7 @@ import
org.apache.cloudstack.api.command.user.ipv6.ListIpv6FirewallRulesCmd;
import org.apache.cloudstack.context.CallContext;
import
org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService;
import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
+import org.apache.commons.lang3.ObjectUtils;
import org.apache.log4j.Logger;
import org.springframework.stereotype.Component;
@@ -391,20 +392,34 @@ public class FirewallManagerImpl extends ManagerBase
implements FirewallService,
((rule.getPurpose() == Purpose.Firewall ||
newRule.getPurpose() == Purpose.Firewall) && ((newRule.getPurpose() !=
rule.getPurpose()) || (!newRule.getProtocol()
.equalsIgnoreCase(rule.getProtocol()))));
- // if both rules are firewall and their cidrs are different, we
can skip port ranges verification
- boolean bothRulesFirewall = (rule.getPurpose() ==
newRule.getPurpose() && rule.getPurpose() == Purpose.Firewall);
+ // if both rules are firewall/port forwarding and their cidrs are
different, we can skip port ranges verification
boolean duplicatedCidrs = false;
+
+ boolean bothRulesFirewall = (rule.getPurpose() ==
newRule.getPurpose() && rule.getPurpose() == Purpose.Firewall);
if (bothRulesFirewall) {
_firewallDao.loadSourceCidrs(rule);
_firewallDao.loadSourceCidrs((FirewallRuleVO)newRule);
+ if (ObjectUtils.anyNull(rule.getSourceCidrList(),
newRule.getSourceCidrList())) {
+ continue;
+ }
+
_firewallDao.loadDestinationCidrs(rule);
_firewallDao.loadDestinationCidrs((FirewallRuleVO) newRule);
- if (rule.getSourceCidrList() == null ||
newRule.getSourceCidrList() == null) {
+ duplicatedCidrs =
detectConflictingCidrs(rule.getSourceCidrList(), newRule.getSourceCidrList())
&& detectConflictingCidrs(rule.getDestinationCidrList(),
newRule.getDestinationCidrList());
+ }
+
+ boolean bothRulesPortForwarding = rule.getPurpose() ==
newRule.getPurpose() && rule.getPurpose() == Purpose.PortForwarding;
+ if (bothRulesPortForwarding) {
+ _firewallDao.loadSourceCidrs(rule);
+ _firewallDao.loadSourceCidrs((FirewallRuleVO) newRule);
+
+ if (ObjectUtils.anyNull(rule.getSourceCidrList(),
newRule.getSourceCidrList())) {
continue;
}
- duplicatedCidrs =
(detectConflictingCidrs(rule.getSourceCidrList(), newRule.getSourceCidrList())
&& detectConflictingCidrs(rule.getDestinationCidrList(),
newRule.getDestinationCidrList()));
+
+ duplicatedCidrs =
detectConflictingCidrs(rule.getSourceCidrList(), newRule.getSourceCidrList());
}
if (!oneOfRulesIsFirewall) {
@@ -440,18 +455,7 @@ public class FirewallManagerImpl extends ManagerBase
implements FirewallService,
if (!notNullPorts) {
continue;
- } else if (!oneOfRulesIsFirewall &&
- !(bothRulesFirewall && !duplicatedCidrs) &&
- ((rule.getSourcePortStart().intValue() <=
newRule.getSourcePortStart().intValue() &&
- rule.getSourcePortEnd().intValue() >=
newRule.getSourcePortStart().intValue()) ||
- (rule.getSourcePortStart().intValue() <=
newRule.getSourcePortEnd().intValue() &&
- rule.getSourcePortEnd().intValue() >=
newRule.getSourcePortEnd().intValue()) ||
- (newRule.getSourcePortStart().intValue() <=
rule.getSourcePortStart().intValue() &&
- newRule.getSourcePortEnd().intValue() >=
rule.getSourcePortStart().intValue()) ||
- (newRule.getSourcePortStart().intValue() <=
rule.getSourcePortEnd().intValue() &&
- newRule.getSourcePortEnd().intValue() >=
rule.getSourcePortEnd().intValue()))) {
- //Above else if conditions checks for the conflicting port
ranges.
-
+ } else if (checkIfRulesHaveConflictingPortRanges(newRule, rule,
oneOfRulesIsFirewall, bothRulesFirewall, bothRulesPortForwarding,
duplicatedCidrs)) {
// we allow port forwarding rules with the same parameters but
different protocols
boolean allowPf =
(rule.getPurpose() == Purpose.PortForwarding &&
newRule.getPurpose() == Purpose.PortForwarding &&
!newRule.getProtocol().equalsIgnoreCase(
@@ -478,6 +482,45 @@ public class FirewallManagerImpl extends ManagerBase
implements FirewallService,
}
}
+ protected boolean checkIfRulesHaveConflictingPortRanges(FirewallRule
newRule, FirewallRule rule, boolean oneOfRulesIsFirewall, boolean
bothRulesFirewall, boolean bothRulesPortForwarding, boolean duplicatedCidrs) {
+ String rulesAsString = String.format("[%s] and [%s]", rule, newRule);
+
+ if (oneOfRulesIsFirewall) {
+ s_logger.debug(String.format("Only one of the rules (%s) is
firewall; therefore, their port ranges will not conflict.",
+ rulesAsString));
+ return false;
+ }
+
+ if ((bothRulesFirewall || bothRulesPortForwarding) &&
!duplicatedCidrs) {
+ s_logger.debug(String.format("Both rules (%s) are firewall/port
forwarding, but they do not have duplicated CIDRs; therefore, their port ranges
will not conflict.",
+ rulesAsString));
+ return false;
+ }
+
+ if (rule.getSourcePortStart() <= newRule.getSourcePortStart() &&
rule.getSourcePortEnd() >= newRule.getSourcePortStart()) {
+ s_logger.debug(String.format("Rules (%s) have conflicting port
ranges.", rulesAsString));
+ return true;
+ }
+
+ if (rule.getSourcePortStart() <= newRule.getSourcePortEnd() &&
rule.getSourcePortEnd() >= newRule.getSourcePortEnd()) {
+ s_logger.debug(String.format("Rules (%s) have conflicting port
ranges.", rulesAsString));
+ return true;
+ }
+
+ if (newRule.getSourcePortStart() <= rule.getSourcePortStart() &&
newRule.getSourcePortEnd() >= rule.getSourcePortStart()) {
+ s_logger.debug(String.format("Rules (%s) have conflicting port
ranges.", rulesAsString));
+ return true;
+ }
+
+ if (newRule.getSourcePortStart() <= rule.getSourcePortEnd() &&
newRule.getSourcePortEnd() >= rule.getSourcePortEnd()) {
+ s_logger.debug(String.format("Rules (%s) have conflicting port
ranges.", rulesAsString));
+ return true;
+ }
+
+ s_logger.debug(String.format("Rules (%s) do not have conflicting port
ranges.", rulesAsString));
+ return false;
+ }
+
@Override
public void validateFirewallRule(Account caller, IPAddressVO ipAddress,
Integer portStart, Integer portEnd, String proto, Purpose purpose,
FirewallRuleType type,
Long networkId, FirewallRule.TrafficType trafficType) {
diff --git
a/server/src/main/java/com/cloud/network/router/CommandSetupHelper.java
b/server/src/main/java/com/cloud/network/router/CommandSetupHelper.java
index a7ed6478efa..0e3acb953c7 100644
--- a/server/src/main/java/com/cloud/network/router/CommandSetupHelper.java
+++ b/server/src/main/java/com/cloud/network/router/CommandSetupHelper.java
@@ -27,6 +27,7 @@ import java.util.Set;
import javax.inject.Inject;
+import com.cloud.network.rules.PortForwardingRuleVO;
import org.apache.cloudstack.api.ApiConstants;
import
org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService;
import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
@@ -396,6 +397,7 @@ public class CommandSetupHelper {
final List<PortForwardingRuleTO> rulesTO = new
ArrayList<PortForwardingRuleTO>();
if (rules != null) {
for (final PortForwardingRule rule : rules) {
+ _rulesDao.loadSourceCidrs((PortForwardingRuleVO) rule);
final IpAddress sourceIp =
_networkModel.getIp(rule.getSourceIpAddressId());
final PortForwardingRuleTO ruleTO = new
PortForwardingRuleTO(rule, null, sourceIp.getAddress().addr());
rulesTO.add(ruleTO);
diff --git a/server/src/main/java/com/cloud/network/rules/RulesManagerImpl.java
b/server/src/main/java/com/cloud/network/rules/RulesManagerImpl.java
index 7686a8d7887..1b7177ec416 100644
--- a/server/src/main/java/com/cloud/network/rules/RulesManagerImpl.java
+++ b/server/src/main/java/com/cloud/network/rules/RulesManagerImpl.java
@@ -33,6 +33,7 @@ import com.cloud.vm.VirtualMachineProfile;
import com.cloud.vm.VirtualMachineProfileImpl;
import org.apache.cloudstack.acl.SecurityChecker;
import
org.apache.cloudstack.api.command.user.firewall.ListPortForwardingRulesCmd;
+import
org.apache.cloudstack.api.command.user.firewall.UpdatePortForwardingRuleCmd;
import org.apache.cloudstack.context.CallContext;
import
org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService;
import org.apache.log4j.Logger;
@@ -103,6 +104,7 @@ import com.cloud.vm.dao.NicSecondaryIpDao;
import com.cloud.vm.dao.NicSecondaryIpVO;
import com.cloud.vm.dao.UserVmDao;
import com.cloud.vm.dao.VMInstanceDao;
+import org.apache.commons.collections.CollectionUtils;
public class RulesManagerImpl extends ManagerBase implements RulesManager,
RulesService {
private static final Logger s_logger =
Logger.getLogger(RulesManagerImpl.class);
@@ -114,7 +116,7 @@ public class RulesManagerImpl extends ManagerBase
implements RulesManager, Rules
@Inject
PortForwardingRulesDao _portForwardingDao;
@Inject
- FirewallRulesCidrsDao _firewallCidrsDao;
+ FirewallRulesCidrsDao firewallCidrsDao;
@Inject
FirewallRulesDao _firewallDao;
@Inject
@@ -249,6 +251,7 @@ public class RulesManagerImpl extends ManagerBase
implements RulesManager, Rules
final Long accountId = ipAddress.getAllocatedToAccountId();
final Long domainId = ipAddress.getAllocatedInDomainId();
+ List<String> sourceCidrList = rule.getSourceCidrList();
// start port can't be bigger than end port
if (rule.getDestinationPortStart() >
rule.getDestinationPortEnd()) {
@@ -312,9 +315,8 @@ public class RulesManagerImpl extends ManagerBase
implements RulesManager, Rules
final IPAddressVO ipAddressFinal = ipAddress;
return
Transaction.execute((TransactionCallbackWithException<PortForwardingRuleVO,
NetworkRuleConflictException>) status -> {
PortForwardingRuleVO newRule =
- new PortForwardingRuleVO(rule.getXid(),
rule.getSourceIpAddressId(), rule.getSourcePortStart(),
rule.getSourcePortEnd(), dstIpFinal,
- rule.getDestinationPortStart(),
rule.getDestinationPortEnd(), rule.getProtocol().toLowerCase(), networkId,
accountId, domainId, vmId);
-
+ new PortForwardingRuleVO(rule.getXid(),
rule.getSourceIpAddressId(), rule.getSourcePortStart(),
rule.getSourcePortEnd(), dstIpFinal,
+ rule.getDestinationPortStart(),
rule.getDestinationPortEnd(), rule.getProtocol().toLowerCase(), networkId,
accountId, domainId, vmId, sourceCidrList);
if (forDisplay != null) {
newRule.setDisplay(forDisplay);
}
@@ -898,6 +900,10 @@ public class RulesManagerImpl extends ManagerBase
implements RulesManager, Rules
_accountMgr.checkAccess(caller, null, true, rules.toArray(new
PortForwardingRuleVO[rules.size()]));
}
+ for (PortForwardingRuleVO rule : rules) {
+
rule.setSourceCidrList(firewallCidrsDao.getSourceCidrs(rule.getId()));
+ }
+
try {
if (!_firewallMgr.applyRules(rules, continueOnError, true)) {
return false;
@@ -951,6 +957,10 @@ public class RulesManagerImpl extends ManagerBase
implements RulesManager, Rules
_accountMgr.checkAccess(caller, null, true, rules.toArray(new
PortForwardingRuleVO[rules.size()]));
}
+ for (PortForwardingRuleVO rule: rules) {
+
rule.setSourceCidrList(firewallCidrsDao.getSourceCidrs(rule.getId()));
+ }
+
try {
if (!_firewallMgr.applyRules(rules, continueOnError, true)) {
return false;
@@ -1570,12 +1580,22 @@ public class RulesManagerImpl extends ManagerBase
implements RulesManager, Rules
@Override
@ActionEvent(eventType = EventTypes.EVENT_NET_RULE_MODIFY,
eventDescription = "updating forwarding rule", async = true)
- public PortForwardingRule updatePortForwardingRule(long id, Integer
privatePort, Integer privateEndPort, Long virtualMachineId, Ip vmGuestIp,
String customId, Boolean forDisplay) {
- Account caller = CallContext.current().getCallingAccount();
+ public PortForwardingRule
updatePortForwardingRule(UpdatePortForwardingRuleCmd cmd) {
+ long id = cmd.getId();
+ Integer privatePort = cmd.getPrivatePort();
+ Integer privateEndPort = cmd.getPrivateEndPort();
+ Long virtualMachineId = cmd.getVirtualMachineId();
+ Ip vmGuestIp = cmd.getVmGuestIp();
+ String customId = cmd.getCustomId();
+ Boolean forDisplay = cmd.getDisplay();
+ List<String> sourceCidrList = cmd.getSourceCidrList();
+
PortForwardingRuleVO rule = _portForwardingDao.findById(id);
if (rule == null) {
throw new InvalidParameterValueException("Unable to find " + id);
}
+
+ Account caller = CallContext.current().getCallingAccount();
_accountMgr.checkAccess(caller, null, true, rule);
if (customId != null) {
@@ -1636,6 +1656,8 @@ public class RulesManagerImpl extends ManagerBase
implements RulesManager, Rules
}
}
+ validatePortForwardingSourceCidrList(sourceCidrList);
+
// revoke old rules at first
List<PortForwardingRuleVO> rules = new
ArrayList<PortForwardingRuleVO>();
rule.setState(State.Revoke);
@@ -1663,6 +1685,11 @@ public class RulesManagerImpl extends ManagerBase
implements RulesManager, Rules
rule.setVirtualMachineId(virtualMachineId);
rule.setDestinationIpAddress(dstIp);
}
+
+ if (sourceCidrList != null) {
+ rule.setSourceCidrList(sourceCidrList);
+ }
+
_portForwardingDao.update(id, rule);
//apply new rules
@@ -1672,4 +1699,17 @@ public class RulesManagerImpl extends ManagerBase
implements RulesManager, Rules
return _portForwardingDao.findById(id);
}
+
+ @Override
+ public void validatePortForwardingSourceCidrList(List<String>
sourceCidrList) {
+ if (CollectionUtils.isEmpty(sourceCidrList)) {
+ return;
+ }
+
+ for (String cidr : sourceCidrList) {
+ if (!NetUtils.isValidCidrList(cidr) &&
!NetUtils.isValidIp6Cidr(cidr)) {
+ throw new InvalidParameterValueException(String.format("The
given source CIDR [%s] is invalid.", cidr));
+ }
+ }
+ }
}
diff --git
a/server/src/test/java/com/cloud/network/firewall/FirewallManagerTest.java
b/server/src/test/java/com/cloud/network/firewall/FirewallManagerTest.java
index 2200d6b125b..e8f01576e4c 100644
--- a/server/src/test/java/com/cloud/network/firewall/FirewallManagerTest.java
+++ b/server/src/test/java/com/cloud/network/firewall/FirewallManagerTest.java
@@ -27,7 +27,6 @@ import com.cloud.network.dao.FirewallRulesDao;
import com.cloud.network.element.FirewallServiceProvider;
import com.cloud.network.element.VirtualRouterElement;
import com.cloud.network.element.VpcVirtualRouterElement;
-import com.cloud.network.rules.FirewallManager;
import com.cloud.network.rules.FirewallRule;
import com.cloud.network.rules.FirewallRule.Purpose;
import com.cloud.network.rules.FirewallRuleVO;
@@ -35,9 +34,9 @@ import com.cloud.network.vpc.VpcManager;
import com.cloud.user.AccountManager;
import com.cloud.user.DomainManager;
import com.cloud.utils.component.ComponentContext;
-import junit.framework.Assert;
import
org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService;
import org.apache.log4j.Logger;
+import org.junit.Assert;
import org.junit.Before;
import org.junit.Ignore;
import org.junit.Test;
@@ -45,7 +44,8 @@ import org.junit.runner.RunWith;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
-import org.mockito.runners.MockitoJUnitRunner;
+import org.mockito.Spy;
+import org.mockito.junit.MockitoJUnitRunner;
import java.util.ArrayList;
import java.util.Arrays;
@@ -108,12 +108,35 @@ public class FirewallManagerTest {
@Mock
FirewallRulesDao _firewallDao;
+ @Spy
@InjectMocks
- FirewallManager _firewallMgr = new FirewallManagerImpl();
+ FirewallManagerImpl _firewallMgr;
+
+ FirewallRule fwRule50to150;
+ FirewallRule fwRule100to200;
+ FirewallRule fwRule151to200;
+
+ FirewallRule pfRule50to150;
+ FirewallRule pfRule100to200;
+ FirewallRule pfRule151to200;
+
@Before
public void initMocks() {
MockitoAnnotations.initMocks(this);
+
+ fwRule50to150 = createFirewallRule(50, 150, Purpose.Firewall);
+ fwRule100to200 = createFirewallRule(100, 150, Purpose.Firewall);
+ fwRule151to200 = createFirewallRule(151, 200, Purpose.Firewall);
+
+ pfRule50to150 = createFirewallRule(50, 150, Purpose.PortForwarding);
+ pfRule100to200 = createFirewallRule(100, 150, Purpose.PortForwarding);
+ pfRule151to200 = createFirewallRule(151, 200, Purpose.PortForwarding);
+ }
+
+ private FirewallRule createFirewallRule(int startPort, int endPort,
Purpose purpose) {
+ return new FirewallRuleVO("xid", 1L, startPort, endPort, "TCP", 2, 3,
4, purpose, new ArrayList<>(),
+ new ArrayList<>(), 5, 6, null,
FirewallRule.TrafficType.Ingress);
}
@Ignore("Requires database to be set up")
@@ -210,6 +233,75 @@ public class FirewallManagerTest {
}
}
+ @Test
+ public void
checkIfRulesHaveConflictingPortRangesTestOnlyOneRuleIsFirewallReturnsFalse()
+ {
+ boolean result =
_firewallMgr.checkIfRulesHaveConflictingPortRanges(fwRule50to150,
pfRule50to150, true, false, false, true);
+
+ Assert.assertFalse(result);
+ }
+
+ @Test
+ public void
checkIfRulesHaveConflictingPortRangesTestBothRulesAreFirewallButNoDuplicateCidrsReturnsFalse()
+ {
+ boolean result =
_firewallMgr.checkIfRulesHaveConflictingPortRanges(fwRule50to150,
fwRule50to150, false, true, false, false);
+
+ Assert.assertFalse(result);
+ }
+
+ @Test
+ public void
checkIfRulesHaveConflictingPortRangesTestBothRulesArePortForwardingButNoDuplicateCidrsReturnsFalse()
+ {
+ boolean result =
_firewallMgr.checkIfRulesHaveConflictingPortRanges(pfRule50to150,
pfRule50to150, false, false, true, false);
+
+ Assert.assertFalse(result);
+ }
+
+ @Test
+ public void
checkIfRulesHaveConflictingPortRangesTestBothRulesAreFirewallAndDuplicatedCidrsAndNewRuleSourceStartPortIsInsideExistingRangeReturnsTrue()
+ {
+ boolean result =
_firewallMgr.checkIfRulesHaveConflictingPortRanges(fwRule100to200,
fwRule50to150, false, true, false, true);
+
+ Assert.assertTrue(result);
+ }
+
+ @Test
+ public void
checkIfRulesHaveConflictingPortRangesTestBothRulesAreFirewallAndDuplicatedCidrsAndNewRuleSourceEndPortIsInsideExistingRangeReturnsTrue()
+ {
+ boolean result =
_firewallMgr.checkIfRulesHaveConflictingPortRanges(fwRule50to150,
fwRule100to200, false, true, false, true);
+
+ Assert.assertTrue(result);
+ }
+
+ @Test
+ public void
checkIfRulesHaveConflictingPortRangesTestBothRulesArePortForwardingAndDuplicatedCidrsAndNewRuleSourceStartPortIsInsideExistingRangeReturnsTrue()
+ {
+ boolean result =
_firewallMgr.checkIfRulesHaveConflictingPortRanges(pfRule50to150,
pfRule100to200, false, false, true, true);
+
+ Assert.assertTrue(result);
+ }
+
+ @Test
+ public void
checkIfRulesHaveConflictingPortRangesTestBothRulesArePortForwardingAndDuplicatedCidrsAndNewRuleSourceEndPortIsInsideExistingRangeReturnsTrue()
+ {
+ boolean result =
_firewallMgr.checkIfRulesHaveConflictingPortRanges(pfRule50to150,
pfRule100to200, false, false, true, true);
+
+ Assert.assertTrue(result);
+ }
+
+ @Test
+ public void
checkIfRulesHaveConflictingPortRangesTestBothRulesAreFirewallAndDuplicatedCidrsAndNoRangeConflictReturnsFalse()
+ {
+ boolean result =
_firewallMgr.checkIfRulesHaveConflictingPortRanges(fwRule50to150,
fwRule151to200, false, true, false, true);
+ Assert.assertFalse(result);
+ }
+ @Test
+ public void
checkIfRulesHaveConflictingPortRangesTestBothRulesArePortForwardingAndDuplicatedCidrsAndNoRangeConflictReturnsFalse()
+ {
+ boolean result =
_firewallMgr.checkIfRulesHaveConflictingPortRanges(pfRule50to150,
pfRule151to200, false, false, true, true);
+
+ Assert.assertFalse(result);
+ }
}
diff --git a/systemvm/debian/opt/cloud/bin/configure.py
b/systemvm/debian/opt/cloud/bin/configure.py
index c261293af0f..fd421fc268f 100755
--- a/systemvm/debian/opt/cloud/bin/configure.py
+++ b/systemvm/debian/opt/cloud/bin/configure.py
@@ -1217,7 +1217,10 @@ class CsForwardingRules(CsDataBag):
self.fw.append(["filter", "", fw7])
def forward_vpc(self, rule):
- fw_prerout_rule = "-A PREROUTING -d %s/32 " % (rule["public_ip"])
+ fw_prerout_rule = "-A PREROUTING"
+ if "source_cidr_list" in rule and rule["source_cidr_list"]:
+ fw_prerout_rule += " -s %s" % rule["source_cidr_list"]
+ fw_prerout_rule += " -d %s/32" % rule["public_ip"]
if not rule["protocol"] == "any":
fw_prerout_rule += " -m %s -p %s" % (rule["protocol"],
rule["protocol"])
if not rule["public_ports"] == "any":
@@ -1226,7 +1229,10 @@ class CsForwardingRules(CsDataBag):
if not rule["internal_ports"] == "any":
fw_prerout_rule += ":" +
self.portsToString(rule["internal_ports"], "-")
- fw_output_rule = "-A OUTPUT -d %s/32" % rule["public_ip"]
+ fw_output_rule = "-A OUTPUT"
+ if "source_cidr_list" in rule and rule["source_cidr_list"]:
+ fw_output_rule += " -s %s" % rule["source_cidr_list"]
+ fw_output_rule += " -d %s/32" % rule["public_ip"]
if not rule["protocol"] == "any":
fw_output_rule += " -m %s -p %s" % (rule["protocol"],
rule["protocol"])
if not rule["public_ports"] == "any":
diff --git a/systemvm/debian/opt/cloud/bin/cs/CsNetfilter.py
b/systemvm/debian/opt/cloud/bin/cs/CsNetfilter.py
index a034034dc8b..14a49feda76 100755
--- a/systemvm/debian/opt/cloud/bin/cs/CsNetfilter.py
+++ b/systemvm/debian/opt/cloud/bin/cs/CsNetfilter.py
@@ -327,7 +327,7 @@ class CsNetfilter(object):
return self.rule
def to_str(self, delete=False):
- """ Convert the rule back into aynactically correct iptables command
"""
+ """ Convert the rule back into syntactically correct iptables command
"""
# Order is important
order = ['-A', '-s', '-d', '!_-d', '-i', '!_-i', '-p', '-m', '-m2',
'--icmp-type', '--state',
'--dport', '--destination-port', '-o', '!_-o', '-j',
'--set-xmark', '--checksum',
diff --git a/systemvm/debian/opt/cloud/bin/cs_forwardingrules.py
b/systemvm/debian/opt/cloud/bin/cs_forwardingrules.py
index 974c468e8dc..c8b4c91ad75 100755
--- a/systemvm/debian/opt/cloud/bin/cs_forwardingrules.py
+++ b/systemvm/debian/opt/cloud/bin/cs_forwardingrules.py
@@ -16,6 +16,8 @@
# under the License.
+import logging
+
def merge(dbag, rules):
for rule in rules["rules"]:
source_ip = rule["source_ip_address"]
@@ -33,6 +35,8 @@ def merge(dbag, rules):
newrule["public_ports"] = rule["source_port_range"]
newrule["internal_ports"] = rule["destination_port_range"]
newrule["protocol"] = rule["protocol"]
+ if "source_cidr_list" in rule:
+ newrule["source_cidr_list"] = rule["source_cidr_list"]
if not revoke:
if rules["type"] == "staticnatrules":
@@ -59,7 +63,7 @@ def merge(dbag, rules):
for forward in dbag[source_ip]:
if ruleCompare(forward, newrule):
index = dbag[source_ip].index(forward)
- print "removing index %s" % str(index)
+ logging.info("Removing forwarding rule [%s] at
index [%s].", forward, index)
if not index == -1:
del dbag[source_ip][index]
@@ -74,4 +78,18 @@ def ruleCompare(ruleA, ruleB):
return ruleA["public_ip"] == ruleB["public_ip"]
elif ruleA["type"] == "forward":
return ruleA["public_ip"] == ruleB["public_ip"] and
ruleA["public_ports"] == ruleB["public_ports"] \
- and ruleA["protocol"] == ruleB["protocol"]
+ and ruleA["protocol"] == ruleB["protocol"] and
cidrsConflict(ruleA.get("source_cidr_list"), ruleB.get("source_cidr_list"))
+
+# Same validation as in
com.cloud.network.firewall.FirewallManagerImpl.detectConflictingCidrs
+def cidrsConflict(cidrListA, cidrListB):
+ if not cidrListA and not cidrListB:
+ return True
+ if not cidrListA:
+ return False
+ if not cidrListB:
+ return False
+
+ cidrListA = set(cidrListA.split(","))
+ cidrListB = set(cidrListB.split(","))
+
+ return len(cidrListA.intersection(cidrListB)) > 0
diff --git a/ui/public/locales/en.json b/ui/public/locales/en.json
index 48f78f598eb..8cc17bbb128 100644
--- a/ui/public/locales/en.json
+++ b/ui/public/locales/en.json
@@ -1946,6 +1946,7 @@
"label.source": "Select Import-Export Source Hypervisor",
"label.source.based": "SourceBased",
"label.sourcecidr": "Source CIDR",
+"label.sourcecidrlist": "Source CIDR list",
"label.sourcehost": "Source host",
"label.sourceipaddress": "Source IP address",
"label.sourceipaddressnetworkid": "Network ID of source IP address",
diff --git a/ui/public/locales/pt_BR.json b/ui/public/locales/pt_BR.json
index c36a5c762a7..937caa69a59 100644
--- a/ui/public/locales/pt_BR.json
+++ b/ui/public/locales/pt_BR.json
@@ -349,7 +349,7 @@
"label.choose.saml.identity": "Escolha um provedor de identidade SAML",
"label.cidr": "CIDR",
"label.cidr.destination.network": "CIDR da rede de destino",
-"label.cidrlist": "Lista de CIDR ",
+"label.cidrlist": "Lista de CIDRs",
"label.cisco.nexus1000v.ip.address": "Endere\u00e7o IP do Nexus 1000v",
"label.cisco.nexus1000v.password": "Senha do Nexus 1000v",
"label.cisco.nexus1000v.username": "Usu\u00e1rio do Nexus 1000v",
@@ -1494,6 +1494,7 @@
"label.sockettimeout": "Tempo limite no socket",
"label.source.based": "SourceBased",
"label.sourcecidr": "CIDR de origem",
+"label.sourcecidrlist": "Lista de CIDRs de origem",
"label.sourceipaddress": "Endere\u00e7o IP de origem",
"label.sourcenat": "Source NAT",
"label.sourcenatsupported": "Suporte \u00e0 source NAT",
diff --git a/ui/src/views/network/LoadBalancing.vue
b/ui/src/views/network/LoadBalancing.vue
index eeec232cee5..09f9a39c4cf 100644
--- a/ui/src/views/network/LoadBalancing.vue
+++ b/ui/src/views/network/LoadBalancing.vue
@@ -37,7 +37,7 @@
</div>
<div class="form">
<div class="form__item" ref="newCidrList">
- <tooltip-label :title="$t('label.cidrlist')" bold
:tooltip="createLoadBalancerRuleParams.cidrlist.description"
:tooltip-placement="'right'"/>
+ <tooltip-label :title="$t('label.sourcecidrlist')" bold
:tooltip="createLoadBalancerRuleParams.cidrlist.description"
:tooltip-placement="'right'"/>
<a-input v-model:value="newRule.cidrlist"></a-input>
</div>
<div class="form__item">
@@ -126,7 +126,7 @@
:rowKey="record => record.id">
<template #bodyCell="{ column, record }">
<template v-if="column.key === 'cidrlist'">
- <span style="white-space: pre-line"> {{
record.cidrlist?.replaceAll(" ", "\n") }}</span>
+ <span style="white-space: pre-line"> {{
record.cidrlist?.replaceAll(",", "\n") }}</span>
</template>
<template v-if="column.key === 'algorithm'">
{{ returnAlgorithmName(record.algorithm) }}
@@ -808,7 +808,7 @@ export default {
},
{
key: 'cidrlist',
- title: this.$t('label.cidrlist')
+ title: this.$t('label.sourcecidrlist')
},
{
key: 'protocol',
diff --git a/ui/src/views/network/PortForwarding.vue
b/ui/src/views/network/PortForwarding.vue
index 81843a1a0b0..2c10b4bcc75 100644
--- a/ui/src/views/network/PortForwarding.vue
+++ b/ui/src/views/network/PortForwarding.vue
@@ -72,6 +72,12 @@
<a-select-option value="udp" :label="$t('label.udp')">{{
$t('label.udp') }}</a-select-option>
</a-select>
</div>
+ <div v-if="isVPC()">
+ <div class="form__item" ref="newCidrList">
+ <tooltip-label :title="$t('label.sourcecidrlist')" bold
:tooltip="apiParams.cidrlist.description" :tooltip-placement="'right'"/>
+ <a-input v-model:value="newRule.cidrlist"></a-input>
+ </div>
+ </div>
<div class="form__item" style="margin-left: auto;">
<div class="form__label">{{ $t('label.add.vm') }}</div>
<a-button :disabled="!('createPortForwardingRule' in
$store.getters.apis)" type="primary" @click="openAddVMModal">{{ $t('label.add')
}}</a-button>
@@ -108,6 +114,9 @@
<template v-if="column.key === 'protocol'">
{{ getCapitalise(record.protocol) }}
</template>
+ <template v-if="column.key === 'cidrlist'">
+ <span style="white-space: pre-line"> {{
record.cidrlist?.replaceAll(",", "\n") }}</span>
+ </template>
<template v-if="column.key === 'vm'">
<div><desktop-outlined/>
<router-link
@@ -334,9 +343,11 @@ import Status from '@/components/widgets/Status'
import TooltipButton from '@/components/widgets/TooltipButton'
import BulkActionView from '@/components/view/BulkActionView'
import eventBus from '@/config/eventBus'
+import TooltipLabel from '@/components/widgets/TooltipLabel.vue'
export default {
components: {
+ TooltipLabel,
Status,
TooltipButton,
BulkActionView
@@ -399,6 +410,11 @@ export default {
key: 'protocol',
title: this.$t('label.protocol')
},
+ {
+ key: 'cidrlist',
+ title: this.$t('label.sourcecidrlist'),
+ hidden: !this.isVPC()
+ },
{
title: this.$t('label.state'),
dataIndex: 'state'
@@ -411,7 +427,7 @@ export default {
key: 'actions',
title: this.$t('label.actions')
}
- ],
+ ].filter(item => !item.hidden),
tiers: {
loading: false,
data: []
@@ -450,7 +466,8 @@ export default {
vmPage: 1,
vmPageSize: 10,
vmCount: 0,
- searchQuery: null
+ searchQuery: null,
+ cidrlist: ''
}
},
computed: {
@@ -458,6 +475,9 @@ export default {
return this.selectedRowKeys.length > 0
}
},
+ beforeCreate () {
+ this.apiParams = this.$getApiParams('createPortForwardingRule')
+ },
created () {
console.log(this.resource)
this.initForm()
@@ -830,6 +850,9 @@ export default {
onSearch (value) {
this.searchQuery = value
this.fetchVirtualMachines()
+ },
+ isVPC () {
+ return 'vpcid' in this.resource
}
}
}
