topdown+builtins: Restore legacy IgnoreDuringPartialEval list. (#5175)

This commit is a followup to the work done in PR #5172, and partially
reverts the changes there to avoid breaking library users of OPA.

The fix restores the `IgnoreDuringPartialEval` list while still
ensuring that non-deterministic builtins are not run during partial
evaluation.

Signed-off-by: Philip Conrad <philipaconrad@gmail.com>

ast/builtins.go

@@ -284,6 +284,21 @@ var DefaultBuiltins = [...]*Builtin{
 // built-in definitions.
 var BuiltinMap map[string]*Builtin
 
+// Deprecated: Builtins can now be directly annotated with the
+// Nondeterministic property, and when set to true, will be ignored
+// for partial evaluation.
+var IgnoreDuringPartialEval = []*Builtin{
+	RandIntn,
+	UUIDRFC4122,
+	JWTDecodeVerify,
+	JWTEncodeSignRaw,
+	JWTEncodeSign,
+	NowNanos,
+	HTTPSend,
+	OPARuntime,
+	NetLookupIPAddr,
+}
+
 /**
  * Unification
  */

docs/content/extensions.md

@@ -107,6 +107,7 @@ r := rego.New(
 			Name: "github.repo",
 			Decl: types.NewFunction(types.Args(types.S, types.S), types.A),
 			Memoize: true,
+			Nondeterministic: true,
 		},
 		func(bctx rego.BuiltinContext, a, b *ast.Term) (*ast.Term, error) {
 			// see implementation below.
@@ -132,6 +133,13 @@ across multiple calls in the same query. If your built-in function performs I/O,
 you should enable memoization as it ensures function evaluation is
 deterministic.
 
+Since this built-in could have non-deterministic results, depending on network
+conditions, the declaration also sets `rego.Function#Nondeterministic` to true.
+This provides basic safety information to the runtime, so that the function
+isn't accidentally run during bundle builds or partial evaluation. If your
+builtin can have non-deterministic results, you should mark it appropriately
+to avoid surprises.
+
 The implementation wraps the Go standard library to perform HTTP requests to
 GitHub's API:
 

rego/example_test.go

@@ -16,6 +16,7 @@ import (
 
 	"github.com/open-policy-agent/opa/logging"
 	"github.com/open-policy-agent/opa/storage/disk"
+	"github.com/open-policy-agent/opa/topdown/builtins"
 
 	"github.com/open-policy-agent/opa/ast"
 	"github.com/open-policy-agent/opa/rego"
@@ -945,6 +946,47 @@ func ExampleRego_custom_function_caching() {
 	// y: 1
 }
 
+func ExampleRego_custom_function_nondeterministic() {
+	ndbCache := builtins.NDBCache{}
+	r := rego.New(
+		// An example query that uses a custom function.
+		rego.Query(`x = myrandom()`),
+
+		// A custom function that uses caching.
+		rego.FunctionDyn(
+			&rego.Function{
+				Name:             "myrandom",
+				Memoize:          true,
+				Nondeterministic: true,
+				Decl: types.NewFunction(
+					nil,     // one string input
+					types.N, // one number output
+				),
+			},
+			func(_ topdown.BuiltinContext, args []*ast.Term) (*ast.Term, error) {
+				i := 55555
+				return ast.IntNumberTerm(i), nil
+			},
+		),
+		rego.NDBuiltinCache(ndbCache),
+	)
+
+	rs, err := r.Eval(context.Background())
+	if err != nil {
+		// handle error
+	}
+
+	// Check the binding, and what the NDBCache saw.
+	// This ensures the Nondeterministic flag propagated through correctly.
+	fmt.Println("x:", rs[0].Bindings["x"])
+	fmt.Println("NDBCache:", ndbCache["myrandom"])
+
+	// Output:
+	//
+	// x: 55555
+	// NDBCache: {[]: 55555}
+}
+
 func ExampleRego_custom_function_global() {
 
 	decl := &rego.Function{

rego/rego.go

@@ -532,9 +532,10 @@ type Rego struct {
 
 // Function represents a built-in function that is callable in Rego.
 type Function struct {
-	Name    string
-	Decl    *types.Function
-	Memoize bool
+	Name             string
+	Decl             *types.Function
+	Memoize          bool
+	Nondeterministic bool
 }
 
 // BuiltinContext contains additional attributes from the evaluator that
@@ -561,8 +562,9 @@ type (
 // RegisterBuiltin1 adds a built-in function globally inside the OPA runtime.
 func RegisterBuiltin1(decl *Function, impl Builtin1) {
 	ast.RegisterBuiltin(&ast.Builtin{
-		Name: decl.Name,
-		Decl: decl.Decl,
+		Name:             decl.Name,
+		Decl:             decl.Decl,
+		Nondeterministic: decl.Nondeterministic,
 	})
 	topdown.RegisterBuiltinFunc(decl.Name, func(bctx BuiltinContext, terms []*ast.Term, iter func(*ast.Term) error) error {
 		result, err := memoize(decl, bctx, terms, func() (*ast.Term, error) { return impl(bctx, terms[0]) })
@@ -573,8 +575,9 @@ func RegisterBuiltin1(decl *Function, impl Builtin1) {
 // RegisterBuiltin2 adds a built-in function globally inside the OPA runtime.
 func RegisterBuiltin2(decl *Function, impl Builtin2) {
 	ast.RegisterBuiltin(&ast.Builtin{
-		Name: decl.Name,
-		Decl: decl.Decl,
+		Name:             decl.Name,
+		Decl:             decl.Decl,
+		Nondeterministic: decl.Nondeterministic,
 	})
 	topdown.RegisterBuiltinFunc(decl.Name, func(bctx BuiltinContext, terms []*ast.Term, iter func(*ast.Term) error) error {
 		result, err := memoize(decl, bctx, terms, func() (*ast.Term, error) { return impl(bctx, terms[0], terms[1]) })
@@ -585,8 +588,9 @@ func RegisterBuiltin2(decl *Function, impl Builtin2) {
 // RegisterBuiltin3 adds a built-in function globally inside the OPA runtime.
 func RegisterBuiltin3(decl *Function, impl Builtin3) {
 	ast.RegisterBuiltin(&ast.Builtin{
-		Name: decl.Name,
-		Decl: decl.Decl,
+		Name:             decl.Name,
+		Decl:             decl.Decl,
+		Nondeterministic: decl.Nondeterministic,
 	})
 	topdown.RegisterBuiltinFunc(decl.Name, func(bctx BuiltinContext, terms []*ast.Term, iter func(*ast.Term) error) error {
 		result, err := memoize(decl, bctx, terms, func() (*ast.Term, error) { return impl(bctx, terms[0], terms[1], terms[2]) })
@@ -597,8 +601,9 @@ func RegisterBuiltin3(decl *Function, impl Builtin3) {
 // RegisterBuiltin4 adds a built-in function globally inside the OPA runtime.
 func RegisterBuiltin4(decl *Function, impl Builtin4) {
 	ast.RegisterBuiltin(&ast.Builtin{
-		Name: decl.Name,
-		Decl: decl.Decl,
+		Name:             decl.Name,
+		Decl:             decl.Decl,
+		Nondeterministic: decl.Nondeterministic,
 	})
 	topdown.RegisterBuiltinFunc(decl.Name, func(bctx BuiltinContext, terms []*ast.Term, iter func(*ast.Term) error) error {
 		result, err := memoize(decl, bctx, terms, func() (*ast.Term, error) { return impl(bctx, terms[0], terms[1], terms[2], terms[3]) })
@@ -609,8 +614,9 @@ func RegisterBuiltin4(decl *Function, impl Builtin4) {
 // RegisterBuiltinDyn adds a built-in function globally inside the OPA runtime.
 func RegisterBuiltinDyn(decl *Function, impl BuiltinDyn) {
 	ast.RegisterBuiltin(&ast.Builtin{
-		Name: decl.Name,
-		Decl: decl.Decl,
+		Name:             decl.Name,
+		Decl:             decl.Decl,
+		Nondeterministic: decl.Nondeterministic,
 	})
 	topdown.RegisterBuiltinFunc(decl.Name, func(bctx BuiltinContext, terms []*ast.Term, iter func(*ast.Term) error) error {
 		result, err := memoize(decl, bctx, terms, func() (*ast.Term, error) { return impl(bctx, terms) })
@@ -2544,8 +2550,9 @@ func finishFunction(name string, bctx topdown.BuiltinContext, result *ast.Term,
 func newFunction(decl *Function, f topdown.BuiltinFunc) func(*Rego) {
 	return func(r *Rego) {
 		r.builtinDecls[decl.Name] = &ast.Builtin{
-			Name: decl.Name,
-			Decl: decl.Decl,
+			Name:             decl.Name,
+			Decl:             decl.Decl,
+			Nondeterministic: decl.Nondeterministic,
 		}
 		r.builtinFuncs[decl.Name] = &topdown.Builtin{
 			Decl: r.builtinDecls[decl.Name],

topdown/save.go

@@ -375,7 +375,15 @@ func ignoreExprDuringPartial(expr *ast.Expr) bool {
 }
 
 func ignoreDuringPartial(bi *ast.Builtin) bool {
-	// Note(philipc): For now, we throw out all non-deterministic builtins.
+	// Note(philipc): We keep this legacy check around to avoid breaking
+	// existing library users.
+	//nolint:staticcheck // We specifically ignore our own linter warning here.
+	for _, ignore := range ast.IgnoreDuringPartialEval {
+		if bi == ignore {
+			return true
+		}
+	}
+	// Otherwise, ensure all non-deterministic builtins are thrown out.
 	return bi.Nondeterministic
 }