ewilliams28/releases
1
0
Fork 0
mirror of https://github.com/open-policy-agent/opa.git synced 2026-08-12 19:32:48 -06:00
Code Issues Packages Projects Releases Wiki Activity

Some small improvements to inmem storage (#7944)

Browse Source 
Mainly making transactions cheaper to create, and read transactions
much cheaper.

- Add exported RootPath shorthand var
- Don't return path on ParsePathEscaped failure
- Allocate nothing for read transactions, other than the transaction itself
- Lazy init of write update collections to avoid needless allocations
- Add benchmarks

**Before**
```
BenchmarkNewTransaction/Read-16                     26707234            44.78 ns/op      144 B/op          3 allocs/op
BenchmarkNewTransaction/Write-16                    20344212            59.44 ns/op      192 B/op          4 allocs/op
BenchmarkReadOne/Go_store_(roundtrip)-16            21963003            54.41 ns/op      144 B/op          3 allocs/op
BenchmarkReadOne/Go_store_(no_roundtrip)-16         22217593            54.18 ns/op      144 B/op          3 allocs/op
BenchmarkReadOne/AST_store_(roundtrip)-16           15626653            76.52 ns/op      160 B/op          4 allocs/op
BenchmarkReadOne/AST_store_(no_roundtrip)-16        15820837            76.15 ns/op      160 B/op          4 allocs/op
```

**After**
```
BenchmarkNewTransaction/Read-16                     68091271            17.37 ns/op       48 B/op          1 allocs/op
BenchmarkNewTransaction/Write-16                    24928028            47.68 ns/op      144 B/op          3 allocs/op
BenchmarkReadOne/Go_store_(roundtrip)-16            42967630            28.10 ns/op       48 B/op          1 allocs/op
BenchmarkReadOne/Go_store_(no_roundtrip)-16         43825009            27.63 ns/op       48 B/op          1 allocs/op
BenchmarkReadOne/AST_store_(roundtrip)-16           24885938            48.06 ns/op       64 B/op          2 allocs/op
BenchmarkReadOne/AST_store_(no_roundtrip)-16        25012396            47.96 ns/op       64 B/op          2 allocs/op
```

Signed-off-by: Anders Eknert <anders@eknert.com>
This commit is contained in:
Anders Eknert
2025-09-30 00:00:39 +02:00
committed by GitHub
parent 3527b57505
commit e1e2bfb876
17 changed files with 264 additions and 164 deletions
Download Patch File Download Diff File Expand all files Collapse all files
cmd/test.go
+1 -1
View File
@@ -302,7 +302,7 @@ func processWatcherUpdate(ctx context.Context, testParams testCommandParams, pat
err := pathwatcher.ProcessWatcherUpdateForRegoVersion(ctx, testParams.RegoVersion(), paths, removed, store, filter, testParams.bundleMode, false,
func(ctx context.Context, txn storage.Transaction, loaded *initload.LoadPathsResult) error {
if len(loaded.Files.Documents) > 0 || removed != "" {
if err := store.Write(ctx, txn, storage.AddOp, storage.Path{}, loaded.Files.Documents); err != nil {
if err := store.Write(ctx, txn, storage.AddOp, storage.RootPath, loaded.Files.Documents); err != nil {
return fmt.Errorf("storage error: %w", err)
}
}
internal/bundle/utils.go
+1 -1
View File
@@ -72,7 +72,7 @@ func LoadWasmResolversFromStore(ctx context.Context, store storage.Store, txn st
var resolvers []*wasm.Resolver
if len(resolversToLoad) > 0 {
// Get a full snapshot of the current data (including any from "outside" the bundles)
data, err := store.Read(ctx, txn, storage.Path{})
data, err := store.Read(ctx, txn, storage.RootPath)
if err != nil {
return nil, fmt.Errorf("failed to initialize wasm runtime: %s", err)
}
internal/presentation/presentation_test.go
+1 -1
View File
@@ -126,7 +126,7 @@ func TestOutputJSONErrorStructuredASTErr(t *testing.T) {
func TestOutputJSONErrorStructuredStorageErr(t *testing.T) {
store := inmem.New()
txn := storage.NewTransactionOrDie(t.Context(), store)
err := store.Write(t.Context(), txn, storage.AddOp, storage.Path{}, map[string]any{"foo": 1})
err := store.Write(t.Context(), txn, storage.AddOp, storage.RootPath, map[string]any{"foo": 1})
expected := `{
"errors": [
{
internal/runtime/init/init.go
+1 -1
View File
@@ -42,7 +42,7 @@ type InsertAndCompileResult struct {
// store contents.
func InsertAndCompile(ctx context.Context, opts InsertAndCompileOptions) (*InsertAndCompileResult, error) {
if len(opts.Files.Documents) > 0 {
if err := opts.Store.Write(ctx, opts.Txn, storage.AddOp, storage.Path{}, opts.Files.Documents); err != nil {
if err := opts.Store.Write(ctx, opts.Txn, storage.AddOp, storage.RootPath, opts.Files.Documents); err != nil {
return nil, fmt.Errorf("storage error: %w", err)
}
}
v1/debug/thread.go
+1 -1
View File
@@ -471,7 +471,7 @@ func (t *thread) inputVars(e *topdown.Event) VarRef {
func (t *thread) dataVars() VarRef {
return t.varManager.addVars(func() []namedVar {
ctx := context.Background()
d, err := storage.ReadOne(ctx, t.store, storage.Path{})
d, err := storage.ReadOne(ctx, t.store, storage.RootPath)
if err != nil {
return nil
}
v1/plugins/bundle/plugin_test.go
+17 -17
View File
@@ -109,7 +109,7 @@ func TestPluginOneShot(t *testing.T) {
t.Fatalf("Bad policy content. Exp:\n%v\n\nGot:\n\n%v", string(exp), string(bs))
}
data, err := manager.Store.Read(ctx, txn, storage.Path{})
data, err := manager.Store.Read(ctx, txn, storage.RootPath)
expData := util.MustUnmarshalJSON([]byte(`{
"foo": {"bar": 1, "baz": "qux"},
"system": {
@@ -163,7 +163,7 @@ func TestPluginOneShotWithAstStore(t *testing.T) {
txn := storage.NewTransactionOrDie(ctx, manager.Store)
defer manager.Store.Abort(ctx, txn)
data, err := manager.Store.Read(ctx, txn, storage.Path{})
data, err := manager.Store.Read(ctx, txn, storage.RootPath)
expData := ast.MustParseTerm(`{"foo": {"bar": 1, "baz": "qux"}, "system": {"bundles": {"test-bundle": {"etag": "foo", "manifest": {"revision": "quickbrownfaux", "roots": [""]}}}}}`)
if err != nil {
t.Fatal(err)
@@ -993,7 +993,7 @@ func TestPluginStartLazyLoadInMem(t *testing.T) {
t.Fatalf("Bad policy content. Exp:\n%v\n\nGot:\n\n%v", string(exp), string(bs))
}
data, err := manager.Store.Read(ctx, txn, storage.Path{})
data, err := manager.Store.Read(ctx, txn, storage.RootPath)
if err != nil {
t.Fatal(err)
}
@@ -1118,7 +1118,7 @@ func TestPluginOneShotDiskStorageMetrics(t *testing.T) {
t.Fatalf("Bad policy content. Exp:\n%v\n\nGot:\n\n%v", string(exp), string(bs))
}
data, err := manager.Store.Read(ctx, txn, storage.Path{})
data, err := manager.Store.Read(ctx, txn, storage.RootPath)
expData := util.MustUnmarshalJSON([]byte(`{
"foo": {"bar": 1, "baz": "qux"},
"system": {
@@ -1227,7 +1227,7 @@ func TestPluginOneShotDeltaBundle(t *testing.T) {
t.Fatalf("Bad policy content. Exp:\n%v\n\nGot:\n\n%v", string(exp), string(bs))
}
data, err := manager.Store.Read(ctx, txn, storage.Path{})
data, err := manager.Store.Read(ctx, txn, storage.RootPath)
if err != nil {
t.Fatal(err)
}
@@ -1338,7 +1338,7 @@ func TestPluginOneShotDeltaBundleWithAstStore(t *testing.T) {
t.Fatalf("Bad policy content. Exp:\n%v\n\nGot:\n\n%v", string(exp), string(bs))
}
data, err := manager.Store.Read(ctx, txn, storage.Path{})
data, err := manager.Store.Read(ctx, txn, storage.RootPath)
if err != nil {
t.Fatal(err)
}
@@ -1535,7 +1535,7 @@ func TestPluginOneShotBundlePersistence(t *testing.T) {
t.Fatalf("Bad policy content. Exp:\n%v\n\nGot:\n\n%v", string(exp), string(bs))
}
data, err := manager.Store.Read(ctx, txn, storage.Path{})
data, err := manager.Store.Read(ctx, txn, storage.RootPath)
expData := util.MustUnmarshalJSON([]byte(`{
"foo": {"bar": 1, "baz": "qux"},
"system": {
@@ -1731,7 +1731,7 @@ corge contains 1 if {
}
}`))
data, err := manager.Store.Read(ctx, txn, storage.Path{})
data, err := manager.Store.Read(ctx, txn, storage.RootPath)
if err != nil {
t.Fatal(err)
} else if !reflect.DeepEqual(data, expData) {
@@ -2017,7 +2017,7 @@ corge contains 1 if {
t.Fatalf("Bad policy content. Exp:\n%v\n\nGot:\n\n%v", string(exp), string(bs))
}
data, err := manager.Store.Read(ctx, txn, storage.Path{})
data, err := manager.Store.Read(ctx, txn, storage.RootPath)
var manifestRegoVersion string
var moduleRegoVersion string
@@ -2143,7 +2143,7 @@ func TestPluginOneShotSignedBundlePersistence(t *testing.T) {
t.Fatal("Expected no policy")
}
data, err := manager.Store.Read(ctx, txn, storage.Path{})
data, err := manager.Store.Read(ctx, txn, storage.RootPath)
if err != nil {
t.Fatal(err)
}
@@ -2228,7 +2228,7 @@ func TestLoadAndActivateBundlesFromDisk(t *testing.T) {
t.Fatalf("Bad policy content. Exp:\n%v\n\nGot:\n\n%v", string(exp), string(bs))
}
data, err := manager.Store.Read(ctx, txn, storage.Path{})
data, err := manager.Store.Read(ctx, txn, storage.RootPath)
expData := util.MustUnmarshalJSON([]byte(`{
"foo": {"bar": 1, "baz": "qux"},
"system": {
@@ -2314,7 +2314,7 @@ func TestLoadAndActivateBundlesFromDiskReservedChars(t *testing.T) {
t.Fatalf("Bad policy content. Exp:\n%v\n\nGot:\n\n%v", string(exp), string(bs))
}
data, err := manager.Store.Read(ctx, txn, storage.Path{})
data, err := manager.Store.Read(ctx, txn, storage.RootPath)
expData := util.MustUnmarshalJSON([]byte(`{
"foo": {"bar": 1, "baz": "qux"},
"system": {
@@ -2570,7 +2570,7 @@ corge contains 2 if {
}
}`))
data, err := manager.Store.Read(ctx, txn, storage.Path{})
data, err := manager.Store.Read(ctx, txn, storage.RootPath)
if err != nil {
fatal(err)
} else if !reflect.DeepEqual(data, expData) {
@@ -2781,7 +2781,7 @@ corge contains 1 if {
t.Fatalf("Bad policy content. Exp:\n%v\n\nGot:\n\n%v", string(exp), string(bs))
}
data, err := manager.Store.Read(ctx, txn, storage.Path{})
data, err := manager.Store.Read(ctx, txn, storage.RootPath)
manifestRegoVersionStr := ""
if tc.bundleRegoVersion != nil {
@@ -3231,7 +3231,7 @@ func TestPluginOneShotActivationRemovesOld(t *testing.T) {
} else if !slices.Equal([]string{filepath.Join(bundleName, "example2.rego")}, ids) {
return errors.New("expected updated policy ids")
}
data, err := manager.Store.Read(ctx, txn, storage.Path{})
data, err := manager.Store.Read(ctx, txn, storage.RootPath)
// remove system key to make comparison simpler
delete(data.(map[string]any), "system")
if err != nil {
@@ -3884,7 +3884,7 @@ func TestPluginActivateScopedBundle(t *testing.T) {
if err := storage.Txn(ctx, manager.Store, storage.WriteParams, func(txn storage.Transaction) error {
externalData := map[string]any{"a": map[string]any{"a1": "x1", "a3": "x2", "a5": "x3"}}
if err := manager.Store.Write(ctx, txn, storage.AddOp, storage.Path{}, externalData); err != nil {
if err := manager.Store.Write(ctx, txn, storage.AddOp, storage.RootPath, externalData); err != nil {
return err
}
if err := manager.Store.UpsertPolicy(ctx, txn, "some/id1", []byte(`package a.a2`)); err != nil {
@@ -7137,7 +7137,7 @@ func TestPluginManualTriggerMultipleDiskStorage(t *testing.T) {
t.Fatalf("Bad policy content. Exp:\n%v\n\nGot:\n\n%v", string(exp), string(bs))
}
data, err := manager.Store.Read(ctx, txn, storage.Path{})
data, err := manager.Store.Read(ctx, txn, storage.RootPath)
expData := util.MustUnmarshalJSON([]byte(`{
"p": "x1", "q": "x2",
"system": {
v1/rego/example_test.go
+1 -1
View File
@@ -372,7 +372,7 @@ func ExampleRego_Eval_persistent_storage() {
// Handle error.
}
err = storage.WriteOne(ctx, store, storage.AddOp, storage.Path{}, json)
err = storage.WriteOne(ctx, store, storage.AddOp, storage.RootPath, json)
if err != nil {
// Handle error
}
v1/rego/rego.go
+2 -2
View File
@@ -1798,7 +1798,7 @@ func (r *Rego) PrepareForEval(ctx context.Context, opts ...PrepareOption) (Prepa
}
// nolint: staticcheck // SA4006 false positive
data, err := r.store.Read(ctx, r.txn, storage.Path{})
data, err := r.store.Read(ctx, r.txn, storage.RootPath)
if err != nil {
_ = txnClose(ctx, err) // Ignore error
return PreparedEvalQuery{}, err
@@ -2020,7 +2020,7 @@ func (r *Rego) loadFiles(ctx context.Context, txn storage.Transaction, m metrics
}
if len(result.Documents) > 0 {
err = r.store.Write(ctx, txn, storage.AddOp, storage.Path{}, result.Documents)
err = r.store.Write(ctx, txn, storage.AddOp, storage.RootPath, result.Documents)
if err != nil {
return err
}
v1/repl/repl.go
+1 -1
View File
@@ -1410,7 +1410,7 @@ func newCommand(line string) *command {
}
func dumpStorage(ctx context.Context, store storage.Store, txn storage.Transaction, w io.Writer) error {
data, err := store.Read(ctx, txn, storage.Path{})
data, err := store.Read(ctx, txn, storage.RootPath)
if err != nil {
return err
}
v1/storage/inmem/inmem.go
+14 -14
View File
@@ -71,7 +71,7 @@ func NewFromObjectWithOpts(data map[string]any, opts ...Opt) storage.Store {
if err != nil {
panic(err)
}
if err := db.Write(ctx, txn, storage.AddOp, storage.Path{}, data); err != nil {
if err := db.Write(ctx, txn, storage.AddOp, storage.RootPath, data); err != nil {
panic(err)
}
if err := db.Commit(ctx, txn); err != nil {
@@ -120,19 +120,23 @@ type handle struct {
}
func (db *store) NewTransaction(_ context.Context, params ...storage.TransactionParams) (storage.Transaction, error) {
var write bool
var ctx *storage.Context
if len(params) > 0 {
write = params[0].Write
ctx = params[0].Context
txn := &transaction{
xid: atomic.AddUint64(&db.xid, uint64(1)),
db: db,
}
xid := atomic.AddUint64(&db.xid, uint64(1))
if write {
if len(params) > 0 {
txn.write = params[0].Write
txn.context = params[0].Context
}
if txn.write {
db.wmu.Lock()
} else {
db.rmu.RLock()
}
return newTransaction(xid, write, ctx, db), nil
return txn, nil
}
// Truncate implements the storage.Store interface. This method must be called within a transaction.
@@ -193,11 +197,7 @@ func (db *store) Truncate(ctx context.Context, txn storage.Transaction, params s
// For backwards compatibility, check if `RootOverwrite` was configured.
if params.RootOverwrite {
newPath, ok := storage.ParsePathEscaped("/")
if !ok {
return fmt.Errorf("storage path invalid: %v", newPath)
}
return underlying.Write(storage.AddOp, newPath, mergedData)
return underlying.Write(storage.AddOp, storage.RootPath, mergedData)
}
for _, root := range params.BasePaths {
v1/storage/inmem/inmem_bench_test.go
+130
View File
@@ -0,0 +1,130 @@
package inmem_test
import (
"testing"
"github.com/open-policy-agent/opa/v1/storage"
"github.com/open-policy-agent/opa/v1/storage/inmem"
)
type (
txnType bool
target struct {
name string
store storage.Store
}
targets []target
withTxnFn func(b *testing.B, target storage.Store, txn storage.Transaction)
noTxnFn func(b *testing.B, target storage.Store)
)
const (
readTxn txnType = false
writeTxn txnType = true
)
func BenchmarkNewTransaction(b *testing.B) {
store := inmem.NewFromObject(map[string]any{})
for name, typ := range map[string]txnType{"Read": readTxn, "Write": writeTxn} {
b.Run(name, func(b *testing.B) {
for b.Loop() {
txn, err := store.NewTransaction(b.Context(), transactionParams(typ)...)
if err != nil {
b.Fatal(err)
}
store.Abort(b.Context(), txn)
}
})
}
}
func BenchmarkReadOne(b *testing.B) {
data := map[string]any{"foo": "bar"}
path := storage.Path{"foo"}
AllStores(data).Bench(b, func(b *testing.B, store storage.Store) {
if _, err := storage.ReadOne(b.Context(), store, path); err != nil {
b.Fatal(err)
}
})
}
func BenchmarkRead(b *testing.B) {
data := map[string]any{"foo": "bar"}
path := storage.Path{"foo"}
AllStores(data).BenchWithTxn(b, readTxn, func(b *testing.B, s storage.Store, txn storage.Transaction) {
if _, err := s.Read(b.Context(), txn, path); err != nil {
b.Fatal(err)
}
})
}
func BenchmarkWriteOne(b *testing.B) {
data := map[string]any{}
path := storage.Path{"foo"}
AllStores(data).Bench(b, func(b *testing.B, store storage.Store) {
if err := storage.WriteOne(b.Context(), store, storage.AddOp, path, "bar"); err != nil {
b.Fatal(err)
}
})
}
func transactionParams(mode txnType) (params []storage.TransactionParams) {
if mode == writeTxn {
params = append(params, storage.WriteParams)
}
return params
}
func AllStores(data map[string]any) targets {
return []target{
{
"Go store (roundtrip)",
inmem.NewFromObjectWithOpts(data, inmem.OptRoundTripOnWrite(true)),
},
{
"Go store (no roundtrip)",
inmem.NewFromObjectWithOpts(data, inmem.OptRoundTripOnWrite(false)),
},
{
"AST store (roundtrip)",
inmem.NewFromObjectWithOpts(data, inmem.OptReturnASTValuesOnRead(true), inmem.OptRoundTripOnWrite(true)),
},
{
"AST store (no roundtrip)",
inmem.NewFromObjectWithOpts(data, inmem.OptReturnASTValuesOnRead(true), inmem.OptRoundTripOnWrite(false)),
},
}
}
func (t targets) Bench(b *testing.B, fn noTxnFn) {
b.Helper()
for _, target := range t {
b.Run(target.name, func(b *testing.B) {
for b.Loop() {
fn(b, target.store)
}
})
}
}
func (t targets) BenchWithTxn(b *testing.B, mode txnType, fn withTxnFn) {
b.Helper()
for _, target := range t {
b.Run(target.name, func(b *testing.B) {
txn := storage.NewTransactionOrDie(b.Context(), target.store, transactionParams(mode)...)
for b.Loop() {
fn(b, target.store, txn)
}
target.store.Abort(b.Context(), txn)
})
}
}
v1/storage/inmem/inmem_test.go
+3 -16
View File
@@ -195,7 +195,7 @@ func TestInMemoryWrite(t *testing.T) {
store := NewFromObjectWithOpts(data, OptReturnASTValuesOnRead(rvt.ast))
// Perform patch and check result
value := loadExpectedSortedResult(tc.value)
value := loadExpectedResult(tc.value)
var op storage.PatchOp
switch tc.op {
@@ -304,7 +304,7 @@ func TestInMemoryWriteOfStruct(t *testing.T) {
t.Fatal(err)
}
expected := loadExpectedSortedResult(tc.expected)
expected := loadExpectedResult(tc.expected)
if !reflect.DeepEqual(expected, actual) {
t.Errorf("expected %v, got %v", tc.expected, actual)
}
@@ -1235,21 +1235,8 @@ func loadExpectedResult(input string) any {
if len(input) == 0 {
return nil
}
var data any
if err := util.UnmarshalJSON([]byte(input), &data); err != nil {
panic(err)
}
return data
}
func loadExpectedSortedResult(input string) any {
data := loadExpectedResult(input)
switch data := data.(type) {
case []any:
return data
default:
return data
}
return util.MustUnmarshalJSON([]byte(input))
}
func loadSmallTestData() map[string]any {
v1/storage/inmem/txn.go
+43 -51
View File
@@ -34,13 +34,13 @@ import (
// Read transactions do not require any special handling and simply passthrough
// to the underlying store. Read transactions do not support upgrade.
type transaction struct {
db *store
updates *list.List
context *storage.Context
policies map[string]policyUpdate
xid uint64
write bool
stale bool
db *store
updates *list.List
policies map[string]policyUpdate
context *storage.Context
}
type policyUpdate struct {
@@ -48,28 +48,17 @@ type policyUpdate struct {
remove bool
}
func newTransaction(xid uint64, write bool, context *storage.Context, db *store) *transaction {
return &transaction{
xid: xid,
write: write,
db: db,
policies: map[string]policyUpdate{},
updates: list.New(),
context: context,
}
}
func (txn *transaction) ID() uint64 {
return txn.xid
}
func (txn *transaction) Write(op storage.PatchOp, path storage.Path, value any) error {
if !txn.write {
return &storage.Error{
Code: storage.InvalidTransactionErr,
Message: "data write during read transaction",
}
return &storage.Error{Code: storage.InvalidTransactionErr, Message: "data write during read transaction"}
}
if txn.updates == nil {
txn.updates = list.New()
}
if len(path) == 0 {
@@ -145,7 +134,7 @@ func (txn *transaction) updateRoot(op storage.PatchOp, value any) error {
}
update = &updateAST{
path: storage.Path{},
path: storage.RootPath,
remove: false,
value: valueAST,
}
@@ -155,7 +144,7 @@ func (txn *transaction) updateRoot(op storage.PatchOp, value any) error {
}
update = &updateRaw{
path: storage.Path{},
path: storage.RootPath,
remove: false,
value: value,
}
@@ -163,20 +152,23 @@ func (txn *transaction) updateRoot(op storage.PatchOp, value any) error {
txn.updates.Init()
txn.updates.PushFront(update)
return nil
}
func (txn *transaction) Commit() (result storage.TriggerEvent) {
result.Context = txn.context
for curr := txn.updates.Front(); curr != nil; curr = curr.Next() {
action := curr.Value.(dataUpdate)
txn.db.data = action.Apply(txn.db.data)
if txn.updates != nil {
for curr := txn.updates.Front(); curr != nil; curr = curr.Next() {
action := curr.Value.(dataUpdate)
txn.db.data = action.Apply(txn.db.data)
result.Data = append(result.Data, storage.DataEvent{
Path: action.Path(),
Data: action.Value(),
Removed: action.Remove(),
})
result.Data = append(result.Data, storage.DataEvent{
Path: action.Path(),
Data: action.Value(),
Removed: action.Remove(),
})
}
}
for id, upd := range txn.policies {
if upd.remove {
@@ -218,8 +210,7 @@ func deepcpy(v any) any {
}
func (txn *transaction) Read(path storage.Path) (any, error) {
if !txn.write {
if !txn.write || txn.updates == nil {
return pointer(txn.db.data, path)
}
@@ -260,8 +251,7 @@ func (txn *transaction) Read(path storage.Path) (any, error) {
return cpy, nil
}
func (txn *transaction) ListPolicies() []string {
var ids []string
func (txn *transaction) ListPolicies() (ids []string) {
for id := range txn.db.policies {
if _, ok := txn.policies[id]; !ok {
ids = append(ids, id)
@@ -276,11 +266,13 @@ func (txn *transaction) ListPolicies() []string {
}
func (txn *transaction) GetPolicy(id string) ([]byte, error) {
if update, ok := txn.policies[id]; ok {
if !update.remove {
return update.value, nil
if txn.policies != nil {
if update, ok := txn.policies[id]; ok {
if !update.remove {
return update.value, nil
}
return nil, errors.NewNotFoundErrorf("policy id %q", id)
}
return nil, errors.NewNotFoundErrorf("policy id %q", id)
}
if exist, ok := txn.db.policies[id]; ok {
return exist, nil
@@ -289,24 +281,24 @@ func (txn *transaction) GetPolicy(id string) ([]byte, error) {
}
func (txn *transaction) UpsertPolicy(id string, bs []byte) error {
if !txn.write {
return &storage.Error{
Code: storage.InvalidTransactionErr,
Message: "policy write during read transaction",
}
}
txn.policies[id] = policyUpdate{bs, false}
return nil
return txn.updatePolicy(id, policyUpdate{bs, false})
}
func (txn *transaction) DeletePolicy(id string) error {
return txn.updatePolicy(id, policyUpdate{nil, true})
}
func (txn *transaction) updatePolicy(id string, update policyUpdate) error {
if !txn.write {
return &storage.Error{
Code: storage.InvalidTransactionErr,
Message: "policy write during read transaction",
}
return &storage.Error{Code: storage.InvalidTransactionErr, Message: "policy write during read transaction"}
}
txn.policies[id] = policyUpdate{nil, true}
if txn.policies == nil {
txn.policies = map[string]policyUpdate{id: update}
} else {
txn.policies[id] = update
}
return nil
}
v1/storage/path.go
+17 -37
View File
@@ -8,40 +8,40 @@ import (
"errors"
"fmt"
"net/url"
"slices"
"strconv"
"strings"
"github.com/open-policy-agent/opa/v1/ast"
)
// RootPath refers to the root document in storage.
var RootPath = Path{}
// Path refers to a document in storage.
type Path []string
// ParsePath returns a new path for the given str.
func ParsePath(str string) (path Path, ok bool) {
if len(str) == 0 {
return nil, false
}
if str[0] != '/' {
if len(str) == 0 || str[0] != '/' {
return nil, false
}
if len(str) == 1 {
return Path{}, true
}
parts := strings.Split(str[1:], "/")
return parts, true
return strings.Split(str[1:], "/"), true
}
// ParsePathEscaped returns a new path for the given escaped str.
func ParsePathEscaped(str string) (path Path, ok bool) {
path, ok = ParsePath(str)
if !ok {
return
}
for i := range path {
segment, err := url.PathUnescape(path[i])
if err == nil {
path[i] = segment
if path, ok = ParsePath(str); ok {
for i := range path {
if segment, err := url.PathUnescape(path[i]); err == nil {
path[i] = segment
} else {
return nil, false
}
}
}
return
@@ -49,7 +49,6 @@ func ParsePathEscaped(str string) (path Path, ok bool) {
// NewPathForRef returns a new path for the given ref.
func NewPathForRef(ref ast.Ref) (path Path, err error) {
if len(ref) == 0 {
return nil, errors.New("empty reference (indicates error in caller)")
}
@@ -85,36 +84,17 @@ func NewPathForRef(ref ast.Ref) (path Path, err error) {
// is less than other, 0 if p is equal to other, or 1 if p is greater than
// other.
func (p Path) Compare(other Path) (cmp int) {
for i := range min(len(p), len(other)) {
if cmp := strings.Compare(p[i], other[i]); cmp != 0 {
return cmp
}
}
if len(p) < len(other) {
return -1
}
if len(p) == len(other) {
return 0
}
return 1
return slices.Compare(p, other)
}
// Equal returns true if p is the same as other.
func (p Path) Equal(other Path) bool {
return p.Compare(other) == 0
return slices.Equal(p, other)
}
// HasPrefix returns true if p starts with other.
func (p Path) HasPrefix(other Path) bool {
if len(other) > len(p) {
return false
}
for i := range other {
if p[i] != other[i] {
return false
}
}
return true
return len(other) <= len(p) && p[:len(other)].Equal(other)
}
// Ref returns a ref that represents p rooted at head.
v1/storage/path_test.go
+21 -13
View File
@@ -23,7 +23,7 @@ func TestNewPathForString(t *testing.T) {
}{
{"", nil, false},
{"foo", nil, false},
{"/", Path{}, true},
{"/", RootPath, true},
{"/", nil, true},
{"/foo", Path{"foo"}, true},
{"/foo/bar", Path{"foo", "bar"}, true},
@@ -53,7 +53,7 @@ func TestNewPathForRef(t *testing.T) {
{ast.MustParseRef("data.foo[{1, 2}]"), nil, fmt.Errorf("composites cannot be base document keys: %v", ast.MustParseRef("data.foo[{1, 2}]"))},
{ast.MustParseRef(`data.foo[{"foo": 2}]`), nil, fmt.Errorf("composites cannot be base document keys: %v", ast.MustParseRef(`data.foo[{"foo": 2}]`))},
{ast.MustParseRef("data"), Path{}, nil},
{ast.MustParseRef("data"), RootPath, nil},
{ast.MustParseRef("data.foo"), Path{"foo"}, nil},
{ast.MustParseRef("data.foo[1]"), Path{"foo", "1"}, nil},
{ast.MustParseRef("data.foo.bar"), Path{"foo", "bar"}, nil},
@@ -70,12 +70,15 @@ func TestNewPathForRef(t *testing.T) {
}
func TestNewPathForStringEscaped(t *testing.T) {
tests := []struct {
input string
result Path
ok bool
}{
{
input: "",
ok: false,
},
{
input: "/foo/bar", // no escaping
result: Path{"foo", "bar"},
@@ -91,6 +94,11 @@ func TestNewPathForStringEscaped(t *testing.T) {
result: Path{"foo//bar", "baz"},
ok: true,
},
{
input: "/foo%%%%bar",
result: nil, // invalid escaping
ok: false,
},
}
for _, tc := range tests {
@@ -107,9 +115,9 @@ func TestPathCompare(t *testing.T) {
b Path
result int
}{
{Path{}, Path{}, 0},
{Path{}, Path{"x"}, -1},
{Path{"x"}, Path{}, 1},
{RootPath, RootPath, 0},
{RootPath, Path{"x"}, -1},
{Path{"x"}, RootPath, 1},
{Path{"x"}, Path{"x"}, 0},
{Path{"x"}, Path{"y"}, -1},
{Path{"x"}, Path{"w"}, 1},
@@ -133,9 +141,9 @@ func TestPathEqual(t *testing.T) {
b Path
result bool
}{
{Path{}, Path{}, true},
{Path{}, Path{"foo"}, false},
{Path{"foo"}, Path{}, false},
{RootPath, RootPath, true},
{RootPath, Path{"foo"}, false},
{Path{"foo"}, RootPath, false},
{Path{"foo", "bar"}, Path{"foo"}, false},
{Path{"foo", "bar"}, Path{"foo", "bar"}, true},
}
@@ -153,15 +161,15 @@ func TestPathHasPrefix(t *testing.T) {
b Path
result bool
}{
{Path{}, Path{}, true},
{Path{}, Path{"foo"}, false},
{Path{"foo"}, Path{}, true},
{RootPath, RootPath, true},
{RootPath, Path{"foo"}, false},
{Path{"foo"}, RootPath, true},
{Path{"foo"}, Path{"bar"}, false},
{Path{"bar"}, Path{"foo"}, false},
{Path{"foo", "bar"}, Path{"foo"}, true},
{Path{"foo", "bar"}, Path{"foo", "bar"}, true},
{Path{"foo", "bar"}, Path{"foo", "bar", "baz"}, false},
{Path{"foo", "bar", "baz"}, Path{}, true},
{Path{"foo", "bar", "baz"}, RootPath, true},
}
for _, tc := range tests {
result := tc.a.HasPrefix(tc.b)
v1/test/authz/authz_bench_test.go
+1 -1
View File
@@ -61,7 +61,7 @@ func runAuthzBenchmark(b *testing.B, mode InputMode, numPaths int, extras ...boo
b.Fatal(err)
}
if err = storage.WriteOne(ctx, store, storage.AddOp, storage.Path{}, data); err != nil {
if err = storage.WriteOne(ctx, store, storage.AddOp, storage.RootPath, data); err != nil {
b.Fatal(err)
}
} else {
v1/tester/runner.go
+9 -6
View File
@@ -1172,10 +1172,12 @@ func LoadWithRegoVersion(args []string, filter loader.Filter, regoVersion ast.Re
}
var store storage.Store
ctx := context.Background()
if bundle.BundleExtStore != nil {
store = bundle.BundleExtStore()
// inline'd NewFromObject
if err := storage.WriteOne(context.Background(), store, storage.AddOp, storage.Path{}, loaded.Documents); err != nil {
if err := storage.WriteOne(ctx, store, storage.AddOp, storage.RootPath, loaded.Documents); err != nil {
return nil, nil, err
}
} else {
@@ -1183,7 +1185,7 @@ func LoadWithRegoVersion(args []string, filter loader.Filter, regoVersion ast.Re
}
modules := make(map[string]*ast.Module, len(loaded.Modules))
ctx := context.Background()
err = storage.Txn(ctx, store, storage.WriteParams, func(txn storage.Transaction) error {
for _, loadedModule := range loaded.Modules {
modules[loadedModule.Name] = loadedModule.Parsed
@@ -1213,12 +1215,15 @@ func LoadWithParserOptions(args []string, filter loader.Filter, popts ast.Parser
if err != nil {
return nil, nil, err
}
var store storage.Store
ctx := context.Background()
// Plumb in storage for external bundle activation plugin, if registered with bundle.RegisterStore.
if bundle.BundleExtStore != nil {
store = bundle.BundleExtStore()
// inline'd NewFromObject
if err := storage.WriteOne(context.Background(), store, storage.AddOp, storage.Path{}, loaded.Documents); err != nil {
if err := storage.WriteOne(ctx, store, storage.AddOp, storage.RootPath, loaded.Documents); err != nil {
return nil, nil, err
}
} else {
@@ -1226,7 +1231,6 @@ func LoadWithParserOptions(args []string, filter loader.Filter, popts ast.Parser
}
modules := make(map[string]*ast.Module, len(loaded.Modules))
ctx := context.Background()
err = storage.Txn(ctx, store, storage.WriteParams, func(txn storage.Transaction) error {
for _, loadedModule := range loaded.Modules {
modules[loadedModule.Name] = loadedModule.Parsed
@@ -1234,8 +1238,7 @@ func LoadWithParserOptions(args []string, filter loader.Filter, popts ast.Parser
// Add the policies to the store to ensure that any future bundle
// activations will preserve them and re-compile the module with
// the bundle modules.
err := store.UpsertPolicy(ctx, txn, loadedModule.Name, loadedModule.Raw)
if err != nil {
if err := store.UpsertPolicy(ctx, txn, loadedModule.Name, loadedModule.Raw); err != nil {
return err
}
}