-
Notifications
You must be signed in to change notification settings - Fork 373
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
Add iptables rule builder #5666
Add iptables rule builder #5666
Conversation
This patch provides a simple way to build iptables rules. Signed-off-by: Hongliang Liu <lhongliang@vmware.com>
c1a7c12
to
312c7a5
Compare
|
||
type IPTablesRuleBuilder interface { | ||
MatchIPSetSrc(ipset string) IPTablesRuleBuilder | ||
MatchIPSetDst(ipset string) IPTablesRuleBuilder |
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.
Maybe add an empty line after each method to be more readable.
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.
Will do
|
||
"antrea.io/antrea/pkg/apis/controlplane/v1beta2" | ||
) | ||
|
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 saw there are "type Interface interface" in the iptables.go, maybe it can also be moved here?
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.
Agree
MatchInputInterface(eth0). | ||
MatchTransProtocol(v1beta2.ProtocolTCP). | ||
MatchDstPort(port8080, nil). | ||
SetComment(`"Accept TCP 8080"`). |
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.
Are these double quotes required for the parameter? if yes, can we move it to inside of SetComment
?
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.
Good idea!
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.
Looks like you already include these changes in #5658, please close this PR if it's no longer needed.
OK |
This patch provides a simple way to build iptables rules.