k8snetworkplumbingwg / multi-networkpolicy-iptables

MultiNetworkPolicy iptable based implementation
Apache License 2.0
13 stars 19 forks source link

Use TCP as default for Port.Protocol #12

Closed zeeke closed 2 years ago

zeeke commented 2 years ago

Hi,

While testing MultiNetworkPolicy samples using these NetworkPolicy examples I got a panic in the controller due to a missing protocol field:

I0117 11:28:40.687123       1 iptables.go:442] running iptables: iptables [-w 5 -W 100000 -C PREROUTING -t nat -i net1 -j RETURN]
I0117 11:28:40.688545       1 iptables.go:442] running iptables: iptables [-w 5 -W 100000 -C OUTPUT -t filter -o net1 -j MULTI-EGRESS]
I0117 11:28:40.690063       1 iptables.go:442] running iptables: iptables [-w 5 -W 100000 -C PREROUTING -t nat -i net1 -j RETURN]
I0117 11:28:40.782829       1 iptables.go:337] running iptables-save [-t filter]
I0117 11:28:40.785286       1 server.go:571] ingress/egress = true/false
I0117 11:28:40.785301       1 iptables.go:337] running iptables-save [-t filter]
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x14a6bec]

goroutine 241 [running, locked to thread]:
github.com/k8snetworkplumbingwg/multi-networkpolicy-iptables/pkg/server.(*iptableBuffer).renderIngressPorts(0xc00001db08, 0xc0002a38c0, 0xc002595490, 0x0, 0x0, 0xc0003dfec0, 0x1, 0x1, 0xc000e6df50, 0x1, ...)
    /usr/src/multi-networkpolicy-iptables/pkg/server/policyrules.go:203 +0x28c
github.com/k8snetworkplumbingwg/multi-networkpolicy-iptables/pkg/server.(*iptableBuffer).renderIngress(0xc00001db08, 0xc0002a38c0, 0xc002595490, 0x0, 0xc000437200, 0xc000e6df50, 0x1, 0x1)
    /usr/src/multi-networkpolicy-iptables/pkg/server/policyrules.go:186 +0x3a5
github.com/k8snetworkplumbingwg/multi-networkpolicy-iptables/pkg/server.(*Server).generatePolicyRules(0xc0002a38c0, 0xc001f97400, 0xc002595490, 0x13d5d2a, 0xc0009bd368)
    /usr/src/multi-networkpolicy-iptables/pkg/server/server.go:588 +0xc39
github.com/k8snetworkplumbingwg/multi-networkpolicy-iptables/pkg/server.(*Server).syncMultiPolicy.func1(0x1a047b8, 0xc000d98ce0, 0x0, 0x0)
    /usr/src/multi-networkpolicy-iptables/pkg/server/server.go2022-01-17T11:28:40.787546007Z :473 +0x4d
github.com/containernetworking/plugins/pkg/ns.(*netNS).Do.func1(0x1a047b8, 0xc000d98ce0, 0x0, 0x0)
    /usr/src/multi-networkpolicy-iptables/vendor/github.com/containernetworking/plugins/pkg/ns/ns_linux.go:183 +0x284
github.com/containernetworking/plugins/pkg/ns.(*netNS).Do.func2(0xc0006eae20, 0xc0003b2f48, 0x1a047b8, 0xc000d98ce02022-01-17T11:28:40.787553153Z , 0xc000d98cf0)
    /usr/src/multi-networkpolicy-iptables/vendor/github.com/containernetworking/plugins/pkg/ns/ns_linux.go:200 +0x6b2022-01-17T11:28:40.787557883Z 
created by github.com/containernetworking/plugins/pkg/ns.(*netNS).Do
    /usr/src/multi-networkpolicy-iptables/vendor/github.com/containernetworking/plugins/pkg/ns/ns_linux.go:197 +0x205

According to the NetworkPolicy doc, protocol should be optional a it should fallback to TCP when missing.

Let me know if it can be implemented in a better way.

Andrea

coveralls commented 2 years ago

Pull Request Test Coverage Report for Build 1723546347


Totals Coverage Status
Change from base Build 786282754: 1.3%
Covered Lines: 845
Relevant Lines: 1661

💛 - Coveralls