Allow to change labels namespace

The aim here is to allow to override the default namespace
of NFD. The allowed namespaces are whitelisted.
See https://github.com/kubernetes-sigs/node-feature-discovery/issues/227

Signed-off-by: Jordan Jacobelli <jjacobelli@nvidia.com>

README.md

@@ -53,27 +53,30 @@ nfd-master.
   Usage:
   nfd-master [--no-publish] [--label-whitelist=<pattern>] [--port=<port>]
      [--ca-file=<path>] [--cert-file=<path>] [--key-file=<path>]
-     [--verify-node-name]
+     [--verify-node-name] [--extra-label-ns=<list>]
   nfd-master -h | --help
   nfd-master --version
 
   Options:
-  -h --help                   Show this screen.
+  -h --help                       Show this screen.
-  --version                   Output version and exit.
+  --version                       Output version and exit.
-  --port=<port>               Port on which to listen for connections.
+  --port=<port>                   Port on which to listen for connections.
-                              [Default: 8080]
+                                  [Default: 8080]
-  --ca-file=<path>            Root certificate for verifying connections
+  --ca-file=<path>                Root certificate for verifying connections
-                              [Default: ]
+                                  [Default: ]
-  --cert-file=<path>          Certificate used for authenticating connections
+  --cert-file=<path>              Certificate used for authenticating connections
-                              [Default: ]
+                                  [Default: ]
-  --key-file=<path>           Private key matching --cert-file
+  --key-file=<path>               Private key matching --cert-file
-                              [Default: ]
+                                  [Default: ]
-  --verify-node-name		  Verify worker node name against CN from the TLS
+  --verify-node-name              Verify worker node name against CN from the TLS
-                              certificate. Only has effect when TLS authentication
+                                  certificate. Only has effect when TLS authentication
-                              has been enabled.
+                                  has been enabled.
-  --no-publish                Do not publish feature labels
+  --no-publish                    Do not publish feature labels
-  --label-whitelist=<pattern> Regular expression to filter label names to
+  --label-whitelist=<pattern>     Regular expression to filter label names to
-                              publish to the Kubernetes API server. [Default: ]
+                                  publish to the Kubernetes API server. [Default: ]
+  --extra-label-ns=<list>         Comma separated list of allowed extra label namespaces
+                                  [Default: ]
+
 ```
 
 ### NFD-Worker
@@ -368,8 +371,13 @@ slash (`/`) it is used as the label name as is, without any additional prefix.
 This makes it possible for the user to fully control the feature label names,
 e.g. for overriding labels created by other feature sources.
 
-The value of the label is either `true` (for binary labels) or `<value>`
+You can also override the default namespace of your labels using this format:
-(for non-binary labels).
+`<namespace>/<name>[=<value>]`. You must whitelist your namespace using the
+`--extra-label-ns` option on the master. In this case, the name of the
+file will not be added to the label name. For example, if you want to add the
+label `my.namespace.org/my-label=value`, your hook output or file must contains
+`my.namespace.org/my-label=value` and you must add
+`--extra-label-ns=my.namespace.org` on the master command line.
 
 `stderr` output of the hooks is propagated to NFD log so it can be used for
 debugging and logging.
@@ -383,6 +391,7 @@ MY_FEATURE_1
 MY_FEATURE_2=myvalue
 /override_source-OVERRIDE_BOOL
 /override_source-OVERRIDE_VALUE=123
+override.namespace/value=456
 ```
 which, in turn, will translate into the following node labels:
 ```
@@ -390,6 +399,7 @@ feature.node.kubernetes.io/my-source-MY_FEATURE_1=true
 feature.node.kubernetes.io/my-source-MY_FEATURE_2=myvalue
 feature.node.kubernetes.io/override_source-OVERRIDE_BOOL=true
 feature.node.kubernetes.io/override_source-OVERRIDE_VALUE=123
+override.namespace/value=456
 ```
 
 **A file example:**<br/>
@@ -401,6 +411,7 @@ MY_FEATURE_1
 MY_FEATURE_2=myvalue
 /override_source-OVERRIDE_BOOL
 /override_source-OVERRIDE_VALUE=123
+override.namespace/value=456
 ```
 which, in turn, will translate into the following node labels:
 ```
@@ -408,6 +419,7 @@ feature.node.kubernetes.io/my-source-MY_FEATURE_1=true
 feature.node.kubernetes.io/my-source-MY_FEATURE_2=myvalue
 feature.node.kubernetes.io/override_source-OVERRIDE_BOOL=true
 feature.node.kubernetes.io/override_source-OVERRIDE_VALUE=123
