nfd-worker: add core.labelWhiteList config option

Add a config file option for label whitelisting. Deprecate the
--label-whitelist command line flag. Note that the command line flag has
higher priority than the config file option.

cmd/nfd-worker/main.go

@@ -19,6 +19,7 @@ package main
 import (
 	"fmt"
 	"log"
+	"regexp"
 	"strings"
 	"time"
 
@@ -99,7 +100,8 @@ func argsParse(argv []string) (worker.Args, error) {
                               publish to the Kubernetes API server.
                               NB: the label namespace is omitted i.e. the filter
                               is only applied to the name part after '/'.
-                              [Default: ]
+                              (DEPRECATED: This parameter should be set via the
+                              config file)
   --oneshot                   Label once and exit.
   --sleep-interval=<seconds>  Time to sleep between re-labeling. Non-positive
                               value implies no re-labeling (i.e. infinite
@@ -124,10 +126,18 @@ func argsParse(argv []string) (worker.Args, error) {
 	args.Server = arguments["--server"].(string)
 	args.ServerNameOverride = arguments["--server-name-override"].(string)
 	args.Sources = strings.Split(arguments["--sources"].(string), ",")
-	args.LabelWhiteList = arguments["--label-whitelist"].(string)
 	args.Oneshot = arguments["--oneshot"].(bool)
 
 	// Parse deprecated/override args
+	if v := arguments["--label-whitelist"]; v != nil {
+		s := v.(string)
+		// Compile labelWhiteList regex
+		if r, err := regexp.Compile(s); err != nil {
+			return args, fmt.Errorf("error parsing --label-whitelist regex (%s): %v", s, err)
+		} else {
+			args.LabelWhiteList = r
+		}
+	}
 	if arguments["--no-publish"].(bool) {
 		b := true
 		args.NoPublish = &b

cmd/nfd-worker/main_test.go

@@ -35,7 +35,7 @@ func TestArgsParse(t *testing.T) {
 				So(*args.NoPublish, ShouldBeTrue)
 				So(args.Oneshot, ShouldBeTrue)
 				So(args.Sources, ShouldResemble, allSources)
-				So(len(args.LabelWhiteList), ShouldEqual, 0)
+				So(args.LabelWhiteList, ShouldBeNil)
 				So(err, ShouldBeNil)
 			})
 		})
@@ -48,7 +48,7 @@ func TestArgsParse(t *testing.T) {
 				So(args.NoPublish, ShouldBeNil)
 				So(args.Oneshot, ShouldBeFalse)
 				So(args.Sources, ShouldResemble, []string{"fake1", "fake2", "fake3"})
-				So(len(args.LabelWhiteList), ShouldEqual, 0)
+				So(args.LabelWhiteList, ShouldBeNil)
 				So(err, ShouldBeNil)
 			})
 		})
@@ -59,7 +59,7 @@ func TestArgsParse(t *testing.T) {
 			Convey("args.labelWhiteList is set to appropriate value and args.sources is set to default value", func() {
 				So(args.NoPublish, ShouldBeNil)
 				So(args.Sources, ShouldResemble, allSources)
-				So(args.LabelWhiteList, ShouldResemble, ".*rdt.*")
+				So(args.LabelWhiteList.String(), ShouldResemble, ".*rdt.*")
 				So(err, ShouldBeNil)
 			})
 		})
@@ -73,7 +73,7 @@ func TestArgsParse(t *testing.T) {
 				So(args.CertFile, ShouldEqual, "crt")
 				So(args.KeyFile, ShouldEqual, "key")
 				So(args.Sources, ShouldResemble, []string{"fake1", "fake2", "fake3"})
-				So(len(args.LabelWhiteList), ShouldEqual, 0)
+				So(args.LabelWhiteList, ShouldBeNil)
 				So(err, ShouldBeNil)
 			})
 		})

docs/advanced/worker-commandline-reference.md

@@ -172,6 +172,9 @@ expression in order to be published.
 Note: The regular expression is only matches against the "basename" part of the
 label, i.e. to the part of the name after '/'. The label namespace is omitted.
 
