From ddab62059c157e101d0b6f2d909ce4044baf69f5 Mon Sep 17 00:00:00 2001 From: Simon Gerber Date: Wed, 29 Apr 2026 14:07:42 +0200 Subject: [PATCH 1/2] Preserve `null` for reference default value defined as reference Until now, a reference with a default value which is a reference that resolves to a YAML `null` value would result in a YAML string `"None"` when the resolution falls back on the default value. The cause for this is that we serialized `Value::Null` to `"None"` until now to preserve compatibility with Python's `str()` for nested reference lookups (since Python Reclass just uses `str()` to serialize nested references). This commit changes the serialization of `Value::Null` to `"null"` which is preserved as a YAML `null` when a reference default value is defined as a nested reference that resolves to YAML `null`. Conversely, any existing nested references that expect YAML `null` to be serialized as `None` will break with this change. --- src/types/value.rs | 6 ++++-- src/types/value/value_tests.rs | 2 +- .../nodes/n12.yml | 13 +++++++++++++ tests/test_inventory_reference_default_values.py | 7 +++++++ 4 files changed, 25 insertions(+), 3 deletions(-) create mode 100644 tests/inventory-reference-default-values/nodes/n12.yml diff --git a/src/types/value.rs b/src/types/value.rs index f8aa736..34349f3 100644 --- a/src/types/value.rs +++ b/src/types/value.rs @@ -484,8 +484,10 @@ impl Value { pub(crate) fn raw_string(&self) -> Result { match self { Value::Literal(s) => Ok(s.clone()), - // We serialize Null as `None` to be compatible with Python's str() - Value::Null => Ok("None".to_string()), + // We serialize Null as `null`. NOTE(sg): This isn't compatible with Python's str(), + // but ensures that a reclass-rs reference default value which is a reference that + // resolves to a Null value is preserved correctly. + Value::Null => Ok("null".to_string()), // We need custom formatting for bool instead of `format!("{b}")`, so that this // function returns strings which match Python's `str()` implementation. Value::Bool(b) => match b { diff --git a/src/types/value/value_tests.rs b/src/types/value/value_tests.rs index a4b50ac..952f03d 100644 --- a/src/types/value/value_tests.rs +++ b/src/types/value/value_tests.rs @@ -301,7 +301,7 @@ fn test_raw_string_literal() { #[test] fn test_raw_string_null() { - assert_eq!(Value::Null.raw_string().unwrap(), "None".to_string()); + assert_eq!(Value::Null.raw_string().unwrap(), "null".to_string()); } #[test] diff --git a/tests/inventory-reference-default-values/nodes/n12.yml b/tests/inventory-reference-default-values/nodes/n12.yml new file mode 100644 index 0000000..9a2e5fd --- /dev/null +++ b/tests/inventory-reference-default-values/nodes/n12.yml @@ -0,0 +1,13 @@ +parameters: + facts: + level: level + data: + "null": null + nonestr: None + bool: true + num: 1.25 + nullref: ${data:${facts:level}::${data:null}} + nulldefault: ${data:${facts:level}::~} + boolref: ${data:${facts:level}::${data:bool}} + numref: ${data:${facts:level}::${data:num}} + nonestrref: ${data:${facts:level}::${data:nonestr}} diff --git a/tests/test_inventory_reference_default_values.py b/tests/test_inventory_reference_default_values.py index 8c43129..5d33f3c 100644 --- a/tests/test_inventory_reference_default_values.py +++ b/tests/test_inventory_reference_default_values.py @@ -61,3 +61,10 @@ def test_inventory_reference_default_values(): n11 = inv["nodes"]["n11"]["parameters"] assert n11["text"] == n11["expected"] + + n12 = inv["nodes"]["n12"]["parameters"] + assert n12["nullref"] is None + assert n12["nulldefault"] is None + assert n12["boolref"] + assert n12["numref"] == 1.25 + assert n12["nonestrref"] == "None" From 0b4365a37831748223651472783baf8cf1b074dc Mon Sep 17 00:00:00 2001 From: Simon Gerber Date: Wed, 29 Apr 2026 14:38:40 +0200 Subject: [PATCH 2/2] Introduce `NestedReferenceNullAsNone` compatibility flag This compat flag changes reclass-rs's serialization of YAML `null` values in nested references to emit string "None" instead of the new default of string "null". Unless your inventory depends on this behavior we strongly recommend not using the compatibility mode. --- README-extensions.md | 10 ++++- README.md | 1 + python/reclass_rs/reclass_rs.pyi | 1 + src/config.rs | 15 +++++++ src/node/mod.rs | 2 +- src/refs/mod.rs | 8 ++-- src/types/mapping.rs | 4 +- src/types/value.rs | 17 +++++--- src/types/value/value_flattened_tests.rs | 3 +- src/types/value/value_tests.rs | 42 ++++++++++++++----- ...test_inventory_reference_default_values.py | 25 +++++++++++ 11 files changed, 103 insertions(+), 25 deletions(-) diff --git a/README-extensions.md b/README-extensions.md index 3766197..7ee008e 100644 --- a/README-extensions.md +++ b/README-extensions.md @@ -21,9 +21,17 @@ For example, given the following inventory, the node's internal path is parsed a ``` However, optionally, reclass-rs can be configured to handle `compose_node_name` the same way that kapicorp-reclass does, by naively splitting node names on each dot. -To enable this compatibility mode, set `compat_flags: ['ComposeNodeNameLiteralDots']` in your inventory's `reclass-config.yml`. +To enable this compatibility mode, set `reclass_rs_compat_flags: ['ComposeNodeNameLiteralDots']` in your inventory's `reclass-config.yml`. In compatibility mode, the node's internal path for the previous inventory is `['path', 'to', 'the', 'node']`. +## Handling of YAML `null` values in nested references + +By default, reclass-rs resolves YAML `null` values in nested references to the string `null`. +This behavior ensures that references which resolve to YAML `null` are correctly preserved when using them as reference default values (see below). + +Optionally, reclass-rs can be configured to preserve the kapicorp-reclass behavior of resolving YAML `null` values in nested references to string "None". +To enable this compatibility mode, set `reclass_rs_compat_flags: ['NestedReferenceNullAsNone']` in your inventory's `reclass-config.yml`. + ## Verbose warnings Reclass-rs supports boolean config option `verbose_warnings`. diff --git a/README.md b/README.md index 7a97d92..ee0d446 100644 --- a/README.md +++ b/README.md @@ -19,6 +19,7 @@ The implementation currently supports the following features of Kapicorp Reclass * Merging referenced lists and dictionaries * Constant parameters * Nested references + * By default reclass-rs uses a non-compatible mode which resolves YAML `null` values in nested references to string "null". * References in class names * Loading classes with relative names * Loading Reclass configuration options from `reclass-config.yaml` diff --git a/python/reclass_rs/reclass_rs.pyi b/python/reclass_rs/reclass_rs.pyi index dd0de04..281be08 100644 --- a/python/reclass_rs/reclass_rs.pyi +++ b/python/reclass_rs/reclass_rs.pyi @@ -15,6 +15,7 @@ __all__: list[str] = [ @final class CompatFlag(Enum): ComposeNodeNameLiteralDots = "ComposeNodeNameLiteralDots" + NestedReferenceNullAsNone = "NestedReferenceNullAsNone" @final class Config: diff --git a/src/config.rs b/src/config.rs index ccec795..cb75292 100644 --- a/src/config.rs +++ b/src/config.rs @@ -25,6 +25,13 @@ pub enum CompatFlag { /// file path when rendering fields `path` and `parts` in `NodeInfoMeta` when /// `compose-node-name` is enabled. ComposeNodeNameLiteralDots, + /// This flag enables Python Reclass-compatible rendering of nested references that resolve to + /// a YAML `null` value. When this flag is set, YAML `null` values encountered during nested + /// reference resolution will be serialized as `"None"`. + /// + /// By default, if this flag isn't enabled, reclass-rs will serialize YAML `null` values as + /// string `"null"` during nested reference resolution. + NestedReferenceNullAsNone, } #[pymethods] @@ -43,6 +50,9 @@ impl TryFrom<&str> for CompatFlag { "compose-node-name-literal-dots" | "compose_node_name_literal_dots" | "ComposeNodeNameLiteralDots" => Ok(Self::ComposeNodeNameLiteralDots), + "nested-reference-null-as-none" + | "nested_reference_null_as_none" + | "NestedReferenceNullAsNone" => Ok(Self::NestedReferenceNullAsNone), _ => Err(anyhow!("Unknown compatibility flag '{value}'")), } } @@ -166,11 +176,13 @@ impl ClassMapping { } } +#[allow(clippy::struct_excessive_bools)] #[derive(Clone, Debug, Default)] pub struct RenderOpts { pub ignore_overwritten_missing_references: bool, pub verbose_warnings: bool, pub(crate) preserve_resolve_error_in_flattened: bool, + pub(crate) nested_reference_null_as_none: bool, } #[pyclass(from_py_object)] @@ -490,6 +502,9 @@ impl From<&Config> for RenderOpts { Self { ignore_overwritten_missing_references: value.ignore_overwritten_missing_references, verbose_warnings: value.verbose_warnings, + nested_reference_null_as_none: value + .compatflags + .contains(&CompatFlag::NestedReferenceNullAsNone), ..Default::default() } } diff --git a/src/node/mod.rs b/src/node/mod.rs index d8b566e..838bf63 100644 --- a/src/node/mod.rs +++ b/src/node/mod.rs @@ -234,7 +234,7 @@ impl Node { let mut state = ResolveState::default(); clstoken .render(&root.parameters, &mut state, &r.config.get_render_opts())? - .raw_string()? + .raw_string(&r.config.get_render_opts())? } else { // If Token::parse() returns None, the class name can't contain any references, // just convert cls into an owned String. diff --git a/src/refs/mod.rs b/src/refs/mod.rs index 1304884..9708121 100644 --- a/src/refs/mod.rs +++ b/src/refs/mod.rs @@ -46,8 +46,8 @@ impl ResolveState { /// Pushes mapping key into the `current_keys` list. If possible, the provided value is /// formatted with `raw_string()`. Additionally, unprocessed `String` values are pushed as-is. /// This function will return an error when it's called with a `Value::ValueList`. - pub(crate) fn push_mapping_key(&mut self, key: &Value) -> Result<()> { - let kstr = match key.raw_string() { + pub(crate) fn push_mapping_key(&mut self, key: &Value, opts: &RenderOpts) -> Result<()> { + let kstr = match key.raw_string(opts) { Ok(s) => s, Err(_) => match key { Value::String(s) => Ok(s.clone()), @@ -173,7 +173,7 @@ impl Token { .interpolate(params, state, opts) } else { Ok(Value::Literal( - self.resolve(params, state, opts)?.raw_string()?, + self.resolve(params, state, opts)?.raw_string(opts)?, )) } } @@ -374,7 +374,7 @@ fn interpolate_token_slice( while v.is_string() { v = v.interpolate(params, &mut st, opts)?; } - res.push_str(&v.raw_string()?); + res.push_str(&v.raw_string(opts)?); } Ok(res) } diff --git a/src/types/mapping.rs b/src/types/mapping.rs index d16403e..d3a5bc5 100644 --- a/src/types/mapping.rs +++ b/src/types/mapping.rs @@ -401,7 +401,7 @@ impl Mapping { && let Some(p) = p { let mut st = state.clone(); - st.push_mapping_key(k)?; + st.push_mapping_key(k, opts)?; if let Some(errmsg) = p.as_resolve_error() { if opts.ignore_overwritten_missing_references { #[cfg(not(feature = "bench"))] @@ -476,7 +476,7 @@ impl Mapping { // either manage to interpolate a value (in which case it doesn't contain a loop) or we // don't and the whole interpolation is aborted. let mut st = state.clone(); - st.push_mapping_key(k)?; + st.push_mapping_key(k, opts)?; let iv = v.interpolate(root, &mut st, opts); let v = if let Err(e) = iv { // convert interpolation errors into `Value::ResolveError` when interpolating diff --git a/src/types/value.rs b/src/types/value.rs index 34349f3..77cc0d2 100644 --- a/src/types/value.rs +++ b/src/types/value.rs @@ -481,13 +481,20 @@ impl Value { /// strings which match Python's `str()` for other types. /// #[inline] - pub(crate) fn raw_string(&self) -> Result { + pub(crate) fn raw_string(&self, opts: &RenderOpts) -> Result { match self { Value::Literal(s) => Ok(s.clone()), - // We serialize Null as `null`. NOTE(sg): This isn't compatible with Python's str(), - // but ensures that a reclass-rs reference default value which is a reference that - // resolves to a Null value is preserved correctly. - Value::Null => Ok("null".to_string()), + // We serialize Null as `null` unless the `NestedReferenceNullAsNone` compat flag is + // set, in which case we serialize Null as `None`. The default serialization isn't + // compatible with Python's str(), but ensures that a reclass-rs reference default + // value which is a reference that resolves to a Null value is preserved correctly. + Value::Null => { + if opts.nested_reference_null_as_none { + Ok("None".to_string()) + } else { + Ok("null".to_string()) + } + } // We need custom formatting for bool instead of `format!("{b}")`, so that this // function returns strings which match Python's `str()` implementation. Value::Bool(b) => match b { diff --git a/src/types/value/value_flattened_tests.rs b/src/types/value/value_flattened_tests.rs index dc90744..f5fdddc 100644 --- a/src/types/value/value_flattened_tests.rs +++ b/src/types/value/value_flattened_tests.rs @@ -42,7 +42,8 @@ test_flattened_simple! { fn test_flattened_string() { let v = Value::String("foo".into()); let mut st = ResolveState::default(); - st.push_mapping_key(&"test".into()).unwrap(); + st.push_mapping_key(&"test".into(), &RenderOpts::default()) + .unwrap(); v.flattened(&mut st, &RenderOpts::default()).unwrap(); } diff --git a/src/types/value/value_tests.rs b/src/types/value/value_tests.rs index 952f03d..b8b827c 100644 --- a/src/types/value/value_tests.rs +++ b/src/types/value/value_tests.rs @@ -294,45 +294,65 @@ fn test_strip_prefix() { #[test] fn test_raw_string_literal() { assert_eq!( - Value::Literal("foo".into()).raw_string().unwrap(), + Value::Literal("foo".into()) + .raw_string(&RenderOpts::default()) + .unwrap(), "foo".to_string() ); } #[test] fn test_raw_string_null() { - assert_eq!(Value::Null.raw_string().unwrap(), "null".to_string()); + assert_eq!( + Value::Null.raw_string(&RenderOpts::default()).unwrap(), + "null".to_string() + ); +} + +#[test] +fn test_raw_string_null_as_none() { + let opts = RenderOpts { + nested_reference_null_as_none: true, + ..RenderOpts::default() + }; + assert_eq!(Value::Null.raw_string(&opts).unwrap(), "None".to_string()); } #[test] fn test_raw_string_number() { assert_eq!( - Value::Number(5.into()).raw_string().unwrap(), + Value::Number(5.into()) + .raw_string(&RenderOpts::default()) + .unwrap(), "5".to_string() ); assert_eq!( - Value::Number((-1).into()).raw_string().unwrap(), + Value::Number((-1).into()) + .raw_string(&RenderOpts::default()) + .unwrap(), "-1".to_string() ); assert_eq!( - Value::Number(3.14.into()).raw_string().unwrap(), + Value::Number(3.14.into()) + .raw_string(&RenderOpts::default()) + .unwrap(), "3.14".to_string() ); assert_eq!( Value::Number(serde_yaml::Number::from(f64::INFINITY)) - .raw_string() + .raw_string(&RenderOpts::default()) .unwrap(), ".inf".to_string() ); assert_eq!( Value::Number(serde_yaml::Number::from(f64::NEG_INFINITY)) - .raw_string() + .raw_string(&RenderOpts::default()) .unwrap(), "-.inf".to_string() ); assert_eq!( Value::Number(serde_yaml::Number::from(f64::NAN)) - .raw_string() + .raw_string(&RenderOpts::default()) .unwrap(), ".nan".to_string() ); @@ -343,7 +363,7 @@ fn test_raw_string_mapping() { let mut m = Value::Mapping(Mapping::from_str("{foo: foo, bar: true, baz: 1.23}").unwrap()); // turn string values into literals by calling flatten m.render(&Mapping::new(), &RenderOpts::default()).unwrap(); - let mstr = m.raw_string().unwrap(); + let mstr = m.raw_string(&RenderOpts::default()).unwrap(); // NOTE(sg): serde_json output is sorted by keys assert_eq!(mstr, r#"{"bar":true,"baz":1.23,"foo":"foo"}"#); } @@ -351,7 +371,7 @@ fn test_raw_string_mapping() { #[test] fn test_raw_string_sequence() { let v = Value::Sequence(vec!["foo".into(), 3.14.into(), Value::Bool(true)]); - let vstr = v.raw_string().unwrap(); + let vstr = v.raw_string(&RenderOpts::default()).unwrap(); assert_eq!(vstr, r#"["foo",3.14,true]"#); } @@ -364,7 +384,7 @@ fn test_raw_string_mapping_nonstring_keys() { let m = Value::Mapping(m) .rendered(&Mapping::new(), &RenderOpts::default()) .unwrap(); - let mstr = m.raw_string().unwrap(); + let mstr = m.raw_string(&RenderOpts::default()).unwrap(); // NOTE(sg): serde_json output is sorted by keys assert_eq!(mstr, r#"{"3.14":true,"null":1.23,"true":"foo"}"#); } diff --git a/tests/test_inventory_reference_default_values.py b/tests/test_inventory_reference_default_values.py index 5d33f3c..e4d3e10 100644 --- a/tests/test_inventory_reference_default_values.py +++ b/tests/test_inventory_reference_default_values.py @@ -68,3 +68,28 @@ def test_inventory_reference_default_values(): assert n12["boolref"] assert n12["numref"] == 1.25 assert n12["nonestrref"] == "None" + + +def test_inventory_reference_default_values_null_as_none(): + config_options = { + "nodes_uri": "nodes", + "classes_uri": "classes", + "reclass_rs_compat_flags": ["NestedReferenceNullAsNone"], + } + c = reclass_rs.Config.from_dict( + "./tests/inventory-reference-default-values", config_options + ) + assert c is not None + + r = reclass_rs.Reclass.from_config(c) + assert r is not None + + inv = r.inventory().as_dict() + n12 = inv["nodes"]["n12"]["parameters"] + # With NestedReferenceNullAsNone compat mode the nested ref to `data.null` is resolved as + # string "None" which isn't recognized as a YAML `null` by reclass-rs's default value handling. + assert n12["nullref"] == "None" + assert n12["nulldefault"] is None + assert n12["boolref"] + assert n12["numref"] == 1.25 + assert n12["nonestrref"] == "None"