Add new parameter to createLoadBalancerRule API (#6460)

JoaoJandre · João Paraquetti · web-flow · commit 9c63c393711d · 2022-08-08T10:48:21.000+02:00
* Add new parameter to createLoadBalancerRule API

* address review

Co-authored-by: João Paraquetti &lt;joao@scclouds.com.br&gt;
diff --git a/api/src/main/java/com/cloud/agent/api/to/LoadBalancerTO.java b/api/src/main/java/com/cloud/agent/api/to/LoadBalancerTO.java
@@ -57,6 +57,8 @@ public class LoadBalancerTO {
     final static int MAX_STICKINESS_POLICIES = 1;
     final static int MAX_HEALTHCHECK_POLICIES = 1;
 
+    private String cidrList;
+
     public LoadBalancerTO(String uuid, String srcIp, int srcPort, String protocol, String algorithm, boolean revoked, boolean alreadyAdded, boolean inline,
             List<LbDestination> destinations) {
         if (destinations == null) { // for autoscaleconfig destinations will be null;
@@ -239,6 +241,14 @@ public void setSrcIpNetmask(String srcIpNetmask) {
         this.srcIpNetmask = srcIpNetmask;
     }
 
+    public void setCidrList(String cidrList){
+        this.cidrList = cidrList;
+    }
+
+    public String getCidrList() {
+        return cidrList;
+    }
+
     public static class StickinessPolicyTO {
         private String methodName;
         private List<Pair<String, String>> params;
diff --git a/api/src/main/java/com/cloud/network/lb/LoadBalancingRule.java b/api/src/main/java/com/cloud/network/lb/LoadBalancingRule.java
@@ -482,4 +482,8 @@ public Ip getSourceIp() {
     public Scheme getScheme() {
         return lb.getScheme();
     }
+
+    public String getCidrList(){
+        return lb.getCidrList();
+    }
 }
diff --git a/api/src/main/java/com/cloud/network/lb/LoadBalancingRulesService.java b/api/src/main/java/com/cloud/network/lb/LoadBalancingRulesService.java
@@ -54,6 +54,10 @@ LoadBalancer createPublicLoadBalancerRule(String xId, String name, String descri
         Long ipAddrId, String protocol, String algorithm, long networkId, long lbOwnerId, boolean openFirewall, String lbProtocol, Boolean forDisplay) throws NetworkRuleConflictException,
         InsufficientAddressCapacityException;
 
+    LoadBalancer createPublicLoadBalancerRule(String xId, String name, String description, int srcPortStart, int srcPortEnd, int defPortStart, int defPortEnd,
+            Long ipAddrId, String protocol, String algorithm, long networkId, long lbOwnerId, boolean openFirewall, String lbProtocol, Boolean forDisplay, List<String> cidrList) throws NetworkRuleConflictException,
+            InsufficientAddressCapacityException;
+
     LoadBalancer updateLoadBalancerRule(UpdateLoadBalancerRuleCmd cmd);
 
     boolean deleteLoadBalancerRule(long lbRuleId, boolean apply);
diff --git a/api/src/main/java/com/cloud/network/rules/LoadBalancer.java b/api/src/main/java/com/cloud/network/rules/LoadBalancer.java
@@ -25,4 +25,6 @@ public interface LoadBalancer extends FirewallRule, LoadBalancerContainer {
 
     int getDefaultPortEnd();
 
+    String getCidrList();
+
 }
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
@@ -107,7 +107,8 @@ 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, 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.")
+    @Parameter(name = ApiConstants.CIDR_LIST, type = CommandType.LIST, collectionType = CommandType.STRING, since = "4.18.0.0", description = "the CIDR list to allow traffic, "
+            + "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;
 
     @Parameter(name = ApiConstants.NETWORK_ID, type = CommandType.UUID, entityType = NetworkResponse.class, description = "The guest network this "
@@ -306,15 +307,11 @@ public void execute() throws ResourceAllocationException, ResourceUnavailableExc
 
     @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");
-        }
         try {
             LoadBalancer result =
                 _lbService.createPublicLoadBalancerRule(getXid(), getName(), getDescription(), getSourcePortStart(), getSourcePortEnd(), getDefaultPortStart(),
-                    getDefaultPortEnd(), getSourceIpAddressId(), getProtocol(), getAlgorithm(), getNetworkId(), getEntityOwnerId(), getOpenFirewall(), getLbProtocol(), isDisplay());
+                    getDefaultPortEnd(), getSourceIpAddressId(), getProtocol(), getAlgorithm(), getNetworkId(), getEntityOwnerId(), getOpenFirewall(), getLbProtocol(), isDisplay(),
+                        getCidrList());
             this.setEntityId(result.getId());
             this.setEntityUuid(result.getUuid());
         } catch (NetworkRuleConflictException e) {
@@ -425,4 +422,9 @@ public String getSyncObjType() {
     public Long getSyncObjId() {
         return getNetworkId();
     }
+
+    public List<String> getCidrList(){
+        return cidrlist;
+    }
+
 }
diff --git a/api/src/main/java/org/apache/cloudstack/api/response/LoadBalancerResponse.java b/api/src/main/java/org/apache/cloudstack/api/response/LoadBalancerResponse.java
@@ -64,7 +64,7 @@ public class LoadBalancerResponse extends BaseResponse implements ControlledEnti
     private String networkId;
 
     @SerializedName(ApiConstants.CIDR_LIST)
-    @Param(description = "the cidr list to forward traffic from. Multiple entries are separated by a single comma character (,).")
+    @Param(description = "the CIDR list to allow traffic, all other CIDRs will be blocked. Multiple entries must be separated by a single comma character (,).")
     private String cidrList;
 
     @SerializedName(ApiConstants.ACCOUNT)
diff --git a/core/src/main/java/com/cloud/network/HAProxyConfigurator.java b/core/src/main/java/com/cloud/network/HAProxyConfigurator.java
@@ -27,6 +27,7 @@
 import java.util.Map;
 import java.util.Set;
 
+import org.apache.commons.lang3.StringUtils;
 import org.apache.log4j.Logger;
 
 import com.cloud.agent.api.routing.LoadBalancerConfigCommand;
@@ -551,6 +552,12 @@ private List<String> getRulesForPool(final LoadBalancerTO lbTO, final boolean ke
             result.add(sb.toString());
         }
 
+        String cidrList = lbTO.getCidrList();
+
+        if (StringUtils.isNotBlank(cidrList)) {
+            result.add(String.format("\tacl network_allowed src %s \n\ttcp-request connection reject if !network_allowed", cidrList));
+        }
+
         result.add(blankLine);
         return result;
     }
diff --git a/core/src/test/java/com/cloud/network/HAProxyConfiguratorTest.java b/core/src/test/java/com/cloud/network/HAProxyConfiguratorTest.java
@@ -23,6 +23,7 @@
 
 import org.junit.After;
 import org.junit.AfterClass;
+import org.junit.Assert;
 import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Test;
@@ -109,6 +110,18 @@ public void testGenerateConfigurationLoadBalancerProxyProtocolConfigCommand() {
         assertTrue("'send-proxy' should result if protocol is 'tcp-proxy'", result.contains("send-proxy"));
     }
 
+    @Test
+    public void generateConfigurationTestWithCidrList() {
+        LoadBalancerTO lb = new LoadBalancerTO("1", "10.2.0.1", 22, "tcp", "roundrobin", false, false, false, null, null);
+        lb.setCidrList("1.1.1.1 2.2.2.2/24");
+        LoadBalancerTO[] lba = new LoadBalancerTO[1];
+        lba[0] = lb;
+        HAProxyConfigurator hpg = new HAProxyConfigurator();
+        LoadBalancerConfigCommand cmd = new LoadBalancerConfigCommand(lba, "10.0.0.1", "10.1.0.1", "10.1.1.1", null, 1L, "12", false);
+        String result = genConfig(hpg, cmd);
+        Assert.assertTrue(result.contains("acl network_allowed src 1.1.1.1 2.2.2.2/24 \n\ttcp-request connection reject if !network_allowed"));
+    }
+
     private String genConfig(HAProxyConfigurator hpg, LoadBalancerConfigCommand cmd) {
         String[] sa = hpg.generateConfiguration(cmd);
         StringBuilder sb = new StringBuilder();
diff --git a/engine/components-api/src/main/java/com/cloud/network/lb/LoadBalancingRulesManager.java b/engine/components-api/src/main/java/com/cloud/network/lb/LoadBalancingRulesManager.java
@@ -34,7 +34,7 @@
 public interface LoadBalancingRulesManager {
 
     LoadBalancer createPublicLoadBalancer(String xId, String name, String description, int srcPort, int destPort, long sourceIpId, String protocol, String algorithm,
-        boolean openFirewall, CallContext caller, String lbProtocol, Boolean forDisplay) throws NetworkRuleConflictException;
+        boolean openFirewall, CallContext caller, String lbProtocol, Boolean forDisplay, String cidrList) throws NetworkRuleConflictException;
 
     boolean removeAllLoadBalanacersForIp(long ipId, Account caller, long callerUserId);
 
diff --git a/engine/schema/src/main/java/com/cloud/network/dao/LoadBalancerVO.java b/engine/schema/src/main/java/com/cloud/network/dao/LoadBalancerVO.java
@@ -29,7 +29,7 @@
 import com.cloud.utils.net.NetUtils;
 
 /**
- * This VO represent Public Load Balancer
+ * This VO represents Public Load Balancer
  * It references source ip address by its Id.
  * To get the VO for Internal Load Balancer rule, please refer to LoadBalancerRuleVO
  *
@@ -62,11 +62,14 @@ public class LoadBalancerVO extends FirewallRuleVO implements LoadBalancer {
     @Column(name = "lb_protocol")
     String lbProtocol;
 
+    @Column(name = "cidr_list")
+    String cidrList;
+
     public LoadBalancerVO() {
     }
 
     public LoadBalancerVO(String xId, String name, String description, long srcIpId, int srcPort, int dstPort, String algorithm, long networkId, long accountId,
-            long domainId, String lbProtocol) {
+            long domainId, String lbProtocol, String cidrList) {
         super(xId, srcIpId, srcPort, NetUtils.TCP_PROTO, networkId, accountId, domainId, Purpose.LoadBalancing, null, null, null, null);
         this.name = name;
         this.description = description;
@@ -75,6 +78,7 @@ public LoadBalancerVO(String xId, String name, String description, long srcIpId,
         this.defaultPortEnd = dstPort;
         this.scheme = Scheme.Public;
         this.lbProtocol = lbProtocol;
+        this.cidrList = cidrList;
     }
 
     @Override
@@ -127,4 +131,9 @@ public void setDescription(String description) {
     public Scheme getScheme() {
         return scheme;
     }
+
+    @Override
+    public String getCidrList() {
+        return cidrList;
+    }
 }
diff --git a/engine/schema/src/main/java/org/apache/cloudstack/lb/ApplicationLoadBalancerRuleVO.java b/engine/schema/src/main/java/org/apache/cloudstack/lb/ApplicationLoadBalancerRuleVO.java
@@ -70,6 +70,9 @@ public class ApplicationLoadBalancerRuleVO extends FirewallRuleVO implements App
     @Column(name = "scheme")
     Scheme scheme;
 
+    @Column(name = "cidr_list")
+    String cidrList = null;
+
     public ApplicationLoadBalancerRuleVO() {
     }
 
@@ -137,4 +140,9 @@ public int getInstancePort() {
         return defaultPortStart;
     }
 
+    @Override
+    public String getCidrList(){
+        return cidrList;
+    }
+
 }
diff --git a/engine/schema/src/main/resources/META-INF/db/schema-41710to41800.sql b/engine/schema/src/main/resources/META-INF/db/schema-41710to41800.sql
@@ -22,4 +22,8 @@
 -- Enable CPU cap for default system offerings;
 UPDATE `cloud`.`service_offering` so
 SET so.limit_cpu_use = 1
-WHERE so.default_use = 1 AND so.vm_type IN ('domainrouter', 'secondarystoragevm', 'consoleproxy', 'internalloadbalancervm', 'elasticloadbalancervm');
+WHERE so.default_use = 1 AND so.vm_type IN ('domainrouter', 'secondarystoragevm', 'consoleproxy', 'internalloadbalancervm', 'elasticloadbalancervm');
+
+-- Add cidr_list column to load_balancing_rules
+ALTER TABLE `cloud`.`load_balancing_rules`
+ADD cidr_list VARCHAR(4096);
diff --git a/plugins/network-elements/elastic-loadbalancer/src/main/java/com/cloud/network/lb/LoadBalanceRuleHandler.java b/plugins/network-elements/elastic-loadbalancer/src/main/java/com/cloud/network/lb/LoadBalanceRuleHandler.java
@@ -358,7 +358,7 @@ private LoadBalancer handleCreateLoadBalancerRuleWithLock(final CreateLoadBalanc
             lb.setSourceIpAddressId(ipId);
 
             result = _lbMgr.createPublicLoadBalancer(lb.getXid(), lb.getName(), lb.getDescription(), lb.getSourcePortStart(), lb.getDefaultPortStart(), ipId.longValue(),
-                    lb.getProtocol(), lb.getAlgorithm(), false, CallContext.current(), lb.getLbProtocol(), true);
+                    lb.getProtocol(), lb.getAlgorithm(), false, CallContext.current(), lb.getLbProtocol(), true, null);
         } catch (final NetworkRuleConflictException e) {
             s_logger.warn("Failed to create LB rule, not continuing with ELB deployment");
             if (newIp) {
diff --git a/plugins/network-elements/internal-loadbalancer/src/main/java/org/apache/cloudstack/network/lb/InternalLoadBalancerVMManagerImpl.java b/plugins/network-elements/internal-loadbalancer/src/main/java/org/apache/cloudstack/network/lb/InternalLoadBalancerVMManagerImpl.java
@@ -467,6 +467,7 @@ private void createApplyLoadBalancingRulesCommands(final List<LoadBalancingRule>
             final List<LbDestination> destinations = rule.getDestinations();
             final List<LbStickinessPolicy> stickinessPolicies = rule.getStickinessPolicies();
             final LoadBalancerTO lb = new LoadBalancerTO(uuid, srcIp, srcPort, protocol, algorithm, revoked, false, inline, destinations, stickinessPolicies);
+            lb.setCidrList(rule.getCidrList());
             lbs[i++] = lb;
         }
 
diff --git a/server/src/main/java/com/cloud/api/ApiResponseHelper.java b/server/src/main/java/com/cloud/api/ApiResponseHelper.java
@@ -1092,6 +1092,8 @@ public LoadBalancerResponse createLoadBalancerResponse(LoadBalancer loadBalancer
         Network ntwk = ApiDBUtils.findNetworkById(loadBalancer.getNetworkId());
         lbResponse.setNetworkId(ntwk.getUuid());
 
+        lbResponse.setCidrList(loadBalancer.getCidrList());
+
         lbResponse.setObjectName("loadbalancer");
         return lbResponse;
     }
diff --git a/server/src/main/java/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java b/server/src/main/java/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java
@@ -33,6 +33,8 @@
 import com.cloud.offerings.dao.NetworkOfferingServiceMapDao;
 import org.apache.cloudstack.acl.SecurityChecker;
 import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.ServerApiException;
 import org.apache.cloudstack.api.command.user.loadbalancer.CreateLBHealthCheckPolicyCmd;
 import org.apache.cloudstack.api.command.user.loadbalancer.CreateLBStickinessPolicyCmd;
 import org.apache.cloudstack.api.command.user.loadbalancer.ListLBHealthCheckPoliciesCmd;
@@ -1590,6 +1592,14 @@ public List<LoadBalancerVMMapVO> doInTransaction(TransactionStatus status) {
     public LoadBalancer createPublicLoadBalancerRule(String xId, String name, String description, int srcPortStart, int srcPortEnd, int defPortStart, int defPortEnd,
         Long ipAddrId, String protocol, String algorithm, long networkId, long lbOwnerId, boolean openFirewall, String lbProtocol, Boolean forDisplay) throws NetworkRuleConflictException,
         InsufficientAddressCapacityException {
+            return createPublicLoadBalancerRule(xId, name, description, srcPortStart, srcPortEnd, defPortStart, defPortEnd, ipAddrId, protocol, algorithm, networkId, lbOwnerId, openFirewall, lbProtocol, forDisplay, null);
+        }
+
+    @Override
+    @ActionEvent(eventType = EventTypes.EVENT_LOAD_BALANCER_CREATE, eventDescription = "creating load balancer")
+    public LoadBalancer createPublicLoadBalancerRule(String xId, String name, String description, int srcPortStart, int srcPortEnd, int defPortStart, int defPortEnd,
+            Long ipAddrId, String protocol, String algorithm, long networkId, long lbOwnerId, boolean openFirewall, String lbProtocol, Boolean forDisplay, List<String> cidrList) throws NetworkRuleConflictException,
+            InsufficientAddressCapacityException {
         Account lbOwner = _accountMgr.getAccount(lbOwnerId);
 
         if (srcPortStart != srcPortEnd) {
@@ -1635,6 +1645,8 @@ public LoadBalancer createPublicLoadBalancerRule(String xId, String name, String
             throw new NetworkRuleConflictException("Can't do load balance on ip address: " + ipVO.getAddress());
         }
 
+        String cidrString = generateCidrString(cidrList);
+
         boolean performedIpAssoc = false;
         try {
             if (ipVO.getAssociatedWithNetworkId() == null) {
@@ -1656,7 +1668,7 @@ public LoadBalancer createPublicLoadBalancerRule(String xId, String name, String
             }
 
             result = createPublicLoadBalancer(xId, name, description, srcPortStart, defPortStart, ipVO.getId(), protocol, algorithm, openFirewall, CallContext.current(),
-                    lbProtocol, forDisplay);
+                    lbProtocol, forDisplay, cidrString);
         } catch (Exception ex) {
             s_logger.warn("Failed to create load balancer due to ", ex);
             if (ex instanceof NetworkRuleConflictException) {
@@ -1685,12 +1697,40 @@ public LoadBalancer createPublicLoadBalancerRule(String xId, String name, String
 
         return result;
     }
+   /**
+    * Transforms the cidrList from a List of Strings to a String which contains all the CIDRs from cidrList separated by whitespaces. This is used to facilitate both the persistence
+    * in the DB and also later when building the configuration String in the getRulesForPool method of the HAProxyConfigurator class.
+   */
+    protected String generateCidrString(List<String> cidrList) {
+        if (cidrList == null) {
+            s_logger.trace("The given CIDR list is null, therefore we will return null.");
+            return null;
+        }
+        String cidrString;
+        StringBuilder sb = new StringBuilder();
+        for (String cidr: cidrList) {
+            cidr = validateCidr(cidr);
+            sb.append(cidr).append(' ');
+        }
+        cidrString = sb.toString();
+        s_logger.trace(String.format("From the cidrList [%s] we generated the following CIDR String [%s].", cidrList, cidrString));
+        return StringUtils.trim(cidrString);
+    }
 
+    private String validateCidr(String cidr) {
+        cidr = StringUtils.trim(cidr);
+        boolean validCidr = NetUtils.isValidIp4Cidr(cidr) || NetUtils.isValidIp6Cidr(cidr);
+        boolean validIp = NetUtils.isValidIp4(cidr) || NetUtils.isValidIp6(cidr);
+        if (!validCidr && !validIp) {
+            throw new ServerApiException(ApiErrorCode.PARAM_ERROR, String.format("CIDR [%s] is invalid.", cidr));
+        }
+        return cidr;
+    }
     @DB
     @Override
     public LoadBalancer createPublicLoadBalancer(final String xId, final String name, final String description, final int srcPort, final int destPort,
  final long sourceIpId,
-            final String protocol, final String algorithm, final boolean openFirewall, final CallContext caller, final String lbProtocol, final Boolean forDisplay)
+            final String protocol, final String algorithm, final boolean openFirewall, final CallContext caller, final String lbProtocol, final Boolean forDisplay, String cidrList)
             throws NetworkRuleConflictException {
 
         if (!NetUtils.isValidPort(destPort)) {
@@ -1734,7 +1774,7 @@ public LoadBalancer createPublicLoadBalancer(final String xId, final String name
 
         LoadBalancerVO newRule =
             new LoadBalancerVO(xId, name, description, sourceIpId, srcPort, destPort, algorithm, networkId, ipAddr.getAllocatedToAccountId(),
-                ipAddr.getAllocatedInDomainId(), lbProtocol);
+                ipAddr.getAllocatedInDomainId(), lbProtocol, cidrList);
 
         // verify rule is supported by Lb provider of the network
         Ip sourceIp = getSourceIp(newRule);
@@ -1750,7 +1790,7 @@ public LoadBalancer createPublicLoadBalancer(final String xId, final String name
             public LoadBalancerVO doInTransaction(TransactionStatus status) throws NetworkRuleConflictException {
                 LoadBalancerVO newRule =
                     new LoadBalancerVO(xId, name, description, sourceIpId, srcPort, destPort, algorithm, networkId, ipAddr.getAllocatedToAccountId(),
-                        ipAddr.getAllocatedInDomainId(), lbProtocol);
+                        ipAddr.getAllocatedInDomainId(), lbProtocol, cidrList);
 
                 if (forDisplay != null) {
                     newRule.setDisplay(forDisplay);
diff --git a/server/src/main/java/com/cloud/network/router/CommandSetupHelper.java b/server/src/main/java/com/cloud/network/router/CommandSetupHelper.java
@@ -328,6 +328,7 @@ public void createApplyLoadBalancingRulesCommands(final List<LoadBalancingRule>
             final List<LbDestination> destinations = rule.getDestinations();
             final List<LbStickinessPolicy> stickinessPolicies = rule.getStickinessPolicies();
             final LoadBalancerTO lb = new LoadBalancerTO(uuid, srcIp, srcPort, protocol, algorithm, revoked, false, inline, destinations, stickinessPolicies);
+            lb.setCidrList(rule.getCidrList());
             lb.setLbProtocol(lb_protocol);
             lbs[i++] = lb;
         }
diff --git a/server/src/test/java/com/cloud/network/lb/AssignLoadBalancerTest.java b/server/src/test/java/com/cloud/network/lb/AssignLoadBalancerTest.java
diff --git a/server/src/test/java/com/cloud/network/lb/LoadBalancingRulesManagerImplTest.java b/server/src/test/java/com/cloud/network/lb/LoadBalancingRulesManagerImplTest.java
diff --git a/server/src/test/java/com/cloud/network/lb/UpdateLoadBalancerTest.java b/server/src/test/java/com/cloud/network/lb/UpdateLoadBalancerTest.java

Original file line number	Diff line number	Diff line change
`@@ -482,4 +482,8 @@ public Ip getSourceIp() {`
`482`	`482`	`public Scheme getScheme() {`
`483`	`483`	`return lb.getScheme();`
`484`	`484`	`}`
	`485`	`+`
	`486`	`+ public String getCidrList(){`
	`487`	`+ return lb.getCidrList();`
	`488`	`+ }`
`485`	`489`	`}`
Original file line number	Diff line number	Diff line change
`@@ -25,4 +25,6 @@ public interface LoadBalancer extends FirewallRule, LoadBalancerContainer {`
`25`	`25`
`26`	`26`	`int getDefaultPortEnd();`
`27`	`27`
	`28`	`+ String getCidrList();`
	`29`	`+`
`28`	`30`	`}`