Add From/To fields in SecurityPolicy by wenqiq · Pull Request #1391 · vmware-tanzu/nsx-operator

wenqiq · 2026-03-12T04:31:24Z

Replace the current Sources/Destinations fields with From/To in SecurityPolicy CRD, this aligns with Kubernetes NetworkPolicy naming conventions.
Introduce from/to peer selectors as the preferred fields in SecurityPolicy rules.
Keep sources/destinations for backward compatibility and normalize aliases so from/to take precedence.
Update CRD schemas, samples, docs, and unit tests accordingly.

codecov-commenter · 2026-03-13T03:08:58Z

Codecov Report

❌ Patch coverage is 74.41860% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.75%. Comparing base (315ffc8) to head (b9716ad).

Files with missing lines	Patch %	Lines
pkg/nsx/services/securitypolicy/firewall.go	64.70%	0 Missing and 6 partials ⚠️
pkg/nsx/services/securitypolicy/builder.go	80.95%	0 Missing and 4 partials ⚠️
pkg/nsx/services/securitypolicy/expand.go	80.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1391      +/-   ##
==========================================
- Coverage   75.34%   73.75%   -1.59%     
==========================================
  Files         151      151              
  Lines       21317    21375      +58     
==========================================
- Hits        16062    15766     -296     
- Misses       3784     3815      +31     
- Partials     1471     1794     +323

Flag	Coverage Δ
unit-tests	`73.75% <74.41%> (-1.59%)`	⬇️

Files with missing lines	Coverage Δ
pkg/nsx/services/securitypolicy/expand.go	`69.49% <80.00%> (-3.47%)`	⬇️
pkg/nsx/services/securitypolicy/builder.go	`71.15% <80.95%> (-16.45%)`	⬇️
pkg/nsx/services/securitypolicy/firewall.go	`60.46% <64.70%> (-11.39%)`	⬇️

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

timdengyun · 2026-03-19T03:07:06Z

pkg/apis/legacy/v1alpha1/securitypolicy_types.go

 	Sources []SecurityPolicyPeer `json:"sources,omitempty"`
 	// Destinations defines the endpoints where the traffic is to. For egress rule only.
 	Destinations []SecurityPolicyPeer `json:"destinations,omitempty"`
+	// From is an alias of Sources for ingress rules.


This is for T1 mode, I think we don't need to change it, it's in deprecated stage.

but internally controller logic, we don't have a real internal version of CRD, in fact, we use v1alpha1.SecurityPolicy acting as an internal version, and do CRD conversion.
https://github.com/vmware-tanzu/nsx-operator/blob/main/pkg/controllers/securitypolicy/securitypolicy_controller.go#L149
So, if necessary, we can keep this change in T1 and make both CRD definitions identical except group version.

pkg/apis/vpc/v1alpha1/securitypolicy_types.go

docs/security-policy.md

timdengyun · 2026-03-19T03:15:20Z

pkg/nsx/services/securitypolicy/firewall.go

+// fields so that rule hashing and tagging are independent of which alias the user used.
+func normalizeRulePeers(rule *v1alpha1.SecurityPolicyRule) {
+	if len(rule.Sources) == 0 && len(rule.From) > 0 {
+		rule.Sources = append(rule.Sources, rule.From...)


we might want to keep rule.From and remove rule.Sources later, so, is better copy to rule.From.
This might need more changes where it's referring to rule.Source.

pkg/nsx/services/securitypolicy/firewall.go

timdengyun · 2026-03-19T03:21:08Z

pkg/apis/legacy/v1alpha1/securitypolicy_types.go

 	Sources []SecurityPolicyPeer `json:"sources,omitempty"`
 	// Destinations defines the endpoints where the traffic is to. For egress rule only.
 	Destinations []SecurityPolicyPeer `json:"destinations,omitempty"`
+	// From is an alias of Sources for ingress rules.


but internally controller logic, we don't have a real internal version of CRD, in fact, we use v1alpha1.SecurityPolicy acting as an internal version, and do CRD conversion.
https://github.com/vmware-tanzu/nsx-operator/blob/main/pkg/controllers/securitypolicy/securitypolicy_controller.go#L149
So, if necessary, we can keep this change in T1 and make both CRD definitions identical except group version.

timdengyun · 2026-03-24T10:30:22Z

pkg/nsx/services/securitypolicy/firewall.go

+//
+//nolint:staticcheck // SA1019: must touch deprecated Sources/Destinations to accept legacy CR YAML.
+func normalizeRulePeers(rule *v1alpha1.SecurityPolicyRule) {
+	if len(rule.From) > 0 { //nolint:staticcheck


we should keep rule.From, since we want to deprecate rule.Source.
And fro upgrade case, we need to consider support both rule.Source and rule.From.
For upgrade, copy rule.Source to rule.From.
For greenfield, we can only support From.

Thanks for reviewing. The current code handles this by ensuring compatibility between the two fields. When both "From" and "Sources" are present, the content of "From" is prioritized. To minimize changes, "Sources" is still used internally. Therefore, the condition here is to replace "Sources" with "From" if the "From" field is not empty.

It's ok. we can reduce changes.

pkg/nsx/services/securitypolicy/firewall.go

heypnus

Could you please update the "Testing done" in the patch, especially the upgrade case? Thanks.

heypnus · 2026-03-30T09:08:56Z

pkg/nsx/services/securitypolicy/builder.go

 		nsxRuleDstGroupPath = nsxRule.DestinationGroups[0]
 	} else {
-		if len(rule.Destinations) > 0 {
+		destionations := getRuleDestinationPeers(rule)


typo: destinations

heypnus · 2026-03-30T09:30:22Z

pkg/nsx/services/securitypolicy/firewall.go


+// normalizeRulePeers migrates deprecated Sources/Destinations into From/To and then clears
+// the deprecated fields so that rule hashing and downstream logic only use From/To.
+// For upgrade: if only Sources/Destinations are set, they are copied to From/To.


For the upgrade case, all security policy's hash will change, right? Will it cause the security policy downtime during the upgrade?

Right, please added test cases.

Signed-off-by: Wenqi Qiu <wenqi.qiu@broadcom.com>

wenqiq marked this pull request as ready for review March 17, 2026 13:37

wenqiq requested review from heypnus, lxiaopei and timdengyun March 17, 2026 13:44

timdengyun reviewed Mar 19, 2026

View reviewed changes

wenqiq force-pushed the topic/wenqi/securityPolicy branch from 8359c7b to 2a64255 Compare March 23, 2026 12:27

timdengyun reviewed Mar 24, 2026

View reviewed changes

wenqiq force-pushed the topic/wenqi/securityPolicy branch 2 times, most recently from 4f93a5f to 515745f Compare March 26, 2026 07:31

heypnus reviewed Mar 30, 2026

View reviewed changes

wenqiq force-pushed the topic/wenqi/securityPolicy branch from 52c503c to 6cdf841 Compare April 1, 2026 05:45

Add From/To fields in SecurityPolicy

b9716ad

Signed-off-by: Wenqi Qiu <wenqi.qiu@broadcom.com>

wenqiq force-pushed the topic/wenqi/securityPolicy branch from 6cdf841 to b9716ad Compare April 1, 2026 10:58

Conversation

wenqiq commented Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented Mar 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

heypnus left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

wenqiq commented Mar 12, 2026 •

edited

Loading

codecov-commenter commented Mar 13, 2026 •

edited

Loading