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

Improve database client options templating #439

Conversation

joachimBurket
Copy link
Contributor

@joachimBurket joachimBurket commented Dec 10, 2024

The sslrootcert parameter specifies the name of a file containing SSL certificate authority (CA) certificate(s). If the file exists, the server's certificate will be verified to be signed by one of these authorities (in the case of an SSL connection to the PostgreSQL DB).

See the option in the documentation here: https://www.postgresql.org/docs/current/libpq-connect.html#LIBPQ-CONNECT-SSLROOTCERT

Resolves #417

@LeoColomb
Copy link
Member

LeoColomb commented Dec 10, 2024

Thanks for submitting this pull request, @joachimBurket.
I'd like to suggest another approach.
I think it would be more future-proof and more flexible for other engines to render the values directly under a new .Values.externalDatabase.options.

It would be something like that:

      OPTIONS: {{- include "common.tplvalues.render" (dict "value" .Values.externalDatabase.options "context" $) | nindent 8 }}

What do you think?
That would help to cover the bunch of potential parameters: https://www.postgresql.org/docs/current/libpq-connect.html#LIBPQ-PARAMKEYWORDS
/cc @RangerRick

@RangerRick
Copy link
Contributor

@LeoColomb yeah, I like this; should be pretty easy to implement and avoids a TON of boilerplate on the user's part

@RangerRick
Copy link
Contributor

and allows you to decide how to set sslmode, whether you want to set up client certificates as well, etc.

charts/netbox/templates/configmap.yaml Outdated Show resolved Hide resolved
charts/netbox/values.yaml Outdated Show resolved Hide resolved
charts/netbox/README.md Outdated Show resolved Hide resolved
@joachimBurket
Copy link
Contributor Author

Sorry for the delay, I haven't seen the notification. Yes this seems a way better idea! 👍

@LeoColomb LeoColomb changed the title Add Postgresql sslrootcert option Improve database client options templating Dec 18, 2024
@LeoColomb LeoColomb force-pushed the fix/support-sslrootcert-option branch from 57918a3 to fc2b6ab Compare December 18, 2024 16:28
@LeoColomb LeoColomb force-pushed the fix/support-sslrootcert-option branch from 6cd2377 to 06df5bc Compare December 18, 2024 16:44
@LeoColomb LeoColomb enabled auto-merge (squash) December 18, 2024 16:45
Copy link
Contributor

@RangerRick RangerRick left a comment

Choose a reason for hiding this comment

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

LGTM, I've also added this to our 5.0.0 release notes WIP:

* The `externalDatabase.sslMode` setting has been removed and replaced by an `options` map that can take an arbitrary list of extra PostgreSQL options.

@RangerRick RangerRick disabled auto-merge December 18, 2024 16:47
@RangerRick RangerRick merged commit ab76db1 into netbox-community:develop Dec 18, 2024
7 of 8 checks passed
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.

Support Postgresql sslrootcert option
4 participants