Merge pull request #1051 from AhmedGrati/feat-add-deny-label-ns-with-wildcard

feat: add deny-label-ns flag which supports wildcard

cmd/nfd-master/main.go

@@ -85,12 +85,15 @@ func main() {
 func initFlags(flagset *flag.FlagSet) *master.Args {
 	args := &master.Args{
 		LabelWhiteList: utils.RegexpVal{Regexp: *regexp.MustCompile("")},
+		DenyLabelNs:    map[string]struct{}{"*.kubernetes.io": {}},
 	}
 
 	flagset.StringVar(&args.CaFile, "ca-file", "",
 		"Root certificate for verifying connections")
 	flagset.StringVar(&args.CertFile, "cert-file", "",
 		"Certificate used for authenticating connections")
+	flagset.Var(&args.DenyLabelNs, "deny-label-ns",
+		"Comma separated list of denied label namespaces")
 	flagset.Var(&args.ExtraLabelNs, "extra-label-ns",
 		"Comma separated list of allowed extra label namespaces")
 	flagset.StringVar(&args.Instance, "instance", "",

deployment/helm/node-feature-discovery/templates/master.yaml

@@ -86,6 +86,9 @@ spec:
             {{- if .Values.master.extraLabelNs | empty | not }}
             - "-extra-label-ns={{- join "," .Values.master.extraLabelNs }}"
             {{- end }}
+            {{- if .Values.master.denyLabelNs | empty | not }}
+            - "-deny-label-ns={{- join "," .Values.master.denyLabelNs }}"
+            {{- end }}
             {{- if .Values.master.resourceLabels | empty | not }}
             - "-resource-labels={{- join "," .Values.master.resourceLabels }}"
             {{- end }}

deployment/helm/node-feature-discovery/values.yaml

@@ -17,6 +17,7 @@ master:
   port: 8080
   instance:
   featureApi:
+  denyLabelNs: []
   extraLabelNs: []
   resourceLabels: []
   crdController: null

docs/reference/master-commandline-reference.md

@@ -216,12 +216,10 @@ nfd-master -label-whitelist='.*cpuid\.'
 ### -extra-label-ns
 
 The `-extra-label-ns` flag specifies a comma-separated list of allowed feature
-label namespaces. By default, nfd-master only allows creating labels in the
+label namespaces. This option can be used to allow
-default `feature.node.kubernetes.io` and `profile.node.kubernetes.io` label
-namespaces and their sub-namespaces (e.g. `vendor.feature.node.kubernetes.io`
-and `sub.ns.profile.node.kubernetes.io`). This option can be used to allow
 other vendor or application specific namespaces for custom labels from the
-local and custom feature sources.
+local and custom feature sources, even though these labels were denied using
+the `deny-label-ns` flag.
 
 The same namespace control and this flag applies Extended Resources (created
 with `-resource-labels`), too.
@@ -234,6 +232,28 @@ Example:
 nfd-master -extra-label-ns=vendor-1.com,vendor-2.io
 ```
 
+### -deny-label-ns
+
+The `-deny-label-ns` flag specifies a comma-separated list of excluded
+label namespaces. By default, nfd-master allows creating labels in all
+namespaces, excluding `kubernetes.io` namespace and its sub-namespaces
+(i.e. `*.kubernetes.io`). However, you should note that
+`kubernetes.io` and its sub-namespaces are always denied.
+For example, `nfd-master -deny-label-ns=""` would still disallow
+`kubernetes.io` and `*.kubernetes.io`.
+This option can be used to exclude some vendors or application specific
+namespaces.
+Note that the namespaces `feature.node.kubernetes.io` and `profile.node.kubernetes.io`
+and their sub-namespaces are always allowed and cannot be denied.
+
+Default: *empty*
+
+Example:
+
+```bash
+nfd-master -deny-label-ns=*.vendor.com,vendor-2.io
+```
+
 ### -resource-labels
 
 The `-resource-labels` flag specifies a comma-separated list of features to be

docs/usage/customization-guide.md

@@ -249,9 +249,6 @@ feature.node.kubernetes.io/my-feature.2: "myvalue"
 my.namespace/my-feature.3: "456"
 ```
 
-Note that in the example above `-extra-label-ns=my.namespace` must be specified
-on the nfd-master command line.
-
 ### Hooks
 
 **DEPRECATED** The `local` source executes hooks found in
@@ -303,11 +300,7 @@ key-value pairs, separated by newlines:
 
 The label value defaults to `true`, if not specified.
 
-Label namespace may be specified with `<namespace>/<name>[=<value>]`. The
+Label namespace may be specified with `<namespace>/<name>[=<value>]`.
-namespace must be explicitly allowed with the `-extra-label-ns` command line
-flag of nfd-master if using something else than
-`[<sub-ns>.]feature.node.kubernetes.io` or
-`[<sub-ns>.]profile.node.kubernetes.io`.
 
 ### Mounts
 
@@ -414,13 +407,13 @@ The namespace part (i.e. prefix) of the labels is controlled by nfd:
 
 - All built-in labels use `feature.node.kubernetes.io`. This is also
   the default for user defined features that don't specify any namespace.
-- User-defined labels are allowed to use:
+- Namespaces may be excluded with the
-  - `feature.node.kubernetes.io` and `profile.node.kubernetes.io` plus their
+  [`-deny-label-ns`](../reference/master-commandline-reference.md#-deny-label-ns)
-    sub-namespaces (e.g. `vendor.profile.node.kubernetes.io` and
-    `sub.ns.profile.node.kubernetes.io`) by default
-  - Additional namespaces may be enabled with the
-    [`-extra-label-ns`](../reference/master-commandline-reference.md#-extra-label-ns)
   command line flag of nfd-master
+  - To allow specific namespaces that were denied, you can use
+    [`-extra-label-ns`](../reference/master-commandline-reference.md#-extra-label-ns)
+    command line flag of nfd-master.
+    e.g: `nfd-master -deny-label-ns="*" -extra-label-ns=example.com`
 
 ## Label rule format
 

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

@@ -352,16 +352,18 @@ func TestSetLabels(t *testing.T) {
 			})
 		})
 
-		Convey("When -extra-label-ns and -instance are specified", func() {
+		Convey("When -extra-label-ns, -deny-label-ns and -instance are specified", func() {
 			// In the gRPC request the label names may omit the default ns
 			instance := "foo"
 			vendorFeatureLabel := "vendor." + nfdv1alpha1.FeatureLabelNs + "/feature-4"
 			vendorProfileLabel := "vendor." + nfdv1alpha1.ProfileLabelNs + "/feature-5"
 			mockLabels := map[string]string{"feature-1": "val-1",
 				"valid.ns/feature-2":             "val-2",
-				"invalid.ns/feature-3": "val-3",
+				"random.denied.ns/feature-3":     "val-3",
-				vendorFeatureLabel:     " val-4",
+				"kubernetes.io/feature-4":        "val-4",
-				vendorProfileLabel:     " val-5"}
+				"sub.ns.kubernetes.io/feature-5": "val-5",
+				vendorFeatureLabel:               " val-6",
+				vendorProfileLabel:               " val-7"}
 			expectedPatches := []apihelper.JsonPatch{
 				apihelper.NewJsonPatch("add", "/metadata/annotations", instance+"."+nfdv1alpha1.WorkerVersionAnnotation, workerVer),
 				apihelper.NewJsonPatch("add", "/metadata/annotations",
@@ -374,6 +376,8 @@ func TestSetLabels(t *testing.T) {
 				apihelper.NewJsonPatch("add", "/metadata/labels", vendorProfileLabel, mockLabels[vendorProfileLabel]),
 			}
 
+			mockMaster.deniedNs.normal = map[string]struct{}{"random.denied.ns": {}}
+			mockMaster.deniedNs.wildcard = map[string]struct{}{"kubernetes.io": {}}
 			mockMaster.args.ExtraLabelNs = map[string]struct{}{"valid.ns": {}}
 			mockMaster.args.Instance = instance
 			mockHelper.On("GetClient").Return(mockClient, nil)

pkg/nfd-master/nfd-master.go

@@ -63,6 +63,7 @@ type Annotations map[string]string
 type Args struct {
 	CaFile               string
 	CertFile             string
+	DenyLabelNs          utils.StringSetVal
 	ExtraLabelNs         utils.StringSetVal
 	Instance             string
 	KeyFile              string
@@ -78,6 +79,11 @@ type Args struct {
 	ResourceLabels       utils.StringSetVal
 }
 
+type deniedNs struct {
+	normal   utils.StringSetVal
+	wildcard utils.StringSetVal
+}
+
 type NfdMaster interface {
 	Run() error
 	Stop()
@@ -95,6 +101,7 @@ type nfdMaster struct {
 	ready      chan bool
 	apihelper  apihelper.APIHelpers
 	kubeconfig *restclient.Config
+	deniedNs
 }
 
 // NewNfdMaster creates a new NfdMaster server instance.
@@ -126,6 +133,16 @@ func NewNfdMaster(args *Args) (NfdMaster, error) {
 			return nfd, fmt.Errorf("-ca-file needs to be specified alongside -cert-file and -key-file")
 		}
 	}
+	if args.DenyLabelNs == nil {
+		args.DenyLabelNs = make(utils.StringSetVal)
+	}
+	// Pre-process DenyLabelNS into 2 lists: one for normal ns, and the other for wildcard ns
+	normalDeniedNs, wildcardDeniedNs := preProcessDeniedNamespaces(args.DenyLabelNs)
+	nfd.deniedNs.normal = normalDeniedNs
+	nfd.deniedNs.wildcard = wildcardDeniedNs
+	// We forcibly deny kubernetes.io
+	nfd.deniedNs.normal["kubernetes.io"] = struct{}{}
+	nfd.deniedNs.wildcard[".kubernetes.io"] = struct{}{}
 
 	// Initialize Kubernetes API helpers
 	if !args.NoPublish {
@@ -392,7 +409,7 @@ func (m *nfdMaster) updateMasterNode() error {
 // into extended resources. This function also handles proper namespacing of
 // labels and ERs, i.e. adds the possibly missing default namespace for labels
 // arriving through the gRPC API.
-func filterFeatureLabels(labels Labels, extraLabelNs map[string]struct{}, labelWhiteList regexp.Regexp, extendedResourceNames map[string]struct{}) (Labels, ExtendedResources) {
+func (m *nfdMaster) filterFeatureLabels(labels Labels) (Labels, ExtendedResources) {
 	outLabels := Labels{}
 
 	for label, value := range labels {
@@ -404,15 +421,18 @@ func filterFeatureLabels(labels Labels, extraLabelNs map[string]struct{}, labelW
 		// Check label namespace, filter out if ns is not whitelisted
 		if ns != nfdv1alpha1.FeatureLabelNs && ns != nfdv1alpha1.ProfileLabelNs &&
 			!strings.HasSuffix(ns, nfdv1alpha1.FeatureLabelSubNsSuffix) && !strings.HasSuffix(ns, nfdv1alpha1.ProfileLabelSubNsSuffix) {
-			if _, ok := extraLabelNs[ns]; !ok {
+			// If the namespace is denied, and not present in the extraLabelNs, label will be ignored
+			if isNamespaceDenied(ns, m.deniedNs.wildcard, m.deniedNs.normal) {
+				if _, ok := m.args.ExtraLabelNs[ns]; !ok {
 					klog.Errorf("Namespace %q is not allowed. Ignoring label %q\n", ns, label)
 					continue
 				}
 			}
+		}
 
 		// Skip if label doesn't match labelWhiteList
-		if !labelWhiteList.MatchString(name) {
+		if !m.args.LabelWhiteList.Regexp.MatchString(name) {
-			klog.Errorf("%s (%s) does not match the whitelist (%s) and will not be published.", name, label, labelWhiteList.String())
+			klog.Errorf("%s (%s) does not match the whitelist (%s) and will not be published.", name, label, m.args.LabelWhiteList.Regexp.String())
 			continue
 		}
 		outLabels[label] = value
@@ -420,7 +440,7 @@ func filterFeatureLabels(labels Labels, extraLabelNs map[string]struct{}, labelW
 
 	// Remove labels which are intended to be extended resources
 	extendedResources := ExtendedResources{}
-	for extendedResourceName := range extendedResourceNames {
+	for extendedResourceName := range m.args.ResourceLabels {
 		// Add possibly missing default ns
 		extendedResourceName = addNs(extendedResourceName, nfdv1alpha1.FeatureLabelNs)
 		if value, ok := outLabels[extendedResourceName]; ok {
@@ -449,6 +469,20 @@ func verifyNodeName(cert *x509.Certificate, nodeName string) error {
 	return nil
 }
 
+func isNamespaceDenied(labelNs string, wildcardDeniedNs map[string]struct{}, normalDeniedNs map[string]struct{}) bool {
+	for deniedNs := range normalDeniedNs {
+		if labelNs == deniedNs {
+			return true
+		}
+	}
+	for deniedNs := range wildcardDeniedNs {
+		if strings.HasSuffix(labelNs, deniedNs) {
+			return true
+		}
+	}
+	return false
+}
+
 // SetLabels implements LabelerServer
 func (m *nfdMaster) SetLabels(c context.Context, r *pb.SetLabelsRequest) (*pb.SetLabelsReply, error) {
 	err := authorizeClient(c, m.args.VerifyNodeName, r.NodeName)
@@ -583,7 +617,7 @@ func (m *nfdMaster) refreshNodeFeatures(cli *kubernetes.Clientset, nodeName stri
 		labels[k] = v
 	}
 
-	labels, extendedResources := filterFeatureLabels(labels, m.args.ExtraLabelNs, m.args.LabelWhiteList.Regexp, m.args.ResourceLabels)
+	labels, extendedResources := m.filterFeatureLabels(labels)
 
 	var taints []corev1.Taint
 	if m.args.EnableTaints {
@@ -918,6 +952,22 @@ func stringToNsNames(cslist, ns string) []string {
 	return names
 }
 
+// Seperate denied namespaces into two lists:
+// one contains wildcard namespaces the other contains normal namespaces
+func preProcessDeniedNamespaces(deniedNs map[string]struct{}) (normalDeniedNs map[string]struct{}, wildcardDeniedNs map[string]struct{}) {
+	normalDeniedNs = map[string]struct{}{}
+	wildcardDeniedNs = map[string]struct{}{}
+	for ns := range deniedNs {
+		if strings.HasPrefix(ns, "*") {
+			trimedNs := strings.TrimLeft(ns, "*")
+			wildcardDeniedNs[trimedNs] = struct{}{}
+		} else {
+			normalDeniedNs[ns] = struct{}{}
+		}
+	}
+	return
+}
+
 func (m *nfdMaster) instanceAnnotation(name string) string {
 	if m.args.Instance == "" {
 		return name

pkg/utils/flags.go

@@ -76,8 +76,7 @@ func (a *StringSetVal) String() string {
 	if *a == nil {
 		return ""
 	}
-
+	vals := make([]string, 0, len(*a))
-	vals := make([]string, len(*a), 0)
 	for val := range *a {
 		vals = append(vals, val)
 	}