mirror of
https://github.com/open-policy-agent/opa.git
synced 2026-08-12 19:32:48 -06:00
compile,planner: improve determinism of plan/wasm bundle builds (#8732)
This commit fixes an issue where `plan` and `wasm` bundle build targets could produce different output bytes across separate invocations of `opa build` for the same inputs. There were two underlying causes, both from Golang random map iteration order leaking through to the order-sensitive planner. Causes: - `compilePlan` (`v1/compile`) and `planQuery` (`v1/rego`) iterated over the compiler's module map without sorting keys first. This caused the planner to have iteration-dependent variations in output. This was fixed by sorting the module names before use. - `planRules` (`internal/planner`) sorted rules by length of the rule name ref, which is not a unique value. Because the sorting of the rules was using an unstable sorting algorithm, and the rule names were coming from iterating over a `map` type in the rule trie, this had edge cases where non-deterministic output ordering could creep in. This was fixed by adding a ref `Compare` call as a tie-breaker to get a stable sorting order, regardless of iteration order in the rule trie. This commit also adds regression tests that assert plan output is independent of module and rule ordering. The two fixes are needed together because both sets of issues hit the planner from different angles, and are mostly independent of each other. Signed-off-by: Philip Conrad <philip_conrad@apple.com>
This commit is contained in:
@@ -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.<entrypoint>`
|
||||
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)
|
||||
}
|
||||
}
|
||||
@@ -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 {
|
||||
|
||||
|
||||
Reference in New Issue
Block a user