Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing Sidecar config which allows the whole [storage] section to be … #330

Merged
merged 1 commit into from
Jul 2, 2024
Merged

Fixing Sidecar config which allows the whole [storage] section to be … #330

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions sidecar/src/component.rs
View file
Open in desktop
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 Expand Up @@ -411,7 +411,7 @@ mod tests {
};
rpc_server.speculative_exec_server = Some(speculative_config);
SidecarConfig {
storage: Default::default(),
storage: Some(Default::default()),
admin_api_server: Some(Default::default()),
rest_api_server: Some(Default::default()),
sse_server: Some(Default::default()),
Expand Down
61 changes: 49 additions & 12 deletions sidecar/src/config.rs
View file
Open in desktop
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 `[storage.storage_folder]` 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,26 @@ 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 if no `[storage.storage_folder]` is 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 +192,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 +210,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 +227,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 +245,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
View file
Open in desktop
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
Loading