movetokube / postgres-operator Goto Github PK

This issue is second part of #31. Since the core business logic of this operator lies in SQL queries and combination of them, unit tests are not enough to test it. This issue aims to rectify it. In order to run proper integration tests, a real postgresql database needs to be used. There are two parts for this issue:

1. setup for integration tests to be run locally via docker-compose
2. setup for integration tests to be run on Github Actions where postgres database could be provided as a service.

Hi,

{"level":"info","ts":1644601161.9636536,"logger":"cmd","msg":"Go Version: go1.13.3"} ││ {"level":"info","ts":1644601161.9637027,"logger":"cmd","msg":"Go OS/Arch: linux/amd64"} ││ {"level":"info","ts":1644601161.9637146,"logger":"cmd","msg":"Version of operator-sdk: v0.17.1"} ││ {"level":"info","ts":1644601161.9640076,"logger":"leader","msg":"Trying to become the leader."} ││ {"level":"info","ts":1644601163.7440937,"logger":"leader","msg":"Found existing lock","LockOwner":"ext-postgres-operator-5d58bdc75-x9jtw"}
││ {"level":"info","ts":1644601163.7601275,"logger":"leader","msg":"Not the leader. Waiting."} ││ {"level":"info","ts":1644601164.886048,"logger":"leader","msg":"Became the leader."} ││ {"level":"info","ts":1644601166.6406956,"logger":"controller-runtime.metrics","msg":"metrics server is starting to listen","addr":"0.0.0.0:8383"}
││ {"level":"info","ts":1644601166.6410973,"logger":"cmd","msg":"Registering Components."} ││ 2022/02/11 17:39:26 failed to connect to PostgreSQL server: parse postgresql://goofybose:mhb:AbC{De<fgh![email protected]/?sslmode=disable: net/url: invalid u ││ serinfo

Im not much into it, but it seems to me that the special character in the password causes the error.
I belive also the issue could be solved here:

Something like:
Before:
pgUserUrl := fmt.Sprintf("postgresql://%s:%s@%s/%s", role, password, r.pgHost, cr.Status.DatabaseName)
Change:

import (
"net/url"
)
....
pgUserUrl := url.Parse(fmt.Sprintf("postgresql://%s:%s@%s/%s", role, password, r.pgHost, cr.Status.DatabaseName))

I dont know go ... so may I'm totally wrong :)

