#1945: Adding support for regular expression values for the Interface Exclude parameter

.gitignore

@@ -23,6 +23,7 @@
 
 # IDE files.
 /.idea/
+/.vscode/
 
 # Patterns to match files from older releases (prevents lots of
 # untracked files from showing up when switching from a maintenance branch.

config/config_params.go

@@ -33,11 +33,18 @@ import (
 )
 
 var (
-	IfaceListRegexp  = regexp.MustCompile(`^[a-zA-Z0-9_-]{1,15}(,[a-zA-Z0-9_-]{1,15})*$`)
-	AuthorityRegexp  = regexp.MustCompile(`^[^:/]+:\d+$`)
-	HostnameRegexp   = regexp.MustCompile(`^[a-zA-Z0-9_.-]+$`)
-	StringRegexp     = regexp.MustCompile(`^.*$`)
-	IfaceParamRegexp = regexp.MustCompile(`^[a-zA-Z0-9:._+-]{1,15}$`)
+	// RegexpIfaceElemRegexp matches an individual element in the overall interface list;
+	// assumes the value represents a regular expression and is marked by '/' at the start
+	// and end and cannot have spaces
+	RegexpIfaceElemRegexp = regexp.MustCompile(`^\/[^\s]+\/$`)
+	// NonRegexpIfaceElemRegexp matches an individual element in the overall interface list;
+	// assumes the value is between 1-15 chars long and only be alphanumeric or - or _
+	NonRegexpIfaceElemRegexp = regexp.MustCompile(`^[a-zA-Z0-9_-]{1,15}$`)
+	IfaceListRegexp          = regexp.MustCompile(`^[a-zA-Z0-9_-]{1,15}(,[a-zA-Z0-9_-]{1,15})*$`)
+	AuthorityRegexp          = regexp.MustCompile(`^[^:/]+:\d+$`)
+	HostnameRegexp           = regexp.MustCompile(`^[a-zA-Z0-9_.-]+$`)
+	StringRegexp             = regexp.MustCompile(`^.*$`)
+	IfaceParamRegexp         = regexp.MustCompile(`^[a-zA-Z0-9:._+-]{1,15}$`)
 )
 
 const (
@@ -143,7 +150,7 @@ type Config struct {
 	OpenstackRegion string `config:"region;;die-on-fail"`
 
 	InterfacePrefix  string `config:"iface-list;cali;non-zero,die-on-fail"`
-	InterfaceExclude string `config:"iface-list;kube-ipvs0"`
+	InterfaceExclude string `config:"iface-list-regexp;kube-ipvs0;die-on-fail"`
 
 	ChainInsertMode             string `config:"oneof(insert,append);insert;non-zero,die-on-fail"`
 	DefaultEndpointToHostAction string `config:"oneof(DROP,RETURN,ACCEPT);DROP;non-zero,die-on-fail"`
@@ -244,8 +251,30 @@ func (c *Config) InterfacePrefixes() []string {
 	return strings.Split(c.InterfacePrefix, ",")
 }
 
-func (c *Config) InterfaceExcludes() []string {
-	return strings.Split(c.InterfaceExclude, ",")
+// InterfaceExcludes returns a list of Regexp objects, each representing either an
+// exact interface value match or a class of interface matches. This function will
+// panic if any individual value cannot be parsed / compiled into a Regexp.
+func (config *Config) InterfaceExcludes() []*regexp.Regexp {
+	log.Debugf("Compiling regexp values for InterfaceExclude %s \n", config.InterfaceExclude)
+	strValues := strings.Split(config.InterfaceExclude, ",")
+	regexpValues := []*regexp.Regexp{}
+
+	for _, strValue := range strValues {
+		// Ignore empty values
+		if len(strValue) == 0 {
+			continue
+		}
+		// Any value not in regexp format will be converted into equivalent regexp
+		// that tests for exact match
+		if !RegexpIfaceElemRegexp.Match([]byte(strValue)) {
+			convertedValue := fmt.Sprintf("^%s$", regexp.QuoteMeta(strValue))
+			regexpValues = append(regexpValues, regexp.MustCompile(convertedValue))
+		} else {
+			parsedValue := strValue[1 : len(strValue)-1]
+			regexpValues = append(regexpValues, regexp.MustCompile(parsedValue))
+		}
+	}
+	return regexpValues
 }
 
 func (config *Config) OpenstackActive() bool {
@@ -510,6 +539,13 @@ func loadParams() {
 		case "iface-list":
 			param = &RegexpParam{Regexp: IfaceListRegexp,
 				Msg: "invalid Linux interface name"}
+		case "iface-list-regexp":
+			param = &RegexpPatternListParam{
+				NonRegexpElemRegexp: NonRegexpIfaceElemRegexp,
+				RegexpElemRegexp:    RegexpIfaceElemRegexp,
+				Delimiter:           ",",
+				Msg:                 "list contains invalid Linux interface name or regex pattern",
+			}
 		case "iface-param":
 			param = &RegexpParam{Regexp: IfaceParamRegexp,
 				Msg: "invalid Linux interface parameter"}

config/config_params_test.go

@@ -15,6 +15,8 @@
 package config_test
 
 import (
+	"regexp"
+
 	. "github.com/projectcalico/felix/config"
 
 	"net"
@@ -29,7 +31,7 @@ import (
 
 	log "github.com/sirupsen/logrus"
 
-	"github.com/projectcalico/libcalico-go/lib/apis/v3"
+	v3 "github.com/projectcalico/libcalico-go/lib/apis/v3"
 	"github.com/projectcalico/libcalico-go/lib/numorstring"
 )
 
@@ -158,8 +160,15 @@ var _ = DescribeTable("Config parsing",
 
 	Entry("InterfacePrefix", "InterfacePrefix", "tap", "tap"),
 	Entry("InterfacePrefix list", "InterfacePrefix", "tap,cali", "tap,cali"),
-	Entry("InterfaceExclude", "InterfaceExclude", "kube-ipvs0", "kube-ipvs0"),
-	Entry("InterfaceExclude list", "InterfaceExclude", "kube-ipvs0,dummy", "kube-ipvs0,dummy"),
+
+	Entry("InterfaceExclude one value no regexp", "InterfaceExclude", "kube-ipvs0", "kube-ipvs0"),
+	Entry("InterfaceExclude list no regexp", "InterfaceExclude", "kube-ipvs0,dummy", "kube-ipvs0,dummy"),
+	Entry("InterfaceExclude one value regexp", "InterfaceExclude", "/kube-ipvs/", "/kube-ipvs/"),
+	Entry("InterfaceExclude list regexp", "InterfaceExclude", "kube-ipvs0,dummy,/^veth*$/", "kube-ipvs0,dummy,/^veth*$/"),
+	Entry("InterfaceExclude no regexp", "InterfaceExclude", "/^kube.*/,/veth/", "/^kube.*/,/veth/"),
+	Entry("InterfaceExclude list regexp empty", "InterfaceExclude", "kube,//", "kube-ipvs0", true),               // empty regexp value
+	Entry("InterfaceExclude list regexp invalid comma", "InterfaceExclude", "/kube,/,dummy", "kube-ipvs0", true), // bad comma use
+	Entry("InterfaceExclude list regexp invalid symbol", "InterfaceExclude", `/^kube\K/`, "kube-ipvs0", true),    // Invalid regexp
 
 	Entry("ChainInsertMode append", "ChainInsertMode", "append", "append"),
 	Entry("ChainInsertMode append", "ChainInsertMode", "Append", "append"),
@@ -457,3 +466,33 @@ var _ = DescribeTable("Config validation",
 		"OpenstackRegion": "my-region-has-a-very-long-and-extremely-interesting-name",
 	}, false),
 )
+
+var _ = DescribeTable("Config InterfaceExcludes",
+	func(excludeList string, expected []*regexp.Regexp) {
+		cfg := New()
+		cfg.InterfaceExclude = excludeList
+		regexps := cfg.InterfaceExcludes()
+		Expect(regexps).To(Equal(expected))
+	},
+
+	Entry("empty exclude list", "", []*regexp.Regexp{}),
+	Entry("non-regexp single value", "kube-ipvs0", []*regexp.Regexp{
+		regexp.MustCompile("^kube-ipvs0$"),
+	}),
+	Entry("non-regexp multiple values", "kube-ipvs0,veth1", []*regexp.Regexp{
+		regexp.MustCompile("^kube-ipvs0$"),
+		regexp.MustCompile("^veth1$"),
+	}),
+	Entry("regexp single value", "/^veth.*/", []*regexp.Regexp{
+		regexp.MustCompile("^veth.*"),
+	}),
+	Entry("regexp multiple values", "/veth/,/^kube.*/", []*regexp.Regexp{
+		regexp.MustCompile("veth"),
+		regexp.MustCompile("^kube.*"),
+	}),
+	Entry("both non-regexp and regexp values", "kube-ipvs0,/veth/,/^kube.*/", []*regexp.Regexp{
+		regexp.MustCompile("^kube-ipvs0$"),
+		regexp.MustCompile("veth"),
+		regexp.MustCompile("^kube.*"),
+	}),
+)

config/param_types.go

@@ -174,6 +174,37 @@ func (p *RegexpParam) Parse(raw string) (result interface{}, err error) {
 	return
 }
 
+// RegexpPatternListParam differs from RegexpParam (above) in that it validates
+// string values that are (themselves) regular expressions.
+type RegexpPatternListParam struct {
+	Metadata
+	RegexpElemRegexp    *regexp.Regexp
+	NonRegexpElemRegexp *regexp.Regexp
+	Delimiter           string
+	Msg                 string
+}
+
+// Parse validates whether the given raw string contains a list of valid values.
+// Validation is dictated by two regexp patterns: one for valid regular expression
+// values, another for non-regular expressions.
+func (p *RegexpPatternListParam) Parse(raw string) (interface{}, error) {
+	// Split into individual elements and validate each one
+	tokens := strings.Split(raw, p.Delimiter)
+	for _, t := range tokens {
+		if p.RegexpElemRegexp.Match([]byte(t)) {
+			// Need to remove the start and end symbols that wrap the actual regexp
+			regexpValue := t[1 : len(t)-1]
+			_, compileErr := regexp.Compile(regexpValue)
+			if compileErr != nil {
+				return nil, p.parseFailed(raw, p.Msg)
+			}
+		} else if !p.NonRegexpElemRegexp.Match([]byte(t)) {
+			return nil, p.parseFailed(raw, p.Msg)
+		}
+	}
+	return raw, nil
+}
+
 type FileParam struct {
 	Metadata
 	MustExist  bool

dataplane/linux/int_dataplane_test.go

@@ -35,6 +35,7 @@ var _ = Describe("Constructor test", func() {
 
 	JustBeforeEach(func() {
 		configParams = config.New()
+		configParams.InterfaceExclude = "/^kube.*/,/veth/,eth2"
 		dpConfig = intdataplane.Config{
 			IfaceMonitorConfig: ifacemonitor.Config{
 				InterfaceExcludes: configParams.InterfaceExcludes(),

ifacemonitor/iface_monitor.go

@@ -15,6 +15,7 @@
 package ifacemonitor
 
 import (
+	"regexp"
 	"syscall"
 	"time"
 
@@ -45,7 +46,7 @@ type AddrStateCallback func(ifaceName string, addrs set.Set)
 
 type Config struct {
 	// List of interface names that dataplane receives no callbacks from them.
-	InterfaceExcludes []string
+	InterfaceExcludes []*regexp.Regexp
 }
 type InterfaceMonitor struct {
 	Config
@@ -136,12 +137,11 @@ readLoop:
 }
 
 func (m *InterfaceMonitor) isExcludedInterface(ifName string) bool {
-	for _, name := range m.InterfaceExcludes {
-		if ifName == name {
+	for _, nameExp := range m.InterfaceExcludes {
+		if nameExp.Match([]byte(ifName)) {
 			return true
 		}
 	}
-
 	return false
 }
 

ifacemonitor/ifacemonitor_test.go

@@ -15,6 +15,8 @@
 package ifacemonitor_test
 
 import (
+	"fmt"
+	"regexp"
 	"strings"
 	"sync"
 	"syscall"
@@ -347,7 +349,12 @@ var _ = Describe("ifacemonitor", func() {
 		}
 		resyncC = make(chan time.Time)
 		config := ifacemonitor.Config{
-			InterfaceExcludes: []string{"kube-ipvs0"},
+			// Test the regexp ability of interface excludes
+			InterfaceExcludes: []*regexp.Regexp{
+				regexp.MustCompile("^kube-ipvs.*"),
+				regexp.MustCompile("^veth1$"),
+				regexp.MustCompile("dummy"),
+			},
 		}
 		im = ifacemonitor.NewWithStubs(config, nl, resyncC)
 
@@ -373,36 +380,50 @@ var _ = Describe("ifacemonitor", func() {
 	})
 
 	It("should skip netlink address updates for ipvs", func() {
-		// Should not receives any address callbacks.
-		nl.addLink("kube-ipvs0")
-		resyncC <- time.Time{}
-		dp.notExpectAddrStateCb()
-		dp.notExpectLinkStateCb()
-		nl.addAddr("kube-ipvs0", "10.100.0.1/32")
-		dp.notExpectAddrStateCb()
-
-		nl.changeLinkState("kube-ipvs0", "up")
-		dp.expectLinkStateCb("kube-ipvs0", ifacemonitor.StateUp)
-		nl.changeLinkState("kube-ipvs0", "down")
-		dp.expectLinkStateCb("kube-ipvs0", ifacemonitor.StateDown)
-
-		// Should notify down from up on deletion.
-		nl.changeLinkState("kube-ipvs0", "up")
-		dp.expectLinkStateCb("kube-ipvs0", ifacemonitor.StateUp)
-		nl.delLink("kube-ipvs0")
-		dp.notExpectAddrStateCb()
-		dp.expectLinkStateCb("kube-ipvs0", ifacemonitor.StateDown)
-
-		// Check it can be added again.
-		nl.addLink("kube-ipvs0")
-		resyncC <- time.Time{}
-		dp.notExpectAddrStateCb()
-		dp.notExpectLinkStateCb()
+		var netlinkUpdates = func(iface string) {
+			// Should not receive any address callbacks.
+			nl.addLink(iface)
+			resyncC <- time.Time{}
+			dp.notExpectAddrStateCb()
+			dp.notExpectLinkStateCb()
+			nl.addAddr(iface, "10.100.0.1/32")
+			dp.notExpectAddrStateCb()
+
+			nl.changeLinkState(iface, "up")
+			dp.expectLinkStateCb(iface, ifacemonitor.StateUp)
+			nl.changeLinkState(iface, "down")
+			dp.expectLinkStateCb(iface, ifacemonitor.StateDown)
+
+			// Should notify down from up on deletion.
+			nl.changeLinkState(iface, "up")
+			dp.expectLinkStateCb(iface, ifacemonitor.StateUp)
+			nl.delLink(iface)
+			dp.notExpectAddrStateCb()
+			dp.expectLinkStateCb(iface, ifacemonitor.StateDown)
+
+			// Check it can be added again.
+			nl.addLink(iface)
+			resyncC <- time.Time{}
+			dp.notExpectAddrStateCb()
+			dp.notExpectLinkStateCb()
+
+			// Clean it.
+			nl.delLink(iface)
+			dp.notExpectAddrStateCb()
+			dp.notExpectLinkStateCb()
+		}
+
+		// Repeat for 3 different interfaces (to test regexp of interface excludes)
+		for index := 0; index < 3; index++ {
+			interfaceName := fmt.Sprintf("kube-ipvs%d", index)
+			netlinkUpdates(interfaceName)
+		}
+
+		// Repeat test for second interface exclude entry
+		netlinkUpdates("veth1")
 
-		// Clean it.
-		nl.delLink("kube-ipvs0")
-		dp.notExpectAddrStateCb()
-		dp.notExpectLinkStateCb()
+		// Repeat test for third interface exclude entry
+		netlinkUpdates("0dummy1")
 	})
 
 	It("should handle mainline netlink updates", func() {