From 7bad0d583ccdcf01521c97d1a84f520b6684b577 Mon Sep 17 00:00:00 2001 From: AhmedGrati Date: Thu, 15 Feb 2024 15:57:50 +0100 Subject: [PATCH 1/3] feat/nfd-master: support CR restrictions Signed-off-by: AhmedGrati --- go.mod | 2 + pkg/nfd-master/nfd-api-controller.go | 56 +++++- pkg/nfd-master/nfd-api-controller_test.go | 56 ++++++ pkg/nfd-master/nfd-master-internal_test.go | 84 +++++++- pkg/nfd-master/nfd-master.go | 62 +++++- test/e2e/node_feature_discovery_test.go | 221 +++++++++++++++++++++ 6 files changed, 453 insertions(+), 28 deletions(-) diff --git a/go.mod b/go.mod index 7c25f60f5..be823b2d3 100644 --- a/go.mod +++ b/go.mod @@ -2,6 +2,8 @@ module sigs.k8s.io/node-feature-discovery go 1.22.2 +toolchain go1.22.0 + require ( github.com/fsnotify/fsnotify v1.7.0 github.com/golang/protobuf v1.5.4 diff --git a/pkg/nfd-master/nfd-api-controller.go b/pkg/nfd-master/nfd-api-controller.go index 81b5bd35e..905a2ed0c 100644 --- a/pkg/nfd-master/nfd-api-controller.go +++ b/pkg/nfd-master/nfd-api-controller.go @@ -17,6 +17,7 @@ limitations under the License. package nfdmaster import ( + "context" "fmt" "time" @@ -26,7 +27,6 @@ import ( "k8s.io/client-go/tools/cache" "k8s.io/klog/v2" - nfdclientset "sigs.k8s.io/node-feature-discovery/api/generated/clientset/versioned" nfdscheme "sigs.k8s.io/node-feature-discovery/api/generated/clientset/versioned/scheme" nfdinformers "sigs.k8s.io/node-feature-discovery/api/generated/informers/externalversions" nfdinformersv1alpha1 "sigs.k8s.io/node-feature-discovery/api/generated/informers/externalversions/nfd/v1alpha1" @@ -65,6 +65,8 @@ func newNfdController(config *restclient.Config, nfdApiControllerOptions nfdApiC updateOneNodeChan: make(chan string), updateAllNodeFeatureGroupsChan: make(chan struct{}, 1), updateNodeFeatureGroupChan: make(chan string), + k8sClient: nfdApiControllerOptions.K8sClient, + nodeFeatureNamespaceSelector: nfdApiControllerOptions.NodeFeatureNamespaceSelector, } nfdClient := nfdclientset.NewForConfigOrDie(config) @@ -89,25 +91,28 @@ func newNfdController(config *restclient.Config, nfdApiControllerOptions nfdApiC AddFunc: func(obj interface{}) { nfr := obj.(*nfdv1alpha1.NodeFeature) klog.V(2).InfoS("NodeFeature added", "nodefeature", klog.KObj(nfr)) - c.updateOneNode("NodeFeature", nfr) - if !nfdApiControllerOptions.DisableNodeFeatureGroup { - c.updateAllNodeFeatureGroups() + if c.isNamespaceSelected(nfr.Namespace) { + c.updateOneNode("NodeFeature", nfr) + } else { + klog.InfoS("NodeFeature not in selected namespace", "namespace", nfr.Namespace, "name", nfr.Name) } }, UpdateFunc: func(oldObj, newObj interface{}) { nfr := newObj.(*nfdv1alpha1.NodeFeature) klog.V(2).InfoS("NodeFeature updated", "nodefeature", klog.KObj(nfr)) - c.updateOneNode("NodeFeature", nfr) - if !nfdApiControllerOptions.DisableNodeFeatureGroup { - c.updateAllNodeFeatureGroups() + if c.isNamespaceSelected(nfr.Namespace) { + c.updateOneNode("NodeFeature", nfr) + } else { + klog.InfoS("NodeFeature not in selected namespace", "namespace", nfr.Namespace, "name", nfr.Name) } }, DeleteFunc: func(obj interface{}) { nfr := obj.(*nfdv1alpha1.NodeFeature) klog.V(2).InfoS("NodeFeature deleted", "nodefeature", klog.KObj(nfr)) - c.updateOneNode("NodeFeature", nfr) - if !nfdApiControllerOptions.DisableNodeFeatureGroup { - c.updateAllNodeFeatureGroups() + if c.isNamespaceSelected(nfr.Namespace) { + c.updateOneNode("NodeFeature", nfr) + } else { + klog.InfoS("NodeFeature not in selected namespace", "namespace", nfr.Namespace, "name", nfr.Name) } }, }); err != nil { @@ -209,6 +214,37 @@ func (c *nfdController) updateOneNode(typ string, obj metav1.Object) { c.updateOneNodeChan <- nodeName } +func (c *nfdController) isNamespaceSelected(namespace string) bool { + // no namespace restrictions are made here + if c.nodeFeatureNamespaceSelector == nil { + return true + } + + labelMap, err := metav1.LabelSelectorAsSelector(c.nodeFeatureNamespaceSelector) + if err != nil { + klog.ErrorS(err, "failed to convert label selector to map", "selector", c.nodeFeatureNamespaceSelector) + return false + } + + listOptions := metav1.ListOptions{ + LabelSelector: labelMap.String(), + } + + namespaces, err := c.k8sClient.CoreV1().Namespaces().List(context.Background(), listOptions) + if err != nil { + klog.ErrorS(err, "failed to query namespaces", "listOptions", listOptions) + return false + } + + for _, ns := range namespaces.Items { + if ns.Name == namespace { + return true + } + } + + return false +} + func (c *nfdController) updateAllNodes() { select { case c.updateAllNodesChan <- struct{}{}: diff --git a/pkg/nfd-master/nfd-api-controller_test.go b/pkg/nfd-master/nfd-api-controller_test.go index 44153f41a..21401d96c 100644 --- a/pkg/nfd-master/nfd-api-controller_test.go +++ b/pkg/nfd-master/nfd-api-controller_test.go @@ -23,6 +23,8 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" nfdv1alpha1 "sigs.k8s.io/node-feature-discovery/api/nfd/v1alpha1" + fakeclient "k8s.io/client-go/kubernetes/fake" + corev1 "k8s.io/api/core/v1" ) func TestGetNodeNameForObj(t *testing.T) { @@ -42,3 +44,57 @@ func TestGetNodeNameForObj(t *testing.T) { assert.Nil(t, err) assert.Equal(t, n, "node-1") } + +func newTestNamespace(name string) *corev1.Namespace{ + return &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Labels: map[string]string{ + "name": name, + }, + }, + } +} + +func TestIsNamespaceAllowed(t *testing.T) { + fakeCli := fakeclient.NewSimpleClientset(newTestNamespace("fake")) + c := &nfdController{ + k8sClient: fakeCli, + } + + testcases := []struct { + name string + objectNamespace string + nodeFeatureNamespaceSelector *metav1.LabelSelector + expectedResult bool + }{ + { + name: "namespace not allowed", + objectNamespace: "random", + nodeFeatureNamespaceSelector: &metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{ + { + Key: "name", + Operator: metav1.LabelSelectorOpIn, + Values: []string{"fake"}, + }, + }, + }, + expectedResult: false, + }, + { + name: "namespace is allowed", + objectNamespace: "fake", + nodeFeatureNamespaceSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"name": "fake"}, + }, + expectedResult: false, + }, + } + + for _, tc := range testcases { + c.nodeFeatureNamespaceSelector = tc.nodeFeatureNamespaceSelector + res := c.isNamespaceSelected(tc.name) + assert.Equal(t, res, tc.expectedResult) + } +} diff --git a/pkg/nfd-master/nfd-master-internal_test.go b/pkg/nfd-master/nfd-master-internal_test.go index 498070a86..e6e73103e 100644 --- a/pkg/nfd-master/nfd-master-internal_test.go +++ b/pkg/nfd-master/nfd-master-internal_test.go @@ -509,15 +509,16 @@ 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", ""), @@ -525,9 +526,9 @@ func TestCreatePatches(t *testing.T) { 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"]), @@ -535,9 +536,9 @@ func TestCreatePatches(t *testing.T) { 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"]), @@ -547,6 +548,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)) + }) }) } @@ -896,3 +907,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)) + }) + }) + } +} diff --git a/pkg/nfd-master/nfd-master.go b/pkg/nfd-master/nfd-master.go index e34bb3e5f..b95685b3b 100644 --- a/pkg/nfd-master/nfd-master.go +++ b/pkg/nfd-master/nfd-master.go @@ -75,6 +75,16 @@ type ExtendedResources map[string]string // Annotations are used for NFD-related node metadata type Annotations map[string]string +// Restrictions contains the restrictions on the NF and NFR Crs +type Restrictions struct { + AllowedNamespaces *metav1.LabelSelector + MaxLabelsPerCR int + MaxTaintsPerCR int + MaxExtendedResourcesPerCR int + DenyNodeFeatureLabels bool + OverwriteLabels bool +} + // NFDConfig contains the configuration settings of NfdMaster. type NFDConfig struct { AutoDefaultNs bool @@ -88,6 +98,7 @@ type NFDConfig struct { LeaderElection LeaderElectionConfig NfdApiParallelism int Klog klogutils.KlogConfigOpts + Restrictions Restrictions } // LeaderElectionConfig contains the configuration for leader election @@ -273,6 +284,13 @@ func newDefaultConfig() *NFDConfig { RenewDeadline: utils.DurationVal{Duration: time.Duration(10) * time.Second}, }, Klog: make(map[string]string), + Restrictions: Restrictions{ + MaxLabelsPerCR: 0, + MaxTaintsPerCR: 0, + MaxExtendedResourcesPerCR: 0, + OverwriteLabels: true, + DenyNodeFeatureLabels: false, + }, } } @@ -620,7 +638,7 @@ func (m *nfdMaster) updateMasterNode() error { p := createPatches([]string{m.instanceAnnotation(nfdv1alpha1.MasterVersionAnnotation)}, node.Annotations, nil, - "/metadata/annotations") + "/metadata/annotations", true) err = patchNode(m.k8sClient, node.Name, p) if err != nil { @@ -661,6 +679,11 @@ func (m *nfdMaster) filterFeatureLabels(labels Labels, features *nfdv1alpha1.Fea } } + if m.config.Restrictions.MaxLabelsPerCR > 0 && len(outLabels) > m.config.Restrictions.MaxLabelsPerCR { + klog.InfoS("number of labels in the request exceeds the restriction maximum labels per CR, skipping") + outLabels = Labels{} + } + return outLabels, extendedResources } @@ -715,7 +738,7 @@ func getDynamicValue(value string, features *nfdv1alpha1.Features) (string, erro return element, nil } -func filterTaints(taints []corev1.Taint) []corev1.Taint { +func (m *nfdMaster) filterTaints(taints []corev1.Taint) []corev1.Taint { outTaints := []corev1.Taint{} for _, taint := range taints { @@ -726,6 +749,12 @@ func filterTaints(taints []corev1.Taint) []corev1.Taint { outTaints = append(outTaints, taint) } } + + if m.config.Restrictions.MaxTaintsPerCR > 0 && len(taints) > m.config.Restrictions.MaxTaintsPerCR { + klog.InfoS("number of taints in the request exceeds the restriction maximum taints per CR, skipping") + outTaints = []corev1.Taint{} + } + return outTaints } @@ -1031,13 +1060,24 @@ func (m *nfdMaster) refreshNodeFeatures(cli k8sclient.Interface, node *corev1.No maps.Copy(extendedResources, crExtendedResources) extendedResources = m.filterExtendedResources(features, extendedResources) + if m.config.Restrictions.MaxExtendedResourcesPerCR > 0 && len(extendedResources) > m.config.Restrictions.MaxExtendedResourcesPerCR { + klog.InfoS("number of extended resources in the request exceeds the restriction maximum extended resources per CR, skipping") + extendedResources = map[string]string{} + } + // Annotations annotations := m.filterFeatureAnnotations(crAnnotations) // Taints var taints []corev1.Taint if m.config.EnableTaints { - taints = filterTaints(crTaints) + taints = m.filterTaints(crTaints) + } + + // If we deny node feature labels, we'll empty the labels variable + if m.config.Restrictions.DenyNodeFeatureLabels { + klog.InfoS("node feature labels are denied, skipping...") + labels = map[string]string{} } if m.config.NoPublish { @@ -1114,7 +1154,7 @@ func setTaints(cli k8sclient.Interface, taints []corev1.Taint, node *corev1.Node newAnnotations[nfdv1alpha1.NodeTaintsAnnotation] = strings.Join(taintStrs, ",") } - patches := createPatches([]string{nfdv1alpha1.NodeTaintsAnnotation}, node.Annotations, newAnnotations, "/metadata/annotations") + patches := createPatches([]string{nfdv1alpha1.NodeTaintsAnnotation}, node.Annotations, newAnnotations, "/metadata/annotations", true) if len(patches) > 0 { if err := patchNode(cli, node.Name, patches); err != nil { return fmt.Errorf("error while patching node object: %w", err) @@ -1254,7 +1294,7 @@ func (m *nfdMaster) updateNodeObject(cli k8sclient.Interface, node *corev1.Node, // Create JSON patches for changes in labels and annotations oldLabels := stringToNsNames(node.Annotations[m.instanceAnnotation(nfdv1alpha1.FeatureLabelsAnnotation)], nfdv1alpha1.FeatureLabelNs) oldAnnotations := stringToNsNames(node.Annotations[m.instanceAnnotation(nfdv1alpha1.FeatureAnnotationsTrackingAnnotation)], nfdv1alpha1.FeatureAnnotationNs) - patches := createPatches(oldLabels, node.Labels, labels, "/metadata/labels") + patches := createPatches(oldLabels, node.Labels, labels, "/metadata/labels", m.config.Restrictions.OverwriteLabels) oldAnnotations = append(oldAnnotations, []string{ m.instanceAnnotation(nfdv1alpha1.FeatureLabelsAnnotation), m.instanceAnnotation(nfdv1alpha1.ExtendedResourceAnnotation), @@ -1262,7 +1302,7 @@ func (m *nfdMaster) updateNodeObject(cli k8sclient.Interface, node *corev1.Node, // Clean up deprecated/stale nfd version annotations m.instanceAnnotation(nfdv1alpha1.MasterVersionAnnotation), m.instanceAnnotation(nfdv1alpha1.WorkerVersionAnnotation)}...) - patches = append(patches, createPatches(oldAnnotations, node.Annotations, annotations, "/metadata/annotations")...) + patches = append(patches, createPatches(oldAnnotations, node.Annotations, annotations, "/metadata/annotations", true)...) // patch node status with extended resource changes statusPatches := m.createExtendedResourcePatches(node, extendedResources) @@ -1294,7 +1334,7 @@ func (m *nfdMaster) updateNodeObject(cli k8sclient.Interface, node *corev1.Node, } // createPatches is a generic helper that returns json patch operations to perform -func createPatches(removeKeys []string, oldItems map[string]string, newItems map[string]string, jsonPath string) []utils.JsonPatch { +func createPatches(removeKeys []string, oldItems map[string]string, newItems map[string]string, jsonPath string, overwrite bool) []utils.JsonPatch { patches := []utils.JsonPatch{} // Determine items to remove @@ -1309,7 +1349,7 @@ func createPatches(removeKeys []string, oldItems map[string]string, newItems map // Determine items to add or replace for key, newVal := range newItems { if oldVal, ok := oldItems[key]; ok { - if newVal != oldVal { + if newVal != oldVal && overwrite { patches = append(patches, utils.NewJsonPatch("replace", jsonPath, key, newVal)) } } else { @@ -1511,8 +1551,10 @@ func (m *nfdMaster) startNfdApiController() error { } klog.InfoS("starting the nfd api controller") m.nfdController, err = newNfdController(kubeconfig, nfdApiControllerOptions{ - DisableNodeFeature: !nfdfeatures.NFDFeatureGate.Enabled(nfdfeatures.NodeFeatureAPI), - ResyncPeriod: m.config.ResyncPeriod.Duration, + DisableNodeFeature: !nfdfeatures.NFDFeatureGate.Enabled(nfdfeatures.NodeFeatureAPI), + ResyncPeriod: m.config.ResyncPeriod.Duration, + K8sClient: m.k8sClient, + NodeFeatureNamespaceSelector: m.config.Restrictions.AllowedNamespaces, }) if err != nil { return fmt.Errorf("failed to initialize CRD controller: %w", err) diff --git a/test/e2e/node_feature_discovery_test.go b/test/e2e/node_feature_discovery_test.go index 38e0a49f0..dd692ce76 100644 --- a/test/e2e/node_feature_discovery_test.go +++ b/test/e2e/node_feature_discovery_test.go @@ -853,6 +853,227 @@ core: } eventuallyNonControlPlaneNodes(ctx, f.ClientSet).WithTimeout(1 * time.Minute).Should(MatchLabels(expectedLabels, nodes)) }) + + 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: + matchLabels: + kubernetes.io/metadata.name: "fake" +`) + _, 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()) + }) + }) + + Context("max labels, taints restrictions should be respected", 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", ` +enableTaints: true +restrictions: + maxLabelsPerCR: 1 + maxTaintsPerCR: 1 +`) + _, err := f.ClientSet.CoreV1().ConfigMaps(f.Namespace.Name).Create(ctx, cm, metav1.CreateOptions{}) + Expect(err).NotTo(HaveOccurred()) + }) + It("No labels or taints should be created", func(ctx context.Context) { + nodes, err := getNonControlPlaneNodes(ctx, f.ClientSet) + Expect(err).NotTo(HaveOccurred()) + + targetNodeName := nodes[0].Name + Expect(targetNodeName).ToNot(BeEmpty(), "No suitable worker node found") + + By("Creating nfd-worker config") + cm := testutils.NewConfigMap("nfd-worker-conf", "nfd-worker.conf", ` +core: + sleepInterval: "1s" + featureSources: ["fake"] + labelSources: [] +`) + cm, err = f.ClientSet.CoreV1().ConfigMaps(f.Namespace.Name).Create(ctx, cm, metav1.CreateOptions{}) + Expect(err).NotTo(HaveOccurred()) + By("Creating nfd-worker daemonset") + podSpecOpts := createPodSpecOpts( + testpod.SpecWithContainerImage(dockerImage()), + testpod.SpecWithConfigMap(cm.Name, "/etc/kubernetes/node-feature-discovery"), + ) + workerDS := testds.NFDWorker(podSpecOpts...) + workerDS, err = f.ClientSet.AppsV1().DaemonSets(f.Namespace.Name).Create(ctx, workerDS, metav1.CreateOptions{}) + Expect(err).NotTo(HaveOccurred()) + + By("Waiting for worker daemonset pods to be ready") + Expect(testpod.WaitForReady(ctx, f.ClientSet, f.Namespace.Name, workerDS.Spec.Template.Labels["name"], 2)).NotTo(HaveOccurred()) + + // Add features from NodeFeatureRule #3 + By("Creating NodeFeatureRules #3") + Expect(testutils.CreateNodeFeatureRulesFromFile(ctx, nfdClient, "nodefeaturerule-3.yaml")).NotTo(HaveOccurred()) + + By("Verifying node taints and annotation from NodeFeatureRules #3") + expectedLabels := map[string]k8sLabels{} + + expectedTaints := map[string][]corev1.Taint{} + + eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchLabels(expectedLabels, nodes)) + eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchTaints(expectedTaints, nodes)) + + By("Deleting NodeFeatureRule #3") + err = nfdClient.NfdV1alpha1().NodeFeatureRules().Delete(ctx, "e2e-test-3", metav1.DeleteOptions{}) + Expect(err).NotTo(HaveOccurred()) + By("Verifying taints from NodeFeatureRules #3 were removed") + expectedTaints["*"] = []corev1.Taint{} + eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchTaints(expectedTaints, nodes)) + }) + }) + + Context("max extended resources restriction should be respected", 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: + maxExtendedResourcesPerCR: 1 +`) + _, 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) { + nodes, err := getNonControlPlaneNodes(ctx, f.ClientSet) + Expect(err).NotTo(HaveOccurred()) + + targetNodeName := nodes[0].Name + Expect(targetNodeName).ToNot(BeEmpty(), "No suitable worker node found") + + By("Creating nfd-worker config") + cm := testutils.NewConfigMap("nfd-worker-conf", "nfd-worker.conf", ` +core: + sleepInterval: "1s" + featureSources: ["fake"] + labelSources: [] +`) + cm, err = f.ClientSet.CoreV1().ConfigMaps(f.Namespace.Name).Create(ctx, cm, metav1.CreateOptions{}) + Expect(err).NotTo(HaveOccurred()) + By("Creating nfd-worker daemonset") + podSpecOpts := createPodSpecOpts( + testpod.SpecWithContainerImage(dockerImage()), + testpod.SpecWithConfigMap(cm.Name, "/etc/kubernetes/node-feature-discovery"), + ) + workerDS := testds.NFDWorker(podSpecOpts...) + workerDS, err = f.ClientSet.AppsV1().DaemonSets(f.Namespace.Name).Create(ctx, workerDS, metav1.CreateOptions{}) + Expect(err).NotTo(HaveOccurred()) + + By("Waiting for worker daemonset pods to be ready") + Expect(testpod.WaitForReady(ctx, f.ClientSet, f.Namespace.Name, workerDS.Spec.Template.Labels["name"], 2)).NotTo(HaveOccurred()) + + expectedAnnotations := map[string]k8sAnnotations{ + "*": {}, + } + expectedCapacity := map[string]corev1.ResourceList{ + "*": {}, + } + + By("Creating NodeFeatureRules #4") + Expect(testutils.CreateNodeFeatureRulesFromFile(ctx, nfdClient, "nodefeaturerule-4.yaml")).NotTo(HaveOccurred()) + + By("Verifying node annotations from NodeFeatureRules #4") + eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchAnnotations(expectedAnnotations, nodes)) + + By("Verifying node status capacity from NodeFeatureRules #4") + eventuallyNonControlPlaneNodes(ctx, f.ClientSet).WithTimeout(1 * time.Minute).Should(MatchCapacity(expectedCapacity, nodes)) + + By("Deleting NodeFeatureRules #4") + err = nfdClient.NfdV1alpha1().NodeFeatureRules().Delete(ctx, "e2e-extened-resource-test", metav1.DeleteOptions{}) + Expect(err).NotTo(HaveOccurred()) + + By("Deleting nfd-worker daemonset") + err = f.ClientSet.AppsV1().DaemonSets(f.Namespace.Name).Delete(ctx, workerDS.Name, metav1.DeleteOptions{}) + Expect(err).NotTo(HaveOccurred()) + + if useNodeFeatureApi { + By("Verify that labels from nfd-worker are garbage-collected") + expectedLabels := map[string]k8sLabels{ + "*": {}, + } + eventuallyNonControlPlaneNodes(ctx, f.ClientSet).WithTimeout(1 * time.Minute).Should(MatchLabels(expectedLabels, nodes)) + } + }) + }) + + Context("deny node feature labels restriction should be respected", 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: + denyNodeFeatureLabels: true +`) + _, err := f.ClientSet.CoreV1().ConfigMaps(f.Namespace.Name).Create(ctx, cm, metav1.CreateOptions{}) + Expect(err).NotTo(HaveOccurred()) + }) + It("No labels 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") + + 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()) + }) + }) }) // Test NodeFeatureGroups From 925a071595b16860543575ab606e562a7aa735ee Mon Sep 17 00:00:00 2001 From: AhmedGrati Date: Fri, 16 Feb 2024 11:51:52 +0100 Subject: [PATCH 2/3] docs: add CR restrictions to the master configuration reference Signed-off-by: AhmedGrati --- .../master-configuration-reference.md | 112 ++++++++++++++++++ examples/nodefeature.yaml | 2 +- nfd-master.conf | 35 ++++++ 3 files changed, 148 insertions(+), 1 deletion(-) create mode 100644 nfd-master.conf diff --git a/docs/reference/master-configuration-reference.md b/docs/reference/master-configuration-reference.md index 863cb2e13..3f04cbc0f 100644 --- a/docs/reference/master-configuration-reference.md +++ b/docs/reference/master-configuration-reference.md @@ -338,3 +338,115 @@ Comma-separated list of `pattern=N` settings for file-filtered logging. Default: *empty* Run-time configurable: yes + + +## restrictions + +The following options specify the restrictions that can be applied by nfd-master +on the deployed Custom Resources in the cluster. + +### restrictions.allowedNamespaces + +The `allowedNamespaces` option specifies the NodeFeatures namespaces to watch. +To select the appropriate namespaces to watch, you can use the `metav1.LabelSelector` +as a type for this option. + +Default: all namespaces are allowed to be watched. + +Example: + +```yaml +restrictions: + allowedNamespaces: + matchLabels: + kubernetes.io/metadata.name: "node-feature-discovery" + matchExpressions: + - key: "kubernetes.io/metadata.name" + operator: "In" + values: + - "node-feature-discovery" +``` + +### restrictions.maxLabelsPerCR + +The `maxLabelsPerCR` option specifies the maximum number of labels that can +be generated by a single CustomResource. + +Default: no limit + +Example: + +```yaml +restrictions: + maxLabelsPerCR: 20 +``` + +### restrictions.maxTaintsPerCR + +The `maxTaintsPerCR` option specifies the maximum number of taints that can +be generated by a single CustomResource. + +Default: no limit + +Example: + +```yaml +restrictions: + maxTaintsPerCR: 10 +``` + +### restrictions.maxExtendedResourcesPerCR + +The `maxExtendedResourcesPerCR` option specifies the maximum number of extended +resources that can be generated by a single CustomResource. + +Default: no limit + +Example: + +```yaml +restrictions: + maxExtendedResourcesPerCR: 15 +``` + +### restrictions.maxExtendedResourcesPerCR + +The `maxExtendedResourcesPerCR` option specifies the maximum number of extended +resources that can be generated by a single CustomResource. + +Default: no limit + +Example: + +```yaml +restrictions: + maxExtendedResourcesPerCR: 15 +``` + +### restrictions.overwriteLabels + +The `overwriteLabels` option specifies whether to overwrite existing +labels, if there's an overlap, or not. + +Default: true + +Example: + +```yaml +restrictions: + overwriteLabels: false +``` + +### restrictions.denyNodeFeatureLabels + +The `denyNodeFeatureLabels` option specifies whether to deny labels from NodeFeature +objects or not. + +Default: false + +Example: + +```yaml +restrictions: + denyNodeFeatureLabels: true +``` diff --git a/examples/nodefeature.yaml b/examples/nodefeature.yaml index 7ef4dd56c..71078c9e2 100644 --- a/examples/nodefeature.yaml +++ b/examples/nodefeature.yaml @@ -4,7 +4,7 @@ apiVersion: nfd.k8s-sigs.io/v1alpha1 kind: NodeFeature metadata: labels: - nfd.node.kubernetes.io/node-name: example-node + nfd.node.kubernetes.io/node-name: nfd-control-plane name: example-node namespace: node-feature-discovery spec: diff --git a/nfd-master.conf b/nfd-master.conf new file mode 100644 index 000000000..bea5ccb56 --- /dev/null +++ b/nfd-master.conf @@ -0,0 +1,35 @@ +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: + matchLabels: + kubernetes.io/metadata.name: "node-feature-discovery" + matchExpressions: + - key: "kubernetes.io/metadata.name" + operator: "In" + values: + - "node-feature-discovery" +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 From 28b40c90b8eb17892bf578c44af3a65167efa7fa Mon Sep 17 00:00:00 2001 From: AhmedGrati Date: Sat, 10 Aug 2024 22:28:52 +0200 Subject: [PATCH 3/3] deploy: add CR restrictions to the helm config Signed-off-by: AhmedGrati Signed-off-by: AhmedThresh Signed-off-by: AhmedGrati Signed-off-by: AhmedThresh Signed-off-by: AhmedGrati Signed-off-by: AhmedThresh --- deployment/base/rbac/master-clusterrole.yaml | 7 + .../master-config/nfd-master.conf.example | 15 + .../templates/clusterrole.yaml | 7 + .../helm/node-feature-discovery/values.yaml | 15 + .../master-configuration-reference.md | 79 ++- examples/nodefeature.yaml | 2 +- go.mod | 2 - nfd-master.conf | 35 -- pkg/nfd-master/namespace-lister.go | 58 ++ pkg/nfd-master/nfd-api-controller.go | 66 +-- pkg/nfd-master/nfd-api-controller_test.go | 45 +- pkg/nfd-master/nfd-master-internal_test.go | 71 +-- pkg/nfd-master/nfd-master.go | 128 +++-- test/e2e/data/nodefeaturerule-6.yaml | 18 + test/e2e/node_feature_discovery_test.go | 498 ++++++++++-------- test/e2e/utils/rbac.go | 5 + 16 files changed, 583 insertions(+), 468 deletions(-) delete mode 100644 nfd-master.conf create mode 100644 pkg/nfd-master/namespace-lister.go create mode 100644 test/e2e/data/nodefeaturerule-6.yaml diff --git a/deployment/base/rbac/master-clusterrole.yaml b/deployment/base/rbac/master-clusterrole.yaml index 529f87e38..b87760d5a 100644 --- a/deployment/base/rbac/master-clusterrole.yaml +++ b/deployment/base/rbac/master-clusterrole.yaml @@ -3,6 +3,13 @@ kind: ClusterRole metadata: name: nfd-master rules: +- apiGroups: + - "" + resources: + - namespaces + verbs: + - watch + - list - apiGroups: - "" resources: diff --git a/deployment/components/master-config/nfd-master.conf.example b/deployment/components/master-config/nfd-master.conf.example index 8aa0cb29c..96dd6d130 100644 --- a/deployment/components/master-config/nfd-master.conf.example +++ b/deployment/components/master-config/nfd-master.conf.example @@ -6,6 +6,21 @@ # enableTaints: false # labelWhiteList: "foo" # resyncPeriod: "2h" +# restrictions: +# disableLabels: true +# disableTaints: true +# disableExtendedResources: true +# disableAnnotations: true +# allowOverwrite: false +# denyNodeFeatureLabels: true +# nodeFeatureNamespaceSelector: +# matchLabels: +# kubernetes.io/metadata.name: "node-feature-discovery" +# matchExpressions: +# - key: "kubernetes.io/metadata.name" +# operator: "In" +# values: +# - "node-feature-discovery" # klog: # addDirHeader: false # alsologtostderr: false diff --git a/deployment/helm/node-feature-discovery/templates/clusterrole.yaml b/deployment/helm/node-feature-discovery/templates/clusterrole.yaml index 958f043fe..0b76e9afa 100644 --- a/deployment/helm/node-feature-discovery/templates/clusterrole.yaml +++ b/deployment/helm/node-feature-discovery/templates/clusterrole.yaml @@ -6,6 +6,13 @@ metadata: labels: {{- include "node-feature-discovery.labels" . | nindent 4 }} rules: +- apiGroups: + - "" + resources: + - namespaces + verbs: + - watch + - list - apiGroups: - "" resources: diff --git a/deployment/helm/node-feature-discovery/values.yaml b/deployment/helm/node-feature-discovery/values.yaml index 524421a05..452bcf1ab 100644 --- a/deployment/helm/node-feature-discovery/values.yaml +++ b/deployment/helm/node-feature-discovery/values.yaml @@ -28,6 +28,21 @@ master: # enableTaints: false # labelWhiteList: "foo" # resyncPeriod: "2h" + # restrictions: + # disableLabels: true + # disableTaints: true + # disableExtendedResources: true + # disableAnnotations: true + # allowOverwrite: false + # denyNodeFeatureLabels: true + # nodeFeatureNamespaceSelector: + # matchLabels: + # kubernetes.io/metadata.name: "node-feature-discovery" + # matchExpressions: + # - key: "kubernetes.io/metadata.name" + # operator: "In" + # values: + # - "node-feature-discovery" # klog: # addDirHeader: false # alsologtostderr: false diff --git a/docs/reference/master-configuration-reference.md b/docs/reference/master-configuration-reference.md index 3f04cbc0f..b7a06fe0a 100644 --- a/docs/reference/master-configuration-reference.md +++ b/docs/reference/master-configuration-reference.md @@ -339,25 +339,24 @@ Default: *empty* Run-time configurable: yes +## restrictions (EXPERIMENTAL) -## restrictions +The following options specify the restrictions that can be applied by the +nfd-master on the deployed Custom Resources in the cluster. -The following options specify the restrictions that can be applied by nfd-master -on the deployed Custom Resources in the cluster. +### restrictions.nodeFeatureNamespaceSelector -### restrictions.allowedNamespaces +The `nodeFeatureNamespaceSelector` option specifies the NodeFeatures namespaces +to watch, which can be selected by using `metav1.LabelSelector` as a type for +this option. An empty value selects all namespaces to be watched. -The `allowedNamespaces` option specifies the NodeFeatures namespaces to watch. -To select the appropriate namespaces to watch, you can use the `metav1.LabelSelector` -as a type for this option. - -Default: all namespaces are allowed to be watched. +Default: *empty* Example: ```yaml restrictions: - allowedNamespaces: + nodeFeatureNamespaceSelector: matchLabels: kubernetes.io/metadata.name: "node-feature-discovery" matchExpressions: @@ -367,66 +366,56 @@ restrictions: - "node-feature-discovery" ``` -### restrictions.maxLabelsPerCR +### restrictions.disableLabels -The `maxLabelsPerCR` option specifies the maximum number of labels that can -be generated by a single CustomResource. +The `disableLabels` option controls whether to allow creation of node labels +from NodeFeature and NodeFeatureRule CRs or not. -Default: no limit +Default: false Example: ```yaml restrictions: - maxLabelsPerCR: 20 + disableLabels: true ``` -### restrictions.maxTaintsPerCR +### restrictions.disableExtendedResources -The `maxTaintsPerCR` option specifies the maximum number of taints that can -be generated by a single CustomResource. +The `disableExtendedResources` option controls whether to allow creation of +node extended resources from NodeFeatureRule CR or not. -Default: no limit +Default: false Example: ```yaml restrictions: - maxTaintsPerCR: 10 + disableExtendedResources: true ``` -### restrictions.maxExtendedResourcesPerCR +### restrictions.disableAnnotations -The `maxExtendedResourcesPerCR` option specifies the maximum number of extended -resources that can be generated by a single CustomResource. +he `disableAnnotations` option controls whether to allow creation of node annotations +from NodeFeatureRule CR or not. -Default: no limit +Default: false Example: ```yaml restrictions: - maxExtendedResourcesPerCR: 15 + disableAnnotations: true ``` -### restrictions.maxExtendedResourcesPerCR +### restrictions.allowOverwrite -The `maxExtendedResourcesPerCR` option specifies the maximum number of extended -resources that can be generated by a single CustomResource. - -Default: no limit - -Example: - -```yaml -restrictions: - maxExtendedResourcesPerCR: 15 -``` - -### restrictions.overwriteLabels - -The `overwriteLabels` option specifies whether to overwrite existing -labels, if there's an overlap, or not. +The `allowOverwrite` option controls whether NFD is allowed to overwrite and +take over management of existing node labels, annotations, and extended resources. +Labels, annotations and extended resources created by NFD itself are not affected +(overwrite cannot be disabled). NFD tracks the labels, annotations and extended +resources that it manages with specific +[node annotations](../get-started/introduction.md#node-annotations). Default: true @@ -434,13 +423,13 @@ Example: ```yaml restrictions: - overwriteLabels: false + allowOverwrite: false ``` ### restrictions.denyNodeFeatureLabels -The `denyNodeFeatureLabels` option specifies whether to deny labels from NodeFeature -objects or not. +The `denyNodeFeatureLabels` option specifies whether to deny labels from 3rd party +NodeFeature objects or not. NodeFeature objects created by nfd-worker are not affected. Default: false diff --git a/examples/nodefeature.yaml b/examples/nodefeature.yaml index 71078c9e2..7ef4dd56c 100644 --- a/examples/nodefeature.yaml +++ b/examples/nodefeature.yaml @@ -4,7 +4,7 @@ apiVersion: nfd.k8s-sigs.io/v1alpha1 kind: NodeFeature metadata: labels: - nfd.node.kubernetes.io/node-name: nfd-control-plane + nfd.node.kubernetes.io/node-name: example-node name: example-node namespace: node-feature-discovery spec: diff --git a/go.mod b/go.mod index be823b2d3..7c25f60f5 100644 --- a/go.mod +++ b/go.mod @@ -2,8 +2,6 @@ module sigs.k8s.io/node-feature-discovery go 1.22.2 -toolchain go1.22.0 - require ( github.com/fsnotify/fsnotify v1.7.0 github.com/golang/protobuf v1.5.4 diff --git a/nfd-master.conf b/nfd-master.conf deleted file mode 100644 index bea5ccb56..000000000 --- a/nfd-master.conf +++ /dev/null @@ -1,35 +0,0 @@ -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: - matchLabels: - kubernetes.io/metadata.name: "node-feature-discovery" - matchExpressions: - - key: "kubernetes.io/metadata.name" - operator: "In" - values: - - "node-feature-discovery" -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 diff --git a/pkg/nfd-master/namespace-lister.go b/pkg/nfd-master/namespace-lister.go new file mode 100644 index 000000000..ea8b97a4c --- /dev/null +++ b/pkg/nfd-master/namespace-lister.go @@ -0,0 +1,58 @@ +/* +Copyright 2024 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +package nfdmaster + +import ( + "time" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/client-go/informers" + k8sclient "k8s.io/client-go/kubernetes" + v1lister "k8s.io/client-go/listers/core/v1" +) + +// NamespaceLister lists kubernetes namespaces. +type NamespaceLister struct { + namespaceLister v1lister.NamespaceLister + labelsSelector labels.Selector + stopChan chan struct{} +} + +func newNamespaceLister(k8sClient k8sclient.Interface, labelsSelector labels.Selector) *NamespaceLister { + factory := informers.NewSharedInformerFactory(k8sClient, time.Hour) + namespaceLister := factory.Core().V1().Namespaces().Lister() + + stopChan := make(chan struct{}) + factory.Start(stopChan) // runs in background + factory.WaitForCacheSync(stopChan) + + return &NamespaceLister{ + namespaceLister: namespaceLister, + labelsSelector: labelsSelector, + stopChan: stopChan, + } +} + +// list returns all kubernetes namespaces. +func (lister *NamespaceLister) list() ([]*corev1.Namespace, error) { + return lister.namespaceLister.List(lister.labelsSelector) +} + +// stop closes the channel used by the lister +func (lister *NamespaceLister) stop() { + close(lister.stopChan) +} diff --git a/pkg/nfd-master/nfd-api-controller.go b/pkg/nfd-master/nfd-api-controller.go index 905a2ed0c..2b248c407 100644 --- a/pkg/nfd-master/nfd-api-controller.go +++ b/pkg/nfd-master/nfd-api-controller.go @@ -17,16 +17,17 @@ limitations under the License. package nfdmaster import ( - "context" "fmt" "time" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" utilruntime "k8s.io/apimachinery/pkg/util/runtime" + k8sclient "k8s.io/client-go/kubernetes" restclient "k8s.io/client-go/rest" "k8s.io/client-go/tools/cache" "k8s.io/klog/v2" + nfdclientset "sigs.k8s.io/node-feature-discovery/api/generated/clientset/versioned" nfdscheme "sigs.k8s.io/node-feature-discovery/api/generated/clientset/versioned/scheme" nfdinformers "sigs.k8s.io/node-feature-discovery/api/generated/informers/externalversions" nfdinformersv1alpha1 "sigs.k8s.io/node-feature-discovery/api/generated/informers/externalversions/nfd/v1alpha1" @@ -46,12 +47,16 @@ type nfdController struct { updateOneNodeChan chan string updateAllNodeFeatureGroupsChan chan struct{} updateNodeFeatureGroupChan chan string + + namespaceLister *NamespaceLister } type nfdApiControllerOptions struct { - DisableNodeFeature bool - DisableNodeFeatureGroup bool - ResyncPeriod time.Duration + DisableNodeFeature bool + DisableNodeFeatureGroup bool + ResyncPeriod time.Duration + K8sClient k8sclient.Interface + NodeFeatureNamespaceSelector *metav1.LabelSelector } func init() { @@ -65,12 +70,18 @@ func newNfdController(config *restclient.Config, nfdApiControllerOptions nfdApiC updateOneNodeChan: make(chan string), updateAllNodeFeatureGroupsChan: make(chan struct{}, 1), updateNodeFeatureGroupChan: make(chan string), - k8sClient: nfdApiControllerOptions.K8sClient, - nodeFeatureNamespaceSelector: nfdApiControllerOptions.NodeFeatureNamespaceSelector, + } + + if nfdApiControllerOptions.NodeFeatureNamespaceSelector != nil { + labelMap, err := metav1.LabelSelectorAsSelector(nfdApiControllerOptions.NodeFeatureNamespaceSelector) + if err != nil { + klog.ErrorS(err, "failed to convert label selector to map", "selector", nfdApiControllerOptions.NodeFeatureNamespaceSelector) + return nil, err + } + c.namespaceLister = newNamespaceLister(nfdApiControllerOptions.K8sClient, labelMap) } nfdClient := nfdclientset.NewForConfigOrDie(config) - klog.V(2).InfoS("initializing new NFD API controller", "options", utils.DelayedDumper(nfdApiControllerOptions)) informerFactory := nfdinformers.NewSharedInformerFactory(nfdClient, nfdApiControllerOptions.ResyncPeriod) @@ -94,25 +105,26 @@ func newNfdController(config *restclient.Config, nfdApiControllerOptions nfdApiC if c.isNamespaceSelected(nfr.Namespace) { c.updateOneNode("NodeFeature", nfr) } else { - klog.InfoS("NodeFeature not in selected namespace", "namespace", nfr.Namespace, "name", nfr.Name) + klog.V(2).InfoS("NodeFeature namespace is not selected, skipping", "nodefeature", klog.KObj(nfr)) + } + if !nfdApiControllerOptions.DisableNodeFeatureGroup { + c.updateAllNodeFeatureGroups() } }, UpdateFunc: func(oldObj, newObj interface{}) { nfr := newObj.(*nfdv1alpha1.NodeFeature) klog.V(2).InfoS("NodeFeature updated", "nodefeature", klog.KObj(nfr)) - if c.isNamespaceSelected(nfr.Namespace) { - c.updateOneNode("NodeFeature", nfr) - } else { - klog.InfoS("NodeFeature not in selected namespace", "namespace", nfr.Namespace, "name", nfr.Name) + c.updateOneNode("NodeFeature", nfr) + if !nfdApiControllerOptions.DisableNodeFeatureGroup { + c.updateAllNodeFeatureGroups() } }, DeleteFunc: func(obj interface{}) { nfr := obj.(*nfdv1alpha1.NodeFeature) klog.V(2).InfoS("NodeFeature deleted", "nodefeature", klog.KObj(nfr)) - if c.isNamespaceSelected(nfr.Namespace) { - c.updateOneNode("NodeFeature", nfr) - } else { - klog.InfoS("NodeFeature not in selected namespace", "namespace", nfr.Namespace, "name", nfr.Name) + c.updateOneNode("NodeFeature", nfr) + if !nfdApiControllerOptions.DisableNodeFeatureGroup { + c.updateAllNodeFeatureGroups() } }, }); err != nil { @@ -192,6 +204,7 @@ func newNfdController(config *restclient.Config, nfdApiControllerOptions nfdApiC func (c *nfdController) stop() { close(c.stopChan) + c.namespaceLister.stop() } func getNodeNameForObj(obj metav1.Object) (string, error) { @@ -215,28 +228,19 @@ func (c *nfdController) updateOneNode(typ string, obj metav1.Object) { } func (c *nfdController) isNamespaceSelected(namespace string) bool { - // no namespace restrictions are made here - if c.nodeFeatureNamespaceSelector == nil { + // this means that the user didn't specify any namespace selector + // which means that we allow all namespaces + if c.namespaceLister == nil { return true } - labelMap, err := metav1.LabelSelectorAsSelector(c.nodeFeatureNamespaceSelector) + namespaces, err := c.namespaceLister.list() if err != nil { - klog.ErrorS(err, "failed to convert label selector to map", "selector", c.nodeFeatureNamespaceSelector) + klog.ErrorS(err, "failed to query namespaces by the namespace lister") return false } - listOptions := metav1.ListOptions{ - LabelSelector: labelMap.String(), - } - - namespaces, err := c.k8sClient.CoreV1().Namespaces().List(context.Background(), listOptions) - if err != nil { - klog.ErrorS(err, "failed to query namespaces", "listOptions", listOptions) - return false - } - - for _, ns := range namespaces.Items { + for _, ns := range namespaces { if ns.Name == namespace { return true } diff --git a/pkg/nfd-master/nfd-api-controller_test.go b/pkg/nfd-master/nfd-api-controller_test.go index 21401d96c..a49c4939b 100644 --- a/pkg/nfd-master/nfd-api-controller_test.go +++ b/pkg/nfd-master/nfd-api-controller_test.go @@ -20,11 +20,13 @@ import ( "testing" "github.com/stretchr/testify/assert" - - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - nfdv1alpha1 "sigs.k8s.io/node-feature-discovery/api/nfd/v1alpha1" - fakeclient "k8s.io/client-go/kubernetes/fake" corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/watch" + fakeclient "k8s.io/client-go/kubernetes/fake" + clienttesting "k8s.io/client-go/testing" + + nfdv1alpha1 "sigs.k8s.io/node-feature-discovery/api/nfd/v1alpha1" ) func TestGetNodeNameForObj(t *testing.T) { @@ -45,7 +47,7 @@ func TestGetNodeNameForObj(t *testing.T) { assert.Equal(t, n, "node-1") } -func newTestNamespace(name string) *corev1.Namespace{ +func newTestNamespace(name string) *corev1.Namespace { return &corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{ Name: name, @@ -56,11 +58,19 @@ func newTestNamespace(name string) *corev1.Namespace{ } } -func TestIsNamespaceAllowed(t *testing.T) { +func TestIsNamespaceSelected(t *testing.T) { fakeCli := fakeclient.NewSimpleClientset(newTestNamespace("fake")) - c := &nfdController{ - k8sClient: fakeCli, - } + fakeCli.PrependWatchReactor("*", func(action clienttesting.Action) (handled bool, ret watch.Interface, err error) { + gvr := action.GetResource() + ns := action.GetNamespace() + watch, err := fakeCli.Tracker().Watch(gvr, ns) + if err != nil { + return false, nil, err + } + return true, watch, nil + }) + + c := &nfdController{} testcases := []struct { name string @@ -69,8 +79,8 @@ func TestIsNamespaceAllowed(t *testing.T) { expectedResult bool }{ { - name: "namespace not allowed", - objectNamespace: "random", + name: "namespace not selected", + objectNamespace: "random", nodeFeatureNamespaceSelector: &metav1.LabelSelector{ MatchExpressions: []metav1.LabelSelectorRequirement{ { @@ -80,21 +90,22 @@ func TestIsNamespaceAllowed(t *testing.T) { }, }, }, - expectedResult: false, + expectedResult: false, }, { - name: "namespace is allowed", - objectNamespace: "fake", + name: "namespace is selected", + objectNamespace: "fake", nodeFeatureNamespaceSelector: &metav1.LabelSelector{ MatchLabels: map[string]string{"name": "fake"}, }, - expectedResult: false, + expectedResult: true, }, } for _, tc := range testcases { - c.nodeFeatureNamespaceSelector = tc.nodeFeatureNamespaceSelector - res := c.isNamespaceSelected(tc.name) + labelMap, _ := metav1.LabelSelectorAsSelector(tc.nodeFeatureNamespaceSelector) + c.namespaceLister = newNamespaceLister(fakeCli, labelMap) + res := c.isNamespaceSelected(tc.objectNamespace) assert.Equal(t, res, tc.expectedResult) } } diff --git a/pkg/nfd-master/nfd-master-internal_test.go b/pkg/nfd-master/nfd-master-internal_test.go index e6e73103e..42504a616 100644 --- a/pkg/nfd-master/nfd-master-internal_test.go +++ b/pkg/nfd-master/nfd-master-internal_test.go @@ -35,6 +35,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime" + "k8s.io/apimachinery/pkg/util/sets" fakeclient "k8s.io/client-go/kubernetes/fake" fakecorev1client "k8s.io/client-go/kubernetes/typed/core/v1/fake" clienttesting "k8s.io/client-go/testing" @@ -112,7 +113,7 @@ func withConfig(config *NFDConfig) NfdMasterOption { func newFakeMaster(opts ...NfdMasterOption) *nfdMaster { defaultOpts := []NfdMasterOption{ withNodeName(testNodeName), - withConfig(&NFDConfig{}), + withConfig(&NFDConfig{Restrictions: Restrictions{AllowOverwrite: true}}), WithKubernetesClient(fakeclient.NewSimpleClientset()), } m, err := NewNfdMaster(append(defaultOpts, opts...)...) @@ -513,12 +514,12 @@ func TestCreatePatches(t *testing.T) { jsonPath := "/root" 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) + p := createPatches(sets.New([]string{"foo", "bar"}...), existingItems, map[string]string{}, jsonPath, overwriteKeys) So(len(p), ShouldEqual, 0) }) 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) + p := createPatches(sets.New([]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", ""), @@ -528,7 +529,7 @@ func TestCreatePatches(t *testing.T) { 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, overwriteKeys) + p := createPatches(sets.New([]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"]), @@ -538,7 +539,7 @@ func TestCreatePatches(t *testing.T) { 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, overwriteKeys) + p := createPatches(sets.New([]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"]), @@ -552,9 +553,10 @@ func TestCreatePatches(t *testing.T) { 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) + p := createPatches(sets.New([]string{}...), existingItems, newItems, jsonPath, overwriteKeys) expected := []utils.JsonPatch{ utils.NewJsonPatch("add", jsonPath, "key-4", newItems["key-4"]), + utils.NewJsonPatch("replace", jsonPath, "key-1", newItems["key-1"]), } So(sortJsonPatches(p), ShouldResemble, sortJsonPatches(expected)) }) @@ -907,60 +909,3 @@ 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)) - }) - }) - } -} diff --git a/pkg/nfd-master/nfd-master.go b/pkg/nfd-master/nfd-master.go index b95685b3b..4a21ca3b5 100644 --- a/pkg/nfd-master/nfd-master.go +++ b/pkg/nfd-master/nfd-master.go @@ -45,14 +45,13 @@ import ( "k8s.io/apimachinery/pkg/labels" k8sLabels "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/sets" k8sclient "k8s.io/client-go/kubernetes" restclient "k8s.io/client-go/rest" "k8s.io/client-go/tools/leaderelection" "k8s.io/client-go/tools/leaderelection/resourcelock" "k8s.io/klog/v2" controller "k8s.io/kubernetes/pkg/controller" - klogutils "sigs.k8s.io/node-feature-discovery/pkg/utils/klog" - taintutils "k8s.io/kubernetes/pkg/util/taints" "sigs.k8s.io/yaml" @@ -63,6 +62,7 @@ import ( nfdfeatures "sigs.k8s.io/node-feature-discovery/pkg/features" pb "sigs.k8s.io/node-feature-discovery/pkg/labeler" "sigs.k8s.io/node-feature-discovery/pkg/utils" + klogutils "sigs.k8s.io/node-feature-discovery/pkg/utils/klog" "sigs.k8s.io/node-feature-discovery/pkg/version" ) @@ -77,12 +77,12 @@ type Annotations map[string]string // Restrictions contains the restrictions on the NF and NFR Crs type Restrictions struct { - AllowedNamespaces *metav1.LabelSelector - MaxLabelsPerCR int - MaxTaintsPerCR int - MaxExtendedResourcesPerCR int - DenyNodeFeatureLabels bool - OverwriteLabels bool + NodeFeatureNamespaceSelector *metav1.LabelSelector + DisableLabels bool + DisableExtendedResources bool + DisableAnnotations bool + DenyNodeFeatureLabels bool + AllowOverwrite bool } // NFDConfig contains the configuration settings of NfdMaster. @@ -285,11 +285,11 @@ func newDefaultConfig() *NFDConfig { }, Klog: make(map[string]string), Restrictions: Restrictions{ - MaxLabelsPerCR: 0, - MaxTaintsPerCR: 0, - MaxExtendedResourcesPerCR: 0, - OverwriteLabels: true, - DenyNodeFeatureLabels: false, + DisableLabels: false, + DisableExtendedResources: false, + DisableAnnotations: false, + AllowOverwrite: true, + DenyNodeFeatureLabels: false, }, } } @@ -635,10 +635,10 @@ func (m *nfdMaster) updateMasterNode() error { } // Advertise NFD version as an annotation - p := createPatches([]string{m.instanceAnnotation(nfdv1alpha1.MasterVersionAnnotation)}, + p := createPatches(sets.New([]string{m.instanceAnnotation(nfdv1alpha1.MasterVersionAnnotation)}...), node.Annotations, nil, - "/metadata/annotations", true) + "/metadata/annotations", m.config.Restrictions.AllowOverwrite) err = patchNode(m.k8sClient, node.Name, p) if err != nil { @@ -679,8 +679,8 @@ func (m *nfdMaster) filterFeatureLabels(labels Labels, features *nfdv1alpha1.Fea } } - if m.config.Restrictions.MaxLabelsPerCR > 0 && len(outLabels) > m.config.Restrictions.MaxLabelsPerCR { - klog.InfoS("number of labels in the request exceeds the restriction maximum labels per CR, skipping") + if len(outLabels) > 0 && m.config.Restrictions.DisableLabels { + klog.V(2).InfoS("node labels are disabled in configuration (restrictions.disableLabels=true)") outLabels = Labels{} } @@ -738,7 +738,7 @@ func getDynamicValue(value string, features *nfdv1alpha1.Features) (string, erro return element, nil } -func (m *nfdMaster) filterTaints(taints []corev1.Taint) []corev1.Taint { +func filterTaints(taints []corev1.Taint) []corev1.Taint { outTaints := []corev1.Taint{} for _, taint := range taints { @@ -750,11 +750,6 @@ func (m *nfdMaster) filterTaints(taints []corev1.Taint) []corev1.Taint { } } - if m.config.Restrictions.MaxTaintsPerCR > 0 && len(taints) > m.config.Restrictions.MaxTaintsPerCR { - klog.InfoS("number of taints in the request exceeds the restriction maximum taints per CR, skipping") - outTaints = []corev1.Taint{} - } - return outTaints } @@ -846,42 +841,62 @@ func (m *nfdMaster) getAndMergeNodeFeatures(nodeName string) (*nfdv1alpha1.NodeF return &nfdv1alpha1.NodeFeature{}, fmt.Errorf("failed to get NodeFeature resources for node %q: %w", nodeName, err) } + filteredObjs := []*nfdv1alpha1.NodeFeature{} + for _, obj := range objs { + if m.isNamespaceSelected(obj.Namespace) { + filteredObjs = append(filteredObjs, obj) + } + } + // Node without a running NFD-Worker - if len(objs) == 0 { + if len(filteredObjs) == 0 { return &nfdv1alpha1.NodeFeature{}, nil } // Sort our objects - sort.Slice(objs, func(i, j int) bool { + sort.Slice(filteredObjs, func(i, j int) bool { // Objects in our nfd namespace gets into the beginning of the list - if objs[i].Namespace == m.namespace && objs[j].Namespace != m.namespace { + if filteredObjs[i].Namespace == m.namespace && filteredObjs[j].Namespace != m.namespace { return true } - if objs[i].Namespace != m.namespace && objs[j].Namespace == m.namespace { + if filteredObjs[i].Namespace != m.namespace && filteredObjs[j].Namespace == m.namespace { return false } // After the nfd namespace, sort objects by their name - if objs[i].Name != objs[j].Name { - return objs[i].Name < objs[j].Name + if filteredObjs[i].Name != filteredObjs[j].Name { + return filteredObjs[i].Name < filteredObjs[j].Name } // Objects with the same name are sorted by their namespace - return objs[i].Namespace < objs[j].Namespace + return filteredObjs[i].Namespace < filteredObjs[j].Namespace }) - if len(objs) > 0 { + if len(filteredObjs) > 0 { // Merge in features // // NOTE: changing the rule api to support handle multiple objects instead // of merging would probably perform better with lot less data to copy. - features := objs[0].Spec.DeepCopy() + features := filteredObjs[0].Spec.DeepCopy() + + if m.config.Restrictions.DenyNodeFeatureLabels && m.isThirdPartyNodeFeature(*filteredObjs[0], nodeName, m.namespace) { + klog.V(2).InfoS("node feature labels are disabled in configuration (restrictions.denyNodeFeatureLabels=true)") + features.Labels = nil + } + if !nfdfeatures.NFDFeatureGate.Enabled(nfdfeatures.DisableAutoPrefix) && m.config.AutoDefaultNs { features.Labels = addNsToMapKeys(features.Labels, nfdv1alpha1.FeatureLabelNs) } - for _, o := range objs[1:] { + + for _, o := range filteredObjs[1:] { s := o.Spec.DeepCopy() + if m.config.Restrictions.DenyNodeFeatureLabels && m.isThirdPartyNodeFeature(*o, nodeName, m.namespace) { + klog.V(2).InfoS("node feature labels are disabled in configuration (restrictions.denyNodeFeatureLabels=true)") + s.Labels = nil + } + if !nfdfeatures.NFDFeatureGate.Enabled(nfdfeatures.DisableAutoPrefix) && m.config.AutoDefaultNs { s.Labels = addNsToMapKeys(s.Labels, nfdv1alpha1.FeatureLabelNs) } + s.MergeInto(features) } @@ -894,6 +909,11 @@ func (m *nfdMaster) getAndMergeNodeFeatures(nodeName string) (*nfdv1alpha1.NodeF return nodeFeatures, nil } +// isThirdPartyNodeFeature determines whether a node feature is a third party one or created by nfd-worker +func (m *nfdMaster) isThirdPartyNodeFeature(nodeFeature nfdv1alpha1.NodeFeature, nodeName, namespace string) bool { + return nodeFeature.Namespace != namespace || nodeFeature.Name != nodeName +} + func (m *nfdMaster) nfdAPIUpdateOneNode(cli k8sclient.Interface, node *corev1.Node) error { if m.nfdController == nil || m.nfdController.featureLister == nil { return nil @@ -1060,8 +1080,8 @@ func (m *nfdMaster) refreshNodeFeatures(cli k8sclient.Interface, node *corev1.No maps.Copy(extendedResources, crExtendedResources) extendedResources = m.filterExtendedResources(features, extendedResources) - if m.config.Restrictions.MaxExtendedResourcesPerCR > 0 && len(extendedResources) > m.config.Restrictions.MaxExtendedResourcesPerCR { - klog.InfoS("number of extended resources in the request exceeds the restriction maximum extended resources per CR, skipping") + if len(extendedResources) > 0 && m.config.Restrictions.DisableExtendedResources { + klog.V(2).InfoS("extended resources are disabled in configuration (restrictions.disableExtendedResources=true)") extendedResources = map[string]string{} } @@ -1071,13 +1091,7 @@ func (m *nfdMaster) refreshNodeFeatures(cli k8sclient.Interface, node *corev1.No // Taints var taints []corev1.Taint if m.config.EnableTaints { - taints = m.filterTaints(crTaints) - } - - // If we deny node feature labels, we'll empty the labels variable - if m.config.Restrictions.DenyNodeFeatureLabels { - klog.InfoS("node feature labels are denied, skipping...") - labels = map[string]string{} + taints = filterTaints(crTaints) } if m.config.NoPublish { @@ -1097,8 +1111,8 @@ func (m *nfdMaster) refreshNodeFeatures(cli k8sclient.Interface, node *corev1.No // setTaints sets node taints and annotations based on the taints passed via // nodeFeatureRule custom resorce. If empty list of taints is passed, currently // NFD owned taints and annotations are removed from the node. -func setTaints(cli k8sclient.Interface, taints []corev1.Taint, node *corev1.Node) error { - // De-serialize the taints annotation into corev1.Taint type for comparison below. +func (m *nfdMaster) setTaints(cli k8sclient.Interface, taints []corev1.Taint, node *corev1.Node) error { + // De-serialize the taints annotation into corev1.Taint type for comparision below. var err error oldTaints := []corev1.Taint{} if val, ok := node.Annotations[nfdv1alpha1.NodeTaintsAnnotation]; ok { @@ -1154,7 +1168,11 @@ func setTaints(cli k8sclient.Interface, taints []corev1.Taint, node *corev1.Node newAnnotations[nfdv1alpha1.NodeTaintsAnnotation] = strings.Join(taintStrs, ",") } - patches := createPatches([]string{nfdv1alpha1.NodeTaintsAnnotation}, node.Annotations, newAnnotations, "/metadata/annotations", true) + patches := createPatches(sets.New([]string{nfdv1alpha1.NodeTaintsAnnotation}...), + node.Annotations, newAnnotations, + "/metadata/annotations", + m.config.Restrictions.AllowOverwrite, + ) if len(patches) > 0 { if err := patchNode(cli, node.Name, patches); err != nil { return fmt.Errorf("error while patching node object: %w", err) @@ -1294,7 +1312,7 @@ func (m *nfdMaster) updateNodeObject(cli k8sclient.Interface, node *corev1.Node, // Create JSON patches for changes in labels and annotations oldLabels := stringToNsNames(node.Annotations[m.instanceAnnotation(nfdv1alpha1.FeatureLabelsAnnotation)], nfdv1alpha1.FeatureLabelNs) oldAnnotations := stringToNsNames(node.Annotations[m.instanceAnnotation(nfdv1alpha1.FeatureAnnotationsTrackingAnnotation)], nfdv1alpha1.FeatureAnnotationNs) - patches := createPatches(oldLabels, node.Labels, labels, "/metadata/labels", m.config.Restrictions.OverwriteLabels) + patches := createPatches(sets.New(oldLabels...), node.Labels, labels, "/metadata/labels", m.config.Restrictions.AllowOverwrite) oldAnnotations = append(oldAnnotations, []string{ m.instanceAnnotation(nfdv1alpha1.FeatureLabelsAnnotation), m.instanceAnnotation(nfdv1alpha1.ExtendedResourceAnnotation), @@ -1302,7 +1320,7 @@ func (m *nfdMaster) updateNodeObject(cli k8sclient.Interface, node *corev1.Node, // Clean up deprecated/stale nfd version annotations m.instanceAnnotation(nfdv1alpha1.MasterVersionAnnotation), m.instanceAnnotation(nfdv1alpha1.WorkerVersionAnnotation)}...) - patches = append(patches, createPatches(oldAnnotations, node.Annotations, annotations, "/metadata/annotations", true)...) + patches = append(patches, createPatches(sets.New(oldAnnotations...), node.Annotations, annotations, "/metadata/annotations", m.config.Restrictions.AllowOverwrite)...) // patch node status with extended resource changes statusPatches := m.createExtendedResourcePatches(node, extendedResources) @@ -1325,7 +1343,7 @@ func (m *nfdMaster) updateNodeObject(cli k8sclient.Interface, node *corev1.Node, } // Set taints - err = setTaints(cli, taints, node) + err = m.setTaints(cli, taints, node) if err != nil { return err } @@ -1334,11 +1352,11 @@ func (m *nfdMaster) updateNodeObject(cli k8sclient.Interface, node *corev1.Node, } // createPatches is a generic helper that returns json patch operations to perform -func createPatches(removeKeys []string, oldItems map[string]string, newItems map[string]string, jsonPath string, overwrite bool) []utils.JsonPatch { +func createPatches(removeKeys sets.Set[string], oldItems map[string]string, newItems map[string]string, jsonPath string, overwrite bool) []utils.JsonPatch { patches := []utils.JsonPatch{} // Determine items to remove - for _, key := range removeKeys { + for key := range removeKeys { if _, ok := oldItems[key]; ok { if _, ok := newItems[key]; !ok { patches = append(patches, utils.NewJsonPatch("remove", jsonPath, key, "")) @@ -1349,7 +1367,7 @@ func createPatches(removeKeys []string, oldItems map[string]string, newItems map // Determine items to add or replace for key, newVal := range newItems { if oldVal, ok := oldItems[key]; ok { - if newVal != oldVal && overwrite { + if newVal != oldVal && (!removeKeys.Has(key) || overwrite) { patches = append(patches, utils.NewJsonPatch("replace", jsonPath, key, newVal)) } } else { @@ -1554,7 +1572,7 @@ func (m *nfdMaster) startNfdApiController() error { DisableNodeFeature: !nfdfeatures.NFDFeatureGate.Enabled(nfdfeatures.NodeFeatureAPI), ResyncPeriod: m.config.ResyncPeriod.Duration, K8sClient: m.k8sClient, - NodeFeatureNamespaceSelector: m.config.Restrictions.AllowedNamespaces, + NodeFeatureNamespaceSelector: m.config.Restrictions.NodeFeatureNamespaceSelector, }) if err != nil { return fmt.Errorf("failed to initialize CRD controller: %w", err) @@ -1615,6 +1633,12 @@ func (m *nfdMaster) filterFeatureAnnotations(annotations map[string]string) map[ outAnnotations[annotation] = value } + + if len(outAnnotations) > 0 && m.config.Restrictions.DisableAnnotations { + klog.V(2).InfoS("node annotations are disabled in configuration (restrictions.disableAnnotations=true)") + outAnnotations = map[string]string{} + } + return outAnnotations } diff --git a/test/e2e/data/nodefeaturerule-6.yaml b/test/e2e/data/nodefeaturerule-6.yaml new file mode 100644 index 000000000..d9dcb3836 --- /dev/null +++ b/test/e2e/data/nodefeaturerule-6.yaml @@ -0,0 +1,18 @@ +apiVersion: nfd.k8s-sigs.io/v1alpha1 +kind: NodeFeatureRule +metadata: + name: e2e-test-6 +spec: + rules: + - name: "e2e-restrictions-test-1" + taints: + - effect: PreferNoSchedule + key: "feature.node.kubernetes.io/fake-special-cpu" + value: "true" + labels: + e2e.feature.node.kubernetes.io/restricted-label-1: "true" + annotations: + e2e.feature.node.kubernetes.io/restricted-annoation-1: "yes" + extendedResources: + e2e.feature.node.kubernetes.io/restricted-er-1: "2" + matchFeatures: diff --git a/test/e2e/node_feature_discovery_test.go b/test/e2e/node_feature_discovery_test.go index dd692ce76..affd9d22b 100644 --- a/test/e2e/node_feature_discovery_test.go +++ b/test/e2e/node_feature_discovery_test.go @@ -28,7 +28,6 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - corev1 "k8s.io/api/core/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" extclient "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset" @@ -853,227 +852,6 @@ core: } eventuallyNonControlPlaneNodes(ctx, f.ClientSet).WithTimeout(1 * time.Minute).Should(MatchLabels(expectedLabels, nodes)) }) - - 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: - matchLabels: - kubernetes.io/metadata.name: "fake" -`) - _, 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()) - }) - }) - - Context("max labels, taints restrictions should be respected", 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", ` -enableTaints: true -restrictions: - maxLabelsPerCR: 1 - maxTaintsPerCR: 1 -`) - _, err := f.ClientSet.CoreV1().ConfigMaps(f.Namespace.Name).Create(ctx, cm, metav1.CreateOptions{}) - Expect(err).NotTo(HaveOccurred()) - }) - It("No labels or taints should be created", func(ctx context.Context) { - nodes, err := getNonControlPlaneNodes(ctx, f.ClientSet) - Expect(err).NotTo(HaveOccurred()) - - targetNodeName := nodes[0].Name - Expect(targetNodeName).ToNot(BeEmpty(), "No suitable worker node found") - - By("Creating nfd-worker config") - cm := testutils.NewConfigMap("nfd-worker-conf", "nfd-worker.conf", ` -core: - sleepInterval: "1s" - featureSources: ["fake"] - labelSources: [] -`) - cm, err = f.ClientSet.CoreV1().ConfigMaps(f.Namespace.Name).Create(ctx, cm, metav1.CreateOptions{}) - Expect(err).NotTo(HaveOccurred()) - By("Creating nfd-worker daemonset") - podSpecOpts := createPodSpecOpts( - testpod.SpecWithContainerImage(dockerImage()), - testpod.SpecWithConfigMap(cm.Name, "/etc/kubernetes/node-feature-discovery"), - ) - workerDS := testds.NFDWorker(podSpecOpts...) - workerDS, err = f.ClientSet.AppsV1().DaemonSets(f.Namespace.Name).Create(ctx, workerDS, metav1.CreateOptions{}) - Expect(err).NotTo(HaveOccurred()) - - By("Waiting for worker daemonset pods to be ready") - Expect(testpod.WaitForReady(ctx, f.ClientSet, f.Namespace.Name, workerDS.Spec.Template.Labels["name"], 2)).NotTo(HaveOccurred()) - - // Add features from NodeFeatureRule #3 - By("Creating NodeFeatureRules #3") - Expect(testutils.CreateNodeFeatureRulesFromFile(ctx, nfdClient, "nodefeaturerule-3.yaml")).NotTo(HaveOccurred()) - - By("Verifying node taints and annotation from NodeFeatureRules #3") - expectedLabels := map[string]k8sLabels{} - - expectedTaints := map[string][]corev1.Taint{} - - eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchLabels(expectedLabels, nodes)) - eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchTaints(expectedTaints, nodes)) - - By("Deleting NodeFeatureRule #3") - err = nfdClient.NfdV1alpha1().NodeFeatureRules().Delete(ctx, "e2e-test-3", metav1.DeleteOptions{}) - Expect(err).NotTo(HaveOccurred()) - By("Verifying taints from NodeFeatureRules #3 were removed") - expectedTaints["*"] = []corev1.Taint{} - eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchTaints(expectedTaints, nodes)) - }) - }) - - Context("max extended resources restriction should be respected", 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: - maxExtendedResourcesPerCR: 1 -`) - _, 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) { - nodes, err := getNonControlPlaneNodes(ctx, f.ClientSet) - Expect(err).NotTo(HaveOccurred()) - - targetNodeName := nodes[0].Name - Expect(targetNodeName).ToNot(BeEmpty(), "No suitable worker node found") - - By("Creating nfd-worker config") - cm := testutils.NewConfigMap("nfd-worker-conf", "nfd-worker.conf", ` -core: - sleepInterval: "1s" - featureSources: ["fake"] - labelSources: [] -`) - cm, err = f.ClientSet.CoreV1().ConfigMaps(f.Namespace.Name).Create(ctx, cm, metav1.CreateOptions{}) - Expect(err).NotTo(HaveOccurred()) - By("Creating nfd-worker daemonset") - podSpecOpts := createPodSpecOpts( - testpod.SpecWithContainerImage(dockerImage()), - testpod.SpecWithConfigMap(cm.Name, "/etc/kubernetes/node-feature-discovery"), - ) - workerDS := testds.NFDWorker(podSpecOpts...) - workerDS, err = f.ClientSet.AppsV1().DaemonSets(f.Namespace.Name).Create(ctx, workerDS, metav1.CreateOptions{}) - Expect(err).NotTo(HaveOccurred()) - - By("Waiting for worker daemonset pods to be ready") - Expect(testpod.WaitForReady(ctx, f.ClientSet, f.Namespace.Name, workerDS.Spec.Template.Labels["name"], 2)).NotTo(HaveOccurred()) - - expectedAnnotations := map[string]k8sAnnotations{ - "*": {}, - } - expectedCapacity := map[string]corev1.ResourceList{ - "*": {}, - } - - By("Creating NodeFeatureRules #4") - Expect(testutils.CreateNodeFeatureRulesFromFile(ctx, nfdClient, "nodefeaturerule-4.yaml")).NotTo(HaveOccurred()) - - By("Verifying node annotations from NodeFeatureRules #4") - eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchAnnotations(expectedAnnotations, nodes)) - - By("Verifying node status capacity from NodeFeatureRules #4") - eventuallyNonControlPlaneNodes(ctx, f.ClientSet).WithTimeout(1 * time.Minute).Should(MatchCapacity(expectedCapacity, nodes)) - - By("Deleting NodeFeatureRules #4") - err = nfdClient.NfdV1alpha1().NodeFeatureRules().Delete(ctx, "e2e-extened-resource-test", metav1.DeleteOptions{}) - Expect(err).NotTo(HaveOccurred()) - - By("Deleting nfd-worker daemonset") - err = f.ClientSet.AppsV1().DaemonSets(f.Namespace.Name).Delete(ctx, workerDS.Name, metav1.DeleteOptions{}) - Expect(err).NotTo(HaveOccurred()) - - if useNodeFeatureApi { - By("Verify that labels from nfd-worker are garbage-collected") - expectedLabels := map[string]k8sLabels{ - "*": {}, - } - eventuallyNonControlPlaneNodes(ctx, f.ClientSet).WithTimeout(1 * time.Minute).Should(MatchLabels(expectedLabels, nodes)) - } - }) - }) - - Context("deny node feature labels restriction should be respected", 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: - denyNodeFeatureLabels: true -`) - _, err := f.ClientSet.CoreV1().ConfigMaps(f.Namespace.Name).Create(ctx, cm, metav1.CreateOptions{}) - Expect(err).NotTo(HaveOccurred()) - }) - It("No labels 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") - - 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()) - }) - }) }) // Test NodeFeatureGroups @@ -1239,6 +1017,282 @@ resyncPeriod: "1s" Expect(err).NotTo(HaveOccurred()) }) }) + + Context("selected namespaces restriction is respected or not", Label("restrictions"), 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: + nodeFeatureNamespaceSelector: + matchLabels: + e2etest: fake + +resyncPeriod: "1s" +`) + _, 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 + nodes, err := getNonControlPlaneNodes(ctx, f.ClientSet) + Expect(err).NotTo(HaveOccurred()) + + targetNodeName := nodes[0].Name + Expect(targetNodeName).ToNot(BeEmpty(), "No suitable worker node found") + + // label the namespace in which node feature object is created + // TODO(TessaIO): add a utility for this. + patches, err := json.Marshal( + []utils.JsonPatch{ + utils.NewJsonPatch( + "add", + "/metadata/labels", + "e2etest", + "fake", + ), + }, + ) + Expect(err).NotTo(HaveOccurred()) + + _, err = f.ClientSet.CoreV1().Namespaces().Patch(ctx, f.Namespace.Name, types.JSONPatchType, patches, metav1.PatchOptions{}) + Expect(err).NotTo(HaveOccurred()) + + // 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 created") + // No labels should be created since the f.Namespace is not in the selected Namespaces + expectedLabels := map[string]k8sLabels{ + targetNodeName: { + nfdv1alpha1.FeatureLabelNs + "/e2e-nodefeature-test-1": "obj-1", + nfdv1alpha1.FeatureLabelNs + "/e2e-nodefeature-test-2": "obj-1", + nfdv1alpha1.FeatureLabelNs + "/fake-fakefeature3": "overridden", + }, + } + eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchLabels(expectedLabels, nodes)) + + // remove label the namespace in which node feature object is created + patches, err = json.Marshal( + []utils.JsonPatch{ + utils.NewJsonPatch( + "remove", + "/metadata/labels", + "e2etest", + "fake", + ), + }, + ) + Expect(err).NotTo(HaveOccurred()) + + _, err = f.ClientSet.CoreV1().Namespaces().Patch(ctx, f.Namespace.Name, types.JSONPatchType, patches, metav1.PatchOptions{}) + 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 selected Namespaces + expectedLabels = map[string]k8sLabels{ + targetNodeName: {}, + } + 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()) + }) + }) + + Context("disable labels restrictions should be respected", Label("restrictions"), func() { + BeforeEach(func(ctx context.Context) { + extraMasterPodSpecOpts = []testpod.SpecOption{ + testpod.SpecWithConfigMap("nfd-master-conf", "/etc/kubernetes/node-feature-discovery"), + testpod.SpecWithContainerExtraArgs("-enable-taints"), + } + cm := testutils.NewConfigMap("nfd-master-conf", "nfd-master.conf", ` +restrictions: + disableLabels: true +`) + _, err := f.ClientSet.CoreV1().ConfigMaps(f.Namespace.Name).Create(ctx, cm, metav1.CreateOptions{}) + Expect(err).NotTo(HaveOccurred()) + }) + It("No labels should be created", func(ctx context.Context) { + // deploy node feature object + nodes, err := getNonControlPlaneNodes(ctx, f.ClientSet) + Expect(err).NotTo(HaveOccurred()) + + // Add features from NodeFeatureRule #6 + By("Creating NodeFeatureRules #6") + Expect(testutils.CreateNodeFeatureRulesFromFile(ctx, nfdClient, "nodefeaturerule-6.yaml")).NotTo(HaveOccurred()) + + By("Verifying node taints, annotations, ERs and labels from NodeFeatureRules #6") + expectedTaints := map[string][]corev1.Taint{ + "*": { + { + Key: "feature.node.kubernetes.io/fake-special-cpu", + Value: "true", + Effect: "PreferNoSchedule", + }, + }, + } + eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchTaints(expectedTaints, nodes)) + + expectedAnnotations := map[string]k8sAnnotations{ + "*": { + "e2e.feature.node.kubernetes.io/restricted-annoation-1": "yes", + "nfd.node.kubernetes.io/feature-annotations": "e2e.feature.node.kubernetes.io/restricted-annoation-1", + "nfd.node.kubernetes.io/extended-resources": "e2e.feature.node.kubernetes.io/restricted-er-1", + "nfd.node.kubernetes.io/taints": "feature.node.kubernetes.io/fake-special-cpu=true:PreferNoSchedule", + }, + } + eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchAnnotations(expectedAnnotations, nodes)) + + expectedCapacity := map[string]corev1.ResourceList{ + "*": { + "e2e.feature.node.kubernetes.io/restricted-er-1": resourcev1.MustParse("2"), + }, + } + eventuallyNonControlPlaneNodes(ctx, f.ClientSet).WithTimeout(1 * time.Minute).Should(MatchCapacity(expectedCapacity, nodes)) + + expectedLabels := map[string]k8sLabels{ + "*": {}, + } + eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchLabels(expectedLabels, nodes)) + + By("Deleting NodeFeatureRule #6") + err = nfdClient.NfdV1alpha1().NodeFeatureRules().Delete(ctx, "e2e-test-6", metav1.DeleteOptions{}) + Expect(err).NotTo(HaveOccurred()) + }) + }) + + Context("disable extended resources restriction should be respected", Label("restrictions"), 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: + disableExtendedResources: true +`) + _, err := f.ClientSet.CoreV1().ConfigMaps(f.Namespace.Name).Create(ctx, cm, metav1.CreateOptions{}) + Expect(err).NotTo(HaveOccurred()) + }) + It("Extended resources should not be created and Labels should be created", func(ctx context.Context) { + // deploy node feature object + nodes, err := getNonControlPlaneNodes(ctx, f.ClientSet) + Expect(err).NotTo(HaveOccurred()) + + targetNodeName := nodes[0].Name + Expect(targetNodeName).ToNot(BeEmpty(), "No suitable worker node found") + + expectedAnnotations := map[string]k8sAnnotations{ + "*": { + "e2e.feature.node.kubernetes.io/restricted-annoation-1": "yes", + "nfd.node.kubernetes.io/feature-annotations": "e2e.feature.node.kubernetes.io/restricted-annoation-1", + "nfd.node.kubernetes.io/feature-labels": "e2e.feature.node.kubernetes.io/restricted-label-1", + }, + } + expectedCapacity := map[string]corev1.ResourceList{ + "*": {}, + } + + expectedLabels := map[string]k8sLabels{ + "*": { + "e2e.feature.node.kubernetes.io/restricted-label-1": "true", + }, + } + + By("Creating NodeFeatureRules #6") + Expect(testutils.CreateNodeFeatureRulesFromFile(ctx, nfdClient, "nodefeaturerule-6.yaml")).NotTo(HaveOccurred()) + + By("Verifying node labels from NodeFeatureRules #6") + eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchLabels(expectedLabels, nodes)) + + By("Verifying node annotations from NodeFeatureRules #6") + eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchAnnotations(expectedAnnotations, nodes)) + + By("Verifying node status capacity from NodeFeatureRules #6") + eventuallyNonControlPlaneNodes(ctx, f.ClientSet).WithTimeout(1 * time.Minute).Should(MatchCapacity(expectedCapacity, nodes)) + + By("Deleting NodeFeatureRules #6") + err = nfdClient.NfdV1alpha1().NodeFeatureRules().Delete(ctx, "e2e-test-6", metav1.DeleteOptions{}) + Expect(err).NotTo(HaveOccurred()) + + By("Verify that labels from nfd-worker are garbage-collected") + expectedLabels = map[string]k8sLabels{ + "*": {}, + } + eventuallyNonControlPlaneNodes(ctx, f.ClientSet).WithTimeout(1 * time.Minute).Should(MatchLabels(expectedLabels, nodes)) + }) + }) + + Context("deny node feature labels restriction should be respected", Label("restrictions"), 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: + denyNodeFeatureLabels: true +`) + _, err := f.ClientSet.CoreV1().ConfigMaps(f.Namespace.Name).Create(ctx, cm, metav1.CreateOptions{}) + Expect(err).NotTo(HaveOccurred()) + }) + It("No feature labels should be created", func(ctx context.Context) { + // deploy node feature object + 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()) + + // Add features from NodeFeatureRule #6 + By("Creating NodeFeatureRules #6") + Expect(testutils.CreateNodeFeatureRulesFromFile(ctx, nfdClient, "nodefeaturerule-6.yaml")).NotTo(HaveOccurred()) + + By("Verifying node taints and labels from NodeFeatureRules #6") + expectedTaints := map[string][]corev1.Taint{ + "*": {}, + } + eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchTaints(expectedTaints, nodes)) + + expectedAnnotations := map[string]k8sAnnotations{ + "*": { + "e2e.feature.node.kubernetes.io/restricted-annoation-1": "yes", + "nfd.node.kubernetes.io/feature-annotations": "e2e.feature.node.kubernetes.io/restricted-annoation-1", + "nfd.node.kubernetes.io/extended-resources": "e2e.feature.node.kubernetes.io/restricted-er-1", + "nfd.node.kubernetes.io/feature-labels": "e2e.feature.node.kubernetes.io/restricted-label-1", + }, + } + eventuallyNonControlPlaneNodes(ctx, f.ClientSet).Should(MatchAnnotations(expectedAnnotations, nodes)) + + expectedCapacity := map[string]corev1.ResourceList{ + "*": { + "e2e.feature.node.kubernetes.io/restricted-er-1": resourcev1.MustParse("2"), + }, + } + eventuallyNonControlPlaneNodes(ctx, f.ClientSet).WithTimeout(1 * time.Minute).Should(MatchCapacity(expectedCapacity, nodes)) + + // TODO(TessaIO): we need one more test where we deploy nfd-worker that would create + // a non 3rd-party NF that shouldn't be ignored by this restriction + By("Verifying node labels from NodeFeature object #6 are not created") + expectedLabels := map[string]k8sLabels{ + "*": { + "e2e.feature.node.kubernetes.io/restricted-label-1": "true", + }, + } + 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()) + }) + }) }) }) diff --git a/test/e2e/utils/rbac.go b/test/e2e/utils/rbac.go index 67470e331..5e57fe4ec 100644 --- a/test/e2e/utils/rbac.go +++ b/test/e2e/utils/rbac.go @@ -176,6 +176,11 @@ func createClusterRoleMaster(ctx context.Context, cs clientset.Interface) (*rbac Name: "nfd-master-e2e", }, Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{""}, + Resources: []string{"namespaces"}, + Verbs: []string{"list", "watch"}, + }, { APIGroups: []string{""}, Resources: []string{"nodes", "nodes/status"},