Remove persist/--policy-dir option

This is the first in series of Spring cleaning around the storage layer.
In the near future we will add local disk-based persistence support to
OPA. That support will handle storage of source files.

The --policy-dir option is almost entirely unused today. Removing it
will make it easier to get rid of the policyStore entirely.

The next thing to do will be to remove the specialized *Policy methods
from the storage layer. This way the storage layer can just accept
policies as normal data.

If policies need to be persisted until then, users can treat the
policies as config files and manage them outside of OPA.
This commit is contained in:
Torin Sandall
2017-03-12 13:11:07 -07:00
parent 6933a84482
commit b1fc681590
11 changed files with 42 additions and 357 deletions
-6
View File
@@ -101,11 +101,6 @@ For example:
3
]
}
If the --policy-dir option is specified any files inside the directory will be
considered policy definitions and will be loaded on startup. API calls to create
new policies save the definition file to this direcory. In addition, API calls
to delete policies will remove the definition file.
`,
Run: func(cmd *cobra.Command, args []string) {
@@ -132,7 +127,6 @@ to delete policies will remove the definition file.
runCommand.Flags().BoolVarP(&params.Server, "server", "s", false, "start the runtime in server mode")
runCommand.Flags().StringVarP(&params.Eval, "eval", "e", "", "evaluate, print, exit")
runCommand.Flags().StringVarP(&params.HistoryPath, "history", "H", historyPath(), "set path of history file")
runCommand.Flags().StringVarP(&params.PolicyDir, "policy-dir", "p", "", "set directory to store policy definitions")
runCommand.Flags().StringVarP(&params.Addr, "addr", "a", defaultAddr, "set listening address of the server")
runCommand.Flags().StringVarP(&params.InsecureAddr, "insecure-addr", "", "", "set insecure listening address of the server")
runCommand.Flags().StringVarP(&params.OutputFormat, "format", "f", "pretty", "set shell output format, i.e, pretty, json")
+3 -3
View File
@@ -36,11 +36,11 @@ q = 2 { true }`)
r = 3 { true }`)
if err := storage.InsertPolicy(ctx, store, "mod1", mod1, nil, false); err != nil {
if err := storage.InsertPolicy(ctx, store, "mod1", mod1, nil); err != nil {
panic(err)
}
if err := storage.InsertPolicy(ctx, store, "mod2", mod2, nil, false); err != nil {
if err := storage.InsertPolicy(ctx, store, "mod2", mod2, nil); err != nil {
panic(err)
}
@@ -418,7 +418,7 @@ func TestEvalData(t *testing.T) {
testmod := ast.MustParseModule(`package ex
p = [1, 2, 3] { true }`)
if err := storage.InsertPolicy(ctx, store, "test", testmod, nil, false); err != nil {
if err := storage.InsertPolicy(ctx, store, "test", testmod, nil); err != nil {
panic(err)
}
repl.OneShot(ctx, "data")
+2 -16
View File
@@ -62,11 +62,6 @@ type Params struct {
// where the contained document should be loaded.
Paths []string
// PolicyDir is the filename of the directory to persist policy
// definitions in. Policy definitions stored in this directory
// are automatically loaded on startup.
PolicyDir string
// Server flag controls whether the OPA instance will start a server.
// By default, the OPA instance acts as an interactive shell.
Server bool
@@ -124,19 +119,13 @@ func (rt *Runtime) Start(params *Params) {
func (rt *Runtime) init(ctx context.Context, params *Params) error {
if len(params.PolicyDir) > 0 {
if err := os.MkdirAll(params.PolicyDir, 0755); err != nil {
return errors.Wrap(err, "unable to make --policy-dir")
}
}
loaded, err := loadAllPaths(params.Paths)
if err != nil {
return err
}
// Open data store and load base documents.
store := storage.New(storage.InMemoryConfig().WithPolicyDir(params.PolicyDir))
store := storage.New(storage.InMemoryConfig())
if err := store.Open(ctx); err != nil {
return err
@@ -172,13 +161,10 @@ func (rt *Runtime) startServer(ctx context.Context, params *Params) {
"insecure_addr": params.InsecureAddr,
}).Infof("First line of log stream.")
persist := len(params.PolicyDir) > 0
s, err := server.New().
WithStorage(rt.Store).
WithAddress(params.Addr).
WithInsecureAddress(params.InsecureAddr).
WithPersist(persist).
WithCertificate(params.Certificate).
WithAuthentication(params.Authentication).
WithAuthorization(params.Authorization).
@@ -295,7 +281,7 @@ func compileAndStoreInputs(modules map[string]*loadedModule, store *storage.Stor
}
for id := range modules {
if err := store.InsertPolicy(txn, id, modules[id].Parsed, modules[id].Raw, false); err != nil {
if err := store.InsertPolicy(txn, id, modules[id].Parsed, modules[id].Raw); err != nil {
return err
}
}
+1 -20
View File
@@ -9,7 +9,6 @@ import (
"context"
"io/ioutil"
"os"
"path/filepath"
"reflect"
"strings"
"testing"
@@ -69,28 +68,10 @@ p = true { 1 = 2 }`
panic(err)
}
tmp3, err := ioutil.TempDir("", "policyDir")
if err != nil {
panic(err)
}
defer os.RemoveAll(tmp3)
tmp4 := filepath.Join(tmp3, "existingPolicy")
err = ioutil.WriteFile(tmp4, []byte(`package a.b.c
q = true { false }`,
), 0644)
if err != nil {
panic(err)
}
rt := Runtime{}
err = rt.init(ctx, &Params{
Paths: []string{tmp1.Name(), tmp2.Name()},
PolicyDir: tmp3,
Paths: []string{tmp1.Name(), tmp2.Name()},
})
if err != nil {
+1 -8
View File
@@ -63,7 +63,6 @@ type Server struct {
authentication AuthenticationScheme
authorization AuthorizationScheme
cert *tls.Certificate
persist bool
mtx sync.RWMutex
compiler *ast.Compiler
store *storage.Storage
@@ -158,12 +157,6 @@ func (s *Server) WithCertificate(cert *tls.Certificate) *Server {
return s
}
// WithPersist indicates to server whether to persist policies.
func (s *Server) WithPersist(yes bool) *Server {
s.persist = yes
return s
}
// WithStorage sets the storage used by the server.
func (s *Server) WithStorage(store *storage.Storage) *Server {
s.store = store
@@ -684,7 +677,7 @@ func (s *Server) v1PoliciesPut(w http.ResponseWriter, r *http.Request) {
return
}
if err := s.store.InsertPolicy(txn, path, parsedMod, buf, s.persist); err != nil {
if err := s.store.InsertPolicy(txn, path, parsedMod, buf); err != nil {
writer.ErrorAuto(w, err)
return
}
+3 -19
View File
@@ -8,10 +8,8 @@ import (
"context"
"encoding/json"
"fmt"
"io/ioutil"
"net/http"
"net/http/httptest"
"os"
"reflect"
"strings"
"testing"
@@ -26,20 +24,6 @@ import (
var policyDir string
// TestMain creates a temporary direcotry for the server to
// save policies to. The directory name is stored in policyDir
// and is used by the newFixture function.
func TestMain(m *testing.M) {
d, err := ioutil.TempDir("", "server_test")
if err != nil {
panic(err)
}
defer os.RemoveAll(d)
policyDir = d
rc := m.Run()
os.Exit(rc)
}
type tr struct {
method string
path string
@@ -852,7 +836,7 @@ func TestQueryV1Explain(t *testing.T) {
func TestAuthorization(t *testing.T) {
ctx := context.Background()
store := storage.New(storage.InMemoryConfig().WithPolicyDir(policyDir))
store := storage.New(storage.InMemoryConfig())
txn := storage.NewTransactionOrDie(ctx, store)
authzPolicy := `package system.authz
@@ -868,7 +852,7 @@ func TestAuthorization(t *testing.T) {
module := ast.MustParseModule(authzPolicy)
if err := store.InsertPolicy(txn, "test", module, nil, false); err != nil {
if err := store.InsertPolicy(txn, "test", module, nil); err != nil {
panic(err)
}
@@ -1025,7 +1009,7 @@ type fixture struct {
func newFixture(t *testing.T) *fixture {
ctx := context.Background()
store := storage.New(storage.InMemoryConfig().WithPolicyDir(policyDir))
store := storage.New(storage.InMemoryConfig())
server, err := New().
WithAddress(":8182").
WithStorage(store).
+2 -2
View File
@@ -71,10 +71,10 @@ $ chmod u+x opa
```
### 3. Run OPA in server mode with logging enabled.
### 3. Run OPA in server mode with debug logging enabled.
```shell
$ ./opa run -s --log-level debug --policy-dir policies
$ ./opa run --server --log-level debug
```
OPA will run until it receives a signal to stop. Open another terminal to continue with the rest of the example.
-61
View File
@@ -9,9 +9,6 @@ import (
"context"
"encoding/json"
"fmt"
"io/ioutil"
"os"
"path/filepath"
"github.com/open-policy-agent/opa/storage"
)
@@ -159,61 +156,3 @@ func ExampleStorage_Write() {
// err2: storage_not_found_error: /users/1/color: document does not exist
}
func ExampleStorage_Open() {
// Initialize context for the example. Normally the caller would obtain the
// context from an input parameter or instantiate their own.
ctx := context.Background()
// Define two example modules and write them to disk in a temporary directory.
ex1 := `
package opa.example
p { q.r != 0 }
`
ex2 := `
package opa.example
q = {"r": 100}
`
path, err := ioutil.TempDir("", "")
if err != nil {
// Handle error.
}
defer os.RemoveAll(path)
if err = ioutil.WriteFile(filepath.Join(path, "ex1.rego"), []byte(ex1), 0644); err != nil {
// Handle error.
}
if err = ioutil.WriteFile(filepath.Join(path, "ex2.rego"), []byte(ex2), 0644); err != nil {
// Handle error.
}
// Instantiate storage layer and configure with a directory to persist policy modules.
store := storage.New(storage.InMemoryConfig().WithPolicyDir(path))
if err = store.Open(ctx); err != nil {
// Handle error.
}
// Inspect one of the loaded policies.
mod, _, err := storage.GetPolicy(ctx, store, "ex1.rego")
if err != nil {
// Handle error.
}
fmt.Println("Expr:", mod.Rules[0].Body[0])
// Output:
// Expr: q.r != 0
}
+10 -113
View File
@@ -5,53 +5,25 @@
package storage
import (
"fmt"
"io/ioutil"
"os"
"path/filepath"
"strings"
"github.com/open-policy-agent/opa/ast"
"github.com/pkg/errors"
)
// TODO(tsandall): update policy store to use correct transaction ids
// TODO(tsandall): remove policy store entirely--the data store should be able
// to handle storage of source files.
// policyStore provides a storage abstraction for policy definitions and modules.
type policyStore struct {
policyDir string
raw map[string][]byte
modules map[string]*ast.Module
}
// loadPolicies is the default callback function that will be used when
// opening the policy store.
func loadPolicies(bufs map[string][]byte) (map[string]*ast.Module, error) {
parsed := map[string]*ast.Module{}
for id, bs := range bufs {
mod, err := ast.ParseModule(id, string(bs))
if err != nil {
return nil, err
}
parsed[id] = mod
}
c := ast.NewCompiler()
if c.Compile(parsed); c.Failed() {
return nil, c.Errors
}
return parsed, nil
raw map[string][]byte
modules map[string]*ast.Module
}
// NewPolicyStore returns an empty PolicyStore.
func newPolicyStore(policyDir string) *policyStore {
func newPolicyStore() *policyStore {
return &policyStore{
policyDir: policyDir,
raw: map[string][]byte{},
modules: map[string]*ast.Module{},
raw: map[string][]byte{},
modules: map[string]*ast.Module{},
}
}
@@ -64,89 +36,18 @@ func (p *policyStore) List() map[string]*ast.Module {
return cpy
}
// Open initializes the policy store.
//
// This should be called on startup to load policies from persistent storage.
// The callback function "f" will be invoked with the buffers representing the
// persisted policies. The callback should return the compiled version of the
// policies so that they can be installed into the store.
func (p *policyStore) Open(txn Transaction, f func(map[string][]byte) (map[string]*ast.Module, error)) error {
if len(p.policyDir) == 0 {
return nil
}
info, err := ioutil.ReadDir(p.policyDir)
if err != nil {
return err
}
raw := map[string][]byte{}
for _, i := range info {
f := i.Name()
bs, err := ioutil.ReadFile(filepath.Join(p.policyDir, f))
if err != nil {
return err
}
id := p.getID(f)
raw[id] = bs
}
mods, err := f(raw)
if err != nil {
return err
}
for id, mod := range mods {
if err := p.Add(id, mod, raw[id], false); err != nil {
return err
}
}
return nil
}
// Add inserts the policy module into the store. If an existing policy module exists with the same ID,
// it is overwritten. If persist is false, then the policy will not be persisted.
func (p *policyStore) Add(id string, mod *ast.Module, raw []byte, persist bool) error {
if persist && len(p.policyDir) == 0 {
return fmt.Errorf("cannot persist without --policy-dir set")
}
// it is overwritten.
func (p *policyStore) Add(id string, mod *ast.Module, raw []byte) error {
p.raw[id] = raw
p.modules[id] = mod
if persist {
filename := p.getFilename(id)
if err := ioutil.WriteFile(filename, raw, 0644); err != nil {
return errors.Wrapf(err, "failed to persist definition but new version was installed: %v", id)
}
}
return nil
}
// Remove removes the policy module for id.
func (p *policyStore) Remove(id string) error {
filename := p.getFilename(id)
if strings.HasPrefix(filename, p.policyDir) {
if err := os.Remove(filename); err != nil {
if !os.IsNotExist(err) {
return errors.Wrapf(err, "failed to delete persisted definition but module was uninstalled: %v", id)
}
}
}
delete(p.raw, id)
delete(p.modules, id)
return nil
}
@@ -163,15 +64,11 @@ func (p *policyStore) Get(id string) (*ast.Module, error) {
func (p *policyStore) GetRaw(id string) ([]byte, error) {
bs, ok := p.raw[id]
if !ok {
return nil, notFoundErrorf("definition not found: %v", id)
return nil, notFoundErrorf("source not found: %v", id)
}
return bs, nil
}
func (p *policyStore) getFilename(id string) string {
return filepath.Join(p.policyDir, id)
}
func (p *policyStore) getID(f string) string {
return filepath.Base(f)
}
+9 -89
View File
@@ -5,9 +5,6 @@
package storage
import (
"io/ioutil"
"os"
"path/filepath"
"testing"
"github.com/open-policy-agent/opa/ast"
@@ -17,62 +14,20 @@ import (
// policyStore is now an implementation detail of the storage layer and these
// are essentially integration tests.
func TestPolicyStoreDefaultOpen(t *testing.T) {
dir, err := ioutil.TempDir("", "policyDir")
if err != nil {
panic(err)
}
defer os.RemoveAll(dir)
filename := filepath.Join(dir, "testMod1")
err = ioutil.WriteFile(filename, []byte(testMod1), 0644)
if err != nil {
panic(err)
}
policyStore := newPolicyStore(dir)
err = policyStore.Open(invalidTXN, loadPolicies)
if err != nil {
t.Errorf("Unexpected error on Open(): %v", err)
return
}
c := ast.NewCompiler()
mod := ast.MustParseModule(testMod1)
if c.Compile(map[string]*ast.Module{"testMod1": mod}); c.Failed() {
panic(c.Errors)
}
stored, err := policyStore.Get("testMod1")
if err != nil {
t.Errorf("Unexpected error on Get(): %v", err)
return
}
if !mod.Equal(stored) {
t.Fatalf("Expected %v from policy store but got: %v", mod, stored)
}
}
func TestPolicyStoreAdd(t *testing.T) {
f := newFixture()
defer f.cleanup()
mod1 := f.compile1(testMod1)
mod2 := f.compile1(testMod2)
err := f.policyStore.Add("testMod1", mod1, []byte(testMod1), true)
err := f.policyStore.Add("testMod1", mod1, []byte(testMod1))
if err != nil {
t.Errorf("Unexpected error on Add(): %v", err)
return
}
err = f.policyStore.Add("testMod2", mod2, []byte(testMod2), true)
err = f.policyStore.Add("testMod2", mod2, []byte(testMod2))
if err != nil {
t.Errorf("Unexpected error on Add(): %v", err)
return
@@ -115,17 +70,16 @@ func TestPolicyStoreAdd(t *testing.T) {
func TestPolicyStoreAddIdempotent(t *testing.T) {
f := newFixture()
defer f.cleanup()
mod1 := f.compile1(testMod1)
err := f.policyStore.Add("testMod1", mod1, []byte(testMod1), true)
err := f.policyStore.Add("testMod1", mod1, []byte(testMod1))
if err != nil {
t.Errorf("Unexpected error on Add(): %v", err)
return
}
err = f.policyStore.Add("testMod1", mod1, []byte(testMod1), true)
err = f.policyStore.Add("testMod1", mod1, []byte(testMod1))
if err != nil {
t.Errorf("Unexpected error on Add(): %v", err)
return
@@ -136,18 +90,17 @@ func TestPolicyStoreAddIdempotent(t *testing.T) {
func TestPolicyStoreRemove(t *testing.T) {
f := newFixture()
defer f.cleanup()
mod1 := f.compile1(testMod1)
mod2 := f.compile1(testMod2)
err := f.policyStore.Add("testMod1", mod1, []byte(testMod1), true)
err := f.policyStore.Add("testMod1", mod1, []byte(testMod1))
if err != nil {
t.Errorf("Unexpected error on Add(): %v", err)
return
}
err = f.policyStore.Add("testMod2", mod2, []byte(testMod2), true)
err = f.policyStore.Add("testMod2", mod2, []byte(testMod2))
if err != nil {
t.Errorf("Unexpected error on Add(): %v", err)
return
@@ -169,36 +122,21 @@ func TestPolicyStoreRemove(t *testing.T) {
t.Errorf("Expected testMod2 to remain after Remove(): %v", mods)
return
}
_, err = os.Stat(f.policyStore.getFilename("testMod1"))
if !os.IsNotExist(err) {
info, err := ioutil.ReadDir(f.policyStore.policyDir)
if err != nil {
panic(err)
}
files := []string{}
for _, i := range info {
files = append(files, i.Name())
}
t.Errorf("Expected testMod1 to be removed from disk but %v contains: %v", f.policyStore.policyDir, files)
return
}
}
func TestPolicyStoreUpdate(t *testing.T) {
f := newFixture()
defer f.cleanup()
mod1 := f.compile1(testMod1)
mod2 := f.compile1(testMod2)
err := f.policyStore.Add("testMod1", mod1, []byte(testMod1), true)
err := f.policyStore.Add("testMod1", mod1, []byte(testMod1))
if err != nil {
t.Errorf("Unexpected error on Add(): %v", err)
return
}
err = f.policyStore.Add("testMod1", mod2, []byte(testMod2), true)
err = f.policyStore.Add("testMod1", mod2, []byte(testMod2))
if err != nil {
t.Errorf("Unexpected error on Add(): %v", err)
return
@@ -222,31 +160,13 @@ type fixture struct {
}
func newFixture() *fixture {
dir, err := ioutil.TempDir("", "policyDir")
if err != nil {
panic(err)
}
policyStore := newPolicyStore(dir)
err = policyStore.Open(invalidTXN, func(map[string][]byte) (map[string]*ast.Module, error) {
return nil, nil
})
if err != nil {
panic(err)
}
policyStore := newPolicyStore()
f := &fixture{
policyStore: policyStore,
}
return f
}
func (f *fixture) cleanup() {
os.RemoveAll(f.policyStore.policyDir)
}
func (f *fixture) compile1(m string) *ast.Module {
mods := f.policyStore.List()
+11 -20
View File
@@ -13,15 +13,13 @@ import (
// Config represents the configuration for the policy engine's storage layer.
type Config struct {
Builtin Store
PolicyDir string
Builtin Store
}
// InMemoryConfig returns a new Config for an in-memory storage layer.
func InMemoryConfig() Config {
return Config{
Builtin: NewDataStore(),
PolicyDir: "",
Builtin: NewDataStore(),
}
}
@@ -29,17 +27,10 @@ func InMemoryConfig() Config {
// using existing JSON data. This is primarily for test purposes.
func InMemoryWithJSONConfig(data map[string]interface{}) Config {
return Config{
Builtin: NewDataStoreFromJSONObject(data),
PolicyDir: "",
Builtin: NewDataStoreFromJSONObject(data),
}
}
// WithPolicyDir returns a new Config with the policy directory configured.
func (c Config) WithPolicyDir(dir string) Config {
c.PolicyDir = dir
return c
}
// Storage represents the policy engine's storage layer.
type Storage struct {
builtin Store
@@ -66,15 +57,15 @@ func New(config Config) *Storage {
return &Storage{
builtin: config.Builtin,
indices: newIndices(),
policyStore: newPolicyStore(config.PolicyDir),
policyStore: newPolicyStore(),
active: map[string]struct{}{},
}
}
// Open initializes the storage layer. Open should normally be called
// immediately after instantiating a new instance of the storage layer. If the
// storage layer is configured to use in-memory storage and is not persisting
// policy modules to disk, the call to Open() may be omitted.
// storage layer is configured to use in-memory storage the Open() call can be
// skiped.
func (s *Storage) Open(ctx context.Context) error {
txn, err := s.NewTransaction(ctx)
@@ -84,7 +75,7 @@ func (s *Storage) Open(ctx context.Context) error {
defer s.Close(ctx, txn)
return s.policyStore.Open(txn, loadPolicies)
return nil
}
// ListPolicies returns a map of policy modules that have been loaded into the
@@ -111,8 +102,8 @@ func (s *Storage) GetPolicy(txn Transaction, id string) (*ast.Module, []byte, er
// InsertPolicy upserts a policy module into the storage layer. If the policy
// module already exists, it is replaced. If the persist flag is true, the
// storage layer will attempt to write the raw policy module content to disk.
func (s *Storage) InsertPolicy(txn Transaction, id string, module *ast.Module, raw []byte, persist bool) error {
return s.policyStore.Add(id, module, raw, persist)
func (s *Storage) InsertPolicy(txn Transaction, id string, module *ast.Module, raw []byte) error {
return s.policyStore.Add(id, module, raw)
}
// DeletePolicy removes a policy from the storage layer.
@@ -367,13 +358,13 @@ func (s *Storage) notifyStoresClose(ctx context.Context, txn Transaction) {
}
// InsertPolicy upserts a policy module into storage inside a new transaction.
func InsertPolicy(ctx context.Context, store *Storage, id string, mod *ast.Module, raw []byte, persist bool) error {
func InsertPolicy(ctx context.Context, store *Storage, id string, mod *ast.Module, raw []byte) error {
txn, err := store.NewTransaction(ctx)
if err != nil {
return err
}
defer store.Close(ctx, txn)
return store.InsertPolicy(txn, id, mod, raw, persist)
return store.InsertPolicy(txn, id, mod, raw)
}
// DeletePolicy removes a policy module from storage inside a new transaction.