fix: support schema privilege diffs#287
Draft
dilame wants to merge 3 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.
Schema owner is now treated as part of named-schema state rather than only as a helper for suppressing implicit owner grants. New schemas are rebuilt with AUTHORIZATION so plan validation can faithfully reconstruct non-default owners, and existing schemas emit ALTER OWNER when target ownership differs. Constraint: declarative schema diffs must converge repeated migrate-plan runs without separate manual owner repair SQL. Rejected: documenting ownership drift as a manual repair step | leaves pg-schema-diff blind to a catalog fact it already reads. Confidence: high Scope-risk: narrow Directive: when changing owner-aware privilege filtering, filter old privileges against old owner and target privileges against target owner. Tested: go test ./internal/migration_acceptance_tests -run TestNamedSchemaTestCases -count=1; go test ./pkg/diff -count=1; go test ./internal/schema -count=1; go test ./internal/migration_acceptance_tests -run TestMaterializedViewTestCases/'Add materialized view with dependent column' -count=1 Not-tested: full acceptance package passed far enough to validate many packages but hit unrelated temp database statement timeouts in this local Postgres environment.
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 authorization drift:
GRANT/REVOKEon schemas), includingWITH GRANT OPTIONchanges;ALTER SCHEMA ... OWNER TO ...) when target owner differs;CREATE SCHEMA ... AUTHORIZATION ...for newly materialized schemas so validation can faithfully rebuild non-default owners.This adds schema ACLs and schema owners to the introspected
NamedSchemamodel. Owner-grantedUSAGE/CREATEACL entries are treated as implicit owner privileges rather than ordinary grant drift.Why
pg-schema-diffalready tracks table privileges, but schema authorization state was incomplete. Before this PR, a migration plan could converge to zero statements while the source and target still differed on schema grants.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, so those owner grants should not create repeated no-opGRANT USAGE ON SCHEMA ... TO servicenoise.A second production-shaped drift then surfaced: the tool read schema owner names only to suppress implicit owner grants, but did not generate
ALTER OWNER. This PR now treats schema ownership as real schema state, so ownership drift is caught by the plan instead of requiring manual repair SQL.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 ownership diffs plus implicit-owner filtering.pkg/diff/sql_generator.go: diff schema privileges, recreate whenWITH GRANT OPTIONchanges, and filter old privileges by old owner / target privileges by target owner.pkg/diff/schema_privilege_sql_generator.go: emitGRANT ... ON SCHEMA/REVOKE ... ON SCHEMAwithAUTHZ_UPDATEhazards.pkg/diff/named_schema_sql_generator.go: emit owner-awareCREATE SCHEMA ... AUTHORIZATION ...andALTER SCHEMA ... OWNER TO ...withAUTHZ_UPDATEhazard.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, owner-grant no-op, and explicit owner drift.Test
Also attempted broader runs:
The root
go test ./...is blocked in this local worktree by an untracked scratchtest_diff.goat repository root. The broader acceptance package hit local temp-database statement-timeout noise in unrelated tests; the named-schema suite and the isolated materialized-view case passed when rerun directly.