Skip to content

Conversation

zregvart
Copy link

Description of your changes

This adds sslCert, sslKey and sslRootCert options to the PostgreSQL ProviderConfig needed to establish a connection to a database that either uses non-public certificates or requires client certificate authentication.

I've opted to create the Options struct to hold these and the existing sslMode parameter, this will allow adding other options[1] easier in the future.

[1] https://www.postgresql.org/docs/current/libpq-connect.html#LIBPQ-PARAMKEYWORDS

I have:

  • Read and followed Crossplane's contribution process.
  • Run make reviewable to ensure this PR is ready for review.

How has this code been tested

Added a simple unit test

This adds `sslCert`, `sslKey` and `sslRootCert` options to the
PostgreSQL `ProviderConfig` needed to establish a connection to a
database that either uses non-public certificates or requires client
certificate authentication.

I've opted to create the `Options` struct to hold these and the existing
`sslMode` parameter, this will allow adding other options[1] easier in
the future.

[1] https://www.postgresql.org/docs/current/libpq-connect.html#LIBPQ-PARAMKEYWORDS

Signed-off-by: Zoran Regvart <[email protected]>
@chlunde
Copy link
Collaborator

chlunde commented May 27, 2025

Clear an nice code, thanks!

One worry:

If we land on this design:

  • Would it be possible to add a small e2e test that shows that this works, and ensure it works when other people do changes, without cluttering the e2e tests significantly?
  • Could you add a full example of this use case to the documentation or create a discussion that shows how to actually mount the files?

@chlunde chlunde self-requested a review May 27, 2025 19:32
@chlunde chlunde linked an issue May 27, 2025 that may be closed by this pull request
@zregvart
Copy link
Author

Thanks for having a look. Yes, this would require mounting files to the provider container, this was simplest for me to do. I can also, or solely, use a Secret, the secret would need to be fetched and placed in the $TMPDIR as well, or, I think, the sslinline=true option could be used to provide the certificate and keys inline.

I had a stab at the e2e tests, and I encountered a number of issues with port forwarding, not starting from the clean slate, etc. I have a work in progress commit here. Have a look at how the CA certificate from secret auto-generated by the Helm chart for PostgreSQL is mounted.

Adds e2e tests for the functionality of providing the TLS options by
testing with TLS turned on on the server side using the
`tls.enabled=true` and `tls.autoGenerated=true` PostgreSQL Chart
parameters.

To make the tests as robust as possible: port forwarding is not used,
but rather exposing the server port via NodePort; timeouts are increased
on kubectl wait's, from 2 min to 3 min.

The test iself reuses parts of the existing tests, and adds one
additional check that asserts if TLS is used.

Signed-off-by: Zoran Regvart <[email protected]>
An example on how to use custom TLS certificates with PostgreSQL.

Signed-off-by: Zoran Regvart <[email protected]>
@zregvart
Copy link
Author

zregvart commented Jun 2, 2025

@chlunde I've added the e2e tests in 3ff7952 and an example in 2e971b5, when you get a chance I would appreciate if you could take another look.

The GitHub Discussions don't appear to be enabled, or I can't access them.

@chlunde
Copy link
Collaborator

chlunde commented Jun 2, 2025

@zregvart cool, I do think we should look at secrets. For the public key it might make sense with a file but for client certs that might be per server or even role, right? I'll have to research a bit here and come back to you.

@zregvart
Copy link
Author

zregvart commented Jun 3, 2025

I'll work on adding support for reading the key and certificates from the Secret as well, I think this can be done via the Options struct I added.

Adds support for configuring TLS options using the credentials Secret.
Very similar to how other providers are configured, this now also reads
the `clusterCA`, `clientCert` and `clientKey` keys from the Secret.

Integration tests are added, though they could be optimized to setup the
PostgreSQL database only once and modify the configuration as needed to
reduce the lenght of the tests, left as is for now.

Signed-off-by: Zoran Regvart <[email protected]>
@zregvart
Copy link
Author

zregvart commented Jun 3, 2025

0f2afad adds reading the Secret in addition to the changes made to the ProviderConfig previously. I needed to place the data from the Secret in a temporary file, the sslinline=true was not helpful the values were broken up on the newlines of the PEM encoded certificates/keys and the single quoting the value did not help as the value was escaped and had no impact.

@chlunde when you get a chance I'd be happy to receive any feedback on this

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 certificat in sslmode
2 participants