proxy: refactor how neon-options are handled (#6306)

## Problem

HTTP connection pool was not respecting the PitR options.

## Summary of changes

1. refactor neon_options a bit to allow easier access to cache_key
2. make HTTP not go through `StartupMessageParams`
3. expose SNI processing to replace what was removed in step 2.

proxy/src/serverless/conn_pool.rs

@@ -1,4 +1,4 @@
-use anyhow::{anyhow, Context};
+use anyhow::Context;
 use async_trait::async_trait;
 use dashmap::DashMap;
 use futures::{future::poll_fn, Future};
@@ -9,7 +9,6 @@ use pbkdf2::{
     password_hash::{PasswordHashString, PasswordHasher, PasswordVerifier, SaltString},
     Params, Pbkdf2,
 };
-use pq_proto::StartupMessageParams;
 use prometheus::{exponential_buckets, register_histogram, Histogram};
 use rand::Rng;
 use smol_str::SmolStr;
@@ -30,7 +29,7 @@ use crate::{
     console,
     context::RequestMonitoring,
     metrics::NUM_DB_CONNECTIONS_GAUGE,
-    proxy::{connect_compute::ConnectMechanism, neon_options},
+    proxy::connect_compute::ConnectMechanism,
     usage_metrics::{Ids, MetricCounter, USAGE_METRICS},
 };
 use crate::{compute, config};
@@ -38,28 +37,37 @@ use crate::{compute, config};
 use tracing::{debug, error, warn, Span};
 use tracing::{info, info_span, Instrument};
 
-pub const APP_NAME: &str = "/sql_over_http";
+pub const APP_NAME: SmolStr = SmolStr::new_inline("/sql_over_http");
 
 #[derive(Debug, Clone)]
 pub struct ConnInfo {
-    pub username: SmolStr,
+    pub user_info: ComputeUserInfo,
     pub dbname: SmolStr,
-    pub hostname: SmolStr,
     pub password: SmolStr,
-    pub options: Option<SmolStr>,
 }
 
 impl ConnInfo {
     // hm, change to hasher to avoid cloning?
     pub fn db_and_user(&self) -> (SmolStr, SmolStr) {
-        (self.dbname.clone(), self.username.clone())
+        (self.dbname.clone(), self.user_info.user.clone())
+    }
+
+    pub fn endpoint_cache_key(&self) -> SmolStr {
+        self.user_info.endpoint_cache_key()
     }
 }
 
 impl fmt::Display for ConnInfo {
     // use custom display to avoid logging password
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(f, "{}@{}/{}", self.username, self.hostname, self.dbname)
+        write!(
+            f,
+            "{}@{}/{}?{}",
+            self.user_info.user,
+            self.user_info.endpoint,
+            self.dbname,
+            self.user_info.options.get_cache_key("")
+        )
     }
 }
 
