diff --git a/internal/planner/determinism_test.go b/internal/planner/determinism_test.go new file mode 100644 index 00000000000..4ed97601185 --- /dev/null +++ b/internal/planner/determinism_test.go @@ -0,0 +1,108 @@ +// Copyright 2026 The OPA Authors. All rights reserved. +// Use of this source code is governed by an Apache2 +// license that can be found in the LICENSE file. + +package planner + +import ( + "bytes" + "encoding/json" + "fmt" + "slices" + "strings" + "testing" + + "github.com/open-policy-agent/opa/v1/ast" +) + +// planToJSON is a helper function that plans an entrypoint using a slice of +// compiled modules, and returns the resulting IR policy as JSON bytes. +// compiler must be the ast.Compiler that produced the modules. +func planToJSON(t *testing.T, compiler *ast.Compiler, entrypoint string, modules []*ast.Module) []byte { + t.Helper() + + // Build the entrypoint query. Query is: `result = data.` + resultSym := ast.VarTerm("result") + ep := ast.MustParseRef("data." + entrypoint) + qc := compiler.QueryCompiler() + compiled, err := qc.Compile(ast.NewBody(ast.Equality.Expr(resultSym, ast.NewTerm(ep)))) + if err != nil { + t.Fatal(err) + } + + p := New(). + WithQueries([]QuerySet{ + { + Name: entrypoint, + Queries: []ast.Body{compiled}, + RewrittenVars: qc.RewrittenVars(), + }, + }). + WithModules(modules). + WithBuiltinDecls(ast.BuiltinMap) + + policy, err := p.Plan() + if err != nil { + t.Fatal(err) + } + + bs, err := json.Marshal(policy) + if err != nil { + t.Fatal(err) + } + return bs +} + +// TestPlannerDeterministicRuleOrder is a regression test ensuring that the +// planner's outputs do not depend on ordering of the rules provided to it. +// +// This test compiles a module once, and then plans it twice, with the Rules +// slice in two different orders. This allows detecting if the planner is +// not iterating over the rule trie in a deterministic ordering. We use +// >12 rules because Go's sort.Slice uses a stable insertion sort for <=12 +// elements and an unstable pdqsort above that. +// +// Warning: This test relies on implementation details of the Golang default +// sorting algorithm. If that algorithm changes, this test might no longer +// accurately exercise unstable sorting algorithm issues. +func TestPlannerDeterministicRuleOrder(t *testing.T) { + const n = 32 // > 12 to force the unstable pdqsort path in the default sort. + + var src strings.Builder + src.WriteString("package authz\nimport rego.v1\n") + for i := range n { + fmt.Fprintf(&src, "p.field%02d := %d\n", i, i) + } + // A parent ref-head rule (defined last) so the trie node for p accumulates + // the 'field' children before the parent node is inserted into the rule trie. + src.WriteString("p[k] := v if { k := input.k; v := input.v }\n") + + m, err := ast.ParseModuleWithOpts("mod.rego", src.String(), ast.ParserOptions{AllFutureKeywords: true}) + if err != nil { + t.Fatal(err) + } + + compiler := ast.NewCompiler() + compiler.Compile(map[string]*ast.Module{"mod.rego": m}) + if compiler.Failed() { + t.Fatalf("compile failed: %v", compiler.Errors) + } + compiled := compiler.Modules["mod.rego"] + + planFrom := func(rules []*ast.Rule) []byte { + clone := compiled.Copy() + clone.Rules = rules + return planToJSON(t, compiler, "authz.p", []*ast.Module{clone}) + } + + forwardRules := slices.Clone(compiled.Rules) + reversedRules := slices.Clone(compiled.Rules) + slices.Reverse(reversedRules) + + forward := planFrom(forwardRules) + backward := planFrom(reversedRules) + + if !bytes.Equal(forward, backward) { + t.Fatalf("plan IR depends on rule order:\nforward=%s\n\nreversed=%s", forward, backward) + } +} diff --git a/internal/planner/planner.go b/internal/planner/planner.go index f3e76ee8542..9e71e83e725 100644 --- a/internal/planner/planner.go +++ b/internal/planner/planner.go @@ -161,7 +161,18 @@ func (p *Planner) buildFunctrie() error { } func (p *Planner) planRules(rules []*ast.Rule) (string, error) { - // We know the rules with closer to the root (shorter static path) are ordered first. + // We sort rules, first by ref length, and then using the + // Ref.Compare method to break ties. This yields a stable + // sorting order for the slice of rules to be planned. + sort.Slice(rules, func(i, j int) bool { + li, lj := len(rules[i].Ref()), len(rules[j].Ref()) + if li != lj { + return li > lj + } + return rules[i].Ref().Compare(rules[j].Ref()) < 0 + }) + + // We know the rules that are closer to the root (shorter static path) are ordered first. pathRef := rules[0].Ref() // figure out what our rules' collective name/path is: @@ -262,12 +273,6 @@ func (p *Planner) planRules(rules []*ast.Rule) (string, error) { var defaultRule *ast.Rule var ruleLoc *location.Location - // We sort rules by ref length, to ensure that when merged, we can detect conflicts when one - // rule attempts to override values (deep and shallow) defined by another rule. - sort.Slice(rules, func(i, j int) bool { - return len(rules[i].Ref()) > len(rules[j].Ref()) - }) - // Generate function blocks for rules. for i := range rules { diff --git a/v1/compile/compile.go b/v1/compile/compile.go index d52f5d898f1..d77ad237809 100644 --- a/v1/compile/compile.go +++ b/v1/compile/compile.go @@ -663,9 +663,11 @@ func (c *Compiler) compilePlan(context.Context) error { } // Prepare modules and builtins for the planner. + // We sort the list of module names here to ensure a deterministic + // output ordering for the planner. modules := make([]*ast.Module, 0, len(c.compiler.Modules)) - for _, module := range c.compiler.Modules { - modules = append(modules, module) + for _, name := range util.KeysSorted(c.compiler.Modules) { + modules = append(modules, c.compiler.Modules[name]) } builtins := make(map[string]*ast.Builtin, len(c.capabilities.Builtins)) diff --git a/v1/compile/compile_test.go b/v1/compile/compile_test.go index ba1ae4a601f..4e3b2df4e69 100644 --- a/v1/compile/compile_test.go +++ b/v1/compile/compile_test.go @@ -2362,6 +2362,56 @@ func TestCompilerPlanTarget(t *testing.T) { } } +// TestCompilerPlanTargetDeterministicModuleOrder is a regression test that +// checks that the plan target's output does not depend on randomized Go +// map iteration order. +// +// To exercise this, we build the plan target with a fixed set of modules, +// each providing rules in the same package, and compile the target several +// times in a row. Because the planner is sensitive to module ordering in its +// inputs, any differences in the output plan files indicates that the +// compiler is providing the modules in a non-deterministic order, such as +// when using the c.compiler.Modules map directly. +func TestCompilerPlanTargetDeterministicModuleOrder(t *testing.T) { + const numModules = 40 + + files := map[string]string{} + for i := range numModules { + files[fmt.Sprintf("mod%03d.rego", i)] = fmt.Sprintf( + "package authz\nimport rego.v1\nallow if input.x == %d\n", i) + } + + planBytes := func() []byte { + t.Helper() + var out []byte + // Use the in-memory FS so module file paths are stable across builds. + test.WithTestFS(files, true, func(root string, fsys fs.FS) { + compiler := New(). + WithFS(fsys). + WithPaths(root). + WithTarget("plan"). + WithEntrypoints("authz/allow") + if err := compiler.Build(t.Context()); err != nil { + t.Fatal(err) + } + if len(compiler.bundle.PlanModules) == 0 { + t.Fatal("expected to find compiled plan module") + } + out = slices.Clone(compiler.bundle.PlanModules[0].Raw) + }) + return out + } + + want := planBytes() + // Build several more times. Go randomizes map iteration order for each map + // instance, so unstable module ordering should surface here. + for i := range 16 { + if got := planBytes(); !bytes.Equal(want, got) { + t.Fatalf("plan bytes differ across builds on iteration %d:\nwant=%s\n\ngot=%s", i, want, got) + } + } +} + func TestCompilerPlanTargetPruneUnused(t *testing.T) { files := map[string]string{ "test.rego": `package test diff --git a/v1/rego/rego.go b/v1/rego/rego.go index 6f2a824cb5a..21747f3e93e 100644 --- a/v1/rego/rego.go +++ b/v1/rego/rego.go @@ -3082,9 +3082,11 @@ func generateJSON(term *ast.Term, ectx *EvalContext) (any, error) { } func (r *Rego) planQuery(queries []ast.Body, evalQueryType queryType) (*ir.Policy, error) { + // We sort the list of module names here to ensure a deterministic + // output ordering for the planner. modules := make([]*ast.Module, 0, len(r.compiler.Modules)) - for _, module := range r.compiler.Modules { - modules = append(modules, module) + for _, name := range util.KeysSorted(r.compiler.Modules) { + modules = append(modules, r.compiler.Modules[name]) } decls := make(map[string]*ast.Builtin, len(r.builtinDecls)+len(ast.BuiltinMap))