All Postgres databases are created with a default public schema that Postgres implicitly uses whenever a schema isn't specified. However, the operator currently operates only on schemas that are explicitly listed in Postgres CRs, which means that PostgresUsers with READ of WRITE privilege levels don't have access to the public schema by default. (This isn't a problem for the OWNER privilege level, because database ownership is sufficient for anything that schema ownership allows.)

Given that non-schema-aware applications are extremely common, I think it would make sense for the schemas list in a Postgres CR to default to ["public"] rather than an empty list. (However, this isn't sufficient on its own. See #61 for details.)

Thanks for all the work on this operator so far. We've found it to be extremely useful. :-)

Describe the bug
The bug is following:
If you have deleted the Postgres CR linked to PostgresUser CR, you can't properly delete a PostgresUser anymore. The operator will loop over an error in case dropOnDelete is set to true or at restart, it will crash.

To Reproduce
Steps to reproduce the behavior:

1. Create a Postgres CR

apiVersion: db.movetokube.com/v1alpha1
kind: Postgres
metadata:
  name: my-db
  namespace: app
spec:
  database: test-db # Name of database created in PostgreSQL
  dropOnDelete: true # Set to true if you want the operator to drop the database and role when this CR is deleted

2. Create a PostgresUser CR

apiVersion: db.movetokube.com/v1alpha1
kind: PostgresUser
metadata:
  name: my-db-user
  namespace: app
spec:
  role: username
  database: my-db # This references the Postgres CR
  secretName: my-secret
  privileges: OWNER # Can be OWNER/READ/WRITE

3. Delete Postgres CR
4. Delete PostgresUser CR
5. See error

Expected behavior
For me, an expected behaviour should be to delete all PostgresUser linked to a Postgres CR when this one is deleted.

Screenshots
None

Version and platform (please complete the following information):

• Platform [e.g. Linux, Mac, Windows, ...]: Linux
• Arch [e.g. amd64, ...]: amd64
• Version [e.g. 22]: 0.4.2

Additional context
None

A have a password that has special characters that needs to be URI encoded to work.

Acctually, all the resulting POSTGRES_SQL should be URI encoded to avoid problemas with passwords and even usernames with special characters.

That's the log I'm getting,

2022/02/24 19:00:15 failed to connect to PostgreSQL server: parse postgresql://postgres:z6fxajZydUO6,x9QN=Hn=K-sdPsE^X@__REDACTED__/postgres?ssl=required                                                                                    
: net/url: invalid control character in URL                                                                                                                                                                                                                                                      

Let me know if that's a desired thing, and I could send a PR for this.

Hello @arnarg and @hitman99 ,

I want to work on a feature to add another CRD called PostgresEngineConfiguration.
This one will allow to manage Postgres databases and users on multiple engines and not only one like now. One of this new CR can be set with a "default: true" flag.
This will create a breaking change according to the point that configuration set by environment variables are not used anymore.

With this change, I also purpose to rename Postgres CRD into PostgresDatabase CRD. Why ? Because having: PostgresEngineConfiguration <= Postgres <= PostgresUser is weird to my point of view and maybe not clear for people/teams using this operator.

This will lead to 2 breaking changes. Maintaining 2 different ways of doing stuff will be difficult and will conduct to difficult debug sessions...

Is it ok for you to do it like that ?

In #48 there was someone that gave POSTGRES_CLOUD_PROVIDER the value AZURE while it should be Azure. This shouldn't matter and we should run the provided value through ToLower() before we make the comparison.

Currently when we apply Postgres CR it creates a DB if it doesn't exists and if it exists it changes its owner to the role created by the Postgres CRs. But the owner of the tables inside it remain unaltered due to which if we try to access it using credentials created by Postgres CRs then we get this error

<database>=> select * from <table>;
ERROR:  permission denied for table <table>

It would be great if we get an option to migrate the owners of all the tables in the database to the role created by Postgres CRs

The current logs are not following a pattern.

They start with JSON, which is nice.

{"level":"info","ts":1645719468.362505,"logger":"cmd","msg":"Go Version: go1.13.3"}
{"level":"info","ts":1645719468.362529,"logger":"cmd","msg":"Go OS/Arch: linux/amd64"}
{"level":"info","ts":1645719468.3625689,"logger":"cmd","msg":"Version of operator-sdk: v0.17.1"}

Then I have something like this, no more JSON:

E0224 19:34:48.369498       1 reflector.go:153] pkg/mod/k8s.io/[email protected]/tools/cache/reflector.go:105: Failed to list *v1alpha1.Postgres: postgres.db.movetokube.com is forbidden: User "system:serviceaccount:kube-system:ext-postgres-operator" cannot list resource "postgres" in API 
E0224 19:34:49.370236       1 reflector.go:153] pkg/mod/k8s.io/[email protected]/tools/cache/reflector.go:105: Failed to list *unstructured.Unstructured: postgres.db.movetokube.com is forbidden: User "system:serviceaccount:kube-system:ext-postgres-operator" cannot list resource "postgres"
E0224 19:34:49.370302       1 reflector.go:153] pkg/mod/k8s.io/[email protected]/tools/cache/reflector.go:105: Failed to list *unstructured.Unstructured: postgresusers.db.movetokube.com is forbidden: User "system:serviceaccount:kube-system:ext-postgres-operator" cannot list resource "post

And when I get some error to connect to the database, I get this in two lines:

2022/02/24 19:00:15 failed to connect to PostgreSQL server: parse postgresql://postgres:z6fxajZydUO6,x9QN=Hn=K-sdPsE^X@__REDACTED__/postgres?ssl=required                                                                                    
: net/url: invalid control character in URL                                                                                                                                                                                                                                                      

Proposal

Select a pattern and sitck to it.

You could go with JSON, that's easy to parse if anyone needs to make some processing before sending the logs somewhere.

If I may suggest, I'd go with something like logfmt.

See:

• https://www.brandur.org/logfmt
• https://www.brandur.org/canonical-log-lines
• https://pkg.go.dev/github.com/kr/logfmt?utm_source=godoc

