Completed config option to allow nested virtualization #10812
Conversation
|
Congratulations on your first Pull Request and welcome to the Apache CloudStack community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md)
|
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a global configuration option to allow nested virtualization by enabling nested VM access and bypassing security group checks for firewall bridging.
- Introduces a new ConfigKey in IpAddressManagerImpl to control nested VM access.
- Updates LibvirtComputingResource to skip security group verification if nested VM access is enabled.
- Adds a test in IpAddressManagerTest to verify the new behavior.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| server/src/test/java/com/cloud/network/IpAddressManagerTest.java | Adds a test to validate that enabling nested VM access allows firewall bridging. |
| server/src/main/java/com/cloud/network/IpAddressManagerImpl.java | Introduces a new config key for nested VM access and its accessor method. |
| plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java | Modifies the firewall bridging logic to bypass security group checks when nested VM access is allowed. |
Comments suppressed due to low confidence (1)
server/src/main/java/com/cloud/network/IpAddressManagerImpl.java:333
- [nitpick] Consider renaming 'AllowNestedVMAccess' to 'ALLOW_NESTED_VM_ACCESS' for consistency with other static config key naming conventions.
public static final ConfigKey<Boolean> AllowNestedVMAccess = new ConfigKey<>("Advanced", Boolean.class, "allow.nested.vm.access",
| if (getAllowNestedVMAccess()) | ||
| return true; // If nested VM is allowed, then we skip call to security group and allow bypassing firewall |
There was a problem hiding this comment.
[nitpick] Consider adding braces around the single-line if statement to enhance readability and reduce potential errors in future modifications.
| if (getAllowNestedVMAccess()) | |
| return true; // If nested VM is allowed, then we skip call to security group and allow bypassing firewall | |
| if (getAllowNestedVMAccess()) { | |
| return true; // If nested VM is allowed, then we skip call to security group and allow bypassing firewall | |
| } |
Copilot uses AI. Check for mistakes.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #10812 +/- ##
=============================================
- Coverage 16.40% 4.00% -12.41%
=============================================
Files 5692 399 -5293
Lines 501962 32597 -469365
Branches 60791 5783 -55008
=============================================
- Hits 82353 1305 -81048
+ Misses 410455 31142 -379313
+ Partials 9154 150 -9004
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| @@ -4846,6 +4846,8 @@ protected long getMemoryFreeInKBs(Domain dm) throws LibvirtException { | |||
| } | |||
|
|
|||
| private boolean canBridgeFirewall(final String prvNic) { | |||
There was a problem hiding this comment.
the method in LibvirtComputingResource are executed on the kvm host (part of cloudstack-agent)
The host cannot access the database.
so, this won't work.
a feasible way is, adding a setting to agent.properties
Description
This PR...
I introduced a new global configuration option, AllowNestedVMAccess, within the IPAddressManagerImpl.java file. This setting is then referenced in LibVirtComputingResource when determining whether the virtualized firewall can be bridged. If the configuration key is enabled, the system returns true for canBridgeFirewall without running security_group.py. As discussed in the issue itself we first wanted to get a functional basic version completed- later on this should include modifications to the script in secuirty_group.py to allow other functionality to occur while still allowing nested VM access.
#10286
10286
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
I added a test in IPAddressManagerTest.java to ensure that the firewall can be bridged when this setting is enabled.
How did you try to break this feature and the system with this change?