From 6d4e8b72efa64d7aabe1b4fcaac594444b81ae7e Mon Sep 17 00:00:00 2001 From: Michael McQuade Date: Wed, 21 Oct 2026 14:20:30 +0000 Subject: [PATCH 1/8] fix: every Filter, Reachable and Holders reads every grant to the tenant's members and the public on the listed types, of built-in roles too, even for the anonymous and guests, who count none of the members' --- README.md | 6 +- authority_test.go | 114 ++++++++++++++++++++ authorize.go | 63 +++++++---- docs/architecture.md | 6 +- migrations/0019_custom_grant_principals.sql | 10 ++ roles.go | 46 +++++--- 6 files changed, 204 insertions(+), 41 deletions(-) create mode 100644 migrations/0019_custom_grant_principals.sql diff --git a/README.md b/README.md index 7850f29..3e1c499 100644 --- a/README.md +++ b/README.md @@ -459,9 +459,9 @@ pg_catalog's, so none stands in for them, and refuse a search_path that names pg would miss the writes it waits for. A migration that rebuilds an index, as `0009_grant_seeks` rebuilds two of `rbac_grants`, locks the table until it commits, so checks, lists and changes wait for the rebuild; one that builds - an index, as `0011_tenant_keys` builds three on `rbac_eligibilities` and `rbac_denies`, keeps - writes to the table waiting until it commits, though reads go on: on a large table, migrate when - that wait is acceptable. Code from before `0012_activation_through` must not run beside code + an index, as `0011_tenant_keys` builds three on `rbac_eligibilities` and `rbac_denies` and + `0019_custom_grant_principals` one on `rbac_grants`, keeps writes to the table waiting until it + commits, though reads go on: on a large table, migrate when that wait is acceptable. Code from before `0012_activation_through` must not run beside code after it: an activation it extends keeps the eligibility recorded before, and can outlast the one it was extended through. Stop the replicas running it before migrating. `0013_bounded_rows` locks every table, history too, until it commits, and reads each once for each check it adds, diff --git a/authority_test.go b/authority_test.go index 2a0a470..53ea043 100644 --- a/authority_test.go +++ b/authority_test.go @@ -3566,6 +3566,120 @@ func TestSearchesReadAsMuchOnceAConnectionPlansThemGenerically(t *testing.T) { } } +// Filter, Reachable and Holders read, of the grants to the tenant's members and the public, only +// those of custom roles that their subject counts, under a generic plan and one for their arguments, +// and as of a time: which custom roles those grants name was read from each of them on scopes of the +// listed types, of built-in roles too, so beside the members' grants on thousands of clusters a +// member's search read them all, and those of the anonymous and a guest, who count none of them; and +// a search of a permission no public role gives read the public's. +func TestSearchesReadOnlyTheBroadGrantsTheyCount(t *testing.T) { + for _, plan := range []string{"force_custom_plan", "force_generic_plan"} { + for _, broad := range []struct { + what string + // grants are the grants on clusters, of the numbers from $1 to $2. + grants string + p rbac.Permission + // counting are the searches that count the grants, and so read them. + counting []string + }{ + // Read by the members' key, or by the custom roles' index, either reads all of one or the + // other. + { + what: "the members' of a built-in role and others' of a custom role", + grants: `SELECT 't1', 'cluster', 'k' || i, 'members', 't1', 'cluster.login' FROM generate_series($1::int + 1, $2::int) i + UNION ALL SELECT 't1', 'cluster', 'o' || i, 'user', 'o' || i, 'custom.view' FROM generate_series($1::int + 1, $2::int) i`, + p: login, counting: []string{"member Reachable"}, + }, + { + what: "the members' of a custom role", + grants: `SELECT 't1', 'cluster', 'k' || i, 'members', 't1', 'custom.view' FROM generate_series($1::int + 1, $2::int) i`, + p: login, counting: []string{"member Filter", "member Reachable"}, + }, + { + what: "the public's of a public custom role, searched for a permission no public role gives", + grants: `SELECT 't1', 'cluster', 'k' || i, 'public', '', 'custom.open' FROM generate_series($1::int + 1, $2::int) i`, + p: root, + }, + } { + t.Run(plan+"/"+broad.what, func(t *testing.T) { + h := crowded(t, pgtest.Open(t, "plan_cache_mode="+plan)) + ctx := t.Context() + c0 := hostTable(t, h)[0] + g, m := h.guest(t, "g"), h.as(t, "m") + h.seed(t, grant(c0.Scope(), "cluster.login", rbac.User, "g"), grant(c0.Scope(), "cluster.login", rbac.User, "m")) + if _, err := h.pool.Exec(ctx, `INSERT INTO rbac_roles (tenant, id, permissions, resource_types, public) + VALUES ('t1', 'custom.view', '{cluster:view,cluster:login}', '{cluster}', false), ('t1', 'custom.open', '{cluster:view}', '{cluster}', true)`); err != nil { + t.Fatal(err) + } + searches := map[string]func(pgx.Tx, *rbac.Authorizer) error{ + "Holders": func(_ pgx.Tx, a *rbac.Authorizer) error { return second(a.Holders(ctx, broad.p, c0)) }, + } + for who, s := range map[string]rbac.Subject{"anonymous": {}, "guest": g, "member": m} { + if !slices.Contains(broad.counting, who+" Filter") { + searches[who+" Filter"] = func(_ pgx.Tx, a *rbac.Authorizer) error { + return second(a.Filter(ctx, s, broad.p, "t1", "cluster", rbac.Columns{Tenant: "tenant", Path: "path"}, 1)) + } + } + if !slices.Contains(broad.counting, who+" Reachable") { + searches[who+" Reachable"] = func(_ pgx.Tx, a *rbac.Authorizer) error { return second(a.Reachable(ctx, s, broad.p, "t1", "cluster")) } + } + } + granted := 0 + more := func(n int) { + t.Helper() + if _, err := h.pool.Exec(ctx, `INSERT INTO rbac_grants (tenant, scope_type, scope_id, principal_kind, principal_id, role) `+broad.grants, granted, n); err != nil { + t.Fatal(err) + } + granted = n + if _, err := h.pool.Exec(ctx, `ANALYZE rbac_grants, rbac_history`); err != nil { + t.Fatal(err) + } + } + // The pages of the grants each reads now, and, planned for its arguments as AsOf plans + // it, of history as of now. + read := func(at time.Time) map[string]int64 { + t.Helper() + pages := map[string]int64{} + for what, f := range searches { + func() { + tx, err := h.pool.Begin(ctx) + if err != nil { + t.Fatal(err) + } + defer func() { _ = tx.Rollback(ctx) }() + start := pagesRead(ctx, t, tx, "rbac_grants") + if err := f(tx, h.authz.WithTx(tx)); err != nil { + t.Fatalf("%s: %v", what, err) + } + pages[what] = pagesRead(ctx, t, tx, "rbac_grants") - start + if plan != "force_custom_plan" || strings.HasSuffix(what, "Filter") { + return + } + if err := h.authz.WithTx(tx).AsOf(ctx, "t1", at, func(a *rbac.Authorizer) error { + start := historyPages(ctx, t, tx) + err := f(tx, a) + pages[what+" as of a time"] = historyPages(ctx, t, tx) - start + return err + }); err != nil { + t.Fatalf("%s as of a time: %v", what, err) + } + }() + } + return pages + } + more(500) + few := read(dbNow(t, h)) + more(5000) + for what, many := range read(dbNow(t, h)) { + if many > few[what]*3/2+deeper { + t.Errorf("%s read %d pages beside 500 grants on clusters that are %s, and %d beside 5000", what, few[what], broad.what, many) + } + } + }) + } + } +} + // Filter's count of the grants that pick its form stops a little past where the form changes, // however many of a built-in role its subject holds, and beside a grant of a custom role read // before them: counted with the custom roles the grants name, which it reads from every one, the diff --git a/authorize.go b/authorize.go index c557962..b039a74 100644 --- a/authorize.go +++ b/authorize.go @@ -748,7 +748,8 @@ func (a *Authorizer) holders(ctx context.Context, p Permission, r Resource, role // One time for every statement, as one snapshot, so a grant, a membership or a group's member // ending between them can't leave a holder that no single moment had. now := a.cfg.Now() - through, err := a.broadArgs(ctx, a.db, r.Tenant, p, r.Type, roles, custom, broadAt, r.path()) + through, err := a.broadArgs(ctx, a.db, r.Tenant, p, r.Type, roles, custom, a.broadAt, r.path(), + broadKeys(r.Tenant, a.counting(p, r.Type)), "", time.Time{}) if err != nil { return nil, err } @@ -1085,12 +1086,14 @@ func (a *Authorizer) reachable(ctx context.Context, s Subject, p Permission, ten if err != nil { return Reach{}, err } - principals := countedOf(keys, a.counting(p, resourceType)) + counting := a.counting(p, resourceType) + principals := countedOf(keys, counting) if len(principals) == 0 { return Reach{}, nil } reaching := a.reaching(resourceType) - through, err := a.broadArgs(ctx, a.db, tenant, p, resourceType, roles, custom, broadOn, reaching) + everyone := slices.DeleteFunc(broadKeys(tenant, counting), func(k string) bool { return !slices.Contains(principals, k) }) + through, err := a.broadArgs(ctx, a.db, tenant, p, resourceType, roles, custom, a.broadOn, reaching, everyone, "", time.Time{}) if err != nil { return Reach{}, err } @@ -1216,14 +1219,16 @@ type Predicate struct { // tenant the subject may do p to. It reads the grants in the same statement as the rows, so // sorting and pagination work and nothing changes between deciding and reading. A condition holds // for the query it was built for: it fixes the time (Config.Now), the subject's label holding, what -// the constraints answered, and which custom roles granted to the public or the tenant's members -// give through them, when Filter was called, so a later query keeps rows Authorize then refuses, as -// once a grant it counted has expired, and leaves out what a custom role first granted to them -// since gives. Build one for each query. The host's query must reach this package's tables and call -// its function rbac_tenant_keys (they are in the pool's schema), and column names are trusted host -// code; every value is an argument. It counts only unconditional grants, and leaves out every -// resource a deny covers, conditional ones included, so it never keeps one Authorize refuses: -// Reachable's Conditional and ConditionalExcept name the scopes where that may be too strict. +// the constraints answered, and which custom roles granted to the public or, while the subject is +// one of them, the tenant's members give through them, when Filter was called, so a later query +// keeps rows Authorize then refuses, as once a grant it counted has expired, and leaves out what a +// custom role first granted to them since gives, or one granted to the members gives a subject +// that has since become one. Build one for each query. The host's query must reach this package's +// tables and call its function rbac_tenant_keys (they are in the pool's schema), and column names +// are trusted host code; every value is an argument. It counts only unconditional grants, and +// leaves out every resource a deny covers, conditional ones included, so it never keeps one +// Authorize refuses: Reachable's Conditional and ConditionalExcept name the scopes where that may +// be too strict. func (a *Authorizer) Filter(ctx context.Context, s Subject, p Permission, tenant, resourceType string, cols Columns, firstArg int) (Predicate, error) { a = a.still() if cols.Tenant == "" || cols.Path == "" || firstArg < 1 { @@ -1294,8 +1299,12 @@ func (a *Authorizer) Filter(ctx context.Context, s Subject, p Permission, tenant now := a.cfg.Now() // Which custom roles give p through the public and the tenant's members is read now, and the // statement keeps of them those that still give it, and through the public those still public: - // one first granted to them later gives nothing through them. - through, err := a.broadArgs(ctx, a.db, tenant, p, resourceType, roles, custom, broadOn, a.reaching(resourceType)) + // one first granted to them later gives nothing through them. Through the members, only while + // the subject is one: the anonymous never is. + counted := counts + counted.members = counted.members && s.Principal != (Principal{}) + through, err := a.broadArgs(ctx, a.db, tenant, p, resourceType, roles, custom, a.broadOn, a.reaching(resourceType), + broadKeys(tenant, counted), s.identityKey(), now) if err != nil { return Predicate{}, err } @@ -1356,15 +1365,29 @@ func byType(types, cols, where string) string { SELECT %[2]s FROM rbac_grants g WHERE %[3]s AND %[4]s OFFSET 0) g`, types, cols, where, ofType("g.scope", "t.t")) } -// broadOn is SQL selecting, for broadArgs, the roles of the grants in the tenant $1 to the principal -// whose key is principal on scopes of the types in the array $4. -func broadOn(principal string) string { - return byType("$4", "g.role", "g.tenant = $1 AND g.principal = "+principal) +// broadOn is SQL selecting, for broadArgs, the roles of the grants of custom roles in the tenant $1 +// to the principal whose key is principal on scopes of the types in the array $4, a type at a time. +// Each is a range of rbac_grants_custom_principal, which holds no grant of a built-in role and leads +// to the principal, the tenant and the scope, so no plan reads them cheaper by the custom roles' +// index on the tenant alone. As of a time it selects the grants of every role, the prefix tested +// apart (namedCustom): history's index of a grant's role leads to the tenant and the role, so a plan +// for the prefix could read every version of every grant of a custom role in the tenant. +func (a *Authorizer) broadOn(principal string) string { + return byType("$4", "g.role", "g.tenant = $1 AND g.principal = "+principal+a.customOnly("g.role")) } -// broadAt is broadOn for the scopes in the array $4. -func broadAt(principal string) string { - return byKey("$4", "rbac_grants", "scope", "role", "tenant = $1 AND principal = "+principal) +// broadAt is broadOn for the scopes in the array $4, a scope at a time. +func (a *Authorizer) broadAt(principal string) string { + return byKey("$4", "rbac_grants", "scope", "role", "tenant = $1 AND principal = "+principal+a.customOnly("role")) +} + +// customOnly is SQL keeping, but as of a time, the grants whose role, col, is a custom one, for +// broadOn and broadAt. +func (a *Authorizer) customOnly(col string) string { + if a.history != "" { + return "" + } + return " AND " + col + " LIKE '" + CustomPrefix + "%'" } // byKey is SQL selecting cols of the rows of table for which where holds whose col is one of the diff --git a/docs/architecture.md b/docs/architecture.md index 2a6bb26..97f86e6 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -208,7 +208,11 @@ erDiagram grants is an index-only scan. The indexes by scope and by principal lead to both too and hold the same, so a connection's cached statement, once PostgreSQL plans it generically by the tables' averages, still reads only the subject's grants on the path, from the index alone, whichever - index it picks. + index it picks. A search decides which custom roles the public's and the tenant's members' grants + name before its statement, from `rbac_grants_custom_principal`, which holds only grants of custom + roles, by principal and scope, and reads only those its subject counts: the anonymous and a guest + read none of the members', and a search of a permission no public role gives none of the + public's, so their grants of built-in roles on thousands of resources cost it nothing. - **A read of a list of keys reads each key's rows on its own** (`byKey`), as of a list's scopes or principals, of the denies on a check's path, of `Activate`'s eligibilities and of the rows `DeleteScopes`, `DeletePrincipal` and `DeleteGroups` remove, and so does a walk through nested diff --git a/migrations/0019_custom_grant_principals.sql b/migrations/0019_custom_grant_principals.sql new file mode 100644 index 0000000..bef9f72 --- /dev/null +++ b/migrations/0019_custom_grant_principals.sql @@ -0,0 +1,10 @@ +-- Filter, Reachable and Holders read which custom roles the public's and the tenant's members' +-- grants on the scopes they search name. rbac_grants_principal finds those grants with every other +-- grant of theirs there, of built-in roles too, which the tenant's members can hold on thousands of +-- resources; and rbac_grants_custom_role leads to the tenant alone, which a generic plan costs as an +-- average tenant's few grants, so it read every grant of a custom role in the tenant. This one +-- holds only grants of custom roles and leads to a principal, a tenant and a scope, so a search +-- seeks just those of the principal on a scope or a range of a type's, from the index alone, and no +-- plan finds them cheaper through the tenant. Building it keeps writes to rbac_grants waiting until +-- the migration commits, though reads go on. +CREATE INDEX rbac_grants_custom_principal ON rbac_grants (principal, tenant, scope) INCLUDE (role) WHERE role LIKE 'custom.%'; diff --git a/roles.go b/roles.go index 9fe132c..2d1061d 100644 --- a/roles.go +++ b/roles.go @@ -205,29 +205,38 @@ func givingSQL(builtin, tenant, perm, resourceType string, custom bool, ids stri // through grants to the public and the tenant's members: the custom ones that are public, NULL // where no grant to the public gives p, as Filter and Reachable then read none and Holders drops // the public; of roles, the built-in ones that aren't privileged; and the custom ones that aren't. -// The custom ones are those giving p there that grants to the public or the tenant's members name -// on the scopes, or scopes of the types, on: only for those grants does it matter. grants is SQL -// selecting the roles of the grants in the tenant $1 to the principal whose key is the SQL it is -// given there, on is the array $4 (broadOn, broadAt). They are decided as Authorizer.current reads -// them, as checks read them; pairing each one's permissions and types with the catalog's in SQL -// took a search seconds. +// The custom ones are those giving p there that the grants on the scopes, or scopes of the types, +// on name of the broad principals in keys, those whose grants the search counts. Only for those +// grants does it matter, and either principal's can be on thousands of resources, though the +// anonymous and a guest count none of the members', and a search of a permission no public role +// gives none of the public's. With identity, the tenant's members in keys are read only while the subject of that key +// is one of them at now, as keysSQL counts them, for a search that reads its subject's keys in its +// own statement (Filter). grants is SQL selecting the roles of the grants in the tenant $1 to the +// principal whose key is the SQL it is given there, on is the array $4 (Authorizer.broadOn, +// Authorizer.broadAt). They are decided as Authorizer.current reads them, as checks read them; +// pairing each one's permissions and types with the catalog's in SQL took a search seconds. func (a *Authorizer) broadArgs(ctx context.Context, q DB, tenant string, p Permission, resourceType string, roles []string, custom bool, - grants func(principal string) string, on []string, + grants func(principal string) string, on, keys []string, identity string, now time.Time, ) ([]any, error) { unprivileged := slices.DeleteFunc(slices.Clone(roles), func(id string) bool { return a.roles[Role(id)].Privileged }) // A grant to either never gives a permission to change access, so no search counts one for it. - if !custom || p.managesAccess() { + if !custom || p.managesAccess() || len(keys) == 0 { return []any{nil, unprivileged, nil}, nil } - // A broad key and a scope or a type at a time, the prefix tested apart (namedCustom), and the - // roles they name by key: matched to both keys at once, and tested for the prefix there, a - // generic plan read the grants by the custom roles' index on the tenant alone, every grant of one - // in the tenant for each scope or type; and as of a time, roles tested against the ids rather - // than read by them are each read at its newest version. - rows, err := q.Query(ctx, `SELECT `+roleColumns+` FROM (`+byKey(namedCustom(`SELECT g.role FROM unnest($5::text[]) AS b(k) + in, args := "$5::text[]", []any{tenant, string(p), resourceType, on, keys} + if identity != "" { + in = `ARRAY(SELECT k FROM unnest($5::text[]) AS c(k) WHERE k NOT LIKE 'members:%' OR EXISTS (SELECT FROM rbac_memberships + WHERE tenant = $1 AND principal = $6 AND (expires_at IS NULL OR expires_at > $7) AND NOT guest AND NOT suspended))` + args = append(args, identity, now) + } + // A broad key and a scope or a type at a time, and the roles they name by key: matched to both + // keys at once, a generic plan read the grants by the custom roles' index on the tenant alone, + // every grant of one in the tenant for each scope or type; and as of a time, roles tested + // against the ids rather than read by them are each read at its newest version. + rows, err := q.Query(ctx, `SELECT `+roleColumns+` FROM (`+byKey(namedCustom(`SELECT g.role FROM unnest(`+in+`) AS b(k) CROSS JOIN LATERAL (`+grants("b.k")+`) g`), "rbac_roles", "id", roleColumns, "tenant = $1 AND $2::text = ANY(permissions) AND $3::text = ANY(resource_types)")+`) r - ORDER BY id`, tenant, string(p), resourceType, on, broadKeys(tenant)) + ORDER BY id`, args...) if err != nil { return nil, err } @@ -250,8 +259,11 @@ func (a *Authorizer) broadArgs(ctx context.Context, q DB, tenant string, p Permi return []any{public, unprivileged, unprivilegedCustom}, nil } -// broadKeys are the keys of a tenant's broad principals: the public and its members. -func broadKeys(tenant string) []string { return []string{publicKey, TenantMembers(tenant).key()} } +// broadKeys are the keys of a tenant's broad principals, the public and its members, whose grants a +// read counts. +func broadKeys(tenant string, b broad) []string { + return countedOf([]string{publicKey, TenantMembers(tenant).key()}, b) +} // namedCustom is SQL for an array of the custom roles, each once, among those grants selects, SQL // for a column role. It tests the prefix outside grants, behind OFFSET 0 or a materialized CTE From 5cae5be80a5cd5b794b2a69c7bd7e634f67d44d1 Mon Sep 17 00:00:00 2001 From: Michael McQuade Date: Wed, 21 Oct 2026 14:40:00 +0000 Subject: [PATCH 2/8] fix: Filter, its condition and Reachable read the row of every grant their subject holds on the listed types, as they tell the public's and the members' grants by a kind no index holds --- authority_test.go | 120 +++++++++++++++++++++++++++++++++++----------- authorize.go | 14 +++--- rbac.go | 15 +++--- 3 files changed, 108 insertions(+), 41 deletions(-) diff --git a/authority_test.go b/authority_test.go index 53ea043..b8418a3 100644 --- a/authority_test.go +++ b/authority_test.go @@ -3342,48 +3342,112 @@ func TestChecksReadTheirGrantsFromAnIndexAlone(t *testing.T) { SELECT 't1', 'tenant', 't1', 'user', id, 'cluster.login' FROM others`, "rbac_grants") } } - // So the visibility map lets a scan of an index skip the rows. VACUUM marks none visible to - // all while an older snapshot elsewhere in the database, such as another run's, is open. - for deadline := time.Now().Add(30 * time.Second); ; time.Sleep(time.Second) { - if _, err := h.pool.Exec(ctx, `VACUUM rbac_grants`); err != nil { - t.Fatal(err) - } - var visible bool - if err := h.pool.QueryRow(ctx, `SELECT relallvisible >= relpages FROM pg_class WHERE oid = 'rbac_grants'::regclass`).Scan(&visible); err != nil { - t.Fatal(err) - } - if visible { - break - } - if time.Now().After(deadline) { - t.Skip("an older snapshot elsewhere in the database keeps rbac_grants' rows from being visible to all") - } - } + grantsVisibleToAll(t, h) tx, err := h.pool.Begin(ctx) if err != nil { t.Fatal(err) } defer func() { _ = tx.Rollback(ctx) }() - // The pages of the table itself, not its indexes: its rows and its visibility map. - rows := func() int64 { - t.Helper() - var n int64 - if err := tx.QueryRow(ctx, `SELECT pg_stat_get_xact_blocks_fetched('rbac_grants'::regclass)`).Scan(&n); err != nil { - t.Fatal(err) - } - return n - } - start := rows() + start := grantRowsRead(ctx, t, tx) if err := h.authz.WithTx(tx).Authorize(ctx, ann, login, cluster); err != nil { t.Fatal(err) } - if read := rows() - start; read > int64(held/4) { + if read := grantRowsRead(ctx, t, tx) - start; read > int64(held/4) { t.Errorf("a check of a subject holding %d grants read %d pages of the table's rows", held, read) } }) } } +// grantsVisibleToAll vacuums rbac_grants until its visibility map marks every row visible to all, so +// a scan of an index can skip the rows. VACUUM marks none so while an older snapshot elsewhere in +// the database, such as another run's, is open: then it skips the test. +func grantsVisibleToAll(t *testing.T, h *harness) { + t.Helper() + for deadline := time.Now().Add(30 * time.Second); ; time.Sleep(time.Second) { + if _, err := h.pool.Exec(t.Context(), `VACUUM rbac_grants`); err != nil { + t.Fatal(err) + } + var visible bool + if err := h.pool.QueryRow(t.Context(), `SELECT relallvisible >= relpages FROM pg_class WHERE oid = 'rbac_grants'::regclass`).Scan(&visible); err != nil { + t.Fatal(err) + } + if visible { + return + } + if time.Now().After(deadline) { + t.Skip("an older snapshot elsewhere in the database keeps rbac_grants' rows from being visible to all") + } + } +} + +// grantRowsRead is how many pages of rbac_grants itself, not its indexes, tx has read: its rows and +// its visibility map. +func grantRowsRead(ctx context.Context, t *testing.T, tx pgx.Tx) int64 { + t.Helper() + var n int64 + if err := tx.QueryRow(ctx, `SELECT pg_stat_get_xact_blocks_fetched('rbac_grants'::regclass)`).Scan(&n); err != nil { + t.Fatal(err) + } + return n +} + +// Filter, its condition in the host's query, and Reachable read their subject's grants from an index +// alone, under a generic plan too, as a check does: they told the broad principals' grants by their +// kind, which no index of the grants holds, so they read the row of each grant of the subject's on +// the listed types, as one on each of thousands of clusters it created, which grants made at +// different times have on pages of their own. +func TestListingsReadTheirGrantsFromAnIndexAlone(t *testing.T) { + for _, plan := range []string{"force_custom_plan", "force_generic_plan"} { + t.Run(plan, func(t *testing.T) { + h := crowded(t, pgtest.Open(t, "plan_cache_mode="+plan)) + ctx := t.Context() + hostTable(t, h) + u := h.as(t, "u") + // u's grants, each made beside others' grants, as grants made over time are. + const held = 1000 + manyMembers(t, h, held, `INSERT INTO rbac_grants (tenant, scope_type, scope_id, principal_kind, principal_id, role) + SELECT 't1', 'cluster', id, 'user', id, 'cluster.login' FROM others + UNION ALL SELECT 't1', 'cluster', id, 'user', 'u', 'cluster.login' FROM others`, "rbac_grants") + grantsVisibleToAll(t, h) + tx, err := h.pool.Begin(ctx) + if err != nil { + t.Fatal(err) + } + defer func() { _ = tx.Rollback(ctx) }() + a := h.authz.WithTx(tx) + for _, c := range []struct { + what string + f func() error + }{ + {"Filter and its condition", func() error { + pred, err := a.Filter(ctx, u, login, "t1", "cluster", rbac.Columns{Tenant: "tenant", Path: "path"}, 1) + if err != nil { + return err + } + var n int + return tx.QueryRow(ctx, `SELECT count(*) FROM clusters WHERE `+pred.SQL, pred.Args...).Scan(&n) + }}, + {"Reachable", func() error { + reach, err := a.Reachable(ctx, u, login, "t1", "cluster") + if err == nil && len(reach.Scopes) != held { + err = fmt.Errorf("Reachable reached %d scopes", len(reach.Scopes)) + } + return err + }}, + } { + start := grantRowsRead(ctx, t, tx) + if err := c.f(); err != nil { + t.Fatalf("%s: %v", c.what, err) + } + if read := grantRowsRead(ctx, t, tx) - start; read > held/100 { + t.Errorf("%s of a subject holding %d grants read %d pages of the table's rows", c.what, held, read) + } + } + }) + } +} + // Filter, its condition in the host's query, and Reachable read, of the grants, only the subject's // principals' on scopes of the types a resource's path holds, a type at a time, under a generic plan // too: of a range of each type's scopes, a plan can seek only one, and reading every grant of the diff --git a/authorize.go b/authorize.go index b039a74..75ff5f0 100644 --- a/authorize.go +++ b/authorize.go @@ -1097,19 +1097,19 @@ func (a *Authorizer) reachable(ctx context.Context, s Subject, p Permission, ten if err != nil { return Reach{}, err } - args := append(append([]any{ + args := append([]any{ tenant, principals, roles, a.cfg.Now(), reaching, publicKey, a.publicRoles(roles), string(p), resourceType, - }, through...), broadKinds) + }, through...) // The subject's grants on scopes of the types are read once, and of the custom roles those that // they name (namedCustom). rows, err := a.db.Query(ctx, ` - WITH held AS MATERIALIZED (`+byType("$5", "g.scope, g.conditional, g.role, g.principal, g.principal_kind", ` + WITH held AS MATERIALIZED (`+byType("$5", "g.scope, g.conditional, g.role, g.principal", ` g.tenant = $1 AND g.principal = ANY($2::text[]) AND (g.expires_at IS NULL OR g.expires_at > $4)`)+`) SELECT scope, bool_and(conditional) FROM held WHERE role = ANY(`+givingSQL("$3", "$1", "$8", "$9", custom, namedCustom("SELECT role FROM held"), false)+`) AND (principal <> $6 OR role = ANY(`+givingSQL("$7", "$1", "$8", "$9", custom, "$10", true)+`)) - AND (principal_kind <> ALL($13::text[]) OR role = ANY(`+givingSQL("$11", "$1", "$8", "$9", custom, "$12", false)+`)) + AND (`+narrowKey("principal")+` OR role = ANY(`+givingSQL("$11", "$1", "$8", "$9", custom, "$12", false)+`)) GROUP BY scope ORDER BY scope`, args...) if err != nil { return Reach{}, err @@ -1489,7 +1489,7 @@ func filterGrants(arg func(int) string, custom bool, types string) string { // filterHeld is SQL selecting the subject's grants filterGrants reads, as held. func filterHeld(arg func(int) string, types string) string { - return byType(types, "g.scope, g.role, g.principal, g.principal_kind", fmt.Sprintf(`g.tenant = %s::text AND g.principal = ANY(%s) + return byType(types, "g.scope, g.role, g.principal", fmt.Sprintf(`g.tenant = %s::text AND g.principal = ANY(%s) AND (g.expires_at IS NULL OR g.expires_at > %s) AND NOT g.conditional`, arg(1), filterKeys(arg, countedKeys(arg(13), arg(17))), arg(4))) } @@ -1497,10 +1497,10 @@ func filterHeld(arg func(int) string, types string) string { func filterCounted(arg func(int) string, custom bool) string { return fmt.Sprintf(`role = ANY(%s) AND (principal <> %s::text OR role = ANY(%s)) - AND (principal_kind <> ALL(%s) OR role = ANY(%s))`, + AND (%s OR role = ANY(%s))`, givingSQL(arg(3), arg(1), arg(8), arg(9), custom, namedCustom("SELECT role FROM held"), false), arg(6), givingSQL(arg(7), arg(1), arg(8), arg(9), custom, arg(10), true), - broadKindsSQL, givingSQL(arg(11), arg(1), arg(8), arg(9), custom, arg(12), false)) + narrowKey("principal"), givingSQL(arg(11), arg(1), arg(8), arg(9), custom, arg(12), false)) } // grantCounts counts, with Filter's arguments args, the grants its condition counts (filterGrants) diff --git a/rbac.go b/rbac.go index b0d8df9..ade0737 100644 --- a/rbac.go +++ b/rbac.go @@ -112,12 +112,15 @@ var publicKey = Principal{Kind: Public}.key() // public. func (p Principal) broad() bool { return p.Kind == Members || p.Kind == Public } -// broadKinds are the kinds of the broad principals, as a statement's argument, and broadKindsSQL -// as SQL. -var ( - broadKinds = []string{string(Members), string(Public)} - broadKindsSQL = fmt.Sprintf(`ARRAY['%s', '%s']::text[]`, Members, Public) -) +// broadKinds are the kinds of the broad principals, as a statement's argument. +var broadKinds = []string{string(Members), string(Public)} + +// narrowKey is SQL for whether the principal key col is neither the public's nor a tenant's +// members', as its kind would say: the indexes leading to a principal hold its key, not its kind, so +// a read testing the kind fetches each grant's row. +func narrowKey(col string) string { + return fmt.Sprintf(`(%[1]s <> '%[2]s' AND %[1]s NOT LIKE '%[3]s:%%')`, col, publicKey, Members) +} // wellFormed reports whether p could hold grants in a tenant, whether or not its kind is // declared, as a revoke needs. From 5abc8d8442de1d3d41d8230f34ccd81674390ad2 Mon Sep 17 00:00:00 2001 From: Michael McQuade Date: Wed, 21 Oct 2026 15:08:13 +0000 Subject: [PATCH 3/8] fix: as of a time, Filter and Reachable read every version of the members' grants on the listed types for the anonymous and guests, and each version of a group's grants on other types once for each type, and checks read every version of their subject's grants --- README.md | 12 +- asof_test.go | 180 ++++++++++++++++++++ authorize.go | 32 ++-- docs/architecture.md | 4 +- migrations/0019_custom_grant_principals.sql | 10 -- migrations/0019_principal_seeks.sql | 30 ++++ 6 files changed, 241 insertions(+), 27 deletions(-) delete mode 100644 migrations/0019_custom_grant_principals.sql create mode 100644 migrations/0019_principal_seeks.sql diff --git a/README.md b/README.md index 3e1c499..eda05a4 100644 --- a/README.md +++ b/README.md @@ -378,8 +378,9 @@ change it waited for, but at REPEATABLE READ or SERIALIZABLE its transaction's s and it is dated before that change. Write around the package at READ COMMITTED. `AsOf` makes no change: one tried is no actor's attempt, and `Config.OnRefused` isn't told of it. A statement reads the history of the rows it names, as a check does its subject's grants, groups and the -denies on its resource's path, a search the custom roles its grants name, and `Holders` the -members of the groups it lists, not the whole tenant's: `AsOf` plans every statement in its +denies on its resource's path, a search its subject's grants, and the public's and the members' it +counts, on each type it lists, and the custom roles they name, and `Holders` the members of the +groups it lists, not the whole tenant's: `AsOf` plans every statement in its transaction for its arguments, the host's too, since a connection's cached plan of a statement it has run often counts any tenant as an average one. A list counts the denies on scopes of its types by history's versions of them, at most a few, and `Grantable`, which can offer any custom role, @@ -460,7 +461,7 @@ pg_catalog's, so none stands in for them, and refuse a search_path that names pg A migration that rebuilds an index, as `0009_grant_seeks` rebuilds two of `rbac_grants`, locks the table until it commits, so checks, lists and changes wait for the rebuild; one that builds an index, as `0011_tenant_keys` builds three on `rbac_eligibilities` and `rbac_denies` and - `0019_custom_grant_principals` one on `rbac_grants`, keeps writes to the table waiting until it + `0019_principal_seeks` one on `rbac_grants`, keeps writes to the table waiting until it commits, though reads go on: on a large table, migrate when that wait is acceptable. Code from before `0012_activation_through` must not run beside code after it: an activation it extends keeps the eligibility recorded before, and can outlast the one it was extended through. Stop the replicas running it before migrating. `0013_bounded_rows` @@ -481,7 +482,10 @@ pg_catalog's, so none stands in for them, and refuse a search_path that names pg table locks each of its indexes, the rebuild waits for the reads of history running, as an export, and every read of history then waits until migrating commits. A transaction that read history and then reads one of those tables deadlocks with the rebuild, and PostgreSQL ends one - of the two: migrate again. + of the two: migrate again. `0019_principal_seeks` rebuilds history's index of a principal's + versions, which reads all of history, and analyzes it: reads of history, and every change, as each + writes history, wait until it commits, and a transaction that read history and then writes a + grant deadlocks with it, as above. - **Transactions:** a change runs in its own transaction at READ COMMITTED, or under `WithTx` in the host's, which must be READ COMMITTED too. A change refused for a race, such as a deadlock between host transactions, matches `rbac.ErrConflict`: try it again. Changes to the same rows diff --git a/asof_test.go b/asof_test.go index 7f6cd14..c9cd4d8 100644 --- a/asof_test.go +++ b/asof_test.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "io/fs" + "maps" "slices" "strings" "testing" @@ -549,6 +550,74 @@ func TestAsOfReadsARowChangedInPlaceAsItsLastVersion(t *testing.T) { } } +// As of a time, a check, Permissions and Grantable seek their subject's grants on the resource's +// path, however their statements are planned, beside thousands of its grants elsewhere, as one on +// each cluster it created: history's index of a principal's versions held no scope, so in a history +// of many tenants a plan read every version of the subject's grants to find the few on the path. +func TestAsOfChecksSeekTheirSubjectsGrantsOnThePath(t *testing.T) { + h := crowded(t, pgtest.Open(t, "")) + ctx := t.Context() + u := h.as(t, "u") + h.seed(t, grant(cluster.Scope(), "cluster.login", rbac.User, "u")) + granted := 0 + more := func(n int) { + t.Helper() + if _, err := h.pool.Exec(ctx, `INSERT INTO rbac_grants (tenant, scope_type, scope_id, principal_kind, principal_id, role) + SELECT 't1', 'cluster', 'k' || i, 'user', 'u', 'cluster.login' FROM generate_series($1::int + 1, $2::int) i`, granted, n); err != nil { + t.Fatal(err) + } + granted = n + if _, err := h.pool.Exec(ctx, `ANALYZE rbac_grants, rbac_history`); err != nil { + t.Fatal(err) + } + } + read := func(at time.Time, plans string) map[string]int64 { + t.Helper() + tx, err := h.pool.Begin(ctx) + if err != nil { + t.Fatal(err) + } + defer func() { _ = tx.Rollback(ctx) }() + pages := map[string]int64{} + if err := h.authz.WithTx(tx).AsOf(ctx, "t1", at, func(a *rbac.Authorizer) error { + if _, err := tx.Exec(ctx, `SELECT set_config('plan_cache_mode', $1, true)`, plans); err != nil { + return err + } + for what, f := range map[string]func() error{ + "Authorize": func() error { return a.Authorize(ctx, u, login, cluster) }, + "Permissions": func() error { return second(a.Permissions(ctx, u, cluster)) }, + "Grantable": func() error { return second(a.Grantable(ctx, u, cluster)) }, + } { + start := historyPages(ctx, t, tx) + if err := f(); err != nil { + return fmt.Errorf("%s: %w", what, err) + } + pages[what+" planned "+plans] = historyPages(ctx, t, tx) - start + } + return nil + }); err != nil { + t.Fatal(err) + } + return pages + } + plans := []string{"force_custom_plan", "force_generic_plan"} + more(500) + few := map[string]int64{} + at := dbNow(t, h) + for _, p := range plans { + maps.Copy(few, read(at, p)) + } + more(5000) + at = dbNow(t, h) + for _, p := range plans { + for what, many := range read(at, p) { + if many > few[what]*3/2+deeper { + t.Errorf("as of a time, %s read %d pages of history beside 500 grants of its subject's on other clusters, and %d beside 5000", what, few[what], many) + } + } + } +} + // A check as of a time reads the history of its subject's grants, memberships and groups, not // every row's, and of a conditional grant it holds, for its condition: other principals' history // costs it nothing. @@ -1207,6 +1276,117 @@ func TestMigratingLetsHistorysRoleCallWhatItsIndexCalls(t *testing.T) { } } +// As of a time, Filter, its condition, Reachable and Holders seek their subject's grants, and the +// public's and the members' they count, on each listed type, in a history of one tenant or of many, +// however their statements are planned: history's index of a principal's versions held no scope, so +// a plan read every version of a principal's grants once for each type, a group's on thousands of +// volumes for a list of clusters; and by history's index of scopes, every version on a type's +// scopes in the tenant, the members' grants on thousands of clusters for the anonymous or a guest, +// who count none of them. +func TestAsOfSearchesSeekTheirPrincipalsGrantsOnEachType(t *testing.T) { + volumes := rbac.RoleDefinition{ID: "volume.admin", Permissions: []rbac.Permission{"volume:view"}, ResourceTypes: []string{"volume"}} + withVolumes := func(c *rbac.Config) { c.Catalog = append(slices.Clone(c.Catalog), volumes) } + for _, tenants := range []string{"one tenant", "many tenants"} { + for _, plans := range []string{"", "force_generic_plan"} { + t.Run(tenants+"/"+cmp.Or(plans, "as AsOf plans"), func(t *testing.T) { + h := setup(t, withVolumes) + if tenants == "many tenants" { + h = crowded(t, pgtest.Open(t, ""), withVolumes) + } + ctx := t.Context() + c0 := hostTable(t, h)[0] + g := h.guest(t, "g", "builders") + h.seed(t, grant(c0.Scope(), "cluster.login", rbac.User, "g")) + granted := 0 + more := func(n int) { + t.Helper() + if _, err := h.pool.Exec(ctx, `INSERT INTO rbac_grants (tenant, scope_type, scope_id, principal_kind, principal_id, role) + SELECT 't1', 'cluster', 'k' || i, 'members', 't1', 'cluster.login' FROM generate_series($1::int + 1, $2::int) i + UNION ALL SELECT 't1', 'volume', 'v' || i, 'group', 'builders', 'volume.admin' FROM generate_series($1::int + 1, $2::int) i`, + granted, n); err != nil { + t.Fatal(err) + } + granted = n + if _, err := h.pool.Exec(ctx, `ANALYZE rbac_grants, rbac_history`); err != nil { + t.Fatal(err) + } + } + read := func(at time.Time) map[string]int64 { + t.Helper() + pages := map[string]int64{} + searches := map[string]func(pgx.Tx, *rbac.Authorizer) error{ + "Holders": func(_ pgx.Tx, a *rbac.Authorizer) error { + hs, err := a.Holders(ctx, view, c0) + if err == nil && !slices.Equal(holderPrincipals(hs), []rbac.Principal{g.Principal}) { + err = fmt.Errorf("Holders = %+v", hs) + } + return err + }, + } + for who, c := range map[string]struct { + s rbac.Subject + kept []string + }{"anonymous": {rbac.Subject{}, nil}, "guest": {g, []string{c0.ID}}} { + searches[who+" Filter"] = func(tx pgx.Tx, a *rbac.Authorizer) error { + pred, err := a.Filter(ctx, c.s, view, "t1", "cluster", rbac.Columns{Tenant: "tenant", Path: "path"}, 1) + if err != nil { + return err + } + rows, err := tx.Query(ctx, `SELECT id FROM clusters WHERE `+pred.SQL+` ORDER BY id`, pred.Args...) + if err != nil { + return err + } + ids, err := pgx.CollectRows(rows, pgx.RowTo[string]) + if err == nil && !slices.Equal(ids, c.kept) { + err = fmt.Errorf("Filter kept %v", ids) + } + return err + } + searches[who+" Reachable"] = func(_ pgx.Tx, a *rbac.Authorizer) error { + reach, err := a.Reachable(ctx, c.s, view, "t1", "cluster") + if err == nil && len(reach.Scopes) != len(c.kept) { + err = fmt.Errorf("Reachable = %+v", reach) + } + return err + } + } + for what, f := range searches { + func() { + tx, err := h.pool.Begin(ctx) + if err != nil { + t.Fatal(err) + } + defer func() { _ = tx.Rollback(ctx) }() + if err := h.authz.WithTx(tx).AsOf(ctx, "t1", at, func(a *rbac.Authorizer) error { + if plans != "" { + if _, err := tx.Exec(ctx, `SELECT set_config('plan_cache_mode', $1, true)`, plans); err != nil { + return err + } + } + start := historyPages(ctx, t, tx) + err := f(tx, a) + pages[what] = historyPages(ctx, t, tx) - start + return err + }); err != nil { + t.Fatalf("%s: %v", what, err) + } + }() + } + return pages + } + more(500) + few := read(dbNow(t, h)) + more(5000) + for what, many := range read(dbNow(t, h)) { + if many > few[what]*3/2+deeper { + t.Errorf("as of a time, %s read %d pages of history beside 500 grants of the members' on clusters and of a group of the guest's on volumes, and %d beside 5000", what, few[what], many) + } + } + }) + } + } +} + // As of a time, a list of the grants or the eligibilities on a scope reads their history, not every // grant's and eligibility's in the tenant. func TestAsOfListsAScopesGrantsAndEligibilitiesAlone(t *testing.T) { diff --git a/authorize.go b/authorize.go index 75ff5f0..a9cb888 100644 --- a/authorize.go +++ b/authorize.go @@ -1104,7 +1104,7 @@ func (a *Authorizer) reachable(ctx context.Context, s Subject, p Permission, ten // The subject's grants on scopes of the types are read once, and of the custom roles those that // they name (namedCustom). rows, err := a.db.Query(ctx, ` - WITH held AS MATERIALIZED (`+byType("$5", "g.scope, g.conditional, g.role, g.principal", ` + WITH held AS MATERIALIZED (`+a.byType("$5", "g.scope, g.conditional, g.role, g.principal", ` g.tenant = $1 AND g.principal = ANY($2::text[]) AND (g.expires_at IS NULL OR g.expires_at > $4)`)+`) SELECT scope, bool_and(conditional) FROM held WHERE role = ANY(`+givingSQL("$3", "$1", "$8", "$9", custom, namedCustom("SELECT role FROM held"), false)+`) @@ -1254,7 +1254,7 @@ func (a *Authorizer) Filter(ctx context.Context, s Subject, p Permission, tenant perm, wildcard = listed, covering } keys := func(of string) string { return filterKeys(arg, of) } - grants := func(types string) string { return filterGrants(arg, custom, types) } + grants := func(types string) string { return a.filterGrants(arg, custom, types) } // The denies on scopes of the types in the array types that bind the subject (bindingDenies). narrow := counted("FALSE", "FALSE") denies := func(types string) string { @@ -1359,10 +1359,18 @@ func (a *Authorizer) Filter(ctx context.Context, s Subject, p Permission, tenant // to a principal seek for each of its principals. Ranges ORed are none an index can seek, so a plan // reads every grant of the principals in the tenant, as a group's on thousands of resources of // another type. OFFSET 0 keeps the planner from flattening it into a join it could run the other -// way round. -func byType(types, cols, where string) string { +// way round. As of a time a scope's type is tested as its text up to its colon, an equality +// history's index of a principal's versions seeks: a range whose bounds are another table's, as the +// types' here, a plan counts as a few rows, so in a history of a few principals, where it counts a +// principal's versions as a large share, it read every version on a type's scopes in the tenant by +// history's index of scopes, the members' too, to find the public's few. +func (a *Authorizer) byType(types, cols, where string) string { + of := ofType("g.scope", "t.t") + if a.history != "" { + of = "split_part(g.scope, ':', 1) = t.t" + } return fmt.Sprintf(`SELECT %[2]s FROM unnest(%[1]s::text[]) AS t(t) CROSS JOIN LATERAL ( - SELECT %[2]s FROM rbac_grants g WHERE %[3]s AND %[4]s OFFSET 0) g`, types, cols, where, ofType("g.scope", "t.t")) + SELECT %[2]s FROM rbac_grants g WHERE %[3]s AND %[4]s OFFSET 0) g`, types, cols, where, of) } // broadOn is SQL selecting, for broadArgs, the roles of the grants of custom roles in the tenant $1 @@ -1373,7 +1381,7 @@ func byType(types, cols, where string) string { // apart (namedCustom): history's index of a grant's role leads to the tenant and the role, so a plan // for the prefix could read every version of every grant of a custom role in the tenant. func (a *Authorizer) broadOn(principal string) string { - return byType("$4", "g.role", "g.tenant = $1 AND g.principal = "+principal+a.customOnly("g.role")) + return a.byType("$4", "g.role", "g.tenant = $1 AND g.principal = "+principal+a.customOnly("g.role")) } // broadAt is broadOn for the scopes in the array $4, a scope at a time. @@ -1482,14 +1490,14 @@ func filterKeys(arg func(int) string, of string) string { // that give the permission, the public's only of public ones and the broad principals' of none // privileged. It reads the subject's grants there once (filterHeld), and of the custom roles those // that they name (namedCustom). arg numbers Filter's arguments. -func filterGrants(arg func(int) string, custom bool, types string) string { - return `WITH held AS MATERIALIZED (` + filterHeld(arg, types) + `) +func (a *Authorizer) filterGrants(arg func(int) string, custom bool, types string) string { + return `WITH held AS MATERIALIZED (` + a.filterHeld(arg, types) + `) SELECT scope COLLATE "default" FROM held WHERE ` + filterCounted(arg, custom) } // filterHeld is SQL selecting the subject's grants filterGrants reads, as held. -func filterHeld(arg func(int) string, types string) string { - return byType(types, "g.scope, g.role, g.principal", fmt.Sprintf(`g.tenant = %s::text AND g.principal = ANY(%s) +func (a *Authorizer) filterHeld(arg func(int) string, types string) string { + return a.byType(types, "g.scope, g.role, g.principal", fmt.Sprintf(`g.tenant = %s::text AND g.principal = ANY(%s) AND (g.expires_at IS NULL OR g.expires_at > %s) AND NOT g.conditional`, arg(1), filterKeys(arg, countedKeys(arg(13), arg(17))), arg(4))) } @@ -1526,12 +1534,12 @@ func (a *Authorizer) grantCounts(ctx context.Context, custom, container bool, ar // form. Both counts read one materialized held, so the subject's grants are read once. count := func(types string, limit int) string { if !custom { - return fmt.Sprintf(`(SELECT count(*) FROM (%s LIMIT %d) g)`, filterGrants(arg, false, types), limit) + return fmt.Sprintf(`(SELECT count(*) FROM (%s LIMIT %d) g)`, a.filterGrants(arg, false, types), limit) } return fmt.Sprintf(`(WITH held AS MATERIALIZED (%[1]s) SELECT CASE WHEN (SELECT count(*) FROM (SELECT FROM held WHERE %[2]s LIMIT %[4]d) g) = %[4]d THEN %[4]d ELSE (SELECT count(*) FROM (SELECT FROM held WHERE %[3]s LIMIT %[4]d) g) END)`, - filterHeld(arg, types), filterCounted(arg, false), filterCounted(arg, true), limit) + a.filterHeld(arg, types), filterCounted(arg, false), filterCounted(arg, true), limit) } onResources := "0" if !container { diff --git a/docs/architecture.md b/docs/architecture.md index 97f86e6..f1ada45 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -651,7 +651,9 @@ flowchart LR - **AsOf** stands temporary views of history in for the tables, in a transaction it rolls back, so every check, list and search answers as it would have at a past time, with the catalog and the label dimensions as they are now. Each statement reads history by the rows it names, through - history's indexes, as a search the custom roles its grants name, by their ids. The view of + history's indexes, as a search the custom roles its grants name, by their ids, and its principals' + grants by principal and type, which history's index of a principal's versions holds beside the + scope, as a check seeks them on its resource's path. The view of custom roles takes each one's newest version, testing none for a later one, so `Grantable`, which can offer any, reads each version once. - **PruneHistory** deletes only what an export has read and later versions supersede. It keeps diff --git a/migrations/0019_custom_grant_principals.sql b/migrations/0019_custom_grant_principals.sql deleted file mode 100644 index bef9f72..0000000 --- a/migrations/0019_custom_grant_principals.sql +++ /dev/null @@ -1,10 +0,0 @@ --- Filter, Reachable and Holders read which custom roles the public's and the tenant's members' --- grants on the scopes they search name. rbac_grants_principal finds those grants with every other --- grant of theirs there, of built-in roles too, which the tenant's members can hold on thousands of --- resources; and rbac_grants_custom_role leads to the tenant alone, which a generic plan costs as an --- average tenant's few grants, so it read every grant of a custom role in the tenant. This one --- holds only grants of custom roles and leads to a principal, a tenant and a scope, so a search --- seeks just those of the principal on a scope or a range of a type's, from the index alone, and no --- plan finds them cheaper through the tenant. Building it keeps writes to rbac_grants waiting until --- the migration commits, though reads go on. -CREATE INDEX rbac_grants_custom_principal ON rbac_grants (principal, tenant, scope) INCLUDE (role) WHERE role LIKE 'custom.%'; diff --git a/migrations/0019_principal_seeks.sql b/migrations/0019_principal_seeks.sql new file mode 100644 index 0000000..ebacb3c --- /dev/null +++ b/migrations/0019_principal_seeks.sql @@ -0,0 +1,30 @@ +-- Filter, Reachable and Holders read which custom roles the public's and the tenant's members' +-- grants on the scopes they search name. rbac_grants_principal finds those grants with every other +-- grant of theirs there, of built-in roles too, which the tenant's members can hold on thousands of +-- resources; and rbac_grants_custom_role leads to the tenant alone, which a generic plan costs as an +-- average tenant's few grants, so it read every grant of a custom role in the tenant. This one +-- holds only grants of custom roles and leads to a principal, a tenant and a scope, so a search +-- seeks just those of the principal on a scope or a range of a type's, from the index alone, and no +-- plan finds them cheaper through the tenant. Building it keeps writes to rbac_grants waiting until +-- the migration commits, though reads go on. +CREATE INDEX rbac_grants_custom_principal ON rbac_grants (principal, tenant, scope) INCLUDE (role) WHERE role LIKE 'custom.%'; + +-- As of a time, a search reads its subject's grants, and the public's and the tenant's members', on +-- scopes of the types it lists, and a check its subject's on its resource's path, but history's index +-- of a principal's versions held no scope: a plan read every version of a principal's grants, of +-- other types too, once for each type, or by history's index of scopes every version of every grant +-- on a type's scopes in the tenant, the members' too. This one leads to the principal, then to its +-- entity, its scope's type and its scope, so a plan seeks a principal's versions of a type or on a +-- scope, and it serves what the one it replaces did, which led to the same three columns. A search +-- names a type as a scope's text up to its colon, an equality: a range of a type's scopes whose +-- bounds are another table's, as a search's types are, a plan counts as a few rows, so where it +-- counts a principal's versions as many, as in a history of a few principals, it took the index of +-- scopes. It is dropped before its replacement is built, so the migration takes history's strongest +-- lock before any other, as 0009 does the grants': reads of history, and every change, each of which +-- writes history, wait until the migration commits, and a transaction that read history and then +-- writes a grant deadlocks with it. Building it reads all of history; the analysis after it gives a +-- plan the statistics of the types, which autovacuum would take long after migrating. +DROP INDEX rbac_history_principal; +CREATE INDEX rbac_history_principal ON rbac_history (tenant, principal, entity, (split_part(scope, ':', 1)), scope, effective_at DESC) + WHERE principal IS NOT NULL; +ANALYZE rbac_history; From c0b86eae8b06f55ab4cc1c97df8606853bf8c7c2 Mon Sep 17 00:00:00 2001 From: Michael McQuade Date: Wed, 21 Oct 2026 15:54:18 +0000 Subject: [PATCH 4/8] chore: the principal-seeks migration is numbered 0018, after 0017 --- migrations/{0019_principal_seeks.sql => 0018_principal_seeks.sql} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename migrations/{0019_principal_seeks.sql => 0018_principal_seeks.sql} (100%) diff --git a/migrations/0019_principal_seeks.sql b/migrations/0018_principal_seeks.sql similarity index 100% rename from migrations/0019_principal_seeks.sql rename to migrations/0018_principal_seeks.sql From 8364db6dede7d9608acc9d4f1756a7d9b5355e4a Mon Sep 17 00:00:00 2001 From: Michael McQuade Date: Wed, 21 Oct 2026 16:24:49 +0000 Subject: [PATCH 5/8] fix: migrating to 0018 deadlocks with a change that starts while it builds the grants' index and writes history before a grant, and the README names it 0019 The migration now locks rbac_grants and then rbac_history before it builds anything, as 0013 does, so a change that starts while it builds waits for it. The README says a transaction under way that changed a table, not only one that read history, deadlocks with it. --- README.md | 20 +++++----- asof_test.go | 62 ++++++++++++++++++++++++++++- migrations/0018_principal_seeks.sql | 17 +++++--- roles.go | 13 +++--- 4 files changed, 91 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index eda05a4..4563b04 100644 --- a/README.md +++ b/README.md @@ -461,12 +461,13 @@ pg_catalog's, so none stands in for them, and refuse a search_path that names pg A migration that rebuilds an index, as `0009_grant_seeks` rebuilds two of `rbac_grants`, locks the table until it commits, so checks, lists and changes wait for the rebuild; one that builds an index, as `0011_tenant_keys` builds three on `rbac_eligibilities` and `rbac_denies` and - `0019_principal_seeks` one on `rbac_grants`, keeps writes to the table waiting until it - commits, though reads go on: on a large table, migrate when that wait is acceptable. Code from before `0012_activation_through` must not run beside code - after it: an activation it extends keeps the eligibility recorded before, and can outlast the one - it was extended through. Stop the replicas running it before migrating. `0013_bounded_rows` - locks every table, history too, until it commits, and reads each once for each check it adds, - so checks, lists, changes and reads of history wait for it. `0015_history_seeks` builds two + `0018_principal_seeks` one on `rbac_grants`, keeps writes to the table waiting until it + commits, though reads go on: on a large table, migrate when that wait is acceptable. Code from + before `0012_activation_through` must not run beside code after it: an activation it extends + keeps the eligibility recorded before, and can outlast the one it was extended through. Stop the + replicas running it before migrating. `0013_bounded_rows` locks every table, history too, until + it commits, and reads each once for each check it adds, so checks, lists, changes and reads of + history wait for it. `0015_history_seeks` builds two indexes on `rbac_history`, which every change writes, and analyzes it, so changes wait until it commits. Building one calls `rbac_tenant_keys` and `rbac_text_array` as history's role, as recording each deny does from then on, so it grants that role `EXECUTE` on both where it lacks @@ -482,10 +483,11 @@ pg_catalog's, so none stands in for them, and refuse a search_path that names pg table locks each of its indexes, the rebuild waits for the reads of history running, as an export, and every read of history then waits until migrating commits. A transaction that read history and then reads one of those tables deadlocks with the rebuild, and PostgreSQL ends one - of the two: migrate again. `0019_principal_seeks` rebuilds history's index of a principal's + of the two: migrate again. `0018_principal_seeks` rebuilds history's index of a principal's versions, which reads all of history, and analyzes it: reads of history, and every change, as each - writes history, wait until it commits, and a transaction that read history and then writes a - grant deadlocks with it, as above. + writes history, wait until it commits, and a transaction under way as it starts that read + history, or changed a table, as deleting a scope's eligibilities and then its grants does, and + then writes a grant deadlocks with it, as above. - **Transactions:** a change runs in its own transaction at READ COMMITTED, or under `WithTx` in the host's, which must be READ COMMITTED too. A change refused for a race, such as a deadlock between host transactions, matches `rbac.ErrConflict`: try it again. Changes to the same rows diff --git a/asof_test.go b/asof_test.go index c9cd4d8..03e032a 100644 --- a/asof_test.go +++ b/asof_test.go @@ -1276,6 +1276,64 @@ func TestMigratingLetsHistorysRoleCallWhatItsIndexCalls(t *testing.T) { } } +// Migrating to 0018 locks history before it builds the grants' index, so a change that starts while +// it builds and writes history before a grant, as DeleteScope removes a scope's eligibilities and +// then its grants, waits for it to commit: locked once the index was built, history was the change's +// while the change waited for the grants, and PostgreSQL ended one of the two as a deadlock. +func TestMigratingThePrincipalSeeksWaitsForAChangeStartedAsItBuilds(t *testing.T) { + // Little memory to sort in, so building the index takes a while. + pool := pgtest.Open(t, "maintenance_work_mem=64kB") + ctx := t.Context() + migrateFiles(t, pool, "", "0018") + for _, q := range []string{ + `ALTER TABLE rbac_grants DISABLE TRIGGER USER`, + `INSERT INTO rbac_grants (tenant, scope_type, scope_id, principal_kind, principal_id, role) + SELECT 't1', 'cluster', 'k' || i, 'user', 'u' || i, 'custom.r' FROM generate_series(1, 100000) i`, + `ALTER TABLE rbac_grants ENABLE TRIGGER USER`, + `INSERT INTO rbac_grants (tenant, scope_type, scope_id, principal_kind, principal_id, role) + VALUES ('t1', 'cluster', 'c1', 'user', 'u', 'cluster.login')`, + `INSERT INTO rbac_eligibilities (tenant, scope_type, scope_id, principal_kind, principal_id, role, max_seconds) + VALUES ('t1', 'cluster', 'c1', 'user', 'u', 'cluster.sudo', 3600)`, + } { + if _, err := pool.Exec(ctx, q); err != nil { + t.Fatal(err) + } + } + authz, err := rbac.New(pool, rbac.Config{ + Catalog: catalog, + ContainerTypes: []string{"org", "workspace"}, + ValidateGrant: func(context.Context, rbac.DB, rbac.GrantInput) error { return nil }, + }) + if err != nil { + t.Fatal(err) + } + migrated := make(chan error, 1) + go func() { migrated <- applyMigrations(ctx, pool, pgx.ReadCommitted, "0018", "") }() + for deadline := time.Now().Add(10 * time.Second); ; time.Sleep(time.Millisecond) { + var building bool + if err := pool.QueryRow(ctx, `SELECT EXISTS (SELECT FROM pg_stat_progress_create_index WHERE relid = 'rbac_grants'::regclass)`).Scan(&building); err != nil { + t.Fatal(err) + } + if building { + break + } + select { + case err := <-migrated: + t.Fatalf("migrated before the grants' index was seen building: %v", err) + default: + } + if time.Now().After(deadline) { + t.Fatal("migrating never built the grants' index") + } + } + if err := authz.DeleteScope(ctx, host, "t1", cluster.Scope()); err != nil { + t.Errorf("deleting a scope while migrating builds the grants' index: %v", err) + } + if err := <-migrated; err != nil { + t.Errorf("migrating while a scope is deleted: %v", err) + } +} + // As of a time, Filter, its condition, Reachable and Holders seek their subject's grants, and the // public's and the members' they count, on each listed type, in a history of one tenant or of many, // however their statements are planned: history's index of a principal's versions held no scope, so @@ -1289,9 +1347,11 @@ func TestAsOfSearchesSeekTheirPrincipalsGrantsOnEachType(t *testing.T) { for _, tenants := range []string{"one tenant", "many tenants"} { for _, plans := range []string{"", "force_generic_plan"} { t.Run(tenants+"/"+cmp.Or(plans, "as AsOf plans"), func(t *testing.T) { - h := setup(t, withVolumes) + var h *harness if tenants == "many tenants" { h = crowded(t, pgtest.Open(t, ""), withVolumes) + } else { + h = setup(t, withVolumes) } ctx := t.Context() c0 := hostTable(t, h)[0] diff --git a/migrations/0018_principal_seeks.sql b/migrations/0018_principal_seeks.sql index ebacb3c..11885ef 100644 --- a/migrations/0018_principal_seeks.sql +++ b/migrations/0018_principal_seeks.sql @@ -1,3 +1,11 @@ +-- Both tables are locked first, history last, as 0013 locks them: locked as each statement reached +-- it, history would be locked only once the grants' index was built, and a change that wrote history +-- meanwhile, as each change to a table does, and then wrote a grant, as deleting a scope's +-- eligibilities and then its grants does, would wait for migrating while migrating waited for it. +-- Reads of rbac_grants go on. +LOCK TABLE rbac_grants IN SHARE MODE; +LOCK TABLE rbac_history IN ACCESS EXCLUSIVE MODE; + -- Filter, Reachable and Holders read which custom roles the public's and the tenant's members' -- grants on the scopes they search name. rbac_grants_principal finds those grants with every other -- grant of theirs there, of built-in roles too, which the tenant's members can hold on thousands of @@ -19,11 +27,10 @@ CREATE INDEX rbac_grants_custom_principal ON rbac_grants (principal, tenant, sco -- names a type as a scope's text up to its colon, an equality: a range of a type's scopes whose -- bounds are another table's, as a search's types are, a plan counts as a few rows, so where it -- counts a principal's versions as many, as in a history of a few principals, it took the index of --- scopes. It is dropped before its replacement is built, so the migration takes history's strongest --- lock before any other, as 0009 does the grants': reads of history, and every change, each of which --- writes history, wait until the migration commits, and a transaction that read history and then --- writes a grant deadlocks with it. Building it reads all of history; the analysis after it gives a --- plan the statistics of the types, which autovacuum would take long after migrating. +-- scopes. Reads of history, and every change, each of which writes history, wait until the +-- migration commits, and a transaction under way as it starts that read history, or changed a table, +-- and then writes a grant deadlocks with it. Building it reads all of history; the analysis after it +-- gives a plan the statistics of the types, which autovacuum would take long after migrating. DROP INDEX rbac_history_principal; CREATE INDEX rbac_history_principal ON rbac_history (tenant, principal, entity, (split_part(scope, ':', 1)), scope, effective_at DESC) WHERE principal IS NOT NULL; diff --git a/roles.go b/roles.go index 2d1061d..5195e91 100644 --- a/roles.go +++ b/roles.go @@ -209,12 +209,13 @@ func givingSQL(builtin, tenant, perm, resourceType string, custom bool, ids stri // on name of the broad principals in keys, those whose grants the search counts. Only for those // grants does it matter, and either principal's can be on thousands of resources, though the // anonymous and a guest count none of the members', and a search of a permission no public role -// gives none of the public's. With identity, the tenant's members in keys are read only while the subject of that key -// is one of them at now, as keysSQL counts them, for a search that reads its subject's keys in its -// own statement (Filter). grants is SQL selecting the roles of the grants in the tenant $1 to the -// principal whose key is the SQL it is given there, on is the array $4 (Authorizer.broadOn, -// Authorizer.broadAt). They are decided as Authorizer.current reads them, as checks read them; -// pairing each one's permissions and types with the catalog's in SQL took a search seconds. +// gives none of the public's. With identity, the tenant's members in keys are read only while the +// subject of that key is one of them at now, as keysSQL counts them, for a search that reads its +// subject's keys in its own statement (Filter). grants is SQL selecting the roles of the grants in +// the tenant $1 to the principal whose key is the SQL it is given there, on is the array $4 +// (Authorizer.broadOn, Authorizer.broadAt). They are decided as Authorizer.current reads them, as +// checks read them; pairing each one's permissions and types with the catalog's in SQL took a +// search seconds. func (a *Authorizer) broadArgs(ctx context.Context, q DB, tenant string, p Permission, resourceType string, roles []string, custom bool, grants func(principal string) string, on, keys []string, identity string, now time.Time, ) ([]any, error) { From 91087fe43f2feab52585efd7d9889d7580036c16 Mon Sep 17 00:00:00 2001 From: Michael McQuade Date: Wed, 21 Oct 2026 16:33:32 +0000 Subject: [PATCH 6/8] test: migrating to 0018 is tried once beside a change that starts while it builds the grants' index, as Migrate now gives way to a lock it waits for Since Migrate waits at most 100 ms for a lock, a change holding history while it waits for the grants made it give way once it had built the index, rather than deadlock. The migration's notes say so. --- asof_test.go | 27 ++++++++++++--------------- migrations/0018_principal_seeks.sql | 11 ++++++----- 2 files changed, 18 insertions(+), 20 deletions(-) diff --git a/asof_test.go b/asof_test.go index 6613843..7caf357 100644 --- a/asof_test.go +++ b/asof_test.go @@ -1351,12 +1351,16 @@ func TestMigratingLetsHistorysRoleCallWhatItsIndexCalls(t *testing.T) { // Migrating to 0018 locks history before it builds the grants' index, so a change that starts while // it builds and writes history before a grant, as DeleteScope removes a scope's eligibilities and // then its grants, waits for it to commit: locked once the index was built, history was the change's -// while the change waited for the grants, and PostgreSQL ended one of the two as a deadlock. +// while the change waited for the grants, so migrating gave way, having built the index, or +// PostgreSQL ended one of the two as a deadlock. func TestMigratingThePrincipalSeeksWaitsForAChangeStartedAsItBuilds(t *testing.T) { // Little memory to sort in, so building the index takes a while. pool := pgtest.Open(t, "maintenance_work_mem=64kB") ctx := t.Context() migrateFiles(t, pool, "", "0018") + recordMigrations(t, pool, "0018") + // So a Migrate that gave way would give up at once. + rbac.SetMigrateAttempts(t, 1, time.Millisecond) for _, q := range []string{ `ALTER TABLE rbac_grants DISABLE TRIGGER USER`, `INSERT INTO rbac_grants (tenant, scope_type, scope_id, principal_kind, principal_id, role) @@ -1371,16 +1375,9 @@ func TestMigratingThePrincipalSeeksWaitsForAChangeStartedAsItBuilds(t *testing.T t.Fatal(err) } } - authz, err := rbac.New(pool, rbac.Config{ - Catalog: catalog, - ContainerTypes: []string{"org", "workspace"}, - ValidateGrant: func(context.Context, rbac.DB, rbac.GrantInput) error { return nil }, - }) - if err != nil { - t.Fatal(err) - } - migrated := make(chan error, 1) - go func() { migrated <- applyMigrations(ctx, pool, pgx.ReadCommitted, "0018", "") }() + h := migrated(t, pool) + done := make(chan error, 1) + go func() { done <- rbac.Migrate(ctx, pool) }() for deadline := time.Now().Add(10 * time.Second); ; time.Sleep(time.Millisecond) { var building bool if err := pool.QueryRow(ctx, `SELECT EXISTS (SELECT FROM pg_stat_progress_create_index WHERE relid = 'rbac_grants'::regclass)`).Scan(&building); err != nil { @@ -1390,7 +1387,7 @@ func TestMigratingThePrincipalSeeksWaitsForAChangeStartedAsItBuilds(t *testing.T break } select { - case err := <-migrated: + case err := <-done: t.Fatalf("migrated before the grants' index was seen building: %v", err) default: } @@ -1398,11 +1395,11 @@ func TestMigratingThePrincipalSeeksWaitsForAChangeStartedAsItBuilds(t *testing.T t.Fatal("migrating never built the grants' index") } } - if err := authz.DeleteScope(ctx, host, "t1", cluster.Scope()); err != nil { + if err := h.authz.DeleteScope(ctx, host, "t1", cluster.Scope()); err != nil { t.Errorf("deleting a scope while migrating builds the grants' index: %v", err) } - if err := <-migrated; err != nil { - t.Errorf("migrating while a scope is deleted: %v", err) + if err := <-done; err != nil { + t.Errorf("Migrate while a scope is deleted: %v", err) } } diff --git a/migrations/0018_principal_seeks.sql b/migrations/0018_principal_seeks.sql index 11885ef..11f870b 100644 --- a/migrations/0018_principal_seeks.sql +++ b/migrations/0018_principal_seeks.sql @@ -1,8 +1,8 @@ -- Both tables are locked first, history last, as 0013 locks them: locked as each statement reached -- it, history would be locked only once the grants' index was built, and a change that wrote history -- meanwhile, as each change to a table does, and then wrote a grant, as deleting a scope's --- eligibilities and then its grants does, would wait for migrating while migrating waited for it. --- Reads of rbac_grants go on. +-- eligibilities and then its grants does, would wait for migrating while migrating waited for it, so +-- migrating would give way, or deadlock, having built the index. Reads of rbac_grants go on. LOCK TABLE rbac_grants IN SHARE MODE; LOCK TABLE rbac_history IN ACCESS EXCLUSIVE MODE; @@ -28,9 +28,10 @@ CREATE INDEX rbac_grants_custom_principal ON rbac_grants (principal, tenant, sco -- bounds are another table's, as a search's types are, a plan counts as a few rows, so where it -- counts a principal's versions as many, as in a history of a few principals, it took the index of -- scopes. Reads of history, and every change, each of which writes history, wait until the --- migration commits, and a transaction under way as it starts that read history, or changed a table, --- and then writes a grant deadlocks with it. Building it reads all of history; the analysis after it --- gives a plan the statistics of the types, which autovacuum would take long after migrating. +-- migration commits, and only a transaction under way as it starts that read history, or changed a +-- table, and then writes a grant makes it give way, before it builds either index. Building it reads +-- all of history; the analysis after it gives a plan the statistics of the types, which autovacuum +-- would take long after migrating. DROP INDEX rbac_history_principal; CREATE INDEX rbac_history_principal ON rbac_history (tenant, principal, entity, (split_part(scope, ':', 1)), scope, effective_at DESC) WHERE principal IS NOT NULL; From b97057220bb108ec21d1c3df305ac1c5be0fef7a Mon Sep 17 00:00:00 2001 From: Michael McQuade Date: Wed, 21 Oct 2026 16:40:53 +0000 Subject: [PATCH 7/8] docs: the architecture's note on AsOf wraps as the rest --- docs/architecture.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/architecture.md b/docs/architecture.md index b2116c9..1bda242 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -658,9 +658,9 @@ flowchart LR the label dimensions as they are now. Each statement reads history by the rows it names, through history's indexes, as a search the custom roles its grants name, by their ids, and its principals' grants by principal and type, which history's index of a principal's versions holds beside the - scope, as a check seeks them on its resource's path. The view of - custom roles takes each one's newest version, testing none for a later one, so `Grantable`, - which can offer any, reads each version once. + scope, as a check seeks them on its resource's path. The view of custom roles takes each one's + newest version, testing none for a later one, so `Grantable`, which can offer any, reads each + version once. - **PruneHistory** deletes only what an export has read and later versions supersede. It keeps each row's last version at or before its time, so AsOf answers as before for any later time, and refuses an earlier one; of a row whose last version by then ended it, it deletes every From de3d3f2ee25fb6689749c9bd631f78951dfa2c13 Mon Sep 17 00:00:00 2001 From: Michael McQuade Date: Wed, 21 Oct 2026 21:18:40 +0000 Subject: [PATCH 8/8] chore: the principal seeks migration follows the history record plans as 0019 --- README.md | 2 +- asof_test.go | 6 +++--- .../{0018_principal_seeks.sql => 0019_principal_seeks.sql} | 0 3 files changed, 4 insertions(+), 4 deletions(-) rename migrations/{0018_principal_seeks.sql => 0019_principal_seeks.sql} (100%) diff --git a/README.md b/README.md index 59145f5..7f012b4 100644 --- a/README.md +++ b/README.md @@ -515,7 +515,7 @@ pg_catalog's, so none stands in for them, and refuse a search_path that names pg migrating while it holds what the rebuild waits for: the rebuild gives way, unless the transaction began waiting long enough before it, as while migrating reads a long history's deny records, for its `deadlock_timeout` to pass first, and PostgreSQL ends the transaction. - `0018_principal_seeks` builds an index on `rbac_grants`, which keeps writes to it waiting until it + `0019_principal_seeks` builds an index on `rbac_grants`, which keeps writes to it waiting until it commits, though reads go on, and rebuilds history's index of a principal's versions, which reads all of history, and analyzes it. It locks `rbac_grants` and then history before it builds either, so reads of history, and every change, as each writes history, wait until it commits too, and only diff --git a/asof_test.go b/asof_test.go index 7caf357..79d9fe0 100644 --- a/asof_test.go +++ b/asof_test.go @@ -1348,7 +1348,7 @@ func TestMigratingLetsHistorysRoleCallWhatItsIndexCalls(t *testing.T) { } } -// Migrating to 0018 locks history before it builds the grants' index, so a change that starts while +// Migrating to 0019 locks history before it builds the grants' index, so a change that starts while // it builds and writes history before a grant, as DeleteScope removes a scope's eligibilities and // then its grants, waits for it to commit: locked once the index was built, history was the change's // while the change waited for the grants, so migrating gave way, having built the index, or @@ -1357,8 +1357,8 @@ func TestMigratingThePrincipalSeeksWaitsForAChangeStartedAsItBuilds(t *testing.T // Little memory to sort in, so building the index takes a while. pool := pgtest.Open(t, "maintenance_work_mem=64kB") ctx := t.Context() - migrateFiles(t, pool, "", "0018") - recordMigrations(t, pool, "0018") + migrateFiles(t, pool, "", "0019") + recordMigrations(t, pool, "0019") // So a Migrate that gave way would give up at once. rbac.SetMigrateAttempts(t, 1, time.Millisecond) for _, q := range []string{ diff --git a/migrations/0018_principal_seeks.sql b/migrations/0019_principal_seeks.sql similarity index 100% rename from migrations/0018_principal_seeks.sql rename to migrations/0019_principal_seeks.sql