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 03d741b.txt new.txt
goos: darwin
goarch: arm64
pkg: github.com/gravitational/teleport/lib/auth
                                        │ 03d741b.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%

                                        │ 03d741b.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%

                                        │ 03d741b.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 215637f.txt new.txt
goos: darwin
goarch: arm64
pkg: github.com/gravitational/teleport/lib/auth
                                        │ 215637f.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%

                                        │ 215637f.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%

                                        │ 215637f.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 6ef1702.txt new.txt
goos: darwin
goarch: arm64
pkg: github.com/gravitational/teleport/lib/auth
                                        │ 6ef1702.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%

                                        │ 6ef1702.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%

                                        │ 6ef1702.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 8ddd25e.txt new.txt
goos: darwin
goarch: arm64
pkg: github.com/gravitational/teleport/lib/auth
                                        │ 8ddd25e.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%

                                        │ 8ddd25e.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%

                                        │ 8ddd25e.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 f8aa6f9.txt new.txt
goos: darwin
goarch: arm64
pkg: github.com/gravitational/teleport/lib/auth
                                        │ f8aa6f9.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%

                                        │ f8aa6f9.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%

                                        │ f8aa6f9.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 b4f5af2.txt new.txt
goos: darwin
goarch: arm64
pkg: github.com/gravitational/teleport/lib/auth
                                        │ b4f5af2.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%

                                        │ b4f5af2.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%

                                        │ b4f5af2.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 874a1dd.txt new.txt
goos: darwin
goarch: arm64
pkg: github.com/gravitational/teleport/lib/auth
                                        │ 874a1dd.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%

                                        │ 874a1dd.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%

                                        │ 874a1dd.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 4782fe4.txt  new.txt
goos: darwin
goarch: arm64
pkg: github.com/gravitational/teleport/lib/auth
                                        │ 4782fe4.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%

                                        │ 4782fe4.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%

                                        │ 4782fe4.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 <30576607+fspmarshall@users.noreply.github.com>

* 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 <30576607+fspmarshall@users.noreply.github.com>
  • Loading branch information
rosstimothy and fspmarshall committed Mar 25, 2024
1 parent 9d9edf2 commit 2b34a11
Show file tree
Hide file tree
Showing 9 changed files with 243 additions and 64 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 @@ -495,13 +495,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 Down Expand Up @@ -535,15 +530,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 @@ -559,6 +550,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 @@ -350,7 +350,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 @@ -492,20 +496,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
46 changes: 38 additions & 8 deletions lib/auth/auth_with_roles.go
Original file line number Diff line number Diff line change
Expand Up @@ -1329,18 +1329,34 @@ func (a *ServerWithRoles) checkUnifiedAccess(resource types.ResourceWithLabels,
return false, trace.Wrap(canAccessErr)
}

