Skip to content

Commit 5d0025e

Browse files
bplunkett-stripebplunkett-stripe
authored
Table privilege support (#264)
1 parent 38e539d commit 5d0025e

File tree

12 files changed

+607
-7
lines changed

12 files changed

+607
-7
lines changed

‎README.md‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,6 @@ Unsupported: <= 13 are not supported. Use at your own risk.
209209

210210
# Unsupported migrations
211211
An abridged list of unsupported migrations:
212-
- Privileges (Planned)
213212
- Types (Only enums are currently supported)
214213
- Renaming. The diffing library relies on names to identify the old and new versions of a table, index, etc. If you rename
215214
an object, it will be treated as a drop and an add

‎cmd/pg-schema-diff/apply_cmd.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ func runPlan(ctx context.Context, cmd *cobra.Command, connConfig *pgx.ConnConfig
150150
return fmt.Errorf("setting lock timeout: %w", err)
151151
}
152152
if _, err := conn.ExecContext(ctx, stmt.ToSQL()); err != nil {
153-
return fmt.Errorf("executing migration statement. the database maybe be in a dirty state: %s: %w", stmt, err)
153+
return fmt.Errorf("executing migration statement. the database maybe be in a dirty state: %s: %w", stmt.DDL, err)
154154
}
155155
cmdPrintf(cmd, "Finished executing statement. Duration: %s\n", time.Since(start))
156156
}

‎internal/migration_acceptance_tests/privilege_cases_test.go‎

