Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

topdown+rego+server: allow opt-in for evaluating non-det builtins in PE #7313

Merged
merged 3 commits into from
Jan 27, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 42 additions & 39 deletions cmd/eval.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,45 +40,46 @@ var (
)

type evalCommandParams struct {
capabilities *capabilitiesFlag
coverage bool
partial bool
unknowns []string
disableInlining []string
shallowInlining bool
disableIndexing bool
disableEarlyExit bool
strictBuiltinErrors bool
showBuiltinErrors bool
dataPaths repeatedStringFlag
inputPath string
imports repeatedStringFlag
pkg string
stdin bool
stdinInput bool
explain *util.EnumFlag
metrics bool
instrument bool
ignore []string
outputFormat *util.EnumFlag
profile bool
profileCriteria repeatedStringFlag
profileLimit intFlag
count int
prettyLimit intFlag
fail bool
failDefined bool
bundlePaths repeatedStringFlag
schema *schemaFlags
target *util.EnumFlag
timeout time.Duration
optimizationLevel int
entrypoints repeatedStringFlag
strict bool
v0Compatible bool
v1Compatible bool
traceVarValues bool
ReadAstValuesFromStore bool
capabilities *capabilitiesFlag
coverage bool
partial bool
unknowns []string
disableInlining []string
nondeterministicBuiltions bool
shallowInlining bool
disableIndexing bool
disableEarlyExit bool
strictBuiltinErrors bool
showBuiltinErrors bool
dataPaths repeatedStringFlag
inputPath string
imports repeatedStringFlag
pkg string
stdin bool
stdinInput bool
explain *util.EnumFlag
metrics bool
instrument bool
ignore []string
outputFormat *util.EnumFlag
profile bool
profileCriteria repeatedStringFlag
profileLimit intFlag
count int
prettyLimit intFlag
fail bool
failDefined bool
bundlePaths repeatedStringFlag
schema *schemaFlags
target *util.EnumFlag
timeout time.Duration
optimizationLevel int
entrypoints repeatedStringFlag
strict bool
v0Compatible bool
v1Compatible bool
traceVarValues bool
ReadAstValuesFromStore bool
}

