diff --git a/backup/metadata_globals_test.go b/backup/metadata_globals_test.go index 2bba402a4..d7b5fd0e3 100644 --- a/backup/metadata_globals_test.go +++ b/backup/metadata_globals_test.go @@ -50,9 +50,9 @@ var _ = Describe("backup/metadata_globals tests", func() { `CREATE DATABASE testdb TEMPLATE template0;`, `COMMENT ON DATABASE testdb IS 'This is a database comment.';`, `ALTER DATABASE testdb OWNER TO testrole;`, - `REVOKE ALL ON DATABASE testdb FROM PUBLIC; -REVOKE ALL ON DATABASE testdb FROM testrole; -GRANT TEMPORARY,CONNECT ON DATABASE testdb TO testrole;`, + `REVOKE ALL ON DATABASE testdb FROM PUBLIC;`, + `REVOKE ALL ON DATABASE testdb FROM testrole;`, + `GRANT TEMPORARY,CONNECT ON DATABASE testdb TO testrole;`, `SECURITY LABEL FOR dummy ON DATABASE testdb IS 'unclassified';`} testutils.AssertBufferContents(tocfile.GlobalEntries, buffer, expectedStatements...) }) @@ -428,9 +428,9 @@ ALTER ROLE "testRole2" WITH SUPERUSER INHERIT CREATEROLE CREATEDB LOGIN REPLICAT `CREATE TABLESPACE test_tablespace FILESPACE test_filespace;`, `COMMENT ON TABLESPACE test_tablespace IS 'This is a tablespace comment.';`, `ALTER TABLESPACE test_tablespace OWNER TO testrole;`, - `REVOKE ALL ON TABLESPACE test_tablespace FROM PUBLIC; -REVOKE ALL ON TABLESPACE test_tablespace FROM testrole; -GRANT ALL ON TABLESPACE test_tablespace TO testrole;`, + `REVOKE ALL ON TABLESPACE test_tablespace FROM PUBLIC;`, + `REVOKE ALL ON TABLESPACE test_tablespace FROM testrole;`, + `GRANT ALL ON TABLESPACE test_tablespace TO testrole;`, `SECURITY LABEL FOR dummy ON TABLESPACE test_tablespace IS 'unclassified';`} testutils.AssertBufferContents(tocfile.GlobalEntries, buffer, expectedStatements...) diff --git a/backup/predata_acl.go b/backup/predata_acl.go index 757695f60..3b0be7253 100644 --- a/backup/predata_acl.go +++ b/backup/predata_acl.go @@ -108,8 +108,8 @@ func PrintObjectMetadata(metadataFile *utils.FileWithByteCount, toc *toc.TOC, statements = append(statements, strings.TrimSpace(owner)) } } - if privileges := metadata.GetPrivilegesStatements(obj.FQN(), entry.ObjectType); privileges != "" { - statements = append(statements, strings.TrimSpace(privileges)) + if privileges := metadata.GetPrivilegesStatements(obj.FQN(), entry.ObjectType); len(privileges) > 0 { + statements = append(statements, privileges...); } if securityLabel := metadata.GetSecurityLabelStatement(obj.FQN(), entry.ObjectType); securityLabel != "" { statements = append(statements, strings.TrimSpace(securityLabel)) @@ -138,8 +138,8 @@ func printExtensionFunctionACLs(metadataFile *utils.FileWithByteCount, toc *toc. }) statements := make([]string, 0) for _, obj := range objects { - if privileges := obj.GetPrivilegesStatements(obj.FQN(), "FUNCTION"); privileges != "" { - statements = append(statements, strings.TrimSpace(privileges)) + if privileges := obj.GetPrivilegesStatements(obj.FQN(), "FUNCTION"); len(privileges) > 0 { + statements = append(statements, privileges...); PrintStatements(metadataFile, toc, obj, statements) } } @@ -290,7 +290,7 @@ func ParseACL(aclStr string) *ACL { return nil } -func (obj ObjectMetadata) GetPrivilegesStatements(objectName string, objectType string, columnName ...string) string { +func (obj ObjectMetadata) GetPrivilegesStatements(objectName string, objectType string, columnName ...string) []string { statements := make([]string, 0) typeStr := fmt.Sprintf("%s ", objectType) if objectType == "VIEW" || objectType == "FOREIGN TABLE" || objectType == "MATERIALIZED VIEW" { @@ -325,10 +325,7 @@ func (obj ObjectMetadata) GetPrivilegesStatements(objectName string, objectType } } } - if len(statements) > 0 { - return "\n\n" + strings.Join(statements, "\n") - } - return "" + return statements } func createPrivilegeStrings(acl ACL, objectType string) (string, string) { diff --git a/backup/predata_acl_test.go b/backup/predata_acl_test.go index 888ce083f..7f173602a 100644 --- a/backup/predata_acl_test.go +++ b/backup/predata_acl_test.go @@ -57,8 +57,14 @@ ALTER TABLE public.tablename OWNER TO testrole;`) testhelper.ExpectRegexp(buffer, ` REVOKE ALL ON TABLE public.tablename FROM PUBLIC; + + GRANT ALL ON TABLE public.tablename TO anothertestrole; + + GRANT SELECT,INSERT,UPDATE,DELETE,TRUNCATE,REFERENCES ON TABLE public.tablename TO testrole; + + GRANT TRIGGER ON TABLE public.tablename TO PUBLIC;`) }) It("prints a block of REVOKE and GRANT statements WITH GRANT OPTION", func() { @@ -67,8 +73,14 @@ GRANT TRIGGER ON TABLE public.tablename TO PUBLIC;`) testhelper.ExpectRegexp(buffer, ` REVOKE ALL ON TABLE public.tablename FROM PUBLIC; + + GRANT ALL ON TABLE public.tablename TO anothertestrole WITH GRANT OPTION; + + GRANT SELECT,INSERT,UPDATE,DELETE,TRUNCATE,REFERENCES ON TABLE public.tablename TO testrole WITH GRANT OPTION; + + GRANT TRIGGER ON TABLE public.tablename TO PUBLIC WITH GRANT OPTION;`) }) It("prints a block of REVOKE and GRANT statements, some with WITH GRANT OPTION, some without", func() { @@ -77,7 +89,11 @@ GRANT TRIGGER ON TABLE public.tablename TO PUBLIC WITH GRANT OPTION;`) testhelper.ExpectRegexp(buffer, ` REVOKE ALL ON TABLE public.tablename FROM PUBLIC; + + GRANT ALL ON TABLE public.tablename TO anothertestrole; + + GRANT SELECT,INSERT,UPDATE,DELETE,TRUNCATE,REFERENCES ON TABLE public.tablename TO testrole WITH GRANT OPTION;`) }) It("prints both an ALTER TABLE ... OWNER TO statement and a table comment", func() { @@ -99,9 +115,17 @@ ALTER TABLE public.tablename OWNER TO testrole; REVOKE ALL ON TABLE public.tablename FROM PUBLIC; + + REVOKE ALL ON TABLE public.tablename FROM testrole; + + GRANT ALL ON TABLE public.tablename TO anothertestrole; + + GRANT SELECT,INSERT,UPDATE,DELETE,TRUNCATE,REFERENCES ON TABLE public.tablename TO testrole; + + GRANT TRIGGER ON TABLE public.tablename TO PUBLIC;`) }) It("prints both a block of REVOKE and GRANT statements and a table comment", func() { @@ -113,8 +137,14 @@ COMMENT ON TABLE public.tablename IS 'This is a table comment.'; REVOKE ALL ON TABLE public.tablename FROM PUBLIC; + + GRANT ALL ON TABLE public.tablename TO anothertestrole; + + GRANT SELECT,INSERT,UPDATE,DELETE,TRUNCATE,REFERENCES ON TABLE public.tablename TO testrole; + + GRANT TRIGGER ON TABLE public.tablename TO PUBLIC;`) }) It("prints REVOKE and GRANT statements, an ALTER TABLE ... OWNER TO statement, and comments", func() { @@ -129,9 +159,17 @@ ALTER TABLE public.tablename OWNER TO testrole; REVOKE ALL ON TABLE public.tablename FROM PUBLIC; + + REVOKE ALL ON TABLE public.tablename FROM testrole; + + GRANT ALL ON TABLE public.tablename TO anothertestrole; + + GRANT SELECT,INSERT,UPDATE,DELETE,TRUNCATE,REFERENCES ON TABLE public.tablename TO testrole; + + GRANT TRIGGER ON TABLE public.tablename TO PUBLIC;`) }) It("prints SERVER for ALTER and FOREIGN SERVER for GRANT/REVOKE for a foreign server", func() { @@ -145,7 +183,11 @@ ALTER SERVER foreignserver OWNER TO testrole; REVOKE ALL ON FOREIGN SERVER foreignserver FROM PUBLIC; + + REVOKE ALL ON FOREIGN SERVER foreignserver FROM testrole; + + GRANT ALL ON FOREIGN SERVER foreignserver TO testrole;`) }) It("prints FUNCTION for REVOKE and AGGREGATE for ALTER for an aggregate function", func() { @@ -159,6 +201,8 @@ ALTER AGGREGATE public.testagg(*) OWNER TO testrole; REVOKE ALL ON FUNCTION public.testagg(*) FROM PUBLIC; + + REVOKE ALL ON FUNCTION public.testagg(*) FROM testrole;`) }) Context("Views and sequences have owners", func() { diff --git a/backup/predata_externals_test.go b/backup/predata_externals_test.go index 9dc0f19fa..dd4c0de5b 100644 --- a/backup/predata_externals_test.go +++ b/backup/predata_externals_test.go @@ -491,9 +491,9 @@ SEGMENT REJECT LIMIT 2 ROWS`) expectedStatements := []string{ "CREATE PROTOCOL s3 (readfunc = public.read_fn_s3, writefunc = public.write_fn_s3);", "ALTER PROTOCOL s3 OWNER TO testrole;", - `REVOKE ALL ON PROTOCOL s3 FROM PUBLIC; -REVOKE ALL ON PROTOCOL s3 FROM testrole; -GRANT ALL ON PROTOCOL s3 TO testrole;`} + `REVOKE ALL ON PROTOCOL s3 FROM PUBLIC;`, + `REVOKE ALL ON PROTOCOL s3 FROM testrole;`, + `GRANT ALL ON PROTOCOL s3 TO testrole;`} testutils.AssertBufferContents(tocfile.PredataEntries, buffer, expectedStatements...) }) It("prints a protocol ACL even when the protocol's CREATE statement is skipped", func() { @@ -511,9 +511,9 @@ GRANT ALL ON PROTOCOL s3 TO testrole;`} backup.PrintCreateExternalProtocolStatement(backupfile, tocfile, protocolUntrustedReadWrite, pgCatalogFuncInfoMap, protoMetadata) expectedStatements := []string{ "ALTER PROTOCOL s3 OWNER TO testrole;", - `REVOKE ALL ON PROTOCOL s3 FROM PUBLIC; -REVOKE ALL ON PROTOCOL s3 FROM testrole; -GRANT ALL ON PROTOCOL s3 TO testrole;`} + `REVOKE ALL ON PROTOCOL s3 FROM PUBLIC;`, + `REVOKE ALL ON PROTOCOL s3 FROM testrole;`, + `GRANT ALL ON PROTOCOL s3 TO testrole;`} testutils.AssertBufferContents(tocfile.PredataEntries, buffer, expectedStatements...) }) }) diff --git a/backup/predata_functions_test.go b/backup/predata_functions_test.go index 375d3223c..dbfeee39e 100644 --- a/backup/predata_functions_test.go +++ b/backup/predata_functions_test.go @@ -61,9 +61,9 @@ $$add_two_ints$$ LANGUAGE internal%s;`, DEFAULT_PARALLEL), "COMMENT ON FUNCTION public.func_name(integer, integer) IS 'This is a function comment.';", "ALTER FUNCTION public.func_name(integer, integer) OWNER TO testrole;", - `REVOKE ALL ON FUNCTION public.func_name(integer, integer) FROM PUBLIC; -REVOKE ALL ON FUNCTION public.func_name(integer, integer) FROM testrole; -GRANT ALL ON FUNCTION public.func_name(integer, integer) TO testrole;`, + `REVOKE ALL ON FUNCTION public.func_name(integer, integer) FROM PUBLIC;`, + `REVOKE ALL ON FUNCTION public.func_name(integer, integer) FROM testrole;`, + `GRANT ALL ON FUNCTION public.func_name(integer, integer) TO testrole;`, "SECURITY LABEL FOR dummy ON FUNCTION public.func_name(integer, integer) IS 'unclassified';"} testutils.AssertBufferContents(tocfile.PredataEntries, buffer, expectedStatements...) @@ -825,9 +825,9 @@ ALTER FUNCTION pg_catalog.plperl_validator(oid) OWNER TO testrole;`, // Languages have implicit owners in 4.3, but do not support ALTER OWNER expectedStatements = append(expectedStatements, "ALTER LANGUAGE plpythonu OWNER TO testrole;") } - expectedStatements = append(expectedStatements, `REVOKE ALL ON LANGUAGE plpythonu FROM PUBLIC; -REVOKE ALL ON LANGUAGE plpythonu FROM testrole; -GRANT ALL ON LANGUAGE plpythonu TO testrole;`, + expectedStatements = append(expectedStatements, `REVOKE ALL ON LANGUAGE plpythonu FROM PUBLIC;`, + `REVOKE ALL ON LANGUAGE plpythonu FROM testrole;`, + `GRANT ALL ON LANGUAGE plpythonu TO testrole;`, "SECURITY LABEL FOR dummy ON LANGUAGE plpythonu IS 'unclassified';") testutils.AssertBufferContents(tocfile.PredataEntries, buffer, expectedStatements...) @@ -853,9 +853,9 @@ GRANT ALL ON LANGUAGE plpythonu TO testrole;`, // Languages have implicit owners in 4.3, but do not support ALTER OWNER expectedStatements = append(expectedStatements, `ALTER LANGUAGE plperl OWNER TO testrole;`) } - expectedStatements = append(expectedStatements, `REVOKE ALL ON LANGUAGE plperl FROM PUBLIC; -REVOKE ALL ON LANGUAGE plperl FROM testrole; -GRANT ALL ON LANGUAGE plperl TO testrole;`, + expectedStatements = append(expectedStatements, `REVOKE ALL ON LANGUAGE plperl FROM PUBLIC;`, + `REVOKE ALL ON LANGUAGE plperl FROM testrole;`, + `GRANT ALL ON LANGUAGE plperl TO testrole;`, "SECURITY LABEL FOR dummy ON LANGUAGE plperl IS 'unclassified';") testutils.AssertBufferContents(tocfile.PredataEntries, buffer, expectedStatements...) diff --git a/backup/predata_relations.go b/backup/predata_relations.go index 75beadb87..c296f8765 100644 --- a/backup/predata_relations.go +++ b/backup/predata_relations.go @@ -254,7 +254,7 @@ func PrintPostCreateTableStatements(metadataFile *utils.FileWithByteCount, toc * if att.Privileges.Valid { columnMetadata := ObjectMetadata{Privileges: getColumnACL(att.Privileges, att.Kind), Owner: tableMetadata.Owner} columnPrivileges := columnMetadata.GetPrivilegesStatements(table.FQN(), "COLUMN", att.Name) - statements = append(statements, strings.TrimSpace(columnPrivileges)) + statements = append(statements, columnPrivileges...) } if att.SecurityLabel != "" { escapedLabel := utils.EscapeSingleQuotes(att.SecurityLabel) diff --git a/backup/predata_relations_other_test.go b/backup/predata_relations_other_test.go index 133c407c4..71df28afe 100644 --- a/backup/predata_relations_other_test.go +++ b/backup/predata_relations_other_test.go @@ -217,9 +217,9 @@ SELECT pg_catalog.setval('public.seq_''name', 7, true);`, getSeqDefReplace())) SELECT pg_catalog.setval('public.seq_name', 7, true);`, getSeqDefReplace()), "COMMENT ON SEQUENCE public.seq_name IS 'This is a sequence comment.';", fmt.Sprintf("ALTER %s public.seq_name OWNER TO testrole;", keywordReplace), - `REVOKE ALL ON SEQUENCE public.seq_name FROM PUBLIC; -REVOKE ALL ON SEQUENCE public.seq_name FROM testrole; -GRANT SELECT,USAGE ON SEQUENCE public.seq_name TO testrole;`} + `REVOKE ALL ON SEQUENCE public.seq_name FROM PUBLIC;`, + `REVOKE ALL ON SEQUENCE public.seq_name FROM testrole;`, + `GRANT SELECT,USAGE ON SEQUENCE public.seq_name TO testrole;`} testutils.AssertBufferContents(tocfile.PredataEntries, buffer, expectedEntries...) }) It("can print a sequence with privileges WITH GRANT OPTION", func() { @@ -235,8 +235,8 @@ GRANT SELECT,USAGE ON SEQUENCE public.seq_name TO testrole;`} CACHE 5; SELECT pg_catalog.setval('public.seq_name', 7, true);`, getSeqDefReplace()), - `REVOKE ALL ON SEQUENCE public.seq_name FROM PUBLIC; -GRANT SELECT,USAGE ON SEQUENCE public.seq_name TO testrole WITH GRANT OPTION;`) + `REVOKE ALL ON SEQUENCE public.seq_name FROM PUBLIC;`, + `GRANT SELECT,USAGE ON SEQUENCE public.seq_name TO testrole WITH GRANT OPTION;`) }) It("prints data_type of the sequence", func() { if connectionPool.Version.Before("7") { @@ -311,9 +311,9 @@ SELECT pg_catalog.setval('public.seq_name', 10, true);`, getSeqDefReplace()), expectedEntries := []string{"CREATE VIEW shamwow.shazam AS SELECT count(*) FROM pg_tables;", "COMMENT ON VIEW shamwow.shazam IS 'This is a view comment.';", fmt.Sprintf("ALTER %s shamwow.shazam OWNER TO testrole;", keywordReplace), - `REVOKE ALL ON shamwow.shazam FROM PUBLIC; -REVOKE ALL ON shamwow.shazam FROM testrole; -GRANT ALL ON shamwow.shazam TO testrole;`} + `REVOKE ALL ON shamwow.shazam FROM PUBLIC;`, + `REVOKE ALL ON shamwow.shazam FROM testrole;`, + `GRANT ALL ON shamwow.shazam TO testrole;`} if connectionPool.Version.AtLeast("6") { expectedEntries = append(expectedEntries, "SECURITY LABEL FOR dummy ON VIEW shamwow.shazam IS 'unclassified';") diff --git a/backup/predata_relations_tables_test.go b/backup/predata_relations_tables_test.go index 0bed7f11d..9d1f111f3 100644 --- a/backup/predata_relations_tables_test.go +++ b/backup/predata_relations_tables_test.go @@ -617,7 +617,11 @@ ALTER FOREIGN TABLE public.tablename OWNER TO testrole; REVOKE ALL ON public.tablename FROM PUBLIC; + + REVOKE ALL ON public.tablename FROM testrole; + + GRANT ALL ON public.tablename TO testrole; @@ -654,12 +658,20 @@ ALTER TABLE public.tablename OWNER TO testrole; REVOKE ALL (i) ON TABLE public.tablename FROM PUBLIC; + + REVOKE ALL (i) ON TABLE public.tablename FROM testrole; + + GRANT SELECT (i) ON TABLE public.tablename TO testrole; REVOKE ALL (j) ON TABLE public.tablename FROM PUBLIC; + + REVOKE ALL (j) ON TABLE public.tablename FROM testrole; + + GRANT ALL (j) ON TABLE public.tablename TO testrole2;`) }) It("prints a security group statement on a table column", func() { diff --git a/backup/predata_shared_test.go b/backup/predata_shared_test.go index 91df019f8..75dff3b96 100644 --- a/backup/predata_shared_test.go +++ b/backup/predata_shared_test.go @@ -156,9 +156,9 @@ var _ = Describe("backup/predata_shared tests", func() { expectedStatements := []string{"CREATE SCHEMA schemaname;", "COMMENT ON SCHEMA schemaname IS 'This is a schema comment.';", "ALTER SCHEMA schemaname OWNER TO testrole;", - `REVOKE ALL ON SCHEMA schemaname FROM PUBLIC; -REVOKE ALL ON SCHEMA schemaname FROM testrole; -GRANT ALL ON SCHEMA schemaname TO testrole;`, + `REVOKE ALL ON SCHEMA schemaname FROM PUBLIC;`, + `REVOKE ALL ON SCHEMA schemaname FROM testrole;`, + `GRANT ALL ON SCHEMA schemaname TO testrole;`, "SECURITY LABEL FOR dummy ON SCHEMA schemaname IS 'unclassified';"} testutils.AssertBufferContents(tocfile.PredataEntries, buffer, expectedStatements...) }) diff --git a/end_to_end/end_to_end_suite_test.go b/end_to_end/end_to_end_suite_test.go index 557e179e7..dc07acdda 100644 --- a/end_to_end/end_to_end_suite_test.go +++ b/end_to_end/end_to_end_suite_test.go @@ -1046,6 +1046,44 @@ var _ = Describe("backup and restore end to end tests", func() { assertRelationsCreatedInSchema(restoreConn, "schema2", 0) assertRelationsCreatedInSchema(restoreConn, "fooschema", 0) }) + It("runs gprestore with --redirect-schema restoring all privileges to the new schema", func() { + skipIfOldBackupVersionBefore("1.17.0") + testhelper.AssertQueryRuns(backupConn, + "DROP ROLE IF EXISTS test_user; CREATE ROLE test_user;") + defer testhelper.AssertQueryRuns(backupConn, + "DROP ROLE test_user") + testhelper.AssertQueryRuns(restoreConn, + "DROP SCHEMA IF EXISTS schema_to CASCADE; CREATE SCHEMA schema_to;") + defer testhelper.AssertQueryRuns(restoreConn, + "DROP SCHEMA schema_to CASCADE") + testhelper.AssertQueryRuns(backupConn, + "DROP SCHEMA IF EXISTS schema_from CASCADE; CREATE SCHEMA schema_from;") + defer testhelper.AssertQueryRuns(backupConn, + "DROP SCHEMA schema_from CASCADE") + testhelper.AssertQueryRuns(backupConn, + "CREATE TABLE schema_from.test_table(i int)") + testhelper.AssertQueryRuns(backupConn, + "GRANT SELECT ON schema_from.test_table TO test_user") + + timestamp := gpbackup(gpbackupPath, backupHelperPath, + "--include-schema", "schema_from") + + gprestore(gprestorePath, restoreHelperPath, timestamp, + "--redirect-db", "restoredb", + "--include-table", "schema_from.test_table", + "--redirect-schema", "schema_to") + + assertRelationsCreatedInSchema(restoreConn, "schema_to", 1) + + // The resulting grantee count is 2 (owner and 'test_user'). + // '--redirect-schema' should cause all privilege statements to be + // edited with new schema name. Restoring should not cause an error + // of not existed schema. + privCount := dbconn.MustSelectString(restoreConn, + `select count(distinct grantee) from information_schema.table_privileges` + + ` where table_schema = 'schema_to' and table_name = 'test_table'`) + Expect(privCount).To(Equal("2")) + }) }) Describe("ACLs for extensions", func() { It("runs gpbackup and gprestores any user defined ACLs on extensions", func() {