Review comments

Signed-off-by: Magnus Jungsbluth <magnus.jungsbluth@zalando.de>

config/config.go

@@ -498,7 +498,7 @@ func NewConfig() *Config {
 	flag.StringVar(&cfg.OpenPolicyAgentConfigTemplate, "open-policy-agent-config-template", "", "file containing a template for an Open Policy Agent configuration file that is interpolated for each OPA filter instance")
 	flag.StringVar(&cfg.OpenPolicyAgentEnvoyMetadata, "open-policy-agent-envoy-metadata", "", "JSON file containing meta-data passed as input for compatibility with Envoy policies in the format")
 	flag.DurationVar(&cfg.OpenPolicyAgentCleanerInterval, "open-policy-agent-cleaner-interval", openpolicyagent.DefaultCleanIdlePeriod, "JSON file containing meta-data passed as input for compatibility with Envoy policies in the format")
-	flag.Uint64Var(&cfg.OpenPolicyAgentMaxBodySize, "open-policy-agent-max-body-size", http.DefaultMaxHeaderBytes, "Maximum number of bytes from the body that are passed as input to the policy")
+	flag.Uint64Var(&cfg.OpenPolicyAgentMaxBodySize, "open-policy-agent-max-body-size", openpolicyagent.DefaultMaxBodySize, "Maximum number of bytes from the body that are passed as input to the policy")
 
 	// TLS client certs
 	flag.StringVar(&cfg.ClientKeyFile, "client-tls-key", "", "TLS Key file for backend connections, multiple keys may be given comma separated - the order must match the certs")

config/config_test.go

@@ -10,6 +10,7 @@ import (
 	"time"
 
 	log "github.com/sirupsen/logrus"
+	"github.com/zalando/skipper/filters/openpolicyagent"
 	"github.com/zalando/skipper/net"
 	"github.com/zalando/skipper/proxy"
 	"gopkg.in/yaml.v2"
@@ -160,8 +161,8 @@ func defaultConfig() *Config {
 		ValidateQueryLog:                        true,
 		LuaModules:                              commaListFlag(),
 		LuaSources:                              commaListFlag(),
-		OpenPolicyAgentCleanerInterval:          10 * time.Second,
-		OpenPolicyAgentMaxBodySize:              1048576,
+		OpenPolicyAgentCleanerInterval:          openpolicyagent.DefaultCleanIdlePeriod,
+		OpenPolicyAgentMaxBodySize:              openpolicyagent.DefaultMaxBodySize,
 	}
 }
 

docs/reference/filters.md

@@ -1804,7 +1804,7 @@ Requests can also be authorized based on the request body the same way that is s
 
 This filter has the same parameters that the `opaAuthorizeRequest` filter has.
 
-The body is parsed up to a maximum size that can be configured via the `-open-policy-agent-max-body-size` command line argument.
+The body is parsed up to a maximum size with a default of 1MB that can be configured via the `-open-policy-agent-max-body-size` command line argument.
 
 The filter also honors the `skip-request-body-parse` of the corresponding [configuration](https://www.openpolicyagent.org/docs/latest/envoy-introduction/#configuration) that the OPA plugin uses. 
 
@@ -1857,7 +1857,7 @@ If you want to serve requests directly from an Open Policy Agent policy that use
 
 This filter has the same parameters that the `opaServeResponse` filter has.
 
-The body is parsed up to a maximum size that can be configured via the `-open-policy-agent-max-body-size` command line argument.
+The body is parsed up to a maximum size with a default of 1MB that can be configured via the `-open-policy-agent-max-body-size` command line argument.
 
 The filter also honors the `skip-request-body-parse` of the corresponding [configuration](https://www.openpolicyagent.org/docs/latest/envoy-introduction/#configuration) that the OPA plugin uses. 
 

filters/openpolicyagent/opaauthorizerequest/opaauthorizerequest_test.go

@@ -21,6 +21,7 @@ func TestAuthorizeRequestFilter(t *testing.T) {
 	for _, ti := range []struct {
 		msg               string
 		filterName        string
+		extraeskip        string
 		bundleName        string
 		regoQuery         string
 		requestPath       string
@@ -204,6 +205,22 @@ func TestAuthorizeRequestFilter(t *testing.T) {
 			backendHeaders:  make(http.Header),
 			removeHeaders:   make(http.Header),
 		},
+		{
+			msg:             "Chained OPA filter with body",
+			filterName:      "opaAuthorizeRequestWithBody",
+			extraeskip:      `-> opaAuthorizeRequestWithBody("somebundle.tar.gz")`,
+			bundleName:      "somebundle.tar.gz",
+			regoQuery:       "envoy/authz/allow_body",
+			requestMethod:   "POST",
+			body:            `{ "target_id" : "123456" }`,
+			requestHeaders:  map[string][]string{"content-type": {"application/json"}},
+			requestPath:     "/allow_body",
+			expectedStatus:  http.StatusOK,
+			expectedBody:    "Welcome!",
+			expectedHeaders: make(http.Header),
+			backendHeaders:  make(http.Header),
+			removeHeaders:   make(http.Header),
+		},
 	} {
 		t.Run(ti.msg, func(t *testing.T) {
 			t.Logf("Running test for %v", ti)
@@ -307,7 +324,7 @@ func TestAuthorizeRequestFilter(t *testing.T) {
 			ftSpec = NewOpaAuthorizeRequestWithBodySpec(opaFactory, openpolicyagent.WithConfigTemplate(config))
 			fr.Register(ftSpec)
 
-			r := eskip.MustParse(fmt.Sprintf(`* -> %s("%s", "%s") -> "%s"`, ti.filterName, ti.bundleName, ti.contextExtensions, clientServer.URL))
+			r := eskip.MustParse(fmt.Sprintf(`* -> %s("%s", "%s") %s -> "%s"`, ti.filterName, ti.bundleName, ti.contextExtensions, ti.extraeskip, clientServer.URL))
 
 			proxy := proxytest.New(fr, r...)
 

filters/openpolicyagent/openpolicyagent.go

@@ -38,10 +38,10 @@ import (
 )
 
 const (
+	DefaultCleanIdlePeriod     = 10 * time.Second
+	DefaultMaxBodySize         = 1 << 20 // 1 MB
 	defaultReuseDuration       = 30 * time.Second
 	defaultShutdownGracePeriod = 30 * time.Second
-	DefaultCleanIdlePeriod     = 10 * time.Second
-	defaultMaxBodySize         = http.DefaultMaxHeaderBytes
 	defaultBodyBufferSize      = 8192 * 1024
 )
 
@@ -105,7 +105,7 @@ func NewOpenPolicyAgentRegistry(opts ...func(*OpenPolicyAgentRegistry) error) *O
 		instances:          make(map[string]*OpenPolicyAgentInstance),
 		lastused:           make(map[*OpenPolicyAgentInstance]time.Time),
 		quit:               make(chan struct{}),
-		maxBodyBytes:       defaultMaxBodySize,
+		maxBodyBytes:       DefaultMaxBodySize,
 		bodyReadBufferSize: defaultBodyBufferSize,
 	}
 
@@ -577,7 +577,7 @@ func (opa *OpenPolicyAgentInstance) ExtractHttpBodyOptionally(req *http.Request)
 	body := req.Body
 
 	if body != nil && !opa.EnvoyPluginConfig().SkipRequestBodyParse &&
-		opa.maxBodyBytes > 0 && req.ContentLength <= int64(opa.maxBodyBytes) {
+		req.ContentLength <= int64(opa.maxBodyBytes) {
 
 		bases, err := dependencies.Base(opa.Compiler(), opa.EnvoyPluginConfig().ParsedQuery)
 		if err != nil {