Skip to content

Commit bcb9c8c

Browse files
committed
fix(config): coerce scalar config leaves the way the Agent does
The Agent never reads a setting as the type its YAML holds: GetBool, GetInt, GetFloat64, and GetString each cast the stored value through spf13/cast. Permissiveness is therefore a property of a leaf's declared type, not of the key, so `dogstatsd_port: "8125"` and a string-typed leaf written as a YAML boolean are configurations the Agent accepts. The generated Datadog source model took each leaf's JSON type literally, so those spellings failed deserialization. That aborts the strict startup gate, and at runtime it rejects the whole Agent snapshot, holding every other key at its last-known-good value. Port cast.To{Bool,Int64,Float64,String}E into cast_de and have codegen attach one by leaf type, keeping the schema's type as the field type. env_decode now shares those parsers, so one accept-set serves the file, the environment, and the Agent stream. A value the cast cannot convert stays a hard error rather than the Agent's silent zero value. Every generated field is classified, and an unrecognized type fails the build, so a schema change cannot quietly ship a leaf that rejects input the Agent accepts. This subsumes the overlay's `input_shape` metadata, whose one shape (a byte size written as an integer) is what a string leaf now accepts by type, so it and string_de are removed.
1 parent ca31d5f commit bcb9c8c

10 files changed

Lines changed: 896 additions & 229 deletions

File tree

.claude/skills/config-system/SKILL.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,19 @@ Reserve `#[serde(flatten)]` for a struct that genuinely groups several *top-leve
123123
example, the forwarder's `forwarder_*` retry settings). Name a Rust field after its canonical
124124
section rather than renaming it onto one.
125125

126+
## Scalar leaf coercion
127+
128+
The Agent reads every setting by casting whatever is stored to the accessor's type (`GetBool`,
129+
`GetInt`, `GetString`, ...), so a leaf's permissiveness follows from its declared type, not from the
130+
key: `dogstatsd_port: "8125"`, or a `string`-typed leaf written as a YAML boolean, are configurations
131+
the Agent accepts. `datadog-agent/config/src/cast_de.rs` ports those casts, codegen attaches one to
132+
every scalar leaf by type (`permissivize`), and `env_decode` shares the same parsers, so every source
133+
accepts the same spellings. A value the cast cannot convert is a hard error rather than the Agent's
134+
silent zero value.
135+
136+
Do not hand-write a per-key tolerant deserializer for a schema-typed scalar; give the coercion to the
137+
type instead. A generated field whose type has no classification fails the build.
138+
126139
## Saluki-only values
127140

128141
Values absent from the Datadog schema reach `SalukiConfiguration` through the `SalukiOnly` source

lib/agent-data-plane-config-system/src/system.rs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -546,6 +546,26 @@ mod tests {
546546
assert_eq!(system.config().domains.dogstatsd.debug_log.log_file_max_size, 10485760);
547547
}
548548

549+
#[tokio::test]
550+
async fn standalone_loads_scalars_written_in_any_form_the_agent_casts() {
551+
// The Agent reads a setting by casting whatever its configuration holds to the accessor's
552+
// type, so a boolean written where the schema declares a string, or a quoted integer, is a
553+
// configuration it accepts. Each must reach the typed model instead of aborting the strict
554+
// startup gate.
555+
let system = standalone_system(
556+
Some(json!({
557+
"use_v3_api": { "series": { "enabled": true } },
558+
"dogstatsd_port": "8126",
559+
})),
560+
None,
561+
)
562+
.await
563+
.expect("scalars in Agent-castable forms boot");
564+
565+
assert_eq!(system.config().shared.metrics_encoding.v3_series_mode.mode, "true");
566+
assert_eq!(system.config().domains.dogstatsd.listeners.port, 8126);
567+
}
568+
549569
#[tokio::test]
550570
async fn translation_invalid_update_is_rejected_keeping_last_known_good() {
551571
let (system, agent_tx) =

lib/datadog-agent/config-overlay-model/src/lib.rs

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,6 @@ pub struct FullSupport {
5959
/// GitHub issue tracking number.
6060
#[serde(default)]
6161
pub issue: Option<String>,
62-
/// Accepted input shape when it is wider than the schema's declared type (see [`InputShape`]).
63-
#[serde(default)]
64-
pub input_shape: Option<InputShape>,
6562
/// Fields to support the `config_registry` and configuration smoke tests.
6663
pub test_support: TestSupport,
6764
}
@@ -82,9 +79,6 @@ pub struct PartialSupport {
8279
/// GitHub issue tracking number.
8380
#[serde(default)]
8481
pub issue: Option<String>,
85-
/// Accepted input shape when it is wider than the schema's declared type (see [`InputShape`]).
86-
#[serde(default)]
87-
pub input_shape: Option<InputShape>,
8882
/// Fields to support the `config_registry` and configuration smoke tests.
8983
pub test_support: TestSupport,
9084
}
@@ -241,19 +235,6 @@ pub enum ValueType {
241235
StringList,
242236
}
243237

244-
/// A widened input shape a schema `string` leaf accepts beyond a bare string.
245-
///
246-
/// The vendored schema types some settings as `string` but documents an equivalent numeric form
247-
/// (for example a byte size given as `10485760` instead of `"10MB"`). The schema cannot express that
248-
/// union, so the overlay names it and codegen attaches a tolerant deserializer to the generated
249-
/// field.
250-
#[derive(Debug, Clone, Copy, PartialEq, Eq, Deserialize)]
251-
#[serde(rename_all = "snake_case")]
252-
pub enum InputShape {
253-
/// Accept a string unchanged, or a non-negative integer normalized to its decimal string.
254-
StringOrInteger,
255-
}
256-
257238
/// File paths to the two YAML files required as input by this library.
258239
///
259240
/// Defaults to the canonical location of the required schema files in this library.

lib/datadog-agent/config/build/datadog_config_gen.rs

Lines changed: 127 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,15 @@
2020
//! Vec<String>>` may carry each map value as either one scalar string or a sequence. Handling those
2121
//! shapes at the deserialization boundary keeps downstream types consistent; see `stringlistize`
2222
//! and `crate::list_de`.
23+
//!
24+
//! Scalar leaves get the Agent's own type coercion for the same reason: the Agent reads every
25+
//! setting through a cast against its declared type, so each leaf must accept the spellings that
26+
//! cast accepts. See `permissivize` and `crate::cast_de`.
2327
use std::collections::{BTreeMap, HashMap, HashSet};
2428
use std::path::Path;
2529

2630
use datadog_agent_config_overlay_model::schema_gen::{FieldInfo, FieldType};
27-
use datadog_agent_config_overlay_model::{load_resolved_schema, InputShape, KnownEntry, SchemaOverlay};
31+
use datadog_agent_config_overlay_model::{load_resolved_schema, KnownEntry, SchemaOverlay};
2832
use indexmap::IndexMap;
2933
use serde_json::{Map, Value};
3034
use syn::visit_mut::{self, VisitMut};
@@ -75,8 +79,7 @@ pub fn generate(
7579
let pruned_schema = Value::Object(root);
7680

7781
let aliases = field_aliases(overlay);
78-
let input_shapes = field_input_shapes(overlay);
79-
let body = render(pruned_schema, &aliases, &durations, &input_shapes);
82+
let body = render(pruned_schema, &aliases, &durations);
8083

8184
let mut out = String::new();
8285
out.push_str("// @generated by build.rs from core_schema.yaml + schema_overlay.yaml — DO NOT EDIT\n");
@@ -111,22 +114,6 @@ fn field_aliases(overlay: &SchemaOverlay) -> HashMap<String, Vec<String>> {
111114
map
112115
}
113116

114-
/// Collect `dotted key -> InputShape` for every supported entry that declares `input_shape`.
115-
fn field_input_shapes(overlay: &SchemaOverlay) -> BTreeMap<String, InputShape> {
116-
let mut map = BTreeMap::new();
117-
for (key, entry) in &overlay.inventory {
118-
let shape = match entry {
119-
KnownEntry::Full(f) => f.input_shape,
120-
KnownEntry::Partial(p) => p.input_shape,
121-
_ => None,
122-
};
123-
if let Some(shape) = shape {
124-
map.insert(key.clone(), shape);
125-
}
126-
}
127-
map
128-
}
129-
130117
/// Collect the dotted paths of every `support: full` / `support: partial` overlay entry.
131118
fn supported_keys(overlay: &SchemaOverlay) -> HashSet<String> {
132119
overlay
@@ -223,10 +210,7 @@ fn duration_default_nanos(info: Option<&FieldInfo>, path: &str) -> u64 {
223210
}
224211

225212
/// Run typify over the pruned schema and pretty-print the generated module body.
226-
fn render(
227-
pruned_schema: Value, aliases: &HashMap<String, Vec<String>>, durations: &BTreeMap<String, u64>,
228-
input_shapes: &BTreeMap<String, InputShape>,
229-
) -> String {
213+
fn render(pruned_schema: Value, aliases: &HashMap<String, Vec<String>>, durations: &BTreeMap<String, u64>) -> String {
230214
let root_schema: schemars::schema::RootSchema =
231215
serde_json::from_value(pruned_schema).expect("pruned schema is not a valid JSON Schema document");
232216

@@ -258,7 +242,7 @@ fn render(
258242
stringlistize(&mut file);
259243
strip_section_prefixes(&mut file);
260244
durationize(&mut file, durations);
261-
inject_input_shapes(&mut file, input_shapes);
245+
permissivize(&mut file);
262246

263247
let rendered = blank_lines_between_fields(&prettyplease::unparse(&file));
264248
let rendered = blank_lines_between_items(&rendered);
@@ -566,18 +550,20 @@ fn option_section_inner(ty: &syn::Type) -> Option<syn::Type> {
566550
}
567551
}
568552

569-
/// Attach the string-or-integer tolerant deserializer to every leaf whose overlay entry declares
570-
/// `input_shape: string_or_integer`.
553+
/// Give every scalar leaf the coercion the Agent applies when it reads that leaf's declared type.
571554
///
572-
/// The target leaf is located by its full dotted path, navigating section structs from the root, so
573-
/// two like-named leaves in different sections never collide (unlike a bare field-name match). The
574-
/// leaf must be a plain `String` field; anything else is an overlay/schema mismatch and fails the
575-
/// build. Runs after `strip_section_prefixes`, so section structs already carry their bare names.
576-
fn inject_input_shapes(file: &mut syn::File, input_shapes: &BTreeMap<String, InputShape>) {
577-
if input_shapes.is_empty() {
578-
return;
579-
}
580-
555+
/// The Agent casts a stored value to the accessor's type, so a leaf's permissiveness follows from its
556+
/// schema type alone and needs no per-key metadata: `crate::cast_de` holds one coercion per type and
557+
/// this attaches it by the leaf's generated Rust type, which typify derived from that schema type.
558+
///
559+
/// Every field is classified, and an unrecognized shape fails the build. A schema change that
560+
/// introduces a new leaf type must then decide how that type coerces instead of silently shipping a
561+
/// leaf that rejects input the Agent accepts. Runs after `durationize` and `stringlistize`, whose
562+
/// leaves carry their own shape-tolerant readers.
563+
// TODO: a leaf the Agent reads through an accessor of a different type than the schema declares
564+
// cannot be resolved from the schema type alone, and would need per-key overlay metadata. Add it back
565+
// if such a leaf turns up.
566+
fn permissivize(file: &mut syn::File) {
581567
let struct_names: HashSet<String> = file
582568
.items
583569
.iter()
@@ -587,26 +573,114 @@ fn inject_input_shapes(file: &mut syn::File, input_shapes: &BTreeMap<String, Inp
587573
})
588574
.collect();
589575

590-
for (dotted, shape) in input_shapes {
591-
let (owner_struct, leaf_field) = resolve_owner_and_leaf(file, &struct_names, dotted);
592-
let field = find_field_mut(file, &owner_struct, &leaf_field).unwrap_or_else(|| {
593-
panic!("input_shape key `{dotted}` resolves to unknown field `{owner_struct}.{leaf_field}`")
594-
});
595-
match shape {
596-
InputShape::StringOrInteger => {
597-
assert!(
598-
is_plain_string(&field.ty),
599-
"input_shape `string_or_integer` on `{dotted}`, but its generated field is not a plain \
600-
`String`; this metadata only applies to schema-string leaves"
601-
);
602-
field.attrs.push(parse_quote!(
603-
#[serde(deserialize_with = "crate::string_de::deserialize_string_or_integer")]
604-
));
605-
}
576+
for item in &mut file.items {
577+
let Item::Struct(s) = item else { continue };
578+
let syn::Fields::Named(fields) = &mut s.fields else {
579+
continue;
580+
};
581+
for field in &mut fields.named {
582+
let name = field.ident.as_ref().expect("a named field has an identifier");
583+
let deserializer = match leaf_kind(&field.ty, &struct_names) {
584+
LeafKind::Bool => "crate::cast_de::deserialize_bool",
585+
LeafKind::Integer => "crate::cast_de::deserialize_i64",
586+
LeafKind::Number => "crate::cast_de::deserialize_f64",
587+
LeafKind::Text => "crate::cast_de::deserialize_string",
588+
LeafKind::OptionalText => "crate::cast_de::deserialize_optional_string",
589+
LeafKind::Exempt => continue,
590+
LeafKind::Unknown => panic!(
591+
"field `{}.{name}` has no declared coercion; classify its type in `leaf_kind` and \
592+
give that type a coercion in `crate::cast_de`",
593+
s.ident
594+
),
595+
};
596+
field
597+
.attrs
598+
.push(parse_quote!(#[serde(deserialize_with = #deserializer)]));
606599
}
607600
}
608601
}
609602

603+
/// How one generated field accepts input.
604+
enum LeafKind {
605+
Bool,
606+
Integer,
607+
Number,
608+
Text,
609+
OptionalText,
610+
/// A nested section, or a leaf whose shape another pass or its own consumer handles.
611+
Exempt,
612+
Unknown,
613+
}
614+
615+
/// Classify a generated field by the type typify derived from its schema type.
616+
fn leaf_kind(ty: &syn::Type, struct_names: &HashSet<String>) -> LeafKind {
617+
if section_struct_name(ty, struct_names).is_some() {
618+
return LeafKind::Exempt;
619+
}
620+
if is_plain_string(ty) {
621+
return LeafKind::Text;
622+
}
623+
if option_inner(ty).is_some_and(is_plain_string) {
624+
return LeafKind::OptionalText;
625+
}
626+
if is_vec_string(ty) || is_string_map_vec_string(ty) || is_json_container(ty) || is_duration(ty) {
627+
return LeafKind::Exempt;
628+
}
629+
match plain_ident(ty) {
630+
Some(ident) if ident == "bool" => LeafKind::Bool,
631+
Some(ident) if ident == "i64" => LeafKind::Integer,
632+
Some(ident) if ident == "f64" => LeafKind::Number,
633+
_ => LeafKind::Unknown,
634+
}
635+
}
636+
637+
/// The final path segment of a type carrying no generic arguments (`bool`, `i64`, `String`, ...).
638+
fn plain_ident(ty: &syn::Type) -> Option<&syn::Ident> {
639+
let syn::Type::Path(tp) = ty else { return None };
640+
let seg = tp.path.segments.last()?;
641+
matches!(seg.arguments, syn::PathArguments::None).then_some(&seg.ident)
642+
}
643+
644+
/// If `ty` is `Option<T>`, return `T`.
645+
fn option_inner(ty: &syn::Type) -> Option<&syn::Type> {
646+
let syn::Type::Path(tp) = ty else { return None };
647+
let last = tp.path.segments.last()?;
648+
if last.ident != "Option" {
649+
return None;
650+
}
651+
let syn::PathArguments::AngleBracketed(args) = &last.arguments else {
652+
return None;
653+
};
654+
match args.args.first()? {
655+
syn::GenericArgument::Type(inner) => Some(inner),
656+
_ => None,
657+
}
658+
}
659+
660+
/// Returns whether `ty` is a raw JSON container, the shape an object-typed or heterogeneous-array
661+
/// leaf keeps so its own consumer can interpret it.
662+
fn is_json_container(ty: &syn::Type) -> bool {
663+
let syn::Type::Path(tp) = ty else { return false };
664+
if tp.path.segments.iter().any(|seg| seg.ident == "serde_json") {
665+
return true;
666+
}
667+
let Some(last) = tp.path.segments.last() else {
668+
return false;
669+
};
670+
if last.ident != "Vec" {
671+
return false;
672+
}
673+
let syn::PathArguments::AngleBracketed(args) = &last.arguments else {
674+
return false;
675+
};
676+
matches!(args.args.first(), Some(syn::GenericArgument::Type(inner)) if is_json_container(inner))
677+
}
678+
679+
/// Returns whether `ty` is the `std::time::Duration` that `durationize` installs.
680+
fn is_duration(ty: &syn::Type) -> bool {
681+
plain_ident(ty).is_some_and(|ident| ident == "Duration")
682+
}
683+
610684
/// Resolve a dotted key to the `(owner struct name, leaf field name)` in the generated tree.
611685
///
612686
/// Every non-final segment is a non-optional nested section struct (its type names one of the
@@ -620,9 +694,9 @@ fn resolve_owner_and_leaf(file: &syn::File, struct_names: &HashSet<String>, dott
620694
return (current, (*segment).to_string());
621695
}
622696
let field = find_field(file, &current, segment)
623-
.unwrap_or_else(|| panic!("input_shape key `{dotted}`: field `{segment}` not found on struct `{current}`"));
697+
.unwrap_or_else(|| panic!("key `{dotted}`: field `{segment}` not found on struct `{current}`"));
624698
current = section_struct_name(&field.ty, struct_names)
625-
.unwrap_or_else(|| panic!("input_shape key `{dotted}`: segment `{segment}` is not a nested section"));
699+
.unwrap_or_else(|| panic!("key `{dotted}`: segment `{segment}` is not a nested section"));
626700
}
627701

628702
unreachable!("a dotted key always has a final segment");

lib/datadog-agent/config/schema/schema_overlay.yaml

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -883,7 +883,6 @@ inventory:
883883
support: full
884884
pipelines: [dogstatsd]
885885
description: "DSD log file max size"
886-
input_shape: string_or_integer
887886
test_support:
888887
used_by: [DogStatsDDebugLogConfiguration]
889888
test_json: '"42MB"'
@@ -1741,7 +1740,6 @@ inventory:
17411740
support: full
17421741
pipelines: [cross_cutting]
17431742
description: "Max log file size before rolling"
1744-
input_shape: string_or_integer
17451743
test_support:
17461744
used_by: [NO_SMOKE]
17471745

0 commit comments

Comments
 (0)