Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support NSX default project for SecurityPolicy #670

Merged
merged 1 commit into from
Sep 11, 2024
Merged

Support NSX default project for SecurityPolicy #670

merged 1 commit into from
Sep 11, 2024

Conversation

timdengyun
Copy link
Contributor

@timdengyun timdengyun commented Aug 6, 2024 •
edited
Loading

Starting from VPC 2.0, it's not allowed to created objects under /orgs/default/projects/default/infra path. So for NetworkPolicy/SecurityPolicy with namespaceSelector, in order to create Groups under Default Project, it's needed to create them under /infra/domains/default/groups/<>. As for Groups under non default Project, it's still allowed to create them under /orgs/default/projects/<custom project>/infra/domains/default/groups.

This patch is to:

  1. Support NetworkPolicy/SecurityPolicy creation under Default Project.
  2. Refactor VPC SecurityPolicy HAPI call process for both creation and deletion.
  3. Refactor VPC SecurityPolicy store apply process after creation and deletion.

Test done:

  1. Create SecurityPolicy and Networkpolicy with namespaceSelector in VPC mode for default project and non-default project, and delete them successfully.
  2. Create SecurityPolicy in T1 mode, and delete them successfully.
  3. Create SecurityPolicy and Networkpolicy with namespaceSelector in VPC mode, and delete them from K8s firstly, and do GC to see created NSX SecurityPolicies are deleted by GC.
  4. Create SecurityPolicy in T1 mode, and delete them from K8s firstly, and do GC to see created NSX SecurityPolicies are deleted by GC.
  5. Create SecurityPolicy with namespaceSelector, hack the code to do an error test:
  • Make operator reboot right after InfraClient.Patch to create infra shares/groups successfully.
  • By that time, SP K8s CR is created with error status. NSX infra shares/groups created, but NSX SecurityPolicy is not created.
  • Reboot operator and make code go the normal process, the NSX SecurityPolicy is created successfully, and SP K8s CR is back to normal status.
  1. Delete SecurityPolicy with namespaceSelector, hack the code to do an error test:
  • Make operator reboot right after OrgRootClient.Patch to delete SecurityPolicy from NSX.
  • By that time, NSX infra shares/groups are not deleted, K8s SP CR is not deleted.
  • Reboot operator and make code go the normal process, K8s SP CR is deleted since there is no the corresponding NSX SecurityPolicy in the store, but we still have infra shares/groups in the store.
  • GC process will delete stale infra shares/groups.

@timdengyun timdengyun force-pushed the support_default_project branch 3 times, most recently from 7866d70 to 512634f Compare August 13, 2024 01:52
@timdengyun timdengyun force-pushed the support_default_project branch 4 times, most recently from 5894d1a to 07dd954 Compare August 25, 2024 09:20
@timdengyun timdengyun force-pushed the support_default_project branch 2 times, most recently from 9bb921a to 6378c2c Compare September 4, 2024 09:09
heypnus
heypnus reviewed Sep 8, 2024
View reviewed changes
pkg/controllers/networkpolicy/networkpolicy_controller.go Show resolved Hide resolved
pkg/nsx/services/securitypolicy/builder.go Outdated Show resolved Hide resolved
pkg/nsx/services/securitypolicy/builder.go Outdated Show resolved Hide resolved
pkg/nsx/services/securitypolicy/builder.go Show resolved Hide resolved
pkg/nsx/services/securitypolicy/builder.go Outdated Show resolved Hide resolved
pkg/nsx/services/securitypolicy/builder.go Show resolved Hide resolved
pkg/nsx/services/securitypolicy/builder.go Outdated Show resolved Hide resolved
heypnus
heypnus previously approved these changes Sep 9, 2024
View reviewed changes
Copy link
Contributor

@heypnus heypnus left a comment

Choose a reason for hiding this comment

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

The logic LGTM. Please correct the typo in commit message:
/orgs/default/projects//infra/domains/default/groups -> /orgs/default/projects/<project-id>/infra/domains/default/groups
HA API -> HAPI

@timdengyun
Copy link
Contributor Author

The logic LGTM. Please correct the typo in commit message: /orgs/default/projects//infra/domains/default/groups -> /orgs/default/projects/<project-id>/infra/domains/default/groups HA API -> HAPI

Thanks so much, Qian, Much appreciation for taking time during vacation to review my code.

zhengxiexie
zhengxiexie reviewed Sep 10, 2024
View reviewed changes
pkg/nsx/services/securitypolicy/firewall.go Outdated Show resolved Hide resolved
pkg/nsx/services/securitypolicy/firewall.go Outdated Show resolved Hide resolved
pkg/nsx/services/securitypolicy/parse.go Show resolved Hide resolved
pkg/nsx/services/securitypolicy/parse.go Outdated Show resolved Hide resolved
@timdengyun
Support NSX default project for SecurityPolicy
4a0a4fb 
Starting from VPC 2.0, it's not allowed to created objects under /orgs/default/projects/default/infra path.
So for NetworkPolicy/SecurityPolicy with namespaceSelector, in order to create Groups under Default Project,
it's needed to create them under /infra/domains/default/groups/<>.
As for Groups under non default Project, it's still allowed to create them
under /orgs/default/projects/<custom project>/infra/domains/default/groups.

This patch is to:
1. Support NetworkPolicy/SecurityPolicy creation under Default Project.
2. Refactor VPC SecurityPolicy HAPI call process for both creation and deletion.
3. Refactor VPC SecurityPolicy store apply process after creation and deletion.
zhengxiexie
zhengxiexie approved these changes Sep 10, 2024
View reviewed changes
Copy link
Contributor

@zhengxiexie zhengxiexie left a comment

Choose a reason for hiding this comment

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

LGTM

@timdengyun
Copy link
Contributor Author

/e2e

@timdengyun timdengyun merged commit d3a36d5 into vmware-tanzu:main Sep 11, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zhengxiexie zhengxiexie zhengxiexie approved these changes

@TaoZou1 TaoZou1 TaoZou1 approved these changes

@heypnus heypnus heypnus left review comments

Assignees
No one assigned
Labels
cla-not-required
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@timdengyun @zhengxiexie @heypnus @TaoZou1 @vmwclabot