Opa: Help debug why caching not working as expected

[brantlk]

Expected Behavior

I've got Rego that's using http.send with "cache":true and I expect that the response will be cached so that when this Rego is evaluated again before the max-age is hit it will not make another http request.

Actual Behavior

Another http request is made.

Steps to Reproduce the Problem

OPA Version: openpolicyagent/opa:0.23.2-istio

The call to http.send is like:

response := http.send({"method": "get", "url": "http://istio-ingressgateway.istio-system/auth/realms/myrealm/protocol/openid-connect/certs", "cache": true})

The response to this request includes the cache-control header:

$ curl -i http://istio-ingressgateway.istio-system/auth/realms/myrealm/protocol/openid-connect/certs
HTTP/1.1 200 OK
content-type: application/json
content-length: 1462
date: Thu, 15 Oct 2020 22:36:31 GMT
x-envoy-upstream-service-time: 12
server: istio-envoy
cache-control: max-age=300

{"keys":[{"kid":"-8G ... elided since I don't think body content matters.

I configured opa with --log-level=debug but nothing for caching.

Additional Info

This is a new change I'm trying out to fetch the certs. I had to upgrade to opa 0.23 to get the cache option.

I tried writing a test using python that used a caching library (CacheControl) and it showed that the response was cached so it was happy with the response's cache-control header.

[brantlk]

Tips for debugging appreciated.

[ashutosh-narkar]

Currently when the OPA-Envoy plugin creates the Rego object it does not set the inter-query cache and hence http.send falls backs to using the intra-query cache. So the plugin does not support inter-query caching atm.

[brantlk]

well, that explains it. Is there already a plan to enhance the plugin?

[ashutosh-narkar]

It should be a pretty small addition something like below and then it should support inter-query caching.

$ git diff internal/internal.go
diff --git a/internal/internal.go b/internal/internal.go
index ed356a3b..771b3c80 100644
--- a/internal/internal.go
+++ b/internal/internal.go
@@ -38,6 +38,8 @@ import (
@@ -38,6 +38,8 @@ import (
        "github.com/open-policy-agent/opa/server"
        "github.com/open-policy-agent/opa/storage"
        "github.com/open-policy-agent/opa/util"
+
+       iCache "github.com/open-policy-agent/opa/topdown/cache"
 )

 const defaultAddr = ":9191"
@@ -104,10 +106,11 @@ func Validate(m *plugins.Manager, bs []byte) (*Config, error) {
 func New(m *plugins.Manager, cfg *Config) plugins.Plugin {

        plugin := &envoyExtAuthzGrpcServer{
-               manager:             m,
-               cfg:                 *cfg,
-               server:              grpc.NewServer(),
-               preparedQueryDoOnce: new(sync.Once),
+               manager:                m,
+               cfg:                    *cfg,
+               server:                 grpc.NewServer(),
+               preparedQueryDoOnce:    new(sync.Once),
+               interQueryBuiltinCache: iCache.NewInterQueryCache(m.InterQueryBuiltinCacheConfig()),
        }

        // Register Authorization Server
@@ -136,11 +139,12 @@ type Config struct {
 }

 type envoyExtAuthzGrpcServer struct {
-       cfg                 Config
-       server              *grpc.Server
-       manager             *plugins.Manager
-       preparedQuery       *rego.PreparedEvalQuery
-       preparedQueryDoOnce *sync.Once
+       cfg                    Config
+       server                 *grpc.Server
+       manager                *plugins.Manager
+       preparedQuery          *rego.PreparedEvalQuery
+       preparedQueryDoOnce    *sync.Once
+       interQueryBuiltinCache iCache.InterQueryCache
 }

 func (p *envoyExtAuthzGrpcServer) Start(ctx context.Context) error {
@@ -366,6 +370,7 @@ func (p *envoyExtAuthzGrpcServer) eval(ctx context.Context, input ast.Value, res
                        rego.EvalParsedInput(input),
                        rego.EvalTransaction(txn),
                        rego.EvalMetrics(result.metrics),
+                       rego.EvalInterQueryBuiltinCache(p.interQueryBuiltinCache),
                )

[srenatus]

@ashutosh-narkar Looks like a worthwhile change to me. Alternatively, we could document this restriction (what @brantlk expected is also what I had naively assumed 😅)...

[ashutosh-narkar]

I think we should add this to the plugin. The code changes are pretty minimal and we could probably add this in a couple of weeks unless someone picks it up sooner !

[srenatus]

unless someone picks it up sooner !

I'll do it, I think. Got some WIP together, just trying to come up with a test.

[srenatus]

⏩ the openpolicyagent/opa:latest-envoy image should have the change, as of 3 minutes ago 😃

[brantlk]

Thanks! I'll give this a try.

[brantlk]

Finally got around to giving this a try and it worked as expected.
Looking forward to this change being a release.

[srenatus]

@brantlk https://github.com/open-policy-agent/opa-envoy-plugin/releases/tag/v0.24.0-envoy-1 looks like it's been released 🎉