enable to create VPC portfowarding rules with source cidr (apache#7081)

Co-authored-by: Lopez <[email protected]>
Co-authored-by: Fabricio Duarte <[email protected]>

Author: 3 people

api/src/main/java/com/cloud/agent/api/to/PortForwardingRuleTO.java

@@ -19,6 +19,9 @@
 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 PortForwardingRuleTO(PortForwardingRule rule, String srcVlanTag, String s
         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 String getStringDstPortRange() {
         return NetUtils.portRangeToString(dstPortRange);
     }
 
+    public String getSourceCidrListAsString() {
+        if (sourceCidrList != null) {
+            return StringUtils.join(sourceCidrList, ",");
+        }
+        return null;
+    }
+
 }

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

@@ -26,6 +26,7 @@
 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 @@ Pair<List<? extends FirewallRule>, Integer> searchStaticNatRules(Long ipId, Long
 
     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);
 
 }

api/src/main/java/org/apache/cloudstack/api/ApiConstants.java

@@ -458,6 +458,8 @@ public class ApiConstants {
     public static final String SNAPSHOT_POLICY_ID = "snapshotpolicyid";
     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 SSL_VERIFICATION = "sslverification";
     public static final String START_ASN = "startasn";

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

@@ -105,8 +105,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"
@@ -155,11 +159,7 @@ public long getVirtualMachineId() {
 
     @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() {
@@ -332,19 +332,15 @@ public int getDestinationPortEnd() {
 
     @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())) {
                 throw new InvalidParameterValueException("Invalid vm ip address");
             }
         }
 
+        _rulesService.validatePortForwardingSourceCidrList(sourceCidrList);
+
         try {
             PortForwardingRule result = _rulesService.createPortForwardingRule(this, virtualMachineId, privateIp, getOpenFirewall(), isDisplay());
             setEntityId(result.getId());

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

@@ -32,6 +32,8 @@
 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},
@@ -63,6 +65,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 ///////////////////////
     /////////////////////////////////////////////////////
@@ -94,6 +103,10 @@ public Boolean getDisplay() {
         return display;
     }
 
+    public List<String> getSourceCidrList() {
+        return sourceCidrList;
+    }
+
     /////////////////////////////////////////////////////
     /////////////// API Implementation///////////////////
     /////////////////////////////////////////////////////
@@ -130,7 +143,7 @@ public void checkUuid() {
 
     @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);

api/src/main/java/org/apache/cloudstack/api/command/user/loadbalancer/CreateLoadBalancerRuleCmd.java

@@ -104,7 +104,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;
 

core/src/main/java/com/cloud/agent/resource/virtualnetwork/facade/SetPortForwardingRulesConfigItem.java

@@ -41,7 +41,7 @@ public List<ConfigItem> generateConfig(final NetworkElementCommand cmd) {
 
         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);
         }
 

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 void setDestinationPortRange(String destinationPortRange) {
         this.destinationPortRange = destinationPortRange;
     }
 
+    public String getSourceCidrList() {
+        return sourceCidrList;
+    }
+
 }

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);
 }

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

@@ -20,6 +20,7 @@
 import java.util.List;
 
 
+import org.apache.commons.collections4.CollectionUtils;
 import org.springframework.stereotype.Component;
 
 import com.cloud.utils.db.DB;
@@ -63,9 +64,27 @@ public List<FirewallRulesCidrsVO> listByFirewallRuleId(long firewallRuleId) {
         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();

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

@@ -251,9 +251,6 @@ public FirewallRuleVO persist(FirewallRuleVO firewallRule) {
     }
 
     public void saveSourceCidrs(FirewallRuleVO firewallRule, List<String> cidrList) {
-        if (cidrList == null) {
-            return;
-        }
         _firewallRulesCidrsDao.persist(firewallRule.getId(), cidrList);
     }
 

engine/schema/src/main/java/com/cloud/network/rules/PortForwardingRuleVO.java

@@ -25,6 +25,7 @@
 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 Long getRelated() {
         return null;
     }
 
+    public void setSourceCidrList(List<String> sourceCidrs) {
+        this.sourceCidrs = sourceCidrs;
+    }
+
+    @Override
+    public List<String> getSourceCidrList() {
+        return sourceCidrs;
+    }
+
 }

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

@@ -31,6 +31,9 @@
 import com.cloud.utils.db.SearchBuilder;
 import com.cloud.utils.db.SearchCriteria;
 import com.cloud.utils.db.SearchCriteria.Op;
+import com.cloud.utils.db.Transaction;
+import com.cloud.utils.db.TransactionCallback;
+import com.cloud.utils.db.TransactionLegacy;
 
 @Component
 public class PortForwardingRulesDaoImpl extends GenericDaoBase<PortForwardingRuleVO, Long> implements PortForwardingRulesDao {
@@ -42,7 +45,7 @@ public class PortForwardingRulesDaoImpl extends GenericDaoBase<PortForwardingRul
     protected final SearchBuilder<PortForwardingRuleVO> ActiveRulesSearchByAccount;
 
     @Inject
-    protected FirewallRulesCidrsDao _portForwardingRulesCidrsDao;
+    protected FirewallRulesCidrsDao portForwardingRulesCidrsDao;
 
     protected PortForwardingRulesDaoImpl() {
         super();
@@ -183,4 +186,43 @@ public int expungeByVmList(List<Long> vmIds, Long batchSize) {
         sc.setParameters("vmIds", vmIds.toArray());
         return batchExpunge(sc, batchSize);
     }
-}
+
+    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;
+    }
+}

plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorker.java

@@ -461,7 +461,7 @@ protected void provisionPublicIpPortForwardingRule(IpAddress publicIp, Network n
                             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);