feat: Extend SG API to have tiers, larger priority range, localAddress and port matches

[abhishek-pandey-1]

Enhances securityGroup API to allow users to fully utilize multiple tiers and do source IP and port filtering if needed.

Pull Request

• Make sure you have followed Kube-OVN Code Style.

What type of this PR

This PR contributes a enhancements to the SecurityGroup feature.

• Users can now use a larger priority range for the SecurityGroup: 1 – 16384. This expansion can be done safely without impacting any existing support for Kubernetes Network policies.
• Users can now optionally define the tier to which a securityGroups will contribute ACL rules and set policy to Pass. This enables stacking of SecurityGroups to do multi-tier ACL processing.
• Users can now also optionally specify localAddress and localPortRanges to do source IP/port based filtering.

More details:

Review of how OVN ACL and kue-ovn

OVN implements a multi-tier ACL:

Total 4 tiers: 0,1,2,3

Each tier can have rules with a priority in the range 0 – 32k.
ACLs are processed starting at tier 0, with tier progressing until a decision is reached.
Tier progression criteria:

• If a match has action “pass”, the ACL processing moves to the next tier.
• If no decision has been made after evaluating all rules in a tier, the tier progresses.
  If all tiers are exhausted, then ACL option set on port-group/switch is checked to see if the packet should be default deny or default allow.

current KubeOVN ACL Usage

Kubeovn has the following interfaces to create/contribute ACL rules to a the ACL evaluated for a port. These are:

• Kubernetes AdminNetworkPolicy
• Kubernetes BaselineNetworkPolicy
• Kubernetes NetworkPolicy
• ACL field on a kubeovn/Subnet resource
• Rules in a kubeovn/SecurityGroup resource

Kubernetes policies

1, 2, and 3 are applied on the default kubernetes network (eth0) of pods selected using namespace or pod selectors.

The behaviour defined by Kubernetes is that:

Admin denies traffic using AdminNetworkPolicies - take precedence over all other network policies and cannot be overridden by network policy.

Admin allows traffic using BaselineAdminNetworkPolicy. Developers can still further restrict this using NetworkPolicies.

KubeOVN current tiered usage

• Tier 1 used for admin policies which range from priority 30000 – 20000

• Tier 2 used for:

  • SecurityGroups 2300 – 2003
  • K8s NetworkPolicy – 2001/2000 (can never override K8s AdminNetworkPolicy)
  • Subnets 0-32000

• Tier 3 used for BaselinNetworkPolicy which range from priority 1800-1700

This means tier 2 and tier 3 have enough space to expand the priority range and tiers used by SecurityGroups.

Behaviour with the changes in this PR

[gemini-code-assist]

Summary of Changes

Hello @abhishek-pandey-1, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the capabilities of the SecurityGroup API by introducing multi-tier support for security policies, expanding the range of rule priorities, and adding new matching criteria for local addresses and ports. These changes provide greater flexibility and control over network traffic management within Kube-OVN, allowing for more complex and finely-tuned security configurations.

Highlights

• Security Group Tiers: Introduced the concept of security group tiers, allowing for a more granular and ordered application of security policies. Security groups can now be assigned a tier (currently between 2 and 3), influencing the order in which ACLs are evaluated.
• Extended Priority Range: Expanded the priority range for security group rules from 1-200 to 1-16384, providing significantly more flexibility in defining the precedence of network policies.
• Local Address and Port Matching: Added new fields (localAddress, localPortRangeMin, localPortRangeMax) to security group rules, enabling policies to match traffic based on the local IP address and port range of the endpoint.
• New 'Pass' Policy Action: Introduced a 'pass' policy action for security group rules, which allows ACL processing to continue to the next tier, offering more sophisticated policy chaining.
• API and Controller Updates: Modified the SecurityGroup CRD, controller logic, and OVN client interfaces to support the new tier, extended priority, local address/port matching, and 'pass' policy features.

Changelog

• charts/kube-ovn/templates/kube-ovn-crd.yaml
  • Updated the policy description for ingress and egress rules to include 'pass' as a valid action.
  • Added localAddress, localPortRangeMin, and localPortRangeMax fields to ingress and egress rules.
  • Introduced securityGroupTier field to the SecurityGroup spec with a default value of 2 and a range of 2 to 3.
• mocks/pkg/ovs/interface.go
  • Modified CreateSgBaseACL and CreateSgDenyAllACL mock methods to accept a tier parameter.
• pkg/apis/kubeovn/v1/security-group.go
  • Defined SgPolicyPass constant for the new 'pass' ACL action.
  • Added SecurityGroupTier field to the SecurityGroupSpec struct.
  • Included LocalAddress, LocalPortRangeMin, and LocalPortRangeMax fields in the SecurityGroupRule struct.
