You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2023/01/11 16:52:22 UTC
[GitHub] [cloudstack] RodrigoDLopez opened a new pull request, #7081: enable to create VPC porfowarding rules with source cidr
RodrigoDLopez opened a new pull request, #7081:
URL: https://github.com/apache/cloudstack/pull/7081
### Description
When a guest network is created, ACS allows the configuration of the firewall and port forwarding, load balancing, and VPN rules for all IPs associated with the network (including the source NAT). However, when creating a VPC, firewall configurations are possible only via ACLs; port forwarding and load balancing rules are only possible for other public IPs assigned to the network. Thus, situations, where it is necessary to combine firewall rules and port forwarding/load balancing in VPCs, are not possible.
To work around the situation, this PR implements an extension to allow the definition of source CIDR when creating port forwarding rules. If the `cidrlist` parameter is not informed, the current behavior is maintained. This attribute will only be used when creating port forwarding rules in VPCs, since in guest networks it is possible to combine firewall and port forwarding/load balancing rules directly on the public IPs allocated to that network.
```
(scc) > create portforwardingrule protocol=tcp privateport=22 publicport=22 openfirewall=false vmguestip=10.1.1.248 virtualmachineid=334d4279-a0e6-4c0a-a999-37aabda08acd ipaddressid=5cca136d-5425-44fa-b6af-aafc29df269e networkid=b61abc91-ee24-4578-bae6-fa2d06cf9370 cidrlist=192.168.200.170/32
{
"portforwardingrule": {
"cidrlist": "192.168.200.170/32",
"fordisplay": true,
"id": "210bab98-91f3-4911-8680-5189387a7135",
"ipaddress": "172.16.200.51",
"ipaddressid": "5cca136d-5425-44fa-b6af-aafc29df269e",
"networkid": "b61abc91-ee24-4578-bae6-fa2d06cf9370",
"privateendport": "22",
"privateport": "22",
"protocol": "tcp",
"publicendport": "22",
"publicport": "22",
"state": "Active",
"tags": [],
"virtualmachinedisplayname": "pf-01",
"virtualmachineid": "334d4279-a0e6-4c0a-a999-37aabda08acd",
"virtualmachinename": "pf-01",
"vmguestip": "10.1.1.248"
}
}
```
Furthermore, it was observed that when removing a port forwarding rule, even if the ACS confirmed the removal of said rule, it remained active in the VR. This behavior has changed so that the rule is removed properly.
### Types of changes
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [x] 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
- [ ] Major
- [x] Minor
### How Has This Been Tested?
For testing purposes, I created a VPC: `vpc-01` and a Tier: `vpc-01-tier-01`. Using this tier three instances `PF01`, `PF02`, `PF03` was created and the port forward and ACL rules described below were created.
**Firewall Rules Applied (ACL)**
| Origin | Start Port | End Port | Protocolo |
|------------------|---------------|-----------------|----------------|
| 0.0.0.0/0 | 22 | 22 | TCP (Allow) |
| 172.16.200.170/32| 23 | 23 | TCP (Allow) |
| 172.16.200.170/32| 24 | 24 | TCP (Allow) |
| 0.0.0.0/0 | ----- | ----- | ALL (Deny) |
**Port forward rules**
| Private port | Public port | Protocol | Source CIDR | VM |
|------------------|---------------|---------------|--- |----------------|
| 22 - 22 | 22 - 22 | TCP | --- | PF01 |
| 22 - 22 | 23 - 23 | TCP | --- | PF02 |
| 22 - 22 | 24 - 24 | TCP | --- | PF03 |
This way, after applying the ACL rule that authorizes access to port `22` from any IP (`0.0.0.0/0`); access to other instances (`PF02` and `PF03`) turns out to be exposed. After applying the port forward rule limiting access to a list of CIDRs, the observed result was that access to the resources/services of instances `PF02` and `PF03` was limited to the CIDR informed when creating the port forward rule
**Regras de port forward**
| Private port | Public port | Protocol | Source CIDR | VM |
|------------------|---------------|---------------|--- |----------------|
| 22 - 22 | 22 - 22 | TCP | --- | A |
| 22 - 22 | 23 - 23 | TCP | 172.16.200.170/32| B |
| 22 - 22 | 24 - 24 | TCP | 172.16.200.170/32 | C |
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #7081: enable to create VPC porfowarding rules with source cidr
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on PR #7081:
URL: https://github.com/apache/cloudstack/pull/7081#issuecomment-1380227564
@RodrigoDLopez can you please look at the sonar warning? They look serious in this case.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] enable to create VPC portfowarding rules with source cidr [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7081:
URL: https://github.com/apache/cloudstack/pull/7081#issuecomment-2452846313
<b>[SF] Trillian test result (tid-11732)</b>
Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8
Total time taken: 56878 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7081-t11732-kvm-ol8.zip
Smoke tests completed. 140 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_01_secure_vm_migration | `Error` | 134.16 | test_vm_life_cycle.py
test_01_secure_vm_migration | `Error` | 134.16 | test_vm_life_cycle.py
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] assistanz247 commented on pull request #7081: enable to create VPC porfowarding rules with source cidr
Posted by "assistanz247 (via GitHub)" <gi...@apache.org>.
assistanz247 commented on PR #7081:
URL: https://github.com/apache/cloudstack/pull/7081#issuecomment-1554358002
@RodrigoDLopez
Yes, our use case has been covered in the PR. Thanks a lot :-)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sonarcloud[bot] commented on pull request #7081: enable to create VPC porfowarding rules with source cidr
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #7081:
URL: https://github.com/apache/cloudstack/pull/7081#issuecomment-1379290022
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=7081)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7081&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7081&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7081&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7081&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7081&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7081&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=7081&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=7081&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=7081&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7081&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7081&resolved=false&types=CODE_SMELL) [4 Code Smells](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7081&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=7081&metric=new_coverage&view=list) [3.4% Coverage](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=7081&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=7081&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=7081&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] enable to create VPC portfowarding rules with source cidr [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7081:
URL: https://github.com/apache/cloudstack/pull/7081#issuecomment-2152249484
@RodrigoDLopez can you look at the conflicts?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] enable to create VPC portfowarding rules with source cidr [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7081:
URL: https://github.com/apache/cloudstack/pull/7081#issuecomment-2451511398
@DaanHoogland a [SL] 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] codecov[bot] commented on pull request #7081: enable to create VPC porfowarding rules with source cidr
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #7081:
URL: https://github.com/apache/cloudstack/pull/7081#issuecomment-1379265085
# [Codecov](https://codecov.io/gh/apache/cloudstack/pull/7081?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7081](https://codecov.io/gh/apache/cloudstack/pull/7081?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0b9b44a) into [main](https://codecov.io/gh/apache/cloudstack/commit/a3289f8ac0c611155130cefdadb2e73428478683?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a3289f8) will **increase** coverage by `0.17%`.
> The diff coverage is `0.00%`.
```diff
@@ Coverage Diff @@
## main #7081 +/- ##
============================================
+ Coverage 11.59% 11.76% +0.17%
- Complexity 7560 7661 +101
============================================
Files 2494 2503 +9
Lines 247109 245973 -1136
Branches 38619 38377 -242
============================================
+ Hits 28646 28946 +300
+ Misses 214719 213262 -1457
- Partials 3744 3765 +21
```
| [Impacted Files](https://codecov.io/gh/apache/cloudstack/pull/7081?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../com/cloud/network/rules/PortForwardingRuleVO.java](https://codecov.io/gh/apache/cloudstack/pull/7081?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZW5naW5lL3NjaGVtYS9zcmMvbWFpbi9qYXZhL2NvbS9jbG91ZC9uZXR3b3JrL3J1bGVzL1BvcnRGb3J3YXJkaW5nUnVsZVZPLmphdmE=) | `11.53% <0.00%> (-0.97%)` | :arrow_down: |
| [.../network/rules/dao/PortForwardingRulesDaoImpl.java](https://codecov.io/gh/apache/cloudstack/pull/7081?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZW5naW5lL3NjaGVtYS9zcmMvbWFpbi9qYXZhL2NvbS9jbG91ZC9uZXR3b3JrL3J1bGVzL2Rhby9Qb3J0Rm9yd2FyZGluZ1J1bGVzRGFvSW1wbC5qYXZh) | `39.02% <0.00%> (-3.65%)` | :arrow_down: |
| [...KubernetesClusterResourceModifierActionWorker.java](https://codecov.io/gh/apache/cloudstack/pull/7081?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGx1Z2lucy9pbnRlZ3JhdGlvbnMva3ViZXJuZXRlcy1zZXJ2aWNlL3NyYy9tYWluL2phdmEvY29tL2Nsb3VkL2t1YmVybmV0ZXMvY2x1c3Rlci9hY3Rpb253b3JrZXJzL0t1YmVybmV0ZXNDbHVzdGVyUmVzb3VyY2VNb2RpZmllckFjdGlvbldvcmtlci5qYXZh) | `0.00% <ø> (ø)` | |
| [...java/com/cloud/network/rules/RulesManagerImpl.java](https://codecov.io/gh/apache/cloudstack/pull/7081?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2VydmVyL3NyYy9tYWluL2phdmEvY29tL2Nsb3VkL25ldHdvcmsvcnVsZXMvUnVsZXNNYW5hZ2VySW1wbC5qYXZh) | `0.23% <0.00%> (-0.01%)` | :arrow_down: |
| [...er/src/main/java/com/cloud/hypervisor/KVMGuru.java](https://codecov.io/gh/apache/cloudstack/pull/7081?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2VydmVyL3NyYy9tYWluL2phdmEvY29tL2Nsb3VkL2h5cGVydmlzb3IvS1ZNR3VydS5qYXZh) | `50.81% <0.00%> (-22.41%)` | :arrow_down: |
| [...om/cloud/deploy/DeploymentPlanningManagerImpl.java](https://codecov.io/gh/apache/cloudstack/pull/7081?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2VydmVyL3NyYy9tYWluL2phdmEvY29tL2Nsb3VkL2RlcGxveS9EZXBsb3ltZW50UGxhbm5pbmdNYW5hZ2VySW1wbC5qYXZh) | `28.17% <0.00%> (-0.76%)` | :arrow_down: |
| [...ava/com/cloud/api/dispatch/ParamProcessWorker.java](https://codecov.io/gh/apache/cloudstack/pull/7081?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2VydmVyL3NyYy9tYWluL2phdmEvY29tL2Nsb3VkL2FwaS9kaXNwYXRjaC9QYXJhbVByb2Nlc3NXb3JrZXIuamF2YQ==) | `13.82% <0.00%> (-0.29%)` | :arrow_down: |
| [...n/java/com/cloud/storage/VolumeApiServiceImpl.java](https://codecov.io/gh/apache/cloudstack/pull/7081?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2VydmVyL3NyYy9tYWluL2phdmEvY29tL2Nsb3VkL3N0b3JhZ2UvVm9sdW1lQXBpU2VydmljZUltcGwuamF2YQ==) | `12.99% <0.00%> (-0.03%)` | :arrow_down: |
| [.../cloud/configuration/ConfigurationManagerImpl.java](https://codecov.io/gh/apache/cloudstack/pull/7081?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2VydmVyL3NyYy9tYWluL2phdmEvY29tL2Nsb3VkL2NvbmZpZ3VyYXRpb24vQ29uZmlndXJhdGlvbk1hbmFnZXJJbXBsLmphdmE=) | `12.48% <0.00%> (-0.02%)` | :arrow_down: |
| [...in/java/com/cloud/server/ManagementServerImpl.java](https://codecov.io/gh/apache/cloudstack/pull/7081?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2VydmVyL3NyYy9tYWluL2phdmEvY29tL2Nsb3VkL3NlcnZlci9NYW5hZ2VtZW50U2VydmVySW1wbC5qYXZh) | `5.72% <0.00%> (-0.01%)` | :arrow_down: |
| ... and [41 more](https://codecov.io/gh/apache/cloudstack/pull/7081?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on pull request #7081: enable to create VPC porfowarding rules with source cidr
Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on PR #7081:
URL: https://github.com/apache/cloudstack/pull/7081#issuecomment-1556639971
> @RodrigoDLopez
>
> Yes, our use case has been covered in the PR. Thanks a lot :-)
cool, good to know it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] enable to create VPC portfowarding rules with source cidr [cloudstack]
Posted by "winterhazel (via GitHub)" <gi...@apache.org>.
winterhazel commented on code in PR #7081:
URL: https://github.com/apache/cloudstack/pull/7081#discussion_r1811012616
##########
api/src/main/java/org/apache/cloudstack/api/command/user/firewall/CreatePortForwardingRuleCmd.java:
##########
@@ -107,8 +108,13 @@ 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 CIDR list to allow traffic, all other CIDRs will be blocked. " +
+ "Multiple entries must be separated by a single comma character (,). This param will be used only for VPC's networks. By default, all CIDRs are allowed.")
Review Comment:
This could be extended to also support isolated networks. However, I don't see a point in doing so, as a similar result can be accomplished by combining firewall and port forwarding rules when using isolated networks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] enable to create VPC portfowarding rules with source cidr [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7081:
URL: https://github.com/apache/cloudstack/pull/7081#issuecomment-1812676241
ping @RodrigoDLopez
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on pull request #7081: enable to create VPC porfowarding rules with source cidr
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on PR #7081:
URL: https://github.com/apache/cloudstack/pull/7081#issuecomment-1381423356
Hi @RodrigoDLopez
Have you tested
- create with a list of cidrs ?
- same operation on isolated networks ?
- create port forwarding rule with same public and same private port, but different cidr
- create port forwarding rule with same public and different private port
- update cidrlist of port forwarding rule ?
- restart vpc with/without cleanup
- restart vpc router
something are missing
- ui changes
- unit tests
- marvin test
Please also ask one of your colleagues to test it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] enable to create VPC portfowarding rules with source cidr [cloudstack]
Posted by "winterhazel (via GitHub)" <gi...@apache.org>.
winterhazel commented on code in PR #7081:
URL: https://github.com/apache/cloudstack/pull/7081#discussion_r1811007783
##########
systemvm/debian/opt/cloud/bin/cs/CsNetfilter.py:
##########
@@ -339,6 +339,9 @@ def to_str(self, delete=False):
printable = printable.replace('!_-', '! -')
if delete:
printable = printable.replace('-A', '-D')
+ if k == "--save-mark":
Review Comment:
Hi Wei,
I talked with Rodrigo, and he said that this change was made to fix an error when deleting port forwarding rules where the VR couldn't find the correct rule to delete. We couldn't figure out the exact situation that resulted in this error though.
From what I've tested so far, this change does not seem necessary (at least anymore). Maybe the problem that caused the error got fixed in another PR. Therefore, I'm going to remove these lines, finish adjusting this PR and do some extensive testing. If it really does not seem to affect anything, I'll leave the PR without these changes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] enable to create VPC portfowarding rules with source cidr [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7081:
URL: https://github.com/apache/cloudstack/pull/7081#issuecomment-2454285388
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 11493
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] enable to create VPC portfowarding rules with source cidr [cloudstack]
Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on PR #7081:
URL: https://github.com/apache/cloudstack/pull/7081#issuecomment-1991268859
Hi @RodrigoDLopez It seems the issue #7483 is fixed with this PR. Can target this PR for 4.19.1?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] enable to create VPC portfowarding rules with source cidr [cloudstack]
Posted by "winterhazel (via GitHub)" <gi...@apache.org>.
winterhazel commented on code in PR #7081:
URL: https://github.com/apache/cloudstack/pull/7081#discussion_r1811012616
##########
api/src/main/java/org/apache/cloudstack/api/command/user/firewall/CreatePortForwardingRuleCmd.java:
##########
@@ -107,8 +108,13 @@ 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 CIDR list to allow traffic, all other CIDRs will be blocked. " +
+ "Multiple entries must be separated by a single comma character (,). This param will be used only for VPC's networks. By default, all CIDRs are allowed.")
Review Comment:
This could be extended to also support isolated networks. However, I don't see a point in doing so, as the same result can be accomplished by combining firewall and port forwarding rules when using isolated networks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] enable to create VPC portfowarding rules with source cidr [cloudstack]
Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on code in PR #7081:
URL: https://github.com/apache/cloudstack/pull/7081#discussion_r1629623888
##########
api/src/main/java/org/apache/cloudstack/api/command/user/firewall/CreatePortForwardingRuleCmd.java:
##########
@@ -107,8 +108,13 @@ 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,
Review Comment:
I suggest we use SOURCE_CIDR_LIST
##########
api/src/main/java/org/apache/cloudstack/api/command/user/firewall/CreatePortForwardingRuleCmd.java:
##########
@@ -107,8 +108,13 @@ 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 CIDR list to allow traffic, all other CIDRs will be blocked. " +
+ "Multiple entries must be separated by a single comma character (,). This param will be used only for VPC's networks. By default, all CIDRs are allowed.")
Review Comment:
I remember there are some changes for source cidr of load balancing rules. Does it work in isolated networks? If yes, this could be extended to support isolated networks as well.
##########
api/src/main/java/org/apache/cloudstack/api/command/user/firewall/CreatePortForwardingRuleCmd.java:
##########
@@ -107,8 +108,13 @@ 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 CIDR list to allow traffic, all other CIDRs will be blocked. " +
+ "Multiple entries must be separated by a single comma character (,). This param will be used only for VPC's networks. By default, all CIDRs are allowed.")
+ private List<String> cidrList;
Review Comment:
Also sourceCidrList
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] github-actions[bot] commented on pull request #7081: enable to create VPC porfowarding rules with source cidr
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #7081:
URL: https://github.com/apache/cloudstack/pull/7081#issuecomment-1571952213
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] enable to create VPC portfowarding rules with source cidr [cloudstack]
Posted by "winterhazel (via GitHub)" <gi...@apache.org>.
winterhazel commented on PR #7081:
URL: https://github.com/apache/cloudstack/pull/7081#issuecomment-2470904693
> @RodrigoDLopez @winterhazel , are you guys testing this?
@DaanHoogland I already tested it in https://github.com/apache/cloudstack/pull/7081#issuecomment-2450828651, and consider this PR as ready for merge.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] enable to create VPC portfowarding rules with source cidr [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7081:
URL: https://github.com/apache/cloudstack/pull/7081#issuecomment-2458140644
<b>[SF] Trillian test result (tid-11741)</b>
Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8
Total time taken: 46174 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7081-t11741-kvm-ol8.zip
Smoke tests completed. 132 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_01_secure_vm_migration | `Error` | 133.38 | test_vm_life_cycle.py
test_01_secure_vm_migration | `Error` | 133.38 | test_vm_life_cycle.py
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] enable to create VPC portfowarding rules with source cidr [cloudstack]
Posted by "winterhazel (via GitHub)" <gi...@apache.org>.
winterhazel commented on PR #7081:
URL: https://github.com/apache/cloudstack/pull/7081#issuecomment-2450611723
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] enable to create VPC portfowarding rules with source cidr [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7081:
URL: https://github.com/apache/cloudstack/pull/7081#issuecomment-2451620874
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 11480
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] enable to create VPC portfowarding rules with source cidr [cloudstack]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #7081:
URL: https://github.com/apache/cloudstack/pull/7081#issuecomment-2451965817
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] enable to create VPC portfowarding rules with source cidr [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7081:
URL: https://github.com/apache/cloudstack/pull/7081#issuecomment-2454157825
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] enable to create VPC portfowarding rules with source cidr [cloudstack]
Posted by "winterhazel (via GitHub)" <gi...@apache.org>.
winterhazel commented on code in PR #7081:
URL: https://github.com/apache/cloudstack/pull/7081#discussion_r1811010176
##########
api/src/main/java/org/apache/cloudstack/api/command/user/firewall/CreatePortForwardingRuleCmd.java:
##########
@@ -107,8 +108,13 @@ 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,
Review Comment:
I'm keeping the param as `cidrlist` for now so that it matches the `createLoadBalancerRule` API and so that we don't add another parameter or change an existing param's name while we don't have a well-defined protocol to introduce breaking changes. However, I'll address it as "source CIDR list" in the UI and change its description to provide a better idea on what the parameter represents.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] enable to create VPC porfowarding rules with source cidr [cloudstack]
Posted by "shwstppr (via GitHub)" <gi...@apache.org>.
shwstppr commented on PR #7081:
URL: https://github.com/apache/cloudstack/pull/7081#issuecomment-1752557250
@RodrigoDLopez can you please address the merge conflicts and check review comments
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] enable to create VPC portfowarding rules with source cidr [cloudstack]
Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on code in PR #7081:
URL: https://github.com/apache/cloudstack/pull/7081#discussion_r1811111096
##########
systemvm/debian/opt/cloud/bin/cs/CsNetfilter.py:
##########
@@ -339,6 +339,9 @@ def to_str(self, delete=False):
printable = printable.replace('!_-', '! -')
if delete:
printable = printable.replace('-A', '-D')
+ if k == "--save-mark":
Review Comment:
Thanks a lot for your reply and testing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] enable to create VPC portfowarding rules with source cidr [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7081:
URL: https://github.com/apache/cloudstack/pull/7081#issuecomment-2450613735
@winterhazel a [SL] 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] enable to create VPC portfowarding rules with source cidr [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7081:
URL: https://github.com/apache/cloudstack/pull/7081#issuecomment-2450717673
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 11476
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] enable to create VPC portfowarding rules with source cidr [cloudstack]
Posted by "winterhazel (via GitHub)" <gi...@apache.org>.
winterhazel commented on PR #7081:
URL: https://github.com/apache/cloudstack/pull/7081#issuecomment-2450828651
Everything seems to be working as intended now. Below are the tests I did.
## Tests
For each test, I created/updated/deleted port forwarding rules via the UI, and verified that the VR had the expected iptables rules. I also tried to access the public ports via different CIDRs to ensure that the packets were being routed as intended.
### 1. Create PF rule with no source CIDR:
```sh
root@r-36-VM:~# iptables -t nat -L
Chain PREROUTING (policy ACCEPT)
target prot opt source destination
DNAT tcp -- anywhere 192.168.122.18 tcp dpt:24 to:10.0.0.199:22
Chain INPUT (policy ACCEPT)
target prot opt source destination
Chain OUTPUT (policy ACCEPT)
target prot opt source destination
DNAT tcp -- anywhere 192.168.122.18 tcp dpt:24 to:10.0.0.199:22
Chain POSTROUTING (policy ACCEPT)
target prot opt source destination
SNAT all -- anywhere anywhere to:192.168.122.17
SNAT tcp -- 10.0.0.0/24 test-vm tcp dpt:ssh to:10.0.0.1
```
### 2. Delete PF rule with no source CIDR:
I deleted the previously created rule.
```sh
root@r-36-VM:~# iptables -t nat -L
Chain PREROUTING (policy ACCEPT)
target prot opt source destination
Chain INPUT (policy ACCEPT)
target prot opt source destination
Chain OUTPUT (policy ACCEPT)
target prot opt source destination
Chain POSTROUTING (policy ACCEPT)
target prot opt source destination
SNAT all -- anywhere anywhere to:192.168.122.17
```
### 3. Create PF rule with a single source CIDR:
```sh
Chain PREROUTING (policy ACCEPT)
target prot opt source destination
DNAT tcp -- 192.168.200.0/24 192.168.122.18 tcp dpt:telnet to:10.0.0.199:22
Chain INPUT (policy ACCEPT)
target prot opt source destination
Chain OUTPUT (policy ACCEPT)
target prot opt source destination
DNAT tcp -- 192.168.200.0/24 192.168.122.18 tcp dpt:telnet to:10.0.0.199:22
Chain POSTROUTING (policy ACCEPT)
target prot opt source destination
SNAT all -- anywhere anywhere to:192.168.122.17
SNAT tcp -- 10.0.0.0/24 test-vm tcp dpt:ssh to:10.0.0.1
```
### 4. Delete PF rule with a single source CIDR:
I deleted the previously created rule.
```sh
root@r-36-VM:~# iptables -t nat -L
Chain PREROUTING (policy ACCEPT)
target prot opt source destination
Chain INPUT (policy ACCEPT)
target prot opt source destination
Chain OUTPUT (policy ACCEPT)
target prot opt source destination
Chain POSTROUTING (policy ACCEPT)
target prot opt source destination
SNAT all -- anywhere anywhere to:192.168.122.17
```
### 5. Create PF rule with a list of source CIDRs:
```sh
root@r-36-VM:~# iptables -t nat -L
Chain PREROUTING (policy ACCEPT)
target prot opt source destination
DNAT tcp -- 192.168.122.1 192.168.122.18 tcp dpt:telnet to:10.0.0.199:22
DNAT tcp -- 192.168.201.0/24 192.168.122.18 tcp dpt:telnet to:10.0.0.199:22
DNAT tcp -- 192.168.202.0/24 192.168.122.18 tcp dpt:telnet to:10.0.0.199:22
Chain INPUT (policy ACCEPT)
target prot opt source destination
Chain OUTPUT (policy ACCEPT)
target prot opt source destination
DNAT tcp -- 192.168.122.1 192.168.122.18 tcp dpt:telnet to:10.0.0.199:22
DNAT tcp -- 192.168.201.0/24 192.168.122.18 tcp dpt:telnet to:10.0.0.199:22
DNAT tcp -- 192.168.202.0/24 192.168.122.18 tcp dpt:telnet to:10.0.0.199:22
Chain POSTROUTING (policy ACCEPT)
target prot opt source destination
SNAT all -- anywhere anywhere to:192.168.122.17
SNAT tcp -- 10.0.0.0/24 test-vm tcp dpt:ssh to:10.0.0.1
```
### 6. Update source CIDR list of PF rule to another list:
```sh
(admin) 🐢 > update portforwardingrule id=650ec01f-9e5e-4b70-9ff2-ab8c9cff2051 cidrlist="192.168.202.0/24,192.168.203.0/24,192.168.204.10/32"
{
"portforwardingrule": {
"cidrlist": "192.168.202.0/24,192.168.203.0/24,192.168.204.10/32",
"fordisplay": true,
"id": "650ec01f-9e5e-4b70-9ff2-ab8c9cff2051",
"ipaddress": "192.168.122.18",
"ipaddressid": "7118532f-a6be-4943-bc06-037ea00ef4d0",
"networkid": "49a35e15-ea93-4434-a158-0ffbd3743de1",
"privateendport": "22",
"privateport": "22",
"protocol": "tcp",
"publicendport": "23",
"publicport": "23",
"state": "Active",
"tags": [],
"virtualmachinedisplayname": "test-vm",
"virtualmachineid": "8584e8a2-0759-4aba-885d-8c0b903fdfc2",
"virtualmachinename": "test-vm",
"vmguestip": "10.0.0.199"
}
}
```
```sh
root@r-36-VM:~# iptables -t nat -L
Chain PREROUTING (policy ACCEPT)
target prot opt source destination
DNAT tcp -- 192.168.202.0/24 192.168.122.18 tcp dpt:telnet to:10.0.0.199:22
DNAT tcp -- 192.168.203.0/24 192.168.122.18 tcp dpt:telnet to:10.0.0.199:22
DNAT tcp -- 192.168.204.10 192.168.122.18 tcp dpt:telnet to:10.0.0.199:22
Chain INPUT (policy ACCEPT)
target prot opt source destination
Chain OUTPUT (policy ACCEPT)
target prot opt source destination
DNAT tcp -- 192.168.202.0/24 192.168.122.18 tcp dpt:telnet to:10.0.0.199:22
DNAT tcp -- 192.168.203.0/24 192.168.122.18 tcp dpt:telnet to:10.0.0.199:22
DNAT tcp -- 192.168.204.10 192.168.122.18 tcp dpt:telnet to:10.0.0.199:22
Chain POSTROUTING (policy ACCEPT)
target prot opt source destination
SNAT all -- anywhere anywhere to:192.168.122.17
SNAT tcp -- 10.0.0.0/24 test-vm tcp dpt:ssh to:10.0.0.1
```
### 7. Delete PF rule with a list of source CIDRs:
I deleted the updated rule.
```sh
root@r-36-VM:~# iptables -t nat -L
Chain PREROUTING (policy ACCEPT)
target prot opt source destination
Chain INPUT (policy ACCEPT)
target prot opt source destination
Chain OUTPUT (policy ACCEPT)
target prot opt source destination
Chain POSTROUTING (policy ACCEPT)
target prot opt source destination
SNAT all -- anywhere anywhere to:192.168.122.17
```
### 8. Update source CIDR of PF rule to empty
Before the update:
```sh
root@r-39-VM:~# iptables -t nat -L
Chain PREROUTING (policy ACCEPT)
target prot opt source destination
DNAT tcp -- 192.168.200.0/24 192.168.122.18 tcp dpt:ssh to:10.0.0.199:23
Chain INPUT (policy ACCEPT)
target prot opt source destination
Chain OUTPUT (policy ACCEPT)
target prot opt source destination
DNAT tcp -- 192.168.200.0/24 192.168.122.18 tcp dpt:ssh to:10.0.0.199:23
Chain POSTROUTING (policy ACCEPT)
target prot opt source destination
SNAT all -- anywhere anywhere to:192.168.122.17
SNAT tcp -- 10.0.0.0/24 test-vm tcp dpt:telnet to:10.0.0.1
```
Update:
```sh
(admin) 🐢 > update portforwardingrule id=7340fcb8-4dee-4f74-9bb9-cae92b944783 cidrlist=""
{
"portforwardingrule": {
"cidrlist": "",
"fordisplay": true,
"id": "7340fcb8-4dee-4f74-9bb9-cae92b944783",
"ipaddress": "192.168.122.18",
"ipaddressid": "7118532f-a6be-4943-bc06-037ea00ef4d0",
"networkid": "49a35e15-ea93-4434-a158-0ffbd3743de1",
"privateendport": "23",
"privateport": "23",
"protocol": "tcp",
"publicendport": "22",
"publicport": "22",
"state": "Active",
"tags": [],
"virtualmachinedisplayname": "test-vm",
"virtualmachineid": "8584e8a2-0759-4aba-885d-8c0b903fdfc2",
"virtualmachinename": "test-vm",
"vmguestip": "10.0.0.199"
}
}
```
```sh
root@r-39-VM:~# iptables -t nat -L
Chain PREROUTING (policy ACCEPT)
target prot opt source destination
DNAT tcp -- anywhere 192.168.122.18 tcp dpt:ssh to:10.0.0.199:23
Chain INPUT (policy ACCEPT)
target prot opt source destination
Chain OUTPUT (policy ACCEPT)
target prot opt source destination
DNAT tcp -- anywhere 192.168.122.18 tcp dpt:ssh to:10.0.0.199:23
Chain POSTROUTING (policy ACCEPT)
target prot opt source destination
SNAT all -- anywhere anywhere to:192.168.122.17
SNAT tcp -- 10.0.0.0/24 test-vm tcp dpt:telnet to:10.0.0.1
```
### 9. Create PF rules with the same public and private ports, but different source CIDRs, for the same VM:
```sh
root@r-36-VM:~# iptables -t nat -L
Chain PREROUTING (policy ACCEPT)
target prot opt source destination
DNAT tcp -- 192.168.200.0/24 192.168.122.18 tcp dpt:ssh to:10.0.0.199:22
DNAT tcp -- 192.168.122.1 192.168.122.18 tcp dpt:ssh to:10.0.0.199:22
Chain INPUT (policy ACCEPT)
target prot opt source destination
Chain OUTPUT (policy ACCEPT)
target prot opt source destination
DNAT tcp -- 192.168.200.0/24 192.168.122.18 tcp dpt:ssh to:10.0.0.199:22
DNAT tcp -- 192.168.122.1 192.168.122.18 tcp dpt:ssh to:10.0.0.199:22
Chain POSTROUTING (policy ACCEPT)
target prot opt source destination
SNAT all -- anywhere anywhere to:192.168.122.17
SNAT tcp -- 10.0.0.0/24 test-vm tcp dpt:ssh to:10.0.0.1
```
### 10. Delete one of the previously created rules:
I deleted the rule that had 192.168.200.0/24 as the source CIDR.
```sh
root@r-36-VM:~# iptables -t nat -L
Chain PREROUTING (policy ACCEPT)
target prot opt source destination
DNAT tcp -- 192.168.122.1 192.168.122.18 tcp dpt:ssh to:10.0.0.199:22
Chain INPUT (policy ACCEPT)
target prot opt source destination
Chain OUTPUT (policy ACCEPT)
target prot opt source destination
DNAT tcp -- 192.168.122.1 192.168.122.18 tcp dpt:ssh to:10.0.0.199:22
Chain POSTROUTING (policy ACCEPT)
target prot opt source destination
SNAT all -- anywhere anywhere to:192.168.122.17
SNAT tcp -- 10.0.0.0/24 test-vm tcp dpt:ssh to:10.0.0.1
```
### 11. Create PF rules with the same public and private ports, but different source CIDRs, for different VMs:
```sh
root@r-36-VM:~# iptables -t nat -L
Chain PREROUTING (policy ACCEPT)
target prot opt source destination
DNAT tcp -- 10.0.0.0/24 192.168.122.18 tcp dpt:ssh to:10.0.0.199:22
DNAT tcp -- 192.168.122.0/24 192.168.122.18 tcp dpt:ssh to:10.0.0.186:22
Chain INPUT (policy ACCEPT)
target prot opt source destination
Chain OUTPUT (policy ACCEPT)
target prot opt source destination
DNAT tcp -- 10.0.0.0/24 192.168.122.18 tcp dpt:ssh to:10.0.0.199:22
DNAT tcp -- 192.168.122.0/24 192.168.122.18 tcp dpt:ssh to:10.0.0.186:22
Chain POSTROUTING (policy ACCEPT)
target prot opt source destination
SNAT all -- anywhere anywhere to:192.168.122.17
SNAT tcp -- 10.0.0.0/24 test-vm tcp dpt:ssh to:10.0.0.1
SNAT tcp -- 10.0.0.0/24 test-vm2 tcp dpt:ssh to:10.0.0.1
```
### 12. Create PF rules with the same public port, but different private port for different source CIDRs:
```sh
root@r-36-VM:~# iptables -t nat -L
Chain PREROUTING (policy ACCEPT)
target prot opt source destination
DNAT tcp -- 10.0.0.0/24 192.168.122.18 tcp dpt:ssh to:10.0.0.199:23
DNAT tcp -- 192.168.122.0/24 192.168.122.18 tcp dpt:ssh to:10.0.0.199:22
Chain INPUT (policy ACCEPT)
target prot opt source destination
Chain OUTPUT (policy ACCEPT)
target prot opt source destination
DNAT tcp -- 10.0.0.0/24 192.168.122.18 tcp dpt:ssh to:10.0.0.199:23
DNAT tcp -- 192.168.122.0/24 192.168.122.18 tcp dpt:ssh to:10.0.0.199:22
Chain POSTROUTING (policy ACCEPT)
target prot opt source destination
SNAT all -- anywhere anywhere to:192.168.122.17
SNAT tcp -- 10.0.0.0/24 test-vm tcp dpt:telnet to:10.0.0.1
SNAT tcp -- 10.0.0.0/24 test-vm tcp dpt:ssh to:10.0.0.1
```
### 13. Delete one of the previously created rules:
I deleted the rule that had `192.168.122.0/24` as source CIDR.
```sh
root@r-36-VM:~# iptables -t nat -L
Chain PREROUTING (policy ACCEPT)
target prot opt source destination
DNAT tcp -- 10.0.0.0/24 192.168.122.18 tcp dpt:ssh to:10.0.0.199:23
Chain INPUT (policy ACCEPT)
target prot opt source destination
Chain OUTPUT (policy ACCEPT)
target prot opt source destination
DNAT tcp -- 10.0.0.0/24 192.168.122.18 tcp dpt:ssh to:10.0.0.199:23
Chain POSTROUTING (policy ACCEPT)
target prot opt source destination
SNAT all -- anywhere anywhere to:192.168.122.17
SNAT tcp -- 10.0.0.0/24 test-vm tcp dpt:telnet to:10.0.0.1
```
### 14. Restart VPC with clean-up:
I added a rule that had a list of source CIDRs.
Then, I restarted the VPC with clean-up. After restarting, I verified the iptables.
```sh
root@r-37-VM:~# iptables -t nat -L
Chain PREROUTING (policy ACCEPT)
target prot opt source destination
DNAT tcp -- 192.168.201.0/24 192.168.122.18 tcp dpt:ssh to:10.0.0.199:22
DNAT tcp -- 192.168.202.0/24 192.168.122.18 tcp dpt:ssh to:10.0.0.199:22
DNAT tcp -- 192.168.203.2 192.168.122.18 tcp dpt:ssh to:10.0.0.199:22
Chain INPUT (policy ACCEPT)
target prot opt source destination
Chain OUTPUT (policy ACCEPT)
target prot opt source destination
DNAT tcp -- 192.168.201.0/24 192.168.122.18 tcp dpt:ssh to:10.0.0.199:22
DNAT tcp -- 192.168.202.0/24 192.168.122.18 tcp dpt:ssh to:10.0.0.199:22
DNAT tcp -- 192.168.203.2 192.168.122.18 tcp dpt:ssh to:10.0.0.199:22
Chain POSTROUTING (policy ACCEPT)
target prot opt source destination
SNAT all -- anywhere anywhere to:192.168.122.17
SNAT tcp -- 10.0.0.0/24 test-vm tcp dpt:ssh to:10.0.0.1
```
### 15. Restart VPC without clean-up:
I restarted the same VPC, but without clean-up. After restarting, I verified the iptables.
```sh
root@r-37-VM:~# iptables -t nat -L
Chain PREROUTING (policy ACCEPT)
target prot opt source destination
DNAT tcp -- 192.168.201.0/24 192.168.122.18 tcp dpt:ssh to:10.0.0.199:22
DNAT tcp -- 192.168.202.0/24 192.168.122.18 tcp dpt:ssh to:10.0.0.199:22
DNAT tcp -- 192.168.203.2 192.168.122.18 tcp dpt:ssh to:10.0.0.199:22
Chain INPUT (policy ACCEPT)
target prot opt source destination
Chain OUTPUT (policy ACCEPT)
target prot opt source destination
DNAT tcp -- 192.168.201.0/24 192.168.122.18 tcp dpt:ssh to:10.0.0.199:22
DNAT tcp -- 192.168.202.0/24 192.168.122.18 tcp dpt:ssh to:10.0.0.199:22
DNAT tcp -- 192.168.203.2 192.168.122.18 tcp dpt:ssh to:10.0.0.199:22
Chain POSTROUTING (policy ACCEPT)
target prot opt source destination
SNAT all -- anywhere anywhere to:192.168.122.17
SNAT tcp -- 10.0.0.0/24 test-vm tcp dpt:ssh to:10.0.0.1
```
### 16. Restart VPC router:
I rebooted the VR of the same VPC. After rebooting, I verified the iptables.
```sh
root@r-37-VM:~# iptables -t nat -L
Chain PREROUTING (policy ACCEPT)
target prot opt source destination
DNAT tcp -- 192.168.201.0/24 192.168.122.18 tcp dpt:ssh to:10.0.0.199:22
DNAT tcp -- 192.168.202.0/24 192.168.122.18 tcp dpt:ssh to:10.0.0.199:22
DNAT tcp -- 192.168.203.2 192.168.122.18 tcp dpt:ssh to:10.0.0.199:22
Chain INPUT (policy ACCEPT)
target prot opt source destination
Chain OUTPUT (policy ACCEPT)
target prot opt source destination
DNAT tcp -- 192.168.201.0/24 192.168.122.18 tcp dpt:ssh to:10.0.0.199:22
DNAT tcp -- 192.168.202.0/24 192.168.122.18 tcp dpt:ssh to:10.0.0.199:22
DNAT tcp -- 192.168.203.2 192.168.122.18 tcp dpt:ssh to:10.0.0.199:22
Chain POSTROUTING (policy ACCEPT)
target prot opt source destination
SNAT all -- anywhere anywhere to:192.168.122.17
SNAT tcp -- 10.0.0.0/24 test-vm tcp dpt:ssh to:10.0.0.1
```
### 17. Try to create a PF rule with an invalid source CIDR
I verified that I got an exception.
### 18. Try update a PF rule's source CIDR to an invalid value
Also got an error.
```sh
(admin) 🐢 > update portforwardingrule id=617b41c6-264c-47bb-bdfa-0bd1fdcf2b01 cidrlist="test"
{
"account": "admin",
"accountid": "60b92e66-28b2-11ef-a04e-6ac09c2b3d6b",
"cmd": "org.apache.cloudstack.api.command.user.firewall.UpdatePortForwardingRuleCmd",
"completed": "2024-10-31T19:42:51+0000",
"created": "2024-10-31T19:42:50+0000",
"domainid": "40815ad7-28b2-11ef-a04e-6ac09c2b3d6b",
"domainpath": "ROOT",
"jobid": "08f8132c-c715-4ebf-97ac-5cad81ea0648",
"jobprocstatus": 0,
"jobresult": {
"errorcode": 431,
"errortext": "The given source CIDR [test] is invalid."
},
"jobresultcode": 431,
"jobresulttype": "object",
"jobstatus": 2,
"userid": "60ba931c-28b2-11ef-a04e-6ac09c2b3d6b"
}
```
### 19. Unit tests
I added unit tests for the modified `if` condition, and verified that the condition produced the same results before and after the changes for the cases that did not involve the new parameter.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] enable to create VPC portfowarding rules with source cidr [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7081:
URL: https://github.com/apache/cloudstack/pull/7081#issuecomment-2456426815
@blueorangutan test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] enable to create VPC portfowarding rules with source cidr [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland merged PR #7081:
URL: https://github.com/apache/cloudstack/pull/7081
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] enable to create VPC portfowarding rules with source cidr [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7081:
URL: https://github.com/apache/cloudstack/pull/7081#issuecomment-2470805809
@RodrigoDLopez @winterhazel , are you guys testing this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] enable to create VPC portfowarding rules with source cidr [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7081:
URL: https://github.com/apache/cloudstack/pull/7081#issuecomment-2454159352
@DaanHoogland a [SL] 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] enable to create VPC portfowarding rules with source cidr [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7081:
URL: https://github.com/apache/cloudstack/pull/7081#issuecomment-2456427843
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] enable to create VPC portfowarding rules with source cidr [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7081:
URL: https://github.com/apache/cloudstack/pull/7081#issuecomment-2451733027
@blueorangutan test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] enable to create VPC portfowarding rules with source cidr [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7081:
URL: https://github.com/apache/cloudstack/pull/7081#issuecomment-2451508577
> Everything seems to be working as intended now. Below are the tests I did.
@weizhouapache , does this cover your concerns?
> Hi @RodrigoDLopez It seems the issue #7483 is fixed with this PR. Can target this PR for 4.19.1?
@winterhazel , will you considder this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] enable to create VPC portfowarding rules with source cidr [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7081:
URL: https://github.com/apache/cloudstack/pull/7081#issuecomment-2451508761
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] enable to create VPC portfowarding rules with source cidr [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7081:
URL: https://github.com/apache/cloudstack/pull/7081#issuecomment-2451734899
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] enable to create VPC portfowarding rules with source cidr [cloudstack]
Posted by "winterhazel (via GitHub)" <gi...@apache.org>.
winterhazel commented on PR #7081:
URL: https://github.com/apache/cloudstack/pull/7081#issuecomment-2451880341
> > Hi @RodrigoDLopez It seems the issue #7483 is fixed with this PR. Can target this PR for 4.19.1?
>
> @winterhazel , will you considder this?
My bad, I missed this message. Yes, I'll target 4.19.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on a diff in pull request #7081: enable to create VPC porfowarding rules with source cidr
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on code in PR #7081:
URL: https://github.com/apache/cloudstack/pull/7081#discussion_r1069033684
##########
systemvm/debian/opt/cloud/bin/cs/CsNetfilter.py:
##########
@@ -339,6 +339,9 @@ def to_str(self, delete=False):
printable = printable.replace('!_-', '! -')
if delete:
printable = printable.replace('-A', '-D')
+ if k == "--save-mark":
Review Comment:
@RodrigoDLopez
what's the purpose of this change ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org