github/kubernetes
4
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Remove errors from LocalTrafficDetector constructors

Browse Source 
The constructors only return an error if you pass them invalid data,
but we only ever pass them data which has already been validated,
making the error checking just annoying. Just make them return garbage
output if you give them garbage input.
This commit is contained in:
Dan Winship
2024-01-28 09:38:55 -05:00
parent 59cecf8a36
commit 3db434d6be
6 changed files with 122 additions and 333 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
pkg/proxy/iptables/proxier_test.go
View File

@@ -93,7 +93,7 @@ func NewFakeProxier(ipt utiliptables.Interface) *Proxier {
ipfamily = v1.IPv6Protocol
podCIDR = "fd00:10::/64"
}
detectLocal, _ := proxyutil.NewDetectLocalByCIDR(podCIDR)
detectLocal := proxyutil.NewDetectLocalByCIDR(podCIDR)
networkInterfacer := proxyutiltest.NewFakeNetwork()
itf := net.Interface{Index: 0, MTU: 0, Name: "lo", HardwareAddr: nil, Flags: 0}

2
pkg/proxy/nftables/proxier_test.go
View File

@@ -84,7 +84,7 @@ func NewFakeProxier(ipFamily v1.IPFamily) (*knftables.Fake, *Proxier) {
podCIDR = "fd00:10::/64"
serviceCIDRs = "fd00:10:96::/112"
}
detectLocal, _ := proxyutil.NewDetectLocalByCIDR(podCIDR)
detectLocal := proxyutil.NewDetectLocalByCIDR(podCIDR)
nodePortAddresses := []string{fmt.Sprintf("%s/32", testNodeIP), fmt.Sprintf("%s/128", testNodeIPv6)}
networkInterfacer := proxyutiltest.NewFakeNetwork()

30
pkg/proxy/util/localdetector.go
View File

@@ -17,8 +17,6 @@ limitations under the License.
package util
import (
"fmt"
netutils "k8s.io/utils/net"
)
@@ -77,15 +75,11 @@ func NewNoOpLocalDetector() LocalTrafficDetector {
}
// 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 {
return nil, err
}
// provided cidr to be from a local pod, and other traffic to be non-local. cidr is
// assumed to be valid.
func NewDetectLocalByCIDR(cidr string) LocalTrafficDetector {
nftFamily := "ip"
if netutils.IsIPv6CIDR(parsed) {
if netutils.IsIPv6CIDRString(cidr) {
nftFamily = "ip6"
}
@@ -94,35 +88,29 @@ func NewDetectLocalByCIDR(cidr string) (LocalTrafficDetector, error) {
ifNotLocal: []string{"!", "-s", cidr},
ifLocalNFT: []string{nftFamily, "saddr", cidr},
ifNotLocalNFT: []string{nftFamily, "saddr", "!=", cidr},
}, nil
}
}
// 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")
}
func NewDetectLocalByBridgeInterface(interfaceName string) LocalTrafficDetector {
return &detectLocal{
ifLocal: []string{"-i", interfaceName},
ifNotLocal: []string{"!", "-i", interfaceName},
ifLocalNFT: []string{"iif", interfaceName},
ifNotLocalNFT: []string{"iif", "!=", interfaceName},
}, nil
}
}
// 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")
}
func NewDetectLocalByInterfaceNamePrefix(interfacePrefix string) LocalTrafficDetector {
return &detectLocal{
ifLocal: []string{"-i", interfacePrefix + "+"},
ifNotLocal: []string{"!", "-i", interfacePrefix + "+"},
ifLocalNFT: []string{"iif", interfacePrefix + "*"},
ifNotLocalNFT: []string{"iif", "!=", interfacePrefix + "*"},
}, nil
}
}

118
pkg/proxy/util/localdetector_test.go
View File

