-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Pick calient 10102 into calico #11458
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.
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
base: master
Are you sure you want to change the base?
Changes from all commits
a601061
a5f65be
ac6e774
0b239d1
b081065
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -15,46 +15,100 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||
| package calc | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| import ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| "slices" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| "strings" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| "unique" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| "github.com/sirupsen/logrus" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| "github.com/tchap/go-patricia/v2/patricia" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| "github.com/projectcalico/calico/felix/ip" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| "github.com/projectcalico/calico/libcalico-go/lib/backend/model" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| "github.com/projectcalico/calico/libcalico-go/lib/set" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Node is represented by cidr as KEY and v1 key data stored in keys. | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Node is represented by cidr as KEY and stores all keys for that CIDR. | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| type IPTrieNode struct { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| cidr ip.CIDR | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| keys []model.Key | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| keys []unique.Handle[model.Key] | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Root of IpTree | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| type IpTrie struct { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| lpmCache *patricia.Trie | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| existingCidrs set.Set[ip.CIDR] | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| // NewIpTrie creates new Patricia trie and Initializes | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| func NewIpTrie() *IpTrie { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| return &IpTrie{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| lpmCache: patricia.NewTrie(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| existingCidrs: set.New[ip.CIDR](), | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| // newIPTrieNode Function creates new empty node containing the CIDR and single key. | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // getLowestSortingKey returns the key with the lexicographically smallest name from a slice of | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // keys. This provides deterministic tie-breaking for network sets with the same prefix length. | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| func getLowestSortingKey(keys []unique.Handle[model.Key]) model.Key { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| if len(keys) == 0 { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| return nil | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| if len(keys) == 1 { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| return keys[0].Value() | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Linear scan to find the lexicographically lowest key | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| lowestHandle := keys[0] | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| lowestKeyStr := lowestHandle.Value().String() | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| for i := 1; i < len(keys); i++ { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| keyStr := keys[i].Value().String() | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| if keyStr < lowestKeyStr { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| lowestHandle = keys[i] | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| lowestKeyStr = keyStr | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| return lowestHandle.Value() | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| func newIPTrieNode(cidr ip.CIDR, key model.Key) *IPTrieNode { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| return &IPTrieNode{cidr: cidr, keys: []model.Key{key}} | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| return &IPTrieNode{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| cidr: cidr, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| keys: []unique.Handle[model.Key]{unique.Make(key)}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| // GetLongestPrefixCidr finds longest prefix match CIDR for the Given IP and if successful return the last key | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // recorded. | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| func (t *IpTrie) GetLongestPrefixCidr(ipAddr ip.Addr) (model.Key, bool) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // NamespaceFilterFunc defines a function that filters keys based on namespace preferences. | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Returns: (isNamespaceMatch, isGlobal, accept) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // - isNamespaceMatch: true if the key matches the preferred namespace | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // - isGlobal: true if the key is a global (non-namespaced) NetworkSet | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // - accept: true if the key should be accepted for processing | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| type NamespaceFilterFunc func(key model.Key, preferredNamespace string) (isNamespaceMatch bool, isGlobal bool, accept bool) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| // DefaultNamespaceFilter is the standard namespace filtering logic | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| func DefaultNamespaceFilter(key model.Key, preferredNamespace string) (isNamespaceMatch bool, isGlobal bool, accept bool) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| nsKey, ok := key.(model.NetworkSetKey) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| if !ok { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| logrus.Debugf("Non-NetworkSet key detected: %v", key) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| return false, false, false // Non-NetworkSet keys are treated as global | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| namespace := nsKey.Namespace() | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| if namespace != "" { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Namespaced NetworkSet | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| isMatch := namespace == preferredNamespace | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| return isMatch, false, true | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Global NetworkSet | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| return false, true, true | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+80
to
+105
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| // NamespaceFilterFunc defines a function that filters keys based on namespace preferences. | |
| // Returns: (isNamespaceMatch, isGlobal, accept) | |
| // - isNamespaceMatch: true if the key matches the preferred namespace | |
| // - isGlobal: true if the key is a global (non-namespaced) NetworkSet | |
| // - accept: true if the key should be accepted for processing | |
| type NamespaceFilterFunc func(key model.Key, preferredNamespace string) (isNamespaceMatch bool, isGlobal bool, accept bool) | |
| // DefaultNamespaceFilter is the standard namespace filtering logic | |
| func DefaultNamespaceFilter(key model.Key, preferredNamespace string) (isNamespaceMatch bool, isGlobal bool, accept bool) { | |
| nsKey, ok := key.(model.NetworkSetKey) | |
| if !ok { | |
| logrus.Debugf("Non-NetworkSet key detected: %v", key) | |
| return false, false, false // Non-NetworkSet keys are treated as global | |
| } | |
| namespace := nsKey.Namespace() | |
| if namespace != "" { | |
| // Namespaced NetworkSet | |
| isMatch := namespace == preferredNamespace | |
| return isMatch, false, true | |
| } | |
| // Global NetworkSet | |
| return false, true, true | |
| } |
Copilot
AI
Nov 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error handling checks len(longestPrefix) == 0 instead of checking if longestItem == nil. This change from the original code could cause a panic if an error occurs but longestPrefix has a non-zero length. The check should be err != nil || longestItem == nil to properly validate that a match was found.
| if err != nil || len(longestPrefix) == 0 { | |
| if err != nil || longestItem == nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment states 'Non-NetworkSet keys are treated as global' but the code returns
accept=false, which means they are rejected, not treated as global. This is inconsistent. Either fix the comment to say 'Non-NetworkSet keys are rejected' or change the implementation to actually treat them as global by returning(false, true, true).