func (p *evalCommandParams) regoVersion() ast.RegoVersion {
Expand Down Expand Up @@ -328,6 +329,7 @@ access.
evalCommand.Flags().BoolVarP(&params.coverage, "coverage", "", false, "report coverage")
evalCommand.Flags().StringArrayVarP(&params.disableInlining, "disable-inlining", "", []string{}, "set paths of documents to exclude from inlining")
evalCommand.Flags().BoolVarP(&params.shallowInlining, "shallow-inlining", "", false, "disable inlining of rules that depend on unknowns")
evalCommand.Flags().BoolVarP(&params.nondeterministicBuiltions, "nondeterminstic-builtins", "", false, "evaluate nondeterministic builtins (if all arguments are known) during partial eval")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if there's a future need for a more fine-grained control, we can invest more time

Agreed, no need to over-engineer this from the get-go 👍 .
If we want to give the user precise control of what built-ins to evaluate, we can add something like a --nondeterminstic-builtin (singular) that takes the name and can be repeated.

evalCommand.Flags().BoolVar(&params.disableIndexing, "disable-indexing", false, "disable indexing optimizations")
evalCommand.Flags().BoolVar(&params.disableEarlyExit, "disable-early-exit", false, "disable 'early exit' optimizations")
evalCommand.Flags().BoolVarP(&params.strictBuiltinErrors, "strict-builtin-errors", "", false, "treat the first built-in function error encountered as fatal")
Expand Down Expand Up @@ -577,6 +579,7 @@ func setupEval(args []string, params evalCommandParams) (*evalContext, error) {
evalArgs := []rego.EvalOption{
rego.EvalRuleIndexing(!params.disableIndexing),
rego.EvalEarlyExit(!params.disableEarlyExit),
rego.EvalNondeterministicBuiltins(params.nondeterministicBuiltions),
}

if len(params.imports.v) > 0 {
Expand Down
7 changes: 6 additions & 1 deletion docs/content/rest-api.md
Original file line number Diff line number Diff line change
Expand Up @@ -1320,6 +1320,11 @@ on the OPA blog shows how SQL can be generated based on Compile API output.
For more details on Partial Evaluation in OPA, please refer to
[this blog post](https://blog.openpolicyagent.org/partial-evaluation-162750eaf422).

Note that nondeterminstic builtins (like `http.send`) are _not evaluated_ during PE.
You can change that by providing `nondeterminsticBuiltins: true` in your payload options.
This would be desirable when using PE for generating filters using extra information
from `http.send`.

#### Request Body

Compile API requests contain the following fields:
Expand All @@ -1328,7 +1333,7 @@ Compile API requests contain the following fields:
| --- | --- | --- | --- |
| `query` | `string` | Yes | The query to partially evaluate and compile. |
| `input` | `any` | No | The input document to use during partial evaluation (default: undefined). |
| `options` | `object[string, any]` | No | Additional options to use during partial evaluation. Only `disableInlining` option is supported. (default: undefined). |
| `options` | `object[string, any]` | No | Additional options to use during partial evaluation: `disableInlining` (default: undefined) and `nondeterminsticBuiltins` (default: false). |
| `unknowns` | `array[string]` | No | The terms to treat as unknown during partial evaluation (default: `["input"]`]). |

### Request Headers
Expand Down
81 changes: 52 additions & 29 deletions v1/rego/rego.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ type EvalContext struct {
compiledQuery compiledQuery
unknowns []string
disableInlining []ast.Ref
nondeterministicBuiltins bool
parsedUnknowns []*ast.Term
indexing bool
earlyExit bool
Expand Down Expand Up @@ -372,6 +373,15 @@ func EvalVirtualCache(vc topdown.VirtualCache) EvalOption {
}
}

// EvalNondeterministicBuiltins causes non-deterministic builtins to be evalued
// during partial evaluation. This is needed to pull in external data, or validate
// a JWT, during PE, so that the result informs what queries are returned.
func EvalNondeterministicBuiltins(yes bool) EvalOption {
return func(e *EvalContext) {
e.nondeterministicBuiltins = yes
}
}

func (pq preparedQuery) Modules() map[string]*ast.Module {
mods := make(map[string]*ast.Module)

Expand All @@ -394,24 +404,25 @@ func (pq preparedQuery) Modules() map[string]*ast.Module {
// been opened.
func (pq preparedQuery) newEvalContext(ctx context.Context, options []EvalOption) (*EvalContext, func(context.Context), error) {
ectx := &EvalContext{
hasInput: false,
rawInput: nil,
parsedInput: nil,
metrics: nil,
txn: nil,
instrument: false,
instrumentation: nil,
partialNamespace: pq.r.partialNamespace,
queryTracers: nil,
unknowns: pq.r.unknowns,
parsedUnknowns: pq.r.parsedUnknowns,
compiledQuery: compiledQuery{},
indexing: true,
earlyExit: true,
resolvers: pq.r.resolvers,
printHook: pq.r.printHook,
capabilities: pq.r.capabilities,
strictBuiltinErrors: pq.r.strictBuiltinErrors,
hasInput: false,
rawInput: nil,
parsedInput: nil,
metrics: nil,
txn: nil,
instrument: false,
instrumentation: nil,
partialNamespace: pq.r.partialNamespace,
queryTracers: nil,
unknowns: pq.r.unknowns,
parsedUnknowns: pq.r.parsedUnknowns,
nondeterministicBuiltins: pq.r.nondeterministicBuiltins,
compiledQuery: compiledQuery{},
indexing: true,
earlyExit: true,
resolvers: pq.r.resolvers,
printHook: pq.r.printHook,
capabilities: pq.r.capabilities,
strictBuiltinErrors: pq.r.strictBuiltinErrors,
}

for _, o := range options {
Expand Down Expand Up @@ -580,6 +591,7 @@ type Rego struct {
parsedUnknowns []*ast.Term
disableInlining []string
shallowInlining bool
nondeterministicBuiltins bool
skipPartialNamespace bool
partialNamespace string
modules []rawModule
Expand Down Expand Up @@ -922,6 +934,15 @@ func DisableInlining(paths []string) func(r *Rego) {
}
}

// NondeterministicBuiltins causes non-deterministic builtins to be evalued during
// partial evaluation. This is needed to pull in external data, or validate a JWT,
// during PE, so that the result informs what queries are returned.
func NondeterministicBuiltins(yes bool) func(r *Rego) {
return func(r *Rego) {
r.nondeterministicBuiltins = yes
}
}

// ShallowInlining prevents rules that depend on unknown values from being inlined.
// Rules that only depend on known values are inlined.
func ShallowInlining(yes bool) func(r *Rego) {
Expand Down Expand Up @@ -2334,17 +2355,18 @@ func (r *Rego) partialResult(ctx context.Context, pCfg *PrepareConfig) (PartialR
}

ectx := &EvalContext{
parsedInput: r.parsedInput,
metrics: r.metrics,
txn: r.txn,
partialNamespace: r.partialNamespace,
queryTracers: r.queryTracers,
compiledQuery: r.compiledQueries[partialResultQueryType],
instrumentation: r.instrumentation,
indexing: true,
resolvers: r.resolvers,
capabilities: r.capabilities,
strictBuiltinErrors: r.strictBuiltinErrors,
parsedInput: r.parsedInput,
metrics: r.metrics,
txn: r.txn,
partialNamespace: r.partialNamespace,
queryTracers: r.queryTracers,
compiledQuery: r.compiledQueries[partialResultQueryType],
instrumentation: r.instrumentation,
indexing: true,
resolvers: r.resolvers,
capabilities: r.capabilities,
strictBuiltinErrors: r.strictBuiltinErrors,
nondeterministicBuiltins: r.nondeterministicBuiltins,
}

disableInlining := r.disableInlining
Expand Down Expand Up @@ -2441,6 +2463,7 @@ func (r *Rego) partial(ctx context.Context, ectx *EvalContext) (*PartialQueries,
WithInstrumentation(ectx.instrumentation).
WithUnknowns(unknowns).
WithDisableInlining(ectx.disableInlining).
WithNondeterministicBuiltins(ectx.nondeterministicBuiltins).
WithRuntime(r.runtime).
WithIndexing(ectx.indexing).
WithEarlyExit(ectx.earlyExit).
Expand Down
13 changes: 7 additions & 6 deletions v1/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -1405,6 +1405,7 @@ func (s *Server) v1CompilePost(w http.ResponseWriter, r *http.Request) {
rego.ParsedInput(request.Input),
rego.ParsedUnknowns(request.Unknowns),
rego.DisableInlining(request.Options.DisableInlining),
rego.NondeterministicBuiltins(request.Options.NondeterminsiticBuiltins),
rego.QueryTracer(buf),
rego.Instrument(includeInstrumentation),
rego.Metrics(m),
Expand Down Expand Up @@ -2856,7 +2857,8 @@ type compileRequest struct {
}

type compileRequestOptions struct {
DisableInlining []string
DisableInlining []string
NondeterminsiticBuiltins bool
}

func readInputCompilePostV1(reqBytes []byte, queryParserOptions ast.ParserOptions) (*compileRequest, *types.ErrorV1) {
Expand Down Expand Up @@ -2898,16 +2900,15 @@ func readInputCompilePostV1(reqBytes []byte, queryParserOptions ast.ParserOption
}
}

result := &compileRequest{
return &compileRequest{
Query: query,
Input: input,
Unknowns: unknowns,
Options: compileRequestOptions{
DisableInlining: request.Options.DisableInlining,
DisableInlining: request.Options.DisableInlining,
NondeterminsiticBuiltins: request.Options.NondeterministicBuiltins,
},
}

return result, nil
}, nil
}

var indexHTML, _ = template.New("index").Parse(`
Expand Down
3 changes: 2 additions & 1 deletion v1/server/types/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,8 @@ type CompileRequestV1 struct {
Query string `json:"query"`
Unknowns *[]string `json:"unknowns"`
Options struct {
DisableInlining []string `json:"disableInlining,omitempty"`
DisableInlining []string `json:"disableInlining,omitempty"`
NondeterministicBuiltins bool `json:"nondeterministicBuiltins"`
} `json:"options,omitempty"`
}

Expand Down
12 changes: 11 additions & 1 deletion v1/topdown/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ type Query struct {
instr *Instrumentation
disableInlining []ast.Ref
shallowInlining bool
nondeterministicBuiltins bool
genvarprefix string
runtime *ast.Term
builtins map[string]*Builtin
Expand Down Expand Up @@ -313,6 +314,14 @@ func (q *Query) WithVirtualCache(vc VirtualCache) *Query {
return q
}

// WithNondeterministicBuiltins causes non-deterministic builtins to be evalued
// during partial evaluation. This is needed to pull in external data, or validate
// a JWT, during PE, so that the result informs what queries are returned.
func (q *Query) WithNondeterministicBuiltins(yes bool) *Query {
q.nondeterministicBuiltins = yes
return q
}

// PartialRun executes partial evaluation on the query with respect to unknown
// values. Partial evaluation attempts to evaluate as much of the query as
// possible without requiring values for the unknowns set on the query. The
Expand Down Expand Up @@ -380,7 +389,8 @@ func (q *Query) PartialRun(ctx context.Context) (partials []ast.Body, support []
saveNamespace: ast.StringTerm(q.partialNamespace),
skipSaveNamespace: q.skipSaveNamespace,
inliningControl: &inliningControl{
shallow: q.shallowInlining,
shallow: q.shallowInlining,
nondeterministicBuiltins: q.nondeterministicBuiltins,
},
genvarprefix: q.genvarprefix,
runtime: q.runtime,
Expand Down
13 changes: 10 additions & 3 deletions v1/topdown/save.go
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,13 @@ func saveRequired(c *ast.Compiler, ic *inliningControl, icIgnoreInternal bool, s
}
switch node := node.(type) {
case *ast.Expr:
found = len(node.With) > 0 || ignoreExprDuringPartial(node)
found = len(node.With) > 0
if found {
return found
}
if !ic.nondeterministicBuiltins { // skip evaluating non-det builtins for PE
found = ignoreExprDuringPartial(node)
}
case *ast.Term:
switch v := node.Value.(type) {
case ast.Var:
Expand Down Expand Up @@ -422,8 +428,9 @@ func ignoreDuringPartial(bi *ast.Builtin) bool {
}

type inliningControl struct {
shallow bool
disable []disableInliningFrame
shallow bool
disable []disableInliningFrame
nondeterministicBuiltins bool // evaluate non-det builtins during PE (if args are known)
}

type disableInliningFrame struct {
Expand Down
Loading