From 2f7e59c11823e664e734416809f350c32fe1502a Mon Sep 17 00:00:00 2001 From: Richard Inskip Date: Mon, 4 Dec 2023 15:47:51 +0000 Subject: [PATCH 1/9] ensure chart env values are strings --- charts/ext-postgres-operator/templates/operator.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/charts/ext-postgres-operator/templates/operator.yaml b/charts/ext-postgres-operator/templates/operator.yaml index 2c46ba34..fdcc2444 100644 --- a/charts/ext-postgres-operator/templates/operator.yaml +++ b/charts/ext-postgres-operator/templates/operator.yaml @@ -51,7 +51,7 @@ spec: value: {{ include "chart.fullname" . }} {{- range $key, $value := .Values.env }} - name: {{ $key }} - value: {{ $value }} + value: {{ $value | quote }} {{- end }} {{- if .Values.volumeMounts }} volumeMounts: From 34223b9d3c6a29d3eb8ced0d9cdfe866c30db207 Mon Sep 17 00:00:00 2001 From: Nicanor Gutierrez Date: Mon, 15 Jan 2024 15:23:12 +0000 Subject: [PATCH 2/9] Add secure string generator --- .../postgresuser/postgresuser_controller.go | 7 ++++++- pkg/utils/random.go | 15 +++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/pkg/controller/postgresuser/postgresuser_controller.go b/pkg/controller/postgresuser/postgresuser_controller.go index ff88927c..62b57489 100644 --- a/pkg/controller/postgresuser/postgresuser_controller.go +++ b/pkg/controller/postgresuser/postgresuser_controller.go @@ -162,7 +162,11 @@ func (r *ReconcilePostgresUser) Reconcile(request reconcile.Request) (reconcile. // Creation logic var role, login string - password := utils.GetRandomString(15) + password, err := utils.GetSecureRandomString(15) + + if err != nil { + return r.requeue(instance, err) + } if instance.Status.PostgresRole == "" { // We need to get the Postgres CR to get the group role name @@ -172,6 +176,7 @@ func (r *ReconcilePostgresUser) Reconcile(request reconcile.Request) (reconcile. } // Create user role suffix := utils.GetRandomString(6) + role = fmt.Sprintf("%s-%s", instance.Spec.Role, suffix) login, err = r.pg.CreateUserRole(role, password) if err != nil { diff --git a/pkg/utils/random.go b/pkg/utils/random.go index cd9d8e60..28583116 100644 --- a/pkg/utils/random.go +++ b/pkg/utils/random.go @@ -1,5 +1,6 @@ package utils +import cryptorand "crypto/rand" import "math/rand" var letterRunes = []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890") @@ -11,3 +12,17 @@ func GetRandomString(length int) string { } return string(b) } + +// If the secure random number generator malfunctions it will return an error +func GetSecureRandomString(length int) (string, error) { + b := make([]rune, length) + for i := 0; i < length; i++ { + num, err := cryptorand.Int(cryptorand.Reader, big.NewInt(int64(len(letterRunes)))) + if err != nil { + return "", err + } + b[i] = letterRunes[num.Int64()] + } + + return string(b), nil +} \ No newline at end of file From 393abd70cfea52326cd55fc2920416c8172ebb82 Mon Sep 17 00:00:00 2001 From: Nicanor Gutierrez Date: Mon, 15 Jan 2024 15:25:14 +0000 Subject: [PATCH 3/9] Remove extra line --- pkg/controller/postgresuser/postgresuser_controller.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/controller/postgresuser/postgresuser_controller.go b/pkg/controller/postgresuser/postgresuser_controller.go index 62b57489..85a9437f 100644 --- a/pkg/controller/postgresuser/postgresuser_controller.go +++ b/pkg/controller/postgresuser/postgresuser_controller.go @@ -176,7 +176,6 @@ func (r *ReconcilePostgresUser) Reconcile(request reconcile.Request) (reconcile. } // Create user role suffix := utils.GetRandomString(6) - role = fmt.Sprintf("%s-%s", instance.Spec.Role, suffix) login, err = r.pg.CreateUserRole(role, password) if err != nil { From a75bc16d73137fc14283ec04ef4e3ec34dc6ebbe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicanor=20Guti=C3=A9rrez=20Requejo?= Date: Mon, 26 Feb 2024 10:27:42 +0000 Subject: [PATCH 4/9] Add missing import of math/big --- pkg/utils/random.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/utils/random.go b/pkg/utils/random.go index 28583116..77aebbeb 100644 --- a/pkg/utils/random.go +++ b/pkg/utils/random.go @@ -2,6 +2,7 @@ package utils import cryptorand "crypto/rand" import "math/rand" +import "math/big" var letterRunes = []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890") From 3a3dcad3f8b468981c04c38664cf764e1bc43aff Mon Sep 17 00:00:00 2001 From: Matthias Fuhrmeister Date: Thu, 26 Oct 2023 11:31:26 +0200 Subject: [PATCH 5/9] 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 6/9] 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 7/9] 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 8/9] 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 From 72961eacd45ceb6201829ff3ddf4f1dfd0e1ed56 Mon Sep 17 00:00:00 2001 From: Tomas Date: Thu, 29 Feb 2024 13:03:23 +0200 Subject: [PATCH 9/9] Bump helm chart to 1.2.6 --- charts/ext-postgres-operator/Chart.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/charts/ext-postgres-operator/Chart.yaml b/charts/ext-postgres-operator/Chart.yaml index b2caee01..54dd3192 100644 --- a/charts/ext-postgres-operator/Chart.yaml +++ b/charts/ext-postgres-operator/Chart.yaml @@ -11,10 +11,10 @@ type: application # This is the chart version. This version number should be incremented each time you make changes # to the chart and its templates, including the app version. # Versions are expected to follow Semantic Versioning (https://semver.org/) -version: 1.2.5 +version: 1.2.6 # This is the version number of the application being deployed. This version number should be # incremented each time you make changes to the application. Versions are not expected to # follow Semantic Versioning. They should reflect the version the application is using. # It is recommended to use it with quotes. -appVersion: "1.3.2" +appVersion: "1.3.3"