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

Add ability to use existing postgres server with infisical-standalone helm chart. Fixes #1765 #1766

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

mpsOxygen
Copy link

@mpsOxygen mpsOxygen commented Apr 30, 2024

Description 📣

This adds the option to use a existing Postgres instance with the infisical-standalone helm chart. It uses an existing secret to get the database connection URI.

Fixes: #1765

Type ✨

  • Bug fix
  • New feature
  • Breaking change
  • Documentation

Tests 🛠️

I set the values and run helm template .


Razvan Valceanu added 3 commits April 30, 2024 14:47
…re to get connection URI

Signed-off-by: Razvan Valceanu <razvan.valceanu@metaminds.com>
Signed-off-by: Razvan Valceanu <razvan.valceanu@metaminds.com>
Signed-off-by: Razvan Valceanu <razvan.valceanu@metaminds.com>
@mpsOxygen
Copy link
Author

While waiting for this to be approved I also tested this in my production environment. Works as intended.

@maidul98
Copy link
Collaborator

maidul98 commented May 2, 2024

Hey @mpsOxygen, you should already be able to do this when you disable the in cluster Postgres and set your own postgres connection URL via DB_CONNECTION_URI. An example is shown in step 4 here https://infisical.com/docs/self-hosting/deployment-options/kubernetes-helm. Is this helpful?

https://github.com/Infisical/infisical/blob/main/helm-charts/infisical-standalone-postgres/templates/infisical.yaml#L67

@mpsOxygen
Copy link
Author

mpsOxygen commented May 2, 2024

Hello @maidul98,

I know about kubeSecretRef: "infisical-secrets", but I now realize I did not explain my use case correctly.

I use an operator to take care of postgres. When it spins up a postgres cluster for me, it creates a secret with all needed information (db name, user, pass, uri, etc). The password for example is randomly generated each time a cluster is spun up. Or it maybe even changes. The operator takes care of everything and puts it in a secret it creates and manages. All I need from my apps is to say: get your DB_CONNECTION_URI from the secret the operator created. So while I know about the secret that gives infisical it's ENVs, using it would mean to manually update it every time something changes. This patch fixes my problem because I just tell infisical the name of the secret the postgres operator uses to keep postgres connection info.

@mpsOxygen
Copy link
Author

Any updates on this? I am willing to make any changes necessary to the code to get it merged.

@WladyX
Copy link

WladyX commented Jun 3, 2024

I'm also using cnpg postgres and would like to see this one merged.

@mpsOxygen
Copy link
Author

Any updates on this? I am willing to make any changes necessary to the code to get it merged.

Copy link
Collaborator

@maidul98 maidul98 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR, i understand your usecase. Currently, we have:

{{- if .Values.postgresql.enabled }}
        - name: DB_CONNECTION_URI
          value: {{ include "infisical.postgresDBConnectionString" . }}
{{- end }}

I think what might make it user for the rest of the user is if we instead have the default connection URL be from {{ include "infisical.postgresDBConnectionString" . }} or user defined one. User defined one can be from the infisical-secret if defined or from the custom one you are adding here.

Does this make sense? If so, can you please make these changes? Thanks

@mpsOxygen
Copy link
Author

It's not clear exactly what changes you want. The current PR leaves current functionality untouched, it just adds an option to enable defining a preexisting secret with the connection string. Could you be more specific please?

@davidshare
Copy link

I see what you are pointing out @mpsOxygen - I am facing the same issues with the chart. In my case, I still want to use the postgres server provided in the charts, but I do not want to expose my secrets in a values file. Which should leave me with an option to pass an existing secret.

@mpsOxygen
Copy link
Author

mpsOxygen commented Sep 5, 2024

@davidshare You could just use the bitami Postgres chart functionality for that: https://artifacthub.io/packages/helm/bitnami/postgresql?modal=values

The path is: postgresql.auth.existingSecret

@cook1emr
Copy link

cook1emr commented Sep 6, 2024

@mpsOxygen have you had clarity from @maidul98 ? I would love to see this merged to use easily with cnpg

@mpsOxygen
Copy link
Author

@cook1emr unfortunately no.

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

Successfully merging this pull request may close these issues.

Ability to use preexisting postgres server with infisical-standalone helm chart
5 participants