+override.namespace/value=456
 ```
 
 NFD tries to run any regular files found from the hooks directory. Any

cmd/nfd-master/main.go

@@ -21,6 +21,7 @@ import (
 	"log"
 	"regexp"
 	"strconv"
+	"strings"
 
 	"github.com/docopt/docopt-go"
 	master "sigs.k8s.io/node-feature-discovery/pkg/nfd-master"
@@ -64,27 +65,29 @@ func argsParse(argv []string) (master.Args, error) {
   Usage:
   %s [--no-publish] [--label-whitelist=<pattern>] [--port=<port>]
      [--ca-file=<path>] [--cert-file=<path>] [--key-file=<path>]
-     [--verify-node-name]
+     [--verify-node-name] [--extra-label-ns=<list>]
   %s -h | --help
   %s --version
 
   Options:
-  -h --help                   Show this screen.
+  -h --help                       Show this screen.
-  --version                   Output version and exit.
+  --version                       Output version and exit.
-  --port=<port>               Port on which to listen for connections.
+  --port=<port>                   Port on which to listen for connections.
-                              [Default: 8080]
+                                  [Default: 8080]
-  --ca-file=<path>            Root certificate for verifying connections
+  --ca-file=<path>                Root certificate for verifying connections
-                              [Default: ]
+                                  [Default: ]
-  --cert-file=<path>          Certificate used for authenticating connections
+  --cert-file=<path>              Certificate used for authenticating connections
-                              [Default: ]
+                                  [Default: ]
-  --key-file=<path>           Private key matching --cert-file
+  --key-file=<path>               Private key matching --cert-file
-                              [Default: ]
+                                  [Default: ]
-  --verify-node-name		  Verify worker node name against CN from the TLS
+  --verify-node-name              Verify worker node name against CN from the TLS
-                              certificate. Only has effect when TLS authentication
+                                  certificate. Only has effect when TLS authentication
-                              has been enabled.
+                                  has been enabled.
-  --no-publish                Do not publish feature labels
+  --no-publish                    Do not publish feature labels
-  --label-whitelist=<pattern> Regular expression to filter label names to
+  --label-whitelist=<pattern>     Regular expression to filter label names to
-                              publish to the Kubernetes API server. [Default: ]`,
+                                  publish to the Kubernetes API server. [Default: ]
+  --extra-label-ns=<list>         Comma separated list of allowed extra label namespaces
+                                  [Default: ]`,
 		ProgramName,
 		ProgramName,
 		ProgramName,
@@ -109,6 +112,7 @@ func argsParse(argv []string) (master.Args, error) {
 		return args, fmt.Errorf("error parsing whitelist regex (%s): %s", arguments["--label-whitelist"], err)
 	}
 	args.VerifyNodeName = arguments["--verify-node-name"].(bool)
+	args.ExtraLabelNs = strings.Split(arguments["--extra-label-ns"].(string), ",")
 
 	return args, nil
 }

pkg/nfd-master/nfd-master-internal_test.go

@@ -237,6 +237,29 @@ func TestSetLabels(t *testing.T) {
 			})
 		})
 
+		Convey("When --extra-label-ns is specified", func() {
+			mockServer.args.ExtraLabelNs = []string{"valid.ns"}
+			mockHelper.On("GetClient").Return(mockClient, nil)
+			mockHelper.On("GetNode", mockClient, workerName).Return(mockNode, nil)
+			mockHelper.On("UpdateNode", mockClient, mockNode).Return(nil)
+			mockLabels := map[string]string{"feature-1": "val-1",
+				"valid.ns/feature-2":   "val-2",
+				"invalid.ns/feature-3": "val-3"}
+			mockReq := &labeler.SetLabelsRequest{NodeName: workerName, NfdVersion: workerVer, Labels: mockLabels}
+			_, err := mockServer.SetLabels(mockCtx, mockReq)
+			Convey("Error is nil", func() {
+				So(err, ShouldBeNil)
+			})
+			Convey("Node object should only have allowed label namespaces", func() {
+				So(len(mockNode.Labels), ShouldEqual, 2)
+				So(mockNode.Labels, ShouldResemble, map[string]string{labelNs + "feature-1": "val-1", "valid.ns/feature-2": "val-2"})
+
+				a := map[string]string{annotationNs + "worker.version": workerVer, annotationNs + "feature-labels": "feature-1,valid.ns/feature-2"}
+				So(len(mockNode.Annotations), ShouldEqual, len(a))
+				So(mockNode.Annotations, ShouldResemble, a)
+			})
+		})
+
 		mockErr := errors.New("mock-error")
 		Convey("When node update fails", func() {
 			mockHelper.On("GetClient").Return(mockClient, mockErr)

pkg/nfd-master/nfd-master.go

@@ -64,6 +64,7 @@ type Annotations map[string]string
 type Args struct {
 	CaFile         string
 	CertFile       string
+	ExtraLabelNs   []string
 	KeyFile        string
 	LabelWhiteList *regexp.Regexp
 	NoPublish      bool
@@ -202,13 +203,30 @@ func updateMasterNode(helper apihelper.APIHelpers) error {
 	return nil
 }
 
-// Filter labels if whitelist has been defined
+// Filter labels by namespace and name whitelist
-func filterFeatureLabels(labels Labels, labelWhiteList *regexp.Regexp) Labels {
+func filterFeatureLabels(labels Labels, extraLabelNs []string, labelWhiteList *regexp.Regexp) Labels {
-	for name := range labels {
+	for label := range labels {
+		split := strings.SplitN(label, "/", 2)
+		name := split[0]
+
+		// Check namespaced labels, filter out if ns is not whitelisted
+		if len(split) == 2 {
+			ns := split[0]
+			name = split[1]
+			for i, extraNs := range extraLabelNs {
+				if ns == extraNs {
+					break
+				} else if i == len(extraLabelNs)-1 {
+					stderrLogger.Printf("Namespace '%s' is not allowed. Ignoring label '%s'\n", ns, label)
+					delete(labels, label)
+				}
+			}
+		}
+
 		// Skip if label doesn't match labelWhiteList
 		if !labelWhiteList.MatchString(name) {
 			stderrLogger.Printf("%s does not match the whitelist (%s) and will not be published.", name, labelWhiteList.String())
-			delete(labels, name)
+			delete(labels, label)
 		}
 	}
 	return labels
@@ -247,7 +265,7 @@ func (s *labelerServer) SetLabels(c context.Context, r *pb.SetLabelsRequest) (*p
 	}
 	stdoutLogger.Printf("REQUEST Node: %s NFD-version: %s Labels: %s", r.NodeName, r.NfdVersion, r.Labels)
 
-	labels := filterFeatureLabels(r.Labels, s.args.LabelWhiteList)
+	labels := filterFeatureLabels(r.Labels, s.args.ExtraLabelNs, s.args.LabelWhiteList)
 
 	if !s.args.NoPublish {
 		// Advertise NFD worker version and label names as annotations
@@ -320,14 +338,22 @@ func removeLabelsWithPrefix(n *api.Node, search string) {
 // Removes NFD labels from a Node object
 func removeLabels(n *api.Node, labelNames []string) {
 	for _, l := range labelNames {
-		delete(n.Labels, labelNs+l)
+		if strings.Contains(l, "/") {
+			delete(n.Labels, l)
+		} else {
+			delete(n.Labels, labelNs+l)
+		}
 	}
 }
 
 // Add NFD labels to a Node object.
 func addLabels(n *api.Node, labels map[string]string) {
 	for k, v := range labels {
-		n.Labels[labelNs+k] = v
+		if strings.Contains(k, "/") {
+			n.Labels[k] = v
+		} else {
+			n.Labels[labelNs+k] = v
+		}
 	}
 }
 

pkg/nfd-worker/nfd-worker.go

@@ -324,7 +324,12 @@ func getFeatureLabels(source source.FeatureSource) (labels Labels, err error) {
 		label := prefix + k
 		// Validate label name. Use dummy namespace 'ns' because there is no
 		// function to validate just the name part
-		errs := validation.IsQualifiedName("ns/" + label)
+		labelName := "ns/" + label
+		// Do not use dummy namespace if there is already a namespace
+		if strings.Contains(label, "/") {
+			labelName = label
+		}
+		errs := validation.IsQualifiedName(labelName)
 		if len(errs) > 0 {
 			stderrLogger.Printf("Ignoring invalid feature name '%s': %s", label, errs)
 			continue

source/local/local.go

@@ -71,9 +71,15 @@ func parseFeatures(lines [][]byte, prefix string) source.Features {
 			lineSplit := strings.SplitN(string(line), "=", 2)
 
 			// Check if we need to add prefix
-			key := prefix + "-" + lineSplit[0]
+			var key string
-			if lineSplit[0][0] == '/' {
+			if strings.Contains(lineSplit[0], "/") {
-				key = lineSplit[0][1:]
+				if lineSplit[0][0] == '/' {
+					key = lineSplit[0][1:]
+				} else {
+					key = lineSplit[0]
+				}
+			} else {
+				key = prefix + "-" + lineSplit[0]
 			}
 
 			// Check if it's a boolean value