This commit adds two checks to the in-memory store to detect improper
use of transactions:
1. Mark aborted/committed transactions as stale and error if an
operation is attempted on a stale transaction. Previously callers
could perform unsafe concurrent operations on stale transactions
without noticiing.
2. Check that supplied transactions are from the underlying store and
not another store. We could consider deprecating the store APIs and
moving them onto the transaction to prevent this kind of mistake in
the future.
With these changes the store will panic on unregister and abort
calls if any of these invariants are violated. Panicing is preferable
to failing silently.
Note, we still have the issue of recursive transactions resulting in
deadlock. Our options there are to implement more sophisticated
locking inside the in-memory store or modify the API so that the store
can update the passed context.Context with a sentinel value.
Fixes#1594
Signed-off-by: Torin Sandall <torinsandall@gmail.com>
These changes update the storage to accept arbitrary key/value
parameters when creating transactions. The container is passed to
trigger callbacks that are invoked when transactions commit.
The use case for this is avoiding parse and compile operations on
modules loaded out of bundles. The parse and compile step on large
bundles can take several seconds. Since the triggers are executed
while the store's read-lock is held, policies queries block.
Signed-off-by: Torin Sandall <torinsandall@gmail.com>
Previously there were no checks in place to ensure that base and
virtual documents do not overlap. As a result, if users loaded raw
JSON and rules into OPA that overlapped, the evaluation results were
not well defined. With these changes, we can detect the overlap and
reject updates (to policies or data) that would cause inconsistent
results.
Fixes#1207
Signed-off-by: Torin Sandall <torinsandall@gmail.com>
I don't know why I ended up looking into this, but I've run
golangci-lint run --disable-all -E deadcode
on the code base and removed everything that came up :)
Signed-off-by: Stephan Renatus <srenatus@chef.io>
This should fix#722. It's a slight variation of the code snippet provided
there:
I wasn't sure what the reflect.Interface part was for, so this is using
only reflect.Ptr. Also, there existing tests would fail without the added
check for reflect.Invalid.
Adds a test case for inmem -- in a new method, as I couldn't quite fit it
into the schema of TestInMemoryWrite.
Also, util.Reference() ensures that the returned value is a pointer to
something -- and not a pointer to a pointer to something. While this wasn't
part of the issue #722, it felt weird not to solve the general problem, but
only the edge case. :)
Signed-off-by: Stephan Renatus <srenatus@chef.io>
These changes extend #702 to include writes. If the path segments are
escaped, the should be unescaped during parsing. This allows callers to
write keys like "foo/bar" into storage.
Fixes#695
Signed-off-by: Torin Sandall <torinsandall@gmail.com>
It's often useful to be able to create a hierarchical structure in one
shot in storage. Previously this functionality was implemented in the
server, but it's better off implemented in the storage package.
Signed-off-by: Torin Sandall <torinsandall@gmail.com>
With this, where before we've gotten
ast: illegal value: []string
and had worked around it by converting all our []string inputs to
[]interface{}, it now would work to pass in the []string values as-is.
This adds a roundtrip through the JSON encoding to both the rego.Input,
and the inmem store's Write.
Signed-off-by: Stephan Renatus <srenatus@chef.io>
Previously, sets and objects were not interfaces and as a result,
callers were relying on the underlying structure for operations such as
iteration.
These changes refactor the ast package to expose sets and objects as
interfaces so that we can change the underlying data structures without
affecting callers.
The store used to require the user to supply a unique ID in order to
register a trigger. Instead, when registering a trigger, the Store now
returns a handle on which the trigger can be unregistered. This has a
number of benefits, including that the user no longer has to reason
about what IDs they have already used, and only the owner of the handle
can end the trigger.
It's common for operations to the store to simply read or write a single
value. The ReadOne and WriteOne functions each create their own
transactions, perform the operation and then clean up after themselves.
This greatly simplifies user code that only performs singular
operations, as it no longer needs to check for errors at every step.
The Store interface was not clear if transactions need to be aborted
if committing them fails. This clarifies that a failed commit must be
equivalent to an abort.
Previously store triggers only told you whether data changed or
policy changed, they did not specify what data/policy changed or
how they changed. Now, TriggerEvents contain lists describing
all changes to policies and data that were made in a given commit.
These changes update the storage layer to support
multi-reader/single-writer txns:
- Writers can read their own writes
- Writers can rollback changes
- Readers only see writes after a successful commit
- Readers can progress during a write txn and only block during a commit
These changes also refactor the trigger interface to better support
transactions.
This is a large change set that contains a few backwards incompatible
changes. Summary of breaking changes:
- Remove storage.Storage in favour of storage.Store interface.
- Remove mount support.
- Remove storage of compiled policies.
- Modify storage.Store to support rollback.
- Modify storage.Store to support raw policy storage.
- Modify storage.Store to support indexing.
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.
- Refactor error codes to use strings instead of ints.
- Simplify error messages throughout.
- Ensure location set on all expressions. There were a couple locations
in the parser/compiler where locations were not being set.
- Fallback to rule location in topdown in case location not set. This
ensures that users get useful locations for API requests with paths
that refer to virtual docs exactly.
Also add Find function to ast.Value. Useful for extracting values
dynamically. Eventually can support JSON pointers.
Fixes#237
This includes all of the changes to plumb the context through from the HTTP
server and the REPL.
Also, this removes the Travis CI build for Go 1.6 as the context package is
not part of the standard library before Go 1.7. Once Go 1.8 is released we can
go back to supporting the previous Go release.
Fixes#155
These changes modify OPA to use json.Number to represent number values in
storage and the AST. Comparisons and numeric operations are handled by the
math/big package. Using json.Number avoids loss of precision when loading
integers greater than 2**53 and use of math/big avoids need for type
conversions in the built-in implementations. In the long run, the math/big
usage may be replaced with specific paths for smaller numbers.
Fixes#154
These changes refactor the storage layer to use storage.Path instead of ast.Ref
for Read/Write/Begin/Unmount/Mount operations.
Previously, the storage layer used ast.Ref values to refer to locations in
storage. Use of ast.Ref introduced unnecessary complexity for storage plugins
as they had to be aware of various details (e.g., array indices specified as
ast.Number/float64 values, potentially nested references, etc.) that were
unnecessary given that ast.Ref values passed to the storage layer were
intended represent JSON pointers.
These changes also remove the need for storage plugins to be aware of where
they are mounted. That is, the paths passed to the read call will be relative
to the mount point.
The remaining dependencies on the ast package from the storage package are for
(1) indexing and (2) policy storage. It may be possible to further decouple
these packages by revisiting how indexing is done and treating policies as
blobs.
Fixes#159
Previously, the compiler would mutate the parsed modules and store them as
output. This made it difficult to reason about the state of modules
constructed programatically in places like the REPL.
This change adds a Copy() function to the AST types that are stored by
reference. This allows the compiler to deep copy the parsed modules before
operating on them. This removes the challenge of reasoning about how the
compiler will behave when compiling the same input multiple times.
Various callers had to be modified to deal with the new calling convention.
Unit tests have been added to verify the new behaviour.
Fixes#158
Before, callers would either flatten errors to a string or pick the first
error reported by the compiler. This was a bad approach and lead to an issue
for users running OPA from the command line, for example:
$ opa run test.rego
If test.rego failed to compile, the run command would only print the first
error...
This change lets callers treat all compiler errors as a single error and
factors the flattening into the Error() implementation of the new ast.Errors
type.
This is small change that allows callers to issue GET /data and receive the
full extent of OPA's data. Secondly, this allows callers to set the data
in OPA with one request (e.g., for initialization).
Previously, rules had to be installed into the built-in store in order for the
topdown implementation to access them. Now that the topdown implementation
receives an instance of the compiler, the rules do not have to be installed
into the built-in store anymore.
Transactions are added to the storage layer to facilitate consistent snapshots
over data provided by storage plugins. For now, the transactions are
serialized by the Storage object using a lock. If/when throughput becomes a
problem, this can be replaced with a more sophisticated locking scheme or
other concurrency control (hopefully) without requiring major changes to the
storage layer APIs.
This is a large set of refactoring changes made to shrink the surface area of
the storage layer APIs. With these changes, PolicyStore and DataStore are no
longer directly used outside of the storage package.
These changes add support for pluggable storage backends. Previously, the
storage layer only supported a single, built-in, in-memory storage backend.
Users integrating with OPA would write a shim layer that pushed data into this
backend so that the policy engine could evaluate over it. This approach is not
sufficient when the dataset is too large to replicate or the process of
replication would introduce race conditions into systems integrated with OPA.
With these changes, users can implement their own storage backends for OPA. The
storage backends are "mounted" into the root document at a particular
location. References to data under this location are then served by the
user's storage backend. This allows the storage backend to fetch data from the
source of truth thereby avoiding race conditions (when that is important).
These changes introduce new interfaces in the storage layer:
- Transaction interface
- Store interface (i.e., the pluggable backend)
- Storage object, represents the policy engine's storage layer
- Trigger interface
Provide a high level description of ast and topdown packages as these are
currently the core of the project. We can flesh out the other package
descriptions as necessary.
The scheduler benchmark is useful as an end-to-end test of OPA so it is nice
to have it run as part of the normal suite (as long as it doesn't take too
long). To accomodate this, these changes split the scheduler benchmark into
two suites: (1) normal test that runs on captured dataset (10 nodes x 30
pods) and (2) benchmark test that pre-populates storage with nodes, pods, RCs,
etc. This allows us to simulate larger data sets (e.g., 1k nodes x 3k pods) in
the benchmark.
Previously, nested references were not allowed. This was particularly annoying
when rules were used to define constant values because it required an
intermediate variable to store the constant value in the current scope (which
could then be used in the reference).
Now, nested references are allowed and the compiler and evaluation engine have
been updated to support them. Specifically, the evaluation engine will
recursively evaluate nested references before the outer most reference is
evaluated. The evaluation adds a binding for the nested reference to the
context so that when the containing term or expression is plugged, the nested
references are replaced with the referred value.
The compiler has been updated to include body safety, reordering, and
recursiong tests involving nested references.
The path handling was broken for patch objects with path = "-".
Refactored Data API test cases as well. They're more concise now and can be
extended easily.
Teemu Koponen