[GitHub] [airflow] potiuk commented on a change in pull request #11384: Mutual SSL added in PGBouncer configuration in the Chart

[GitBox <gi...@apache.org>]

potiuk commented on a change in pull request #11384:
URL: https://github.com/apache/airflow/pull/11384#discussion_r502696980



##########
File path: chart/templates/_helpers.yaml
##########
@@ -262,6 +266,13 @@ max_client_conn = {{ .Values.pgbouncer.maxClientConn }}
 verbose = {{ .Values.pgbouncer.verbose }}
 log_disconnections = {{ .Values.pgbouncer.logDisconnections }}
 log_connections = {{ .Values.pgbouncer.logConnections }}
+
+server_tls_sslmode = {{ .Values.pgbouncer.sslmode }}
+server_tls_ca_file = /etc/pgbouncer/root.crt

Review comment:
       I was waiting for this comment :) and I believe we should not guard it. 
   
   There is rather complex logic determining if those values should be used or not: https://www.pgbouncer.org/config.html
   
   The default setting now is "prefer" which requests the TLS from the server and if it is ok, it proceeds, but the server cert is not validated (so not used),  similar case is in "require" mode (surprisingly). If SSL is not accepted, the connection is rejected, but if it is accepted, neither the cA nor cert are validated (soi they are not used).
   
   Only in case of "verify-ca" or "verify-full" one or both are validated (respectively).
   
   Additionally - If the server does not request client certificate, it's not presented at all (thus not read). So the client cert is only actually used if the server requests it.
   
   So both server/client certs are not used by default even in  "prefer" mode. And having them "empty" makes no difference whatsoever.
   
   From my tests, the connection will work just fine in "prefer" mode if those are empty, and I'd rather leave it to the pgbouncer to use it as needed using its logic rather than trying to duplicate the logic in the helm chart.
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org