On our kubernetes clusters we have many namespaces one for each customer. We create a postgres-operator in each namespace for compliance reasons, to keep each deployment completely separate. To remove a customer, the typical action is to just delete the entire namespace. The problem though, is that the postgres-operator exits immediately, and leaves all of the postgres CRs stuck in terminating state because they still contain the finalizer.db.movetokube.com finalizer.

Proposal:

The controller runtime has a GracefulShutdownTimeout option that is not set in this operator (see code here). This likely means it defaults to time.Duration(0), which according to the docs means that the graceful shutdown is ignored. It is likely that if we set this to a reasonable value (say 30 seconds?) that the operator will have some time to notice that terminating CRs and cleanup after itself.

It would be nice to be able to generate connection strings as part of the secret to interoperate with .NET applications more easily

Hello team, I will like to find out if this service works just on the cloud or if it's usable locally.

Hi

I was looking for this exact functionality but so far I don't see instructions for using this operator with Cloud SQL Postgres instance.

Has anyone tried it? I am willing to but I need some pointers on how to get started.

Thanks.

I think this doesn't cover it all, but just the ext-postgres-operator secret, it's lacking the POSTGRES_URL generated by the Postgres CRD.

Will send a fixup later.

Originally posted by @endersonmaia in #79 (comment)

Can you please publish the new version 0.4.2 on operatorhub.io.
Thank you for you work.

Creating a database fails unless the operator user is either SUPERUSER (not the case in some public clouds) or is in the new owner role.

The issue can be reproduced locally with a non-superuser role:

CREATE ROLE pgoperator WITH
	PASSWORD 'pgoperator'
	LOGIN
	CREATEDB
	CREATEROLE;

GRANTing the created role to the operator user is already done as a workaround for some clouds, but it should be done by default. It doesn't hurt when the user has SUPERUSER privileges and makes some already implemented and needed workarounds (e.g. for the IBM cloud) unnecessary.

Using the following Postgres CR as an example:

apiVersion: db.movetokube.com/v1alpha1
kind: Postgres
metadata:
  name: my-db
spec:
  database: test-db

1. Apply the manifest
   • A database with the name my-db and role with name role-db-group is created
2. Use the role (through a PostgresRole) to create a table in my-db database
3. Delete the Postgres CR
   • All objects owned by my-db-group are transferred to the operator's role and the role is dropped, database ownership is transferred to the operator's role
4. Recreate the Postgres CR
   • my-db already exists so its ownership is transferred to new group role my-db-group but all tables in the database are still owned by the operator's role

In general it sure seems like the RBAC here is very wide open. However, it still gave me an error on creation of the servicemonitor that the operator tries to create when it runs.

I think based on how open the RBAC cluster-level permissions are this is fairly un-usable from a security standpoint. I did, however, go ahead and give it permissions to work with the servicemonitor object.

I discovered you on operatorhub.io, but also found https://github.com/kubehippie/database-controller

As I see both operators do almost the same, but they support both pg & mysql and is a bit older - was anything wrong with them?

Currently PostgresUser can only be a database owner over one database.

It would be better if we could also give more granular permissions to users. It could look something like this:

apiVersion: db.movetokube.com/v1alpha1
kind: PostgresUser
metadata:
  name: my-db-user
  namespace: app
spec:
  role: username
  permissions:
  - priv: CREATE
    database: my-db
  - priv: SELECT
    database: other-db
    schema: customer
    table: *
  secretName: my-credential-secret

It could also look similar to Ansible's postgresql_user module.

The owner could still be supported by instead of specifying a permissions list we simply specify a database (the current PostgresUser CRD), have a special priv: OWNER that we would handle specially by adding the user to the group role that owns the database or simply move the current PostgresUser to another CRD called PostgresOwner or something similar.

What do you think?

Hi! 👋🏻

Have you considered listing the postgres operator directly in Artifact Hub?

At the moment it is already listed there, because the Artifact Hub team has added the community-operators repository. However, listing it yourself directly has some benefits:

• You add your repository once, and new versions (or even new operators) committed to your git repository will be indexed automatically and listed in Artifact Hub, with no extra PRs needed.
• You can display the Verified Publisher label in your operators, increasing their visibility and potentially the users' trust in your content.
• Increased visibility of your organization in urls and search results. Users will be able to see your organization's description, a link to the home page and search for other content published by you.
• If something goes wrong indexing your repository, you will be notified and you can even inspect the logs to check what went wrong.

