diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b0a7f3f9..eec844175 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Standardized log libraries and configuration - Moved GraphQL schema generation to the CI. Now it is distributed as a Github artifact +- Replace `POSTGRES_*` environment variables with `POSTGRES_ENDPOINT` ### Removed diff --git a/offchain/Cargo.lock b/offchain/Cargo.lock index c22a72032..16db3eff3 100644 --- a/offchain/Cargo.lock +++ b/offchain/Cargo.lock @@ -4174,12 +4174,12 @@ dependencies = [ "redacted", "serial_test", "snafu", + "tempfile", "test-fixtures", "test-log", "testcontainers", "tracing", "tracing-subscriber", - "urlencoding", ] [[package]] @@ -5666,12 +5666,6 @@ dependencies = [ "percent-encoding", ] -[[package]] -name = "urlencoding" -version = "2.1.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "daf8dba3b7eb870caf1ddeed7bc9d2a049f3cfdfae7cb521b087cc33ae4c49da" - [[package]] name = "users" version = "0.11.0" diff --git a/offchain/Cargo.toml b/offchain/Cargo.toml index c336fb3c5..b774e3333 100644 --- a/offchain/Cargo.toml +++ b/offchain/Cargo.toml @@ -88,7 +88,6 @@ tracing-actix-web = "0.7" tracing-subscriber = "0.3" tracing-test = "0.2" url = "2" -urlencoding = "2.1" users = "0.11" uuid = "1.4" diff --git a/offchain/data/Cargo.toml b/offchain/data/Cargo.toml index 9df0b87dd..a8b0d86b6 100644 --- a/offchain/data/Cargo.toml +++ b/offchain/data/Cargo.toml @@ -14,13 +14,13 @@ diesel_migrations.workspace = true diesel = { workspace = true, features = ["postgres", "r2d2"]} snafu.workspace = true tracing.workspace = true -urlencoding.workspace = true [dev-dependencies] test-fixtures = { path = "../test-fixtures" } serial_test.workspace = true env_logger.workspace = true +tempfile.workspace = true testcontainers.workspace = true test-log = { workspace = true, features = ["trace"] } tracing-subscriber = { workspace = true, features = ["env-filter"] } diff --git a/offchain/data/src/config.rs b/offchain/data/src/config.rs index f55e20d9e..3d1208036 100644 --- a/offchain/data/src/config.rs +++ b/offchain/data/src/config.rs @@ -3,77 +3,62 @@ use backoff::{ExponentialBackoff, ExponentialBackoffBuilder}; use clap::Parser; -pub use redacted::Redacted; +pub use redacted::{RedactedUrl, Url}; use std::time::Duration; #[derive(Debug)] pub struct RepositoryConfig { - pub user: String, - pub password: Redacted, - pub hostname: String, - pub port: u16, - pub db: String, + pub redacted_endpoint: Option, pub connection_pool_size: u32, pub backoff: ExponentialBackoff, } impl RepositoryConfig { - pub fn endpoint(&self) -> Redacted { - Redacted::new(format!( - "postgres://{}:{}@{}:{}/{}", - urlencoding::encode(&self.user), - urlencoding::encode(self.password.inner()), - urlencoding::encode(&self.hostname), - self.port, - urlencoding::encode(&self.db) - )) + /// Get the string with the endpoint if it is set, otherwise return an empty string + pub fn endpoint(&self) -> String { + match &self.redacted_endpoint { + None => String::from(""), + Some(endpoint) => endpoint.inner().to_string(), + } } } #[derive(Debug, Parser)] pub struct RepositoryCLIConfig { - #[arg(long, env, default_value = "postgres")] - postgres_user: String, - - #[arg(long, env)] - postgres_password: Option, - + /// Postgres endpoint in the format 'postgres://user:password@hostname:port/database'. + /// + /// If not set, or set to empty string, will defer the behaviour to the Pg driver. + /// See: https://www.postgresql.org/docs/current/libpq-envars.html + /// + /// It is also possible set the endpoint without the password and load it from Postgres' + /// passfile. + /// See: https://www.postgresql.org/docs/current/libpq-connect.html#LIBPQ-CONNECT-PASSFILE #[arg(long, env)] - postgres_password_file: Option, - - #[arg(long, env, default_value = "127.0.0.1")] - postgres_hostname: String, - - #[arg(long, env, default_value_t = 5432)] - postgres_port: u16, - - #[arg(long, env, default_value = "postgres")] - postgres_db: String, + postgres_endpoint: Option, + /// Number of connections to to the database #[arg(long, env, default_value_t = 3)] postgres_connection_pool_size: u32, + /// Max elapsed time for timeout #[arg(long, env, default_value = "120000")] postgres_backoff_max_elapsed_duration: u64, } impl From for RepositoryConfig { fn from(cli_config: RepositoryCLIConfig) -> RepositoryConfig { - let password = if let Some(filename) = cli_config.postgres_password_file - { - if cli_config.postgres_password.is_some() { - panic!("Both `postgres_password` and `postgres_password_file` arguments are set"); - } - match std::fs::read_to_string(filename) { - Ok(password) => password, - Err(e) => { - panic!("Failed to read password from file: {:?}", e); + let redacted_endpoint = match cli_config.postgres_endpoint { + None => None, + Some(endpoint) => { + if endpoint == "" { + None + } else { + Some(RedactedUrl::new( + Url::parse(endpoint.as_str()) + .expect("failed to parse Postgres URL"), + )) } } - } else { - cli_config - .postgres_password - .expect("Database Postgres password was not provided") }; let connection_pool_size = cli_config.postgres_connection_pool_size; let backoff_max_elapsed_duration = Duration::from_millis( @@ -83,11 +68,7 @@ impl From for RepositoryConfig { .with_max_elapsed_time(Some(backoff_max_elapsed_duration)) .build(); RepositoryConfig { - user: cli_config.postgres_user, - password: Redacted::new(password), - hostname: cli_config.postgres_hostname, - port: cli_config.postgres_port, - db: cli_config.postgres_db, + redacted_endpoint, connection_pool_size, backoff, } diff --git a/offchain/data/src/lib.rs b/offchain/data/src/lib.rs index 8d22f5d30..548414c1f 100644 --- a/offchain/data/src/lib.rs +++ b/offchain/data/src/lib.rs @@ -9,7 +9,7 @@ mod repository; mod schema; mod types; -pub use config::{Redacted, RepositoryCLIConfig, RepositoryConfig}; +pub use config::{RedactedUrl, RepositoryCLIConfig, RepositoryConfig, Url}; pub use error::Error; pub use migrations::{run_migrations, MigrationError}; pub use pagination::{Connection, Cursor, Edge, PageInfo}; diff --git a/offchain/data/src/repository.rs b/offchain/data/src/repository.rs index f51583bbf..f65b67b58 100644 --- a/offchain/data/src/repository.rs +++ b/offchain/data/src/repository.rs @@ -34,7 +34,7 @@ impl Repository { Pool::builder() .max_size(POOL_CONNECTION_SIZE) .build(ConnectionManager::::new( - config.endpoint().into_inner(), + config.endpoint(), )) .map_err(backoff::Error::transient) }) diff --git a/offchain/data/tests/repository.rs b/offchain/data/tests/repository.rs index e7b7322fc..24334786f 100644 --- a/offchain/data/tests/repository.rs +++ b/offchain/data/tests/repository.rs @@ -6,13 +6,15 @@ use diesel::pg::Pg; use diesel::{ sql_query, Connection, PgConnection, QueryableByName, RunQueryDsl, }; -use redacted::Redacted; use rollups_data::Connection as PaginationConnection; use rollups_data::{ CompletionStatus, Cursor, Edge, Error, Input, InputQueryFilter, Notice, - PageInfo, Proof, Report, Repository, RepositoryConfig, Voucher, + PageInfo, Proof, RedactedUrl, Report, Repository, RepositoryConfig, Url, + Voucher, }; use serial_test::serial; +use std::io::Write; +use std::os::unix::fs::PermissionsExt; use std::time::{Duration, UNIX_EPOCH}; use test_fixtures::DataFixture; use testcontainers::clients::Cli; @@ -36,12 +38,19 @@ impl TestState<'_> { ))) .build(); + let redacted_endpoint = Some(RedactedUrl::new( + Url::parse(&format!( + "postgres://{}:{}@{}:{}/{}", + self.data.user, + self.data.password, + self.data.hostname, + self.data.port, + self.data.db, + )) + .expect("failed to generate Postgres endpoint"), + )); Repository::new(RepositoryConfig { - user: self.data.user.clone(), - password: Redacted::new(self.data.password.clone()), - hostname: self.data.hostname.clone(), - port: self.data.port.clone(), - db: self.data.db.clone(), + redacted_endpoint, connection_pool_size: 3, backoff, }) @@ -89,7 +98,7 @@ pub fn create_input() -> Input { } } -#[test] +#[test_log::test] #[serial] fn test_create_repository() { let docker = Cli::default(); @@ -100,7 +109,7 @@ fn test_create_repository() { test.get_repository(); } -#[test] +#[test_log::test] #[serial] fn test_fail_to_create_repository() { let docker = Cli::default(); @@ -110,12 +119,19 @@ fn test_fail_to_create_repository() { .with_max_elapsed_time(Some(Duration::from_millis(2000))) .build(); + let redacted_endpoint = Some(RedactedUrl::new( + Url::parse(&format!( + "postgres://{}:{}@{}:{}/{}", + "Err", + test.data.password, + test.data.hostname, + test.data.port, + test.data.db, + )) + .expect("failed to generate Postgres endpoint"), + )); let err = Repository::new(RepositoryConfig { - user: "Err".to_string(), - password: Redacted::new(test.data.password.clone()), - hostname: test.data.hostname.clone(), - port: test.data.port.clone(), - db: test.data.db.clone(), + redacted_endpoint, connection_pool_size: 3, backoff, }) @@ -124,7 +140,61 @@ fn test_fail_to_create_repository() { assert!(matches!(err, Error::DatabaseConnectionError { source: _ })); } -#[test] +#[test_log::test] +#[serial] +fn test_postgres_file_configuration() { + let docker = Cli::default(); + let test = TestState::setup(&docker); + + // Create Postgres pgfile + let tempdir = tempfile::tempdir().expect("failed to create tempdir"); + let pgpass_path = + tempdir.path().join("pgpass").to_string_lossy().to_string(); + tracing::info!("Storing pgfile to {}", pgpass_path); + { + let mut pgpass = std::fs::File::create(&pgpass_path) + .expect("failed to create pgpass"); + // Set permission to 600 + let metadata = + pgpass.metadata().expect("failed to get pgpass metadata"); + let mut permissions = metadata.permissions(); + permissions.set_mode(0o600); + pgpass + .set_permissions(permissions) + .expect("failed to set pgpass permissions"); + // Write pgfile contents + write!( + &mut pgpass, + "{}:{}:{}:{}:{}\n", + test.data.hostname, + test.data.port, + test.data.db, + test.data.user, + test.data.password + ) + .expect("failed to write pgpass"); + } + + // Set Postgres environment variables + std::env::set_var("PGPASSFILE", pgpass_path); + std::env::set_var("PGHOST", test.data.hostname); + std::env::set_var("PGPORT", test.data.port.to_string()); + std::env::set_var("PGDATABASE", test.data.db); + std::env::set_var("PGUSER", test.data.user); + + // Connect to postgres using pgpass file + let backoff = ExponentialBackoffBuilder::new() + .with_max_elapsed_time(Some(Duration::from_millis(100))) + .build(); + Repository::new(RepositoryConfig { + redacted_endpoint: None, + connection_pool_size: 3, + backoff, + }) + .expect("failed to create repository"); +} + +#[test_log::test] #[serial] fn test_insert_input() { let docker = Cli::default(); @@ -141,7 +211,7 @@ fn test_insert_input() { assert_eq!(result, input); } -#[test] +#[test_log::test] #[serial] fn test_get_input() { let docker = Cli::default(); @@ -158,7 +228,7 @@ fn test_get_input() { assert_eq!(input, get_input); } -#[test] +#[test_log::test] #[serial] fn test_get_input_error() { let docker = Cli::default(); @@ -178,7 +248,7 @@ fn test_get_input_error() { )); } -#[test] +#[test_log::test] #[serial] fn test_update_input_status() { let docker = Cli::default(); @@ -198,7 +268,7 @@ fn test_update_input_status() { assert_eq!(get_input.status, CompletionStatus::Accepted); } -#[test] +#[test_log::test] #[serial] fn test_insert_notice() { let docker = Cli::default(); @@ -221,7 +291,7 @@ fn test_insert_notice() { assert_eq!(result, notice); } -#[test] +#[test_log::test] #[serial] fn test_get_notice() { let docker = Cli::default(); @@ -246,7 +316,7 @@ fn test_get_notice() { assert_eq!(notice, get_notice); } -#[test] +#[test_log::test] #[serial] fn test_insert_notice_error() { let docker = Cli::default(); @@ -267,7 +337,7 @@ fn test_insert_notice_error() { assert!(matches!(notice_error, Error::DatabaseError { source: _ })); } -#[test] +#[test_log::test] #[serial] fn test_get_notice_error() { let docker = Cli::default(); @@ -293,7 +363,7 @@ fn test_get_notice_error() { )); } -#[test] +#[test_log::test] #[serial] fn test_insert_voucher() { let docker = Cli::default(); @@ -317,7 +387,7 @@ fn test_insert_voucher() { assert_eq!(result, voucher); } -#[test] +#[test_log::test] #[serial] fn test_get_voucher() { let docker = Cli::default(); @@ -341,7 +411,7 @@ fn test_get_voucher() { assert_eq!(voucher, get_voucher); } -#[test] +#[test_log::test] #[serial] fn test_insert_voucher_error() { let docker = Cli::default(); @@ -363,7 +433,7 @@ fn test_insert_voucher_error() { assert!(matches!(voucher_error, Error::DatabaseError { source: _ })); } -#[test] +#[test_log::test] #[serial] fn test_get_voucher_error() { let docker = Cli::default(); @@ -390,7 +460,7 @@ fn test_get_voucher_error() { )); } -#[test] +#[test_log::test] #[serial] fn test_insert_report() { let docker = Cli::default(); @@ -413,7 +483,7 @@ fn test_insert_report() { assert_eq!(result, report); } -#[test] +#[test_log::test] #[serial] fn test_get_report() { let docker = Cli::default(); @@ -435,7 +505,7 @@ fn test_get_report() { assert_eq!(report, get_report); } -#[test] +#[test_log::test] #[serial] fn test_insert_report_error() { let docker = Cli::default(); @@ -456,7 +526,7 @@ fn test_insert_report_error() { assert!(matches!(report_error, Error::DatabaseError { source: _ })); } -#[test] +#[test_log::test] #[serial] fn test_get_report_error() { let docker = Cli::default(); @@ -482,7 +552,7 @@ fn test_get_report_error() { )); } -#[test] +#[test_log::test] #[serial] fn test_insert_proof() { let docker = Cli::default(); @@ -516,7 +586,7 @@ fn test_insert_proof() { assert_eq!(result, proof); } -#[test] +#[test_log::test] #[serial] fn test_get_proof() { let docker = Cli::default(); @@ -552,7 +622,7 @@ fn test_get_proof() { assert_eq!(proof, get_proof); } -#[test] +#[test_log::test] #[serial] fn test_get_proof_error() { let docker = Cli::default(); @@ -589,7 +659,7 @@ fn test_get_proof_error() { } } -#[test] +#[test_log::test] #[serial] fn test_pagination_macro() { let docker = Cli::default(); diff --git a/offchain/indexer/src/indexer.rs b/offchain/indexer/src/indexer.rs index fb8a8b7aa..47dc38ed7 100644 --- a/offchain/indexer/src/indexer.rs +++ b/offchain/indexer/src/indexer.rs @@ -25,8 +25,7 @@ impl Indexer { pub async fn start(config: IndexerConfig) -> Result<(), IndexerError> { tracing::info!("running database migrations"); let endpoint = config.repository_config.endpoint(); - rollups_data::run_migrations(&endpoint.into_inner()) - .context(MigrationsSnafu)?; + rollups_data::run_migrations(&endpoint).context(MigrationsSnafu)?; tracing::info!("runned migrations; connecting to DB"); let repository = tokio::task::spawn_blocking(|| { diff --git a/offchain/test-fixtures/src/repository.rs b/offchain/test-fixtures/src/repository.rs index 14b470727..db6bb87ff 100644 --- a/offchain/test-fixtures/src/repository.rs +++ b/offchain/test-fixtures/src/repository.rs @@ -2,7 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 (see LICENSE) use backoff::ExponentialBackoffBuilder; -use rollups_data::{Redacted, Repository, RepositoryConfig}; +use rollups_data::{RedactedUrl, Repository, RepositoryConfig, Url}; use std::time::Duration; use testcontainers::clients::Cli; @@ -68,12 +68,19 @@ impl RepositoryFixture<'_> { fn create_repository_config(postgres_port: u16) -> RepositoryConfig { use crate::data::*; + let redacted_endpoint = Some(RedactedUrl::new( + Url::parse(&format!( + "postgres://{}:{}@{}:{}/{}", + POSTGRES_USER, + POSTGRES_PASSWORD, + POSTGRES_HOST, + postgres_port, + POSTGRES_DB, + )) + .expect("failed to generate Postgres endpoint"), + )); RepositoryConfig { - user: POSTGRES_USER.to_owned(), - password: Redacted::new(POSTGRES_PASSWORD.to_owned()), - hostname: POSTGRES_HOST.to_owned(), - port: postgres_port, - db: POSTGRES_DB.to_owned(), + redacted_endpoint, connection_pool_size: 1, backoff: Default::default(), }