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"},