Skip to content

Commit 5ada426

Browse files
[library-config] Skip error on unknown config key (#868)
# What does this PR do? * Add custom deserializer for the configuration map to not error on unknown keys
1 parent 1a7e673 commit 5ada426

File tree

1 file changed

+70
-27
lines changed

1 file changed

+70
-27
lines changed

library-config/src/lib.rs

Lines changed: 70 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -83,10 +83,7 @@ impl<'a> Matcher<'a> {
8383
}
8484

8585
/// Returns the first set of configurations that match the current process
86-
fn find_stable_config<'b>(
87-
&'a self,
88-
cfg: &'b StableConfig,
89-
) -> Option<&'b HashMap<LibraryConfigName, String>> {
86+
fn find_stable_config<'b>(&'a self, cfg: &'b StableConfig) -> Option<&'b ConfigMap> {
9087
for rule in &cfg.rules {
9188
if rule.selectors.iter().all(|s| self.selector_match(s)) {
9289
return Some(&rule.configuration);
@@ -248,6 +245,52 @@ impl ProcessInfo {
248245
}
249246
}
250247

248+
/// A (key, value) struct
249+
///
250+
/// This type has a custom serde Deserialize implementation from maps:
251+
/// * It skips invalid/unknown keys in the map
252+
/// * Since the storage is a Boxed slice and not a Hashmap, it doesn't over-allocate
253+
#[derive(Debug, Default, PartialEq, Eq)]
254+
struct ConfigMap(Box<[(LibraryConfigName, String)]>);
255+
256+
impl<'de> serde::Deserialize<'de> for ConfigMap {
257+
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
258+
where
259+
D: serde::Deserializer<'de>,
260+
{
261+
struct ConfigMapVisitor;
262+
impl<'de> serde::de::Visitor<'de> for ConfigMapVisitor {
263+
type Value = ConfigMap;
264+
265+
fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result {
266+
formatter.write_str("struct ConfigMap(HashMap<LibraryConfig, String>)")
267+
}
268+
269+
fn visit_map<A>(self, mut map: A) -> Result<Self::Value, A::Error>
270+
where
271+
A: serde::de::MapAccess<'de>,
272+
{
273+
let mut configs = Vec::new();
274+
configs.reserve_exact(map.size_hint().unwrap_or(0));
275+
loop {
276+
let k = match map.next_key::<LibraryConfigName>() {
277+
Ok(Some(k)) => k,
278+
Ok(None) => break,
279+
Err(_) => {
280+
map.next_value::<serde::de::IgnoredAny>()?;
281+
continue;
282+
}
283+
};
284+
let v = map.next_value::<String>()?;
285+
configs.push((k, v));
286+
}
287+
Ok(ConfigMap(configs.into_boxed_slice()))
288+
}
289+
}
290+
deserializer.deserialize_map(ConfigMapVisitor)
291+
}
292+
}
293+
251294
#[repr(C)]
252295
#[derive(Clone, Copy, serde::Deserialize, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)]
253296
#[serde(rename_all = "SCREAMING_SNAKE_CASE")]
@@ -349,7 +392,7 @@ struct Selector {
349392
#[derive(serde::Deserialize, Debug, PartialEq, Eq)]
350393
struct Rule {
351394
selectors: Vec<Selector>,
352-
configuration: HashMap<LibraryConfigName, String>,
395+
configuration: ConfigMap,
353396
}
354397

355398
#[derive(serde::Deserialize, Default, Debug, PartialEq, Eq)]
@@ -358,7 +401,7 @@ struct StableConfig {
358401
#[serde(default)]
359402
config_id: Option<String>,
360403
#[serde(default)]
361-
apm_configuration_default: HashMap<LibraryConfigName, String>,
404+
apm_configuration_default: ConfigMap,
362405

363406
// Phase 2
364407
#[serde(default)]
@@ -589,6 +632,9 @@ impl Configurator {
589632
// Phase 1: take host default config
590633
cfg.extend(
591634
mem::take(&mut stable_config.apm_configuration_default)
635+
.0
636+
// TODO(paullgdc): use Box<[I]>::into_iter when we can use rust 1.80
637+
.into_vec()
592638
.into_iter()
593639
.map(|(k, v)| {
594640
(
@@ -623,7 +669,7 @@ impl Configurator {
623669
return Ok(());
624670
};
625671

626-
for (name, config_val) in configs {
672+
for (name, config_val) in configs.0.iter() {
627673
let value = matcher.template_config(config_val)?;
628674
library_config.insert(
629675
*name,
@@ -682,23 +728,10 @@ mod tests {
682728

683729
use super::{Configurator, ProcessInfo};
684730
use crate::{
685-
LibraryConfig, LibraryConfigName, LibraryConfigSource, Matcher, Operator, Origin, Rule,
686-
Selector, StableConfig,
731+
ConfigMap, LibraryConfig, LibraryConfigName, LibraryConfigSource, Matcher, Operator,
732+
Origin, Rule, Selector, StableConfig,
687733
};
688734

689-
macro_rules! map {
690-
($(($key:expr , $value:expr)),* $(,)?) => {
691-
{
692-
#[allow(unused_mut)]
693-
let mut map = std::collections::HashMap::new();
694-
$(
695-
map.insert($key, $value);
696-
)*
697-
map
698-
}
699-
};
700-
}
701-
702735
fn test_config(local_cfg: &[u8], fleet_cfg: &[u8], expected: Vec<LibraryConfig>) {
703736
let process_info: ProcessInfo = ProcessInfo {
704737
args: vec![
@@ -842,8 +875,12 @@ apm_configuration_default:
842875
DD_APPSEC_ENABLED: true
843876
DD_IAST_ENABLED: true
844877
DD_DYNAMIC_INSTRUMENTATION_ENABLED: true
878+
# extra keys should be skipped without errors
879+
FOO_BAR: quoicoubeh
845880
DD_DATA_JOBS_ENABLED: true
846881
DD_APPSEC_SCA_ENABLED: true
882+
wtf:
883+
- 1
847884
",
848885
vec![
849886
LibraryConfig {
@@ -991,6 +1028,7 @@ rules:
9911028

9921029
#[test]
9931030
fn test_parse_static_config() {
1031+
use LibraryConfigName::*;
9941032
let mut tmp = tempfile::NamedTempFile::new().unwrap();
9951033
tmp.reopen()
9961034
.unwrap()
@@ -1004,6 +1042,8 @@ rules:
10041042
configuration:
10051043
DD_PROFILING_ENABLED: true
10061044
DD_SERVICE: my-service
1045+
# extra keys should be skipped without errors
1046+
FOOBAR: maybe??
10071047
",
10081048
)
10091049
.unwrap();
@@ -1015,7 +1055,7 @@ rules:
10151055
cfg,
10161056
StableConfig {
10171057
config_id: None,
1018-
apm_configuration_default: HashMap::default(),
1058+
apm_configuration_default: ConfigMap::default(),
10191059
tags: HashMap::default(),
10201060
rules: vec![Rule {
10211061
selectors: vec![Selector {
@@ -1025,10 +1065,13 @@ rules:
10251065
},
10261066
key: None,
10271067
}],
1028-
configuration: map![
1029-
(LibraryConfigName::DdProfilingEnabled, "true".to_owned()),
1030-
(LibraryConfigName::DdService, "my-service".to_owned())
1031-
],
1068+
configuration: ConfigMap(
1069+
vec![
1070+
(DdProfilingEnabled, "true".to_owned()),
1071+
(DdService, "my-service".to_owned())
1072+
]
1073+
.into_boxed_slice()
1074+
),
10321075
}]
10331076
}
10341077
)

0 commit comments

Comments
 (0)