Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nfd-master: refactor filtering of labels, taints and ERs #1202

Merged
merged 3 commits into from
May 1, 2023
Merged

nfd-master: refactor filtering of labels, taints and ERs #1202

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6ca687f
nfd-master: refactor filtering of extended resources
marquiz Apr 28, 2023
43ced0c
nfd-master: refactor filtering of feature labels
marquiz Apr 28, 2023
fb20388
nfd-master: refactor filtering of taints
marquiz Apr 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
190 changes: 91 additions & 99 deletions pkg/nfd-master/nfd-master.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -465,30 +465,15 @@ func (m *nfdMaster) updateMasterNode() error {
// arriving through the gRPC API.
func (m *nfdMaster) filterFeatureLabels(labels Labels) (Labels, ExtendedResources) {
outLabels := Labels{}
for label, value := range labels {
for name, value := range labels {
// Add possibly missing default ns
label := addNs(label, nfdv1alpha1.FeatureLabelNs)

ns, name := splitNs(label)

// 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 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.config.ExtraLabelNs[ns]; !ok {
klog.Errorf("Namespace %q is not allowed. Ignoring label %q\n", ns, label)
continue
}
}
}
name := addNs(name, nfdv1alpha1.FeatureLabelNs)

// Skip if label doesn't match labelWhiteList
if !m.config.LabelWhiteList.Regexp.MatchString(name) {
klog.Errorf("%s (%s) does not match the whitelist (%s) and will not be published.", name, label, m.config.LabelWhiteList.Regexp.String())
continue
if err := m.filterFeatureLabel(name); err != nil {
klog.Errorf("ignoring label %s=%v: %v", name, value, err)
} else {
outLabels[name] = value
}
outLabels[label] = value
}

// Remove labels which are intended to be extended resources
Expand All @@ -510,28 +495,52 @@ func (m *nfdMaster) filterFeatureLabels(labels Labels) (Labels, ExtendedResource
return outLabels, extendedResources
}

func (m *nfdMaster) filterFeatureLabel(name string) error {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer to name the argument label instead of name, but that's not a big deal.

// Check label namespace, filter out if ns is not whitelisted
ns, base := splitNs(name)
if ns != nfdv1alpha1.FeatureLabelNs && ns != nfdv1alpha1.ProfileLabelNs &&
!strings.HasSuffix(ns, nfdv1alpha1.FeatureLabelSubNsSuffix) && !strings.HasSuffix(ns, nfdv1alpha1.ProfileLabelSubNsSuffix) {
// 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.config.ExtraLabelNs[ns]; !ok {
return fmt.Errorf("namespace %q is not allowed", ns)
}
}
}

// Skip if label doesn't match labelWhiteList
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())
}
return nil
}

