refacto: [NPM] Handle 0.0.0.0/0 ipblock and de-duplicated elements in except field #1109
Conversation
JungukCho
requested review from
huntergregory,
matmerr,
neaggarwMS and
vakalapa
| if ipBlockRule.CIDR == "0.0.0.0/0" { | ||
| // two cidrs (0.0.0.0/1 and 128.0.0.0/1) for 0.0.0.0/0 + except. | ||
| members = make([]string, lenOfDeDupExcepts+splitCIDRLen) | ||
| // in case of 0.0.0.0/0, 0.0.0.0/1 or 0.0.0.0/1nomatch comes eariler than 128.0.0.0/1 or 128.0.0.0/1nomatch. |
There was a problem hiding this comment.
nitpick: missing a space or newline
There was a problem hiding this comment.
The comment was about 0.0.0.0/0nomatch'. Changed it to 0.0.0.0/0 nomatch` for better management in comments and codes as well.
| except := deDupExcepts[i] | ||
| if splitCIDRIndex, exist := splitCIDRSet[except]; exist { | ||
| // replace stored splitCIDR with "nomatch" option | ||
| members[splitCIDRIndex] = except + util.IpsetNomatch |
There was a problem hiding this comment.
we will have to update dataplane to be able to understand a cidr block like "1.2.3.4/16nomatch"
There was a problem hiding this comment.
Good catch. Took the ad-hoc approach for linux now, but we will refactor this in next PR if you are ok.
| ipBlockInfo: createIPBlockInfo("test", "default", policies.Ingress, policies.SrcMatch, 0), | ||
| ipBlockRule: &networkingv1.IPBlock{ | ||
| CIDR: "0.0.0.0/0", | ||
| Except: []string{"0.0.0.0/1", "128.0.0.0/1", "128.0.0.0/1"}, |
There was a problem hiding this comment.
by the test name, did you mean to have two "0.0.0.0/1" in the excepts?
There was a problem hiding this comment.
Reorganize UTs for better understanding.
huntergregory
left a comment
huntergregory
left a comment
There was a problem hiding this comment.
just noting that we'll revise data structures to incorporate nomatch approach in a followup PR. We will also need to do integration testing to make sure it works correctly for Linux ipset command
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
2 participants
Reason for Change:
To handle special IPBlock (0.0.0.0/0) and de-duplicated elements in except in translation logic.
Issue Fixed:
Requirements:
Notes:
Instead of using
nomatch, a better data structure to indicate whethercidrorexceptis needed for generic data planes.Will improve it in next PR with revising data structure.
In addition, 0.0.0.0/0 handling was not decided yet in windows dataplane since windows does not support
nomatchforcidryet.Related previous PR: