1
0
Fork 0
mirror of https://github.com/kubernetes-sigs/node-feature-discovery.git synced 2024-12-14 11:57:51 +00:00

Option to stop implicitly adding default prefix to names

Add new autoDefaultNs (default is "true") config option to nfd-master.
Setting the config option to false stops NFD from automatically adding
the "feature.node.kubernetes.io/" prefix to labels, annotations and
extended resources. Taints are not affected as for them no prefix is
automatically added. The user-visible part of enabling the option change
is that NodeFeatureRules, local feature files, hooks and configuration
of the "custom" may need to be altereda (if the auto-prefixing is
relied on).

For now, the config option defaults to "true", meaning no change in
default behavior. However, the intent is to change the default to
"false" in a future release, deprecating the option and eventually
removing it (forcing it to "false").

The goal of stopping doing "auto-prefixing" is to simplify the operation
(of nfd and users). Make the naming more straightforward and easier to
understand and debug (kind of WYSIWYG), eliminating peculiar corner
cases:

1. Make validation simpler and unambiguous
2. Remove "overloading" of names, i.e. the mapping two values to the
   same actual name. E.g. previously something like

      labels:
        feature.node.kubernetes.io/foo: bar
        foo: baz

   Could actually result in node label:

     feature.node.kubernetes.io/foo: baz

3. Make the processing/usagee of the "rule.matched" and "local.labels"
   feature in NodeFeatureRules unambiguous and more understadable. E.g.
   previously you could have node label
   "feature.node.kubernetes.io/local-foo: bar" but in the NodeFeatureRule
   you'd need to use the unprefixed name "local-foo" or the fully
   prefixed name, depending on what was specified in the feature file (or
   hook) on the node(s).

NOTE: setting autoDefaultNs to false is a breaking change for users who
rely on automatic prefixing with the default feature.node.kubernetes.io/
namespace. NodeFeatureRules, feature files, hooks and custom rules
(configuration of the "custom" source of nfd-worker) will need to be
altered.  Unprefixed labels, annoations and extended resources will be
denied by nfd-master.
This commit is contained in:
Markus Lehtonen 2023-11-08 09:51:19 +02:00
parent 4e7f8b10be
commit 1d012a28cd
10 changed files with 245 additions and 118 deletions

View file

@ -1,4 +1,5 @@
# 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"]

View file

@ -82,7 +82,7 @@
# # The following feature demonstrates the capabilities of the matchFeatures
# - name: "my custom rule"
# labels:
# my-ng-feature: "true"
# "vendor.io/my-ng-feature": "true"
# # matchFeatures implements a logical AND over all matcher terms in the
# # list (i.e. all of the terms, or per-feature matchers, must match)
# matchFeatures:
@ -153,7 +153,7 @@
# # The following feature demonstrates the capabilities of the matchAny
# - name: "my matchAny rule"
# labels:
# my-ng-feature-2: "my-value"
# "vendor.io/my-ng-feature-2": "my-value"
# # matchAny implements a logical IF over all elements (sub-matchers) in
# # the list (i.e. at least one feature matcher must match)
# matchAny:
@ -177,7 +177,7 @@
# # The following features demonstreate label templating capabilities
# - name: "my template rule"
# labelsTemplate: |
# {{ range .system.osrelease }}my-system-feature.{{ .Name }}={{ .Value }}
# {{ range .system.osrelease }}vendor.io/my-system-feature.{{ .Name }}={{ .Value }}
# {{ end }}
# matchFeatures:
# - feature: system.osrelease
@ -187,7 +187,7 @@
#
# - name: "my template rule 2"
# labelsTemplate: |
# {{ range .pci.device }}my-pci-device.{{ .class }}-{{ .device }}=with-cpuid
# {{ range .pci.device }}vendor.io/my-pci-device.{{ .class }}-{{ .device }}=with-cpuid
# {{ end }}
# matchFeatures:
# - feature: pci.device
@ -202,7 +202,7 @@
# # previous labels and vars
# - name: "my dummy kernel rule"
# labels:
# "my.kernel.feature": "true"
# "vendor.io/my.kernel.feature": "true"
# matchFeatures:
# - feature: kernel.version
# matchExpressions:
@ -217,10 +217,10 @@
#
# - name: "my rule using backrefs"
# labels:
# "my.backref.feature": "true"
# "vendor.io/my.backref.feature": "true"
# matchFeatures:
# - feature: rule.matched
# matchExpressions:
# my.kernel.feature: {op: IsTrue}
# vendor.io/my.kernel.feature: {op: IsTrue}
# my.dummy.var: {op: Gt, value: ["0"]}
#

