expand: ExpansionFilter hook to suppress redundant derived grants#778
Draft
leet-c1 wants to merge 1 commit into
Draft
expand: ExpansionFilter hook to suppress redundant derived grants#778leet-c1 wants to merge 1 commit into
leet-c1 wants to merge 1 commit into
Conversation
Add ExpansionFilter — an optional per-source-grant predicate on Expander
that, when it returns false, skips emission of the derived grant for that
(principal, descendant_entitlement) pair. Callers opt in via
expand.WithExpansionFilter(f); default behavior (nil filter) is unchanged.
Motivation
----------
Connectors that use a sparse/authoritative grant model — e.g. anything
emitting ScopeBindingTrait — can end up double-representing access when
they also wire GrantExpandable edges from role entitlements to action
entitlements. The expander fans those out into derived grants, and the
derived grants duplicate information the sparse grants already carry.
This hook lets a connector tell the expander "don't emit a derived grant
for this source" when it knows the same access is captured elsewhere.
The SDK intentionally encodes no semantic knowledge — it only exposes
the primitive; connectors install the policy they need.
API
---
type ExpansionFilter func(ctx, sourceGrant, sourceEnt, descEnt *v2.Entitlement) bool
type ExpanderOption func(*Expander)
func WithExpansionFilter(f ExpansionFilter) ExpanderOption
func NewExpander(store, graph, opts ...ExpanderOption) *Expander
NewExpander stays backward compatible via variadic options.
Regression surface
------------------
Purely additive: a nil filter is a no-op on the existing fast path
(one cheap nil check per source grant). Full expand test suite passes
unchanged. No proto changes, no store interface changes, no behavior
change for any existing caller.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds an opt-in
ExpansionFilterhook on the grant expander. When installed, the filter decides per-source-grant whether a derived grant should be emitted. Default (nil filter) preserves existing behavior exactly.NewExpanderkeep the signature backward compatible.Why
Connectors that use a sparse/authoritative grant model (e.g. anything emitting
ScopeBindingTrait) can double-represent access when they also wireGrantExpandableedges from role entitlements to action entitlements. The expander fans those edges into derived grants, and those derived grants duplicate information the sparse grants already carry.Today the only way for such a connector to stop emitting the duplicates is either (a) remove the
GrantExpandableannotations androle.Grants()emission entirely (a large refactor per connector) or (b) add an ad-hoc early-return gate in the connector'srole.Grants. Neither composes; both bleed policy across the connector codebase.This primitive lets the connector express the policy once, as a predicate:
The SDK intentionally encodes no semantic knowledge about ScopeBinding or any specific model — it only exposes the hook. Connectors install whatever policy they need.
Regression surface
NewExpander(store, graph)signature unchanged (variadic options).filterfield defaults nil → one added nil check per source grant on the hot path, identical behavior otherwise.pkg/sync/expandtest suite passes unchanged.Tests
New
expansion_filter_test.gocovers:Status
Draft. Opening for discussion on:
sync.WithExpansionFilter) that plumbs through automatically. A syncer-level option would be additive on top of this change; happy to add in a follow-up or amend here.ExpansionFilterProvider) on theConnectorBuilderside would be preferable to per-caller wiring.No proto changes, no schema changes, no c1z format changes. Net-zero risk to existing connectors that don't adopt it.