Skip to content

Commit

Permalink
Fixing Sidecar config which allows the whole [storage] section to be …
Browse files Browse the repository at this point in the history
…not-present. [storage] can be omitted if sse server is either disabled or not provided.
  • Loading branch information
Jakub Zajkowski committed Jul 2, 2024
1 parent f47133e commit 433ea42
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 14 deletions.
2 changes: 1 addition & 1 deletion sidecar/src/component.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ impl Component for SseServerComponent {
&self,
config: &SidecarConfig,
) -> Result<Option<BoxFuture<'_, Result<ExitCode, ComponentError>>>, ComponentError> {
if let (maybe_database, Some(sse_server_config), storage_config) =
if let (maybe_database, Some(sse_server_config), Some(storage_config)) =
(&self.maybe_database, &config.sse_server, &config.storage)
{
if sse_server_config.enable_server {
Expand Down
62 changes: 50 additions & 12 deletions sidecar/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use thiserror::Error;
pub struct SidecarConfigTarget {
max_thread_count: Option<usize>,
max_blocking_thread_count: Option<usize>,
storage: StorageConfigSerdeTarget,
storage: Option<StorageConfigSerdeTarget>,
rest_api_server: Option<RestApiServerConfig>,
admin_api_server: Option<AdminApiServerConfig>,
sse_server: Option<SseEventServerConfig>,
Expand All @@ -25,7 +25,7 @@ pub struct SidecarConfig {
pub max_blocking_thread_count: Option<usize>,
pub sse_server: Option<SseEventServerConfig>,
pub rpc_server: Option<RpcServerConfig>,
pub storage: StorageConfig,
pub storage: Option<StorageConfig>,
pub rest_api_server: Option<RestApiServerConfig>,
pub admin_api_server: Option<AdminApiServerConfig>,
}
Expand All @@ -38,6 +38,9 @@ impl SidecarConfig {
let is_storage_enabled = self.is_storage_enabled();
let is_rest_api_server_enabled = self.is_rest_api_server_enabled();
let is_sse_storing_events = self.is_sse_storing_events();
if self.is_sse_server_enabled() && self.storage.is_none() {
bail!("Can't run SSE if no `sotrage_folder` storage is defined")
}
if !is_storage_enabled && is_sse_storing_events {
bail!("Can't run SSE with events persistence enabled without storage defined")
}
Expand All @@ -48,16 +51,19 @@ impl SidecarConfig {
if !is_sse_storing_events && is_rest_api_server_enabled {
bail!("Can't run Rest api server with SSE events persistence disabled")
}
let is_postgres_enabled = self.storage.is_postgres_enabled();
let is_sqlite_enabled = self.storage.is_sqlite_enabled();
let is_postgres_enabled = self.is_postgres_enabled();
let is_sqlite_enabled = self.is_sqlite_enabled();
if is_storage_enabled && is_postgres_enabled && is_sqlite_enabled {
bail!("Can't run with both postgres and sqlite enabled")
}
Ok(())
}

fn is_storage_enabled(&self) -> bool {
self.storage.is_enabled()
self.storage
.as_ref()
.map(|x| x.is_enabled())
.unwrap_or(false)
}

fn is_rpc_server_enabled(&self) -> bool {
Expand All @@ -77,6 +83,20 @@ impl SidecarConfig {
.is_event_persistence_disabled()
}

fn is_postgres_enabled(&self) -> bool {
self.storage
.as_ref()
.map(|x| x.is_postgres_enabled())
.unwrap_or(false)
}

fn is_sqlite_enabled(&self) -> bool {
self.storage
.as_ref()
.map(|x| x.is_sqlite_enabled())
.unwrap_or(false)
}

fn is_rest_api_server_enabled(&self) -> bool {
self.rest_api_server.is_some() && self.rest_api_server.as_ref().unwrap().enable_server
}
Expand All @@ -87,8 +107,10 @@ impl TryFrom<SidecarConfigTarget> for SidecarConfig {

fn try_from(value: SidecarConfigTarget) -> Result<Self, Self::Error> {
let sse_server_config = value.sse_server;
let storage_config_res: Result<StorageConfig, DatabaseConfigError> =
value.storage.try_into();
let storage_config_res: Result<Option<StorageConfig>, DatabaseConfigError> = value
.storage
.map(|target| target.try_into().map(Some))
.unwrap_or(Ok(None));
let storage_config = storage_config_res?;
let rpc_server_config_res: Option<Result<RpcServerConfig, FieldParseError>> =
value.rpc_server.map(|target| target.try_into());
Expand Down Expand Up @@ -135,11 +157,27 @@ impl From<DatabaseConfigError> for ConfigReadError {
mod tests {
use super::*;

#[test]
fn sidecar_config_should_fail_validation_when_sse_server_and_no_storage() {
let config = SidecarConfig {
sse_server: Some(SseEventServerConfig::default_no_persistence()),
storage: None,
..Default::default()
};

let res = config.validate();

assert!(res.is_err());
let error_message = res.err().unwrap().to_string();
assert!(error_message
.contains("Can't run SSE with events persistence enabled without storage defined"));
}

#[test]
fn sidecar_config_should_fail_validation_when_sse_server_and_no_defined_dbs() {
let config = SidecarConfig {
sse_server: Some(SseEventServerConfig::default()),
storage: StorageConfig::no_dbs(),
storage: Some(StorageConfig::no_dbs()),
..Default::default()
};

Expand All @@ -155,7 +193,7 @@ mod tests {
fn sidecar_config_should_fail_validation_when_sse_server_and_no_enabled_dbs() {
let config = SidecarConfig {
sse_server: Some(SseEventServerConfig::default()),
storage: StorageConfig::no_enabled_dbs(),
storage: Some(StorageConfig::no_enabled_dbs()),
..Default::default()
};

Expand All @@ -173,7 +211,7 @@ mod tests {
rpc_server: Some(RpcServerConfig::default()),
sse_server: None,
rest_api_server: Some(RestApiServerConfig::default()),
storage: StorageConfig::default(),
storage: Some(StorageConfig::default()),
..Default::default()
};

Expand All @@ -190,7 +228,7 @@ mod tests {
let config = SidecarConfig {
rpc_server: Some(RpcServerConfig::default()),
sse_server: None,
storage: StorageConfig::two_dbs(),
storage: Some(StorageConfig::two_dbs()),
..Default::default()
};

Expand All @@ -208,7 +246,7 @@ mod tests {
rpc_server: Some(RpcServerConfig::default()),
sse_server: Some(sse_server),
rest_api_server: Some(RestApiServerConfig::default()),
storage: StorageConfig::default(),
storage: Some(StorageConfig::default()),
..Default::default()
};

Expand Down
4 changes: 3 additions & 1 deletion sidecar/src/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ use std::process::ExitCode;
use tracing::info;

pub async fn run(config: SidecarConfig) -> Result<ExitCode, Error> {
let maybe_database = Some(&config.storage)
let maybe_database = config
.storage
.as_ref()
.filter(|storage_config| storage_config.is_enabled())
.map(|storage_config| LazyDatabaseWrapper::new(storage_config.clone()));
let mut components: Vec<Box<dyn Component>> = Vec::new();
Expand Down

0 comments on commit 433ea42

Please sign in to comment.