fix: support schema privilege diffs#287
Draft
dilame wants to merge 2 commits into
Draft
Conversation
Schema-level grants were invisible to the schema model, so migrations could converge while a role was missing USAGE on a schema. Constraint: pg-schema-diff skips privilege DDL during plan validation because temp roles are absent. Rejected: Treat public schema default USAGE as implicit | would keep schema ACL introspection incomplete. Confidence: high Scope-risk: moderate Directive: Keep schema privilege validation clearing in sync with every SkipValidation privilege generator. Tested: PATH="/Applications/Postgres.app/Contents/Versions/16/bin:$PATH" go test ./... Not-tested: Cross-version PostgreSQL matrix beyond local PostgreSQL 16.12.
Schema owners receive implicit USAGE and CREATE privileges, so ACL rows granted to the current owner should not be diffed as ordinary schema grants. Track schema owner during fetch and filter owner USAGE/CREATE while comparing schema privileges. Constraint: PostgreSQL schema owners have implicit schema privileges, and pg-schema-diff does not generate schema ownership migrations. Rejected: Treat owner-granted USAGE/CREATE as regular ACL drift | this repeats no-op GRANT/REVOKE statements when the desired schema grants the current owner. Confidence: high Scope-risk: narrow Directive: Keep owner-aware privilege filtering local to schema privilege diffing until schema ownership migration is supported. Tested: targeted named-schema acceptance; failed acceptance groups with -parallel 1; internal/schema; pkg/diff; pkg/tempdb; all non-acceptance cmd/internal/pkg packages. Not-tested: Production migrate-plan against Cloud SQL; full go test ./... is blocked by untracked scratch test_diff.go in this worktree and parallel temp database statement-timeout noise in acceptance tests.
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.
What
Plan and apply schema-level privilege changes (
GRANT/REVOKEon schemas), includingWITH GRANT OPTIONchanges.This adds schema ACLs to the introspected
NamedSchemamodel and emits authz-update migration statements when schema privileges drift between source and target. It also tracks schema owner names so owner-grantedUSAGE/CREATEACL entries are treated as implicit owner privileges, not ordinary grant drift.Why
pg-schema-diffalready tracks table privileges, but schema privileges were invisible:NamedSchemaonly stored the schema name,GetSchemasonly readnspname, and schemaAlteremitted no SQL. That allowed a migration plan to converge to zero statements even when the target hadGRANT USAGE ON SCHEMA ... TO ...and the source did not.After the first schema-privilege fix, one production-shaped no-op remained: schemas owned by a role can expose ACL rows like
service=UC/service. PostgreSQL owners already have implicit schema privileges, but the diff could still compare the target's explicit owner grant from the temp database as ordinary drift and repeatedly emit a no-opGRANT USAGE ON SCHEMA ... TO service.How
internal/queries/queries.sql: addGetSchemaPrivilegesfrompg_namespace.nspacl; makeGetSchemasreturn schema owner names.internal/schema/schema.go: addSchemaPrivilege, attach sorted privileges toNamedSchema, and keep schema owner for implicit-owner classification.pkg/diff/sql_generator.go: diff schema privileges, recreate whenWITH GRANT OPTIONchanges, and filter current-ownerUSAGE/CREATEgrants during comparison.pkg/diff/schema_privilege_sql_generator.go: emitGRANT ... ON SCHEMA/REVOKE ... ON SCHEMAwithAUTHZ_UPDATEhazards.pkg/diff/plan_generator.go: clear schema privileges during validation for skipped privilege statements, matching table privilege handling.internal/migration_acceptance_tests/named_schema_cases_test.go: cover grant, revoke, grant-option drift, non-ownerCREATErevocation, and the owner-grant no-op case.Test
go test ./...was also attempted, but this worktree currently has an untracked scratchtest_diff.goat the repository root, and the parallel acceptance package hit local temp-database statement-timeout noise. The same failed acceptance groups passed when rerun with-parallel 1.