Completed config option to allow nested virtualization by bradywilkin · Pull Request #10812 · apache/cloudstack · GitHub

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

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)
build/CI
test (unit or integration test code)

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?

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?

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)
Here are some useful points:

In case of a new feature add useful documentation (raise doc PR at https://github.com/apache/cloudstack-documentation)
Be patient and persistent. It might take some time to get a review or get the final approval from the committers.
Pay attention to the quality of your code, ensure tests are passing and your PR doesn't have conflicts.
Please follow ASF Code of Conduct for all communication including (but not limited to) comments on Pull Requests, Issues, Mailing list and Slack.
Be sure to read the CloudStack Coding Conventions.
Apache CloudStack is a community-driven project and together we are making it better 🚀.
In case of doubts contact the developers at:
Mailing List: dev@cloudstack.apache.org (https://cloudstack.apache.org/mailing-lists.html)
Slack: https://apachecloudstack.slack.com/

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",

[nitpick] Consider adding braces around the single-line if statement to enhance readability and reduce potential errors in future modifications.

Suggested change

      
                    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
          
                    }

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 4.00%. Comparing base (fd74895) to head (7821838).
⚠️ Report is 1068 commits behind head on 4.22.

❗ There is a different number of reports uploaded between BASE (fd74895) and HEAD (7821838). Click for more details.
HEAD has 1 upload less than BASE

Flag BASE (fd74895) HEAD (7821838)

unittests 1 0

Additional details and impacted files

@@              Coverage Diff              @@
##               4.22   #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

Flag	Coverage Δ
uitests	4.00% <ø> (ø)
unittests	?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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

@bradywilkin
Append the new configKey "AllowNestedVMAccess" to the list returned by the getConfigKeys() method in IpAddressManagerImpl. Please refer getConfigKeys().

@bradywilkin Since this is for the 4.22.1 release, could you retarget the PR to the 4.22 branch?

Hi @bradywilkin can you check the build errors, and outstanding comments. thanks.

Completed config option to allow nested virtualization

7821838

boring-cyborg Bot added component:kvm component:networking labels May 4, 2025

DaanHoogland requested review from Copilot, weizhouapache and winterhazel May 5, 2025 08:12

Copilot AI reviewed May 5, 2025

View reviewed changes

weizhouapache reviewed May 5, 2025

View reviewed changes

sureshanaparti added this to the 4.21.0 milestone Jun 5, 2025

sureshanaparti added this to Apache CloudStack 4.21.0 Jun 5, 2025

sureshanaparti moved this to In Progress in Apache CloudStack 4.21.0 Jul 14, 2025

weizhouapache modified the milestones: 4.21.0, 4.22.0 Aug 29, 2025

sureshanaparti removed this from Apache CloudStack 4.21.0 Aug 29, 2025

sureshanaparti added this to Apache CloudStack 4.22.0 Aug 29, 2025

sureshanaparti moved this to In Progress in Apache CloudStack 4.22.0 Aug 29, 2025

pavanaravapalli reviewed Oct 18, 2025

View reviewed changes

harikrishna-patnala modified the milestones: 4.22.0, 4.22.1 Nov 12, 2025

harikrishna-patnala removed this from Apache CloudStack 4.22.0 Nov 12, 2025

rajujith added this to Apache CloudStack 4.22.1 Jan 5, 2026

sureshanaparti moved this to In Progress in Apache CloudStack 4.22.1 Jan 7, 2026

DaanHoogland changed the base branch from main to 4.22 January 13, 2026 12:33

sureshanaparti added the status:waiting-for-author label Apr 24, 2026

sureshanaparti removed this from Apache CloudStack 4.22.1 Apr 24, 2026

sureshanaparti removed this from the 4.22.1 milestone Apr 24, 2026

sureshanaparti added this to the 4.22.2 milestone Apr 24, 2026

Completed config option to allow nested virtualization by bradywilkin · Pull Request #10812 · apache/cloudstack · GitHub

Uh oh!

Completed config option to allow nested virtualization - #10812

Completed config option to allow nested virtualization#10812
bradywilkin wants to merge 1 commit into
apache:4.22from
bradywilkin:config_nested_vm

bradywilkin commented May 4, 2025 •
edited by sureshanaparti
Loading

Uh oh!

Uh oh!

boring-cyborg Bot commented May 4, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI May 5, 2025

Uh oh!

codecov Bot commented May 5, 2025 •
edited
Loading

Uh oh!

Uh oh!

weizhouapache May 5, 2025

Uh oh!

pavanaravapalli Oct 18, 2025 •
edited
Loading

Uh oh!

Uh oh!

rajujith commented Jan 13, 2026

Uh oh!

sureshanaparti commented Feb 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

		if (getAllowNestedVMAccess())
		return true; // If nested VM is allowed, then we skip call to security group and allow bypassing firewall

		@@ -4846,6 +4846,8 @@ protected long getMemoryFreeInKBs(Domain dm) throws LibvirtException {
		}

		private boolean canBridgeFirewall(final String prvNic) {

                   public static final ConfigKey<Integer> PUBLIC_IP_ADDRESS_QUARANTINE_DURATION = new ConfigKey<>("Network", Integer.class, "public.ip.address.quarantine.duration",
                           "0", "The duration (in minutes) for the public IP address to be quarantined when it is disassociated.", true, ConfigKey.Scope.Domain);
+                  public static final ConfigKey<Boolean> AllowNestedVMAccess = new ConfigKey<>("Advanced", Boolean.class, "allow.nested.vm.access",

Completed config option to allow nested virtualization by bradywilkin · Pull Request #10812 · apache/cloudstack · GitHub

Uh oh!

Completed config option to allow nested virtualization - #10812

Completed config option to allow nested virtualization#10812bradywilkin wants to merge 1 commit intoapache:4.22apache/cloudstack:4.22from bradywilkin:config_nested_vmbradywilkin/cloudstack:config_nested_vmCopy head branch name to clipboard

Conversation

bradywilkin commented May 4, 2025 • edited by sureshanaparti Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

Uh oh!

boring-cyborg Bot commented May 4, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI May 5, 2025

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented May 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

weizhouapache May 5, 2025

Choose a reason for hiding this comment

Uh oh!

pavanaravapalli Oct 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rajujith commented Jan 13, 2026

Uh oh!

sureshanaparti commented Feb 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Completed config option to allow nested virtualization#10812
bradywilkin wants to merge 1 commit into
apache:4.22from
bradywilkin:config_nested_vm

bradywilkin commented May 4, 2025 •
edited by sureshanaparti
Loading

codecov Bot commented May 5, 2025 •
edited
Loading

pavanaravapalli Oct 18, 2025 •
edited
Loading