From 8ac8ef5f2314882019e60e203db99b56a9487e38 Mon Sep 17 00:00:00 2001 From: Frank Elsinga Date: Sun, 20 Oct 2024 08:07:54 +0200 Subject: [PATCH] feat:added a postgres version check (#1427) - [x] adds a postgres version check - [x] added a testcase (using testcontainers) that the version is parsed correctly Resolves #1230 (I think at this point the PR can be considered unasigned again, feel free to reject otherwise. Don't want to step on peoples toes ^^) Resolves #1229 (by using the minumum supported postgres version for postgis) --------- Co-authored-by: Yuri Astrakhan --- Cargo.lock | 275 ++++++++++++++++++++++++++++++++-- Cargo.toml | 3 +- martin/Cargo.toml | 2 + martin/src/pg/errors.rs | 6 + martin/src/pg/pool.rs | 145 ++++++++++++++---- martin/src/pg/query_tables.rs | 5 +- tests/test.sh | 1 + 7 files changed, 395 insertions(+), 42 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c669dc527..4c796d477 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -580,6 +580,56 @@ dependencies = [ "generic-array 0.14.7", ] +[[package]] +name = "bollard" +version = "0.17.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d41711ad46fda47cd701f6908e59d1bd6b9a2b7464c0d0aeab95c6d37096ff8a" +dependencies = [ + "base64 0.22.1", + "bollard-stubs", + "bytes", + "futures-core", + "futures-util", + "hex", + "home", + "http 1.1.0", + "http-body-util", + "hyper 1.5.0", + "hyper-named-pipe", + "hyper-rustls", + "hyper-util", + "hyperlocal", + "log", + "pin-project-lite", + "rustls", + "rustls-native-certs 0.7.3", + "rustls-pemfile", + "rustls-pki-types", + "serde", + "serde_derive", + "serde_json", + "serde_repr", + "serde_urlencoded", + "thiserror", + "tokio", + "tokio-util", + "tower-service", + "url", + "winapi", +] + +[[package]] +name = "bollard-stubs" +version = "1.45.0-rc.26.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6d7c5415e3a6bc6d3e99eff6268e488fd4ee25e7b28c10f08fa6760bd9de16e4" +dependencies = [ + "serde", + "serde_repr", + "serde_with", +] + [[package]] name = "brotli" version = "3.5.0" @@ -602,6 +652,17 @@ dependencies = [ "brotli-decompressor 4.0.1", ] +[[package]] +name = "brotli" +version = "7.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cc97b8f16f944bba54f0433f07e30be199b6dc2bd25937444bbad560bcea29bd" +dependencies = [ + "alloc-no-stdlib", + "alloc-stdlib", + "brotli-decompressor 4.0.1", +] + [[package]] name = "brotli-decompressor" version = "2.5.1" @@ -1204,6 +1265,17 @@ dependencies = [ "subtle", ] +[[package]] +name = "docker_credential" +version = "1.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "31951f49556e34d90ed28342e1df7e1cb7a229c4cab0aecc627b5d91edd41d07" +dependencies = [ + "base64 0.21.7", + "serde", + "serde_json", +] + [[package]] name = "dotenvy" version = "0.15.7" @@ -1452,7 +1524,7 @@ dependencies = [ "enum_dispatch", "fs4", "memmapix", - "parse-display", + "parse-display 0.8.2", "pin-project-lite", "tokio", ] @@ -1936,6 +2008,7 @@ dependencies = [ "http 1.1.0", "http-body 1.0.1", "httparse", + "httpdate", "itoa", "pin-project-lite", "smallvec", @@ -1943,6 +2016,21 @@ dependencies = [ "want", ] +[[package]] +name = "hyper-named-pipe" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "73b7d8abf35697b81a825e386fc151e0d503e8cb5fcb93cc8669c376dfd6f278" +dependencies = [ + "hex", + "hyper 1.5.0", + "hyper-util", + "pin-project-lite", + "tokio", + "tower-service", + "winapi", +] + [[package]] name = "hyper-rustls" version = "0.27.3" @@ -1954,7 +2042,7 @@ dependencies = [ "hyper 1.5.0", "hyper-util", "rustls", - "rustls-native-certs", + "rustls-native-certs 0.8.0", "rustls-pki-types", "tokio", "tokio-rustls", @@ -1980,6 +2068,21 @@ dependencies = [ "tracing", ] +[[package]] +name = "hyperlocal" +version = "0.9.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "986c5ce3b994526b3cd75578e62554abd09f0899d6206de48b3e96ab34ccc8c7" +dependencies = [ + "hex", + "http-body-util", + "hyper 1.5.0", + "hyper-util", + "pin-project-lite", + "tokio", + "tower-service", +] + [[package]] name = "iana-time-zone" version = "0.1.61" @@ -2321,7 +2424,7 @@ checksum = "c0ff37bd590ca25063e35af745c343cb7a0271906fb7b37e4813e8f79f00268d" dependencies = [ "bitflags 2.6.0", "libc", - "redox_syscall", + "redox_syscall 0.5.7", ] [[package]] @@ -2395,9 +2498,10 @@ dependencies = [ "actix-rt", "actix-web", "actix-web-static-files", + "anyhow", "async-trait", "bit-set", - "brotli 6.0.0", + "brotli 7.0.0", "cargo-husky", "clap", "criterion", @@ -2425,7 +2529,7 @@ dependencies = [ "regex", "rstest", "rustls", - "rustls-native-certs", + "rustls-native-certs 0.8.0", "rustls-pemfile", "semver", "serde", @@ -2435,6 +2539,7 @@ dependencies = [ "spreet", "static-files", "subst", + "testcontainers-modules", "thiserror", "tilejson", "tokio", @@ -2447,7 +2552,7 @@ name = "martin-tile-utils" version = "0.5.1" dependencies = [ "approx", - "brotli 6.0.0", + "brotli 7.0.0", "flate2", "insta", ] @@ -2828,7 +2933,7 @@ checksum = "1e401f977ab385c9e4e3ab30627d6f26d00e2c73eef317493c4ec6d468726cf8" dependencies = [ "cfg-if", "libc", - "redox_syscall", + "redox_syscall 0.5.7", "smallvec", "windows-targets 0.52.6", ] @@ -2840,10 +2945,21 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c6509d08722b53e8dafe97f2027b22ccbe3a5db83cb352931e9716b0aa44bc5c" dependencies = [ "once_cell", - "parse-display-derive", + "parse-display-derive 0.8.2", "regex", ] +[[package]] +name = "parse-display" +version = "0.9.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "914a1c2265c98e2446911282c6ac86d8524f495792c38c5bd884f80499c7538a" +dependencies = [ + "parse-display-derive 0.9.1", + "regex", + "regex-syntax 0.8.5", +] + [[package]] name = "parse-display-derive" version = "0.8.2" @@ -2855,7 +2971,21 @@ dependencies = [ "quote", "regex", "regex-syntax 0.7.5", - "structmeta", + "structmeta 0.2.0", + "syn 2.0.80", +] + +[[package]] +name = "parse-display-derive" +version = "0.9.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2ae7800a4c974efd12df917266338e79a7a74415173caf7e70aa0a0707345281" +dependencies = [ + "proc-macro2", + "quote", + "regex", + "regex-syntax 0.8.5", + "structmeta 0.3.0", "syn 2.0.80", ] @@ -3447,6 +3577,15 @@ version = "0.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3b42e27ef78c35d3998403c1d26f3efd9e135d3e5121b0a4845cc5cc27547f4f" +[[package]] +name = "redox_syscall" +version = "0.3.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "567664f262709473930a4bf9e51bf2ebf3348f2e748ccc50dea20646858f8f29" +dependencies = [ + "bitflags 1.3.2", +] + [[package]] name = "redox_syscall" version = "0.5.7" @@ -3528,7 +3667,7 @@ dependencies = [ "pin-project-lite", "quinn", "rustls", - "rustls-native-certs", + "rustls-native-certs 0.8.0", "rustls-pemfile", "rustls-pki-types", "serde", @@ -3727,6 +3866,19 @@ dependencies = [ "zeroize", ] +[[package]] +name = "rustls-native-certs" +version = "0.7.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e5bfb394eeed242e909609f56089eecfe5fda225042e8b171791b9c95f5931e5" +dependencies = [ + "openssl-probe", + "rustls-pemfile", + "rustls-pki-types", + "schannel", + "security-framework", +] + [[package]] name = "rustls-native-certs" version = "0.8.0" @@ -3884,6 +4036,17 @@ dependencies = [ "serde", ] +[[package]] +name = "serde_repr" +version = "0.1.19" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6c64451ba24fc7a6a2d60fc75dd9c83c90903b19028d4eff35e88fc1e86564e9" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.80", +] + [[package]] name = "serde_tuple" version = "0.5.0" @@ -4397,7 +4560,19 @@ checksum = "78ad9e09554f0456d67a69c1584c9798ba733a5b50349a6c0d0948710523922d" dependencies = [ "proc-macro2", "quote", - "structmeta-derive", + "structmeta-derive 0.2.0", + "syn 2.0.80", +] + +[[package]] +name = "structmeta" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2e1575d8d40908d70f6fd05537266b90ae71b15dbbe7a8b7dffa2b759306d329" +dependencies = [ + "proc-macro2", + "quote", + "structmeta-derive 0.3.0", "syn 2.0.80", ] @@ -4412,6 +4587,17 @@ dependencies = [ "syn 2.0.80", ] +[[package]] +name = "structmeta-derive" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "152a0b65a590ff6c3da95cabe2353ee04e6167c896b28e3b14478c2636c922fc" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.80", +] + [[package]] name = "subst" version = "0.3.5" @@ -4519,6 +4705,44 @@ dependencies = [ "windows-sys 0.59.0", ] +[[package]] +name = "testcontainers" +version = "0.23.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5f40cc2bd72e17f328faf8ca7687fe337e61bccd8acf9674fa78dd3792b045e1" +dependencies = [ + "async-trait", + "bollard", + "bollard-stubs", + "bytes", + "docker_credential", + "either", + "etcetera", + "futures", + "log", + "memchr", + "parse-display 0.9.1", + "pin-project-lite", + "serde", + "serde_json", + "serde_with", + "thiserror", + "tokio", + "tokio-stream", + "tokio-tar", + "tokio-util", + "url", +] + +[[package]] +name = "testcontainers-modules" +version = "0.11.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "708fce58200e480633a428b7356fc39eb7fef02e47bd6faa94ba1d0746e6f17e" +dependencies = [ + "testcontainers", +] + [[package]] name = "thiserror" version = "1.0.64" @@ -4746,6 +4970,21 @@ dependencies = [ "tokio", ] +[[package]] +name = "tokio-tar" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9d5714c010ca3e5c27114c1cdeb9d14641ace49874aa5626d7149e47aedace75" +dependencies = [ + "filetime", + "futures-core", + "libc", + "redox_syscall 0.3.5", + "tokio", + "tokio-stream", + "xattr", +] + [[package]] name = "tokio-util" version = "0.7.12" @@ -4964,6 +5203,7 @@ dependencies = [ "form_urlencoded", "idna", "percent-encoding", + "serde", ] [[package]] @@ -5198,7 +5438,7 @@ version = "1.5.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "372d5b87f58ec45c384ba03563b03544dc5fadc3983e434b286913f5b4a9bb6d" dependencies = [ - "redox_syscall", + "redox_syscall 0.5.7", "wasite", "web-sys", ] @@ -5451,6 +5691,17 @@ dependencies = [ "tls_codec", ] +[[package]] +name = "xattr" +version = "1.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8da84f1a25939b27f6820d92aed108f83ff920fdf11a7b19366c27c4cda81d4f" +dependencies = [ + "libc", + "linux-raw-sys", + "rustix", +] + [[package]] name = "xmlparser" version = "0.13.6" diff --git a/Cargo.toml b/Cargo.toml index 8ed9582af..27b11a0c1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -34,7 +34,7 @@ anyhow = "1.0" approx = "0.5.1" async-trait = "0.1" bit-set = "0.8" -brotli = ">=5, <7" +brotli = ">=5, <8" cargo-husky = { version = "1", features = ["user-hooks"], default-features = false } clap = { version = "4", features = ["derive"] } criterion = { version = "0.5", features = ["async_futures", "async_tokio", "html_reports"] } @@ -83,6 +83,7 @@ sqlite-hashes = { version = "0.7.6", default-features = false, features = ["md5" sqlx = { version = "0.7", features = ["sqlite", "runtime-tokio"] } static-files = "0.2" subst = { version = "0.3", features = ["yaml"] } +testcontainers-modules = { version = "0.11.3", features = ["postgres"] } thiserror = "1" tile-grid = "0.6" tilejson = "0.4" diff --git a/martin/Cargo.toml b/martin/Cargo.toml index 7d9196052..28dc1c4d8 100644 --- a/martin/Cargo.toml +++ b/martin/Cargo.toml @@ -118,6 +118,7 @@ url.workspace = true static-files = { workspace = true, optional = true } [dev-dependencies] +anyhow.workspace = true cargo-husky.workspace = true criterion.workspace = true ctor.workspace = true @@ -125,3 +126,4 @@ indoc.workspace = true insta = { workspace = true, features = ["yaml"] } pprof.workspace = true rstest.workspace = true +testcontainers-modules.workspace = true diff --git a/martin/src/pg/errors.rs b/martin/src/pg/errors.rs index 69fc0fb91..b8b995ccb 100644 --- a/martin/src/pg/errors.rs +++ b/martin/src/pg/errors.rs @@ -49,9 +49,15 @@ pub enum PgError { #[error("Unable to parse PostGIS version {1}: {0}")] BadPostgisVersion(#[source] semver::Error, String), + #[error("Unable to parse PostgreSQL version {1}: {0}")] + BadPostgresVersion(#[source] semver::Error, String), + #[error("PostGIS version {0} is too old, minimum required is {1}")] PostgisTooOld(Version, Version), + #[error("PostgreSQL version {0} is too old, minimum required is {1}")] + PostgresqlTooOld(Version, Version), + #[error("Invalid extent setting in source {0} for table {1}: extent=0")] InvalidTableExtent(String, String), diff --git a/martin/src/pg/pool.rs b/martin/src/pg/pool.rs index 6fb3c7103..efa7d3ec2 100755 --- a/martin/src/pg/pool.rs +++ b/martin/src/pg/pool.rs @@ -6,17 +6,22 @@ use semver::Version; use crate::pg::config::PgConfig; use crate::pg::tls::{make_connector, parse_conn_str, SslModeOverride}; use crate::pg::PgError::{ - BadPostgisVersion, PostgisTooOld, PostgresError, PostgresPoolBuildError, PostgresPoolConnError, + BadPostgisVersion, BadPostgresVersion, PostgisTooOld, PostgresError, PostgresPoolBuildError, + PostgresPoolConnError, PostgresqlTooOld, }; use crate::pg::PgResult; pub const POOL_SIZE_DEFAULT: usize = 20; -// We require ST_TileEnvelope that was added in PostGIS 3.0.0 -// See https://postgis.net/docs/ST_TileEnvelope.html +/// We require `ST_TileEnvelope` that was added in [`PostGIS 3.0.0`](https://postgis.net/2019/10/PostGIS-3.0.0/) +/// See const MINIMUM_POSTGIS_VER: Version = Version::new(3, 0, 0); -// After this version we can use margin parameter in ST_TileEnvelope +/// Minimum version of postgres required for [`MINIMUM_POSTGIS_VER`] according to the [Support Matrix](https://trac.osgeo.org/postgis/wiki/UsersWikiPostgreSQLPostGIS) +const MINIMUM_POSTGRES_VER: Version = Version::new(11, 0, 0); +/// After this [`PostGIS`](https://postgis.net/) version we can use margin parameter in `ST_TileEnvelope` const RECOMMENDED_POSTGIS_VER: Version = Version::new(3, 1, 0); +/// Minimum version of postgres required for [`RECOMMENDED_POSTGIS_VER`] according to the [Support Matrix](https://trac.osgeo.org/postgis/wiki/UsersWikiPostgreSQLPostGIS) +const RECOMMENDED_POSTGRES_VER: Version = Version::new(12, 0, 0); #[derive(Clone, Debug)] pub struct PgPool { @@ -35,32 +40,26 @@ impl PgPool { .build() .map_err(|e| PostgresPoolBuildError(e, id.clone()))?; - let version: String = get_conn(&pool, id.as_str()) - .await? - .query_one( - r" -SELECT - (regexp_matches( - PostGIS_Lib_Version(), - '^(\d+\.\d+\.\d+)', - 'g' - ))[1] as version; - ", - &[], - ) - .await - .map(|row| row.get("version")) - .map_err(|e| PostgresError(e, "querying postgis version"))?; - - let version: Version = version.parse().map_err(|e| BadPostgisVersion(e, version))?; - if version < MINIMUM_POSTGIS_VER { - return Err(PostgisTooOld(version, MINIMUM_POSTGIS_VER)); + let conn = get_conn(&pool, &id).await?; + let pg_ver = get_postgres_version(&conn).await?; + if pg_ver < MINIMUM_POSTGRES_VER { + return Err(PostgresqlTooOld(pg_ver, MINIMUM_POSTGRES_VER)); + } + if pg_ver < RECOMMENDED_POSTGRES_VER { + warn!("PostgreSQL {pg_ver} is older than the recommended {RECOMMENDED_POSTGRES_VER}."); } - if version < RECOMMENDED_POSTGIS_VER { - warn!("PostGIS {version} is before the recommended {RECOMMENDED_POSTGIS_VER}. Margin parameter in ST_TileEnvelope is not supported, so tiles may be cut off at the edges."); + + let postgis_ver = get_postgis_version(&conn).await?; + if postgis_ver < MINIMUM_POSTGIS_VER { + return Err(PostgisTooOld(postgis_ver, MINIMUM_POSTGIS_VER)); + } + let margin = postgis_ver >= RECOMMENDED_POSTGIS_VER; + if !margin { + warn!("PostGIS {postgis_ver} is older than the recommended {RECOMMENDED_POSTGIS_VER}. Margin parameter in ST_TileEnvelope is not supported, so tiles may be cut off at the edges."); } - let margin = version >= RECOMMENDED_POSTGIS_VER; + info!("Connected to PostgreSQL {pg_ver} / PostGIS {postgis_ver} for source {id}"); + Ok(Self { id, pool, margin }) } @@ -120,3 +119,95 @@ async fn get_conn(pool: &Pool, id: &str) -> PgResult { .await .map_err(|e| PostgresPoolConnError(e, id.to_string())) } + +/// Get [PostgreSQL version](https://www.postgresql.org/support/versioning/). +/// `PostgreSQL` only has a Major.Minor versioning, so we use 0 the patch version +async fn get_postgres_version(conn: &Object) -> PgResult { + let version: String = conn + .query_one( + r" +SELECT (regexp_matches( + current_setting('server_version'), + '^(\d+\.\d+)', + 'g' + ))[1] || '.0' as version;", + &[], + ) + .await + .map(|row| row.get("version")) + .map_err(|e| PostgresError(e, "querying postgres version"))?; + + let version: Version = version + .parse() + .map_err(|e| BadPostgresVersion(e, version))?; + + Ok(version) +} + +/// Get [PostGIS version](https://postgis.net/docs/PostGIS_Lib_Version.html) +async fn get_postgis_version(conn: &Object) -> PgResult { + let version: String = conn + .query_one( + r" +SELECT (regexp_matches( + PostGIS_Lib_Version(), + '^(\d+\.\d+\.\d+)', + 'g' + ))[1] as version;", + &[], + ) + .await + .map(|row| row.get("version")) + .map_err(|e| PostgresError(e, "querying postgis version"))?; + + let version: Version = version.parse().map_err(|e| BadPostgisVersion(e, version))?; + + Ok(version) +} + +#[cfg(test)] +mod tests { + use super::*; + use deadpool_postgres::tokio_postgres::Config; + use postgres::NoTls; + use testcontainers_modules::postgres::Postgres; + use testcontainers_modules::testcontainers::runners::AsyncRunner as _; + use testcontainers_modules::testcontainers::ImageExt as _; + + #[tokio::test] + async fn parse_version() -> anyhow::Result<()> { + let node = Postgres::default() + .with_name("postgis/postgis") + .with_tag("11-3.0") // purposely very old and stable + .start() + .await?; + + let pg_config = Config::new() + .host(node.get_host().await?.to_string()) + .port(node.get_host_port_ipv4(5432).await?) + .dbname("postgres") + .user("postgres") + .password("postgres") + .to_owned(); + + let mgr_config = ManagerConfig { + recycling_method: RecyclingMethod::Fast, + }; + + let mgr = Manager::from_config(pg_config, NoTls, mgr_config); + let pool = Pool::builder(mgr).max_size(2).build()?; + let conn = pool.get().await?; + + let pg_version = get_postgres_version(&conn).await?; + assert_eq!(pg_version.major, 11); + assert!(pg_version.minor >= 10); // we don't want to break this testcase just because postgis updates that image + assert_eq!(pg_version.patch, 0); + + let postgis_version = get_postgis_version(&conn).await?; + assert_eq!(postgis_version.major, 3); + assert_eq!(postgis_version.minor, 0); + assert!(postgis_version.patch >= 3); // we don't want to break this testcase just because postgis updates that image + + Ok(()) + } +} diff --git a/martin/src/pg/query_tables.rs b/martin/src/pg/query_tables.rs index 23a13c15a..9aea4d575 100644 --- a/martin/src/pg/query_tables.rs +++ b/martin/src/pg/query_tables.rs @@ -24,8 +24,9 @@ static DEFAULT_CLIP_GEOM: bool = true; /// Examine a database to get a list of all tables that have geometry columns. pub async fn query_available_tables(pool: &PgPool) -> PgResult { - let conn = pool.get().await?; - let rows = conn + let rows = pool + .get() + .await? .query(include_str!("scripts/query_available_tables.sql"), &[]) .await .map_err(|e| PostgresError(e, "querying available tables"))?; diff --git a/tests/test.sh b/tests/test.sh index 8e73fdf6e..af36adb2f 100755 --- a/tests/test.sh +++ b/tests/test.sh @@ -172,6 +172,7 @@ validate_log() { # Older versions of PostGIS don't support the margin parameter, so we need to remove it from the log remove_line "$LOG_FILE" 'Margin parameter in ST_TileEnvelope is not supported' remove_line "$LOG_FILE" 'Source IDs must be unique' + remove_line "$LOG_FILE" 'PostgreSQL 11.10.0 is older than the recommended 12.0.0' echo "Checking for no other warnings or errors in the log" if grep -e ' ERROR ' -e ' WARN ' "$LOG_FILE"; then