If you decide to go ahead, you just need to sign in and add your repository from the control panel. You can add it using a single user or create an organization for it, whatever suits your needs best.

You can find some notes about the expected repository url format and repository structure in the repositories guide. There is also available an example of an operator repository already listed in Artifact Hub in the documentation. Operators are expected to be packaged using the format defined in the Operator Framework documentation to facilitate the process.

Please let me know if you have any questions or if you encounter any issue during the process 🙂

First off, loving many things about this operator but one issue I ran into trying to migrate some things to it is that I have a few third party apps I run in my cluster that are designed in a way in which they need multiple Postgres databases (not schemas, completely separate databases) but only allow for a single hostname, username, password config. Afaict there's no way for ext-postgres-operator to handle this currently.

Ideally I'd like to be able to do something like this:

---
apiVersion: db.movetokube.com/v1alpha1
kind: Postgres
metadata:
  name: radarr-config-db
  namespace: media
spec:
  database: radarr-config
---
apiVersion: db.movetokube.com/v1alpha1
kind: Postgres
metadata:
  name: radarr-log-db
  namespace: media
spec:
  database: radarr-log
---
apiVersion: db.movetokube.com/v1alpha1
kind: PostgresUser
metadata:
  name: radarr-user
  namespace: media
spec:
  role: radarr
  databases:
    - name: radarr-config-db
      privileges: OWNER
    - name: radarr-log-db
      privileges: OWNER
  secretName: database
---

and would expect the database-radarr-user secret created to contain a role the has the referenced privileges on both referenced databases.

Hi there,

first off all, thank you for creating and maintaining this operator. It saved me from a lot of workarounds and empowers our team to self-manage our usage of database. 👍

