diff --git a/internal/migration_acceptance_tests/object_owner_cases_test.go b/internal/migration_acceptance_tests/object_owner_cases_test.go new file mode 100644 index 0000000..f520e8b --- /dev/null +++ b/internal/migration_acceptance_tests/object_owner_cases_test.go @@ -0,0 +1,134 @@ +package migration_acceptance_tests + +import ( + "testing" + + "github.com/stripe/pg-schema-diff/pkg/diff" +) + +var objectOwnerAcceptanceTestCases = []acceptanceTestCase{ + { + name: "Change table owner", + roles: []string{"owner_a", "owner_b"}, + oldSchemaDDL: []string{` + CREATE TABLE owner_test(id int); + ALTER TABLE owner_test OWNER TO owner_a; + `}, + newSchemaDDL: []string{` + CREATE TABLE owner_test(id int); + ALTER TABLE owner_test OWNER TO owner_b; + `}, + expectedHazardTypes: []diff.MigrationHazardType{diff.MigrationHazardTypeAuthzUpdate}, + expectedPlanDDL: []string{`ALTER TABLE "public"."owner_test" OWNER TO "owner_b"`}, + }, + { + name: "Change function owner", + roles: []string{"owner_a", "owner_b"}, + oldSchemaDDL: []string{` + CREATE FUNCTION owner_add(a integer, b integer) RETURNS integer + LANGUAGE SQL + IMMUTABLE + RETURNS NULL ON NULL INPUT + RETURN a + b; + ALTER FUNCTION owner_add(a integer, b integer) OWNER TO owner_a; + `}, + newSchemaDDL: []string{` + CREATE FUNCTION owner_add(a integer, b integer) RETURNS integer + LANGUAGE SQL + IMMUTABLE + RETURNS NULL ON NULL INPUT + RETURN a + b; + ALTER FUNCTION owner_add(a integer, b integer) OWNER TO owner_b; + `}, + expectedHazardTypes: []diff.MigrationHazardType{diff.MigrationHazardTypeAuthzUpdate}, + expectedPlanDDL: []string{`ALTER FUNCTION "public"."owner_add"(a integer, b integer) OWNER TO "owner_b"`}, + }, + { + name: "Change procedure owner", + roles: []string{"owner_a", "owner_b"}, + oldSchemaDDL: []string{` + CREATE PROCEDURE owner_work() + LANGUAGE plpgsql + AS $$ + BEGIN + END; + $$; + ALTER PROCEDURE owner_work() OWNER TO owner_a; + `}, + newSchemaDDL: []string{` + CREATE PROCEDURE owner_work() + LANGUAGE plpgsql + AS $$ + BEGIN + END; + $$; + ALTER PROCEDURE owner_work() OWNER TO owner_b; + `}, + expectedHazardTypes: []diff.MigrationHazardType{diff.MigrationHazardTypeAuthzUpdate}, + expectedPlanDDL: []string{`ALTER PROCEDURE "public"."owner_work"() OWNER TO "owner_b"`}, + }, + { + name: "Change enum owner", + roles: []string{"owner_a", "owner_b"}, + oldSchemaDDL: []string{` + CREATE TYPE owner_color AS ENUM ('red', 'blue'); + ALTER TYPE owner_color OWNER TO owner_a; + `}, + newSchemaDDL: []string{` + CREATE TYPE owner_color AS ENUM ('red', 'blue'); + ALTER TYPE owner_color OWNER TO owner_b; + `}, + expectedHazardTypes: []diff.MigrationHazardType{diff.MigrationHazardTypeAuthzUpdate}, + expectedPlanDDL: []string{`ALTER TYPE "public"."owner_color" OWNER TO "owner_b"`}, + }, + { + name: "Change sequence owner", + roles: []string{"owner_a", "owner_b"}, + oldSchemaDDL: []string{` + CREATE SEQUENCE owner_seq OWNED BY NONE; + ALTER SEQUENCE owner_seq OWNER TO owner_a; + `}, + newSchemaDDL: []string{` + CREATE SEQUENCE owner_seq OWNED BY NONE; + ALTER SEQUENCE owner_seq OWNER TO owner_b; + `}, + expectedHazardTypes: []diff.MigrationHazardType{diff.MigrationHazardTypeAuthzUpdate}, + expectedPlanDDL: []string{`ALTER SEQUENCE "public"."owner_seq" OWNER TO "owner_b"`}, + }, + { + name: "Change view owner", + roles: []string{"owner_a", "owner_b"}, + oldSchemaDDL: []string{` + CREATE TABLE owner_base(id int); + CREATE VIEW owner_view AS SELECT id FROM owner_base; + ALTER VIEW owner_view OWNER TO owner_a; + `}, + newSchemaDDL: []string{` + CREATE TABLE owner_base(id int); + CREATE VIEW owner_view AS SELECT id FROM owner_base; + ALTER VIEW owner_view OWNER TO owner_b; + `}, + expectedHazardTypes: []diff.MigrationHazardType{diff.MigrationHazardTypeAuthzUpdate}, + expectedPlanDDL: []string{`ALTER VIEW "public"."owner_view" OWNER TO "owner_b"`}, + }, + { + name: "Change materialized view owner", + roles: []string{"owner_a", "owner_b"}, + oldSchemaDDL: []string{` + CREATE TABLE owner_base(id int); + CREATE MATERIALIZED VIEW owner_mview AS SELECT id FROM owner_base; + ALTER MATERIALIZED VIEW owner_mview OWNER TO owner_a; + `}, + newSchemaDDL: []string{` + CREATE TABLE owner_base(id int); + CREATE MATERIALIZED VIEW owner_mview AS SELECT id FROM owner_base; + ALTER MATERIALIZED VIEW owner_mview OWNER TO owner_b; + `}, + expectedHazardTypes: []diff.MigrationHazardType{diff.MigrationHazardTypeAuthzUpdate}, + expectedPlanDDL: []string{`ALTER MATERIALIZED VIEW "public"."owner_mview" OWNER TO "owner_b"`}, + }, +} + +func TestObjectOwnerTestCases(t *testing.T) { + runTestCases(t, objectOwnerAcceptanceTestCases) +} diff --git a/internal/queries/queries.sql b/internal/queries/queries.sql index d8397c8..a937d9e 100644 --- a/internal/queries/queries.sql +++ b/internal/queries/queries.sql @@ -20,6 +20,7 @@ SELECT c.oid, c.relname::TEXT AS table_name, table_namespace.nspname::TEXT AS table_schema_name, + owner_role.rolname::TEXT AS owner, c.relreplident::TEXT AS replica_identity, c.relrowsecurity AS rls_enabled, c.relforcerowsecurity AS rls_forced, @@ -38,6 +39,7 @@ FROM pg_catalog.pg_class AS c INNER JOIN pg_catalog.pg_namespace AS table_namespace ON c.relnamespace = table_namespace.oid +INNER JOIN pg_catalog.pg_roles AS owner_role ON c.relowner = owner_role.oid LEFT JOIN pg_catalog.pg_inherits AS table_inherits ON c.oid = table_inherits.inhrelid @@ -262,6 +264,7 @@ SELECT pg_proc.oid, pg_proc.proname::TEXT AS func_name, proc_namespace.nspname::TEXT AS func_schema_name, + owner_role.rolname::TEXT AS owner, proc_lang.lanname::TEXT AS func_lang, pg_catalog.pg_get_function_identity_arguments( pg_proc.oid @@ -271,6 +274,7 @@ FROM pg_catalog.pg_proc INNER JOIN pg_catalog.pg_namespace AS proc_namespace ON pg_proc.pronamespace = proc_namespace.oid +INNER JOIN pg_catalog.pg_roles AS owner_role ON pg_proc.proowner = owner_role.oid INNER JOIN pg_catalog.pg_language AS proc_lang ON pg_proc.prolang = proc_lang.oid @@ -341,6 +345,7 @@ WHERE SELECT seq_c.relname::TEXT AS sequence_name, seq_ns.nspname::TEXT AS sequence_schema_name, + owner_role.rolname::TEXT AS owner, COALESCE(owner_attr.attname, '')::TEXT AS owner_column_name, COALESCE(owner_ns.nspname, '')::TEXT AS owner_schema_name, COALESCE(owner_c.relname, '')::TEXT AS owner_table_name, @@ -354,6 +359,7 @@ SELECT FROM pg_catalog.pg_sequence AS pg_seq INNER JOIN pg_catalog.pg_class AS seq_c ON pg_seq.seqrelid = seq_c.oid INNER JOIN pg_catalog.pg_namespace AS seq_ns ON seq_c.relnamespace = seq_ns.oid +INNER JOIN pg_catalog.pg_roles AS owner_role ON seq_c.relowner = owner_role.oid LEFT JOIN pg_catalog.pg_depend AS depend ON depend.classid = 'pg_class'::REGCLASS @@ -405,6 +411,7 @@ WHERE SELECT pg_type.typname::TEXT AS enum_name, type_namespace.nspname::TEXT AS enum_schema_name, + owner_role.rolname::TEXT AS owner, (SELECT ARRAY_AGG( pg_enum.enumlabel @@ -416,6 +423,7 @@ FROM pg_catalog.pg_type AS pg_type INNER JOIN pg_catalog.pg_namespace AS type_namespace ON pg_type.typnamespace = type_namespace.oid +INNER JOIN pg_catalog.pg_roles AS owner_role ON pg_type.typowner = owner_role.oid WHERE pg_type.typtype = 'e' AND type_namespace.nspname NOT IN ('pg_catalog', 'information_schema') @@ -488,6 +496,7 @@ WHERE SELECT n.nspname::TEXT AS schema_name, c.relname::TEXT AS view_name, + owner_role.rolname::TEXT AS owner, c.reloptions::TEXT [] AS rel_options, (SELECT ARRAY_AGG(DISTINCT JSONB_BUILD_OBJECT( @@ -539,6 +548,7 @@ SELECT PG_GET_VIEWDEF(c.oid, true) AS view_definition FROM pg_catalog.pg_class AS c INNER JOIN pg_catalog.pg_namespace AS n ON c.relnamespace = n.oid +INNER JOIN pg_catalog.pg_roles AS owner_role ON c.relowner = owner_role.oid WHERE c.relkind = 'v' AND n.nspname NOT IN ('pg_catalog', 'information_schema') @@ -557,6 +567,7 @@ WHERE SELECT n.nspname::TEXT AS schema_name, c.relname::TEXT AS view_name, + owner_role.rolname::TEXT AS owner, c.reloptions::TEXT [] AS rel_options, COALESCE(ts.spcname, '')::TEXT AS tablespace_name, (SELECT @@ -610,6 +621,7 @@ SELECT PG_GET_VIEWDEF(c.oid, true) AS view_definition FROM pg_catalog.pg_class AS c INNER JOIN pg_catalog.pg_namespace AS n ON c.relnamespace = n.oid +INNER JOIN pg_catalog.pg_roles AS owner_role ON c.relowner = owner_role.oid LEFT JOIN pg_catalog.pg_tablespace AS ts ON c.reltablespace = ts.oid WHERE c.relkind = 'm' diff --git a/internal/queries/queries.sql.go b/internal/queries/queries.sql.go index 4315102..a8ae1c5 100644 --- a/internal/queries/queries.sql.go +++ b/internal/queries/queries.sql.go @@ -1,6 +1,6 @@ // Code generated by sqlc. DO NOT EDIT. // versions: -// sqlc v1.18.0 +// sqlc v1.31.1 // source: queries.sql package queries @@ -282,6 +282,7 @@ const getEnums = `-- name: GetEnums :many SELECT pg_type.typname::TEXT AS enum_name, type_namespace.nspname::TEXT AS enum_schema_name, + owner_role.rolname::TEXT AS owner, (SELECT ARRAY_AGG( pg_enum.enumlabel @@ -293,6 +294,7 @@ FROM pg_catalog.pg_type AS pg_type INNER JOIN pg_catalog.pg_namespace AS type_namespace ON pg_type.typnamespace = type_namespace.oid +INNER JOIN pg_catalog.pg_roles AS owner_role ON pg_type.typowner = owner_role.oid WHERE pg_type.typtype = 'e' AND type_namespace.nspname NOT IN ('pg_catalog', 'information_schema') @@ -312,6 +314,7 @@ WHERE type GetEnumsRow struct { EnumName string EnumSchemaName string + Owner string EnumLabels []string } @@ -324,7 +327,12 @@ func (q *Queries) GetEnums(ctx context.Context) ([]GetEnumsRow, error) { var items []GetEnumsRow for rows.Next() { var i GetEnumsRow - if err := rows.Scan(&i.EnumName, &i.EnumSchemaName, pq.Array(&i.EnumLabels)); err != nil { + if err := rows.Scan( + &i.EnumName, + &i.EnumSchemaName, + &i.Owner, + pq.Array(&i.EnumLabels), + ); err != nil { return nil, err } items = append(items, i) @@ -585,6 +593,7 @@ const getMaterializedViews = `-- name: GetMaterializedViews :many SELECT n.nspname::TEXT AS schema_name, c.relname::TEXT AS view_name, + owner_role.rolname::TEXT AS owner, c.reloptions::TEXT [] AS rel_options, COALESCE(ts.spcname, '')::TEXT AS tablespace_name, (SELECT @@ -638,6 +647,7 @@ SELECT PG_GET_VIEWDEF(c.oid, true) AS view_definition FROM pg_catalog.pg_class AS c INNER JOIN pg_catalog.pg_namespace AS n ON c.relnamespace = n.oid +INNER JOIN pg_catalog.pg_roles AS owner_role ON c.relowner = owner_role.oid LEFT JOIN pg_catalog.pg_tablespace AS ts ON c.reltablespace = ts.oid WHERE c.relkind = 'm' @@ -657,6 +667,7 @@ WHERE type GetMaterializedViewsRow struct { SchemaName string ViewName string + Owner string RelOptions []string TablespaceName string TableDependencies []string @@ -675,6 +686,7 @@ func (q *Queries) GetMaterializedViews(ctx context.Context) ([]GetMaterializedVi if err := rows.Scan( &i.SchemaName, &i.ViewName, + &i.Owner, pq.Array(&i.RelOptions), &i.TablespaceName, pq.Array(&i.TableDependencies), @@ -796,6 +808,7 @@ SELECT pg_proc.oid, pg_proc.proname::TEXT AS func_name, proc_namespace.nspname::TEXT AS func_schema_name, + owner_role.rolname::TEXT AS owner, proc_lang.lanname::TEXT AS func_lang, pg_catalog.pg_get_function_identity_arguments( pg_proc.oid @@ -805,6 +818,7 @@ FROM pg_catalog.pg_proc INNER JOIN pg_catalog.pg_namespace AS proc_namespace ON pg_proc.pronamespace = proc_namespace.oid +INNER JOIN pg_catalog.pg_roles AS owner_role ON pg_proc.proowner = owner_role.oid INNER JOIN pg_catalog.pg_language AS proc_lang ON pg_proc.prolang = proc_lang.oid @@ -828,6 +842,7 @@ type GetProcsRow struct { Oid interface{} FuncName string FuncSchemaName string + Owner string FuncLang string FuncIdentityArguments string FuncDef string @@ -846,6 +861,7 @@ func (q *Queries) GetProcs(ctx context.Context, prokind interface{}) ([]GetProcs &i.Oid, &i.FuncName, &i.FuncSchemaName, + &i.Owner, &i.FuncLang, &i.FuncIdentityArguments, &i.FuncDef, @@ -908,6 +924,7 @@ const getSequences = `-- name: GetSequences :many SELECT seq_c.relname::TEXT AS sequence_name, seq_ns.nspname::TEXT AS sequence_schema_name, + owner_role.rolname::TEXT AS owner, COALESCE(owner_attr.attname, '')::TEXT AS owner_column_name, COALESCE(owner_ns.nspname, '')::TEXT AS owner_schema_name, COALESCE(owner_c.relname, '')::TEXT AS owner_table_name, @@ -921,6 +938,7 @@ SELECT FROM pg_catalog.pg_sequence AS pg_seq INNER JOIN pg_catalog.pg_class AS seq_c ON pg_seq.seqrelid = seq_c.oid INNER JOIN pg_catalog.pg_namespace AS seq_ns ON seq_c.relnamespace = seq_ns.oid +INNER JOIN pg_catalog.pg_roles AS owner_role ON seq_c.relowner = owner_role.oid LEFT JOIN pg_catalog.pg_depend AS depend ON depend.classid = 'pg_class'::REGCLASS @@ -956,6 +974,7 @@ WHERE type GetSequencesRow struct { SequenceName string SequenceSchemaName string + Owner string OwnerColumnName string OwnerSchemaName string OwnerTableName string @@ -980,6 +999,7 @@ func (q *Queries) GetSequences(ctx context.Context) ([]GetSequencesRow, error) { if err := rows.Scan( &i.SequenceName, &i.SequenceSchemaName, + &i.Owner, &i.OwnerColumnName, &i.OwnerSchemaName, &i.OwnerTableName, @@ -1089,6 +1109,7 @@ SELECT c.oid, c.relname::TEXT AS table_name, table_namespace.nspname::TEXT AS table_schema_name, + owner_role.rolname::TEXT AS owner, c.relreplident::TEXT AS replica_identity, c.relrowsecurity AS rls_enabled, c.relforcerowsecurity AS rls_forced, @@ -1107,6 +1128,7 @@ FROM pg_catalog.pg_class AS c INNER JOIN pg_catalog.pg_namespace AS table_namespace ON c.relnamespace = table_namespace.oid +INNER JOIN pg_catalog.pg_roles AS owner_role ON c.relowner = owner_role.oid LEFT JOIN pg_catalog.pg_inherits AS table_inherits ON c.oid = table_inherits.inhrelid @@ -1136,6 +1158,7 @@ type GetTablesRow struct { Oid interface{} TableName string TableSchemaName string + Owner string ReplicaIdentity string RlsEnabled bool RlsForced bool @@ -1158,6 +1181,7 @@ func (q *Queries) GetTables(ctx context.Context) ([]GetTablesRow, error) { &i.Oid, &i.TableName, &i.TableSchemaName, + &i.Owner, &i.ReplicaIdentity, &i.RlsEnabled, &i.RlsForced, @@ -1255,6 +1279,7 @@ const getViews = `-- name: GetViews :many SELECT n.nspname::TEXT AS schema_name, c.relname::TEXT AS view_name, + owner_role.rolname::TEXT AS owner, c.reloptions::TEXT [] AS rel_options, (SELECT ARRAY_AGG(DISTINCT JSONB_BUILD_OBJECT( @@ -1306,6 +1331,7 @@ SELECT PG_GET_VIEWDEF(c.oid, true) AS view_definition FROM pg_catalog.pg_class AS c INNER JOIN pg_catalog.pg_namespace AS n ON c.relnamespace = n.oid +INNER JOIN pg_catalog.pg_roles AS owner_role ON c.relowner = owner_role.oid WHERE c.relkind = 'v' AND n.nspname NOT IN ('pg_catalog', 'information_schema') @@ -1324,6 +1350,7 @@ WHERE type GetViewsRow struct { SchemaName string ViewName string + Owner string RelOptions []string TableDependencies []string ViewDefinition string @@ -1341,6 +1368,7 @@ func (q *Queries) GetViews(ctx context.Context) ([]GetViewsRow, error) { if err := rows.Scan( &i.SchemaName, &i.ViewName, + &i.Owner, pq.Array(&i.RelOptions), pq.Array(&i.TableDependencies), &i.ViewDefinition, diff --git a/internal/schema/schema.go b/internal/schema/schema.go index b3945d5..370ac12 100644 --- a/internal/schema/schema.go +++ b/internal/schema/schema.go @@ -209,11 +209,15 @@ type Extension struct { type Enum struct { SchemaQualifiedName + // Owner is the role that owns the enum type. + Owner string Labels []string } type Table struct { SchemaQualifiedName + // Owner is the role that owns the table. + Owner string Columns []Column CheckConstraints []CheckConstraint Policies []Policy @@ -428,6 +432,9 @@ type ( Sequence struct { SchemaQualifiedName + // RoleOwner is the role that owns the sequence. Owner is already used for + // the sequence's OWNED BY table/column dependency. + RoleOwner string Owner *SequenceOwner Type string StartValue int64 @@ -441,6 +448,8 @@ type ( type Function struct { SchemaQualifiedName + // Owner is the role that owns the function. + Owner string // FunctionDef is the statement required to completely (re)create // the function, as returned by `pg_get_functiondef`. It is a CREATE OR REPLACE // statement @@ -453,6 +462,8 @@ type Function struct { type Procedure struct { SchemaQualifiedName + // Owner is the role that owns the procedure. + Owner string // Def is the statement required to completely (re)create // the procedure, as returned by `pg_get_functiondef`. It is a CREATE OR REPLACE // statement. @@ -524,6 +535,8 @@ type TableDependency struct { type View struct { SchemaQualifiedName + // Owner is the role that owns the view. + Owner string // ViewDefinition is the select query that defines the view. It is derived from pg_get_viewdef. ViewDefinition string // Options represents key value map of view options, i.e., pg_class.reloptions. @@ -535,6 +548,8 @@ type View struct { type MaterializedView struct { SchemaQualifiedName + // Owner is the role that owns the materialized view. + Owner string // ViewDefinition is the select query that defines the materialized view. It is derived from pg_get_viewdef. ViewDefinition string // Options represents key value map of materialized view options, i.e., pg_class.reloptions. @@ -909,6 +924,7 @@ func (s *schemaFetcher) fetchEnums(ctx context.Context) ([]Enum, error) { SchemaName: rawEnum.EnumSchemaName, EscapedName: EscapeIdentifier(rawEnum.EnumName), }, + Owner: rawEnum.Owner, Labels: rawEnum.EnumLabels, }) } @@ -1052,6 +1068,7 @@ func (s *schemaFetcher) buildTable( } return Table{ SchemaQualifiedName: schemaQualifiedName, + Owner: table.Owner, Columns: columns, CheckConstraints: checkConsByTable[schemaQualifiedName.GetFQEscapedName()], Policies: policiesByTable[schemaQualifiedName.GetFQEscapedName()], @@ -1254,6 +1271,7 @@ func (s *schemaFetcher) fetchSequences(ctx context.Context) ([]Sequence, error) SchemaName: rawSeq.SequenceSchemaName, EscapedName: EscapeIdentifier(rawSeq.SequenceName), }, + RoleOwner: rawSeq.Owner, Owner: owner, Type: rawSeq.DataType, StartValue: rawSeq.StartValue, @@ -1322,6 +1340,7 @@ func (s *schemaFetcher) buildFunction(ctx context.Context, rawFunction queries.G return Function{ SchemaQualifiedName: buildProcName(rawFunction.FuncName, rawFunction.FuncIdentityArguments, rawFunction.FuncSchemaName), + Owner: rawFunction.Owner, FunctionDef: rawFunction.FuncDef, Language: rawFunction.FuncLang, DependsOnFunctions: dependsOnFunctions, @@ -1355,6 +1374,7 @@ func (s *schemaFetcher) fetchProcedures(ctx context.Context) ([]Procedure, error for _, rawProcedure := range rawProcedures { p := Procedure{ SchemaQualifiedName: buildProcName(rawProcedure.FuncName, rawProcedure.FuncIdentityArguments, rawProcedure.FuncSchemaName), + Owner: rawProcedure.Owner, Def: rawProcedure.FuncDef, } procedures = append(procedures, p) @@ -1505,6 +1525,7 @@ func (s *schemaFetcher) fetchViews(ctx context.Context) ([]View, error) { views = append(views, View{ SchemaQualifiedName: buildNameFromUnescaped(v.ViewName, v.SchemaName), + Owner: v.Owner, ViewDefinition: v.ViewDefinition, Options: options, @@ -1543,6 +1564,7 @@ func (s *schemaFetcher) fetchMaterializedViews(ctx context.Context) ([]Materiali materializedViews = append(materializedViews, MaterializedView{ SchemaQualifiedName: buildNameFromUnescaped(mv.ViewName, mv.SchemaName), + Owner: mv.Owner, ViewDefinition: mv.ViewDefinition, Options: options, Tablespace: mv.TablespaceName, diff --git a/internal/schema/schema_test.go b/internal/schema/schema_test.go index c73bb17..4260d35 100644 --- a/internal/schema/schema_test.go +++ b/internal/schema/schema_test.go @@ -239,7 +239,7 @@ var ( GRANT SELECT ON schema_2.foo TO some_role_1; GRANT INSERT ON schema_2.foo TO some_role_2 WITH GRANT OPTION; `}, - expectedHash: "4c2174e2cac3956b", + expectedHash: "e045c528fd265b07", expectedSchema: Schema{ NamedSchemas: []NamedSchema{ {Name: "public"}, @@ -591,7 +591,7 @@ var ( ALTER TABLE foo_fk_1 ADD CONSTRAINT foo_fk_1_fk FOREIGN KEY (author, content) REFERENCES foo_1 (author, content) NOT VALID; `}, - expectedHash: "32c5a9c52dcfb15e", + expectedHash: "a8c17a395024ffde", expectedSchema: Schema{ NamedSchemas: []NamedSchema{ {Name: "public"}, @@ -1343,13 +1343,14 @@ func runTestCase(t *testing.T, engine *pgengine.Engine, testCase *testCase, getD require.NoError(t, err) } - expectedNormalized := testCase.expectedSchema.Normalize() + expectedSchema := fillDefaultObjectOwners(testCase.expectedSchema) + expectedNormalized := expectedSchema.Normalize() fetchedNormalized := fetchedSchema.Normalize() assert.Equal(t, expectedNormalized, fetchedNormalized, "expected=\n%# v \n fetched=%# v\n", pretty.Formatter(expectedNormalized), pretty.Formatter(fetchedNormalized)) fetchedSchemaHash, err := fetchedSchema.Hash() require.NoError(t, err) - expectedSchemaHash, err := testCase.expectedSchema.Hash() + expectedSchemaHash, err := expectedSchema.Hash() require.NoError(t, err) // same schemas should have the same hashes assert.Equal(t, expectedSchemaHash, fetchedSchemaHash, "hash of expected schema should match fetched hash") @@ -1359,6 +1360,45 @@ func runTestCase(t *testing.T, engine *pgengine.Engine, testCase *testCase, getD } } +func fillDefaultObjectOwners(s Schema) Schema { + for i := range s.Enums { + if s.Enums[i].Owner == "" { + s.Enums[i].Owner = "postgres" + } + } + for i := range s.Tables { + if s.Tables[i].Owner == "" { + s.Tables[i].Owner = "postgres" + } + } + for i := range s.Sequences { + if s.Sequences[i].RoleOwner == "" { + s.Sequences[i].RoleOwner = "postgres" + } + } + for i := range s.Functions { + if s.Functions[i].Owner == "" { + s.Functions[i].Owner = "postgres" + } + } + for i := range s.Procedures { + if s.Procedures[i].Owner == "" { + s.Procedures[i].Owner = "postgres" + } + } + for i := range s.Views { + if s.Views[i].Owner == "" { + s.Views[i].Owner = "postgres" + } + } + for i := range s.MaterializedViews { + if s.MaterializedViews[i].Owner == "" { + s.MaterializedViews[i].Owner = "postgres" + } + } + return s +} + func TestIdxDefStmtToCreateIdxConcurrently(t *testing.T) { for _, tc := range []struct { name string diff --git a/pkg/diff/enum_sql_generator.go b/pkg/diff/enum_sql_generator.go index de5ab4f..ee3d35d 100644 --- a/pkg/diff/enum_sql_generator.go +++ b/pkg/diff/enum_sql_generator.go @@ -19,13 +19,15 @@ func (e *enumSQLGenerator) Add(enum schema.Enum) ([]Statement, error) { for _, val := range enum.Labels { escapedEnumVals = append(escapedEnumVals, fmt.Sprintf("'%s'", val)) } - return []Statement{ + stmts := []Statement{ { DDL: fmt.Sprintf("CREATE TYPE %s AS ENUM (%s)", enum.GetFQEscapedName(), strings.Join(escapedEnumVals, ", ")), Timeout: statementTimeoutDefault, LockTimeout: lockTimeoutDefault, }, - }, nil + } + stmts = append(stmts, ownerDDLForAdd(ownershipTarget("TYPE", enum.SchemaQualifiedName), enum.Owner)...) + return stmts, nil } func (e *enumSQLGenerator) Delete(enum schema.Enum) ([]Statement, error) { @@ -87,6 +89,8 @@ func (e *enumSQLGenerator) Alter(diff enumDiff) ([]Statement, error) { }) } oldCopy.Labels = diff.new.Labels + stmts = append(stmts, ownerDDLForAlter(ownershipTarget("TYPE", diff.new.SchemaQualifiedName), diff.old.Owner, diff.new.Owner)...) + oldCopy.Owner = diff.new.Owner if diff := cmp.Diff(oldCopy, diff.new); diff != "" { return nil, fmt.Errorf("unable to resolve the diff %s: %w", diff, ErrNotImplemented) diff --git a/pkg/diff/function_sql_vertex_generator.go b/pkg/diff/function_sql_vertex_generator.go index 088ff6a..238147e 100644 --- a/pkg/diff/function_sql_vertex_generator.go +++ b/pkg/diff/function_sql_vertex_generator.go @@ -30,12 +30,14 @@ func (f *functionSQLVertexGenerator) Add(function schema.Function) ([]Statement, "created/altered before this statement.", }) } - return []Statement{{ + stmts := []Statement{{ DDL: function.FunctionDef, Timeout: statementTimeoutDefault, LockTimeout: lockTimeoutDefault, Hazards: hazards, - }}, nil + }} + stmts = append(stmts, ownerDDLForAdd(ownershipTarget("FUNCTION", function.SchemaQualifiedName), function.Owner)...) + return stmts, nil } func (f *functionSQLVertexGenerator) Delete(function schema.Function) ([]Statement, error) { @@ -60,10 +62,22 @@ func (f *functionSQLVertexGenerator) Delete(function schema.Function) ([]Stateme func (f *functionSQLVertexGenerator) Alter(diff functionDiff) ([]Statement, error) { // We are assuming the function has been normalized, i.e., we don't have to worry DependsOnFunctions ordering // causing a false positive diff detected. + oldCopy := diff.old + oldCopy.Owner = diff.new.Owner + if cmp.Equal(oldCopy, diff.new) { + return ownerDDLForAlter(ownershipTarget("FUNCTION", diff.new.SchemaQualifiedName), diff.old.Owner, diff.new.Owner), nil + } if cmp.Equal(diff.old, diff.new) { return nil, nil } - return f.Add(diff.new) + newForAlter := diff.new + newForAlter.Owner = "" + stmts, err := f.Add(newForAlter) + if err != nil { + return nil, err + } + stmts = append(stmts, ownerDDLForAlter(ownershipTarget("FUNCTION", diff.new.SchemaQualifiedName), diff.old.Owner, diff.new.Owner)...) + return stmts, nil } func canFunctionDependenciesBeTracked(function schema.Function) bool { diff --git a/pkg/diff/materialized_view_sql_generator.go b/pkg/diff/materialized_view_sql_generator.go index 9b5d216..1326f76 100644 --- a/pkg/diff/materialized_view_sql_generator.go +++ b/pkg/diff/materialized_view_sql_generator.go @@ -109,15 +109,18 @@ func (mvsg *materializedViewSQLGenerator) Add(mv schema.MaterializedView) (parti deps = append(deps, mustRun(addVertexId).after(buildTableVertexId(t.SchemaQualifiedName, diffTypeAddAlter))) } + stmts := []Statement{{ + DDL: materializedViewSb.String(), + Timeout: statementTimeoutDefault, + LockTimeout: lockTimeoutDefault, + }} + stmts = append(stmts, ownerDDLForAdd(ownershipTarget("MATERIALIZED VIEW", mv.SchemaQualifiedName), mv.Owner)...) + return partialSQLGraph{ vertices: []sqlVertex{{ - id: addVertexId, - priority: sqlPrioritySooner, - statements: []Statement{{ - DDL: materializedViewSb.String(), - Timeout: statementTimeoutDefault, - LockTimeout: lockTimeoutDefault, - }}, + id: addVertexId, + priority: sqlPrioritySooner, + statements: stmts, }}, dependencies: deps, }, nil @@ -149,10 +152,22 @@ func (mvsg *materializedViewSQLGenerator) Delete(mv schema.MaterializedView) (pa func (mvsg *materializedViewSQLGenerator) Alter(mvd materializedViewDiff) (partialSQLGraph, error) { // In the initial MVP, we will not support altering. - if !cmp.Equal(mvd.old, mvd.new) { + oldCopy := mvd.old + oldCopy.Owner = mvd.new.Owner + if !cmp.Equal(oldCopy, mvd.new) { return partialSQLGraph{}, ErrNotImplemented } - return partialSQLGraph{}, nil + stmts := ownerDDLForAlter(ownershipTarget("MATERIALIZED VIEW", mvd.new.SchemaQualifiedName), mvd.old.Owner, mvd.new.Owner) + if len(stmts) == 0 { + return partialSQLGraph{}, nil + } + return partialSQLGraph{ + vertices: []sqlVertex{{ + id: buildMaterializedViewVertexId(mvd.new.SchemaQualifiedName, diffTypeAddAlter), + priority: sqlPrioritySooner, + statements: stmts, + }}, + }, nil } func buildMaterializedViewVertexId(n schema.SchemaQualifiedName, d diffType) sqlVertexId { diff --git a/pkg/diff/owner_sql_generator.go b/pkg/diff/owner_sql_generator.go new file mode 100644 index 0000000..ca894e5 --- /dev/null +++ b/pkg/diff/owner_sql_generator.go @@ -0,0 +1,39 @@ +package diff + +import ( + "fmt" + + "github.com/stripe/pg-schema-diff/internal/schema" +) + +var migrationHazardOwnerChanged = MigrationHazard{ + Type: MigrationHazardTypeAuthzUpdate, + Message: "Changing object ownership changes implicit privileges for the old and new owners.", +} + +func ownerDDLForAdd(kindAndTarget, owner string) []Statement { + if owner == "" { + return nil + } + return stripMigrationHazards(buildAlterOwnerStatement(kindAndTarget, owner)) +} + +func ownerDDLForAlter(kindAndTarget, oldOwner, newOwner string) []Statement { + if oldOwner == newOwner || newOwner == "" { + return nil + } + return []Statement{buildAlterOwnerStatement(kindAndTarget, newOwner)} +} + +func ownershipTarget(kind string, name schema.SchemaQualifiedName) string { + return fmt.Sprintf("%s %s", kind, name.GetFQEscapedName()) +} + +func buildAlterOwnerStatement(kindAndTarget, owner string) Statement { + return Statement{ + DDL: fmt.Sprintf("ALTER %s OWNER TO %s", kindAndTarget, schema.EscapeIdentifier(owner)), + Timeout: statementTimeoutDefault, + LockTimeout: lockTimeoutDefault, + Hazards: []MigrationHazard{migrationHazardOwnerChanged}, + } +} diff --git a/pkg/diff/procedure_sql_vertex_generator.go b/pkg/diff/procedure_sql_vertex_generator.go index d9f725b..f6004be 100644 --- a/pkg/diff/procedure_sql_vertex_generator.go +++ b/pkg/diff/procedure_sql_vertex_generator.go @@ -39,22 +39,25 @@ func (p procedureSQLVertexGenerator) Add(s schema.Procedure) (partialSQLGraph, e deps = append(deps, mustRun(buildProcedureVertexId(s.SchemaQualifiedName, diffTypeAddAlter)).after(buildSequenceVertexId(seq.SchemaQualifiedName, diffTypeAddAlter))) } + stmts := []Statement{{ + DDL: s.Def, + Timeout: statementTimeoutDefault, + LockTimeout: lockTimeoutDefault, + Hazards: []MigrationHazard{{ + Type: MigrationHazardTypeHasUntrackableDependencies, + Message: "Dependencies of procedures are not tracked by Postgres. " + + "As a result, we cannot guarantee that this procedure's dependencies are ordered properly relative to " + + "this statement. For adds, this means you need to ensure that all objects this function depends on " + + "are added before this statement.", + }}, + }} + stmts = append(stmts, ownerDDLForAdd(ownershipTarget("PROCEDURE", s.SchemaQualifiedName), s.Owner)...) + return partialSQLGraph{ vertices: []sqlVertex{{ - id: buildProcedureVertexId(s.SchemaQualifiedName, diffTypeAddAlter), - priority: sqlPrioritySooner, - statements: []Statement{{ - DDL: s.Def, - Timeout: statementTimeoutDefault, - LockTimeout: lockTimeoutDefault, - Hazards: []MigrationHazard{{ - Type: MigrationHazardTypeHasUntrackableDependencies, - Message: "Dependencies of procedures are not tracked by Postgres. " + - "As a result, we cannot guarantee that this procedure's dependencies are ordered properly relative to " + - "this statement. For adds, this means you need to ensure that all objects this function depends on " + - "are added before this statement.", - }}, - }}, + id: buildProcedureVertexId(s.SchemaQualifiedName, diffTypeAddAlter), + priority: sqlPrioritySooner, + statements: stmts, }}, dependencies: deps, }, nil @@ -105,11 +108,36 @@ func (p procedureSQLVertexGenerator) Delete(s schema.Procedure) (partialSQLGraph } func (p procedureSQLVertexGenerator) Alter(d procedureDiff) (partialSQLGraph, error) { + oldCopy := d.old + oldCopy.Owner = d.new.Owner + if cmp.Equal(oldCopy, d.new) { + stmts := ownerDDLForAlter(ownershipTarget("PROCEDURE", d.new.SchemaQualifiedName), d.old.Owner, d.new.Owner) + if len(stmts) == 0 { + return partialSQLGraph{}, nil + } + return partialSQLGraph{ + vertices: []sqlVertex{{ + id: buildProcedureVertexId(d.new.SchemaQualifiedName, diffTypeAddAlter), + priority: sqlPrioritySooner, + statements: stmts, + }}, + }, nil + } if cmp.Equal(d.old, d.new) { return partialSQLGraph{}, nil } // New adds or replaces the procedure. - return p.Add(d.new) + newForAlter := d.new + newForAlter.Owner = "" + graph, err := p.Add(newForAlter) + if err != nil { + return partialSQLGraph{}, err + } + for i := range graph.vertices { + graph.vertices[i].statements = append(graph.vertices[i].statements, + ownerDDLForAlter(ownershipTarget("PROCEDURE", d.new.SchemaQualifiedName), d.old.Owner, d.new.Owner)...) + } + return graph, nil } func buildProcedureVertexId(name schema.SchemaQualifiedName, diffType diffType) sqlVertexId { diff --git a/pkg/diff/sql_generator.go b/pkg/diff/sql_generator.go index 6c31e22..ea85886 100644 --- a/pkg/diff/sql_generator.go +++ b/pkg/diff/sql_generator.go @@ -840,6 +840,7 @@ func (t *tableSQLVertexGenerator) Add(table schema.Table) ([]Statement, error) { Timeout: statementTimeoutDefault, LockTimeout: lockTimeoutDefault, }) + stmts = append(stmts, ownerDDLForAdd(ownershipTarget("TABLE", table.SchemaQualifiedName), table.Owner)...) csg := checkConstraintSQLVertexGenerator{ tableName: table.SchemaQualifiedName, @@ -930,6 +931,7 @@ func (t *tableSQLVertexGenerator) Alter(diff tableDiff) ([]Statement, error) { } var stmts []Statement + stmts = append(stmts, ownerDDLForAlter(ownershipTarget("TABLE", diff.new.SchemaQualifiedName), diff.old.Owner, diff.new.Owner)...) // Only handle disabling RLS if it was previously enabled. // We want to disable RLS before we do any other operations on the table, e.g., delete policies, to avoid creating an // outage while RLS is being disabled @@ -2520,9 +2522,11 @@ type sequenceSQLVertexGenerator struct { } func (s *sequenceSQLVertexGenerator) Add(seq schema.Sequence) ([]Statement, error) { - return []Statement{ + stmts := []Statement{ s.buildAddAlterSequenceStatement(seq, false), - }, nil + } + stmts = append(stmts, ownerDDLForAdd(ownershipTarget("SEQUENCE", seq.SchemaQualifiedName), seq.RoleOwner)...) + return stmts, nil } func (s *sequenceSQLVertexGenerator) Delete(seq schema.Sequence) ([]Statement, error) { @@ -2547,6 +2551,9 @@ func (s *sequenceSQLVertexGenerator) Alter(diff sequenceDiff) ([]Statement, erro var stmts []Statement // Ownership changes handled by the sequenceOwnershipSQLVertexGenerator diff.old.Owner = diff.new.Owner + oldRoleOwner := diff.old.RoleOwner + roleOwnerChanged := oldRoleOwner != diff.new.RoleOwner + diff.old.RoleOwner = diff.new.RoleOwner // Explicitly list all the diffs supported by the alter statement, rather than just using !cmp.Equal, so we don't // risk introducing a bug if we add new fields to schema.Sequence @@ -2572,6 +2579,9 @@ func (s *sequenceSQLVertexGenerator) Alter(diff sequenceDiff) ([]Statement, erro if !cmp.Equal(diff.old, diff.new) { return nil, fmt.Errorf("altering sequence to resolve the following diff %s: %w", cmp.Diff(diff.old, diff.new), ErrNotImplemented) } + if roleOwnerChanged { + stmts = append(stmts, ownerDDLForAlter(ownershipTarget("SEQUENCE", diff.new.SchemaQualifiedName), oldRoleOwner, diff.new.RoleOwner)...) + } return stmts, nil } diff --git a/pkg/diff/view_sql_generator.go b/pkg/diff/view_sql_generator.go index d96f28d..b0433cc 100644 --- a/pkg/diff/view_sql_generator.go +++ b/pkg/diff/view_sql_generator.go @@ -104,15 +104,18 @@ func (vsg *viewSQLGenerator) Add(v schema.View) (partialSQLGraph, error) { deps = append(deps, mustRun(addVertexId).after(buildTableVertexId(t.SchemaQualifiedName, diffTypeAddAlter))) } + stmts := []Statement{{ + DDL: viewSb.String(), + Timeout: statementTimeoutDefault, + LockTimeout: lockTimeoutDefault, + }} + stmts = append(stmts, ownerDDLForAdd(ownershipTarget("VIEW", v.SchemaQualifiedName), v.Owner)...) + return partialSQLGraph{ vertices: []sqlVertex{{ - id: addVertexId, - priority: sqlPrioritySooner, - statements: []Statement{{ - DDL: viewSb.String(), - Timeout: statementTimeoutDefault, - LockTimeout: lockTimeoutDefault, - }}, + id: addVertexId, + priority: sqlPrioritySooner, + statements: stmts, }}, dependencies: deps, }, nil @@ -144,10 +147,22 @@ func (vsg *viewSQLGenerator) Delete(v schema.View) (partialSQLGraph, error) { func (vsg *viewSQLGenerator) Alter(vd viewDiff) (partialSQLGraph, error) { // In the initial MVP, we will not support altering. - if !cmp.Equal(vd.old, vd.new) { + oldCopy := vd.old + oldCopy.Owner = vd.new.Owner + if !cmp.Equal(oldCopy, vd.new) { return partialSQLGraph{}, ErrNotImplemented } - return partialSQLGraph{}, nil + stmts := ownerDDLForAlter(ownershipTarget("VIEW", vd.new.SchemaQualifiedName), vd.old.Owner, vd.new.Owner) + if len(stmts) == 0 { + return partialSQLGraph{}, nil + } + return partialSQLGraph{ + vertices: []sqlVertex{{ + id: buildViewVertexId(vd.new.SchemaQualifiedName, diffTypeAddAlter), + priority: sqlPrioritySooner, + statements: stmts, + }}, + }, nil } func buildViewVertexId(n schema.SchemaQualifiedName, d diffType) sqlVertexId {