Skip to content

Conversation

zhengxiexie
Copy link
Contributor

✨ What's Changed

  • Add support for 'Isolated' AccessMode in Subnet configuration
  • Update kubebuilder validation enum to include new 'Isolated' option
  • Add test coverage for mapping NSX Subnet with Isolated AccessMode
  • Clean up minor code formatting and documentation improvements

🎯 Motivation

The current Subnet API only supports three AccessMode types: 'Private', 'Public', and 'PrivateTGW'. There is a need to support 'Isolated' AccessMode for specific network segmentation requirements where subnets need to be completely isolated from other network segments.

This enhancement provides:

  • Enhanced Network Segmentation: Allow creation of completely isolated subnets
  • Infrastructure Alignment: Match NSX-T capabilities for isolated networking
  • Flexibility: Give users more options for network design patterns

✅ Testing

  • All existing unit tests pass
  • New test case added for 'Isolated' AccessMode mapping
  • Updated controller tests to use proper type casting for AccessMode
  • Validates that NSX Subnet with 'Isolated' access mode correctly maps to v1alpha1.Subnet

🔧 Technical Details

Before: Subnet AccessMode was limited to three options

// +kubebuilder:validation:Enum=Private;Public;PrivateTGW

After: Added 'Isolated' as a fourth option

// +kubebuilder:validation:Enum=Private;Public;PrivateTGW;Isolated
const AccessModeIsolated string = "Isolated"

Key Changes:

  1. Added AccessModeIsolated constant in subnet_types.go
  2. Updated kubebuilder validation enum to include 'Isolated'
  3. Enhanced NSX-to-CR mapping logic to handle 'Isolated' AccessMode
  4. Added comprehensive test coverage for the new AccessMode
  5. Fixed minor formatting issues and improved documentation clarity

📎 Additional Notes

  • Zero risk change - purely additive functionality
  • Maintains backward compatibility with existing AccessMode values
  • No API breaking changes
  • Aligns with NSX-T subnet isolation capabilities
  • Ready for immediate use in network configurations requiring isolation

@zhengxiexie
Copy link
Contributor Author

/e2e

@codecov-commenter
Copy link

codecov-commenter commented Jul 31, 2025

Codecov Report

❌ Patch coverage is 76.47059% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.81%. Comparing base (051011c) to head (3518096).
⚠️ Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
pkg/nsx/services/subnet/subnet.go 62.50% 3 Missing ⚠️
pkg/nsx/services/subnet/builder.go 66.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1183   +/-   ##
=======================================
  Coverage   73.81%   73.81%           
=======================================
  Files         141      141           
  Lines       22350    22359    +9     
=======================================
+ Hits        16498    16505    +7     
- Misses       4829     4830    +1     
- Partials     1023     1024    +1     
Flag Coverage Δ
unit-tests 73.81% <76.47%> (+<0.01%) ⬆️
Files with missing lines Coverage Δ
pkg/util/utils.go 85.29% <100.00%> (+0.09%) ⬆️
pkg/nsx/services/subnet/builder.go 91.17% <66.66%> (ø)
pkg/nsx/services/subnet/subnet.go 74.00% <62.50%> (+0.50%) ⬆️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@zhengxiexie
Copy link
Contributor Author

/e2e

1 similar comment
@zhengxiexie
Copy link
Contributor Author

/e2e

@zhengxiexie
Copy link
Contributor Author

/e23

@zhengxiexie
Copy link
Contributor Author

/e2e

3 similar comments
@zhengxiexie
Copy link
Contributor Author

/e2e

@zhengxiexie
Copy link
Contributor Author

/e2e

@zhengxiexie
Copy link
Contributor Author

/e2e

}

// Additional DHCP server config for a VPC Subnet.
// DHCPServerAdditionalConfig Additional DHCP server config for a VPC Subnet.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// DHCPServerAdditionalConfig Additional DHCP server config for a VPC Subnet.
// DHCPServerAdditionalConfig defines the additional DHCP server config for a VPC Subnet.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

Comment on lines 15 to 18
AccessModePublic string = "Public"
AccessModePrivate string = "Private"
AccessModeProject string = "PrivateTGW"
AccessModeIsolated string = "Isolated"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not introduced in this PR, would you think we can use AccessMode instead of string here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, updated

IPv4SubnetSize int `json:"ipv4SubnetSize,omitempty"`
// Access mode of Subnet, accessible only from within VPC or from outside VPC.
// +kubebuilder:validation:Enum=Private;Public;PrivateTGW
// +kubebuilder:validation:Enum=Private;Public;PrivateTGW;Isolated
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you run make manifests and make generate-api-docs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

IPv4SubnetSize int `json:"ipv4SubnetSize,omitempty"`
// Access mode of Subnet, accessible only from within VPC or from outside VPC.
// +kubebuilder:validation:Enum=Private;Public;PrivateTGW
// +kubebuilder:validation:Enum=Private;Public;PrivateTGW;Isolated
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you know if we would like to support Isolated AccessMode for Subnetset?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so

@zhengxiexie zhengxiexie force-pushed the topic/zhengxie/main/isolated_subnet_type branch from 55df814 to ea64b48 Compare August 1, 2025 08:16
@zhengxiexie
Copy link
Contributor Author

/e2e

1 similar comment
@zhengxiexie
Copy link
Contributor Author

/e2e

@zhengxiexie zhengxiexie force-pushed the topic/zhengxie/main/isolated_subnet_type branch from ea64b48 to e6c523e Compare August 5, 2025 03:42
@zhengxiexie
Copy link
Contributor Author

/e2e

@zhengxiexie zhengxiexie force-pushed the topic/zhengxie/main/isolated_subnet_type branch from e6c523e to 295f95f Compare August 6, 2025 02:21
@zhengxiexie zhengxiexie force-pushed the topic/zhengxie/main/isolated_subnet_type branch from 295f95f to 3518096 Compare August 6, 2025 02:25
Copy link
Contributor

@yanjunz97 yanjunz97 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lxiaopei
Copy link
Contributor

lxiaopei commented Aug 7, 2025

from NSX API doc https://github-vcf.devops.broadcom.net/vcf/nsx/blob/nsx-main/mp/policy/policy-api/src/main/resources/api_spec/root/nsx-policy/policy-vpc-subnet.yml#L219. "Isolated - VPC Subnet is not accessible from other VPC Subnets within the same VPC.
Please make use of Private, Private_TGW or Public Subnets with connectivity_state as DISCONNECTED to have a disconnected Subnet.
This value is deprecated." "Isolated" access mode is deprecated, I think we do not need to add new access mode "Isolated" in Subnet CR, but indicate user to use "connectivity_state as DISCONNECTED". WDYT?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants