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

feat: add cache to hydrator #418

Merged
merged 11 commits into from
May 1, 2020
15 changes: 15 additions & 0 deletions .schema/config.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -843,6 +843,21 @@
"$ref": "#/definitions/retry"
}
}
},
"cache": {
"additionalProperties": false,
"required": [
"ttl"
],
"type": "object",
"properties": {
"ttl": {
"type": "string",
"pattern": "^[0-9]+(ns|us|ms|s|m|h)$",
"title": "Cache Time to Live",
"description": "How long to cache hydrate calls"
}
}
}
},
"required": [
Expand Down
5 changes: 5 additions & 0 deletions docs/docs/pipeline/mutator.md
Original file line number Diff line number Diff line change
Expand Up @@ -491,6 +491,7 @@ in the `header` field will be added to the final request headers.
- `api.url` (string - required) - The API URL.
- `api.auth.basic.*` (optional) - Enables HTTP Basic Authorization.
- `api.auth.retry.*` (optional) - Configures the retry logic.
- `cache.ttl` (optional) - Configures how long to cache hydrate requests

```yaml
# Global configuration file oathkeeper.yml
Expand All @@ -508,6 +509,8 @@ mutators:
retry:
give_up_after: 2s
max_delay: 100ms
cache:
ttl: 60s
```

```yaml
Expand All @@ -527,6 +530,8 @@ mutators:
retry:
give_up_after: 2s
max_delay: 100ms
cache:
ttl: 60s
```

### Access Rule Example
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
module github.com/ory/oathkeeper

require (
github.com/Azure/go-autorest/logger v0.1.0 // indirect
github.com/Masterminds/goutils v1.1.0 // indirect
github.com/Masterminds/sprig v2.20.0+incompatible
github.com/asaskevich/govalidator v0.0.0-20190424111038-f61b66f89f4a
Expand Down
5 changes: 5 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ cloud.google.com/go/datastore v1.0.0/go.mod h1:LXYbyblFSglQ5pkeyhO+Qmw7ukd3C+pD7
cloud.google.com/go/pubsub v1.0.1/go.mod h1:R0Gpsv3s54REJCy4fxDixWD93lHJMoZTyQ2kNxGRt3I=
github.com/Azure/go-ansiterm v0.0.0-20170929234023-d6e3b3328b78 h1:w+iIsaOQNcT7OZ575w+acHgRric5iCyQh+xv+KJ4HB8=
github.com/Azure/go-ansiterm v0.0.0-20170929234023-d6e3b3328b78/go.mod h1:LmzpDX56iTiv29bbRTIsUNlaFfuhWRQBWjQdVyAevI8=
github.com/Azure/go-autorest v14.0.1+incompatible h1:YhojO9jolWIvvTW7ORhz2ZSNF6Q1TbLqUunKd3jrtyw=
github.com/Azure/go-autorest/logger v0.1.0 h1:ruG4BSDXONFRrZZJ2GUXDiUyVpayPmb1GnWeHDdaNKY=
github.com/Azure/go-autorest/logger v0.1.0/go.mod h1:oExouG+K6PryycPJfVSxi/koC6LSNgds39diKLz7Vrc=
github.com/BurntSushi/toml v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ=
github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU=
github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802/go.mod h1:IVnqGOEym/WlBOVXweHU+Q+/VP0lqqI8lqeDx9IjBqo=
Expand Down Expand Up @@ -897,6 +900,7 @@ github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXf
github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
github.com/stretchr/testify v1.4.0 h1:2E4SXV/wtOkTonXsotYi4li6zVWxYlZuYNCXe9XRJyk=
github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
github.com/stretchr/testify v1.5.1 h1:nOGnQDM7FYENwehXlg/kFVnos3rEvtKTjRvOWSzb6H4=
github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA=
github.com/subosito/gotenv v1.1.1/go.mod h1:N0PQaV/YGNqwC0u51sEeR/aUtSLEXKX9iv69rRypqCw=
github.com/subosito/gotenv v1.2.0 h1:Slr1R9HxAlEKefgq5jn9U+DnETlIUa6HfgEzj0g5d7s=
Expand Down Expand Up @@ -1197,6 +1201,7 @@ google.golang.org/appengine v1.5.0/go.mod h1:xpcJRLb0r/rnEns0DIKYYv+WjYCduHsrkT7
google.golang.org/appengine v1.6.1/go.mod h1:i06prIuMbXzDqacNJfV5OdTW448YApPu5ww/cMBSeb0=
google.golang.org/appengine v1.6.2 h1:j8RI1yW0SkI+paT6uGwMlrMI/6zwYA6/CFil8rxOzGI=
google.golang.org/appengine v1.6.2/go.mod h1:i06prIuMbXzDqacNJfV5OdTW448YApPu5ww/cMBSeb0=
google.golang.org/appengine v1.6.5 h1:tycE03LOZYQNhDpS27tcQdAzLCVMaj7QT2SXxebnpCM=
google.golang.org/appengine v1.6.5/go.mod h1:8WjMMxjGQR8xUklV/ARdw2HLXBOI7O7uCIDZVag1xfc=
google.golang.org/genproto v0.0.0-20180817151627-c66870c02cf8/go.mod h1:JiN7NxoALGmiZfu7CAH4rXhgtRTLTxftemlI0sWmxmc=
google.golang.org/genproto v0.0.0-20190307195333-5fe7a883aa19/go.mod h1:VzzqZJRnGkLBvHegQrXjBqPurQTc5/KpmUdxsrq26oE=
Expand Down
59 changes: 57 additions & 2 deletions pipeline/mutate/mutator_hydrator.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,13 @@ package mutate
import (
"bytes"
"encoding/json"
"fmt"
"net/http"
"net/url"
"time"

"github.com/dgraph-io/ristretto"

"github.com/ory/oathkeeper/pipeline/authn"
"github.com/ory/oathkeeper/x"

Expand Down Expand Up @@ -55,6 +58,8 @@ type MutatorHydrator struct {
c configuration.Provider
client *http.Client
d mutatorHydratorDependencies

hydrateCache *ristretto.Cache
}

type BasicAuth struct {
Expand All @@ -77,22 +82,56 @@ type externalAPIConfig struct {
Retry *retryConfig `json:"retry"`
}

type cacheConfig struct {
Ttl string `json:"ttl"`
pike1212 marked this conversation as resolved.
Show resolved Hide resolved
}

type MutatorHydratorConfig struct {
Api externalAPIConfig `json:"api"`
Api externalAPIConfig `json:"api"`
Cache cacheConfig `json:"cache"`
}

type mutatorHydratorDependencies interface {
x.RegistryLogger
}

func NewMutatorHydrator(c configuration.Provider, d mutatorHydratorDependencies) *MutatorHydrator {
return &MutatorHydrator{c: c, d: d, client: httpx.NewResilientClientLatencyToleranceSmall(nil)}
cache, _ := ristretto.NewCache(&ristretto.Config{
NumCounters: 10000,
MaxCost: 1 << 25,
pike1212 marked this conversation as resolved.
Show resolved Hide resolved
BufferItems: 64,
pike1212 marked this conversation as resolved.
Show resolved Hide resolved
})
return &MutatorHydrator{c: c, d: d, client: httpx.NewResilientClientLatencyToleranceSmall(nil), hydrateCache: cache}
}

func (a *MutatorHydrator) GetID() string {
return "hydrator"
}

func (a *MutatorHydrator) cacheKey(config *MutatorHydratorConfig, session *authn.AuthenticationSession) string {
return fmt.Sprintf("%s|%s", config.Api.URL, session.Subject)
}

func (a *MutatorHydrator) hydrateFromCache(config *MutatorHydratorConfig, session *authn.AuthenticationSession) (*authn.AuthenticationSession, bool) {
key := a.cacheKey(config, session)

item, found := a.hydrateCache.Get(key)
if !found {
return nil, false
}

container := item.(*authn.AuthenticationSession)
return container, true
}

func (a *MutatorHydrator) hydrateToCache(config *MutatorHydratorConfig, session *authn.AuthenticationSession, ttl time.Duration) {
key := a.cacheKey(config, session)
cached := a.hydrateCache.SetWithTTL(key, session, 0, ttl)
if !cached {
a.d.Logger().Warn("Item not added to cache")
}
}

func (a *MutatorHydrator) Mutate(r *http.Request, session *authn.AuthenticationSession, config json.RawMessage, _ pipeline.Rule) error {
cfg, err := a.Config(config)
if err != nil {
Expand All @@ -104,6 +143,13 @@ func (a *MutatorHydrator) Mutate(r *http.Request, session *authn.AuthenticationS
return errors.WithStack(err)
}

if cfg.Cache.Ttl != "" {
Copy link
Member

Choose a reason for hiding this comment

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

Should we not also respsect Cache-Control headers? Maybe the server doesn't want us to cache the requests and sends Cache-Control: no-cache or some other cache value?

Copy link
Member

Choose a reason for hiding this comment

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

There's actually a http.NetTransport that does that: https://github.com/gregjones/httpcache

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That library will only work for GET requests. I imagine the hydrator is mostly used where someone is controlling both Oathkeeper and the hydrate service.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I see - yeah I think it makes sense. We can also add this at a later stage anyways as it won't be a breaking change.

if cacheSession, ok := a.hydrateFromCache(cfg, session); ok {
*session = *cacheSession
return nil
}
}

if cfg.Api.URL == "" {
return errors.New(ErrMissingAPIURL)
} else if _, err := url.ParseRequestURI(cfg.Api.URL); err != nil {
Expand Down Expand Up @@ -174,6 +220,15 @@ func (a *MutatorHydrator) Mutate(r *http.Request, session *authn.AuthenticationS
}
*session = sessionFromUpstream

if cfg.Cache.Ttl != "" {
pike1212 marked this conversation as resolved.
Show resolved Hide resolved
d, err := time.ParseDuration(cfg.Cache.Ttl)
if err != nil {
a.d.Logger().WithError(err).Error("Unable to parse cache ttl in the Hydrator Mutator.")
return errors.WithStack(err)
}
a.hydrateToCache(cfg, session, d)
}

return nil
}

Expand Down