@@ -319,7 +327,7 @@ impl GlobalConnPool {
         let mut hash_valid = false;
         let mut endpoint_pool = Weak::new();
         if !force_new {
-            let pool = self.get_or_create_endpoint_pool(&conn_info.hostname);
+            let pool = self.get_or_create_endpoint_pool(&conn_info.endpoint_cache_key());
             endpoint_pool = Arc::downgrade(&pool);
             let mut hash = None;
 
@@ -401,7 +409,7 @@ impl GlobalConnPool {
             Err(err)
                 if hash_valid && err.to_string().contains("password authentication failed") =>
             {
-                let pool = self.get_or_create_endpoint_pool(&conn_info.hostname);
+                let pool = self.get_or_create_endpoint_pool(&conn_info.endpoint_cache_key());
                 let mut pool = pool.write();
                 if let Some(entry) = pool.pools.get_mut(&conn_info.db_and_user()) {
                     entry.password_hash = None;
@@ -418,7 +426,7 @@ impl GlobalConnPool {
                 })
                 .await??;
 
-                let pool = self.get_or_create_endpoint_pool(&conn_info.hostname);
+                let pool = self.get_or_create_endpoint_pool(&conn_info.endpoint_cache_key());
                 let mut pool = pool.write();
                 pool.pools
                     .entry(conn_info.db_and_user())
@@ -520,23 +528,11 @@ async fn connect_to_compute(
     conn_id: uuid::Uuid,
     pool: Weak<RwLock<EndpointConnPool>>,
 ) -> anyhow::Result<ClientInner> {
-    let tls = config.tls_config.as_ref();
-    let common_names = tls.and_then(|tls| tls.common_names.clone());
-
-    let params = StartupMessageParams::new([
-        ("user", &conn_info.username),
-        ("database", &conn_info.dbname),
-        ("application_name", APP_NAME),
-        ("options", conn_info.options.as_deref().unwrap_or("")),
-    ]);
-    let creds =
-        auth::ClientCredentials::parse(ctx, &params, Some(&conn_info.hostname), common_names)?;
-
-    let creds =
-        ComputeUserInfo::try_from(creds).map_err(|_| anyhow!("missing endpoint identifier"))?;
-    let backend = config.auth_backend.as_ref().map(|_| creds);
-
-    let console_options = neon_options(&params);
+    ctx.set_application(Some(APP_NAME));
+    let backend = config
+        .auth_backend
+        .as_ref()
+        .map(|_| conn_info.user_info.clone());
 
     if !config.disable_ip_check_for_http {
         let allowed_ips = backend.get_allowed_ips(ctx).await?;
@@ -544,11 +540,8 @@ async fn connect_to_compute(
             return Err(auth::AuthError::ip_address_not_allowed().into());
         }
     }
-    let extra = console::ConsoleReqExtra {
-        options: console_options,
-    };
     let node_info = backend
-        .wake_compute(ctx, &extra)
+        .wake_compute(ctx)
         .await?
         .context("missing cache entry from wake_compute")?;
 
@@ -563,7 +556,6 @@ async fn connect_to_compute(
             idle: config.http_config.pool_options.idle_timeout,
         },
         node_info,
-        &extra,
         &backend,
     )
     .await
@@ -582,7 +574,7 @@ async fn connect_to_compute_once(
     let mut session = ctx.session_id;
 
     let (client, mut connection) = config
-        .user(&conn_info.username)
+        .user(&conn_info.user_info.user)
         .password(&*conn_info.password)
         .dbname(&conn_info.dbname)
         .connect_timeout(timeout)

proxy/src/serverless/sql_over_http.rs

@@ -28,9 +28,13 @@ use url::Url;
 use utils::http::error::ApiError;
 use utils::http::json::json_response;
 
+use crate::auth::backend::ComputeUserInfo;
+use crate::auth::endpoint_sni;
 use crate::config::HttpConfig;
+use crate::config::TlsConfig;
 use crate::context::RequestMonitoring;
 use crate::metrics::NUM_CONNECTION_REQUESTS_GAUGE;
+use crate::proxy::NeonOptions;
 
 use super::conn_pool::ConnInfo;
 use super::conn_pool::GlobalConnPool;
@@ -125,6 +129,7 @@ fn get_conn_info(
     ctx: &mut RequestMonitoring,
     headers: &HeaderMap,
     sni_hostname: Option<String>,
+    tls: &TlsConfig,
 ) -> Result<ConnInfo, anyhow::Error> {
     let connection_string = headers
         .get("Neon-Connection-String")
@@ -179,8 +184,10 @@ fn get_conn_info(
         }
     }
 
-    let hostname: SmolStr = hostname.into();
-    ctx.set_endpoint_id(Some(hostname.clone()));
+    let endpoint = endpoint_sni(hostname, &tls.common_names)?;
+
+    let endpoint: SmolStr = endpoint.into();
+    ctx.set_endpoint_id(Some(endpoint.clone()));
 
     let pairs = connection_url.query_pairs();
 
@@ -188,22 +195,27 @@ fn get_conn_info(
 
     for (key, value) in pairs {
         if key == "options" {
-            options = Some(value.into());
+            options = Some(NeonOptions::parse_options_raw(&value));
             break;
         }
     }
 
+    let user_info = ComputeUserInfo {
+        endpoint,
+        user: username,
+        options: options.unwrap_or_default(),
+    };
+
     Ok(ConnInfo {
-        username,
+        user_info,
         dbname: dbname.into(),
-        hostname,
         password: password.into(),
-        options,
     })
 }
 
 // TODO: return different http error codes
 pub async fn handle(
+    tls: &'static TlsConfig,
     config: &'static HttpConfig,
     ctx: &mut RequestMonitoring,
     request: Request<Body>,
@@ -212,7 +224,7 @@ pub async fn handle(
 ) -> Result<Response<Body>, ApiError> {
     let result = tokio::time::timeout(
         config.request_timeout,
-        handle_inner(config, ctx, request, sni_hostname, conn_pool),
+        handle_inner(tls, config, ctx, request, sni_hostname, conn_pool),
     )
     .await;
     let mut response = match result {
@@ -294,6 +306,7 @@ pub async fn handle(
 
 #[instrument(name = "sql-over-http", fields(pid = tracing::field::Empty), skip_all)]
 async fn handle_inner(
+    tls: &'static TlsConfig,
     config: &'static HttpConfig,
     ctx: &mut RequestMonitoring,
     request: Request<Body>,
@@ -308,7 +321,7 @@ async fn handle_inner(
     // Determine the destination and connection params
     //
     let headers = request.headers();
-    let conn_info = get_conn_info(ctx, headers, sni_hostname)?;
+    let conn_info = get_conn_info(ctx, headers, sni_hostname, tls)?;
 
     // Determine the output options. Default behaviour is 'false'. Anything that is not
     // strictly 'true' assumed to be false.