From 37b631e89e2b13d42e0ab39ce3b09c98e8684088 Mon Sep 17 00:00:00 2001 From: Alex Chi Z Date: Wed, 28 Feb 2024 11:36:12 -0500 Subject: [PATCH 1/3] compute_ctl: only try zenith_admin if could not auth Signed-off-by: Alex Chi Z --- compute_tools/src/compute.rs | 48 ++++++++++++++++++++++-------------- 1 file changed, 29 insertions(+), 19 deletions(-) diff --git a/compute_tools/src/compute.rs b/compute_tools/src/compute.rs index 142bb14fe588..768aaedf8ed0 100644 --- a/compute_tools/src/compute.rs +++ b/compute_tools/src/compute.rs @@ -17,6 +17,7 @@ use chrono::{DateTime, Utc}; use futures::future::join_all; use futures::stream::FuturesUnordered; use futures::StreamExt; +use postgres::error::SqlState; use postgres::{Client, NoTls}; use tokio; use tokio_postgres; @@ -777,25 +778,34 @@ impl ComputeNode { let connstr = self.connstr.clone(); let mut client = match Client::connect(connstr.as_str(), NoTls) { Err(e) => { - info!( - "cannot connect to postgres: {}, retrying with `zenith_admin` username", - e - ); - let mut zenith_admin_connstr = connstr.clone(); - - zenith_admin_connstr - .set_username("zenith_admin") - .map_err(|_| anyhow::anyhow!("invalid connstr"))?; - - let mut client = Client::connect(zenith_admin_connstr.as_str(), NoTls)?; - // Disable forwarding so that users don't get a cloud_admin role - client.simple_query("SET neon.forward_ddl = false")?; - client.simple_query("CREATE USER cloud_admin WITH SUPERUSER")?; - client.simple_query("GRANT zenith_admin TO cloud_admin")?; - drop(client); - - // reconnect with connstring with expected name - Client::connect(connstr.as_str(), NoTls)? + if let Some(code) = e.code() { + if code == &SqlState::INVALID_PASSWORD { + // connect with zenith_admin if cloud_admin could not authenticate + info!( + "cannot connect to postgres: {}, retrying with `zenith_admin` username", + e + ); + let mut zenith_admin_connstr = connstr.clone(); + + zenith_admin_connstr + .set_username("zenith_admin") + .map_err(|_| anyhow::anyhow!("invalid connstr"))?; + + let mut client = Client::connect(zenith_admin_connstr.as_str(), NoTls)?; + // Disable forwarding so that users don't get a cloud_admin role + client.simple_query("SET neon.forward_ddl = false")?; + client.simple_query("CREATE USER cloud_admin WITH SUPERUSER")?; + client.simple_query("GRANT zenith_admin TO cloud_admin")?; + drop(client); + + // reconnect with connstring with expected name + Client::connect(connstr.as_str(), NoTls)? + } else { + return Err(e.into()); + } + } else { + return Err(e.into()); + } } Ok(client) => client, }; From 25f5d4a28c2c6e0b7d54770bbb5c1b70a32d8d98 Mon Sep 17 00:00:00 2001 From: Alex Chi Z Date: Fri, 1 Mar 2024 14:37:01 -0500 Subject: [PATCH 2/3] apply suggestion + add new errtyp Signed-off-by: Alex Chi Z --- compute_tools/src/compute.rs | 54 +++++++++++++++++------------------- 1 file changed, 25 insertions(+), 29 deletions(-) diff --git a/compute_tools/src/compute.rs b/compute_tools/src/compute.rs index 768aaedf8ed0..6bf079bbed2e 100644 --- a/compute_tools/src/compute.rs +++ b/compute_tools/src/compute.rs @@ -777,36 +777,32 @@ impl ComputeNode { // but we can create a new one and grant it all privileges. let connstr = self.connstr.clone(); let mut client = match Client::connect(connstr.as_str(), NoTls) { - Err(e) => { - if let Some(code) = e.code() { - if code == &SqlState::INVALID_PASSWORD { - // connect with zenith_admin if cloud_admin could not authenticate - info!( - "cannot connect to postgres: {}, retrying with `zenith_admin` username", - e - ); - let mut zenith_admin_connstr = connstr.clone(); - - zenith_admin_connstr - .set_username("zenith_admin") - .map_err(|_| anyhow::anyhow!("invalid connstr"))?; - - let mut client = Client::connect(zenith_admin_connstr.as_str(), NoTls)?; - // Disable forwarding so that users don't get a cloud_admin role - client.simple_query("SET neon.forward_ddl = false")?; - client.simple_query("CREATE USER cloud_admin WITH SUPERUSER")?; - client.simple_query("GRANT zenith_admin TO cloud_admin")?; - drop(client); - - // reconnect with connstring with expected name - Client::connect(connstr.as_str(), NoTls)? - } else { - return Err(e.into()); - } - } else { - return Err(e.into()); + Err(e) => match e.code() { + Some(&SqlState::INVALID_PASSWORD) + | Some(&SqlState::INVALID_AUTHORIZATION_SPECIFICATION) => { + // connect with zenith_admin if cloud_admin could not authenticate + info!( + "cannot connect to postgres: {}, retrying with `zenith_admin` username", + e + ); + let mut zenith_admin_connstr = connstr.clone(); + + zenith_admin_connstr + .set_username("zenith_admin") + .map_err(|_| anyhow::anyhow!("invalid connstr"))?; + + let mut client = Client::connect(zenith_admin_connstr.as_str(), NoTls)?; + // Disable forwarding so that users don't get a cloud_admin role + client.simple_query("SET neon.forward_ddl = false")?; + client.simple_query("CREATE USER cloud_admin WITH SUPERUSER")?; + client.simple_query("GRANT zenith_admin TO cloud_admin")?; + drop(client); + + // reconnect with connstring with expected name + Client::connect(connstr.as_str(), NoTls)? } - } + _ => return Err(e.into()), + }, Ok(client) => client, }; From 0b202f179d7c301f546d82586123a47907934b7b Mon Sep 17 00:00:00 2001 From: Alex Chi Z Date: Fri, 1 Mar 2024 14:45:06 -0500 Subject: [PATCH 3/3] more context for error Signed-off-by: Alex Chi Z --- compute_tools/src/compute.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/compute_tools/src/compute.rs b/compute_tools/src/compute.rs index 6bf079bbed2e..2ae103a07cb3 100644 --- a/compute_tools/src/compute.rs +++ b/compute_tools/src/compute.rs @@ -791,7 +791,9 @@ impl ComputeNode { .set_username("zenith_admin") .map_err(|_| anyhow::anyhow!("invalid connstr"))?; - let mut client = Client::connect(zenith_admin_connstr.as_str(), NoTls)?; + let mut client = + Client::connect(zenith_admin_connstr.as_str(), NoTls) + .context("broken cloud_admin credential: tried connecting with cloud_admin but could not authenticate, and zenith_admin does not work either")?; // Disable forwarding so that users don't get a cloud_admin role client.simple_query("SET neon.forward_ddl = false")?; client.simple_query("CREATE USER cloud_admin WITH SUPERUSER")?;