From 3a3dcad3f8b468981c04c38664cf764e1bc43aff Mon Sep 17 00:00:00 2001 From: Matthias Fuhrmeister Date: Thu, 26 Oct 2023 11:31:26 +0200 Subject: [PATCH 1/4] Allow writer user to create tables in schema In postgres 15 they change the behaviour of the public schema. Now only the owner can create tables in this schema. And the user is in charge to configure the permissions. Grant the writer user to also create tables in a schema. add the public schema explicitly to the list of schemas to create, to force the schema privileges to be applied. --- pkg/controller/postgres/postgres_controller.go | 5 +++++ .../postgres/postgres_controller_test.go | 5 +++++ pkg/postgres/database.go | 16 ++++++++++++++++ pkg/postgres/mock/postgres.go | 14 ++++++++++++++ pkg/postgres/postgres.go | 1 + 5 files changed, 41 insertions(+) diff --git a/pkg/controller/postgres/postgres_controller.go b/pkg/controller/postgres/postgres_controller.go index 35641b3b..8fda12a5 100644 --- a/pkg/controller/postgres/postgres_controller.go +++ b/pkg/controller/postgres/postgres_controller.go @@ -230,6 +230,11 @@ func (r *ReconcilePostgres) Reconcile(request reconcile.Request) (_ reconcile.Re reqLogger.Error(err, fmt.Sprintf("Could not give %s permissions \"%s\"", writer, writerPrivs)) continue } + err = r.pg.SetSchemaPrivilegesCreate(database, owner, writer, schema, writerPrivs, reqLogger) + if err != nil { + reqLogger.Error(err, fmt.Sprintf("Could not give %s permissions \"%s\"", writer, writerPrivs)) + continue + } instance.Status.Schemas = append(instance.Status.Schemas, schema) } diff --git a/pkg/controller/postgres/postgres_controller_test.go b/pkg/controller/postgres/postgres_controller_test.go index efe900ea..685aae41 100644 --- a/pkg/controller/postgres/postgres_controller_test.go +++ b/pkg/controller/postgres/postgres_controller_test.go @@ -683,9 +683,11 @@ var _ = Describe("ReconcilePostgres", func() { // customers schema pg.EXPECT().CreateSchema(name, name+"-group", "customers", gomock.Any()).Return(nil).Times(1) pg.EXPECT().SetSchemaPrivileges(name, name+"-group", gomock.Any(), "customers", gomock.Any(), gomock.Any()).Return(nil).Times(2) + pg.EXPECT().SetSchemaPrivilegesCreate(name, name+"-group", name+"-writer", "customers", gomock.Any(), gomock.Any()).Return(nil).Times(1) // stores schema pg.EXPECT().CreateSchema(name, name+"-group", "stores", gomock.Any()).Return(nil).Times(1) pg.EXPECT().SetSchemaPrivileges(name, name+"-group", gomock.Any(), "stores", gomock.Any(), gomock.Any()).Return(nil).Times(2) + pg.EXPECT().SetSchemaPrivilegesCreate(name, name+"-group", name+"-writer", "stores", gomock.Any(), gomock.Any()).Return(nil).Times(1) }) It("should update status", func() { @@ -708,9 +710,11 @@ var _ = Describe("ReconcilePostgres", func() { // customers schema errors pg.EXPECT().CreateSchema(name, name+"-group", "customers", gomock.Any()).Return(fmt.Errorf("Could not create schema")).Times(1) pg.EXPECT().SetSchemaPrivileges(name, name+"-group", gomock.Any(), "customers", gomock.Any(), gomock.Any()).Return(nil).Times(0) + pg.EXPECT().SetSchemaPrivilegesCreate(name, name+"-group", name+"-writer", "customers", gomock.Any(), gomock.Any()).Return(nil).Times(0) // stores schema pg.EXPECT().CreateSchema(name, name+"-group", "stores", gomock.Any()).Return(nil).Times(1) pg.EXPECT().SetSchemaPrivileges(name, name+"-group", gomock.Any(), "stores", gomock.Any(), gomock.Any()).Return(nil).Times(2) + pg.EXPECT().SetSchemaPrivilegesCreate(name, name+"-group", name+"-writer", "stores", gomock.Any(), gomock.Any()).Return(nil).Times(1) }) It("should update status", func() { @@ -752,6 +756,7 @@ var _ = Describe("ReconcilePostgres", func() { // customers schema pg.EXPECT().CreateSchema(name, name+"-group", "customers", gomock.Any()).Return(nil).Times(1) pg.EXPECT().SetSchemaPrivileges(name, name+"-group", gomock.Any(), "customers", gomock.Any(), gomock.Any()).Return(nil).Times(2) + pg.EXPECT().SetSchemaPrivilegesCreate(name, name+"-group", name+"-writer", "customers", gomock.Any(), gomock.Any()).Return(nil).Times(1) // stores schema already exists pg.EXPECT().CreateSchema(name, name+"-group", "stores", gomock.Any()).Times(0) // Call reconcile diff --git a/pkg/postgres/database.go b/pkg/postgres/database.go index b01d59fb..da198605 100644 --- a/pkg/postgres/database.go +++ b/pkg/postgres/database.go @@ -14,6 +14,7 @@ const ( ALTER_DB_OWNER = `ALTER DATABASE "%s" OWNER TO "%s"` DROP_DATABASE = `DROP DATABASE "%s"` GRANT_USAGE_SCHEMA = `GRANT USAGE ON SCHEMA "%s" TO "%s"` + GRANT_CREATE_TABLE = `GRANT CREATE ON SCHEMA "%s" TO "%s"` GRANT_ALL_TABLES = `GRANT %s ON ALL TABLES IN SCHEMA "%s" TO "%s"` DEFAULT_PRIVS_SCHEMA = `ALTER DEFAULT PRIVILEGES FOR ROLE "%s" IN SCHEMA "%s" GRANT %s ON TABLES TO "%s"` REVOKE_CONNECT = `REVOKE CONNECT ON DATABASE "%s" FROM public` @@ -120,3 +121,18 @@ func (c *pg) SetSchemaPrivileges(db, creator, role, schema, privs string, logger } return nil } + +func (c *pg) SetSchemaPrivilegesCreate(db, creator, role, schema, privs string, logger logr.Logger) error { + tmpDb, err := GetConnection(c.user, c.pass, c.host, db, c.args, logger) + if err != nil { + return err + } + defer tmpDb.Close() + + // Grant role usage on schema + _, err = tmpDb.Exec(fmt.Sprintf(GRANT_CREATE_TABLE, schema, role)) + if err != nil { + return err + } + return nil +} diff --git a/pkg/postgres/mock/postgres.go b/pkg/postgres/mock/postgres.go index 6cd73efb..47386eab 100644 --- a/pkg/postgres/mock/postgres.go +++ b/pkg/postgres/mock/postgres.go @@ -146,6 +146,20 @@ func (mr *MockPGMockRecorder) SetSchemaPrivileges(db, creator, role, schema, pri return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetSchemaPrivileges", reflect.TypeOf((*MockPG)(nil).SetSchemaPrivileges), db, creator, role, schema, privs, logger) } +// SetSchemaPrivilegesCreate mocks base method +func (m *MockPG) SetSchemaPrivilegesCreate(db, creator, role, schema, privs string, logger logr.Logger) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "SetSchemaPrivilegesCreate", db, creator, role, schema, privs, logger) + ret0, _ := ret[0].(error) + return ret0 +} + +// SetSchemaPrivilegesCreate indicates an expected call of SetSchemaPrivilegesCreate +func (mr *MockPGMockRecorder) SetSchemaPrivilegesCreate(db, creator, role, schema, privs, logger interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetSchemaPrivilegesCreate", reflect.TypeOf((*MockPG)(nil).SetSchemaPrivilegesCreate), db, creator, role, schema, privs, logger) +} + // RevokeRole mocks base method func (m *MockPG) RevokeRole(role, revoked string) error { m.ctrl.T.Helper() diff --git a/pkg/postgres/postgres.go b/pkg/postgres/postgres.go index 22f8f56e..f8fa0f88 100644 --- a/pkg/postgres/postgres.go +++ b/pkg/postgres/postgres.go @@ -17,6 +17,7 @@ type PG interface { UpdatePassword(role, password string) error GrantRole(role, grantee string) error SetSchemaPrivileges(db, creator, role, schema, privs string, logger logr.Logger) error + SetSchemaPrivilegesCreate(db, creator, role, schema, privs string, logger logr.Logger) error RevokeRole(role, revoked string) error AlterDefaultLoginRole(role, setRole string) error DropDatabase(db string, logger logr.Logger) error From 024afb26ee0cfa4e671545ecf044f8a8bd7d21dc Mon Sep 17 00:00:00 2001 From: Matthias Fuhrmeister Date: Tue, 13 Feb 2024 14:57:31 +0100 Subject: [PATCH 2/4] integrate schema create permission into exisiting method --- .../postgres/postgres_controller.go | 9 ++------ .../postgres/postgres_controller_test.go | 16 +++++--------- pkg/postgres/database.go | 20 ++++++----------- pkg/postgres/mock/postgres.go | 22 ++++--------------- pkg/postgres/postgres.go | 3 +-- 5 files changed, 20 insertions(+), 50 deletions(-) diff --git a/pkg/controller/postgres/postgres_controller.go b/pkg/controller/postgres/postgres_controller.go index 8fda12a5..1d19dceb 100644 --- a/pkg/controller/postgres/postgres_controller.go +++ b/pkg/controller/postgres/postgres_controller.go @@ -220,17 +220,12 @@ func (r *ReconcilePostgres) Reconcile(request reconcile.Request) (_ reconcile.Re } // Set privileges on schema - err = r.pg.SetSchemaPrivileges(database, owner, reader, schema, readerPrivs, reqLogger) + err = r.pg.SetSchemaPrivileges(database, owner, reader, schema, readerPrivs, false, reqLogger) if err != nil { reqLogger.Error(err, fmt.Sprintf("Could not give %s permissions \"%s\"", reader, readerPrivs)) continue } - err = r.pg.SetSchemaPrivileges(database, owner, writer, schema, writerPrivs, reqLogger) - if err != nil { - reqLogger.Error(err, fmt.Sprintf("Could not give %s permissions \"%s\"", writer, writerPrivs)) - continue - } - err = r.pg.SetSchemaPrivilegesCreate(database, owner, writer, schema, writerPrivs, reqLogger) + err = r.pg.SetSchemaPrivileges(database, owner, writer, schema, writerPrivs, true, reqLogger) if err != nil { reqLogger.Error(err, fmt.Sprintf("Could not give %s permissions \"%s\"", writer, writerPrivs)) continue diff --git a/pkg/controller/postgres/postgres_controller_test.go b/pkg/controller/postgres/postgres_controller_test.go index 685aae41..97bb74a9 100644 --- a/pkg/controller/postgres/postgres_controller_test.go +++ b/pkg/controller/postgres/postgres_controller_test.go @@ -682,12 +682,10 @@ var _ = Describe("ReconcilePostgres", func() { // Expected method calls // customers schema pg.EXPECT().CreateSchema(name, name+"-group", "customers", gomock.Any()).Return(nil).Times(1) - pg.EXPECT().SetSchemaPrivileges(name, name+"-group", gomock.Any(), "customers", gomock.Any(), gomock.Any()).Return(nil).Times(2) - pg.EXPECT().SetSchemaPrivilegesCreate(name, name+"-group", name+"-writer", "customers", gomock.Any(), gomock.Any()).Return(nil).Times(1) + pg.EXPECT().SetSchemaPrivileges(name, name+"-group", gomock.Any(), "customers", gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).Times(2) // stores schema pg.EXPECT().CreateSchema(name, name+"-group", "stores", gomock.Any()).Return(nil).Times(1) - pg.EXPECT().SetSchemaPrivileges(name, name+"-group", gomock.Any(), "stores", gomock.Any(), gomock.Any()).Return(nil).Times(2) - pg.EXPECT().SetSchemaPrivilegesCreate(name, name+"-group", name+"-writer", "stores", gomock.Any(), gomock.Any()).Return(nil).Times(1) + pg.EXPECT().SetSchemaPrivileges(name, name+"-group", gomock.Any(), "stores", gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).Times(2) }) It("should update status", func() { @@ -709,12 +707,11 @@ var _ = Describe("ReconcilePostgres", func() { // Expected method calls // customers schema errors pg.EXPECT().CreateSchema(name, name+"-group", "customers", gomock.Any()).Return(fmt.Errorf("Could not create schema")).Times(1) - pg.EXPECT().SetSchemaPrivileges(name, name+"-group", gomock.Any(), "customers", gomock.Any(), gomock.Any()).Return(nil).Times(0) - pg.EXPECT().SetSchemaPrivilegesCreate(name, name+"-group", name+"-writer", "customers", gomock.Any(), gomock.Any()).Return(nil).Times(0) + pg.EXPECT().SetSchemaPrivileges(name, name+"-group", gomock.Any(), "customers", gomock.Any(), gomock.Any() ,gomock.Any()).Return(nil).Times(0) // stores schema pg.EXPECT().CreateSchema(name, name+"-group", "stores", gomock.Any()).Return(nil).Times(1) - pg.EXPECT().SetSchemaPrivileges(name, name+"-group", gomock.Any(), "stores", gomock.Any(), gomock.Any()).Return(nil).Times(2) - pg.EXPECT().SetSchemaPrivilegesCreate(name, name+"-group", name+"-writer", "stores", gomock.Any(), gomock.Any()).Return(nil).Times(1) + pg.EXPECT().SetSchemaPrivileges(name, name+"-group", gomock.Any(), "stores", gomock.Any(), false, gomock.Any()).Return(nil).Times(1) + pg.EXPECT().SetSchemaPrivileges(name, name+"-group", gomock.Any(), "stores", gomock.Any(), true, gomock.Any()).Return(nil).Times(1) }) It("should update status", func() { @@ -755,8 +752,7 @@ var _ = Describe("ReconcilePostgres", func() { // Expected method calls // customers schema pg.EXPECT().CreateSchema(name, name+"-group", "customers", gomock.Any()).Return(nil).Times(1) - pg.EXPECT().SetSchemaPrivileges(name, name+"-group", gomock.Any(), "customers", gomock.Any(), gomock.Any()).Return(nil).Times(2) - pg.EXPECT().SetSchemaPrivilegesCreate(name, name+"-group", name+"-writer", "customers", gomock.Any(), gomock.Any()).Return(nil).Times(1) + pg.EXPECT().SetSchemaPrivileges(name, name+"-group", gomock.Any(), "customers", gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).Times(2) // stores schema already exists pg.EXPECT().CreateSchema(name, name+"-group", "stores", gomock.Any()).Times(0) // Call reconcile diff --git a/pkg/postgres/database.go b/pkg/postgres/database.go index da198605..4fc95bef 100644 --- a/pkg/postgres/database.go +++ b/pkg/postgres/database.go @@ -95,7 +95,7 @@ func (c *pg) CreateExtension(db, extension string, logger logr.Logger) error { return nil } -func (c *pg) SetSchemaPrivileges(db, creator, role, schema, privs string, logger logr.Logger) error { +func (c *pg) SetSchemaPrivileges(db, creator, role, schema, privs string, createSchema bool, logger logr.Logger) error { tmpDb, err := GetConnection(c.user, c.pass, c.host, db, c.args, logger) if err != nil { return err @@ -119,20 +119,14 @@ func (c *pg) SetSchemaPrivileges(db, creator, role, schema, privs string, logger if err != nil { return err } - return nil -} -func (c *pg) SetSchemaPrivilegesCreate(db, creator, role, schema, privs string, logger logr.Logger) error { - tmpDb, err := GetConnection(c.user, c.pass, c.host, db, c.args, logger) - if err != nil { - return err + // Grant role usage on schema if createSchema + if createSchema { + _, err = tmpDb.Exec(fmt.Sprintf(GRANT_CREATE_TABLE, schema, role)) + if err != nil { + return err + } } - defer tmpDb.Close() - // Grant role usage on schema - _, err = tmpDb.Exec(fmt.Sprintf(GRANT_CREATE_TABLE, schema, role)) - if err != nil { - return err - } return nil } diff --git a/pkg/postgres/mock/postgres.go b/pkg/postgres/mock/postgres.go index 47386eab..ce58f671 100644 --- a/pkg/postgres/mock/postgres.go +++ b/pkg/postgres/mock/postgres.go @@ -133,31 +133,17 @@ func (mr *MockPGMockRecorder) GrantRole(role, grantee interface{}) *gomock.Call } // SetSchemaPrivileges mocks base method -func (m *MockPG) SetSchemaPrivileges(db, creator, role, schema, privs string, logger logr.Logger) error { +func (m *MockPG) SetSchemaPrivileges(db, creator, role, schema, privs string, createSchema bool, logger logr.Logger) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "SetSchemaPrivileges", db, creator, role, schema, privs, logger) + ret := m.ctrl.Call(m, "SetSchemaPrivileges", db, creator, role, schema, privs, createSchema, logger) ret0, _ := ret[0].(error) return ret0 } // SetSchemaPrivileges indicates an expected call of SetSchemaPrivileges -func (mr *MockPGMockRecorder) SetSchemaPrivileges(db, creator, role, schema, privs, logger interface{}) *gomock.Call { +func (mr *MockPGMockRecorder) SetSchemaPrivileges(db, creator, role, schema, privs, createSchema, logger interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetSchemaPrivileges", reflect.TypeOf((*MockPG)(nil).SetSchemaPrivileges), db, creator, role, schema, privs, logger) -} - -// SetSchemaPrivilegesCreate mocks base method -func (m *MockPG) SetSchemaPrivilegesCreate(db, creator, role, schema, privs string, logger logr.Logger) error { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "SetSchemaPrivilegesCreate", db, creator, role, schema, privs, logger) - ret0, _ := ret[0].(error) - return ret0 -} - -// SetSchemaPrivilegesCreate indicates an expected call of SetSchemaPrivilegesCreate -func (mr *MockPGMockRecorder) SetSchemaPrivilegesCreate(db, creator, role, schema, privs, logger interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetSchemaPrivilegesCreate", reflect.TypeOf((*MockPG)(nil).SetSchemaPrivilegesCreate), db, creator, role, schema, privs, logger) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetSchemaPrivileges", reflect.TypeOf((*MockPG)(nil).SetSchemaPrivileges), db, creator, role, schema, privs, createSchema, logger) } // RevokeRole mocks base method diff --git a/pkg/postgres/postgres.go b/pkg/postgres/postgres.go index f8fa0f88..f596dd46 100644 --- a/pkg/postgres/postgres.go +++ b/pkg/postgres/postgres.go @@ -16,8 +16,7 @@ type PG interface { CreateUserRole(role, password string) (string, error) UpdatePassword(role, password string) error GrantRole(role, grantee string) error - SetSchemaPrivileges(db, creator, role, schema, privs string, logger logr.Logger) error - SetSchemaPrivilegesCreate(db, creator, role, schema, privs string, logger logr.Logger) error + SetSchemaPrivileges(db, creator, role, schema, privs string, createSchema bool, logger logr.Logger) error RevokeRole(role, revoked string) error AlterDefaultLoginRole(role, setRole string) error DropDatabase(db string, logger logr.Logger) error From 40e602264e58a99a7966d3decbc375dfe6cb5cef Mon Sep 17 00:00:00 2001 From: Matthias Fuhrmeister Date: Tue, 27 Feb 2024 14:48:17 +0100 Subject: [PATCH 3/4] use a struct for passing arguments --- pkg/controller/postgres/postgres_controller.go | 6 ++++-- pkg/postgres/database.go | 14 +++++++------- pkg/postgres/mock/postgres.go | 8 +++++--- pkg/postgres/postgres.go | 11 ++++++++++- 4 files changed, 26 insertions(+), 13 deletions(-) diff --git a/pkg/controller/postgres/postgres_controller.go b/pkg/controller/postgres/postgres_controller.go index 1d19dceb..56c75e03 100644 --- a/pkg/controller/postgres/postgres_controller.go +++ b/pkg/controller/postgres/postgres_controller.go @@ -220,12 +220,14 @@ func (r *ReconcilePostgres) Reconcile(request reconcile.Request) (_ reconcile.Re } // Set privileges on schema - err = r.pg.SetSchemaPrivileges(database, owner, reader, schema, readerPrivs, false, reqLogger) + schemaPrivilegesReader := postgres.PostgresSchemaPrivileges{database, owner, reader, schema, readerPrivs, false} + err = r.pg.SetSchemaPrivileges(schemaPrivilegesReader, reqLogger) if err != nil { reqLogger.Error(err, fmt.Sprintf("Could not give %s permissions \"%s\"", reader, readerPrivs)) continue } - err = r.pg.SetSchemaPrivileges(database, owner, writer, schema, writerPrivs, true, reqLogger) + schemaPrivilegesWriter := postgres.PostgresSchemaPrivileges{database, owner, reader, schema, readerPrivs, true} + err = r.pg.SetSchemaPrivileges(schemaPrivilegesWriter, reqLogger) if err != nil { reqLogger.Error(err, fmt.Sprintf("Could not give %s permissions \"%s\"", writer, writerPrivs)) continue diff --git a/pkg/postgres/database.go b/pkg/postgres/database.go index 4fc95bef..db856e7f 100644 --- a/pkg/postgres/database.go +++ b/pkg/postgres/database.go @@ -95,34 +95,34 @@ func (c *pg) CreateExtension(db, extension string, logger logr.Logger) error { return nil } -func (c *pg) SetSchemaPrivileges(db, creator, role, schema, privs string, createSchema bool, logger logr.Logger) error { - tmpDb, err := GetConnection(c.user, c.pass, c.host, db, c.args, logger) +func (c *pg) SetSchemaPrivileges(schemaPrivileges PostgresSchemaPrivileges, logger logr.Logger) error { + tmpDb, err := GetConnection(c.user, c.pass, c.host, schemaPrivileges.DB, c.args, logger) if err != nil { return err } defer tmpDb.Close() // Grant role usage on schema - _, err = tmpDb.Exec(fmt.Sprintf(GRANT_USAGE_SCHEMA, schema, role)) + _, err = tmpDb.Exec(fmt.Sprintf(GRANT_USAGE_SCHEMA, schemaPrivileges.Schema, schemaPrivileges.Role)) if err != nil { return err } // Grant role privs on existing tables in schema - _, err = tmpDb.Exec(fmt.Sprintf(GRANT_ALL_TABLES, privs, schema, role)) + _, err = tmpDb.Exec(fmt.Sprintf(GRANT_ALL_TABLES, schemaPrivileges.Privs, schemaPrivileges.Schema, schemaPrivileges.Role)) if err != nil { return err } // Grant role privs on future tables in schema - _, err = tmpDb.Exec(fmt.Sprintf(DEFAULT_PRIVS_SCHEMA, creator, schema, privs, role)) + _, err = tmpDb.Exec(fmt.Sprintf(DEFAULT_PRIVS_SCHEMA, schemaPrivileges.Creator, schemaPrivileges.Schema, schemaPrivileges.Privs, schemaPrivileges.Role)) if err != nil { return err } // Grant role usage on schema if createSchema - if createSchema { - _, err = tmpDb.Exec(fmt.Sprintf(GRANT_CREATE_TABLE, schema, role)) + if schemaPrivileges.CreateSchema { + _, err = tmpDb.Exec(fmt.Sprintf(GRANT_CREATE_TABLE, schemaPrivileges.Schema, schemaPrivileges.Role)) if err != nil { return err } diff --git a/pkg/postgres/mock/postgres.go b/pkg/postgres/mock/postgres.go index ce58f671..c14e541f 100644 --- a/pkg/postgres/mock/postgres.go +++ b/pkg/postgres/mock/postgres.go @@ -5,9 +5,11 @@ package mock import ( + reflect "reflect" + logr "github.com/go-logr/logr" gomock "github.com/golang/mock/gomock" - reflect "reflect" + "github.com/movetokube/postgres-operator/pkg/postgres" ) // MockPG is a mock of PG interface @@ -133,9 +135,9 @@ func (mr *MockPGMockRecorder) GrantRole(role, grantee interface{}) *gomock.Call } // SetSchemaPrivileges mocks base method -func (m *MockPG) SetSchemaPrivileges(db, creator, role, schema, privs string, createSchema bool, logger logr.Logger) error { +func (m *MockPG) SetSchemaPrivileges(privileges postgres.PostgresSchemaPrivileges, logger logr.Logger) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "SetSchemaPrivileges", db, creator, role, schema, privs, createSchema, logger) + ret := m.ctrl.Call(m, "SetSchemaPrivileges", privileges.DB, privileges.Creator, privileges.Role, privileges.Schema, privileges.Privs, privileges.CreateSchema, logger) ret0, _ := ret[0].(error) return ret0 } diff --git a/pkg/postgres/postgres.go b/pkg/postgres/postgres.go index f596dd46..a5c66b0a 100644 --- a/pkg/postgres/postgres.go +++ b/pkg/postgres/postgres.go @@ -16,7 +16,7 @@ type PG interface { CreateUserRole(role, password string) (string, error) UpdatePassword(role, password string) error GrantRole(role, grantee string) error - SetSchemaPrivileges(db, creator, role, schema, privs string, createSchema bool, logger logr.Logger) error + SetSchemaPrivileges(schemaPrivileges PostgresSchemaPrivileges, logger logr.Logger) error RevokeRole(role, revoked string) error AlterDefaultLoginRole(role, setRole string) error DropDatabase(db string, logger logr.Logger) error @@ -35,6 +35,15 @@ type pg struct { default_database string } +type PostgresSchemaPrivileges struct { + DB string + Creator string + Role string + Schema string + Privs string + CreateSchema bool +} + func NewPG(host, user, password, uri_args, default_database, cloud_type string, logger logr.Logger) (PG, error) { db, err := GetConnection(user, password, host, default_database, uri_args, logger) if err != nil { From 2c506be25b7569b433ef9bf9b1fd6a2a65a153f3 Mon Sep 17 00:00:00 2001 From: Matthias Fuhrmeister Date: Tue, 27 Feb 2024 14:52:18 +0100 Subject: [PATCH 4/4] Give owner also permission to create tables --- pkg/controller/postgres/postgres_controller.go | 8 +++++++- pkg/controller/postgres/postgres_controller_test.go | 11 ++++++----- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/pkg/controller/postgres/postgres_controller.go b/pkg/controller/postgres/postgres_controller.go index 56c75e03..37f0d8a6 100644 --- a/pkg/controller/postgres/postgres_controller.go +++ b/pkg/controller/postgres/postgres_controller.go @@ -226,12 +226,18 @@ func (r *ReconcilePostgres) Reconcile(request reconcile.Request) (_ reconcile.Re reqLogger.Error(err, fmt.Sprintf("Could not give %s permissions \"%s\"", reader, readerPrivs)) continue } - schemaPrivilegesWriter := postgres.PostgresSchemaPrivileges{database, owner, reader, schema, readerPrivs, true} + schemaPrivilegesWriter := postgres.PostgresSchemaPrivileges{database, owner, writer, schema, readerPrivs, true} err = r.pg.SetSchemaPrivileges(schemaPrivilegesWriter, reqLogger) if err != nil { reqLogger.Error(err, fmt.Sprintf("Could not give %s permissions \"%s\"", writer, writerPrivs)) continue } + schemaPrivilegesOwner := postgres.PostgresSchemaPrivileges{database, owner, owner, schema, readerPrivs, true} + err = r.pg.SetSchemaPrivileges(schemaPrivilegesOwner, reqLogger) + if err != nil { + reqLogger.Error(err, fmt.Sprintf("Could not give %s permissions \"%s\"", writer, writerPrivs)) + continue + } instance.Status.Schemas = append(instance.Status.Schemas, schema) } diff --git a/pkg/controller/postgres/postgres_controller_test.go b/pkg/controller/postgres/postgres_controller_test.go index 97bb74a9..4c45db3b 100644 --- a/pkg/controller/postgres/postgres_controller_test.go +++ b/pkg/controller/postgres/postgres_controller_test.go @@ -682,10 +682,10 @@ var _ = Describe("ReconcilePostgres", func() { // Expected method calls // customers schema pg.EXPECT().CreateSchema(name, name+"-group", "customers", gomock.Any()).Return(nil).Times(1) - pg.EXPECT().SetSchemaPrivileges(name, name+"-group", gomock.Any(), "customers", gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).Times(2) + pg.EXPECT().SetSchemaPrivileges(name, name+"-group", gomock.Any(), "customers", gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).Times(3) // stores schema pg.EXPECT().CreateSchema(name, name+"-group", "stores", gomock.Any()).Return(nil).Times(1) - pg.EXPECT().SetSchemaPrivileges(name, name+"-group", gomock.Any(), "stores", gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).Times(2) + pg.EXPECT().SetSchemaPrivileges(name, name+"-group", gomock.Any(), "stores", gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).Times(3) }) It("should update status", func() { @@ -710,8 +710,9 @@ var _ = Describe("ReconcilePostgres", func() { pg.EXPECT().SetSchemaPrivileges(name, name+"-group", gomock.Any(), "customers", gomock.Any(), gomock.Any() ,gomock.Any()).Return(nil).Times(0) // stores schema pg.EXPECT().CreateSchema(name, name+"-group", "stores", gomock.Any()).Return(nil).Times(1) - pg.EXPECT().SetSchemaPrivileges(name, name+"-group", gomock.Any(), "stores", gomock.Any(), false, gomock.Any()).Return(nil).Times(1) - pg.EXPECT().SetSchemaPrivileges(name, name+"-group", gomock.Any(), "stores", gomock.Any(), true, gomock.Any()).Return(nil).Times(1) + pg.EXPECT().SetSchemaPrivileges(name, name+"-group", name+"-reader", "stores", gomock.Any(), false, gomock.Any()).Return(nil).Times(1) + pg.EXPECT().SetSchemaPrivileges(name, name+"-group", name+"-writer", "stores", gomock.Any(), true, gomock.Any()).Return(nil).Times(1) + pg.EXPECT().SetSchemaPrivileges(name, name+"-group", name+"-group", "stores", gomock.Any(), true, gomock.Any()).Return(nil).Times(1) }) It("should update status", func() { @@ -752,7 +753,7 @@ var _ = Describe("ReconcilePostgres", func() { // Expected method calls // customers schema pg.EXPECT().CreateSchema(name, name+"-group", "customers", gomock.Any()).Return(nil).Times(1) - pg.EXPECT().SetSchemaPrivileges(name, name+"-group", gomock.Any(), "customers", gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).Times(2) + pg.EXPECT().SetSchemaPrivileges(name, name+"-group", gomock.Any(), "customers", gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).Times(3) // stores schema already exists pg.EXPECT().CreateSchema(name, name+"-group", "stores", gomock.Any()).Times(0) // Call reconcile