fix: [NPM] add check for valid IPV4 addresses in TranslatePolicy #1738
Conversation
huntergregory
left a comment
huntergregory
left a comment
There was a problem hiding this comment.
left some comments. Could you also add a short description of the current issue and how your PR fixes it?
| klog.Errorf("Failed to translate podSelector in NetworkPolicy %s in namespace %s: %s", netPolObj.ObjectMeta.Name, netPolObj.ObjectMeta.Namespace, err.Error()) | ||
| // The exec time isn't relevant here, so consider a no-op. | ||
| return metrics.NoOp, errNetPolTranslationFailure | ||
| return metrics.NoOp, nil |
There was a problem hiding this comment.
can you add a comment why we return nil here?
| npmNetPol := policies.NewNPMNetworkPolicy(netPolName, npObj.Namespace) | ||
|
|
||
| // check if IPs are valid IPV4 addresses | ||
| for _, egress := range npObj.Spec.Egress { |
There was a problem hiding this comment.
Currently, we perform all error checking in translateRule(), with one exception below in the ad-hoc validation on the final NPMNetworkPolicy. See #1717 for an example of erroring through translateRule().
In general, I feel it's best to keep code consistent so long as it doesn't add complexity. In this situation, it seems there could be a small, clean solution through translateRule().
| // ErrUnsupportedSCTP is returned when SCTP protocol is used in windows. | ||
| ErrUnsupportedSCTP = errors.New("unsupported SCTP protocol used on windows") | ||
| ErrUnsupportedSCTP = errors.New("unsupported SCTP protocol used on windows") | ||
| ErrInvalidIPV6Address = errors.New("invalid IPV6 address") |
There was a problem hiding this comment.
nitpick: seems this error will be thrown for any IPv6 or for an invalid IPv4. Maybe "unsupported" is a better word for IPv6?
* add check for ipv6 addresses in translatePolicy * fix static error lint issue * updated static error name and moved IPV4 logic * moved check to ipBlockRule * updated UT --------- Co-authored-by: Hunter Gregory <42728408+huntergregory@users.noreply.github.com>
Reason for Change:
This fixes an issue where unsupported IPV6 addresses were not being validated at the translate step, causing an error when attempting to add them to the dataplane.
Issue Fixed:
Requirements:
Notes:
Add check for valid IPV4 addresses in the translate policy step