if resourceKind != types.KindSAMLIdPServiceProvider {
if err := checker.CanAccess(resource); err != nil {
// Filter first and only check RBAC if there is a match to improve perf.
match, err := services.MatchResourceByFilters(resource, filter, nil)
if err != nil {
log.WithFields(logrus.Fields{
"resource_name": resource.GetName(),
"resource_kind": resourceKind,
"error": err,
}).
Warn("Unable to determine access to resource, matching with filter failed")
return false, nil
}

if trace.IsAccessDenied(err) {
return false, nil
}
return false, trace.Wrap(err)
if !match {
return false, nil
}

if resourceKind == types.KindSAMLIdPServiceProvider {
return true, nil
}

if err := checker.CanAccess(resource); err != nil {
if trace.IsAccessDenied(err) {
return false, nil
}
return false, trace.Wrap(err)
}

match, err := services.MatchResourceByFilters(resource, filter, nil)
return match, trace.Wrap(err)
return true, nil
}

// ListUnifiedResources returns a paginated list of unified resources filtered by user access.
Expand All @@ -1358,6 +1374,20 @@ func (a *ServerWithRoles) ListUnifiedResources(ctx context.Context, req *proto.L
Kinds: req.Kinds,
}

// If a predicate expression was provided, evaluate it with an empty
// server to determine if the expression is valid before attempting
// to do any listing.
if filter.PredicateExpression != "" {
parser, err := services.NewResourceParser(&types.ServerV2{})
if err != nil {
return nil, trace.Wrap(err)
}

if _, err := parser.EvalBoolPredicate(filter.PredicateExpression); err != nil {
return nil, trace.BadParameter("failed to parse predicate expression: %s", err.Error())
}
}

// Populate resourceAccessMap with any access errors the user has for each possible
// resource kind. This allows the access check to occur a single time per resource
// kind instead of once per matching resource.
Expand Down
137 changes: 133 additions & 4 deletions lib/auth/auth_with_roles_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4721,6 +4721,130 @@ func TestListUnifiedResources_WithPredicate(t *testing.T) {
require.Error(t, err)
}

func BenchmarkListUnifiedResourcesFilter(b *testing.B) {
const nodeCount = 150_000
const roleCount = 32

logger := logrus.StandardLogger()
logger.ReplaceHooks(make(logrus.LevelHooks))
logrus.SetFormatter(logutils.NewTestJSONFormatter())
logger.SetLevel(logrus.PanicLevel)
logger.SetOutput(io.Discard)

ctx := context.Background()
srv := newTestTLSServer(b)

var ids []string
for i := 0; i < roleCount; i++ {
ids = append(ids, uuid.New().String())
}

ids[0] = "hidden"

var hiddenNodes int
// Create test nodes.
for i := 0; i < nodeCount; i++ {
name := uuid.New().String()
id := ids[i%len(ids)]
if id == "hidden" {
hiddenNodes++
}

labels := map[string]string{
"kEy": id,
"grouP": "useRs",
}

if i == 10 {
labels["ip"] = "10.20.30.40"
labels["ADDRESS"] = "10.20.30.41"
labels["food"] = "POTATO"
}

node, err := types.NewServerWithLabels(
name,
types.KindNode,
types.ServerSpecV2{},
labels,
)
require.NoError(b, err)

_, err = srv.Auth().UpsertNode(ctx, node)
require.NoError(b, err)
}

b.Run("labels", func(b *testing.B) {
benchmarkListUnifiedResources(
b, ctx,
1,
srv,
ids,
func(role types.Role, id string) {
role.SetNodeLabels(types.Allow, types.Labels{types.Wildcard: []string{types.Wildcard}})
},
func(req *proto.ListUnifiedResourcesRequest) {
req.Labels = map[string]string{"ip": "10.20.30.40"}
},
)
})
b.Run("predicate path", func(b *testing.B) {
benchmarkListUnifiedResources(
b, ctx,
1,
srv,
ids,
func(role types.Role, id string) {
role.SetNodeLabels(types.Allow, types.Labels{types.Wildcard: []string{types.Wildcard}})
},
func(req *proto.ListUnifiedResourcesRequest) {
req.PredicateExpression = `labels.ip == "10.20.30.40"`
},
)
})
b.Run("predicate index", func(b *testing.B) {
benchmarkListUnifiedResources(
b, ctx,
1,
srv,
ids,
func(role types.Role, id string) {
role.SetNodeLabels(types.Allow, types.Labels{types.Wildcard: []string{types.Wildcard}})
},
func(req *proto.ListUnifiedResourcesRequest) {
req.PredicateExpression = `labels["ip"] == "10.20.30.40"`
},
)
})
b.Run("search lower", func(b *testing.B) {
benchmarkListUnifiedResources(
b, ctx,
1,
srv,
ids,
func(role types.Role, id string) {
role.SetNodeLabels(types.Allow, types.Labels{types.Wildcard: []string{types.Wildcard}})
},
func(req *proto.ListUnifiedResourcesRequest) {
req.SearchKeywords = []string{"10.20.30.40"}
},
)
})
b.Run("search upper", func(b *testing.B) {
benchmarkListUnifiedResources(
b, ctx,
1,
srv,
ids,
func(role types.Role, id string) {
role.SetNodeLabels(types.Allow, types.Labels{types.Wildcard: []string{types.Wildcard}})
},
func(req *proto.ListUnifiedResourcesRequest) {
req.SearchKeywords = []string{"POTATO"}
},
)
})
}

// go test ./lib/auth -bench=BenchmarkListUnifiedResources -run=^$ -v -benchtime 1x
// goos: darwin
// goarch: arm64
Expand All @@ -4731,7 +4855,7 @@ func TestListUnifiedResources_WithPredicate(t *testing.T) {
// PASS
// ok github.com/gravitational/teleport/lib/auth 2.878s
func BenchmarkListUnifiedResources(b *testing.B) {
const nodeCount = 50_000
const nodeCount = 150_000
const roleCount = 32

logger := logrus.StandardLogger()
Expand Down Expand Up @@ -4791,21 +4915,23 @@ func BenchmarkListUnifiedResources(b *testing.B) {
b.Run(tc.desc, func(b *testing.B) {
benchmarkListUnifiedResources(
b, ctx,
nodeCount, hiddenNodes,
nodeCount-hiddenNodes,
srv,
ids,
tc.editRole,
func(req *proto.ListUnifiedResourcesRequest) {},
)
})
}
}

func benchmarkListUnifiedResources(
b *testing.B, ctx context.Context,
nodeCount, hiddenNodes int,
expectedCount int,
srv *TestTLSServer,
ids []string,
editRole func(r types.Role, id string),
editReq func(req *proto.ListUnifiedResourcesRequest),
) {
var roles []types.Role
for _, id := range ids {
Expand Down Expand Up @@ -4839,6 +4965,9 @@ func benchmarkListUnifiedResources(
SortBy: types.SortBy{IsDesc: false, Field: types.ResourceMetadataName},
Limit: 1_000,
}

editReq(req)

for {
rsp, err := clt.ListUnifiedResources(ctx, req)
require.NoError(b, err)
Expand All @@ -4849,7 +4978,7 @@ func benchmarkListUnifiedResources(
break
}
}
require.Len(b, resources, nodeCount-hiddenNodes)
require.Len(b, resources, expectedCount)
}
}

Expand Down

0 comments on commit 2b34a11

Please sign in to comment.