func filterTaints(taints []corev1.Taint) []corev1.Taint {
outTaints := []corev1.Taint{}

for _, taint := range taints {
ns, _ := splitNs(taint.Key)

// Check prefix of the key, filter out disallowed ones
if ns == "" {
klog.Errorf("taint keys without namespace (prefix/) are not allowed. Ignoring taint %v", ns, taint)
continue
}
if ns != nfdv1alpha1.TaintNs && !strings.HasSuffix(ns, nfdv1alpha1.TaintSubNsSuffix) &&
(ns == "kubernetes.io" || strings.HasSuffix(ns, ".kubernetes.io")) {
klog.Errorf("Prefix %q is not allowed for taint key. Ignoring taint %v", ns, taint)
continue
if err := filterTaint(&taint); err != nil {
klog.Errorf("ignoring taint %q: %w", taint.ToString(), err)
} else {
outTaints = append(outTaints, taint)
}
outTaints = append(outTaints, taint)
}

return outTaints
}

func filterTaint(taint *corev1.Taint) error {
// Check prefix of the key, filter out disallowed ones
ns, _ := splitNs(taint.Key)
if ns == "" {
return fmt.Errorf("taint keys without namespace (prefix/) are not allowed")
}
if ns != nfdv1alpha1.TaintNs && !strings.HasSuffix(ns, nfdv1alpha1.TaintSubNsSuffix) &&
(ns == "kubernetes.io" || strings.HasSuffix(ns, ".kubernetes.io")) {
return fmt.Errorf("prefix %q is not allowed for taint key", ns)
}
return nil
}

func verifyNodeName(cert *x509.Certificate, nodeName string) error {
if cert.Subject.CommonName == nodeName {
return nil
Expand All @@ -558,20 +567,6 @@ func isNamespaceDenied(labelNs string, wildcardDeniedNs map[string]struct{}, nor
return false
}

func isNamespaceAllowed(labelNs string, wildcardAllowedNs map[string]struct{}, normalAllowedNs map[string]struct{}) bool {
for allowedNs := range normalAllowedNs {
if labelNs == allowedNs {
return true
}
}
for allowedNs := range wildcardAllowedNs {
if strings.HasSuffix(labelNs, allowedNs) {
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)
Expand Down Expand Up @@ -696,64 +691,60 @@ func (m *nfdMaster) nfdAPIUpdateOneNode(nodeName string) error {

// filterExtendedResources filters extended resources and returns a map
// of valid extended resources.
func (m *nfdMaster) filterExtendedResources(features *nfdv1alpha1.Features, extendedResources ExtendedResources) ExtendedResources {
func filterExtendedResources(features *nfdv1alpha1.Features, extendedResources ExtendedResources) ExtendedResources {
outExtendedResources := ExtendedResources{}
deniedNs := map[string]struct{}{"kubernetes.io": {}}
deniedWildCarNs := map[string]struct{}{".kubernetes.io": {}}
allowedNs := map[string]struct{}{nfdv1alpha1.ExtendedResourceNs: {}}
allowedWildCardNs := map[string]struct{}{nfdv1alpha1.ExtendedResourceSubNsSuffix: {}}
for extendedResource, capacity := range extendedResources {
if strings.Contains(extendedResource, "/") {
// Check if given NS is allowed
ns, _ := splitNs(extendedResource)
if isNamespaceDenied(ns, deniedWildCarNs, deniedNs) {
if !isNamespaceAllowed(ns, allowedWildCardNs, allowedNs) {
klog.Errorf("namespace %q is not allowed. Ignoring Extended Resource %q", ns, extendedResource)
continue
}
}
for name, value := range extendedResources {
// Add possibly missing default ns
name = addNs(name, nfdv1alpha1.ExtendedResourceNs)

capacity, err := filterExtendedResource(name, value, features)
if err != nil {
klog.Errorf("failed to create extended resources %s=%s: %v", name, value, err)
} else {
// Add possibly missing default ns
extendedResource = path.Join(nfdv1alpha1.ExtendedResourceNs, extendedResource)
outExtendedResources[name] = capacity
}
}
return outExtendedResources
}

// Dynamic Value
if strings.HasPrefix(capacity, "@") {
// capacity is a string in the form of attribute.featureset.elements
split := strings.SplitN(capacity[1:], ".", 3)
if len(split) != 3 {
klog.Errorf("capacity %s is not in the form of '@domain.feature.element',. Ignoring Extended Resource %q", capacity, extendedResource)
continue
}
featureName := split[0] + "." + split[1]
elementName := split[2]
attrFeatureSet, ok := features.Attributes[featureName]
if !ok {
klog.Errorf("feature %s not found. Ignoring Extended Resource %q", featureName, extendedResource)
continue
}
element, ok := attrFeatureSet.Elements[elementName]
if !ok {
klog.Errorf("element %s not found on feature %s. Ignoring Extended Resource %q", elementName, featureName, extendedResource)
continue
}
q, err := k8sQuantity.ParseQuantity(element)
if err != nil {
klog.Errorf("bad label value %s encountered for extended resource: %s", q.String(), extendedResource, err)
continue
}
outExtendedResources[extendedResource] = q.String()
continue
func filterExtendedResource(name, value string, features *nfdv1alpha1.Features) (string, error) {
// Check if given NS is allowed
ns, _ := splitNs(name)
if ns != nfdv1alpha1.ExtendedResourceNs && !strings.HasPrefix(ns, nfdv1alpha1.ExtendedResourceSubNsSuffix) {
if ns == "kubernetes.io" || strings.HasSuffix(ns, ".kubernetes.io") {
return "", fmt.Errorf("namespace %q is not allowed", ns)
}
// Static Value (Pre-Defined at the NodeFeatureRule)
q, err := k8sQuantity.ParseQuantity(capacity)
}

// Dynamic Value
if strings.HasPrefix(value, "@") {
// value is a string in the form of attribute.featureset.elements
split := strings.SplitN(value[1:], ".", 3)
if len(split) != 3 {
return "", fmt.Errorf("value %s is not in the form of '@domain.feature.element'", value)
}
featureName := split[0] + "." + split[1]
elementName := split[2]
attrFeatureSet, ok := features.Attributes[featureName]
if !ok {
return "", fmt.Errorf("feature %s not found", featureName)
}
element, ok := attrFeatureSet.Elements[elementName]
if !ok {
return "", fmt.Errorf("element %s not found on feature %s", elementName, featureName)
}
q, err := k8sQuantity.ParseQuantity(element)
if err != nil {
klog.Errorf("bad label value %s encountered for extended resource: %s", capacity, extendedResource, err)
continue
return "", fmt.Errorf("invalid value %s (from %s): %w", element, value, err)
}
outExtendedResources[extendedResource] = q.String()
return q.String(), nil
}
return outExtendedResources
// Static Value (Pre-Defined at the NodeFeatureRule)
q, err := k8sQuantity.ParseQuantity(value)
if err != nil {
return "", fmt.Errorf("invalid value %s: %w", value, err)
}
return q.String(), nil
}

func (m *nfdMaster) refreshNodeFeatures(cli *kubernetes.Clientset, nodeName string, annotations Annotations, labels map[string]string, features *nfdv1alpha1.Features) error {
Expand All @@ -777,7 +768,7 @@ func (m *nfdMaster) refreshNodeFeatures(cli *kubernetes.Clientset, nodeName stri
for k, v := range crExtendedResources {
extendedResources[k] = v
}
extendedResources = m.filterExtendedResources(features, extendedResources)
extendedResources = filterExtendedResources(features, extendedResources)

var taints []corev1.Taint
if m.config.EnableTaints {
Expand Down Expand Up @@ -1157,6 +1148,7 @@ func (m *nfdMaster) configure(filepath string, overrides string) error {
m.deniedNs.normal = normalDeniedNs
m.deniedNs.wildcard = wildcardDeniedNs
// We forcibly deny kubernetes.io
m.deniedNs.normal[""] = struct{}{}
m.deniedNs.normal["kubernetes.io"] = struct{}{}
m.deniedNs.wildcard[".kubernetes.io"] = struct{}{}

Expand Down