Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

filters/auth: cache yaml config #3225

Merged
merged 2 commits into from
Sep 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need to enforce this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea is to ensure that user of yamlConfigParser is aware that config values must be immutable.

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
Loading