Repository: kubernetes-sigs/node-feature-discovery
Pull request #1657 (2024-04-05 11:22:53)

nfd-master: prevent crash on empty config struct
Change the handling of LabelWhiteList config option to use a pointer to detect when the option is unset. This doesn't fix any detected crash but is merely general improvement and stabilization, serving easier testing.

Also, use the regexp type from the core libs for the config struct - dropping the unmasrhalling code for our custom regexp type - as the core regexp now implements unmarshaller itself.


Patch:
diff --git a/pkg/nfd-master/nfd-master.go b/pkg/nfd-master/nfd-master.go
index e48517f39..abf1894d5 100644
--- a/pkg/nfd-master/nfd-master.go
+++ b/pkg/nfd-master/nfd-master.go
@@ -76,7 +76,7 @@ type NFDConfig struct {
 	AutoDefaultNs     bool
 	DenyLabelNs       utils.StringSetVal
 	ExtraLabelNs      utils.StringSetVal
-	LabelWhiteList    utils.RegexpVal
+	LabelWhiteList    *regexp.Regexp
 	NoPublish         bool
 	ResourceLabels    utils.StringSetVal
 	EnableTaints      bool
@@ -197,7 +197,6 @@ func NewNfdMaster(args *Args) (NfdMaster, error) {
 
 func newDefaultConfig() *NFDConfig {
 	return &NFDConfig{
-		LabelWhiteList:    utils.RegexpVal{Regexp: *regexp.MustCompile("")},
 		DenyLabelNs:       utils.StringSetVal{},
 		ExtraLabelNs:      utils.StringSetVal{},
 		NoPublish:         false,
@@ -442,7 +441,7 @@ func (m *nfdMaster) nfdAPIUpdateHandler() {
 				}
 			} else {
 				for nodeName := range updateNodes {
-					m.nodeUpdaterPool.addNode(nodeName)
+					m.nodeUpdaterPool.queue.Add(nodeName)
 				}
 			}
 
@@ -608,8 +607,8 @@ func (m *nfdMaster) filterFeatureLabel(name, value string, features *nfdv1alpha1
 	}
 
 	// Skip if label doesn't match labelWhiteList
-	if !m.config.LabelWhiteList.Regexp.MatchString(base) {
-		return "", fmt.Errorf("%s (%s) does not match the whitelist (%s)", base, name, m.config.LabelWhiteList.Regexp.String())
+	if m.config.LabelWhiteList != nil && !m.config.LabelWhiteList.MatchString(base) {
+		return "", fmt.Errorf("%s (%s) does not match the whitelist (%s)", base, name, m.config.LabelWhiteList.String())
 	}
 
 	return filteredValue, nil
@@ -715,7 +714,7 @@ func (m *nfdMaster) nfdAPIUpdateAllNodes() error {
 	}
 
 	for _, node := range nodes.Items {
-		m.nodeUpdaterPool.addNode(node.Name)
+		m.nodeUpdaterPool.queue.Add(node.Name)
 	}
 
 	return nil
@@ -1210,7 +1209,7 @@ func (m *nfdMaster) configure(filepath string, overrides string) error {
 		c.EnableTaints = *m.args.Overrides.EnableTaints
 	}
 	if m.args.Overrides.LabelWhiteList != nil {
-		c.LabelWhiteList = *m.args.Overrides.LabelWhiteList
+		c.LabelWhiteList = &m.args.Overrides.LabelWhiteList.Regexp
 	}
 	if m.args.Overrides.ResyncPeriod != nil {
 		c.ResyncPeriod = *m.args.Overrides.ResyncPeriod
diff --git a/pkg/nfd-master/node-updater-pool.go b/pkg/nfd-master/node-updater-pool.go
index f98b4f235..2fa4ae3c0 100644
--- a/pkg/nfd-master/node-updater-pool.go
+++ b/pkg/nfd-master/node-updater-pool.go
@@ -28,7 +28,7 @@ import (
 
 type nodeUpdaterPool struct {
 	queue workqueue.RateLimitingInterface
-	sync.RWMutex
+	sync.Mutex
 
 	wg        sync.WaitGroup
 	nfdMaster *nfdMaster
@@ -114,9 +114,3 @@ func (u *nodeUpdaterPool) stop() {
 	u.queue.ShutDown()
 	u.wg.Wait()
 }
-
-func (u *nodeUpdaterPool) addNode(nodeName string) {
-	u.RLock()
-	defer u.RUnlock()
-	u.queue.Add(nodeName)
-}
diff --git a/pkg/utils/flags.go b/pkg/utils/flags.go
index 1038b5156..fe7f5d2cd 100644
--- a/pkg/utils/flags.go
+++ b/pkg/utils/flags.go
@@ -42,25 +42,6 @@ func (a *RegexpVal) Set(val string) error {
 	return err
 }
 
-// UnmarshalJSON implements the Unmarshaler interface from "encoding/json"
-func (a *RegexpVal) UnmarshalJSON(data []byte) error {
-	var v interface{}
-	if err := json.Unmarshal(data, &v); err != nil {
-		return err
-	}
-	switch val := v.(type) {
-	case string:
-		if r, err := regexp.Compile(string(val)); err != nil {
-			return err
-		} else {
-			*a = RegexpVal{*r}
-		}
-	default:
-		return fmt.Errorf("invalid regexp %s", data)
-	}
-	return nil
-}
-
 // StringSetVal is a Value encapsulating a set of comma-separated strings
 type StringSetVal map[string]struct{}
 


Test patch:
diff --git a/pkg/nfd-master/nfd-master-internal_test.go b/pkg/nfd-master/nfd-master-internal_test.go
index 548fcfebd..16ba81088 100644
--- a/pkg/nfd-master/nfd-master-internal_test.go
+++ b/pkg/nfd-master/nfd-master-internal_test.go
@@ -22,7 +22,6 @@ import (
 	"maps"
 	"os"
 	"path/filepath"
-	"regexp"
 	"sort"
 	"strings"
 	"testing"
@@ -108,7 +107,7 @@ func newFakeNfdAPIController(client *fakenfdclient.Clientset) *nfdController {
 func newFakeMaster(cli k8sclient.Interface) *nfdMaster {
 	return &nfdMaster{
 		nodeName:  testNodeName,
-		config:    &NFDConfig{LabelWhiteList: utils.RegexpVal{Regexp: *regexp.MustCompile("")}},
+		config:    &NFDConfig{},
 		k8sClient: cli,
 	}
 }