Lines changed: 230 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,230 @@
1+
package migration_acceptance_tests
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stripe/pg-schema-diff/pkg/diff"
7+
)
8+
9+
var privilegeAcceptanceTestCases = []acceptanceTestCase{
10+
{
11+
name: "no-op",
12+
roles: []string{
13+
"app_user",
14+
},
15+
oldSchemaDDL: []string{
16+
`
17+
CREATE TABLE foobar(id INT);
18+
GRANT SELECT ON foobar TO app_user;
19+
`,
20+
},
21+
newSchemaDDL: []string{
22+
`
23+
CREATE TABLE foobar(id INT);
24+
GRANT SELECT ON foobar TO app_user;
25+
`,
26+
},
27+
expectEmptyPlan: true,
28+
},
29+
{
30+
name: "Grant multiple privileges to role",
31+
roles: []string{"app_user"},
32+
oldSchemaDDL: []string{
33+
`CREATE TABLE foobar(id INT);`,
34+
},
35+
newSchemaDDL: []string{
36+
`
37+
CREATE TABLE foobar(id INT);
38+
GRANT SELECT, INSERT, UPDATE, DELETE ON foobar TO app_user;
39+
`,
40+
},
41+
expectedHazardTypes: []diff.MigrationHazardType{
42+
diff.MigrationHazardTypeAuthzUpdate,
43+
},
44+
},
45+
{
46+
name: "Revoke privilege from role",
47+
roles: []string{"app_user"},
48+
oldSchemaDDL: []string{
49+
`
50+
CREATE TABLE foobar(id INT);
51+
GRANT SELECT ON foobar TO app_user;
52+
`,
53+
},
54+
newSchemaDDL: []string{
55+
`CREATE TABLE foobar(id INT);`,
56+
},
57+
expectedHazardTypes: []diff.MigrationHazardType{
58+
diff.MigrationHazardTypeAuthzUpdate,
59+
},
60+
},
61+
{
62+
name: "Grant WITH GRANT OPTION",
63+
roles: []string{"app_user"},
64+
oldSchemaDDL: []string{
65+
`CREATE TABLE foobar(id INT);`,
66+
},
67+
newSchemaDDL: []string{
68+
`
69+
CREATE TABLE foobar(id INT);
70+
GRANT SELECT ON foobar TO app_user WITH GRANT OPTION;
71+
`,
72+
},
73+
expectedHazardTypes: []diff.MigrationHazardType{
74+
diff.MigrationHazardTypeAuthzUpdate,
75+
},
76+
},
77+
{
78+
name: "Change GRANT OPTION (recreates privilege)",
79+
roles: []string{"app_user"},
80+
oldSchemaDDL: []string{
81+
`
82+
CREATE TABLE foobar(id INT);
83+
GRANT SELECT ON foobar TO app_user;
84+
`,
85+
},
86+
newSchemaDDL: []string{
87+
`
88+
CREATE TABLE foobar(id INT);
89+
GRANT SELECT ON foobar TO app_user WITH GRANT OPTION;
90+
`,
91+
},
92+
expectedHazardTypes: []diff.MigrationHazardType{
93+
diff.MigrationHazardTypeAuthzUpdate,
94+
},
95+
},
96+
{
97+
name: "Remove GRANT OPTION (recreates privilege)",
98+
roles: []string{"app_user"},
99+
oldSchemaDDL: []string{
100+
`
101+
CREATE TABLE foobar(id INT);
102+
GRANT SELECT ON foobar TO app_user WITH GRANT OPTION;
103+
`,
104+
},
105+
newSchemaDDL: []string{
106+
`
107+
CREATE TABLE foobar(id INT);
108+
GRANT SELECT ON foobar TO app_user;
109+
`,
110+
},
111+
expectedHazardTypes: []diff.MigrationHazardType{
112+
diff.MigrationHazardTypeAuthzUpdate,
113+
},
114+
},
115+
{
116+
name: "Grant on new table (no hazards since table is new)",
117+
roles: []string{"app_user"},
118+
newSchemaDDL: []string{
119+
`
120+
CREATE TABLE foobar(id INT);
121+
GRANT SELECT ON foobar TO app_user;
122+
`,
123+
},
124+
// No hazards expected since table is brand new
125+
},
126+
{
127+
name: "Drop table with privileges (only DeletesData hazard)",
128+
roles: []string{"app_user"},
129+
oldSchemaDDL: []string{
130+
`
131+
CREATE TABLE foobar(id INT);
132+
GRANT SELECT ON foobar TO app_user;
133+
`,
134+
},
135+
expectedHazardTypes: []diff.MigrationHazardType{
136+
diff.MigrationHazardTypeDeletesData,
137+
},
138+
},
139+
{
140+
name: "Grant on non-public schema table",
141+
roles: []string{"app_user"},
142+
oldSchemaDDL: []string{
143+
`
144+
CREATE SCHEMA app_schema;
145+
CREATE TABLE app_schema.foobar(id INT);
146+
`,
147+
},
148+
newSchemaDDL: []string{
149+
`
150+
CREATE SCHEMA app_schema;
151+
CREATE TABLE app_schema.foobar(id INT);
152+
GRANT SELECT ON app_schema.foobar TO app_user;
153+
`,
154+
},
155+
expectedHazardTypes: []diff.MigrationHazardType{
156+
diff.MigrationHazardTypeAuthzUpdate,
157+
},
158+
},
159+
{
160+
name: "Grant on partitioned parent table",
161+
roles: []string{"app_user"},
162+
oldSchemaDDL: []string{
163+
`
164+
CREATE TABLE foobar(
165+
category TEXT
166+
) partition by list (category);
167+
CREATE TABLE foobar_1 PARTITION OF foobar FOR VALUES IN ('category_1');
168+
`,
169+
},
170+
newSchemaDDL: []string{
171+
`
172+
CREATE TABLE foobar(
173+
category TEXT
174+
) partition by list (category);
175+
CREATE TABLE foobar_1 PARTITION OF foobar FOR VALUES IN ('category_1');
176+
GRANT SELECT ON foobar TO app_user;
177+
`,
178+
},
179+
expectedHazardTypes: []diff.MigrationHazardType{
180+
diff.MigrationHazardTypeAuthzUpdate,
181+
},
182+
},
183+
{
184+
name: "Privilege on new partition (not implemented)",
185+
roles: []string{"app_user"},
186+
oldSchemaDDL: []string{
187+
`
188+
CREATE TABLE foobar(
189+
category TEXT
190+
) partition by list (category);
191+
`,
192+
},
193+
newSchemaDDL: []string{
194+
`
195+
CREATE TABLE foobar(
196+
category TEXT
197+
) partition by list (category);
198+
CREATE TABLE foobar_1 PARTITION OF foobar FOR VALUES IN ('category');
199+
GRANT SELECT ON foobar_1 TO app_user;
200+
`,
201+
},
202+
expectedPlanErrorIs: diff.ErrNotImplemented,
203+
},
204+
{
205+
name: "Add privilege on existing partition (not implemented)",
206+
roles: []string{"app_user"},
207+
oldSchemaDDL: []string{
208+
`
209+
CREATE TABLE foobar(
210+
category TEXT
211+
) partition by list (category);
212+
CREATE TABLE foobar_1 PARTITION OF foobar FOR VALUES IN ('category');
213+
`,
214+
},
215+
newSchemaDDL: []string{
216+
`
217+
CREATE TABLE foobar(
218+
category TEXT
219+
) partition by list (category);
220+
CREATE TABLE foobar_1 PARTITION OF foobar FOR VALUES IN ('category');
221+
GRANT SELECT ON foobar_1 TO app_user;
222+
`,
223+
},
224+
expectedPlanErrorIs: diff.ErrNotImplemented,
225+
},
226+
}
227+
228+
func TestPrivilegeCases(t *testing.T) {
229+
runTestCases(t, privilegeAcceptanceTestCases)
230+
}

