[WIP] feat: configure CR restrictions

cmd/nfd-master/main.go

@@ -72,6 +72,18 @@ func main() {
 			args.Overrides.ResyncPeriod = overrides.ResyncPeriod
 		case "nfd-api-parallelism":
 			args.Overrides.NfdApiParallelism = overrides.NfdApiParallelism
+		case "max-labels-per-cr":
+			args.Overrides.MaxLabelsPerCR = overrides.MaxLabelsPerCR
+		case "max-taints-per-cr":
+			args.Overrides.MaxTaintsPerCR = overrides.MaxTaintsPerCR
+		case "max-extended-resources-per-cr":
+			args.Overrides.MaxExtendedResourcesPerCR = overrides.MaxExtendedResourcesPerCR
+		case "allowed-namespaces":
+			args.Overrides.AllowedNamespaces = overrides.AllowedNamespaces
+		case "deny-node-feature-labels":
+			args.Overrides.DenyNodeFeatureLabels = overrides.DenyNodeFeatureLabels
+		case "overwrite-labels":
+			args.Overrides.OverwriteLabels = overrides.OverwriteLabels
 		case "enable-nodefeature-api":
 			klog.InfoS("-enable-nodefeature-api is deprecated, will be removed in a future release along with the deprecated gRPC API")
 		case "ca-file":
@@ -159,11 +171,12 @@ func initFlags(flagset *flag.FlagSet) (*master.Args, *master.ConfigOverrideArgs)
 	args.Klog = klogutils.InitKlogFlags(flagset)
 
 	overrides := &master.ConfigOverrideArgs{
-		LabelWhiteList: &utils.RegexpVal{},
-		DenyLabelNs:    &utils.StringSetVal{},
-		ExtraLabelNs:   &utils.StringSetVal{},
-		ResourceLabels: &utils.StringSetVal{},
-		ResyncPeriod:   &utils.DurationVal{Duration: time.Duration(1) * time.Hour},
+		LabelWhiteList:    &utils.RegexpVal{},
+		DenyLabelNs:       &utils.StringSetVal{},
+		ExtraLabelNs:      &utils.StringSetVal{},
+		ResourceLabels:    &utils.StringSetVal{},
+		AllowedNamespaces: &utils.StringSliceVal{},
+		ResyncPeriod:      &utils.DurationVal{Duration: time.Duration(1) * time.Hour},
 	}
 	flagset.Var(overrides.ExtraLabelNs, "extra-label-ns",
 		"Comma separated list of allowed extra label namespaces")
@@ -174,6 +187,9 @@ func initFlags(flagset *flag.FlagSet) (*master.Args, *master.ConfigOverrideArgs)
 		"Enable node tainting feature")
 	overrides.NoPublish = flagset.Bool("no-publish", false,
 		"Do not publish feature labels")
+	overrides.DenyNodeFeatureLabels = flagset.Bool("deny-node-feature-labels", false,
+		"Deny third-party features from being created i.e. only create raw features")
+	overrides.OverwriteLabels = flagset.Bool("overwrite-labels", true, "Allow to overwrite labels")
 	flagset.Var(overrides.DenyLabelNs, "deny-label-ns",
 		"Comma separated list of denied label namespaces")
 	flagset.Var(overrides.ResourceLabels, "resource-labels",
@@ -184,5 +200,11 @@ func initFlags(flagset *flag.FlagSet) (*master.Args, *master.ConfigOverrideArgs)
 	overrides.NfdApiParallelism = flagset.Int("nfd-api-parallelism", 10, "Defines the maximum number of goroutines responsible of updating nodes. "+
 		"Can be used for the throttling mechanism. It has effect only when -enable-nodefeature-api has been set.")
 
+	// Restrictions flags
+	overrides.MaxLabelsPerCR = flagset.Int("max-labels-per-cr", -1, "Defines the maximum number of labels that can be added by a single NodeFeature CR.")
+	overrides.MaxTaintsPerCR = flagset.Int("max-taints-per-cr", -1, "Defines the maximum number of taints that can be added by a single NodeFeature CR.")
+	overrides.MaxExtendedResourcesPerCR = flagset.Int("max-extended-resources-per-cr", -1, "Defines the maximum number of extended resources that can be added by a single NodeFeature CR.")
+	flagset.Var(overrides.AllowedNamespaces, "allowed-namespaces",
+		"Comma separated list of allowed Kubernetes namespaces to watch")
 	return args, overrides
 }