• pkg/controller/security_group.go
  • Updated initDefaultDenyAllSecurityGroup to create deny-all ACLs for all defined security group tiers.
  • Modified handleAddOrUpdateSg to pass the security group's tier when creating base ACLs.
  • Enhanced validateSgRule to include validation for securityGroupTier range and prevent 'pass' policy on the maximum tier.
  • Added validation for localAddress to ensure it's a valid IP or CIDR.
  • Added validation for localPortRangeMin and localPortRangeMax when localAddress is specified.
• pkg/ovs/interface.go
  • Updated ACL interface methods CreateSgDenyAllACL and CreateSgBaseACL to include a tier parameter.
• pkg/ovs/ovn-nb-acl.go
  • Modified CreateSgDenyAllACL to accept and use a tier parameter for ACL creation.
  • Modified CreateSgBaseACL to accept and use a tier parameter for ACL creation.
  • Updated UpdateSgACL to pass the security group's tier when creating allow-related ACLs.
  • Updated newSgRuleACL to accept and use a tier parameter for ACL creation.
  • Refactored newSgRuleACL to correctly handle localAddress and localPortRange matching for both ingress and egress rules.
  • Implemented the 'pass' action for security group rules in newSgRuleACL.
• pkg/ovs/ovn-nb-acl_test.go
  • Updated test cases for CreateGatewayACL, CreateNodeACL, CreateSgDenyAllACL, CreateSgBaseACL, UpdateSgACL, and newSgRuleACL to align with the new tier parameter and expanded priority ranges.
  • Adjusted expected ACL counts and priority values in various test assertions.
• pkg/util/const.go
  • Updated SecurityGroupHighestPriority to reflect the new expanded priority range.
  • Added new constants SecurityGroupPriorityMax, SecurityGroupPriorityMin, SecurityGroupTierMinimum, and SecurityGroupTierMaximum to define the new limits.

Activity

• The pull request was created by abhishek-pandey-1.
• The pull request description indicates it introduces new features.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request enhances the SecurityGroup API by introducing tiers, a larger priority range, and matching on local address and port. The changes are extensive, affecting the CRD, controller logic, and OVS ACL creation. I've identified a couple of critical issues: the CRD validation for rule priority has not been updated to reflect the new, larger range, and there's a bug in the port matching logic for ingress rules that causes them to be ineffective. Additionally, I've suggested a minor refactoring to improve code maintainability in the validation logic.

[coveralls]

Pull Request Test Coverage Report for Build 22989950013

Details

• 119 of 144 (82.64%) changed or added relevant lines in 4 files are covered.
• 6 unchanged lines in 3 files lost coverage.
• Overall coverage increased (+0.1%) to 23.372%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
pkg/controller/network_policy.go	0	2	0.0%
pkg/util/security_group.go	13	17	76.47%
pkg/controller/security_group.go	21	26	80.77%
pkg/ovs/ovn-nb-acl.go	85	99	85.86%

Files with Coverage Reduction	New Missed Lines	%
pkg/controller/security_group.go	1	14.44%
pkg/ovs/ovn-nb-logical_router_route.go	2	74.6%
pkg/ovs/ovn-nb-acl.go	3	77.87%

Totals
Change from base Build 22986226470:	0.1%
Covered Lines:	12809
Relevant Lines:	54804

---

💛 - Coveralls

[abhishek-pandey-1]

@oilbeater these are the tiered ACL changes we discussed a while back. These are fully back-compatible and only provide optional additive features to allow users to stack securityGroups to create a multi-tier ACL filtering and optionally do source ip/port based filtering as well.

I could not think of a sensible e2e test to add for this so I have skipped that and only added UTs. Let me know if there are any concerns.

(Apologies for the delay!)

[oilbeater]

The install.sh should also be updated

[abhishek-pandey-1]

Thanks - I forgot about that one. Do you know if there is a plan to converge to a single CRD? Duplicate CRDs tend to go out of sync. Happy to contribute something here if you think that is a good change?

[oilbeater]

I usually just tell AI to sync them.

[abhishek-pandey-1]

Fair enough haha. Think I broke the tests, let me fix and ping you back for approvals. Feel free to suggest any more changes in the meanwhile.

[abhishek-pandey-1]

Hi - I am a bit perplexed as to why min and max was being rejected for the tier but not for the the other fields like priority, portRange etc. given that min and max are not valid and the correct fields to use is minimum and maximum.

I wanted to look at this more but haven't had time to do so - for now fixed all instances of min and max and think everything is happy now.

[oilbeater]

Thanks! @abhishek-pandey-1 can you also update the doc in https://github.com/kubeovn/docs/blob/master/docs/vpc/security-group.en.md