Skip to content

Commit 135115a

Browse files
apollo_node: add new CLI arg to parse config
1 parent 5ef42b9 commit 135115a

8 files changed

Lines changed: 336 additions & 7 deletions

File tree

crates/apollo_config/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ description = "A library for handling node configuration."
88

99
[dependencies]
1010
apollo_infra_utils.workspace = true
11-
clap = { workspace = true, features = ["env", "string"] }
11+
clap = { workspace = true, features = ["derive", "env", "string"] }
1212
colored = { workspace = true, optional = true }
1313
const_format.workspace = true
1414
itertools.workspace = true

crates/apollo_config/src/command.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,13 @@ use serde_json::{json, Value};
77
use crate::loading::update_config_map;
88
use crate::{
99
ConfigError,
10+
ConfigFormat,
1011
ParamPath,
1112
SerializationType,
1213
SerializedParam,
1314
CONFIG_FILE_ARG_NAME,
1415
CONFIG_FILE_SHORT_ARG_NAME,
16+
CONFIG_FORMAT_ARG_NAME,
1517
};
1618

1719
pub(crate) fn get_command_matches(
@@ -51,6 +53,16 @@ fn build_args_parser(config_map: &BTreeMap<ParamPath, SerializedParam>) -> Vec<A
5153
.value_parser(value_parser!(PathBuf))
5254
.num_args(1..) // Allow multiple values
5355
.action(clap::ArgAction::Append), // Collect multiple occurrences
56+
// How the --config_file arguments are interpreted. Absent => ConfigFormat::Preset.
57+
Arg::new(CONFIG_FORMAT_ARG_NAME)
58+
.long(CONFIG_FORMAT_ARG_NAME)
59+
.num_args(1)
60+
.value_parser(value_parser!(ConfigFormat))
61+
.help(
62+
"How the --config_file arguments are interpreted: 'preset' (flat dotted-key, \
63+
layered) or 'native' (the first file is nested serde, later files are flat \
64+
secret overrides)",
65+
),
5466
];
5567

