Skip to content

Commit

Permalink
feat(database)!: simplify postgres config
Browse files Browse the repository at this point in the history
Receive a single environment variable to setup the postgres connection.
It is now possible to omit the password and let the Pg drive load it
from its own passfile.
  • Loading branch information
gligneul committed Sep 27, 2023
1 parent f87562d commit 979b514
Show file tree
Hide file tree
Showing 10 changed files with 132 additions and 80 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
8 changes: 1 addition & 7 deletions offchain/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion offchain/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down
2 changes: 1 addition & 1 deletion offchain/data/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"] }
79 changes: 30 additions & 49 deletions offchain/data/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<String>,
pub hostname: String,
pub port: u16,
pub db: String,
pub redacted_endpoint: Option<RedactedUrl>,
pub connection_pool_size: u32,
pub backoff: ExponentialBackoff,
}

impl RepositoryConfig {
pub fn endpoint(&self) -> Redacted<String> {
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<String>,

/// 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 to set the endpoint without a 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<String>,

#[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<String>,

/// Number of connections 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<RepositoryCLIConfig> 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(
Expand All @@ -83,11 +68,7 @@ impl From<RepositoryCLIConfig> 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,
}
Expand Down
2 changes: 1 addition & 1 deletion offchain/data/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down
2 changes: 1 addition & 1 deletion offchain/data/src/repository.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ impl Repository {
Pool::builder()
.max_size(POOL_CONNECTION_SIZE)
.build(ConnectionManager::<PgConnection>::new(
config.endpoint().into_inner(),
config.endpoint(),
))
.map_err(backoff::Error::transient)
})
Expand Down
95 changes: 83 additions & 12 deletions offchain/data/tests/repository.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,18 @@ 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 test_log::test;
use testcontainers::clients::Cli;

const BACKOFF_DURATION: u64 = 120000;
Expand All @@ -36,12 +39,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,
})
Expand Down Expand Up @@ -110,12 +120,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,
})
Expand All @@ -124,6 +141,60 @@ fn test_fail_to_create_repository() {
assert!(matches!(err, Error::DatabaseConnectionError { source: _ }));
}

#[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]
#[serial]
fn test_insert_input() {
Expand Down
3 changes: 1 addition & 2 deletions offchain/indexer/src/indexer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(|| {
Expand Down
19 changes: 13 additions & 6 deletions offchain/test-fixtures/src/repository.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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(),
}
Expand Down

0 comments on commit 979b514

Please sign in to comment.