diff --git a/cmd/run.go b/cmd/run.go index ab5485183c..0ae050f4c5 100644 --- a/cmd/run.go +++ b/cmd/run.go @@ -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(¶ms.Server, "server", "s", false, "start the runtime in server mode") runCommand.Flags().StringVarP(¶ms.Eval, "eval", "e", "", "evaluate, print, exit") runCommand.Flags().StringVarP(¶ms.HistoryPath, "history", "H", historyPath(), "set path of history file") - runCommand.Flags().StringVarP(¶ms.PolicyDir, "policy-dir", "p", "", "set directory to store policy definitions") runCommand.Flags().StringVarP(¶ms.Addr, "addr", "a", defaultAddr, "set listening address of the server") runCommand.Flags().StringVarP(¶ms.InsecureAddr, "insecure-addr", "", "", "set insecure listening address of the server") runCommand.Flags().StringVarP(¶ms.OutputFormat, "format", "f", "pretty", "set shell output format, i.e, pretty, json") diff --git a/repl/repl_test.go b/repl/repl_test.go index 7c5f5ef8c7..fde6da6bf7 100644 --- a/repl/repl_test.go +++ b/repl/repl_test.go @@ -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") diff --git a/runtime/runtime.go b/runtime/runtime.go index 202618110a..ab51844197 100644 --- a/runtime/runtime.go +++ b/runtime/runtime.go @@ -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 } } diff --git a/runtime/runtime_test.go b/runtime/runtime_test.go index 8e4727c2c5..d51ab6c89d 100644 --- a/runtime/runtime_test.go +++ b/runtime/runtime_test.go @@ -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 { diff --git a/server/server.go b/server/server.go index f4dd4895df..126e6ae810 100644 --- a/server/server.go +++ b/server/server.go @@ -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 } diff --git a/server/server_test.go b/server/server_test.go index eae2f0ad2f..71c7b4fa5b 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -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). diff --git a/site/examples/docker-authorization/index.md b/site/examples/docker-authorization/index.md index 287da00550..4de646025d 100644 --- a/site/examples/docker-authorization/index.md +++ b/site/examples/docker-authorization/index.md @@ -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. diff --git a/storage/example_test.go b/storage/example_test.go index bc38cb11fd..25befc1931 100644 --- a/storage/example_test.go +++ b/storage/example_test.go @@ -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 - -} diff --git a/storage/policystore.go b/storage/policystore.go index 1c66ea9773..6d52ae479d 100644 --- a/storage/policystore.go +++ b/storage/policystore.go @@ -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) } diff --git a/storage/policystore_test.go b/storage/policystore_test.go index 8a69fb8625..c7798aa2b4 100644 --- a/storage/policystore_test.go +++ b/storage/policystore_test.go @@ -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() diff --git a/storage/storage.go b/storage/storage.go index 33d66e6188..163c4d3cc7 100644 --- a/storage/storage.go +++ b/storage/storage.go @@ -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.