From 976d0bd66381e1b1f17a5041edb34c73be4cf314 Mon Sep 17 00:00:00 2001 From: rosstimothy <39066650+rosstimothy@users.noreply.github.com> Date: Fri, 24 Mar 2023 14:44:39 -0400 Subject: [PATCH] Improve performance of `ListResources` (#23534) * Add benchmark for ListNodes * Move RBAC logging to trace level BenchmarkListNodes is twice as slow when RBAC logging is enabled. By switching RBAC logging from debug to trace we can eliminate the performance hit while still providing a way for users to opt in to the behavior if they need to debug RBAC. * Intern compiled regular expressions Profiles of the benchmark test revealed that the `regexp.Compile` done within `utils.matchString` was the most cpu and memory intensive portion of the tests. By leveraging a `lru.Cache` to intern the compiled regular expressions we get quite a performance improvement. * Only fetch a single page of resources Increases the request limit prior to loading the resources from the cache so that we load enough items in a single page to determine the start key of the next page. * Remove version checking from `services.UnmarshalServer` Unmarshal directly to a `types.ServerV2` instead of first creating a `types.ResourceHeader` to inspect the version. There is only a single version for `types.ServerV2` making the check unnecessary. * Add `GetLabel` to `types.ResourceWithLables` `GetAllLabels` can be overkill if one simply needs to look up the value for a particular label. It creates a new `map[string]string` and copies all of a resources existing labels. RBAC decisions driven by labels incurred the penalty of the copy each time access was checked. The impact of the copy is much more noticeable when a resource has several labels or really long strings in the key or value. By leveraging `GetLabel` RBAC can avoid copying the labels altogether and simply lookup each label key when required. --- api/types/access_request.go | 7 +++ api/types/app.go | 11 ++++ api/types/appserver.go | 13 ++++ api/types/database.go | 11 ++++ api/types/databaseserver.go | 13 ++++ api/types/kubernetes.go | 18 ++++++ api/types/kubernetes_server.go | 13 ++++ api/types/resource.go | 9 +++ api/types/server.go | 17 ++++- go.mod | 2 +- go.sum | 4 +- lib/auth/auth_with_roles.go | 5 ++ lib/auth/auth_with_roles_test.go | 104 +++++++++++++++++++++++++++++++ lib/auth/tls_test.go | 2 +- lib/backend/report.go | 11 +--- lib/services/role.go | 48 +++++++++----- lib/services/server.go | 48 ++++++-------- lib/utils/replace.go | 25 +++++++- 18 files changed, 302 insertions(+), 59 deletions(-) diff --git a/api/types/access_request.go b/api/types/access_request.go index a08c604c4bc2e..b8a09cdfc3bd5 100644 --- a/api/types/access_request.go +++ b/api/types/access_request.go @@ -412,6 +412,13 @@ func (r *AccessRequestV3) SetDryRun(dryRun bool) { r.Spec.DryRun = dryRun } +// GetLabel retrieves the label with the provided key. If not found +// value will be empty and ok will be false. +func (r *AccessRequestV3) GetLabel(key string) (value string, ok bool) { + v, ok := r.Metadata.Labels[key] + return v, ok +} + // GetStaticLabels returns the access request static labels. func (r *AccessRequestV3) GetStaticLabels() map[string]string { return r.Metadata.Labels diff --git a/api/types/app.go b/api/types/app.go index dbf78805a156f..95fb0a77bbbde 100644 --- a/api/types/app.go +++ b/api/types/app.go @@ -182,6 +182,17 @@ func (a *AppV3) SetDynamicLabels(dl map[string]CommandLabel) { a.Spec.DynamicLabels = LabelsToV2(dl) } +// GetLabel retrieves the label with the provided key. If not found +// value will be empty and ok will be false. +func (a *AppV3) GetLabel(key string) (value string, ok bool) { + if cmd, ok := a.Spec.DynamicLabels[key]; ok { + return cmd.Result, ok + } + + v, ok := a.Metadata.Labels[key] + return v, ok +} + // GetAllLabels returns the app combined static and dynamic labels. func (a *AppV3) GetAllLabels() map[string]string { return CombineLabels(a.Metadata.Labels, a.Spec.DynamicLabels) diff --git a/api/types/appserver.go b/api/types/appserver.go index 12d361e9d27a1..fb09c6a90bfe6 100644 --- a/api/types/appserver.go +++ b/api/types/appserver.go @@ -231,6 +231,19 @@ func (s *AppServerV3) SetProxyIDs(proxyIDs []string) { s.Spec.ProxyIDs = proxyIDs } +// GetLabel retrieves the label with the provided key. If not found +// value will be empty and ok will be false. +func (s *AppServerV3) GetLabel(key string) (value string, ok bool) { + if s.Spec.App != nil { + if v, ok := s.Spec.App.GetLabel(key); ok { + return v, ok + } + } + + v, ok := s.Metadata.Labels[key] + return v, ok +} + // GetAllLabels returns all resource's labels. Considering: // * Static labels from `Metadata.Labels` and `Spec.App`. // * Dynamic labels from `Spec.App.Spec`. diff --git a/api/types/database.go b/api/types/database.go index 05dcaa97e6a09..93cde98115f34 100644 --- a/api/types/database.go +++ b/api/types/database.go @@ -223,6 +223,17 @@ func (d *DatabaseV3) SetDynamicLabels(dl map[string]CommandLabel) { d.Spec.DynamicLabels = LabelsToV2(dl) } +// GetLabel retrieves the label with the provided key. If not found +// value will be empty and ok will be false. +func (d *DatabaseV3) GetLabel(key string) (value string, ok bool) { + if cmd, ok := d.Spec.DynamicLabels[key]; ok { + return cmd.Result, ok + } + + v, ok := d.Metadata.Labels[key] + return v, ok +} + // GetAllLabels returns the database combined static and dynamic labels. func (d *DatabaseV3) GetAllLabels() map[string]string { return CombineLabels(d.Metadata.Labels, d.Spec.DynamicLabels) diff --git a/api/types/databaseserver.go b/api/types/databaseserver.go index 11c3c859ade93..f7e30c1e037d8 100644 --- a/api/types/databaseserver.go +++ b/api/types/databaseserver.go @@ -266,6 +266,19 @@ func (s *DatabaseServerV3) SetOrigin(origin string) { s.Metadata.SetOrigin(origin) } +// GetLabel retrieves the label with the provided key. If not found +// value will be empty and ok will be false. +func (s *DatabaseServerV3) GetLabel(key string) (value string, ok bool) { + if s.Spec.Database != nil { + if v, ok := s.Spec.Database.GetLabel(key); ok { + return v, ok + } + } + + v, ok := s.Metadata.Labels[key] + return v, ok +} + // GetAllLabels returns all resource's labels. Considering: // * Static labels from `Metadata.Labels` and `Spec.Database`. // * Dynamic labels from `Spec.DynamicLabels`. diff --git a/api/types/kubernetes.go b/api/types/kubernetes.go index e949f79889bb9..5e1ae037233bc 100644 --- a/api/types/kubernetes.go +++ b/api/types/kubernetes.go @@ -195,6 +195,17 @@ func (k *KubernetesClusterV3) SetName(name string) { k.Metadata.Name = name } +// GetLabel retrieves the label with the provided key. If not found +// value will be empty and ok will be false. +func (k *KubernetesClusterV3) GetLabel(key string) (value string, ok bool) { + if cmd, ok := k.Spec.DynamicLabels[key]; ok { + return cmd.Result, ok + } + + v, ok := k.Metadata.Labels[key] + return v, ok +} + // GetStaticLabels returns the static labels. func (k *KubernetesClusterV3) GetStaticLabels() map[string]string { return k.Metadata.Labels @@ -581,6 +592,13 @@ func (k *KubernetesResourceV1) SetOrigin(origin string) { k.Metadata.SetOrigin(origin) } +// GetLabel retrieves the label with the provided key. If not found +// value will be empty and ok will be false. +func (k *KubernetesResourceV1) GetLabel(key string) (value string, ok bool) { + v, ok := k.Metadata.Labels[key] + return v, ok +} + // GetAllLabels returns all resource's labels. func (k *KubernetesResourceV1) GetAllLabels() map[string]string { return k.Metadata.Labels diff --git a/api/types/kubernetes_server.go b/api/types/kubernetes_server.go index bedc45878edb9..912e4cdc38007 100644 --- a/api/types/kubernetes_server.go +++ b/api/types/kubernetes_server.go @@ -278,6 +278,19 @@ func (s *KubernetesServerV3) SetProxyIDs(proxyIDs []string) { s.Spec.ProxyIDs = proxyIDs } +// GetLabel retrieves the label with the provided key. If not found +// value will be empty and ok will be false. +func (s *KubernetesServerV3) GetLabel(key string) (value string, ok bool) { + if s.Spec.Cluster != nil { + if v, ok := s.Spec.Cluster.GetLabel(key); ok { + return v, ok + } + } + + v, ok := s.Metadata.Labels[key] + return v, ok +} + // GetAllLabels returns all resource's labels. Considering: // * Static labels from `Metadata.Labels` and `Spec.Cluster`. // * Dynamic labels from `Spec.Cluster.Spec`. diff --git a/api/types/resource.go b/api/types/resource.go index 13f155e42dc05..8f956c7872789 100644 --- a/api/types/resource.go +++ b/api/types/resource.go @@ -86,6 +86,8 @@ type ResourceWithOrigin interface { type ResourceWithLabels interface { // ResourceWithOrigin is the base resource interface. ResourceWithOrigin + // GetLabel retrieves the label with the provided key. + GetLabel(key string) (value string, ok bool) // GetAllLabels returns all resource's labels. GetAllLabels() map[string]string // GetStaticLabels returns the resource's static labels. @@ -318,6 +320,13 @@ func (h *ResourceHeader) SetStaticLabels(sl map[string]string) { h.Metadata.Labels = sl } +// GetLabel retrieves the label with the provided key. If not found +// value will be empty and ok will be false. +func (h *ResourceHeader) GetLabel(key string) (value string, ok bool) { + v, ok := h.Metadata.Labels[key] + return v, ok +} + // GetAllLabels returns all labels from the resource.. func (h *ResourceHeader) GetAllLabels() map[string]string { return h.Metadata.Labels diff --git a/api/types/server.go b/api/types/server.go index 1968784703236..f6d51eae73cbd 100644 --- a/api/types/server.go +++ b/api/types/server.go @@ -229,16 +229,29 @@ func (s *ServerV2) GetHostname() string { return s.Spec.Hostname } +// GetLabel retrieves the label with the provided key. If not found +// value will be empty and ok will be false. +func (s *ServerV2) GetLabel(key string) (value string, ok bool) { + if cmd, ok := s.Spec.CmdLabels[key]; ok { + return cmd.Result, ok + } + + v, ok := s.Metadata.Labels[key] + return v, ok +} + +// GetLabels returns server's static label key pairs. // GetLabels and GetStaticLabels are the same, and that is intentional. GetLabels // exists to preserve backwards compatibility, while GetStaticLabels exists to // implement ResourcesWithLabels. - -// GetLabels returns server's static label key pairs func (s *ServerV2) GetLabels() map[string]string { return s.Metadata.Labels } // GetStaticLabels returns the server static labels. +// GetLabels and GetStaticLabels are the same, and that is intentional. GetLabels +// exists to preserve backwards compatibility, while GetStaticLabels exists to +// implement ResourcesWithLabels. func (s *ServerV2) GetStaticLabels() map[string]string { return s.Metadata.Labels } diff --git a/go.mod b/go.mod index 42c453911cbd0..b6a9420caa09b 100644 --- a/go.mod +++ b/go.mod @@ -77,7 +77,7 @@ require ( github.com/gravitational/ttlmap v0.0.0-20171116003245-91fd36b9004c github.com/grpc-ecosystem/go-grpc-middleware/providers/openmetrics/v2 v2.0.0-rc.3 github.com/hashicorp/go-version v1.2.0 - github.com/hashicorp/golang-lru v0.6.0 + github.com/hashicorp/golang-lru/v2 v2.0.2 github.com/jackc/pgconn v1.13.0 github.com/jackc/pgerrcode v0.0.0-20220416144525-469b46aa5efa github.com/jackc/pgproto3/v2 v2.3.1 diff --git a/go.sum b/go.sum index 3b2727c0432c6..845512a6bc5d0 100644 --- a/go.sum +++ b/go.sum @@ -732,8 +732,8 @@ github.com/hashicorp/go-version v1.2.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09 github.com/hashicorp/go.net v0.0.1/go.mod h1:hjKkEWcCURg++eb33jQU7oqQcI9XDCnUzHA0oac0k90= github.com/hashicorp/golang-lru v0.5.0/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ4Ao+sR/qLZy8= github.com/hashicorp/golang-lru v0.5.1/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ4Ao+sR/qLZy8= -github.com/hashicorp/golang-lru v0.6.0 h1:uL2shRDx7RTrOrTCUZEGP/wJUFiUI8QT6E7z5o8jga4= -github.com/hashicorp/golang-lru v0.6.0/go.mod h1:iADmTwqILo4mZ8BN3D2Q6+9jd8WM5uGBxy+E8yxSoD4= +github.com/hashicorp/golang-lru/v2 v2.0.2 h1:Dwmkdr5Nc/oBiXgJS3CDHNhJtIHkuZ3DZF5twqnfBdU= +github.com/hashicorp/golang-lru/v2 v2.0.2/go.mod h1:QeFd9opnmA6QUJc5vARoKUSoFhyfM2/ZepoAG6RGpeM= github.com/hashicorp/hcl v1.0.0/go.mod h1:E5yfLk+7swimpb2L/Alb/PJmXilQ/rhwaUYs4T20WEQ= github.com/hashicorp/logutils v1.0.0/go.mod h1:QIAnNjmIWmVIIkWDTG1z5v++HQmx9WQRO+LraFDTW64= github.com/hashicorp/mdns v1.0.0/go.mod h1:tL+uN++7HEJ6SQLQ2/p+z2pH24WQKWjBPkE0mNTz8vQ= diff --git a/lib/auth/auth_with_roles.go b/lib/auth/auth_with_roles.go index 1fc46c02e2296..1e47d4cb22015 100644 --- a/lib/auth/auth_with_roles.go +++ b/lib/auth/auth_with_roles.go @@ -1412,6 +1412,11 @@ func (a *ServerWithRoles) ListResources(ctx context.Context, req proto.ListResou req.SearchKeywords = nil req.PredicateExpression = "" + // Increase the limit to one more than was requested so + // that an additional page load is not needed to determine + // the next key. + req.Limit++ + resourceChecker, err := a.newResourceAccessChecker(req.ResourceType) if err != nil { return nil, trace.Wrap(err) diff --git a/lib/auth/auth_with_roles_test.go b/lib/auth/auth_with_roles_test.go index 80ffb2d8b548b..63f8a74935226 100644 --- a/lib/auth/auth_with_roles_test.go +++ b/lib/auth/auth_with_roles_test.go @@ -21,6 +21,7 @@ import ( "crypto/tls" "crypto/x509/pkix" "fmt" + "io" "testing" "time" @@ -29,12 +30,14 @@ import ( "github.com/google/uuid" "github.com/gravitational/trace" "github.com/pquerna/otp/totp" + "github.com/sirupsen/logrus" "github.com/stretchr/testify/require" "github.com/gravitational/teleport" "github.com/gravitational/teleport/api/client/proto" "github.com/gravitational/teleport/api/constants" "github.com/gravitational/teleport/api/defaults" + apidefaults "github.com/gravitational/teleport/api/defaults" "github.com/gravitational/teleport/api/types" apievents "github.com/gravitational/teleport/api/types/events" "github.com/gravitational/teleport/api/types/installers" @@ -50,6 +53,7 @@ import ( "github.com/gravitational/teleport/lib/services" "github.com/gravitational/teleport/lib/session" "github.com/gravitational/teleport/lib/tlsca" + "github.com/gravitational/teleport/lib/utils" ) func TestGenerateUserCerts_MFAVerifiedFieldSet(t *testing.T) { @@ -1143,6 +1147,106 @@ func TestSessionRecordingConfigRBAC(t *testing.T) { }) } +// time go test ./lib/auth -bench=. -run=^$ -v +// goos: darwin +// goarch: amd64 +// pkg: github.com/gravitational/teleport/lib/auth +// cpu: Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz +// BenchmarkListNodes +// BenchmarkListNodes-16 1 1000469673 ns/op 518721960 B/op 8344858 allocs/op +// PASS +// ok github.com/gravitational/teleport/lib/auth 3.695s +// go test ./lib/auth -bench=. -run=^$ -v 19.02s user 3.87s system 244% cpu 9.376 total +func BenchmarkListNodes(b *testing.B) { + const nodeCount = 50_000 + const roleCount = 32 + + logger := logrus.StandardLogger() + logger.ReplaceHooks(make(logrus.LevelHooks)) + logrus.SetFormatter(utils.NewTestJSONFormatter()) + logger.SetLevel(logrus.DebugLevel) + logger.SetOutput(io.Discard) + + ctx := context.Background() + srv := newTestTLSServer(b) + + var values []string + for i := 0; i < roleCount; i++ { + values = append(values, uuid.New().String()) + } + + values[0] = "hidden" + + var hiddenNodes int + // Create test nodes. + for i := 0; i < nodeCount; i++ { + name := uuid.New().String() + val := values[i%len(values)] + if val == "hidden" { + hiddenNodes++ + } + node, err := types.NewServerWithLabels( + name, + types.KindNode, + types.ServerSpecV2{}, + map[string]string{"key": val}, + ) + require.NoError(b, err) + + _, err = srv.Auth().UpsertNode(ctx, node) + require.NoError(b, err) + } + + testNodes, err := srv.Auth().GetNodes(ctx, defaults.Namespace) + require.NoError(b, err) + require.Len(b, testNodes, nodeCount) + + var roles []types.Role + for _, val := range values { + role, err := types.NewRole(fmt.Sprintf("role-%s", val), types.RoleSpecV6{}) + require.NoError(b, err) + + if val == "hidden" { + role.SetNodeLabels(types.Deny, types.Labels{"key": {val}}) + } else { + role.SetNodeLabels(types.Allow, types.Labels{"key": {val}}) + } + roles = append(roles, role) + } + + // create user, role, and client + username := "user" + + user, err := CreateUser(srv.Auth(), username, roles...) + require.NoError(b, err) + identity := TestUser(user.GetName()) + clt, err := srv.NewClient(identity) + require.NoError(b, err) + + b.ReportAllocs() + b.ResetTimer() + + for n := 0; n < b.N; n++ { + var resources []types.ResourceWithLabels + req := proto.ListResourcesRequest{ + ResourceType: types.KindNode, + Namespace: apidefaults.Namespace, + Limit: 1_000, + } + for { + rsp, err := clt.ListResources(ctx, req) + require.NoError(b, err) + + resources = append(resources, rsp.Resources...) + req.StartKey = rsp.NextKey + if req.StartKey == "" { + break + } + } + require.Len(b, resources, nodeCount-hiddenNodes) + } +} + // TestGetAndList_Nodes users can retrieve nodes with various filters // and with the appropriate permissions. func TestGetAndList_Nodes(t *testing.T) { diff --git a/lib/auth/tls_test.go b/lib/auth/tls_test.go index 25b3fcfd29abb..4eeb5b32cb970 100644 --- a/lib/auth/tls_test.go +++ b/lib/auth/tls_test.go @@ -3594,7 +3594,7 @@ func verifyJWT(clock clockwork.Clock, clusterName string, pairs []*types.JWTKeyP return nil, trace.NewAggregate(errs...) } -func newTestTLSServer(t *testing.T) *TestTLSServer { +func newTestTLSServer(t testing.TB) *TestTLSServer { as, err := NewTestAuthServer(TestAuthServerConfig{ Dir: t.TempDir(), Clock: clockwork.NewFakeClock(), diff --git a/lib/backend/report.go b/lib/backend/report.go index f3885eea1f2fe..4e12caa242c7d 100644 --- a/lib/backend/report.go +++ b/lib/backend/report.go @@ -22,7 +22,7 @@ import ( "time" "github.com/gravitational/trace" - lru "github.com/hashicorp/golang-lru" + lru "github.com/hashicorp/golang-lru/v2" "github.com/jonboulle/clockwork" "github.com/prometheus/client_golang/prometheus" log "github.com/sirupsen/logrus" @@ -81,7 +81,7 @@ type Reporter struct { // // This will keep an upper limit on our memory usage while still always // reporting the most active keys. - topRequestsCache *lru.Cache + topRequestsCache *lru.Cache[topRequestsCacheKey, struct{}] } // NewReporter returns a new Reporter. @@ -95,12 +95,7 @@ func NewReporter(cfg ReporterConfig) (*Reporter, error) { return nil, trace.Wrap(err) } - cache, err := lru.NewWithEvict(cfg.TopRequestsCount, func(key interface{}, value interface{}) { - labels, ok := key.(topRequestsCacheKey) - if !ok { - log.Errorf("BUG: invalid cache key type: %T", key) - return - } + cache, err := lru.NewWithEvict(cfg.TopRequestsCount, func(labels topRequestsCacheKey, value struct{}) { // Evict the key from requests metric. requests.DeleteLabelValues(labels.component, labels.key, labels.isRange) }) diff --git a/lib/services/role.go b/lib/services/role.go index b891fce4f6ed0..de9cd5de69707 100644 --- a/lib/services/role.go +++ b/lib/services/role.go @@ -1133,6 +1133,24 @@ func MatchDatabaseUser(selectors []string, user string, matchWildcard bool) (boo // MatchLabels matches selector against target. Empty selector matches // nothing, wildcard matches everything. func MatchLabels(selector types.Labels, target map[string]string) (bool, string, error) { + return MatchLabelGetter(selector, mapLabelGetter(target)) +} + +// LabelGetter allows retrieving a particular label by name. +type LabelGetter interface { + GetLabel(key string) (value string, ok bool) +} + +type mapLabelGetter map[string]string + +func (m mapLabelGetter) GetLabel(key string) (value string, ok bool) { + v, ok := m[key] + return v, ok +} + +// MatchLabelGetter matches selector against labelGetter. Empty selector matches +// nothing, wildcard matches everything. +func MatchLabelGetter(selector types.Labels, labelGetter LabelGetter) (bool, string, error) { // Empty selector matches nothing. if len(selector) == 0 { return false, "no match, empty selector", nil @@ -1146,19 +1164,20 @@ func MatchLabels(selector types.Labels, target map[string]string) (bool, string, // Perform full match. for key, selectorValues := range selector { - targetVal, hasKey := target[key] - + targetVal, hasKey := labelGetter.GetLabel(key) if !hasKey { return false, fmt.Sprintf("no key match: '%v'", key), nil } - if !slices.Contains(selectorValues, types.Wildcard) { - result, err := utils.SliceMatchesRegex(targetVal, selectorValues) - if err != nil { - return false, "", trace.Wrap(err) - } else if !result { - return false, fmt.Sprintf("no value match: got '%v' want: '%v'", targetVal, selectorValues), nil - } + if slices.Contains(selectorValues, types.Wildcard) { + continue + } + + result, err := utils.SliceMatchesRegex(targetVal, selectorValues) + if err != nil { + return false, "", trace.Wrap(err) + } else if !result { + return false, fmt.Sprintf("no value match: got '%v' want: '%v'", targetVal, selectorValues), nil } } @@ -2361,16 +2380,16 @@ type AccessCheckable interface { GetKind() string GetName() string GetMetadata() types.Metadata - GetAllLabels() map[string]string + GetLabel(key string) (value string, ok bool) } // rbacDebugLogger creates a debug logger for Teleport's RBAC component. // 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.DebugLevel) + isDebugEnabled := log.IsLevelEnabled(log.TraceLevel) log := log.WithField(trace.Component, teleport.ComponentRBAC) - return isDebugEnabled, log.Debugf + return isDebugEnabled, log.Tracef } // checkAccess checks if this role set has access to a particular resource r, @@ -2387,7 +2406,6 @@ func (set RoleSet) checkAccess(r AccessCheckable, state AccessState, matchers .. } namespace := types.ProcessNamespace(r.GetMetadata().Namespace) - allLabels := r.GetAllLabels() // Additional message depending on kind of resource // so there's more context on why the user might not have access. @@ -2425,7 +2443,7 @@ func (set RoleSet) checkAccess(r AccessCheckable, state AccessState, matchers .. continue } - matchLabels, labelsMessage, err := MatchLabels(getRoleLabels(role, types.Deny), allLabels) + matchLabels, labelsMessage, err := MatchLabelGetter(getRoleLabels(role, types.Deny), r) if err != nil { return trace.Wrap(err) } @@ -2469,7 +2487,7 @@ func (set RoleSet) checkAccess(r AccessCheckable, state AccessState, matchers .. continue } - matchLabels, labelsMessage, err := MatchLabels(getRoleLabels(role, types.Allow), allLabels) + matchLabels, labelsMessage, err := MatchLabelGetter(getRoleLabels(role, types.Allow), r) if err != nil { return trace.Wrap(err) } diff --git a/lib/services/server.go b/lib/services/server.go index e93ccb08c82c3..e19a64abb22fe 100644 --- a/lib/services/server.go +++ b/lib/services/server.go @@ -344,38 +344,28 @@ func UnmarshalServer(bytes []byte, kind string, opts ...MarshalOption) (types.Se return nil, trace.BadParameter("missing server data") } - var h types.ResourceHeader - if err = utils.FastUnmarshal(bytes, &h); err != nil { + var s types.ServerV2 + if err := utils.FastUnmarshal(bytes, &s); err != nil { + return nil, trace.BadParameter(err.Error()) + } + s.Kind = kind + if err := s.CheckAndSetDefaults(); err != nil { return nil, trace.Wrap(err) } - - switch h.Version { - case types.V2: - var s types.ServerV2 - - if err := utils.FastUnmarshal(bytes, &s); err != nil { - return nil, trace.BadParameter(err.Error()) - } - s.Kind = kind - if err := s.CheckAndSetDefaults(); err != nil { - return nil, trace.Wrap(err) - } - if cfg.ID != 0 { - s.SetResourceID(cfg.ID) - } - if !cfg.Expires.IsZero() { - s.SetExpiry(cfg.Expires) - } - if s.Metadata.Expires != nil { - apiutils.UTC(s.Metadata.Expires) - } - // Force the timestamps to UTC for consistency. - // See https://github.com/gogo/protobuf/issues/519 for details on issues this causes for proto.Clone - apiutils.UTC(&s.Spec.Rotation.Started) - apiutils.UTC(&s.Spec.Rotation.LastRotated) - return &s, nil + if cfg.ID != 0 { + s.SetResourceID(cfg.ID) + } + if !cfg.Expires.IsZero() { + s.SetExpiry(cfg.Expires) + } + if s.Metadata.Expires != nil { + apiutils.UTC(s.Metadata.Expires) } - return nil, trace.BadParameter("server resource version %q is not supported", h.Version) + // Force the timestamps to UTC for consistency. + // See https://github.com/gogo/protobuf/issues/519 for details on issues this causes for proto.Clone + apiutils.UTC(&s.Spec.Rotation.Started) + apiutils.UTC(&s.Spec.Rotation.LastRotated) + return &s, nil } // MarshalServer marshals the Server resource to JSON. diff --git a/lib/utils/replace.go b/lib/utils/replace.go index e13094aaddda1..bc3682fbe301a 100644 --- a/lib/utils/replace.go +++ b/lib/utils/replace.go @@ -19,6 +19,7 @@ import ( "strings" "github.com/gravitational/trace" + lru "github.com/hashicorp/golang-lru/v2" "github.com/gravitational/teleport/api/types" ) @@ -127,7 +128,27 @@ func SliceMatchesRegex(input string, expressions []string) (bool, error) { return false, nil } +// mustCache initializes a new [lru.Cache] with the provided size. +// A panic will be triggered if the creation of the cache fails. +func mustCache[K comparable, V any](size int) *lru.Cache[K, V] { + cache, err := lru.New[K, V](size) + if err != nil { + panic(err) + } + + return cache +} + +// exprCache interns compiled regular expressions created in matchString +// to improve performance. +var exprCache = mustCache[string, *regexp.Regexp](1000) + func matchString(input, expression string) (bool, error) { + if expr, ok := exprCache.Get(expression); ok { + return expr.MatchString(input), nil + } + + original := expression if !strings.HasPrefix(expression, "^") || !strings.HasSuffix(expression, "$") { // replace glob-style wildcards with regexp wildcards // for plain strings, and quote all characters that could @@ -140,8 +161,10 @@ func matchString(input, expression string) (bool, error) { return false, trace.BadParameter(err.Error()) } + exprCache.Add(original, expr) + // Since the expression is always surrounded by ^ and $ this is an exact - // match for either a a plain string (for example ^hello$) or for a regexp + // match for either a plain string (for example ^hello$) or for a regexp // (for example ^hel*o$). return expr.MatchString(input), nil }