-
Notifications
You must be signed in to change notification settings - Fork 260
feat: [NPM] iptables comments for v2 #1167
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
Conversation
| */ | ||
|
|
||
| func (networkPolicy *NPMNetworkPolicy) commentForJumpToIngress() string { | ||
| return networkPolicy.commentForJump(true) |
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.
Can you change true/false to constant types for ingressJump or egressJump, that was it will be easier to read ?
| return fmt.Sprintf("-ON-%s", string(proto)) | ||
| } | ||
|
|
||
| func (portRange *Ports) comment() string { |
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.
instead of having these .comment() functions for all in this file, can we place these near their type declarations? that will be cleaner in my opinion ? No strong opinion though
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.
I think we should leave as is since comments are only used in Linux, and we don't have a policies_linux.go file
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.
@vakalapa is correct, methods should be defined next to their receivers
the farther a declaration is from the usage, the harder it is to grok
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.
edit: we do have a policies_linux.go file. Might make sense to compile iptables code into one file?
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.
as discussed offline, moving these Linux-specific codes to policy_linux.go
iptables comments will allow us to debug/understand which policies and what ACL rules are which within iptables. For an overview of what the comments will look like, see the description in iptables-comments_linux.go.
This PR also updates the validation check for port/protocol constraints in policy.go so that ACLs with a namedport are required to have
UnspecifiedProtocol