Skip to content

Commit

Permalink
Improve performance of resource filtering (#39724)
Browse files Browse the repository at this point in the history
* update benchmark tests

* Avoid creating a logrus logger if RBAC logging is not enabled

```bash

$ benchstat 03d741b33f153ff98d516b54a3e163bd443fdf63.txt new.txt
goos: darwin
goarch: arm64
pkg: github.com/gravitational/teleport/lib/auth
                                        │ 03d741b33f153ff98d516b54a3e163bd443fdf63.txt │              new.txt               │
                                        │                    sec/op                    │   sec/op     vs base               │
ListUnifiedResources/labels-12                                            281.6m ± 10%   279.1m ± 6%       ~ (p=0.631 n=10)
ListUnifiedResources/predicate_path-12                                    503.1m ±  2%   509.2m ± 1%       ~ (p=0.165 n=10)
ListUnifiedResources/predicate_index-12                                   504.9m ±  4%   494.5m ± 4%       ~ (p=0.190 n=10)
ListUnifiedResources/search_lower-12                                      335.7m ±  1%   320.4m ± 7%  -4.57% (p=0.002 n=10)
ListUnifiedResources/search_upper-12                                      340.2m ± 13%   330.9m ± 4%       ~ (p=0.105 n=10)
geomean                                                                   382.3m         375.3m       -1.83%

                                        │ 03d741b33f153ff98d516b54a3e163bd443fdf63.txt │               new.txt                │
                                        │                     B/op                     │     B/op      vs base                │
ListUnifiedResources/labels-12                                           158.75Mi ± 0%   85.50Mi ± 0%  -46.14% (p=0.000 n=10)
ListUnifiedResources/predicate_path-12                                    396.9Mi ± 0%   323.7Mi ± 0%  -18.45% (p=0.000 n=10)
ListUnifiedResources/predicate_index-12                                   396.9Mi ± 0%   323.6Mi ± 0%  -18.46% (p=0.000 n=10)
ListUnifiedResources/search_lower-12                                     158.62Mi ± 0%   85.33Mi ± 0%  -46.21% (p=0.000 n=10)
ListUnifiedResources/search_upper-12                                     159.58Mi ± 0%   86.31Mi ± 0%  -45.91% (p=0.000 n=10)
geomean                                                                   229.2Mi        145.8Mi       -36.38%

                                        │ 03d741b33f153ff98d516b54a3e163bd443fdf63.txt │               new.txt               │
                                        │                  allocs/op                   │  allocs/op   vs base                │
ListUnifiedResources/labels-12                                             2.412M ± 0%   1.662M ± 0%  -31.10% (p=0.000 n=10)
ListUnifiedResources/predicate_path-12                                     7.363M ± 0%   6.613M ± 0%  -10.19% (p=0.000 n=10)
ListUnifiedResources/predicate_index-12                                    7.212M ± 0%   6.462M ± 0%  -10.40% (p=0.000 n=10)
ListUnifiedResources/search_lower-12                                       3.162M ± 0%   2.412M ± 0%  -23.73% (p=0.000 n=10)
ListUnifiedResources/search_upper-12                                       3.312M ± 0%   2.562M ± 0%  -22.65% (p=0.000 n=10)
geomean                                                                    4.222M        3.376M       -20.03%
```

* Simplify and reduce memory usage of MatchResourceByFilters

```bash

$ benchstat 215637fd4bca413a43d3ae3db25e1aa382c4b032.txt new.txt
goos: darwin
goarch: arm64
pkg: github.com/gravitational/teleport/lib/auth
                                        │ 215637fd4bca413a43d3ae3db25e1aa382c4b032.txt │               new.txt                │
                                        │                    sec/op                    │    sec/op     vs base                │
ListUnifiedResources/labels-12                                             279.1m ± 6%   210.3m ±  2%  -24.66% (p=0.000 n=10)
ListUnifiedResources/predicate_path-12                                     509.2m ± 1%   448.5m ± 12%  -11.91% (p=0.009 n=10)
ListUnifiedResources/predicate_index-12                                    494.5m ± 4%   478.1m ±  2%        ~ (p=0.089 n=10)
ListUnifiedResources/search_lower-12                                       320.4m ± 7%   271.6m ±  4%  -15.24% (p=0.000 n=10)
ListUnifiedResources/search_upper-12                                       330.9m ± 4%   272.8m ±  4%  -17.57% (p=0.000 n=10)
geomean                                                                    375.3m        319.7m        -14.83%

                                        │ 215637fd4bca413a43d3ae3db25e1aa382c4b032.txt │               new.txt                │
                                        │                     B/op                     │     B/op      vs base                │
ListUnifiedResources/labels-12                                            85.50Mi ± 0%   74.03Mi ± 0%  -13.42% (p=0.000 n=10)
ListUnifiedResources/predicate_path-12                                    323.7Mi ± 0%   312.2Mi ± 0%   -3.56% (p=0.000 n=10)
ListUnifiedResources/predicate_index-12                                   323.6Mi ± 0%   312.2Mi ± 0%   -3.53% (p=0.000 n=10)
ListUnifiedResources/search_lower-12                                      85.33Mi ± 0%   73.88Mi ± 0%  -13.41% (p=0.000 n=10)
ListUnifiedResources/search_upper-12                                      86.31Mi ± 0%   74.85Mi ± 0%  -13.27% (p=0.000 n=10)
geomean                                                                   145.8Mi        131.9Mi        -9.57%

                                        │ 215637fd4bca413a43d3ae3db25e1aa382c4b032.txt │               new.txt               │
                                        │                  allocs/op                   │  allocs/op   vs base                │
ListUnifiedResources/labels-12                                             1.662M ± 0%   1.212M ± 0%  -27.09% (p=0.000 n=10)
ListUnifiedResources/predicate_path-12                                     6.613M ± 0%   6.162M ± 0%   -6.81% (p=0.000 n=10)
ListUnifiedResources/predicate_index-12                                    6.462M ± 0%   6.012M ± 0%   -6.96% (p=0.000 n=10)
ListUnifiedResources/search_lower-12                                       2.412M ± 0%   1.962M ± 0%  -18.66% (p=0.000 n=10)
ListUnifiedResources/search_upper-12                                       2.562M ± 0%   2.112M ± 0%  -17.57% (p=0.000 n=10)
geomean                                                                    3.376M        2.844M       -15.77%
```

* Optimize memory usage of CombineLabels

```bash

$ benchstat 6ef17028eb6f3d83cee141c4ecd9d30bfbf54460.txt new.txt
goos: darwin
goarch: arm64
pkg: github.com/gravitational/teleport/lib/auth
                                        │ 6ef17028eb6f3d83cee141c4ecd9d30bfbf54460.txt │               new.txt               │
                                        │                    sec/op                    │   sec/op     vs base                │
ListUnifiedResourcesFilter/labels-12                                            210.3m ±  2%   186.1m ± 3%  -11.48% (p=0.000 n=10)
ListUnifiedResourcesFilter/predicate_path-12                                    448.5m ± 12%   418.5m ± 3%   -6.68% (p=0.000 n=10)
ListUnifiedResourcesFilter/predicate_index-12                                   478.1m ±  2%   422.8m ± 4%  -11.56% (p=0.000 n=10)
ListUnifiedResourcesFilter/search_lower-12                                      271.6m ±  4%   295.1m ± 7%   +8.65% (p=0.001 n=10)
ListUnifiedResourcesFilter/search_upper-12                                      272.8m ±  4%   270.4m ± 4%        ~ (p=0.481 n=10)
geomean                                                                   319.7m         304.7m        -4.68%

                                        │ 6ef17028eb6f3d83cee141c4ecd9d30bfbf54460.txt │               new.txt                │
                                        │                     B/op                     │     B/op      vs base                │
ListUnifiedResourcesFilter/labels-12                                            74.03Mi ± 0%   25.95Mi ± 0%  -64.95% (p=0.000 n=10)
ListUnifiedResourcesFilter/predicate_path-12                                    312.2Mi ± 0%   264.1Mi ± 0%  -15.40% (p=0.000 n=10)
ListUnifiedResourcesFilter/predicate_index-12                                   312.2Mi ± 0%   264.1Mi ± 0%  -15.40% (p=0.000 n=10)
ListUnifiedResourcesFilter/search_lower-12                                      73.88Mi ± 0%   73.94Mi ± 0%   +0.07% (p=0.000 n=10)
ListUnifiedResourcesFilter/search_upper-12                                      74.85Mi ± 0%   74.84Mi ± 0%   -0.02% (p=0.015 n=10)
geomean                                                                   131.9Mi        100.0Mi       -24.15%

                                        │ 6ef17028eb6f3d83cee141c4ecd9d30bfbf54460.txt │               new.txt               │
                                        │                  allocs/op                   │  allocs/op   vs base                │
ListUnifiedResourcesFilter/labels-12                                            1211.8k ± 0%   911.7k ± 0%  -24.76% (p=0.000 n=10)
ListUnifiedResourcesFilter/predicate_path-12                                     6.162M ± 0%   5.862M ± 0%   -4.87% (p=0.000 n=10)
ListUnifiedResourcesFilter/predicate_index-12                                    6.012M ± 0%   5.712M ± 0%   -4.99% (p=0.000 n=10)
ListUnifiedResourcesFilter/search_lower-12                                       1.962M ± 0%   1.962M ± 0%   +0.00% (p=0.010 n=10)
ListUnifiedResourcesFilter/search_upper-12                                       2.112M ± 0%   2.112M ± 0%        ~ (p=0.423 n=10)
geomean                                                                    2.844M        2.633M        -7.42%
```

* Avoid combining labels in parser

```bash

$ benchstat 8ddd25edecca2cb8cb16dc76e921b984defdefff.txt new.txt
goos: darwin
goarch: arm64
pkg: github.com/gravitational/teleport/lib/auth
                                        │ 8ddd25edecca2cb8cb16dc76e921b984defdefff.txt │               new.txt               │
                                        │                    sec/op                    │   sec/op     vs base                │
ListUnifiedResourcesFilter/labels-12                                             186.1m ± 3%   161.1m ± 4%  -13.45% (p=0.000 n=10)
ListUnifiedResourcesFilter/predicate_path-12                                     418.5m ± 3%   383.6m ± 1%   -8.34% (p=0.000 n=10)
ListUnifiedResourcesFilter/predicate_index-12                                    422.8m ± 4%   396.6m ± 1%   -6.21% (p=0.000 n=10)
ListUnifiedResourcesFilter/search_lower-12                                       295.1m ± 7%   258.6m ± 2%  -12.35% (p=0.000 n=10)
ListUnifiedResourcesFilter/search_upper-12                                       270.4m ± 4%   266.1m ± 8%        ~ (p=0.796 n=10)
geomean                                                                    304.7m        278.9m        -8.48%

                                        │ 8ddd25edecca2cb8cb16dc76e921b984defdefff.txt │               new.txt               │
                                        │                     B/op                     │     B/op      vs base               │
ListUnifiedResourcesFilter/labels-12                                            25.95Mi ± 0%   25.93Mi ± 0%  -0.06% (p=0.000 n=10)
ListUnifiedResourcesFilter/predicate_path-12                                    264.1Mi ± 0%   264.1Mi ± 0%       ~ (p=0.393 n=10)
ListUnifiedResourcesFilter/predicate_index-12                                   264.1Mi ± 0%   264.1Mi ± 0%       ~ (p=0.971 n=10)
ListUnifiedResourcesFilter/search_lower-12                                      73.94Mi ± 0%   73.94Mi ± 0%       ~ (p=0.684 n=10)
ListUnifiedResourcesFilter/search_upper-12                                      74.84Mi ± 0%   74.84Mi ± 0%       ~ (p=0.971 n=10)
geomean                                                                   100.0Mi        100.0Mi       -0.01%

                                        │ 8ddd25edecca2cb8cb16dc76e921b984defdefff.txt │              new.txt               │
                                        │                  allocs/op                   │  allocs/op   vs base               │
ListUnifiedResourcesFilter/labels-12                                             911.7k ± 0%   911.7k ± 0%  -0.01% (p=0.000 n=10)
ListUnifiedResourcesFilter/predicate_path-12                                     5.862M ± 0%   5.862M ± 0%       ~ (p=0.959 n=10)
ListUnifiedResourcesFilter/predicate_index-12                                    5.712M ± 0%   5.712M ± 0%  -0.00% (p=0.037 n=10)
ListUnifiedResourcesFilter/search_lower-12                                       1.962M ± 0%   1.962M ± 0%       ~ (p=0.085 n=10)
ListUnifiedResourcesFilter/search_upper-12                                       2.112M ± 0%   2.112M ± 0%       ~ (p=0.402 n=10)
geomean                                                                    2.633M        2.633M       -0.00%
```

* Reduce memory usage of MatchSearch

```bash

$ benchstat f8aa6f9a7a26d34796a0ecb9d3eadf245c3b478d.txt new.txt
goos: darwin
goarch: arm64
pkg: github.com/gravitational/teleport/lib/auth
                                        │ f8aa6f9a7a26d34796a0ecb9d3eadf245c3b478d.txt │              new.txt               │
                                        │                    sec/op                    │   sec/op     vs base               │
ListUnifiedResourcesFilter/labels-12                                             161.1m ± 4%   168.7m ± 5%  +4.73% (p=0.043 n=10)
ListUnifiedResourcesFilter/predicate_path-12                                     383.6m ± 1%   389.4m ± 1%  +1.50% (p=0.000 n=10)
ListUnifiedResourcesFilter/predicate_index-12                                    396.6m ± 1%   397.7m ± 3%       ~ (p=0.190 n=10)
ListUnifiedResourcesFilter/search_lower-12                                       258.6m ± 2%   266.6m ± 3%  +3.07% (p=0.007 n=10)
ListUnifiedResourcesFilter/search_upper-12                                       266.1m ± 8%   259.2m ± 2%       ~ (p=0.052 n=10)
geomean                                                                    278.9m        282.7m       +1.36%

                                        │ f8aa6f9a7a26d34796a0ecb9d3eadf245c3b478d.txt │               new.txt                │
                                        │                     B/op                     │     B/op      vs base                │
ListUnifiedResourcesFilter/labels-12                                            25.93Mi ± 0%   25.94Mi ± 0%        ~ (p=0.089 n=10)
ListUnifiedResourcesFilter/predicate_path-12                                    264.1Mi ± 0%   264.1Mi ± 0%        ~ (p=0.853 n=10)
ListUnifiedResourcesFilter/predicate_index-12                                   264.1Mi ± 0%   264.1Mi ± 0%        ~ (p=0.927 n=10)
ListUnifiedResourcesFilter/search_lower-12                                      73.94Mi ± 0%   55.63Mi ± 0%  -24.77% (p=0.000 n=10)
ListUnifiedResourcesFilter/search_upper-12                                      74.84Mi ± 0%   56.53Mi ± 0%  -24.46% (p=0.000 n=10)
geomean                                                                   100.0Mi        89.33Mi       -10.68%

                                        │ f8aa6f9a7a26d34796a0ecb9d3eadf245c3b478d.txt │               new.txt               │
                                        │                  allocs/op                   │  allocs/op   vs base                │
ListUnifiedResourcesFilter/labels-12                                             911.7k ± 0%   911.7k ± 0%        ~ (p=0.469 n=10)
ListUnifiedResourcesFilter/predicate_path-12                                     5.862M ± 0%   5.862M ± 0%   -0.00% (p=0.019 n=10)
ListUnifiedResourcesFilter/predicate_index-12                                    5.712M ± 0%   5.712M ± 0%        ~ (p=0.305 n=10)
ListUnifiedResourcesFilter/search_lower-12                                       1.962M ± 0%   1.662M ± 0%  -15.29% (p=0.000 n=10)
ListUnifiedResourcesFilter/search_upper-12                                       2.112M ± 0%   1.812M ± 0%  -14.20% (p=0.000 n=10)
geomean                                                                    2.633M        2.470M        -6.18%
```

* Switch to using containsFold instead of ToLower and Contains

```bash

$ benchstat b4f5af277a8da79470673a9d4da95e1ae747deea.txt new.txt
goos: darwin
goarch: arm64
pkg: github.com/gravitational/teleport/lib/auth
                                        │ b4f5af277a8da79470673a9d4da95e1ae747deea.txt │               new.txt               │
                                        │                    sec/op                    │   sec/op     vs base                │
ListUnifiedResourcesFilter/labels-12                                             168.7m ± 5%   160.5m ± 2%   -4.84% (p=0.002 n=10)
ListUnifiedResourcesFilter/predicate_path-12                                     389.4m ± 1%   387.2m ± 2%        ~ (p=0.393 n=10)
ListUnifiedResourcesFilter/predicate_index-12                                    397.7m ± 3%   396.4m ± 1%        ~ (p=0.247 n=10)
ListUnifiedResourcesFilter/search_lower-12                                       266.6m ± 3%   214.8m ± 1%  -19.43% (p=0.000 n=10)
ListUnifiedResourcesFilter/search_upper-12                                       259.2m ± 2%   231.0m ± 1%  -10.89% (p=0.000 n=10)
geomean                                                                    282.7m        261.5m        -7.50%

                                        │ b4f5af277a8da79470673a9d4da95e1ae747deea.txt │               new.txt               │
                                        │                     B/op                     │     B/op      vs base               │
ListUnifiedResourcesFilter/labels-12                                            25.94Mi ± 0%   25.93Mi ± 0%  -0.04% (p=0.004 n=10)
ListUnifiedResourcesFilter/predicate_path-12                                    264.1Mi ± 0%   264.1Mi ± 0%       ~ (p=0.481 n=10)
ListUnifiedResourcesFilter/predicate_index-12                                   264.1Mi ± 0%   264.1Mi ± 0%       ~ (p=0.684 n=10)
ListUnifiedResourcesFilter/search_lower-12                                      55.63Mi ± 0%   53.42Mi ± 0%  -3.97% (p=0.000 n=10)
ListUnifiedResourcesFilter/search_upper-12                                      56.53Mi ± 0%   53.42Mi ± 0%  -5.50% (p=0.000 n=10)
geomean                                                                   89.33Mi        87.61Mi       -1.93%

                                        │ b4f5af277a8da79470673a9d4da95e1ae747deea.txt │               new.txt               │
                                        │                  allocs/op                   │  allocs/op   vs base                │
ListUnifiedResourcesFilter/labels-12                                             911.7k ± 0%   911.6k ± 0%   -0.00% (p=0.011 n=10)
ListUnifiedResourcesFilter/predicate_path-12                                     5.862M ± 0%   5.862M ± 0%        ~ (p=0.812 n=10)
ListUnifiedResourcesFilter/predicate_index-12                                    5.712M ± 0%   5.712M ± 0%        ~ (p=0.812 n=10)
ListUnifiedResourcesFilter/search_lower-12                                       1.662M ± 0%   1.212M ± 0%  -27.09% (p=0.000 n=10)
ListUnifiedResourcesFilter/search_upper-12                                       1.812M ± 0%   1.212M ± 0%  -33.12% (p=0.000 n=10)
geomean                                                                    2.470M        2.139M       -13.38%
```

* Perform RBAC only on matches

```bash

$ benchstat 874a1dd0b8090dd0ae6f22754f0f989a57f08293.txt new.txt
goos: darwin
goarch: arm64
pkg: github.com/gravitational/teleport/lib/auth
                                        │ 874a1dd0b8090dd0ae6f22754f0f989a57f08293.txt │               new.txt               │
                                        │                    sec/op                    │   sec/op     vs base                │
ListUnifiedResourcesFilter/labels-12                                            160.52m ± 2%   79.33m ± 3%  -50.58% (p=0.000 n=10)
ListUnifiedResourcesFilter/predicate_path-12                                     387.2m ± 2%   317.1m ± 8%  -18.10% (p=0.000 n=10)
ListUnifiedResourcesFilter/predicate_index-12                                    396.4m ± 1%   337.6m ± 2%  -14.84% (p=0.000 n=10)
ListUnifiedResourcesFilter/search_lower-12                                       214.8m ± 1%   154.4m ± 3%  -28.11% (p=0.000 n=10)
ListUnifiedResourcesFilter/search_upper-12                                       231.0m ± 1%   171.9m ± 3%  -25.57% (p=0.000 n=10)
geomean                                                                    261.5m        186.5m       -28.69%

                                        │ 874a1dd0b8090dd0ae6f22754f0f989a57f08293.txt │               new.txt                │
                                        │                     B/op                     │     B/op      vs base                │
ListUnifiedResourcesFilter/labels-12                                          26551.2Ki ± 0%   732.7Ki ± 1%  -97.24% (p=0.000 n=10)
ListUnifiedResourcesFilter/predicate_path-12                                    264.1Mi ± 0%   238.9Mi ± 0%   -9.54% (p=0.000 n=10)
ListUnifiedResourcesFilter/predicate_index-12                                   264.1Mi ± 0%   238.9Mi ± 0%   -9.54% (p=0.000 n=10)
ListUnifiedResourcesFilter/search_lower-12                                      53.42Mi ± 0%   28.22Mi ± 0%  -47.17% (p=0.000 n=10)
ListUnifiedResourcesFilter/search_upper-12                                      53.42Mi ± 0%   28.23Mi ± 0%  -47.16% (p=0.000 n=10)
geomean                                                                   87.61Mi        31.80Mi       -63.70%

                                        │ 874a1dd0b8090dd0ae6f22754f0f989a57f08293.txt │               new.txt               │
                                        │                  allocs/op                   │  allocs/op   vs base                │
ListUnifiedResourcesFilter/labels-12                                            911.65k ± 0%   11.45k ± 0%  -98.74% (p=0.000 n=10)
ListUnifiedResourcesFilter/predicate_path-12                                     5.862M ± 0%   4.962M ± 0%  -15.36% (p=0.000 n=10)
ListUnifiedResourcesFilter/predicate_index-12                                    5.712M ± 0%   4.812M ± 0%  -15.76% (p=0.000 n=10)
ListUnifiedResourcesFilter/search_lower-12                                      1211.8k ± 0%   311.7k ± 0%  -74.28% (p=0.000 n=10)
ListUnifiedResourcesFilter/search_upper-12                                      1211.8k ± 0%   311.7k ± 0%  -74.28% (p=0.000 n=10)
geomean                                                                    2.139M        484.0k       -77.38%
```

* Reduce allocations in (ServerV2) MatchSearch

```bash

$benchstat 4782fe42f72a5ed7628bdb4a94f16d5cb06b35bb.txt  new.txt
goos: darwin
goarch: arm64
pkg: github.com/gravitational/teleport/lib/auth
                                        │ 4782fe42f72a5ed7628bdb4a94f16d5cb06b35bb.txt │              new.txt               │
                                        │                    sec/op                    │   sec/op     vs base               │
ListUnifiedResourcesFilter/labels-12                                             79.33m ± 3%   77.79m ± 4%  -1.94% (p=0.019 n=10)
ListUnifiedResourcesFilter/predicate_path-12                                     317.1m ± 8%   307.0m ± 2%  -3.18% (p=0.023 n=10)
ListUnifiedResourcesFilter/predicate_index-12                                    337.6m ± 2%   310.2m ± 1%  -8.12% (p=0.000 n=10)
ListUnifiedResourcesFilter/search_lower-12                                       154.4m ± 3%   139.5m ± 2%  -9.65% (p=0.000 n=10)
ListUnifiedResourcesFilter/search_upper-12                                       171.9m ± 3%   154.9m ± 4%  -9.94% (p=0.000 n=10)
geomean                                                                    186.5m        174.1m       -6.63%

                                        │ 4782fe42f72a5ed7628bdb4a94f16d5cb06b35bb.txt │               new.txt                │
                                        │                     B/op                     │     B/op      vs base                │
ListUnifiedResourcesFilter/labels-12                                            732.7Ki ± 1%   731.0Ki ± 0%        ~ (p=0.481 n=10)
ListUnifiedResourcesFilter/predicate_path-12                                    238.9Mi ± 0%   238.9Mi ± 0%        ~ (p=0.123 n=10)
ListUnifiedResourcesFilter/predicate_index-12                                   238.9Mi ± 0%   238.9Mi ± 0%   -0.01% (p=0.001 n=10)
ListUnifiedResourcesFilter/search_lower-12                                      28.22Mi ± 0%   16.76Mi ± 0%  -40.60% (p=0.000 n=10)
ListUnifiedResourcesFilter/search_upper-12                                      28.23Mi ± 0%   16.77Mi ± 0%  -40.58% (p=0.000 n=10)
geomean                                                                   31.80Mi        25.81Mi       -18.84%

                                        │ 4782fe42f72a5ed7628bdb4a94f16d5cb06b35bb.txt │               new.txt               │
                                        │                  allocs/op                   │  allocs/op   vs base                │
ListUnifiedResourcesFilter/labels-12                                             11.45k ± 0%   11.44k ± 0%        ~ (p=0.077 n=10)
ListUnifiedResourcesFilter/predicate_path-12                                     4.962M ± 0%   4.962M ± 0%        ~ (p=0.060 n=10)
ListUnifiedResourcesFilter/predicate_index-12                                    4.812M ± 0%   4.812M ± 0%   -0.00% (p=0.015 n=10)
ListUnifiedResourcesFilter/search_lower-12                                       311.7k ± 0%   161.6k ± 0%  -48.15% (p=0.000 n=10)
ListUnifiedResourcesFilter/search_upper-12                                       311.7k ± 0%   161.6k ± 0%  -48.14% (p=0.000 n=10)
geomean                                                                    484.0k        372.1k       -23.11%
```

* Remove allocations from MatchLabels

* fix: TestServerLabels

* Add comment to containsFold

Co-authored-by: Forrest <[email protected]>

* fix: log and skip match failures

* fix: only create closure if required

* fix: func (a *AppServerOrSAMLIdPServiceProviderV1) GetLabel

* fix: validate provided predicate expressions

---------

Co-authored-by: Forrest <[email protected]>
  • Loading branch information
rosstimothy and fspmarshall committed Mar 25, 2024
1 parent e5ed940 commit c3d07df
Show file tree
Hide file tree
Showing 7 changed files with 71 additions and 59 deletions.
4 changes: 3 additions & 1 deletion api/types/appserver_or_saml_idp_sp.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}

Expand Down
36 changes: 22 additions & 14 deletions api/types/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand All @@ -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
}
}
Expand All @@ -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
Expand Down
31 changes: 21 additions & 10 deletions api/types/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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)
}

Expand Down
36 changes: 13 additions & 23 deletions lib/services/matchers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
//
Expand All @@ -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)
}
Expand All @@ -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
Expand Down
11 changes: 4 additions & 7 deletions lib/services/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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)
}
Expand All @@ -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
}
Expand All @@ -853,5 +852,3 @@ func NewResourceParser(resource types.ResourceWithLabels) (BoolPredicateParser,
type label struct {
key, value string
}

type labels map[string]string
11 changes: 8 additions & 3 deletions lib/services/role.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
1 change: 0 additions & 1 deletion lib/services/suite/presence_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit c3d07df

Please sign in to comment.