From 1490f1a989135316d8888f88eca6762373b16094 Mon Sep 17 00:00:00 2001 From: Tayfun Bocek Date: Fri, 19 Jul 2024 01:24:53 +0300 Subject: [PATCH] feat: Abort build if config file contains invalid keys. fix(example): Remove extra `open` field from example config file. feat: Reject aditional fields in schema. fix: Flatten `Core` for schema generation. fix: Actually deny unknown fields in schema. --- Trunk.toml | 2 - schemas/config.json | 22 +++++-- src/config/models/build.rs | 1 + src/config/models/clean.rs | 1 + src/config/models/core.rs | 1 + src/config/models/hook.rs | 1 + src/config/models/mod.rs | 132 ++++++++++++++++++++++++++++++------- src/config/models/proxy.rs | 1 + src/config/models/serve.rs | 1 + src/config/models/tools.rs | 1 + src/config/models/watch.rs | 1 + 11 files changed, 130 insertions(+), 34 deletions(-) diff --git a/Trunk.toml b/Trunk.toml index 245dc341..e70d177c 100644 --- a/Trunk.toml +++ b/Trunk.toml @@ -38,8 +38,6 @@ ignore = [] addresses = ["127.0.0.1"] # The port to serve on. port = 8080 -# Open a browser tab once the initial build is complete. -open = false # Whether to disable fallback to index.html for missing files. no_spa = false # Disable auto-reload of the web app. diff --git a/schemas/config.json b/schemas/config.json index eb764332..b197761b 100644 --- a/schemas/config.json +++ b/schemas/config.json @@ -75,6 +75,7 @@ "$ref": "#/definitions/Watch" } }, + "additionalProperties": false, "definitions": { "AddressFamily": { "type": "string", @@ -221,7 +222,8 @@ "default": "index.html", "type": "string" } - } + }, + "additionalProperties": false }, "Clean": { "description": "Config options for the serve system.", @@ -240,7 +242,8 @@ "null" ] } - } + }, + "additionalProperties": false }, "Hook": { "description": "Config options for build system hooks.", @@ -269,7 +272,8 @@ } ] } - } + }, + "additionalProperties": false }, "Hooks": { "description": "Newtype for handling `Vec`", @@ -372,7 +376,8 @@ "default": false, "type": "boolean" } - } + }, + "additionalProperties": false }, "Serve": { "description": "Config options for the serve system.", @@ -528,7 +533,8 @@ } ] } - } + }, + "additionalProperties": false }, "Tools": { "description": "Config options for automatic application downloads.", @@ -566,7 +572,8 @@ "null" ] } - } + }, + "additionalProperties": false }, "Uri": { "type": "string", @@ -593,7 +600,8 @@ "type": "string" } } - } + }, + "additionalProperties": false }, "WsProtocol": { "description": "WebSocket protocol", diff --git a/src/config/models/build.rs b/src/config/models/build.rs index 088f1678..57c5b441 100644 --- a/src/config/models/build.rs +++ b/src/config/models/build.rs @@ -14,6 +14,7 @@ use std::{ /// Config options for the build system. #[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize, JsonSchema)] +#[serde(deny_unknown_fields)] pub struct Build { /// The index HTML file to drive the bundling process #[serde(default = "default::target")] diff --git a/src/config/models/clean.rs b/src/config/models/clean.rs index b933cf96..108b8f0b 100644 --- a/src/config/models/clean.rs +++ b/src/config/models/clean.rs @@ -6,6 +6,7 @@ use std::path::PathBuf; /// Config options for the serve system. #[derive(Clone, Debug, Default, PartialEq, Eq, Deserialize, Serialize, JsonSchema)] +#[serde(deny_unknown_fields)] pub struct Clean { /// The output dir for all final assets #[serde(default, skip_serializing_if = "Option::is_none")] diff --git a/src/config/models/core.rs b/src/config/models/core.rs index 89adbeed..493e9f0d 100644 --- a/src/config/models/core.rs +++ b/src/config/models/core.rs @@ -6,6 +6,7 @@ use std::path::PathBuf; /// Config options for the core project. #[derive(Clone, Debug, Default, PartialEq, Eq, Deserialize, Serialize, JsonSchema)] +#[serde(deny_unknown_fields)] pub struct Core { #[serde(default)] // align that with cargo's `rust-version` diff --git a/src/config/models/hook.rs b/src/config/models/hook.rs index c4994af6..5cf7f59f 100644 --- a/src/config/models/hook.rs +++ b/src/config/models/hook.rs @@ -6,6 +6,7 @@ use serde::{Deserialize, Serialize}; /// Config options for build system hooks. #[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize, JsonSchema)] #[serde(rename_all = "snake_case")] +#[serde(deny_unknown_fields)] pub struct Hook { /// The stage in the build process to execute this hook. pub stage: PipelineStage, diff --git a/src/config/models/mod.rs b/src/config/models/mod.rs index cc8306af..3f5bc958 100644 --- a/src/config/models/mod.rs +++ b/src/config/models/mod.rs @@ -19,6 +19,7 @@ pub use clean::*; pub use core::*; pub use hook::*; pub use proxy::*; +use serde_json::Value; pub use serve::*; pub use tools::*; pub use watch::*; @@ -28,9 +29,9 @@ mod test; use anyhow::{bail, Context, Result}; use schemars::JsonSchema; -use serde::Deserialize; +use serde::{de::IntoDeserializer, Deserialize}; use source::Source; -use std::path::PathBuf; +use std::{collections::HashMap, path::PathBuf}; use tracing::log; /// Common configuration model functionality @@ -41,33 +42,114 @@ pub trait ConfigModel { } } -/// The persisted Trunk configuration model -#[derive(Clone, Debug, Default, PartialEq, Eq, Deserialize, JsonSchema)] -pub struct Configuration { - #[serde(flatten)] - pub core: Core, - - #[serde(default)] - pub build: Build, - - #[serde(default)] - pub tools: Tools, - - #[serde(default)] - pub hooks: Hooks, +// Generator macro for the Configuration structure. +// +// Any field that does not require flattening (#[serde(flatten)]) should be defined inside the +// macro. +macro_rules! config_gen { + ($(#[$meta:meta])* $vis:vis $ident:ident, $($(#[$field_meta:meta])* $field_vis:vis $field:ident: $ty:ty),*) => { + $( + #[$meta] + )* + $vis struct $ident { + #[serde(default)] + pub build: Build, + + #[serde(default)] + pub tools: Tools, + + #[serde(default)] + pub hooks: Hooks, + + #[serde(default)] + pub watch: Watch, + + #[serde(default)] + pub serve: Serve, + + #[serde(default)] + pub clean: Clean, + + #[serde(default)] + #[serde(alias = "proxy")] + pub proxies: Proxies, + $( + $( + #[$field_meta] + )* + $field_vis $field: $ty + )* + } + }; +} - #[serde(default)] - pub watch: Watch, +config_gen! { + #[doc="The persisted Trunk configuration model"] + #[derive(Clone, Debug, Default, PartialEq, Eq, JsonSchema)] + // The deny unknown fields here is actually not for serde, but for `schemars::JsonSchema`. + #[serde(deny_unknown_fields)] + pub Configuration, + // The flatten is fine as we are not using it for serde, but for `schemars::JsonSchema`. + // Meaning there isn't any conflicts between "flatten", and "deny_unknown_fields". + #[serde(flatten)] + pub core: Core +} - #[serde(default)] - pub serve: Serve, +// HACK: We do not have a way to properly use `#[serde(flatten)]`, and `#[serde(deny_unknown_fields)]` together. +// However, we can still implement `Deserialize` manually so that `Configuration` returns cleaner errors. +// Once the mentioned attributes can be used together, this implementation can simply be replaced +// with two attributes on the structs field. +impl<'de> Deserialize<'de> for Configuration { + fn deserialize(deserializer: D) -> std::result::Result + where + D: serde::Deserializer<'de>, + { + // Collect the configuration into a temporary struct, and store extra fields, and values + // inside the hashmap. + // + // The extra fields stored in the `HashMap`, are handled by `Core::deserialize` later. + config_gen! { + #[derive(Deserialize)] + InnerCfg, + #[serde(flatten)] + extras: HashMap + } - #[serde(default)] - pub clean: Clean, + impl From<(Core, InnerCfg)> for Configuration { + fn from((core, inner_cfg): (Core, InnerCfg)) -> Self { + Configuration { + core, + build: inner_cfg.build, + tools: inner_cfg.tools, + hooks: inner_cfg.hooks, + watch: inner_cfg.watch, + serve: inner_cfg.serve, + clean: inner_cfg.clean, + proxies: inner_cfg.proxies, + } + } + } - #[serde(default)] - #[serde(alias = "proxy")] - pub proxies: Proxies, + // This can still return an error due to unknown fields from the structs other fields + // which deny them (such as `Build`, `Serve`...). + // However any top level fields defined are stored in `extras`. + let inner_cfg = InnerCfg::deserialize(deserializer)?; + + let extras = inner_cfg.extras.clone(); + + // Use the extra fields from `InnerCfg` to initialize the `Core`. + // This is essentially the same as flattening and denying unknown fields. + // Since the top level unknown fields come here, and `Core` denies unknown fields, it is + // returned as an error. + // + // NOTE: Top level key/field errors will return less information compared to the error from + // `InnerCfg`. The most important information, which is the key/field name will always be + // returned. + let core = + Core::deserialize(extras.into_deserializer()).map_err(serde::de::Error::custom)?; + + Ok((core, inner_cfg).into()) + } } impl ConfigModel for Configuration { diff --git a/src/config/models/proxy.rs b/src/config/models/proxy.rs index 8a13a1b1..9b6b1560 100644 --- a/src/config/models/proxy.rs +++ b/src/config/models/proxy.rs @@ -4,6 +4,7 @@ use serde::Deserialize; /// Config options for building proxies. #[derive(Clone, Debug, PartialEq, Eq, Deserialize, JsonSchema)] +#[serde(deny_unknown_fields)] pub struct Proxy { /// The URL of the backend to which requests are to be proxied. pub backend: Uri, diff --git a/src/config/models/serve.rs b/src/config/models/serve.rs index d61bf914..e1431976 100644 --- a/src/config/models/serve.rs +++ b/src/config/models/serve.rs @@ -9,6 +9,7 @@ use tracing::log; /// Config options for the serve system. #[derive(Clone, Debug, PartialEq, Eq, Deserialize, JsonSchema)] +#[serde(deny_unknown_fields)] pub struct Serve { /// A single address to serve on. // This is required for the TOML to allow a single "address" field as before diff --git a/src/config/models/tools.rs b/src/config/models/tools.rs index 81ce88ac..e98c877f 100644 --- a/src/config/models/tools.rs +++ b/src/config/models/tools.rs @@ -9,6 +9,7 @@ use serde::Deserialize; // at all, this struct is used for both configuration as well as CLI arguments. #[derive(Clone, Debug, Default, PartialEq, Eq, Deserialize, Args, JsonSchema)] #[command(next_help_heading = "Tools")] +#[serde(deny_unknown_fields)] pub struct Tools { /// Version of `dart-sass` to use. #[serde(default)] diff --git a/src/config/models/watch.rs b/src/config/models/watch.rs index abad4e81..4cdef5ae 100644 --- a/src/config/models/watch.rs +++ b/src/config/models/watch.rs @@ -5,6 +5,7 @@ use std::path::PathBuf; /// Config options for the watch system. #[derive(Clone, Debug, Default, PartialEq, Eq, Deserialize, JsonSchema)] +#[serde(deny_unknown_fields)] pub struct Watch { /// Watch specific file(s) or folder(s) [default: build target parent folder] #[serde(default, skip_serializing_if = "Vec::is_empty")]