Verified Commit 7c8b6e90 authored by Michael Usachenko's avatar Michael Usachenko Committed by GitLab
Browse files

fix(config): fix handler config deserialization and drop unused env var source

parent 2501d505
Loading
Loading
Loading
Loading
+3 −4
Original line number Diff line number Diff line
@@ -58,14 +58,13 @@ engine:
      concurrency_group: sdlc
      max_attempts: 1  # re-dispatched every minute, no need to retry
      retry_interval_secs: 60
    code-push-event:
    code-indexing-task:
      concurrency_group: code
      max_attempts: 5  # event-driven, must retry to avoid data loss
      retry_interval_secs: 60
      events_stream_name: siphon_stream_main_db
    code-project-reconciliation:
    namespace-deletion:
      concurrency_group: code
      max_attempts: 1  # re-dispatched every 30 minutes, no need to retry
      max_attempts: 1  # re-dispatched daily, no need to retry

schedule:
  tasks: {}
+84 −9
Original line number Diff line number Diff line
@@ -79,14 +79,6 @@ impl AppConfig {
        let config = config::Config::builder()
            .add_source(config::File::with_name("config/default").required(false))
            .add_source(SecretFileSource::new(secret_dir))
            .add_source(
                config::Environment::with_prefix("GKG")
                    .prefix_separator("_")
                    .separator("__")
                    .list_separator(",")
                    .with_list_parse_key("health_check.services")
                    .try_parsing(true),
            )
            .build()
            .map_err(ConfigError::Config)?;

@@ -116,6 +108,89 @@ pub type SharedAppConfig = Arc<AppConfig>;
pub enum ConfigError {
    #[error("configuration error: {0}")]
    Config(#[from] config::ConfigError),
    #[error("GKG_GITLAB__JWT__VERIFYING_KEY is required")]
    #[error(
        "gitlab.jwt.verifying_key is required (set in config/default.yaml or mount at /etc/secrets/gitlab/jwt/verifying_key)"
    )]
    MissingJwtSecret,
}

#[cfg(test)]
mod tests {
    use crate::engine::EngineConfiguration;

    /// Verifies the kebab-case handler config keys in YAML actually
    /// deserialize into the correct Rust struct fields.
    #[test]
    fn handler_configs_deserialize_from_kebab_case_yaml() {
        let yaml = r#"
max_concurrent_workers: 16
concurrency_groups:
  sdlc: 12
  code: 4
handlers:
  global-handler:
    concurrency_group: sdlc
    max_attempts: 1
    retry_interval_secs: 60
  namespace-handler:
    concurrency_group: sdlc
    max_attempts: 1
    retry_interval_secs: 60
  code-indexing-task:
    concurrency_group: code
    max_attempts: 5
    retry_interval_secs: 60
  namespace-deletion:
    concurrency_group: code
    max_attempts: 1
"#;

        let engine: EngineConfiguration =
            serde_yaml::from_str(yaml).expect("engine config should deserialize");

        assert_eq!(
            engine
                .handlers
                .global_handler
                .engine
                .concurrency_group
                .as_deref(),
            Some("sdlc"),
        );
        assert_eq!(
            engine
                .handlers
                .namespace_handler
                .engine
                .concurrency_group
                .as_deref(),
            Some("sdlc"),
        );
        assert_eq!(
            engine
                .handlers
                .code_indexing_task
                .engine
                .concurrency_group
                .as_deref(),
            Some("code"),
        );
        assert_eq!(
            engine.handlers.code_indexing_task.engine.max_attempts,
            Some(5)
        );
        assert_eq!(
            engine
                .handlers
                .namespace_deletion
                .engine
                .concurrency_group
                .as_deref(),
            Some("code"),
        );
        assert_eq!(
            engine.handlers.namespace_deletion.engine.max_attempts,
            Some(1)
        );
    }
}
+2 −0
Original line number Diff line number Diff line
@@ -116,6 +116,7 @@ pub struct NamespaceDeletionHandlerConfig {

/// Typed per-handler configuration for all registered handlers.
#[derive(Debug, Clone, Default, Serialize, Deserialize)]
#[serde(rename_all = "kebab-case")]
pub struct HandlersConfiguration {
    #[serde(default)]
    pub global_handler: GlobalHandlerConfig,
@@ -227,6 +228,7 @@ impl Default for NamespaceDeletionSchedulerConfig {

/// Typed per-task configuration for all registered scheduled tasks.
#[derive(Debug, Clone, Default, Serialize, Deserialize)]
#[serde(rename_all = "kebab-case")]
pub struct ScheduledTasksConfiguration {
    #[serde(default)]
    pub global: GlobalDispatcherConfig,
+0 −70
Original line number Diff line number Diff line
@@ -25,73 +25,3 @@ pub async fn load_tls_config(tls: &TlsConfig) -> anyhow::Result<Option<ServerTls
        (None, None) => Ok(None),
    }
}

#[cfg(test)]
#[allow(unsafe_code)]
mod tests {
    use super::*;

    /// Reproduces the crash seen when deploying with GKG_ prefixed env vars.
    /// `try_parsing(true)` + `list_separator(",")` without `with_list_parse_key`
    /// wraps every value in a sequence, causing "invalid type: sequence, expected
    /// a string" errors for plain string fields.
    #[test]
    fn env_string_fields_not_parsed_as_lists() {
        let vars = [
            ("GKG_NATS__URL", "gkg-nats:4222"),
            ("GKG_GRAPH__URL", "http://clickhouse:8123"),
            ("GKG_GRAPH__DATABASE", "gkg"),
            ("GKG_GRAPH__USERNAME", "default"),
            ("GKG_GRAPH__PASSWORD", "supersecret"),
            ("GKG_METRICS__OTEL__ENDPOINT", "http://gkg-obs-alloy:4317"),
        ];

        // SAFETY: nextest runs each test in its own process, so env mutations are isolated
        unsafe {
            for (k, v) in &vars {
                std::env::set_var(k, v);
            }
        }

        let result = AppConfig::load();

        unsafe {
            for (k, _) in &vars {
                std::env::remove_var(k);
            }
        }

        let config = result.expect("AppConfig::load should not fail for plain string env vars");
        assert_eq!(config.nats.url, "gkg-nats:4222");
        assert_eq!(config.graph.password.as_deref(), Some("supersecret"));
        assert_eq!(config.metrics.otel.endpoint, "http://gkg-obs-alloy:4317");
    }

    #[test]
    fn health_check_services_parsed_as_list() {
        let vars = [(
            "GKG_HEALTH_CHECK__SERVICES",
            "siphon-consumer,nats,gkg-indexer",
        )];

        unsafe {
            for (k, v) in &vars {
                std::env::set_var(k, v);
            }
        }

        let result = AppConfig::load();

        unsafe {
            for (k, _) in &vars {
                std::env::remove_var(k);
            }
        }

        let config = result.expect("AppConfig::load should parse health_check.services as a list");
        assert_eq!(
            config.health_check.services,
            vec!["siphon-consumer", "nats", "gkg-indexer"],
        );
    }
}