From 5efe95a008bb6a19ec9676a0c7b1a5516f85e4c1 Mon Sep 17 00:00:00 2001 From: Anna Khanova <32508607+khanova@users.noreply.github.com> Date: Wed, 10 Apr 2024 10:30:09 +0200 Subject: [PATCH] proxy: fix credentials cache lookup (#7349) ## Problem Incorrect processing of `-pooler` connections. ## Summary of changes Fix TODO: add e2e tests for caching --- proxy/src/cache/endpoints.rs | 5 ++--- proxy/src/console/provider/neon.rs | 32 ++++++++++++++++++------------ 2 files changed, 21 insertions(+), 16 deletions(-) diff --git a/proxy/src/cache/endpoints.rs b/proxy/src/cache/endpoints.rs index 9bc019c2d8..31e3ef6891 100644 --- a/proxy/src/cache/endpoints.rs +++ b/proxy/src/cache/endpoints.rs @@ -21,7 +21,7 @@ use crate::{ metrics::REDIS_BROKEN_MESSAGES, rate_limiter::GlobalRateLimiter, redis::connection_with_credentials_provider::ConnectionWithCredentialsProvider, - EndpointId, Normalize, + EndpointId, }; #[derive(Deserialize, Debug, Clone)] @@ -72,9 +72,8 @@ impl EndpointsCache { !rejected } fn should_reject(&self, endpoint: &EndpointId) -> bool { - let endpoint = endpoint.normalize(); if endpoint.is_endpoint() { - !self.endpoints.contains(&EndpointIdInt::from(&endpoint)) + !self.endpoints.contains(&EndpointIdInt::from(endpoint)) } else if endpoint.is_branch() { !self .branches diff --git a/proxy/src/console/provider/neon.rs b/proxy/src/console/provider/neon.rs index 3a0e5609d8..68b91447f9 100644 --- a/proxy/src/console/provider/neon.rs +++ b/proxy/src/console/provider/neon.rs @@ -59,7 +59,7 @@ impl Api { if !self .caches .endpoints_cache - .is_valid(ctx, &user_info.endpoint) + .is_valid(ctx, &user_info.endpoint.normalize()) .await { info!("endpoint is not valid, skipping the request"); @@ -186,23 +186,27 @@ impl super::Api for Api { ctx: &mut RequestMonitoring, user_info: &ComputeUserInfo, ) -> Result { - let ep = &user_info.endpoint; + let normalized_ep = &user_info.endpoint.normalize(); let user = &user_info.user; - if let Some(role_secret) = self.caches.project_info.get_role_secret(ep, user) { + if let Some(role_secret) = self + .caches + .project_info + .get_role_secret(normalized_ep, user) + { return Ok(role_secret); } let auth_info = self.do_get_auth_info(ctx, user_info).await?; if let Some(project_id) = auth_info.project_id { - let ep_int = ep.normalize().into(); + let normalized_ep_int = normalized_ep.into(); self.caches.project_info.insert_role_secret( project_id, - ep_int, + normalized_ep_int, user.into(), auth_info.secret.clone(), ); self.caches.project_info.insert_allowed_ips( project_id, - ep_int, + normalized_ep_int, Arc::new(auth_info.allowed_ips), ); ctx.set_project_id(project_id); @@ -216,8 +220,8 @@ impl super::Api for Api { ctx: &mut RequestMonitoring, user_info: &ComputeUserInfo, ) -> Result<(CachedAllowedIps, Option), GetAuthInfoError> { - let ep = &user_info.endpoint; - if let Some(allowed_ips) = self.caches.project_info.get_allowed_ips(ep) { + let normalized_ep = &user_info.endpoint.normalize(); + if let Some(allowed_ips) = self.caches.project_info.get_allowed_ips(normalized_ep) { ALLOWED_IPS_BY_CACHE_OUTCOME .with_label_values(&["hit"]) .inc(); @@ -230,16 +234,18 @@ impl super::Api for Api { let allowed_ips = Arc::new(auth_info.allowed_ips); let user = &user_info.user; if let Some(project_id) = auth_info.project_id { - let ep_int = ep.normalize().into(); + let normalized_ep_int = normalized_ep.into(); self.caches.project_info.insert_role_secret( project_id, - ep_int, + normalized_ep_int, user.into(), auth_info.secret.clone(), ); - self.caches - .project_info - .insert_allowed_ips(project_id, ep_int, allowed_ips.clone()); + self.caches.project_info.insert_allowed_ips( + project_id, + normalized_ep_int, + allowed_ips.clone(), + ); ctx.set_project_id(project_id); } Ok((