Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

External postgress connection string is created incorrectly #5374

Open
DarthSlider opened this issue Mar 23, 2023 · 3 comments
Open

External postgress connection string is created incorrectly #5374

DarthSlider opened this issue Mar 23, 2023 · 3 comments

Comments

@DarthSlider
Copy link

Issue description

Postgress connection string generation is incorrect

Details

Here is my helm values:

  db:
    enabled: true
    external: true
    source:
      connectionString: postgresql://user@10.0.0.1:5000/stackrox-db
      minConns: 10
      maxConns: 90
      statementTimeoutMs: 60000
    password:
      value: <path:dp-srv/prod/patroni/prod1#stackrox-prod>

Here is an error i`ve got:

globaldb: 2023/03/23 22:42:59.846963 postgres.go:84: Fatal: Could not parse postgres config: Could not parse postgres config: cannot parse `postgresql:xxxxxx@10.48.5.69:5000/stackrox-db-prod statement_timeout=60000 pool_min_conns=10 pool_max_conns=90 password=xxxxx`: failed to parse as URL (parse "postgresql://stackrox-prod@10.48.5.69:5000/stackrox-db-prod statement_timeout=60000 pool_min_conns=10 pool_max_conns=90 password=Non-Masked-Password\n": net/url: invalid control character in URL)

Looks like there are 2 erorrs, one in helm chart:
https://github.com/stackrox/helm-charts/blob/main/3.74.1/central-services/templates/01-central-08-external-db-configmap.yaml#L16
It should be

source: {{ ._rox.central.db.source.connectionString }}?statement_timeout={{ ._rox.central.db.source.statementTimeoutMs }}&pool_min_conns={{ ._rox.central.db.source.minConns }}&pool_max_conns={{ ._rox.central.db.source.maxConns }}

instead

And another one is here:

source := fmt.Sprintf("%s password=%s", centralConfig.CentralDB.Source, password)

I`m not that good with golang, but looks like it should be:

source := fmt.Sprintf("%s&password=%s", centralConfig.CentralDB.Source, password)

But it would be even better to add password to the username separated with ":", because then it is added as an parameter it is not masked properly in logs.

@connorgorman
Copy link
Collaborator

@DarthSlider We currently only support keyword/value connection strings and not URIs. I will specify this more completely in the helm documentation. https://www.postgresql.org/docs/current/static/libpq-connect.html#LIBPQ-CONNSTRING

@DarthSlider
Copy link
Author

Looks like it ignores dbname key
Here is my connection string:
source: "host=10.0.0.1 port=5000 dbname=stackrox-db-prod user=stackrox-user statement_timeout=60000 pool_min_conns=10 pool_max_conns=90"

Also:
"NOTICE: database "central_backup" does not exist, skipping"
looks like it`s not suited for shared PG servers.
It would be great if there would be a configurable list of needed databases, so you could create them in advance

@porridge
Copy link
Contributor

FTR, there are plans to support a "bring your own DB" use case better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants