ast+topdown: let external sources distinguish absent from unknown

External rule sources received a resolver (via ExternalIndex.Tree) that reported both an input reference absent from a concrete input and one that is symbolic under partial evaluation as the same UnknownValueErr. A source therefore could not tell 'concretely missing' apart from 'deliberately unknown' on a per-reference basis (e.g. input.foo unknown while input.bar is known), which matters when a source translates input into an external lookup during partial evaluation. Pass the save-set-aware evaluator to ExternalIndex.Tree instead of the raw input document, mirroring the resolver the built-in rule indexer already uses. Sources opt into the new behavior via ExternalSourceOptions.DistinguishAbsentFromUnknown: when set, an unknown reference returns UnknownValueErr while an absent one resolves to (nil, nil). The default is unchanged, preserving the previous collapse for existing sources. The ExternalRuleSource/ExternalRuleIndex interfaces are unchanged. Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
2026-08-12 19:32:48 -06:00 · 2026-07-09 14:12:08 +02:00
parent 78f31260f6
commit d517d1b914
4 changed files with 202 additions and 18 deletions
@@ -4298,16 +4298,44 @@ func (n *TreeNode) add(path Ref, val any) {
 	}
 }

+// ExternalIndex ties an ExternalRuleSource-provided index to the package Ref it
+// serves. It is internal plumbing exported only so the topdown evaluator can
+// reach it across the ast/topdown package boundary; it is not part of OPA's
+// supported public API and may change without notice. The stable surface for
+// implementing external rule sources is the ExternalRuleSource and
+// ExternalRuleIndex interfaces.
 type ExternalIndex struct {
 	Index ExternalRuleIndex
 	Ref   Ref
 }

