diff --git a/v1/ast/compile.go b/v1/ast/compile.go index fe1a656306..311dd9917c 100644 --- a/v1/ast/compile.go +++ b/v1/ast/compile.go @@ -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 { diff --git a/v1/ast/external_source.go b/v1/ast/external_source.go index ce3433cce0..88c98eedb9 100644 --- a/v1/ast/external_source.go +++ b/v1/ast/external_source.go @@ -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. diff --git a/v1/ast/external_source_test.go b/v1/ast/external_source_test.go index 45adb822ae..21d1ff61e4 100644 --- a/v1/ast/external_source_test.go +++ b/v1/ast/external_source_test.go @@ -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) + } + } + }) +} diff --git a/v1/topdown/eval.go b/v1/topdown/eval.go index 7822477e58..2f644ac52b 100644 --- a/v1/topdown/eval.go +++ b/v1/topdown/eval.go @@ -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