diff --git a/Cargo.lock b/Cargo.lock index a8e400524e..13cfbc4ca5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4270,6 +4270,7 @@ dependencies = [ "scopeguard", "send-future", "serde", + "serde_ignored", "serde_json", "serde_path_to_error", "serde_with", @@ -4692,7 +4693,7 @@ dependencies = [ [[package]] name = "postgres-protocol" version = "0.6.6" -source = "git+https://github.com/neondatabase/rust-postgres.git?branch=neon#1f21e7959a96a34dcfbfce1b14b73286cdadffe9" +source = "git+https://github.com/neondatabase/rust-postgres.git?branch=neon#394851e467755562b4173ff68f9eb0e7f737be13" dependencies = [ "base64 0.22.1", "byteorder", @@ -4726,7 +4727,7 @@ dependencies = [ [[package]] name = "postgres-types" version = "0.2.6" -source = "git+https://github.com/neondatabase/rust-postgres.git?branch=neon#1f21e7959a96a34dcfbfce1b14b73286cdadffe9" +source = "git+https://github.com/neondatabase/rust-postgres.git?branch=neon#394851e467755562b4173ff68f9eb0e7f737be13" dependencies = [ "bytes", "chrono", @@ -6287,6 +6288,15 @@ dependencies = [ "syn 2.0.100", ] +[[package]] +name = "serde_ignored" +version = "0.1.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "566da67d80e92e009728b3731ff0e5360cb181432b8ca73ea30bb1d170700d76" +dependencies = [ + "serde", +] + [[package]] name = "serde_json" version = "1.0.125" @@ -7170,7 +7180,7 @@ dependencies = [ [[package]] name = "tokio-postgres" version = "0.7.10" -source = "git+https://github.com/neondatabase/rust-postgres.git?branch=neon#1f21e7959a96a34dcfbfce1b14b73286cdadffe9" +source = "git+https://github.com/neondatabase/rust-postgres.git?branch=neon#394851e467755562b4173ff68f9eb0e7f737be13" dependencies = [ "async-trait", "byteorder", diff --git a/Cargo.toml b/Cargo.toml index 9bbc5a1a38..e93b39cafc 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -164,6 +164,7 @@ sd-notify = "0.4.1" send-future = "0.1.0" sentry = { version = "0.32", default-features = false, features = ["backtrace", "contexts", "panic", "rustls", "reqwest" ] } serde = { version = "1.0", features = ["derive"] } +serde_ignored = "0.1.11" serde_json = "1" serde_path_to_error = "0.1" serde_with = { version = "2.0", features = [ "base64" ] } diff --git a/pageserver/Cargo.toml b/pageserver/Cargo.toml index 56d97bf8a9..2c6ce4fd4e 100644 --- a/pageserver/Cargo.toml +++ b/pageserver/Cargo.toml @@ -52,6 +52,7 @@ rustls.workspace = true scopeguard.workspace = true send-future.workspace = true serde.workspace = true +serde_ignored.workspace = true serde_json = { workspace = true, features = ["raw_value"] } serde_path_to_error.workspace = true serde_with.workspace = true diff --git a/pageserver/src/bin/pageserver.rs b/pageserver/src/bin/pageserver.rs index 8e29fdbf54..f22e68f775 100644 --- a/pageserver/src/bin/pageserver.rs +++ b/pageserver/src/bin/pageserver.rs @@ -96,7 +96,7 @@ fn main() -> anyhow::Result<()> { env::set_current_dir(&workdir) .with_context(|| format!("Failed to set application's current dir to '{workdir}'"))?; - let conf = initialize_config(&identity_file_path, &cfg_file_path, &workdir)?; + let (conf, ignored) = initialize_config(&identity_file_path, &cfg_file_path, &workdir)?; // Initialize logging. // @@ -127,7 +127,17 @@ fn main() -> anyhow::Result<()> { &[("node_id", &conf.id.to_string())], ); - // after setting up logging, log the effective IO engine choice and read path implementations + // Warn about ignored config items; see pageserver_api::config::ConfigToml + // doc comment for rationale why we prefer this over serde(deny_unknown_fields). + { + let IgnoredConfigItems { paths } = ignored; + for path in paths { + warn!(?path, "ignoring unknown configuration item"); + } + } + + // Log configuration items for feature-flag-like config + // (maybe we should automate this with a visitor?). info!(?conf.virtual_file_io_engine, "starting with virtual_file IO engine"); info!(?conf.virtual_file_io_mode, "starting with virtual_file IO mode"); info!(?conf.wal_receiver_protocol, "starting with WAL receiver protocol"); @@ -196,11 +206,15 @@ fn main() -> anyhow::Result<()> { Ok(()) } +struct IgnoredConfigItems { + paths: Vec, +} + fn initialize_config( identity_file_path: &Utf8Path, cfg_file_path: &Utf8Path, workdir: &Utf8Path, -) -> anyhow::Result<&'static PageServerConf> { +) -> anyhow::Result<(&'static PageServerConf, IgnoredConfigItems)> { // The deployment orchestrator writes out an indentity file containing the node id // for all pageservers. This file is the source of truth for the node id. In order // to allow for rolling back pageserver releases, the node id is also included in @@ -229,15 +243,23 @@ fn initialize_config( let config_file_contents = std::fs::read_to_string(cfg_file_path).context("read config file from filesystem")?; - let config_toml = serde_path_to_error::deserialize( - toml_edit::de::Deserializer::from_str(&config_file_contents) - .context("build toml deserializer")?, - ) - .context("deserialize config toml")?; + let deserializer = toml_edit::de::Deserializer::from_str(&config_file_contents) + .context("build toml deserializer")?; + let mut path_to_error_track = serde_path_to_error::Track::new(); + let deserializer = + serde_path_to_error::Deserializer::new(deserializer, &mut path_to_error_track); + let mut ignored = Vec::new(); + let mut push_to_ignored = |path: serde_ignored::Path<'_>| { + ignored.push(path.to_string()); + }; + let deserializer = serde_ignored::Deserializer::new(deserializer, &mut push_to_ignored); + let config_toml: pageserver_api::config::ConfigToml = + serde::Deserialize::deserialize(deserializer).context("deserialize config toml")?; let conf = PageServerConf::parse_and_validate(identity.id, config_toml, workdir) .context("runtime-validation of config toml")?; - - Ok(Box::leak(Box::new(conf))) + let conf = Box::leak(Box::new(conf)); + let ignored = IgnoredConfigItems { paths: ignored }; + Ok((conf, ignored)) } struct WaitForPhaseResult { diff --git a/test_runner/regress/test_pageserver_config.py b/test_runner/regress/test_pageserver_config.py new file mode 100644 index 0000000000..3053781298 --- /dev/null +++ b/test_runner/regress/test_pageserver_config.py @@ -0,0 +1,35 @@ +import pytest +from fixtures.neon_fixtures import NeonEnv + + +@pytest.mark.parametrize("what", ["default", "top_level", "nested"]) +def test_unknown_config_items_handling(neon_simple_env: NeonEnv, what: str): + env = neon_simple_env + + def edit_fn(config) -> str | None: + if what == "default": + return None + elif what == "top_level": + config["unknown_top_level_config_item"] = 23 + return r".*unknown_top_level_config_item.*" + elif what == "nested": + config["remote_storage"]["unknown_config_item"] = 23 + return r".*remote_storage\.unknown_config_item.*" + else: + raise ValueError(f"Unknown what: {what}") + + expect_warn_re = env.pageserver.edit_config_toml(edit_fn) + + if expect_warn_re is not None: + env.pageserver.allowed_errors.append(expect_warn_re) + + if expect_warn_re is not None: + assert not env.pageserver.log_contains(expect_warn_re) + + # in any way, unknown config items should not fail pageserver to start + # TODO: extend this test with the config validator mode once we introduce it + # https://github.com/neondatabase/cloud/issues/24349 + env.pageserver.restart() + + if expect_warn_re is not None: + assert env.pageserver.log_contains(expect_warn_re)