test: add unit and e2e tests of the cr restrictions

Signed-off-by: AhmedGrati <ahmedgrati1999@gmail.com>

cmd/nfd-master/main.go

@@ -169,11 +169,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")

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

@@ -596,35 +596,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, true)
+		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, true)
+		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 := []apihelper.JsonPatch{
 				apihelper.NewJsonPatch("remove", jsonPath, "key-2", ""),
 				apihelper.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, true)
+			p := createPatches([]string{"key-1"}, existingItems, newItems, jsonPath, overwriteKeys)
 			expected := []apihelper.JsonPatch{
 				apihelper.NewJsonPatch("add", jsonPath, "new-key", newItems["new-key"]),
 				apihelper.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, true)
+			p := createPatches([]string{"key-1", "key-2", "key-3", "foo"}, existingItems, newItems, jsonPath, overwriteKeys)
 			expected := []apihelper.JsonPatch{
 				apihelper.NewJsonPatch("add", jsonPath, "new-key", newItems["new-key"]),
 				apihelper.NewJsonPatch("add", jsonPath, "key-4", newItems["key-4"]),
@@ -634,6 +635,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 := []apihelper.JsonPatch{
+				apihelper.NewJsonPatch("add", jsonPath, "key-4", newItems["key-4"]),
+			}
+			So(sortJsonPatches(p), ShouldResemble, sortJsonPatches(expected))
+		})
 	})
 }
 
@@ -719,6 +730,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)
@@ -737,6 +755,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)
 			})
 		})
 
@@ -1002,3 +1026,61 @@ 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{},
+		},
+	}
+
+	mockHelper := &apihelper.MockAPIHelpers{}
+	mockMaster := newMockMaster(mockHelper)
+
+	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))
+			})
+		})
+	}
+}

test/e2e/node_feature_discovery_test.go

@@ -995,6 +995,46 @@ resyncPeriod: "1s"
 					Expect(err).NotTo(HaveOccurred())
 				})
 			})
+
+			Context("allowed namespaces restriction is respected or not", func() {
+				BeforeEach(func(ctx context.Context) {
+					extraMasterPodSpecOpts = []testpod.SpecOption{
+						testpod.SpecWithConfigMap("nfd-master-conf", "/etc/kubernetes/node-feature-discovery"),
+					}
+					cm := testutils.NewConfigMap("nfd-master-conf", "nfd-master.conf", `
+restrictions:
+	allowedNamespaces: ["non-existant-namespace"]
+`)
+					_, err := f.ClientSet.CoreV1().ConfigMaps(f.Namespace.Name).Create(ctx, cm, metav1.CreateOptions{})
+					Expect(err).NotTo(HaveOccurred())
+				})
+				It("Nothing should be created", func(ctx context.Context) {
+					// deploy node feature object
+					if !useNodeFeatureApi {
+						Skip("NodeFeature API not enabled")
+					}
+
+					nodes, err := getNonControlPlaneNodes(ctx, f.ClientSet)
+					Expect(err).NotTo(HaveOccurred())
+
+					targetNodeName := nodes[0].Name
+					Expect(targetNodeName).ToNot(BeEmpty(), "No suitable worker node found")
+
+					// Apply Node Feature object
+					By("Creating NodeFeature object")
+					nodeFeatures, err := testutils.CreateOrUpdateNodeFeaturesFromFile(ctx, nfdClient, "nodefeature-1.yaml", f.Namespace.Name, targetNodeName)
+					Expect(err).NotTo(HaveOccurred())
+
+					By("Verifying node labels from NodeFeature object #1 are not created")
+					// No labels should be created since the f.Namespace is not in the allowed Namespaces
+					expectedLabels := map[string]k8sLabels{}
+					eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchLabels(expectedLabels, nodes))
+
+					By("Deleting NodeFeature object")
+					err = nfdClient.NfdV1alpha1().NodeFeatures(f.Namespace.Name).Delete(ctx, nodeFeatures[0], metav1.DeleteOptions{})
+					Expect(err).NotTo(HaveOccurred())
+				})
+			})
 		})
 	}