From df1f8e13c45930f9ad53fe18076186616fed3f80 Mon Sep 17 00:00:00 2001 From: Andrew Rudenko Date: Fri, 8 Dec 2023 19:58:36 +0100 Subject: [PATCH] proxy: pass neon options in deep object format (#6068) --------- Co-authored-by: Conrad Ludgate --- Cargo.lock | 33 +++++++++++++++++-------- Cargo.toml | 2 +- proxy/src/auth/credentials.rs | 9 +++---- proxy/src/compute.rs | 4 +-- proxy/src/console/provider.rs | 13 +++++++++- proxy/src/console/provider/neon.rs | 13 +++++++--- proxy/src/proxy.rs | 39 +++++++++++++++--------------- proxy/src/proxy/tests.rs | 2 +- proxy/src/serverless/conn_pool.rs | 2 +- workspace_hack/Cargo.toml | 6 +++-- 10 files changed, 76 insertions(+), 47 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d9489cdd97..1364c9d84f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -896,7 +896,7 @@ checksum = "a246e68bb43f6cd9db24bea052a53e40405417c5fb372e3d1a8a7f770a564ef5" dependencies = [ "memchr", "once_cell", - "regex-automata", + "regex-automata 0.1.10", "serde", ] @@ -2543,7 +2543,7 @@ version = "0.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8263075bb86c5a1b1427b5ae862e8889656f126e9f77c484496e8b47cf5c5558" dependencies = [ - "regex-automata", + "regex-automata 0.1.10", ] [[package]] @@ -2569,9 +2569,9 @@ checksum = "490cc448043f947bae3cbee9c203358d62dbee0db12107a74be5c30ccfd09771" [[package]] name = "memchr" -version = "2.5.0" +version = "2.6.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2dffe52ecf27772e601905b7522cb4ef790d2cc203488bbd0e2fe85fcb74566d" +checksum = "f665ee40bc4a3c5590afb1e9677db74a508659dfd71e126420da8274909a0167" [[package]] name = "memoffset" @@ -3820,13 +3820,14 @@ dependencies = [ [[package]] name = "regex" -version = "1.8.2" +version = "1.10.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d1a59b5d8e97dee33696bf13c5ba8ab85341c002922fba050069326b9c498974" +checksum = "380b951a9c5e80ddfd6136919eef32310721aa4aacd4889a8d39124b026ab343" dependencies = [ "aho-corasick", "memchr", - "regex-syntax 0.7.2", + "regex-automata 0.4.3", + "regex-syntax 0.8.2", ] [[package]] @@ -3838,6 +3839,17 @@ dependencies = [ "regex-syntax 0.6.29", ] +[[package]] +name = "regex-automata" +version = "0.4.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5f804c7828047e88b2d32e2d7fe5a105da8ee3264f01902f796c8e067dc2483f" +dependencies = [ + "aho-corasick", + "memchr", + "regex-syntax 0.8.2", +] + [[package]] name = "regex-syntax" version = "0.6.29" @@ -3846,9 +3858,9 @@ checksum = "f162c6dd7b008981e4d40210aca20b4bd0f9b60ca9271061b07f78537722f2e1" [[package]] name = "regex-syntax" -version = "0.7.2" +version = "0.8.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "436b050e76ed2903236f032a59761c1eb99e1b0aead2c257922771dab1fc8c78" +checksum = "c08c74e62047bb2de4ff487b251e4a92e24f48745648451635cec7d591162d9f" [[package]] name = "relative-path" @@ -6231,7 +6243,8 @@ dependencies = [ "prost", "rand 0.8.5", "regex", - "regex-syntax 0.7.2", + "regex-automata 0.4.3", + "regex-syntax 0.8.2", "reqwest", "ring 0.16.20", "rustls", diff --git a/Cargo.toml b/Cargo.toml index ce590f3c7a..33f56e084f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -109,7 +109,7 @@ pin-project-lite = "0.2" prometheus = {version = "0.13", default_features=false, features = ["process"]} # removes protobuf dependency prost = "0.11" rand = "0.8" -regex = "1.4" +regex = "1.10.2" reqwest = { version = "0.11", default-features = false, features = ["rustls-tls"] } reqwest-tracing = { version = "0.4.0", features = ["opentelemetry_0_19"] } reqwest-middleware = "0.2.0" diff --git a/proxy/src/auth/credentials.rs b/proxy/src/auth/credentials.rs index dd7c58255f..72149e8e29 100644 --- a/proxy/src/auth/credentials.rs +++ b/proxy/src/auth/credentials.rs @@ -3,7 +3,7 @@ use crate::{ auth::password_hack::parse_endpoint_param, error::UserFacingError, - proxy::{neon_options, NUM_CONNECTION_ACCEPTED_BY_SNI}, + proxy::{neon_options_str, NUM_CONNECTION_ACCEPTED_BY_SNI}, }; use itertools::Itertools; use pq_proto::StartupMessageParams; @@ -140,7 +140,7 @@ impl ClientCredentials { let cache_key = format!( "{}{}", project.as_deref().unwrap_or(""), - neon_options(params).unwrap_or("".to_string()) + neon_options_str(params) ) .into(); @@ -406,10 +406,7 @@ mod tests { let peer_addr = IpAddr::from([127, 0, 0, 1]); let creds = ClientCredentials::parse(&options, sni, common_names, peer_addr)?; assert_eq!(creds.project.as_deref(), Some("project")); - assert_eq!( - creds.cache_key, - "projectneon_endpoint_type:read_write neon_lsn:0/2" - ); + assert_eq!(creds.cache_key, "projectendpoint_type:read_write lsn:0/2"); Ok(()) } diff --git a/proxy/src/compute.rs b/proxy/src/compute.rs index c838c8fc38..78c56300a5 100644 --- a/proxy/src/compute.rs +++ b/proxy/src/compute.rs @@ -1,6 +1,6 @@ use crate::{ auth::parse_endpoint_param, cancellation::CancelClosure, console::errors::WakeComputeError, - error::UserFacingError, proxy::is_neon_param, + error::UserFacingError, proxy::neon_option, }; use futures::{FutureExt, TryFutureExt}; use itertools::Itertools; @@ -275,7 +275,7 @@ fn filtered_options(params: &StartupMessageParams) -> Option { #[allow(unstable_name_collisions)] let options: String = params .options_raw()? - .filter(|opt| parse_endpoint_param(opt).is_none() && !is_neon_param(opt)) + .filter(|opt| parse_endpoint_param(opt).is_none() && neon_option(opt).is_none()) .intersperse(" ") // TODO: use impl from std once it's stabilized .collect(); diff --git a/proxy/src/console/provider.rs b/proxy/src/console/provider.rs index ccb5cbdb92..b0a73fd03d 100644 --- a/proxy/src/console/provider.rs +++ b/proxy/src/console/provider.rs @@ -201,7 +201,18 @@ pub struct ConsoleReqExtra<'a> { pub session_id: uuid::Uuid, /// Name of client application, if set. pub application_name: Option<&'a str>, - pub options: Option<&'a str>, + pub options: Vec<(String, String)>, +} + +impl<'a> ConsoleReqExtra<'a> { + // https://swagger.io/docs/specification/serialization/ DeepObject format + // paramName[prop1]=value1¶mName[prop2]=value2&.... + pub fn options_as_deep_object(&self) -> Vec<(String, String)> { + self.options + .iter() + .map(|(k, v)| (format!("options[{}]", k), v.to_string())) + .collect() + } } /// Auth secret which is managed by the cloud. diff --git a/proxy/src/console/provider/neon.rs b/proxy/src/console/provider/neon.rs index f0510e91ea..f8c3ee5b58 100644 --- a/proxy/src/console/provider/neon.rs +++ b/proxy/src/console/provider/neon.rs @@ -106,7 +106,7 @@ impl Api { ) -> Result { let request_id = uuid::Uuid::new_v4().to_string(); async { - let request = self + let mut request_builder = self .endpoint .get("proxy_wake_compute") .header("X-Request-ID", &request_id) @@ -115,9 +115,14 @@ impl Api { .query(&[ ("application_name", extra.application_name), ("project", Some(&creds.endpoint)), - ("options", extra.options), - ]) - .build()?; + ]); + + request_builder = if extra.options.is_empty() { + request_builder + } else { + request_builder.query(&extra.options_as_deep_object()) + }; + let request = request_builder.build()?; info!(url = request.url().as_str(), "sending http request"); let start = Instant::now(); diff --git a/proxy/src/proxy.rs b/proxy/src/proxy.rs index 4dbffa850a..018f774c7e 100644 --- a/proxy/src/proxy.rs +++ b/proxy/src/proxy.rs @@ -968,12 +968,10 @@ impl Client<'_, S> { allow_self_signed_compute, } = self; - let console_options = neon_options(params); - let extra = console::ConsoleReqExtra { session_id, // aka this connection's id application_name: params.get("application_name"), - options: console_options.as_deref(), + options: neon_options(params), }; let mut latency_timer = LatencyTimer::new(mode.protocol_label()); @@ -1033,26 +1031,29 @@ impl Client<'_, S> { } } -pub fn neon_options(params: &StartupMessageParams) -> Option { +pub fn neon_options(params: &StartupMessageParams) -> Vec<(String, String)> { #[allow(unstable_name_collisions)] - let options: String = params - .options_raw()? - .filter(|opt| is_neon_param(opt)) - .sorted() // we sort it to use as cache key - .intersperse(" ") // TODO: use impl from std once it's stabilized - .collect(); - - // Don't even bother with empty options. - if options.is_empty() { - return None; + match params.options_raw() { + Some(options) => options.filter_map(neon_option).collect(), + None => vec![], } - - Some(options) } -pub fn is_neon_param(bytes: &str) -> bool { +pub fn neon_options_str(params: &StartupMessageParams) -> String { + #[allow(unstable_name_collisions)] + neon_options(params) + .iter() + .map(|(k, v)| format!("{}:{}", k, v)) + .sorted() // we sort it to use as cache key + .intersperse(" ".to_owned()) + .collect() +} + +pub fn neon_option(bytes: &str) -> Option<(String, String)> { static RE: OnceCell = OnceCell::new(); - RE.get_or_init(|| Regex::new(r"^neon_\w+:").unwrap()); + let re = RE.get_or_init(|| Regex::new(r"^neon_(\w+):(.+)").unwrap()); - RE.get().unwrap().is_match(bytes) + let cap = re.captures(bytes)?; + let (_, [k, v]) = cap.extract(); + Some((k.to_owned(), v.to_owned())) } diff --git a/proxy/src/proxy/tests.rs b/proxy/src/proxy/tests.rs index 222661db4a..31c3ad1055 100644 --- a/proxy/src/proxy/tests.rs +++ b/proxy/src/proxy/tests.rs @@ -491,7 +491,7 @@ fn helper_create_connect_info( let extra = console::ConsoleReqExtra { session_id: uuid::Uuid::new_v4(), application_name: Some("TEST"), - options: None, + options: vec![], }; let creds = auth::BackendType::Test(mechanism); (cache, extra, creds) diff --git a/proxy/src/serverless/conn_pool.rs b/proxy/src/serverless/conn_pool.rs index b9d1a9692d..734df11368 100644 --- a/proxy/src/serverless/conn_pool.rs +++ b/proxy/src/serverless/conn_pool.rs @@ -433,7 +433,7 @@ async fn connect_to_compute( let extra = console::ConsoleReqExtra { session_id: uuid::Uuid::new_v4(), application_name: Some(APP_NAME), - options: console_options.as_deref(), + options: console_options, }; // TODO(anna): this is a bit hacky way, consider using console notification listener. if !config.disable_ip_check_for_http { diff --git a/workspace_hack/Cargo.toml b/workspace_hack/Cargo.toml index 3653643d7e..4621a75c0b 100644 --- a/workspace_hack/Cargo.toml +++ b/workspace_hack/Cargo.toml @@ -53,7 +53,8 @@ num-traits = { version = "0.2", features = ["i128"] } prost = { version = "0.11" } rand = { version = "0.8", features = ["small_rng"] } regex = { version = "1" } -regex-syntax = { version = "0.7" } +regex-automata = { version = "0.4", default-features = false, features = ["dfa-onepass", "hybrid", "meta", "nfa-backtrack", "perf-inline", "perf-literal", "unicode"] } +regex-syntax = { version = "0.8" } reqwest = { version = "0.11", default-features = false, features = ["blocking", "default-tls", "json", "multipart", "rustls-tls", "stream"] } ring = { version = "0.16", features = ["std"] } rustls = { version = "0.21", features = ["dangerous_configuration"] } @@ -90,7 +91,8 @@ memchr = { version = "2" } nom = { version = "7" } prost = { version = "0.11" } regex = { version = "1" } -regex-syntax = { version = "0.7" } +regex-automata = { version = "0.4", default-features = false, features = ["dfa-onepass", "hybrid", "meta", "nfa-backtrack", "perf-inline", "perf-literal", "unicode"] } +regex-syntax = { version = "0.8" } serde = { version = "1", features = ["alloc", "derive"] } syn-dff4ba8e3ae991db = { package = "syn", version = "1", features = ["extra-traits", "full", "visit"] } syn-f595c2ba2a3f28df = { package = "syn", version = "2", features = ["extra-traits", "full", "visit", "visit-mut"] }