From 291ba82fa7ef64dc29d36c387aee5fd819d51e10 Mon Sep 17 00:00:00 2001 From: Markus Lehtonen Date: Fri, 26 May 2023 16:06:56 +0300 Subject: [PATCH] nfd-master: add validation of label names and values Validate labels before trying to update the node. Makes us fail early nad prevent useless retries in case invalid labels are tried. --- .../topologyupdater-daemonset.yaml | 2 +- pkg/nfd-master/nfd-master-internal_test.go | 6 ++++-- pkg/nfd-master/nfd-master.go | 16 ++++++++++++++-- 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/deployment/base/topologyupdater-daemonset/topologyupdater-daemonset.yaml b/deployment/base/topologyupdater-daemonset/topologyupdater-daemonset.yaml index 8971ea8abb..6b64cf3647 100644 --- a/deployment/base/topologyupdater-daemonset/topologyupdater-daemonset.yaml +++ b/deployment/base/topologyupdater-daemonset/topologyupdater-daemonset.yaml @@ -18,7 +18,7 @@ spec: containers: - name: nfd-topology-updater image: gcr.io/k8s-staging-nfd/node-feature-discovery:master - imagePullPolicy: Always + imagePullPolicy: IfNotPresent command: - "nfd-topology-updater" args: [] diff --git a/pkg/nfd-master/nfd-master-internal_test.go b/pkg/nfd-master/nfd-master-internal_test.go index ffcac269c1..9e828b3022 100644 --- a/pkg/nfd-master/nfd-master-internal_test.go +++ b/pkg/nfd-master/nfd-master-internal_test.go @@ -365,8 +365,10 @@ func TestSetLabels(t *testing.T) { "random.denied.ns/feature-3": "val-3", "kubernetes.io/feature-4": "val-4", "sub.ns.kubernetes.io/feature-5": "val-5", - vendorFeatureLabel: " val-6", - vendorProfileLabel: " val-7"} + vendorFeatureLabel: "val-6", + vendorProfileLabel: "val-7", + "--invalid-name--": "valid-val", + "valid-name": "--invalid-val--"} expectedPatches := []apihelper.JsonPatch{ apihelper.NewJsonPatch("add", "/metadata/annotations", instance+"."+nfdv1alpha1.WorkerVersionAnnotation, workerVer), apihelper.NewJsonPatch("add", "/metadata/annotations", diff --git a/pkg/nfd-master/nfd-master.go b/pkg/nfd-master/nfd-master.go index 98ef6689de..b5a31cbd5a 100644 --- a/pkg/nfd-master/nfd-master.go +++ b/pkg/nfd-master/nfd-master.go @@ -41,6 +41,7 @@ import ( k8sQuantity "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" k8sLabels "k8s.io/apimachinery/pkg/labels" + k8svalidation "k8s.io/apimachinery/pkg/util/validation" "k8s.io/client-go/kubernetes" restclient "k8s.io/client-go/rest" "k8s.io/client-go/tools/leaderelection" @@ -490,7 +491,7 @@ func (m *nfdMaster) filterFeatureLabels(labels Labels) (Labels, ExtendedResource // Add possibly missing default ns name := addNs(name, nfdv1alpha1.FeatureLabelNs) - if err := m.filterFeatureLabel(name); err != nil { + if err := m.filterFeatureLabel(name, value); err != nil { klog.Errorf("ignoring label %s=%v: %v", name, value, err) } else { outLabels[name] = value @@ -516,7 +517,12 @@ func (m *nfdMaster) filterFeatureLabels(labels Labels) (Labels, ExtendedResource return outLabels, extendedResources } -func (m *nfdMaster) filterFeatureLabel(name string) error { +func (m *nfdMaster) filterFeatureLabel(name, value string) error { + //Validate label name + if errs := k8svalidation.IsQualifiedName(name); len(errs) > 0 { + return fmt.Errorf("invalid name %q: %s", name, strings.Join(errs, "; ")) + } + // Check label namespace, filter out if ns is not whitelisted ns, base := splitNs(name) if ns != nfdv1alpha1.FeatureLabelNs && ns != nfdv1alpha1.ProfileLabelNs && @@ -533,6 +539,12 @@ func (m *nfdMaster) filterFeatureLabel(name string) error { if !m.config.LabelWhiteList.Regexp.MatchString(base) { return fmt.Errorf("%s (%s) does not match the whitelist (%s)", base, name, m.config.LabelWhiteList.Regexp.String()) } + + // Validate the label value + if errs := k8svalidation.IsValidLabelValue(value); len(errs) > 0 { + return fmt.Errorf("invalid value %q: %s", value, strings.Join(errs, "; ")) + } + return nil }