unfortunately, the Ext Postgres Operator does not provide POSTGRES_URL, USERNAME and PASSWORD as atomic information (i. e. USERNAME and PASSWORD are included in POSTGRES_URL). As a result, this information cannot be used as JDBC connection string in any Java-based Application (see also https://jdbc.postgresql.org/documentation/head/connect.html).

We have implemented a temporary fix / bash script now that simply copies the original secret (that is auto-generated by the Ext Postgres Operator) and swaps the non-compliant POSTGRES_URL for a JDBC driver compatible URI which only includes server name and database name - such as:jdbc:postgresql://postgres.core-services:5432/my-database

Would be great if the Ext Postgres Operator would offer a POSTGRES_JDBC_URL so that we could throw away our temporary fix soon.

Currently there are no tests of any kind for the operator code, this issue addresses that. Test coverage should be at least 80% Just unit tests are fine as integration tests might be a bit tricky to do.

I suppose that originally provided POSTGRES_URI_ARGS should also be included into the generated secrets

ex: sslmod=disable

I can make PR but it will not be very soon.

Some cloud providers' implementations of their PostgreSQL offering have some nuances and require workarounds for postgres-operator to function properly. We should offer a config option to specify the cloud provider. This could be called POSTGRES_CLOUD_PROVIDER.

This should wrap and compose a base postgres struct for each cloud provider that does require workarounds.

Trying to use this operator (version 1.1.1) against AWS Aurora (postgres):

apiVersion: db.movetokube.com/v1alpha1
kind: Postgres
metadata:
  name: my-db
spec:
  database: test-db # Name of database created in PostgreSQL
  dropOnDelete: true # 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
  extensions: # List of extensions that should be created in the database (optional)
  - fuzzystrmatch
  - pgcrypto
---
apiVersion: db.movetokube.com/v1alpha1
kind: PostgresUser
metadata:
  name: my-db-user
spec:
  role: username
  database: my-db       # This references the Postgres CR
  secretName: my-secret
  privileges: OWNER     # Can be OWNER/READ/WRITE

Logs from operator:

ext-postgres-operator-65bc584d44-rgqzc ext-postgres-operator {"level":"error","ts":1657720145.3735487,"logger":"controller_postgres","msg":"Could not create schema stores","Request.Namespace":"developer","Request.Name":"my-db","error":"pq: must be member of role \"test-db-group\"","stacktrace":"github.com/go-logr/zapr.(*zapLogger).Error\n\t/go/pkg/mod/github.com/go-logr/[email protected]/zapr.go:128\ngithub.com/movetokube/postgres-operator/pkg/controller/postgres.(*ReconcilePostgres).Reconcile\n\t/go/src/github.com/movetokube/postgres-operator/pkg/controller/postgres/postgres_controller.go:212\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler\n\t/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:256\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem\n\t/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:232\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).worker\n\t/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:211\nk8s.io/apimachinery/pkg/util/wait.BackoffUntil.func1\n\t/go/pkg/mod/k8s.io/[email protected]/pkg/util/wait/wait.go:155\nk8s.io/apimachinery/pkg/util/wait.BackoffUntil\n\t/go/pkg/mod/k8s.io/[email protected]/pkg/util/wait/wait.go:156\nk8s.io/apimachinery/pkg/util/wait.JitterUntil\n\t/go/pkg/mod/k8s.io/[email protected]/pkg/util/wait/wait.go:133\nk8s.io/apimachinery/pkg/util/wait.Until\n\t/go/pkg/mod/k8s.io/[email protected]/pkg/util/wait/wait.go:90"}

Operator is using same config in multiple places via utils.MustGetEnv, this could be consolidated into a separate config package. To save syscals, config getter needs to use sync.Once

I'm using a software that creates schemas during its Database Migration process and at runtime.

As a workaround, I did a GRANT CREATE ON DATABASE "my-db" TO "my-db-writer";

Schemas that already exist when the operator reconciles a database keep their original ownership. This causes problems with permissions management and effectively makes it impossible to use non-OWNER PostgresUsers for any pre-existing schema, including the default public schema that Postgres creates in every database.

Currently, the operator tries to create all schemas in the schemas list in a Postgres CR. For schemas that don't already exist and schemas that do exist but are owned by the operator-managed owner role, this succeeds and the operator sets the appropriate privileges for the reader and writer roles. For schemas that already exist but have a different owner, creation fails with a permission denied error and the reader/write roles never get their privileges.

After running into this problem, I was able to manually run the privilege-grant queries in psql as a user with the owner role, which indicates that merely checking for the existence of a schema before attempting to create it will fix the issue. However, I think it would be a good idea to set the ownership of these schemas so that the result of reconciliation is the same whether they were created by the operator or already existed.

Either way, the public schema is by far the most likely "existing" schema anyone's going to need, which means that most people are going to run into #60 before they get this far.

There is a problem every time a new version of this operator needs to be pushed to operatorhub.io. The process to push to operatorhub.io is as follows:

• package operator (container image)
• update operator OLM (operator lifecycle manager) catalog manifest
• bundle up all the CRDs
• open a PR for community-operators

After the PR is opened, there is an automated check which deploys the operator to minikube. Operator deployment depends on the secret, which is not automatically created and thus the automated check fails.

We need to update the Readme and CR examples to reflect full feature set. @arnarg since you added the latest features regarding permissions, would you be able to do this?

The apiextensions.k8s.io/v1beta1 API version of CustomResourceDefinition is no longer served as of Kubernetes v1.22. https://kubernetes.io/docs/reference/using-api/deprecation-guide/#customresourcedefinition-v122

https://github.com/movetokube/postgres-operator/blob/8879217a37d87caf2e1bb8e2c220d5ae172a9fc7/deploy/crds/db.movetokube.com_postgres_crd.yaml

https://github.com/movetokube/postgres-operator/blob/8879217a37d87caf2e1bb8e2c220d5ae172a9fc7/deploy/crds/db.movetokube.com_postgresusers_crd.yaml

Opening issue #75 made me think about the passwords leaking on the logs. That should be avoided.

Somehing like:

2022/02/24 19:00:15 failed to connect to PostgreSQL server: parse postgresql://postgres:[email protected]/postgres?ssl=required                                                                                    
: net/url: invalid control character in URL                                                                                                                                                                                                                                                      ```

First off, thanks for this Operator - it's ideal for our use-case of dynamically provisioning a database for dynamically created 'preview' instances of our in-development applications.

I am experiencing the following issue using 'vanilla' PostgreSQL (not on AWS or RDS):

• Create a Postgres resource with .spec.dropOnDelete: true
• Ensure that the db is created correctly on the PostgreSQL server
• Delete the resource
• The Operator process crashes with the following log entry:

{"level":"info","ts":1579085139.616076,"logger":"controller_postgres","msg":"Reconciling Postgres","Request.Namespace":"tmt","Request.Name":"example-temp-db"}
2020/01/15 10:45:39 failed to connect to PostgreSQL server: pq: database "example-temp-db" does not exist

• However, the DB has been deleted from the PostgreSQL database server

Since the DB does get deleted, it kind-of works, but has the following annoyances:

1. The Operator is now stuck in a crash loop, citing the same two log entries before each crash
2. The Postgres resource I wanted to delete originally is now 'stuck' in a Pending state - I guess it's waiting for the Finalizer to it's job (and always crashes before it can signal success)

Happy to provide further input or give you some supervised hands-on time with the affected dev cluster to help identify the cause of this.

PostgresUsers should be garbage collected by Kubernetes if the Postgres they reference are deleted.

Code needs to be covered by unit tests, where possible, this issue is part of #31, which was split in half. Test coverage should be at least 80%. If some package is not testable, it must be refactored into a testable one.

We would like to be able to set limits on the max. connections in databases or roles.

In psql that would be for example:

alter database mydb connection limit 50;
alter role myrole connection limit 50;

Can these settings be defined somewhere? I could not find it, so my question is, is it possible in the current state and if not, would a feature request be viable?

GO modules are now the standard way of dependency management. This project was boostraped with an older version of operator-sdk which used dep for dependency managemen by default. Dependency management needs to be refactored to use go modules.

line:
config.PostgresUriArgs = url.QueryEscape(utils.MustGetEnv("POSTGRES_URI_ARGS"))

url.QueryEscape Is replacing "sslmode=disable" with "sslmode%3Ddisable" and it wont work with postgresql.

operator gets error: "failed to connect to PostgreSQL server: pq: SSL is not enabled on the server"

For security and reproducibility of deploys, the operator container image should use a tagged one.

https://github.com/movetokube/postgres-operator/blob/master/deploy/operator.yaml#L18

...
      containers:
        - name: ext-postgres-operator
          image: movetokube/postgres-operator:0.5.2
...

Hi

When using the operator to manage an Azure Database for PostgreSQL, the operator fails when the dropOnDelete option is enabled on the databases.

Configuration used:

apiVersion: db.movetokube.com/v1alpha1
kind: Postgres
metadata:
  name: test-db
  namespace: dev-0
spec:
  database: test-db
  dropOnDelete: true
  schemas:
  - test_db

When this object is deleted from the Kubernetes API the following error appears:

{"level":"info","ts":1601564677.148541,"logger":"controller_postgres","msg":"Reconciling Postgres","Request.Namespace":"sit-1","Request.Name":"test-db"}
 {"level":"info","ts":1601564677.3637729,"logger":"controller_postgres","msg":"connected to postgres server","Request.Namespace":"sit-1","Request.Name":"test-db"}
 {"level":"error","ts":1601564677.3776052,"logger":"kubebuilder.controller","msg":"Reconciler error","controller":"postgres-controller","request":"sit-1/test-db","error":"pq: role \"test-db-group
 \" cannot be dropped because some objects depend on it","stacktrace":"github.com/go-logr/zapr.(*zapLogger).Error\n\t/go/pkg/mod/github.com/go-logr/[email protected]/zapr.go:128\nsigs.k8s.io/controller-runtime/p
 kg/internal/controller.(*Controller).processNextWorkItem\n\t/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:217\nsigs.k8s.io/controller-runtime/pkg/internal/contro
 ller.(*Controller).Start.func1\n\t/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:158\nk8s.io/apimachinery/pkg/util/wait.JitterUntil.func1\n\t/go/pkg/mod/k8s.io/ap
 [email protected]/pkg/util/wait/wait.go:133\nk8s.io/apimachinery/pkg/util/wait.JitterUntil\n\t/go/pkg/mod/k8s.io/[email protected]/pkg/util/wait/w
 ait.go:134\nk8s.io/apimachinery/pkg/util/wait.Until\n\t/go/pkg/mod/k8s.io/[email protected]/pkg/util/wait/wait.go:88"}

We're running version 0.4.1 of the operator. We're running the operator for containerized databases as well (Zalando Postgres Operator) and there we don't see the same issue.

Upon looking into the issue, I noticed that for the containerized databases, the ownership was properly transferred before the roles and database were deleted. Unfortunately, this seems to be the problem area on Azure. The re-assigning of the ownership fails on Azure due to the usage of the standard username instead of the one where the @ is stripped.

This could be solved by overloading the getUser function in the Azure class and re-using the getLogin I think? (Not really a go developer :( )

Hi all

I'm trying to use CR to create/delete database/users over a pre-generated Google PostreSQL instance.

Prerequisites (done manually outside operator)

• Create database instance
• create default database postgresql-operator
• add a devops-operator user
• configured secret to connect to default database using such user

Operator resource

---
apiVersion: db.movetokube.com/v1alpha1
kind: Postgres
metadata:
  name: operator-test-devops
  namespace: devops-operators
spec:
  database: test-devops-db # Name of database created in PostgreSQL
  dropOnDelete: true # Set to true if you want the operator to drop the database and role when this CR is deleted (optional) 
  masterRole: devops-operator # must match DB creator's role

Applying CR sucessfylly connects and creates test-devops-db database into my postrgresql
It also creates test-devops-db-reader and test-devops-db-writer roles
But once I try to remove the CR the operator fails with

[ext-postgres-operator-5d58bdc75-dhpx5 ext-postgres-operator]  {"level":"error","ts":1648634579.2694623,"logger":"controller-runtime.controller","msg":"Reconciler error","controller":"postgres-controller","request":"devops-operators/operator-test-devops","error":"pq: current user cannot be dropped","stacktrace":"github.com/go-logr/zapr.(*zapLogger).Error
/go/pkg/mod/github.com/go-logr/[email protected]/zapr.go:128\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler
/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:258\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem
/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:232\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).worker
/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:211\nk8s.io/apimachinery/pkg/util/wait.BackoffUntil.func1
/go/pkg/mod/k8s.io/[email protected]/pkg/util/wait/wait.go:155\nk8s.io/apimachinery/pkg/util/wait.BackoffUntil
/go/pkg/mod/k8s.io/[email protected]/pkg/util/wait/wait.go:156\nk8s.io/apimachinery/pkg/util/wait.JitterUntil
/go/pkg/mod/k8s.io/[email protected]/pkg/util/wait/wait.go:133\nk8s.io/apimachinery/pkg/util/wait.Until
/go/pkg/mod/k8s.io/[email protected]/pkg/util/wait/wait.go:90"}

Not sure if this is linked to
masterRole: devops-operator

but omitting such line or assigning it to a different role other then the one configured for connection in operator secrets results in

[ext-postgres-operator-5d58bdc75-rbf26 ext-postgres-operator]  {"level":"error","ts":1648623901.489596,"logger":"controller-runtime.controller","msg":"Reconciler error","controller":"postgres-controller","request":"devops-operators/operator-test-devops","error":"Internal error occurred: pq: must be member of role \"test-devops-db-group\"","stacktrace":"github.com/go-logr/zapr.(*zapLogger).Error
/go/pkg/mod/github.com/go-logr/[email protected]/zapr.go:128\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler
/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:258\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem
/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:232\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).worker
/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:211\nk8s.io/apimachinery/pkg/util/wait.BackoffUntil.func1
/go/pkg/mod/k8s.io/[email protected]/pkg/util/wait/wait.go:155\nk8s.io/apimachinery/pkg/util/wait.BackoffUntil
/go/pkg/mod/k8s.io/[email protected]/pkg/util/wait/wait.go:156\nk8s.io/apimachinery/pkg/util/wait.JitterUntil
/go/pkg/mod/k8s.io/[email protected]/pkg/util/wait/wait.go:133\nk8s.io/apimachinery/pkg/util/wait.Until
/go/pkg/mod/k8s.io/[email protected]/pkg/util/wait/wait.go:90"}

Which is the proper way to have Users and DB properly removed upon CR deetion?
thanks in advance!

This could be a variable spec.keepResources in the Postgres CRD of type boolean.

When set to true it should keep the owner group role and database on deletion. This should be default.

When set to false it should drop the owner group role and database on deletion.

Currently you can't have users for the same database in more than one namespace. The role will be created in the second namespace but it won't get any privileges. This can be fixed by simply not returning from CreateDB on duplicate database error but I think there are more perks to splitting them apart.

So here's my idea:

apiVersion: db.movetokube.com/v1alpha1
kind: Postgres
metadata:
  name: my-db
  namespace: app
spec:
  database: test-db

This obviously creates the db test-db in postgres and a group role test-db-group that becomes the owner of the database.

CREATE ROLE test-db-group;
CREATE DATABASE test-db OWNER test-db-group;

apiVersion: db.movetokube.com/v1alpha1
kind: PostgresUser
metadata:
  name: my-db-user
  namespace: app
spec:
  role: test-db-user
  database: test-db
  secretName: test-db-postgres-user

This will create a user role test-db-user in postgres and then grant test-db-group to test-db-user.

CREATE ROLE test-db-user WITH LOGIN PASSWORD '<random>';
GRANT test-db-group TO test-db-user;

The PostgresUser CRD could even be extended to give us more powerful options.

apiVersion: db.movetokube.com/v1alpha1
kind: PostgresUser
metadata:
  name: my-db-user
  namespace: app
spec:
  role: test-db-user
  databases:
  - name: test-db
    privileges:
    - OWNER
  - name: another-db
    privileges:
    - SELECT
    - INSERT
  secretName: test-db-postgres-user

CREATE ROLE test-db-user WITH LOGIN PASSWORD '<random>';
GRANT test-db-group TO test-db-user;
GRANT SELECT ON DATABASE another-db TO test-db-user;
GRANT INSERT ON DATABASE another-db TO test-db-user;

I'm not 100% sure how to reproduce this, but this happened to me when I needed to remove postgres-operator form my cluster, and install it again, but wanting to keep things running.

The workloads that used this secrets was kept during this postgres-operator replacement, when the operator was back, I started getting this errors on its logs.

What's the correct procedure when doing this kind of operation ?

Hi there,

following up on my other issue (#67), I have seen your implementation for the generation of the secret in https://github.com/movetokube/postgres-operator/blob/master/pkg/controller/postgresuser/postgresuser_controller.go which includes the database credentials:

"POSTGRES_URL":      []byte(pgUserUrl),
"POSTGRES_JDBC_URL": []byte(pgJDBCUrl),
"HOST":              []byte(r.pgHost),
"DATABASE_NAME":     []byte(cr.Status.DatabaseName),
"ROLE":              []byte(role),
"PASSWORD":          []byte(password),
"LOGIN":             []byte(login),

Is there a reason why not all Variables are prefixed with "POSTGRES_"? I believe this would be beneficial since you can have Kubernetes apps which are integrated with multiple managed services like Postres AND Kafka etc. In that case you have multiple service credentials (potentially named LOGIN and PASSWORD). Thus, it would be a cleaner approach to prefix every variable with accordingly.

I understand this is a breaking change but I might be worth it. You could try to mitigate this by duplicating that variables for a transition period:

"POSTGRES_URL":      []byte(pgUserUrl),
"POSTGRES_JDBC_URL": []byte(pgJDBCUrl),

=== legacy variables ===
  "HOST":              []byte(r.pgHost),
  "DATABASE_NAME":     []byte(cr.Status.DatabaseName),
  "ROLE":              []byte(role),
  "PASSWORD":          []byte(password),
  "LOGIN":             []byte(login),
=== legacy variables ===

=== new variables ===
  "POSTGRES_HOST":              []byte(r.pgHost),
  "POSTGRES_DATABASE_NAME":     []byte(cr.Status.DatabaseName),
  "POSTGRES_ROLE":              []byte(role),
  "POSTGRES_PASSWORD":          []byte(password),
  "POSTGRES_LOGIN":             []byte(login),
=== new variables ===

best regards

This could be a variable spec.masterRole in the Postgres CRD of type string.

When set it should skip creating a group role owner of the database and instead use the existing one.

Now that Github Actions has been made generally available we should add CI where we can just tag a commit to make a release.