View file

@ -16,6 +16,7 @@ master:
enable: true
config: ### <NFD-MASTER-CONF-START-DO-NOT-REMOVE>
# 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"]
@ -219,7 +220,7 @@ worker:
# # The following feature demonstrates the capabilities of the matchFeatures
# - name: "my custom rule"
# labels:
# my-ng-feature: "true"
# "vendor.io/my-ng-feature": "true"
# # matchFeatures implements a logical AND over all matcher terms in the
# # list (i.e. all of the terms, or per-feature matchers, must match)
# matchFeatures:
@ -290,7 +291,7 @@ worker:
# # The following feature demonstrates the capabilities of the matchAny
# - name: "my matchAny rule"
# labels:
# my-ng-feature-2: "my-value"
# "vendor.io/my-ng-feature-2": "my-value"
# # matchAny implements a logical IF over all elements (sub-matchers) in
# # the list (i.e. at least one feature matcher must match)
# matchAny:
@ -314,7 +315,7 @@ worker:
# # The following features demonstreate label templating capabilities
# - name: "my template rule"
# labelsTemplate: |
# {{ range .system.osrelease }}my-system-feature.{{ .Name }}={{ .Value }}
# {{ range .system.osrelease }}vendor.io/my-system-feature.{{ .Name }}={{ .Value }}
# {{ end }}
# matchFeatures:
# - feature: system.osrelease
@ -324,7 +325,7 @@ worker:
#
# - name: "my template rule 2"
# labelsTemplate: |
# {{ range .pci.device }}my-pci-device.{{ .class }}-{{ .device }}=with-cpuid
# {{ range .pci.device }}vendor.io/my-pci-device.{{ .class }}-{{ .device }}=with-cpuid
# {{ end }}
# matchFeatures:
# - feature: pci.device
@ -339,7 +340,7 @@ worker:
# # previous labels and vars
# - name: "my dummy kernel rule"
# labels:
# "my.kernel.feature": "true"
# "vendor.io/my.kernel.feature": "true"
# matchFeatures:
# - feature: kernel.version
# matchExpressions:
@ -354,11 +355,11 @@ worker:
#
# - name: "my rule using backrefs"
# labels:
# "my.backref.feature": "true"
# "vendor.io/my.backref.feature": "true"
# matchFeatures:
# - feature: rule.matched
# matchExpressions:
# my.kernel.feature: {op: IsTrue}
# vendor.io/my.kernel.feature: {op: IsTrue}
# my.dummy.var: {op: Gt, value: ["0"]}
#
### <NFD-WORKER-CONF-END-DO-NOT-REMOVE>

View file