nfd-master.conf

@@ -0,0 +1,29 @@
+noPublish: false
+autoDefaultNs: true
+extraLabelNs: ["added.ns.io","added.kubernets.io"]
+denyLabelNs: ["denied.ns.io","denied.kubernetes.io"]
+resourceLabels: ["vendor-1.com/feature-1","vendor-2.io/feature-2"]
+enableTaints: false
+labelWhiteList: ""
+resyncPeriod: "2h"
+restrictions:
+    allowedNamespaces: ["default"]
+    denyNodeFeatureLabels: true
+klog:
+    addDirHeader: false
+    alsologtostderr: false
+    logBacktraceAt:
+    logtostderr: true
+    skipHeaders: false
+    stderrthreshold: 2
+    v: 0
+    vmodule:
+    logDir:
+    logFile:
+    logFileMaxSize: 1800
+    skipLogHeaders: false
+leaderElection:
+    leaseDuration: 15s
+    renewDeadline: 10s
+    retryPeriod: 2s
+nfdApiParallelism: 10

pkg/nfd-master/nfd-api-controller.go

@@ -35,8 +35,9 @@
 )
 
 type nfdController struct {
-	featureLister nfdlisters.NodeFeatureLister
-	ruleLister    nfdlisters.NodeFeatureRuleLister
+	featureLister     nfdlisters.NodeFeatureLister
+	ruleLister        nfdlisters.NodeFeatureRuleLister
+	allowedNamespaces []string
 
 	stopChan chan struct{}
 
@@ -47,13 +48,15 @@
 type nfdApiControllerOptions struct {
 	DisableNodeFeature bool
 	ResyncPeriod       time.Duration
+	AllowedNamespaces  []string
 }
 
 func newNfdController(config *restclient.Config, nfdApiControllerOptions nfdApiControllerOptions) (*nfdController, error) {
 	c := &nfdController{
 		stopChan:           make(chan struct{}, 1),
 		updateAllNodesChan: make(chan struct{}, 1),
 		updateOneNodeChan:  make(chan string),
+		allowedNamespaces:  nfdApiControllerOptions.AllowedNamespaces,
 	}
 
 	nfdClient := nfdclientset.NewForConfigOrDie(config)
@@ -68,17 +71,23 @@
 			AddFunc: func(obj interface{}) {
 				nfr := obj.(*nfdv1alpha1.NodeFeature)
 				klog.V(2).InfoS("NodeFeature added", "nodefeature", klog.KObj(nfr))
-				c.updateOneNode("NodeFeature", nfr)
+				if c.isNamespaceAllowed(nfr.Namespace) {
+					c.updateOneNode("NodeFeature", nfr)
+				}
 			},
 			UpdateFunc: func(oldObj, newObj interface{}) {
 				nfr := newObj.(*nfdv1alpha1.NodeFeature)
 				klog.V(2).InfoS("NodeFeature updated", "nodefeature", klog.KObj(nfr))
-				c.updateOneNode("NodeFeature", nfr)
+				if c.isNamespaceAllowed(nfr.Namespace) {
+					c.updateOneNode("NodeFeature", nfr)
+				}
 			},
 			DeleteFunc: func(obj interface{}) {
 				nfr := obj.(*nfdv1alpha1.NodeFeature)
 				klog.V(2).InfoS("NodeFeature deleted", "nodefeature", klog.KObj(nfr))
-				c.updateOneNode("NodeFeature", nfr)
+				if c.isNamespaceAllowed(nfr.Namespace) {
+					c.updateOneNode("NodeFeature", nfr)
+				}
 			},
 		}); err != nil {
 			return nil, err
@@ -129,6 +138,16 @@
 	}
 }
 