@@ -38,46 +38,6 @@ func TestNoOpLocalDetector(t *testing.T) {
}
}
func TestNewDetectLocalByCIDR(t *testing.T) {
cases := []struct {
cidr string
errExpected bool
}{
{
cidr: "10.0.0.0/14",
errExpected: false,
},
{
cidr: "2002:0:0:1234::/64",
errExpected: false,
},
{
cidr: "10.0.0.0",
errExpected: true,
},
{
cidr: "2002:0:0:1234::",
errExpected: true,
},
{
cidr: "",
errExpected: true,
},
}
for i, c := range cases {
r, err := NewDetectLocalByCIDR(c.cidr)
if c.errExpected {
if err == nil {
t.Errorf("Case[%d] expected error, but succeeded with: %q", i, r)
}
continue
}
if err != nil {
t.Errorf("Case[%d] failed with error: %v", i, err)
}
}
}
func TestDetectLocalByCIDR(t *testing.T) {
cases := []struct {
cidr string
@@ -96,11 +56,7 @@ func TestDetectLocalByCIDR(t *testing.T) {
},
}
for _, c := range cases {
localDetector, err := NewDetectLocalByCIDR(c.cidr)
if err != nil {
t.Errorf("Error initializing localDetector: %v", err)
continue
}
localDetector := NewDetectLocalByCIDR(c.cidr)
if !localDetector.IsImplemented() {
t.Error("DetectLocalByCIDR returns false for IsImplemented")
}
@@ -118,66 +74,6 @@ func TestDetectLocalByCIDR(t *testing.T) {
}
}
func TestNewDetectLocalByBridgeInterface(t *testing.T) {
cases := []struct {
ifaceName string
errExpected bool
}{
{
ifaceName: "avz",
errExpected: false,
},
{
ifaceName: "",
errExpected: true,
},
}
for i, c := range cases {
r, err := NewDetectLocalByBridgeInterface(c.ifaceName)
if c.errExpected {
if err == nil {
t.Errorf("Case[%d] expected error, but succeeded with: %q", i, r)
}
continue
}
if err != nil {
t.Errorf("Case[%d] failed with error: %v", i, err)
}
}
}
func TestNewDetectLocalByInterfaceNamePrefix(t *testing.T) {
cases := []struct {
ifacePrefix string
errExpected bool
}{
{
ifacePrefix: "veth",
errExpected: false,
},
{
ifacePrefix: "cbr0",
errExpected: false,
},
{
ifacePrefix: "",
errExpected: true,
},
}
for i, c := range cases {
r, err := NewDetectLocalByInterfaceNamePrefix(c.ifacePrefix)
if c.errExpected {
if err == nil {
t.Errorf("Case[%d] expected error, but succeeded with: %q", i, r)
}
continue
}
if err != nil {
t.Errorf("Case[%d] failed with error: %v", i, err)
}
}
}
func TestDetectLocalByBridgeInterface(t *testing.T) {
cases := []struct {
ifaceName string
@@ -191,11 +87,7 @@ func TestDetectLocalByBridgeInterface(t *testing.T) {
},
}
for _, c := range cases {
localDetector, err := NewDetectLocalByBridgeInterface(c.ifaceName)
if err != nil {
t.Errorf("Error initializing localDetector: %v", err)
continue
}
localDetector := NewDetectLocalByBridgeInterface(c.ifaceName)
if !localDetector.IsImplemented() {
t.Error("DetectLocalByBridgeInterface returns false for IsImplemented")
}
@@ -228,11 +120,7 @@ func TestDetectLocalByInterfaceNamePrefix(t *testing.T) {
},
}
for _, c := range cases {
localDetector, err := NewDetectLocalByInterfaceNamePrefix(c.ifacePrefix)
if err != nil {
t.Errorf("Error initializing localDetector: %v", err)
continue
}
localDetector := NewDetectLocalByInterfaceNamePrefix(c.ifacePrefix)
if !localDetector.IsImplemented() {
t.Error("DetectLocalByInterfaceNamePrefix returns false for IsImplemented")
}