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 arguments to control what interfaces can
be selected by the driver.
  • Loading branch information
@erhudy
erhudy committed Mar 5, 2024
1 parent 4336741 commit a5ce319
Show file tree
Hide file tree
Showing 6 changed files with 253 additions and 20 deletions.
6 changes: 6 additions & 0 deletions main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,4 +106,10 @@ const usage = ` X_CSI_POWERMAX_ENDPOINT
X_CSI_POWERMAX_DEBUG
Turns on debugging of the PowerMax (REST interface to Unisphere) layer
X_CSI_IFACE_INCLUDE_FILTER
Regular expression of interface names to include.
X_CSI_IFACE_EXCLUDE_FILTER
Regular expression of interface names to exclude.
`
6 changes: 6 additions & 0 deletions service/envvars.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,4 +153,10 @@ const (

// EnvVCPassword is an env variable that has vCenter password
EnvVCPassword = "X_CSI_VCENTER_PWD" // #nosec G101

// EnvIfaceIncludeFilter is an env variable with a regex of interface names to include
EnvIfaceIncludeFilter = "X_CSI_IFACE_INCLUDE_FILTER"

// 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
View file
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.IfaceIncludeFilter, 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
34 changes: 22 additions & 12 deletions service/service.go
View file
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,21 @@ 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
IfaceIncludeFilter *regexp.Regexp // regex of interface names to keep in
IfaceExcludeFilter *regexp.Regexp // regex of interface names to keep in

}

// NodeConfig defines rules for given node
Expand Down Expand Up @@ -393,6 +397,12 @@ func (s *service) BeforeServe(
if replicationPrefix, ok := csictx.LookupEnv(ctx, EnvReplicationPrefix); ok {
opts.ReplicationPrefix = replicationPrefix
}
if ifaceIncludeFilter, ok := csictx.LookupEnv(ctx, EnvIfaceIncludeFilter); ok {
opts.IfaceIncludeFilter = regexp.MustCompile(ifaceIncludeFilter)
}
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
195 changes: 195 additions & 0 deletions service/service_unit_test.go
View file
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,195 @@ 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)
ifaceIncludeFilter *regexp.Regexp
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
},
ifaceIncludeFilter: 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
},
ifaceIncludeFilter: nil,
ifaceExcludeFilter: nil,
},
{
testName: "expected error from only lo",
expectErr: true,
ifaceFunc: func() ([]net.Interface, error) {
return onlyLoIfs, nil
},
ifaceIncludeFilter: nil,
ifaceExcludeFilter: nil,
},
{
testName: "expected error from error in ifaceFunc",
expectErr: true,
ifaceFunc: func() ([]net.Interface, error) {
return []net.Interface{}, errors.New("kaboom")
},
ifaceIncludeFilter: nil,
ifaceExcludeFilter: nil,
},
{
testName: "filter down to just ens192",
expectErr: false,
expected: "10:11:12:13:14:15",
ifaceFunc: func() ([]net.Interface, error) {
return unorderedIfs, nil
},
ifaceIncludeFilter: regexp.MustCompile("^ens192$"),
ifaceExcludeFilter: nil,
},
{
testName: "exclude cali interfaces",
expectErr: false,
expected: "10:11:12:13:14:15",
ifaceFunc: func() ([]net.Interface, error) {
return unorderedIfs, nil
},
ifaceIncludeFilter: nil,
ifaceExcludeFilter: regexp.MustCompile("^cali.+"),
},
{
testName: "combine exclude and include",
expectErr: false,
expected: "16:17:18:19:1a:1b",
ifaceFunc: func() ([]net.Interface, error) {
return unorderedIfs, nil
},
ifaceIncludeFilter: regexp.MustCompile("^wontmatch$"),
ifaceExcludeFilter: regexp.MustCompile("^cali.+|ens192"),
},
{
testName: "unmatchable filters",
expectErr: true,
ifaceFunc: func() ([]net.Interface, error) {
return unorderedIfs, nil
},
ifaceIncludeFilter: regexp.MustCompile("^wontmatch$"),
ifaceExcludeFilter: regexp.MustCompile("^cali.+|ens192|^vxlan.calico$"),
},
}

for _, tt := range tests {
t.Run(tt.testName, func(t *testing.T) {
returnedIf, err := getLocalMAC(tt.ifaceFunc, tt.ifaceIncludeFilter, tt.ifaceExcludeFilter)
if tt.expectErr {
assert.Error(t, err)
} else {
assert.NoError(t, err)
assert.Equal(t, tt.expected, returnedIf)
}
})
}
}
30 changes: 23 additions & 7 deletions service/vsphere.go
View file
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, ifaceIncludeFilter *regexp.Regexp, 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, ifaceIncludeFilter, ifaceExcludeFilter)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -90,17 +91,32 @@ 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), ifaceIncludeFilter *regexp.Regexp, 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 || ifaceIncludeFilter != nil {
if ifaceIncludeFilter != nil && ifaceIncludeFilter.MatchString(v.Name) {
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")
}

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

0 comments on commit a5ce319

Please sign in to comment.