‎internal/queries/queries.sql‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -624,3 +624,45 @@ WHERE
624624
AND depend.objid = c.oid
625625
AND depend.deptype = 'e'
626626
);
627+
628+
-- name: GetTablePrivileges :many
629+
WITH parsed_acl AS (
630+
SELECT
631+
c.oid AS table_oid,
632+
c.relname AS table_name,
633+
n.nspname AS table_schema_name,
634+
c.relowner AS owner_oid,
635+
(ACLEXPLODE(c.relacl)).grantee AS grantee_oid,
636+
(ACLEXPLODE(c.relacl)).privilege_type AS privilege_type,
637+
(ACLEXPLODE(c.relacl)).is_grantable AS is_grantable
638+
FROM pg_catalog.pg_class AS c
639+
INNER JOIN pg_catalog.pg_namespace AS n ON c.relnamespace = n.oid
640+
WHERE
641+
n.nspname NOT IN ('pg_catalog', 'information_schema')
642+
AND n.nspname !~ '^pg_toast'
643+
AND n.nspname !~ '^pg_temp'
644+
AND (c.relkind = 'r' OR c.relkind = 'p')
645+
AND c.relacl IS NOT null
646+
-- Exclude tables owned by extensions
647+
AND NOT EXISTS (
648+
SELECT depend.objid
649+
FROM pg_catalog.pg_depend AS depend
650+
WHERE
651+
depend.classid = 'pg_class'::REGCLASS
652+
AND depend.objid = c.oid
653+
AND depend.deptype = 'e'
654+
)
655+
)
656+
657+
SELECT
658+
pa.table_name::TEXT,
659+
pa.table_schema_name::TEXT,
660+
COALESCE(grantee_role.rolname, '')::TEXT AS grantee,
661+
pa.privilege_type::TEXT AS privilege,
662+
pa.is_grantable
663+
FROM parsed_acl AS pa
664+
LEFT JOIN pg_catalog.pg_roles AS grantee_role
665+
ON pa.grantee_oid = grantee_role.oid
666+
-- Exclude privileges granted to the table owner (these are implicit)
667+
WHERE pa.grantee_oid != pa.owner_oid OR pa.grantee_oid = 0
668+
ORDER BY pa.table_schema_name, pa.table_name, grantee, pa.privilege_type;

‎internal/queries/queries.sql.go‎

Lines changed: 80 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)