Skip to content

Commit 102de97

Browse files
apollo_config,apollo_node_config,apollo_config_manager: rework get_config_presentation off dump()
get_config_presentation was the last native-path consumer of dump() (it used dump() only as a privacy registry to redact Private params from the startup-log / config-diff presentation). Rework it to take an injected private-path set: get_config_presentation<T: Serialize>(config, include_private_parameters, private_paths) — dropping the SerializeConfig bound. Callers (load_and_validate_config startup log, config_manager diff) inject &private_parameters() (the committed config_secrets_schema.json set). The injected set equals the old dump()-derived Private set exactly (is_private() == privacy==Private), so redaction is preserved with no leak / no over-redaction; a new no-leak test guards it. remove_path_from_json (the redaction mechanism) is retained. This unblocks deleting dump()/SerializeConfig in Phase 9. apollo_config 27+4, apollo_config_manager 15, apollo_node_config 33, apollo_deployments 8 green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 98aaff6 commit 102de97

4 files changed

Lines changed: 67 additions & 19 deletions

File tree

crates/apollo_config/src/config_test.rs

Lines changed: 50 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use std::collections::{BTreeMap, HashSet};
1+
use std::collections::{BTreeMap, BTreeSet, HashSet};
22
use std::time::Duration;
33

44
use assert_matches::assert_matches;
@@ -116,6 +116,18 @@ impl SerializeConfig for TypicalConfig {
116116
}
117117
}
118118

