Skip to content

Commit e5673da

Browse files
tianzhouclaude
tianzhou
and
claude
authored
fix: skip privileges for ignored objects in .pgschemaignore (#392) (#393)
* fix: skip privileges for ignored objects in .pgschemaignore (#392) When functions/tables/etc. were ignored via .pgschemaignore patterns, their GRANT/REVOKE statements still appeared in dump and plan output. Now privileges on ignored objects are automatically excluded by checking the object name against its type-specific ignore patterns. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address review feedback - signature stripping, TABLE/VIEW separation, view column privileges - Strip function/procedure argument lists before matching ignore patterns (fixes exact-name patterns like "dblink_connect_u" vs "dblink_connect_u(text)") - Separate TABLE and VIEW into distinct switch cases to prevent cross-contamination - Also filter column privileges on ignored views, not just tables - Fix cleanup comment in test Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 4d2a26c commit e5673da

3 files changed

Lines changed: 216 additions & 0 deletions

File tree

‎cmd/ignore_integration_test.go‎

Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1166,6 +1166,174 @@ func cleanupSharedEmbeddedPG(t *testing.T) {
11661166
}
11671167
}
11681168

1169+
// TestIgnorePrivilegesForIgnoredObjects tests that privileges on ignored objects
1170+
// (functions, tables, etc.) are also excluded from dump/plan output.
1171+
// Reproduces https://github.com/pgplex/pgschema/issues/392
1172+
func TestIgnorePrivilegesForIgnoredObjects(t *testing.T) {
1173+
if testing.Short() {
1174+
t.Skip("Skipping integration test in short mode")
1175+
}
1176+
1177+
embeddedPG := testutil.SetupPostgres(t)
1178+
defer embeddedPG.Stop()
1179+
conn, host, port, dbname, user, password := testutil.ConnectToPostgres(t, embeddedPG)
1180+
defer conn.Close()
1181+
1182+
containerInfo := &struct {
1183+
Conn *sql.DB
1184+
Host string
1185+
Port int
1186+
DBName string
1187+
User string
1188+
Password string
1189+
}{
1190+
Conn: conn,
1191+
Host: host,
1192+
Port: port,
1193+
DBName: dbname,
1194+
User: user,
1195+
Password: password,
1196+
}
1197+
1198+
// Create schema with functions and revoked PUBLIC privileges (simulates extension-installed functions)
1199+
setupSQL := `
1200+
CREATE TABLE users (
1201+
id SERIAL PRIMARY KEY,
1202+
name TEXT NOT NULL
1203+
);
1204+
1205+
-- Simulate extension-installed functions with revoked PUBLIC EXECUTE
1206+
CREATE OR REPLACE FUNCTION dblink_connect_u(text) RETURNS text AS $$
1207+
BEGIN RETURN 'stub'; END;
1208+
$$ LANGUAGE plpgsql;
1209+
1210+
CREATE OR REPLACE FUNCTION dblink_connect_u(text, text) RETURNS text AS $$
1211+
BEGIN RETURN 'stub'; END;
1212+
$$ LANGUAGE plpgsql;
1213+
1214+
-- Revoke default PUBLIC EXECUTE (simulates extension behavior)
1215+
REVOKE EXECUTE ON FUNCTION dblink_connect_u(text) FROM PUBLIC;
1216+
REVOKE EXECUTE ON FUNCTION dblink_connect_u(text, text) FROM PUBLIC;
1217+
1218+
-- Also test table privileges on ignored tables
1219+
CREATE TABLE temp_data (
1220+
id SERIAL PRIMARY KEY,
1221+
value TEXT
1222+
);
1223+
1224+
DO $$
1225+
BEGIN
1226+
IF NOT EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'app_user') THEN
1227+
CREATE ROLE app_user;
1228+
END IF;
1229+
END $$;
1230+
1231+
GRANT SELECT ON temp_data TO app_user;
1232+
GRANT SELECT ON users TO app_user;
1233+
`
1234+
_, err := conn.Exec(setupSQL)
1235+
if err != nil {
1236+
t.Fatalf("Failed to create test schema: %v", err)
1237+
}
1238+
1239+
originalWd, err := os.Getwd()
1240+
if err != nil {
1241+
t.Fatalf("Failed to get current working directory: %v", err)
1242+
}
1243+
defer func() {
1244+
if err := os.Chdir(originalWd); err != nil {
1245+
t.Fatalf("Failed to restore working directory: %v", err)
1246+
}
1247+
}()
1248+
1249+
tmpDir := t.TempDir()
1250+
if err := os.Chdir(tmpDir); err != nil {
1251+
t.Fatalf("Failed to change to temp directory: %v", err)
1252+
}
1253+
1254+
// Create .pgschemaignore that ignores dblink_* functions and temp_* tables
1255+
ignoreContent := `[functions]
1256+
patterns = ["dblink_*"]
1257+
1258+
[tables]
1259+
patterns = ["temp_*"]
1260+
`
1261+
err = os.WriteFile(".pgschemaignore", []byte(ignoreContent), 0644)
1262+
if err != nil {
1263+
t.Fatalf("Failed to create .pgschemaignore: %v", err)
1264+
}
1265+
1266+
t.Run("dump", func(t *testing.T) {
1267+
output := executeIgnoreDumpCommand(t, containerInfo)
1268+
1269+
// Users table and its privileges should be present
1270+
if !strings.Contains(output, "CREATE TABLE IF NOT EXISTS users") {
1271+
t.Error("Dump should include users table")
1272+
}
1273+
if !strings.Contains(output, "app_user") {
1274+
t.Error("Dump should include privileges for app_user on non-ignored tables")
1275+
}
1276+
1277+
// Ignored functions should not appear
1278+
if strings.Contains(output, "dblink_connect_u") {
1279+
t.Error("Dump should not include dblink_connect_u (function is ignored, so REVOKE statements should also be ignored)")
1280+
}
1281+
1282+
// Ignored table privileges should not appear
1283+
if strings.Contains(output, "temp_data") {
1284+
t.Error("Dump should not include temp_data or its privileges (table is ignored)")
1285+
}
1286+
})
1287+
1288+
t.Run("plan", func(t *testing.T) {
1289+
schemaSQL := `
1290+
CREATE TABLE users (
1291+
id SERIAL PRIMARY KEY,
1292+
name TEXT NOT NULL
1293+
);
1294+
1295+
DO $$
1296+
BEGIN
1297+
IF NOT EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'app_user') THEN
1298+
CREATE ROLE app_user;
1299+
END IF;
1300+
END $$;
1301+
1302+
GRANT SELECT ON users TO app_user;
1303+
`
1304+
schemaFile := "schema.sql"
1305+
err := os.WriteFile(schemaFile, []byte(schemaSQL), 0644)
1306+
if err != nil {
1307+
t.Fatalf("Failed to create schema file: %v", err)
1308+
}
1309+
defer os.Remove(schemaFile)
1310+
1311+
output := executeIgnorePlanCommand(t, containerInfo, schemaFile)
1312+
1313+
// Plan should not reference dblink functions or their privileges
1314+
if strings.Contains(output, "dblink_connect_u") {
1315+
t.Error("Plan should not include dblink_connect_u (function is ignored)")
1316+
}
1317+
1318+
// Plan should not reference ignored tables
1319+
if strings.Contains(output, "temp_data") {
1320+
t.Error("Plan should not include temp_data (table is ignored)")
1321+
}
1322+
})
1323+
1324+
// Clean up roles from sharedEmbeddedPG (plan subtest may create roles there)
1325+
cleanupStatements := []string{
1326+
"REASSIGN OWNED BY app_user TO testuser",
1327+
"DROP OWNED BY app_user",
1328+
"DROP ROLE IF EXISTS app_user",
1329+
}
1330+
sharedConn, _, _, _, _, _ := testutil.ConnectToPostgres(t, sharedEmbeddedPG)
1331+
defer sharedConn.Close()
1332+
for _, stmt := range cleanupStatements {
1333+
sharedConn.Exec(stmt)
1334+
}
1335+
}
1336+
11691337
// verifyPlanOutput checks that plan output excludes ignored objects
11701338
func verifyPlanOutput(t *testing.T, output string) {
11711339
// Changes that should appear in plan (regular objects)

‎ir/ignore.go‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,15 @@ import (
55
"strings"
66
)
77

8+
// stripFunctionArgs removes the argument list from a function/procedure signature.
9+
// e.g. "dblink_connect_u(text, text)" -> "dblink_connect_u"
10+
func stripFunctionArgs(name string) string {
11+
if idx := strings.Index(name, "("); idx != -1 {
12+
return name[:idx]
13+
}
14+
return name
15+
}
16+
817
// IgnoreConfig represents the configuration for ignoring database objects
918
type IgnoreConfig struct {
1019
Tables []string `toml:"tables,omitempty"`
@@ -65,6 +74,30 @@ func (c *IgnoreConfig) ShouldIgnoreSequence(sequenceName string) bool {
6574
return c.shouldIgnore(sequenceName, c.Sequences)
6675
}
6776

77+
// ShouldIgnorePrivilegeByObjectType checks if a privilege should be ignored based on the object name
78+
// and its type. When an object (function, table, etc.) is ignored via its section pattern,
79+
// privileges on that object should also be ignored.
80+
func (c *IgnoreConfig) ShouldIgnorePrivilegeByObjectType(objectName string, objectType string) bool {
81+
if c == nil {
82+
return false
83+
}
84+
switch objectType {
85+
case "TABLE":
86+
return c.shouldIgnore(objectName, c.Tables)
87+
case "VIEW":
88+
return c.shouldIgnore(objectName, c.Views)
89+
case "FUNCTION":
90+
return c.shouldIgnore(stripFunctionArgs(objectName), c.Functions)
91+
case "PROCEDURE":
92+
return c.shouldIgnore(stripFunctionArgs(objectName), c.Procedures)
93+
case "SEQUENCE":
94+
return c.shouldIgnore(objectName, c.Sequences)
95+
case "TYPE":
96+
return c.shouldIgnore(objectName, c.Types)
97+
}
98+
return false
99+
}
100+
68101
// ShouldIgnorePrivilege checks if a privilege should be ignored based on the grantee role name
69102
func (c *IgnoreConfig) ShouldIgnorePrivilege(grantee string) bool {
70103
if c == nil {

‎ir/inspector.go‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2121,6 +2121,11 @@ func (i *Inspector) buildPrivileges(ctx context.Context, schema *IR, targetSchem
21212121
continue
21222122
}
21232123

2124+
// Skip privileges for ignored objects (e.g., ignored functions should have their privileges ignored too)
2125+
if i.ignoreConfig != nil && i.ignoreConfig.ShouldIgnorePrivilegeByObjectType(objectName, objectType) {
2126+
continue
2127+
}
2128+
21242129
// Check for default PUBLIC grants that should be excluded
21252130
if grantee == "PUBLIC" {
21262131
if (objectType == "FUNCTION" || objectType == "PROCEDURE") && privilegeType == "EXECUTE" {
@@ -2216,6 +2221,11 @@ func (i *Inspector) buildRevokedDefaultPrivileges(ctx context.Context, targetSch
22162221
objectName := row.ObjectName.String
22172222
objectType := row.ObjectType.String
22182223

2224+
// Skip revoked default privileges for ignored objects
2225+
if i.ignoreConfig != nil && i.ignoreConfig.ShouldIgnorePrivilegeByObjectType(objectName, objectType) {
2226+
continue
2227+
}
2228+
22192229
var objType PrivilegeObjectType
22202230
var privs []string
22212231
switch objectType {
@@ -2470,6 +2480,11 @@ func (i *Inspector) buildColumnPrivileges(ctx context.Context, schema *IR, targe
24702480
continue
24712481
}
24722482

2483+
// Skip column privileges for ignored tables/views
2484+
if i.ignoreConfig != nil && (i.ignoreConfig.ShouldIgnoreTable(tableName) || i.ignoreConfig.ShouldIgnoreView(tableName)) {
2485+
continue
2486+
}
2487+
24732488
key := colPrivKey{
24742489
TableName: tableName,
24752490
Grantee: grantee,

0 commit comments

Comments
 (0)