use serde_ignored to warn about ignored unknown fields

This commit is contained in:
Christian Schwarz
2025-03-20 09:44:06 +01:00
parent 13fa4b48c3
commit e275df8640
5 changed files with 82 additions and 13 deletions

16
Cargo.lock generated
View File

@@ -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",

View File

@@ -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" ] }

View File

@@ -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

View File

@@ -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<String>,
}
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<F: std::future::Future + Unpin> {

View File

@@ -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)