119+
/// Derives the set of `Private` param paths from a fixture's `dump()`. Mirrors the privacy registry
120+
/// that production callers inject (e.g. `private_parameters()`), so the presentation tests stay
121+
/// pinned to the fixtures' own privacy declarations.
122+
fn private_paths_from_dump<T: SerializeConfig>(config: &T) -> BTreeSet<ParamPath> {
123+
config
124+
.dump()
125+
.into_iter()
126+
.filter(|(_, serialized_param)| serialized_param.privacy == ParamPrivacy::Private)
127+
.map(|(param_path, _)| param_path)
128+
.collect()
129+
}
130+
119131
#[test]
120132
fn test_config_presentation() {
121133
let config = TypicalConfig {
@@ -126,15 +138,44 @@ fn test_config_presentation() {
126138
e: 10,
127139
f: 0.5,
128140
};
129-
let presentation = get_config_presentation(&config, true).unwrap();
141+
let private_paths = private_paths_from_dump(&config);
142+
// `c` is the only `Private` param in this fixture.
143+
assert_eq!(private_paths, BTreeSet::from(["c".to_owned()]));
144+
145+
let presentation = get_config_presentation(&config, true, &private_paths).unwrap();
130146
let keys: Vec<_> = presentation.as_object().unwrap().keys().collect();
131147
assert_eq!(keys, vec!["a", "b", "c", "d", "e", "f"]);
132148

133-
let public_presentation = get_config_presentation(&config, false).unwrap();
149+
let public_presentation = get_config_presentation(&config, false, &private_paths).unwrap();
134150
let keys: Vec<_> = public_presentation.as_object().unwrap().keys().collect();
135151
assert_eq!(keys, vec!["a", "b", "d", "e", "f"]);
136152
}
137153

154+
#[test]
155+
fn test_config_presentation_does_not_leak_private_params() {
156+
let config = TypicalConfig {
157+
a: Duration::from_secs(1),
158+
b: "bbb".to_owned(),
159+
c: true,
160+
d: -1,
161+
e: 10,
162+
f: 0.5,
163+
};
164+
// Inject `c` as the private path (the secret), matching what production callers do.
165+
let private_paths = BTreeSet::from(["c".to_owned()]);
166+
167+
// Redacted presentation must NOT contain the private param.
168+
let public_presentation = get_config_presentation(&config, false, &private_paths).unwrap();
169+
assert!(
170+
public_presentation.as_object().unwrap().get("c").is_none(),
171+
"private param `c` leaked into the redacted presentation"
172+
);
173+
174+
// Full presentation MUST contain it.
175+
let full_presentation = get_config_presentation(&config, true, &private_paths).unwrap();
176+
assert!(full_presentation.as_object().unwrap().get("c").is_some());
177+
}
178+
138179
#[test]
139180
fn test_nested_config_presentation() {
140181
let configs = vec![
@@ -152,10 +193,14 @@ fn test_nested_config_presentation() {
152193
];
153194

154195
for config in configs {
155-
let presentation = get_config_presentation(&config, true).unwrap();
196+
let private_paths = private_paths_from_dump(&config);
197+
// This fixture declares no `Private` params, so nothing is redacted.
198+
assert!(private_paths.is_empty());
199+
200+
let presentation = get_config_presentation(&config, true, &private_paths).unwrap();
156201
let keys: Vec<_> = presentation.as_object().unwrap().keys().collect();
157202
assert_eq!(keys, vec!["inner_config", "opt_config", "opt_elem"]);
158-
let public_presentation = get_config_presentation(&config, false).unwrap();
203+
let public_presentation = get_config_presentation(&config, false, &private_paths).unwrap();
159204
let keys: Vec<_> = public_presentation.as_object().unwrap().keys().collect();
160205
assert_eq!(keys, vec!["inner_config", "opt_config", "opt_elem"]);
161206
}

crates/apollo_config/src/presentation.rs

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,32 @@
11
//! presentation of a configuration, with hiding or exposing private parameters.
22
3+
use std::collections::BTreeSet;
34
use std::ops::IndexMut;
45

56
use itertools::Itertools;
67
use serde::Serialize;
78

8-
use crate::dumping::SerializeConfig;
9-
use crate::{ConfigError, ParamPrivacy};
9+
use crate::{ConfigError, ParamPath};
1010

1111
/// Returns presentation of the public parameters in the config.
12-
pub fn get_config_presentation<T: Serialize + SerializeConfig>(
12+
///
13+
/// When `include_private_parameters` is `false`, every path in `private_paths` is redacted from the
14+
/// presentation (the dump-independent redaction mechanism). `private_paths` is the set of `Private`
15+
/// param paths for `config`'s type, injected by the caller (this crate sits below the crate that
16+
/// owns the privacy registry, so it cannot derive the set itself).
17+
pub fn get_config_presentation<T: Serialize>(
1318
config: &T,
1419
include_private_parameters: bool,
20+
private_paths: &BTreeSet<ParamPath>,
1521
) -> Result<serde_json::Value, ConfigError> {
1622
let mut config_presentation = serde_json::to_value(config)?;
1723
if include_private_parameters {
1824
return Ok(config_presentation);
1925
}
2026

21-
// Iterates over flatten param paths for removing non-public parameters from the nested config.
22-
for (param_path, serialized_param) in config.dump() {
23-
match serialized_param.privacy {
24-
ParamPrivacy::Public => continue,
25-
ParamPrivacy::TemporaryValue => continue,
26-
ParamPrivacy::Private => remove_path_from_json(&param_path, &mut config_presentation)?,
27-
}
27+
// Remove every private param path from the nested config presentation.
28+
for param_path in private_paths {
29+
remove_path_from_json(param_path, &mut config_presentation)?;
2830
}
2931
Ok(config_presentation)
3032
}

crates/apollo_config_manager/src/config_manager_runner.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use apollo_config_manager_config::config::ConfigManagerConfig;
99
use apollo_config_manager_types::communication::SharedConfigManagerClient;
1010
use apollo_infra::component_definitions::{default_component_start_fn, ComponentStarter};
1111
use apollo_infra::component_server::WrapperServer;
12-
use apollo_node_config::config_utils::load_and_validate_config;
12+
use apollo_node_config::config_utils::{load_and_validate_config, private_parameters};
1313
use apollo_node_config::node_config::NodeDynamicConfig;
1414
use async_trait::async_trait;
1515
use notify::{Config as NotifyConfig, EventKind, RecommendedWatcher, RecursiveMode, Watcher};
@@ -155,8 +155,9 @@ impl ConfigManagerRunner {
155155
}
156156

157157
fn log_config_diff(&self, old_config: &NodeDynamicConfig, new_config: &NodeDynamicConfig) {
158-
let old_config = get_config_presentation(old_config, false).unwrap();
159-
let new_config = get_config_presentation(new_config, false).unwrap();
158+
let private_paths = private_parameters();
159+
let old_config = get_config_presentation(old_config, false, &private_paths).unwrap();
160+
let new_config = get_config_presentation(new_config, false, &private_paths).unwrap();
160161
let all_keys: BTreeSet<_> = old_config
161162
.as_object()
162163
.unwrap()

crates/apollo_node_config/src/config_utils.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ pub fn load_and_validate_config(
156156
info!("Config map:");
157157
info!(
158158
"{:#?}",
159-
get_config_presentation::<SequencerNodeConfig>(&loaded_config, false)
159+
get_config_presentation(&loaded_config, false, &private_parameters())
160160
.expect("Should be able to get representation.")
161161
);
162162
info!("Finished dumping configuration.");

0 commit comments

Comments
 (0)