@ -70,6 +70,38 @@ Example:
denyLabelNs: ["denied.ns.io","denied.kubernetes.io"]
```
## autoDefaultNs
The `autoDefaultNs` option controls the automatic prefixing of names. When set
to true (the default in {{ site.version }}) nfd-master automatically adds the
default `feature.node.kubernetes.io/` prefix to unprefixed labels, annotations
and extended resources - this is also the default behavior in NFD v0.13 and
earlier. When the option is set to `false`, no prefix will be prepended to
unprefixed names, effectively causing them to be filtered out (as NFD does not
currently allow unprefixed names of labels, annotations or extended resources).
The default will be changed to `false` in a future release.
For example, with the `autoDefaultNs` set to `true`, a NodeFeatureRule with
```yaml
labels:
foo: bar
```
Will turn into `feature.node.kubernetes.io/foo=bar` node label. With
`autoDefaultNs` set to `false`, no prefix is added and the label will be
filtered out.
Note that taint keys are not affected by this option.
Default: `true`
Example:
```yaml
autoDefaultNs: false
```
## resourceLabels
**DEPRECATED**: [NodeFeatureRule](../usage/custom-resources.md#nodefeaturerule)

View file

@ -81,7 +81,7 @@ spec:
vendor: "acme"
# Labels to be created
labels:
vendor-feature.enabled: "true"
vendor.io/feature.enabled: "true"
```
The object targets node named `node-1`. It lists two "flag type" features under
@ -92,8 +92,7 @@ labels but they will be used as input when the
[`NodeFeatureRule`](#nodefeaturerule-custom-resource) objects are evaluated.
In addition, the example requests directly the
`feature.node.kubenernetes.io/vendor-feature.enabled=true` node label to be
created.
`vendor.io/feature.enabled=true` node label to be created.
The `nfd.node.kubernetes.io/node-name=<node-name>` must be in place for each
NodeFeature object as NFD uses it to determine the node which it is targeting.
@ -130,7 +129,7 @@ spec:
rules:
- name: "my sample rule"
labels:
"my-sample-feature": "true"
"feature.node.kubernetes.io/my-sample-feature": "true"
matchFeatures:
- feature: kernel.loadedmodule
matchExpressions:
@ -141,7 +140,7 @@ spec:
```
It specifies one rule which creates node label
`feature.node.kubenernetes.io/my-sample-feature=true` if both of the following
`feature.node.kubernetes.io/my-sample-feature=true` if both of the following
conditions are true (`matchFeatures` implements a logical AND over the
matchers):
@ -216,9 +215,9 @@ having the following contents (or alternatively a shell script
following stdout output):
```plaintext
my-feature.1
my-feature.2=myvalue
my.namespace/my-feature.3=456
feature.node.kubernetes.io/my-feature.1
feature.node.kubernetes.io/my-feature.2=myvalue
vendor.io/my-feature.3=456
```
This will translate into the following node labels:
@ -226,7 +225,7 @@ This will translate into the following node labels:
```yaml
feature.node.kubernetes.io/my-feature.1: "true"
feature.node.kubernetes.io/my-feature.2: "myvalue"
my.namespace/my-feature.3: "456"
vendor.io/my-feature.3: "456"
```
### Feature files
@ -277,12 +276,12 @@ key-value pairs, separated by newlines:
```plaintext
# This is a comment
<name>[=<value>]
<key>[=<value>]
```
The label value defaults to `true`, if not specified.
Label namespace may be specified with `<namespace>/<name>[=<value>]`.
Label namespace must be specified with `<namespace>/<name>[=<value>]`.
> **NOTE:** The feature file size limit it 64kB. The feature file will be
> ignored if the size limit is exceeded.
@ -302,20 +301,20 @@ Considering the following file:
```plaintext
# +expiry-time=2012-07-28T11:22:33Z
featureKey=featureValue
vendor.io/feature1=featureValue
# +expiry-time=2080-07-28T11:22:33Z
featureKey2=featureValue2
vendor.io/feature2=featureValue2
# +expiry-time=2070-07-28T11:22:33Z
featureKey3=featureValue3
vendor.io/feature3=featureValue3
# +expiry-time=2002-07-28T11:22:33Z
featureKey4=featureValue4
vendor.io/feature4=featureValue4
```
After processing the above file, only `featureKey2` and `featureKey3` would be
included in the list of accepted features.
After processing the above file, only `vendor.io/feature2` and
`vendor.io/feature3` would be included in the list of accepted features.
> **NOTE:** The time format that we are supporting is RFC3339. Also, the `expiry-time`
> tag is only evaluated in each re-discovery period, and the expiration of
@ -329,11 +328,12 @@ Considering the following file:
```plaintext
# +no-feature
label-only=value
vendor.io/label-only=value
my-feature=value
vendor.io/my-feature=value
vendor.io/foo=bar
foo=bar
# +no-label
foo=baz
```
@ -343,19 +343,27 @@ Processing the above file would result in the following Features:
```yaml
local.features:
foo: baz
my-feature: value
vendor.io/my-feature: value
local.labels:
label-only: value
my-feature: value
vendor.io/label-only: value
vendor.io/my-feature: value
```
and the following labels added to the Node:
```plaintext
feature.node.kubernetes.io/label-only=value
feature.node.kubernetes.io/my-feature=value
vendor.io/label-only=value
vendor.io/my-feature=value
```
> **NOTE:** use of unprefixed label names (like `foo=bar`) should not be used.
> In NFD {{ site.version }} unprefixed names will be automatically prefixed
> with `feature.node.kubernetes.io/` but this will change in a future version
> (see
> [autoDefaultNs config option](../reference/master-configuration-reference.md#autoDefaultNs).
> Unprefixed names for plain Features (tagged with `# +no-label`) can be used
> without restrictions, however.
### Mounts
The standard NFD deployments contain `hostPath` mounts for
@ -395,7 +403,7 @@ sources:
custom:
- name: "my sample rule"
labels:
"my-sample-feature": "true"
"feature.node.kubenernetes.io/my-sample-feature": "true"
matchFeatures:
- feature: kernel.loadedmodule
matchExpressions:
@ -434,7 +442,7 @@ following content:
```yaml
- name: "my e1000 rule"
labels:
"e1000.present": "true"
"feature.node.kubenernetes.io/e1000.present": "true"
matchFeatures:
- feature: kernel.loadedmodule
matchExpressions:
@ -459,8 +467,7 @@ Feature labels have the following format:
The namespace part (i.e. prefix) of the labels is controlled by nfd:
- All built-in labels use `feature.node.kubernetes.io`. This is also
the default for user defined features that don't specify any namespace.
- All built-in labels use `feature.node.kubernetes.io`.
- Namespaces may be excluded with the
[`-deny-label-ns`](../reference/master-commandline-reference.md#-deny-label-ns)
command line flag of nfd-master
@ -468,6 +475,9 @@ The namespace part (i.e. prefix) of the labels is controlled by nfd:
[`-extra-label-ns`](../reference/master-commandline-reference.md#-extra-label-ns)
command line flag of nfd-master.
e.g: `nfd-master -deny-label-ns="*" -extra-label-ns=example.com`
- Built-in default namespaces `feature.node.kubernetes.io` and
`profile.node.kubernetes.io` (and their sub-namespaces) are always allowed
and cannot be denied.
## Feature rule format
@ -485,7 +495,7 @@ Take this rule as a referential example:
```yaml
- name: "my feature rule"
labels:
"my-special-feature": "my-value"
"feature.node.kubernetes.io/my-special-feature": "my-value"
matchFeatures:
- feature: cpu.cpuid
matchExpressions:
@ -500,7 +510,7 @@ Take this rule as a referential example:
class: {op: In, value: ["0200"]}
```
This will yield `feature.node.kubenernetes.io/my-special-feature=my-value` node
This will yield `feature.node.kubernetes.io/my-special-feature=my-value` node
label if all of these are true (`matchFeatures` implements a logical AND over
the matchers):
@ -529,8 +539,8 @@ spec:
rules:
- name: "my dynamic label value rule"
labels:
linux-lsm-enabled: "@kernel.config.LSM"
custom-label: "customlabel"
feature.node.kubernetes.io/linux-lsm-enabled: "@kernel.config.LSM"
feature.node.kubernetes.io/custom-label: "customlabel"
```
Label `linux-lsm-enabled` uses the `@` notation for dynamic values.
@ -576,8 +586,7 @@ spec:
rules:
- name: "annotation-example"
annotations:
defaul-ns-annotation: "foo"
feature.node.kubernetes.io/defaul-ns-annotation-2: "bar"
feature.node.kubernetes.io/defaul-ns-annotation: "foo"
custom.vendor.io/feature: "baz"
matchFeatures:
- feature: kernel.version
@ -591,7 +600,6 @@ This will yield into the following node annotations:
annotations:
...
feature.node.kubernetes.io/defaul-ns-annotation: "foo"
feature.node.kubernetes.io/defaul-ns-annotation-2: "bar"
custom.vendor.io/feature: "baz"
...
```
@ -602,8 +610,10 @@ NFD enforces some limitations to the namespace (or prefix)/ of the annotations:
generally be used
- the only exception is `feature.node.kubernetes.io/` and its sub-namespaces
(like `sub.ns.feature.node.kubernetes.io`)
- unprefixed names will get prefixed with `feature.node.kubernetes.io/`
automatically (e.g. `foo` becomes `feature.node.kubernetes.io/foo`)
- unprefixed names (like `my-annotation`) should not be used. In NFD {{
site.version }} unprefixed names will be automatically prefixed with
`feature.node.kubernetes.io/` but this will change in a future version (see
[autoDefaultNs config option](../reference/master-configuration-reference.md#autoDefaultNs).
> **NOTE:** The `annotations` field has will only advertise features via node
> annotations the features won't be advertised as node labels unless they are
@ -719,8 +729,10 @@ Resources names:
generally be used
- the only exception is `feature.node.kubernetes.io/` and its sub-namespaces
(like `sub.ns.feature.node.kubernetes.io`)
- unprefixed names will get prefixed with `feature.node.kubernetes.io/`
automatically (e.g. `foo` becomes `feature.node.kubernetes.io/foo`)
- unprefixed names (like `my-er`) site.version }} unprefixed names will be
automatically prefixed with `feature.node.kubernetes.io/` but this will
change in a future version (see
[autoDefaultNs config option](../reference/master-configuration-reference.md#autoDefaultNs).
> **NOTE:** `.extendedResources` is not supported by the
> [custom feature source](#custom-feature-source) -- it can only be used in

View file

@ -6,7 +6,7 @@ spec:
rules:
- name: "my sample rule"
labels:
"my-sample-feature": "true"
"vendor.io/my-sample-feature": "true"
matchFeatures:
- feature: kernel.loadedmodule
matchExpressions:
@ -28,4 +28,4 @@ spec:
matchFeatures:
- feature: kernel.version
matchExpressions:
major: {op: Exists}
major: {op: Exists}

View file

@ -370,12 +370,12 @@ func TestSetLabels(t *testing.T) {
mockNode := newMockNode()
mockCtx := context.Background()
// In the gRPC request the label names may omit the default ns
mockLabels := map[string]string{"feature-1": "1", "feature-2": "val-2", "feature-3": "3"}
mockLabels := map[string]string{"feature.node.kubernetes.io/feature-1": "1", "example.io/feature-2": "val-2", "feature.node.kubernetes.io/feature-3": "3"}
mockReq := &labeler.SetLabelsRequest{NodeName: workerName, NfdVersion: workerVer, Labels: mockLabels}
mockLabelNames := make([]string, 0, len(mockLabels))
for k := range mockLabels {
mockLabelNames = append(mockLabelNames, k)
mockLabelNames = append(mockLabelNames, strings.TrimPrefix(k, nfdv1alpha1.FeatureLabelNs+"/"))
}
sort.Strings(mockLabelNames)
@ -386,7 +386,7 @@ func TestSetLabels(t *testing.T) {
apihelper.NewJsonPatch("add", "/metadata/annotations", nfdv1alpha1.FeatureLabelsAnnotation, strings.Join(mockLabelNames, ",")),
}
for k, v := range mockLabels {
expectedPatches = append(expectedPatches, apihelper.NewJsonPatch("add", "/metadata/labels", nfdv1alpha1.FeatureLabelNs+"/"+k, v))
expectedPatches = append(expectedPatches, apihelper.NewJsonPatch("add", "/metadata/labels", k, v))
}
mockHelper.On("GetClient").Return(mockClient, nil)
@ -401,8 +401,8 @@ func TestSetLabels(t *testing.T) {
Convey("When -label-whitelist is specified", func() {
expectedPatches := []apihelper.JsonPatch{
apihelper.NewJsonPatch("add", "/metadata/annotations", nfdv1alpha1.FeatureLabelsAnnotation, "feature-2"),
apihelper.NewJsonPatch("add", "/metadata/labels", nfdv1alpha1.FeatureLabelNs+"/feature-2", mockLabels["feature-2"]),
apihelper.NewJsonPatch("add", "/metadata/annotations", nfdv1alpha1.FeatureLabelsAnnotation, "example.io/feature-2"),
apihelper.NewJsonPatch("add", "/metadata/labels", "example.io/feature-2", mockLabels["example.io/feature-2"]),
}
mockMaster.config.LabelWhiteList.Regexp = *regexp.MustCompile("^f.*2$")
@ -460,13 +460,13 @@ func TestSetLabels(t *testing.T) {
Convey("When -resource-labels is specified", func() {
expectedPatches := []apihelper.JsonPatch{
apihelper.NewJsonPatch("add", "/metadata/annotations", nfdv1alpha1.FeatureLabelsAnnotation, "feature-2"),
apihelper.NewJsonPatch("add", "/metadata/annotations", nfdv1alpha1.FeatureLabelsAnnotation, "example.io/feature-2"),
apihelper.NewJsonPatch("add", "/metadata/annotations", nfdv1alpha1.ExtendedResourceAnnotation, "feature-1,feature-3"),
apihelper.NewJsonPatch("add", "/metadata/labels", nfdv1alpha1.FeatureLabelNs+"/feature-2", mockLabels["feature-2"]),
apihelper.NewJsonPatch("add", "/metadata/labels", "example.io/feature-2", mockLabels["example.io/feature-2"]),
}
expectedStatusPatches := []apihelper.JsonPatch{
apihelper.NewJsonPatch("add", "/status/capacity", nfdv1alpha1.FeatureLabelNs+"/feature-1", mockLabels["feature-1"]),
apihelper.NewJsonPatch("add", "/status/capacity", nfdv1alpha1.FeatureLabelNs+"/feature-3", mockLabels["feature-3"]),
apihelper.NewJsonPatch("add", "/status/capacity", "feature.node.kubernetes.io/feature-1", mockLabels["feature.node.kubernetes.io/feature-1"]),
apihelper.NewJsonPatch("add", "/status/capacity", "feature.node.kubernetes.io/feature-3", mockLabels["feature.node.kubernetes.io/feature-3"]),
}
mockMaster.config.ResourceLabels = map[string]struct{}{"feature.node.kubernetes.io/feature-3": {}, "feature-1": {}}
@ -502,29 +502,92 @@ func TestSetLabels(t *testing.T) {
func TestFilterLabels(t *testing.T) {
mockHelper := &apihelper.MockAPIHelpers{}
mockMaster := newMockMaster(mockHelper)
mockMaster.deniedNs = deniedNs{
normal: map[string]struct{}{"": struct{}{}, "kubernetes.io": struct{}{}, "denied.ns": struct{}{}},
wildcard: map[string]struct{}{".kubernetes.io": struct{}{}, ".denied.subns": struct{}{}},
}
Convey("When using dynamic values", t, func() {
labelName := "ns/testLabel"
labelValue := "@test.feature.LSM"
features := nfdv1alpha1.Features{
Attributes: map[string]nfdv1alpha1.AttributeFeatureSet{
"test.feature": nfdv1alpha1.AttributeFeatureSet{
Elements: map[string]string{
"LSM": "123",
type TC struct {
description string
labelName string
labelValue string
features nfdv1alpha1.Features
expectErr bool
expectedValue string
}
tcs := []TC{
TC{
description: "Static value",
labelName: "example.io/test",
labelValue: "test-val",
expectedValue: "test-val",
},
TC{
description: "Dynamic value",
labelName: "example.io/testLabel",
labelValue: "@test.feature.LSM",
features: nfdv1alpha1.Features{
Attributes: map[string]nfdv1alpha1.AttributeFeatureSet{
"test.feature": nfdv1alpha1.AttributeFeatureSet{
Elements: map[string]string{
"LSM": "123",
},
},
},
},
}
labelValue, err := mockMaster.filterFeatureLabel(labelName, labelValue, &features)
expectedValue: "123",
},
TC{
description: "Unprefixed should be denied",
labelName: "test-label",
labelValue: "test-value",
expectErr: true,
},
TC{
description: "kubernetes.io ns should be denied",
labelName: "kubernetes.io/test-label",
labelValue: "test-value",
expectErr: true,
},
TC{
description: "*.kubernetes.io ns should be denied",
labelName: "sub.ns.kubernetes.io/test-label",
labelValue: "test-value",
expectErr: true,
},
TC{
description: "denied.ns ns should be denied",
labelName: "denied.ns/test-label",
labelValue: "test-value",
expectErr: true,
},
TC{
description: "*.denied.subns ns should be denied",
labelName: "my.denied.subns/test-label",
labelValue: "test-value",
expectErr: true,
},
}
Convey("Operation should succeed", func() {
So(err, ShouldBeNil)
})
for _, tc := range tcs {
t.Run(tc.description, func(t *testing.T) {
labelValue, err := mockMaster.filterFeatureLabel(tc.labelName, tc.labelValue, &tc.features)
Convey("Label value should change", func() {
So(labelValue, ShouldEqual, "123")
if tc.expectErr {
Convey("Label should be filtered out", t, func() {
So(err, ShouldBeError)
})
} else {
Convey("Label should not be filtered out", t, func() {
So(err, ShouldBeNil)
})
Convey("Label value should be correct", t, func() {
So(labelValue, ShouldEqual, tc.expectedValue)
})
}
})
})
}
}
func TestCreatePatches(t *testing.T) {

View file

@ -71,6 +71,7 @@ type Annotations map[string]string
// NFDConfig contains the configuration settings of NfdMaster.
type NFDConfig struct {
AutoDefaultNs bool
DenyLabelNs utils.StringSetVal
ExtraLabelNs utils.StringSetVal
LabelWhiteList utils.RegexpVal
@ -196,6 +197,7 @@ func newDefaultConfig() *NFDConfig {
DenyLabelNs: utils.StringSetVal{},
ExtraLabelNs: utils.StringSetVal{},
NoPublish: false,
AutoDefaultNs: true,
NfdApiParallelism: 10,
ResourceLabels: utils.StringSetVal{},
EnableTaints: false,
@ -551,7 +553,6 @@ func (m *nfdMaster) filterFeatureLabels(labels Labels, features *nfdv1alpha1.Fea
}
func (m *nfdMaster) filterFeatureLabel(name, value string, features *nfdv1alpha1.Features) (string, error) {
//Validate label name
if errs := k8svalidation.IsQualifiedName(name); len(errs) > 0 {
return "", fmt.Errorf("invalid name %q: %s", name, strings.Join(errs, "; "))
@ -763,10 +764,14 @@ func (m *nfdMaster) nfdAPIUpdateOneNode(nodeName string) error {
// 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.Labels = addNsToMapKeys(features.Labels, nfdv1alpha1.FeatureLabelNs)
if m.config.AutoDefaultNs {
features.Labels = addNsToMapKeys(features.Labels, nfdv1alpha1.FeatureLabelNs)
}
for _, o := range objs[1:] {
s := o.Spec.DeepCopy()
s.Labels = addNsToMapKeys(s.Labels, nfdv1alpha1.FeatureLabelNs)
if m.config.AutoDefaultNs {
s.Labels = addNsToMapKeys(s.Labels, nfdv1alpha1.FeatureLabelNs)
}
s.MergeInto(features)
}
@ -789,7 +794,7 @@ func (m *nfdMaster) nfdAPIUpdateOneNode(nodeName string) error {
// filterExtendedResources filters extended resources and returns a map
// of valid extended resources.
func filterExtendedResources(features *nfdv1alpha1.Features, extendedResources ExtendedResources) ExtendedResources {
func (m *nfdMaster) filterExtendedResources(features *nfdv1alpha1.Features, extendedResources ExtendedResources) ExtendedResources {
outExtendedResources := ExtendedResources{}
for name, value := range extendedResources {
capacity, err := filterExtendedResource(name, value, features)
@ -836,7 +841,11 @@ func filterExtendedResource(name, value string, features *nfdv1alpha1.Features)
}
func (m *nfdMaster) refreshNodeFeatures(cli *kubernetes.Clientset, nodeName string, labels map[string]string, features *nfdv1alpha1.Features) error {
labels = addNsToMapKeys(labels, nfdv1alpha1.FeatureLabelNs)
if m.config.AutoDefaultNs {
labels = addNsToMapKeys(labels, nfdv1alpha1.FeatureLabelNs)
} else if labels == nil {
labels = make(map[string]string)
}
crLabels, crAnnotations, crExtendedResources, crTaints := m.processNodeFeatureRule(nodeName, features)
@ -853,7 +862,7 @@ func (m *nfdMaster) refreshNodeFeatures(cli *kubernetes.Clientset, nodeName stri
for k, v := range crExtendedResources {
extendedResources[k] = v
}
extendedResources = filterExtendedResources(features, extendedResources)
extendedResources = m.filterExtendedResources(features, extendedResources)
// Annotations
annotations := m.filterFeatureAnnotations(crAnnotations)
@ -1011,13 +1020,22 @@ func (m *nfdMaster) processNodeFeatureRule(nodeName string, features *nfdv1alpha
continue
}
taints = append(taints, ruleOut.Taints...)
for k, v := range addNsToMapKeys(ruleOut.Labels, nfdv1alpha1.FeatureLabelNs) {
l := ruleOut.Labels
e := ruleOut.ExtendedResources
a := ruleOut.Annotations
if m.config.AutoDefaultNs {
l = addNsToMapKeys(ruleOut.Labels, nfdv1alpha1.FeatureLabelNs)
e = addNsToMapKeys(ruleOut.ExtendedResources, nfdv1alpha1.ExtendedResourceNs)
a = addNsToMapKeys(ruleOut.Annotations, nfdv1alpha1.FeatureAnnotationNs)
}
for k, v := range l {
labels[k] = v
}
for k, v := range addNsToMapKeys(ruleOut.ExtendedResources, nfdv1alpha1.ExtendedResourceNs) {
for k, v := range e {
extendedResources[k] = v
}
for k, v := range addNsToMapKeys(ruleOut.Annotations, nfdv1alpha1.FeatureAnnotationNs) {
for k, v := range a {
annotations[k] = v
}
@ -1430,9 +1448,13 @@ func (m *nfdMaster) filterFeatureAnnotations(annotations map[string]string) map[
}
// Check annotation namespace, filter out if ns is not whitelisted
if ns != nfdv1alpha1.FeatureAnnotationNs && !strings.HasSuffix(ns, nfdv1alpha1.FeatureAnnotationSubNsSuffix) {
// If the namespace is denied, and not present in the extraLabelNs, label will be ignored
// If the namespace is denied the annotation will be ignored
if ns == "" {
klog.ErrorS(fmt.Errorf("labels without namespace (prefix/) not allowed"), fmt.Sprintf("Ignoring annotation %s", annotation))
continue
}
if ns == "kubernetes.io" || strings.HasSuffix(ns, ".kubernetes.io") || ns == nfdv1alpha1.AnnotationNs {
klog.ErrorS(fmt.Errorf("namespace %v is not allowed", ns), fmt.Sprintf("Ignoring annotation %v\n", annotation))
klog.ErrorS(fmt.Errorf("namespace %q is not allowed", ns), fmt.Sprintf("Ignoring annotation %s", annotation))
continue
}
}

View file

@ -28,6 +28,7 @@ import (
"github.com/stretchr/testify/mock"
"github.com/vektra/errors"
nfdv1alpha1 "sigs.k8s.io/node-feature-discovery/pkg/apis/nfd/v1alpha1"
"sigs.k8s.io/node-feature-discovery/pkg/labeler"
"sigs.k8s.io/node-feature-discovery/pkg/utils"
"sigs.k8s.io/node-feature-discovery/source"
@ -84,7 +85,7 @@ func makeFakeFeatures(names []string) (source.FeatureLabels, Labels) {
labels := Labels{}
for _, f := range names {
features[f] = true
labelName := fakeLabelSourceName + "-" + f
labelName := nfdv1alpha1.FeatureLabelNs + "/" + fakeLabelSourceName + "-" + f
if strings.IndexByte(f, '/') >= 0 {
labelName = f
}
@ -352,9 +353,9 @@ func TestCreateFeatureLabels(t *testing.T) {
Convey("Proper fake labels are returned", func() {
So(len(labels), ShouldEqual, 3)
So(labels, ShouldContainKey, "fake-fakefeature1")
So(labels, ShouldContainKey, "fake-fakefeature2")
So(labels, ShouldContainKey, "fake-fakefeature3")
So(labels, ShouldContainKey, nfdv1alpha1.FeatureLabelNs+"/"+"fake-fakefeature1")
So(labels, ShouldContainKey, nfdv1alpha1.FeatureLabelNs+"/"+"fake-fakefeature2")
So(labels, ShouldContainKey, nfdv1alpha1.FeatureLabelNs+"/"+"fake-fakefeature3")
})
})
Convey("When fake feature source is configured with a whitelist that doesn't match", func() {

View file

@ -580,34 +580,29 @@ func getFeatureLabels(source source.LabelSource, labelWhiteList regexp.Regexp) (
return nil, err
}
// Prefix for labels in the default namespace
prefix := source.Name() + "-"
switch source.Name() {
case "local", "custom":
// Do not prefix labels from the custom rules, hooks or feature files
prefix = ""
}
for k, v := range features {
// Split label name into namespace and name compoents. Use dummy 'ns'
// default namespace because there is no function to validate just
// the name part
split := strings.SplitN(k, "/", 2)
label := prefix + split[0]
nameForValidation := "ns/" + label
nameForWhiteListing := label
name := k
switch sourceName := source.Name(); sourceName {
case "local", "custom":
// No mangling of labels from the custom rules, hooks or feature files
default:
// Prefix for labels from other sources
if !strings.Contains(name, "/") {
name = nfdv1alpha1.FeatureLabelNs + "/" + sourceName + "-" + name
}
}
// Split label name into namespace and name compoents
split := strings.SplitN(name, "/", 2)
nameForWhiteListing := name
if len(split) == 2 {
label = k
nameForValidation = label
nameForWhiteListing = split[1]
}
// Validate label name.
errs := validation.IsQualifiedName(nameForValidation)
errs := validation.IsQualifiedName(name)
if len(errs) > 0 {
klog.InfoS("ignoring label with invalid name", "lableKey", label, "errors", errs)
klog.InfoS("ignoring label with invalid name", "lableKey", name, "errors", errs)
continue
}
@ -615,7 +610,7 @@ func getFeatureLabels(source source.LabelSource, labelWhiteList regexp.Regexp) (
// Validate label value
errs = validation.IsValidLabelValue(value)
if len(errs) > 0 {
klog.InfoS("ignoring label with invalide value", "labelKey", label, "labelValue", value, "errors", errs)
klog.InfoS("ignoring label with invalide value", "labelKey", name, "labelValue", value, "errors", errs)
continue
}
@ -625,7 +620,7 @@ func getFeatureLabels(source source.LabelSource, labelWhiteList regexp.Regexp) (
continue
}
labels[label] = value
labels[name] = value
}
return labels, nil
}