Add new parameter to createLoadBalancerRule API

[JoaoJandre]

Description

ACL rules are currently not enforced when a Load Balancer is used. When created, ACL rules are implemented in the VR in the FORWARD chain. In order for the LB to work only through the configured IPs, it is necessary to configure the rule in the INPUT chain. However, since all traffic into the VR is being allowed, only forwarding is blocked to the unconfigured IPs.

To work around this situation, it is possible to configure LB rules via createLoadBalancerRule API. However, the cidrlist parameter used to configure the CIDRs that can access the load balancer, had been deprecated in favor of the firewall rule implementation, which is not intuitive and does not support VRs in VPCs. Therefore, the parameter was reimplemented to allow users to create rules in the Load Balancer backend that restrict the access according to the source IP.

Types of changes

• Breaking change (fix or feature that would cause existing functionality to change)
• New feature (non-breaking change which adds functionality)
• Bug fix (non-breaking change which fixes an issue)
• Enhancement (improves an existing feature and functionality)
• Cleanup (Code refactoring and cleanup, that may add test cases)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

• Major
• Minor

Bug Severity

• BLOCKER
• Critical
• Major
• Minor
• Trivial

Screenshots (if appropriate):

How Has This Been Tested?

New unit tests were created. Alongside that, the changes were manually tested in a local lab. By using the createLoadBalancerRule API with the cidrlist parameter to restrict network traffic.

[sonarqubecloud]

SonarCloud Quality Gate failed.   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
3 Code Smells

18.5% Coverage
0.0% Duplication

[weizhouapache]

@JoaoJandre
there are many parameters supported by haproxy but not supported in cloudstack.
I suggest you to collaborate with @ravening on #5799

[JoaoJandre]

@JoaoJandre there are many parameters supported by haproxy but not supported in cloudstack. I suggest you to collaborate with @ravening on #5799

I would rather move on with this PR only, as it is re-adding a feature that was deprecated/removed without a proper alternative. This PR is small, concise, unit tested, and ready to be used in production. The alternative you pointed out is a huge PR that looks a bit too complicated. Therefore, I would rather move on with this small addition to restore the API workings.
Anyways, I can note that PR here, and if we ever need such a featureset, we might be able to help there.

[weizhouapache]

@JoaoJandre there are many parameters supported by haproxy but not supported in cloudstack. I suggest you to collaborate with @ravening on #5799

I would rather move on with this PR only, as it is re-adding a feature that was deprecated/removed without a proper alternative. This PR is small, concise, unit tested, and ready to be used in production. The alternative you pointed out is a huge PR that looks a bit too complicated. Therefore, I would rather move on with this small addition to restore the API workings. Anyways, I can note that PR here, and if we ever need such a featureset, we might be able to help there.

@JoaoJandre
I understand your want to have this feature in main branch as soon as possible.
however, in my opinion, it is not a good idea to add such minor haproxy setting to the load_balancing_rules table (and VO/TO). If we support more haproxy settings by this way, the table/VO/TO will be very big.

#5799 has milestone "4.18.0.0". Once it is merged, it will be much easier to add more haproxy setting like the one you added in this PR.

[GutoVeronezi]

@weizhouapache, PR #5799 has being developed through 2 years from now (originally developed in PR #4141). It passed through 4.15, 4.16, and 4.17, and we do not have guarantee that will be merged in 4.18. Also, there is a lot of changes in #5799, which would require a good documentation and a lot of tests. Therefore, I would rather go with these small changes instead of blocking the feature for something we do not have guarantee.

[github-actions]

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

[weizhouapache]

@weizhouapache, PR #5799 has being developed through 2 years from now (originally developed in PR #4141). It passed through 4.15, 4.16, and 4.17, and we do not have guarantee that will be merged in 4.18. Also, there is a lot of changes in #5799, which would require a good documentation and a lot of tests. Therefore, I would rather go with these small changes instead of blocking the feature for something we do not have guarantee.

@GutoVeronezi @JoaoJandre
as far as I know, #5799 is not beeing developed, tt has been deployed in their production for around two years.
It is ready for review and test. The reason it is not merged is, nobody in cloudstack community has reviewed and tested it.
I hope you guys can review/test it and collaborate with @ravening @soreana on merging #5799 and changes for cidrlist into 4.18

[RodrigoDLopez]

based on tests performed in our labs and code reviews. Lgtm.

[soreana]

@weizhouapache
I have a suggestion for #5799. We can test it gradually, like what we did when we developed it. We can have a branch named HAproxy-LoadBalancer in the community. I will port all the changes from #5799 to HAproxy-LoadBalancer one by one. When satisfied with its quality, we can merge it to the main branch. That way, it would be far easier for community members to test it.

This feature could be part of it, too.

[weizhouapache]

@weizhouapache I have a suggestion for #5799. We can test it gradually, like what we did when we developed it. We can have a branch named HAproxy-LoadBalancer in the community. I will port all the changes from #5799 to HAproxy-LoadBalancer one by one. When satisfied with its quality, we can merge it to the main branch. That way, it would be far easier for community members to test it.

This feature could be part of it, too.

@soreana
Each PR can be merged only if there are at least 2 approvals and all smoke tests pass.
Most users use official release. If someone want to test a feature, they can build and test the specific branch, it is not needed to merge it into main.

[JoaoJandre]

Hey @DaanHoogland could you review and run the tests?

[DaanHoogland]

@blueorangutan package

[blueorangutan]

@DaanHoogland a Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

[blueorangutan]

Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 3900

[DaanHoogland]

@blueorangutan test

[blueorangutan]

@DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

[DaanHoogland]

clgtm needs testing

[blueorangutan]

Trillian test result (tid-4618)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 37051 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6460-t4618-kvm-centos7.zip
Smoke tests completed. 101 look OK, 0 have errors
Only failed tests results shown below:

Test	Result	Time (s)	Test File

[acs-robot]

Found UI changes, kicking a new UI QA build
@blueorangutan ui

[blueorangutan]

@acs-robot a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.

[blueorangutan]

UI build: ✔️
Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/6460 (SL-JID-2086)

[sonarqubecloud]

SonarCloud Quality Gate failed.   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
3 Code Smells

18.5% Coverage
0.0% Duplication