+Note: This flag takes precedence over the `core.labelWhiteList` configuration
+file option.
+
 Default: *empty*
 
 Example:
@@ -180,6 +183,9 @@ Example:
 nfd-worker --label-whitelist='.*cpuid\.'
 ```
 
+**DEPRECATED**: you should use the `core.labelWhiteList` option in the
+configuration file, instead.
+
 ### --oneshot
 
 The `--oneshot` flag causes nfd-worker to exit after one pass of feature

nfd-daemonset-combined.yaml.template

@@ -143,6 +143,7 @@ metadata:
 data:
   nfd-worker.conf: | ### <NFD-WORKER-CONF-START-DO-NOT-REMOVE>
     #core:
+    #  labelWhiteList:
     #  noPublish: false
     #  sleepInterval: 60s
     #sources:

nfd-worker-daemonset.yaml.template

@@ -103,6 +103,7 @@ metadata:
 data:
   nfd-worker.conf: | ### <NFD-WORKER-CONF-START-DO-NOT-REMOVE>
     #core:
+    #  labelWhiteList:
     #  noPublish: false
     #  sleepInterval: 60s
     #sources:

nfd-worker-job.yaml.template

@@ -113,6 +113,7 @@ metadata:
 data:
   nfd-worker.conf: | ### <NFD-WORKER-CONF-START-DO-NOT-REMOVE>
     #core:
+    #  labelWhiteList:
     #  noPublish: false
     #  sleepInterval: 60s
     #sources:

nfd-worker.conf.example

@@ -1,4 +1,5 @@
 #core:
+#  labelWhiteList:
 #  noPublish: false
 #  sleepInterval: 60s
 #sources:

pkg/nfd-worker/nfd-worker-internal_test.go

@@ -49,7 +49,7 @@ func TestDiscoveryWithMockSources(t *testing.T) {
 
 		fakeFeatureSource := source.FeatureSource(mockFeatureSource)
 
-		labelWhiteList := regexp.MustCompile("^test")
+		labelWhiteList := regex{*regexp.MustCompile("^test")}
 
 		Convey("When I successfully get the labels from the mock source", func() {
 			mockFeatureSource.On("Name").Return(fakeFeatureSourceName)
@@ -161,9 +161,10 @@ func TestConfigParse(t *testing.T) {
 func TestNewNfdWorker(t *testing.T) {
 	Convey("When creating new NfdWorker instance", t, func() {
 
+		emptyRegexp := regex{*regexp.MustCompile("")}
+
 		Convey("without any args specified", func() {
 			args := Args{}
-			emptyRegexp, _ := regexp.Compile("")
 			w, err := NewNfdWorker(args)
 			Convey("no error should be returned", func() {
 				So(err, ShouldBeNil)
@@ -172,13 +173,12 @@ func TestNewNfdWorker(t *testing.T) {
 			worker.configure("", "")
 			Convey("no sources should be enabled and the whitelist regexp should be empty", func() {
 				So(len(worker.sources), ShouldEqual, 0)
-				So(worker.labelWhiteList, ShouldResemble, emptyRegexp)
+				So(worker.config.Core.LabelWhiteList, ShouldResemble, emptyRegexp)
 			})
 		})
 
 		Convey("with non-empty Sources arg specified", func() {
 			args := Args{Sources: []string{"fake"}}
-			emptyRegexp, _ := regexp.Compile("")
 			w, err := NewNfdWorker(args)
 			Convey("no error should be returned", func() {
 				So(err, ShouldBeNil)
@@ -188,34 +188,22 @@ func TestNewNfdWorker(t *testing.T) {
 			Convey("proper sources should be enabled", func() {
 				So(len(worker.sources), ShouldEqual, 1)
 				So(worker.sources[0], ShouldHaveSameTypeAs, &fake.Source{})
-				So(worker.labelWhiteList, ShouldResemble, emptyRegexp)
-			})
-		})
-
-		Convey("with invalid LabelWhiteList arg specified", func() {
-			args := Args{LabelWhiteList: "*"}
-			w, err := NewNfdWorker(args)
-			worker := w.(*nfdWorker)
-			worker.configure("", "")
-			Convey("an error should be returned", func() {
-				So(len(worker.sources), ShouldEqual, 0)
-				So(worker.labelWhiteList, ShouldBeNil)
-				So(err, ShouldNotBeNil)
+				So(worker.config.Core.LabelWhiteList, ShouldResemble, emptyRegexp)
 			})
 		})
 
 		Convey("with valid LabelWhiteListStr arg specified", func() {
-			args := Args{LabelWhiteList: ".*rdt.*"}
+			args := Args{LabelWhiteList: regexp.MustCompile(".*rdt.*")}
 			w, err := NewNfdWorker(args)
 			Convey("no error should be returned", func() {
 				So(err, ShouldBeNil)
 			})
 			worker := w.(*nfdWorker)
 			worker.configure("", "")
-			expectRegexp := regexp.MustCompile(".*rdt.*")
+			expectRegexp := regex{*regexp.MustCompile(".*rdt.*")}
 			Convey("proper labelWhiteList regexp should be produced", func() {
 				So(len(worker.sources), ShouldEqual, 0)
-				So(worker.labelWhiteList, ShouldResemble, expectRegexp)
+				So(worker.config.Core.LabelWhiteList, ShouldResemble, expectRegexp)
 			})
 		})
 	})
@@ -224,7 +212,7 @@ func TestNewNfdWorker(t *testing.T) {
 func TestCreateFeatureLabels(t *testing.T) {
 	Convey("When creating feature labels from the configured sources", t, func() {
 		Convey("When fake feature source is configured", func() {
-			emptyLabelWL, _ := regexp.Compile("")
+			emptyLabelWL := regex{*regexp.MustCompile("")}
 			fakeFeatureSource := source.FeatureSource(new(fake.Source))
 			sources := []source.FeatureSource{}
 			sources = append(sources, fakeFeatureSource)
@@ -238,11 +226,11 @@ func TestCreateFeatureLabels(t *testing.T) {
 			})
 		})
 		Convey("When fake feature source is configured with a whitelist that doesn't match", func() {
-			emptyLabelWL, _ := regexp.Compile(".*rdt.*")
+			labelWL := regex{*regexp.MustCompile(".*rdt.*")}
 			fakeFeatureSource := source.FeatureSource(new(fake.Source))
 			sources := []source.FeatureSource{}
 			sources = append(sources, fakeFeatureSource)
-			labels := createFeatureLabels(sources, emptyLabelWL)
+			labels := createFeatureLabels(sources, labelWL)
 
 			Convey("fake labels are not returned", func() {
 				So(len(labels), ShouldEqual, 0)
@@ -258,7 +246,7 @@ func TestGetFeatureLabels(t *testing.T) {
 	Convey("When I get feature labels and panic occurs during discovery of a feature source", t, func() {
 		fakePanicFeatureSource := source.FeatureSource(new(panicfake.Source))
 
-		returnedLabels, err := getFeatureLabels(fakePanicFeatureSource, regexp.MustCompile(""))
+		returnedLabels, err := getFeatureLabels(fakePanicFeatureSource, regex{*regexp.MustCompile("")})
 		Convey("No label is returned", func() {
 			So(len(returnedLabels), ShouldEqual, 0)
 		})

pkg/nfd-worker/nfd-worker.go

@@ -66,6 +66,7 @@ type NFDConfig struct {
 }
 
 type coreConfig struct {
+	LabelWhiteList regex
 	NoPublish      bool
 	SleepInterval  duration
 }
@@ -77,7 +78,6 @@ type Labels map[string]string
 
 // Command line arguments
 type Args struct {
-	LabelWhiteList     string
 	CaFile             string
 	CertFile           string
 	KeyFile            string
@@ -88,6 +88,7 @@ type Args struct {
 	ServerNameOverride string
 	Sources            []string
 	// Deprecated options that should be set via the config file
+	LabelWhiteList *regexp.Regexp
 	NoPublish      *bool
 	SleepInterval  *time.Duration
 }
@@ -103,7 +104,10 @@ type nfdWorker struct {
 	configFilePath string
 	config         *NFDConfig
 	sources        []source.FeatureSource
-	labelWhiteList *regexp.Regexp
+}
+
+type regex struct {
+	regexp.Regexp
 }
 
 type duration struct {
@@ -182,13 +186,6 @@ func NewNfdWorker(args Args) (NfdWorker, error) {
 		}
 	}
 
-	// Compile labelWhiteList regex
-	var err error
-	nfd.labelWhiteList, err = regexp.Compile(args.LabelWhiteList)
-	if err != nil {
-		return nfd, fmt.Errorf("error parsing label whitelist regex (%s): %s", args.LabelWhiteList, err)
-	}
-
 	return nfd, nil
 }
 
@@ -229,6 +226,7 @@ func addConfigWatch(path string) (*fsnotify.Watcher, map[string]struct{}, error)
 func newDefaultConfig() *NFDConfig {
 	return &NFDConfig{
 		Core: coreConfig{
+			LabelWhiteList: regex{*regexp.MustCompile("")},
 			SleepInterval:  duration{60 * time.Second},
 		},
 	}
@@ -260,7 +258,7 @@ func (w *nfdWorker) Run() error {
 		select {
 		case <-labelTrigger:
 			// Get the set of feature labels.
-			labels := createFeatureLabels(w.sources, w.labelWhiteList)
+			labels := createFeatureLabels(w.sources, w.config.Core.LabelWhiteList)
 
 			// Update the node with the feature labels.
 			if w.client != nil {
@@ -416,6 +414,9 @@ func (w *nfdWorker) configure(filepath string, overrides string) {
 		stderrLogger.Printf("Failed to parse --options: %s", err)
 	}
 
+	if w.args.LabelWhiteList != nil {
+		c.Core.LabelWhiteList = regex{*w.args.LabelWhiteList}
+	}
 	if w.args.NoPublish != nil {
 		c.Core.NoPublish = *w.args.NoPublish
 	}
@@ -435,7 +436,7 @@ func (w *nfdWorker) configure(filepath string, overrides string) {
 
 // createFeatureLabels returns the set of feature labels from the enabled
 // sources and the whitelist argument.
-func createFeatureLabels(sources []source.FeatureSource, labelWhiteList *regexp.Regexp) (labels Labels) {
+func createFeatureLabels(sources []source.FeatureSource, labelWhiteList regex) (labels Labels) {
 	labels = Labels{}
 
 	// Do feature discovery from all configured sources.
@@ -458,7 +459,7 @@ func createFeatureLabels(sources []source.FeatureSource, labelWhiteList *regexp.
 
 // getFeatureLabels returns node labels for features discovered by the
 // supplied source.
-func getFeatureLabels(source source.FeatureSource, labelWhiteList *regexp.Regexp) (labels Labels, err error) {
+func getFeatureLabels(source source.FeatureSource, labelWhiteList regex) (labels Labels, err error) {
 	defer func() {
 		if r := recover(); r != nil {
 			stderrLogger.Printf("panic occurred during discovery of source [%s]: %v", source.Name(), r)
@@ -542,6 +543,25 @@ func advertiseFeatureLabels(client pb.LabelerClient, labels Labels) error {
 	return nil
 }
 
+// UnmarshalJSON implements the Unmarshaler interface from "encoding/json"
+func (r *regex) UnmarshalJSON(data []byte) error {
+	var v interface{}
+	if err := json.Unmarshal(data, &v); err != nil {
+		return err
+	}
+	switch val := v.(type) {
+	case string:
+		if rr, err := regexp.Compile(string(val)); err != nil {
+			return err
+		} else {
+			*r = regex{*rr}
+		}
+	default:
+		return fmt.Errorf("invalid regexp %s", data)
+	}
+	return nil
+}
+
 // UnmarshalJSON implements the Unmarshaler interface from "encoding/json"
 func (d *duration) UnmarshalJSON(data []byte) error {
 	var v interface{}