ParthibanRajasekaran commented on code in PR #4350: URL: https://github.com/apache/iggy/pull/4350#discussion_r4178812482
########## core/configs/src/configs_impl/env_listing.rs: ########## @@ -0,0 +1,59 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +/// Writes each name in `names`, sorted and deduplicated, to the provided writer — one +/// call, one error policy. A closed writer (e.g. `iggy-server --list-config-env-vars | head -1`) +/// is expected, not a failure: on `BrokenPipe` this stops writing and returns `Ok(())`. Any +/// other I/O error is real and is propagated so the caller exits non-zero. +pub fn print_env_var_names<I, S, W>(names: I, writer: &mut W) -> std::io::Result<()> Review Comment: Fixed in af7e26e. Added focused writer tests for BrokenPipe returning Ok and StorageFull being propagated as Err. ########## core/configs_derive/src/config_env.rs: ########## @@ -255,108 +261,67 @@ fn generate_struct_impl( let (mappings, nested_fields) = collect_mappings(&fields); let const_definitions = generate_const_definitions(&mappings, prefix_str); - let mapping_entries = generate_mapping_entries(&mappings); let builder_methods = generate_builder_methods(&mappings, prefix_str); let builder_name = format_ident!("{}EnvBuilder", struct_name); - let mappings_count = mappings.len(); - // Generate code to include nested type mappings - let nested_extends: Vec<TokenStream2> = nested_fields - .iter() - .map(|info| { - let ty = &info.element_type; - let field_env_segment = &info.field_env_segment; - let field_name = &info.field_name; - - if info.is_vec { - // For Vec fields, expand with array indices - let max_elements = info.max_elements; - quote! { - let nested_mappings = <#ty as configs::ConfigEnvMappings>::env_mappings(); - for i in 0..#max_elements { - for mapping in nested_mappings { - // Transform env_name by prepending FIELD_INDEX_ - let new_env_name: &'static str = Box::leak( - format!("{}_{}_{}", #field_env_segment, i, mapping.env_name).into_boxed_str() - ); - // Transform config_path by prepending field.index. - let new_config_path: &'static str = Box::leak( - format!("{}.{}.{}", #field_name, i, mapping.config_path).into_boxed_str() - ); - all_mappings.push(configs::EnvVarMapping { - env_name: new_env_name, - config_path: new_config_path, - is_secret: mapping.is_secret, - }); - } - } + let own_template_entries = mappings.iter().map(|mapping| { + let env_suffix = &mapping.env_suffix; + let config_path = &mapping.config_path; + let is_secret = mapping.is_secret; + quote! { + configs::EnvVarTemplate { + env_name: #env_suffix, + config_path: #config_path, + is_secret: #is_secret, + max_elements: &[], + } + } + }); + let nested_template_extends = nested_fields.iter().map(|info| { + let ty = &info.element_type; + let segment = &info.field_env_segment; + let field_name = &info.field_name; + if info.is_vec { Review Comment: Fixed in af7e26e. Vector and scalar nested fields now choose segments and limits at macro expansion time and share one emitted loop. ########## core/configs/src/server_config/server.rs: ########## @@ -47,6 +47,21 @@ pub use crate::common::server::{ TelemetryConfig, TelemetryLogsConfig, TelemetryTracesConfig, TelemetryTransport, }; +/// Vars used by sibling binaries (iggy CLI) or test/CI-only: never advertised +/// via `--list-config-env-vars`, but still non-config. Included in +/// `SERVER_PROCESS_ENV_VARS` to suppress "unknown env var" warnings during +/// config scanning. +pub const SERVER_SCAN_ONLY_ENV_VARS: &[&str] = &[ Review Comment: Fixed in af7e26e. Scan-only and advertised lists are private and disjoint, with server_process_env_vars providing the shared union to scanning, tests, and the harness. ########## core/configs/src/configs_impl/env_listing.rs: ########## @@ -0,0 +1,59 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +/// Writes each name in `names`, sorted and deduplicated, to the provided writer — one +/// call, one error policy. A closed writer (e.g. `iggy-server --list-config-env-vars | head -1`) +/// is expected, not a failure: on `BrokenPipe` this stops writing and returns `Ok(())`. Any +/// other I/O error is real and is propagated so the caller exits non-zero. +pub fn print_env_var_names<I, S, W>(names: I, writer: &mut W) -> std::io::Result<()> +where + I: IntoIterator<Item = S>, + S: Into<String>, + W: std::io::Write, +{ + let mut names: Vec<String> = names.into_iter().map(Into::into).collect(); + names.sort_unstable(); + names.dedup(); + + for name in names { + if let Err(err) = writeln!(writer, "{name}") { + return if err.kind() == std::io::ErrorKind::BrokenPipe { + Ok(()) + } else { + Err(err) + }; + } + } + Ok(()) +} + +/// Env vars `iggy-mcp --list-config-env-vars` advertises beyond the derived Review Comment: Fixed in af7e26e. Both rustdoc comments now attach directly to their corresponding runtime variable list constants. ########## core/configs/src/server_config/server.rs: ########## @@ -64,6 +79,17 @@ pub const SERVER_PROCESS_ENV_VARS: &[&str] = &[ "IGGY_PASSWORD", ]; +/// Vars safe to advertise to end users via `--list-config-env-vars` — every +/// entry in `SERVER_PROCESS_ENV_VARS` except the sibling-binary/test-only +/// ones in `SERVER_SCAN_ONLY_ENV_VARS`. Computed rather than retyped, so the Review Comment: Fixed in af7e26e. Advertised and scan-only variables are disjoint private lists, server_process_env_vars exposes their union, and an independent literal test pins every scan-only name. ########## core/configs/src/configs_impl/env_mapping.rs: ########## @@ -59,3 +78,38 @@ pub trait ConfigEnvMappings { .collect() } } + +impl EnvVarTemplate { + /// Expands this template into every concrete mapping it represents. + pub fn expand(&self) -> Vec<EnvVarMapping> { Review Comment: Fixed in af7e26e. EnvVarTemplate::expand is private, and expand_env_templates documents the intentional leaked names and paths plus its once-per-config-type usage. ########## core/integration/tests/config_env_listing/mod.rs: ########## @@ -0,0 +1,351 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +use assert_cmd::Command; +use std::collections::HashSet; +use std::time::Duration; + +const LIST_ENV_VARS_TIMEOUT: Duration = Duration::from_secs(5); + +/// Runs `binary --list-config-env-vars` (plus `extra_args`) with `env` applied, +/// isolated to `directory`. Shared by every test below so each only states +/// what differs: the binary, the env it sets, and any extra flags. +fn run_list_config_env_vars_in<I, K, V>( + binary: &str, + directory: &std::path::Path, + env: I, + extra_args: &[&str], +) -> std::process::Output +where + I: IntoIterator<Item = (K, V)>, + K: AsRef<std::ffi::OsStr>, + V: AsRef<std::ffi::OsStr>, +{ + let mut cmd = Command::cargo_bin(binary).expect("binary should be built"); + cmd.current_dir(directory) + .arg("--list-config-env-vars") + .timeout(LIST_ENV_VARS_TIMEOUT); + + for (key, value) in env { + cmd.env(key, value); + } + + for arg in extra_args { + cmd.arg(arg); + } + + cmd.output().expect("listing command should run") +} + +/// Convenience wrapper over [`run_list_config_env_vars_in`] for tests that +/// don't need to inspect the working directory afterward. +fn run_list_config_env_vars<I, K, V>( + binary: &str, + env: I, + extra_args: &[&str], +) -> std::process::Output +where + I: IntoIterator<Item = (K, V)>, + K: AsRef<std::ffi::OsStr>, + V: AsRef<std::ffi::OsStr>, +{ + let directory = tempfile::tempdir().expect("temporary directory"); + run_list_config_env_vars_in(binary, directory.path(), env, extra_args) +} + +/// Convenience wrapper over [`run_list_config_env_vars`] for tests that don't +/// need to set any environment variables. +fn run_list_config_env_vars_plain(binary: &str, extra_args: &[&str]) -> std::process::Output { + run_list_config_env_vars(binary, std::iter::empty::<(&str, &str)>(), extra_args) +} + +#[test] +fn config_env_listing_exits_before_startup_for_each_binary() { Review Comment: Fixed in af7e26e. The integration test now pins representative server, connectors, and MCP runtime variables by exact name. ########## core/integration/tests/config_env_listing/mod.rs: ########## @@ -0,0 +1,351 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +use assert_cmd::Command; +use std::collections::HashSet; +use std::time::Duration; + +const LIST_ENV_VARS_TIMEOUT: Duration = Duration::from_secs(5); + +/// Runs `binary --list-config-env-vars` (plus `extra_args`) with `env` applied, +/// isolated to `directory`. Shared by every test below so each only states +/// what differs: the binary, the env it sets, and any extra flags. +fn run_list_config_env_vars_in<I, K, V>( + binary: &str, + directory: &std::path::Path, + env: I, + extra_args: &[&str], +) -> std::process::Output +where + I: IntoIterator<Item = (K, V)>, + K: AsRef<std::ffi::OsStr>, + V: AsRef<std::ffi::OsStr>, +{ + let mut cmd = Command::cargo_bin(binary).expect("binary should be built"); + cmd.current_dir(directory) + .arg("--list-config-env-vars") + .timeout(LIST_ENV_VARS_TIMEOUT); + + for (key, value) in env { Review Comment: Fixed in af7e26e. The command builder now uses envs(env) and args(extra_args). ########## core/configs/src/configs_impl/typed_env_provider.rs: ########## @@ -625,19 +620,13 @@ mod tests { #[test] Review Comment: Fixed in af7e26e. The independent assertion loop is restored, so the documented invariant is checked in both debug and release profiles. ########## core/integration/tests/config_env_listing/mod.rs: ########## @@ -0,0 +1,351 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +use assert_cmd::Command; +use std::collections::HashSet; +use std::time::Duration; + +const LIST_ENV_VARS_TIMEOUT: Duration = Duration::from_secs(5); + +/// Runs `binary --list-config-env-vars` (plus `extra_args`) with `env` applied, +/// isolated to `directory`. Shared by every test below so each only states +/// what differs: the binary, the env it sets, and any extra flags. +fn run_list_config_env_vars_in<I, K, V>( + binary: &str, + directory: &std::path::Path, + env: I, + extra_args: &[&str], +) -> std::process::Output +where + I: IntoIterator<Item = (K, V)>, + K: AsRef<std::ffi::OsStr>, + V: AsRef<std::ffi::OsStr>, +{ + let mut cmd = Command::cargo_bin(binary).expect("binary should be built"); + cmd.current_dir(directory) + .arg("--list-config-env-vars") + .timeout(LIST_ENV_VARS_TIMEOUT); + + for (key, value) in env { + cmd.env(key, value); + } + + for arg in extra_args { + cmd.arg(arg); + } + + cmd.output().expect("listing command should run") +} + +/// Convenience wrapper over [`run_list_config_env_vars_in`] for tests that +/// don't need to inspect the working directory afterward. +fn run_list_config_env_vars<I, K, V>( + binary: &str, + env: I, + extra_args: &[&str], +) -> std::process::Output +where + I: IntoIterator<Item = (K, V)>, + K: AsRef<std::ffi::OsStr>, + V: AsRef<std::ffi::OsStr>, +{ + let directory = tempfile::tempdir().expect("temporary directory"); + run_list_config_env_vars_in(binary, directory.path(), env, extra_args) +} + +/// Convenience wrapper over [`run_list_config_env_vars`] for tests that don't +/// need to set any environment variables. +fn run_list_config_env_vars_plain(binary: &str, extra_args: &[&str]) -> std::process::Output { + run_list_config_env_vars(binary, std::iter::empty::<(&str, &str)>(), extra_args) +} + +#[test] +fn config_env_listing_exits_before_startup_for_each_binary() { + for (binary, config_env, dotenv_env, invalid_env) in [ + ( + "iggy-server", + "IGGY_CONFIG_PATH", + "IGGY_ENV_PATH", + "IGGY_HTTP_ADDRESS", + ), + ( + "iggy-connectors", + "IGGY_CONNECTORS_CONFIG_PATH", + "IGGY_CONNECTORS_ENV_PATH", + "IGGY_CONNECTORS_HTTP_ADDRESS", + ), + ( + "iggy-mcp", + "IGGY_MCP_CONFIG_PATH", + "IGGY_MCP_ENV_PATH", + "IGGY_MCP_HTTP_ADDRESS", + ), + ] { + let directory = tempfile::tempdir().expect("temporary directory"); + let config_path = directory.path().join("invalid-config.toml"); + let dotenv_path = directory.path().join("invalid.env"); + std::fs::write(&config_path, "not valid toml = [").expect("write invalid config"); + std::fs::write(&dotenv_path, "not a dotenv assignment").expect("write invalid dotenv"); + + let entries_before: HashSet<_> = std::fs::read_dir(directory.path()) + .expect("read temporary directory") + .map(|entry| entry.expect("directory entry").file_name()) + .collect(); + let output = run_list_config_env_vars_in( + binary, + directory.path(), + [ + (config_env, config_path.into_os_string()), + (dotenv_env, dotenv_path.into_os_string()), + (invalid_env, "invalid-value".into()), + ], + &[], + ); + + assert!( + output.status.success(), + "{binary} failed: {:?}\nstderr: {}", + output.status, + String::from_utf8_lossy(&output.stderr) + ); + assert!(output.stderr.is_empty(), "{binary} wrote to stderr"); + + let baseline = run_list_config_env_vars_plain(binary, &[]); + assert_eq!( + output.stdout, baseline.stdout, + "{binary} output changed with invalid config, dotenv or env values" + ); + let entries_after: HashSet<_> = std::fs::read_dir(directory.path()) + .expect("read temporary directory") + .map(|entry| entry.expect("directory entry").file_name()) + .collect(); + assert_eq!( + entries_after, entries_before, + "{binary} created startup side effects" + ); + + let stdout = String::from_utf8(output.stdout).expect("UTF-8 output"); + let names: Vec<_> = stdout.lines().collect(); + assert!(!names.is_empty(), "{binary} returned no variables"); + assert!( + names.windows(2).all(|pair| pair[0] < pair[1]), + "{binary} output must be sorted and deduplicated" + ); + assert!( + names.iter().all(|name| name.starts_with("IGGY_")), + "{binary} output contained non-IGGY_ prefixed names" + ); + } +} + +#[test] +fn config_env_listing_includes_vector_index_templates() { + // Verify that vector fields are correctly represented with <N> placeholder + let output = run_list_config_env_vars_plain("iggy-server", &[]); + + let stdout = String::from_utf8(output.stdout).expect("UTF-8 output"); + let names: HashSet<_> = stdout.lines().collect(); + + // Cluster nodes should have indexed templates + assert!( + names + .iter() + .any(|name| name.contains("IGGY_CLUSTER_NODES_<N>_")), Review Comment: Fixed in af7e26e. Removed the broad implied prefix checks and retained exact representative connector assertions. ########## core/integration/tests/config_env_listing/mod.rs: ########## @@ -0,0 +1,351 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +use assert_cmd::Command; +use std::collections::HashSet; +use std::time::Duration; + +const LIST_ENV_VARS_TIMEOUT: Duration = Duration::from_secs(5); + +/// Runs `binary --list-config-env-vars` (plus `extra_args`) with `env` applied, +/// isolated to `directory`. Shared by every test below so each only states +/// what differs: the binary, the env it sets, and any extra flags. +fn run_list_config_env_vars_in<I, K, V>( Review Comment: Fixed in af7e26e. Consolidated the wrappers into one execution helper returning lines plus a minimal plain helper. ########## core/connectors/runtime/src/main.rs: ########## @@ -67,7 +72,24 @@ static GLOBAL: MiMalloc = MiMalloc; #[derive(Parser, Debug)] #[command(author = "Apache Iggy", version)] -struct Args {} +struct Args { + #[arg( + long, + help = "Print supported configuration environment variables and exit", + long_help = r#"Print supported configuration environment variables and exit. + +Lists all supported IGGY_* environment variable names and templates, +sorted and deduplicated. Template syntax: +- <N> represents vector indices (0-255 for stream fields) +- <KEY> represents connector keys (uppercased from config). + Overrides via <KEY> require the local connectors provider. +- <FIELD> represents plugin configuration field names, excluding Review Comment: Fixed in af7e26e. Help now states that FIELD selects one lowercased top-level plugin_config key, preserves underscores, and cannot address nested keys. ########## core/configs_derive/src/config_env.rs: ########## @@ -255,108 +261,67 @@ fn generate_struct_impl( let (mappings, nested_fields) = collect_mappings(&fields); let const_definitions = generate_const_definitions(&mappings, prefix_str); - let mapping_entries = generate_mapping_entries(&mappings); let builder_methods = generate_builder_methods(&mappings, prefix_str); let builder_name = format_ident!("{}EnvBuilder", struct_name); - let mappings_count = mappings.len(); - // Generate code to include nested type mappings - let nested_extends: Vec<TokenStream2> = nested_fields - .iter() - .map(|info| { - let ty = &info.element_type; - let field_env_segment = &info.field_env_segment; - let field_name = &info.field_name; - - if info.is_vec { - // For Vec fields, expand with array indices - let max_elements = info.max_elements; - quote! { - let nested_mappings = <#ty as configs::ConfigEnvMappings>::env_mappings(); - for i in 0..#max_elements { - for mapping in nested_mappings { - // Transform env_name by prepending FIELD_INDEX_ - let new_env_name: &'static str = Box::leak( - format!("{}_{}_{}", #field_env_segment, i, mapping.env_name).into_boxed_str() - ); - // Transform config_path by prepending field.index. - let new_config_path: &'static str = Box::leak( - format!("{}.{}.{}", #field_name, i, mapping.config_path).into_boxed_str() - ); - all_mappings.push(configs::EnvVarMapping { - env_name: new_env_name, - config_path: new_config_path, - is_secret: mapping.is_secret, - }); - } - } + let own_template_entries = mappings.iter().map(|mapping| { + let env_suffix = &mapping.env_suffix; + let config_path = &mapping.config_path; + let is_secret = mapping.is_secret; + quote! { + configs::EnvVarTemplate { + env_name: #env_suffix, + config_path: #config_path, + is_secret: #is_secret, + max_elements: &[], + } + } + }); + let nested_template_extends = nested_fields.iter().map(|info| { + let ty = &info.element_type; + let segment = &info.field_env_segment; + let field_name = &info.field_name; + if info.is_vec { + let max_elements = info.max_elements; + quote! { + for template in <#ty as configs::ConfigEnvMappings>::env_templates() { + let env_name = Box::leak(format!("{}_<N>_{}", #segment, template.env_name).into_boxed_str()); + let config_path = Box::leak(format!("{}.<N>.{}", #field_name, template.config_path).into_boxed_str()); + let limits = Box::leak(std::iter::once(#max_elements) + .chain(template.max_elements.iter().copied()) + .collect::<Vec<_>>() + .into_boxed_slice()); + all_templates.push(configs::EnvVarTemplate { + env_name, + config_path, + is_secret: template.is_secret, + max_elements: limits, + }); } - } else { - // For non-Vec nested types, transform by prepending field name - quote! { - let nested_mappings = <#ty as configs::ConfigEnvMappings>::env_mappings(); - for mapping in nested_mappings { - // Transform env_name by prepending FIELD_ - let new_env_name: &'static str = Box::leak( - format!("{}_{}", #field_env_segment, mapping.env_name).into_boxed_str() - ); - // Transform config_path by prepending field. - let new_config_path: &'static str = Box::leak( - format!("{}.{}", #field_name, mapping.config_path).into_boxed_str() - ); - all_mappings.push(configs::EnvVarMapping { - env_name: new_env_name, - config_path: new_config_path, - is_secret: mapping.is_secret, - }); - } + } + } else { + quote! { + for template in <#ty as configs::ConfigEnvMappings>::env_templates() { + let env_name = Box::leak(format!("{}_{}", #segment, template.env_name).into_boxed_str()); + let config_path = Box::leak(format!("{}.{}", #field_name, template.config_path).into_boxed_str()); + all_templates.push(configs::EnvVarTemplate { + env_name, + config_path, + is_secret: template.is_secret, + max_elements: template.max_elements, + }); } } - }) - .collect(); - - let prefix_application = if has_prefix { - quote! { - let all_mappings: Vec<configs::EnvVarMapping> = all_mappings - .into_iter() - .map(|m| configs::EnvVarMapping { - env_name: Box::leak(format!("{}{}", #prefix_str, m.env_name).into_boxed_str()), - config_path: m.config_path, - is_secret: m.is_secret, - }) - .collect(); } - } else { - quote! {} - }; - - // If there are nested types, we need to use OnceLock to combine mappings - let env_mappings_impl = if nested_fields.is_empty() && !has_prefix { - // Simple case: no nested fields, no prefix transformation needed + }); + let template_prefix_application = if has_prefix { Review Comment: Fixed in af7e26e. Prefixes are baked into generated literals at macro expansion time; the runtime prefixing pass and its extra leaks are gone. ########## core/connectors/runtime/src/main.rs: ########## @@ -117,13 +139,52 @@ fn print_ascii_art(text: &str) { println!("{}", figure.unwrap()); } -#[tokio::main] -async fn main() -> Result<(), RuntimeError> { +fn main() -> Result<(), RuntimeError> { + let args = Args::parse(); + if args.list_config_env_vars { + print_config_env_vars()?; Review Comment: Fixed in af7e26e. Connector listing writes and Tokio runtime creation now map to separate contextual RuntimeError variants. ########## core/integration/tests/config_env_listing/mod.rs: ########## @@ -0,0 +1,351 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +use assert_cmd::Command; +use std::collections::HashSet; +use std::time::Duration; + +const LIST_ENV_VARS_TIMEOUT: Duration = Duration::from_secs(5); + +/// Runs `binary --list-config-env-vars` (plus `extra_args`) with `env` applied, +/// isolated to `directory`. Shared by every test below so each only states +/// what differs: the binary, the env it sets, and any extra flags. +fn run_list_config_env_vars_in<I, K, V>( + binary: &str, + directory: &std::path::Path, + env: I, + extra_args: &[&str], +) -> std::process::Output +where + I: IntoIterator<Item = (K, V)>, + K: AsRef<std::ffi::OsStr>, + V: AsRef<std::ffi::OsStr>, +{ + let mut cmd = Command::cargo_bin(binary).expect("binary should be built"); + cmd.current_dir(directory) + .arg("--list-config-env-vars") + .timeout(LIST_ENV_VARS_TIMEOUT); + + for (key, value) in env { + cmd.env(key, value); + } + + for arg in extra_args { + cmd.arg(arg); + } + + cmd.output().expect("listing command should run") +} + +/// Convenience wrapper over [`run_list_config_env_vars_in`] for tests that +/// don't need to inspect the working directory afterward. +fn run_list_config_env_vars<I, K, V>( + binary: &str, + env: I, + extra_args: &[&str], +) -> std::process::Output +where + I: IntoIterator<Item = (K, V)>, + K: AsRef<std::ffi::OsStr>, + V: AsRef<std::ffi::OsStr>, +{ + let directory = tempfile::tempdir().expect("temporary directory"); + run_list_config_env_vars_in(binary, directory.path(), env, extra_args) +} + +/// Convenience wrapper over [`run_list_config_env_vars`] for tests that don't +/// need to set any environment variables. +fn run_list_config_env_vars_plain(binary: &str, extra_args: &[&str]) -> std::process::Output { + run_list_config_env_vars(binary, std::iter::empty::<(&str, &str)>(), extra_args) +} + +#[test] +fn config_env_listing_exits_before_startup_for_each_binary() { + for (binary, config_env, dotenv_env, invalid_env) in [ + ( + "iggy-server", + "IGGY_CONFIG_PATH", + "IGGY_ENV_PATH", + "IGGY_HTTP_ADDRESS", + ), + ( + "iggy-connectors", + "IGGY_CONNECTORS_CONFIG_PATH", + "IGGY_CONNECTORS_ENV_PATH", + "IGGY_CONNECTORS_HTTP_ADDRESS", + ), + ( + "iggy-mcp", + "IGGY_MCP_CONFIG_PATH", + "IGGY_MCP_ENV_PATH", + "IGGY_MCP_HTTP_ADDRESS", + ), + ] { + let directory = tempfile::tempdir().expect("temporary directory"); + let config_path = directory.path().join("invalid-config.toml"); + let dotenv_path = directory.path().join("invalid.env"); + std::fs::write(&config_path, "not valid toml = [").expect("write invalid config"); + std::fs::write(&dotenv_path, "not a dotenv assignment").expect("write invalid dotenv"); + + let entries_before: HashSet<_> = std::fs::read_dir(directory.path()) + .expect("read temporary directory") + .map(|entry| entry.expect("directory entry").file_name()) + .collect(); + let output = run_list_config_env_vars_in( + binary, + directory.path(), + [ + (config_env, config_path.into_os_string()), + (dotenv_env, dotenv_path.into_os_string()), + (invalid_env, "invalid-value".into()), + ], + &[], + ); + + assert!( + output.status.success(), + "{binary} failed: {:?}\nstderr: {}", + output.status, + String::from_utf8_lossy(&output.stderr) + ); + assert!(output.stderr.is_empty(), "{binary} wrote to stderr"); + + let baseline = run_list_config_env_vars_plain(binary, &[]); + assert_eq!( + output.stdout, baseline.stdout, + "{binary} output changed with invalid config, dotenv or env values" + ); + let entries_after: HashSet<_> = std::fs::read_dir(directory.path()) + .expect("read temporary directory") + .map(|entry| entry.expect("directory entry").file_name()) + .collect(); + assert_eq!( + entries_after, entries_before, + "{binary} created startup side effects" + ); + + let stdout = String::from_utf8(output.stdout).expect("UTF-8 output"); + let names: Vec<_> = stdout.lines().collect(); + assert!(!names.is_empty(), "{binary} returned no variables"); + assert!( + names.windows(2).all(|pair| pair[0] < pair[1]), + "{binary} output must be sorted and deduplicated" + ); + assert!( + names.iter().all(|name| name.starts_with("IGGY_")), + "{binary} output contained non-IGGY_ prefixed names" + ); + } +} + +#[test] +fn config_env_listing_includes_vector_index_templates() { + // Verify that vector fields are correctly represented with <N> placeholder + let output = run_list_config_env_vars_plain("iggy-server", &[]); + + let stdout = String::from_utf8(output.stdout).expect("UTF-8 output"); + let names: HashSet<_> = stdout.lines().collect(); + + // Cluster nodes should have indexed templates + assert!( + names + .iter() + .any(|name| name.contains("IGGY_CLUSTER_NODES_<N>_")), + "server should list cluster node index templates" + ); + + // Verify nested vector expansion (nested <N> placeholders) + assert!( + names + .iter() + .any(|name| name.contains("IGGY_CLUSTER_NODES_<N>_ADVERTISED_ADDRESSES_<N>_")), + "server should list nested vector templates" + ); +} + +#[test] +fn config_env_listing_includes_connector_templates() { + // Verify that connector SINK/SOURCE templates are correctly formatted + let output = run_list_config_env_vars_plain("iggy-connectors", &[]); + + let stdout = String::from_utf8(output.stdout).expect("UTF-8 output"); + let names: HashSet<_> = stdout.lines().collect(); + + // Both SINK and SOURCE templates should be present + assert!( + names + .iter() + .any(|name| name.contains("IGGY_CONNECTORS_SINK_<KEY>_")), + "connectors should list SINK templates with <KEY> placeholder" + ); + assert!( + names + .iter() + .any(|name| name.contains("IGGY_CONNECTORS_SOURCE_<KEY>_")), + "connectors should list SOURCE templates with <KEY> placeholder" + ); + + // Plugin config templates should be present + assert!( + names + .iter() + .any(|name| name.contains("IGGY_CONNECTORS_SINK_<KEY>_PLUGIN_CONFIG_<FIELD>")), + "connectors should list SINK plugin config templates" + ); + assert!( + names + .iter() + .any(|name| name.contains("IGGY_CONNECTORS_SOURCE_<KEY>_PLUGIN_CONFIG_<FIELD>")), + "connectors should list SOURCE plugin config templates" + ); + assert!( + names.contains("IGGY_CONNECTORS_SINK_<KEY>_PLUGIN_CONFIG_FORMAT"), + "connectors should list the typed SINK plugin config format" + ); + assert!( + names.contains("IGGY_CONNECTORS_SOURCE_<KEY>_PLUGIN_CONFIG_FORMAT"), + "connectors should list the typed SOURCE plugin config format" + ); + + // Verify SINK and SOURCE ENABLED fields are present + assert!( + names + .iter() + .any(|name| name.contains("IGGY_CONNECTORS_SINK_<KEY>_ENABLED")), + "connectors should list SINK_<KEY>_ENABLED" + ); + assert!( + names + .iter() + .any(|name| name.contains("IGGY_CONNECTORS_SOURCE_<KEY>_ENABLED")), + "connectors should list SOURCE_<KEY>_ENABLED" + ); + + assert_eq!( + stdout + .lines() + .filter(|name| *name == "IGGY_CONNECTORS_CONNECTORS_CONFIG_TYPE") + .count(), + 1, + "enum variants should produce one deduplicated tag name" + ); +} + +#[test] +fn config_env_listing_excludes_other_processes_variables() { + let server = run_list_config_env_vars_plain("iggy-server", &[]); + let server = String::from_utf8(server.stdout).expect("UTF-8 output"); + let server_names: HashSet<_> = server.lines().collect(); + for excluded in [ + "IGGY_CI_BUILD", + "IGGY_HOME", + "IGGY_PASSWORD", + "IGGY_TEST_CLEANUP_DISABLED", + "IGGY_TEST_CLUSTER_NODES", + "IGGY_TEST_VERBOSE", + "IGGY_USERNAME", + ] { + assert!(!server_names.contains(excluded), "server listed {excluded}"); + } + assert!( + server_names + .iter() + .all(|name| !name.starts_with("IGGY_CONNECTORS_") + && !name.starts_with("IGGY_KAFKA_") + && !name.starts_with("IGGY_MCP_")), + "server listed a sibling binary's variables" + ); + + for (binary, prefix) in [ + ("iggy-connectors", "IGGY_CONNECTORS_"), + ("iggy-mcp", "IGGY_MCP_"), + ] { + let output = run_list_config_env_vars_plain(binary, &[]); + let stdout = String::from_utf8(output.stdout).expect("UTF-8 output"); + assert!( + stdout + .lines() + .all(|name| name == "IGGY_DISPLAY_CONFIG" || name.starts_with(prefix)), + "{binary} listed a variable outside {prefix}" + ); + } +} + +#[test] +fn config_env_listing_exits_before_runtime_with_invalid_env_values() { Review Comment: Fixed in af7e26e. Removed the redundant invalid-env-only test and centralized the stderr assertion in the helper. ########## core/configs/src/configs_impl/env_listing.rs: ########## @@ -0,0 +1,59 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +/// Writes each name in `names`, sorted and deduplicated, to the provided writer — one +/// call, one error policy. A closed writer (e.g. `iggy-server --list-config-env-vars | head -1`) +/// is expected, not a failure: on `BrokenPipe` this stops writing and returns `Ok(())`. Any +/// other I/O error is real and is propagated so the caller exits non-zero. +pub fn print_env_var_names<I, S, W>(names: I, writer: &mut W) -> std::io::Result<()> +where + I: IntoIterator<Item = S>, + S: Into<String>, + W: std::io::Write, +{ + let mut names: Vec<String> = names.into_iter().map(Into::into).collect(); + names.sort_unstable(); + names.dedup(); + + for name in names { + if let Err(err) = writeln!(writer, "{name}") { + return if err.kind() == std::io::ErrorKind::BrokenPipe { + Ok(()) + } else { + Err(err) + }; + } + } + Ok(()) +} + +/// Env vars `iggy-mcp --list-config-env-vars` advertises beyond the derived +/// `McpServerConfig` templates. +pub const MCP_CONFIG_PATH_ENV: &str = "IGGY_MCP_CONFIG_PATH"; +pub const MCP_ENV_PATH_ENV: &str = "IGGY_MCP_ENV_PATH"; +pub const MCP_RUNTIME_ENV_VARS: &[&str] = + &["IGGY_DISPLAY_CONFIG", MCP_CONFIG_PATH_ENV, MCP_ENV_PATH_ENV]; Review Comment: Fixed in af7e26e. DISPLAY_CONFIG_ENV is now pub(crate) and both MCP and connectors runtime lists reuse it. ########## core/integration/tests/config_env_listing/mod.rs: ########## @@ -0,0 +1,351 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +use assert_cmd::Command; +use std::collections::HashSet; +use std::time::Duration; + +const LIST_ENV_VARS_TIMEOUT: Duration = Duration::from_secs(5); + +/// Runs `binary --list-config-env-vars` (plus `extra_args`) with `env` applied, +/// isolated to `directory`. Shared by every test below so each only states +/// what differs: the binary, the env it sets, and any extra flags. +fn run_list_config_env_vars_in<I, K, V>( + binary: &str, + directory: &std::path::Path, + env: I, + extra_args: &[&str], +) -> std::process::Output +where + I: IntoIterator<Item = (K, V)>, + K: AsRef<std::ffi::OsStr>, + V: AsRef<std::ffi::OsStr>, +{ + let mut cmd = Command::cargo_bin(binary).expect("binary should be built"); + cmd.current_dir(directory) + .arg("--list-config-env-vars") + .timeout(LIST_ENV_VARS_TIMEOUT); + + for (key, value) in env { + cmd.env(key, value); + } + + for arg in extra_args { + cmd.arg(arg); + } + + cmd.output().expect("listing command should run") +} + +/// Convenience wrapper over [`run_list_config_env_vars_in`] for tests that +/// don't need to inspect the working directory afterward. +fn run_list_config_env_vars<I, K, V>( + binary: &str, + env: I, + extra_args: &[&str], +) -> std::process::Output +where + I: IntoIterator<Item = (K, V)>, + K: AsRef<std::ffi::OsStr>, + V: AsRef<std::ffi::OsStr>, +{ + let directory = tempfile::tempdir().expect("temporary directory"); + run_list_config_env_vars_in(binary, directory.path(), env, extra_args) +} + +/// Convenience wrapper over [`run_list_config_env_vars`] for tests that don't +/// need to set any environment variables. +fn run_list_config_env_vars_plain(binary: &str, extra_args: &[&str]) -> std::process::Output { + run_list_config_env_vars(binary, std::iter::empty::<(&str, &str)>(), extra_args) +} + +#[test] +fn config_env_listing_exits_before_startup_for_each_binary() { + for (binary, config_env, dotenv_env, invalid_env) in [ + ( + "iggy-server", + "IGGY_CONFIG_PATH", + "IGGY_ENV_PATH", + "IGGY_HTTP_ADDRESS", + ), + ( + "iggy-connectors", + "IGGY_CONNECTORS_CONFIG_PATH", + "IGGY_CONNECTORS_ENV_PATH", + "IGGY_CONNECTORS_HTTP_ADDRESS", + ), + ( + "iggy-mcp", + "IGGY_MCP_CONFIG_PATH", + "IGGY_MCP_ENV_PATH", + "IGGY_MCP_HTTP_ADDRESS", + ), + ] { + let directory = tempfile::tempdir().expect("temporary directory"); + let config_path = directory.path().join("invalid-config.toml"); + let dotenv_path = directory.path().join("invalid.env"); + std::fs::write(&config_path, "not valid toml = [").expect("write invalid config"); + std::fs::write(&dotenv_path, "not a dotenv assignment").expect("write invalid dotenv"); + + let entries_before: HashSet<_> = std::fs::read_dir(directory.path()) + .expect("read temporary directory") + .map(|entry| entry.expect("directory entry").file_name()) + .collect(); + let output = run_list_config_env_vars_in( + binary, + directory.path(), + [ + (config_env, config_path.into_os_string()), + (dotenv_env, dotenv_path.into_os_string()), + (invalid_env, "invalid-value".into()), + ], + &[], + ); + + assert!( + output.status.success(), + "{binary} failed: {:?}\nstderr: {}", + output.status, + String::from_utf8_lossy(&output.stderr) + ); + assert!(output.stderr.is_empty(), "{binary} wrote to stderr"); + + let baseline = run_list_config_env_vars_plain(binary, &[]); + assert_eq!( + output.stdout, baseline.stdout, + "{binary} output changed with invalid config, dotenv or env values" + ); + let entries_after: HashSet<_> = std::fs::read_dir(directory.path()) + .expect("read temporary directory") + .map(|entry| entry.expect("directory entry").file_name()) + .collect(); + assert_eq!( + entries_after, entries_before, + "{binary} created startup side effects" + ); + + let stdout = String::from_utf8(output.stdout).expect("UTF-8 output"); + let names: Vec<_> = stdout.lines().collect(); + assert!(!names.is_empty(), "{binary} returned no variables"); + assert!( + names.windows(2).all(|pair| pair[0] < pair[1]), + "{binary} output must be sorted and deduplicated" + ); + assert!( + names.iter().all(|name| name.starts_with("IGGY_")), + "{binary} output contained non-IGGY_ prefixed names" + ); + } +} + +#[test] +fn config_env_listing_includes_vector_index_templates() { + // Verify that vector fields are correctly represented with <N> placeholder + let output = run_list_config_env_vars_plain("iggy-server", &[]); + + let stdout = String::from_utf8(output.stdout).expect("UTF-8 output"); + let names: HashSet<_> = stdout.lines().collect(); + + // Cluster nodes should have indexed templates + assert!( + names + .iter() + .any(|name| name.contains("IGGY_CLUSTER_NODES_<N>_")), + "server should list cluster node index templates" + ); + + // Verify nested vector expansion (nested <N> placeholders) + assert!( + names + .iter() + .any(|name| name.contains("IGGY_CLUSTER_NODES_<N>_ADVERTISED_ADDRESSES_<N>_")), + "server should list nested vector templates" + ); +} + +#[test] +fn config_env_listing_includes_connector_templates() { + // Verify that connector SINK/SOURCE templates are correctly formatted + let output = run_list_config_env_vars_plain("iggy-connectors", &[]); + + let stdout = String::from_utf8(output.stdout).expect("UTF-8 output"); + let names: HashSet<_> = stdout.lines().collect(); + + // Both SINK and SOURCE templates should be present + assert!( + names + .iter() + .any(|name| name.contains("IGGY_CONNECTORS_SINK_<KEY>_")), + "connectors should list SINK templates with <KEY> placeholder" + ); + assert!( + names + .iter() + .any(|name| name.contains("IGGY_CONNECTORS_SOURCE_<KEY>_")), + "connectors should list SOURCE templates with <KEY> placeholder" + ); + + // Plugin config templates should be present + assert!( + names + .iter() + .any(|name| name.contains("IGGY_CONNECTORS_SINK_<KEY>_PLUGIN_CONFIG_<FIELD>")), + "connectors should list SINK plugin config templates" + ); + assert!( + names + .iter() + .any(|name| name.contains("IGGY_CONNECTORS_SOURCE_<KEY>_PLUGIN_CONFIG_<FIELD>")), + "connectors should list SOURCE plugin config templates" + ); + assert!( + names.contains("IGGY_CONNECTORS_SINK_<KEY>_PLUGIN_CONFIG_FORMAT"), + "connectors should list the typed SINK plugin config format" + ); + assert!( + names.contains("IGGY_CONNECTORS_SOURCE_<KEY>_PLUGIN_CONFIG_FORMAT"), + "connectors should list the typed SOURCE plugin config format" + ); + + // Verify SINK and SOURCE ENABLED fields are present + assert!( + names + .iter() + .any(|name| name.contains("IGGY_CONNECTORS_SINK_<KEY>_ENABLED")), + "connectors should list SINK_<KEY>_ENABLED" + ); + assert!( + names + .iter() + .any(|name| name.contains("IGGY_CONNECTORS_SOURCE_<KEY>_ENABLED")), + "connectors should list SOURCE_<KEY>_ENABLED" + ); + + assert_eq!( + stdout + .lines() + .filter(|name| *name == "IGGY_CONNECTORS_CONNECTORS_CONFIG_TYPE") + .count(), + 1, + "enum variants should produce one deduplicated tag name" Review Comment: Fixed in af7e26e. Added a direct print_env_var_names unit test with input b, a, b and exact output a, b. ########## core/configs/src/configs_impl/typed_env_provider.rs: ########## @@ -625,19 +620,13 @@ mod tests { #[test] #[serial_test::serial] fn ignored_env_vars_are_skipped_by_the_unknown_variable_scan() { - for name in [ - "IGGY_CONNECTORS_CONFIG_PATH", - "IGGY_CONNECTORS_ENV_PATH", - "IGGY_MCP_CONFIG_PATH", - "IGGY_MCP_ENV_PATH", - ] { - assert!( - IGNORED_ENV_VARS.contains(&name), - "{name} is read by a sibling binary before its config loads, so the scan must skip it" - ); - } + let runtime_env_vars = super::super::CONNECTORS_RUNTIME_ENV_VARS Review Comment: Fixed in af7e26e. The test now pins the four literal config and env path names, asserts each is classified as a runtime variable, and then verifies the unknown-variable scan ignores it. ########## core/connectors/runtime/src/configs/connectors/local_provider.rs: ########## @@ -395,6 +398,20 @@ impl BaseConnectorConfig { } } +/// Builds the env-var prefix a connector's config and plugin-config overrides +/// are matched against: `IGGY_CONNECTORS_{TYPE}_{KEY}_`. `connector_type` and +/// `key` are expected uppercased, as `BaseConnectorConfig`'s callers already +/// do. Shared by the runtime's actual override lookup here and by +/// `--list-config-env-vars`'s listing in `main.rs` (passing the literal +/// `<KEY>` as the key), so the two can't drift apart on the prefix the +/// server actually reads. Review Comment: Fixed in af7e26e. The comment now correctly refers to the connectors runtime. ########## core/configs/src/server_config/cluster.rs: ########## @@ -1569,6 +1569,16 @@ mod tests { "selector env expansion must stop at max_elements = 16" ); } + + #[test] + fn advertised_addresses_env_template_keeps_each_vector_limit() { Review Comment: Fixed in af7e26e. The cluster unit test now pins NODES_3_ADVERTISED_ADDRESSES_7_CLIENT_CIDR to nodes.3.advertised_addresses.7.client_cidr. ########## core/integration/tests/config_env_listing/mod.rs: ########## @@ -0,0 +1,351 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +use assert_cmd::Command; +use std::collections::HashSet; +use std::time::Duration; + +const LIST_ENV_VARS_TIMEOUT: Duration = Duration::from_secs(5); + +/// Runs `binary --list-config-env-vars` (plus `extra_args`) with `env` applied, +/// isolated to `directory`. Shared by every test below so each only states +/// what differs: the binary, the env it sets, and any extra flags. +fn run_list_config_env_vars_in<I, K, V>( + binary: &str, + directory: &std::path::Path, + env: I, + extra_args: &[&str], +) -> std::process::Output +where + I: IntoIterator<Item = (K, V)>, + K: AsRef<std::ffi::OsStr>, + V: AsRef<std::ffi::OsStr>, +{ + let mut cmd = Command::cargo_bin(binary).expect("binary should be built"); + cmd.current_dir(directory) + .arg("--list-config-env-vars") + .timeout(LIST_ENV_VARS_TIMEOUT); + + for (key, value) in env { + cmd.env(key, value); + } + + for arg in extra_args { + cmd.arg(arg); + } + + cmd.output().expect("listing command should run") +} + +/// Convenience wrapper over [`run_list_config_env_vars_in`] for tests that +/// don't need to inspect the working directory afterward. +fn run_list_config_env_vars<I, K, V>( + binary: &str, + env: I, + extra_args: &[&str], +) -> std::process::Output +where + I: IntoIterator<Item = (K, V)>, + K: AsRef<std::ffi::OsStr>, + V: AsRef<std::ffi::OsStr>, +{ + let directory = tempfile::tempdir().expect("temporary directory"); + run_list_config_env_vars_in(binary, directory.path(), env, extra_args) +} + +/// Convenience wrapper over [`run_list_config_env_vars`] for tests that don't +/// need to set any environment variables. +fn run_list_config_env_vars_plain(binary: &str, extra_args: &[&str]) -> std::process::Output { + run_list_config_env_vars(binary, std::iter::empty::<(&str, &str)>(), extra_args) +} + +#[test] +fn config_env_listing_exits_before_startup_for_each_binary() { + for (binary, config_env, dotenv_env, invalid_env) in [ + ( + "iggy-server", + "IGGY_CONFIG_PATH", + "IGGY_ENV_PATH", + "IGGY_HTTP_ADDRESS", + ), + ( + "iggy-connectors", + "IGGY_CONNECTORS_CONFIG_PATH", + "IGGY_CONNECTORS_ENV_PATH", + "IGGY_CONNECTORS_HTTP_ADDRESS", + ), + ( + "iggy-mcp", + "IGGY_MCP_CONFIG_PATH", + "IGGY_MCP_ENV_PATH", + "IGGY_MCP_HTTP_ADDRESS", + ), + ] { + let directory = tempfile::tempdir().expect("temporary directory"); + let config_path = directory.path().join("invalid-config.toml"); + let dotenv_path = directory.path().join("invalid.env"); + std::fs::write(&config_path, "not valid toml = [").expect("write invalid config"); + std::fs::write(&dotenv_path, "not a dotenv assignment").expect("write invalid dotenv"); + + let entries_before: HashSet<_> = std::fs::read_dir(directory.path()) + .expect("read temporary directory") + .map(|entry| entry.expect("directory entry").file_name()) + .collect(); + let output = run_list_config_env_vars_in( + binary, + directory.path(), + [ + (config_env, config_path.into_os_string()), + (dotenv_env, dotenv_path.into_os_string()), + (invalid_env, "invalid-value".into()), + ], + &[], + ); + + assert!( + output.status.success(), + "{binary} failed: {:?}\nstderr: {}", + output.status, + String::from_utf8_lossy(&output.stderr) + ); + assert!(output.stderr.is_empty(), "{binary} wrote to stderr"); + + let baseline = run_list_config_env_vars_plain(binary, &[]); + assert_eq!( + output.stdout, baseline.stdout, + "{binary} output changed with invalid config, dotenv or env values" + ); + let entries_after: HashSet<_> = std::fs::read_dir(directory.path()) + .expect("read temporary directory") + .map(|entry| entry.expect("directory entry").file_name()) + .collect(); + assert_eq!( + entries_after, entries_before, + "{binary} created startup side effects" + ); + + let stdout = String::from_utf8(output.stdout).expect("UTF-8 output"); + let names: Vec<_> = stdout.lines().collect(); + assert!(!names.is_empty(), "{binary} returned no variables"); + assert!( + names.windows(2).all(|pair| pair[0] < pair[1]), + "{binary} output must be sorted and deduplicated" + ); + assert!( + names.iter().all(|name| name.starts_with("IGGY_")), + "{binary} output contained non-IGGY_ prefixed names" + ); + } +} + +#[test] +fn config_env_listing_includes_vector_index_templates() { + // Verify that vector fields are correctly represented with <N> placeholder + let output = run_list_config_env_vars_plain("iggy-server", &[]); + + let stdout = String::from_utf8(output.stdout).expect("UTF-8 output"); + let names: HashSet<_> = stdout.lines().collect(); + + // Cluster nodes should have indexed templates + assert!( + names + .iter() + .any(|name| name.contains("IGGY_CLUSTER_NODES_<N>_")), + "server should list cluster node index templates" + ); + + // Verify nested vector expansion (nested <N> placeholders) + assert!( + names + .iter() + .any(|name| name.contains("IGGY_CLUSTER_NODES_<N>_ADVERTISED_ADDRESSES_<N>_")), + "server should list nested vector templates" + ); +} + +#[test] +fn config_env_listing_includes_connector_templates() { + // Verify that connector SINK/SOURCE templates are correctly formatted + let output = run_list_config_env_vars_plain("iggy-connectors", &[]); + + let stdout = String::from_utf8(output.stdout).expect("UTF-8 output"); + let names: HashSet<_> = stdout.lines().collect(); + + // Both SINK and SOURCE templates should be present + assert!( + names + .iter() + .any(|name| name.contains("IGGY_CONNECTORS_SINK_<KEY>_")), + "connectors should list SINK templates with <KEY> placeholder" + ); + assert!( + names + .iter() + .any(|name| name.contains("IGGY_CONNECTORS_SOURCE_<KEY>_")), + "connectors should list SOURCE templates with <KEY> placeholder" + ); + + // Plugin config templates should be present + assert!( + names + .iter() + .any(|name| name.contains("IGGY_CONNECTORS_SINK_<KEY>_PLUGIN_CONFIG_<FIELD>")), + "connectors should list SINK plugin config templates" + ); + assert!( + names + .iter() + .any(|name| name.contains("IGGY_CONNECTORS_SOURCE_<KEY>_PLUGIN_CONFIG_<FIELD>")), + "connectors should list SOURCE plugin config templates" + ); + assert!( + names.contains("IGGY_CONNECTORS_SINK_<KEY>_PLUGIN_CONFIG_FORMAT"), + "connectors should list the typed SINK plugin config format" + ); + assert!( + names.contains("IGGY_CONNECTORS_SOURCE_<KEY>_PLUGIN_CONFIG_FORMAT"), + "connectors should list the typed SOURCE plugin config format" + ); + + // Verify SINK and SOURCE ENABLED fields are present + assert!( + names + .iter() + .any(|name| name.contains("IGGY_CONNECTORS_SINK_<KEY>_ENABLED")), + "connectors should list SINK_<KEY>_ENABLED" + ); + assert!( + names + .iter() + .any(|name| name.contains("IGGY_CONNECTORS_SOURCE_<KEY>_ENABLED")), + "connectors should list SOURCE_<KEY>_ENABLED" + ); + + assert_eq!( + stdout + .lines() + .filter(|name| *name == "IGGY_CONNECTORS_CONNECTORS_CONFIG_TYPE") + .count(), + 1, + "enum variants should produce one deduplicated tag name" + ); +} + +#[test] +fn config_env_listing_excludes_other_processes_variables() { Review Comment: Fixed in af7e26e. The shared helper now asserts successful exit, empty stderr, valid UTF-8, and non-empty output. ########## core/connectors/runtime/src/main.rs: ########## @@ -117,13 +139,52 @@ fn print_ascii_art(text: &str) { println!("{}", figure.unwrap()); } -#[tokio::main] -async fn main() -> Result<(), RuntimeError> { +fn main() -> Result<(), RuntimeError> { + let args = Args::parse(); + if args.list_config_env_vars { + print_config_env_vars()?; + return Ok(()); + } capture_allowed_cpus(); - Args::parse(); + Builder::new_multi_thread() + .enable_all() + .build()? + .block_on(run()) +} + +fn print_config_env_vars() -> std::io::Result<()> { + let sink_source_templates = [ + ("SINK", SinkConfig::env_templates()), + ("SOURCE", SourceConfig::env_templates()), + ] + .into_iter() + .flat_map(|(kind, templates)| { + // "<KEY>" stands in for the real, per-connector key `local_provider` + // uppercases at runtime - same prefix rule, so the listing can't + // drift from the names the runtime actually reads. + let prefix = + crate::configs::connectors::local_provider::connector_env_prefix(kind, "<KEY>"); + let plugin_config = format!("{prefix}PLUGIN_CONFIG_<FIELD>"); Review Comment: Fixed in af7e26e. PLUGIN_CONFIG_ENV_SEGMENT is shared by listing generation, local override lookup, and the unknown-variable scan. ########## core/ai/mcp/src/main.rs: ########## @@ -42,7 +46,20 @@ const VERSION: &str = env!("CARGO_PKG_VERSION"); const DEFAULT_CONFIG_PATH: &str = "core/ai/mcp/config.toml"; +#[derive(Debug, Parser)] +#[command(author = "Apache Iggy", version)] +struct Args { + /// Print supported configuration environment variables and exit. + #[arg(long)] + list_config_env_vars: bool, +} + fn main() -> Result<(), McpRuntimeError> { + let args = Args::parse(); Review Comment: Fixed in af7e26e and companion docs PR apache/iggy-website#130. The MCP README beside the Claude example now documents exit code 2 for unknown arguments. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
