Add dynamic credentials support to Postgres
Fixes https://github.com/launchbadge/sqlx/issues/445.
We've been using this in development to refresh RDS tokens and it works pretty well. Some outstanding work is left to do error typing properly.
I just closed #1349 as not-planned but it just occurred to me that if we pivot this to being callback that provides ConnectOptions on-demand then it can cover both use-cases plus pretty much any thing else conceivable, and also wouldn't require baking this feature into every database's ConnectOptions separately.
I just closed #1349 as not-planned but it just occurred to me that if we pivot this to being callback that provides
ConnectOptionson-demand then it can cover both use-cases plus pretty much any thing else conceivable, and also wouldn't require baking this feature into every database'sConnectOptionsseparately.
That seems like a good idea! I had a quick look at generalizing this approach in the way you described and the only thing that immediately jumps out as a potential blocker is: https://github.com/launchbadge/sqlx/blob/79ebd3005a404d2a4452ee2a0284b039e96d84a2/sqlx-core/src/pool/mod.rs#L512-L519
We would need to execute the callback in order to determine the kind, and if the callback returns a future this method would have to become async as well. This feels like it would be a surprising change if I were using the Any driver.
Hmm, that is a problem, yeah. I would probably say that we shouldn't have the callback just return AnyConnectOptions in that case as changing database kinds on the fly sounds like a nightmare to reason about.
I'm conceiving of a new trait and impls for this:
pub trait ConnectOptionsProvider<DB: Database>: Send + Sync + 'static {
type Error: Into<Box<dyn Error>>;
type Future: Future<Output = Result<DB::ConnectOptions, Self::Error>>;
fn any_kind(&self) -> AnyKind;
fn provide_connect_options(&self) -> Self::Future;
}
#[cfg(feature = "postgres")]
impl<F, Fut, E> ConnectOptionsProvider<Postgres> for F
where
F: Fn() -> Fut + Send + Sync + 'static,
E: Into<Box<dyn Error>>,
Fut: Future<Output = Result<PgConnectOptions, E>>
{
type Error = E;
type Future = Fut;
fn any_kind(&self) -> AnyKind { AnyKind::Postgres }
fn provide_connect_options(&self) -> Self::Future { (self)() }
}
// plus impls for other dbs
We can't have a blanket impl of ConnectOptionsProvider<Any> for these types because of coherence issues, but we can use wrapper types. I'm thinking of something like this:
// Bikeshedding?
#[cfg(feature = "postgres")]
pub fn provide_pg_options_as_any<P: ConnectOptionsProvider<Postgres>>(provider: P) -> impl ConnectOptionsProvider<Any> {
struct PgWrapper<P>(P);
impl<P> ConnectOptionsProvider<Any> for PgWrapper<P>
where P: ConnectOptionsProvider<Postgres> {
type Error = P::Error;
type Future = futures_util::future::OkInto<P::Future, AnyConnectOptions>;
fn any_kind(&self) -> AnyKind { AnyKind::Postgres }
fn provide_connect_options(&self) -> Self::Future { self.0.provide_connect_options().ok_into() }
}
And then sqlx::Error would gain a new error variant, ProviderError(Box<dyn Error>)
@abonander I took that idea and ran with it. I think its definitely feasible and I was able to thread it through until I ran into one blocker which is the following:
https://github.com/launchbadge/sqlx/blob/58712ae55263857c60c0df5f1adac8a076a74fab/sqlx-core/src/pool/mod.rs#L493
If the pool holds onto the ConnectOptionsProvider, we cannot evaluate this method synchronously. I expect we wouldn't want to fork the Pool<DB> type, but maybe we could put this feature (ConnectOptionProvider) behind a feature flag which could remove that method/make it async.
What do you think?
I think the easiest fix would just be to panic if an async ConnectOptionsProvider was set. As long as it's well documented I don't see any problem with that.
Closing due to inactivity from the original author. @jamiebrynes7 I don't think I ever saw a PR from you, I'm still interested in your approach.