Skip to content

Commit

Permalink
Merge pull request #1202 from marquiz/devel/nfd-master-refactor
Browse files Browse the repository at this point in the history 
nfd-master: refactor filtering of labels, taints and ERs
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot authored May 1, 2023
2 parents e4dfa2d + fb20388 commit 62cd9b3
Showing 1 changed file with 91 additions and 99 deletions.
190 changes: 91 additions & 99 deletions pkg/nfd-master/nfd-master.go
View file
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 {
// 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

0 comments on commit 62cd9b3

Please sign in to comment.