Skip to content

Commit

Permalink
refact: read bundle revisions if decision log enabled
Browse files Browse the repository at this point in the history
Signed-off-by: Anthony Regeda <[email protected]>
  • Loading branch information
regeda committed Jan 14, 2025
1 parent 36980be commit d19b6d7
Show file tree
Hide file tree
Showing 5 changed files with 136 additions and 129 deletions.
33 changes: 0 additions & 33 deletions envoyauth/evaluation.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"fmt"

"github.com/open-policy-agent/opa/ast"
"github.com/open-policy-agent/opa/bundle"
"github.com/open-policy-agent/opa/config"
"github.com/open-policy-agent/opa/logging"
"github.com/open-policy-agent/opa/rego"
Expand Down Expand Up @@ -53,11 +52,6 @@ func Eval(ctx context.Context, evalContext EvalContext, input ast.Value, result
result.Txn = txn
}

err = getRevision(ctx, evalContext.Store(), result.Txn, result)
if err != nil {
return err
}

result.TxnID = result.Txn.ID()

if logger.GetLevel() == logging.Debug {
Expand Down Expand Up @@ -124,33 +118,6 @@ func Eval(ctx context.Context, evalContext EvalContext, input ast.Value, result
return nil
}

func getRevision(ctx context.Context, store storage.Store, txn storage.Transaction, result *EvalResult) error {
revisions := map[string]string{}

names, err := bundle.ReadBundleNamesFromStore(ctx, store, txn)
if err != nil && !storage.IsNotFound(err) {
return err
}

for _, name := range names {
r, err := bundle.ReadBundleRevisionFromStore(ctx, store, txn, name)
if err != nil && !storage.IsNotFound(err) {
return err
}
revisions[name] = r
}

// Check legacy bundle manifest in the store
revision, err := bundle.LegacyReadRevisionFromStore(ctx, store, txn)
if err != nil && !storage.IsNotFound(err) {
return err
}

result.Revisions = revisions
result.Revision = revision
return nil
}

type hook struct {
logger logging.Logger
}
Expand Down
97 changes: 1 addition & 96 deletions envoyauth/evaluation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package envoyauth

import (
"context"
"reflect"
"strings"
"sync"
"testing"
Expand All @@ -15,7 +14,6 @@ import (
"github.com/open-policy-agent/opa/plugins/logs"

"github.com/open-policy-agent/opa/ast"
"github.com/open-policy-agent/opa/bundle"
"github.com/open-policy-agent/opa/plugins"
"github.com/open-policy-agent/opa/rego"
"github.com/open-policy-agent/opa/storage"
Expand All @@ -24,99 +22,6 @@ import (
"github.com/open-policy-agent/opa/topdown/print"
)

func TestGetRevisionLegacy(t *testing.T) {
store := inmem.New()
ctx := context.Background()

result := EvalResult{}

tb := bundle.Manifest{
Revision: "abc123",
Roots: &[]string{"/a/b", "/a/c"},
}

// write a "legacy" manifest
err := storage.Txn(ctx, store, storage.WriteParams, func(txn storage.Transaction) error {
if err := bundle.LegacyWriteManifestToStore(ctx, store, txn, tb); err != nil {
t.Fatalf("Failed to write manifest to store: %s", err)
return err
}
return nil
})
if err != nil {
t.Fatalf("Unexpected error finishing transaction: %s", err)
}

txn := storage.NewTransactionOrDie(ctx, store, storage.WriteParams)

err = getRevision(ctx, store, txn, &result)
if err != nil {
t.Fatal(err)
}

expected := "abc123"
if result.Revision != "abc123" {
t.Fatalf("Expected revision %v but got %v", expected, result.Revision)
}

if len(result.Revisions) != 0 {
t.Fatal("Unexpected multiple bundles")
}
}

func TestGetRevisionMulti(t *testing.T) {
store := inmem.New()
ctx := context.Background()

result := EvalResult{}

bundles := map[string]bundle.Manifest{
"bundle1": {
Revision: "abc123",
Roots: &[]string{"/a/b", "/a/c"},
},
"bundle2": {
Revision: "def123",
Roots: &[]string{"/x/y", "/z"},
},
}

// write bundles
for name, manifest := range bundles {
err := storage.Txn(ctx, store, storage.WriteParams, func(txn storage.Transaction) error {
err := bundle.WriteManifestToStore(ctx, store, txn, name, manifest)
if err != nil {
t.Fatalf("Failed to write manifest to store: %s", err)
}
return err
})
if err != nil {
t.Fatalf("Unexpected error finishing transaction: %s", err)
}
}

txn := storage.NewTransactionOrDie(ctx, store, storage.WriteParams)

err := getRevision(ctx, store, txn, &result)
if err != nil {
t.Fatal(err)
}

if len(result.Revisions) != 2 {
t.Fatalf("Expected two bundles but got %v", len(result.Revisions))
}

expected := map[string]string{"bundle1": "abc123", "bundle2": "def123"}
if !reflect.DeepEqual(result.Revisions, expected) {
t.Fatalf("Expected result: %v, got: %v", expected, result.Revisions)
}

if result.Revision != "" {
t.Fatalf("Unexpected revision %v", result.Revision)
}

}

type testPrintHook struct {
printed string
}
Expand Down Expand Up @@ -211,7 +116,7 @@ func testAuthzServer(logger logging.Logger) (*mockExtAuthzGrpcServer, error) {
default allow = false
allow {
allow {
input.parsed_body.firstname == "foo"
input.parsed_body.lastname == "bar"
print(input.parsed_body)
Expand Down
31 changes: 31 additions & 0 deletions envoyauth/response.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
ext_type_v3 "github.com/envoyproxy/go-control-plane/envoy/type/v3"
_structpb "github.com/golang/protobuf/ptypes/struct"
"github.com/google/uuid"
"github.com/open-policy-agent/opa/bundle"
"github.com/open-policy-agent/opa/metrics"
"github.com/open-policy-agent/opa/storage"
"github.com/open-policy-agent/opa/topdown/builtins"
Expand Down Expand Up @@ -62,6 +63,36 @@ func NewEvalResult(opts ...func(*EvalResult)) (*EvalResult, StopFunc, error) {
return er, stop, nil
}

// ReadRevisions adds bundle revisions to the result.
func (result *EvalResult) ReadRevisions(ctx context.Context, store storage.Store) error {
if result.Txn == nil {
return nil
}
names, err := bundle.ReadBundleNamesFromStore(ctx, store, result.Txn)
if err != nil && !storage.IsNotFound(err) {
return err
}

revisions := make(map[string]string, len(names))
for _, name := range names {
r, err := bundle.ReadBundleRevisionFromStore(ctx, store, result.Txn, name)
if err != nil && !storage.IsNotFound(err) {
return err
}
revisions[name] = r
}

// Check legacy bundle manifest in the store
revision, err := bundle.LegacyReadRevisionFromStore(ctx, store, result.Txn)
if err != nil && !storage.IsNotFound(err) {
return err
}

result.Revisions = revisions
result.Revision = revision
return nil
}

// GetTxn creates a read transaction suitable for the configured EvalResult object
func (result *EvalResult) GetTxn(ctx context.Context, store storage.Store) (storage.Transaction, TransactionCloser, error) {
params := storage.TransactionParams{}
Expand Down
100 changes: 100 additions & 0 deletions envoyauth/response_test.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
package envoyauth

import (
"context"
"encoding/json"
"reflect"
"strings"
"testing"

_structpb "github.com/golang/protobuf/ptypes/struct"
"github.com/open-policy-agent/opa/bundle"
"github.com/open-policy-agent/opa/storage"
"github.com/open-policy-agent/opa/storage/inmem"
"google.golang.org/protobuf/proto"
)

Expand Down Expand Up @@ -46,6 +50,102 @@ func TestIsAllowed(t *testing.T) {
}
}

func TestReadRevisionsLegacy(t *testing.T) {
store := inmem.New()
ctx := context.Background()

tb := bundle.Manifest{
Revision: "abc123",
Roots: &[]string{"/a/b", "/a/c"},
}

// write a "legacy" manifest
err := storage.Txn(ctx, store, storage.WriteParams, func(txn storage.Transaction) error {
if err := bundle.LegacyWriteManifestToStore(ctx, store, txn, tb); err != nil {
t.Fatalf("Failed to write manifest to store: %s", err)
return err
}
return nil
})
if err != nil {
t.Fatalf("Unexpected error finishing transaction: %s", err)
}

txn := storage.NewTransactionOrDie(ctx, store, storage.WriteParams)

result := EvalResult{
Txn: txn,
}

err = result.ReadRevisions(ctx, store)
if err != nil {
t.Fatal(err)
}

expected := "abc123"
if result.Revision != "abc123" {
t.Fatalf("Expected revision %v but got %v", expected, result.Revision)
}

if len(result.Revisions) != 0 {
t.Fatal("Unexpected multiple bundles")
}
}

func TestReadRevisionsMulti(t *testing.T) {
store := inmem.New()
ctx := context.Background()

bundles := map[string]bundle.Manifest{
"bundle1": {
Revision: "abc123",
Roots: &[]string{"/a/b", "/a/c"},
},
"bundle2": {
Revision: "def123",
Roots: &[]string{"/x/y", "/z"},
},
}

// write bundles
for name, manifest := range bundles {
err := storage.Txn(ctx, store, storage.WriteParams, func(txn storage.Transaction) error {
err := bundle.WriteManifestToStore(ctx, store, txn, name, manifest)
if err != nil {
t.Fatalf("Failed to write manifest to store: %s", err)
}
return err
})
if err != nil {
t.Fatalf("Unexpected error finishing transaction: %s", err)
}
}

txn := storage.NewTransactionOrDie(ctx, store, storage.WriteParams)

result := EvalResult{
Txn: txn,
}

err := result.ReadRevisions(ctx, store)
if err != nil {
t.Fatal(err)
}

if len(result.Revisions) != 2 {
t.Fatalf("Expected two bundles but got %v", len(result.Revisions))
}

expected := map[string]string{"bundle1": "abc123", "bundle2": "def123"}
if !reflect.DeepEqual(result.Revisions, expected) {
t.Fatalf("Expected result: %v, got: %v", expected, result.Revisions)
}

if result.Revision != "" {
t.Fatalf("Unexpected revision %v", result.Revision)
}
}

func TestGetRequestQueryParametersToRemove(t *testing.T) {
tests := map[string]struct {
decision interface{}
Expand Down
4 changes: 4 additions & 0 deletions internal/internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -651,6 +651,10 @@ func (p *envoyExtAuthzGrpcServer) logDecision(ctx context.Context, input interfa
info.NDBuiltinCache = &x
}

if err := result.ReadRevisions(ctx, p.Store()); err != nil {
return err
}

return decisionlog.LogDecision(ctx, plugin, info, result, err)
}

Expand Down

0 comments on commit d19b6d7

Please sign in to comment.