Skip to content

Commit

Permalink
filters/auth: cache yaml config (#3225)
Browse files Browse the repository at this point in the history
* filters/auth: benchmark jwtMetrics filter creation

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>

* filters/auth: cache yaml config

Cache parsed yaml config for `jwtMetrics` and `oauthTokeninfoValidate`
filters to avoid parsing the same configuration over and over
when these filters are appended as default filters to all routes.

```
                          │     HEAD~1     │                 HEAD                 │
                          │     sec/op     │    sec/op     vs base                │
JwtMetrics_CreateFilter-8   30777.00n ± 6%   18.45n ± 11%  -99.94% (p=0.000 n=10)

                          │    HEAD~1    │                 HEAD                 │
                          │     B/op     │    B/op      vs base                 │
JwtMetrics_CreateFilter-8   24.33Ki ± 0%   0.00Ki ± 0%  -100.00% (p=0.000 n=10)

                          │   HEAD~1   │                HEAD                │
                          │ allocs/op  │ allocs/op  vs base                 │
JwtMetrics_CreateFilter-8   180.0 ± 0%    0.0 ± 0%  -100.00% (p=0.000 n=10)
```

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>

---------

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
  • Loading branch information
AlexanderYastrebov committed Sep 19, 2024
1 parent e085caa commit 7b74d47
Show file tree
Hide file tree
Showing 5 changed files with 259 additions and 42 deletions.
30 changes: 15 additions & 15 deletions filters/auth/jwt_metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,19 @@ import (
"slices"
"strings"

"github.com/ghodss/yaml"
"github.com/opentracing/opentracing-go"
"github.com/zalando/skipper/filters"
"github.com/zalando/skipper/filters/annotate"
"github.com/zalando/skipper/jwt"
)

type (
jwtMetricsSpec struct{}
jwtMetricsSpec struct {
yamlConfigParser[jwtMetricsFilter]
}

// jwtMetricsFilter implements [yamlConfig],
// make sure it is not modified after initialization.
jwtMetricsFilter struct {
Issuers []string `json:"issuers,omitempty"`
OptOutAnnotations []string `json:"optOutAnnotations,omitempty"`
Expand All @@ -27,34 +30,31 @@ type (
)

func NewJwtMetrics() filters.Spec {
return &jwtMetricsSpec{}
return &jwtMetricsSpec{
newYamlConfigParser[jwtMetricsFilter](64),
}
}

func (s *jwtMetricsSpec) Name() string {
return filters.JwtMetricsName
}

func (s *jwtMetricsSpec) CreateFilter(args []interface{}) (filters.Filter, error) {
f := &jwtMetricsFilter{}

if len(args) == 1 {
if config, ok := args[0].(string); !ok {
return nil, fmt.Errorf("requires single string argument")
} else if err := yaml.Unmarshal([]byte(config), f); err != nil {
return nil, fmt.Errorf("failed to parse configuration")
}
} else if len(args) > 1 {
return nil, fmt.Errorf("requires single string argument")
if len(args) == 0 {
return &jwtMetricsFilter{}, nil
}
return s.parseSingleArg(args)
}

func (f *jwtMetricsFilter) initialize() error {
for _, host := range f.OptOutHosts {
if r, err := regexp.Compile(host); err != nil {
return nil, fmt.Errorf("failed to compile opt-out host pattern: %q", host)
return fmt.Errorf("failed to compile opt-out host pattern: %q", host)
} else {
f.optOutHostsCompiled = append(f.optOutHostsCompiled, r)
}
}
return f, nil
return nil
}

func (f *jwtMetricsFilter) Request(ctx filters.FilterContext) {}
Expand Down
14 changes: 14 additions & 0 deletions filters/auth/jwt_metrics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -368,3 +368,17 @@ func marshalBase64JSON(t *testing.T, v any) string {
}
return base64.RawURLEncoding.EncodeToString(d)
}

func BenchmarkJwtMetrics_CreateFilter(b *testing.B) {
spec := auth.NewJwtMetrics()
args := []any{`{issuers: [foo, bar], optOutAnnotations: [oauth.disabled], optOutHosts: [ '^.+[.]domain[.]test$' ]}`}

_, err := spec.CreateFilter(args)
require.NoError(b, err)

b.ReportAllocs()
b.ResetTimer()
for i := 0; i < b.N; i++ {
_, _ = spec.CreateFilter(args)
}
}
59 changes: 32 additions & 27 deletions filters/auth/tokeninfo.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"strings"
"time"

"github.com/ghodss/yaml"
"github.com/opentracing/opentracing-go"
"github.com/zalando/skipper/filters"
"github.com/zalando/skipper/filters/annotate"
Expand Down Expand Up @@ -49,6 +48,8 @@ type (
tokeninfoSpec struct {
typ roleCheckType
options TokeninfoOptions

tokeninfoValidateYamlConfigParser *yamlConfigParser[tokeninfoValidateFilterConfig]
}

tokeninfoFilter struct {
Expand All @@ -60,11 +61,16 @@ type (

tokeninfoValidateFilter struct {
client tokeninfoClient
config struct {
OptOutAnnotations []string `json:"optOutAnnotations,omitempty"`
UnauthorizedResponse string `json:"unauthorizedResponse,omitempty"`
OptOutHosts []string `json:"optOutHosts,omitempty"`
}
config *tokeninfoValidateFilterConfig
}

// tokeninfoValidateFilterConfig implements [yamlConfig],
// make sure it is not modified after initialization.
tokeninfoValidateFilterConfig struct {
OptOutAnnotations []string `json:"optOutAnnotations,omitempty"`
UnauthorizedResponse string `json:"unauthorizedResponse,omitempty"`
OptOutHosts []string `json:"optOutHosts,omitempty"`

optOutHostsCompiled []*regexp.Regexp
}
)
Expand Down Expand Up @@ -167,9 +173,12 @@ func NewOAuthTokeninfoAnyKVWithOptions(to TokeninfoOptions) filters.Spec {
}

func NewOAuthTokeninfoValidate(to TokeninfoOptions) filters.Spec {
p := newYamlConfigParser[tokeninfoValidateFilterConfig](64)
return &tokeninfoSpec{
typ: checkOAuthTokeninfoValidate,
options: to,

tokeninfoValidateYamlConfigParser: &p,
}
}

Expand Down Expand Up @@ -244,10 +253,11 @@ func (s *tokeninfoSpec) CreateFilter(args []interface{}) (filters.Filter, error)
}

if s.typ == checkOAuthTokeninfoValidate {
if len(sargs) != 1 {
return nil, fmt.Errorf("requires single string argument")
config, err := s.tokeninfoValidateYamlConfigParser.parseSingleArg(args)
if err != nil {
return nil, err
}
return createTokeninfoValidateFilter(ac, sargs[0])
return &tokeninfoValidateFilter{client: ac, config: config}, nil
}

f := &tokeninfoFilter{typ: s.typ, client: ac, kv: make(map[string][]string)}
Expand All @@ -274,22 +284,6 @@ func (s *tokeninfoSpec) CreateFilter(args []interface{}) (filters.Filter, error)
return f, nil
}

func createTokeninfoValidateFilter(client tokeninfoClient, arg string) (filters.Filter, error) {
f := &tokeninfoValidateFilter{client: client}
if err := yaml.Unmarshal([]byte(arg), &f.config); err != nil {
return nil, fmt.Errorf("failed to parse configuration")
}

for _, host := range f.config.OptOutHosts {
if r, err := regexp.Compile(host); err != nil {
return nil, fmt.Errorf("failed to compile opt-out host pattern: %q", host)
} else {
f.optOutHostsCompiled = append(f.optOutHostsCompiled, r)
}
}
return f, nil
}

// String prints nicely the tokeninfoFilter configuration based on the
// configuration and check used.
func (f *tokeninfoFilter) String() string {
Expand Down Expand Up @@ -444,6 +438,17 @@ func (f *tokeninfoFilter) Request(ctx filters.FilterContext) {

func (f *tokeninfoFilter) Response(filters.FilterContext) {}

func (c *tokeninfoValidateFilterConfig) initialize() error {
for _, host := range c.OptOutHosts {
if r, err := regexp.Compile(host); err != nil {
return fmt.Errorf("failed to compile opt-out host pattern: %q", host)
} else {
c.optOutHostsCompiled = append(c.optOutHostsCompiled, r)
}
}
return nil
}

func (f *tokeninfoValidateFilter) Request(ctx filters.FilterContext) {
if _, ok := ctx.StateBag()[tokeninfoCacheKey]; ok {
return // tokeninfo was already validated by a preceding filter
Expand All @@ -458,9 +463,9 @@ func (f *tokeninfoValidateFilter) Request(ctx filters.FilterContext) {
}
}

if len(f.optOutHostsCompiled) > 0 {
if len(f.config.optOutHostsCompiled) > 0 {
host := ctx.Request().Host
for _, r := range f.optOutHostsCompiled {
for _, r := range f.config.optOutHostsCompiled {
if r.MatchString(host) {
return // opt-out from validation
}
Expand Down
84 changes: 84 additions & 0 deletions filters/auth/yamlconfig.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
package auth

import (
"fmt"

"github.com/ghodss/yaml"
)

// yamlConfigParser parses and caches yaml configurations of type T.
// Use [newYamlConfigParser] to create instances and ensure that *T implements [yamlConfig].
type yamlConfigParser[T any] struct {
initialize func(*T) error
cacheSize int
cache map[string]*T
}

// yamlConfig must be implemented by config value pointer type.
// It is used to initialize the value after parsing.
type yamlConfig interface {
initialize() error
}

// newYamlConfigParser creates a new parser with a given cache size.
func newYamlConfigParser[T any, PT interface {
*T
yamlConfig
}](cacheSize int) yamlConfigParser[T] {
// We want user to specify config type T but ensure that *T implements [yamlConfig].
//
// Type inference only works for functions but not for types
// (see https://github.com/golang/go/issues/57270 and https://github.com/golang/go/issues/51527)
// therefore we create instances using function with two type parameters
// but second parameter is inferred from the first so the caller does not have to specify it.
//
// To use *T.initialize we setup initialize field
return yamlConfigParser[T]{
initialize: func(v *T) error { return PT(v).initialize() },
cacheSize: cacheSize,
cache: make(map[string]*T, cacheSize),
}
}

// parseSingleArg calls [yamlConfigParser.parse] with the first string argument.
// If args slice does not contain a single string, it returns an error.
func (p *yamlConfigParser[T]) parseSingleArg(args []any) (*T, error) {
if len(args) != 1 {
return nil, fmt.Errorf("requires single string argument")
}
config, ok := args[0].(string)
if !ok {
return nil, fmt.Errorf("requires single string argument")
}
return p.parse(config)
}

// parse parses a yaml configuration or returns a cached value
// if the exact configuration was already parsed before.
// Returned value is shared by multiple callers and therefore must not be modified.
func (p *yamlConfigParser[T]) parse(config string) (*T, error) {
if v, ok := p.cache[config]; ok {
return v, nil
}

v := new(T)
if err := yaml.Unmarshal([]byte(config), v); err != nil {
return nil, err
}

if err := p.initialize(v); err != nil {
return nil, err
}

// evict random element if cache is full
if p.cacheSize > 0 && len(p.cache) == p.cacheSize {
for k := range p.cache {
delete(p.cache, k)
break
}
}

p.cache[config] = v

return v, nil
}
Loading

0 comments on commit 7b74d47

Please sign in to comment.