From e3a2795beeb090f8b5e8eab008aeaeb3a8c3a5fd Mon Sep 17 00:00:00 2001 From: Spencer Ferris <3319370+spencewenski@users.noreply.github.com> Date: Sun, 19 May 2024 12:14:32 -0700 Subject: [PATCH] Add validation of the AppConfig Most things don't need validation, so they just have the Validate derive and `#[validate(nested)]` applied to get the validations initially chained through the app config. However, we did implement a validation for the `DefaultRoutes` config. --- Cargo.toml | 1 + src/app.rs | 16 +++++ src/cli/mod.rs | 5 ++ src/cli/print_config.rs | 10 ++-- src/config/app_config.rs | 22 ++++--- src/config/service/http/initializer.rs | 3 +- src/config/service/http/middleware.rs | 3 +- src/config/service/http/mod.rs | 74 +++++++++++++++++++++--- src/config/service/mod.rs | 10 +++- src/config/service/worker/sidekiq/mod.rs | 14 +++-- src/controller/http/docs.rs | 13 ----- src/service/worker/sidekiq/app_worker.rs | 3 +- 12 files changed, 130 insertions(+), 44 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 1eb93133..abeda96c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -94,6 +94,7 @@ num-traits = "0.2.19" log = "0.4.21" mockall_double = "0.3.1" futures = "0.3.30" +validator = { version = "0.18.1", features = ["derive"] } [dev-dependencies] cargo-husky = { version = "1.5.0", default-features = false, features = ["user-hooks"] } diff --git a/src/app.rs b/src/app.rs index f329a402..9001a285 100644 --- a/src/app.rs +++ b/src/app.rs @@ -25,6 +25,7 @@ use std::future::Future; use tokio::task::JoinSet; use tokio_util::sync::CancellationToken; use tracing::{error, info, instrument, warn}; +use validator::Validate; // todo: this method is getting unweildy, we should break it up pub async fn start( @@ -84,6 +85,11 @@ where A::init_tracing(&config)?; + #[cfg(not(feature = "cli"))] + validate_config(&config, true)?; + #[cfg(feature = "cli")] + validate_config(&config, !roadster_cli.skip_validate_config)?; + #[cfg(all(not(test), feature = "db-sql"))] let db = Database::connect(A::db_connection_options(&config)?).await?; @@ -242,6 +248,16 @@ where Ok(()) } +fn validate_config(config: &AppConfig, exit_on_error: bool) -> anyhow::Result<()> { + let result = config.validate(); + if exit_on_error { + result?; + } else if let Err(err) = result { + warn!("An error occurred when validating the app config: {}", err); + } + Ok(()) +} + #[async_trait] pub trait App: Send + Sync { // Todo: Are clone, etc necessary if we store it inside an Arc? diff --git a/src/cli/mod.rs b/src/cli/mod.rs index 97d2c561..ba45e27f 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -71,6 +71,11 @@ pub struct RoadsterCli { #[clap(short, long)] pub environment: Option, + /// Skip validation of the app config. This can be useful for debugging the app config + /// when used in conjunction with the `print-config` command. + #[clap(long, action)] + pub skip_validate_config: bool, + /// Allow dangerous/destructive operations when running in the `production` environment. If /// this argument is not provided, dangerous/destructive operations will not be performed /// when running in `production`. diff --git a/src/cli/print_config.rs b/src/cli/print_config.rs index 1f754a1c..c528c5b9 100644 --- a/src/cli/print_config.rs +++ b/src/cli/print_config.rs @@ -42,19 +42,19 @@ where ) -> anyhow::Result { match self.format { Format::Debug => { - info!("{:?}", context.config()) + info!("\n{:?}", context.config()) } Format::Json => { - info!("{}", serde_json::to_string(&context.config())?) + info!("\n{}", serde_json::to_string(&context.config())?) } Format::JsonPretty => { - info!("{}", serde_json::to_string_pretty(&context.config())?) + info!("\n{}", serde_json::to_string_pretty(&context.config())?) } Format::Toml => { - info!("{}", toml::to_string(&context.config())?) + info!("\n{}", toml::to_string(&context.config())?) } Format::TomlPretty => { - info!("{}", toml::to_string_pretty(&context.config())?) + info!("\n{}", toml::to_string_pretty(&context.config())?) } } diff --git a/src/config/app_config.rs b/src/config/app_config.rs index 25a36cb4..85d1093e 100644 --- a/src/config/app_config.rs +++ b/src/config/app_config.rs @@ -13,16 +13,22 @@ use std::collections::BTreeMap; use std::time::Duration; #[cfg(any(feature = "otel", feature = "db-sql"))] use url::Url; +use validator::Validate; -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, Validate, Serialize, Deserialize)] #[serde(rename_all = "kebab-case")] pub struct AppConfig { + #[validate(nested)] pub app: App, + #[validate(nested)] pub service: Service, + #[validate(nested)] pub auth: Auth, + #[validate(nested)] pub tracing: Tracing, pub environment: Environment, #[cfg(feature = "db-sql")] + #[validate(nested)] pub database: Database, /// Allows providing custom config values. Any configs that aren't pre-defined above /// will be collected here. @@ -119,7 +125,7 @@ impl AppConfig { } } -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, Validate, Serialize, Deserialize)] #[serde(rename_all = "kebab-case")] pub struct App { pub name: String, @@ -128,21 +134,23 @@ pub struct App { pub shutdown_on_error: bool, } -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, Validate, Serialize, Deserialize)] #[serde(rename_all = "kebab-case")] pub struct Auth { + #[validate(nested)] pub jwt: Jwt, } -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, Validate, Serialize, Deserialize)] #[serde(rename_all = "kebab-case")] pub struct Jwt { pub secret: String, #[serde(default)] + #[validate(nested)] pub claims: JwtClaims, } -#[derive(Debug, Clone, Default, Serialize, Deserialize)] +#[derive(Debug, Clone, Validate, Default, Serialize, Deserialize)] #[serde(rename_all = "kebab-case")] pub struct JwtClaims { // Todo: Default to the server URL? @@ -151,7 +159,7 @@ pub struct JwtClaims { pub required_claims: Vec, } -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, Validate, Serialize, Deserialize)] #[serde(rename_all = "kebab-case")] pub struct Tracing { pub level: String, @@ -173,7 +181,7 @@ pub struct Tracing { #[cfg(feature = "db-sql")] #[serde_as] -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, Validate, Serialize, Deserialize)] #[serde(rename_all = "kebab-case")] pub struct Database { /// This can be overridden with an environment variable, e.g. `ROADSTER.DATABASE.URI=postgres://example:example@example:1234/example_app` diff --git a/src/config/service/http/initializer.rs b/src/config/service/http/initializer.rs index 09e985fd..e1d47510 100644 --- a/src/config/service/http/initializer.rs +++ b/src/config/service/http/initializer.rs @@ -4,11 +4,12 @@ use crate::config::app_config::CustomConfig; use crate::service::http::initializer::normalize_path::NormalizePathConfig; use serde_derive::{Deserialize, Serialize}; use std::collections::BTreeMap; +use validator::Validate; pub const PRIORITY_FIRST: i32 = -10_000; pub const PRIORITY_LAST: i32 = 10_000; -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, Validate, Serialize, Deserialize)] #[serde(rename_all = "kebab-case", default)] pub struct Initializer { pub default_enable: bool, diff --git a/src/config/service/http/middleware.rs b/src/config/service/http/middleware.rs index 53973318..7e1a6986 100644 --- a/src/config/service/http/middleware.rs +++ b/src/config/service/http/middleware.rs @@ -14,11 +14,12 @@ use crate::service::http::middleware::timeout::TimeoutConfig; use crate::service::http::middleware::tracing::TracingConfig; use serde_derive::{Deserialize, Serialize}; use std::collections::BTreeMap; +use validator::Validate; pub const PRIORITY_FIRST: i32 = -10_000; pub const PRIORITY_LAST: i32 = 10_000; -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, Validate, Serialize, Deserialize)] #[serde(rename_all = "kebab-case", default)] pub struct Middleware { pub default_enable: bool, diff --git a/src/config/service/http/mod.rs b/src/config/service/http/mod.rs index 679ff16d..2572abf0 100644 --- a/src/config/service/http/mod.rs +++ b/src/config/service/http/mod.rs @@ -3,25 +3,31 @@ use crate::app_context::AppContext; use crate::config::service::http::initializer::Initializer; use crate::config::service::http::middleware::Middleware; use crate::controller::http::build_path; +use crate::util::serde_util::default_true; use serde_derive::{Deserialize, Serialize}; +use validator::{Validate, ValidationError}; pub mod initializer; pub mod middleware; -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, Validate, Serialize, Deserialize)] #[serde(rename_all = "kebab-case")] pub struct HttpServiceConfig { #[serde(flatten)] + #[validate(nested)] pub address: Address, #[serde(default)] + #[validate(nested)] pub middleware: Middleware, #[serde(default)] + #[validate(nested)] pub initializer: Initializer, #[serde(default)] + #[validate(nested)] pub default_routes: DefaultRoutes, } -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, Validate, Serialize, Deserialize)] #[serde(rename_all = "kebab-case")] pub struct Address { pub host: String, @@ -34,36 +40,63 @@ impl Address { } } -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, Validate, Serialize, Deserialize)] #[serde(rename_all = "kebab-case")] +#[validate(schema(function = "validate_default_routes"))] pub struct DefaultRoutes { + #[serde(default = "default_true")] pub default_enable: bool, + #[serde(default = "DefaultRouteConfig::default_ping")] pub ping: DefaultRouteConfig, + #[serde(default = "DefaultRouteConfig::default_health")] pub health: DefaultRouteConfig, #[cfg(feature = "open-api")] + #[serde(default = "DefaultRouteConfig::default_api_schema")] pub api_schema: DefaultRouteConfig, #[cfg(feature = "open-api")] + #[serde(default = "DefaultRouteConfig::default_scalar")] pub scalar: DefaultRouteConfig, #[cfg(feature = "open-api")] + #[serde(default = "DefaultRouteConfig::default_redoc")] pub redoc: DefaultRouteConfig, } impl Default for DefaultRoutes { fn default() -> Self { Self { - default_enable: true, - ping: DefaultRouteConfig::new("_ping"), - health: DefaultRouteConfig::new("_health"), + default_enable: default_true(), + ping: DefaultRouteConfig::default_ping(), + health: DefaultRouteConfig::default_health(), #[cfg(feature = "open-api")] - api_schema: DefaultRouteConfig::new("_docs/api.json"), + api_schema: DefaultRouteConfig::default_api_schema(), #[cfg(feature = "open-api")] - scalar: DefaultRouteConfig::new("_docs"), + scalar: DefaultRouteConfig::default_scalar(), #[cfg(feature = "open-api")] - redoc: DefaultRouteConfig::new("_docs/redoc"), + redoc: DefaultRouteConfig::default_redoc(), } } } +fn validate_default_routes(default_routes: &DefaultRoutes) -> Result<(), ValidationError> { + let default_enable = default_routes.default_enable; + let api_schema_enabled = default_routes.api_schema.enable.unwrap_or(default_enable); + let scalar_enabled = default_routes.scalar.enable.unwrap_or(default_enable); + let redoc_enabled = default_routes.redoc.enable.unwrap_or(default_enable); + + if scalar_enabled && !api_schema_enabled { + return Err(ValidationError::new( + "The Open API schema route must be enabled in order to use the Scalar docs route.", + )); + } + if redoc_enabled && !api_schema_enabled { + return Err(ValidationError::new( + "The Open API schema route must be enabled in order to use the Redoc docs route.", + )); + } + + Ok(()) +} + #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(rename_all = "kebab-case")] pub struct DefaultRouteConfig { @@ -79,6 +112,29 @@ impl DefaultRouteConfig { } } + fn default_ping() -> Self { + DefaultRouteConfig::new("_ping") + } + + fn default_health() -> Self { + DefaultRouteConfig::new("_health") + } + + #[cfg(feature = "open-api")] + fn default_api_schema() -> Self { + DefaultRouteConfig::new("_docs/api.json") + } + + #[cfg(feature = "open-api")] + fn default_scalar() -> Self { + DefaultRouteConfig::new("_docs") + } + + #[cfg(feature = "open-api")] + fn default_redoc() -> Self { + DefaultRouteConfig::new("_docs/redoc") + } + pub fn enabled(&self, context: &AppContext) -> bool { self.enable.unwrap_or( context diff --git a/src/config/service/mod.rs b/src/config/service/mod.rs index d7ea5028..a104eca0 100644 --- a/src/config/service/mod.rs +++ b/src/config/service/mod.rs @@ -10,15 +10,18 @@ use crate::config::service::http::HttpServiceConfig; use crate::config::service::worker::sidekiq::SidekiqServiceConfig; use crate::util::serde_util::default_true; use serde_derive::{Deserialize, Serialize}; +use validator::Validate; -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, Validate, Serialize, Deserialize)] #[serde(rename_all = "kebab-case")] pub struct Service { #[serde(default = "default_true")] pub default_enable: bool, #[cfg(feature = "http")] + #[validate(nested)] pub http: ServiceConfig, #[cfg(feature = "sidekiq")] + #[validate(nested)] pub sidekiq: ServiceConfig, } @@ -39,11 +42,12 @@ impl CommonConfig { } } -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, Validate, Serialize, Deserialize)] #[serde(rename_all = "kebab-case")] -pub struct ServiceConfig { +pub struct ServiceConfig { #[serde(flatten, default)] pub common: CommonConfig, #[serde(flatten)] + #[validate(nested)] pub custom: T, } diff --git a/src/config/service/worker/sidekiq/mod.rs b/src/config/service/worker/sidekiq/mod.rs index 97a7a743..9f212126 100644 --- a/src/config/service/worker/sidekiq/mod.rs +++ b/src/config/service/worker/sidekiq/mod.rs @@ -2,10 +2,12 @@ use crate::service::worker::sidekiq::app_worker::AppWorkerConfig; use serde_derive::{Deserialize, Serialize}; use strum_macros::{EnumString, IntoStaticStr}; use url::Url; +use validator::Validate; -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, Validate, Serialize, Deserialize)] #[serde(rename_all = "kebab-case")] pub struct SidekiqServiceConfig { + #[validate(nested)] pub redis: Redis, /// The number of Sidekiq workers that can run at the same time. Adjust as needed based on @@ -25,11 +27,13 @@ pub struct SidekiqServiceConfig { pub queues: Vec, #[serde(default)] + #[validate(nested)] pub periodic: Periodic, /// The default app worker config. Values can be overridden on a per-worker basis by /// implementing the corresponding [crate::service::worker::sidekiq::app_worker::AppWorker] methods. #[serde(default, flatten)] + #[validate(nested)] pub worker_config: AppWorkerConfig, } @@ -39,7 +43,7 @@ impl SidekiqServiceConfig { } } -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, Validate, Serialize, Deserialize)] #[serde(rename_all = "kebab-case")] pub struct Periodic { pub stale_cleanup: StaleCleanUpBehavior, @@ -67,20 +71,22 @@ pub enum StaleCleanUpBehavior { AutoCleanStale, } -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, Validate, Serialize, Deserialize)] #[serde(rename_all = "kebab-case")] pub struct Redis { pub uri: Url, /// The configuration for the Redis connection pool used for enqueuing Sidekiq jobs in Redis. #[serde(default)] + #[validate(nested)] pub enqueue_pool: ConnectionPool, /// The configuration for the Redis connection pool used by [sidekiq::Processor] to fetch /// Sidekiq jobs from Redis. #[serde(default)] + #[validate(nested)] pub fetch_pool: ConnectionPool, } -#[derive(Debug, Default, Clone, Serialize, Deserialize)] +#[derive(Debug, Default, Validate, Clone, Serialize, Deserialize)] #[serde(default, rename_all = "kebab-case")] pub struct ConnectionPool { pub min_idle: Option, diff --git a/src/controller/http/docs.rs b/src/controller/http/docs.rs index 0d4dcc57..e18aed41 100644 --- a/src/controller/http/docs.rs +++ b/src/controller/http/docs.rs @@ -19,19 +19,6 @@ pub fn routes(parent: &str, context: &AppContext) -> ApiRouter