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 PostgreSQL SSL options support #829

Merged
merged 4 commits into from Nov 29, 2018
Merged

Add PostgreSQL SSL options support #829

merged 4 commits into from Nov 29, 2018

Conversation

oldskool
Copy link
Contributor

@oldskool oldskool commented Jun 27, 2018

I was trying to setup an SSL connection to a PostgreSQL database, using the pdo_pgsql driver. According to the documentation, the following SSL options should be supported:

  • sslmode
  • sslrootcert
  • sslcert
  • sslkey
  • sslcrl

Although, when setting the configuration as such (with the given parameters set in parameters.yml file):

doctrine:
  dbal:
    sslmode: 'verify-ca'
    sslrootcert: '%database_sslrootcert_path%'
    sslcert: '%database_sslcert_path%'
    sslkey: '%database_sslkey_path%'

I would get the following exception from the Symfony Config component:

In ArrayNode.php line 311:
                                                                                    
  [Symfony\Component\Config\Definition\Exception\InvalidConfigurationException]     
  Unrecognized options "sslcert, sslkey" under "doctrine.dbal.connections.default"

This patch allows for these options to be actually set/used and results in a working SSL connection.

I'm not sure if/how I should write any tests for this change, but please give me some pointers if these need to be added.

@k20human
Copy link

Any news on this ?

@kimhemsoe
Copy link
Member

@oldskool Can you provide some tests?

@oldskool
Copy link
Contributor Author

oldskool commented Oct 30, 2018

@kimhemsoe I checked the current source against occurences of the sslrootcert key that was already there and supported. I basically added the "new" sslcert, sslkey and sslcrl keys to this files as well. I figure the complete configuration is tested against the DI test, so this might be sufficient?

If more tests need to be added, please let me know!

@oldskool
Copy link
Contributor Author

@kimhemsoe Were the tests sufficient or do additional tests need to be added before this can be merged?

@kimhemsoe kimhemsoe self-requested a review November 29, 2018 10:10
@kimhemsoe kimhemsoe added this to the 1.9.2 milestone Nov 29, 2018
@kimhemsoe kimhemsoe merged commit bca3c13 into doctrine:master Nov 29, 2018
@kimhemsoe
Copy link
Member

@oldskool Thanks, sorry missed that you already had added the tests

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants