From b499799364b0b019c361228bc3ed6565babf2379 Mon Sep 17 00:00:00 2001 From: AhmedGrati Date: Fri, 3 Feb 2023 20:27:26 +0100 Subject: [PATCH] feat: add deny-label-ns flag which supports wildcard Signed-off-by: AhmedGrati --- cmd/nfd-master/main.go | 3 + .../templates/master.yaml | 3 + .../helm/node-feature-discovery/values.yaml | 1 + .../reference/master-commandline-reference.md | 30 +++++++-- docs/usage/customization-guide.md | 21 ++---- pkg/nfd-master/nfd-master-internal_test.go | 14 ++-- pkg/nfd-master/nfd-master.go | 66 ++++++++++++++++--- pkg/utils/flags.go | 3 +- 8 files changed, 107 insertions(+), 34 deletions(-) diff --git a/cmd/nfd-master/main.go b/cmd/nfd-master/main.go index b42cc6286..7f1d582c8 100644 --- a/cmd/nfd-master/main.go +++ b/cmd/nfd-master/main.go @@ -85,12 +85,15 @@ func main() { func initFlags(flagset *flag.FlagSet) *master.Args { args := &master.Args{ LabelWhiteList: utils.RegexpVal{Regexp: *regexp.MustCompile("")}, + DenyLabelNs: map[string]struct{}{"*.kubernetes.io": {}}, } flagset.StringVar(&args.CaFile, "ca-file", "", "Root certificate for verifying connections") flagset.StringVar(&args.CertFile, "cert-file", "", "Certificate used for authenticating connections") + flagset.Var(&args.DenyLabelNs, "deny-label-ns", + "Comma separated list of denied label namespaces") flagset.Var(&args.ExtraLabelNs, "extra-label-ns", "Comma separated list of allowed extra label namespaces") flagset.StringVar(&args.Instance, "instance", "", diff --git a/deployment/helm/node-feature-discovery/templates/master.yaml b/deployment/helm/node-feature-discovery/templates/master.yaml index 8c66a7955..c259df529 100644 --- a/deployment/helm/node-feature-discovery/templates/master.yaml +++ b/deployment/helm/node-feature-discovery/templates/master.yaml @@ -86,6 +86,9 @@ spec: {{- if .Values.master.extraLabelNs | empty | not }} - "-extra-label-ns={{- join "," .Values.master.extraLabelNs }}" {{- end }} + {{- if .Values.master.denyLabelNs | empty | not }} + - "-deny-label-ns={{- join "," .Values.master.denyLabelNs }}" + {{- end }} {{- if .Values.master.resourceLabels | empty | not }} - "-resource-labels={{- join "," .Values.master.resourceLabels }}" {{- end }} diff --git a/deployment/helm/node-feature-discovery/values.yaml b/deployment/helm/node-feature-discovery/values.yaml index 799a0bec3..5f2835c24 100644 --- a/deployment/helm/node-feature-discovery/values.yaml +++ b/deployment/helm/node-feature-discovery/values.yaml @@ -17,6 +17,7 @@ master: port: 8080 instance: featureApi: + denyLabelNs: [] extraLabelNs: [] resourceLabels: [] crdController: null diff --git a/docs/reference/master-commandline-reference.md b/docs/reference/master-commandline-reference.md index 7b8c000ae..a62f348d1 100644 --- a/docs/reference/master-commandline-reference.md +++ b/docs/reference/master-commandline-reference.md @@ -216,12 +216,10 @@ nfd-master -label-whitelist='.*cpuid\.' ### -extra-label-ns The `-extra-label-ns` flag specifies a comma-separated list of allowed feature -label namespaces. By default, nfd-master only allows creating labels in the -default `feature.node.kubernetes.io` and `profile.node.kubernetes.io` label -namespaces and their sub-namespaces (e.g. `vendor.feature.node.kubernetes.io` -and `sub.ns.profile.node.kubernetes.io`). This option can be used to allow +label namespaces. This option can be used to allow other vendor or application specific namespaces for custom labels from the -local and custom feature sources. +local and custom feature sources, even though these labels were denied using +the `deny-label-ns` flag. The same namespace control and this flag applies Extended Resources (created with `-resource-labels`), too. @@ -234,6 +232,28 @@ Example: nfd-master -extra-label-ns=vendor-1.com,vendor-2.io ``` +### -deny-label-ns + +The `-deny-label-ns` flag specifies a comma-separated list of excluded +label namespaces. By default, nfd-master allows creating labels in all +namespaces, excluding `kubernetes.io` namespace and its sub-namespaces +(i.e. `*.kubernetes.io`). However, you should note that +`kubernetes.io` and its sub-namespaces are always denied. +For example, `nfd-master -deny-label-ns=""` would still disallow +`kubernetes.io` and `*.kubernetes.io`. +This option can be used to exclude some vendors or application specific +namespaces. +Note that the namespaces `feature.node.kubernetes.io` and `profile.node.kubernetes.io` +and their sub-namespaces are always allowed and cannot be denied. + +Default: *empty* + +Example: + +```bash +nfd-master -deny-label-ns=*.vendor.com,vendor-2.io +``` + ### -resource-labels The `-resource-labels` flag specifies a comma-separated list of features to be diff --git a/docs/usage/customization-guide.md b/docs/usage/customization-guide.md index 1bdb0799c..2c88dcc30 100644 --- a/docs/usage/customization-guide.md +++ b/docs/usage/customization-guide.md @@ -249,9 +249,6 @@ feature.node.kubernetes.io/my-feature.2: "myvalue" my.namespace/my-feature.3: "456" ``` -Note that in the example above `-extra-label-ns=my.namespace` must be specified -on the nfd-master command line. - ### Hooks **DEPRECATED** The `local` source executes hooks found in @@ -303,11 +300,7 @@ key-value pairs, separated by newlines: The label value defaults to `true`, if not specified. -Label namespace may be specified with `/[=]`. The -namespace must be explicitly allowed with the `-extra-label-ns` command line -flag of nfd-master if using something else than -`[.]feature.node.kubernetes.io` or -`[.]profile.node.kubernetes.io`. +Label namespace may be specified with `/[=]`. ### Mounts @@ -414,13 +407,13 @@ 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. -- User-defined labels are allowed to use: - - `feature.node.kubernetes.io` and `profile.node.kubernetes.io` plus their - sub-namespaces (e.g. `vendor.profile.node.kubernetes.io` and - `sub.ns.profile.node.kubernetes.io`) by default - - Additional namespaces may be enabled with the +- Namespaces may be excluded with the + [`-deny-label-ns`](../reference/master-commandline-reference.md#-deny-label-ns) + command line flag of nfd-master + - To allow specific namespaces that were denied, you can use [`-extra-label-ns`](../reference/master-commandline-reference.md#-extra-label-ns) - command line flag of nfd-master + command line flag of nfd-master. + e.g: `nfd-master -deny-label-ns="*" -extra-label-ns=example.com` ## Label rule format diff --git a/pkg/nfd-master/nfd-master-internal_test.go b/pkg/nfd-master/nfd-master-internal_test.go index cf50e4906..2281e8f20 100644 --- a/pkg/nfd-master/nfd-master-internal_test.go +++ b/pkg/nfd-master/nfd-master-internal_test.go @@ -352,16 +352,18 @@ func TestSetLabels(t *testing.T) { }) }) - Convey("When -extra-label-ns and -instance are specified", func() { + Convey("When -extra-label-ns, -deny-label-ns and -instance are specified", func() { // In the gRPC request the label names may omit the default ns instance := "foo" vendorFeatureLabel := "vendor." + nfdv1alpha1.FeatureLabelNs + "/feature-4" vendorProfileLabel := "vendor." + nfdv1alpha1.ProfileLabelNs + "/feature-5" mockLabels := map[string]string{"feature-1": "val-1", - "valid.ns/feature-2": "val-2", - "invalid.ns/feature-3": "val-3", - vendorFeatureLabel: " val-4", - vendorProfileLabel: " val-5"} + "valid.ns/feature-2": "val-2", + "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"} expectedPatches := []apihelper.JsonPatch{ apihelper.NewJsonPatch("add", "/metadata/annotations", instance+"."+nfdv1alpha1.WorkerVersionAnnotation, workerVer), apihelper.NewJsonPatch("add", "/metadata/annotations", @@ -374,6 +376,8 @@ func TestSetLabels(t *testing.T) { apihelper.NewJsonPatch("add", "/metadata/labels", vendorProfileLabel, mockLabels[vendorProfileLabel]), } + mockMaster.deniedNs.normal = map[string]struct{}{"random.denied.ns": {}} + mockMaster.deniedNs.wildcard = map[string]struct{}{"kubernetes.io": {}} mockMaster.args.ExtraLabelNs = map[string]struct{}{"valid.ns": {}} mockMaster.args.Instance = instance mockHelper.On("GetClient").Return(mockClient, nil) diff --git a/pkg/nfd-master/nfd-master.go b/pkg/nfd-master/nfd-master.go index 33549f9b4..288773e68 100644 --- a/pkg/nfd-master/nfd-master.go +++ b/pkg/nfd-master/nfd-master.go @@ -63,6 +63,7 @@ type Annotations map[string]string type Args struct { CaFile string CertFile string + DenyLabelNs utils.StringSetVal ExtraLabelNs utils.StringSetVal Instance string KeyFile string @@ -78,6 +79,11 @@ type Args struct { ResourceLabels utils.StringSetVal } +type deniedNs struct { + normal utils.StringSetVal + wildcard utils.StringSetVal +} + type NfdMaster interface { Run() error Stop() @@ -95,6 +101,7 @@ type nfdMaster struct { ready chan bool apihelper apihelper.APIHelpers kubeconfig *restclient.Config + deniedNs } // NewNfdMaster creates a new NfdMaster server instance. @@ -126,6 +133,16 @@ func NewNfdMaster(args *Args) (NfdMaster, error) { return nfd, fmt.Errorf("-ca-file needs to be specified alongside -cert-file and -key-file") } } + if args.DenyLabelNs == nil { + args.DenyLabelNs = make(utils.StringSetVal) + } + // Pre-process DenyLabelNS into 2 lists: one for normal ns, and the other for wildcard ns + normalDeniedNs, wildcardDeniedNs := preProcessDeniedNamespaces(args.DenyLabelNs) + nfd.deniedNs.normal = normalDeniedNs + nfd.deniedNs.wildcard = wildcardDeniedNs + // We forcibly deny kubernetes.io + nfd.deniedNs.normal["kubernetes.io"] = struct{}{} + nfd.deniedNs.wildcard[".kubernetes.io"] = struct{}{} // Initialize Kubernetes API helpers if !args.NoPublish { @@ -392,7 +409,7 @@ func (m *nfdMaster) updateMasterNode() error { // into extended resources. This function also handles proper namespacing of // labels and ERs, i.e. adds the possibly missing default namespace for labels // arriving through the gRPC API. -func filterFeatureLabels(labels Labels, extraLabelNs map[string]struct{}, labelWhiteList regexp.Regexp, extendedResourceNames map[string]struct{}) (Labels, ExtendedResources) { +func (m *nfdMaster) filterFeatureLabels(labels Labels) (Labels, ExtendedResources) { outLabels := Labels{} for label, value := range labels { @@ -404,15 +421,18 @@ func filterFeatureLabels(labels Labels, extraLabelNs map[string]struct{}, labelW // Check label namespace, filter out if ns is not whitelisted if ns != nfdv1alpha1.FeatureLabelNs && ns != nfdv1alpha1.ProfileLabelNs && !strings.HasSuffix(ns, nfdv1alpha1.FeatureLabelSubNsSuffix) && !strings.HasSuffix(ns, nfdv1alpha1.ProfileLabelSubNsSuffix) { - if _, ok := extraLabelNs[ns]; !ok { - klog.Errorf("Namespace %q is not allowed. Ignoring label %q\n", ns, label) - continue + // If the namespace is denied, and not present in the extraLabelNs, label will be ignored + if isNamespaceDenied(ns, m.deniedNs.wildcard, m.deniedNs.normal) { + if _, ok := m.args.ExtraLabelNs[ns]; !ok { + klog.Errorf("Namespace %q is not allowed. Ignoring label %q\n", ns, label) + continue + } } } // Skip if label doesn't match labelWhiteList - if !labelWhiteList.MatchString(name) { - klog.Errorf("%s (%s) does not match the whitelist (%s) and will not be published.", name, label, labelWhiteList.String()) + if !m.args.LabelWhiteList.Regexp.MatchString(name) { + klog.Errorf("%s (%s) does not match the whitelist (%s) and will not be published.", name, label, m.args.LabelWhiteList.Regexp.String()) continue } outLabels[label] = value @@ -420,7 +440,7 @@ func filterFeatureLabels(labels Labels, extraLabelNs map[string]struct{}, labelW // Remove labels which are intended to be extended resources extendedResources := ExtendedResources{} - for extendedResourceName := range extendedResourceNames { + for extendedResourceName := range m.args.ResourceLabels { // Add possibly missing default ns extendedResourceName = addNs(extendedResourceName, nfdv1alpha1.FeatureLabelNs) if value, ok := outLabels[extendedResourceName]; ok { @@ -449,6 +469,20 @@ func verifyNodeName(cert *x509.Certificate, nodeName string) error { return nil } +func isNamespaceDenied(labelNs string, wildcardDeniedNs map[string]struct{}, normalDeniedNs map[string]struct{}) bool { + for deniedNs := range normalDeniedNs { + if labelNs == deniedNs { + return true + } + } + for deniedNs := range wildcardDeniedNs { + if strings.HasSuffix(labelNs, deniedNs) { + return true + } + } + return false +} + // SetLabels implements LabelerServer func (m *nfdMaster) SetLabels(c context.Context, r *pb.SetLabelsRequest) (*pb.SetLabelsReply, error) { err := authorizeClient(c, m.args.VerifyNodeName, r.NodeName) @@ -583,7 +617,7 @@ func (m *nfdMaster) refreshNodeFeatures(cli *kubernetes.Clientset, nodeName stri labels[k] = v } - labels, extendedResources := filterFeatureLabels(labels, m.args.ExtraLabelNs, m.args.LabelWhiteList.Regexp, m.args.ResourceLabels) + labels, extendedResources := m.filterFeatureLabels(labels) var taints []corev1.Taint if m.args.EnableTaints { @@ -918,6 +952,22 @@ func stringToNsNames(cslist, ns string) []string { return names } +// Seperate denied namespaces into two lists: +// one contains wildcard namespaces the other contains normal namespaces +func preProcessDeniedNamespaces(deniedNs map[string]struct{}) (normalDeniedNs map[string]struct{}, wildcardDeniedNs map[string]struct{}) { + normalDeniedNs = map[string]struct{}{} + wildcardDeniedNs = map[string]struct{}{} + for ns := range deniedNs { + if strings.HasPrefix(ns, "*") { + trimedNs := strings.TrimLeft(ns, "*") + wildcardDeniedNs[trimedNs] = struct{}{} + } else { + normalDeniedNs[ns] = struct{}{} + } + } + return +} + func (m *nfdMaster) instanceAnnotation(name string) string { if m.args.Instance == "" { return name diff --git a/pkg/utils/flags.go b/pkg/utils/flags.go index cfd23806e..2045bd5fa 100644 --- a/pkg/utils/flags.go +++ b/pkg/utils/flags.go @@ -76,8 +76,7 @@ func (a *StringSetVal) String() string { if *a == nil { return "" } - - vals := make([]string, len(*a), 0) + vals := make([]string, 0, len(*a)) for val := range *a { vals = append(vals, val) }