-func (ei *ExternalIndex) Tree(ctx context.Context, rt *TreeNode, prefix Ref, input *Term, m metrics.Metrics, reqMD map[string]any, respMD map[string]any) (*TreeNode, ExternalRuleIndex, error) {
-	resolver := &termResolver{input: input}
+// Tree resolves external rules for prefix, using resolver to resolve references
+// while building search queries. Passing a save-set-aware resolver (e.g. the
+// topdown evaluator) lets sources that opt into
+// ExternalSourceOptions.DistinguishAbsentFromUnknown distinguish absent input
+// from values that are unknown under partial evaluation.
+//
+// Like ExternalIndex, Tree is internal plumbing exported only for the topdown
+// evaluator. It is not part of OPA's supported public API and may change
+// without notice.
+func (ei *ExternalIndex) Tree(ctx context.Context, rt *TreeNode, prefix Ref, resolver ValueResolver, m metrics.Metrics, reqMD map[string]any, respMD map[string]any) (*TreeNode, ExternalRuleIndex, error) {
+	o := ei.Index.Opts()
+
+	// Select the resolver handed to the source. By default we wrap the caller's
+	// resolver so external sources see the legacy behavior (absent and unknown
+	// both collapse to UnknownValueErr, non-input refs are never resolved).
+	// Sources that set DistinguishAbsentFromUnknown receive the caller's
+	// save-set-aware resolver unchanged, letting them tell absent from unknown.
+	lookupResolver := resolver
+	switch {
+	case lookupResolver == nil:
+		lookupResolver = unknownResolver{}
+	case o == nil || !o.DistinguishAbsentFromUnknown:
+		lookupResolver = legacyExternalResolver{inner: lookupResolver}
+	}

 	rules, updatedIndex, err := ei.Index.Lookup(ctx,
-		LookupResolver(resolver),
+		LookupResolver(lookupResolver),
 		LookupMetrics(m),
 		LookupRequestMetadata(reqMD),
 		LookupResponseMetadata(respMD),
@@ -4317,7 +4345,7 @@ func (ei *ExternalIndex) Tree(ctx context.Context, rt *TreeNode, prefix Ref, inp
 	}
 	c0 := NewCompiler()

-	if o := ei.Index.Opts(); o != nil {
+	if o != nil {
 		if len(o.SkippedStages) > 0 {
 			c0.WithSkipStages(o.SkippedStages...)
 		}
@@ -4356,24 +4384,37 @@ func (ei *ExternalIndex) Tree(ctx context.Context, rt *TreeNode, prefix Ref, inp
 	return node, updatedIndex, nil
 }

-type termResolver struct {
-	input *Term
+// legacyExternalResolver reproduces the historical external-source resolver
+// behavior on top of an arbitrary (typically save-set-aware) resolver: only
+// input references are resolvable, and any input reference that does not
+// resolve to a concrete value is reported as UnknownValueErr. This collapses
+// "absent from the concrete input" and "symbolic under partial evaluation"
+// into a single signal, matching what external sources saw before
+// ExternalSourceOptions.DistinguishAbsentFromUnknown existed.
+type legacyExternalResolver struct {
+	inner ValueResolver
 }

-func (r *termResolver) Resolve(ref Ref) (Value, error) {
-	if ref.HasPrefix(InputRootRef) {
-		if r.input == nil {
-			return nil, UnknownValueErr{}
-		}
-		v, err := r.input.Value.Find(ref[1:])
-		if err != nil {
-			return nil, UnknownValueErr{}
-		}
-		return v, nil
+func (r legacyExternalResolver) Resolve(ref Ref) (Value, error) {
+	if !ref.HasPrefix(InputRootRef) {
+		return nil, UnknownValueErr{}
 	}
-	return nil, UnknownValueErr{}
+	v, err := r.inner.Resolve(ref)
+	if err != nil {
+		return nil, err
+	}
+	if v == nil {
+		return nil, UnknownValueErr{}
+	}
+	return v, nil
 }

+// unknownResolver treats every reference as unknown. It is used as a safe
+// fallback when Tree is invoked without a resolver.
+type unknownResolver struct{}
+
+func (unknownResolver) Resolve(Ref) (Value, error) { return nil, UnknownValueErr{} }
+
 // Size returns the number of rules in the tree.
 func (n *TreeNode) Size() (s int) {
 	for _, c := range n.Children {
@@ -68,6 +68,25 @@ type ExternalSourceOptions struct {
 	// This is forward-compatible: new compiler stages added in future releases
 	// will be skipped automatically rather than running unexpectedly.
 	SkippedStages []StageID
+
+	// DistinguishAbsentFromUnknown controls how the resolver passed to Lookup
+	// (via LookupOptions.Resolver) reports references that do not resolve to a
+	// concrete value.
+	//
+	// When false (default), the legacy behavior is preserved for backwards
+	// compatibility: only input references are resolvable, and any input
+	// reference that cannot be resolved — whether it is genuinely absent from a
+	// concrete input or symbolic under partial evaluation — surfaces as
+	// UnknownValueErr. The two cases are indistinguishable.
+	//
+	// When true, the source opts into the same save-set-aware resolver the
+	// built-in rule indexer uses: a reference that is unknown under partial
+	// evaluation returns UnknownValueErr, while a reference that is simply
+	// absent from an otherwise-concrete input resolves to (nil, nil). This lets
+	// a source tell "deliberately symbolic" apart from "concretely missing"
+	// on a per-reference basis (e.g. input.foo unknown while input.bar is
+	// known). See ValueResolver and IsUnknownValueErr.
+	DistinguishAbsentFromUnknown bool
 }

 // LookupOption is a functional option for ExternalRuleIndex.Lookup calls.
@@ -70,3 +70,121 @@ func TestCompilerRuleIndexReturnsNilForExternalSources(t *testing.T) {
 		t.Errorf("Expected GetRules NOT to be called at compile-time, got %d calls", source.getCallCount())
 	}
 }
+
+// fakeEvalResolver mimics the topdown evaluator's save-set-aware resolver:
+// refs covered by an unknown prefix are UnknownValueErr, input refs present in
+// the concrete input resolve to their value, and input refs that are simply
+// absent resolve to (nil, nil).
+type fakeEvalResolver struct {
+	unknowns []Ref
+	input    Value
+}
+
+func (r fakeEvalResolver) Resolve(ref Ref) (Value, error) {
+	for _, u := range r.unknowns {
+		if ref.HasPrefix(u) {
+			return nil, UnknownValueErr{}
+		}
+	}
+	if ref.HasPrefix(InputRootRef) {
+		if r.input == nil {
+			return nil, nil
+		}
+		v, err := r.input.Find(ref[1:])
+		if err != nil {
+			return nil, nil
+		}
+		return v, nil
+	}
+	return nil, UnknownValueErr{}
+}
+
+type resolveResult struct {
+	val     Value
+	unknown bool
+	err     error
+}
+
+// resolverCapturingIndex records what the resolver handed to Lookup returns for
+// a fixed set of refs, so tests can assert how absent/unknown/known are
+// surfaced under each ExternalSourceOptions setting.
+type resolverCapturingIndex struct {
+	distinguish bool
+	got         map[string]resolveResult
+}
+
+func (idx *resolverCapturingIndex) Opts() *ExternalSourceOptions {
+	return &ExternalSourceOptions{DistinguishAbsentFromUnknown: idx.distinguish}
+}
+
+func (idx *resolverCapturingIndex) Lookup(_ context.Context, opts ...LookupOption) ([]*Rule, ExternalRuleIndex, error) {
+	o := LookupOptions{}
+	for _, opt := range opts {
+		opt(&o)
+	}
+	res := o.Resolver()
+	for _, s := range []string{"input.foo", "input.bar", "input.baz"} {
+		v, err := res.Resolve(MustParseRef(s))
+		idx.got[s] = resolveResult{val: v, unknown: IsUnknownValueErr(err), err: err}
+	}
+	return nil, nil, nil
+}
+
+func TestExternalSourceResolverDistinguishesAbsentFromUnknown(t *testing.T) {
+	rt := NewRuleTree(NewModuleTree(nil))
+	prefix := MustParseRef("data.authz")
+
+	// input.foo is unknown (partial eval), input.bar is concretely known, and
+	// input.baz is neither declared unknown nor present -> genuinely absent.
+	resolver := fakeEvalResolver{
+		unknowns: []Ref{MustParseRef("input.foo")},
+		input:    MustParseTerm(`{"bar": "known"}`).Value,
+	}
+
+	t.Run("opted-in source tells absent from unknown", func(t *testing.T) {
+		idx := &resolverCapturingIndex{distinguish: true, got: map[string]resolveResult{}}
+		ei := &ExternalIndex{Index: idx, Ref: prefix}
+		if _, _, err := ei.Tree(context.Background(), rt, prefix, resolver, nil, nil, nil); err != nil {
+			t.Fatal(err)
+		}
+		if got := idx.got["input.foo"]; !got.unknown {
+			t.Errorf("input.foo: want UnknownValueErr, got val=%v err=%v", got.val, got.err)
+		}
+		if got := idx.got["input.bar"]; got.unknown || got.val == nil || got.val.String() != `"known"` {
+			t.Errorf("input.bar: want value \"known\", got val=%v unknown=%v", got.val, got.unknown)
+		}
+		if got := idx.got["input.baz"]; got.unknown || got.err != nil || got.val != nil {
+			t.Errorf("input.baz: want absent (nil,nil), got val=%v unknown=%v err=%v", got.val, got.unknown, got.err)
+		}
+	})
+
+	t.Run("default collapses absent into unknown", func(t *testing.T) {
+		idx := &resolverCapturingIndex{distinguish: false, got: map[string]resolveResult{}}
+		ei := &ExternalIndex{Index: idx, Ref: prefix}
+		if _, _, err := ei.Tree(context.Background(), rt, prefix, resolver, nil, nil, nil); err != nil {
+			t.Fatal(err)
+		}
+		if got := idx.got["input.foo"]; !got.unknown {
+			t.Errorf("input.foo: want UnknownValueErr, got %+v", got)
+		}
+		if got := idx.got["input.bar"]; got.unknown || got.val == nil {
+			t.Errorf("input.bar: want value, got %+v", got)
+		}
+		if got := idx.got["input.baz"]; !got.unknown {
+			t.Errorf("input.baz: want UnknownValueErr (legacy collapse), got %+v", got)
+		}
+	})
+
+	t.Run("nil resolver is treated as all-unknown", func(t *testing.T) {
+		idx := &resolverCapturingIndex{distinguish: true, got: map[string]resolveResult{}}
+		ei := &ExternalIndex{Index: idx, Ref: prefix}
+		if _, _, err := ei.Tree(context.Background(), rt, prefix, nil, nil, nil, nil); err != nil {
+			t.Fatal(err)
+		}
+		for _, ref := range []string{"input.foo", "input.bar", "input.baz"} {
+			if got := idx.got[ref]; !got.unknown {
+				t.Errorf("%s: want UnknownValueErr with nil resolver, got %+v", ref, got)
+			}
+		}
+	})
+}
@@ -2641,7 +2641,13 @@ func (e evalTree) next(iter unifyIterator, plugged *ast.Term) error {
 				} else {
 					// Call Tree() and cache the result
 					e.e.instr.startTimer(evalOpExternalRuleSource)
-					tree, updatedIndex, err := node.External.Tree(e.e.ctx, e.e.compiler.RuleTree, externalRef, e.e.input, e.e.metrics, e.e.requestMetadata, e.e.responseMetadata)
+					// Pass the eval itself as the resolver: it is save-set
+					// aware, so external sources that opt into
+					// ExternalSourceOptions.DistinguishAbsentFromUnknown can
+					// tell references that are unknown under partial evaluation
+					// apart from references that are simply absent from the
+					// concrete input.
+					tree, updatedIndex, err := node.External.Tree(e.e.ctx, e.e.compiler.RuleTree, externalRef, e.e, e.e.metrics, e.e.requestMetadata, e.e.responseMetadata)
 					e.e.instr.stopTimer(evalOpExternalRuleSource)
 					if err != nil {
 						return err