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

Expose way to configure the requests session so client certs or headers which need to be passed to a LB can be set #522

Open
1 task done
mrendi29 opened this issue Jan 22, 2025 · 2 comments
Labels
enhancement New feature or request

Comments

@mrendi29
Copy link

mrendi29 commented Jan 22, 2025

Describe the feature

In cases where Trino is sitting behind a LB which is doing mTLS there is no way to connect to Trino using the python client.

Using the trino java cli this would be: ./trino --server https://trino-server.com/ --keystore-path=certs.pem --keystore-type=pem --truststore-path=ca.pem --user=test --catalog tpch --password

After a conversation with @hashhar:

We need to expose a way to configure the requests session when creating a connection so client certs or headers which need to be passed to LB (which Trino doesn't care about) can be set

I see two options. Add a dedicated arg to the connect method to present client cert. I worry it might confuse people who don't know mtls to figure out what's different between verify and cert/client_cert and they might try to use it interchangeably and fail.

Or make it possible to configure the requests session we use for making requests - this would be more flexible at the cost of complexity to the end user maybe.

I am willing to work on this this/next week.

Describe alternatives you've considered

No response

Are you willing to submit PR?

  • Yes I am willing to submit a PR!
@hashhar hashhar added the enhancement New feature or request label Jan 23, 2025
@hashhar
Copy link
Member

hashhar commented Jan 23, 2025

Thanks for deciding to work on this.

@ebyhr / @damian3031 do you have opinions between the two choices?

  • new arg to connect to present cert + client_cert for mTLS?
  • allow customizing the requests library Session that we use (by passing it into connect for example).

I worry the first option can create confusion compared to verify arg which is for self-signed certs and nothing to do with mTLS? We can probably add a single dict[string, string] arg to connect called mtls_config maybe to clarify?

@mrendi29
Copy link
Author

mrendi29 commented Jan 23, 2025

I wonder if there is also a way to make this work with superset as well: https://superset.apache.org/docs/configuration/databases/#trino (maybe out of scope for this fix)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

No branches or pull requests

2 participants