diff --git a/cmd/ignore_integration_test.go b/cmd/ignore_integration_test.go index c06956da..e85b5438 100644 --- a/cmd/ignore_integration_test.go +++ b/cmd/ignore_integration_test.go @@ -1166,6 +1166,174 @@ func cleanupSharedEmbeddedPG(t *testing.T) { } } +// TestIgnorePrivilegesForIgnoredObjects tests that privileges on ignored objects +// (functions, tables, etc.) are also excluded from dump/plan output. +// Reproduces https://github.com/pgplex/pgschema/issues/392 +func TestIgnorePrivilegesForIgnoredObjects(t *testing.T) { + if testing.Short() { + t.Skip("Skipping integration test in short mode") + } + + embeddedPG := testutil.SetupPostgres(t) + defer embeddedPG.Stop() + conn, host, port, dbname, user, password := testutil.ConnectToPostgres(t, embeddedPG) + defer conn.Close() + + containerInfo := &struct { + Conn *sql.DB + Host string + Port int + DBName string + User string + Password string + }{ + Conn: conn, + Host: host, + Port: port, + DBName: dbname, + User: user, + Password: password, + } + + // Create schema with functions and revoked PUBLIC privileges (simulates extension-installed functions) + setupSQL := ` +CREATE TABLE users ( + id SERIAL PRIMARY KEY, + name TEXT NOT NULL +); + +-- Simulate extension-installed functions with revoked PUBLIC EXECUTE +CREATE OR REPLACE FUNCTION dblink_connect_u(text) RETURNS text AS $$ +BEGIN RETURN 'stub'; END; +$$ LANGUAGE plpgsql; + +CREATE OR REPLACE FUNCTION dblink_connect_u(text, text) RETURNS text AS $$ +BEGIN RETURN 'stub'; END; +$$ LANGUAGE plpgsql; + +-- Revoke default PUBLIC EXECUTE (simulates extension behavior) +REVOKE EXECUTE ON FUNCTION dblink_connect_u(text) FROM PUBLIC; +REVOKE EXECUTE ON FUNCTION dblink_connect_u(text, text) FROM PUBLIC; + +-- Also test table privileges on ignored tables +CREATE TABLE temp_data ( + id SERIAL PRIMARY KEY, + value TEXT +); + +DO $$ +BEGIN + IF NOT EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'app_user') THEN + CREATE ROLE app_user; + END IF; +END $$; + +GRANT SELECT ON temp_data TO app_user; +GRANT SELECT ON users TO app_user; +` + _, err := conn.Exec(setupSQL) + if err != nil { + t.Fatalf("Failed to create test schema: %v", err) + } + + originalWd, err := os.Getwd() + if err != nil { + t.Fatalf("Failed to get current working directory: %v", err) + } + defer func() { + if err := os.Chdir(originalWd); err != nil { + t.Fatalf("Failed to restore working directory: %v", err) + } + }() + + tmpDir := t.TempDir() + if err := os.Chdir(tmpDir); err != nil { + t.Fatalf("Failed to change to temp directory: %v", err) + } + + // Create .pgschemaignore that ignores dblink_* functions and temp_* tables + ignoreContent := `[functions] +patterns = ["dblink_*"] + +[tables] +patterns = ["temp_*"] +` + err = os.WriteFile(".pgschemaignore", []byte(ignoreContent), 0644) + if err != nil { + t.Fatalf("Failed to create .pgschemaignore: %v", err) + } + + t.Run("dump", func(t *testing.T) { + output := executeIgnoreDumpCommand(t, containerInfo) + + // Users table and its privileges should be present + if !strings.Contains(output, "CREATE TABLE IF NOT EXISTS users") { + t.Error("Dump should include users table") + } + if !strings.Contains(output, "app_user") { + t.Error("Dump should include privileges for app_user on non-ignored tables") + } + + // Ignored functions should not appear + if strings.Contains(output, "dblink_connect_u") { + t.Error("Dump should not include dblink_connect_u (function is ignored, so REVOKE statements should also be ignored)") + } + + // Ignored table privileges should not appear + if strings.Contains(output, "temp_data") { + t.Error("Dump should not include temp_data or its privileges (table is ignored)") + } + }) + + t.Run("plan", func(t *testing.T) { + schemaSQL := ` +CREATE TABLE users ( + id SERIAL PRIMARY KEY, + name TEXT NOT NULL +); + +DO $$ +BEGIN + IF NOT EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'app_user') THEN + CREATE ROLE app_user; + END IF; +END $$; + +GRANT SELECT ON users TO app_user; +` + schemaFile := "schema.sql" + err := os.WriteFile(schemaFile, []byte(schemaSQL), 0644) + if err != nil { + t.Fatalf("Failed to create schema file: %v", err) + } + defer os.Remove(schemaFile) + + output := executeIgnorePlanCommand(t, containerInfo, schemaFile) + + // Plan should not reference dblink functions or their privileges + if strings.Contains(output, "dblink_connect_u") { + t.Error("Plan should not include dblink_connect_u (function is ignored)") + } + + // Plan should not reference ignored tables + if strings.Contains(output, "temp_data") { + t.Error("Plan should not include temp_data (table is ignored)") + } + }) + + // Clean up roles from sharedEmbeddedPG (plan subtest may create roles there) + cleanupStatements := []string{ + "REASSIGN OWNED BY app_user TO testuser", + "DROP OWNED BY app_user", + "DROP ROLE IF EXISTS app_user", + } + sharedConn, _, _, _, _, _ := testutil.ConnectToPostgres(t, sharedEmbeddedPG) + defer sharedConn.Close() + for _, stmt := range cleanupStatements { + sharedConn.Exec(stmt) + } +} + // verifyPlanOutput checks that plan output excludes ignored objects func verifyPlanOutput(t *testing.T, output string) { // Changes that should appear in plan (regular objects) diff --git a/ir/ignore.go b/ir/ignore.go index aac78508..a4f6d638 100644 --- a/ir/ignore.go +++ b/ir/ignore.go @@ -5,6 +5,15 @@ import ( "strings" ) +// stripFunctionArgs removes the argument list from a function/procedure signature. +// e.g. "dblink_connect_u(text, text)" -> "dblink_connect_u" +func stripFunctionArgs(name string) string { + if idx := strings.Index(name, "("); idx != -1 { + return name[:idx] + } + return name +} + // IgnoreConfig represents the configuration for ignoring database objects type IgnoreConfig struct { Tables []string `toml:"tables,omitempty"` @@ -65,6 +74,30 @@ func (c *IgnoreConfig) ShouldIgnoreSequence(sequenceName string) bool { return c.shouldIgnore(sequenceName, c.Sequences) } +// ShouldIgnorePrivilegeByObjectType checks if a privilege should be ignored based on the object name +// and its type. When an object (function, table, etc.) is ignored via its section pattern, +// privileges on that object should also be ignored. +func (c *IgnoreConfig) ShouldIgnorePrivilegeByObjectType(objectName string, objectType string) bool { + if c == nil { + return false + } + switch objectType { + case "TABLE": + return c.shouldIgnore(objectName, c.Tables) + case "VIEW": + return c.shouldIgnore(objectName, c.Views) + case "FUNCTION": + return c.shouldIgnore(stripFunctionArgs(objectName), c.Functions) + case "PROCEDURE": + return c.shouldIgnore(stripFunctionArgs(objectName), c.Procedures) + case "SEQUENCE": + return c.shouldIgnore(objectName, c.Sequences) + case "TYPE": + return c.shouldIgnore(objectName, c.Types) + } + return false +} + // ShouldIgnorePrivilege checks if a privilege should be ignored based on the grantee role name func (c *IgnoreConfig) ShouldIgnorePrivilege(grantee string) bool { if c == nil { diff --git a/ir/inspector.go b/ir/inspector.go index 6b902f56..06768763 100644 --- a/ir/inspector.go +++ b/ir/inspector.go @@ -2121,6 +2121,11 @@ func (i *Inspector) buildPrivileges(ctx context.Context, schema *IR, targetSchem continue } + // Skip privileges for ignored objects (e.g., ignored functions should have their privileges ignored too) + if i.ignoreConfig != nil && i.ignoreConfig.ShouldIgnorePrivilegeByObjectType(objectName, objectType) { + continue + } + // Check for default PUBLIC grants that should be excluded if grantee == "PUBLIC" { if (objectType == "FUNCTION" || objectType == "PROCEDURE") && privilegeType == "EXECUTE" { @@ -2216,6 +2221,11 @@ func (i *Inspector) buildRevokedDefaultPrivileges(ctx context.Context, targetSch objectName := row.ObjectName.String objectType := row.ObjectType.String + // Skip revoked default privileges for ignored objects + if i.ignoreConfig != nil && i.ignoreConfig.ShouldIgnorePrivilegeByObjectType(objectName, objectType) { + continue + } + var objType PrivilegeObjectType var privs []string switch objectType { @@ -2470,6 +2480,11 @@ func (i *Inspector) buildColumnPrivileges(ctx context.Context, schema *IR, targe continue } + // Skip column privileges for ignored tables/views + if i.ignoreConfig != nil && (i.ignoreConfig.ShouldIgnoreTable(tableName) || i.ignoreConfig.ShouldIgnoreView(tableName)) { + continue + } + key := colPrivKey{ TableName: tableName, Grantee: grantee,