diff --git a/CHANGELOG.md b/CHANGELOG.md index 790da2ca..efd80fc1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ This changelog documents the changes between release versions. ### Changed +- **BREAKING:** If `configuration.json` cannot be parsed the connector will fail to start. This change also prohibits unknown keys in that file. These changes will help to prevent typos configuration being silently ignored. ([#115](https://github.com/hasura/ndc-mongodb/pull/115)) + ### Fixed - Fixes for filtering by complex predicate that references variables, or field names that require escaping ([#111](https://github.com/hasura/ndc-mongodb/pull/111)) diff --git a/Cargo.lock b/Cargo.lock index 9157fbe5..9157cbc5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -142,6 +142,15 @@ dependencies = [ "syn 2.0.66", ] +[[package]] +name = "async-tempfile" +version = "0.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "acb90d9834a8015109afc79f1f548223a0614edcbab62fb35b62d4b707e975e7" +dependencies = [ + "tokio", +] + [[package]] name = "async-trait" version = "0.1.80" @@ -442,7 +451,9 @@ name = "configuration" version = "1.3.0" dependencies = [ "anyhow", + "async-tempfile", "futures", + "googletest", "itertools", "mongodb", "mongodb-support", @@ -972,6 +983,28 @@ version = "0.3.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d2fabcfbdc87f4758337ca535fb41a6d701b65693ce38287d856d1674551ec9b" +[[package]] +name = "googletest" +version = "0.12.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "22e38fa267f4db1a2fa51795ea4234eaadc3617a97486a9f158de9256672260e" +dependencies = [ + "googletest_macro", + "num-traits", + "regex", + "rustversion", +] + +[[package]] +name = "googletest_macro" +version = "0.12.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "171deab504ad43a9ea80324a3686a0cbe9436220d9d0b48ae4d7f7bd303b48a9" +dependencies = [ + "quote", + "syn 2.0.66", +] + [[package]] name = "h2" version = "0.3.26" @@ -2938,9 +2971,9 @@ checksum = "388a1df253eca08550bef6c72392cfe7c30914bf41df5269b68cbd6ff8f570a3" [[package]] name = "serde" -version = "1.0.203" +version = "1.0.210" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7253ab4de971e72fb7be983802300c30b5a7f0c2e56fab8abfc6a214307c0094" +checksum = "c8e3592472072e6e22e0a54d5904d9febf8508f65fb8552499a1abc7d1078c3a" dependencies = [ "serde_derive", ] @@ -2956,9 +2989,9 @@ dependencies = [ [[package]] name = "serde_derive" -version = "1.0.203" +version = "1.0.210" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "500cbc0ebeb6f46627f50f3f5811ccf6bf00643be300b4c3eabc0ef55dc5b5ba" +checksum = "243902eda00fad750862fc144cea25caca5e20d615af0a81bee94ca738f1df1f" dependencies = [ "proc-macro2", "quote", @@ -2978,12 +3011,13 @@ dependencies = [ [[package]] name = "serde_json" -version = "1.0.117" +version = "1.0.128" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "455182ea6142b14f93f4bc5320a2b31c1f266b66a4a5c858b013302a5d8cbfc3" +checksum = "6ff5456707a1de34e7e37f2a6fd3d3f808c318259cbd01ab6377795054b483d8" dependencies = [ "indexmap 2.2.6", "itoa", + "memchr", "ryu", "serde", ] diff --git a/Cargo.toml b/Cargo.toml index a810491a..5a86c314 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -27,6 +27,8 @@ indexmap = { version = "2", features = [ itertools = "^0.12.1" mongodb = { version = "2.8", features = ["tracing-unstable"] } schemars = "^0.8.12" +serde = { version = "1", features = ["derive"] } +serde_json = { version = "1.0", features = ["preserve_order", "raw_value"] } ref-cast = "1.0.23" # Connecting to MongoDB Atlas database with time series collections fails in the diff --git a/crates/cli/Cargo.toml b/crates/cli/Cargo.toml index 40b77c19..e4a18735 100644 --- a/crates/cli/Cargo.toml +++ b/crates/cli/Cargo.toml @@ -20,8 +20,8 @@ indexmap = { workspace = true } itertools = { workspace = true } ndc-models = { workspace = true } nom = "^7.1.3" -serde = { version = "1.0", features = ["derive"] } -serde_json = { version = "1.0.113", features = ["raw_value"] } +serde = { workspace = true } +serde_json = { workspace = true } thiserror = "1.0.57" tokio = { version = "1.36.0", features = ["full"] } diff --git a/crates/cli/src/lib.rs b/crates/cli/src/lib.rs index 0e4e81a8..e09ae645 100644 --- a/crates/cli/src/lib.rs +++ b/crates/cli/src/lib.rs @@ -61,7 +61,7 @@ async fn update(context: &Context, args: &UpdateArgs) -> anyhow::Result<()> { let connector_state = try_init_state_from_uri(context.connection_uri.as_ref()).await?; let configuration_options = - configuration::parse_configuration_options_file(&context.path).await; + configuration::parse_configuration_options_file(&context.path).await?; // Prefer arguments passed to cli, and fallback to the configuration file let sample_size = match args.sample_size { Some(size) => size, diff --git a/crates/cli/src/native_query/mod.rs b/crates/cli/src/native_query/mod.rs index f25be213..90221bfe 100644 --- a/crates/cli/src/native_query/mod.rs +++ b/crates/cli/src/native_query/mod.rs @@ -56,7 +56,7 @@ pub async fn run(context: &Context, command: Command) -> anyhow::Result<()> { let configuration = match read_directory(&context.path).await { Ok(c) => c, Err(err) => { - eprintln!("Could not read connector configuration - configuration must be initialized before creating native queries.\n\n{err}"); + eprintln!("Could not read connector configuration - configuration must be initialized before creating native queries.\n\n{err:#}"); exit(ExitCode::CouldNotReadConfiguration.into()) } }; diff --git a/crates/configuration/Cargo.toml b/crates/configuration/Cargo.toml index 264c51d5..8c3aa88e 100644 --- a/crates/configuration/Cargo.toml +++ b/crates/configuration/Cargo.toml @@ -14,9 +14,13 @@ mongodb = { workspace = true } ndc-models = { workspace = true } ref-cast = { workspace = true } schemars = { workspace = true } -serde = { version = "1", features = ["derive"] } -serde_json = { version = "1" } +serde = { workspace = true } +serde_json = { workspace = true } serde_yaml = "^0.9" tokio = "1" tokio-stream = { version = "^0.1", features = ["fs"] } tracing = "0.1" + +[dev-dependencies] +async-tempfile = "^0.6.0" +googletest = "^0.12.0" diff --git a/crates/configuration/src/configuration.rs b/crates/configuration/src/configuration.rs index d1c6a38b..729b680b 100644 --- a/crates/configuration/src/configuration.rs +++ b/crates/configuration/src/configuration.rs @@ -203,7 +203,7 @@ impl Configuration { } #[derive(Clone, Debug, Default, Deserialize, Serialize)] -#[serde(rename_all = "camelCase")] +#[serde(rename_all = "camelCase", deny_unknown_fields)] pub struct ConfigurationOptions { /// Options for introspection pub introspection_options: ConfigurationIntrospectionOptions, @@ -215,7 +215,7 @@ pub struct ConfigurationOptions { } #[derive(Copy, Clone, Debug, Deserialize, Serialize)] -#[serde(rename_all = "camelCase")] +#[serde(rename_all = "camelCase", deny_unknown_fields)] pub struct ConfigurationIntrospectionOptions { // For introspection how many documents should be sampled per collection. pub sample_size: u32, @@ -238,7 +238,7 @@ impl Default for ConfigurationIntrospectionOptions { } #[derive(Clone, Debug, Default, Deserialize, Serialize)] -#[serde(rename_all = "camelCase")] +#[serde(rename_all = "camelCase", deny_unknown_fields)] pub struct ConfigurationSerializationOptions { /// Extended JSON has two modes: canonical and relaxed. This option determines which mode is /// used for output. This setting has no effect on inputs (query arguments, etc.). diff --git a/crates/configuration/src/directory.rs b/crates/configuration/src/directory.rs index 3976e99f..b6fd1899 100644 --- a/crates/configuration/src/directory.rs +++ b/crates/configuration/src/directory.rs @@ -63,7 +63,7 @@ pub async fn read_directory( .await? .unwrap_or_default(); - let options = parse_configuration_options_file(dir).await; + let options = parse_configuration_options_file(dir).await?; native_mutations.extend(native_procedures.into_iter()); @@ -129,24 +129,35 @@ where } } -pub async fn parse_configuration_options_file(dir: &Path) -> ConfigurationOptions { - let json_filename = CONFIGURATION_OPTIONS_BASENAME.to_owned() + ".json"; - let json_config_file = parse_config_file(&dir.join(json_filename), JSON).await; - if let Ok(config_options) = json_config_file { - return config_options; +pub async fn parse_configuration_options_file(dir: &Path) -> anyhow::Result { + let json_filename = configuration_file_path(dir, JSON); + if fs::try_exists(&json_filename).await? { + return parse_config_file(json_filename, JSON).await; } - let yaml_filename = CONFIGURATION_OPTIONS_BASENAME.to_owned() + ".yaml"; - let yaml_config_file = parse_config_file(&dir.join(yaml_filename), YAML).await; - if let Ok(config_options) = yaml_config_file { - return config_options; + let yaml_filename = configuration_file_path(dir, YAML); + if fs::try_exists(&yaml_filename).await? { + return parse_config_file(yaml_filename, YAML).await; } + tracing::warn!( + "{CONFIGURATION_OPTIONS_BASENAME}.json not found, using default connector settings" + ); + // If a configuration file does not exist use defaults and write the file let defaults: ConfigurationOptions = Default::default(); let _ = write_file(dir, CONFIGURATION_OPTIONS_BASENAME, &defaults).await; let _ = write_config_metadata_file(dir).await; - defaults + Ok(defaults) +} + +fn configuration_file_path(dir: &Path, format: FileFormat) -> PathBuf { + let mut file_path = dir.join(CONFIGURATION_OPTIONS_BASENAME); + match format { + FileFormat::Json => file_path.set_extension("json"), + FileFormat::Yaml => file_path.set_extension("yaml"), + }; + file_path } async fn parse_config_file(path: impl AsRef, format: FileFormat) -> anyhow::Result @@ -276,3 +287,45 @@ pub async fn get_config_file_changed(dir: impl AsRef) -> anyhow::Result Ok(true), } } + +#[cfg(test)] +mod tests { + use async_tempfile::TempDir; + use googletest::prelude::*; + use serde_json::json; + use tokio::fs; + + use super::{read_directory, CONFIGURATION_OPTIONS_BASENAME}; + + #[googletest::test] + #[tokio::test] + async fn errors_on_typo_in_extended_json_mode_string() -> Result<()> { + let input = json!({ + "introspectionOptions": { + "sampleSize": 1_000, + "noValidatorSchema": true, + "allSchemaNullable": false, + }, + "serializationOptions": { + "extendedJsonMode": "no-such-mode", + }, + }); + + let config_dir = TempDir::new().await?; + let mut config_file = config_dir.join(CONFIGURATION_OPTIONS_BASENAME); + config_file.set_extension("json"); + fs::write(config_file, serde_json::to_vec(&input)?).await?; + + let actual = read_directory(config_dir).await; + + expect_that!( + actual, + err(predicate(|e: &anyhow::Error| e + .root_cause() + .to_string() + .contains("unknown variant `no-such-mode`"))) + ); + + Ok(()) + } +} diff --git a/crates/integration-tests/Cargo.toml b/crates/integration-tests/Cargo.toml index 598c39a3..8986e0a0 100644 --- a/crates/integration-tests/Cargo.toml +++ b/crates/integration-tests/Cargo.toml @@ -14,7 +14,7 @@ anyhow = "1" assert_json = "^0.1" insta = { version = "^1.38", features = ["yaml"] } reqwest = { version = "^0.12.4", features = ["json"] } -serde = { version = "1", features = ["derive"] } -serde_json = "1" +serde = { workspace = true } +serde_json = { workspace = true } tokio = { version = "^1.37.0", features = ["full"] } url = "^2.5.0" diff --git a/crates/mongodb-agent-common/Cargo.toml b/crates/mongodb-agent-common/Cargo.toml index d123e86f..6ad0ca18 100644 --- a/crates/mongodb-agent-common/Cargo.toml +++ b/crates/mongodb-agent-common/Cargo.toml @@ -26,8 +26,8 @@ ndc-models = { workspace = true } once_cell = "1" regex = "1" schemars = { version = "^0.8.12", features = ["smol_str"] } -serde = { version = "1.0", features = ["derive"] } -serde_json = { version = "1.0", features = ["preserve_order"] } +serde = { workspace = true } +serde_json = { workspace = true } serde_with = { version = "^3.7", features = ["base64", "hex"] } thiserror = "1" time = { version = "0.3.29", features = ["formatting", "parsing", "serde"] } diff --git a/crates/mongodb-connector/Cargo.toml b/crates/mongodb-connector/Cargo.toml index 65de56c5..26c0ec6e 100644 --- a/crates/mongodb-connector/Cargo.toml +++ b/crates/mongodb-connector/Cargo.toml @@ -19,8 +19,8 @@ itertools = { workspace = true } mongodb = { workspace = true } ndc-sdk = { workspace = true } prometheus = "*" # share version from ndc-sdk -serde = { version = "1.0", features = ["derive"] } -serde_json = { version = "1.0", features = ["preserve_order"] } +serde = { workspace = true } +serde_json = { workspace = true } thiserror = "1" tokio = { version = "1.28.1", features = ["full"] } tracing = "0.1" diff --git a/crates/mongodb-connector/src/mongo_connector.rs b/crates/mongodb-connector/src/mongo_connector.rs index 538913af..3545621f 100644 --- a/crates/mongodb-connector/src/mongo_connector.rs +++ b/crates/mongodb-connector/src/mongo_connector.rs @@ -38,10 +38,11 @@ impl ConnectorSetup for MongoConnector { .map_err(|err| { ErrorResponse::new( StatusCode::INTERNAL_SERVER_ERROR, - err.to_string(), + format!("{err:#}"), // alternate selector (:#) includes root cause in string json!({}), ) })?; + tracing::debug!(?configuration); Ok(MongoConfiguration(configuration)) } diff --git a/crates/mongodb-support/Cargo.toml b/crates/mongodb-support/Cargo.toml index 95ca3c3b..d8ea8c91 100644 --- a/crates/mongodb-support/Cargo.toml +++ b/crates/mongodb-support/Cargo.toml @@ -9,6 +9,6 @@ enum-iterator = "^2.0.0" indexmap = { workspace = true } mongodb = { workspace = true } schemars = "^0.8.12" -serde = { version = "1", features = ["derive"] } -serde_json = "1" +serde = { workspace = true } +serde_json = { workspace = true } thiserror = "1" diff --git a/crates/ndc-query-plan/Cargo.toml b/crates/ndc-query-plan/Cargo.toml index 39110ce2..732640c9 100644 --- a/crates/ndc-query-plan/Cargo.toml +++ b/crates/ndc-query-plan/Cargo.toml @@ -10,7 +10,7 @@ indexmap = { workspace = true } itertools = { workspace = true } ndc-models = { workspace = true } nonempty = "^0.10" -serde_json = "1" +serde_json = { workspace = true } thiserror = "1" ref-cast = { workspace = true }