zalando / skipper

An HTTP router and reverse proxy for service composition, including use cases like Kubernetes Ingress
https://opensource.zalando.com/skipper/
Other
3.09k stars 350 forks source link

Facilitate OPA decision correlation with business flows #3041

Closed JanardhanSharma closed 3 months ago

JanardhanSharma commented 5 months ago

Context

Problem

Currently there is no recommended way of correlating OPA decisions with execution flows. This leaves the OPA integrators to take different approaches which could carry security risks and break as the platform evolves.

Correlation is required by two main use cases,

Solution

Improve the Skipper filter opaAuthorizeRequest to inject decisionID as an input to the policy. Add a new attribute to the filterMetadata called open_policy_agent and under that add the decision_id.

{
   "input":{
      "attributes":{
         "metadataContext":{
            "filterMetadata":{
               "open_policy_agent":{
                  "decision_id":"xxx.xx.xxx.xxx"
               }
            }
         },
         "request":{
         }
      }
   }
}

Similar solution for Envoy exists: https://github.com/open-policy-agent/opa/issues/6519 Ref: dynamic meta-data

Changes

Decision id is added to the request object and is available to be used in the policies.

szuecs commented 5 months ago

@JanardhanSharma please sign off your commits use -s, that's required for DCO. The issue link was non public. Please copy the description to the public such that it's self contained. Thanks!

szuecs commented 5 months ago
 --- FAIL: TestEval (0.10s)
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
    panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x22b9568]

goroutine 257 [running]:
testing.tRunner.func1.2({0x2486180, 0x34b9810})
    /opt/hostedtoolcache/go/1.22.2/x64/src/testing/testing.go:1631 +0x3f7
testing.tRunner.func1()
    /opt/hostedtoolcache/go/1.22.2/x64/src/testing/testing.go:1634 +0x6b6
panic({0x2486180?, 0x34b9810?})
    /opt/hostedtoolcache/go/1.22.2/x64/src/runtime/panic.go:770 +0x132
github.com/zalando/skipper/filters/openpolicyagent.metaDataContextDoesNotExist(...)
    /home/runner/work/skipper/skipper/filters/openpolicyagent/evaluation.go:81
github.com/zalando/skipper/filters/openpolicyagent.setDecisionIdInRequest(0xc0006a7800, {0xc000047d80, 0x20})
    /home/runner/work/skipper/skipper/filters/openpolicyagent/evaluation.go:73 +0x68
github.com/zalando/skipper/filters/openpolicyagent.(*OpenPolicyAgentInstance).Eval(0xc00035ef00, {0x29db2d0, 0xc0006a77d0}, 0xc0006a7800)
    /home/runner/work/skipper/skipper/filters/openpolicyagent/evaluation.go:27 +0x2ac
github.com/zalando/skipper/filters/openpolicyagent.TestEval(0xc00071f380)
    /home/runner/work/skipper/skipper/filters/openpolicyagent/openpolicyagent_test.go:424 +0x3d5
testing.tRunner(0xc00071f380, 0x2764af8)
    /opt/hostedtoolcache/go/1.22.2/x64/src/testing/testing.go:1689 +0x21f
created by testing.(*T).Run in goroutine 1
    /opt/hostedtoolcache/go/1.22.2/x64/src/testing/testing.go:1742 +0x826
JanardhanSharma commented 5 months ago

@szuecs Could you may be add @mjungsbluth as reviewer as well, please. He can have a quick look as well.

JanardhanSharma commented 5 months ago
 --- FAIL: TestEval (0.10s)
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
  panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x22b9568]

goroutine 257 [running]:
testing.tRunner.func1.2({0x2486180, 0x34b9810})
  /opt/hostedtoolcache/go/1.22.2/x64/src/testing/testing.go:1631 +0x3f7
testing.tRunner.func1()
  /opt/hostedtoolcache/go/1.22.2/x64/src/testing/testing.go:1634 +0x6b6
panic({0x2486180?, 0x34b9810?})
  /opt/hostedtoolcache/go/1.22.2/x64/src/runtime/panic.go:770 +0x132
github.com/zalando/skipper/filters/openpolicyagent.metaDataContextDoesNotExist(...)
  /home/runner/work/skipper/skipper/filters/openpolicyagent/evaluation.go:81
github.com/zalando/skipper/filters/openpolicyagent.setDecisionIdInRequest(0xc0006a7800, {0xc000047d80, 0x20})
  /home/runner/work/skipper/skipper/filters/openpolicyagent/evaluation.go:73 +0x68
github.com/zalando/skipper/filters/openpolicyagent.(*OpenPolicyAgentInstance).Eval(0xc00035ef00, {0x29db2d0, 0xc0006a77d0}, 0xc0006a7800)
  /home/runner/work/skipper/skipper/filters/openpolicyagent/evaluation.go:27 +0x2ac
github.com/zalando/skipper/filters/openpolicyagent.TestEval(0xc00071f380)
  /home/runner/work/skipper/skipper/filters/openpolicyagent/openpolicyagent_test.go:424 +0x3d5
testing.tRunner(0xc00071f380, 0x2764af8)
  /opt/hostedtoolcache/go/1.22.2/x64/src/testing/testing.go:1689 +0x21f
created by testing.(*T).Run in goroutine 1
  /opt/hostedtoolcache/go/1.22.2/x64/src/testing/testing.go:1742 +0x826

fixed

szuecs commented 4 months ago
taticcheck -checks "all,-ST1000,-ST1003,-ST1012,-ST1020,-ST1021" ./...
Error: filters/openpolicyagent/openpolicyagent_test.go:18:2: package "github.com/envoyproxy/go-control-plane/envoy/service/auth/v3" is being imported more than once (ST1019)
Error:  filters/openpolicyagent/openpolicyagent_test.go:19:2: other import of "github.com/envoyproxy/go-control-plane/envoy/service/auth/v3"
make: *** [Makefile:138: staticcheck] Error 1
Error: Process completed with exit code 2.
szuecs commented 4 months ago

:+1:

JanardhanSharma commented 3 months ago

@AlexanderYastrebov Could you please merge this please. This has been reviewed and approved long back, just left to be merged.

AlexanderYastrebov commented 3 months ago

@JanardhanSharma

Run make check-fmt
make: *** [Makefile:168: check-fmt] Error 1
Error: Process completed with exit code 2.

I think formatting needs to be fixed (via make fmt).

JanardhanSharma commented 3 months ago

@JanardhanSharma

Run make check-fmt
make: *** [Makefile:168: check-fmt] Error 1
Error: Process completed with exit code 2.

I think formatting needs to be fixed (via make fmt).

fixed. Please go ahead.

AlexanderYastrebov commented 3 months ago

:+1:

szuecs commented 3 months ago

:+1: