Skip to content

Commit

Permalink
fix: add way to filter network interfaces
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
erhudy committed May 6, 2024
1 parent 129e4f9 commit f824df0
Show file tree
Hide file tree
Showing 6 changed files with 202 additions and 20 deletions.
3 changes: 3 additions & 0 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
`
3 changes: 3 additions & 0 deletions service/envvars.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
2 changes: 1 addition & 1 deletion service/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
29 changes: 17 additions & 12 deletions service/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"math/rand"
"net"
"os"
"regexp"
"strconv"
"strings"
"sync"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
160 changes: 160 additions & 0 deletions service/service_unit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,12 @@ package service

import (
"context"
"errors"
"fmt"
"math/rand"
"net"
"os"
"regexp"
"strconv"
"strings"
"sync"
Expand Down Expand Up @@ -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)
}
})
}
}
25 changes: 18 additions & 7 deletions service/vsphere.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"net"
"net/url"
"reflect"
"regexp"
"strings"

log "github.com/sirupsen/logrus"
Expand All @@ -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)
Expand All @@ -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
}
Expand Down Expand Up @@ -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 {

Check warning on line 103 in service/vsphere.go

View workflow job for this annotation

GitHub Actions / golangci-lint

superfluous-else: if block ends with a continue statement, so drop this else and outdent its block (revive)
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")
}

///////////////////////////////////////////////////////////////////
Expand Down

0 comments on commit f824df0

Please sign in to comment.