search

open-policy-agent / opa

Open Policy Agent (OPA) is an open source, general-purpose policy engine.
https://www.openpolicyagent.org
Apache License 2.0
9.5k stars 1.32k forks source link

Race condition in bundle plugin #6849

Closed Pushpalanka closed 3 weeks ago

Pushpalanka commented 2 months ago

Short description

We are using OPA v0.65.0 as a library and encountered below race condition while using listeners on bundle plugin status. It seems there is a race between writes from reconfigure method and reads at persistbundle methods on bundle plugin configuration.

Error trace:

WARNING: DATA RACE
Write at 0x00c0000001a0 by goroutine 98:
  github.com/open-policy-agent/opa/plugins/bundle.(*Plugin).Reconfigure()
      /home/runner/go/pkg/mod/github.com/open-policy-agent/opa@v0.66.0/plugins/bundle/plugin.go:160 +0x147
  github.com/open-policy-agent/opa/plugins/discovery.(*Discovery).reconfigure()
      /home/runner/go/pkg/mod/github.com/open-policy-agent/opa@v0.66.0/plugins/discovery/discovery.go:405 +0x196
  github.com/open-policy-agent/opa/plugins/discovery.(*Discovery).processUpdate()
      /home/runner/go/pkg/mod/github.com/open-policy-agent/opa@v0.66.0/plugins/discovery/discovery.go:339 +0x513
  github.com/open-policy-agent/opa/plugins/discovery.(*Discovery).oneShot()
      /home/runner/go/pkg/mod/github.com/open-policy-agent/opa@v0.66.0/plugins/discovery/discovery.go:308 +0x8f
  github.com/open-policy-agent/opa/plugins/discovery.(*Discovery).oneShot-fm()
      <autogenerated>:1 +0x[75](https://github.com/zalando/skipper/actions/runs/9785181899/job/27017670664?pr=3120#step:6:76)
  github.com/open-policy-agent/opa/download.(*Downloader).oneShot()
      /home/runner/go/pkg/mod/github.com/open-policy-agent/opa@v0.66.0/download/download.go:279 +0x506
  github.com/open-policy-agent/opa/download.(*Downloader).loop()
      /home/runner/go/pkg/mod/github.com/open-policy-agent/opa@v0.66.0/download/download.go:227 +0x10d
  github.com/open-policy-agent/opa/download.(*Downloader).doStart.gowrap1()
      /home/runner/go/pkg/mod/github.com/open-policy-agent/opa@v0.66.0/download/download.go:191 +0x4f

Previous read at 0x00c0000001a0 by goroutine 107:
  github.com/open-policy-agent/opa/plugins/bundle.(*Plugin).persistBundle()
      /home/runner/go/pkg/mod/github.com/open-policy-agent/opa@v0.66.0/plugins/bundle/plugin.go:646 +0xe18
  github.com/open-policy-agent/opa/plugins/bundle.(*Plugin).process()
      /home/runner/go/pkg/mod/github.com/open-policy-agent/opa@v0.66.0/plugins/bundle/plugin.go:512 +0xd93
  github.com/open-policy-agent/opa/plugins/bundle.(*Plugin).oneShot()
      /home/runner/go/pkg/mod/github.com/open-policy-agent/opa@v0.66.0/plugins/bundle/plugin.go:453 +0x124
  github.com/open-policy-agent/opa/plugins/bundle.(*Plugin).newDownloader.func1()
      /home/runner/go/pkg/mod/github.com/open-policy-agent/opa@v0.66.0/plugins/bundle/plugin.go:425 +0xa4
  github.com/open-policy-agent/opa/download.(*Downloader).oneShot()
      /home/runner/go/pkg/mod/github.com/open-policy-agent/opa@v0.66.0/download/download.go:2[79](https://github.com/zalando/skipper/actions/runs/9785181899/job/27017670664?pr=3120#step:6:80) +0x506
  github.com/open-policy-agent/opa/download.(*Downloader).loop()
      /home/runner/go/pkg/mod/github.com/open-policy-agent/opa@v0.66.0/download/download.go:227 +0x10d
  github.com/open-policy-agent/opa/download.(*Downloader).doStart.gowrap1()
      /home/runner/go/pkg/mod/github.com/open-policy-agent/opa@v0.66.0/download/download.go:191 +0x4f

Goroutine 98 (running) created at:
  github.com/open-policy-agent/opa/download.(*Downloader).doStart()
      /home/runner/go/pkg/mod/github.com/open-policy-agent/opa@v0.66.0/download/download.go:191 +0x113
  github.com/open-policy-agent/opa/download.(*Downloader).Start.gowrap1()
      /home/runner/go/pkg/mod/github.com/open-policy-agent/opa@v0.66.0/download/download.go:1[82](https://github.com/zalando/skipper/actions/runs/9785181899/job/27017670664?pr=3120#step:6:83) +0x4f

Goroutine 107 (running) created at:
  github.com/open-policy-agent/opa/download.(*Downloader).doStart()
      /home/runner/go/pkg/mod/github.com/open-policy-agent/opa@v0.66.0/download/download.go:191 +0x113
  github.com/open-policy-agent/opa/download.(*Downloader).Start.gowrap1()
      /home/runner/go/pkg/mod/github.com/open-policy-agent/opa@v0.66.0/download/download.go:182 +0x4f
==================

cc: @mjungsbluth

ashutosh-narkar commented 2 months ago

One option would be to use the config mutex in places where the config is accessed.

mjungsbluth commented 2 months ago

@ashutosh-narkar that is possible but will be brittle given the number of code sites and entry points, we might worsen the situation by introducing deadlocks. We could change it to a RW Lock which would make it easier to control. Wdyt?

ashutosh-narkar commented 1 month ago

Sure. That should be fine.