-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Command line flag to set the sslmode for PostgreSQL (Closes: #326) #353
Conversation
This PR has been without a review for over a year, let's try to get it some reviews now. If the git commit subject line was
then the issue in question would be auto-closed when this is merged on master. In general it is also good practice to write a git commit message body. In this case if would have explained that the implementation and naming follows the existing mysql-ssl added in e5c8052, or at least almost. This PR should also add/update tests, at least the file https://github.com/akopytov/sysbench/blob/master/tests/t/help_drv_pgsql.t When you @ddinu update this, please also rebase on latest master. Thanks! |
Hi @ottok,I have updated the commit and PR with your suggestions, I hope the changes are ok. Thank you for your feedback! |
…#326) The flag name and its values match libpq's sslmode connection parameter. The default value (prefer) will first try an SSL connection; if that fails, it will try a non-SSL connection. Libpq documentation: https://www.postgresql.org/docs/14/libpq-connect.html#LIBPQ-CONNECT-SSLMODE
Hi @ddinu @ottok and @akopytov, please find the testing evidence of this change below. Step-by-step shown to facilitate the repro of the test if required. Testing1- install docker containers
2- setup postgres server to run in ssl mode
run in container
restart container
3- compile sysbench
run in container
4- run a test with sslmode=prefer (ssl enabled)
run in container
run benchmark
On a second Terminal while "sysbench run" is running confirm ssl is enabled since argument is "--pgsql-sslmode=prefer". To confirm this look at the query below and check column ssl with value "t" (true) indicating ssl is enabled.
check database connection
5- run a test with sslmode=disable (ssl disabled) On first terminal
run a benchmark
On a second Terminal while "sysbench run" is running confirm ssl is disabled since argument is "--pgsql-sslmode=disable". To confirm this look at the query below and check column ssl with value "f" (false) indicating ssl is disabled.
check database connection
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change has been tested locally using docker. It does work when pgsql-sslmode is set to prefer or disable, see my comments in the Conversation #353 (comment).
Approved.
@akopytov This seems complete, tested and reviewed to me. Cloud you Alexey please merge it? Thanks :) |
Merged. My apologies for a very slow response. Thank you for the contribution! |
The flag name and its values match libpq's sslmode connection parameter.
The default value (prefer) will first try an SSL connection; if that fails, it will try a non-SSL connection.
Libpq documentation: https://www.postgresql.org/docs/14/libpq-connect.html#LIBPQ-CONNECT-SSLMODE