[v13] Improve performance of resource filtering

api/types/appserver_or_saml_idp_sp.go

@@ -193,7 +193,9 @@ func (a *AppServerOrSAMLIdPServiceProviderV1) GetLabel(key string) (value string
 		v, ok := appServer.Spec.App.Metadata.Labels[key]
 		return v, ok
 	} else {
-		return "", true
+		sp := a.GetSAMLIdPServiceProvider()
+		v, ok := sp.Metadata.Labels[key]
+		return v, ok
 	}
 }
 

api/types/resource.go

@@ -455,13 +455,8 @@ func (m *Metadata) CheckAndSetDefaults() error {
 // MatchLabels takes a map of labels and returns `true` if the resource has ALL
 // of them.
 func MatchLabels(resource ResourceWithLabels, labels map[string]string) bool {
-	if len(labels) == 0 {
-		return true
-	}
-
-	resourceLabels := resource.GetAllLabels()
-	for name, value := range labels {
-		if resourceLabels[name] != value {
+	for key, value := range labels {
+		if v, ok := resource.GetLabel(key); !ok || v != value {
 			return false
 		}
 	}
@@ -480,15 +475,11 @@ func IsValidLabelKey(s string) bool {
 // Returns true if all search vals were matched (or if nil search vals).
 // Returns false if no or partial match (or nil field values).
 func MatchSearch(fieldVals []string, searchVals []string, customMatch func(val string) bool) bool {
-	// Case fold all values to avoid repeated case folding while matching.
-	caseFoldedSearchVals := utils.ToLowerStrings(searchVals)
-	caseFoldedFieldVals := utils.ToLowerStrings(fieldVals)
-
 Outer:
-	for _, searchV := range caseFoldedSearchVals {
+	for _, searchV := range searchVals {
 		// Iterate through field values to look for a match.
-		for _, fieldV := range caseFoldedFieldVals {
-			if strings.Contains(fieldV, searchV) {
+		for _, fieldV := range fieldVals {
+			if containsFold(fieldV, searchV) {
 				continue Outer
 			}
 		}
@@ -504,6 +495,23 @@ Outer:
 	return true
 }
 
+// containsFold is a case-insensitive alternative to strings.Contains, used to help avoid excess allocations during searches.
+func containsFold(s, substr string) bool {
+	if len(s) < len(substr) {
+		return false
+	}
+
+	n := len(s) - len(substr)
+
+	for i := 0; i <= n; i++ {
+		if strings.EqualFold(s[i:i+len(substr)], substr) {
+			return true
+		}
+	}
+
+	return false
+}
+
 func stringCompare(a string, b string, isDesc bool) bool {
 	if isDesc {
 		return a > b

api/types/server.go

@@ -361,7 +361,11 @@ func (s *ServerV2) GetAllLabels() map[string]string {
 
 // CombineLabels combines the passed in static and dynamic labels.
 func CombineLabels(static map[string]string, dynamic map[string]CommandLabelV2) map[string]string {
-	lmap := make(map[string]string)
+	if len(dynamic) == 0 {
+		return static
+	}
+
+	lmap := make(map[string]string, len(static)+len(dynamic))
 	for key, value := range static {
 		lmap[key] = value
 	}
@@ -503,20 +507,27 @@ func (s *ServerV2) CheckAndSetDefaults() error {
 // MatchSearch goes through select field values and tries to
 // match against the list of search values.
 func (s *ServerV2) MatchSearch(values []string) bool {
-	var fieldVals []string
+	if s.GetKind() != KindNode {
+		return false
+	}
+
 	var custom func(val string) bool
+	if s.GetUseTunnel() {
+		custom = func(val string) bool {
+			return strings.EqualFold(val, "tunnel")
+		}
+	}
 
-	if s.GetKind() == KindNode {
-		fieldVals = append(utils.MapToStrings(s.GetAllLabels()), s.GetName(), s.GetHostname(), s.GetAddr())
-		fieldVals = append(fieldVals, s.GetPublicAddrs()...)
+	fieldVals := make([]string, 0, (len(s.Metadata.Labels)*2)+(len(s.Spec.CmdLabels)*2)+len(s.Spec.PublicAddrs)+3)
 
-		if s.GetUseTunnel() {
-			custom = func(val string) bool {
-				return strings.EqualFold(val, "tunnel")
-			}
-		}
+	labels := CombineLabels(s.Metadata.Labels, s.Spec.CmdLabels)
+	for key, value := range labels {
+		fieldVals = append(fieldVals, key, value)
 	}
 
+	fieldVals = append(fieldVals, s.Metadata.Name, s.Spec.Hostname, s.Spec.Addr)
+	fieldVals = append(fieldVals, s.Spec.PublicAddrs...)
+
 	return MatchSearch(fieldVals, values, custom)
 }
 

lib/services/matchers.go

@@ -125,7 +125,7 @@ func MatchResourceLabels(matchers []ResourceMatcher, labels map[string]string) b
 // ResourceSeenKey is used as a key for a map that keeps track
 // of unique resource names and address. Currently "addr"
 // only applies to resource Application.
-type ResourceSeenKey struct{ name, addr string }
+type ResourceSeenKey struct{ name, kind, addr string }
 
 // MatchResourceByFilters returns true if all filter values given matched against the resource.
 //
@@ -143,50 +143,40 @@ func MatchResourceByFilters(resource types.ResourceWithLabels, filter MatchResou
 
 	// We assume when filtering for services like KubeService, AppServer, and DatabaseServer
 	// the user is wanting to filter the contained resource ie. KubeClusters, Application, and Database.
-	resourceKey := ResourceSeenKey{}
+	key := ResourceSeenKey{
+		kind: filter.ResourceKind,
+	}
 	switch filter.ResourceKind {
 	case types.KindNode,
 		types.KindDatabaseService,
 		types.KindKubernetesCluster, types.KindKubePod,
 		types.KindWindowsDesktop, types.KindWindowsDesktopService,
 		types.KindUserGroup:
 		specResource = resource
-		resourceKey.name = specResource.GetName()
-
+		key.name = resource.GetName()
 	case types.KindKubeServer:
 		if seenMap != nil {
 			return false, trace.BadParameter("checking for duplicate matches for resource kind %q is not supported", filter.ResourceKind)
 		}
+		key.name = resource.GetName()
 		return matchAndFilterKubeClusters(resource, filter)
-
-	case types.KindAppServer:
-		server, ok := resource.(types.AppServer)
-		if !ok {
-			return false, trace.BadParameter("expected types.AppServer, got %T", resource)
-		}
-		specResource = server.GetApp()
-		app := server.GetApp()
-		resourceKey.name = app.GetName()
-		resourceKey.addr = app.GetPublicAddr()
-
 	case types.KindDatabaseServer:
 		server, ok := resource.(types.DatabaseServer)
 		if !ok {
 			return false, trace.BadParameter("expected types.DatabaseServer, got %T", resource)
 		}
 		specResource = server.GetDatabase()
-		resourceKey.name = specResource.GetName()
-
-	case types.KindAppOrSAMLIdPServiceProvider:
+		key.name = specResource.GetName()
+	case types.KindAppServer, types.KindSAMLIdPServiceProvider, types.KindAppOrSAMLIdPServiceProvider:
 		switch appOrSP := resource.(type) {
 		case types.AppServer:
 			app := appOrSP.GetApp()
 			specResource = app
-			resourceKey.name = app.GetName()
-			resourceKey.addr = app.GetPublicAddr()
+			key.addr = app.GetPublicAddr()
+			key.name = app.GetName()
 		case types.SAMLIdPServiceProvider:
 			specResource = appOrSP
-			resourceKey.name = appOrSP.GetName()
+			key.name = specResource.GetName()
 		default:
 			return false, trace.BadParameter("expected types.SAMLIdPServiceProvider or types.AppServer, got %T", resource)
 		}
@@ -210,10 +200,10 @@ func MatchResourceByFilters(resource types.ResourceWithLabels, filter MatchResou
 
 	// Deduplicate matches.
 	if match && seenMap != nil {
-		if _, exists := seenMap[resourceKey]; exists {
+		if _, exists := seenMap[key]; exists {
 			return false, nil
 		}
-		seenMap[resourceKey] = struct{}{}
+		seenMap[key] = struct{}{}
 	}
 
 	return match, nil

lib/services/parser.go

@@ -788,18 +788,17 @@ func NewResourceParser(resource types.ResourceWithLabels) (BoolPredicateParser,
 		GetIdentifier: func(fields []string) (interface{}, error) {
 			switch fields[0] {
 			case ResourceLabelsIdentifier:
-				combinedLabels := resource.GetAllLabels()
 				switch {
 				// Field length of 1 means the user is using
 				// an index expression ie: labels["env"], which the
 				// parser will expect a map for lookup in `GetProperty`.
 				case len(fields) == 1:
-					return labels(combinedLabels), nil
+					return resource, nil
 				case len(fields) > 2:
 					return nil, trace.BadParameter("only two fields are supported with identifier %q, got %d: %v", ResourceLabelsIdentifier, len(fields), fields)
 				default:
 					key := fields[1]
-					val, ok := combinedLabels[key]
+					val, ok := resource.GetLabel(key)
 					if ok {
 						return label{key: key, value: val}, nil
 					}
@@ -826,7 +825,7 @@ func NewResourceParser(resource types.ResourceWithLabels) (BoolPredicateParser,
 			}
 		},
 		GetProperty: func(mapVal, keyVal interface{}) (interface{}, error) {
-			m, ok := mapVal.(labels)
+			r, ok := mapVal.(types.ResourceWithLabels)
 			if !ok {
 				return GetStringMapValue(mapVal, keyVal)
 			}
@@ -836,7 +835,7 @@ func NewResourceParser(resource types.ResourceWithLabels) (BoolPredicateParser,
 				return nil, trace.BadParameter("only string keys are supported")
 			}
 
-			val, ok := m[key]
+			val, ok := r.GetLabel(key)
 			if ok {
 				return label{key: key, value: val}, nil
 			}
@@ -853,5 +852,3 @@ func NewResourceParser(resource types.ResourceWithLabels) (BoolPredicateParser,
 type label struct {
 	key, value string
 }
-
-type labels map[string]string

lib/services/role.go

@@ -2306,9 +2306,14 @@ type AccessCheckable interface {
 // It also returns a flag indicating whether debug logging is enabled,
 // allowing the RBAC system to generate more verbose errors in debug mode.
 func rbacDebugLogger() (debugEnabled bool, debugf func(format string, args ...interface{})) {
-	isDebugEnabled := log.IsLevelEnabled(log.TraceLevel)
-	log := log.WithField(trace.Component, teleport.ComponentRBAC)
-	return isDebugEnabled, log.Tracef
+	debugEnabled = log.IsLevelEnabled(log.TraceLevel)
+	debugf = func(format string, args ...interface{}) {}
+
+	if debugEnabled {
+		debugf = log.WithField(trace.Component, teleport.ComponentRBAC).Tracef
+	}
+
+	return
 }
 
 func (set RoleSet) checkAccess(r AccessCheckable, traits wrappers.Traits, state AccessState, matchers ...RoleMatcher) error {

lib/services/suite/presence_test.go

@@ -30,7 +30,6 @@ func TestServerLabels(t *testing.T) {
 	emptyLabels := make(map[string]string)
 	// empty
 	server := &types.ServerV2{}
-	require.Empty(t, cmp.Diff(server.GetAllLabels(), emptyLabels))
 	require.Empty(t, server.GetAllLabels())
 	require.Equal(t, types.MatchLabels(server, emptyLabels), true)
 	require.Equal(t, types.MatchLabels(server, map[string]string{"a": "b"}), false)