From 59cecf8a366c3e0bf19f243d535314db831a9e8e Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Sat, 27 Jan 2024 11:49:11 -0500 Subject: [PATCH] Simplify redundant LocalTrafficDetector implementations All of the LocalTrafficDetector implementations were essentially identical after construction time, so just reduce them to a single implementation with multiple constructors. Also, improve the comments. --- pkg/proxy/util/localdetector.go | 166 +++++++++----------------------- 1 file changed, 47 insertions(+), 119 deletions(-) diff --git a/pkg/proxy/util/localdetector.go b/pkg/proxy/util/localdetector.go index a4c5bc62987..6f2d77a9b0d 100644 --- a/pkg/proxy/util/localdetector.go +++ b/pkg/proxy/util/localdetector.go @@ -22,61 +22,62 @@ import ( netutils "k8s.io/utils/net" ) -// LocalTrafficDetector in a interface to take action (jump) based on whether traffic originated locally -// at the node or not +// LocalTrafficDetector generates iptables or nftables rules to detect traffic from local pods. type LocalTrafficDetector interface { - // IsImplemented returns true if the implementation does something, false otherwise + // IsImplemented returns true if the implementation does something, false + // otherwise. You should not call the other methods if IsImplemented() returns + // false. IsImplemented() bool - // IfLocal returns iptables arguments that will match traffic from a pod + // IfLocal returns iptables arguments that will match traffic from a local pod. IfLocal() []string - // IfNotLocal returns iptables arguments that will match traffic that is not from a pod + // IfNotLocal returns iptables arguments that will match traffic that is not from + // a local pod. IfNotLocal() []string - // IfLocalNFT returns nftables arguments that will match traffic from a pod + // IfLocalNFT returns nftables arguments that will match traffic from a local pod. IfLocalNFT() []string - // IfNotLocalNFT returns nftables arguments that will match traffic that is not from a pod + // IfNotLocalNFT returns nftables arguments that will match traffic that is not + // from a local pod. IfNotLocalNFT() []string } -type noOpLocalDetector struct{} - -// NewNoOpLocalDetector is a no-op implementation of LocalTrafficDetector -func NewNoOpLocalDetector() LocalTrafficDetector { - return &noOpLocalDetector{} -} - -func (n *noOpLocalDetector) IsImplemented() bool { - return false -} - -func (n *noOpLocalDetector) IfLocal() []string { - return nil // no-op; matches all traffic -} - -func (n *noOpLocalDetector) IfNotLocal() []string { - return nil // no-op; matches all traffic -} - -func (n *noOpLocalDetector) IfLocalNFT() []string { - return nil // no-op; matches all traffic -} - -func (n *noOpLocalDetector) IfNotLocalNFT() []string { - return nil // no-op; matches all traffic -} - -type detectLocalByCIDR struct { +type detectLocal struct { ifLocal []string ifNotLocal []string ifLocalNFT []string ifNotLocalNFT []string } -// NewDetectLocalByCIDR implements the LocalTrafficDetector interface using a CIDR. This can be used when a single CIDR -// range can be used to capture the notion of local traffic. +func (d *detectLocal) IsImplemented() bool { + return len(d.ifLocal) > 0 +} + +func (d *detectLocal) IfLocal() []string { + return d.ifLocal +} + +func (d *detectLocal) IfNotLocal() []string { + return d.ifNotLocal +} + +func (d *detectLocal) IfLocalNFT() []string { + return d.ifLocalNFT +} + +func (d *detectLocal) IfNotLocalNFT() []string { + return d.ifNotLocalNFT +} + +// NewNoOpLocalDetector returns a no-op implementation of LocalTrafficDetector. +func NewNoOpLocalDetector() LocalTrafficDetector { + return &detectLocal{} +} + +// NewDetectLocalByCIDR returns a LocalTrafficDetector that considers traffic from the +// provided cidr to be from a local pod, and other traffic to be non-local. func NewDetectLocalByCIDR(cidr string) (LocalTrafficDetector, error) { _, parsed, err := netutils.ParseCIDRSloppy(cidr) if err != nil { @@ -88,7 +89,7 @@ func NewDetectLocalByCIDR(cidr string) (LocalTrafficDetector, error) { nftFamily = "ip6" } - return &detectLocalByCIDR{ + return &detectLocal{ ifLocal: []string{"-s", cidr}, ifNotLocal: []string{"!", "-s", cidr}, ifLocalNFT: []string{nftFamily, "saddr", cidr}, @@ -96,40 +97,14 @@ func NewDetectLocalByCIDR(cidr string) (LocalTrafficDetector, error) { }, nil } -func (d *detectLocalByCIDR) IsImplemented() bool { - return true -} - -func (d *detectLocalByCIDR) IfLocal() []string { - return d.ifLocal -} - -func (d *detectLocalByCIDR) IfNotLocal() []string { - return d.ifNotLocal -} - -func (d *detectLocalByCIDR) IfLocalNFT() []string { - return d.ifLocalNFT -} - -func (d *detectLocalByCIDR) IfNotLocalNFT() []string { - return d.ifNotLocalNFT -} - -type detectLocalByBridgeInterface struct { - ifLocal []string - ifNotLocal []string - ifLocalNFT []string - ifNotLocalNFT []string -} - -// NewDetectLocalByBridgeInterface implements the LocalTrafficDetector interface using a bridge interface name. -// This can be used when a bridge can be used to capture the notion of local traffic from pods. +// NewDetectLocalByBridgeInterface returns a LocalTrafficDetector that considers traffic +// from interfaceName to be from a local pod, and traffic from other interfaces to be +// non-local. func NewDetectLocalByBridgeInterface(interfaceName string) (LocalTrafficDetector, error) { if len(interfaceName) == 0 { return nil, fmt.Errorf("no bridge interface name set") } - return &detectLocalByBridgeInterface{ + return &detectLocal{ ifLocal: []string{"-i", interfaceName}, ifNotLocal: []string{"!", "-i", interfaceName}, ifLocalNFT: []string{"iif", interfaceName}, @@ -137,64 +112,17 @@ func NewDetectLocalByBridgeInterface(interfaceName string) (LocalTrafficDetector }, nil } -func (d *detectLocalByBridgeInterface) IsImplemented() bool { - return true -} - -func (d *detectLocalByBridgeInterface) IfLocal() []string { - return d.ifLocal -} - -func (d *detectLocalByBridgeInterface) IfNotLocal() []string { - return d.ifNotLocal -} - -func (d *detectLocalByBridgeInterface) IfLocalNFT() []string { - return d.ifLocalNFT -} - -func (d *detectLocalByBridgeInterface) IfNotLocalNFT() []string { - return d.ifNotLocalNFT -} - -type detectLocalByInterfaceNamePrefix struct { - ifLocal []string - ifNotLocal []string - ifLocalNFT []string - ifNotLocalNFT []string -} - -// NewDetectLocalByInterfaceNamePrefix implements the LocalTrafficDetector interface using an interface name prefix. -// This can be used when a pod interface name prefix can be used to capture the notion of local traffic. Note -// that this will match on all interfaces that start with the given prefix. +// NewDetectLocalByInterfaceNamePrefix returns a LocalTrafficDetector that considers +// traffic from interfaces starting with interfacePrefix to be from a local pod, and +// traffic from other interfaces to be non-local. func NewDetectLocalByInterfaceNamePrefix(interfacePrefix string) (LocalTrafficDetector, error) { if len(interfacePrefix) == 0 { return nil, fmt.Errorf("no interface prefix set") } - return &detectLocalByInterfaceNamePrefix{ + return &detectLocal{ ifLocal: []string{"-i", interfacePrefix + "+"}, ifNotLocal: []string{"!", "-i", interfacePrefix + "+"}, ifLocalNFT: []string{"iif", interfacePrefix + "*"}, ifNotLocalNFT: []string{"iif", "!=", interfacePrefix + "*"}, }, nil } - -func (d *detectLocalByInterfaceNamePrefix) IsImplemented() bool { - return true -} - -func (d *detectLocalByInterfaceNamePrefix) IfLocal() []string { - return d.ifLocal -} - -func (d *detectLocalByInterfaceNamePrefix) IfNotLocal() []string { - return d.ifNotLocal -} - -func (d *detectLocalByInterfaceNamePrefix) IfLocalNFT() []string { - return d.ifLocalNFT -} - -func (d *detectLocalByInterfaceNamePrefix) IfNotLocalNFT() []string { - return d.ifNotLocalNFT -}