tpope / vim-dadbod

dadbod.vim: Modern database interface for Vim
https://www.vim.org/scripts/script.php?script_id=5665
3.75k stars 132 forks source link

Adding ssl support to presto adapter. #122

Closed tcopple closed 7 months ago

tcopple commented 1 year ago

Currently the presto adapter doesn't support they keysstore-path and truststore-path parameters which can be used for custom SSL key setups. This is an attempt to add support for those.

Also, I had an issue any time I specified a :DB presto://https://hostname?... I opted to add support for it as a parameter on the schema. I imagine there's a better place to add this support, maybe db#url#parse but I'm not very familiar with vimscript regex differences, happy to rework that bit if necessary.

tpope commented 1 year ago

Also, I had an issue any time I specified a :DB presto://https://hostname?... I opted to add support for it as a parameter on the schema.

Is this presto://https://hostname?... style URL a thing people do? Is there other software that supports it?

tcopple commented 1 year ago

I don't know if that's a thing, I expect that it isn't. It was unclear to me how to specify the SSL spec at the beginning of the url via the interface provided by vim-dadbod. The CLI accepts it in something like, > trino --server https://... that's what I was trying to mimic.

tpope commented 1 year ago

Okay, did a big of digging, and I think the approach to take is to mimic the approach used for JDBC URLs, which is basically the same as what you have except the parameter is named SSL. Additionally, we should make sure not to use SSL when SSL=false. Recommended conditional:

if get(url.params, 'SSL') =~# '^[1tTyY]'

(This loose match also allows for SSL=1 and SSL=yes, which may or may not be necessary, but there's no downside to false positives, and it follows the pattern established in the SQL Server adapter.)

And it follows naturally that the other 2 URL parameters should be named SSLKeyStorePath and SSLTrustStorePath.

Edit: For future reference: the CLI documentation (this was surprisingly hard to track down)