+func (c *nfdController) isNamespaceAllowed(namespace string) bool {
+	for _, ns := range c.allowedNamespaces {
+		if ns == namespace {
+			return true
+		}
+	}
+
+	return false
+}
+
 func (c *nfdController) updateOneNode(typ string, obj metav1.Object) {
 	nodeName, err := getNodeNameForObj(obj)
 	if err != nil {

pkg/nfd-master/nfd-api-controller_test.go

@@ -42,3 +42,33 @@ func TestGetNodeNameForObj(t *testing.T) {
 	assert.Nil(t, err)
 	assert.Equal(t, n, "node-1")
 }
+
+func TestIsNamespaceAllowed(t *testing.T) {
+	c := &nfdController{}
+
+	testcases := []struct {
+		name              string
+		objectNamespace   string
+		allowedNamespaces []string
+		expectedResult    bool
+	}{
+		{
+			name:              "namespace not allowed",
+			objectNamespace:   "ns3",
+			allowedNamespaces: []string{"ns1", "ns2"},
+			expectedResult:    false,
+		},
+		{
+			name:              "namespace is allowed",
+			objectNamespace:   "ns1",
+			allowedNamespaces: []string{"ns2", "ns1"},
+			expectedResult:    false,
+		},
+	}
+
+	for _, tc := range testcases {
+		c.allowedNamespaces = tc.allowedNamespaces
+		res := c.isNamespaceAllowed(tc.name)
+		assert.Equal(t, res, tc.expectedResult)
+	}
+}

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

@@ -500,35 +500,36 @@ func TestFilterLabels(t *testing.T) {
 func TestCreatePatches(t *testing.T) {
 	Convey("When creating JSON patches", t, func() {
 		existingItems := map[string]string{"key-1": "val-1", "key-2": "val-2", "key-3": "val-3"}
+		overwriteKeys := true
 		jsonPath := "/root"
 
-		Convey("When when there are neither itmes to remoe nor to add or update", func() {
-			p := createPatches([]string{"foo", "bar"}, existingItems, map[string]string{}, jsonPath)
+		Convey("When there are neither itmes to remoe nor to add or update", func() {
+			p := createPatches([]string{"foo", "bar"}, existingItems, map[string]string{}, jsonPath, overwriteKeys)
 			So(len(p), ShouldEqual, 0)
 		})
 
-		Convey("When when there are itmes to remoe but none to add or update", func() {
-			p := createPatches([]string{"key-2", "key-3", "foo"}, existingItems, map[string]string{}, jsonPath)
+		Convey("When there are itmes to remoe but none to add or update", func() {
+			p := createPatches([]string{"key-2", "key-3", "foo"}, existingItems, map[string]string{}, jsonPath, overwriteKeys)
 			expected := []utils.JsonPatch{
 				utils.NewJsonPatch("remove", jsonPath, "key-2", ""),
 				utils.NewJsonPatch("remove", jsonPath, "key-3", ""),
 			}
 			So(sortJsonPatches(p), ShouldResemble, sortJsonPatches(expected))
 		})
 
-		Convey("When when there are no itmes to remove but new items to add", func() {
+		Convey("When there are no itmes to remove but new items to add", func() {
 			newItems := map[string]string{"new-key": "new-val", "key-1": "new-1"}
-			p := createPatches([]string{"key-1"}, existingItems, newItems, jsonPath)
+			p := createPatches([]string{"key-1"}, existingItems, newItems, jsonPath, overwriteKeys)
 			expected := []utils.JsonPatch{
 				utils.NewJsonPatch("add", jsonPath, "new-key", newItems["new-key"]),
 				utils.NewJsonPatch("replace", jsonPath, "key-1", newItems["key-1"]),
 			}
 			So(sortJsonPatches(p), ShouldResemble, sortJsonPatches(expected))
 		})
 
-		Convey("When when there are items to remove add and update", func() {
+		Convey("When there are items to remove add and update", func() {
 			newItems := map[string]string{"new-key": "new-val", "key-2": "new-2", "key-4": "val-4"}
-			p := createPatches([]string{"key-1", "key-2", "key-3", "foo"}, existingItems, newItems, jsonPath)
+			p := createPatches([]string{"key-1", "key-2", "key-3", "foo"}, existingItems, newItems, jsonPath, overwriteKeys)
 			expected := []utils.JsonPatch{
 				utils.NewJsonPatch("add", jsonPath, "new-key", newItems["new-key"]),
 				utils.NewJsonPatch("add", jsonPath, "key-4", newItems["key-4"]),
@@ -538,6 +539,16 @@ func TestCreatePatches(t *testing.T) {
 			}
 			So(sortJsonPatches(p), ShouldResemble, sortJsonPatches(expected))
 		})
+
+		Convey("When overwrite of keys is denied and there is already an existant key", func() {
+			overwriteKeys = false
+			newItems := map[string]string{"key-1": "new-2", "key-4": "val-4"}
+			p := createPatches([]string{}, existingItems, newItems, jsonPath, overwriteKeys)
+			expected := []utils.JsonPatch{
+				utils.NewJsonPatch("add", jsonPath, "key-4", newItems["key-4"]),
+			}
+			So(sortJsonPatches(p), ShouldResemble, sortJsonPatches(expected))
+		})
 	})
 }
 
@@ -623,6 +634,13 @@ leaderElection:
   leaseDuration: 20s
   renewDeadline: 4s
   retryPeriod: 30s
+restrictions:
+  allowedNamespaces: ["default"]
+  denyNodeFeatureLabels: true
+  maxLabelsPerCr: 3
+  maxTaintsPerCr: 4
+  maxExtendedResourcesPerCr: 5
+  overwriteLabels: true
 `)
 		f.Close()
 		So(err, ShouldBeNil)
@@ -641,6 +659,12 @@ leaderElection:
 				So(master.config.LeaderElection.LeaseDuration.Seconds(), ShouldEqual, float64(20))
 				So(master.config.LeaderElection.RenewDeadline.Seconds(), ShouldEqual, float64(4))
 				So(master.config.LeaderElection.RetryPeriod.Seconds(), ShouldEqual, float64(30))
+				So(master.config.Restrictions.AllowedNamespaces.String(), ShouldEqual, "default")
+				So(master.config.Restrictions.DenyNodeFeatureLabels, ShouldEqual, true)
+				So(master.config.Restrictions.MaxLabelsPerCR, ShouldEqual, 3)
+				So(master.config.Restrictions.MaxTaintsPerCR, ShouldEqual, 4)
+				So(master.config.Restrictions.MaxExtendedResourcesPerCR, ShouldEqual, 5)
+				So(master.config.Restrictions.OverwriteLabels, ShouldEqual, true)
 			})
 		})
 
@@ -884,3 +908,60 @@ func TestGetDynamicValue(t *testing.T) {
 		})
 	}
 }
+
+func TestFilterTaints(t *testing.T) {
+	testcases := []struct {
+		name           string
+		taints         []corev1.Taint
+		maxTaints      int
+		expectedResult []corev1.Taint
+	}{
+		{
+			name: "no restriction on the number of taints",
+			taints: []corev1.Taint{
+				{
+					Key:    "feature.node.kubernetes.io/key1",
+					Value:  "dummy",
+					Effect: corev1.TaintEffectNoSchedule,
+				},
+			},
+			maxTaints: 0,
+			expectedResult: []corev1.Taint{
+				{
+					Key:    "feature.node.kubernetes.io/key1",
+					Value:  "dummy",
+					Effect: corev1.TaintEffectNoSchedule,
+				},
+			},
+		},
+		{
+			name: "max of 1 Taint should be generated",
+			taints: []corev1.Taint{
+				{
+					Key:    "feature.node.kubernetes.io/key1",
+					Value:  "dummy",
+					Effect: corev1.TaintEffectNoSchedule,
+				},
+				{
+					Key:    "feature.node.kubernetes.io/key2",
+					Value:  "dummy",
+					Effect: corev1.TaintEffectNoSchedule,
+				},
+			},
+			maxTaints:      1,
+			expectedResult: []corev1.Taint{},
+		},
+	}
+
+	mockMaster := newFakeMaster(nil)
+
+	for _, tc := range testcases {
+		t.Run(tc.name, func(t *testing.T) {
+			mockMaster.config.Restrictions.MaxTaintsPerCR = tc.maxTaints
+			res := mockMaster.filterTaints(tc.taints)
+			Convey("The expected number of taints should be correct", t, func() {
+				So(len(res), ShouldEqual, len(tc.expectedResult))
+			})
+		})
+	}
+}