From b788b3d6b05a38b6e1aaba197e2f581a2d3666ea Mon Sep 17 00:00:00 2001 From: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> Date: Wed, 14 Oct 2026 14:36:48 -0400 Subject: [PATCH] test(push-gateway): compare full column types, table options, and non-table objects in schema parity Read columns from pg_attribute (format_type with typmod, default expr, identity, generation, collation) instead of information_schema, which drops typmods and array element types. Compare relpersistence, RLS flags and reloptions per table, and require both probes to hold no functions, non-internal triggers, policies, views, sequences or user types. Hoist GATEWAY_TABLES so readiness, runtime grants and the parity test share one table list. Note in the manifest why the relay's post-pgschema reconcile step does not apply, and point the deployment doc at schema/. Signed-off-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> --- crates/buzz-push-gateway/schema/schema.sql | 3 + crates/buzz-push-gateway/src/postgres.rs | 31 ++++----- .../postgres/desired_schema_postgres_tests.rs | 69 +++++++++++++++---- docs/push-gateway-deployment.md | 2 +- 4 files changed, 75 insertions(+), 30 deletions(-) diff --git a/crates/buzz-push-gateway/schema/schema.sql b/crates/buzz-push-gateway/schema/schema.sql index 8e4b072811..7c71f8c50c 100644 --- a/crates/buzz-push-gateway/schema/schema.sql +++ b/crates/buzz-push-gateway/schema/schema.sql @@ -13,6 +13,9 @@ -- namespace, so nothing else may live under it. The gateway has no types, -- functions, triggers, or seed rows. Its pre-migration bootstrap guard and -- runtime-role grants are operational steps in `src/postgres.rs`, not schema. +-- It also has nothing pgschema cannot express (no seed DML, no storage +-- parameters), so the relay's `scripts/reconcile-schema-after-pgschema.sql` +-- step (root `AGENTS.md` gotcha #7) does not apply to it. -- -- `./bin/pgschema apply --file crates/buzz-push-gateway/schema/schema.sql` -- resolves the `\i` includes relative to this file. Raw `psql` resolves `\i` diff --git a/crates/buzz-push-gateway/src/postgres.rs b/crates/buzz-push-gateway/src/postgres.rs index dd842bf7fd..887dd77d07 100644 --- a/crates/buzz-push-gateway/src/postgres.rs +++ b/crates/buzz-push-gateway/src/postgres.rs @@ -16,6 +16,17 @@ mod desired_schema_postgres_tests; static GATEWAY_MIGRATOR: sqlx::migrate::Migrator = sqlx::migrate!("./migrations"); +/// Every gateway authority table. Readiness probes and runtime-role grants +/// cover exactly this set; the desired-state parity test pins it to `schema/`. +const GATEWAY_TABLES: [&str; 6] = [ + "push_gateway_challenges", + "push_gateway_installations", + "push_gateway_delegations", + "push_gateway_endpoint_quotas", + "push_gateway_delivery_auth_replays", + "push_gateway_delivery_request_replays", +]; + #[cfg(test)] async fn migrate_gateway_through( pool: &PgPool, @@ -62,14 +73,8 @@ impl PostgresAuthorityStore { REVOKE CREATE ON SCHEMA public FROM {role}; GRANT CONNECT ON DATABASE {database} TO {role}; GRANT USAGE ON SCHEMA public TO {role}; - GRANT SELECT, INSERT, UPDATE, DELETE ON TABLE - push_gateway_challenges, - push_gateway_installations, - push_gateway_delegations, - push_gateway_endpoint_quotas, - push_gateway_delivery_auth_replays, - push_gateway_delivery_request_replays - TO {role};" + GRANT SELECT, INSERT, UPDATE, DELETE ON TABLE {tables} TO {role};", + tables = GATEWAY_TABLES.join(", "), ); sqlx::raw_sql(AssertSqlSafe(grants)).execute(pool).await?; Ok(()) @@ -91,16 +96,8 @@ fn bytes32(v: Vec) -> Result<[u8; 32], AuthorityError> { #[async_trait] impl AuthorityStore for PostgresAuthorityStore { async fn ready(&self) -> Result<(), AuthorityError> { - const TABLES: [&str; 6] = [ - "push_gateway_challenges", - "push_gateway_installations", - "push_gateway_delegations", - "push_gateway_endpoint_quotas", - "push_gateway_delivery_auth_replays", - "push_gateway_delivery_request_replays", - ]; let mut tx = self.pool.begin().await.map_err(db)?; - for table in TABLES { + for table in GATEWAY_TABLES { let ready: bool = sqlx::query_scalar( "SELECT to_regclass($1) IS NOT NULL AND COALESCE(has_table_privilege(current_user, to_regclass($1), 'SELECT'), false) diff --git a/crates/buzz-push-gateway/src/postgres/desired_schema_postgres_tests.rs b/crates/buzz-push-gateway/src/postgres/desired_schema_postgres_tests.rs index b6d7d86c5d..0e6e7e0365 100644 --- a/crates/buzz-push-gateway/src/postgres/desired_schema_postgres_tests.rs +++ b/crates/buzz-push-gateway/src/postgres/desired_schema_postgres_tests.rs @@ -112,10 +112,12 @@ fn desired_state_schema_layout_is_complete_and_schemabot_shaped() { /// One probe database is bootstrapped from `schema/schema.sql` through the /// real `bin/pgschema apply` binary (the SchemaBot-equivalent desired-state /// path); another runs every gateway migration. Their `public` catalogs must -/// match: the same tables, column shapes (keyed by name, since migrations drop -/// and append columns), constraints, and index shapes including per-key -/// `indoption`. The whole catalog is compared, so a new migration that is not -/// mirrored in `schema/` fails here. +/// match: the same tables, column shapes (full type with typmod, default, +/// nullability, identity, generation, collation; keyed by name, since +/// migrations drop and append columns), table options, constraints, and index +/// shapes including per-key `indoption`. Neither side may hold functions, +/// triggers, policies, views, sequences, or user types. The whole catalog is +/// compared, so a new migration that is not mirrored in `schema/` fails here. #[tokio::test] #[ignore = "requires PostgreSQL"] async fn desired_state_schema_matches_migrations() { @@ -135,10 +137,23 @@ async fn desired_state_schema_matches_migrations() { } const TABLES: &str = "SELECT tablename::text FROM pg_tables \ WHERE schemaname = 'public' AND tablename <> '_sqlx_migrations' ORDER BY 1"; - const COLUMNS: &str = "SELECT table_name::text, column_name::text, data_type::text, \ - is_nullable::text, column_default::text, is_identity::text \ - FROM information_schema.columns \ - WHERE table_schema = 'public' AND table_name <> '_sqlx_migrations' ORDER BY 1, 2"; + // `pg_attribute` rather than `information_schema.columns`: the latter's + // `data_type` drops typmods and array element types (`varchar(64)` and + // `varchar(128)` both read `character varying`). + const COLUMNS: &str = "SELECT t.relname::text, a.attname::text, \ + format_type(a.atttypid, a.atttypmod), a.attnotnull::text, \ + pg_get_expr(d.adbin, d.adrelid), a.attidentity::text, a.attgenerated::text, \ + a.attcollation::regcollation::text \ + FROM pg_attribute a JOIN pg_class t ON t.oid = a.attrelid \ + JOIN pg_namespace n ON n.oid = t.relnamespace \ + LEFT JOIN pg_attrdef d ON d.adrelid = a.attrelid AND d.adnum = a.attnum \ + WHERE n.nspname = 'public' AND t.relkind = 'r' AND t.relname <> '_sqlx_migrations' \ + AND a.attnum > 0 AND NOT a.attisdropped ORDER BY 1, 2"; + const TABLE_OPTIONS: &str = "SELECT t.relname::text, t.relpersistence::text, \ + t.relrowsecurity::text, t.relforcerowsecurity::text, t.reloptions::text \ + FROM pg_class t JOIN pg_namespace n ON n.oid = t.relnamespace \ + WHERE n.nspname = 'public' AND t.relkind = 'r' AND t.relname <> '_sqlx_migrations' \ + ORDER BY 1"; const CONSTRAINTS: &str = "SELECT t.relname::text, c.conname::text, c.contype::text, \ pg_get_constraintdef(c.oid) \ FROM pg_constraint c JOIN pg_class t ON t.oid = c.conrelid \ @@ -150,6 +165,23 @@ async fn desired_state_schema_matches_migrations() { JOIN pg_class t ON t.oid = i.indrelid \ JOIN pg_namespace n ON n.oid = t.relnamespace \ WHERE n.nspname = 'public' AND t.relname <> '_sqlx_migrations' ORDER BY 1, 2"; + // `schema/` may only declare tables and indexes, so neither path may + // create anything else in `public`: SchemaBot would never reproduce it. + const NON_TABLE_OBJECTS: &str = "SELECT 'function', p.proname::text FROM pg_proc p \ + JOIN pg_namespace n ON n.oid = p.pronamespace WHERE n.nspname = 'public' \ + UNION ALL SELECT 'trigger', tg.tgname::text FROM pg_trigger tg \ + JOIN pg_class t ON t.oid = tg.tgrelid JOIN pg_namespace n ON n.oid = t.relnamespace \ + WHERE n.nspname = 'public' AND NOT tg.tgisinternal \ + UNION ALL SELECT 'policy', pol.polname::text FROM pg_policy pol \ + JOIN pg_class t ON t.oid = pol.polrelid JOIN pg_namespace n ON n.oid = t.relnamespace \ + WHERE n.nspname = 'public' \ + UNION ALL SELECT 'relation', c.relname::text FROM pg_class c \ + JOIN pg_namespace n ON n.oid = c.relnamespace \ + WHERE n.nspname = 'public' AND c.relkind NOT IN ('r', 'i') \ + UNION ALL SELECT 'type', ty.typname::text FROM pg_type ty \ + JOIN pg_namespace n ON n.oid = ty.typnamespace \ + WHERE n.nspname = 'public' AND ty.typtype IN ('d', 'e', 'r', 'm') \ + ORDER BY 1, 2"; let base_url = std::env::var("BUZZ_TEST_DATABASE_URL").expect("test database URL"); let url = url::Url::parse(&base_url).expect("parse test database URL"); @@ -229,15 +261,28 @@ async fn desired_state_schema_matches_migrations() { .await .expect("apply gateway migrations"); - let tables = catalog(&desired, TABLES).await; + let declared: BTreeSet = catalog(&desired, TABLES) + .await + .into_iter() + .filter_map(|row| row.into_iter().next().flatten()) + .collect(); + let served: BTreeSet = GATEWAY_TABLES.iter().map(|t| t.to_string()).collect(); assert_eq!( - tables.len(), - 6, - "desired state declares the six gateway tables" + declared, served, + "schema/ must declare exactly GATEWAY_TABLES, the set readiness probes and \ + runtime grants cover" ); + for pool in [&desired, &migrated] { + assert_eq!( + catalog(pool, NON_TABLE_OBJECTS).await, + Rows::new(), + "the gateway schema holds only tables and indexes" + ); + } for (what, sql) in [ ("table set", TABLES), ("column", COLUMNS), + ("table option", TABLE_OPTIONS), ("constraint", CONSTRAINTS), ("index", INDEXES), ] { diff --git a/docs/push-gateway-deployment.md b/docs/push-gateway-deployment.md index 331d49708f..fa5406bf43 100644 --- a/docs/push-gateway-deployment.md +++ b/docs/push-gateway-deployment.md @@ -87,7 +87,7 @@ that app identity; omitting it is not a push shutdown mechanism. The gateway's dedicated pool does not consume the relay-oriented `BUZZ_DB_LOCK_TIMEOUT_MS`, `BUZZ_DB_IDLE_TXN_TIMEOUT_MS`, or `BUZZ_DB_STATEMENT_TIMEOUT_MS` settings. Its session-timeout policy remains separate from the `buzz-db` writer policy and must be designed and rolled out independently. -All replicas must share one PostgreSQL database. Delivery authority, replay admission, and endpoint quota reservation are transactional there, so replica count does not multiply the abuse ceiling. The gateway owns a scoped migration history under `crates/buzz-push-gateway/migrations`; it creates only the six `push_gateway_*` authority tables plus SQLx's migration-history table and never runs relay migrations. +All replicas must share one PostgreSQL database. Delivery authority, replay admission, and endpoint quota reservation are transactional there, so replica count does not multiply the abuse ceiling. The gateway owns a scoped migration history under `crates/buzz-push-gateway/migrations`; it creates only the six `push_gateway_*` authority tables plus SQLx's migration-history table and never runs relay migrations. `crates/buzz-push-gateway/schema/` mirrors those migrations' end state as declarative desired-state DDL, one file per table; a parity test keeps the two identical. The Helm chart runs a single pre-install/pre-upgrade migration Job using `migration.existingSecret`; that secret contains a DDL-capable `DATABASE_URL`. The URL MUST name a dedicated gateway database, not the relay database: SQLx stores its `_sqlx_migrations` history in `public`, so sharing a database would collide with another application's migration history. `migration.runtimeDatabaseRole` names an existing LOGIN role (the default is `buzz_push_gateway_runtime`) used by runtime `DATABASE_URL`. After scoped migrations, the Job revokes database `CREATE` from that role and schema `CREATE` from both `PUBLIC` and the role, then grants only database `CONNECT`, schema `USAGE`, and `SELECT, INSERT, UPDATE, DELETE` on the six gateway tables. The migration role must own the database/schema objects or otherwise be allowed to issue those grants; it is never provided to runtime replicas. Readiness rejects an empty/partial schema, missing DML, or a runtime role that retains database/schema `CREATE`. Helm waits for the migration hook before updating replicas, so rolling deployments never race unconditional startup migration. Readiness must be removed from load-balancer service endpoints before terminating a pod.