Review comments: split filters, read in chunks

Signed-off-by: Magnus Jungsbluth <[email protected]>

config/config.go

@@ -281,7 +281,7 @@ type Config struct {
 	OpenPolicyAgentConfigTemplate  string        `yaml:"open-policy-agent-config-template"`
 	OpenPolicyAgentEnvoyMetadata   string        `yaml:"open-policy-agent-envoy-metadata"`
 	OpenPolicyAgentCleanerInterval time.Duration `yaml:"open-policy-agent-cleaner-interval"`
-	OpenPolicyAgentMaxBodySize     int           `yaml:"open-policy-agent-max-body-size"`
+	OpenPolicyAgentMaxBodySize     uint64        `yaml:"open-policy-agent-max-body-size"`
 }
 
 const (
@@ -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.IntVar(&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")
+	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")
 
 	// 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")

docs/reference/filters.md

@@ -1798,12 +1798,16 @@ Headers both to the upstream and the downstream service can be manipulated the s
 
 This allows both to add and remove unwanted headers in allow/deny cases.
 
-*Authorize based on request body*
+#### opaAuthorizeRequestWithBody
 
-Requests can also be authorized based on the request body the same way that is supported with the [Open Policy Agent Envoy plugin](https://www.openpolicyagent.org/docs/latest/envoy-primer/#example-input), look for the input attribute `parsed_body` in the upstream documentation. 
+Requests can also be authorized based on the request body the same way that is supported with the [Open Policy Agent Envoy plugin](https://www.openpolicyagent.org/docs/latest/envoy-primer/#example-input), look for the input attribute `parsed_body` in the upstream documentation.
+
+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 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. 
+
 #### opaServeResponse
 
 Always serves the response even if the policy allows the request and can customize the response completely. Can be used to re-implement legacy authorization services by already using data in Open Policy Agent but implementing an old REST API. This can also be useful to support Single Page Applications to return the calling users' permissions.
@@ -1847,6 +1851,16 @@ For this filter, the data flow looks like this independent of an allow/deny deci
 
 ```
 
+#### opaServeResponseWithReqBody
+
+If you want to serve requests directly from an Open Policy Agent policy that uses the request body, this can be done by using the `input.parsed_body` attribute the same way that is supported with the [Open Policy Agent Envoy plugin](https://www.openpolicyagent.org/docs/latest/envoy-primer/#example-input).
+
+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 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. 
+
 ## Cookie Handling
 ### dropRequestCookie
 

filters/filters.go

@@ -342,7 +342,9 @@ const (
 	ConsistentHashKeyName                      = "consistentHashKey"
 	ConsistentHashBalanceFactorName            = "consistentHashBalanceFactor"
 	OpaAuthorizeRequestName                    = "opaAuthorizeRequest"
+	OpaAuthorizeRequestWithBodyName            = "opaAuthorizeRequestWithBody"
 	OpaServeResponseName                       = "opaServeResponse"
+	OpaServeResponseWithReqBodyName            = "opaServeResponseWithReqBody"
 
 	// Undocumented filters
 	HealthCheckName        = "healthcheck"

filters/openpolicyagent/opaauthorizerequest/opaauthorizerequest.go

@@ -3,6 +3,7 @@ package opaauthorizerequest
 import (
 	"encoding/json"
 	"errors"
+	"io"
 	"net/http"
 	"time"
 
@@ -18,19 +19,31 @@ import (
 const responseHeadersKey = "open-policy-agent:decision-response-headers"
 
 type spec struct {
-	registry *openpolicyagent.OpenPolicyAgentRegistry
-	opts     []func(*openpolicyagent.OpenPolicyAgentInstanceConfig) error
+	registry    *openpolicyagent.OpenPolicyAgentRegistry
+	opts        []func(*openpolicyagent.OpenPolicyAgentInstanceConfig) error
+	name        string
+	bodyParsing bool
 }
 
 func NewOpaAuthorizeRequestSpec(registry *openpolicyagent.OpenPolicyAgentRegistry, opts ...func(*openpolicyagent.OpenPolicyAgentInstanceConfig) error) filters.Spec {
 	return &spec{
 		registry: registry,
 		opts:     opts,
+		name:     filters.OpaAuthorizeRequestName,
+	}
+}
+
+func NewOpaAuthorizeRequestWithBodySpec(registry *openpolicyagent.OpenPolicyAgentRegistry, opts ...func(*openpolicyagent.OpenPolicyAgentInstanceConfig) error) filters.Spec {
+	return &spec{
+		registry:    registry,
+		opts:        opts,
+		name:        filters.OpaAuthorizeRequestWithBodyName,
+		bodyParsing: true,
 	}
 }
 
 func (s *spec) Name() string {
-	return filters.OpaAuthorizeRequestName
+	return s.name
 }
 
 func (s *spec) CreateFilter(args []interface{}) (filters.Filter, error) {
@@ -78,26 +91,33 @@ func (s *spec) CreateFilter(args []interface{}) (filters.Filter, error) {
 		opa:                    opa,
 		registry:               s.registry,
 		envoyContextExtensions: envoyContextExtensions,
+		bodyParsing:            s.bodyParsing,
 	}, nil
 }
 
 type opaAuthorizeRequestFilter struct {
 	opa                    *openpolicyagent.OpenPolicyAgentInstance
 	registry               *openpolicyagent.OpenPolicyAgentRegistry
 	envoyContextExtensions map[string]string
+	bodyParsing            bool
 }
 
 func (f *opaAuthorizeRequestFilter) Request(fc filters.FilterContext) {
 	req := fc.Request()
 	span, ctx := f.opa.StartSpanFromFilterContext(fc)
 	defer span.Finish()
 
-	body, rawBodyBytes, err := f.opa.ExtractHttpBodyOptionally(req)
-	if err != nil {
-		f.opa.HandleInvalidDecisionError(fc, span, nil, err, !f.opa.EnvoyPluginConfig().DryRun)
-		return
+	var rawBodyBytes []byte
+	if f.bodyParsing {
+		var body io.ReadCloser
+		var err error
+		body, rawBodyBytes, err = f.opa.ExtractHttpBodyOptionally(req)
+		if err != nil {
+			f.opa.HandleInvalidDecisionError(fc, span, nil, err, !f.opa.EnvoyPluginConfig().DryRun)
+			return
+		}
+		req.Body = body
 	}
-	req.Body = body
 
 	authzreq := envoy.AdaptToExtAuthRequest(req, f.opa.InstanceConfig().GetEnvoyMetadata(), f.envoyContextExtensions, rawBodyBytes)
 

filters/openpolicyagent/opaauthorizerequest/opaauthorizerequest_test.go

@@ -20,6 +20,7 @@ import (
 func TestAuthorizeRequestFilter(t *testing.T) {
 	for _, ti := range []struct {
 		msg               string
+		filterName        string
 		bundleName        string
 		regoQuery         string
 		requestPath       string
@@ -35,6 +36,7 @@ func TestAuthorizeRequestFilter(t *testing.T) {
 	}{
 		{
 			msg:               "Allow Requests",
+			filterName:        "opaAuthorizeRequest",
 			bundleName:        "somebundle.tar.gz",
 			regoQuery:         "envoy/authz/allow",
 			requestPath:       "/allow",
@@ -48,6 +50,7 @@ func TestAuthorizeRequestFilter(t *testing.T) {
 		},
 		{
 			msg:               "Allow Matching Context Extension",
+			filterName:        "opaAuthorizeRequest",
 			bundleName:        "somebundle.tar.gz",
 			regoQuery:         "envoy/authz/allow_context_extensions",
 			requestPath:       "/allow",
@@ -61,6 +64,7 @@ func TestAuthorizeRequestFilter(t *testing.T) {
 		},
 		{
 			msg:               "Simple Forbidden",
+			filterName:        "opaAuthorizeRequest",
 			bundleName:        "somebundle.tar.gz",
 			regoQuery:         "envoy/authz/allow",
 			requestPath:       "/forbidden",
@@ -73,6 +77,7 @@ func TestAuthorizeRequestFilter(t *testing.T) {
 		},
 		{
 			msg:               "Allow With Structured Rules",
+			filterName:        "opaAuthorizeRequest",
 			bundleName:        "somebundle.tar.gz",
 			regoQuery:         "envoy/authz/allow_object",
 			requestPath:       "/allow/structured",
@@ -86,6 +91,7 @@ func TestAuthorizeRequestFilter(t *testing.T) {
 		},
 		{
 			msg:               "Forbidden With Body",
+			filterName:        "opaAuthorizeRequest",
 			bundleName:        "somebundle.tar.gz",
 			regoQuery:         "envoy/authz/allow_object",
 			requestPath:       "/forbidden",
@@ -99,6 +105,7 @@ func TestAuthorizeRequestFilter(t *testing.T) {
 		},
 		{
 			msg:               "Misconfigured Rego Query",
+			filterName:        "opaAuthorizeRequest",
 			bundleName:        "somebundle.tar.gz",
 			regoQuery:         "envoy/authz/invalid_path",
 			requestPath:       "/allow",
@@ -112,6 +119,7 @@ func TestAuthorizeRequestFilter(t *testing.T) {
 		},
 		{
 			msg:               "Wrong Query Data Type",
+			filterName:        "opaAuthorizeRequest",
 			bundleName:        "somebundle.tar.gz",
 			regoQuery:         "envoy/authz/allow_wrong_type",
 			requestPath:       "/allow",
@@ -125,6 +133,7 @@ func TestAuthorizeRequestFilter(t *testing.T) {
 		},
 		{
 			msg:               "Wrong Query Data Type",
+			filterName:        "opaAuthorizeRequest",
 			bundleName:        "somebundle.tar.gz",
 			regoQuery:         "envoy/authz/allow_object_invalid_headers_to_remove",
 			requestPath:       "/allow",
@@ -138,6 +147,7 @@ func TestAuthorizeRequestFilter(t *testing.T) {
 		},
 		{
 			msg:               "Wrong Query Data Type",
+			filterName:        "opaAuthorizeRequest",
 			bundleName:        "somebundle.tar.gz",
 			regoQuery:         "envoy/authz/allow_object_invalid_headers",
 			requestPath:       "/allow",
@@ -151,6 +161,7 @@ func TestAuthorizeRequestFilter(t *testing.T) {
 		},
 		{
 			msg:             "Allow With Body",
+			filterName:      "opaAuthorizeRequestWithBody",
 			bundleName:      "somebundle.tar.gz",
 			regoQuery:       "envoy/authz/allow_body",
 			requestMethod:   "POST",
@@ -165,6 +176,7 @@ func TestAuthorizeRequestFilter(t *testing.T) {
 		},
 		{
 			msg:             "Forbidden With Body",
+			filterName:      "opaAuthorizeRequestWithBody",
 			bundleName:      "somebundle.tar.gz",
 			regoQuery:       "envoy/authz/allow_body",
 			requestMethod:   "POST",
@@ -179,6 +191,7 @@ func TestAuthorizeRequestFilter(t *testing.T) {
 		},
 		{
 			msg:             "Broken Body",
+			filterName:      "opaAuthorizeRequestWithBody",
 			bundleName:      "somebundle.tar.gz",
 			regoQuery:       "envoy/authz/allow_body",
 			requestMethod:   "POST",
@@ -291,8 +304,10 @@ func TestAuthorizeRequestFilter(t *testing.T) {
 			opaFactory := openpolicyagent.NewOpenPolicyAgentRegistry()
 			ftSpec := NewOpaAuthorizeRequestSpec(opaFactory, openpolicyagent.WithConfigTemplate(config))
 			fr.Register(ftSpec)
+			ftSpec = NewOpaAuthorizeRequestWithBodySpec(opaFactory, openpolicyagent.WithConfigTemplate(config))
+			fr.Register(ftSpec)
 
-			r := eskip.MustParse(fmt.Sprintf(`* -> opaAuthorizeRequest("%s", "%s") -> "%s"`, ti.bundleName, ti.contextExtensions, clientServer.URL))
+			r := eskip.MustParse(fmt.Sprintf(`* -> %s("%s", "%s") -> "%s"`, ti.filterName, ti.bundleName, ti.contextExtensions, clientServer.URL))
 
 			proxy := proxytest.New(fr, r...)
 

filters/openpolicyagent/opaserveresponse/opaserveresponse.go

@@ -1,6 +1,7 @@
 package opaserveresponse
 
 import (
+	"io"
 	"time"
 
 	"github.com/zalando/skipper/filters"
@@ -11,19 +12,32 @@ import (
 )
 
 type spec struct {
-	registry *openpolicyagent.OpenPolicyAgentRegistry
-	opts     []func(*openpolicyagent.OpenPolicyAgentInstanceConfig) error
+	registry    *openpolicyagent.OpenPolicyAgentRegistry
+	opts        []func(*openpolicyagent.OpenPolicyAgentInstanceConfig) error
+	name        string
+	bodyParsing bool
 }
 
 func NewOpaServeResponseSpec(registry *openpolicyagent.OpenPolicyAgentRegistry, opts ...func(*openpolicyagent.OpenPolicyAgentInstanceConfig) error) filters.Spec {
 	return &spec{
-		registry: registry,
-		opts:     opts,
+		registry:    registry,
+		opts:        opts,
+		name:        filters.OpaServeResponseName,
+		bodyParsing: false,
+	}
+}
+
+func NewOpaServeResponseWithReqBodySpec(registry *openpolicyagent.OpenPolicyAgentRegistry, opts ...func(*openpolicyagent.OpenPolicyAgentInstanceConfig) error) filters.Spec {
+	return &spec{
+		registry:    registry,
+		opts:        opts,
+		name:        filters.OpaServeResponseWithReqBodyName,
+		bodyParsing: true,
 	}
 }
 
 func (s *spec) Name() string {
-	return filters.OpaServeResponseName
+	return s.name
 }
 
 func (s *spec) CreateFilter(args []interface{}) (filters.Filter, error) {
@@ -70,26 +84,33 @@ func (s *spec) CreateFilter(args []interface{}) (filters.Filter, error) {
 		opa:                    opa,
 		registry:               s.registry,
 		envoyContextExtensions: envoyContextExtensions,
+		bodyParsing:            s.bodyParsing,
 	}, nil
 }
 
 type opaServeResponseFilter struct {
 	opa                    *openpolicyagent.OpenPolicyAgentInstance
 	registry               *openpolicyagent.OpenPolicyAgentRegistry
 	envoyContextExtensions map[string]string
+	bodyParsing            bool
 }
 
 func (f *opaServeResponseFilter) Request(fc filters.FilterContext) {
 	span, ctx := f.opa.StartSpanFromFilterContext(fc)
 	defer span.Finish()
-
 	req := fc.Request()
-	body, rawBodyBytes, err := f.opa.ExtractHttpBodyOptionally(req)
-	if err != nil {
-		f.opa.ServeInvalidDecisionError(fc, span, nil, err)
-		return
+
+	var rawBodyBytes []byte
+	if f.bodyParsing {
+		var body io.ReadCloser
+		var err error
+		body, rawBodyBytes, err = f.opa.ExtractHttpBodyOptionally(req)
+		if err != nil {
+			f.opa.ServeInvalidDecisionError(fc, span, nil, err)
+			return
+		}
+		req.Body = body
 	}
-	req.Body = body
 
 	authzreq := envoy.AdaptToExtAuthRequest(fc.Request(), f.opa.InstanceConfig().GetEnvoyMetadata(), f.envoyContextExtensions, rawBodyBytes)