Skip to content

Commit

Permalink
nfd-master: add validation of label names and values
Browse files Browse the repository at this point in the history
Validate labels before trying to update the node. Makes us fail early
nad prevent useless retries in case invalid labels are tried.
  • Loading branch information
marquiz committed May 26, 2023
1 parent 7d71510 commit 291ba82
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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: []
6 changes: 4 additions & 2 deletions pkg/nfd-master/nfd-master-internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
16 changes: 14 additions & 2 deletions pkg/nfd-master/nfd-master.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand All @@ -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 &&
Expand All @@ -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
}

Expand Down

0 comments on commit 291ba82

Please sign in to comment.