From 76a99d7df6a03d62f7b4c3d2b92d634be08f74a3 Mon Sep 17 00:00:00 2001 From: TK Date: Mon, 9 Mar 2026 17:08:39 +0200 Subject: [PATCH 01/14] feat: AWS RDS pg_repack additional support --- internal/controller/postgres_controller.go | 4 +- .../controller/postgres_controller_test.go | 28 +++++++++++- pkg/postgres/aws.go | 45 +++++++++++++++++++ 3 files changed, 73 insertions(+), 4 deletions(-) diff --git a/internal/controller/postgres_controller.go b/internal/controller/postgres_controller.go index da19b93e..645328aa 100644 --- a/internal/controller/postgres_controller.go +++ b/internal/controller/postgres_controller.go @@ -181,8 +181,8 @@ func (r *PostgresReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c if err != nil { return requeue(errors.NewInternalError(err)) } - // Alter database owner if the owner role was changed - err = r.pg.AlterDatabaseOwner(instance.Spec.Database, instance.Status.Roles.Owner) + // Alter database owner to desiredOwner if the owner role was changed + err = r.pg.AlterDatabaseOwner(instance.Spec.Database, desiredOwner) if err != nil { return requeue(errors.NewInternalError(err)) } diff --git a/internal/controller/postgres_controller_test.go b/internal/controller/postgres_controller_test.go index 2f9d62b0..949f72fb 100644 --- a/internal/controller/postgres_controller_test.go +++ b/internal/controller/postgres_controller_test.go @@ -71,8 +71,6 @@ var _ = Describe("PostgresReconciler", func() { // Gomock mockCtrl = gomock.NewController(GinkgoT()) pg = mockpg.NewMockPG(mockCtrl) - pg.EXPECT().AlterDatabaseOwner(gomock.Any(), gomock.Any()).Return(nil).AnyTimes() - pg.EXPECT().ReassignDatabaseOwner(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes() cl = k8sClient // Create runtime scheme sc = scheme.Scheme @@ -365,6 +363,32 @@ var _ = Describe("PostgresReconciler", func() { }) }) + Context("MasterRole is changed for existing database", func() { + BeforeEach(func() { + modPostgres := postgresCR.DeepCopy() + modPostgres.Spec.MasterRole = "new-master-role" + modPostgres.Status = v1alpha1.PostgresStatus{ + Succeeded: true, + Roles: v1alpha1.PostgresRoles{ + Owner: "old-master-role", + }, + } + initClient(modPostgres, false) + }) + + It("should alter database owner to the desired role", func() { + pg.EXPECT().RenameGroupRole("old-master-role", "new-master-role").Return(nil).Times(1) + pg.EXPECT().AlterDatabaseOwner(name, "new-master-role").Return(nil).Times(1) + + err := runReconcile(rp, ctx, req) + Expect(err).NotTo(HaveOccurred()) + + foundPostgres := &v1alpha1.Postgres{} + Expect(cl.Get(ctx, types.NamespacedName{Name: name, Namespace: namespace}, foundPostgres)).To(BeNil()) + Expect(foundPostgres.Status.Roles.Owner).To(Equal("new-master-role")) + }) + }) + Context("Correct annotation filter is set", func() { BeforeEach(func() { // Create client diff --git a/pkg/postgres/aws.go b/pkg/postgres/aws.go index a27167a4..f9c7e096 100644 --- a/pkg/postgres/aws.go +++ b/pkg/postgres/aws.go @@ -2,6 +2,7 @@ package postgres import ( "fmt" + "strings" "github.com/lib/pq" ) @@ -10,6 +11,11 @@ type awspg struct { pg } +const ( + AWS_ALTER_REPACK_DEFAULT_PRIVS_TABLES = `ALTER DEFAULT PRIVILEGES FOR ROLE "%s" IN SCHEMA "repack" GRANT INSERT ON TABLES TO PUBLIC` + AWS_ALTER_REPACK_DEFAULT_PRIVS_SEQUENCES = `ALTER DEFAULT PRIVILEGES FOR ROLE "%s" IN SCHEMA "repack" GRANT USAGE, SELECT ON SEQUENCES TO PUBLIC` +) + func newAWSPG(postgres *pg) PG { return &awspg{ *postgres, @@ -38,6 +44,45 @@ func (c *awspg) CreateDB(dbname, role string) error { return c.pg.CreateDB(dbname, role) } +func (c *awspg) CreateExtension(dbname, extension string) error { + // Keep standard extension creation behavior for AWS as well. + err := c.pg.CreateExtension(dbname, extension) + if err != nil { + return err + } + + // AWS-specific workaround is only required for pg_repack. + if !strings.EqualFold(extension, "pg_repack") { + return nil + } + + var owner string + // Resolve current database owner role to target ALTER DEFAULT PRIVILEGES FOR ROLE. + err = c.db.QueryRow(fmt.Sprintf(GET_DB_OWNER, dbname)).Scan(&owner) + if err != nil { + return err + } + + // Execute pg_repack privilege statements in the target database. + tmpDb, err := GetConnection(c.user, c.pass, c.host, dbname, c.args) + if err != nil { + return err + } + defer tmpDb.Close() + + _, err = tmpDb.Exec(fmt.Sprintf(AWS_ALTER_REPACK_DEFAULT_PRIVS_TABLES, owner)) + if err != nil { + return err + } + + _, err = tmpDb.Exec(fmt.Sprintf(AWS_ALTER_REPACK_DEFAULT_PRIVS_SEQUENCES, owner)) + if err != nil { + return err + } + + return nil +} + func (c *awspg) CreateUserRole(role, password string) (string, error) { returnedRole, err := c.pg.CreateUserRole(role, password) if err != nil { From d32fe472f90226ec05fd41513e8ad6645ef010eb Mon Sep 17 00:00:00 2001 From: TK Date: Mon, 9 Mar 2026 18:09:23 +0200 Subject: [PATCH 02/14] feat: AWS RDS pg_repack unit tests --- go.mod | 1 + go.sum | 3 ++ pkg/postgres/aws.go | 11 +++++-- pkg/postgres/aws_test.go | 68 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 81 insertions(+), 2 deletions(-) create mode 100644 pkg/postgres/aws_test.go diff --git a/go.mod b/go.mod index dce6f3f1..fddeabdb 100644 --- a/go.mod +++ b/go.mod @@ -16,6 +16,7 @@ require ( require ( cel.dev/expr v0.24.0 // indirect + github.com/DATA-DOG/go-sqlmock v1.5.2 // indirect github.com/Masterminds/semver/v3 v3.4.0 // indirect github.com/antlr4-go/antlr/v4 v4.13.0 // indirect github.com/beorn7/perks v1.0.1 // indirect diff --git a/go.sum b/go.sum index ec4cf339..d8ee4dda 100644 --- a/go.sum +++ b/go.sum @@ -1,5 +1,7 @@ cel.dev/expr v0.24.0 h1:56OvJKSH3hDGL0ml5uSxZmz3/3Pq4tJ+fb1unVLAFcY= cel.dev/expr v0.24.0/go.mod h1:hLPLo1W4QUmuYdA72RBX06QTs6MXw941piREPl3Yfiw= +github.com/DATA-DOG/go-sqlmock v1.5.2 h1:OcvFkGmslmlZibjAjaHm3L//6LiuBgolP7OputlJIzU= +github.com/DATA-DOG/go-sqlmock v1.5.2/go.mod h1:88MAG/4G7SMwSE3CeA0ZKzrT5CiOU3OJ+JlNzwDqpNU= github.com/Masterminds/semver/v3 v3.4.0 h1:Zog+i5UMtVoCU8oKka5P7i9q9HgrJeGzI9SA1Xbatp0= github.com/Masterminds/semver/v3 v3.4.0/go.mod h1:4V+yj/TJE1HU9XfppCwVMZq3I84lprf4nC11bSS5beM= github.com/antlr4-go/antlr/v4 v4.13.0 h1:lxCg3LAv+EUK6t1i0y1V6/SLeUi0eKEKdhQAlS8TVTI= @@ -82,6 +84,7 @@ github.com/joshdk/go-junit v1.0.0 h1:S86cUKIdwBHWwA6xCmFlf3RTLfVXYQfvanM5Uh+K6GE github.com/joshdk/go-junit v1.0.0/go.mod h1:TiiV0PqkaNfFXjEiyjWM3XXrhVyCa1K4Zfga6W52ung= github.com/json-iterator/go v1.1.12 h1:PV8peI4a0ysnczrg+LtxykD8LfKY9ML6u2jnxaEnrnM= github.com/json-iterator/go v1.1.12/go.mod h1:e30LSqwooZae/UwlEbR2852Gd8hjQvJoHmT4TnhNGBo= +github.com/kisielk/sqlstruct v0.0.0-20201105191214-5f3e10d3ab46/go.mod h1:yyMNCyc/Ib3bDTKd379tNMpB/7/H5TjM2Y9QJ5THLbE= github.com/klauspost/compress v1.18.0 h1:c/Cqfb0r+Yi+JtIEq73FWXVkRonBlf0CRNYc8Zttxdo= github.com/klauspost/compress v1.18.0/go.mod h1:2Pp+KzxcywXVXMr50+X0Q/Lsb43OQHYWRCY2AiWywWQ= github.com/kr/pretty v0.2.1/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI= diff --git a/pkg/postgres/aws.go b/pkg/postgres/aws.go index f9c7e096..4f75d884 100644 --- a/pkg/postgres/aws.go +++ b/pkg/postgres/aws.go @@ -16,6 +16,9 @@ const ( AWS_ALTER_REPACK_DEFAULT_PRIVS_SEQUENCES = `ALTER DEFAULT PRIVILEGES FOR ROLE "%s" IN SCHEMA "repack" GRANT USAGE, SELECT ON SEQUENCES TO PUBLIC` ) +// Test seam: defaults to GetConnection in production, but can be overridden in unit tests. +var awsGetConnection = GetConnection + func newAWSPG(postgres *pg) PG { return &awspg{ *postgres, @@ -56,15 +59,19 @@ func (c *awspg) CreateExtension(dbname, extension string) error { return nil } + return c.applyPgRepackPrivileges(dbname) +} + +func (c *awspg) applyPgRepackPrivileges(dbname string) error { var owner string // Resolve current database owner role to target ALTER DEFAULT PRIVILEGES FOR ROLE. - err = c.db.QueryRow(fmt.Sprintf(GET_DB_OWNER, dbname)).Scan(&owner) + err := c.db.QueryRow(fmt.Sprintf(GET_DB_OWNER, dbname)).Scan(&owner) if err != nil { return err } // Execute pg_repack privilege statements in the target database. - tmpDb, err := GetConnection(c.user, c.pass, c.host, dbname, c.args) + tmpDb, err := awsGetConnection(c.user, c.pass, c.host, dbname, c.args) if err != nil { return err } diff --git a/pkg/postgres/aws_test.go b/pkg/postgres/aws_test.go new file mode 100644 index 00000000..736284f3 --- /dev/null +++ b/pkg/postgres/aws_test.go @@ -0,0 +1,68 @@ +package postgres + +import ( + "database/sql" + "fmt" + "regexp" + "testing" + + sqlmock "github.com/DATA-DOG/go-sqlmock" +) + +func TestApplyPgRepackPrivileges(t *testing.T) { + originalGetConnection := awsGetConnection + defer func() { + awsGetConnection = originalGetConnection + }() + + mainDB, mainMock, err := sqlmock.New() + if err != nil { + t.Fatalf("failed to create main sqlmock: %v", err) + } + defer mainDB.Close() + + tmpDB, tmpMock, err := sqlmock.New() + if err != nil { + t.Fatalf("failed to create tmp sqlmock: %v", err) + } + defer tmpDB.Close() + + dbname := "test-db-dev" + owner := "test-db-dev-owner" + + mainMock.ExpectQuery(regexp.QuoteMeta(fmt.Sprintf(GET_DB_OWNER, dbname))). + WillReturnRows(sqlmock.NewRows([]string{"pg_get_userbyid"}).AddRow(owner)) + + awsGetConnection = func(user, password, host, database, uriArgs string) (*sql.DB, error) { + if database != dbname { + t.Fatalf("expected database %s, got %s", dbname, database) + } + return tmpDB, nil + } + + tmpMock.ExpectExec(regexp.QuoteMeta(fmt.Sprintf(AWS_ALTER_REPACK_DEFAULT_PRIVS_TABLES, owner))). + WillReturnResult(sqlmock.NewResult(0, 0)) + tmpMock.ExpectExec(regexp.QuoteMeta(fmt.Sprintf(AWS_ALTER_REPACK_DEFAULT_PRIVS_SEQUENCES, owner))). + WillReturnResult(sqlmock.NewResult(0, 0)) + + c := &awspg{ + pg: pg{ + db: mainDB, + host: "localhost:5432", + user: "postgres", + pass: "postgres", + args: "sslmode=disable", + }, + } + + if err := c.applyPgRepackPrivileges(dbname); err != nil { + t.Fatalf("expected nil error, got %v", err) + } + + if err := mainMock.ExpectationsWereMet(); err != nil { + t.Fatalf("main DB expectations were not met: %v", err) + } + if err := tmpMock.ExpectationsWereMet(); err != nil { + t.Fatalf("tmp DB expectations were not met: %v", err) + } +} From cc57b261598ec7c625988383183804f050b3985a Mon Sep 17 00:00:00 2001 From: TK Date: Mon, 9 Mar 2026 18:22:39 +0200 Subject: [PATCH 03/14] docs: AWS RDS add some documentation for AWS Specific features --- README.md | 21 +++++++++++++++++++++ charts/ext-postgres-operator/Chart.yaml | 2 +- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 8f5d8bd3..f93d86aa 100644 --- a/README.md +++ b/README.md @@ -40,6 +40,27 @@ _None yet. [Become a sponsor!](https://github.com/sponsors/hitman99)_ - Handles CRs in dynamically created namespaces - Customizable secret values using templates +## AWS Specific Features when cloud_provider: "AWS" + +- Enable IAM authentication for this user (PostgreSQL on AWS RDS only) + + ```yaml + kind: PostgresUser + .... + spec: + aws: + enableIamAuth: false # (by Default false) + ``` +- AWS PG_repack extension installation / properly alter privileges for the owner user if cloud_provider: "AWS" + + ```yaml + kind: Postgres + ... + spec: + extensions: + - pg_repack + ``` + --- ## Supported Cloud Providers diff --git a/charts/ext-postgres-operator/Chart.yaml b/charts/ext-postgres-operator/Chart.yaml index 1407a4fd..499f38d3 100644 --- a/charts/ext-postgres-operator/Chart.yaml +++ b/charts/ext-postgres-operator/Chart.yaml @@ -9,4 +9,4 @@ description: | type: application version: 3.0.0 -appVersion: "2.4.0" +appVersion: "2.5.0" From 50810db1fe8b4b393a2007bdc3589bff692abf92 Mon Sep 17 00:00:00 2001 From: TK Date: Mon, 9 Mar 2026 18:27:17 +0200 Subject: [PATCH 04/14] chore: Update comment --- pkg/postgres/aws.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/postgres/aws.go b/pkg/postgres/aws.go index 4f75d884..a606ebbf 100644 --- a/pkg/postgres/aws.go +++ b/pkg/postgres/aws.go @@ -16,7 +16,7 @@ const ( AWS_ALTER_REPACK_DEFAULT_PRIVS_SEQUENCES = `ALTER DEFAULT PRIVILEGES FOR ROLE "%s" IN SCHEMA "repack" GRANT USAGE, SELECT ON SEQUENCES TO PUBLIC` ) -// Test seam: defaults to GetConnection in production, but can be overridden in unit tests. +// defaults to GetConnection in production, but can be overridden in unit tests. var awsGetConnection = GetConnection func newAWSPG(postgres *pg) PG { From e50bf0caf1a76912e33706a4d3a2efbe50ed32ee Mon Sep 17 00:00:00 2001 From: TK Date: Wed, 9 Sep 2026 23:08:42 +0300 Subject: [PATCH 05/14] fix: missing module --- go.mod | 1 + 1 file changed, 1 insertion(+) diff --git a/go.mod b/go.mod index c89f8640..8b1d0272 100644 --- a/go.mod +++ b/go.mod @@ -4,6 +4,7 @@ go 1.26.0 require ( github.com/go-logr/logr v1.4.4 + github.com/DATA-DOG/go-sqlmock v1.5.2 github.com/lib/pq v1.12.3 github.com/onsi/ginkgo/v2 v2.32.0 github.com/onsi/gomega v1.42.1 From f47b51da1ba3377f7d96d534d9cce9a0310e7dce Mon Sep 17 00:00:00 2001 From: TK Date: Wed, 9 Sep 2026 23:13:02 +0300 Subject: [PATCH 06/14] fix: missing module --- go.sum | 3 +++ 1 file changed, 3 insertions(+) diff --git a/go.sum b/go.sum index 4bc62390..cb0fb1c9 100644 --- a/go.sum +++ b/go.sum @@ -1,5 +1,7 @@ cel.dev/expr v0.25.1 h1:1KrZg61W6TWSxuNZ37Xy49ps13NUovb66QLprthtwi4= cel.dev/expr v0.25.1/go.mod h1:hrXvqGP6G6gyx8UAHSHJ5RGk//1Oj5nXQ2NI02Nrsg4= +github.com/DATA-DOG/go-sqlmock v1.5.2 h1:OcvFkGmslmlZibjAjaHm3L//6LiuBgolP7OputlJIzU= +github.com/DATA-DOG/go-sqlmock v1.5.2/go.mod h1:88MAG/4G7SMwSE3CeA0ZKzrT5CiOU3OJ+JlNzwDqpNU= github.com/Masterminds/semver/v3 v3.4.0 h1:Zog+i5UMtVoCU8oKka5P7i9q9HgrJeGzI9SA1Xbatp0= github.com/Masterminds/semver/v3 v3.4.0/go.mod h1:4V+yj/TJE1HU9XfppCwVMZq3I84lprf4nC11bSS5beM= github.com/antlr4-go/antlr/v4 v4.13.1 h1:SqQKkuVZ+zWkMMNkjy5FZe5mr5WURWnlpmOuzYWrPrQ= @@ -103,6 +105,7 @@ github.com/joshdk/go-junit v1.0.0 h1:S86cUKIdwBHWwA6xCmFlf3RTLfVXYQfvanM5Uh+K6GE github.com/joshdk/go-junit v1.0.0/go.mod h1:TiiV0PqkaNfFXjEiyjWM3XXrhVyCa1K4Zfga6W52ung= github.com/json-iterator/go v1.1.12 h1:PV8peI4a0ysnczrg+LtxykD8LfKY9ML6u2jnxaEnrnM= github.com/json-iterator/go v1.1.12/go.mod h1:e30LSqwooZae/UwlEbR2852Gd8hjQvJoHmT4TnhNGBo= +github.com/kisielk/sqlstruct v0.0.0-20201105191214-5f3e10d3ab46/go.mod h1:yyMNCyc/Ib3bDTKd379tNMpB/7/H5TjM2Y9QJ5THLbE= github.com/klauspost/compress v1.19.0 h1:sXLILfc9jV2QYWkzFOPWStmcUVH2RHEB1JCdY2oVvCQ= github.com/klauspost/compress v1.19.0/go.mod h1:cwPg85FWrGar70rWktvGQj8/hthj3wpl0PGDogxkrSQ= github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= From 3069d541f002583445f7f8bcd94ed47ef25ceff0 Mon Sep 17 00:00:00 2001 From: TK Date: Thu, 10 Sep 2026 12:12:23 +0300 Subject: [PATCH 07/14] fix: configure default privileges for database owner --- internal/controller/postgres_controller.go | 3 +++ pkg/postgres/database.go | 16 ++++++++++++++++ pkg/postgres/postgres.go | 1 + 3 files changed, 20 insertions(+) diff --git a/internal/controller/postgres_controller.go b/internal/controller/postgres_controller.go index 645328aa..c19c2c43 100644 --- a/internal/controller/postgres_controller.go +++ b/internal/controller/postgres_controller.go @@ -237,6 +237,7 @@ func (r *PostgresReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c // Set privileges on schema schemaPrivilegesReader := postgres.PostgresSchemaPrivileges{ + Owner: owner, DB: database, Role: reader, Schema: schema, @@ -249,6 +250,7 @@ func (r *PostgresReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c continue } schemaPrivilegesWriter := postgres.PostgresSchemaPrivileges{ + Owner: owner, DB: database, Role: writer, Schema: schema, @@ -263,6 +265,7 @@ func (r *PostgresReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c continue } schemaPrivilegesOwner := postgres.PostgresSchemaPrivileges{ + Owner: owner, DB: database, Role: owner, Schema: schema, diff --git a/pkg/postgres/database.go b/pkg/postgres/database.go index 11fe8c43..b734f781 100644 --- a/pkg/postgres/database.go +++ b/pkg/postgres/database.go @@ -136,6 +136,22 @@ func (c *pg) SetSchemaPrivileges(schemaPrivileges PostgresSchemaPrivileges) erro } defer tmpDb.Close() + // Preserve administrator defaults while also configuring the stable object owner. + if schemaPrivileges.Owner != "" && schemaPrivileges.Owner != c.user { + for _, grant := range []struct{ objects, privileges string }{ + {"TABLES", schemaPrivileges.Privs}, {"SEQUENCES", schemaPrivileges.SequencePrivs}, {"FUNCTIONS", schemaPrivileges.FunctionPrivs}, + } { + if grant.privileges == "" { + continue + } + _, err = tmpDb.Exec(fmt.Sprintf("ALTER DEFAULT PRIVILEGES FOR ROLE %s IN SCHEMA %s GRANT %s ON %s TO %s", + pq.QuoteIdentifier(schemaPrivileges.Owner), pq.QuoteIdentifier(schemaPrivileges.Schema), grant.privileges, grant.objects, pq.QuoteIdentifier(schemaPrivileges.Role))) + if err != nil { + return err + } + } + } + // Grant role usage on schema _, err = tmpDb.Exec(fmt.Sprintf(GRANT_USAGE_SCHEMA, schemaPrivileges.Schema, schemaPrivileges.Role)) if err != nil { diff --git a/pkg/postgres/postgres.go b/pkg/postgres/postgres.go index dd5886a5..4ac60f09 100644 --- a/pkg/postgres/postgres.go +++ b/pkg/postgres/postgres.go @@ -39,6 +39,7 @@ type pg struct { } type PostgresSchemaPrivileges struct { + Owner string DB string Role string Schema string From d98d9f4dba9608421668b8610ced1d526f6ca827 Mon Sep 17 00:00:00 2001 From: TK Date: Thu, 10 Sep 2026 12:20:58 +0300 Subject: [PATCH 08/14] chore: update comment /Keep default privileges aligned with the current database owner and configured access roles --- README.md | 83 +++++++++++++++++++++------------------- pkg/postgres/database.go | 2 +- 2 files changed, 45 insertions(+), 40 deletions(-) diff --git a/README.md b/README.md index f93d86aa..16c7d773 100644 --- a/README.md +++ b/README.md @@ -44,18 +44,20 @@ _None yet. [Become a sponsor!](https://github.com/sponsors/hitman99)_ - Enable IAM authentication for this user (PostgreSQL on AWS RDS only) - ```yaml + ````yaml kind: PostgresUser .... spec: aws: enableIamAuth: false # (by Default false) ``` + ```` + - AWS PG_repack extension installation / properly alter privileges for the owner user if cloud_provider: "AWS" ```yaml kind: Postgres - ... + --- spec: extensions: - pg_repack @@ -87,11 +89,11 @@ Set `POSTGRES_CLOUD_PROVIDER` to `AWS` via environment variable, Kubernetes Secr Set environment variables in [`config/manager/operator.yaml`](config/manager/operator.yaml): -| Name | Description | Default | -| --- | --- | --- | -| `WATCH_NAMESPACE` | Namespace to watch. Empty string = all namespaces. | (all namespaces) | -| `POSTGRES_INSTANCE` | Operator identity for multi-instance deployments. | (empty) | -| `KEEP_SECRET_NAME` | Use user-provided secret names instead of auto-generated ones. | disabled | +| Name | Description | Default | +| ------------------- | -------------------------------------------------------------- | ---------------- | +| `WATCH_NAMESPACE` | Namespace to watch. Empty string = all namespaces. | (all namespaces) | +| `POSTGRES_INSTANCE` | Operator identity for multi-instance deployments. | (empty) | +| `KEEP_SECRET_NAME` | Use user-provided secret names instead of auto-generated ones. | disabled | > **Note:** > If enabling `KEEP_SECRET_NAME`, ensure there are no secret name conflicts in your namespace to avoid reconcile loops. @@ -103,11 +105,13 @@ Set environment variables in [`config/manager/operator.yaml`](config/manager/ope The Helm chart for this operator is located in the `charts/ext-postgres-operator` subdirectory. Follow these steps to install: 1. Add the Helm repository: + ```bash helm repo add ext-postgres-operator https://movetokube.github.io/postgres-operator/ ``` 2. Install the operator: + ```bash helm install -n operators ext-postgres-operator ext-postgres-operator/ext-postgres-operator ``` @@ -142,11 +146,13 @@ To install the operator using Kustomize, follow these steps: 1. Configure Postgres credentials for the operator in `config/default/secret.yaml`. 2. Deploy the operator: + ```bash kubectl kustomize config/default/ | kubectl apply -f - ``` Alternatively, use [Kustomize](https://github.com/kubernetes-sigs/kustomize) directly: + ```bash kustomize build config/default/ | kubectl apply -f - ``` @@ -170,11 +176,11 @@ spec: dropOnDelete: false # Set to true if you want the operator to drop the database and role when this CR is deleted (optional) masterRole: test-db-group (optional) schemas: # List of schemas the operator should create in database (optional) - - stores - - customers + - stores + - customers extensions: # List of extensions that should be created in the database (optional) - - fuzzystrmatch - - pgcrypto + - fuzzystrmatch + - pgcrypto ``` This creates a database called `test-db` and a role `test-db-group` that is set as the owner of the database. @@ -194,14 +200,14 @@ metadata: postgres.db.movetokube.com/instance: POSTGRES_INSTANCE spec: role: username - database: my-db # This references the Postgres CR + database: my-db # This references the Postgres CR secretName: my-secret - privileges: OWNER # Can be OWNER/READ/WRITE - annotations: # Annotations to be propagated to the secrets metadata section (optional) + privileges: OWNER # Can be OWNER/READ/WRITE + annotations: # Annotations to be propagated to the secrets metadata section (optional) foo: "bar" labels: - foo: "bar" # Labels to be propagated to the secrets metadata section (optional) - secretTemplate: # Output secrets can be customized using standard Go templates + foo: "bar" # Labels to be propagated to the secrets metadata section (optional) + secretTemplate: # Output secrets can be customized using standard Go templates PQ_URL: "host={{.Host}} user={{.Role}} password={{.Password}} dbname={{.Database}}" ``` @@ -212,22 +218,22 @@ This creates a user role `username-` and grants role `test-db-group`, `tes Two `Postgres` referencing the same database can exist in more than one namespace. The last CR referencing a database will drop the group role and transfer database ownership to the role used by the operator. Every PostgresUser has a generated Kubernetes secret attached to it, which contains the following data (i.e.): -| Key | Comment | -|----------------------|---------------------| -| `DATABASE_NAME` | Name of the database, same as in `Postgres` CR, copied for convenience | -| `HOST` | PostgreSQL server host (including port number) | -| `URI_ARGS` | URI Args, same as in `Postgres` CR, copied for convenience | -| `PASSWORD` | Autogenerated password for user | -| `ROLE` | Autogenerated role with login enabled (user) | -| `LOGIN` | Same as `ROLE`. In case `POSTGRES_CLOUD_PROVIDER` is set to "Azure", `LOGIN` it will be set to `{role}@{serverName}`, serverName is extracted from `POSTGRES_USER` from operator's config. | -| `POSTGRES_URL` | Connection string for Posgres, could be used for Go applications | -| `POSTGRES_JDBC_URL` | JDBC compatible Postgres URI, formatter as `jdbc:postgresql://{POSTGRES_HOST}/{DATABASE_NAME}` | -| `HOSTNAME` | The PostgreSQL server hostname (without port) | -| `PORT` | The PostgreSQL server port | - -| Functions | Meaning | -|----------------|-------------------------------------------------------------------| -| `mergeUriArgs` | Merge any provided uri args with any set in the `Postgres` CR | +| Key | Comment | +| ------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| `DATABASE_NAME` | Name of the database, same as in `Postgres` CR, copied for convenience | +| `HOST` | PostgreSQL server host (including port number) | +| `URI_ARGS` | URI Args, same as in `Postgres` CR, copied for convenience | +| `PASSWORD` | Autogenerated password for user | +| `ROLE` | Autogenerated role with login enabled (user) | +| `LOGIN` | Same as `ROLE`. In case `POSTGRES_CLOUD_PROVIDER` is set to "Azure", `LOGIN` it will be set to `{role}@{serverName}`, serverName is extracted from `POSTGRES_USER` from operator's config. | +| `POSTGRES_URL` | Connection string for Posgres, could be used for Go applications | +| `POSTGRES_JDBC_URL` | JDBC compatible Postgres URI, formatter as `jdbc:postgresql://{POSTGRES_HOST}/{DATABASE_NAME}` | +| `HOSTNAME` | The PostgreSQL server hostname (without port) | +| `PORT` | The PostgreSQL server port | + +| Functions | Meaning | +| -------------- | ------------------------------------------------------------- | +| `mergeUriArgs` | Merge any provided uri args with any set in the `Postgres` CR | ### Multiple operator support @@ -248,7 +254,7 @@ meeting the specific needs of different applications. Available context: | Variable | Meaning | -|-------------|------------------------------| +| ----------- | ---------------------------- | | `.Host` | Database host | | `.Role` | Generated user/role name | | `.Database` | Referenced database name | @@ -264,12 +270,11 @@ can be found [here](https://github.com/kubernetes/client-go/blob/master/README.m Postgres operator compatibility with Operator SDK version is in the table below | | Operator SDK version | apiextensions.k8s.io | -|---------------------------|----------------------|----------------------| -| `postgres-operator 0.4.x` | v0.17 | v1beta1 | -| `postgres-operator 1.x.x` | v0.18 | v1 | -| `postgres-operator 2.x.x` | v1.39 | v1 | -| `HEAD` | v1.39 | v1 | - +| ------------------------- | -------------------- | -------------------- | +| `postgres-operator 0.4.x` | v0.17 | v1beta1 | +| `postgres-operator 1.x.x` | v0.18 | v1 | +| `postgres-operator 2.x.x` | v1.39 | v1 | +| `HEAD` | v1.39 | v1 | ## Contributing diff --git a/pkg/postgres/database.go b/pkg/postgres/database.go index b734f781..6d19d17b 100644 --- a/pkg/postgres/database.go +++ b/pkg/postgres/database.go @@ -136,7 +136,7 @@ func (c *pg) SetSchemaPrivileges(schemaPrivileges PostgresSchemaPrivileges) erro } defer tmpDb.Close() - // Preserve administrator defaults while also configuring the stable object owner. + // Keep default privileges aligned with the current database owner and configured access roles if schemaPrivileges.Owner != "" && schemaPrivileges.Owner != c.user { for _, grant := range []struct{ objects, privileges string }{ {"TABLES", schemaPrivileges.Privs}, {"SEQUENCES", schemaPrivileges.SequencePrivs}, {"FUNCTIONS", schemaPrivileges.FunctionPrivs}, From f0f2d521154931b3a99b5be153235a4a81a6fdc6 Mon Sep 17 00:00:00 2001 From: TK Date: Thu, 10 Sep 2026 12:15:17 +0300 Subject: [PATCH 09/14] feat: add scheduled database permission repair --- .github/workflows/test.yml | 28 ++ README.md | 78 ++++++ api/v1alpha1/postgres_types.go | 41 ++- api/v1alpha1/zz_generated.deepcopy.go | 52 ++++ .../crds/db.movetokube.com_postgres_crd.yaml | 44 ++++ .../crd/bases/db.movetokube.com_postgres.yaml | 44 ++++ config/samples/db_v1alpha1_postgres.yaml | 5 + go.mod | 1 + go.sum | 2 + internal/controller/permission_repair.go | 143 ++++++++++ internal/controller/permission_repair_test.go | 245 ++++++++++++++++++ internal/controller/postgres_controller.go | 100 +++---- pkg/postgres/mock/postgres.go | 21 +- pkg/postgres/permission_repair.go | 182 +++++++++++++ .../permission_repair_integration_test.go | 214 +++++++++++++++ pkg/postgres/permission_repair_test.go | 56 ++++ pkg/postgres/postgres.go | 2 + 17 files changed, 1206 insertions(+), 52 deletions(-) create mode 100644 internal/controller/permission_repair.go create mode 100644 internal/controller/permission_repair_test.go create mode 100644 pkg/postgres/permission_repair.go create mode 100644 pkg/postgres/permission_repair_integration_test.go create mode 100644 pkg/postgres/permission_repair_test.go diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index f700a6a9..b9557d74 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -32,3 +32,31 @@ jobs: go clean -modcache make build file bin/manager + permission-repair: + name: Permission repair (PostgreSQL ${{ matrix.postgres }}) + runs-on: ubuntu-latest + timeout-minutes: 15 + strategy: + matrix: + postgres: ["16", "18"] + services: + postgres: + image: postgres:${{ matrix.postgres }} + env: + POSTGRES_HOST_AUTH_METHOD: trust + ports: + - 5432:5432 + options: >- + --health-cmd "pg_isready -U postgres" + --health-interval 5s + --health-timeout 5s + --health-retries 10 + steps: + - uses: actions/checkout@v7 + - uses: actions/setup-go@v7 + with: + go-version-file: go.mod + - name: Verify permission recovery and rollback + env: + PERMISSION_REPAIR_TEST_DSN: postgresql://postgres@localhost:5432/postgres?sslmode=disable + run: go test -race ./pkg/postgres -run TestPermissionRepairIntegration -count=1 diff --git a/README.md b/README.md index 16c7d773..2e65c44c 100644 --- a/README.md +++ b/README.md @@ -186,6 +186,79 @@ spec: This creates a database called `test-db` and a role `test-db-group` that is set as the owner of the database. Reader and writer roles are also created. These roles have read and write permissions to all tables in the schemas created by the operator, if any. +### Scheduled permission repair + +Permission repair is optional per `Postgres`. It adds missing grants; it never +revokes custom grants, changes ownership, recreates roles or rotates credentials. + +```yaml +spec: + # Keep the existing database, masterRole and schema configuration. + permissionRepair: + schedule: "0 2 * * *" # Five cron fields; every day at 02:00 UTC + windowDuration: "30m" # Latest allowed start/end; defaults to 30m + timeout: "5m" # Maximum transaction duration; defaults to 5m +``` + +The schedule accepts five-field cron syntax (including lists, ranges and steps), +without seconds or `@daily`-style shortcuts. Schedules always use UTC, regardless +of the operator host timezone or daylight-saving changes. Timezone overrides in +the cron expression are rejected. `windowDuration` must be positive and at most `24h`; `timeout` must be +positive and no longer than the window. Invalid configuration is reported in +`status.permissionRepair.error` and no scheduled repair runs. + +With a schedule configured, initial provisioning still applies normal grants. +Subsequent permission repair runs only in its window, using `spec.schemas` and the +existing roles in `status.roles`. Keep the full list of application schemas in +`spec.schemas`; system schemas are rejected. Removing `permissionRepair` restores +the original event-driven schema-grant behavior. + +Repair covers database `CONNECT`, schema `USAGE` (and writer `CREATE`), tables, +partitions, views, materialized views, foreign tables, sequences, functions, +procedures, types/domains and owner-managed large objects. Reader gets table/large +object `SELECT` and type `USAGE`. Writer gets table CRUD, sequence `USAGE, SELECT`, +routine `EXECUTE`, type `USAGE`, and large-object `SELECT, UPDATE`. It does not grant +reader execution of routines or writer `TRUNCATE`/ownership rights. + +Existing application objects must belong to the stable owner. Extension-managed objects and PostgreSQL-generated internal routines (such as +range constructors) retain their existing policy. RLS policies, foreign-server credentials +and user mappings are not repaired. Large objects are scoped by owner in the +current database because they do not belong to schemas. + +Default privileges target the stable owner explicitly, in each configured schema. +They therefore apply to migration objects created as that role, not objects +created as an administrator or another login/group. Large-object defaults require +PostgreSQL 18; earlier servers receive existing large-object grants only. New +schemas must be added to `spec.schemas`. Reconcile extensions through their normal +configuration. + +The controller persists `nextRunTime`, `lastAttemptTime`, `lastSuccessTime` and +`error` in `status.permissionRepair`. Provisioning `status.succeeded` stays separate +from repair failures. Enabling/changing the schedule (or its database/schema/role +scope) schedules the next future occurrence. After a restart, a pending occurrence +runs only if still inside its window; expired occurrences are skipped. Each +occurrence is claimed before SQL, attempted at most once, and failures wait until +the next occurrence. A crash after claiming can skip that attempt; interrupted +attempts remain visible in status. This avoids immediate retry loops and repairs +outside maintenance windows. + +Repairs run in a transaction, use cancellation/timeouts capped at the window end, +and take a PostgreSQL advisory lock to prevent concurrent repairs of the same +database. Run the operator with its default leader election enabled. Error status +never includes connection strings or passwords. + +Upgrade the installed CRD as well as the controller before configuring the new +fields. Helm does not automatically upgrade CRDs from a chart's `crds/` directory. + +To run the real privilege/rollback tests against a **disposable** PostgreSQL server: + +```bash +PERMISSION_REPAIR_TEST_DSN='postgresql://postgres@127.0.0.1:55439/postgres?sslmode=disable' go test ./pkg/postgres -run TestPermissionRepairIntegration -count=1 +``` + +The integration test creates and removes a temporary database and roles. CI runs +it on PostgreSQL 16 and 18; scheduling/controller tests require no live cluster. + ### PostgresUser ```yaml @@ -199,6 +272,11 @@ metadata: # use this to target which instance of operator should process this CR. See general config postgres.db.movetokube.com/instance: POSTGRES_INSTANCE spec: + # Keep the existing database, masterRole and schema configuration. + permissionRepair: + schedule: "0 2 * * *" # Five cron fields; every day at 02:00 UTC + windowDuration: "30m" # Latest allowed start/end; defaults to 30m + timeout: "5m" # Maximum transaction duration; defaults to 5m role: username database: my-db # This references the Postgres CR secretName: my-secret diff --git a/api/v1alpha1/postgres_types.go b/api/v1alpha1/postgres_types.go index 8ce68189..2196cc1b 100644 --- a/api/v1alpha1/postgres_types.go +++ b/api/v1alpha1/postgres_types.go @@ -9,7 +9,10 @@ import ( // PostgresSpec defines the desired state of Postgres type PostgresSpec struct { - Database string `json:"database"` + // PermissionRepair enables scheduled, additive permission repair. Omit to disable. + // +optional + PermissionRepair *PermissionRepairSpec `json:"permissionRepair,omitempty"` + Database string `json:"database"` // +optional MasterRole string `json:"masterRole,omitempty"` // +optional @@ -22,10 +25,42 @@ type PostgresSpec struct { Extensions []string `json:"extensions,omitempty"` } +// PermissionRepairSpec schedules maintenance using a five-field UTC cron expression. +type PermissionRepairSpec struct { + // Schedule uses minute, hour, day of month, month, day of week, always in UTC. + // +kubebuilder:validation:MinLength=9 + Schedule string `json:"schedule"` + // WindowDuration limits how late an occurrence can start, including after restart. + // +kubebuilder:default="30m" + // +optional + WindowDuration string `json:"windowDuration,omitempty"` + // Timeout bounds each transaction, also capped by the end of the window. + // +kubebuilder:default="5m" + // +optional + Timeout string `json:"timeout,omitempty"` +} + +// PermissionRepairStatus persists scheduling across restarts and leader changes. +type PermissionRepairStatus struct { + // Configuration identifies the schedule configuration used for NextRunTime. + Configuration string `json:"configuration,omitempty"` + // +optional + NextRunTime *metav1.Time `json:"nextRunTime,omitempty"` + // +optional + LastAttemptTime *metav1.Time `json:"lastAttemptTime,omitempty"` + // +optional + LastSuccessTime *metav1.Time `json:"lastSuccessTime,omitempty"` + // Error is empty after a successful repair. It never contains connection credentials. + // +optional + Error string `json:"error,omitempty"` +} + // PostgresStatus defines the observed state of Postgres type PostgresStatus struct { - Succeeded bool `json:"succeeded"` - Roles PostgresRoles `json:"roles"` + // +optional + PermissionRepair *PermissionRepairStatus `json:"permissionRepair,omitempty"` + Succeeded bool `json:"succeeded"` + Roles PostgresRoles `json:"roles"` // +optional // +listType=set Schemas []string `json:"schemas,omitempty"` diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index c2112808..dbe284a3 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -8,6 +8,48 @@ import ( runtime "k8s.io/apimachinery/pkg/runtime" ) +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *PermissionRepairSpec) DeepCopyInto(out *PermissionRepairSpec) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PermissionRepairSpec. +func (in *PermissionRepairSpec) DeepCopy() *PermissionRepairSpec { + if in == nil { + return nil + } + out := new(PermissionRepairSpec) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *PermissionRepairStatus) DeepCopyInto(out *PermissionRepairStatus) { + *out = *in + if in.NextRunTime != nil { + in, out := &in.NextRunTime, &out.NextRunTime + *out = (*in).DeepCopy() + } + if in.LastAttemptTime != nil { + in, out := &in.LastAttemptTime, &out.LastAttemptTime + *out = (*in).DeepCopy() + } + if in.LastSuccessTime != nil { + in, out := &in.LastSuccessTime, &out.LastSuccessTime + *out = (*in).DeepCopy() + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PermissionRepairStatus. +func (in *PermissionRepairStatus) DeepCopy() *PermissionRepairStatus { + if in == nil { + return nil + } + out := new(PermissionRepairStatus) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Postgres) DeepCopyInto(out *Postgres) { *out = *in @@ -85,6 +127,11 @@ func (in *PostgresRoles) DeepCopy() *PostgresRoles { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PostgresSpec) DeepCopyInto(out *PostgresSpec) { *out = *in + if in.PermissionRepair != nil { + in, out := &in.PermissionRepair, &out.PermissionRepair + *out = new(PermissionRepairSpec) + **out = **in + } if in.Schemas != nil { in, out := &in.Schemas, &out.Schemas *out = make([]string, len(*in)) @@ -110,6 +157,11 @@ func (in *PostgresSpec) DeepCopy() *PostgresSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PostgresStatus) DeepCopyInto(out *PostgresStatus) { *out = *in + if in.PermissionRepair != nil { + in, out := &in.PermissionRepair, &out.PermissionRepair + *out = new(PermissionRepairStatus) + (*in).DeepCopyInto(*out) + } out.Roles = in.Roles if in.Schemas != nil { in, out := &in.Schemas, &out.Schemas diff --git a/charts/ext-postgres-operator/crds/db.movetokube.com_postgres_crd.yaml b/charts/ext-postgres-operator/crds/db.movetokube.com_postgres_crd.yaml index 4977deff..bfa6f246 100644 --- a/charts/ext-postgres-operator/crds/db.movetokube.com_postgres_crd.yaml +++ b/charts/ext-postgres-operator/crds/db.movetokube.com_postgres_crd.yaml @@ -42,6 +42,28 @@ spec: x-kubernetes-list-type: set masterRole: type: string + permissionRepair: + description: PermissionRepair enables scheduled, additive permission + repair. Omit to disable. + properties: + schedule: + description: Schedule uses minute, hour, day of month, month, + day of week, always in UTC. + minLength: 9 + type: string + timeout: + default: 5m + description: Timeout bounds each transaction, also capped by the + end of the window. + type: string + windowDuration: + default: 30m + description: WindowDuration limits how late an occurrence can + start, including after restart. + type: string + required: + - schedule + type: object schemas: items: type: string @@ -58,6 +80,28 @@ spec: type: string type: array x-kubernetes-list-type: set + permissionRepair: + description: PermissionRepairStatus persists scheduling across restarts + and leader changes. + properties: + configuration: + description: Configuration identifies the schedule configuration + used for NextRunTime. + type: string + error: + description: Error is empty after a successful repair. It never + contains connection credentials. + type: string + lastAttemptTime: + format: date-time + type: string + lastSuccessTime: + format: date-time + type: string + nextRunTime: + format: date-time + type: string + type: object roles: description: PostgresRoles stores the different group roles for database properties: diff --git a/config/crd/bases/db.movetokube.com_postgres.yaml b/config/crd/bases/db.movetokube.com_postgres.yaml index 10b1f258..748f75bd 100644 --- a/config/crd/bases/db.movetokube.com_postgres.yaml +++ b/config/crd/bases/db.movetokube.com_postgres.yaml @@ -50,6 +50,28 @@ spec: x-kubernetes-list-type: set masterRole: type: string + permissionRepair: + description: PermissionRepair enables scheduled, additive permission + repair. Omit to disable. + properties: + schedule: + description: Schedule uses minute, hour, day of month, month, + day of week, always in UTC. + minLength: 9 + type: string + timeout: + default: 5m + description: Timeout bounds each transaction, also capped by the + end of the window. + type: string + windowDuration: + default: 30m + description: WindowDuration limits how late an occurrence can + start, including after restart. + type: string + required: + - schedule + type: object schemas: items: type: string @@ -66,6 +88,28 @@ spec: type: string type: array x-kubernetes-list-type: set + permissionRepair: + description: PermissionRepairStatus persists scheduling across restarts + and leader changes. + properties: + configuration: + description: Configuration identifies the schedule configuration + used for NextRunTime. + type: string + error: + description: Error is empty after a successful repair. It never + contains connection credentials. + type: string + lastAttemptTime: + format: date-time + type: string + lastSuccessTime: + format: date-time + type: string + nextRunTime: + format: date-time + type: string + type: object roles: description: PostgresRoles stores the different group roles for database properties: diff --git a/config/samples/db_v1alpha1_postgres.yaml b/config/samples/db_v1alpha1_postgres.yaml index a1d0525c..0e7667c1 100644 --- a/config/samples/db_v1alpha1_postgres.yaml +++ b/config/samples/db_v1alpha1_postgres.yaml @@ -12,3 +12,8 @@ spec: schemas: # List of schemas the operator should create in database - stores - customers + # Optional: reapply application permissions daily in a bounded window. + # permissionRepair: + # schedule: "0 2 * * *" # Every day at 02:00 UTC + # windowDuration: "30m" + # timeout: "5m" diff --git a/go.mod b/go.mod index 8b1d0272..4a738fad 100644 --- a/go.mod +++ b/go.mod @@ -8,6 +8,7 @@ require ( github.com/lib/pq v1.12.3 github.com/onsi/ginkgo/v2 v2.32.0 github.com/onsi/gomega v1.42.1 + github.com/robfig/cron/v3 v3.0.1 go.uber.org/mock v0.6.0 k8s.io/api v0.37.0 k8s.io/apimachinery v0.37.0 diff --git a/go.sum b/go.sum index cb0fb1c9..37169a27 100644 --- a/go.sum +++ b/go.sum @@ -145,6 +145,8 @@ github.com/prometheus/common v0.70.0 h1:bcpru3tWPVnxGnETLgOV5jbp/JRXgYEyv65CuBLA github.com/prometheus/common v0.70.0/go.mod h1:S/SFasQmgGiYH6C81LKCtYa8QACgthGg5zxL2udV7SY= github.com/prometheus/procfs v0.21.1 h1:GljZCt+zSTS+NZq88cyQ1LjZ+RCHp3uVuabBWA5+OJI= github.com/prometheus/procfs v0.21.1/go.mod h1:aB55Cww9pdSJVHk0hUf0inxWyyjPogFIjmHKYgMKmtY= +github.com/robfig/cron/v3 v3.0.1 h1:WdRxkvbJztn8LMz/QEvLN5sBU+xKpSqwwUO1Pjr4qDs= +github.com/robfig/cron/v3 v3.0.1/go.mod h1:eQICP3HwyT7UooqI/z+Ov+PtYAWygg1TEWWzGIFLtro= github.com/rogpeppe/go-internal v1.14.1 h1:UQB4HGPB6osV0SQTLymcB4TgvyWu6ZyliaW0tI/otEQ= github.com/rogpeppe/go-internal v1.14.1/go.mod h1:MaRKkUm5W0goXpeCfT7UZI6fk/L7L7so1lCWt35ZSgc= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= diff --git a/internal/controller/permission_repair.go b/internal/controller/permission_repair.go new file mode 100644 index 00000000..54cbbe81 --- /dev/null +++ b/internal/controller/permission_repair.go @@ -0,0 +1,143 @@ +package controller + +import ( + "context" + "crypto/sha256" + "fmt" + "strings" + "time" + + dbv1alpha1 "github.com/movetokube/postgres-operator/api/v1alpha1" + "github.com/movetokube/postgres-operator/pkg/postgres" + "github.com/robfig/cron/v3" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/log" +) + +type repairSchedule struct { + cron.Schedule + window, timeout time.Duration + key string +} + +func parseRepairSchedule(spec *dbv1alpha1.PermissionRepairSpec, now time.Time) (repairSchedule, error) { + var result repairSchedule + if len(strings.Fields(spec.Schedule)) != 5 || strings.Contains(spec.Schedule, "=") { + return result, fmt.Errorf("schedule must have exactly five cron fields") + } + parser := cron.NewParser(cron.Minute | cron.Hour | cron.Dom | cron.Month | cron.Dow) + schedule, err := parser.Parse("CRON_TZ=UTC " + spec.Schedule) + if err != nil { + return result, fmt.Errorf("invalid cron schedule") + } + if schedule.Next(now).IsZero() { + return result, fmt.Errorf("cron schedule has no next occurrence") + } + window := spec.WindowDuration + if window == "" { + window = "30m" + } + timeout := spec.Timeout + if timeout == "" { + timeout = "5m" + } + result.window, err = time.ParseDuration(window) + if err != nil || result.window <= 0 || result.window > 24*time.Hour { + return result, fmt.Errorf("windowDuration must be positive and at most 24h") + } + result.timeout, err = time.ParseDuration(timeout) + if err != nil || result.timeout <= 0 || result.timeout > result.window { + return result, fmt.Errorf("timeout must be positive and no longer than windowDuration") + } + result.Schedule = schedule + result.key = fmt.Sprintf("%x", sha256.Sum256([]byte(spec.Schedule+"|UTC|"+window+"|"+timeout))) + return result, nil +} + +func (r *PostgresReconciler) repairNow() time.Time { + if r.now != nil { + return r.now().UTC() + } + return time.Now().UTC() +} + +func (r *PostgresReconciler) reconcilePermissionRepair(ctx context.Context, instance *dbv1alpha1.Postgres) (ctrl.Result, error) { + before := instance.DeepCopy() + if instance.Spec.PermissionRepair == nil { + if instance.Status.PermissionRepair != nil { + instance.Status.PermissionRepair = nil + return ctrl.Result{}, r.Status().Patch(ctx, instance, client.MergeFrom(before)) + } + return ctrl.Result{}, nil + } + now := r.repairNow() + schedule, err := parseRepairSchedule(instance.Spec.PermissionRepair, now) + if instance.Status.PermissionRepair == nil { + instance.Status.PermissionRepair = &dbv1alpha1.PermissionRepairStatus{} + } + status := instance.Status.PermissionRepair + if err != nil { + status.Error = err.Error() + status.NextRunTime = nil + status.Configuration = "" + return ctrl.Result{}, r.Status().Patch(ctx, instance, client.MergeFrom(before)) + } + // A schema/role change starts a new schedule instead of applying stale work. + schedule.key = fmt.Sprintf("%x", sha256.Sum256([]byte(fmt.Sprintf("%s|%s|%v|%v", schedule.key, instance.Spec.Database, instance.Spec.Schemas, instance.Status.Roles)))) + if status.Configuration != schedule.key || status.NextRunTime == nil { + next := metav1.NewTime(schedule.Next(now)) + status.NextRunTime = &next + status.Configuration = schedule.key + status.Error = "" + return ctrl.Result{RequeueAfter: next.Sub(now)}, r.Status().Patch(ctx, instance, client.MergeFrom(before)) + } + due := status.NextRunTime.Time + if now.Before(due) { + return ctrl.Result{RequeueAfter: due.Sub(now)}, nil + } + next := metav1.NewTime(schedule.Next(now)) + status.NextRunTime = &next + end := due.Add(schedule.window) + if !now.Before(end) { + // Do not catch up outside the maintenance window. + return ctrl.Result{RequeueAfter: next.Sub(now)}, r.Status().Patch(ctx, instance, client.MergeFrom(before)) + } + // Persist the occurrence claim before SQL. A restart will not repeat this occurrence. + attempt := metav1.NewTime(now) + status.LastAttemptTime = &attempt + status.Error = "repair interrupted before completion" + if err := r.Status().Patch(ctx, instance, client.MergeFromWithOptions(before, client.MergeFromWithOptimisticLock{})); err != nil { + return ctrl.Result{}, err + } + before = instance.DeepCopy() + status = instance.Status.PermissionRepair + deadline := now.Add(schedule.timeout) + if end.Before(deadline) { + deadline = end + } + repairCtx, cancel := context.WithTimeout(ctx, deadline.Sub(now)) + defer cancel() + err = r.pg.RepairPermissions(repairCtx, postgres.PermissionRepair{ + Database: instance.Spec.Database, Schemas: instance.Spec.Schemas, + Owner: instance.Status.Roles.Owner, Reader: instance.Status.Roles.Reader, Writer: instance.Status.Roles.Writer, + }) + if err != nil { + // Connection errors can contain credentials. Store/log a bounded safe diagnostic. + status.Error = postgres.PermissionRepairError(err) + log.FromContext(ctx).Info("Permission repair failed", "reason", status.Error) + } else { + success := metav1.NewTime(r.repairNow()) + status.LastSuccessTime = &success + status.Error = "" + } + if err := r.Status().Patch(ctx, instance, client.MergeFromWithOptions(before, client.MergeFromWithOptimisticLock{})); err != nil { + return ctrl.Result{}, err + } + delay := status.NextRunTime.Sub(r.repairNow()) + if delay <= 0 { + delay = time.Second + } + return ctrl.Result{RequeueAfter: delay}, nil +} diff --git a/internal/controller/permission_repair_test.go b/internal/controller/permission_repair_test.go new file mode 100644 index 00000000..0ca63055 --- /dev/null +++ b/internal/controller/permission_repair_test.go @@ -0,0 +1,245 @@ +package controller + +import ( + "context" + "errors" + "strings" + "testing" + "time" + + db "github.com/movetokube/postgres-operator/api/v1alpha1" + "github.com/movetokube/postgres-operator/pkg/postgres" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +type repairSpy struct { + postgres.PG + calls int + request postgres.PermissionRepair + err error + timeout time.Duration +} + +func (s *repairSpy) RepairPermissions(ctx context.Context, p postgres.PermissionRepair) error { + s.calls++ + s.request = p + if deadline, ok := ctx.Deadline(); ok { + s.timeout = time.Until(deadline) + } + return s.err +} +func repairFixture(t *testing.T) (*PostgresReconciler, *db.Postgres, *repairSpy, *time.Time) { + t.Helper() + now := time.Date(2026, 9, 9, 0, 0, 0, 0, time.UTC) + instance := &db.Postgres{ObjectMeta: metav1.ObjectMeta{Name: "app", Namespace: "test", Finalizers: []string{"finalizer.db.movetokube.com"}}, + Spec: db.PostgresSpec{Database: "app-db", MasterRole: "app-owner", Schemas: []string{"public", "billing"}, PermissionRepair: &db.PermissionRepairSpec{Schedule: "0 2 * * *"}}, + Status: db.PostgresStatus{Succeeded: true, Schemas: []string{"public", "billing"}, Roles: db.PostgresRoles{Owner: "app-owner", Reader: "app-reader", Writer: "app-writer"}}} + scheme := runtime.NewScheme() + if err := db.AddToScheme(scheme); err != nil { + t.Fatal(err) + } + spy := &repairSpy{} + r := &PostgresReconciler{Client: fake.NewClientBuilder().WithScheme(scheme).WithStatusSubresource(instance).WithObjects(instance).Build(), pg: spy, now: func() time.Time { return now }} + return r, instance, spy, &now +} +func runRepair(t *testing.T, r *PostgresReconciler, p *db.Postgres) ctrl.Result { + t.Helper() + result, err := r.Reconcile(context.Background(), ctrl.Request{NamespacedName: types.NamespacedName{Name: p.Name, Namespace: p.Namespace}}) + if err != nil { + t.Fatal(err) + } + if err = r.Get(context.Background(), types.NamespacedName{Name: p.Name, Namespace: p.Namespace}, p); err != nil { + t.Fatal(err) + } + return result +} +func TestPermissionRepairSchedule(t *testing.T) { + now := time.Date(2026, 9, 9, 0, 0, 0, 0, time.UTC) + tests := []struct { + name string + spec db.PermissionRepairSpec + want string + bad bool + }{ + {"UTC", db.PermissionRepairSpec{Schedule: "0 2 * * *"}, "2026-09-09T02:00:00Z", false}, + {"steps", db.PermissionRepairSpec{Schedule: "*/15 * * * *"}, "2026-09-09T00:15:00Z", false}, + {"timezone override", db.PermissionRepairSpec{Schedule: "CRON_TZ=Europe/Sofia 0 2 * * *"}, "", true}, + {"short timezone override", db.PermissionRepairSpec{Schedule: "TZ=Europe/Sofia 2 * * *"}, "", true}, + {"six fields", db.PermissionRepairSpec{Schedule: "0 0 2 * * *"}, "", true}, + {"descriptor", db.PermissionRepairSpec{Schedule: "@daily"}, "", true}, + {"range", db.PermissionRepairSpec{Schedule: "65 2 * * *"}, "", true}, + {"impossible", db.PermissionRepairSpec{Schedule: "0 2 31 2 *"}, "", true}, + {"timeout", db.PermissionRepairSpec{Schedule: "0 2 * * *", Timeout: "0s"}, "", true}, + {"oversized timeout", db.PermissionRepairSpec{Schedule: "0 2 * * *", Timeout: "31m"}, "", true}, + {"window", db.PermissionRepairSpec{Schedule: "0 2 * * *", WindowDuration: "48h"}, "", true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + s, err := parseRepairSchedule(&tt.spec, now) + if (err != nil) != tt.bad { + t.Fatalf("error=%v", err) + } + if err == nil && s.Next(now).UTC().Format(time.RFC3339) != tt.want { + t.Fatalf("next=%v", s.Next(now)) + } + }) + } + +} +func TestPermissionRepairLifecycle(t *testing.T) { + r, p, spy, now := repairFixture(t) + result := runRepair(t, r, p) + if spy.calls != 0 || result.RequeueAfter != 2*time.Hour { + t.Fatalf("initial: calls=%d result=%v", spy.calls, result) + } + // A fresh reconciler uses persisted state, not an in-memory cron job. + r = &PostgresReconciler{Client: r.Client, pg: spy, now: r.now} + *now = now.Add(2*time.Hour + time.Minute) + runRepair(t, r, p) + status := p.Status.PermissionRepair + if spy.calls != 1 || status.LastSuccessTime == nil || status.Error != "" || !p.Status.Succeeded { + t.Fatalf("status=%+v calls=%d", status, spy.calls) + } + if spy.request.Owner != "app-owner" || len(spy.request.Schemas) != 2 || spy.timeout <= 0 || spy.timeout > 5*time.Minute { + t.Fatalf("request=%+v timeout=%v", spy.request, spy.timeout) + } + runRepair(t, r, p) + if spy.calls != 1 { + t.Fatal("status event repeated SQL") + } + // Failure preserves last success and provisioning state; retry is next cron occurrence. + success := status.LastSuccessTime.DeepCopy() + *now = status.NextRunTime.Time + spy.err = errors.New("postgresql://user:secret@host/db") + runRepair(t, r, p) + if spy.calls != 2 || !p.Status.Succeeded || !p.Status.PermissionRepair.LastSuccessTime.Equal(success) || strings.Contains(p.Status.PermissionRepair.Error, "secret") || p.Status.PermissionRepair.Error == "" { + t.Fatalf("failure status=%+v", p.Status) + } + runRepair(t, r, p) + if spy.calls != 2 { + t.Fatal("failure retried outside schedule") + } +} +func TestPermissionRepairMissedWindowAndChanges(t *testing.T) { + r, p, spy, now := repairFixture(t) + runRepair(t, r, p) + *now = now.Add(3 * time.Hour) + runRepair(t, r, p) + if spy.calls != 0 || p.Status.PermissionRepair.NextRunTime.Day() != 10 { + t.Fatal("missed window executed") + } + p.Spec.PermissionRepair.Schedule = "0 4 * * *" + if err := r.Update(context.Background(), p); err != nil { + t.Fatal(err) + } + runRepair(t, r, p) + if p.Status.PermissionRepair.NextRunTime.UTC().Hour() != 4 { + t.Fatalf("schedule change not applied: spec=%+v status=%+v", p.Spec.PermissionRepair, p.Status.PermissionRepair) + } + p.Spec.PermissionRepair.Schedule = "invalid" + if err := r.Update(context.Background(), p); err != nil { + t.Fatal(err) + } + result := runRepair(t, r, p) + if result.RequeueAfter != 0 || p.Status.PermissionRepair.Error == "" || spy.calls != 0 { + t.Fatal("invalid config not rejected") + } + // Removing schedule clears state (helper avoids intentional legacy schema grants). + p.Spec.PermissionRepair = nil + if _, err := r.reconcilePermissionRepair(context.Background(), p); err != nil { + t.Fatal(err) + } + if p.Status.PermissionRepair != nil { + t.Fatal("disabled schedule retained") + } +} +func TestPermissionRepairDeadlineAtWindowEnd(t *testing.T) { + r, p, spy, now := repairFixture(t) + runRepair(t, r, p) + *now = now.Add(2*time.Hour + 29*time.Minute) + runRepair(t, r, p) + if spy.timeout <= 0 || spy.timeout > time.Minute { + t.Fatalf("timeout=%v", spy.timeout) + } +} + +func TestPermissionRepairClaimConflict(t *testing.T) { + r, p, spy, now := repairFixture(t) + runRepair(t, r, p) + stale := p.DeepCopy() + *now = now.Add(2 * time.Hour) + if _, err := r.reconcilePermissionRepair(context.Background(), p); err != nil { + t.Fatal(err) + } + if _, err := r.reconcilePermissionRepair(context.Background(), stale); err == nil { + t.Fatal("stale claim should conflict") + } + if spy.calls != 1 { + t.Fatal("conflicting claim executed SQL") + } +} +func TestPermissionRepairExcludedResources(t *testing.T) { + t.Run("other instance", func(t *testing.T) { + r, p, spy, _ := repairFixture(t) + r.instanceFilter = "other" + runRepair(t, r, p) + if spy.calls != 0 || p.Status.PermissionRepair != nil { + t.Fatal("wrong instance scheduled") + } + }) + t.Run("deleted", func(t *testing.T) { + r, p, spy, _ := repairFixture(t) + if err := r.Delete(context.Background(), p); err != nil { + t.Fatal(err) + } + _, err := r.Reconcile(context.Background(), ctrl.Request{NamespacedName: types.NamespacedName{Name: p.Name, Namespace: p.Namespace}}) + if err != nil { + t.Fatal(err) + } + if spy.calls != 0 { + t.Fatal("deleted resource repaired") + } + }) +} + +// A host-local timezone or daylight-saving boundary must never move the UTC window. +func TestPermissionRepairAlwaysUTC(t *testing.T) { + previousLocal := time.Local + time.Local = time.FixedZone("operator-local", -7*60*60) + t.Cleanup(func() { time.Local = previousLocal }) + spec := &db.PermissionRepairSpec{Schedule: "0 2 * * *"} + for _, day := range []string{"2026-03-28", "2026-03-29", "2026-10-24", "2026-10-25"} { + t.Run(day, func(t *testing.T) { + midnight, err := time.Parse("2006-01-02", day) + if err != nil { + t.Fatal(err) + } + now := midnight.In(time.Local) + schedule, err := parseRepairSchedule(spec, now) + if err != nil { + t.Fatal(err) + } + if got := schedule.Next(now); !got.Equal(midnight.Add(2 * time.Hour)) { + t.Fatalf("next=%v, expected 02:00 UTC", got) + } + r, p, spy, clock := repairFixture(t) + *clock = now + if r.repairNow().Location() != time.UTC { + t.Fatal("controller clock is not UTC") + } + result := runRepair(t, r, p) + if result.RequeueAfter != 2*time.Hour || !p.Status.PermissionRepair.NextRunTime.Equal(&metav1.Time{Time: midnight.Add(2 * time.Hour)}) || spy.calls != 0 { + t.Fatalf("wrong UTC scheduling: %+v", p.Status.PermissionRepair) + } + *clock = now.Add(2 * time.Hour) + runRepair(t, r, p) + if spy.calls != 1 { + t.Fatal("repair did not run at 02:00 UTC") + } + }) + } +} diff --git a/internal/controller/postgres_controller.go b/internal/controller/postgres_controller.go index c19c2c43..dce1cc99 100644 --- a/internal/controller/postgres_controller.go +++ b/internal/controller/postgres_controller.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "slices" + "time" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" @@ -26,6 +27,7 @@ type PostgresReconciler struct { client.Client Scheme *runtime.Scheme pg postgres.PG + now func() time.Time // pgHost string instanceFilter string } @@ -131,6 +133,7 @@ func (r *PostgresReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c return ctrl.Result{Requeue: true}, err } + wasProvisioned := instance.Status.Succeeded // creation logic if !instance.Status.Succeeded { owner := instance.Spec.MasterRole @@ -232,53 +235,58 @@ func (r *PostgresReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c instance.Status.Schemas = append(instance.Status.Schemas, schema) } - // Set privileges on schemas during every reconcile to ensure privileges are correct - for _, schema := range instance.Spec.Schemas { + // Without a schedule preserve event-driven grants. With a schedule, grant + // during initial provisioning; subsequent repair is restricted to its window. + if instance.Spec.PermissionRepair == nil || !wasProvisioned { + // Set privileges on schemas during every reconcile to ensure privileges are correct + for _, schema := range instance.Spec.Schemas { - // Set privileges on schema - schemaPrivilegesReader := postgres.PostgresSchemaPrivileges{ - Owner: owner, - DB: database, - Role: reader, - Schema: schema, - Privs: readerPrivs, - CreateSchema: false, - } - err = r.pg.SetSchemaPrivileges(schemaPrivilegesReader) - if err != nil { - reqLogger.Error(err, fmt.Sprintf("Could not give %s permissions \"%s\"", reader, readerPrivs)) - continue - } - schemaPrivilegesWriter := postgres.PostgresSchemaPrivileges{ - Owner: owner, - DB: database, - Role: writer, - Schema: schema, - Privs: writerPrivs, - SequencePrivs: writerSequencePrivs, - FunctionPrivs: writerFunctionPrivs, - CreateSchema: true, - } - err = r.pg.SetSchemaPrivileges(schemaPrivilegesWriter) - if err != nil { - reqLogger.Error(err, fmt.Sprintf("Could not give %s permissions \"%s\", sequence privileges \"%s\", and function privileges \"%s\"", writer, writerPrivs, writerSequencePrivs, writerFunctionPrivs)) - continue - } - schemaPrivilegesOwner := postgres.PostgresSchemaPrivileges{ - Owner: owner, - DB: database, - Role: owner, - Schema: schema, - Privs: ownerPrivs, - SequencePrivs: ownerSequencePrivs, - FunctionPrivs: ownerFunctionPrivs, - CreateSchema: true, - } - err = r.pg.SetSchemaPrivileges(schemaPrivilegesOwner) - if err != nil { - reqLogger.Error(err, fmt.Sprintf("Could not give %s permissions \"%s\", sequence privileges \"%s\", and function privileges \"%s\"", owner, ownerPrivs, ownerSequencePrivs, ownerFunctionPrivs)) - continue + // Set privileges on schema + schemaPrivilegesReader := postgres.PostgresSchemaPrivileges{ + Owner: owner, + DB: database, + Role: reader, + Schema: schema, + Privs: readerPrivs, + CreateSchema: false, + } + err = r.pg.SetSchemaPrivileges(schemaPrivilegesReader) + if err != nil { + reqLogger.Error(err, fmt.Sprintf("Could not give %s permissions \"%s\"", reader, readerPrivs)) + continue + } + schemaPrivilegesWriter := postgres.PostgresSchemaPrivileges{ + Owner: owner, + DB: database, + Role: writer, + Schema: schema, + Privs: writerPrivs, + SequencePrivs: writerSequencePrivs, + FunctionPrivs: writerFunctionPrivs, + CreateSchema: true, + } + err = r.pg.SetSchemaPrivileges(schemaPrivilegesWriter) + if err != nil { + reqLogger.Error(err, fmt.Sprintf("Could not give %s permissions \"%s\", sequence privileges \"%s\", and function privileges \"%s\"", writer, writerPrivs, writerSequencePrivs, writerFunctionPrivs)) + continue + } + schemaPrivilegesOwner := postgres.PostgresSchemaPrivileges{ + Owner: owner, + DB: database, + Role: owner, + Schema: schema, + Privs: ownerPrivs, + SequencePrivs: ownerSequencePrivs, + FunctionPrivs: ownerFunctionPrivs, + CreateSchema: true, + } + err = r.pg.SetSchemaPrivileges(schemaPrivilegesOwner) + if err != nil { + reqLogger.Error(err, fmt.Sprintf("Could not give %s permissions \"%s\", sequence privileges \"%s\", and function privileges \"%s\"", owner, ownerPrivs, ownerSequencePrivs, ownerFunctionPrivs)) + continue + } } + } err = r.Status().Patch(ctx, instance, client.MergeFrom(before)) @@ -294,7 +302,7 @@ func (r *PostgresReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c } reqLogger.Info("Reconciling done") - return ctrl.Result{}, nil + return r.reconcilePermissionRepair(ctx, instance) } func (r *PostgresReconciler) addFinalizer(reqLogger logr.Logger, m *dbv1alpha1.Postgres) error { diff --git a/pkg/postgres/mock/postgres.go b/pkg/postgres/mock/postgres.go index 23cfdba8..a663abe7 100644 --- a/pkg/postgres/mock/postgres.go +++ b/pkg/postgres/mock/postgres.go @@ -3,13 +3,14 @@ // // Generated by this command: // -// mockgen -source pkg/postgres/postgres.go +// mockgen -source=pkg/postgres/postgres.go -destination=pkg/postgres/mock/postgres.go -package=mock // -// Package mock_postgres is a generated GoMock package. -package mock_postgres +// Package mock is a generated GoMock package. +package mock import ( + context "context" reflect "reflect" postgres "github.com/movetokube/postgres-operator/pkg/postgres" @@ -237,6 +238,20 @@ func (mr *MockPGMockRecorder) RenameGroupRole(currentRole, newRole any) *gomock. return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RenameGroupRole", reflect.TypeOf((*MockPG)(nil).RenameGroupRole), currentRole, newRole) } +// RepairPermissions mocks base method. +func (m *MockPG) RepairPermissions(arg0 context.Context, arg1 postgres.PermissionRepair) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "RepairPermissions", arg0, arg1) + ret0, _ := ret[0].(error) + return ret0 +} + +// RepairPermissions indicates an expected call of RepairPermissions. +func (mr *MockPGMockRecorder) RepairPermissions(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RepairPermissions", reflect.TypeOf((*MockPG)(nil).RepairPermissions), arg0, arg1) +} + // RevokeRole mocks base method. func (m *MockPG) RevokeRole(role, revoked string) error { m.ctrl.T.Helper() diff --git a/pkg/postgres/permission_repair.go b/pkg/postgres/permission_repair.go new file mode 100644 index 00000000..26410384 --- /dev/null +++ b/pkg/postgres/permission_repair.go @@ -0,0 +1,182 @@ +package postgres + +import ( + "context" + "database/sql" + "errors" + "fmt" + "net/url" + "strings" + + "github.com/lib/pq" +) + +// PermissionRepair uses existing roles and only the explicitly configured schemas. +type PermissionRepair struct { + Database, Owner, Reader, Writer string + Schemas []string +} + +// PermissionRepairError deliberately excludes connection strings and server detail. +func PermissionRepairError(err error) string { + if errors.Is(err, context.DeadlineExceeded) { + return "permission repair timed out" + } + if errors.Is(err, context.Canceled) { + return "permission repair canceled" + } + var pgErr *pq.Error + if errors.As(err, &pgErr) { + return "permission repair failed (SQLSTATE " + string(pgErr.Code) + ")" + } + return "permission repair failed; check database connectivity, configured schemas and owner privileges" +} + +func (c *pg) RepairPermissions(ctx context.Context, repair PermissionRepair) error { + // Use a URL encoder and PingContext: both connection setup and SQL are bounded. + uri := &url.URL{Scheme: "postgresql", User: url.UserPassword(c.user, c.pass), Host: c.host, Path: "/" + repair.Database, RawQuery: c.args} + db, err := sql.Open("postgres", uri.String()) + if err != nil { + return err + } + defer db.Close() + if err := db.PingContext(ctx); err != nil { + return err + } + return repairPermissions(ctx, db, repair) +} + +func repairPermissions(ctx context.Context, db *sql.DB, repair PermissionRepair) error { + if repair.Database == "" || repair.Owner == "" || repair.Reader == "" || repair.Writer == "" || len(repair.Schemas) == 0 { + return fmt.Errorf("database, all roles and at least one schema are required") + } + if repair.Owner == repair.Reader || repair.Owner == repair.Writer || repair.Reader == repair.Writer { + return fmt.Errorf("repair roles must be distinct") + } + for _, name := range append([]string{repair.Database, repair.Owner, repair.Reader, repair.Writer}, repair.Schemas...) { + if strings.ContainsRune(name, 0) { + return fmt.Errorf("identifiers cannot contain NUL") + } + } + for _, schema := range repair.Schemas { + if schema == "" || schema == "information_schema" || strings.HasPrefix(schema, "pg_") { + return fmt.Errorf("system or empty schema cannot be repaired") + } + } + tx, err := db.BeginTx(ctx, nil) + if err != nil { + return err + } + defer tx.Rollback() + // Transaction-scoped lock also prevents overlap between CRs referring to one database. + var locked bool + if err := tx.QueryRowContext(ctx, "SELECT pg_try_advisory_xact_lock(hashtextextended(current_database(), 716913))").Scan(&locked); err != nil { + return err + } + if !locked { + return fmt.Errorf("another permission repair is running") + } + q := pq.QuoteIdentifier + exec := func(statement string) error { _, err := tx.ExecContext(ctx, statement); return err } + if err := exec("SET LOCAL ROLE " + q(repair.Owner)); err != nil { + return err + } + // Fail instead of silently accepting GRANT warnings on objects belonging to another owner. + var invalid bool + if err := tx.QueryRowContext(ctx, repairOwnershipCheck, pq.Array(repair.Schemas)).Scan(&invalid); err != nil { + return err + } + if invalid { + return fmt.Errorf("configured schemas contain application objects owned by another role") + } + if err := exec(fmt.Sprintf("GRANT CONNECT ON DATABASE %s TO %s, %s", q(repair.Database), q(repair.Reader), q(repair.Writer))); err != nil { + return err + } + for _, schema := range repair.Schemas { + for _, statement := range []string{ + fmt.Sprintf("GRANT USAGE ON SCHEMA %s TO %s, %s", q(schema), q(repair.Reader), q(repair.Writer)), + fmt.Sprintf("GRANT CREATE ON SCHEMA %s TO %s", q(schema), q(repair.Writer)), + } { + if err := exec(statement); err != nil { + return err + } + } + } + // Enumerate objects to avoid extensions, internal routines and automatic types. + // Multiranges inherit range ACLs; arrays and table row types use their parent ACLs. + rows, err := tx.QueryContext(ctx, repairObjectGrants, pq.Array(repair.Schemas), repair.Reader, repair.Writer) + if err != nil { + return err + } + var statements []string + for rows.Next() { + var statement string + if err := rows.Scan(&statement); err != nil { + rows.Close() + return err + } + statements = append(statements, statement) + } + err = rows.Err() + rows.Close() + if err != nil { + return err + } + for _, statement := range statements { + if err := exec(statement); err != nil { + return err + } + } + // Only the stable owner creates migration objects. Do not change administrator defaults. + for _, schema := range repair.Schemas { + for _, grant := range []struct{ objects, privileges, role string }{ + {"TABLES", "SELECT", repair.Reader}, {"TABLES", "SELECT, INSERT, UPDATE, DELETE", repair.Writer}, + {"SEQUENCES", "USAGE, SELECT", repair.Writer}, {"ROUTINES", "EXECUTE", repair.Writer}, + {"TYPES", "USAGE", repair.Reader}, {"TYPES", "USAGE", repair.Writer}, + } { + if err := exec(fmt.Sprintf("ALTER DEFAULT PRIVILEGES FOR ROLE %s IN SCHEMA %s GRANT %s ON %s TO %s", q(repair.Owner), q(schema), grant.privileges, grant.objects, q(grant.role))); err != nil { + return err + } + } + } + // Large objects are database-local, not schema-local. Their defaults require PG18. + var version int + if err := tx.QueryRowContext(ctx, "SELECT current_setting('server_version_num')::int").Scan(&version); err != nil { + return err + } + if version >= 180000 { + if err := exec(fmt.Sprintf("ALTER DEFAULT PRIVILEGES FOR ROLE %s GRANT SELECT ON LARGE OBJECTS TO %s", q(repair.Owner), q(repair.Reader))); err != nil { + return err + } + if err := exec(fmt.Sprintf("ALTER DEFAULT PRIVILEGES FOR ROLE %s GRANT SELECT, UPDATE ON LARGE OBJECTS TO %s", q(repair.Owner), q(repair.Writer))); err != nil { + return err + } + } + return tx.Commit() +} + +// System and extension objects retain their own privilege policy. Non-extension +// application objects must belong to the stable owner before adoption is repaired. +const repairOwnershipCheck = `SELECT EXISTS ( + SELECT 1 FROM ( + SELECT 'pg_class'::regclass AS classid, c.oid, c.relowner AS owner FROM pg_class c JOIN pg_namespace n ON n.oid=c.relnamespace WHERE n.nspname=ANY($1) AND c.relkind IN ('r','p','v','m','f','S') + UNION ALL SELECT 'pg_proc'::regclass, p.oid, p.proowner FROM pg_proc p JOIN pg_namespace n ON n.oid=p.pronamespace WHERE n.nspname=ANY($1) + UNION ALL SELECT 'pg_type'::regclass, t.oid, t.typowner FROM pg_type t JOIN pg_namespace n ON n.oid=t.typnamespace LEFT JOIN pg_class c ON c.oid=t.typrelid WHERE n.nspname=ANY($1) AND t.typisdefined AND t.typtype NOT IN ('p','m') AND (t.typrelid=0 OR c.relkind='c') AND NOT EXISTS (SELECT 1 FROM pg_type a WHERE a.typarray=t.oid) + ) obj WHERE owner<>current_user::regrole AND NOT EXISTS (SELECT 1 FROM pg_depend d WHERE d.classid=obj.classid AND d.objid=obj.oid AND (d.deptype='e' OR (obj.classid='pg_proc'::regclass AND d.deptype='i'))) +) OR EXISTS (SELECT 1 FROM pg_database WHERE datname=current_database() AND datdba<>current_user::regrole) OR EXISTS (SELECT 1 FROM pg_namespace WHERE nspname=ANY($1) AND nspowner NOT IN (current_user::regrole, 'pg_database_owner'::regrole))` + +const repairObjectGrants = `SELECT statement FROM ( + SELECT format('GRANT SELECT ON TABLE %I.%I TO %I; GRANT SELECT, INSERT, UPDATE, DELETE ON TABLE %I.%I TO %I', n.nspname,c.relname,$2::text,n.nspname,c.relname,$3::text) AS statement + FROM pg_class c JOIN pg_namespace n ON n.oid=c.relnamespace + WHERE n.nspname=ANY($1) AND c.relowner=current_user::regrole AND c.relkind IN ('r','p','v','m','f') AND NOT EXISTS (SELECT 1 FROM pg_depend d WHERE d.classid='pg_class'::regclass AND d.objid=c.oid AND d.deptype='e') + UNION ALL SELECT format('GRANT USAGE, SELECT ON SEQUENCE %I.%I TO %I',n.nspname,c.relname,$3::text) + FROM pg_class c JOIN pg_namespace n ON n.oid=c.relnamespace + WHERE n.nspname=ANY($1) AND c.relowner=current_user::regrole AND c.relkind='S' AND NOT EXISTS (SELECT 1 FROM pg_depend d WHERE d.classid='pg_class'::regclass AND d.objid=c.oid AND d.deptype='e') + UNION ALL SELECT format('GRANT EXECUTE ON ROUTINE %I.%I(%s) TO %I',n.nspname,p.proname,pg_get_function_identity_arguments(p.oid),$3::text) + FROM pg_proc p JOIN pg_namespace n ON n.oid=p.pronamespace + WHERE n.nspname=ANY($1) AND p.proowner=current_user::regrole AND NOT EXISTS (SELECT 1 FROM pg_depend d WHERE d.classid='pg_proc'::regclass AND d.objid=p.oid AND d.deptype IN ('e','i')) + UNION ALL SELECT format('GRANT USAGE ON TYPE %I.%I TO %I, %I',n.nspname,t.typname,$2::text,$3::text) + FROM pg_type t JOIN pg_namespace n ON n.oid=t.typnamespace LEFT JOIN pg_class c ON c.oid=t.typrelid + WHERE n.nspname=ANY($1) AND t.typowner=current_user::regrole AND t.typisdefined AND t.typtype NOT IN ('p','m') AND (t.typrelid=0 OR c.relkind='c') AND NOT EXISTS (SELECT 1 FROM pg_type a WHERE a.typarray=t.oid) AND NOT EXISTS (SELECT 1 FROM pg_depend d WHERE d.classid='pg_type'::regclass AND d.objid=t.oid AND d.deptype='e') + UNION ALL SELECT format('GRANT SELECT ON LARGE OBJECT %s TO %I; GRANT SELECT, UPDATE ON LARGE OBJECT %s TO %I',oid,$2::text,oid,$3::text) FROM pg_largeobject_metadata WHERE lomowner=current_user::regrole +) grants ORDER BY statement` diff --git a/pkg/postgres/permission_repair_integration_test.go b/pkg/postgres/permission_repair_integration_test.go new file mode 100644 index 00000000..bdfbbdac --- /dev/null +++ b/pkg/postgres/permission_repair_integration_test.go @@ -0,0 +1,214 @@ +package postgres + +import ( + "context" + "database/sql" + "fmt" + "net/url" + "os" + "strings" + "testing" + "time" + + "github.com/lib/pq" +) + +// Run only against a disposable PostgreSQL server; this test creates and drops its own database/roles. +func TestPermissionRepairIntegration(t *testing.T) { + dsn := os.Getenv("PERMISSION_REPAIR_TEST_DSN") + if dsn == "" { + t.Skip("set PERMISSION_REPAIR_TEST_DSN to a disposable PostgreSQL server") + } + admin, err := sql.Open("postgres", dsn) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { admin.Close() }) + suffix := fmt.Sprintf("%d", time.Now().UnixNano()) + database := "repair_" + suffix + owner := "owner_\"" + suffix + reader := "reader_" + suffix + writer := "writer_" + suffix + q := pq.QuoteIdentifier + mustExec := func(db *sql.DB, query string) { + t.Helper() + if _, err := db.Exec(query); err != nil { + t.Fatal(err) + } + } + for _, role := range []string{owner, reader, writer} { + mustExec(admin, "CREATE ROLE "+q(role)) + } + t.Cleanup(func() { + if _, err := admin.Exec("DROP DATABASE IF EXISTS " + q(database) + " WITH (FORCE)"); err != nil { + t.Error(err) + } + for _, role := range []string{owner, reader, writer} { + if _, err := admin.Exec("DROP ROLE " + q(role)); err != nil { + t.Error(err) + } + } + }) + mustExec(admin, "CREATE DATABASE "+q(database)+" OWNER "+q(owner)) + uri, err := url.Parse(dsn) + if err != nil { + t.Fatal(err) + } + uri.Path = "/" + database + target, err := sql.Open("postgres", uri.String()) + if err != nil { + t.Fatal(err) + } + defer target.Close() + target.SetMaxOpenConns(3) + schema := "billing\"items" + mustExec(target, "CREATE EXTENSION hstore; CREATE EXTENSION postgres_fdw; CREATE SERVER dummy FOREIGN DATA WRAPPER postgres_fdw") + mustExec(target, "GRANT USAGE ON FOREIGN SERVER dummy TO "+q(owner)) + mustExec(target, "SET ROLE "+q(owner)+`; CREATE SCHEMA `+q(schema)+`; CREATE SCHEMA private; + CREATE TABLE public.existing(id int, value text); + CREATE TABLE public.partitioned(id int) PARTITION BY RANGE(id); + CREATE TABLE public.partition_1 PARTITION OF public.partitioned FOR VALUES FROM (0) TO (10); + CREATE VIEW public.a_view AS SELECT * FROM public.existing; + CREATE MATERIALIZED VIEW public.a_matview AS SELECT * FROM public.existing; + CREATE SEQUENCE public.counter; + CREATE FUNCTION public.work(i int) RETURNS int LANGUAGE sql AS 'SELECT i'; + CREATE FUNCTION public.work(i text) RETURNS text LANGUAGE sql AS 'SELECT i'; + CREATE PROCEDURE public.proc() LANGUAGE sql AS 'SELECT 1'; + CREATE TYPE public.mood AS ENUM ('ok'); + CREATE DOMAIN public.positive AS int CHECK(VALUE>0); + CREATE TYPE public.pair AS (a int,b text); + CREATE TYPE public.custom_range AS RANGE (subtype=integer); + CREATE FOREIGN TABLE public.foreign_t(id int) SERVER dummy; + CREATE TABLE `+q(schema)+`.other(id int); + CREATE TABLE private.hidden(id int); + REVOKE ALL ON ALL ROUTINES IN SCHEMA public FROM PUBLIC; + ALTER DEFAULT PRIVILEGES REVOKE EXECUTE ON ROUTINES FROM PUBLIC; + ALTER DEFAULT PRIVILEGES REVOKE USAGE ON TYPES FROM PUBLIC; + RESET ROLE;`) + // Preserve custom grants, while repairing only the expected baseline. + mustExec(target, "GRANT TRUNCATE ON public.existing TO "+q(writer)) + createLO := func() uint32 { + t.Helper() + tx, err := target.Begin() + if err != nil { + t.Fatal(err) + } + defer tx.Rollback() + if _, err = tx.Exec("SET LOCAL ROLE " + q(owner)); err != nil { + t.Fatal(err) + } + var oid uint32 + if err = tx.QueryRow("SELECT lo_create(0)").Scan(&oid); err != nil { + t.Fatal(err) + } + if err = tx.Commit(); err != nil { + t.Fatal(err) + } + return oid + } + lo := createLO() + repair := PermissionRepair{Database: database, Owner: owner, Reader: reader, Writer: writer, Schemas: []string{"public", schema}} + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + password, _ := uri.User.Password() + pgClient := &pg{host: uri.Host, user: uri.User.Username(), pass: password, args: uri.RawQuery} + if err = pgClient.RepairPermissions(ctx, repair); err != nil { + t.Fatal(err) + } + // Idempotent repair and custom grants must survive repeated runs. + if err = repairPermissions(ctx, target, repair); err != nil { + t.Fatal(err) + } + check := func(query string, want bool, args ...any) { + t.Helper() + var got bool + if err := target.QueryRow(query, args...).Scan(&got); err != nil { + t.Fatal(err) + } + if got != want { + t.Fatalf("%s args=%v: got %v want %v", query, args, got, want) + } + } + for _, table := range []string{"public.existing", "public.partitioned", "public.partition_1", "public.a_view", "public.a_matview", "public.foreign_t", q(schema) + ".other"} { + check("SELECT has_table_privilege($1,$2,'SELECT')", true, reader, table) + for _, priv := range []string{"SELECT", "INSERT", "UPDATE", "DELETE"} { + check("SELECT has_table_privilege($1,$2,$3)", true, writer, table, priv) + } + check("SELECT has_table_privilege($1,$2,'INSERT')", false, reader, table) + } + check("SELECT has_table_privilege($1,'public.existing','TRUNCATE')", true, writer) + check("SELECT has_table_privilege($1,'private.hidden','SELECT')", false, reader) + check("SELECT has_sequence_privilege($1,'public.counter','USAGE')", true, writer) + check("SELECT has_sequence_privilege($1,'public.counter','UPDATE')", false, writer) + for _, routine := range []string{"public.work(integer)", "public.work(text)", "public.proc()"} { + check("SELECT has_function_privilege($1,$2,'EXECUTE')", true, writer, routine) + check("SELECT has_function_privilege($1,$2,'EXECUTE')", false, reader, routine) + } + for _, typ := range []string{"public.mood", "public.positive", "public.pair", "public.custom_range", "public.custom_multirange"} { + check("SELECT has_type_privilege($1,$2,'USAGE')", true, reader, typ) + } + check("SELECT EXISTS (SELECT 1 FROM pg_largeobject_metadata l, LATERAL aclexplode(l.lomacl) a WHERE l.oid=$2 AND a.grantee=$1::regrole AND a.privilege_type='SELECT')", true, reader, lo) + check("SELECT EXISTS (SELECT 1 FROM pg_largeobject_metadata l, LATERAL aclexplode(l.lomacl) a WHERE l.oid=$2 AND a.grantee=$1::regrole AND a.privilege_type='UPDATE')", true, writer, lo) + // Normal provisioning must also repair the stable owner's defaults. + mustExec(target, "ALTER DEFAULT PRIVILEGES FOR ROLE "+q(owner)+" IN SCHEMA public REVOKE SELECT ON TABLES FROM "+q(reader)) + if err := pgClient.SetSchemaPrivileges(PostgresSchemaPrivileges{DB: database, Owner: owner, Role: reader, Schema: "public", Privs: "SELECT"}); err != nil { + t.Fatal(err) + } + // Defaults must be on the owner, even though repair was invoked through an admin connection. + mustExec(target, "SET ROLE "+q(owner)+`; CREATE TABLE public.future(id int); CREATE SEQUENCE public.future_seq; + CREATE PROCEDURE public.future_proc() LANGUAGE sql AS 'SELECT 1'; CREATE TYPE public.future_type AS ENUM ('new'); RESET ROLE;`) + check("SELECT has_table_privilege($1,'public.future','SELECT')", true, reader) + check("SELECT has_table_privilege($1,'public.future','UPDATE')", true, writer) + check("SELECT has_sequence_privilege($1,'public.future_seq','USAGE')", true, writer) + check("SELECT has_function_privilege($1,'public.future_proc()','EXECUTE')", true, writer) + check("SELECT has_type_privilege($1,'public.future_type','USAGE')", true, reader) + var version int + if err = target.QueryRow("SHOW server_version_num").Scan(&version); err != nil { + t.Fatal(err) + } + if version >= 180000 { + check("SELECT EXISTS (SELECT 1 FROM pg_largeobject_metadata l, LATERAL aclexplode(l.lomacl) a WHERE l.oid=$2 AND a.grantee=$1::regrole AND a.privilege_type='SELECT')", true, reader, createLO()) + } + // Explicit drift repair. + mustExec(target, "REVOKE SELECT ON public.existing FROM "+q(reader)+"; REVOKE USAGE ON public.counter FROM "+q(writer)) + if err = repairPermissions(ctx, target, repair); err != nil { + t.Fatal(err) + } + check("SELECT has_table_privilege($1,'public.existing','SELECT')", true, reader) + check("SELECT has_sequence_privilege($1,'public.counter','USAGE')", true, writer) + // A second CR/operator cannot overlap while the transaction lock is held. + tx, err := target.Begin() + if err != nil { + t.Fatal(err) + } + if _, err = tx.Exec("SELECT pg_advisory_xact_lock(hashtextextended(current_database(),716913))"); err != nil { + t.Fatal(err) + } + err = repairPermissions(ctx, target, repair) + tx.Rollback() + if err == nil || !strings.Contains(err.Error(), "another permission repair") { + t.Fatalf("lock: %v", err) + } + // A failed transaction cannot leave grants on earlier objects committed. + mustExec(target, "REVOKE SELECT ON public.existing, public.future FROM "+q(reader)) + tx, err = target.Begin() + if err != nil { + t.Fatal(err) + } + if _, err = tx.Exec("SELECT oid FROM pg_class WHERE oid = 'public.future'::regclass FOR UPDATE"); err != nil { + t.Fatal(err) + } + short, cancelShort := context.WithTimeout(context.Background(), 100*time.Millisecond) + err = repairPermissions(short, target, repair) + cancelShort() + tx.Rollback() + if err == nil { + t.Fatal("expected timeout") + } + check("SELECT has_table_privilege($1,'public.existing','SELECT')", false, reader) + // Wrong ownership must not be silently treated as repaired. + mustExec(target, "CREATE TABLE public.not_adopted(id int)") + if err = repairPermissions(ctx, target, repair); err == nil { + t.Fatal("expected ownership failure") + } +} diff --git a/pkg/postgres/permission_repair_test.go b/pkg/postgres/permission_repair_test.go new file mode 100644 index 00000000..c2f9b47f --- /dev/null +++ b/pkg/postgres/permission_repair_test.go @@ -0,0 +1,56 @@ +package postgres + +import ( + "context" + "errors" + "strings" + "testing" + + "github.com/DATA-DOG/go-sqlmock" + "github.com/lib/pq" +) + +func TestPermissionRepairValidation(t *testing.T) { + cases := []PermissionRepair{ + {}, {Database: "db", Owner: "o", Reader: "r", Writer: "w"}, + {Database: "db", Owner: "o", Reader: "o", Writer: "w", Schemas: []string{"public"}}, + {Database: "db", Owner: "o", Reader: "r", Writer: "w", Schemas: []string{"pg_catalog"}}, + {Database: "db", Owner: "o", Reader: "r", Writer: "w", Schemas: []string{"information_schema"}}, + {Database: "db", Owner: "o", Reader: "r", Writer: "w", Schemas: []string{"pg_temp_2"}}, + {Database: "db", Owner: "o", Reader: "r", Writer: "w", Schemas: []string{""}}, + {Database: "db", Owner: "o\x00", Reader: "r", Writer: "w", Schemas: []string{"public"}}, + } + for _, p := range cases { + if err := repairPermissions(context.Background(), nil, p); err == nil { + t.Fatalf("accepted %+v", p) + } + } +} +func TestPermissionRepairErrorsAreSafe(t *testing.T) { + for _, err := range []error{errors.New("postgresql://admin:secret@example/db"), &pq.Error{Code: "42501", Message: "secret"}, context.Canceled, context.DeadlineExceeded} { + got := PermissionRepairError(err) + if got == "" || strings.Contains(got, "secret") { + t.Fatalf("unsafe diagnostic %q", got) + } + } +} +func TestPermissionRepairRollbackOnGrantFailure(t *testing.T) { + db, m, err := sqlmock.New() + if err != nil { + t.Fatal(err) + } + defer db.Close() + m.ExpectBegin() + m.ExpectQuery("SELECT pg_try_advisory").WillReturnRows(sqlmock.NewRows([]string{"locked"}).AddRow(true)) + m.ExpectExec("SET LOCAL ROLE").WillReturnResult(sqlmock.NewResult(0, 0)) + m.ExpectQuery("SELECT EXISTS").WillReturnRows(sqlmock.NewRows([]string{"invalid"}).AddRow(false)) + m.ExpectExec("GRANT CONNECT").WillReturnResult(sqlmock.NewResult(0, 0)) + m.ExpectExec("GRANT USAGE ON SCHEMA").WillReturnError(&pq.Error{Code: "42501"}) + m.ExpectRollback() + if err := repairPermissions(context.Background(), db, PermissionRepair{Database: "db", Owner: "o", Reader: "r", Writer: "w", Schemas: []string{"public"}}); err == nil { + t.Fatal("failure reported as success") + } + if err := m.ExpectationsWereMet(); err != nil { + t.Fatal(err) + } +} diff --git a/pkg/postgres/postgres.go b/pkg/postgres/postgres.go index 4ac60f09..ab8d51c6 100644 --- a/pkg/postgres/postgres.go +++ b/pkg/postgres/postgres.go @@ -1,6 +1,7 @@ package postgres import ( + "context" "database/sql" "fmt" @@ -9,6 +10,7 @@ import ( ) type PG interface { + RepairPermissions(context.Context, PermissionRepair) error CreateDB(dbname, username string) error CreateSchema(db, role, schema string) error CreateExtension(db, extension string) error From dd8d994fceba0196f5adac5c115651b832c82563 Mon Sep 17 00:00:00 2001 From: TK Date: Thu, 10 Sep 2026 12:30:11 +0300 Subject: [PATCH 10/14] docs: update and format REDME.md --- README.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 2e65c44c..52ad7822 100644 --- a/README.md +++ b/README.md @@ -272,11 +272,12 @@ metadata: # use this to target which instance of operator should process this CR. See general config postgres.db.movetokube.com/instance: POSTGRES_INSTANCE spec: + # Omitting the entire field disables scheduled repair. # Keep the existing database, masterRole and schema configuration. - permissionRepair: - schedule: "0 2 * * *" # Five cron fields; every day at 02:00 UTC - windowDuration: "30m" # Latest allowed start/end; defaults to 30m - timeout: "5m" # Maximum transaction duration; defaults to 5m + # permissionRepair: + # schedule: "0 2 * * *" # Five cron fields; every day at 02:00 UTC + # windowDuration: "30m" # Latest allowed start/end; defaults to 30m + # timeout: "5m" # Maximum transaction duration; defaults to 5m role: username database: my-db # This references the Postgres CR secretName: my-secret From 5bd99a609d0553280eee45fc35d63c2aec3e850c Mon Sep 17 00:00:00 2001 From: TK Date: Thu, 10 Sep 2026 12:34:35 +0300 Subject: [PATCH 11/14] feat: Update Helm-Chart Version and App Version --- 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 499f38d3..7359664a 100644 --- a/charts/ext-postgres-operator/Chart.yaml +++ b/charts/ext-postgres-operator/Chart.yaml @@ -8,5 +8,5 @@ description: | type: application -version: 3.0.0 -appVersion: "2.5.0" +version: 3.1.0 +appVersion: "2.6.0" From dcb76a2d483b4fb8998a3f60e5246d36b8568bf0 Mon Sep 17 00:00:00 2001 From: TK Date: Thu, 10 Sep 2026 13:27:27 +0300 Subject: [PATCH 12/14] docs: update and format REDME.md --- README.md | 143 +++++++++++---------------------------- docs/permissionRepair.md | 85 +++++++++++++++++++++++ 2 files changed, 126 insertions(+), 102 deletions(-) create mode 100644 docs/permissionRepair.md diff --git a/README.md b/README.md index 52ad7822..985744e1 100644 --- a/README.md +++ b/README.md @@ -18,6 +18,7 @@ Manage external PostgreSQL databases in Kubernetes with ease—supporting AWS RD - [Multiple Operator Support](#multiple-operator-support) - [Secret Templating](#secret-templating) - [Compatibility](#compatibility) +- [Additional Features](#additional-features) - [Contributing](#contributing) - [License](#license) @@ -40,29 +41,6 @@ _None yet. [Become a sponsor!](https://github.com/sponsors/hitman99)_ - Handles CRs in dynamically created namespaces - Customizable secret values using templates -## AWS Specific Features when cloud_provider: "AWS" - -- Enable IAM authentication for this user (PostgreSQL on AWS RDS only) - - ````yaml - kind: PostgresUser - .... - spec: - aws: - enableIamAuth: false # (by Default false) - ``` - ```` - -- AWS PG_repack extension installation / properly alter privileges for the owner user if cloud_provider: "AWS" - - ```yaml - kind: Postgres - --- - spec: - extensions: - - pg_repack - ``` - --- ## Supported Cloud Providers @@ -186,79 +164,6 @@ spec: This creates a database called `test-db` and a role `test-db-group` that is set as the owner of the database. Reader and writer roles are also created. These roles have read and write permissions to all tables in the schemas created by the operator, if any. -### Scheduled permission repair - -Permission repair is optional per `Postgres`. It adds missing grants; it never -revokes custom grants, changes ownership, recreates roles or rotates credentials. - -```yaml -spec: - # Keep the existing database, masterRole and schema configuration. - permissionRepair: - schedule: "0 2 * * *" # Five cron fields; every day at 02:00 UTC - windowDuration: "30m" # Latest allowed start/end; defaults to 30m - timeout: "5m" # Maximum transaction duration; defaults to 5m -``` - -The schedule accepts five-field cron syntax (including lists, ranges and steps), -without seconds or `@daily`-style shortcuts. Schedules always use UTC, regardless -of the operator host timezone or daylight-saving changes. Timezone overrides in -the cron expression are rejected. `windowDuration` must be positive and at most `24h`; `timeout` must be -positive and no longer than the window. Invalid configuration is reported in -`status.permissionRepair.error` and no scheduled repair runs. - -With a schedule configured, initial provisioning still applies normal grants. -Subsequent permission repair runs only in its window, using `spec.schemas` and the -existing roles in `status.roles`. Keep the full list of application schemas in -`spec.schemas`; system schemas are rejected. Removing `permissionRepair` restores -the original event-driven schema-grant behavior. - -Repair covers database `CONNECT`, schema `USAGE` (and writer `CREATE`), tables, -partitions, views, materialized views, foreign tables, sequences, functions, -procedures, types/domains and owner-managed large objects. Reader gets table/large -object `SELECT` and type `USAGE`. Writer gets table CRUD, sequence `USAGE, SELECT`, -routine `EXECUTE`, type `USAGE`, and large-object `SELECT, UPDATE`. It does not grant -reader execution of routines or writer `TRUNCATE`/ownership rights. - -Existing application objects must belong to the stable owner. Extension-managed objects and PostgreSQL-generated internal routines (such as -range constructors) retain their existing policy. RLS policies, foreign-server credentials -and user mappings are not repaired. Large objects are scoped by owner in the -current database because they do not belong to schemas. - -Default privileges target the stable owner explicitly, in each configured schema. -They therefore apply to migration objects created as that role, not objects -created as an administrator or another login/group. Large-object defaults require -PostgreSQL 18; earlier servers receive existing large-object grants only. New -schemas must be added to `spec.schemas`. Reconcile extensions through their normal -configuration. - -The controller persists `nextRunTime`, `lastAttemptTime`, `lastSuccessTime` and -`error` in `status.permissionRepair`. Provisioning `status.succeeded` stays separate -from repair failures. Enabling/changing the schedule (or its database/schema/role -scope) schedules the next future occurrence. After a restart, a pending occurrence -runs only if still inside its window; expired occurrences are skipped. Each -occurrence is claimed before SQL, attempted at most once, and failures wait until -the next occurrence. A crash after claiming can skip that attempt; interrupted -attempts remain visible in status. This avoids immediate retry loops and repairs -outside maintenance windows. - -Repairs run in a transaction, use cancellation/timeouts capped at the window end, -and take a PostgreSQL advisory lock to prevent concurrent repairs of the same -database. Run the operator with its default leader election enabled. Error status -never includes connection strings or passwords. - -Upgrade the installed CRD as well as the controller before configuring the new -fields. Helm does not automatically upgrade CRDs from a chart's `crds/` directory. - -To run the real privilege/rollback tests against a **disposable** PostgreSQL server: - -```bash -PERMISSION_REPAIR_TEST_DSN='postgresql://postgres@127.0.0.1:55439/postgres?sslmode=disable' go test ./pkg/postgres -run TestPermissionRepairIntegration -count=1 -``` - -The integration test creates and removes a temporary database and roles. CI runs -it on PostgreSQL 16 and 18; scheduling/controller tests require no live cluster. - ### PostgresUser ```yaml @@ -272,12 +177,6 @@ metadata: # use this to target which instance of operator should process this CR. See general config postgres.db.movetokube.com/instance: POSTGRES_INSTANCE spec: - # Omitting the entire field disables scheduled repair. - # Keep the existing database, masterRole and schema configuration. - # permissionRepair: - # schedule: "0 2 * * *" # Five cron fields; every day at 02:00 UTC - # windowDuration: "30m" # Latest allowed start/end; defaults to 30m - # timeout: "5m" # Maximum transaction duration; defaults to 5m role: username database: my-db # This references the Postgres CR secretName: my-secret @@ -355,6 +254,46 @@ Postgres operator compatibility with Operator SDK version is in the table below | `postgres-operator 2.x.x` | v1.39 | v1 | | `HEAD` | v1.39 | v1 | +## Additional Features + +### AWS specific features (`cloud_provider: "AWS"`) + +- Enable IAM authentication for this user (PostgreSQL on AWS RDS only) + + ```yaml + kind: PostgresUser + .... + spec: + aws: + enableIamAuth: false # (by Default false) + ``` + +- AWS `pg_repack` extension installation / properly alter privileges for the owner user if `cloud_provider: "AWS"` + + ```yaml + kind: Postgres + --- + spec: + extensions: + - pg_repack + ``` + +### Scheduled permission repair + +Permission repair is optional per `Postgres`. It adds missing grants; it never +revokes custom grants, changes ownership, recreates roles or rotates credentials. +See [docs/permissionRepair.md](docs/permissionRepair.md) for the full reference. + +```yaml +spec: + # Keep the existing database, masterRole and schema configuration. + permissionRepair: + schedule: "0 2 * * *" # Five cron fields; every day at 02:00 UTC + timeZone: "Europe/Sofia" # IANA zone; defaults to UTC + windowDuration: "30m" # Latest allowed start/end; defaults to 30m + timeout: "5m" # Maximum transaction duration; defaults to 5m +``` + ## Contributing See [CONTRIBUTING.md](CONTRIBUTING.md) diff --git a/docs/permissionRepair.md b/docs/permissionRepair.md new file mode 100644 index 00000000..6b09ee4c --- /dev/null +++ b/docs/permissionRepair.md @@ -0,0 +1,85 @@ +# Scheduled permission repair + +Permission repair is optional per `Postgres`. It adds missing grants; it never +revokes custom grants, changes ownership, recreates roles or rotates credentials. + +What is covered by permissionRepair: + +- All application schemas. +- Tables, partitions, views, materialized views, and foreign tables. +- Sequences. +- Functions and procedures. +- Owner-managed types, domains, and large objects. +- Default privileges for future objects created by the owner role. + +Note: Time format is UTC + +This is part of Postgres object spec + +```yaml +spec: + # Keep the existing database, masterRole and schema configuration. + permissionRepair: + schedule: "0 2 * * *" # Five cron fields; every day at 02:00 UTC + windowDuration: "30m" # Latest allowed start/end; defaults to 30m + timeout: "5m" # Maximum transaction duration; defaults to 5m +``` + +The schedule accepts five-field cron syntax (including lists, ranges and steps), +without seconds or `@daily`-style shortcuts. Schedules always use UTC, regardless +of the operator host timezone or daylight-saving changes. Timezone overrides in +the cron expression are rejected. `windowDuration` must be positive and at most `24h`; `timeout` must be +positive and no longer than the window. Invalid configuration is reported in +`status.permissionRepair.error` and no scheduled repair runs. + +With a schedule configured, initial provisioning still applies normal grants. +Subsequent permission repair runs only in its window, using `spec.schemas` and the +existing roles in `status.roles`. Keep the full list of application schemas in +`spec.schemas`; system schemas are rejected. Removing `permissionRepair` restores +the original event-driven schema-grant behavior. + +Repair covers database `CONNECT`, schema `USAGE` (and writer `CREATE`), tables, +partitions, views, materialized views, foreign tables, sequences, functions, +procedures, types/domains and owner-managed large objects. Reader gets table/large +object `SELECT` and type `USAGE`. Writer gets table CRUD, sequence `USAGE, SELECT`, +routine `EXECUTE`, type `USAGE`, and large-object `SELECT, UPDATE`. It does not grant +reader execution of routines or writer `TRUNCATE`/ownership rights. + +Existing application objects must belong to the stable owner. Extension-managed objects and PostgreSQL-generated internal routines (such as +range constructors) retain their existing policy. RLS policies, foreign-server credentials +and user mappings are not repaired. Large objects are scoped by owner in the +current database because they do not belong to schemas. + +Default privileges target the stable owner explicitly, in each configured schema. +They therefore apply to migration objects created as that role, not objects +created as an administrator or another login/group. Large-object defaults require +PostgreSQL 18; earlier servers receive existing large-object grants only. New +schemas must be added to `spec.schemas`. Reconcile extensions through their normal +configuration. + +The controller persists `nextRunTime`, `lastAttemptTime`, `lastSuccessTime` and +`error` in `status.permissionRepair`. Provisioning `status.succeeded` stays separate +from repair failures. Enabling/changing the schedule (or its database/schema/role +scope) schedules the next future occurrence. After a restart, a pending occurrence +runs only if still inside its window; expired occurrences are skipped. Each +occurrence is claimed before SQL, attempted at most once, and failures wait until +the next occurrence. A crash after claiming can skip that attempt; interrupted +attempts remain visible in status. This avoids immediate retry loops and repairs +outside maintenance windows. + +Repairs run in a transaction, use cancellation/timeouts capped at the window end, +and take a PostgreSQL advisory lock to prevent concurrent repairs of the same +database. Run the operator with its default leader election enabled. Error status +never includes connection strings or passwords. + +Upgrade the installed CRD as well as the controller before configuring the new +fields. Helm does not automatically upgrade CRDs from a chart's `crds/` directory. + +To run the real privilege/rollback tests against a **disposable** PostgreSQL server: + +```bash +PERMISSION_REPAIR_TEST_DSN='postgresql://postgres@127.0.0.1:55439/postgres?sslmode=disable' go test ./pkg/postgres -run TestPermissionRepairIntegration -count=1 +``` + +The integration test creates and removes a temporary database and roles. CI runs +it on PostgreSQL 16 and 18; scheduling/controller tests require no live cluster. From 40983385e4759dcfb311f6049eac53dfc599f6e6 Mon Sep 17 00:00:00 2001 From: TK Date: Thu, 10 Sep 2026 13:36:30 +0300 Subject: [PATCH 13/14] docs: update and format REDME.md --- README.md | 12 ++++++++++++ docs/permissionRepair.md | 21 +++++++++++++++++++-- 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 985744e1..4083e853 100644 --- a/README.md +++ b/README.md @@ -282,9 +282,21 @@ Postgres operator compatibility with Operator SDK version is in the table below Permission repair is optional per `Postgres`. It adds missing grants; it never revokes custom grants, changes ownership, recreates roles or rotates credentials. + +What is covered by permissionRepair: + +- All application schemas. +- Tables, partitions, views, materialized views, and foreign tables. +- Sequences. +- Functions and procedures. +- Owner-managed types, domains, and large objects. +- Default privileges for future objects created by the owner role. + See [docs/permissionRepair.md](docs/permissionRepair.md) for the full reference. ```yaml +kind: Postgres +.... spec: # Keep the existing database, masterRole and schema configuration. permissionRepair: diff --git a/docs/permissionRepair.md b/docs/permissionRepair.md index 6b09ee4c..9de94c59 100644 --- a/docs/permissionRepair.md +++ b/docs/permissionRepair.md @@ -17,12 +17,29 @@ Note: Time format is UTC This is part of Postgres object spec ```yaml +apiVersion: db.movetokube.com/v1alpha1 +kind: Postgres +metadata: + name: my-db + namespace: app + annotations: + # OPTIONAL + # use this to target which instance of operator should process this CR. See General config + postgres.db.movetokube.com/instance: POSTGRES_INSTANCE spec: - # Keep the existing database, masterRole and schema configuration. - permissionRepair: + database: test-db # Name of database created in PostgreSQL + dropOnDelete: false # Set to true if you want the operator to drop the database and role when this CR is deleted (optional) + masterRole: test-db-group (optional) + permissionRepair: # If that field is omitted permissionRepair will be disabled schedule: "0 2 * * *" # Five cron fields; every day at 02:00 UTC windowDuration: "30m" # Latest allowed start/end; defaults to 30m timeout: "5m" # Maximum transaction duration; defaults to 5m + schemas: # List of schemas the operator should create in database (optional) + - stores + - customers + extensions: # List of extensions that should be created in the database (optional) + - fuzzystrmatch + - pgcrypto ``` The schedule accepts five-field cron syntax (including lists, ranges and steps), From 0a70c23404fdd244db14d7148cd4c29b4c22cfba Mon Sep 17 00:00:00 2001 From: TK Date: Sat, 12 Sep 2026 22:19:53 +0300 Subject: [PATCH 14/14] fix: remove timezone from the code --- README.md | 1 - docs/permissionRepair.md | 7 +------ internal/controller/permission_repair_test.go | 2 -- 3 files changed, 1 insertion(+), 9 deletions(-) diff --git a/README.md b/README.md index 4083e853..b617fd57 100644 --- a/README.md +++ b/README.md @@ -301,7 +301,6 @@ spec: # Keep the existing database, masterRole and schema configuration. permissionRepair: schedule: "0 2 * * *" # Five cron fields; every day at 02:00 UTC - timeZone: "Europe/Sofia" # IANA zone; defaults to UTC windowDuration: "30m" # Latest allowed start/end; defaults to 30m timeout: "5m" # Maximum transaction duration; defaults to 5m ``` diff --git a/docs/permissionRepair.md b/docs/permissionRepair.md index 9de94c59..3230c113 100644 --- a/docs/permissionRepair.md +++ b/docs/permissionRepair.md @@ -42,12 +42,7 @@ spec: - pgcrypto ``` -The schedule accepts five-field cron syntax (including lists, ranges and steps), -without seconds or `@daily`-style shortcuts. Schedules always use UTC, regardless -of the operator host timezone or daylight-saving changes. Timezone overrides in -the cron expression are rejected. `windowDuration` must be positive and at most `24h`; `timeout` must be -positive and no longer than the window. Invalid configuration is reported in -`status.permissionRepair.error` and no scheduled repair runs. +The schedule accepts five-field cron syntax Schedules always run in UTC. With a schedule configured, initial provisioning still applies normal grants. Subsequent permission repair runs only in its window, using `spec.schemas` and the diff --git a/internal/controller/permission_repair_test.go b/internal/controller/permission_repair_test.go index 0ca63055..1b2db3c7 100644 --- a/internal/controller/permission_repair_test.go +++ b/internal/controller/permission_repair_test.go @@ -67,8 +67,6 @@ func TestPermissionRepairSchedule(t *testing.T) { }{ {"UTC", db.PermissionRepairSpec{Schedule: "0 2 * * *"}, "2026-09-09T02:00:00Z", false}, {"steps", db.PermissionRepairSpec{Schedule: "*/15 * * * *"}, "2026-09-09T00:15:00Z", false}, - {"timezone override", db.PermissionRepairSpec{Schedule: "CRON_TZ=Europe/Sofia 0 2 * * *"}, "", true}, - {"short timezone override", db.PermissionRepairSpec{Schedule: "TZ=Europe/Sofia 2 * * *"}, "", true}, {"six fields", db.PermissionRepairSpec{Schedule: "0 0 2 * * *"}, "", true}, {"descriptor", db.PermissionRepairSpec{Schedule: "@daily"}, "", true}, {"range", db.PermissionRepairSpec{Schedule: "65 2 * * *"}, "", true},