diff --git a/Cargo.lock b/Cargo.lock index ef1b7327c5..e812ce7eab 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -461,7 +461,7 @@ dependencies = [ "tar", "tokio", "tokio-postgres", - "urlencoding", + "url", "workspace_hack", ] @@ -3685,12 +3685,6 @@ dependencies = [ "percent-encoding", ] -[[package]] -name = "urlencoding" -version = "2.1.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "68b90931029ab9b034b300b797048cf23723400aa757e8a2bfb9d748102f9821" - [[package]] name = "utils" version = "0.1.0" diff --git a/compute_tools/Cargo.toml b/compute_tools/Cargo.toml index a47f9998e6..1022438c2e 100644 --- a/compute_tools/Cargo.toml +++ b/compute_tools/Cargo.toml @@ -18,5 +18,5 @@ serde_json = "1" tar = "0.4" tokio = { version = "1.17", features = ["macros", "rt", "rt-multi-thread"] } tokio-postgres = { git = "https://github.com/zenithdb/rust-postgres.git", rev="d052ee8b86fff9897c77b0fe89ea9daba0e1fa38" } -urlencoding = "2.1.0" +url = "2.2.2" workspace_hack = { version = "0.1", path = "../workspace_hack" } diff --git a/compute_tools/src/bin/compute_ctl.rs b/compute_tools/src/bin/compute_ctl.rs index ba116af11b..f535adfd87 100644 --- a/compute_tools/src/bin/compute_ctl.rs +++ b/compute_tools/src/bin/compute_ctl.rs @@ -33,7 +33,7 @@ use std::process::exit; use std::sync::{Arc, RwLock}; use std::{thread, time::Duration}; -use anyhow::Result; +use anyhow::{Context, Result}; use chrono::Utc; use clap::Arg; use log::{error, info}; @@ -45,6 +45,7 @@ use compute_tools::monitor::launch_monitor; use compute_tools::params::*; use compute_tools::pg_helpers::*; use compute_tools::spec::*; +use url::Url; fn main() -> Result<()> { // TODO: re-use `utils::logging` later @@ -131,7 +132,7 @@ fn main() -> Result<()> { let compute_state = ComputeNode { start_time: Utc::now(), - connstr: connstr.to_string(), + connstr: Url::parse(connstr).context("cannot parse connstr as a URL")?, pgdata: pgdata.to_string(), pgbin: pgbin.to_string(), spec, diff --git a/compute_tools/src/checker.rs b/compute_tools/src/checker.rs index dbb70a74cf..b6ba1692f9 100644 --- a/compute_tools/src/checker.rs +++ b/compute_tools/src/checker.rs @@ -1,5 +1,3 @@ -use std::sync::Arc; - use anyhow::{anyhow, Result}; use log::error; use postgres::Client; @@ -23,9 +21,8 @@ pub fn create_writablity_check_data(client: &mut Client) -> Result<()> { Ok(()) } -pub async fn check_writability(compute: &Arc) -> Result<()> { - let connstr = &compute.connstr; - let (client, connection) = tokio_postgres::connect(connstr, NoTls).await?; +pub async fn check_writability(compute: &ComputeNode) -> Result<()> { + let (client, connection) = tokio_postgres::connect(compute.connstr.as_str(), NoTls).await?; if client.is_closed() { return Err(anyhow!("connection to postgres closed")); } diff --git a/compute_tools/src/compute.rs b/compute_tools/src/compute.rs index abf7081cb7..8bcaf5494a 100644 --- a/compute_tools/src/compute.rs +++ b/compute_tools/src/compute.rs @@ -35,7 +35,8 @@ use crate::spec::*; /// Compute node info shared across several `compute_ctl` threads. pub struct ComputeNode { pub start_time: DateTime, - pub connstr: String, + // Url type maintains proper escaping + pub connstr: url::Url, pub pgdata: String, pub pgbin: String, pub spec: ComputeSpec, @@ -268,21 +269,25 @@ impl ComputeNode { // In this case we need to connect with old `zenith_admin`name // and create new user. We cannot simply rename connected user, // but we can create a new one and grant it all privileges. - let mut client = match Client::connect(&self.connstr, NoTls) { + let mut client = match Client::connect(self.connstr.as_str(), NoTls) { Err(e) => { info!( "cannot connect to postgres: {}, retrying with `zenith_admin` username", e ); - let zenith_admin_connstr = self.connstr.replacen("cloud_admin", "zenith_admin", 1); + let mut zenith_admin_connstr = self.connstr.clone(); - let mut client = Client::connect(&zenith_admin_connstr, NoTls)?; + zenith_admin_connstr + .set_username("zenith_admin") + .map_err(|_| anyhow::anyhow!("invalid connstr"))?; + + let mut client = Client::connect(zenith_admin_connstr.as_str(), NoTls)?; client.simple_query("CREATE USER cloud_admin WITH SUPERUSER")?; client.simple_query("GRANT zenith_admin TO cloud_admin")?; drop(client); // reconnect with connsting with expected name - Client::connect(&self.connstr, NoTls)? + Client::connect(self.connstr.as_str(), NoTls)? } Ok(client) => client, }; diff --git a/compute_tools/src/monitor.rs b/compute_tools/src/monitor.rs index 041b4875bd..58cdf796bc 100644 --- a/compute_tools/src/monitor.rs +++ b/compute_tools/src/monitor.rs @@ -13,11 +13,11 @@ const MONITOR_CHECK_INTERVAL: u64 = 500; // milliseconds // Spin in a loop and figure out the last activity time in the Postgres. // Then update it in the shared state. This function never errors out. // XXX: the only expected panic is at `RwLock` unwrap(). -fn watch_compute_activity(compute: &Arc) { +fn watch_compute_activity(compute: &ComputeNode) { // Suppose that `connstr` doesn't change - let connstr = compute.connstr.clone(); + let connstr = compute.connstr.as_str(); // Define `client` outside of the loop to reuse existing connection if it's active. - let mut client = Client::connect(&connstr, NoTls); + let mut client = Client::connect(connstr, NoTls); let timeout = time::Duration::from_millis(MONITOR_CHECK_INTERVAL); info!("watching Postgres activity at {}", connstr); @@ -32,7 +32,7 @@ fn watch_compute_activity(compute: &Arc) { info!("connection to postgres closed, trying to reconnect"); // Connection is closed, reconnect and try again. - client = Client::connect(&connstr, NoTls); + client = Client::connect(connstr, NoTls); continue; } @@ -93,7 +93,7 @@ fn watch_compute_activity(compute: &Arc) { debug!("cannot connect to postgres: {}, retrying", e); // Establish a new connection and try again. - client = Client::connect(&connstr, NoTls); + client = Client::connect(connstr, NoTls); } } } diff --git a/compute_tools/src/spec.rs b/compute_tools/src/spec.rs index d2cfb6d726..041f42acde 100644 --- a/compute_tools/src/spec.rs +++ b/compute_tools/src/spec.rs @@ -4,7 +4,6 @@ use anyhow::Result; use log::{info, log_enabled, warn, Level}; use postgres::{Client, NoTls}; use serde::Deserialize; -use urlencoding::encode; use crate::compute::ComputeNode; use crate::config; @@ -231,9 +230,11 @@ pub fn handle_role_deletions(node: &ComputeNode, client: &mut Client) -> Result< fn reassign_owned_objects(node: &ComputeNode, role_name: &PgIdent) -> Result<()> { for db in &node.spec.cluster.databases { if db.owner != *role_name { - let db_name_encoded = format!("/{}", encode(&db.name)); - let db_connstr = node.connstr.replacen("/postgres", &db_name_encoded, 1); - let mut client = Client::connect(&db_connstr, NoTls)?; + let mut connstr = node.connstr.clone(); + // database name is always the last and the only component of the path + connstr.set_path(&db.name); + + let mut client = Client::connect(connstr.as_str(), NoTls)?; // This will reassign all dependent objects to the db owner let reassign_query = format!(