From f824df0e4188861bdb9c01ff12d547a65d1ac677 Mon Sep 17 00:00:00 2001 From: Edmund Rhudy <1065271+erhudy@users.noreply.github.com> Date: Mon, 4 Mar 2024 22:27:03 -0500 Subject: [PATCH] fix: add way to filter network interfaces Under certain circumstances, net.Interfaces() will begin returning interfaces in unusual orders (e.g. if the interface index goes above 255 or 256). This could cause the driver to select the wrong interface, which in turn would mean that it could not find a matching VM in vSphere and would crash. This changeset adds an argument to control what interfaces can be selected by the driver, by allowing certain interfaces to be excluded from consideration. --- main.go | 3 + service/envvars.go | 3 + service/node.go | 2 +- service/service.go | 29 ++++--- service/service_unit_test.go | 160 +++++++++++++++++++++++++++++++++++ service/vsphere.go | 25 ++++-- 6 files changed, 202 insertions(+), 20 deletions(-) diff --git a/main.go b/main.go index 8a95c9cf..8e88fac0 100644 --- a/main.go +++ b/main.go @@ -106,4 +106,7 @@ const usage = ` X_CSI_POWERMAX_ENDPOINT X_CSI_POWERMAX_DEBUG Turns on debugging of the PowerMax (REST interface to Unisphere) layer + + X_CSI_IFACE_EXCLUDE_FILTER + Regular expression of interface names to exclude. ` diff --git a/service/envvars.go b/service/envvars.go index 7d7725d6..2935d32a 100644 --- a/service/envvars.go +++ b/service/envvars.go @@ -153,4 +153,7 @@ const ( // EnvVCPassword is an env variable that has vCenter password EnvVCPassword = "X_CSI_VCENTER_PWD" // #nosec G101 + + // EnvIfaceExcludeFilter is an env variable with a regex of interface names to exclude + EnvIfaceExcludeFilter = "X_CSI_IFACE_EXCLUDE_FILTER" ) diff --git a/service/node.go b/service/node.go index 38c854ce..608fbbea 100644 --- a/service/node.go +++ b/service/node.go @@ -1358,7 +1358,7 @@ func (s *service) nodeStartup(ctx context.Context) error { // setVMHost create client for the vCenter func (s *service) setVMHost() error { // Create a VM host - host, err := NewVMHost(true, s.opts.VCenterHostURL, s.opts.VCenterHostUserName, s.opts.VCenterHostPassword) + host, err := NewVMHost(true, s.opts.VCenterHostURL, s.opts.VCenterHostUserName, s.opts.VCenterHostPassword, s.opts.IfaceExcludeFilter) if err != nil { log.Errorf("can not create VM host object: (%s)", err.Error()) return fmt.Errorf("Can not create VM host object: (%s)", err.Error()) diff --git a/service/service.go b/service/service.go index 5f47d5f6..9f335a91 100644 --- a/service/service.go +++ b/service/service.go @@ -20,6 +20,7 @@ import ( "math/rand" "net" "os" + "regexp" "strconv" "strings" "sync" @@ -117,18 +118,19 @@ type Opts struct { NonDefaultRetries bool // Indicates if non-default retry values to be used for deletion worker, only for unit testing NodeNameTemplate string ModifyHostName bool - ReplicationContextPrefix string // Enables sidecars to read required information from volume context - ReplicationPrefix string // Used as a prefix to find out if replication is enabled - IsHealthMonitorEnabled bool // used to check if health monitor for volume is enabled - IsTopologyControlEnabled bool // used to filter topology keys based on user config - IsVsphereEnabled bool // used to check if vSphere is enabled - VSpherePortGroup string // port group for vsphere - VSphereHostName string // host (initiator group) for vsphere - VCenterHostURL string // vCenter host url - VCenterHostUserName string // vCenter host username - VCenterHostPassword string // vCenter password - MaxVolumesPerNode int64 // to specify volume limits - KubeConfigPath string // to specify k8s configuration to be used CSI driver + ReplicationContextPrefix string // Enables sidecars to read required information from volume context + ReplicationPrefix string // Used as a prefix to find out if replication is enabled + IsHealthMonitorEnabled bool // used to check if health monitor for volume is enabled + IsTopologyControlEnabled bool // used to filter topology keys based on user config + IsVsphereEnabled bool // used to check if vSphere is enabled + VSpherePortGroup string // port group for vsphere + VSphereHostName string // host (initiator group) for vsphere + VCenterHostURL string // vCenter host url + VCenterHostUserName string // vCenter host username + VCenterHostPassword string // vCenter password + MaxVolumesPerNode int64 // to specify volume limits + KubeConfigPath string // to specify k8s configuration to be used CSI driver + IfaceExcludeFilter *regexp.Regexp // regex of interface names to exclude from consideration } // NodeConfig defines rules for given node @@ -393,6 +395,9 @@ func (s *service) BeforeServe( if replicationPrefix, ok := csictx.LookupEnv(ctx, EnvReplicationPrefix); ok { opts.ReplicationPrefix = replicationPrefix } + if ifaceExcludeFilter, ok := csictx.LookupEnv(ctx, EnvIfaceExcludeFilter); ok { + opts.IfaceExcludeFilter = regexp.MustCompile(ifaceExcludeFilter) + } if MaxVolumesPerNode, ok := csictx.LookupEnv(ctx, EnvMaxVolumesPerNode); ok { val, err := strconv.ParseInt(MaxVolumesPerNode, 10, 64) diff --git a/service/service_unit_test.go b/service/service_unit_test.go index 032bc8cd..f96cdc54 100644 --- a/service/service_unit_test.go +++ b/service/service_unit_test.go @@ -17,9 +17,12 @@ package service import ( "context" + "errors" "fmt" "math/rand" + "net" "os" + "regexp" "strconv" "strings" "sync" @@ -526,3 +529,160 @@ func TestEsnureISCSIDaemonIsStarted(t *testing.T) { errMsg = fmt.Sprintf("mock - unit is masked - failed to start the unit") assert.PanicsWithError(t, errMsg, func() { s.ensureISCSIDaemonStarted() }) } + +func TestGetLocalMAC(t *testing.T) { + orderedIfs := []net.Interface{ + { + Index: 1, + MTU: 65536, + Name: "lo", + HardwareAddr: nil, + Flags: 0x25, + }, + { + Index: 2, + MTU: 1500, + Name: "ens192", + HardwareAddr: net.HardwareAddr{0x0, 0x1, 0x2, 0x3, 0x4, 0x5}, + Flags: 0x33, + }, + { + Index: 5, + MTU: 1450, + Name: "vxlan.calico", + HardwareAddr: net.HardwareAddr{0x6, 0x7, 0x8, 0x9, 0xa, 0xb}, + Flags: 0x33, + }, + { + Index: 8, + MTU: 1450, + Name: "cali1d87cc7ab3f", + HardwareAddr: net.HardwareAddr{0xee, 0xee, 0xee, 0xee, 0xee, 0xee}, + Flags: 0x33, + }, + { + Index: 13, + MTU: 1450, + Name: "cali06df89a6f82", + HardwareAddr: net.HardwareAddr{0xee, 0xee, 0xee, 0xee, 0xee, 0xee}, + Flags: 0x33, + }, + } + unorderedIfs := []net.Interface{ + { + Index: 257, + MTU: 1450, + Name: "cali24aa7dc293b", + HardwareAddr: net.HardwareAddr{0xee, 0xee, 0xee, 0xee, 0xee, 0xee}, + Flags: 0x33, + }, + { + Index: 1, + MTU: 65536, + Name: "lo", + HardwareAddr: nil, + Flags: 0x25, + }, + { + Index: 2, + MTU: 1500, + Name: "ens192", + HardwareAddr: net.HardwareAddr{0x10, 0x11, 0x12, 0x13, 0x14, 0x15}, + Flags: 0x33, + }, + { + Index: 259, + MTU: 1450, + Name: "cali1d87cc7ab3f", + HardwareAddr: net.HardwareAddr{0xee, 0xee, 0xee, 0xee, 0xee, 0xee}, + Flags: 0x33, + }, + { + Index: 7, + MTU: 1450, + Name: "vxlan.calico", + HardwareAddr: net.HardwareAddr{0x16, 0x17, 0x18, 0x19, 0x1a, 0x1b}, + Flags: 0x33, + }, + { + Index: 11, + MTU: 1450, + Name: "calibbe3ea81e70", + HardwareAddr: net.HardwareAddr{0xee, 0xee, 0xee, 0xee, 0xee, 0xee}, + Flags: 0x33, + }, + } + onlyLoIfs := []net.Interface{ + { + Index: 1, + MTU: 65536, + Name: "lo", + HardwareAddr: nil, + Flags: 0x25, + }, + } + + tests := []struct { + expected string + expectErr bool + ifaceFunc func() ([]net.Interface, error) + ifaceExcludeFilter *regexp.Regexp + testName string + }{ + { + testName: "basic ordered IFs test", + expectErr: false, + expected: "00:01:02:03:04:05", + ifaceFunc: func() ([]net.Interface, error) { + return orderedIfs, nil + }, + ifaceExcludeFilter: nil, + }, + { + testName: "basic unordered IFs test", + expectErr: false, + expected: "ee:ee:ee:ee:ee:ee", + ifaceFunc: func() ([]net.Interface, error) { + return unorderedIfs, nil + }, + ifaceExcludeFilter: nil, + }, + { + testName: "expected error from only lo", + expectErr: true, + ifaceFunc: func() ([]net.Interface, error) { + return onlyLoIfs, nil + }, + ifaceExcludeFilter: nil, + }, + { + testName: "expected error from error in ifaceFunc", + expectErr: true, + ifaceFunc: func() ([]net.Interface, error) { + return []net.Interface{}, errors.New("kaboom") + }, + ifaceExcludeFilter: nil, + }, + { + testName: "exclude cali interfaces", + expectErr: false, + expected: "10:11:12:13:14:15", + ifaceFunc: func() ([]net.Interface, error) { + return unorderedIfs, nil + }, + ifaceExcludeFilter: regexp.MustCompile("^cali.+"), + }, + } + + for _, tt := range tests { + t.Run(tt.testName, func(t *testing.T) { + returnedIf, err := getLocalMAC(tt.ifaceFunc, tt.ifaceExcludeFilter) + if tt.expectErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + assert.Equal(t, tt.expected, returnedIf) + } + }) + } +} diff --git a/service/vsphere.go b/service/vsphere.go index ba7f7a07..dcaa6ab4 100644 --- a/service/vsphere.go +++ b/service/vsphere.go @@ -23,6 +23,7 @@ import ( "net" "net/url" "reflect" + "regexp" "strings" log "github.com/sirupsen/logrus" @@ -48,7 +49,7 @@ type VMHost struct { // NewVMHost connects to a ESXi or vCenter instance and returns a *VMHost // This method is referenced from https://github.com/codedellemc/govmax/blob/master/api/v1/vmomi.go -func NewVMHost(insecure bool, hostURLparam, user, pass string) (*VMHost, error) { +func NewVMHost(insecure bool, hostURLparam, user, pass string, ifaceExcludeFilter *regexp.Regexp) (*VMHost, error) { ctx, _ := context.WithCancel(context.Background()) hostURL, err := url.Parse("https://" + hostURLparam + "/sdk") hostURL.User = url.UserPassword(user, pass) @@ -58,7 +59,7 @@ func NewVMHost(insecure bool, hostURLparam, user, pass string) (*VMHost, error) return nil, err } - mac, err := getLocalMAC() + mac, err := getLocalMAC(net.Interfaces, ifaceExcludeFilter) if err != nil { return nil, err } @@ -90,17 +91,27 @@ func (vmh *VMHost) getMACAddressOfVM(vm *object.VirtualMachine) (string, error) return vmDeviceList.PrimaryMacAddress(), nil } -func getLocalMAC() (string, error) { - ifs, err := net.Interfaces() +func getLocalMAC(ifaceListFunc func() ([]net.Interface, error), ifaceExcludeFilter *regexp.Regexp) (string, error) { + ifs, err := ifaceListFunc() if err != nil { return "", err } for _, v := range ifs { - if v.HardwareAddr.String() != "" { - return v.HardwareAddr.String(), nil + if ifaceExcludeFilter != nil { + if ifaceExcludeFilter.MatchString(v.Name) { + continue + } else { + if v.HardwareAddr.String() != "" { + return v.HardwareAddr.String(), nil + } + } + } else { + if v.HardwareAddr.String() != "" { + return v.HardwareAddr.String(), nil + } } } - return "", errors.New("No network interface found") + return "", errors.New("no network interface found") } ///////////////////////////////////////////////////////////////////