5668
for (param_path, serialized_param) in config_map.iter() {

crates/apollo_config/src/config_test.rs

Lines changed: 209 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -647,6 +647,215 @@ fn test_load_many_custom_config_files() {
647647
assert_eq!(param_path, "custom value");
648648
}
649649

650+
#[derive(Deserialize, Debug, PartialEq)]
651+
struct NativeInnerConfig {
652+
a: usize,
653+
b: String,
654+
}
655+
656+
#[derive(Deserialize, Debug, PartialEq)]
657+
struct NativeConfig {
658+
top: String,
659+
inner: NativeInnerConfig,
660+
}
661+
662+
// Mirrors the real secrets layout: dotted keys reach deep, real, nested leaves (e.g.
663+
// `consensus_manager_config.network_config.secret_key`), never single-token pointer targets.
664+
#[derive(Deserialize, Debug, PartialEq)]
665+
struct NativeNetworkConfig {
666+
secret_key: String,
667+
other_network_field: bool,
668+
}
669+
670+
#[derive(Deserialize, Debug, PartialEq)]
671+
struct NativeComponentConfig {
672+
network_config: NativeNetworkConfig,
673+
other_component_field: u32,
674+
}
675+
676+
#[derive(Deserialize, Debug, PartialEq)]
677+
struct NativeDeepConfig {
678+
component_config: NativeComponentConfig,
679+
other_top_field: String,
680+
}
681+
682+
// A `None` optional sub-config, mirroring a disabled component (e.g. `central_sync_client_config`).
683+
#[derive(Deserialize, Debug, PartialEq)]
684+
struct NativeOptionalConfig {
685+
optional_component: Option<NativeComponentConfig>,
686+
top_field: String,
687+
}
688+
689+
// Loads a config in `--config_format native` mode. The schema file is unused by the native path but
690+
// still parsed, so an empty object is supplied as a valid (empty) schema.
691+
fn load_native_config<T: for<'a> Deserialize<'a>>(
692+
config_file_args: Vec<&str>,
693+
) -> Result<T, ConfigError> {
694+
let schema_file = NamedTempFile::new().unwrap();
695+
std::fs::write(schema_file.path(), json!({}).to_string()).unwrap();
696+
697+
let args = chain!(["Testing", "--config_format", "native"], config_file_args)
698+
.map(|arg| arg.to_owned())
699+
.collect();
700+
701+
load_and_process_config::<T>(
702+
File::open(schema_file.path()).unwrap(),
703+
Command::new("Program"),
704+
args,
705+
false,
706+
)
707+
}
708+
709+
#[test]
710+
fn test_native_config_with_secret_overrides() {
711+
let base_file = NamedTempFile::new().unwrap();
712+
let base_config = json!({"top": "base_top", "inner": {"a": 1, "b": "base_b"}});
713+
std::fs::write(base_file.path(), base_config.to_string()).unwrap();
714+
// Secrets stay flat dotted-key and are applied onto the nested base.
715+
let secret_file = NamedTempFile::new().unwrap();
716+
std::fs::write(secret_file.path(), json!({"inner.b": "secret_b"}).to_string()).unwrap();
717+
718+
let loaded: NativeConfig = load_native_config(vec![
719+
CONFIG_FILE_ARG,
720+
base_file.path().to_str().unwrap(),
721+
CONFIG_FILE_ARG,
722+
secret_file.path().to_str().unwrap(),
723+
])
724+
.unwrap();
725+
726+
assert_eq!(
727+
loaded,
728+
NativeConfig {
729+
top: "base_top".to_owned(),
730+
inner: NativeInnerConfig { a: 1, b: "secret_b".to_owned() },
731+
}
732+
);
733+
}
734+
735+
#[test]
736+
fn test_native_config_single_file() {
737+
let base_file = NamedTempFile::new().unwrap();
738+
let base_config = json!({"top": "base_top", "inner": {"a": 1, "b": "base_b"}});
739+
std::fs::write(base_file.path(), base_config.to_string()).unwrap();
740+
741+
let loaded: NativeConfig =
742+
load_native_config(vec![CONFIG_FILE_ARG, base_file.path().to_str().unwrap()]).unwrap();
743+
744+
assert_eq!(
745+
loaded,
746+
NativeConfig {
747+
top: "base_top".to_owned(),
748+
inner: NativeInnerConfig { a: 1, b: "base_b".to_owned() },
749+
}
750+
);
751+
}
752+
753+
#[test]
754+
fn test_native_config_without_config_file() {
755+
let result = load_native_config::<NativeConfig>(vec![]);
756+
assert_matches!(result, Err(ConfigError::NativeModeRequiresConfigFile));
757+
}
758+
759+
// Mirrors the production secrets file (crates/apollo_deployments/resources/testing_secrets.json):
760+
// each secret is a flat dotted key naming a deep, real nested leaf. Asserts the override lands at
761+
// exactly that leaf and every sibling at every level is preserved.
762+
#[test]
763+
fn test_native_config_deep_secret_override_preserves_siblings() {
764+
let base_file = NamedTempFile::new().unwrap();
765+
let base_config = json!({
766+
"other_top_field": "base_top",
767+
"component_config": {
768+
"other_component_field": 7,
769+
"network_config": {
770+
"secret_key": "base_secret",
771+
"other_network_field": true
772+
}
773+
}
774+
});
775+
std::fs::write(base_file.path(), base_config.to_string()).unwrap();
776+
777+
let secret_file = NamedTempFile::new().unwrap();
778+
let secret_config = json!({"component_config.network_config.secret_key": "0xabc"});
779+
std::fs::write(secret_file.path(), secret_config.to_string()).unwrap();
780+
781+
let loaded: NativeDeepConfig = load_native_config(vec![
782+
CONFIG_FILE_ARG,
783+
base_file.path().to_str().unwrap(),
784+
CONFIG_FILE_ARG,
785+
secret_file.path().to_str().unwrap(),
786+
])
787+
.unwrap();
788+
789+
assert_eq!(
790+
loaded,
791+
NativeDeepConfig {
792+
other_top_field: "base_top".to_owned(),
793+
component_config: NativeComponentConfig {
794+
other_component_field: 7,
795+
network_config: NativeNetworkConfig {
796+
secret_key: "0xabc".to_owned(),
797+
other_network_field: true,
798+
},
799+
},
800+
}
801+
);
802+
}
803+
804+
// A secret aimed at a child of a `None` (null) intermediate must be skipped, not vivified into a
805+
// partial object — otherwise deserialization of the (incomplete) sub-config would fail. The
806+
// disabled component stays `None`.
807+
#[test]
808+
fn test_native_config_skips_secret_under_none_intermediate() {
809+
let base_file = NamedTempFile::new().unwrap();
810+
let base_config = json!({"optional_component": null, "top_field": "base"});
811+
std::fs::write(base_file.path(), base_config.to_string()).unwrap();
812+
813+
let secret_file = NamedTempFile::new().unwrap();
814+
let secret_config = json!({"optional_component.network_config.secret_key": "0xabc"});
815+
std::fs::write(secret_file.path(), secret_config.to_string()).unwrap();
816+
817+
let loaded: NativeOptionalConfig = load_native_config(vec![
818+
CONFIG_FILE_ARG,
819+
base_file.path().to_str().unwrap(),
820+
CONFIG_FILE_ARG,
821+
secret_file.path().to_str().unwrap(),
822+
])
823+
.unwrap();
824+
825+
assert_eq!(
826+
loaded,
827+
NativeOptionalConfig { optional_component: None, top_field: "base".to_owned() }
828+
);
829+
}
830+
831+
// A secret naming a leaf that is absent from an existing parent object is created (the parent
832+
// exists, so the component is enabled and the override is relevant).
833+
#[test]
834+
fn test_native_config_creates_absent_leaf_in_existing_parent() {
835+
let base_file = NamedTempFile::new().unwrap();
836+
let base_config = json!({"top": "base_top", "inner": {"a": 1}});
837+
std::fs::write(base_file.path(), base_config.to_string()).unwrap();
838+
839+
let secret_file = NamedTempFile::new().unwrap();
840+
std::fs::write(secret_file.path(), json!({"inner.b": "created_b"}).to_string()).unwrap();
841+
842+
let loaded: NativeConfig = load_native_config(vec![
843+
CONFIG_FILE_ARG,
844+
base_file.path().to_str().unwrap(),
845+
CONFIG_FILE_ARG,
846+
secret_file.path().to_str().unwrap(),
847+
])
848+
.unwrap();
849+
850+
assert_eq!(
851+
loaded,
852+
NativeConfig {
853+
top: "base_top".to_owned(),
854+
inner: NativeInnerConfig { a: 1, b: "created_b".to_owned() },
855+
}
856+
);
857+
}
858+
650859
// Make sure that if we have a field `foo_bar` and an optional field called `foo` with a value of
651860
// None, we don't remove the foo_bar field from the config.
652861
// This test was added following bug #37984 (see bug for more details).

crates/apollo_config/src/lib.rs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,21 @@ pub const CONFIG_FILE_SHORT_ARG_NAME: char = 'f';
6464
/// The config file arg name prepended with a double dash.
6565
pub const CONFIG_FILE_ARG: &str = formatcp!("--{}", CONFIG_FILE_ARG_NAME);
6666

67+
/// Arg name for selecting how the config files are interpreted.
68+
pub const CONFIG_FORMAT_ARG_NAME: &str = "config_format";
69+
70+
/// Selects how the `--config_file` arguments are interpreted when loading a config.
71+
#[derive(Clone, Copy, Debug, Default, PartialEq, Eq, clap::ValueEnum)]
72+
pub enum ConfigFormat {
73+
/// Flat dotted-key files, layered through the full pipeline (pointers, env/CLI overrides,
74+
/// `#is_none` marks). This is the historical behavior that will be deprecated.
75+
#[default]
76+
Preset,
77+
/// The first file is a nested JSON object deserialized directly with serde; any subsequent
78+
/// files are flat dotted-key secret overrides applied onto it.
79+
Native,
80+
}
81+
6782
/// A config indicator for optional parameters.
6883
pub const IS_NONE_MARK: &str = "#is_none";
6984
/// A config indicator for a sub config.
@@ -208,6 +223,11 @@ pub enum ConfigError {
208223
IOError(#[from] std::io::Error),
209224
#[error(transparent)]
210225
MissingParam(#[from] serde_json::Error),
226+
#[error(
227+
"Native config format requires exactly two config files: the base config and the secret \
228+
config."
229+
)]
230+
NativeModeRequiresTwoConfigFiles,
211231
#[error("{pointing_param} is not found.")]
212232
PointerSourceNotFound { pointing_param: String },
213233
#[error("{target_param} is not found.")]

crates/apollo_config/src/loading.rs

Lines changed: 75 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,17 +14,19 @@ use command::{get_command_matches, update_config_map_by_command_args};
1414
use itertools::any;
1515
use serde::Deserialize;
1616
use serde_json::{json, Map, Value};
17-
use tracing::{error, info, instrument};
17+
use tracing::{debug, error, info, instrument};
1818

1919
use crate::validators::validate_path_exists;
2020
use crate::{
2121
command,
2222
ConfigError,
23+
ConfigFormat,
2324
ParamPath,
2425
SerializationType,
2526
SerializedContent,
2627
SerializedParam,
2728
CONFIG_FILE_ARG_NAME,
29+
CONFIG_FORMAT_ARG_NAME,
2830
FIELD_SEPARATOR,
2931
IS_NONE_MARK,
3032
};
@@ -47,6 +49,68 @@ pub fn load<T: for<'a> Deserialize<'a>>(
4749
Ok(serde_json::from_value(nested_map)?)
4850
}
4951

52+
/// Deserializes the config directly from nested JSON files (the `ConfigFormat::Native` path).
53+
///
54+
/// Requires exactly two paths. The first is the base config: a nested JSON object matching the
55+
/// target struct's field hierarchy. The second is a flat dotted-key secret file whose entries are
56+
/// written into the nested base before deserialization. Unlike the preset path, no pointers,
57+
/// env/CLI overrides, or `#is_none` marks are applied — the base file is expected to be a complete,
58+
/// resolved config.
59+
fn load_native<T: for<'a> Deserialize<'a>>(
60+
custom_config_paths: Vec<PathBuf>,
61+
) -> Result<T, ConfigError> {
62+
let [base_config_path, secret_config_path] =
63+
custom_config_paths.as_array().ok_or(ConfigError::NativeModeRequiresTwoConfigFiles)?;
64+
65+
validate_path_exists(base_config_path)?;
66+
info!("Loading native config file: {:?}", base_config_path);
67+
let mut nested_map: Value = serde_json::from_reader(File::open(base_config_path)?)?;
68+
validate_path_exists(secret_config_path)?;
69+
info!("Loading native secret config file: {:?}", secret_config_path);
70+
71+
let secret_config: Map<String, Value> =
72+
serde_json::from_reader(File::open(secret_config_path)?)?;
73+
for (param_path, value) in secret_config {
74+
set_nested_value(&mut nested_map, &param_path, value);
75+
}
76+
77+
Ok(serde_json::from_value(nested_map)?)
78+
}
79+
80+
/// Writes `value` into `nested_map` at the leaf named by the dotted `param_path`, descending
81+
/// through (and preserving the siblings of) the intermediate objects. The leaf key itself is
82+
/// created if absent.
83+
///
84+
/// Traversal stops without writing anything if an intermediate on the path is `null` (a `None`
85+
/// optional, e.g. a disabled component) or is missing/not an object. The base config is the source
86+
/// of truth for which subtrees exist; a secret aimed at a subtree the base omits is irrelevant, and
87+
/// vivifying it would produce a partial object that fails deserialization. This also avoids any
88+
/// panic on a type-mismatched intermediate.
89+
fn set_nested_value(nested_map: &mut Value, param_path: &str, value: Value) {
90+
let mut segments = param_path.split(FIELD_SEPARATOR).peekable();
91+
let mut entry = nested_map;
92+
while let Some(segment) = segments.next() {
93+
let Value::Object(map) = entry else {
94+
return;
95+
};
96+
if segments.peek().is_none() {
97+
map.insert(segment.to_owned(), value);
98+
return;
99+
}
100+
// Descend into the intermediate; stop if it is absent or an explicit `null` (`None`).
101+
match map.get_mut(segment) {
102+
Some(child) if !child.is_null() => entry = child,
103+
_ => {
104+
debug!(
105+
"Skipping secret override {param_path:?}: intermediate {segment:?} is absent \
106+
or None in the base config."
107+
);
108+
return;
109+
}
110+
}
111+
}
112+
}
113+
50114
/// Deserializes a json config file, updates the values by the given arguments for the command, and
51115
/// set values for the pointers.
52116
pub fn load_and_process_config<T: for<'a> Deserialize<'a>>(
@@ -62,6 +126,16 @@ pub fn load_and_process_config<T: for<'a> Deserialize<'a>>(
62126
let (config_map, pointers_map) = split_pointers_map(deserialized_config_schema.clone());
63127
// Take param paths with corresponding descriptions, and get the matching arguments.
64128
let mut arg_matches = get_command_matches(&config_map, command, args)?;
129+
130+
let config_format =
131+
arg_matches.remove_one::<ConfigFormat>(CONFIG_FORMAT_ARG_NAME).unwrap_or_default();
132+
if config_format == ConfigFormat::Native {
133+
let custom_config_paths: Vec<PathBuf> = arg_matches
134+
.remove_many::<PathBuf>(CONFIG_FILE_ARG_NAME)
135+
.map(|paths| paths.collect())
136+
.unwrap_or_default();
137+
return load_native(custom_config_paths);
138+
}
65139
// Retaining values from the default config map for backward compatibility.
66140
let (mut values_map, types_map) = split_values_and_types(config_map);
67141
if ignore_default_values {

0 commit comments

Comments
 (0)