Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Strengthen postgres-docker-utils #633

Merged
merged 1 commit into from
Oct 13, 2023
Merged

Strengthen postgres-docker-utils #633

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 19 additions & 40 deletions crates/postgres-docker-utils/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,21 @@
use std::net::SocketAddr;
use std::process::{Command, Stdio};
use std::time::Duration;

use anyhow::Context;

pub struct DockerContainerGuard {
container_id: String,
container_port: u16,
container_id: String,
socket_addr: SocketAddr,
}

impl DockerContainerGuard {
pub fn port(&self) -> u16 {
self.container_port
pub fn socket_addr(&self) -> SocketAddr {
self.socket_addr
}

pub fn address(&self) -> String {
self.socket_addr.to_string()
}
}

Expand Down Expand Up @@ -40,13 +45,13 @@ pub async fn setup() -> anyhow::Result<DockerContainerGuard> {

let exposed_port = run_cmd_to_output(&format!("docker container port {container_id} 5432"))
.context("Fetching container exposed port")?;
let container_port = parse_exposed_port(&exposed_port)?;
let socket_addr = parse_exposed_port(&exposed_port)?;

std::thread::sleep(Duration::from_secs_f32(2.0));

Ok(DockerContainerGuard {
container_id,
container_port,
socket_addr,
})
}

Expand Down Expand Up @@ -76,38 +81,18 @@ fn run_cmd(cmd_str: &str) -> anyhow::Result<()> {
Ok(())
}

fn parse_exposed_port(s: &str) -> anyhow::Result<u16> {
fn parse_exposed_port(s: &str) -> anyhow::Result<SocketAddr> {
let parts: Vec<_> = s
.split_whitespace()
.map(|s| s.trim())
.filter(|s| !s.is_empty())
.collect();

let ports: Vec<_> = parts.into_iter().filter_map(extract_port).collect();

let mut parsed_port = None;

for port in ports {
let port: u16 = port.parse().with_context(|| format!("Parsing `{port}`"))?;

if let Some(current) = parsed_port {
if current != port {
anyhow::bail!(
"Multiple different ports exposed: `{}` and `{}`",
current,
port
);
}
} else {
parsed_port = Some(port);
}
}

parsed_port.context("No ports parsed")
}

fn extract_port(s: &str) -> Option<&str> {
s.split(':').last()
Ok(parts
.iter()
.map(|p| p.parse::<SocketAddr>())
.next()
.context("Missing exposed port")??)
}

#[cfg(test)]
Expand All @@ -120,14 +105,8 @@ mod tests {
#[test_case(" 0.0.0.0:55837 " => 55837 ; "ignore whitespace")]
#[test_case("[::]:12345" => 12345 ; "works with ipv6")]
#[test_case("0.0.0.0:12345 \n [::]:12345" => 12345 ; "works with multiple ips")]
#[test_case("0.0.0.0:12345 \n [::]:54321" => 12345 ; "yields first of multiple ips")]
fn test_parse_exposed_port(s: &str) -> u16 {
parse_exposed_port(s).unwrap()
}

#[test]
fn different_ports_result_in_failure() {
const S: &str = "0.0.0.0:12345 [::]:54321";

let _err = parse_exposed_port(S).unwrap_err();
parse_exposed_port(s).unwrap().port()
}
}
8 changes: 4 additions & 4 deletions src/database/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -369,8 +369,8 @@
pub async fn get_latest_insertion_timestamp(&self) -> Result<Option<DateTime<Utc>>, Error> {
let query = sqlx::query(
r#"
SELECT insertion_timestamp
FROM latest_insertion_timestamp
SELECT insertion_timestamp
FROM latest_insertion_timestamp
WHERE Lock = 'X';"#,
);

Expand Down Expand Up @@ -655,10 +655,10 @@
.collect::<Vec<String>>()
.join(", ");

let query = format!(
"DELETE FROM deletions WHERE commitment IN ({})",
placeholders
);

Check warning on line 661 in src/database/mod.rs

View workflow job for this annotation

GitHub Actions / clippy

variables can be used directly in the `format!` string 

warning: variables can be used directly in the `format!` string
   --> src/database/mod.rs:658:21
    |
658 |           let query = format!(
    |  _____________________^
659 | |             "DELETE FROM deletions WHERE commitment IN ({})",
660 | |             placeholders
661 | |         );
    | |_________^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#uninlined_format_args

Check warning on line 661 in src/database/mod.rs

View workflow job for this annotation

GitHub Actions / clippy

variables can be used directly in the `format!` string 

warning: variables can be used directly in the `format!` string
   --> src/database/mod.rs:658:21
    |
658 |           let query = format!(
    |  _____________________^
659 | |             "DELETE FROM deletions WHERE commitment IN ({})",
660 | |             placeholders
661 | |         );
    | |_________^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#uninlined_format_args

Check warning on line 661 in src/database/mod.rs

View workflow job for this annotation

GitHub Actions / clippy

variables can be used directly in the `format!` string 

warning: variables can be used directly in the `format!` string
   --> src/database/mod.rs:658:21
    |
658 |           let query = format!(
    |  _____________________^
659 | |             "DELETE FROM deletions WHERE commitment IN ({})",
660 | |             placeholders
661 | |         );
    | |_________^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#uninlined_format_args

let mut query = sqlx::query(&query);

Expand Down Expand Up @@ -835,8 +835,8 @@
// TODO: either use anyhow or eyre
async fn setup_db() -> anyhow::Result<(Database, DockerContainerGuard)> {
let db_container = postgres_docker_utils::setup().await?;
let port = db_container.port();
let url = format!("postgres://postgres:postgres@localhost:{port}/database");
let db_socket_addr = db_container.address();
let url = format!("postgres://postgres:postgres@{db_socket_addr}/database");

let db = Database::new(Options {
database: SecretUrl::from_str(&url)?,
Expand Down
4 changes: 2 additions & 2 deletions tests/delete_identities.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@
let mock_insertion_prover = &insertion_prover_map[&insertion_batch_size];
let mock_deletion_prover = &deletion_prover_map[&deletion_batch_size];

let port = db_container.port();
let db_url = format!("postgres://postgres:postgres@localhost:{port}/database");
let db_socket_addr = db_container.address();
let db_url = format!("postgres://postgres:postgres@{db_socket_addr}/database");

let mut options = Options::try_parse_from([
"signup-sequencer",
Expand Down Expand Up @@ -104,7 +104,7 @@
tokio::time::sleep(Duration::from_secs(IDLE_TIME)).await;

// Check that we can also get these inclusion proofs back.
for i in 0..insertion_batch_size {

Check warning on line 107 in tests/delete_identities.rs

View workflow job for this annotation

GitHub Actions / clippy

the loop variable `i` is used to index `test_identities` 

warning: the loop variable `i` is used to index `test_identities`
   --> tests/delete_identities.rs:107:14
    |
107 |     for i in 0..insertion_batch_size {
    |              ^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_range_loop
    = note: `#[warn(clippy::needless_range_loop)]` on by default
help: consider using an iterator and enumerate()
    |
107 |     for (i, <item>) in test_identities.iter().enumerate().take(insertion_batch_size) {
    |         ~~~~~~~~~~~    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
test_inclusion_proof(
&uri,
&client,
Expand All @@ -125,7 +125,7 @@
tokio::time::sleep(Duration::from_secs(IDLE_TIME * 8)).await;

// Ensure that identities have been deleted
for i in 0..deletion_batch_size {

Check warning on line 128 in tests/delete_identities.rs

View workflow job for this annotation

GitHub Actions / clippy

the loop variable `i` is used to index `test_identities` 

warning: the loop variable `i` is used to index `test_identities`
   --> tests/delete_identities.rs:128:14
    |
128 |     for i in 0..deletion_batch_size {
    |              ^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_range_loop
help: consider using an iterator and enumerate()
    |
128 |     for (i, <item>) in test_identities.iter().enumerate().take(deletion_batch_size) {
    |         ~~~~~~~~~~~    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
test_inclusion_proof(
&uri,
&client,
Expand Down Expand Up @@ -158,7 +158,7 @@
let uri = "http://".to_owned() + &local_addr.to_string();

// Ensure that identities have been deleted
for i in 0..deletion_batch_size {

Check warning on line 161 in tests/delete_identities.rs

View workflow job for this annotation

GitHub Actions / clippy

the loop variable `i` is used to index `test_identities` 

warning: the loop variable `i` is used to index `test_identities`
   --> tests/delete_identities.rs:161:14
    |
161 |     for i in 0..deletion_batch_size {
    |              ^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_range_loop
help: consider using an iterator and enumerate()
    |
161 |     for (i, <item>) in test_identities.iter().enumerate().take(deletion_batch_size) {
    |         ~~~~~~~~~~~    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
test_inclusion_proof(
&uri,
&client,
Expand All @@ -173,7 +173,7 @@

// Ensure that valid proofs can still be received after restart for identities
// that have not been deleted
for i in deletion_batch_size + 1..insertion_batch_size {

Check warning on line 176 in tests/delete_identities.rs

View workflow job for this annotation

GitHub Actions / clippy

the loop variable `i` is used to index `test_identities` 

warning: the loop variable `i` is used to index `test_identities`
   --> tests/delete_identities.rs:176:14
    |
176 |     for i in deletion_batch_size + 1..insertion_batch_size {
    |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_range_loop
help: consider using an iterator and enumerate()
    |
176 |     for (i, <item>) in test_identities.iter().enumerate().take(insertion_batch_size).skip(deletion_batch_size + 1) {
    |         ~~~~~~~~~~~    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
test_inclusion_proof(
&uri,
&client,
Expand Down
4 changes: 2 additions & 2 deletions tests/delete_padded_identity.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@
let mock_insertion_prover = &insertion_prover_map[&insertion_batch_size];
let mock_deletion_prover = &deletion_prover_map[&deletion_batch_size];

let port = db_container.port();
let db_url = format!("postgres://postgres:postgres@localhost:{port}/database");
let db_socket_addr = db_container.address();
let db_url = format!("postgres://postgres:postgres@{db_socket_addr}/database");

let mut options = Options::try_parse_from([
"signup-sequencer",
Expand Down Expand Up @@ -104,7 +104,7 @@
tokio::time::sleep(Duration::from_secs(IDLE_TIME)).await;

// Check that we can also get these inclusion proofs back.
for i in 0..insertion_batch_size {

Check warning on line 107 in tests/delete_padded_identity.rs

View workflow job for this annotation

GitHub Actions / clippy

the loop variable `i` is used to index `test_identities` 

warning: the loop variable `i` is used to index `test_identities`
   --> tests/delete_padded_identity.rs:107:14
    |
107 |     for i in 0..insertion_batch_size {
    |              ^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_range_loop
    = note: `#[warn(clippy::needless_range_loop)]` on by default
help: consider using an iterator and enumerate()
    |
107 |     for (i, <item>) in test_identities.iter().enumerate().take(insertion_batch_size) {
    |         ~~~~~~~~~~~    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
test_inclusion_proof(
&uri,
&client,
Expand Down
4 changes: 2 additions & 2 deletions tests/dynamic_batch_sizes.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ async fn dynamic_batch_sizes() -> anyhow::Result<()> {
let first_prover = &insertion_prover_map[&first_batch_size];
let second_prover = &insertion_prover_map[&second_batch_size];

let port = db_container.port();
let db_url = format!("postgres://postgres:postgres@localhost:{port}/database");
let db_socket_addr = db_container.address();
let db_url = format!("postgres://postgres:postgres@{db_socket_addr}/database");

// We initially spawn the service with a single prover for batch size 3.

Expand Down
4 changes: 2 additions & 2 deletions tests/insert_identity_and_proofs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ async fn insert_identity_and_proofs() -> anyhow::Result<()> {

let prover_mock = &insertion_prover_map[&batch_size];

let port = db_container.port();
let db_url = format!("postgres://postgres:postgres@localhost:{port}/database");
let db_socket_addr = db_container.address();
let db_url = format!("postgres://postgres:postgres@{db_socket_addr}/database");

let mut options = Options::try_parse_from([
"signup-sequencer",
Expand Down
4 changes: 2 additions & 2 deletions tests/malformed_payload.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ async fn malformed_payload() -> anyhow::Result<()> {

let prover_mock = &insertion_prover_map[&batch_size];

let port = db_container.port();
let db_url = format!("postgres://postgres:postgres@localhost:{port}/database");
let db_socket_addr = db_container.address();
let db_url = format!("postgres://postgres:postgres@{db_socket_addr}/database");
let mut options = Options::try_parse_from([
"signup-sequencer",
"--identity-manager-address",
Expand Down
4 changes: 2 additions & 2 deletions tests/multi_prover.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ async fn multi_prover() -> anyhow::Result<()> {

info!("Running with {prover_arg_string}");

let port = db_container.port();
let db_url = format!("postgres://postgres:postgres@localhost:{port}/database");
let db_socket_addr = db_container.address();
let db_url = format!("postgres://postgres:postgres@{db_socket_addr}/database");
let mut options = Options::try_parse_from([
"signup-sequencer",
"--identity-manager-address",
Expand Down
4 changes: 2 additions & 2 deletions tests/recover_identities.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@
let mock_insertion_prover = &insertion_prover_map[&insertion_batch_size];
let mock_deletion_prover = &deletion_prover_map[&deletion_batch_size];

let port = db_container.port();
let db_url = format!("postgres://postgres:postgres@localhost:{port}/database");
let db_socket_addr = db_container.address();
let db_url = format!("postgres://postgres:postgres@{db_socket_addr}/database");

let mut options = Options::try_parse_from([
"signup-sequencer",
Expand Down Expand Up @@ -115,7 +115,7 @@

tokio::time::sleep(Duration::from_secs(IDLE_TIME)).await;
// Check that we can also get these inclusion proofs back.
for i in 0..insertion_batch_size {

Check warning on line 118 in tests/recover_identities.rs

View workflow job for this annotation

GitHub Actions / clippy

the loop variable `i` is used to index `identities_ref` 

warning: the loop variable `i` is used to index `identities_ref`
   --> tests/recover_identities.rs:118:14
    |
118 |     for i in 0..insertion_batch_size {
    |              ^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_range_loop
    = note: `#[warn(clippy::needless_range_loop)]` on by default
help: consider using an iterator and enumerate()
    |
118 |     for (i, <item>) in identities_ref.iter().enumerate().take(insertion_batch_size) {
    |         ~~~~~~~~~~~    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
test_inclusion_proof(&uri, &client, i, &ref_tree, &identities_ref[i], false).await;
}

Expand Down
4 changes: 2 additions & 2 deletions tests/unavailable_prover.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ async fn unavailable_prover() -> anyhow::Result<()> {

prover_mock.set_availability(false).await;

let port = db_container.port();
let db_url = format!("postgres://postgres:postgres@localhost:{port}/database");
let db_socket_addr = db_container.address();
let db_url = format!("postgres://postgres:postgres@{db_socket_addr}/database");
let mut options = Options::try_parse_from([
"signup-sequencer",
"--identity-manager-address",
Expand Down
4 changes: 2 additions & 2 deletions tests/unreduced_identity.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ async fn test_unreduced_identity() -> anyhow::Result<()> {
let prover_mock = &insertion_prover_map[&batch_size];
prover_mock.set_availability(false).await;

let port = db_container.port();
let db_url = format!("postgres://postgres:postgres@localhost:{port}/database");
let db_socket_addr = db_container.address();
let db_url = format!("postgres://postgres:postgres@{db_socket_addr}/database");
let mut options = Options::try_parse_from([
"signup-sequencer",
"--identity-manager-address",
Expand Down
4 changes: 2 additions & 2 deletions tests/validate_proof_with_age.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ async fn validate_proof_with_age() -> anyhow::Result<()> {

let prover_mock = &insertion_prover_map[&batch_size];

let port = db_container.port();
let db_url = format!("postgres://postgres:postgres@localhost:{port}/database");
let db_socket_addr = db_container.address();
let db_url = format!("postgres://postgres:postgres@{db_socket_addr}/database");

let mut options = Options::try_parse_from([
"signup-sequencer",
Expand Down
4 changes: 2 additions & 2 deletions tests/validate_proofs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ async fn validate_proofs() -> anyhow::Result<()> {

let identity_manager = mock_chain.identity_manager.clone();

let port = db_container.port();
let db_url = format!("postgres://postgres:postgres@localhost:{port}/database");
let db_socket_addr = db_container.address();
let db_url = format!("postgres://postgres:postgres@{db_socket_addr}/database");

let mut options = Options::try_parse_from([
"signup-sequencer",
Expand Down
Loading