diff --git a/README.md b/README.md index 8f5d8bd3..fc46d847 100644 --- a/README.md +++ b/README.md @@ -13,6 +13,7 @@ Manage external PostgreSQL databases in Kubernetes with ease—supporting AWS RD - [Features](#features) - [Supported Cloud Providers](#supported-cloud-providers) - [Configuration](#configuration) +- [Dedicated Operator Role](#dedicated-operator-role) - [Installation](#installation) - [Custom Resources (CRs)](#custom-resources-crs) - [Multiple Operator Support](#multiple-operator-support) @@ -66,15 +67,69 @@ 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. +## Dedicated Operator Role + +The operator connects to PostgreSQL using the credentials configured via the `POSTGRES_*` environment variables / Secret (see below). In many setups these credentials are the _server admin_ or _master user_. + +You can also run the operator using a **dedicated operator login role** (recommended for production), for better separation of duties and easier auditing/rotation. + +### What privileges are required? + +This operator manages databases and roles, and also runs some operations inside the created databases. Your operator login role must be able to: + +- Create databases and set database owners (`CREATE DATABASE`, `ALTER DATABASE ... OWNER TO ...`) +- Grant database-level privileges (the operator runs `GRANT CREATE ON DATABASE ...`) +- Create roles/users and manage role membership (`CREATE ROLE`, `DROP ROLE`, `GRANT TO `, `REVOKE ...`) +- Connect to managed databases and: + - Create schemas (`CREATE SCHEMA ... AUTHORIZATION ...`) + - Create extensions (`CREATE EXTENSION ...`) + - Grant privileges / alter default privileges within schemas + +The operator also grants each created role to itself, so it can later revoke privileges, reassign ownership, and drop roles cleanly. + +### Example: creating an operator role + +The exact SQL depends on how your PostgreSQL instance is managed. In plain PostgreSQL (self-hosted), you can often do something like: + +```sql +-- Create a dedicated login for the Kubernetes operator +CREATE ROLE pgoperator WITH + PASSWORD 'YourSecurePassword123!' + LOGIN + CREATEDB + CREATEROLE; +``` + +For managed services, you typically create `ext_postgres_operator` while connected as the platform-provided admin and grant only the capabilities supported by that platform. + +### Cloud provider notes + +Because this is an _external / managed PostgreSQL_ operator, the feasibility of least-privilege depends on your provider. + +- **AWS RDS (PostgreSQL)** + - The initial “master user” is a member of the `rds_superuser` role. + - A dedicated operator role is usually possible: create a login role with `CREATEDB`/`CREATEROLE`, then grant it any extra permissions you need for extensions/schemas. + - Docs: + +- **GCP Cloud SQL (PostgreSQL)** + - Cloud SQL does not expose true `SUPERUSER`. The default `postgres` user is a member of `cloudsqlsuperuser` and has `CREATEROLE` and `CREATEDB`. + - You can create other users/roles with reduced privileges (for example, an operator role with `CREATEROLE`/`CREATEDB`), but some operations (notably certain extensions) may require `cloudsqlsuperuser`. + - Docs: + +- **Azure Database for PostgreSQL  Flexible Server** + - The server admin user is a member of `azure_pg_admin` and has `CREATEDB` and `CREATEROLE`; the `azuresu` superuser role is reserved for Microsoft. + - A dedicated operator role is supported: create a user with `CREATEDB`/`CREATEROLE`, optionally add it to `azure_pg_admin` if you need additional capabilities. + - Docs: + ## Installation ### Install Using Helm (Recommended) @@ -82,11 +137,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 ``` @@ -121,11 +178,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 - ``` @@ -149,11 +208,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. @@ -173,14 +232,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}}" ``` @@ -191,22 +250,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 @@ -227,7 +286,7 @@ meeting the specific needs of different applications. Available context: | Variable | Meaning | -|-------------|------------------------------| +| ----------- | ---------------------------- | | `.Host` | Database host | | `.Role` | Generated user/role name | | `.Database` | Referenced database name | @@ -243,12 +302,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/go.mod b/go.mod index d86d70fa..c9954cc8 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 04f23ab5..b659c79d 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= @@ -86,6 +88,7 @@ github.com/json-iterator/go v1.1.12 h1:PV8peI4a0ysnczrg+LtxykD8LfKY9ML6u2jnxaEnr github.com/json-iterator/go v1.1.12/go.mod h1:e30LSqwooZae/UwlEbR2852Gd8hjQvJoHmT4TnhNGBo= github.com/kisielk/errcheck v1.5.0/go.mod h1:pFxgyoBC7bSaBwPgfKdkLd5X25qrDl4LWUI2bnpBCr8= github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck= +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 deleted file mode 100644 index a27167a4..00000000 --- a/pkg/postgres/aws.go +++ /dev/null @@ -1,79 +0,0 @@ -package postgres - -import ( - "fmt" - - "github.com/lib/pq" -) - -type awspg struct { - pg -} - -func newAWSPG(postgres *pg) PG { - return &awspg{ - *postgres, - } -} - -func (c *awspg) AlterDefaultLoginRole(role, setRole string) error { - // On AWS RDS the postgres user isn't really superuser so he doesn't have permissions - // to ALTER USER unless he belongs to both roles - err := c.GrantRole(role, c.user) - if err != nil { - return err - } - defer c.RevokeRole(role, c.user) - - return c.pg.AlterDefaultLoginRole(role, setRole) -} - -func (c *awspg) CreateDB(dbname, role string) error { - // Have to add the master role to the group role before we can transfer the database owner - err := c.GrantRole(role, c.user) - if err != nil { - return err - } - - return c.pg.CreateDB(dbname, role) -} - -func (c *awspg) CreateUserRole(role, password string) (string, error) { - returnedRole, err := c.pg.CreateUserRole(role, password) - if err != nil { - return "", err - } - // On AWS RDS the postgres user isn't really superuser so he doesn't have permissions - // to ALTER DEFAULT PRIVILEGES FOR ROLE unless he belongs to the role - err = c.GrantRole(role, c.user) - if err != nil { - return "", err - } - - return returnedRole, nil -} - -func (c *awspg) DropRole(role, newOwner, database string) error { - // On AWS RDS the postgres user isn't really superuser so he doesn't have permissions - // to REASSIGN OWNED BY unless he belongs to both roles - err := c.GrantRole(role, c.user) - if err != nil && err.(*pq.Error).Code != "0LP01" { - if err.(*pq.Error).Code == "42704" { - // The group role does not exist, no point in continuing - return nil - } - return err - } - err = c.GrantRole(newOwner, c.user) - if err != nil && err.(*pq.Error).Code != "0LP01" { - if err.(*pq.Error).Code == "42704" { - // The group role does not exist, no point of granting roles - c.log.Info(fmt.Sprintf("not granting %s to %s as %s does not exist", role, newOwner, newOwner)) - return nil - } - return err - } - defer c.RevokeRole(newOwner, c.user) - - return c.pg.DropRole(role, newOwner, database) -} diff --git a/pkg/postgres/azure.go b/pkg/postgres/azure.go deleted file mode 100644 index a87696a0..00000000 --- a/pkg/postgres/azure.go +++ /dev/null @@ -1,49 +0,0 @@ -package postgres - -import ( - "github.com/lib/pq" -) - -type AzureType string - -type azurepg struct { - pg -} - -func newAzurePG(postgres *pg) PG { - return &azurepg{ - pg: *postgres, - } -} - -func (azpg *azurepg) CreateUserRole(role, password string) (string, error) { - returnedRole, err := azpg.pg.CreateUserRole(role, password) - if err != nil { - return "", err - } - return returnedRole, nil -} - -func (azpg *azurepg) CreateDB(dbname, role string) error { - // This step is necessary before we can set the specified role as the database owner - err := azpg.GrantRole(role, azpg.user) - if err != nil { - return err - } - - return azpg.pg.CreateDB(dbname, role) -} - -func (azpg *azurepg) DropRole(role, newOwner, database string) error { - // Grant the role to the user first - err := azpg.GrantRole(role, azpg.user) - if err != nil && err.(*pq.Error).Code != "0LP01" { - if err.(*pq.Error).Code == "42704" { - return nil - } - return err - } - - // Delegate to parent implementation to perform the actual drop - return azpg.pg.DropRole(role, newOwner, database) -} diff --git a/pkg/postgres/database.go b/pkg/postgres/database.go index 11fe8c43..b0f1df19 100644 --- a/pkg/postgres/database.go +++ b/pkg/postgres/database.go @@ -2,8 +2,6 @@ package postgres import ( "fmt" - - "github.com/lib/pq" ) const ( @@ -12,7 +10,7 @@ const ( CREATE_EXTENSION = `CREATE EXTENSION IF NOT EXISTS "%s"` ALTER_DB_OWNER = `ALTER DATABASE "%s" OWNER TO "%s"` REASSIGN_DB_OWNER = `REASSIGN OWNED BY "%s" TO "%s"` - DROP_DATABASE = `DROP DATABASE "%s"` + DROP_DATABASE = `DROP DATABASE "%s" WITH (FORCE)` GRANT_USAGE_SCHEMA = `GRANT USAGE ON SCHEMA "%s" TO "%s"` GRANT_CREATE_TABLE = `GRANT CREATE ON SCHEMA "%s" TO "%s"` GRANT_ALL_TABLES = `GRANT %s ON ALL TABLES IN SCHEMA "%s" TO "%s"` @@ -22,28 +20,40 @@ const ( GRANT_ALL_SEQUENCES = `GRANT %s ON ALL SEQUENCES IN SCHEMA "%s" TO "%s"` DEFAULT_PRIVS_SEQUENCES = `ALTER DEFAULT PRIVILEGES IN SCHEMA "%s" GRANT %s ON SEQUENCES TO "%s"` REVOKE_CONNECT = `REVOKE CONNECT ON DATABASE "%s" FROM public` - TERMINATE_BACKEND = `SELECT pg_terminate_backend(pg_stat_activity.pid) FROM pg_stat_activity WHERE pg_stat_activity.datname = '%s' AND pid <> pg_backend_pid()` GET_DB_OWNER = `SELECT pg_catalog.pg_get_userbyid(d.datdba) FROM pg_catalog.pg_database d WHERE d.datname = '%s'` GRANT_CREATE_SCHEMA = `GRANT CREATE ON DATABASE "%s" TO "%s"` + GRANT_CONNECT = `GRANT CONNECT ON DATABASE "%s" TO "%s"` ) func (c *pg) CreateDB(dbname, role string) error { - _, err := c.db.Exec(fmt.Sprintf(CREATE_DB, dbname)) + // Create database + err := c.execute(fmt.Sprintf(CREATE_DB, dbname)) if err != nil { // eat DUPLICATE DATABASE ERROR - if err.(*pq.Error).Code != "42P04" { + if !isPgError(err, "42P04") { return err } } - _, err = c.db.Exec(fmt.Sprintf(ALTER_DB_OWNER, dbname, role)) + err = c.execute(fmt.Sprintf(ALTER_DB_OWNER, dbname, role)) if err != nil { return err } - _, err = c.db.Exec(fmt.Sprintf(GRANT_CREATE_SCHEMA, dbname, role)) - if err != nil { - return err + // Grant CREATE on database to owner and operator user + usersToGrant := []string{c.user, role} + for _, u := range usersToGrant { + err = c.execute(fmt.Sprintf(GRANT_CREATE_SCHEMA, dbname, u)) + if err != nil { + return fmt.Errorf("failed to grant create schema on %s to %s: %w", dbname, u, err) + } + } + // Grant CONNECT on database to owner and operator user + for _, u := range usersToGrant { + err = c.execute(fmt.Sprintf(GRANT_CONNECT, dbname, u)) + if err != nil { + return fmt.Errorf("failed to grant connect on %s to %s: %w", dbname, u, err) + } } return nil } @@ -53,8 +63,7 @@ func (c *pg) AlterDatabaseOwner(dbname, owner string) error { if owner == "" { return nil } - _, err := c.db.Exec(fmt.Sprintf(ALTER_DB_OWNER, dbname, owner)) - return err + return c.execute(fmt.Sprintf(ALTER_DB_OWNER, dbname, owner)) } func (c *pg) ReassignDatabaseOwner(dbName, currentOwner, newOwner string) error { @@ -62,7 +71,7 @@ func (c *pg) ReassignDatabaseOwner(dbName, currentOwner, newOwner string) error return nil } - tmpDb, err := GetConnection(c.user, c.pass, c.host, dbName, c.args) + tmpDb, err := getConnection(c.user, c.pass, c.host, dbName, c.args) if err != nil { return err } @@ -70,7 +79,7 @@ func (c *pg) ReassignDatabaseOwner(dbName, currentOwner, newOwner string) error _, err = tmpDb.Exec(fmt.Sprintf(REASSIGN_DB_OWNER, currentOwner, newOwner)) if err != nil { - if pqErr, ok := err.(*pq.Error); ok && pqErr.Code == "42704" { + if isPgError(err, "42704") { return nil } return err @@ -79,7 +88,7 @@ func (c *pg) ReassignDatabaseOwner(dbName, currentOwner, newOwner string) error } func (c *pg) CreateSchema(db, role, schema string) error { - tmpDb, err := GetConnection(c.user, c.pass, c.host, db, c.args) + tmpDb, err := getConnection(c.user, c.pass, c.host, db, c.args) if err != nil { return err } @@ -93,20 +102,15 @@ func (c *pg) CreateSchema(db, role, schema string) error { } func (c *pg) DropDatabase(database string) error { - _, err := c.db.Exec(fmt.Sprintf(REVOKE_CONNECT, database)) + err := c.execute(fmt.Sprintf(REVOKE_CONNECT, database)) // Error code 3D000 is returned if database doesn't exist - if err != nil && err.(*pq.Error).Code != "3D000" { + if err != nil && !isPgError(err, "3D000") { return err } - _, err = c.db.Exec(fmt.Sprintf(TERMINATE_BACKEND, database)) - // Error code 3D000 is returned if database doesn't exist - if err != nil && err.(*pq.Error).Code != "3D000" { - return err - } - _, err = c.db.Exec(fmt.Sprintf(DROP_DATABASE, database)) + err = c.execute(fmt.Sprintf(DROP_DATABASE, database)) // Error code 3D000 is returned if database doesn't exist - if err != nil && err.(*pq.Error).Code != "3D000" { + if err != nil && !isPgError(err, "3D000") { return err } @@ -116,7 +120,7 @@ func (c *pg) DropDatabase(database string) error { } func (c *pg) CreateExtension(db, extension string) error { - tmpDb, err := GetConnection(c.user, c.pass, c.host, db, c.args) + tmpDb, err := getConnection(c.user, c.pass, c.host, db, c.args) if err != nil { return err } @@ -130,7 +134,7 @@ func (c *pg) CreateExtension(db, extension string) error { } func (c *pg) SetSchemaPrivileges(schemaPrivileges PostgresSchemaPrivileges) error { - tmpDb, err := GetConnection(c.user, c.pass, c.host, schemaPrivileges.DB, c.args) + tmpDb, err := getConnection(c.user, c.pass, c.host, schemaPrivileges.DB, c.args) if err != nil { return err } diff --git a/pkg/postgres/database_test.go b/pkg/postgres/database_test.go new file mode 100644 index 00000000..4262ff73 --- /dev/null +++ b/pkg/postgres/database_test.go @@ -0,0 +1,319 @@ +package postgres + +import ( + "database/sql" + "regexp" + "testing" + + "github.com/DATA-DOG/go-sqlmock" +) + +func TestCreateDB(t *testing.T) { + db, mock, err := sqlmock.New() + if err != nil { + t.Fatalf("sqlmock.New: %v", err) + } + defer db.Close() + + p := &pg{db: db, user: "operator"} + + mock.ExpectExec(regexp.QuoteMeta(`CREATE DATABASE "mydb"`)).WillReturnResult(sqlmock.NewResult(0, 1)) + mock.ExpectExec(regexp.QuoteMeta(`ALTER DATABASE "mydb" OWNER TO "owner"`)).WillReturnResult(sqlmock.NewResult(0, 1)) + mock.ExpectExec(regexp.QuoteMeta(`GRANT CREATE ON DATABASE "mydb" TO "operator"`)).WillReturnResult(sqlmock.NewResult(0, 1)) + mock.ExpectExec(regexp.QuoteMeta(`GRANT CREATE ON DATABASE "mydb" TO "owner"`)).WillReturnResult(sqlmock.NewResult(0, 1)) + mock.ExpectExec(regexp.QuoteMeta(`GRANT CONNECT ON DATABASE "mydb" TO "operator"`)).WillReturnResult(sqlmock.NewResult(0, 1)) + mock.ExpectExec(regexp.QuoteMeta(`GRANT CONNECT ON DATABASE "mydb" TO "owner"`)).WillReturnResult(sqlmock.NewResult(0, 1)) + + if err := p.CreateDB("mydb", "owner"); err != nil { + t.Fatalf("CreateDB: %v", err) + } + + if err := mock.ExpectationsWereMet(); err != nil { + t.Fatalf("expectations: %v", err) + } +} + +func TestRenameGroupRole(t *testing.T) { + db, mock, err := sqlmock.New() + if err != nil { + t.Fatalf("sqlmock.New: %v", err) + } + defer db.Close() + + p := &pg{db: db} + + mock.ExpectExec(regexp.QuoteMeta(`ALTER ROLE "old" RENAME TO "new"`)).WillReturnResult(sqlmock.NewResult(0, 1)) + + if err := p.RenameGroupRole("old", "new"); err != nil { + t.Fatalf("RenameGroupRole: %v", err) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Fatalf("expectations: %v", err) + } +} + +func TestUpdatePassword(t *testing.T) { + db, mock, err := sqlmock.New() + if err != nil { + t.Fatalf("sqlmock.New: %v", err) + } + defer db.Close() + + p := &pg{db: db} + + mock.ExpectExec(regexp.QuoteMeta(`ALTER ROLE "user" WITH PASSWORD 'newpass'`)).WillReturnResult(sqlmock.NewResult(0, 1)) + + if err := p.UpdatePassword("user", "newpass"); err != nil { + t.Fatalf("UpdatePassword: %v", err) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Fatalf("expectations: %v", err) + } +} + +func TestGrantRole(t *testing.T) { + db, mock, err := sqlmock.New() + if err != nil { + t.Fatalf("sqlmock.New: %v", err) + } + defer db.Close() + + p := &pg{db: db} + + mock.ExpectExec(regexp.QuoteMeta(`GRANT "role" TO "grantee"`)).WillReturnResult(sqlmock.NewResult(0, 1)) + + if err := p.GrantRole("role", "grantee"); err != nil { + t.Fatalf("GrantRole: %v", err) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Fatalf("expectations: %v", err) + } +} + +func TestRevokeRole(t *testing.T) { + db, mock, err := sqlmock.New() + if err != nil { + t.Fatalf("sqlmock.New: %v", err) + } + defer db.Close() + + p := &pg{db: db} + + mock.ExpectExec(regexp.QuoteMeta(`REVOKE "role" FROM "revoked"`)).WillReturnResult(sqlmock.NewResult(0, 1)) + + if err := p.RevokeRole("role", "revoked"); err != nil { + t.Fatalf("RevokeRole: %v", err) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Fatalf("expectations: %v", err) + } +} + +func TestAlterDefaultLoginRole(t *testing.T) { + db, mock, err := sqlmock.New() + if err != nil { + t.Fatalf("sqlmock.New: %v", err) + } + defer db.Close() + + p := &pg{db: db} + + mock.ExpectExec(regexp.QuoteMeta(`ALTER USER "user" SET ROLE "group"`)).WillReturnResult(sqlmock.NewResult(0, 1)) + + if err := p.AlterDefaultLoginRole("user", "group"); err != nil { + t.Fatalf("AlterDefaultLoginRole: %v", err) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Fatalf("expectations: %v", err) + } +} + +func TestAlterDatabaseOwner(t *testing.T) { + db, mock, err := sqlmock.New() + if err != nil { + t.Fatalf("sqlmock.New: %v", err) + } + defer db.Close() + + p := &pg{db: db} + + mock.ExpectExec(regexp.QuoteMeta(`ALTER DATABASE "mydb" OWNER TO "newowner"`)).WillReturnResult(sqlmock.NewResult(0, 1)) + + if err := p.AlterDatabaseOwner("mydb", "newowner"); err != nil { + t.Fatalf("AlterDatabaseOwner: %v", err) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Fatalf("expectations: %v", err) + } +} + +func TestAlterDatabaseOwner_EmptyOwnerNoop(t *testing.T) { + p := &pg{} + if err := p.AlterDatabaseOwner("mydb", ""); err != nil { + t.Fatalf("AlterDatabaseOwner: %v", err) + } +} + +func TestDropDatabase(t *testing.T) { + db, mock, err := sqlmock.New() + if err != nil { + t.Fatalf("sqlmock.New: %v", err) + } + defer db.Close() + + p := &pg{db: db} + + mock.ExpectExec(regexp.QuoteMeta(`REVOKE CONNECT ON DATABASE "mydb" FROM public`)).WillReturnResult(sqlmock.NewResult(0, 1)) + mock.ExpectExec(regexp.QuoteMeta(`DROP DATABASE "mydb" WITH (FORCE)`)).WillReturnResult(sqlmock.NewResult(0, 1)) + + if err := p.DropDatabase("mydb"); err != nil { + t.Fatalf("DropDatabase: %v", err) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Fatalf("expectations: %v", err) + } +} + +func TestReassignDatabaseOwner(t *testing.T) { + rootDB, _, err := sqlmock.New() + if err != nil { + t.Fatalf("sqlmock.New root: %v", err) + } + defer rootDB.Close() + + perDB, perMock, err := sqlmock.New() + if err != nil { + t.Fatalf("sqlmock.New per-db: %v", err) + } + defer perDB.Close() + + original := getConnection + getConnection = func(user, password, host, database, uriArgs string) (*sql.DB, error) { + return perDB, nil + } + t.Cleanup(func() { getConnection = original }) + + p := &pg{db: rootDB, user: "operator", pass: "pass", host: "host", args: ""} + + perMock.ExpectExec(regexp.QuoteMeta(`REASSIGN OWNED BY "old" TO "new"`)).WillReturnResult(sqlmock.NewResult(0, 1)) + + if err := p.ReassignDatabaseOwner("mydb", "old", "new"); err != nil { + t.Fatalf("ReassignDatabaseOwner: %v", err) + } + if err := perMock.ExpectationsWereMet(); err != nil { + t.Fatalf("expectations: %v", err) + } +} + +func TestReassignDatabaseOwner_NoOpWhenSame(t *testing.T) { + p := &pg{} + if err := p.ReassignDatabaseOwner("mydb", "owner", "owner"); err != nil { + t.Fatalf("ReassignDatabaseOwner: %v", err) + } +} + +func TestCreateSchema(t *testing.T) { + rootDB, _, err := sqlmock.New() + if err != nil { + t.Fatalf("sqlmock.New root: %v", err) + } + defer rootDB.Close() + + perDB, perMock, err := sqlmock.New() + if err != nil { + t.Fatalf("sqlmock.New per-db: %v", err) + } + defer perDB.Close() + + original := getConnection + getConnection = func(user, password, host, database, uriArgs string) (*sql.DB, error) { + return perDB, nil + } + t.Cleanup(func() { getConnection = original }) + + p := &pg{db: rootDB, user: "operator", pass: "pass", host: "host", args: ""} + + perMock.ExpectExec(regexp.QuoteMeta(`CREATE SCHEMA IF NOT EXISTS "myschema" AUTHORIZATION "owner"`)).WillReturnResult(sqlmock.NewResult(0, 1)) + + if err := p.CreateSchema("mydb", "owner", "myschema"); err != nil { + t.Fatalf("CreateSchema: %v", err) + } + if err := perMock.ExpectationsWereMet(); err != nil { + t.Fatalf("expectations: %v", err) + } +} + +func TestCreateExtension(t *testing.T) { + rootDB, _, err := sqlmock.New() + if err != nil { + t.Fatalf("sqlmock.New root: %v", err) + } + defer rootDB.Close() + + perDB, perMock, err := sqlmock.New() + if err != nil { + t.Fatalf("sqlmock.New per-db: %v", err) + } + defer perDB.Close() + + original := getConnection + getConnection = func(user, password, host, database, uriArgs string) (*sql.DB, error) { + return perDB, nil + } + t.Cleanup(func() { getConnection = original }) + + p := &pg{db: rootDB, user: "operator", pass: "pass", host: "host", args: ""} + + perMock.ExpectExec(regexp.QuoteMeta(`CREATE EXTENSION IF NOT EXISTS "pgcrypto"`)).WillReturnResult(sqlmock.NewResult(0, 1)) + + if err := p.CreateExtension("mydb", "pgcrypto"); err != nil { + t.Fatalf("CreateExtension: %v", err) + } + if err := perMock.ExpectationsWereMet(); err != nil { + t.Fatalf("expectations: %v", err) + } +} + +func TestSetSchemaPrivileges(t *testing.T) { + rootDB, _, err := sqlmock.New() + if err != nil { + t.Fatalf("sqlmock.New root: %v", err) + } + defer rootDB.Close() + + perDB, perMock, err := sqlmock.New() + if err != nil { + t.Fatalf("sqlmock.New per-db: %v", err) + } + defer perDB.Close() + + original := getConnection + getConnection = func(user, password, host, database, uriArgs string) (*sql.DB, error) { + return perDB, nil + } + t.Cleanup(func() { getConnection = original }) + + p := &pg{db: rootDB, user: "operator", pass: "pass", host: "host", args: ""} + + privs := PostgresSchemaPrivileges{ + DB: "mydb", + Role: "app", + Schema: "public", + Privs: "SELECT", + SequencePrivs: "", + FunctionPrivs: "", + CreateSchema: true, + } + + perMock.ExpectExec(regexp.QuoteMeta(`GRANT USAGE ON SCHEMA "public" TO "app"`)).WillReturnResult(sqlmock.NewResult(0, 1)) + perMock.ExpectExec(regexp.QuoteMeta(`GRANT SELECT ON ALL TABLES IN SCHEMA "public" TO "app"`)).WillReturnResult(sqlmock.NewResult(0, 1)) + perMock.ExpectExec(regexp.QuoteMeta(`ALTER DEFAULT PRIVILEGES IN SCHEMA "public" GRANT SELECT ON TABLES TO "app"`)).WillReturnResult(sqlmock.NewResult(0, 1)) + perMock.ExpectExec(regexp.QuoteMeta(`GRANT CREATE ON SCHEMA "public" TO "app"`)).WillReturnResult(sqlmock.NewResult(0, 1)) + + if err := p.SetSchemaPrivileges(privs); err != nil { + t.Fatalf("SetSchemaPrivileges: %v", err) + } + if err := perMock.ExpectationsWereMet(); err != nil { + t.Fatalf("expectations: %v", err) + } +} diff --git a/pkg/postgres/gcp.go b/pkg/postgres/gcp.go deleted file mode 100644 index a16faab4..00000000 --- a/pkg/postgres/gcp.go +++ /dev/null @@ -1,61 +0,0 @@ -package postgres - -import ( - "fmt" - - "github.com/lib/pq" -) - -type gcppg struct { - pg -} - -func newGCPPG(postgres *pg) PG { - return &gcppg{ - *postgres, - } -} - -func (c *gcppg) CreateDB(dbname, role string) error { - err := c.GrantRole(role, c.user) - if err != nil { - return err - } - err = c.pg.CreateDB(dbname, role) - if err != nil { - return err - } - return nil -} - -func (c *gcppg) DropRole(role, newOwner, database string) error { - if role == "alloydbsuperuser" || role == "postgres" { - c.log.V(1).Info(fmt.Sprintf("not dropping %s as it is a reserved AlloyDB role", role)) - return nil - } - // On AlloyDB the postgres or other alloydbsuperuser members, aren't really superusers so they don't have permissions - // to REASSIGN OWNED BY unless he belongs to both roles - err := c.GrantRole(role, c.user) - if err != nil && err.(*pq.Error).Code != "0LP01" { - if err.(*pq.Error).Code == "42704" { - // The group role does not exist, no point in continuing - return nil - } - return err - } - defer c.RevokeRole(role, c.user) - if newOwner != c.user { - err = c.GrantRole(newOwner, c.user) - if err != nil && err.(*pq.Error).Code != "0LP01" { - if err.(*pq.Error).Code == "42704" { - // The group role does not exist, no point of granting roles - c.log.V(1).Info(fmt.Sprintf("not granting %s to %s as %s does not exist", role, newOwner, newOwner)) - return nil - } - return err - } - defer c.RevokeRole(newOwner, c.user) - } - - return c.pg.DropRole(role, newOwner, database) -} diff --git a/pkg/postgres/postgres.go b/pkg/postgres/postgres.go index dd5886a5..49525b1e 100644 --- a/pkg/postgres/postgres.go +++ b/pkg/postgres/postgres.go @@ -2,9 +2,11 @@ package postgres import ( "database/sql" + "errors" "fmt" "github.com/go-logr/logr" + "github.com/lib/pq" "github.com/movetokube/postgres-operator/pkg/config" ) @@ -48,8 +50,14 @@ type PostgresSchemaPrivileges struct { CreateSchema bool } +var ( + getConnection = GetConnection + openSQL = sql.Open + pingDB = func(db *sql.DB) error { return db.Ping() } +) + func NewPG(cfg *config.Cfg, logger logr.Logger) (PG, error) { - db, err := GetConnection( + db, err := getConnection( cfg.PostgresUser, cfg.PostgresPass, cfg.PostgresHost, @@ -69,20 +77,7 @@ func NewPG(cfg *config.Cfg, logger logr.Logger) (PG, error) { defaultDatabase: cfg.PostgresDefaultDb, } - switch cfg.CloudProvider { - case config.CloudProviderAWS: - logger.Info("Using AWS wrapper") - return newAWSPG(postgres), nil - case config.CloudProviderAzure: - logger.Info("Using Azure wrapper") - return newAzurePG(postgres), nil - case config.CloudProviderGCP: - logger.Info("Using GCP wrapper") - return newGCPPG(postgres), nil - default: - logger.Info("Using default postgres implementation") - return postgres, nil - } + return postgres, nil } func (c *pg) GetUser() string { @@ -93,11 +88,23 @@ func (c *pg) GetDefaultDatabase() string { return c.defaultDatabase } -func GetConnection(user, password, host, database, uriArgs string) (*sql.DB, error) { - db, err := sql.Open("postgres", fmt.Sprintf("postgresql://%s:%s@%s/%s?%s", user, password, host, database, uriArgs)) +func GetConnection(user, password, host, database, uri_args string) (*sql.DB, error) { + db, err := openSQL("postgres", fmt.Sprintf("postgresql://%s:%s@%s/%s?%s", user, password, host, database, uri_args)) if err != nil { return nil, err } - err = db.Ping() - return db, err + return db, pingDB(db) +} + +func (c *pg) execute(query string) error { + _, err := c.db.Exec(query) + return err +} + +func isPgError(err error, code string) bool { + var pqErr *pq.Error + if errors.As(err, &pqErr) { + return string(pqErr.Code) == code + } + return false } diff --git a/pkg/postgres/role.go b/pkg/postgres/role.go index d6e57d2c..b5cdae13 100644 --- a/pkg/postgres/role.go +++ b/pkg/postgres/role.go @@ -2,8 +2,6 @@ package postgres import ( "fmt" - - "github.com/lib/pq" ) const ( @@ -19,23 +17,34 @@ const ( REASIGN_OBJECTS = `REASSIGN OWNED BY "%s" TO "%s"` ) +var reservedRoles = map[string]struct{}{ + "alloydbsuperuser": {}, // GCP AlloyDB + "cloudsqlsuperuser": {}, // GCP Cloud SQL + "rdsadmin": {}, // AWS RDS + "azuresu": {}, // Azure Database for PostgreSQL +} + func (c *pg) CreateGroupRole(role string) error { // Error code 42710 is duplicate_object (role already exists) - _, err := c.db.Exec(fmt.Sprintf(CREATE_GROUP_ROLE, role)) - if err != nil && err.(*pq.Error).Code != "42710" { + err := c.execute(fmt.Sprintf(CREATE_GROUP_ROLE, role)) + if err != nil && !isPgError(err, "42710") { return err } + // Grant role also to the operator role to be able to manage it + err = c.GrantRole(role, c.user) + if err != nil && !isPgError(err, "0LP01") { + return err + } + return nil } func (c *pg) RenameGroupRole(currentRole, newRole string) error { - _, err := c.db.Exec(fmt.Sprintf(RENAME_GROUP_ROLE, currentRole, newRole)) + err := c.execute(fmt.Sprintf(RENAME_GROUP_ROLE, currentRole, newRole)) if err != nil { - if pqErr, ok := err.(*pq.Error); ok { - // 42704 => role does not exist; treat as success so caller can recreate - if pqErr.Code == "42704" { - return nil - } + // 42704 => role does not exist; treat as success so caller can recreate + if isPgError(err, "42704") { + return nil } return err } @@ -43,42 +52,70 @@ func (c *pg) RenameGroupRole(currentRole, newRole string) error { } func (c *pg) CreateUserRole(role, password string) (string, error) { - _, err := c.db.Exec(fmt.Sprintf(CREATE_USER_ROLE, role, password)) + err := c.execute(fmt.Sprintf(CREATE_USER_ROLE, role, password)) if err != nil { return "", err } + + err = c.GrantRole(role, c.user) + if err != nil && !isPgError(err, "0LP01") { + return "", err + } return role, nil } func (c *pg) GrantRole(role, grantee string) error { - _, err := c.db.Exec(fmt.Sprintf(GRANT_ROLE, role, grantee)) - if err != nil { - return err - } - return nil + return c.execute(fmt.Sprintf(GRANT_ROLE, role, grantee)) } func (c *pg) AlterDefaultLoginRole(role, setRole string) error { - _, err := c.db.Exec(fmt.Sprintf(ALTER_USER_SET_ROLE, role, setRole)) - if err != nil { - return err - } - return nil + return c.execute(fmt.Sprintf(ALTER_USER_SET_ROLE, role, setRole)) } func (c *pg) RevokeRole(role, revoked string) error { - _, err := c.db.Exec(fmt.Sprintf(REVOKE_ROLE, role, revoked)) - if err != nil { - return err - } - return nil + return c.execute(fmt.Sprintf(REVOKE_ROLE, role, revoked)) } func (c *pg) DropRole(role, newOwner, database string) error { + if _, reserved := reservedRoles[role]; reserved || role == c.user { + c.log.Info(fmt.Sprintf("not dropping %s as it is a reserved role", role)) + return nil + } + + err := c.GrantRole(role, c.user) + if err != nil && !isPgError(err, "0LP01") { + if isPgError(err, "42704") { + // The group role does not exist, no point in continuing + return nil + } + return err + } + defer func() { + if err := c.RevokeRole(role, c.user); err != nil && !isPgError(err, "0LP01") { + c.log.Error(err, "failed to revoke role from operator", "role", role) + } + }() + if newOwner != c.user { + err = c.GrantRole(newOwner, c.user) + if err != nil && !isPgError(err, "0LP01") { + if isPgError(err, "42704") { + // The group role does not exist, no point of granting roles + c.log.Info(fmt.Sprintf("not granting %s to %s as %s does not exist", role, newOwner, newOwner)) + return nil + } + return err + } + defer func() { + if err := c.RevokeRole(newOwner, c.user); err != nil && !isPgError(err, "0LP01") { + c.log.Error(err, "failed to revoke newOwner role from operator", "role", newOwner) + } + }() + } + // REASSIGN OWNED BY only works if the correct database is selected - tmpDb, err := GetConnection(c.user, c.pass, c.host, database, c.args) + tmpDb, err := getConnection(c.user, c.pass, c.host, database, c.args) if err != nil { - if err.(*pq.Error).Code == "3D000" { + if isPgError(err, "3D000") { return nil // Database is does not exist (anymore) } else { return err @@ -87,30 +124,25 @@ func (c *pg) DropRole(role, newOwner, database string) error { _, err = tmpDb.Exec(fmt.Sprintf(REASIGN_OBJECTS, role, newOwner)) defer tmpDb.Close() // Check if error exists and if different from "ROLE NOT FOUND" => 42704 - if err != nil && err.(*pq.Error).Code != "42704" { + if err != nil && !isPgError(err, "42704") { return err } // We previously assigned all objects to the operator's role so DROP OWNED BY will drop privileges of role _, err = tmpDb.Exec(fmt.Sprintf(DROP_OWNED_BY, role)) // Check if error exists and if different from "ROLE NOT FOUND" => 42704 - if err != nil && err.(*pq.Error).Code != "42704" { + if err != nil && !isPgError(err, "42704") { return err } - _, err = c.db.Exec(fmt.Sprintf(DROP_ROLE, role)) + err = c.execute(fmt.Sprintf(DROP_ROLE, role)) // Check if error exists and if different from "ROLE NOT FOUND" => 42704 - if err != nil && err.(*pq.Error).Code != "42704" { + if err != nil && !isPgError(err, "42704") { return err } return nil } func (c *pg) UpdatePassword(role, password string) error { - _, err := c.db.Exec(fmt.Sprintf(UPDATE_PASSWORD, role, password)) - if err != nil { - return err - } - - return nil + return c.execute(fmt.Sprintf(UPDATE_PASSWORD, role, password)) } diff --git a/pkg/postgres/role_test.go b/pkg/postgres/role_test.go new file mode 100644 index 00000000..df0dd32d --- /dev/null +++ b/pkg/postgres/role_test.go @@ -0,0 +1,136 @@ +package postgres + +import ( + "database/sql" + "regexp" + "testing" + + "github.com/DATA-DOG/go-sqlmock" + "github.com/lib/pq" +) + +func TestDropRole_NoOpForReservedRole(t *testing.T) { + db, _, err := sqlmock.New() + if err != nil { + t.Fatalf("sqlmock.New: %v", err) + } + defer db.Close() + + p := &pg{db: db, user: "operator"} + + if err := p.DropRole("rdsadmin", "newowner", "mydb"); err != nil { + t.Fatalf("DropRole: %v", err) + } +} + +func TestDropRole_ReassignsAndDrops(t *testing.T) { + rootDB, rootMock, err := sqlmock.New() + if err != nil { + t.Fatalf("sqlmock.New root: %v", err) + } + defer rootDB.Close() + + perDB, perMock, err := sqlmock.New() + if err != nil { + t.Fatalf("sqlmock.New per-db: %v", err) + } + defer perDB.Close() + + original := getConnection + getConnection = func(user, password, host, database, uriArgs string) (*sql.DB, error) { + return perDB, nil + } + t.Cleanup(func() { getConnection = original }) + + p := &pg{db: rootDB, user: "operator", pass: "pass", host: "host", args: ""} + + // Ensure operator can manage the role, and newOwner if needed. + rootMock.ExpectExec(regexp.QuoteMeta(`GRANT "todelete" TO "operator"`)).WillReturnResult(sqlmock.NewResult(0, 1)) + rootMock.ExpectExec(regexp.QuoteMeta(`GRANT "newowner" TO "operator"`)).WillReturnResult(sqlmock.NewResult(0, 1)) + + perMock.ExpectExec(regexp.QuoteMeta(`REASSIGN OWNED BY "todelete" TO "newowner"`)).WillReturnResult(sqlmock.NewResult(0, 1)) + perMock.ExpectExec(regexp.QuoteMeta(`DROP OWNED BY "todelete"`)).WillReturnResult(sqlmock.NewResult(0, 1)) + rootMock.ExpectExec(regexp.QuoteMeta(`DROP ROLE "todelete"`)).WillReturnResult(sqlmock.NewResult(0, 1)) + + if err := p.DropRole("todelete", "newowner", "mydb"); err != nil { + t.Fatalf("DropRole: %v", err) + } + + if err := rootMock.ExpectationsWereMet(); err != nil { + t.Fatalf("root expectations: %v", err) + } + if err := perMock.ExpectationsWereMet(); err != nil { + t.Fatalf("per-db expectations: %v", err) + } +} + +func TestCreateGroupRole_GrantsRoleToOperatorUser(t *testing.T) { + db, mock, err := sqlmock.New() + if err != nil { + t.Fatalf("sqlmock.New: %v", err) + } + defer db.Close() + + p := &pg{db: db, user: "operator"} + + mock.ExpectExec(regexp.QuoteMeta(`CREATE ROLE "myrole"`)).WillReturnResult(sqlmock.NewResult(0, 1)) + mock.ExpectExec(regexp.QuoteMeta(`GRANT "myrole" TO "operator"`)).WillReturnResult(sqlmock.NewResult(0, 1)) + + if err := p.CreateGroupRole("myrole"); err != nil { + t.Fatalf("CreateGroupRole: %v", err) + } + + if err := mock.ExpectationsWereMet(); err != nil { + t.Fatalf("expectations: %v", err) + } +} + +func TestCreateUserRole_GrantsRoleToOperatorUser(t *testing.T) { + db, mock, err := sqlmock.New() + if err != nil { + t.Fatalf("sqlmock.New: %v", err) + } + defer db.Close() + + p := &pg{db: db, user: "operator"} + + mock.ExpectExec(regexp.QuoteMeta(`CREATE ROLE "app-123" WITH LOGIN PASSWORD 'pass'`)).WillReturnResult(sqlmock.NewResult(0, 1)) + mock.ExpectExec(regexp.QuoteMeta(`GRANT "app-123" TO "operator"`)).WillReturnResult(sqlmock.NewResult(0, 1)) + + role, err := p.CreateUserRole("app-123", "pass") + if err != nil { + t.Fatalf("CreateUserRole: %v", err) + } + if role != "app-123" { + t.Fatalf("CreateUserRole role: got %q want %q", role, "app-123") + } + + if err := mock.ExpectationsWereMet(); err != nil { + t.Fatalf("expectations: %v", err) + } +} + +func TestCreateUserRole_IgnoresAlreadyMemberError(t *testing.T) { + db, mock, err := sqlmock.New() + if err != nil { + t.Fatalf("sqlmock.New: %v", err) + } + defer db.Close() + + p := &pg{db: db, user: "operator"} + + mock.ExpectExec(regexp.QuoteMeta(`CREATE ROLE "app-123" WITH LOGIN PASSWORD 'pass'`)).WillReturnResult(sqlmock.NewResult(0, 1)) + mock.ExpectExec(regexp.QuoteMeta(`GRANT "app-123" TO "operator"`)).WillReturnError(&pq.Error{Code: "0LP01"}) + + role, err := p.CreateUserRole("app-123", "pass") + if err != nil { + t.Fatalf("CreateUserRole: %v", err) + } + if role != "app-123" { + t.Fatalf("CreateUserRole role: got %q want %q", role, "app-123") + } + + if err := mock.ExpectationsWereMet(); err != nil { + t.Fatalf("expectations: %v", err) + } +} diff --git a/tests/kuttl-test-self-hosted-postgres.yaml b/tests/kuttl-test-self-hosted-postgres.yaml index 1f9769f5..5f8e53c9 100644 --- a/tests/kuttl-test-self-hosted-postgres.yaml +++ b/tests/kuttl-test-self-hosted-postgres.yaml @@ -15,12 +15,24 @@ commands: --set global.postgresql.auth.username=postgres --wait timeout: 120 + - command: >- + kubectl -n $NAMESPACE exec postgresql-0 -- bash -lc + "export PGPASSWORD=postgres; + psql -U postgres -d postgres -v ON_ERROR_STOP=1 -c \" + DO \\$\\$ + BEGIN + IF NOT EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'pgoperator') THEN + CREATE ROLE pgoperator WITH PASSWORD 'pgoperator' LOGIN CREATEDB CREATEROLE; + END IF; + END + \\$\\$;\"" + timeout: 10 - command: >- helm install -n $NAMESPACE ext-postgres-operator ./charts/ext-postgres-operator --set image.repository=postgres-operator --set image.tag=build --set postgres.host=postgresql - --set postgres.user=postgres - --set postgres.password=postgres + --set postgres.user=pgoperator + --set postgres.password=pgoperator --set postgres.uri_args="sslmode=disable" --wait