Skip to content
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

Stop crashing when parsing network policies that only specify protocol and not port #643

Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 66 additions & 25 deletions pkg/controllers/netpol/network_policy_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"encoding/base32"
"errors"
"fmt"
"k8s.io/apimachinery/pkg/util/intstr"
"net"
"regexp"
"strconv"
Expand Down Expand Up @@ -121,6 +122,16 @@ type protocolAndPort struct {
port string
}

func newProtocolAndPort(protocol string, port *intstr.IntOrString) protocolAndPort {
strPort := ""

if port != nil {
strPort = port.String()
}

return protocolAndPort{protocol: protocol, port: strPort}
}

// Run runs forver till we receive notification on stopCh
func (npc *NetworkPolicyController) Run(healthChan chan<- *healthcheck.ControllerHeartbeat, stopCh <-chan struct{}, wg *sync.WaitGroup) {
t := time.NewTicker(npc.syncPeriod)
Expand Down Expand Up @@ -371,16 +382,21 @@ func (npc *NetworkPolicyController) processIngressRules(policy networkPolicyInfo

if len(ingressRule.ports) != 0 {
// case where 'ports' details and 'from' details specified in the ingress rule
// so match on specified source and destination ip's and specified port and protocol
// so match on specified source and destination ip's and specified port (if any) and protocol
for _, portProtocol := range ingressRule.ports {
comment := "rule to ACCEPT traffic from source pods to dest pods selected by policy name " +
policy.name + " namespace " + policy.namespace
args := []string{"-m", "comment", "--comment", comment,
"-m", "set", "--set", srcPodIpSetName, "src",
"-m", "set", "--set", targetDestPodIpSetName, "dst",
"-p", portProtocol.protocol,
"--dport", portProtocol.port,
"-j", "ACCEPT"}
"-p", portProtocol.protocol}

if portProtocol.port != "" {
args = append(args, "--dport", portProtocol.port)
}

args = append(args, "-j", "ACCEPT")

err := iptablesCmdHandler.AppendUnique("filter", policyChainName, args...)
if err != nil {
return fmt.Errorf("Failed to run iptables command: %s", err.Error())
Expand All @@ -403,16 +419,21 @@ func (npc *NetworkPolicyController) processIngressRules(policy networkPolicyInfo
}

// case where only 'ports' details specified but no 'from' details in the ingress rule
// so match on all sources, with specified port and protocol
// so match on all sources, with specified port (if any) and protocol
if ingressRule.matchAllSource && !ingressRule.matchAllPorts {
for _, portProtocol := range ingressRule.ports {
comment := "rule to ACCEPT traffic from all sources to dest pods selected by policy name: " +
policy.name + " namespace " + policy.namespace
args := []string{"-m", "comment", "--comment", comment,
"-m", "set", "--set", targetDestPodIpSetName, "dst",
"-p", portProtocol.protocol,
"--dport", portProtocol.port,
"-j", "ACCEPT"}
"-p", portProtocol.protocol}

if portProtocol.port != "" {
args = append(args, "--dport", portProtocol.port)
}

args = append(args, "-j", "ACCEPT")

err := iptablesCmdHandler.AppendUnique("filter", policyChainName, args...)
if err != nil {
return fmt.Errorf("Failed to run iptables command: %s", err.Error())
Expand Down Expand Up @@ -452,9 +473,14 @@ func (npc *NetworkPolicyController) processIngressRules(policy networkPolicyInfo
args := []string{"-m", "comment", "--comment", comment,
"-m", "set", "--set", srcIpBlockIpSetName, "src",
"-m", "set", "--set", targetDestPodIpSetName, "dst",
"-p", portProtocol.protocol,
"--dport", portProtocol.port,
"-j", "ACCEPT"}
"-p", portProtocol.protocol}

if portProtocol.port != "" {
args = append(args, "--dport", portProtocol.port)
}

args = append(args, "-j", "ACCEPT")

err := iptablesCmdHandler.AppendUnique("filter", policyChainName, args...)
if err != nil {
return fmt.Errorf("Failed to run iptables command: %s", err.Error())
Expand Down Expand Up @@ -516,16 +542,21 @@ func (npc *NetworkPolicyController) processEgressRules(policy networkPolicyInfo,

if len(egressRule.ports) != 0 {
// case where 'ports' details and 'from' details specified in the egress rule
// so match on specified source and destination ip's and specified port and protocol
// so match on specified source and destination ip's and specified port (if any) and protocol
for _, portProtocol := range egressRule.ports {
comment := "rule to ACCEPT traffic from source pods to dest pods selected by policy name " +
policy.name + " namespace " + policy.namespace
args := []string{"-m", "comment", "--comment", comment,
"-m", "set", "--set", targetSourcePodIpSetName, "src",
"-m", "set", "--set", dstPodIpSetName, "dst",
"-p", portProtocol.protocol,
"--dport", portProtocol.port,
"-j", "ACCEPT"}
"-p", portProtocol.protocol}

if portProtocol.port != "" {
args = append(args, "--dport", portProtocol.port)
}

args = append(args, "-j", "ACCEPT")

err := iptablesCmdHandler.AppendUnique("filter", policyChainName, args...)
if err != nil {
return fmt.Errorf("Failed to run iptables command: %s", err.Error())
Expand All @@ -548,16 +579,21 @@ func (npc *NetworkPolicyController) processEgressRules(policy networkPolicyInfo,
}

// case where only 'ports' details specified but no 'to' details in the egress rule
// so match on all sources, with specified port and protocol
// so match on all sources, with specified port (if any) and protocol
if egressRule.matchAllDestinations && !egressRule.matchAllPorts {
for _, portProtocol := range egressRule.ports {
comment := "rule to ACCEPT traffic from source pods to all destinations selected by policy name: " +
policy.name + " namespace " + policy.namespace
args := []string{"-m", "comment", "--comment", comment,
"-m", "set", "--set", targetSourcePodIpSetName, "src",
"-p", portProtocol.protocol,
"--dport", portProtocol.port,
"-j", "ACCEPT"}
"-p", portProtocol.protocol}

if portProtocol.port != "" {
args = append(args, "--dport", portProtocol.port)
}

args = append(args, "-j", "ACCEPT")

err := iptablesCmdHandler.AppendUnique("filter", policyChainName, args...)
if err != nil {
return fmt.Errorf("Failed to run iptables command: %s", err.Error())
Expand Down Expand Up @@ -596,9 +632,14 @@ func (npc *NetworkPolicyController) processEgressRules(policy networkPolicyInfo,
args := []string{"-m", "comment", "--comment", comment,
"-m", "set", "--set", targetSourcePodIpSetName, "src",
"-m", "set", "--set", dstIpBlockIpSetName, "dst",
"-p", portProtocol.protocol,
"--dport", portProtocol.port,
"-j", "ACCEPT"}
"-p", portProtocol.protocol}

if portProtocol.port != "" {
args = append(args, "--dport", portProtocol.port)
}

args = append(args, "-j", "ACCEPT")

err := iptablesCmdHandler.AppendUnique("filter", policyChainName, args...)
if err != nil {
return fmt.Errorf("Failed to run iptables command: %s", err.Error())
Expand Down Expand Up @@ -1127,7 +1168,7 @@ func (npc *NetworkPolicyController) buildNetworkPoliciesInfo() (*[]networkPolicy
} else {
ingressRule.matchAllPorts = false
for _, port := range specIngressRule.Ports {
protocolAndPort := protocolAndPort{protocol: string(*port.Protocol), port: port.Port.String()}
protocolAndPort := newProtocolAndPort(string(*port.Protocol), port.Port)
ingressRule.ports = append(ingressRule.ports, protocolAndPort)
}
}
Expand Down Expand Up @@ -1174,7 +1215,7 @@ func (npc *NetworkPolicyController) buildNetworkPoliciesInfo() (*[]networkPolicy
} else {
egressRule.matchAllPorts = false
for _, port := range specEgressRule.Ports {
protocolAndPort := protocolAndPort{protocol: string(*port.Protocol), port: port.Port.String()}
protocolAndPort := newProtocolAndPort(string(*port.Protocol), port.Port)
egressRule.ports = append(egressRule.ports, protocolAndPort)
}
}
Expand Down Expand Up @@ -1311,7 +1352,7 @@ func (npc *NetworkPolicyController) buildBetaNetworkPoliciesInfo() (*[]networkPo

ingressRule.ports = make([]protocolAndPort, 0)
for _, port := range specIngressRule.Ports {
protocolAndPort := protocolAndPort{protocol: string(*port.Protocol), port: port.Port.String()}
protocolAndPort := newProtocolAndPort(string(*port.Protocol), port.Port)
ingressRule.ports = append(ingressRule.ports, protocolAndPort)
}

Expand Down